fix(release): bound the crates.io publish phase by the token's real lifetime, not just the budget

.github/workflows/release.yml

@@ -214,6 +214,22 @@ jobs:
         id: crates-auth
         uses: rust-lang/crates-io-auth-action@c6f97d42243bad5fab37ca0427f495c86d5b1a18 # v1.0.5
 
+      # Stamp the mint time immediately after the token above, NOT after the
+      # publish-script step that consumes it (#3258). `crates-io-auth-action`
+      # exposes only the token — no expiry (see its action.yml) — so the only
+      # way to know how much of the token's claimed 30-minute lifetime is
+      # left is to record when this step ran and do the arithmetic ourselves
+      # downstream. Passed to the changesets step below as
+      # CRATES_TOKEN_MINTED_AT_MS, `scripts/release-crates.mjs` takes the
+      # publish-phase deadline as the earlier of its own release-wide budget
+      # and (mint time + 30 minutes − a margin), so time already spent on the
+      # second build, `test:esm`, and the full npm publish between here and
+      # the crates loop is charged against the token's real remaining
+      # lifetime instead of escaping the bound entirely.
+      - name: Stamp crates.io token mint time
+        id: crates-auth-time
+        run: echo "minted_at_ms=$(node -e 'process.stdout.write(String(Date.now()))')" >> "$GITHUB_OUTPUT"
+
       - name: Create Release Pull Request or Publish
         id: changesets
         uses: changesets/action@198f833dd7d863100ea6e28967bc9a9fdefadb0a # v2.1.0
@@ -242,6 +258,12 @@ jobs:
           NPM_CONFIG_PROVENANCE: 'true'
           # crates.io token minted via OIDC above; expires in 30 minutes.
           CARGO_REGISTRY_TOKEN: ${{ steps.crates-auth.outputs.token }}
+          # When the token above was minted, so `scripts/release-crates.mjs`
+          # can bound the publish phase by the token's real remaining
+          # lifetime rather than by a budget that only starts counting once
+          # this whole step (build, test:esm, npm publish) is already done.
+          # See the comment on the "Stamp crates.io token mint time" step.
+          CRATES_TOKEN_MINTED_AT_MS: ${{ steps.crates-auth-time.outputs.minted_at_ms }}
 
       # Whether to create this version's release artifacts.
       #

scripts/release-crates.mjs

@@ -98,8 +98,12 @@ const PUBLISH_POLL_TIMEOUT_MS = 660_000;
 // (elapsed since mint) + this budget, which is not bounded above by 900s.
 //
 // Bounding the real constraint needs the mint timestamp plumbed in from the
-// workflow and the deadline taken as the earlier of the two. That is #3258;
-// this constant only stops the unbounded 77-minute case.
+// workflow and the deadline taken as the earlier of the two — this is #3258,
+// closed by `tokenMintedAtMs` below. When it is supplied, the deadline is
+// whichever comes first: this budget, or the token's own claimed lifetime.
+// When it is NOT supplied (a manual/local run with no minted token), this
+// constant is the only bound, same as before #3258 — it stops the unbounded
+// 77-minute case but not total token consumption.
 //
 // For the same reason, do not read 900s as "enough for one 600s CDN TTL stall
 // plus slack". That subtraction ignores the seven verification builds inside
@@ -110,6 +114,47 @@ const PUBLISH_POLL_TIMEOUT_MS = 660_000;
 // read off `crates-io-auth-action`, whose action.yml says only "temporary".
 const PUBLISH_PHASE_BUDGET_MS = 900_000;
 
