fix: escape policy role names and function/procedure names in generated SQL

[jtayal-stripe]

Summary

Fixes two additional SQL injection sinks identified via the same root cause as #295 (enum labels). Schema-derived values were re-emitted into generated DDL without proper escaping.

1. Policy role names (policy_sql_generator.go)

AppliesTo role names (sourced from pg_roles.rolname) were interpolated raw into CREATE POLICY ... TO and ALTER POLICY ... TO statements. A user with CREATEROLE privilege could plant a role whose name contains an embedded double-quote to inject arbitrary SQL during plan execution.

Fix: Added escapeRoleNames() helper that applies EscapeIdentifier to each role name, preserving PUBLIC as an unquoted SQL keyword (matching the existing pattern in privilege_sql_generator.go).

2. Function/procedure names (schema.go buildProcName)

The function used hand-rolled quoting (fmt.Sprintf("\"%s\"(%s)", name, ...)) that did not double embedded double-quotes. A user with CREATE FUNCTION privilege could create a function with " in its name to inject SQL when DROP FUNCTION/DROP PROCEDURE statements are generated.

Fix: Replaced the hand-rolled quoting with EscapeIdentifier(name).

Evidence of vulnerability without fix

buildProcName (function names)

When the fix is reverted, buildProcName produces broken/injectable identifiers:

=== RUN   TestBuildProcName/function_name_with_embedded_double_quote
    expected: "evil""func"(integer)
    actual  : "evil"func"(integer)      <-- unescaped, invalid SQL

=== RUN   TestBuildProcName/injection_attempt_via_double_quote
    expected: "x""; DROP TABLE foo; --"()
    actual  : "x"; DROP TABLE foo; --"()   <-- SQL injection!

Policy role names

When the fix is reverted, role names are emitted raw:

=== RUN   TestPolicySQLGenerator_Add_EscapesRoleNames/regular_role_quoted
    TO my_role           <-- unescaped (should be TO "my_role")

=== RUN   TestPolicySQLGenerator_Add_EscapesRoleNames/role_with_embedded_double_quote
    TO evil"role         <-- SQL injection! (should be TO "evil""role")

Evidence of all acceptance tests passing with fix

All 50 policy and function acceptance tests pass against a live PostgreSQL 14 instance (Docker, with -race):

--- PASS: TestPolicyCases (0.01s)
    --- PASS: TestPolicyCases/Alter_policy_check (3.53s)
    --- PASS: TestPolicyCases/Add_policy_on_existing_partition_(not_implemented) (1.15s)
    --- PASS: TestPolicyCases/Policy_on_new_partition_(not_implemented) (1.21s)
    --- PASS: TestPolicyCases/no-op (5.42s)
    --- PASS: TestPolicyCases/Create_policy_with_PUBLIC_keyword_unquoted (4.14s)
    --- PASS: TestPolicyCases/Alter_policy_(table_is_re-created) (3.27s)
    --- PASS: TestPolicyCases/Remove_check_for_ALL_policy (3.31s)
    --- PASS: TestPolicyCases/Remove_using_check_for_ALL_policy (3.30s)
    --- PASS: TestPolicyCases/Alter_policy_that_references_deleted_columns_and_new_columns_(non-public_schema) (3.31s)
    --- PASS: TestPolicyCases/Re-create_policy_that_references_deleted_columns_and_new_columns_(non-public_schema) (3.33s)
    --- PASS: TestPolicyCases/Alter_policy_using (2.87s)
    --- PASS: TestPolicyCases/Add_policy_on_new_table (3.45s)
    --- PASS: TestPolicyCases/Alter_policy_with_role_containing_SQL_injection_payload (5.89s)
    --- PASS: TestPolicyCases/Create_policy_with_role_containing_double_quote (6.02s)
    --- PASS: TestPolicyCases/Alter_all_alterable_attributes_on_non-public_schema (6.02s)
    --- PASS: TestPolicyCases/Alter_policy_target (3.71s)
    --- PASS: TestPolicyCases/Drop_policy_and_columns (3.63s)
    --- PASS: TestPolicyCases/Alter_policy_applies_to (5.84s)
    --- PASS: TestPolicyCases/Disable_RLS_then_drop_policy (3.90s)
    --- PASS: TestPolicyCases/Drop_policy_and_table (3.90s)
    --- PASS: TestPolicyCases/Drop_non-public_schema_policy (3.91s)
    --- PASS: TestPolicyCases/Add_policy_then_enable_RLS (3.91s)
    --- PASS: TestPolicyCases/Create_INSERT_policy (3.83s)
    --- PASS: TestPolicyCases/Create_DELETE_policy (3.15s)
    --- PASS: TestPolicyCases/Restrictive_to_permissive_policy (3.04s)
    --- PASS: TestPolicyCases/Create_SELECT_policy (2.77s)
    --- PASS: TestPolicyCases/Create_UPDATE_policy (2.49s)
    --- PASS: TestPolicyCases/Add_permissive_ALL_policy_target_on_non-public_schema (3.71s)
    --- PASS: TestPolicyCases/Create_INSERT_policy (4.13s)
    --- PASS: TestPolicyCases/Create_UPDATE_policy (4.14s)
