From 2c32c32b7f312fe9b45a290d5759fd2e0ac905d7 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 18 Nov 2025 01:44:18 +0000 Subject: [PATCH 1/2] Fix concurrent role creation with exception handling only (no locks) - Catch both duplicate_object and unique_violation exceptions - No advisory locks - relies on exception handling alone - Applied to all role creation code paths: - bootstrap-roles.sql (anonymous, authenticated, administrator) - bootstrap-test-roles.sql (app_user, app_admin) - LaunchQLInit.bootstrapDbRoles() - DbAdmin.createUserRole() This approach handles the unique_violation error that occurs under concurrent CREATE ROLE execution without using advisory locks. This allows comparison of exception-only vs lock-based approaches. Co-Authored-By: Dan Lynch --- packages/core/src/init/client.ts | 3 +-- packages/core/src/init/sql/bootstrap-roles.sql | 11 ++++------- packages/core/src/init/sql/bootstrap-test-roles.sql | 6 ++---- packages/pgsql-test/src/admin.ts | 3 +-- 4 files changed, 8 insertions(+), 15 deletions(-) diff --git a/packages/core/src/init/client.ts b/packages/core/src/init/client.ts index ca7a7fa91..7226fadfd 100644 --- a/packages/core/src/init/client.ts +++ b/packages/core/src/init/client.ts @@ -72,8 +72,7 @@ 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 + WHEN duplicate_object OR unique_violation THEN NULL; END; END diff --git a/packages/core/src/init/sql/bootstrap-roles.sql b/packages/core/src/init/sql/bootstrap-roles.sql index 6b455b741..8afa53442 100644 --- a/packages/core/src/init/sql/bootstrap-roles.sql +++ b/packages/core/src/init/sql/bootstrap-roles.sql @@ -5,8 +5,7 @@ BEGIN BEGIN EXECUTE format('CREATE ROLE %I', 'anonymous'); EXCEPTION - WHEN duplicate_object THEN - -- Role already exists; optionally sync attributes here with ALTER ROLE + WHEN duplicate_object OR unique_violation THEN NULL; END; @@ -14,8 +13,7 @@ BEGIN BEGIN EXECUTE format('CREATE ROLE %I', 'authenticated'); EXCEPTION - WHEN duplicate_object THEN - -- Role already exists; optionally sync attributes here with ALTER ROLE + WHEN duplicate_object OR unique_violation THEN NULL; END; @@ -23,8 +21,7 @@ BEGIN BEGIN EXECUTE format('CREATE ROLE %I', 'administrator'); EXCEPTION - WHEN duplicate_object THEN - -- Role already exists; optionally sync attributes here with ALTER ROLE + WHEN duplicate_object OR unique_violation THEN NULL; END; END @@ -52,4 +49,4 @@ ALTER USER administrator WITH NOLOGIN; ALTER USER administrator WITH NOREPLICATION; -- they CAN bypass RLS ALTER USER administrator WITH BYPASSRLS; -COMMIT; \ No newline at end of file +COMMIT; diff --git a/packages/core/src/init/sql/bootstrap-test-roles.sql b/packages/core/src/init/sql/bootstrap-test-roles.sql index d5bc68f21..0a679614a 100644 --- a/packages/core/src/init/sql/bootstrap-test-roles.sql +++ b/packages/core/src/init/sql/bootstrap-test-roles.sql @@ -4,16 +4,14 @@ BEGIN BEGIN EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', 'app_user', 'app_password'); EXCEPTION - WHEN duplicate_object THEN - -- Role already exists; optionally sync attributes here with ALTER ROLE + WHEN duplicate_object OR unique_violation THEN NULL; END; BEGIN EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', 'app_admin', 'admin_password'); EXCEPTION - WHEN duplicate_object THEN - -- Role already exists; optionally sync attributes here with ALTER ROLE + WHEN duplicate_object OR unique_violation THEN NULL; END; END diff --git a/packages/pgsql-test/src/admin.ts b/packages/pgsql-test/src/admin.ts index 16adf666e..36e67d3cb 100644 --- a/packages/pgsql-test/src/admin.ts +++ b/packages/pgsql-test/src/admin.ts @@ -159,8 +159,7 @@ $$; 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 + WHEN duplicate_object OR unique_violation THEN NULL; END; From 0b5d6589206e30c9c3deb0aa4d10041586e49e7e Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 18 Nov 2025 01:51:29 +0000 Subject: [PATCH 2/2] Add pre-existence checks to all CREATE ROLE and GRANT statements (no locks) - Add IF NOT EXISTS checks before every CREATE ROLE statement - Add membership pre-checks before every GRANT statement - NO advisory locks (exceptions-only variant) - Keep both exception handlers (duplicate_object OR unique_violation) - Defense-in-depth: pre-check + exceptions (no locks) This pedantic approach ensures safety against concurrent role creation and membership grant races without using advisory locks. Co-Authored-By: Dan Lynch --- packages/core/src/init/client.ts | 65 ++++---- .../core/src/init/sql/bootstrap-roles.sql | 45 +++--- .../src/init/sql/bootstrap-test-roles.sql | 145 +++++++++++------- packages/pgsql-test/src/admin.ts | 15 +- 4 files changed, 161 insertions(+), 109 deletions(-) diff --git a/packages/core/src/init/client.ts b/packages/core/src/init/client.ts index 7226fadfd..67bad480d 100644 --- a/packages/core/src/init/client.ts +++ b/packages/core/src/init/client.ts @@ -69,41 +69,52 @@ 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 OR unique_violation THEN - NULL; - END; + IF NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = v_username) THEN + BEGIN + EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', v_username, v_password); + EXCEPTION + WHEN duplicate_object OR unique_violation THEN + NULL; + END; + END IF; 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; + 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 = 'anonymous' AND r2.rolname = v_username + ) THEN + BEGIN + EXECUTE format('GRANT %I TO %I', 'anonymous', v_username); + EXCEPTION + WHEN unique_violation THEN + NULL; + WHEN undefined_object THEN + RAISE NOTICE 'Missing role when granting % to %', 'anonymous', v_username; + END; + END IF; - 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; + 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 = 'authenticated' AND r2.rolname = v_username + ) THEN + BEGIN + EXECUTE format('GRANT %I TO %I', 'authenticated', v_username); + EXCEPTION + WHEN unique_violation THEN + NULL; + WHEN undefined_object THEN + RAISE NOTICE 'Missing role when granting % to %', 'authenticated', v_username; + END; + END IF; END $do$; COMMIT; diff --git a/packages/core/src/init/sql/bootstrap-roles.sql b/packages/core/src/init/sql/bootstrap-roles.sql index 8afa53442..bd70a068a 100644 --- a/packages/core/src/init/sql/bootstrap-roles.sql +++ b/packages/core/src/init/sql/bootstrap-roles.sql @@ -1,29 +1,32 @@ BEGIN; DO $do$ BEGIN - -- anonymous - BEGIN - EXECUTE format('CREATE ROLE %I', 'anonymous'); - EXCEPTION - WHEN duplicate_object OR unique_violation THEN - NULL; - END; + IF NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = 'anonymous') THEN + BEGIN + EXECUTE format('CREATE ROLE %I', 'anonymous'); + EXCEPTION + WHEN duplicate_object OR unique_violation THEN + NULL; + END; + END IF; - -- authenticated - BEGIN - EXECUTE format('CREATE ROLE %I', 'authenticated'); - EXCEPTION - WHEN duplicate_object OR unique_violation THEN - NULL; - END; + IF NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = 'authenticated') THEN + BEGIN + EXECUTE format('CREATE ROLE %I', 'authenticated'); + EXCEPTION + WHEN duplicate_object OR unique_violation THEN + NULL; + END; + END IF; - -- administrator - BEGIN - EXECUTE format('CREATE ROLE %I', 'administrator'); - EXCEPTION - WHEN duplicate_object OR unique_violation THEN - NULL; - END; + IF NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = 'administrator') THEN + BEGIN + EXECUTE format('CREATE ROLE %I', 'administrator'); + EXCEPTION + WHEN duplicate_object OR unique_violation THEN + NULL; + END; + END IF; END $do$; diff --git a/packages/core/src/init/sql/bootstrap-test-roles.sql b/packages/core/src/init/sql/bootstrap-test-roles.sql index 0a679614a..3110483e9 100644 --- a/packages/core/src/init/sql/bootstrap-test-roles.sql +++ b/packages/core/src/init/sql/bootstrap-test-roles.sql @@ -1,70 +1,107 @@ BEGIN; DO $do$ BEGIN - BEGIN - EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', 'app_user', 'app_password'); - EXCEPTION - WHEN duplicate_object OR unique_violation THEN - NULL; - END; + IF NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = 'app_user') THEN + BEGIN + EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', 'app_user', 'app_password'); + EXCEPTION + WHEN duplicate_object OR unique_violation THEN + NULL; + END; + END IF; - BEGIN - EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', 'app_admin', 'admin_password'); - EXCEPTION - WHEN duplicate_object OR unique_violation THEN - NULL; - END; + IF NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = 'app_admin') THEN + BEGIN + EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', 'app_admin', 'admin_password'); + EXCEPTION + WHEN duplicate_object OR unique_violation THEN + NULL; + END; + END IF; END $do$; DO $do$ BEGIN - BEGIN - EXECUTE format('GRANT %I TO %I', 'anonymous', 'app_user'); - 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', 'app_user'; - END; + 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 = 'anonymous' AND r2.rolname = 'app_user' + ) THEN + BEGIN + EXECUTE format('GRANT %I TO %I', 'anonymous', 'app_user'); + EXCEPTION + WHEN unique_violation THEN + NULL; + WHEN undefined_object THEN + RAISE NOTICE 'Missing role when granting % to %', 'anonymous', 'app_user'; + END; + END IF; - BEGIN - EXECUTE format('GRANT %I TO %I', 'authenticated', 'app_user'); - EXCEPTION - WHEN unique_violation THEN - NULL; - WHEN undefined_object THEN - RAISE NOTICE 'Missing role when granting % to %', 'authenticated', 'app_user'; - END; + 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 = 'authenticated' AND r2.rolname = 'app_user' + ) THEN + BEGIN + EXECUTE format('GRANT %I TO %I', 'authenticated', 'app_user'); + EXCEPTION + WHEN unique_violation THEN + NULL; + WHEN undefined_object THEN + RAISE NOTICE 'Missing role when granting % to %', 'authenticated', 'app_user'; + END; + END IF; - BEGIN - EXECUTE format('GRANT %I TO %I', 'anonymous', 'administrator'); - EXCEPTION - WHEN unique_violation THEN - NULL; - WHEN undefined_object THEN - RAISE NOTICE 'Missing role when granting % to %', 'anonymous', 'administrator'; - END; + 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 = 'anonymous' AND r2.rolname = 'administrator' + ) THEN + BEGIN + EXECUTE format('GRANT %I TO %I', 'anonymous', 'administrator'); + EXCEPTION + WHEN unique_violation THEN + NULL; + WHEN undefined_object THEN + RAISE NOTICE 'Missing role when granting % to %', 'anonymous', 'administrator'; + END; + END IF; - BEGIN - EXECUTE format('GRANT %I TO %I', 'authenticated', 'administrator'); - EXCEPTION - WHEN unique_violation THEN - NULL; - WHEN undefined_object THEN - RAISE NOTICE 'Missing role when granting % to %', 'authenticated', 'administrator'; - END; + 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 = 'authenticated' AND r2.rolname = 'administrator' + ) THEN + BEGIN + EXECUTE format('GRANT %I TO %I', 'authenticated', 'administrator'); + EXCEPTION + WHEN unique_violation THEN + NULL; + WHEN undefined_object THEN + RAISE NOTICE 'Missing role when granting % to %', 'authenticated', 'administrator'; + END; + END IF; - BEGIN - EXECUTE format('GRANT %I TO %I', 'administrator', 'app_admin'); - EXCEPTION - WHEN unique_violation THEN - NULL; - WHEN undefined_object THEN - RAISE NOTICE 'Missing role when granting % to %', 'administrator', 'app_admin'; - END; + 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 = 'administrator' AND r2.rolname = 'app_admin' + ) THEN + BEGIN + EXECUTE format('GRANT %I TO %I', 'administrator', 'app_admin'); + EXCEPTION + WHEN unique_violation THEN + NULL; + WHEN undefined_object THEN + RAISE NOTICE 'Missing role when granting % to %', 'administrator', 'app_admin'; + END; + END IF; END $do$; COMMIT; diff --git a/packages/pgsql-test/src/admin.ts b/packages/pgsql-test/src/admin.ts index 36e67d3cb..cb015b048 100644 --- a/packages/pgsql-test/src/admin.ts +++ b/packages/pgsql-test/src/admin.ts @@ -155,13 +155,14 @@ $$; 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 OR unique_violation THEN - NULL; - END; + IF NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = v_user) THEN + BEGIN + EXECUTE format('CREATE ROLE %I LOGIN PASSWORD %L', v_user, v_password); + EXCEPTION + WHEN duplicate_object OR unique_violation THEN + NULL; + END; + END IF; -- 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.