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
16 changes: 16 additions & 0 deletions .claude/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,22 @@
"Bash(gh api * -F *)",
"Bash(gh api --input*)",
"Bash(gh api * --input*)",
"Bash(gh api --field *)",
"Bash(gh api * --field *)",
"Bash(gh api --raw-field *)",
"Bash(gh api * --raw-field *)",
"Bash(gh api --method=*)",
"Bash(gh api * --method=*)",
"Bash(gh api -X*)",
"Bash(gh api * -X*)",
"Bash(gh api -f*)",
"Bash(gh api * -f*)",
"Bash(gh api -F*)",
"Bash(gh api * -F*)",
"Bash(gh api --field=*)",
"Bash(gh api * --field=*)",
"Bash(gh api --raw-field=*)",
"Bash(gh api * --raw-field=*)",
"Bash(gh issue close*)"
]
},
Expand Down
25 changes: 25 additions & 0 deletions backend/tests/test_agent_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,31 @@ def test_gh_api_writes_ask_in_either_flag_position(rules: dict[str, list[str]])
f"gh api --method DELETE {endpoint}",
f"gh api {endpoint} -f body=hi",
f"gh api -f body=hi {endpoint}",
# Long forms. `gh api --help`: `-F, --field` and `-f, --raw-field`.
# Enumerating only the short spellings left these reaching `allow` via
# `Bash(gh api *)` -- a write that never prompts. This is the exact
# leak the enumeration keeps re-opening, so it is pinned by command
# string here rather than by asserting which rule happens to catch it.
f"gh api {endpoint} --field body=hi",
f"gh api --field body=hi {endpoint}",
f"gh api {endpoint} --raw-field body=hi",
f"gh api --raw-field body=hi {endpoint}",
# Attached-value spellings. cobra/pflag takes `--flag=value` and
# `-fvalue` as readily as `--flag value`; the space-separated globs
# above end in ` --field ` (with a space) and cannot reach them, so
# `--field=body=hi` resolved to `allow` -- a write that never prompts.
f"gh api {endpoint} --field=body=hi",
f"gh api --field=body=hi {endpoint}",
f"gh api {endpoint} --raw-field=body=hi",
f"gh api --raw-field=body=hi {endpoint}",
f"gh api {endpoint} --method=PUT",
f"gh api --method=PUT {endpoint}",
f"gh api {endpoint} -XPUT",
f"gh api -XPUT {endpoint}",
f"gh api {endpoint} -fbody=hi",
f"gh api -fbody=hi {endpoint}",
f"gh api {endpoint} -Fbody=hi",
f"gh api -Fbody=hi {endpoint}",
):
assert decide(write, rules) == "ask", f"{write!r} must ask"

Expand Down
32 changes: 25 additions & 7 deletions docs/agents/local-agent-environment.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ not `.git`'s own recovery data:

| Category | Rules |
|---|---|
| Escapes to GitHub | **every `gh api`**, `gh pr merge`, `gh release create` / `edit` / `delete` / `delete-asset` / `upload`, `gh repo edit`, `gh secret`, `gh workflow run` |
| Escapes to GitHub | **every `gh api` WRITE** (see below), `gh pr merge`, `gh release create` / `edit` / `delete` / `delete-asset` / `upload`, `gh repo edit`, `gh secret`, `gh workflow run` |
| Destroys the recovery mechanism | `git gc`, `git prune`, `git repack`, `git maintenance`, `git reflog expire`, `git reflog delete`, `git update-ref`, `git tag -d` / `--delete` / `-f` |
| Leaves the user boundary | `sudo` |

Expand Down Expand Up @@ -117,18 +117,36 @@ because leaving `rm` and `reset --hard` unattended is only defensible while the
object database and reflog can recover them — a `gc --prune=now` that ran
unprompted would remove the ground that argument stands on.

**`gh api` is guarded bluntly, and that is deliberate.** It was enumerated by
shape first, and it leaked:
**`gh api` asks on WRITES only, and the write flags are enumerated.** Reads
run unattended — `gh api <path>` and `gh api <path> --jq ...` are how a session
reads inline review comments, which no `gh pr view` field returns.

It was blanket-guarded first (`Bash(gh api)` / `Bash(gh api *)` in `ask`),
because the marker that makes a call a write sits at an arbitrary argument
position and **prefix globbing cannot reach it**:

```
gh api repos/o/r/pulls/N/merge -X PUT # merges, bypassing `gh pr merge`
gh api <path> -f key=val # any -f/-F makes it a POST
```