--- PASS: TestFunctionTestCases (0.01s)
    --- PASS: TestFunctionTestCases/No-op (4.05s)
    --- PASS: TestFunctionTestCases/Alter_functions_(with_conflicting_names) (4.21s)
    --- PASS: TestFunctionTestCases/Drop_function_with_name_containing_double_quote (4.03s)
    --- PASS: TestFunctionTestCases/Create_function_with_name_containing_double_quote (3.99s)
    --- PASS: TestFunctionTestCases/Alter_a_dependent_function (3.98s)
    --- PASS: TestFunctionTestCases/Create_function_with_SQL_injection_in_name (3.98s)
    --- PASS: TestFunctionTestCases/Alter_a_function_to_no_longer_depend_on_a_function_and_drop_that_function (3.16s)
    --- PASS: TestFunctionTestCases/Alter_non-sql_function_to_be_sql_function_(no_dependency_tracking_error) (3.49s)
    --- PASS: TestFunctionTestCases/Alter_a_function's_dependencies (3.98s)
    --- PASS: TestFunctionTestCases/Alter_non-sql_function (3.53s)
    --- PASS: TestFunctionTestCases/Alter_functions_with_quoted_names_(with_conflicting_names) (3.55s)
    --- PASS: TestFunctionTestCases/Drop_non-sql_function (3.52s)
    --- PASS: TestFunctionTestCases/Create_functions_with_quoted_names_(with_conflicting_names) (3.54s)
    --- PASS: TestFunctionTestCases/Create_non-sql_function (3.58s)
    --- PASS: TestFunctionTestCases/Drop_function_with_dependencies (3.59s)
    --- PASS: TestFunctionTestCases/Alter_sql_function_to_be_non-sql_function (3.61s)
    --- PASS: TestFunctionTestCases/Drop_functions_with_quoted_names_(with_conflicting_names) (4.01s)
    --- PASS: TestFunctionTestCases/Drop_functions_(with_conflicting_names) (4.02s)
    --- PASS: TestFunctionTestCases/Create_function_with_an_extension_that_also_creates_functions_installed (3.07s)
    --- PASS: TestFunctionTestCases/Create_function_with_dependencies (3.87s)
    --- PASS: TestFunctionTestCases/Add_and_drop_functions_with_dependencies_(conflicting_schemas) (3.98s)
    --- PASS: TestFunctionTestCases/Create_functions_(with_conflicting_names) (3.04s)
ok  	github.com/stripe/pg-schema-diff/internal/migration_acceptance_tests	19.901s

Test plan

• Unit tests for buildProcName with normal, double-quote, and injection payloads
• Unit tests for escapeRoleNames including PUBLIC keyword handling
• Unit tests for policySQLGenerator.Add verifying escaped role output
• Acceptance tests for policy creation/alteration with double-quote roles and injection payloads (live PostgreSQL)
• Acceptance tests for function create/drop with double-quote names and injection payloads (live PostgreSQL)
• All existing policy and function acceptance tests pass (no regressions)
• Verified test failures when fix is reverted (evidence above)

[alexaub-stripe]

Looks great! One minor question