Skip to content

fix(redact): strip controls before masking secrets - #58079

Closed
tianma-if wants to merge 1 commit into
NousResearch:mainfrom
tianma-if:codex/fix-mask-secret-control-chars
Closed

fix(redact): strip controls before masking secrets#58079
tianma-if wants to merge 1 commit into
NousResearch:mainfrom
tianma-if:codex/fix-mask-secret-control-chars

Conversation

@tianma-if

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes display-time secret masking so agent.redact.mask_secret() strips C0/C1/DEL control characters before applying the existing visible head/tail mask.

Before this change, malformed pasted credentials could produce masked output containing newlines or invisible control bytes when those bytes landed in the preserved prefix/suffix. Printable secrets keep the same mask shape.

Fixes #55319

Type of Change

  • Bug fix
  • Tests

Changes

  • Adds a tiny _is_display_control_char() helper covering C0, DEL, and C1 controls.
  • Normalizes mask_secret() input before the existing floor/head/tail masking logic.
  • Adds focused regression coverage for prefix newline controls and suffix C1/DEL controls.
  • Adds a printable-value guard to prove normal mask output remains unchanged.

Duplicate check

Checked open PRs for #55319, masked secret display, mask_secret control characters, redaction control characters, C1, and DEL masking. The only nearby open PR is #58074, which targets save-time credential normalization in save_env_value(); this PR targets display-time masking in agent.redact.mask_secret().

How to Test

  • .venv/bin/python -m pytest tests/agent/test_redact.py -q -k TestMaskSecretDisplay
  • .venv/bin/python -m pytest tests/agent/test_redact.py -q
  • .venv/bin/python -m ruff check agent/redact.py tests/agent/test_redact.py
  • git diff --check

Platform: macOS, Python 3.13, local .venv.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/auth Authentication, OAuth, credential pools P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jul 4, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #55321 — both strip C0/C1/DEL control characters from mask_secret() output in agent/redact.py (+ regression tests) to fix #55319. #55321 is the earlier open fix PR for the same code site and mechanism; this is the canonical/duplicate relationship (maintainer to pick one). A printable-value guard here doesn't change the underlying same-mechanism duplication.

@tianma-if

Copy link
Copy Markdown
Contributor Author

Closing as duplicate of #55321 per triage; the earlier PR covers the same mask_secret control-character fix. Thanks.

@tianma-if tianma-if closed this Jul 4, 2026
thatssoheil added a commit to thatssoheil/hermes-agent that referenced this pull request Aug 3, 2026
…ch#55319, NousResearch#55321)

A masked secret's visible head/tail could carry control bytes (newline,
NUL, DEL, C1 0x80-0x9F, zero-width) into config/status/dump output.
Strip every control incl. \n/\t (display differs from redact_sensitive_text,
which preserves \n/\t as line structure) before slicing; all-control values
return the configured empty fallback.

Consolidates the previously-closed NousResearch#58079 approach (strip controls before
masking) - supersedes it.
kshitijk4poor pushed a commit to kshitijk4poor/hermes-agent that referenced this pull request Aug 7, 2026
…ch#55319, NousResearch#55321)

A masked secret's visible head/tail could carry control bytes (newline,
NUL, DEL, C1 0x80-0x9F, zero-width) into config/status/dump output.
Strip every control incl. \n/\t (display differs from redact_sensitive_text,
which preserves \n/\t as line structure) before slicing; all-control values
return the configured empty fallback.

Consolidates the previously-closed NousResearch#58079 approach (strip controls before
masking) - supersedes it.
kshitijk4poor pushed a commit that referenced this pull request Aug 7, 2026
…5321)

A masked secret's visible head/tail could carry control bytes (newline,
NUL, DEL, C1 0x80-0x9F, zero-width) into config/status/dump output.
Strip every control incl. \n/\t (display differs from redact_sensitive_text,
which preserves \n/\t as line structure) before slicing; all-control values
return the configured empty fallback.

Consolidates the previously-closed #58079 approach (strip controls before
masking) - supersedes it.
ma1138569845 pushed a commit to ma1138569845/dechnicAuditor-agent that referenced this pull request Aug 10, 2026
…ch#55319, NousResearch#55321)

A masked secret's visible head/tail could carry control bytes (newline,
NUL, DEL, C1 0x80-0x9F, zero-width) into config/status/dump output.
Strip every control incl. \n/\t (display differs from redact_sensitive_text,
which preserves \n/\t as line structure) before slicing; all-control values
return the configured empty fallback.

Consolidates the previously-closed NousResearch#58079 approach (strip controls before
masking) - supersedes it.
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…ch#55319, NousResearch#55321)

A masked secret's visible head/tail could carry control bytes (newline,
NUL, DEL, C1 0x80-0x9F, zero-width) into config/status/dump output.
Strip every control incl. \n/\t (display differs from redact_sensitive_text,
which preserves \n/\t as line structure) before slicing; all-control values
return the configured empty fallback.

Consolidates the previously-closed NousResearch#58079 approach (strip controls before
masking) - supersedes it.
sanshi2018 pushed a commit to sanshi2018/hermes-agent that referenced this pull request Aug 18, 2026
…ch#55319, NousResearch#55321)

A masked secret's visible head/tail could carry control bytes (newline,
NUL, DEL, C1 0x80-0x9F, zero-width) into config/status/dump output.
Strip every control incl. \n/\t (display differs from redact_sensitive_text,
which preserves \n/\t as line structure) before slicing; all-control values
return the configured empty fallback.

Consolidates the previously-closed NousResearch#58079 approach (strip controls before
masking) - supersedes it.

(cherry picked from commit e9d1551)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Masked secret display can preserve control characters

2 participants