Skip to content

fix(redact): strip controls from masked secrets - #55321

Closed
ooiuuii wants to merge 1 commit into
NousResearch:mainfrom
ooiuuii:fix/mask-secret-strip-controls
Closed

fix(redact): strip controls from masked secrets#55321
ooiuuii wants to merge 1 commit into
NousResearch:mainfrom
ooiuuii:fix/mask-secret-strip-controls

Conversation

@ooiuuii

@ooiuuii ooiuuii commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • strip ASCII/C1 control characters before rendering the visible head/tail of mask_secret() output
  • keep printable secret masks unchanged
  • return the configured empty fallback when a value only contains controls

Closes #55319

Validation

  • python -m pytest tests\agent\test_redact.py -q -k MaskSecret --basetemp .pytest-tmp-mask-secret-controls -> 3 passed
  • python -m ruff check agent\redact.py tests\agent\test_redact.py -> passed
  • python -m pytest tests\agent\test_redact.py -q --basetemp .pytest-tmp-redact-full -> 125 passed
  • git diff --check -> clean

Agent transcript (redacted)

  • Compared the OpenClaw control-character masking fix with Hermes' agent/redact.py implementation.
  • Confirmed Hermes reproduced the same visible-control issue in masked head/tail output before the patch.
  • Added focused regression coverage for unchanged printable masks, stripped newline/C1 controls, and all-control input fallback.

@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 P3 Low — cosmetic, nice to have labels Jun 30, 2026
tonydwb
tonydwb reviewed Jun 30, 2026
View reviewed changes

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Strips control characters from masked secrets. Clean with tests.

@alt-glitch alt-glitch mentioned this pull request Jul 4, 2026
Closed
6 tasks
@ooiuuii
ooiuuii force-pushed the fix/mask-secret-strip-controls branch from 7b9d219 to bc74172 Compare July 6, 2026 02:01
@teknium1

teknium1 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. Current main still returns raw value[:head] and value[-tail:] in agent/redact.py:351-353, so the reported newline/C1 leakage remains reproducible from the implementation. The proposed normalization runs before those slices and before the floor check, including the configured empty fallback for all-control input.

The canonical helper covers the relevant sibling paths: log-token masking via _mask_token() (agent/redact.py:356-361) and display masking in config, dump, and status (hermes_cli/config.py:7795-7796, hermes_cli/dump.py:122-123, hermes_cli/status.py:40-41). The added tests are behavior-focused rather than snapshot-based. GitHub reports the branch cleanly mergeable; the cross-referenced duplicate #58079 was closed in favor of this earlier PR.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@GottZ GottZ mentioned this pull request Jul 30, 2026
Closed
@alt-glitch alt-glitch mentioned this pull request Aug 3, 2026
Closed
thatssoheil added a commit to thatssoheil/hermes-agent that referenced this pull request Aug 3, 2026
@thatssoheil
fix(redact): strip control chars from mask_secret display (NousResear…
64ee645 
…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 kshitijk4poor mentioned this pull request Aug 7, 2026
Merged
kshitijk4poor pushed a commit to kshitijk4poor/hermes-agent that referenced this pull request Aug 7, 2026
@thatssoheil @kshitijk4poor
fix(redact): strip control chars from mask_secret display (NousResear…
9942519 
…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
@thatssoheil @kshitijk4poor
fix(redact): strip control chars from mask_secret display (#55319, #5…
e9d1551 
…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.
@kshitijk4poor kshitijk4poor mentioned this pull request Aug 7, 2026
Closed
ma1138569845 pushed a commit to ma1138569845/dechnicAuditor-agent that referenced this pull request Aug 10, 2026
@thatssoheil @ma1138569845
fix(redact): strip control chars from mask_secret display (NousResear…
79b5b9b 
…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
@thatssoheil @kshitijk4poor
fix(redact): strip control chars from mask_secret display (NousResear…
1e6f0fe 
…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
@thatssoheil xuqinglei
fix(redact): strip control chars from mask_secret display (NousResear…
6c03a76 
…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

Reviewers

1 more reviewer
@tonydwb tonydwb tonydwb left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Masked secret display can preserve control characters

4 participants

@ooiuuii @teknium1 @tonydwb @alt-glitch