Skip to content

fix(github): key the response cache by installation identity, not the raw token - #2584

Merged
JSONbored merged 1 commit into
mainfrom
fix/selfhost-github-cache-key-stability
Jul 2, 2026
Merged

fix(github): key the response cache by installation identity, not the raw token#2584
JSONbored merged 1 commit into
mainfrom
fix/selfhost-github-cache-key-stability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Closes #2538.

The self-host GitHub response cache (used for branch-protection, bare-commit, and repo/user/installation-metadata reads) derives its Redis cache key from a hash of the literal Authorization header value. Installation tokens rotate roughly hourly by design, plus on any auth failure, plus (if the token isn't preserved across restarts) on every redeploy. Because the cache key hashes the token bytes rather than a stable identity, every rotation invalidated the ENTIRE cached-response namespace for that installation across all cache classes simultaneously — not just the entries that were actually stale — even though every entry was still within its own TTL.

An equivalent bug for a sibling credential (the GitHub App's own signing JWT) was already identified and fixed by reusing a minted JWT for part of its validity window specifically to keep its cache key stable (#1940). The same fix pattern had never been applied to the longer-lived installation token.

What changed

  • src/github/client.ts: responseCacheKey now accepts the same stable per-installation/public-token admission-key identity already threaded through the codebase for rate-limit admission scoping (githubRateLimitAdmissionKey), and uses it in place of the token hash when a caller provides one. A caller that doesn't yet thread an admission key falls back to the previous token-hash behavior — still correctly isolated, just without the cross-rotation benefit. The cache-key version prefix moved from v2 to v3 so old and new key shapes never ambiguously interact during a rolling deploy (old entries simply lapse via their own TTL).
  • Added cacheKeyAdmissionIdentity, deliberately NOT gated on the existing githubRateLimitAdmission boolean (unlike the sibling rateLimitAdmissionKey helper used for admission-control observation) — a caller can supply a stable identity for cache keying without opting into local rate-limit observation for that specific call.
  • src/upstream/commit.ts: resolveUpstreamCommitSha (the one caller reading a cacheable endpoint — bare /commits/{ref} — with the shared public token rather than an installation token) now passes githubRateLimitAdmissionKeyForPublicToken(), giving it the same stable-key benefit with a scope that's distinct from any installation-scoped key by construction.

No change to what gets cached, cache TTLs, or cache-class eligibility — this is purely a key-derivation fix.

Tests

  • test/unit/github-client.test.ts: a cacheable GET keyed by a stable admission identity survives a simulated token rotation (single fetch across both calls); two installations' admission-keyed entries stay isolated even when they momentarily share raw token bytes; the public-token admission key is scoped distinctly from an installation-scoped key for the same URL; a blank/whitespace admission key is treated as absent and falls back to the token-hash keying (proving no accidental cache leak from a malformed key).
  • test/unit/upstream-commit.test.ts: resolveUpstreamCommitSha's cache entry survives a GITHUB_PUBLIC_TOKEN value change (a single network fetch across both calls).

Validation

  • npm run typecheck
  • npx vitest run test/unit/github-client.test.ts test/unit/upstream-commit.test.ts
  • npm run test:changed
  • npm run test:coverage (unsharded, full suite — 100% line and branch coverage on both changed files per coverage/lcov.info)
  • npm run test:workers
  • npm run db:migrations:check / npm run ui:openapi:check / npm run ui:version-audit (no-op — no schema/API/binding changes)
  • npm run ui:lint / npm run ui:typecheck (no UI files touched)
  • npm run test:ci (the full local gate, exit 0)
  • npm audit --audit-level=moderate
  • git diff --check

Scope

  • Change is narrow and limited to the stated problem
  • No secrets, wallets, hotkeys, trust scores, or reward values added anywhere
  • No edits to site/, CNAME, **/lovable/**, or CHANGELOG.md

Safety

  • No auth/session/CORS surface touched
  • Cross-installation cache isolation is preserved and directly tested; the public-token key is distinctly scoped from every installation key by construction (public-token vs installation:{id})

… raw token

Closes #2538.

The self-host GitHub response cache derived its Redis key from a hash of the
literal Authorization header, so every installation-token rotation (roughly
hourly by design, plus on auth failure, plus on redeploy if the token isn't
persisted) invalidated the entire cached-response namespace for that
installation across every cache class at once, even though the underlying
resources hadn't changed and every entry was still within its own TTL.

Key by the same stable per-installation/public-token identity already used
for rate-limit admission scoping instead, via the existing
githubRateLimitAdmissionKey plumbing. A caller that doesn't thread an
admission key falls back to the previous token-hash behavior -- still
correctly isolated, just without the cross-rotation benefit. Also fixes the
one caller authenticating with the shared public token instead of an
installation token (resolveUpstreamCommitSha) to pass its own distinctly-
scoped key, so it gets the same benefit without colliding with any
installation-scoped entry.
@dosubot dosubot Bot added the size:M label Jul 2, 2026
@loopover-orb

loopover-orb Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-02 10:44:51 UTC

4 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
The change moves cache identity from rotating bearer-token bytes to the existing stable admission identity when callers provide one, and it preserves the old auth-hash fallback for unthreaded callers. The added upstream commit wiring correctly gives the shared public-token path a stable, distinct cache identity, and the tests cover rotation survival, installation isolation, public-token scoping, and blank-key fallback. I do not see a reachable correctness break in the visible diff.

Nits — 5 non-blocking
  • nit: src/github/client.ts:63 has a malformed doc sentence, `present whenever it, so...`, which should be tightened before it becomes copied API documentation.
  • nit: src/github/client.ts:238 now embeds the admission key directly into the Redis key; that is fine for the current helper-produced identities, but a short encode/hash step would keep future arbitrary caller-provided keys from adding delimiter/length surprises.
  • src/github/client.ts:63: reword the option comment to explicitly say the key is used for cache keying whenever present, independently of `githubRateLimitAdmission`.
  • src/github/client.ts:238: consider encoding or hashing `admissionKey` before interpolating it into `v3:${authIdentity}:...`, matching the defensive treatment already used for header-derived inputs.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #2538
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (size label size:M; 1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 65 registered-repo PR(s), 55 merged, 553 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 65 PR(s), 553 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 65 PR(s), 553 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • No action.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 2, 2026
@JSONbored JSONbored self-assigned this Jul 2, 2026
@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.98%. Comparing base (a08b959) to head (a5a4492).
⚠️ Report is 21 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2584      +/-   ##
==========================================
+ Coverage   95.94%   95.98%   +0.04%     
==========================================
  Files         228      229       +1     
  Lines       25559    25812     +253     
  Branches     9303     9390      +87     
==========================================
+ Hits        24522    24776     +254     
+ Misses        426      425       -1     
  Partials      611      611              
Files with missing lines Coverage Δ
src/github/client.ts 100.00% <100.00%> (ø)
src/upstream/commit.ts 100.00% <ø> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit 335d9cd into main Jul 2, 2026
12 checks passed
@JSONbored
JSONbored deleted the fix/selfhost-github-cache-key-stability branch July 2, 2026 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Development

Successfully merging this pull request may close these issues.

fix(github): key the GitHub response cache by installation identity, not the raw token

1 participant