fix(security): mask applied secret values in tool-result egress content - #77179
Closed
andrexibiza wants to merge 2 commits into
Closed
fix(security): mask applied secret values in tool-result egress content#77179andrexibiza wants to merge 2 commits into
andrexibiza wants to merge 2 commits into
Conversation
Contributor
Author
|
worktree newb sorry |
Contributor
Author
|
Superseded by #77198 (same fix, QA'd + wire-path gate test). Closing to avoid duplicate-PR confusion. |
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
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.
What changed and why
Tool results are assembled in
make_tool_result_message(agent/tool_dispatch_helpers.py) and sent to the LLM provider API. Existing redaction at that boundary is shape-based only (vendor prefixes, URL credentials, auth headers). Opaque secret values — values applied from Bitwarden/1Password/command secret sources under any name (DATABASE_URL,FOO, arbitrary 1Password keys), or credential-suffixed env values likeMY_SERVICE_TOKEN=abc123randomstring— pass through unmasked and ship verbatim to the provider. That is the highest-severity disclosure channel in the product.This PR closes it with an exact-value pass:
mask_known_secret_values(text, extra_values=None):_known_secret_values()collects values of env vars whose name ends with a credential suffix (_API_KEY,_TOKEN,_SECRET,_KEY,_PASSWORD) and whose value is ≥ 6 chars (shorter values collide with ordinary prose and are never masked).extra_valuesthreads in values applied from external secret sources (e.g.DATABASE_URL=postgres://user:supersecret@db).***wherever it appears. Falsy input returns unchanged; the function never raises.make_tool_result_messagemasks the content after untrusted-content wrapping, before the message dict is built. Applied-secret values are resolved lazily at call time (get_hermes_home()→hermes_cli.env_loader.get_secret_source_values(home)). Handles plain string content and multimodal content lists: only{"type": "text"}parts are masked, non-text parts (e.g.image_url) are preserved. The whole pass is best-effort — any failure (import error, home resolution error) skips masking and never breaks message construction.Structural fields (
tool_call_id,name,role) are never touched.Why this matters to you as a user
If you use external secret sources (Bitwarden, 1Password, command-based secret resolution) or keep credentials in
.env/shell env under any name, tool output that echoes those values back — connection strings, config dumps,printenv-style output, error messages — could previously carry the raw secret value to the model provider. After this change, the exact values of every known/applied secret are masked (***) at the egress boundary, so the provider never receives them, regardless of what name the secret was stored under.How to test
make_tool_result_messageegress tests — plain content, multimodal text-masked/image-preserved, and message construction surviving a secret-source failure).Regression:
Hygiene:
git diff --checkclean;scripts/check-windows-footguns.pyclean on all changed files.Platforms tested
Windows 11, git-bash (MSYS), Python 3.12, via
scripts/run_tests.sh.Related
Closes #77162. Part of the secrets-exfiltration disclosure-class series: #77008 #77012 #77020 #77027 #77031 #77039.