ssrf-guard: bracketed IPv6 literals skip the private-IP branch, and the module has no tests

[groupthinking]

Summary

Two defects in apps/web/src/lib/ssrf-guard.ts, both surfaced while reconciling #1428 against #1381 (which merged as feae3d3 and rewrote this module's rejection paths).

1. Bracketed IPv6 literals never reach ipIsPrivate

URL.hostname keeps the brackets on an IPv6 literal — new URL('http://[::1]/').hostname is [::1], not ::1. net.isIP does not accept that spelling, so the literal branch is skipped:

const host = u.hostname.toLowerCase().replace(/\.$/, '');   // "[::1]"
if (net.isIP(host)) {                                        // false
  if (ipIsPrivate(host)) throw new Error('Blocked private IP literal');
  return u;
}
// ...falls through to dns.lookup("[::1]")

Consequences:

• Every IPv6 literal — private or public — is handed to dns.lookup instead of being range-checked. ipIsPrivate never sees it.
• http://[::1]/ is still rejected, but only incidentally: the resolver errors on a bracketed name and the DNS branch converts that into a rejection. The loopback range check plays no part. All of the careful IPv6 work in ipIsPrivate/ipv6ToHextets (link-local, unique-local, IPv4-mapped in any spelling) is dead code for literals supplied in a URL.
• Public IPv6 literals such as http://[2606:4700:4700::1111]/ are rejected too, which this guard is not meant to do.

Verified against main (5934cbf): a test asserting dns.lookup is not consulted for http://[::1]/ fails, with the mock recording a call for "[::1]".

Fix: strip the surrounding brackets before net.isIP.

2. No test coverage

#1381 rewrote all of this module's rejection paths to close the CWE-209 DNS oracle and shipped without tests. ssrf-guard.ts — a security-critical module — currently has none, so the indistinguishability property #1381 argued for is unpinned and a future edit can silently reopen the oracle.

The gap is not hypothetical: writing the tests is what exposed defect 1 above.

Acceptance criteria

• A bracketed IPv6 literal is range-checked by ipIsPrivate, not resolved.
• http://[::1]/ is rejected by the IP-literal branch; a public IPv6 literal is allowed.
• The four DNS outcomes (resolver rejection, transient failure, zero results, private result) remain byte-identical to the caller, asserted by comparing branches against each other rather than against a literal.
• The operator-side signal is asserted to still differ, so the cause is moved to the logs rather than lost.
• The IP-literal message stays deliberately distinct from the DNS one (fix(web): stop leaking upstream and Stripe error details to clients #1381's documented choice), pinned by a test so a future uniformity pass has to change a failing test rather than silently flip it.
• Tests proven non-vacuous against both the pre-fix(web): stop leaking upstream and Stripe error details to clients #1381 guard and current main.

Relationship to #1427 / #1428

#1428 is the open PR for #1427 and proposed a broader redesign (a typed SsrfGuardError with a diagnostic reason, plus collapsing all six rejection paths onto one exported constant). It was authored before #1381 merged, states "No file overlap — #1381 changes neither file", and is now conflicted (mergeable_state: dirty) against main on both files it touches.

This issue is deliberately narrower: keep #1381's merged behaviour, fix the bracket bug, and add the regression coverage. It does not decide the SsrfGuardError question, which remains #1428's to settle.

[linear-code]

GRV-383

[coderabbitai]

🔗 Related PRs

#265 - test(e2e): forward Vercel protection-bypass header for preview testing [merged]
#274 - test(e2e): harden withBypass header merge against non-object HeadersInit [merged]
#605 - fix(follow-up): correct Trivy commit pin + broaden YouTube host allowlist (CodeRabbit findings on merged #604) [merged]
#658 - fix(web): fail open for cheap AI status GETs, closed only for paid AI calls (follow-up to #655) [merged]
#693 - fix(ci): unblock the entire PR backlog — resolve invalid agent_network.json + restore apps/web type-check script [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[groupthinking]

Defect 1 confirmed independently — but two of this issue's premises are stale, and acting on them as written would duplicate work that already exists.

Defect 1 reproduces

Not taking it on description. Against Node directly:

"http://[::1]/"                    hostname="[::1]"                  net.isIP=0
"http://[2606:4700:4700::1111]/"   hostname="[2606:4700:4700::1111]" net.isIP=0
"http://127.0.0.1/"                hostname="127.0.0.1"              net.isIP=4

So the literal branch is skipped for every IPv6 spelling and ipIsPrivate never runs on one. The bracket-stripping fix is right.

One refinement on severity: this is over-blocking and dead code, not an exploitable bypass. [::1] still gets rejected — dns.lookup("[::1]") errors and the catch converts it to NOT_PUBLIC. The live defect is the false negative in the other direction: a legitimate public IPv6 literal is rejected too. Worth stating so nobody triages this as an open SSRF hole.

Correction: the module is not untested

#1381 rewrote all of this module's rejection paths … and shipped without tests. ssrf-guard.ts — a security-critical module — currently has none

feae3d3 (#1381) shipped both of these:

apps/web/src/lib/__tests__/ssrf-guard-dns-leakage.test.ts        126 ++++++
apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts 143 ++++++

Attribution: git log origin/main --diff-filter=A -- apps/web/src/lib/__tests__/ssrf-guard-dns-leakage.test.ts → feae3d321.

Since this issue was filed, 75ef3ca (#1428) added a third, ssrf-guard-private-address-detection.test.ts (+76). Current coverage on main is 345 lines across three files.

That matters for the acceptance criteria here — criteria 3 and 4 are already asserted by ssrf-guard-dns-leakage.test.ts, which compares the four DNS outcomes against each other rather than against a literal, and asserts the operator-side console.error still differs. Writing them again would duplicate it. Criteria 1, 2 and 5 (the bracket behaviour) are genuinely uncovered and are the real remaining work.

Correction: the SsrfGuardError question is now orphaned

It does not decide the SsrfGuardError question, which remains #1428's to settle.

#1428 merged as 75ef3ca — and merged test-only. Both source files kept #1381's versions; no SsrfGuardError, no SSRF_REJECTION_MESSAGE reached main. That was the right call on a conflicted branch, but it means nothing now owns that decision.

The substance it would have addressed is still open: assertPublicHttpUrl throws four distinguishable literals (Invalid URL, Blocked URL scheme: <protocol>, Blocked host, Blocked private IP literal). Not currently reachable — the only live caller returns a bare constant, and transcription-service-improved.ts imports the guard without calling it — so it is defense-in-depth against a future caller that forwards err.message, which is precisely how #1427 arrived. Either fold it in here or file it separately, but it should not be left pointing at a merged PR.

---

Generated by Claude Code