From 2d65d6d75749372301bb9add8d9f9116e21c4da0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 21:48:49 +0000 Subject: [PATCH 1/2] test(web): pin both edges of the SSRF guard's IPv6 bit-mask ranges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1520 `ssrf-guard.test.ts` covers the too-narrow direction for every blocked range — each has an address squarely inside it. Nothing covers too-wide. `fec0::/10` and `100::/64` are the two checks written as bit masks rather than exact matches, and a widened mask fails silently in the opposite direction: it refuses public space, reporting the same uniform message every other rejection uses. Neither edge of either mask was pinned, so widening one broke no test. Adds both edges of each range plus a just-outside neighbour, through both paths into `ipIsPrivate` — URL literals and DNS answers enter via different branches of `assertPublicHttpUrl` and report different messages, so a regression can land on one and not the other. Also covers `64:ff9b::169.254.169.254`, the dotted-quad spelling of a case `ssrf-guard.test.ts` already has in hex. `ipv6ToHextets` folds the trailing quad before any range check runs, so both spellings must agree. These pass on `main` by construction — there is no bug here. Non-vacuity is therefore shown by mutation instead, each mutant caught by exactly the test that claims to cover it: (h[0] & 0xffc0) === 0xfec0 -> (h[0] & 0xff00) === 0xfe00 fails: still allows [fe00::1], which is below the range h[0] === 0x0100 && h[1..3] === 0 -> h[0] === 0x0100 fails: still allows [100:0:0:1::1] (inside 100::/16, outside the /64) Tests only. Full apps/web suite 59 files / 354 passed; tsc and eslint clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NNXdtee4Wwwjew74cNZ1C5 --- .../ssrf-guard-range-boundaries.test.ts | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 apps/web/src/lib/__tests__/ssrf-guard-range-boundaries.test.ts diff --git a/apps/web/src/lib/__tests__/ssrf-guard-range-boundaries.test.ts b/apps/web/src/lib/__tests__/ssrf-guard-range-boundaries.test.ts new file mode 100644 index 000000000..176f4b62a --- /dev/null +++ b/apps/web/src/lib/__tests__/ssrf-guard-range-boundaries.test.ts @@ -0,0 +1,124 @@ +import { describe, it, expect, afterEach, vi } from 'vitest'; + +vi.mock('node:dns/promises', () => ({ + lookup: vi.fn(), +})); + +import * as dns from 'node:dns/promises'; +import { assertPublicHttpUrl } from '@/lib/ssrf-guard'; + +afterEach(() => { + vi.restoreAllMocks(); + vi.mocked(dns.lookup).mockReset(); +}); + +/** + * Boundary coverage for the IPv6 range checks in `ipIsPrivate`. + * + * `ssrf-guard.test.ts` establishes that each blocked range *is* blocked — it + * walks NAT64, 6to4, IPv4-translated, site-local and discard with an address + * squarely inside each. That is the "too narrow" direction: a check that missed + * would let an address through. + * + * Nothing yet holds the other direction. `fec0::/10` and `100::/64` are the two + * checks written as bit masks rather than exact matches, and a mask that is too + * *wide* fails silently in the opposite way — it refuses public space, and the + * only symptom is a legitimate fetch being rejected with the same uniform + * message every other rejection uses. Neither edge of either mask is pinned + * today, so widening one would break no test. + * + * Each range therefore gets both edges plus a just-outside neighbour. + */ +describe('assertPublicHttpUrl IPv6 range boundaries', () => { + const rejectionOf = async (url: string): Promise => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + await assertPublicHttpUrl(url); + } catch (err) { + return err as Error; + } + throw new Error(`${url} was expected to be rejected, but was allowed`); + }; + + describe('fec0::/10 site-local', () => { + it.each([ + ['fec0::', 'first address in the range'], + ['feff:ffff:ffff:ffff:ffff:ffff:ffff:ffff', 'last address in the range'], + ])('rejects [%s] (%s)', async (address) => { + const err = await rejectionOf(`http://[${address}]/`); + + expect(err.message).toBe('Blocked private IP literal'); + }); + + /** + * `fe00::1` sits below `fec0::/10` and is not caught by `fe80::/10` either, + * so it lands in the gap between the two masks. If the site-local check were + * widened to `fe00::/8` — an easy slip, since both start `fe` — this is the + * address that would start being refused. + */ + it('still allows [fe00::1], which is below the range', async () => { + const url = await assertPublicHttpUrl('http://[fe00::1]/'); + + expect(url.hostname).toBe('[fe00::1]'); + expect(dns.lookup).not.toHaveBeenCalled(); + }); + }); + + describe('100::/64 discard', () => { + it('rejects [100::] (first address in the range)', async () => { + const err = await rejectionOf('http://[100::]/'); + + expect(err.message).toBe('Blocked private IP literal'); + }); + + /** + * The discard prefix is a /64, not a /16 or /32. Both of these share the + * leading `100` hextet and would be refused if the check tested only `h[0]`. + */ + it.each([ + ['100:0:0:1::1', 'inside 100::/16 but outside the /64'], + ['101::1', 'adjacent prefix'], + ])('still allows [%s] (%s)', async (address) => { + const url = await assertPublicHttpUrl(`http://[${address}]/`); + + expect(url.hostname).toBe(`[${address}]`); + }); + }); + + /** + * `ipv6ToHextets` folds a trailing dotted quad into two hextets before any + * range check runs, so the dotted spelling of a NAT64 address has to reach the + * same verdict as the hex one. `ssrf-guard.test.ts` covers + * `64:ff9b::a9fe:a9fe`; this is the same destination written the other way. + */ + it('rejects the dotted-quad spelling of a NAT64 address', async () => { + const err = await rejectionOf('http://[64:ff9b::169.254.169.254]/'); + + expect(err.message).toBe('Blocked private IP literal'); + }); + + /** + * The same boundaries via DNS answers rather than URL literals. The two paths + * reach `ipIsPrivate` through different branches of `assertPublicHttpUrl` and + * report different messages, so a regression could land on one and not the + * other. + */ + describe('applied to resolved addresses', () => { + it('rejects a site-local answer at the top of the range', async () => { + vi.mocked(dns.lookup).mockResolvedValue([{ address: 'feff::1', family: 6 }] as never); + vi.spyOn(console, 'error').mockImplementation(() => {}); + + await expect(assertPublicHttpUrl('https://edge.example/a.mp3')).rejects.toThrow( + 'Host does not resolve to a public address', + ); + }); + + it('still allows an answer just outside the discard prefix', async () => { + vi.mocked(dns.lookup).mockResolvedValue([{ address: '101::1', family: 6 }] as never); + + const url = await assertPublicHttpUrl('https://edgeok.example/a.mp3'); + + expect(url.hostname).toBe('edgeok.example'); + }); + }); +}); From 75a901d528649a6d4dba23e159d5aa59b510f8ab Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 22:03:11 +0000 Subject: [PATCH 2/2] test(web): address CodeRabbit review on the SSRF boundary tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both findings were valid. 1. `err as Error` in the `rejectionOf` helper was an unsafe assertion on a value `catch` binds as `unknown`, which `.cursorrules` forbids outright. Narrow with `instanceof` and rethrow anything else, so a non-Error throw surfaces as itself rather than being mistyped and failing later on a missing `.message`. 2. The `100::/64` range was pinned only at its first address, `100::`. That is an inconsistency in this file's own premise: `fec0::/10` gets both edges, and the whole point of the file is that one edge does not hold a range. A regression blocking only the first address would have passed. Add `100:0:0:0:ffff:ffff:ffff:ffff`. The second finding is confirmed by mutation rather than taken on faith — replacing the range check with an exact match on `100::` alone: h[0] === 0x0100 && h[1..3] === 0 -> h.join(':') === '256:0:0:0:0:0:0:0' fails: rejects [100:0:0:0:ffff:ffff:ffff:ffff] (last address in the range) That mutant survived the previous version of this file, so the gap was real. The two mutants already covered still fail exactly one test each. Full apps/web suite 59 files / 355 passed; tsc and eslint clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NNXdtee4Wwwjew74cNZ1C5 --- .../__tests__/ssrf-guard-range-boundaries.test.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/apps/web/src/lib/__tests__/ssrf-guard-range-boundaries.test.ts b/apps/web/src/lib/__tests__/ssrf-guard-range-boundaries.test.ts index 176f4b62a..03d1a6570 100644 --- a/apps/web/src/lib/__tests__/ssrf-guard-range-boundaries.test.ts +++ b/apps/web/src/lib/__tests__/ssrf-guard-range-boundaries.test.ts @@ -35,7 +35,11 @@ describe('assertPublicHttpUrl IPv6 range boundaries', () => { try { await assertPublicHttpUrl(url); } catch (err) { - return err as Error; + // `catch` binds as `unknown`; narrow rather than assert, so a non-Error + // throw surfaces as itself instead of being mistyped as an Error and + // failing later on a missing `.message`. + if (err instanceof Error) return err; + throw err; } throw new Error(`${url} was expected to be rejected, but was allowed`); }; @@ -65,8 +69,11 @@ describe('assertPublicHttpUrl IPv6 range boundaries', () => { }); describe('100::/64 discard', () => { - it('rejects [100::] (first address in the range)', async () => { - const err = await rejectionOf('http://[100::]/'); + it.each([ + ['100::', 'first address in the range'], + ['100:0:0:0:ffff:ffff:ffff:ffff', 'last address in the range'], + ])('rejects [%s] (%s)', async (address) => { + const err = await rejectionOf(`http://[${address}]/`); expect(err.message).toBe('Blocked private IP literal'); });