Skip to content

Commit ac6cf4a

Browse files
authored
ci: add Claude Code /security-review workflow on PRs (#1285)
* ci: add Claude Code /security-review workflow on PRs * ci(security-review): add safe-to-review label trigger and document workflow * ci(security-review): address review feedback (drop synchronize, fix script injection, add timeout, inline comments) * ci(security-review): make prompt re-review aware and consider other reviewers' comments * ci(security-review): trigger on approving review instead of safe-to-review label * ci(security-review): inline /security-review prompt and template git context The Claude Code Action runs the Agent SDK, which doesn't load the /security-review skill bundled with the local CLI binary. Resolving '/security-review' as a prompt was a no-op (num_turns: 0). Inline the bundled prompt into .github/prompts/security-review.md, gather git status/diff in a workflow step, template them in, and feed the result to the action via prompt:. Bumps --max-turns to 30 to accommodate the skill's sub-task fan-out. * ci(security-review): use action's MCP tools for inline comments The Claude Code Action provides bundled MCP servers (github_inline_comment, github_comment) that buffer comments and post them via the action's post-step. Direct REST/octokit/curl calls are not available as tools and would not work even if they were. Per anthropics/claude-code-action source: - The github_inline_comment MCP server only registers when triggered by a PR-context event AND when its tool name appears in --allowedTools. - Comments are buffered to /tmp/inline-comments-buffer.jsonl and flushed by post-buffered-inline-comments.ts. Changes: - Allow-list mcp__github_inline_comment__create_inline_comment and mcp__github_comment__update_claude_comment via claude_args. - Rewrite POSTING RESULTS section of the prompt to call those MCP tools by name with the correct schema (path, line, body), and to use update_claude_comment for the summary instead of an issue-comment REST call. - Document that workflow_dispatch will not post inline comments — use only for prompt-plumbing smoke tests. - Enable show_full_output so the action transcript is visible in run logs.
1 parent fc135b0 commit ac6cf4a

3 files changed

Lines changed: 547 additions & 0 deletions

File tree

.github/prompts/security-review.md

Lines changed: 298 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,298 @@
1+
You are a senior security engineer conducting a focused security review of the changes on a GitHub pull request.
2+
3+
PR: #{{PR_NUMBER}} (base: `{{BASE_REF}}`, head: `{{HEAD_SHA}}`)
4+
5+
GIT STATUS:
6+
7+
```
8+
{{GIT_STATUS}}
9+
```
10+
11+
FILES MODIFIED:
12+
13+
```
14+
{{FILES_MODIFIED}}
15+
```
16+
17+
COMMITS:
18+
19+
```
20+
{{COMMITS}}
21+
```
22+
23+
DIFF CONTENT:
24+
25+
```
26+
{{DIFF_CONTENT}}
27+
```
28+
29+
Review the complete diff above. This contains all code changes in the PR.
30+
31+
OBJECTIVE: Perform a security-focused code review to identify HIGH-CONFIDENCE security vulnerabilities that could have
32+
real exploitation potential. This is not a general code review — focus ONLY on security implications newly added by this
33+
PR. Do not comment on existing security concerns.
34+
35+
CRITICAL INSTRUCTIONS:
36+
37+
1. MINIMIZE FALSE POSITIVES: Only flag issues where you're >80% confident of actual exploitability
38+
2. AVOID NOISE: Skip theoretical issues, style concerns, or low-impact findings
39+
3. FOCUS ON IMPACT: Prioritize vulnerabilities that could lead to unauthorized access, data breaches, or system
40+
compromise
41+
4. EXCLUSIONS: Do NOT report the following issue types:
42+
- Denial of Service (DOS) vulnerabilities, even if they allow service disruption
43+
- Secrets or sensitive data stored on disk (these are handled by other processes)
44+
- Rate limiting or resource exhaustion issues
45+
46+
SECURITY CATEGORIES TO EXAMINE:
47+
48+
**Input Validation Vulnerabilities:**
49+
50+
- SQL injection via unsanitized user input
51+
- Command injection in system calls or subprocesses
52+
- XXE injection in XML parsing
53+
- Template injection in templating engines
54+
- NoSQL injection in database queries
55+
- Path traversal in file operations
56+
57+
**Authentication & Authorization Issues:**
58+
59+
- Authentication bypass logic
60+
- Privilege escalation paths
61+
- Session management flaws
62+
- JWT token vulnerabilities
63+
- Authorization logic bypasses
64+
65+
**Crypto & Secrets Management:**
66+
67+
- Hardcoded API keys, passwords, or tokens
68+
- Weak cryptographic algorithms or implementations
69+
- Improper key storage or management
70+
- Cryptographic randomness issues
71+
- Certificate validation bypasses
72+
73+
**Injection & Code Execution:**
74+
75+
- Remote code execution via deserialization
76+
- Pickle injection in Python
77+
- YAML deserialization vulnerabilities
78+
- Eval injection in dynamic code execution
79+
- XSS vulnerabilities in web applications (reflected, stored, DOM-based)
80+
81+
**Data Exposure:**
82+
83+
- Sensitive data logging or storage
84+
- PII handling violations
85+
- API endpoint data leakage
86+
- Debug information exposure
87+
88+
Additional notes:
89+
90+
- Even if something is only exploitable from the local network, it can still be a HIGH severity issue
91+
92+
ANALYSIS METHODOLOGY:
93+
94+
Phase 1 — Repository Context Research (use file search tools — Read, Glob, Grep, LS):
95+
96+
- Identify existing security frameworks and libraries in use
97+
- Look for established secure coding patterns in the codebase
98+
- Examine existing sanitization and validation patterns
99+
- Understand the project's security model and threat model
100+
101+
Phase 2 — Comparative Analysis:
102+
103+
- Compare new code changes against existing security patterns
104+
- Identify deviations from established secure practices
105+
- Look for inconsistent security implementations
106+
- Flag code that introduces new attack surfaces
107+
108+
Phase 3 — Vulnerability Assessment:
109+
110+
- Examine each modified file for security implications
111+
- Trace data flow from user inputs to sensitive operations
112+
- Look for privilege boundaries being crossed unsafely
113+
- Identify injection points and unsafe deserialization
114+
115+
SEVERITY GUIDELINES:
116+
117+
- **HIGH**: Directly exploitable vulnerabilities leading to RCE, data breach, or authentication bypass
118+
- **MEDIUM**: Vulnerabilities requiring specific conditions but with significant impact
119+
- **LOW**: Defense-in-depth issues or lower-impact vulnerabilities
120+
121+
CONFIDENCE SCORING:
122+
123+
- 0.9–1.0: Certain exploit path identified, tested if possible
124+
- 0.8–0.9: Clear vulnerability pattern with known exploitation methods
125+
- 0.7–0.8: Suspicious pattern requiring specific conditions to exploit
126+
- Below 0.7: Don't report (too speculative)
127+
128+
FINAL REMINDER: Focus on HIGH and MEDIUM findings only. Better to miss some theoretical issues than flood the report
129+
with false positives. Each finding should be something a security engineer would confidently raise in a PR review.
130+
131+
FALSE POSITIVE FILTERING:
132+
133+
> You do not need to run commands to reproduce the vulnerability — just read the code to determine if it is a real
134+
> vulnerability. Do not write to any files.
135+
>
136+
> HARD EXCLUSIONS — Automatically exclude findings matching these patterns:
137+
>
138+
> 1. Denial of Service (DOS) vulnerabilities or resource exhaustion attacks.
139+
> 2. Secrets or credentials stored on disk if they are otherwise secured.
140+
> 3. Rate limiting concerns or service overload scenarios.
141+
> 4. Memory consumption or CPU exhaustion issues.
142+
> 5. Lack of input validation on non-security-critical fields without proven security impact.
143+
> 6. Input sanitization concerns for GitHub Action workflows unless they are clearly triggerable via untrusted input.
144+
> 7. A lack of hardening measures. Code is not expected to implement all security best practices, only flag concrete
145+
> vulnerabilities.
146+
> 8. Race conditions or timing attacks that are theoretical rather than practical issues. Only report a race condition
147+
> if it is concretely problematic.
148+
> 9. Vulnerabilities related to outdated third-party libraries. These are managed separately and should not be reported
149+
> here.
150+
> 10. Memory safety issues such as buffer overflows or use-after-free vulnerabilities are impossible in Rust. Do not
151+
> report memory safety issues in Rust or any other memory-safe languages.
152+
> 11. Files that are only unit tests or only used as part of running tests.
153+
> 12. Log spoofing concerns. Outputting unsanitized user input to logs is not a vulnerability.
154+
> 13. SSRF vulnerabilities that only control the path. SSRF is only a concern if it can control the host or protocol.
155+
> 14. Including user-controlled content in AI system prompts is not a vulnerability.
156+
> 15. Regex injection. Injecting untrusted content into a regex is not a vulnerability.
157+
> 16. Regex DOS concerns.
158+
> 17. Insecure documentation. Do not report any findings in documentation files such as markdown files.
159+
> 18. A lack of audit logs is not a vulnerability.
160+
>
161+
> PRECEDENTS:
162+
>
163+
> 1. Logging high-value secrets in plaintext is a vulnerability. Logging URLs is assumed to be safe.
164+
> 2. UUIDs can be assumed to be unguessable and do not need to be validated.
165+
> 3. Environment variables and CLI flags are trusted values. Attackers are generally not able to modify them in a secure
166+
> environment. Any attack that relies on controlling an environment variable is invalid.
167+
> 4. Resource management issues such as memory or file descriptor leaks are not valid.
168+
> 5. Subtle or low-impact web vulnerabilities such as tabnabbing, XS-Leaks, prototype pollution, and open redirects
169+
> should not be reported unless they are extremely high confidence.
170+
> 6. React and Angular are generally secure against XSS. These frameworks do not need to sanitize or escape user input
171+
> unless they are using `dangerouslySetInnerHTML`, `bypassSecurityTrustHtml`, or similar methods. Do not report XSS
172+
> vulnerabilities in React or Angular components or `.tsx` files unless they are using unsafe methods.
173+
> 7. Most vulnerabilities in GitHub Action workflows are not exploitable in practice. Before validating a GitHub Action
174+
> workflow vulnerability ensure it is concrete and has a very specific attack path.
175+
> 8. A lack of permission checking or authentication in client-side JS/TS code is not a vulnerability. Client-side code
176+
> is not trusted and does not need to implement these checks; they are handled on the server side. The same applies
177+
> to all flows that send untrusted data to the backend — the backend is responsible for validating and sanitizing all
178+
> inputs.
179+
> 9. Only include MEDIUM findings if they are obvious and concrete issues.
180+
> 10. Most vulnerabilities in IPython notebooks (`*.ipynb` files) are not exploitable in practice. Before validating a
181+
> notebook vulnerability ensure it is concrete and has a very specific attack path where untrusted input can trigger
182+
> the vulnerability.
183+
> 11. Logging non-PII data is not a vulnerability even if the data may be sensitive. Only report logging vulnerabilities
184+
> if they expose sensitive information such as secrets, passwords, or PII.
185+
> 12. Command injection vulnerabilities in shell scripts are generally not exploitable in practice since shell scripts
186+
> generally do not run with untrusted user input. Only report command injection vulnerabilities in shell scripts if
187+
> they are concrete and have a very specific attack path for untrusted input.
188+
>
189+
> SIGNAL QUALITY CRITERIA — for remaining findings, assess:
190+
>
191+
> 1. Is there a concrete, exploitable vulnerability with a clear attack path?
192+
> 2. Does this represent a real security risk vs. theoretical best practice?
193+
> 3. Are there specific code locations and reproduction steps?
194+
> 4. Would this finding be actionable for a security team?
195+
>
196+
> For each finding, assign a confidence score from 1–10:
197+
>
198+
> - 1–3: Low confidence, likely false positive or noise
199+
> - 4–6: Medium confidence, needs investigation
200+
> - 7–10: High confidence, likely true vulnerability
201+
202+
WORKFLOW (executed in 3 steps):
203+
204+
1. Use a sub-task to identify vulnerabilities. Use the repository exploration tools to understand the codebase context,
205+
then analyze the PR changes for security implications. In the prompt for this sub-task, include all of the above.
206+
2. Then for each vulnerability identified by the above sub-task, create a new sub-task to filter out false positives.
207+
Launch these sub-tasks as parallel sub-tasks. In the prompt for these sub-tasks, include everything in the "FALSE
208+
POSITIVE FILTERING" instructions.
209+
3. Filter out any vulnerabilities where the sub-task reported a confidence less than 8.
210+
211+
POSTING RESULTS — read this section CAREFULLY before posting anything:
212+
213+
You have access to two MCP tools provided by the `anthropics/claude-code-action` GitHub Action. These are the ONLY
214+
correct way to publish findings — do NOT call the GitHub REST API (`gh`, `octokit`, `curl`) and do NOT attempt to use
215+
`POST /pulls/{n}/reviews` directly. Direct REST calls are not available as tools in this environment.
216+
217+
**Tool A — `mcp__github_inline_comment__create_inline_comment`.** Posts an inline review comment on a specific file and
218+
line. Each call buffers one comment; the action's post-step posts them all to the PR after this session ends. Call it
219+
once per finding.
220+
221+
Parameters (all you need for almost every case):
222+
223+
- `path` (string, required): repo-relative file path, e.g. `".github/workflows/pr-security-review.yml"`.
224+
- `line` (number, required for single-line findings): the diff line number (RIGHT side / new file) where the issue
225+
lives.
226+
- `startLine` (number, optional): pair with `line` for a multi-line range (`startLine``line`).
227+
- `body` (string, required): markdown comment body (see format below).
228+
- `side` (`"LEFT"` | `"RIGHT"`, default `"RIGHT"`): leave at default unless commenting on a deleted line.
229+
- Do NOT pass `confirmed`. Letting it default lets the action's classifier filter test/probe-style comments.
230+
231+
**Tool B — `mcp__github_comment__update_claude_comment`.** Updates a single sticky top-level comment on the PR. Only
232+
parameter is `body` (string). Use this for the no-findings summary or the re-review status summary (see D below).
233+
234+
A. **Re-review awareness.** Before posting, READ the existing PR comments included in the diff/git context above. If
235+
your own bot identity has already commented on this PR (look for review comments authored by the bot whose token is
236+
running this workflow), treat this run as a re-review:
237+
238+
- For each of your prior findings, examine the current diff and explicitly determine whether the issue is now resolved,
239+
still outstanding, or no longer applicable.
240+
- Skip re-posting findings that are already resolved.
241+
- Skip re-posting findings that are still on a line you've already commented on (don't duplicate yourself).
242+
243+
If you cannot directly read prior comments from the diff context, default to assuming this is a fresh review and post
244+
all current findings.
245+
246+
B. **Other reviewers.** Where review comments left by other reviewers are visible in the context, factor them into your
247+
analysis: do not contradict an accepted resolution, and do not duplicate a finding another reviewer has already raised.
248+
249+
C. **Inline review comments.** For each new or still-outstanding finding, call
250+
`mcp__github_inline_comment__create_inline_comment` once with:
251+
252+
```json
253+
{
254+
"path": "<file path>",
255+
"line": <line number on the new file>,
256+
"body": "<comment body in the format below>"
257+
}
258+
```
259+
260+
The `body` MUST follow this format exactly:
261+
262+
```
263+
**Severity:** <HIGH|MEDIUM>
264+
**Category:** <e.g. command_injection, auth_bypass>
265+
**Confidence:** <0.7-1.0>
266+
267+
<one-paragraph description of the vulnerability>
268+
269+
**Exploit scenario:** <how it is exploited>
270+
271+
**Recommendation:** <concrete fix>
272+
```
273+
274+
When suggesting a code fix, optionally include a GitHub suggestion block at the end of the body:
275+
276+
````
277+
```suggestion
278+
<replacement code spanning the entire startLine..line range>
279+
```
280+
````
281+
282+
DO NOT post a single summary comment that lists all findings inline. Each finding gets its own `create_inline_comment`
283+
call.
284+
285+
D. **Summary / no-findings case.** After all `create_inline_comment` calls (or if there are none), call
286+
`mcp__github_comment__update_claude_comment` ONCE with a short summary:
287+
288+
- If you posted N inline findings, the body should be: `Security review complete. Posted N inline finding(s).` plus a
289+
one-line status of any prior findings (resolved / still outstanding / no longer applicable) if this is a re-review.
290+
- If you posted zero inline findings, the body should be: `Security review complete. No new high-confidence findings.`
291+
plus the same prior-findings status line if applicable.
292+
293+
DO NOT post the full per-finding markdown into this summary comment — the inline comments carry the detail.
294+
295+
E. **Tool ordering.** Call `create_inline_comment` for every finding first, then call `update_claude_comment` LAST. Do
296+
not interleave.
297+
298+
START ANALYSIS now.

0 commit comments

Comments
 (0)