fix(lockdown): isolate repo-access cache per caller identity

pkg/github/dependencies.go

@@ -463,8 +463,15 @@ func (d *RequestDeps) GetRepoAccessCache(ctx context.Context) (*lockdown.RepoAcc
 		return nil, err
 	}
 
+	// RepoAccessOpts is shared across requests, so copy before appending the
+	// per-request identity scope.
+	opts := d.RepoAccessOpts
+	if tokenInfo, ok := ghcontext.GetTokenInfo(ctx); ok && tokenInfo.Token != "" {
+		opts = append(append([]lockdown.RepoAccessOption{}, d.RepoAccessOpts...), lockdown.WithIdentity(tokenInfo.Token))
+	}
+
 	// Create repo access cache
-	instance := lockdown.NewRepoAccessCache(gqlClient, restClient, d.RepoAccessOpts...)
+	instance := lockdown.NewRepoAccessCache(gqlClient, restClient, opts...)
 	return instance, nil
 }
 

pkg/github/dependencies_test.go

@@ -7,6 +7,8 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
+	"sync"
 	"testing"
 
 	ghcontext "github.com/github/github-mcp-server/pkg/context"
@@ -122,6 +124,80 @@ func TestRequestDepsScopesTokensToConfiguredHosts(t *testing.T) {
 	assert.Empty(t, foreignAuth, "GraphQL redirect must not authenticate to a foreign host")
 }
 
+// Regression test for #3107: RequestDeps is built once at startup and shared,
+// so identity scoping has to happen per request in GetRepoAccessCache.
+func TestGetRepoAccessCacheIsolatesTrustDecisionsPerIdentity(t *testing.T) {
+	t.Parallel()
+
+	var mu sync.Mutex
+	var gqlCalls, restCalls int
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set(headers.ContentTypeHeader, headers.ContentTypeJSON)
+		mu.Lock()
+		defer mu.Unlock()
+		if strings.Contains(r.URL.Path, "/collaborators/") {
+			restCalls++
+			_, _ = w.Write([]byte(`{"permission":"write"}`))
+			return
+		}
+		gqlCalls++
+		_, _ = w.Write([]byte(`{"data":{"viewer":{"login":"someone"},"repository":{"isPrivate":false}}}`))
+	}))
+	defer server.Close()
+
+	callCounts := func() (int, int) {
+		mu.Lock()
+		defer mu.Unlock()
+		return gqlCalls, restCalls
+	}
+
+	// Built as pkg/http/server.go does: no per-identity options.
+	deps := github.NewRequestDeps(
+		newRequestDepsAPIHostResolver(t, server.URL),
+		"test",
+		true, // lockdownMode
+		nil,  // RepoAccessOpts
+		translations.NullTranslationHelper,
+		0,
+		nil,
+		testExporters(),
+	)
+
+	ctxAlice := ghcontext.WithTokenInfo(context.Background(), &ghcontext.TokenInfo{Token: "token-for-alice"})
+	cacheAlice, err := deps.GetRepoAccessCache(ctxAlice)
+	require.NoError(t, err)
+	require.NotNil(t, cacheAlice)
+
+	_, err = cacheAlice.IsSafeContent(ctxAlice, "mallory", "owner", "repo")
+	require.NoError(t, err)
+
+	gqlN, restN := callCounts()
+	require.Equal(t, 1, gqlN)
+	require.Equal(t, 1, restN)
+
+	ctxBob := ghcontext.WithTokenInfo(context.Background(), &ghcontext.TokenInfo{Token: "token-for-bob"})
+	cacheBob, err := deps.GetRepoAccessCache(ctxBob)
+	require.NoError(t, err)
+	require.NotNil(t, cacheBob)
+
+	_, err = cacheBob.IsSafeContent(ctxBob, "mallory", "owner", "repo")
+	require.NoError(t, err)
+
+	gqlN, restN = callCounts()
+	require.Equal(t, 2, gqlN, "a different identity's request must not be served from another identity's cached trust decision")
+	require.Equal(t, 2, restN, "a different identity's request must not be served from another identity's cached trust decision")
+
+	cacheAliceAgain, err := deps.GetRepoAccessCache(ctxAlice)
+	require.NoError(t, err)
+	_, err = cacheAliceAgain.IsSafeContent(ctxAlice, "mallory", "owner", "repo")
+	require.NoError(t, err)
+
+	gqlN, restN = callCounts()
+	require.Equal(t, 2, gqlN, "repeated requests from the same identity should reuse the warm cache")
+	require.Equal(t, 2, restN, "repeated requests from the same identity should reuse the warm cache")
+}
+
 func TestIsFeatureEnabled_WithEnabledFlag(t *testing.T) {
 	t.Parallel()
 

pkg/lockdown/lockdown.go

@@ -2,6 +2,8 @@ package lockdown
 
 import (
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"fmt"
 	"log/slog"
 	"maps"
@@ -25,6 +27,7 @@ type RepoAccessCache struct {
 	ttl              time.Duration
 	logger           *slog.Logger
 	trustedBotLogins map[string]struct{}
+	identityDigest   string
 
 	viewerMu    sync.Mutex
 	viewerLogin string
@@ -66,6 +69,10 @@ func WithLogger(logger *slog.Logger) RepoAccessOption {
 
 // WithCacheName overrides the cache table name used for storing entries.
 // Use this to isolate cache entries between tenants or in tests.
+//
+// cache2go never reclaims a named table, so names must come from a bounded,
+// known set; never derive one from request data. Use WithIdentity instead to
+// isolate per request identity.
 func WithCacheName(name string) RepoAccessOption {
 	return func(c *RepoAccessCache) {
 		if name != "" {
@@ -74,6 +81,24 @@ func WithCacheName(name string) RepoAccessOption {
 	}
 }
 
+// WithIdentity scopes cache entries to a single request identity, typically an
+// auth token, so a decision computed under one caller's credentials is never
+// served to another. Equal identities share a warm cache; an empty one is a
+// no-op.
+//
+// Scoping lives in the entry key rather than the table so per-identity state
+// stays bounded and is reclaimed by ordinary idle-TTL cleanup. The identity is
+// hashed so it never appears verbatim in a key.
+func WithIdentity(identity string) RepoAccessOption {
+	return func(c *RepoAccessCache) {
+		if identity == "" {
+			return
+		}
+		sum := sha256.Sum256([]byte(identity))
+		c.identityDigest = hex.EncodeToString(sum[:])
+	}
+}
+
 // NewRepoAccessCache creates a RepoAccessCache bound to the supplied clients.
 func NewRepoAccessCache(client *githubv4.Client, restClient *github.Client, opts ...RepoAccessOption) *RepoAccessCache {
 	c := &RepoAccessCache{
@@ -180,7 +205,7 @@ func (c *RepoAccessCache) getRepoAccessInfo(ctx context.Context, username, owner
 		return RepoAccessInfo{}, fmt.Errorf("nil repo access cache")
 	}
 
-	key := cacheKey(owner, repo)
+	key := c.cacheKey(owner, repo)
 	userKey := strings.ToLower(username)
 
 	// Entries are immutable once added: the cache table is shared across instances,
@@ -305,6 +330,12 @@ func (c *RepoAccessCache) isTrustedBot(username string) bool {
 	return ok
 }
 
-func cacheKey(owner, repo string) string {
-	return fmt.Sprintf("%s/%s", strings.ToLower(owner), strings.ToLower(repo))
+// cacheKey scopes the owner/repo key to this cache's identity, so identities
+// sharing a table cannot observe each other's entries.
+func (c *RepoAccessCache) cacheKey(owner, repo string) string {
+	key := fmt.Sprintf("%s/%s", strings.ToLower(owner), strings.ToLower(repo))
+	if c.identityDigest == "" {
+		return key
+	}
+	return c.identityDigest + ":" + key
 }

pkg/lockdown/lockdown_test.go

@@ -5,12 +5,14 @@ import (
 	"errors"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"sync"
 	"testing"
 	"time"
 
 	"github.com/github/github-mcp-server/internal/githubv4mock"
 	gogithub "github.com/google/go-github/v89/github"
+	"github.com/muesli/cache2go"
 	"github.com/shurcooL/githubv4"
 	"github.com/stretchr/testify/require"
 )
@@ -152,6 +154,90 @@ func TestRepoAccessCacheIsolatesViewerPerInstance(t *testing.T) {
 	require.True(t, safe)
 }
 
+func TestRepoAccessCacheIdentityScopedKeys(t *testing.T) {
+	restClient := newMockRESTServer(t, "write")
+	gqlClient, _ := newMockGQLClient(testUser, false)
+
+	newCache := func(opts ...RepoAccessOption) *RepoAccessCache {
+		return NewRepoAccessCache(gqlClient, restClient, opts...)
+	}
+
+	unscoped := newCache().cacheKey(testOwner, testRepo)
+	alice := newCache(WithIdentity("token-alice")).cacheKey(testOwner, testRepo)
+	aliceAgain := newCache(WithIdentity("token-alice")).cacheKey(testOwner, testRepo)
+	bob := newCache(WithIdentity("token-bob")).cacheKey(testOwner, testRepo)
+
+	require.Equal(t, alice, aliceAgain, "the same identity must map to the same key so it keeps a warm cache")
+	require.NotEqual(t, alice, bob, "different identities must map to different keys")
+	require.NotEqual(t, alice, unscoped, "a scoped identity must not collide with unscoped entries")
+	require.NotContains(t, alice, "token-alice", "the raw identity must never appear in a cache key")
+
+	require.Equal(t, unscoped, newCache(WithIdentity("")).cacheKey(testOwner, testRepo),
+		"an empty identity must leave entries unscoped")
+	require.Equal(t, alice, newCache(WithIdentity("token-alice")).cacheKey(strings.ToUpper(testOwner), strings.ToUpper(testRepo)),
+		"identity scoping must preserve owner/repo case-insensitivity")
+}
+
+// Regression test for #3107: a table per identity leaks, so isolation must come
+// from the entry key inside one table.
+func TestRepoAccessCacheIdentityScopingIsolatesWithinOneTable(t *testing.T) {
+	ctx := t.Context()
+
+	restClient := newMockRESTServer(t, "write")
+	table := cache2go.Cache(t.Name())
+	t.Cleanup(table.Flush)
+
+	newCache := func(gqlClient *githubv4.Client, identity string) *RepoAccessCache {
+		return NewRepoAccessCache(gqlClient, restClient, WithCacheName(t.Name()), WithIdentity(identity))
+	}
+
+	aliceGQL, aliceTransport := newMockGQLClient("alice", true)
+	_, err := newCache(aliceGQL, "token-alice").getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	require.NoError(t, err)
+	require.EqualValues(t, 1, aliceTransport.CallCount())
+
+	bobGQL, bobTransport := newMockGQLClient("bob", true)
+	_, err = newCache(bobGQL, "token-bob").getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	require.NoError(t, err)
+	require.EqualValues(t, 1, bobTransport.CallCount(),
+		"a different identity must fetch its own trust decision, not reuse another identity's cached entry")
+
+	require.EqualValues(t, 2, table.Count(),
+		"per-identity entries must be stored in one shared table rather than a table per identity")
+
+	_, err = newCache(aliceGQL, "token-alice").getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	require.NoError(t, err)
+	require.EqualValues(t, 1, aliceTransport.CallCount(), "repeated requests from the same identity should reuse the warm cache")
+	require.EqualValues(t, 2, table.Count(), "a repeated request from a known identity must not add another entry")
+}
+
+// Key-scoped entries stay bounded because ordinary idle-TTL cleanup reclaims
+// them; a table per identity could not shrink this way.
+func TestRepoAccessCacheIdentityScopedEntriesAreReclaimed(t *testing.T) {
+	ctx := t.Context()
+
+	restClient := newMockRESTServer(t, "write")
+	table := cache2go.Cache(t.Name())
+	t.Cleanup(table.Flush)
+
+	identities := []string{"token-a", "token-b", "token-c"}
+	for _, identity := range identities {
+		gqlClient, _ := newMockGQLClient(testUser, false)
+		cache := NewRepoAccessCache(gqlClient, restClient,
+			WithCacheName(t.Name()),
+			WithIdentity(identity),
+			WithTTL(500*time.Millisecond),
+		)
+		_, err := cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+		require.NoError(t, err)
+	}
+
+	require.EqualValues(t, len(identities), table.Count(), "each identity should hold exactly one entry in the shared table")
+
+	require.Eventually(t, func() bool { return table.Count() == 0 }, 30*time.Second, 10*time.Millisecond,
+		"per-identity entries must be reclaimed by ordinary idle-TTL cleanup so cache storage stays bounded")
+}
+
 type flakyTransport struct {
 	mu    sync.Mutex
 	failN int