diff --git a/eslint-plugins/healthlog/__tests__/safe-fetch-required.test.mjs b/eslint-plugins/healthlog/__tests__/safe-fetch-required.test.mjs new file mode 100644 index 000000000..6d54ae1cd --- /dev/null +++ b/eslint-plugins/healthlog/__tests__/safe-fetch-required.test.mjs @@ -0,0 +1,145 @@ +import { describe, it } from "vitest"; +import { RuleTester } from "eslint"; +import rule from "../safe-fetch-required.js"; + +RuleTester.describe = describe; +RuleTester.it = it; +RuleTester.itOnly = it.only; + +const ruleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2023, + sourceType: "module", + parserOptions: { ecmaFeatures: { jsx: true } }, + }, +}); + +ruleTester.run("safe-fetch-required", rule, { + valid: [ + { + code: "safeFetch(url, { requirePublicHost: true });", + filename: "/repo/src/lib/notifications/senders/ntfy.ts", + }, + // Same-origin relative paths never leave the origin. + { + code: 'fetch("/api/measurements");', + filename: "/repo/src/app/dashboard/load.ts", + }, + { + code: "fetch(`/api/measurements/${id}`);", + filename: "/repo/src/lib/queries/measurements.ts", + }, + // The wrapper's own internals, including the aliased undici import it + // actually uses. This is the spelling the rule learned to see, so the + // exemption has to be proven, not assumed. + { + code: 'import { fetch as undiciFetch } from "undici"; undiciFetch(url);', + filename: "/repo/src/lib/safe-fetch.ts", + }, + { + code: "fetch(url);", + filename: "/repo/src/lib/safe-fetch-dispatcher.ts", + }, + { + code: "fetch(url);", + filename: "/repo/src/lib/api/api-fetch.ts", + }, + // Tests mock and assert against fetch directly. + { + code: 'import { fetch as f } from "undici"; f(url);', + filename: "/repo/src/lib/__tests__/egress.test.ts", + }, + { + code: "fetch(url);", + filename: "/repo/src/lib/__mocks__/transport.ts", + }, + // Outside every enforced path: scripts and config are out of scope. + { + code: "fetch(url);", + filename: "/repo/scripts/backfill.ts", + }, + // An aliased import pointed at a relative path is still same-origin. + { + code: 'import { fetch as f } from "undici"; f("/api/version");', + filename: "/repo/src/lib/version.ts", + }, + // A named import that is not `fetch` binds nothing this rule cares about. + { + code: 'import { Agent } from "undici"; Agent(url);', + filename: "/repo/src/lib/safe-fetch-dispatcher-helper.ts", + }, + ], + invalid: [ + { + code: "fetch(url);", + filename: "/repo/src/lib/ai/openai-client.ts", + errors: [{ messageId: "rawFetch" }], + }, + { + code: "globalThis.fetch(url);", + filename: "/repo/src/app/api/withings/route.ts", + errors: [{ messageId: "rawFetch" }], + }, + { + code: "self.fetch(url);", + filename: "/repo/src/lib/pwa/register.ts", + errors: [{ messageId: "rawFetch" }], + }, + { + code: 'globalThis["fetch"](url);', + filename: "/repo/src/lib/ai/local-client.ts", + errors: [{ messageId: "rawFetch" }], + }, + // Protocol-relative and backslash forms resolve off-origin, so the + // same-origin exemption must not swallow them. + { + code: 'fetch("//evil.example/api");', + filename: "/repo/src/lib/ai/anthropic-client.ts", + errors: [{ messageId: "rawFetch" }], + }, + // The three paths that sat outside the enforced roots. They are + // egress-free today; instrumentation.ts is where an exporter pointed at + // an operator-supplied URL would land, and nothing would have said so. + { + code: "fetch(url);", + filename: "/repo/src/proxy.ts", + errors: [{ messageId: "rawFetch" }], + }, + { + code: "fetch(otlpEndpoint, { method: 'POST' });", + filename: "/repo/src/instrumentation.ts", + errors: [{ messageId: "rawFetch" }], + }, + { + code: "fetch(url);", + filename: "/repo/src/cli/mcp-stdio.ts", + errors: [{ messageId: "rawFetch" }], + }, + // The aliased import. `src/lib/safe-fetch.ts` writes exactly this + // spelling, so it is a form the codebase demonstrably reaches for, and + // the identifier check never matched it. + { + code: 'import { fetch as f } from "undici"; f(url);', + filename: "/repo/src/lib/ai/codex-client.ts", + errors: [{ messageId: "rawFetch" }], + }, + { + code: 'import { fetch as undiciFetch } from "undici"; undiciFetch(url);', + filename: "/repo/src/instrumentation.ts", + errors: [{ messageId: "rawFetch" }], + }, + // The namespace form of the same escape. + { + code: 'import * as undici from "undici"; undici.fetch(url);', + filename: "/repo/src/lib/notifications/senders/telegram.ts", + errors: [{ messageId: "rawFetch" }], + }, + // A default import named `fetch` was always caught; kept so the widening + // cannot regress it. + { + code: 'import fetch from "node-fetch"; fetch(url);', + filename: "/repo/src/lib/ai/local-client.ts", + errors: [{ messageId: "rawFetch" }], + }, + ], +}); diff --git a/eslint-plugins/healthlog/safe-fetch-required.js b/eslint-plugins/healthlog/safe-fetch-required.js index 4cff31d44..37fdd45c0 100644 --- a/eslint-plugins/healthlog/safe-fetch-required.js +++ b/eslint-plugins/healthlog/safe-fetch-required.js @@ -16,9 +16,22 @@ * mock or assert against `fetch` directly. * * The check is a syntactic `CallExpression` match against a bare - * `fetch(` callee (Identifier `fetch`) and `globalThis.fetch(` / - * `window.fetch(` member forms. `safeFetch` is a distinct identifier and - * never matches. + * `fetch(` callee (Identifier `fetch`), the `globalThis.fetch(` / + * `window.fetch(` / `self.fetch(` member forms, and — since 2026-09-03 — + * any local name bound to a module's `fetch` export or reached through a + * namespace import. `safeFetch` is a distinct identifier and never matches. + * + * The aliased spelling is not hypothetical: `src/lib/safe-fetch.ts` opens + * with `import { fetch as undiciFetch } from "undici"`, so the codebase + * demonstrably reaches for it, and every copy of that spelling outside the + * wrapper escaped the rule in every file. + * + * Limits, so a green run is not read as more than it is: the binding scan + * walks top-level `import` declarations only. A `require("undici").fetch`, + * a dynamic `await import(...)`, a fetch handed through a parameter or + * stored on an object, and a local shadow of an imported name are all + * outside it — as is any egress that never spells `fetch` at all, such as + * `http.request` or a client library with its own transport. * * Same-origin client fetches are exempt: a first argument that is a * string literal (or template head) starting with `/` is a relative, @@ -45,9 +58,27 @@ const EXEMPT_FILES = [ "src/lib/api/api-fetch.ts", ]; -// Only enforce inside the application source roots. Scripts, config, and -// generated code are out of scope. -const ENFORCED_ROOTS = ["src/lib/", "src/app/"]; +// Only enforce inside the application source. Scripts, config, and generated +// code are out of scope. +// +// The list carries three single files alongside the two directory roots. +// `src/proxy.ts`, `src/instrumentation.ts` and `src/cli/` sit outside +// `src/lib/` and `src/app/`, and the companion `healthlog/api-fetch-required` +// does not reach them either (it guards the client surface), so between them +// they were egress-unguarded. They are egress-free today; instrumentation is +// precisely where an exporter pointed at an operator-supplied URL would land, +// and nothing would have flagged it. +const ENFORCED_PATHS = [ + "src/lib/", + "src/app/", + "src/proxy.ts", + "src/instrumentation.ts", + "src/cli/", +]; + +// Objects that carry the platform `fetch`. Treated as namespaces below so the +// member form goes through one code path with the imported-namespace case. +const GLOBAL_FETCH_OBJECTS = ["globalThis", "window", "self"]; function toPosix(filename) { return filename.replace(/\\/g, "/"); @@ -64,7 +95,7 @@ function isTestFile(posix) { function isEnforced(filename) { const posix = toPosix(filename); - if (!ENFORCED_ROOTS.some((root) => posix.includes(root))) return false; + if (!ENFORCED_PATHS.some((path) => posix.includes(path))) return false; if (EXEMPT_FILES.some((f) => posix.includes(f))) return false; if (isTestFile(posix)) return false; return true; @@ -86,27 +117,54 @@ const safeFetchRequiredRule = { }, create(context) { const filename = context.filename ?? context.getFilename?.(); + const sourceCode = context.sourceCode ?? context.getSourceCode(); if (!filename || !isEnforced(filename)) { return {}; } - function isFetchCallee(callee) { - // Bare `fetch(...)`. - if (callee.type === "Identifier" && callee.name === "fetch") return true; - // `globalThis.fetch(...)` / `window.fetch(...)` / `self.fetch(...)`. - if ( - callee.type === "MemberExpression" && - !callee.computed && - callee.property.type === "Identifier" && - callee.property.name === "fetch" && - callee.object.type === "Identifier" && - (callee.object.name === "globalThis" || - callee.object.name === "window" || - callee.object.name === "self") - ) { - return true; + // Local names this module bound to somebody's `fetch`, collected from the + // top-level imports before any call site is visited. `import { fetch as f } + // from "undici"` makes `f(...)` a fetch call; `import * as undici from + // "undici"` makes `undici.fetch(...)` one. + const aliasedFetch = new Set(); + const namespaces = new Set([...GLOBAL_FETCH_OBJECTS]); + for (const node of sourceCode.ast.body) { + if (node.type !== "ImportDeclaration") continue; + for (const specifier of node.specifiers) { + if (specifier.type === "ImportSpecifier") { + const imported = + specifier.imported.type === "Identifier" + ? specifier.imported.name + : specifier.imported.value; + if (imported === "fetch") aliasedFetch.add(specifier.local.name); + } else if (specifier.type === "ImportNamespaceSpecifier") { + namespaces.add(specifier.local.name); + } } - return false; + } + + function isFetchCallee(callee) { + // Bare `fetch(...)`, and any local name bound to an imported `fetch`. + if (callee.type === "Identifier") { + return callee.name === "fetch" || aliasedFetch.has(callee.name); + } + if (callee.type !== "MemberExpression") return false; + // `globalThis.fetch(...)` / `window.fetch(...)` / `self.fetch(...)` and + // `.fetch(...)`, including the static computed spelling. + if ( + callee.object.type !== "Identifier" || + !namespaces.has(callee.object.name) + ) { + return false; + } + const property = !callee.computed + ? callee.property.type === "Identifier" + ? callee.property.name + : null + : callee.property.type === "Literal" + ? callee.property.value + : null; + return property === "fetch"; } function isSameOriginRelative(arg) {