diff --git a/eslint-factory/README.md b/eslint-factory/README.md index a2c238e1097..adca5f865ca 100644 --- a/eslint-factory/README.md +++ b/eslint-factory/README.md @@ -26,6 +26,12 @@ Using an interpolated route bypasses Octokit's typed route dispatch, can silentl - `` github.request(`GET /repos/${owner}/${repo}`, ...) `` — template literal with interpolations. - `github.request("GET /repos/" + owner + "/" + repo, ...)` — string concatenation. +**Out of scope:** +- `this.github.request(...)` / `context.github.request(...)` — only direct identifier clients are matched today. +- `github.request(route, ...)` — variable indirection is not resolved. +- `github.request("GET /repos/".concat(owner), ...)` — `.concat()`-built routes are not inspected. +- `github.request("GET /repos" + "/{owner}/{repo}", ...)` — compile-time constant concatenations are accepted. + **Safe alternative:** ```js github.request("GET /repos/{owner}/{repo}", { owner, repo }); @@ -63,3 +69,97 @@ Flagged forms: - `global.isNaN(x)` / `global["isNaN"](x)` Locally shadowed bindings (e.g. `const isNaN = Number.isNaN`) are intentionally excluded. + +### `no-core-setoutput-non-string` + +Require `core.setOutput(name, value)` calls to pass an explicit string value for the targeted low-false-positive cases: numeric literals, boolean literals, `null`, `undefined`, and `.length` member accesses. + +Why: GitHub Actions step outputs are strings. Relying on implicit coercion can silently emit `"null"`, `"undefined"`, `"true"`, or other unintended values into downstream expressions. + +Typical fixes: +- `core.setOutput("count", String(count))` +- `core.setOutput("optional", "")` when empty-string semantics are intended for `null` / `undefined` + +### `no-unsafe-catch-error-property` + +Disallow direct access to `.message`, `.stack`, `.code`, `.status`, `.cause`, or `.name` on a `catch (err)` binding unless the code first proves the thrown value is safe to inspect. + +Accepted guards: +- `getErrorMessage(err)` +- `err instanceof Error` +- `typeof err === "object" && err !== null` + +Why: JavaScript can throw non-`Error` values, so `err.message` is not always safe. + +### `no-unsafe-promise-catch-error-property` + +Disallow the same unsafe error-property accesses inside inline promise rejection handlers such as `.catch(err => ...)`. + +This rule mirrors `no-unsafe-catch-error-property`, but for promise rejection values rather than `catch` clauses. Truthiness checks such as `err && err.message` are recognized for the accessed property. + +### `prefer-get-error-message` + +Prefer `getErrorMessage(err)` over the repeated pattern `err instanceof Error ? err.message : String(err)`. + +Why: `getErrorMessage(err)` centralizes safe error extraction and also sanitizes HTML error-page responses in the gh-aw runtime helpers. + +### `require-async-entrypoint-catch` + +Require bare calls to module-scope async entrypoints such as `main()` to be chained with `.catch(...)` when they are invoked outside an async context. + +Flagged form: +- `main();` + +Safe alternatives: +- `main().catch(err => { ... });` +- `await main();` when already inside an async function + +### `require-await-core-summary-write` + +Require `core.summary.write()` (including known aliases and fluent `core.summary.*().write()` chains) to be awaited when used as a bare expression. + +Why: `core.summary.write()` returns a promise. Dropping it can truncate or lose the step summary if the process exits first. + +Intentional exception: +- `void core.summary.write()` is treated as an explicit deliberate discard marker. + +### `require-error-cause-in-rethrow` + +Require rethrown `new Error(...)` values inside a `catch` block to preserve the original failure with `{ cause: err }` when the new message already references the caught error or a direct alias of it. + +Flagged form: +- `throw new Error(\`failed: ${getErrorMessage(err)}\`);` + +Safe alternative: +- `throw new Error(\`failed: ${getErrorMessage(err)}\`, { cause: err });` + +### `require-fs-sync-try-catch` + +Require `fs.readFileSync`, `fs.writeFileSync`, and `fs.appendFileSync` calls to be wrapped in `try/catch`. + +Why: these synchronous filesystem calls throw on missing files, permission errors, and disk failures, which otherwise crash the action without useful context. + +Current scope: +- direct `fs.readFileSync(...)` +- known `require("fs")` aliases +- destructured aliases such as `const { readFileSync } = require("fs")` + +### `require-json-parse-try-catch` + +Require `JSON.parse(...)` calls to be wrapped in `try/catch`. + +Why: malformed JSON should produce a controlled failure path in runtime scripts rather than an uncaught exception. + +Out of scope: +- aliased or destructured `JSON.parse` references such as `const parse = JSON.parse` + +### `require-parseInt-radix` + +Require `parseInt()` to include an explicit radix argument. + +Flagged forms: +- `parseInt(value)` +- `Number.parseInt(value)` +- `globalThis.parseInt(value)` + +Why: omitting the radix allows implicit base detection, which can silently accept prefixes such as `0x`. diff --git a/eslint-factory/src/rules/no-github-request-interpolated-route.test.ts b/eslint-factory/src/rules/no-github-request-interpolated-route.test.ts index 4b44a003296..90b2f46d7c9 100644 --- a/eslint-factory/src/rules/no-github-request-interpolated-route.test.ts +++ b/eslint-factory/src/rules/no-github-request-interpolated-route.test.ts @@ -62,6 +62,19 @@ describe("no-github-request-interpolated-route", () => { }); }); + it("valid: intentionally out-of-scope route forms are accepted", () => { + cjsRuleTester.run("no-github-request-interpolated-route", noGithubRequestInterpolatedRouteRule, { + valid: [ + "this.github.request(`GET /repos/${owner}/${repo}`, { owner, repo });", + "context.github.request(`GET /repos/${owner}/${repo}`, { owner, repo });", + "const route = `GET /repos/${owner}/${repo}`; github.request(route, { owner, repo });", + `github.request("GET /repos/".concat(owner, "/", repo), { owner, repo });`, + `github.request("GET /repos" + "/{owner}/{repo}", { owner, repo });`, + ], + invalid: [], + }); + }); + it("invalid: template literal with interpolations is flagged for all known client names", () => { cjsRuleTester.run("no-github-request-interpolated-route", noGithubRequestInterpolatedRouteRule, { valid: [], diff --git a/eslint-factory/src/rules/no-github-request-interpolated-route.ts b/eslint-factory/src/rules/no-github-request-interpolated-route.ts index 86f6fe9a6e8..ad9dc58ab10 100644 --- a/eslint-factory/src/rules/no-github-request-interpolated-route.ts +++ b/eslint-factory/src/rules/no-github-request-interpolated-route.ts @@ -13,15 +13,29 @@ function isInterpolatedTemplateLiteral(node: TSESTree.Node): boolean { return node.type === "TemplateLiteral" && node.expressions.length > 0; } +/** + * Returns true when the node is a compile-time constant route expression built + * from literals only. + */ +function isStaticRouteExpression(node: TSESTree.Node): boolean { + if (node.type === "Literal") return true; + if (node.type === "TemplateLiteral") return node.expressions.length === 0; + if (node.type === "BinaryExpression" && node.operator === "+") { + return isStaticRouteExpression(node.left) && isStaticRouteExpression(node.right); + } + return false; +} + /** * Returns true when the node is a binary `+` expression, which indicates * string concatenation. Nested concatenations such as * `"GET /repos/" + owner + "/" + repo` parse as left-associative * BinaryExpressions, so the outermost node is still a `+` BinaryExpression - * and is caught by this check. + * and is caught by this check. Compile-time constant concatenations built + * from literals only are intentionally excluded. */ function isStringConcatenation(node: TSESTree.Node): boolean { - return node.type === "BinaryExpression" && node.operator === "+"; + return node.type === "BinaryExpression" && node.operator === "+" && !isStaticRouteExpression(node); } export const noGithubRequestInterpolatedRouteRule = createRule({