From 8caae5d6695e94d363ccaa172cc0413f644bea5f Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 18 Dec 2025 20:54:38 +0000 Subject: [PATCH 1/4] refactor(core,pgsql-test): modular role management with SQL templates - Add new SQL template files for role operations: - ensure-base-roles.sql: Create anonymous/authenticated/administrator roles - ensure-login-role.sql: Create LOGIN role with optional advisory locks - ensure-membership.sql: Grant role membership with optional locks - grant-connect.sql: Grant CONNECT privilege on database - Add TypeScript types for role operation options: - RoleNameMapping, EnsureLoginRoleOptions, EnsureRoleMembershipsOptions - CreateDbUserOptions, BootstrapTestUsersOptions - OnMissingRoleAction for configurable error handling - Add shared role-utils.ts with modular functions: - ensureBaseRoles, ensureLoginRole, ensureRoleMembership - ensureRoleMemberships, grantConnect, createDbUser, createTestUsers - Refactor PgpmInit to use new modular functions: - bootstrapRoles now uses ensureBaseRoles - bootstrapTestRoles now uses createTestUsers - bootstrapDbRoles now uses createDbUser with configurable options - Refactor DbAdmin in pgsql-test: - grantRole now supports useLocks, lockNamespace, onMissingRole options - createUserRole now supports useLocks, grantAdmin options - Add streamSqlWithParams for parameterized queries - Export new types and functions from @pgpmjs/core This refactor consolidates duplicated role creation logic, adds optional advisory locks for concurrent CI/CD safety, and makes admin role granting explicit and configurable. Note: Test credentials (app_user/app_password, app_admin/admin_password) are intentional test fixtures matching existing bootstrap-test-roles.sql Co-Authored-By: Dan Lynch --- packages/core/src/index.ts | 23 +- packages/core/src/init/client.ts | 148 ++++++------ packages/core/src/init/role-utils.ts | 225 +++++++++++++++++ .../core/src/init/sql/ensure-base-roles.sql | 54 +++++ .../core/src/init/sql/ensure-login-role.sql | 30 +++ .../core/src/init/sql/ensure-membership.sql | 46 ++++ packages/core/src/init/sql/grant-connect.sql | 17 ++ packages/core/src/init/types.ts | 80 ++++++ packages/pgsql-test/src/admin.ts | 227 ++++++++++++------ 9 files changed, 695 insertions(+), 155 deletions(-) create mode 100644 packages/core/src/init/role-utils.ts create mode 100644 packages/core/src/init/sql/ensure-base-roles.sql create mode 100644 packages/core/src/init/sql/ensure-login-role.sql create mode 100644 packages/core/src/init/sql/ensure-membership.sql create mode 100644 packages/core/src/init/sql/grant-connect.sql create mode 100644 packages/core/src/init/types.ts diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 36264515b..00dd0196b 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -18,8 +18,29 @@ export * from './files'; export { cleanSql } from './migrate/clean'; export { PgpmMigrate } from './migrate/client'; export { PgpmInit } from './init/client'; +export { + ensureBaseRoles, + ensureLoginRole, + ensureRoleMembership, + ensureRoleMemberships, + grantConnect, + createDbUser, + createTestUsers, + getRoleNames +} from './init/role-utils'; +export { + RoleNameMapping, + DEFAULT_ROLE_NAMES, + OnMissingRoleAction, + EnsureLoginRoleOptions, + EnsureRoleMembershipsOptions, + EnsureBaseRolesOptions, + GrantConnectOptions, + CreateDbUserOptions, + BootstrapTestUsersOptions +} from './init/types'; export { - DeployOptions, + DeployOptions, DeployResult, MigrateChange, MigratePlanFile, diff --git a/packages/core/src/init/client.ts b/packages/core/src/init/client.ts index 401b58eb7..56ed5d3ea 100644 --- a/packages/core/src/init/client.ts +++ b/packages/core/src/init/client.ts @@ -5,6 +5,20 @@ import { Pool } from 'pg'; import { getPgPool } from 'pg-cache'; import { PgConfig } from 'pg-env'; +import { + ensureBaseRoles, + ensureLoginRole, + ensureRoleMemberships, + createDbUser, + createTestUsers, + getRoleNames +} from './role-utils'; +import { + CreateDbUserOptions, + BootstrapTestUsersOptions, + RoleNameMapping +} from './types'; + const log = new Logger('init'); export class PgpmInit { @@ -18,15 +32,13 @@ export class PgpmInit { /** * Bootstrap standard roles (anonymous, authenticated, administrator) + * Uses the new modular ensureBaseRoles function */ - async bootstrapRoles(): Promise { + async bootstrapRoles(options?: { roleNames?: RoleNameMapping }): Promise { try { log.info('Bootstrapping PGPM roles...'); - const sqlPath = join(__dirname, 'sql', 'bootstrap-roles.sql'); - const sql = readFileSync(sqlPath, 'utf-8'); - - await this.pool.query(sql); + await ensureBaseRoles(this.pool, options); log.success('Successfully bootstrapped PGPM roles'); } catch (error) { @@ -36,81 +48,50 @@ export class PgpmInit { } /** - * Bootstrap test roles (roles only, no users) + * Bootstrap test users (app_user and app_admin) with appropriate role memberships + * WARNING: This should NEVER be run on a production database! */ - async bootstrapTestRoles(): Promise { + async bootstrapTestRoles(options?: BootstrapTestUsersOptions): Promise { try { - log.warn('WARNING: This command creates test roles and should NEVER be run on a production database!'); - log.info('Bootstrapping PGPM test roles...'); - - const sqlPath = join(__dirname, 'sql', 'bootstrap-test-roles.sql'); - const sql = readFileSync(sqlPath, 'utf-8'); + log.warn('WARNING: This command creates test users and should NEVER be run on a production database!'); + log.info('Bootstrapping PGPM test users...'); - await this.pool.query(sql); + await createTestUsers(this.pool, options); - log.success('Successfully bootstrapped PGPM test roles'); + log.success('Successfully bootstrapped PGPM test users'); } catch (error) { - log.error('Failed to bootstrap test roles:', error); + log.error('Failed to bootstrap test users:', error); throw error; } } /** * Bootstrap database roles with custom username and password + * Creates a login role and grants anonymous + authenticated memberships + * + * @param username - The username for the new role + * @param password - The password for the new role + * @param options - Optional configuration for locks and role names */ - async bootstrapDbRoles(username: string, password: string): Promise { + async bootstrapDbRoles( + username: string, + password: string, + options?: Omit + ): Promise { try { log.info(`Bootstrapping PGPM database roles for user: ${username}...`); - const sql = ` -BEGIN; -DO $do$ -DECLARE - v_username TEXT := '${username.replace(/'/g, "''")}'; - v_password TEXT := '${password.replace(/'/g, "''")}'; -BEGIN - BEGIN - EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', v_username, v_password); - EXCEPTION - WHEN duplicate_object THEN - -- Role already exists; optionally sync attributes here with ALTER ROLE - NULL; - END; -END -$do$; - --- Robust GRANTs under concurrency: GRANT can race on pg_auth_members unique index. --- Catch unique_violation (23505) and continue so CI/CD concurrent jobs don't fail. -DO $do$ -DECLARE - v_username TEXT := '${username.replace(/'/g, "''")}'; -BEGIN - BEGIN - EXECUTE format('GRANT %I TO %I', 'anonymous', v_username); - EXCEPTION - WHEN unique_violation THEN - -- Membership was granted concurrently; ignore. - NULL; - WHEN undefined_object THEN - -- One of the roles doesn't exist yet; order operations as needed. - RAISE NOTICE 'Missing role when granting % to %', 'anonymous', v_username; - END; - - BEGIN - EXECUTE format('GRANT %I TO %I', 'authenticated', v_username); - EXCEPTION - WHEN unique_violation THEN - -- Membership was granted concurrently; ignore. - NULL; - WHEN undefined_object THEN - RAISE NOTICE 'Missing role when granting % to %', 'authenticated', v_username; - END; -END -$do$; -COMMIT; - `; + const names = getRoleNames(options?.roleNames); - await this.pool.query(sql); + await createDbUser(this.pool, { + username, + password, + rolesToGrant: options?.rolesToGrant || [names.anonymous, names.authenticated], + useLocks: options?.useLocks, + lockNamespace: options?.lockNamespace, + onMissingRole: options?.onMissingRole, + roleNames: options?.roleNames + }); log.success(`Successfully bootstrapped PGPM database roles for user: ${username}`); } catch (error) { @@ -122,30 +103,45 @@ COMMIT; /** * Remove database roles and revoke grants */ - async removeDbRoles(username: string): Promise { + async removeDbRoles(username: string, options?: { roleNames?: RoleNameMapping }): Promise { try { log.info(`Removing PGPM database roles for user: ${username}...`); + const names = getRoleNames(options?.roleNames); + const sql = ` BEGIN; DO $do$ +DECLARE + v_username TEXT := $1; + v_anonymous TEXT := $2; + v_authenticated TEXT := $3; BEGIN - IF EXISTS ( - SELECT 1 - FROM - pg_catalog.pg_roles - WHERE - rolname = '${username}') THEN - REVOKE anonymous FROM ${username}; - REVOKE authenticated FROM ${username}; - DROP ROLE ${username}; -END IF; + IF EXISTS ( + SELECT 1 + FROM pg_catalog.pg_roles + WHERE rolname = v_username + ) THEN + BEGIN + EXECUTE format('REVOKE %I FROM %I', v_anonymous, v_username); + EXCEPTION + WHEN undefined_object THEN + NULL; + END; + BEGIN + EXECUTE format('REVOKE %I FROM %I', v_authenticated, v_username); + EXCEPTION + WHEN undefined_object THEN + NULL; + END; + EXECUTE format('DROP ROLE %I', v_username); + END IF; END $do$; COMMIT; `; - await this.pool.query(sql); + await this.pool.query(sql, [username, names.anonymous, names.authenticated]); log.success(`Successfully removed PGPM database roles for user: ${username}`); } catch (error) { diff --git a/packages/core/src/init/role-utils.ts b/packages/core/src/init/role-utils.ts new file mode 100644 index 000000000..be3c40df4 --- /dev/null +++ b/packages/core/src/init/role-utils.ts @@ -0,0 +1,225 @@ +import { Logger } from '@pgpmjs/logger'; +import { readFileSync } from 'fs'; +import { join } from 'path'; +import { Pool } from 'pg'; + +import { + DEFAULT_ROLE_NAMES, + EnsureBaseRolesOptions, + EnsureLoginRoleOptions, + EnsureRoleMembershipsOptions, + GrantConnectOptions, + CreateDbUserOptions, + RoleNameMapping +} from './types'; + +const log = new Logger('role-utils'); + +/** + * Load a SQL template file from the sql directory + */ +function loadSqlTemplate(templateName: string): string { + const sqlPath = join(__dirname, 'sql', `${templateName}.sql`); + return readFileSync(sqlPath, 'utf-8'); +} + +/** + * Get resolved role names with defaults + */ +export function getRoleNames(roleNames?: RoleNameMapping): Required { + return { + ...DEFAULT_ROLE_NAMES, + ...(roleNames || {}) + }; +} + +/** + * Ensure base roles exist (anonymous, authenticated, administrator) + * Creates the three standard NOLOGIN group roles with appropriate attributes + */ +export async function ensureBaseRoles( + pool: Pool, + options: EnsureBaseRolesOptions = {} +): Promise { + const names = getRoleNames(options.roleNames); + + log.info('Ensuring base roles exist...'); + + const sql = loadSqlTemplate('ensure-base-roles'); + + await pool.query(sql, [names.anonymous, names.authenticated, names.administrator]); + + log.success('Base roles ensured successfully'); +} + +/** + * Ensure a login role exists with the given username and password + * Optionally uses advisory locks for concurrent CI/CD safety + */ +export async function ensureLoginRole( + pool: Pool, + options: EnsureLoginRoleOptions +): Promise { + const { username, password, useLocks = false, lockNamespace = 42 } = options; + + log.info(`Ensuring login role exists: ${username}...`); + + const sql = loadSqlTemplate('ensure-login-role'); + + await pool.query(sql, [username, password, useLocks, lockNamespace]); + + log.success(`Login role ensured: ${username}`); +} + +/** + * Ensure role membership (grant a role to a user) + * Optionally uses advisory locks for concurrent CI/CD safety + */ +export async function ensureRoleMembership( + pool: Pool, + roleToGrant: string, + username: string, + options: { + useLocks?: boolean; + lockNamespace?: number; + onMissingRole?: 'error' | 'notice' | 'ignore'; + } = {} +): Promise { + const { useLocks = false, lockNamespace = 43, onMissingRole = 'notice' } = options; + + const sql = loadSqlTemplate('ensure-membership'); + + await pool.query(sql, [roleToGrant, username, useLocks, lockNamespace, onMissingRole]); +} + +/** + * Ensure multiple role memberships for a user + * Grants all specified roles to the user + */ +export async function ensureRoleMemberships( + pool: Pool, + options: EnsureRoleMembershipsOptions +): Promise { + const { + username, + rolesToGrant, + useLocks = false, + lockNamespace = 43, + onMissingRole = 'notice' + } = options; + + log.info(`Ensuring role memberships for ${username}: ${rolesToGrant.join(', ')}...`); + + for (const role of rolesToGrant) { + await ensureRoleMembership(pool, role, username, { + useLocks, + lockNamespace, + onMissingRole + }); + } + + log.success(`Role memberships ensured for ${username}`); +} + +/** + * Grant CONNECT privilege on a database to a role + */ +export async function grantConnect( + pool: Pool, + options: GrantConnectOptions +): Promise { + const { roleName, dbName } = options; + + log.info(`Granting CONNECT on ${dbName} to ${roleName}...`); + + const sql = loadSqlTemplate('grant-connect'); + + await pool.query(sql, [roleName, dbName]); + + log.success(`CONNECT granted on ${dbName} to ${roleName}`); +} + +/** + * Create a database user with role memberships + * This is a convenience function that combines ensureLoginRole and ensureRoleMemberships + */ +export async function createDbUser( + pool: Pool, + options: CreateDbUserOptions +): Promise { + const { + username, + password, + rolesToGrant, + useLocks = false, + lockNamespace = 42, + onMissingRole = 'notice', + roleNames + } = options; + + const names = getRoleNames(roleNames); + + // Default roles to grant if not specified + const roles = rolesToGrant || [names.anonymous, names.authenticated]; + + log.info(`Creating database user: ${username}...`); + + // Ensure login role exists + await ensureLoginRole(pool, { + username, + password, + useLocks, + lockNamespace + }); + + // Grant role memberships + await ensureRoleMemberships(pool, { + username, + rolesToGrant: roles, + useLocks, + lockNamespace: lockNamespace + 1, // Use different namespace for memberships + onMissingRole + }); + + log.success(`Database user created: ${username}`); +} + +/** + * Create test users (app_user and app_admin) with appropriate role memberships + * WARNING: This should NEVER be run on a production database! + */ +export async function createTestUsers( + pool: Pool, + options: { + useLocks?: boolean; + lockNamespace?: number; + roleNames?: RoleNameMapping; + } = {} +): Promise { + const { useLocks = false, lockNamespace = 42, roleNames } = options; + const names = getRoleNames(roleNames); + + log.warn('WARNING: Creating test users - should NEVER be run on production!'); + + // Create app_user with anonymous + authenticated + await createDbUser(pool, { + username: 'app_user', + password: 'app_password', + rolesToGrant: [names.anonymous, names.authenticated], + useLocks, + lockNamespace, + roleNames + }); + + // Create app_admin with anonymous + authenticated + administrator + await createDbUser(pool, { + username: 'app_admin', + password: 'admin_password', + rolesToGrant: [names.anonymous, names.authenticated, names.administrator], + useLocks, + lockNamespace, + roleNames + }); + + log.success('Test users created successfully'); +} diff --git a/packages/core/src/init/sql/ensure-base-roles.sql b/packages/core/src/init/sql/ensure-base-roles.sql new file mode 100644 index 000000000..aba02a1a0 --- /dev/null +++ b/packages/core/src/init/sql/ensure-base-roles.sql @@ -0,0 +1,54 @@ +-- Ensure base roles exist (anonymous, authenticated, administrator) +-- Parameters: $1 = anonymous role name, $2 = authenticated role name, $3 = administrator role name +BEGIN; +DO $do$ +DECLARE + v_anonymous TEXT := COALESCE($1, 'anonymous'); + v_authenticated TEXT := COALESCE($2, 'authenticated'); + v_administrator TEXT := COALESCE($3, 'administrator'); +BEGIN + -- Create anonymous role + BEGIN + EXECUTE format('CREATE ROLE %I', v_anonymous); + EXCEPTION + WHEN duplicate_object THEN + NULL; + END; + + -- Create authenticated role + BEGIN + EXECUTE format('CREATE ROLE %I', v_authenticated); + EXCEPTION + WHEN duplicate_object THEN + NULL; + END; + + -- Create administrator role + BEGIN + EXECUTE format('CREATE ROLE %I', v_administrator); + EXCEPTION + WHEN duplicate_object THEN + NULL; + END; +END +$do$; + +-- Set role attributes (safe to run even if role already exists) +-- These use dynamic SQL to support custom role names +DO $do$ +DECLARE + v_anonymous TEXT := COALESCE($1, 'anonymous'); + v_authenticated TEXT := COALESCE($2, 'authenticated'); + v_administrator TEXT := COALESCE($3, 'administrator'); +BEGIN + -- Anonymous role attributes + EXECUTE format('ALTER ROLE %I WITH NOCREATEDB NOSUPERUSER NOCREATEROLE NOLOGIN NOREPLICATION NOBYPASSRLS', v_anonymous); + + -- Authenticated role attributes + EXECUTE format('ALTER ROLE %I WITH NOCREATEDB NOSUPERUSER NOCREATEROLE NOLOGIN NOREPLICATION NOBYPASSRLS', v_authenticated); + + -- Administrator role attributes (CAN bypass RLS) + EXECUTE format('ALTER ROLE %I WITH NOCREATEDB NOSUPERUSER NOCREATEROLE NOLOGIN NOREPLICATION BYPASSRLS', v_administrator); +END +$do$; +COMMIT; diff --git a/packages/core/src/init/sql/ensure-login-role.sql b/packages/core/src/init/sql/ensure-login-role.sql new file mode 100644 index 000000000..17d796937 --- /dev/null +++ b/packages/core/src/init/sql/ensure-login-role.sql @@ -0,0 +1,30 @@ +-- Ensure a login role exists with the given username and password +-- Parameters: $1 = username, $2 = password, $3 = use_locks (boolean), $4 = lock_namespace (int) +DO $do$ +DECLARE + v_username TEXT := $1; + v_password TEXT := $2; + v_use_locks BOOLEAN := COALESCE($3::boolean, false); + v_lock_namespace INT := COALESCE($4::int, 42); +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = v_username) THEN + BEGIN + -- Acquire advisory lock if requested (prevents race conditions in concurrent CI/CD) + IF v_use_locks THEN + PERFORM pg_advisory_xact_lock(v_lock_namespace, hashtext(v_username)); + END IF; + + EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', v_username, v_password); + EXCEPTION + WHEN duplicate_object THEN + -- Role was created concurrently, safe to ignore + NULL; + WHEN unique_violation THEN + -- Concurrent insert, safe to ignore + NULL; + WHEN insufficient_privilege THEN + RAISE EXCEPTION 'Insufficient privileges to create role %: ensure the connecting user has CREATEROLE', v_username; + END; + END IF; +END +$do$; diff --git a/packages/core/src/init/sql/ensure-membership.sql b/packages/core/src/init/sql/ensure-membership.sql new file mode 100644 index 000000000..423fa2661 --- /dev/null +++ b/packages/core/src/init/sql/ensure-membership.sql @@ -0,0 +1,46 @@ +-- Ensure role membership (grant a role to a user) +-- Parameters: $1 = role_to_grant, $2 = username, $3 = use_locks (boolean), $4 = lock_namespace (int), $5 = on_missing_role ('error', 'notice', 'ignore') +DO $do$ +DECLARE + v_role_to_grant TEXT := $1; + v_username TEXT := $2; + v_use_locks BOOLEAN := COALESCE($3::boolean, false); + v_lock_namespace INT := COALESCE($4::int, 43); + v_on_missing_role TEXT := COALESCE($5, 'notice'); +BEGIN + -- Check if membership already exists + IF NOT EXISTS ( + SELECT 1 FROM pg_auth_members am + JOIN pg_roles r1 ON am.roleid = r1.oid + JOIN pg_roles r2 ON am.member = r2.oid + WHERE r1.rolname = v_role_to_grant AND r2.rolname = v_username + ) THEN + BEGIN + -- Acquire advisory lock if requested (prevents race conditions in concurrent CI/CD) + IF v_use_locks THEN + PERFORM pg_advisory_xact_lock(v_lock_namespace, hashtext(v_role_to_grant || ':' || v_username)); + END IF; + + EXECUTE format('GRANT %I TO %I', v_role_to_grant, v_username); + EXCEPTION + WHEN unique_violation THEN + -- Membership was granted concurrently, safe to ignore + NULL; + WHEN undefined_object THEN + -- Role doesn't exist + CASE v_on_missing_role + WHEN 'error' THEN + RAISE EXCEPTION 'Role % does not exist when granting to %', v_role_to_grant, v_username; + WHEN 'notice' THEN + RAISE NOTICE 'Missing role when granting % to %', v_role_to_grant, v_username; + WHEN 'ignore' THEN + NULL; + ELSE + RAISE NOTICE 'Missing role when granting % to %', v_role_to_grant, v_username; + END CASE; + WHEN insufficient_privilege THEN + RAISE EXCEPTION 'Insufficient privileges to grant % to %: ensure the connecting user has appropriate permissions', v_role_to_grant, v_username; + END; + END IF; +END +$do$; diff --git a/packages/core/src/init/sql/grant-connect.sql b/packages/core/src/init/sql/grant-connect.sql new file mode 100644 index 000000000..802bdcd35 --- /dev/null +++ b/packages/core/src/init/sql/grant-connect.sql @@ -0,0 +1,17 @@ +-- Grant CONNECT privilege on a database to a role +-- Parameters: $1 = role_name, $2 = db_name +DO $do$ +DECLARE + v_role_name TEXT := $1; + v_db_name TEXT := $2; +BEGIN + BEGIN + EXECUTE format('GRANT CONNECT ON DATABASE %I TO %I', v_db_name, v_role_name); + EXCEPTION + WHEN undefined_object THEN + RAISE NOTICE 'Role % does not exist when granting CONNECT on %', v_role_name, v_db_name; + WHEN insufficient_privilege THEN + RAISE EXCEPTION 'Insufficient privileges to grant CONNECT on % to %', v_db_name, v_role_name; + END; +END +$do$; diff --git a/packages/core/src/init/types.ts b/packages/core/src/init/types.ts new file mode 100644 index 000000000..959d2fb6b --- /dev/null +++ b/packages/core/src/init/types.ts @@ -0,0 +1,80 @@ +/** + * Role name mapping for customizable role names + */ +export interface RoleNameMapping { + anonymous?: string; + authenticated?: string; + administrator?: string; +} + +/** + * Default role names used throughout the system + */ +export const DEFAULT_ROLE_NAMES: Required = { + anonymous: 'anonymous', + authenticated: 'authenticated', + administrator: 'administrator' +}; + +/** + * How to handle missing base roles during membership grants + */ +export type OnMissingRoleAction = 'error' | 'notice' | 'ignore'; + +/** + * Options for ensuring a login role exists + */ +export interface EnsureLoginRoleOptions { + username: string; + password: string; + useLocks?: boolean; + lockNamespace?: number; +} + +/** + * Options for ensuring role memberships + */ +export interface EnsureRoleMembershipsOptions { + username: string; + rolesToGrant: string[]; + useLocks?: boolean; + lockNamespace?: number; + onMissingRole?: OnMissingRoleAction; +} + +/** + * Options for ensuring base roles exist + */ +export interface EnsureBaseRolesOptions { + roleNames?: RoleNameMapping; +} + +/** + * Options for granting database connect privilege + */ +export interface GrantConnectOptions { + roleName: string; + dbName: string; +} + +/** + * Combined options for creating a database user with memberships + */ +export interface CreateDbUserOptions { + username: string; + password: string; + rolesToGrant?: string[]; + useLocks?: boolean; + lockNamespace?: number; + onMissingRole?: OnMissingRoleAction; + roleNames?: RoleNameMapping; +} + +/** + * Options for bootstrapping test users + */ +export interface BootstrapTestUsersOptions { + useLocks?: boolean; + lockNamespace?: number; + roleNames?: RoleNameMapping; +} diff --git a/packages/pgsql-test/src/admin.ts b/packages/pgsql-test/src/admin.ts index 0f9643341..e91e90763 100644 --- a/packages/pgsql-test/src/admin.ts +++ b/packages/pgsql-test/src/admin.ts @@ -2,6 +2,7 @@ import { Logger } from '@pgpmjs/logger'; import { PgTestConnectionOptions } from '@pgpmjs/types'; import { execSync } from 'child_process'; import { existsSync } from 'fs'; +import { Client } from 'pg'; import { getPgEnvOptions, PgConfig } from 'pg-env'; import { getRoleName } from './roles'; @@ -10,6 +11,20 @@ import { streamSql as stream } from './stream'; const log = new Logger('db-admin'); +/** + * Options for creating a user role in the test framework + */ +export interface CreateUserRoleOptions { + /** Enable advisory locks for concurrent CI/CD safety (default: false) */ + useLocks?: boolean; + /** Lock namespace for advisory locks (default: 42) */ + lockNamespace?: number; + /** Grant administrator role to the user (default: true for backward compat) */ + grantAdmin?: boolean; + /** How to handle missing base roles: 'error', 'notice', or 'ignore' (default: 'notice') */ + onMissingRole?: 'error' | 'notice' | 'ignore'; +} + export class DbAdmin { constructor( private config: PgConfig, @@ -103,13 +118,30 @@ export class DbAdmin { this.safeDropDb(template); } - async grantRole(role: string, user: string, dbName?: string): Promise { + /** + * Grant a role to a user with optional advisory locks for concurrent CI/CD safety + */ + async grantRole( + role: string, + user: string, + dbName?: string, + options?: { + useLocks?: boolean; + lockNamespace?: number; + onMissingRole?: 'error' | 'notice' | 'ignore'; + } + ): Promise { const db = dbName ?? this.config.database; + const { useLocks = false, lockNamespace = 43, onMissingRole = 'notice' } = options || {}; + const sql = ` DO $$ DECLARE - v_user TEXT := '${user.replace(/'/g, "''")}'; - v_role TEXT := '${role.replace(/'/g, "''")}'; + v_user TEXT := $1; + v_role TEXT := $2; + v_use_locks BOOLEAN := $3; + v_lock_namespace INT := $4; + v_on_missing_role TEXT := $5; BEGIN -- Pre-check to avoid unnecessary GRANTs; still catch TOCTOU under concurrency IF NOT EXISTS ( @@ -119,20 +151,36 @@ BEGIN WHERE r1.rolname = v_role AND r2.rolname = v_user ) THEN BEGIN + -- Acquire advisory lock if requested (prevents race conditions in concurrent CI/CD) + IF v_use_locks THEN + PERFORM pg_advisory_xact_lock(v_lock_namespace, hashtext(v_role || ':' || v_user)); + END IF; + EXECUTE format('GRANT %I TO %I', v_role, v_user); EXCEPTION WHEN unique_violation THEN -- Concurrent membership grant; safe to ignore NULL; WHEN undefined_object THEN - -- Role or user missing; emit notice and continue - RAISE NOTICE 'Missing role when granting % to %', v_role, v_user; + -- Role or user missing + CASE v_on_missing_role + WHEN 'error' THEN + RAISE EXCEPTION 'Role % does not exist when granting to %', v_role, v_user; + WHEN 'notice' THEN + RAISE NOTICE 'Missing role when granting % to %', v_role, v_user; + WHEN 'ignore' THEN + NULL; + ELSE + RAISE NOTICE 'Missing role when granting % to %', v_role, v_user; + END CASE; + WHEN insufficient_privilege THEN + RAISE EXCEPTION 'Insufficient privileges to grant % to %', v_role, v_user; END; END IF; END $$; `; - await this.streamSql(sql, db); + await this.streamSqlWithParams(sql, [user, role, useLocks, lockNamespace, onMissingRole], db); } async grantConnect(role: string, dbName?: string): Promise { @@ -141,82 +189,84 @@ $$; await this.streamSql(sql, db); } - // TODO: make adminRole a configurable option - // ONLY granting admin role for testing purposes, normally the db connection for apps won't have admin role - // DO NOT USE THIS FOR PRODUCTION - async createUserRole(user: string, password: string, dbName: string): Promise { + /** + * Create a user role with role memberships for testing + * + * By default, grants anonymous, authenticated, AND administrator roles for backward compatibility. + * Use the `grantAdmin` option to control whether administrator role is granted. + * + * WARNING: This grants elevated privileges and should NEVER be used in production! + * + * @param user - Username for the new role + * @param password - Password for the new role + * @param dbName - Database name to connect to + * @param options - Optional configuration for locks and role grants + */ + async createUserRole( + user: string, + password: string, + dbName: string, + options?: CreateUserRoleOptions + ): Promise { + const { + useLocks = false, + lockNamespace = 42, + grantAdmin = true, // Default true for backward compatibility + onMissingRole = 'notice' + } = options || {}; + const anonRole = getRoleName('anonymous', this.roleConfig); const authRole = getRoleName('authenticated', this.roleConfig); const adminRole = getRoleName('administrator', this.roleConfig); - const sql = ` - DO $$ - DECLARE - v_user TEXT := '${user.replace(/'/g, "''")}'; - v_password TEXT := '${password.replace(/'/g, "''")}'; - BEGIN - -- Create role if it doesn't exist - BEGIN - EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', v_user, v_password); - EXCEPTION - WHEN duplicate_object THEN - -- Role already exists; optionally sync attributes here with ALTER ROLE - NULL; - END; - - -- CI/CD concurrency note: GRANT role membership can race on pg_auth_members unique index - -- We pre-check membership and still catch unique_violation to handle TOCTOU safely. - IF NOT EXISTS ( - SELECT 1 FROM pg_auth_members am - JOIN pg_roles r1 ON am.roleid = r1.oid - JOIN pg_roles r2 ON am.member = r2.oid - WHERE r1.rolname = '${anonRole.replace(/'/g, "''")}' AND r2.rolname = v_user - ) THEN - BEGIN - EXECUTE format('GRANT %I TO %I', '${anonRole.replace(/'/g, "''")}', v_user); - EXCEPTION - WHEN unique_violation THEN - NULL; - WHEN undefined_object THEN - RAISE NOTICE 'Missing role when granting % to %', '${anonRole.replace(/'/g, "''")}', v_user; - END; - END IF; - - IF NOT EXISTS ( - SELECT 1 FROM pg_auth_members am - JOIN pg_roles r1 ON am.roleid = r1.oid - JOIN pg_roles r2 ON am.member = r2.oid - WHERE r1.rolname = '${authRole.replace(/'/g, "''")}' AND r2.rolname = v_user - ) THEN - BEGIN - EXECUTE format('GRANT %I TO %I', '${authRole.replace(/'/g, "''")}', v_user); - EXCEPTION - WHEN unique_violation THEN - NULL; - WHEN undefined_object THEN - RAISE NOTICE 'Missing role when granting % to %', '${authRole.replace(/'/g, "''")}', v_user; - END; - END IF; - - IF NOT EXISTS ( - SELECT 1 FROM pg_auth_members am - JOIN pg_roles r1 ON am.roleid = r1.oid - JOIN pg_roles r2 ON am.member = r2.oid - WHERE r1.rolname = '${adminRole.replace(/'/g, "''")}' AND r2.rolname = v_user - ) THEN - BEGIN - EXECUTE format('GRANT %I TO %I', '${adminRole.replace(/'/g, "''")}', v_user); - EXCEPTION - WHEN unique_violation THEN - NULL; - WHEN undefined_object THEN - RAISE NOTICE 'Missing role when granting % to %', '${adminRole.replace(/'/g, "''")}', v_user; - END; - END IF; - END $$; - `.trim(); - - await this.streamSql(sql, dbName); + // Build the list of roles to grant + const rolesToGrant = [anonRole, authRole]; + if (grantAdmin) { + rolesToGrant.push(adminRole); + } + + // Create the login role with optional advisory locks + const createRoleSql = ` +DO $$ +DECLARE + v_user TEXT := $1; + v_password TEXT := $2; + v_use_locks BOOLEAN := $3; + v_lock_namespace INT := $4; +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = v_user) THEN + BEGIN + -- Acquire advisory lock if requested (prevents race conditions in concurrent CI/CD) + IF v_use_locks THEN + PERFORM pg_advisory_xact_lock(v_lock_namespace, hashtext(v_user)); + END IF; + + EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', v_user, v_password); + EXCEPTION + WHEN duplicate_object THEN + -- Role was created concurrently, safe to ignore + NULL; + WHEN unique_violation THEN + -- Concurrent insert, safe to ignore + NULL; + WHEN insufficient_privilege THEN + RAISE EXCEPTION 'Insufficient privileges to create role %: ensure the connecting user has CREATEROLE', v_user; + END; + END IF; +END +$$; + `; + + await this.streamSqlWithParams(createRoleSql, [user, password, useLocks, lockNamespace], dbName); + + // Grant role memberships + for (const role of rolesToGrant) { + await this.grantRole(role, user, dbName, { + useLocks, + lockNamespace: lockNamespace + 1, // Use different namespace for memberships + onMissingRole + }); + } } loadSql(file: string, dbName: string): void { @@ -236,6 +286,27 @@ $$; ); } + /** + * Execute SQL with parameterized values using pg Client + * This is safer than string interpolation for user-provided values + */ + async streamSqlWithParams(sql: string, params: any[], dbName: string): Promise { + const client = new Client({ + host: this.config.host, + port: this.config.port, + user: this.config.user, + password: this.config.password, + database: dbName + }); + + try { + await client.connect(); + await client.query(sql, params); + } finally { + await client.end(); + } + } + async createSeededTemplate(templateName: string, adapter: SeedAdapter): Promise { const seedDb = this.config.database; this.create(seedDb); From 6a8469f2b6ce19433991049313a053b4cb722313 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 18 Dec 2025 21:02:54 +0000 Subject: [PATCH 2/4] fix: consolidate ensure-base-roles.sql into single DO block PostgreSQL's pg library doesn't support multiple statements in a parameterized query. Consolidate the two DO blocks and BEGIN/COMMIT into a single DO block to fix the 'cannot insert multiple commands into a prepared statement' error. Co-Authored-By: Dan Lynch --- packages/core/src/init/sql/ensure-base-roles.sql | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/packages/core/src/init/sql/ensure-base-roles.sql b/packages/core/src/init/sql/ensure-base-roles.sql index aba02a1a0..7d07419ae 100644 --- a/packages/core/src/init/sql/ensure-base-roles.sql +++ b/packages/core/src/init/sql/ensure-base-roles.sql @@ -1,6 +1,6 @@ -- Ensure base roles exist (anonymous, authenticated, administrator) -- Parameters: $1 = anonymous role name, $2 = authenticated role name, $3 = administrator role name -BEGIN; +-- Note: This is a single DO block to work with parameterized queries (pg library limitation) DO $do$ DECLARE v_anonymous TEXT := COALESCE($1, 'anonymous'); @@ -30,17 +30,8 @@ BEGIN WHEN duplicate_object THEN NULL; END; -END -$do$; - --- Set role attributes (safe to run even if role already exists) --- These use dynamic SQL to support custom role names -DO $do$ -DECLARE - v_anonymous TEXT := COALESCE($1, 'anonymous'); - v_authenticated TEXT := COALESCE($2, 'authenticated'); - v_administrator TEXT := COALESCE($3, 'administrator'); -BEGIN + + -- Set role attributes (safe to run even if role already exists) -- Anonymous role attributes EXECUTE format('ALTER ROLE %I WITH NOCREATEDB NOSUPERUSER NOCREATEROLE NOLOGIN NOREPLICATION NOBYPASSRLS', v_anonymous); @@ -51,4 +42,3 @@ BEGIN EXECUTE format('ALTER ROLE %I WITH NOCREATEDB NOSUPERUSER NOCREATEROLE NOLOGIN NOREPLICATION BYPASSRLS', v_administrator); END $do$; -COMMIT; From 7692fb7671d751a70f58f3746e5f84e1913007aa Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 18 Dec 2025 21:13:45 +0000 Subject: [PATCH 3/4] fix: use inline SQL with escaping instead of parameterized DO blocks DO blocks in PostgreSQL don't support parameterized queries (, , etc.) because the block body is parsed as a string literal. This fix: - Removes loadSqlTemplate and uses inline SQL with safe string interpolation - Uses escapeLiteral() for SQL string values - Uses format('%I', ...) inside PL/pgSQL for identifier escaping - Applies fix to all role management functions Co-Authored-By: Dan Lynch --- packages/core/src/init/role-utils.ts | 149 ++++++++++++++++++++++++--- 1 file changed, 135 insertions(+), 14 deletions(-) diff --git a/packages/core/src/init/role-utils.ts b/packages/core/src/init/role-utils.ts index be3c40df4..8c8b5ae57 100644 --- a/packages/core/src/init/role-utils.ts +++ b/packages/core/src/init/role-utils.ts @@ -1,6 +1,4 @@ import { Logger } from '@pgpmjs/logger'; -import { readFileSync } from 'fs'; -import { join } from 'path'; import { Pool } from 'pg'; import { @@ -16,11 +14,21 @@ import { const log = new Logger('role-utils'); /** - * Load a SQL template file from the sql directory + * Escape a SQL identifier (role name, database name, etc.) + * This prevents SQL injection by properly quoting identifiers */ -function loadSqlTemplate(templateName: string): string { - const sqlPath = join(__dirname, 'sql', `${templateName}.sql`); - return readFileSync(sqlPath, 'utf-8'); +function escapeIdentifier(identifier: string): string { + // Double any double quotes and wrap in double quotes + return '"' + identifier.replace(/"/g, '""') + '"'; +} + +/** + * Escape a SQL literal (string value) + * This prevents SQL injection by properly quoting string literals + */ +function escapeLiteral(value: string): string { + // Double any single quotes and wrap in single quotes + return "'" + value.replace(/'/g, "''") + "'"; } /** @@ -45,9 +53,48 @@ export async function ensureBaseRoles( log.info('Ensuring base roles exist...'); - const sql = loadSqlTemplate('ensure-base-roles'); + // DO blocks don't support parameterized queries, so we use safe string interpolation + // with format() inside PL/pgSQL for identifier escaping + const sql = ` +DO $do$ +DECLARE + v_anonymous TEXT := ${escapeLiteral(names.anonymous)}; + v_authenticated TEXT := ${escapeLiteral(names.authenticated)}; + v_administrator TEXT := ${escapeLiteral(names.administrator)}; +BEGIN + -- Create anonymous role + BEGIN + EXECUTE format('CREATE ROLE %I', v_anonymous); + EXCEPTION + WHEN duplicate_object THEN + NULL; + END; + + -- Create authenticated role + BEGIN + EXECUTE format('CREATE ROLE %I', v_authenticated); + EXCEPTION + WHEN duplicate_object THEN + NULL; + END; - await pool.query(sql, [names.anonymous, names.authenticated, names.administrator]); + -- Create administrator role + BEGIN + EXECUTE format('CREATE ROLE %I', v_administrator); + EXCEPTION + WHEN duplicate_object THEN + NULL; + END; + + -- Set role attributes (safe to run even if role already exists) + EXECUTE format('ALTER ROLE %I WITH NOCREATEDB NOSUPERUSER NOCREATEROLE NOLOGIN NOREPLICATION NOBYPASSRLS', v_anonymous); + EXECUTE format('ALTER ROLE %I WITH NOCREATEDB NOSUPERUSER NOCREATEROLE NOLOGIN NOREPLICATION NOBYPASSRLS', v_authenticated); + EXECUTE format('ALTER ROLE %I WITH NOCREATEDB NOSUPERUSER NOCREATEROLE NOLOGIN NOREPLICATION BYPASSRLS', v_administrator); +END +$do$; + `; + + await pool.query(sql); log.success('Base roles ensured successfully'); } @@ -64,9 +111,31 @@ export async function ensureLoginRole( log.info(`Ensuring login role exists: ${username}...`); - const sql = loadSqlTemplate('ensure-login-role'); + // DO blocks don't support parameterized queries, so we use safe string interpolation + const sql = ` +DO $do$ +DECLARE + v_username TEXT := ${escapeLiteral(username)}; + v_password TEXT := ${escapeLiteral(password)}; +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = v_username) THEN + BEGIN + ${useLocks ? `PERFORM pg_advisory_xact_lock(${lockNamespace}, hashtext(v_username));` : '-- Locks disabled'} + EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', v_username, v_password); + EXCEPTION + WHEN duplicate_object THEN + NULL; + WHEN unique_violation THEN + NULL; + WHEN insufficient_privilege THEN + RAISE EXCEPTION 'Insufficient privileges to create role %: ensure the connecting user has CREATEROLE', v_username; + END; + END IF; +END +$do$; + `; - await pool.query(sql, [username, password, useLocks, lockNamespace]); + await pool.query(sql); log.success(`Login role ensured: ${username}`); } @@ -87,9 +156,44 @@ export async function ensureRoleMembership( ): Promise { const { useLocks = false, lockNamespace = 43, onMissingRole = 'notice' } = options; - const sql = loadSqlTemplate('ensure-membership'); + // Build the exception handler based on onMissingRole option + let undefinedObjectHandler: string; + if (onMissingRole === 'error') { + undefinedObjectHandler = `RAISE EXCEPTION 'Missing role when granting % to %', v_role_to_grant, v_username;`; + } else if (onMissingRole === 'ignore') { + undefinedObjectHandler = `NULL;`; + } else { + undefinedObjectHandler = `RAISE NOTICE 'Missing role when granting % to %', v_role_to_grant, v_username;`; + } + + // DO blocks don't support parameterized queries, so we use safe string interpolation + const sql = ` +DO $do$ +DECLARE + v_role_to_grant TEXT := ${escapeLiteral(roleToGrant)}; + v_username TEXT := ${escapeLiteral(username)}; +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_auth_members am + JOIN pg_roles r1 ON am.roleid = r1.oid + JOIN pg_roles r2 ON am.member = r2.oid + WHERE r1.rolname = v_role_to_grant AND r2.rolname = v_username + ) THEN + BEGIN + ${useLocks ? `PERFORM pg_advisory_xact_lock(${lockNamespace}, hashtext(v_role_to_grant || ':' || v_username));` : '-- Locks disabled'} + EXECUTE format('GRANT %I TO %I', v_role_to_grant, v_username); + EXCEPTION + WHEN unique_violation THEN + NULL; + WHEN undefined_object THEN + ${undefinedObjectHandler} + END; + END IF; +END +$do$; + `; - await pool.query(sql, [roleToGrant, username, useLocks, lockNamespace, onMissingRole]); + await pool.query(sql); } /** @@ -132,9 +236,26 @@ export async function grantConnect( log.info(`Granting CONNECT on ${dbName} to ${roleName}...`); - const sql = loadSqlTemplate('grant-connect'); + // DO blocks don't support parameterized queries, so we use safe string interpolation + const sql = ` +DO $do$ +DECLARE + v_role_name TEXT := ${escapeLiteral(roleName)}; + v_db_name TEXT := ${escapeLiteral(dbName)}; +BEGIN + BEGIN + EXECUTE format('GRANT CONNECT ON DATABASE %I TO %I', v_db_name, v_role_name); + EXCEPTION + WHEN undefined_object THEN + RAISE NOTICE 'Role % does not exist, skipping GRANT CONNECT', v_role_name; + WHEN invalid_catalog_name THEN + RAISE NOTICE 'Database % does not exist, skipping GRANT CONNECT', v_db_name; + END; +END +$do$; + `; - await pool.query(sql, [roleName, dbName]); + await pool.query(sql); log.success(`CONNECT granted on ${dbName} to ${roleName}`); } From 5b7d93afe7366cef639643e42b7866955ab98b87 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 18 Dec 2025 21:20:38 +0000 Subject: [PATCH 4/4] fix: use inline SQL with escaping in pgsql-test admin.ts DO blocks in PostgreSQL don't support parameterized queries (, , etc.) because the block body is parsed as a string literal. This fix: - Uses escapeLiteral() for SQL string values in grantRole and createUserRole - Uses conditional string interpolation for optional advisory locks - Removes unused streamSqlWithParams method and Client import - Applies same fix pattern as role-utils.ts Co-Authored-By: Dan Lynch --- packages/pgsql-test/src/admin.ts | 82 +++++++++++--------------------- 1 file changed, 29 insertions(+), 53 deletions(-) diff --git a/packages/pgsql-test/src/admin.ts b/packages/pgsql-test/src/admin.ts index e91e90763..80a5abb88 100644 --- a/packages/pgsql-test/src/admin.ts +++ b/packages/pgsql-test/src/admin.ts @@ -2,7 +2,6 @@ import { Logger } from '@pgpmjs/logger'; import { PgTestConnectionOptions } from '@pgpmjs/types'; import { execSync } from 'child_process'; import { existsSync } from 'fs'; -import { Client } from 'pg'; import { getPgEnvOptions, PgConfig } from 'pg-env'; import { getRoleName } from './roles'; @@ -11,6 +10,14 @@ import { streamSql as stream } from './stream'; const log = new Logger('db-admin'); +/** + * Escape a SQL literal (string value) + * This prevents SQL injection by properly quoting string literals + */ +function escapeLiteral(value: string): string { + return "'" + value.replace(/'/g, "''") + "'"; +} + /** * Options for creating a user role in the test framework */ @@ -134,14 +141,22 @@ export class DbAdmin { const db = dbName ?? this.config.database; const { useLocks = false, lockNamespace = 43, onMissingRole = 'notice' } = options || {}; + // Build the exception handler based on onMissingRole option + let undefinedObjectHandler: string; + if (onMissingRole === 'error') { + undefinedObjectHandler = `RAISE EXCEPTION 'Role % does not exist when granting to %', v_role, v_user;`; + } else if (onMissingRole === 'ignore') { + undefinedObjectHandler = `NULL;`; + } else { + undefinedObjectHandler = `RAISE NOTICE 'Missing role when granting % to %', v_role, v_user;`; + } + + // DO blocks don't support parameterized queries, so we use safe string interpolation const sql = ` DO $$ DECLARE - v_user TEXT := $1; - v_role TEXT := $2; - v_use_locks BOOLEAN := $3; - v_lock_namespace INT := $4; - v_on_missing_role TEXT := $5; + v_user TEXT := ${escapeLiteral(user)}; + v_role TEXT := ${escapeLiteral(role)}; BEGIN -- Pre-check to avoid unnecessary GRANTs; still catch TOCTOU under concurrency IF NOT EXISTS ( @@ -151,11 +166,7 @@ BEGIN WHERE r1.rolname = v_role AND r2.rolname = v_user ) THEN BEGIN - -- Acquire advisory lock if requested (prevents race conditions in concurrent CI/CD) - IF v_use_locks THEN - PERFORM pg_advisory_xact_lock(v_lock_namespace, hashtext(v_role || ':' || v_user)); - END IF; - + ${useLocks ? `PERFORM pg_advisory_xact_lock(${lockNamespace}, hashtext(v_role || ':' || v_user));` : '-- Locks disabled'} EXECUTE format('GRANT %I TO %I', v_role, v_user); EXCEPTION WHEN unique_violation THEN @@ -163,16 +174,7 @@ BEGIN NULL; WHEN undefined_object THEN -- Role or user missing - CASE v_on_missing_role - WHEN 'error' THEN - RAISE EXCEPTION 'Role % does not exist when granting to %', v_role, v_user; - WHEN 'notice' THEN - RAISE NOTICE 'Missing role when granting % to %', v_role, v_user; - WHEN 'ignore' THEN - NULL; - ELSE - RAISE NOTICE 'Missing role when granting % to %', v_role, v_user; - END CASE; + ${undefinedObjectHandler} WHEN insufficient_privilege THEN RAISE EXCEPTION 'Insufficient privileges to grant % to %', v_role, v_user; END; @@ -180,7 +182,7 @@ BEGIN END $$; `; - await this.streamSqlWithParams(sql, [user, role, useLocks, lockNamespace, onMissingRole], db); + await this.streamSql(sql, db); } async grantConnect(role: string, dbName?: string): Promise { @@ -226,21 +228,16 @@ $$; } // Create the login role with optional advisory locks + // DO blocks don't support parameterized queries, so we use safe string interpolation const createRoleSql = ` DO $$ DECLARE - v_user TEXT := $1; - v_password TEXT := $2; - v_use_locks BOOLEAN := $3; - v_lock_namespace INT := $4; + v_user TEXT := ${escapeLiteral(user)}; + v_password TEXT := ${escapeLiteral(password)}; BEGIN IF NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = v_user) THEN BEGIN - -- Acquire advisory lock if requested (prevents race conditions in concurrent CI/CD) - IF v_use_locks THEN - PERFORM pg_advisory_xact_lock(v_lock_namespace, hashtext(v_user)); - END IF; - + ${useLocks ? `PERFORM pg_advisory_xact_lock(${lockNamespace}, hashtext(v_user));` : '-- Locks disabled'} EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', v_user, v_password); EXCEPTION WHEN duplicate_object THEN @@ -257,7 +254,7 @@ END $$; `; - await this.streamSqlWithParams(createRoleSql, [user, password, useLocks, lockNamespace], dbName); + await this.streamSql(createRoleSql, dbName); // Grant role memberships for (const role of rolesToGrant) { @@ -286,27 +283,6 @@ $$; ); } - /** - * Execute SQL with parameterized values using pg Client - * This is safer than string interpolation for user-provided values - */ - async streamSqlWithParams(sql: string, params: any[], dbName: string): Promise { - const client = new Client({ - host: this.config.host, - port: this.config.port, - user: this.config.user, - password: this.config.password, - database: dbName - }); - - try { - await client.connect(); - await client.query(sql, params); - } finally { - await client.end(); - } - } - async createSeededTemplate(templateName: string, adapter: SeedAdapter): Promise { const seedDb = this.config.database; this.create(seedDb);