You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
For spawn/spawnSync/execFile/execFileSync (the SHELL_CONDITIONAL_METHODS set — these only go through a shell when shell: true/shell: "<path>" is explicitly passed), the rule only reports an interpolated/concatenated command when hasShellTrueOptions() returns true. That in turn depends on getShellPropertyValue():
This only recognizes a literaltrue or string. Any non-literal shell value — an Identifier (shell: isWindows), a BinaryExpression/comparison (shell: process.platform === 'win32'), a ConditionalExpression, a MemberExpression, etc. — falls through to return false, so the rule treats the call as shell-disabled and never reports it, even though the option may well evaluate to true at runtime. This is inconsistent with the two lines above in the same function, where an options-arg SpreadElement is deliberately treated as "conservatively possibly-shell" (isShellTrueOption returns true for SpreadElement) — the same conservative default isn't applied to a non-literal shell property value.
Why this matters / grounding
{ shell: process.platform === 'win32' } is a standard, common Node.js idiom for cross-platform spawn/execFile calls (needed so Windows .cmd/.bat shims resolve), so this is not a hypothetical shape. Grepping the current actions/setup/js/*.cjs corpus shows zero literal shell: usages today (grep -n "shell\s*:" actions/setup/js/*.cjs → no non-comment hits), which means this branch of the rule is entirely unexercised in this codebase right now — it's a latent gap, not a live miss. But it means the very first spawn/execFile call added with a computed shell option and an interpolated command argument will silently pass review with a rule that looks like it should catch it.
Ask
Treat any non-Literalshell property value the same conservative way the rule already treats SpreadElement options args: assume possibly-shell-enabled rather than defaulting to false.
Acceptance criteria
getShellPropertyValue (or its caller isShellTrueOption) returns/treats a non-Literalshell: value (Identifier, MemberExpression, BinaryExpression, ConditionalExpression, etc.) as possibly-true rather than false.
New test: spawn(cmd, args, { shell: isWindows }) with an interpolated cmd → reports interpolatedCommand.
New test: spawn(cmd, args, { shell: process.platform === 'win32' }) with a concatenated cmd → reports interpolatedCommand.
New test: execFileSync(cmd, args, { shell: cond ? true : false }) with an interpolated cmd → reports interpolatedCommand.
Existing tests for shell: false, shell: undefined/omitted, and literal shell: true continue to pass unchanged (no regressions to the currently-correct literal-value handling).
Scope
eslint-factory/src/rules/no-child-process-interpolated-command.ts (+ test file) only. This is a proactive soundness fix for an unexercised code path — no production file in actions/setup/js/** currently sets a non-literal shell option, so no target-code changes are implied.
Rule
eslint-factory/src/rules/no-child-process-interpolated-command.tsDesign gap
For
spawn/spawnSync/execFile/execFileSync(theSHELL_CONDITIONAL_METHODSset — these only go through a shell whenshell: true/shell: "<path>"is explicitly passed), the rule only reports an interpolated/concatenated command whenhasShellTrueOptions()returns true. That in turn depends ongetShellPropertyValue():This only recognizes a literal
trueor string. Any non-literalshellvalue — anIdentifier(shell: isWindows), aBinaryExpression/comparison (shell: process.platform === 'win32'), aConditionalExpression, aMemberExpression, etc. — falls through toreturn false, so the rule treats the call as shell-disabled and never reports it, even though the option may well evaluate totrueat runtime. This is inconsistent with the two lines above in the same function, where an options-argSpreadElementis deliberately treated as "conservatively possibly-shell" (isShellTrueOptionreturnstrueforSpreadElement) — the same conservative default isn't applied to a non-literalshellproperty value.Why this matters / grounding
{ shell: process.platform === 'win32' }is a standard, common Node.js idiom for cross-platformspawn/execFilecalls (needed so Windows.cmd/.batshims resolve), so this is not a hypothetical shape. Grepping the currentactions/setup/js/*.cjscorpus shows zero literalshell:usages today (grep -n "shell\s*:" actions/setup/js/*.cjs→ no non-comment hits), which means this branch of the rule is entirely unexercised in this codebase right now — it's a latent gap, not a live miss. But it means the very firstspawn/execFilecall added with a computedshelloption and an interpolated command argument will silently pass review with a rule that looks like it should catch it.Ask
Treat any non-
Literalshellproperty value the same conservative way the rule already treatsSpreadElementoptions args: assume possibly-shell-enabled rather than defaulting to false.Acceptance criteria
getShellPropertyValue(or its callerisShellTrueOption) returns/treats a non-Literalshell:value (Identifier,MemberExpression,BinaryExpression,ConditionalExpression, etc.) as possibly-true rather than false.spawn(cmd, args, { shell: isWindows })with an interpolatedcmd→ reportsinterpolatedCommand.spawn(cmd, args, { shell: process.platform === 'win32' })with a concatenatedcmd→ reportsinterpolatedCommand.execFileSync(cmd, args, { shell: cond ? true : false })with an interpolatedcmd→ reportsinterpolatedCommand.shell: false,shell: undefined/omitted, and literalshell: truecontinue to pass unchanged (no regressions to the currently-correct literal-value handling).Scope
eslint-factory/src/rules/no-child-process-interpolated-command.ts(+ test file) only. This is a proactive soundness fix for an unexercised code path — no production file inactions/setup/js/**currently sets a non-literalshelloption, so no target-code changes are implied.