Copilot commented on code in PR #2561:
URL: https://github.com/apache/age/pull/2561#discussion_r3929768149
##########
src/backend/utils/adt/age_global_graph.c:
##########
@@ -724,6 +786,279 @@ static bool insert_vertex_edge(GRAPH_global_context
*ggctx,
return false;
}
+/*
+ * RLS-aware loader for a single vertex label table.
+ *
+ * Loads (id, ctid) through SPI so the planner applies row-level security
+ * policies (and table/column ACL) for the current role, then inserts each
+ * visible row into the graph's global vertex hashtable. The physical tuple
+ * location (ctid) is stored for the same lazy property fetch the direct-scan
+ * path uses (get_vertex_entry_properties); because only RLS-visible rows are
+ * loaded, that later heap_fetch only ever reads authorized tuples. Used in
+ * place of the direct heap scan when age.enforce_rls_in_traversal is on and
RLS
+ * is active on the label table. Rows are fetched in batches so the entire
label
+ * table is never materialized at once.
+ */
+static void load_vertex_label_rls(GRAPH_global_context *ggctx,
+ Oid vertex_label_table_oid,
+ char *vertex_label_name)
+{
+ MemoryContext ggctx_cxt = CurrentMemoryContext;
+ StringInfoData query;
+ SPIPlanPtr plan;
+ Portal portal;
+
+ if (SPI_connect() != SPI_OK_CONNECT)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg("load_vertex_label_rls: SPI_connect failed")));
+ }
+
+ /*
+ * Build the query text AFTER SPI_connect(): SPI switches to its own
+ * procedure memory context here, so the StringInfo buffer and the
+ * quote_qualified_identifier() result are allocated in that context and
+ * freed by SPI_finish(), instead of leaking into the caller's long-lived
+ * (TopMemoryContext) cache-build context that is re-entered on every RLS
+ * cache rebuild.
+ */
+ initStringInfo(&query);
+ appendStringInfo(&query, "SELECT id, ctid FROM ONLY %s",
+ quote_qualified_identifier(ggctx->graph_name,
+ vertex_label_name));
+
+ plan = SPI_prepare(query.data, 0, NULL);
+ if (plan == NULL)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg("load_vertex_label_rls: SPI_prepare failed: %s",
+ SPI_result_code_string(SPI_result))));
+ }
+
+ /* read-only cursor: reuses the active snapshot; RLS applied by planner */
+ portal = SPI_cursor_open(NULL, plan, NULL, NULL, true);
Review Comment:
SPI_cursor_open() can return NULL (and set SPI_result) on failure; the
subsequent SPI_cursor_fetch()/SPI_cursor_close() would then dereference a NULL
portal and likely crash. Add an explicit NULL check and raise an ERROR with
SPI_result_code_string(SPI_result).
This issue also appears on line 929 of the same file.
--
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]
