diff --git a/docs/standards/security.md b/docs/standards/security.md index b9d32dcad..d9ad20a18 100644 --- a/docs/standards/security.md +++ b/docs/standards/security.md @@ -25,6 +25,7 @@ These were raised in review, judged valid, and accepted on purpose. Changing the - The production canary account's credentials reach `prod-canary.yml` through Actions secrets, approved 2026-09-24 and recorded in AGENTS.md ([#68](https://github.com/Programcode/DebateLab/pull/68#discussion_r4090664336)). - Manual canary runs can exhaust the account's daily quota; that shows as a quota error, not a false pass ([#68](https://github.com/Programcode/DebateLab/pull/68#discussion_r4093482054)). - CodeRabbit rated the shared preview-origin allowlist from PR #95 as a high retained concern: an approved production preview runs unmerged code that can receive demo-session handoffs ([#95 security review](https://github.com/Programcode/DebateLab/pull/95)). [docs/operations.md](../operations.md#per-pr-previews) describes the current setup. This one has no recorded decision yet. +- `import-source` checks every resolved A and AAAA address against the IANA special-purpose ranges before each fetch, but the Edge runtime cannot pin the connection to the checked address. A hostname whose DNS answer changes between the check and the connection can still reach a blocked range. Citation verification avoids the same gap by not fetching at all (`verifyUrl` in [`supabase/functions/_shared/research.ts`](../../supabase/functions/_shared/research.ts)). Closing it needs an egress proxy outside the runtime. Raised as finding [7] of the 2026-08-19 security review; this needs Adam's decision. ## Telemetry and error reports diff --git a/src/test/security-hardening.test.ts b/src/test/security-hardening.test.ts index 67085015a..e960eda45 100644 --- a/src/test/security-hardening.test.ts +++ b/src/test/security-hardening.test.ts @@ -69,11 +69,9 @@ describe('security hardening regressions', () => { expect(promo).toContain('userClient.rpc("redeem_promo_code_atomic", { p_code: code })'); }); - it('resolves DNS and bounds importer bodies before processing', () => { - expect(importer).toContain('validateResolvedPublicHost(currentUrl)'); - expect(importer).toContain('readBoundedText(response)'); - expect(read('supabase/functions/import-source/resource-limits.ts')).toContain('MAX_RESPONSE_BYTES'); - }); + // Importer DNS checks, body caps and the deadline are covered by behavior in + // import-source/url-validation.test.ts, import-source/deadline_test.ts and + // _shared/bounded-body.test.ts. it('uses allowlisted model/provider pairs and SSRF-safe citation verification', () => { expect(modelCatalog).toContain('id: "z-ai/glm-5.3-flash"'); diff --git a/supabase/README.md b/supabase/README.md index 16ec20ce6..8f40f2d6d 100644 --- a/supabase/README.md +++ b/supabase/README.md @@ -42,8 +42,8 @@ The browser boundary is [`../src/lib/ai/client.ts`](../src/lib/ai/client.ts). Sh ### Content and sharing -- `import-source`: authenticated public-URL import with SSRF protection on redirects, timeout, content-type and size limits, readable-text extraction, quota checks, and logging. -- `share`: authenticated creation of expiring share codes for supported saved content. +- `import-source`: authenticated public-URL import with SSRF protection (resolved addresses checked against the IANA special-purpose ranges on every redirect), one 10-second deadline covering DNS, redirects and the body, content-type and 1 MB size limits, readable-text extraction, a daily quota, and logging. +- `share`: authenticated creation of share codes for supported saved content, with a 512 KB payload cap, expiry from 1 to 90 days (30 when omitted), and a daily quota of 50 per user. - `workspace-invites`: part of the hidden Workspace. Authenticated workspace code and email invitation creation, rotation, revocation, and redemption. The handler keeps raw invite values and email addresses out of database calls and uses service-role-only transactional RPCs for limits and membership changes. - `export-gdoc`: authenticated Google Docs export using a short-lived Google access token. It rejects refresh tokens. diff --git a/supabase/functions/_shared/bounded-body.test.ts b/supabase/functions/_shared/bounded-body.test.ts new file mode 100644 index 000000000..2fd69120c --- /dev/null +++ b/supabase/functions/_shared/bounded-body.test.ts @@ -0,0 +1,26 @@ +import { assertEquals, assertRejects } from "jsr:@std/assert@1.0.19"; +import { BodyTooLargeError, readBoundedBody } from "./bounded-body.ts"; + +// A streamed body with no Content-Length, sent in 1000-byte chunks. +function streamOf(bytes: number, headers: HeadersInit = {}) { + let sent = 0; + const body = new ReadableStream({ + pull(controller) { + if (sent >= bytes) return controller.close(); + const size = Math.min(1000, bytes - sent); + sent += size; + controller.enqueue(new Uint8Array(size).fill(0x61)); + }, + }); + return new Response(body, { headers }); +} + +Deno.test("readBoundedBody accepts a body of exactly the cap and rejects one byte more", async () => { + assertEquals((await readBoundedBody(streamOf(4096), 4096)).length, 4096); + await assertRejects(() => readBoundedBody(streamOf(4097), 4096), BodyTooLargeError); +}); + +Deno.test("readBoundedBody counts bytes received, not the declared Content-Length", async () => { + await assertRejects(() => readBoundedBody(streamOf(8000, { "content-length": "10" }), 4096), BodyTooLargeError); + await assertRejects(() => readBoundedBody(new Response("tiny", { headers: { "content-length": "4097" } }), 4096), BodyTooLargeError); +}); diff --git a/supabase/functions/_shared/bounded-body.ts b/supabase/functions/_shared/bounded-body.ts new file mode 100644 index 000000000..3ab787567 --- /dev/null +++ b/supabase/functions/_shared/bounded-body.ts @@ -0,0 +1,51 @@ +// Reads untrusted request and response bodies with a byte cap and, optionally, +// a deadline. A slow sender is cut off when the signal aborts, not when the +// platform's wall-clock limit ends the invocation. + +export class BodyTooLargeError extends Error { + constructor(readonly maxBytes: number) { + super(`Body exceeds ${maxBytes} bytes.`); + } +} + +/** Settles with `promise`, or rejects with the signal's reason once it aborts. */ +export function untilAborted(promise: Promise, signal?: AbortSignal): Promise { + if (!signal) return promise; + if (signal.aborted) return Promise.reject(signal.reason); + return new Promise((resolve, reject) => { + const onAbort = () => reject(signal.reason); + signal.addEventListener("abort", onAbort, { once: true }); + promise.then(resolve, reject).finally(() => signal.removeEventListener("abort", onAbort)); + }); +} + +export async function readBoundedBody( + source: Request | Response, + maxBytes: number, + signal?: AbortSignal, +): Promise { + const declaredLength = Number(source.headers.get("content-length")); + if (Number.isFinite(declaredLength) && declaredLength > maxBytes) { + await source.body?.cancel().catch(() => undefined); + throw new BodyTooLargeError(maxBytes); + } + if (!source.body) return ""; + const reader = source.body.getReader(); + const decoder = new TextDecoder(); + let received = 0; + let text = ""; + try { + while (true) { + // A pending read is not guaranteed to reject when the fetch signal + // aborts, so race each read against the signal directly. + const { done, value } = await untilAborted(reader.read(), signal); + if (done) break; + received += value.byteLength; + if (received > maxBytes) throw new BodyTooLargeError(maxBytes); + text += decoder.decode(value, { stream: true }); + } + return text + decoder.decode(); + } finally { + reader.cancel().catch(() => undefined); + } +} diff --git a/supabase/functions/_shared/url-validation.ts b/supabase/functions/_shared/url-validation.ts index ab76b9e08..6265bd777 100644 --- a/supabase/functions/_shared/url-validation.ts +++ b/supabase/functions/_shared/url-validation.ts @@ -1,42 +1,113 @@ -// SSRF-safe URL validation for outbound fetches to AI/search-supplied URLs -// (citation verification, content fetching). Rejects loopback, link-local, -// RFC1918, and IPv4-mapped-IPv6 private targets by literal hostname. +// SSRF-safe URL validation for outbound fetches to user- or AI-supplied URLs. +// Rejects every IPv4 and IPv6 range in the IANA special-purpose registries that +// is not globally reachable, plus multicast, reserved and NAT64 space. Whole +// blocks are rejected even where the registry marks a few anycast or ORCHID +// addresses inside them as global (192.0.0.9, 2001:3::/32 and the like): none +// of them serve web pages, so failing closed costs nothing. -export function isPrivateIpv4(hostname: string) { +type Cidr = readonly [bytes: readonly number[], prefixLength: number]; + +// https://www.iana.org/assignments/iana-ipv4-special-registry +const BLOCKED_IPV4: readonly Cidr[] = [ + [[0, 0, 0, 0], 8], // "this network" + [[10, 0, 0, 0], 8], // private-use + [[100, 64, 0, 0], 10], // shared address space (CGNAT) + [[127, 0, 0, 0], 8], // loopback + [[169, 254, 0, 0], 16], // link-local, including cloud metadata + [[172, 16, 0, 0], 12], // private-use + [[192, 0, 0, 0], 24], // IETF protocol assignments + [[192, 0, 2, 0], 24], // documentation (TEST-NET-1) + [[192, 88, 99, 0], 24], // deprecated 6to4 relay anycast + [[192, 168, 0, 0], 16], // private-use + [[198, 18, 0, 0], 15], // benchmarking + [[198, 51, 100, 0], 24], // documentation (TEST-NET-2) + [[203, 0, 113, 0], 24], // documentation (TEST-NET-3) + [[224, 0, 0, 0], 4], // multicast + [[240, 0, 0, 0], 4], // reserved, including limited broadcast +]; + +// ::ffff:0:0/96 carries an IPv4 address, which is checked against the IPv4 list. +const IPV4_MAPPED: Cidr = [[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff], 96]; + +// https://www.iana.org/assignments/iana-ipv6-special-registry +const BLOCKED_IPV6: readonly Cidr[] = [ + [[], 96], // ::/96: unspecified, loopback and deprecated IPv4-compatible + [[0, 0x64, 0xff, 0x9b], 96], // NAT64 well-known prefix + [[0, 0x64, 0xff, 0x9b, 0, 1], 48], // local-use NAT64 + [[0x01, 0x00], 64], // discard-only + [[0x01, 0x00, 0, 0, 0, 0, 0, 0x01], 64], // dummy prefix + [[0x20, 0x01, 0x00, 0x00], 23], // IETF protocol assignments, including Teredo + [[0x20, 0x01, 0x0d, 0xb8], 32], // documentation + [[0x20, 0x02], 16], // 6to4 + [[0x3f, 0xff], 20], // documentation + [[0x5f, 0x00], 16], // segment routing (SRv6) SIDs + [[0xfc], 7], // unique local + [[0xfe, 0x80], 10], // link-local + [[0xfe, 0xc0], 10], // deprecated site-local + [[0xff], 8], // multicast +]; + +function inCidr(address: readonly number[], [network, prefixLength]: Cidr): boolean { + for (let bit = 0; bit < prefixLength; bit += 8) { + const index = bit / 8; + const bits = Math.min(8, prefixLength - bit); + const mask = (0xff << (8 - bits)) & 0xff; + if (((address[index] ?? 0) & mask) !== ((network[index] ?? 0) & mask)) return false; + } + return true; +} + +function parseIpv4(hostname: string): number[] | null { // Canonicalize numeric/octal/hex/short IPv4 forms (e.g. "127.1", "0x7f000001", // "2130706433") to dotted-decimal via the same WHATWG host parser `new URL()` - // uses, so an obfuscated loopback/private address can't slip past the range - // checks below just because it wasn't written as four decimal octets. + // uses, so an obfuscated address can't slip past the range checks. let canonical = hostname; try { canonical = new URL(`http://${hostname}`).hostname; } catch { - // Not parseable as a host on its own; fall through and let the range - // checks below reject it via the length/range guard. + // Not parseable as a host on its own; the octet check below rejects it. } const parts = canonical.split(".").map((part) => Number(part)); - if (parts.length !== 4 || parts.some((part) => !Number.isInteger(part) || part < 0 || part > 255)) return false; - const [a, b] = parts; - return ( - a === 10 || - a === 127 || - a === 0 || - (a === 100 && b >= 64 && b <= 127) || - (a === 169 && b === 254) || - (a === 172 && b >= 16 && b <= 31) || - (a === 192 && b === 168) - ); + if (parts.length !== 4 || parts.some((part) => !Number.isInteger(part) || part < 0 || part > 255)) return null; + return parts; +} + +function parseIpv6(hostname: string): number[] | null { + const literal = hostname.replace(/^\[|\]$/g, "").replace(/%.*$/, ""); + if (!literal.includes(":")) return null; + let canonical: string; + try { + // The WHATWG parser validates the literal and rewrites any embedded + // dotted IPv4 tail as two hex groups. + canonical = new URL(`http://[${literal}]`).hostname.slice(1, -1); + } catch { + return null; + } + const [head, tail = ""] = canonical.split("::"); + const headGroups = head ? head.split(":") : []; + const tailGroups = tail ? tail.split(":") : []; + const groups = canonical.includes("::") + ? [...headGroups, ...Array(8 - headGroups.length - tailGroups.length).fill("0"), ...tailGroups] + : headGroups; + if (groups.length !== 8) return null; + return groups.flatMap((group) => { + const value = parseInt(group, 16); + return [value >> 8, value & 0xff]; + }); +} + +export function isPrivateIpv4(hostname: string) { + const address = parseIpv4(hostname); + return address !== null && BLOCKED_IPV4.some((cidr) => inCidr(address, cidr)); } export function isPrivateIpv6(hostname: string) { - const h = hostname.toLowerCase().replace(/%.*$/, ""); - if (!h.includes(":")) return false; - if (h === "::1" || h === "::" || h === "0:0:0:0:0:0:0:1" || h === "0:0:0:0:0:0:0:0") return true; - if (/^f[cd][0-9a-f]{2}:/.test(h)) return true; - if (/^fe[89ab][0-9a-f]:/.test(h)) return true; - const v4MappedMatch = h.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/); - if (v4MappedMatch && isPrivateIpv4(v4MappedMatch[1])) return true; - return false; + const address = parseIpv6(hostname.toLowerCase()); + if (address === null) return false; + if (inCidr(address, IPV4_MAPPED)) { + return isPrivateIpv4(address.slice(12).join(".")); + } + return BLOCKED_IPV6.some((cidr) => inCidr(address, cidr)); } export function validatePublicHttpUrl(url: string): URL { diff --git a/supabase/functions/import-source/deadline_test.ts b/supabase/functions/import-source/deadline_test.ts new file mode 100644 index 000000000..501b61fdf --- /dev/null +++ b/supabase/functions/import-source/deadline_test.ts @@ -0,0 +1,133 @@ +import { FakeTime } from "jsr:@std/testing@1.0.17/time"; + +for (const [key, value] of Object.entries({ + SUPABASE_URL: "https://supabase.test", + SUPABASE_ANON_KEY: "test-anon-key", + SUPABASE_SERVICE_ROLE_KEY: "test-service-role-key", +})) Deno.env.set(key, value); + +const { handleRequest } = await import("./index.ts"); + +// A body that sends one small chunk every 500 ms forever, or never sends at all. +function slowBody(mode: "drip" | "stall", onCancel: () => void) { + let timer: ReturnType | undefined; + return new ReadableStream({ + pull(controller) { + if (mode === "stall") return new Promise(() => {}); + return new Promise((resolve) => { + timer = setTimeout(() => { + controller.enqueue(new TextEncoder().encode("

slow words arriving

")); + resolve(); + }, 500); + }); + }, + cancel() { + clearTimeout(timer); + onCancel(); + }, + }); +} + +async function importAgainstSlowServer(mode: "drip" | "stall") { + const time = new FakeTime(); + const originalFetch = globalThis.fetch; + const originalResolveDns = Deno.resolveDns; + let bodyCancelled = false; + + (Deno as unknown as { resolveDns: unknown }).resolveDns = (_host: string, type: string) => Promise.resolve(type === "A" ? ["93.184.216.34"] : []); + globalThis.fetch = (async (input: Request | URL | string) => { + const url = new URL(input instanceof Request ? input.url : String(input)); + if (url.origin === "https://slow.example") { + return new Response(slowBody(mode, () => (bodyCancelled = true)), { headers: { "content-type": "text/html" } }); + } + if (url.pathname === "/auth/v1/user") return Response.json({ id: "user-1", aud: "authenticated" }); + if (url.pathname.includes("/rest/v1/user_roles")) return Response.json({ role: "user" }); + if (url.pathname.includes("/rest/v1/rpc/consume_daily_quota")) return Response.json([{ allowed: true, current_count: 1 }]); + if (url.pathname.includes("/rest/v1/ai_requests")) return new Response(null, { status: 201 }); + throw new Error(`Unexpected request: ${url}`); + }) as typeof fetch; + + try { + let settled: Response | undefined; + const pending = handleRequest( + new Request("http://127.0.0.1/functions/v1/import-source", { + method: "POST", + headers: { Authorization: "Bearer test-user-token", "Content-Type": "application/json" }, + body: JSON.stringify({ url: "https://slow.example/article" }), + }), + ).then((response) => (settled = response)); + + // Past the 10 s deadline, plus a margin, in 250 ms steps so drips interleave. + for (let elapsed = 0; elapsed < 11_000 && !settled; elapsed += 250) { + await time.tickAsync(250); + } + if (!settled) throw new Error(`import-source was still reading the ${mode} body after 11 s`); + await pending; + + const body = await settled.json(); + if (settled.status !== 504) throw new Error(`expected 504, got ${settled.status}: ${JSON.stringify(body)}`); + if (!bodyCancelled) throw new Error("the upstream body was not cancelled at the deadline"); + } finally { + globalThis.fetch = originalFetch; + (Deno as unknown as { resolveDns: unknown }).resolveDns = originalResolveDns; + time.restore(); + } +} + +Deno.test("import-source stops reading a body that drips slower than the deadline", async () => { + await importAgainstSlowServer("drip"); +}); + +Deno.test("import-source stops waiting for a body that never arrives", async () => { + await importAgainstSlowServer("stall"); +}); + +Deno.test("import-source stops waiting for a request body that never finishes", async () => { + const time = new FakeTime(); + const originalFetch = globalThis.fetch; + const sourceRequests: string[] = []; + globalThis.fetch = (async (input: Request | URL | string) => { + const url = new URL(input instanceof Request ? input.url : String(input)); + if (url.pathname === "/auth/v1/user") return Response.json({ id: "user-1", aud: "authenticated" }); + if (url.pathname.includes("/rest/v1/user_roles")) return Response.json({ role: "user" }); + if (url.pathname.includes("/rest/v1/rpc/consume_daily_quota")) return Response.json([{ allowed: true, current_count: 1 }]); + if (url.pathname.includes("/rest/v1/ai_requests")) return new Response(null, { status: 201 }); + sourceRequests.push(url.toString()); + throw new Error(`Unexpected request: ${url}`); + }) as typeof fetch; + + let requestCancelled = false; + const stalledUpload = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('{"url": "https://exam')); + }, + pull: () => new Promise(() => {}), + cancel() { + requestCancelled = true; + }, + }); + + try { + let settled: Response | undefined; + const pending = handleRequest(new Request("http://127.0.0.1/functions/v1/import-source", { + method: "POST", + headers: { Authorization: "Bearer test-user-token", "Content-Type": "application/json" }, + body: stalledUpload, + duplex: "half", + } as RequestInit)).then((response) => (settled = response)); + + for (let elapsed = 0; elapsed < 11_000 && !settled; elapsed += 250) { + await time.tickAsync(250); + } + if (!settled) throw new Error("import-source was still reading the request body after 11 s"); + await pending; + + const body = await settled.json(); + if (settled.status !== 408) throw new Error(`expected 408, got ${settled.status}: ${JSON.stringify(body)}`); + if (!requestCancelled) throw new Error("the request body was not cancelled at the deadline"); + if (sourceRequests.length) throw new Error(`a source was fetched: ${sourceRequests}`); + } finally { + globalThis.fetch = originalFetch; + time.restore(); + } +}); diff --git a/supabase/functions/import-source/index.ts b/supabase/functions/import-source/index.ts index d2d64090c..d4996834b 100644 --- a/supabase/functions/import-source/index.ts +++ b/supabase/functions/import-source/index.ts @@ -1,6 +1,7 @@ import { createClient, type User } from "https://esm.sh/@supabase/supabase-js@2"; import { isPrivateIpv4, isPrivateIpv6, validatePublicHttpUrl, validateResolvedPublicHost } from "./url-validation.ts"; import { readBoundedText } from "./resource-limits.ts"; +import { BodyTooLargeError, readBoundedBody, untilAborted } from "../_shared/bounded-body.ts"; import { HttpError, jsonError, requireUser } from "../_shared/auth.ts"; import { getCorsHeaders } from "../_shared/cors.ts"; @@ -9,8 +10,10 @@ export { isPrivateIpv4, isPrivateIpv6, validatePublicHttpUrl }; const MAX_CONTENT_CHARS = 50_000; +// Total time for the request body, DNS, redirects and the whole source body. const FETCH_TIMEOUT_MS = 10_000; const MAX_REDIRECTS = 5; +const MAX_REQUEST_BYTES = 16_384; const DAILY_IMPORT_LIMIT = 50; @@ -58,42 +61,45 @@ async function logImport(userId: string, status: "success" | "error", errorCode? } } -async function fetchWithTimeout(initialUrl: URL) { - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS); - try { - let currentUrl = initialUrl; - for (let hop = 0; hop <= MAX_REDIRECTS; hop += 1) { - await validateResolvedPublicHost(currentUrl); - const response = await fetch(currentUrl.toString(), { - headers: { - "Accept": "text/html, text/plain;q=0.9, application/xhtml+xml;q=0.8", - "User-Agent": "DebateLabSourceImporter/1.0", - }, - signal: controller.signal, - redirect: "manual", - }); - - // Manual redirect handling so we can re-validate against SSRF on every hop. - if (response.status >= 300 && response.status < 400) { - const location = response.headers.get("location"); - if (!location) { - throw new Error("Redirect without Location header."); - } - if (hop === MAX_REDIRECTS) { - throw new Error("Too many redirects."); - } - const nextUrl = new URL(location, currentUrl); - currentUrl = validatePublicHttpUrl(nextUrl.toString()); - continue; - } +class SourceTimeoutError extends Error { + constructor() { + super("The source took too long to respond."); + } +} - return { response, finalUrl: currentUrl }; +// The caller's signal is the import's one deadline: it covers DNS, every +// redirect hop and the body read that follows. +async function fetchWithRedirects(initialUrl: URL, signal: AbortSignal) { + let currentUrl = initialUrl; + for (let hop = 0; hop <= MAX_REDIRECTS; hop += 1) { + await validateResolvedPublicHost(currentUrl, signal); + const response = await untilAborted(fetch(currentUrl.toString(), { + headers: { + "Accept": "text/html, text/plain;q=0.9, application/xhtml+xml;q=0.8", + "User-Agent": "DebateLabSourceImporter/1.0", + }, + signal, + redirect: "manual", + }), signal); + + // Manual redirect handling so we can re-validate against SSRF on every hop. + if (response.status >= 300 && response.status < 400) { + await response.body?.cancel().catch(() => undefined); + const location = response.headers.get("location"); + if (!location) { + throw new Error("Redirect without Location header."); + } + if (hop === MAX_REDIRECTS) { + throw new Error("Too many redirects."); + } + const nextUrl = new URL(location, currentUrl); + currentUrl = validatePublicHttpUrl(nextUrl.toString()); + continue; } - throw new Error("Too many redirects."); - } finally { - clearTimeout(timeout); + + return { response, finalUrl: currentUrl }; } + throw new Error("Too many redirects."); } function decodeEntities(text: string) { @@ -266,27 +272,44 @@ export async function handleRequest(req: Request): Promise { } const startedAt = Date.now(); + const deadline = new AbortController(); + const deadlineTimer = setTimeout(() => deadline.abort(new SourceTimeoutError()), FETCH_TIMEOUT_MS); try { - const body = await req.json().catch(() => null); + let body: { url?: unknown } | null = null; + try { + body = JSON.parse(await readBoundedBody(req, MAX_REQUEST_BYTES, deadline.signal)); + } catch (error) { + if (deadline.signal.aborted) { + await logImport(user.id, "error", "request_timeout", Date.now() - startedAt); + return jsonResponse({ error: "The request took too long to arrive." }, 408, corsHeaders); + } + if (error instanceof BodyTooLargeError) { + await logImport(user.id, "error", "bad_request", Date.now() - startedAt); + return jsonResponse({ error: "Request body is too large." }, 413, corsHeaders); + } + } if (!body || typeof body !== "object") { await logImport(user.id, "error", "bad_request", Date.now() - startedAt); return jsonResponse({ error: "Invalid JSON body." }, 400, corsHeaders); } const parsedUrl = validatePublicHttpUrl(String(body.url || "").trim()); - const { response, finalUrl } = await fetchWithTimeout(parsedUrl); + const { response, finalUrl } = await fetchWithRedirects(parsedUrl, deadline.signal); if (!response.ok) { + await response.body?.cancel().catch(() => undefined); await logImport(user.id, "error", `upstream_${response.status}`, Date.now() - startedAt); return jsonResponse({ error: `Could not fetch source (${response.status}).` }, 502, corsHeaders); } const contentType = response.headers.get("content-type") || ""; if (!contentType.includes("text/html") && !contentType.includes("text/plain") && !contentType.includes("application/xhtml+xml")) { + await response.body?.cancel().catch(() => undefined); await logImport(user.id, "error", "unsupported_type", Date.now() - startedAt); return jsonResponse({ error: "Only HTML and plain text pages can be imported from a URL." }, 415, corsHeaders); } - const raw = await readBoundedText(response); + const raw = await readBoundedText(response, deadline.signal); + clearTimeout(deadlineTimer); const isHtml = contentType.includes("html") || /]/i.test(raw); const content = normalizeText(isHtml ? stripHtml(raw) : raw); @@ -312,9 +335,15 @@ export async function handleRequest(req: Request): Promise { url: finalUrl.toString(), }, 200, corsHeaders); } catch (error) { + if (deadline.signal.aborted) { + await logImport(user.id, "error", "timeout", Date.now() - startedAt); + return jsonResponse({ error: new SourceTimeoutError().message }, 504, corsHeaders); + } const message = error instanceof Error ? error.message : "Failed to import source."; await logImport(user.id, "error", "exception", Date.now() - startedAt); return jsonResponse({ error: message }, 400, corsHeaders); + } finally { + clearTimeout(deadlineTimer); } } diff --git a/supabase/functions/import-source/resource-limits.ts b/supabase/functions/import-source/resource-limits.ts index cdf381e22..82ca89734 100644 --- a/supabase/functions/import-source/resource-limits.ts +++ b/supabase/functions/import-source/resource-limits.ts @@ -1,25 +1,12 @@ +import { BodyTooLargeError, readBoundedBody } from "../_shared/bounded-body.ts"; + export const MAX_RESPONSE_BYTES = 1_000_000; -export async function readBoundedText(response: Response, maxBytes = MAX_RESPONSE_BYTES): Promise { - const declaredLength = Number(response.headers.get("content-length")); - if (Number.isFinite(declaredLength) && declaredLength > maxBytes) { - throw new Error("Source response exceeds the 1 MB import limit."); - } - if (!response.body) return ""; - const reader = response.body.getReader(); - const decoder = new TextDecoder(); - let received = 0; - let text = ""; +export async function readBoundedText(response: Response, signal?: AbortSignal, maxBytes = MAX_RESPONSE_BYTES): Promise { try { - while (true) { - const { done, value } = await reader.read(); - if (done) break; - received += value.byteLength; - if (received > maxBytes) throw new Error("Source response exceeds the 1 MB import limit."); - text += decoder.decode(value, { stream: true }); - } - return text + decoder.decode(); - } finally { - reader.cancel().catch(() => undefined); + return await readBoundedBody(response, maxBytes, signal); + } catch (error) { + if (error instanceof BodyTooLargeError) throw new Error("Source response exceeds the 1 MB import limit."); + throw error; } } diff --git a/supabase/functions/import-source/url-validation.test.ts b/supabase/functions/import-source/url-validation.test.ts index 13b6a774f..014c65921 100644 --- a/supabase/functions/import-source/url-validation.test.ts +++ b/supabase/functions/import-source/url-validation.test.ts @@ -1,4 +1,4 @@ -import { isPrivateIpv4, isPrivateIpv6, validatePublicHttpUrl } from "./url-validation.ts"; +import { isPrivateIpv4, isPrivateIpv6, validatePublicHttpUrl, validateResolvedPublicHost } from "./url-validation.ts"; function assert(condition: boolean, message: string) { if (!condition) throw new Error(message); @@ -58,6 +58,89 @@ Deno.test("isPrivateIpv6 catches loopback, ULA, link-local, IPv4-mapped private" assert(!isPrivateIpv6("not-ipv6"), "non-ipv6 hostname should not match"); }); +Deno.test("isPrivateIpv4 rejects every IANA special-purpose range that is not globally reachable", () => { + for (const address of [ + "192.0.0.1", "192.0.0.255", // IETF protocol assignments 192.0.0.0/24 + "192.0.2.10", "198.51.100.7", "203.0.113.99", // documentation (TEST-NET-1/2/3) + "192.88.99.1", // deprecated 6to4 relay anycast + "198.18.0.1", "198.19.255.254", // benchmarking 198.18.0.0/15 + "224.0.0.1", "239.255.255.250", // multicast 224.0.0.0/4 + "240.0.0.1", "255.255.255.255", // reserved 240.0.0.0/4 and limited broadcast + "0.1.2.3", // "this network" 0.0.0.0/8 + ]) { + assert(isPrivateIpv4(address), `${address} should be blocked`); + } + for (const address of ["192.0.1.1", "198.17.255.255", "198.20.0.1", "223.255.255.255", "93.184.216.34"]) { + assert(!isPrivateIpv4(address), `${address} is public and should not be blocked`); + } +}); + +Deno.test("isPrivateIpv6 rejects every IANA special-purpose range that is not globally reachable", () => { + for (const address of [ + "::", "::1", "0:0:0:0:0:0:0:1", + "::a00:1", // IPv4-compatible (deprecated) ::/96 + "::ffff:a00:1", "::ffff:7f00:1", // IPv4-mapped private, in hex form + "64:ff9b::c000:201", "64:ff9b::808:808", // NAT64 well-known prefix 64:ff9b::/96 + "64:ff9b:1::1", // local-use NAT64 64:ff9b:1::/48 + "100::1", // discard-only 100::/64 + "100:0:0:1::1", // dummy prefix 100:0:0:1::/64 + "2001::1", "2001:1ff::1", // IETF protocol assignments 2001::/23 (Teredo included) + "2001:db8::1", // documentation 2001:db8::/32 + "2002:c0a8:101::1", // 6to4 2002::/16 + "3fff::1", // documentation 3fff::/20 + "5f00::1", // SRv6 SIDs 5f00::/16 + "fec0::1", // deprecated site-local fec0::/10 + "ff02::1", "ff0e::1", // multicast ff00::/8 + "FE80::1%eth0", // zone id and upper case + "[fe80::1]", // bracketed literal + ]) { + assert(isPrivateIpv6(address), `${address} should be blocked`); + } + for (const address of ["2606:4700:4700::1111", "2001:200::1", "2a00:1450:4001:80b::200e", "::ffff:5db8:d822"]) { + assert(!isPrivateIpv6(address), `${address} is public and should not be blocked`); + } +}); + +async function withResolvedAddresses(records: Partial>, run: () => Promise) { + const original = Deno.resolveDns; + (Deno as unknown as { resolveDns: unknown }).resolveDns = (_host: string, type: "A" | "AAAA") => Promise.resolve(records[type] ?? []); + try { + await run(); + } finally { + (Deno as unknown as { resolveDns: unknown }).resolveDns = original; + } +} + +async function assertRejects(promise: Promise, expectedMessageFragment: string) { + try { + await promise; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (!message.includes(expectedMessageFragment)) { + throw new Error(`Expected error containing "${expectedMessageFragment}", got "${message}"`); + } + return; + } + throw new Error(`Expected rejection containing "${expectedMessageFragment}"`); +} + +Deno.test("validateResolvedPublicHost rejects a public name that resolves into a reserved range", async () => { + const url = new URL("https://reserved.example/"); + for (const records of [ + { A: ["198.18.0.1"] }, + { A: ["93.184.216.34", "224.0.0.251"] }, + { AAAA: ["64:ff9b::a9fe:a9fe"] }, + { AAAA: ["2001:db8::1"] }, + ]) { + await withResolvedAddresses(records, () => assertRejects(validateResolvedPublicHost(url), "Private and local")); + } +}); + +Deno.test("validateResolvedPublicHost accepts a name that resolves only to public addresses", async () => { + await withResolvedAddresses({ A: ["93.184.216.34"], AAAA: ["2606:2800:21f:cb07:6820:80da:af6b:8b2c"] }, () => + validateResolvedPublicHost(new URL("https://example.com/"))); +}); + Deno.test("validatePublicHttpUrl accepts plain public https URLs", () => { const parsed = validatePublicHttpUrl("https://example.com/article#frag"); assert(parsed.hostname === "example.com", `expected example.com, got ${parsed.hostname}`); diff --git a/supabase/functions/import-source/url-validation.ts b/supabase/functions/import-source/url-validation.ts index 13f10b6e4..cfe3dbbea 100644 --- a/supabase/functions/import-source/url-validation.ts +++ b/supabase/functions/import-source/url-validation.ts @@ -6,6 +6,7 @@ // re-exported here so this file and the shared copy can't drift apart. import { isPrivateIpv4, isPrivateIpv6 } from "../_shared/url-validation.ts"; +import { untilAborted } from "../_shared/bounded-body.ts"; export { isPrivateIpv4, isPrivateIpv6 }; @@ -39,18 +40,21 @@ export function validatePublicHttpUrl(rawUrl: string): URL { } // Re-checks the hostname's *resolved* addresses, not just its literal text, so a -// public-looking domain that resolves to a private/link-local IP (DNS rebinding) -// is still rejected. Call this immediately before every fetch, including redirect hops. -export async function validateResolvedPublicHost(url: URL): Promise { +// public-looking domain that resolves to a private, reserved or link-local IP is +// still rejected. Call this immediately before every fetch, including redirect +// hops. The runtime cannot pin the checked address for the connection, so a +// resolver that answers differently a moment later is not covered; see +// docs/standards/security.md "Recorded exceptions". +export async function validateResolvedPublicHost(url: URL, signal?: AbortSignal): Promise { const hostname = url.hostname.toLowerCase(); if (isPrivateIpv4(hostname) || isPrivateIpv6(hostname)) { throw new Error("Private and local network URLs cannot be imported."); } - const results = await Promise.allSettled([ - Deno.resolveDns(hostname, "A"), - Deno.resolveDns(hostname, "AAAA"), - ]); + const results = await untilAborted(Promise.allSettled([ + Deno.resolveDns(hostname, "A", { signal }), + Deno.resolveDns(hostname, "AAAA", { signal }), + ]), signal); const addresses = results.flatMap((result) => (result.status === "fulfilled" ? result.value : [])); if (addresses.length === 0) { diff --git a/supabase/functions/share/auth_test.ts b/supabase/functions/share/auth_test.ts index 8bfd15ca2..fbf1903b5 100644 --- a/supabase/functions/share/auth_test.ts +++ b/supabase/functions/share/auth_test.ts @@ -35,12 +35,13 @@ Deno.test("share records the token's user as the creator", async () => { label: "valid token", authorization: `Bearer ${VALID_TOKEN}`, status: 200, - body: { shareCode: "abc123", shareUrl: "https://debate.pi-stack.org/share/abc123", expiresAt: null }, + body: { shareCode: "abc123", shareUrl: "https://debate.pi-stack.org/share/abc123", expiresAt: "2026-01-01T00:00:00.000Z" }, authCalls: 1, }], (url, _headers, init) => { + if (url.pathname === "/rest/v1/rpc/consume_daily_quota") return Response.json([{ allowed: true, current_count: 1 }]); if (url.pathname !== "/rest/v1/shared_items") throw new Error(`Unexpected request: ${url}`); creators.push((JSON.parse(String(init?.body)) as { created_by?: unknown }).created_by); - return Response.json({ share_code: "abc123", expires_at: null }, { status: 201 }); + return Response.json({ share_code: "abc123", expires_at: "2026-01-01T00:00:00.000Z" }, { status: 201 }); }); if (JSON.stringify(creators) !== JSON.stringify([TEST_USER.id])) throw new Error(`creators: ${JSON.stringify(creators)}`); }); diff --git a/supabase/functions/share/index.ts b/supabase/functions/share/index.ts index 15039b951..7de934906 100644 --- a/supabase/functions/share/index.ts +++ b/supabase/functions/share/index.ts @@ -4,10 +4,14 @@ import { createClient } from "https://esm.sh/@supabase/supabase-js@2"; import { HttpError, jsonError, requireUser } from "../_shared/auth.ts"; import { PUBLIC_WEB_APP_ORIGIN, getCorsHeaders } from "../_shared/cors.ts"; +import { BodyTooLargeError, readBoundedBody } from "../_shared/bounded-body.ts"; - -// SECURITY: Allow specific origins for CORS - +// Shared items are saved text, so 512 KB of JSON leaves room for long cases and +// chats while keeping one request from filling the table. +const MAX_SHARE_BYTES = 512 * 1024; +const DEFAULT_EXPIRES_IN_DAYS = 30; +const MAX_EXPIRES_IN_DAYS = 90; +const DAILY_SHARE_LIMIT = 50; const supabaseUrl = Deno.env.get("SUPABASE_URL")!; const supabaseServiceKey = Deno.env.get("SUPABASE_SERVICE_ROLE_KEY")!; @@ -23,6 +27,24 @@ function generateShareCode(): string { return code; } +function jsonResponse(body: unknown, status: number, corsHeaders: Record) { + return new Response(JSON.stringify(body), { + status, + headers: { ...corsHeaders, "Content-Type": "application/json" }, + }); +} + +async function consumeDailyShareQuota(userId: string): Promise<{ allowed: boolean; currentCount: number }> { + const { data, error } = await supabase.rpc("consume_daily_quota", { + p_user_id: userId, + p_limit: DAILY_SHARE_LIMIT, + p_quota_key: "share", + }); + if (error) throw error; + const row = Array.isArray(data) ? data[0] : data; + return { allowed: Boolean(row?.allowed), currentCount: row?.current_count ?? 0 }; +} + function isUniqueViolation(error: { code?: string; message?: string } | null): boolean { return error?.code === "23505" || Boolean(error?.message?.toLowerCase().includes("duplicate")); } @@ -52,31 +74,51 @@ export async function handleRequest(req: Request): Promise { } try { - const body = await req.json(); - const { itemType, itemData, title, expiresInDays } = body; + let body: Record; + try { + const parsed = JSON.parse(await readBoundedBody(req, MAX_SHARE_BYTES)); + if (!parsed || typeof parsed !== "object") throw new SyntaxError("not an object"); + body = parsed; + } catch (error) { + if (error instanceof BodyTooLargeError) { + return jsonResponse({ error: "Shared content is too large.", code: "payload_too_large" }, 413, corsHeaders); + } + return jsonResponse({ error: "Invalid JSON body." }, 400, corsHeaders); + } + const { itemType, itemData, title } = body; if (!itemType || !itemData) { - return new Response(JSON.stringify({ error: "Missing itemType or itemData" }), { - status: 400, - headers: { ...corsHeaders, "Content-Type": "application/json" }, - }); + return jsonResponse({ error: "Missing itemType or itemData" }, 400, corsHeaders); } // Validate item type const validTypes = ['case', 'attack', 'brainstorm', 'factoids', 'chat', 'sources']; - if (!validTypes.includes(itemType)) { - return new Response(JSON.stringify({ error: "Invalid item type" }), { - status: 400, - headers: { ...corsHeaders, "Content-Type": "application/json" }, - }); + if (typeof itemType !== "string" || !validTypes.includes(itemType)) { + return jsonResponse({ error: "Invalid item type" }, 400, corsHeaders); } - // Calculate expiration - let expiresAt: string | null = null; - if (expiresInDays && typeof expiresInDays === 'number') { - const expiry = new Date(); - expiry.setDate(expiry.getDate() + expiresInDays); - expiresAt = expiry.toISOString(); + // Every share expires. No client sends expiresInDays today; an omitted + // value gets the default, and an explicit one must be in range. + const expiresInDays = "expiresInDays" in body ? body.expiresInDays : DEFAULT_EXPIRES_IN_DAYS; + if (typeof expiresInDays !== "number" || !Number.isInteger(expiresInDays) || expiresInDays < 1 || expiresInDays > MAX_EXPIRES_IN_DAYS) { + return jsonResponse({ error: `expiresInDays must be a whole number from 1 to ${MAX_EXPIRES_IN_DAYS}.` }, 400, corsHeaders); + } + const expiresAt = new Date(Date.now() + expiresInDays * 24 * 60 * 60 * 1000).toISOString(); + + let quota: { allowed: boolean; currentCount: number }; + try { + quota = await consumeDailyShareQuota(userId); + } catch (err) { + console.error("[SHARE] quota reservation failed", err instanceof Error ? err.message : String(err)); + return jsonResponse({ error: "Unable to verify your quota. Please try again." }, 500, corsHeaders); + } + if (!quota.allowed) { + return jsonResponse({ + error: `You've reached the daily share limit (${DAILY_SHARE_LIMIT}). Try again tomorrow.`, + code: "limit_reached", + limit: DAILY_SHARE_LIMIT, + current: quota.currentCount, + }, 429, corsHeaders); } let data: { share_code: string; expires_at: string | null } | null = null; diff --git a/supabase/functions/share/limits_test.ts b/supabase/functions/share/limits_test.ts new file mode 100644 index 000000000..540e22c09 --- /dev/null +++ b/supabase/functions/share/limits_test.ts @@ -0,0 +1,114 @@ +import { assertEquals } from "jsr:@std/assert@1.0.19"; +import { TEST_ANON_KEY, TEST_SUPABASE_URL, TEST_USER, VALID_TOKEN } from "../_shared/testing/bearer-auth.ts"; + +for (const [key, value] of Object.entries({ + SUPABASE_URL: TEST_SUPABASE_URL, + SUPABASE_ANON_KEY: TEST_ANON_KEY, + SUPABASE_SERVICE_ROLE_KEY: "test-service-role-key", +})) Deno.env.set(key, value); + +const { handleRequest } = await import("./index.ts"); + +const DAY_MS = 24 * 60 * 60 * 1000; + +type Calls = { quota: Record[]; inserts: Record[] }; + +// Sends one signed-in share request against a fake Supabase backend and +// records the quota reservations and inserts it makes. +async function sendShare(body: string, { quotaAllowed = true } = {}) { + const calls: Calls = { quota: [], inserts: [] }; + const originalFetch = globalThis.fetch; + globalThis.fetch = (async (input: Request | URL | string, init?: RequestInit) => { + const url = new URL(input instanceof Request ? input.url : String(input)); + if (url.origin !== TEST_SUPABASE_URL) throw new Error(`Unexpected request: ${url}`); + if (url.pathname === "/auth/v1/user") return Response.json(TEST_USER); + const payload = JSON.parse(String(init?.body)) as Record; + if (url.pathname === "/rest/v1/rpc/consume_daily_quota") { + calls.quota.push(payload); + return Response.json([{ allowed: quotaAllowed, current_count: quotaAllowed ? 1 : 50 }]); + } + if (url.pathname === "/rest/v1/shared_items") { + calls.inserts.push(payload); + return Response.json({ share_code: "abc123", expires_at: payload.expires_at }, { status: 201 }); + } + throw new Error(`Unexpected request: ${url}`); + }) as typeof fetch; + try { + const response = await handleRequest(new Request("http://127.0.0.1/functions/v1/share", { + method: "POST", + headers: { "Content-Type": "application/json", Authorization: `Bearer ${VALID_TOKEN}` }, + body, + })); + return { status: response.status, body: await response.json(), calls }; + } finally { + globalThis.fetch = originalFetch; + } +} + +const shareBody = (fields: Record) => + JSON.stringify({ itemType: "case", itemData: { motion: "This house would test" }, ...fields }); + +function assertExpiresInDays(expiresAt: unknown, days: number, before: number) { + const at = Date.parse(String(expiresAt)); + if (!(at >= before + days * DAY_MS - 1000 && at <= Date.now() + days * DAY_MS + 1000)) { + throw new Error(`expected an expiry ${days} days out, got ${expiresAt}`); + } +} + +Deno.test("share rejects a payload over the size cap before touching the database", async () => { + const result = await sendShare(shareBody({ itemData: { body: "x".repeat(600 * 1024) } })); + assertEquals(result, { + status: 413, + body: { error: "Shared content is too large.", code: "payload_too_large" }, + calls: { quota: [], inserts: [] }, + }); +}); + +Deno.test("share rejects expiries that are not 1 to 90 whole days", async () => { + for (const expiresInDays of [91, 0, -1, 1.5, "30", 1e9, null]) { + const result = await sendShare(shareBody({ expiresInDays })); + assertEquals(result, { + status: 400, + body: { error: "expiresInDays must be a whole number from 1 to 90." }, + calls: { quota: [], inserts: [] }, + }, `expiresInDays=${expiresInDays}`); + } +}); + +Deno.test("share honours expiries at both ends of the 1 to 90 day range", async () => { + for (const days of [1, 90]) { + const before = Date.now(); + const result = await sendShare(shareBody({ expiresInDays: days })); + assertEquals(result.status, 200, `expiresInDays=${days}`); + assertEquals(result.calls.inserts.length, 1); + assertExpiresInDays(result.calls.inserts[0].expires_at, days, before); + } +}); + +Deno.test("share expires a link without an expiry after 30 days and reserves the share quota", async () => { + const before = Date.now(); + const result = await sendShare(shareBody({})); + assertEquals(result.status, 200); + assertEquals(result.calls.quota, [{ p_user_id: TEST_USER.id, p_limit: 50, p_quota_key: "share" }]); + assertEquals(result.calls.inserts.length, 1); + assertExpiresInDays(result.calls.inserts[0].expires_at, 30, before); + assertExpiresInDays(result.body.expiresAt, 30, before); +}); + +Deno.test("share refuses once the daily share quota is spent", async () => { + const result = await sendShare(shareBody({}), { quotaAllowed: false }); + assertEquals(result.status, 429); + assertEquals(result.body, { + error: "You've reached the daily share limit (50). Try again tomorrow.", + code: "limit_reached", + limit: 50, + current: 50, + }); + assertEquals(result.calls.quota.length, 1); + assertEquals(result.calls.inserts, []); +}); + +Deno.test("share answers malformed JSON with 400", async () => { + const result = await sendShare("{not json"); + assertEquals(result, { status: 400, body: { error: "Invalid JSON body." }, calls: { quota: [], inserts: [] } }); +});