Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions eslint-factory/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -600,9 +600,11 @@ 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`, `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).
- 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`.

Expand Down Expand Up @@ -718,9 +720,11 @@ 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`, `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.
- 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`
Expand Down
149 changes: 149 additions & 0 deletions eslint-factory/src/rules/command-initializer-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,152 @@ 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 and the call
* arguments so callers can inspect the underlying command string. Returns null
* otherwise.
*/
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 { 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 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;
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 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;
}

/**
* 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);
}

/**
* 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<TSESTree.Expression>): 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.
*
* 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,
* including a replacer callback's return value (for example
* `.replace(pattern, () => branch)`).
*/
export function getDynamicCommandKind(expression: TSESTree.Expression, sourceCode: TSESLint.SourceCode, seen: Set<TSESTree.Expression> = 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) ?? getDynamicKindFromCallbackReturn(arg, sourceCode, seen);
if (argKind) return argKind;
Comment on lines +191 to +194
}
return null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,43 @@ 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());` },
// Static replacer callback with a fully static return value — safe
{ code: `const { execSync } = require("child_process"); execSync("git-status".replace("-", () => " "));` },
],
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" } }],
},
// 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" } }],
Expand Down
24 changes: 2 additions & 22 deletions eslint-factory/src/rules/no-child-process-interpolated-command.ts
Original file line number Diff line number Diff line change
@@ -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}`);
Expand All @@ -8,25 +8,6 @@ type SourceCodeScope = ReturnType<TSESLint.SourceCode["getScope"]>;
type ChildProcessMethod = "exec" | "execSync" | "spawn" | "spawnSync" | "execFile" | "execFileSync";
const SHELL_CONDITIONAL_METHODS = new Set<ChildProcessMethod>(["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;
Expand Down Expand Up @@ -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({
Expand Down
43 changes: 43 additions & 0 deletions eslint-factory/src/rules/no-exec-interpolated-command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ 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]);` },
// 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
Expand Down Expand Up @@ -99,6 +107,41 @@ 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" } }],
},
// 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" } }],
},
// 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, []); }",
Expand Down
Loading