security: block cloud-metadata SSRF targets in snapshot navigation + discovery (PER-8614)

[Shivanshu-07]

Summary

Snapshot URLs and discovered subresource requests were navigated/fetched by Chromium without validating the target host. A malicious or compromised snapshot URL (or a subresource it references) could point Chromium at a cloud instance-metadata endpoint and exfiltrate short-lived cloud credentials — an SSRF (F-016 / PER-8614).

This PR adds a surgical block of cloud-metadata endpoints only. It deliberately does not block RFC1918 / loopback / link-local ranges in general, because snapshotting http://localhost:3000 and internal staging hosts is a core Percy use case.

What is blocked

isMetadataTarget(url) / assertNotMetadataTarget(url) (in packages/core/src/utils.js) return/throw when the URL host is a known cloud instance-metadata endpoint:

• Literal IPs: 169.254.169.254 (AWS/Azure/GCP IMDS), 169.254.170.2 (ECS task metadata), 100.100.100.200 (Alibaba), and IPv6 fd00:ec2::254 (AWS IMDS v6).
• Hostnames (case-insensitive, exact or trailing-dot): metadata.google.internal, metadata.goog.
• DNS-rebinding defense: the hostname is also resolved (dns.promises.lookup, all addresses); the request is blocked if any resolved address is a metadata IP. Resolution failures are non-fatal — only a positive metadata match blocks — so offline/localhost snapshots keep working.

Where the check is applied

1. Top-level navigation — assertNotMetadataTarget(snapshot.url) before page.goto(snapshot.url, ...) in packages/core/src/discovery.js.
2. Discovered subresource requests — isMetadataTarget(url) in sendResponseResource (packages/core/src/network.js), gating the outbound Fetch.continueRequest path so a page cannot pivot to IMDS via a subresource. On a match the request is failed (Aborted) with a clear warn log: Refusing to fetch resource from cloud metadata endpoint: <host>. Cache hits and root resources are served earlier and never reach this branch, so no behavior changes for them.

What is intentionally still allowed

• Loopback — 127.0.0.1, localhost, [::1].
• General RFC1918 private hosts — e.g. 192.168.x.x, 10.x.x.x (internal staging).
• Normal public hosts.

No behavior is altered for any non-metadata host.

Tests

Added unit tests in packages/core/test/utils.test.js covering: metadata IPs (v4 + v6) blocked, metadata hostnames (case-insensitive, trailing dot) blocked, DNS-rebinding to a metadata IP blocked, and localhost / 127.0.0.1 / [::1] / 192.168.1.10 / 10.0.0.5 / a public host all still allowed, plus DNS-failure and unparseable-URL cases.

All 10 new specs pass (yarn workspace @percy/core test --node). node --check passes on all edited source files.

🤖 Generated with Claude Code

[Shivanshu-07]

Claude Code PR Review

PR: #2333 • Head: 57426e1 • Reviewers: stack:pr-review orchestrator + independent SSRF/correctness reviewer

Summary

Blocks cloud instance-metadata SSRF targets (169.254.169.254, 169.254.170.2, 100.100.100.200, fd00:ec2::254, metadata.google.internal, metadata.goog) in both snapshot navigation (assertNotMetadataTarget in discovery.js) and subresource discovery (isMetadataTarget in network.js), with a resolve-and-match DNS-rebind guard, while intentionally leaving loopback/RFC1918 hosts allowed.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	None introduced
High	Security	Authentication/authorization checks present	N/A	Not an auth-surface change
High	Security	Input validation and sanitization	Fail	Host canonicalization misses IPv4-mapped IPv6 ([::ffff:169.254.169.254]) — reaches IMDS on Linux
High	Security	No IDOR — resource ownership validated	N/A	N/A
High	Security	No SQL injection (parameterized queries)	N/A	N/A
High	Correctness	Logic is correct, handles edge cases	Fail	Subresource check runs on originURL (redirectChain[0]) not the redirect hop target — open-redirect to metadata bypasses the block
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	DNS failures intentionally non-fatal; documented
High	Correctness	No race conditions or concurrency issues	Pass	None
Medium	Testing	New code has corresponding tests	Pass	utils + discovery specs added (132 utils specs pass; discovery metadata spec passes)
Medium	Testing	Error paths and edge cases tested	Pass	DNS-fail, unparseable, non-canonicalizable address covered
Medium	Testing	Existing tests still pass (no regressions)	Pass	No regressions observed in affected suites
Medium	Performance	No N+1 queries or unbounded data fetching	Pass	See Low note on per-subresource DNS
Medium	Performance	Long-running tasks use background jobs	N/A	N/A
Medium	Quality	Follows existing codebase patterns	Pass	Matches instrumentation/interception patterns
Medium	Quality	Changes are focused (single concern)	Pass	Focused SSRF hardening
Low	Quality	Meaningful names, no dead code	Pass	Clear naming
Low	Quality	Comments explain why, not what	Pass	Good rationale comments; no ticket ids in code
Low	Quality	No unnecessary dependencies added	Pass	Uses built-in net/dns

