feat(agent-guard): add harness-neutral --check and --exec CLI modes#765
feat(agent-guard): add harness-neutral --check and --exec CLI modes#765justinmclean wants to merge 6 commits into
Conversation
potiuk
left a comment
There was a problem hiding this comment.
Thanks @justinmclean — the --check / --exec modes are a clean idea, well-documented and well-tested on the happy paths. One blocker, plus a few minors.
Blocker — the documented --exec wrapper install silently disables the guard. dispatch() matches the command head on exact seg.argv[0] in {"gh","git"} (and the inner guards on seg.argv[:2] == ["git","commit"] / ["git","push"]). The exec_main docstring / README document the PATH-wrapper install as exec … --exec /usr/bin/git "$@" — but /usr/bin/git ≠ "git", so dispatch returns None and the command is allowed, unguarded. A Co-Authored-By commit or a git push routed through the documented wrapper sails straight through. The bare-name variant (--exec git) instead makes os.execvp re-resolve git → the wrapper → loop → trip _EXEC_DEPTH_MAX and refuse to run. So the wrapper-script pattern is a no-op guard either way; only the shell-alias pattern actually works.
The upside — this uncovered a hardening opportunity in the shared guard layer, not just in --exec. None of the adapters normalize the command head: Claude main(), opencode_main(), and the new --exec/--check all hand the raw command straight to dispatch, which compares exact "git"/"gh". So the absolute-path (and ./git) bypass is latent for every harness today — it just isn't the path a model normally takes, whereas --exec makes it the documented invocation. Fix it once at the Segment layer (basename-normalize argv[0]) and it closes the hole for --exec and hardens Claude + OpenCode in a single change. Execution is unaffected: exec_main hands the original argv to os.execvp, so absolute-path wrappers still run the real binary; only the guard decision is normalized.
Concretely, in Segment.__init__ (currently self.argv = tokens[i:]):
argv = tokens[i:]
# Normalise the command head to its basename so a path-qualified
# invocation (`/usr/bin/git`, `./git`) is guarded identically to the
# bare name. Guard rules only inspect argv[0] as a command *name*; the
# real execution path is untouched (exec_main hands the original argv
# to os.execvp).
if argv:
argv[0] = os.path.basename(argv[0])
self.argv: list[str] = argv(This covers path-qualified heads but not prefix wrappers like env git … / command git …, which keep argv[0] == "env" — a separate, pre-existing, all-harness gap worth a follow-up.)
Please also add a regression test asserting /usr/bin/git commit … Co-Authored-By … is denied — that's exactly the config that's currently unguarded and untested.
Minors (non-blocking):
- README calls both modes "fail-open," but the
_EXEC_DEPTH_MAXpath returns1without running the command — that's fail-closed. Worth reconciling the wording. _AGENT_GUARD_EXEC_DEPTHis inherited by the child and never reset, so a legitimately deepgit→shell→gitchain accumulates toward 20. Generous, but a per-invocation reset on the allow path would be safer.test_check_decision_matches_dispatchbuilds input with" ".join(...)whilecheck_mainusesshlex.join(...); it passes by luck of the substring scan, not by exercising the real join/re-split path.
This review was drafted by an AI-assisted tool and confirmed by a Magpie maintainer. After you've addressed the points above and pushed an update, a Magpie maintainer — a real person — will take the next look. The findings cite the project's review criteria; if you think one is mis-applied, please reply on the PR and a maintainer will weigh in.
More on how Magpie handles maintainer review: CONTRIBUTING.md.
| # re-resolve the bare name 'git' through $PATH, find this wrapper again, | ||
| # and loop. Adjust the path to your real git. | ||
| #!/usr/bin/env bash | ||
| exec python3 /path/to/agent-guard.py --exec /usr/bin/git "$@" |
There was a problem hiding this comment.
This is the install that's silently unguarded today (/usr/bin/git ≠ "git" in the head match). With the basename-normalization described in the review summary it becomes correct as written. Until then, only the alias git='… --exec git' form actually enforces anything — worth calling that out here so nobody installs the wrapper expecting protection they don't have.
Any runtime without a harness-specific hook API can now enforce guard rules by wrapping command execution with the new entry points: - `--check <cmd…>` — inspects the command; exits 0 (allow, silent) or 2 (deny, reason on stdout). Shell scripts can capture the reason and gate execution on the exit code. - `--exec <cmd…>` — inspects then exec-replaces this process with the command on allow; exits 2 with the deny reason on stderr on deny. Suitable as a transparent PATH-shadowing wrapper or shell alias for `git` and `gh`. Both modes use the same `dispatch()` core as the Claude Code and OpenCode adapters, so guard decisions are identical across all paths. Also adds `__main__.py` so the package is runnable as `python -m agent_guard`, and documents the new enforcement path in the README under a "Harness-neutral path (any runtime)" wiring section. Validation: all 74 agent-guard tests pass; vendor-neutrality score remains 100% (agent-guard stays portable). Generated-by: Claude (claude-sonnet-4-6)
b5a2d70 to
7688192
Compare
…nvocations can't bypass the guard
Addresses the review blocker on the --exec wrapper install: dispatch matched
the command head on exact seg.argv[0] in {gh,git}, so a documented
'--exec /usr/bin/git' wrapper (or ./git, or a model emitting an absolute path
in Claude/OpenCode) took the unguarded fast path. Segment now normalises
argv[0] to os.path.basename, closing the hole across all harnesses. Execution
is untouched (exec_main hands the original argv to os.execvp). Adds regression
tests for absolute/relative-path git heads.
Summary
Any runtime without a harness-specific hook API can now enforce guard rules by wrapping command execution with the new entry points:
--check <cmd…>— inspects the command; exits 0 (allow, silent) or 2 (deny, reason on stdout). Shell scripts can capture the reason and gate execution on the exit code.--exec <cmd…>— inspects then exec-replaces this process with the command on allow; exits 2 with the deny reason on stderr on deny. Suitable as a transparent PATH-shadowing wrapper or shell alias forgitandgh.Both modes use the same
dispatch()core as the Claude Code and OpenCode adapters, so guard decisions are identical across all paths.Also adds
__main__.pyso the package is runnable aspython -m agent_guard, and documents the new enforcement path in the README under a "Harness-neutral path (any runtime)" wiring section.Validation: all 74 agent-guard tests pass; vendor-neutrality score remains 100% (agent-guard stays portable).
Generated-by: Claude (claude-sonnet-4-6)
Type of change
.claude/skills/<name>/) — eval fixtures updated belowtools/<system>/*.md)tools/*/withpyproject.toml)docs/,README.md,CONTRIBUTING.md)projects/_template/)prek, workflows, validators)Test plan
prek run --all-filespassesuv run pytest/ruff check/mypypasses(
PYTHONPATH=tools/skill-evals/src python3 -m skill_evals.runner tools/skill-evals/evals/<skill>/)(a regression test for the bug fixed / the behaviour added — see CONTRIBUTING.md)