fix(security): redact credentials before persistence in session capture (salvage #24758 + #19855) - #31758
Merged
Conversation
Two-layer redaction at the persistence boundary so credentials never reach
state.db, session_*.json, or compression:
1. agent/chat_completion_helpers.py :: build_assistant_message
- Redact assistant content before the message dict is constructed
(catches PATs / API keys the model inlines into natural language)
- Redact tool_call.function.arguments at the same site (catches secrets
inlined into tool args, e.g. terminal command=curl -H 'Authorization: ...')
Tool execution uses the raw API response object, not this dict, so
redacting the persisted shape is safe.
2. run_agent.py :: _save_session_log
- Add _redact_message_content() static helper that handles both string
content and OpenAI/Anthropic multimodal list-of-parts (image parts
pass through untouched, only text/content fields are redacted)
- Apply to every message + the cached system prompt before writing
session_*.json
Both layers respect HERMES_REDACT_SECRETS via redact_sensitive_text —
no-op when disabled.
Tests (TestSaveSessionLogRedactsSecrets, 4 cases):
- api key in tool content
- api key in user message
- api key in system prompt
- multimodal list-of-parts (image part preserved, text redacted)
Tests use an autouse fixture to force _REDACT_ENABLED=True because the
hermetic conftest defaults the env var to false.
Salvaged from PR #24758 by @vgocoder (build_assistant_message + session_log)
+ PR #19855 by @liuhao1024 (multimodal list helper, system_prompt redaction).
Kept only the redaction concern from #19855; its unrelated whatsapp npm
timeout + PATCH_SCHEMA changes are out of scope and dropped.
Refs #19798 (PAT leak via assistant inline mention), #19845 (session capture
credential leak).
Co-authored-by: liuhao1024 <liuhao03@bilibili.com>
Co-authored-by: teknium1 <127238744+teknium1@users.noreply.github.com>
Contributor
🔎 Lint report:
|
alt-glitch
added
type/security
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Combined salvage of two overlapping PRs that both target credential leaks in persisted history.
Summary
Two-layer redaction at the persistence boundary so credentials never reach state.db, session_*.json, or compression input.
Threat
Models occasionally inline secrets into natural-language responses or tool arguments — a curl with
Authorization: Bearer sk-..., a github PAT mentioned in an explanation, an API key pasted into user input. Without redaction at the persistence boundary, these end up in:Changes
Layer 1 — earliest interception, before message enters history:
agent/chat_completion_helpers.py :: build_assistant_messagetool_call.function.arguments(tool execution uses the raw API response, not this dict, so the persisted shape is safe to redact)Layer 2 — defense-in-depth at JSON snapshot time:
run_agent.py :: _save_session_log_redact_message_content()static helper that handles both string content and OpenAI/Anthropic multimodal list-of-parts shape (image parts pass through untouched, only text/content fields go through the redactor)Both layers respect
HERMES_REDACT_SECRETS(viaredact_sensitive_text) — no-op when disabled.Test plan
4 new tests in
TestSaveSessionLogRedactsSecrets:Tests use an autouse fixture forcing
_REDACT_ENABLED=Truebecause the hermetic conftest defaults the env var to false.Salvage scope
build_assistant_messageredaction (content + tool args) + initial_save_session_logredaction. Two-site approach is the cleanest interception.PATCH_SCHEMArewrite (out of scope for a redaction PR).Co-authored-by: vgocoder hua.zhong@kingsmith.com
Co-authored-by: liuhao1024 liuhao03@bilibili.com
Closes #24758
Closes #19855