Grill my backend

Setup — read the shared protocol first

Before doing anything else, read these two files in full:

1. ${CLAUDE_PLUGIN_ROOT}/skills/_grill-shared/tone.md
2. ${CLAUDE_PLUGIN_ROOT}/skills/_grill-shared/protocol.md

They contain the tone, hard rules, catalog protocol, the interactive-vs-report fork, and the punch-list/report formats. The rest of this file specifies the backend-review flavor.

If either Read fails (file not found or permission denied), stop and ask the user where the shared grill files live (they ship in _grill-shared/ beside this skill). Do not improvise the protocol from memory — the whole point of the shared file is that it stays consistent across the family.

Also: this skill layers on top of grill-my-code. The general code-review library (architecture, correctness, code craft, testability, performance, distributed systems, observability, supply chain) is the foundation; this file adds the backend-specific principles a generic review tends to miss. Rule for naming an issue: pick the most specific principle that fits. If a backend principle below applies, use that; otherwise reach for grill-my-code's library. Don't invent hybrids; don't paraphrase.

Step 1 — Pick the target

Before reading anything, ask the user exactly this:

What backend surface am I grilling?

1. Current diff vs main (or another base branch) — I'll filter to backend-relevant changes
2. A specific file or directory — give me the path
3. A specific endpoint, query, migration, or job — name it and I'll find the code

