Summary
no-setfailed-then-exit-zero only forward-scans for a dangerous process.exit(0) / process.exitCode = 0 within the same statement list as the triggering core.setFailed(...) call (a single BlockStatement, SwitchCase consequent, or Program body), stopping at any return/throw/break/continue/process.exit(anything). If setFailed sits inside a nested block (e.g. an if { core.setFailed(...); } with no following return/throw inside that block) and the risky process.exit(0) is a sibling statement after the if in the enclosing block, the scan never starts — the if statement itself isn't a setFailed statement, so the enclosing list is never scanned from that point.
No live occurrence of this exact shape was found in actions/setup/js/**/*.cjs (58 files call core.setFailed(; every sampled site already returns/throws immediately after), so this is a design/soundness gap rather than a currently-active false negative. It has no test coverage either way.
Ask
- Extend detection so that when a block containing
core.setFailed(...) ends without a terminating control-transfer (return/throw/break/continue), the scan continues into the enclosing statement list rather than stopping at the block boundary — this is the one realistic path by which the bug class (report failure, then still exit 0) could reappear via a differently-shaped if/else refactor.
- Add regression tests for: (a)
setFailed nested one block deeper than a later sibling exit(0) in the enclosing scope (currently invisible → should become invalid), and (b) confirm the existing same-block detection still holds (should stay invalid).
Secondary, lower-priority note
isProcessExitZero explicitly excludes computed member access (callee.computed), so process["exit"](0) is never flagged — but the sibling setFailed-detector does support the computed form (core["setFailed"](...)). This is an internal inconsistency in the rule's own design (not grounded in any live call site). Worth a one-line fix for symmetry, either by dropping computed support from the setFailed side or adding it to the exit/exitCode side — whichever matches the rule's intended threat model.
Grounding notes
Reviewed eslint-factory/src/rules/no-setfailed-then-exit-zero.ts + its test file. Cross-referenced every core.setFailed( site against every process.exit(0)/process.exit()/process.exitCode = 0 site in the live non-test corpus (grep-based; no node_modules/dist, npm firewalled, so no live eslint run). Only render_template.cjs contains both, at lines 151-152 (setFailed + exit(1), safe) and line 163 (unrelated exit(0) in a separate if branch) — correctly not flagged, true negative. Sampled true negatives showing the rule's core same-block detection works correctly: check_command_position.cjs:19-39, check_stop_time.cjs:11-27, check_workflow_timestamp.cjs:20-67, generate_aw_info.cjs:64-65, validate_secrets.cjs:739-740.
Generated by 🤖 ESLint Refiner · agent · 381.2 AIC · ⌖ 7.71 AIC · ⊞ 5.8K · ◷
Summary
no-setfailed-then-exit-zeroonly forward-scans for a dangerousprocess.exit(0)/process.exitCode = 0within the same statement list as the triggeringcore.setFailed(...)call (a singleBlockStatement,SwitchCaseconsequent, orProgrambody), stopping at anyreturn/throw/break/continue/process.exit(anything). IfsetFailedsits inside a nested block (e.g. anif { core.setFailed(...); }with no followingreturn/throwinside that block) and the riskyprocess.exit(0)is a sibling statement after theifin the enclosing block, the scan never starts — theifstatement itself isn't asetFailedstatement, so the enclosing list is never scanned from that point.No live occurrence of this exact shape was found in
actions/setup/js/**/*.cjs(58 files callcore.setFailed(; every sampled site already returns/throws immediately after), so this is a design/soundness gap rather than a currently-active false negative. It has no test coverage either way.Ask
core.setFailed(...)ends without a terminating control-transfer (return/throw/break/continue), the scan continues into the enclosing statement list rather than stopping at the block boundary — this is the one realistic path by which the bug class (report failure, then still exit 0) could reappear via a differently-shapedif/elserefactor.setFailednested one block deeper than a later siblingexit(0)in the enclosing scope (currently invisible → should become invalid), and (b) confirm the existing same-block detection still holds (should stay invalid).Secondary, lower-priority note
isProcessExitZeroexplicitly excludes computed member access (callee.computed), soprocess["exit"](0)is never flagged — but the siblingsetFailed-detector does support the computed form (core["setFailed"](...)). This is an internal inconsistency in the rule's own design (not grounded in any live call site). Worth a one-line fix for symmetry, either by dropping computed support from thesetFailedside or adding it to theexit/exitCodeside — whichever matches the rule's intended threat model.Grounding notes
Reviewed
eslint-factory/src/rules/no-setfailed-then-exit-zero.ts+ its test file. Cross-referenced everycore.setFailed(site against everyprocess.exit(0)/process.exit()/process.exitCode = 0site in the live non-test corpus (grep-based; no node_modules/dist, npm firewalled, so no liveeslintrun). Onlyrender_template.cjscontains both, at lines 151-152 (setFailed+exit(1), safe) and line 163 (unrelatedexit(0)in a separateifbranch) — correctly not flagged, true negative. Sampled true negatives showing the rule's core same-block detection works correctly:check_command_position.cjs:19-39,check_stop_time.cjs:11-27,check_workflow_timestamp.cjs:20-67,generate_aw_info.cjs:64-65,validate_secrets.cjs:739-740.