From 930490d1b4eb0b19cbc3850031989e5aba1bb3d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=9C=A0=EC=B0=BD=EC=9A=B1?= Date: Fri, 17 Jul 2026 12:58:27 +0900 Subject: [PATCH] feat(cloud,edge): event dedup (event_uuid) + site-scoped access control (P1-9, P3-2b) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two approved data-model changes. P1-9 — ingest idempotency. A POST can succeed server-side but time out client-side; the edge then re-queues and re-sends the same event, so the cloud stored duplicate alerts. The edge Event now carries a stable event_uuid (uuid4 hex) generated once at creation and preserved through the offline queue and every retry. Cloud: events.event_uuid CHAR(32) NULL UNIQUE; EventValidator accepts/validates it; IngestController returns the already-stored event (200, dedup:true) on a repeat uuid, race-safe against the UNIQUE key (catches the insert-race PDOException and returns the winner). P3-2b — site-scoped access control. site_scope was stored in the session but never enforced, so any authenticated user could read every site's worker snapshots (PIPA). Added devices.site; ingest denormalizes it onto events.site (server-derived from the authenticated device_id, never client-supplied). App\Security\SiteScope centralizes the rule (NULL scope = global operator). The event list filters in SQL (page(...,$site)); event detail + acknowledge and the snapshot proxy guard per-row and return 404 (not 403) cross-site so existence never leaks. Schema migration ALTERs and per-device/user provisioning UPDATEs are documented at the bottom of schema.sql. Tests: edge queue round-trip keeps the uuid; repeat-uuid dedups to one row; list scoped to caller site; detail cross-site 404; snapshot cross-site 404 / same-site 200; event_uuid validation. phpunit 97 passing (4 skipped: PDO), pytest 158 passing (2 skipped). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_019Xmn8KZWqiQUguEMRtn6rW --- cloud/public/index.php | 2 +- cloud/schema.sql | 29 +++++++++- cloud/src/Dashboard/EventDetailController.php | 16 +++++- cloud/src/Dashboard/EventListController.php | 6 ++- cloud/src/Dashboard/SnapshotProxy.php | 18 ++++++- cloud/src/Ingest/EventValidator.php | 7 +++ cloud/src/Ingest/IngestController.php | 24 ++++++++- cloud/src/Repository/DeviceRepository.php | 2 + cloud/src/Repository/EventRepository.php | 7 ++- .../Repository/Pdo/PdoDeviceRepository.php | 9 ++++ .../src/Repository/Pdo/PdoEventRepository.php | 25 +++++++-- .../Repository/Pdo/PdoSnapshotRepository.php | 11 ++++ cloud/src/Repository/SnapshotRepository.php | 3 ++ cloud/src/Security/SiteScope.php | 29 ++++++++++ cloud/tests/DashboardTest.php | 54 ++++++++++++++++++- cloud/tests/EventValidatorTest.php | 16 ++++++ cloud/tests/IngestControllerTest.php | 31 +++++++++++ cloud/tests/PdoIntegrationTest.php | 15 ++++++ cloud/tests/Support/InMemoryRepositories.php | 15 +++++- docs/delivery-audit.md | 28 +++++++--- edge/events.py | 8 ++- edge/queue.py | 7 ++- tests/test_queue.py | 20 +++++++ 23 files changed, 355 insertions(+), 27 deletions(-) create mode 100644 cloud/src/Security/SiteScope.php diff --git a/cloud/public/index.php b/cloud/public/index.php index 2859c95..88ac631 100644 --- a/cloud/public/index.php +++ b/cloud/public/index.php @@ -91,7 +91,7 @@ $auth = new AuthController($users, $audit, $rate); $csrfCtl = new CsrfController(); $list = new EventListController($events, $sess); $detail = new EventDetailController($events, $snaps, $audit, $sess); -$proxy = new SnapshotProxy($snaps, $sess); +$proxy = new SnapshotProxy($snaps, $events, $sess); $zoneCtl = new ZoneConfigController($zones, $audit, $sess); $render = function (string $view): Response { diff --git a/cloud/schema.sql b/cloud/schema.sql index 80d44ae..e24368e 100644 --- a/cloud/schema.sql +++ b/cloud/schema.sql @@ -58,6 +58,7 @@ CREATE TABLE users ( CREATE TABLE devices ( device_id VARCHAR(64) NOT NULL, name VARCHAR(190) NULL, + site VARCHAR(64) NULL, -- access-control scope; matches users.site_scope (NULL = unassigned) secret VARCHAR(255) NOT NULL, -- per-device HMAC key; rotate via admin CLI, never log config_version INT UNSIGNED NOT NULL DEFAULT 1, -- authoritative version served by GET /api/config last_fps DOUBLE NULL, @@ -66,7 +67,8 @@ CREATE TABLE devices ( reported_config_version INT UNSIGNED NULL, -- version the device last claimed via heartbeat last_heartbeat_at BIGINT UNSIGNED NULL, created_at BIGINT UNSIGNED NOT NULL DEFAULT (UNIX_TIMESTAMP()), - PRIMARY KEY (device_id) + PRIMARY KEY (device_id), + KEY ix_devices_site (site) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; -- --------------------------------------------------------------------------- @@ -112,7 +114,9 @@ CREATE TABLE zones ( -- --------------------------------------------------------------------------- CREATE TABLE events ( event_id INT UNSIGNED NOT NULL AUTO_INCREMENT, + event_uuid CHAR(32) NULL, -- edge-generated idempotency key (dedup a re-sent event); NULL for legacy rows device_id VARCHAR(64) NOT NULL, + site VARCHAR(64) NULL, -- denormalized from devices.site at ingest; access-control scope camera_id VARCHAR(64) NOT NULL, event_type VARCHAR(32) NOT NULL, severity VARCHAR(16) NOT NULL, @@ -125,8 +129,13 @@ CREATE TABLE events ( acknowledged_at BIGINT UNSIGNED NULL, received_at BIGINT UNSIGNED NOT NULL, -- server receipt time (PHP time()), for clock-skew flagging (design doc §8) PRIMARY KEY (event_id), + -- Idempotency: a UNIQUE key on event_uuid makes a re-sent event (POST succeeded server-side + -- but the ACK timed out client-side) collide instead of duplicating. MySQL allows multiple + -- NULLs in a UNIQUE index, so legacy rows without a uuid do not conflict. + UNIQUE KEY uq_events_uuid (event_uuid), KEY ix_events_occurred_at (occurred_at), KEY ix_events_severity_occurred_at (severity, occurred_at), + KEY ix_events_site_occurred_at (site, occurred_at), -- site-scoped dashboard list (P3-2b) KEY ix_events_device (device_id), CONSTRAINT chk_events_type CHECK (event_type IN ( 'fall','no_motion','immobility','ghost','zone_warning', @@ -217,3 +226,21 @@ CREATE TABLE audit_log ( KEY ix_audit_log_ts (ts), KEY ix_audit_log_actor (actor) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- =========================================================================== +-- MIGRATIONS for an EXISTING database (fresh installs already have the columns +-- above). Apply once via phpMyAdmin, then set each device's site with: +-- UPDATE devices SET site = 'factory-a' WHERE device_id = 'pi-1'; +-- and scope a dashboard user with: +-- UPDATE users SET site_scope = 'factory-a' WHERE username = '...'; +-- A user with site_scope = NULL is a global operator (sees every site). +-- --------------------------------------------------------------------------- +-- P3-2b (site-scoped access control): +-- ALTER TABLE devices ADD COLUMN site VARCHAR(64) NULL AFTER name, +-- ADD KEY ix_devices_site (site); +-- ALTER TABLE events ADD COLUMN site VARCHAR(64) NULL AFTER device_id, +-- ADD KEY ix_events_site_occurred_at (site, occurred_at); +-- P1-9 (ingest idempotency / dedup): +-- ALTER TABLE events ADD COLUMN event_uuid CHAR(32) NULL AFTER event_id, +-- ADD UNIQUE KEY uq_events_uuid (event_uuid); +-- =========================================================================== diff --git a/cloud/src/Dashboard/EventDetailController.php b/cloud/src/Dashboard/EventDetailController.php index 90f8c97..344fff3 100644 --- a/cloud/src/Dashboard/EventDetailController.php +++ b/cloud/src/Dashboard/EventDetailController.php @@ -4,7 +4,7 @@ namespace App\Dashboard; use App\Http\{Request, Response}; use App\Auth\{SessionStore, Rbac}; -use App\Security\Csrf; +use App\Security\{Csrf, SiteScope}; use App\Repository\{EventRepository, SnapshotRepository, AuditRepository}; final class EventDetailController @@ -14,12 +14,20 @@ final class EventDetailController private AuditRepository $audit, private SessionStore $sess ) {} + // A scoped operator must not read — or acknowledge — an event outside their site. Return + // 404 (not 403) so a cross-site id is indistinguishable from a non-existent one (no leak). + private function outOfScope(array $event): bool + { + $scope = $this->sess->get('site_scope'); + return !SiteScope::allows(is_string($scope) ? $scope : null, $event['site'] ?? null); + } + public function show(Request $req, array $params): Response { if ($this->sess->get('user_id') === null) return Response::json(['error' => 'unauthorized'], 401); $id = (int)($params['id'] ?? 0); $event = $this->events->find($id); - if ($event === null) return Response::json(['error' => 'not_found'], 404); + if ($event === null || $this->outOfScope($event)) return Response::json(['error' => 'not_found'], 404); return Response::json(['event' => $event, 'snapshots' => $this->snaps->forEvent($id)], 200); } @@ -31,6 +39,10 @@ final class EventDetailController return Response::json(['error' => 'csrf'], 403); } $id = (int)($params['id'] ?? 0); + // Scope BEFORE the write: without the find()+guard a scoped user could acknowledge another + // site's event by guessing its id. + $event = $this->events->find($id); + if ($event === null || $this->outOfScope($event)) return Response::json(['error' => 'not_found'], 404); $user = (string)$this->sess->get('username'); if (!$this->events->acknowledge($id, $user)) return Response::json(['error' => 'not_found'], 404); $this->audit->append($user, 'acknowledge', "event:$id", [], []); diff --git a/cloud/src/Dashboard/EventListController.php b/cloud/src/Dashboard/EventListController.php index 144ac2b..e67b622 100644 --- a/cloud/src/Dashboard/EventListController.php +++ b/cloud/src/Dashboard/EventListController.php @@ -19,7 +19,11 @@ final class EventListController $severity = $req->query('severity'); $limit = self::clampLimit($req->query('limit')); [$beforeTs, $beforeId] = self::parseCursor((string)$req->query('cursor')); - $rows = $this->events->page($limit, $severity, $beforeTs, $beforeId); + // Site-scope access control (P3-2b): a scoped operator sees only their site's events; a + // null/empty scope is a global operator (no site filter). Pushed into the SQL WHERE. + $scope = $this->sess->get('site_scope'); + $site = (is_string($scope) && $scope !== '') ? $scope : null; + $rows = $this->events->page($limit, $severity, $beforeTs, $beforeId, $site); // Advertise a next cursor only on a full page — a short page means we hit the end. // The cursor is ":" of the last (oldest) row on this page. diff --git a/cloud/src/Dashboard/SnapshotProxy.php b/cloud/src/Dashboard/SnapshotProxy.php index 94675b4..1dea062 100644 --- a/cloud/src/Dashboard/SnapshotProxy.php +++ b/cloud/src/Dashboard/SnapshotProxy.php @@ -4,11 +4,14 @@ namespace App\Dashboard; use App\Http\{Request, Response}; use App\Auth\SessionStore; -use App\Repository\SnapshotRepository; +use App\Security\SiteScope; +use App\Repository\{SnapshotRepository, EventRepository}; final class SnapshotProxy { - public function __construct(private SnapshotRepository $snaps, private SessionStore $sess) {} + public function __construct( + private SnapshotRepository $snaps, private EventRepository $events, private SessionStore $sess + ) {} public function serve(Request $req, array $params): Response { @@ -16,6 +19,17 @@ final class SnapshotProxy $uuid = (string)($params['uuid'] ?? ''); $path = $this->snaps->pathByUuid($uuid); if ($path === null || !is_file($path)) return Response::json(['error' => 'not_found'], 404); + // Site-scope (P3-2b): a snapshot inherits its owning event's site; a scoped operator must + // not fetch another site's worker image (PIPA). 404, not 403, so it can't be used to probe + // which uuids exist. Global operators skip the lookup. + $scope = $this->sess->get('site_scope'); + if (!SiteScope::isGlobal(is_string($scope) ? $scope : null)) { + $eventId = $this->snaps->eventIdByUuid($uuid); + $event = $eventId !== null ? $this->events->find($eventId) : null; + if ($event === null || !SiteScope::allows((string)$scope, $event['site'] ?? null)) { + return Response::json(['error' => 'not_found'], 404); + } + } return Response::bytes((string)file_get_contents($path), 'image/jpeg', 200); } } diff --git a/cloud/src/Ingest/EventValidator.php b/cloud/src/Ingest/EventValidator.php index 1dde7b6..600ec2a 100644 --- a/cloud/src/Ingest/EventValidator.php +++ b/cloud/src/Ingest/EventValidator.php @@ -31,6 +31,12 @@ final class EventValidator if (isset($p['track_id']) && !is_numeric($p['track_id'])) $errors[] = 'bad:track_id'; if (isset($p['zone_id']) && !is_string($p['zone_id'])) $errors[] = 'bad:zone_id'; if (isset($p['duration_s']) && !is_numeric($p['duration_s'])) $errors[] = 'bad:duration_s'; + // Optional idempotency key: the edge generates a stable 32-hex uuid per event and re-sends + // it on retry so the cloud can dedup. Enforce the exact shape (matches CHAR(32) + the + // snapshot-uuid convention); a malformed key is a bug, not a silent pass. + if (isset($p['event_uuid']) && (!is_string($p['event_uuid']) || !preg_match('/^[0-9a-f]{32}$/', $p['event_uuid']))) { + $errors[] = 'bad:event_uuid'; + } // Length caps matching the DB columns (device_id/camera_id VARCHAR(64), zone_id // CHAR(32)). Without these an overlong id is a MySQL 'Data too long' 500 that the edge @@ -42,6 +48,7 @@ final class EventValidator if ($errors) return ['ok' => false, 'errors' => $errors, 'clean' => []]; $clean = [ + 'event_uuid' => isset($p['event_uuid']) ? (string)$p['event_uuid'] : null, 'device_id' => (string)$p['device_id'], 'camera_id' => (string)$p['camera_id'], 'event_type' => (string)$p['event_type'], diff --git a/cloud/src/Ingest/IngestController.php b/cloud/src/Ingest/IngestController.php index cc80e8d..518fe91 100644 --- a/cloud/src/Ingest/IngestController.php +++ b/cloud/src/Ingest/IngestController.php @@ -66,7 +66,29 @@ final class IngestController $v = EventValidator::validate($payload); if (!$v['ok']) return Response::json(['error' => 'invalid', 'details' => $v['errors']], 400); - $eventId = $this->events->insert($v['clean']); + $clean = $v['clean']; + // Site denormalization (P3-2b): the dashboard filters events by site for access control, + // so stamp the DEVICE's site onto the event here. Server-derived from the authenticated + // device_id — a client-supplied site is never trusted. + $clean['site'] = $this->devices->siteFor($deviceId); + + // Idempotency (P1-9): a POST can succeed server-side but time out client-side; the edge + // then re-queues and re-sends the SAME event_uuid. Return the already-stored event + // instead of inserting a duplicate alert. + $uuid = $clean['event_uuid'] ?? null; + if ($uuid !== null && ($existing = $this->events->idByUuid($uuid)) !== null) { + return Response::json(['event_id' => $existing, 'dedup' => true], 200); + } + try { + $eventId = $this->events->insert($clean); + } catch (\PDOException $e) { + // Lost the insert race to a concurrent retry with the same uuid (the UNIQUE key + // fired). That is a successful dedup, not a 500 — return the winning row. + if ($uuid !== null && ($existing = $this->events->idByUuid($uuid)) !== null) { + return Response::json(['event_id' => $existing, 'dedup' => true], 200); + } + throw $e; + } foreach (($payload['snapshots'] ?? []) as $b64) { $this->storeSnapshot($eventId, (string)$b64); } diff --git a/cloud/src/Repository/DeviceRepository.php b/cloud/src/Repository/DeviceRepository.php index 5b5a06e..430b21f 100644 --- a/cloud/src/Repository/DeviceRepository.php +++ b/cloud/src/Repository/DeviceRepository.php @@ -5,6 +5,8 @@ namespace App\Repository; interface DeviceRepository { public function secretFor(string $deviceId): ?string; + /** The device's access-control site (devices.site), or null if unassigned/unknown. */ + public function siteFor(string $deviceId): ?string; public function touchHeartbeat(string $deviceId, array $meta): void; public function configVersion(string $deviceId): int; } diff --git a/cloud/src/Repository/EventRepository.php b/cloud/src/Repository/EventRepository.php index 9b4f1ff..86246d1 100644 --- a/cloud/src/Repository/EventRepository.php +++ b/cloud/src/Repository/EventRepository.php @@ -5,14 +5,19 @@ namespace App\Repository; interface EventRepository { public function insert(array $event): int; + /** The event_id of a previously-ingested event with this uuid, or null. Backs ingest + * idempotency (a re-sent event dedups instead of duplicating). */ + public function idByUuid(string $uuid): ?int; public function recent(int $limit, ?string $severity): array; /** * Keyset (cursor) page over the (occurred_at DESC, event_id DESC) sort key. * Pass null/null for the first page; then the (occurred_at, event_id) of the last * returned row to fetch strictly older events. Stable under concurrent inserts — * new events land at the head and never shift a page window. + * $site (when non-null) restricts the page to events whose denormalized site matches — + * the SQL half of the site-scope access control (see App\Security\SiteScope). */ - public function page(int $limit, ?string $severity, ?float $beforeTs, ?int $beforeId): array; + public function page(int $limit, ?string $severity, ?float $beforeTs, ?int $beforeId, ?string $site = null): array; public function find(int $id): ?array; public function acknowledge(int $id, string $user): bool; } diff --git a/cloud/src/Repository/Pdo/PdoDeviceRepository.php b/cloud/src/Repository/Pdo/PdoDeviceRepository.php index e43191d..ad0cb25 100644 --- a/cloud/src/Repository/Pdo/PdoDeviceRepository.php +++ b/cloud/src/Repository/Pdo/PdoDeviceRepository.php @@ -17,6 +17,15 @@ final class PdoDeviceRepository implements DeviceRepository return $secret === false ? null : (string)$secret; } + public function siteFor(string $deviceId): ?string + { + $st = $this->pdo->prepare('SELECT site FROM devices WHERE device_id = ? LIMIT 1'); + $st->execute([$deviceId]); + $site = $st->fetchColumn(); + // false = no such device; null = device exists but unassigned. Both -> null (unscoped). + return ($site === false || $site === null) ? null : (string)$site; + } + public function touchHeartbeat(string $deviceId, array $meta): void { // A plain UPDATE (not upsert) is safe here: the caller (HeartbeatController) diff --git a/cloud/src/Repository/Pdo/PdoEventRepository.php b/cloud/src/Repository/Pdo/PdoEventRepository.php index 84de8de..a7ba436 100644 --- a/cloud/src/Repository/Pdo/PdoEventRepository.php +++ b/cloud/src/Repository/Pdo/PdoEventRepository.php @@ -8,7 +8,7 @@ use PDO; final class PdoEventRepository implements EventRepository { private const SELECT_COLUMNS = - 'event_id, device_id, camera_id, event_type, severity, track_id, zone_id, + 'event_id, event_uuid, device_id, site, camera_id, event_type, severity, track_id, zone_id, occurred_at, duration_s, metrics_json, acknowledged_by, acknowledged_at'; public function __construct(private PDO $pdo) {} @@ -17,12 +17,14 @@ final class PdoEventRepository implements EventRepository { $st = $this->pdo->prepare( 'INSERT INTO events - (device_id, camera_id, event_type, severity, track_id, zone_id, + (event_uuid, device_id, site, camera_id, event_type, severity, track_id, zone_id, occurred_at, duration_s, metrics_json, received_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)' + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)' ); $st->execute([ + $event['event_uuid'] ?? null, // NULL when the edge sent no idempotency key (legacy) (string)$event['device_id'], + $event['site'] ?? null, // denormalized from devices.site by IngestController (string)$event['camera_id'], (string)$event['event_type'], (string)$event['severity'], @@ -36,6 +38,14 @@ final class PdoEventRepository implements EventRepository return (int)$this->pdo->lastInsertId(); } + public function idByUuid(string $uuid): ?int + { + $st = $this->pdo->prepare('SELECT event_id FROM events WHERE event_uuid = ? LIMIT 1'); + $st->execute([$uuid]); + $id = $st->fetchColumn(); + return $id === false ? null : (int)$id; + } + public function recent(int $limit, ?string $severity): array { if ($severity !== null) { @@ -58,7 +68,7 @@ final class PdoEventRepository implements EventRepository return array_map(self::mapRow(...), $st->fetchAll()); } - public function page(int $limit, ?string $severity, ?float $beforeTs, ?int $beforeId): array + public function page(int $limit, ?string $severity, ?float $beforeTs, ?int $beforeId, ?string $site = null): array { // Bind list as [value, PDO type] pairs, positional in SQL order. $where = []; @@ -67,6 +77,11 @@ final class PdoEventRepository implements EventRepository $where[] = 'severity = ?'; $binds[] = [$severity, PDO::PARAM_STR]; } + if ($site !== null) { + // Site-scope access control: a scoped operator sees only their site's events. + $where[] = 'site = ?'; + $binds[] = [$site, PDO::PARAM_STR]; + } // Keyset predicate: strictly "older" than the client's last-seen (occurred_at, event_id) // on the (occurred_at DESC, event_id DESC) order. occurred_at is bound as a string so the // DECIMAL comparison keeps full precision (no float round-trip in the driver). @@ -119,7 +134,9 @@ final class PdoEventRepository implements EventRepository { return [ 'event_id' => (int)$row['event_id'], + 'event_uuid' => $row['event_uuid'] ?? null, 'device_id' => $row['device_id'], + 'site' => $row['site'] ?? null, 'camera_id' => $row['camera_id'], 'event_type' => $row['event_type'], 'severity' => $row['severity'], diff --git a/cloud/src/Repository/Pdo/PdoSnapshotRepository.php b/cloud/src/Repository/Pdo/PdoSnapshotRepository.php index 18bac0d..277d4fe 100644 --- a/cloud/src/Repository/Pdo/PdoSnapshotRepository.php +++ b/cloud/src/Repository/Pdo/PdoSnapshotRepository.php @@ -48,6 +48,17 @@ final class PdoSnapshotRepository implements SnapshotRepository return rtrim($this->snapshotDir, '/\\') . '/' . $uuid . '.jpg'; } + public function eventIdByUuid(string $uuid): ?int + { + if (strlen($uuid) !== 32 || !ctype_xdigit($uuid)) { + return null; + } + $st = $this->pdo->prepare('SELECT event_id FROM event_snapshots WHERE file_uuid = ? LIMIT 1'); + $st->execute([$uuid]); + $id = $st->fetchColumn(); + return $id === false ? null : (int)$id; + } + public function purgeOlderThan(int $cutoff, int $limit = 500): int { // Bounded batch: a single request must not stall unlinking thousands of files on a diff --git a/cloud/src/Repository/SnapshotRepository.php b/cloud/src/Repository/SnapshotRepository.php index 5fccdfd..69c6cf4 100644 --- a/cloud/src/Repository/SnapshotRepository.php +++ b/cloud/src/Repository/SnapshotRepository.php @@ -7,6 +7,9 @@ interface SnapshotRepository public function add(int $eventId, string $fileUuid, string $contentHash): void; public function forEvent(int $eventId): array; public function pathByUuid(string $uuid): ?string; + /** The event_id that owns this snapshot uuid, or null. Lets the proxy resolve a + * snapshot's site (via the owning event) for access control. */ + public function eventIdByUuid(string $uuid): ?int; /** Delete snapshot files + rows created before $cutoff (unix ts). PIPA retention: * worker images must not be kept forever. Returns the number purged. */ diff --git a/cloud/src/Security/SiteScope.php b/cloud/src/Security/SiteScope.php new file mode 100644 index 0000000..e58cdd0 --- /dev/null +++ b/cloud/src/Security/SiteScope.php @@ -0,0 +1,29 @@ +assertSame(401, $proxy->serve(new Request('GET', '/snap/x', [], '', [], []), ['uuid' => 'x'])->status()); - $proxy2 = new SnapshotProxy($snaps, $this->authed()); + $proxy2 = new SnapshotProxy($snaps, $events, $this->authed()); $this->assertSame(404, $proxy2->serve(new Request('GET', '/snap/missing', [], '', [], []), ['uuid' => 'missing'])->status()); } + + private function authedSite(string $site): ArraySessionStore { + $s = $this->authed('operator'); $s->set('site_scope', $site); return $s; + } + public function testListIsScopedToTheCallersSite(): void { + $repo = new InMemoryEventRepository(); + $repo->insert(['severity' => 'high', 'event_type' => 'fall', 'occurred_at' => 1.0, 'site' => 'factory-a']); + $repo->insert(['severity' => 'high', 'event_type' => 'fall', 'occurred_at' => 2.0, 'site' => 'factory-b']); + // A factory-a operator sees only the factory-a event. + $scoped = new EventListController($repo, $this->authedSite('factory-a')); + $rows = json_decode($scoped->json(new Request('GET', '/api/events', [], '', [], []))->body(), true)['events']; + $this->assertCount(1, $rows); + $this->assertSame('factory-a', $rows[0]['site']); + // A global operator (no site_scope) sees both. + $global = new EventListController($repo, $this->authed('operator')); + $all = json_decode($global->json(new Request('GET', '/api/events', [], '', [], []))->body(), true)['events']; + $this->assertCount(2, $all); + } + public function testDetailHidesOtherSitesEvent(): void { + $repo = new InMemoryEventRepository(); + $id = $repo->insert(['severity' => 'high', 'event_type' => 'fall', 'site' => 'factory-b']); + $c = new EventDetailController($repo, new InMemorySnapshotRepository(), new InMemoryAuditRepository(), $this->authedSite('factory-a')); + // 404 (not 403) so a cross-site id is indistinguishable from a non-existent one. + $this->assertSame(404, $c->show(new Request('GET', "/events/$id", [], '', [], []), ['id' => (string)$id])->status()); + // The owning site sees it. + $own = new EventDetailController($repo, new InMemorySnapshotRepository(), new InMemoryAuditRepository(), $this->authedSite('factory-b')); + $this->assertSame(200, $own->show(new Request('GET', "/events/$id", [], '', [], []), ['id' => (string)$id])->status()); + } + public function testSnapshotServeHidesOtherSitesImage(): void { + // A real on-disk file so the 404 can only come from the site guard, not a missing file. + $repo = new InMemoryEventRepository(); + $id = $repo->insert(['severity' => 'high', 'event_type' => 'fall', 'site' => 'factory-b']); + $file = sys_get_temp_dir() . '/snaptest' . bin2hex(random_bytes(4)) . '.jpg'; + file_put_contents($file, 'jpegbytes'); + $uuid = str_repeat('a', 32); + $snaps = new class($file, $id, $uuid) implements \App\Repository\SnapshotRepository { + public function __construct(private string $file, private int $eid, private string $uuid) {} + public function add(int $e, string $u, string $h): void {} + public function forEvent(int $e): array { return []; } + public function pathByUuid(string $u): ?string { return $u === $this->uuid ? $this->file : null; } + public function eventIdByUuid(string $u): ?int { return $u === $this->uuid ? $this->eid : null; } + public function purgeOlderThan(int $c): int { return 0; } + }; + $cross = new SnapshotProxy($snaps, $repo, $this->authedSite('factory-a')); + $this->assertSame(404, $cross->serve(new Request('GET', '/s', [], '', [], []), ['uuid' => $uuid])->status()); + $same = new SnapshotProxy($snaps, $repo, $this->authedSite('factory-b')); + $this->assertSame(200, $same->serve(new Request('GET', '/s', [], '', [], []), ['uuid' => $uuid])->status()); + @unlink($file); + } } diff --git a/cloud/tests/EventValidatorTest.php b/cloud/tests/EventValidatorTest.php index 7f58677..13aa62c 100644 --- a/cloud/tests/EventValidatorTest.php +++ b/cloud/tests/EventValidatorTest.php @@ -28,6 +28,22 @@ final class EventValidatorTest extends TestCase $ok = $this->valid(); $ok['device_id'] = str_repeat('x', 64); $this->assertTrue(EventValidator::validate($ok)['ok']); } + public function testAcceptsAndCleansEventUuid(): void { + $p = $this->valid(); $p['event_uuid'] = str_repeat('a', 32); + $r = EventValidator::validate($p); + $this->assertTrue($r['ok'], implode(',', $r['errors'])); + $this->assertSame(str_repeat('a', 32), $r['clean']['event_uuid']); + // Optional: omitting it is valid and clean carries null (legacy edges send no uuid). + $none = EventValidator::validate($this->valid()); + $this->assertTrue($none['ok']); + $this->assertNull($none['clean']['event_uuid']); + } + public function testRejectsMalformedEventUuid(): void { + foreach (['XYZ', str_repeat('a', 31), str_repeat('A', 32), 'g' . str_repeat('a', 31)] as $bad) { + $p = $this->valid(); $p['event_uuid'] = $bad; + $this->assertFalse(EventValidator::validate($p)['ok'], "uuid '$bad' must be rejected"); + } + } public function testRejectsUnknownEventType(): void { $p = $this->valid(); $p['event_type'] = 'explode'; $this->assertFalse(EventValidator::validate($p)['ok']); diff --git a/cloud/tests/IngestControllerTest.php b/cloud/tests/IngestControllerTest.php index 0d72b3f..5b9c8e9 100644 --- a/cloud/tests/IngestControllerTest.php +++ b/cloud/tests/IngestControllerTest.php @@ -92,6 +92,7 @@ final class IngestControllerTest extends TestCase public function add(int $e, string $u, string $h): void { throw new \RuntimeException('db down'); } public function forEvent(int $e): array { return []; } public function pathByUuid(string $u): ?string { return null; } + public function eventIdByUuid(string $u): ?int { return null; } public function purgeOlderThan(int $c): int { return 0; } }; $events = new InMemoryEventRepository(); @@ -104,6 +105,36 @@ final class IngestControllerTest extends TestCase 'orphan snapshot file must be removed when the DB index write fails'); } + public function testDeduplicatesRepeatedEventUuid(): void { + // A POST that succeeded server-side but timed out client-side is re-sent with the SAME + // event_uuid (fresh nonce). The cloud must return the already-stored event, not duplicate. + [$c, $events] = $this->make(); + $uuid = str_repeat('a', 32); + $p = $this->payload(); $p['event_uuid'] = $uuid; + $r1 = $c->handle($this->signedReq('sekret', $p, nonce: 'n1'), now: 1000, clientIp: '1.2.3.4'); + $this->assertSame(201, $r1->status()); + $r2 = $c->handle($this->signedReq('sekret', $p, nonce: 'n2'), now: 1000, clientIp: '1.2.3.4'); + $this->assertSame(200, $r2->status()); + $this->assertTrue(json_decode($r2->body(), true)['dedup']); + $this->assertCount(1, $events->rows, 'a re-sent event_uuid must not create a second event'); + $this->assertSame( + json_decode($r1->body(), true)['event_id'], + json_decode($r2->body(), true)['event_id']); + } + + public function testStampsDeviceSiteOntoTheEvent(): void { + // Access control filters events by site, so ingest denormalizes the DEVICE's site onto + // the event (server-derived from the authenticated device_id, not client-supplied). + $cfg = Config::fromArray(['snapshot_dir' => $this->dir, 'timestamp_window' => 300, 'rate_limit_per_min' => 60]); + $dev = new InMemoryDeviceRepository(['pi-1' => 'sekret'], [], ['pi-1' => 'factory-a']); + $events = new InMemoryEventRepository(); + $c = new IngestController($cfg, $dev, new InMemoryNonceRepository(), + new InMemoryRateLimiter(), $events, new InMemorySnapshotRepository()); + $c->handle($this->signedReq('sekret', $this->payload()), now: 1000, clientIp: '1.2.3.4'); + $this->assertCount(1, $events->rows); + $this->assertSame('factory-a', $events->rows[1]['site']); + } + public function testIpFloodRateLimited(): void { // Attacker without a valid device secret floods from one IP; the pre-auth // IP limiter must trip BEFORE any HMAC/device-secret work, protecting diff --git a/cloud/tests/PdoIntegrationTest.php b/cloud/tests/PdoIntegrationTest.php index fa2fa80..e5e77f0 100644 --- a/cloud/tests/PdoIntegrationTest.php +++ b/cloud/tests/PdoIntegrationTest.php @@ -47,8 +47,11 @@ final class PdoIntegrationTest extends TestCase public function testEventInsertFindAcknowledge(): void { $repo = new PdoEventRepository($this->pdo); + $uuid = str_repeat('a', 32); $id = $repo->insert([ + 'event_uuid' => $uuid, 'device_id' => 'dev-1', + 'site' => 'factory-a', 'camera_id' => 'cam-1', 'event_type' => 'fall', 'severity' => 'critical', @@ -63,10 +66,20 @@ final class PdoIntegrationTest extends TestCase $found = $repo->find($id); $this->assertNotNull($found); $this->assertSame('dev-1', $found['device_id']); + $this->assertSame('factory-a', $found['site']); + $this->assertSame($uuid, $found['event_uuid']); $this->assertSame('critical', $found['severity']); $this->assertSame(['confidence' => 0.9], $found['metrics']); $this->assertNull($found['acknowledged_by']); + // Idempotency key lookup (P1-9). + $this->assertSame($id, $repo->idByUuid($uuid)); + $this->assertNull($repo->idByUuid(str_repeat('z', 32))); + + // Site-scoped page (P3-2b): matches its own site, excluded from another. + $this->assertCount(1, $repo->page(10, null, null, null, 'factory-a')); + $this->assertCount(0, $repo->page(10, null, null, null, 'factory-b')); + $recent = $repo->recent(10, 'critical'); $this->assertCount(1, $recent); $this->assertSame([], $repo->recent(10, 'info')); @@ -142,7 +155,9 @@ final class PdoIntegrationTest extends TestCase CREATE TABLE events ( event_id INTEGER PRIMARY KEY AUTOINCREMENT, + event_uuid TEXT UNIQUE, device_id TEXT NOT NULL, + site TEXT, camera_id TEXT NOT NULL, event_type TEXT NOT NULL, severity TEXT NOT NULL, diff --git a/cloud/tests/Support/InMemoryRepositories.php b/cloud/tests/Support/InMemoryRepositories.php index dfec277..2c219f7 100644 --- a/cloud/tests/Support/InMemoryRepositories.php +++ b/cloud/tests/Support/InMemoryRepositories.php @@ -7,8 +7,10 @@ use App\Repository\{DeviceRepository, NonceRepository, RateLimiterRepository, final class InMemoryDeviceRepository implements DeviceRepository { public array $heartbeats = []; - public function __construct(private array $secrets = [], private array $versions = []) {} + public function __construct(private array $secrets = [], private array $versions = [], + private array $sites = []) {} public function secretFor(string $d): ?string { return $this->secrets[$d] ?? null; } + public function siteFor(string $d): ?string { return $this->sites[$d] ?? null; } public function touchHeartbeat(string $d, array $m): void { $this->heartbeats[$d] = $m; } public function configVersion(string $d): int { return $this->versions[$d] ?? 1; } } @@ -24,14 +26,19 @@ final class InMemoryRateLimiter implements RateLimiterRepository { final class InMemoryEventRepository implements EventRepository { public array $rows = []; private int $seq = 0; public function insert(array $e): int { $id = ++$this->seq; $this->rows[$id] = $e + ['event_id' => $id, 'acknowledged_by' => null]; return $id; } + public function idByUuid(string $uuid): ?int { + foreach ($this->rows as $id => $r) if (($r['event_uuid'] ?? null) === $uuid) return (int)$id; + return null; + } public function recent(int $limit, ?string $sev): array { $r = array_reverse($this->rows, true); if ($sev !== null) $r = array_filter($r, fn($x) => $x['severity'] === $sev); return array_slice(array_values($r), 0, $limit); } - public function page(int $limit, ?string $sev, ?float $beforeTs, ?int $beforeId): array { + public function page(int $limit, ?string $sev, ?float $beforeTs, ?int $beforeId, ?string $site = null): array { $r = array_values($this->rows); if ($sev !== null) $r = array_filter($r, fn($x) => $x['severity'] === $sev); + if ($site !== null) $r = array_filter($r, fn($x) => ($x['site'] ?? null) === $site); // (occurred_at DESC, event_id DESC); events inserted without occurred_at sort as 0. usort($r, fn($a, $b) => [$b['occurred_at'] ?? 0, $b['event_id']] <=> [$a['occurred_at'] ?? 0, $a['event_id']]); if ($beforeTs !== null && $beforeId !== null) { @@ -48,6 +55,10 @@ final class InMemorySnapshotRepository implements SnapshotRepository { public function add(int $e, string $uuid, string $hash): void { $this->rows[$e][] = ['file_uuid' => $uuid, 'content_hash' => $hash]; $this->paths[$uuid] = "/tmp/$uuid.jpg"; $this->created[$uuid] = time(); } public function forEvent(int $e): array { return $this->rows[$e] ?? []; } public function pathByUuid(string $u): ?string { return $this->paths[$u] ?? null; } + public function eventIdByUuid(string $u): ?int { + foreach ($this->rows as $e => $list) foreach ($list as $r) if ($r['file_uuid'] === $u) return (int)$e; + return null; + } public function purgeOlderThan(int $cutoff): int { $n = 0; foreach ($this->created as $uuid => $ts) { diff --git a/docs/delivery-audit.md b/docs/delivery-audit.md index 85eef97..7d6312a 100644 --- a/docs/delivery-audit.md +++ b/docs/delivery-audit.md @@ -52,11 +52,14 @@ misconfig/degradation fail LOUD. forever at the head, blocking the whole queue. **Fix:** drop/quarantine after N failures. DONE — `attempts` column; a row is dropped after `max_send_attempts` (default 20) so a poison head unwedges the queue; test locks the drain-past-poison behavior. -- [ ] **P1-9 [M]** `edge/alert.py:43` — a POST that succeeds server-side but times out +- [x] **P1-9 [M]** `edge/alert.py:43` — a POST that succeeds server-side but times out client-side is re-queued and re-sent; no dedup key → duplicate alerts. **Fix:** stable client-generated `event_id` in the payload; cloud dedups on it. - DEFERRED to the cloud-ingest batch (needs a schema column + IngestController dedup); - sequenced with Phase 4 / P4-7. + DONE — the edge `Event` now carries a stable `event_uuid` (uuid4 hex, generated once at + creation, preserved through the offline queue + every retry). Cloud: `events.event_uuid` + CHAR(32) NULL UNIQUE; `EventValidator` accepts/validates it; `IngestController` returns the + already-stored event (200 `dedup:true`) on a repeat uuid, race-safe against the UNIQUE key. + Tests: queue round-trip keeps the uuid (edge); repeat-uuid dedups to one row (cloud). - [x] **P1-10 [M]** `edge/daemon.py:134` — `TARGET_FPS=0`/non-numeric → ZeroDivision/ValueError at every start → infinite systemd restart loop. **Fix:** validate, default sanely. DONE — `frame_interval_from_env` validates; test_config.py locks it. @@ -128,13 +131,15 @@ misconfig/degradation fail LOUD. DONE (retention = **7 days**) — `snapshot_retention_days` config; `purgeOlderThan` on the interface + Pdo + double (files then rows, bounded batch); opportunistic purge in the ingest path (cafe24 has no cron; the edge POSTs 24/7). IngestControllerTest locks it. -- [ ] **P3-2b [M]** `cloud/src/Dashboard/EventListController.php:17` — `site_scope` is stored +- [x] **P3-2b [M]** `cloud/src/Dashboard/EventListController.php:17` — `site_scope` is stored in the session but never enforced; any authenticated user sees every device's worker snapshots. **Fix:** scope event/snapshot queries by the session's `site_scope`. - NEEDS A DECISION — the schema has `users.site_scope` (free string) but `devices` has NO site - column and `events` link only by `device_id`. Enforcing scope needs a site↔device model - (e.g. add `devices.site`, filter events by the caller's site). Deferred until that is - decided. Interim mitigation: keep every dashboard account admin/single-site so scope is moot. + DONE (site↔device model approved) — added `devices.site`; ingest denormalizes it onto + `events.site` (server-derived, never client-supplied). `App\Security\SiteScope` centralizes the + rule (NULL scope = global operator). The event LIST filters in SQL (`page(...,$site)`); event + DETAIL + ACK and the SNAPSHOT proxy guard per-row and return 404 (not 403) cross-site so + existence never leaks. Migration ALTERs + provisioning UPDATEs documented in schema.sql. + Tests: list scoped, detail cross-site 404, snapshot cross-site 404 / same-site 200. - [x] **P3-3 [H]** `edge/tools/fetch_movenet.py:12` — downloads the ONNX from a mutable ref with no checksum; spec mandates SHA256. **Fix:** pin URL to a commit + sha256+size, mirror `fetch_hailo_hef.py`. @@ -247,6 +252,13 @@ misconfig/degradation fail LOUD. (updated as each item lands — newest first) +- **2026-07-17 — P1-9 + P3-2b landed (both approved data-model changes).** 40/43 done. + P1-9: edge `event_uuid` idempotency key (stable through the queue) + cloud dedup on a UNIQUE + column. P3-2b: `devices.site` → denormalized `events.site`; `SiteScope` enforced on the event + list (SQL filter), event detail/ack, and snapshot proxy (per-row 404). Schema migration + + provisioning UPDATEs documented in schema.sql. phpunit 97, pytest 158 — all green. REMAINING 3: + P2-6/7/8 (safety-timing / tracking-match — deferred to on-hardware commissioning). + - **2026-07-17 — off-hardware solo work COMPLETE (35/43).** Landed this pass, each test-locked: P4-8 (login CSRF), P4-6 (zone 400-not-500), P4-5 (cursor pagination), P4-7 (snapshot atomicity + orphan cleanup), P5-3 (HEF in the offline bundle + fixed the broken build), P5-4 (pin + hash + diff --git a/edge/events.py b/edge/events.py index 48fd08c..5c8fb04 100644 --- a/edge/events.py +++ b/edge/events.py @@ -1,4 +1,5 @@ -from dataclasses import dataclass +import uuid as _uuid +from dataclasses import dataclass, field from enum import Enum @@ -49,10 +50,15 @@ class Event: duration_s: float metrics: dict spatial: tuple | None # (x, y) 유령사고 등 공간 기준 + # Stable idempotency key: generated ONCE at event creation and carried unchanged through the + # offline queue and every retry, so a POST that succeeded server-side but timed out + # client-side is deduplicated by the cloud (see cloud IngestController) instead of stored twice. + event_uuid: str = field(default_factory=lambda: _uuid.uuid4().hex) def to_payload(self, device_id: str) -> dict: return { "device_id": device_id, + "event_uuid": self.event_uuid, "camera_id": self.camera_id, "event_type": self.event_type.value, "severity": self.severity.value, diff --git a/edge/queue.py b/edge/queue.py index fdb37f6..41e40de 100644 --- a/edge/queue.py +++ b/edge/queue.py @@ -25,10 +25,15 @@ class OfflineQueue: self.db.commit() def _rehydrate(self, payload: dict, snaps: list[bytes]) -> tuple[Event, list[bytes]]: + # Preserve the stored event_uuid so a re-sent event keeps its idempotency key and the + # cloud dedups it. Rows queued before this field existed simply get a fresh uuid (they + # predate dedup, so a first-and-only delivery is correct for them). + uuid_kw = {"event_uuid": payload["event_uuid"]} if payload.get("event_uuid") else {} e = Event(EventType(payload["event_type"]), Severity(payload["severity"]), payload["camera_id"], payload["track_id"], payload["zone_id"], payload["occurred_at"], payload["duration_s"], payload["metrics"], - tuple(payload["spatial"]) if payload.get("spatial") else None) + tuple(payload["spatial"]) if payload.get("spatial") else None, + **uuid_kw) return e, snaps def enqueue(self, event: Event, snapshots: list[bytes]) -> None: diff --git a/tests/test_queue.py b/tests/test_queue.py index cdf75e7..93a361a 100644 --- a/tests/test_queue.py +++ b/tests/test_queue.py @@ -60,6 +60,26 @@ def test_queue_is_bounded_and_keeps_newest(tmp_path): assert q.client.sent == [15, 16, 17, 18, 19] # the newest 5 survived +def test_event_uuid_survives_the_queue_roundtrip(tmp_path): + # A re-sent event (failed once -> queued -> flushed) must carry the SAME event_uuid so the + # cloud dedups it instead of storing a duplicate alert. This is the whole point of the key. + class _CaptureClient: + def __init__(self): + self.fail = True + self.uuids = [] + def send(self, event, snapshots): + self.uuids.append(event.event_uuid) + return not self.fail + client = _CaptureClient() + q = OfflineQueue(str(tmp_path / "q.db"), client) + ev = _event(1) + q.try_send(ev, [b"x"]) # fails -> queued (1st send attempt) + client.fail = False + assert q.flush() == 1 # re-sent from the queue (2nd send attempt) + assert len(client.uuids) == 2 + assert client.uuids[0] == client.uuids[1] == ev.event_uuid + + def test_poison_head_is_dropped_and_unblocks_the_queue(tmp_path): # A permanently-rejected event at the head must not wedge the whole queue forever; after # max_send_attempts it is dropped so the events behind it can drain.