And: anything off-limits? (generated code, vendored deps, files you already know are bad and don't want to hear about?)

And: if this code talks to a database or another service, point me at the schema/migration/contract too — I can't grill what I can't read.

Wait for the answer. Do not guess.

If they pick (1): run git diff <base>...HEAD (default base: main, fall back to master, then ask). Read the full diff and the surrounding context of each changed function — never review a diff hunk in isolation.

If they pick (2): read every file in scope, in full. Do not skim. Do not rely on memory of the codebase.

If they pick (3): use grep/file search to locate the endpoint / query / migration / job. Read the handler and its dependencies — the repository it calls, the model it operates on, the middleware that wraps it. A backend review that stops at the handler is a vibe-check.

Step 1.5 — Read the contract before the implementation

If the surface includes an HTTP endpoint, gRPC method, GraphQL resolver, or webhook handler:

1. Read the contract artifact — OpenAPI spec, proto file, GraphQL schema, route table, type-level API definition, or the test file that documents the expected shape.
2. Read the relevant schema — the migration that created the touched table, or the model definition.
3. Read the consumer if it lives in this repo — SDK, client library, frontend caller, integration test. A "contract" not validated against a consumer is just a spec lying to itself.

The contract is what consumers depend on; the implementation is what's actually true. Mismatches are usually CRITICAL when consumers exist outside the repo (public API, shipped SDK, mobile build, third-party integration). For internal-only or never-shipped endpoints, demote to HIGH. Apply the protocol's tier definitions, not gut feel.

If no contract artifact exists at all, raise that under Contract drift — but don't make it the only issue you raise.

Step 2 — Catalog, choose mode, grill or report

Now follow protocol.md Phases A → B → (C or D) → E.

Three backend-flavor adaptations during the catalog:

• Severity calibration. Backend code is where CRITICAL actually means CRITICAL: data loss, auth bypass, tenant cross-talk, irreversible migrations already in prod. The protocol's tier definitions apply unchanged — don't soften them out of politeness because the code is "just a script". But also don't inflate: "no API versioning" or "no max page size" are usually HIGH, not CRITICAL, on internal endpoints. "Stack trace leaks in error response" is HIGH by default and CRITICAL only when secrets or PII bubble. "Cache stampede" is HIGH unless the cache is acting as source-of-truth for an auth/balance/quota decision.
• Concrete consequence must name the failure trigger. "Could cause inconsistency" fails the Phase A test. "The second time the worker retries this job, the Stripe charge ships twice because the handler is keyed on order_id but the idempotency key is regenerated per attempt" passes. Backend bugs almost always have a specific trigger — name it.
• Most-specific principle wins. A backend principle below + a general principle in grill-my-code will often both fit. Use the more specific name (the backend one). If only grill-my-code has a name for it (race condition, error swallowing, partial write across stores), use that. Don't bundle two principles into one finding.

Backend-review principle library

Use these names. Don't paraphrase them into mush.

API contract & HTTP semantics

• Contract drift — the handler returns a shape, status code, or field set that the OpenAPI/proto/schema/route doc claims it doesn't (or claims it does and doesn't). Consumers built against the contract will break.
• Status code misuse — 200 OK returned with {error: ...} body; 500 for client-input errors; 204 with a body; 201 Created without a Location header; 200 on a successful DELETE that returns the deleted entity.
• Verb misuse — GET that mutates; POST for an idempotent read; PATCH that replaces the entire resource; PUT that partially updates.
• Missing idempotency key on mutating POST — payment creation, order submission, anything where "retry" must not double-fire, lacks an Idempotency-Key header path or server-side dedupe.
• Unsafe deserialization on request bodies — request payload parsed by any deserializer that instantiates arbitrary classes from the wire format: Python's stdlib binary serializer, yaml.load (not safe_load), Java ObjectInputStream, .NET BinaryFormatter. RCE on every request.
• Permissive schema validation — request body validated with extra="allow" (Pydantic), no additionalProperties: false (JSON Schema), or no validation at all. Unknown fields silently accepted; mass-assignment and protocol-confusion attacks land.
• Pagination without max page size — ?limit= accepts arbitrarily large values; a single bad client paginates the entire table into one response.
• Pagination shape inconsistent across endpoints — /users uses ?page=&limit=, /orders uses cursor, /events uses offset; clients can't share pagination code, and silent shape changes ship as "fixes".
• Bulk-action atomicity unspecified — POST /users/bulk_delete with 1000 IDs; halfway through, one fails; the contract doesn't say whether the others rolled back, were skipped, or partially applied. Whichever the implementation does, callers can't trust it.
• Error response inconsistency — sometimes {error: "x"}, sometimes {errors: [...]}, sometimes a plain string, sometimes a stack trace. Clients can't write one error handler.
• Internal-detail leakage in errors — SQL fragments, internal paths, stack traces, library names, vendor error codes bubbled to the public response. Free recon for an attacker; CRITICAL only when secrets or PII bubble (raw stack traces alone are HIGH).
• No versioning strategy — public endpoint without /v1 or Accept header negotiation; a breaking change ships and existing consumers are silently broken.
• CORS misuse — Access-Control-Allow-Origin: * paired with Allow-Credentials: true; CORS configured at app layer when the edge could enforce it; preflight ignored.
• CSRF defenses absent — state-changing endpoint that accepts session cookie auth without a CSRF token, double-submit cookie, or SameSite enforcement.

AuthN / AuthZ & multi-tenancy

