mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(http): raw writeHead routes lost every header the security hook set
`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) <noreply@anthropic.com>
This commit is contained in:
@@ -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`).
|
||||||
@@ -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<number | string | string[]>;
|
||||||
|
}): Record<string, number | string | string[]> {
|
||||||
|
const out: Record<string, number | string | string[]> = {};
|
||||||
|
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 {
|
export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & EventPort & ConfigPort): void {
|
||||||
// Lazy filesystem listing for the Link Existing and mobile input path pickers.
|
// Lazy filesystem listing for the Link Existing and mobile input path pickers.
|
||||||
app.get('/api/filesystem/browse', async (req, reply): Promise<ApiResponse<FilesystemBrowseData>> => {
|
app.get('/api/filesystem/browse', async (req, reply): Promise<ApiResponse<FilesystemBrowseData>> => {
|
||||||
@@ -1337,6 +1356,7 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even
|
|||||||
const basename = rawBasename.replace(/["\\\r\n]/g, '_');
|
const basename = rawBasename.replace(/["\\\r\n]/g, '_');
|
||||||
if (download === 'true' || ext === 'svg') {
|
if (download === 'true' || ext === 'svg') {
|
||||||
reply.raw.writeHead(200, {
|
reply.raw.writeHead(200, {
|
||||||
|
...inheritedHeaders(reply),
|
||||||
'Content-Type': ext === 'svg' ? 'application/octet-stream' : mimeTypes[ext] || 'application/octet-stream',
|
'Content-Type': ext === 'svg' ? 'application/octet-stream' : mimeTypes[ext] || 'application/octet-stream',
|
||||||
'Content-Disposition': `attachment; filename="${basename}"`,
|
'Content-Disposition': `attachment; filename="${basename}"`,
|
||||||
'Content-Length': content.length,
|
'Content-Length': content.length,
|
||||||
@@ -1576,6 +1596,7 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even
|
|||||||
|
|
||||||
// Set up SSE headers
|
// Set up SSE headers
|
||||||
reply.raw.writeHead(200, {
|
reply.raw.writeHead(200, {
|
||||||
|
...inheritedHeaders(reply),
|
||||||
'Content-Type': 'text/event-stream',
|
'Content-Type': 'text/event-stream',
|
||||||
'Cache-Control': 'no-cache',
|
'Cache-Control': 'no-cache',
|
||||||
Connection: 'keep-alive',
|
Connection: 'keep-alive',
|
||||||
@@ -1709,6 +1730,7 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even
|
|||||||
const content = await fs.readFile(resolvedPath);
|
const content = await fs.readFile(resolvedPath);
|
||||||
// Bypass Fastify compression — write directly to raw response
|
// Bypass Fastify compression — write directly to raw response
|
||||||
reply.raw.writeHead(200, {
|
reply.raw.writeHead(200, {
|
||||||
|
...inheritedHeaders(reply),
|
||||||
'Content-Type': mimeTypes[ext] || 'application/octet-stream',
|
'Content-Type': mimeTypes[ext] || 'application/octet-stream',
|
||||||
'Content-Disposition': `attachment; filename="${filename}"`,
|
'Content-Disposition': `attachment; filename="${filename}"`,
|
||||||
'Content-Length': content.length,
|
'Content-Length': content.length,
|
||||||
|
|||||||
@@ -805,7 +805,24 @@ export class WebServer extends EventEmitter {
|
|||||||
const clientId =
|
const clientId =
|
||||||
typeof query.clientId === 'string' && SSE_CLIENT_ID_RE.test(query.clientId) ? query.clientId : undefined;
|
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<string, number | string | string[]> = {};
|
||||||
|
for (const [name, value] of Object.entries(reply.getHeaders())) {
|
||||||
|
if (value !== undefined) inherited[name] = value;
|
||||||
|
}
|
||||||
reply.raw.writeHead(200, {
|
reply.raw.writeHead(200, {
|
||||||
|
...inherited,
|
||||||
'Content-Type': 'text/event-stream',
|
'Content-Type': 'text/event-stream',
|
||||||
'Cache-Control': 'no-cache',
|
'Cache-Control': 'no-cache',
|
||||||
Connection: 'keep-alive',
|
Connection: 'keep-alive',
|
||||||
|
|||||||
@@ -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<Headers> {
|
||||||
|
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'));
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user