test(phase-1): feature coverage for security + data-integrity fixes

Seven feature tests, run against an in-memory SQLite forced via putenv
in setUp (keeps the shared dev DB safe). Covers:

- Reserved-identifier rule rejects admin / super_admin / takeone /
  support / etc, matches both name and email local-part only, and
  lets normal names through.
- SafeNotifier swallows a notification throw without re-raising.
- Identical comments within 60s collapse to the first row.
- X-Idempotency-Key returns the first comment on retry.
- Owner viewing their own video does not insert a video_views row.

AppServiceProvider stops force-upgrading the scheme in the testing env
so the HTTP test client isn't 302'd before reaching the controller.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

app/Providers/AppServiceProvider.php

@@ -26,8 +26,12 @@ class AppServiceProvider extends ServiceProvider
             \Illuminate\Http\Request::HEADER_X_FORWARDED_AWS_ELB
         );
         
-        // Force HTTPS
+        // Force HTTPS unless we're running under phpunit — the test client
-        URL::forceScheme('https');
+        // speaks plain http://localhost; forcing https would make every POST
+        // in a feature test resolve to a 302 instead of hitting the controller.
+        if (! $this->app->environment('testing')) {
+            URL::forceScheme('https');
+        }
 
         // Universal pagination view — used everywhere by default
         Paginator::defaultView('partials.pagination');

tests/Feature/Phase1SecurityTest.php

@@ -0,0 +1,180 @@
+<?php
+
+namespace Tests\Feature;
+
+use App\Models\Comment;
+use App\Models\User;
+use App\Models\Video;
+use App\Rules\ReservedIdentifier;
+use App\Support\SafeNotifier;
+use App\Http\Middleware\VerifyCsrfToken;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Support\Facades\DB;
+use Tests\TestCase;
+
+/**
+ * Phase 1 audit remediation: security + data-integrity regression tests.
+ *
+ * These tests run on a scratch SQLite file in storage/testing.sqlite so they
+ * can't collide with the shared dev DB. phpunit.xml keeps DB_CONNECTION out
+ * of the test env so this per-class override is where the swap happens.
+ */
+class Phase1SecurityTest extends TestCase
+{
+    use RefreshDatabase;
+
+    protected function setUp(): void
+    {
+        // Force the test runner onto an in-memory SQLite BEFORE the framework
+        // boots, so RefreshDatabase migrates the throwaway DB and never the
+        // shared dev file. Env vars are read during app bootstrap in
+        // CreatesApplication, which is why putenv has to come first.
+        putenv('DB_CONNECTION=sqlite');
+        putenv('DB_DATABASE=:memory:');
+        $_ENV['DB_CONNECTION'] = 'sqlite';
+        $_ENV['DB_DATABASE']   = ':memory:';
+
+        parent::setUp();
+
+        // Strip the two middleware that fight the test client: CSRF (no
+        // token flow in postJson) and AuthenticateSession (which rotates
+        // the session on cross-request assertions and 302s to APP_URL).
+        $this->withoutMiddleware([
+            VerifyCsrfToken::class,
+            \Illuminate\Session\Middleware\AuthenticateSession::class,
+        ]);
+    }
+
+    // ─── ReservedIdentifier rule ──────────────────────────
+
+    public function test_reserved_name_rule_blocks_impersonation_candidates(): void
+    {
+        $rule = new ReservedIdentifier('name');
+
+        foreach (['admin', 'Administrator', 'super_admin', 'super-admin',
+                  'SuperAdmin', 'TAKEONE', 'support', 'root', 'Postmaster'] as $name) {
+            $failed = false;
+            $rule->validate('name', $name, function () use (&$failed) { $failed = true; });
+            $this->assertTrue($failed, "Expected name '{$name}' to be reserved");
+        }
+    }
+
+    public function test_reserved_email_rule_matches_on_local_part_only(): void
+    {
+        $rule = new ReservedIdentifier('email');
+
+        $failed = false;
+        $rule->validate('email', '[email protected]', function () use (&$failed) { $failed = true; });
+        $this->assertTrue($failed);
+
+        $failed = false;
+        $rule->validate('email', '[email protected]', function () use (&$failed) { $failed = true; });
+        $this->assertFalse($failed, 'Only exact local-part should match; "admin.user" is a real person.');
+    }
+
+    public function test_reserved_rule_allows_normal_names(): void
+    {
+        $rule = new ReservedIdentifier('name');
+
+        foreach (['Alice', 'Bob Smith', 'John Doe', 'الحسن'] as $name) {
+            $failed = false;
+            $rule->validate('name', $name, function () use (&$failed) { $failed = true; });
+            $this->assertFalse($failed, "Expected name '{$name}' to be allowed");
+        }
+    }
+
+    // ─── SafeNotifier swallows mail failures ──────────────
+
+    public function test_safe_notifier_logs_but_does_not_throw_on_failure(): void
+    {
+        // A fake notifiable whose notify() blows up — stands in for the Gmail
+        // 535 case where sending throws out of the transport layer.
+        $exploding = new class {
+            public function notify($n): void { throw new \RuntimeException('smtp exploded'); }
+            public function getKey(): int { return 1; }
+        };
+
+        $notification = new \App\Notifications\VerifyEmail;
+
+        // Call must not throw — if it did, PHPUnit would mark this test as errored.
+        SafeNotifier::send($exploding, $notification);
+
+        $this->assertTrue(true, 'SafeNotifier swallowed the throw');
+    }
+
+    // ─── Comment dedupe ───────────────────────────────────
+
+    public function test_identical_comments_within_60s_are_collapsed(): void
+    {
+        [$user, $video] = $this->makeUserAndVideo();
+        $this->actingAs($user);
+
+        $first = $this->postJson("/videos/".Video::encodeId($video->id)."/comments", ['body' => 'Great goal!']);
+        $first->assertOk();
+        $firstId = $first->json('comment.id');
+
+        $second = $this->postJson("/videos/".Video::encodeId($video->id)."/comments", ['body' => 'Great goal!']);
+        $second->assertOk();
+        $secondId = $second->json('comment.id');
+
+        $this->assertSame($firstId, $secondId, 'Duplicate within 60s should return the first row, not insert a new one.');
+        $this->assertSame(1, Comment::where('video_id', $video->id)->count());
+    }
+
+    public function test_idempotency_key_returns_the_same_comment(): void
+    {
+        [$user, $video] = $this->makeUserAndVideo();
+        $this->actingAs($user);
+
+        $key = 'test-idem-key-' . bin2hex(random_bytes(8));
+
+        $a = $this->withHeaders(['X-Idempotency-Key' => $key])
+                  ->postJson("/videos/".Video::encodeId($video->id)."/comments", ['body' => 'First body']);
+        $a->assertOk();
+        $aId = $a->json('comment.id');
+
+        // Different body, same key — because the first write cached the id,
+        // the second call returns it unchanged.
+        $b = $this->withHeaders(['X-Idempotency-Key' => $key])
+                  ->postJson("/videos/".Video::encodeId($video->id)."/comments", ['body' => 'Different body']);
+        $b->assertOk();
+        $bId = $b->json('comment.id');
+
+        $this->assertSame($aId, $bId, 'Same idempotency key must return the same comment id.');
+    }
+
+    // ─── Owner view exclusion ─────────────────────────────
+
+    public function test_owner_view_is_not_recorded(): void
+    {
+        [$user, $video] = $this->makeUserAndVideo();
+        $this->actingAs($user);
+
+        $this->get("/videos/{$video->id}");
+
+        $this->assertSame(0, DB::table('video_views')->where('video_id', $video->id)->count(),
+            'Views by the video owner must not be counted.');
+    }
+
+    // ─── Helpers ──────────────────────────────────────────
+
+    private function makeUserAndVideo(): array
+    {
+        $user = User::factory()->create([
+            'username' => 'u'.substr(bin2hex(random_bytes(4)), 0, 8),
+        ]);
+
+        $video = Video::create([
+            'user_id'    => $user->id,
+            'title'      => 'Test video',
+            'description'=> 'Hello',
+            'filename'   => 'users/x/videos/y/video.mp4',
+            'path'       => 'users/x/videos/y/video.mp4',
+            'type'       => 'generic',
+            'visibility' => 'public',
+            'status'     => 'ready',
+        ]);
+
+        return [$user, $video];
+    }
+}