diff --git a/eslint-factory/README.md b/eslint-factory/README.md index 0bbf80b4f98..c60405a0832 100644 --- a/eslint-factory/README.md +++ b/eslint-factory/README.md @@ -31,6 +31,7 @@ This project hosts custom ESLint linters for `/actions/setup/js`. | [`require-async-entrypoint-catch`](#require-async-entrypoint-catch) | Require `.catch(...)` on bare async entrypoint calls | | [`require-await-core-summary-write`](#require-await-core-summary-write) | Require `await` on `core.summary.write()` calls | | [`require-error-cause-in-rethrow`](#require-error-cause-in-rethrow) | Require `{ cause: err }` when rethrowing inside a `catch` block | +| [`require-fs-io-try-catch`](#require-fs-io-try-catch) | Require try/catch around `fs.statSync`, `readdirSync`, `copyFileSync`, `unlinkSync`, and `renameSync` | | [`require-fs-sync-try-catch`](#require-fs-sync-try-catch) | Require try/catch around `fs.readFileSync`, `writeFileSync`, and `appendFileSync` | | [`require-json-parse-try-catch`](#require-json-parse-try-catch) | Require try/catch around `JSON.parse(...)` calls | | [`require-mkdirsync-try-catch`](#require-mkdirsync-try-catch) | Require try/catch around `fs.mkdirSync` calls | @@ -215,6 +216,33 @@ Flagged form: Safe alternative: - `throw new Error(\`failed: ${getErrorMessage(err)}\`, { cause: err });` +### `require-fs-io-try-catch` + +Require `fs.statSync`, `fs.readdirSync`, `fs.copyFileSync`, `fs.unlinkSync`, and `fs.renameSync` calls to be wrapped in `try/catch`. + +Why: these synchronous filesystem methods throw on missing files, permission errors (`EACCES`), busy resources (`EBUSY`), and other I/O failures. An unhandled throw crashes the action without surfacing a useful diagnostic message. + +**Detected forms:** +- `fs.statSync(path)` — direct call on a known `require("fs")` result. +- `fs["readdirSync"](dir)` — computed string-literal property access. +- `const { unlinkSync } = require("fs"); unlinkSync(path)` — destructured binding from `require("fs")` or `require("node:fs")`. +- ESM namespace imports: `import * as fs from "fs"; fs.copyFileSync(src, dest)`. +- ESM named imports: `import { renameSync } from "fs"; renameSync(src, dest)`. +- Bare unbound identifiers: `statSync(path)` when `statSync` is not a locally bound variable. + +**Out of scope:** +- Objects whose `require` source is not the Node `fs` / `node:fs` module. +- `try { ... } finally { ... }` without a `catch` clause is still flagged. + +**Safe alternative:** +```js +try { + fs.statSync(filePath); +} catch (err) { + throw new Error("fs.statSync failed: " + (err instanceof Error ? err.message : String(err)), { cause: err }); +} +``` + ### `require-fs-sync-try-catch` Require `fs.readFileSync`, `fs.writeFileSync`, and `fs.appendFileSync` calls to be wrapped in `try/catch`. @@ -261,7 +289,7 @@ Why: `mkdirSync` throws synchronously on permission errors, invalid paths, or un **Out of scope:** - Objects whose `require` source is not the Node `fs` / `node:fs` module (e.g. `mockFs.mkdirSync`, `storage.mkdirSync`, or `const fs = require("mock-fs"); fs.mkdirSync`). -- Other `fs` methods such as `existsSync`, `unlinkSync`, or `statSync` — use `require-fs-sync-try-catch` for `readFileSync`, `writeFileSync`, and `appendFileSync`. +- Other `fs` methods such as `existsSync` — use `require-fs-sync-try-catch` for `readFileSync`, `writeFileSync`, and `appendFileSync`; use `require-fs-io-try-catch` for `statSync`, `readdirSync`, `copyFileSync`, `unlinkSync`, and `renameSync`. - `try { ... } finally { ... }` without a `catch` clause is still flagged. **Safe alternative:** diff --git a/eslint-factory/eslint.config.cjs b/eslint-factory/eslint.config.cjs index 6ef400a276f..fd278b8b237 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", "gh-aw-custom/no-setfailed-then-exit-zero": "warn", }, }, diff --git a/eslint-factory/src/index.ts b/eslint-factory/src/index.ts index 37b7e7c5707..2e37862a201 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"; import { noSetFailedThenExitZeroRule } from "./rules/no-setfailed-then-exit-zero"; const plugin = { @@ -54,6 +55,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, "no-setfailed-then-exit-zero": noSetFailedThenExitZeroRule, }, }; 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..310f3e8dd0e --- /dev/null +++ b/eslint-factory/src/rules/require-fs-io-try-catch.test.ts @@ -0,0 +1,111 @@ +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: `, + }) + ); + }, + }, + ] + : [], + }); + }, + }; + }, +});