Browse Source

docs: mark SEC_REVIEW F51 as fixed in 9c0fef5

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 3 days ago
parent
commit
d336550c19
1 changed files with 14 additions and 1 deletions
  1. 14 1
      doc/SEC_REVIEW.md

+ 14 - 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 (27 fixed, 0 open), 42 sev-1 (18 fixed, 24 open).
+> **Findings rolled up:** 5 sev-3 (5 fixed, 0 open), 27 sev-2 (27 fixed, 0 open), 42 sev-1 (19 fixed, 23 open).
 
 ---
 
@@ -1686,6 +1686,19 @@
   `array_filter($groupIds, 'is_string')` to make the contract
   explicit.
 - **Severity: 1**
+- **Status:** Fixed. `RoleMappingRepository::resolveRole` now does
+  `array_values(array_filter($groupIds, 'is_string'))` as the first
+  step, so the `count($groupIds)` placeholder generation and the
+  bind-list always agree even when a future caller passes a mixed
+  array, a hash with skipped indexes, or anything else that violates
+  the PHPDoc `list<string>` contract. After filtering, the empty
+  case correctly falls back to the default role. Regression tests
+  in `api/tests/Integration/Auth/RoleMappingRepositoryTest.php`:
+  - happy-path highest-role-resolution and default-fallback,
+  - non-string entries are filtered, retaining valid string IDs,
+  - all-non-string list collapses to the default,
+  - a hash with non-contiguous keys (`[5 => 'group-x', 12 => '…']`)
+    is re-keyed and the IN clause still works.
 
 ### F52 — Reporter / consumer / category `name`/`reason` accept control characters
 - **Files:** `api/src/Application/Admin/ReportersController.php:69-72`,