Quellcode durchsuchen

docs: mark SEC_REVIEW F4 and F5 as fixed in 8d948ae

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa vor 5 Tagen
Ursprung
Commit
5a749338b2
1 geänderte Dateien mit 33 neuen und 1 gelöschten Zeilen
  1. 33 1
      doc/SEC_REVIEW.md

+ 33 - 1
doc/SEC_REVIEW.md

@@ -11,7 +11,7 @@
 >
 > Each finding is referenced as **F<N>** for later citation.
 >
-> **Findings rolled up:** 5 sev-3 (3 fixed, 2 open), 27 sev-2, 42 sev-1.
+> **Findings rolled up:** 5 sev-3 (5 fixed, 0 open), 27 sev-2, 42 sev-1.
 
 ---
 
@@ -98,6 +98,25 @@
   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`
@@ -110,6 +129,19 @@
   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`.
 
 ---