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

[SamMorrowDrums]

Problem

In HTTP mode, the pkg/lockdown repo-access cache was shared across callers.

RequestDeps.GetRepoAccessCache builds a fresh *lockdown.RepoAccessCache per request, but entries were keyed on owner/repo alone inside a process-wide cache2go table. A trust decision computed under one caller's credentials — is this repo private, does this author have push access — could therefore be served to a completely different caller, without that second caller's own credentials ever being checked.

I verified this concretely with a throwaway reproduction before fixing it: a "bob" request was served "alice"'s cached decision, and bob's own REST client was never invoked.

Stdio mode was never affected. It builds one RepoAccessCache for a single process-wide identity.

Fix

• Added lockdown.WithIdentity(identity string). It stores a SHA-256 digest of the request identity (typically the auth token) and prefixes every entry key with it. Equal identities share a warm cache; different identities can no longer observe each other's entries. The raw identity never appears in a key.
• RequestDeps.GetRepoAccessCache applies WithIdentity(tokenInfo.Token) per request. RepoAccessOpts is built once at startup and shared by every request, so the slice is copied before appending.

Isolation lives in the entry key, not in a cache table per identity. cache2go.Cache(name) never reclaims a named table, so deriving a table name from request data would build a per-token registry that only ever grows. Key scoping keeps every identity in one bounded table, where ordinary idle-TTL cleanup reclaims entries as it always has.

Expiry semantics are deliberately unchanged. cache2go refreshes an entry's TTL on access, so hot repos stay cached and idle entries fall out — the documented behaviour this cache has relied on, matching the hand-rolled cache it replaced. Whether repo-access decisions should also carry a freshness bound is a separate question: a fixed max age would make every hot repo refetch on a timer, so it wants singleflight or a shared store first. Out of scope here.

Tests

• TestRepoAccessCacheIdentityScopedKeys — same identity maps to the same key, different identities map to different keys, and the raw identity never appears in a key.
• TestRepoAccessCacheIdentityScopingIsolatesWithinOneTable — two identities cannot share a trust decision, and both are stored in a single shared table rather than a table per identity.
• TestRepoAccessCacheIdentityScopedEntriesAreReclaimed — per-identity entries are reclaimed by ordinary idle-TTL cleanup, so cache storage stays bounded.
• TestGetRepoAccessCacheIsolatesTrustDecisionsPerIdentity (pkg/github) — mirrors the HTTP server's exact construction pattern end-to-end.

I confirmed each of these fails against the pre-fix code and passes with the fix. script/lint and script/test (go test -race ./...) both pass.

Fixes #3107

Acknowledgments

Thanks @YuvalElbar6 for the report that led to this hardening.

[Copilot]

Pull request overview

Bounds lockdown cache expiry and isolates trust decisions by request identity.

Changes:

• Adds absolute TTL enforcement with deterministic tests.
• Derives hashed, identity-scoped cache names.
• Applies identity isolation in HTTP request dependencies.

Show a summary per file

File	Description
pkg/lockdown/lockdown.go	Implements bounded expiry and identity cache naming.
pkg/lockdown/lockdown_test.go	Tests expiry and identity isolation.
pkg/github/dependencies.go	Scopes request caches by token.
pkg/github/dependencies_test.go	Tests request-level isolation.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced