fix(security): harden interpreter-based command detection against obf…

[MorAlekss]

Summary

Fixes two independent security checks that only recognized shell
interpreters (bash/sh/zsh/etc), letting the same attack shapes they
were built to catch slip through under a general-purpose interpreter
name (python3, node, perl, ruby) instead. Also hardens the underlying
command-detection regex against combined short flags, no-space flag
gluing, long-form flags, and versioned interpreter binaries, all of
which evaded detection even for the interpreters that were already
covered.

---

Root cause

hermes_cli/mcp_security.py's validate_mcp_server_entry() checks an
MCP server config entry for the June 2026 hermes-0day network-egress
and OS-persistence attack shapes, but only when command matches
_SHELL_INTERPRETERS (bash, sh, zsh, dash, fish, cmd, powershell,
pwsh). An entry shaped like command: python3, args: [-c, "<the exact same payload>"] returned early with no warnings, since python3 isn't
a shell, even though the inline script it runs is just as capable of
network egress or writing to ~/.ssh/authorized_keys as a bash
one-liner.

tools/approval.py has a parallel gap in its own,
independent detection. Its "script execution via -e/-c flag" pattern
existed to flag exactly this class of command, but it required a
bare, space-separated -c/-e immediately after the literal
interpreter name. Real, unremarkable command forms evade this:
combined short flags (python3 -uc "..."), no-space flag gluing
(python3 -c"..."), long-form flags (node --eval="..."), and
versioned interpreter binaries (python3.11, python3.12 — the norm
under pyenv/homebrew/most distro packaging, where the unversioned name
is often just a symlink). Since approvals.mode, yolo, and the
allowlist all live in ~/.hermes/config.yaml, a command in one of
these forms that rewrites that file to plant a malicious MCP server
entry gets no specific warning about what it targets, and in
sufficiently obfuscated forms evaded detection outright.

---

Behavioral change

Before: an MCP server entry using a general-purpose interpreter instead
of a shell bypassed the egress/persistence checks entirely. Separately,
a command rewriting ~/.hermes/config.yaml through an interpreter's
native file I/O, in a combined-flag, no-space, long-form, or
versioned-binary form, could evade the dangerous-command detection
that would otherwise require approval.

After: hermes_cli/mcp_security.py runs the same egress/persistence
checks for python/node/perl/ruby (in addition to shells), matching
network calls native to each language rather than only shell tools
like curl/wget. tools/approval.py's interpreter/flag matching
recognizes combined flags, no-space gluing, long-form flags, and
versioned binaries, and a command in one of these forms that also
references ~/.hermes/config.yaml or .env gets a specific approval
reason naming the config file, ahead of the generic
script-execution reason.

---

What changed

