fix(ci): make the credential scrub failure-safe, and stop the lists drifting

Round 5, and the best findings are again about the fix from round 4.

The scrub only ran on success. `set -e` aborts the step when `go mod download`
fails, so the cleanup line after it never executed — leaving a push-capable
credential on a long-lived self-hosted runner for whatever job landed there
next. It is now a `trap ... EXIT`, verified against a simulated failure.

It also scrubbed the wrong file in principle: `git config --global` writes to
GIT_CONFIG_GLOBAL, else $XDG_CONFIG_HOME/git/config when that exists, else
~/.gitconfig — so deleting ~/.gitconfig can scrub a path the credential was
never in. The step now names GIT_CONFIG_GLOBAL itself, leaving exactly one file
to remove.

And the verification failed open in the case that matters most: `grep -F ""`
matches every file, so a run WITHOUT the secret — a fork PR, the threat model —
failed the check with a message accusing it of leaking a credential it never
had. Guarded on a non-empty secret.

Credentials move to an Authorization header instead of being embedded in the
URL, so a password containing @ : / or # can no longer break URL parsing in a
way that reads as a bad password.

Two list-drift holes closed with one test that reads across languages:
TestOpenAICompatProvidersAreFullyWired asserts every openAICompatProviders
entry is both advertised in endpointProviderNames and has a credential arm in
scripts/preflight.sh. Adding a compat provider touches three places in two
languages and nothing connected them. Break-checked in both directions.

Finally, a whitespace-only GADFLY_BASE_URL disagreed across the boundary: Go
TrimSpaces it and takes the registry path, bash called it "set" and skipped the
pre-flight, so the missing key arrived as a bare 401 with no notice. Both now
agree on what unset means.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

.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 —
+          rm -f "$GIT_CONFIG_GLOBAL"
-          # checking only the file just deleted would pass no matter what, and
+          test ! -e "$GIT_CONFIG_GLOBAL"
-          # the credential can also reach ~/.netrc or ~/.config/go/env.
+
-          test ! -e "$HOME/.gitconfig"
+          # Prove the scrub across the whole home dir, not just the file we
-          if grep -rqF "${REGISTRY_PASSWORD}" "$HOME" 2>/dev/null; then
+          # 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

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)
+		}
+	}
+}

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

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