From 51907810b27f3856396d8123047646ad46c0e125 Mon Sep 17 00:00:00 2001 From: Eeshwar Date: Tue, 4 Aug 2026 00:57:05 +0000 Subject: [PATCH 1/2] Fix drop_label privilege check for non-superusers range_var_callback_for_remove_relation() called object_ownercheck() with the wrong argument order for the PG16+ API object_ownercheck(classid, objectid, roleid). It passed rel_oid as the classid and a namespace OID as the objectid, so the classid lookup hit the default case and raised "unrecognized class ID" for any non-superuser dropping a graph label. Superusers were unaffected because object_ownercheck() early-returns via superuser_arg(). Pass RelationRelationId as the classid and rel_oid as the objectid, matching upstream PostgreSQL's RangeVarCallbackForDropRelation. Add a regression test in security.sql covering a NOSUPERUSER role that owns a graph/label and successfully runs drop_label. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- regress/expected/security.out | 58 +++++++++++++++++++++++++++ regress/sql/security.sql | 34 ++++++++++++++++ src/backend/commands/label_commands.c | 2 +- 3 files changed, 93 insertions(+), 1 deletion(-) diff --git a/regress/expected/security.out b/regress/expected/security.out index 59e58cb05..eaa3c956e 100644 --- a/regress/expected/security.out +++ b/regress/expected/security.out @@ -1655,3 +1655,61 @@ NOTICE: graph "rls_graph" has been dropped (1 row) +-- ============================================================================ +-- NON-SUPERUSER drop_label REGRESSION TEST +-- +-- Regression test for object_ownercheck() argument order in +-- range_var_callback_for_remove_relation(). A non-superuser that OWNS a +-- graph/label previously failed drop_label() with "unrecognized class ID" +-- because rel_oid was passed as the classid instead of RelationRelationId. +-- ============================================================================ +DROP ROLE IF EXISTS age_nonsuper; +NOTICE: role "age_nonsuper" does not exist, skipping +CREATE ROLE age_nonsuper LOGIN NOSUPERUSER; +-- create_graph() creates a new schema in the current database, so the role +-- needs CREATE on the database; managing labels needs USAGE + CREATE on +-- ag_catalog. Grant CREATE on whatever database the tests run in. +SELECT format('GRANT CREATE ON DATABASE %I TO age_nonsuper', current_database()) +\gexec +GRANT CREATE ON DATABASE contrib_regression TO age_nonsuper +GRANT USAGE ON SCHEMA ag_catalog TO age_nonsuper; +GRANT CREATE ON SCHEMA ag_catalog TO age_nonsuper; +-- Reproduce as the non-superuser role: it creates (and therefore OWNS) the +-- graph and the label, then drops the label. Before the fix this raised +-- "unrecognized class ID"; after the fix the label is dropped successfully. +SET ROLE age_nonsuper; +SELECT create_graph('repro_graph'); +NOTICE: graph "repro_graph" has been created + create_graph +-------------- + +(1 row) + +SELECT create_vlabel('repro_graph', 'repro_label'); +NOTICE: VLabel "repro_label" has been created + create_vlabel +--------------- + +(1 row) + +SELECT drop_label('repro_graph', 'repro_label'); +NOTICE: label "repro_graph"."repro_label" has been dropped + drop_label +------------ + +(1 row) + +SELECT drop_graph('repro_graph', true); +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to table repro_graph._ag_label_vertex +drop cascades to table repro_graph._ag_label_edge +NOTICE: graph "repro_graph" has been dropped + drop_graph +------------ + +(1 row) + +RESET ROLE; +-- Cleanup +DROP OWNED BY age_nonsuper CASCADE; +DROP ROLE age_nonsuper; diff --git a/regress/sql/security.sql b/regress/sql/security.sql index 344dd23d4..af5b3ccb6 100644 --- a/regress/sql/security.sql +++ b/regress/sql/security.sql @@ -1449,3 +1449,37 @@ DROP ROLE rls_admin; -- Drop test graph SELECT drop_graph('rls_graph', true); + +-- ============================================================================ +-- NON-SUPERUSER drop_label REGRESSION TEST +-- +-- Regression test for object_ownercheck() argument order in +-- range_var_callback_for_remove_relation(). A non-superuser that OWNS a +-- graph/label previously failed drop_label() with "unrecognized class ID" +-- because rel_oid was passed as the classid instead of RelationRelationId. +-- ============================================================================ + +DROP ROLE IF EXISTS age_nonsuper; +CREATE ROLE age_nonsuper LOGIN NOSUPERUSER; + +-- create_graph() creates a new schema in the current database, so the role +-- needs CREATE on the database; managing labels needs USAGE + CREATE on +-- ag_catalog. Grant CREATE on whatever database the tests run in. +SELECT format('GRANT CREATE ON DATABASE %I TO age_nonsuper', current_database()) +\gexec +GRANT USAGE ON SCHEMA ag_catalog TO age_nonsuper; +GRANT CREATE ON SCHEMA ag_catalog TO age_nonsuper; + +-- Reproduce as the non-superuser role: it creates (and therefore OWNS) the +-- graph and the label, then drops the label. Before the fix this raised +-- "unrecognized class ID"; after the fix the label is dropped successfully. +SET ROLE age_nonsuper; +SELECT create_graph('repro_graph'); +SELECT create_vlabel('repro_graph', 'repro_label'); +SELECT drop_label('repro_graph', 'repro_label'); +SELECT drop_graph('repro_graph', true); +RESET ROLE; + +-- Cleanup +DROP OWNED BY age_nonsuper CASCADE; +DROP ROLE age_nonsuper; diff --git a/src/backend/commands/label_commands.c b/src/backend/commands/label_commands.c index ac789ecce..bc8d63dab 100644 --- a/src/backend/commands/label_commands.c +++ b/src/backend/commands/label_commands.c @@ -1036,7 +1036,7 @@ static void range_var_callback_for_remove_relation(const RangeVar *rel, /* relkind == expected_relkind */ - if (!object_ownercheck(rel_oid, get_rel_namespace(rel_oid), GetUserId())) + if (!object_ownercheck(RelationRelationId, rel_oid, GetUserId())) { aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(rel_oid)), From 0ef455d1cdf7891e528f8106ccb3762bc630edc9 Mon Sep 17 00:00:00 2001 From: Eeshwar Date: Thu, 6 Aug 2026 19:04:44 +0000 Subject: [PATCH 2/2] Make drop_label regression test output db-name independent Replace the SELECT ... \gexec dynamic GRANT with a DO block that runs EXECUTE format(...) internally. The \gexec approach echoed the expanded GRANT (hardcoding the regression database name) into the expected output, making the test brittle across harnesses. The DO block grants on current_database() while emitting a deterministic 'DO'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- regress/expected/security.out | 9 ++++++--- regress/sql/security.sql | 8 ++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/regress/expected/security.out b/regress/expected/security.out index eaa3c956e..0433b214f 100644 --- a/regress/expected/security.out +++ b/regress/expected/security.out @@ -1669,9 +1669,12 @@ CREATE ROLE age_nonsuper LOGIN NOSUPERUSER; -- create_graph() creates a new schema in the current database, so the role -- needs CREATE on the database; managing labels needs USAGE + CREATE on -- ag_catalog. Grant CREATE on whatever database the tests run in. -SELECT format('GRANT CREATE ON DATABASE %I TO age_nonsuper', current_database()) -\gexec -GRANT CREATE ON DATABASE contrib_regression TO age_nonsuper +DO $$ +BEGIN + EXECUTE format('GRANT CREATE ON DATABASE %I TO age_nonsuper', + current_database()); +END +$$; GRANT USAGE ON SCHEMA ag_catalog TO age_nonsuper; GRANT CREATE ON SCHEMA ag_catalog TO age_nonsuper; -- Reproduce as the non-superuser role: it creates (and therefore OWNS) the diff --git a/regress/sql/security.sql b/regress/sql/security.sql index af5b3ccb6..82b1cbd72 100644 --- a/regress/sql/security.sql +++ b/regress/sql/security.sql @@ -1465,8 +1465,12 @@ CREATE ROLE age_nonsuper LOGIN NOSUPERUSER; -- create_graph() creates a new schema in the current database, so the role -- needs CREATE on the database; managing labels needs USAGE + CREATE on -- ag_catalog. Grant CREATE on whatever database the tests run in. -SELECT format('GRANT CREATE ON DATABASE %I TO age_nonsuper', current_database()) -\gexec +DO $$ +BEGIN + EXECUTE format('GRANT CREATE ON DATABASE %I TO age_nonsuper', + current_database()); +END +$$; GRANT USAGE ON SCHEMA ag_catalog TO age_nonsuper; GRANT CREATE ON SCHEMA ag_catalog TO age_nonsuper;