feat(net): SSRF-safe fetch primitive + DNS-pinned rawMcpProbe#598
Merged
Conversation
Extract the SSRF-validation + DNS-pinning guard previously local to the storyboard `fetchProbe` into a reusable `@adcp/client/net` primitive, and rewire `rawMcpProbe` to dispatch through it so attacker-influenced agent URLs can't escape validation via naive global `fetch`. - `ssrfSafeFetch(url, options)` — buffered GET/POST with typed `SsrfRefusedError` codes. DNS resolved once, validated across the full address set, pinned into the undici `Agent.connect.lookup`, and torn down per-request so connection reuse can't carry a rebind. - Address classification via `net.BlockList` (canonicalizes IPv4-mapped IPv6 natively) plus defense-in-depth on wrapper prefixes: NAT64 (`64:ff9b::/96`) and 6to4 (`2002::/16`). Zone IDs (`%eth0`) and URL brackets (`[::1]`) are stripped before classification. - IMDS (`169.254.169.254`, `fe80::/10`) refused even under the `allowPrivateIp` dev opt-in. - Error messages carry hostname only; the resolved IP stays on `SsrfRefusedError.address` so counterparty-supplied URLs can't leak internal-network topology into compliance reports. - `rawMcpProbe` now accepts `allowPrivateIp` and refuses private / non-HTTPS targets by default; the storyboard runner threads `options.allow_http` through for dev loops. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #573. First of three PRs bundling the verifier follow-ups from the RFC 9421 review (#583 items 1+2, #584, #585 remain for PR B/C).
Summary
src/lib/testing/storyboard/probes.ts::fetchProbeinto a reusable@adcp/client/netprimitive. Future HTTPS fetchers (JWKS auto-refresh, revocation-list polling in signing: JWKS auto-refresh + revocation-list polling for long-running servers #584) consume the same guard.rawMcpProbe— previously using the node globalfetchwith no SSRF check — to dispatch through the primitive. Closes the TOCTOU gap where an attacker's authoritative nameserver could serve a public IP to a validator and a private IP to the actual connect.The primitive
ssrfSafeFetch(url, options)returns a buffered{ status, headers, body, pinnedAddress, pinnedFamily }or throws a typedSsrfRefusedErrorwith one of:invalid_url,scheme_not_allowed,non_https_without_opt_in,dns_lookup_failed,dns_empty,always_blocked_address,private_address,body_exceeds_limit.Guarantees (behavior preserved from existing
fetchProbe, now uniformly applied):https:only;http:underallowPrivateIpdev opt-in.file:/data:/ftp:always refused.undici.Agent.connect.lookup. Connect-time DNS is bypassed entirely — no rebind window.169.254.0.0/16+fe80::/10refused even underallowPrivateIp.redirect: 'manual'). 3xx responses are returned withLocationfor inspection; callers MUST re-invokessrfSafeFetchwith the new URL to re-validate — documented on the module contract.body_exceeds_limitif exceeded.Agentper call, closed infinally. Two concurrent calls with different pinned IPs don't interfere.Address classification hardening (from security review)
Addressed two bypasses the reviewer caught:
fe80::1%eth0now correctly flags as link-local. Previously a bracketed URL literalhttp://[fe80::1%eth0]/underallowPrivateIp: truecould reach IMDS.net.BlockListinstead of regex matching.0:0:0:0:0:ffff:127.0.0.1now classifies as loopback (previous regex required::ffff:prefix).64:ff9b::/96) and 6to4 (2002::/16) wrapper prefixes refused unconditionally whenallowPrivateIpis off — defense in depth against caller-edge translation into private targets.https://[::1]/now works correctly underallowPrivateIpand is rejected as loopback by default.Info-disclosure hygiene
SsrfRefusedError.messagecarries the counterparty-supplied hostname only; the resolved IP lives on.address. A hostname that resolves into the caller's internal network now won't leak topology into compliance reports / log aggregators.Changes in tree
src/lib/net/{address-guards,ssrf-fetch,index}.tssrc/lib/testing/storyboard/probes.ts(−160 lines;fetchProbeandrawMcpProbeare now thin wrappers over the primitive)src/lib/testing/storyboard/runner.ts(threadallow_httpthrough torawMcpProbe)test/lib/net-ssrf-fetch.test.js(26 tests)test/lib/storyboard-security.test.js(updated 3 localhost tests to opt intoallowPrivateIp; added 3 regression tests for rawMcpProbe should dispatch through DNS-pinned undici Agent (close TOCTOU gap) #573).changeset/ssrf-safe-fetch-primitive.md(patch bump —rawMcpProbeis not in the public barrel; the tightened default is the point of rawMcpProbe should dispatch through DNS-pinned undici Agent (close TOCTOU gap) #573)Test plan
npm run typecheckcleannpm run format:checkcleannpm run buildcleannode --test test/lib/net-ssrf-fetch.test.js— 26/26 pass (all new)node --test test/lib/storyboard-security.test.js— 68/68 passnpm test— 3671/3701 pass. The 12 failures are pre-existingGovernance E2Esuites broken by v3.0.0-rc.4: migrate to reallocation_threshold + human_review_required (breaking) #576'sreallocation_thresholdmigration (verified they fail identically on cleanmain: 7 pass / 20 fail ongovernance-e2e.test.jsalone).🤖 Generated with Claude Code