fix(tasks): bound an endpoint-supplied Retry-After so it cannot strand a job - #758
Merged
sroussey merged 1 commit intoAug 13, 2026
Conversation
…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
Coverage Report
File CoverageNo changed files found. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up fixes on top of #744 (based on
claude/notify-merge-main, notmain).What breaks
Retry-Afteris attacker-controlled input. Four sites turned it into a retryDatewith no ceiling:WebhookPost.tsretryDateFromResponse(header, delay-seconds form)Number(header)WebhookPost.tsretryDateFromResponse(header, HTTP-date form)new Date(header)WebhookPost.tsretryDateFromResponse(Discord JSON body)Number(body.retry_after)— straight out of the response payloadFetchUrlTask.ts(429/503 branch)Two distinct failure shapes:
Retry-After: 1e20—Number.isFinite(1e20)istrue, so the guard passes;Date.now() + 1e23exceeds the maximum representable timestamp, sonew 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):
JobQueueWorker.rescheduleJobaccepted the value withretryDate instanceof Date ? retryDate : nextAvailableTime— an Invalid Date passesinstanceof Date.instanceofis a type check wearing a validity check's clothes.const delaySeconds = Math.max(0, (job.visibleAt.getTime() - Date.now()) / 1000)→Math.max(0, NaN)isNaN, not0.claim.retry({ delaySeconds })→wrapQueueStorage.ts's claim implementation computesnew Date(Date.now() + delay * 1000).toISOString(), which throwsRangeError: Invalid time value. (The same shape exists inapplySendOptionson the send path; the retry path is the one reached here.)rescheduleJob's owncatch(it only logs). Thejob_retryevent is never emitted, and thefinallydrops the claim fromactiveClaims.So the observable bug is not a wrong timestamp — it is that the retry never happens. The row is left in
PROCESSINGholding 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— newSECURITY_LIMITS.httpRetryAfterMaxSeconds: 86_400. InSECURITY_LIMITS, notDEFAULT_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)returnsundefinedfor non-finite input, clamps to[now, now + max], and gates the return onNumber.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 invalidDatefrom this module, so none has to remember that aDatecan be invalid.retryDateFromRetryAfterHeader(header)routes both RFC 9110 forms (delay-seconds and HTTP-date) through it. A date already in the past still yieldsundefined, preserving today's fall-through to the next source.WebhookPost.ts—retryDateFromResponsedelegates all three branches; the inlinenew 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 whenretryDate instanceof Date && Number.isFinite(retryDate.getTime()), else falls back tonextAvailableTime. 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
FetchUrlTaskis included on purpose. It is the path that actually reachesJobQueueWorkertoday — the notify tasks callpostWebhookJsoninline and never go through a queue, so the stranded-job consequence above is currently only reachable viaFetchUrlTask. Fixing onlyWebhookPostwould 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 revertingWebhookPost.tsalone):Retry-After: 1e20on a 429 → finiteDatewithin the ceiling. Was:expected false to be true(Number.isFinite(retryDate.getTime())).{"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.Retry-After: 30case still yields ~30s (unchanged, green).packages/job-queue/src/job/__tests__/JobQueueWorker.test.ts—rescheduleJob(job, new Date(NaN))against a real claim fromwrapQueueStorage: assertsclaim.retryis called with a finitedelaySeconds, thatjob.visibleAtfalls back to the limiter's time (aDelayLimiterset 60s out, so the fallback is unmistakable), and that the stored row lands inPENDINGwith a parseablevisible_at. RED before the fix, witherror: RangeError: Invalid time valuein 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— okbun run build:types— 41/41 successful (this repo has nobun run types;build:typesis the equivalent)bun scripts/test.ts task vitest— 67 files, 1062 passed, 24 skippedbun scripts/test.ts job-queue vitest— 13 files, 118 passed (this section covers the co-locatedpackages/job-queue/src/**/__tests__files, including the new one)prettier --checkandeslintclean on every touched fileGenerated by Claude Code