hermes_cli/mcp_security.py: renamed _SHELL_INTERPRETERS to
_SCRIPT_INTERPRETERS, adding python/python3/node/nodejs/deno/perl/
ruby/php (and their .exe forms). Extended _EGRESS_PATTERN with
interpreter-native network-call patterns (urllib, requests, socket,
http.client, httpx, node's require/fetch, Perl's LWP/Net::HTTP, Ruby's
open-uri, PHP's curl_init/fsockopen/file_get_contents). Updated the
warning text and docstrings from "shell interpreter" to "interpreter"
to match the widened scope.

tools/approval.py: added _INLINE_SCRIPT_INTERPRETER and
_INLINE_SCRIPT_FLAG, shared regex fragments matching versioned
interpreter binaries and inline-script flags in combined, glued, or
long-form shapes. The existing "script execution via -e/-c flag" rule
now uses these. Added a new rule, placed immediately before it, that
matches the same interpreter/flag shape when the command also
references ~/.hermes/config.yaml or .env, giving that case a
specific reason instead of the generic one.

tests/hermes_cli/test_mcp_security.py: added tests covering the
network-egress and persistence shapes via python3/node/perl/ruby, and
a test confirming a benign python/node MCP server (module invocation,
plain script file) is not flagged.

tests/tools/test_approval.py: added a test class covering the
flag-obfuscation and versioned-binary forms against both the generic
and config-specific rules, plus benign cases that must not match.
Updated one existing test whose prior expectation relied on the exact
combined-flag gap this fix closes.

---

What is NOT changed

• PHP is not added to tools/approval.py's interpreter/flag matching:
  its inline-eval flag is -r, which would collide with Ruby's common
  -r<library> require flag under a shared character class, and
  working out that ambiguity is out of scope here
• Variable indirection (X=python3; $X -c ...) and encoded payloads
  (base64 piped through eval) are not addressed; these are inherent
  limitations of regex-based static detection, consistent with the
  rest of this file's approach, not something this PR claims to solve
• Cron jobs run in non-interactive mode where tool invocations are
  auto-approved regardless of which pattern matches (a separate,
  already-open issue); the tools/approval.py half of this fix does
  not change that. The hermes_cli/mcp_security.py half is unaffected
  by this, since it runs at MCP config load/spawn time independent of
  how the entry was written
• All existing approval and MCP-security tests pass

[egilewski]

suggesting changes

This still leaves interpreter-based command detection bypasses in both touched security checks. In hermes_cli/mcp_security.py, the widened MCP validator compares os.path.basename(command) to exact _SCRIPT_INTERPRETERS names, so common versioned interpreter binaries such as python3.11, /usr/bin/python3.11, ruby3.2, and perl5.36 still return no warnings for the same inline persistence payloads that bare python3, ruby, and perl now catch. In tools/approval.py, the new _INLINE_SCRIPT_FLAG match still requires the inline flag to appear immediately after the interpreter and only treats glued payloads as detected when the payload starts with a quote, so valid forms like python3 -S -c "open('~/.hermes/config.yaml','a').write(...)", python3 -copen('~/.hermes/config.yaml','a').write(...), and ruby -eFile.write('~/.hermes/config.yaml',...) bypass both the config/env-specific rule and the generic script-execution rule.

Security evidence:

• trust boundary: user-controlled MCP server configs and terminal commands cross into local interpreter execution and approval policy enforcement.
• source/sink/invariant: inline-script interpreters should not evade MCP egress or OS-persistence checks, nor terminal approval warnings for Hermes config/env access, by using common interpreter spellings or ordinary interpreter flags.
• current-main reproduction: with modules imported from current GitHub main 605727e3b471f22a11ba3698f75d4171f5534674, python3 MCP persistence entries and approval commands such as python3 -uc "open('~/.hermes/config.yaml','a').write(...)" returned no warnings.
• PR replay validation: after replaying the meaningful patch for head 805ab95e3b6ba1444e98c691baa45dacd4f2a152 onto current main, bare MCP interpreters and quoted/glued approval cases are improved, but validate_mcp_server_entry() still returns [] for python3.11, /usr/bin/python3.11, ruby3.2, and perl5.36 persistence entries, and detect_dangerous_command() still returns (False, None, None) for python3 -S -c ..., python3 -I -S -c ..., python3 -copen(...), and ruby -eFile.write(...) targeting ~/.hermes/config.yaml.
• positive/negative cases: python3 -uc ..., python3 -c"...", and node --eval=... now trigger the intended approval rules, while benign python3 -m my_mcp_server and node server.js stay unflagged.
• residual bypass search: the MCP side has exact-name interpreter membership, while the approval-side regex at tools/approval.py only allows the inline flag immediately after the interpreter and its glued-payload branch is limited to quote-starting payloads.
• reviewer validation: focused tests passed (344 passed), and CodeRabbit reported the intervening-flags approval bypass; I reproduced that finding on the replay worktree.

I reviewed the meaningful patch replayed onto current GitHub main because the submitted branch content-conflicts in tools/approval.py against current main; the replay applied cleanly and is suitable for source validation, but it does not by itself prove the submitted branch will merge without conflict resolution.

Signed: GPT-5.5-xhigh in Codex

[MorAlekss]

Fixed:

• hermes_cli/mcp_security.py's _SCRIPT_INTERPRETERS used exact-name frozenset membership, missing versioned binaries (python3.11, python3.12, ruby3.2, perl5.36 — the norm under pyenv/homebrew/most distro packaging). Replaced with a regex pattern that matches these version suffixes.
• tools/approval.py's _INLINE_SCRIPT_FLAG required the target flag immediately after the interpreter, missing intervening flags (python3 -S -c "..."). Also required a quote character right after a glued flag, missing forms like python3 -copen(...) and ruby -eFile.write(...) that aren't glued to a quoted value.

Added:

• _OTHER_INLINE_FLAGS to allow standalone flags before the target -c/-e flag, and widened the glued-flag boundary from requiring a quote to accepting any non-alphanumeric character.
• A shared _VERSIONED_PERL_RUBY pattern for two rules I found while verifying the fix ("in-place edit of Hermes config/env" and "in-place edit of sensitive credential/SSH/shell-rc path") that also used a bare (?:perl|ruby) alternation and missed the same versioned spellings.
• A fix for the "script execution via heredoc" rule, which used its own separate hardcoded interpreter list instead of the shared pattern, so python3.11 << EOF bypassed it the same way.
• A reorder fix: the _OTHER_INLINE_FLAGS change made perl -i -pe '...' config.yaml also match the general interpreter/config rule (since -pe contains a real -e pivot), which would have overridden the more specific, pre-existing "-i in-place edit" message. The specific rule now wins.
• Regression tests for all of the above, verified against the actual bypass commands before and after the fix.

[egilewski]

suggesting changes

tools/approval.py still misses ordinary interpreter option forms before the inline execution flag, so the config/env-specific approval warning can still be bypassed with the same class of interpreter-native writes this PR is trying to cover. On the current head, these all return (False, None, None) from detect_dangerous_command():

python3 -W ignore -c "open('~/.hermes/config.yaml','a').write(1)"
python3 -X dev -c "open('~/.hermes/config.yaml','a').write(1)"
node --no-warnings --eval="require('fs').appendFileSync('~/.hermes/config.yaml','x')"
node -p "require('fs').appendFileSync('~/.hermes/config.yaml','x')"
node --print "require('fs').appendFileSync('~/.hermes/config.yaml','x')"

The first two are Python options that take their own argument before -c; the current _OTHER_INLINE_FLAGS only accepts standalone short flags. The Node --no-warnings --eval case is the same intervening-option issue for a long option, and CodeRabbit also pointed out that node -p / node --print evaluates JavaScript but is not included in _INLINE_SCRIPT_FLAG; I reproduced that locally with the commands above.

Security evidence:

• trust boundary: terminal commands cross into local interpreter execution and approval policy enforcement for writes to Hermes config/env files.
• source/sink/invariant: interpreter-native scripts that reference ~/.hermes/config.yaml or .env should not evade the specific approval warning by inserting normal interpreter options before the inline execution flag or by using Node's print-eval forms.
• current-main reproduction: current GitHub main 1c473bc6a6a0f62e4c264fa0c59ce58606100301 returns (False, None, None) for the bypass commands above.
• PR-head or patch-replay validation: head 56074ee26d033274dd32d3a24e0538e91b91efe3 fixes the earlier same-head review cases such as python3 -S -c ..., python3 -copen(...), and ruby -eFile.write(...), but still returns (False, None, None) for the commands listed above on both the submitted head and a clean patch replay onto current main.
• positive/negative cases: focused tests pass (362 passed for tests/hermes_cli/test_mcp_security.py tests/tools/test_approval.py), and benign python3 -m my_mcp_server / node server.js remain unflagged in the probe.
• residual bypass search: _OTHER_INLINE_FLAGS only models standalone short options without option arguments and _INLINE_SCRIPT_FLAG covers -e, -c, and --eval, but not Node -p / --print.
• reviewer validation: CodeRabbit reported the Node -p / --print gap; I reproduced it with detect_dangerous_command() on the PR head and current-main replay. The Python -W ignore -c, -X dev -c, and Node --no-warnings --eval gaps were found and reproduced in the same local probe.

I reviewed the meaningful patch replayed onto current GitHub main because local git merge-tree reports unrelated histories for the submitted branch while GitHub reports the PR as mergeable/clean; the replay applied cleanly and is suitable for source validation, but it does not by itself prove the submitted branch history merges locally without setup work.

Signed: GPT-5.5-xhigh in Codex

[teknium1]

Thanks for addressing two real interpreter-substitution gaps.

Problems

• The MCP gate still only classifies the top-level command. tools/mcp_tool.py:2182-2235 passes command and args directly to StdioServerParameters, so a launcher form such as env python3 -c ... executes the same payload but bypasses the submitted interpreter-basename pattern in commit 56074ee26d03.
• The approval matcher added in 56074ee26d03 remains incomplete: _OTHER_INLINE_FLAGS handles standalone short options only. As documented in the latest review discussion, ordinary option-argument and long-option forms before the evaluation flag, plus Node -p/--print, remain outside that matcher.

Suggested changes

• Unwrap supported launcher argv before MCP interpreter classification and add wrapped egress/persistence regression tests.
• Use token-based interpreter/flag detection for the approval path, with tests for the documented option-argument, long-option, and Node print-eval forms.

Automated hermes-sweeper review.

[MorAlekss]

@teknium1 thank you for the review.

Rebased onto current main (5349c7c), squashed into one commit.

The obfuscation regexes this PR originally added are gone: #63585 closed those
forms while this was open, so keeping _INLINE_SCRIPT_INTERPRETER,
_OTHER_INLINE_FLAGS and _INLINE_SCRIPT_FLAG would have been a second,
weaker mechanism for the same thing.

MCP gate. validate_mcp_server_entry now strips launcher wrappers from the
full argv before classifying, so the real executable is what gets matched. I
reused the project's own _COMMAND_WRAPPER_WORDS rather than inventing a set,
with _SUDO_OPTIONS_WITH_ARG and _ENV_ASSIGNMENT_RE handling options and
VAR=value exactly as _iter_shell_command_word_spans does; the new helper is
an argv-based counterpart next to those constants, and argv is never re-joined
into a string. Both command: "env", args: ["python3","-c",...] and
command: "env python3", args: ["-c",...] are covered, and messages now name
the unwrapped executable. I left mcp_tool.py untouched on purpose: the fix sits
at the classification gate, so every caller inherits it.

Approval matcher. I removed it rather than widening it. The tokenizer
already resolves every form you listed; on unmodified main, with none of this
branch applied, node -p, node --print, perl -I /tmp -e,
python3 -W ignore -c, python3 --check-hash-based-pycs always -c and
node --max-old-space-size=4096 -e are all detected via
_INTERPRETER_EXEC_FLAGS and _INTERPRETER_WITH_ARG. The suite had no tests
for any of them, so I pinned them here.

Also in this branch, three narrower gaps still open on main:
_interpreter_family accepted php but not php8.2, the default binary name
on Debian/Ubuntu, so that family escaped exec-flag detection entirely (same
spelling fixed in _SCRIPT_INTERPRETER_PATTERN); the two in-place-edit rules
still matched \b(?:perl|ruby)\b, so perl5.36 -pi -e ... config.yaml
degraded to the generic message; and an inline script targeting
~/.hermes/config.yaml or .env was reported as ordinary inline code, which reads
harmless for the file holding approvals.mode and the permanent allowlist. I put
that last one inside _execution_flag_findings rather than DANGEROUS_PATTERNS,
since a parallel regex would have reproduced the exact option-argument blind
spot you raised.

One behavioral change. The new description is not registered in
_REMOVED_PATTERN_KEY_ALIASES, so it does not inherit standing grants for
"script execution via -e/-c flag": a user who permanently approved ordinary
inline execution gets asked again the first time a script targets the policy
file. Intentional, but happy to alias it if you'd rather preserve those grants.

28 tests added to the existing files. Every positive case was verified to fail
before the change and pass after; the negatives hold in both directions.

[egilewski]

suggesting changes

The new MCP launcher unwrapping still stops before the real interpreter when a supported env wrapper uses an option that consumes a separate argument. unwrap_launcher_argv() uses _SUDO_OPTIONS_WITH_ARG for both sudo and env, so entries such as these return no warnings from validate_mcp_server_entry() on this head even though GNU env executes the Python payload:

command: env
args: [--chdir, /tmp, python3, -c, "import urllib.request; ..."]

command: env
args: [--unset, PATH, /usr/bin/python3, -c, "import urllib.request; ..."]

command: env
args: [-S, "python3 -c", "import urllib.request; ..."]

The loop skips the option token but not its owned value, then mistakes /tmp, PATH, or the split-string value for the executable and stops before interpreter classification. Please parse env with its own argument-taking option grammar (including --chdir, --unset, and --split-string/-S) and add egress/persistence regressions for those wrapper forms.

Security evidence:

• trust boundary: user-controlled MCP command and args cross into local process execution through launcher wrappers.
• source/sink/invariant: wrapper parsing must reach the real interpreter after consuming each wrapper option's owned argument.
• current-main reproduction: current main returns no warnings for both plain and option-bearing env interpreter wrappers.
• PR-head or patch-replay validation: this head flags plain env python3 -c ..., but still returns no warnings for the three option-bearing forms above.
• positive/negative cases: plain env and sudo -u wrappers are flagged, while safe GNU env probes confirm the option-bearing forms execute their payloads.
• residual bypass search: the shared parser uses the sudo option-argument set while processing env, which omits the GNU env options above.
• reviewer validation: focused MCP-security and approval tests produced 376 passes; the one failure reproduces unchanged on current main and is unrelated.

Signed: GPT-5.6-sol-xhigh in Codex

[MorAlekss]

Confirmed and fixed in 2cc861a. You were right, and it reaches further than the
MCP gate.

Root cause: skip_wrapper_options was a boolean, so the scan never knew which
wrapper it was inside and applied sudo's option-argument set to everything.
-C and -u only appeared to work because case folding collided them with
sudo's --close-from and --user.

The same defect is on unmodified main in _iter_shell_command_word_spans,
which feeds the approval gate, so these are not flagged as dangerous at all
there:

env --chdir /tmp python3 -c "import os"
env --unset PATH python3 -c "import os"
env -S "python3 -c import os"
exec -a innocent python3 -c "import os"
time -f %U python3 -c "import os"

exec -a and time -f/-o are the same class: both own a separate value, and
neither wrapper was in the option-skipping mode at all, so the scan stopped on
the option itself. I fixed both consumers rather than the argv one alone, since
it is one defect in two callers of the same constant.

skip_wrapper_options is now wrapper_name, with _WRAPPER_OPTIONS_WITH_ARG
carrying a grammar per wrapper for sudo, env, exec and time. Options with an
optional argument (--block-signal, --default-signal, --ignore-signal) are
deliberately excluded: getopt_long accepts those only as --opt=VALUE, so a
bare spelling must not swallow the following word, which is the command.

-S is unwrapped rather than skipped, since env splits the value and executes
it. Spliced into the argv through shlex in the structured path, and expanded
during normalization in the string path, anchored on env so ssh -S and
grep -S are untouched.

One bug the new tests caught in my own fix: unwrap_launcher_argv() ran the
option check through os.path.basename(), which cut --chdir=/tmp down to
tmp so it stopped looking like an option. basename now applies only to the
wrapper-name test. The string path never had this since it compares the raw
word.

Tests, both directions as you asked. Egress and persistence regressions across
--chdir, -C, --unset, -u, the = form, -S, --split-string=,
exec -a and time -f, plus benign option-bearing entries staying clean and
the -S expansion staying anchored to env. Local run 402 passed with the same
single unrelated failure you saw.

[egilewski]

suggesting changes

The new per-wrapper option grammar still omits GNU env's -a / --argv0 option, which consumes an argument before the command. On this head, both of these forms still evade the two changed security checks:

env --argv0 innocent python3 -c "import urllib.request"
env -a innocent python3 -c "import urllib.request"

unwrap_launcher_argv() returns an argv beginning with innocent instead of python3, so validate_mcp_server_entry() returns no warnings. The string-command path stops at the same option operand, so detect_dangerous_command() also returns (False, None, None). GNU env 9.10 documents -a, --argv0=ARG, and a safe invocation-level probe (env --argv0 innocent /usr/bin/printf ...) confirmed that this form executes the following command.

Please add -a / --argv0 to the env argument-owning option grammar and add regressions for both structured MCP argv unwrapping and string-command approval detection.

Security evidence:

• trust boundary: user-controlled MCP argv and terminal command text cross into interpreter execution after launcher-wrapper parsing.
• source/sink/invariant: every supported wrapper option that owns a separate value must consume that value before the scanner identifies the real executable.
• current-main reproduction: current main 588b7059a8b57b0e3dea98b480048eb7199ce0b6 returns no MCP warning and no dangerous-command finding for the --argv0 form.
• PR-head or patch-replay validation: head 2cc861a497089a22b54ba00a5a50796310d2b7f4 returns [] from MCP validation and (False, None, None) from approval detection for both -a and --argv0.
• positive/negative cases: GNU env --argv0 innocent /usr/bin/printf ... executed the following command; the focused MCP-security and approval suite produced 409 passes.
• residual bypass search: _ENV_OPTIONS_WITH_ARG covers unset, chdir, and split-string options but not the documented argv0 option.
• reviewer validation: the focused suite's one failure reproduces unchanged on current main and is unrelated to this PR.

Signed: GPT-5.6-sol-xhigh in Codex

[MorAlekss]

Confirmed and fixed in 1df1f3c. Checking the rest of the tables against their
current manuals turned up more of the same, so this covers three layers.

--argv0. Added to env's option grammar in both the structured argv path and
the string-command path.

sudo's own set. The same check found four more it omits, each stopping the
scan on the operand:

sudo -D /tmp python3 -c "import os"       was not dangerous
sudo -R / python3 -c "import os"          was not dangerous
sudo -r sysadm_r python3 -c "import os"   was not dangerous
sudo -t sysadm_t python3 -c "import os"   was not dangerous

-U/--other-user only escapes this by folding onto --user. The additions sit
in a separate _SUDO_EXTRA_OPTIONS_WITH_ARG so the original constant stays as
it was defined.

Launchers absent from the wrapper set entirely. One level up from the
option grammar: nice, stdbuf, doas, unshare, timeout and chroot
were never unwrapped, so the scan stopped on the launcher itself. In the
approval path the content is still regex-scanned and only the exec-flag rule
goes missing; in the MCP validator the miss is total, since the basename gate
returns before any scan. All six added. timeout and chroot take a
positional operand before the command, which no existing wrapper does, so a
small table drives skipping it.

Where this stops, and why the list is not complete.

su and runuser take the command as a string via -c. That is a shell
invocation rather than a pass-through, so modelling them as wrappers would
skip the payload instead of reaching into it. They belong with the interpreter
families, which is a different change.

xargs short options disagree with their long spellings once case-folded:
-E END against -e/--eof[=END], -I R against -i/--replace[=R], -L
against -l[MAX-LINES], -P against -p/--interactive. In each pair one
form requires a value and the other does not, so a single table cannot express
it safely.

Past those, the honest position is that the wrapper list is open-ended and
this PR does not close it. ionice, taskset, chrt, flock, nsenter,
strace, ltrace, valgrind, firejail, bwrap, systemd-run,
proxychains and torsocks all behave the same way and are still unhandled,
and that is not an exhaustive list either. Most of them would fit the
machinery already here, so the reason for stopping is not difficulty: which
programs belong in a security-relevant wrapper set is a call for the project
rather than for this PR, and every table added carries its own chance of
getting an option wrong in the silent direction. unshare -r is the example.
It reads like a value-taking option next to -R/--root, it is actually
--map-root-user, and treating it as the former swallowed the command until a
probe caught it; there is a regression test for that shape now.

So what this PR fixes is the parsing defect, the wrong option grammar being
applied, plus the launchers most likely to turn up in practice. Closing the
enumeration is a separate question, and enumeration is probably not the answer
to it. Happy to open an issue for that if it is useful.

[egilewski]

not enough evidence

A coherent review tree against current main is unavailable. The run-owned review setup confirmed that PR 57666 conflicts with current main c9de69c6d5ed602059f5e9c9950c150e07b89212 and that deterministic patch replay failed with patch_replay_conflict, so the checkout remained at submitted head 1df1f3cb4e7dcd968c34d2b144dd403ab9869bb0 rather than a successful replay. The submitted branch's stale/conflicted state is informational and is not being treated as a standalone blocker. The evidence gap is the absence of an integrated code state in which the security behavior can be traced and tested.

Without a conflict-resolved replay, I cannot determine whether the proposed interpreter-substitution hardening preserves the intended command-validation and approval invariants while still accepting benign commands. Please provide a deterministic conflict-resolved replay onto the bound current-main SHA, including the exact conflict resolutions, so current-main reproduction, positive and negative cases, both validation paths, and residual bypass variants can be reviewed.

Uncertainty: The integrated files, lines, and semantics after conflict resolution are unknown, so this review does not classify the change as fixed, bypassable, or mergeable.

Signed: GPT-5.6-sol-xhigh in Codex