Skip to content

Commit 343fde2

Browse files
d-csclaude
andcommitted
fix(webapp): apply streaming content-type bypass on patchedEnd too
patchedWrite checks isStreamingContentType() before buffering; patchedEnd only had the headersSent half. A 5xx response that sets a streaming content-type and delivers its body via single-call res.end(chunk) would get buffered, UTF-8 decoded, and run through the leak regex — wrong shape for binary/SSE payloads. Mirrors the patchedWrite guard so the same bypass applies regardless of write/end call shape. Test pins the contract: a 5xx octet-stream body whose bytes contain "P1001" passes through unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 05c5e18 commit 343fde2

2 files changed

Lines changed: 45 additions & 9 deletions

File tree

apps/webapp/app/services/apiErrorBoundary.server.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,13 @@ export const apiErrorBoundary: RequestHandler = (req, res, next) => {
104104
};
105105

106106
const patchedEnd = (chunk?: unknown, ...rest: unknown[]) => {
107-
// Same guard as patchedWrite: if headers were flushed (e.g. via an
108-
// explicit `res.flushHeaders()` before end-with-body), we cannot rewrite
109-
// the response — flush buffered chunks unchanged and skip sanitization.
110-
if (!bypass && res.headersSent) flushAndBypass();
107+
// Mirror patchedWrite's bypass guards. headersSent: if headers were
108+
// flushed (e.g. via an explicit `res.flushHeaders()` before end-with-body),
109+
// we cannot rewrite the response. isStreamingContentType: if a single-call
110+
// `res.end(chunk)` carries a streaming content-type (SSE / octet-stream),
111+
// the leak rules are JSON-text heuristics and have no business interpreting
112+
// that payload — flush unchanged and skip sanitization.
113+
if (!bypass && (res.headersSent || isStreamingContentType())) flushAndBypass();
111114
if (bypass) {
112115
return (originalEnd as (c?: unknown, ...r: unknown[]) => typeof res)(chunk, ...rest);
113116
}

apps/webapp/test/apiErrorBoundary.test.ts

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,12 +238,45 @@ describe("apiErrorBoundary", () => {
238238
expect(response.body).toEqual({ part1: "hello", part2: "world" });
239239
});
240240

241+
// The streaming-content-type bypass must fire on single-call res.end
242+
// too, not only on res.write. If a 5xx response sets a streaming
243+
// content-type and delivers its body via res.end(chunk) directly, the
244+
// body must pass through untouched — the leak rules are JSON-text
245+
// heuristics and have no business interpreting binary or SSE payloads.
246+
test("does NOT sanitize a 5xx octet-stream body delivered via single-call res.end(chunk), even if bytes contain leak pattern", async () => {
247+
const app = buildApp();
248+
// Construct a binary payload whose bytes happen to spell "P1001"
249+
// somewhere — proves the streaming bypass fires before the regex.
250+
const binary = Buffer.concat([
251+
Buffer.from([0x00, 0xff, 0xab]),
252+
Buffer.from("P1001", "ascii"),
253+
Buffer.from([0xcd, 0xef, 0x00]),
254+
]);
255+
256+
app.get("/api/v1/octet-5xx", (_req, res) => {
257+
res.statusCode = 500;
258+
res.setHeader("Content-Type", "application/octet-stream");
259+
res.end(binary);
260+
});
261+
262+
const response = await request(app)
263+
.get("/api/v1/octet-5xx")
264+
.buffer(true)
265+
.parse((res, cb) => {
266+
const chunks: Buffer[] = [];
267+
res.on("data", (c: Buffer) => chunks.push(c));
268+
res.on("end", () => cb(null, Buffer.concat(chunks)));
269+
});
270+
271+
expect(response.status).toBe(500);
272+
expect(Buffer.compare(response.body as Buffer, binary)).toBe(0);
273+
});
274+
241275
// patchedWrite applies the isStreamingContentType / MAX_BUFFER_BYTES
242-
// bypass checks before buffering; patchedEnd does not, because a
243-
// single-call `res.end(chunk)` is one-shot rather than streaming
244-
// pileup — the body is fully assembled either way. These tests pin
245-
// the byte-integrity contract for that single-call shape: octet-stream
246-
// bodies and >64KB bodies both flow through unchanged.
276+
// bypass checks before buffering; patchedEnd applies the same
277+
// streaming-content-type check. These tests pin the byte-integrity
278+
// contract for the single-call res.end shape: octet-stream bodies
279+
// and >64KB bodies both flow through unchanged.
247280
test("preserves bytes for octet-stream body delivered via single-call res.end(chunk)", async () => {
248281
const app = buildApp();
249282
// Construct binary bytes that span the full 0-255 range to catch any

0 commit comments

Comments
 (0)