fix(web-api-discovery): the literal pass emitted a sentinel where a JSON value belonged - #485
Closed
MarkMichaelis wants to merge 1 commit into
Closed
MarkMichaelis wants to merge 1 commit into
MarkMichaelis wants to merge 1 commit into
Conversation
…SON value belonged
applyLiteralPass runs over the SERIALIZED HAR as text, so it does not know what
context a literal sat in. Replacing a quoted string is harmless -- the quotes
are already there. Replacing a bare JSON NUMBER is not:
"productIdentifier":61593494464534 -> "productIdentifier":<Sentinel>
An angle-bracket sentinel is not a JSON token. The document stops parsing, and
the leak gate passes it anyway, because the gate asks whether a secret survived
and never whether the document is still a document.
The placeholder is now quoted when it stands in a whole-value position --
preceded by `:`, `,` or `[` and followed by `,`, `}` or `]`. BOTH sides are
required: "the character before it is a quote" is not a sufficient test, because
a literal in the middle of a string has ordinary characters on both sides and
must not acquire quotes.
TWO CONTEXTS, AND THE SECOND IS THE ONE IT WAS FOUND IN.
A HAR carries bodies as strings, so an embedded document appears with its quotes
escaped (`\"id\":1`). Emitting a bare `"` there would repair the outer document
by breaking the inner one, so the quote is escaped to the depth it is inserted
at, read off the nearest preceding quote.
And by the time this pass runs, a form parameter the scrub rewrote has been
re-encoded (#483), so the payload's delimiters are `%3A` and `%7D` rather than
`:` and `}`. A version of this fix that understood only literal structure passed
all eight unit tests and left the real capture exactly as broken as before. The
check recognises percent-encoded delimiters too, and emits `%22` so the quote
matches the encoding it lands in.
Verified on every capture the corruption was measured in -- all three now scrub
to zero unparseable request payloads:
2026-09-07-071206 12 of 26 -> 0 of 26
2026-09-07-160347 59 of 110 -> 0 of 110
2026-09-10-124423 15 of 24 -> 0 of 24 (1 of 24 after #483 alone)
Eleven tests. Four are falsifiers; the rest are labelled GUARDS in the file,
including the two that pin the encoded path against a fix that quotes too much.
Closes #482.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4oCSX5MzcTAVKFWzFvZ9x
Contributor
Author
|
Closing unmerged. An independent review (Sonnet 5) reproduced three cases where this corrupts documents that were previously fine -- prose, CSV, and top-level JSON siblings. The approach is unsound rather than under-tested; details and the redirected approach are on #482. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #482. Together with #483 this closes #479 — all three captures the corruption was
measured in now scrub to zero unparseable request payloads.
The defect
applyLiteralPassruns over the serialized HAR as text, so it does not know what JSONcontext a literal sat in. Replacing a quoted string is harmless — the quotes are already
there. Replacing a bare JSON number is not:
An angle-bracket sentinel is not a JSON token. The document stops parsing, and the leak gate
passes it anyway — the gate asks whether a secret survived, never whether the document is
still a document.
The fix
The placeholder is quoted when it stands in a whole-value position: preceded by
:,,or
[and followed by,,}or], whitespace ignored.Both sides are required. "The character before it is a quote" is not a sufficient test —
a literal in the middle of a string has ordinary characters on both sides and must not
acquire quotes. Requiring structure on both sides admits exactly the whole-value positions.
Two contexts, and the second is the one it was found in
Nested. A HAR carries bodies as strings, so an embedded document appears with its quotes
escaped (
\"id\":1). Emitting a bare"there would repair the outer document by breakingthe inner one. The quote is escaped to the depth it lands at, read off the nearest preceding
quote.
Encoded. By the time this pass runs, a form parameter the scrub rewrote has been
re-encoded by #483 — so the payload's delimiters are
%3Aand%7D, not:and}.The check recognises percent-encoded delimiters and emits
%22, so the quote matches theencoding it is inserted into.
Verification — every capture the corruption was measured in
2026-09-07-0712062026-09-07-1603472026-09-10-124423The whole
scrubbed.harparses, and 0 of 470 URLs are unparseable.Tests
Eleven, in
har-literal-json-context.test.js. Four are falsifiers (verified failingagainst the code they fix); the rest are labelled GUARDS in the file, including the two
that pin the encoded path against a fix that quotes too much — an ordinary
av=<literal>&…form parameter must still be replaced bare.
The fixtures nest on purpose: a single-level fixture cannot express the escaped-depth
case and would pass a fix that is wrong one level down.
CI is billing-blocked, so all runs are local.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Q4oCSX5MzcTAVKFWzFvZ9x