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>
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>
Use exact configured host authorities for every REST, GraphQL, and raw client so redirects cannot reattach credentials to foreign hosts or ports. Add adversarial redirect and lookalike coverage.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* wip injecting ff function into tool as dep
* remove debug
* fix linter
* add better test
* adding compile time check
* move experimental to seperate config/ff value
* adding test var
* fixing test
* adding flag and possibility to call feature checker
* fixing name
Sam Morrow