1
0
Эх сурвалжийг харах

fix: dedicated audit row when reporter/consumer audit_enabled flips (SEC_REVIEW F41)

PATCH `/api/v1/admin/reporters/{id}` and `/admin/consumers/{id}` accept
`audit_enabled` as a mass-assignable field. The flip is recorded inside
the standard `reporter.updated` / `consumer.updated` row's `changes`
metadata blob, but SIEM tools that match on flat `action` columns have
no clean way to alert on "someone disabled audit emission for this
entity" — the signal is buried in JSON.

An admin (or attacker who reached admin) can therefore silently
disable `report.received` / `blocklist.requested` audit emission
before performing further activity, then re-enable, with the only
trail being a generic `reporter.updated` row whose `changes` field
they'd have to pattern-match on after the fact.

Add two dedicated audit actions —
`AuditAction::REPORTER_AUDIT_TOGGLED` (`reporter.audit_toggled`) and
`AuditAction::CONSUMER_AUDIT_TOGGLED` (`consumer.audit_toggled`) —
emitted in addition to the standard `*.updated` row whenever
`audit_enabled` actually flips. The toggle row's metadata records the
flat `{from, to}` booleans so an alert rule can fire on the action
alone or include the direction. Both rows live in the same DB
transaction as the underlying field update; a partial commit cannot
flip the field without leaving the toggle trail.

The implementation captures the new value into a local
`$auditToggleTo` *before* the closure, which both keeps the toggle
detection out of the transactional callback's responsibility and
narrows the field type for phpstan (`array_key_exists` inside a `use`
closure does not refine the static type otherwise).

UI's `AuditController::ALLOWED_ACTIONS` filter dropdown is extended
to expose the new actions so an operator browsing the audit page can
filter to just toggle events.

Regression tests in
`api/tests/Integration/Admin/ReportersControllerTest.php` and
`api/tests/Integration/Admin/ConsumersControllerTest.php`:

  - `testAuditEnabledToggleEmitsDedicatedAuditRow` PATCHes
    `audit_enabled: false` and asserts both `reporter.updated` /
    `consumer.updated` AND the new `*.audit_toggled` action are
    written, with `details_json` recording `from: true, to: false`.
  - `testAuditEnabledNoOpDoesNotEmitDedicatedRow` PATCHes the field
    to its current value and asserts the dedicated signal is NOT
    emitted (so SOC alerts don't see noise).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 3 өдөр өмнө
parent
commit
4ca69f30b6

+ 25 - 1
api/src/Application/Admin/ConsumersController.php

@@ -209,8 +209,18 @@ final class ConsumersController
             'audit_enabled' => $existing->auditEnabled ? 1 : 0,
         ];
 
+        // SEC_REVIEW F41: dedicated alertable audit signal whenever
+        // `audit_enabled` actually flips. The regular `consumer.updated`
+        // row also fires (carrying the full field diff for context).
+        $auditToggleTo = null;
+        if (array_key_exists('audit_enabled', $fields)
+            && $fields['audit_enabled'] !== ($existing->auditEnabled ? 1 : 0)
+        ) {
+            $auditToggleTo = $fields['audit_enabled'] === 1;
+        }
+
         $auditCtx = self::auditContext($request);