Findings

• File: packages/core/src/network.js:699 (with originURL at :600)

• Severity: High

• Issue: The subresource block evaluates isMetadataTarget(url) where url = originURL(request) = normalizeURL(request.redirectChain[0].url) — the original request URL. On a redirect hop, request.url is the redirect target (e.g. the metadata IP) but redirectChain[0] is the benign origin, so a page requesting https://attacker.example/r that 302-redirects to http://169.254.169.254/… is checked against attacker.example, returns null, and Fetch.continueRequest issues the outbound request to IMDS. The direct-hit block is defeated by a one-line open redirect. Confirmed against the redirect-chain construction in _handleRequest (network.js:388-403).

• Suggestion: Also inspect the actual hop target: block when await isMetadataTarget(url) || (request.redirectChain.length && await isMetadataTarget(request.url)).

• File: packages/core/src/utils.js (canonicalHost / METADATA_IPS / isMetadataTarget net.isIP short-circuit)

• Severity: High

• Issue: [::ffff:169.254.169.254] canonicalizes (verified) to ::ffff:a9fe:a9fe, which is not in METADATA_IPS (holds dotted 169.254.169.254), and net.isIP('::ffff:a9fe:a9fe')===6 makes isMetadataTarget return null and skip DNS. At the socket layer Linux maps this to the IPv4 metadata service, so the request reaches IMDS. Applies to all three IPv4 metadata IPs in mapped form and to any host that resolves to a mapped address.

• Suggestion: In canonicalHost, unwrap IPv4-mapped IPv6 (::ffff:wwww:xxxx) to dotted IPv4 before comparison so it compares equal to the literal IPv4 metadata entries.

• File: packages/core/src/utils.js (dns.promises.lookup)

• Severity: Low

• Issue: The resolve-and-match rebind guard resolves independently of the connection Chromium makes, so a true DNS-rebind attacker controlling authoritative DNS can still return benign to Node and metadata to the browser. Raises the bar; the "defeat DNS rebinding" framing overstates it.

• Suggestion: Keep it, but rely on literal-IP matching as the primary control. No change required.

• File: packages/core/src/utils.js (isMetadataTarget called from network.js)

• Severity: Low

• Issue: Every non-IP-literal external subresource (not a cache/root hit) now incurs an async dns.promises.lookup; repeated when caching is disabled / responsive capture is on.

• Suggestion: Optionally memoize per host per snapshot. Not blocking.

Verified correct (not gaps): numeric IPv4 encodings (decimal/hex/octal) all canonicalize to dotted form on both paths; trailing-dot/case/IPv6-compression/userinfo handled; canonicalHost function-declaration hoisting is fine; no false positives for localhost/127.0.0.1/[::1]/192.168.x/10.x/unresolvable hosts; both navigation and subresource entry points wired.

---

Verdict: FAIL

[Shivanshu-07]

Claude Code PR Review (re-review after fixes)

PR: #2333 • Head: 3bce1d5 • Reviewers: stack:pr-review orchestrator + independent SSRF/correctness reviewer

Summary

