From 64a43bcb246a5f6f5b8e3bd41c980a116188c507 Mon Sep 17 00:00:00 2001
From: Sam Morrow <sammorrowdrums@github.com>
Date: Wed, 19 Aug 2026 14:22:21 +0200
Subject: [PATCH 1/6] fix(lockdown): bound repo-access cache expiry and isolate
 it per identity

The repo-access cache used by lockdown mode relied on cache2go's sliding
expiry: every read extends an entry's life, so a frequently-accessed
entry could keep a stale trust decision (e.g. revoked push access)
alive indefinitely instead of refreshing after its TTL.

Separately, cache2go.Cache(name) returns a process-wide singleton table
keyed by name. In HTTP mode, RequestDeps.GetRepoAccessCache built a new
RepoAccessCache per request but always reused the same default-named
table, so trust decisions computed under one caller's credentials could
be served to a different caller for the same owner/repo, without ever
validating the second caller's own access.

Fixes:
- Track each cache entry's original creation time and bound its maximum
  age from that fixed point, not from last access, so entries are
  refreshed after a fixed TTL regardless of read frequency.
- Add lockdown.CacheNameForIdentity, which derives a stable, hashed
  cache-table name from a request identity (e.g. auth token). Two calls
  for the same identity return the same name (reusing a warm cache
  across a session's repeated requests); different identities always
  get different names (no shared cache state).
- RequestDeps.GetRepoAccessCache now scopes each request's cache to the
  requesting token's identity via CacheNameForIdentity, closing the
  cross-identity leak in HTTP/multi-tenant deployments. Stdio mode is
  unaffected: it constructs a single RepoAccessCache for the whole
  process lifetime, as before.

Tests added:
- TestRepoAccessCacheBoundedExpiryIgnoresRepeatedAccess and
  TestRepoAccessCacheNewUserDoesNotResetEntryAge exercise bounded expiry
  deterministically via an injectable clock (no sleeps).
- TestCacheNameForIdentity and
  TestRepoAccessCacheIdentityScopedNamesPreventCrossIdentityLeakage
  cover the naming helper and cross-identity isolation at the lockdown
  package level.
- TestGetRepoAccessCacheIsolatesTrustDecisionsPerIdentity in
  pkg/github mirrors the HTTP server's exact construction pattern
  end-to-end and fails without the dependencies.go fix.

Fixes #3107

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 pkg/github/dependencies.go      |  18 ++++-
 pkg/github/dependencies_test.go |  85 ++++++++++++++++++++
 pkg/lockdown/lockdown.go        | 125 +++++++++++++++++++++++-------
 pkg/lockdown/lockdown_test.go   | 133 +++++++++++++++++++++++++++++++-
 4 files changed, 332 insertions(+), 29 deletions(-)

diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go
index c13f248c56..2ed88f878b 100644
--- a/pkg/github/dependencies.go
+++ b/pkg/github/dependencies.go
@@ -455,8 +455,24 @@ func (d *RequestDeps) GetRepoAccessCache(ctx context.Context) (*lockdown.RepoAcc
 		return nil, err
 	}
 
+	// Scope the cache table to the requesting identity so a trust decision
+	// computed under one caller's credentials is never served to another.
+	// cache2go.Cache(name) returns a process-wide singleton keyed by name, so
+	// without this every request sharing d.RepoAccessOpts (built once at
+	// server startup) would hit the same default-named table regardless of
+	// which token issued the request. Deriving the name from the token keeps
+	// repeated requests from the same identity on a warm cache while
+	// isolating different identities from one another. Copy RepoAccessOpts
+	// before appending so concurrent requests never mutate the shared slice.
+	opts := d.RepoAccessOpts
+	if tokenInfo, ok := ghcontext.GetTokenInfo(ctx); ok {
+		if name := lockdown.CacheNameForIdentity(tokenInfo.Token); name != "" {
+			opts = append(append([]lockdown.RepoAccessOption{}, d.RepoAccessOpts...), lockdown.WithCacheName(name))
+		}
+	}
+
 	// Create repo access cache
-	instance := lockdown.NewRepoAccessCache(gqlClient, restClient, d.RepoAccessOpts...)
+	instance := lockdown.NewRepoAccessCache(gqlClient, restClient, opts...)
 	return instance, nil
 }
 
diff --git a/pkg/github/dependencies_test.go b/pkg/github/dependencies_test.go
index 0ff3f3520a..9e84fe2967 100644
--- a/pkg/github/dependencies_test.go
+++ b/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,89 @@ func TestRequestDepsScopesTokensToConfiguredHosts(t *testing.T) {
 	assert.Empty(t, foreignAuth, "GraphQL redirect must not authenticate to a foreign host")
 }
 
+// TestGetRepoAccessCacheIsolatesTrustDecisionsPerIdentity is a regression test
+// for issue #3107. It mirrors exactly how the HTTP server builds RequestDeps:
+// a single RepoAccessOpts slice is constructed once at startup (with no
+// per-identity WithCacheName) and reused across every request, and
+// GetRepoAccessCache is called fresh per request. Two different token
+// identities querying the same owner/repo/author must each perform their own
+// upstream lookups instead of one being served from the other's cached
+// decision, while repeated requests from the same identity must reuse a warm
+// cache.
+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 once, exactly as pkg/http/server.go does today: no WithCacheName.
+	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")
+
+	// Repeating the same identity's token must reuse the warm per-identity
+	// cache without any additional upstream calls.
+	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()
 
