[PR #1225] [MERGED] fix(process): refuse malformed timeouts for stoppable commands #2735

New issue

Closed

opened 2026-10-17 21:09:40 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-17 21:09:40 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1225
Author: @kaoru0822-kitauji
Created: 10/9/2026
Status: ✅ Merged
Merged: 10/9/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/exec-stop-timeout-validation

📝 Commits (2)

59c91c8 fix(process): refuse malformed timeouts for stoppable commands
3d32819 fix(process): apply one timeout rule to both execFileOutput modes

📊 Changes

2 files changed (+36 additions, -0 deletions)

View changed files

📝 src/process/ExecFileOutput.test.ts (+16 -0)
📝 src/process/ExecFileOutput.ts (+20 -0)

📄 Description

Follow-up to #1221, addressing the Codex review finding that was posted after it merged.

Problem

#1221 made execFileOutput handle the timeout itself when stopSignal is set, so it no longer passes timeout to execFile. That also skipped execFile's validation of the value. With stopSignal:

timeout: -1 or NaN failed the > 0 check and started the command with no deadline;
fractional or infinite values were silently coerced by setTimeout.

execFile rejects all of these with ERR_OUT_OF_RANGE before starting the child. Current callers pass constant 60 s timeouts, so this is a contract gap rather than a live failure.

Change

execFileOutput now checks timeout before spawning, in both modes: it must be a safe non-negative integer. An invalid value rejects with a RangeError whose code is ERR_OUT_OF_RANGE, and the command does not start.

The check covers both modes because execFile's own rule differs across the supported runtimes (^22.19 || ^24.11 || >=26):

Node	`execFile` accepts
v22.19.0, v24.18.0	any non-negative integer (`NumberIsInteger(timeout) && timeout >= 0`)
v26.5.0	non-negative integers up to `Number.MAX_SAFE_INTEGER` (`validateInteger(timeout, 'timeout', 0)`)

Mirroring either rule would leave stoppable and plain commands disagreeing on one runtime. With one rule, every runtime gives the same answer. Values above 2^31-1 already fail on Node 22/24 in practice, because the timer clamps them to 1 ms. So the only plain-mode change is to refuse integers ≥ 2^53 on Node 22/24 instead of killing the command after 1 ms.

Verification

npx vitest run src/process/ExecFileOutput.test.ts: 12 passed. The new cases (-1, NaN, 1.5, Infinity, 2 ** 53) target a nonexistent command and run in both modes, so a command that starts fails with ENOENT instead. The stop-mode cases fail on main with that ENOENT.
npm run check:changed: typecheck, lint, and 500 affected tests passed.

No Hopper or Ghidra verification was needed; neither provider is involved.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1225 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/9/2026 **Status:** ✅ Merged **Merged:** 10/9/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/exec-stop-timeout-validation` --- ### 📝 Commits (2) - [`59c91c8`](https://github.com/morluto/rea/commit/59c91c88fe88210b4b22b9f1365a16567513da4a) fix(process): refuse malformed timeouts for stoppable commands - [`3d32819`](https://github.com/morluto/rea/commit/3d328193d69a63a355644c51bfb70f524d99bca9) fix(process): apply one timeout rule to both execFileOutput modes ### 📊 Changes **2 files changed** (+36 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/process/ExecFileOutput.test.ts` (+16 -0) 📝 `src/process/ExecFileOutput.ts` (+20 -0) </details> ### 📄 Description Follow-up to #1221, addressing the Codex review finding that was posted after it merged. ## Problem #1221 made `execFileOutput` handle the timeout itself when `stopSignal` is set, so it no longer passes `timeout` to `execFile`. That also skipped `execFile`'s validation of the value. With `stopSignal`: - `timeout: -1` or `NaN` failed the `> 0` check and started the command with **no deadline**; - fractional or infinite values were silently coerced by `setTimeout`. `execFile` rejects all of these with `ERR_OUT_OF_RANGE` before starting the child. Current callers pass constant 60 s timeouts, so this is a contract gap rather than a live failure. ## Change `execFileOutput` now checks `timeout` before spawning, in **both** modes: it must be a safe non-negative integer. An invalid value rejects with a `RangeError` whose `code` is `ERR_OUT_OF_RANGE`, and the command does not start. The check covers both modes because `execFile`'s own rule differs across the supported runtimes (`^22.19 || ^24.11 || >=26`): | Node | `execFile` accepts | | --- | --- | | v22.19.0, v24.18.0 | any non-negative integer (`NumberIsInteger(timeout) && timeout >= 0`) | | v26.5.0 | non-negative integers up to `Number.MAX_SAFE_INTEGER` (`validateInteger(timeout, 'timeout', 0)`) | Mirroring either rule would leave stoppable and plain commands disagreeing on one runtime. With one rule, every runtime gives the same answer. Values above 2^31-1 already fail on Node 22/24 in practice, because the timer clamps them to 1 ms. So the only plain-mode change is to refuse integers ≥ 2^53 on Node 22/24 instead of killing the command after 1 ms. ## Verification - `npx vitest run src/process/ExecFileOutput.test.ts`: 12 passed. The new cases (`-1`, `NaN`, `1.5`, `Infinity`, `2 ** 53`) target a nonexistent command and run in both modes, so a command that starts fails with `ENOENT` instead. The stop-mode cases fail on main with that `ENOENT`. - `npm run check:changed`: typecheck, lint, and 500 affected tests passed. No Hopper or Ghidra verification was needed; neither provider is involved. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-17 21:09:40 +00:00