-
Notifications
You must be signed in to change notification settings - Fork 0
feat(ci): default-on security review — path-filtered caller + posture ADR #722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
3feef14
feat(ci): default-on security review — path-filtered caller + posture…
kyle-sexton a14450a
fix(ci): cover MCP-server source and compiled JS in the security-revi…
kyle-sexton a845663
fix(ci): include MCP launch configurations in the security-review pat…
kyle-sexton b2277e5
fix(ci): include plugin bin wrappers in the security-review path filter
kyle-sexton 1066c43
fix(ci): cover dependency and plugin manifests in the security-review…
kyle-sexton 28cffd3
fix(ci): treat instruction assets and the marketplace manifest as sec…
kyle-sexton 6e4066c
docs(adr): frame ADR 0002 as the interim posture pending the #509 enf…
kyle-sexton 0465acc
fix(ci): include root agent-instruction files in the security-review …
kyle-sexton File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| name: claude-security-review | ||
|
|
||
| # Dedicated LLM security-review pass via the ci-workflows reusable workflow — the | ||
| # security sibling of claude-review.yml, ADR 0002's default-on advisory posture. | ||
| # The caller owns the triggers, the GITHUB_TOKEN grant, and the PATH FILTER: a | ||
| # security pass on every prose PR is noise, so this filter scopes default-on to | ||
| # the repo's security-sensitive surfaces. Fork PRs receive no secrets and a | ||
| # read-only token, so they are simply not reviewed by design. Requires | ||
| # `claude-code-plugins` in the CLAUDE_CODE_OAUTH_TOKEN org secret's | ||
| # selected-repositories scope. Public repo: hosted default runner. | ||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, ready_for_review, reopened] | ||
| paths: | ||
|
kyle-sexton marked this conversation as resolved.
kyle-sexton marked this conversation as resolved.
|
||
| - ".github/**" | ||
| - ".claude/**" | ||
| - "scripts/**" | ||
| - "plugins/*/hooks/**" | ||
| - "plugins/*/bin/**" | ||
| - "plugins/*/tools/**" | ||
| - "plugins/*/skills/**" | ||
| - "plugins/*/agents/**" | ||
| - "plugins/*/commands/**" | ||
| - ".claude-plugin/**" | ||
| - "REVIEW.md" | ||
| - "CLAUDE.md" | ||
| - "AGENTS.md" | ||
| - "plugins/*/src/**" | ||
| - "plugins/*/dist/**" | ||
|
kyle-sexton marked this conversation as resolved.
|
||
| - "**/*.sh" | ||
| - "**/*.ps1" | ||
| - "**/*.mjs" | ||
|
kyle-sexton marked this conversation as resolved.
|
||
| - "**/*.ts" | ||
| - "**/*.js" | ||
| - "**/*.cjs" | ||
| - "**/.mcp.json" | ||
| - "**/mcp.json" | ||
| - "**/package.json" | ||
| - "**/package-lock.json" | ||
| - "plugins/*/.claude-plugin/plugin.json" | ||
|
kyle-sexton marked this conversation as resolved.
|
||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| concurrency: | ||
| group: claude-security-review-${{ github.event.pull_request.number }} | ||
| cancel-in-progress: false | ||
|
|
||
| jobs: | ||
| security-review: | ||
| permissions: | ||
| contents: read # checkout + read the diff | ||
| pull-requests: write # post the security review | ||
| id-token: write # OIDC — mints the Claude GitHub App token | ||
| uses: melodic-software/ci-workflows/.github/workflows/claude-security-review.yml@0a5fd4c578f2a6278fdc0e586b8e08392e64299a # 0a5fd4c 2026-07-20 | ||
|
kyle-sexton marked this conversation as resolved.
|
||
| with: | ||
| runner: ubuntu-24.04 | ||
| skip-actors: "dependabot[bot],melodic-standards-sync[bot]" | ||
| # One named secret (least privilege), never `secrets: inherit`. | ||
| secrets: | ||
| CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} | ||
51 changes: 51 additions & 0 deletions
51
docs/adr/0002-default-on-ai-review-advisory-with-earned-promotion.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| # Default-on AI review: advisory lanes with earned promotion to blocking | ||
|
|
||
| - Status: accepted (interim — pending the #509 enforcement design ruling) | ||
| - Date: 2026-07-20 | ||
|
|
||
| ## Context | ||
|
|
||
| Boris Cherny's step-2 posture ("Steps of AI Adoption", Jul 16 2026) names "Automated code | ||
| review and security review are on by default"; the AI-adoption-ladder residue items #696 | ||
| (default-on review decision + wiring) and #509 (no dedicated security-review pass) carried | ||
| the two open choices: promote the LLM review lane from advisory to blocking or keep it | ||
| advisory with a recorded promotion trigger, and where a dedicated security pass runs. | ||
|
|
||
| Evidence in hand at decision time: the LLM code-review lane (`claude-review` reusable | ||
| workflow, consumed by this repo's caller) already auto-invokes on every PR event — | ||
| default-on is satisfied for invocation; the open question was gating. #618 documents the | ||
| noise cost of advisory bot threads blocking merges on every PR; the WP5 guardrail work | ||
| ratified the verification-promotion discipline (a gate flips from advisory to blocking only | ||
| on demonstrated precision — an earned flip with a ratified record, trust before scale). | ||
| 2026-07-20 transcript mining (548 real permission prompts; #697's evidence) reinforced that | ||
| review friction compounds fast at fleet scale. | ||
|
|
||
| ## Decision | ||
|
|
||
| 1. **Both AI review lanes are DEFAULT-ON and ADVISORY**: the general code-review lane on | ||
| every PR (existing caller), and the dedicated security-review lane | ||
| (`claude-security-review` reusable workflow, ci-workflows) wired by this repo's caller | ||
| with a PATH FILTER over security-sensitive surfaces — workflows, scripts, hooks, shell, | ||
| and permission/settings configuration. Path filtering is the scope control that keeps | ||
| default-on affordable in a doc-heavy repo: most PRs are prose and get the general lane | ||
| only. | ||
| 2. **Promotion to blocking is earned, not assumed**: either lane flips to a required gate | ||
| (security: blocking on CRITICAL findings) only after its precision is proven over a | ||
| sustained window, recorded as a reviewed change citing that evidence — mirroring the | ||
| WP5 verification-promotion discipline. No calendar-based flip. | ||
| 3. **Severity vocabulary**: the security lane reports CRITICAL/IMPORTANT/SUGGESTION with a | ||
| confidence axis, matching the review-toolkit convention. | ||
|
|
||
| This is the interim posture pending #509's enforcement design session (operator-deferred, | ||
| 2026-07-19): how the pipeline enforces and evidences that a security pass ran on every PR is | ||
| that session's question, and it may promote the security lane to a required gate on its own | ||
| terms without a further precision window. | ||
|
|
||
| ## Revisit triggers | ||
|
|
||
| - The security lane's findings prove precise over a sustained window → open the promotion | ||
| change (blocking on CRITICAL) citing the evidence. | ||
| - The general lane's advisory threads measurably gate merges again (#618's class) → tune | ||
| scope before considering demotion. | ||
| - ci-workflows ships a release changing either reusable workflow's contract → re-pin the | ||
| callers through the ordinary Dependabot/SHA-bump path. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.