Skip to content

Commit 582aa25

Browse files
authored
fix(github): guard the NaN expiry in mintInstallationToken (#10074)
`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 #10026
1 parent 6bdb776 commit 582aa25

2 files changed

Lines changed: 46 additions & 3 deletions

File tree

src/github/app.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -361,9 +361,11 @@ async function mintInstallationToken(
361361
throw new Error(
362362
"GitHub installation token response did not include a token.",
363363
);
364-
const expiresAtMs = payload.expires_at
365-
? Date.parse(payload.expires_at)
366-
: Date.now() + 50 * 60_000;
364+
// A present-but-UNPARSEABLE expires_at must fall back like an absent one (#10026): Date.parse returns NaN for
365+
// a malformed string, and a NaN expiry makes `expiresAtMs - MARGIN > Date.now()` forever false — silently
366+
// disabling the cache so every job re-mints and re-triggers the thundering-herd this cache exists to prevent.
367+
const parsedExpiry = payload.expires_at ? Date.parse(payload.expires_at) : Number.NaN;
368+
const expiresAtMs = Number.isFinite(parsedExpiry) ? parsedExpiry : Date.now() + 50 * 60_000;
367369
await writeCachedToken(installationId, { token: payload.token, expiresAtMs });
368370
return payload.token;
369371
}

test/unit/github-app.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,47 @@ describe("GitHub check runs", () => {
200200
expect(mints).toBe(1);
201201
});
202202

203+
it("REGRESSION: an unparseable expires_at must not disable the installation-token cache (#10026)", async () => {
204+
const privateKey = await generatePrivateKeyPem();
205+
let mints = 0;
206+
vi.stubGlobal("fetch", async (input: RequestInfo | URL) => {
207+
const url = input.toString();
208+
if (url.includes("/access_tokens")) {
209+
mints += 1;
210+
return Response.json({ token: `installation-token-${mints}`, expires_at: "not-a-date" }); // present but unparseable
211+
}
212+
return new Response("not found", { status: 404 });
213+
});
214+
215+
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: privateKey });
216+
const first = await createInstallationToken(env, 777);
217+
const second = await createInstallationToken(env, 777);
218+
// The NaN expiry used to make every cache comparison false, re-minting on every call. With the finite
219+
// fallback the entry is honored: exactly one mint, the same token reused.
220+
expect(first).toBe("installation-token-1");
221+
expect(second).toBe("installation-token-1");
222+
expect(mints).toBe(1); // NOT re-minted
223+
});
224+
225+
it("a well-formed expires_at is unchanged: token reused on the second call (#10026)", async () => {
226+
const privateKey = await generatePrivateKeyPem();
227+
let mints = 0;
228+
const expiresAt = "2030-01-01T00:00:00Z";
229+
vi.stubGlobal("fetch", async (input: RequestInfo | URL) => {
230+
const url = input.toString();
231+
if (url.includes("/access_tokens")) {
232+
mints += 1;
233+
return Response.json({ token: `installation-token-${mints}`, expires_at: expiresAt });
234+
}
235+
return new Response("not found", { status: 404 });
236+
});
237+
238+
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: privateKey });
239+
expect(await createInstallationToken(env, 888)).toBe("installation-token-1");
240+
expect(await createInstallationToken(env, 888)).toBe("installation-token-1");
241+
expect(mints).toBe(1); // cached against the far-future, well-formed expiry
242+
});
243+
203244
it("(#3811) samples clock skew from the installation-token mint response's Date header", async () => {
204245
const privateKey = await generatePrivateKeyPem();
205246
vi.useFakeTimers();

0 commit comments

Comments
 (0)