From dacf6a95651b53a129cc50bb12283782c9babbd5 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Thu, 20 Aug 2026 22:43:27 -0700 Subject: [PATCH 1/4] fix: stub roles for ALTER DEFAULT PRIVILEGES in external plan DB When using an external plan database, ALTER DEFAULT PRIVILEGES FOR ROLE statements fail with "permission denied" (SQLSTATE 42501) because the plan user is not a member of the referenced role. Fix by parsing role names from the desired SQL, creating stub roles in the plan DB, and granting membership to the plan user before applying the schema. Stub roles are cleaned up in Stop(). Fixes #553 Co-Authored-By: Claude Opus 4.6 --- internal/postgres/external.go | 34 +++++++++++++++++++++-- internal/postgres/fk_refs.go | 46 +++++++++++++++++++++++++++++++ internal/postgres/fk_refs_test.go | 45 ++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 2 deletions(-) diff --git a/internal/postgres/external.go b/internal/postgres/external.go index 4ff20164..de904a35 100644 --- a/internal/postgres/external.go +++ b/internal/postgres/external.go @@ -21,8 +21,9 @@ type ExternalDatabase struct { database string username string password string - tempSchema string // Temporary schema name with timestamp suffix - targetMajorVersion int // Expected major version (from target database) + tempSchema string // Temporary schema name with timestamp suffix + targetMajorVersion int // Expected major version (from target database) + stubRoles []string // Roles created for ALTER DEFAULT PRIVILEGES (issue #553) } // ExternalDatabaseConfig holds configuration for connecting to an external database @@ -170,6 +171,26 @@ func (ed *ExternalDatabase) ApplySchema(ctx context.Context, schema string, sql // so we need to rewrite it to point to the temporary schema (issue #335) schemaAgnosticSQL = replaceSchemaInSearchPath(schemaAgnosticSQL, schema, ed.tempSchema) + // Create stub roles referenced by ALTER DEFAULT PRIVILEGES FOR ROLE so that + // the desired SQL can apply in the plan database without "permission denied" + // errors (issue #553). The plan user must be a member of these roles for + // PostgreSQL to accept the ALTER DEFAULT PRIVILEGES statement. + stubRoles := ExtractDefaultPrivilegeRoles(schemaAgnosticSQL) + for _, role := range stubRoles { + createRoleSQL := fmt.Sprintf("DO $$ BEGIN CREATE ROLE %s; EXCEPTION WHEN duplicate_object THEN NULL; END $$", quoteIdent(role)) + if _, err := util.ExecContextWithLogging(ctx, conn, createRoleSQL, "create stub role for default privileges"); err != nil { + return fmt.Errorf("failed to create stub role %s: %w", role, err) + } + grantSQL := fmt.Sprintf("GRANT %s TO %s", quoteIdent(role), quoteIdent(ed.username)) + if _, err := util.ExecContextWithLogging(ctx, conn, grantSQL, "grant stub role membership"); err != nil { + // Ignore if already a member + if !strings.Contains(err.Error(), "already a member") { + return fmt.Errorf("failed to grant role %s to %s: %w", role, ed.username, err) + } + } + } + ed.stubRoles = stubRoles + // Execute the SQL directly // Note: Desired state SQL should never contain operations like CREATE INDEX CONCURRENTLY // that cannot run in transactions. Those are migration details, not state declarations. @@ -192,6 +213,11 @@ func (ed *ExternalDatabase) Stop() error { dropSchemaSQL := fmt.Sprintf("DROP SCHEMA IF EXISTS \"%s\" CASCADE", ed.tempSchema) // Ignore errors - this is best effort cleanup _, _ = ed.db.ExecContext(ctx, dropSchemaSQL) + + // Drop stub roles created for ALTER DEFAULT PRIVILEGES (issue #553) + for _, role := range ed.stubRoles { + _, _ = ed.db.ExecContext(ctx, fmt.Sprintf("DROP ROLE IF EXISTS %s", quoteIdent(role))) + } } // Close database connection @@ -260,6 +286,10 @@ func getExtensionSchemas(db *sql.DB) (map[string]string, error) { return extensions, rows.Err() } +func quoteIdent(name string) string { + return `"` + strings.ReplaceAll(name, `"`, `""`) + `"` +} + // detectMajorVersion queries the database to determine its PostgreSQL major version func detectMajorVersion(db *sql.DB) (int, error) { ctx := context.Background() diff --git a/internal/postgres/fk_refs.go b/internal/postgres/fk_refs.go index 5aac59b8..4faf2ca8 100644 --- a/internal/postgres/fk_refs.go +++ b/internal/postgres/fk_refs.go @@ -175,6 +175,52 @@ func ExtractPartitionOfTargets(sql, defaultSchema string) []QualifiedName { return out } +// ExtractDefaultPrivilegeRoles returns distinct role names that appear in +// ALTER DEFAULT PRIVILEGES FOR ROLE statements in sql. +// String literals, comments, and dollar-quoted bodies are skipped. +func ExtractDefaultPrivilegeRoles(sql string) []string { + seen := make(map[string]bool) + var out []string + + walkSQLCode(sql, func(code string) { + i := 0 + for i < len(code) { + idx := indexKeyword(code, i, "default") + if idx < 0 { + return + } + i = idx + len("default") + i = skipSpace(code, i) + if !hasKeywordAt(code, i, "privileges") { + continue + } + i += len("privileges") + i = skipSpace(code, i) + if !hasKeywordAt(code, i, "for") { + continue + } + i += len("for") + i = skipSpace(code, i) + if !hasKeywordAt(code, i, "role") { + continue + } + i += len("role") + _, role, next, ok := parseQualifiedName(code, i) + if !ok { + i = next + continue + } + i = next + if !seen[role] { + seen[role] = true + out = append(out, role) + } + } + }) + + return out +} + func walkSQLCode(sql string, fn func(code string)) { for _, seg := range splitDollarQuotedSegments(sql) { if seg.quoted { diff --git a/internal/postgres/fk_refs_test.go b/internal/postgres/fk_refs_test.go index 7bb27cd5..cd2d9c06 100644 --- a/internal/postgres/fk_refs_test.go +++ b/internal/postgres/fk_refs_test.go @@ -175,3 +175,48 @@ func TestExtractPartitionOfTargets(t *testing.T) { }) } } + +func TestExtractDefaultPrivilegeRoles(t *testing.T) { + tests := []struct { + name string + sql string + want []string + }{ + { + name: "single role", + sql: `ALTER DEFAULT PRIVILEGES FOR ROLE postgres IN SCHEMA public GRANT SELECT ON TABLES TO anon;`, + want: []string{"postgres"}, + }, + { + name: "quoted role", + sql: `ALTER DEFAULT PRIVILEGES FOR ROLE "supabase_admin" IN SCHEMA public GRANT ALL ON TABLES TO authenticated;`, + want: []string{"supabase_admin"}, + }, + { + name: "multiple distinct roles deduped", + sql: `ALTER DEFAULT PRIVILEGES FOR ROLE postgres IN SCHEMA public GRANT SELECT ON TABLES TO anon; +ALTER DEFAULT PRIVILEGES FOR ROLE postgres IN SCHEMA public GRANT INSERT ON TABLES TO anon; +ALTER DEFAULT PRIVILEGES FOR ROLE "supabase_admin" IN SCHEMA public GRANT ALL ON TABLES TO authenticated;`, + want: []string{"postgres", "supabase_admin"}, + }, + { + name: "no alter default privileges", + sql: `CREATE TABLE users (id int); ALTER TABLE users ADD COLUMN name text;`, + want: nil, + }, + { + name: "case insensitive keywords", + sql: `alter default privileges for role myuser in schema public grant select on tables to reader;`, + want: []string{"myuser"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ExtractDefaultPrivilegeRoles(tt.sql) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ExtractDefaultPrivilegeRoles() = %#v, want %#v", got, tt.want) + } + }) + } +} From 22c05f0494dd6e7fdf0836f459c8f46297825ab6 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Thu, 20 Aug 2026 22:46:37 -0700 Subject: [PATCH 2/4] fix: only create and drop stub roles that did not pre-exist Check pg_roles before creating each stub role. Skip roles that already exist in the plan DB so Stop() never drops a pre-existing role. Also revoke grant before dropping to ensure clean removal. Co-Authored-By: Claude Opus 4.6 --- internal/postgres/external.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/internal/postgres/external.go b/internal/postgres/external.go index de904a35..0af895fe 100644 --- a/internal/postgres/external.go +++ b/internal/postgres/external.go @@ -175,21 +175,26 @@ func (ed *ExternalDatabase) ApplySchema(ctx context.Context, schema string, sql // the desired SQL can apply in the plan database without "permission denied" // errors (issue #553). The plan user must be a member of these roles for // PostgreSQL to accept the ALTER DEFAULT PRIVILEGES statement. - stubRoles := ExtractDefaultPrivilegeRoles(schemaAgnosticSQL) - for _, role := range stubRoles { - createRoleSQL := fmt.Sprintf("DO $$ BEGIN CREATE ROLE %s; EXCEPTION WHEN duplicate_object THEN NULL; END $$", quoteIdent(role)) + // Only roles that do not already exist are created and tracked for cleanup. + candidateRoles := ExtractDefaultPrivilegeRoles(schemaAgnosticSQL) + for _, role := range candidateRoles { + var exists bool + if err := conn.QueryRowContext(ctx, "SELECT EXISTS(SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = $1)", role).Scan(&exists); err != nil { + return fmt.Errorf("failed to check existence of role %s: %w", role, err) + } + if exists { + continue + } + createRoleSQL := fmt.Sprintf("CREATE ROLE %s", quoteIdent(role)) if _, err := util.ExecContextWithLogging(ctx, conn, createRoleSQL, "create stub role for default privileges"); err != nil { return fmt.Errorf("failed to create stub role %s: %w", role, err) } grantSQL := fmt.Sprintf("GRANT %s TO %s", quoteIdent(role), quoteIdent(ed.username)) if _, err := util.ExecContextWithLogging(ctx, conn, grantSQL, "grant stub role membership"); err != nil { - // Ignore if already a member - if !strings.Contains(err.Error(), "already a member") { - return fmt.Errorf("failed to grant role %s to %s: %w", role, ed.username, err) - } + return fmt.Errorf("failed to grant role %s to %s: %w", role, ed.username, err) } + ed.stubRoles = append(ed.stubRoles, role) } - ed.stubRoles = stubRoles // Execute the SQL directly // Note: Desired state SQL should never contain operations like CREATE INDEX CONCURRENTLY @@ -214,8 +219,10 @@ func (ed *ExternalDatabase) Stop() error { // Ignore errors - this is best effort cleanup _, _ = ed.db.ExecContext(ctx, dropSchemaSQL) - // Drop stub roles created for ALTER DEFAULT PRIVILEGES (issue #553) + // Clean up stub roles we created for ALTER DEFAULT PRIVILEGES (issue #553). + // Only roles we actually created are tracked, so this won't touch pre-existing roles. for _, role := range ed.stubRoles { + _, _ = ed.db.ExecContext(ctx, fmt.Sprintf("REVOKE %s FROM %s", quoteIdent(role), quoteIdent(ed.username))) _, _ = ed.db.ExecContext(ctx, fmt.Sprintf("DROP ROLE IF EXISTS %s", quoteIdent(role))) } } From 8477a8ac987b1888fe1cda1128d35e8d3e9b662b Mon Sep 17 00:00:00 2001 From: tianzhou Date: Thu, 20 Aug 2026 22:47:06 -0700 Subject: [PATCH 3/4] fix: error on pre-existing role without membership instead of mutating When a referenced role already exists in the plan DB, check whether the plan user is already a member. If not, return an actionable error instead of granting membership on a pre-existing cluster-level object. Co-Authored-By: Claude Opus 4.6 --- internal/postgres/external.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/postgres/external.go b/internal/postgres/external.go index 0af895fe..86b57130 100644 --- a/internal/postgres/external.go +++ b/internal/postgres/external.go @@ -183,6 +183,10 @@ func (ed *ExternalDatabase) ApplySchema(ctx context.Context, schema string, sql return fmt.Errorf("failed to check existence of role %s: %w", role, err) } if exists { + var isMember bool + if err := conn.QueryRowContext(ctx, "SELECT pg_has_role($1, $2, 'MEMBER')", ed.username, role).Scan(&isMember); err != nil || !isMember { + return fmt.Errorf("role %q already exists in the plan database and the plan user %q is not a member of it; grant membership manually or use a plan user that is already a member of that role", role, ed.username) + } continue } createRoleSQL := fmt.Sprintf("CREATE ROLE %s", quoteIdent(role)) From edcd530bea15a00d16be50db4b8f445caac4775b Mon Sep 17 00:00:00 2001 From: tianzhou Date: Thu, 20 Aug 2026 23:17:35 -0700 Subject: [PATCH 4/4] fix: support FOR USER synonym and comma-separated role lists ExtractDefaultPrivilegeRoles now handles: - FOR USER as a synonym for FOR ROLE (valid PostgreSQL syntax) - Comma-separated role lists: FOR ROLE r1, r2, r3 Added 3 test cases covering these forms. Co-Authored-By: Claude Opus 4.6 --- internal/postgres/fk_refs.go | 34 +++++++++++++++++++++---------- internal/postgres/fk_refs_test.go | 15 ++++++++++++++ 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/internal/postgres/fk_refs.go b/internal/postgres/fk_refs.go index 4faf2ca8..e4fa2e83 100644 --- a/internal/postgres/fk_refs.go +++ b/internal/postgres/fk_refs.go @@ -176,7 +176,7 @@ func ExtractPartitionOfTargets(sql, defaultSchema string) []QualifiedName { } // ExtractDefaultPrivilegeRoles returns distinct role names that appear in -// ALTER DEFAULT PRIVILEGES FOR ROLE statements in sql. +// ALTER DEFAULT PRIVILEGES FOR { ROLE | USER } [, ...] statements. // String literals, comments, and dollar-quoted bodies are skipped. func ExtractDefaultPrivilegeRoles(sql string) []string { seen := make(map[string]bool) @@ -201,19 +201,31 @@ func ExtractDefaultPrivilegeRoles(sql string) []string { } i += len("for") i = skipSpace(code, i) - if !hasKeywordAt(code, i, "role") { + if !hasKeywordAt(code, i, "role") && !hasKeywordAt(code, i, "user") { continue } - i += len("role") - _, role, next, ok := parseQualifiedName(code, i) - if !ok { - i = next - continue + if hasKeywordAt(code, i, "role") { + i += len("role") + } else { + i += len("user") } - i = next - if !seen[role] { - seen[role] = true - out = append(out, role) + for { + _, role, next, ok := parseQualifiedName(code, i) + if !ok { + i = next + break + } + i = next + if !seen[role] { + seen[role] = true + out = append(out, role) + } + i = skipSpace(code, i) + if i < len(code) && code[i] == ',' { + i++ + } else { + break + } } } }) diff --git a/internal/postgres/fk_refs_test.go b/internal/postgres/fk_refs_test.go index cd2d9c06..3531173b 100644 --- a/internal/postgres/fk_refs_test.go +++ b/internal/postgres/fk_refs_test.go @@ -209,6 +209,21 @@ ALTER DEFAULT PRIVILEGES FOR ROLE "supabase_admin" IN SCHEMA public GRANT ALL ON sql: `alter default privileges for role myuser in schema public grant select on tables to reader;`, want: []string{"myuser"}, }, + { + name: "FOR USER synonym", + sql: `ALTER DEFAULT PRIVILEGES FOR USER myuser IN SCHEMA public GRANT SELECT ON TABLES TO reader;`, + want: []string{"myuser"}, + }, + { + name: "comma-separated role list", + sql: `ALTER DEFAULT PRIVILEGES FOR ROLE role1, role2, role3 IN SCHEMA public GRANT SELECT ON TABLES TO reader;`, + want: []string{"role1", "role2", "role3"}, + }, + { + name: "comma-separated with FOR USER", + sql: `ALTER DEFAULT PRIVILEGES FOR USER admin, "app_owner" IN SCHEMA public GRANT ALL ON TABLES TO app;`, + want: []string{"admin", "app_owner"}, + }, } for _, tt := range tests {