fix(cloud): verify login CSRF + zone ownership 400 not 500 (P4-8, P4-6)

P4-8: AuthController::login now verifies the CSRF token the login form
already emits (Csrf::check over session vs posted field), returning 403
before the user lookup. Closes a login-CSRF / session-fixation hole where
a forged cross-site POST could silently log a victim into an attacker's
account. Tests seed the session+body token; adds
testLoginRejectsMissingOrWrongCsrf.

P4-6: ZoneConfigController::save wraps the repository write in
try/catch(\RuntimeException) so an ownership/device mismatch is a 400
(invalid_zone) client error, not an uncaught 500. Adds
testSaveOwnershipMismatchIs400NotError with a throwing repo double.

phpunit: 88 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/Auth/AuthController.php

@@ -3,7 +3,7 @@ declare(strict_types=1);
 namespace App\Auth;
 
 use App\Http\{Request, Response};
-use App\Security\Password;
+use App\Security\{Password, Csrf};
 use App\Repository\{UserRepository, AuditRepository, RateLimiterRepository};
 
 final class AuthController
@@ -28,6 +28,14 @@ final class AuthController
             return Response::json(['error' => 'rate_limited'], 429);
         }
 
+        // Login CSRF: the GET /login render seeds $_SESSION['csrf'] and embeds it as a hidden
+        // field; the POST must echo it back. Without this an attacker can silently log a victim
+        // into the attacker's account (session fixation via forged login). Checked before the
+        // user lookup so a forged request never triggers the (expensive) Argon2id verify.
+        if (!Csrf::check((string)$sess->get('csrf'), (string)$req->post('csrf'))) {
+            return Response::json(['error' => 'csrf'], 403);
+        }
+
         $username = (string)$req->post('username');
         $password = (string)$req->post('password');
         $u = $this->users->byUsername($username);

cloud/src/Dashboard/ZoneConfigController.php

@@ -41,13 +41,19 @@ final class ZoneConfigController
         if (!in_array($level, self::LEVELS, true) || !is_array($polygon) || !Polygon::validate($polygon)) {
             return Response::json(['error' => 'invalid_zone'], 400);
         }
-        $zoneId = $this->zones->save([
-            'zone_id' => $b['zone_id'] ?? null,
-            'device_id' => (string)($b['device_id'] ?? ''),
-            'camera_id' => (string)($b['camera_id'] ?? ''),
-            'level' => $level,
-            'polygon_json' => json_encode($polygon),
-        ]);
+        try {
+            $zoneId = $this->zones->save([
+                'zone_id' => $b['zone_id'] ?? null,
+                'device_id' => (string)($b['device_id'] ?? ''),
+                'camera_id' => (string)($b['camera_id'] ?? ''),
+                'level' => $level,
+                'polygon_json' => json_encode($polygon),
+            ]);
+        } catch (\RuntimeException $e) {
+            // Editing a zone with a device_id/camera_id that does not own it (or a zone deleted
+            // concurrently) is a CLIENT error, not a server fault — return 400, not a 500.
+            return Response::json(['error' => 'invalid_zone'], 400);
+        }
         $this->audit->append((string)$this->sess->get('username'), 'zone_save', "zone:$zoneId", [], ['level' => $level]);
         return Response::json(['zone_id' => $zoneId], 201);
     }

cloud/tests/AuthControllerTest.php

@@ -15,12 +15,16 @@ final class AuthControllerTest extends TestCase
             'role' => 'operator', 'site_scope' => null, 'failed_attempts' => 0, 'locked_until' => 0,
         ]]);
     }