-        $this->connection->transactional(function () use ($id, $fields, $existing, $beforeSnapshot, $auditCtx): void {
+        $this->connection->transactional(function () use ($id, $fields, $existing, $beforeSnapshot, $auditCtx, $auditToggleTo): void {
             $this->consumers->update($id, $fields);
             $updatedName = isset($fields['name']) ? (string) $fields['name'] : $existing->name;
             $this->audit->emitOrThrow(
@@ -221,6 +231,20 @@ final class ConsumersController
                 $auditCtx,
                 $updatedName,
             );
+            if ($auditToggleTo !== null) {
+                $this->audit->emitOrThrow(
+                    AuditAction::CONSUMER_AUDIT_TOGGLED,
+                    'consumer',
+                    $id,
+                    [
+                        'name' => $existing->name,
+                        'from' => $beforeSnapshot['audit_enabled'] === 1,
+                        'to' => $auditToggleTo,
+                    ],
+                    $auditCtx,
+                    $updatedName,
+                );
+            }
         });
 
         $updated = $this->consumers->findById($id);

+ 25 - 1
api/src/Application/Admin/ReportersController.php

@@ -203,8 +203,18 @@ final class ReportersController
             'audit_enabled' => $existing->auditEnabled ? 1 : 0,
         ];
 
+        // SEC_REVIEW F41: dedicated alertable audit signal whenever
+        // `audit_enabled` actually flips. The regular `reporter.updated`
+        // row also fires (carrying the full field diff for context).
+        $auditToggleTo = null;
+        if (array_key_exists('audit_enabled', $fields)
+            && $fields['audit_enabled'] !== ($existing->auditEnabled ? 1 : 0)
+        ) {
+            $auditToggleTo = $fields['audit_enabled'] === 1;
+        }
+
         $auditCtx = self::auditContext($request);
-        $this->connection->transactional(function () use ($id, $fields, $existing, $beforeSnapshot, $auditCtx): void {
+        $this->connection->transactional(function () use ($id, $fields, $existing, $beforeSnapshot, $auditCtx, $auditToggleTo): void {
             $this->reporters->update($id, $fields);
             $updatedName = isset($fields['name']) ? (string) $fields['name'] : $existing->name;
             $this->audit->emitOrThrow(
@@ -215,6 +225,20 @@ final class ReportersController
                 $auditCtx,
                 $updatedName,
             );
+            if ($auditToggleTo !== null) {
+                $this->audit->emitOrThrow(
+                    AuditAction::REPORTER_AUDIT_TOGGLED,
+                    'reporter',
+                    $id,
+                    [
+                        'name' => $existing->name,
+                        'from' => $beforeSnapshot['audit_enabled'] === 1,
+                        'to' => $auditToggleTo,
+                    ],
+                    $auditCtx,
+                    $updatedName,
+                );
+            }
         });
 
         $updated = $this->reporters->findById($id);

+ 9 - 0
api/src/Domain/Audit/AuditAction.php

@@ -20,10 +20,19 @@ final class AuditAction
     public const REPORTER_CREATED = 'reporter.created';
     public const REPORTER_UPDATED = 'reporter.updated';
     public const REPORTER_DELETED = 'reporter.deleted';
+    // SEC_REVIEW F41: dedicated alertable signal for the per-reporter
+    // `audit_enabled` flag flipping. The standard `reporter.updated`
+    // row continues to carry the full field diff; this row exists so
+    // SIEM tooling can match a toggle of audit emission with a flat
+    // `action = 'reporter.audit_toggled'` rule rather than walking
+    // into the metadata `changes` blob.
+    public const REPORTER_AUDIT_TOGGLED = 'reporter.audit_toggled';
 
     public const CONSUMER_CREATED = 'consumer.created';
     public const CONSUMER_UPDATED = 'consumer.updated';
     public const CONSUMER_DELETED = 'consumer.deleted';
+    // SEC_REVIEW F41: same as the reporter sibling above.
+    public const CONSUMER_AUDIT_TOGGLED = 'consumer.audit_toggled';
 
     public const TOKEN_CREATED = 'token.created';
     public const TOKEN_REVOKED = 'token.revoked';

+ 80 - 0
api/tests/Integration/Admin/ConsumersControllerTest.php

@@ -73,4 +73,84 @@ final class ConsumersControllerTest extends AppTestCase
         self::assertSame(200, $patch->getStatusCode());
         self::assertFalse($this->decode($patch)['audit_enabled']);
     }
+
+    public function testAuditEnabledToggleEmitsDedicatedAuditRow(): void
+    {
+        // SEC_REVIEW F41: an admin flipping `audit_enabled` for a consumer
+        // must leave a flat alertable trail SOC tooling can match on with
+        // `action = 'consumer.audit_toggled'` — without walking into the
+        // metadata `changes` blob of the standard `consumer.updated` row.
+        $token = $this->createToken(TokenKind::Admin, role: Role::Admin);
+        $policyId = (int) $this->db->fetchOne(
+            'SELECT id FROM policies WHERE name = :name',
+            ['name' => 'moderate'],
+        );
+
+        $created = $this->request(
+            'POST',
+            '/api/v1/admin/consumers',
+            ['Authorization' => 'Bearer ' . $token, 'Content-Type' => 'application/json'],
+            json_encode(['name' => 'fw-audit-signal', 'policy_id' => $policyId]) ?: null,
+        );
+        $consumerId = (int) $this->decode($created)['id'];
+
+        $this->request(
+            'PATCH',
+            "/api/v1/admin/consumers/{$consumerId}",
+            ['Authorization' => 'Bearer ' . $token, 'Content-Type' => 'application/json'],
+            json_encode(['audit_enabled' => false]) ?: null,
+        );
+
+        $rows = $this->db->fetchAllAssociative(
+            "SELECT action, details_json FROM audit_log WHERE target_type = 'consumer' AND target_id = ? ORDER BY id",
+            [(string) $consumerId],
+        );
+        $actions = array_column($rows, 'action');
+        self::assertContains('consumer.updated', $actions);
+        self::assertContains('consumer.audit_toggled', $actions);
+
+        $toggleRow = null;
+        foreach ($rows as $row) {
+            if ($row['action'] === 'consumer.audit_toggled') {
+                $toggleRow = $row;
+                break;
+            }
+        }
+        self::assertNotNull($toggleRow);
+        $meta = json_decode((string) $toggleRow['details_json'], true);
+        self::assertSame(true, $meta['from'] ?? null);
+        self::assertSame(false, $meta['to'] ?? null);
+    }
+
+    public function testAuditEnabledNoOpDoesNotEmitDedicatedRow(): void
+    {
+        $token = $this->createToken(TokenKind::Admin, role: Role::Admin);
+        $policyId = (int) $this->db->fetchOne(
+            'SELECT id FROM policies WHERE name = :name',
+            ['name' => 'moderate'],
+        );
+
+        $created = $this->request(
+            'POST',
+            '/api/v1/admin/consumers',
+            ['Authorization' => 'Bearer ' . $token, 'Content-Type' => 'application/json'],
+            json_encode(['name' => 'fw-noop', 'policy_id' => $policyId]) ?: null,
+        );
+        $consumerId = (int) $this->decode($created)['id'];
+
+        // PATCH `audit_enabled` to its current value (no-op) — must NOT
+        // fire the toggle signal.
+        $this->request(
+            'PATCH',
+            "/api/v1/admin/consumers/{$consumerId}",
+            ['Authorization' => 'Bearer ' . $token, 'Content-Type' => 'application/json'],
+            json_encode(['audit_enabled' => true]) ?: null,
+        );
+
+        $actions = $this->db->fetchFirstColumn(
+            "SELECT action FROM audit_log WHERE target_type = 'consumer' AND target_id = ?",
+            [(string) $consumerId],
+        );
+        self::assertNotContains('consumer.audit_toggled', $actions);
+    }
 }

+ 60 - 0
api/tests/Integration/Admin/ReportersControllerTest.php

@@ -121,6 +121,66 @@ final class ReportersControllerTest extends AppTestCase
         self::assertFalse($this->decode($patch)['audit_enabled']);
     }
 
