From 1325d04cec872192f7fc54dd168ff5a8e3cd18fd Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Sat, 8 Aug 2026 19:28:20 +0300 Subject: [PATCH 1/2] fix(send): raise self-hosted attachment caps to document size, and the body budget with them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 512KiB per file cannot carry a scanned page, so the self-hosted send route refused ordinary notarised paperwork. Raise the caps to 5 files / 10MiB per file / 20MiB total. Raising them alone would have changed nothing. Attachments travel base64 inside the JSON send body, and base64(768KiB) is exactly 1MiB — the route's MAX_JSON_BODY_BYTES. The old total cap WAS the body cap, expressed in raw bytes. readJsonBody also runs before the attachment branch, so an oversize document was refused 413 before any attachment rule was consulted. So the body budget moves too, and is DERIVED from the attachment caps (requiredSendJsonBodyBytes) rather than hand-typed, which is what stops the two drifting apart again. The larger budget is scoped to /v1/messages/send; every other route keeps the unchanged 1MiB default, and it is only reachable after authenticate() has run. SES is the ceiling above this: SESv2 with raw content accepts 40MB after base64 (the v1 API's figure is 10MB). Worst case under the new caps is ~30.8MB encoded, leaving ~9.2MB margin. SES_MAX_MESSAGE_BYTES and mimeEncodedUpperBound put that ceiling in code with a test instead of leaving it tribal knowledge. Two rejection strings that read "512KiB" and "768KiB" as literals now render from the constants, and test fixtures that hardcoded sizes chosen against the old caps now derive from them — those literals had silently inverted what several tests asserted. Agent: agent-chief-operations --- src/lib/send-attachment-limits.test.ts | 160 +++++++++++++++++++++++++ src/lib/send-attachment-limits.ts | 86 ++++++++++++- src/lib/send-preflight.test.ts | 44 +++++-- src/lib/send-preflight.ts | 8 +- src/server/self-hosted/inbound.test.ts | 8 +- src/server/self-hosted/service.test.ts | 153 +++++++++++++++++++++++ src/server/self-hosted/service.ts | 46 +++++-- 7 files changed, 475 insertions(+), 30 deletions(-) create mode 100644 src/lib/send-attachment-limits.test.ts 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..e3bdbb70 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,152 @@ 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", async () => { + // Each file is legal on its own; together they exceed the total. + const perFile = SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxBytesPerFile; + const count = Math.ceil(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxTotalBytes / perFile) + 1; + expect(count).toBeLessThanOrEqual(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxFiles); + const files = Array.from({ length: count }, (_, i) => attachmentOf(perFile, `f${i}.pdf`)); + const res = await handleSelfHostedRequest( + deps(), + req("POST", "/v1/messages/send", { token: sendToken(), body: sendBody(files) }), + ); + const body = await res!.json().catch(() => ({})); + expect(isAttachmentRefusal(res!.status, body)).toBe(true); + }); + + 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`); From 60259c70ed73f96c5468af00f87851c4d191c437 Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Sat, 8 Aug 2026 19:52:03 +0300 Subject: [PATCH 2/2] test(send): make the total-attachment-cap test actually able to fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review (correctness lens, PR #216) found this by mutation: deleting the route's total-cap enforcement outright left the suite at 103 pass / 0 fail. The fixture built 3 x maxBytesPerFile = 30MiB raw, whose JSON is 41,943,406 bytes against a 30,059,180-byte body budget. So the BODY cap answered 413 first and the assertion — which accepted any 413 as an attachment refusal — passed without ever reaching the total-cap branch it was named for. Use the smallest overage that still fits the body budget (two max-size files plus one byte), assert the specific 400 and its message rather than "some refusal", and assert the preconditions so the fixture cannot silently drift back over the budget. Verified both ways: passes normally (45 pass / 0 fail), and with the route's total-cap enforcement deleted it now FAILS (44 pass / 1 fail) where it previously stayed green. Production code is unchanged by this commit. Agent: agent-chief-operations --- src/server/self-hosted/service.test.ts | 29 +++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/server/self-hosted/service.test.ts b/src/server/self-hosted/service.test.ts index e3bdbb70..25ffe66a 100644 --- a/src/server/self-hosted/service.test.ts +++ b/src/server/self-hosted/service.test.ts @@ -1702,18 +1702,33 @@ describe("POST /v1/messages/send attachment caps", () => { expect(isAttachmentRefusal(res!.status, body)).toBe(true); }); - test("a set over the total cap is still refused", async () => { - // Each file is legal on its own; together they exceed the total. + 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 count = Math.ceil(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxTotalBytes / perFile) + 1; - expect(count).toBeLessThanOrEqual(SELF_HOSTED_SEND_ATTACHMENT_LIMITS.maxFiles); - const files = Array.from({ length: count }, (_, i) => attachmentOf(perFile, `f${i}.pdf`)); + 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) }), ); - const body = await res!.json().catch(() => ({})); - expect(isAttachmentRefusal(res!.status, body)).toBe(true); + // 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 () => {