From 9a1becde487b4e93c5699067b03644eec72b1281 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 11:57:20 +0000 Subject: [PATCH 1/4] Initial plan From 6ab9a288aeaabc15da21eaef29041abd1eca7bf0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 12:04:16 +0000 Subject: [PATCH 2/4] Detect chained string methods on interpolated commands in exec/child_process rules Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- eslint-factory/README.md | 6 +- .../src/rules/command-initializer-utils.ts | 68 +++++++++++++++++++ ...child-process-interpolated-command.test.ts | 19 ++++++ .../no-child-process-interpolated-command.ts | 24 +------ .../no-exec-interpolated-command.test.ts | 24 +++++++ .../src/rules/no-exec-interpolated-command.ts | 37 +--------- 6 files changed, 119 insertions(+), 59 deletions(-) diff --git a/eslint-factory/README.md b/eslint-factory/README.md index 26a2c5f9a04..3416fd3a07f 100644 --- a/eslint-factory/README.md +++ b/eslint-factory/README.md @@ -600,9 +600,10 @@ Why: command strings evaluated by a shell (`exec`, `execSync`, `spawn` / `spawnS - `execFileSync("git " + branch, ["status"], { shell: true })` — shell-enabled execFileSync. - `spawn("git checkout " + branch, ...opts)` — spread options are treated conservatively as potentially shell-enabled. - ESM imports are recognized (`import { execSync } from "node:child_process"`). +- `` execSync(`git checkout ${branch}`.trim()) `` — chained string-normalizing methods (`trim`, `trimStart`, `trimEnd`, `toLowerCase`, `toUpperCase`, `replace`, `replaceAll`, `normalize`) are unwrapped before the check. **Not flagged:** -- Fully static command strings (`"git status"`, `` `git status` ``, and fully static `+` concatenations). +- Fully static command strings (`"git status"`, `` `git status` ``, and fully static `+` concatenations), including when a string-normalizing method is chained onto them. - `spawn(cmd, [args])` / `spawnSync(cmd, [args])` without `shell: true`. - `execFile` / `execFileSync` without `shell: true`. @@ -718,9 +719,10 @@ Disallow passing an interpolated template literal or dynamic string concatenatio **Detected forms:** - `` exec.exec(`git checkout ${branchName}`) `` — interpolated template literal. - `exec.exec("git " + branchName)` — dynamic string concatenation. +- `` exec.exec(`git checkout ${branchName}`.trim()) `` — chained string-normalizing methods (`trim`, `trimStart`, `trimEnd`, `toLowerCase`, `toUpperCase`, `replace`, `replaceAll`, `normalize`) are unwrapped before the check. **Not flagged:** -- Static command strings, including string concatenation of only static expressions. +- Static command strings, including string concatenation of only static expressions and chained string-normalizing methods on them. - Arguments passed correctly via the `args` array. ### `no-setfailed-then-exit-zero` diff --git a/eslint-factory/src/rules/command-initializer-utils.ts b/eslint-factory/src/rules/command-initializer-utils.ts index eaf2d5418b4..7ed7227fc59 100644 --- a/eslint-factory/src/rules/command-initializer-utils.ts +++ b/eslint-factory/src/rules/command-initializer-utils.ts @@ -46,3 +46,71 @@ export function resolveWriteOnceInitializerChain(expression: TSESTree.Expression } return candidate; } + +/** + * String methods that return a normalized copy of their receiver. Chaining one + * of these after a command string (for example `` `git checkout ${branch}`.trim() ``) + * keeps the interpolated value in the resulting command, so the receiver must be + * inspected instead of the outer call expression. + */ +const STRING_TRANSFORM_METHODS = new Set(["trim", "trimStart", "trimEnd", "toLowerCase", "toUpperCase", "toLocaleLowerCase", "toLocaleUpperCase", "replace", "replaceAll", "normalize"]); + +/** + * When the node is a call to a string-normalizing method (for example + * `.trim()` or `.toLowerCase()`), returns the receiver expression so callers + * can inspect the underlying command string. Returns null otherwise. + */ +function getStringTransformReceiver(node: TSESTree.Expression): TSESTree.Expression | null { + if (node.type !== AST_NODE_TYPES.CallExpression) return null; + const callee = node.callee; + if (callee.type !== AST_NODE_TYPES.MemberExpression || callee.computed) return null; + if (callee.property.type !== AST_NODE_TYPES.Identifier || !STRING_TRANSFORM_METHODS.has(callee.property.name)) return null; + return callee.object; +} + +/** + * Returns true when the node is a purely static expression (no runtime + * interpolation): a literal, a no-expression template literal, a binary `+` of + * two static expressions, or a string-normalizing method call on a static + * receiver. + */ +export function isStaticExpression(node: TSESTree.Expression): boolean { + if (node.type === AST_NODE_TYPES.Literal) return true; + if (node.type === AST_NODE_TYPES.TemplateLiteral) return node.expressions.length === 0; + if (node.type === AST_NODE_TYPES.BinaryExpression && node.operator === "+") { + return isStaticExpression(node.left) && isStaticExpression(node.right); + } + const receiver = getStringTransformReceiver(node); + if (receiver) return isStaticExpression(receiver); + return false; +} + +/** + * Returns true when the node is a dynamic string concatenation (binary `+` + * that is not entirely static). + */ +export function isDynamicStringConcatenation(node: TSESTree.Expression): boolean { + return node.type === AST_NODE_TYPES.BinaryExpression && node.operator === "+" && !isStaticExpression(node); +} + +/** + * Returns the display kind string for the problematic command expression, or + * null when the expression is not one of the flagged shapes. + * + * Write-once local bindings and chained string-normalizing calls (for example + * `` `git checkout ${branch}`.trim() ``) are unwrapped before the check so the + * underlying command string is inspected. + */ +export function getDynamicCommandKind(expression: TSESTree.Expression, sourceCode: TSESLint.SourceCode): string | null { + const seen = new Set(); + let candidate = resolveWriteOnceInitializerChain(expression, sourceCode); + while (!seen.has(candidate)) { + seen.add(candidate); + if (candidate.type === AST_NODE_TYPES.TemplateLiteral && candidate.expressions.length > 0) return "interpolated template literal"; + if (isDynamicStringConcatenation(candidate)) return "dynamic string concatenation"; + const receiver = getStringTransformReceiver(candidate); + if (!receiver) return null; + candidate = resolveWriteOnceInitializerChain(receiver, sourceCode); + } + return null; +} diff --git a/eslint-factory/src/rules/no-child-process-interpolated-command.test.ts b/eslint-factory/src/rules/no-child-process-interpolated-command.test.ts index 74d9f682a7d..64fbe9c40d4 100644 --- a/eslint-factory/src/rules/no-child-process-interpolated-command.test.ts +++ b/eslint-factory/src/rules/no-child-process-interpolated-command.test.ts @@ -33,12 +33,31 @@ describe("no-child-process-interpolated-command", () => { code: `import { execSync } from "child_process"; import { cmd } from "./cmd"; execSync(cmd);`, languageOptions: { sourceType: "module" }, }, + // Static command with a chained string method — still static, safe + { code: `const { execSync } = require("child_process"); execSync("git status".trim());` }, + // Fully static concatenation with a chained string method — safe + { code: `const { execSync } = require("child_process"); execSync(("git" + " status").toLowerCase());` }, ], invalid: [ { code: `const { execSync } = require("child_process"); execSync(\`git checkout \${branch}\`);`, errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "execSync" } }], }, + // Chained .trim() must not defeat the check + { + code: `const { execSync } = require("child_process"); execSync(\`git checkout \${branch}\`.trim());`, + errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "execSync" } }], + }, + // Chained .toLowerCase() must not defeat the check + { + code: `const { execSync } = require("child_process"); execSync(\`git log --author=\${author}\`.toLowerCase());`, + errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "execSync" } }], + }, + // Chained string method on a dynamic concatenation is also flagged + { + code: `const { execSync } = require("child_process"); execSync(("git checkout " + branch).trim());`, + errors: [{ messageId: "interpolatedCommand", data: { kind: "dynamic string concatenation", method: "execSync" } }], + }, { code: `function run(x) { const { execSync } = require("child_process"); const cmd = \`git log --author=\${x}\`; execSync(cmd); }`, errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "execSync" } }], diff --git a/eslint-factory/src/rules/no-child-process-interpolated-command.ts b/eslint-factory/src/rules/no-child-process-interpolated-command.ts index 68106cc1cd8..b41d91c1ec8 100644 --- a/eslint-factory/src/rules/no-child-process-interpolated-command.ts +++ b/eslint-factory/src/rules/no-child-process-interpolated-command.ts @@ -1,5 +1,5 @@ import { AST_NODE_TYPES, ESLintUtils, TSESLint, TSESTree } from "@typescript-eslint/utils"; -import { resolveWriteOnceInitializerChain } from "./command-initializer-utils"; +import { getDynamicCommandKind } from "./command-initializer-utils"; import { isChildProcessImportBinding, isChildProcessObjectBinding, isRequireChildProcess } from "./try-catch-rule-utils"; const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`); @@ -8,25 +8,6 @@ type SourceCodeScope = ReturnType; type ChildProcessMethod = "exec" | "execSync" | "spawn" | "spawnSync" | "execFile" | "execFileSync"; const SHELL_CONDITIONAL_METHODS = new Set(["spawn", "spawnSync", "execFile", "execFileSync"]); -function isStaticExpression(node: TSESTree.Expression): boolean { - if (node.type === AST_NODE_TYPES.Literal) return true; - if (node.type === AST_NODE_TYPES.TemplateLiteral) return node.expressions.length === 0; - if (node.type === AST_NODE_TYPES.BinaryExpression && node.operator === "+") { - return isStaticExpression(node.left) && isStaticExpression(node.right); - } - return false; -} - -function isDynamicStringConcatenation(node: TSESTree.Expression): boolean { - return node.type === AST_NODE_TYPES.BinaryExpression && node.operator === "+" && !isStaticExpression(node); -} - -function getDynamicCommandKind(node: TSESTree.Expression): string | null { - if (node.type === AST_NODE_TYPES.TemplateLiteral && node.expressions.length > 0) return "interpolated template literal"; - if (isDynamicStringConcatenation(node)) return "dynamic string concatenation"; - return null; -} - function getImportSpecifierName(node: TSESTree.ImportSpecifier): string | null { if (node.imported.type === AST_NODE_TYPES.Identifier) return node.imported.name; if (node.imported.type === AST_NODE_TYPES.Literal && typeof node.imported.value === "string") return node.imported.value; @@ -176,8 +157,7 @@ export const noChildProcessInterpolatedCommandRule = createRule({ const firstArg = node.arguments[0]; if (!firstArg || firstArg.type === AST_NODE_TYPES.SpreadElement) return; - const candidate = resolveWriteOnceInitializerChain(firstArg as TSESTree.Expression, sourceCode); - const kind = getDynamicCommandKind(candidate); + const kind = getDynamicCommandKind(firstArg as TSESTree.Expression, sourceCode); if (!kind) return; context.report({ diff --git a/eslint-factory/src/rules/no-exec-interpolated-command.test.ts b/eslint-factory/src/rules/no-exec-interpolated-command.test.ts index 944eede606e..233b98fd885 100644 --- a/eslint-factory/src/rules/no-exec-interpolated-command.test.ts +++ b/eslint-factory/src/rules/no-exec-interpolated-command.test.ts @@ -47,6 +47,10 @@ describe("no-exec-interpolated-command", () => { { code: `(function(cmd) { exec.exec(cmd, []); })("git");` }, // Cross-function binding is intentionally out of scope { code: "function outer(branch) { const cmd = `git checkout ${branch}`; function inner() { exec.exec(cmd, []); } }" }, + // Static template literal with a chained string method — still static, safe + { code: "exec.exec(`git`.trim(), [branch]);" }, + // Fully static concatenation with a chained string method — safe + { code: `exec.exec(("git" + " checkout").toLowerCase(), [branch]);` }, ], invalid: [ // Template literal with interpolation as command @@ -99,6 +103,26 @@ describe("no-exec-interpolated-command", () => { code: `const cmd = "git checkout " + branchName; exec.exec(cmd, []);`, errors: [{ messageId: "interpolatedCommand", data: { kind: "dynamic string concatenation", method: "exec" } }], }, + // Chained .trim() must not defeat the check + { + code: "exec.exec(`git checkout ${branch}`.trim(), []);", + errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "exec" } }], + }, + // Chained .toLowerCase() must not defeat the check + { + code: "exec.exec(`git log --author=${author}`.toLowerCase(), []);", + errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "exec" } }], + }, + // Chained string methods on a dynamic concatenation are also flagged + { + code: `exec.exec(("git checkout " + branchName).trim(), []);`, + errors: [{ messageId: "interpolatedCommand", data: { kind: "dynamic string concatenation", method: "exec" } }], + }, + // Chained string method on a variable holding a dynamic command + { + code: "function run(branch) { const cmd = `git checkout ${branch}`; exec.exec(cmd.trim(), []); }", + errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "exec" } }], + }, // Chained aliases are also flagged when they resolve to a dynamic command { code: "function run(branch) { const dynamic = `git checkout ${branch}`; const cmd = dynamic; exec.exec(cmd, []); }", diff --git a/eslint-factory/src/rules/no-exec-interpolated-command.ts b/eslint-factory/src/rules/no-exec-interpolated-command.ts index 778790550eb..c00d33cb21d 100644 --- a/eslint-factory/src/rules/no-exec-interpolated-command.ts +++ b/eslint-factory/src/rules/no-exec-interpolated-command.ts @@ -1,41 +1,9 @@ import { AST_NODE_TYPES, ESLintUtils, TSESTree } from "@typescript-eslint/utils"; -import { resolveWriteOnceInitializerChain } from "./command-initializer-utils"; +import { getDynamicCommandKind } from "./command-initializer-utils"; const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`); type ExecMethodName = "exec" | "getExecOutput"; -/** - * Returns true when the node is a purely static expression (no runtime - * interpolation): a literal, a no-expression template literal, or a binary - * `+` of two static expressions. - */ -function isStaticExpression(node: TSESTree.Expression): boolean { - if (node.type === "Literal") return true; - if (node.type === "TemplateLiteral") return node.expressions.length === 0; - if (node.type === "BinaryExpression" && node.operator === "+") { - return isStaticExpression(node.left) && isStaticExpression(node.right); - } - return false; -} - -/** - * Returns true when the node is a dynamic string concatenation (binary `+` - * that is not entirely static). - */ -function isDynamicStringConcatenation(node: TSESTree.Expression): boolean { - return node.type === "BinaryExpression" && node.operator === "+" && !isStaticExpression(node); -} - -/** - * Returns the display kind string for the problematic first argument, or null - * when the argument is not one of the flagged shapes. - */ -function getDynamicCommandKind(node: TSESTree.Expression): string | null { - if (node.type === "TemplateLiteral" && node.expressions.length > 0) return "interpolated template literal"; - if (isDynamicStringConcatenation(node)) return "dynamic string concatenation"; - return null; -} - /** * Returns true when the call expression looks like `exec.exec(...)` or * `exec.getExecOutput(...)` — the `exec` global injected by github-script. @@ -86,8 +54,7 @@ export const noExecInterpolatedCommandRule = createRule({ const firstArg = node.arguments[0]; if (!firstArg || firstArg.type === AST_NODE_TYPES.SpreadElement) return; - const candidate = resolveWriteOnceInitializerChain(firstArg as TSESTree.Expression, sourceCode); - const kind = getDynamicCommandKind(candidate); + const kind = getDynamicCommandKind(firstArg as TSESTree.Expression, sourceCode); if (!kind) return; context.report({ From 42b20b1b08da9c1be597e72b461536c3d4f24279 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 12:06:01 +0000 Subject: [PATCH 3/4] Inspect string-transform call arguments so dynamic .replace() replacements are flagged Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../src/rules/command-initializer-utils.ts | 51 ++++++++++++------- .../no-exec-interpolated-command.test.ts | 7 +++ 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/eslint-factory/src/rules/command-initializer-utils.ts b/eslint-factory/src/rules/command-initializer-utils.ts index 7ed7227fc59..2e2a29712f8 100644 --- a/eslint-factory/src/rules/command-initializer-utils.ts +++ b/eslint-factory/src/rules/command-initializer-utils.ts @@ -57,22 +57,24 @@ const STRING_TRANSFORM_METHODS = new Set(["trim", "trimStart", "trimEnd", "toLow /** * When the node is a call to a string-normalizing method (for example - * `.trim()` or `.toLowerCase()`), returns the receiver expression so callers - * can inspect the underlying command string. Returns null otherwise. + * `.trim()` or `.toLowerCase()`), returns the receiver expression and the call + * arguments so callers can inspect the underlying command string. Returns null + * otherwise. */ -function getStringTransformReceiver(node: TSESTree.Expression): TSESTree.Expression | null { +function getStringTransformCall(node: TSESTree.Expression): { receiver: TSESTree.Expression; args: TSESTree.CallExpressionArgument[] } | null { if (node.type !== AST_NODE_TYPES.CallExpression) return null; const callee = node.callee; if (callee.type !== AST_NODE_TYPES.MemberExpression || callee.computed) return null; if (callee.property.type !== AST_NODE_TYPES.Identifier || !STRING_TRANSFORM_METHODS.has(callee.property.name)) return null; - return callee.object; + return { receiver: callee.object, args: node.arguments }; } /** * Returns true when the node is a purely static expression (no runtime * interpolation): a literal, a no-expression template literal, a binary `+` of - * two static expressions, or a string-normalizing method call on a static - * receiver. + * two static expressions, or a string-normalizing method call whose receiver + * and arguments are all static (for example `.replace()` can inject dynamic + * content through its replacement argument). */ export function isStaticExpression(node: TSESTree.Expression): boolean { if (node.type === AST_NODE_TYPES.Literal) return true; @@ -80,8 +82,11 @@ export function isStaticExpression(node: TSESTree.Expression): boolean { if (node.type === AST_NODE_TYPES.BinaryExpression && node.operator === "+") { return isStaticExpression(node.left) && isStaticExpression(node.right); } - const receiver = getStringTransformReceiver(node); - if (receiver) return isStaticExpression(receiver); + const transform = getStringTransformCall(node); + if (transform) { + if (!isStaticExpression(transform.receiver)) return false; + return transform.args.every(arg => arg.type !== AST_NODE_TYPES.SpreadElement && isStaticExpression(arg)); + } return false; } @@ -99,18 +104,26 @@ export function isDynamicStringConcatenation(node: TSESTree.Expression): boolean * * Write-once local bindings and chained string-normalizing calls (for example * `` `git checkout ${branch}`.trim() ``) are unwrapped before the check so the - * underlying command string is inspected. + * underlying command string is inspected. For calls that accept arguments (for + * example `.replace(pattern, value)`), the arguments are inspected as well. */ -export function getDynamicCommandKind(expression: TSESTree.Expression, sourceCode: TSESLint.SourceCode): string | null { - const seen = new Set(); - let candidate = resolveWriteOnceInitializerChain(expression, sourceCode); - while (!seen.has(candidate)) { - seen.add(candidate); - if (candidate.type === AST_NODE_TYPES.TemplateLiteral && candidate.expressions.length > 0) return "interpolated template literal"; - if (isDynamicStringConcatenation(candidate)) return "dynamic string concatenation"; - const receiver = getStringTransformReceiver(candidate); - if (!receiver) return null; - candidate = resolveWriteOnceInitializerChain(receiver, sourceCode); +export function getDynamicCommandKind(expression: TSESTree.Expression, sourceCode: TSESLint.SourceCode, seen: Set = new Set()): string | null { + const candidate = resolveWriteOnceInitializerChain(expression, sourceCode); + if (seen.has(candidate)) return null; + seen.add(candidate); + + if (candidate.type === AST_NODE_TYPES.TemplateLiteral && candidate.expressions.length > 0) return "interpolated template literal"; + if (isDynamicStringConcatenation(candidate)) return "dynamic string concatenation"; + + const transform = getStringTransformCall(candidate); + if (!transform) return null; + + const receiverKind = getDynamicCommandKind(transform.receiver, sourceCode, seen); + if (receiverKind) return receiverKind; + for (const arg of transform.args) { + if (arg.type === AST_NODE_TYPES.SpreadElement) continue; + const argKind = getDynamicCommandKind(arg, sourceCode, seen); + if (argKind) return argKind; } return null; } diff --git a/eslint-factory/src/rules/no-exec-interpolated-command.test.ts b/eslint-factory/src/rules/no-exec-interpolated-command.test.ts index 233b98fd885..4dc022f4897 100644 --- a/eslint-factory/src/rules/no-exec-interpolated-command.test.ts +++ b/eslint-factory/src/rules/no-exec-interpolated-command.test.ts @@ -51,6 +51,8 @@ describe("no-exec-interpolated-command", () => { { code: "exec.exec(`git`.trim(), [branch]);" }, // Fully static concatenation with a chained string method — safe { code: `exec.exec(("git" + " checkout").toLowerCase(), [branch]);` }, + // Static replace with static arguments — safe + { code: `exec.exec("git-checkout".replace("-", " "), [branch]);` }, ], invalid: [ // Template literal with interpolation as command @@ -123,6 +125,11 @@ describe("no-exec-interpolated-command", () => { code: "function run(branch) { const cmd = `git checkout ${branch}`; exec.exec(cmd.trim(), []); }", errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "exec" } }], }, + // Dynamic replacement argument on a static receiver is also flagged + { + code: 'exec.exec("git checkout PLACEHOLDER".replace("PLACEHOLDER", `${branch}-x`), []);', + errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "exec" } }], + }, // Chained aliases are also flagged when they resolve to a dynamic command { code: "function run(branch) { const dynamic = `git checkout ${branch}`; const cmd = dynamic; exec.exec(cmd, []); }", From 298c8565c0da4a46193a966646bea0ddc4424b13 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:15:48 +0000 Subject: [PATCH 4/4] Detect dynamic replace/replaceAll callback returns; document toLocale case methods Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- eslint-factory/README.md | 6 +- .../src/rules/command-initializer-utils.ts | 72 ++++++++++++++++++- ...child-process-interpolated-command.test.ts | 12 ++++ .../no-exec-interpolated-command.test.ts | 12 ++++ 4 files changed, 98 insertions(+), 4 deletions(-) diff --git a/eslint-factory/README.md b/eslint-factory/README.md index 3416fd3a07f..d404861e485 100644 --- a/eslint-factory/README.md +++ b/eslint-factory/README.md @@ -600,7 +600,8 @@ Why: command strings evaluated by a shell (`exec`, `execSync`, `spawn` / `spawnS - `execFileSync("git " + branch, ["status"], { shell: true })` — shell-enabled execFileSync. - `spawn("git checkout " + branch, ...opts)` — spread options are treated conservatively as potentially shell-enabled. - ESM imports are recognized (`import { execSync } from "node:child_process"`). -- `` execSync(`git checkout ${branch}`.trim()) `` — chained string-normalizing methods (`trim`, `trimStart`, `trimEnd`, `toLowerCase`, `toUpperCase`, `replace`, `replaceAll`, `normalize`) are unwrapped before the check. +- `` execSync(`git checkout ${branch}`.trim()) `` — chained string-normalizing methods (`trim`, `trimStart`, `trimEnd`, `toLowerCase`, `toUpperCase`, `toLocaleLowerCase`, `toLocaleUpperCase`, `replace`, `replaceAll`, `normalize`) are unwrapped before the check. +- `` execSync("git checkout PLACEHOLDER".replace("PLACEHOLDER", () => branch)) `` — a `.replace()` / `.replaceAll()` replacer callback's return value is also inspected. **Not flagged:** - Fully static command strings (`"git status"`, `` `git status` ``, and fully static `+` concatenations), including when a string-normalizing method is chained onto them. @@ -719,7 +720,8 @@ Disallow passing an interpolated template literal or dynamic string concatenatio **Detected forms:** - `` exec.exec(`git checkout ${branchName}`) `` — interpolated template literal. - `exec.exec("git " + branchName)` — dynamic string concatenation. -- `` exec.exec(`git checkout ${branchName}`.trim()) `` — chained string-normalizing methods (`trim`, `trimStart`, `trimEnd`, `toLowerCase`, `toUpperCase`, `replace`, `replaceAll`, `normalize`) are unwrapped before the check. +- `` exec.exec(`git checkout ${branchName}`.trim()) `` — chained string-normalizing methods (`trim`, `trimStart`, `trimEnd`, `toLowerCase`, `toUpperCase`, `toLocaleLowerCase`, `toLocaleUpperCase`, `replace`, `replaceAll`, `normalize`) are unwrapped before the check. +- `` exec.exec("git checkout PLACEHOLDER".replace("PLACEHOLDER", () => branchName)) `` — a `.replace()` / `.replaceAll()` replacer callback's return value is also inspected. **Not flagged:** - Static command strings, including string concatenation of only static expressions and chained string-normalizing methods on them. diff --git a/eslint-factory/src/rules/command-initializer-utils.ts b/eslint-factory/src/rules/command-initializer-utils.ts index 2e2a29712f8..ba123d1d9c1 100644 --- a/eslint-factory/src/rules/command-initializer-utils.ts +++ b/eslint-factory/src/rules/command-initializer-utils.ts @@ -98,6 +98,72 @@ export function isDynamicStringConcatenation(node: TSESTree.Expression): boolean return node.type === AST_NODE_TYPES.BinaryExpression && node.operator === "+" && !isStaticExpression(node); } +/** + * Collects the argument expressions of every `return` statement reachable + * from `node` without crossing into a nested function boundary. Used to + * inspect what a `.replace()` / `.replaceAll()` replacer callback can inject + * into the resulting command string. + */ +function collectReturnArguments(node: TSESTree.Statement, results: TSESTree.Expression[]): void { + switch (node.type) { + case AST_NODE_TYPES.ReturnStatement: + if (node.argument) results.push(node.argument); + return; + case AST_NODE_TYPES.BlockStatement: + for (const stmt of node.body) collectReturnArguments(stmt, results); + return; + case AST_NODE_TYPES.IfStatement: + collectReturnArguments(node.consequent, results); + if (node.alternate) collectReturnArguments(node.alternate, results); + return; + case AST_NODE_TYPES.ForStatement: + case AST_NODE_TYPES.ForInStatement: + case AST_NODE_TYPES.ForOfStatement: + case AST_NODE_TYPES.WhileStatement: + case AST_NODE_TYPES.DoWhileStatement: + collectReturnArguments(node.body, results); + return; + case AST_NODE_TYPES.TryStatement: + collectReturnArguments(node.block, results); + if (node.handler) collectReturnArguments(node.handler.body, results); + if (node.finalizer) collectReturnArguments(node.finalizer, results); + return; + case AST_NODE_TYPES.SwitchStatement: + for (const switchCase of node.cases) { + for (const stmt of switchCase.consequent) collectReturnArguments(stmt, results); + } + return; + case AST_NODE_TYPES.LabeledStatement: + collectReturnArguments(node.body, results); + return; + default: + // Do not descend into nested function/arrow expressions or other + // statement kinds that cannot contain a same-scope `return`. + return; + } +} + +/** + * When `node` is a function or arrow expression (for example a `.replace()` / + * `.replaceAll()` replacer callback), returns the dynamic kind of any value it + * can return: the expression body of an arrow function, or the argument of + * any `return` statement reachable without crossing a nested function + * boundary. Returns null when `node` is not a function/arrow expression or + * none of its return values are dynamic. + */ +function getDynamicKindFromCallbackReturn(node: TSESTree.Node, sourceCode: TSESLint.SourceCode, seen: Set): string | null { + if (node.type !== AST_NODE_TYPES.ArrowFunctionExpression && node.type !== AST_NODE_TYPES.FunctionExpression) return null; + if (node.body.type !== AST_NODE_TYPES.BlockStatement) return getDynamicCommandKind(node.body, sourceCode, seen); + + const returnArguments: TSESTree.Expression[] = []; + collectReturnArguments(node.body, returnArguments); + for (const argument of returnArguments) { + const kind = getDynamicCommandKind(argument, sourceCode, seen); + if (kind) return kind; + } + return null; +} + /** * Returns the display kind string for the problematic command expression, or * null when the expression is not one of the flagged shapes. @@ -105,7 +171,9 @@ export function isDynamicStringConcatenation(node: TSESTree.Expression): boolean * Write-once local bindings and chained string-normalizing calls (for example * `` `git checkout ${branch}`.trim() ``) are unwrapped before the check so the * underlying command string is inspected. For calls that accept arguments (for - * example `.replace(pattern, value)`), the arguments are inspected as well. + * example `.replace(pattern, value)`), the arguments are inspected as well, + * including a replacer callback's return value (for example + * `.replace(pattern, () => branch)`). */ export function getDynamicCommandKind(expression: TSESTree.Expression, sourceCode: TSESLint.SourceCode, seen: Set = new Set()): string | null { const candidate = resolveWriteOnceInitializerChain(expression, sourceCode); @@ -122,7 +190,7 @@ export function getDynamicCommandKind(expression: TSESTree.Expression, sourceCod if (receiverKind) return receiverKind; for (const arg of transform.args) { if (arg.type === AST_NODE_TYPES.SpreadElement) continue; - const argKind = getDynamicCommandKind(arg, sourceCode, seen); + const argKind = getDynamicCommandKind(arg, sourceCode, seen) ?? getDynamicKindFromCallbackReturn(arg, sourceCode, seen); if (argKind) return argKind; } return null; diff --git a/eslint-factory/src/rules/no-child-process-interpolated-command.test.ts b/eslint-factory/src/rules/no-child-process-interpolated-command.test.ts index 64fbe9c40d4..e5c49c40877 100644 --- a/eslint-factory/src/rules/no-child-process-interpolated-command.test.ts +++ b/eslint-factory/src/rules/no-child-process-interpolated-command.test.ts @@ -37,6 +37,8 @@ describe("no-child-process-interpolated-command", () => { { code: `const { execSync } = require("child_process"); execSync("git status".trim());` }, // Fully static concatenation with a chained string method — safe { code: `const { execSync } = require("child_process"); execSync(("git" + " status").toLowerCase());` }, + // Static replacer callback with a fully static return value — safe + { code: `const { execSync } = require("child_process"); execSync("git-status".replace("-", () => " "));` }, ], invalid: [ { @@ -58,6 +60,16 @@ describe("no-child-process-interpolated-command", () => { code: `const { execSync } = require("child_process"); execSync(("git checkout " + branch).trim());`, errors: [{ messageId: "interpolatedCommand", data: { kind: "dynamic string concatenation", method: "execSync" } }], }, + // Replacer callback returning a dynamic template literal is also flagged + { + code: `const { execSync } = require("child_process"); execSync("git checkout PLACEHOLDER".replace("PLACEHOLDER", () => \`\${branch}-x\`));`, + errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "execSync" } }], + }, + // Replacer callback with a block body / return statement is also flagged + { + code: `const { execSync } = require("child_process"); execSync("git checkout PLACEHOLDER".replaceAll("PLACEHOLDER", function() { return "x-" + branch; }));`, + errors: [{ messageId: "interpolatedCommand", data: { kind: "dynamic string concatenation", method: "execSync" } }], + }, { code: `function run(x) { const { execSync } = require("child_process"); const cmd = \`git log --author=\${x}\`; execSync(cmd); }`, errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "execSync" } }], diff --git a/eslint-factory/src/rules/no-exec-interpolated-command.test.ts b/eslint-factory/src/rules/no-exec-interpolated-command.test.ts index 4dc022f4897..3b1c45bd1a8 100644 --- a/eslint-factory/src/rules/no-exec-interpolated-command.test.ts +++ b/eslint-factory/src/rules/no-exec-interpolated-command.test.ts @@ -53,6 +53,8 @@ describe("no-exec-interpolated-command", () => { { code: `exec.exec(("git" + " checkout").toLowerCase(), [branch]);` }, // Static replace with static arguments — safe { code: `exec.exec("git-checkout".replace("-", " "), [branch]);` }, + // Static replace with a fully static replacer callback — safe + { code: `exec.exec("git-checkout".replace("-", () => " "), [branch]);` }, ], invalid: [ // Template literal with interpolation as command @@ -130,6 +132,16 @@ describe("no-exec-interpolated-command", () => { code: 'exec.exec("git checkout PLACEHOLDER".replace("PLACEHOLDER", `${branch}-x`), []);', errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "exec" } }], }, + // Replacer callback returning a dynamic template literal is also flagged + { + code: 'exec.exec("git checkout PLACEHOLDER".replace("PLACEHOLDER", () => `${branch}-x`), []);', + errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "exec" } }], + }, + // Replacer callback with a block body / return statement is also flagged + { + code: 'exec.exec("git checkout PLACEHOLDER".replaceAll("PLACEHOLDER", function() { return "x-" + branch; }), []);', + errors: [{ messageId: "interpolatedCommand", data: { kind: "dynamic string concatenation", method: "exec" } }], + }, // Chained aliases are also flagged when they resolve to a dynamic command { code: "function run(branch) { const dynamic = `git checkout ${branch}`; const cmd = dynamic; exec.exec(cmd, []); }",