Repository navigation
Catch sensitive values split across response chunks - #981
Conversation
The ServerResponse patch scanned each write()/end() chunk on its own, and detection is a substring match against complete values, so a secret split across a chunk boundary passed both scans. Streaming SSR flushes at arbitrary points, so this is the normal case rather than an edge case. Each chunk is now scanned with the tail of the previous one, and trailing text that looks like the start of a sensitive value is held back until the next chunk (flushed on a short timer so a paused stream is not stalled). The same carry-over is applied to the ReadableStream scanner used by the edge/Response paths. Also fixed alongside it: - end() now handles compressed chunks, which were never decompressed or scanned - binary chunks decode with a streaming decoder, so a multi-byte character split across chunks no longer decodes to replacement characters - end() redacts under redactInsteadOfThrow instead of always throwing, and recomputes Content-Length when redaction shortens the body
|
The changes in this PR will be included in the next version bump.
|
📦 Bundle size
dist/ only; native binaries are versioned separately and not counted here. |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Compressed Unicode secrets can still bypass the new boundary scan, and split redaction can leave an invalid Content-Length that hangs the client. Both paths need correction before merge.
Reviewed changes across the Node and Web streaming leak scanners, response redaction behavior, focused runtime tests, documentation, and release metadata.
- Cross-chunk scanning: Adds per-response carry and pending state for Node responses, plus carry and streaming UTF-8 decoding for Web
ReadableStreambodies. - Response redaction: Holds possible secret prefixes briefly, redacts final chunks in Next.js development, and updates
Content-Lengthfor rewritten final bodies when possible. - Compressed responses: Accumulates compressed chunks, scans newly decoded output, and adds final compressed-chunk scanning.
- Regression coverage: Adds ASCII split-boundary, gzip, multibyte pass-through, timed-flush, redaction, and response-framing tests.
- User-facing guidance: Documents cross-chunk detection and the 100 ms holdback, with a patch release entry.
azure/gpt-5.6-sol ｜ 𝕏
varlock
@varlock/native-helper-darwin
@varlock/native-helper-linux-arm64
@varlock/native-helper-linux-x64
@varlock/native-helper-win32-x64
commit: |
|
Addressed and resolved both review threads in commit Task list (5/5 completed)
|
Fragments that split a secret arrive within the same tick or a few ms, so a 15ms flush valve covers them the same as 100ms while keeping the worst-case stall of a lookalike tail imperceptible. New tests: callback fires when a whole chunk is withheld, bare end() flushes withheld text, and a secret completed after the timed flush is still caught via carry.
e147fed to
0c9b770
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focused on the revised holdback timing and added regression coverage.
- Shortened holdback window: Reduced the pending prefix flush timeout from 100 ms to 15 ms while retaining carry-based detection after a timed flush.
- Expanded holdback coverage: Added tests for detection after timer expiry, callback completion when an entire write is withheld, and pending text delivery through a bare
end(). - Updated documentation: Aligned the documented maximum response delay with the new 15 ms timeout.
azure/gpt-5.6-sol ｜ 𝕏
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
varlock-website | 0c9b770 | Commit Preview URL Branch Preview URL |
Aug 25 2026, 06:43 PM |
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.
There was a problem hiding this comment.
Important
The new re-encoding path can mutate clean responses for mixed chunk types and malformed UTF-8, so the byte-stream integrity regressions should be fixed before merge.
Reviewed changes since the prior Pullfrog review, focused on the binary decoder-tail correction and its new regression coverage.
- Added persistent re-encoding: Switched subsequent binary output to re-encoded decoded text after a chunk ends inside a UTF-8 character, preventing a held lead byte from being emitted twice during later redaction.
- Added redaction regression coverage: Verified that a valid multibyte split before a redacted secret produces the intended character without duplicate or replacement bytes.
azure/gpt-5.6-sol ｜ 𝕏
Address two review findings on the re-encode fix: - The trailing-byte check used broad bitmasks, so invalid UTF-8 leads (0xC0/0xC1, 0xF5+) and out-of-range second bytes counted as held even though TextDecoder replaces them immediately. That flipped reEncode and mutated malformed-but-clean bytes. The check now mirrors the decoder's actual hold conditions (verified against TextDecoder behavior). - A response switching from binary to string chunks while the decoder held tail bytes dropped or reordered those bytes. String writes now flush the held bytes in place (as a replacement char, matching what a client would have decoded) and rewrite comparisons use the original chunk, so a flushed tail is never silently discarded in write() or end().
There was a problem hiding this comment.
Important
The mixed binary-to-string path still duplicates held bytes when the binary chunk has no decoded prefix, so one response-integrity regression remains before merge.
Reviewed changes since the prior Pullfrog review, focused on the UTF-8 decoder-state corrections and their regression coverage.
- Tightened incomplete-sequence detection: Restricted re-encoding to valid UTF-8 leads and valid second-byte ranges so malformed lead bytes continue passing through unchanged.
- Flushed decoder state before strings: Prepended a pending decoder replacement before subsequent string writes and final chunks, then realigned the raw-output comparison.
- Expanded socket coverage: Added binary-to-string and invalid-lead-byte response tests.
azure/gpt-5.6-sol ｜ 𝕏
A binary write consisting only of an incomplete UTF-8 sequence decodes to an empty string, so the re-encode rewrite was skipped and the raw bytes went out anyway - then the flush on the next string chunk emitted the same held bytes again as a replacement char. Such a chunk is now reported as written without sending anything, matching the withheld-text path; its bytes go out once the character completes or the held tail is flushed.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focused on the remaining decoder-only binary chunk correction and its regression coverage.
- Withheld decoder-only chunks: Prevented a binary chunk made entirely of an incomplete UTF-8 sequence from being sent raw before the decoder later flushes or completes it.
- Added socket regression coverage: Verified that an incomplete lead-byte chunk followed by a string
end()produces one replacement character rather than duplicated malformed output.
azure/gpt-5.6-sol ｜ 𝕏
Property tests that split fixtures into random mixes of string and Buffer chunks (including mid-character byte splits) across 30 seeds each, asserting three invariants over a real socket: clean responses arrive byte-for-byte intact, throw mode never delivers the secret, and redact mode produces exactly the redacted text. A deterministic seeded PRNG makes any failure replayable. Verified against prior revisions: these tests catch the byte-duplication, mixed-chunk, and wholly-held-chunk bugs fixed in the last three commits.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focused on the new deterministic fuzz coverage for response chunking.
- Added reproducible chunk fuzzing: Introduced a seeded generator that mixes string and binary chunks, including byte splits inside multibyte characters and both supported response-ending paths.
- Verified scanner invariants over sockets: Added exact assertions that clean responses remain byte-for-byte intact, throw mode never delivers the secret, and redaction mode emits precisely the expected redacted body across 30 chunkings.
azure/gpt-5.6-sol ｜ 𝕏
2 participants
Varlock's response leak scanner checked each
ServerResponse.write()/end()chunk in isolation, and detection is a substring match against complete values. A sensitive value split across a chunk boundary was therefore present in neither scan and went out to the client, even though the same value in a single chunk was blocked. Streaming SSR flushes at arbitrary points, so this is the normal case for the workload the patch was written for, not an edge case.The application controls where chunks break, not a remote caller, so this is a gap in a safety net rather than a new way to reach secrets. It still means the net silently fails open on its main use case.
What changed
write→end,write→write, and the gzip/br/zstd delta path.getRedactionHoldbackLength, already used by CLI output redaction.ReadableStreamscanner inenv.ts, which the edgeResponsepatch and the Cloudflare integration use.Fixed alongside it, in the same code paths:
end()never decompressed compressed chunks, so a secret in a final compressed chunk was not scanned at all.TextDecoder. A multi-byte character split across chunks previously decoded to replacement characters, a second source of missed matches and a corruption risk once chunks get rewritten. Once a chunk ends mid-character, the response switches to emitting re-encoded decoded text, so the held tail bytes can't be sent twice when a later chunk gets rewritten.end()now redacts underredactInsteadOfThrowinstead of always throwing, matchingwrite(). This is required for the splitwrite→endcase to be redactable, and it replaces the hung request the old code left behind (there was a TODO on that line about it). It affects Next.js dev only; production and every other integration still throw.Content-Lengthis recomputed. Next.js setsContent-Lengthfor non-streamed payloads and sends them in a singleend(), so a stale length left the client waiting on bytes that never arrived.Clean responses stay byte-for-byte identical: the outgoing chunk is only rewritten when something was actually redacted or held back.
Testing
17 new tests across the three runtime test files, each confirmed to fail against the unpatched source and pass with the fix. A seeded fuzz suite (30 chunkings per property over a real socket) additionally asserts that clean responses arrive byte-for-byte intact, throw mode never delivers the secret, and redact mode produces exactly the redacted text; it reproduces every byte-integrity bug fixed during review when run against the earlier revisions. Regression guards cover held-back text delivered intact, multi-byte splits, the timed flush arriving before
end(), the write callback firing when a whole chunk is withheld, bareend()flushing withheld text, detection still working after a timed flush,Content-Lengthcorrection, and text that merely starts like a secret not being flagged.Full framework test suite run locally: 670 passed across Astro 5/6/7, Next.js 14/15/16 (webpack + turbopack), Vite 5/6/7/8, Cloudflare, TanStack Start, SvelteKit, Expo, and vanilla-node. Two suites hit harness
beforeAlltimeouts under concurrent load and passed on a targeted re-run (200/200).Reported by @7thParkk.