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
38 changes: 33 additions & 5 deletions .claude/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,22 @@
"Bash(gh pr diff *)",
"Bash(gh pr list *)",
"Bash(gh pr review *)",
"Bash(gh pr create *)"
"Bash(gh pr create *)",
"Bash(git *)",
"Bash(gh pr checks *)",
"Bash(gh pr ready *)",
"Bash(gh pr comment *)",
"Bash(gh run list *)",
"Bash(gh run view *)",
"Bash(gh run watch *)",
"Bash(gh api *)",
"Bash(scripts/request-pr-review.sh *)",
"Bash(scripts/gh-agent.sh *)",
"Bash(./scripts/worktree-setup.sh*)",
"Bash(./scripts/quality-check.sh*)",
"Bash(.venv/bin/python scripts/*)",
"Bash(npm run build*)",
"Bash(npx *)"
],
"deny": [
"Bash(podman machine rm *)",
Expand Down Expand Up @@ -87,11 +102,13 @@
"Bash(git -* stash clear*)",
"Bash(git -* stash branch*)",
"Bash(git -* stash create*)",
"Bash(git -* stash store*)"
"Bash(git -* stash store*)",
"Bash(git reset --hard*)",
"Bash(git push --force*)",
"Bash(git push -f*)",
"Bash(gh auth token*)"
],
"ask": [
"Bash(gh api)",
"Bash(gh api *)",
"Bash(gh pr merge*)",
"Bash(gh release create*)",
"Bash(gh release edit*)",
Expand Down Expand Up @@ -124,7 +141,18 @@
"Bash(git -* tag -d*)",
"Bash(git -* tag --delete*)",
"Bash(git -* tag -f*)",
"Bash(sudo *)"
"Bash(sudo *)",
"Bash(gh api -X *)",
"Bash(gh api * -X *)",
"Bash(gh api --method *)",
"Bash(gh api * --method *)",
"Bash(gh api -f *)",
"Bash(gh api * -f *)",
"Bash(gh api -F *)",
"Bash(gh api * -F *)",
"Bash(gh api --input*)",
"Bash(gh api * --input*)",
"Bash(gh issue close*)"
]
},
"enabledPlugins": {
Expand Down
213 changes: 213 additions & 0 deletions backend/tests/test_agent_permissions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
"""Pins the permission profile in `.claude/settings.json` to what the skills run.

`implement-issue` drives an issue end-to-end inside a worktree. Every command
it needs must run unattended, or the skill stalls overnight waiting for an
approval nobody is awake to give; every command that is the maintainer's call
must stop and ask. Those two lists are derivable — they are written down in
`.claude/skills/*/SKILL.md` — so they can be pinned instead of remembered.

This caught a real inversion: `Bash(gh api *)` sat in `ask` while the blanket
`Bash(gh *)` allow let `gh auth token` and `gh issue close` through. The one
command Step 11 explicitly prescribes (reading inline review comments, which
has no `gh pr view` equivalent) was the one thing blocked.

**What this test does and does not prove.** It models the matcher as
`deny > ask > allow` with `fnmatch` over the rule strings. Claude Code's real
matcher parses shell more carefully than that, so this asserts *the rule set
expresses the intent* — not that the harness behaves this way. The one
behaviour verified against the real matcher by hand is that mid-string
wildcards match (`git -c core.pager=cat stash --help` is caught by
`Bash(git -* stash --*)`), which the `gh api` write-form rules rely on.
"""

import fnmatch
import json
import re
from pathlib import Path

import pytest

REPO_ROOT = Path(__file__).resolve().parents[2]
SETTINGS = REPO_ROOT / ".claude" / "settings.json"

# deny wins over ask, ask wins over allow -- by category, not by specificity.
PRECEDENCE = ("deny", "ask", "allow")


def _rules() -> dict[str, list[str]]:
perms = json.loads(SETTINGS.read_text())["permissions"]
out: dict[str, list[str]] = {}
for category in PRECEDENCE:
patterns = []
for rule in perms.get(category, []):
# "Bash(gh pr view *)" -> "gh pr view *"; trailing " in <repo>" scoping ignored.
match = re.match(r"^Bash\((.*)\)$", rule.split(" in ")[0])
if match:
patterns.append(match.group(1))
out[category] = patterns
return out


def decide(command: str, rules: dict[str, list[str]]) -> str:
for category in PRECEDENCE:
for pattern in rules[category]:
if command == pattern or fnmatch.fnmatchcase(command, pattern):
return category
return "unmatched"


@pytest.fixture(scope="module")
def rules() -> dict[str, list[str]]:
return _rules()


# Every command `implement-issue` runs to take an issue from fetch to a ready
# PR. Sources are noted where the skill delegates rather than spelling it out.
RUNS_UNATTENDED = [
# Step 0/1 -- resume check and scoping
"gh issue view 275 --json title,body,comments",
"gh pr list --state open --search 275 --json number,headRefName",
"git branch --list *issue-275*",
# Step 4 -- prune merged worktrees, then cut the branch.
# `git -C <path> ...` is why git stays blanket-allowed: the flag sits
# between the binary and the subcommand, so `git branch *` misses it.
"gh pr list --state merged --limit 200 --json headRefName",
"git worktree list",
"git -C ../wt branch --show-current",
"git -C ../wt status --porcelain -uno",
"git worktree remove ../wt",
"git branch -D issue-275",
"git fetch origin --prune",
"git worktree add ../wt origin/main", # via superpowers:using-git-worktrees
"./scripts/worktree-setup.sh",
# Steps 5-8 -- TDD, quality gate, local verification
".venv/bin/pytest -m not slow",
".venv/bin/python scripts/capture_vpp_baseline.py",
"./scripts/quality-check.sh",
"podman-compose -f docker-compose.ci.yml up -d",
"npm run build",
"npx vitest run",
# Step 9 -- commit, sync, push, draft PR.
# via superpowers:finishing-a-development-branch (Option 2: push + PR)
"git commit -m fix: something",
"git fetch origin",
"git merge origin/main",
"git push origin HEAD",
"gh pr create --draft --title x --body y",
# Step 10 -- watch this PR to green
"gh pr checks 614 --watch --fail-fast",
"gh run list --workflow PR Review --limit 20",
"gh run view 123 --log-failed",
# Step 11 -- the review loop, and the flag that ends it
"scripts/request-pr-review.sh 614",
"scripts/gh-agent.sh --as dev pr comment 614 --body ok",
"gh pr view 614 --json reviews",
# No `gh pr view` field returns inline review comments -- this endpoint is
# the only way to read them, and it is a plain GET (SKILL.md:698).
"gh api repos/johanzander/bess-manager/pulls/614/comments --jq .[]",
"gh pr ready 614",
]

# Outward-facing, irreversible, or costly -- and none of them enforceable by a
# server-side ruleset, which only governs ref integrity on protected branches.
NEEDS_APPROVAL = [
"gh pr merge 614 --squash", # CLAUDE.md: the merge is the maintainer's, always
"gh pr close 614",
"gh issue close 275", # only the final prod PR closes a reporter's issue
"gh release create v9.9.0",
"gh release delete v9.9.0",
"gh secret set FOO",
"gh workflow run issue-fix.yml", # spends money on a paid agent stage
"gh repo edit --visibility public",
]

# Prohibited outright: the skills' own "never do this" lines, plus the one
# command that would hand a credential to anything reading stdout.
FORBIDDEN = [
"git reset --hard origin/main", # SKILL.md:176, never on a resumed branch
"git push --force origin HEAD",
"git push -f origin HEAD",
"git stash", # denied repo-wide: one shared stack across worktrees
"gh auth token",
]

# `deny` has no override (rules.md:21-24), so a pattern that is broader than
# the danger it targets removes a workflow outright rather than gating it.
# `git reset --hard` discards committed work; the other forms do not, and
# rules.md:36-40 prescribes `--soft` as the WIP-commit recovery pattern now
# that `git stash` is denied repo-wide. An earlier revision of this profile
# denied `git reset*` wholesale and took that recovery path with it.
MUST_NOT_BE_DENIED = [
"git reset --soft HEAD~1", # rules.md:36-40, picking a WIP commit back up
"git reset HEAD backend/app.py", # unstaging; discards nothing
]


@pytest.mark.parametrize("command", RUNS_UNATTENDED)
def test_implement_issue_runs_without_prompting(command, rules):
verdict = decide(command, rules)
assert verdict == "allow", (
f"{command!r} resolves to {verdict!r}. implement-issue runs this "
f"unattended; anything but 'allow' stalls the skill mid-issue."
)


@pytest.mark.parametrize("command", NEEDS_APPROVAL)
def test_maintainer_decisions_still_ask(command, rules):
verdict = decide(command, rules)
assert verdict == "ask", (
f"{command!r} resolves to {verdict!r}, expected 'ask'. This action is "
f"outward-facing or irreversible and no server-side ruleset covers it."
)


@pytest.mark.parametrize("command", FORBIDDEN)
def test_prohibited_commands_are_denied(command, rules):
verdict = decide(command, rules)
assert verdict == "deny", f"{command!r} resolves to {verdict!r}, expected 'deny'."


@pytest.mark.parametrize("command", MUST_NOT_BE_DENIED)
def test_non_destructive_forms_keep_an_escape_hatch(command, rules):
verdict = decide(command, rules)
assert verdict != "deny", (
f"{command!r} is denied. `deny` never prompts, so this removes the "
f"workflow entirely rather than gating it -- narrow the pattern to the "
f"destructive form instead."
)


def test_gh_api_writes_ask_in_either_flag_position(rules):
"""`gh api` reads must pass; writes must ask regardless of argument order.

`Bash(gh api * -X *)` alone does not match `gh api -X POST <endpoint>` --
the leading `*` cannot match the empty span before the flag. Both
orderings need their own rule, so both are asserted here.
"""
endpoint = "repos/johanzander/bess-manager/pulls/614/comments"
assert decide(f"gh api {endpoint}", rules) == "allow"

for write in (
f"gh api {endpoint} -X POST -f body=hi",
f"gh api -X POST {endpoint} -f body=hi",
f"gh api {endpoint} --method DELETE",
f"gh api --method DELETE {endpoint}",
f"gh api {endpoint} -f body=hi",
f"gh api -f body=hi {endpoint}",
):
assert decide(write, rules) == "ask", f"{write!r} must ask"


def test_no_client_rule_duplicates_a_server_ruleset(rules):
"""Ref protection is the server's job; duplicating it is pure friction.

`main`, `beta` and tags are protected by GitHub rulesets with empty bypass
lists, so a push that should not happen is already impossible. A
client-side `git push` gate therefore cannot fire on the dangerous case --
only on the routine one, prompting on every feature-branch push.
"""
for pattern in rules["ask"]:
assert not pattern.startswith("git push"), (
f"{pattern!r} duplicates server-side ref protection. The dangerous "
f"case is already blocked; this only prompts on routine pushes."
)
Loading