fix(proxy): select one credential per header name, independent of order

CHANGELOG.md

@@ -4,6 +4,12 @@ Gatekeeper is a standalone credential-injecting TLS-intercepting proxy. It trans
 
 Gatekeeper is pre-1.0. The configuration schema and credential source interface may change between minor versions.
 
+## Unreleased
+
+### Fixed
+
+- **A client placeholder no longer selects the wrong credential, and the canonical log line no longer over-reports grants** — `injectCredentials`' placeholder pass tested `req.Header.Get(c.Name)` *after* earlier iterations had already written that header, so when several credentials for a host shared a header name and the client sent a placeholder for it, every same-named credential passed the "client sent this" check. Three consequences: the last credential in config order silently won the wire regardless of which grant the placeholder meant to select; `Grants` recorded every same-named credential as injected, so audit logs named credentials that never left the proxy; and `Injected` (used for cache invalidation since v0.15.0) listed credentials the destination never saw. Client-sent headers are now sampled once, before any injection, and exactly one credential is chosen per header name by a tie-break that does not depend on config order. **Behavior change:** the two selection paths break the tie in opposite directions — auto-injection still prefers a non-`claude` grant, while placeholder selection now prefers `claude`, since the claude grant is Claude Code's OAuth flow and a client that explicitly sends the header is asking for exactly it. Previously a placeholder could never reliably select it ([#40](https://github.com/majorcontext/gatekeeper/issues/40))
+
 ## v0.15.0 — 2026-07-09
 
 ### Added

proxy/inject_credentials_test.go

@@ -0,0 +1,212 @@
+package proxy
+
+import (
+	"net/http"
+	"slices"
+	"sort"
+	"testing"
+)
+
+// grantsOf returns the grants injectCredentials recorded, sorted for comparison.
+func grantsOf(res credentialInjectionResult) []string {
+	g := slices.Clone(res.Grants)
+	sort.Strings(g)
+	return g
+}
+
+// injectedGrantsOf returns the grants of the credentials actually on the wire.
+func injectedGrantsOf(res credentialInjectionResult) []string {
+	var g []string
+	for _, c := range res.Injected {
+		g = append(g, c.Grant)
+	}
+	sort.Strings(g)
+	return g
+}
+
+func newInjectReq(t *testing.T, clientHeaders map[string]string) *http.Request {
+	t.Helper()
+	req, err := http.NewRequest("GET", "https://example.test/x", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for k, v := range clientHeaders {
+		req.Header.Set(k, v)
+	}
+	return req
+}
+
+// When several credentials share a header name and the client sends a
+// placeholder for it, exactly one credential may be injected. The "claude"
+// grant wins the placeholder path: it is OAuth, and per injectCredentials'
+// contract it should only be injected when Claude Code explicitly asks for it
+// by sending the header.
+func TestInjectCredentials_PlaceholderSameHeaderPrefersClaude(t *testing.T) {
+	creds := []credentialHeader{
+		{Name: "Authorization", Value: "Bearer other", Grant: "other"},
+		{Name: "Authorization", Value: "Bearer claude", Grant: "claude"},
+	}
+	req := newInjectReq(t, map[string]string{"Authorization": "placeholder"})
+
+	res := injectCredentials(req, creds, "example.test", "GET", "/x")
+
+	if got := req.Header.Get("Authorization"); got != "Bearer claude" {
+		t.Errorf("wire Authorization = %q, want %q", got, "Bearer claude")
+	}
+	if got := grantsOf(res); !slices.Equal(got, []string{"claude"}) {
+		t.Errorf("Grants = %v, want [claude] (only the injected credential)", got)
+	}
+	if got := injectedGrantsOf(res); !slices.Equal(got, []string{"claude"}) {
+		t.Errorf("Injected = %v, want [claude]", got)
+	}
+}
+
+// Slice order must not decide the winner. Same as above with the credentials
+// reversed: claude still wins.
+func TestInjectCredentials_PlaceholderWinnerIndependentOfOrder(t *testing.T) {
+	creds := []credentialHeader{
+		{Name: "Authorization", Value: "Bearer claude", Grant: "claude"},
+		{Name: "Authorization", Value: "Bearer other", Grant: "other"},
+	}
+	req := newInjectReq(t, map[string]string{"Authorization": "placeholder"})
+
+	res := injectCredentials(req, creds, "example.test", "GET", "/x")
+
+	if got := req.Header.Get("Authorization"); got != "Bearer claude" {
+		t.Errorf("wire Authorization = %q, want %q", got, "Bearer claude")
+	}
+	if got := grantsOf(res); !slices.Equal(got, []string{"claude"}) {
+		t.Errorf("Grants = %v, want [claude]", got)
+	}
+}
+
+// The first pass must test what the *client* sent, not what an earlier
+// iteration wrote. A credential whose header the client never sent must not be
+// injected just because a same-named credential was injected before it.
+func TestInjectCredentials_PlaceholderDoesNotSelectUnsentHeaders(t *testing.T) {
+	creds := []credentialHeader{
+		{Name: "Authorization", Value: "Bearer auth", Grant: "auth"},
+		{Name: "X-Api-Key", Value: "real-key", Grant: "apikey"},
+	}
+	req := newInjectReq(t, map[string]string{"Authorization": "placeholder"})
+
+	res := injectCredentials(req, creds, "example.test", "GET", "/x")
+
+	if got := req.Header.Get("Authorization"); got != "Bearer auth" {
+		t.Errorf("wire Authorization = %q, want %q", got, "Bearer auth")
+	}
+	if got := req.Header.Get("X-Api-Key"); got != "" {
+		t.Errorf("X-Api-Key = %q, want empty (client never sent it)", got)
+	}
+	if got := grantsOf(res); !slices.Equal(got, []string{"auth"}) {
+		t.Errorf("Grants = %v, want [auth]", got)
+	}
+	if res.InjectedHeaders["x-api-key"] {
+		t.Error("InjectedHeaders marks x-api-key injected, but it was not")
+	}
+}
+
+// Unchanged behavior: with no placeholder, auto-injection prefers the
+// non-claude grant on a shared header name.
+func TestInjectCredentials_AutoInjectPrefersNonClaude(t *testing.T) {
+	creds := []credentialHeader{
+		{Name: "Authorization", Value: "Bearer claude", Grant: "claude"},
+		{Name: "Authorization", Value: "Bearer other", Grant: "other"},
+	}
+	req := newInjectReq(t, nil)
+
+	res := injectCredentials(req, creds, "example.test", "GET", "/x")
+
+	if got := req.Header.Get("Authorization"); got != "Bearer other" {
+		t.Errorf("wire Authorization = %q, want %q", got, "Bearer other")
+	}
+	if got := grantsOf(res); !slices.Equal(got, []string{"other"}) {
+		t.Errorf("Grants = %v, want [other] (claude must not be reported)", got)
+	}
+	if got := injectedGrantsOf(res); !slices.Equal(got, []string{"other"}) {
+		t.Errorf("Injected = %v, want [other]", got)
+	}
+}
+
+// Unchanged behavior: distinct header names are all auto-injected.
+func TestInjectCredentials_AutoInjectDistinctHeaders(t *testing.T) {
+	creds := []credentialHeader{
+		{Name: "Authorization", Value: "Bearer a", Grant: "a"},
+		{Name: "X-Api-Key", Value: "k", Grant: "b"},
+	}
+	req := newInjectReq(t, nil)
+
+	res := injectCredentials(req, creds, "example.test", "GET", "/x")
+
+	if got := req.Header.Get("Authorization"); got != "Bearer a" {
+		t.Errorf("Authorization = %q, want %q", got, "Bearer a")
+	}
+	if got := req.Header.Get("X-Api-Key"); got != "k" {
+		t.Errorf("X-Api-Key = %q, want %q", got, "k")
+	}
+	if got := grantsOf(res); !slices.Equal(got, []string{"a", "b"}) {
+		t.Errorf("Grants = %v, want [a b]", got)
+	}
+}
+
+// A placeholder on one header selects that credential; credentials for other
+// headers are not auto-injected in the same request (existing behavior: the
+// auto pass only runs when no placeholder matched).
+func TestInjectCredentials_PlaceholderSuppressesAutoInject(t *testing.T) {
+	creds := []credentialHeader{
+		{Name: "Authorization", Value: "Bearer a", Grant: "a"},
+		{Name: "X-Api-Key", Value: "k", Grant: "b"},
+	}
+	req := newInjectReq(t, map[string]string{"X-Api-Key": "placeholder"})
+
+	res := injectCredentials(req, creds, "example.test", "GET", "/x")
+
+	if got := req.Header.Get("X-Api-Key"); got != "k" {
+		t.Errorf("X-Api-Key = %q, want %q", got, "k")
+	}
+	if got := req.Header.Get("Authorization"); got != "" {
+		t.Errorf("Authorization = %q, want empty", got)
+	}
+	if got := grantsOf(res); !slices.Equal(got, []string{"b"}) {
+		t.Errorf("Grants = %v, want [b]", got)
+	}
+}
+
+// Grants must never name a credential that did not reach the wire: the
+// canonical log line is an audit record.
+func TestInjectCredentials_GrantsNeverOverReport(t *testing.T) {
+	creds := []credentialHeader{
+		{Name: "Authorization", Value: "Bearer claude", Grant: "claude"},
+		{Name: "Authorization", Value: "Bearer other", Grant: "other"},
+		{Name: "Authorization", Value: "Bearer third", Grant: "third"},
+	}
+
+	for _, tc := range []struct {
+		name       string
+		clientHdrs map[string]string
+		wantGrants []string
+	}{
+		{"placeholder", map[string]string{"Authorization": "ph"}, []string{"claude"}},
+		{"auto", nil, []string{"other"}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			req := newInjectReq(t, tc.clientHdrs)
+			res := injectCredentials(req, creds, "example.test", "GET", "/x")
+
+			if got := grantsOf(res); !slices.Equal(got, tc.wantGrants) {
+				t.Errorf("Grants = %v, want %v", got, tc.wantGrants)
+			}
+			if len(res.Injected) != 1 {
+				t.Errorf("Injected has %d creds, want 1", len(res.Injected))
+			}
+		})
+	}
+}
+
+func TestInjectCredentials_Empty(t *testing.T) {
+	req := newInjectReq(t, nil)
+	res := injectCredentials(req, nil, "example.test", "GET", "/x")
+	if len(res.Injected) != 0 || len(res.Grants) != 0 || len(res.InjectedHeaders) != 0 {
+		t.Errorf("empty creds should produce an empty result, got %+v", res)
+	}
+}

proxy/intercept_test.go

@@ -261,27 +261,24 @@ func TestIntercept_AuthFailureInvalidatesOnlyPlaceholderSelectedCredential(t *te
 	}
 }
 
-// Placeholder selection combined with a shared header name: injectCredentials'
-// first pass re-reads the header after a prior iteration overwrote it, so every
-// same-named credential passes its "client sent this header" check and the last
-// one in slice order lands on the wire. Only that last writer — the credential
-// the destination actually rejected — may be evicted.
-//
-// (The wire-selection and grant-logging consequences of that re-read are
-// pre-existing and deliberately left alone here; see the injectCredentials
-// follow-up. This pins the invalidation behavior only.)
+// Placeholder selection combined with a shared header name: exactly one
+// credential reaches the destination, so exactly one may be evicted on a 403.
+// The credential that lost the tie was never sent, and evicting it would drop a
+// cache entry the destination never saw — and, because eviction is
+// cooldown-gated per key, could suppress its own legitimate eviction later.
 func TestIntercept_AuthFailureInvalidatesOnlyWireCredentialWithPlaceholder(t *testing.T) {
 	setup := newInterceptTestSetup(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusForbidden)
 	}))
 
