diff --git a/deploy/postgres/001-bootstrap-roles.sh b/deploy/postgres/001-bootstrap-roles.sh index 83d6c945..cee4a948 100755 --- a/deploy/postgres/001-bootstrap-roles.sh +++ b/deploy/postgres/001-bootstrap-roles.sh @@ -69,4 +69,7 @@ SELECT format( 'GRANT CONNECT ON DATABASE %I TO identity_runtime, app_runtime, admin_runtime, migration_runner, backup_reader', :'database_name' ) \gexec + +ALTER SCHEMA public OWNER TO schema_owner; +REVOKE ALL ON SCHEMA public FROM PUBLIC; SQL diff --git a/frontend/package.json b/frontend/package.json index 25266a37..0b2c162b 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -7,7 +7,7 @@ "dev": "next dev", "build": "next build", "start": "next start", - "test": "tsx --test --test-concurrency=1 tests/*.test.ts", + "test": "tsx --test tests/*.test.ts", "test:db": "tsx --test --test-concurrency=1 tests/database-*.test.ts", "db:migrate": "node scripts/db-migrate.mjs", "db:migrate:check": "node scripts/db-migrate.mjs --check", diff --git a/frontend/tests/database-foundation.test.ts b/frontend/tests/database-foundation.test.ts index 1191920d..bd32b0ad 100644 --- a/frontend/tests/database-foundation.test.ts +++ b/frontend/tests/database-foundation.test.ts @@ -1,11 +1,12 @@ import assert from "node:assert/strict"; -import { spawnSync, type SpawnSyncReturns } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import { appendFileSync, copyFileSync, mkdtempSync, mkdirSync, rmSync, + unlinkSync, writeFileSync, } from "node:fs"; import { tmpdir } from "node:os"; @@ -17,6 +18,8 @@ import { startPostgresFixture } from "./helpers/postgres-fixture"; const migrationFilename = "20260720000100_backend_foundation.sql"; const pendingFilename = "20260720000200_pending_check.sql"; +const concurrentFilename = "20260720000300_concurrent_lock.sql"; +const failingFilename = "20260720000400_atomic_rollback.sql"; const runnerPath = fileURLToPath( new URL("../scripts/db-migrate.mjs", import.meta.url), ); @@ -33,26 +36,72 @@ const fixturePasswords = [ "postgres-test-password", ]; +type MigrationResult = { + status: number | null; + stdout: string; + stderr: string; +}; + +function migrationInvocation( + connectionString: string, + options: { check?: boolean; migrationsDirectory?: string }, +) { + return { + arguments: [runnerPath, ...(options.check ? ["--check"] : [])], + environment: { + SCHEMA_DATABASE_URL: connectionString, + ...(options.migrationsDirectory + ? { MIGRATIONS_DIRECTORY: options.migrationsDirectory } + : {}), + } as NodeJS.ProcessEnv, + }; +} + function runMigration( connectionString: string, options: { check?: boolean; migrationsDirectory?: string } = {}, -): SpawnSyncReturns { - return spawnSync( +): MigrationResult { + const invocation = migrationInvocation(connectionString, options); + const result = spawnSync( process.execPath, - [runnerPath, ...(options.check ? ["--check"] : [])], + invocation.arguments, { encoding: "utf8", - env: { - SCHEMA_DATABASE_URL: connectionString, - ...(options.migrationsDirectory - ? { MIGRATIONS_DIRECTORY: options.migrationsDirectory } - : {}), - } as NodeJS.ProcessEnv, + env: invocation.environment, }, ); + return { status: result.status, stdout: result.stdout, stderr: result.stderr }; } -function assertSafeOutput(result: SpawnSyncReturns): void { +function runMigrationAsync( + connectionString: string, + migrationsDirectory: string, +): Promise { + const invocation = migrationInvocation(connectionString, { + migrationsDirectory, + }); + + return new Promise((resolve, reject) => { + const child = spawn(process.execPath, invocation.arguments, { + env: invocation.environment, + stdio: ["ignore", "pipe", "pipe"], + }); + let stdout = ""; + let stderr = ""; + child.stdout.setEncoding("utf8"); + child.stderr.setEncoding("utf8"); + child.stdout.on("data", (chunk) => { + stdout += chunk; + }); + child.stderr.on("data", (chunk) => { + stderr += chunk; + }); + child.once("error", reject); + child.once("close", (status) => resolve({ status, stdout, stderr })); + }); +} + +function assertSafeOutput(result: MigrationResult): void { const output = `${result.stdout}${result.stderr}`; for (const password of fixturePasswords) { assert.doesNotMatch(output, new RegExp(password)); @@ -66,7 +115,7 @@ test("readDatabaseUrl requires APP_DATABASE_URL", () => { ); }); -test("migration runner applies once, detects drift, and checks read-only", () => { +test("migration runner is serialized, atomic, drift-safe, and read-only in check mode", async () => { const fixture = startPostgresFixture(); const temporaryDirectory = mkdtempSync(join(tmpdir(), "jyotisha-migrations-")); const copiedMigration = join(temporaryDirectory, migrationFilename); @@ -74,7 +123,7 @@ test("migration runner applies once, detects drift, and checks read-only", () => "schema_owner", "schema-owner-test-password", ); - const results: SpawnSyncReturns[] = []; + const results: MigrationResult[] = []; try { copyFileSync(migrationPath, copiedMigration); @@ -93,7 +142,9 @@ test("migration runner applies once, detects drift, and checks read-only", () => "f", ); - const firstRun = runMigration(schemaUrl); + const firstRun = runMigration(schemaUrl, { + migrationsDirectory: temporaryDirectory, + }); results.push(firstRun); assert.equal(firstRun.status, 0, firstRun.stderr); assert.match(firstRun.stdout, new RegExp(`applied ${migrationFilename}`)); @@ -106,7 +157,9 @@ test("migration runner applies once, detects drift, and checks read-only", () => assert.equal(filename, migrationFilename); assert.equal(checksum.length, 64); - const secondRun = runMigration(schemaUrl); + const secondRun = runMigration(schemaUrl, { + migrationsDirectory: temporaryDirectory, + }); results.push(secondRun); assert.equal(secondRun.status, 0, secondRun.stderr); assert.match( @@ -121,7 +174,10 @@ test("migration runner applies once, detects drift, and checks read-only", () => ledgerRow, ); - const currentCheck = runMigration(schemaUrl, { check: true }); + const currentCheck = runMigration(schemaUrl, { + check: true, + migrationsDirectory: temporaryDirectory, + }); results.push(currentCheck); assert.equal(currentCheck.status, 0, currentCheck.stderr); @@ -161,8 +217,19 @@ test("migration runner applies once, detects drift, and checks read-only", () => ), "f", ); + unlinkSync(pendingPath); appendFileSync(copiedMigration, " "); + const driftRun = runMigration(schemaUrl, { + migrationsDirectory: temporaryDirectory, + }); + results.push(driftRun); + assert.equal(driftRun.status, 1); + assert.match( + driftRun.stderr, + new RegExp(`migration checksum mismatch: ${migrationFilename}`), + ); + const driftCheck = runMigration(schemaUrl, { check: true, migrationsDirectory: temporaryDirectory, @@ -173,15 +240,75 @@ test("migration runner applies once, detects drift, and checks read-only", () => driftCheck.stderr, new RegExp(`migration checksum mismatch: ${migrationFilename}`), ); + copyFileSync(migrationPath, copiedMigration); - const driftRun = runMigration(schemaUrl, { + writeFileSync( + join(temporaryDirectory, concurrentFilename), + "select pg_sleep(1);\ncreate schema concurrent_lock_probe;\n", + ); + const concurrentResults = await Promise.all([ + runMigrationAsync(schemaUrl, temporaryDirectory), + runMigrationAsync(schemaUrl, temporaryDirectory), + ]); + results.push(...concurrentResults); + assert.deepEqual( + concurrentResults.map((result) => result.status), + [0, 0], + ); + assert.deepEqual( + concurrentResults + .flatMap((result) => result.stdout.trim().split("\n")) + .filter((line) => line.endsWith(concurrentFilename)) + .sort(), + [ + `already applied ${concurrentFilename}`, + `applied ${concurrentFilename}`, + ].sort(), + ); + assert.equal( + fixture.psql(` + select count(*) + from migration.schema_migrations + where filename = '${concurrentFilename}' + `), + "1", + ); + + writeFileSync( + join(temporaryDirectory, failingFilename), + `create schema atomic_rollback_probe authorization schema_owner; +create table atomic_rollback_probe.parent (id integer primary key); +create table atomic_rollback_probe.child ( + parent_id integer references atomic_rollback_probe.parent(id) + deferrable initially deferred +); +insert into atomic_rollback_probe.child (parent_id) values (1); +`, + ); + const failingRun = runMigration(schemaUrl, { migrationsDirectory: temporaryDirectory, }); - results.push(driftRun); - assert.equal(driftRun.status, 1); + results.push(failingRun); + assert.equal(failingRun.status, 1); assert.match( - driftRun.stderr, - new RegExp(`migration checksum mismatch: ${migrationFilename}`), + failingRun.stderr, + new RegExp(`migration failed: ${failingFilename}`), + ); + assert.equal( + fixture.psql(` + select exists ( + select from pg_namespace where nspname = 'atomic_rollback_probe' + ) + `), + "f", + ); + assert.equal( + fixture.psql(` + select count(*) + from migration.schema_migrations + where filename = '${failingFilename}' + `), + "0", ); assert.equal( diff --git a/frontend/tests/database-topology.test.ts b/frontend/tests/database-topology.test.ts index 5af2b456..07f31500 100644 --- a/frontend/tests/database-topology.test.ts +++ b/frontend/tests/database-topology.test.ts @@ -33,6 +33,38 @@ test("database roles have no cluster privileges", () => { "schema_owner:f:f:f:f:f:f", ].join("\n"), ); + assert.equal( + fixture.psql( + "select nspowner::regrole::text from pg_namespace where nspname = 'public'", + ), + "schema_owner", + ); + assert.equal( + fixture.psql(` + select has_schema_privilege('schema_owner', 'public', 'create') + and has_schema_privilege('schema_owner', 'public', 'usage') + `), + "t", + ); + assert.equal( + fixture.psql(` + select coalesce(string_agg(privilege_type, ',' order by privilege_type), '') + from pg_namespace, + aclexplode(coalesce(nspacl, acldefault('n', nspowner))) + where nspname = 'public' + and grantee = 0 + and privilege_type in ('CREATE', 'USAGE') + `), + "", + ); + assert.equal( + fixture.psql(` + select pg_get_userbyid(datdba) + from pg_database + where datname = current_database() + `), + "postgres", + ); } finally { fixture.stop(); } diff --git a/frontend/tests/helpers/postgres-fixture.ts b/frontend/tests/helpers/postgres-fixture.ts index 6e5cee46..5fb9aa0a 100644 --- a/frontend/tests/helpers/postgres-fixture.ts +++ b/frontend/tests/helpers/postgres-fixture.ts @@ -1,5 +1,11 @@ import { execFileSync, spawnSync } from "node:child_process"; -import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { + chmodSync, + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -43,21 +49,43 @@ server.listen({ host: "127.0.0.1", port: Number(process.argv[1]) }, () => ); } -function findAvailablePort(): number { +type PortReservation = { port: number; directory: string }; + +function reserveAvailablePort(): PortReservation { for (let port = 55432; port <= 55531; port += 1) { - if (isPortAvailable(port)) { - return port; + const directory = join(tmpdir(), `jyotisha-postgres-port-${port}.lock`); + try { + mkdirSync(directory); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "EEXIST") continue; + throw error; } + + if (isPortAvailable(port)) { + return { port, directory }; + } + rmSync(directory, { force: true, recursive: true }); } throw new Error("no available PostgreSQL test port in 55432..55531"); } +function releasePort(reservation: PortReservation): void { + rmSync(reservation.directory, { force: true, recursive: true }); +} + export function startPostgresFixture(): PostgresFixture { const projectName = `jyotisha-postgres-${process.pid}-${Date.now()}`; const temporaryDirectory = mkdtempSync(join(tmpdir(), "jyotisha-postgres-")); const databaseEnvFile = join(temporaryDirectory, "database.env"); - const hostPort = findAvailablePort(); + let portReservation: PortReservation; + try { + portReservation = reserveAvailablePort(); + } catch (error) { + rmSync(temporaryDirectory, { force: true, recursive: true }); + throw error; + } + const hostPort = portReservation.port; const composeArguments = [ "compose", "--project-name", @@ -75,10 +103,9 @@ export function startPostgresFixture(): PostgresFixture { POSTGRES_HOST_PORT: String(hostPort), }; - writeFileSync(databaseEnvFile, databaseEnvironment, { mode: 0o600 }); - chmodSync(databaseEnvFile, 0o600); - try { + writeFileSync(databaseEnvFile, databaseEnvironment, { mode: 0o600 }); + chmodSync(databaseEnvFile, 0o600); execFileSync( "docker", [...composeArguments, "up", "-d", "--wait", "postgres"], @@ -86,12 +113,17 @@ export function startPostgresFixture(): PostgresFixture { ); } catch (error) { try { - execFileSync( - "docker", - [...composeArguments, "down", "-v", "--remove-orphans"], - { env: environment, stdio: "inherit" }, - ); + try { + execFileSync( + "docker", + [...composeArguments, "down", "-v", "--remove-orphans"], + { env: environment, stdio: "inherit" }, + ); + } catch { + // Preserve the original startup failure. + } } finally { + releasePort(portReservation); rmSync(temporaryDirectory, { force: true, recursive: true }); } throw error; @@ -121,6 +153,7 @@ export function startPostgresFixture(): PostgresFixture { { env: environment, stdio: "inherit" }, ); } finally { + releasePort(portReservation); rmSync(temporaryDirectory, { force: true, recursive: true }); } },