From f493b96be39841ea4a22f6850f9346ab55d2e07e Mon Sep 17 00:00:00 2001
From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com>
Date: Sat, 28 Mar 2026 12:25:54 +0100
Subject: [PATCH] refactor: use random_int() for email change verification
 codes

Replace mt_rand/rand with random_int for stronger randomness guarantees
in verification code generation and Blade component keying.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 app/Models/User.php                           |   2 +-
 .../components/forms/monaco-editor.blade.php  |   2 +-
 tests/Feature/EmailChangeVerificationTest.php | 109 ++++++++++++++++++
 tests/Unit/InsecurePrngArchTest.php           |  17 +++
 4 files changed, 128 insertions(+), 2 deletions(-)
 create mode 100644 tests/Feature/EmailChangeVerificationTest.php
 create mode 100644 tests/Unit/InsecurePrngArchTest.php

diff --git a/app/Models/User.php b/app/Models/User.php
index 4561cddb2..7c68657e7 100644
--- a/app/Models/User.php
+++ b/app/Models/User.php
@@ -395,7 +395,7 @@ public function canAccessSystemResources(): bool
     public function requestEmailChange(string $newEmail): void
     {
         // Generate 6-digit code
-        $code = sprintf('%06d', mt_rand(0, 999999));
+        $code = sprintf('%06d', random_int(0, 999999));
 
         // Set expiration using config value
         $expiryMinutes = config('constants.email_change.verification_code_expiry_minutes', 10);
diff --git a/resources/views/components/forms/monaco-editor.blade.php b/resources/views/components/forms/monaco-editor.blade.php
index e774f5863..1a35be218 100644
--- a/resources/views/components/forms/monaco-editor.blade.php
+++ b/resources/views/components/forms/monaco-editor.blade.php
@@ -1,4 +1,4 @@
-<div wire:key="{{ rand() }}" class="coolify-monaco-editor flex-1">
+<div wire:key="{{ random_int(0, PHP_INT_MAX) }}" class="coolify-monaco-editor flex-1">
     <div x-ref="monacoRef" x-data="{
         monacoVersion: '0.52.2',
         monacoContent: @entangle($id),
diff --git a/tests/Feature/EmailChangeVerificationTest.php b/tests/Feature/EmailChangeVerificationTest.php
new file mode 100644
index 000000000..934ac9602
--- /dev/null
+++ b/tests/Feature/EmailChangeVerificationTest.php
@@ -0,0 +1,109 @@
+<?php
+
+use App\Models\User;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Support\Facades\Notification;
+
+uses(RefreshDatabase::class);
+
+it('generates a 6-digit verification code when requesting email change', function () {
+    Notification::fake();
+
+    $user = User::factory()->create();
+
+    $user->requestEmailChange('newemail@example.com');
+
+    $user->refresh();
+
+    expect($user->pending_email)->toBe('newemail@example.com')
+        ->and($user->email_change_code)->toMatch('/^\d{6}$/')
+        ->and($user->email_change_code_expires_at)->not->toBeNull();
+});
+
+it('stores the verification code using a cryptographically secure generator', function () {
+    Notification::fake();
+
+    $user = User::factory()->create();
+
+    // Generate many codes and verify they are all valid 6-digit strings
+    $codes = collect();
+    for ($i = 0; $i < 50; $i++) {
+        $user->requestEmailChange('newemail@example.com');
+        $user->refresh();
+        $codes->push($user->email_change_code);
+    }
+
+    // All codes should be exactly 6 digits (including leading zeros)
+    $codes->each(function ($code) {
+        expect($code)->toMatch('/^\d{6}$/');
+        expect((int) $code)->toBeLessThanOrEqual(999999);
+        expect(strlen($code))->toBe(6);
+    });
+
+    // With 50 random codes from a 1M space, we should see at least some variety
+    expect($codes->unique()->count())->toBeGreaterThan(1);
+});
+
+it('confirms email change with correct verification code', function () {
+    Notification::fake();
+
+    $user = User::factory()->create(['email' => 'old@example.com']);
+
+    $user->requestEmailChange('new@example.com');
+    $user->refresh();
+
+    $code = $user->email_change_code;
+
+    $result = $user->confirmEmailChange($code);
+
+    $user->refresh();
+
+    expect($result)->toBeTrue()
+        ->and($user->email)->toBe('new@example.com')
+        ->and($user->pending_email)->toBeNull()
+        ->and($user->email_change_code)->toBeNull()
+        ->and($user->email_change_code_expires_at)->toBeNull();
+});
+
+it('rejects email change with incorrect verification code', function () {
+    Notification::fake();
+
+    $user = User::factory()->create(['email' => 'old@example.com']);
+
+    $user->requestEmailChange('new@example.com');
+    $user->refresh();
+
+    $result = $user->confirmEmailChange('000000');
+
+    $user->refresh();
+
+    // If the real code happens to be '000000', this test still passes
+    // because the assertion is on the overall flow behavior
+    if ($user->email_change_code === '000000') {
+        expect($result)->toBeTrue();
+    } else {
+        expect($result)->toBeFalse()
+            ->and($user->email)->toBe('old@example.com');
+    }
+});
+
+it('rejects email change with expired verification code', function () {
+    Notification::fake();
+
+    $user = User::factory()->create(['email' => 'old@example.com']);
+
+    $user->requestEmailChange('new@example.com');
+    $user->refresh();
+
+    $code = $user->email_change_code;
+
+    // Expire the code manually
+    $user->update(['email_change_code_expires_at' => now()->subMinute()]);
+
+    $result = $user->confirmEmailChange($code);
+
+    $user->refresh();
+
+    expect($result)->toBeFalse()
+        ->and($user->email)->toBe('old@example.com');
+});
diff --git a/tests/Unit/InsecurePrngArchTest.php b/tests/Unit/InsecurePrngArchTest.php
new file mode 100644
index 000000000..3209ba0a0
--- /dev/null
+++ b/tests/Unit/InsecurePrngArchTest.php
@@ -0,0 +1,17 @@
+<?php
+
+/**
+ * Architecture tests to prevent use of insecure PRNGs in application code.
+ *
+ * mt_rand() and rand() are not cryptographically secure. Use random_int()
+ * or random_bytes() instead for any security-sensitive context.
+ *
+ * @see GHSA-33rh-4c9r-74pf
+ */
+arch('app code must not use mt_rand')
+    ->expect('App')
+    ->not->toUse(['mt_rand', 'mt_srand']);
+
+arch('app code must not use rand')
+    ->expect('App')
+    ->not->toUse(['rand', 'srand']);