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

docs: mark SEC_REVIEW F12 as fixed in 4006743

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

+ 24 - 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 (6 fixed, 21 open), 42 sev-1.
+> **Findings rolled up:** 5 sev-3 (5 fixed, 0 open), 27 sev-2 (7 fixed, 20 open), 42 sev-1.
 
 ---
 
@@ -351,6 +351,29 @@
   fix flipping `is_local`) could be matched on local-admin login,
   binding the local-admin password to the wrong identity.
 - **Severity: 2**
+- **Status:** Fixed in `4006743`. The display-name match was already
+  removed by the F3 fix (`8a94dff`) — `findLocal()` looks up by
+  `is_local = 1` and the partial unique index `uniq_users_one_local`
+  enforces "at most one local row" at the DB. F12's residual concern
+  (a row reaching the state `is_local=1 AND subject IS NOT NULL` via
+  direct DB tampering) is now addressed in two layers:
+  1. **Application:** `UserRepository::findLocal()` adds
+     `AND subject IS NULL` to the query. A row with `is_local=1` but a
+     non-null OIDC subject is structurally not a local-admin row and
+     will not bind the local-admin password — `upsertLocal` then
+     trips the unique-index INSERT, surfacing the tamper as a 500.
+  2. **DB:** Migration
+     `20260505100000_add_users_local_subject_invariant` enforces the
+     same predicate at the storage layer — MySQL via a `CHECK
+     (NOT (is_local = 1 AND subject IS NOT NULL))` constraint, SQLite
+     via paired `BEFORE INSERT` / `BEFORE UPDATE` triggers that
+     `RAISE(ABORT)` on the violating predicate. So a "data fix"
+     script attempting `UPDATE users SET is_local = 1 WHERE id = …`
+     against an OIDC row fails at the DB before any login can bind.
+  Regression tests in `api/tests/Integration/Auth/AuthEndpointsTest.php`
+  (`testDbLayerRejectsInsertingLocalRowWithNonNullSubject`,
+  `testDbLayerRejectsFlippingIsLocalOnOidcRow`,
+  `testFindLocalIgnoresHijackedRowEvenIfDbConstraintIsBypassed`).
 
 ### F13 — Service token rotation leaves the old hash valid indefinitely
 - **File:** `api/src/Infrastructure/Auth/ServiceTokenBootstrap.php:65-89`