Blocks cloud instance-metadata SSRF targets (169.254.169.254, 169.254.170.2, 100.100.100.200, fd00:ec2::254, metadata.google.internal, metadata.goog) in both snapshot navigation (assertNotMetadataTarget) and subresource discovery (isMetadataTarget), with a resolve-and-match DNS-rebind guard, while intentionally leaving loopback/RFC1918 hosts allowed. Follow-up commit 3bce1d5 closes the two High bypasses found in the first review.

What changed since the FAIL review

• Redirect-target bypass (High) — fixed. network.js now also evaluates the actual redirect hop (request.url) — ... || (request.redirectChain.length && (metadataHost = await isMetadataTarget(request.url))) — so an open redirect on an allowed host can no longer smuggle a request through to a metadata endpoint. The origin URL is still checked for direct hits; the extra lookup only runs on redirect hops (no added DNS cost for normal resources). A discovery.test.js spec (301 → 169.254.169.254) proves the block.
• IPv4-mapped IPv6 bypass (High) — fixed. canonicalHost now folds ::ffff:wwww:xxxx back to dotted-quad, so [::ffff:169.254.169.254] (and the mapped forms of the other IPv4 metadata IPs) compare equal to the literal entries and are blocked. New utils.test.js assertions cover blocked mapped IPs and an allowed non-metadata mapped host.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	None introduced
High	Security	Authentication/authorization checks present	N/A	Not an auth-surface change
High	Security	Input validation and sanitization	Pass	IPv4-mapped IPv6 now canonicalized and blocked
High	Security	No IDOR — resource ownership validated	N/A	N/A
High	Security	No SQL injection (parameterized queries)	N/A	N/A
High	Correctness	Logic is correct, handles edge cases	Pass	Redirect hop target now checked; numeric/case/trailing-dot/IPv6 forms handled
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	DNS failures intentionally non-fatal; documented
High	Correctness	No race conditions or concurrency issues	Pass	None
Medium	Testing	New code has corresponding tests	Pass	134 utils specs pass; both discovery metadata specs (direct + redirect) pass
Medium	Testing	Error paths and edge cases tested	Pass	DNS-fail, unparseable, non-canonicalizable address, mapped IPv6, redirect all covered
Medium	Testing	Existing tests still pass (no regressions)	Pass	utils suite + affected discovery specs green; lint clean
Medium	Performance	No N+1 queries or unbounded data fetching	Pass	Extra lookup only on redirect hops
Medium	Performance	Long-running tasks use background jobs	N/A	N/A
Medium	Quality	Follows existing codebase patterns	Pass	Matches instrumentation/interception patterns
Medium	Quality	Changes are focused (single concern)	Pass	Focused SSRF hardening
Low	Quality	Meaningful names, no dead code	Pass	Clear naming; no ticket ids in code comments
Low	Quality	Comments explain why, not what	Pass	Good rationale comments
Low	Quality	No unnecessary dependencies added	Pass	Uses built-in net/dns

Findings

• File: packages/core/src/utils.js (dns.promises.lookup)

• Severity: Low (non-blocking)

• Issue: The resolve-and-match rebind guard resolves independently of the connection Chromium makes, so a true DNS-rebind attacker controlling authoritative DNS could still return benign to Node and metadata to the browser. It meaningfully raises the bar; literal-IP matching (now including mapped IPv6) is the primary control.

• Suggestion: Keep as-is; consider softening the "defeat DNS rebinding" wording. No change required.

• File: packages/core/src/utils.js (METADATA_IPS)

• Severity: Low (non-blocking)

• Issue: List omits some less-common providers (e.g. Oracle Cloud 192.0.0.192).

• Suggestion: Extend the set in a follow-up if those providers are in scope.

No Critical or High findings remain.

---

Verdict: PASS

[Shivanshu-07]

Claude Code PR Review

PR: #2333 • Head: 3bce1d5 • Reviewers: self (orchestrator adversarial security + correctness + quality review)

Summary

