Skip to content

fix(tasks): bound an endpoint-supplied Retry-After so it cannot strand a job - #758

Merged
sroussey merged 1 commit into
claude/notify-merge-mainfrom
claude/optimistic-goldberg-4xvngr-retry-after-clamp
Aug 13, 2026
Merged

fix(tasks): bound an endpoint-supplied Retry-After so it cannot strand a job#758
sroussey merged 1 commit into
claude/notify-merge-mainfrom
claude/optimistic-goldberg-4xvngr-retry-after-clamp

Conversation

@sroussey

Copy link
Copy Markdown
Collaborator

Follow-up fixes on top of #744 (based on claude/notify-merge-main, not main).

What breaks

Retry-After is attacker-controlled input. Four sites turned it into a retry Date with no ceiling:

site source
WebhookPost.ts retryDateFromResponse (header, delay-seconds form) Number(header)
WebhookPost.ts retryDateFromResponse (header, HTTP-date form) new Date(header)
WebhookPost.ts retryDateFromResponse (Discord JSON body) Number(body.retry_after) — straight out of the response payload
FetchUrlTask.ts (429/503 branch) the identical unbounded parse

Two distinct failure shapes:

  • Retry-After: 1e20Number.isFinite(1e20) is true, so the guard passes; Date.now() + 1e23 exceeds the maximum representable timestamp, so new Date(...) is an Invalid Date.
  • Retry-After: Fri, 01 Jan 9999 00:00:00 GMT — parses perfectly well and is genuinely in the future, so it passes every existing check and parks the job for ~8000 years. No overflow needed.

The full consequence: a stranded job, not just a bad timestamp

