From 2edb7a1fdfe798c15c2876d02091e966cd93639e Mon Sep 17 00:00:00 2001 From: Praveen K Palaniswamy Date: Sat, 22 Aug 2026 19:38:44 -0400 Subject: [PATCH] fix(dashboard): keep the provider registry out of node:net (#11122) (#11154) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validated on a worktree over the current tip: the red it fixes reproduced exactly as described (media-page-client-browser-bundle red since #11122 — providerRegistry became reachable from the dashboard client bundle via node:net). Post-fix: bundle test 2/2 green, new ip-parity suite + is-local-provider 7/7, all 7 outboundUrlGuard consumer suites 76/76 (the moved normalizeHost/isPrivateHost keep their re-exports; routing behavior untouched). Thank you @yourspraveen — clean surgical extraction with a pure-JS ipVersion mirroring Node's own regexes. --- ...11154-provider-registry-node-net-bundle.md | 1 + open-sse/config/providerRegistry.ts | 5 +- src/shared/network/outboundUrlGuard.ts | 66 ++------- src/shared/network/privateHost.ts | 103 +++++++++++++ .../unit/private-host-ip-parity-11122.test.ts | 135 ++++++++++++++++++ 5 files changed, 252 insertions(+), 58 deletions(-) create mode 100644 changelog.d/fixes/11154-provider-registry-node-net-bundle.md create mode 100644 src/shared/network/privateHost.ts create mode 100644 tests/unit/private-host-ip-parity-11122.test.ts diff --git a/changelog.d/fixes/11154-provider-registry-node-net-bundle.md b/changelog.d/fixes/11154-provider-registry-node-net-bundle.md new file mode 100644 index 0000000000..b30d9e1392 --- /dev/null +++ b/changelog.d/fixes/11154-provider-registry-node-net-bundle.md @@ -0,0 +1 @@ +- fix(dashboard): keep `open-sse/config/providerRegistry.ts` free of `node:net` so the provider detail client bundle builds again — the host classification moved to a platform-free `src/shared/network/privateHost.ts` with a pure-JS `isIP` equivalent, leaving the #11122 routing behaviour unchanged (#11154) diff --git a/open-sse/config/providerRegistry.ts b/open-sse/config/providerRegistry.ts index 94bb036c95..69841c055c 100644 --- a/open-sse/config/providerRegistry.ts +++ b/open-sse/config/providerRegistry.ts @@ -10,7 +10,10 @@ export { } from "./providers/registry/alibaba/index.ts"; export { REGISTRY } from "./providers/index.ts"; import { REGISTRY } from "./providers/index.ts"; -import { isPrivateHost } from "@/shared/network/outboundUrlGuard"; +// Imported from `privateHost` rather than `outboundUrlGuard`: this module is reachable from +// `ProviderDetailPageClient.tsx`, so anything it pulls in has to survive a browser bundle +// (#11122). `privateHost` is platform-free by contract; the guard module is not. +import { isPrivateHost } from "@/shared/network/privateHost"; import { RegistryModel, REASONING_UNSUPPORTED, diff --git a/src/shared/network/outboundUrlGuard.ts b/src/shared/network/outboundUrlGuard.ts index e7175da0bc..45a7ebde7b 100644 --- a/src/shared/network/outboundUrlGuard.ts +++ b/src/shared/network/outboundUrlGuard.ts @@ -1,4 +1,10 @@ -import { isIP } from "node:net"; +import { ipVersion, isPrivateHost, normalizeHost } from "./privateHost"; + +// #11122: the host classification lives in `./privateHost.ts` because +// `open-sse/config/providerRegistry.ts` imports it from a module reachable by a browser +// bundle, and `node:net` cannot be resolved there. Re-exported so every existing caller of +// `isPrivateHost` from this module keeps working unchanged. +export { isPrivateHost }; export const PROVIDER_URL_BLOCKED_MESSAGE = "Blocked private or local provider URL"; export const CLOUD_METADATA_BLOCKED_MESSAGE = "Blocked cloud-metadata endpoint"; @@ -29,61 +35,6 @@ export class OutboundUrlGuardError extends Error { } } -function normalizeHost(hostname: string) { - const normalized = hostname.trim().toLowerCase(); - if (normalized.startsWith("[") && normalized.endsWith("]")) { - return normalized.slice(1, -1); - } - return normalized; -} - -export function isPrivateHost(hostname: string) { - const normalized = normalizeHost(hostname); - if (!normalized) return true; - - if ( - normalized === "localhost" || - normalized === "0.0.0.0" || - // `::` is the IPv6 twin of `0.0.0.0`: connecting to it reaches a service bound - // to the IPv6 loopback, so it has to be refused alongside its IPv4 spelling. - normalized === "::" || - normalized === "127.0.0.1" || - normalized === "::1" || - normalized.endsWith(".localhost") || - normalized.endsWith(".local") || - // `.internal` is reserved for private use (ICANN-style) and is the - // hostname suffix used by GCP/Azure metadata probes - // (e.g. `metadata.google.internal`). - normalized.endsWith(".internal") || - normalized.startsWith("::ffff:") - ) { - return true; - } - - if (isIP(normalized) === 4) { - const octets = normalized.split(".").map((segment) => parseInt(segment, 10)); - const [a, b] = octets; - - if (a === 0 || a === 10 || a === 127) return true; - if (a === 169 && b === 254) return true; - if (a === 192 && b === 168) return true; - if (a === 172 && b >= 16 && b <= 31) return true; - if (a === 100 && b >= 64 && b <= 127) return true; - return false; - } - - if (isIP(normalized) === 6) { - return ( - normalized === "::1" || - normalized.startsWith("fc") || - normalized.startsWith("fd") || - normalized.startsWith("fe80:") - ); - } - - return false; -} - // WHATWG URL serialises an IPv4-mapped IPv6 address as hextets, so // `http://[::ffff:169.254.169.254]/` reaches these helpers as `::ffff:a9fe:a9fe`. // Matching the dotted spelling alone therefore misses every mapped address that @@ -92,7 +43,7 @@ function mappedIpv4Host(hostname: string): string | null { const normalized = normalizeHost(hostname); if (!normalized.startsWith("::ffff:")) return null; const embedded = normalized.slice("::ffff:".length); - if (isIP(embedded) === 4) return embedded; + if (ipVersion(embedded) === 4) return embedded; const hextets = embedded.split(":"); if (hextets.length !== 2) return null; const [high, low] = hextets.map((part) => @@ -202,3 +153,4 @@ export function parseAndValidateNonMetadataUrl(input: string | URL) { // opencode.ts) where no `tsconfig.json` is present to resolve the `@/*` path alias. Keeping // this module free of ANY `@/`-aliased import is what makes it safe to load from the CLI. // Do not add a `@/`-aliased import here — see docs/security/… (packaging) and #7682. +// The same rule binds `./privateHost.ts`, which this module re-exports from. diff --git a/src/shared/network/privateHost.ts b/src/shared/network/privateHost.ts new file mode 100644 index 0000000000..f3120e3d1b --- /dev/null +++ b/src/shared/network/privateHost.ts @@ -0,0 +1,103 @@ +// Host classification shared by the outbound URL guard and the provider registry. +// +// #11122: `open-sse/config/providerRegistry.ts` needs `isPrivateHost`, and that module is +// reachable from `ProviderDetailPageClient.tsx`. `outboundUrlGuard.ts` reached for `node:net`'s +// `isIP`, so importing it from the registry broke the browser bundle with +// `Could not resolve "node:net"` (caught by tests/unit/media-page-client-browser-bundle.test.ts, +// which has been red on the release branch since #11122 merged). +// The classification therefore lives here, on a pure-JS `ipVersion`, with NO platform imports. +// +// Two constraints this module MUST keep — both enforced by tests: +// 1. No `node:*` import: it is bundled for the browser. +// 2. No `@/`-aliased import: `./outboundUrlGuard.ts` re-exports from here and is loaded by the +// packaged CLI (`omniroute setup-opencode`), where no tsconfig resolves the alias (#7682). + +// Vendored from Node's own `lib/internal/net.js` so `ipVersion` stays verdict-for-verdict +// identical to `isIP` — a NARROWER match would silently reclassify a private address as public +// and open the very egress the guard exists to close. `tests/unit/private-host-ip-parity-11122` +// asserts that parity against `node:net` directly. +const V4_SEG = "(?:25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)"; +const V4_STR = `(?:${V4_SEG}\\.){3}${V4_SEG}`; +const V6_SEG = "(?:[0-9a-fA-F]{1,4})"; + +const IPV4_RE = new RegExp(`^${V4_STR}$`); + +const IPV6_RE = new RegExp( + "^(?:" + + `(?:${V6_SEG}:){7}(?:${V6_SEG}|:)|` + + `(?:${V6_SEG}:){6}(?:${V4_STR}|:${V6_SEG}|:)|` + + `(?:${V6_SEG}:){5}(?::${V4_STR}|(?::${V6_SEG}){1,2}|:)|` + + `(?:${V6_SEG}:){4}(?:(?::${V6_SEG}){0,1}:${V4_STR}|(?::${V6_SEG}){1,3}|:)|` + + `(?:${V6_SEG}:){3}(?:(?::${V6_SEG}){0,2}:${V4_STR}|(?::${V6_SEG}){1,4}|:)|` + + `(?:${V6_SEG}:){2}(?:(?::${V6_SEG}){0,3}:${V4_STR}|(?::${V6_SEG}){1,5}|:)|` + + `(?:${V6_SEG}:){1}(?:(?::${V6_SEG}){0,4}:${V4_STR}|(?::${V6_SEG}){1,6}|:)|` + + `(?::(?:(?::${V6_SEG}){0,5}:${V4_STR}|(?::${V6_SEG}){1,7}|:))` + + ")(?:%[0-9a-zA-Z-.:]{1,64})?$" +); + +// Longest legal literal is 45 chars (`ffff:…:255.255.255.255`) plus a `%zone`. Every quantifier +// above is bounded, and this guard keeps the alternation from ever seeing a long hostile string +// (AGENTS.md → "Regex Security (ReDoS)"). +const MAX_IP_LITERAL_LENGTH = 110; + +/** Pure-JS `node:net#isIP`: 4, 6, or 0 when the string is not an IP literal. */ +export function ipVersion(host: string): 0 | 4 | 6 { + if (!host || host.length > MAX_IP_LITERAL_LENGTH) return 0; + if (IPV4_RE.test(host)) return 4; + return IPV6_RE.test(host) ? 6 : 0; +} + +export function normalizeHost(hostname: string) { + const normalized = hostname.trim().toLowerCase(); + if (normalized.startsWith("[") && normalized.endsWith("]")) { + return normalized.slice(1, -1); + } + return normalized; +} + +export function isPrivateHost(hostname: string) { + const normalized = normalizeHost(hostname); + if (!normalized) return true; + + if ( + normalized === "localhost" || + normalized === "0.0.0.0" || + // `::` is the IPv6 twin of `0.0.0.0`: connecting to it reaches a service bound + // to the IPv6 loopback, so it has to be refused alongside its IPv4 spelling. + normalized === "::" || + normalized === "127.0.0.1" || + normalized === "::1" || + normalized.endsWith(".localhost") || + normalized.endsWith(".local") || + // `.internal` is reserved for private use (ICANN-style) and is the + // hostname suffix used by GCP/Azure metadata probes + // (e.g. `metadata.google.internal`). + normalized.endsWith(".internal") || + normalized.startsWith("::ffff:") + ) { + return true; + } + + if (ipVersion(normalized) === 4) { + const octets = normalized.split(".").map((segment) => parseInt(segment, 10)); + const [a, b] = octets; + + if (a === 0 || a === 10 || a === 127) return true; + if (a === 169 && b === 254) return true; + if (a === 192 && b === 168) return true; + if (a === 172 && b >= 16 && b <= 31) return true; + if (a === 100 && b >= 64 && b <= 127) return true; + return false; + } + + if (ipVersion(normalized) === 6) { + return ( + normalized === "::1" || + normalized.startsWith("fc") || + normalized.startsWith("fd") || + normalized.startsWith("fe80:") + ); + } + + return false; +} diff --git a/tests/unit/private-host-ip-parity-11122.test.ts b/tests/unit/private-host-ip-parity-11122.test.ts new file mode 100644 index 0000000000..3fb4f10490 --- /dev/null +++ b/tests/unit/private-host-ip-parity-11122.test.ts @@ -0,0 +1,135 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { isIP } from "node:net"; +import { fileURLToPath } from "node:url"; + +import { build } from "esbuild"; + +import { ipVersion, isPrivateHost } from "../../src/shared/network/privateHost.ts"; + +// #11122 — that PR pointed `isLocalProvider()` at `isPrivateHost`, imported from +// `outboundUrlGuard.ts` (which imports `node:net`). `open-sse/config/providerRegistry.ts` is in +// the `ProviderDetailPageClient.tsx` graph, so the browser bundle broke and +// media-page-client-browser-bundle.test.ts went red on release/v3.8.50. `isPrivateHost` moved +// here to fix it; two things must hold for that move to be safe: +// 1. `ipVersion` agrees with `node:net#isIP` on every input — a NARROWER match would classify +// a private address as public and open the egress the guard exists to close. +// 2. The module stays bundleable for the browser (no `node:*`, no `@/` alias). + +const REPO_ROOT = fileURLToPath(new URL("../..", import.meta.url)); + +const LITERALS = [ + // IPv4 — valid + "0.0.0.0", + "127.0.0.1", + "10.0.0.1", + "100.64.0.1", + "169.254.169.254", + "172.16.0.1", + "172.31.255.254", + "192.168.1.50", + "8.8.8.8", + "255.255.255.255", + // IPv4 — invalid spellings node rejects + "010.1.1.1", + "1.2.3.4.5", + "1.2.3", + "256.1.1.1", + "1.2.3.-1", + "1.2.3.4 ", + " 1.2.3.4", + "1.2.3.04", + // IPv6 — valid + "::", + "::1", + "fd00::1", + "fe80::1", + "fc00::abcd", + "2001:db8::1", + "2001:0db8:0000:0000:0000:0000:0000:0001", + "::ffff:192.168.1.1", + "::ffff:a9fe:a9fe", + "64:ff9b::8.8.8.8", + "fe80::1%eth0", + "fe80::1%25", + // IPv6 — invalid + ":::", + "2001:db8::1::2", + "fe80::1%", + "gggg::1", + "2001:db8:::1", + // not IP literals at all + "", + "localhost", + "studio.local", + "api.openai.com", + "0x7f.1", + "2130706433", + "..", + "999", +]; + +test("ipVersion matches node:net#isIP across IP literals and near-misses", () => { + for (const host of LITERALS) { + assert.equal( + ipVersion(host), + isIP(host), + `ipVersion disagreed with isIP for ${JSON.stringify(host)}` + ); + } +}); + +test("ipVersion matches node:net#isIP across generated IPv4 permutations", () => { + const segments = ["0", "00", "01", "9", "10", "099", "127", "192", "255", "256", "300", ""]; + for (const a of segments) { + for (const b of segments) { + const host = `${a}.${b}.${a}.${b}`; + assert.equal(ipVersion(host), isIP(host), `ipVersion disagreed with isIP for ${host}`); + } + } +}); + +test("ipVersion matches node:net#isIP across generated IPv6 permutations", () => { + const groups = ["", "0", "1", "abcd", "ffff", "fffff", "xyz"]; + for (const g of groups) { + for (const host of [`${g}::1`, `::${g}`, `${g}:${g}::${g}`, `2001:db8::${g}`, `[${g}::1]`]) { + assert.equal(ipVersion(host), isIP(host), `ipVersion disagreed with isIP for ${host}`); + } + } +}); + +test("an over-long input is rejected rather than fed to the alternation", () => { + // The length guard is the ReDoS bound (AGENTS.md → "Regex Security"). Node agrees: no legal + // literal is this long, so the fast path costs no accuracy. + const long = `${"f".repeat(200)}::1`; + assert.equal(ipVersion(long), 0); + assert.equal(isIP(long), 0); +}); + +test("isPrivateHost keeps its verdicts after the move", () => { + for (const host of ["", "localhost", "127.0.0.1", "::1", "[::1]", "10.1.2.3", "192.168.0.15"]) { + assert.equal(isPrivateHost(host), true, `expected private: ${JSON.stringify(host)}`); + } + for (const host of ["api.openai.com", "8.8.8.8", "172.32.0.1", "2001:db8::1"]) { + assert.equal(isPrivateHost(host), false, `expected public: ${host}`); + } +}); + +test("privateHost stays browser-bundle safe", async () => { + // The direct guard for the regression: providerRegistry -> privateHost is in the + // ProviderDetailPageClient graph, so a `node:*` import here breaks the dashboard build. + await assert.doesNotReject( + build({ + absWorkingDir: REPO_ROOT, + entryPoints: [ + fileURLToPath(new URL("../../src/shared/network/privateHost.ts", import.meta.url)), + ], + bundle: true, + format: "esm", + logLevel: "silent", + platform: "browser", + tsconfig: "tsconfig.json", + write: false, + }) + ); +});