Skip to content

fix(debug): redact log content at upload time in hermes debug share#19318

Merged
teknium1 merged 1 commit into
NousResearch:mainfrom
GodsBoy:fix/debug-share-redact-upload
May 3, 2026
Merged

fix(debug): redact log content at upload time in hermes debug share#19318
teknium1 merged 1 commit into
NousResearch:mainfrom
GodsBoy:fix/debug-share-redact-upload

Conversation

@GodsBoy
Copy link
Copy Markdown
Contributor

@GodsBoy GodsBoy commented May 3, 2026

What does this PR do?

Closes a public-leak path: hermes debug share uploads logs to paste.rs / dpaste.com per the bug-report templates' explicit instructions, but never applies redact_sensitive_text before upload. With security.redact_secrets off by default after #16794, those uploads have been carrying credentials onto a public paste service.

This PR applies agent.redact.redact_sensitive_text(text, force=True) to log content at the _capture_log_snapshot boundary, before it reaches upload_to_pastebin. On-disk logs are not modified. A visible banner is prepended to each upload-bound log paste so reviewers know redaction was applied. A --no-redact flag preserves deliberate unredacted sharing for maintainer-coordinated cases.

force=True is non-negotiable: without it, redact_sensitive_text short-circuits at agent/redact.py:322 when HERMES_REDACT_SECRETS is unset, so the fix would silently be a no-op for its target audience. The regression test TestCaptureLogSnapshotRedaction::test_force_true_overrides_unset_env_var pins this down so a future refactor cannot accidentally drop it.

This is upload-time-only, not local-disk redaction. It does not change security.redact_secrets defaults; it closes the public-leak path that is structurally upstream of the local-redaction question.

Related Issue

Fixes #19316

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/debug.py (+89, -7): added _redact_log_text helper that calls redact_sensitive_text(text, force=True). Threaded a redact: bool = True parameter through _capture_log_snapshot and _capture_default_log_snapshots so tail_text and full_text are sanitized before being returned. run_debug_share now reads args.no_redact, passes the inverted flag through, and prepends a visible _REDACTION_BANNER to the report and each upload-bound log paste when redaction is enabled. Help-fallback text in run_debug updated. INFO log line records that redaction was applied.
  • hermes_cli/main.py (+11): added --no-redact argument to the debug share subparser and updated the epilog example.
  • tests/hermes_cli/test_debug.py (+213): two new test classes covering the capture-time redaction path (TestCaptureLogSnapshotRedaction, 5 tests) and the CLI-level wiring (TestRunDebugShareRedaction, 3 tests). The regression test test_force_true_overrides_unset_env_var pins the force=True requirement.

How to Test

  1. Ensure security.redact_secrets is unset (or false) and HERMES_REDACT_SECRETS is not exported.
  2. Write a fixture log to ~/.hermes/logs/agent.log containing a vendor-prefixed token (e.g., the literal string sk-proj-A1B2C3D4E5F6G7H8I9J0aA).
  3. Run hermes debug share --local. Observe that the printed report and full-log block do NOT contain the literal token, and a redaction banner appears at the top of each log section.
  4. Run hermes debug share --no-redact --local against the same fixture. Observe that the original token is preserved and no banner appears.
  5. Run the focused test suite: pytest tests/hermes_cli/test_debug.py -v. All 66 tests pass (8 new + 58 existing).

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/hermes_cli/test_debug.py -q and all tests pass (66/66; full suite not run locally because the development venv was not provisioned in this environment, but the change is bounded to debug.py and main.py and the focused tests cover the new code paths)
  • I've added tests for my changes (8 new tests across 2 classes)
  • I've tested on my platform: Ubuntu 24.04, Python 3.12.3

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) : docstrings on _redact_log_text, _capture_log_snapshot, _capture_default_log_snapshots, and the file-level docstring in hermes_cli/debug.py were updated. Help-fallback text in run_debug and the argparse epilog example in hermes_cli/main.py reference --no-redact.
  • I've updated cli-config.yaml.example if I added/changed config keys : N/A (no config keys added)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows : N/A (no architecture or workflow change)
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide : change uses only stdlib (logging) plus existing Hermes patterns (pathlib, the canonical redact_sensitive_text and _capture_log_snapshot paths). No termios, fcntl, encoding-specific, or platform-conditional code added.
  • I've updated tool descriptions/schemas if I changed tool behavior : N/A (no tool behavior changed)

Screenshots / Logs

Empirical confirmation that force=True is required (run from the repo root):

$ python3 -c "
import sys
sys.path.insert(0, '.')
from agent.redact import redact_sensitive_text
sample = 'INFO config: api_key=sk-proj-A1B2C3D4E5F6G7H8I9J0aA loaded'
print('default:', repr(redact_sensitive_text(sample)))
print('force:  ', repr(redact_sensitive_text(sample, force=True)))
"
default: 'INFO config: api_key=sk-proj-A1B2C3D4E5F6G7H8I9J0aA loaded'
force:   'INFO config: api_key=sk-pro...J0aA loaded'

Without force=True, redact_sensitive_text returns input unchanged (agent/redact.py:322). The regression test in this PR pins this down.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P1 High — major feature broken, no workaround comp/cli CLI entry point, hermes_cli/, setup wizard labels May 3, 2026
@alt-glitch
Copy link
Copy Markdown
Collaborator

Fixes #19316 — applying at upload boundary in .

Apply agent.redact.redact_sensitive_text with force=True to log content
captured by _capture_log_snapshot before it reaches upload_to_pastebin.
On-disk logs are untouched. Compatible with the off-by-default local
redaction policy from NousResearch#16794: this is upload-time-only and applies
regardless of security.redact_secrets because the public paste service
is the leak surface. A visible banner is prepended to each uploaded log
paste so reviewers know redaction was applied. --no-redact preserves
deliberate unredacted sharing for maintainer-coordinated cases.

The bug-report, setup-help, and feature-request issue templates direct
users to run hermes debug share and paste the resulting public URLs.
With redaction off by default per NousResearch#16794, those uploads have been
carrying credentials onto paste.rs and dpaste.com.

force=True is non-negotiable: without it, redact_sensitive_text
short-circuits at agent/redact.py:322 when the env var is unset, so the
fix would silently be a no-op for its target audience. A regression
test pins this down.

Fixes NousResearch#19316
@GodsBoy GodsBoy force-pushed the fix/debug-share-redact-upload branch from 9d50635 to a983a5e Compare May 3, 2026 18:14
@teknium1 teknium1 merged commit b8ae8cc into NousResearch:main May 3, 2026
@Bartok9
Copy link
Copy Markdown
Contributor

Bartok9 commented May 3, 2026

Nice catch @GodsBoy — this is a real leak path that's easy to miss. The issue is well-described and the fix is targeted at the right location.

A few thoughts on the diff that might help it land faster:

  1. The pre-upload redaction is exactly right — applying redact_sensitive_text before the paste.rs/dpaste POST is the correct fix location rather than trying to intercept at the collection stage.

  2. Worth double-checking that the redaction also covers the case where security.redact_secrets is explicitly set to True — the default-off behavior after feat(security): make secret redaction off by default #16794 is what makes this exploitable, so verifying the fix works in both states would strengthen the PR.

  3. If there's a quick smoke test that exercises the upload path with a redacted API key in the session log, that would likely get this merged faster.

The type/security + P1 labels look right. Flagging for anyone with maintainer access — this is worth expediting past the first-time contributor gate.

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

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P1 High — major feature broken, no workaround type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] hermes debug share uploads logs with credentials to public paste service

4 participants