+    private function sess(): ArraySessionStore {
+        // Model the real flow: GET /login already seeded the session csrf token before POST.
+        $s = new ArraySessionStore(); $s->set('csrf', 'tok'); return $s;
+    }
     private function req(string $u, string $p): Request {
-        return new Request('POST', '/login', [], '', [], ['username' => $u, 'password' => $p]);
+        return new Request('POST', '/login', [], '', [], ['username' => $u, 'password' => $p, 'csrf' => 'tok']);
     }
     public function testLoginSuccessSetsSession(): void {
         $c = new AuthController($this->users(), new InMemoryAuditRepository(), new InMemoryRateLimiter());
-        $sess = new ArraySessionStore();
+        $sess = $this->sess();
         $res = $c->login($this->req('alice', 'pw'), $sess, now: 1000, clientIp: '1.2.3.4');
         $this->assertSame(200, $res->status());
         $this->assertSame(1, $sess->get('user_id'));
@@ -28,39 +32,39 @@ final class AuthControllerTest extends TestCase
     }
     public function testBadPasswordIs401(): void {
         $c = new AuthController($this->users(), new InMemoryAuditRepository(), new InMemoryRateLimiter());
-        $this->assertSame(401, $c->login($this->req('alice', 'nope'), new ArraySessionStore(), now: 1000, clientIp: '1.2.3.4')->status());
+        $this->assertSame(401, $c->login($this->req('alice', 'nope'), $this->sess(), now: 1000, clientIp: '1.2.3.4')->status());
     }
     public function testLockoutAfterMaxAttempts(): void {
         $repo = $this->users();
         $c = new AuthController($repo, new InMemoryAuditRepository(), new InMemoryRateLimiter(), maxAttempts: 3, lockSeconds: 900);
-        for ($i = 0; $i < 3; $i++) $c->login($this->req('alice', 'bad'), new ArraySessionStore(), now: 1000, clientIp: '1.2.3.4');
+        for ($i = 0; $i < 3; $i++) $c->login($this->req('alice', 'bad'), $this->sess(), now: 1000, clientIp: '1.2.3.4');
         // now locked, even correct password is 423
-        $this->assertSame(423, $c->login($this->req('alice', 'pw'), new ArraySessionStore(), now: 1000, clientIp: '1.2.3.4')->status());
+        $this->assertSame(423, $c->login($this->req('alice', 'pw'), $this->sess(), now: 1000, clientIp: '1.2.3.4')->status());
     }
     public function testExpiredLockResetsAttempts(): void {
         $repo = $this->users();
         $c = new AuthController($repo, new InMemoryAuditRepository(), new InMemoryRateLimiter(), maxAttempts: 3, lockSeconds: 900);
         // lock the account with 3 bad tries at now=1000
-        for ($i = 0; $i < 3; $i++) $c->login($this->req('alice', 'bad'), new ArraySessionStore(), now: 1000, clientIp: '1.2.3.4');
+        for ($i = 0; $i < 3; $i++) $c->login($this->req('alice', 'bad'), $this->sess(), now: 1000, clientIp: '1.2.3.4');
         // locked_until is now 1000 + 900 = 1900, verify it's locked
-        $this->assertSame(423, $c->login($this->req('alice', 'pw'), new ArraySessionStore(), now: 1000, clientIp: '1.2.3.4')->status());
+        $this->assertSame(423, $c->login($this->req('alice', 'pw'), $this->sess(), now: 1000, clientIp: '1.2.3.4')->status());
         // now at now=2000 (after lock window expires at 1900), login with correct password should succeed
-        $sess = new ArraySessionStore();
+        $sess = $this->sess();
         $res = $c->login($this->req('alice', 'pw'), $sess, now: 2000, clientIp: '1.2.3.4');
         $this->assertSame(200, $res->status());
         $this->assertSame(1, $sess->get('user_id'));
     }
     public function testUnknownUsernameReturns401GenericBody(): void {
         $c = new AuthController($this->users(), new InMemoryAuditRepository(), new InMemoryRateLimiter());
-        $res = $c->login($this->req('nobody', 'whatever'), new ArraySessionStore(), now: 1000, clientIp: '1.2.3.4');
+        $res = $c->login($this->req('nobody', 'whatever'), $this->sess(), now: 1000, clientIp: '1.2.3.4');
         $this->assertSame(401, $res->status());
         // identical generic body to the wrong-password case (no user-enumeration leak)
-        $wrong = $c->login($this->req('alice', 'nope'), new ArraySessionStore(), now: 1000, clientIp: '1.2.3.4');
+        $wrong = $c->login($this->req('alice', 'nope'), $this->sess(), now: 1000, clientIp: '1.2.3.4');
         $this->assertSame($wrong->body(), $res->body());
     }
     public function testLogoutClearsSession(): void {
         $c = new AuthController($this->users(), new InMemoryAuditRepository(), new InMemoryRateLimiter());
-        $sess = new ArraySessionStore();
+        $sess = $this->sess();
         $c->login($this->req('alice', 'pw'), $sess, now: 1000, clientIp: '1.2.3.4');
         $this->assertSame(1, $sess->get('user_id'));
         $res = $c->logout($sess);
@@ -72,8 +76,22 @@ final class AuthControllerTest extends TestCase
         // throttled before the (expensive) Argon2id verify runs repeatedly.
         $c = new AuthController($this->users(), new InMemoryAuditRepository(), new InMemoryRateLimiter(), loginIpLimitPerMin: 2);
         $ip = '9.9.9.9';
-        $this->assertSame(200, $c->login($this->req('alice', 'pw'), new ArraySessionStore(), now: 1000, clientIp: $ip)->status());
-        $this->assertSame(200, $c->login($this->req('alice', 'pw'), new ArraySessionStore(), now: 1000, clientIp: $ip)->status());
-        $this->assertSame(429, $c->login($this->req('alice', 'pw'), new ArraySessionStore(), now: 1000, clientIp: $ip)->status());
+        $this->assertSame(200, $c->login($this->req('alice', 'pw'), $this->sess(), now: 1000, clientIp: $ip)->status());
+        $this->assertSame(200, $c->login($this->req('alice', 'pw'), $this->sess(), now: 1000, clientIp: $ip)->status());
+        $this->assertSame(429, $c->login($this->req('alice', 'pw'), $this->sess(), now: 1000, clientIp: $ip)->status());
+    }
+    public function testLoginRejectsMissingOrWrongCsrf(): void {
+        // Login CSRF: a forged cross-site POST would carry no (or a wrong) csrf field and must be
+        // rejected 403 BEFORE any credential check — even with an otherwise-valid password.
+        $c = new AuthController($this->users(), new InMemoryAuditRepository(), new InMemoryRateLimiter());
+        // missing csrf field in the body
+        $missing = new Request('POST', '/login', [], '', [], ['username' => 'alice', 'password' => 'pw']);
+        $this->assertSame(403, $c->login($missing, $this->sess(), now: 1000, clientIp: '1.2.3.4')->status());
+        // wrong csrf token
+        $wrong = new Request('POST', '/login', [], '', [], ['username' => 'alice', 'password' => 'pw', 'csrf' => 'nope']);
+        $this->assertSame(403, $c->login($wrong, $this->sess(), now: 1000, clientIp: '1.2.3.4')->status());
+        // session that never seeded a token (empty expected) also rejects, even with a matching-looking field
+        $noTok = new ArraySessionStore();
+        $this->assertSame(403, $c->login($this->req('alice', 'pw'), $noTok, now: 1000, clientIp: '1.2.3.4')->status());
     }
 }

