security(discovery): wire network-consistency-checker + property-crawler through ssrfSafeFetch (#1627 cross-cutting follow-up)

[bokelley]

Summary

Surfaced from the security review of #1632 (the TOCTOU rebind close for detectProtocol). Two more buyer-side fetch sites still use raw fetch() against agent-supplied URLs — they have the same TOCTOU rebind window that #1627 just closed for detectProtocol.

Affected sites

High priority — same TOCTOU class as #1618/#1627

src/lib/discovery/network-consistency-checker.ts:375,397,569,590 — uses fetch(agent.url, ...) with a validateAgentUrl literal-hostname gate. A hostname that passes the literal validation but resolves to 169.254.169.254 reaches IMDS. Worse: it has its own redirect: 'manual' + validateAgentUrl(redirectUrl) path followed by a second native fetch — a hostname rebind between the two calls slips through.

src/lib/discovery/property-crawler.ts:259 — uses fetch(url) to crawl adcp+ properties from agent-supplied data. No SSRF gate visible in the surrounding context.

Medium — depends on call-site trust

src/lib/auth/index.ts:60 — transport wrapper used by adapters. If the URL it's called with comes from agent metadata (not operator config), it inherits the gap.

Already safe

• src/lib/signing/jwks-https.ts, src/lib/signing/brand-jwks.ts, src/lib/signing/agent-resolver/fetch-helpers.ts — already route through ssrfSafeFetch.
• src/lib/auth-introspection.ts, webhook emitters — operator-configured URLs (lower risk).

Proposed approach

Same shape as #1632:

1. Replace native fetch with ssrfSafeFetch at each affected call site.
2. Pass allowPrivateIp: isInternalProbesAllowed() so the env opt-in flows through.
3. Adopt the same dns_lookup_failed / dns_empty / body_exceeds_limit → suspect carve-out that detectProtocol uses (consider exporting a shared helper — see "centralized constant" suggestion below).
4. For the network-consistency-checker.ts redirect-follow path: replace the manual two-call dance with a single ssrfSafeFetch invocation if cards are small enough; if not, validate each redirect target as a fresh ssrfSafeFetch call (which DNS-pins each hop).

Companion suggestion: centralize the carve-out

Security-reviewer of #1632 suggested:

Consider exporting SsrfRefusedCode as a centralized constant of "must propagate" vs "treat as transient" so the next caller wiring up ssrfSafeFetch doesn't reinvent the carve-out and accidentally re-introduce the catch-swallow class.

Concrete shape: export SSRF_TRANSIENT_CODES: ReadonlySet<SsrfRefusedCode> from src/lib/net/ssrf-fetch.ts. Callers do if (err instanceof SsrfRefusedError && SSRF_TRANSIENT_CODES.has(err.code)) { suspect = true; continue; } throw err;.

Tracking

Filed from the security-reviewer's #1632 review. Defense-in-depth — not a regression caused by #1627. Worth landing before any server-side comply runner ships.

[bokelley]

Triage

Classification: Bug (security — SSRF/TOCTOU)
Bucket(s): library
Status: ready-for-human

What the experts said:

• security-reviewer: All three raw fetch sites are ship-blockers — property-crawler.ts:259 is highest risk (no SSRF gate, fully agent-controlled URL with 5-hop recursive redirect allowed); network-consistency-checker.ts TOCTOU rebind window is real and worsens under the 50-parallel-fetch concurrency cap; SSRF_TRANSIENT_CODES must be scoped to exactly {dns_lookup_failed, dns_empty} — including any policy code creates a retry loop that turns one SSRF probe into a sustained scan.
• ad-tech-protocol-expert: Fix is architecturally consistent with fix(security): detectProtocol through ssrfSafeFetch for DNS-pin / TOCTOU defense (#1627) #1632 — both probe sites are confirmed hot-path per AdCP discovery spec; validateAgentUrl can stay as a cheap pre-flight parse check but must not be the sole SSRF gate; audit createAuthenticatedFetch callers in src/lib/auth/index.ts before closing.

My take: Flagging for @bokelley review before a PR opens — security-sensitive by rule, and three pre-PR items need human sign-off: (1) http:// behavioral break requires an explicit allowHttp? config flag (same precedent as LOOPBACK_OK_WEBHOOK_SSRF_POLICY) rather than a silent refusal that breaks local test agents, (2) SSRF_TRANSIENT_CODES must exclude all permanent policy codes, (3) sanitizeError must not expose .address/.hostname from SsrfRefusedError through log paths after migration. Fix direction is clear once those three are resolved.

Note: no security label exists in this repo — gap flagged in run summary.

---

Triaged by Claude Code. Session: https://claude.ai/code/session_01PkpZRgrFgXywKj5weoU4zx

---

Generated by Claude Code