+// The token's claimed lifetime — same unverified 30-minute figure as above,
+// this repo's own claim (release.yml lines 13, 205, 243), not something
+// `crates-io-auth-action` exposes (its action.yml outputs only `token`, no
+// expiry).
+const CRATES_TOKEN_LIFETIME_MS = 30 * 60 * 1000;
+
+// Subtracted from CRATES_TOKEN_LIFETIME_MS before treating it as a deadline.
+// `tokenMintedAtMs` is stamped by a workflow step that runs AFTER the auth
+// action returns, not at the instant the token is actually issued, and the
+// gap between "cargo publish presents the token" and "the registry checks
+// it" is not zero either. This margin keeps that slack from being counted as
+// usable budget.
+const CRATES_TOKEN_MARGIN_MS = 60_000;
+
+/**
+ * Reads `CRATES_TOKEN_MINTED_AT_MS` from an env-like object. Returns
+ * `undefined` when unset (a manual/local run with no minted token — the
+ * budget-only deadline still applies) and THROWS when set but not a finite
+ * epoch-milliseconds number, rather than letting a malformed value silently
+ * defeat the bound: an un-validated NaN propagates through Math.min and every
+ * `<= 0` / `<` comparison below without ever being caught, which is exactly
+ * how a non-numeric budget was found to hang the release forever (see the
+ * PR/issue discussion on #3258). Validating at this boundary is what makes
+ * that latent failure mode unreachable once this value is read from
+ * `process.env` in `main()`.
+ */
+export function parseTokenMintedAtMs(env = process.env) {
+  const raw = env.CRATES_TOKEN_MINTED_AT_MS;
+  if (raw === undefined || raw === '') return undefined;
+  const parsed = Number(raw);
+  if (!Number.isFinite(parsed)) {
+    throw new Error(
+      `CRATES_TOKEN_MINTED_AT_MS is set to ${JSON.stringify(raw)}, which is not a finite ` +
+        `number of milliseconds since the epoch. Refusing to start the crates.io publish ` +
+        `phase with a token-budget deadline that cannot be computed, rather than silently ` +
+        `falling back to an unbounded one.`
+    );
+  }
+  return parsed;
+}
+
 export async function publishAllCrates({
   crates = CRATES,
   version,
@@ -128,12 +173,33 @@ export async function publishAllCrates({
   intervalMs = PUBLISH_POLL_INTERVAL_MS,
   timeoutMs = PUBLISH_POLL_TIMEOUT_MS,
   totalBudgetMs = PUBLISH_PHASE_BUDGET_MS,
+  // Epoch-ms timestamp of when the CARGO_REGISTRY_TOKEN was minted, stamped
+  // by `release.yml` right after the OIDC exchange and plumbed down through
+  // `pnpm run release` as `CRATES_TOKEN_MINTED_AT_MS`. When supplied, the
+  // deadline below is capped by the token's own claimed lifetime — not just
+  // this function's own budget — so time already spent between the mint and
+  // this call (a second build, test:esm, the whole npm publish; see the
+  // header) is charged against it. `undefined` (no minted token — a
+  // manual/local run) falls back to the budget-only deadline this function
+  // has always used.
+  tokenMintedAtMs,
   sleepFn,
 } = {}) {
+  const startedAt = Date.now();
+  const budgetOnlyDeadline = startedAt + totalBudgetMs;
+  const tokenDeadline =
+    tokenMintedAtMs == null ? undefined : tokenMintedAtMs + CRATES_TOKEN_LIFETIME_MS - CRATES_TOKEN_MARGIN_MS;
   // One deadline for the whole run, not one per crate. See the note on
   // PUBLISH_PHASE_BUDGET_MS: the binding constraint is the registry token,
-  // which does not restart between crates.
-  const budgetDeadline = Date.now() + totalBudgetMs;
+  // which does not restart between crates. Whichever of the two candidate
+  // deadlines is EARLIER wins — a large release-wide budget must not paper
+  // over a token that is already close to (or past) its own lifetime.
+  const budgetDeadline = tokenDeadline === undefined ? budgetOnlyDeadline : Math.min(budgetOnlyDeadline, tokenDeadline);
+  const boundByToken = tokenDeadline !== undefined && tokenDeadline < budgetOnlyDeadline;
+  // Seconds-from-now this run actually has, for the messages below. Equal to
+  // `totalBudgetMs` whenever the token isn't the tighter bound, so every
+  // pre-#3258 message stays byte-identical when no token is supplied.
+  const effectiveBudgetMs = budgetDeadline - startedAt;
 
   for (const crate of crates) {
     // BEFORE the publish, not after. Placed below it, this guard still ran
@@ -145,8 +211,14 @@ export async function publishAllCrates({
       throw new Error(
         `Ran out of publish-phase budget before ${crate}@${version}, which has NOT ` +
           `been published. The release has spent its whole ` +
-          `${Math.round(totalBudgetMs / 1000)}s allowance on publishing and on waiting ` +
-          `for crates to become resolvable. The crates.io token from ` +
+          `${Math.round(effectiveBudgetMs / 1000)}s allowance on publishing and on waiting ` +
+          `for crates to become resolvable` +
+          (boundByToken
+            ? ` — this run is bounded by the crates.io token's own remaining lifetime ` +
+              `(minted at ${new Date(tokenMintedAtMs).toISOString()}), tighter here than the ` +
+              `${Math.round(totalBudgetMs / 1000)}s release-wide budget`
+            : '') +
+          `. The crates.io token from ` +
           `crates-io-auth-action is short-lived (this repo's release.yml documents 30 ` +
           `minutes from the mint), so publishing further crates now risks failing on ` +
           `AUTHENTICATION with earlier crates already on the registry. Stopping while ` +
@@ -211,9 +283,11 @@ export async function publishAllCrates({
         `${crate}@${version} did not appear in the crates.io index within ` +
           `${Math.round(thisWaitMs / 1000)}s (${attempts} checks)` +
           (boundedByBudget
-            ? `, which was the remainder of the ${Math.round(totalBudgetMs / 1000)}s ` +
+            ? `, which was the remainder of the ${Math.round(effectiveBudgetMs / 1000)}s ` +
               `release-wide budget rather than the ${Math.round(timeoutMs / 1000)}s ` +
-              `per-crate cap. `
+              `per-crate cap` +
+              (boundByToken ? ` (itself capped by the crates.io token's remaining lifetime)` : ``) +
+              `. `
             : `. `) +
           `The upload succeeded (or the version was already uploaded), but the index ` +
           `cargo resolves against has not caught up — publishing the next crate now ` +
@@ -246,7 +320,11 @@ export async function publishAllCrates({
 
 async function main() {
   const version = readWorkspaceVersion(rootDir);
-  await publishAllCrates({ version, cwd: rootDir });
+  // Validated at this boundary, once, before anything else runs — see
+  // `parseTokenMintedAtMs`. Unset (no `release.yml` step wired it in) falls
+  // back to the budget-only deadline `publishAllCrates` has always used.
+  const tokenMintedAtMs = parseTokenMintedAtMs();
+  await publishAllCrates({ version, cwd: rootDir, tokenMintedAtMs });
 }
 
 // Only run when invoked directly (`node scripts/release-crates.mjs`), not

scripts/release-crates.test.mjs

@@ -15,7 +15,7 @@
 
 import { test } from 'node:test';
 import assert from 'node:assert/strict';
-import { publishAllCrates } from './release-crates.mjs';
+import { publishAllCrates, parseTokenMintedAtMs } from './release-crates.mjs';
 
 function fakeClock(start = 0) {
   let now = start;
@@ -460,3 +460,122 @@ test('the DEFAULT wiring polls the sparse index and pre-checks the API — not o
     clock.restore();
   }
 });
+
+// #3258: the budget started counting only inside `publishAllCrates`, so
+// unmeasured work between the token mint and this loop (a second build,
+// test:esm, the whole npm publish) was never charged against it. Passing
+// `tokenMintedAtMs` closes that: the deadline becomes whichever is EARLIER,
+// this budget or the token's own claimed lifetime measured from the mint.
+
+test('a token minted well before the loop starts is already past its lifetime, and a large budget alone would have missed that (#3258)', async () => {
+  const clock = fakeClock(); // "now" = 0
+  try {
+    // Simulates the real defect: by the time this loop runs, the token was
+    // already minted 2,000,000ms (~33min) ago — standing in for the
+    // unmeasured second build + test:esm + full npm publish that happens
+    // between the mint and here. The 30-minute token (minus the 60s margin)
+    // is therefore already expired, even though the release-wide budget
+    // below is generous and has never been touched.
+    const mintedAtMs = -2_000_000;
+    await assert.rejects(
+      () =>
+        publishAllCrates({
+          crates: ['ifc-lite-core'],
+          version: '6.0.0',
+          publishFn: () => {
+            throw new Error('must not publish: the crates.io token is already past its lifetime');
+          },
+          preCheckFn: async () => false,
+          indexCheckFn: async () => false,
+          intervalMs: 5000,
+          timeoutMs: 660_000,
+          totalBudgetMs: 900_000, // plenty of budget-only headroom
+          tokenMintedAtMs: mintedAtMs,
+          sleepFn: clock.sleepFn,
+        }),
+      /Ran out of publish-phase budget before ifc-lite-core@6\.0\.0[\s\S]*crates\.io token's own remaining lifetime/
+    );
+  } finally {
+    clock.restore();
+  }
+});
+
+test('the token bound charges the SAME publish-phase work as the release-wide budget, so a stall it alone would have allowed is now caught', async () => {
+  // With no token, the budget is 3,000,000ms and would not bind here at all.
+  // With the token, the 30-minute (minus 60s margin) lifetime from the mint
+  // is the tighter deadline, so the same publish + index-wait sequence must
+  // now fail against it instead of sailing through on the oversized budget.
+  const clock = fakeClock();
+  try {
+    await assert.rejects(
+      () =>
+        publishAllCrates({
+          crates: ['ifc-lite-core'],
+          version: '6.0.0',
+          // Mint happens at the same instant this loop starts (the best
+          // case for the old, unfixed code — even here it must now bind).
+          tokenMintedAtMs: 0,
+          publishFn: () => clock.advance(1_700_000), // 28m20s of build+verify
+          preCheckFn: async () => false,
+          indexCheckFn: async () => false,
+          intervalMs: 5000,
+          timeoutMs: 660_000,
+          totalBudgetMs: 3_000_000, // deliberately far looser than the token
+          sleepFn: clock.sleepFn,
+        }),
+      /did not appear in the crates\.io index within 40s[\s\S]*remainder of the 1740s release-wide budget[\s\S]*capped by the crates\.io token/
+    );
+  } finally {
+    clock.restore();
+  }
+});
+
+test('without a minted-token timestamp, the deadline falls back to the release-wide budget only (manual/local run)', async () => {
+  // Pins the pre-#3258 fallback: `tokenMintedAtMs` omitted must reproduce the
+  // exact old message, unchanged, so a local `node scripts/release-crates.mjs`
+  // run (no minted token to bound against) is not newly and needlessly
+  // stricter.
+  const clock = fakeClock();
+  try {
+    await assert.rejects(
+      () =>
+        publishAllCrates({
+          crates: ['ifc-lite-core'],
+          version: '6.0.0',
+          publishFn: () => clock.advance(400_000),
+          preCheckFn: async () => false,
+          indexCheckFn: async () => false,
+          intervalMs: 5000,
+          timeoutMs: 660_000,
+          totalBudgetMs: 500_000,
+          sleepFn: clock.sleepFn,
+        }),
+      /within 100s[\s\S]*remainder of the 500s release-wide budget rather than the 660s per-crate cap\. /
+    );
+  } finally {
+    clock.restore();
+  }
+});
+
+test('parseTokenMintedAtMs returns undefined when CRATES_TOKEN_MINTED_AT_MS is unset', () => {
+  assert.equal(parseTokenMintedAtMs({}), undefined);
+});
+
+test('parseTokenMintedAtMs returns undefined when CRATES_TOKEN_MINTED_AT_MS is the empty string', () => {
+  assert.equal(parseTokenMintedAtMs({ CRATES_TOKEN_MINTED_AT_MS: '' }), undefined);
+});
+
+test('parseTokenMintedAtMs parses a valid numeric string', () => {
+  assert.equal(parseTokenMintedAtMs({ CRATES_TOKEN_MINTED_AT_MS: '1735689600000' }), 1735689600000);
+});
+
+test('parseTokenMintedAtMs REFUSES a non-numeric value instead of letting NaN silently defeat the bound', () => {
+  // A NaN deadline would propagate through Math.min and every `<=`/`<`
+  // comparison in `publishAllCrates` without ever tripping — the exact "hangs
+  // the release forever" shape flagged on #3258 for an unvalidated budget
+  // read from the environment.
+  assert.throws(
+    () => parseTokenMintedAtMs({ CRATES_TOKEN_MINTED_AT_MS: 'not-a-number' }),
+    /CRATES_TOKEN_MINTED_AT_MS.*not-a-number/
+  );
+});