-	var firstInvalidated, lastInvalidated atomic.Int32
+	var claudeInvalidated, loserInvalidated atomic.Int32
 	setup.Proxy.SetCredentialResolver(setup.BackendHost, func(ctx context.Context, proxyReq, innerReq *http.Request, host string) ([]credentialHeader, error) {
 		return []credentialHeader{
+			// A client-sent placeholder selects the claude grant.
 			{Name: "Authorization", Value: "Bearer claude", Grant: "claude",
-				Invalidate: func() { firstInvalidated.Add(1) }},
+				Invalidate: func() { claudeInvalidated.Add(1) }},
 			{Name: "Authorization", Value: "Bearer github", Grant: "github-user",
-				Invalidate: func() { lastInvalidated.Add(1) }},
+				Invalidate: func() { loserInvalidated.Add(1) }},
 		}, nil
 	})
 
@@ -298,13 +295,13 @@ func TestIntercept_AuthFailureInvalidatesOnlyWireCredentialWithPlaceholder(t *te
 	defer resp.Body.Close()
 	io.ReadAll(resp.Body)
 
-	// "Bearer github" is what reached the destination, so it is the credential
+	// "Bearer claude" is what reached the destination, so it is the credential
 	// the 403 refers to.
-	if got := lastInvalidated.Load(); got != 1 {
+	if got := claudeInvalidated.Load(); got != 1 {
 		t.Errorf("on-the-wire credential Invalidate called %d times, want 1", got)
 	}
-	if got := firstInvalidated.Load(); got != 0 {
-		t.Errorf("overwritten credential Invalidate called %d times, want 0", got)
+	if got := loserInvalidated.Load(); got != 0 {
+		t.Errorf("credential that lost the tie Invalidate called %d times, want 0", got)
 	}
 }