From 8563fe34359381a100d13b3d62b17a9a89df402e Mon Sep 17 00:00:00 2001 From: Soheil Fakour Date: Thu, 6 Aug 2026 19:26:08 -0400 Subject: [PATCH] fix(redact): close emission gaps - env suffix keys, control-char splits, process(list) (#77484) --- agent/redact.py | 98 ++++++++++++++++++++++++++-- tests/agent/test_redact.py | 62 ++++++++++++++++++ tests/tools/test_process_registry.py | 18 +++++ tools/process_registry.py | 7 +- 4 files changed, 180 insertions(+), 5 deletions(-) diff --git a/agent/redact.py b/agent/redact.py index df2565b21b68b..7e29e73e97c99 100644 --- a/agent/redact.py +++ b/agent/redact.py @@ -140,10 +140,26 @@ _PREFIX_PATTERNS = [ # ENV assignment patterns: KEY=value where KEY contains a secret-like name. # Uppercase keys tolerate spaces around "=" (e.g. ``FOO_SECRET = bar``) because # an all-caps key is almost never prose/code. -_SECRET_ENV_NAMES = r"(?:API_?KEY|TOKEN|SECRET|PASSWORD|PASSWD|CREDENTIAL|AUTH)" +# Bare ``KEY`` / ``PASS`` / ``PW`` suffixes are included (``FAL_KEY=…``, +# ``MYSQL_PASS=…``, ``DB_PW=…``) — issue #77484. The regex is IGNORECASE so +# lowercase env names (``openai_key=…``) are caught here too. The secret name +# must sit at a word boundary (``_``-delimited or whole-word) so generic +# prose words (``password=``, ``token=``, ``KEYBOARD=``, ``PASSAGE=``) do not +# match — those are handled by the config/form/URL paths, and a bare +# ``password=…`` in a form body must not be swallowed greedily by ``\S+``. +_SECRET_ENV_NAMES = r"(?:API_?KEY|KEY|TOKEN|SECRET|PASSWORD|PASSWD|PASS|PW|CREDENTIAL|AUTH)" +# Uppercase keys keep the legacy embedded match (``MYTOKEN=…``, ``FOO_SECRET``) +# — an all-caps key is almost never prose. _ENV_ASSIGN_RE = re.compile( rf"([A-Z0-9_]{{0,50}}{_SECRET_ENV_NAMES}[A-Z0-9_]{{0,50}})\s*=\s*(['\"]?)(\S+)\2", ) +# Lowercase env names: only underscore-boundary forms (``openai_key=…``, +# ``FAL_KEY=…``, ``db_pw=…``) — NOT bare ``password=``/``token=``/``secret=``, +# which appear in prose, URLs, and form bodies (issue #77484). +_ENV_ASSIGN_LOWER_RE = re.compile( + rf"([a-z0-9_]+(?:_|^)(?:key|pass|pw|token|secret|password|passwd|credential|auth)(?=[^a-z0-9_]|$))\s*=\s*(['\"]?)(\S+)\2", + re.IGNORECASE, +) # Lowercase / dotted / hyphenated config keys from config files # (application.properties, .env, YAML-ish dumps): ``spring.datasource.password=secret``, @@ -236,8 +252,8 @@ _YAML_ASSIGN_RE = re.compile( # match. ALL-CAPS keys keep the legacy embedded matching (``MYTOKEN=…``) — an # all-caps key is almost never prose, the same rationale as _ENV_ASSIGN_RE. _KEY_KEYWORD_RE = re.compile( - r"(?:api|auth|access|refresh|session|secret)[ _.\-]?(?:key|token)" - r"|token|secret|passwd|password|credential|auth", + r"(?:api|auth|access|refresh|session|secret)[ _.\\-]?(?:key|token)" + r"|token|secret|passwd|password|pass|pw|credential|auth|key", re.IGNORECASE, ) @@ -283,7 +299,15 @@ def _key_has_secret_keyword(key: str) -> bool: """ letters = [c for c in key if c.isalpha()] if letters and all(c.isupper() for c in letters): - return True # legacy all-caps behavior (MYTOKEN=…) + # Legacy all-caps behavior (MYTOKEN=…): an all-caps key is almost + # never prose. Exception: a bare ``KEY``/``PASS``/``PW`` embedded in + # a longer all-caps word (``KEYBOARD``, ``PASSAGE``) is prose, not a + # credential — only a word-bounded compound (``API_KEY``, + # ``MYSQL_PASSWORD``, ``FAL_KEY``, ``DB_PW``) counts (issue #77484). + for m in _KEY_KEYWORD_RE.finditer(key): + if _is_word_start(key, m.start()) and _is_word_end(key, m.end()): + return True + return False for m in _KEY_KEYWORD_RE.finditer(key): if _is_word_start(key, m.start()) and _is_word_end(key, m.end()): return True @@ -440,12 +464,63 @@ _FORM_BODY_RE = re.compile( r"^[A-Za-z_][A-Za-z0-9_.-]*=[^&\s]*(?:&[A-Za-z_][A-Za-z0-9_.-]*=[^&\s]*)+$" ) +# Control / zero-width characters that can split a token body: a secret +# smuggled as ``sk-abc\x1bdef…`` or ``ghp_abc\n123…`` escapes the contiguous +# prefix regexes (issue #77484). Used by _mask_control_split_tokens. +_CONTROL_CHARS_RE = re.compile( + r"[\x00-\x1f\x7f\u200b-\u200f\u2028-\u202f\u2060\ufeff]" +) + +# Union of every _PREFIX_PATTERNS body class — a control-stripped match may +# only span original chars that are token-body or control chars (see +# _mask_control_split_tokens). ``=`` is deliberately excluded: a KEY=value +# assignment separator must never let a match span across unrelated text. +_TOKEN_BODY_CHARS = frozenset( + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_-." +) + # Compile known prefix patterns into one alternation _PREFIX_RE = re.compile( r"(? str: + """Mask tokens whose body is split by control/zero-width characters. + + A credential like ``sk-abc\\x1bdef456…`` or ``ghp_abc\\n123def…`` has its + token body interrupted, so the contiguous _PREFIX_RE cannot match it and + the secret leaks verbatim (issue #77484). Strategy: build a copy with all + control chars removed (the token is contiguous again, matching even when + each fragment alone is too short), match on that, then mask the + corresponding span in the *original* — but only when the original span + contains solely token-body and control chars (a match that crosses into a + different line's unrelated text, e.g. ``EXA_API_KEY=*** is rejected). + """ + stripped = _CONTROL_CHARS_RE.sub("", text) + if stripped == text: + return text + orig_idx = [i for i, c in enumerate(text) if not _CONTROL_CHARS_RE.match(c)] + out = list(text) + matches = [] + for m in _PREFIX_RE.finditer(stripped): + body = m.group(1) + start_orig = orig_idx[m.start(1)] + end_orig = orig_idx[m.end(1) - 1] + 1 + # Reject matches whose original span crosses a non-token char + # (e.g. ``sk_abc…\nTAVILY_API_KEY=…`` — the ``=`` is not part of a + # token body, so the regex matched across unrelated lines). Also + # 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]) + 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): + out[start_orig:end_orig] = list(replacement) + return "".join(out) + + def mask_secret( value: str, *, @@ -725,6 +800,13 @@ def redact_sensitive_text( # Known prefixes (sk-, ghp_, etc.) — gate on substring presence if _has_known_prefix_substring(text): _prefix_sub = _mask_token_nonreusable if file_read else _mask_token + # Control/zero-width chars (\\n, \\r, ESC, U+200B, …) split a token + # body so _PREFIX_RE cannot match across them — a secret smuggled as + # ``sk-abc\\x1bdef…`` leaks verbatim (issue #77484). Mask such runs by + # first matching on a control-stripped copy, then re-masking the + # corresponding span in the original (the stripped copy and the + # original are aligned 1:1 for non-control chars). + text = _mask_control_split_tokens(text, _prefix_sub) text = _PREFIX_RE.sub(lambda m: _prefix_sub(m.group(1)), text) # ENV assignments: OPENAI_API_KEY=*** (skip for code files — false positives) @@ -746,6 +828,14 @@ def redact_sensitive_text( return m.group(0) return f"{name}={quote}{_mask_token(value)}{quote}" text = _ENV_ASSIGN_RE.sub(_redact_env, text) + # Lowercase env names (``openai_key=…``). Skip URLs — the query + # string may contain ``token=``/``key=`` params that are + # intentionally passed through (see note near the bottom of this + # function; _redact_strict_url_credentials handles the opt-in + # case). The uppercase regex above is all-caps-only, so it never + # matches URL params; the lowercase one would (issue #77484). + if "://" not in text: + text = _ENV_ASSIGN_LOWER_RE.sub(_redact_env, text) # Lowercase/dotted config keys (issue #16413). Skip URLs entirely — # web-URL query params are intentionally passed through (see note # near the bottom of this function); _DB_CONNSTR_RE still guards diff --git a/tests/agent/test_redact.py b/tests/agent/test_redact.py index 740e3c70eb90a..d218e3c5841a6 100644 --- a/tests/agent/test_redact.py +++ b/tests/agent/test_redact.py @@ -114,6 +114,68 @@ class TestEnvAssignments: assert "mypassword" not in result +class TestBareSecretEnvSuffixes: + """Bare *_KEY / *_PASS / *_PW env suffixes mask, incl. lowercase — #77484.""" + + def test_upper_suffix_keys_mask(self): + for text in ("FAL_KEY=sk-abc123def456", "OPENAI_KEY=sk-abc123def456", + "MYSQL_PASS=ghi789", "DB_PW=jkl012"): + result = redact_sensitive_text(text, force=True) + assert "=" in result and result.split("=", 1)[1] != text.split("=", 1)[1] + + def test_lowercase_env_name_masks(self): + result = redact_sensitive_text("openai_key=sk-abc123def456", force=True) + assert "sk-abc123def456" not in result + + def test_prose_words_with_keyword_unchanged(self): + # KEYBOARD / PASSAGE embed the bare keyword but are prose, not creds + for text in ("KEYBOARD=notsecret", "PASSAGE=notsecret"): + result = redact_sensitive_text(text, force=True) + assert result == text + + def test_form_body_not_swallowed(self): + # A bare `password=`/`token=` in a form body must not be eaten greedily + text = "password=mysecret&username=bob&token=opaqueValue" + result = redact_sensitive_text(text, force=True) + assert "mysecret" not in result + assert "opaqueValue" not in result + assert "username=bob" in result + + +class TestControlCharSplitTokens: + """Tokens split by control/zero-width chars must still mask — #77484.""" + + def test_newline_split_token_masks(self): + tok = "ghp_abcdef1234567890ABCDEF1234567890abcdef" + text = f"token={tok[:10]}\n{tok[10:]}" + result = redact_sensitive_text(text, force=True) + assert tok not in result + + def test_esc_split_token_masks(self): + tok = "ghp_abcdef1234567890ABCDEF1234567890abcdef" + text = f"token={tok[:10]}\x1b{tok[10:]}" + result = redact_sensitive_text(text, force=True) + assert tok not in result + + def test_zero_width_split_token_masks(self): + tok = "ghp_abcdef1234567890ABCDEF1234567890abcdef" + text = f"token={tok[:10]}\u200b{tok[10:]}" + result = redact_sensitive_text(text, force=True) + assert tok not in result + + def test_env_dump_lines_not_joined(self): + # Control-stripping must not join unrelated env lines into one match + env_dump = ( + "HOME=/home/user\n" + "ELEVENLABS_API_KEY=sk_abc123def456ghi789jkl\n" + "EXA_API_KEY=exa_XY789abcdef01234\n" + "SHELL=/bin/bash\n" + ) + result = redact_sensitive_text(env_dump, force=True) + assert "SHELL=/bin/bash" in result + assert "HOME=/home/user" in result + + class TestEnvLookupPreserved: """Programmatic env var lookups must not be corrupted (issue #2852).""" diff --git a/tests/tools/test_process_registry.py b/tests/tools/test_process_registry.py index 93c2cfcf82816..0ad0c658e5847 100644 --- a/tests/tools/test_process_registry.py +++ b/tests/tools/test_process_registry.py @@ -1417,6 +1417,24 @@ class TestHandleProcessRedaction: out = json.loads(pr._handle_process({"action": "poll", "session_id": sess.id})) assert "abc123def456" not in out["output_preview"] + def test_list_redacts_command_and_output(self, monkeypatch): + """`process(action=list)` redacts command + output_preview — issue #77484. + + The list branch previously returned raw ``command[:200]`` and + ``output_preview[-200:]`` with no redaction wrap, leaking inline + secrets (unlike poll/log/wait/kill). + """ + pr, sess = self._setup( + monkeypatch, "curl -H 'Authorization: Bearer sk-abc123def456ghi789jkl012345'", + "opaque token sk-proj-AAAABBBBCCCCDDDDEEEEFFFFGGGG output", + ) + out = json.loads(pr._handle_process({"action": "list"})) + assert len(out["processes"]) >= 1 + entry = out["processes"][0] + assert "sk-abc123def456ghi789jkl012345" not in entry["command"] + assert "sk-proj-AAAABBBBCCCCDDDDEEEEFFFFGGGG" not in entry["output_preview"] + assert "curl" in entry["command"] + def test_disabled_passes_through(self, monkeypatch): import agent.redact as _r monkeypatch.setattr(_r, "_REDACT_ENABLED", False) diff --git a/tools/process_registry.py b/tools/process_registry.py index 29af51f000680..c04148e47e145 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -2493,7 +2493,12 @@ def _handle_process(args, **kw): except Exception: session_key = "" return json.dumps( - {"processes": process_registry.list_sessions(task_id=task_id, session_key=session_key or None)}, + { + "processes": [ + _redact_process_result(p) + for p in process_registry.list_sessions(task_id=task_id, session_key=session_key or None) + ] + }, ensure_ascii=False, ) elif action in {"poll", "log", "wait", "kill", "write", "submit", "close"}: