Skip to content

fix(security): redact reasoning display and session exports; cover Google OAuth client secrets - #82895

Open
vgarde wants to merge 2 commits into
NousResearch:mainfrom
vgarde:fix/redact-reasoning-exports-oauth
Open

fix(security): redact reasoning display and session exports; cover Google OAuth client secrets#82895
vgarde wants to merge 2 commits into
NousResearch:mainfrom
vgarde:fix/redact-reasoning-exports-oauth

Conversation

@vgarde

@vgarde vgarde commented Aug 10, 2026

Copy link
Copy Markdown

Summary

Closes the remaining short-term item from #20785: the May fix enabled secret redaction for chat output, tool output, and logs, but reasoning/thinking blocks were never covered, and Google OAuth client secrets (GOCSPX-… format) were missing from the redactor's pattern set entirely.

Root cause

  1. Reasoning blocks leak rawgateway/run.py and cli.py prepend last_reasoning (the model's scratch thinking) to user-facing output with no redact_sensitive_text pass. Since reasoning is display-only text that the model produces while echoing things it saw in context, any credential in context can surface verbatim in chat. Reproduction confirmed against v0.20.0 (2026.8.3).
  2. Google OAuth client secrets pass every pattern"client_secret": "GOCSPX-…" matched neither _JSON_FIELD_RE (key names list only matched the bare word secret, not client_secret) nor any prefix token. Verified by feeding the exact Google OAuth JSON shape through redact_sensitive_text — the value came back unredacted. These secrets sit in google_client_secret.json in every user's Hermes home, so any tool output or reasoning echoing one would leak it.
  3. hermes sessions export writes raw transcriptsrender_sessions_export in hermes_cli/session_export.py emitted session data with no redaction, so reasoning fields (stored raw by design for API replay) could leak into exported JSONL/Markdown.

Changes

  • agent/redact.py: add GOCSPX-[A-Za-z0-9_-]{10,} prefix pattern (Google OAuth client secret); add secret_key, client_secret, app_secret to _JSON_KEY_NAMES so "client_secret": "…" JSON fields redact.
  • gateway/run.py: redact display_reasoning before rendering the thinking block (all platforms).
  • cli.py: redact display_reasoning before rendering the CLI reasoning box.
  • hermes_cli/session_export.py: run redact_sensitive_text over rendered exports (JSONL and Markdown). Redaction replaces matches with a plain placeholder, so JSON/Markdown structure is preserved.

Verification

  • python -m py_compile passes on all four files.
  • Functional test through redact_sensitive_text: DeepSeek sk-…, Firecrawl fc-…, Google AIza…, Google OAuth JSON (client_secret), JWT/Bearer, and KEY=*** env assignments all mask; ordinary words (Secretary, tokenizer`) are untouched (no false positives).

Notes

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery area/sessions Session lifecycle, resume, persistence, history sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 10, 2026
@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

The new export redaction runs after JSONL/Markdown serialization (hermes_cli/session_export.py:66). For a transcript message containing {"client_secret": "opaque-client-secret-..."}, JSON escaping prevents the new client_secret matcher from seeing the key, so the opaque value is still exported on this PR head. The GOCSPX- prefix is masked, but non-prefix-shaped client secrets are not. Please redact structured message content before serialization (or use a JSON-aware traversal), and add a regression test that parses exported JSONL and checks an embedded client_secret value is absent.

The new reasoning/export call sites (cli.py:14419, gateway/run.py:17789, and hermes_cli/session_export.py:66) omit force=True. With the documented security.redact_secrets: false setting, both reasoning and export can bypass this protection. Either force-redact these egress boundaries or explicitly narrow the guarantee and cover the chosen behavior with tests.

Security evidence:

  • trust boundary: Model reasoning and persisted transcript/message content leave Hermes through CLI/gateway display and session export.
  • source/sink/invariant: Credential values must be sanitized before user-facing display/export, including structured content and regardless of the global redaction toggle.
  • current-main reproduction: Synthetic GOCSPX-... and embedded opaque client_secret values remained visible on current main.
  • PR-head or patch-replay validation: The PR/replay masked GOCSPX-..., but the opaque embedded client_secret remained in exported JSONL; JSONL stayed parseable.
  • positive/negative cases: The known prefix was masked and ordinary prose stayed unchanged; the escaped opaque client_secret survived, and disabling redaction left both synthetic values present.
  • residual bypass search: The changed CLI, gateway, and session-export call sites use non-forced redaction, while serialized structured content bypasses the raw JSON-key matcher.
  • reviewer validation: Focused redaction/export tests covered 99 passing cases; one environment-dependent dotenv import case was omitted in both baseline and replay.

Not checked:

  • Full test suite
  • CodeRabbit review

Signed: GPT-5.6-sol-xhigh in Codex

@vgarde vgarde closed this Aug 18, 2026
@vgarde vgarde reopened this Aug 18, 2026
@vgarde
vgarde force-pushed the fix/redact-reasoning-exports-oauth branch from d4b934b to 93e57d8 Compare August 18, 2026 16:07
@vgarde

vgarde commented Aug 18, 2026

Copy link
Copy Markdown
Author

Addressed all review points — pushed as 93e57d875.

1. Structured redaction before serialization. Added redact_structured() in agent/redact.py: a deep-walker that masks values under credential-named keys (client_secret, apiKey, token, password, …) by key context alone, so opaque values with no recognizable prefix are masked even though the serialized key is JSON-escaped (\"client_secret\"). render_sessions_export now redacts the session structures before _render_jsonl/_render_markdown; the text pass over the serialized output remains as a belt-and-suspenders layer.

2. force=True at all three egress call sites (cli.py reasoning display, gateway/run.py reasoning display, session_export.py render + structured pass). Reasoning and exports now redact regardless of security.redact_secrets: false.

3. Regression tests.

  • tests/hermes_cli/test_session_export.py: JSONL with an embedded opaque client_secret (both dict-shaped and JSON-string payloads) is absent from the output; each line still parses as JSON; redaction holds with _REDACT_ENABLED=False.
  • tests/agent/test_redact.py (TestStructuredRedaction): key-context masking, nested structures, JSON-string payloads, force behavior with the global toggle off, and no false positives on non-secret structure.

Verification: 102/102 test_redact.py + 6/6 test_session_export.py pass; manual replay of the review's repro ({"client_secret": "opaque-client-secret-abc123XYZ"} in a transcript) exports with zero leaks and parseable JSONL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants