mirror of
https://github.com/stablyai/orca.git
synced 2026-09-25 08:02:31 +00:00
m4air
4057dd32f4
One lookbehind in the path redactor fails in opposite directions on the two
shapes a `file://` URL can take.
`PATH_PATTERNS` rule 5 is `(?<![A-Za-z0-9./])\/[A-Za-z0-9._-]+\/…`. Its
lookbehind rejects a slash preceded by a slash.
- On POSIX a frame reads `file:///Users/alice/…`. The path's own leading slash
sits behind the scheme's third slash, so the rule never fires and NOTHING is
redacted — the account name and full filesystem layout ship in the report.
- On Windows the same frame reads `file:///C:/Users/alice/…`. The drive letter
interposes, the leading slash is preceded by `:`, and the rule fires.
So Windows file URLs were redacted and POSIX ones were not. Three payloads in a 326-file corpus carry a real account name: 25 occurrences,
13, and one with 48 path prefixes holding the name twice each — 96 literal
occurrences of a developer's account in a single submitted report, via
`/home/<name>/src/<name>/`.
Replaying those three payloads' actual leaking lines through both versions:
134 account-name occurrences before, 13 after. The 13 are payload C, whose
vector is a Vite dev-server URL this change does not address (below).
This adds two rules at the head of `PATH_PATTERNS`: a quoted `file://` URL, then
an unquoted one. Quoted first, so a URL holding spaces is taken whole rather
than truncated at the first one — that case previously leaked the entire path.
WHAT THIS DELIBERATELY DOES NOT DO
An earlier revision also preserved a stack frame's asset name and `:line:col`
while redacting its directory, because those offsets are the only axis a
minified React stack can be clustered on, and Windows loses all of them: across
17 minified React #185 payloads, darwin and linux kept 130 of 130 frames and
win32 kept 0 of 40.
That half is withdrawn. Gating on "ends in .js with a line and column" preserved
the basename of any user file with that shape. Gating instead on "carries a
bundler content hash" only narrowed it, because the shape it actually tests is
"the last 8 characters are not all lowercase letters" — which admits YYYYMMDD
dates, `_v2` suffixes, and every capitalised eight-letter dictionary word.
A grammar fuzz found 691,200 inputs where a user-identifying basename survived
the new code and did not survive the old, including
`tax-return-20241231.js` and `therapy-journal-20250101.js`. Against 5,000
ordinary basenames from this repo's node_modules, 99.3% become gate-passing once
one common date or version suffix is appended.
Two further defects went with it: running the frame pass first destroyed the
quoted-path rules' anchor, so a quoted region beginning with a frame fell
through to the unquoted rules and leaked from the first space onward; and its
two lookaheads made the regex quadratic on adversarial input — 4.1s at 50k
chars, 64s at 200k, on the unclamped breadcrumb path in the main process. With
the pass removed, 100k chars of the worst shape cost 0.10ms.
Preserving build frames is worth doing and needs a gate that constrains the
whole basename rather than its last 8 characters. It is not worth rushing into
the code path that decides what leaves a user's machine.
COST, STATED PLAINLY — AND IT IS SMALLER THAN IT FIRST LOOKS
darwin and linux stacks lose the frame `:line:col` offsets they currently keep,
because they were only keeping them by not being redacted at all. Windows is
byte-identical — it already lost them.
What survives on every platform: all 62 frame lines of an 8000-char stack, the
complete ordered minified function-name sequence (`_i, Ac, zv, Ll, kt, …`), the
component-stack component names, and `boundary_id` / `surface`. The function-name
sequence is the primary fingerprint crash aggregators group on; `line:col` is
the build-specific, weaker key. So these reports do not become unclusterable.
And redaction shortens the output enough to escape the 4000-char stack cap that
was silently truncating these. Measured on five real payloads (4 darwin, 1
linux) that previously shipped at exactly 4003 chars — the truncation marker:
they now ship at 899 / 1428 / 1476 / 1476 / 1499 chars, untruncated, with the
minified function-name sequence identical to the input in all five. Frame counts
come out equal rather than higher (37/37, 56/56, 57/57), because every over-cap
stack in the corpus is already one of the old code's truncated artifacts, so its
pre-truncation frame count is unrecoverable. The headroom gain is real and
measured; a frame-count gain is plausible but not demonstrable from this corpus.
A follow-up worth doing, and measured: preserving the `:line:col` alone — with
no basename — would restore the offset axis on every platform with no leak
surface at all, since digits carry nothing. Of the 17 payloads whose error_stack
holds the minified #185 string, 13 still carry offsets, and clustering those on
the INNERMOST THREE frames gives exactly 2 shapes (8 and 5). Stated precisely
because it matters: the full ordered offset sequence gives 9 clusters, since
outer frames drift per React build and the app-bundle frame moves every release.
The innermost frames are the stable part.
RESIDUALS — ONE OF THEM NEW
- A Vite dev-server frame `http://localhost:5173/@fs/Users/<name>/…` still
ships the account name. Not the `?v=` query or the `.tsx` extension, as an
earlier draft of this message claimed: nothing anchors inside an `http://`
URL at all, because every separator in `/@fs/Users/` is preceded by an
alphanumeric. One corpus payload leaks a name 13 times this way.
- `~user/…`, `.../Users/…` and `webpack:///Users/…` are unhandled.
- An UNQUOTED path containing a literal space still stops at the space. Worst
case is a Windows profile folder from a domain display name, leaving surname
and employer. Quoted `file://` URLs holding spaces are now taken whole,
which is new.
- NEW, and the one thing here that is not pre-existing: redaction happens
BEFORE truncation, so collapsing a long `file://` URL to `[redacted-path]`
makes room under the length cap for content that previously fell past it. In
a constructed dev-mode stack mixing `file://` frames with `/@fs/` frames, the
old output truncated away all ten `/@fs/` frames and the new one ships all
ten. Net leakage falls in every case measured this way (37→10, 34→10, 30→10,
20→10 name occurrences) — but that does not generalise: a constructed input
whose old output was name-sparse and whose newly-reachable tail is name-dense
can raise the count, worst observed 5→515. Nothing newly exposed is content
the old code redacted; it is unredacted residual whose reachability
increased. No corpus payload contains both shapes.
- Diagnostic BUNDLES, as opposed to crash payloads, are not sanitized by this
code at all: roughly 274-282 of 321 expose 210-212 distinct home-directory
usernames across some 800,000 occurrences (the range is definition-sensitive). That is a separate pipeline, and roughly four orders of
magnitude larger than the payload leak this commit fixes.
No test covered a `file://` URL, which is how this shipped. There are now 49 in
this file: a per-platform frame case for darwin, linux, win32 URL and win32
backslash; nine document-shaped fixtures, each carrying a real `:line:col`, kept
so a future attempt to preserve frames has to clear the counterexamples that
killed this one; and three pinning the properties review found load-bearing but
untested — the `i` flag (an uppercase scheme was previously redacted by nothing
at all), the rule's inability to cross a newline, and quote-adjacency, which is
what stops it stealing an opening quote the way the withdrawn revision did.
One change post-dates the final review round and carries a round less scrutiny
than the rest: the unquoted rule was `file:\/\/\/?(?:[A-Za-z]:)?[\\/]…`, which
required a separator where an authority can appear, so `file://localhost/…`,
`file://host/share/…` and `file://Users/…` escaped every rule in the list. A
test added for those found it. It now takes everything after the scheme, which
is strictly more redaction; the `+` keeps the bare token in prose intact, and a
replay of all 31,497 payload lines leaves the same 13 residual account-name
prefixes, all Vite.
Review: four rounds. Round 1 found the frame pass preserved any `.js` basename;
round 2 found the hash gate meant to fix that was really a "last 8 characters are
not all lowercase" gate, with 691,200 leaking fuzz cases, and that the pass also
broke the quoted rules' anchor and made the regex quadratic. Removing the pass
was round 2's own recommendation. Round 3 fuzzed the remainder against the
pre-commit baseline over 604,800 grammar cases and 272,571 realistic cases and
found zero regressions, confirmed the anchor break is structurally impossible for
these two rules, and measured the worst adversarial shape at 2.9ms per 200k
chars against the removed pass's 64 seconds.