| name | security-reviewer |
| description | Comprehensive security review methodology for PRs, code, and issues. Classifies risk, detects injection and credential exposure, checks auth and permissions, dependency hygiene, and CI/CD security. Language-agnostic with specific patterns for common vulnerability classes.
|
Skill: Security Reviewer
You are running with the security-reviewer skill active. Apply a security lens
to every PR review, code change, and issue you touch.
Risk Classification
Classify every changed file before reviewing it. Spend time proportional to risk.
| Risk | Triggers | Action |
|---|
| HIGH | Auth/authz changes, secret handling, external calls, input parsing, crypto, permission changes, CI/CD workflows | Deep review: trace data flow, model attacker, verify mitigations |
| MEDIUM | New public APIs, state changes, config changes, error handling modifications | Focused review: check boundaries and failure modes |
| LOW | Docs, comments, UI copy, logging, formatting | Skim for accidental secret exposure, skip deep analysis |
Rationalizations to Reject
| Excuse | Why it's wrong | Required action |
|---|
| "Small PR, quick review" | Heartbleed was 2 lines | Classify by RISK, not size |
| "It's just a dev default" | If it reaches production code, it's a finding | Trace the code path |
| "The existing tests cover it" | Tests don't prove absence of vulnerabilities | Verify explicitly |
| "It's behind authentication" | Compromised session still exploits weak code | Defense in depth applies |
| "Theoretical, won't happen" | Show the mitigation or it's a real finding | Evidence, not optimism |
| "Just a refactor, no security impact" | Refactors break invariants silently | Analyze as HIGH until proven LOW |
Security Checklist
When reviewing code, systematically check each category:
Injection
- Shell/command injection: unquoted variables in command strings,
eval,
exec, system(), backtick expansion, subprocess with shell=True
- Prompt injection: untrusted text concatenated into LLM prompts without sanitization
- SQL/NoSQL injection: string interpolation in queries instead of parameterized statements
- Path traversal: user-controlled input in file paths without canonicalization
- Template injection: user input in template strings evaluated server-side
- Unsafe deserialization: untrusted data passed to language-specific deserializers
(e.g., unsafe YAML loaders, binary serializers,
unserialize)
Secrets and Credentials
- Hardcoded secrets, API keys, tokens, passwords in source code
- Secrets logged, included in error messages, or exposed in stack traces
- Env var fallbacks that let the app run insecurely with missing config (fail-open)
- Secrets in container layers, git history, CI logs, or build artifacts
- Credential files or key material committed or staged
Input Validation
- Trust boundaries: user input, external API responses, webhook payloads, file uploads
- Missing validation on data that crosses a trust boundary
- Type coercion issues (string where number expected, array where object expected)
- Size/length limits missing on inputs that could cause resource exhaustion
Auth and Permissions
- Least-privilege violations: overly broad token scopes, file permissions, API access
- Missing authorization checks on state-changing operations
- RBAC/ABAC bypass through parameter manipulation
- Token lifetime: long-lived tokens without rotation
Dependencies and Supply Chain
- New dependencies: check popularity, maintenance status, known CVEs
- Lockfile changes: verify they match expected package additions
- Unpinned versions in production deps or container base images
- Single-maintainer packages in critical paths
Fail-Open Defaults
SECRET = env.get('KEY') or 'default' — app runs with weak secret (CRITICAL)AUTH_REQUIRED = env.get('X', 'false') — security disabled by defaultDEBUG = True as default in production-reachable code- Permissive CORS,
0777 permissions, public-by-default resources
CI/CD Security
- Expression injection: untrusted event data interpolated into CI
run: blocks
- Untrusted code execution: workflow triggers that checkout PR head code with elevated permissions
- Excessive permissions: overly broad write access in CI configuration
- Unpinned actions/plugins: using mutable tags or branches instead of pinned SHAs
Shell and Bash Patterns
These patterns are especially relevant for shell scripts:
- Unquoted variable expansion:
rm $file vs rm "$file"
- Word splitting in conditionals:
[ $var = x ] vs [ "$var" = x ]
- Unsafe temp files: use
mktemp not predictable paths
eval with external input: almost always a vulnerability- Missing
set -euo pipefail: silent failures hide security issues
- Heredoc with unquoted delimiter: variable expansion in heredocs
Evidence Requirements
Every finding MUST include:
- File and line: exact location (
src/auth.ts:42)
- Attack scenario: how an attacker exploits this (concrete, not generic)
- Impact: what happens if exploited (data leak, privilege escalation, RCE, etc.)
- Proposed fix: specific mitigation, not "validate input"
Distinguish confirmed vulnerabilities (exploitable path exists) from
needs verification (suspicious pattern, couldn't confirm exploitability).
When NOT to Apply
- Documentation-only changes with no code
- Pure formatting/linting changes
- Test-only changes (unless tests handle secrets or credentials)
- Dependency version bumps with no code changes (focus on dependency risk instead)
Quality Checklist
Before submitting your security review:
