Skip to content

fix(redact): Python repr dict secrets no longer leak in tracebacks and pytest output (salvage #71370) - #92586

Merged
teknium1 merged 4 commits into
mainfrom
salvage/71370-python-repr-redact
Sep 14, 2026
Merged

teknium1 merged 4 commits into
mainfrom
salvage/71370-python-repr-redact

Conversation

@teknium1

Copy link
Copy Markdown
Collaborator

Summary

Opaque credentials in Python mapping repr output — {'BRAVE_API_KEY': 'opaque-value'} in tracebacks, pytest assertion introspection (E kwargs=...), and final exception lines — are now masked. The double-quoted JSON pass never covered single-quoted repr fields, so any dict repr surfaced by terminal/process output leaked opaque secrets verbatim.

Salvage of #71370 by @aerbaser (cherry-picked, authorship preserved), rebased onto current main and widened one notch: the key class is now case-insensitive for credential-suffixed names (UserPassword, sessionToken, clientApiKey) — the same uppercase-only gap OpenHands fixed in software-agent-sdk#4508.

Changes

  • agent/redact.py: new _redact_python_repr_fields() pass for single/double-quoted mapping-repr fields (str + bytes values, full mask so escaped atoms stay parseable); narrowly scoped _redact_python_diagnostic_repr_fields() applied to pytest E lines and final Error:/Exception: lines in otherwise source-preserving (code_file=True) terminal output; env-lookup values (os.getenv(...)) preserved like the ENV/JSON/YAML passes.
  • agent/redact.py: mixed-case credential-suffix widening (_PYTHON_REPR_CREDENTIAL_SUFFIXES, casefolded, suffix-only so TOKEN_COUNT/password_policy/tokenizer never match).
  • Conflict resolution keeps main's newer _command_reads_env_file gate in redact_terminal_output and adds the diagnostic pass on top.
  • tests/agent/test_redact.py: contributor's repr/diagnostic tests + new mixed-case positive/negative parametrizations.

Validation

Before After
{'api_key': 's3cr3t…'} leaked 'api_key': '***'
{'UserPassword': 'p@ss…'} leaked 'UserPassword': '***'
E kwargs={'env': {'BRAVE_API_KEY': …}} (pytest) leaked masked
CONFIG = {...} via cat config.py preserved preserved
{'TOKEN_COUNT': '12345'}, Secretary of the Treasury preserved preserved

tests/agent/test_redact.py: 139 passed.

Closes #71370.

Infographic

Secret Redaction: Python Repr Fields

@github-actions

github-actions Bot commented Aug 23, 2026 •

Copy link
Copy Markdown

૮ >ﻌ< ა ci review

ran on 7af2fc8 — fix(redact): repr pass leaves already-masked values alone

⚠️ Warnings

CI timings · View report · View job

Wall time 72m45s vs 4m51s (+1400.0%). 10 job(s) slower, 3 faster, 1 unchanged.

  • Check contributors / check-attribution: +23.0s
  • Python lints / Windows footguns (blocking): +20.0s
  • Python tests / Run tests: +15.0s
  • OS-specific tests / Windows-only tests: +14.0s
  • Detect affected areas: +8.0s

OSV vulnerability scan · View job

76 known vulnerabilities found in pinned dependencies.

How to fix:

Review the findings in the Security tab. Update the affected dependencies if a patched version is available.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/auth Authentication, OAuth, credential pools P3 Low — cosmetic, nice to have sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 23, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference, author can ignore or act on any point.

Solid addition to the redaction stack. The design choices are well-reasoned and tested against the actual failure modes: full *** masking instead of head/tail because escaped repr atoms can straddle slicing boundaries (with ast.literal_eval round-trip proofs for both str and bytes values, including apostrophes near quote edges), preservation of programmatic env lookups consistent with the ENV/JSON/YAML passes, and the terminal-output pass scoped narrowly to pytest E diagnostic lines plus final exception lines so code_file=True source-preservation semantics stay intact (agent/redact.py:1271-1279). Keeping TOKEN_COUNT/PASSWORD_POLICY-style metadata untouched while catching UserPassword/sessionToken shows the case analysis was done carefully.

Two points:

1. Suffix matching has predictable collateral on LLM-shaped fields — agent/redact.py:~684-691 (folded.endswith(_PYTHON_REPR_CREDENTIAL_SUFFIXES)): "token" as a bare suffix will mask next_token, stop_token, eos_token, logprobs token-adjacent keys whenever they flow through a repr into diagnostics. That's an availability trade, not a leak, and the upstream widening (#4508) evidently accepted it — but since Hermes transcripts contain streaming-response reprs far more often than the average Python project, one documented sentence (or a small allowlist like next_token/eos_token) would save someone a confusing "why is my pagination token starred" session. The existing negative tests (tokenizer, secretary, token_count) cover embedded words; these are suffix hits, which the suite currently doesn't pin either way.

2. Unrelated file in a focused security PR — contributors/emails/demerzel@hermes.local (+1 line) rides along with the redaction change. Harmless mailmap material, but it drags a second logical change into every backport/cherry-pick of this fix; consider splitting it out or landing it separately.

Minor observations, no action needed:

  • The key.isupper() + _PYTHON_REPR_ENV_SUFFIXES branch (agent/redact.py:~681-682) is fully subsumed by the casefolded-suffix fallback below it (every env suffix ends with a credential suffix), so it's dead weight — keeping it is fine if it documents intent.
  • The double gate ":" in text and "'" in text before running the repr regex (agent/redact.py:~1036) keeps the hot path clean; the same gating is correctly repeated before the per-line diagnostic pass.
  • Exception-line masking only fires on the final XxxError: ... line of a traceback; intermediate frames rarely carry repr payloads, and chained-exception causes do match their own final lines — coverage looks right.

The parametrized matrix across command shapes (python -m pytest, uv run, poetry run, bash -lc 'pytest ...') for deciding code_file is exactly where past regressions lived — good regression fence.

Demerzel (backend-eng) and others added 4 commits September 13, 2026 20:06
…ixes

Port from OpenHands/software-agent-sdk#4508: their dict-entry secret
redaction was uppercase-only and leaked mixed-case keys (UserPassword,
sessionToken). Apply the same case-insensitive treatment to the Python
mapping-repr pass: a casefolded credential suffix (apikey/token/secret/
password/passwd/credential) now qualifies a key, while metadata names
(TOKEN_COUNT, password_policy, tokenizer) stay untouched.
The Python-repr pass ran after the MCP probe header scrub and collapsed
'Authorization': 'Digest ***' to '***', erasing the scheme word that scrub
deliberately keeps (tests/hermes_cli/test_mcp_probe_redaction.py went red
on rebase). Skip values that already carry a mask marker.
@teknium1
teknium1 force-pushed the salvage/71370-python-repr-redact branch from 2af31a0 to 7af2fc8 Compare September 14, 2026 03:14
@teknium1
teknium1 merged commit 5280dec into main Sep 14, 2026
91 of 95 checks passed
@teknium1
teknium1 deleted the salvage/71370-python-repr-redact branch September 14, 2026 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants