# Post-Implementation Production Hardening Audit

Use when reviewing code that has already been committed (a completed phase,
feature, or sprint) for systemic production gaps. This is NOT a pre-commit
review — the code has already landed and passed its own tests. The goal is to
catch cross-cutting concerns that single-PR reviews miss.

## When to use

- A feature/phase was just completed — review its actual implementation against
  production standards, not what the PR description claims
- The user says "review and improve" or "bring to production level"
- Multiple modules touch shared infrastructure (Redis, auth, proxy, config)

## Audit dimensions

Scan each dimension across ALL modules that touch it. Systemic gaps span files.

### 1. Atomicity / concurrency safety

- **Redis multi-step operations** — multiple sequential Redis commands (e.g.
  `ZREMRANGEBYSCORE` + `ZCARD` + `ZADD` + `EXPIRE`) race when multiple worker
  processes fire concurrently. Single-step atomic operations are required in
  gunicorn/uvicorn multi-worker deployments.
  - ✅ **Fix:** Wrap prune + count + add + TTL in a single Lua `EVAL` script on
    the Redis server. Use Redis `TIME` inside the script for consistent timing.
    Use unique sorted-set members (UUID) to prevent undercounting.
  - ⚠️ **Test gap:** Unit fakes don't validate Lua syntax. Flag this for a real
    Redis integration test or `fakeredis` with Lua support.

### 2. Authorization completeness

- **Cross-route consistency** — if one route checks JTI denylist (e.g., auth
  refresh/logout) but another route doesn't (e.g., stream resolve, debrid
  account CRUD), revoked tokens grant partial access.
  - ✅ **Fix:** Centralize JTI checking in a shared auth module
    (`app/core/auth.py`). Wire it into every `Depends()` that extracts user
    identity from a token, not just the auth module's own routes.
  - **Check every protected route** — list all routes that require
    authentication and verify each one rejects denylisted JTIs.

### 3. Proxy / client identity security

- **X-Forwarded-For spoofing** — an app that blindly trusts
  `request.headers["x-forwarded-for"]` lets external clients forge their IP,
  bypassing IP-based rate limits and corrupting audit metadata.
  - ✅ **Fix:** Validate `X-Forwarded-For` only when `request.client.host`
    matches a configured `TRUSTED_PROXY_IPS` list (supports CIDR notation). Use
    `ipaddress.ip_address()` / `ip_network()` for validation. Fall back to
    socket peer if proxy is untrusted.
  - **Nginx edge** — set `proxy_set_header X-Forwarded-For $remote_addr;` in
    the outermost reverse proxy to overwrite any client-sent header before it
    reaches inner proxies or the app.
- **Same helper everywhere** — ensure login/session metadata, IP-based rate
  limits, audit logs, and auth-adjacent IP extraction all use the same client-IP
  helper so they agree on identity.

### 4. Infrastructure configuration safety

- **Docker port binding** — if the app container binds `0.0.0.0:$PORT` (the
  default for most frameworks), it is accessible directly if Docker's published
  port bypasses the reverse proxy. There is no way to enforce proxy-only access
  at the host firewall without documentation every deployer must follow.
  - ✅ **Fix:** Bind the application to `127.0.0.1` inside the container
    (`--host 127.0.0.1`) so direct access is impossible even if published.
    Document that external access requires the proxy.
- **Dockerfile runtime flags** — flags like
  `--proxy-headers --forwarded-allow-ips "*"` in the Docker CMD override safe
  defaults for every deployment, not just the ones that know to override them.
  - ✅ **Fix:** Remove runtime header trust flags from the Docker image. Move
    trusted-proxy logic into application code driven by config env vars so
    deployments opt in explicitly.
- **Healthchecks** — must authenticate when the service requires auth. A
  `redis-cli ping` healthcheck that doesn't pass `-a $REDIS_PASSWORD` will
  report `NOAUTH` and keep a healthy service stuck as unhealthy.
- **App healthchecks** — prefer Python stdlib (`urllib.request` over
  `curl`/`wget`) to avoid depending on additional binaries in a slim image.

### 5. Readiness / liveness comprehensiveness

