diff --git a/changelog/20260716141500-security-review-lens-5d.md b/changelog/20260716141500-security-review-lens-5d.md new file mode 100644 index 00000000..75e80a72 --- /dev/null +++ b/changelog/20260716141500-security-review-lens-5d.md @@ -0,0 +1,9 @@ +# Add security review lens 5d to setup-aiengineering + +- Added a fourth parallel code-review lens (5d) to the injected verification protocol's gate 5, invoking the harness's built-in security review (Claude Code: `/security-review`). Defaults ON in the Step 4 module menu, deselectable, self-gating on host availability. +- Why: gate 5 covered correctness (5a), general review (5b), and structure (5c), but nothing owned vulnerability classes specifically. Injection, XSS, SSRF, hardcoded secrets, IDOR, auth bypass, unsafe deserialization, and path traversal now have a named lens. +- Triage mirrors 5b: `critical`/`major` auto-apply then re-run gates 1-3, `minor`/`info` get listed for the user. +- Made gate 5 count-agnostic ("every lens below" instead of "all three"), so a deselected 5d needs no conditional rewrite and a future 5e needs no edit here at all. +- Added Step 9b, a suggestion-only nudge for the `security-guidance` plugin. Fires only on Claude Code, only when 5d was selected, and only when the plugin is absent. The plugin is hooks-only (pattern warnings, LLM diff review on stop, agentic review on commit) and is a separate always-on layer, not the source of `/security-review`. The skill never installs it. +- Bumped baseline **Skill version v3 to v4** and added the two matching checklist rows, so re-runs offer both concerns to repos already stamped v3. +- No new dependency. `/security-review` is a Claude Code built-in and needs no plugin. diff --git a/skills/setup-aiengineering/SKILL.md b/skills/setup-aiengineering/SKILL.md index 6b8a2a18..11385511 100644 --- a/skills/setup-aiengineering/SKILL.md +++ b/skills/setup-aiengineering/SKILL.md @@ -19,6 +19,7 @@ TypeScript app, a Python service, and a Docker-config repo each get a correct, w | Module | Delivery | |--------|----------| | Verification protocol (lint → typecheck → test → coverage ≥90% → code review → docs alignment) | inject (`references/verification-protocol.md`; no-framework branch → `references/test-setup.md`) | +| Security review lens (5d) — harness security review inside verification gate 5 | inject (rides with `references/verification-protocol.md`) | | Git policy | inject (`references/git-policy.md`) | | File organization | inject (`references/file-organization.md`) | | PRD gate (require a PRD before substantial features) — opt-in | inject (`references/prd-gate.md`) | @@ -83,7 +84,7 @@ Detect greenfield vs working repo (heuristic in `references/backfill-guide.md`). ### Step 4: Module menu -Present the eight modules and let the user pick per project. Seven default to selected — +Present the nine modules and let the user pick per project. Eight default to selected — deselect to opt out. The **PRD gate is opt-in — default it OFF**, and select it only if the user wants PRD-first enforcement. If the repo has no build tooling, flag the verification module as degraded and let them keep or skip it. @@ -97,6 +98,12 @@ Flag the **worktree auto-bootstrap** module as Claude-Code-only (it scaffolds a `.claude/settings.json` hook and a `.worktreeinclude`); on any other host it will be skipped in Step 7 regardless of selection. +The **security review lens (5d)** defaults to **ON** — deselect to opt out. It has no standalone +block: it rides inside the verification module's gate 5, so selecting it with verification +deselected is a no-op. It self-gates on availability — if the host provides no security-review +capability, the injected lens tells the user and labels itself `skipped (security review +unavailable)`. Selecting it is also what enables the Step 9b plugin offer. + ### Step 5: Inject the policy modules For each chosen inject module (verification, git policy, file organization, PRD gate): @@ -121,9 +128,13 @@ For each chosen inject module (verification, git policy, file organization, PRD + `{{COVERAGE_THRESHOLD}}`. - **Config/no-source repo** → drop the test + coverage gates and renumber, exactly as the existing no-tool degradation above already says. -3. Append the `##` section to the target file. If its heading already exists, **ask** before +3. **Security review lens (5d).** Include the 5d bullet in the injected verification block only when + the security review module was selected in Step 4; when it was not, omit that bullet. Gate 5's + wording is count-agnostic, so nothing needs renumbering. 5d invokes the harness's own + security-review capability — it does not depend on the `security-guidance` plugin (see Step 9b). +4. Append the `##` section to the target file. If its heading already exists, **ask** before replacing — never silently duplicate. -4. **Provenance note (once, versioned).** Above the first injected policy `##` section, add a single +5. **Provenance note (once, versioned).** Above the first injected policy `##` section, add a single visible italic line naming this skill, stamping the current **Skill version** from `references/baseline-checklist.md`, and listing the sections it generated, e.g. `_The Verification, Git Policy, and File Organization sections below were generated by the `setup-aiengineering` skill @@ -200,6 +211,7 @@ Confirm in one short message: added or refreshed. - Policy modules injected (with which gates were dropped for missing tools), and that the detected lint/typecheck/test commands were confirmed with the user. +- Security review lens (5d): injected, or skipped (user opted out). - Coverage gate: whether it was wired (with the chosen `{{COVERAGE_THRESHOLD}}` and `{{COVERAGE_CMD}}`), or that a test framework was scaffolded via the `references/test-setup.md` prompt, or that it was N/A (config/no-source repo). @@ -245,6 +257,26 @@ one-line suggestion: the user can set up the Claude GitHub App — `@claude` on automatic PR review — by typing `/install-github-app` (needs repo admin + the `gh` CLI). It is a suggestion only; the user runs it. +### Step 9b: Offer the security-guidance plugin (Claude Code only) + +Runs last, alongside Step 9, as a closing suggestion. Fires only when ALL of these hold — +otherwise skip silently: + +1. **Host is Claude Code.** `printenv CLAUDECODE` returns `1`. Unset/empty (Copilot CLI, Gemini + CLI, Cursor, any other host) → skip. +2. **The security review lens was selected** in Step 4. If the user opted out, skip — do not pitch + a security plugin to someone who just declined security review. +3. **Not already installed.** Read the plugins registry + (`${CLAUDE_CONFIG_DIR:-~/.claude}/plugins/installed_plugins.json`) and look for a + `security-guidance@` key. Present → skip. + +When all pass, do NOT install anything — the skill never installs. End with a one-line suggestion: +the user can add always-on security hooks (pattern warnings on `Edit`/`Write`, an LLM diff review +when a turn ends, and an agentic reviewer on `git commit`) by running +`/plugin install security-guidance@claude-plugins-official`. It complements lens 5d — 5d is +on-demand and per-change, the plugin is automatic and machine-wide. Suggestion only; the user runs +it. + ## Rules - Inject into the project's existing agent file (first match: `AGENTS.md` → root `CLAUDE.md` → @@ -286,6 +318,13 @@ suggestion only; the user runs it. - The GitHub-App offer (Step 9) is Claude-Code-only (gated on `CLAUDECODE=1`) and GitHub-only. It suggests the built-in `/install-github-app` — never an action the skill performs. Skip silently on non-Claude-Code hosts or non-GitHub repos; note-and-skip if the workflows exist. +- The security review lens (5d) rides inside verification gate 5, defaults ON, and self-gates on + the host's security-review capability; when absent the injected lens labels itself `skipped + (security review unavailable)` — never skip silently. It invokes the harness built-in (Claude + Code: `/security-review`) and does not require the `security-guidance` plugin. +- The security-guidance plugin offer (Step 9b) is Claude-Code-only, fires only when the security + review lens was selected and the plugin is not already installed, and is a suggestion only — + never install it. ## References diff --git a/skills/setup-aiengineering/references/baseline-checklist.md b/skills/setup-aiengineering/references/baseline-checklist.md index 76021ead..250e47a9 100644 --- a/skills/setup-aiengineering/references/baseline-checklist.md +++ b/skills/setup-aiengineering/references/baseline-checklist.md @@ -4,7 +4,7 @@ The canonical list of AI-engineering baseline concerns this skill installs. It i Step 8b coverage self-audit, and it defines what each skill version contains, so re-run upgrade mode (Step 1) can tell an older setup what it is missing. -**Skill version: v3** +**Skill version: v4** Bump this number whenever a concern is added below (see the maintainer loop at the bottom). The version stamped into a repo's provenance note is compared against this number on every re-run. @@ -24,6 +24,7 @@ first shipped in. `Since` lets re-run mode compute the delta for a repo stamped | Code review lens 5a — harness-native | inject | 5 | v1 | | Code review lens 5b — CodeRabbit CLI | inject | 5 | v1 | | Code review lens 5c — nuclear structural (optional) | inject | 5 | v1 | +| Code review lens 5d — security review (default on, deselectable) | inject | 5 | v4 | | Docs & instructions alignment gate | inject | 5 | v1 | | Git policy | inject | 5 | v1 | | File organization | inject | 5 | v1 | @@ -36,6 +37,7 @@ first shipped in. `Since` lets re-run mode compute the delta for a repo stamped | Worktree auto-bootstrap (hook + `.worktreeinclude` + `.mcp.json` carry) | scaffold | 7 | v1 | | MCP-config reminder (when no `.mcp.json`) | inject | 7b | v1 | | GitHub App offer (Claude Code + GitHub only) | suggest | 9 | v1 | +| Security-guidance plugin offer (Claude Code only) | suggest | 9b | v4 | ## How the self-audit uses this (Step 8b) diff --git a/skills/setup-aiengineering/references/verification-protocol.md b/skills/setup-aiengineering/references/verification-protocol.md index 87279ea3..8e00ba5e 100644 --- a/skills/setup-aiengineering/references/verification-protocol.md +++ b/skills/setup-aiengineering/references/verification-protocol.md @@ -52,7 +52,7 @@ says otherwise. New tests for new code are necessary but not sufficient either: if the run reports the overall percentage under `{{COVERAGE_THRESHOLD}}%`, the gate fails and more tests are needed before proceeding. -5. **Code review** — run **all three** in parallel on this session's changes: +5. **Code review** — run **every lens below** in parallel on this session's changes: - **5a. Harness-native code review** — invoke your harness's `code-review` agent (Claude Code: `Task` tool with `subagent_type: "code-review"`; Copilot CLI: the `code-review` skill). Cover bugs, security, logic errors, race conditions, unhandled edge cases, and the project's own @@ -73,9 +73,18 @@ says otherwise. cover those). Surface its findings for the user; never auto-apply. If the skill isn't available, **tell the user and skip 5c** — label it `skipped (nuclear review unavailable)`; never skip silently. - - **Merge** — wait for all three (5a, 5b, 5c) to finish — a `skipped` 5b or 5c still counts as - done — then deduplicate findings across them and present one combined "Code review findings" - section. + - **5d. Security review** — if your harness provides a security-review capability (Claude Code: + the built-in `/security-review` skill; Copilot CLI: its built-in security review), spawn a + subagent that runs it against `{{DEFAULT_BRANCH}}...HEAD`. Vulnerability classes only — + injection, XSS, SSRF, hardcoded secrets, IDOR, auth bypass, unsafe deserialization, and path + traversal. Structural and correctness concerns belong to 5a and 5c, not here. + - **Triage** — `critical`/`major` → auto-apply the fix, then re-run gates 1–3. + `minor`/`trivial`/`info` → do **not** auto-apply; list them for the user (file:line + + suggested fix). + - If your harness provides no security-review capability, **tell the user and skip 5d** — + label it `skipped (security review unavailable)`; never skip silently. + - **Merge** — wait for **every lens** to finish — a `skipped` lens still counts as done — then + deduplicate findings across them and present one combined "Code review findings" section. 6. **Docs & instructions alignment** — before marking the task done, check whether this session's changes made any documentation stale: - **Project docs** (`README.md`, `docs/`, `ARCHITECTURE.md`, other human-facing docs) — stale @@ -96,7 +105,9 @@ to gate on. If the project has no lint/typecheck/test tooling, keep only gates 5 & instructions alignment; renumbered 1–2) and append: *"No automated lint/typecheck/test gates were detected for this repo. Add them here when build tooling lands."* If a source repo has a test framework but no coverage tooling, the skill wires `{{COVERAGE_THRESHOLD}}` once coverage tooling is -chosen — see `references/test-setup.md`. +chosen — see `references/test-setup.md`. Lens 5d ships only when the security review module is +selected in Step 4; when it is not, omit the 5d bullet — gate 5's wording is count-agnostic, so no +renumbering or count edit is needed. **Version / drift.** This block's version is recorded by the versioned provenance note the skill stamps (SKILL.md Step 5.4), not by a marker inside the block. On re-run upgrade mode (SKILL.md Step