You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@age.apache.org by "eeshwarg (via GitHub)" <gi...@apache.org> on 2026/08/04 00:58:27 UTC
[PR] Fix drop_label privilege check for non-superusers (unrecognized class ID) [age]
eeshwarg opened a new pull request, #2497:
URL: https://github.com/apache/age/pull/2497
## Problem
Any non-superuser calling `drop_label('graph', 'label')` (or dropping a vertex/edge label) fails with:
```
ERROR: unrecognized class ID: <oid>
```
Superusers are unaffected.
## Root cause
In `src/backend/commands/label_commands.c`, `range_var_callback_for_remove_relation()` calls the PG16+ `object_ownercheck()` with the wrong argument order. The API is:
```c
bool object_ownercheck(Oid classid, Oid objectid, Oid roleid);
```
AGE passed `rel_oid` as `classid` (should be `RelationRelationId`) and a namespace OID as `objectid` (should be `rel_oid`). Since `classid` isn't a real catalog OID, the lookup hits the `default` case in `get_object_property` and raises `unrecognized class ID`. Superusers escape because `object_ownercheck` early-returns via `superuser_arg(roleid)` before the classid lookup.
This was introduced in the PG16 port: before PG16 the code used the correct 2-arg `pg_class_ownercheck(rel_oid, GetUserId())`. The same defect is present on `master`, `PG16`, `PG17`, and `PG18`; PG15 and earlier are unaffected. This PR fixes `master`.
## Fix
```c
- if (!object_ownercheck(rel_oid, get_rel_namespace(rel_oid), GetUserId()))
+ if (!object_ownercheck(RelationRelationId, rel_oid, GetUserId()))
```
This matches upstream PostgreSQL's own `RangeVarCallbackForDropRelation` usage. `RelationRelationId` is already available via existing includes (`catalog/pg_class_d.h`); no new include needed.
## Testing
- Added a regression test in `regress/sql/security.sql`: a `NOSUPERUSER` role creates (and therefore owns) a graph and label, then successfully runs `drop_label`. Verified it **fails** with `ERROR: unrecognized class ID` on the unpatched build and **passes** after the fix.
- `make installcheck` full suite: **42/42 tests pass** (PostgreSQL 18).
Co-authored-by: Copilot <22...@users.noreply.github.com>
--
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: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix drop_label privilege check for non-superusers (unrecognized class ID) [age]
Posted by "eeshwarg (via GitHub)" <gi...@apache.org>.
eeshwarg commented on PR #2497:
URL: https://github.com/apache/age/pull/2497#issuecomment-5222833952
> Verified the argument-order fix directly:
>
> * `object_ownercheck(Oid classid, Oid objectid, Oid roleid)` — confirmed the signature in `utils/acl.h`.
> * `RelationRelationId` is already in scope in this file (used two lines earlier via `ObjectAddressSet` at line 1007) and `catalog/pg_class_d.h` is already included, so no new dependency.
> * The old call passed `rel_oid` as `classid` and a namespace OID as `objectid` — neither is a valid catalog OID for the classid slot, which explains the `unrecognized class ID` error hitting the default case in `get_object_property`. Superusers skip this path entirely via the `superuser_arg()` short-circuit, which is why only non-superusers hit it.
> * Regression test reproduces the failure end-to-end as a non-superuser role that owns the graph/label, and cleans up after itself (drops the role, graph). Good coverage.
>
> Note: CI hasn't run yet on this branch (workflow runs sitting unapproved, same as #2469) — needs a maintainer to approve the run before this can merge.
>
> LGTM.
Thanks for the review and approval Greg! Would you able to merge the PR for me? Or do I need another approval before that?
--
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: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix drop_label privilege check for non-superusers (unrecognized class ID) [age]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2497:
URL: https://github.com/apache/age/pull/2497#discussion_r3715445984
##########
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
Review Comment:
Using `SELECT ... \\gexec` makes the regression output depend on the database name (and on how psql echoes executed statements), which can make the test more brittle across different regression harnesses/configurations. Consider doing the dynamic GRANT inside a `DO $$ ... EXECUTE format(...) ... $$;` block so the output is deterministic (e.g., just `DO`) while still granting on `current_database()`.
--
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: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix drop_label privilege check for non-superusers (unrecognized class ID) [age]
Posted by "jrgemignani (via GitHub)" <gi...@apache.org>.
jrgemignani merged PR #2497:
URL: https://github.com/apache/age/pull/2497
--
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: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org