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
eslint-factory/src/rules/no-exec-interpolated-command.ts and eslint-factory/src/rules/no-child-process-interpolated-command.ts — both ship an identical, independently-duplicated getDynamicCommandKind / isDynamicStringConcatenation / isStaticExpression trio.
Problem
getDynamicCommandKind only recognizes exactly two shapes as "dynamic":
Any other expression wrapping an interpolated template literal or dynamic concatenation returns null (not flagged) — including the extremely common pattern of chaining a string method after the template literal, e.g. `git checkout ${branch}`.trim() or `git log --author=${author}`.toLowerCase(). In that case the argument node is a CallExpression (the .trim() call), not a TemplateLiteral, so getDynamicCommandKind sees an unrecognized node type and bails out — even though the underlying string is exactly as attacker-controllable as the untrimmed version.
The companion resolver resolveWriteOnceInitializerChain (command-initializer-utils.ts) doesn't help here either: its loop condition is while (candidate.type === AST_NODE_TYPES.Identifier ...), so it only unwraps identifier indirection (const cmd = ...; exec.exec(cmd)), never unwraps a CallExpression like .trim() sitting directly in the argument position or inside an identifier's initializer.
Concrete escape (PoC, not currently live in the codebase)
// no-exec-interpolated-commandexec.exec(`git checkout ${branch}`.trim(),[]);// NOT flagged// no-child-process-interpolated-commandconst{ execSync }=require("child_process");execSync(`git log --author=${author}`.toLowerCase());// NOT flagged
Both are functionally identical injection risks to the already-covered exec.exec(`git checkout ${branch}`, []) and execSync(`git log --author=${x}`) invalid-test cases in each rule's test suite — the only difference is the trailing method call, which is a routine normalization step (trimming whitespace, case-folding a branch/author name) that a developer would very plausibly add without realizing it defeats the lint rule.
Acceptance criteria
In both rules, extend getDynamicCommandKind (or add a small recursive unwrap step before calling it, mirroring the recursive containsEnvAccess traversal already used in require-nan-check-after-env-numeric-parse.ts) to see through a CallExpression whose callee is a MemberExpression on a dynamic receiver (e.g. .trim(), .trimStart(), .trimEnd(), .toLowerCase(), .toUpperCase(), .replace(), .replaceAll()) — i.e. check the receiver (callee.object) recursively instead of only the outermost node.
Add an invalid test to each rule's test file for the .trim()-chained interpolated-template-literal case, and one for the .toLowerCase()-chained case.
Keep all existing valid cases passing unchanged, in particular the already-valid "fully static concatenation" and "static template literal" cases — a .trim() call on a fully static string must remain unflagged.
Consider factoring the now-triplicated isStaticExpression / isDynamicStringConcatenation / getDynamicCommandKind trio (identical in both rule files today) into the shared command-initializer-utils.ts so this fix (and future ones) only needs to land once.
Scope
eslint-factory/** (in scope). No live grounding found in actions/setup/js/** today (grep across non-test .cjs files found template-literal-then-.trim()/.toLowerCase() chains only in unrelated string-building code, never as a direct exec/child_process command argument) — the gap is a code-reasoning finding against the rule's own stated threat model, not an active exploit.
Rules
eslint-factory/src/rules/no-exec-interpolated-command.tsandeslint-factory/src/rules/no-child-process-interpolated-command.ts— both ship an identical, independently-duplicatedgetDynamicCommandKind/isDynamicStringConcatenation/isStaticExpressiontrio.Problem
getDynamicCommandKindonly recognizes exactly two shapes as "dynamic":Any other expression wrapping an interpolated template literal or dynamic concatenation returns
null(not flagged) — including the extremely common pattern of chaining a string method after the template literal, e.g.`git checkout ${branch}`.trim()or`git log --author=${author}`.toLowerCase(). In that case the argument node is aCallExpression(the.trim()call), not aTemplateLiteral, sogetDynamicCommandKindsees an unrecognized node type and bails out — even though the underlying string is exactly as attacker-controllable as the untrimmed version.The companion resolver
resolveWriteOnceInitializerChain(command-initializer-utils.ts) doesn't help here either: its loop condition iswhile (candidate.type === AST_NODE_TYPES.Identifier ...), so it only unwraps identifier indirection (const cmd = ...; exec.exec(cmd)), never unwraps aCallExpressionlike.trim()sitting directly in the argument position or inside an identifier's initializer.Concrete escape (PoC, not currently live in the codebase)
Both are functionally identical injection risks to the already-covered
exec.exec(`git checkout ${branch}`, [])andexecSync(`git log --author=${x}`)invalid-test cases in each rule's test suite — the only difference is the trailing method call, which is a routine normalization step (trimming whitespace, case-folding a branch/author name) that a developer would very plausibly add without realizing it defeats the lint rule.Acceptance criteria
getDynamicCommandKind(or add a small recursive unwrap step before calling it, mirroring the recursivecontainsEnvAccesstraversal already used inrequire-nan-check-after-env-numeric-parse.ts) to see through aCallExpressionwhose callee is aMemberExpressionon a dynamic receiver (e.g..trim(),.trimStart(),.trimEnd(),.toLowerCase(),.toUpperCase(),.replace(),.replaceAll()) — i.e. check the receiver (callee.object) recursively instead of only the outermost node..trim()-chained interpolated-template-literal case, and one for the.toLowerCase()-chained case..trim()call on a fully static string must remain unflagged.isStaticExpression/isDynamicStringConcatenation/getDynamicCommandKindtrio (identical in both rule files today) into the sharedcommand-initializer-utils.tsso this fix (and future ones) only needs to land once.Scope
eslint-factory/**(in scope). No live grounding found inactions/setup/js/**today (grep across non-test.cjsfiles found template-literal-then-.trim()/.toLowerCase()chains only in unrelated string-building code, never as a directexec/child_processcommand argument) — the gap is a code-reasoning finding against the rule's own stated threat model, not an active exploit.