From 121d2d5e31cabd36325a9525c5f495ed89cd5b10 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Mon, 26 Oct 2026 14:32:27 +0100 Subject: [PATCH] fix: stop buffering up to 100 MB of every unauthenticated request body before routing --- app/api/webdav/route.ts | 7 +- app/api/wopi/files/[fileId]/contents/route.ts | 13 +++- lib/__tests__/proxy-matcher.test.ts | 76 +++++++++++++++++++ lib/__tests__/webdav-route.test.ts | 9 +++ lib/__tests__/wopi-file-routes.test.ts | 17 +++++ lib/plugin-secrets/proxy.ts | 6 +- lib/security/capped-body.ts | 4 +- next.config.ts | 21 +++-- proxy.ts | 23 +++++- 9 files changed, 156 insertions(+), 20 deletions(-) create mode 100644 lib/__tests__/proxy-matcher.test.ts diff --git a/app/api/webdav/route.ts b/app/api/webdav/route.ts index 2ef3bed43..5c40f416f 100644 --- a/app/api/webdav/route.ts +++ b/app/api/webdav/route.ts @@ -4,10 +4,12 @@ import { logger } from '@/lib/logger'; import { getStalwartCredentials } from '@/lib/stalwart/credentials'; import { fetchJmapServer } from '@/lib/stalwart/server-fetch'; import { DisallowedUrlError } from '@/lib/security/url-guard'; -import { UpstreamBodyTooLargeError, readTextCapped } from '@/lib/security/capped-body'; +import { UpstreamBodyTooLargeError, readBodyCapped, readTextCapped } from '@/lib/security/capped-body'; /** A PROPFIND listing of a folder with many thousand entries still fits. */ const MAX_PROPFIND_RESPONSE_BYTES = 50 * 1024 * 1024; +/** PROPFIND and MKCALENDAR bodies are a few lines of XML; proxy.ts doesn't run here to cap them. */ +const MAX_XML_REQUEST_BYTES = 1024 * 1024; const ALLOWED_METHODS = new Set(['PROPFIND', 'MKCOL', 'MKCALENDAR', 'GET', 'PUT', 'DELETE', 'MOVE', 'COPY']); @@ -106,7 +108,8 @@ export async function POST(request: NextRequest) { // PROPFIND and MKCALENDAR bodies are small XML and are read fully. let body: ArrayBuffer | ReadableStream | null = null; if (method === 'PROPFIND' || method === 'MKCALENDAR') { - body = await request.arrayBuffer(); + body = await readBodyCapped(request, MAX_XML_REQUEST_BYTES); + if (!body) return NextResponse.json({ error: 'Request body too large' }, { status: 413 }); } else if (method === 'PUT') { body = request.body; } diff --git a/app/api/wopi/files/[fileId]/contents/route.ts b/app/api/wopi/files/[fileId]/contents/route.ts index 750a2ea8c..d60ddf0ed 100644 --- a/app/api/wopi/files/[fileId]/contents/route.ts +++ b/app/api/wopi/files/[fileId]/contents/route.ts @@ -2,6 +2,14 @@ import { NextRequest, NextResponse } from 'next/server'; import { logger } from '@/lib/logger'; import { downloadFileBlob, getWopiFileNode, updateFileNodeBlob, uploadFileBlob } from '@/lib/wopi/files'; import { wopiContext } from '@/lib/wopi/request'; +import { readBodyCapped } from '@/lib/security/capped-body'; + +/** + * Largest document PutFile takes. proxy.ts doesn't run for this route, so + * nothing else caps the copy held in memory; past it the save is refused, not + * cut short. Stalwart's own upload limit (maxSizeUpload) is usually lower. + */ +const MAX_PUT_FILE_BYTES = 100 * 1024 * 1024; /** * WOPI GetFile / PutFile (#425). Content flows purely over JMAP: GetFile @@ -111,7 +119,10 @@ export async function POST( } } - const body = await request.arrayBuffer(); + const body = await readBodyCapped(request, MAX_PUT_FILE_BYTES); + if (!body) { + return NextResponse.json({ error: 'Document too large' }, { status: 413 }); + } if (body.byteLength === 0) { return NextResponse.json({ error: 'Empty document rejected' }, { status: 409 }); } diff --git a/lib/__tests__/proxy-matcher.test.ts b/lib/__tests__/proxy-matcher.test.ts new file mode 100644 index 000000000..046569632 --- /dev/null +++ b/lib/__tests__/proxy-matcher.test.ts @@ -0,0 +1,76 @@ +/** + * @vitest-environment node + * + * Next copies the body of every request proxy.ts runs for, up to + * proxyClientMaxBodySize, before the route sees it. That limit stays at the + * 10 MB default; the upload routes that need more are left out of the + * proxy's matcher. The matcher is read from proxy.ts the way `next build` + * reads it (static extraction), and a path counts as proxied the way Next's + * router decides it: when the raw or the decoded path matches. + */ +import path from 'node:path'; +import { describe, it, expect, vi } from 'vitest'; +import type { NextConfig } from 'next'; +import { loadBindings } from 'next/dist/build/swc'; +import { getPagesPageStaticInfo } from 'next/dist/build/analysis/get-page-static-info'; +import { getMiddlewareRouteMatcher } from 'next/dist/shared/lib/router/utils/middleware-route-matcher'; +import type { PAGE_TYPES } from 'next/dist/lib/page-types'; + +async function proxied(basePath = '') { + await loadBindings(); + const info = await getPagesPageStaticInfo({ + pageFilePath: path.resolve(__dirname, '../../proxy.ts'), + nextConfig: { basePath }, + isDev: false, + page: '/proxy', + pageType: 'pages' as PAGE_TYPES, + }); + const matchers = info.middleware?.matchers; + expect(matchers?.length).toBeGreaterThan(0); + const match = getMiddlewareRouteMatcher(matchers!); + const req = { headers: {} } as Parameters[1]; + return (p: string) => { + let decoded = p; + try { decoded = decodeURIComponent(p); } catch { /* as Next: keep it */ } + return match(p, req, {}) || match(decoded, req, {}); + }; +} + +describe('proxy matcher', () => { + it('leaves out exactly the upload routes', async () => { + const runs = await proxied(); + for (const p of ['/api/webdav', '/api/plugin-proxy', '/api/wopi/files/f123/contents', '/api/wopi/files/a%20b/contents']) { + expect(runs(p), p).toBe(false); + } + }); + + it('runs for every other path, including near misses and other spellings', async () => { + const runs = await proxied(); + for (const p of [ + '/', '/en/mail', '/admin', '/setup', '/api/settings', '/api/auth/session', '/api/account/stalwart/jmap', + '/api/webdav/', '/api/webdav/x', '/api/webdavx', '/api/plugin-proxy/x', '/api/plugin-proxy2', + '/api/wopi/files/f123', '/api/wopi/files/f123/contents/x', '/api/wopi/launch', '/api/wopi/files//contents', + '/api/%77ebdav', '/api/wopi/files/a%2Fb/contents', '/x/api/webdav', '/_next/static/chunk.js', + ]) expect(runs(p), p).toBe(true); + }); + + it('applies under a base path', async () => { + const runs = await proxied('/webmail'); + expect(runs('/webmail/api/webdav')).toBe(false); + expect(runs('/webmail/api/settings')).toBe(true); + }); + + it('keeps the body limit at the default and gives the left-out routes the proxy\'s API headers', async () => { + vi.resetModules(); + const { default: nextConfig } = (await import('../../next.config')) as { default: NextConfig }; + expect(nextConfig.experimental?.proxyClientMaxBodySize).toBeUndefined(); + const rules = (await nextConfig.headers?.()) ?? []; + for (const source of ['/api/webdav', '/api/plugin-proxy', '/api/wopi/files/:fileId/contents']) { + const headers = rules.find((rule) => rule.source === source)?.headers ?? []; + expect(headers, source).toEqual(expect.arrayContaining([ + { key: 'X-Content-Type-Options', value: 'nosniff' }, + { key: 'X-Frame-Options', value: 'DENY' }, + ])); + } + }); +}); diff --git a/lib/__tests__/webdav-route.test.ts b/lib/__tests__/webdav-route.test.ts index bb5c64033..52cfe7703 100644 --- a/lib/__tests__/webdav-route.test.ts +++ b/lib/__tests__/webdav-route.test.ts @@ -114,6 +114,15 @@ describe('POST /api/webdav — proxying', () => { expect(res.headers!.get('Content-Type')).toBe('application/xml; charset=utf-8'); }); + it('PROPFIND refuses a request body over 1 MB without sending it on', async () => { + const big = endlessResponse(50); + const req = { ...makeReq({ 'X-WebDAV-Method': 'PROPFIND', 'X-WebDAV-Path': 'dir' }), body: big.response.body }; + const res = read(await POST(req as unknown as Parameters[0])); + expect(res.status).toBe(413); + expect(big.pulled()).toBeLessThan(5); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + it('PROPFIND answers 502 instead of buffering a listing that never ends', async () => { const { response, pulled } = endlessResponse(500, { status: 207 }); fetchSpy.mockResolvedValueOnce(response); diff --git a/lib/__tests__/wopi-file-routes.test.ts b/lib/__tests__/wopi-file-routes.test.ts index 8518a69ec..0d5eb11f1 100644 --- a/lib/__tests__/wopi-file-routes.test.ts +++ b/lib/__tests__/wopi-file-routes.test.ts @@ -44,6 +44,7 @@ import { fetchJmapServer } from '@/lib/stalwart/server-fetch'; import { fetchJmapSession, postJmap } from '@/lib/stalwart/jmap-api'; import { shareeSavesNeedRename, updateFileNodeBlob } from '@/lib/wopi/files'; import { mintWopiToken, wopiDocumentId } from '@/lib/wopi/token'; +import { endlessResponse } from './helpers/endless-body'; process.env.ADMIN_STATE_DIR = mkdtempSync(path.join(tmpdir(), 'bw-wopi-files-')); @@ -263,6 +264,22 @@ describe('WOPI PutFile for a Files node', () => { expect(setCalls()).toEqual([]); }); + it('refuses a document over the size limit instead of saving part of it', async () => { + const { data } = await launchFile({ fileId: 'f1' }); + const call = editorCall(data); + const big = endlessResponse(200); + const req = { + ...request(`/api/wopi/files/${call.documentId}/contents?${call.token}`, { headers: { 'X-WOPI-Override': 'PUT' } }), + body: big.response.body, + } as unknown as Parameters[0]; + + const res = await putFile(req, params(call.documentId)); + expect(res.status).toBe(413); + expect(big.pulled()).toBeLessThan(110); + expect(uploads()).toEqual([]); + expect(setCalls()).toEqual([]); + }); + it('turns a refused update into a server error', async () => { const { data } = await launchFile({ fileId: 'f1' }); (postJmap as Mock).mockImplementation(async (_url: string, _auth: string, body: string) => { diff --git a/lib/plugin-secrets/proxy.ts b/lib/plugin-secrets/proxy.ts index d703a35bf..26de61aa9 100644 --- a/lib/plugin-secrets/proxy.ts +++ b/lib/plugin-secrets/proxy.ts @@ -13,7 +13,7 @@ import { authHeaderFor, credentialOrigin, privateProxyOrigins, type CredentialIn import { getUserCredential, setUserCredentialSecret, userKey, type VaultUser } from './vault'; import { declaredUserCredential } from './known-plugins'; -/** Largest request body the proxy forwards (one upload chunk); see proxyClientMaxBodySize in next.config.ts. */ +/** Largest request body the proxy forwards (one upload chunk). */ export const MAX_PROXY_REQUEST_BYTES = 100 * 1024 * 1024; /** Largest response body it passes back (a download). */ export const MAX_PROXY_RESPONSE_BYTES = 2 * 1024 * 1024 * 1024; @@ -306,8 +306,8 @@ export async function proxyRequest(input: ProxyInput): Promise { const headers = forwardHeaders(input.headers, credential); // Read the whole body first and send it with a Content-Length. Streamed // (chunked) uploads reach some servers empty: Paperless (Django) answers - // "No file was submitted". Next has buffered the body for proxy.ts - // already, up to the same limit (see proxyClientMaxBodySize). + // "No file was submitted". proxy.ts doesn't run for /api/plugin-proxy, + // so this is the only copy, and the only limit. let body = input.body && method !== 'GET' && method !== 'HEAD' ? await readBody(input.body, MAX_PROXY_REQUEST_BYTES) : null; diff --git a/lib/security/capped-body.ts b/lib/security/capped-body.ts index 3c5ddd046..69d25561a 100644 --- a/lib/security/capped-body.ts +++ b/lib/security/capped-body.ts @@ -21,10 +21,10 @@ export class UpstreamBodyTooLargeError extends Error { /** * The body as bytes, or null once it exceeds `maxBytes`, in which case the - * upstream connection is aborted rather than drained. + * upstream connection is aborted rather than drained. Takes a request too. */ export async function readBodyCapped( - response: PublicFetchResponse | Response, + response: PublicFetchResponse | Response | Request, maxBytes: number, controller?: AbortController, ): Promise { diff --git a/next.config.ts b/next.config.ts index 75a2d03bd..f0572a81b 100644 --- a/next.config.ts +++ b/next.config.ts @@ -61,14 +61,10 @@ const nextConfig: NextConfig = { // it from node_modules at runtime instead of trying to bundle it. Used by // PLUGIN_DEV_DIR's on-the-fly bundler. serverExternalPackages: ["esbuild"], - experimental: { - // proxy.ts runs for every request, /api included, and Next buffers the - // body for it up to this size and silently cuts off the rest. 10 MB (the - // default) truncated WOPI saves and WebDAV uploads of bigger files, and - // plugin uploads through /api/plugin-proxy. Keep MAX_PROXY_REQUEST_BYTES - // in lib/plugin-secrets/proxy.ts in step with it. - proxyClientMaxBodySize: "100mb", - }, + // No proxyClientMaxBodySize: Next copies every body proxy.ts runs for up to + // that size before routing, so it stays at the 10 MB default. The routes + // that take bigger uploads (WOPI saves, WebDAV, /api/plugin-proxy) are left + // out of the proxy's matcher instead. // Response headers are the host's job in a static export (see out/_headers). ...(isLite ? {} @@ -91,6 +87,15 @@ const nextConfig: NextConfig = { { key: "Cache-Control", value: "public, max-age=31536000, immutable" }, ], }, + // The upload routes proxy.ts's matcher leaves out, with the + // headers it sets on every other API response. + ...["/api/webdav", "/api/plugin-proxy", "/api/wopi/files/:fileId/contents"].map((source) => ({ + source, + headers: [ + { key: "X-Content-Type-Options", value: "nosniff" }, + { key: "X-Frame-Options", value: "DENY" }, + ], + })), ]; }, }), diff --git a/proxy.ts b/proxy.ts index 2b28e284a..22860f4c2 100644 --- a/proxy.ts +++ b/proxy.ts @@ -49,12 +49,27 @@ function withMatchedChineseAcceptLanguage(request: NextRequest): NextRequest { return new NextRequest(request, { headers, nextConfig: basePath ? { basePath } : undefined }); } -// Next 16's Proxy always runs on Node.js runtime and route-segment config -// (e.g. `export const config = { matcher }`) is no longer allowed in the -// proxy file. We replicate the previous matcher inline by short-circuiting -// requests for API routes, Next internals and static assets. +// Next 16's Proxy always runs on Node.js runtime (a `runtime` in the config +// below is refused). The matcher only leaves out the upload routes; API +// routes, Next internals and static assets still pass through here and are +// short-circuited inline after the setup and CSRF gates. const PROXY_SKIP_PATTERN = /^\/(?:api|_next)(?:\/|$)/; +/** + * Routes the proxy does not run for: the ones that take uploads over 10 MB. + * For every request it runs for, Next first copies the body for it, up to + * proxyClientMaxBodySize (10 MB) and silently cutting off the rest; raising + * that limit made any unauthenticated POST hold up to it in memory. These + * handlers gate cross-origin requests themselves (the WOPI file routes are + * token-authed) and cap what they read. Next tests the raw and the decoded + * path; the proxy is skipped only when both are one of these exactly. + * next.config.ts gives them the headers the proxy would have set. + * A literal: Next reads it at build time. + */ +export const config = { + matcher: ["/((?!api/webdav$|api/plugin-proxy$|api/wopi/files/[^/]+/contents$).*)"], +}; + // What Next serves from public/: a single dotted segment at the root // (/sw.js, /favicon.ico, /manifest.webmanifest) and the branding/, // notification/, demo/ and plugin-sandbox-assets/ asset folders. The upstream matcher treated EVERY path whose