The marker sits at an arbitrary argument position, and **prefix globbing cannot
reach it.** Narrowing it back to specific forms re-opens both lines, so
`quality-check.sh` requires the blanket spelling rather than merely "some `gh
api` rule exists".
The blanket cost a prompt on every read, so #657 replaced it with one rule per
write flag in each argument position. That trade is the standing one, and it
has a standing risk: **a write flag nobody enumerated resolves to `allow`.**
Not hypothetical — `-F, --field` and `-f, --raw-field` (`gh api --help`) had
only their short forms listed, so `gh api <path> --field k=v` was a GitHub
write that never prompted, until it was pinned.

So `quality-check.sh` no longer asserts the *spelling*. It used to require the
literal blanket rules, which made it fail on a clean `main` the moment #657
respelled them, while the property it cared about still held. It now asserts
the *property*, by command string, in `MUST_BE_GUARDED` — those pins survive a
respelling.

**When you add a `gh api` write flag, add its command string to
`MUST_BE_GUARDED` too**, in both argument positions **and both the
`--flag value` and `--flag=value` spellings**. That list is the guard; the
rules in `settings.json` are just how it is currently satisfied.

### Pushing is guarded server-side, not by a prompt

Expand Down
50 changes: 43 additions & 7 deletions scripts/quality-check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -210,12 +210,27 @@ if ! python3 - <<'PY'
import json, re, sys

# Patterns match the command AS WRITTEN -- prefix globbing, no normalisation.
# `gh api` is guarded by a BLANKET rule on purpose: the dangerous shapes put
# their marker at an arbitrary argument position (`gh api <path> -X PUT`,
# `gh api <path> -f k=v`), which a prefix glob cannot reach. Enumerating them
# left real holes twice. Narrowing it back to specific forms re-opens the
# holes, so the check requires the blanket spelling rather than merely "some
# rule exists".
#
# `gh api` USED to be pinned here by spelling: this list demanded the literal
# `Bash(gh api)` / `Bash(gh api *)` blanket rules, on the grounds that the
# dangerous shapes put their marker at an arbitrary argument position
# (`gh api <path> -X PUT`, `gh api <path> -f k=v`) which a prefix glob cannot
# reach. #657 replaced that blanket with an enumeration of the write flags, to
# stop `gh api` reads prompting on every call -- and this gate then failed on
# a clean `origin/main`, because the strings it demanded were gone while the
# PROPERTY it cared about still held.
#
# Pinning a spelling was the wrong assertion. What matters is that a `gh api`
# WRITE cannot reach `allow`, not which rule stops it -- and that is already
# asserted below, by command string, in MUST_BE_GUARDED. Those entries survive
# any future respelling; the literal ones did not.
#
# The enumeration's real risk is a write flag nobody listed. That is not
# hypothetical: `-F, --field` and `-f, --raw-field` (see `gh api --help`) had
# only their SHORT forms enumerated, so `gh api <path> --field k=v` resolved to
# `allow` -- a write that never prompted. Both long forms are pinned below now.
# When adding a `gh api` write flag, add its command string there too, or the
# next respelling re-opens the same hole silently.
#
# `git push` USED to be in that same sentence and no longer is. It was never
# the glob that made it safe -- the glob was a blunt instrument compensating
Expand Down Expand Up @@ -259,7 +274,6 @@ REQUIRED = {
"Bash(podman machine rm)", "Bash(podman system reset)",
],
"ask": [
"Bash(gh api)", "Bash(gh api *)",
"Bash(gh pr merge*)", "Bash(gh repo edit*)",
"Bash(gh release create*)", "Bash(gh release edit*)",
"Bash(gh release delete*)", "Bash(gh release delete-asset*)",
Expand Down Expand Up @@ -322,6 +336,28 @@ MUST_BE_GUARDED = [
# gh reaching GitHub, including the raw API path
"gh api repos/o/r/pulls/1/merge -X PUT",
"gh api repos/o/r/releases -f tag_name=v1",
# Long forms of the two field flags. Enumerating only `-f`/`-F` left these
# reaching `allow`; see the REQUIRED comment above.
"gh api repos/o/r/releases --field tag_name=v1",
"gh api repos/o/r/releases --raw-field tag_name=v1",
"gh api --field tag_name=v1 repos/o/r/releases",
"gh api --raw-field tag_name=v1 repos/o/r/releases",
# Attached-value spellings. cobra/pflag takes `--flag=value`, `-fvalue`
# and `-f=value` as readily as `--flag value`, and a glob ending in
# ` --field ` (with a space) cannot reach them -- a write that resolves to
# `allow` by the same argument the long-form pins above were added for.
"gh api repos/o/r/releases --field=tag_name=v1",
"gh api repos/o/r/releases --raw-field=tag_name=v1",
"gh api --field=tag_name=v1 repos/o/r/releases",
"gh api --raw-field=tag_name=v1 repos/o/r/releases",
"gh api repos/o/r/pulls/1/merge --method=PUT",
"gh api --method=PUT repos/o/r/pulls/1/merge",
"gh api repos/o/r/pulls/1/merge -XPUT",
"gh api -XPUT repos/o/r/pulls/1/merge",
"gh api repos/o/r/releases -fbody=hi",
"gh api -fbody=hi repos/o/r/releases",
"gh api repos/o/r/releases -Fbody=hi",
"gh api -Fbody=hi repos/o/r/releases",
"gh pr merge 588 --squash",
# Publishing a release escapes to GitHub irreversibly. The read-only verbs
# are exempt (see MUST_NOT_BE_GUARDED) -- the gate pins both directions so
Expand Down
Loading