Loading...
Loading...
Perform security-focused review of code diffs and pull requests, identifying newly introduced vulnerabilities, security regressions, and unsafe patterns in changed code.
npx skill4agent add oimiragieo/agent-studio differential-review# Review staged changes
git diff --cached
# Review specific commit
git diff HEAD~1..HEAD
# Review pull request (GitHub)
gh pr diff <PR-NUMBER>
# Review specific files
git diff --cached -- src/auth/ src/api/
# Review with context (10 lines)
git diff -U10 HEAD~1..HEAD
# Show only changed file names
git diff --name-only HEAD~1..HEAD
# Show stats (insertions/deletions per file)
git diff --stat HEAD~1..HEAD| Priority | File Patterns | Reason |
|---|---|---|
| P0 | | Direct security code |
| P0 | | Configuration and secrets |
| P0 | | Security controls |
| P1 | | Attack surface |
| P1 | | Dependency changes |
| P1 | | Infrastructure config |
| P2 | | Data access layer |
| P2 | | Shared utility code |
| P3 | | Tests and documentation |
CHECK: Did the change modify input validation?
- Added validation: POSITIVE (verify correctness)
- Removed validation: CRITICAL (likely regression)
- Changed validation: INVESTIGATE (may weaken security)
- No validation on new input: WARNING (missing validation)strictlooseanyCHECK: Did the change affect auth?
- New endpoint without auth middleware: CRITICAL
- Removed auth check: CRITICAL
- Changed permission levels: INVESTIGATE
- Modified token handling: INVESTIGATE
- Added new auth bypass: CRITICALisAdminCHECK: Did the change introduce new data flows?
- User input to database: CHECK for injection
- User input to HTML: CHECK for XSS
- User input to file system: CHECK for path traversal
- User input to command execution: CHECK for command injection
- User input to redirect: CHECK for open redirectCHECK: Did the change affect cryptography?
- Algorithm downgrade: CRITICAL (e.g., SHA-256 to MD5)
- Key size reduction: CRITICAL
- Removed encryption: CRITICAL
- Changed to ECB mode: CRITICAL
- Hardcoded key/IV: CRITICALCHECK: Did the change affect error handling?
- Removed try/catch: WARNING
- Added stack trace in response: CRITICAL (info disclosure)
- Changed error to success: CRITICAL (fail-open)
- Swallowed exceptions: WARNINGCHECK: Did dependencies change?
- New dependency: CHECK for known CVEs
- Version downgrade: INVESTIGATE
- Removed security dependency: CRITICAL
- Changed to fork/alternative: INVESTIGATE# Check new dependencies for known vulnerabilities
npm audit
pip audit
go list -m -json all | nancy sleuth**SECURITY [SEVERITY]**: [Brief description]
**Location**: `file.js:42` (in diff hunk)
**Category**: [OWASP/CWE category]
**Impact**: [What could go wrong]
**Remediation**: [How to fix]
```diff
- // Current (vulnerable)
- db.query("SELECT * FROM users WHERE id = " + userId);
+ // Suggested (safe)
+ db.query("SELECT * FROM users WHERE id = $1", [userId]);
```
### Severity Levels for Diff Findings
| Severity | Criteria | Action |
|----------|----------|--------|
| **CRITICAL** | Exploitable vulnerability introduced | Block merge |
| **HIGH** | Security regression or missing control | Block merge |
| **MEDIUM** | Weak pattern that could lead to vulnerability | Request changes |
| **LOW** | Style issue with security implications | Suggest improvement |
| **INFO** | Security observation, no immediate risk | Note for awareness |
## Step 4: Differential Security Report
### Report Template
```markdown
## Differential Security Review
**PR/Commit**: [reference]
**Author**: [author]
**Reviewer**: security-architect
**Date**: YYYY-MM-DD
**Files Changed**: X | Additions: +Y | Deletions: -Z
### Security Impact Summary
| Category | Before | After | Change |
|----------|--------|-------|--------|
| Input validation | X checks | Y checks | +/-N |
| Auth-protected routes | X routes | Y routes | +/-N |
| SQL parameterization | X% | Y% | +/-N% |
| Secrets exposure | X | Y | +/-N |
### Findings
#### CRITICAL
1. [Finding with full details and remediation]
#### HIGH
1. [Finding with full details and remediation]
#### MEDIUM
1. [Finding with full details and remediation]
### Verdict
- [ ] APPROVE: No security issues found
- [ ] APPROVE WITH CONDITIONS: Minor issues, fix before deploy
- [ ] REQUEST CHANGES: Security issues must be addressed
- [ ] BLOCK: Critical vulnerability introduced# Scan only changed files
semgrep scan --config=p/security-audit --baseline-commit=main
# Scan diff between branches
semgrep scan --config=p/security-audit --baseline-commit=origin/main
# Output as SARIF for CI integration
semgrep scan --config=p/security-audit --baseline-commit=main --sarif --output=diff-results.sarif# Check for secrets in diff
git diff --cached | grep -iE "(password|secret|api.?key|token|credential)\s*[=:]"
# Check for dangerous function additions
git diff --cached | grep -E "^\+" | grep -iE "(eval|exec|system|innerHTML|dangerouslySetInnerHTML)"
# Check for removed security middleware
git diff --cached | grep -E "^\-" | grep -iE "(authenticate|authorize|validate|sanitize|escape)"
# Check for new TODO/FIXME security items
git diff --cached | grep -E "^\+" | grep -iE "(TODO|FIXME|HACK|XXX).*(security|auth|vuln)"name: Security Diff Review
on: [pull_request]
jobs:
security-diff:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Semgrep diff scan
uses: returntocorp/semgrep-action@v1
with:
config: p/security-audit
- name: Check for secrets
run: |
git diff origin/main..HEAD | grep -iE "(password|secret|api.?key|token)\s*[=:]" && exit 1 || exit 0| Pattern | What Changed | Risk |
|---|---|---|
Removed | Security headers removed | Header injection, clickjacking |
Changed | Cookie policy weakened | CSRF attacks |
| Removed rate limiting middleware | Rate limit removed | Brute force, DoS |
Added | CORS wildcard | Cross-origin attacks |
Removed | CSRF protection removed | CSRF attacks |
Changed | Cookie accessible to JS | XSS token theft |
static-analysisvariant-analysissemgrep-rule-creatorinsecure-defaultssecurity-architect.claude/context/memory/learnings.md.claude/context/memory/learnings.md.claude/context/memory/issues.md.claude/context/memory/decisions.mdASSUME INTERRUPTION: If it's not in memory, it didn't happen.