fix(security): judge outbound hosts by address, not by spelling - #10843
diegosouzapw merged 2 commits into
Conversation
isCloudMetadataHost() matched cloud-metadata endpoints only in their dotted-decimal spelling. new URL() serialises an IPv4-mapped IPv6 host as hextets, so http://[::ffff:169.254.169.254]/ arrives as ::ffff:a9fe:a9fe and the check never fires for an address the OS routes to 169.254.169.254. That block is the only control left in the "block-metadata" guard mode, which is the default for provider validation (areLocalProviderUrlsAllowed defaults on) and deliberately permits private/LAN targets. The same helper backs parseAndValidateWebhookUrl, assertSafeCatalogUrl, and the no-auth local embedding classifier, all of which document metadata as unconditionally blocked. Fold the embedded IPv4 out of a mapped literal before matching so the verdict follows the address. Also block "::" in isPrivateHost(): 0.0.0.0 was already refused, but its IPv6 twin reaches a service bound to the IPv6 loopback. The guard stays literal-only — hostnames that resolve to a metadata or private address, and redirects to one, are unchanged and out of scope here.
|
Build result, as promised in the description — it fails locally, but identically on a clean base, so it is my environment, not this change.
Same count, same message, same resolution chain ( Neither log mentions So I cannot give you a green build from here, and I would rather say that plainly than tick the box. The verification that is meaningful:
|
Validated on a 17-PR combined board: upstream-proxy-host-spelling 8/8 within the board's 287/287, typecheck:core clean. Routes src/lib/db/upstreamProxy.ts through the shared outbound-guard helpers instead of a private dotted-quad regex copy that had drifted since #10843 — closes the IPv4-mapped IPv6, ULA, link-local and CGNAT bypasses while preserving the deliberate loopback allow (CLIProxyAPI on localhost:8317). Multicast widened from /224\. to the full 224.0.0.0/4, called out explicitly. Thank you @ntdat812!
…osouzapw#10843) Obrigado — fix de segurança real (reportado via GHSA-qcfj-c39q-88jh): isCloudMetadataHost() decidia por spelling dotted-decimal, então um literal IPv4-mapped IPv6 (ex.: [::ffff:169.254.169.254]) alcançava o guard já canonicalizado por new URL() e não era reconhecido como endpoint de metadata de cloud — bypass no modo que permite endpoints privados/LAN (o default local-first). Também fecha o gap equivalente de 0.0.0.0/::. Validação (worktree combinado a partir de origin/release/v3.8.50, 0 conflitos): - typecheck:core limpo, complexity/cognitive-complexity dentro do baseline - tests/unit/outbound-guard-mapped-ipv4.test.ts — 12/12 passando (IMDS, Alibaba, ECS task role, ambas as grafias, hosts públicos, guard `::`) - Suítes SSRF relacionadas (webhook/firecrawl/kiro/provider-validation) — verdes
…ouzapw#11319) Validated on a 17-PR combined board: upstream-proxy-host-spelling 8/8 within the board's 287/287, typecheck:core clean. Routes src/lib/db/upstreamProxy.ts through the shared outbound-guard helpers instead of a private dotted-quad regex copy that had drifted since diegosouzapw#10843 — closes the IPv4-mapped IPv6, ULA, link-local and CGNAT bypasses while preserving the deliberate loopback allow (CLIProxyAPI on localhost:8317). Multicast widened from /224\. to the full 224.0.0.0/4, called out explicitly. Thank you @ntdat812!
Summary
isCloudMetadataHost()decides whether a host is a cloud-metadata endpoint by matching itsdotted-decimal spelling, so an IPv4-mapped IPv6 literal that routes to the same address is not
recognised.
http://[::ffff:169.254.169.254]/reaches the guard as::ffff:a9fe:a9feand passes acheck documented as absolute. This makes the verdict follow the address instead of its spelling.
new URL()canonicalises the embedded quad to hextets before any guard sees it:isPrivateHost()is unaffected — it refuses the whole::ffff:prefix. The exposure is in the twoguard modes that deliberately skip the private-host check and rely on the metadata block alone:
parseAndValidateNonMetadataUrl()— selected bygetProviderValidationGuard()wheneverareLocalProviderUrlsAllowed()is true, which is the local-first default. It intentionallypermits private/LAN provider endpoints, so the metadata block is the only control left.
parseAndValidateWebhookUrl()— documents metadata as blocked "even when the private opt-in isenabled".
assertSafeCatalogUrl()andisNoAuthLocalEmbeddingHost()read the same helper; in the latter amapped metadata host is classified as a no-auth local provider.
Fix:
mappedIpv4Host()folds the embedded IPv4 out of a::ffff:literal — accepting both thedotted form (raw hosts) and the hextet form (what
new URL()yields) — andisCloudMetadataHost()applies the existing IPv4 rules to it. Also refuses
::inisPrivateHost():0.0.0.0was alreadyrefused, but its IPv6 twin was not, and connecting to
[::]reaches a service bound to the IPv6loopback. No call sites change; both helpers keep their signatures.
Reported privately first via GitHub Security Advisories (
GHSA-qcfj-c39q-88jh); opening the fix as aPR at the maintainer's discretion — happy to move it back behind a private fork if preferred.
Related Issues
GHSA-qcfj-c39q-88jh(private advisory, this repo)Validation
npm run lintrelease/v3.8.50@bc6129bc); focused checks rerun afterwardCommands run locally (Windows, Node 22):
The one failure in
security/**+shared/**(error() with a destroyed stderr does not crash …)reproduces identically on a clean
release/v3.8.50checkout, as does theEBUSYteardown-hookfailure in
proxy-fallback-ssrf.test.ts(Windows file locking; its 4 assertions pass). Neither istouched by this change. I did not run the full
npm test.npm run buildexits 1 here, but identically on a cleanrelease/v3.8.50checkout — 2 xModule not found: Can't resolve 'better-sqlite3', anoptionalDependencywhose native binding doesnot compile on this Windows box. Neither log mentions the changed files. Details in the comment below.
Tests Added Or Updated
tests/unit/outbound-guard-mapped-ipv4.test.ts(new, 84 lines)It fails 10 of 12 on
release/v3.8.50and passes 12/12 with this change, so it pins thebehaviour rather than merely describing it. Covers: mapped metadata literals in both spellings
(IMDS, Alibaba, ECS task role), public hosts staying allowed,
parseAndValidateNonMetadataUrlrejecting mapped metadata while still permitting a private LAN provider endpoint, and
::.Coverage Notes
Touches one
src/file,src/shared/network/outboundUrlGuard.ts. Every branch added is exercised bythe new test: the dotted-quad path, the hextet path, the malformed-hextet rejection, the
non-
::ffff:early return, and the::literal. Existing coverage of this module(
webhook-ssrf-guard.test.ts, 29 cases) is unchanged and still passes, so coverage on the touchedfile moves up, not down.
Reviewer Notes
Quality Gates,DAST smoke (PR)and thesemgrepworkflow are all sitting ataction_requiredbecause this is my first PR from a fork;only the Semgrep app check ran on its own (it passed). Nothing is misconfigured —
quality.ymltargets
release/**and applies to this PR — the runs just need the approval click. Until thenthe local results above are the only evidence, which is why I listed the exact commands and the
pre-existing failures rather than a bare "tests pass".
private address (DNS rebinding) is still accepted, and redirects are not re-validated per hop.
Both are larger architectural changes and out of scope here.
them —
parseAndValidateNonMetadataUrl("http://192.168.1.50:11434/v1")still resolves (covered bya test). The only requests newly refused are IPv4-mapped metadata literals and
[::].outboundUrlGuard.tsis the CLI-loadable half of the guard (fix(providers): OpenCode Free models do not appear in Combo Builder on Windows #7682), so the patch adds no@/-aliased import — it uses onlynode:net, which the file already imported.