From 86d99edf4fc1e13b0d772131ad7cf3e0473f8270 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 09:39:45 +0000 Subject: [PATCH 1/2] eslint: add require-fs-io-try-catch rule for statSync/readdirSync/copyFileSync/unlinkSync/renameSync Add a new custom ESLint rule that flags fs.statSync, fs.readdirSync, fs.copyFileSync, fs.unlinkSync, and fs.renameSync calls in actions/setup/js when they are not wrapped in try/catch. These methods are the next-highest-risk group of synchronous fs calls after readFileSync/writeFileSync/appendFileSync (already covered by require-fs-sync-try-catch). A scan of actions/setup/js found 33 unguarded call sites across files including artifact_client.cjs, check_workflow_timestamp.cjs, comment_memory_helpers.cjs, merge_remote_agent_github_folder.cjs, and send_otlp_span.cjs. All five methods throw synchronously on ENOENT, EACCES, EBUSY, etc. Without a try/catch, the error propagates as an unhandled exception that crashes the action step with no useful diagnostic message. The rule reuses the createFsSyncMethodResolver / isInsideTryBlock helpers from try-catch-rule-utils so resolver coverage (fs import, destructured bindings, computed member access) is consistent with the existing rule family. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- eslint-factory/eslint.config.cjs | 1 + eslint-factory/src/index.ts | 2 + .../src/rules/require-fs-io-try-catch.test.ts | 120 ++++++++++++++++++ .../src/rules/require-fs-io-try-catch.ts | 73 +++++++++++ 4 files changed, 196 insertions(+) create mode 100644 eslint-factory/src/rules/require-fs-io-try-catch.test.ts create mode 100644 eslint-factory/src/rules/require-fs-io-try-catch.ts diff --git a/eslint-factory/eslint.config.cjs b/eslint-factory/eslint.config.cjs index 6388892b007..28154ae8370 100644 --- a/eslint-factory/eslint.config.cjs +++ b/eslint-factory/eslint.config.cjs @@ -36,6 +36,7 @@ module.exports = [ "gh-aw-custom/no-core-error-then-process-exitcode": "warn", "gh-aw-custom/no-exec-interpolated-command": "warn", "gh-aw-custom/require-execsync-try-catch": "warn", + "gh-aw-custom/require-fs-io-try-catch": "warn", }, }, { diff --git a/eslint-factory/src/index.ts b/eslint-factory/src/index.ts index fb5f2b75361..3b02b81a04f 100644 --- a/eslint-factory/src/index.ts +++ b/eslint-factory/src/index.ts @@ -22,6 +22,7 @@ import { noCoreErrorThenProcessExitRule } from "./rules/no-core-error-then-proce import { noCoreErrorThenProcessExitCodeRule } from "./rules/no-core-error-then-process-exitcode"; import { noExecInterpolatedCommandRule } from "./rules/no-exec-interpolated-command"; import { requireExecSyncTryCatchRule } from "./rules/require-execsync-try-catch"; +import { requireFsIoTryCatchRule } from "./rules/require-fs-io-try-catch"; const plugin = { meta: { @@ -53,6 +54,7 @@ const plugin = { "no-core-error-then-process-exitcode": noCoreErrorThenProcessExitCodeRule, "no-exec-interpolated-command": noExecInterpolatedCommandRule, "require-execsync-try-catch": requireExecSyncTryCatchRule, + "require-fs-io-try-catch": requireFsIoTryCatchRule, }, }; diff --git a/eslint-factory/src/rules/require-fs-io-try-catch.test.ts b/eslint-factory/src/rules/require-fs-io-try-catch.test.ts new file mode 100644 index 00000000000..dc73d239a9b --- /dev/null +++ b/eslint-factory/src/rules/require-fs-io-try-catch.test.ts @@ -0,0 +1,120 @@ +import { RuleTester } from "eslint"; +import { describe, it } from "vitest"; +import { requireFsIoTryCatchRule } from "./require-fs-io-try-catch"; + +const cjsRuleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: "commonjs", + }, +}); + +describe("require-fs-io-try-catch", () => { + it("valid: fs.statSync inside try block passes", () => { + cjsRuleTester.run("require-fs-io-try-catch", requireFsIoTryCatchRule, { + valid: [ + `try { const s = fs.statSync(path); } catch (e) {}`, + `try { const entries = fs.readdirSync(dir); } catch (e) {}`, + `try { fs.copyFileSync(src, dest); } catch (e) {}`, + `try { fs.unlinkSync(path); } catch (e) {}`, + `try { fs.renameSync(oldPath, newPath); } catch (e) {}`, + ], + invalid: [], + }); + }); + + it("valid: other fs methods not in scope are ignored", () => { + cjsRuleTester.run("require-fs-io-try-catch", requireFsIoTryCatchRule, { + valid: [ + `fs.existsSync(path);`, + `fs.readFileSync(path, "utf8");`, + `fs.writeFileSync(path, data);`, + `mockFs.statSync(path);`, + `storage.readdirSync(dir);`, + ], + invalid: [], + }); + }); + + it("invalid: fs.statSync outside try/catch is flagged", () => { + cjsRuleTester.run("require-fs-io-try-catch", requireFsIoTryCatchRule, { + valid: [], + invalid: [ + { + code: `fs.statSync(path);`, + errors: [{ messageId: "requireTryCatch", data: { method: "statSync", arg: "path" } }], + }, + ], + }); + }); + + it("invalid: fs.readdirSync outside try/catch is flagged", () => { + cjsRuleTester.run("require-fs-io-try-catch", requireFsIoTryCatchRule, { + valid: [], + invalid: [ + { + code: `const entries = fs.readdirSync(dir);`, + errors: [{ messageId: "requireTryCatch", data: { method: "readdirSync", arg: "dir" } }], + }, + ], + }); + }); + + it("invalid: fs.copyFileSync outside try/catch is flagged", () => { + cjsRuleTester.run("require-fs-io-try-catch", requireFsIoTryCatchRule, { + valid: [], + invalid: [ + { + code: `fs.copyFileSync(src, dest);`, + errors: [{ messageId: "requireTryCatch", data: { method: "copyFileSync", arg: "src" } }], + }, + ], + }); + }); + + it("invalid: fs.unlinkSync outside try/catch is flagged", () => { + cjsRuleTester.run("require-fs-io-try-catch", requireFsIoTryCatchRule, { + valid: [], + invalid: [ + { + code: `fs.unlinkSync(outputFile);`, + errors: [{ messageId: "requireTryCatch", data: { method: "unlinkSync", arg: "outputFile" } }], + }, + ], + }); + }); + + it("invalid: fs.renameSync outside try/catch is flagged", () => { + cjsRuleTester.run("require-fs-io-try-catch", requireFsIoTryCatchRule, { + valid: [], + invalid: [ + { + code: `fs.renameSync(tmpPath, finalPath);`, + errors: [{ messageId: "requireTryCatch", data: { method: "renameSync", arg: "tmpPath" } }], + }, + ], + }); + }); + + it("valid: node:fs destructured inside try block passes", () => { + cjsRuleTester.run("require-fs-io-try-catch", requireFsIoTryCatchRule, { + valid: [ + `const { statSync } = require("node:fs"); try { statSync(path); } catch (e) {}`, + `const { readdirSync } = require("fs"); try { readdirSync(dir); } catch (e) {}`, + ], + invalid: [], + }); + }); + + it("invalid: destructured fs methods outside try/catch are flagged", () => { + cjsRuleTester.run("require-fs-io-try-catch", requireFsIoTryCatchRule, { + valid: [], + invalid: [ + { + code: `const { statSync } = require("node:fs"); statSync(path);`, + errors: [{ messageId: "requireTryCatch" }], + }, + ], + }); + }); +}); diff --git a/eslint-factory/src/rules/require-fs-io-try-catch.ts b/eslint-factory/src/rules/require-fs-io-try-catch.ts new file mode 100644 index 00000000000..a5a5591585b --- /dev/null +++ b/eslint-factory/src/rules/require-fs-io-try-catch.ts @@ -0,0 +1,73 @@ +import { ESLintUtils } from "@typescript-eslint/utils"; +import { buildTryCatchSuggestion, createFsSyncMethodResolver, findEnclosingStatement, isInsideTryBlock } from "./try-catch-rule-utils"; + +const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`); + +// fs methods beyond readFileSync/writeFileSync/appendFileSync that throw on I/O failure +// and appear frequently unguarded in actions/setup/js. +const FS_IO_METHODS = new Set(["statSync", "readdirSync", "copyFileSync", "unlinkSync", "renameSync"]); + +export const requireFsIoTryCatchRule = createRule({ + name: "require-fs-io-try-catch", + meta: { + type: "problem", + hasSuggestions: true, + docs: { + description: + "Require fs.statSync, fs.readdirSync, fs.copyFileSync, fs.unlinkSync, and fs.renameSync calls in actions/setup/js scripts to be wrapped in try/catch. " + + "These methods throw synchronously on missing files, permission errors, and other I/O failures; " + + "an unhandled throw crashes the action without surfacing a useful error message.", + }, + schema: [], + messages: { + requireTryCatch: "Wrap fs.{{method}}({{arg}}) in try/catch — synchronous fs methods throw on I/O errors " + "(missing file, permission denied, disk full) and will crash the action if unhandled.", + wrapInTryCatch: "Wrap in try { ... } catch { ... } and re-throw with { cause: err } to preserve context.", + }, + }, + defaultOptions: [], + create(context) { + const sourceCode = context.sourceCode; + const resolveFsIoMethod = createFsSyncMethodResolver(sourceCode, FS_IO_METHODS, { allowUnboundFsIdentifier: true }); + + return { + CallExpression(node) { + const methodName = resolveFsIoMethod(node); + + if (!methodName) return; + + if (isInsideTryBlock(sourceCode, node)) return; + + const argText = node.arguments.length > 0 ? sourceCode.getText(node.arguments[0]) : ""; + const method = methodName; + const stmt = findEnclosingStatement(sourceCode, node); + + context.report({ + node, + messageId: "requireTryCatch", + data: { method, 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 I/O failure for this fs.${method} call.`, + errorPrefix: `fs.${method} failed: `, + }) + ); + }, + }, + ] + : [], + }); + }, + }; + }, +}); From 4d109064d900c71bd6fc3a90ef7a5cdf8d9b470f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:10:11 +0000 Subject: [PATCH 2/2] docs: add require-fs-io-try-catch README section; merge main Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../src/rules/require-fs-io-try-catch.test.ts | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/eslint-factory/src/rules/require-fs-io-try-catch.test.ts b/eslint-factory/src/rules/require-fs-io-try-catch.test.ts index dc73d239a9b..310f3e8dd0e 100644 --- a/eslint-factory/src/rules/require-fs-io-try-catch.test.ts +++ b/eslint-factory/src/rules/require-fs-io-try-catch.test.ts @@ -25,13 +25,7 @@ describe("require-fs-io-try-catch", () => { it("valid: other fs methods not in scope are ignored", () => { cjsRuleTester.run("require-fs-io-try-catch", requireFsIoTryCatchRule, { - valid: [ - `fs.existsSync(path);`, - `fs.readFileSync(path, "utf8");`, - `fs.writeFileSync(path, data);`, - `mockFs.statSync(path);`, - `storage.readdirSync(dir);`, - ], + valid: [`fs.existsSync(path);`, `fs.readFileSync(path, "utf8");`, `fs.writeFileSync(path, data);`, `mockFs.statSync(path);`, `storage.readdirSync(dir);`], invalid: [], }); }); @@ -98,10 +92,7 @@ describe("require-fs-io-try-catch", () => { it("valid: node:fs destructured inside try block passes", () => { cjsRuleTester.run("require-fs-io-try-catch", requireFsIoTryCatchRule, { - valid: [ - `const { statSync } = require("node:fs"); try { statSync(path); } catch (e) {}`, - `const { readdirSync } = require("fs"); try { readdirSync(dir); } catch (e) {}`, - ], + valid: [`const { statSync } = require("node:fs"); try { statSync(path); } catch (e) {}`, `const { readdirSync } = require("fs"); try { readdirSync(dir); } catch (e) {}`], invalid: [], }); });