+    public function testAuditEnabledToggleEmitsDedicatedAuditRow(): void
+    {
+        // SEC_REVIEW F41: an admin flipping `audit_enabled` for a reporter
+        // must leave a flat alertable trail SOC tooling can match on with
+        // `action = 'reporter.audit_toggled'` — without walking into the
+        // details_json `changes` blob of the standard `reporter.updated` row.
+        $token = $this->createToken(TokenKind::Admin, role: Role::Admin);
+        $reporterId = $this->createReporter('web-audit-toggle-audit');
+
+        $this->request(
+            'PATCH',
+            "/api/v1/admin/reporters/{$reporterId}",
+            ['Authorization' => 'Bearer ' . $token, 'Content-Type' => 'application/json'],
+            json_encode(['audit_enabled' => false]) ?: null,
+        );
+
+        $rows = $this->db->fetchAllAssociative(
+            "SELECT action, details_json FROM audit_log WHERE target_type = 'reporter' AND target_id = ? ORDER BY id",
+            [(string) $reporterId],
+        );
+        $actions = array_column($rows, 'action');
+        // Both signals fire: the existing standard row plus the dedicated toggle.
+        self::assertContains('reporter.updated', $actions);
+        self::assertContains('reporter.audit_toggled', $actions);
+
+        $toggleRow = null;
+        foreach ($rows as $row) {
+            if ($row['action'] === 'reporter.audit_toggled') {
+                $toggleRow = $row;
+                break;
+            }
+        }
+        self::assertNotNull($toggleRow);
+        $meta = json_decode((string) $toggleRow['details_json'], true);
+        self::assertSame(true, $meta['from'] ?? null);
+        self::assertSame(false, $meta['to'] ?? null);
+    }
+
+    public function testAuditEnabledNoOpDoesNotEmitDedicatedRow(): void
+    {
+        // PATCHing `audit_enabled` to its current value (no-op) must NOT
+        // fire the toggle signal — SOC alerts would otherwise see noise.
+        $token = $this->createToken(TokenKind::Admin, role: Role::Admin);
+        $reporterId = $this->createReporter('web-audit-noop');
+
+        // Default is true; PATCH it to true (no-op).
+        $this->request(
+            'PATCH',
+            "/api/v1/admin/reporters/{$reporterId}",
+            ['Authorization' => 'Bearer ' . $token, 'Content-Type' => 'application/json'],
+            json_encode(['audit_enabled' => true]) ?: null,
+        );
+
+        $actions = $this->db->fetchFirstColumn(
+            "SELECT action FROM audit_log WHERE target_type = 'reporter' AND target_id = ?",
+            [(string) $reporterId],
+        );
+        self::assertNotContains('reporter.audit_toggled', $actions);
+    }
+
     public function testDeleteWithoutReportsSoftDeletes(): void
     {
         $token = $this->createToken(TokenKind::Admin, role: Role::Admin);

+ 2 - 0
ui/src/Controllers/AuditController.php

@@ -23,7 +23,9 @@ final class AuditController
 
     private const ALLOWED_ACTIONS = [
         'reporter.created', 'reporter.updated', 'reporter.deleted',
+        'reporter.audit_toggled',
         'consumer.created', 'consumer.updated', 'consumer.deleted',
+        'consumer.audit_toggled',
         'token.created', 'token.revoked',
         'policy.created', 'policy.updated', 'policy.deleted',
         'category.created', 'category.updated', 'category.deleted',