diff --git a/eslint-factory/README.md b/eslint-factory/README.md index caea32e855b..ff8c044098b 100644 --- a/eslint-factory/README.md +++ b/eslint-factory/README.md @@ -50,6 +50,7 @@ This project hosts custom ESLint linters for `/actions/setup/js`. | [`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 | | [`require-mkdtempsync-try-catch`](#require-mkdtempsync-try-catch) | Require try/catch around `fs.mkdtempSync` calls | +| [`require-realpathsync-try-catch`](#require-realpathsync-try-catch) | Require try/catch around `fs.realpathSync` calls | | [`require-new-url-try-catch`](#require-new-url-try-catch) | Require try/catch around `new URL(variable)` calls | | [`require-parseInt-radix`](#require-parseInt-radix) | Require an explicit radix argument to `parseInt()` | | [`require-nan-check-after-env-numeric-parse`](#require-nan-check-after-env-numeric-parse) | Require NaN validation after parsing numeric values from `process.env` | @@ -515,6 +516,35 @@ try { } ``` +### `require-realpathsync-try-catch` + +Require `fs.realpathSync` calls to be wrapped in `try/catch`. + +Why: `realpathSync` throws synchronously when the target path is missing, permissions are denied, or a symlink cycle is encountered. Wrapping the call preserves call-site-specific error context and ensures path containment checks are not skipped on failure. + +**Detected forms:** +- `fs.realpathSync(path)` — direct call on a known `require("fs")` result. +- `fs["realpathSync"](path)` — computed string-literal property access. +- `const { realpathSync } = require("fs"); realpathSync(path)` — destructured binding from `require("fs")` or `require("node:fs")`. +- ESM namespace imports: `import * as fs from "fs"; fs.realpathSync(path)`. +- ESM named imports: `import { realpathSync } from "fs"; realpathSync(path)`. + +**Out of scope:** +- Objects whose `require` source is not the Node `fs` / `node:fs` module. +- Calls already inside a `try` block with a `catch` clause. +- `try { ... } finally { ... }` without a `catch` clause is still flagged. + +**Known limitation — no autofix for `VariableDeclaration`:** when the flagged call appears as a variable initializer, the rule reports the error but emits no autofix suggestion. Only `ExpressionStatement` and `ReturnStatement` positions receive an autofix suggestion. + +**Safe alternative:** +```js +try { + const resolved = fs.realpathSync(path); +} catch (err) { + throw new Error("fs.realpathSync failed: " + (err instanceof Error ? err.message : String(err)), { cause: err }); +} +``` + ### `require-new-url-try-catch` Require `new URL(variable)` calls to be wrapped in `try/catch`. diff --git a/eslint-factory/eslint.config.cjs b/eslint-factory/eslint.config.cjs index 2ed4c2897bc..9a22edc2ca3 100644 --- a/eslint-factory/eslint.config.cjs +++ b/eslint-factory/eslint.config.cjs @@ -32,6 +32,7 @@ module.exports = [ "gh-aw-custom/require-json-parse-try-catch": "warn", "gh-aw-custom/require-mkdirsync-try-catch": "warn", "gh-aw-custom/require-mkdtempsync-try-catch": "warn", + "gh-aw-custom/require-realpathsync-try-catch": "warn", "gh-aw-custom/require-rmsync-try-catch": "warn", "gh-aw-custom/require-parseInt-radix": "warn", "gh-aw-custom/require-return-after-core-setfailed": "warn", diff --git a/eslint-factory/src/index.ts b/eslint-factory/src/index.ts index 87cd5b199a5..6d1734d3c35 100644 --- a/eslint-factory/src/index.ts +++ b/eslint-factory/src/index.ts @@ -19,6 +19,7 @@ import { requireErrorCauseInRethrowRule } from "./rules/require-error-cause-in-r import { requireParseIntRadixRule } from "./rules/require-parseInt-radix"; import { requireMkdirSyncTryCatchRule } from "./rules/require-mkdirsync-try-catch"; import { requireMkdtempSyncTryCatchRule } from "./rules/require-mkdtempsync-try-catch"; +import { requireRealpathSyncTryCatchRule } from "./rules/require-realpathsync-try-catch"; import { requireRmSyncTryCatchRule } from "./rules/require-rmsync-try-catch"; import { requireReturnAfterCoreSetFailedRule } from "./rules/require-return-after-core-setfailed"; import { requireSpawnSyncErrorCheckRule } from "./rules/require-spawnsync-error-check"; @@ -84,6 +85,7 @@ const plugin = { "require-json-parse-try-catch": requireJsonParseTryCatchRule, "require-mkdirsync-try-catch": requireMkdirSyncTryCatchRule, "require-mkdtempsync-try-catch": requireMkdtempSyncTryCatchRule, + "require-realpathsync-try-catch": requireRealpathSyncTryCatchRule, "require-rmsync-try-catch": requireRmSyncTryCatchRule, "require-parseInt-radix": requireParseIntRadixRule, "require-return-after-core-setfailed": requireReturnAfterCoreSetFailedRule, diff --git a/eslint-factory/src/rules/require-realpathsync-try-catch.test.ts b/eslint-factory/src/rules/require-realpathsync-try-catch.test.ts new file mode 100644 index 00000000000..8cadb7eecc4 --- /dev/null +++ b/eslint-factory/src/rules/require-realpathsync-try-catch.test.ts @@ -0,0 +1,83 @@ +import { RuleTester } from "eslint"; +import { describe, it } from "vitest"; +import { requireRealpathSyncTryCatchRule } from "./require-realpathsync-try-catch"; + +const cjsRuleTester = new RuleTester({ languageOptions: { ecmaVersion: 2022, sourceType: "commonjs" } }); +const esmRuleTester = new RuleTester({ languageOptions: { ecmaVersion: 2022, sourceType: "module" } }); + +describe("require-realpathsync-try-catch", () => { + it("allows calls inside try/catch and ignores non-fs receivers", () => { + cjsRuleTester.run("require-realpathsync-try-catch", requireRealpathSyncTryCatchRule, { + valid: [ + `const fs = require("fs"); try { fs.realpathSync(path); } catch (e) {}`, + `const { realpathSync } = require("node:fs"); try { realpathSync(path); } catch (e) {}`, + `mockFs.realpathSync(path);`, + `const fs = require("mock-fs"); fs.realpathSync(path);`, + ], + invalid: [], + }); + }); + + it("flags CommonJS calls and offers an autofix", () => { + cjsRuleTester.run("require-realpathsync-try-catch", requireRealpathSyncTryCatchRule, { + valid: [], + invalid: [ + { + code: `fs.realpathSync(unresolved);`, + errors: [ + { + messageId: "requireTryCatch", + data: { arg: "unresolved" }, + suggestions: [ + { + messageId: "wrapInTryCatch", + output: `try {\n fs.realpathSync(unresolved);\n} catch (err) {\n // TODO: handle filesystem failure for this fs.realpathSync call.\n throw new Error(\n "fs.realpathSync failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n}`, + }, + ], + }, + ], + }, + { + code: `const fs = require("fs"); fs["realpathSync"](root);`, + errors: [{ messageId: "requireTryCatch", data: { arg: "root" }, suggestions: 1 }], + }, + { + code: `const { realpathSync } = require("fs"); realpathSync(directoryPath);`, + errors: [{ messageId: "requireTryCatch", data: { arg: "directoryPath" }, suggestions: 1 }], + }, + ], + }); + }); + + it("handles ESM namespace and named imports", () => { + esmRuleTester.run("require-realpathsync-try-catch", requireRealpathSyncTryCatchRule, { + valid: [`import * as fs from "fs"; try { fs.realpathSync(path); } catch (e) {}`], + invalid: [ + { + code: `import * as fs from "node:fs"; fs.realpathSync(path);`, + errors: [{ messageId: "requireTryCatch", data: { arg: "path" }, suggestions: 1 }], + }, + { + code: `import { realpathSync } from "fs"; realpathSync(path);`, + errors: [{ messageId: "requireTryCatch", data: { arg: "path" }, suggestions: 1 }], + }, + ], + }); + }); + + it("flags calls in async functions and try/finally without catch", () => { + cjsRuleTester.run("require-realpathsync-try-catch", requireRealpathSyncTryCatchRule, { + valid: [], + invalid: [ + { + code: `async function run() { fs.realpathSync(path); }`, + errors: [{ messageId: "requireTryCatch", suggestions: 1 }], + }, + { + code: `try { fs.realpathSync(path); } finally { cleanup(); }`, + errors: [{ messageId: "requireTryCatch", suggestions: 1 }], + }, + ], + }); + }); +}); diff --git a/eslint-factory/src/rules/require-realpathsync-try-catch.ts b/eslint-factory/src/rules/require-realpathsync-try-catch.ts new file mode 100644 index 00000000000..f05c1f59584 --- /dev/null +++ b/eslint-factory/src/rules/require-realpathsync-try-catch.ts @@ -0,0 +1,69 @@ +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}`); + +const FS_SYNC_METHODS = new Set(["realpathSync"]); + +export const requireRealpathSyncTryCatchRule = createRule({ + name: "require-realpathsync-try-catch", + meta: { + type: "problem", + hasSuggestions: true, + docs: { + description: + "Require fs.realpathSync calls in actions/setup/js scripts to be wrapped in try/catch. " + + "realpathSync throws synchronously when the target path is missing, permissions are denied, or a symlink cycle is encountered; " + + "without a call-site try/catch, the containment check is skipped and the original error loses useful call-site context.", + }, + schema: [], + messages: { + requireTryCatch: + "Wrap fs.realpathSync({{arg}}) in try/catch — realpathSync throws on missing paths, permission denied, or symlink cycles; without a call-site try/catch, you lose the original error context and get a generic engine-level stack instead of a specific message with `{ cause }`.", + wrapInTryCatch: "Wrap in try { ... } catch { ... } and re-throw with { cause: err } to preserve context.", + }, + }, + defaultOptions: [], + create(context) { + const sourceCode = context.sourceCode; + const resolveFsSyncMethod = createFsSyncMethodResolver(sourceCode, FS_SYNC_METHODS, { allowUnboundFsIdentifier: true }); + + return { + CallExpression(node) { + const methodName = resolveFsSyncMethod(node); + if (methodName !== "realpathSync") return; + if (isInsideTryBlock(sourceCode, node)) return; + + 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 filesystem failure for this fs.realpathSync call.", + errorPrefix: "fs.realpathSync failed: ", + }) + ); + }, + }, + ] + : [], + }); + }, + }; + }, +});