From 4ccdb8a1faa375223def63c796f81c3e9263ad6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 4 Aug 2026 14:48:04 +0000 Subject: [PATCH] db: the owner account cannot be demoted or deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two guards on user 1, the bootstrap account, so ownership survives whatever happens to the rows. ck_users_owner_is_super_admin — CHECK ((id <> 1) OR (role = 'Super Admin')). In the database rather than in application code because the point is that it holds against a stray UPDATE, a migration script or someone at a psql prompt, not just against the API. A row-level CHECK can say "if this row is user 1 then its role is Super Admin"; it cannot say "some row must be Super Admin", which would need to see other rows. So it pins the bootstrap account and nothing else — promoting and demoting everyone else stays free. deleteUser() refuses id 1, because a CHECK cannot stop a DELETE and removing the owner reaches the same end by another route: nobody who can open the vault, no identity for the agent sidecar to run as, a web origin restricted to a Super Admin that no longer exists, and the passkeys cascaded away so there is no signing back in. It throws rather than returning false — its eventual caller is a manage-users flow, where a silent false reads as "already gone". OWNER_USER_ID is exported from the schema and used by both, so the number appears once. Tested on a scratch database, and the first harness was wrong — a shell variable holding a command did not expand, every statement failed with "command not found", and the check reported them all as allowed. Re-run directly: insert user 1 as Super Admin allowed demote user 1 -> Member REJECTED by CHECK demote user 1 -> Admin REJECTED by CHECK insert user 1 as Member REJECTED by CHECK demote/promote users 2 and 3 allowed deleteUser(1) refused with the message above deleteUser(3) deleted Pushing twice showed the CHECK adds no diff churn — still only the two known pk_music_now_playing statements. Every row in officer_dev already satisfies it, so it will apply without touching data. NOT covered: nothing stops a second account also being Super Admin. The rule asked for was "user 1 is always Super Admin", not "only user 1 is", and the two are different constraints. Co-Authored-By: Claude Opus 5 (1M context) --- src/databases/officer_db/src/queries/auth.ts | 38 +++++++++-- src/databases/officer_db/src/schema/auth.ts | 67 ++++++++++++++------ 2 files changed, 79 insertions(+), 26 deletions(-) diff --git a/src/databases/officer_db/src/queries/auth.ts b/src/databases/officer_db/src/queries/auth.ts index 4385b632..79a5dea3 100644 --- a/src/databases/officer_db/src/queries/auth.ts +++ b/src/databases/officer_db/src/queries/auth.ts @@ -1,6 +1,6 @@ import { eq, and, lt, sql } from 'drizzle-orm'; import { db } from '../db'; -import { users, passkeys, passkeyChallenges, tokenBlacklist } from '../schema'; +import { users, passkeys, passkeyChallenges, tokenBlacklist, OWNER_USER_ID } from '../schema'; import type { UserSelect, UserInsert, PasskeySelect, PasskeyInsert } from '../types'; // ── Users ── @@ -43,11 +43,26 @@ export async function createUser(data: UserInsert): Promise { } export async function updateUser(id: number, data: Partial>): Promise { - const [user] = await db.update(users).set({ ...data, updatedAt: new Date() }).where(eq(users.id, id)).returning(); + const [user] = await db + .update(users) + .set({ ...data, updatedAt: new Date() }) + .where(eq(users.id, id)) + .returning(); return user; } +// The owner cannot be deleted. ck_users_owner_is_super_admin pins user 1's role, but a row-level CHECK +// cannot stop a DELETE, and deleting the owner is the same outcome by another route: a platform with +// nobody who can reach the vault, no identity for the agent sidecar to run as, and a web origin +// restricted to a Super Admin that no longer exists. Cascades would take the passkeys with it, so there +// is no signing back in either. +// +// Throws rather than returning false. This has no callers today; the one it will get is a +// manage-users flow, and a silent `false` there reads as "already gone". export async function deleteUser(id: number): Promise { + if (id === OWNER_USER_ID) { + throw new Error(`Refusing to delete user ${OWNER_USER_ID}: the platform owner cannot be removed.`); + } const result = await db.delete(users).where(eq(users.id, id)).returning({ id: users.id }); return result.length > 0; } @@ -59,10 +74,16 @@ export async function getPasskeysByUserId(userId: number): Promise { - return db.select().from(passkeys).where(and(eq(passkeys.userId, userId), eq(passkeys.origin, origin))); + return db + .select() + .from(passkeys) + .where(and(eq(passkeys.userId, userId), eq(passkeys.origin, origin))); } -export async function getPasskeyByCredentialId(userId: number, credentialId: string): Promise { +export async function getPasskeyByCredentialId( + userId: number, + credentialId: string, +): Promise { const [passkey] = await db .select() .from(passkeys) @@ -75,7 +96,10 @@ export async function createPasskey(data: PasskeyInsert): Promise return passkey!; } -export async function updatePasskey(id: number, data: Partial>): Promise { +export async function updatePasskey( + id: number, + data: Partial>, +): Promise { const [passkey] = await db.update(passkeys).set(data).where(eq(passkeys.id, id)).returning(); return passkey; } @@ -86,7 +110,9 @@ export async function storeChallenge(userId: number, origin: string, challenge: const expiresAt = new Date(Date.now() + ttlMs); // Upsert: delete existing challenge for this user+origin, then insert - await db.delete(passkeyChallenges).where(and(eq(passkeyChallenges.userId, userId), eq(passkeyChallenges.origin, origin))); + await db + .delete(passkeyChallenges) + .where(and(eq(passkeyChallenges.userId, userId), eq(passkeyChallenges.origin, origin))); await db.insert(passkeyChallenges).values({ userId, origin, challenge, expiresAt }); } diff --git a/src/databases/officer_db/src/schema/auth.ts b/src/databases/officer_db/src/schema/auth.ts index 85ca075f..866af299 100644 --- a/src/databases/officer_db/src/schema/auth.ts +++ b/src/databases/officer_db/src/schema/auth.ts @@ -1,4 +1,10 @@ -import { pgTable, serial, text, integer, timestamp, index } from 'drizzle-orm/pg-core'; +import { pgTable, serial, text, integer, timestamp, index, check } from 'drizzle-orm/pg-core'; +import { sql } from 'drizzle-orm'; + +// The bootstrap account. `id` is a serial starting at 1 and bootstrap is gated on an empty user table, +// so the first account created is always 1. Exported so the CHECK below and the delete guard in +// queries/auth.ts state the number once between them. +export const OWNER_USER_ID = 1; // Roles, in descending authority. Exported as a value so the API and the UI can enumerate them from // the same place the column is defined — a second hand-written list of role names is how they drift. @@ -10,28 +16,49 @@ import { pgTable, serial, text, integer, timestamp, index } from 'drizzle-orm/pg // 'Super Admin' is the server owner, and this column is the ONLY thing that says so. isSuperAdmin() // and getOwnerUser() both read it; there is no SUPER_ADMIN_EMAIL and no "first account wins" fallback // any more. Two consequences worth holding on to: bootstrap must set it explicitly on the first -// account, and demoting the last Super Admin leaves the platform with no owner — nothing in the schema -// prevents that, so whatever eventually edits roles has to. +// account, and demoting the last Super Admin would leave the platform with no owner. The CHECK on the +// table below stops that for the bootstrap account specifically: user 1 cannot be anything else. export const USER_ROLES = ['Super Admin', 'Admin', 'Member', 'Developer'] as const; export type UserRole = (typeof USER_ROLES)[number]; -export const users = pgTable('users', { - id: serial('id').primaryKey(), - email: text('email').notNull().unique(), - password: text('password'), - status: text('status', { enum: ['Unverified', 'Active', 'Prospect', 'Invited', 'Blocked', 'Banned', 'Deleted'] }) - .notNull() - .default('Unverified'), - // Defaults to the least privileged role so a row created by any path that does not think about - // authorisation cannot accidentally mint an admin. - role: text('role', { enum: USER_ROLES }).notNull().default('Member'), - name: text('name'), - username: text('username').unique(), - avatar: text('avatar'), - passwordChangedAt: timestamp('password_changed_at', { withTimezone: true }), - createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), - updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(), -}); +export const users = pgTable( + 'users', + { + id: serial('id').primaryKey(), + email: text('email').notNull().unique(), + password: text('password'), + status: text('status', { enum: ['Unverified', 'Active', 'Prospect', 'Invited', 'Blocked', 'Banned', 'Deleted'] }) + .notNull() + .default('Unverified'), + // Defaults to the least privileged role so a row created by any path that does not think about + // authorisation cannot accidentally mint an admin. + role: text('role', { enum: USER_ROLES }).notNull().default('Member'), + name: text('name'), + username: text('username').unique(), + avatar: text('avatar'), + passwordChangedAt: timestamp('password_changed_at', { withTimezone: true }), + createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), + updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(), + }, + (table) => [ + // The owner cannot be demoted. Enforced here rather than in application code because the whole + // point is that it holds "whatever happens" — a stray UPDATE, a migration script, someone at a psql + // prompt. Postgres rejects the write; there is no path around it. + // + // A row-level CHECK can say "if this row is user 1 then its role is Super Admin". It cannot say + // "some row must be Super Admin" — that would need to see other rows. So this pins the bootstrap + // account and nothing more; promoting or demoting anyone else is still free. + // + // It cannot stop a DELETE either. deleteUser() in queries/auth.ts refuses id 1 for that half. + // + // sql.raw for the literal — an interpolated JS string binds as `$1`, and Postgres will not accept a + // parameter inside a CHECK. Same trap as ck_server_integrations_provider. + check( + 'ck_users_owner_is_super_admin', + sql`${table.id} <> ${sql.raw(String(OWNER_USER_ID))} OR ${table.role} = ${sql.raw("'Super Admin'")}`, + ), + ], +); export const passkeys = pgTable('passkeys', { id: serial('id').primaryKey(),