mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-07 04:12:04 +00:00
fix(lint): cover the unguarded egress paths and aliased fetch
Two spellings the egress guard could not see, both probed.
src/proxy.ts, src/instrumentation.ts and src/cli/ sat outside the enforced
roots (src/lib/ and src/app/), and the companion api-fetch-required guards
the client surface, so it did not reach them either. A raw fetch dropped
into instrumentation.ts linted clean. All three are egress-free today, but
instrumentation is exactly where an exporter pointed at an operator-supplied
URL would land.
The callee match knew the identifier `fetch` and the globalThis / window /
self member forms only, so an aliased import escaped it in every file:
`import { fetch as f } from "undici"` then `f(url)` linted clean under
src/lib. That spelling is what src/lib/safe-fetch.ts itself writes. Local
names bound to an imported `fetch`, and namespace-imported `ns.fetch(…)`,
are now callees; the wrapper internals and test files stay exempt as
before, and the wrapper's own aliased import is pinned as a valid case.
Adds the rule's missing unit-test file. The header records what the
binding scan still cannot see: require(), dynamic import, a fetch passed
as a parameter, and egress that never spells fetch at all.
This commit is contained in:
1 parent
1f5115fbf3
commit
6633f0fb19
2 files changed
+226
-23
No files matched your search
@@ -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" }],
|
||||
},
|
||||
],
|
||||
});
|
||||
@@ -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
|
||||
// `<namespace>.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) {
|
||||
|
||||
Reference in new issue
Block a user