fix: session store kind column + duplicate error handling (#3172)

2026-08-26 16:07:13 +00:00 · 2026-05-27 11:44:08 -07:00
parent e95cf44fec
commit 716a8d06e2
4 changed files with 206 additions and 32 deletions
@@ -82,6 +82,9 @@ const AVAILABLE_MIGRATIONS: [number, string[]][] = [
    [47, ['0051_sessions_v2.sql']],
    [48, ['0052_sessions_v2_lookups.sql']],
    [49, ['0053_sessions_access_token_uid.sql']],
+    [50, ['0054_sessions_workers.sql']],
+    [50, ['0055_username_nocase_unique.sql']],
+    [51, ['0056_sessions_kind_worker.sql']],
 ];

 export class SqliteDatabaseClient extends AbstractDatabaseClient {
@@ -0,0 +1,50 @@
+-- Copyright (C) 2024-present Puter Technologies Inc.
+--
+-- This file is part of Puter.
+--
+-- Puter is free software: you can redistribute it and/or modify
+-- it under the terms of the GNU Affero General Public License as published
+-- by the Free Software Foundation, either version 3 of the License, or
+-- (at your option) any later version.
+--
+-- This program is distributed in the hope that it will be useful,
+-- but WITHOUT ANY WARRANTY; without even the implied warranty of
+-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+-- GNU Affero General Public License for more details.
+--
+-- You should have received a copy of the GNU Affero General Public License
+-- along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+-- Add 'worker' to the `sessions.kind` ENUM. mig_11 already added the
+-- worker uniqueness index and the application code in SessionStore
+-- inserts rows with kind='worker', but the ENUM defined in mig_8 never
+-- listed 'worker' as a permitted value. Under STRICT_TRANS_TABLES the
+-- INSERT fails outright; under a relaxed sql_mode the value is coerced
+-- to '' (and the worker SELECT-by-kind path then misses the row).
+-- Mirrors SQLite migration 0056.
+--
+-- Idempotent: guarded against COLUMN_TYPE so re-running the directory is
+-- a no-op once 'worker' is in the ENUM.
+
+DROP PROCEDURE IF EXISTS _puter_sessions_kind_worker;
+DELIMITER //
+CREATE PROCEDURE _puter_sessions_kind_worker()
+BEGIN
+  IF NOT EXISTS (
+    SELECT 1 FROM INFORMATION_SCHEMA.COLUMNS
+    WHERE TABLE_SCHEMA = DATABASE()
+      AND TABLE_NAME = 'sessions'
+      AND COLUMN_NAME = 'kind'
+      AND FIND_IN_SET('worker', REPLACE(REPLACE(REPLACE(COLUMN_TYPE, 'enum(', ''), ')', ''), '''', '')) > 0
+  ) THEN
+    ALTER TABLE `sessions`
+      MODIFY COLUMN `kind`
+        ENUM('web', 'app', 'access_token', 'asset', 'worker')
+        NOT NULL DEFAULT 'web';
+  END IF;
+END//
+DELIMITER ;
+
+CALL _puter_sessions_kind_worker();
+
+DROP PROCEDURE IF EXISTS _puter_sessions_kind_worker;
@@ -0,0 +1,90 @@
+-- Copyright (C) 2024-present Puter Technologies Inc.
+--
+-- This file is part of Puter.
+--
+-- Puter is free software: you can redistribute it and/or modify
+-- it under the terms of the GNU Affero General Public License as published
+-- by the Free Software Foundation, either version 3 of the License, or
+-- (at your option) any later version.
+--
+-- This program is distributed in the hope that it will be useful,
+-- but WITHOUT ANY WARRANTY; without even the implied warranty of
+-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+-- GNU Affero General Public License for more details.
+--
+-- You should have received a copy of the GNU Affero General Public License
+-- along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+-- Add 'worker' to the `kind` CHECK constraint on `sessions`. Mirrors the
+-- MySQL ENUM extension in mysql_mig_12.
+--
+-- SQLite cannot ALTER a CHECK constraint in place, so we follow the
+-- standard 12-step rebuild: create `sessions_new` with the corrected
+-- constraint, copy rows, drop the old table, rename, then recreate every
+-- index that lived on the original (indexes are auto-dropped with their
+-- table).
+
+CREATE TABLE `sessions_new` (
+    "id" INTEGER PRIMARY KEY AUTOINCREMENT,
+    "user_id" INTEGER NOT NULL,
+    "uuid" TEXT NOT NULL,
+    "meta" JSON DEFAULT NULL,
+    "created_at" INTEGER DEFAULT 0,
+    "last_activity" INTEGER DEFAULT 0,
+    "kind" TEXT NOT NULL DEFAULT 'web'
+        CHECK (`kind` IN ('web', 'app', 'access_token', 'asset', 'worker')),
+    "label" TEXT,
+    "parent_session_id" TEXT,
+    "last_ip" TEXT,
+    "last_user_agent" TEXT,
+    "revoked_at" INTEGER,
+    "expires_at" INTEGER,
+    "app_uid" TEXT,
+    "legacy_token_uid" TEXT,
+    "created_via" TEXT,
+    "auth_id" TEXT,
+    "access_token_uid" TEXT,
+    FOREIGN KEY("user_id") REFERENCES "user" ("id") ON DELETE CASCADE ON UPDATE CASCADE
+);
+
+INSERT INTO `sessions_new` (
+    `id`, `user_id`, `uuid`, `meta`, `created_at`, `last_activity`,
+    `kind`, `label`, `parent_session_id`, `last_ip`, `last_user_agent`,
+    `revoked_at`, `expires_at`, `app_uid`, `legacy_token_uid`,
+    `created_via`, `auth_id`, `access_token_uid`
+)
+SELECT
+    `id`, `user_id`, `uuid`, `meta`, `created_at`, `last_activity`,
+    `kind`, `label`, `parent_session_id`, `last_ip`, `last_user_agent`,
+    `revoked_at`, `expires_at`, `app_uid`, `legacy_token_uid`,
+    `created_via`, `auth_id`, `access_token_uid`
+FROM `sessions`;
+
+DROP TABLE `sessions`;
+
+ALTER TABLE `sessions_new` RENAME TO `sessions`;
+
+CREATE INDEX IF NOT EXISTS `idx_sessions_user_revoked`
+    ON `sessions` (`user_id`, `revoked_at`);
+
+CREATE INDEX IF NOT EXISTS `idx_sessions_parent`
+    ON `sessions` (`parent_session_id`);
+
+CREATE UNIQUE INDEX IF NOT EXISTS `idx_sessions_user_app_active`
+    ON `sessions` (`user_id`, `app_uid`)
+    WHERE `kind` = 'app' AND `revoked_at` IS NULL;
+
+CREATE UNIQUE INDEX IF NOT EXISTS `idx_sessions_legacy_token_active`
+    ON `sessions` (`legacy_token_uid`)
+    WHERE `legacy_token_uid` IS NOT NULL AND `revoked_at` IS NULL;
+
+CREATE INDEX IF NOT EXISTS `idx_sessions_kind_user`
+    ON `sessions` (`kind`, `user_id`);
+
+CREATE INDEX IF NOT EXISTS `idx_sessions_access_token_uid`
+    ON `sessions` (`access_token_uid`)
+    WHERE `access_token_uid` IS NOT NULL;
+
+CREATE UNIQUE INDEX IF NOT EXISTS `idx_sessions_user_worker_active`
+    ON `sessions` (`user_id`, IFNULL(`app_uid`, ''), json_extract(`meta`, '$.worker_name'))
+    WHERE `kind` = 'worker' AND `revoked_at` IS NULL;
@@ -48,6 +48,26 @@ const TOUCH_THROTTLE_MAX_ENTRIES = 10000;
 export const WEB_WINDOW_SECONDS = 365 * 24 * 60 * 60; // 1y
 export const APP_WINDOW_SECONDS = 365 * 24 * 60 * 60; // 1y
 export const WORKER_WINDOW_SECONDS = 99 * 365 * 24 * 60 * 60; // 99y (virtually infinite);
+
+// Duplicate-key error codes used by the `getOrCreate*` paths to detect
+// "another caller won the partial-unique-index race" — the only error
+// category they're prepared to silently no-op through. CHECK / NOT NULL /
+// FK / type violations must bubble up; otherwise the caller caches a
+// row that was never inserted.
+//   better-sqlite3 surfaces SqliteError.code; mysql2 surfaces .code and
+//   .errno (1062 = ER_DUP_ENTRY).
+function isUniqueViolation(err) {
+    if (!err) return false;
+    const code = err.code;
+    if (
+        code === 'SQLITE_CONSTRAINT_UNIQUE' ||
+        code === 'SQLITE_CONSTRAINT_PRIMARYKEY' ||
+        code === 'ER_DUP_ENTRY'
+    ) {
+        return true;
+    }
+    return err.errno === 1062;
+}
 export const ASSET_WINDOW_SECONDS = 7 * 24 * 60 * 60; // 7 days

 const sqlTimestamp = (ms) =>
@@ -140,10 +160,10 @@ export class SessionStore extends PuterStore {

    /**
     * Shared INSERT implementation for `create()` and the idempotent
-     * `getOrCreate*` paths. `ignoreConflict: true` switches to engine-
-     * specific INSERT-IGNORE so partial-unique-index collisions silently
-     * no-op rather than throw — the idempotent callers handle the "row
-     * already existed" path via a re-SELECT.
+     * `getOrCreate*` paths. With `ignoreConflict: true`, only a duplicate-
+     * key error from the partial unique indexes is swallowed (concurrent
+     * caller won the race); every other failure — CHECK, NOT NULL, FK,
+     * type — throws. The caller then re-SELECTs to find the winning row.
     */
    async #insertSession(
        userId,
@@ -169,34 +189,45 @@ export class SessionStore extends PuterStore {
        meta.created = new Date().toISOString();
        meta.created_unix = now;

-        const insertVerb = ignoreConflict
-            ? this.clients.db.case({
-                  sqlite: 'INSERT OR IGNORE INTO',
-                  otherwise: 'INSERT IGNORE INTO',
-              })
-            : 'INSERT INTO';
-
-        await this.clients.db.write(
-            `${insertVerb} \`sessions\` (\`uuid\`, \`user_id\`, \`meta\`, \`last_activity\`, \`created_at\`, \`kind\`, \`label\`, \`parent_session_id\`, \`last_ip\`, \`last_user_agent\`, \`expires_at\`, \`app_uid\`, \`legacy_token_uid\`, \`access_token_uid\`, \`created_via\`, \`auth_id\`) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
-            [
-                uuid,
-                userId,
-                JSON.stringify(meta),
-                now,
-                now,
-                kind,
-                label,
-                parent_session_id,
-                last_ip,
-                last_user_agent,
-                expires_at,
-                app_uid,
-                legacy_token_uid,
-                access_token_uid,
-                created_via,
-                auth_id,
-            ],
-        );
+        // Always issue a plain INSERT. The `getOrCreate*` paths set
+        // ignoreConflict=true so the partial-unique-index race against a
+        // concurrent caller can no-op, but only that specific error class
+        // is swallowed — every other failure (CHECK, NOT NULL, FK, type)
+        // bubbles up. Engine-specific INSERT-IGNORE swallowed all
+        // constraint violations, which masked schema bugs as "row didn't
+        // appear in the DB but the caller cached a synthetic row anyway".
+        try {
+            await this.clients.db.write(
+                `INSERT INTO \`sessions\` (\`uuid\`, \`user_id\`, \`meta\`, \`last_activity\`, \`created_at\`, \`kind\`, \`label\`, \`parent_session_id\`, \`last_ip\`, \`last_user_agent\`, \`expires_at\`, \`app_uid\`, \`legacy_token_uid\`, \`access_token_uid\`, \`created_via\`, \`auth_id\`) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
+                [
+                    uuid,
+                    userId,
+                    JSON.stringify(meta),
+                    now,
+                    now,
+                    kind,
+                    label,
+                    parent_session_id,
+                    last_ip,
+                    last_user_agent,
+                    expires_at,
+                    app_uid,
+                    legacy_token_uid,
+                    access_token_uid,
+                    created_via,
+                    auth_id,
+                ],
+            );
+        } catch (err) {
+            if (!ignoreConflict || !isUniqueViolation(err)) {
+                throw err;
+            }
+            // Concurrent caller won the partial-unique-index race. The
+            // caller's re-SELECT will return that winning row; the local
+            // `row` object below is a placeholder for the caller's
+            // `winner ?? created` fallback shape and is never cached on
+            // the ignoreConflict path.
+        }

        const row = {
            uuid,