From 6a14dfc5b44b7477d1ba05bb942971e98ae39fde Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Thu, 23 Jul 2026 11:57:45 -0600 Subject: [PATCH 1/5] fix(auth): reject reserved role-permission names as database names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A role's `permission` object keys per-database permission blocks by database name alongside named flags (super_user, cluster_user, structure_user, operations, _expandedOperations). 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 — producing undefined behavior, including a privilege-escalation path: 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. Reject the reserved names as database identifiers on both creation surfaces: - the operations API (`create_schema` / `create_table`), via DB_NAME_CONSTRAINTS - schema authoring (`schema.graphql` `@table(database:)` / programmatic `table()`), which registers databases through resources/databases.ts and bypasses the operations validation The reserved set is defined once in hdbTerms as RESERVED_DATABASE_NAMES and also replaces the incomplete `USERS_NOT_DBS` list in roles.ts (which was missing cluster_user / operations, so a roles.yaml using those flags mis-parsed them as database names). Fixes harper#1016. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- dataLayer/schema.ts | 6 ++- resources/databases.ts | 14 ++++- resources/roles.ts | 6 ++- .../dataLayer/reservedSchemaNames.test.js | 53 +++++++++++++++++++ utility/hdbTerms.ts | 16 ++++++ 5 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 unitTests/dataLayer/reservedSchemaNames.test.js diff --git a/dataLayer/schema.ts b/dataLayer/schema.ts index d5debf43ef..03fd8f8aa3 100644 --- a/dataLayer/schema.ts +++ b/dataLayer/schema.ts @@ -23,7 +23,11 @@ const DB_NAME_CONSTRAINTS = Joi.string() .min(1) .max(commonValidators.schema_length.maximum) .pattern(schemaRegex) - .messages({ 'string.pattern.base': '{:#label} ' + commonValidators.schema_format.message }); + .invalid(...hdbTerms.RESERVED_DATABASE_NAMES) + .messages({ + 'string.pattern.base': '{:#label} ' + commonValidators.schema_format.message, + 'any.invalid': "'{#value}' is a reserved name and cannot be used as a database name", + }); const TABLE_NAME_CONSTRAINTS = Joi.string() .min(1) diff --git a/resources/databases.ts b/resources/databases.ts index 32f141146e..ad4989a62c 100644 --- a/resources/databases.ts +++ b/resources/databases.ts @@ -11,7 +11,12 @@ import { } from '../dataLayer/harperBridge/lmdbBridge/lmdbUtility/initializePaths.js'; import { makeTable } from './Table.ts'; import OpenEnvironmentObject from '../utility/lmdb/OpenEnvironmentObject.ts'; -import { CONFIG_PARAMS, LEGACY_DATABASES_DIR_NAME, DATABASES_DIR_NAME } from '../utility/hdbTerms.ts'; +import { + CONFIG_PARAMS, + LEGACY_DATABASES_DIR_NAME, + DATABASES_DIR_NAME, + RESERVED_DATABASE_NAMES, +} from '../utility/hdbTerms.ts'; import { getConfigPath } from '../config/configUtils.ts'; import { ClientError } from '../utility/errors/hdbError.ts'; import { _assignPackageExport } from '../globals.js'; @@ -1089,6 +1094,13 @@ export function table(tableDefinition: TableDefinition): Tabl cacheControl, } = tableDefinition; if (!databaseName) databaseName = DEFAULT_DATABASE_NAME; + // Reject reserved names here too, not only at the operations API: a database + // is also created by schema authoring — a `schema.graphql` `@table(database:)` + // or a programmatic `table()` call — which bypasses the create_schema + // validation. A reserved name collides with a role permission flag (harper#1016). + if ((RESERVED_DATABASE_NAMES as readonly string[]).includes(databaseName)) { + throw new ClientError(`'${databaseName}' is a reserved name and cannot be used as a database name`); + } const rootStore = database({ database: databaseName, table: tableName }); const tables = databases[databaseName]; logger.trace(`Defining ${tableName} in ${databaseName}`); diff --git a/resources/roles.ts b/resources/roles.ts index 117ed0f173..0e150ef14c 100644 --- a/resources/roles.ts +++ b/resources/roles.ts @@ -2,8 +2,12 @@ import { getDatabases } from './databases.ts'; import { alterRole, addRole } from '../security/role.ts'; import { parseDocument } from 'yaml'; import { isEqual } from 'lodash'; +import { RESERVED_DATABASE_NAMES } from '../utility/hdbTerms.ts'; -const USERS_NOT_DBS = ['super_user', 'structure_user']; +// Named permission flags are keyed alongside per-database permissions in a role's +// `permission` object; skip them when walking database permissions. This is the +// same reserved set that database names are rejected from (harper#1016). +const USERS_NOT_DBS: readonly string[] = RESERVED_DATABASE_NAMES; /** * This is the component for handling role declarations in the Harper system. This will read roles.yaml for role diff --git a/unitTests/dataLayer/reservedSchemaNames.test.js b/unitTests/dataLayer/reservedSchemaNames.test.js new file mode 100644 index 0000000000..df022c4f55 --- /dev/null +++ b/unitTests/dataLayer/reservedSchemaNames.test.js @@ -0,0 +1,53 @@ +const assert = require('node:assert/strict'); +const schema = require('#src/dataLayer/schema'); +const { table } = require('#src/resources/databases'); + +// harper#1016: a role's `permission` object keys per-database permissions by +// database name alongside named flags (super_user / cluster_user / +// structure_user / operations / _expandedOperations). A database named after one +// of those flags collides with it, so those names are rejected as database +// identifiers — at both the operations API (create_schema / create_table) and +// the schema-authoring path (graphql `@table(database:)` / programmatic table()). +const RESERVED = ['super_user', 'cluster_user', 'structure_user', 'operations', '_expandedOperations']; + +describe('#1016 reserved database names', () => { + for (const name of RESERVED) { + it(`create_schema rejects reserved name '${name}'`, async () => { + await assert.rejects( + () => schema.createSchemaStructure({ operation: 'create_schema', schema: name }), + /reserved name/, + `create_schema accepted reserved name '${name}'` + ); + }); + + it(`create_table rejects reserved database '${name}'`, async () => { + await assert.rejects( + () => schema.createTableStructure({ operation: 'create_table', schema: name, table: 't', primary_key: 'id' }), + /reserved name/, + `create_table accepted reserved database '${name}'` + ); + }); + + it(`schema authoring (table()) rejects reserved database '${name}'`, () => { + // graphql `@table(database:)` and programmatic table() funnel through here, + // bypassing the operations-API validation. The check throws before any store IO. + assert.throws( + () => table({ database: name, table: 't', attributes: [{ name: 'id', isPrimaryKey: true }] }), + /reserved name/, + `table() accepted reserved database '${name}'` + ); + }); + } + + it('does not reject a non-reserved database name as reserved', async () => { + // A normal name must pass the reserved-name check. It may still fail later + // (no schema metadata / bridge in this unit context) — just not as reserved. + let err; + try { + await schema.createSchemaStructure({ operation: 'create_schema', schema: 'my_normal_db' }); + } catch (e) { + err = e; + } + if (err) assert.doesNotMatch(err.message, /reserved name/); + }); +}); diff --git a/utility/hdbTerms.ts b/utility/hdbTerms.ts index 2c1f04bfe8..c5487ddb19 100644 --- a/utility/hdbTerms.ts +++ b/utility/hdbTerms.ts @@ -88,6 +88,22 @@ export const ROLE_TYPES_ENUM = { SUPER_USER: 'super_user', } as const; +/** + * Named permission flags that live in a role's `permission` object alongside + * per-database permission blocks, which are keyed by database name. A database + * named after one of these flags collides with it — the role parser cannot tell + * a `super_user` flag from a `super_user` database's permissions — producing + * undefined behavior, so these are rejected as database/schema identifiers + * (harper#1016). Keep in sync with `UserRoleNamedPermissions` in security/user.ts. + */ +export const RESERVED_DATABASE_NAMES = [ + 'super_user', + 'cluster_user', + 'structure_user', + 'operations', + '_expandedOperations', +] as const; + /** Email address for support requests */ export const HDB_SUPPORT_ADDRESS = 'support@harperdb.io'; From 4894330070a90992b337f4d6bae9dd3692c196ca Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Thu, 23 Jul 2026 12:43:01 -0600 Subject: [PATCH 2/5] fix(auth): complete cluster_user role-flag handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-on to the reserved-name fix, closing three related pre-existing gaps in how `cluster_user` is treated as a role permission flag (surfaced in review): - Role validation derived its boolean-flag list from ROLE_TYPES_ENUM, which held only `super_user`. A non-boolean `cluster_user` therefore skipped the boolean check and fell through to database validation, reporting a misleading "database 'cluster_user' does not exist" instead of the boolean-type error the SU_CU_ROLE_BOOLEAN_ERROR name already implies. Added CLUSTER_USER to ROLE_TYPES_ENUM. - translateRolePermissions preserved super_user / structure_user / operations but dropped `cluster_user`; now preserved alongside them for consistency (cluster_user is otherwise unread at runtime today, so no behavior change — just a faithful translation). - Removed the dead SU_CU_ROLE_COMBINED_ERROR from the test-error helper: it has no production counterpart and no test uses it (the su+cu rejection is covered by SU_CU_ROLE_NO_PERMS_ALLOWED). Refs harper#1016. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- security/permissionsTranslator.js | 3 +++ unitTests/commonTestErrors.js | 2 -- unitTests/validation/role_validation.test.js | 25 ++++++++++++++++++++ utility/hdbTerms.ts | 9 ++++++- 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/security/permissionsTranslator.js b/security/permissionsTranslator.js index d921d4ec78..a80bc4521c 100644 --- a/security/permissionsTranslator.js +++ b/security/permissionsTranslator.js @@ -132,6 +132,9 @@ function translateRolePermissions(role, schema) { const perms = role.permission; finalPermissions[terms.SYSTEM_SCHEMA_NAME] = perms[terms.SYSTEM_SCHEMA_NAME]; finalPermissions.structure_user = perms.structure_user; + // Preserve cluster_user like the other named flags so a translated role keeps + // what it declared, rather than silently dropping it (harper#1016). + finalPermissions.cluster_user = perms.cluster_user; // Preserve operations allowlist and its pre-expanded Set so the gate check in // verifyPerms still has access after role.permission is replaced with fullRolePerms. finalPermissions.operations = perms.operations; diff --git a/unitTests/commonTestErrors.js b/unitTests/commonTestErrors.js index 786d6a8f8c..debf9ce51c 100644 --- a/unitTests/commonTestErrors.js +++ b/unitTests/commonTestErrors.js @@ -110,8 +110,6 @@ const TEST_ROLE_PERMS_ERROR = { SU_ROLE_MISSING_ERROR: "Missing 'super_user' key/value in permission set", SU_CU_ROLE_BOOLEAN_ERROR: (role) => `Value for '${role}' permission must be a boolean`, SU_CU_ROLE_NO_PERMS_ALLOWED: (role) => `Roles with '${role}' set to true cannot have other permissions set.`, - SU_CU_ROLE_COMBINED_ERROR: - "Roles cannot have both 'super_user' and 'cluster_user' values included in their permissions set.", TABLE_PERM_MISSING: (perm) => `Missing table ${perm.toUpperCase()} permission`, TABLE_PERM_NOT_BOOLEAN: (perm) => `Table ${perm.toUpperCase()} permission must be a boolean`, STRUCTURE_USER_ROLE_TYPE_ERROR: (role) => `Value for '${role}' permission must be a boolean or Array`, diff --git a/unitTests/validation/role_validation.test.js b/unitTests/validation/role_validation.test.js index 5463812c95..167d99d197 100644 --- a/unitTests/validation/role_validation.test.js +++ b/unitTests/validation/role_validation.test.js @@ -249,6 +249,31 @@ describe('Test role_validation module ', () => { expect(test_result).to.equal(null); }); + it('NOMINAL - should return null for valid cluster_user = true ADD_ROLE object', () => { + const test_role_json = TEST_ADD_ROLE_OBJECT(); + test_role_json.permission = { cluster_user: true }; + const test_result = customValidate_rw(test_role_json, getAddRoleConstraints()); + + expect(test_result).to.equal(null); + }); + + // harper#1016: cluster_user is a boolean role flag, not a database name. A + // non-boolean value must report the boolean-type error, not a misleading + // "database 'cluster_user' does not exist". + it('should return the boolean error (not SCHEMA_NOT_FOUND) for non-boolean cluster_user', () => { + const test_role_json = TEST_ADD_ROLE_OBJECT(); + test_role_json.permission = { cluster_user: 'wut' }; + const test_result = customValidate_rw(test_role_json, getAddRoleConstraints()); + + expect(test_result.statusCode).to.equal(400); + expect(test_result.http_resp_msg.main_permissions).to.include( + TEST_ROLE_PERMS_ERROR.SU_CU_ROLE_BOOLEAN_ERROR('cluster_user') + ); + expect(test_result.http_resp_msg.main_permissions).to.not.include( + TEST_SCHEMA_OP_ERROR.SCHEMA_NOT_FOUND('cluster_user') + ); + }); + it('NOMINAL - should return null for valid ALTER_ROLE object', () => { const test_result = customValidate_rw(TEST_ALTER_ROLE_OBJECT(), getAlterRoleConstraints()); expect(test_result).to.equal(null); diff --git a/utility/hdbTerms.ts b/utility/hdbTerms.ts index c5487ddb19..f85d24bdda 100644 --- a/utility/hdbTerms.ts +++ b/utility/hdbTerms.ts @@ -83,9 +83,16 @@ export const LAUNCH_SERVICE_SCRIPTS = { MAIN: 'dist/bin/harper.js', } as const; -/** Specifies user role types */ +/** + * Special boolean role flags validated as role types (not database permissions). + * `cluster_user` belongs here alongside `super_user`: role validation checks both + * are booleans (SU_CU_ROLE_BOOLEAN_ERROR), and without it a non-boolean + * `cluster_user` falls through to database validation and reports a misleading + * "database 'cluster_user' does not exist" error (harper#1016). + */ export const ROLE_TYPES_ENUM = { SUPER_USER: 'super_user', + CLUSTER_USER: 'cluster_user', } as const; /** From ec635b98734fc102b3817cc81f06c5d6a0a869e8 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Thu, 23 Jul 2026 12:59:29 -0600 Subject: [PATCH 3/5] test(auth): use plain node:assert per house style (reserved-names test) Comment generated by kAIle (Claude Opus 4.8) --- unitTests/dataLayer/reservedSchemaNames.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unitTests/dataLayer/reservedSchemaNames.test.js b/unitTests/dataLayer/reservedSchemaNames.test.js index df022c4f55..fda1a72756 100644 --- a/unitTests/dataLayer/reservedSchemaNames.test.js +++ b/unitTests/dataLayer/reservedSchemaNames.test.js @@ -1,4 +1,4 @@ -const assert = require('node:assert/strict'); +const assert = require('node:assert'); const schema = require('#src/dataLayer/schema'); const { table } = require('#src/resources/databases'); From 560e5dd9530e5592b5214a351196fca844bfcdf4 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Thu, 23 Jul 2026 13:54:17 -0600 Subject: [PATCH 4/5] docs(auth): note access-key exclusion and why the reserved-name check is authoring-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deep-review follow-up (comments only): document that `access` is intentionally outside RESERVED_DATABASE_NAMES (stripped pre-persistence) and that the reserved-name check lives on the authoring path — not database()/makeTable — so a pre-existing reserved-name database still loads and can be dropped to remediate. Comment generated by kAIle (Claude Opus 4.8) --- resources/databases.ts | 3 +++ utility/hdbTerms.ts | 2 ++ 2 files changed, 5 insertions(+) diff --git a/resources/databases.ts b/resources/databases.ts index ad4989a62c..94254c3cf8 100644 --- a/resources/databases.ts +++ b/resources/databases.ts @@ -1098,6 +1098,9 @@ export function table(tableDefinition: TableDefinition): Tabl // is also created by schema authoring — a `schema.graphql` `@table(database:)` // or a programmatic `table()` call — which bypasses the create_schema // validation. A reserved name collides with a role permission flag (harper#1016). + // Deliberately on this authoring path only, NOT in `database()`/`makeTable`: those + // are also the load and drop paths, so a reserved-name database created before this + // fix still loads (data stays accessible) and can be dropped to remediate. if ((RESERVED_DATABASE_NAMES as readonly string[]).includes(databaseName)) { throw new ClientError(`'${databaseName}' is a reserved name and cannot be used as a database name`); } diff --git a/utility/hdbTerms.ts b/utility/hdbTerms.ts index f85d24bdda..bba4fa3cb0 100644 --- a/utility/hdbTerms.ts +++ b/utility/hdbTerms.ts @@ -102,6 +102,8 @@ export const ROLE_TYPES_ENUM = { * a `super_user` flag from a `super_user` database's permissions — producing * undefined behavior, so these are rejected as database/schema identifiers * (harper#1016). Keep in sync with `UserRoleNamedPermissions` in security/user.ts. + * `access` is deliberately NOT here: roles.ts strips it out of `permission` + * before persistence (scoped-delegation), so it never collides at runtime. */ export const RESERVED_DATABASE_NAMES = [ 'super_user', From 54962ed0cac38f1592c5fead9ebb9e762336150a Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Thu, 23 Jul 2026 18:16:26 -0600 Subject: [PATCH 5/5] fix(auth): gate the su/cu boolean check on key presence, not truthiness (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `object.permission[role] && !isBoolean(...)` gate let a falsey non-boolean (0, '', null) short-circuit past both the boolean check and — since the key is a role type — the database-permission loop, so add/alter role silently accepted a value that violates the boolean contract. Check `role in object.permission` instead. Closes the latent super_user variant too. Added falsey-non-boolean regression cases (0, '', null) for cluster_user and super_user. Raised by @kriszyp in review. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- unitTests/validation/role_validation.test.js | 22 ++++++++++++++++++++ validation/role_validation.ts | 6 +++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/unitTests/validation/role_validation.test.js b/unitTests/validation/role_validation.test.js index 167d99d197..d63292a0a3 100644 --- a/unitTests/validation/role_validation.test.js +++ b/unitTests/validation/role_validation.test.js @@ -274,6 +274,28 @@ describe('Test role_validation module ', () => { ); }); + // harper#1016 (review): the boolean check must gate on key presence, not truthiness — a falsey + // non-boolean (0, '', null) must still be rejected rather than silently skipping both the check + // and the database-permission loop. Covers cluster_user and the latent super_user variant. + [ + { role: 'cluster_user', value: 0 }, + { role: 'cluster_user', value: null }, + { role: 'cluster_user', value: '' }, + { role: 'super_user', value: 0 }, + { role: 'super_user', value: null }, + ].forEach(({ role, value }) => { + it(`should reject a falsey non-boolean ${role} = ${JSON.stringify(value)} as a boolean error`, () => { + const test_role_json = TEST_ADD_ROLE_OBJECT(); + test_role_json.permission = { [role]: value }; + const test_result = customValidate_rw(test_role_json, getAddRoleConstraints()); + + expect(test_result.statusCode).to.equal(400); + expect(test_result.http_resp_msg.main_permissions).to.include( + TEST_ROLE_PERMS_ERROR.SU_CU_ROLE_BOOLEAN_ERROR(role) + ); + }); + }); + it('NOMINAL - should return null for valid ALTER_ROLE object', () => { const test_result = customValidate_rw(TEST_ALTER_ROLE_OBJECT(), getAlterRoleConstraints()); expect(test_result).to.equal(null); diff --git a/validation/role_validation.ts b/validation/role_validation.ts index d49b862d54..d170db5efa 100644 --- a/validation/role_validation.ts +++ b/validation/role_validation.ts @@ -100,7 +100,11 @@ function customValidate(object, constraints) { } //check if cu or su values, if included, are booleans ROLE_TYPES.forEach((role) => { - if (object.permission[role as any] && !validate.isBoolean(object.permission[role as any])) { + // Gate on key presence, not truthiness: a falsey non-boolean (0, '', null) must still be + // rejected as a non-boolean. A truthiness gate would let those skip both this check AND the + // database-permission loop below (the key is a role type, so that loop excludes it), so an + // add/alter role would silently accept a value that violates the boolean contract. + if (role in object.permission && !validate.isBoolean(object.permission[role as any])) { addPermError(HDB_ERROR_MSGS.SU_CU_ROLE_BOOLEAN_ERROR(role as any), validationErrors, undefined, undefined); } });