Skip to content

Commit af1c770

Browse files
craig[bot]spilchenRaduBerinde
committed
143063: sql: enforce SELECT policies for UPDATE/DELETE r=spilchen a=spilchen To align with postgres behavior, SELECT policies should be applied in specific cases: - For DELETE statements, apply SELECT policies as filters. - For UPDATE statements: - Apply SELECT policies as filters when selecting existing rows. - Apply SELECT policies as check constraints for new rows. Additionally, when a SELECT query includes locking clauses (e.g., SELECT ... FOR UPDATE|SHARE), UPDATE policies should also be applied. For more details on the postgres behaviour, see this: https://www.postgresql.org/docs/17/sql-createpolicy.html#SQL-CREATEPOLICY-UPDATE Closes #136742 Epic: CRDB-45203 Release note: none 143139: backup: add panic log in restoreDataProcessor r=RaduBerinde a=RaduBerinde A test is causing a panic being emitted by `Wait()`, but the original stack trace of the panic does not show up in logs. Add a Fatalf that should print the full details of the underlying panic. Informs: #141606 Release note: None Co-authored-by: Matt Spilchen <[email protected]> Co-authored-by: Radu Berinde <[email protected]>
3 parents 606cc0d + 2bbd01a + b47a96e commit af1c770

File tree

6 files changed

+473
-63
lines changed

6 files changed

+473
-63
lines changed

