From 99ef0f768414ecfeeb22a97e192bb666fde2af19 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 20 Jul 2026 09:54:38 +0000 Subject: [PATCH 1/2] eslint: add require-execsync-try-catch rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit execSync from child_process throws a ChildProcessError when the child process exits non-zero or is killed by a signal. An unhandled throw crashes the action step with no useful diagnostic. Evidence in actions/setup/js: - start_mcp_gateway.cjs:824 — bare execSync(`node "${converterPath}"`) without a surrounding try/catch (confirmed by lint:setup-js producing one new warning for this exact line) The rule: - Detects execSync sourced from child_process / node:child_process via destructured require, namespace require, or ESM import - Skips calls already inside a try block - Offers a hasSuggestions fixer to wrap the statement in try/catch with { cause: err } re-throw - Zero false positives: execSync from unrelated modules and bare (unresolved) identifiers are ignored Tests: 8 new unit tests (valid + invalid cases); all 306 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- eslint-factory/eslint.config.cjs | 1 + eslint-factory/src/index.ts | 2 + .../rules/require-execsync-try-catch.test.ts | 149 +++++++++++++ .../src/rules/require-execsync-try-catch.ts | 202 ++++++++++++++++++ 4 files changed, 354 insertions(+) create mode 100644 eslint-factory/src/rules/require-execsync-try-catch.test.ts create mode 100644 eslint-factory/src/rules/require-execsync-try-catch.ts diff --git a/eslint-factory/eslint.config.cjs b/eslint-factory/eslint.config.cjs index b61b4ff1b6f..c29a90335e9 100644 --- a/eslint-factory/eslint.config.cjs +++ b/eslint-factory/eslint.config.cjs @@ -34,6 +34,7 @@ module.exports = [ "gh-aw-custom/prefer-core-logging": "warn", "gh-aw-custom/no-core-error-then-process-exit": "warn", "gh-aw-custom/no-exec-interpolated-command": "warn", + "gh-aw-custom/require-execsync-try-catch": "warn", }, }, { diff --git a/eslint-factory/src/index.ts b/eslint-factory/src/index.ts index 8e574b0e61a..f14c137cf8b 100644 --- a/eslint-factory/src/index.ts +++ b/eslint-factory/src/index.ts @@ -20,6 +20,7 @@ import { requireNewUrlTryCatchRule } from "./rules/require-new-url-try-catch"; import { preferCoreLoggingRule } from "./rules/prefer-core-logging"; import { noCoreErrorThenProcessExitRule } from "./rules/no-core-error-then-process-exit"; import { noExecInterpolatedCommandRule } from "./rules/no-exec-interpolated-command"; +import { requireExecSyncTryCatchRule } from "./rules/require-execsync-try-catch"; const plugin = { meta: { @@ -49,6 +50,7 @@ const plugin = { "prefer-core-logging": preferCoreLoggingRule, "no-core-error-then-process-exit": noCoreErrorThenProcessExitRule, "no-exec-interpolated-command": noExecInterpolatedCommandRule, + "require-execsync-try-catch": requireExecSyncTryCatchRule, }, }; diff --git a/eslint-factory/src/rules/require-execsync-try-catch.test.ts b/eslint-factory/src/rules/require-execsync-try-catch.test.ts new file mode 100644 index 00000000000..7f04e31eb19 --- /dev/null +++ b/eslint-factory/src/rules/require-execsync-try-catch.test.ts @@ -0,0 +1,149 @@ +import { RuleTester } from "eslint"; +import { describe, it } from "vitest"; +import { requireExecSyncTryCatchRule } from "./require-execsync-try-catch"; + +const cjsRuleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: "commonjs", + }, +}); + +const esmRuleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: "module", + }, +}); + +describe("require-execsync-try-catch", () => { + it("valid: execSync inside try block passes (CommonJS, destructured)", () => { + cjsRuleTester.run("require-execsync-try-catch", requireExecSyncTryCatchRule, { + valid: [ + `const { execSync } = require("child_process"); try { execSync("ls"); } catch (e) {}`, + `const { execSync } = require("node:child_process"); try { execSync("ls"); } catch (e) {}`, + `const { execSync } = require("child_process"); function f() { try { execSync("ls"); } catch (e) {} }`, + ], + invalid: [], + }); + }); + + it("valid: execSync inside try block passes (CommonJS, namespace)", () => { + cjsRuleTester.run("require-execsync-try-catch", requireExecSyncTryCatchRule, { + valid: [`const cp = require("child_process"); try { cp.execSync("ls"); } catch (e) {}`], + invalid: [], + }); + }); + + it("valid: execSync inside try block passes (ES module)", () => { + esmRuleTester.run("require-execsync-try-catch", requireExecSyncTryCatchRule, { + valid: [ + `import { execSync } from "child_process"; try { execSync("ls"); } catch (e) {}`, + `import { execSync } from "node:child_process"; try { execSync("ls"); } catch (e) {}`, + ], + invalid: [], + }); + }); + + it("valid: execSync from non-child_process module is ignored", () => { + cjsRuleTester.run("require-execsync-try-catch", requireExecSyncTryCatchRule, { + valid: [ + // execSync from an unrelated module — should not be flagged + `const { execSync } = require("some-other-lib"); execSync("ls");`, + // bare execSync without any require — should not be flagged + `execSync("ls");`, + // member call on unrelated object + `mockChild.execSync("ls");`, + ], + invalid: [], + }); + }); + + it("invalid: execSync without try/catch (CommonJS, destructured)", () => { + cjsRuleTester.run("require-execsync-try-catch", requireExecSyncTryCatchRule, { + valid: [], + invalid: [ + { + code: `const { execSync } = require("child_process"); execSync("ls");`, + errors: [ + { + messageId: "requireTryCatch", + suggestions: [ + { + messageId: "wrapInTryCatch", + output: `const { execSync } = require("child_process"); try {\n execSync("ls");\n} catch (err) {\n // TODO: handle execSync failure (non-zero exit / signal termination).\n throw new Error(\n "execSync failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n}`, + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: execSync without try/catch (CommonJS, namespace)", () => { + cjsRuleTester.run("require-execsync-try-catch", requireExecSyncTryCatchRule, { + valid: [], + invalid: [ + { + code: `const cp = require("child_process"); cp.execSync("ls");`, + errors: [ + { + messageId: "requireTryCatch", + suggestions: [ + { + messageId: "wrapInTryCatch", + output: `const cp = require("child_process"); try {\n cp.execSync("ls");\n} catch (err) {\n // TODO: handle execSync failure (non-zero exit / signal termination).\n throw new Error(\n "execSync failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n}`, + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: execSync without try/catch (ES module)", () => { + esmRuleTester.run("require-execsync-try-catch", requireExecSyncTryCatchRule, { + valid: [], + invalid: [ + { + code: `import { execSync } from "child_process"; execSync("ls");`, + errors: [ + { + messageId: "requireTryCatch", + suggestions: [ + { + messageId: "wrapInTryCatch", + output: `import { execSync } from "child_process"; try {\n execSync("ls");\n} catch (err) {\n // TODO: handle execSync failure (non-zero exit / signal termination).\n throw new Error(\n "execSync failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n}`, + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: aliased execSync without try/catch is flagged", () => { + cjsRuleTester.run("require-execsync-try-catch", requireExecSyncTryCatchRule, { + valid: [], + invalid: [ + { + code: `const { execSync: run } = require("child_process"); run("ls");`, + errors: [ + { + messageId: "requireTryCatch", + suggestions: [ + { + messageId: "wrapInTryCatch", + output: `const { execSync: run } = require("child_process"); try {\n run("ls");\n} catch (err) {\n // TODO: handle execSync failure (non-zero exit / signal termination).\n throw new Error(\n "execSync failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n}`, + }, + ], + }, + ], + }, + ], + }); + }); +}); diff --git a/eslint-factory/src/rules/require-execsync-try-catch.ts b/eslint-factory/src/rules/require-execsync-try-catch.ts new file mode 100644 index 00000000000..18e41d65794 --- /dev/null +++ b/eslint-factory/src/rules/require-execsync-try-catch.ts @@ -0,0 +1,202 @@ +import { AST_NODE_TYPES, ESLintUtils, TSESLint, TSESTree } from "@typescript-eslint/utils"; +import { buildTryCatchSuggestion, findEnclosingStatement, isDeferredCallback, isInsideTryBlock } from "./try-catch-rule-utils"; + +const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`); + +const CHILD_PROCESS_SPECIFIERS = new Set(["child_process", "node:child_process"]); + +type SourceCodeScope = ReturnType; + +function isRequireChildProcess(node: TSESTree.Node | null | undefined): boolean { + if (!node) return false; + return ( + node.type === AST_NODE_TYPES.CallExpression && + node.callee.type === AST_NODE_TYPES.Identifier && + node.callee.name === "require" && + node.arguments.length >= 1 && + node.arguments[0].type === AST_NODE_TYPES.Literal && + typeof (node.arguments[0] as TSESTree.Literal).value === "string" && + CHILD_PROCESS_SPECIFIERS.has((node.arguments[0] as TSESTree.Literal).value as string) + ); +} + +function isChildProcessImportBinding(def: { type: string; node: TSESTree.Node; parent?: TSESTree.Node | null }): boolean { + if (def.type !== "ImportBinding") return false; + if (!def.parent || def.parent.type !== AST_NODE_TYPES.ImportDeclaration) return false; + if (def.parent.source.type !== AST_NODE_TYPES.Literal) return false; + return typeof def.parent.source.value === "string" && CHILD_PROCESS_SPECIFIERS.has(def.parent.source.value); +} + +/** + * Walks the scope chain to decide whether `identifierName` resolves to + * `execSync` from `child_process`. + */ +function isExecSyncBinding(identifierName: string, scopeNode: TSESTree.Node, sourceCode: TSESLint.SourceCode): boolean { + let scope: SourceCodeScope | null = sourceCode.getScope(scopeNode); + while (scope) { + const variable = scope.set.get(identifierName); + if (variable && variable.defs.length > 0) { + for (const def of variable.defs) { + // ESM: import { execSync } from "child_process" + if (isChildProcessImportBinding(def) && def.node.type === AST_NODE_TYPES.ImportSpecifier) { + const specifier = def.node as TSESTree.ImportSpecifier; + const importedName = specifier.imported.type === AST_NODE_TYPES.Identifier ? specifier.imported.name : null; + if (importedName === "execSync") return true; + } + // CJS: const { execSync } = require("child_process") + if (def.type === "Variable") { + const declarator = def.node as TSESTree.VariableDeclarator; + if (declarator.id.type === AST_NODE_TYPES.ObjectPattern && isRequireChildProcess(declarator.init)) { + for (const prop of declarator.id.properties) { + if (prop.type !== AST_NODE_TYPES.Property) continue; + if (prop.key.type !== AST_NODE_TYPES.Identifier || prop.key.name !== "execSync") continue; + const boundName = prop.value.type === AST_NODE_TYPES.Identifier ? prop.value.name : null; + if (boundName === identifierName) return true; + } + } + // const execSync = childProcess.execSync + if (declarator.id.type === AST_NODE_TYPES.Identifier && declarator.init?.type === AST_NODE_TYPES.MemberExpression) { + const init = declarator.init; + if ( + !init.computed && + init.object.type === AST_NODE_TYPES.Identifier && + isChildProcessObjectBinding(init.object.name, init.object, sourceCode) && + init.property.type === AST_NODE_TYPES.Identifier && + init.property.name === "execSync" + ) { + return true; + } + } + } + } + return false; + } + scope = scope.upper; + } + return false; +} + +function isChildProcessObjectBinding(name: string, scopeNode: TSESTree.Node, sourceCode: TSESLint.SourceCode): boolean { + let scope: SourceCodeScope | null = sourceCode.getScope(scopeNode); + while (scope) { + const variable = scope.set.get(name); + if (variable && variable.defs.length > 0) { + for (const def of variable.defs) { + if (def.type === "Variable") { + const declarator = def.node as TSESTree.VariableDeclarator; + if (declarator.id.type === AST_NODE_TYPES.Identifier && isRequireChildProcess(declarator.init)) { + return true; + } + } + if (isChildProcessImportBinding(def) && def.node.type === AST_NODE_TYPES.ImportNamespaceSpecifier) { + return true; + } + } + return false; + } + scope = scope.upper; + } + return false; +} + +/** + * Returns true if the CallExpression is an `execSync(...)` call sourced from + * the `child_process` module. + */ +function isExecSyncCall(node: TSESTree.CallExpression, sourceCode: TSESLint.SourceCode): boolean { + const callee = node.callee; + + // execSync(...) — destructured or aliased + if (callee.type === AST_NODE_TYPES.Identifier) { + return isExecSyncBinding(callee.name, callee, sourceCode); + } + + // childProcess.execSync(...) or cp.execSync(...) + if ( + callee.type === AST_NODE_TYPES.MemberExpression && + !callee.computed && + callee.object.type === AST_NODE_TYPES.Identifier && + callee.property.type === AST_NODE_TYPES.Identifier && + callee.property.name === "execSync" + ) { + return isChildProcessObjectBinding(callee.object.name, callee.object, sourceCode); + } + + return false; +} + +export const requireExecSyncTryCatchRule = createRule({ + name: "require-execsync-try-catch", + meta: { + type: "problem", + hasSuggestions: true, + docs: { + description: + "Require execSync calls in actions/setup/js scripts to be wrapped in try/catch. " + + "execSync throws a ChildProcessError when the child process exits with a non-zero status code or is killed by a signal; " + + "an unhandled throw crashes the action without surfacing a useful diagnostic.", + }, + schema: [], + messages: { + requireTryCatch: + "Wrap execSync({{arg}}) in try/catch — execSync throws when the process exits non-zero or is killed by a signal, " + + "and will crash the action if the error is unhandled.", + wrapInTryCatch: "Wrap in try { ... } catch { ... } and re-throw with { cause: err } to preserve context.", + }, + }, + defaultOptions: [], + create(context) { + const sourceCode = context.sourceCode; + + return { + CallExpression(node) { + if (!isExecSyncCall(node, sourceCode)) return; + if (isInsideTryBlock(sourceCode, node)) return; + + // Ignore execSync inside deferred callbacks — the parent try block does not protect them. + // isInsideTryBlock already handles this, but we skip reporting when the node itself is + // inside a deferred callback that has no enclosing try block (same FP-avoidance as other rules). + const ancestors = sourceCode.getAncestors(node); + let withinDeferredBoundary = false; + for (let i = ancestors.length - 1; i >= 0; i--) { + if (isDeferredCallback(ancestors[i])) { + withinDeferredBoundary = true; + break; + } + } + // Still flag it even in deferred callbacks — execSync in async callbacks is still risky. + void withinDeferredBoundary; + + const argText = node.arguments.length > 0 ? sourceCode.getText(node.arguments[0]) : ""; + const stmt = findEnclosingStatement(sourceCode, node); + + context.report({ + node, + messageId: "requireTryCatch", + data: { arg: argText }, + suggest: stmt + ? [ + { + messageId: "wrapInTryCatch", + fix(fixer) { + const stmtText = sourceCode.getText(stmt); + const startLine = stmt.loc?.start.line; + const stmtLine = startLine !== undefined ? (sourceCode.lines[startLine - 1] ?? "") : ""; + const indent = stmtLine.match(/^(\s*)/)?.[1] ?? ""; + return fixer.replaceText( + stmt, + buildTryCatchSuggestion(stmtText, { + indent, + todoComment: "TODO: handle execSync failure (non-zero exit / signal termination).", + errorPrefix: "execSync failed: ", + }) + ); + }, + }, + ] + : [], + }); + }, + }; + }, +}); From ccbf9b80c03ba808f16f8d531f382b7512ac4471 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 20 Jul 2026 11:42:39 +0000 Subject: [PATCH 2/2] style: format code --- .../src/rules/require-execsync-try-catch.test.ts | 5 +---- .../src/rules/require-execsync-try-catch.ts | 12 ++---------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/eslint-factory/src/rules/require-execsync-try-catch.test.ts b/eslint-factory/src/rules/require-execsync-try-catch.test.ts index 7f04e31eb19..59ccb749403 100644 --- a/eslint-factory/src/rules/require-execsync-try-catch.test.ts +++ b/eslint-factory/src/rules/require-execsync-try-catch.test.ts @@ -37,10 +37,7 @@ describe("require-execsync-try-catch", () => { it("valid: execSync inside try block passes (ES module)", () => { esmRuleTester.run("require-execsync-try-catch", requireExecSyncTryCatchRule, { - valid: [ - `import { execSync } from "child_process"; try { execSync("ls"); } catch (e) {}`, - `import { execSync } from "node:child_process"; try { execSync("ls"); } catch (e) {}`, - ], + valid: [`import { execSync } from "child_process"; try { execSync("ls"); } catch (e) {}`, `import { execSync } from "node:child_process"; try { execSync("ls"); } catch (e) {}`], invalid: [], }); }); diff --git a/eslint-factory/src/rules/require-execsync-try-catch.ts b/eslint-factory/src/rules/require-execsync-try-catch.ts index 18e41d65794..092e642ae3e 100644 --- a/eslint-factory/src/rules/require-execsync-try-catch.ts +++ b/eslint-factory/src/rules/require-execsync-try-catch.ts @@ -112,13 +112,7 @@ function isExecSyncCall(node: TSESTree.CallExpression, sourceCode: TSESLint.Sour } // childProcess.execSync(...) or cp.execSync(...) - if ( - callee.type === AST_NODE_TYPES.MemberExpression && - !callee.computed && - callee.object.type === AST_NODE_TYPES.Identifier && - callee.property.type === AST_NODE_TYPES.Identifier && - callee.property.name === "execSync" - ) { + if (callee.type === AST_NODE_TYPES.MemberExpression && !callee.computed && callee.object.type === AST_NODE_TYPES.Identifier && callee.property.type === AST_NODE_TYPES.Identifier && callee.property.name === "execSync") { return isChildProcessObjectBinding(callee.object.name, callee.object, sourceCode); } @@ -138,9 +132,7 @@ export const requireExecSyncTryCatchRule = createRule({ }, schema: [], messages: { - requireTryCatch: - "Wrap execSync({{arg}}) in try/catch — execSync throws when the process exits non-zero or is killed by a signal, " + - "and will crash the action if the error is unhandled.", + requireTryCatch: "Wrap execSync({{arg}}) in try/catch — execSync throws when the process exits non-zero or is killed by a signal, " + "and will crash the action if the error is unhandled.", wrapInTryCatch: "Wrap in try { ... } catch { ... } and re-throw with { cause: err } to preserve context.", }, },