Преглед на файлове

fix: charset gate on AuditController *_kind filters (SEC_REVIEW F47)

The 128-char length cap from F31 already bounded `action`,
`entity_type`, `entity_id`, `subject_kind`, and `subject_id` against
multi-megabyte payloads, but the F47 finding also asked for an
allowlist regex on the `*_kind` fields. The audit emitter writes a
small finite vocabulary into `actor_kind` / `target_type` (`reporter`,
`consumer`, `admin-token`, `manual_block`, `oidc_role_mapping`, …);
anything outside that shape is bytes the column cannot hold and
shouldn't reach the prepared statement.

Add `AuditController::KIND_PATTERN = '/^[a-z0-9][a-z0-9_-]*$/'` and
apply it to `entity_type` and `subject_kind` (the two free-form
*_kind filters; `actor_kind` and `actor_via` were already in-array
allowlisted). Anything else — uppercase (`Reporter`), dotted
(`reporter.created`), spaced, CR/LF, leading-dash — gets a 400
`validation_failed` with the relevant field key in the details
envelope.

Regression test in
`api/tests/Integration/Admin/AuditLogControllerTest.php`:
`testKindFilterCharsetGate` exercises five reject cases each on
`entity_type` and `subject_kind`, then smoke-passes five real
kinds (`reporter`, `consumer`, `manual_block`, `admin-token`,
`oidc_role_mapping`) so the regex stays compatible with what the
audit emitter actually writes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa преди 3 дни
родител
ревизия
f7a727da7c
променени са 2 файла, в които са добавени 59 реда и са изтрити 0 реда
  1. 16 0
      api/src/Application/Admin/AuditController.php
  2. 43 0
      api/tests/Integration/Admin/AuditLogControllerTest.php

+ 16 - 0
api/src/Application/Admin/AuditController.php

@@ -29,6 +29,15 @@ final class AuditController
     // entity types/ids short identifiers); 128 is comfortable headroom.
     private const MAX_FILTER_LENGTH = 128;
 
+    // SEC_REVIEW F47: charset gate for the *_kind columns the audit
+    // filters touch (`subject_kind`, `entity_type`). Every known kind
+    // is a short snake/kebab identifier (`reporter`, `consumer`,
+    // `admin-token`, `manual_block`, `oidc_role_mapping`); reject
+    // anything that isn't a-z0-9 plus `_-` so a caller can't push
+    // bytes that wouldn't match any real `target_type` / `actor_kind`
+    // column value through the prepared statement.
+    private const KIND_PATTERN = '/^[a-z0-9][a-z0-9_-]*$/';
+
     // SEC_REVIEW F31: cap pagination depth so a Viewer cannot force
     // `LIMIT n OFFSET huge` deep scans. 10 000 rows is well past any
     // human-driven browse and bounds the worst-case scan cost.
@@ -86,6 +95,8 @@ final class AuditController
         if (isset($params['entity_type']) && is_string($params['entity_type']) && $params['entity_type'] !== '') {
             if (strlen($params['entity_type']) > self::MAX_FILTER_LENGTH) {
                 $errors['entity_type'] = 'must be at most ' . self::MAX_FILTER_LENGTH . ' characters';
+            } elseif (preg_match(self::KIND_PATTERN, $params['entity_type']) !== 1) {
+                $errors['entity_type'] = 'must match `[a-z0-9][a-z0-9_-]*`';
             } else {
                 $filters['entity_type'] = $params['entity_type'];
             }
@@ -109,6 +120,11 @@ final class AuditController
                 $errors['subject'] = 'subject_kind and subject_id must be supplied together';
             } elseif (strlen($subjectKind) > self::MAX_FILTER_LENGTH || strlen($subjectId) > self::MAX_FILTER_LENGTH) {
                 $errors['subject'] = 'subject_kind and subject_id must each be at most ' . self::MAX_FILTER_LENGTH . ' characters';
+            } elseif (preg_match(self::KIND_PATTERN, $subjectKind) !== 1) {
+                // SEC_REVIEW F47: subject_kind is matched against
+                // `target_type` / `actor_kind` columns whose real values
+                // are all short snake/kebab identifiers.
+                $errors['subject'] = 'subject_kind must match `[a-z0-9][a-z0-9_-]*`';
             } else {
                 $filters['subject_kind'] = $subjectKind;
                 $filters['subject_id'] = $subjectId;

+ 43 - 0
api/tests/Integration/Admin/AuditLogControllerTest.php

@@ -144,6 +144,49 @@ final class AuditLogControllerTest extends AppTestCase
         self::assertArrayHasKey('subject', $this->decode($resp)['details']);
     }
 
+    public function testKindFilterCharsetGate(): void
+    {
+        // SEC_REVIEW F47: `entity_type` and `subject_kind` are matched
+        // against `actor_kind` / `target_type` columns whose real values
+        // are short snake/kebab identifiers. Reject anything outside
+        // `^[a-z0-9][a-z0-9_-]*$` so wild bytes can't reach the
+        // prepared statement (no SQLi today, but the zero-byte and
+        // multi-megabyte cases waste planner work).
+        $token = $this->createToken(TokenKind::Admin, Role::Viewer);
+
+        // entity_type with uppercase / dot / space
+        foreach (['Reporter', 'reporter.created', 'reporter manual', '-leading-dash', "report\nfaked"] as $bad) {
+            $resp = $this->request(
+                'GET',
+                '/api/v1/admin/audit-log?entity_type=' . rawurlencode($bad),
+                ['Authorization' => 'Bearer ' . $token],
+            );
+            self::assertSame(400, $resp->getStatusCode(), "expected 400 for entity_type={$bad}");
+            self::assertArrayHasKey('entity_type', $this->decode($resp)['details']);
+        }
+
+        // subject_kind needs the same gate (paired with subject_id).
+        foreach (['Reporter', 'reporter.created', 'rep orter', '-leading-dash'] as $bad) {
+            $resp = $this->request(
+                'GET',
+                '/api/v1/admin/audit-log?subject_kind=' . rawurlencode($bad) . '&subject_id=5',
+                ['Authorization' => 'Bearer ' . $token],
+            );
+            self::assertSame(400, $resp->getStatusCode(), "expected 400 for subject_kind={$bad}");
+            self::assertArrayHasKey('subject', $this->decode($resp)['details']);
+        }
+
+        // Real kinds still pass through (smoke).
+        foreach (['reporter', 'consumer', 'manual_block', 'admin-token', 'oidc_role_mapping'] as $good) {
+            $resp = $this->request(
+                'GET',
+                '/api/v1/admin/audit-log?entity_type=' . rawurlencode($good),
+                ['Authorization' => 'Bearer ' . $token],
+            );
+            self::assertSame(200, $resp->getStatusCode(), "expected 200 for entity_type={$good}");
+        }
+    }
+
     public function testDeepOffsetRejected(): void
     {
         // SEC_REVIEW F31: pagination is capped at 10 000 offset rows so a