Fix segfault on DETACH DELETE under RLS with a function-based edge policy (#2475)

A non-superuser subject to row-level security crashed the backend with
SIGSEGV when running DETACH DELETE on a vertex that has a connected edge,
if the edge label carried an RLS policy whose USING/WITH CHECK qual
invokes a function (e.g. a STABLE tenant/owner accessor). Reported in
issue #2474; reproduces on 1.7.0 and master.

Mechanism:

AGE deletes a vertex's connected edges in check_for_connected_edges(),
which is called from end_cypher_delete() -- i.e. during executor shutdown
(ExecEndPlan, reached via PortalCleanup -> ExecutorEnd when the portal is
dropped). By that point the portal's active snapshot has already been
popped, so the active-snapshot stack is empty.

RLS for Cypher DELETE is enforced at the executor level (added in #2309):
check_for_connected_edges() compiles the edge label's security quals and
evaluates them per candidate edge with check_security_quals() -> ExecQual().
When the qual calls a SQL-language function, ExecQual() dispatches into
fmgr_sql() -> postquel_start(), which runs the function's query and reads
the current snapshot with GetActiveSnapshot(). With no snapshot on the
stack that is a NULL-pointer dereference -> SIGSEGV (an assert build trips
the Assert(ActiveSnapshotSet()) in postquel_start first).

This is why the narrowing in the report holds: the vertex path
(process_delete_list) runs during normal execution while a snapshot is
active; edge-only DELETE and node-only DETACH DELETE never evaluate an
edge qual at teardown; a superuser bypasses RLS; and a constant-only edge
policy never enters fmgr_sql, so only DETACH DELETE of an edge-connected
vertex under a function-bearing edge policy trips it.

Fix:

Ensure an active snapshot for the duration of the connected-edge scan in
check_for_connected_edges(). es_snapshot is still valid there (the EState
is not torn down until this returns) and is the correct snapshot for
reading the edges, so push it if none is active and pop it before return.
The scans themselves already pass es_snapshot explicitly, so this only
affects RLS qual evaluation; non-RLS paths are unchanged. On the error
path (e.g. an RLS denial raised mid-scan) transaction abort resets the
active-snapshot stack, so the unpaired push is cleaned up.

Test:

Adds a regression to the security suite (PART 11b): an edge label policy
whose qual calls a STABLE SQL function, then DETACH DELETE of an
edge-connected vertex as the RLS-bound role. It must delete the vertex and
its edge (leaving the other endpoint) instead of crashing. The existing
PART 11 uses a constant-only edge policy, which does not exercise the
snapshot-dependent function path.

Author: BenSpex

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
 -- ============================================================================

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
 -- ============================================================================

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();
+    }
 }