- **Feature-to-readiness mapping** — every runtime dependency that is
  auto-selected (not behind a separate feature flag) must have corresponding
  readiness coverage. If `REDIS_HOST` being set enables Redis for rate limiting,
  JTI denylist, AND provider cooldown, `/ready` must verify Redis connectivity
  when `REDIS_HOST` is configured — not just when the newest feature flag is
  true.
  - Rationale: A silent Redis outage degrades all three features simultaneously.
    Fail-open (proceed without Redis) is acceptable only if documented as
    degraded mode, not full health.
- **Coordination** — when a single health endpoint is composed of checks from
  different modules (e.g., DB check in shared, Redis check in core), ensure they
  aggregate correctly. A 503 from one subsystem should be surfaced as 503,
  not hidden by another subsystem's 200.

### 6. Documentation accuracy

- **Trace the runtime path** — env defaults, behavior description, and
  architecture claims in docs must match actual code behavior. Common mismatches:
  - Env variable default differs between `config.py` and docs
  - Docs claim a feature is "always active" but it requires an env flag
  - Architecture diagram shows a topology different from what deploy
    instructions produce
  - Tradeoff/known-issue docs don't reflect what was actually built

### 7. Data integrity / idempotency

- **(Only for mutable external integrations)** — Check that provider write
  operations (creating torrents, submitting metadata) are idempotent or matched
  with deduplication at the list level.

## Workflow

1. **List the review surface** — every file touched by the target feature/phase,
   plus every file that interacts with shared infrastructure (Redis, auth,
   proxy, config, health endpoints, Docker, Nginx, deployment docs).

2. **Audit each dimension above** — for each dimension, trace the implementation
  path through all affected modules. Record findings with file paths and line
  references.

3. **Prioritize by impact:**
   - SEVERE: security bypass, data loss, data corruption, privilege escalation
   - HIGH: availability degradation, race conditions in shared infrastructure
   - MEDIUM: documentation drift, missing tests, observability gaps
   - LOW: style, naming, non-functional preferences

4. **Fix each gap** — one change per gap, or fewest changes per gap when they
   share a fix (e.g., a single `get_client_ip()` helper fixes IP-spoofing for
   rate limits AND session metadata).

5. **Patch → Verify cycle** — after each fix (or small batch of related fixes),
   re-run lint + type + test. Fixing 5+ gaps then running verification once is
   harder to debug than fixing at most 2 gaps at a time and verifying after
   each cycle. If a fix breaks baseline, you know exactly which change caused
   it.

6. **Independent review** — dispatch the independent reviewer subagent
   (delegate_task) once all fixes are applied and baseline is green. This is
   the final gate.

7. **Independent reviewer unavailable (429 / timeout)** — do NOT stop. Run the
   static scan and self-review checklist from the main `requesting-code-review`
   skill (Steps 2 and 4) manually. Report to the user that an external review
   was attempted but could not complete, along with your own findings. Own the
   path even without the tool.

8. **Document** — update deployment docs, env var docs, and the project's
   NEXT_SESSION_START / PROJECT_CONTEXT as needed so the handoff reflects
   current reality. Run final full verification (lint + type + test + docker
   compose config if infra changed) and report the complete baseline.

## Pitfalls

- **Narrow scope** — reviewing only the diff from the latest commit misses
  pre-existing vulnerabilities that the new code depends on or inherits.
- **Single-module tunnel vision** — JTI enforcement on auth routes only but not
  on debrid/streaming routes is a cross-module gap that a file-level review
  won't catch.
- **Assuming docs are correct** — documentation is frequently written before or
  during implementation and not updated after the final change. Verify env
  defaults, architecture claims, and behavioral descriptions against the actual
  code.
- **Treating Redis readiness as feature-flag-scoped** — when REDIS_HOST is set
  and multiple features auto-enable from Redis, readiness must cover all of
  them. A separate flag like `REDIS_RATE_LIMITER=true` controls behavior, not
  readiness scope.
- **Proxy header trust in the image** — defaulting `--forwarded-allow-ips '*'`
  in the Dockerfile CMD means every deployment is insecure by default until the
  deployer knows to override it. Move header trust to app-level code gated by
  explicit config.