/meta-reviewing-api-reviewing

name: meta-reviewing-api-reviewing
description: Backend code review — injection and SSRF, boundary validation, object-level authorization, mass assignment and data exposure, error leakage, N+1 and unbounded queries. Use when a diff touches API routes or handlers, database access, auth middleware, or server code that reads files, starts processes or fetches URLs.

API Code Review Patterns

> **Quick Guide:** Review a backend diff's inputs and outputs before anything else in it: what enters without a schema, what reaches an interpreter or an outbound request as part of a string, which route answers without checking whose row it returns, and which fields leave in a response, a log or an error. Those are Must Fix and block approval; error text and query shape are Should Fix, addressed before merge without blocking it; the rest of the codebase's habits are not this diff's findings. Two facts change the verdict often enough to hold up front: a tagged `sql` template binds its interpolated values, so the injection is the untagged template literal; and an argument array keeps the shell out but not the program's own option parser.

**Detailed Resources:**

• [examples/core.md](examples/core.md) — before-and-after code for each pattern below, under the same headings

---

<critical_requirements>

Before reviewing API code

**Follow every value the diff reads from a request — body, params, query, headers, cookies — to the line that uses it.** The defect lives at the use: a handler that parses its body and then reads `limit` raw from the query string is unvalidated exactly where it matters.

**Treat request data reaching SQL, a shell, a file path or an outbound URL as Must Fix unless it arrives as a bound parameter, an argument-array element that cannot start with `-`, a path confirmed inside its base directory, or an allowlisted value.** Spliced into a string something else parses, data becomes instructions, and nothing downstream can tell the two apart.

**For every route the diff adds or changes, find the guard that authenticates the caller and the condition that ties the row to them.** Authentication says who is asking; without the ownership condition, changing the id in the URL returns someone else's row.

**Read each response body, write, log line and error message the diff adds field by field.** A serialized row carries every column, a write bound to the request body accepts every column, and a logged request carries its credentials to everyone who reads the logs.

</critical_requirements>

---

**Auto-detection:** API review, backend PR review, endpoint review, route handler review, SQL injection, command injection, argument injection, path traversal, SSRF, IDOR, BOLA, broken object level authorization, mass assignment, excessive data exposure, error leakage, secrets in logs, N+1 query, unbounded query

**Applies to:**

• A diff that adds or changes route handlers, middleware, the queries behind them, database schema changes, or ORM usage
• Authentication, role checks and row ownership
• Server code that reads files, starts processes or fetches URLs using request data
• What responses, log lines and error messages carry out of the server

**Handled elsewhere:**

• Severity vocabulary, the cost gate and the approval decision — the general review discipline; this skill only places API defects on it.
• UI in the same diff — rendering, client state and accessibility.
• Pipelines, container images and deployment configuration, including the secrets they hold.
• Model calls, prompt assembly and handling of model output.
• How the config module itself loads and validates the environment.
• Throughput work the diff's workload does not call for — caching layers, rate limits, query plans on tables that stay small.

---

<philosophy>

Philosophy

A UI defect reaches the user who triggers it; a server defect reaches every row the server can read. So a backend diff's inputs and outputs are reviewed before its structure, and each route gets two questions asked separately — who may call this, and may they touch this row — because the second is the one that goes missing. Error paths get the same reading as the happy path: what a failure returns is part of the contract, and it is where internals leak.

</philosophy>

---

<decision_framework>

Severity for API findings

Does request data reach an interpreter, an outbound request, or another user's data —
or can a failure escape the error handler?
├─ Spliced into SQL, a shell command, a file path or a fetched URL → Must Fix
├─ A route with no guard, or a query with no ownership condition → Must Fix
├─ A write bound to the request body rather than to named fields → Must Fix
├─ Secrets, tokens or personal data in a response, a log line or source → Must Fix
├─ Request data used with no schema at the boundary → Must Fix
├─ A promise the handler neither awaits, returns nor gives its own .catch → Must Fix
│     (a task detached on purpose with its own .catch, or through the
│      codebase's documented fire-and-forget helper, is fine)
└─ NO → Does it fail under real input or real volume?
    ├─ Stack, SQL or driver text in a client-facing error → Should Fix
    ├─ A query per row in a loop over records that grow with usage → Should Fix
    ├─ A list endpoint with no bound on a growing table → Should Fix
    ├─ Writes that must land together, outside a transaction → Should Fix
    ├─ A status that contradicts the failure (200 wrapping an error, 500 for bad input) → Should Fix
    └─ NO → Is it an improvement the diff's purpose supports?
        ├─ Narrowing a schema that is already safe → Nice to Have
        └─ Rate limits, caching, a repository layer, an index on a table that stays small → don't write it

A performance finding names the workload that makes it real: a query per row over a user's orders grows with every order, and a missing index on a twelve-row lookup table never will.

</decision_framework>

---

<patterns>

Core patte