Verified against the source on this branch, and reproduced in the new job-queue test (the RangeError is visible in the failing run's log):

  1. JobQueueWorker.rescheduleJob accepted the value with retryDate instanceof Date ? retryDate : nextAvailableTime — an Invalid Date passes instanceof Date. instanceof is a type check wearing a validity check's clothes.
  2. const delaySeconds = Math.max(0, (job.visibleAt.getTime() - Date.now()) / 1000)Math.max(0, NaN) is NaN, not 0.
  3. claim.retry({ delaySeconds })wrapQueueStorage.ts's claim implementation computes new Date(Date.now() + delay * 1000).toISOString(), which throws RangeError: Invalid time value. (The same shape exists in applySendOptions on the send path; the retry path is the one reached here.)
  4. That throw happens before any storage write, and is swallowed by rescheduleJob's own catch (it only logs). The job_retry event is never emitted, and the finally drops the claim from activeClaims.

So the observable bug is not a wrong timestamp — it is that the retry never happens. The row is left in PROCESSING holding its lease, recoverable only by lease expiry (and not at all if the worker is stopped). A hostile or merely broken endpoint answering one 429 takes a job out of circulation.

Fix

  • packages/util/src/limits.ts — new SECURITY_LIMITS.httpRetryAfterMaxSeconds: 86_400. In SECURITY_LIMITS, not DEFAULT_LIMITS: a caller able to raise it re-opens the hole. 24h is far beyond any real provider's back-off and keeps the millisecond arithmetic well inside the safe-integer range.
  • packages/tasks/src/util/RetryAfter.ts (new) — one funnel.
    • retryDateFromEpochMs(ms) returns undefined for non-finite input, clamps to [now, now + max], and gates the return on Number.isFinite(date.getTime()). That gate lives here as the single return point rather than at each call site, and it is unreachable after the clamp — which is the point: no caller can be handed an invalid Date from this module, so none has to remember that a Date can be invalid.
    • retryDateFromRetryAfterHeader(header) routes both RFC 9110 forms (delay-seconds and HTTP-date) through it. A date already in the past still yields undefined, preserving today's fall-through to the next source.
  • WebhookPost.tsretryDateFromResponse delegates all three branches; the inline new Date(Date.now() + seconds * 1000) constructions are gone.
  • FetchUrlTask.ts — the identical unbounded parse, replaced by the same funnel.
  • JobQueueWorker.ts — accepts a retry date only when retryDate instanceof Date && Number.isFinite(retryDate.getTime()), else falls back to nextAvailableTime. Deliberately no clamping here: the parse site bounds policy, the worker rejects the impossible. Any job, provider, or third-party error object can construct an Invalid Date, so this guard is not redundant with the parse-site fix.

Intentional scope extension beyond #744

FetchUrlTask is included on purpose. It is the path that actually reaches JobQueueWorker today — the notify tasks call postWebhookJson inline and never go through a queue, so the stranded-job consequence above is currently only reachable via FetchUrlTask. Fixing only WebhookPost would have hardened the path that cannot yet strand a job and left the live one open.

What the tests catch

packages/test/src/test/task/NotifyTask.test.ts (all three RED before the fix, verified by reverting WebhookPost.ts alone):

  • Retry-After: 1e20 on a 429 → finite Date within the ceiling. Was: expected false to be true (Number.isFinite(retryDate.getTime())).
  • Discord 429 body {"retry_after": 1e20} → same. Was: expected false to be true.
  • Retry-After: Fri, 01 Jan 9999 00:00:00 GMT → clamped. Was: expected 253370764800000 to be less than or equal to 1786697386149.
  • The existing Retry-After: 30 case still yields ~30s (unchanged, green).

packages/job-queue/src/job/__tests__/JobQueueWorker.test.tsrescheduleJob(job, new Date(NaN)) against a real claim from wrapQueueStorage: asserts claim.retry is called with a finite delaySeconds, that job.visibleAt falls back to the limiter's time (a DelayLimiter set 60s out, so the fallback is unmistakable), and that the stored row lands in PENDING with a parseable visible_at. RED before the fix, with error: RangeError: Invalid time value in the run log and no retry scheduled.

Verification

Run in a clean worktree off claude/notify-merge-main (Node v22.22.2 — the repo asks for 24+; nothing here is ABI-sensitive, but noting it):

  • bun install — ok (1880 packages), bun run use-source — ok
  • bun run build:types41/41 successful (this repo has no bun run types; build:types is the equivalent)
  • bun scripts/test.ts task vitest67 files, 1062 passed, 24 skipped
  • bun scripts/test.ts job-queue vitest13 files, 118 passed (this section covers the co-located packages/job-queue/src/**/__tests__ files, including the new one)
  • prettier --check and eslint clean on every touched file

Generated by Claude Code

…d a job

A `Retry-After` header (or Discord's JSON `retry_after`) is remote-controlled
input, and both the webhook post path and FetchUrlTask turned it straight into
a Date with no ceiling. `Retry-After: 1e20` overflows the maximum representable
timestamp into an Invalid Date, and `Fri, 01 Jan 9999 00:00:00 GMT` parses
perfectly well into a date that parks the job for millennia.

The Invalid Date is the worse half: it passes `instanceof Date` in
JobQueueWorker.rescheduleJob, its NaN reaches `delaySeconds`, and
`new Date(NaN).toISOString()` throws a RangeError that rescheduleJob's own
catch swallows — no retry is scheduled, the claim is dropped, and the job is
stranded until lease expiry.

Adds SECURITY_LIMITS.httpRetryAfterMaxSeconds (24h) and a single
RetryAfter.ts funnel that every parse routes through, and makes the worker
reject an invalid date defensively rather than clamp it: the parse site owns
the policy, the worker only refuses the impossible.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RomTUtZSTgUbFCYqFs4pcu
@github-actions

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 59.86% 37265 / 62244
🔵 Statements 59.37% 39086 / 65828
🔵 Functions 60.87% 7217 / 11855
🔵 Branches 48.09% 18934 / 39366
File CoverageNo changed files found.
Generated in workflow #3031 for commit ce5eeed by the Vitest Coverage Report Action

@sroussey
sroussey merged commit 438e37e into claude/notify-merge-main Aug 13, 2026
10 of 11 checks passed
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.

2 participants