Skip to content

Commit 079bde8

Browse files
tianzhouclaude
tianzhou
and
claude
authored
fix: preserve nested function calls in RLS policy expressions (#377) (#378)
normalizeExpressionParentheses had a regex that incorrectly matched nested function calls like unnest(get_user_assigned_projects()) as redundantly-wrapped single function calls, stripping the outer parens and merging tokens into invalid SQL (unnestget_user_assigned_projects()). Remove the broken regex. The parenthesized form that pg_get_expr returns (e.g., (current_setting(...))::integer) is valid SQL and needs no unwrapping. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fc218b5 commit 079bde8

18 files changed

Lines changed: 149 additions & 36 deletions

File tree

‎ir/normalize.go‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -819,28 +819,7 @@ func normalizeExpressionParentheses(expr string) string {
819819
expr = fmt.Sprintf("(%s)", expr)
820820
}
821821

822-
// Step 2: Remove unnecessary parentheses around function calls within the expression
823-
// Specifically targets patterns like (function_name(...)) -> function_name(...)
824-
// This pattern looks for:
825-
// \( - opening parenthesis
826-
// ([a-zA-Z_][a-zA-Z0-9_]*) - function name (captured)
827-
// \( - opening parenthesis for function call
828-
// ([^)]*) - function arguments (captured, non-greedy to avoid matching nested parens)
829-
// \) - closing parenthesis for function call
830-
// \) - closing parenthesis around the whole function
831-
functionParensRegex := regexp.MustCompile(`\(([a-zA-Z_][a-zA-Z0-9_]*\([^)]*\))\)`)
832-
833-
// Replace (function(...)) with function(...)
834-
// Keep applying until no more matches to handle nested cases
835-
for {
836-
original := expr
837-
expr = functionParensRegex.ReplaceAllString(expr, "$1")
838-
if expr == original {
839-
break
840-
}
841-
}
842-
843-
// Step 3: Normalize redundant type casts in function arguments
822+
// Step 2: Normalize redundant type casts in function arguments
844823
// Pattern: 'text'::text -> 'text' (removing redundant text cast from literals)
845824
// IMPORTANT: Do NOT match when followed by [] (array cast is semantically significant)
846825
// e.g., '{nested,key}'::text[] must be preserved as-is

‎ir/normalize_test.go‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,75 @@ func TestStripSchemaFromReturnType(t *testing.T) {
321321
}
322322
}
323323

324+
func TestNormalizeExpressionParentheses(t *testing.T) {
325+
tests := []struct {
326+
name string
327+
input string
328+
expected string
329+
}{
330+
{
331+
name: "simple expression gets wrapped",
332+
input: "tenant_id = 1",
333+
expected: "(tenant_id = 1)",
334+
},
335+
{
336+
name: "already parenthesized",
337+
input: "(tenant_id = 1)",
338+
expected: "(tenant_id = 1)",
339+
},
340+
{
341+
name: "nested function call preserved",
342+
input: "(id IN ( SELECT unnest(get_user_assigned_projects()) AS unnest))",
343+
expected: "(id IN ( SELECT unnest(get_user_assigned_projects()) AS unnest))",
344+
},
345+
{
346+
name: "simple function call preserved",
347+
input: "(has_scope('admin'))",
348+
expected: "(has_scope('admin'))",
349+
},
350+
}
351+
352+
for _, tt := range tests {
353+
t.Run(tt.name, func(t *testing.T) {
354+
result := normalizeExpressionParentheses(tt.input)
355+
if result != tt.expected {
356+
t.Errorf("normalizeExpressionParentheses(%q) = %q, want %q", tt.input, result, tt.expected)
357+
}
358+
})
359+
}
360+
}
361+
362+
func TestNormalizePolicyExpression(t *testing.T) {
363+
tests := []struct {
364+
name string
365+
expr string
366+
tableSchema string
367+
expected string
368+
}{
369+
{
370+
name: "nested function call in policy expression",
371+
expr: "(id IN ( SELECT unnest(get_user_assigned_projects()) AS unnest))",
372+
tableSchema: "public",
373+
expected: "(id IN ( SELECT unnest(get_user_assigned_projects()) AS unnest))",
374+
},
375+
{
376+
name: "schema-qualified nested function call",
377+
expr: "(id IN ( SELECT unnest(public.get_user_assigned_projects()) AS unnest))",
378+
tableSchema: "public",
379+
expected: "(id IN ( SELECT unnest(get_user_assigned_projects()) AS unnest))",
380+
},
381+
}
382+
383+
for _, tt := range tests {
384+
t.Run(tt.name, func(t *testing.T) {
385+
result := normalizePolicyExpression(tt.expr, tt.tableSchema)
386+
if result != tt.expected {
387+
t.Errorf("normalizePolicyExpression(%q, %q) = %q, want %q", tt.expr, tt.tableSchema, result, tt.expected)
388+
}
389+
})
390+
}
391+
}
392+
324393
func TestNormalizeCheckClause(t *testing.T) {
325394
tests := []struct {
326395
name string

‎testdata/diff/create_policy/add_policy/diff.sql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
ALTER TABLE orders ENABLE ROW LEVEL SECURITY;
22
CREATE POLICY orders_user_access ON orders FOR SELECT TO PUBLIC USING (user_id IN ( SELECT users.id FROM users));
3-
CREATE POLICY "UserPolicy" ON users TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer);
3+
CREATE POLICY "UserPolicy" ON users TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer);
44
CREATE POLICY admin_only ON users FOR DELETE TO PUBLIC USING (is_admin());
55
CREATE POLICY "my-policy" ON users FOR INSERT TO PUBLIC WITH CHECK ((role)::text = 'user');
66
CREATE POLICY "select" ON users FOR SELECT TO PUBLIC USING (true);
7-
CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer);
7+
CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer);

‎testdata/diff/create_policy/add_policy/plan.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"path": "public.orders.orders_user_access"
2222
},
2323
{
24-
"sql": "CREATE POLICY \"UserPolicy\" ON users TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer);",
24+
"sql": "CREATE POLICY \"UserPolicy\" ON users TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer);",
2525
"type": "table.policy",
2626
"operation": "create",
2727
"path": "public.users.UserPolicy"
@@ -45,7 +45,7 @@
4545
"path": "public.users.select"
4646
},
4747
{
48-
"sql": "CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer);",
48+
"sql": "CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer);",
4949
"type": "table.policy",
5050
"operation": "create",
5151
"path": "public.users.user_tenant_isolation"

‎testdata/diff/create_policy/add_policy/plan.sql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ ALTER TABLE orders ENABLE ROW LEVEL SECURITY;
22

33
CREATE POLICY orders_user_access ON orders FOR SELECT TO PUBLIC USING (user_id IN ( SELECT users.id FROM users));
44

5-
CREATE POLICY "UserPolicy" ON users TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer);
5+
CREATE POLICY "UserPolicy" ON users TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer);
66

77
CREATE POLICY admin_only ON users FOR DELETE TO PUBLIC USING (is_admin());
88

99
CREATE POLICY "my-policy" ON users FOR INSERT TO PUBLIC WITH CHECK ((role)::text = 'user');
1010

1111
CREATE POLICY "select" ON users FOR SELECT TO PUBLIC USING (true);
1212

13-
CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer);
13+
CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer);

‎testdata/diff/create_policy/add_policy/plan.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ ALTER TABLE orders ENABLE ROW LEVEL SECURITY;
2121

2222
CREATE POLICY orders_user_access ON orders FOR SELECT TO PUBLIC USING (user_id IN ( SELECT users.id FROM users));
2323

24-
CREATE POLICY "UserPolicy" ON users TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer);
24+
CREATE POLICY "UserPolicy" ON users TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer);
2525

2626
CREATE POLICY admin_only ON users FOR DELETE TO PUBLIC USING (is_admin());
2727

2828
CREATE POLICY "my-policy" ON users FOR INSERT TO PUBLIC WITH CHECK ((role)::text = 'user');
2929

3030
CREATE POLICY "select" ON users FOR SELECT TO PUBLIC USING (true);
3131

32-
CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer);
32+
CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer);

‎testdata/diff/create_policy/alter_policy_roles/plan.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"pgschema_version": "1.8.0",
44
"created_at": "1970-01-01T00:00:00Z",
55
"source_fingerprint": {
6-
"hash": "a24989791c9ba13d2d21f70cd0405e0e509c55341872b552ab87a0c79d238ddd"
6+
"hash": "a5fcd005530df97b095b020e1a8c2fb12333baf056c3a2318bc34f2157893877"
77
},
88
"groups": [
99
{

‎testdata/diff/create_policy/alter_policy_using/plan.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"pgschema_version": "1.8.0",
44
"created_at": "1970-01-01T00:00:00Z",
55
"source_fingerprint": {
6-
"hash": "e7aac12e5d350ee9d014b756c838017ddd0c17180b7baa33300f4a773c4c89b1"
6+
"hash": "b23d70f93fe88b9bfcf8d05f46e3faa60d54cf469813e2ead871e9c9d9231ab7"
77
},
88
"groups": [
99
{

‎testdata/diff/create_policy/disable_rls/plan.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"pgschema_version": "1.8.0",
44
"created_at": "1970-01-01T00:00:00Z",
55
"source_fingerprint": {
6-
"hash": "a24989791c9ba13d2d21f70cd0405e0e509c55341872b552ab87a0c79d238ddd"
6+
"hash": "a5fcd005530df97b095b020e1a8c2fb12333baf056c3a2318bc34f2157893877"
77
},
88
"groups": [
99
{

‎testdata/diff/create_policy/drop_policy/plan.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"pgschema_version": "1.8.0",
44
"created_at": "1970-01-01T00:00:00Z",
55
"source_fingerprint": {
6-
"hash": "a24989791c9ba13d2d21f70cd0405e0e509c55341872b552ab87a0c79d238ddd"
6+
"hash": "a5fcd005530df97b095b020e1a8c2fb12333baf056c3a2318bc34f2157893877"
77
},
88
"groups": [
99
{

0 commit comments

Comments
 (0)