‎pkg/backup/restore_data_processor.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,14 @@ func (rd *restoreDataProcessor) Start(ctx context.Context) {
191191

192192
ctx, cancel := context.WithCancel(ctx)
193193
rd.cancelWorkersAndWait = func() {
194+
defer func() {
195+
// A panic here will not have a useful stack trace. If the panic is an
196+
// error that contains a stack trace, we want those full details.
197+
// TODO(jeffswenson): improve panic recovery more generally.
198+
if p := recover(); p != nil {
199+
panic(fmt.Sprintf("restore worker hit panic: %+v", p))
200+
}
201+
}()
194202
cancel()
195203
_ = rd.phaseGroup.Wait()
196204
}

‎pkg/sql/logictest/testdata/logic_test/row_level_security‎

Lines changed: 290 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2059,6 +2059,295 @@ DROP TABLE multip;
20592059
statement ok
20602060
DROP USER multi_user;
20612061

2062-
subtest multiple_insert_policies
2062+
# This is another test for multiple policies. But the focus here is how multiple
2063+
# policies are applied when they apply for other commands. For example, having
2064+
# a policy that applies to SELECT and a policy that applies to DELETE.
2065+
subtest multi_policies_multi_cmd
2066+
2067+
statement ok
2068+
create table r1 (c1 int);
2069+
2070+
statement ok
2071+
alter table r1 enable row level security;
2072+
2073+
statement ok
2074+
insert into r1 values (0),(1),(2),(3),(4),(5),(22);
2075+
2076+
statement ok
2077+
create policy sel1 on r1 for select using (c1 % 2 = 0);
2078+
2079+
statement ok
2080+
create policy upd1 on r1 for update using (c1 between 0 and 20);
2081+
2082+
statement ok
2083+
create user r1_user;
2084+
2085+
statement ok
2086+
grant all on r1 to r1_user;
2087+
2088+
statement ok
2089+
set role r1_user;
2090+
2091+
# UPDATE should only apply to the combination of both policies.
2092+
statement ok
2093+
update r1 set c1 = c1 ;
2094+
2095+
query I rowsort
2096+
update r1 set c1 = c1 returning c1;
2097+
----
2098+
0
2099+
2
2100+
4
2101+
2102+
statement error pq: new row violates row-level security policy for table "r1"
2103+
update r1 set c1 = c1 + 1;
2104+
2105+
statement error pq: new row violates row-level security policy for table "r1"
2106+
update r1 set c1 = c1 + 1 where c1 between 1 and 3;
2107+
2108+
statement ok
2109+
update r1 set c1 = c1 + 2 where c1 between 1 and 10;
2110+
2111+
query I rowsort
2112+
update r1 set c1 = c1 + 2 where c1 between 1 and 10 returning c1;
2113+
----
2114+
6
2115+
8
2116+
2117+
query I
2118+
update r1 set c1 = c1 + 2 where c1 > 20 returning c1;
2119+
----
2120+
2121+
query I rowsort
2122+
SELECT * FROM r1;
2123+
----
2124+
0
2125+
6
2126+
8
2127+
22
2128+
2129+
statement ok
2130+
SET ROLE root;
2131+
2132+
query I rowsort
2133+
SELECT * FROM r1;
2134+
----
2135+
0
2136+
1
2137+
3
2138+
5
2139+
6
2140+
8
2141+
22
2142+
2143+
statement ok
2144+
DROP TABLE r1;
2145+
2146+
# Do another test of select and update policies, but use simple expressions and
2147+
# try out different combinations of those expressions.
2148+
statement ok
2149+
CREATE TABLE cnt (counter INT);
2150+
2151+
statement ok
2152+
INSERT INTO cnt VALUES (1);
2153+
2154+
statement ok
2155+
GRANT ALL ON cnt TO r1_user;
2156+
2157+
statement ok
2158+
ALTER TABLE cnt ENABLE ROW LEVEL SECURITY, FORCE ROW LEVEL SECURITY;
2159+
2160+
statement ok
2161+
ALTER TABLE cnt OWNER TO r1_user;
2162+
2163+
statement ok
2164+
SET ROLE r1_user;
2165+
2166+
statement ok
2167+
CREATE POLICY upd1 ON cnt FOR UPDATE USING (true);
2168+
2169+
# Only an UPDATE policy and no SELECT policy. Nothing is updated.
2170+
query I
2171+
UPDATE cnt SET counter = counter + 1 RETURNING counter;
2172+
----
2173+
2174+
# Now create only a SELECT policy. Same behaviour as before.
2175+
statement ok
2176+
DROP POLICY upd1 ON cnt;
2177+
2178+
statement ok
2179+
CREATE POLICY sel1 ON cnt FOR SELECT USING (true);
2180+
2181+
query I
2182+
SELECT * FROM cnt;
2183+
----
2184+
1
2185+
2186+
# Any locking modes on the query will cause the UPDATE policies to be applied,
2187+
# which will filter out the row.
2188+
query I
2189+
SELECT * FROM cnt FOR UPDATE;
2190+
----
2191+
2192+
2193+
query I
2194+
SELECT * FROM cnt FOR SHARE;
2195+
----
2196+
2197+
2198+
let $cnt_oid
2199+
SELECT 'cnt'::REGCLASS::OID;
2200+
2201+
query I
2202+
SELECT * FROM [$cnt_oid as t];
2203+
----
2204+
1
2205+
2206+
query I
2207+
SELECT * FROM [$cnt_oid as t] FOR UPDATE;
2208+
----
2209+
2210+
query I
2211+
SELECT * FROM [$cnt_oid as t] FOR SHARE;
2212+
----
2213+
2214+
# Update with a SELECT policy but no UPDATE policy. Nothing should be returned.
2215+
query I
2216+
UPDATE cnt SET counter = counter + 1 RETURNING counter;
2217+
----
2218+
2219+
# Now add a SELECT policy, but it still filters everything out.
2220+
statement ok
2221+
CREATE POLICY upd1 ON cnt FOR UPDATE USING (false);
2222+
2223+
# Same as before, no update occurs because the reading of existing rows filters
2224+
# everything out.
2225+
query I
2226+
UPDATE cnt SET counter = counter + 1 RETURNING counter;
2227+
----
2228+
2229+
# Now replace the UPDATE policy with one that allows everything.
2230+
statement ok
2231+
DROP POLICY upd1 ON cnt;
2232+
2233+
statement ok
2234+
CREATE POLICY upd1 ON cnt FOR UPDATE USING (true);
2235+
2236+
query I
2237+
UPDATE cnt SET counter = counter + 1 RETURNING counter;
2238+
----
2239+
2
2240+
2241+
# Update the UPDATE policy so that it allows old rows but blocks all new rows.
2242+
statement ok
2243+
DROP POLICY upd1 ON cnt;
2244+
2245+
statement ok
2246+
CREATE POLICY upd1 ON cnt FOR UPDATE USING (true) WITH CHECK (false);
2247+
2248+
# We are able to read the row but cannot write a new row as it violates the
2249+
# update policy.
2250+
statement error pq: new row violates row-level security policy for table "cnt"
2251+
UPDATE cnt SET counter = counter + 1 RETURNING counter;
2252+
2253+
# Verify that select policy (true) with no delete policy will not delete anything.
2254+
query I
2255+
delete from cnt where counter is not null returning counter;
2256+
----
2257+
2258+
statement ok
2259+
delete from cnt;
2260+
2261+
query I
2262+
select counter from cnt;
2263+
----
2264+
2
2265+
2266+
# Now change the select policy to be always false, and delete policy to be always true.
2267+
statement ok
2268+
DROP POLICY sel1 ON cnt;
2269+
2270+
statement ok
2271+
CREATE POLICY sel1 ON cnt FOR SELECT USING (false);
2272+
2273+
statement ok
2274+
CREATE POLICY del1 ON cnt FOR DELETE USING (true);
2275+
2276+
query I
2277+
delete from cnt where counter > 0 returning counter;
2278+
----
2279+
2280+
statement ok
2281+
delete from cnt;
2282+
2283+
statement ok
2284+
ALTER TABLE cnt NO FORCE ROW LEVEL SECURITY;
2285+
2286+
query I
2287+
SELECT counter FROM cnt;
2288+
----
2289+
2
2290+
2291+
# Drop the select policy entirely. It should be no different than a deny-all select policy.
2292+
statement ok
2293+
DROP POLICY sel1 ON cnt;
2294+
2295+
statement ok
2296+
ALTER TABLE cnt FORCE ROW LEVEL SECURITY;
2297+
2298+
query I
2299+
delete from cnt where counter > 0 returning counter;
2300+
----
2301+
2302+
statement ok
2303+
delete from cnt;
2304+
2305+
statement ok
2306+
ALTER TABLE cnt NO FORCE ROW LEVEL SECURITY;
2307+
2308+
query I
2309+
SELECT counter FROM cnt;
2310+
----
2311+
2
2312+
2313+
statement ok
2314+
ALTER TABLE cnt FORCE ROW LEVEL SECURITY;
2315+
2316+
# Now change the select policy to always return true, and delete policy to always return false.
2317+
statement ok
2318+
DROP POLICY del1 ON cnt;
2319+
2320+
statement ok
2321+
CREATE POLICY sel1 ON cnt FOR SELECT USING (true);
2322+
2323+
statement ok
2324+
CREATE POLICY del1 ON cnt FOR DELETE USING (false);
2325+
2326+
query I
2327+
DELETE FROM cnt WHERE counter > 0 RETURNING counter;
2328+
----
2329+
2330+
statement ok
2331+
DELETE FROM cnt;
2332+
2333+
statement ok
2334+
ALTER TABLE cnt NO FORCE ROW LEVEL SECURITY;
2335+
2336+
query I
2337+
SELECT counter FROM cnt;
2338+
----
2339+
2
2340+
2341+
statement ok
2342+
ALTER TABLE cnt NO FORCE ROW LEVEL SECURITY;
2343+
2344+
statement ok
2345+
SET ROLE root
2346+
2347+
statement ok
2348+
DROP TABLE cnt;
2349+
2350+
statement ok
2351+
DROP USER r1_user;
20632352

20642353
subtest end

0 commit comments

Comments
 (0)