Skip to content

feat(setup-steward): verify check 8d — propose permission allow-list audit on adopt + upgrade#415

Merged
potiuk merged 1 commit into
apache:mainfrom
potiuk:feat-setup-steward-permission-audit
May 31, 2026
Merged

feat(setup-steward): verify check 8d — propose permission allow-list audit on adopt + upgrade#415
potiuk merged 1 commit into
apache:mainfrom
potiuk:feat-setup-steward-permission-audit

Conversation

@potiuk

@potiuk potiuk commented May 31, 2026

Copy link
Copy Markdown
Member

Summary

Adds a new verify check (8d) that audits the adopter's per-machine
permission allow-list (<repo>/.claude/settings.json and
<repo>/.claude/settings.local.json) and surfaces two kinds of finding:

  • ✗ Forbidden entries — broad wildcards that grant arbitrary code
    execution. Per-entry proposal to remove. Examples already seen in
    real adopter trees: Bash(uv run *), Bash(python3 *),
    Bash(npm run *), Bash(bash *), Bash(gh api *),
    Bash(docker run *), Bash(sudo *).
  • ⚠ Recommended entries — narrow read-only patterns the framework's
    security family invokes constantly. Family-scoped so the proposal
    matches the adopter's opted-in skill set.

Because verify is chained from both /setup-steward adopt (Step 12
sanity check) and /setup-steward upgrade (Step 9 sanity check), every
adopt + upgrade pass now surfaces this audit automatically — no
separate skill invocation needed.

Why

The user's own airflow-s allow-list had silently accumulated
Bash(uv run *) and Bash(python3 *) over time. Both are
arbitrary-code-execution holes; both stayed for months because no
periodic check flagged them. The verify chain is the right place for a
periodic capability-surface review — it already runs on every adopt +
upgrade, and proposing-rather-than-applying matches the framework's
golden rule that operator changes to capability surface stay under
operator control.

What

### 8d. Permission allow-list hygiene:

  • Reads both .claude/settings.json and .claude/settings.local.json.

  • For each entry in permissions.allow[]:

    • Buckets against a forbidden list (interpreters, shells, package
      runners, task-runner wildcards, broad gh api / docker run /
      sudo wildcards). ✗ per hit. List is explicitly not exhaustive
      — the same-category rule extends to anything that can spawn an
      arbitrary process.
    • Buckets against a family-scoped recommended list:
      • security family → 9 Gmail/PonyMail read-only MCPs +
        Bash(vulnogram-api-record-fetch *)
      • Any adopter shipping docs → Bash(lychee *)
    • ⚠ per recommended entry missing.
  • Reports the per-entry JSON-pointer path
    (.permissions.allow[<index>]) so the operator can locate findings
    instantly.

  • Read-only — proposes, never auto-writes. Interactive apply via
    /setup-steward verify --apply-permission-audit (atomic JSON
    read → mutate → write so concurrent setup-isolated-setup-install
    writes do not silently clobber; per-write sandbox-bypass
    authorisation when the target file is at a denyWithinAllow path).

The recommended list is deliberately narrow — every entry is
verified against Claude Code's auto-allowed harness exclusions
(READONLY_COMMANDS, GIT_READ_ONLY_COMMANDS, GH_READ_ONLY_COMMANDS,
etc.) so the framework does not redundantly propose entries that never
prompt anyway.

Test plan

  • /setup-steward verify on a checkout whose .claude/settings.local.json has Bash(uv run *) → ✗ on 8d with JSON-pointer path, removal proposal printed
  • On a checkout whose .claude/settings.local.json is missing mcp__claude_ai_Gmail__get_thread and the adopter opted into the security family → ⚠ on 8d, paste-ready string printed
  • On a checkout whose .claude/settings.local.json is missing mcp__claude_ai_Gmail__get_thread but the adopter did not opt into security → no ⚠ (family-scoped)
  • On a checkout with no .claude/settings*.json at all → ⚠ (soft signal), suggests /setup-isolated-setup-install
  • /setup-steward adopt and /setup-steward upgrade both chain through verify and surface the 8d findings in their final report

🤖 Generated with Claude Code

…audit on adopt + upgrade

Adds a verify check that audits the adopter's per-machine permission
allow-list and surfaces:

- **Forbidden entries** (✗) — broad wildcards that grant arbitrary
  code execution, e.g. `Bash(uv run *)`, `Bash(python3 *)`,
  `Bash(npm run *)`, `Bash(bash *)`, `Bash(gh api *)`,
  `Bash(docker run *)`. Per-entry proposal to remove.
