fix(github): guard the NaN expiry in mintInstallationToken - #10074
Conversation
`mintInstallationToken` parsed GitHub's `expires_at` with `Date.parse` and no finiteness check. A present-but-unparseable value returns NaN, and a NaN `expiresAtMs` makes `expiresAtMs - TOKEN_SAFETY_MARGIN_MS > Date.now()` forever false — silently disabling the installation-token cache so every job re-mints, re-triggering the thundering-herd (and the GitHub secondary-rate-limit) the cache exists to prevent. Treat an unparseable `expires_at` exactly like an absent one: fall back to `Date.now() + 50 * 60_000`. A well-formed value still produces exactly `Date.parse(payload.expires_at)`, and an absent one still uses the same fallback — both byte-identical to before. No cache-lifetime constant changes. Closes JSONbored#10026
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-31 06:36:34 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10074 +/- ##
==========================================
+ Coverage 79.57% 79.66% +0.09%
==========================================
Files 282 283 +1
Lines 58664 58964 +300
Branches 6842 6955 +113
==========================================
+ Hits 46682 46976 +294
- Misses 11694 11695 +1
- Partials 288 293 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
What & why
mintInstallationToken's local App-JWT path parsed GitHub'sexpires_atstraight into the cache entry with no finiteness check:Date.parsereturns NaN for a present-but-unparseable string, and a NaNexpiresAtMsmakes the cache-freshness comparisonexpiresAtMs - TOKEN_SAFETY_MARGIN_MS > Date.now()forever false. So a single malformedexpires_atsilently disables the installation-token cache: every job re-mints, re-triggering exactly the thundering-herd (and the GitHub secondary-rate-limit on the token endpoint) the cache exists to prevent.The fix
Treat an unparseable
expires_atidentically to an absent one — fall back toDate.now() + 50 * 60_000:expires_at→ still exactlyDate.parse(payload.expires_at)(byte-identical).expires_at→ still theDate.now() + 50 * 60_000fallback.expires_at→ now the same finite fallback instead of NaN.No cache-lifetime constant changes (
TOKEN_SAFETY_MARGIN_MS, the 50-minute fallback, all unchanged).Tests (
test/unit/github-app.test.ts)expires_atis unparseable, a secondcreateInstallationTokenfor the same installation issues no further/access_tokensrequest — asserted by the mint-call count (mints === 1), not just the token string (fails onmain, which re-mints).expires_at: the cached token is reused on the second call (mint count 1) — unchanged behaviour.Validation
npm run typecheckclean for this file (the only local errors are a pre-existing missing-release-please-dep phantom in unrelated release-tooling, present on baremain); the github-app suite green.src/github/app.tsis 100% (bothNumber.isFinitearms and bothexpires_at?arms).git diff --check <base> HEADclean; single-file source change + its test.Closes #10026