• IDOR (Insecure Direct Object Reference) — endpoint accepts an ID and acts on the row without verifying the caller owns it (or has explicit access). GET /orders/42 returns anyone's order.
• Authz duplicated per handler instead of enforced once — auth checks copy-pasted across handlers. Adding the next endpoint requires remembering the check; sooner or later someone forgets. Lift to middleware, a domain service, or a policy object.
• Tenancy at query time only — tenant_id injected by the ORM at query time but no row-level security / no enforced filter at the connection boundary. One missed where tenant_id = ? and tenants see each other's data.
• Mass-assignment authz bypass — model accepts arbitrary fields from request body (User.update(request.json)); a client sets is_admin=true and the framework happily updates it.
• Privileged operation gated by client-supplied flag — if request.json.get("is_admin") instead of if caller.is_admin. Client lies; server believes it.
• AuthN passed for authZ — @login_required on an endpoint that also needs a role check; "they have a session" is treated as "they're allowed". Two decisions, two checks — the second one is missing.
• JWT pitfall — alg: none accepted; signing key reused across environments; no exp; no revocation list; refresh token stored in a place the attacker can read; secret committed to repo.
• Session not rotated on auth-state change — login, logout, role escalation, password change all reuse the same session ID. Session fixation is now a viable attack.
• Cookie attribute hygiene — auth/session cookie set without Secure, HttpOnly, or a sensible SameSite; Domain set too broadly (e.g. .example.com exposes the cookie to every subdomain, including untrusted ones); cookie shipped over plaintext in dev and the dev habit leaks to prod.
• Signed-URL / token scope & TTL — presigned upload URL valid for 7 days with no IP / method / object-key scoping; password-reset token that never expires, isn't single-use, or isn't invalidated after a successful reset. The token is the credential — treat its lifetime and scope accordingly.
• Secret in URL query — API key or session token passed as ?token=...; it lands in access logs, CDN logs, browser history, referrer headers.
• Service-to-service auth missing or unscoped — internal services trust any caller on the private network, or share one shared-secret token across all of them. One compromised service = lateral movement to all.

Persistence, transactions & ORM

• Transaction boundary too wide — entire HTTP request inside a single transaction; the transaction holds row locks across slow I/O (an external API call, a queue publish). Connection pool starves under load.
• Transaction boundary too narrow — multi-step write done as separate transactions when atomicity was the entire point. Step 2 fails, step 1 has already committed, system is now inconsistent.
• Isolation level assumed — code treats READ COMMITTED as SERIALIZABLE; a "check-then-insert" pattern is wide open to phantom rows.
• ORM lazy-load N+1 — list view iterates for user in users: print(user.profile.name); one query becomes 200 because profile is a lazy relation. The performance issue is invisible until prod load.
• Implicit cascade — ON DELETE CASCADE (or ORM-level dependent: :destroy) silently nukes a forest of related rows; no audit trail, no soft-delete check, no warning on the row count.
• Connection pool exhaustion path — long-running query, no statement timeout, no per-request budget. One slow query degrades the entire service.
• Pessimistic vs optimistic locking absent — concurrent updates on the same row use last-write-wins; the user who saved second silently overwrites the first user's changes.
• Replica routing without read-your-writes — write goes to primary, immediate read goes to replica, replica hasn't caught up, code assumes the read sees the write.
• Soft-delete invariant break — deleted_at column added but unique constraints don't account for it (two "deleted" rows with the same email can exist, but two live rows can't, and the "create" flow doesn't filter); queries scattered across the codebase forget the where deleted_at is null.
• ORM/schema column drift — ORM model declares columns that don't exist in the deployed schema, or vice versa. Reads return the wrong shape or nil where data was expected; writes fail at runtime in prod after passing in dev.
• Read-after-write on eventually-consistent stores — code uploads to S3 and immediately lists the bucket; writes to DynamoDB with a strongly-consistent write but reads with the default eventually-consistent read; generates a presigned URL for an object that may not be visible to the next request. The replica-routing principle is RDBMS-shaped; this is the object-store / NoSQL shape.

Schema migrations & evolution

