When to use
Use when a change touches any of the following surface area:
- Authentication, session handling, password reset, MFA
- Authorization checks (RBAC, ABAC, ownership, multi-tenancy)
- Secret handling, key rotation, KMS access
- Cryptographic operations (encryption, hashing, signing, RNG)
- Deserialization of untrusted input (JSON with allowlists, pickle, YAML load)
- Network egress and ingress (URL handling, SSRF, DNS rebinding)
- File system access (path traversal, symlink attacks, temp file races)
- Database queries (SQL injection, ORM pitfalls, transaction boundaries)
- HTML/email rendering (XSS, content sniffing, URL sanitization)
- Process execution (shell injection, command arguments)
- Dependency changes (new package, version bump)
Skip this checklist if the change is documentation-only, test-only, or
purely cosmetic. Run it once per PR, even if the change feels small.
Examples
For example, a PR that adds a new POST /api/v2/users/{id}/avatar endpoint:
[✓] Auth: the route requires an authenticated session (not just any token)
[✓] Authz: the loader enforces the caller owns {id} OR has admin role
[ ] Input: file MIME type is sniffed, not trusted from Content-Type
[ ] Input: file size limit enforced before streaming to disk
[✓] Path: filename is sanitised — no ../, no NUL, length capped
[ ] Storage: files written outside the web root, served via signed URL
[✓] Secrets: the S3 credentials are loaded from the secret manager,
not from environment variables
[ ] Logging: the request body is NOT logged (PII + secrets)
[ ] Errors: 4xx and 5xx return JSON, not HTML or stack traces
[ ] Tests: a request without auth returns 401 (not 500, not 302)
[ ] Tests: a path-traversal payload returns 400 (not 500, not 200)
A PR that bumps a dependency from requests==2.31.0 to requests==2.32.0:
[✓] Changelog entry links the CVE-2024-35195 fix
[✓] The pinned hash matches the upstream release
[✓] No transitive regression in urllib3 (lock file diff reviewed)
[✓] CI re-runs the full integration suite on the bumped version
[ ] Dependabot / Renovate config updated if needed
Pitfalls to avoid
- Do not treat a passing security linter as a complete review. Bandit,
Semgrep, and CodeQL catch known patterns; they do not catch missing
auth checks, broken object-level authorisation, or business-logic flaws.
- Do not approve a change because "we already do this elsewhere".
Each instance is a new attack surface until proven otherwise.
- Do not flag findings without a concrete reproduction. "Looks
suspicious" is not actionable; "POST /api with
Authorization: Bearer
plus a tampered JWT signature returns 200" is.
- Do not defer security fixes to follow-up PRs. A separate fix PR
is fine; a separate fix sprint is not.
- Do not assume secrets in environment variables are safe. They show
up in
ps, in crash dumps, in error reports, in container images, and
in CI logs.
- Do not approve a change that adds a new dependency without
checking the package's history, the maintainer's reputation, and
whether it has had a security incident in the last 12 months.
1---2name: security-review-checklist3description: Use when reviewing a code change for security implications — pull requests touching auth, secrets, crypto, input handling, deserialization, network calls, file system access, or any user-controlled data flow. Triggered by "security review", "is this safe to ship", "any vulns here", or "audit this change". Do not use for general code review (use code-review-checklist), threat modelling new systems (use a dedicated threat-modelling skill), or compliance audits.4---56## When to use78Use when a change touches any of the following surface area:910- Authentication, session handling, password reset, MFA11- Authorization checks (RBAC, ABAC, ownership, multi-tenancy)12- Secret handling, key rotation, KMS access13- Cryptographic operations (encryption, hashing, signing, RNG)14- Deserialization of untrusted input (JSON with allowlists, pickle, YAML load)15- Network egress and ingress (URL handling, SSRF, DNS rebinding)16- File system access (path traversal, symlink attacks, temp file races)17- Database queries (SQL injection, ORM pitfalls, transaction boundaries)18- HTML/email rendering (XSS, content sniffing, URL sanitization)19- Process execution (shell injection, command arguments)20- Dependency changes (new package, version bump)2122Skip this checklist if the change is documentation-only, test-only, or23purely cosmetic. Run it once per PR, even if the change feels small.2425## Examples2627For example, a PR that adds a new `POST /api/v2/users/{id}/avatar` endpoint:2829```text30[✓] Auth: the route requires an authenticated session (not just any token)31[✓] Authz: the loader enforces the caller owns {id} OR has admin role32[ ] Input: file MIME type is sniffed, not trusted from Content-Type33[ ] Input: file size limit enforced before streaming to disk34[✓] Path: filename is sanitised — no ../, no NUL, length capped35[ ] Storage: files written outside the web root, served via signed URL36[✓] Secrets: the S3 credentials are loaded from the secret manager,37 not from environment variables38[ ] Logging: the request body is NOT logged (PII + secrets)39[ ] Errors: 4xx and 5xx return JSON, not HTML or stack traces40[ ] Tests: a request without auth returns 401 (not 500, not 302)41[ ] Tests: a path-traversal payload returns 400 (not 500, not 200)42```4344A PR that bumps a dependency from `requests==2.31.0` to `requests==2.32.0`:4546```text47[✓] Changelog entry links the CVE-2024-35195 fix48[✓] The pinned hash matches the upstream release49[✓] No transitive regression in urllib3 (lock file diff reviewed)50[✓] CI re-runs the full integration suite on the bumped version51[ ] Dependabot / Renovate config updated if needed52```5354## Pitfalls to avoid5556- **Do not** treat a passing security linter as a complete review. Bandit,57 Semgrep, and CodeQL catch known patterns; they do not catch missing58 auth checks, broken object-level authorisation, or business-logic flaws.59- **Do not** approve a change because "we already do this elsewhere".60 Each instance is a new attack surface until proven otherwise.61- **Do not** flag findings without a concrete reproduction. "Looks62 suspicious" is not actionable; "POST /api with `Authorization: Bearer`63 plus a tampered JWT signature returns 200" is.64- **Do not** defer security fixes to follow-up PRs. A separate fix PR65 is fine; a separate fix sprint is not.66- **Do not** assume secrets in environment variables are safe. They show67 up in `ps`, in crash dumps, in error reports, in container images, and68 in CI logs.69- **Do not** approve a change that adds a new dependency without70 checking the package's history, the maintainer's reputation, and71 whether it has had a security incident in the last 12 months.
Mine-FNL