diff --git a/plugins/magpie-setup/skills/setup/config.md b/plugins/magpie-setup/skills/setup/config.md index 82ce7e630..b520156c6 100644 --- a/plugins/magpie-setup/skills/setup/config.md +++ b/plugins/magpie-setup/skills/setup/config.md @@ -270,6 +270,22 @@ When a vetted-ops policy exists, show the `[tracker]` table it needs — and write it only on the user's confirmation; the policy is a security control, not configuration this step may change unasked. Grant the security callers `issue-create`, `issue-search`, `issue-list-label`, `issue-reopen`, `issue-body`, `issue-attach`, `tracker-doctor`, `tracker-pending` and `tracker-publish` in the same diff. +For the security issue-import workflows, also check the policy's `[callers]` +table. When those workflows are configured, propose exactly these grants: + +| Caller | Required operations | +|---|---| +| `security-issue-import` | `rollup-append` | +| `security-issue-import-from-md` | `rollup-append` | +| `security-issue-import-from-pr` | `pr-view-with-body`, `rollup-append` | + +These are separate caller identities. Do not substitute one for another or +grant `body-field-set` to an import caller: none of these workflows invokes it. +Show the proposed policy diff and apply it **only on explicit user confirmation**. +Merge missing operations into an existing caller array; never duplicate a TOML +key, remove existing grants, or change unrelated callers. If no vetted-ops +policy exists, do not create one implicitly. + ## Step 3b — Record what this run reconciled For every skill in scope (Step 1) whose `requires_config:` set now fully diff --git a/plugins/magpie-setup/skills/setup/reconcile.md b/plugins/magpie-setup/skills/setup/reconcile.md index a2ef8cb57..8c4d0cbdf 100644 --- a/plugins/magpie-setup/skills/setup/reconcile.md +++ b/plugins/magpie-setup/skills/setup/reconcile.md @@ -150,6 +150,29 @@ baseline required. name>` requires ``, which is not configured"* — propose `/magpie-setup:setup config ` for it. +3a. **Import caller policy grants.** When an existing vetted-ops policy + is present and a security issue-import skill is in scope (step 1), + read its `[callers]` table. Compare the applicable caller against + the minimum permissions used by the skill: + + Use the exact caller-to-operation mapping in [`config.md` § Step 3a](config.md) for the three security import workflows. Do not infer or add other permissions. + + A missing caller or missing required operation is a finding. Name the + caller, the missing operations, and the existing policy path. Propose + adding only those grants; do not grant `body-field-set`, which these + import workflows do not use. When a caller already exists, merge + missing operations into its existing TOML array rather than creating + a duplicate key. Preserve comments, unrelated callers and existing + permissions. + + This is a security-control change, not an automatic repair. Show the + exact proposed diff and obtain item-by-item confirmation before + writing anything. A declined change leaves the policy unchanged and + the finding unresolved; do not report or stamp that configuration + as clean. If the policy does not exist, mark this check not applicable: + do not create a vetted-ops policy during reconciliation. If the + policy cannot be read or parsed, report it as unchecked, not clean. + 4. **Sandboxed sessions cover what they can reach.** Resolving a skill's anchors (check 2) needs that skill's `SKILL.md` **and its sibling detail files**. On a pinned-snapshot install they sit inside @@ -182,8 +205,8 @@ baseline required. ## Step 1 — Present the findings -List every finding from checks 2–3 as a numbered proposal, one item per -override or `requires_config` gap: +List every finding from checks 2–3a as a numbered proposal, one item per +override, `requires_config` gap, or missing import caller policy grant: ```text 1. .apache-magpie-overrides/issue-triage.md anchors to @@ -195,8 +218,8 @@ override or `requires_config` gap: → /magpie-setup:setup config pr-management-code-review ``` -No findings → say so plainly (*"every configured skill's anchors and -`requires_config` entries resolve"*), name any `unchecked` skills from +No findings → say so plainly (*"every configured skill's anchors, +`requires_config` entries and applicable vetted-ops caller grants resolve"*), name any `unchecked` skills from check 4, and go straight to Step 2 with nothing to confirm — writing the stamp needs no confirmation when nothing is changing. @@ -209,6 +232,13 @@ maintainer's judgement call, not pattern-matching. A `requires_config` gap can be closed mechanically by chaining into `/magpie-setup:setup config `; offer it per item. +For an import caller policy finding (check 3a), show the minimum-grant +TOML diff before asking. Apply it only after confirmation. A refusal does +not authorize a policy edit or a clean reconciliation stamp for that +unresolved configuration. Do not treat resolved `requires_config` files +alone as proof of a clean skill when its required caller grants are missing. +Preserve existing policy content not covered by the confirmed change. + Both `acknowledged` writes below fire **on decline**, not on show — unlike the pre-flight block's own per-skill check ([`tools/dev/preflight-block.md`](../../../../tools/dev/preflight-block.md) diff --git a/tools/skill-evals/README.md b/tools/skill-evals/README.md index fabbfb775..58605390f 100644 --- a/tools/skill-evals/README.md +++ b/tools/skill-evals/README.md @@ -11,7 +11,7 @@ Behavioral eval harness for Apache Magpie skills. Each eval suite tests a skill Suites are currently implemented for: -- **setup** — 141 cases across 30 steps (step-verify-drift, step-overrides-surface, step-override-bypass, step-m3-baseline-pick, step-m4-install-gates, step-m5-no-repo-offer, step-m5-companion-offer, step-upgrade-companion-offer, step-adopt-settings-merge, verify-default-set, uninstall-default-set, lock-marketplace-parse, adopt-write-floor, setup-prefill-from-floor, preflight-floor, upgrade-adoption-split, verify-floor, adopt-review-process, adopt-security-list, step-reconcile, step-verify, step-config-stamp, step-config-adversarial, step-config-mail, step-adopt-stamp, step-upgrade-stamp, step-delegate-propose, config-vcs-backend, config-local-custom-parser) +- **setup** — 142 cases across 30 steps (step-verify-drift, step-overrides-surface, step-override-bypass, step-m3-baseline-pick, step-m4-install-gates, step-m5-no-repo-offer, step-m5-companion-offer, step-upgrade-companion-offer, step-adopt-settings-merge, verify-default-set, uninstall-default-set, lock-marketplace-parse, adopt-write-floor, setup-prefill-from-floor, preflight-floor, upgrade-adoption-split, verify-floor, adopt-review-process, adopt-security-list, step-reconcile, step-verify, step-config-stamp, step-config-adversarial, step-config-mail, step-adopt-stamp, step-upgrade-stamp, step-delegate-propose, config-vcs-backend, config-local-custom-parser) - **setup-isolated-setup-install** — 19 cases across 5 steps (runtime-routing, step-snapshot-drift, step-scope-confirm, step-hardware-key, step-roborev-exclusion) - **setup-privacy-llm** — 6 cases across 2 steps (step-1-resolve, step-4-gate) - **setup-shared-config-sync** — 12 cases across 2 steps (step-3-decide-action, step-5-draft-commit) diff --git a/tools/skill-evals/evals/setup/README.md b/tools/skill-evals/evals/setup/README.md index fe5336d5e..b30fed9d8 100644 --- a/tools/skill-evals/evals/setup/README.md +++ b/tools/skill-evals/evals/setup/README.md @@ -5,7 +5,7 @@ Behavioral evals for the `setup` skill. -## Suites (141 cases total) +## Suites (142 cases total) | Suite | Step | Cases | What it covers | |---|---|---|---| @@ -28,7 +28,7 @@ Behavioral evals for the `setup` skill. | preflight-floor | preflight-block.md § Pre-flight | 21 | at floor (silent), below floor (update), plugin missing (install), foreign marketplace (ask first, run nothing), no `claude` CLI (print), unadopted (propose setup), `apache/magpie` in an alarming context (update anyway, unasked), unreadable `claude plugin list --json` (silent, treated as unknown not absent), first Magpie run (show the general welcome guide, record it, carry on), first utilities run (show the utilities welcome guide, record it, carry on), first mentoring run (show the mentoring welcome guide, record it, carry on), first pairing run (show the pairing welcome guide, record it, carry on), first issue run (show the issue welcome guide, record it, carry on), first pr-management run (show the pr-management welcome guide, record it, carry on), first contributor-growth run (show the contributor-growth welcome guide, record it, carry on), below floor on Codex (marketplace upgrade), below floor on Gemini (update the extension once), at floor on Gemini (silent), first repo-health run (show the repo-health welcome guide, record it, carry on), first setup run (show the setup welcome guide, record it, carry on), first release-management run (show the release-management welcome guide, record it, carry on) | | upgrade-adoption-split | upgrade.md § Step 0b | 4 | not adopted (nothing staged), adopted (floor raised and staged), already ahead (floor unchanged), snapshot method (falls through) | | verify-floor | verify.md § Adoption floor | 4 | no lock (not a fault), ahead of floor with extra plugins (not a fault), a shortfall (a fault), a floor plugin the marketplace no longer ships (a fault, not installed around) | -| step-reconcile | reconcile.md § The sweep | 3 | a clean sweep on a pinned-snapshot install (anchor present, config resolved — stamp written, nothing proposed), a renamed step heading stranding an override's anchor (one re-anchor proposal named), a marketplace install whose plugin cache is sandbox-denied (anchor resolution left `unchecked`, config resolution still completes) | +| step-reconcile | reconcile.md § The sweep | 4 | a clean sweep on a pinned-snapshot install (anchor present, config resolved — stamp written, nothing proposed), a renamed step heading stranding an override's anchor (one re-anchor proposal named), a marketplace install whose plugin cache is sandbox-denied (anchor resolution left `unchecked`, config resolution still completes), a configured security import policy with missing callers and operations (propose minimum grants, do not stamp clean) | | step-verify | verify.md § 12. Latest available plugin version | 2 | a dev-to-dev delta where the marketplace clone is one dev build ahead of an installed plugin (`update_available` carries the newer dev version — pins decision 7: nothing strips `.devN`), and a sandbox-denied marketplace clone (`update_available: null` **and** `unchecked: ["latest-version"]`, distinguishing *nothing newer* from *could not look*) | | step-config-stamp | config.md § Step 3b | 6 | `config` on an already-adopted project where Step 3 wrote the skill's last missing file this run, which records `acknowledged.skills` locally and never touches the committed lock; `config` on an unadopted one, whose entries land in the git-directory home's (`/apache-magpie/reconciled.json`) `skills` map with `version`/`at` regardless of whether this run touched a file; the R2 skip on a repo where nothing has ever been configured or adopted (`write_stamp: false`); and an already-adopted project where the one skill in scope was already fully configured *before* this run, so nothing is recorded even though the lock exists (`write_stamp: false` for a different reason — the narrowed acknowledged-write trigger); a linked worktree of an adopted project with no `.apache-magpie-local/` of its own, which records into the main checkout's (`write_location: main-checkout`); and a linked worktree of an unadopted clone, which records into the shared git-directory home (`write_location: git-dir`) | | config-vcs-backend | config.md § Step 3g | 5 | an SVN working copy as the `` clone beside a Git tracker repo (propose `vcs_backend: svn`, the URLs and `trunk`, written only on confirmation); a plain Git project (write `git`, ask nothing); a git-svn mirror on GitHub (propose svn with the repository root left `TODO`, say an `svn checkout` is needed, never run it); a checkout outside `trunk`/`branches`/`tags` (no default branch proposed); and an SVN checkout whose URLs could not be read (propose `svn`, leave both URLs `TODO` rather than guess) | diff --git a/tools/skill-evals/evals/setup/step-reconcile/fixtures/case-4-import-policy-grants/expected.json b/tools/skill-evals/evals/setup/step-reconcile/fixtures/case-4-import-policy-grants/expected.json new file mode 100644 index 000000000..49f0aff9b --- /dev/null +++ b/tools/skill-evals/evals/setup/step-reconcile/fixtures/case-4-import-policy-grants/expected.json @@ -0,0 +1,24 @@ +{ + "proposals": [ + { + "kind": "policy", + "skill": "security-issue-import", + "override_file": null, + "detail": "Missing caller security-issue-import in .apache-magpie-overrides/tools/vetted-ops/config.toml; propose rollup-append only." + }, + { + "kind": "policy", + "skill": "security-issue-import-from-md", + "override_file": null, + "detail": "Existing caller security-issue-import-from-md lacks rollup-append; preserve its issue-view grant." + }, + { + "kind": "policy", + "skill": "security-issue-import-from-pr", + "override_file": null, + "detail": "Existing caller security-issue-import-from-pr lacks pr-view-with-body; preserve its rollup-append grant." + } + ], + "unchecked": [], + "stamp_written": false +} diff --git a/tools/skill-evals/evals/setup/step-reconcile/fixtures/case-4-import-policy-grants/report.md b/tools/skill-evals/evals/setup/step-reconcile/fixtures/case-4-import-policy-grants/report.md new file mode 100644 index 000000000..0672ac509 --- /dev/null +++ b/tools/skill-evals/evals/setup/step-reconcile/fixtures/case-4-import-policy-grants/report.md @@ -0,0 +1,31 @@ + + +Install method: `git-branch` (pinned snapshot). + +Scope: 3 configured security import skills, no override files. +All three skills' `requires_config:` entries resolve from the personal +layer or committed overrides. All their SKILL.md files are readable. +No anchor or required-configuration findings exist. + +Existing vetted-ops policy: +`.apache-magpie-overrides/tools/vetted-ops/config.toml` +The policy is valid TOML and has this `[callers]` section: + +```toml +[callers] +"security-issue-import-from-md" = ["issue-view"] +"security-issue-import-from-pr" = ["rollup-append"] +"security-issue-sync" = ["issue-view", "rollup-append"] +``` + +The following workflows are configured and in scope: + +- `security-issue-import` requires `rollup-append`. +- `security-issue-import-from-md` requires `rollup-append`. +- `security-issue-import-from-pr` requires `pr-view-with-body` + and `rollup-append`. + +No one has confirmed changes to the vetted-ops policy. +Do not modify the policy or write a clean reconciliation stamp. +Return the missing caller/grant proposals only. diff --git a/tools/skill-evals/evals/setup/step-reconcile/fixtures/output-spec.md b/tools/skill-evals/evals/setup/step-reconcile/fixtures/output-spec.md index 8e6d7e623..6bfb5a1db 100644 --- a/tools/skill-evals/evals/setup/step-reconcile/fixtures/output-spec.md +++ b/tools/skill-evals/evals/setup/step-reconcile/fixtures/output-spec.md @@ -9,27 +9,32 @@ Return ONLY valid JSON with this structure: { "proposals": [ { - "kind": "reanchor" | "config", + "kind": "reanchor" | "config" | "policy", "skill": "", - "override_file": "", + "override_file": "", "detail": "" } ], - "unchecked": ["anchor-resolution" | "requires_config"], + "unchecked": ["anchor-resolution" | "requires_config" | "policy-grants"], "stamp_written": true | false } ``` -- `proposals` — one entry per finding from the two checks. `kind: "reanchor"` +- `proposals` — one entry per finding from the reconciliation checks. `kind: "reanchor"` when an override's referenced anchor no longer resolves in the target skill's current `SKILL.md`; `kind: "config"` when a `requires_config` - entry does not resolve through the lookup chain. Empty when both checks - pass cleanly for everything in scope. + entry does not resolve through the lookup chain; `kind: "policy"` when + an applicable import caller or required operation is absent from an + existing vetted-ops policy. Use the caller name as `skill` and name + the exact missing operations in `detail`. Empty when all applicable + checks pass cleanly for everything in scope. - `unchecked` — which check(s) could not be completed for at least one skill in scope, because that skill's `SKILL.md` was not readable this session (a marketplace install's plugin cache, denied under the sandbox). `requires_config` resolution needs only files already in the - repository, so it is never unchecked. Empty when nothing was skipped. + repository, so it is never unchecked. Use `policy-grants` when an + existing vetted-ops policy cannot be read or parsed. A missing + policy is not applicable, not unchecked. Empty when nothing was skipped. - `stamp_written` is `true` only when `proposals` and `unchecked` are both empty — nothing changed, so the stamp is written immediately with no confirmation needed. Any finding, or anything left unchecked, means diff --git a/tools/skill-evals/evals/setup/step-reconcile/fixtures/user-prompt-template.md b/tools/skill-evals/evals/setup/step-reconcile/fixtures/user-prompt-template.md index 57c8c3c8c..0b237e5e4 100644 --- a/tools/skill-evals/evals/setup/step-reconcile/fixtures/user-prompt-template.md +++ b/tools/skill-evals/evals/setup/step-reconcile/fixtures/user-prompt-template.md @@ -5,4 +5,4 @@ {report} -Apply the sweep's two checks and return JSON only. +Apply the reconciliation sweep checks, including applicable vetted-ops import caller grants, and return JSON only.