From 717f41df59706375874d71363228e90fa01fb89e Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 7 Oct 2026 15:03:48 +0000 Subject: [PATCH 1/2] fix(edge): bound import-source and share requests import-source now keeps one 10-second deadline from DNS through the last body byte, so a slow sender is cut off instead of holding the function open. Resolved addresses are checked against the full IANA special-purpose lists, including NAT64, benchmarking, multicast and reserved space. share now caps the request at 512 KB, accepts expiries of 1 to 90 days (30 when omitted), and reserves a "share" daily quota of 50 per user through consume_daily_quota. The DNS pinning gap that the runtime cannot close is recorded as an exception in docs/standards/security.md, pending Adam's decision. Closes findings [4], [7] and [10] of the 2026-08-19 security review. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/standards/security.md | 1 + src/test/security-hardening.test.ts | 4 +- supabase/README.md | 4 +- supabase/functions/_shared/bounded-body.ts | 51 ++++++++ supabase/functions/_shared/url-validation.ts | 121 ++++++++++++++---- .../functions/import-source/deadline_test.ts | 83 ++++++++++++ supabase/functions/import-source/index.ts | 86 ++++++++----- .../import-source/resource-limits.ts | 27 +--- .../import-source/url-validation.test.ts | 84 +++++++++++- .../functions/import-source/url-validation.ts | 18 ++- supabase/functions/share/auth_test.ts | 5 +- supabase/functions/share/index.ts | 82 +++++++++--- supabase/functions/share/limits_test.ts | 112 ++++++++++++++++ 13 files changed, 562 insertions(+), 116 deletions(-) create mode 100644 supabase/functions/_shared/bounded-body.ts create mode 100644 supabase/functions/import-source/deadline_test.ts create mode 100644 supabase/functions/share/limits_test.ts 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..d90e46265 100644 --- a/src/test/security-hardening.test.ts +++ b/src/test/security-hardening.test.ts @@ -70,8 +70,8 @@ describe('security hardening regressions', () => { }); it('resolves DNS and bounds importer bodies before processing', () => { - expect(importer).toContain('validateResolvedPublicHost(currentUrl)'); - expect(importer).toContain('readBoundedText(response)'); + expect(importer).toContain('validateResolvedPublicHost(currentUrl, signal)'); + expect(importer).toContain('readBoundedText(response, deadline.signal)'); expect(read('supabase/functions/import-source/resource-limits.ts')).toContain('MAX_RESPONSE_BYTES'); }); 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.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..f7c925e45 100644 --- a/supabase/functions/_shared/url-validation.ts +++ b/supabase/functions/_shared/url-validation.ts @@ -1,42 +1,109 @@ -// 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. -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 + [[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..2be68513e --- /dev/null +++ b/supabase/functions/import-source/deadline_test.ts @@ -0,0 +1,83 @@ +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"); +}); diff --git a/supabase/functions/import-source/index.ts b/supabase/functions/import-source/index.ts index d2d64090c..aff9b15f4 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 { untilAborted } from "../_shared/bounded-body.ts"; import { HttpError, jsonError, requireUser } from "../_shared/auth.ts"; import { getCorsHeaders } from "../_shared/cors.ts"; @@ -9,6 +10,7 @@ export { isPrivateIpv4, isPrivateIpv6, validatePublicHttpUrl }; const MAX_CONTENT_CHARS = 50_000; +// Total time for DNS, redirects and the whole body, not just the headers. const FETCH_TIMEOUT_MS = 10_000; const MAX_REDIRECTS = 5; const DAILY_IMPORT_LIMIT = 50; @@ -58,42 +60,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,6 +271,8 @@ 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); if (!body || typeof body !== "object") { @@ -273,20 +280,23 @@ export async function handleRequest(req: Request): Promise { 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 +322,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..79145e8b8 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,88 @@ 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 + "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..cd948c05d --- /dev/null +++ b/supabase/functions/share/limits_test.ts @@ -0,0 +1,112 @@ +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 an expiry at the 90-day limit", async () => { + const before = Date.now(); + const result = await sendShare(shareBody({ expiresInDays: 90 })); + assertEquals(result.status, 200); + assertEquals(result.calls.inserts.length, 1); + assertExpiresInDays(result.calls.inserts[0].expires_at, 90, 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: [] } }); +}); From 72af8556cff7425ec2cfb062e25585fae7cea5ee Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 7 Oct 2026 15:17:53 +0000 Subject: [PATCH 2/2] fix(edge): bound the import request body and block the dummy IPv6 prefix import-source now reads its own request body with a 16 KB cap under the same deadline, so a stalled upload gets a 408 instead of holding the function open and then reporting a misleading source timeout. Adds 100:0:0:1::/64 to the blocked IPv6 ranges. Replaces the importer source-string assertions with behavior tests for the byte cap at its exact boundary and the one-day share expiry. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/test/security-hardening.test.ts | 8 ++- .../functions/_shared/bounded-body.test.ts | 26 ++++++++++ supabase/functions/_shared/url-validation.ts | 6 ++- .../functions/import-source/deadline_test.ts | 50 +++++++++++++++++++ supabase/functions/import-source/index.ts | 19 +++++-- .../import-source/url-validation.test.ts | 1 + supabase/functions/share/limits_test.ts | 14 +++--- 7 files changed, 109 insertions(+), 15 deletions(-) create mode 100644 supabase/functions/_shared/bounded-body.test.ts diff --git a/src/test/security-hardening.test.ts b/src/test/security-hardening.test.ts index d90e46265..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, signal)'); - expect(importer).toContain('readBoundedText(response, deadline.signal)'); - 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/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/url-validation.ts b/supabase/functions/_shared/url-validation.ts index f7c925e45..6265bd777 100644 --- a/supabase/functions/_shared/url-validation.ts +++ b/supabase/functions/_shared/url-validation.ts @@ -1,6 +1,9 @@ // 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. +// 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. type Cidr = readonly [bytes: readonly number[], prefixLength: number]; @@ -32,6 +35,7 @@ const BLOCKED_IPV6: readonly Cidr[] = [ [[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 diff --git a/supabase/functions/import-source/deadline_test.ts b/supabase/functions/import-source/deadline_test.ts index 2be68513e..501b61fdf 100644 --- a/supabase/functions/import-source/deadline_test.ts +++ b/supabase/functions/import-source/deadline_test.ts @@ -81,3 +81,53 @@ Deno.test("import-source stops reading a body that drips slower than the deadlin 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 aff9b15f4..d4996834b 100644 --- a/supabase/functions/import-source/index.ts +++ b/supabase/functions/import-source/index.ts @@ -1,7 +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 { untilAborted } from "../_shared/bounded-body.ts"; +import { BodyTooLargeError, readBoundedBody, untilAborted } from "../_shared/bounded-body.ts"; import { HttpError, jsonError, requireUser } from "../_shared/auth.ts"; import { getCorsHeaders } from "../_shared/cors.ts"; @@ -10,9 +10,10 @@ export { isPrivateIpv4, isPrivateIpv6, validatePublicHttpUrl }; const MAX_CONTENT_CHARS = 50_000; -// Total time for DNS, redirects and the whole body, not just the headers. +// 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; @@ -274,7 +275,19 @@ export async function handleRequest(req: Request): Promise { 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); diff --git a/supabase/functions/import-source/url-validation.test.ts b/supabase/functions/import-source/url-validation.test.ts index 79145e8b8..014c65921 100644 --- a/supabase/functions/import-source/url-validation.test.ts +++ b/supabase/functions/import-source/url-validation.test.ts @@ -83,6 +83,7 @@ Deno.test("isPrivateIpv6 rejects every IANA special-purpose range that is not gl "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 diff --git a/supabase/functions/share/limits_test.ts b/supabase/functions/share/limits_test.ts index cd948c05d..540e22c09 100644 --- a/supabase/functions/share/limits_test.ts +++ b/supabase/functions/share/limits_test.ts @@ -75,12 +75,14 @@ Deno.test("share rejects expiries that are not 1 to 90 whole days", async () => } }); -Deno.test("share honours an expiry at the 90-day limit", async () => { - const before = Date.now(); - const result = await sendShare(shareBody({ expiresInDays: 90 })); - assertEquals(result.status, 200); - assertEquals(result.calls.inserts.length, 1); - assertExpiresInDays(result.calls.inserts[0].expires_at, 90, before); +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 () => {