/pr-review-security

Selected when the diff touches auth, secrets, permissions, credentials, tokens, or environment configuration. Report exploitable reality, not a compliance checklist.

What to check

1. Authentication / authorization — a new route, tool, or IPC surface without an ownership check; a check that runs after the side effect; a permission assertion that trusts client-supplied identity.
2. Secrets — tokens, keys, or credentials in source, logs, error messages, telemetry, or committed fixtures. Also secrets widened from one process to another.
3. Injection — shell commands built from untrusted strings, SQL string concatenation, path traversal (..) on user-supplied paths, eval-class evaluation of fetched content.
4. Data exposure — a response, event, or log that now carries more than the caller is entitled to; PII in analytics.
5. Trust boundaries — treating an external response, MCP tool result, or model output as trusted input.
6. Dependency and transport — a new dependency pinned loosely, TLS/verification disabled, redirects followed blindly.

Severity

blocker for anything exploitable as merged: secret leak, missing authorization on a mutating path, injection sink. major for a real weakening of a boundary with a narrow precondition. minor for defence-in-depth. No praise unless the PR actually closed a hole.

Do not nit

No generic "consider adding validation". No threat models for code paths that cannot be reached by an untrusted caller. No CVE trivia about versions the repo does not use.

Evidence

Always attach evidence with the exact path and line range for the sink or the missing check. Security findings without a locus are hard to act on.