From 1a32e63765c4db19a22638fb9d9ab579e3d45f98 Mon Sep 17 00:00:00 2001 From: Claudia Date: Fri, 7 Aug 2026 01:31:50 +0200 Subject: [PATCH] fix(http): raw writeHead routes lost every header the security hook set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `reply.raw.writeHead()` writes straight to the Node response and bypasses Fastify's header store, so everything the `onRequest` security hook granted is silently dropped on every route that answers that way. The visible symptom is CORS. The hook emits `Access-Control-Allow-Origin` for localhost origins, so a page served from a local dev server may call every `/api` endpoint cross-origin — except the four below, whose requests fail. The security headers (`X-Content-Type-Options`, `X-Frame-Options`, CSP) were being lost the same way. Affected: `GET /api/events`, and `file-raw` / `tail-file` / `download` in file-routes.ts. Each now spreads the inherited headers first and lets its own headers win over them. Tests drive a real WebServer and compare `/api/events` against `/api/status` for the same Origin — the point of the fix being that the SSE route stops being the odd one out. Verified in both directions: with the fix removed, 3 of the 5 fail. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/sse-inherits-security-headers.md | 16 ++++ src/web/routes/file-routes.ts | 22 +++++ src/web/server.ts | 17 ++++ test/sse-cors-headers.test.ts | 89 +++++++++++++++++++++ 4 files changed, 144 insertions(+) create mode 100644 .changeset/sse-inherits-security-headers.md create mode 100644 test/sse-cors-headers.test.ts diff --git a/.changeset/sse-inherits-security-headers.md b/.changeset/sse-inherits-security-headers.md new file mode 100644 index 00000000..27db693e --- /dev/null +++ b/.changeset/sse-inherits-security-headers.md @@ -0,0 +1,16 @@ +--- +'aicodeman': patch +--- + +Routes that answer with `reply.raw.writeHead()` no longer drop the headers the +security hook set. + +`writeHead` writes straight to the Node response and bypasses Fastify's header +store, so everything the `onRequest` hook granted was silently lost — including the +`Access-Control-Allow-Origin` it emits for localhost origins, and the +`X-Content-Type-Options` / `X-Frame-Options` / CSP headers. A localhost page could +therefore call every other `/api` endpoint cross-origin while its EventSource +failed CORS. + +Affects `GET /api/events` and the three raw-writing routes in `file-routes.ts` +(`file-raw`, `tail-file`, `download`). diff --git a/src/web/routes/file-routes.ts b/src/web/routes/file-routes.ts index ce607191..4a444044 100644 --- a/src/web/routes/file-routes.ts +++ b/src/web/routes/file-routes.ts @@ -640,6 +640,25 @@ async function buildExternalAttachmentRouteItem( } } +/** + * Headers Fastify already put on the reply, in a shape `writeHead` accepts. + * + * `reply.raw.writeHead()` writes straight to the Node response and bypasses + * Fastify's header store, so anything the security `onRequest` hook granted — CORS + * for localhost origins, nosniff, frame-options, CSP — is silently dropped on every + * route that answers this way. Spread this first and let the route's own headers + * win over it. + */ +function inheritedHeaders(reply: { + getHeaders(): NodeJS.Dict; +}): Record { + const out: Record = {}; + for (const [name, value] of Object.entries(reply.getHeaders())) { + if (value !== undefined) out[name] = value; + } + return out; +} + export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & EventPort & ConfigPort): void { // Lazy filesystem listing for the Link Existing and mobile input path pickers. app.get('/api/filesystem/browse', async (req, reply): Promise> => { @@ -1337,6 +1356,7 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even const basename = rawBasename.replace(/["\\\r\n]/g, '_'); if (download === 'true' || ext === 'svg') { reply.raw.writeHead(200, { + ...inheritedHeaders(reply), 'Content-Type': ext === 'svg' ? 'application/octet-stream' : mimeTypes[ext] || 'application/octet-stream', 'Content-Disposition': `attachment; filename="${basename}"`, 'Content-Length': content.length, @@ -1576,6 +1596,7 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even // Set up SSE headers reply.raw.writeHead(200, { + ...inheritedHeaders(reply), 'Content-Type': 'text/event-stream', 'Cache-Control': 'no-cache', Connection: 'keep-alive', @@ -1709,6 +1730,7 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even const content = await fs.readFile(resolvedPath); // Bypass Fastify compression — write directly to raw response reply.raw.writeHead(200, { + ...inheritedHeaders(reply), 'Content-Type': mimeTypes[ext] || 'application/octet-stream', 'Content-Disposition': `attachment; filename="${filename}"`, 'Content-Length': content.length, diff --git a/src/web/server.ts b/src/web/server.ts index de5492d2..74b9c71d 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -805,7 +805,24 @@ export class WebServer extends EventEmitter { const clientId = typeof query.clientId === 'string' && SSE_CLIENT_ID_RE.test(query.clientId) ? query.clientId : undefined; + // Carry over the headers the security hook already set on this reply. + // + // writeHead goes straight to the Node response and bypasses Fastify's header + // store, so everything the onRequest hook granted is silently dropped — + // including the Access-Control-Allow-Origin it emits for localhost origins. + // The result is an internal contradiction: a localhost page may call every + // /api endpoint cross-origin, but its EventSource fails CORS. The security + // headers (nosniff, frame-options, CSP) were lost the same way. + // + // The other raw-writeHead routes live in file-routes.ts and share a helper; + // this one keeps its own copy so the server does not import from a route + // module it registers. + const inherited: Record = {}; + for (const [name, value] of Object.entries(reply.getHeaders())) { + if (value !== undefined) inherited[name] = value; + } reply.raw.writeHead(200, { + ...inherited, 'Content-Type': 'text/event-stream', 'Cache-Control': 'no-cache', Connection: 'keep-alive', diff --git a/test/sse-cors-headers.test.ts b/test/sse-cors-headers.test.ts new file mode 100644 index 00000000..f0edc206 --- /dev/null +++ b/test/sse-cors-headers.test.ts @@ -0,0 +1,89 @@ +/** + * @fileoverview `/api/events` must not lose the headers the security hook set. + * + * The SSE route answers with `reply.raw.writeHead()`, which writes straight to the + * Node response and bypasses Fastify's header store. Everything the `onRequest` + * security hook had granted was therefore dropped — including the + * `Access-Control-Allow-Origin` it emits for localhost origins. The contradiction is + * visible from a browser: a localhost page may call every other `/api` endpoint + * cross-origin, but its EventSource fails CORS. + * + * These tests drive a REAL WebServer. An earlier version asserted against an inline + * copy of the hook and the handler, which proved nothing: reverting the fix in + * `server.ts` left every test green. + */ + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +import { WebServer } from '../src/web/server.js'; + +const TEST_PORT = 3119; +const LOCAL_ORIGIN = 'http://localhost:5173'; + +/** Open /api/events, read the response headers, then abort — it never ends on its own. */ +async function eventsHeaders(baseUrl: string, origin?: string): Promise { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 2000); + try { + const res = await fetch(`${baseUrl}/api/events`, { + signal: controller.signal, + headers: origin ? { Origin: origin } : undefined, + }); + const headers = res.headers; + controller.abort(); // stop consuming the stream + return headers; + } finally { + clearTimeout(timeout); + } +} + +describe('GET /api/events header inheritance', () => { + let server: WebServer; + let baseUrl: string; + + beforeAll(async () => { + server = new WebServer(TEST_PORT, false, true); + await server.start(); + baseUrl = `http://localhost:${TEST_PORT}`; + }); + + afterAll(async () => { + await server.stop(); + }, 60000); + + it('keeps the CORS header the security hook granted a localhost origin', async () => { + // The regression: this header is set on the Fastify reply and was then thrown + // away by writeHead, so an EventSource from a localhost dev server failed CORS + // while every other endpoint worked. + const headers = await eventsHeaders(baseUrl, LOCAL_ORIGIN); + expect(headers.get('access-control-allow-origin')).toBe(LOCAL_ORIGIN); + }); + + it('keeps the security headers the hook set', async () => { + const headers = await eventsHeaders(baseUrl); + expect(headers.get('x-content-type-options')).toBe('nosniff'); + expect(headers.get('x-frame-options')).toBe('SAMEORIGIN'); + expect(headers.get('content-security-policy')).toBeTruthy(); + }); + + it('still sets the SSE headers, and they win over anything inherited', async () => { + const headers = await eventsHeaders(baseUrl); + expect(headers.get('content-type')).toBe('text/event-stream'); + expect(headers.get('cache-control')).toBe('no-cache'); + expect(headers.get('x-accel-buffering')).toBe('no'); + }); + + it('grants nothing to a non-localhost origin — the hook decides, not this route', async () => { + const headers = await eventsHeaders(baseUrl, 'https://evil.example'); + expect(headers.get('access-control-allow-origin')).toBeNull(); + }); + + it('matches what a normal JSON endpoint returns for the same origin', async () => { + // The point of the fix: /api/events stops being the odd one out. + const json = await fetch(`${baseUrl}/api/status`, { headers: { Origin: LOCAL_ORIGIN } }); + const sse = await eventsHeaders(baseUrl, LOCAL_ORIGIN); + + expect(sse.get('access-control-allow-origin')).toBe(json.headers.get('access-control-allow-origin')); + expect(sse.get('x-content-type-options')).toBe(json.headers.get('x-content-type-options')); + }); +});