diff --git a/regress/expected/security.out b/regress/expected/security.out index 59e58cb05..521b76b62 100644 --- a/regress/expected/security.out +++ b/regress/expected/security.out @@ -1357,6 +1357,100 @@ $$) AS (a agtype); --- (0 rows) +-- ============================================================================ +-- PART 11b: DETACH DELETE with a function-based edge policy (issue #2474) +-- ============================================================================ +-- Regression for a SIGSEGV: an edge-label RLS policy whose USING / WITH CHECK +-- qual invokes a SQL-language function (e.g. a tenant/owner accessor) crashed +-- the backend on DETACH DELETE of an edge-connected vertex. The connected-edge +-- RLS check runs from end_cypher_delete() during executor shutdown, after the +-- portal's active snapshot has been popped; evaluating the qual then called the +-- function via fmgr_sql()/postquel_start(), which dereferences +-- GetActiveSnapshot() unconditionally -> NULL deref. A constant-only edge +-- policy (PART 11 above) never enters that path, so it did not crash. DETACH +-- DELETE must now complete and delete the vertex and its edge under the policy. +-- A STABLE SQL-language accessor (LANGUAGE sql is what exercises the +-- snapshot-dependent postquel executor path; a C/builtin function would not). +CREATE FUNCTION rls_detach_owner() RETURNS text + LANGUAGE sql STABLE AS $$ SELECT current_user::text $$; +ALTER TABLE rls_graph."Person" ENABLE ROW LEVEL SECURITY; +CREATE POLICY detach_fn_person_all ON rls_graph."Person" + FOR ALL USING (true) WITH CHECK (true); +ALTER TABLE rls_graph."KNOWS" ENABLE ROW LEVEL SECURITY; +CREATE POLICY detach_fn_knows_owner ON rls_graph."KNOWS" + FOR ALL + USING (properties->>'"owner"' = rls_detach_owner()) + WITH CHECK (properties->>'"owner"' = rls_detach_owner()); +-- Seed (as superuser, bypassing the WITH CHECK) an edge owned by rls_user1. +SELECT * FROM cypher('rls_graph', $$ + CREATE (:Person {name: 'DetachFn1', owner: 'rls_user1', department: 'DetachFn'}) +$$) AS (a agtype); + a +--- +(0 rows) + +SELECT * FROM cypher('rls_graph', $$ + CREATE (:Person {name: 'DetachFn2', owner: 'rls_user1', department: 'DetachFn'}) +$$) AS (a agtype); + a +--- +(0 rows) + +SELECT * FROM cypher('rls_graph', $$ + MATCH (a:Person {name: 'DetachFn1'}), (b:Person {name: 'DetachFn2'}) + CREATE (a)-[:KNOWS {since: 2021, owner: 'rls_user1'}]->(b) +$$) AS (a agtype); + a +--- +(0 rows) + +SET ROLE rls_user1; +-- Must NOT crash: deletes DetachFn1 and its connected edge under a +-- function-based edge policy the role satisfies. +SELECT * FROM cypher('rls_graph', $$ + MATCH (p:Person {name: 'DetachFn1'}) DETACH DELETE p +$$) AS (a agtype); + a +--- +(0 rows) + +RESET ROLE; +-- DetachFn1 is gone. +SELECT * FROM cypher('rls_graph', $$ + MATCH (p:Person {name: 'DetachFn1'}) RETURN p.name +$$) AS (name agtype); + name +------ +(0 rows) + +-- DetachFn2 (the other endpoint) survives. +SELECT * FROM cypher('rls_graph', $$ + MATCH (p:Person {name: 'DetachFn2'}) RETURN p.name +$$) AS (name agtype); + name +------------- + "DetachFn2" +(1 row) + +-- The connecting edge is gone (no edge now points into DetachFn2). +SELECT * FROM cypher('rls_graph', $$ + MATCH ()-[k:KNOWS]->(b:Person {name: 'DetachFn2'}) RETURN k.since +$$) AS (since agtype); + since +------- +(0 rows) + +-- cleanup +DROP POLICY detach_fn_knows_owner ON rls_graph."KNOWS"; +DROP POLICY detach_fn_person_all ON rls_graph."Person"; +DROP FUNCTION rls_detach_owner(); +SELECT * FROM cypher('rls_graph', $$ + MATCH (p:Person) WHERE p.department = 'DetachFn' DETACH DELETE p +$$) AS (a agtype); + a +--- +(0 rows) + -- ============================================================================ -- PART 12: Multiple Labels in Single Query -- ============================================================================ diff --git a/regress/sql/security.sql b/regress/sql/security.sql index 344dd23d4..65910cb5f 100644 --- a/regress/sql/security.sql +++ b/regress/sql/security.sql @@ -1179,6 +1179,81 @@ SELECT * FROM cypher('rls_graph', $$ MATCH (p:Person) WHERE p.department = 'Detach' DETACH DELETE p $$) AS (a agtype); +-- ============================================================================ +-- PART 11b: DETACH DELETE with a function-based edge policy (issue #2474) +-- ============================================================================ +-- Regression for a SIGSEGV: an edge-label RLS policy whose USING / WITH CHECK +-- qual invokes a SQL-language function (e.g. a tenant/owner accessor) crashed +-- the backend on DETACH DELETE of an edge-connected vertex. The connected-edge +-- RLS check runs from end_cypher_delete() during executor shutdown, after the +-- portal's active snapshot has been popped; evaluating the qual then called the +-- function via fmgr_sql()/postquel_start(), which dereferences +-- GetActiveSnapshot() unconditionally -> NULL deref. A constant-only edge +-- policy (PART 11 above) never enters that path, so it did not crash. DETACH +-- DELETE must now complete and delete the vertex and its edge under the policy. + +-- A STABLE SQL-language accessor (LANGUAGE sql is what exercises the +-- snapshot-dependent postquel executor path; a C/builtin function would not). +CREATE FUNCTION rls_detach_owner() RETURNS text + LANGUAGE sql STABLE AS $$ SELECT current_user::text $$; + +ALTER TABLE rls_graph."Person" ENABLE ROW LEVEL SECURITY; +CREATE POLICY detach_fn_person_all ON rls_graph."Person" + FOR ALL USING (true) WITH CHECK (true); + +ALTER TABLE rls_graph."KNOWS" ENABLE ROW LEVEL SECURITY; +CREATE POLICY detach_fn_knows_owner ON rls_graph."KNOWS" + FOR ALL + USING (properties->>'"owner"' = rls_detach_owner()) + WITH CHECK (properties->>'"owner"' = rls_detach_owner()); + +-- Seed (as superuser, bypassing the WITH CHECK) an edge owned by rls_user1. +SELECT * FROM cypher('rls_graph', $$ + CREATE (:Person {name: 'DetachFn1', owner: 'rls_user1', department: 'DetachFn'}) +$$) AS (a agtype); + +SELECT * FROM cypher('rls_graph', $$ + CREATE (:Person {name: 'DetachFn2', owner: 'rls_user1', department: 'DetachFn'}) +$$) AS (a agtype); + +SELECT * FROM cypher('rls_graph', $$ + MATCH (a:Person {name: 'DetachFn1'}), (b:Person {name: 'DetachFn2'}) + CREATE (a)-[:KNOWS {since: 2021, owner: 'rls_user1'}]->(b) +$$) AS (a agtype); + +SET ROLE rls_user1; + +-- Must NOT crash: deletes DetachFn1 and its connected edge under a +-- function-based edge policy the role satisfies. +SELECT * FROM cypher('rls_graph', $$ + MATCH (p:Person {name: 'DetachFn1'}) DETACH DELETE p +$$) AS (a agtype); + +RESET ROLE; + +-- DetachFn1 is gone. +SELECT * FROM cypher('rls_graph', $$ + MATCH (p:Person {name: 'DetachFn1'}) RETURN p.name +$$) AS (name agtype); + +-- DetachFn2 (the other endpoint) survives. +SELECT * FROM cypher('rls_graph', $$ + MATCH (p:Person {name: 'DetachFn2'}) RETURN p.name +$$) AS (name agtype); + +-- The connecting edge is gone (no edge now points into DetachFn2). +SELECT * FROM cypher('rls_graph', $$ + MATCH ()-[k:KNOWS]->(b:Person {name: 'DetachFn2'}) RETURN k.since +$$) AS (since agtype); + +-- cleanup +DROP POLICY detach_fn_knows_owner ON rls_graph."KNOWS"; +DROP POLICY detach_fn_person_all ON rls_graph."Person"; +DROP FUNCTION rls_detach_owner(); +SELECT * FROM cypher('rls_graph', $$ + MATCH (p:Person) WHERE p.department = 'DetachFn' DETACH DELETE p +$$) AS (a agtype); + -- ============================================================================ -- PART 12: Multiple Labels in Single Query -- ============================================================================ diff --git a/src/backend/executor/cypher_delete.c b/src/backend/executor/cypher_delete.c index 19eca3ad6..e2161e66e 100644 --- a/src/backend/executor/cypher_delete.c +++ b/src/backend/executor/cypher_delete.c @@ -25,6 +25,7 @@ #include "miscadmin.h" #include "utils/acl.h" #include "utils/rls.h" +#include "utils/snapmgr.h" #include "catalog/ag_label.h" #include "executor/cypher_executor.h" @@ -689,6 +690,29 @@ static void check_for_connected_edges(CustomScanState *node) (cypher_delete_custom_scan_state *)node; EState *estate = css->css.ss.ps.state; char *graph_name = css->delete_data->graph_name; + bool pushed_snapshot = false; + + /* + * check_for_connected_edges() runs from end_cypher_delete(), i.e. during + * executor shutdown (ExecEndPlan), by which point the portal's active + * snapshot has already been popped. Evaluating an edge-label RLS policy + * whose USING/WITH CHECK qual invokes a function (e.g. a STABLE tenant + * accessor) requires an active snapshot: check_security_quals() -> + * ExecQual() -> fmgr_sql() -> postquel_start() runs the function's query + * with GetActiveSnapshot() and dereferences it unconditionally. With no + * snapshot on the stack that is a NULL dereference -> SIGSEGV (#2474). + * + * Ensure a snapshot is active for the duration of the scan. es_snapshot is + * still valid here (the EState is not torn down until after this returns), + * and is the correct snapshot for reading connected edges. If an error is + * raised mid-scan (e.g. an RLS denial), transaction abort resets the active + * snapshot stack, so the unpaired push on that path is cleaned up. + */ + if (!ActiveSnapshotSet()) + { + PushActiveSnapshot(estate->es_snapshot); + pushed_snapshot = true; + } /* scans each label from css->edge_labels */ foreach (lc, css->edge_labels) @@ -834,4 +858,9 @@ static void check_for_connected_edges(CustomScanState *node) destroy_entity_result_rel_info(resultRelInfo); } + + if (pushed_snapshot) + { + PopActiveSnapshot(); + } }