Skip to content
Closed
42 changes: 39 additions & 3 deletions .claude/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
"filesystem": {
"allowWrite": [
".",
"~/GitHub/bess-manager"
"~/GitHub/bess-manager",
"~/.claude/jobs"
]
}
},
Expand Down Expand Up @@ -87,8 +88,43 @@
],
"ask": [
"Bash(git push)",
"Bash(git push *)",
"Bash(git -* push*)",
"Bash(git push *--force*)",
"Bash(git push *-f)",
"Bash(git push *-f *)",
"Bash(git push *--delete*)",
"Bash(git push *-d)",
"Bash(git push *-d *)",
"Bash(git push *--mirror*)",
"Bash(git push *--prune*)",
"Bash(git push *--tags*)",
"Bash(git push *--all*)",
"Bash(git push *--branches*)",
"Bash(git push * +*)",
"Bash(git push *refs/tags/*)",
"Bash(git push * v*)",
"Bash(git push beta*)",
"Bash(git push * beta*)",
"Bash(git push *:beta*)",
"Bash(git push *refs/heads/beta*)",
"Bash(git -* push)",
"Bash(git -* push *--force*)",
"Bash(git -* push *-f)",
"Bash(git -* push *-f *)",
"Bash(git -* push *--delete*)",
"Bash(git -* push *-d)",
"Bash(git -* push *-d *)",
"Bash(git -* push *--mirror*)",
"Bash(git -* push *--prune*)",
"Bash(git -* push *--tags*)",
"Bash(git -* push *--all*)",
"Bash(git -* push *--branches*)",
"Bash(git -* push * +*)",
"Bash(git -* push *refs/tags/*)",
"Bash(git -* push * v*)",
"Bash(git -* push beta*)",
"Bash(git -* push * beta*)",
"Bash(git -* push *:beta*)",
"Bash(git -* push *refs/heads/beta*)",
"Bash(gh api)",
"Bash(gh api *)",
"Bash(gh pr merge*)",
Expand Down
133 changes: 106 additions & 27 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -239,16 +239,17 @@ frontend/node_modules` first) rather than installing through it. Re-running
already contains.** Prompts are the cost, not the safety: a stalled autonomous
run is a guaranteed loss, while anything the sandbox bounds is recoverable.
`rm`, `git reset --hard`, `rebase`, `merge`, `git branch -D`, `git worktree
remove` all run unattended. **Every `git push` asks**, `--force-with-lease`
included — see below for why that one is not carved out.
remove` all run unattended. **A `git push` naming a feature branch runs
unattended too**; a push that forces, deletes a ref, publishes a tag, or targets
`main`/`master`/`beta` asks — see below for how that line is drawn.

What still asks is one closed list, and every entry is there because the
**sandbox cannot contain it** — it bounds the filesystem, not the network, and
not `.git`'s own recovery data:

| Category | Rules |
|---|---|
| Escapes to GitHub | **every `git push`**, **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 | **a `git push` that forces / deletes a ref / publishes a tag / targets `main`, `master` or `beta`**, **every `gh api`**, `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 @@ -301,24 +302,102 @@ 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.

**`git push` and `gh api` are guarded bluntly, and that is deliberate.** Both
were enumerated by shape first, and both leaked, twice:
**`gh api` is guarded bluntly, and that is deliberate. `git push` no longer is,
and that took three tries to get right.** Both were enumerated by shape first,
and both leaked:

```
git push origin main --force # --force not adjacent to `push`
git push origin +beta-release-9.9 # force via refspec
git push origin --delete release-X.Y # destroys a shared ref
git push origin v9.9.0 # publishes a release tag
git push origin HEAD:main # protected ref via colon refspec
git push origin refs/heads/main # bare source-side refspec: no space, no colon
git push origin refs/tags/v1.2.3 # same, for a TAG: `*:refs/tags/*` saw only the colon form
git push --all # every local branch, main included
git push --branches origin # exact synonym for --all
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 these two back to specific forms re-opens every line
above, so `quality-check.sh` requires the blanket spelling rather than merely
"some push rule exists". The cost is one prompt at Step 9 per issue, and a
prompt during release work — which the release skill requires explicit approval
for anyway.
**The last three were found by the Stage 4 review of the PR that fixed the
first five**, which is the point worth remembering: each round of this
enumeration has been incomplete, and each hole was invisible to the round
before. `refs/heads/main` slipped because every protected-branch rule keyed off
the character *before* the branch name — a space in `* main`, a colon in
`*:main` — and in `refs/heads/main` that character is `/`. The guards are now
spelled `*refs/heads/main*` without the colon, which subsumes the
destination-side form rather than sitting beside it. `--all` slipped because
its neighbours `--mirror`, `--prune` and `--tags` were all covered and it
simply was not; `--branches` is its documented synonym (`git push --help`:
"--all, --branches") and was added in the same pass rather than waiting to
become the fourth leak.

`refs/tags/` had the same colon-shaped blind spot as `refs/heads/`, and it is
the one entry here that **branch protection can never replace**: a tag is not a
branch, so no protected-branch setting covers publishing a release tag. Both
guards are now spelled without the colon (`*refs/tags/*`, `*refs/heads/main*`),
each subsuming its destination-side form rather than sitting beside it.

The marker sits at an arbitrary argument position. The first enumeration was
**prefix-anchored** — `Bash(git push --force*)` — which genuinely cannot reach
position 3, so it leaked every line above and was replaced by a blanket
`Bash(git push *)`. That blanket rule was justified here by the claim that "a
prefix glob cannot reach it", **and that claim was wrong**: `*` compiles to `.*`
under `re.fullmatch` and spans spaces, so `Bash(git push *--force*)` matches
`git push origin main --force` perfectly well. The very next paragraph about
greedy globs said so all along; the two contradicted each other for four review
rounds.

**The protected-ref half of that enumeration is now GONE, and branch protection
replaces it.** `enforce_admins` is `true` on `origin/main` and `beta/main`, so
GitHub refuses a push to `main` server-side — including from the owner token
every agent uses. That is a *provable* guarantee; an enumeration of command
spellings is only ever a list of the ones somebody thought of, and that list
leaked three times, the last two spellings (`git push origin refs/heads/main`,
`git push --all`) found by the review of the PR meant to fix it. A fourth round
was the expected outcome of keeping it.

So `git push origin main`, `HEAD:main`, `refs/heads/main`, `-u origin main`
and the `master` equivalents now run unattended and are rejected by GitHub
instead of by a prompt.

**`enforce_admins: true` is therefore LOAD-BEARING.** Turn it off and this repo
has no guard on pushing to `main` at all. `quality-check.sh` asserts it on both
remotes for exactly that reason — a false answer is an error naming the fix, an
unreadable one is a warning, because a CI token cannot read branch protection
and "I could not check" is not "it is off". If it ever fires, either re-enable
the setting or restore the protected-ref patterns; do not silence the check.

What stays guarded is what branch protection does **not** cover — and each of
these is a real gap, not belt-and-braces:

- **force in any position**, `--force-with-lease` included, and refspec `+`
- **ref deletion**, `--mirror`, `--prune`, `--tags`, `--all`, `--branches`
- **tags**: `refs/tags/` on either side, and `* v*`. A tag is not a branch, so
no protected-branch setting will ever cover publishing a release tag
- **the `beta` remote**, and `beta-release-*` branches on origin. Only `main`
is protected, so these are still ours

A push naming a feature branch runs unattended, which is what
`implement-issue` Step 9 and `sweep-prs` actually do.

`gh api` keeps its blanket rule because it is **not** separable this way: any
`-f`, `-F` or `-X` turns a read into a mutation, so there is no lexical marker
that isolates the safe subset. If it starts stalling runs, the fix is an
allow-list of read-only paths, not a shape guard.

`quality-check.sh` pins all of this by COMMAND STRING in three lists, never by
rule name. Adding a rule that "looks right" without adding its command string is
how every previous version of this section went stale.

**But the gate proves "the strings we thought of are covered", not "no push
spelling reaches a protected branch"** — and after three incomplete rounds that
distinction is the honest summary of where this stands. The durable fix is
normalisation: resolve a refspec's effective destination ref before matching,
instead of pattern-matching the raw command string. That is tracked separately;
until it exists, treat any newly-discovered spelling as expected rather than
surprising, and add it to `MUST_BE_GUARDED` first.

**Patterns match the command as written — prefix globbing, no normalisation.**
This is the single biggest source of rules that look right and match nothing,
Expand Down Expand Up @@ -555,22 +634,22 @@ and won't remove a worktree holding uncommitted or untracked files). A second
prompt there buys nothing and costs a stall on every run — `implement-issue`
Step 4's prune loop alone would have hit ~24 of them.

`git push --force-with-lease` **used to** be excluded on the same reasoning, and
is not any more. It is a push, and the push guard has to be blanket (above): the
only spellings that would exempt it — `Bash(git push --force-with-lease*)` — are
prefix-anchored, so they cannot exempt `git push origin main
--force-with-lease`, while any pattern loose enough to catch that also catches
plain `--force`. Exempting it means re-opening the hole. It asks.

**That prompt costs nothing here, which is why the hole stays closed rather
than the rule loosened.** Neither `implement-issue` nor `release` mentions
`rebase`, `--force` or `--force-with-lease` anywhere, and this project merges
the target branch before a PR instead of rebasing — a merge-based flow never
needs a force push. So the rule should lie dormant. **If it ever fires, treat
that as the finding**: an agent has gone off-script into a rebase or an amend
of already-pushed commits. Answer the prompt on its merits; do not "fix" it by
narrowing the push rule — that is how the gaps PR #596 had to close were
introduced in #588.
`git push --force-with-lease` **still asks**, and unlike the ordinary
feature-branch push it is not carved out. `Bash(git push *--force*)` catches it
at any argument position, and that is intended: a lease makes the force safer
against a *concurrent* writer, not against a wrong local history.

**That prompt costs nothing here, which is why it stays.** Neither
`implement-issue` nor `release` mentions `rebase`, `--force` or
`--force-with-lease` anywhere, and this project merges the target branch before
a PR instead of rebasing — a merge-based flow never needs a force push. So the
rule should lie dormant. **If it ever fires, treat that as the finding**: an
agent has gone off-script into a rebase or an amend of already-pushed commits.
Answer the prompt on its merits; do not "fix" it by widening what runs
unattended — that is how the gaps PR #596 had to close were introduced in #588.
The feature-branch carve-out is safe precisely because it is defined by what the
command *lacks* (no force, no `+`, no `--delete`, no tag, no protected ref),
with every one of those shapes pinned by command string in the gate.

**What the unattended set actually risks is uncommitted work**, since the
sandbox contains writes to the repo but cannot distinguish a wanted write from
Expand Down
Loading
Loading