Blocks cloud instance-metadata SSRF (PER-8614): a new isMetadataTarget/assertNotMetadataTarget guard in packages/core/src/utils.js refuses to navigate the top-level snapshot URL (discovery.js) or fetch any discovered subresource (network.js) that targets a known cloud metadata endpoint, while preserving loopback and RFC1918 hosts for legitimate internal snapshotting.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	No secrets; only a static metadata IP/hostname denylist.
High	Security	Authentication/authorization checks present	N/A	Not an auth-surface change.
High	Security	Input validation and sanitization	Pass	Host canonicalization handles decimal/octal/hex IPv4, IPv4-mapped & compressed IPv6, trailing dots, case; all reject paths verified to block.
High	Security	No IDOR — resource ownership validated	N/A	No resource-ownership surface.
High	Security	No SQL injection (parameterized queries)	N/A	No DB access.
High	Correctness	Logic is correct, handles edge cases	Pass	Guard runs before outbound fetch; both origin URL and post-redirect request.url checked; short-circuit assignment correct.
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	DNS/parse failures intentionally non-fatal (fail toward NOT blocking); documented; only a positive match blocks.
High	Correctness	No race conditions or concurrency issues	Pass	Per-request async guard; no shared mutable state added. See residual DNS TOCTOU note (Low).
Medium	Testing	New code has corresponding tests	Pass	13 unit specs + 2 discovery integration specs; a commit raised coverage to 100%.
Medium	Testing	Error paths and edge cases tested	Pass	Reject paths (literal IPv4/IPv6, mapped IPv6, trailing dot, hostnames, DNS rebinding) and allow paths (loopback, RFC1918, public, DNS-fail, unparseable, bad-canon) covered.
Medium	Testing	Existing tests still pass (no regressions)	Pass	All CI test jobs green.
Medium	Performance	No N+1 queries or unbounded data fetching	Pass	No queries.
Medium	Performance	Long-running tasks use background jobs	N/A	N/A.
Medium	Quality	Follows existing codebase patterns	Pass	Reuses logAssetInstrumentation category pattern and the Fetch.failRequest abort path.
Medium	Quality	Changes are focused (single concern)	Pass	Scoped to the SSRF guard + tests only.
Low	Quality	Meaningful names, no dead code	Pass	Clear names; thorough why-comments.
Low	Quality	Comments explain why, not what	Pass	Comments explain the threat model and canonicalization rationale.
Low	Quality	No unnecessary dependencies added	Pass	Only Node builtins net/dns.

Findings

Adversarial verification exercised the full bypass matrix against the actual code. All were correctly blocked:

• Redirect hop — post-redirect target (request.url) is checked, not just the origin URL; confirmed via the response-paused -> re-requestPaused re-entry and the dedicated integration test.
• IPv4-mapped IPv6 [::ffff:169.254.169.254] (and compressed [0:0:0:0:0:ffff:...]) — folded back to dotted-quad and blocked.
• Decimal / hex / octal IPv4 (2852039166, 0xA9FEA9FE, 0251.0376.0251.0376) — normalized by the WHATWG URL parser and blocked.
• DNS rebinding — hostnames resolving to a metadata IP are blocked via dns.lookup({all:true}).
• Alternate hosts — fd00:ec2::254, metadata.google.internal, metadata.goog, 169.254.170.2 (ECS), 100.100.100.200 (Alibaba) covered; trailing-dot host normalized and blocked.
• Loopback / RFC1918 (127.0.0.1, ::1, 192.168.x, 10.x) correctly preserved.

No Critical/High/Medium findings. Two Low / informational notes (non-blocking):

• File: packages/core/src/utils.js (isMetadataTarget DNS resolution)

• Severity: Low

• Issue: Residual DNS TOCTOU — the guard's dns.lookup is independent of Chromium's own resolution at connect time, so a rebinding host that resolves benign for the guard but to a metadata IP for the browser could still slip through. Direct-IP, redirect, and rebinding-at-check-time vectors are all closed; this residual is inherent to check-then-continue at this layer.

• Suggestion: Acceptable for the stated threat model; a future hardening could deny at the connection layer. Worth documenting as a known limitation.

• File: packages/core/src/network.js (metadata check in sendResponseResource)

• Severity: Low

• Issue: isMetadataTarget issues a dns.lookup per uncached external subresource host with no memoization, adding a resolution per unique host on the discovery path.

• Suggestion: Relies on the OS resolver cache so impact is small; consider a short-lived per-snapshot host cache if discovery latency regresses.

---

