diff --git a/src/lib/send-attachment-limits.test.ts b/src/lib/send-attachment-limits.test.ts new file mode 100644 index 00000000..76b76bca --- /dev/null +++ b/src/lib/send-attachment-limits.test.ts @@ -0,0 +1,160 @@ +// The send attachment caps, and the constraint that actually bound them. +// +// The self-hosted caps were 5 files / 512KiB each / 768KiB total, and 768KiB is +// not an arbitrary number: attachments travel base64-encoded inside the JSON +// send body, and base64(768KiB) is 1048576 bytes — EXACTLY the 1 MiB +// `MAX_JSON_BODY_BYTES` the send route reads its body with. The attachment cap +// was the largest raw payload whose encoding fits the body cap. +// +// That coupling is why raising the attachment numbers ALONE would have changed +// nothing: `readJsonBody` runs before the attachment branch, so an oversize +// document is refused 413 by the body cap before any attachment rule is +// consulted. These tests pin the coupling so the two cannot drift apart again, +// in EITHER direction — a body budget too small silently re-blocks the send, +// and attachment caps too large silently overrun the provider. + +import { describe, expect, test } from "bun:test"; +import { + base64EncodedBytes, + describeSendAttachmentLimits, + LOCAL_SEND_ATTACHMENT_LIMITS, + mimeEncodedUpperBound, + requiredSendJsonBodyBytes, + SELF_HOSTED_SEND_ATTACHMENT_LIMITS, + SES_MAX_MESSAGE_BYTES, + type SendAttachmentLimits, +} from "./send-attachment-limits.js"; + +/** The live case this cap was raised for: a notarised 6-page scan. */ +const NOTARISED_SCAN_BYTES = 3_800_000; + +describe("base64EncodedBytes", () => { + test("matches the standard 4/3 expansion with padding", () => { + // Hand-checkable vectors: base64 emits 4 characters per 3 input bytes, + // padding the final group. + expect(base64EncodedBytes(0)).toBe(0); + expect(base64EncodedBytes(1)).toBe(4); + expect(base64EncodedBytes(2)).toBe(4); + expect(base64EncodedBytes(3)).toBe(4); + expect(base64EncodedBytes(4)).toBe(8); + }); + + test("agrees with a real Buffer encoding", () => { + // The formula is only worth having if it predicts what Buffer actually does. + for (const size of [1, 2, 3, 5, 17, 1024, 100_000]) { + const encoded = Buffer.alloc(size).toString("base64").length; + expect(base64EncodedBytes(size)).toBe(encoded); + } + }); +}); + +describe("self-hosted send attachment caps carry a real document", () => { + test("a notarised scan fits under the per-file cap", () => { + expect(NOTARISED_SCAN_BYTES).toBeLessThanOrEqual( + SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxBytesPerFile, + ); + }); + + test("two notarised scans fit under the total cap in one message", () => { + expect(NOTARISED_SCAN_BYTES * 2).toBeLessThanOrEqual( + SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxTotalBytes, + ); + }); + + test("the file-count cap still admits a two-document send", () => { + expect(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxFiles).toBeGreaterThanOrEqual(2); + }); +}); + +// The cap must still BE a cap. A change that removed enforcement entirely would +// pass every "a big file now works" assertion above, so each bound is pinned +// from the other side too. +describe("the caps still bound", () => { + test("per-file and total caps are finite and ordered", () => { + const { maxFiles, maxBytesPerFile, maxTotalBytes } = SELF_HOSTED_SEND_ATTACHMENT_LIMITS; + expect(Number.isFinite(maxBytesPerFile)).toBe(true); + expect(Number.isFinite(maxTotalBytes)).toBe(true); + expect(Number.isFinite(maxFiles)).toBe(true); + expect(maxBytesPerFile).toBeGreaterThan(0); + expect(maxFiles).toBeGreaterThan(0); + // A per-file cap above the total would be unreachable and misleading. + expect(maxBytesPerFile).toBeLessThanOrEqual(maxTotalBytes); + }); + + test("one byte over the per-file cap is over the per-file cap", () => { + expect(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxBytesPerFile + 1) + .toBeGreaterThan(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxBytesPerFile); + }); + + test("the self-hosted caps stay at or below the local ones", () => { + // `src/lib/send.local.ts` enforces its own 25MiB ceiling on the local send + // path. If the self-hosted per-file cap ever rose above it, that layer would + // start rejecting sends the hosted route had just accepted. + expect(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxBytesPerFile) + .toBeLessThanOrEqual(LOCAL_SEND_ATTACHMENT_LIMITS.maxBytesPerFile); + }); +}); + +// THE TEST THAT WOULD HAVE CAUGHT THE ORIGINAL DEFECT. +describe("the JSON body budget can actually carry a full-size attachment set", () => { + test("the send body budget exceeds the encoded worst-case attachment set", () => { + const encoded = base64EncodedBytes(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxTotalBytes); + expect(requiredSendJsonBodyBytes(SELF_HOSTED_SEND_ATTACHMENT_LIMITS)) + .toBeGreaterThan(encoded); + }); + + test("it leaves room for the envelope around the attachments", () => { + // Subject, addresses, filenames, content types, and a text/html body all + // share the same request body. + const encoded = base64EncodedBytes(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxTotalBytes); + const slack = requiredSendJsonBodyBytes(SELF_HOSTED_SEND_ATTACHMENT_LIMITS) - encoded; + expect(slack).toBeGreaterThanOrEqual(1024 * 1024); + }); + + test("the budget tracks the cap rather than being a hand-typed number", () => { + // Doubling the cap must move the budget. A constant that ignored its input + // is exactly how the body cap and the attachment cap drifted apart. + const doubled: SendAttachmentLimits = { + ...SELF_HOSTED_SEND_ATTACHMENT_LIMITS, + maxTotalBytes: SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxTotalBytes * 2, + }; + expect(requiredSendJsonBodyBytes(doubled)) + .toBeGreaterThan(requiredSendJsonBodyBytes(SELF_HOSTED_SEND_ATTACHMENT_LIMITS)); + }); +}); + +describe("the caps stay inside the provider ceiling", () => { + test("SES is pinned at its documented v2 value", () => { + // AWS: SESv2/SMTP maximum message size is 40MB per message, measured AFTER + // base64 encoding, and is NOT adjustable. (The v1 API's limit is 10MB.) + expect(SES_MAX_MESSAGE_BYTES).toBe(40_000_000); + }); + + test("a worst-case message stays under the SES ceiling", () => { + expect(mimeEncodedUpperBound(SELF_HOSTED_SEND_ATTACHMENT_LIMITS)) + .toBeLessThan(SES_MAX_MESSAGE_BYTES); + }); + + test("it keeps a real margin, not a rounding-error one", () => { + const margin = SES_MAX_MESSAGE_BYTES - mimeEncodedUpperBound(SELF_HOSTED_SEND_ATTACHMENT_LIMITS); + expect(margin).toBeGreaterThan(5_000_000); + }); + + test("the bound can fail — a cap over the ceiling is reported as over", () => { + // A ceiling check that cannot fail is not a check. This proves the + // instrument fires on a known-bad input. + const oversize: SendAttachmentLimits = { + maxFiles: 5, + maxBytesPerFile: 60 * 1024 * 1024, + maxTotalBytes: 60 * 1024 * 1024, + }; + expect(mimeEncodedUpperBound(oversize)).toBeGreaterThan(SES_MAX_MESSAGE_BYTES); + }); +}); + +describe("describeSendAttachmentLimits", () => { + test("renders the raised self-hosted caps", () => { + expect(describeSendAttachmentLimits(SELF_HOSTED_SEND_ATTACHMENT_LIMITS)) + .toBe("5 files, 10MB each, 20MB total"); + }); +}); diff --git a/src/lib/send-attachment-limits.ts b/src/lib/send-attachment-limits.ts index 2f148467..2033937b 100644 --- a/src/lib/send-attachment-limits.ts +++ b/src/lib/send-attachment-limits.ts @@ -14,11 +14,22 @@ export interface SendAttachmentLimits { maxTotalBytes: number; } -/** Enforced by the self-hosted JSON send route (`POST /v1/messages/send`). */ +/** + * Enforced by the self-hosted JSON send route (`POST /v1/messages/send`). + * + * These are DOCUMENT-SIZED on purpose: 512KiB could not carry a scanned page, + * so the route refused ordinary notarised paperwork. The ceiling above them is + * the provider's, not ours — see `SES_MAX_MESSAGE_BYTES` and + * `mimeEncodedUpperBound`. + * + * The per-file cap MUST stay at or below the local path's own 25MiB ceiling + * (`MAX_ATTACHMENT_SIZE_BYTES` in `send.local.ts`), or that layer would reject + * sends this route had just accepted. + */ export const SELF_HOSTED_SEND_ATTACHMENT_LIMITS: SendAttachmentLimits = { maxFiles: 5, - maxBytesPerFile: 512 * 1024, - maxTotalBytes: 768 * 1024, + maxBytesPerFile: 10 * 1024 * 1024, + maxTotalBytes: 20 * 1024 * 1024, }; /** @@ -31,12 +42,75 @@ export const LOCAL_SEND_ATTACHMENT_LIMITS: SendAttachmentLimits = { maxTotalBytes: 25 * 1024 * 1024, }; -function humanBytes(bytes: number): string { +/** + * Bytes needed to base64-encode `rawBytes`: 4 characters per 3 input bytes, + * final group padded. + * + * This is the conversion that made the old caps look arbitrary. Attachments + * travel base64-encoded inside the JSON send body, so every raw cap below is + * spent against the body budget at 4/3 its size. + */ +export function base64EncodedBytes(rawBytes: number): number { + return Math.ceil(rawBytes / 3) * 4; +} + +/** + * Room reserved in the send body for everything that is not attachment content: + * addresses, subject, filenames, content types, JSON structure, and a text or + * HTML body. + */ +export const SEND_ENVELOPE_HEADROOM_BYTES = 2 * 1024 * 1024; + +/** + * The JSON request-body budget the send route needs in order for `limits` to be + * reachable at all. + * + * DERIVED, never hand-typed. The previous 768KiB total cap was exactly + * `base64EncodedBytes(768KiB) === 1MiB`, the route's body cap — so the two + * numbers were coupled in fact and independent in code, and raising one alone + * silently accomplished nothing. Deriving the body budget from the attachment + * cap makes that drift impossible. + */ +export function requiredSendJsonBodyBytes(limits: SendAttachmentLimits): number { + return base64EncodedBytes(limits.maxTotalBytes) + SEND_ENVELOPE_HEADROOM_BYTES; +} + +/** + * Amazon SES maximum message size, measured AFTER base64 encoding. Not + * adjustable. + * + * 40MB is the SESv2/SMTP figure, and `src/providers/ses.ts` sends through + * SESv2 `SendEmailCommand` with raw content. The v1 API's limit is 10MB; do not + * mix them up if that provider path ever changes. + */ +export const SES_MAX_MESSAGE_BYTES = 40_000_000; + +/** + * Upper bound on the encoded MIME message a full-size `limits` set produces, + * for comparison against `SES_MAX_MESSAGE_BYTES`. + * + * MIME wraps base64 at 76 characters per line plus CRLF, so the encoded payload + * costs a further 78/76 on top of the 4/3 expansion. + */ +export function mimeEncodedUpperBound(limits: SendAttachmentLimits): number { + const encoded = base64EncodedBytes(limits.maxTotalBytes); + return Math.ceil((encoded * 78) / 76) + SEND_ENVELOPE_HEADROOM_BYTES; +} + +/** + * Renders a cap for an operator-facing message. + * + * Exported so the send route's rejection text is DERIVED from the constant it + * enforces. Those strings previously read "512KiB" and "768KiB" as literals, so + * raising the caps would have left the route refusing at one size while telling + * the operator another. + */ +export function humanLimitBytes(bytes: number): string { if (bytes >= 1024 * 1024) return `${Math.round(bytes / (1024 * 1024))}MB`; return `${Math.round(bytes / 1024)}KiB`; } export function describeSendAttachmentLimits(limits: SendAttachmentLimits): string { - return `${limits.maxFiles} files, ${humanBytes(limits.maxBytesPerFile)} each, ` - + `${humanBytes(limits.maxTotalBytes)} total`; + return `${limits.maxFiles} files, ${humanLimitBytes(limits.maxBytesPerFile)} each, ` + + `${humanLimitBytes(limits.maxTotalBytes)} total`; } diff --git a/src/lib/send-preflight.test.ts b/src/lib/send-preflight.test.ts index 5c2da1e4..55ef1745 100644 --- a/src/lib/send-preflight.test.ts +++ b/src/lib/send-preflight.test.ts @@ -91,41 +91,59 @@ describe("attachment caps are evaluated, not merely printed", () => { )).toEqual([]); }); + // These fixtures are DERIVED from the caps rather than written as literals. + // They used to be fixed sizes (600KiB, 400KiB) chosen against a 512KiB/768KiB + // cap, so raising the caps turned every "oversize" fixture into a legal one and + // the suite asserted the opposite of what it was named for. A fixture that + // encodes a constant is the same drift this module exists to prevent. + const SH = SELF_HOSTED_SEND_ATTACHMENT_LIMITS; + it("catches a per-file overage that the old preview printed as fine", () => { - // 600KiB is under the 25MB global ceiling readSendAttachments enforces, so + // Still under the much larger local ceiling readSendAttachments enforces, so // nothing else in the pipeline would have caught it before the server did. + const oversize = SH.maxBytesPerFile + 1; + expect(oversize).toBeLessThanOrEqual(LOCAL_SEND_ATTACHMENT_LIMITS.maxBytesPerFile); const findings = evaluateAttachmentCaps( - [{ filename: "big.pdf", bytes: 600 * K }], - SELF_HOSTED_SEND_ATTACHMENT_LIMITS, + [{ filename: "big.pdf", bytes: oversize }], + SH, ); expect(findings.map((f) => f.rule)).toContain("bytes_per_file"); expect(findings[0]!.detail).toContain("big.pdf"); }); it("catches a total overage even when every file is individually legal", () => { - const files = Array.from({ length: 3 }, (_, i) => ({ filename: `f${i}.pdf`, bytes: 400 * K })); - const findings = evaluateAttachmentCaps(files, SELF_HOSTED_SEND_ATTACHMENT_LIMITS); + // Enough max-size files to exceed the total, each one legal on its own. + const count = Math.ceil(SH.maxTotalBytes / SH.maxBytesPerFile) + 1; + expect(count).toBeLessThanOrEqual(SH.maxFiles); + const files = Array.from({ length: count }, (_, i) => ({ filename: `f${i}.pdf`, bytes: SH.maxBytesPerFile })); + const findings = evaluateAttachmentCaps(files, SH); expect(findings.map((f) => f.rule)).toContain("total_bytes"); expect(findings.map((f) => f.rule)).not.toContain("bytes_per_file"); }); it("catches too many files", () => { - const files = Array.from({ length: 6 }, (_, i) => ({ filename: `f${i}.txt`, bytes: 1 })); - expect(evaluateAttachmentCaps(files, SELF_HOSTED_SEND_ATTACHMENT_LIMITS).map((f) => f.rule)) + const files = Array.from({ length: SH.maxFiles + 1 }, (_, i) => ({ filename: `f${i}.txt`, bytes: 1 })); + expect(evaluateAttachmentCaps(files, SH).map((f) => f.rule)) .toContain("file_count"); }); it("applies the LOCAL caps in local mode, not the server's", () => { - // A 600KiB file is refused self-hosted and fine locally. Predicting the wrong - // mode's limits is the same class of defect as not predicting at all. - const files = [{ filename: "big.pdf", bytes: 600 * K }]; + // A file over the self-hosted per-file cap but inside the local one is + // refused self-hosted and fine locally. Predicting the wrong mode's limits is + // the same class of defect as not predicting at all. + const bytes = SH.maxBytesPerFile + 1; + expect(bytes).toBeLessThanOrEqual(LOCAL_SEND_ATTACHMENT_LIMITS.maxBytesPerFile); + const files = [{ filename: "big.pdf", bytes }]; expect(evaluateAttachmentCaps(files, LOCAL_SEND_ATTACHMENT_LIMITS)).toEqual([]); - expect(evaluateAttachmentCaps(files, SELF_HOSTED_SEND_ATTACHMENT_LIMITS).length).toBeGreaterThan(0); + expect(evaluateAttachmentCaps(files, SH).length).toBeGreaterThan(0); }); it("reports every distinct violation rather than stopping at the first", () => { - const files = Array.from({ length: 6 }, (_, i) => ({ filename: `f${i}.pdf`, bytes: 600 * K })); - const rules = new Set(evaluateAttachmentCaps(files, SELF_HOSTED_SEND_ATTACHMENT_LIMITS).map((f) => f.rule)); + const files = Array.from( + { length: SH.maxFiles + 1 }, + (_, i) => ({ filename: `f${i}.pdf`, bytes: SH.maxBytesPerFile + 1 }), + ); + const rules = new Set(evaluateAttachmentCaps(files, SH).map((f) => f.rule)); expect(rules).toEqual(new Set(["file_count", "bytes_per_file", "total_bytes"])); }); }); diff --git a/src/lib/send-preflight.ts b/src/lib/send-preflight.ts index 0136964f..e3875a59 100644 --- a/src/lib/send-preflight.ts +++ b/src/lib/send-preflight.ts @@ -110,9 +110,11 @@ export interface AttachmentCapFinding { * Evaluate the REAL files against the mode's caps. * * These caps were printed as prose next to the attachment count and never checked, - * so a set that the self-hosted route refuses (5 files / 512KiB each / 768KiB - * total) previewed as fine. `readSendAttachments` only enforces the much larger - * global ceilings (25MB), so nothing else caught it either. + * so a set that the self-hosted route refuses previewed as fine. + * `readSendAttachments` only enforces the much larger local ceilings, so nothing + * else caught it either. The exact numbers deliberately are not repeated here — + * they live in `SELF_HOSTED_SEND_ATTACHMENT_LIMITS`, and a prose copy of them is + * how the prediction drifted from the enforcement in the first place. * * `sizes` are decoded byte lengths, not base64 lengths — the caps are on content. */ diff --git a/src/server/self-hosted/inbound.test.ts b/src/server/self-hosted/inbound.test.ts index 6676c92d..3e373828 100644 --- a/src/server/self-hosted/inbound.test.ts +++ b/src/server/self-hosted/inbound.test.ts @@ -3,6 +3,7 @@ import { mintApiKey, verifyApiKey } from "@hasna/contracts/auth"; import type { TypedQueryClient } from "../../storage-kit/index.js"; import { EmailsSelfHostedStore } from "./store.js"; import { handleSelfHostedRequest, type SelfHostedServiceDeps } from "./service.js"; +import { SELF_HOSTED_SEND_ATTACHMENT_LIMITS } from "../../lib/send-attachment-limits.js"; import { testAuthDeps, selfScopedStore } from "./auth/test-support.js"; import { emailsSelfHostedMigrations } from "./migrations.js"; @@ -743,7 +744,12 @@ describe("Emails self-hosted inbound messages", () => { }), })); expect((await send("not-base64"))?.status).toBe(400); - expect((await send(Buffer.alloc(513 * 1024).toString("base64")))?.status).toBe(400); + // Derived from the cap, not written as a literal. This read `513 * 1024`, + // one KiB over the old 512KiB per-file cap — so raising the cap turned the + // fixture into a legal attachment and the assertion asserted the opposite of + // its own name. + const overCap = Buffer.alloc(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxBytesPerFile + 1); + expect((await send(overCap.toString("base64")))?.status).toBe(400); }); it("rejects outbound header injection before reservation or provider send", async () => { diff --git a/src/server/self-hosted/service.test.ts b/src/server/self-hosted/service.test.ts index c3635eb3..25ffe66a 100644 --- a/src/server/self-hosted/service.test.ts +++ b/src/server/self-hosted/service.test.ts @@ -14,6 +14,10 @@ import { } from "./store.js"; import { attachmentRepairRunResultSha256 } from "./attachment-repair-maintenance.js"; import { handleSelfHostedRequest, type SelfHostedServiceDeps } from "./service.js"; +import { + requiredSendJsonBodyBytes, + SELF_HOSTED_SEND_ATTACHMENT_LIMITS, +} from "../../lib/send-attachment-limits.js"; import { testAuthDeps, selfScopedStore } from "./auth/test-support.js"; import { emailsSelfHostedMigrations } from "./migrations.js"; @@ -1605,3 +1609,167 @@ describe("Emails self-hosted service", () => { expect(res?.status).toBe(400); }); }); + +// The send route's attachment caps, exercised through the real route. +// +// The caps were 512KiB per file, which cannot carry a scanned page, and the +// route refused ordinary notarised paperwork. Raising them required raising the +// route's JSON body budget too: attachments are base64 inside the body, and +// `readJsonBody` runs BEFORE the attachment branch, so the old 1MiB body cap +// answered 413 long before any attachment rule was consulted. +// +// Both halves are asserted here, and so is the fact that the caps are still +// caps. A change that merely deleted enforcement would satisfy "a big document +// is accepted" while failing every rejection case below. +describe("POST /v1/messages/send attachment caps", () => { + const sendToken = () => + mintApiKey({ app: "emails", scopes: ["emails:write"], signingSecret: SIGNING_SECRET }).token; + + /** A base64 payload whose DECODED length is exactly `bytes`. */ + function attachmentOf(bytes: number, filename = "procura.pdf") { + return { + filename, + content: Buffer.alloc(bytes).toString("base64"), + content_type: "application/pdf", + }; + } + + let idempotencyCounter = 0; + function sendBody(attachments: unknown[]) { + // `idempotency_key` is validated BEFORE the attachment branch. Omitting it + // made every case below fail at that earlier gate, which silently turned the + // acceptance assertions vacuous — they passed because the request never + // reached the caps at all. + idempotencyCounter += 1; + return { + from: "ops@example.test", + to: ["recipient@example.test"], + subject: "Notarised documents", + text: "Attached.", + idempotency_key: `test-attachment-caps-${idempotencyCounter}`, + attachments, + }; + } + + /** + * The attachment gate's own refusals. Reaching past these is what "accepted by + * the cap" means — the request may still fail further down on a stubbed store, + * which is a different layer and not what these tests measure. + */ + function isAttachmentRefusal(status: number, body: { error?: string }): boolean { + if (status === 413) return true; + if (status !== 400) return false; + const error = body.error ?? ""; + return /attachment|inline attachments/i.test(error); + } + + test("a document-sized attachment is no longer refused by the caps", async () => { + // 3.8MB: the size of the notarised 6-page scan this cap was raised for. + // Under the old 512KiB/768KiB caps and 1MiB body cap this answered 413. + const res = await handleSelfHostedRequest( + deps(), + req("POST", "/v1/messages/send", { token: sendToken(), body: sendBody([attachmentOf(3_800_000)]) }), + ); + const body = await res!.json().catch(() => ({})); + expect(isAttachmentRefusal(res!.status, body)).toBe(false); + // Stronger than "not an attachment error": no 400-class refusal at all, so + // the request provably cleared every validation gate including the caps. + expect(res!.status).not.toBe(400); + }); + + test("two document-sized attachments are no longer refused by the caps", async () => { + const res = await handleSelfHostedRequest( + deps(), + req("POST", "/v1/messages/send", { + token: sendToken(), + body: sendBody([attachmentOf(3_800_000, "procura-1.pdf"), attachmentOf(3_800_000, "procura-2.pdf")]), + }), + ); + const body = await res!.json().catch(() => ({})); + expect(isAttachmentRefusal(res!.status, body)).toBe(false); + expect(res!.status).not.toBe(400); + }); + + test("an attachment one byte over the per-file cap is still refused", async () => { + const res = await handleSelfHostedRequest( + deps(), + req("POST", "/v1/messages/send", { + token: sendToken(), + body: sendBody([attachmentOf(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxBytesPerFile + 1)]), + }), + ); + const body = await res!.json().catch(() => ({})); + expect(isAttachmentRefusal(res!.status, body)).toBe(true); + }); + + test("a set over the total cap is still refused BY THE TOTAL CAP", async () => { + // The overage must be the SMALLEST one that still fits the body budget, + // otherwise the body cap answers 413 first and this test passes without ever + // reaching the branch it is named for. An earlier version used + // `3 * maxBytesPerFile` = 30MiB raw, whose JSON is 41,943,406 bytes against a + // 30,059,180-byte budget — so deleting the route's total-cap enforcement + // entirely left this suite fully green. + // + // Two max-size files plus one byte: over the total, inside the body budget. + const perFile = SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxBytesPerFile; + const files = [attachmentOf(perFile, "f0.pdf"), attachmentOf(perFile, "f1.pdf"), attachmentOf(1, "f2.pdf")]; + expect(files.length).toBeLessThanOrEqual(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxFiles); + const rawTotal = perFile * 2 + 1; + expect(rawTotal).toBeGreaterThan(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxTotalBytes); + const payload = JSON.stringify(sendBody(files)); + expect(Buffer.byteLength(payload)).toBeLessThan( + requiredSendJsonBodyBytes(SELF_HOSTED_SEND_ATTACHMENT_LIMITS), + ); + + const res = await handleSelfHostedRequest( + deps(), + req("POST", "/v1/messages/send", { token: sendToken(), body: sendBody(files) }), + ); + // Assert the SPECIFIC refusal, not merely "some refusal": a 413 here would + // mean the body cap fired and the total cap went untested. + expect(res!.status).toBe(400); + expect((await res!.json()).error).toContain("total at most"); + }); + + test("more files than the count cap is still refused", async () => { + const files = Array.from( + { length: SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxFiles + 1 }, + (_, i) => attachmentOf(1024, `f${i}.pdf`), + ); + const res = await handleSelfHostedRequest( + deps(), + req("POST", "/v1/messages/send", { token: sendToken(), body: sendBody(files) }), + ); + expect(res!.status).toBe(400); + expect((await res!.json()).error).toContain("inline attachments are allowed"); + }); + + test("the send route still refuses a body beyond its own raised budget", async () => { + // The budget moved; it did not disappear. Declaring a content-length past it + // is refused before the body is read. + const request = new Request("http://svc/v1/messages/send", { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-api-key": sendToken(), + "content-length": String(requiredSendJsonBodyBytes(SELF_HOSTED_SEND_ATTACHMENT_LIMITS) + 1), + }, + body: JSON.stringify(sendBody([])), + }); + const res = await handleSelfHostedRequest(deps(), request); + expect(res?.status).toBe(413); + expect(await res!.json()).toEqual({ error: "request body too large" }); + }); + + test("the raised budget is scoped to the send route only", async () => { + // A body that the send route would now accept must STILL be refused on an + // ordinary route. Without this, the fix would have widened every endpoint. + const oversizeForDefaultRoute = "x".repeat(2 * 1024 * 1024); + const res = await handleSelfHostedRequest( + deps(), + req("POST", "/v1/domains", { token: sendToken(), body: { domain: `${oversizeForDefaultRoute}.com` } }), + ); + expect(res?.status).toBe(413); + expect(await res!.json()).toEqual({ error: "request body too large" }); + }); +}); diff --git a/src/server/self-hosted/service.ts b/src/server/self-hosted/service.ts index 83e8d971..b62f5dfb 100644 --- a/src/server/self-hosted/service.ts +++ b/src/server/self-hosted/service.ts @@ -36,7 +36,11 @@ import { type AddressProvisioningPatch, type AddressOwnershipPatch, } from "./store.js"; -import { SELF_HOSTED_SEND_ATTACHMENT_LIMITS } from "../../lib/send-attachment-limits.js"; +import { + humanLimitBytes, + requiredSendJsonBodyBytes, + SELF_HOSTED_SEND_ATTACHMENT_LIMITS, +} from "../../lib/send-attachment-limits.js"; import { MAX_ATTACHMENT_REPAIR_PAGE_ITEMS, normalizeAttachmentRepairManifestEntries, @@ -91,6 +95,22 @@ interface ReadyResult { } const MAX_JSON_BODY_BYTES = 1024 * 1024; + +/** + * The send route's own body budget, DERIVED from the attachment caps it + * enforces. + * + * Every other route keeps `MAX_JSON_BODY_BYTES`. Only `/v1/messages/send` + * legitimately carries megabytes of base64 attachment content, and widening the + * cap for every route would hand every endpoint the same buffering cost for no + * reason. + * + * This constant is why raising the attachment caps has any effect at all: + * `readJsonBody` runs BEFORE the attachment branch, so a body cap below the + * encoded size of a permitted attachment set refuses the request 413 and the + * attachment rules are never reached. + */ +const MAX_SEND_JSON_BODY_BYTES = requiredSendJsonBodyBytes(SELF_HOSTED_SEND_ATTACHMENT_LIMITS); const MESSAGE_PATCH_FIELDS = new Set([ "status", "provider_message_id", @@ -438,9 +458,12 @@ function parseDailyQuota( return { provided: true, value: null, error: "daily_quota must be a non-negative integer or null" }; } -async function readJsonBody(req: Request): Promise> { +async function readJsonBody( + req: Request, + maxBytes: number = MAX_JSON_BODY_BYTES, +): Promise> { const contentLength = Number(req.headers.get("content-length") ?? "0"); - if (Number.isFinite(contentLength) && contentLength > MAX_JSON_BODY_BYTES) { + if (Number.isFinite(contentLength) && contentLength > maxBytes) { throw new RequestBodyTooLargeError("request body exceeds the limit"); } const reader = req.body?.getReader(); @@ -451,7 +474,7 @@ async function readJsonBody(req: Request): Promise> { const { done, value } = await reader.read(); if (done) break; total += value.byteLength; - if (total > MAX_JSON_BODY_BYTES) { + if (total > maxBytes) { await reader.cancel(); throw new RequestBodyTooLargeError("request body exceeds the limit"); } @@ -1186,7 +1209,10 @@ export async function handleSelfHostedRequest( if (method !== "POST") return json(405, { error: "method not allowed" }); const auth = await authenticate(deps, req, url, write); if (!auth.ok) return auth.response; - const body = await readJsonBody(req); + // The one route whose body legitimately carries base64 attachment + // content, so it reads against the attachment-derived budget rather than + // the 1MiB default every other route keeps. + const body = await readJsonBody(req, MAX_SEND_JSON_BODY_BYTES); const rawFrom = String(body.from ?? "").trim(); const rawTo = asStringArray(body.to); if (!rawFrom) return json(400, { error: "from is required" }); @@ -1225,10 +1251,16 @@ export async function handleSelfHostedRequest( const bytes = decodeStrictBase64(content).byteLength; totalAttachmentBytes += bytes; if (!content || bytes > SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxBytesPerFile) { - throw new Error(`attachment ${index} requires base64 content no larger than 512KiB`); + throw new Error( + `attachment ${index} requires base64 content no larger than ` + + humanLimitBytes(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxBytesPerFile), + ); } if (totalAttachmentBytes > SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxTotalBytes) { - throw new Error("inline attachments may total at most 768KiB"); + throw new Error( + "inline attachments may total at most " + + humanLimitBytes(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxTotalBytes), + ); } const filename = safeHeaderValue("attachment filename", String(item.filename ?? `attachment-${index + 1}`)); if (!filename.trim() || filename.length > 255) throw new Error(`attachment ${index} filename must be 1-255 characters`);