Skip to content

fix(dashboard): keep the provider registry out of node:net (#11122) - #11154

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.50from
yourspraveen:fix/providerregistry-node-net-bundle
Aug 22, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.50from
yourspraveen:fix/providerregistry-node-net-bundle

Conversation

@yourspraveen

Copy link
Copy Markdown
Contributor

Problem

#11122 pointed isLocalProvider() at isPrivateHost(), imported from src/shared/network/outboundUrlGuard.ts. That module's first line is:

import { isIP } from "node:net";

open-sse/config/providerRegistry.ts is reachable from ProviderDetailPageClient.tsx, so the dashboard's client bundle can no longer resolve node:net. tests/unit/media-page-client-browser-bundle.test.ts has been red on release/v3.8.50 since that merge — reproduced on the current tip (367ae2fb9):

✖ provider detail client entry stays browser-bundle safe
  Build failed with 1 error:
  src/shared/network/outboundUrlGuard.ts:1:21: ERROR: Could not resolve "node:net"

Fix

The routing behaviour from #11122 is correct and is left exactly as merged — isLocalProvider() is untouched. Only the import path moves.

  • normalizeHost() / isPrivateHost() move into a new, platform-free src/shared/network/privateHost.ts.
  • isIP is swapped for ipVersion(), a pure-JS equivalent built from the regexes Node itself uses in lib/internal/net.js.
  • outboundUrlGuard.ts re-exports isPrivateHost, so its ~10 existing callers are unchanged.
  • providerRegistry imports the narrow module directly, so a future node: import in the guard cannot re-break the bundle.

The diff to providerRegistry.ts is one import line.

Why the parity test matters

Replacing isIP is security-critical: a narrower matcher would classify a private address as public and open the very egress the SSRF guard exists to close. tests/unit/private-host-ip-parity-11122.test.ts therefore asserts ipVersion against node:net#isIP verdict-for-verdict, not just spot behaviour:

  • valid IPv4/IPv6 literals, IPv4-mapped forms, zone ids (fe80::1%eth0)
  • near-misses that matter — leading zeros (010.1.1.1), out-of-range octets, 2001:db8::1::2
  • generated permutations across both families
  • an over-long input, plus the length bound that keeps the alternation ReDoS-safe (AGENTS.md → Regex Security)
  • a re-bundle of privateHost.ts for the browser, pinning the invariant at the module

privateHost.ts also inherits the #7682 rule from its parent — no @/-aliased import — so the packaged CLI keeps loading outboundUrlGuard without a tsconfig.

Verification

Check Result
media-page-client-browser-bundle.test.ts red on 367ae2fb9 → green
is-local-provider-11091.test.ts (from #11122) still green
Guard / SSRF / CLI-alias / consumer suites (16 files) 253 pass, 0 fail
typecheck:core, eslint, check:cycles clean

⚠️ base-red inherited: #9985

@yourspraveen

Copy link
Copy Markdown
Contributor Author

CI triage: all 5 red checks are inherited from the base — none originate here

Verified against a clean worktree at the base this PR was cut from (367ae2fb9) and cross-checked against sibling PR #11146, which targets the same branch with an unrelated diff.

Check Failure Verdict
Docs Gates (fast-path) 3 stale 157 migrations claims vs 158 in code inherited — #11103
Fast Quality Gates test-discovery, check:dashboard-typecheck inherited — also red on #11146
Unit Tests 1/4 – 4/4 17 files inherited — 16 reproduce at 367ae2fb9, 17th confirmed on #11146

Docs Gates

✗ README.md — stale migrations: "157 migrations" — code has 158
✗ AGENTS.md — stale migrations: "157 migrations" — code has 158
✗ llm.txt   — stale migrations: "157 migrations" — code has 158

84c9dfdd2 (#11103, config audit log) added src/lib/db/migrations/161_config_audit_log.sql without updating the doc counts. Base has 158 migrations and so does this branch — the diff here is 5 files and adds none. #11146 passed this gate only because its run predates #11103 landing.

Fast Quality Gates

✗ [órfão NOVO] tests/unit/providers/uncloseai-noauth.test.ts     → test-discovery
✗ src/app/(dashboard)/dashboard/combos/page.tsx TS2554 (baseline 0, live 188)
✗ .../providers/[id]/components/HarImportButton.tsx TS2339        → dashboard-typecheck

All three are present on #11146 as well; HarImportButton traces to the HAR-import work in 6cd4d38e2. Every gate that could plausibly have been affected by adding a module passes locally on this branch: file-size, duplication, dead-code, type-coverage, cycles, build-scope, known-symbols, mutation-test-coverage.

Unit shards

All 17 CI-failing files were re-run at 367ae2fb9; 16 fail identically there. The 17th, tests/unit/systemd-notify.test.mjs, is skipped on macOS (systemd-notify and/or python3 unavailable), so it was confirmed against #11146's shard logs instead, where it fails too.

Two files that failed in a local full-suite run but not in CI were also cleared: call-log-artifact-worker.test.ts passes in isolation (an artifact of --test-concurrency=20), and 8510-adobe-firefly-edits-route.test.ts fails identically at base when run in isolation.

What this PR does to the suite

From the shard logs on this PR's own run:

✔ provider detail client entry stays browser-bundle safe (933ms)   ← red on the base
✔ media page client entry stays browser-bundle safe (210ms)
✔ privateHost stays browser-bundle safe (29ms)                     ← new guard

Net effect is one fewer failure than the base, and no new ones. Vitest, No new ESLint warnings, Build, semgrep, dast-smoke and Merge integrity are green.

Worth a separate look

Two of the above look like fresh base-reds not captured in the (empty) failure block of #9985: the migration-count drift from #11103, and the uncloseai-noauth.test.ts orphan. Both need fixing on the release branch rather than in a feature PR.

Note that release/v3.8.50 has advanced past 367ae2fb9 since this run, so a re-run may show a different inherited set.

@diegosouzapw
diegosouzapw force-pushed the fix/providerregistry-node-net-bundle branch from a90d003 to 3d4546e Compare August 22, 2026 23:38
@diegosouzapw
diegosouzapw merged commit 2edb7a1 into diegosouzapw:release/v3.8.50 Aug 22, 2026
4 of 7 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…apw#11122) (diegosouzapw#11154)

Validated on a worktree over the current tip: the red it fixes reproduced exactly as described (media-page-client-browser-bundle red since diegosouzapw#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.
@yourspraveen
yourspraveen deleted the fix/providerregistry-node-net-bundle branch October 3, 2026 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants