Skip to content

fix(web): keep user-token lookups out of the shared result cache (#164) - #173

Open
lukaso-bot wants to merge 1 commit into
mainfrom
fix/pat-results-not-shared-cache
Open

lukaso-bot wants to merge 1 commit into
mainfrom
fix/pat-results-not-shared-cache

Conversation

@lukaso-bot

Copy link
Copy Markdown
Collaborator

Closes #164.

A lookup made with X-User-Github-Token / X-User-Gitlab-Token can be a private repo's answer. On main it was written to the same 5-part cache slot that the public permalink, badge and OG routes read without auth, and an anonymous request running at the same time could join its single-flight and get the result.

Change

  • auth.ts: hasUserToken(req) (either header, blank ignored, same rule as the token resolvers).
  • cache.ts: makeWorkerCache returns a non-shared cache for such requests: no reads, no writes, shared: false. All six routes get their cache from here, so this covers every slot read and write.
  • resolve.ts, routes/lookup.ts, routes/internal.ts: the three singleFlight sites skip coalescing when !cache.shared.

This is option 2 from the issue (do not cache PAT lookups). Cost: a private lookup recomputes every time. Option 1 (a per-PAT key namespace) can come later if that cost matters.

Evidence

New tests, each red on origin/main before the fix:

  • cache.test.ts: a token request never calls caches.default.put / match (both headers). Control: a blank header still uses the store.
  • resolve.test.ts: an anonymous call concurrent with a PAT call on one key runs its own load. Control: two anonymous calls still share one load.
  • integration.test.ts: /api/lookup with a PAT does not return a seeded shared slot, and an anonymous call still does. /api/lookup and /internal/h/...: a PAT request and a concurrent anonymous one get different bodies.

Mutants (each hunk reverted on its own, tests re-run):

  • the makeWorkerCache early return removed: 5 tests red
  • the resolve.ts flight gate forced on: 1 red
  • the lookup.ts flight gate forced on: 1 red
  • the internal.ts flight gate forced on: 1 red

pnpm -r typecheck clean, pnpm test green (core 230, cli 45, web 285, web-og 44+8).

Out of scope

  • GET permalink/badge responses built with a token still send cache-control: public, …. Cloudflare does not store those itself, but a shared proxy could. Nothing in the repo sends these headers on GET. Noted in the backlog.
  • A GitLab token header on a github.com lookup also skips the cache (the check is host-agnostic). Harmless.

🤖 Generated with Claude Code

A lookup made with X-User-Github-Token / X-User-Gitlab-Token may be a private
repo's answer. It was written to the same anonymous cache slot that public
permalinks, badges and OG cards read, and could join an anonymous request's
single-flight. makeWorkerCache now returns a non-shared cache for such requests
(no reads, no writes), and the three single-flight sites skip coalescing for it.

Closes #164

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Preview deployed

Federated GitLab lookups (freedesktop / GNOME) degrade to the "use the CLI" card — the Anubis relay is off in preview. GitHub lookups, permalinks, and OG render work once INTERNAL_SECRET/GITHUB_TOKEN are set on the preview env.

@lukaso-bot

Copy link
Copy Markdown
Collaborator Author

Why the osv dependency scan check is red (not caused by this PR)

This PR changes no dependencies (8 files under packages/web/src + test, no pnpm-lock.yaml). The scan fails on 3 High advisories that are already in main's lockfile:

package on main fixed in
fflate 0.7.4 0.7.5 (GHSA-px8p-9vwx-vf98)
js-yaml 4.3.1 4.3.2 (GHSA-2883-xcg3-v3hh)
sharp 0.35.3 0.35.4 (GHSA-rgj7-g3m4-5g8c)

#170 (Dependabot group bump) moves all three to the fixed versions, and its osv check is green.

Merge order: merge #170 first, then click "Update branch" here (or say rebase). The osv check on this PR then goes green with no code change.

Preview check (pr-173 on released-web-preview), same commit permalink warm in cache:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PAT-computed results are cached in a public, auth-agnostic key namespace

1 participant