Copilot commented on code in PR #2309:
URL: https://github.com/apache/age/pull/2309#discussion_r2702623615
##########
src/backend/executor/cypher_delete.c:
##########
@@ -537,6 +562,41 @@ static void check_for_connected_edges(CustomScanState
*node)
{
if (css->delete_data->detach)
{
+ AclResult aclresult;
+ Oid relid =
RelationGetRelid(resultRelInfo->ri_RelationDesc);
+
+ /* Check that the user has DELETE permission on the edge
table */
+ aclresult = pg_class_aclcheck(relid, GetUserId(),
ACL_DELETE);
+ if (aclresult != ACLCHECK_OK)
+ {
+ aclcheck_error(aclresult, OBJECT_TABLE, label_name);
+ }
+
+ /* Check RLS security quals (USING policy) before delete */
+ if (check_enable_rls(relid, InvalidOid, true) ==
RLS_ENABLED)
+ {
+ ExprContext *econtext = css->css.ss.ps.ps_ExprContext;
+ SecurityQualContext *sqc;
+
+ sqc = setup_security_quals(resultRelInfo, estate, node,
+ CMD_DELETE);
Review Comment:
The setup_security_quals function is called inside a nested loop for every
connected edge during DETACH DELETE. This is extremely inefficient as it will
be called once per edge. The security quals should be set up once before
processing the edges and reused for all edges of the same label.
##########
src/backend/executor/cypher_utils.c:
##########
@@ -268,3 +297,715 @@ HeapTuple insert_entity_tuple_cid(ResultRelInfo
*resultRelInfo,
return tuple;
}
+
+/*
+ * setup_wcos
+ *
+ * WithCheckOptions are added during the rewrite phase, but since AGE uses
+ * CMD_SELECT for all queries, WCOs don't get added for CREATE/SET/MERGE
+ * operations. This function compensates by adding WCOs at execution time.
+ *
+ * Based on PostgreSQL's row security implementation in rowsecurity.c
+ */
+void setup_wcos(ResultRelInfo *resultRelInfo, EState *estate,
+ CustomScanState *node, CmdType cmd)
+{
+ List *permissive_policies;
+ List *restrictive_policies;
+ List *withCheckOptions = NIL;
+ List *wcoExprs = NIL;
+ ListCell *lc;
+ Relation rel;
+ Oid user_id;
+ int rt_index;
+ WCOKind wco_kind;
+ bool hasSubLinks = false;
+
+ /* Determine the WCO kind based on command type */
+ if (cmd == CMD_INSERT)
+ {
+ wco_kind = WCO_RLS_INSERT_CHECK;
+ }
+ else if (cmd == CMD_UPDATE)
+ {
+ wco_kind = WCO_RLS_UPDATE_CHECK;
+ }
+ else
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg_internal("unexpected command type for setup_wcos")));
+ }
+
+ rel = resultRelInfo->ri_RelationDesc;
+
+ /*
+ * Use rt_index=1 since we're evaluating policies against a single
relation.
+ * Policy quals are stored with varno=1, and we set ecxt_scantuple to the
+ * tuple we want to check, so keeping varno=1 is correct.
+ */
+ rt_index = 1;
+ user_id = GetUserId();
+
+ /* Get the policies for the specified command type */
+ get_policies_for_relation(rel, cmd, user_id,
+ &permissive_policies,
+ &restrictive_policies);
+
+ /* Build WithCheckOptions from the policies */
+ add_with_check_options(rel, rt_index, wco_kind,
+ permissive_policies,
+ restrictive_policies,
+ &withCheckOptions,
+ &hasSubLinks,
+ false);
+
+ /* Compile the WCO expressions */
+ foreach(lc, withCheckOptions)
+ {
+ WithCheckOption *wco = lfirst_node(WithCheckOption, lc);
+ ExprState *wcoExpr;
+
+ /* Ensure qual is a List for ExecInitQual */
+ if (!IsA(wco->qual, List))
+ {
+ wco->qual = (Node *) list_make1(wco->qual);
+ }
+
+ wcoExpr = ExecInitQual((List *) wco->qual, (PlanState *) node);
+ wcoExprs = lappend(wcoExprs, wcoExpr);
+ }
+
+ /* Set up the ResultRelInfo with WCOs */
+ resultRelInfo->ri_WithCheckOptions = withCheckOptions;
+ resultRelInfo->ri_WithCheckOptionExprs = wcoExprs;
+}
+
+/*
+ * get_policies_for_relation
+ *
+ * Returns lists of permissive and restrictive policies to be applied to the
+ * specified relation, based on the command type and role.
+ *
+ * This includes any policies added by extensions.
+ *
+ * Copied from PostgreSQL's src/backend/rewrite/rowsecurity.c
+ */
+static void
+get_policies_for_relation(Relation relation, CmdType cmd, Oid user_id,
+ List **permissive_policies,
+ List **restrictive_policies)
+{
+ ListCell *item;
+
+ *permissive_policies = NIL;
+ *restrictive_policies = NIL;
+
+ /* First find all internal policies for the relation. */
+ foreach(item, relation->rd_rsdesc->policies)
+ {
+ bool cmd_matches = false;
+ RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+
+ /* Always add ALL policies, if they exist. */
+ if (policy->polcmd == '*')
+ cmd_matches = true;
+ else
+ {
+ /* Check whether the policy applies to the specified
command type */
+ switch (cmd)
+ {
+ case CMD_SELECT:
+ if (policy->polcmd == ACL_SELECT_CHR)
+ cmd_matches = true;
+ break;
+ case CMD_INSERT:
+ if (policy->polcmd == ACL_INSERT_CHR)
+ cmd_matches = true;
+ break;
+ case CMD_UPDATE:
+ if (policy->polcmd == ACL_UPDATE_CHR)
+ cmd_matches = true;
+ break;
+ case CMD_DELETE:
+ if (policy->polcmd == ACL_DELETE_CHR)
+ cmd_matches = true;
+ break;
+ case CMD_MERGE:
+
+ /*
+ * We do not support a separate policy
for MERGE command.
+ * Instead it derives from the policies
defined for other
+ * commands.
+ */
+ break;
+ default:
+ elog(ERROR, "unrecognized policy
command type %d",
+ (int) cmd);
+ break;
+ }
+ }
+
+ /*
+ * Add this policy to the relevant list of policies if it
applies to
+ * the specified role.
+ */
+ if (cmd_matches && check_role_for_policy(policy->roles,
user_id))
+ {
+ if (policy->permissive)
+ *permissive_policies =
lappend(*permissive_policies, policy);
+ else
+ *restrictive_policies =
lappend(*restrictive_policies, policy);
+ }
+ }
+
+ /*
+ * We sort restrictive policies by name so that any WCOs they generate
are
+ * checked in a well-defined order.
+ */
+ sort_policies_by_name(*restrictive_policies);
+
+ /*
+ * Then add any permissive or restrictive policies defined by
extensions.
+ * These are simply appended to the lists of internal policies, if they
+ * apply to the specified role.
+ */
+ if (row_security_policy_hook_restrictive)
+ {
+ List *hook_policies =
+ (*row_security_policy_hook_restrictive) (cmd, relation);
+
+ /*
+ * As with built-in restrictive policies, we sort any
hook-provided
+ * restrictive policies by name also. Note that we also
intentionally
+ * always check all built-in restrictive policies, in name
order,
+ * before checking restrictive policies added by hooks, in name
order.
+ */
+ sort_policies_by_name(hook_policies);
+
+ foreach(item, hook_policies)
+ {
+ RowSecurityPolicy *policy = (RowSecurityPolicy *)
lfirst(item);
+
+ if (check_role_for_policy(policy->roles, user_id))
+ *restrictive_policies =
lappend(*restrictive_policies, policy);
+ }
+ }
+
+ if (row_security_policy_hook_permissive)
+ {
+ List *hook_policies =
+ (*row_security_policy_hook_permissive) (cmd, relation);
+
+ foreach(item, hook_policies)
+ {
+ RowSecurityPolicy *policy = (RowSecurityPolicy *)
lfirst(item);
+
+ if (check_role_for_policy(policy->roles, user_id))
+ *permissive_policies =
lappend(*permissive_policies, policy);
+ }
+ }
+}
+
+/*
+ * add_with_check_options
+ *
+ * Add WithCheckOptions of the specified kind to check that new records
+ * added by an INSERT or UPDATE are consistent with the specified RLS
+ * policies. Normally new data must satisfy the WITH CHECK clauses from the
+ * policies. If a policy has no explicit WITH CHECK clause, its USING clause
+ * is used instead. In the special case of an UPDATE arising from an
+ * INSERT ... ON CONFLICT DO UPDATE, existing records are first checked using
+ * a WCO_RLS_CONFLICT_CHECK WithCheckOption, which always uses the USING
+ * clauses from RLS policies.
+ *
+ * New WCOs are added to withCheckOptions, and hasSubLinks is set to true if
+ * any of the check clauses added contain sublink subqueries.
+ *
+ * Copied from PostgreSQL's src/backend/rewrite/rowsecurity.c
+ */
+static void
+add_with_check_options(Relation rel,
+ int rt_index,
+ WCOKind kind,
+ List *permissive_policies,
+ List *restrictive_policies,
+ List **withCheckOptions,
+ bool *hasSubLinks,
+ bool force_using)
+{
+ ListCell *item;
+ List *permissive_quals = NIL;
+
+#define QUAL_FOR_WCO(policy) \
+ ( !force_using && \
+ (policy)->with_check_qual != NULL ? \
+ (policy)->with_check_qual : (policy)->qual )
+
+ /*
+ * First collect up the permissive policy clauses, similar to
+ * add_security_quals.
+ */
+ foreach(item, permissive_policies)
+ {
+ RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+ Expr *qual = QUAL_FOR_WCO(policy);
+
+ if (qual != NULL)
+ {
+ permissive_quals = lappend(permissive_quals,
copyObject(qual));
+ *hasSubLinks |= policy->hassublinks;
+ }
+ }
+
+ /*
+ * There must be at least one permissive qual found or no rows are
allowed
+ * to be added. This is the same as in add_security_quals.
+ *
+ * If there are no permissive_quals then we fall through and return a
+ * single 'false' WCO, preventing all new rows.
+ */
+ if (permissive_quals != NIL)
+ {
+ /*
+ * Add a single WithCheckOption for all the permissive policy
clauses,
+ * combining them together using OR. This check has no policy
name,
+ * since if the check fails it means that no policy granted
permission
+ * to perform the update, rather than any particular policy
being
+ * violated.
+ */
+ WithCheckOption *wco;
+
+ wco = makeNode(WithCheckOption);
+ wco->kind = kind;
+ wco->relname = pstrdup(RelationGetRelationName(rel));
+ wco->polname = NULL;
+ wco->cascaded = false;
+
+ if (list_length(permissive_quals) == 1)
+ wco->qual = (Node *) linitial(permissive_quals);
+ else
+ wco->qual = (Node *) makeBoolExpr(OR_EXPR,
permissive_quals, -1);
+
+ ChangeVarNodes(wco->qual, 1, rt_index, 0);
+
+ *withCheckOptions = list_append_unique(*withCheckOptions, wco);
+
+ /*
+ * Now add WithCheckOptions for each of the restrictive policy
clauses
+ * (which will be combined together using AND). We use a
separate
+ * WithCheckOption for each restrictive policy to allow the
policy
+ * name to be included in error reports if the policy is
violated.
+ */
+ foreach(item, restrictive_policies)
+ {
+ RowSecurityPolicy *policy = (RowSecurityPolicy *)
lfirst(item);
+ Expr *qual = QUAL_FOR_WCO(policy);
+
+ if (qual != NULL)
+ {
+ qual = copyObject(qual);
+ ChangeVarNodes((Node *) qual, 1, rt_index, 0);
+
+ wco = makeNode(WithCheckOption);
+ wco->kind = kind;
+ wco->relname =
pstrdup(RelationGetRelationName(rel));
+ wco->polname = pstrdup(policy->policy_name);
+ wco->qual = (Node *) qual;
+ wco->cascaded = false;
+
+ *withCheckOptions =
list_append_unique(*withCheckOptions, wco);
+ *hasSubLinks |= policy->hassublinks;
+ }
+ }
+ }
+ else
+ {
+ /*
+ * If there were no policy clauses to check new data, add a
single
+ * always-false WCO (a default-deny policy).
+ */
+ WithCheckOption *wco;
+
+ wco = makeNode(WithCheckOption);
+ wco->kind = kind;
+ wco->relname = pstrdup(RelationGetRelationName(rel));
+ wco->polname = NULL;
+ wco->qual = (Node *) makeConst(BOOLOID, -1, InvalidOid,
+
sizeof(bool), BoolGetDatum(false),
+
false, true);
+ wco->cascaded = false;
+
+ *withCheckOptions = lappend(*withCheckOptions, wco);
+ }
+}
+
+/*
+ * sort_policies_by_name
+ *
+ * This is only used for restrictive policies, ensuring that any
+ * WithCheckOptions they generate are applied in a well-defined order.
+ * This is not necessary for permissive policies, since they are all combined
+ * together using OR into a single WithCheckOption check.
+ *
+ * Copied from PostgreSQL's src/backend/rewrite/rowsecurity.c
+ */
+static void
+sort_policies_by_name(List *policies)
+{
+ list_sort(policies, row_security_policy_cmp);
+}
+
+/*
+ * list_sort comparator to sort RowSecurityPolicy entries by name
+ *
+ * Copied from PostgreSQL's src/backend/rewrite/rowsecurity.c
+ */
+static int
+row_security_policy_cmp(const ListCell *a, const ListCell *b)
+{
+ const RowSecurityPolicy *pa = (const RowSecurityPolicy *) lfirst(a);
+ const RowSecurityPolicy *pb = (const RowSecurityPolicy *) lfirst(b);
+
+ /* Guard against NULL policy names from extensions */
+ if (pa->policy_name == NULL)
+ return pb->policy_name == NULL ? 0 : 1;
+ if (pb->policy_name == NULL)
+ return -1;
+
+ return strcmp(pa->policy_name, pb->policy_name);
+}
+
+/*
+ * check_role_for_policy -
+ * determines if the policy should be applied for the current role
+ *
+ * Copied from PostgreSQL's src/backend/rewrite/rowsecurity.c
+ */
+static bool
+check_role_for_policy(ArrayType *policy_roles, Oid user_id)
+{
+ int i;
+ Oid *roles = (Oid *) ARR_DATA_PTR(policy_roles);
+
+ /* Quick fall-thru for policies applied to all roles */
+ if (roles[0] == ACL_ID_PUBLIC)
+ return true;
+
+ for (i = 0; i < ARR_DIMS(policy_roles)[0]; i++)
+ {
+ if (has_privs_of_role(user_id, roles[i]))
+ return true;
+ }
+
+ return false;
+}
+
+/*
+ * add_security_quals
+ *
+ * Add security quals to enforce the specified RLS policies, restricting
+ * access to existing data in a table. If there are no policies controlling
+ * access to the table, then all access is prohibited --- i.e., an implicit
+ * default-deny policy is used.
+ *
+ * New security quals are added to securityQuals, and hasSubLinks is set to
+ * true if any of the quals added contain sublink subqueries.
+ *
+ * Copied from PostgreSQL's src/backend/rewrite/rowsecurity.c
+ */
+static void
+add_security_quals(int rt_index,
+ List *permissive_policies,
+ List *restrictive_policies,
+ List **securityQuals,
+ bool *hasSubLinks)
+{
+ ListCell *item;
+ List *permissive_quals = NIL;
+ Expr *rowsec_expr;
+
+ /*
+ * First collect up the permissive quals. If we do not find any
+ * permissive policies then no rows are visible (this is handled below).
+ */
+ foreach(item, permissive_policies)
+ {
+ RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+
+ if (policy->qual != NULL)
+ {
+ permissive_quals = lappend(permissive_quals,
+
copyObject(policy->qual));
+ *hasSubLinks |= policy->hassublinks;
+ }
+ }
+
+ /*
+ * We must have permissive quals, always, or no rows are visible.
+ *
+ * If we do not, then we simply return a single 'false' qual which
results
+ * in no rows being visible.
+ */
+ if (permissive_quals != NIL)
+ {
+ /*
+ * We now know that permissive policies exist, so we can now add
+ * security quals based on the USING clauses from the
restrictive
+ * policies. Since these need to be combined together using
AND, we
+ * can just add them one at a time.
+ */
+ foreach(item, restrictive_policies)
+ {
+ RowSecurityPolicy *policy = (RowSecurityPolicy *)
lfirst(item);
+ Expr *qual;
+
+ if (policy->qual != NULL)
+ {
+ qual = copyObject(policy->qual);
+ ChangeVarNodes((Node *) qual, 1, rt_index, 0);
+
+ *securityQuals =
list_append_unique(*securityQuals, qual);
+ *hasSubLinks |= policy->hassublinks;
+ }
+ }
+
+ /*
+ * Then add a single security qual combining together the USING
+ * clauses from all the permissive policies using OR.
+ */
+ if (list_length(permissive_quals) == 1)
+ rowsec_expr = (Expr *) linitial(permissive_quals);
+ else
+ rowsec_expr = makeBoolExpr(OR_EXPR, permissive_quals,
-1);
+
+ ChangeVarNodes((Node *) rowsec_expr, 1, rt_index, 0);
+ *securityQuals = list_append_unique(*securityQuals,
rowsec_expr);
+ }
+ else
+
+ /*
+ * A permissive policy must exist for rows to be visible at all.
+ * Therefore, if there were no permissive policies found,
return a
+ * single always-false clause.
+ */
+ *securityQuals = lappend(*securityQuals,
+
makeConst(BOOLOID, -1, InvalidOid,
+
sizeof(bool), BoolGetDatum(false),
+
false, true));
+}
+
+/*
+ * setup_security_quals
+ *
+ * Security quals (USING policies) are added during the rewrite phase, but
+ * since AGE uses CMD_SELECT for all queries, they don't get added for
+ * UPDATE/DELETE operations. This function sets up security quals at
+ * execution time to be evaluated against each tuple before modification.
+ */
+SecurityQualContext *
+setup_security_quals(ResultRelInfo *resultRelInfo, EState *estate,
+ CustomScanState *node, CmdType cmd)
+{
+ List *permissive_policies;
+ List *restrictive_policies;
+ List *securityQuals = NIL;
+ SecurityQualContext *context;
+ ListCell *lc;
+ Relation rel;
+ Oid user_id;
+ int rt_index;
+ bool hasSubLinks = false;
+
+ /* Only UPDATE and DELETE have security quals */
+ if (cmd != CMD_UPDATE && cmd != CMD_DELETE)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg_internal("unexpected command type for
setup_security_quals")));
+ }
+
+ context = palloc0(sizeof(SecurityQualContext));
+ context->qualExprs = NIL;
+
+ rel = resultRelInfo->ri_RelationDesc;
+
+ /* If no RLS policies exist, return empty context */
+ if (rel->rd_rsdesc == NULL)
+ {
+ return context;
+ }
+
+ /*
+ * Use rt_index=1 since we're evaluating policies against a single
relation.
+ * Policy quals are stored with varno=1, and we set ecxt_scantuple to the
+ * tuple we want to check, so keeping varno=1 is correct.
+ */
+ rt_index = 1;
+ user_id = GetUserId();
+
+ /* Get the policies for the specified command type */
+ get_policies_for_relation(rel, cmd, user_id,
+ &permissive_policies,
+ &restrictive_policies);
+
+ /* Build security quals from the policies */
+ add_security_quals(rt_index, permissive_policies, restrictive_policies,
+ &securityQuals, &hasSubLinks);
+
+ /* Compile the security qual expressions */
+ foreach(lc, securityQuals)
+ {
+ Expr *qual = (Expr *) lfirst(lc);
+ ExprState *qualExpr;
+
+ /* Ensure qual is a List for ExecInitQual */
+ if (!IsA(qual, List))
+ {
+ qual = (Expr *) list_make1(qual);
+ }
+
+ qualExpr = ExecInitQual((List *) qual, (PlanState *) node);
+ context->qualExprs = lappend(context->qualExprs, qualExpr);
+ }
+
+ context->relname = pstrdup(RelationGetRelationName(rel));
Review Comment:
The relname field is allocated with pstrdup but the SecurityQualContext
structure documentation and usage don't indicate who is responsible for freeing
this memory. This could lead to a memory leak if the context is allocated in a
long-lived memory context.
##########
src/backend/executor/cypher_delete.c:
##########
@@ -537,6 +562,41 @@ static void check_for_connected_edges(CustomScanState
*node)
{
if (css->delete_data->detach)
{
+ AclResult aclresult;
+ Oid relid =
RelationGetRelid(resultRelInfo->ri_RelationDesc);
+
+ /* Check that the user has DELETE permission on the edge
table */
+ aclresult = pg_class_aclcheck(relid, GetUserId(),
ACL_DELETE);
+ if (aclresult != ACLCHECK_OK)
+ {
+ aclcheck_error(aclresult, OBJECT_TABLE, label_name);
+ }
+
+ /* Check RLS security quals (USING policy) before delete */
+ if (check_enable_rls(relid, InvalidOid, true) ==
RLS_ENABLED)
+ {
+ ExprContext *econtext = css->css.ss.ps.ps_ExprContext;
+ SecurityQualContext *sqc;
+
+ sqc = setup_security_quals(resultRelInfo, estate, node,
+ CMD_DELETE);
+
+ /*
+ * For DETACH DELETE, error out if edge RLS check
fails.
+ * Unlike normal DELETE which silently skips, we cannot
+ * silently skip edges here as it would leave dangling
+ * edges pointing to deleted vertices.
+ */
+ if (!check_security_quals(sqc, slot, econtext))
Review Comment:
In DETACH DELETE, the SecurityQualContext is created but the check is
performed on 'slot' which is initialized earlier in the loop for a different
purpose (storing the edge tuple). However, the security quals check should be
performed on the edge tuple being deleted. There's a mismatch - 'slot' at line
590 contains the edge tuple, but it's unclear if this is the correct tuple to
check against the DELETE policy. This needs clarification or correction.
```suggestion
if (!check_security_quals(sqc, tuple, econtext))
```
##########
src/backend/executor/cypher_set.c:
##########
@@ -580,8 +597,30 @@ static void process_update_list(CustomScanState *node)
*/
if (HeapTupleIsValid(heap_tuple))
{
- heap_tuple = update_entity_tuple(resultRelInfo, slot, estate,
- heap_tuple);
+ bool should_update = true;
+
+ /* Check RLS security quals (USING policy) before update */
+ if (check_enable_rls(resultRelInfo->ri_RelationDesc->rd_id,
+ InvalidOid, true) == RLS_ENABLED)
+ {
+ TupleTableSlot *oldslot;
+ SecurityQualContext *sqc;
+
+ sqc = setup_security_quals(resultRelInfo, estate, node,
+ CMD_UPDATE);
Review Comment:
The setup_security_quals function is called inside the loop for every tuple
being updated. This is inefficient as the security quals setup only depends on
the relation and command type, not on individual tuples. The function should be
called once before the loop and reused for all tuples.
##########
src/backend/executor/cypher_delete.c:
##########
@@ -447,6 +450,28 @@ static void process_delete_list(CustomScanState *node)
continue;
}
+ /* Check RLS security quals (USING policy) before delete */
+ if (check_enable_rls(resultRelInfo->ri_RelationDesc->rd_id,
+ InvalidOid, true) == RLS_ENABLED)
+ {
+ TupleTableSlot *slot;
+ SecurityQualContext *sqc;
+
+ sqc = setup_security_quals(resultRelInfo, estate, node,
CMD_DELETE);
+ slot = ExecInitExtraTupleSlot(
+ estate, RelationGetDescr(resultRelInfo->ri_RelationDesc),
+ &TTSOpsHeapTuple);
+ ExecStoreHeapTuple(heap_tuple, slot, false);
Review Comment:
The slot TupleTableSlot created with ExecInitExtraTupleSlot is never cleaned
up. This creates a resource leak as a new slot is allocated for each tuple
being deleted. The slot should be cleaned up after use or reused across
iterations.
##########
src/backend/parser/cypher_clause.c:
##########
@@ -346,6 +346,100 @@ static bool isa_special_VLE_case(cypher_path *path);
static ParseNamespaceItem *find_pnsi(cypher_parsestate *cpstate, char
*varname);
static bool has_list_comp_or_subquery(Node *expr, void *context);
+/*
+ * Add required permissions to the RTEPermissionInfo for a relation.
+ * Recursively searches through RTEs including subqueries.
+ */
+static bool
+add_rte_permissions_recurse(List *rtable, List *rteperminfos,
+ Oid relid, AclMode permissions)
+{
+ ListCell *lc;
+
+ /* First check the perminfos at this level */
+ foreach(lc, rteperminfos)
+ {
+ RTEPermissionInfo *perminfo = lfirst(lc);
+
+ if (perminfo->relid == relid)
+ {
+ perminfo->requiredPerms |= permissions;
+ return true;
+ }
+ }
+
+ /* Then recurse into subqueries */
+ foreach(lc, rtable)
+ {
+ RangeTblEntry *rte = lfirst(lc);
+
+ if (rte->rtekind == RTE_SUBQUERY && rte->subquery != NULL)
+ {
+ if (add_rte_permissions_recurse(rte->subquery->rtable,
+ rte->subquery->rteperminfos,
+ relid, permissions))
+ {
+ return true;
+ }
+ }
+ }
+
+ return false;
+}
+
+/*
+ * Add required permissions to the RTEPermissionInfo for a relation.
+ * Searches through p_rteperminfos and subqueries for a matching relOid
+ * and adds the specified permissions to requiredPerms.
+ */
+static void
+add_rte_permissions(ParseState *pstate, Oid relid, AclMode permissions)
+{
+ add_rte_permissions_recurse(pstate->p_rtable, pstate->p_rteperminfos,
+ relid, permissions);
Review Comment:
The function add_rte_permissions silently ignores the case where the relid
is not found in any RTEPermissionInfo. This means that if a permission cannot
be added (e.g., if the relation is not in the RTE list), the operation will
silently proceed without the required permission check. Consider logging a
warning or error when permissions cannot be added to help diagnose
permission-related issues.
```suggestion
bool found;
found = add_rte_permissions_recurse(pstate->p_rtable,
pstate->p_rteperminfos,
relid, permissions);
/*
* If we couldn't find a matching RTEPermissionInfo for this relation,
* then the requested permissions were not added. Emit a warning to help
* diagnose potential missing permission checks.
*/
if (!found)
{
ereport(WARNING,
(errmsg("could not add required permissions for relation %u:
"
"relation not found in rangetable", relid)));
}
```
##########
src/backend/executor/cypher_set.c:
##########
@@ -580,8 +597,30 @@ static void process_update_list(CustomScanState *node)
*/
if (HeapTupleIsValid(heap_tuple))
{
- heap_tuple = update_entity_tuple(resultRelInfo, slot, estate,
- heap_tuple);
+ bool should_update = true;
+
+ /* Check RLS security quals (USING policy) before update */
+ if (check_enable_rls(resultRelInfo->ri_RelationDesc->rd_id,
+ InvalidOid, true) == RLS_ENABLED)
+ {
+ TupleTableSlot *oldslot;
+ SecurityQualContext *sqc;
+
+ sqc = setup_security_quals(resultRelInfo, estate, node,
+ CMD_UPDATE);
+ oldslot = ExecInitExtraTupleSlot(
+ estate,
RelationGetDescr(resultRelInfo->ri_RelationDesc),
+ &TTSOpsHeapTuple);
+ ExecStoreHeapTuple(heap_tuple, oldslot, false);
+ should_update = check_security_quals(sqc, oldslot,
econtext);
Review Comment:
The oldslot TupleTableSlot created with ExecInitExtraTupleSlot is never
cleaned up. This creates a resource leak as each updated tuple will allocate a
new slot that is never freed. The slot should be cleaned up after the security
check or reused across iterations.
```suggestion
should_update = check_security_quals(sqc, oldslot,
econtext);
ExecDropSingleTupleTableSlot(oldslot);
```
##########
src/backend/executor/cypher_utils.c:
##########
@@ -268,3 +297,715 @@ HeapTuple insert_entity_tuple_cid(ResultRelInfo
*resultRelInfo,
return tuple;
}
+
+/*
+ * setup_wcos
+ *
+ * WithCheckOptions are added during the rewrite phase, but since AGE uses
+ * CMD_SELECT for all queries, WCOs don't get added for CREATE/SET/MERGE
+ * operations. This function compensates by adding WCOs at execution time.
+ *
+ * Based on PostgreSQL's row security implementation in rowsecurity.c
+ */
+void setup_wcos(ResultRelInfo *resultRelInfo, EState *estate,
+ CustomScanState *node, CmdType cmd)
+{
+ List *permissive_policies;
+ List *restrictive_policies;
+ List *withCheckOptions = NIL;
+ List *wcoExprs = NIL;
+ ListCell *lc;
+ Relation rel;
+ Oid user_id;
+ int rt_index;
+ WCOKind wco_kind;
+ bool hasSubLinks = false;
+
+ /* Determine the WCO kind based on command type */
+ if (cmd == CMD_INSERT)
+ {
+ wco_kind = WCO_RLS_INSERT_CHECK;
+ }
+ else if (cmd == CMD_UPDATE)
+ {
+ wco_kind = WCO_RLS_UPDATE_CHECK;
+ }
+ else
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg_internal("unexpected command type for setup_wcos")));
+ }
+
+ rel = resultRelInfo->ri_RelationDesc;
+
+ /*
+ * Use rt_index=1 since we're evaluating policies against a single
relation.
+ * Policy quals are stored with varno=1, and we set ecxt_scantuple to the
+ * tuple we want to check, so keeping varno=1 is correct.
+ */
+ rt_index = 1;
+ user_id = GetUserId();
+
+ /* Get the policies for the specified command type */
+ get_policies_for_relation(rel, cmd, user_id,
+ &permissive_policies,
+ &restrictive_policies);
+
+ /* Build WithCheckOptions from the policies */
+ add_with_check_options(rel, rt_index, wco_kind,
+ permissive_policies,
+ restrictive_policies,
+ &withCheckOptions,
+ &hasSubLinks,
+ false);
+
+ /* Compile the WCO expressions */
+ foreach(lc, withCheckOptions)
+ {
+ WithCheckOption *wco = lfirst_node(WithCheckOption, lc);
+ ExprState *wcoExpr;
+
+ /* Ensure qual is a List for ExecInitQual */
+ if (!IsA(wco->qual, List))
+ {
+ wco->qual = (Node *) list_make1(wco->qual);
+ }
+
+ wcoExpr = ExecInitQual((List *) wco->qual, (PlanState *) node);
+ wcoExprs = lappend(wcoExprs, wcoExpr);
+ }
+
+ /* Set up the ResultRelInfo with WCOs */
+ resultRelInfo->ri_WithCheckOptions = withCheckOptions;
+ resultRelInfo->ri_WithCheckOptionExprs = wcoExprs;
+}
+
+/*
+ * get_policies_for_relation
+ *
+ * Returns lists of permissive and restrictive policies to be applied to the
+ * specified relation, based on the command type and role.
+ *
+ * This includes any policies added by extensions.
+ *
+ * Copied from PostgreSQL's src/backend/rewrite/rowsecurity.c
+ */
+static void
+get_policies_for_relation(Relation relation, CmdType cmd, Oid user_id,
+ List **permissive_policies,
+ List **restrictive_policies)
+{
+ ListCell *item;
+
+ *permissive_policies = NIL;
+ *restrictive_policies = NIL;
+
Review Comment:
Missing NULL check for rel->rd_rsdesc before accessing it. If the relation
does not have RLS configured, rd_rsdesc may be NULL, which would cause a
segmentation fault when accessing rd_rsdesc->policies.
```suggestion
/* If there is no RLS descriptor, there are no policies to apply. */
if (relation->rd_rsdesc == NULL)
return;
```
##########
src/backend/executor/cypher_delete.c:
##########
@@ -447,6 +450,28 @@ static void process_delete_list(CustomScanState *node)
continue;
}
+ /* Check RLS security quals (USING policy) before delete */
+ if (check_enable_rls(resultRelInfo->ri_RelationDesc->rd_id,
+ InvalidOid, true) == RLS_ENABLED)
+ {
+ TupleTableSlot *slot;
+ SecurityQualContext *sqc;
+
+ sqc = setup_security_quals(resultRelInfo, estate, node,
CMD_DELETE);
+ slot = ExecInitExtraTupleSlot(
+ estate, RelationGetDescr(resultRelInfo->ri_RelationDesc),
+ &TTSOpsHeapTuple);
+ ExecStoreHeapTuple(heap_tuple, slot, false);
+
+ /* Silently skip if USING policy filters out this row */
+ if (!check_security_quals(sqc, slot, econtext))
Review Comment:
The setup_security_quals function is called inside the loop for every tuple
being deleted. This is inefficient as the security quals setup only depends on
the relation and command type, not on individual tuples. The function should be
called once before the loop and reused for all tuples.
```suggestion
/*
* Initialize RLS security quals context and evaluation slot
once,
* then reuse them for all tuples processed by this node. The
* setup only depends on the relation and command type, not on
* individual tuples.
*/
static bool rls_sqc_initialized = false;
static SecurityQualContext *cached_sqc = NULL;
static TupleTableSlot *cached_slot = NULL;
if (!rls_sqc_initialized)
{
cached_sqc = setup_security_quals(resultRelInfo,
estate,
node,
CMD_DELETE);
cached_slot = ExecInitExtraTupleSlot(
estate,
RelationGetDescr(resultRelInfo->ri_RelationDesc),
&TTSOpsHeapTuple);
rls_sqc_initialized = true;
}
ExecStoreHeapTuple(heap_tuple, cached_slot, false);
/* Silently skip if USING policy filters out this row */
if (!check_security_quals(cached_sqc, cached_slot, econtext))
```
##########
src/backend/executor/cypher_utils.c:
##########
@@ -268,3 +297,715 @@ HeapTuple insert_entity_tuple_cid(ResultRelInfo
*resultRelInfo,
return tuple;
}
+
+/*
+ * setup_wcos
+ *
+ * WithCheckOptions are added during the rewrite phase, but since AGE uses
+ * CMD_SELECT for all queries, WCOs don't get added for CREATE/SET/MERGE
+ * operations. This function compensates by adding WCOs at execution time.
+ *
+ * Based on PostgreSQL's row security implementation in rowsecurity.c
+ */
+void setup_wcos(ResultRelInfo *resultRelInfo, EState *estate,
+ CustomScanState *node, CmdType cmd)
+{
+ List *permissive_policies;
+ List *restrictive_policies;
+ List *withCheckOptions = NIL;
+ List *wcoExprs = NIL;
+ ListCell *lc;
+ Relation rel;
+ Oid user_id;
+ int rt_index;
+ WCOKind wco_kind;
+ bool hasSubLinks = false;
+
+ /* Determine the WCO kind based on command type */
+ if (cmd == CMD_INSERT)
+ {
+ wco_kind = WCO_RLS_INSERT_CHECK;
+ }
+ else if (cmd == CMD_UPDATE)
+ {
+ wco_kind = WCO_RLS_UPDATE_CHECK;
+ }
+ else
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg_internal("unexpected command type for setup_wcos")));
+ }
+
+ rel = resultRelInfo->ri_RelationDesc;
+
+ /*
+ * Use rt_index=1 since we're evaluating policies against a single
relation.
+ * Policy quals are stored with varno=1, and we set ecxt_scantuple to the
+ * tuple we want to check, so keeping varno=1 is correct.
+ */
+ rt_index = 1;
+ user_id = GetUserId();
+
+ /* Get the policies for the specified command type */
+ get_policies_for_relation(rel, cmd, user_id,
+ &permissive_policies,
+ &restrictive_policies);
+
+ /* Build WithCheckOptions from the policies */
+ add_with_check_options(rel, rt_index, wco_kind,
+ permissive_policies,
+ restrictive_policies,
+ &withCheckOptions,
+ &hasSubLinks,
+ false);
+
+ /* Compile the WCO expressions */
+ foreach(lc, withCheckOptions)
+ {
+ WithCheckOption *wco = lfirst_node(WithCheckOption, lc);
+ ExprState *wcoExpr;
+
+ /* Ensure qual is a List for ExecInitQual */
+ if (!IsA(wco->qual, List))
+ {
+ wco->qual = (Node *) list_make1(wco->qual);
+ }
+
+ wcoExpr = ExecInitQual((List *) wco->qual, (PlanState *) node);
+ wcoExprs = lappend(wcoExprs, wcoExpr);
+ }
+
+ /* Set up the ResultRelInfo with WCOs */
+ resultRelInfo->ri_WithCheckOptions = withCheckOptions;
+ resultRelInfo->ri_WithCheckOptionExprs = wcoExprs;
+}
+
+/*
+ * get_policies_for_relation
+ *
+ * Returns lists of permissive and restrictive policies to be applied to the
+ * specified relation, based on the command type and role.
+ *
+ * This includes any policies added by extensions.
+ *
+ * Copied from PostgreSQL's src/backend/rewrite/rowsecurity.c
+ */
+static void
+get_policies_for_relation(Relation relation, CmdType cmd, Oid user_id,
+ List **permissive_policies,
+ List **restrictive_policies)
+{
+ ListCell *item;
+
+ *permissive_policies = NIL;
+ *restrictive_policies = NIL;
+
+ /* First find all internal policies for the relation. */
+ foreach(item, relation->rd_rsdesc->policies)
+ {
+ bool cmd_matches = false;
+ RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+
+ /* Always add ALL policies, if they exist. */
+ if (policy->polcmd == '*')
+ cmd_matches = true;
+ else
+ {
+ /* Check whether the policy applies to the specified
command type */
+ switch (cmd)
+ {
+ case CMD_SELECT:
+ if (policy->polcmd == ACL_SELECT_CHR)
+ cmd_matches = true;
+ break;
+ case CMD_INSERT:
+ if (policy->polcmd == ACL_INSERT_CHR)
+ cmd_matches = true;
+ break;
+ case CMD_UPDATE:
+ if (policy->polcmd == ACL_UPDATE_CHR)
+ cmd_matches = true;
+ break;
+ case CMD_DELETE:
+ if (policy->polcmd == ACL_DELETE_CHR)
+ cmd_matches = true;
+ break;
+ case CMD_MERGE:
+
+ /*
+ * We do not support a separate policy
for MERGE command.
+ * Instead it derives from the policies
defined for other
+ * commands.
+ */
+ break;
+ default:
+ elog(ERROR, "unrecognized policy
command type %d",
+ (int) cmd);
+ break;
+ }
+ }
+
+ /*
+ * Add this policy to the relevant list of policies if it
applies to
+ * the specified role.
+ */
+ if (cmd_matches && check_role_for_policy(policy->roles,
user_id))
+ {
+ if (policy->permissive)
+ *permissive_policies =
lappend(*permissive_policies, policy);
+ else
+ *restrictive_policies =
lappend(*restrictive_policies, policy);
+ }
+ }
+
+ /*
+ * We sort restrictive policies by name so that any WCOs they generate
are
+ * checked in a well-defined order.
+ */
+ sort_policies_by_name(*restrictive_policies);
+
+ /*
+ * Then add any permissive or restrictive policies defined by
extensions.
+ * These are simply appended to the lists of internal policies, if they
+ * apply to the specified role.
+ */
+ if (row_security_policy_hook_restrictive)
+ {
+ List *hook_policies =
+ (*row_security_policy_hook_restrictive) (cmd, relation);
+
+ /*
+ * As with built-in restrictive policies, we sort any
hook-provided
+ * restrictive policies by name also. Note that we also
intentionally
+ * always check all built-in restrictive policies, in name
order,
+ * before checking restrictive policies added by hooks, in name
order.
+ */
+ sort_policies_by_name(hook_policies);
+
+ foreach(item, hook_policies)
+ {
+ RowSecurityPolicy *policy = (RowSecurityPolicy *)
lfirst(item);
+
+ if (check_role_for_policy(policy->roles, user_id))
+ *restrictive_policies =
lappend(*restrictive_policies, policy);
+ }
+ }
+
+ if (row_security_policy_hook_permissive)
+ {
+ List *hook_policies =
+ (*row_security_policy_hook_permissive) (cmd, relation);
+
+ foreach(item, hook_policies)
+ {
+ RowSecurityPolicy *policy = (RowSecurityPolicy *)
lfirst(item);
+
+ if (check_role_for_policy(policy->roles, user_id))
+ *permissive_policies =
lappend(*permissive_policies, policy);
+ }
+ }
+}
+
+/*
+ * add_with_check_options
+ *
+ * Add WithCheckOptions of the specified kind to check that new records
+ * added by an INSERT or UPDATE are consistent with the specified RLS
+ * policies. Normally new data must satisfy the WITH CHECK clauses from the
+ * policies. If a policy has no explicit WITH CHECK clause, its USING clause
+ * is used instead. In the special case of an UPDATE arising from an
+ * INSERT ... ON CONFLICT DO UPDATE, existing records are first checked using
+ * a WCO_RLS_CONFLICT_CHECK WithCheckOption, which always uses the USING
+ * clauses from RLS policies.
+ *
+ * New WCOs are added to withCheckOptions, and hasSubLinks is set to true if
+ * any of the check clauses added contain sublink subqueries.
+ *
+ * Copied from PostgreSQL's src/backend/rewrite/rowsecurity.c
+ */
+static void
+add_with_check_options(Relation rel,
+ int rt_index,
+ WCOKind kind,
+ List *permissive_policies,
+ List *restrictive_policies,
+ List **withCheckOptions,
+ bool *hasSubLinks,
+ bool force_using)
+{
+ ListCell *item;
+ List *permissive_quals = NIL;
+
+#define QUAL_FOR_WCO(policy) \
+ ( !force_using && \
+ (policy)->with_check_qual != NULL ? \
+ (policy)->with_check_qual : (policy)->qual )
+
+ /*
+ * First collect up the permissive policy clauses, similar to
+ * add_security_quals.
+ */
+ foreach(item, permissive_policies)
+ {
+ RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+ Expr *qual = QUAL_FOR_WCO(policy);
+
+ if (qual != NULL)
+ {
+ permissive_quals = lappend(permissive_quals,
copyObject(qual));
+ *hasSubLinks |= policy->hassublinks;
+ }
+ }
+
+ /*
+ * There must be at least one permissive qual found or no rows are
allowed
+ * to be added. This is the same as in add_security_quals.
+ *
+ * If there are no permissive_quals then we fall through and return a
+ * single 'false' WCO, preventing all new rows.
+ */
+ if (permissive_quals != NIL)
+ {
+ /*
+ * Add a single WithCheckOption for all the permissive policy
clauses,
+ * combining them together using OR. This check has no policy
name,
+ * since if the check fails it means that no policy granted
permission
+ * to perform the update, rather than any particular policy
being
+ * violated.
+ */
+ WithCheckOption *wco;
+
+ wco = makeNode(WithCheckOption);
+ wco->kind = kind;
+ wco->relname = pstrdup(RelationGetRelationName(rel));
+ wco->polname = NULL;
+ wco->cascaded = false;
+
+ if (list_length(permissive_quals) == 1)
+ wco->qual = (Node *) linitial(permissive_quals);
+ else
+ wco->qual = (Node *) makeBoolExpr(OR_EXPR,
permissive_quals, -1);
+
+ ChangeVarNodes(wco->qual, 1, rt_index, 0);
+
+ *withCheckOptions = list_append_unique(*withCheckOptions, wco);
+
+ /*
+ * Now add WithCheckOptions for each of the restrictive policy
clauses
+ * (which will be combined together using AND). We use a
separate
+ * WithCheckOption for each restrictive policy to allow the
policy
+ * name to be included in error reports if the policy is
violated.
+ */
+ foreach(item, restrictive_policies)
+ {
+ RowSecurityPolicy *policy = (RowSecurityPolicy *)
lfirst(item);
+ Expr *qual = QUAL_FOR_WCO(policy);
+
+ if (qual != NULL)
+ {
+ qual = copyObject(qual);
+ ChangeVarNodes((Node *) qual, 1, rt_index, 0);
+
+ wco = makeNode(WithCheckOption);
+ wco->kind = kind;
+ wco->relname =
pstrdup(RelationGetRelationName(rel));
+ wco->polname = pstrdup(policy->policy_name);
+ wco->qual = (Node *) qual;
+ wco->cascaded = false;
+
+ *withCheckOptions =
list_append_unique(*withCheckOptions, wco);
+ *hasSubLinks |= policy->hassublinks;
+ }
+ }
+ }
+ else
+ {
+ /*
+ * If there were no policy clauses to check new data, add a
single
+ * always-false WCO (a default-deny policy).
+ */
+ WithCheckOption *wco;
+
+ wco = makeNode(WithCheckOption);
+ wco->kind = kind;
+ wco->relname = pstrdup(RelationGetRelationName(rel));
+ wco->polname = NULL;
+ wco->qual = (Node *) makeConst(BOOLOID, -1, InvalidOid,
+
sizeof(bool), BoolGetDatum(false),
+
false, true);
+ wco->cascaded = false;
+
+ *withCheckOptions = lappend(*withCheckOptions, wco);
+ }
+}
+
+/*
+ * sort_policies_by_name
+ *
+ * This is only used for restrictive policies, ensuring that any
+ * WithCheckOptions they generate are applied in a well-defined order.
+ * This is not necessary for permissive policies, since they are all combined
+ * together using OR into a single WithCheckOption check.
+ *
+ * Copied from PostgreSQL's src/backend/rewrite/rowsecurity.c
+ */
+static void
+sort_policies_by_name(List *policies)
+{
+ list_sort(policies, row_security_policy_cmp);
+}
+
+/*
+ * list_sort comparator to sort RowSecurityPolicy entries by name
+ *
+ * Copied from PostgreSQL's src/backend/rewrite/rowsecurity.c
+ */
+static int
+row_security_policy_cmp(const ListCell *a, const ListCell *b)
+{
+ const RowSecurityPolicy *pa = (const RowSecurityPolicy *) lfirst(a);
+ const RowSecurityPolicy *pb = (const RowSecurityPolicy *) lfirst(b);
+
+ /* Guard against NULL policy names from extensions */
+ if (pa->policy_name == NULL)
+ return pb->policy_name == NULL ? 0 : 1;
+ if (pb->policy_name == NULL)
+ return -1;
+
+ return strcmp(pa->policy_name, pb->policy_name);
+}
+
+/*
+ * check_role_for_policy -
+ * determines if the policy should be applied for the current role
+ *
+ * Copied from PostgreSQL's src/backend/rewrite/rowsecurity.c
+ */
+static bool
+check_role_for_policy(ArrayType *policy_roles, Oid user_id)
+{
+ int i;
+ Oid *roles = (Oid *) ARR_DATA_PTR(policy_roles);
+
+ /* Quick fall-thru for policies applied to all roles */
+ if (roles[0] == ACL_ID_PUBLIC)
+ return true;
+
+ for (i = 0; i < ARR_DIMS(policy_roles)[0]; i++)
+ {
+ if (has_privs_of_role(user_id, roles[i]))
+ return true;
+ }
+
+ return false;
+}
+
+/*
+ * add_security_quals
+ *
+ * Add security quals to enforce the specified RLS policies, restricting
+ * access to existing data in a table. If there are no policies controlling
+ * access to the table, then all access is prohibited --- i.e., an implicit
+ * default-deny policy is used.
+ *
+ * New security quals are added to securityQuals, and hasSubLinks is set to
+ * true if any of the quals added contain sublink subqueries.
+ *
+ * Copied from PostgreSQL's src/backend/rewrite/rowsecurity.c
+ */
+static void
+add_security_quals(int rt_index,
+ List *permissive_policies,
+ List *restrictive_policies,
+ List **securityQuals,
+ bool *hasSubLinks)
+{
+ ListCell *item;
+ List *permissive_quals = NIL;
+ Expr *rowsec_expr;
+
+ /*
+ * First collect up the permissive quals. If we do not find any
+ * permissive policies then no rows are visible (this is handled below).
+ */
+ foreach(item, permissive_policies)
+ {
+ RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+
+ if (policy->qual != NULL)
+ {
+ permissive_quals = lappend(permissive_quals,
+
copyObject(policy->qual));
+ *hasSubLinks |= policy->hassublinks;
+ }
+ }
+
+ /*
+ * We must have permissive quals, always, or no rows are visible.
+ *
+ * If we do not, then we simply return a single 'false' qual which
results
+ * in no rows being visible.
+ */
+ if (permissive_quals != NIL)
+ {
+ /*
+ * We now know that permissive policies exist, so we can now add
+ * security quals based on the USING clauses from the
restrictive
+ * policies. Since these need to be combined together using
AND, we
+ * can just add them one at a time.
+ */
+ foreach(item, restrictive_policies)
+ {
+ RowSecurityPolicy *policy = (RowSecurityPolicy *)
lfirst(item);
+ Expr *qual;
+
+ if (policy->qual != NULL)
+ {
+ qual = copyObject(policy->qual);
+ ChangeVarNodes((Node *) qual, 1, rt_index, 0);
+
+ *securityQuals =
list_append_unique(*securityQuals, qual);
+ *hasSubLinks |= policy->hassublinks;
+ }
+ }
+
+ /*
+ * Then add a single security qual combining together the USING
+ * clauses from all the permissive policies using OR.
+ */
+ if (list_length(permissive_quals) == 1)
+ rowsec_expr = (Expr *) linitial(permissive_quals);
+ else
+ rowsec_expr = makeBoolExpr(OR_EXPR, permissive_quals,
-1);
+
+ ChangeVarNodes((Node *) rowsec_expr, 1, rt_index, 0);
+ *securityQuals = list_append_unique(*securityQuals,
rowsec_expr);
+ }
+ else
+
+ /*
+ * A permissive policy must exist for rows to be visible at all.
+ * Therefore, if there were no permissive policies found,
return a
+ * single always-false clause.
+ */
+ *securityQuals = lappend(*securityQuals,
+
makeConst(BOOLOID, -1, InvalidOid,
+
sizeof(bool), BoolGetDatum(false),
+
false, true));
+}
+
+/*
+ * setup_security_quals
+ *
+ * Security quals (USING policies) are added during the rewrite phase, but
+ * since AGE uses CMD_SELECT for all queries, they don't get added for
+ * UPDATE/DELETE operations. This function sets up security quals at
+ * execution time to be evaluated against each tuple before modification.
+ */
+SecurityQualContext *
+setup_security_quals(ResultRelInfo *resultRelInfo, EState *estate,
+ CustomScanState *node, CmdType cmd)
+{
+ List *permissive_policies;
+ List *restrictive_policies;
+ List *securityQuals = NIL;
+ SecurityQualContext *context;
+ ListCell *lc;
+ Relation rel;
+ Oid user_id;
+ int rt_index;
+ bool hasSubLinks = false;
+
+ /* Only UPDATE and DELETE have security quals */
+ if (cmd != CMD_UPDATE && cmd != CMD_DELETE)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg_internal("unexpected command type for
setup_security_quals")));
+ }
+
+ context = palloc0(sizeof(SecurityQualContext));
+ context->qualExprs = NIL;
+
+ rel = resultRelInfo->ri_RelationDesc;
+
+ /* If no RLS policies exist, return empty context */
+ if (rel->rd_rsdesc == NULL)
+ {
+ return context;
+ }
+
+ /*
+ * Use rt_index=1 since we're evaluating policies against a single
relation.
+ * Policy quals are stored with varno=1, and we set ecxt_scantuple to the
+ * tuple we want to check, so keeping varno=1 is correct.
+ */
+ rt_index = 1;
+ user_id = GetUserId();
+
+ /* Get the policies for the specified command type */
+ get_policies_for_relation(rel, cmd, user_id,
+ &permissive_policies,
+ &restrictive_policies);
+
+ /* Build security quals from the policies */
+ add_security_quals(rt_index, permissive_policies, restrictive_policies,
+ &securityQuals, &hasSubLinks);
+
+ /* Compile the security qual expressions */
+ foreach(lc, securityQuals)
+ {
+ Expr *qual = (Expr *) lfirst(lc);
+ ExprState *qualExpr;
+
+ /* Ensure qual is a List for ExecInitQual */
+ if (!IsA(qual, List))
+ {
+ qual = (Expr *) list_make1(qual);
+ }
+
+ qualExpr = ExecInitQual((List *) qual, (PlanState *) node);
+ context->qualExprs = lappend(context->qualExprs, qualExpr);
+ }
+
+ context->relname = pstrdup(RelationGetRelationName(rel));
+
+ return context;
Review Comment:
The SecurityQualContext structure is allocated but never freed in the
calling functions. This creates a memory leak as the context is set up for each
UPDATE/DELETE operation but never released. Consider either documenting that
the caller is responsible for cleanup, or ensuring the memory is allocated in a
memory context that will be freed appropriately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