cloud/tests/ZoneConfigTest.php

@@ -33,6 +33,20 @@ final class ZoneConfigTest extends TestCase
         $this->assertSame(201, $res->status());
         $this->assertCount(1, $repo->forCamera('cam-1'));
     }
+    public function testSaveOwnershipMismatchIs400NotError(): void {
+        // The repo throws RuntimeException when the zone isn't owned by the given device/camera.
+        // The controller must turn that client error into 400, not let it become a 500.
+        $throwing = new class implements \App\Repository\ZoneRepository {
+            public function forDevice(string $d): array { return []; }
+            public function forCamera(string $c): array { return []; }
+            public function save(array $z): string { throw new \RuntimeException('not owned'); }
+            public function delete(string $id): bool { return true; }
+        };
+        $c = new ZoneConfigController($throwing, new InMemoryAuditRepository(), $this->authed('operator'));
+        $res = $c->save($this->saveReq(['zone_id' => 'z1', 'device_id' => 'pi-2', 'camera_id' => 'cam-1',
+            'level' => 'restricted', 'polygon' => [[0, 0], [9, 0], [9, 9]]]));
+        $this->assertSame(400, $res->status());
+    }
     public function testSaveRejectsBadLevelOrPolygon(): void {
         $c = new ZoneConfigController(new InMemoryZoneRepository(), new InMemoryAuditRepository(), $this->authed('operator'));
         $this->assertSame(400, $c->save($this->saveReq(['camera_id' => 'cam-1', 'level' => 'BOOM', 'polygon' => [[0,0],[9,0],[9,9]]]))->status());

docs/delivery-audit.md

@@ -161,14 +161,20 @@ misconfig/degradation fail LOUD.
   DONE — dispatch rawurldecodes captured params; BasePathTest locks space + Korean cases.
 - [ ] **P4-5 [M]** `cloud/src/Dashboard/EventListController.php:17` — event list hard-capped at
   newest 100, no pagination/date filter; older events unreachable. **Fix:** cursor pagination.
-- [ ] **P4-6 [M]** `cloud/src/Repository/Pdo/PdoZoneRepository.php:86` — editing a zone with a
+- [x] **P4-6 [M]** `cloud/src/Repository/Pdo/PdoZoneRepository.php:86` — editing a zone with a
   changed device_id/camera_id throws an uncaught RuntimeException → 500. **Fix:** 400 with a
   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
   swallowed (201 with evidence lost); file-write/DB-insert non-atomic → orphan JPEGs. **Fix:**
   order write→insert, log failures, reconcile orphans.
-- [ ] **P4-8 [L]** `cloud/src/Auth/AuthController.php:31` — login form emits a CSRF token that
+- [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
+  `Csrf::check($sess->get('csrf'), $req->post('csrf'))` → 403 before the user lookup, closing the
+  login-CSRF / session-fixation hole. Tests updated to seed the session+body token; added
+  `testLoginRejectsMissingOrWrongCsrf`.
 - [x] **P4-9 [L]** `cloud/src/Auth/AuthController.php:71` — logout calls `regenerate()` after
   `session_destroy()` → E_WARNING, old cookie not replaced. **Fix:** reorder.
   DONE — logout() rotates the id then wipes (was regenerate-after-destroy).
@@ -209,6 +215,11 @@ misconfig/degradation fail LOUD.
 
 (updated as each item lands — newest first)
 
+- **2026-07-17 — P4-6 + P4-8 landed (cloud error/auth hardening).** ZoneConfig ownership-mismatch
+  now 400 not 500 (P4-6); login CSRF token now verified — closes login-CSRF/session-fixation
+  (P4-8). phpunit 88 green (4 skipped: PDO). Remaining solo: P4-5 (pagination), P4-7 (atomic
+  snapshot), P5-3 (HEF in bundle), P5-4 (pin+hash+SBOM).
+
 - **2026-07-17 — 17 items landed, all with tests, full suite green.** ALL 4 BLOCKERS cleared
   (P1-2, P2-1, P2-2, P5-1) + all Phase-1 silent-failure (P1-1,3,4,5,6,7,8,10,11) + Phase-2 HIGH
   correctness (P2-3,4,5). Remaining: P1-9 (deferred to cloud batch), P2-6..11 (mostly threshold