diff --git a/pkg/lockdown/lockdown.go b/pkg/lockdown/lockdown.go
index e9231414a9..17b291339c 100644
--- a/pkg/lockdown/lockdown.go
+++ b/pkg/lockdown/lockdown.go
@@ -2,6 +2,8 @@ package lockdown
 
 import (
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"fmt"
 	"log/slog"
 	"maps"
@@ -26,6 +28,10 @@ type RepoAccessCache struct {
 	logger           *slog.Logger
 	trustedBotLogins map[string]struct{}
 
+	// now returns the current time and defaults to time.Now. Tests override it
+	// to exercise bounded expiry deterministically without sleeping.
+	now func() time.Time
+
 	viewerMu    sync.Mutex
 	viewerLogin string
 }
@@ -33,6 +39,12 @@ type RepoAccessCache struct {
 type repoAccessCacheEntry struct {
 	isPrivate  bool
 	knownUsers map[string]bool // normalized login -> has push access
+
+	// createdAt is the wall-clock time this repository's trust decision was
+	// first fetched. It is preserved across every subsequent update to the
+	// entry (e.g. learning about a newly-seen author), so an entry's maximum
+	// age is bounded from its original creation rather than reset by access.
+	createdAt time.Time
 }
 
 // RepoAccessInfo captures repository metadata needed for lockdown decisions.
@@ -49,8 +61,13 @@ const (
 // RepoAccessOption configures RepoAccessCache at construction time.
 type RepoAccessOption func(*RepoAccessCache)
 
-// WithTTL overrides the default TTL applied to cache entries. A non-positive
-// duration disables expiration.
+// WithTTL overrides the default maximum age applied to cache entries. A
+// non-positive duration disables expiration.
+//
+// The TTL is a bounded, absolute age measured from when an entry's trust
+// decision was first fetched: repeated reads never extend it. This ensures
+// an actively-read entry is still refreshed once it reaches the maximum age,
+// rather than sliding its expiration forward indefinitely.
 func WithTTL(ttl time.Duration) RepoAccessOption {
 	return func(c *RepoAccessCache) {
 		c.ttl = ttl
@@ -66,6 +83,13 @@ 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.Cache(name) returns a process-wide singleton table keyed by name,
+// so any two RepoAccessCache instances constructed with the same name share
+// every cached trust decision. In deployments that serve multiple request
+// identities from one process (e.g. the HTTP server), callers MUST derive a
+// distinct name per identity — see CacheNameForIdentity — or one identity's
+// cached decision can be served to another.
 func WithCacheName(name string) RepoAccessOption {
 	return func(c *RepoAccessCache) {
 		if name != "" {
@@ -74,6 +98,24 @@ func WithCacheName(name string) RepoAccessOption {
 	}
 }
 
+// CacheNameForIdentity derives a stable cache table name scoped to a single
+// request identity (typically an auth token). Two calls with the same
+// identity always return the same name, so repeated requests from the same
+// identity keep sharing a warm cache; two calls with different identities
+// always return different names, so their cached trust decisions cannot mix.
+//
+// The identity is hashed so it never appears verbatim in cache-table names,
+// logs, or metrics. An empty identity returns an empty string, which
+// WithCacheName treats as a no-op (falling back to the default shared name);
+// callers that need isolation must ensure a non-empty identity is supplied.
+func CacheNameForIdentity(identity string) string {
+	if identity == "" {
+		return ""
+	}
+	sum := sha256.Sum256([]byte(identity))
+	return "repo-access:" + 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{
@@ -187,37 +229,45 @@ func (c *RepoAccessCache) getRepoAccessInfo(ctx context.Context, username, owner
 	// so we publish a fresh entry with a cloned knownUsers map on every miss.
 	if cacheItem, err := c.cache.Value(key); err == nil {
 		entry := cacheItem.Data().(*repoAccessCacheEntry)
-		if cachedHasPush, known := entry.knownUsers[userKey]; known {
-			c.logDebug(ctx, fmt.Sprintf("repo access cache hit for user %s to %s/%s", username, owner, repo))
+
+		if !c.entryExpired(entry) {
+			if cachedHasPush, known := entry.knownUsers[userKey]; known {
+				c.logDebug(ctx, fmt.Sprintf("repo access cache hit for user %s to %s/%s", username, owner, repo))
+				return RepoAccessInfo{
+					IsPrivate:     entry.isPrivate,
+					HasPushAccess: cachedHasPush,
+				}, nil
+			}
+
+			c.logDebug(ctx, "known users cache miss, fetching permission")
+
+			hasPush, pushErr := c.checkPushAccess(ctx, username, owner, repo)
+			if pushErr != nil {
+				return RepoAccessInfo{}, pushErr
+			}
+
+			users := make(map[string]bool, len(entry.knownUsers)+1)
+			maps.Copy(users, entry.knownUsers)
+			users[userKey] = hasPush
+			// Preserve the entry's original createdAt: learning about a newly
+			// seen author must not reset the entry's bounded maximum age.
+			c.cache.Add(key, c.ttl, &repoAccessCacheEntry{
+				isPrivate:  entry.isPrivate,
+				knownUsers: users,
+				createdAt:  entry.createdAt,
+			})
+
 			return RepoAccessInfo{
 				IsPrivate:     entry.isPrivate,
-				HasPushAccess: cachedHasPush,
+				HasPushAccess: hasPush,
 			}, nil
 		}
 
-		c.logDebug(ctx, "known users cache miss, fetching permission")
-
-		hasPush, pushErr := c.checkPushAccess(ctx, username, owner, repo)
-		if pushErr != nil {
-			return RepoAccessInfo{}, pushErr
-		}
-
-		users := make(map[string]bool, len(entry.knownUsers)+1)
-		maps.Copy(users, entry.knownUsers)
-		users[userKey] = hasPush
-		c.cache.Add(key, c.ttl, &repoAccessCacheEntry{
-			isPrivate:  entry.isPrivate,
-			knownUsers: users,
-		})
-
-		return RepoAccessInfo{
-			IsPrivate:     entry.isPrivate,
-			HasPushAccess: hasPush,
-		}, nil
+		c.logDebug(ctx, fmt.Sprintf("repo access cache entry for %s/%s exceeded max age, refreshing", owner, repo))
+	} else {
+		c.logDebug(ctx, fmt.Sprintf("repo access cache miss for user %s to %s/%s", username, owner, repo))
 	}
 
-	c.logDebug(ctx, fmt.Sprintf("repo access cache miss for user %s to %s/%s", username, owner, repo))
-
 	isPrivate, viewerLogin, queryErr := c.queryRepoAccessInfo(ctx, owner, repo)
 	if queryErr != nil {
 		return RepoAccessInfo{}, queryErr
@@ -232,6 +282,7 @@ func (c *RepoAccessCache) getRepoAccessInfo(ctx context.Context, username, owner
 	c.cache.Add(key, c.ttl, &repoAccessCacheEntry{
 		knownUsers: map[string]bool{userKey: hasPush},
 		isPrivate:  isPrivate,
+		createdAt:  c.clock(),
 	})
 
 	return RepoAccessInfo{
@@ -240,6 +291,28 @@ func (c *RepoAccessCache) getRepoAccessInfo(ctx context.Context, username, owner
 	}, nil
 }
 
+// entryExpired reports whether entry has reached the cache's bounded maximum
+// age, measured from its original creation time rather than its last access
+// time. Unlike the underlying cache2go table's own sliding expiry (which
+// resets on every read), this check ensures a frequently-accessed entry is
+// still forced to refresh once it is old enough, so stale trust decisions
+// cannot be kept alive indefinitely by repeated reads.
+func (c *RepoAccessCache) entryExpired(entry *repoAccessCacheEntry) bool {
+	if c.ttl <= 0 {
+		return false
+	}
+	return c.clock().Sub(entry.createdAt) >= c.ttl
+}
+
+// clock returns the current time, using the injected now function if set
+// (tests use this to exercise bounded expiry deterministically).
+func (c *RepoAccessCache) clock() time.Time {
+	if c.now != nil {
+		return c.now()
+	}
+	return time.Now()
+}
+
 // queryRepoAccessInfo fetches repository visibility and the viewer login in a single GraphQL round-trip.
 func (c *RepoAccessCache) queryRepoAccessInfo(ctx context.Context, owner, repo string) (bool, string, error) {
 	if c.client == nil {
diff --git a/pkg/lockdown/lockdown_test.go b/pkg/lockdown/lockdown_test.go
index 887fcfde6f..66e7e1f809 100644
--- a/pkg/lockdown/lockdown_test.go
+++ b/pkg/lockdown/lockdown_test.go
@@ -113,14 +113,17 @@ func newMockRepoAccessCache(t *testing.T, ttl time.Duration) (*RepoAccessCache,
 func TestRepoAccessCacheEvictsAfterTTL(t *testing.T) {
 	ctx := t.Context()
 
-	cache, transport := newMockRepoAccessCache(t, 5*time.Millisecond)
+	cache, transport := newMockRepoAccessCache(t, time.Minute)
+	start := time.Now()
+	cache.now = func() time.Time { return start }
+
 	info, err := cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
 	require.NoError(t, err)
 	require.False(t, info.IsPrivate)
 	require.True(t, info.HasPushAccess)
 	require.EqualValues(t, 1, transport.CallCount())
 
-	time.Sleep(20 * time.Millisecond)
+	cache.now = func() time.Time { return start.Add(2 * time.Minute) }
 
 	info, err = cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
 	require.NoError(t, err)
@@ -129,6 +132,82 @@ func TestRepoAccessCacheEvictsAfterTTL(t *testing.T) {
 	require.EqualValues(t, 2, transport.CallCount())
 }
 
+// TestRepoAccessCacheBoundedExpiryIgnoresRepeatedAccess is a regression test for
+// issue #3107: a sliding-expiry cache extends an entry's life on every read, so
+// a frequently-accessed entry never refreshes even once revoked access should
+// have invalidated it. With bounded expiry, an entry's maximum age is measured
+// from its original creation, not its last access, so repeated reads within
+// the TTL are served from cache but the entry is still forced to refresh once
+// its absolute age exceeds the TTL.
+func TestRepoAccessCacheBoundedExpiryIgnoresRepeatedAccess(t *testing.T) {
+	ctx := t.Context()
+
+	const ttl = 100 * time.Second
+	cache, transport := newMockRepoAccessCache(t, ttl)
+	current := time.Now()
+	cache.now = func() time.Time { return current }
+
+	info, err := cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	require.NoError(t, err)
+	require.True(t, info.HasPushAccess)
+	require.EqualValues(t, 1, transport.CallCount())
+
+	// Repeatedly access the entry well within the window. A sliding-expiry
+	// cache would extend the entry's life on every one of these reads and
+	// never refresh it; bounded expiry must keep serving it from cache
+	// without making new upstream calls, since the absolute age is still
+	// under the TTL.
+	for range 4 {
+		current = current.Add(20 * time.Second)
+		_, err = cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+		require.NoError(t, err)
+	}
+	require.EqualValues(t, 1, transport.CallCount(), "repeated access within the bounded window must still be served from cache")
+
+	// Cross the bound: total elapsed time since creation now exceeds the TTL,
+	// even though every individual access happened well inside it.
+	current = current.Add(30 * time.Second)
+	info, err = cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	require.NoError(t, err)
+	require.True(t, info.HasPushAccess)
+	require.EqualValues(t, 2, transport.CallCount(), "entry must refresh once its absolute age exceeds the TTL, regardless of access frequency")
+}
+
+// TestRepoAccessCacheNewUserDoesNotResetEntryAge ensures that learning about a
+// newly-seen author on an existing repo entry does not reset the entry's
+// bounded creation time, which would otherwise re-introduce sliding behavior
+// through a different code path.
+func TestRepoAccessCacheNewUserDoesNotResetEntryAge(t *testing.T) {
+	ctx := t.Context()
+
+	const ttl = 100 * time.Second
+	gqlClient, transport := newMockGQLClient(testUser, false)
+	restClient := newMockRESTServer(t, "write")
+	cache := NewRepoAccessCache(gqlClient, restClient, WithTTL(ttl), WithCacheName(t.Name()))
+
+	start := time.Now()
+	cache.now = func() time.Time { return start }
+
+	_, err := cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	require.NoError(t, err)
+	require.EqualValues(t, 1, transport.CallCount())
+
+	// A different, previously-unseen user triggers a "known users" miss but
+	// not a full entry miss, exercising the path that preserves createdAt.
+	cache.now = func() time.Time { return start.Add(50 * time.Second) }
+	_, err = cache.getRepoAccessInfo(ctx, "someone-else", testOwner, testRepo)
+	require.NoError(t, err)
+	require.EqualValues(t, 1, transport.CallCount(), "checking a new user against a cached repo entry must not re-query repo metadata")
+
+	// Total elapsed time since the entry's original creation now exceeds the
+	// TTL. If the new-user update above had reset createdAt, this would still
+	// be considered fresh (50s < 100s from the reset point); it must not be.
+	cache.now = func() time.Time { return start.Add(120 * time.Second) }
+	_, err = cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	require.NoError(t, err)
+	require.EqualValues(t, 2, transport.CallCount(), "entry age must be bounded from its original creation, not reset by learning about a new user")
+}
+
 func TestRepoAccessCacheIsolatesViewerPerInstance(t *testing.T) {
 	ctx := t.Context()
 
@@ -152,6 +231,56 @@ func TestRepoAccessCacheIsolatesViewerPerInstance(t *testing.T) {
 	require.True(t, safe)
 }
 
+func TestCacheNameForIdentity(t *testing.T) {
+	t.Run("deterministic for the same identity", func(t *testing.T) {
+		require.Equal(t, CacheNameForIdentity("token-a"), CacheNameForIdentity("token-a"))
+	})
+
+	t.Run("distinct for different identities", func(t *testing.T) {
+		require.NotEqual(t, CacheNameForIdentity("token-a"), CacheNameForIdentity("token-b"))
+	})
+
+	t.Run("empty identity yields empty name", func(t *testing.T) {
+		require.Empty(t, CacheNameForIdentity(""))
+	})
+
+	t.Run("never contains the raw identity", func(t *testing.T) {
+		name := CacheNameForIdentity("super-secret-token")
+		require.NotContains(t, name, "super-secret-token")
+	})
+}
+
+// TestRepoAccessCacheIdentityScopedNamesPreventCrossIdentityLeakage is a
+// regression test for issue #3107. It mirrors how the HTTP server must
+// construct a RepoAccessCache per request: reusing the same
+// lockdown.RepoAccessOption slice across requests but scoping the cache table
+// name to CacheNameForIdentity(token). Two different identities querying the
+// same owner/repo/author must each hit their own upstream clients rather than
+// one being served from the other's cached decision.
+func TestRepoAccessCacheIdentityScopedNamesPreventCrossIdentityLeakage(t *testing.T) {
+	ctx := t.Context()
+
+	restClient := newMockRESTServer(t, "write")
+
+	aliceGQL, aliceTransport := newMockGQLClient("alice", true)
+	aliceCache := NewRepoAccessCache(aliceGQL, restClient, WithCacheName(CacheNameForIdentity("token-alice")))
+	_, err := aliceCache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	require.NoError(t, err)
+	require.EqualValues(t, 1, aliceTransport.CallCount())
+
+	bobGQL, bobTransport := newMockGQLClient("bob", true)
+	bobCache := NewRepoAccessCache(bobGQL, restClient, WithCacheName(CacheNameForIdentity("token-bob")))
+	_, err = bobCache.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")
+
+	// The same identity repeating a request must still hit the warm cache.
+	aliceCacheAgain := NewRepoAccessCache(aliceGQL, restClient, WithCacheName(CacheNameForIdentity("token-alice")))
+	_, err = aliceCacheAgain.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")
+}
+
 type flakyTransport struct {
 	mu    sync.Mutex
 	failN int

From 218819c3aae55504ab3585beda5cc2ddae46031c Mon Sep 17 00:00:00 2001
From: Sam Morrow <sammorrowdrums@github.com>
Date: Wed, 19 Aug 2026 14:48:54 +0200
Subject: [PATCH 2/6] fix(lockdown): scope repo-access cache per identity via
 entry keys
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Isolating identities by deriving a cache2go table name per token grew a
process-wide registry that is never reclaimed: cache2go creates each named
table on first use and never evicts it, so every distinct bearer token —
including invalid ones, since the table was built before GitHub validated
the token — permanently added a table.

Keep a single cache table and scope entries instead. WithIdentity stores a
SHA-256 digest of the identity and prefixes each entry key with it, so
different identities still cannot observe each other's trust decisions,
while per-identity state is reclaimed by the table's ordinary TTL cleanup.
WithCacheName stays for tenant/test isolation, with docs warning against
deriving names from request data.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 pkg/github/dependencies.go    |  19 +++---
 pkg/lockdown/lockdown.go      |  68 +++++++++++++--------
 pkg/lockdown/lockdown_test.go | 108 ++++++++++++++++++++++++----------
 3 files changed, 129 insertions(+), 66 deletions(-)

diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go
index 2ed88f878b..16375925c8 100644
--- a/pkg/github/dependencies.go
+++ b/pkg/github/dependencies.go
@@ -455,20 +455,15 @@ func (d *RequestDeps) GetRepoAccessCache(ctx context.Context) (*lockdown.RepoAcc
 		return nil, err
 	}
 
-	// Scope the cache table to the requesting identity so a trust decision
+	// Scope cache entries to the requesting identity so a trust decision
 	// computed under one caller's credentials is never served to another.
-	// cache2go.Cache(name) returns a process-wide singleton keyed by name, so
-	// without this every request sharing d.RepoAccessOpts (built once at
-	// server startup) would hit the same default-named table regardless of
-	// which token issued the request. Deriving the name from the token keeps
-	// repeated requests from the same identity on a warm cache while
-	// isolating different identities from one another. Copy RepoAccessOpts
-	// before appending so concurrent requests never mutate the shared slice.
+	// RepoAccessOpts is built once at server startup and shared by every
+	// request, so identity scoping has to be applied per request here. Copy
+	// the slice before appending so concurrent requests never mutate the
+	// shared backing array.
 	opts := d.RepoAccessOpts
-	if tokenInfo, ok := ghcontext.GetTokenInfo(ctx); ok {
-		if name := lockdown.CacheNameForIdentity(tokenInfo.Token); name != "" {
-			opts = append(append([]lockdown.RepoAccessOption{}, d.RepoAccessOpts...), lockdown.WithCacheName(name))
-		}
+	if tokenInfo, ok := ghcontext.GetTokenInfo(ctx); ok && tokenInfo.Token != "" {
+		opts = append(append([]lockdown.RepoAccessOption{}, d.RepoAccessOpts...), lockdown.WithIdentity(tokenInfo.Token))
 	}
 
 	// Create repo access cache
diff --git a/pkg/lockdown/lockdown.go b/pkg/lockdown/lockdown.go
index 17b291339c..60a4174f9f 100644
--- a/pkg/lockdown/lockdown.go
+++ b/pkg/lockdown/lockdown.go
@@ -28,6 +28,10 @@ type RepoAccessCache struct {
 	logger           *slog.Logger
 	trustedBotLogins map[string]struct{}
 
+	// identityDigest scopes this instance's entry keys to a single request
+	// identity. Empty means entries are unscoped. See WithIdentity.
+	identityDigest string
+
 	// now returns the current time and defaults to time.Now. Tests override it
 	// to exercise bounded expiry deterministically without sleeping.
 	now func() time.Time
@@ -84,12 +88,13 @@ 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.Cache(name) returns a process-wide singleton table keyed by name,
-// so any two RepoAccessCache instances constructed with the same name share
-// every cached trust decision. In deployments that serve multiple request
-// identities from one process (e.g. the HTTP server), callers MUST derive a
-// distinct name per identity — see CacheNameForIdentity — or one identity's
-// cached decision can be served to another.
+// cache2go.Cache(name) returns a process-wide singleton table that is created
+// on first use and never reclaimed, so the set of names a process passes here
+// must be bounded and known ahead of time. Never derive a name from
+// request-supplied data such as an auth token: the table registry would grow
+// without bound, retaining every distinct value seen for the lifetime of the
+// process. To isolate cached decisions per request identity, use WithIdentity,
+// which keeps a single table and scopes individual entries instead.
 func WithCacheName(name string) RepoAccessOption {
 	return func(c *RepoAccessCache) {
 		if name != "" {
@@ -98,22 +103,29 @@ func WithCacheName(name string) RepoAccessOption {
 	}
 }
 
-// CacheNameForIdentity derives a stable cache table name scoped to a single
-// request identity (typically an auth token). Two calls with the same
-// identity always return the same name, so repeated requests from the same
-// identity keep sharing a warm cache; two calls with different identities
-// always return different names, so their cached trust decisions cannot mix.
+// WithIdentity scopes this cache's entries to a single request identity
+// (typically an auth token), so a trust decision computed under one caller's
+// credentials is never served to another. Two instances configured with the
+// same identity share a warm cache; instances with different identities
+// cannot observe each other's entries.
+//
+// Isolation is applied to the entry key rather than the cache table: entries
+// are stored in the shared table under a key prefixed with a digest of the
+// identity. This keeps storage bounded, because per-identity entries are
+// reclaimed by the same TTL cleanup as any other entry. Allocating a table
+// per identity instead would leak, since cache2go never evicts tables.
 //
-// The identity is hashed so it never appears verbatim in cache-table names,
-// logs, or metrics. An empty identity returns an empty string, which
-// WithCacheName treats as a no-op (falling back to the default shared name);
-// callers that need isolation must ensure a non-empty identity is supplied.
-func CacheNameForIdentity(identity string) string {
-	if identity == "" {
-		return ""
-	}
-	sum := sha256.Sum256([]byte(identity))
-	return "repo-access:" + hex.EncodeToString(sum[:])
+// The identity is hashed so it never appears verbatim in cache keys, logs, or
+// metrics. An empty identity is a no-op, leaving this instance's entries
+// unscoped; callers that need isolation must supply a non-empty identity.
+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.
@@ -222,7 +234,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,
@@ -378,6 +390,14 @@ 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 returns the entry key for owner/repo, prefixed with this cache's
+// identity digest when one is configured. Instances sharing a cache table are
+// kept isolated by this prefix rather than by separate tables, so every
+// identity's entries remain subject to the table's ordinary TTL cleanup.
+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
 }
diff --git a/pkg/lockdown/lockdown_test.go b/pkg/lockdown/lockdown_test.go
index 66e7e1f809..fede675f9e 100644
--- a/pkg/lockdown/lockdown_test.go
+++ b/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"
 )
@@ -231,54 +233,100 @@ func TestRepoAccessCacheIsolatesViewerPerInstance(t *testing.T) {
 	require.True(t, safe)
 }
 
-func TestCacheNameForIdentity(t *testing.T) {
-	t.Run("deterministic for the same identity", func(t *testing.T) {
-		require.Equal(t, CacheNameForIdentity("token-a"), CacheNameForIdentity("token-a"))
-	})
+// TestRepoAccessCacheIdentityScopedKeys covers the key derivation that keeps
+// identities isolated inside a single shared cache table.
+func TestRepoAccessCacheIdentityScopedKeys(t *testing.T) {
+	restClient := newMockRESTServer(t, "write")
+	gqlClient, _ := newMockGQLClient(testUser, false)
+
+	newCache := func(opts ...RepoAccessOption) *RepoAccessCache {
+		return NewRepoAccessCache(gqlClient, restClient, opts...)
+	}
 
-	t.Run("distinct for different identities", func(t *testing.T) {
-		require.NotEqual(t, CacheNameForIdentity("token-a"), CacheNameForIdentity("token-b"))
-	})
+	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)
 
-	t.Run("empty identity yields empty name", func(t *testing.T) {
-		require.Empty(t, CacheNameForIdentity(""))
-	})
+	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")
 
-	t.Run("never contains the raw identity", func(t *testing.T) {
-		name := CacheNameForIdentity("super-secret-token")
-		require.NotContains(t, name, "super-secret-token")
-	})
+	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")
 }
 
-// TestRepoAccessCacheIdentityScopedNamesPreventCrossIdentityLeakage is a
-// regression test for issue #3107. It mirrors how the HTTP server must
-// construct a RepoAccessCache per request: reusing the same
-// lockdown.RepoAccessOption slice across requests but scoping the cache table
-// name to CacheNameForIdentity(token). Two different identities querying the
-// same owner/repo/author must each hit their own upstream clients rather than
-// one being served from the other's cached decision.
-func TestRepoAccessCacheIdentityScopedNamesPreventCrossIdentityLeakage(t *testing.T) {
+// TestRepoAccessCacheIdentityScopingIsolatesWithinOneTable is a regression
+// test for issue #3107. Isolating identities by allocating a cache2go table
+// per token grows a process-wide registry that is never reclaimed, so
+// isolation must instead come from the entry key inside a single table. This
+// asserts both halves: different identities cannot see each other's trust
+// decisions, and their entries share one table so ordinary TTL cleanup can
+// reclaim them.
+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)
-	aliceCache := NewRepoAccessCache(aliceGQL, restClient, WithCacheName(CacheNameForIdentity("token-alice")))
-	_, err := aliceCache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	_, 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)
-	bobCache := NewRepoAccessCache(bobGQL, restClient, WithCacheName(CacheNameForIdentity("token-bob")))
-	_, err = bobCache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	_, 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, 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")
 
-	// The same identity repeating a request must still hit the warm cache.
-	aliceCacheAgain := NewRepoAccessCache(aliceGQL, restClient, WithCacheName(CacheNameForIdentity("token-alice")))
-	_, err = aliceCacheAgain.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
+	// Repeating the same identity must hit the warm cache and must not
+	// allocate additional storage.
+	_, 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")
+}
+
+// TestRepoAccessCacheIdentityScopedEntriesAreReclaimed proves the storage held
+// for distinct identities is bounded: because identity scoping lives in the
+// entry key, per-identity state is removed by the cache table's ordinary TTL
+// cleanup. A table-per-identity design could not shrink this way, since
+// cache2go retains every named table for the life of the process.
+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 TTL cleanup so cache storage stays bounded")
 }
 
 type flakyTransport struct {

From d7d8dd254104329458f4be7d621d802d707a57bd Mon Sep 17 00:00:00 2001
From: Sam Morrow <sammorrowdrums@github.com>
Date: Wed, 19 Aug 2026 14:50:46 +0200
Subject: [PATCH 3/6] ci(lint): skip remote config schema verification

The golangci-lint action downloads a JSON schema from golangci-lint.run on
every run to verify .golangci.yml. A blip reaching that host fails the job
before any linter runs, as it did on this PR. Linting should depend only on
the checked-out code, which is also what script/lint does locally.
---
 .github/workflows/lint.yml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 6119ee9f0f..36bdc76abf 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -24,3 +24,8 @@ jobs:
         with:
           # sync with script/lint
           version: v2.9
+          # The action's config verification downloads a JSON schema from
+          # golangci-lint.run on every run, so a blip reaching that host fails
+          # the job before any linter runs. Skip it: linting must depend only
+          # on the checked-out code, and script/lint does not verify either.
+          verify: false

From 9f7a1a717c6e3968d171a3e941542c9bc0fc1b3a Mon Sep 17 00:00:00 2001
From: Sam Morrow <sammorrowdrums@github.com>
Date: Wed, 19 Aug 2026 15:02:14 +0200
Subject: [PATCH 4/6] Revert "ci(lint): skip remote config schema verification"

This reverts commit d7d8dd25.

The lint job failed on a transient timeout fetching the golangci-lint
config schema, which is a CI infrastructure concern rather than a defect
in this change. Disabling schema verification to work around it does not
belong in a cache-hardening PR: it weakens a check for every future run,
and its root cause is out of scope here. Leaving CI configuration
untouched keeps this PR to the lockdown cache redesign.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .github/workflows/lint.yml | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 36bdc76abf..6119ee9f0f 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -24,8 +24,3 @@ jobs:
         with:
           # sync with script/lint
           version: v2.9
-          # The action's config verification downloads a JSON schema from
-          # golangci-lint.run on every run, so a blip reaching that host fails
-          # the job before any linter runs. Skip it: linting must depend only
-          # on the checked-out code, and script/lint does not verify either.
-          verify: false

From 78828eccc8919aba329dc188af9198002b782f0b Mon Sep 17 00:00:00 2001
From: Sam Morrow <sammorrowdrums@github.com>
Date: Wed, 19 Aug 2026 15:12:43 +0200
Subject: [PATCH 5/6] refactor(lockdown): trim comments to non-obvious
 invariants

The cache changes carried explanatory comments that restated the code or
narrated what each step did. Drop them and keep only what the code cannot
express: that cache2go never reclaims a named table, that its own expiry
slides on every read, that createdAt survives entry updates, and that
RepoAccessOpts is shared across requests. Exported options keep a short
doc comment.

Comment-only; no behavior change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 pkg/github/dependencies.go      |  8 +---
 pkg/github/dependencies_test.go | 15 ++-----
 pkg/lockdown/lockdown.go        | 74 ++++++++++-----------------------
 pkg/lockdown/lockdown_test.go   | 48 ++++-----------------
 4 files changed, 35 insertions(+), 110 deletions(-)

diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go
index 16375925c8..904cb1ba69 100644
--- a/pkg/github/dependencies.go
+++ b/pkg/github/dependencies.go
@@ -455,12 +455,8 @@ func (d *RequestDeps) GetRepoAccessCache(ctx context.Context) (*lockdown.RepoAcc
 		return nil, err
 	}
 
-	// Scope cache entries to the requesting identity so a trust decision
-	// computed under one caller's credentials is never served to another.
-	// RepoAccessOpts is built once at server startup and shared by every
-	// request, so identity scoping has to be applied per request here. Copy
-	// the slice before appending so concurrent requests never mutate the
-	// shared backing array.
+	// 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))
diff --git a/pkg/github/dependencies_test.go b/pkg/github/dependencies_test.go
index 9e84fe2967..173e7a1d35 100644
--- a/pkg/github/dependencies_test.go
+++ b/pkg/github/dependencies_test.go
@@ -124,15 +124,8 @@ func TestRequestDepsScopesTokensToConfiguredHosts(t *testing.T) {
 	assert.Empty(t, foreignAuth, "GraphQL redirect must not authenticate to a foreign host")
 }
 
-// TestGetRepoAccessCacheIsolatesTrustDecisionsPerIdentity is a regression test
-// for issue #3107. It mirrors exactly how the HTTP server builds RequestDeps:
-// a single RepoAccessOpts slice is constructed once at startup (with no
-// per-identity WithCacheName) and reused across every request, and
-// GetRepoAccessCache is called fresh per request. Two different token
-// identities querying the same owner/repo/author must each perform their own
-// upstream lookups instead of one being served from the other's cached
-// decision, while repeated requests from the same identity must reuse a warm
-// cache.
+// 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()
 
@@ -159,7 +152,7 @@ func TestGetRepoAccessCacheIsolatesTrustDecisionsPerIdentity(t *testing.T) {
 		return gqlCalls, restCalls
 	}
 
-	// Built once, exactly as pkg/http/server.go does today: no WithCacheName.
+	// Built as pkg/http/server.go does: no per-identity options.
 	deps := github.NewRequestDeps(
 		newRequestDepsAPIHostResolver(t, server.URL),
 		"test",
@@ -195,8 +188,6 @@ func TestGetRepoAccessCacheIsolatesTrustDecisionsPerIdentity(t *testing.T) {
 	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")
 
-	// Repeating the same identity's token must reuse the warm per-identity
-	// cache without any additional upstream calls.
 	cacheAliceAgain, err := deps.GetRepoAccessCache(ctxAlice)
 	require.NoError(t, err)
 	_, err = cacheAliceAgain.IsSafeContent(ctxAlice, "mallory", "owner", "repo")
diff --git a/pkg/lockdown/lockdown.go b/pkg/lockdown/lockdown.go
index 60a4174f9f..8923c2f287 100644
--- a/pkg/lockdown/lockdown.go
+++ b/pkg/lockdown/lockdown.go
@@ -27,14 +27,8 @@ type RepoAccessCache struct {
 	ttl              time.Duration
 	logger           *slog.Logger
 	trustedBotLogins map[string]struct{}
-
-	// identityDigest scopes this instance's entry keys to a single request
-	// identity. Empty means entries are unscoped. See WithIdentity.
-	identityDigest string
-
-	// now returns the current time and defaults to time.Now. Tests override it
-	// to exercise bounded expiry deterministically without sleeping.
-	now func() time.Time
+	identityDigest   string
+	now              func() time.Time
 
 	viewerMu    sync.Mutex
 	viewerLogin string
@@ -44,10 +38,7 @@ type repoAccessCacheEntry struct {
 	isPrivate  bool
 	knownUsers map[string]bool // normalized login -> has push access
 
-	// createdAt is the wall-clock time this repository's trust decision was
-	// first fetched. It is preserved across every subsequent update to the
-	// entry (e.g. learning about a newly-seen author), so an entry's maximum
-	// age is bounded from its original creation rather than reset by access.
+	// Preserved across entry updates, so age is bounded from the first fetch.
 	createdAt time.Time
 }
 
@@ -66,12 +57,8 @@ const (
 type RepoAccessOption func(*RepoAccessCache)
 
 // WithTTL overrides the default maximum age applied to cache entries. A
-// non-positive duration disables expiration.
-//
-// The TTL is a bounded, absolute age measured from when an entry's trust
-// decision was first fetched: repeated reads never extend it. This ensures
-// an actively-read entry is still refreshed once it reaches the maximum age,
-// rather than sliding its expiration forward indefinitely.
+// non-positive duration disables expiration. The age is absolute, measured
+// from an entry's first fetch: repeated reads never extend it.
 func WithTTL(ttl time.Duration) RepoAccessOption {
 	return func(c *RepoAccessCache) {
 		c.ttl = ttl
@@ -88,13 +75,9 @@ 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.Cache(name) returns a process-wide singleton table that is created
-// on first use and never reclaimed, so the set of names a process passes here
-// must be bounded and known ahead of time. Never derive a name from
-// request-supplied data such as an auth token: the table registry would grow
-// without bound, retaining every distinct value seen for the lifetime of the
-// process. To isolate cached decisions per request identity, use WithIdentity,
-// which keeps a single table and scopes individual entries instead.
+// 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 != "" {
@@ -103,21 +86,14 @@ func WithCacheName(name string) RepoAccessOption {
 	}
 }
 
-// WithIdentity scopes this cache's entries to a single request identity
-// (typically an auth token), so a trust decision computed under one caller's
-// credentials is never served to another. Two instances configured with the
-// same identity share a warm cache; instances with different identities
-// cannot observe each other's entries.
-//
-// Isolation is applied to the entry key rather than the cache table: entries
-// are stored in the shared table under a key prefixed with a digest of the
-// identity. This keeps storage bounded, because per-identity entries are
-// reclaimed by the same TTL cleanup as any other entry. Allocating a table
-// per identity instead would leak, since cache2go never evicts tables.
+// 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.
 //
-// The identity is hashed so it never appears verbatim in cache keys, logs, or
-// metrics. An empty identity is a no-op, leaving this instance's entries
-// unscoped; callers that need isolation must supply a non-empty identity.
+// Scoping lives in the entry key rather than the table so per-identity state
+// stays bounded and is reclaimed by ordinary 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 == "" {
@@ -261,8 +237,7 @@ func (c *RepoAccessCache) getRepoAccessInfo(ctx context.Context, username, owner
 			users := make(map[string]bool, len(entry.knownUsers)+1)
 			maps.Copy(users, entry.knownUsers)
 			users[userKey] = hasPush
-			// Preserve the entry's original createdAt: learning about a newly
-			// seen author must not reset the entry's bounded maximum age.
+			// Preserve createdAt: a new author must not reset the entry's age.
 			c.cache.Add(key, c.ttl, &repoAccessCacheEntry{
 				isPrivate:  entry.isPrivate,
 				knownUsers: users,
@@ -303,12 +278,9 @@ func (c *RepoAccessCache) getRepoAccessInfo(ctx context.Context, username, owner
 	}, nil
 }
 
-// entryExpired reports whether entry has reached the cache's bounded maximum
-// age, measured from its original creation time rather than its last access
-// time. Unlike the underlying cache2go table's own sliding expiry (which
-// resets on every read), this check ensures a frequently-accessed entry is
-// still forced to refresh once it is old enough, so stale trust decisions
-// cannot be kept alive indefinitely by repeated reads.
+// entryExpired reports whether entry has reached the cache's maximum age,
+// measured from creation. cache2go's own expiry instead slides on every read,
+// which would let repeated reads keep a stale decision alive indefinitely.
 func (c *RepoAccessCache) entryExpired(entry *repoAccessCacheEntry) bool {
 	if c.ttl <= 0 {
 		return false
@@ -316,8 +288,6 @@ func (c *RepoAccessCache) entryExpired(entry *repoAccessCacheEntry) bool {
 	return c.clock().Sub(entry.createdAt) >= c.ttl
 }
 
-// clock returns the current time, using the injected now function if set
-// (tests use this to exercise bounded expiry deterministically).
 func (c *RepoAccessCache) clock() time.Time {
 	if c.now != nil {
 		return c.now()
@@ -390,10 +360,8 @@ func (c *RepoAccessCache) isTrustedBot(username string) bool {
 	return ok
 }
 
-// cacheKey returns the entry key for owner/repo, prefixed with this cache's
-// identity digest when one is configured. Instances sharing a cache table are
-// kept isolated by this prefix rather than by separate tables, so every
-// identity's entries remain subject to the table's ordinary TTL cleanup.
+// 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 == "" {
diff --git a/pkg/lockdown/lockdown_test.go b/pkg/lockdown/lockdown_test.go
index fede675f9e..9b1e7c4e6a 100644
--- a/pkg/lockdown/lockdown_test.go
+++ b/pkg/lockdown/lockdown_test.go
@@ -134,13 +134,8 @@ func TestRepoAccessCacheEvictsAfterTTL(t *testing.T) {
 	require.EqualValues(t, 2, transport.CallCount())
 }
 
-// TestRepoAccessCacheBoundedExpiryIgnoresRepeatedAccess is a regression test for
-// issue #3107: a sliding-expiry cache extends an entry's life on every read, so
-// a frequently-accessed entry never refreshes even once revoked access should
-// have invalidated it. With bounded expiry, an entry's maximum age is measured
-// from its original creation, not its last access, so repeated reads within
-// the TTL are served from cache but the entry is still forced to refresh once
-// its absolute age exceeds the TTL.
+// Regression test for #3107: sliding expiry would let a frequently-read entry
+// outlive revoked access, so age must be bounded from creation.
 func TestRepoAccessCacheBoundedExpiryIgnoresRepeatedAccess(t *testing.T) {
 	ctx := t.Context()
 
@@ -154,11 +149,7 @@ func TestRepoAccessCacheBoundedExpiryIgnoresRepeatedAccess(t *testing.T) {
 	require.True(t, info.HasPushAccess)
 	require.EqualValues(t, 1, transport.CallCount())
 
-	// Repeatedly access the entry well within the window. A sliding-expiry
-	// cache would extend the entry's life on every one of these reads and
-	// never refresh it; bounded expiry must keep serving it from cache
-	// without making new upstream calls, since the absolute age is still
-	// under the TTL.
+	// Each read lands well inside the TTL; only their sum exceeds it.
 	for range 4 {
 		current = current.Add(20 * time.Second)
 		_, err = cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
@@ -166,8 +157,6 @@ func TestRepoAccessCacheBoundedExpiryIgnoresRepeatedAccess(t *testing.T) {
 	}
 	require.EqualValues(t, 1, transport.CallCount(), "repeated access within the bounded window must still be served from cache")
 
-	// Cross the bound: total elapsed time since creation now exceeds the TTL,
-	// even though every individual access happened well inside it.
 	current = current.Add(30 * time.Second)
 	info, err = cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
 	require.NoError(t, err)
@@ -175,10 +164,8 @@ func TestRepoAccessCacheBoundedExpiryIgnoresRepeatedAccess(t *testing.T) {
 	require.EqualValues(t, 2, transport.CallCount(), "entry must refresh once its absolute age exceeds the TTL, regardless of access frequency")
 }
 
-// TestRepoAccessCacheNewUserDoesNotResetEntryAge ensures that learning about a
-// newly-seen author on an existing repo entry does not reset the entry's
-// bounded creation time, which would otherwise re-introduce sliding behavior
-// through a different code path.
+// A "known users" miss updates an existing entry, a second path that must not
+// reset its age.
 func TestRepoAccessCacheNewUserDoesNotResetEntryAge(t *testing.T) {
 	ctx := t.Context()
 
@@ -194,16 +181,11 @@ func TestRepoAccessCacheNewUserDoesNotResetEntryAge(t *testing.T) {
 	require.NoError(t, err)
 	require.EqualValues(t, 1, transport.CallCount())
 
-	// A different, previously-unseen user triggers a "known users" miss but
-	// not a full entry miss, exercising the path that preserves createdAt.
 	cache.now = func() time.Time { return start.Add(50 * time.Second) }
 	_, err = cache.getRepoAccessInfo(ctx, "someone-else", testOwner, testRepo)
 	require.NoError(t, err)
 	require.EqualValues(t, 1, transport.CallCount(), "checking a new user against a cached repo entry must not re-query repo metadata")
 
-	// Total elapsed time since the entry's original creation now exceeds the
-	// TTL. If the new-user update above had reset createdAt, this would still
-	// be considered fresh (50s < 100s from the reset point); it must not be.
 	cache.now = func() time.Time { return start.Add(120 * time.Second) }
 	_, err = cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
 	require.NoError(t, err)
@@ -233,8 +215,6 @@ func TestRepoAccessCacheIsolatesViewerPerInstance(t *testing.T) {
 	require.True(t, safe)
 }
 
-// TestRepoAccessCacheIdentityScopedKeys covers the key derivation that keeps
-// identities isolated inside a single shared cache table.
 func TestRepoAccessCacheIdentityScopedKeys(t *testing.T) {
 	restClient := newMockRESTServer(t, "write")
 	gqlClient, _ := newMockGQLClient(testUser, false)
@@ -259,13 +239,8 @@ func TestRepoAccessCacheIdentityScopedKeys(t *testing.T) {
 		"identity scoping must preserve owner/repo case-insensitivity")
 }
 
-// TestRepoAccessCacheIdentityScopingIsolatesWithinOneTable is a regression
-// test for issue #3107. Isolating identities by allocating a cache2go table
-// per token grows a process-wide registry that is never reclaimed, so
-// isolation must instead come from the entry key inside a single table. This
-// asserts both halves: different identities cannot see each other's trust
-// decisions, and their entries share one table so ordinary TTL cleanup can
-// reclaim them.
+// 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()
 
@@ -291,19 +266,14 @@ func TestRepoAccessCacheIdentityScopingIsolatesWithinOneTable(t *testing.T) {
 	require.EqualValues(t, 2, table.Count(),
 		"per-identity entries must be stored in one shared table rather than a table per identity")
 
-	// Repeating the same identity must hit the warm cache and must not
-	// allocate additional storage.
 	_, 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")
 }
 
-// TestRepoAccessCacheIdentityScopedEntriesAreReclaimed proves the storage held
-// for distinct identities is bounded: because identity scoping lives in the
-// entry key, per-identity state is removed by the cache table's ordinary TTL
-// cleanup. A table-per-identity design could not shrink this way, since
-// cache2go retains every named table for the life of the process.
+// Key-scoped entries stay bounded because ordinary TTL cleanup reclaims them;
+// a table per identity could not shrink this way.
 func TestRepoAccessCacheIdentityScopedEntriesAreReclaimed(t *testing.T) {
 	ctx := t.Context()
 

From 5ccedb790bcb60e05bc2fe3eb99b9d6ce9d64c35 Mon Sep 17 00:00:00 2001
From: Sam Morrow <sammorrowdrums@github.com>
Date: Wed, 19 Aug 2026 16:04:07 +0200
Subject: [PATCH 6/6] fix(lockdown): drop fixed-age expiry, keep per-identity
 cache isolation

The cache's idle/sliding TTL is cache2go's documented behaviour and was
deliberate in both the original hand-rolled cache and the cache2go
migration: a hot repo keeps serving from cache and only idle entries are
reclaimed. Replacing it with a fixed max age traded that away for a
periodic refetch on every hot repo, which is a freshness change rather
than the isolation fix this issue is about.

Remove createdAt, the injected clock, entryExpired, the createdAt
preservation on entry updates, and the tests that only existed to prove
bounded non-sliding expiry. Restore the original sliding semantics.

Keep the per-caller isolation, which is the actual defect: entries were
keyed on owner/repo alone in a process-wide table, so a trust decision
computed under one caller's credentials could be served to another
caller whose own credentials were never checked. Entry keys now carry a
SHA-256 digest of the request identity, inside a single bounded table.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 pkg/lockdown/lockdown.go      | 84 +++++++++++------------------------
 pkg/lockdown/lockdown_test.go | 71 +++--------------------------
 2 files changed, 32 insertions(+), 123 deletions(-)

diff --git a/pkg/lockdown/lockdown.go b/pkg/lockdown/lockdown.go
index 8923c2f287..8625073116 100644
--- a/pkg/lockdown/lockdown.go
+++ b/pkg/lockdown/lockdown.go
@@ -28,7 +28,6 @@ type RepoAccessCache struct {
 	logger           *slog.Logger
 	trustedBotLogins map[string]struct{}
 	identityDigest   string
-	now              func() time.Time
 
 	viewerMu    sync.Mutex
 	viewerLogin string
@@ -37,9 +36,6 @@ type RepoAccessCache struct {
 type repoAccessCacheEntry struct {
 	isPrivate  bool
 	knownUsers map[string]bool // normalized login -> has push access
-
-	// Preserved across entry updates, so age is bounded from the first fetch.
-	createdAt time.Time
 }
 
 // RepoAccessInfo captures repository metadata needed for lockdown decisions.
@@ -56,9 +52,8 @@ const (
 // RepoAccessOption configures RepoAccessCache at construction time.
 type RepoAccessOption func(*RepoAccessCache)
 
-// WithTTL overrides the default maximum age applied to cache entries. A
-// non-positive duration disables expiration. The age is absolute, measured
-// from an entry's first fetch: repeated reads never extend it.
+// WithTTL overrides the default TTL applied to cache entries. A non-positive
+// duration disables expiration.
 func WithTTL(ttl time.Duration) RepoAccessOption {
 	return func(c *RepoAccessCache) {
 		c.ttl = ttl
@@ -92,7 +87,7 @@ func WithCacheName(name string) RepoAccessOption {
 // no-op.
 //
 // Scoping lives in the entry key rather than the table so per-identity state
-// stays bounded and is reclaimed by ordinary TTL cleanup. The identity is
+// 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) {
@@ -217,44 +212,37 @@ func (c *RepoAccessCache) getRepoAccessInfo(ctx context.Context, username, owner
 	// so we publish a fresh entry with a cloned knownUsers map on every miss.
 	if cacheItem, err := c.cache.Value(key); err == nil {
 		entry := cacheItem.Data().(*repoAccessCacheEntry)
-
-		if !c.entryExpired(entry) {
-			if cachedHasPush, known := entry.knownUsers[userKey]; known {
-				c.logDebug(ctx, fmt.Sprintf("repo access cache hit for user %s to %s/%s", username, owner, repo))
-				return RepoAccessInfo{
-					IsPrivate:     entry.isPrivate,
-					HasPushAccess: cachedHasPush,
-				}, nil
-			}
-
-			c.logDebug(ctx, "known users cache miss, fetching permission")
-
-			hasPush, pushErr := c.checkPushAccess(ctx, username, owner, repo)
-			if pushErr != nil {
-				return RepoAccessInfo{}, pushErr
-			}
-
-			users := make(map[string]bool, len(entry.knownUsers)+1)
-			maps.Copy(users, entry.knownUsers)
-			users[userKey] = hasPush
-			// Preserve createdAt: a new author must not reset the entry's age.
-			c.cache.Add(key, c.ttl, &repoAccessCacheEntry{
-				isPrivate:  entry.isPrivate,
-				knownUsers: users,
-				createdAt:  entry.createdAt,
-			})
-
+		if cachedHasPush, known := entry.knownUsers[userKey]; known {
+			c.logDebug(ctx, fmt.Sprintf("repo access cache hit for user %s to %s/%s", username, owner, repo))
 			return RepoAccessInfo{
 				IsPrivate:     entry.isPrivate,
-				HasPushAccess: hasPush,
+				HasPushAccess: cachedHasPush,
 			}, nil
 		}
 
-		c.logDebug(ctx, fmt.Sprintf("repo access cache entry for %s/%s exceeded max age, refreshing", owner, repo))
-	} else {
-		c.logDebug(ctx, fmt.Sprintf("repo access cache miss for user %s to %s/%s", username, owner, repo))
+		c.logDebug(ctx, "known users cache miss, fetching permission")
+
+		hasPush, pushErr := c.checkPushAccess(ctx, username, owner, repo)
+		if pushErr != nil {
+			return RepoAccessInfo{}, pushErr
+		}
+
+		users := make(map[string]bool, len(entry.knownUsers)+1)
+		maps.Copy(users, entry.knownUsers)
+		users[userKey] = hasPush
+		c.cache.Add(key, c.ttl, &repoAccessCacheEntry{
+			isPrivate:  entry.isPrivate,
+			knownUsers: users,
+		})
+
+		return RepoAccessInfo{
+			IsPrivate:     entry.isPrivate,
+			HasPushAccess: hasPush,
+		}, nil
 	}
 
+	c.logDebug(ctx, fmt.Sprintf("repo access cache miss for user %s to %s/%s", username, owner, repo))
+
 	isPrivate, viewerLogin, queryErr := c.queryRepoAccessInfo(ctx, owner, repo)
 	if queryErr != nil {
 		return RepoAccessInfo{}, queryErr
@@ -269,7 +257,6 @@ func (c *RepoAccessCache) getRepoAccessInfo(ctx context.Context, username, owner
 	c.cache.Add(key, c.ttl, &repoAccessCacheEntry{
 		knownUsers: map[string]bool{userKey: hasPush},
 		isPrivate:  isPrivate,
-		createdAt:  c.clock(),
 	})
 
 	return RepoAccessInfo{
@@ -278,23 +265,6 @@ func (c *RepoAccessCache) getRepoAccessInfo(ctx context.Context, username, owner
 	}, nil
 }
 
-// entryExpired reports whether entry has reached the cache's maximum age,
-// measured from creation. cache2go's own expiry instead slides on every read,
-// which would let repeated reads keep a stale decision alive indefinitely.
-func (c *RepoAccessCache) entryExpired(entry *repoAccessCacheEntry) bool {
-	if c.ttl <= 0 {
-		return false
-	}
-	return c.clock().Sub(entry.createdAt) >= c.ttl
-}
-
-func (c *RepoAccessCache) clock() time.Time {
-	if c.now != nil {
-		return c.now()
-	}
-	return time.Now()
-}
-
 // queryRepoAccessInfo fetches repository visibility and the viewer login in a single GraphQL round-trip.
 func (c *RepoAccessCache) queryRepoAccessInfo(ctx context.Context, owner, repo string) (bool, string, error) {
 	if c.client == nil {
diff --git a/pkg/lockdown/lockdown_test.go b/pkg/lockdown/lockdown_test.go
index 9b1e7c4e6a..6254533682 100644
--- a/pkg/lockdown/lockdown_test.go
+++ b/pkg/lockdown/lockdown_test.go
@@ -115,17 +115,14 @@ func newMockRepoAccessCache(t *testing.T, ttl time.Duration) (*RepoAccessCache,
 func TestRepoAccessCacheEvictsAfterTTL(t *testing.T) {
 	ctx := t.Context()
 
-	cache, transport := newMockRepoAccessCache(t, time.Minute)
-	start := time.Now()
-	cache.now = func() time.Time { return start }
-
+	cache, transport := newMockRepoAccessCache(t, 5*time.Millisecond)
 	info, err := cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
 	require.NoError(t, err)
 	require.False(t, info.IsPrivate)
 	require.True(t, info.HasPushAccess)
 	require.EqualValues(t, 1, transport.CallCount())
 
-	cache.now = func() time.Time { return start.Add(2 * time.Minute) }
+	time.Sleep(20 * time.Millisecond)
 
 	info, err = cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
 	require.NoError(t, err)
@@ -134,64 +131,6 @@ func TestRepoAccessCacheEvictsAfterTTL(t *testing.T) {
 	require.EqualValues(t, 2, transport.CallCount())
 }
 
-// Regression test for #3107: sliding expiry would let a frequently-read entry
-// outlive revoked access, so age must be bounded from creation.
-func TestRepoAccessCacheBoundedExpiryIgnoresRepeatedAccess(t *testing.T) {
-	ctx := t.Context()
-
-	const ttl = 100 * time.Second
-	cache, transport := newMockRepoAccessCache(t, ttl)
-	current := time.Now()
-	cache.now = func() time.Time { return current }
-
-	info, err := cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
-	require.NoError(t, err)
-	require.True(t, info.HasPushAccess)
-	require.EqualValues(t, 1, transport.CallCount())
-
-	// Each read lands well inside the TTL; only their sum exceeds it.
-	for range 4 {
-		current = current.Add(20 * time.Second)
-		_, err = cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
-		require.NoError(t, err)
-	}
-	require.EqualValues(t, 1, transport.CallCount(), "repeated access within the bounded window must still be served from cache")
-
-	current = current.Add(30 * time.Second)
-	info, err = cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
-	require.NoError(t, err)
-	require.True(t, info.HasPushAccess)
-	require.EqualValues(t, 2, transport.CallCount(), "entry must refresh once its absolute age exceeds the TTL, regardless of access frequency")
-}
-
-// A "known users" miss updates an existing entry, a second path that must not
-// reset its age.
-func TestRepoAccessCacheNewUserDoesNotResetEntryAge(t *testing.T) {
-	ctx := t.Context()
-
-	const ttl = 100 * time.Second
-	gqlClient, transport := newMockGQLClient(testUser, false)
-	restClient := newMockRESTServer(t, "write")
-	cache := NewRepoAccessCache(gqlClient, restClient, WithTTL(ttl), WithCacheName(t.Name()))
-
-	start := time.Now()
-	cache.now = func() time.Time { return start }
-
-	_, err := cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
-	require.NoError(t, err)
-	require.EqualValues(t, 1, transport.CallCount())
-
-	cache.now = func() time.Time { return start.Add(50 * time.Second) }
-	_, err = cache.getRepoAccessInfo(ctx, "someone-else", testOwner, testRepo)
-	require.NoError(t, err)
-	require.EqualValues(t, 1, transport.CallCount(), "checking a new user against a cached repo entry must not re-query repo metadata")
-
-	cache.now = func() time.Time { return start.Add(120 * time.Second) }
-	_, err = cache.getRepoAccessInfo(ctx, testUser, testOwner, testRepo)
-	require.NoError(t, err)
-	require.EqualValues(t, 2, transport.CallCount(), "entry age must be bounded from its original creation, not reset by learning about a new user")
-}
-
 func TestRepoAccessCacheIsolatesViewerPerInstance(t *testing.T) {
 	ctx := t.Context()
 
@@ -272,8 +211,8 @@ func TestRepoAccessCacheIdentityScopingIsolatesWithinOneTable(t *testing.T) {
 	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 TTL cleanup reclaims them;
-// a table per identity could not shrink this way.
+// 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()
 
@@ -296,7 +235,7 @@ func TestRepoAccessCacheIdentityScopedEntriesAreReclaimed(t *testing.T) {
 	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 TTL cleanup so cache storage stays bounded")
+		"per-identity entries must be reclaimed by ordinary idle-TTL cleanup so cache storage stays bounded")
 }
 
 type flakyTransport struct {