fix(redact): strip complete CSI/SGR sequences in token-split shadow (#81012) - #81060
fix(redact): strip complete CSI/SGR sequences in token-split shadow (#81012)#81060Enough1122 wants to merge 1 commit into
Conversation
…ousResearch#81012) `_mask_control_split_tokens` stripped only the bare ESC byte, leaving the CSI parameter bytes (`[32m` etc.) glued to the token head. The literal `m` defeated `_PREFIX_RE`'s `(?<![A-Za-z0-9_-])` lookbehind so neither the ordinary prefix pass nor the split-join pass matched and the secret (`sk-…`, `ghp_…`, etc.) leaked verbatim in colored log output. Strip complete CSI sequences (`\x1b[...letter`) from the shadow copy alongside the existing bare control char strip, and update the maskable-span check to also strip CSI so CSI parameter bytes inside a span don't get rejected as `unknown non-token chars`. Regression tests cover: * `\x1b[32m sk-… \x1b[0m` — the colored-log header case, * `sk-<head>\x1b[32m<tail>` — CSI between fragments (the bare-ESC strip used to defeat the join), * `\x1b[1;32mWelcome to the dashboard\x1b[0m` — CSI in prose with no token must still pass through unchanged.
alt-glitch
added
type/security
|
Collision note (from a full open-PR scan): #81121 (Parker-Fawcett, opened later) addresses the same issue #81012 with the same approach in the same files ( |
|
Closing in favor of #81121 per maintainer @andrexibiza's routing decision on issue #81012: the five-head matrix test showed #81121 as the only implementation covering both the CSI-glue leak and mixed ESC+newline cases, with this PR classified as partial coverage and the explicit instruction to 'route the fix through #81121'. Happy to contribute this branch's CSI regression tests to #81121 if useful. Branch left for manual cleanup. |
I'll never say no to more regression tests. Thank you for your work! |
3 participants
Fixes #81012
_mask_control_split_tokensstripped only the bare ESC byte, leavingthe CSI parameter bytes (
[32metc.) glued to the token head. Theliteral
m(or any other letter) defeated_PREFIX_RE's(?<![A-Za-z0-9_-])lookbehind, so neither the ordinary prefix passnor the split-join pass matched and the secret leaked verbatim — common
when logs are colored with
\x1b[32mor a terminal adds a SGRprefix around a tool output line.
This PR:
_CSI_SEQUENCE_REmatching the SGR-relevant CSI subset(
\x1b\[[0-9;?]*[ -/]*[@-~])._CSI_SEQUENCE_REand_CONTROL_CHARS_REapplied in that order (CSI matches first, so wenever delete just the ESC byte and leave the parameter bytes glued to
adjacent text).
orig_idxfromtextdirectly, skipping both CSIsequences and bare control chars in one pass.
CSI parameter bytes inside a span don't get rejected as "unknown
non-token chars".
Regression tests:
tests/agent/test_redact.py::test_complete_csi_sgr_glued_to_token_masked—\x1b[32m sk-… \x1b[0m(the colored-log header case).tests/agent/test_redact.py::test_csi_between_token_fragments_joins—sk-<head>\x1b[32m<tail>, the bare-ESC strip used to defeat the join.tests/agent/test_redact.py::test_csi_within_prose_does_not_match—CSI in prose with no token must still pass through unchanged.