---
name: security-lens
description: Security review checklist for a PR diff — injection, authn/authz, secrets, crypto, SSRF/path traversal, deserialization — with the data-flow test each finding must pass. Load when the diff touches a risk path and no @security subagent was delegated.
---

# Security lens

The `@security` subagent exists for large or heavily security-sensitive diffs.
On `lite` and most `full` PRs you do this inline — the fan-out isn't worth it.
This is that checklist.

## The test every finding must pass

Before reporting, establish a **source → sink** path:

- **source** — where attacker-influenced data enters (request param, header,
  body, uploaded filename, env in a multi-tenant context, DB row that a user
  wrote, webhook payload, PR/issue text)
- **sink** — where it does something (SQL string, shell, filesystem path, HTTP
  request, deserializer, template, redirect, HTML)
- **no effective sanitizer between them** — check for one; grep the helper it
  calls. "There might be no validation" is not a finding.

If you cannot name the source and the sink, you have a code smell, not a
vulnerability. Report it at `low` or not at all. A false `critical` costs more
trust than a missed `low` costs risk.

## Checklist

**Injection**
- SQL/NoSQL built by concatenation or f-string/template interpolation → must be
  parameterized. An ORM `raw()`/`literal()` call is the usual escape hatch.
- Shell: `subprocess` with `shell=True`, backticks, `exec`, `system` on anything
  derived from input. Argument-array form is the fix.
- Template injection (Jinja/ERB/Handlebars rendering a user-supplied *template*,
  not just user data).
- LDAP, XPath, header injection (CRLF in a redirect/`Set-Cookie`).

**AuthN / AuthZ**
- A new endpoint/handler/route with no auth decorator/middleware its siblings
  have. Grep a neighbouring route to see the house pattern.
- Authorization that checks *authentication* only — "is logged in" where it needs
  "owns this record". IDOR: an object fetched by an id from the request with no
  ownership predicate.
- Secret/token/password compared with `==` → constant-time compare.
- Session/JWT: missing expiry check, `alg: none` accepted, signature verify
  skipped, token in a URL or a log line.

**Secrets**
- Literal keys, passwords, private keys, connection strings in the diff — even
  in tests or fixtures, if they're real.
- Secrets reaching logs, error strings, telemetry, or a URL query.
- A secret added to a client-side bundle or a container image layer.

**Crypto**
- MD5/SHA1 for anything security-bearing; unsalted password hashing (needs
  bcrypt/scrypt/argon2).
- `Math.random()` / `random.random()` for tokens, IDs, or nonces → CSPRNG.
- Hand-rolled crypto, ECB mode, a static/reused IV, a hardcoded salt.

**SSRF / path traversal / upload**
- A URL from input fetched server-side with no allow-list → SSRF (cloud metadata
  endpoints are the classic target).
- A path built from input reaching the filesystem with no containment check.
  A `..` check alone isn't enough — the resolved path must be verified inside the
  intended root.
- Archive extraction without a traversal/symlink check (zip-slip / tar-slip).
- Uploads trusted by client-supplied filename or `Content-Type`.

**Deserialization & parsing**
- `pickle`, `yaml.load` (not `safe_load`), Java native deserialization,
  `unserialize`, `eval`/`Function` on input.
- XML without external-entity handling disabled (XXE).

**Supply chain & CI**
- A new dependency: is the name plausible, or a typosquat of a known package?
- A pinned version loosened to a range, or an integrity hash dropped.
- CI changes: a workflow gaining access to secrets on an untrusted trigger, a
  third-party action pinned to a mutable tag rather than a SHA, a step that runs
  PR-authored code in a privileged context.

**Web**
- Reflected/stored XSS: `innerHTML`, `dangerouslySetInnerHTML`,
  `v-html`, `|safe` on input.
- CSRF protection removed or an endpoint switched from POST to GET.
- CORS widened to `*` alongside credentials.
- An open redirect from a `next`/`return_to` parameter.

## Severity for security findings

- `critical` — exploitable now by an unauthenticated or low-privileged actor:
  injection with a reachable sink, auth bypass, a live secret, RCE.
- `high` — exploitable with a precondition (a specific role, a race, a
  non-default config), or a secret in a log.
- `medium` — real weakness, no demonstrated path: missing defence in depth,
  weak crypto not currently load-bearing.
- `low` — hardening.

Cite an authoritative URL in `reference` when the finding turns on a specific
CVE or a documented library footgun. Don't cite a generic OWASP page for a
generic point, and never invent a URL.