secure-code-review

• SKILL.mdcore · 5,347 bytes
• SKILL.mdcore · 5,347 bytes
• metadata.jsonasset · 1,437 bytes
• references/verifying-compliance-findings.mddoc · 3,971 bytes
• references/verifying-findings.mddoc · 3,310 bytes

<!-- Native skill bundle for Claude Code. Generated by `secure-vibe dev regenerate`. -->
<!-- Do not edit by hand; the source of truth is skills/secure-code-review/SKILL.md. -->

# Secure Code Review

The sinks and boundaries that belong to no framework: passing input to a shell or an evaluator, letting input choose a filesystem path, validating at a boundary that is not an HTTP request, and handling regulated data (PII, PHI, cardholder data) so it does not spread into paths nobody reviews. Use when generating or reviewing code that shells out, touches the filesystem, reads a queue message, CLI argument, or parsed file, or when a change crosses a PII, PHI, or PCI boundary.

## ALWAYS

- Validate input at **every** trust boundary, not only the HTTP one. A CLI argument, an environment variable, a queue message, a filename, a parsed config or spreadsheet cell all arrive from outside and none of them pass through the request-validation layer. Check type, length, allowed characters and allowed range, and reject before processing. `api-security` owns the HTTP request body and its schema; this rule is every other way input gets in.
- Build a command as an **argument vector**, never as a string a shell will parse: `subprocess.run([...])` without `shell=True`, `execFile`/`spawn` with an array, `exec.Command(name, args...)`. Passing an array is what removes the shell, and removing the shell is what removes the injection — quoting and escaping are the fallback for when you cannot.
- **Canonicalize then check** any path built from input: resolve it to an absolute real path, then confirm it is inside the intended directory. Checking the raw string first misses `..`, symlinks, absolute-path replacement, and encoded separators. `file-upload-security` owns stored uploads and archive extraction; this rule covers a static-file handler, a template loader, a log path, an export filename.
- Tag any function or module handling **PII, PHI or cardholder data** with an explicit classification comment (`// classification: PII`), and keep that handling in clearly-named modules rather than in general utility code. An untagged helper is how regulated data spreads into paths nobody reviews for it, and the tag is what makes the later review possible.
- Consult the owner for anything with one: `database-security` when code builds a query, `crypto-misuse` when it hashes, encrypts, signs or compares a secret, `frontend-security` for output encoding and response headers, `auth-security` for who the caller is, `iam-best-practices` for the privileges a process runs with. Each of those states its rule with the API names and the false positives this skill would have to leave out.