diff --git a/.changeset/curated-capability-name-refused-at-write-door.md b/.changeset/curated-capability-name-refused-at-write-door.md new file mode 100644 index 0000000000..baf3d86b8f --- /dev/null +++ b/.changeset/curated-capability-name-refused-at-write-door.md @@ -0,0 +1,29 @@ +--- +'@objectstack/plugin-security': minor +--- + +Setup can no longer create — or rename a row to — a `sys_capability` whose `name` is in +the platform's curated set (`PLATFORM_CAPABILITY_NAMES`). **An authoring call that +previously answered 200 now refuses**: the admin-door data write (`insert`, and `update` +that renames TO a curated name) is rejected at the security middleware with +`403 PERMISSION_DENIED` and a message naming the colliding curated name. The affected +names are the curated registry's members, currently: `manage_users`, `manage_org_users`, +`manage_metadata`, `manage_platform_settings`, `setup.access`, `setup.write`, +`studio.access`, `manage_sharing`, `export_data` (maintainer ruling on #8552, applying +the "refuse a declaration the platform cannot honour" principle — such a row can never +be the curated capability, and it blocks the platform's own definition from ever +seeding). + +What is NOT changed: + +- creating capabilities with non-curated names stays open, in every deployment shape + (including the community NULL-organization-bucket shape with no org stamper); +- existing colliding rows are deliberately left to the operator (no adoption, no + provenance backfill — ruling options 2/3 are rejected): they can still be edited, and + renamed AWAY from the curated name (a payload repeating the row's own unchanged name + is not a rename and is not refused); +- the curated seeder's decline-and-warn behaviour on existing collisions stands; its + per-boot `blockedCurated` warning now also carries one operator-facing remediation + line (rename or remove the blocking row, then restart, and the platform definition + seeds); +- boot/system writes (`isSystem`) — the seeders and package publish — are unaffected. diff --git a/packages/plugins/plugin-security/src/bootstrap-system-capabilities.test.ts b/packages/plugins/plugin-security/src/bootstrap-system-capabilities.test.ts index e57f0014c6..c8f8c36a35 100644 --- a/packages/plugins/plugin-security/src/bootstrap-system-capabilities.test.ts +++ b/packages/plugins/plugin-security/src/bootstrap-system-capabilities.test.ts @@ -450,6 +450,11 @@ describe('[#8470] the curated half owns its row, not whichever row shares the na managedBy === undefined ? 'a row carrying no managed_by value' : `managed_by='${managedBy}'`, ); expect(warn.mock.calls[0][0]).not.toContain('does not own'); + // [#8552] The ruling leaves an existing colliding row to the OPERATOR, + // so the warning that reports it carries the one line that says how to + // resolve it by hand. + expect(warn.mock.calls[0][0]).toContain('To resolve by hand'); + expect(warn.mock.calls[0][0]).toContain('rename the blocking row to a name outside the curated set'); expect(warn.mock.calls[0][1]).toEqual({ name: 'manage_users', blockingRowId: 'aaa_pre_existing', @@ -487,6 +492,11 @@ describe('[#8470] the curated half owns its row, not whichever row shares the na // …and it must NOT borrow either of the other two branches' sentences. expect(warn.mock.calls[0][0]).not.toContain('carrying no managed_by value'); expect(warn.mock.calls[0][0]).not.toContain('left exactly as it is'); + // [#8552] The remediation line instructs renaming THE BLOCKING ROW; with + // no row visible there is nothing to rename, so it must not print here — + // telling the operator to rename a row the read just failed to find would + // assert what was not seen. + expect(warn.mock.calls[0][0]).not.toContain('To resolve by hand'); expect(warn.mock.calls[0][1]).toEqual({ name: 'manage_users', blockingRowId: undefined, blockingManagedBy: null, }); diff --git a/packages/plugins/plugin-security/src/bootstrap-system-capabilities.ts b/packages/plugins/plugin-security/src/bootstrap-system-capabilities.ts index 9da35b2eb9..813c67bba3 100644 --- a/packages/plugins/plugin-security/src/bootstrap-system-capabilities.ts +++ b/packages/plugins/plugin-security/src/bootstrap-system-capabilities.ts @@ -296,12 +296,24 @@ export async function bootstrapSystemCapabilities( ? 'a row carrying no managed_by value already holds the name, and was left exactly as it is' : `a row with managed_by='${String(blocking.managed_by)}' already holds the name, and was ` + 'left exactly as it is'; + // [#8552] The maintainer's ruling deliberately leaves an existing + // colliding row to the OPERATOR (option 1: no adoption, no backfill), + // so the warning that reports the collision owes them the one line + // that says how to resolve it by hand. Only when a blocking row was + // actually observed — telling an operator to rename a row the read + // just failed to find would be asserting what was not seen. + const remediation = blocking === undefined + ? '' + : ' To resolve by hand: rename the blocking row to a name outside the curated set (or delete ' + + 'it) — through Setup for an admin-authored row, or by editing and re-publishing the owning ' + + "package for a package-declared one — then restart; the seeder will then seed the platform's " + + 'definition. (New Setup rows can no longer take a curated name — refused at the write door.)'; options.logger?.warn?.( `[security] curated capability "${def.name}" has no platform row and could not be seeded. ` + 'In the platform (NULL-organization) bucket, where the declared unique key admits one row ' + `per name: ${observation}. The platform definition is therefore missing from sys_capability ` + 'installation-wide. Grants and requiredPermissions referencing the name are unaffected — ' + - 'they resolve by name, not by row.', + `they resolve by name, not by row.${remediation}`, { name: def.name, blockingRowId: blocking?.id, blockingManagedBy: blocking?.managed_by ?? null }, ); } diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index 3def7bdf11..6bf9ef05bf 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -2392,6 +2392,208 @@ describe('SecurityPlugin', () => { ).resolves.toBeDefined(); }); }); + + // ── #8552 / ADR-0066 D1 — curated capability-name refusal (create + rename) ── + // A `sys_capability` name in PLATFORM_CAPABILITY_NAMES is the platform's: + // Setup may neither CREATE a row with it nor RENAME a row TO it — refused at + // this write door with an error naming the colliding curated name, never + // answered 200. Existing colliding rows stay the operator's (ruling option + // 1): a payload repeating a row's own curated name is not a rename and + // passes, and renaming AWAY stays open. The load-bearing half of this suite + // is the POSITIVE controls — a refusal too broad passes every "it refuses" + // assertion, so non-curated authoring is pinned open in BOTH deployment + // shapes (org-scoped, and the community NULL-bucket shape with no tenant). + describe('curated capability-name write gate (#8552, sys_capability create/rename)', () => { + const adminSet: PermissionSet = { + name: 'admin_full_access', label: 'Admin', + objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, modifyAllRecords: true } }, + } as any; + // Enterprise / org-scoped shape: the caller carries an active organization. + const orgCtx = { userId: 'admin1', tenantId: 'org-1', positions: [], permissions: ['admin_full_access'] }; + // Community NULL-bucket shape: no `@objectstack/organizations` stamper, no + // tenantId — every Setup-authored row lands in the NULL-organization + // bucket. This is the default community deployment, not an edge case. + const communityCtx = { userId: 'admin1', positions: [], permissions: ['admin_full_access'] }; + + const runGate = async (opCtx: any, findOneImpl?: (q: any) => any) => { + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'admin_full_access' }); + const harness = makeMiddlewareCtx({ + permissionSets: [adminSet], + objectFields: ['id', 'name', 'label', 'managed_by', 'active'], + ...(findOneImpl ? { findOneImpl } : {}), + }); + await plugin.init(harness.ctx); + await plugin.start(harness.ctx); + return harness.run(opCtx); + }; + + // The refusal envelope, asserted whole (code AND status, plus the located + // name) — `rejects.toThrow()` alone would stay green on any refusal from + // any gate, which is exactly the vacuity this suite must not have. + const curatedRefusal = (name: string) => ({ + name: 'PermissionDeniedError', + code: 'PERMISSION_DENIED', + statusCode: 403, + message: expect.stringContaining(`'${name}' is a platform-curated capability name`), + }); + + it('DENIES creating a capability with a curated name (org-scoped shape), naming the collision', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'insert', + data: { name: 'manage_users', label: 'Mine now' }, context: orgCtx, + }; + await expect(runGate(opCtx)).rejects.toMatchObject(curatedRefusal('manage_users')); + }); + + it('DENIES creating a capability with a curated name (community NULL-bucket shape)', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'insert', + data: { name: 'setup.write', label: 'Setup Write' }, context: communityCtx, + }; + await expect(runGate(opCtx)).rejects.toMatchObject(curatedRefusal('setup.write')); + }); + + it('DENIES an ARRAY insert when ANY element claims a curated name', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'insert', + data: [{ name: 'my_cap_a' }, { name: 'manage_sharing' }], + context: orgCtx, + }; + await expect(runGate(opCtx)).rejects.toMatchObject(curatedRefusal('manage_sharing')); + }); + + it('DENIES RENAMING an admin-authored capability TO a curated name (by-id update)', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'update', + data: { id: 'cap_admin', name: 'manage_users' }, options: { where: { id: 'cap_admin' } }, + context: orgCtx, + }; + await expect( + runGate(opCtx, () => ({ id: 'cap_admin', name: 'my_cap', managed_by: 'admin' })), + ).rejects.toMatchObject(curatedRefusal('manage_users')); + }); + + it('DENIES a rename-to-curated even for a principal-less context (gate is before the fall-open)', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'update', + data: { id: 'cap_admin', name: 'manage_users' }, options: { where: { id: 'cap_admin' } }, + context: {}, + }; + await expect( + runGate(opCtx, () => ({ id: 'cap_admin', name: 'my_cap', managed_by: 'admin' })), + ).rejects.toMatchObject(curatedRefusal('manage_users')); + }); + + it('FAILS CLOSED when the renamed row cannot be read back (absent pre-image refuses)', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'update', + data: { id: 'cap_gone', name: 'manage_users' }, options: { where: { id: 'cap_gone' } }, + context: orgCtx, + }; + // Every findOne answers null: no pre-image exists to prove the payload + // repeats the row's own name, so the write is refused. + await expect(runGate(opCtx, () => null)).rejects.toMatchObject(curatedRefusal('manage_users')); + }); + + it('DENIES a FILTER update stamping a curated name onto differently-named rows', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'update', + data: { name: 'manage_users', active: false }, options: { where: { active: true } }, + context: orgCtx, + }; + // The $ne probe (and only it) finds a row NOT already named + // manage_users inside the write's own filter; the system-row gate's + // managed-row probe finds nothing, so the refusal measured here is this + // gate's, not a neighbour's. + const impl = (q: any) => { + const and = q?.where?.$and; + if (Array.isArray(and) && and.some((c: any) => c && typeof c === 'object' && c.name && typeof c.name === 'object' && '$ne' in c.name)) { + return { id: 'cap_other', name: 'my_cap', managed_by: 'admin' }; + } + return null; + }; + await expect(runGate(opCtx, impl)).rejects.toMatchObject(curatedRefusal('manage_users')); + }); + + it('DENIES a WHOLE-TABLE update stamping a curated name (no filter to scope the rename)', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'update', + data: { name: 'manage_users' }, context: orgCtx, + }; + await expect(runGate(opCtx, () => null)).rejects.toMatchObject(curatedRefusal('manage_users')); + }); + + // ── positive controls — the load-bearing half ── + + it('ALLOWS creating a NON-curated capability (org-scoped shape)', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'insert', + data: { name: 'org_export_reports', label: 'Export Reports', managed_by: 'admin' }, + context: orgCtx, + }; + await expect(runGate(opCtx)).resolves.toBeDefined(); + }); + + it('ALLOWS creating a NON-curated capability (community NULL-bucket shape)', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'insert', + data: { name: 'org_export_reports', label: 'Export Reports', managed_by: 'admin' }, + context: communityCtx, + }; + await expect(runGate(opCtx)).resolves.toBeDefined(); + }); + + it('ALLOWS a full-record update of an EXISTING colliding admin row that repeats its own name (not a rename)', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'update', + data: { id: 'cap_collide', name: 'manage_users', label: 'Renamed label', active: false }, + options: { where: { id: 'cap_collide' } }, + context: communityCtx, + }; + await expect( + runGate(opCtx, () => ({ id: 'cap_collide', name: 'manage_users', managed_by: 'admin' })), + ).resolves.toBeDefined(); + }); + + it('ALLOWS renaming an EXISTING colliding admin row AWAY from the curated name (the operator remediation)', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'update', + data: { id: 'cap_collide', name: 'org_manage_users' }, + options: { where: { id: 'cap_collide' } }, + context: communityCtx, + }; + await expect( + runGate(opCtx, () => ({ id: 'cap_collide', name: 'manage_users', managed_by: 'admin' })), + ).resolves.toBeDefined(); + }); + + it('ALLOWS a FILTER update stamping a curated name when it provably renames nothing (probe finds no other-named row)', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'update', + data: { name: 'manage_users', active: true }, options: { where: { name: 'manage_users' } }, + context: communityCtx, + }; + await expect(runGate(opCtx, () => null)).resolves.toBeDefined(); + }); + + it('lets system/boot writes through (isSystem bypass) — the curated seeder itself is unaffected', async () => { + const opCtx: any = { + object: 'sys_capability', operation: 'insert', + data: { name: 'manage_users', label: 'Manage Users', managed_by: 'platform' }, + context: { isSystem: true }, + }; + await expect(runGate(opCtx)).resolves.toBeDefined(); + }); + + it('does NOT reach sys_position — a position may share a curated capability STRING (scope control)', async () => { + const opCtx: any = { + object: 'sys_position', operation: 'insert', + data: { name: 'manage_users', label: 'A position, not a capability' }, + context: orgCtx, + }; + await expect(runGate(opCtx)).resolves.toBeDefined(); + }); + }); }); // --------------------------------------------------------------------------- describe('PermissionEvaluator', () => { diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 064447c7ff..1774b7a59b 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -64,6 +64,7 @@ import { postureUsesUnionScope, type TenancyPosture, } from '@objectstack/spec/security'; +import { PLATFORM_CAPABILITY_NAMES } from '@objectstack/spec/security'; import { RLS_MEMBERSHIP_RESOLVER_SERVICE, RESERVED_RLS_MEMBERSHIP_KEYS, @@ -1156,6 +1157,18 @@ export class SecurityPlugin implements Plugin { // so the seeder and package publish are unaffected. await this.assertSystemRowWriteGate(opCtx); + // [#8552 / ADR-0066 D1] Curated-name authoring refusal for + // sys_capability. A name in PLATFORM_CAPABILITY_NAMES is a capability + // the PLATFORM defines; a Setup-authored row claiming one is a + // declaration the platform cannot honour, so it is refused here — + // create and rename-to both — rather than answered 200 and then + // declined by the seeder on every boot. Runs AFTER the provenance gate + // above (a platform/package row's rename is already refused there on + // provenance) and, like it, BEFORE the empty-principal fall-open and + // the CRUD check. System/boot writes carry `isSystem` and + // short-circuited above, so the curated seeder itself is unaffected. + await this.assertCuratedCapabilityNameGate(opCtx); + // [ADR-0090 D5/D9] Audience-anchor binding guard — like the package // gate above, an unconditional data-layer boundary: a permission set // carrying high-privilege bits must never be bound to the `everyone` @@ -4009,6 +4022,108 @@ export class SecurityPlugin implements Plugin { } } + /** + * [#8552 / ADR-0066 D1] Curated-name authoring refusal for `sys_capability`. + * + * A name in `PLATFORM_CAPABILITY_NAMES` is a capability the PLATFORM + * defines: grants (`systemPermissions`) and requirements + * (`requiredPermissions`) resolve it by name, and the curated seeder owns + * the row for it in the NULL-organization bucket + * (`bootstrap-system-capabilities.ts`). A Setup-authored row claiming such + * a name is a declaration the platform cannot honour — it can never BE the + * curated capability (provenance forging is refused by the gate above), and + * once it holds the name's bucket slot the seeder must decline the + * collision on every boot (`blockedCurated`, #8470). Maintainer ruling on + * #8552 (2026-08-13): refuse the collision AT THE WRITE DOOR — create and + * rename both — naming the colliding curated name; never answer 200. + * + * What is refused, and exactly that: + * - `insert` whose payload names a curated capability (every row of an + * array payload); + * - `update` that RENAMES a row TO a curated name. A payload repeating + * the row's OWN existing curated name is not a rename and passes: the + * ruling deliberately leaves pre-existing colliding rows to the + * operator, so the operator's remediation on one (edit the label, + * deactivate, rename AWAY) must stay open, and a full-record PATCH from + * Setup echoes `name` unchanged. The pre-image is read under a system + * context to establish that fact; per the #7505 ruling the read fails + * CLOSED — an engine fault propagates, and an absent pre-image refuses, + * because a write claiming a curated name for a row that cannot be + * shown to already hold it is exactly the state this gate prevents. + * - a FILTER update stamping a curated name is allowed only when the + * filter provably matches nothing but rows already holding that name + * (`$ne` probe, the sibling gate's bulk-check shape); no filter at all + * is a whole-table stamp and is refused. + * + * Deliberately NOT covered: `restore` of a soft-deleted row that already + * held a curated name re-establishes a pre-existing collision rather than + * minting a new one — that residue is the operator's (option 1 of the + * ruling), not this gate's. Creating NON-curated names stays open — the + * legitimate half of both measured flows (community NULL-bucket `admin` + * rows, package declarations), and the seeders themselves carry `isSystem` + * and short-circuit the middleware entirely. + */ + private async assertCuratedCapabilityNameGate(opCtx: any): Promise { + if (opCtx?.object !== 'sys_capability') return; + const op = opCtx.operation; + if (op !== 'insert' && op !== 'update') return; + + const payloadRows: unknown[] = Array.isArray(opCtx.data) + ? opCtx.data + : opCtx.data && typeof opCtx.data === 'object' + ? [opCtx.data] + : []; + + const curatedNameOf = (r: unknown): string | null => { + if (!r || typeof r !== 'object') return null; + const name = (r as Record).name; + return typeof name === 'string' && PLATFORM_CAPABILITY_NAMES.has(name) ? name : null; + }; + + const refuse = (name: string): never => { + throw new PermissionDeniedError( + `[Security] Access denied: '${name}' is a platform-curated capability name — a sys_capability ` + + `row cannot be created with it or renamed to it through the admin door. The platform defines ` + + `this capability and seeds its own row for it; grants and requiredPermissions already resolve ` + + `the name. Choose a different capability name (ADR-0066 asset ownership, #8552).`, + { operation: op, object: opCtx.object, name, curated: true }, + ); + }; + + if (op === 'insert') { + for (const r of payloadRows) { + const name = curatedNameOf(r); + if (name != null) refuse(name); + } + return; + } + + // update — refuse a rename TO a curated name. + const isScalarId = (v: unknown): v is string | number | bigint => + v !== null && (typeof v === 'string' || typeof v === 'number' || typeof v === 'bigint'); + for (const r of payloadRows) { + const name = curatedNameOf(r); + if (name == null) continue; + const ownId = (r as Record).id; + const rowId = isScalarId(ownId) ? ownId : this.extractSingleId(opCtx); + if (rowId != null) { + if (!this.ql) refuse(name); + // [#7505] Fail-closed: a probe fault propagates; `null` means absent. + const existing = await this.readRowById('sys_capability', rowId, { isSystem: true }); + if (existing && String((existing as Record).name ?? '') === name) continue; + refuse(name); + } + const writeWhere = opCtx?.options?.where; + if (!writeWhere || typeof writeWhere !== 'object' || !this.ql) refuse(name); + // [#7505] No swallow here either — a probe fault propagates (fail closed). + const renamesOtherRow = await this.ql.findOne('sys_capability', { + where: { $and: [writeWhere, { name: { $ne: name } }] }, + context: { isSystem: true }, + }); + if (renamesOtherRow) refuse(name); + } + } + private extractSingleId(opCtx: any): string | number | bigint | null { const isScalar = (v: unknown): v is string | number | bigint => v !== null && (typeof v === 'string' || typeof v === 'number' || typeof v === 'bigint');