From 0acdf4ef6aa577379e509a473e21773a6ec8b83f Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 7 Aug 2026 17:09:05 +0530 Subject: [PATCH 1/2] fix(redact): narrow control-split join guard to line-crossing spans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-merge review of aecb9ca89 found the join guard over-broad: skipping the join whenever ANY fragment self-matches _PREFIX_RE reopened a leak for non-newline splits — sk-<15 chars>ESC<25 chars> masked only the self-matching head and left the 25-char tail in cleartext (fully masked before the guard; main never masked this shape at all, so the merged state was still >= main, but the salvage's own coverage regressed). Skip the join only when the span crosses a line boundary (\n / \r) — that is the shape where adjacent legitimate text gets swallowed (ghp_-then-'button [ref=e3]' annotation bug). ESC/zero-width controls never legitimately separate a token from prose, so joining there is safe and restores full-tail masking. Both legs mutation-checked: reverting to the unconditional skip fails the new tail-mask test; removing the guard fails the annotation test. --- agent/redact.py | 21 +++++++++++++-------- tests/agent/test_redact.py | 11 +++++++++++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/agent/redact.py b/agent/redact.py index 1f4bbf583a637..8e5ed1eaf4379 100644 --- a/agent/redact.py +++ b/agent/redact.py @@ -507,14 +507,19 @@ def _mask_control_split_tokens(text: str, mask_fn) -> str: body = m.group(1) start_orig = orig_idx[m.start(1)] end_orig = orig_idx[m.end(1) - 1] + 1 - # If any fragment inside the original span already matches _PREFIX_RE - # on its own, the ordinary prefix pass will mask it — do NOT join. - # Joining here would swallow adjacent legitimate text: a complete - # token at end-of-line followed by a word line (``ghp_\n - # button [ref=e3]``) joins into one stripped-copy match and the - # mask eats ``button``. Join only when fragments alone are too - # short/broken to match (the actual smuggling shape). - if _PREFIX_RE.search(text[start_orig:end_orig]): + # If a fragment inside the span already matches _PREFIX_RE on its + # own AND the span crosses a LINE boundary (\n / \r), do NOT join. + # A complete token at end-of-line followed by a word line + # (``ghp_\nbutton [ref=e3]``) joins into one stripped-copy + # match and the mask eats ``button``. Line structure is legitimate; + # the self-matching fragment is handled by the ordinary prefix pass. + # For NON-newline controls (ESC, ZWSP, ...) the join proceeds even + # when a fragment self-matches: those bytes never legitimately sit + # between a token and adjacent prose, and skipping there let the + # non-matching remainder of a split token leak + # (``sk-\x1b`` masked only the head). + span = text[start_orig:end_orig] + if _PREFIX_RE.search(span) and ("\n" in span or "\r" in span): continue # Reject matches whose original span crosses a non-token char # (e.g. ``sk_abc…\nTAVILY_API_KEY=…`` — the ``=`` is not part of a diff --git a/tests/agent/test_redact.py b/tests/agent/test_redact.py index 65e9fe03b323c..b52c7c1aed91b 100644 --- a/tests/agent/test_redact.py +++ b/tests/agent/test_redact.py @@ -183,6 +183,17 @@ def test_complete_token_does_not_swallow_next_line(self): assert "button" in result assert "ref=e3" in result + def test_selfmatching_head_esc_split_tail_masked(self): + # A split where the HEAD fragment alone already matches _PREFIX_RE + # (>= 10 body chars) but the tail doesn't: the join must still run + # for non-newline controls, or the tail leaks in cleartext. Only + # LINE-crossing spans skip the join (see the annotation test). + head = "sk-" + "a" * 15 + tail = "b" * 25 + result = redact_sensitive_text(head + "\x1b" + tail, force=True) + assert tail not in result + assert "a" * 12 not in result + def test_env_dump_lines_not_joined(self): # Control-stripping must not join unrelated env lines into one match env_dump = ( From 589bb223eb0535e7d2a42459685eadcc26ebce0b Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 7 Aug 2026 17:46:34 +0530 Subject: [PATCH 2/2] polish: document newline residual, reuse span local, cheap check first Review follow-ups on the guard: state the accepted \n-residual in the comment, reuse the span local in the next condition instead of re-slicing, and short-circuit the substring checks before the regex. --- agent/redact.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/agent/redact.py b/agent/redact.py index 8e5ed1eaf4379..b5dca332c4ade 100644 --- a/agent/redact.py +++ b/agent/redact.py @@ -512,14 +512,16 @@ def _mask_control_split_tokens(text: str, mask_fn) -> str: # A complete token at end-of-line followed by a word line # (``ghp_\nbutton [ref=e3]``) joins into one stripped-copy # match and the mask eats ``button``. Line structure is legitimate; - # the self-matching fragment is handled by the ordinary prefix pass. + # the self-matching fragment is handled by the ordinary prefix pass + # (any remainder past the newline is left unmasked — accepted + # residual to preserve line structure). # For NON-newline controls (ESC, ZWSP, ...) the join proceeds even # when a fragment self-matches: those bytes never legitimately sit # between a token and adjacent prose, and skipping there let the # non-matching remainder of a split token leak # (``sk-\x1b`` masked only the head). span = text[start_orig:end_orig] - if _PREFIX_RE.search(span) and ("\n" in span or "\r" in span): + if ("\n" in span or "\r" in span) and _PREFIX_RE.search(span): continue # Reject matches whose original span crosses a non-token char # (e.g. ``sk_abc…\nTAVILY_API_KEY=…`` — the ``=`` is not part of a @@ -527,7 +529,7 @@ def _mask_control_split_tokens(text: str, mask_fn) -> str: # reject when the match runs into a ``KEY=`` name: a real token value # is followed by a newline/space/end, not ``=``. if (all(c in _TOKEN_BODY_CHARS or _CONTROL_CHARS_RE.match(c) - for c in text[start_orig:end_orig]) + for c in span) and (end_orig >= len(text) or text[end_orig] != "=")): matches.append((start_orig, end_orig, mask_fn(body))) for start_orig, end_orig, replacement in reversed(matches):