The RFC 6750 guard in `_SUPPORT_BEARER_RE` kept any `<letters>="` after
Bearer as an auth-param, so `Bearer qwertyuiopas="v"` (and UPPER/Capitalised
or 20+ letter runs) reached the support upload whole; the merge base scrubbed
it. Found by the REDACT2 fold5 leak table (`="v"` tail), which must show zero
head-only leaks.
Keep only the named challenge params (realm, scope, error,
error_description, error_uri): `Bearer realm="api"` still stays, anything
else key-shaped is scrubbed (fail closed).
agent.redact rewrites `x-api-key: Bearer <k>` and `Authorization: *** <k>`
to `...: *** <k>` and keeps the key. The `***` scheme branch then let an
all-letter 8-19 char key (lower, UPPER or Capitalised) through as prose,
because `_NOT_WORD` treats such a run as a word; the merge base scrubbed any
`[A-Za-z0-9._~+/-]{8,}` value there (REDACT2 gate r5 2c, Medium: 3,488
head-only leaks, e.g. `x-api-key: *** qwertyuiopas`).
Put the merge base's 8+ branch first in the `***` alternation, with its own
boundary so `sk-a:b=v` shapes stay covered too. Zero head-only leaks is the
rule here; eating prose such as `configured` in `*** configured via env` is
accepted fail-closed over-redaction, so the two prose rows that pinned the
leaking behaviour go. `_NOT_WORD` and its 19-letter cap stay: the 4-7 char
and colon branches and free-text `Basic` still need them.
The key-end lookahead was copied verbatim into _KEY and _NOT_WORD; if one
copy changed, the "is it a key" and "is it a word" boundaries would drift
apart. Define it once. Compiled patterns are byte-identical before/after.
Gate r2 finding (quality Low).
The widened _SUPPORT_BEARER_RE value class admits `=`, so challenge text
became a "token": `Bearer realm="api"` -> `Bearer [REDACTED]"api"`,
`WWW-Authenticate: Bearer realm=example` -> `Bearer [REDACTED]`, which main
kept. Those params are what a 401 support bundle needs. Skip a value that
opens with a quoted auth-param (`name="`) or a known RFC 6750/7235 param name
(realm|scope|error|error_description|error_uri) followed by `=`. Tokens with
inner/trailing `=` (`abcd1234=efgh5678`, `abcd1234==x`, `Zm9v...NA==`) and a
`realmabc=...` lookalike are still masked.
Gate r2 finding (2c Low).
`_SUPPORT_BEARER_RE` matched a key-shaped prefix that had to end on a value
boundary, so `Bearer abcd1234=efgh5678 x`, `Bearer abcd1234==x` and
`Bearer Zm9vYmFyMTIzNA==X` matched nothing and leaked whole (REDACT2 gate 2c,
Medium). Free text has no header boundary: consume the value greedily
(`[KEY_CHARS=]+(:[KEY_CHARS=]+)*`), keeping the not-a-word guard and the
8-char lookahead so `Bearer tokens expire` stays prose.
`_NOT_WORD` kept any letters-only single-case run as prose, so a 32-char
all-lowercase or all-uppercase key after agent.redact's masked `***` scheme
(`x-api-key: *** <key>`, `--header x-api-key *** <KEY>`) reached the support
upload unredacted (REDACT2 gate 2c, Medium). Cap the word run at 19 letters:
no prose word in the kept rows comes close, and a longer single-case run is a key.
Follow-up to #132270. The support-upload scrub still let some credentials
through:
- short (<8 char) keys after a scheme or a masked `***` in header/argv
context (`x-api-key: Bearer abc1234`, `-H 'x-auth-token: Basic s3cr3t'`,
`--header x-api-key Basic short`);
- keys with an inner colon (`Bearer sk-abc:xyz123456`, `Basic user:passw0rd`);
- double-encoded URL params (`%253Ftoken%253D<T>`, `%2526access_token%253D<T>`).
It also deleted any all-letter word of 8+ chars after a masked value
(`x-api-key: *** undefined` -> `x-api-key: ***`).
A value after `***` now counts as a key when it is key-shaped: not a plain
word (all lower, Capitalised or ALL CAPS, case-sensitive), and either 4+
chars before any colon or 6+ chars mixing letters and digits. A short word
that ends the header (`Basic short`) also counts. A trailing colon is prose
punctuation, so `*** see: documentation` and `*** time: 12:30:45` stay. The
`--header NAME` operand form uses the same scheme rule. The encoded-param
pass also accepts the `%25`-prefixed separators and writes `%252A` stars.
Probe (fold/probe-REDACT2.py, real imports): base leaks 156/560 short-key
and 117/420 colon-key cases; head leaks 0 with 0 unstable, the 336-case
BR1 sweep stays at 0, prose is kept, and timing is linear from 100KB to 1MB.
Review finding (BR1 r3 efficiency, Low): the _ENCODED_URL_PARAM_RE key
group excluded only `%3D`, so a key could run across `%26`/`%3F`/`%23`/
`%3B`. Two consequences: a nested `?next=%2Fx%3Ffoo%26token%3D<secret>`
matched the key `foo%26token`, which is not a sensitive name, and the
secret leaked; and each separator restarted a match that scanned up to
160 chars, about 8x slower than base on 1 MiB of `%26a` (1.72s vs 0.21s;
0.36s now).
Exclude every encoded separator from the key so it ends at the nearest
one. The nested leak now redacts to `%3D%2A%2A%2A`, and the
`redirect=...%3Fapi_key%3D...` case is unchanged.
Review finding (BR1 r3 2c Medium / 2ab Low): the `***` arm added to
_BARE_HEADER_RE treated ANY same-line word after an already-masked
value as the secret, so benign prose was eaten and every re-scrub ate
one more word (`x-api-key: *** see docs for details` -> `... docs for
details` -> `... details`; same for `Authorization: *** then prose`,
`X-Api-Key: *** (set in env)`). The scrub was not a fixed point.
Require the follow-on value to look like a credential (>=8 token chars,
optional `=` padding, then a token boundary) before `***` counts as a
masked scheme. The floor is 8, not 16, so 15-char keys still redact.
The 336-case header sweep stays at 0 leaks / 0 unstable, the 18
`name: <scheme> <key>` cases are still redacted, and the benign lines
are stable over three passes.
agent.redact's _SECRET_HEADER_RE treats the first word after an API-key
header as the value. With `x-api-key: Basic <key>`, that word is the scheme,
so the scheme becomes `***` and the key is left in the text. The support scrub
then saw `***` as the value and stopped, so `x-api-key: *** <key>` reached the
debug/share upload with the raw key. This affects the x-api-key, api-key and
x-auth-token families with Bearer/Basic/Digest, in both bare and `-H '...'`
forms. Base and main leak the same input.
_BARE_HEADER_RE now also accepts `***` plus a same-line value as a scheme.
That value must not be another secret header, so `x-api-key: *** cookie=...`
is not consumed. The key is redacted, and the redacted output is a fixed
point. The fix stays in the support layer. Teaching agent.redact the scheme
does not help, because its YAML/assignment passes mask the scheme word first.
Gate finding: BR1 fold2 "still open", a bare-header scheme leak.
In `(?>(?:Bearer|Basic|Digest)\s+)?` the `?` sat outside the atomic group, so
the engine could skip the group and let the `(?!\[REDACTED\])` lookahead pass
on the scheme word itself: re-scrubbing `--header Authorization Bearer
[REDACTED]` gave `--header Authorization [REDACTED] [REDACTED]`. Moving the
optional quantifier inside the atomic group makes the scheme consumed whenever
present, so already-redacted values are a fixed point.
Gate finding: BR1 r2 2c (Low), scheme idempotence.
Reuse review (Low): _decoded_compact_name re-implemented
agent.redact._canonical_url_param_name (3-round unquote_plus + casefold).
Compacting the canonical name is equivalent (0 mismatches over the
sensitive list + 20k random %-nested/+/-/_ names), so build on it and
drop the local loop and unquote_plus import.
Gate review (Low, convergent 2c/efficiency/quality/reuse):
_SENSITIVE_HEADER_COMPACT_NAMES lost its only reader with the removed
structured walker, and _redact_log_text / redact_debug_support_error
were pure pass-throughs to redact_debug_support_text. Delete all three
and call redact_debug_support_text at every site (debug.py, dashboard
status router, /debug slash command, share_nous RPC); test docstrings
now name the real function.
Gate 2c + quality review (Low, convergent): the header/argv value
classes matched "[REDACTED" inside an already-scrubbed value, so a
second pass rendered "cookie=[REDACTED]]". That second pass is real:
upload_to_pastebin scrubbed each per-service error and every caller
(debug share, dashboard, /debug) scrubbed the wrapping RuntimeError
again. Skip already-redacted values with a (?!\[REDACTED\]) lookahead
in the four value-group regexes (scheme prefix made atomic so it cannot
backtrack around the guard), and drop the inner scrub in
upload_to_pastebin since all callers redact the combined error.
redact_debug_support_text ran redact_for_egress (itself a forced
redact_sensitive_text pass) and then a second forced pass with URL
credentials; collect_share_bundle then re-scrubbed every bundle member
and build_nous_bundle walked it a third time. A ~1MB log took ~1.0s vs
0.32s for one pass, per log. Run redact_sensitive_text once (the
support bearer sweep already covers redact_for_egress's residue
sweep), drop the bundle re-scrub and the Nous re-walk (every member is
already scrubbed at capture), and with them the now-unused structured
value walker. Also drop _redact_log_text's ignored
redact_url_credentials kwarg and fail closed with the shared
REDACTION_UNAVAILABLE sentinel.
The support scrub re-listed agent.redact's sensitive query params and
secret header names; two copies of one pattern list drift. Derive the
support-only compact names from _SENSITIVE_QUERY_PARAMS (plus the five
support-only extras) and expose the header names as a tuple that both
the agent regex and the support module build from. agent.redact is a
hard dependency, so import it at module level and drop the try/except
around is_secret_field_name.
