# Security Review

> Review a code change or a file for security issues — secret leaks, injection, auth bypass, deserialization, common OWASP hits, and dependency risks. Triggers on "security review", "review this for security", "check for vulns", "did I leak a secret", or before pushing anything that touches auth / crypto / user input / SQL / shell / eval.

- Skill: `duckcode-js/security-review` (Agent Skill, multi-file: 4 files)
- Install (CLI): `npx skillmds@latest add duckcode-js/security-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/duckcode-js/security-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Data & Analytics
- Author: DuckCode-js (https://skillmd.com/u/duckcode-js)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/duckcode-js/security-review

---


# security-review

Focused security review of a diff, a file, or a small module. Not a
comprehensive audit — that's an engagement. This finds the classes of bug
that regularly ship because nobody looked, and it's specific about what it
did and did not check.

## When to use this

- "Review this for security"
- "Check for vulns"
- "Did I leak a secret in this diff"
- Before pushing / opening a PR that touches: auth, session, crypto, user
  input, SQL, shell exec, file paths from users, deserialization, `eval`,
  regex on untrusted input, or third-party API calls with secrets
- After bumping a dependency to a version with a known CVE (`npm audit`,
  `pip-audit`, `cargo audit`)

Not for: comprehensive audits of an entire codebase (too big for a skill;
scope carefully), threat modeling, cryptographic protocol design, or
regulated-compliance certification. Say so if asked.

## Procedure

### 1. Establish scope

Ask for or infer the exact scope. Options in priority order:

1. A named file or files.
2. A branch diff: `git diff <base>...HEAD`.
3. Staged changes: `git diff --cached`.
4. A directory or module.

Do NOT try to review a whole repo in one pass. If the scope is too large
(more than ~500 changed lines or ~10 files touching sensitive areas), say
so and offer to review the most sensitive slice first.

### 2. Run the fast, cheap checks first

Before deep reading, do the mechanical scans. These are cheap and catch
the highest-severity, dumbest bugs.

**Secrets in diff:**

- `git diff <range> | grep -iE '(api[_-]?key|secret|token|password|bearer|BEGIN [A-Z ]+PRIVATE KEY|xox[baprs]-|ghp_|sk-|AKIA[0-9A-Z]{16}|AIza[0-9A-Za-z_-]{35})'`
- Also check for `.env`-shaped strings: `[A-Z_]+=[^ ]+` in non-config files
- If anything hits, **halt** and report the exact lines. Do not continue
  until the user confirms whether it's real.

**Dependency CVEs:**

- Detect package manager from lockfile
- Suggest running: `npm audit`, `pip-audit`, `cargo audit`, `bundle audit`,
  or equivalent — don't run automatically; the user does it
- If the diff bumps a dependency, check the version bump for a corresponding
  advisory

### 3. Read for the OWASP-shaped classic bugs

Look at each changed file for these patterns. Note anything that matches;
don't guess when unsure.

**Injection:**

- **SQL** — string concatenation of user input into SQL, template literals
  with query fragments, missing parameterization. `execute("SELECT * FROM u
  WHERE id = " + id)` is a hit. Check for prepared statements / parameter
  binding / ORM usage.
- **Command / shell** — `subprocess.run(..., shell=True)` or backticks or
  `os.system` with user input. `shell=False` + list args is safe.
- **NoSQL** — `find({field: userInput})` without validation, especially
  if the field allows `$where` or operators.
- **XSS** — `innerHTML = userInput`, unescaped template output, missing
  `htmlspecialchars` / `escape`. React auto-escapes; `dangerouslySetInnerHTML`
  reverses that.
- **XXE / SSRF** — user-controlled URLs passed to `fetch` / `requests.get`,
  XML parsers without external-entity restriction.

**Auth / access control:**

- Missing authorization checks on state-changing endpoints
- IDOR: `GET /users/:id` returning any user's data without checking whether
  the caller can view that ID
- Session fixation, missing CSRF tokens on state-changing routes
- Weak password hashing (MD5, SHA1, plain SHA256 without salt)
- JWT with `alg: none`, JWT verification skipped, JWT secret hardcoded

**Crypto / data:**

- Home-rolled crypto instead of stdlib primitives
- ECB mode block cipher, static IV, missing MAC
- Insecure randomness for security purposes (`Math.random()`, `random.random()`)
- Timing-attack-vulnerable comparisons on secrets (`==` on tokens; should
  be constant-time compare)

**Deserialization:**

- `pickle.load` on untrusted input, `yaml.load` (not `safe_load`),
  `unserialize`, Java `readObject` on external data — all remote-code-exec
  waiting to happen

**File & path:**

- Path joining with user input without normalization → path traversal
  (`../../etc/passwd`)
- File upload endpoints that don't check extension AND content type AND
  filename length
- Symlink attacks if the code follows symlinks in user-controlled dirs

**Race conditions on security-relevant state:**

- TOCTOU: check-then-use with a gap (e.g. check file permissions, then read)
- Non-atomic "check if user exists, then create user" without unique constraint

### 4. Write the findings

For each real finding, structure it:

```
### <severity> · <one-line title>

**File / line:** path/to/file.py:42

**What:** <one-sentence description of the bug>

**Why it matters:** <what an attacker could do with this>

**Fix:** <concrete change — code or approach>

**Confidence:** <high | medium | low — how sure you are this is real>
```

Severity:
- **Critical** — direct compromise of secrets, data, or system integrity
- **High** — meaningful data exposure or bypassable auth
- **Medium** — exploitable under some conditions; defense-in-depth failure
- **Low** — style / hygiene / hardening opportunity

Order the findings most-severe first. If there are none, say so — do not
manufacture findings. "No security issues found in the reviewed scope
(<what you looked at>). Not reviewed: <what you didn't cover>." is a valid
and honest report.

### 5. Say what you did and did NOT check

End with an explicit scope reminder. Reviewers of your review need to know
what you covered:

```
Reviewed: <files, diff range, or module>
Did not review: <what you deliberately skipped and why>
Not in scope: <things a security review can't verify without running
              the code / a specialist audit — e.g. cryptographic proof,
              side-channel analysis, business logic soundness>
```

## Anti-patterns

- Do not run any exploit or PoC yourself. Describe how the bug would be
  exploited in the write-up; do not demonstrate.
- Do not report "you should use HTTPS" or other generic hygiene as a finding
  unless it's actually violated in the code you reviewed.
- Do not upgrade the confidence of a finding to make it sound scarier. "Low
  confidence, might be a bug" beats a false positive that erodes trust.
- Do not review your own code with less rigor. If you wrote the change
  earlier in the conversation, apply the same procedure.