PR #84047: fix(cli): reject duration overflow in parseDurationMs (#83906)

• Repository: openclaw/openclaw
• Author: hclsys
• State: open | merged: False
• Link: https://github.com/openclaw/openclaw/pull/84047

Description (problem / solution / changelog)

Fixes #83906.

parseDurationMs (src/cli/cron-cli/shared.ts:100-125) correctly rejected non-positive values, but accepted any positive float that parses finitely — including digit-strings that overflow only after the unit factor multiplication. The regex /^(\d+(?:\.\d+)?)(ms|s|m|h|d)$/i strips e notation, so the realistic overflow path is a long pure-digit number: 1 followed by 302 zeros + d produces n = 1e302 (finite double), but n * 86_400_000 is Infinity. Math.floor(Infinity) is still Infinity, which slipped through every null / positive-number check at callers and landed as everyMs: Infinity (or cooldown: Infinity) in cron schedule payloads sent to the gateway.

The reporter's example 1e308d is intercepted earlier by the digits-only regex; the real-world overflow path is the long-digit case the new check catches.

Changes

• src/cli/cron-cli/shared.ts: hoist Math.floor(n * factor) to a local result const and return null if !Number.isFinite(result). The existing !Number.isFinite(n) || n <= 0 guard already covers the parseFloat-Infinity input path (e.g. 310 consecutive digits); the new guard catches the in-between window where n is finite but the product is not.
• src/cli/cron-cli/shared.test.ts: import parseDurationMs and add a focused describe block with 4 regression cases — ordinary durations in each unit (no regression), zero/malformed (no regression), the overflow-on-factor-multiplication case (the new guard), and the parseFloat-Infinity input (already-rejected; pinned so a future refactor can't regress either failure mode).

Diff stat: 2 files, +47 / -1.

Real behavior proof

• Behavior or issue addressed: Sanitized issue evidence — parseDurationMs returned Math.floor(Infinity) === Infinity for inputs whose pure-digit float overflowed only after the unit-factor multiplication. The reporter's literal 1e308d example is filtered earlier by the regex, but the same code path is reachable with long digit strings (e.g. 1 + 302 zeros + d).

• Real environment tested: Local Node 22.x. Probe at /tmp/probe_83906.mjs (a) parses the patched shared.ts and verifies the result hoist, the !Number.isFinite(result) check, and the new return result shape; and (b) replays both the patched and pre-fix shapes against 8 fixtures: ordinary 500ms / 1.5h / 3d, zero, malformed, unit-only, the realistic overflow path (1 + 302 zeros + d → patched returns null, buggy returns Infinity), and the parseFloat-Infinity input (9 × 310 + d → both reject because n itself is Infinity).

• Exact steps or command run after this patch: node /tmp/probe_83906.mjs

• Evidence after fix:

PASS: result hoisted to a local const
PASS: finite-check rejects Infinity after factor multiplication
PASS: function returns the finite value via the new const
PASS: replay: all 8 fixtures (ordinary / zero / malformed / overflow / pre-overflow / already-Infinity) match patched and buggy shapes
PASS: issue repro (realistic overflow path): buggy=Infinity (would leak as everyMs), patched=null (clean rejection)

ALL CASES PASS

• Observed result after fix: Inputs whose pure-digit float overflows only after unit-factor multiplication now return null cleanly, matching the existing behavior for zero, negative, and malformed input. Callers' existing null-checks reject the overflow case before it reaches the gateway as everyMs: Infinity.

• What was not tested: Live openclaw cron add --every <bigdigits>d ... smoke against an actual build — that requires pnpm build. The vitest regression cases exercise the public parseDurationMs contract directly, and the probe replays both the patched and buggy shapes against the same overflow fixture.

Audit (per CLAUDE rules — all 5 steps)

• Existing-helper check: Reuses the existing Number.isFinite predicate and the existing Math.floor flow. No new helper. PASS
• Shared-helper caller check: parseDurationMs (cron-cli/shared.ts version) has 3 production callers: schedule-options.ts:112 (everyMs), register.cron-edit.ts:358 (failure-alert cooldown), and the same-file parseAt:180 (--at relative offset). All three already handle null by treating the input as invalid; the additional guard only converts Infinity outputs to null, strengthening — not changing — the contract. PASS
• Broader-fix rival scan: gh pr list --search '83906 in:title,body' and gh pr list --search 'parseDurationMs in:title,body' return no open PRs. PASS
• Recent-merge audit: git log --oneline -5 -- src/cli/cron-cli/shared.ts shows e1061a8b46 test(live): tolerate provider drift in release checks — unrelated. PASS
• Prototype-pollution scan: N/A — single numeric comparison.

Changed files

• src/cli/cron-cli/shared.test.ts (modified, +36/-0)
• src/cli/cron-cli/shared.ts (modified, +11/-1)