feat(skills): add security-code-review agent skill#1366
Merged
Conversation
Adds a user-invocable skill that performs a 9-category security review of GitHub PR or issue changes, producing PASS/WARNING/FAIL verdicts. Requested by security PICs for standard PR review workflows. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
📝 WalkthroughWalkthroughAdds a new skill documentation file defining a "security-code-review" capability for GitHub PRs. Specifies prerequisites, GitHub URL parsing, PR checkout procedures, file diff analysis, triage logic for large PRs, a 9-category security checklist with per-category verdicts, and structured report output format. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.agents/skills/security-code-review/SKILL.md (2)
40-47: Add warning about not executing code from cloned repositories.When reviewing external repositories, the skill clones arbitrary code to a temporary directory. While this likely runs in a sandbox, the documentation should explicitly warn against executing any files from the cloned repository (build scripts, tests, git hooks, etc.), as they could be malicious.
🛡️ Suggested addition
If reviewing a different repo, clone it to a temporary directory first: ```bash TMPDIR=$(mktemp -d) gh repo clone OWNER/REPO "$TMPDIR" cd "$TMPDIR" gh pr checkout <number>
+Security Note: Never execute any files from the cloned repository (build scripts, tests, binaries, etc.). Only read files for static analysis.
</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In @.agents/skills/security-code-review/SKILL.md around lines 40 - 47, Add a
clear security warning to the cloning instructions: after the TMPDIR, gh repo
clone, cd and gh pr checkout steps, insert a bolded "Security Note" reminding
reviewers to never execute any files from the cloned repository (e.g., build
scripts, tests, binaries, git hooks) and to limit actions to static file
inspection only; reference the existing commands (TMPDIR, gh repo clone, gh pr
checkout) so the note appears immediately after those steps for visibility.</details> --- `33-77`: **Consider adding error handling guidance for shell commands.** The shell command examples don't include error handling. Add guidance on handling common failure scenarios such as repository not found, PR doesn't exist, authentication failures, or network issues. This will help the agent respond gracefully instead of continuing with invalid state. <details> <summary>🎯 Example error handling note</summary> Add after Step 2 or in the Important Notes section: ```markdown **Error Handling**: If any `gh` or `git` command fails: - Report the error message to the user. - If the PR/repo is inaccessible, verify the URL and that you have appropriate permissions. - If checkout fails, the PR may be closed, the branch deleted, or there may be merge conflicts. - Do not proceed with analysis if the code cannot be checked out.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/security-code-review/SKILL.md around lines 33 - 77, The shell examples using gh and git lack guidance for handling failures: update the SKILL.md around the checkout and diff steps (references: the gh repo view check, gh pr checkout commands, and git diff main...HEAD --name-status / -- <file>) to include an "Error Handling" note that instructs the agent to detect command failures, surface the exact error message to the user, verify repo/PR URLs and permissions, stop further analysis if checkout or clone fails (e.g., repo not found, PR missing, auth or network errors), and suggest retry steps or authentication checks before proceeding.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/security-code-review/SKILL.md:
- Around line 49-61: Replace the hardcoded "main" in the git diff command by
first capturing the PR base branch with the existing gh pr view call and using
that variable in the git diff invocation: create a variable (e.g., BASE_BRANCH)
from gh pr view <number> --json baseRefName -q .baseRefName and then call git
diff "${BASE_BRANCH}...HEAD" --name-status; update the Step 3 content to
reference the new BASE_BRANCH variable instead of main and remove the
contradictory note about using a different base branch separately.
- Around line 63-69: The diff command in Step 4 hardcodes the branch name "main"
(the string "git diff main...HEAD -- <file>") which breaks PRs targeting other
base branches; update the command to use the BASE_BRANCH variable captured in
Step 3 by replacing that hardcoded token with the variable reference (e.g. use
"${BASE_BRANCH}...HEAD") so the git diff invocation reads the base branch
dynamically.
- Around line 33-47: The skill currently advertises support for both PRs and
issues but only provides checkout commands for PRs (see use of "gh pr checkout
<number>" and the TMPDIR clone flow); update the skill to either restrict scope
to PRs by removing mentions of issues from the description and "When to Use"
section and clarifying steps around "gh pr checkout <number>", or implement
issue-handling logic that extracts referenced branch names or commit SHAs from
an issue body/comments and attempts to git fetch and checkout those refs (e.g.,
parse the issue URL, use "gh issue view" to get content, then derive
branch/commit refs to git fetch/checkout); ensure the README text and Step 2
reflect the chosen approach so users are not given an unusable "gh pr checkout"
for issues.
---
Nitpick comments:
In @.agents/skills/security-code-review/SKILL.md:
- Around line 40-47: Add a clear security warning to the cloning instructions:
after the TMPDIR, gh repo clone, cd and gh pr checkout steps, insert a bolded
"Security Note" reminding reviewers to never execute any files from the cloned
repository (e.g., build scripts, tests, binaries, git hooks) and to limit
actions to static file inspection only; reference the existing commands (TMPDIR,
gh repo clone, gh pr checkout) so the note appears immediately after those steps
for visibility.
- Around line 33-77: The shell examples using gh and git lack guidance for
handling failures: update the SKILL.md around the checkout and diff steps
(references: the gh repo view check, gh pr checkout commands, and git diff
main...HEAD --name-status / -- <file>) to include an "Error Handling" note that
instructs the agent to detect command failures, surface the exact error message
to the user, verify repo/PR URLs and permissions, stop further analysis if
checkout or clone fails (e.g., repo not found, PR missing, auth or network
errors), and suggest retry steps or authentication checks before proceeding.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 027d8d7e-4ceb-4590-bcb0-c9dd2a1e7234
📒 Files selected for processing (1)
.agents/skills/security-code-review/SKILL.md
Comment on lines
+33
to
+47
|
|
||
| Determine whether you are already in the target repository (compare `gh repo view --json nameWithOwner -q .nameWithOwner` against the URL). If you are: | ||
|
|
||
| ```bash | ||
| gh pr checkout <number> | ||
| ``` | ||
|
|
||
| If reviewing a different repo, clone it to a temporary directory first: | ||
|
|
||
| ```bash | ||
| TMPDIR=$(mktemp -d) | ||
| gh repo clone OWNER/REPO "$TMPDIR" | ||
| cd "$TMPDIR" | ||
| gh pr checkout <number> | ||
| ``` |
Contributor
There was a problem hiding this comment.
Issue checkout logic is missing.
The skill description (line 3) and "When to Use" section (line 20) advertise support for both PRs and issues, but Step 2 only provides checkout logic for PRs via gh pr checkout <number>. GitHub issues don't have associated branches to check out, so this command would fail if the user provides an issue URL.
Either:
- Remove issue support from the skill description and scope it to PRs only, OR
- Add logic to extract and check out commits/branches referenced in issue descriptions or comments.
📝 Suggested clarification
If limiting to PRs only:
-description: Performs a comprehensive security review of code changes in a GitHub PR or issue. Checks out the branch, analyzes changed files against a 9-category security checklist, and produces PASS/WARNING/FAIL verdicts. Use when reviewing pull requests for security vulnerabilities, hardcoded secrets, injection flaws, auth bypasses, or insecure configurations. Trigger keywords - security review, code review, appsec, vulnerability assessment, security audit, review PR security.
+description: Performs a comprehensive security review of code changes in a GitHub pull request. Checks out the branch, analyzes changed files against a 9-category security checklist, and produces PASS/WARNING/FAIL verdicts. Use when reviewing pull requests for security vulnerabilities, hardcoded secrets, injection flaws, auth bypasses, or insecure configurations. Trigger keywords - security review, code review, appsec, vulnerability assessment, security audit, review PR security.-- Triaging a GitHub issue that reports a potential security flaw.-Supported URL formats:
-
-- `https://github.com/OWNER/REPO/pull/NUMBER`
-- `https://github.com/OWNER/REPO/issues/NUMBER`
+Supported URL format:
+
+- `https://github.com/OWNER/REPO/pull/NUMBER`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/security-code-review/SKILL.md around lines 33 - 47, The skill
currently advertises support for both PRs and issues but only provides checkout
commands for PRs (see use of "gh pr checkout <number>" and the TMPDIR clone
flow); update the skill to either restrict scope to PRs by removing mentions of
issues from the description and "When to Use" section and clarifying steps
around "gh pr checkout <number>", or implement issue-handling logic that
extracts referenced branch names or commit SHAs from an issue body/comments and
attempts to git fetch and checkout those refs (e.g., parse the issue URL, use
"gh issue view" to get content, then derive branch/commit refs to git
fetch/checkout); ensure the README text and Step 2 reflect the chosen approach
so users are not given an unusable "gh pr checkout" for issues.
Comment on lines
+49
to
+61
| ## Step 3: Identify Changed Files | ||
|
|
||
| List all files changed relative to the base branch: | ||
|
|
||
| ```bash | ||
| git diff main...HEAD --name-status | ||
| ``` | ||
|
|
||
| If the PR targets a branch other than `main`, use the correct base. Check with: | ||
|
|
||
| ```bash | ||
| gh pr view <number> --json baseRefName -q .baseRefName | ||
| ``` |
Contributor
There was a problem hiding this comment.
Use the actual base branch instead of hardcoding main.
Line 54 hardcodes main in the diff command, but lines 57-61 acknowledge that PRs may target different branches. If the PR targets develop, release/*, or another branch, the diff will include unrelated changes or miss actual changes, leading to incorrect security analysis.
Capture the base branch first and use it in the diff command.
🔧 Proposed fix
## Step 3: Identify Changed Files
-List all files changed relative to the base branch:
+Determine the base branch and list all files changed relative to it:
```bash
-git diff main...HEAD --name-status
+BASE_BRANCH=$(gh pr view <number> --json baseRefName -q .baseRefName)
+git diff "${BASE_BRANCH}...HEAD" --name-status-If the PR targets a branch other than main, use the correct base. Check with:
-bash -gh pr view <number> --json baseRefName -q .baseRefName -
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/security-code-review/SKILL.md around lines 49 - 61, Replace
the hardcoded "main" in the git diff command by first capturing the PR base
branch with the existing gh pr view call and using that variable in the git diff
invocation: create a variable (e.g., BASE_BRANCH) from gh pr view
--json baseRefName -q .baseRefName and then call git diff
"${BASE_BRANCH}...HEAD" --name-status; update the Step 3 content to reference
the new BASE_BRANCH variable instead of main and remove the contradictory note
about using a different base branch separately.
</details>
<!-- fingerprinting:phantom:triton:puma:4e18c5ee-dd6f-473c-8800-a038990ca073 -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+63
to
+69
| ## Step 4: Read Every Changed File and Diff | ||
|
|
||
| Read the full content of each changed file and the diff for that file: | ||
|
|
||
| ```bash | ||
| git diff main...HEAD -- <file> | ||
| ``` |
Contributor
There was a problem hiding this comment.
Use the base branch variable here as well.
Line 68 also hardcodes main in the diff command, which will produce incorrect diffs if the PR targets a different branch. Use the BASE_BRANCH variable captured in Step 3.
🔧 Proposed fix
Read the full content of each changed file and the diff for that file:
```bash
-git diff main...HEAD -- <file>
+git diff "${BASE_BRANCH}...HEAD" -- <file>
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
## Step 4: Read Every Changed File and Diff
Read the full content of each changed file and the diff for that file:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/security-code-review/SKILL.md around lines 63 - 69, The diff
command in Step 4 hardcodes the branch name "main" (the string "git diff
main...HEAD -- <file>") which breaks PRs targeting other base branches; update
the command to use the BASE_BRANCH variable captured in Step 3 by replacing that
hardcoded token with the variable reference (e.g. use "${BASE_BRANCH}...HEAD")
so the git diff invocation reads the base branch dynamically.
cv
approved these changes
Apr 2, 2026
HagegeR
reviewed
Apr 2, 2026
| Read the full content of each changed file and the diff for that file: | ||
|
|
||
| ```bash | ||
| git diff main...HEAD -- <file> |
Contributor
There was a problem hiding this comment.
Maybe this will not give enough context for the agent to decide on the severity of the issue?
cjagwani
pushed a commit
to fdzdev/NemoClaw
that referenced
this pull request
Apr 3, 2026
## Summary - Adds a new `security-code-review` user-invocable agent skill that performs a 9-category security review of GitHub PR or issue changes - Produces structured PASS/WARNING/FAIL verdicts per category with a findings table - Checklist covers: secrets, input validation, auth, dependencies, error handling, crypto, config/headers, security testing, and holistic posture - Includes NemoClaw-specific guidance for sandbox escape vectors (SSRF bypass, Dockerfile injection, network policy circumvention, credential leakage, blueprint tampering) ## Test plan - [ ] Invoke `/security-code-review` on a known-clean PR and verify all 9 categories produce PASS verdicts - [ ] Invoke on a PR with a known issue (e.g., hardcoded token in test fixture) and verify WARNING or FAIL is raised - [ ] Verify the skill works both when already inside the target repo and when pointed at an external repo URL - [ ] Confirm the structured report includes verdict, findings table, detailed analysis, and files reviewed sections <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added a security code review capability for automated analysis of pull requests and issues. * Performs comprehensive security assessment across 9 categories including secrets detection, input validation, authorization, dependencies, cryptography, and security configuration. * Generates structured reports with detailed findings, severity levels, and recommendations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
lakamsani
pushed a commit
to lakamsani/NemoClaw
that referenced
this pull request
Apr 4, 2026
## Summary - Adds a new `security-code-review` user-invocable agent skill that performs a 9-category security review of GitHub PR or issue changes - Produces structured PASS/WARNING/FAIL verdicts per category with a findings table - Checklist covers: secrets, input validation, auth, dependencies, error handling, crypto, config/headers, security testing, and holistic posture - Includes NemoClaw-specific guidance for sandbox escape vectors (SSRF bypass, Dockerfile injection, network policy circumvention, credential leakage, blueprint tampering) ## Test plan - [ ] Invoke `/security-code-review` on a known-clean PR and verify all 9 categories produce PASS verdicts - [ ] Invoke on a PR with a known issue (e.g., hardcoded token in test fixture) and verify WARNING or FAIL is raised - [ ] Verify the skill works both when already inside the target repo and when pointed at an external repo URL - [ ] Confirm the structured report includes verdict, findings table, detailed analysis, and files reviewed sections <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added a security code review capability for automated analysis of pull requests and issues. * Performs comprehensive security assessment across 9 categories including secrets detection, input validation, authorization, dependencies, cryptography, and security configuration. * Generates structured reports with detailed findings, severity levels, and recommendations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
gemini2026
pushed a commit
to gemini2026/NemoClaw
that referenced
this pull request
Apr 14, 2026
## Summary - Adds a new `security-code-review` user-invocable agent skill that performs a 9-category security review of GitHub PR or issue changes - Produces structured PASS/WARNING/FAIL verdicts per category with a findings table - Checklist covers: secrets, input validation, auth, dependencies, error handling, crypto, config/headers, security testing, and holistic posture - Includes NemoClaw-specific guidance for sandbox escape vectors (SSRF bypass, Dockerfile injection, network policy circumvention, credential leakage, blueprint tampering) ## Test plan - [ ] Invoke `/security-code-review` on a known-clean PR and verify all 9 categories produce PASS verdicts - [ ] Invoke on a PR with a known issue (e.g., hardcoded token in test fixture) and verify WARNING or FAIL is raised - [ ] Verify the skill works both when already inside the target repo and when pointed at an external repo URL - [ ] Confirm the structured report includes verdict, findings table, detailed analysis, and files reviewed sections <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added a security code review capability for automated analysis of pull requests and issues. * Performs comprehensive security assessment across 9 categories including secrets detection, input validation, authorization, dependencies, cryptography, and security configuration. * Generates structured reports with detailed findings, severity levels, and recommendations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
security-code-reviewuser-invocable agent skill that performs a 9-category security review of GitHub PR or issue changesTest plan
/security-code-reviewon a known-clean PR and verify all 9 categories produce PASS verdictsSummary by CodeRabbit
Release Notes