Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ GOAL: make the server fetch an internal/metadata target
├── A2 cloud metadata IP (http://169.254.169.254/…) [blocked: link-local /16]
├── A3 loopback (http://127.0.0.1:3000/…) [blocked: 127/8]
├── A4 IPv6 loopback / ULA (http://[::1]/, fd00::/fe80::) [blocked: isBlockedIpv6]
├── A5 IPv4-mapped IPv6 (http://[::ffff:10.0.0.1]/) [blocked: mapped→v4 check]
├── A5 IPv4-mapped IPv6 (http://[::ffff:10.0.0.1]/) [blocked: mapped→v4 check, dotted + hex*]
├── A6 alternate scheme (file://, gopher://) [blocked: scheme allowlist]
├── A7 internal hostname (db.internal, svc.local) [blocked: .internal/.local]
├── A8 abuse engine internal route (relative "/api/…") [EXEMPT by design — see §5]
Expand All @@ -115,6 +115,17 @@ The leaves marked *blocked* map 1:1 to code in §6. The two unclosed leaves (**A
**B2a IP rotation**) are the honest edges of this PR and are called out as residual risk — a recurring
theme in security: *you ship the high-value mitigations and document the remainder.*

> **\*Post-release finding (Sept 2026) — test through the parser, not around it.** The original A5
> check only matched the dotted form `::ffff:a.b.c.d`, and its unit test passed that string straight to
> `isBlockedHost`. But the engine calls `checkOutboundUrl(url)`, and the WHATWG `URL` parser
> **normalizes** the host first: `http://[::ffff:169.254.169.254]/` arrives as `[::ffff:a9fe:a9fe]`.
> The hex form slipped past the guard. The fix decodes hex-embedded IPv4 (`::ffff:x:y` and the
> deprecated `::x:y`) and re-applies the IPv4 rules; the regression tests now go through
> `checkOutboundUrl` so the parser is part of what's tested. Decimal/octal/hex IPv4 hosts
> (`http://2130706433/`, `http://0x7f.1/`) were already safe — the parser turns them into dotted quads.
> It was caught when a second consumer (the builder's validation panel) was pointed at the same guard
> and its tests exercised real URLs.

## 5. Design considerations (decisions & trade-offs)

1. **Blocklist, not allowlist — for now.** A general HTTP node must reach arbitrary public APIs, so a
Expand Down Expand Up @@ -225,6 +236,9 @@ rate-limit store (needs a dep), and the JS-node sandbox replacement (separate W1
5. **Red-team (stretch).** Write `evil.example` DNS that resolves to `127.0.0.1`, point the HTTP node at
it, and show the literal check **doesn't** stop it (A9). Then sketch a fix (resolve + check resolved
IP + pin). Discuss the latency/complexity trade-off.
6. **Parser differential.** Run `node -e 'console.log(new URL("http://[::ffff:127.0.0.1]/").hostname)'`.
Explain why a guard must validate the host *as the HTTP client will see it*, then find the tests in
`ssrf.test.ts` that pin this down (`URL-normalized host forms`).

**Discussion:** Where else in this app does untrusted input choose a side effect? (Hint: the
`javascript`/`tool` node's `new Function` — a future tutorial.) Should rate limits be per-IP, per-token,
Expand Down
39 changes: 39 additions & 0 deletions lib/security/__tests__/ssrf.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,42 @@ describe("assertSafeOutboundUrl", () => {
expect(() => assertSafeOutboundUrl("https://api.osv.dev/v1/query")).not.toThrow()
})
})

// Regression: the URL parser NORMALIZES hosts before isBlockedHost sees them.
// IPv4-mapped IPv6 comes out in hex (`[::ffff:127.0.0.1]` → `[::ffff:7f00:1]`), and
// decimal/octal/hex IPv4 forms become dotted-quad. These must be checked through
// checkOutboundUrl (what the engine calls), not just as raw host strings.
describe("checkOutboundUrl — URL-normalized host forms", () => {
test.each([
"http://[::ffff:127.0.0.1]/", // → [::ffff:7f00:1]
"http://[::ffff:169.254.169.254]/latest/meta-data/", // cloud metadata → [::ffff:a9fe:a9fe]
"http://[::ffff:10.0.0.1]/", // → [::ffff:a00:1]
"http://[::ffff:192.168.1.1]/",
"http://[0:0:0:0:0:ffff:7f00:1]/", // expanded mapped form
"http://[::ffff:7f00:1]/", // hex written directly
"http://[::127.0.0.1]/", // deprecated IPv4-compatible → [::7f00:1]
"http://[0:0:0:0:0:0:0:1]/", // expanded loopback → [::1]
"http://2130706433/", // decimal 127.0.0.1
"http://0177.0.0.1/", // octal 127.0.0.1
"http://0x7f.1/", // hex/short 127.0.0.1
"http://0xa9fea9fe/", // hex 169.254.169.254
])("blocks %s", (url) => {
expect(checkOutboundUrl(url).safe).toBe(false)
})

test.each([
"http://[::ffff:8.8.8.8]/", // mapped public v4
"http://[2606:4700:4700::1111]/", // public IPv6
])("allows %s", (url) => {
expect(checkOutboundUrl(url).safe).toBe(true)
})
})

describe("isBlockedHost — IPv6 hex-embedded IPv4", () => {
test.each(["[::ffff:7f00:1]", "::ffff:a9fe:a9fe", "[::7f00:1]", "::ffff:a00:1"])("blocks %s", (host) => {
expect(isBlockedHost(host)).toBe(true)
})
test("allows mapped public v4 in hex (::ffff:808:808 = 8.8.8.8)", () => {
expect(isBlockedHost("::ffff:808:808")).toBe(false)
})
})
10 changes: 10 additions & 0 deletions lib/security/ssrf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,16 @@ function isBlockedIpv6(host: string): boolean {
if (h.startsWith("fc") || h.startsWith("fd")) return true // unique-local fc00::/7
const mapped = h.match(/::ffff:(\d{1,3}(?:\.\d{1,3}){3})$/) // IPv4-mapped ::ffff:a.b.c.d
if (mapped) return isBlockedIpv4(mapped[1])
// The WHATWG URL parser normalizes embedded IPv4 to hex: `[::ffff:127.0.0.1]` →
// `[::ffff:7f00:1]`, `[::127.0.0.1]` → `[::7f00:1]`. Decode and re-check, so
// IPv4-mapped (::ffff:0:0/96) and deprecated IPv4-compatible (::/96) addresses
// can't smuggle a blocked IPv4 target past the guard.
const hexEmbedded = h.match(/^::(?:ffff:)?([0-9a-f]{1,4}):([0-9a-f]{1,4})$/)
if (hexEmbedded) {
const hi = parseInt(hexEmbedded[1], 16)
const lo = parseInt(hexEmbedded[2], 16)
return isBlockedIpv4(`${hi >> 8}.${hi & 0xff}.${lo >> 8}.${lo & 0xff}`)
}
return false
}

Expand Down
Loading