diff --git a/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py b/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py index fd9255962..9a7419f6b 100644 --- a/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py +++ b/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py @@ -133,8 +133,11 @@ # `env` options (GNU coreutils and BSD): flags without a value, long options # whose value is optional (`--default-signal[=SIG]`), and — handled in -# `_env_inner` — `-u`/`--unset`, `-C`/`--chdir`, `-S`/`--split-string`, which -# take one. Any other option leaves the command unparsed, and refused. +# `_env_inner` — `-u`/`--unset` and `-C`/`--chdir`, which take one. Any other +# option leaves the command unparsed, and refused. That includes +# `-S`/`--split-string`: env splits its string with its own escapes (`\_`, +# `\c`, `${VAR}`), which shlex does not reproduce, so the guards would see a +# different command from the one env runs. _ENV_SHORT_FLAGS = frozenset("i0v") _ENV_LONG_FLAGS = frozenset({"ignore-environment", "null", "debug", "list-signal-handling"}) _ENV_LONG_OPTIONAL = frozenset({"block-signal", "default-signal", "ignore-signal"}) @@ -228,21 +231,15 @@ def _unwrap(self, argv: list[str]) -> list[str]: def _env_inner(self, args: list[str]) -> list[str] | None: """The command ``env`` runs, after its options and assignments; None - when an option is not one GNU / BSD ``env`` documents, or a ``-S`` - string does not tokenise.""" - try: - return self._env_inner_parse(list(args)) - except ValueError: - return None - - def _env_inner_parse(self, args: list[str]) -> list[str] | None: + when an option is not one GNU / BSD ``env`` documents, or is ``-S``.""" + args = list(args) while args: tok = args[0] if tok == "--": args.pop(0) break if tok.startswith("--"): - name, eq, value = tok[2:].partition("=") + name, eq, _ = tok[2:].partition("=") if name in _ENV_LONG_FLAGS or name in _ENV_LONG_OPTIONAL: args.pop(0) continue @@ -253,14 +250,6 @@ def _env_inner_parse(self, args: list[str]) -> list[str] | None: return None args.pop(0) continue - if name == "split-string": - args.pop(0) - if not eq: - if not args: - return None - value = args.pop(0) - args = [*shlex.split(value), *args] - continue return None if tok.startswith("-") and tok != "-": args.pop(0) @@ -268,14 +257,11 @@ def _env_inner_parse(self, args: list[str]) -> list[str] | None: for pos, ch in enumerate(letters): if ch in _ENV_SHORT_FLAGS: continue - if ch in "uCS": - attached = letters[pos + 1 :] - if not attached: + if ch in "uC": + if not letters[pos + 1 :]: if not args: return None - attached = args.pop(0) - if ch == "S": - args = [*shlex.split(attached), *args] + args.pop(0) break return None continue diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py index aeb0902cf..11ed7f100 100644 --- a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py +++ b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py @@ -21,4 +21,4 @@ for installs that do not carry the framework source (see `isolated.py`). """ -FRAMEWORK_FINGERPRINT = "sha256:82f684713bcd2b43" +FRAMEWORK_FINGERPRINT = "sha256:5434b3d2df2523b6" diff --git a/tools/agent-guard/README.md b/tools/agent-guard/README.md index c85bac539..00bd986b6 100644 --- a/tools/agent-guard/README.md +++ b/tools/agent-guard/README.md @@ -133,9 +133,13 @@ A denied command is **not** posted/run; the model is shown the reason and the deterministic fix (e.g. "use a backtick `` `login` `` instead of `@login`"). Every guard sees through the prefix wrappers that run their argument as the -command: `env` (with `-i`, `-u NAME`, `-S "…"` and `NAME=value` assignments, +command: `env` (with `-i`, `-u NAME`, `-C DIR` and `NAME=value` assignments, which count as inline overrides) and `command` (`-p`). `env git commit …` is checked as `git commit …`; `command -v git` is a lookup and is left alone. +A wrapper the guards cannot parse is refused (`agent-guard[wrapper]`). That +includes `env -S` / `--split-string`: env splits that string with its own +escapes (`\_`, `\c`, `${VAR}`), so the guards could not be sure they see the +command env runs. ## Per-command overrides diff --git a/tools/agent-guard/src/agent_guard/__init__.py b/tools/agent-guard/src/agent_guard/__init__.py index fd9255962..9a7419f6b 100644 --- a/tools/agent-guard/src/agent_guard/__init__.py +++ b/tools/agent-guard/src/agent_guard/__init__.py @@ -133,8 +133,11 @@ # `env` options (GNU coreutils and BSD): flags without a value, long options # whose value is optional (`--default-signal[=SIG]`), and — handled in -# `_env_inner` — `-u`/`--unset`, `-C`/`--chdir`, `-S`/`--split-string`, which -# take one. Any other option leaves the command unparsed, and refused. +# `_env_inner` — `-u`/`--unset` and `-C`/`--chdir`, which take one. Any other +# option leaves the command unparsed, and refused. That includes +# `-S`/`--split-string`: env splits its string with its own escapes (`\_`, +# `\c`, `${VAR}`), which shlex does not reproduce, so the guards would see a +# different command from the one env runs. _ENV_SHORT_FLAGS = frozenset("i0v") _ENV_LONG_FLAGS = frozenset({"ignore-environment", "null", "debug", "list-signal-handling"}) _ENV_LONG_OPTIONAL = frozenset({"block-signal", "default-signal", "ignore-signal"}) @@ -228,21 +231,15 @@ def _unwrap(self, argv: list[str]) -> list[str]: def _env_inner(self, args: list[str]) -> list[str] | None: """The command ``env`` runs, after its options and assignments; None - when an option is not one GNU / BSD ``env`` documents, or a ``-S`` - string does not tokenise.""" - try: - return self._env_inner_parse(list(args)) - except ValueError: - return None - - def _env_inner_parse(self, args: list[str]) -> list[str] | None: + when an option is not one GNU / BSD ``env`` documents, or is ``-S``.""" + args = list(args) while args: tok = args[0] if tok == "--": args.pop(0) break if tok.startswith("--"): - name, eq, value = tok[2:].partition("=") + name, eq, _ = tok[2:].partition("=") if name in _ENV_LONG_FLAGS or name in _ENV_LONG_OPTIONAL: args.pop(0) continue @@ -253,14 +250,6 @@ def _env_inner_parse(self, args: list[str]) -> list[str] | None: return None args.pop(0) continue - if name == "split-string": - args.pop(0) - if not eq: - if not args: - return None - value = args.pop(0) - args = [*shlex.split(value), *args] - continue return None if tok.startswith("-") and tok != "-": args.pop(0) @@ -268,14 +257,11 @@ def _env_inner_parse(self, args: list[str]) -> list[str] | None: for pos, ch in enumerate(letters): if ch in _ENV_SHORT_FLAGS: continue - if ch in "uCS": - attached = letters[pos + 1 :] - if not attached: + if ch in "uC": + if not letters[pos + 1 :]: if not args: return None - attached = args.pop(0) - if ch == "S": - args = [*shlex.split(attached), *args] + args.pop(0) break return None continue diff --git a/tools/agent-guard/tests/test_guards.py b/tools/agent-guard/tests/test_guards.py index aca3b5807..4e11c3f9d 100644 --- a/tools/agent-guard/tests/test_guards.py +++ b/tools/agent-guard/tests/test_guards.py @@ -856,11 +856,6 @@ def test_prefix_wrappers_reach_the_bundled_guards(prefix): assert reason and "agent-guard[vetted-ops-spelling]" in reason -def test_env_split_string_reaches_the_guards(): - reason = agent_guard.dispatch("""env -S 'git commit -m "x Co-Authored-By: a"'""") - assert reason and "Co-Authored-By" in reason - - def test_command_lookup_is_not_an_execution(): assert agent_guard.dispatch("command -v git") is None @@ -956,7 +951,13 @@ def test_vetted_ops_spelling_refuses_the_equals_form_with_a_tilde(): "env --bogus-option git commit -m x", "env -X git commit -m x", "env -u", # option missing its value - 'env -S "commit \'unbalanced"', # the -S string itself does not tokenise + # env splits a -S string with escapes shlex does not reproduce. + "env -S 'git status'", + "env -S'git status'", + "env -iS 'git status'", + "env --split-string='git status'", + "env --split-string 'git status'", + """env -S 'git\\_commit -m x'""", "command -x git commit -m x", ], ) @@ -980,8 +981,3 @@ def test_an_unparsable_wrapper_is_refused(command): def test_wrapper_option_forms_still_reach_the_guards(prefix): reason = agent_guard.dispatch(f'{prefix}git commit -m "x\n\nCo-Authored-By: a"') assert reason and "Co-Authored-By" in reason - - -def test_a_clustered_split_string_reaches_the_guards(): - reason = agent_guard.dispatch("""env -iS'git commit -m "x Co-Authored-By: a"'""") - assert reason and "Co-Authored-By" in reason diff --git a/tools/agent-guard/tests/test_skill_guards.py b/tools/agent-guard/tests/test_skill_guards.py index 889513400..e4c2de2de 100644 --- a/tools/agent-guard/tests/test_skill_guards.py +++ b/tools/agent-guard/tests/test_skill_guards.py @@ -546,16 +546,10 @@ def test_security_override(): assert dispatch(cmd) is None -@pytest.mark.parametrize( - "prefix", ["env ", "env -i ", "env FOO=1 ", "command ", "env command ", "env -S 'gh"] -) +@pytest.mark.parametrize("prefix", ["env ", "env -i ", "env FOO=1 ", "command ", "env command "]) def test_prefix_wrappers_reach_the_mention_guard(monkeypatch, prefix): monkeypatch.setattr(agent_guard, "_run", fake_run(gh_stub(author="alice"))) - if prefix.endswith("'gh"): - command = f"""{prefix} pr comment 5 --body @bob'""" - else: - command = f'{prefix}gh pr comment 5 --body "@bob please review"' - reason = dispatch(command) + reason = dispatch(f'{prefix}gh pr comment 5 --body "@bob please review"') assert reason and "bob" in reason and "mention" in reason