fix(approvals): honor GNU env split escapes and argv0 operands

This commit is contained in:
Teknium
2026-09-07 14:20:56 -07:00
parent 50617d1c75
commit 6178e9f4ee
4 changed files with 128 additions and 9 deletions

View File

@@ -42,11 +42,39 @@ with tempfile.TemporaryDirectory(prefix="hermes-deny-dispatch-") as directory:
"env -S 'printf %s' 'sudo -n id'", "env -S printf BENIGN",
"env -S " + shlex.quote("bash -c " + shlex.quote(binary + " -n id")),
"echo ok # ignored\n bash -c " + shlex.quote(binary + " -n id")]
cases += [
r"env -S 'printf\_SAFE'", r"env -S 'printf\_SAFE\c ignored'",
"env -S 'printf SAFE # ignored'", "env -S 'printf # ignored' SAFE",
"env -a marker printf SAFE", "env --argv0 marker printf SAFE",
"env --argv0=marker printf SAFE", "env -amarker printf SAFE",
r"env -a marker -S 'printf\_SAFE'",
"env -S '\"printf\"\\_SAFE'", "env -S \"'printf' SAFE\"",
r"env -S 'printf\_\_SAFE'", r"env -S 'printf\c ignored' SAFE",
"env -a sudo printf BENIGN", "env --argv0 sudo printf BENIGN",
"env -S 'printf %s \"sudo\\_-n\\_id\"'",
r"env -S 'printf BENIGN\c bash -c sudo'",
"env -S 'printf BENIGN # bash -c sudo'",
r"env -S 'printf %s \${IGNORED}'",
]
rows = []
for command in cases:
result = registry.dispatch("terminal", {"command": command, "workdir": directory, "timeout": 10})
if isinstance(result, str):
result = json.loads(result)
rows.append({"command": command, "result": result})
# Optional assertions keep the same harness usable for before/after receipts.
expected_outputs = {5: 'sudo -n id -u', 6: 'sudo;-n;id;', 9: 'ok',
10: 'sudo -n id', 11: 'BENIGN', 27: 'BENIGN', 28: 'BENIGN',
29: 'sudo -n id', 30: 'BENIGN', 31: 'BENIGN', 32: '${IGNORED}'}
if '--verify' in sys.argv:
for index, row in enumerate(rows):
result = row['result']
if index in expected_outputs:
assert result.get('exit_code') == 0, row
assert result.get('output', '').strip() == expected_outputs[index], row
else:
assert result.get('status') == 'blocked', row
assert 'user-defined deny rule' in result.get('error', ''), row
assert not result.get('output'), row
print(json.dumps({"source": str(source), "config": approval_context._get_approval_config(),
"rows": rows}, indent=2))

View File

