Skip to content

fix(redact): mask secrets in Python mapping reprs - #71370

Closed
aerbaser wants to merge 1 commit into
NousResearch:mainfrom
aerbaser:fix/redact-python-repr-secret-fields
Closed

aerbaser wants to merge 1 commit into
NousResearch:mainfrom
aerbaser:fix/redact-python-repr-secret-fields

Conversation

@aerbaser

Copy link
Copy Markdown

What does this PR do?

Masks opaque credential values embedded in Python mapping repr output, including pytest/traceback shapes such as:

E       kwargs={'env': {'BRAVE_API_KEY': 'opaque-provider-value'}}

Hermes already redacts KEY=value, double-quoted JSON fields, known vendor prefixes, headers, URLs, and other credential forms. Python repr normally uses single-quoted mapping fields, so an opaque value with no recognized vendor prefix bypassed both redact_sensitive_text(..., force=True) and pytest diagnostic output.

Before

sample = "kwargs={'env': {'BRAVE_API_KEY': 'opaque-provider-value-1234567890'}}"
redact_sensitive_text(sample, force=True) == sample

After

kwargs={'env': {'BRAVE_API_KEY': '***'}}

The full value is replaced with *** rather than a head/tail mask. This prevents an escaped quote from being split at a masking boundary and keeps real repr() output parseable for both str and bytes values.

Design

  • Captures identifier-shaped keys, then applies a high-confidence policy:
    • exact established credential fields (api_key, access_token, client_secret, private_key, etc.);
    • uppercase env keys ending in credential suffixes (_API_KEY, _TOKEN, _PASSWORD, etc.).
  • Explicitly leaves metadata such as TOKEN_COUNT, AUTH_METHOD, PASSWORD_POLICY, SECRET_NAME, and CREDENTIAL_TYPE unchanged.
  • Supports single- or double-quoted values, optional b prefixes, and escaped characters.
  • Preserves programmatic env lookup fixtures such as {'API_KEY': "os.getenv('OPENAI_API_KEY')"}.
  • Keeps code_file=True byte-preserving.
  • At terminal/process boundaries, applies repr masking only to high-confidence pytest diagnostic lines (E ...) and final Python *Error / *Exception / *Warning lines. Ordinary source emitted by cat, Python scripts, or pytest remains unchanged.
  • Diagnostic classification is content-based rather than launcher-based, so wrapped invocations (uv run pytest, poetry run pytest, coverage run -m pytest, /usr/bin/env pytest, etc.) behave consistently.
  • Uses the existing redaction preference and force=True behavior; no new config or tool surface.

Related work

No exact open issue/PR for generic Python mapping repr redaction was found.

Type of change

  • Bug fix
  • Security hardening
  • Tests

Verification

scripts/run_tests.sh \
  tests/agent/test_redact.py \
  tests/tools/test_terminal_output_transform_hook.py -q
→ 198 passed

uvx --from ruff==0.15.10 ruff check agent/redact.py tests/agent/test_redact.py
→ All checks passed

python3 -m py_compile agent/redact.py tests/agent/test_redact.py
git diff --check
→ passed

Additional synthetic verification:

  • 32-case escaped str/bytes repr oracle passed with ast.literal_eval;
  • 6 negative metadata-key controls passed;
  • 7 wrapped pytest launcher diagnostics passed;
  • Python/cat/pytest source-preservation controls passed;
  • a 1,000,039-character escaped repr value was fully masked in ~109 ms on the test host.

All test values are synthetic; no real credentials are used.

Checklist

  • Scoped to agent/redact.py and tests/agent/test_redact.py
  • Existing source-code false-positive protections preserved
  • Focused canonical test runner passed
  • Ruff and syntax checks passed
  • No config/schema/tool changes
  • Linux verification

@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 labels Jul 25, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for tracing the single-quoted repr() gap. Current main still lacks this coverage: agent/redact.py:700-764 has only ENV, double-quoted JSON, and YAML structured-field passes, so the reported opaque dict-repr shape remains exposed.

Problems

  • tools/code_execution_tool.py:1152-1156 and :1572-1580 redact executor output with code_file=True. The PR adds the generic repr pass only under not code_file and calls its diagnostic-only helper only from redact_terminal_output; therefore equivalent traceback/pytest diagnostics from execute_code remain unmasked.

Suggested changes

  • Reuse the narrow diagnostic repr pass for both code-execution output paths, with regression tests covering stdout/stderr diagnostics and source-preservation controls.
  • During salvage, retain current main's false-positive guard at agent/redact.py:245-260 and config-regex pre-gate at :724-733; GitHub currently reports this PR as merge-conflicted.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Salvaged onto current main in #92586 (your commit cherry-picked, authorship preserved) with two additions: the conflict resolution keeps main's newer .env-file-read gate in redact_terminal_output, and the repr key class was widened to mixed-case credential suffixes (UserPassword, sessionToken) per the same gap OpenHands fixed in software-agent-sdk#4508. Thanks @aerbaser — credit stays with you in the git history. #92586 will close this when it merges.

RFingAdam pushed a commit to RFingAdam/hermes-agent that referenced this pull request Aug 26, 2026
teknium1 added a commit that referenced this pull request Sep 14, 2026
teknium1 added a commit that referenced this pull request Sep 14, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Landed via #92586 — thanks @aerbaser. Merged as 5280dec0ee with your commits on main under your authorship; we added one guard so the repr pass leaves values an upstream pass already masked alone (kept Digest *** intact).

QuixThe2nd pushed a commit to QuixThe2nd/hermes-ide that referenced this pull request Sep 15, 2026
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:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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