fix(plugin): cap origin response headers at 64 KiB, not undici's 16 KiB default; v0.30.0

packages/plugin/package.json

@@ -1,6 +1,6 @@
 {
 	"name": "@harperfast/prerender",
-	"version": "0.28.0",
+	"version": "0.30.0",
 	"type": "module",
 	"description": "Configurable Harper plugin for prerendering pages for bots and crawlers",
 	"license": "Apache-2.0",

packages/plugin/src/configSchema.js

@@ -249,6 +249,27 @@ export const configSchema = group('Prerender plugin configuration.', {
 				'authorization, and the security-token/debug header names). Matched case-insensitively.',
 			{ movedFrom: 'ignoredHeaders', itemType: 'string' }
 		),
+		maxResponseHeaderBytes: option(
+			64 * 1024,
+			'Largest response head Harper will accept from the origin, summed across every header name ' +
+				'and value in the response (not per header).\n\n' +
+				'Undici defaults this to Node’s `http.maxHeaderSize` (16 KiB), which is a header-flood ' +
+				'mitigation for servers accepting untrusted requests — too strict for a reverse proxy reading ' +
+				'its own origin. A real origin can exceed 16 KiB on a single page (several Set-Cookie plus ' +
+				'CSP, Link rel=preload, NEL, Report-To), and undici responds by destroying the connection ' +
+				'with UND_ERR_HEADERS_OVERFLOW, so the crawler gets a 500 for a page browsers and the CDN ' +
+				'load normally. It fails deterministically for those URLs, since it is a property of the ' +
+				'origin’s response rather than a transient. Hence a default well above Node’s, matching what ' +
+				'a CDN in front of the same origin already tolerates.\n\n' +
+				'Raising it raises the worst-case memory held per connection while a response head is ' +
+				'parsed, which is why it is bounded at both ends. The 1 MiB ceiling is far above any ' +
+				'legitimate response head — it exists to catch a typo (a stray factor of a thousand) ' +
+				'before it becomes an out-of-memory risk multiplied across concurrent connections.\n\n' +
+				'Restart-scoped: undici fixes `maxHeaderSize` when the dispatcher is constructed and offers ' +
+				'no way to change it afterwards, so a live edit is reported as pending-restart and the ' +
+				'running dispatchers keep the value they were built with.',
+			{ unit: 'bytes', min: 16 * 1024, max: 1024 * 1024, scope: 'restart' }
+		),
 	}),
 
 	debugHeader: group('Debug response headers, emitted when the request carries this header (any value).', {

packages/plugin/src/util/upstream.js

@@ -3,8 +3,6 @@ import { isIP } from 'node:net';
 import { Agent } from 'undici';
 import { config } from '../config.js';
 
-const agent = new Agent({});
-
 /**
  * The staging IP to connect to for this origin fetch, or undefined for a normal fetch.
  * Staging passthrough is active only when a staging `ip` is configured (and valid) AND
@@ -28,19 +26,42 @@ export const configuredStagingIp = () => {
 	return ip && isIP(ip) ? ip : undefined;
 };
 
-// Dispatchers that pin DNS resolution to a fixed IP (staging passthrough), one per IP.
-// Only the connect address is overridden — the origin (so Host header + TLS SNI + cert
-// validation) stays the real origin host, the server-side equivalent of Chrome's
-// --host-resolver-rules=MAP host ip. In practice there is at most one entry (the single
-// configured staging IP); the map just keeps it stable across requests and across a
-// config reload that changes the IP.
+// `maxHeaderSize` is fixed at Agent construction — undici exposes no way to change it on a live
+// Agent — so `origin.maxResponseHeaderBytes` is restart-scoped: config.js reports a live change
+// as pending-restart and the running dispatchers keep the value they were built with. Without it
+// undici falls back to Node's http.maxHeaderSize (16 KiB), which a real origin can exceed on a
+// single page (a Set-Cookie pile-up plus CSP/Link-preload is enough), and undici answers by
+// DESTROYING THE SOCKET with UND_ERR_HEADERS_OVERFLOW. The crawler then gets a 500 for a page
+// browsers and the CDN load fine, deterministically, because it is a property of that response.
+// Captured on first use and reused by every dispatcher built afterwards, so restart scope holds
+// for all of them. Re-reading config per construction would not: `origin.staging.ip` is
+// live-scoped, so a pinned dispatcher can be built long after boot, and it would then pick up a
+// cap edited in the meantime while the unpinned singleton kept the boot value — two dispatchers
+// disagreeing, and a pending-restart notice that was only half true.
+let capturedMaxHeaderSize;
+const agentOptions = () => ({
+	maxHeaderSize: (capturedMaxHeaderSize ??= config.origin.maxResponseHeaderBytes),
+});
+
+// The unpinned dispatcher carries every cache-miss and passthrough fetch, so it stays a plain
+// lazily-built singleton: one `??=` test on the hot path, no key to build and no Map to probe.
+// It cannot be built at import time because the cap is not known until the component applies
+// its options; by the first origin fetch it always is.
+let agent;
+
+// Dispatchers that pin DNS resolution to a fixed IP (staging passthrough), one per IP. Only the
+// connect address is overridden — the origin (so Host header + TLS SNI + cert validation) stays
+// the real origin host, the server-side equivalent of Chrome's --host-resolver-rules=MAP host ip.
+// In practice there is at most one entry (the single configured staging IP); the map just keeps
+// it stable across requests and across a config reload that changes the IP.
 const pinnedDispatchers = new Map();
 export const dispatcherFor = (ip) => {
-	if (!ip) return agent;
+	if (!ip) return (agent ??= new Agent(agentOptions()));
 	let dispatcher = pinnedDispatchers.get(ip);
 	if (!dispatcher) {
 		const family = isIP(ip);
 		dispatcher = new Agent({
+			...agentOptions(),
 			connect: {
 				// Node's lookup callback has two shapes depending on the `all` option.
 				lookup: (_hostname, options, callback) =>

packages/plugin/test/config.test.js

@@ -203,7 +203,11 @@ test('defaultConfig returns fresh deep copies (no shared references)', () => {
 
 test('secret and restart paths are what the schema declares', () => {
 	assert.deepEqual(secretPaths().sort(), ['origin.securityToken.value', 'renderNow.token']);
-	assert.deepEqual(restartPaths().sort(), ['render.reconcile.startDelay', 'render.reconcile.startJitter']);
+	assert.deepEqual(restartPaths().sort(), [
+		'origin.maxResponseHeaderBytes',
+		'render.reconcile.startDelay',
+		'render.reconcile.startJitter',
+	]);
 });
 
 test('describeConfigSchema is JSON-serializable and carries the editor contract', () => {

packages/plugin/test/upstream.test.js

@@ -1,8 +1,11 @@
 import { test } from 'node:test';
 import assert from 'node:assert/strict';
+import http from 'node:http';
 import { applyOptions, config } from '../src/config.js';
+import { restartPaths } from '../src/configSchema.js';
 import {
 	configuredStagingIp,
+	dispatcherFor,
 	resolveUpstreamHeaders,
 	sanitizeOriginResponseHeaders,
 	stagingTargetIp,
@@ -194,3 +197,160 @@ test('resolveUpstreamHeaders picks up ignoredHeaders changes across applyOptions
 	assert.equal(upstream['x-first'], 'a');
 	assert.equal(upstream['x-second'], undefined);
 });
+
+// --- origin response-header cap -------------------------------------------------------------
+//
+// Asserted behaviorally against a real server rather than by reading undici's internal
+// kMaxHeadersSize symbol, so the tests survive an undici refactor and actually prove the thing
+// that broke in production: a large-but-legitimate origin response head must not kill the request.
+
+// Serve a response whose head sums to `bytes` across many headers — the shape a real origin
+// produces (a Set-Cookie pile-up plus CSP/Link-preload), since the cap is cumulative over the
+// whole head, not per header.
+//
+// undici counts header NAME and VALUE bytes (Parser.onHeaderField / onHeaderValue each call
+// trackHeader with their own buffer length) and not the `: ` / CRLF delimiters, so budgeting
+// `name.length + value.length` per header is exactly what the cap sees.
+const HEADER_BYTES = 1024;
+const serverWithHeadBytes = async (bytes) => {
+	const server = http.createServer((_req, res) => {
+		const headers = {};
+		for (let i = 0; i < Math.ceil(bytes / HEADER_BYTES); i++) {
+			const name = `x-pad-${i}`;
+			headers[name] = 'a'.repeat(HEADER_BYTES - name.length);
+		}
+		res.writeHead(200, headers);
+		res.end('ok');
+	});
+	// Reject on a listen error rather than leaving the await to hang forever (EADDRINUSE, or a
+	// sandbox that forbids binding) — a hung test is far harder to diagnose than a failed one.
+	await new Promise((resolve, reject) => {
+		server.once('error', reject);
+		server.listen(0, '127.0.0.1', resolve);
+	});
+	return { server, origin: `http://127.0.0.1:${server.address().port}` };
+};
+
+const withServer = async (bytes, fn) => {
+	const { server, origin } = await serverWithHeadBytes(bytes);
+	try {
+		return await fn(origin);
+	} finally {
+		// close() is async; awaiting it keeps a lingering handle from leaking into the next test.
+		await new Promise((resolve) => server.close(resolve));
+	}
+};
+
+test('a 32 KiB origin response head succeeds under the default cap', async () => {
+	applyOptions({});
+	assert.equal(config.origin.maxResponseHeaderBytes, 64 * 1024);
+	// The whole point: the default must clear Node's http.maxHeaderSize, which is what undici
+	// falls back to and what produced UND_ERR_HEADERS_OVERFLOW -> 500 for the crawler.
+	assert.ok(config.origin.maxResponseHeaderBytes > http.maxHeaderSize);
+
+	await withServer(32 * 1024, async (origin) => {
+		const res = await dispatcherFor(undefined).request({ origin, path: '/', method: 'GET' });
+		assert.equal(res.statusCode, 200);
+		await res.body.text();
+	});
+});
+
+test('the cap is genuinely enforced (a head above it still overflows)', async () => {
+	// Proves the option is wired to undici rather than merely stored. A fresh module instance is
+	// needed because the unpinned dispatcher is built once per process — which is the restart
+	// scope, asserted below. The query string gives a distinct module URL; config.js resolves to
+	// the same URL either way, so the singleton config this sets is what the fresh module reads.
+	applyOptions({ origin: { maxResponseHeaderBytes: 16 * 1024 } });
+	const fresh = await import('../src/util/upstream.js?fresh=low-cap');
+	try {
+		await withServer(32 * 1024, async (origin) => {
+			await assert.rejects(
+				() => fresh.dispatcherFor(undefined).request({ origin, path: '/', method: 'GET' }),
+				(err) => err.code === 'UND_ERR_HEADERS_OVERFLOW'
+			);
+		});
+	} finally {
+		applyOptions({});
+	}
+});
+
+test('the unpinned dispatcher is built once and ignores a live cap change', async () => {
+	// Both halves of restart scope: the hot path must not rebuild per request (efficiency), and a
+	// live edit must not silently take effect (correctness — config.js reports pending-restart).
+	applyOptions({});
+	const first = dispatcherFor(undefined);
+	assert.equal(dispatcherFor(undefined), first);
+
+	applyOptions({ origin: { maxResponseHeaderBytes: 128 * 1024 } });
+	assert.equal(dispatcherFor(undefined), first, 'a live cap change must not swap the dispatcher');
+
+	// ...and it still honors the cap it was constructed with, not the newly configured one.
+	await withServer(32 * 1024, async (origin) => {
+		const res = await first.request({ origin, path: '/', method: 'GET' });
+		assert.equal(res.statusCode, 200);
+		await res.body.text();
+	});
+	applyOptions({});
+});
+
+test('maxResponseHeaderBytes is declared restart-scoped', () => {
+	// Guards the scope declaration itself: dropping it would make the option look live while the
+	// running dispatcher quietly kept the old cap.
+	assert.ok(restartPaths().includes('origin.maxResponseHeaderBytes'));
+});
+
+test('a dispatcher built after a live cap edit still uses the captured cap', async () => {
+	// origin.staging.ip IS live-scoped, so enabling staging mints a pinned dispatcher long after
+	// boot. If that construction re-read config it would pick up a cap edited in the meantime
+	// while the unpinned singleton kept the boot value — two dispatchers disagreeing, and a
+	// pending-restart notice that was only half true. The cap is captured once instead.
+	// A fresh module instance so the pinned entry is genuinely built here rather than reused from
+	// an earlier test, and so the capture starts unset.
+	applyOptions({});
+	const fresh = await import('../src/util/upstream.js?fresh=capture-once');
+	fresh.dispatcherFor(undefined); // force the capture at the default
+
+	applyOptions({ origin: { maxResponseHeaderBytes: 16 * 1024, staging: { ip: '127.0.0.1' } } });
+	const pinnedAfterEdit = fresh.dispatcherFor('127.0.0.1');
+
+	// Built after the edit, but still honors the captured 64 KiB — a 32 KiB head must pass. Were
+	// it reading config at construction it would have taken the 16 KiB cap and overflowed.
+	await withServer(32 * 1024, async (origin) => {
+		const res = await pinnedAfterEdit.request({ origin, path: '/', method: 'GET' });
+		assert.equal(res.statusCode, 200);
+		await res.body.text();
+	});
+	applyOptions({});
+});
+
+test('the staging-pinned dispatcher carries the cap too', async () => {
+	// Constructed on its own branch, so it is the easy one to miss — and a staging deploy that
+	// 500s on every large-header page would look like a staging-edge fault, not a config gap.
+	applyOptions({});
+	const pinned = dispatcherFor('127.0.0.1');
+	assert.notEqual(pinned, dispatcherFor(undefined));
+
+	await withServer(32 * 1024, async (origin) => {
+		// The pin rewrites DNS to 127.0.0.1; the port still comes from the origin URL.
+		const res = await pinned.request({ origin, path: '/', method: 'GET' });
+		assert.equal(res.statusCode, 200);
+		await res.body.text();
+	});
+});
+
+test('a cap outside the schema bounds is rejected back to the default', () => {
+	// enforceSchemaConstraints warns and restores the default rather than throwing, so a typo
+	// degrades to the safe 64 KiB instead of silently reintroducing the 16 KiB failure...
+	applyOptions({ origin: { maxResponseHeaderBytes: 1024 } });
+	assert.equal(config.origin.maxResponseHeaderBytes, 64 * 1024);
+
+	// ...and the ceiling catches the opposite typo — a stray factor of a thousand — before it
+	// becomes an out-of-memory risk multiplied across concurrent connections.
+	applyOptions({ origin: { maxResponseHeaderBytes: 64 * 1024 * 1024 } });
+	assert.equal(config.origin.maxResponseHeaderBytes, 64 * 1024);
+
+	// The bounds themselves are inclusive and must stay usable.
+	applyOptions({ origin: { maxResponseHeaderBytes: 1024 * 1024 } });
+	assert.equal(config.origin.maxResponseHeaderBytes, 1024 * 1024);
+	applyOptions({});
+});