From 51907810b27f3856396d8123047646ad46c0e125 Mon Sep 17 00:00:00 2001 From: Eeshwar Date: Tue, 4 Aug 2026 00:57:05 +0000 Subject: [PATCH] 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)),