From b57957e691a0a577499b73601ffd58af01f91169 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Fri, 27 Mar 2026 04:09:49 -0700 Subject: [PATCH 1/2] fix: strip temp schema prefix from GRANT/REVOKE function signatures (#376) When using an external database for plan generation, pg_get_function_identity_arguments() qualifies user-defined argument types with the temporary schema name (e.g., pgschema_tmp_20260326_161823_31f3dbda.my_input[]). This leaked into GRANT/REVOKE statements, causing apply to fail with "schema does not exist". Fix: normalize privilege ObjectName to strip the schema prefix from function/procedure argument types, consistent with how other schema objects are normalized. Co-Authored-By: Claude Opus 4.6 (1M context) --- ir/normalize.go | 53 +++++++++++++++++++ .../diff.sql | 3 ++ .../new.sql | 20 +++++++ .../old.sql | 17 ++++++ .../plan.json | 26 +++++++++ .../plan.sql | 3 ++ .../plan.txt | 18 +++++++ 7 files changed, 140 insertions(+) create mode 100644 testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/diff.sql create mode 100644 testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/new.sql create mode 100644 testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/old.sql create mode 100644 testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/plan.json create mode 100644 testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/plan.sql create mode 100644 testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/plan.txt diff --git a/ir/normalize.go b/ir/normalize.go index aeddf102..612fac89 100644 --- a/ir/normalize.go +++ b/ir/normalize.go @@ -57,6 +57,18 @@ func normalizeSchema(schema *Schema) { for _, typeObj := range schema.Types { normalizeType(typeObj) } + + // Normalize privileges - strip schema qualifiers from function/procedure signatures + for _, priv := range schema.Privileges { + if priv.ObjectType == PrivilegeObjectTypeFunction || priv.ObjectType == PrivilegeObjectTypeProcedure { + priv.ObjectName = normalizePrivilegeObjectName(priv.ObjectName, schema.Name) + } + } + for _, revoked := range schema.RevokedDefaultPrivileges { + if revoked.ObjectType == PrivilegeObjectTypeFunction || revoked.ObjectType == PrivilegeObjectTypeProcedure { + revoked.ObjectName = normalizePrivilegeObjectName(revoked.ObjectName, schema.Name) + } + } } // normalizeTable normalizes table-related objects @@ -634,6 +646,47 @@ func stripSchemaPrefix(typeName, prefix string) string { return typeName } +// normalizePrivilegeObjectName strips schema qualifiers from argument types +// in function/procedure signatures used in GRANT/REVOKE statements. +// e.g., "f_test(p_items pgschema_tmp_20260326_161823_31f3dbda.my_input[])" → "f_test(p_items my_input[])" +func normalizePrivilegeObjectName(objectName, schemaName string) string { + if objectName == "" || schemaName == "" { + return objectName + } + + // Find the argument list in the signature: name(args) + parenOpen := strings.Index(objectName, "(") + parenClose := strings.LastIndex(objectName, ")") + if parenOpen < 0 || parenClose < 0 || parenClose <= parenOpen { + return objectName + } + + funcName := objectName[:parenOpen] + args := objectName[parenOpen+1 : parenClose] + + if args == "" { + return objectName + } + + prefix := schemaName + "." + + // Split args by comma, strip schema prefix from each type reference, rejoin + parts := strings.Split(args, ", ") + changed := false + for i, part := range parts { + if strings.Contains(part, prefix) { + parts[i] = strings.ReplaceAll(part, prefix, "") + changed = true + } + } + + if !changed { + return objectName + } + + return funcName + "(" + strings.Join(parts, ", ") + ")" +} + // normalizeTrigger normalizes trigger representation func normalizeTrigger(trigger *Trigger) { if trigger == nil { diff --git a/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/diff.sql b/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/diff.sql new file mode 100644 index 00000000..f0dc8437 --- /dev/null +++ b/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/diff.sql @@ -0,0 +1,3 @@ +REVOKE EXECUTE ON FUNCTION f_test(p_items my_input[]) FROM PUBLIC; + +GRANT EXECUTE ON FUNCTION f_test(p_items my_input[]) TO appname_apiuser; diff --git a/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/new.sql b/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/new.sql new file mode 100644 index 00000000..6ced1030 --- /dev/null +++ b/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/new.sql @@ -0,0 +1,20 @@ +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'appname_apiuser') THEN + CREATE ROLE appname_apiuser; + END IF; +END $$; + +CREATE TYPE my_input AS ( + id uuid +); + +CREATE FUNCTION f_test(p_items my_input[]) +RETURNS integer +LANGUAGE sql +AS $$ + SELECT COALESCE(array_length(p_items, 1), 0); +$$; + +REVOKE ALL ON FUNCTION f_test(my_input[]) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION f_test(my_input[]) TO appname_apiuser; diff --git a/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/old.sql b/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/old.sql new file mode 100644 index 00000000..ccf5756a --- /dev/null +++ b/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/old.sql @@ -0,0 +1,17 @@ +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'appname_apiuser') THEN + CREATE ROLE appname_apiuser; + END IF; +END $$; + +CREATE TYPE my_input AS ( + id uuid +); + +CREATE FUNCTION f_test(p_items my_input[]) +RETURNS integer +LANGUAGE sql +AS $$ + SELECT COALESCE(array_length(p_items, 1), 0); +$$; diff --git a/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/plan.json b/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/plan.json new file mode 100644 index 00000000..006a1470 --- /dev/null +++ b/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/plan.json @@ -0,0 +1,26 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.8.0", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "27f3398a052dfc62c099d3054981ea0e5493b954bff2ffd763222a4fc21de01c" + }, + "groups": [ + { + "steps": [ + { + "sql": "REVOKE EXECUTE ON FUNCTION f_test(p_items my_input[]) FROM PUBLIC;", + "type": "revoked_default_privilege", + "operation": "create", + "path": "revoked_default.FUNCTION.f_test(p_items my_input[])" + }, + { + "sql": "GRANT EXECUTE ON FUNCTION f_test(p_items my_input[]) TO appname_apiuser;", + "type": "privilege", + "operation": "create", + "path": "privileges.FUNCTION.f_test(p_items my_input[]).appname_apiuser" + } + ] + } + ] +} diff --git a/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/plan.sql b/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/plan.sql new file mode 100644 index 00000000..f0dc8437 --- /dev/null +++ b/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/plan.sql @@ -0,0 +1,3 @@ +REVOKE EXECUTE ON FUNCTION f_test(p_items my_input[]) FROM PUBLIC; + +GRANT EXECUTE ON FUNCTION f_test(p_items my_input[]) TO appname_apiuser; diff --git a/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/plan.txt b/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/plan.txt new file mode 100644 index 00000000..7b4c96e2 --- /dev/null +++ b/testdata/diff/privilege/issue_376_grant_revoke_temp_schema_leak/plan.txt @@ -0,0 +1,18 @@ +Plan: 2 to add. + +Summary by type: + privileges: 1 to add + revoked default privileges: 1 to add + +Privileges: + + appname_apiuser + +Revoked default privileges: + + f_test(p_items my_input[]) + +DDL to be executed: +-------------------------------------------------- + +REVOKE EXECUTE ON FUNCTION f_test(p_items my_input[]) FROM PUBLIC; + +GRANT EXECUTE ON FUNCTION f_test(p_items my_input[]) TO appname_apiuser; From 5d37c9273b123da3b6c5944781afe26fd0f1ba41 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Fri, 27 Mar 2026 04:21:17 -0700 Subject: [PATCH 2/2] refactor: use splitTableColumns and add unit tests for normalizePrivilegeObjectName Address review feedback: - Use splitTableColumns helper for arg splitting to correctly handle nested parentheses in type modifiers (e.g., numeric(10, 2)) - Add table-driven unit tests covering edge cases: empty inputs, no-arg functions, array types, multi-arg, temp schema prefix, precision types Co-Authored-By: Claude Opus 4.6 (1M context) --- ir/normalize.go | 12 ++++--- ir/normalize_test.go | 79 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/ir/normalize.go b/ir/normalize.go index 612fac89..42d8820d 100644 --- a/ir/normalize.go +++ b/ir/normalize.go @@ -670,13 +670,17 @@ func normalizePrivilegeObjectName(objectName, schemaName string) string { prefix := schemaName + "." - // Split args by comma, strip schema prefix from each type reference, rejoin - parts := strings.Split(args, ", ") + // Use splitTableColumns to correctly handle nested parentheses in type modifiers + // (e.g., numeric(10, 2)) — consistent with other normalization helpers in this file + parts := splitTableColumns(args) changed := false for i, part := range parts { - if strings.Contains(part, prefix) { - parts[i] = strings.ReplaceAll(part, prefix, "") + trimmed := strings.TrimSpace(part) + if strings.Contains(trimmed, prefix) { + parts[i] = strings.ReplaceAll(trimmed, prefix, "") changed = true + } else { + parts[i] = trimmed } } diff --git a/ir/normalize_test.go b/ir/normalize_test.go index f7510387..73ee9f2e 100644 --- a/ir/normalize_test.go +++ b/ir/normalize_test.go @@ -390,6 +390,85 @@ func TestNormalizePolicyExpression(t *testing.T) { } } +func TestNormalizePrivilegeObjectName(t *testing.T) { + tests := []struct { + name string + objectName string + schemaName string + expected string + }{ + { + name: "empty object name", + objectName: "", + schemaName: "public", + expected: "", + }, + { + name: "empty schema name", + objectName: "f_test(p_items my_input[])", + schemaName: "", + expected: "f_test(p_items my_input[])", + }, + { + name: "no args", + objectName: "f_test()", + schemaName: "public", + expected: "f_test()", + }, + { + name: "no schema prefix in args", + objectName: "f_test(p_items my_input[])", + schemaName: "public", + expected: "f_test(p_items my_input[])", + }, + { + name: "temp schema prefix in array type", + objectName: "f_test(p_items pgschema_tmp_20260326_161823_31f3dbda.my_input[])", + schemaName: "pgschema_tmp_20260326_161823_31f3dbda", + expected: "f_test(p_items my_input[])", + }, + { + name: "public schema prefix", + objectName: "f_test(p_items public.my_input[])", + schemaName: "public", + expected: "f_test(p_items my_input[])", + }, + { + name: "multiple args with schema prefix", + objectName: "f_test(a public.my_type, b integer, c public.other_type[])", + schemaName: "public", + expected: "f_test(a my_type, b integer, c other_type[])", + }, + { + name: "no parentheses", + objectName: "my_table", + schemaName: "public", + expected: "my_table", + }, + { + name: "builtin types unchanged", + objectName: "calculate_total(quantity integer, unit_price numeric)", + schemaName: "public", + expected: "calculate_total(quantity integer, unit_price numeric)", + }, + { + name: "type with precision in parens", + objectName: "f_test(a public.my_type, b numeric(10, 2))", + schemaName: "public", + expected: "f_test(a my_type, b numeric(10, 2))", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := normalizePrivilegeObjectName(tt.objectName, tt.schemaName) + if result != tt.expected { + t.Errorf("normalizePrivilegeObjectName(%q, %q) = %q, want %q", tt.objectName, tt.schemaName, result, tt.expected) + } + }) + } +} + func TestNormalizeCheckClause(t *testing.T) { tests := []struct { name string