fix(auth): reject reserved role-permission names as database names + complete cluster_user handling

[kylebernhardy]

Summary

A role's permission object mixes named flags — super_user, cluster_user, structure_user, operations, _expandedOperations — with per-database permission blocks keyed by database name. A database named after one of those flags collides with it: the role parser cannot tell a super_user flag from a super_user database's permissions.

Purpose

Fixes #1016. Beyond "undefined behavior," there's a concrete privilege-escalation path (surfaced in review): a component shipping @table(database: "super_user") plus a roles.yaml granting permissions on that "database" makes permission.super_user a truthy object, and the super-user check (!!permission.super_user) then reads as granted.

The fix

Reject the reserved names as database identifiers on both creation surfaces, from one shared source of truth (RESERVED_DATABASE_NAMES in hdbTerms.ts):

1. Operations API (create_schema / create_table) — .invalid(...) on DB_NAME_CONSTRAINTS in dataLayer/schema.ts.
2. Schema authoring (schema.graphql @table(database:) / programmatic table()) — a check in resources/databases.ts::table(), which registers databases and bypasses the operations validation. This is the path the escalation uses.

It also folds the incomplete USERS_NOT_DBS in resources/roles.ts (was ['super_user', 'structure_user'] — missing cluster_user/operations) into the same shared constant, fixing a latent bug where a roles.yaml using those flags mis-parsed them as database names.

Where to look

• Enforcement at two layers. The operations-only version was incomplete — the graphql/programmatic path was the reviewers' blocker. table() throws a ClientError (matching the existing schema-validation throws in graphql.ts), so it's load-safe: an invalid schema fails to load rather than bricking the instance.
• Reserved set completeness. _expandedOperations is included (an internal, _-prefixed key) because schemaRegex permits leading underscores and a same-named database would overwrite the cached operations Set at translation time (a TypeError crash). Confirm the set matches UserRoleNamedPermissions in security/user.ts.
• Case sensitivity. .invalid() matches exact strings; database names are currently case-sensitive (case-insensitive handling is separate issue Case-insensitive schema and table name handling #1028), so exact match is correct.

Tests

New unitTests/dataLayer/reservedSchemaNames.test.js — each reserved name × three surfaces (create_schema, create_table, table()), plus a non-reserved control. 15 of 16 fail on origin/main (verified by reverting the source files). Existing resources/databases, query, crud, defineTable-registration, permissions, permissionsTranslator suites pass unchanged.

Also fixed (related cluster_user gaps, second commit)

Three pre-existing gaps in how cluster_user is handled as a role flag, surfaced in review:

• role_validation.ts misleading error — its boolean-flag list came from ROLE_TYPES_ENUM (only super_user), so a non-boolean cluster_user skipped the boolean check and fell through to database validation, reporting database 'cluster_user' does not exist instead of the boolean-type error the SU_CU_ROLE_BOOLEAN_ERROR name already implies. Fixed by adding CLUSTER_USER to ROLE_TYPES_ENUM. Guarded by two new role_validation tests (both fail on origin/main).
• permissionsTranslator.js dropped cluster_user — preserved super_user/structure_user/operations but not cluster_user. Now preserved alongside them. Note: cluster_user is read nowhere at runtime today (only a type def + an impersonation-clear write), so this is a consistency/hygiene fix with no behavioral effect — flagging that explicitly.
• Dead SU_CU_ROLE_COMBINED_ERROR removed from the test-error helper — no production counterpart, no test uses it (the su+cu rejection is covered by SU_CU_ROLE_NO_PERMS_ALLOWED).

Upgrade / migration note

Deep-review (auth + data-integrity domain passes) confirmed the fix is startup-safe: a reserved-name database that already exists on disk still loads (schema replay goes through makeTable, not the guarded table()), so its data stays accessible, and it can be dropped to remediate (the drop path goes through database(), also unguarded — which is why the check is deliberately on the authoring path only).

But such a database's component will fail to load after upgrade — the @table(database:) authoring call now throws, so that Resource becomes an ErrorResource (logged, isolated to that component; not fatal to the instance). The highest-risk name is operations, which only became a permission flag recently (95aa06fd1), so pre-existing operations databases are more plausible than super_user ones. Release notes should tell operators to rename any pre-existing database named after a reserved flag before upgrading.

Note

Security-adjacent (the escalation path). Milestone is v5.2; could be a v5.1 patch given the security angle — flagging for a decision rather than labeling.

Generated by an LLM (Claude Opus 4.8).

[gemini-code-assist]

Code Review

This pull request introduces validation to prevent the use of reserved database names (such as super_user, cluster_user, structure_user, operations, and _expandedOperations) that collide with role permission flags. This validation is applied to both the operations API and the schema-authoring path. A new test file is added to verify these constraints. The review feedback points out a style guide violation in the test file where node:assert/strict is imported instead of the plain node:assert module, which is lint-rejected in this repository.

[claude]

Reviewed; no blockers found.

[kylebernhardy]

@kriszyp — a release-targeting question on this one.

The core fix rejects reserved role-permission names (super_user, cluster_user, structure_user, operations, _expandedOperations) as database names, at both the operations API and the schema-authoring path (@table(database:) / table()). The schema-authoring path is the one that matters most: a component declaring @table(database: "super_user") plus a roles.yaml granting permissions on that "database" makes permission.super_user a truthy object, which the super-user check reads as granted — a privilege-escalation.

We're planning to land this on main / v5.2. Given the escalation angle, do you think it also warrants a v5.1 patch (via the patch label / cherry-pick), or are you comfortable v5.2-only? It cherry-picks cleanly as far as I can tell; your call on whether the security aspect justifies backporting.

(This PR also folds in three related pre-existing cluster_user gaps you'd expect to travel with it — detailed in the description.)

Comment generated by kAIle (Claude Opus 4.8)