fix(security): normalize input before dangerous command detection#3260
Merged
fix(security): normalize input before dangerous command detection#3260
Conversation
detect_dangerous_command() ran regex patterns against raw command strings without normalization, allowing bypass via Unicode fullwidth chars, ANSI escape codes, null bytes, and 8-bit C1 controls. Adds _normalize_command_for_detection() that: - Strips ANSI escapes using the full ECMA-48 strip_ansi() from tools/ansi_strip (CSI, OSC, DCS, 8-bit C1, nF sequences) - Removes null bytes - Normalizes Unicode via NFKC (fullwidth Latin → ASCII, etc.) Includes 12 regression tests covering fullwidth, ANSI, C1, null byte, and combined obfuscation bypasses. Salvaged from PR #3089 by thakoreh — improved ANSI stripping to use existing comprehensive strip_ansi() instead of a weaker hand-rolled regex, and added test coverage. Co-authored-by: Hiren <hiren.thakore58@gmail.com>
StreamOfRon
pushed a commit
to StreamOfRon/hermes-agent
that referenced
this pull request
Mar 29, 2026
…usResearch#3260) detect_dangerous_command() ran regex patterns against raw command strings without normalization, allowing bypass via Unicode fullwidth chars, ANSI escape codes, null bytes, and 8-bit C1 controls. Adds _normalize_command_for_detection() that: - Strips ANSI escapes using the full ECMA-48 strip_ansi() from tools/ansi_strip (CSI, OSC, DCS, 8-bit C1, nF sequences) - Removes null bytes - Normalizes Unicode via NFKC (fullwidth Latin → ASCII, etc.) Includes 12 regression tests covering fullwidth, ANSI, C1, null byte, and combined obfuscation bypasses. Salvaged from PR NousResearch#3089 by thakoreh — improved ANSI stripping to use existing comprehensive strip_ansi() instead of a weaker hand-rolled regex, and added test coverage. Co-authored-by: Hiren <hiren.thakore58@gmail.com>
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.
Summary
Salvaged from PR #3089 by @thakoreh — cherry-picked onto current main with authorship preserved, improved ANSI stripping, and added comprehensive tests.
Root cause
detect_dangerous_command()ran regex patterns againstcommand.lower()with no normalization. Obfuscation bypasses:Fix
Adds
_normalize_command_for_detection()that runs before pattern matching:strip_ansi()fromtools/ansi_strip(full ECMA-48 — CSI, OSC, DCS, 8-bit C1, nF sequences)Improvement over original PR
The original PR rolled its own ANSI regex (
\x1b\[[0-9;]*[a-zA-Z]) which missed private-mode CSI, 8-bit C1 controls, DCS/APC strings, and nF sequences. This version uses our existing comprehensivestrip_ansi()which handles all of those.Validation
Unit tests (12 new)
Tests cover: fullwidth rm/dd/chmod/mkfs, ANSI CSI/OSC/C1, null bytes, combined obfuscation, and safe command passthrough.
E2E: 21 obfuscation vectors through detect_dangerous_command
All caught, including DCS string prefix, 8-bit C1 CSI, and mixed fullwidth+ANSI combos.
Live PTY: Interactive CLI session
Sent fullwidth dangerous command as user input → model passed it to terminal tool → dangerous command approval dialog triggered with "delete in root path" — confirming the full pipeline works end-to-end.
Full suite
6237 passed, 1 pre-existing failure (unrelated
test_429_exhausts_all_retries)Closes #3089
Fixes #3076
Co-authored-by: Hiren hiren.thakore58@gmail.com