diff --git a/.gitea/workflows/build-image.yml b/.gitea/workflows/build-image.yml index 634e9f0..fb8ddd9 100644 --- a/.gitea/workflows/build-image.yml +++ b/.gitea/workflows/build-image.yml @@ -70,15 +70,39 @@ jobs: REGISTRY_USER: ${{ secrets.REGISTRY_USER }} REGISTRY_PASSWORD: ${{ secrets.REGISTRY_PASSWORD }} run: | + set -euo pipefail + + # Own the config path outright. `git config --global` writes to + # GIT_CONFIG_GLOBAL, else $XDG_CONFIG_HOME/git/config when that + # directory exists, else ~/.gitconfig — so "delete ~/.gitconfig" + # scrubs a file the credential may never have been in. Naming the + # path leaves exactly one file to remove. + export GIT_CONFIG_GLOBAL="$(mktemp)" + + # Scrub on ANY exit, not just success. `set -e` means a failed + # `go mod download` aborts this step, and a cleanup written as the + # next line would never run — leaving a push-capable credential on a + # long-lived self-hosted runner for whatever job lands there next. + trap 'rm -f "$GIT_CONFIG_GLOBAL"' EXIT + go env -w GOPRIVATE=gitea.stevedudenhoeffer.com/* - git config --global url."https://${REGISTRY_USER}:${REGISTRY_PASSWORD}@gitea.stevedudenhoeffer.com/".insteadOf "https://gitea.stevedudenhoeffer.com/" + # Basic-auth header rather than credentials inside the URL: a + # password containing @ : / or # breaks URL parsing, and the failure + # would look like a bad password rather than a quoting bug. + git config --global \ + "http.https://gitea.stevedudenhoeffer.com/.extraheader" \ + "Authorization: Basic $(printf '%s:%s' "$REGISTRY_USER" "$REGISTRY_PASSWORD" | base64 | tr -d '\n')" go mod download - rm -f "$HOME/.gitconfig" - # Prove the scrub worked, and prove it against the whole home dir — - # checking only the file just deleted would pass no matter what, and - # the credential can also reach ~/.netrc or ~/.config/go/env. - test ! -e "$HOME/.gitconfig" - if grep -rqF "${REGISTRY_PASSWORD}" "$HOME" 2>/dev/null; then + + rm -f "$GIT_CONFIG_GLOBAL" + test ! -e "$GIT_CONFIG_GLOBAL" + + # Prove the scrub across the whole home dir, not just the file we + # deleted — that check would pass no matter what, and git/go can also + # write ~/.netrc or ~/.config/go/env. Guarded on a non-empty secret: + # `grep -F ""` matches every file, so a secretless run (fork PR) would + # fail here with a message accusing it of leaking nothing. + if [ -n "${REGISTRY_PASSWORD:-}" ] && grep -rqF "$REGISTRY_PASSWORD" "$HOME" 2>/dev/null; then echo "::error::registry credential still present under \$HOME after scrub" exit 1 fi diff --git a/cmd/gadfly/model_test.go b/cmd/gadfly/model_test.go index 4cf4145..20d42d1 100644 --- a/cmd/gadfly/model_test.go +++ b/cmd/gadfly/model_test.go @@ -1,6 +1,9 @@ package main import ( + "os" + "path/filepath" + "regexp" "strings" "testing" ) @@ -156,3 +159,37 @@ func TestBuildSpec(t *testing.T) { }) } } + +// TestOpenAICompatProvidersAreFullyWired closes the two remaining ways this +// provider family can be half-added. Adding one means touching three places in +// two languages, and nothing but this test connects them: +// +// - endpointProviderNames is the operator-facing list. A provider the code +// accepts but the list omits sends someone debugging a name that works. +// - scripts/preflight.sh needs a credential arm, or a missing key for that +// provider skips the pre-flight and arrives as five unexplained per-lens +// failures — the exact thing the pre-flight exists to replace. +func TestOpenAICompatProvidersAreFullyWired(t *testing.T) { + advertised := make(map[string]bool) + for _, n := range strings.Split(endpointProviderNames, "/") { + advertised[strings.TrimSpace(n)] = true + } + + preflight, err := os.ReadFile(filepath.Join("..", "..", "scripts", "preflight.sh")) + if err != nil { + t.Fatalf("read preflight.sh: %v", err) + } + + for _, p := range openAICompatProviders { + if !advertised[p] { + t.Errorf("openAICompatProviders has %q but endpointProviderNames does not list it — "+ + "the error message operators read would omit a name that works", p) + } + // The arm may be shared ("openai|openai-compatible)"), so match the + // bare name as a case alternative rather than a whole line. + if !regexp.MustCompile(`(?m)^\s*(\w[\w-]*\|)*` + regexp.QuoteMeta(p) + `(\|[\w-]+)*\)`).Match(preflight) { + t.Errorf("openAICompatProviders has %q but scripts/preflight.sh has no credential arm for it — "+ + "a missing key for %s would skip the pre-flight and surface as unexplained lens failures", p, p) + } + } +} diff --git a/scripts/preflight.sh b/scripts/preflight.sh old mode 100644 new mode 100755 index 392a4ab..f8586ec --- a/scripts/preflight.sh +++ b/scripts/preflight.sh @@ -27,7 +27,13 @@ gadfly_preflight_key() { local provider="$1" key_env="" key_hint="" # Only the registry path has knowable credential rules — see above. - if [ -n "${GADFLY_BASE_URL:-}" ]; then + # Trim before testing: resolveModel does strings.TrimSpace on GADFLY_BASE_URL, + # so a whitespace-only value takes the REGISTRY path there. Testing the raw + # value here would call it "set", skip the check, and let the missing key + # arrive as a 401 with no notice — the two must agree on what "unset" means. + local base_url + base_url="$(printf '%s' "${GADFLY_BASE_URL:-}" | tr -d '[:space:]')" + if [ -n "$base_url" ]; then echo "" return 0 fi diff --git a/scripts/preflight_test.sh b/scripts/preflight_test.sh old mode 100644 new mode 100755 index baad214..a24fdea --- a/scripts/preflight_test.sh +++ b/scripts/preflight_test.sh @@ -79,6 +79,11 @@ done # GEMINI_API_KEY, so a one-name arm would skip a correctly-configured run. check "google w/ only GEMINI_API_KEY" "" "$(probe google GEMINI_API_KEY=k)" +echo "== a whitespace-only GADFLY_BASE_URL counts as unset, as it does in Go ==" +# resolveModel TrimSpaces it and takes the registry path; if this check +# disagreed, the missing key would arrive as a bare 401 with no skip notice. +check "qwen + blank BASE_URL" "QWEN_API_KEY" "$(probe qwen GADFLY_BASE_URL=" ")" + if [ "$fail" -ne 0 ]; then echo "RESULT: preflight table FAILED" exit 1