Skip to content

Commit 21f2eed

Browse files
committed
fix byte duplication when a chunk splits a character before a redaction
Once a binary chunk ends mid-character the streaming decoder holds its tail bytes, but the raw chunk (partial bytes included) was still passed through. If a later chunk then got rewritten (redaction or holdback), re-encoding the decoded text emitted those held bytes a second time. Now the response switches to emitting re-encoded text from the first mid-character split onward. Also logs a debug line when a compressed end() chunk fails to decode instead of silently skipping the scan.
1 parent 0c9b770 commit 21f2eed

2 files changed

Lines changed: 52 additions & 1 deletion

File tree

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ type ScanState = {
6363
/** streaming decoder, so a multi-byte character split across chunks doesn't decode to garbage
6464
* (created lazily - responses that never hit the binary path don't need one) */
6565
decoder: TextDecoder | undefined,
66+
/** set once a binary chunk ends mid-character: the decoder is holding its tail bytes, so raw
67+
* chunks no longer line up with the decoded text and all further output must be re-encoded
68+
* from it (otherwise the held bytes would go out twice if a later chunk gets rewritten) */
69+
reEncode: boolean,
6670
zlibChunks: Array<Buffer>,
6771
/** streaming decoder for decompressed deltas, which may end inside a multi-byte character */
6872
decompressedDecoder: TextDecoder | undefined,
@@ -76,6 +80,7 @@ function getScanState(res: any): ScanState {
7680
pending: '',
7781
carry: '',
7882
decoder: undefined,
83+
reEncode: false,
7984
zlibChunks: [],
8085
decompressedDecoder: undefined,
8186
decompressedLength: 0,
@@ -100,6 +105,25 @@ function decodeDecompressedDelta(state: ScanState, decompressed: Buffer, isFinal
100105
return state.decompressedDecoder.decode(delta, { stream: !isFinal });
101106
}
102107

108+
/**
109+
* Number of bytes at the end of `chunk` that are the start of an incomplete UTF-8 character
110+
* (0 when the chunk ends on a character boundary). These are the bytes a streaming
111+
* TextDecoder holds back until the rest of the character arrives.
112+
*/
113+
function incompleteTrailingUtf8Bytes(chunk: Uint8Array): number {
114+
/* eslint-disable no-bitwise */
115+
for (let i = 1; i <= 3 && i <= chunk.length; i++) {
116+
const byte = chunk[chunk.length - i];
117+
if ((byte & 0b11000000) === 0b10000000) continue; // continuation byte - keep looking for the lead
118+
if ((byte & 0b11100000) === 0b11000000) return i < 2 ? i : 0; // 2-byte lead
119+
if ((byte & 0b11110000) === 0b11100000) return i < 3 ? i : 0; // 3-byte lead
120+
if ((byte & 0b11111000) === 0b11110000) return i < 4 ? i : 0; // 4-byte lead
121+
return 0; // ascii or invalid lead - the decoder holds nothing
122+
}
123+
return 0;
124+
/* eslint-enable no-bitwise */
125+
}
126+
103127
function clearPendingFlush(state: ScanState) {
104128
if (state.flushTimer !== undefined) {
105129
clearTimeout(state.flushTimer);
@@ -230,6 +254,7 @@ export function patchGlobalServerResponse(opts?: {
230254
} else if (!compressionType) {
231255
chunkType = 'encoded';
232256
chunkStr = decodeChunk(state, rawChunk);
257+
if (!state.reEncode && incompleteTrailingUtf8Bytes(rawChunk)) state.reEncode = true;
233258
} else {
234259
const decompress = getDecompressor(String(compressionType).toLowerCase());
235260
if (decompress) {
@@ -287,6 +312,11 @@ export function patchGlobalServerResponse(opts?: {
287312
return true;
288313
}
289314
args[0] = chunkType === 'encoded' ? new TextEncoder().encode(emit) : emit;
315+
} else if (chunkType === 'encoded' && state.reEncode && emit) {
316+
// the decoder is holding tail bytes of a split character, so the raw chunk no
317+
// longer lines up with the decoded text - emit re-encoded text instead of the
318+
// raw bytes to keep the outgoing byte stream consistent
319+
args[0] = new TextEncoder().encode(emit);
290320
}
291321
}
292322
}
@@ -319,7 +349,8 @@ export function patchGlobalServerResponse(opts?: {
319349
try {
320350
decompressed = decompress(Buffer.concat(state.zlibChunks));
321351
} catch (err) {
322-
// stream didn't decode, nothing more we can do at this point
352+
// stream didn't decode, nothing more we can do at this point (fails open)
353+
debug(`⚠️ leak scan skipped - compressed response did not decode at end() (${compressionType})`);
323354
}
324355
}
325356
if (decompressed !== undefined) {
@@ -367,6 +398,10 @@ export function patchGlobalServerResponse(opts?: {
367398
if (!this.headersSent && this.getHeader('content-length') !== undefined) {
368399
this.setHeader('content-length', Buffer.byteLength(emit));
369400
}
401+
} else if (isBinaryChunk && state.reEncode && emit) {
402+
// raw bytes stopped lining up with the decoded text earlier in the response
403+
// (a chunk ended mid-character), so the final chunk is re-encoded too
404+
args[0] = new TextEncoder().encode(emit);
370405
}
371406
}
372407

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,13 @@ beforeAll(async () => {
7373
// ends mid-lookalike but never completes the secret - must arrive intact
7474
res.write(`<html>${SECRET.slice(0, 12)}`);
7575
res.end('-not-a-secret</html>');
76+
} else if (req.url === '/multibyte-split-then-redact') {
77+
// first chunk ends mid-character (its lead byte must not go out raw), and the
78+
// chunk completing it contains a secret, so the rest gets re-encoded on redaction
79+
const buf = Buffer.from(`<html>é leak: ${SECRET}</html>`);
80+
const mid = buf.indexOf(0xc3) + 1; // one byte into the 2-byte é
81+
res.write(buf.subarray(0, mid));
82+
res.end(buf.subarray(mid));
7683
} else if (req.url === '/gzip-leak') {
7784
res.setHeader('content-encoding', 'gzip');
7885
try {
@@ -147,6 +154,15 @@ describe('patchGlobalServerResponse with redactInsteadOfThrow', () => {
147154
expect(resp.headers.get('transfer-encoding')).toBe('chunked');
148155
});
149156

157+
it('does not duplicate bytes when a chunk splits a character before a redaction', async () => {
158+
const body = await (await fetch(`${baseUrl}/multibyte-split-then-redact`)).text();
159+
expect(body).not.toContain(SECRET);
160+
expect(body).toContain('▒');
161+
// a duplicated lead byte would decode as a replacement char before the é
162+
expect(body.startsWith('<html>é leak: ')).toBe(true);
163+
expect(body).not.toContain('�');
164+
});
165+
150166
it('leaves text that only looks like the start of a secret intact', async () => {
151167
const body = await (await fetch(`${baseUrl}/partial-lookalike`)).text();
152168
expect(body).toBe(`<html>${SECRET.slice(0, 12)}-not-a-secret</html>`);

0 commit comments

Comments
 (0)