feat(security): mask opaque credential values in log output - #77020
feat(security): mask opaque credential values in log output#77020andrexibiza wants to merge 2 commits into
Conversation
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 adds an exact-value pass to RedactingFormatter.format() that masks the literal values of credential-named env vars (self-contained: no dependency on other secret-redaction changes). 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. Tests: 76 redact tests pass (3 new: opaque _TOKEN value, _PASSWORD value, and BWS_ACCESS_TOKEN value masked in formatted log records) via scripts/run_tests.sh.
alt-glitch
added
type/security
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting a real logging disclosure gap: current RedactingFormatter only delegates to redact_sensitive_text (agent/redact.py:1006-1008), and a bare opaque literal is not covered by the existing assignment/prefix matchers.
Problems
agent/redact.py:1047in this PR unconditionally applies_mask_known_env_values(...). That bypasses the existing opt-out atagent/redact.py:710-711: whensecurity.redact_secrets: falseis configured,redact_sensitive_textintentionally returns the original text. The documented contract says this setting controls logs as well (website/docs/user-guide/configuration.md:2091;website/docs/reference/environment-variables.md:818).
Suggested changes
- Run the exact-value formatter pass only while
_REDACT_ENABLEDis true, and add a formatter regression test for the disabled setting. The enabled opaque-token, password, and BWS cases are otherwise well scoped.
Automated hermes-sweeper review.
| # Shape-based regex first (vendor prefixes, headers, URLs), then the | ||
| # exact-value pass so opaque credential values with no recognizable | ||
| # prefix (e.g. MY_SERVICE_TOKEN=abc123randomstring, BWS_ACCESS_TOKEN) | ||
| # are masked from log output too. |
There was a problem hiding this comment.
Please preserve the global redaction opt-out here. redact_sensitive_text() returns raw text when _REDACT_ENABLED is false (agent/redact.py:710-711), and the documented security.redact_secrets setting controls logs too. Gate this exact-value pass on _REDACT_ENABLED and add a disabled-formatter regression test.
There was a problem hiding this comment.
Addressed in bddf34a091 — the exact-value formatter pass is gated on _REDACT_ENABLED, honoring security.redact_secrets: false (agent/redact.py:710), so the opt-out is respected in the exact-value pass, not only the shape-based one. Verified at head bddf34a091: 77/77 redact tests pass.
…pass The opaque credential-value pass in RedactingFormatter.format ran unconditionally, masking *_TOKEN/*_PASSWORD env values even when security.redact_secrets: false (HERMES_REDACT_SECRETS=false) was set. redact_sensitive_text honors that opt-out and returns the original text, but the exact-value pass masked around it, violating the documented contract that the setting controls logs too. Gate the exact-value pass on _REDACT_ENABLED and add a formatter regression test asserting the disabled setting passes an opaque credential value through unmasked.
|
Fixed in The exact-value formatter pass now honors the global redaction opt-out:
Validation: |
teknium1
added
sweeper:risk-compatibility
Address review feedback on the end-to-end gate: 1. The capture handler is now installed BEFORE load_hermes_dotenv() runs, so it observes records emitted during external-secret loading instead of only records produced after the load completed. The mocked fetch path emits a warning record per leaked value (the realistic backend-echo case), so the formatted-output assertions prove the formatter actually masked records that carried the values. 2. Every value the test claims to cover is now passed through the formatter: the mocked load path logs ALL _LEAK_VALUES (both prefix-shaped), and the assertion iterates all of them with a non-vacuous guard (capture must contain at least one record per value). The second value's assertion is no longer vacuous. Both values are prefix-shaped so main's shape-based regex masks them; opaque values with no vendor prefix remain pinned by NousResearch#77020's own regression tests (documented in the test docstring). Tests: gate passes standalone; 92 total across the gate + env_loader + redact suites via scripts/run_tests.sh.
…cription, skill test Addresses teknium1's review on NousResearch#77097: 1. 'Not true on main' — the security contract is now explicitly scoped as implemented by the secrets-exfiltration hardening series (NousResearch#77008/NousResearch#77012/NousResearch#77020/NousResearch#77027/NousResearch#77031/NousResearch#77039). The docs state current main behavior plainly (plaintext bws_cache.json read/written when encryption disabled, default false) and keep the rotation instruction mandatory today, since that exposure already exists on main. The posture framing stays — this eliminates an entire vulnerability class — but the claim is now sequenced truthfully. 2. Skill description shortened to 53 chars, one sentence, ends with a period (AGENTS.md hardline). 3. tests/skills/test_bitwarden_secrets_skill.py added: validates frontmatter, description length, required sections, user-only rotation + clipboard discipline, honest series scoping (no claim the gate test is on main), and docs-page metadata consistency. 4. Clipboard discipline added to rotation instructions (docs + skill): create token, copy to clipboard, paste into terminal, save nowhere in between.
GottZ
left a comment
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address the opaque-credential disclosure: #77014 combines formatter-level exact-value masking with external-secret-source stderr masking, while #77020 isolates the formatter fix and preserves the documented global redaction opt-out.
Related pull requests
- #77014 [closed]
related— (+201/-5) — superseded overlap: The closed PR contains substantially the same formatter-level exact-value masking as #77020, plus separate masking for external-secret-source errors, hints, warnings, and conflicts; it remains relevant as the broader predecessor whose formatter portion was superseded by #77020. - #77020
related— (+123/-1) — keep open with a salvage path: The diff directly closes the reported formatter gap by masking values from credential-named environment variables and adds coverage for opaque tokens, passwords, and BWS tokens. It also addresses the contributor's keep_open review by gating the new pass on_REDACT_ENABLEDand adding a disabled-setting regression test, preserving the documented opt-out.
Duplicates
#77014 and #77020 substantially duplicate the formatter-level exact-value masking; #77014 additionally covers external-secret-source stderr output.
Suggested consolidation
Keep #77020 open with a salvage path: retain its self-contained formatter change, _REDACT_ENABLED gate, and enabled/disabled regression coverage. Treat the already-closed #77014 as superseded by #77020 for the overlapping formatter work; its separate external-secret-source stderr masking should remain associated with the stated #77012 follow-up rather than being folded into #77020.
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 17 kB of PR diffs, 5 kB of issue/PR text, 3 kB of discussion (4 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
|
Review receipt — comment from 2026-08-02 on agent/redact.py (RedactingFormatter exact-value pass): Addressed in bddf34a — the exact-value pass is now gated on _REDACT_ENABLED, matching redact_sensitive_text's own opt-out: with security.redact_secrets: false (or HERMES_REDACT_SECRETS=false) the documented contract that logs are NOT redacted is preserved, and no masking runs around the returned original text. Regression test added per your ask: test_disabled_setting_passes_opaque_value_through (asserts an opaque credential value passes through the formatter unmasked when the setting is disabled). 77/77 test_redact.py tests pass. |
|
suggesting changes The patch fixes the reported current-main leak for contiguous opaque environment credentials in handlers using RedactingFormatter and honors the documented opt-out. However, two active stderr sinks still use plain formatters and can emit the same credentials verbatim.
Security evidence:
Not checked:
Signed: GPT-5.6-luna-max in Codex |
5 participants
Related #77008 #77012 #77014
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 adds an exact-value pass to
RedactingFormatter.format()that masks the literal values of credential-named env vars (*_API_KEY,*_TOKEN,*_SECRET,*_KEY,*_PASSWORD). The change is self-contained — it does not depend on any other PR.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.Related
main.main.Part of #77162
Part of #77165