Rule: require-sync-exec-timeout
Gap (soundness bug, currently ungrounded in the live .cjs corpus): hasTimeoutOption (eslint-factory/src/rules/require-sync-exec-timeout.ts:113-135) only excludes a timeout property whose value is nullish (null/undefined) — any other literal value, including 0, is accepted as "has timeout" and the call is not reported:
const isNullish = (value.type === Literal && value.value == null) || (value.type === Identifier && value.name === "undefined");
if (!isNullish) return true;
But per Node's child_process docs, timeout defaults to 0, and 0 is explicitly documented as "no timeout" (the value is only checked with > 0 internally before the child is killed). So { timeout: 0, ...opts } is functionally identical to omitting timeout entirely — it provides zero protection against a hung child process — yet the rule treats it as fully compliant. This defeats the rule's own stated purpose ("a hung or runaway child process ... blocks indefinitely"): a caller who writes timeout: 0 (e.g. copy-pasted from a config object, or explicitly "disabling" a shared default) gets no warning at all, even though the code is exactly as vulnerable as the un-timed calls the rule exists to catch.
Grounding note: no live timeout: 0 call site was found in the current actions/setup/js/*.cjs corpus (the only match, mcp_cli_bridge.test.cjs:376, is an unrelated test mock for a different function) — this is a latent correctness gap in the rule's logic rather than a currently-firing false negative, in the same spirit as the previously-filed catchless-tryfinally-protective-FN soundness fix for require-fs-sync-try-catch.
Ask:
- Treat a statically-known
timeout: 0 (or any non-positive numeric literal) the same as "no timeout": isNullish should also cover value.type === Literal && typeof value.value === "number" && value.value <= 0.
- Add regression tests:
execSync(cmd, { timeout: 0 }) must be invalid; execSync(cmd, { timeout: 5000 }) must remain valid; a non-literal timeout: userConfig.timeout must remain unreported (can't statically prove it's non-positive).
- Consider whether the diagnostic message should call out the
timeout: 0 case specifically ("timeout: 0 disables the timeout — pass a positive millisecond value") so the fix is self-explanatory when it fires.
Generated by 🤖 ESLint Refiner · agent · 255.1 AIC · ⌖ 34.6 AIC · ⊞ 4.7K · ◷
Rule:
require-sync-exec-timeoutGap (soundness bug, currently ungrounded in the live
.cjscorpus):hasTimeoutOption(eslint-factory/src/rules/require-sync-exec-timeout.ts:113-135) only excludes atimeoutproperty whose value is nullish (null/undefined) — any other literal value, including0, is accepted as "has timeout" and the call is not reported:But per Node's
child_processdocs,timeoutdefaults to0, and0is explicitly documented as "no timeout" (the value is only checked with> 0internally before the child is killed). So{ timeout: 0, ...opts }is functionally identical to omittingtimeoutentirely — it provides zero protection against a hung child process — yet the rule treats it as fully compliant. This defeats the rule's own stated purpose ("a hung or runaway child process ... blocks indefinitely"): a caller who writestimeout: 0(e.g. copy-pasted from a config object, or explicitly "disabling" a shared default) gets no warning at all, even though the code is exactly as vulnerable as the un-timed calls the rule exists to catch.Grounding note: no live
timeout: 0call site was found in the currentactions/setup/js/*.cjscorpus (the only match,mcp_cli_bridge.test.cjs:376, is an unrelated test mock for a different function) — this is a latent correctness gap in the rule's logic rather than a currently-firing false negative, in the same spirit as the previously-filedcatchless-tryfinally-protective-FNsoundness fix forrequire-fs-sync-try-catch.Ask:
timeout: 0(or any non-positive numeric literal) the same as "no timeout":isNullishshould also covervalue.type === Literal && typeof value.value === "number" && value.value <= 0.execSync(cmd, { timeout: 0 })must be invalid;execSync(cmd, { timeout: 5000 })must remain valid; a non-literaltimeout: userConfig.timeoutmust remain unreported (can't statically prove it's non-positive).timeout: 0case specifically ("timeout: 0 disables the timeout — pass a positive millisecond value") so the fix is self-explanatory when it fires.