feat: claude-security-review reusable workflow — dedicated advisory security pass

[kyle-sexton]

Summary

Adds the dedicated LLM security-review lane the autonomy pipeline lacks (melodic-software/claude-code-plugins#509; the security half of #696 — Boris's step-2 posture: "Automated code review and security review are on by default"):

• claude-security-review.yml (reusable): security-only review of the PR's changed files — logic flaws, authorization gaps, injection surfaces, token/secret handling, dangerous workflow patterns (pull_request_target, github-context script injection), permission-widening config changes, supply-chain pin loosening — reported as a PR review with CRITICAL/IMPORTANT/SUGGESTION severity and a confidence axis. No style/quality commentary.
• Security model copied verbatim from claude-review.yml: pull_request-only + reject-privileged-triggers tripwire, fork PRs skipped by design, single named CLAUDE_CODE_OAUTH_TOKEN, contents: read + pull-requests: write + id-token: write, display_report hardcoded off, freshness/superseded-head guard, per-head concurrency, strip-persisted-credentials, identical SHA pins.
• Advisory with a recorded promotion trigger: flip to blocking on CRITICAL findings once the lane's precision is proven over a sustained window — an earned promotion (trust-before-scale), mirroring the guardrail-matrix verification-promotion discipline.
• Caller-owned path filtering (documented, with a canonical caller snippet): a security pass on every prose PR is noise; the path filter over security-sensitive surfaces is the default-on discipline's scope control. Dogfood self-caller filters to this repo's workflows/actions/shell surfaces.
• Validations run locally: actionlint (embedded shellcheck), check-jsonschema vendor.github-workflows, markdownlint — all green.

Consumer wiring (claude-code-plugins caller + the default-on decision record) follows once this lands and a release SHA exists to pin.

Related

• autonomy pipeline: no dedicated security-review pass — automated code review only claude-code-plugins#509 / #696 (tracking items — closed when the consumer wiring lands)
• claude-review.yml (the sibling lane and security-model precedent)

No linked issue in this repository.

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 3m 11s —— View job

---

Security Review — PR #160

• Gather context and read changed files
• Review claude-security-review.yml (reusable workflow)
• Review claude-security-review-self.yml (self-caller)
• Review README additions
• Verify CLAUDE.md security ground rules compliance
• Post findings

---

Summary

This PR adds a well-constructed dedicated security-review lane that faithfully copies the security model from claude-review.yml. The ground rules from CLAUDE.md are all satisfied. I found two minor defensive-depth gaps and one design note worth documenting, all non-blocking.

---

CLAUDE.md Ground Rule Compliance

Rule	Status	Notes
pull_request only; tripwire on pull_request_target/workflow_run	✅	reject-privileged-triggers step hard-fails those two events
Fork PRs skipped by design	✅	Both failure/success comment steps gate on head.repo.full_name == github.repository
SHA-pinned both layers	✅	actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 and anthropics/claude-code-action@12531344451323133b0493233c759991ac61da12 # v1.0.174 — identical SHAs to the sibling
Least privilege	✅	Workflow-level contents: read; job-level adds pull-requests: write + id-token: write
Named secret only, no secrets: inherit	✅	Self-caller passes CLAUDE_CODE_OAUTH_TOKEN explicitly
display_report off	✅	Hardcoded false — actually stricter than claude-review.yml, which exposes this as a caller-configurable input
No untrusted checkout before the action	✅	actions/checkout with filter: blob:none only; no PR head ref injection
Artipacked mitigation (strip persisted credentials)	✅	Final git config --unset-all step runs if: always()

---

Findings

SUGGESTION — show_full_output not explicitly hardcoded off

CLAUDE.md says "Keep display_report and show_full_output off (defaults)". The workflow hardcodes display_report: false, which is correct (and stricter than the sibling). show_full_output is not set at all. The action's current default is false, so this is safe today. But since display_report is already explicitly locked, adding show_full_output: false alongside it would be consistent and defense-in-depth against the action's default ever changing.

Confidence: High (the gap is real; the risk is theoretical given the current action default).

Relevant line — both display_report and show_full_output should appear together.

SUGGESTION — review_detail written to GITHUB_OUTPUT via single-line form

At line 289:

echo "review_detail=$detail" >> "$GITHUB_OUTPUT"

detail comes from jq -c which produces compact (single-line) JSON, and the projected fields are all primitive types (subtype: string, is_error: bool, num_turns/duration_ms/total_cost_usd: number), so a newline in detail is implausible in practice. However, CLAUDE.md notes that display_report/show_full_output are off precisely because model output can be unsafe — the subtype field is a string from the SDK schema and could theoretically be manipulated by a hostile action build. The sibling compose-args step explicitly documents this risk and uses the heredoc form. The same defense applies here:

delimiter="gha_${RANDOM}${RANDOM}_EOF"
{
  echo "review_detail<<$delimiter"
  printf '%s\n' "$detail"
  echo "$delimiter"
} >> "$GITHUB_OUTPUT"

Confidence: Medium (the jq projection makes actual exploitation implausible; the fix is purely defensive).

Design note — dual concurrency groups in self-caller

The self-caller has a workflow-level concurrency group (per PR number, no head SHA). The reusable workflow's comments explicitly warn against workflow-level groups because they cancel before the actor-skip if evaluates, which could let a bot's push cancel a human's in-progress review.

In the self-caller this is benign — dependabot[bot] doesn't push code to ci-workflows and the pull_request trigger only fires on same-repo events — so the double-cancel scenario doesn't arise. But external callers who follow the self-caller as a template and also have Dependabot-authored pushes should be aware of this interaction. Worth a comment in the self-caller pointing back to the reusable's concurrency rationale, or removing the outer group and letting the job-level group do the full job (which is what the reusable's design intends).

This is a documentation/template-clarity gap, not a security issue.

---

What's Well Done

• display_report hardcoded off (non-overridable) is actually a security improvement over claude-review.yml, which allows callers to enable it. Locking it at the lane level is the right call for a public security-review lane.
• No compose-args step needed — by passing inputs.claude-args directly as an action with: input (not via a shell output variable), the multiline-value GITHUB_OUTPUT corruption risk the sibling's compose-args step guards against simply doesn't arise here.
• Freshness guard, per-head concurrency, and marker-comment lifecycle are all correctly ported from the sibling with no regressions.
• Path-triggered by design is a sound architecture — a security pass on prose-only PRs would be noise, and the caller-owned path filter is the right abstraction boundary.
• Self-caller dogfoods correctly using ./.github/workflows/claude-security-review.yml (local ref) and passes only the one named secret.
• README addition is accurate, well-structured, and includes the canonical caller snippet.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0e2c0ca4ea

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".