• Lock-acquiring DDL on a large table — ALTER TABLE users ADD COLUMN x NOT NULL DEFAULT 'foo' on a 50M-row table holds an ACCESS EXCLUSIVE lock; writes block for the duration. On Postgres CREATE INDEX without CONCURRENTLY has the same shape.
• Backfill without batching — one giant UPDATE to populate a new column locks the table for hours. The migration should chunk by primary key with sleeps between batches.
• No dual-write window for rename/restructure — column renamed from name to full_name in one migration; the old code reading name and the new code reading full_name cannot coexist during deploy. Either deploy is atomic (it isn't) or there's downtime.
• Non-reversible migration with no rollback plan — DROP COLUMN, DROP TABLE, TRUNCATE shipped as a forward migration; the down migration is empty or destructive. If we need to roll back the deploy, we can't.
• Foreign-key add without NOT VALID + VALIDATE phase — adding an FK to an existing populated table scans every row inside the schema lock; the safe shape is ADD CONSTRAINT ... NOT VALID then VALIDATE CONSTRAINT (or the equivalent in the DB at hand).
• Online-migration tool ignored — repo uses gh-ost / pt-online-schema-change / Liquibase online but this migration goes through raw ALTER. Skipped the safety net.
• Default-value backfill (DB-specific semantics) — the framework's "add column with default" may rewrite every row (MySQL pre-8.0) or be metadata-only (Postgres 11+). Don't guess; verify for the DB at hand.
• Stats not refreshed after migration — CREATE INDEX succeeds, but query planner stats are stale; the new index is never picked, queries still seq-scan, perf degrades silently. ANALYZE (or its equivalent) is absent from the migration.
• Down migration is a lie — the down method exists but doesn't restore data, doesn't restore constraints, or is pass. Treat that as "no rollback".

Background jobs & messaging

• Job not idempotent — worker assumes single delivery; broker guarantees at-least-once; duplicate side effects ship (charge twice, email twice, notify twice).
• Retry semantics unspecified — no max_attempts, no backoff strategy, no jitter, no dead-letter destination. Either the job retries forever, or it fails once and silently vanishes.
• DLQ absent or unwatched — failed jobs land in a dead-letter queue nobody reads; the failure mode is "silently lost work for weeks".
• Job ordering assumed where the broker doesn't guarantee it — code depends on event B arriving after event A; SQS standard, Pub/Sub, Kafka across partitions — none of them promise that.
• Scheduling drift / overlap — cron-style job that takes longer than its interval; runs pile up; or a job that runs simultaneously on multiple workers when it should be single-flight.
• Cron timezone / DST trap — schedule defined in server-local time on a host that observes DST; "every day at 02:30" runs zero times or twice on transition days. Or the cron string is UTC but the operator was thinking in their local time. (Distinct from grill-my-code's clock skew — this one is calendar drift, not node disagreement.)
• Worker pool not isolated by job class — one slow/expensive job class starves the queue for everything else; no separate worker pool, no priority lane.
• Stale payload in job — job receives a snapshot of an entity at enqueue time; by the time the worker runs, the entity has changed. The job should carry an ID and refetch.
• Synchronous work where async was the right call — handler does the expensive thing inline (sends 12 emails, makes 4 API calls) instead of enqueueing. Tail latency is now bounded by the slowest dependency.

Caching

• Cache stampede / dogpile — popular key expires; N concurrent requests all miss; all N hit the backend simultaneously; backend falls over. Needs single-flight, jittered TTL, or stale-while-revalidate.
• Invalidation strategy absent — write succeeds, cache still serves the old value; nothing in the write path invalidates or updates the cache.
• Cache as source of truth — code relies on the cache returning a fresh value to make a correctness decision (auth check, balance check, quota check). When the cache is wrong, the system is wrong.
• TTL mismatched to data change rate — user settings cached for 24h; user changes setting; UI lies for a day. Or: cache TTL is one minute on data that hasn't changed in a year (no benefit, all eviction churn).
• Negative caching missing — every 404 hits the database; an enumeration attack or a popular dead URL is now a DoS vector.
• Cache key collision — key built from f"user:{id}" where id is shared across types (user 42 and tenant 42 hash to the same slot); cross-type contamination.

External integration & webhooks

• No timeout on outbound call — requests.get(url) with no timeout=; default is forever. One sick vendor stalls every worker until the pool is dead.
• SSRF via user-controlled URL — server fetches a URL whose host/path is influenced by request input, with no allowlist and no metadata-endpoint block. Attacker hits 169.254.169.254 (cloud metadata), internal services on the private network, file://, or localhost admin ports. Pulls credentials and pivots.
• Path traversal / archive extraction — user-supplied filename joined into a filesystem path without realpath containment; uploaded zip/tar extracted without sanitizing entry names (zip-slip / tar-slip). One crafted upload overwrites arbitrary files.
• TLS verification disabled — outbound HTTPS calls with verify=False, rejectUnauthorized: false, sslmode=disable, or a CA bundle stubbed for "dev convenience". MITM is now an unsigned door.
• Retry without idempotency key forwarded — outbound retry on a POST /charges; no Idempotency-Key header carried; vendor charges twice.
• No circuit breaker / retry budget — flaky vendor returns 5xx on 30% of calls; retries quintuple the load on the already-sick vendor; you participate in their outage.
• Webhook signature not verified — endpoint accepts vendor webhooks but doesn't verify the HMAC / Ed25519 signature; anyone can spoof.
• Webhook replay protection absent — signature is verified but no timestamp tolerance, no nonce/dedupe. A captured webhook can be replayed forever.
• Webhook acks success before durably handling — handler returns 200 and then does the work; the worker crashes mid-handler; vendor sees success and stops retrying; event is lost. Either persist-then-ack, or do the work inline before responding.
• Vendor pagination not exhausted — code reads the first page of a paginated API and assumes that's all. Customer with >100 records gets silently truncated.
• Synchronous coupling to a flaky vendor — signup blocks on a third-party email-verification or KYC call. Vendor outage = signup outage.
• API version pinning absent — code calls https://api.vendor.com/v1/foo with no version pin in headers, or worse, calls /latest. Vendor ships breaking change; you find out via PagerDuty.

Backend ops, rollout & safety

• Request / correlation ID missing — no X-Request-ID propagated; no traceparent; impossible to follow a single user request through services. On-call has nothing to grep.
• Audit log absent on privileged ops — admin changes a role, edits another user's data, exports PII — no record. When compliance asks "who did this", you don't know.
• Sensitive data in logs — PII, full request bodies, auth tokens, full card numbers, password fields logged at INFO. Logs are a liability now.
• Health check that lies — /health returns 200 unconditionally; load balancer keeps routing traffic to a node whose DB is unreachable. Real health checks probe the dependencies they care about.
• Graceful shutdown missing — SIGTERM kills in-flight requests instead of draining; deploys = brief 502 storm.
• Rate limit at wrong granularity — 100 req/sec is per-process but you have 20 processes (real limit is 2000); or rate limit is global where it should be per-user; or there's no rate limit on the obvious abuse surfaces (login, password reset, search, signup).
• Secret in code or unrotatable env — credential committed to repo, or in an env var with no rotation story, no expiry, no scope. When it leaks, the blast radius is "everything".
• Service account over-privileged — one shared credential with full DB access used by every service. Steal it once = full read/write on everything.
• No kill switch / feature flag on risky change — new code path shipped 100% on day one; no flag to disable it without a redeploy.

Plus the general library

When a finding fits better under one of grill-my-code's general principles — race condition / TOCTOU, error swallowing, unbounded resource use, trust boundary violation, at-least-once vs exactly-once, partial write across stores, broken trace propagation, no structured logging at trust boundaries, unpinned dep, etc. — use that name. Backend-specific principles win only when they're a sharper match.

Pick the principle that fits. If none fits, the issue probably isn't real — drop it.

A note on grilling backend code

Frontend bugs degrade experience; backend bugs corrupt state. That asymmetry should land in severity. A handler that returns the wrong shape under a rare edge case is HIGH (consumers break, but state is intact). A migration that runs once and drops a column is CRITICAL (state is gone). A leaked auth token in a log line is CRITICAL even if nobody has read it yet — the cost is in what's now possible, not in what's already happened.

When in doubt, ask: what becomes irreversibly true if this ships? If the answer names a state change you can't take back (data deleted, secret rotated by an attacker, token issued, charge captured), the issue is CRITICAL. If the worst case is "one customer sees a stale value for 30 seconds," it's MEDIUM.