Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
Expand Down Expand Up @@ -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
Expand All @@ -253,29 +250,18 @@ 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)
letters = tok[1:]
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@
for installs that do not carry the framework source (see `isolated.py`).
"""

FRAMEWORK_FINGERPRINT = "sha256:82f684713bcd2b43"
FRAMEWORK_FINGERPRINT = "sha256:5434b3d2df2523b6"
6 changes: 5 additions & 1 deletion tools/agent-guard/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
36 changes: 11 additions & 25 deletions tools/agent-guard/src/agent_guard/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
Expand Down Expand Up @@ -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
Expand All @@ -253,29 +250,18 @@ 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)
letters = tok[1:]
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
Expand Down
18 changes: 7 additions & 11 deletions tools/agent-guard/tests/test_guards.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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",
],
)
Expand All @@ -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
10 changes: 2 additions & 8 deletions tools/agent-guard/tests/test_skill_guards.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down