/meta-reviewing-api-reviewing

name: meta-reviewing-api-reviewing
description: Backend code review patterns. Use when reviewing API routes, database operations, auth middleware, and server utilities. Covers injection, boundary validation, authorization coverage, secret/PII exposure, error leakage, and query patterns.

API Code Review Patterns

> **Quick Guide:** When a diff touches server code, trace every external input to where it is used - it must pass schema validation at the boundary and never reach a query or shell as a concatenated string. Verify every new route names its auth expectation and checks object-level access. Check what errors and logs expose. Security findings outrank everything else in the diff.

---

<critical_requirements>

CRITICAL: Before Reviewing API Code

> **All code must follow project conventions in CLAUDE.md** (kebab-case, named exports, import ordering, `import type`, named constants)

**(You MUST trace every external input in the diff - body, params, query, headers - to its use, verifying schema validation at the boundary)**

**(You MUST verify no user input is concatenated into SQL, shell commands, or file paths - parameterized queries and validated paths only)**

**(You MUST verify every route the diff adds declares its authentication requirement and checks authorization for the object it touches)**

**(You MUST check that secrets, tokens, passwords, and PII do not reach logs, error responses, or client payloads)**

**(You MUST verify error handling in the diff returns intentional messages - no stack traces or raw driver errors to the client)**

</critical_requirements>

---

**Auto-detection:** review API, backend PR review, route review, endpoint review, database query review, auth middleware review, server code review

**When to use:**

• Reviewing diffs containing API routes or handlers
• Reviewing database queries, schema changes, or ORM usage
• Reviewing authentication/authorization middleware or session handling
• Reviewing server utilities that touch external input, files, or child processes

**When NOT to use:**

• When implementing backend code (use the relevant API implementation skill)
• For UI components in the same diff (use the web reviewing skill)
• For CI/CD pipelines and deployment configs (use the infra reviewing skill)

**Key patterns covered:**

• Injection review: SQL, shell, and path traversal
• Boundary validation with schemas
• Authentication and object-level authorization coverage
• Secret and PII exposure in logs and responses
• Error responses that don't leak internals
• Query patterns: N+1 and unbounded reads the diff introduces

**Detailed Resources:**

• [examples/core.md](examples/core.md) - Good/bad backend patterns to look for during review

---

<philosophy>

Philosophy

**Server code is the trust boundary.** A UI bug annoys one user; an injection or authorization gap exposes every user's data. Review the diff's inputs and outputs before its style: what enters unvalidated, and what leaves that shouldn't.

**When reviewing API code:**

• Follow the data: entry point → validation → use → response, for each input the diff adds
• Assume every request is hostile until a schema says otherwise
• Ask "who may call this?" and "may they touch THIS row?" for every new route - the second question is the one that gets missed
• Read the error paths as carefully as the happy path; that is where internals leak

**When NOT to flag:**

• Don't demand rate limiting, caching, or pagination the spec never asked for on an internal or low-traffic endpoint
• Don't demand a repository/service layer around a query the codebase writes inline everywhere else
• Don't flag missing observability on code following the file's existing logging pattern
• Don't rank a hypothetical scale problem above a real correctness issue in the same diff

**Core principles:**

• **Validate at the boundary**: inside the handler, data is typed and trusted because the schema ran, not because the client is polite
• **Authorization is per-object**: authentication says who you are; the query must still scope to what you own
• **Errors are API surface**: what a failure returns is part of the contract
• **Performance findings need a workload**: an N+1 in a loop over user data is real; a missing index on a ten-row table is not

</philosophy>

---

<patterns>

Core Patterns

Pattern 1: Injection Review

No external input reaches an interpreter as a string fragment.

## Injection Review

For EACH place the diff sends data to SQL, a shell, or the filesystem:

- [ ] SQL uses parameterized queries or the ORM's binding - no template literals with user input
- [ ] Shell commands use argument arrays (execFile/spawn), never string-built exec with input
- [ ] File paths derived from input are validated against a base directory (no ../ traversal)
- [ ] Dynamic column/table names come from an allowlist, not from the request

// Must Fix: classic injection
const rows = await db.query(
  `SELECT * FROM users WHERE name = '${req.query.name}'`,
);

// Good: parameterized
const rows = await db.query("SELECT * FROM users WHERE name = $1", [
  req.query.name,
]);

**Why this matters:** String-built queries and commands turn any input field into an execution vector. This is always a blocking finding, regardless of how internal the endpoint seems.

---

Pattern 2: Boundary Validation

Every input the diff reads gets a schema before it gets used.

## Validation Review

For EACH route or handler in the diff:

- [ ] Body, params, and query are parsed through a schema (Zod or the codebase's equivalent) before use
- [ ] Validation failures return 400 with a safe message - not a 500 from downstream
- [ ] The schema is as narrow as the contract: enums for enums, bounds on numbers, formats on ids
- [ ] Handler code reads the schema's OUTPUT type, not the raw request

// Should Fix: trusts the wire shape
const { limit } = req.query;
const items =