fix(cloud): don't swallow snapshot failures, clean orphan JPEGs (P4-7)

The ingest snapshot loop silently `continue`d on re-encode and file-write
failures (evidence lost with a 201), and a DB-index throw after the file
landed left an orphan JPEG on disk with no row — or bubbled into a 500
that discarded an already-stored event.

Extracts storeSnapshot(): re-encode → write file → index, in that order
so bytes exist before a row points at them. Every failure is error_log'd
instead of swallowed; a DB-index failure after the write @unlinks the
orphan file. A snapshot problem no longer fails the ingest — the event is
already persisted, so the 201 stands.

Test: testSnapshotIndexFailureCleansOrphanFileButKeepsEvent uses a
throwing snapshot repo and asserts 201, the event is stored, and no *.jpg
is left in the snapshot dir.

phpunit: 90 passing (4 skipped: PDO-backed).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019Xmn8KZWqiQUguEMRtn6rW

cloud/src/Ingest/IngestController.php

@@ -68,11 +68,7 @@ final class IngestController
 
         $eventId = $this->events->insert($v['clean']);
         foreach (($payload['snapshots'] ?? []) as $b64) {
-            $img = ImageProcessor::reencode((string)$b64);
-            if (!$img['ok']) continue;
-            $path = rtrim($this->cfg->snapshotDir(), '/\\') . '/' . $img['uuid'] . '.jpg';
-            if (file_put_contents($path, $img['bytes']) === false) continue;
-            $this->snapshots->add($eventId, $img['uuid'], $img['hash']);
+            $this->storeSnapshot($eventId, (string)$b64);
         }
         // PIPA retention: purge worker snapshots older than the configured window. Runs rarely
         // (the gate) and off the critical path — the event is already stored.
@@ -82,4 +78,31 @@ final class IngestController
         }
         return Response::json(['event_id' => $eventId], 201);
     }
+
+    /**
+     * Persist one snapshot: re-encode → write file → index in the DB, in that order so the
+     * bytes exist on disk before a row can point at them. Failures are logged (never silently
+     * swallowed) and, critically, a DB-index failure AFTER the file lands deletes the orphan
+     * JPEG so disk never accumulates evidence with no index row. A snapshot problem never fails
+     * the ingest — the event itself is already stored and the 201 stands.
+     */
+    private function storeSnapshot(int $eventId, string $b64): void
+    {
+        $img = ImageProcessor::reencode($b64);
+        if (!$img['ok']) {
+            error_log("ingest: snapshot re-encode failed for event $eventId (malformed or oversized)");
+            return;
+        }
+        $path = rtrim($this->cfg->snapshotDir(), '/\\') . '/' . $img['uuid'] . '.jpg';
+        if (file_put_contents($path, $img['bytes']) === false) {
+            error_log("ingest: snapshot file write failed for event $eventId at $path");
+            return;
+        }
+        try {
+            $this->snapshots->add($eventId, $img['uuid'], $img['hash']);
+        } catch (\Throwable $e) {
+            @unlink($path);
+            error_log("ingest: snapshot index failed for event $eventId, removed orphan {$img['uuid']}: " . $e->getMessage());
+        }
+    }
 }

cloud/tests/IngestControllerTest.php

@@ -84,6 +84,26 @@ final class IngestControllerTest extends TestCase
         $this->assertArrayHasKey($recent, $snaps->created, 'a 1-day-old snapshot must survive');
     }
 
+    public function testSnapshotIndexFailureCleansOrphanFileButKeepsEvent(): void {
+        // The file lands, then the DB index throws. The event must still be stored (201) and the
+        // orphan JPEG must be removed — never left on disk with no row pointing at it.
+        $cfg = Config::fromArray(['snapshot_dir' => $this->dir, 'timestamp_window' => 300, 'rate_limit_per_min' => 60]);
+        $throwingSnaps = new class implements \App\Repository\SnapshotRepository {
+            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 purgeOlderThan(int $c): int { return 0; }
+        };
+        $events = new InMemoryEventRepository();
+        $c = new IngestController($cfg, new InMemoryDeviceRepository(['pi-1' => 'sekret']),
+            new InMemoryNonceRepository(), new InMemoryRateLimiter(), $events, $throwingSnaps);
+        $res = $c->handle($this->signedReq('sekret', $this->payload()), now: 1000, clientIp: '1.2.3.4');
+        $this->assertSame(201, $res->status());  // a snapshot failure never discards the event
+        $this->assertCount(1, $events->rows);
+        $this->assertSame([], glob($this->dir . '/*.jpg'),
+            'orphan snapshot file must be removed when the DB index write fails');
+    }
+
     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

docs/delivery-audit.md

@@ -171,9 +171,14 @@ misconfig/degradation fail LOUD.
   clear error.
   DONE — ZoneConfigController::save wraps `$this->zones->save()` in try/catch(\RuntimeException)
   → `invalid_zone` 400; test `testSaveOwnershipMismatchIs400NotError` with a throwing repo double.
-- [ ] **P4-7 [L]** `cloud/src/Ingest/IngestController.php:64` — snapshot write failure is
+- [x] **P4-7 [L]** `cloud/src/Ingest/IngestController.php:64` — snapshot write failure is
   swallowed (201 with evidence lost); file-write/DB-insert non-atomic → orphan JPEGs. **Fix:**
   order write→insert, log failures, reconcile orphans.
+  DONE — extracted `storeSnapshot()`: re-encode→write→index in order; every failure (re-encode,
+  file write, DB index) now `error_log`s instead of a silent `continue`; a DB-index failure after
+  the file lands `@unlink`s the orphan. A snapshot problem no longer fails the ingest — the event
+  is already stored, 201 stands. Test `testSnapshotIndexFailureCleansOrphanFileButKeepsEvent`
+  (throwing repo double → asserts 201, event stored, no leftover *.jpg).
 - [x] **P4-8 [L]** `cloud/src/Auth/AuthController.php:31` — login form emits a CSRF token that
   `login` never checks. **Fix:** verify it (or drop it).
   DONE — verified (completed the half-built defense, not dropped). login() checks