Skip to content

Check INSERT privileges in graph generators - #2585

Open
rioriost wants to merge 1 commit into
apache:masterfrom
rioriost:codex/age-graph-generation-permissions
Open

rioriost wants to merge 1 commit into
apache:masterfrom
rioriost:codex/age-graph-generation-permissions

Conversation

@rioriost

Copy link
Copy Markdown
Contributor

create_complete_graph and age_create_barbell_graph insert label rows directly. A role with sequence USAGE could add vertices and edges without table INSERT permission, or when forced RLS denies the same operation through SQL.

This change checks schema USAGE and INSERT privileges on both labels before generating IDs. It rejects graph generation when RLS applies, since the direct insert path cannot evaluate its policies. Column-level INSERT grants remain supported.

The new regression test covers both functions, a missing edge grant, column grants, and forced RLS. The standalone reproducer is in tools/graph-generation-security/. On PostgreSQL 18.6, the full AGE regression suite passes (43/43).

@MuhammadTahaNaveed MuhammadTahaNaveed left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: GPT-6-Astra xhigh

The RLS check still allows writes if RLS is enabled while generation waits on a table lock. Reproducer and test cleanup suggestions inline.

Tested on PostgreSQL 18.3: build and all 47 regressions passed.

}

/* These direct writes cannot evaluate INSERT WITH CHECK policies. */
if (check_enable_rls(relid, InvalidOid, true) == RLS_ENABLED)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please lock both label tables before checking RLS and retain the locks through the writes. There is no table lock here, so RLS can change while the generator waits to insert.

Reproduced on PostgreSQL 18.3:

Setup as an administrator:

CREATE EXTENSION IF NOT EXISTS age;
LOAD 'age';
SET search_path = ag_catalog, public;
SELECT create_graph('rls_repro');
SELECT create_vlabel('rls_repro', 'Node');
SELECT create_elabel('rls_repro', 'LINK');
CREATE ROLE rls_writer NOSUPERUSER NOBYPASSRLS;
GRANT USAGE ON SCHEMA ag_catalog, rls_repro TO rls_writer;
GRANT USAGE ON ALL SEQUENCES IN SCHEMA rls_repro TO rls_writer;
GRANT INSERT ON rls_repro."Node", rls_repro."LINK" TO rls_writer;

Session A:

BEGIN;
LOCK TABLE rls_repro."Node" IN ACCESS EXCLUSIVE MODE;

Session B — leave the SELECT waiting:

LOAD 'age';
SET search_path = ag_catalog, public;
SET ROLE rls_writer;
SELECT create_complete_graph('rls_repro', 2, 'LINK', 'Node');

Then in session A:

ALTER TABLE rls_repro."Node" ENABLE ROW LEVEL SECURITY;
ALTER TABLE rls_repro."Node" FORCE ROW LEVEL SECURITY;
COMMIT;

Session B succeeds and commits two vertices and one edge, despite RLS having no INSERT policy. A normal SQL INSERT under the same lock/RLS sequence correctly fails. Please cover this with a two-session regression.

@@ -0,0 +1,65 @@
/* Graph generators must respect table INSERT privileges and RLS. */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move these cases into regress/sql/graph_generation.sql and its expected output. It already covers both generators; there is no need for a separate test file and Makefile entry.

@@ -0,0 +1,104 @@
#!/usr/bin/env python3

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These checks can go in the SQL regression tests using SET ROLE. The Python script isn’t run by CI and only prints results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants