Davidson CI testing & AI refinement

create-critter.php

@@ -327,15 +327,23 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && $userCredits > 0 && !$dailyLimitRea
 
             if ($imageData !== false && strlen($imageData) > 0 && strlen($imageData) <= 15 * 1024 * 1024) {
                 $imageInfo = @getimagesizefromstring($imageData);
-                $allowedImageTypes = ['image/webp', 'image/png', 'image/jpeg'];
+                // Extension is derived from the detected mime type below, so it always
+                // matches the actual bytes on disk instead of assuming every allowed
+                // type is webp.
+                $imageExtensionsByMime = [
+                    'image/webp' => 'webp',
+                    'image/png' => 'png',
+                    'image/jpeg' => 'jpg',
+                ];
+                $detectedMime = $imageInfo['mime'] ?? '';
 
-                if ($imageInfo !== false && in_array($imageInfo['mime'] ?? '', $allowedImageTypes, true)) {
+                if ($imageInfo !== false && isset($imageExtensionsByMime[$detectedMime])) {
                     $imagesDir = __DIR__ . '/critter-images';
                     if (!is_dir($imagesDir)) {
                         mkdir($imagesDir, 0755, true);
                     }
 
-                    $candidateFilename = bin2hex(random_bytes(16)) . '.webp';
+                    $candidateFilename = bin2hex(random_bytes(16)) . '.' . $imageExtensionsByMime[$detectedMime];
                     $bytesWritten = file_put_contents($imagesDir . '/' . $candidateFilename, $imageData);
                     if ($bytesWritten !== false) {
                         $imageFilename = $candidateFilename;

forgot-password.php

@@ -51,13 +51,27 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') {
         $ipIdentifier = 'ip:' . ($_SERVER['REMOTE_ADDR'] ?? 'unknown');
         $emailIdentifier = 'email:' . strtolower($email);
 
-        $rateLimited = passwordResetRequestCount($pdo, $ipIdentifier, PASSWORD_RESET_REQUEST_WINDOW_SECONDS) >= MAX_PASSWORD_RESET_REQUESTS_PER_IP
-            || passwordResetRequestCount($pdo, $emailIdentifier, PASSWORD_RESET_REQUEST_WINDOW_SECONDS) >= MAX_PASSWORD_RESET_REQUESTS_PER_EMAIL;
+        // Check both limits and record the reservation atomically, so concurrent requests
+        // can't all read the same stale counts before any of them commits. Kept as a short
+        // transaction that excludes the mail send below, since holding a write transaction
+        // open across a slow network call is what caused the "database is locked" issue
+        // fixed earlier in create-critter.php.
+        $pdo->exec('BEGIN IMMEDIATE');
+        try {
+            $rateLimited = passwordResetRequestCount($pdo, $ipIdentifier, PASSWORD_RESET_REQUEST_WINDOW_SECONDS) >= MAX_PASSWORD_RESET_REQUESTS_PER_IP
+                || passwordResetRequestCount($pdo, $emailIdentifier, PASSWORD_RESET_REQUEST_WINDOW_SECONDS) >= MAX_PASSWORD_RESET_REQUESTS_PER_EMAIL;
+
+            if (!$rateLimited) {
+                recordPasswordResetRequest($pdo, $ipIdentifier);
+                recordPasswordResetRequest($pdo, $emailIdentifier);
+            }
+            $pdo->exec('COMMIT');
+        } catch (Throwable $e) {
+            $pdo->exec('ROLLBACK');
+            throw $e;
+        }
 
         if (!$rateLimited) {
-            recordPasswordResetRequest($pdo, $ipIdentifier);
-            recordPasswordResetRequest($pdo, $emailIdentifier);
-
             $stmt = $pdo->prepare('SELECT id, username FROM users WHERE email = :email');
             $stmt->execute(['email' => $email]);
             $user = $stmt->fetch(PDO::FETCH_ASSOC);

login.php

@@ -54,22 +54,45 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') {
 
         $ipIdentifier = 'ip:' . ($_SERVER['REMOTE_ADDR'] ?? 'unknown');
         $emailIdentifier = 'email:' . strtolower($email);
+        $attemptIds = [];
 
-        if (
-            loginAttemptCount($pdo, $ipIdentifier, LOGIN_ATTEMPT_WINDOW_SECONDS) >= MAX_LOGIN_ATTEMPTS_PER_IP
-            || loginAttemptCount($pdo, $emailIdentifier, LOGIN_ATTEMPT_WINDOW_SECONDS) >= MAX_LOGIN_ATTEMPTS_PER_EMAIL
-        ) {
-            $errors['login'] = 'Too many login attempts. Please wait a while before trying again.';
-        } else {
+        // Check both limits and reserve attempt rows for both identifiers atomically, so
+        // concurrent requests can't all read the same stale counts before any of them
+        // commits. Reserving happens before password_verify() runs (rather than only on a
+        // confirmed failure) so the check-then-record pair is a single fast transaction;
+        // the reservation is released below if the login actually succeeds, so only failed
+        // attempts count against the limit — matching the previous behavior.
+        $pdo->exec('BEGIN IMMEDIATE');
+        try {
+            if (
+                loginAttemptCount($pdo, $ipIdentifier, LOGIN_ATTEMPT_WINDOW_SECONDS) >= MAX_LOGIN_ATTEMPTS_PER_IP
+                || loginAttemptCount($pdo, $emailIdentifier, LOGIN_ATTEMPT_WINDOW_SECONDS) >= MAX_LOGIN_ATTEMPTS_PER_EMAIL
+            ) {
+                $errors['login'] = 'Too many login attempts. Please wait a while before trying again.';
+            } else {
+                recordLoginAttempt($pdo, $ipIdentifier);
+                $attemptIds[] = (int) $pdo->lastInsertId();
+                recordLoginAttempt($pdo, $emailIdentifier);
+                $attemptIds[] = (int) $pdo->lastInsertId();
+            }
+            $pdo->exec('COMMIT');
+        } catch (Throwable $e) {
+            $pdo->exec('ROLLBACK');
+            throw $e;
+        }
+
+        if (empty($errors)) {
             $stmt = $pdo->prepare('SELECT * FROM users WHERE email = :email');
             $stmt->execute(['email' => $email]);
             $user = $stmt->fetch(PDO::FETCH_ASSOC);
 
             if (!$user || !password_verify($password, $user['password'])) {
-                recordLoginAttempt($pdo, $ipIdentifier);
-                recordLoginAttempt($pdo, $emailIdentifier);
                 $errors['login'] = 'Invalid email or password.';
             } else {
+                foreach ($attemptIds as $attemptId) {
+                    $pdo->prepare('DELETE FROM login_attempts WHERE id = :id')->execute(['id' => $attemptId]);
+                }
+
                 start_app_session();
                 session_regenerate_id(true);
                 $_SESSION['user_id'] = $user['id'];

register.php

@@ -45,12 +45,31 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') {
     $pdo->exec('CREATE INDEX IF NOT EXISTS idx_registration_attempts_identifier ON registration_attempts (identifier)');
 
     $ipIdentifier = 'ip:' . ($_SERVER['REMOTE_ADDR'] ?? 'unknown');
+    $registrationAttemptId = null;
 
     if (!csrf_verify($_POST['csrf_token'] ?? null)) {
         $errors['csrf'] = 'Your session has expired. Please refresh the page and try again.';
-    } elseif (registrationAttemptCount($pdo, $ipIdentifier, REGISTRATION_WINDOW_SECONDS) >= MAX_REGISTRATIONS_PER_IP) {
-        $errors['rate_limit'] = 'Too many accounts have been created from this network. Please try again later.';
     } else {
+        // Check the per-IP limit and reserve a slot atomically, so concurrent requests
+        // can't all read the same stale count before any of them commits. The reservation
+        // is released below if this request doesn't end up creating an account, so only
+        // actual registrations count against the limit — matching the previous behavior.
+        $pdo->exec('BEGIN IMMEDIATE');
+        try {
+            if (registrationAttemptCount($pdo, $ipIdentifier, REGISTRATION_WINDOW_SECONDS) >= MAX_REGISTRATIONS_PER_IP) {
+                $errors['rate_limit'] = 'Too many accounts have been created from this network. Please try again later.';
+            } else {
+                recordRegistrationAttempt($pdo, $ipIdentifier);
+                $registrationAttemptId = (int) $pdo->lastInsertId();
+            }
+            $pdo->exec('COMMIT');
+        } catch (Throwable $e) {
+            $pdo->exec('ROLLBACK');
+            throw $e;
+        }
+    }
+
+    if (empty($errors)) {
         if ($username === '') {
             $errors['username'] = 'Username is required.';
         } elseif (strlen($username) < 3 || strlen($username) > 12) {
@@ -97,12 +116,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') {
                     'email' => $email,
                     'password' => $hashedPassword,
                 ]);
-                // Capture the new user's id before any other insert runs on this connection —
-                // SQLite's lastInsertId() returns the most recent rowid inserted on the
-                // connection regardless of table, so recordRegistrationAttempt()'s own insert
-                // would otherwise clobber it and log the session in as the wrong user.
                 $newUserId = (int) $pdo->lastInsertId();
-                recordRegistrationAttempt($pdo, $ipIdentifier);
             } catch (PDOException $e) {
                 if (str_contains($e->getMessage(), 'UNIQUE constraint failed: users.username')) {
                     $errors['username'] = 'That username is not available.';
@@ -130,6 +144,13 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') {
             }
         }
     }
+
+    // Reaching here means this request didn't end in a created account (validation
+    // failed, a username/email collision was caught, or the rate limit itself
+    // rejected it) — release any reserved slot so only actual registrations count.
+    if ($registrationAttemptId !== null) {
+        $pdo->prepare('DELETE FROM registration_attempts WHERE id = :id')->execute(['id' => $registrationAttemptId]);
+    }
 }
 
 $pageTitle = 'Register';