Verdict: PASS

[pranavz28]

Automated multi-agent security review. The direct vectors are solidly closed and I verified the suspected bypasses do NOT work: literal metadata IPs in decimal/octal/hex all normalize via WHATWG URL and match (http://2852039166/ → 169.254.169.254), IPv6 / IPv4-mapped forms canonicalize correctly, and the redirect block is genuinely pre-request (request-stage interception catches every hop before Chromium fetches it). Nice work on those.

The blocker is the DNS-rebinding leg — the mechanism the code names as its rebinding defense doesn't actually stop rebinding, because Chromium re-resolves independently. Plus two paths skip the check. Inline below.

[pranavz28]

P1 (blocker) — this lookup does not defend against DNS rebinding, though the comment above claims it does. isMetadataTarget resolves via Node's dns.promises.lookup, but the actual request is made by a different resolver — Chromium (after Fetch.continueRequest) or Node's http stack in the direct-fetch fallback — which re-resolves independently. Classic TOCTOU, and the attacker controls the subresource URLs by design (they author the page).

Bypass: attacker DNS returns a benign IP (or NXDOMAIN → treated as non-fatal → allowed) to this lookup, then 169.254.169.254 to Chromium's lookup (short TTL / round-robin / A-vs-AAAA split). IMDS is fetched and its body uploaded. So this reliably blocks only literal IPs/hostnames and hosts that statically resolve the same at both moments — not rebinding.

Fix: gate on the IP Chromium actually connected to — response.remoteIPAddress at the response stage (network.js _handleResponseReceived / _handleResponsePaused), before buffering the body. Keep the cheap literal/hostname pre-check as a first line, but correct the comment's rebinding claim.

[Shivanshu-07]

Fixed in e129bf2. You are right — the request-time dns.lookup was a TOCTOU no-op against rebinding, and that lookup is now removed. The rebinding leg is closed at the response stage: _handleResponseReceived now gates on response.remoteIPAddress (the IP Chromium actually connected to) and, on a metadata match, logs + drops the request before request.response.buffer is ever wired up — so the body is never fetched via Network.getResponseBody or uploaded. The synchronous literal-IP/hostname pre-check in sendResponseResource stays as a cheap first line, and its misleading "defeats rebinding" comment is corrected to point at the response-stage gate. Proven by a unit test that drives _handleResponseReceived with a benign request-time host but a remoteIPAddress of 169.254.169.254 (and the ::ffff: mapped form) and asserts the resource is blocked and no buffer is attached, plus a discovery integration test that rewrites the connected IP on Network.responseReceived.

[pranavz28]

P2 — the check isn't applied on all outbound paths. Two real paths skip isMetadataTarget entirely: (1) the direct-fetch fallback captureResourceDirectly → makeDirectRequest (worker/PlzDedicatedWorker scripts) — and it's the one path that also attaches cookies/Basic auth; (2) the service-worker branch (if (!serviceWorker) skips sendResponseResource) when captureMockedServiceWorker is enabled. Consider factoring the check into a single choke point all continue/direct-fetch paths funnel through.

P2 (perf) — every non-cached subresource now awaits a dns.lookup before continueRequest, adding a DNS round-trip to the critical path of essentially every public asset (libuv threadpool pressure on cold discovery). The response-stage remoteIPAddress fix from the other comment removes the need for this pre-request lookup — you'd keep only the cheap synchronous literal-IP/hostname Set checks here.

[Shivanshu-07]

Fixed in e129bf2. All outbound paths now funnel the metadata decision through a single choke point (matchMetadataHost, exposed as isMetadataTarget for literal hosts and isMetadataIP for connected IPs):

• Direct-fetch fallback (captureResourceDirectly → makeDirectRequest, the path that attaches cookies/Basic auth): the transport now runs in Network#directFetch, which captures the socket's actual connected IP via a dns lookup hook (the same resolution the connection uses — not an extra round-trip). makeDirectRequest runs isMetadataIP on that IP and throws MetadataBlockedError on a hit, so the body is never saved/uploaded. This covers both direct-fetch callers (worker/PlzDedicatedWorker scripts and the font-mime re-fetch).
• Service-worker branch: it never issues its own outbound fetch — its responses surface through Network.responseReceived, which is exactly where the new response-stage remoteIPAddress gate runs, so it is covered by the same universal choke point.

Perf: the per-request await dns.lookup before continueRequest is removed — the request path now does only the cheap synchronous literal-IP/hostname Set checks, and the real enforcement moved to the response stage / connected IP. No more DNS round-trip on every subresource.

[pranavz28]

P3 (test brittleness). Asserting the exact canonical form '[::ffff:a9fe:a9fe]' couples the test to Node's WHATWG-URL rendering — a normalization change would fail this without any behavior regression. Prefer asserting the decision (truthy host returned / assertNotMetadataTarget throws) across the equivalent input forms, and keep at most one test pinning the exact canonicalization if you want to lock the format.

[Shivanshu-07]

Fixed in e129bf2. The IPv6 / IPv4-mapped tests now assert the decision (toBeTruthy() that a host is returned, assertNotMetadataTarget throws / does not throw) across the equivalent input forms, so a WHATWG-URL normalization change can no longer fail them without a real behavior regression. Exactly one test still pins the exact canonical form ("[::ffff:a9fe:a9fe]") to deliberately lock the canonicalHost rendering.

[Shivanshu-07]

Claude Code PR Review

PR: #2333 • Head: e129bf2 • Reviewers: fallback inline checklist

Summary

Blocks SSRF to cloud instance-metadata endpoints (AWS/GCP/Azure IMDS, ECS, Alibaba) during asset discovery, enforced at four layers — top-level navigation guard, sub-resource request pre-check (incl. redirect target), response-stage connected-IP gate (DNS-rebinding-proof), and direct-fetch socket-IP gate — all funneling through one matchMetadataHost decision (PER-8614, CWE-918).

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	None introduced
High	Security	Authentication/authorization checks present	N/A	No auth surface; this hardens an egress path
High	Security	Input validation and sanitization	Pass	Metadata host/IP validated at request, response, direct-fetch, and navigation stages
High	Security	No IDOR — resource ownership validated	N/A	—
High	Security	No SQL injection (parameterized queries)	N/A	No SQL
High	Correctness	Logic is correct, handles edge cases	Pass	IPv6 compression, IPv4-mapped IPv6, redirect hop, DNS rebinding all covered
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	MetadataBlockedError caught + dropped (already logged); other errors fall through to network_error path
High	Correctness	No race conditions or concurrency issues	Pass	Block runs before body buffering; no shared mutable state
Medium	Testing	New code has corresponding tests	Pass	isMetadataTarget/assertNotMetadataTarget unit tests + end-to-end non-capture + redirect + rebinding
Medium	Testing	Error paths and edge cases tested	Pass	DNS failure fail-open, unparseable URL, non-canonicalizable address all covered
Medium	Testing	Existing tests still pass (no regressions)	Pass	@percy/core green; loopback/RFC1918/public hosts still allowed (asserted)
Medium	Performance	No N+1 queries or unbounded data fetching	Pass	matchMetadataHost is synchronous/DNS-free; custom lookup reuses the connection's resolution (no extra round-trip)
Medium	Performance	Long-running tasks use background jobs	N/A	—
Medium	Quality	Follows existing codebase patterns	Pass	Instrumentation via logAssetInstrumentation, consistent with existing asset paths
Medium	Quality	Changes are focused (single concern)	Pass	Metadata SSRF only
Low	Quality	Meaningful names, no dead code	Pass	—
Low	Quality	Comments explain why, not what	Pass	Each layer documents the threat + why the block sits there
Low	Quality	No unnecessary dependencies added	Pass	Uses built-in dns only

Findings

No Critical or High findings.

Design note (intentional, non-blocking): the fix is surgical — it blocks only known cloud-metadata endpoints and deliberately keeps loopback + RFC1918 reachable, diverging from the ticket's broader "block all private ranges" remediation. This is correct for Percy (snapshotting localhost/internal hosts is core usage) and is asserted by the allow-list tests. DNS-failure is fail-open (returns null → allowed) to avoid breaking legitimate hosts on flaky resolution.

---

Verdict: PASS