fix response scanner chunk boundaries

Author: pullfrog[bot]

packages/varlock/src/runtime/patch-server-response.ts

@@ -61,7 +61,9 @@ type ScanState = {
    * (created lazily - responses that never hit the binary path don't need one) */
   decoder: TextDecoder | undefined,
   zlibChunks: Array<Buffer>,
-  /** length of the decompressed output already scanned, so each chunk only rescans the new tail */
+  /** streaming decoder for decompressed deltas, which may end inside a multi-byte character */
+  decompressedDecoder: TextDecoder | undefined,
+  /** byte length of the decompressed output already scanned, so each chunk only scans new bytes */
   decompressedLength: number,
   flushTimer: ReturnType<typeof setTimeout> | undefined,
 };
@@ -72,6 +74,7 @@ function getScanState(res: any): ScanState {
     carry: '',
     decoder: undefined,
     zlibChunks: [],
+    decompressedDecoder: undefined,
     decompressedLength: 0,
     flushTimer: undefined,
   } satisfies ScanState;
@@ -87,6 +90,13 @@ function decodeChunk(state: ScanState, chunk?: Uint8Array, isFinal?: boolean) {
   return state.decoder.decode(chunk, { stream: !isFinal });
 }
 
+function decodeDecompressedDelta(state: ScanState, decompressed: Buffer, isFinal = false) {
+  state.decompressedDecoder ||= new TextDecoder();
+  const delta = decompressed.subarray(state.decompressedLength);
+  state.decompressedLength = decompressed.byteLength;
+  return state.decompressedDecoder.decode(delta, { stream: !isFinal });
+}
+
 function clearPendingFlush(state: ScanState) {
   if (state.flushTimer !== undefined) {
     clearTimeout(state.flushTimer);
@@ -201,6 +211,12 @@ export function patchGlobalServerResponse(opts?: {
     const state = getScanState(this);
     clearPendingFlush(state);
 
+    // A later chunk may be redacted to a different length. Once write() sends the
+    // headers, Content-Length cannot be corrected, so use chunked framing instead.
+    if (opts?.redactInsteadOfThrow && !this.headersSent && this.getHeader('content-length') !== undefined) {
+      this.removeHeader('content-length');
+    }
+
     // have to deal with compressed data, which is awkward but possible
     const compressionType = this.getHeader('Content-Encoding');
     let chunkStr;
@@ -223,9 +239,7 @@ export function patchGlobalServerResponse(opts?: {
         // partial stream fails to decode here and gets scanned once more chunks arrive.
         try {
           const decompressedChunk = decompress(Buffer.concat(state.zlibChunks));
-          const fullDecompressedData = decompressedChunk.toString('utf-8');
-          chunkStr = fullDecompressedData.substring(state.decompressedLength);
-          state.decompressedLength = fullDecompressedData.length;
+          chunkStr = decodeDecompressedDelta(state, decompressedChunk);
         } catch (err) {
           // partial compressed data that doesn't decode yet — scanned when more chunks arrive
         }
@@ -296,18 +310,18 @@ export function patchGlobalServerResponse(opts?: {
 
     if (isBinaryChunk && compressionType) {
       const decompress = getDecompressor(String(compressionType).toLowerCase());
-      let decompressed: string | undefined;
+      let decompressed: Buffer | undefined;
       if (decompress) {
         state.zlibChunks.push(endChunk as Buffer);
         try {
-          decompressed = decompress(Buffer.concat(state.zlibChunks)).toString('utf-8');
+          decompressed = decompress(Buffer.concat(state.zlibChunks));
         } catch (err) {
           // stream didn't decode, nothing more we can do at this point
         }
       }
       if (decompressed !== undefined) {
         // compressed output can't be scrubbed, so a detected leak always throws (see write above)
-        scanForLeaks(state.carry + decompressed.substring(state.decompressedLength), {
+        scanForLeaks(state.carry + decodeDecompressedDelta(state, decompressed, true), {
           method: 'patched ServerResponse.end',
           file: (this as any).req?.url,
         });

packages/varlock/src/runtime/test/patch-server-response-redact.test.ts

@@ -64,6 +64,11 @@ beforeAll(async () => {
       const body = `<html>leak: ${SECRET}</html>`;
       res.setHeader('content-length', Buffer.byteLength(body));
       res.end(body);
+    } else if (req.url === '/content-length-split-leak') {
+      const body = `<html>leak: ${SECRET}</html>`;
+      res.setHeader('content-length', Buffer.byteLength(body));
+      res.write(`<html>leak: ${SECRET.slice(0, 10)}`);
+      res.end(`${SECRET.slice(10)}</html>`);
     } else if (req.url === '/partial-lookalike') {
       // ends mid-lookalike but never completes the secret - must arrive intact
       res.write(`<html>${SECRET.slice(0, 12)}`);
@@ -133,6 +138,15 @@ describe('patchGlobalServerResponse with redactInsteadOfThrow', () => {
     expect(resp.headers.get('content-length')).toBe(String(Buffer.byteLength(body)));
   });
 
+  it('switches from Content-Length when a split response may be redacted', async () => {
+    const resp = await fetch(`${baseUrl}/content-length-split-leak`);
+    const body = await resp.text();
+    expect(body).not.toContain(SECRET);
+    expect(body).toContain('▒');
+    expect(resp.headers.get('content-length')).toBeNull();
+    expect(resp.headers.get('transfer-encoding')).toBe('chunked');
+  });
+
   it('leaves text that only looks like the start of a secret intact', async () => {
     const body = await (await fetch(`${baseUrl}/partial-lookalike`)).text();
     expect(body).toBe(`<html>${SECRET.slice(0, 12)}-not-a-secret</html>`);

packages/varlock/src/runtime/test/patch-server-response.test.ts

@@ -17,12 +17,14 @@ import { patchGlobalServerResponse } from '../patch-server-response';
 import { resetRedactionMap } from '../env';
 
 const SECRET = 'super-secret-value-abc123';
+const UNICODE_SECRET = 'super-sëcret-value-abc123';
 
 const FAKE_GRAPH = {
   sources: [],
   settings: {},
   config: {
     SECRET_KEY: { value: SECRET, isSensitive: true },
+    UNICODE_SECRET_KEY: { value: UNICODE_SECRET, isSensitive: true },
     PUBLIC_KEY: { value: 'public-value', isSensitive: false },
   },
 } as any;
@@ -41,6 +43,21 @@ function makeRes(headers: Record<string, string> = {}) {
   return res;
 }
 
+async function gzipInTwoFlushes(input: Buffer, splitAt: number) {
+  const gz = zlib.createGzip();
+  const compressed: Array<Buffer> = [];
+  gz.on('data', (chunk) => compressed.push(chunk));
+  const first = await new Promise<Buffer>((resolve) => {
+    gz.write(input.subarray(0, splitAt));
+    gz.flush(zlib.constants.Z_SYNC_FLUSH, () => resolve(Buffer.concat(compressed.splice(0))));
+  });
+  const second = await new Promise<Buffer>((resolve) => {
+    gz.on('end', () => resolve(Buffer.concat(compressed)));
+    gz.end(input.subarray(splitAt));
+  });
+  return [first, second] as const;
+}
+
 beforeAll(() => {
   resetRedactionMap(FAKE_GRAPH);
   patchGlobalServerResponse();
@@ -172,26 +189,35 @@ describe('patched ServerResponse - sensitive values split across chunks', () =>
   });
 
   it('detects a secret split across gzip flush boundaries', async () => {
-    const gz = zlib.createGzip();
-    const compressed: Array<Buffer> = [];
-    gz.on('data', (c) => compressed.push(c));
+    const input = Buffer.from(`<html>leaked: ${SECRET}</html>`);
+    const splitAt = Buffer.byteLength(`<html>leaked: ${head}`);
     // Z_SYNC_FLUSH ends the first block on a byte boundary, which is how a server
     // streaming a compressed response emits a chunk mid-body
-    const first = await new Promise<Buffer>((resolve) => {
-      gz.write(`<html>leaked: ${head}`);
-      gz.flush(zlib.constants.Z_SYNC_FLUSH, () => resolve(Buffer.concat(compressed.splice(0))));
-    });
-    const second = await new Promise<Buffer>((resolve) => {
-      gz.on('end', () => resolve(Buffer.concat(compressed)));
-      gz.end(`${tail}</html>`);
-    });
+    const [first, second] = await gzipInTwoFlushes(input, splitAt);
 
     const res = makeRes({ 'content-encoding': 'gzip' });
     // each half decompresses on its own without ever containing the whole secret
     expect(() => res.write(first)).not.toThrow();
     expect(() => res.write(second)).toThrow(/DETECTED LEAKED SENSITIVE CONFIG/);
   });
 
+  it.each(['write', 'end'] as const)(
+    'detects a Unicode secret when a compressed delta ends mid-character before %s()',
+    async (completionMethod) => {
+      const input = Buffer.from(`<html>leaked: ${UNICODE_SECRET}</html>`);
+      const splitAt = input.indexOf(Buffer.from('ë')) + 1;
+      const [first, second] = await gzipInTwoFlushes(input, splitAt);
+      const firstDecompressed = zlib.unzipSync(first, {
+        finishFlush: zlib.constants.Z_SYNC_FLUSH,
+      });
+      expect(firstDecompressed.subarray(-1)).toEqual(Buffer.from('ë').subarray(0, 1));
+
+      const res = makeRes({ 'content-encoding': 'gzip' });
+      expect(() => res.write(first)).not.toThrow();
+      expect(() => res[completionMethod](second)).toThrow(/DETECTED LEAKED SENSITIVE CONFIG/);
+    },
+  );
+
   it('does not false-positive on text that merely starts like a secret', () => {
     const res = makeRes();
     expect(() => res.write(`<html>${head}`)).not.toThrow();