feat(security): mask opaque credential values in log output - #77014
Closed
andrexibiza wants to merge 2 commits into
Closed
feat(security): mask opaque credential values in log output#77014andrexibiza wants to merge 2 commits into
andrexibiza wants to merge 2 commits into
Conversation
The startup status block in env_loader prints error, remediation-hint, warning, and conflict lines straight to stderr. The merged secret-name suppression (NousResearch#60295/NousResearch#69054) removed the applied-name list, but a source error or warning that echoes a secret VALUE would still print it. This change routes all four status-line types through a value masker: - agent.redact.mask_known_secret_values() masks the exact values of credential-named env vars (opaque tokens with no vendor prefix) - env_loader._mask_secret_text() additionally masks every value applied from external secret sources this process (Bitwarden/1Password), which is the authoritative set for what the status line is about Why this matters to users: secret names already stopped being printed; now secret values cannot leak through error or warning text either. If a backend echoes a key back in an error message, you see the diagnostic without the key. Tests: 67 passed across env_loader + secret-sources suites (2 new regression tests: value in warning masked, value in error masked) plus the existing 76-test redact suite.
RedactingFormatter already ran the shape-based regex redactor (vendor prefixes, auth headers, URLs) on every log record. Opaque credential values with no recognizable prefix — MY_SERVICE_TOKEN=abc123, BWS_ACCESS_TOKEN, *_PASSWORD vars — passed through log output unmasked. This wires the exact-value pass (mask_known_secret_values, added in the sibling status-line PR) into RedactingFormatter.format(), so every log record also masks the literal values of credential-named env vars. Why this matters to users: a log line that happens to include a secret's value — an error echoing a token, a debug line printing a connection string — no longer writes that value to your log files. The diagnostic text survives; the secret doesn't. Stacked on fix/security-secret-source-status-formatter (NousResearch#77012) which adds mask_known_secret_values to agent/redact.py. This PR contains only the formatter wiring + tests on top of that base. Tests: 76 redact tests pass (3 new: opaque value, *_PASSWORD value, and BWS_ACCESS_TOKEN value masked in log records); 99 across the redact + env_loader suites via scripts/run_tests.sh.
Contributor
Author
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.
What changed and why
RedactingFormatter(used by every Hermes log handler) already ran the shape-based regex redactor — vendor prefixes (sk-,ghp_), auth headers, URLs — on every log record. But opaque credential values with no recognizable prefix passed through log output unmasked:MY_SERVICE_TOKEN=abc123randomstring,BWS_ACCESS_TOKEN,*_PASSWORDvalues.This PR wires the exact-value pass (
mask_known_secret_values) intoRedactingFormatter.format(), so every log record also masks the literal values of credential-named env vars.Why this matters to you as a user
A log line that happens to include a secret's value — an error message echoing a token, a debug line printing a connection string — no longer writes that value into your log files. The diagnostic text survives; the secret doesn't. This closes the value-in-logs half of the disclosure class for every Hermes user, not just BWS users.
Reproduction steps (current behavior on
main)export MY_SERVICE_TOKEN=opaque-secret-value-xyzlogger.warning("starting with opaque-secret-value-xyz")).Current: opaque credential values written to logs.
Expected: value replaced with
***; surrounding diagnostic preserved.How to test
scripts/run_tests.sh tests/agent/test_redact.py→ 76 passed (3 new: opaque_TOKENvalue,_PASSWORDvalue, andBWS_ACCESS_TOKENvalue masked in formatted log records).Platforms tested
scripts/run_tests.sh.git diff --checkclean;check-windows-footguns.pyclean on changed files.Stacking note (important)
This PR is stacked on #77012 (
fix(security): mask secret values in secret-source status lines), which introducesmask_known_secret_valuesinagent/redact.py. Because the fork-based base branch isn't addressable upstream, this PR currently targetsmainand therefore includes #77012's commit in its diff. Please review/merge #77012 first, then this one — after #77012 lands on main, this PR can be rebased and its diff shrinks to exactly the formatter wiring + tests. The formatter change itself does not depend on anything else.Related