From a14e804c2f820f9aab5d685c1f183e8ad4327d10 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Tue, 30 Jun 2026 15:22:29 +0800 Subject: [PATCH v4] Fix RLS checks for FOR PORTION OF leftover rows UPDATE/DELETE FOR PORTION OF may insert leftover rows to preserve the parts of the old row that are outside the target range. Those inserts go through ExecInsert(), which checks RLS policies using WCO_RLS_INSERT_CHECK. However, the rewriter only added RLS WITH CHECK options for the original statement command. For UPDATE, that meant only WCO_RLS_UPDATE_CHECK options were available, so ExecInsert() skipped them. For DELETE, no RLS WITH CHECK options were added at all. As a result, leftover rows could be inserted even when they violated INSERT RLS policies. Fix this by adding INSERT RLS WITH CHECK options for UPDATE/DELETE FOR PORTION OF target relations. Also add regression coverage for both UPDATE and DELETE, including cases where allowed leftovers still succeed and disallowed leftovers are rejected. Author: Chao Li Reviewed-by: Paul A Jungwirth Reviewed-by: Ayush Tiwari Discussion: https://postgr.es/m/6C34A987-AC50-4477-BD71-2D4AFEE1A589@gmail.com Discussion: https://postgr.es/m/CAJTYsWWdeBkoH5g8D-k9LDw9ciqsMxb21EJSiFXAzP4J=XyxOQ@mail.gmail.com --- doc/src/sgml/ref/create_policy.sgml | 34 +++++++++ doc/src/sgml/ref/delete.sgml | 3 +- doc/src/sgml/ref/update.sgml | 3 +- src/backend/rewrite/rowsecurity.c | 45 ++++++++++++ src/test/regress/expected/for_portion_of.out | 75 ++++++++++++++++++++ src/test/regress/expected/rowsecurity.out | 44 +++++++++++- src/test/regress/sql/for_portion_of.sql | 58 +++++++++++++++ src/test/regress/sql/rowsecurity.sql | 35 ++++++++- 8 files changed, 293 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ref/create_policy.sgml b/doc/src/sgml/ref/create_policy.sgml index d8a036739c0..2dff11d47d2 100644 --- a/doc/src/sgml/ref/create_policy.sgml +++ b/doc/src/sgml/ref/create_policy.sgml @@ -530,6 +530,27 @@ CREATE POLICY name ON Check new row — + + UPDATE ... FOR PORTION OF + + Filter existing row  & + check new row  + + + Check new row  + + FOR PORTION OF re-inserts the portions of the + affected row that fall outside the targeted range, to preserve them. + These leftover rows are checked against the INSERT + policy's WITH CHECK expression, even though no + INSERT privilege is required to store them. + + + + Filter existing row + Check new row + — + DELETE @@ -540,6 +561,19 @@ CREATE POLICY name ON — Filter existing row + + DELETE ... FOR PORTION OF + + Filter existing row  & + check new row  + + + Check new row  + + — + — + Filter existing row + INSERT ... ON CONFLICT diff --git a/doc/src/sgml/ref/delete.sgml b/doc/src/sgml/ref/delete.sgml index ffdcd7fc4fa..3ab1f60525a 100644 --- a/doc/src/sgml/ref/delete.sgml +++ b/doc/src/sgml/ref/delete.sgml @@ -97,7 +97,8 @@ DELETE FROM [ ONLY ] table_name [ * When FOR PORTION OF is used, the secondary inserts do not require INSERT privilege on the table. (This is because conceptually no new information is being added; the inserted rows - only preserve existing data about the untargeted time period.) + only preserve existing data about the untargeted time period.) Row-level + security INSERT policies are still checked for these leftover inserts. diff --git a/doc/src/sgml/ref/update.sgml b/doc/src/sgml/ref/update.sgml index 21a8fd8b037..3175671d475 100644 --- a/doc/src/sgml/ref/update.sgml +++ b/doc/src/sgml/ref/update.sgml @@ -101,7 +101,8 @@ UPDATE [ ONLY ] table_name [ * ] When FOR PORTION OF is used, the secondary inserts do not require INSERT privilege on the table. (This is because conceptually no new information is being added; the inserted rows - only preserve existing data about the untargeted time period.) + only preserve existing data about the untargeted time period.) Row-level + security INSERT policies are still checked for these leftover inserts. diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c index e88a1bc1a89..cacf30f25a0 100644 --- a/src/backend/rewrite/rowsecurity.c +++ b/src/backend/rewrite/rowsecurity.c @@ -393,6 +393,51 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, } } + /* + * UPDATE/DELETE FOR PORTION OF may insert leftover rows to preserve the + * portions of the old row not covered by the target range. Those hidden + * inserts go through ExecInsert(), so they need the same INSERT RLS WITH + * CHECK options as ordinary INSERTs. + */ + if (root->forPortionOf != NULL && rt_index == root->resultRelation && + (commandType == CMD_UPDATE || commandType == CMD_DELETE)) + { + List *insert_permissive_policies; + List *insert_restrictive_policies; + + get_policies_for_relation(rel, CMD_INSERT, user_id, + &insert_permissive_policies, + &insert_restrictive_policies); + add_with_check_options(rel, rt_index, + WCO_RLS_INSERT_CHECK, + insert_permissive_policies, + insert_restrictive_policies, + withCheckOptions, + hasSubLinks, + false); + + /* + * As with regular INSERT/UPDATE above, if SELECT rights are needed + * for the statement, ensure the leftover row remains visible. + */ + if (perminfo->requiredPerms & ACL_SELECT) + { + List *select_permissive_policies; + List *select_restrictive_policies; + + get_policies_for_relation(rel, CMD_SELECT, user_id, + &select_permissive_policies, + &select_restrictive_policies); + add_with_check_options(rel, rt_index, + WCO_RLS_INSERT_CHECK, + select_permissive_policies, + select_restrictive_policies, + withCheckOptions, + hasSubLinks, + true); + } + } + /* * FOR MERGE, we fetch policies for UPDATE, DELETE and INSERT (and ALL) * and set them up so that we can enforce the appropriate policy depending diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out index a050fc2dadf..271282c2d3b 100644 --- a/src/test/regress/expected/for_portion_of.out +++ b/src/test/regress/expected/for_portion_of.out @@ -2561,4 +2561,79 @@ SELECT * FROM fpo_cursed; (1 row) DROP TABLE fpo_cursed; +-- UPDATE/DELETE FOR PORTION OF leftover rows must satisfy RLS INSERT checks. +CREATE ROLE regress_fpo_rls; +CREATE TABLE fpo_rls ( + id int, + valid_at int4range +); +ALTER TABLE fpo_rls ENABLE ROW LEVEL SECURITY; +CREATE POLICY fpo_rls_select ON fpo_rls + FOR SELECT TO regress_fpo_rls + USING (true); +CREATE POLICY fpo_rls_update ON fpo_rls + FOR UPDATE TO regress_fpo_rls + USING (lower(valid_at) < 50) + WITH CHECK (lower(valid_at) < 50); +CREATE POLICY fpo_rls_delete ON fpo_rls + FOR DELETE TO regress_fpo_rls + USING (lower(valid_at) < 50); +CREATE POLICY fpo_rls_insert ON fpo_rls + FOR INSERT TO regress_fpo_rls + WITH CHECK (lower(valid_at) < 50); +GRANT SELECT, UPDATE, DELETE ON fpo_rls TO regress_fpo_rls; +INSERT INTO fpo_rls VALUES (1, '[10,100)'); +SET ROLE regress_fpo_rls; +UPDATE fpo_rls + FOR PORTION OF valid_at FROM 30 TO 100 + SET id = 2; +RESET ROLE; +SELECT * FROM fpo_rls ORDER BY valid_at; + id | valid_at +----+---------- + 1 | [10,30) + 2 | [30,100) +(2 rows) + +TRUNCATE fpo_rls; +INSERT INTO fpo_rls VALUES (1, '[10,100)'); +SET ROLE regress_fpo_rls; +DELETE FROM fpo_rls + FOR PORTION OF valid_at FROM 30 TO 100; +RESET ROLE; +SELECT * FROM fpo_rls ORDER BY valid_at; + id | valid_at +----+---------- + 1 | [10,30) +(1 row) + +TRUNCATE fpo_rls; +INSERT INTO fpo_rls VALUES (1, '[10,100)'); +SET ROLE regress_fpo_rls; +UPDATE fpo_rls + FOR PORTION OF valid_at FROM 30 TO 70 + SET id = 2; +ERROR: new row violates row-level security policy for table "fpo_rls" +RESET ROLE; +SELECT * FROM fpo_rls ORDER BY valid_at; + id | valid_at +----+---------- + 1 | [10,100) +(1 row) + +TRUNCATE fpo_rls; +INSERT INTO fpo_rls VALUES (1, '[10,100)'); +SET ROLE regress_fpo_rls; +DELETE FROM fpo_rls + FOR PORTION OF valid_at FROM 30 TO 70; +ERROR: new row violates row-level security policy for table "fpo_rls" +RESET ROLE; +SELECT * FROM fpo_rls ORDER BY valid_at; + id | valid_at +----+---------- + 1 | [10,100) +(1 row) + +DROP TABLE fpo_rls; +DROP ROLE regress_fpo_rls; RESET datestyle; diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 3a5e82c35bd..e62311cfb62 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -330,9 +330,51 @@ NOTICE: DELETE USING on rls_test_tgt.(1,"tgt c","TGT C") 1 | src a | 1 | tgt c | TGT C (1 row) +-- setup a temporal target table for FOR PORTION OF, with the same set of +-- NOTICE-emitting policies as rls_test_tgt +SET SESSION AUTHORIZATION regress_rls_alice; +CREATE TABLE rls_test_fpo (a int, valid_at int4range, b text); +ALTER TABLE rls_test_fpo ENABLE ROW LEVEL SECURITY; +GRANT SELECT, INSERT, UPDATE, DELETE ON rls_test_fpo TO public; +CREATE POLICY sel_pol ON rls_test_fpo FOR SELECT + USING (rls_test_policy_fn('SELECT USING on rls_test_fpo', rls_test_fpo)); +CREATE POLICY ins_pol ON rls_test_fpo FOR INSERT + WITH CHECK (rls_test_policy_fn('INSERT CHECK on rls_test_fpo', rls_test_fpo)); +CREATE POLICY upd_pol ON rls_test_fpo FOR UPDATE + USING (rls_test_policy_fn('UPDATE USING on rls_test_fpo', rls_test_fpo)) + WITH CHECK (rls_test_policy_fn('UPDATE CHECK on rls_test_fpo', rls_test_fpo)); +CREATE POLICY del_pol ON rls_test_fpo FOR DELETE + USING (rls_test_policy_fn('DELETE USING on rls_test_fpo', rls_test_fpo)); +INSERT INTO rls_test_fpo VALUES (1, '[1,10)', 'tgt a'); +-- test policies applied to regress_rls_bob +SET SESSION AUTHORIZATION regress_rls_bob; +-- UPDATE ... FOR PORTION OF filters with USING, checks the change with CHECK, +-- and checks the temporal leftovers with the FOR INSERT policy. +BEGIN; +UPDATE rls_test_fpo FOR PORTION OF valid_at FROM 3 TO 7 SET b = 'tgt b'; +NOTICE: UPDATE USING on rls_test_fpo.(1,"[1,10)","tgt a") +NOTICE: SELECT USING on rls_test_fpo.(1,"[1,10)","tgt a") +NOTICE: UPDATE CHECK on rls_test_fpo.(1,"[3,7)","tgt b") +NOTICE: SELECT USING on rls_test_fpo.(1,"[3,7)","tgt b") +NOTICE: INSERT CHECK on rls_test_fpo.(1,"[1,3)","tgt a") +NOTICE: SELECT USING on rls_test_fpo.(1,"[1,3)","tgt a") +NOTICE: INSERT CHECK on rls_test_fpo.(1,"[7,10)","tgt a") +NOTICE: SELECT USING on rls_test_fpo.(1,"[7,10)","tgt a") +ROLLBACK; +-- DELETE ... FOR PORTION OF filters with USING +-- and checks the temporal leftovers with the FOR INSERT policy. +BEGIN; +DELETE FROM rls_test_fpo FOR PORTION OF valid_at FROM 3 TO 7; +NOTICE: DELETE USING on rls_test_fpo.(1,"[1,10)","tgt a") +NOTICE: SELECT USING on rls_test_fpo.(1,"[1,10)","tgt a") +NOTICE: INSERT CHECK on rls_test_fpo.(1,"[1,3)","tgt a") +NOTICE: SELECT USING on rls_test_fpo.(1,"[1,3)","tgt a") +NOTICE: INSERT CHECK on rls_test_fpo.(1,"[7,10)","tgt a") +NOTICE: SELECT USING on rls_test_fpo.(1,"[7,10)","tgt a") +ROLLBACK; -- Tidy up RESET SESSION AUTHORIZATION; -DROP TABLE rls_test_src, rls_test_tgt; +DROP TABLE rls_test_src, rls_test_tgt, rls_test_fpo; DROP FUNCTION rls_test_tgt_set_c; DROP FUNCTION rls_test_policy_fn; -- BASIC Row-Level Security Scenario diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql index a1ee1d5e501..f48644347d1 100644 --- a/src/test/regress/sql/for_portion_of.sql +++ b/src/test/regress/sql/for_portion_of.sql @@ -1676,4 +1676,62 @@ ROLLBACK; SELECT * FROM fpo_cursed; DROP TABLE fpo_cursed; +-- UPDATE/DELETE FOR PORTION OF leftover rows must satisfy RLS INSERT checks. +CREATE ROLE regress_fpo_rls; +CREATE TABLE fpo_rls ( + id int, + valid_at int4range +); +ALTER TABLE fpo_rls ENABLE ROW LEVEL SECURITY; +CREATE POLICY fpo_rls_select ON fpo_rls + FOR SELECT TO regress_fpo_rls + USING (true); +CREATE POLICY fpo_rls_update ON fpo_rls + FOR UPDATE TO regress_fpo_rls + USING (lower(valid_at) < 50) + WITH CHECK (lower(valid_at) < 50); +CREATE POLICY fpo_rls_delete ON fpo_rls + FOR DELETE TO regress_fpo_rls + USING (lower(valid_at) < 50); +CREATE POLICY fpo_rls_insert ON fpo_rls + FOR INSERT TO regress_fpo_rls + WITH CHECK (lower(valid_at) < 50); +GRANT SELECT, UPDATE, DELETE ON fpo_rls TO regress_fpo_rls; + +INSERT INTO fpo_rls VALUES (1, '[10,100)'); +SET ROLE regress_fpo_rls; +UPDATE fpo_rls + FOR PORTION OF valid_at FROM 30 TO 100 + SET id = 2; +RESET ROLE; +SELECT * FROM fpo_rls ORDER BY valid_at; + +TRUNCATE fpo_rls; +INSERT INTO fpo_rls VALUES (1, '[10,100)'); +SET ROLE regress_fpo_rls; +DELETE FROM fpo_rls + FOR PORTION OF valid_at FROM 30 TO 100; +RESET ROLE; +SELECT * FROM fpo_rls ORDER BY valid_at; + +TRUNCATE fpo_rls; +INSERT INTO fpo_rls VALUES (1, '[10,100)'); +SET ROLE regress_fpo_rls; +UPDATE fpo_rls + FOR PORTION OF valid_at FROM 30 TO 70 + SET id = 2; +RESET ROLE; +SELECT * FROM fpo_rls ORDER BY valid_at; + +TRUNCATE fpo_rls; +INSERT INTO fpo_rls VALUES (1, '[10,100)'); +SET ROLE regress_fpo_rls; +DELETE FROM fpo_rls + FOR PORTION OF valid_at FROM 30 TO 70; +RESET ROLE; +SELECT * FROM fpo_rls ORDER BY valid_at; + +DROP TABLE fpo_rls; +DROP ROLE regress_fpo_rls; + RESET datestyle; diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index 6b3566271df..b3fc1a1fe9a 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -190,9 +190,42 @@ MERGE INTO rls_test_tgt t USING rls_test_src s ON t.a = s.a WHEN MATCHED THEN DELETE RETURNING *; +-- setup a temporal target table for FOR PORTION OF, with the same set of +-- NOTICE-emitting policies as rls_test_tgt +SET SESSION AUTHORIZATION regress_rls_alice; +CREATE TABLE rls_test_fpo (a int, valid_at int4range, b text); +ALTER TABLE rls_test_fpo ENABLE ROW LEVEL SECURITY; +GRANT SELECT, INSERT, UPDATE, DELETE ON rls_test_fpo TO public; +CREATE POLICY sel_pol ON rls_test_fpo FOR SELECT + USING (rls_test_policy_fn('SELECT USING on rls_test_fpo', rls_test_fpo)); +CREATE POLICY ins_pol ON rls_test_fpo FOR INSERT + WITH CHECK (rls_test_policy_fn('INSERT CHECK on rls_test_fpo', rls_test_fpo)); +CREATE POLICY upd_pol ON rls_test_fpo FOR UPDATE + USING (rls_test_policy_fn('UPDATE USING on rls_test_fpo', rls_test_fpo)) + WITH CHECK (rls_test_policy_fn('UPDATE CHECK on rls_test_fpo', rls_test_fpo)); +CREATE POLICY del_pol ON rls_test_fpo FOR DELETE + USING (rls_test_policy_fn('DELETE USING on rls_test_fpo', rls_test_fpo)); + +INSERT INTO rls_test_fpo VALUES (1, '[1,10)', 'tgt a'); + +-- test policies applied to regress_rls_bob +SET SESSION AUTHORIZATION regress_rls_bob; + +-- UPDATE ... FOR PORTION OF filters with USING, checks the change with CHECK, +-- and checks the temporal leftovers with the FOR INSERT policy. +BEGIN; +UPDATE rls_test_fpo FOR PORTION OF valid_at FROM 3 TO 7 SET b = 'tgt b'; +ROLLBACK; + +-- DELETE ... FOR PORTION OF filters with USING +-- and checks the temporal leftovers with the FOR INSERT policy. +BEGIN; +DELETE FROM rls_test_fpo FOR PORTION OF valid_at FROM 3 TO 7; +ROLLBACK; + -- Tidy up RESET SESSION AUTHORIZATION; -DROP TABLE rls_test_src, rls_test_tgt; +DROP TABLE rls_test_src, rls_test_tgt, rls_test_fpo; DROP FUNCTION rls_test_tgt_set_c; DROP FUNCTION rls_test_policy_fn; -- 2.47.3