diff --git a/apps/gateway/src/admin/bootstrap.controller.ts b/apps/gateway/src/admin/bootstrap.controller.ts index a5bf36d5..68c70739 100644 --- a/apps/gateway/src/admin/bootstrap.controller.ts +++ b/apps/gateway/src/admin/bootstrap.controller.ts @@ -8,7 +8,7 @@ import { Post, } from '@nestjs/common'; import { randomBytes, createHash } from 'node:crypto'; -import { count, eq, type Db, users as usersTable, adminTokens } from '@mosaicstack/db'; +import { count, eq, sql, type Db, users as usersTable, adminTokens } from '@mosaicstack/db'; import type { Auth } from '@mosaicstack/auth'; import { v4 as uuid } from 'uuid'; import { AUTH } from '../auth/auth.tokens.js'; @@ -16,6 +16,12 @@ import { DB } from '../database/database.module.js'; import { BootstrapSetupDto } from './bootstrap.dto.js'; import type { BootstrapStatusDto, BootstrapResultDto } from './bootstrap.dto.js'; +/** + * Advisory lock key serializing bootstrap setup. Arbitrary constant; must only + * be unique among advisory lock keys used against this database. + */ +export const BOOTSTRAP_SETUP_LOCK_KEY = 0x626f6f74; // 'boot' + @Controller('api/bootstrap') export class BootstrapController { constructor( @@ -31,72 +37,83 @@ export class BootstrapController { @Post('setup') async setup(@Body() dto: BootstrapSetupDto): Promise { - // Only allow setup when zero users exist - const [result] = await this.db.select({ total: count() }).from(usersTable); - if ((result?.total ?? 0) > 0) { - throw new ForbiddenException('Setup already completed — users exist'); - } + // #1430: the zero-user check and the admin creation must be one critical + // section, or two concurrent setup calls can each create an admin. The + // transaction-scoped advisory lock serializes setup across all gateway + // instances sharing this database; the second caller blocks on the lock, + // then re-reads the count and gets 403. + return this.db.transaction(async (tx) => { + await tx.execute(sql`select pg_advisory_xact_lock(${BOOTSTRAP_SETUP_LOCK_KEY})`); - // Create admin user via BetterAuth API - const authApi = this.auth.api as unknown as { - createUser: (opts: { - body: { name: string; email: string; password: string; role?: string }; - }) => Promise<{ - user: { id: string; name: string; email: string }; - }>; - }; + // Only allow setup when zero users exist + const [result] = await tx.select({ total: count() }).from(usersTable); + if ((result?.total ?? 0) > 0) { + throw new ForbiddenException('Setup already completed — users exist'); + } - const created = await authApi.createUser({ - body: { - name: dto.name, - email: dto.email, - password: dto.password, - role: 'admin', - }, + // Create admin user via BetterAuth API. BetterAuth writes on its own + // connection and commits independently of this transaction; the reads + // below still see the committed row (READ COMMITTED statement snapshot). + const authApi = this.auth.api as unknown as { + createUser: (opts: { + body: { name: string; email: string; password: string; role?: string }; + }) => Promise<{ + user: { id: string; name: string; email: string }; + }>; + }; + + const created = await authApi.createUser({ + body: { + name: dto.name, + email: dto.email, + password: dto.password, + role: 'admin', + }, + }); + + // Verify user was created + const [user] = await tx + .select() + .from(usersTable) + .where(eq(usersTable.id, created.user.id)) + .limit(1); + + if (!user) throw new InternalServerErrorException('User created but not found'); + + // Ensure role is admin (createUser may not set it via BetterAuth) + if (user.role !== 'admin') { + await tx.update(usersTable).set({ role: 'admin' }).where(eq(usersTable.id, user.id)); + } + + // Generate admin API token + const plaintext = randomBytes(32).toString('hex'); + const tokenHash = createHash('sha256').update(plaintext).digest('hex'); + const tokenId = uuid(); + + const [token] = await tx + .insert(adminTokens) + .values({ + id: tokenId, + userId: user.id, + tokenHash, + label: 'Initial setup token', + scope: 'admin', + }) + .returning(); + + return { + user: { + id: user.id, + name: user.name, + email: user.email, + role: 'admin', + }, + token: { + id: token!.id, + plaintext, + label: token!.label, + }, + }; }); - - // Verify user was created - const [user] = await this.db - .select() - .from(usersTable) - .where(eq(usersTable.id, created.user.id)) - .limit(1); - - if (!user) throw new InternalServerErrorException('User created but not found'); - - // Ensure role is admin (createUser may not set it via BetterAuth) - if (user.role !== 'admin') { - await this.db.update(usersTable).set({ role: 'admin' }).where(eq(usersTable.id, user.id)); - } - - // Generate admin API token - const plaintext = randomBytes(32).toString('hex'); - const tokenHash = createHash('sha256').update(plaintext).digest('hex'); - const tokenId = uuid(); - - const [token] = await this.db - .insert(adminTokens) - .values({ - id: tokenId, - userId: user.id, - tokenHash, - label: 'Initial setup token', - scope: 'admin', - }) - .returning(); - - return { - user: { - id: user.id, - name: user.name, - email: user.email, - role: 'admin', - }, - token: { - id: token!.id, - plaintext, - label: token!.label, - }, - }; } } diff --git a/apps/gateway/src/admin/bootstrap.e2e.spec.ts b/apps/gateway/src/admin/bootstrap.e2e.spec.ts index cbfb14e7..8b81f9b5 100644 --- a/apps/gateway/src/admin/bootstrap.e2e.spec.ts +++ b/apps/gateway/src/admin/bootstrap.e2e.spec.ts @@ -20,7 +20,7 @@ */ import 'reflect-metadata'; -import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import { describe, it, expect, afterAll, beforeAll, vi } from 'vitest'; import { Test } from '@nestjs/testing'; import { ValidationPipe, type INestApplication } from '@nestjs/common'; import { FastifyAdapter, type NestFastifyApplication } from '@nestjs/platform-fastify'; @@ -52,13 +52,22 @@ const mockAuth = { }, }; -// Override db.select() so the second query (verify user exists) returns a user. -// The bootstrap controller calls select().from() twice: +// The controller runs setup inside db.transaction(tx) and first takes the +// #1430 advisory lock via tx.execute(). Inside the transaction it calls +// select().from() twice: // 1. count() to check zero users → returns [{total: 0}] // 2. select().where().limit() → returns [the created user] +// callLog records the tx call order so the lock-before-check invariant is +// testable. let selectCallCount = 0; -const mockDbWithUser = { +const callLog: string[] = []; +const mockTx = { + execute: () => { + callLog.push('execute'); + return Promise.resolve([]); + }, select: () => { + callLog.push('select'); selectCallCount++; return { from: () => { @@ -100,6 +109,9 @@ const mockDbWithUser = { }), }), }; +const mockDbWithUser = { + transaction: (cb: (tx: typeof mockTx) => Promise) => cb(mockTx), +}; // ─── Test suite ─────────────────────────────────────────────────────────────── @@ -153,6 +165,11 @@ describe('POST /api/bootstrap/setup — ValidationPipe DTO binding', () => { expect(body.user.email).toBe('admin@example.com'); expect(body.token).toBeDefined(); expect(body.token.plaintext).toBeDefined(); + + // #1430: the advisory lock must be taken before the zero-user count, or + // two concurrent setups can both pass the check. + expect(callLog[0]).toBe('execute'); + expect(callLog[1]).toBe('select'); }); it('returns 400 when extra forbidden properties are sent', async () => { @@ -188,3 +205,55 @@ describe('POST /api/bootstrap/setup — ValidationPipe DTO binding', () => { expect(res.status).toBe(400); }); }); + +// ─── #1430 regression: users-exist check runs inside the locked transaction ── + +describe('POST /api/bootstrap/setup — setup already completed', () => { + let app: INestApplication; + const createUserSpy = vi.fn(); + const lockedTx = { + execute: () => Promise.resolve([]), + select: () => ({ + // count() sees an existing user — the locked re-check must reject. + from: () => Promise.resolve([{ total: 1 }]), + }), + }; + const mockDbUsersExist = { + transaction: (cb: (tx: typeof lockedTx) => Promise) => cb(lockedTx), + }; + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ + controllers: [BootstrapController], + providers: [ + { provide: AUTH, useValue: { api: { createUser: createUserSpy } } }, + { provide: DB, useValue: mockDbUsersExist }, + ], + }).compile(); + + app = moduleRef.createNestApplication(new FastifyAdapter()); + app.useGlobalPipes( + new ValidationPipe({ + whitelist: true, + forbidNonWhitelisted: true, + transform: true, + }), + ); + await app.init(); + await app.getHttpAdapter().getInstance().ready(); + }); + + afterAll(async () => { + await app.close(); + }); + + it('returns 403 and never calls createUser when users already exist', async () => { + const res = await request(app.getHttpServer()) + .post('/api/bootstrap/setup') + .send({ name: 'Admin', email: 'admin@example.com', password: 'password123' }) + .set('Content-Type', 'application/json'); + + expect(res.status).toBe(403); + expect(createUserSpy).not.toHaveBeenCalled(); + }); +});