diff --git a/agent/redact.py b/agent/redact.py index 6c59a91df6c14..0479640e0bcfb 100644 --- a/agent/redact.py +++ b/agent/redact.py @@ -367,6 +367,72 @@ def _should_redact_assignment(key: str, value: str, *, check_keyword: bool) -> b _JSON_KEY_NAMES = r"(?:api_?[Kk]ey|token|secret|password|access_token|refresh_token|auth_token|bearer|secret_value|raw_secret|secret_input|key_material)" _JSON_FIELD_RE = re.compile(rf'("{_JSON_KEY_NAMES}")\s*:\s*"([^"]+)"', re.IGNORECASE) +# Python ``repr`` uses single-quoted mapping fields, so opaque credentials in +# tracebacks and pytest failure introspection bypass the double-quoted JSON rule +# above: ``{'BRAVE_API_KEY': 'opaque-value'}``. Capture identifier-shaped keys +# here, then apply the canonical high-confidence key policy in the callback. +_PYTHON_REPR_SECRET_KEYS = frozenset({ + "access_token", + "refresh_token", + "id_token", + "auth_token", + "token", + "api_key", + "apikey", + "client_secret", + "secret", + "password", + "passwd", + "private_key", + "credential", + "credentials", + "authorization", + "bearer", + "secret_value", + "raw_secret", + "secret_input", + "key_material", +}) +_PYTHON_REPR_ENV_SUFFIXES = ( + "_API_KEY", + "_TOKEN", + "_SECRET", + "_PASSWORD", + "_PASSWD", + "_CREDENTIAL", + "_CREDENTIALS", +) +# Casefolded credential suffixes for mixed/camel-case key names +# (``UserPassword``, ``sessionToken``, ``clientApiKey``). Suffix-only so +# ``token_count`` / ``password_policy`` metadata keys never match. Widened per +# OpenHands/software-agent-sdk#4508. +_PYTHON_REPR_CREDENTIAL_SUFFIXES = ( + "apikey", + "api_key", + "token", + "secret", + "password", + "passwd", + "credential", + "credentials", +) +_PYTHON_REPR_FIELD_RE = re.compile( + r"'(?P[A-Za-z_][A-Za-z0-9_]*)'(?P\s*:\s*)" + r"(?:" + r"(?P[bB]?)'(?P(?:\\.|[^'\\])+)'" + r"|(?P[bB]?)\"(?P(?:\\.|[^\"\\])+)\"" + r")" +) + +# Terminal/process output normally uses ``code_file=True`` to preserve source. +# Add repr masking only to high-confidence diagnostic lines: pytest assertion +# introspection (``E ...``) and final Python exception lines. +_PYTEST_DIAGNOSTIC_LINE_RE = re.compile(r"^(?P[ \t]*E[ \t]{2,})(?P.*)$") +_PYTHON_EXCEPTION_LINE_RE = re.compile( + r"^(?P(?:[A-Za-z_]\w*\.)*[A-Za-z_]\w*" + r"(?:Error|Exception|Warning):[ \t]*)(?P.*)$" +) + # Authorization / Proxy-Authorization, any scheme or bare credential; header # name and scheme word preserved. The credential class excludes quotes: pulling # a closing quote into the mask turns value corruption into SYNTAX corruption @@ -515,6 +581,79 @@ def _mask_token(token: str) -> str: return mask_secret(token, head=6, tail=4, floor=18) +def _is_python_repr_secret_key(key: str) -> bool: + """Return True for exact secret keys or credential-suffixed key names.""" + folded = key.casefold() + if folded in _PYTHON_REPR_SECRET_KEYS: + return True + if key.isupper() and key.endswith(_PYTHON_REPR_ENV_SUFFIXES): + return True + # Mixed/camel-case keys ending in a credential word (``UserPassword``, + # ``sessionToken``, ``clientApiKey``) — the exact-set and uppercase-suffix + # rules above miss these. Suffix-only matching keeps metadata names like + # ``TOKEN_COUNT`` / ``PASSWORD_POLICY`` / ``SECRET_NAME`` untouched. + # Class widened per OpenHands/software-agent-sdk#4508 (their dict-entry + # redaction was uppercase-only and leaked mixed-case keys). + return folded.endswith(_PYTHON_REPR_CREDENTIAL_SUFFIXES) + + +def _redact_python_repr_fields(text: str) -> str: + """Fully mask credential fields in Python mapping ``repr`` output.""" + def _sub(match: re.Match) -> str: + key = match.group("key") + if not _is_python_repr_secret_key(key): + return match.group(0) + + single_value = match.group("single_value") + if single_value is not None: + prefix = match.group("single_prefix") or "" + quote = "'" + value = single_value + else: + prefix = match.group("double_prefix") or "" + quote = '"' + value = match.group("double_value") + + # Mapping repr can contain code-shaped fixture values too. Preserve + # programmatic env lookups just like the ENV/JSON/YAML passes do. + if _ENV_LOOKUP_VALUE_RE.match(value): + return match.group(0) + # An upstream pass (MCP probe header scrub, _mask_token) already masked this + # value; re-masking would erase the scheme word it deliberately kept + # (``'Authorization': 'Digest ***'`` → ``'***'``). + if "***" in value or value.startswith("«redacted:"): + return match.group(0) + # Do not retain head/tail characters here: escaped repr atoms can cross + # a slicing boundary and leave an unescaped quote behind. A full mask is + # parseable for both str and bytes values and leaks no opaque bytes. + return f"'{key}'{match.group('sep')}{prefix}{quote}***{quote}" + + return _PYTHON_REPR_FIELD_RE.sub(_sub, text) + + +def _redact_python_diagnostic_repr_fields(text: str) -> str: + """Mask repr fields only on pytest/error lines in source-preserving output.""" + lines = text.splitlines(keepends=True) + for index, line in enumerate(lines): + ending = "" + body_line = line + if line.endswith("\r\n"): + body_line, ending = line[:-2], "\r\n" + elif line.endswith("\n") or line.endswith("\r"): + body_line, ending = line[:-1], line[-1:] + + match = _PYTEST_DIAGNOSTIC_LINE_RE.match(body_line) + if match is None: + match = _PYTHON_EXCEPTION_LINE_RE.match(body_line) + if match is not None: + lines[index] = ( + match.group("prefix") + + _redact_python_repr_fields(match.group("body")) + + ending + ) + return "".join(lines) + + def _redact_query_string(query: str) -> str: """Replace values of sensitive ``k=v&k=v`` params with ``***``; others pass through.""" if not query: @@ -624,6 +763,11 @@ def _redact_assignments(text: str) -> str: text = _JSON_FIELD_RE.sub( _assignment_sub(lambda g: f'{g[0]}: "{_mask_token(g[1])}"', check_keyword=False), text) + # Python mapping repr fields ({'API_KEY': '…'}): single-quoted, so the JSON rule + # above never sees them — the traceback / pytest-introspection leak shape. + if ":" in text and "'" in text: + text = _redact_python_repr_fields(text) + # YAML after JSON: quoted values are handled there (_YAML_ASSIGN_RE skips quotes). if ":" in text and "://" not in text: text = _YAML_ASSIGN_RE.sub( @@ -670,6 +814,10 @@ def redact_sensitive_text(text: str, *, force: bool = False, code_file: bool = F Every regex sits behind a cheap substring gate that its pattern requires, so the gates are never false-negative. + Set code_file=True to also skip the Python-repr mapping pass (``{'API_KEY': '…'}`` + fixtures in source); pytest/exception diagnostic lines get a narrow pass in + redact_terminal_output instead. + Set file_read=True for file *content* returned to the agent (read_file / search_files / cat). The old mask looked like a real-but-truncated key, so an agent reading it from config.yaml and writing it back silently corrupted the stored credential into a dead 13-char value → 401 (issue #35519). The sentinel is @@ -892,7 +1040,13 @@ def redact_terminal_output(output: str, command: str | None = None, *, force: bo if not output: return output code_file = not (is_env_dump_command(command) or _command_reads_secret_file(command)) - return redact_sensitive_text(output, force=force, code_file=code_file) + redacted = redact_sensitive_text(output, force=force, code_file=code_file) + # Source-preserving output still gets the Python-repr pass on high-confidence + # diagnostic lines (pytest ``E `` introspection, final exception lines): that is + # where {'BRAVE_API_KEY': '…'} leaks, not in source dumps. + if code_file and (force or _redact_enabled()) and ":" in redacted and "'" in redacted: + redacted = _redact_python_diagnostic_repr_fields(redacted) + return redacted # --- Prefix pre-screen: derived from _PREFIX_PATTERNS so a new prefix can't diff --git a/contributors/emails/demerzel@hermes.local b/contributors/emails/demerzel@hermes.local new file mode 100644 index 0000000000000..fab86918beae2 --- /dev/null +++ b/contributors/emails/demerzel@hermes.local @@ -0,0 +1 @@ +aerbaser diff --git a/tests/agent/test_redact.py b/tests/agent/test_redact.py index 316f64669a4b8..3df2b82879dab 100644 --- a/tests/agent/test_redact.py +++ b/tests/agent/test_redact.py @@ -1,5 +1,6 @@ """Tests for agent.redact -- secret masking in logs and output.""" +import ast import logging import pytest @@ -290,6 +291,104 @@ def test_json_non_secret_unchanged(self): assert result == text +class TestPythonReprFields: + @pytest.mark.parametrize( + "key", + [ + "BRAVE_API_KEY", + "SERVICE_TOKEN", + "DB_PASSWORD", + "api_key", + "access_token", + "client_secret", + "id_token", + "private_key", + ], + ) + def test_secret_field_in_nested_python_repr_is_redacted(self, key): + secret = f"opaque-{key.lower()}-value-1234567890" + text = f"kwargs={{'env': {{'{key}': '{secret}'}}}}" + + result = redact_sensitive_text(text, force=True) + + assert secret not in result + assert f"'{key}': '***'" in result + + @pytest.mark.parametrize( + "key", + ["TOKEN_COUNT", "AUTH_METHOD", "PASSWORD_POLICY", "SECRET_NAME", "CREDENTIAL_TYPE"], + ) + def test_uppercase_metadata_field_is_unchanged(self, key): + text = f"{{'{key}': 'public-metadata-value'}}" + assert redact_sensitive_text(text, force=True) == text + + @pytest.mark.parametrize( + "key", + ["UserPassword", "sessionToken", "clientApiKey", "gh_token", "webhookSecret"], + ) + def test_mixed_case_credential_suffix_key_is_redacted(self, key): + """Case-insensitive dict-entry class per OpenHands/software-agent-sdk#4508.""" + secret = f"opaque-{key.lower()}-value-1234567890" + text = f"{{'{key}': '{secret}'}}" + + result = redact_sensitive_text(text, force=True) + + assert secret not in result + assert f"'{key}': '***'" in result + + @pytest.mark.parametrize( + "key", + ["tokenizer", "secretary", "password_policy", "token_count", "keyring"], + ) + def test_embedded_or_metadata_keyword_key_is_unchanged(self, key): + text = f"{{'{key}': 'ordinary-value-1234567890'}}" + assert redact_sensitive_text(text, force=True) == text + + @pytest.mark.parametrize( + "value", + [ + "apostrophe-near-head-'1234567890", + "apostrophe-near-tail-1234567890'", + b"bytes-apostrophe-near-head-'1234567890", + b"bytes-apostrophe-near-tail-1234567890'", + ], + ) + def test_escaped_repr_value_remains_parseable(self, value): + text = repr({"API_KEY": value}) + + result = redact_sensitive_text(text, force=True) + parsed = ast.literal_eval(result) + + assert parsed["API_KEY"] == (b"***" if isinstance(value, bytes) else "***") + + def test_double_quoted_repr_value_is_redacted(self): + secret = "opaque-double-quoted-value-1234567890" + text = f"payload={{'SERVICE_TOKEN': \"{secret}\"}}" + + result = redact_sensitive_text(text, force=True) + + assert secret not in result + assert "'SERVICE_TOKEN': \"***\"" in result + + def test_programmatic_env_lookup_is_preserved(self): + text = "{'API_KEY': \"os.getenv('OPENAI_API_KEY')\"}" + assert redact_sensitive_text(text, force=True) == text + + def test_non_secret_python_repr_field_is_unchanged(self): + text = "{'model': 'gpt-5', 'token_count': '123'}" + assert redact_sensitive_text(text, force=True) == text + + def test_already_masked_repr_value_keeps_its_scheme_word(self): + # An upstream scrub (MCP probe headers) leaves ``Digest ***``; the repr + # pass must not collapse that to a bare ``***`` and lose the scheme. + text = "headers={'Authorization': 'Digest ***'}" + assert redact_sensitive_text(text, force=True) == text + + def test_code_file_preserves_secret_shaped_fixture(self): + text = "CONFIG = {'BRAVE_API_KEY': 'fixture-value-1234567890'}" + assert redact_sensitive_text(text, force=True, code_file=True) == text + + class TestAuthHeaders: @@ -1051,6 +1150,66 @@ def test_disabled_passes_through(self, monkeypatch): red = redact_terminal_output(out, "printenv") assert "zzzopaque1234567890abcdef" in red + @pytest.mark.parametrize( + "command", + [ + "python -m pytest", + "uv run pytest", + "poetry run pytest", + "coverage run -m pytest", + "PYTHONPATH=. pytest", + "/usr/bin/env pytest", + "bash -lc 'pytest tests/agent/test_redact.py'", + ], + ) + def test_pytest_diagnostic_masks_python_repr_secret_field(self, command): + from agent.redact import redact_terminal_output + + secret = "opaque-brave-value-1234567890" + out = f"E kwargs={{'env': {{'BRAVE_API_KEY': '{secret}'}}}}" + + red = redact_terminal_output(out, command, force=True) + + assert secret not in red + assert "'BRAVE_API_KEY': '***'" in red + + def test_exception_line_masks_python_repr_secret_field(self): + from agent.redact import redact_terminal_output + + secret = "opaque-exception-value-1234567890" + out = f"RuntimeError: payload={{'API_KEY': '{secret}'}}" + + red = redact_terminal_output(out, "python app.py", force=True) + + assert secret not in red + assert "'API_KEY': '***'" in red + + def test_source_dump_preserves_python_repr_fixture(self): + from agent.redact import redact_terminal_output + + out = "CONFIG = {'BRAVE_API_KEY': 'fixture-value-1234567890'}" + assert redact_terminal_output(out, "cat config.py", force=True) == out + assert redact_terminal_output(out, "python dump_source.py", force=True) == out + + def test_pytest_source_line_preserves_python_repr_fixture(self): + from agent.redact import redact_terminal_output + + source = " CONFIG = {'BRAVE_API_KEY': 'fixture-value-1234567890'}" + out = source + "\nE assert False" + + red = redact_terminal_output(out, "uv run pytest", force=True) + + assert source in red + + def test_disabled_pytest_diagnostic_passes_through(self, monkeypatch): + from agent.redact import redact_terminal_output + + monkeypatch.setattr("agent.redact._REDACT_ENABLED", False) + secret = "opaque-disabled-value-1234567890" + out = f"E payload={{'API_KEY': '{secret}'}}" + + assert redact_terminal_output(out, "uv run pytest") == out + class TestFileReadNonReusableRedaction: """#35519: prefix-matched credentials in FILE CONTENT (read_file /