- **Recommended entries** (⚠) — narrow read-only patterns the
  framework's `security` family invokes constantly: Gmail MCP
  `get_thread` / `search_threads` / `list_drafts` / `list_labels`,
  PonyMail MCP `search_list` / `auth_status` / `get_thread` /
  `get_email` / `list_restrictions`, `Bash(vulnogram-api-record-fetch
  *)`. Plus `Bash(lychee *)` for every adopter that ships docs.

Both surfaces are *proposals only* — the allow-list is the operator's
capability surface for the agent in this checkout, and even an
objectively-safer edit (drop a known-dangerous wildcard) is a
capability change the operator must own. The framework's job is to
surface the gap; the operator's job is to close it.

Because verify is chained from both `/setup-steward adopt` (Step 12
sanity check) and `/setup-steward upgrade` (Step 9 sanity check),
every adopt + upgrade pass now surfaces this audit automatically.
The recommended-entry list is family-scoped — an adopter who skipped
the `security` family will not see the Gmail/PonyMail entries surfaced
as gaps.

The interactive apply path (`/setup-steward verify
--apply-permission-audit`) does an atomic JSON read → mutate → write
so concurrent `setup-isolated-setup-install` writes do not silently
clobber the diff, and requires per-write sandbox-bypass authorisation
when the target settings file lives at a `denyWithinAllow` path.

Motivation: the user's own `airflow-s` allow-list had accumulated
`Bash(uv run *)` and `Bash(python3 *)` over time, both of which are
arbitrary-code-execution holes. A periodic audit catches this drift
on every adopt + upgrade rather than relying on the operator to
remember to run `/fewer-permission-prompts`.
@potiuk potiuk merged commit bb23c55 into apache:main May 31, 2026
16 checks passed
potiuk added a commit that referenced this pull request May 31, 2026
… of permissions.allow[] (#417)

Real implementation behind the `--apply-permission-audit` flag of
`/setup-steward verify` check 8d (introduced in #415). The skill
previously described the audit + atomic-edit semantics abstractly;
this PR adds the actual CLI the skill calls.

Three subcommands:

- `audit <path> [--families ...]` — read a settings file, classify
  `permissions.allow[]` against the forbidden + family-scoped
  recommended lists, print structured JSON. Exit 1 on any forbidden
  hit so a shell caller can pipeline. The JSON-pointer path
  (`.permissions.allow[<index>]`) returned for each forbidden entry
  lets the verify report point operators at the exact location.

- `apply <path> [--add ...]... [--remove ...]... [--create-if-missing]`
  — atomic add/remove. POSIX `fcntl.flock` advisory exclusive lock on
  the target file, re-parse under the lock, mutate, write to a sibling
  temp file, `os.replace` into place. Concurrent
  `setup-isolated-setup-install` writes (which touch
  `sandbox.filesystem.*` on the same file) serialise without either
  losing work. Unrelated top-level keys (`extraKnownMarketplaces`,
  `hooks`, `permissions.deny`) preserved verbatim.

- `list-known` — dump the canonical forbidden + recommended-by-family
  lists for diff-friendly inspection.

The forbidden list covers the broad arbitrary-exec wildcards observed
in real adopter trees: `Bash(uv run *)`, `Bash(python3 *)`,
`Bash(npm run *)`, `Bash(bash *)`, `Bash(gh api *)`,
`Bash(docker run *)`, `Bash(sudo *)`, plus the interpreter / shell /
package-runner extensions documented in verify check 8d.

The recommended list is family-scoped — every entry is read-only and
verified against Claude Code's auto-allowed harness exclusions
(`READONLY_COMMANDS`, `GIT_READ_ONLY_COMMANDS`, ...) so the tool
does not propose entries that never prompt anyway.

Wired into:

- `.pre-commit-config.yaml` — ruff + mypy + pytest gates on
  `tools/permission-audit/**` (matches the github-body-field convention).
- `docs/labels-and-capabilities.md` — new tool row, `capability:setup`.
- `.claude/skills/setup-steward/verify.md` — check 8d's
  "Implementation" paragraph now references the CLI with concrete
  `uv run --project` invocations.

Stdlib-only runtime — fast `uv run` resolution + no surprises on a
fresh install. 22 tests covering classification, family-scoping,
duplicate JSON-pointer numbering, atomic add/remove, no-op-rewrite
skip, malformed-JSON detection.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant