On Tue, Oct 28, 2025 at 11:06 AM Chao Li <[email protected]> wrote: > > The attached patch did what the $subject says. > > demo: > > > > begin; > > create role alice login; > > grant all on schema public to alice; > > drop table if exists tts; > > create table tts(a int); > > grant insert on tts to alice; > > ALTER TABLE tts ENABLE ROW LEVEL SECURITY; > > CREATE POLICY p1 ON tts FOR ALL USING (a = 1 or a = 2 or a = 3); > > commit; > > > > SET ROLE alice; > > insert into tts values (4); --error > > > > old ERROR message: > > ERROR: new row violates row-level security policy for table "tts" > > > > new ERROR message: > > ERROR: new row violates row-level security policy "p1" for table "tts" > > > > There are fewer than 10 lines of C code changes, but turns out that in the > > regression tests, there are many cases where only one permissive policy > > exists > > for INSERT or UPDATE. > > So the patch is not smaller. > > <v1-0001-minor-RLS-violation-error-report-enhance.patch> > > I agree printing policy name to the log helps. I tried to “make" and “make > check”, all passed.
https://cirrus-ci.com/task/5006265459408896?logs=test_world#L145 says test_rls_hooks test failed. > > A tiny comment wrt the code comment: > > ``` > * since if the check fails it means that no policy granted > permission > * to perform the update, rather than any particular policy > being > * violated. > + * However, if there is only a single permissive policy > clause, we can > + * include that specific policy name in error reports when > the policy is > + * violated. > ``` > > * “However …” doesn’t have to go to a new line. But if you really want that, > an empty comment line should be added above “However …”. See the comment of > “if” that is right above this piece of code. > > * “include that specific policy name” => “include that specific policy’s > name”. > ok. now the comment is * However, if there is only a single permissive policy clause, we can * include that specific policy’s name in error reports when the policy * is violated.
From ea2e164414c2bdad14c437e072c387e453158ce3 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Fri, 31 Oct 2025 19:51:12 +0800 Subject: [PATCH v2 1/1] minor RLS violation error report enhance if only one permissive policy exists then error report also include that RLS policy name. discussion: https://postgr.es/m/cacjufxehb0gqeckugre5usnq-q2cyawoyflhvp9qpzh7pbx...@mail.gmail.com --- src/backend/rewrite/rowsecurity.c | 9 +++ .../expected/test_rls_hooks.out | 12 ++-- src/test/regress/expected/merge.out | 4 +- src/test/regress/expected/rowsecurity.out | 58 +++++++++---------- src/test/regress/expected/update.out | 4 +- 5 files changed, 48 insertions(+), 39 deletions(-) diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c index 4dad384d04d..576da767b3b 100644 --- a/src/backend/rewrite/rowsecurity.c +++ b/src/backend/rewrite/rowsecurity.c @@ -841,6 +841,10 @@ add_with_check_options(Relation rel, * since if the check fails it means that no policy granted permission * to perform the update, rather than any particular policy being * violated. + * + * However, if there is only a single permissive policy clause, we can + * include that specific policy’s name in error reports when the policy + * is violated. */ WithCheckOption *wco; @@ -851,7 +855,12 @@ add_with_check_options(Relation rel, wco->cascaded = false; if (list_length(permissive_quals) == 1) + { + RowSecurityPolicy *policy = (RowSecurityPolicy *) linitial(permissive_policies); + wco->qual = (Node *) linitial(permissive_quals); + wco->polname = pstrdup(policy->policy_name); + } else wco->qual = (Node *) makeBoolExpr(OR_EXPR, permissive_quals, -1); diff --git a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out index b8c6d385814..b374e5adab4 100644 --- a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out +++ b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out @@ -62,7 +62,7 @@ SELECT * FROM rls_test_permissive; INSERT INTO rls_test_permissive VALUES ('regress_r1','regress_s1',10); -- failure INSERT INTO rls_test_permissive VALUES ('regress_r4','regress_s4',10); -ERROR: new row violates row-level security policy for table "rls_test_permissive" +ERROR: new row violates row-level security policy "extension policy" for table "rls_test_permissive" SET ROLE regress_s1; -- With only the hook's policies, restrictive -- hook's policy is current_user = supervisor @@ -103,13 +103,13 @@ SELECT * FROM rls_test_both; -- failure INSERT INTO rls_test_both VALUES ('regress_r1','regress_s1',10); -ERROR: new row violates row-level security policy for table "rls_test_both" +ERROR: new row violates row-level security policy "extension policy" for table "rls_test_both" -- failure INSERT INTO rls_test_both VALUES ('regress_r4','regress_s1',10); -ERROR: new row violates row-level security policy for table "rls_test_both" +ERROR: new row violates row-level security policy "extension policy" for table "rls_test_both" -- failure INSERT INTO rls_test_both VALUES ('regress_r4','regress_s4',10); -ERROR: new row violates row-level security policy for table "rls_test_both" +ERROR: new row violates row-level security policy "extension policy" for table "rls_test_both" RESET ROLE; -- Create "internal" policies, to check that the policies from -- the hooks are combined correctly. @@ -164,10 +164,10 @@ INSERT INTO rls_test_restrictive VALUES ('regress_r3','regress_s3',10); ERROR: new row violates row-level security policy "extension policy" for table "rls_test_restrictive" -- failure INSERT INTO rls_test_restrictive VALUES ('regress_r1','regress_s1',7); -ERROR: new row violates row-level security policy for table "rls_test_restrictive" +ERROR: new row violates row-level security policy "p1" for table "rls_test_restrictive" -- failure INSERT INTO rls_test_restrictive VALUES ('regress_r4','regress_s4',7); -ERROR: new row violates row-level security policy for table "rls_test_restrictive" +ERROR: new row violates row-level security policy "p1" for table "rls_test_restrictive" -- With both internal and hook policies, both permissive -- and restrictive hook policies EXPLAIN (costs off) SELECT * FROM rls_test_both; diff --git a/src/test/regress/expected/merge.out b/src/test/regress/expected/merge.out index 9cb1d87066a..f84ad47a435 100644 --- a/src/test/regress/expected/merge.out +++ b/src/test/regress/expected/merge.out @@ -2306,7 +2306,7 @@ MERGE INTO pa_target t ON t.tid = s.sid AND t.tid IN (1,2,3,4) WHEN MATCHED THEN UPDATE SET tid = tid - 1; -ERROR: new row violates row-level security policy for table "pa_target" +ERROR: new row violates row-level security policy "pa_target_pol" for table "pa_target" ROLLBACK; DROP TABLE pa_source; DROP TABLE pa_target CASCADE; @@ -2747,7 +2747,7 @@ MERGE INTO measurement m WHEN NOT MATCHED THEN INSERT (city_id, logdate, peaktemp, unitsales) VALUES (city_id - 1, logdate, NULL, 100); -- should fail -ERROR: new row violates row-level security policy for table "measurement" +ERROR: new row violates row-level security policy "measurement_p" for table "measurement" MERGE INTO measurement m USING (VALUES (1, '01-17-2007'::date)) nm(city_id, logdate) ON (m.city_id = nm.city_id and m.logdate=nm.logdate) diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index c958ef4d70a..87b125a732d 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -743,9 +743,9 @@ SELECT * FROM document WHERE did = 8; -- and confirm we can't see it -- RLS policies are checked before constraints INSERT INTO document VALUES (8, 44, 1, 'regress_rls_carol', 'my third manga'); -- Should fail with RLS check violation, not duplicate key violation -ERROR: new row violates row-level security policy for table "document" +ERROR: new row violates row-level security policy "p1" for table "document" UPDATE document SET did = 8, dauthor = 'regress_rls_carol' WHERE did = 5; -- Should fail with RLS check violation, not duplicate key violation -ERROR: new row violates row-level security policy for table "document" +ERROR: new row violates row-level security policy "p1" for table "document" -- database superuser does bypass RLS policy when enabled RESET SESSION AUTHORIZATION; SET row_security TO ON; @@ -1350,7 +1350,7 @@ EXPLAIN (COSTS OFF) SELECT * FROM part_document WHERE f_leak(dtitle); -- pp1 ERROR INSERT INTO part_document VALUES (100, 11, 5, 'regress_rls_dave', 'testing pp1'); -- fail -ERROR: new row violates row-level security policy for table "part_document" +ERROR: new row violates row-level security policy "pp1" for table "part_document" -- pp1r ERROR INSERT INTO part_document VALUES (100, 99, 1, 'regress_rls_dave', 'testing pp1r'); -- fail ERROR: new row violates row-level security policy "pp1r" for table "part_document" @@ -2209,9 +2209,9 @@ NOTICE: f_leak => 4a44dc15364204a80fe80e9039455cc1 (5 rows) INSERT INTO bv1 VALUES (-1, 'xxx'); -- should fail view WCO -ERROR: new row violates row-level security policy for table "b1" +ERROR: new row violates row-level security policy "p1" for table "b1" INSERT INTO bv1 VALUES (11, 'xxx'); -- should fail RLS check -ERROR: new row violates row-level security policy for table "b1" +ERROR: new row violates row-level security policy "p1" for table "b1" INSERT INTO bv1 VALUES (12, 'xxx'); -- ok EXPLAIN (COSTS OFF) UPDATE bv1 SET b = 'yyy' WHERE a = 4 AND f_leak(b); QUERY PLAN @@ -2283,7 +2283,7 @@ SELECT * FROM document WHERE did = 2; -- alternative UPDATE path happens to be taken): INSERT INTO document VALUES (2, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_carol', 'my first novel') ON CONFLICT (did) DO UPDATE SET dtitle = EXCLUDED.dtitle, dauthor = EXCLUDED.dauthor; -ERROR: new row violates row-level security policy for table "document" +ERROR: new row violates row-level security policy "p2" for table "document" -- Violates USING qual for UPDATE policy p3. -- -- UPDATE path is taken, but UPDATE fails purely because *existing* row to be @@ -2292,7 +2292,7 @@ ERROR: new row violates row-level security policy for table "document" INSERT INTO document VALUES (33, 22, 1, 'regress_rls_bob', 'okay science fiction'); -- preparation for next statement INSERT INTO document VALUES (33, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'Some novel, replaces sci-fi') -- takes UPDATE path ON CONFLICT (did) DO UPDATE SET dtitle = EXCLUDED.dtitle; -ERROR: new row violates row-level security policy (USING expression) for table "document" +ERROR: new row violates row-level security policy "p3" (USING expression) for table "document" -- Fine (we UPDATE, since INSERT WCOs and UPDATE security barrier quals + WCOs -- not violated): INSERT INTO document VALUES (2, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'my first novel') @@ -2323,7 +2323,7 @@ INSERT INTO document VALUES (78, (SELECT cid from category WHERE cname = 'novel' -- passing quals: INSERT INTO document VALUES (78, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'some technology novel') ON CONFLICT (did) DO UPDATE SET dtitle = EXCLUDED.dtitle, cid = 33 RETURNING *; -ERROR: new row violates row-level security policy (USING expression) for table "document" +ERROR: new row violates row-level security policy "p3" (USING expression) for table "document" -- Don't fail just because INSERT doesn't satisfy WITH CHECK option that -- originated as a barrier/USING() qual from the UPDATE. Note that the UPDATE -- path *isn't* taken, and so UPDATE-related policy does not apply: @@ -2340,7 +2340,7 @@ INSERT INTO document VALUES (79, (SELECT cid from category WHERE cname = 'techno -- irrelevant, in fact. INSERT INTO document VALUES (79, (SELECT cid from category WHERE cname = 'technology'), 1, 'regress_rls_bob', 'technology book, can only insert') ON CONFLICT (did) DO UPDATE SET dtitle = EXCLUDED.dtitle RETURNING *; -ERROR: new row violates row-level security policy (USING expression) for table "document" +ERROR: new row violates row-level security policy "p3" (USING expression) for table "document" -- Test default USING qual enforced as WCO SET SESSION AUTHORIZATION regress_rls_alice; DROP POLICY p1 ON document; @@ -2383,16 +2383,16 @@ SET SESSION AUTHORIZATION regress_rls_bob; -- Fails, since ALL WCO is enforced in insert path: INSERT INTO document VALUES (80, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_carol', 'my first novel') ON CONFLICT (did) DO UPDATE SET dtitle = EXCLUDED.dtitle, cid = 33; -ERROR: new row violates row-level security policy for table "document" +ERROR: new row violates row-level security policy "p3_with_all" for table "document" -- Fails, since ALL policy USING qual is enforced (existing, target tuple is in -- violation, since it has the "manga" cid): INSERT INTO document VALUES (4, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'my first novel') ON CONFLICT (did) DO UPDATE SET dtitle = EXCLUDED.dtitle; -ERROR: new row violates row-level security policy (USING expression) for table "document" +ERROR: new row violates row-level security policy "p3_with_all" (USING expression) for table "document" -- Fails, since ALL WCO are enforced: INSERT INTO document VALUES (1, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'my first novel') ON CONFLICT (did) DO UPDATE SET dauthor = 'regress_rls_carol'; -ERROR: new row violates row-level security policy for table "document" +ERROR: new row violates row-level security policy "p3_with_all" for table "document" -- -- MERGE -- @@ -2436,7 +2436,7 @@ USING (SELECT 1 as sdid) s ON did = s.sdid WHEN MATCHED THEN UPDATE SET dnotes = dnotes || ' notes added by merge1 ', dlevel = 0; -ERROR: new row violates row-level security policy for table "document" +ERROR: new row violates row-level security policy "p3" for table "document" -- Should be OK since USING and WITH CHECK quals pass MERGE INTO document d USING (SELECT 1 as sdid) s @@ -2456,7 +2456,7 @@ USING (SELECT 3 as sdid) s ON did = s.sdid WHEN MATCHED THEN UPDATE SET dnotes = dnotes || ' notes added by merge '; -ERROR: target row violates row-level security policy (USING expression) for table "document" +ERROR: target row violates row-level security policy "p3" (USING expression) for table "document" -- The same thing with DELETE action, but fails again because no permissions -- to delete items in 'science fiction' category that did 3 belongs to. MERGE INTO document d @@ -2464,7 +2464,7 @@ USING (SELECT 3 as sdid) s ON did = s.sdid WHEN MATCHED THEN DELETE; -ERROR: target row violates row-level security policy (USING expression) for table "document" +ERROR: target row violates row-level security policy "p4" (USING expression) for table "document" -- Document with did 4 belongs to 'manga' category which is allowed for -- deletion. But this fails because the UPDATE action is matched first and -- UPDATE policy does not allow updation in the category. @@ -2475,7 +2475,7 @@ WHEN MATCHED AND dnotes = '' THEN UPDATE SET dnotes = dnotes || ' notes added by merge ' WHEN MATCHED THEN DELETE; -ERROR: target row violates row-level security policy (USING expression) for table "document" +ERROR: target row violates row-level security policy "p3" (USING expression) for table "document" -- UPDATE action is not matched this time because of the WHEN qual. -- DELETE still fails because role regress_rls_bob does not have SELECT -- privileges on 'manga' category row in the category table. @@ -2486,7 +2486,7 @@ WHEN MATCHED AND dnotes <> '' THEN UPDATE SET dnotes = dnotes || ' notes added by merge ' WHEN MATCHED THEN DELETE; -ERROR: target row violates row-level security policy (USING expression) for table "document" +ERROR: target row violates row-level security policy "p4" (USING expression) for table "document" -- OK if DELETE is replaced with DO NOTHING MERGE INTO document d USING (SELECT 4 as sdid) s @@ -2525,7 +2525,7 @@ WHEN MATCHED THEN DELETE WHEN NOT MATCHED THEN INSERT VALUES (12, 11, 1, 'regress_rls_dave', 'another novel'); -ERROR: new row violates row-level security policy for table "document" +ERROR: new row violates row-level security policy "p2" for table "document" -- This should be fine MERGE INTO document d USING (SELECT 12 as sdid) s @@ -2566,7 +2566,7 @@ ON did = s.sdid WHEN MATCHED THEN UPDATE SET dnotes = dnotes || ' notes added by merge6 ', cid = (SELECT cid from category WHERE cname = 'technology'); -ERROR: new row violates row-level security policy for table "document" +ERROR: new row violates row-level security policy "p1" for table "document" -- but OK if new row is visible MERGE INTO document d USING (SELECT 1 as sdid) s @@ -2596,7 +2596,7 @@ WHEN MATCHED THEN WHEN NOT MATCHED THEN INSERT VALUES (14, 44, 1, 'regress_rls_bob', 'new manga') RETURNING *; -ERROR: new row violates row-level security policy for table "document" +ERROR: new row violates row-level security policy "p1" for table "document" -- but OK if new row is visible MERGE INTO document d USING (SELECT 14 as sdid) s @@ -3617,7 +3617,7 @@ WITH cte1 AS MATERIALIZED (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FROM cte1; (4 rows) WITH cte1 AS (UPDATE t1 SET a = a + 1 RETURNING *) SELECT * FROM cte1; --fail -ERROR: new row violates row-level security policy for table "t1" +ERROR: new row violates row-level security policy "p1" for table "t1" WITH cte1 AS (UPDATE t1 SET a = a RETURNING *) SELECT * FROM cte1; --ok a | b ----+---------------------------------- @@ -3635,7 +3635,7 @@ WITH cte1 AS (UPDATE t1 SET a = a RETURNING *) SELECT * FROM cte1; --ok (11 rows) WITH cte1 AS (INSERT INTO t1 VALUES (21, 'Fail') RETURNING *) SELECT * FROM cte1; --fail -ERROR: new row violates row-level security policy for table "t1" +ERROR: new row violates row-level security policy "p1" for table "t1" WITH cte1 AS (INSERT INTO t1 VALUES (20, 'Success') RETURNING *) SELECT * FROM cte1; --ok a | b ----+--------- @@ -4400,7 +4400,7 @@ SELECT * FROM r2; -- r2 is read-only INSERT INTO r2 VALUES (2); -- Not allowed -ERROR: new row violates row-level security policy for table "r2" +ERROR: new row violates row-level security policy "p2" for table "r2" UPDATE r2 SET a = 2 RETURNING *; -- Updates nothing a --- @@ -4468,7 +4468,7 @@ TABLE r1; -- RLS error INSERT INTO r1 VALUES (1); -ERROR: new row violates row-level security policy for table "r1" +ERROR: new row violates row-level security policy "p1" for table "r1" -- No error (unable to see any rows to update) UPDATE r1 SET a = 1; TABLE r1; @@ -4611,7 +4611,7 @@ HINT: To disable the policy for the table's owner, use ALTER TABLE NO FORCE ROW SET row_security = on; -- Error INSERT INTO r1 VALUES (10), (20) RETURNING *; -ERROR: new row violates row-level security policy for table "r1" +ERROR: new row violates row-level security policy "p1" for table "r1" DROP TABLE r1; -- -- Test UPDATE+RETURNING applies SELECT policies as @@ -4648,19 +4648,19 @@ TABLE r1; ALTER TABLE r1 FORCE ROW LEVEL SECURITY; -- Error UPDATE r1 SET a = 30 RETURNING *; -ERROR: new row violates row-level security policy for table "r1" +ERROR: new row violates row-level security policy "p1" for table "r1" -- UPDATE path of INSERT ... ON CONFLICT DO UPDATE should also error out INSERT INTO r1 VALUES (10) ON CONFLICT (a) DO UPDATE SET a = 30 RETURNING *; -ERROR: new row violates row-level security policy for table "r1" +ERROR: new row violates row-level security policy "p1" for table "r1" -- Should still error out without RETURNING (use of arbiter always requires -- SELECT permissions) INSERT INTO r1 VALUES (10) ON CONFLICT (a) DO UPDATE SET a = 30; -ERROR: new row violates row-level security policy for table "r1" +ERROR: new row violates row-level security policy "p1" for table "r1" INSERT INTO r1 VALUES (10) ON CONFLICT ON CONSTRAINT r1_pkey DO UPDATE SET a = 30; -ERROR: new row violates row-level security policy for table "r1" +ERROR: new row violates row-level security policy "p1" for table "r1" DROP TABLE r1; -- -- Test policies using virtual generated columns diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out index eef2bac1cbf..c7f4e2d3229 100644 --- a/src/test/regress/expected/update.out +++ b/src/test/regress/expected/update.out @@ -617,7 +617,7 @@ SET SESSION AUTHORIZATION regress_range_parted_user; -- This should fail with RLS violation error while moving row from -- part_a_10_a_20 to part_d_1_15, because we are setting 'c' to an odd number. UPDATE range_parted set a = 'b', c = 151 WHERE a = 'a' and c = 200; -ERROR: new row violates row-level security policy for table "range_parted" +ERROR: new row violates row-level security policy "policy_range_parted" for table "range_parted" RESET SESSION AUTHORIZATION; -- Create a trigger on part_d_1_15 CREATE FUNCTION func_d_1_15() RETURNS trigger AS $$ @@ -640,7 +640,7 @@ SET SESSION AUTHORIZATION regress_range_parted_user; -- 'c' to an even number, the trigger at the destination partition again makes -- it an odd number. UPDATE range_parted set a = 'b', c = 150 WHERE a = 'a' and c = 200; -ERROR: new row violates row-level security policy for table "range_parted" +ERROR: new row violates row-level security policy "policy_range_parted" for table "range_parted" -- Cleanup RESET SESSION AUTHORIZATION; DROP TRIGGER trig_d_1_15 ON part_d_1_15; -- 2.34.1
