diff --git a/.bumpy/leak-scan-finish-response.md b/.bumpy/leak-scan-finish-response.md new file mode 100644 index 000000000..405c63db9 --- /dev/null +++ b/.bumpy/leak-scan-finish-response.md @@ -0,0 +1,5 @@ +--- +varlock: patch +--- + +A leak detected in `ServerResponse.end` no longer leaves the HTTP client hanging. The response is finished before the leak error is rethrown (a plaintext 500 if the headers have not gone out yet, otherwise the connection is closed), so a Next.js Pages Router `res.json()` that would have leaked a sensitive value fails the request instead of stalling the client on a body that never arrives. diff --git a/framework-tests/frameworks/nextjs/files/pages-router/leaky-api-route.ts b/framework-tests/frameworks/nextjs/files/pages-router/leaky-api-route.ts new file mode 100644 index 000000000..77aa8be07 --- /dev/null +++ b/framework-tests/frameworks/nextjs/files/pages-router/leaky-api-route.ts @@ -0,0 +1,12 @@ +import type { NextApiRequest, NextApiResponse } from 'next'; + +import { ENV } from 'varlock/env'; + +// Deliberately leak a sensitive value from a pages-router API route. `res.json()` +// writes the whole body through a single `ServerResponse.end()` call, so the leak is +// caught there rather than in `write()`. Varlock has to finish the response itself +// (the client would otherwise hang), and next's `apiResolver` catch path then calls +// `res.end()` a second time - which must not take the dev server down. +export default function handler(req: NextApiRequest, res: NextApiResponse) { + res.status(200).json({ leaked: ENV.SENSITIVE_VAR }); +} diff --git a/framework-tests/frameworks/nextjs/nextjs-shared.ts b/framework-tests/frameworks/nextjs/nextjs-shared.ts index ac795ecc4..a06ed131f 100644 --- a/framework-tests/frameworks/nextjs/nextjs-shared.ts +++ b/framework-tests/frameworks/nextjs/nextjs-shared.ts @@ -175,6 +175,7 @@ export function defineNextjsTests(versionOrCanary: number | 'canary', testDir: s 'app/page.tsx': 'pages/basic-page.tsx', 'pages/pages-ssr.tsx': 'pages-router/ssr-page.tsx', 'pages/leaky-ssr.tsx': 'pages-router/leaky-ssr-page.tsx', + 'pages/api/leaky.ts': 'pages-router/leaky-api-route.ts', 'middleware.ts': 'middleware/middleware.ts', }, requests: [ @@ -267,6 +268,19 @@ export function defineNextjsTests(versionOrCanary: number | 'canary', testDir: s shouldNotContain: ['super-secret-var'], }, }, + { + // API route leak: `res.json()` sends the whole body through a single + // `end()` with no `write()`, which is the only path where the scanner + // sees the body for the first time at `end`. In dev the integration + // patches in redact mode, so the response still completes - the secret + // just comes out scrubbed, and the client is never left hanging. + label: 'runtime leak detection redacts a pages-router API route body', + path: '/api/leaky', + bodyAssertions: { + shouldContain: ['leaked'], + shouldNotContain: ['super-secret-var'], + }, + }, { // The fail-closed kill must only affect that one response — the dev // server has to keep serving afterwards. The gzip header assertion @@ -652,6 +666,9 @@ export function defineNextjsTests(versionOrCanary: number | 'canary', testDir: s replacements: { '// OUTPUT-MODE': "output: 'standalone'," }, }, 'app/page.tsx': 'pages/runtime-boot-page.tsx', + // production is the only mode where the response scanner throws rather + // than redacts, so this is where an `end()`-only leak has to be handled + 'pages/api/leaky.ts': 'pages-router/leaky-api-route.ts', }, requests: [ { @@ -667,6 +684,38 @@ export function defineNextjsTests(versionOrCanary: number | 'canary', testDir: s ], }, }, + { + // `res.json()` reaches the scanner at `end()` with no preceding + // `write()`, but next's compression layer has already emitted the + // headers by then, so the response cannot be rewritten and the + // connection is killed instead. Before this was handled, the client got + // a 200 whose Content-Length promised more bytes than were ever sent, + // and hung waiting for them. + label: 'leaking pages-router API route does not leave the client hanging', + path: '/api/leaky', + // a network failure, specifically: accepting any failure would let the + // original hang (a client-side timeout) pass this scenario + expectedFailure: 'network', + bodyAssertions: { + shouldNotContain: ['super-secret-var'], + }, + }, + { + // next answers the rethrown leak error by calling `res.end()` a second + // time; the server has to survive that and keep serving. + label: 'server still serves after a leaking API route response', + path: '/', + bodyAssertions: { + shouldContain: ['Varlock Framework Test - runtime boot'], + }, + }, + ], + outputAssertions: [ + { + description: 'leak detection fires for the API route, without logging the secret', + shouldContain: ['DETECTED LEAKED SENSITIVE CONFIG'], + shouldNotContain: ['super-secret-var'], + }, ], }); }); diff --git a/framework-tests/harness/dev-server.ts b/framework-tests/harness/dev-server.ts index 587560790..23d874b47 100644 --- a/framework-tests/harness/dev-server.ts +++ b/framework-tests/harness/dev-server.ts @@ -49,6 +49,16 @@ const sleep = (ms: number) => new Promise((r) => { setTimeout(r, ms); }); +/** + * A client-side abort (`AbortSignal.timeout`) means the server never answered - the + * request hung. Anything else is the connection itself failing, which is what a killed + * response looks like to the client. + */ +function classifyRequestFailure(err: unknown): 'network' | 'timeout' { + const name = (err as { name?: string } | undefined)?.name; + return name === 'TimeoutError' || name === 'AbortError' ? 'timeout' : 'network'; +} + /** * Fetch a URL with retries (server may report ready before accepting connections). * @@ -70,6 +80,8 @@ async function fetchWithRetry( } = {}, ): Promise { const { + // `retries: 0` means a single attempt with no retry sleeps, for requests whose + // expected outcome is a failure retries = 3, delayMs = 500, timeoutMs = 15_000, retryWhileUnavailable = true, unavailableTimeoutMs = 30_000, } = opts; @@ -364,15 +376,25 @@ export async function runDevServer( const url = `${currentUrl}${req.path}`; log(`Request ${i + 1}/${scenario.requests.length}: GET ${url}`); try { - const result = await fetchWithRetry(url, { retryWhileUnavailable: req.expectedStatus !== 503 }); + const result = await fetchWithRetry(url, { + retryWhileUnavailable: req.expectedStatus !== 503, + // a request that is expected to fail gets one attempt: retrying it only buys + // `delayMs` of sleeping per retry on the path where the test passes, and a + // shorter timeout bounds the wait if a hang regression puts one back + ...req.expectedFailure ? { timeoutMs: 5_000, retries: 0 } : {}, + }); log(`Response: status=${result.status}, body=${result.body.length} bytes`); responses.push(result); } catch (err) { - if (req.allowRequestFailure) { + if (req.allowRequestFailure || req.expectedFailure) { // e.g. runtime leak detection kills the response mid-stream — record a - // synthetic result so scenario/log assertions can still run - log(`Request failed (allowed): ${(err as Error).message}`); - responses.push({ status: 0, body: '', headers: {} }); + // synthetic result so scenario/log assertions can still run, keeping the + // kind of failure so `expectedFailure` can tell a reset from a hang + const failure = classifyRequestFailure(err); + log(`Request failed (allowed, ${failure}): ${(err as Error).message}`); + responses.push({ + status: 0, body: '', headers: {}, failure, + }); continue; } logError(`Request failed: ${(err as Error).message}`); diff --git a/framework-tests/harness/test-fixture.ts b/framework-tests/harness/test-fixture.ts index 32fe293bf..50317b985 100644 --- a/framework-tests/harness/test-fixture.ts +++ b/framework-tests/harness/test-fixture.ts @@ -422,8 +422,15 @@ export class FrameworkTestEnv { test(testLabel, () => { const resp = ctx.result!.responses[i]; expect(resp, `No response for request ${i} (GET ${req.path})`).toBeDefined(); + if (req.expectedFailure) { + expect( + resp.failure, + `Expected GET ${req.path} to fail with a ${req.expectedFailure} error` + + `${resp.failure ? `, got ${resp.failure}` : ` but it returned ${resp.status}`}`, + ).toBe(req.expectedFailure); + } // requests marked allowRequestFailure only assert status when explicitly set - if (req.expectedStatus !== undefined || !req.allowRequestFailure) { + if (req.expectedStatus !== undefined || !(req.allowRequestFailure || req.expectedFailure)) { const expectedStatus = req.expectedStatus ?? 200; expect(resp.status, `Expected status ${expectedStatus} for GET ${req.path}, got ${resp.status}`).toBe(expectedStatus); } diff --git a/framework-tests/harness/types.ts b/framework-tests/harness/types.ts index cc9f9831a..83ecb124b 100644 --- a/framework-tests/harness/types.ts +++ b/framework-tests/harness/types.ts @@ -167,6 +167,16 @@ export interface DevServerRequest { * unless `expectedStatus` is set explicitly. */ allowRequestFailure?: boolean; + /** + * Assert HOW the request failed, not merely that it did. `allowRequestFailure` on its + * own records the same empty synthetic response for every error, so a request that hung + * until the client timed out is indistinguishable from one the server killed promptly - + * which makes a "does not hang" regression test pass even when the hang comes back. + * `network` is a refused/reset connection, `timeout` is the client giving up. + * Implies `allowRequestFailure`, and shortens the per-attempt timeout since the failure + * is expected to be prompt. + */ + expectedFailure?: 'network' | 'timeout'; } /** Result of a single HTTP request to the dev server */ @@ -175,6 +185,8 @@ export interface DevServerRequestResult { body: string; /** response headers, lowercase keys */ headers: Record; + /** set only when the request failed and the scenario allowed it */ + failure?: 'network' | 'timeout'; } /** Configuration for a dev server scenario */ diff --git a/packages/varlock-website/src/content/docs/guides/secrets.mdx b/packages/varlock-website/src/content/docs/guides/secrets.mdx index f4ff8717f..b8babd818 100644 --- a/packages/varlock-website/src/content/docs/guides/secrets.mdx +++ b/packages/varlock-website/src/content/docs/guides/secrets.mdx @@ -176,7 +176,7 @@ The `reveal` command displays values in an **alternate screen buffer**. When you When the output of [`varlock run`](/reference/cli/load-and-run/#run) is piped or redirected (CI logs, files, `| tee`, etc.), the child process's stdout/stderr is piped through the same redaction engine, so any sensitive values that end up in your application's output will be masked automatically before they persist anywhere. -When output is attached to an interactive terminal, it passes straight through instead. Piping would break interactive tools (e.g., `psql`, `claude`) that rely on TTY detection, and a human at the terminal already has access to the secrets. Use `--no-redact-stdout` to force-disable redaction for piped output, or `--redact-stdout` to force it (e.g., to override `@redactLogs=false`; errors if output is attached to an interactive terminal, where redaction is not possible without breaking TTY behavior): +When output is attached to an interactive terminal, it passes straight through instead. Piping would break interactive tools (e.g., `psql`, `claude`) that rely on TTY detection, and a human at the terminal already has access to the secrets. `--no-redact-stdout` disables redaction entirely. `--redact-stdout` forces it on piped output, which is how you override `@redactLogs=false`; it errors when output is attached to an interactive terminal, where redaction would break TTY behavior. ```bash varlock run --no-redact-stdout -- node app.js | tee log.txt # force-disable redaction for piped output @@ -196,7 +196,7 @@ console.log(process.env.SECRET_KEY); // outputs "my▒▒▒▒▒" instead of "my-secret-value" ``` -This works by intercepting Node.js internal console internals, with an additional layer that wraps the console methods themselves to handle environments where `console.log` has been patched by the platform (e.g., AWS Lambda) or where those internals do not exist at all (edge runtimes like Cloudflare Workers and Vercel Edge). +This works by intercepting Node's console internals, with an additional layer that wraps the console methods themselves to handle environments where `console.log` has been patched by the platform (e.g., AWS Lambda) or where those internals do not exist at all (edge runtimes like Cloudflare Workers and Vercel Edge). Redaction covers whatever you pass to the console methods: strings, arrays, plain objects, and `Error` objects (including messages, stack traces, and anything nested inside). Varlock logs a redacted copy rather than modifying what you passed in. @@ -214,10 +214,10 @@ To disable runtime log redaction, set the [`@redactLogs`](/reference/root-decora _Only available in JavaScript/Node.js projects using varlock's runtime integrations._ -Varlock scans outgoing HTTP responses at runtime to detect if any sensitive values are being accidentally sent to clients. If a leak is detected, varlock throws an error with a detailed diagnostic message including the config item key and where the leak was detected. +Varlock scans outgoing HTTP responses at runtime to detect if any sensitive values are being accidentally sent to clients. If a leak is detected, varlock throws an error with a detailed diagnostic message including the config item key and where the leak was detected. The rejected body is never delivered and the request fails rather than hanging, so look to your server log for the diagnostic. This works by patching: -- **Node.js `ServerResponse`**: intercepts `write()` and `end()` calls, scanning text and JSON response bodies (including gzip-compressed responses) +- **Node.js `ServerResponse`**: intercepts `write()` and `end()` calls, scanning text and JSON response bodies, including compressed ones - **Global `Response` constructor**: intercepts the `Response` class used in edge runtimes (e.g., Cloudflare Workers), scanning bodies passed to the constructor and `Response.json()` Streamed responses are scanned across chunk boundaries, so a sensitive value that gets split between two writes is still detected. When the end of a chunk looks like the start of a sensitive value, that trailing text is held back briefly (up to 15ms, or until the next chunk arrives) so the full value can be caught before any of it is sent. @@ -238,7 +238,7 @@ The [`varlock scan` command](/reference/cli/project/#scan) checks your project f varlock scan ``` -This is intended to be used as a pre-commit git hook to prevent accidentally committing secrets into version control. If no sensitive values are found in plaintext, it exits successfully. If any are detected, it reports the file, line number,and which secret was found, then exits with a non-zero status code. +This is intended to be used as a pre-commit git hook to prevent accidentally committing secrets into version control. If no sensitive values are found in plaintext, it exits successfully. If any are detected, it reports the file, line number, and which secret was found, then exits with a non-zero status code. It can also be used to scan build output as an extra step to prevent accidentally bundling secrets into client-facing code. Our [drop-in integrations](/integrations/overview/) usually do this automatically, but this can be useful in some scenarios. diff --git a/packages/varlock/src/runtime/patch-server-response.ts b/packages/varlock/src/runtime/patch-server-response.ts index 492f07167..0e9fb7b55 100644 --- a/packages/varlock/src/runtime/patch-server-response.ts +++ b/packages/varlock/src/runtime/patch-server-response.ts @@ -183,6 +183,61 @@ function scanChunk(state: ScanState, chunkStr: string, o: { return emit; } +/** marks a response varlock already finished (or destroyed) because of a detected leak */ +const leakFinalizedKey = '_varlockLeakFinalized'; + +/** + * Leak detection on `end` used to throw before the original `end` ran, which left + * the HTTP client hanging (Next.js Pages Router `res.json()` answered a 200 whose + * Content-Length promised more bytes than were ever sent). Finish the response first, + * then rethrow so callers still see the leak error. + * + * Which branch runs depends on how much of the response is already committed. A + * compression layer in front of `end()` emits the headers before varlock sees the body, + * which is what next.js does in production, so an api route there takes the destroy + * branch. A vite dev middleware reaches `end()` with the headers still unsent and gets + * the 500. Both branches are load-bearing; neither is a fallback for the other. + * + * The response is flagged as finalized before the rethrow, because framework error + * handling generally responds to a thrown error by ending the response itself (Next.js + * Pages Router `apiResolver` catches and calls `sendError`, which calls `res.end()` + * again). A second `end()` on an already-ended response emits `ERR_STREAM_WRITE_AFTER_END` + * on the response, which nothing listens for and takes the whole process down - so + * further writes and ends through the patched methods become no-ops. + */ +function finishResponseOnLeak( + res: ServerResponse, + originalEnd: typeof ServerResponse.prototype.end, + err: unknown, +): never { + (res as any)[leakFinalizedKey] = true; + if (!res.headersSent) { + const body = 'Internal Server Error'; + res.statusCode = 500; + // node keeps a reason phrase the route set for the rejected body when only the + // status code changes, which would send this out as `500 ` + res.statusMessage = body; + // every header on this response describes the representation that was just + // rejected, and picking off the ones that are obviously wrong (length, content + // encoding, entity tag) leaves the rest: `Cache-Control: s-maxage=...` would have + // a CDN cache the error, `Set-Cookie` would still be set, and route-specific + // headers would describe a body that is not being sent. Drop them all and + // declare only what the replacement body needs. + for (const name of res.getHeaderNames()) res.removeHeader(name); + res.setHeader('Content-Type', 'text/plain; charset=utf-8'); + res.setHeader('Content-Length', Buffer.byteLength(body)); + try { + // @ts-ignore Node's end overloads confuse Function.call + originalEnd.call(res, body); + } catch { + res.destroy(); + } + } else { + res.destroy(); + } + throw err; +} + export function patchGlobalServerResponse(opts?: { ignoreUrlPatterns?: Array, redactInsteadOfThrow?: boolean, @@ -223,6 +278,13 @@ export function patchGlobalServerResponse(opts?: { ServerResponse.prototype.write = function varlockPatchedServerResponseWrite(...args) { // TODO: do we want to filter out some requests here? maybe based on the file type? + // varlock already finished this response because of a leak - see finishResponseOnLeak + if ((this as any)[leakFinalizedKey]) { + const cb = args.find((arg: any) => typeof arg === 'function'); + if (cb) process.nextTick(cb); + return true; + } + const rawChunk = args[0]; // console.log('⚡️ patched ServerResponse.write', rawChunk); @@ -363,6 +425,14 @@ export function patchGlobalServerResponse(opts?: { // @ts-ignore ServerResponse.prototype.end = function patchedServerResponseEnd(...args) { // console.log('⚡️ patched ServerResponse.end'); + + // varlock already finished this response because of a leak - see finishResponseOnLeak + if ((this as any)[leakFinalizedKey]) { + const cb = args.find((arg: any) => typeof arg === 'function'); + if (cb) process.nextTick(cb); + return this; + } + const endChunk = args[0]; const state = getScanState(this); clearPendingFlush(state); @@ -386,10 +456,14 @@ export function patchGlobalServerResponse(opts?: { } if (decompressed !== undefined) { // compressed output can't be scrubbed, so a detected leak always throws (see write above) - scanForLeaks(state.carry + decodeDecompressedDelta(state, decompressed, true), { - method: 'patched ServerResponse.end', - file: (this as any).req?.url, - }); + try { + scanForLeaks(state.carry + decodeDecompressedDelta(state, decompressed, true), { + method: 'patched ServerResponse.end', + file: (this as any).req?.url, + }); + } catch (err) { + finishResponseOnLeak(this, serverResponseEnd, err); + } } // @ts-ignore return serverResponseEnd.apply(this, args); @@ -409,11 +483,16 @@ export function patchGlobalServerResponse(opts?: { if (chunkStr || state.pending) { // last chunk, so nothing can be withheld for later - it all goes out now - const emit = scanChunk(state, chunkStr, { - canHoldBack: false, - redactInsteadOfThrow: opts?.redactInsteadOfThrow, - meta: { method: 'patched ServerResponse.end', file: (this as any).req?.url }, - }); + let emit: string; + try { + emit = scanChunk(state, chunkStr, { + canHoldBack: false, + redactInsteadOfThrow: opts?.redactInsteadOfThrow, + meta: { method: 'patched ServerResponse.end', file: (this as any).req?.url }, + }); + } catch (err) { + finishResponseOnLeak(this, serverResponseEnd, err); + } // for a string (or absent) final chunk, `chunkStr` may carry a flushed decoder tail // that the outgoing chunk needs to pick up, so compare against what was actually passed let originalStr = ''; diff --git a/packages/varlock/src/runtime/test/patch-server-response.test.ts b/packages/varlock/src/runtime/test/patch-server-response.test.ts index 662b859b4..20de9d8a4 100644 --- a/packages/varlock/src/runtime/test/patch-server-response.test.ts +++ b/packages/varlock/src/runtime/test/patch-server-response.test.ts @@ -152,6 +152,169 @@ describe('patched ServerResponse.end', () => { const res = makeRes({ 'content-type': 'application/json' }); expect(() => res.end(Buffer.from(JSON.stringify({ leaked: SECRET })))).toThrow(/DETECTED LEAKED SENSITIVE CONFIG/); }); + + // while the headers are still unsent the response can be replaced outright, which is + // what a vite dev middleware or an uncompressed node handler hits + it('replaces the response with a 500 when the headers have not gone out', () => { + const res = makeRes({ 'content-type': 'application/json' }); + expect(() => res.end(JSON.stringify({ leaked: SECRET }))).toThrow(/DETECTED LEAKED SENSITIVE CONFIG/); + expect(res.statusCode).toBe(500); + expect(res.writableEnded).toBe(true); + }); + + // once the headers are out there is nothing left to rewrite - next.js in production + // reaches `end()` through a compression layer that has already emitted them + it('kills the connection when the headers have already gone out', () => { + const res = makeRes({ 'content-type': 'text/html' }); + res.write(''); + expect(res.headersSent).toBe(true); + expect(() => res.end(`leaked: ${SECRET}`)).toThrow(/DETECTED LEAKED SENSITIVE CONFIG/); + expect(res.destroyed).toBe(true); + }); + + // frameworks answer the rethrown leak error by ending the response themselves + // (next.js pages router `apiResolver` -> `sendError` -> `res.end('Internal Server Error')`). + // Node turns a second end() into an ERR_STREAM_WRITE_AFTER_END `error` event that + // nothing listens for, which would crash the process. + it('absorbs a second end() from framework error handling', () => { + const res = makeRes({ 'content-type': 'application/json' }); + const errors: Array = []; + res.on('error', (err) => errors.push(err)); + expect(() => res.end(JSON.stringify({ leaked: SECRET }))).toThrow(/DETECTED LEAKED SENSITIVE CONFIG/); + expect(() => res.end('Internal Server Error')).not.toThrow(); + expect(() => res.write('more')).not.toThrow(); + expect(errors).toEqual([]); + }); +}); + +describe('patched ServerResponse.end - over a real connection', () => { + let server: http.Server; + let baseUrl: string; + + beforeAll(async () => { + server = http.createServer((req, res) => { + try { + if (req.url === '/leak-json' || req.url === '/leak-json-only-logged') { + // everything a route may have set for the body that is being rejected: a + // length the client would otherwise wait on, an etag computed from it, + // caching directives, cookies, and route-specific headers + const leaked = JSON.stringify({ leaked: SECRET }); + res.setHeader('content-type', 'application/json'); + res.setHeader('content-length', String(Buffer.byteLength(leaked))); + res.setHeader('etag', 'W/"deadbeef"'); + res.setHeader('cache-control', 's-maxage=3600, stale-while-revalidate'); + res.setHeader('set-cookie', 'session=abc; Path=/'); + res.setHeader('x-route-header', 'from-rejected-response'); + res.end(leaked); + return; + } + if (req.url === '/leak-status-message') { + // a route that set its own reason phrase for the body being rejected + res.statusMessage = 'Everything Is Fine'; + res.setHeader('content-type', 'application/json'); + res.end(JSON.stringify({ leaked: SECRET })); + return; + } + if (req.url === '/leak-gzip') { + res.setHeader('content-type', 'text/html'); + res.setHeader('content-encoding', 'gzip'); + res.end(zlib.gzipSync(htmlWithSecret)); + return; + } + if (req.url === '/leak-streamed') { + res.setHeader('content-type', 'text/html'); + res.write(''); + res.end(`leaked: ${SECRET}`); + return; + } + res.setHeader('content-type', 'text/plain'); + res.end('ok'); + } catch { + // frameworks that only log the error and never touch the response again + if (req.url?.includes('only-logged')) return; + // mirror next.js pages router error handling, which ends the response again + res.statusCode = 500; + res.end('Internal Server Error'); + } + }); + await new Promise((resolve) => { + server.listen(0, () => resolve()); + }); + const address = server.address(); + if (!address || typeof address === 'string') throw new Error('expected address info'); + baseUrl = `http://127.0.0.1:${address.port}`; + }); + + afterAll(async () => { + await new Promise((resolve) => { + server.close(resolve); + }); + }); + + it('serves a complete 500 the client is not left waiting on', async () => { + const resp = await fetch(`${baseUrl}/leak-json`, { signal: AbortSignal.timeout(2000) }); + expect(resp.status).toBe(500); + const body = await resp.text(); + expect(body).toBe('Internal Server Error'); + expect(resp.headers.get('content-length')).toBe(String(Buffer.byteLength(body))); + }); + + // headers describing the rejected representation must not survive onto the + // replacement - a cached 500 (`s-maxage`) is the one that actually bites + it('drops every header the rejected response had set', async () => { + const resp = await fetch(`${baseUrl}/leak-json`); + expect(resp.headers.get('etag')).toBeNull(); + expect(resp.headers.get('cache-control')).toBeNull(); + expect(resp.headers.get('set-cookie')).toBeNull(); + expect(resp.headers.get('x-route-header')).toBeNull(); + expect(resp.headers.get('content-encoding')).toBeNull(); + expect(resp.headers.get('content-type')).toBe('text/plain; charset=utf-8'); + }); + + // node keeps a reason phrase set by the route when only `statusCode` changes, so the + // replacement would otherwise go out as `500 Everything Is Fine` + it('does not keep the rejected response reason phrase', async () => { + const resp = await fetch(`${baseUrl}/leak-status-message`, { signal: AbortSignal.timeout(2000) }); + expect(resp.status).toBe(500); + expect(resp.statusText).toBe('Internal Server Error'); + }); + + // #897: the client used to sit waiting on a Content-Length that promised bytes + // which were never going to be sent + it('answers without waiting on framework error handling', async () => { + const resp = await fetch(`${baseUrl}/leak-json-only-logged`, { signal: AbortSignal.timeout(2000) }); + expect(resp.status).toBe(500); + expect(await resp.text()).toBe('Internal Server Error'); + }); + + it('replaces a compressed body with the plaintext 500', async () => { + const resp = await fetch(`${baseUrl}/leak-gzip`); + expect(resp.status).toBe(500); + expect(resp.headers.get('content-encoding')).toBeNull(); + expect(await resp.text()).toBe('Internal Server Error'); + }); + + // nothing can be rewritten once the first chunk is on the wire + it('kills the connection when the leak lands after the headers went out', async () => { + let caught: any; + try { + const resp = await fetch(`${baseUrl}/leak-streamed`, { signal: AbortSignal.timeout(2000) }); + await resp.text(); + } catch (err) { + caught = err; + } + expect(caught).toBeDefined(); + // a TimeoutError here means the connection was left open and the client hung + expect(caught.name).not.toBe('TimeoutError'); + }); + + // the framework's second end() must not take the server down with it + it('keeps serving after a leak was caught', async () => { + await fetch(`${baseUrl}/leak-json`); + const resp = await fetch(`${baseUrl}/clean`); + expect(resp.status).toBe(200); + expect(await resp.text()).toBe('ok'); + }); }); // a scan that only ever sees one chunk at a time misses any value that straddles a boundary,