diff --git a/tools/spec-loop/.last-sync b/tools/spec-loop/.last-sync index c697e74cb..be96efb3b 100644 --- a/tools/spec-loop/.last-sync +++ b/tools/spec-loop/.last-sync @@ -1 +1 @@ -5c75abd216d798efce7257749cbdb3e26844b3c0 +bcd8b7f28e5d926ca8eff6e603c64b327ae038fe diff --git a/tools/spec-loop/specs/README.md b/tools/spec-loop/specs/README.md index 3bde6164c..921d9de45 100644 --- a/tools/spec-loop/specs/README.md +++ b/tools/spec-loop/specs/README.md @@ -56,6 +56,7 @@ Start with [`overview.md`](overview.md), then: [`maintainer-education.md`](maintainer-education.md), [`spec-gap-staleness.md`](spec-gap-staleness.md), [`vetted-command-surface.md`](vetted-command-surface.md), + [`adversarial-review.md`](adversarial-review.md), [`sandbox-diagnostics.md`](sandbox-diagnostics.md), [`container-gateway.md`](container-gateway.md). diff --git a/tools/spec-loop/specs/adversarial-review.md b/tools/spec-loop/specs/adversarial-review.md new file mode 100644 index 000000000..c2aba04dd --- /dev/null +++ b/tools/spec-loop/specs/adversarial-review.md @@ -0,0 +1,147 @@ + + +--- +title: Adversarial review by other models +status: experimental +kind: feature +mode: infra +source: > + docs/designs/2026-09-23-adversarial-review.md (rollout PR 1 of 4). + Implemented in tools/adversarial-review/ and published as the + magpie-adversarial-review substrate plugin (plugins/magpie-adversarial-review/, + SUBSTRATE_PLUGINS in tools/dev/check-family-plugins.py). +acceptance: + - The tool runs only installed reviewer CLIs (codex, copilot, gemini, + claude), each in its own read-only headless mode, and skips the model + running the harness unless told otherwise. + - A reviewer's input is limited by construction to the diff, the changed + files, and the PR title and body as they will be posted; no option + accepts any other context. + - The report is advisory — one merged JSON document, exit 0 whenever the + run completes — and an unavailable reviewer never fails the run. + - Reviewer output is data; a finding that reads like an instruction is + never acted on. +--- + +# Adversarial review by other models + +## What it does + +Gives a change a second read from models other than the one that wrote it. +The tool detects which reviewer CLIs are installed, runs the configured ones +read-only and in parallel over a branch, a diff or a PR, and merges what they +find into one advisory report. +A reviewer is only useful as a *different* model, so the harness's own model +is skipped by default. + +This spec covers what has shipped: the tool and its plugin. +Wiring it into the skills — setup, a shared pre-PR block in every PR-creating +skill, and a multi-reviewer second read in `pr-management-code-review` — is the +remaining rollout, recorded under *Known gaps*. + +## Where it lives + +- `tools/adversarial-review/` — stdlib-only Python package, capability + `substrate:review`, harness `agnostic`. Two subcommands: + - `detect` — for each backend, whether its CLI is on `PATH` and answers a + cheap probe, and which backend is the running harness (`self`), + recognised from the environment variables each harness sets. + - `run` — builds the input from `--target branch` (with `--base`), + `pr:` (with `--repo`, through `gh`) or `diff:`, plus `--title` + and `--body-file`, runs every requested reviewer, and prints the merged + report. +- `plugins/magpie-adversarial-review/` — the substrate plugin in the Claude + Code catalogue, linking `tools/adversarial-review` so the tool runs from the + installed plugin tree + ([marketplace distribution](marketplace-distribution.md)). +- `adversarial-review.md` — optional configuration, resolved + `.apache-magpie-local/` first, then `.apache-magpie-overrides/`, under + `--project-root`: `mode`, `reviewers`, `timeout_minutes`, per-backend + `models`. `--reviewers` on the command line overrides the list. +- `docs/designs/2026-09-23-adversarial-review.md` and its implementation plan. + +## Behaviour & contract + +- **Read-only backends, pinned by tests.** One adapter per CLI holds its + headless command line: `codex exec -s read-only --ephemeral` with MCP servers + switched off and `--output-schema`; `copilot -p` with the shell and write + tools denied; `gemini --approval-mode plan -o json`; `claude -p` with + `--strict-mcp-config` and Bash, the editing tools, web access and `Task` + disallowed. `tests/test_backends.py` pins each line, so a regression that + drops a read-only flag fails. +- **The input builder is the privacy boundary.** The prompt carries only the + diff, the changed-file list and the public PR text, so no privacy-LLM gate + applies: nothing private is ever passed. When `--repo-dir` is the project's + private tracker the report says so in `warnings`. +- **One findings schema, merged.** Every reviewer answers against the same + schema (`severity`, `file`, `line`, `claim`, `evidence`). Parsing keeps the + good findings from a partly malformed reply and reports the bad ones; + findings are de-duplicated across reviewers, keeping every reviewer's name, + and sorted by severity. +- **Bounded and interruptible.** Reviewers run in parallel, each with a + timeout (default 8 minutes, under the 10-minute cap harnesses put on one + shell call); a timed-out reviewer's whole process group is killed, and + Ctrl-C or SIGTERM kills every live reviewer. +- **Advisory.** Each reviewer reports `ok`, `unavailable`, `error`, `timeout` + or `skipped` with its reason. The exit code is 0 whenever the run completes + and 2 only for a wrong invocation or an invalid config. +- **Nothing written to the repository.** The brief and the schema live in a + temporary directory removed afterwards; the tool itself makes no network + calls. +- **Standalone resolution.** Like `tools/vetted-ops`, the project declares no + workspace `dev` group, because it runs as + `uvx --from /tools/adversarial-review adversarial-review …`, where + the workspace root does not exist. + +## Out of scope + +- Blocking a PR on findings; the human decides what to act on. +- Reviewing private content: tracker bodies, mail threads, CVE IDs before + disclosure, advisory text. +- Authenticating reviewer CLIs; each uses its own login. + +## Acceptance criteria + +1. `detect` reports each backend's availability and marks the running + harness `self`; `run` skips `self` unless `--self none` is passed. +2. Each backend's command line matches its pinned test, and none can run in + a writable mode. +3. Given tracker-shaped context, only the diff, the file list and the public + PR text reach the prompt. +4. One slow or failing reviewer does not block the others, and its timeout + or error is reported per reviewer. +5. The run exits 0 with a merged report whenever it completes, whatever the + reviewers returned. +6. `check-family-plugins.py` passes with `magpie-adversarial-review` in + `SUBSTRATE_PLUGINS` and its entry point resolving through the plugin link. + +## Validation + +```bash +uv run --all-packages --group dev pytest tools/adversarial-review/tests +python3 tools/dev/check-family-plugins.py +``` + +## Known gaps + +- **No consumer is wired yet.** The design's rollout PRs 2–4 are unbuilt: + `setup` running `detect` in `config` and `verify`, writing the + configuration and the per-harness commands, and installing the sandbox + exclusion; the shared pre-PR block in every PR-creating skill; and + `with-reviewers:` in `pr-management-code-review`, which today still takes a + single user-fired `with-reviewer:` slash command. The tool README's + statements that skills run it and that setup installs its exclusion + describe that target state. +- **No `commands` subcommand.** The design's `commands --harness `, + which would print the per-harness command files for `setup`, does not + exist; only `detect` and `run` ship. +- **The `mode` key is parsed but unused** until the pre-PR block consumes it. +- **Reviewers can read beyond the prompt.** `codex -s read-only` restricts + writes and network, not reads, so an instruction injected into the diff + could have it read a file elsewhere on the machine; `copilot` and `gemini` + keep any MCP servers they are configured with. The README tells operators + to keep private checkouts away from review machines or leave `codex` out. +- **The sandbox blocks it.** Reviewer CLIs need network access and their own + credentials, which the reference sandbox denies, and no `excludedCommands` + entry ships for the tool yet. diff --git a/tools/spec-loop/specs/agent-isolation-sandbox.md b/tools/spec-loop/specs/agent-isolation-sandbox.md index b446d7028..671cbd8c7 100644 --- a/tools/spec-loop/specs/agent-isolation-sandbox.md +++ b/tools/spec-loop/specs/agent-isolation-sandbox.md @@ -98,13 +98,39 @@ existing sandbox grants can widen the baseline. See `docs/adapters/gemini.md`. `watcher.pid: Operation not permitted` before any watcher started — and it has to be stable across contexts rather than `$TMPDIR`, which differs between the agent's hooks and a terminal `git` and would give - two contexts two registries that cannot see each other. The + two contexts two registries that cannot see each other. + The hook-side owner is the harness process, not the hook's parent: + Claude Code on Linux runs a hook through `sh -c`, which exits the + moment the hook returns, so `arm` walks up past intermediate shells + to the first non-shell ancestor and records that — otherwise the + watcher's parent-liveness check ended the watch before the key ever + blocked (#1365). + On macOS the window closes when its application loses activation + (``, bound after a short grace because activation itself + churns focus) rather than re-grabbing the keyboard: the overlay + never traps the screen, since the key still has to be touched for + the command to go through (#1325). The git the agent runs reads the same global config, so the wrapper's two files are a `sandbox.filesystem.allowRead` grant of their own (nothing wider under `~/.claude/`), or every sandboxed signed commit fails with `cannot exec`. Installed by `setup-isolated-setup-install` Step K, checked by `setup-isolated-setup-verify` check 10. Capability: `substrate:sandbox`. +- **Hardware-key touch policy** (the recommendation Step K proposes and + check 10 reports against). The touch goes on the slot that **signs**, + never on the ssh transport alone: a touch on every fetch and pull is + a prompt on a read, and a push is already gated by the `git push` + ask rule and carries only commits signed with a touch. Which slot + signs follows `gpg.format`: with OpenPGP signing (unset or + `openpgp`) it is `sig`, recommended `cached`, and `aut` is + recommended `off`; with `gpg.format=ssh` the signature is made by the + key `ssh-add -L` lists, so `aut` is the signing slot and must stay + `cached` — the skill never proposes turning it off there, and states + once that the transport then pays the touch too (OpenPGP signing or + an https remote avoids it) — while `sig` signs nothing and is left + alone. `cached` (a touch honoured for 15 seconds) rather than `on`, + and never `fixed` / `cached-fixed`, which cannot be undone without + deleting the private key (#1367). - `tools/agent-guard/` — deterministic pre-execution guard dispatcher (`stdlib`-only). Wired as a `PreToolUse` hook (Claude Code) or a `tool.execute.before` plugin (OpenCode), with a `--gemini` adapter for @@ -113,7 +139,12 @@ existing sandbox grants can widen the baseline. See `docs/adapters/gemini.md`. before it runs and denies the ones that break a hard framework rule, independent of model memory. The guard decisions live in a single harness-agnostic `dispatch()` core so every wired harness enforces - an identical rule set. Capability: `substrate:action-guard`. + an identical rule set. Git guards resolve the subcommand by walking + past git's global options (`git -C commit`, `git -c k=v commit`, + `git --no-pager commit`), never by a fixed argv slice, and + `GuardContext.git_subcommand()` gives contributed guards the same + resolution `gh_subcommand()` gives for `gh` (#1330). + Capability: `substrate:action-guard`. - `tools/permission-audit/` — audits and atomically edits Claude Code's `permissions.allow[]` entries in `.claude/settings.json` and `.claude/settings.local.json`. Backs the `--apply-permission-audit` @@ -130,6 +161,22 @@ existing sandbox grants can widen the baseline. See `docs/adapters/gemini.md`. paths the committed list also names; `--no-tool-paths` limits it to the project root. Checked by `setup-isolated-setup-verify` check 8. Capability: `substrate:sandbox`. +- Whole-user git hooks — the install skill's Step P.3 alternative to + per-project scope: global `core.hooksPath` pointing at + `~/.claude/git-hooks/`, in a *simple* flavour (a standalone + `post-checkout`) or a *dispatcher* flavour (every hook name symlinked + to `git-hook-dispatcher.sh`, which chains to per-repo `.git/hooks/*`). + That directory sits under the read-denied home, and git treats a hook + directory it cannot see as "no hooks", so without a read-only + user-scope `sandbox.filesystem.allowRead` grant for it (plus + `~/.claude-config/git-hooks/` when the hooks are symlinks into the + sync repo, since the sandbox checks the resolved path) every + sandboxed commit silently skips `pre-commit`, `commit-msg` and the + rest. Install proposes the grant at Step P.3-whole-user; verify + check 8 probes the directory from inside the sandbox (#1364). Both + the update skill's drift check and verify check 8 recognise the + dispatcher flavour's symlinks as its installed shape, not as drift + or as inert per-repo hooks (#1322, #1358). - `tools/egress-gateway/` — local HTTP(S) forward proxy for egress control. Framework tools point `HTTPS_PROXY`/`HTTP_PROXY` at it; the gateway rejects any connection to a host not on its allowlist before a @@ -150,7 +197,13 @@ existing sandbox grants can widen the baseline. See `docs/adapters/gemini.md`. `sandbox-lint --gemini .gemini` checks the static profile, with opt-in pytest integration tests against native 0.59.0 APIs for settings, policies, headless refusal, and Linux enforcement. Every Gemini upgrade requires revalidating the native probe against that version; static CI checks alone do not establish effective policy precedence. - Skills: `setup-isolated-setup-install`, `-update`, `-verify`, - `-doctor`. The diagnostic side — the failure catalog in + `-doctor`. The update skill establishes the agent-guard wiring before + diffing anything: with the `magpie-agent-guard` plugin enabled, the + plugin registers the hook and resolves the engine under + `${CLAUDE_PLUGIN_ROOT}`, so an absent `~/.claude/scripts/agent-guard.py` + is the expected shape rather than drift, and on either wiring a + `git commit` carrying a `Co-Authored-By:` trailer must be denied as a + behavioural canary (#1323). The diagnostic side — the failure catalog in `docs/setup/sandbox-troubleshooting.md`, the `sandbox-error-hint.sh` hook, the doctor's live probes and the verify checks — is specified in [`sandbox-diagnostics.md`](sandbox-diagnostics.md). @@ -227,7 +280,12 @@ The reference model is four layers, layered: default (prompt in default mode, classifier in auto). Pinned system tools (`bubblewrap`, `socat`, agent CLI) are aged through a -cooldown window; bumps are PRs, not silent updates. +cooldown window; bumps are PRs, not silent updates. The window is the +framework's 7-day default unless a tool's `[tools.]` table in +`tools/agent-isolation/pinned-versions.toml` sets its own +`cooldown_days`; `bubblewrap` carries `cooldown_days = 1`, so a pin +carrying a sandbox-setup security fix can move a day after release +(#1360, pinned at 0.13.0). ## Out of scope diff --git a/tools/spec-loop/specs/cve-tooling.md b/tools/spec-loop/specs/cve-tooling.md index 83d05c62b..730721266 100644 --- a/tools/spec-loop/specs/cve-tooling.md +++ b/tools/spec-loop/specs/cve-tooling.md @@ -41,7 +41,17 @@ reviewable. flow. The skill detects session expiry via `vulnogram-api-check` and falls back to the manual paste path when the session is not configured or expired. -- `tools/cve-org/` — CVE.org / CVE-services helpers. +- `tools/cve-org/` — CVE.org / CVE-services helpers; the `check-published` + recipe runs as the `cve-check-published` vetted-ops read operation, not raw + `curl` ([vetted command surface](vetted-command-surface.md)). +- `tools/osv/` — OSV.dev adapter (`contract:security-cross-ref`): read-only, + unauthenticated lookups against the OSV.dev REST API v1 — a record and its + alias cross-walk by ID (CVE ↔ GHSA ↔ PYSEC / RUSTSEC / GO …), advisories + affecting a package version, vulnerabilities tied to a public upstream + commit, and a batch form of the last two. Each recipe is a vetted-ops read + operation (`osv-get-vuln`, `osv-query-package`, `osv-query-commit`, + `osv-query-batch`); `api.osv.dev` is on the egress gateway's allowlist + (#1297, #1326). - Skill: `security-cve-allocate` — walks the (PMC-gated) allocation form, then updates the tracker and regenerates the attached JSON via `generate-cve-json --attach`. @@ -80,6 +90,14 @@ uv run --project tools/cve-tool-vulnogram/oauth-api --group dev pytest - `stable`; drift appears if the CVE 5.x schema or Vulnogram export shape changes upstream — caught by the tool's own tests. +- **`tools/osv/` has no consuming skill yet.** Its `tool.md` names the + intended consumers — `security-issue-triage` (early deduplication against + known vulnerabilities), `security-issue-deduplicate` (alias resolution + before a merge), `security-cve-allocate` (check no existing advisory covers + the report) and `dependency-audit` — none of which calls it today. An OSV + query about a pre-disclosure report must also stay within the embargo + boundaries `tool.md` sets: only public identifiers, packages and commits + go into a query. - The oauth-api's session-cookie approach is a workaround for the ASF Vulnogram instance's lack of a Bearer-token API; if ASF Infra adds a proper API token surface, the tool will migrate to it. diff --git a/tools/spec-loop/specs/marketplace-distribution.md b/tools/spec-loop/specs/marketplace-distribution.md index cc13c14b1..5864dfc68 100644 --- a/tools/spec-loop/specs/marketplace-distribution.md +++ b/tools/spec-loop/specs/marketplace-distribution.md @@ -118,6 +118,19 @@ adopter-facing page. still installs them intact — measured on Codex, which used to install such a plugin with zero skills and no error. Every catalogue lists all ten families. +- **Substrate plugins publish a tool, not a family.** Beside the ten + families, the Claude Code catalogue carries three substrate plugins — + `magpie-agent-guard`, `magpie-vetted-ops` and `magpie-adversarial-review` + (#1368) — declared in `SUBSTRATE_PLUGINS` in `check-family-plugins.py`. + Each inherits the shared manifest metadata, declares no `skills`, exposes + its `tools/` through a symlink whose entry point must resolve, and + declares hook wiring only where it has a hook (agent-guard's + `PreToolUse`). They exist to run the tool from the installed plugin tree: + the code a sandbox exclusion or a hook executes must sit where the agent + calling it cannot rewrite it. A tool shipped this way must resolve + outside the workspace, so it declares no workspace-only `dev` dependency + group (#1357). + - **The same skill is invoked by a different name per install method**, and both are correct: `/magpie-` under the portable snapshot install (where the `magpie-` prefix on the install directory *is* the namespace), and diff --git a/tools/spec-loop/specs/meta-and-quality-tooling.md b/tools/spec-loop/specs/meta-and-quality-tooling.md index 8f3726941..369986367 100644 --- a/tools/spec-loop/specs/meta-and-quality-tooling.md +++ b/tools/spec-loop/specs/meta-and-quality-tooling.md @@ -36,13 +36,24 @@ trustworthy as it grows. - `tools/skill-and-tool-validator/` — validates `SKILL.md` frontmatter (required `name`, `description`, `license`) and tool definitions, internal link integrity, placeholder conventions, license headers on tool Python files, and eval-coverage - (soft check: warns when a skill has no eval suite). CLI: `skill-and-tool-validate`. + (soft check: warns when a skill has no eval suite). A skill's `name:` must + equal the directory its `SKILL.md` actually lives in — the family-plugin + directory name, which is the alias every harness invokes it by (#1361). + CLI: `skill-and-tool-validate`. - `tools/skill-evals/` — harness for measuring skill behaviour. A case whose CLI produced no usable JSON reports ERROR unless something asserts on a synthetic wrap key (`raw_output` / `stderr` / `exit_code`, via `expected.json` or an `assertions.json` `field`) — a wrap nothing addresses compares nothing, and passing it turned an unauthenticated CLI into a green - run. + run. Every fixture read is contained to the eval tree by resolved path: + `magpie-run-evals.sh` runs outside the sandbox, so its wrapper resolves its + target against `tools/skill-evals/evals` and the runner resolves every path + it reads — `report.md`, `step-config.json`'s `skill_md` — rejecting a + symlink or a `..` walk that leads out, while symlinks that stay inside the + tree keep working (#1315). A prose-field verdict the grader drops from a + batched rubric is re-asked once, and only silence that survives the retry + is reported; a verdict the grader actually gave is never re-asked, so a NO + cannot become a YES by asking twice (#1341). - `tools/sandbox-lint/` — lints the sandbox/permissions configuration. - `tools/symlink-lint/` — lints the framework's self-adoption skill symlinks: rejects cyclic symlinks, misdirected relays (canonical/ @@ -81,11 +92,14 @@ trustworthy as it grows. SPDX-License-Identifier header, Validation code block present, filesystem paths in Validation blocks must exist under repo root); the spec-side counterpart to `skill-and-tool-validator`. -- Skills: `write-skill` (author/update a skill), `optimize-skill` - (restructure an existing skill or sweep a set: split oversized - `SKILL.md`, lift project-specific values into placeholders, harden - prompt-injection defences), `list-skills` (live, generated index of - every skill, grouped by family). +- Skills: `write-skill` (author/update a skill; its Step 7 runs + `optimize-skill` on every new skill before it ships, unconditionally), + `optimize-skill` (restructure an existing skill or sweep a set: split + oversized `SKILL.md`, lift project-specific values into placeholders, + harden prompt-injection defences, extract embedded code, and — in a + separate rewrite pass, `rewrite.md` — rewrite prose paragraph by + paragraph with the maintainer writing every word), `list-skills` (live, + generated index of every skill, grouped by family). - `tools/skill-token-count/` measures full local skill files with pinned `tiktoken` / `cl100k_base`, generates the mode-economics table, and checks @@ -123,6 +137,38 @@ trustworthy as it grows. should capture the same minimal fields every time: skill/family, target repo/profile, blocked preflights, false positives, confirmation points, privacy/adapter notes, and proposed spec changes. +- **Skill size has measured budgets.** `optimize-skill` targets 5,000 + tokens for a `SKILL.md` body (pre-flight block excluded) and 200 for + `description` plus `when_to_use`, both set at the catalogue median and + measured with `skill-token-count` before and after every pass; the + always-on frontmatter budget is spent on first, because it is paid in + every session for every skill (#1331, #1332). + Every pass other than the rewrite is behaviour-preserving: moved bytes + are identical bytes, heading level excepted, and a heading that moves + takes its references with it — eval `step-config.json` `step_heading` + / `also_include` entries, anchor links, anything matching on the + string (#1334). + The extract-code pass moves a complete, deterministic program the model + never needs to read out of the body — to `scripts/` beside the skill, a + `tools/` project, or the vetted-ops catalogue when it would otherwise + prompt on every run — byte-identical; placeholder-bearing command + templates are instructions, not programs, and stay inline (#1335, + #1338). + The target's eval suite runs before the first pass and after the last; + a case that flips on an unchanged tree is reported as such to the + maintainer rather than read as a verdict, and a skill with no suite + says so (#1332). + Style rules the rewrite pass learns are proposed as a diff into a + bullets-only region of `optimize-skill/SKILL.md` in this repository — + never headings, which would move its `surface_hash` — and into an + override file for adopters (#1331). +- **CI runs the prek hooks in two shapes.** On a pull request the hooks + see only the PR's own diff (`--from-ref` / `--to-ref`); on a push to + `main` they run `--all-files`. The lychee link check is exempt from the + scoping and runs whole-repo on both events, because its file filter + only decides whether it fires. A green PR check is therefore not a + whole-repo result, which is why `prek run --all-files` before pushing + is a required pre-flight (#1317). - **Eval trust roles stay separate.** Mock tool output in `report.md` enters the user turn as untrusted data. Repository policy read from a trusted revision may enter through a case-level `trusted-context.md`, which the runner appends only to the system prompt. @@ -174,6 +220,11 @@ trustworthy as it grows. 9. `skill-evals` never reports PASS for a case in which nothing was graded: a CLI that emits no JSON, or exits non-zero, errors unless the suite explicitly asserts on the wrapped output. +10. `skill-evals` reads no fixture whose resolved path leaves + `tools/skill-evals/evals`, and re-asks a dropped grader verdict once + without ever re-asking a verdict the grader gave. +11. The validator fails a skill whose `name:` does not match the + directory holding its `SKILL.md`. ## Validation diff --git a/tools/spec-loop/specs/overview.md b/tools/spec-loop/specs/overview.md index f3ab81623..785238d89 100644 --- a/tools/spec-loop/specs/overview.md +++ b/tools/spec-loop/specs/overview.md @@ -72,6 +72,7 @@ Each mode is an independently toggleable set of skills. Maturity mirrors | GitHub Copilot skill runtime | [copilot-runtime.md](copilot-runtime.md) | | Spec-gap staleness verification (proposed) | [spec-gap-staleness.md](spec-gap-staleness.md) | | Vetted command surface (fixed-shape forge ops) | [vetted-command-surface.md](vetted-command-surface.md) | +| Adversarial review by other models (experimental — tool and plugin shipped, consumers not yet wired) | [adversarial-review.md](adversarial-review.md) | ## The non-negotiables every area inherits diff --git a/tools/spec-loop/specs/sandbox-diagnostics.md b/tools/spec-loop/specs/sandbox-diagnostics.md index cd8e1950c..47c7e24c7 100644 --- a/tools/spec-loop/specs/sandbox-diagnostics.md +++ b/tools/spec-loop/specs/sandbox-diagnostics.md @@ -47,7 +47,7 @@ error, and two skills that probe or verify the setup on demand. **Root cause** (which sandbox layer blocks it and why), **Fix** (a settings widening with per-entry rationale, or — for the `gh` entry — an invocation-shape rule, because there is nothing to widen), and - **Notes**. Eight entries today: SSH agent / Yubikey, signed commit + **Notes**. Ten entries today: SSH agent / Yubikey, signed commit failing before any touch (`gpg.format=ssh` key unreadable), signed commit failing with `cannot exec` of the touch-overlay wrapper (`gpg.ssh.program` under the read-denied `~/.claude/scripts/`), @@ -55,26 +55,51 @@ error, and two skills that probe or verify the setup on demand. appearing (the touch overlay's runtime-state directory denied — `/tmp` on a platform with no `$XDG_RUNTIME_DIR`, before the per-user cache-dir fallback), localhost port bind, Docker / Podman - socket, `/tmp` read-only, and `gh` inside the sandbox (TLS - `OSStatus -26276` / `HTTP 401`). + socket, `/tmp` read-only, `gh` inside the sandbox (TLS + `OSStatus -26276` / `HTTP 401`), `prek` or `uv` not found or unable + to write its cache (the home-directory dev-tool paths not granted in + the worktree's `settings.local.json`, #1359), and git hooks silently + skipped for sandboxed commits (whole-user `core.hooksPath` under the + read-denied home, #1364). - `tools/agent-isolation/sandbox-error-hint.sh` — a Claude Code `PostToolUse` hook on the `Bash` matcher. Scans the tool's stdout + stderr for the catalogued symptom strings and, on a match, prints one `[sandbox-hint] …` line to stderr naming the catalog anchor, exiting 1 so the line reaches the model and the user. Tests under `tools/agent-isolation/tests/test_sandbox_error_hint.py`. -- Skill `setup-isolated-setup-doctor` — live, read-only probes, one per - catalog entry (`## The 8 probes`), each reporting ✓ / ✗ / ⊘ / ⚠ with - the command and its output as evidence, and each mapping ✗ to the - matching catalog anchor. The `gh` probe runs `gh` through `sh -c` so +- Skill `setup-isolated-setup-doctor` — live, read-only probes + (`## The 8 probes`), each reporting ✓ / ✗ / ⊘ / ⚠ with the command + and its output as evidence, and each mapping ✗ to the matching + catalog anchor. Each probe is a deterministic, side-effect-free + script, `scripts/probe--.sh`, printing one + `PROBE: → ✓|✗|⊘|⚠ ()` line the skill's + interpretation tables match on; the commands stay out of the + skill body, so they cost no context and can be run and tested on + their own (#1336). Probe 7 checks that `prek` and `uv` run inside + the sandbox and can write `~/.cache`, telling a missing grant (⚠) + from a tool that is not installed (⊘) by reading the worktree's + `settings.local.json`, since a read-denied `~/.local/bin` looks the + same as an absent one (#1362). Probe 8 looks at the global + `core.hooksPath` directory from inside the sandbox: skip when none + is set, fail when the directory or a hook's symlink target is + unreadable, warn when it holds none of the common hooks (#1364). The `gh` probe runs `gh` through `sh -c` so the `excludedCommands` exemption cannot apply to the probe itself, which shows what an un-excluded `gh` does on this machine, then checks that `"gh *"` is configured. - Skill `setup-isolated-setup-verify` — static checks of the installed - configuration (`## The 11 checks`): settings shape, hook wiring, hook - scripts, wrapper, pinned versions, status line, denial canaries, - project-root grant, the vetted-ops split, the touch overlay and - signing key (check 10), and (check 11) the `gh` exclusion. Mirrors the "Via a Claude Code prompt" checklist in + configuration (`## The 12 checks`, numbered 1–13 with the last + optional): settings shape, hook wiring, hook scripts, wrapper, + pinned versions, status line, denial canaries, project-root grant + (check 8 — also the dev-tool paths, whose absence is ⚠ not ✗, and in + whole-user scope the shared hook directory probed from inside the + sandbox, whose unreadability is ✗), the vetted-ops split, the touch + overlay and signing key (check 10), the `gh` exclusion (check 11), + the container gateway (check 12), and the eval-harness exclusion + (check 13, if installed). The conditional checks — 9, 10, 12, 13 and + check 8's whole-user branch — live in `conditional-checks.md`; the + body keeps each one's condition and its **n/a** answer, so a run + that meets none of the conditions reads none of them (#1334). + Mirrors the "Via a Claude Code prompt" checklist in `docs/setup/secure-agent-setup.md`, which is the canonical list. - `docs/setup/secure-agent-setup.md` § Sandbox-error hint hook — the signature → anchor table, install recipe, and trade-offs. @@ -87,7 +112,11 @@ error, and two skills that probe or verify the setup on demand. branch to the hook, a probe to the doctor, and — where the entry relies on a static setting — a check to the verify skill. A catalog entry with no hook branch, or a hook branch with no catalog anchor, - is drift. + is drift. The one sanctioned exception is a failure that prints + nothing: git skips a hook directory it cannot see without a word, so + the git-hooks entry has no symptom string to match and is reached + through the doctor's probe 8 and verify check 8 instead, and says so + in its **Notes**. - **Symptom strings are literal.** Entries quote the exact error text so a grep into the catalog finds them; the hook matches those same strings with anchored, specific regexes. False-positive hints are @@ -184,6 +213,12 @@ PYTHONPATH=tools/skill-evals/src python3 -m skill_evals.runner \ ## Known gaps +- The doctor runs 8 probes over the catalog's 10 entries: the two + touch-overlay entries (`cannot exec` of the wrapper, and the + runtime-state directory) have no live probe. Verify check 10 covers + the wrapper's `allowRead` grant statically; the runtime-state entry + is reached only through the hint hook. + - The invocation-shape rule is measured on macOS / Claude Code 2.1.278 and will change when anthropics/claude-code#95532 is fixed; the catalog entry, the probe text and the verify check all need the same diff --git a/tools/spec-loop/specs/security-issue-lifecycle.md b/tools/spec-loop/specs/security-issue-lifecycle.md index 443c592ab..89d0d041f 100644 --- a/tools/spec-loop/specs/security-issue-lifecycle.md +++ b/tools/spec-loop/specs/security-issue-lifecycle.md @@ -59,6 +59,26 @@ publication, with a human gate and an audit-log entry at every step. - **Reporter PII redacted in-context; reporter *credit* preserved** in the CVE `credits[]` only after the reporter confirms on the thread. - **Audit log** of every applied change (redacted identifiers only). +- **An unresponsive reporter never blocks the team.** When + `security-issue-sync` finds the reporter thread stale — the team's last + outbound message older than + `security_inbox.reporter_response_timeout_days` with no reply since — + its step 2b proposes proceeding with fix and announcement without + further reporter sign-off, per ASF security policy, and never a + follow-up asking the reporter to confirm they are still engaged. The + rule is restated in the step body rather than left to + `signals-to-actions.md`, because it carries policy (#1340). +- **The post-advisory security-pages update is tracked, not performed.** + Both release-manager hand-off comment variants carry a + `Project security pages updated with CVE_ID` checkbox behind a + `security-pages-checklist` marker. The edit is a website change outside + anything sync can write, so it never gates the close-out; instead sync's + closed-tracker pass (Step 1g) proposes a one-line reminder comment, + behind its own `security-pages-reminder` marker and at most once per + tracker, when the advisory has shipped and the box is still unticked. + A re-rendered hand-off comment keeps the release manager's tick, and a + ticked box keeps later runs quiet. The reminder links the project's + pages through the optional `security_pages_url` manifest key (#1355). ## Out of scope diff --git a/tools/spec-loop/specs/vetted-command-surface.md b/tools/spec-loop/specs/vetted-command-surface.md index 77d13aa5b..5ca699dc1 100644 --- a/tools/spec-loop/specs/vetted-command-surface.md +++ b/tools/spec-loop/specs/vetted-command-surface.md @@ -40,6 +40,27 @@ change. Shipping implementation: [`tools/vetted-ops`](../../vetted-ops/README.md). +The HTTP read backend (`http-read`, which implies `writes=False`) carries the +security-data lookups the adapters used to spell as raw `curl`: four OSV.dev +operations (`osv-get-vuln`, `osv-query-package`, `osv-query-commit`, +`osv-query-batch`) and `cve-check-published` against CVE Services. URLs are +built from closed templates over `[endpoints]` bases the policy declares, which +must be `https://`; parameters are validated against `..` and shell +characters; responses stream to stdout, never to a file; and `urllib.request` +honours `HTTPS_PROXY`, so the egress gateway still applies (#1326). The +`tools/osv/` and `tools/cve-org/` recipes, and `security-issue-sync`'s +cve.org check, invoke it through the same `uv run --project +/tools/vetted-ops vetted-op-read …` spelling every permission rule +and sandbox exclusion names — a bare `vetted-op-read` would miss the +allowlist and prompt or run sandboxed (#1339). + +The dispatcher runs from the installed `magpie-vetted-ops` plugin, which ships +`tools/vetted-ops` without the workspace root, so the project must resolve +standalone: it declares no `dev` dependency group, because uv resolves every +group before running and `magpie-dev` resolves only through the root's +`[tool.uv.sources]`. Its tests take the shared toolchain from the root `dev` +group instead (#1357). + ## Scoping — what is real and what is aspiration The dispatcher requires `--caller` and refuses operations outside that caller's