Просмотр исходного кода

docs: mark SEC_REVIEW F17 as fixed in 57ab1ba

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 5 дней назад
Родитель
Сommit
9339948cf1
1 измененных файлов с 41 добавлено и 1 удалено
  1. 41 1
      doc/SEC_REVIEW.md

+ 41 - 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 (5 fixed, 0 open), 27 sev-2 (9 fixed, 18 open), 42 sev-1.
+> **Findings rolled up:** 5 sev-3 (5 fixed, 0 open), 27 sev-2 (10 fixed, 17 open), 42 sev-1.
 
 ---
 
@@ -539,6 +539,46 @@
   `role`, `is_local`. No rate limit (F14), no audit (F5), no
   defensive sleep.
 - **Severity: 2**
+- **Status:** Fixed by F14 (rate limit) and read-side audit added in
+  `57ab1ba`.
+
+  1. **Rate limit.** F14 already attaches `RateLimitMiddleware` to
+     the `/api/v1/auth/*` route group, so a leaked service token is
+     capped at `API_RATE_LIMIT_PER_SECOND × 2` (default 60/s,
+     capacity 120) per token id — both `getUser/{id}` enumeration and
+     `upsert-{oidc,local}` abuse are bucketed on the same limiter.
+
+  2. **Read-side audit.** New `AuditAction::USER_FETCHED` constant
+     (`user.fetched`). `AuthController::getUser` now emits a
+     best-effort audit row on **both** the 200 (`outcome: found`,
+     `target_label = email|display_name`) and 404
+     (`outcome: not_found`, `target_label = null`) paths. The 400
+     malformed-id path stays silent — that's a protocol error, not a
+     valid-shape probe. `emit()` (best-effort) is used so a DB hiccup
+     on the audit insert does not 500 a UI session refresh; volume is
+     bounded by the F14 rate limit anyway. This is the only read
+     action recorded in `audit_log` — every other action recorded
+     there is state-changing per SPEC §4. The exception is documented
+     inline at the constant.
+
+  3. **No defensive sleep.** Skipped intentionally. The 200/404 bodies
+     differ structurally (`{"user_id": ...}` vs
+     `{"error":"not_found"}`), so existence is trivially detectable
+     from the response — timing is not the leak vector here. Adding
+     `usleep` would only impose latency on legitimate UI session
+     refreshes without strengthening the actual mitigation surface.
+
+  Regression tests in
+  `api/tests/Integration/Auth/AuthEndpointsTest.php`:
+  `testGetUserFoundEmitsUserFetchedAudit` (200 path emits row with
+  `outcome=found` and email label),
+  `testGetUserNotFoundEmitsUserFetchedAudit` (404 path emits row with
+  `outcome=not_found` and null label),
+  `testGetUserInvalidIdDoesNotEmitAudit` (malformed id stays silent),
+  `testEnumerationProducesOneAuditRowPerProbe` (a 5-id sweep produces
+  exactly 5 `user.fetched` rows — the SOC detection signal).
+  OpenAPI yaml + `openapi.php` document the audit emission and the
+  404 response.
 
 ### F18 — Containers run as root (no `USER` directive)
 - **Files:** `api/Dockerfile:42-43`, `ui/Dockerfile:46-47`