A raw newline in the _CMDPOS start-position class made ANY multi-line
quoted argument look like a command boundary, so hermes send message
bodies, multi-line git commit -m messages, and heredoc text that merely
mentioned dangerous command names tripped the unconditional hardline
blocklist and could not run at all.
Mask newlines inside single/double quotes (detection-only, mirroring the
quote tracking in _iter_shell_command_starts) before building detection
variants. Real threats keep blocking: unquoted newlines stay command
separators, command substitutions inside quotes still anchor, and
_mark_command_starts still re-inserts newlines at genuine quote-aware
command starts. Masking runs on the RAW command before normalization,
which strips escapes and would otherwise corrupt quote state.
Regression tests cover both directions: multi-line quoted data passes
(hermes send, git commit -m, heredocs); bare/chained/substituted
shutdown-class and rm-floor commands still block.
When an approval prompt expired without a response, every CLI-side path
collapsed the timeout into the same 'deny' choice as an explicit user
refusal, so the agent was told the user denied the action when the user
simply never answered. The gateway wait already distinguished the two
('timed out without user response... Silence is not consent.'); this
brings the CLI/TUI/ACP surfaces to parity.
- prompt_dangerous_approval(): input()-path expiry now returns a distinct
'timeout' choice (still fail-closed).
- cli.py _approval_callback + hermes_cli/callbacks.py approval_callback:
deadline expiry returns 'timeout' instead of 'deny'.
- check_all_command_guards / _run_approval_gate CLI tails: 'timeout' maps
to outcome='timeout' with a 'timed out without user response... Silence
is not consent.' BLOCKED message (matching the gateway wording);
explicit deny keeps outcome='denied' and gains user_consent=False for
shape parity.
- computer_use: 'timeout' verdict threads through the CLI adapter and
yields a 'prompt timed out — the user did not respond' error instead of
'denied by user'.
- ACP permissions bridge: FutureTimeout returns 'timeout' (other failures
still 'deny'); elicitation maps 'timeout' to 'cancel' like the gateway's
unresolved outcome; codex wire mapping documents deny/timeout→decline.
- write_approval already treats unknown choices as 'stage, not drop', so
a timeout now stages the memory write instead of silently refusing it.
Every timeout path remains fail-closed — the action never runs; only the
classification reported to the agent changes.
Follow-up to the recovery-recipe commit on this branch, per review:
instead of only TELLING the model to re-author the payload via
write_file (2 turns), materialize the blocked command to
~/.hermes/cache/blocked-scripts/blocked-*.sh and point the recovery
at it directly: 'saved to <path> - review it, then run
terminal(command="bash <path>")' (1 turn).
Safety posture is unchanged or better:
- Nothing is executed here; the file is only written.
- The bash <path> follow-up goes through the normal execution
pipeline, including the referenced-script content guard, which
inspects script files named in commands - the payload is MORE
visible to policy than it was inline.
- Genuine hardline blocks (destructive ops) never save anything
(test-asserted).
- Save failures fall back to the previous manual write_file recipe.
- 7-day opportunistic cleanup of saved payloads.
Two block classes from production mining (250k-call window) that
models answered with blind rephrase-retries:
1. Parser-limit / malformed-payload hardline blocks (198x): these fire
on oversized inline payloads (heredocs, giant one-liners), not on a
forbidden operation - but the message read like a permanent ban.
The block now appends: 'RECOVERY: ... write the script to a file
with write_file, then run bash /path/script.sh - do not retry
inline.' Genuine hardline blocks (destructive filesystem
operations) are unchanged.
2. Backgrounding-wrapper blocks (200x): the guidance now spells out
the exact corrected call shape - 're-send WITHOUT the wrapper as
terminal(command="<cmd>", background=true,
notify_on_complete=true)' - instead of describing the feature
abstractly.
Replace the process-global HERMES_CRON_SESSION env var with a per-session
ContextVar so a cron tick in the gateway process cannot leak into unrelated
live gateway/API/TUI turns. The cron scheduler now sets the ContextVar
inside the job's try/finally scope and resets it on cleanup. Gateway, API
server, ACP adapter, and TUI gateway all pass cron_session='' to explicitly
mark their sessions as non-cron, masking any stale process env.
Co-authored-by: hinablue <hinablue@gmail.com>
Closes#37968
Review follow-up on the #76194 salvage: the readonly swap makes this
function leak the live config-cache 'approvals' sub-dict to callers.
All current callers are read-only (audited); the docstring now carries
the do-not-mutate contract for future ones.
The terminal-command guard path loaded config 2-3x per invocation via
load_config(), which pays a defensive deepcopy of the entire config on
every call (~356us of the ~376us warm-cache cost measured on a real
config.yaml). All six swapped call sites were audited read-only — every
caller takes scalar reads or iterates the returned structures; none
mutate (the save path at save_permanent_allowlist keeps load_config) —
so they now use load_config_readonly(), the API built for exactly this
(precedent: #74211, #74322; the one unsafe-site lesson from #56085's
salvage is covered by the mutation audit and a cache-integrity test).
Measured (real config.yaml, warm cache): load_config 376.0us ->
load_config_readonly 19.9us (18.9x); full guard pass
check_all_command_guards('ls -la','local') 930.7us -> 241.8us (3.85x).
Tests: new test_approval_config_readonly.py drives the real functions
against a temp HERMES_HOME — readonly call counts per function, a
no-deepcopy pin for the full guard pass, and cache-identity/integrity
checks. Existing test mocks retargeted from load_config to
load_config_readonly (same injection intent). Note: 6
test_approval_mode_parity failures are pre-existing ordering flakes —
identical with the change stashed on clean main.
Port from openai/codex#33464: GNU rm permutes options, so
`rm build/ -rf`, `rm build/ -r -f`, and `rm build/ --recursive
--force` are equivalent to the flags-first spellings — but every
existing rm pattern required the flag group BEFORE the path, so these
spellings ran with no approval prompt at all (proven live on main).
The hardline floor was NOT affected: protected paths (/, system dirs,
$HOME) match regardless of flag position because the hardline path
matcher does not require flags. The gap was the approval-prompt layer
for arbitrary paths.
New DANGEROUS_PATTERNS entry with a tempered operand run: cannot cross
command separators (; | & newline), quotes, or a bare -- end-of-options
separator (after --, -rf is a literal filename). Flag token must be
whitespace-anchored so the r inside long options like --registry does
not count.
9 positive + 7 negative shapes in tests; approval cluster (868 tests)
green.
Inspired by Claude Code 2.1.214, which added permission prompts for
container-CLI commands (including the Podman shim) carrying
daemon-redirect flags (--url, --connection, --identity, remote mode)
that previously ran without one.
A daemon redirect makes a local-looking command operate on a different
(often remote) daemon, silently acting on production infrastructure.
Any container-CLI invocation carrying a redirect now requires approval
regardless of subcommand:
- -H/--host and --context global flags (value required, global-flag
position only — bare -h help and run-level -h <hostname> stay allowed)
- context use (persistently switches the default daemon)
- podman --url/--connection/--identity and -r/--remote
- DOCKER_HOST=/DOCKER_CONTEXT=/CONTAINER_HOST=/CONTAINER_CONNECTION=
environment prefixes
Sibling-site widening: the existing container lifecycle rules matched
only the verb directly adjacent to the binary name, so a global flag or
a compose -f file flag slipped past the guard, and the legacy hyphenated
compose binary was never covered. They now tolerate global flags — the
same treatment the 'hermes ... gateway' rule already has — and match the
hyphenated compose binary.
Validation: 33 new tests; 339 pass in test_approval.py + new file;
442 pass across the adjacent guard suites; E2E battery of 12 dangerous
+ 17 safe commands via real imports, hot path ~330us/call.
After N consecutive guardian denials in a session the deny message escalates to a hard-stop instruction. Inspired by ChatGPT Work auto-review circuit breaker.
Adds an allow_session flag to the gateway approval payload so adapters
can render the session tier independently of the permanent tier. Matrix
gains a session reaction (🌀) and a reaction legend; pure-tirith prompts
now offer once/session/deny instead of collapsing to once/deny.
Salvaged from PR #67312, adapted to the allow_permanent semantics that
landed in #68597 (Always offered when any dangerous-pattern warning is
persistable; pure-tirith prompts stay session-max).
Three related messaging-approval fixes:
1. approvals.timeout default 60 -> 300. PR #63501 collapsed the gateway
wait onto the canonical approvals.timeout (previously
gateway_timeout=300), silently shrinking messaging approval windows
to 60s. Push-notification approvals routinely arrive later than a
minute; taps landed after the wait had already failed closed.
2. Stale-tap honesty: adapters resolved the approval AFTER rendering
'<checkmark> Approved by <user>' (Telegram/Discord/Slack), or ignored a zero
resolve count (WhatsApp Cloud/Feishu). A tap on an expired prompt
claimed approval while the command had already been denied. All
button paths now resolve first and render 'Approval expired -
command was not run' when nothing was waiting.
3. Mixed-warning prompts (dangerous pattern + tirith finding) now offer
Always: the persistence layer already permanently allowlists the
pattern key and downgrades the tirith key to session scope, but the
UI hid Always whenever ANY tirith warning was present. Pure-tirith
prompts still withhold Always (content findings are session-max by
design), and Smart-DENY overrides remain once-only.
_run_approval_gate's gateway branch only queued via submit_pending, so
plugin-escalated approvals never sent the interactive embed+buttons on
Discord/Telegram/Slack (#59413) - the user was never notified and the
action stayed silently blocked. Mirror check_dangerous_command's path:
when a session notify callback is registered, run the blocking
_await_gateway_decision round-trip (redacted payload, once/session/
always persistence, deny/timeout produce definitive BLOCKED outcomes);
fall back to submit_pending only when no callback exists.
Fixes#59413.
Adds approvals.deny to config.yaml — a list of fnmatch globs matched
against terminal commands. A match blocks unconditionally, BEFORE the
--yolo / /yolo / approvals.mode=off bypass, making it the user-editable
counterpart to the code-shipped hardline blocklist.
- Checked in both command gates (check_dangerous_command and
check_all_command_guards), after the hardline floor and sudo-stdin
guard, before the yolo bypass and permanent allowlist.
- Matching runs over the same normalized/deobfuscated command variants
as the dangerous-pattern detector, case-insensitive.
- Opt-in: empty/absent list is a no-op; behavior unchanged.
Supersedes the trust-engine approach from #21500 with a minimal
config-native design: the only capability the existing stack lacked
was deny-that-beats-yolo. Allow already exists (command_allowlist),
ask already exists (session approvals).
Follow-up to #56236: the broadened root token /[/.]*\** treats any run of
dots after the root slash as a collapse spelling, so a literal root-level
directory named '...' (rm -rf /...) was unconditionally hardline-blocked
with no approval path. Tighten the token to /(?:(?:\.\.?)?/)*(?:\.\.?)?\**
so each inter-slash segment must be exactly '.' or '..' — all real collapse
spellings (//, /., /./, /.., //*, ///, /../..) stay on the hardline floor
while literal dot-run dirs fall through to the softer DANGEROUS_PATTERNS
rules like every other real path.
* feat(approvals): /deny <reason> relays denial reason to the agent
Port from qwibitai/nanoclaw#2832 (reject with reason).
Gateway /deny now accepts an optional trailing reason (/deny <reason>
or /deny all <reason>). The reason rides on the per-session approval
entry through resolve_gateway_approval -> _await_gateway_decision and is
appended to the BLOCKED tool result the agent receives, so a declined
agent can adapt instead of only hearing 'denied'.
Adapted to hermes-agent's synchronous single-command /deny model: no DB
state, no second-message capture step, no migration. Reason is capped at
280 chars and threaded through both the terminal-command guard and the
execute_code guard. Plain /deny and the approve paths are unchanged.
- tools/approval.py: _ApprovalEntry.reason; resolve_gateway_approval gains
optional reason; _await_gateway_decision returns it; both gateway BLOCKED
messages include it
- gateway/slash_commands.py: parse leading 'all' + trailing reason
- locales/en.yaml: deny.denied_reason_{singular,plural}
- hermes_cli/commands.py: /deny args_hint '[all] [reason]'
- tests: 3 new (with-reason, all+reason, plain-deny regression)
* fix(ci): localize deny-reason keys across all locales + update interrupt-path assertions
CI surfaced two enforced invariants broken by the deny-with-reason change:
- test_i18n catalog-parity requires every locale to carry the same keys as
en.yaml with matching placeholders. Added deny.denied_reason_singular/plural
(with {count}/{reason}) to all 15 non-English locales.
- test_approval_interrupt asserts the exact dict from _await_gateway_decision,
which now carries a 'reason' key (None on the interrupt/timeout paths).
Extend the pre_tool_call plugin hook return contract with a new directive:
{"action": "approve", "message": "why this needs human confirmation"}
Previously a pre_tool_call hook could only veto a tool call (action: block)
or allow it silently. It could not escalate to the existing human-approval
flow. This unlocks user-defined runtime approval rules on ANY tool (HTTP
writes, file writes to sensitive paths, email sends), enforced at runtime —
resolving #51221 as a pure plugin, with no core approval.py rule schema.
Mechanism:
- get_pre_tool_call_directive() returns (action, message) for block|approve;
get_pre_tool_call_block_message() kept as a block-only back-compat shim.
- resolve_pre_tool_block() is the single dispatch-site chokepoint: fetches
the directive and, for approve, invokes the human gate; fail-closed to a
block on denial, timeout, or gate exception. ALL FOUR tool-dispatch sites
now call it: tool_executor (concurrent + sequential), agent_runtime_helpers,
and model_tools.handle_function_call.
- request_tool_approval() escalates via the SAME machinery as Tier-2
dangerous commands: session/permanent allowlist, prompt_dangerous_approval
(CLI) / submit_pending (gateway), [o]nce/[s]ession/[a]lways/[d]eny,
timeout fail-closed, approvals.cron_mode for cron contexts.
Architecture: extracted the shared decision core into _run_approval_gate(),
called by BOTH check_dangerous_command() and request_tool_approval() so the
fail-closed / cron / gateway / yolo / persist policy lives in ONE place and
cannot drift. Fixed a latent divergence — the plugin path now honors --yolo.
Approval grain: [a]lways is keyed on tool_name + a hash of the reason (an
explicit plugin rule_key overrides), so distinct reasons on the same tool
persist independently instead of one 'always' blanketing the whole tool.
Non-interactive: cron honors approvals.cron_mode (parity with commands); any
other non-interactive non-gateway context fails CLOSED for the plugin path
(the command path keeps its historical fail-open default, unchanged).
No new config schema, no new env vars, no new hook events.
Self-review follow-up on the salvaged approval-routing fix.
The initial adaptation re-read os.getenv("HERMES_YOLO_MODE") at session-build
time. That diverges from the repo's security invariant: HERMES_YOLO_MODE is
frozen into tools.approval._YOLO_MODE_FROZEN at import time precisely so a skill
running mid-process cannot set the env var and instantly flip the approval
bypass (a prompt-injection escalation path). A live re-read re-opened that hole
for the codex routing path.
- Add tools.approval.is_approval_bypass_active() — the canonical three-source
bypass check (frozen --yolo/HERMES_YOLO_MODE + session /yolo + approvals.mode
off) in one place. This is the 4th inline copy of that OR-chain (the three
sites in approval.py and tui_gateway/server.py:3121 all use the same idiom);
the helper is the shared chokepoint they can collapse onto.
- codex_runtime.py now calls is_approval_bypass_active() instead of the
hand-rolled mode-or-session check plus a runtime env re-read.
- Update the env-yolo test to patch _YOLO_MODE_FROZEN (the canonical test
pattern, e.g. tests/tools/test_yolo_mode.py) rather than setenv, which is
dead-on-arrival against the frozen constant.
Fail-closed default preserved on every branch; 28 integration + 77 session/yolo
tests pass; E2E confirms the real exec decision flips decline->accept only when
bypass is active.
git's and sudo's option parsers resolve unambiguous long-flag prefixes, so
`git reset --har`, `git branch --delete --force`, and `sudo --stdi`/`--ask`
execute identically to their full-flag forms while evading the exact-string
DANGEROUS_PATTERNS regexes that gate them. Verified live against real git
and sudo binaries. Widen the patterns to accept unambiguous abbreviations,
scoped narrowly enough to avoid colliding with sibling flags (--help,
--soft/--mixed/--merge/--keep, --shell/--set-home).
Rework follow-up on the Windows destructive-shell detection. The PowerShell
pattern required an explicit -Command/-c before the verb, but PowerShell runs
the verb as the DEFAULT POSITIONAL arg — so `powershell Remove-Item -Recurse
-Force C:\x` (no -Command) slipped through, the exact case the PR body claims
to close. Also missing the canonical `ri` alias.
Anchor the verb to the command position (after the shell name + any leading
-Flag switches + optional -Command/-c) so bare invocations are caught while a
benign path arg containing 'del'/'rm' (e.g. -File c:\del-logs\run.ps1) is not.
Add ri to the verb list. Mutation-verified regression tests for the bare
invocation, ri alias, and the benign-path negative.
Wrapping a catastrophic command in a bare subshell or brace group walked
straight past the unconditional hardline floor -- even under --yolo,
/yolo, approvals.mode=off, and cron approve mode. The command-substitution
forms were already caught; the bare paren / brace-group forms were the gap.
Rather than add the paren and brace openers to the flat _CMDPOS pattern
class (which cannot tell a real subshell opener from one sitting inside a
quoted argument, and would false-positive on ordinary prose such as a PR
title that merely mentions the trigger word), teach the existing
QUOTE-AWARE command-start tokenizer (_iter_shell_command_starts) to treat
the paren and brace openers as command starts, then emit a detection
variant that marks each real command start with a newline (already a
_CMDPOS separator). Openers inside quotes never register as starts, so
quoted arguments are left untouched while real subshell/brace bypasses now
anchor. One place covers every _CMDPOS rule (shutdown/reboot/init/
systemctl/telinit and the rm root/home/system floor).
Tests: subshell/brace bypasses added to the hardline-block, root-wipe, and
yolo-bypass sets; a regression set asserts quoted paren/brace prose is NOT
blocked (guards our own gh-pr-create workflow).
rm -rf //, /., /./, /.. and //* all resolve to / in the shell but slipped
past the root-filesystem hardline pattern, whose target group only matched
the literal / and /* tokens. They fell to the softer DANGEROUS_PATTERNS
'delete in root path' rule, which --yolo / approvals.mode=off / cron
approve-mode are designed to bypass — leaving the one unconditional floor
open to a full root wipe under yolo.
Broaden the root token from '/|/\\*|/ \\*' to '/[/.]*\\**' inside
_hardline_rm_path so any root-anchored path whose components collapse back
to / (repeated slashes plus ./.. segments) with an optional trailing glob
is caught. A trailing real segment (/tmp, /home, /.ssh) still fails to
match and stays with the softer rules.
Co-authored-by: kernel-t1 <214165399+kernel-t1@users.noreply.github.com>
## What does this PR do?
Closes a critical bypass of the dangerous-command approval system. The
normalizer that every command passes through before pattern matching
(`_normalize_command_for_detection`) already strips ANSI, null bytes,
fullwidth Unicode, backslash escapes and empty-quote token splits — but
it did nothing about the shell `IFS` variable. In any POSIX shell `$IFS`
and `${IFS}` expand to whitespace, so a command written as
`rm${IFS}-rf${IFS}/` is executed by the live shell as `rm -rf /` while
the detection regexes — which anchor on literal `\s` between a command and
its arguments — never fire.
The impact is severe: this evades BOTH layers at once. It slips past every
entry in `DANGEROUS_PATTERNS` (so `curl${IFS}...|sh`, `sed${IFS}-i`
against `~/.hermes/config.yaml`, sudo privilege flags, etc. auto-run with
no approval prompt) AND the unconditional hardline floor that is
documented as un-bypassable "not even with --yolo" (`rm -rf /`, `mkfs`,
`dd` to a raw block device, `shutdown`/`reboot`, fork bomb). A
prompt-injected or malicious instruction could wipe the host filesystem or
power the box off while the approval system reports nothing. Confirmed at
runtime before the fix: `detect_hardline_command('rm${IFS}-rf /')` returned
`(False, None)`.
The fix mirrors the shell's own expansion: it collapses `$IFS` / `${IFS}`
(including the bash substring form `${IFS:0:1}`) to a single space inside
the existing de-obfuscation block, so the whitespace-anchored patterns
match exactly as they do for the un-obfuscated command. It is deliberately
narrow and safe — a `\b` word boundary keeps it from touching unrelated
variables like `$IFSACONFIG`, so it cannot introduce false positives on
legitimate commands.
## Related Issue
N/A
## Type of Change
- [x] 🔒 Security fix
## Changes Made
- `tools/approval.py`: in `_normalize_command_for_detection`, substitute
`$IFS` / `${IFS}` (and `${IFS:...}`) expansions with a literal space
before dangerous/hardline pattern matching, alongside the existing
backslash and empty-quote de-obfuscation.
- `tests/tools/test_approval.py`: add `TestIFSWhitespaceBypass` covering
the brace, bare and substring IFS forms against both
`detect_hardline_command` and `detect_dangerous_command`, plus
regression guards that a look-alike variable (`$IFSACONFIG`) and plain
safe commands are not flagged. Import `detect_hardline_command`.
## How to Test
1. Reproduce the hole (pre-fix): `detect_hardline_command('rm${IFS}-rf /')`
returns `(False, None)` and `detect_dangerous_command(...)` returns
`(False, ...)`, i.e. a host-destroying command is auto-approved.
2. With the fix applied, both now flag the command: hardline match
"recursive delete of root filesystem" and dangerous match "delete in
root path".
3. Run the suite: `pytest tests/tools/test_approval.py
tests/tools/test_hardline_blocklist.py -q` — the new
`TestIFSWhitespaceBypass` cases pass and nothing else regresses.
## Checklist
### Code
- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits (`fix(scope):`, etc.)
- [x] I searched for existing PRs to make sure this isn't a duplicate
- [x] My PR contains **only** changes related to this fix (no unrelated commits)
- [x] I've run the relevant tests and they pass (two pre-existing failures
are environmental: missing optional deps in the minimal venv, not
caused by this change)
- [x] I've added tests for my changes
- [x] I've tested on my platform: macOS 15 (Darwin 25.5)
### Documentation & Housekeeping
- [x] I've updated relevant documentation (README, `docs/`, docstrings) — or N/A
- [x] I've updated `cli-config.yaml.example` if I added/changed config keys — or N/A
- [x] I've updated `CONTRIBUTING.md` or `AGENTS.md` if I changed architecture or workflows — or N/A
- [x] I've considered cross-platform impact (Windows, macOS) — the change is a
pure string transform with no platform-specific behavior; footgun gate passes
- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
A literal "rm -rf /" carried as DATA inside another command's quoted
argument — a PR title, a git commit -m message, an echo/printf arg —
tripped the unconditional root-filesystem hardline and could not run at
all. `gh pr create --title "block rm -rf / spellings"` was blocked
outright, because the bare rm path branch matched the mid-string "rm"
(via \brm) with the space after "/" satisfying its (\s|$) terminator.
Anchor the shared _RM_FLAG_PREFIX to _CMDPOS so the rm hardline rules
fire only when rm is an actual command word (start of line, after a
separator ; && || |, after a subshell opener $()/backtick, or after
sudo/env/exec wrappers) — not when the string appears as an argument
value. Broaden the bare-path terminator to also accept shell
metacharacters ) ` ; | & so a real wipe inside a command substitution
is still caught.
The quoted-path branch is unchanged, so quoted root/HOME paths stay
blocked. Adds regression tests for both directions: data-arg false
positives must NOT block, real wipes at every command position must block.
echo <base64> | base64 -d | bash (and base32/base16, xxd -r, tr
transforms, openssl base64/enc -d) decode a dangerous command at
runtime — the raw text carries no dangerous keyword, so the denylist
never fired. Adds DANGEROUS_PATTERNS entries for decode-and-execute
pipes into a shell.
eval $(curl ...), source $(wget ...), and . $(curl ...) executed
remote content but were not covered by the existing pipe-to-shell /
process-substitution patterns. Adds a DANGEROUS_PATTERNS entry so these
command-substitution forms consistently request approval.
Original authorship preserved from PR #26965 (bot-authored commit
re-attributed to the human contributor).
Command-name obfuscation bypassed the dangerous-command denylist: the
executable name could be spelled with shell tricks that survive regex
matching but still resolve to a blocked command at runtime —
$(echo rm), ${0/x/r}m, backticks, and printf substitutions.
Adds a non-executing shell-word scanner that deobfuscates only at
command positions (start, after ;|&&||, inside $(...), after
sudo/env/exec/... wrappers) and feeds the resulting variants through
the existing HARDLINE_PATTERNS / DANGEROUS_PATTERNS — no second
blocklist. Scoping to command words keeps ordinary arguments
(echo $(echo rm) -rf /) from being promoted into command names.
Co-authored-by: egilewski <1078345+egilewski@users.noreply.github.com>
`_normalize_command_for_detection` strips backslash-escapes before matching
DANGEROUS_PATTERNS and HARDLINE_PATTERNS, but the strip rule was
`re.sub(r'\\([^\n])', r'\1', ...)` — its `[^\n]` class deliberately skips
newlines. A backslash immediately followed by a newline is a POSIX line
continuation: the shell removes BOTH characters and joins the tokens, so
`rm -rf \<newline>/` executes as `rm -rf /`. With the dangling backslash left
in place, the structured rm/dd/mkfs patterns no longer match because a literal
`\` sits wedged between the tokens they expect to be adjacent.
The worst consequence is on the HARDLINE floor. The dangerous-command layer
still fired here only by accident (the generic `\brm\s+-[^\s]*r` "recursive
delete" rule needs no path), and that layer is bypassed by `--yolo` /
`approvals.mode=off`. The hardline blocklist — the unconditional floor reserved
for catastrophic, unrecoverable commands and meant to hold even under yolo —
anchors the root path directly after the flags, so `rm -rf \<newline>/`,
`rm -r\<newline>f /`, and `rm -rf \<newline>~` all slipped past it entirely.
A yolo session could therefore wipe the root filesystem.
The fix collapses line continuations (`\` + `\n` or `\r\n`) to nothing,
mirroring the shell, before the existing escape strip runs. This was the gap
left by 621bf3a87, which added the escape strip but only for non-newline chars.
## What does this PR do?
Closes a shell line-continuation bypass in the dangerous-command detector.
Before: `rm -rf \<newline>/` normalized to `rm -rf \<newline>/`, so the
hardline root-delete patterns did not match and the command could run under
`--yolo`. After: line continuations are collapsed first, the command
normalizes to `rm -rf /`, and the hardline floor blocks it unconditionally.
## Related Issue
N/A
## Type of Change
- [x] 🔒 Security fix
## Changes Made
- `tools/approval.py`: in `_normalize_command_for_detection`, add
`command = re.sub(r'\\\r?\n', '', command)` ahead of the existing
backslash-escape strip so shell line continuations (`\`+newline, LF or CRLF)
are removed exactly as the shell would, instead of leaving a stray backslash
that breaks the structured patterns.
- `tests/tools/test_hardline_blocklist.py`: add a parametrized
`test_hardline_blocks_line_continuation` covering the root, in-flag, home,
CRLF, and mkfs continuation forms, plus
`test_line_continuation_root_wipe_cannot_bypass_hardline` asserting the
continuation root wipe stays blocked even with `HERMES_YOLO_MODE=1`.
## How to Test
1. Reproduce: stash the `tools/approval.py` change and run
`scripts/run_tests.sh tests/tools/test_hardline_blocklist.py` — the new
line-continuation cases fail (`rm -rf \<newline>/` is not flagged hardline,
and leaks past the floor under yolo).
2. Restore the change and rerun the file — all 106 tests pass.
3. Regression: `scripts/run_tests.sh tests/tools/test_approval.py` (the
existing fullwidth/ANSI/null-byte normalization and multiline cases still
pass).
## Checklist
### Code
- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits (`fix(scope):`, `feat(scope):`, etc.)
- [x] I searched for existing PRs to make sure this isn't a duplicate
- [x] My PR contains **only** changes related to this fix/feature (no unrelated commits)
- [x] I've run `pytest tests/ -q` and all tests pass
- [x] I've added tests for my changes (required for bug fixes, strongly encouraged for features)
- [x] I've tested on my platform: macOS 15 (Darwin 25.5.0)
### Documentation & Housekeeping
- [x] I've updated relevant documentation (README, `docs/`, docstrings) — or N/A
- [x] I've updated `cli-config.yaml.example` if I added/changed config keys — or N/A
- [x] I've updated `CONTRIBUTING.md` or `AGENTS.md` if I changed architecture or workflows — or N/A
- [x] I've considered cross-platform impact (Windows, macOS) — handles both LF and CRLF line endings
- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
# Conflicts:
# tools/approval.py
The salvaged write-target boundary included `#` in its char class, so a
`#` glued to the redirect/tee path (`echo x > .env#backup`) matched as a
comment boundary and flagged the write as dangerous. But the shell writes
to the distinct file `.env#backup`, not `.env` — a false positive, same
class as the config.yaml.bak case the PR already excluded. Drop `#` from
the boundary; a real trailing comment is always whitespace-preceded (\\s).
Adds regression tests for .env#backup, config.yaml#backup, and
tee .env#backup staying out of the deny.
The dangerous-command approval gate has rules that flag a shell command
when it overwrites a project `.env` or `config.yaml` — these files hold
API keys, DB passwords, and (for `config.yaml`) the approval policy
itself, so a write to them should require user approval. The matching
`write_file`/`patch` deny on the file-tools side was paired with these
terminal-side rules so neither path is an open door.
The redirection and `tee` rules anchored the sensitive path with
`_COMMAND_TAIL` (`(?:\s*(?:&&|\|\||;).*)?$`), which only tolerates the
rest of the line being empty or a command separator. The problem: in
POSIX shell the redirection target is fixed regardless of what trails it.
`echo secret > .env extra` still truncates `.env` (the `extra` is just
another argument to `echo`), and `echo secret > .env # note` does too
(the `#` starts a comment). Because neither tail is a separator, the old
anchor failed to match and the command sailed through approval — a
prompt-injected step could overwrite a project `.env`/`config.yaml`
unprompted. The system-path redirection rule one line above never had
this restriction and already caught these forms.
The fix introduces `_WRITE_TARGET_BOUNDARY`, a lookahead that only
requires the path token to END at a shell word boundary (whitespace,
quote, separator, redirection operator, `#`, or EOL) rather than
demanding the rest of the line be empty. It is applied to the two
stream-write rules (redirection and `tee`) where the sensitive path is
always a write target. The `cp`/`mv`/`install` rule deliberately keeps
`_COMMAND_TAIL`: there the sensitive file is only a target when it is the
LAST argument (the destination), so requiring end-of-line is correct and
keeps `cp config.yaml backup.yaml` (config.yaml as the source) out of the
deny.
## What does this PR do?
Closes a bypass in the dangerous-command approval gate where a trailing
argument or `#` comment after a `>`/`>>`/`tee` write target let a command
overwrite a project `.env` or `config.yaml` without triggering approval,
even though the shell still overwrites the file.
## Related Issue
N/A
## Type of Change
- [x] 🔒 Security fix
## Changes Made
- `tools/approval.py`: add `_WRITE_TARGET_BOUNDARY` (a word-boundary
lookahead) and use it instead of `_COMMAND_TAIL` in the two
project-env/config stream-write patterns ("overwrite project env/config
via tee" and "via redirection"). `_COMMAND_TAIL` is kept and still used
by the `cp`/`mv`/`install` rule, where end-of-line anchoring is the
correct semantics.
- `tests/tools/test_approval.py`: add regression tests for
`> .env extra`, `> .env # note`, `>> config.yaml foo`, and
`tee .env backup` (now flagged), plus `> config.yaml.bak` (must stay
safe — different file).
## How to Test
1. Reproduce: before the fix,
`detect_dangerous_command("echo secret > .env extra")` returns
`(False, None, None)` — the overwrite is not flagged.
2. Apply the fix; the same call now returns the "overwrite project
env/config via redirection" detection.
3. Run `pytest tests/tools/test_approval.py -q` — the new cases pass and
the existing `cp config.yaml backup.yaml` / `config.yaml.bak`
false-positive guards still hold.
## Checklist
### Code
- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits
- [x] I searched for existing PRs to make sure this isn't a duplicate
- [x] My PR contains only changes related to this fix
- [x] I've run the relevant tests and they pass
- [x] I've added tests for my changes
- [x] I've tested on my platform: macOS 15 (Darwin 25.5)
### Documentation & Housekeeping
- [x] I've updated relevant documentation (README, docs/, docstrings) — or N/A
- [x] I've updated cli-config.yaml.example if I added/changed config keys — or N/A
- [x] I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
- [x] I've considered cross-platform impact (Windows, macOS) — or N/A
- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
Teknium
kshitij
spfcraze
Francesco Bonacci
Alex Fournier
Jeffrey Quesnelle
Jeffrey Quesnelle
Joshua
hellno
Jake Long Vu
ansel-f
doncazper
srojk34
dsad
amathxbt
rrevenanttt
YLChen-007
xy200303
friendshipisover