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
146 changes: 119 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 @@ -503,6 +582,19 @@ redundant:
one from outside. `verify-sandbox.sh` skips its symlink check outside a
linked worktree rather than reporting a PASS that proves nothing.

- **`~/.claude/jobs` in the same `allowWrite`** — `claude agents --json` reads
the session list from there, and a sandboxed call silently TRUNCATES it
rather than failing: it returned **1 session where the truth was 12**, so
every other session read as dead. That is not cosmetic. `backlog-rhythm.sh`
keys `resume_implementation` off "no live session", and with a truncated
listing it told the maintainer to re-enter a worktree that a live session was
actively working — a second session on one branch, against commits the advice
itself calls the only copy. `implement-issue` Step 0 depends on the same
reading before it touches a resumed branch.

The failure mode is the dangerous one: not an error, an under-count. Nothing
about a short list looks wrong.

`sandbox.excludedCommands` is **not** used and is not needed. It was tried in
both project and user settings while the four knobs above were missing, appeared
to do nothing, and is now moot.
Expand Down Expand Up @@ -555,22 +647,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