@@ -6,6 +6,7 @@ making it the user-editable counterpart to the code-shipped hardline floor.
"""
import os
import shlex
import pytest
@@ -88,6 +89,13 @@ def test_deny_follows_executable_identity(deny_config, clean_env, monkeypatch):
"echo ok # ignored\n bash -c '/usr/bin/sudo -n id -u'",
"printf SAFE", "env -S printf SAFE", "env -Sprintf SAFE",
"env --split-string=printf SAFE", "env -S 'printf' SAFE",
r"env -S 'printf\_SAFE'", r"env -S 'printf\_SAFE\c ignored'",
"env -S 'printf SAFE # ignored'", "env -S 'printf # ignored' SAFE",
"env -a marker printf SAFE", "env --argv0 marker printf SAFE",
"env --argv0=marker printf SAFE", "env -amarker printf SAFE",
r"env -a marker -S 'printf\_SAFE'",
"env -S '\"printf\"\\_SAFE'", "env -S \"'printf' SAFE\"",
r"env -S 'printf\_\_SAFE'", r"env -S 'printf\c ignored' SAFE",
"2>/tmp/log FOO=bar /usr/bin/sudo -n id -u",
"if true; then /usr/bin/sudo -n id -u; fi",
]
@@ -95,6 +103,7 @@ def test_deny_follows_executable_identity(deny_config, clean_env, monkeypatch):
deny_config(["sudo *", "printf SAFE"], mode=mode)
monkeypatch.setattr(mod, "_YOLO_MODE_FROZEN", yolo)
for command in commands:
assert approval_floors._match_user_deny_rule(command), command
for guard in (mod.check_dangerous_command, mod.check_all_command_guards):
result = guard(command, "local")
assert result.get("user_deny") is True, (mode, yolo, command, result)
@@ -112,6 +121,12 @@ def test_deny_projection_preserves_data_and_path_rules(deny_config):
'printf "%s" "$(printf safe) sudo -n id -u"',
"command -v sudo", "command -V sudo", "command -pv sudo",
"env -u sudo printf ok", "exec -a sudo printf ok",
"env -a sudo printf ok", "env --argv0 sudo printf ok",
r"env -S 'printf %s\_sudo\_-n\_id'",
"env -S 'printf %s \"sudo\\_-n\\_id\"'",
r"env -S 'printf ok\c bash -c sudo'",
"env -S 'printf ok # bash -c sudo'",
r"env -S 'printf %s \${IGNORED}'",
"ionice --pid sudo", "chrt --pid sudo", "taskset --pid 1 sudo",
"echo 'first\nsudo -n id -u'", "echo ok # ; sudo -n id -u",
"env -S 'printf %s; sudo -n id'",
@@ -131,6 +146,25 @@ def test_deny_projection_preserves_data_and_path_rules(deny_config):
deny_config(['printf "a b"'])
assert approval_floors._match_user_deny_rule('env printf "a b"')
assert approval_floors._match_user_deny_rule('env printf "a b"') is None
# The argv projection must retain GNU escapes as data, not shell syntax.
from tools.approval_detection import _split_env_string
for literal, expected in (
(r'a\_b', ['a', 'b']), (r'"a\_b"', ['a b']),
(r"'a\_b'", [r'a\_b']), (r'a\cb ignored', ['a']),
('a # ignored', ['a']), ('a#b', ['a#b']), (r'\#a', ['#a']),
(r'\${NAME}', ['${NAME}']), ("'${NAME}'", ['${NAME}']),
(r"'a\'b'", ["a'b"]), (r"'a\\b'", [r'a\b']),
(r'a\"b', ['a"b']), ('"" a', ['', 'a']),
*((rf'a\{key}b', ['a' + value + 'b'])
for key, value in {'f': '\f', 'n': '\n', 'r': '\r', 't': '\t', 'v': '\v'}.items()),
):
assert _split_env_string(literal) == expected, literal
command = 'env -S ' + shlex.quote('printf %s ' + literal)
deny_config(['sudo *'])
assert approval_floors._match_user_deny_rule(command) is None, command
for unresolved in ('${NAME}', '"${NAME}"', r'"a\cb"', r'a\qb', "'unclosed"):
assert _split_env_string(unresolved) is None
class TestDenyBeatsYolo:

View File

@@ -506,7 +506,8 @@ _SUDO_OPTIONS_WITH_ARG = {"-c", "--close-from", "-g", "--group", "-h", "--host",
# data, not executable positions; option spelling remains case-sensitive.
_COMMAND_WRAPPER_OPTIONS_WITH_ARG = {
"chroot": {"--groups", "--userspec"},
"sudo": _SUDO_OPTIONS_WITH_ARG, "env": {"-C", "--chdir", "-S", "--split-string", "-u", "--unset"},
"sudo": _SUDO_OPTIONS_WITH_ARG,
"env": {"-a", "--argv0", "-C", "--chdir", "-S", "--split-string", "-u", "--unset"},
"exec": {"-a"}, "nice": {"-n", "--adjustment"},
"time": {"-f", "--format", "-o", "--output"},
"timeout": {"-k", "--kill-after", "-s", "--signal"},
@@ -1181,6 +1182,64 @@ def _shell_command_segment(command: str, start: int) -> str:
return command[start:end].strip()
def _split_env_string(payload: str) -> list[str] | None:
r"""Project GNU env -S literal argv, not POSIX shell words.
Dynamic ${NAME} expansion is deliberately not evaluated: the execution
backend's environment need not be this process's environment.
"""
escapes = {"f": "\f", "n": "\n", "r": "\r", "t": "\t", "v": "\v",
"#": "#", "$": "$", "\"": "\"", "'": "'", "\\": "\\"}
args, word = [], []
quote, started, index = None, False, 0
while index < len(payload):
char = payload[index]
index += 1
if char == "\\":
if index == len(payload):
return None
escaped = payload[index]
if quote == "'" and escaped not in ("'", "\\"):
word.append(char)
started = True
continue
index += 1
if escaped == "c":
if quote:
return None
break
if escaped == "_" and quote is None:
if started:
args.append("".join(word))
word, started = [], False
continue
if escaped not in escapes and escaped != "_":
return None
word.append(" " if escaped == "_" else escapes[escaped])
started = True
continue
if char in ("'", '"') and (quote is None or char == quote):
quote = char if quote is None else None
started = True
continue
if quote is None and char in " \t\n\r\v\f":
if started:
args.append("".join(word))
word, started = [], False
continue
if quote is None and char == "#" and not started:
break
if char == "$" and quote != "'":
return None
word.append(char)
started = True
if quote:
return None
if started:
args.append("".join(word))
return args
def _env_split_payload(tokens: list[str]) -> str | None:
index = 1
while index < len(tokens):
@@ -1194,12 +1253,10 @@ def _env_split_payload(tokens: list[str]) -> str | None:
index += 1
payload = (value if option == "--split-string" else token[2:]) if attached else (
tokens[index] if index < len(tokens) else "")
try:
# env splits argv, not shell syntax: protect literal separators and
# expansions when feeding the existing command-position scanner.
return shlex.join(shlex.split(payload) + tokens[index + 1:])
except ValueError:
return None
args = _split_env_string(payload)
# Protect literal separators when reusing command-position detection;
# only a real shell -c carrier may turn these argv bytes into code.
return shlex.join(args + tokens[index + 1:]) if args is not None else None
index += 2 if not equals and option in _COMMAND_WRAPPER_OPTIONS_WITH_ARG["env"] else 1
return None

View File

@@ -157,7 +157,7 @@ Details:
- Patterns are [fnmatch](https://docs.python.org/3/library/fnmatch.html) globs (`*`, `?`, `[...]`) matched **case-insensitively** against the whole command text and individual executable-command candidates. `git push --force*` matches `git push --force origin main` but not `git push origin main`.
- Matching runs over the same normalized/deobfuscated command variants the dangerous-pattern detector uses, so simple quoting tricks (`git pu""sh --force`) don't slip past a rule.
- Executable candidates retain the literal path and also match its basename: `sudo *` covers `/usr/bin/sudo -n id` and `./sudo -n id`. A path-specific rule such as `/usr/bin/sudo *` does **not** become a rule for every binary named `sudo`.
- Quote-aware parsing exposes commands after assignments, leading redirections, `;`, `&&`, `||`, pipelines, groups, command substitutions, and ordinary `if`/`then`/`else`/`do` transitions. Supported launchers include `sudo`, `env`, `command`, `exec`, `nohup`, `setsid`, `time`, `nice`, `timeout`, `stdbuf`, `ionice`, `chrt`, `taskset`, and `chroot`. Known option operands are skipped; `command -v`/`-V` lookups are not executions. Shell `-c` payloads are inspected recursively. Literal `env -S` / `--split-string` strings are split into arguments with the remaining command arguments appended; shell punctuation inside those arguments stays data unless an actual shell `-c` consumes it. Shell comments do not introduce executable candidates.
- Quote-aware parsing exposes commands after assignments, leading redirections, `;`, `&&`, `||`, pipelines, groups, command substitutions, and ordinary `if`/`then`/`else`/`do` transitions. Supported launchers include `sudo`, `env`, `command`, `exec`, `nohup`, `setsid`, `time`, `nice`, `timeout`, `stdbuf`, `ionice`, `chrt`, `taskset`, and `chroot`. Known option operands are skipped; `command -v`/`-V` lookups are not executions. Shell `-c` payloads are inspected recursively. Literal executable-and-argument strings in `env -S` / `--split-string` use GNU quoting and escapes (including `\_` word boundaries and `\c` termination), with the remaining command arguments appended; shell punctuation inside those arguments stays data unless an actual shell `-c` consumes it. `env -a` / `--argv0` values are arguments, not executable names. Shell and GNU split-string comments do not introduce executable candidates.
- In the additional executable candidates, whitespace **between** words is collapsed, but quoted argument content and argument paths are retained. An exact rule such as `git status` therefore also matches `env git\tstatus; echo done` (where `\t` represents a tab). Quoted mentions such as `echo 'sudo -n id'` are not promoted to commands. Existing whole-input globs such as `*sudo*` still intentionally match mentions anywhere.
- **YAML quoting:** always quote patterns. A bare leading `*` is a YAML alias and fails to parse; `{`, `!`, and `: ` have their own YAML meanings. Single quotes are safest for shell-ish content.
- User-defined deny rules apply to all terminal backends, including isolated containers, before any backend-specific approval shortcut.
@@ -166,7 +166,7 @@ Details:
Like the rest of the approval config, changes take effect immediately (the config cache is mtime-keyed) — no session restart needed.
:::note Threat model
Deny rules are a shell-command policy, not a complete shell interpreter or an OS capability sandbox. Normalization does not resolve arbitrary variables, aliases, functions, renamed binaries, scripts, interpreter programs, or every shell/launcher grammar (for example, case-pattern syntax). Do not use a basename deny rule as a guarantee that a capability cannot be reached by other means. For containment, use OS permissions and an isolated backend with appropriately restricted mounts, credentials, and network access. This matching behavior does not change the configured approval mode or the empty-deny-list default.
Deny rules are a shell-command policy, not a complete shell interpreter or an OS capability sandbox. Normalization does not resolve arbitrary variables (including GNU `env -S` `${NAME}` expansion), aliases, functions, renamed binaries, scripts, interpreter programs, or every shell/launcher grammar (for example, case-pattern syntax, clustered launcher options, or options embedded inside an `env -S` string). Do not use a basename deny rule as a guarantee that a capability cannot be reached by other means. For containment, use OS permissions and an isolated backend with appropriately restricted mounts, credentials, and network access. This matching behavior does not change the configured approval mode or the empty-deny-list default.
:::
### Approval Timeout