diff --git a/eslint-factory/README.md b/eslint-factory/README.md index 1fac0ddbdde..7eac2767f41 100644 --- a/eslint-factory/README.md +++ b/eslint-factory/README.md @@ -33,6 +33,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-fetch-timeout`](#require-fetch-timeout) | Require `fetch(...)` calls to include a non-nullish abort `signal` option | | [`require-fetch-try-catch`](#require-fetch-try-catch) | Require try/catch around awaited `fetch(...)` calls, including chained promise forms without rejection handlers | | [`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` | @@ -247,6 +248,27 @@ Why: `fetch` rejects with `TypeError` on network failures (DNS errors, connectio - locally shadowed `fetch` bindings such as `async function f(fetch) { await fetch(url); }` - named-reference rejection handlers are not inspected for correctness; the rule only checks that `.catch(handler)` or `.then(ok, onErr)` is present on the awaited fetch chain +### `require-fetch-timeout` + +Require `fetch(...)` calls to include a `signal` option so requests can be aborted instead of hanging indefinitely. + +Why: without an abort signal, a stalled network call can block the action until the workflow/job timeout ends it. + +**Flagged forms:** +- `fetch(url);` +- `fetch(url, null);` +- `fetch(url, undefined);` +- `fetch(url, { method: "GET" });` +- `fetch(url, { signal: null });` +- `globalThis.fetch(url, { method: "GET" });` + +**Not flagged:** +- `fetch(url, { signal: AbortSignal.timeout(10_000) });` +- `fetch(url, { signal: controller.signal });` +- `fetch(url, options);` (options object is not statically resolved) +- `fetch(url, { ...options });` (spread may already include `signal`) +- `obj.fetch(url);` (only global `fetch` calls are in scope) + ### `require-fs-io-try-catch` Require `fs.statSync`, `fs.readdirSync`, `fs.copyFileSync`, `fs.unlinkSync`, and `fs.renameSync` calls to be wrapped in `try/catch`. diff --git a/eslint-factory/eslint.config.cjs b/eslint-factory/eslint.config.cjs index ce7a3a2b1ff..65202db729b 100644 --- a/eslint-factory/eslint.config.cjs +++ b/eslint-factory/eslint.config.cjs @@ -47,6 +47,7 @@ module.exports = [ "gh-aw-custom/no-core-error-then-setfailed": "warn", "gh-aw-custom/no-duplicate-constant-values": "warn", "gh-aw-custom/require-escaped-regexp-interpolation": "warn", + "gh-aw-custom/require-fetch-timeout": "warn", }, }, { diff --git a/eslint-factory/src/index.ts b/eslint-factory/src/index.ts index a2b66e8094d..3c858f0c4f0 100644 --- a/eslint-factory/src/index.ts +++ b/eslint-factory/src/index.ts @@ -33,6 +33,7 @@ import { requireFetchTryCatchRule } from "./rules/require-fetch-try-catch"; import { noCoreErrorThenSetFailedRule } from "./rules/no-core-error-then-setfailed"; import { noDuplicateConstantValuesRule } from "./rules/no-duplicate-constant-values"; import { requireEscapedRegexpInterpolationRule } from "./rules/require-escaped-regexp-interpolation"; +import { requireFetchTimeoutRule } from "./rules/require-fetch-timeout"; const plugin = { meta: { @@ -75,6 +76,7 @@ const plugin = { "no-core-error-then-setfailed": noCoreErrorThenSetFailedRule, "no-duplicate-constant-values": noDuplicateConstantValuesRule, "require-escaped-regexp-interpolation": requireEscapedRegexpInterpolationRule, + "require-fetch-timeout": requireFetchTimeoutRule, }, }; diff --git a/eslint-factory/src/rules/require-fetch-timeout.test.ts b/eslint-factory/src/rules/require-fetch-timeout.test.ts new file mode 100644 index 00000000000..312d78e1d12 --- /dev/null +++ b/eslint-factory/src/rules/require-fetch-timeout.test.ts @@ -0,0 +1,99 @@ +import { RuleTester } from "eslint"; +import { describe, it } from "vitest"; +import { requireFetchTimeoutRule } from "./require-fetch-timeout"; + +const cjsRuleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: "commonjs", + }, +}); + +describe("require-fetch-timeout", () => { + it("valid: fetch with AbortSignal.timeout", () => { + cjsRuleTester.run("require-fetch-timeout", requireFetchTimeoutRule, { + valid: [ + `async function f() { const res = await fetch(url, { signal: AbortSignal.timeout(10000) }); }`, + `async function f() { const res = await fetch(url, { method: "POST", signal: ac.signal }); }`, + `async function f() { const res = await globalThis.fetch(url, { signal: AbortSignal.timeout(10000) }); }`, + ], + invalid: [], + }); + }); + + it("valid: fetch with spread options or identifier options object (unresolvable statically)", () => { + cjsRuleTester.run("require-fetch-timeout", requireFetchTimeoutRule, { + valid: [`async function f() { const res = await fetch(url, ...opts); }`, `async function f() { const res = await fetch(url, options); }`, `async function f() { const res = await fetch(url, { ...baseOptions }); }`], + invalid: [], + }); + }); + + it("valid: non-fetch calls are not flagged", () => { + cjsRuleTester.run("require-fetch-timeout", requireFetchTimeoutRule, { + valid: [ + `async function f() { const res = await axios.get(url); }`, + `function fetch2() { return 1; }`, + `obj.fetch(url);`, + `async function f(fetch) { return fetch(url); }`, + `async function f() { const fetch = (u) => Promise.resolve(u); return fetch(url); }`, + ], + invalid: [], + }); + }); + + it("invalid: fetch with no options argument", () => { + cjsRuleTester.run("require-fetch-timeout", requireFetchTimeoutRule, { + valid: [], + invalid: [ + { + code: `async function f() { const res = await fetch(url); }`, + errors: [{ messageId: "requireSignal" }], + }, + ], + }); + }); + + it("invalid: fetch with options object missing signal", () => { + cjsRuleTester.run("require-fetch-timeout", requireFetchTimeoutRule, { + valid: [], + invalid: [ + { + code: `async function f() { const res = await fetch(url, { method: "POST", headers: { "Content-Type": "application/json" } }); }`, + errors: [{ messageId: "requireSignal" }], + }, + { + code: `async function f() { const res = await fetch(url, { method: "GET" }); }`, + errors: [{ messageId: "requireSignal" }], + }, + { + code: `async function f() { const res = await globalThis.fetch(url, { method: "GET" }); }`, + errors: [{ messageId: "requireSignal" }], + }, + { + code: `async function f() { const res = await global["fetch"](url, { method: "GET" }); }`, + errors: [{ messageId: "requireSignal" }], + }, + { + code: `async function f() { const res = await fetch(url, { signal: null }); }`, + errors: [{ messageId: "requireSignal" }], + }, + { + code: `async function f() { const res = await fetch(url, { signal: undefined }); }`, + errors: [{ messageId: "requireSignal" }], + }, + { + code: `async function f() { const res = await fetch(url, { signal: void 0 }); }`, + errors: [{ messageId: "requireSignal" }], + }, + { + code: `async function f() { const res = await fetch(url, undefined); }`, + errors: [{ messageId: "requireSignal" }], + }, + { + code: `async function f() { const res = await fetch(url, null); }`, + errors: [{ messageId: "requireSignal" }], + }, + ], + }); + }); +}); diff --git a/eslint-factory/src/rules/require-fetch-timeout.ts b/eslint-factory/src/rules/require-fetch-timeout.ts new file mode 100644 index 00000000000..9e98aeec126 --- /dev/null +++ b/eslint-factory/src/rules/require-fetch-timeout.ts @@ -0,0 +1,100 @@ +import { AST_NODE_TYPES, ESLintUtils, TSESTree } from "@typescript-eslint/utils"; + +const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`); + +/** + * Returns true when the fetch call's options object argument (if any) carries + * a `signal` property, which is how callers wire in `AbortSignal.timeout(...)` + * or an `AbortController`-backed abort deadline. + */ +function isStaticallyNullish(node: TSESTree.Node): boolean { + if (node.type === AST_NODE_TYPES.Literal && node.value == null) return true; + if (node.type === AST_NODE_TYPES.Identifier && node.name === "undefined") return true; + if (node.type === AST_NODE_TYPES.UnaryExpression && node.operator === "void") return true; + return false; +} + +function hasSignalOption(callExpression: TSESTree.CallExpression): boolean { + const optionsArg = callExpression.arguments[1]; + if (!optionsArg) return false; + if (isStaticallyNullish(optionsArg)) return false; + + // Spread arguments (`fetch(url, ...opts)`) can't be statically inspected; + // assume the caller may have included a signal to avoid false positives. + if (optionsArg.type === AST_NODE_TYPES.SpreadElement) return true; + + if (optionsArg.type === AST_NODE_TYPES.ObjectExpression) { + for (const prop of optionsArg.properties) { + if (prop.type === AST_NODE_TYPES.SpreadElement) return true; + if (prop.type === AST_NODE_TYPES.Property) { + const isSignalProp = (!prop.computed && prop.key.type === AST_NODE_TYPES.Identifier && prop.key.name === "signal") || (!prop.computed && prop.key.type === AST_NODE_TYPES.Literal && prop.key.value === "signal"); + if (!isSignalProp) continue; + + if (!isStaticallyNullish(prop.value)) return true; + } + } + return false; + } + + // Options passed as an identifier/expression (e.g. a shared config object) + // can't be statically inspected; assume it may already carry a signal. + return true; +} + +export const requireFetchTimeoutRule = createRule({ + name: "require-fetch-timeout", + meta: { + type: "problem", + docs: { + description: "Require fetch() calls in actions/setup/js scripts to pass an abort signal so requests cannot hang indefinitely in CI", + }, + schema: [], + messages: { + requireSignal: "fetch() call has no `signal` option. Pass `signal: AbortSignal.timeout()` (or an AbortController-backed signal) so a stalled network request cannot hang the job indefinitely.", + }, + }, + defaultOptions: [], + create(context) { + const sourceCode = context.sourceCode; + type SourceCodeScope = ReturnType; + + function hasLocalBinding(node: TSESTree.Node, name: string): boolean { + let scope: SourceCodeScope | null = sourceCode.getScope(node); + while (scope) { + const variable = scope.set.get(name); + if (variable?.defs.length) return true; + scope = scope.upper; + } + return false; + } + + function isGlobalFetchCall(callee: TSESTree.Expression): callee is TSESTree.MemberExpression { + if (callee.type !== AST_NODE_TYPES.MemberExpression) return false; + + const propertyName = + !callee.computed && callee.property.type === AST_NODE_TYPES.Identifier + ? callee.property.name + : callee.computed && callee.property.type === AST_NODE_TYPES.Literal && typeof callee.property.value === "string" + ? callee.property.value + : null; + if (propertyName !== "fetch") return false; + + return callee.object.type === AST_NODE_TYPES.Identifier && (callee.object.name === "globalThis" || callee.object.name === "global"); + } + + return { + CallExpression(node: TSESTree.CallExpression) { + const callee = node.callee; + const isBareFetch = callee.type === AST_NODE_TYPES.Identifier && callee.name === "fetch"; + const isMemberFetch = isGlobalFetchCall(callee); + if (!isBareFetch && !isMemberFetch) return; + if (isBareFetch && hasLocalBinding(node, "fetch")) return; + if (isMemberFetch && callee.object.type === AST_NODE_TYPES.Identifier && hasLocalBinding(node, callee.object.name)) return; + + if (hasSignalOption(node)) return; + + context.report({ node, messageId: "requireSignal" }); + }, + }; + }, +});