diff --git a/.changeset/15204-s2a-security-readers.md b/.changeset/15204-s2a-security-readers.md new file mode 100644 index 00000000000..e0d821bfa2f --- /dev/null +++ b/.changeset/15204-s2a-security-readers.md @@ -0,0 +1,40 @@ +--- +'@objectstack/core': minor +'@objectstack/plugin-security': minor +--- + +feat(plugin-security)!: the security plugin's own readers (set resolution, the delegated-administration gate, the permission explainer and the position-assignment refusal) read the security catalog and the activation ledger, not the catalog rows (ADR-0131 D3/D4) + +Clause-②: yes + + + +**BREAKING** (enforcement stops reading `sys_permission_set` rows), shipped as `minor` under the launch-window convention for breaking changes. This is the second step of the cutover that `position-permission-sets-declared` registers: the authorization resolver already reads the catalog; now the security plugin's readers read the same catalog. + +**Permission-set resolution (`@objectstack/plugin-security`).** + +- FROM: a set name the metadata service and the bootstrap sets did not answer was loaded from a `sys_permission_set` row (the caller's organization's, else an organization-less one), its object map and field map included, and dropped when the row's `active` was false. +- TO: it is loaded from the security catalog (the environment registry the security plugin binds to its engine): every package's sets and every set an author saved through the metadata door. A set that only a row carries resolves nothing. The resolver already granted nothing through such a set; now its object and field permissions stop applying too. +- Fix: declare the set through the metadata door (`PUT /api/v1/meta/permission/NAME`) or in a package. +- A permission set that resolves only because a POSITION of the same name was folded into the request is dropped when the activation ledger (`sys_metadata_activation`, type `permission`) switched it off. The row's `active` column is not read. + +**Delegated administration (ADR-0090 D12).** + +- FROM: the sets a position distributes were its `sys_position_permission_set` rows in the caller's organization; `delegatable` was the organization's `sys_position` row's; the assignable-positions listing (`describeDelegableScope`) listed `sys_position` rows; a direct grant's set (and its `adminScope`) was its `sys_permission_set` row. +- TO: all four read the security catalog by name. A position distributes the sets its definition's `permissionSets` names, so the containment check and the listing see a binding declared only on the definition. `delegatable` is the definition's. The listing offers every position the catalog holds. A direct grant's set is the catalog's definition of the name the grant carries; a grant naming a set the catalog does not hold is refused for a delegate (a tenant administrator is not judged). +- A name a position's `permissionSets` carries that the catalog does not hold must be on the delegate's allowlist too. It grants nothing today, but it starts granting as soon as a set of that name is authored. +- A catalog read that fails refuses the delegate's write. Before, it approved it as "distributes nothing". + +**The permission explainer.** "deactivated" is read from the activation ledger through the resolver's own read, so explain and enforce agree. A direct grant whose set the catalog does not hold is reported neither as expired nor as deactivated: there is no set to lose. + +**Position assignments (`sys_user_position.position`).** + +- FROM: refused `400 VALIDATION_FAILED` unless a `sys_position` row (the writer's organization's, or an organization-less one) carried the name. A position declared only in a package or saved only through the metadata door was refused. +- TO: refused unless the security catalog holds a position of that name. A registry-declared position is accepted. A name only a `sys_position` row carries (with no definition) is refused, because it grants nothing. A deactivated position is still accepted. + +**One position name per deployment under `single`.** + +- FROM: a Setup create of a position, or a rename into a name, that an environment definition already held overwrote that definition (another organization's label and description, an empty `permissionSets`). +- TO: refused `409 UNIQUE_VIOLATION`, the answer a second row of the name in one organization already gets, and the row write is undone. This covers a definition another organization's row wrote, one saved through the metadata door, and one an application stack declares. A name a package or a built-in holds keeps its `403 NOT_OVERRIDABLE`. + +**`@objectstack/core`.** `readDisabledCatalogNames(ql, positions, sets)` is exported: the resolver's one read of the activation ledger. It returns the names switched off for this deployment and is now shared by the explainer and the security plugin. diff --git a/content/docs/permissions/authorization.mdx b/content/docs/permissions/authorization.mdx index 06ddf0b61e8..6614d8a29e3 100644 --- a/content/docs/permissions/authorization.mdx +++ b/content/docs/permissions/authorization.mdx @@ -430,7 +430,8 @@ an expiry: deactivation is an incident-response control, installation-wide, and there is no date on the user's own grant row to notice — without the state, the admin diagnosing "why did access disappear" would get the same answer as for a user who never held the grant, with no pointer to the -catalogue row somebody switched off. Reported for the grant rows explain +`sys_metadata_activation` row somebody switched off — the explainer reads the +same ledger the resolver does, so explain and enforce agree. Reported for the grant rows explain already walks: `sys_user_position` rows and direct `sys_user_permission_set` grants (a deactivated set reached only through an active position's linkage is not re-derived — that would replicate the resolver's aggregation). @@ -455,9 +456,12 @@ Deactivation is an incident-response control, so what it does **not** touch is deliberate. Administration surfaces keep listing and editing a deactivated position — an admin must still be able to unbind and clean up what they just switched off — and the write gates that judge audience-anchor bindings and a -delegated administrator's blast radius keep reading every row, deactivated -included: dropping rows there would make a refused binding *permitted* and a -delegate's boundary *narrower*, which is the opposite of switching access off. +delegated administrator's blast radius do not consult the activation ledger: +they judge every binding and assignment in front of them — including the +permission sets a position's definition names — whether or not the ledger +switched it off, because skipping a switched-off one would make a refused +binding *permitted* and a delegate's boundary *narrower*, which is the +opposite of switching access off. ## Governance: how "declared = enforced" is kept true diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index b11f52401f8..3e9919c52a3 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -129,7 +129,7 @@ that silently does not happen. | 21b | **`readonly` strip bypassed — the write PREVIEW (`ObjectQL.validate`)** | objectql | Same as 19–21, in the preview: it runs the write's caller-write strips under the write's own gate, so a system preview reports no `readonly` drop and judges the value a system write would store. The `computed` strip of a `formula` value is NOT gated here or on either write verb — no driver has a column for it, so it runs for every caller | `packages/objectql/src/engine.ts#validate` | | 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing. One exception: a `formula` value, which the `computed` strip takes from every caller, so strict still refuses it | `packages/objectql/src/engine.ts#insert`, `packages/objectql/src/readonly-strict-errors.ts#READONLY_CLASS_REASONS` | | 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `packages/objectql/src/engine.ts#assertReferencesResolve` | -| 23b | **Position-catalog check skipped** — a `sys_user_position` write whose `position` names no `sys_position` row is not refused | plugin-security | Get: a system writer can store an assignment naming no catalog row — the seed loader writes `stack.data` from `AppPlugin.start()`, before `kernel:ready` seeds the declared position catalog, so a refusal there would fail every authored assignment seed on a fresh boot. Lose: such a row grants nothing and nothing says so. A non-system insert, or a non-system update that changes `position`, is refused `400 VALIDATION_FAILED` / `reference_not_found` instead when no catalog row the writer's context reaches carries the name: its organization's rows plus the organization-less ones, read as `{ ...context, isSystem: true }`, never a bare `{ isSystem: true }`, so a name only another organization carries is refused like one nobody carries. It is the same stand-down, and the same tenant scope, as the engine's referential-integrity check for a lookup column | `packages/plugins/plugin-security/src/position-catalog-refusal.ts#assertPositionNamesCatalogRow` | +| 23b | **Position-catalog check skipped** — a `sys_user_position` write whose `position` names no position in the security catalog is not refused | plugin-security | Get: a system writer can store an assignment naming no position — the seed loader writes `stack.data` from `AppPlugin.start()`, before `kernel:ready` seeds the declared position catalog, so a refusal there would fail every authored assignment seed on a fresh boot. Lose: such a row grants nothing and nothing says so. A non-system insert, or a non-system update that changes `position`, is refused `400 VALIDATION_FAILED` / `reference_not_found` instead when the security catalog holds no position of that name: the environment registry (ADR-0131 D3), one per deployment, which takes no organization, so a name only a `sys_position` row carries — any organization's — is refused like one nobody carries. It is the same stand-down as the engine's referential-integrity check for a lookup column | `packages/plugins/plugin-security/src/position-catalog-refusal.ts#assertPositionNamesCatalogRow` | | 23c | **Grant-name check relaxed for an unresolvable id** — a `sys_user_permission_set` write that supplies `permission_set` beside a `permission_set_id` naming no set the writer's context reaches keeps the supplied name | plugin-security | Get: a system writer can store a grant naming a set whose row does not exist yet — seed replay and boot provisioning may write the grant before the set row, the ordering the engine's referential-integrity check also stands down for. Lose: such a row's name is unchecked until a backfill names it. Everything else is judged for every caller, system included: the name is derived from the id on every write that carries one, and a supplied name that names any other set is refused `400 VALIDATION_FAILED` / `invalid_value`; a non-system caller's name beside an unresolvable id is refused the same way, read as `{ ...context, isSystem: true }` through the hook's `ctx.api.sudo()`, never a bare `{ isSystem: true }` (ADR-0131 D4) | `packages/plugins/plugin-security/src/grant-permission-set-name.ts#settle` | | 23d | **Grant-holder membership check skipped** — a `sys_user_position` or `sys_user_permission_set` write naming a user with no `sys_member` row in the row's organization is not refused | plugin-security | Get: a system writer can store such a grant — invitation acceptance writes the membership and its placement together, and seed replay, the organization-admin reconcile and the platform bootstraps write grants as the system. Lose: such a row takes effect once its user is admitted to the organization. A non-system insert, or a non-system update that moves `user_id` or `organization_id`, is refused `400 VALIDATION_FAILED` / `reference_not_found` at `user_id` instead, for every caller, a platform administrator included; the membership is read as `{ isSystem: true, tenantId }` with `tenantId` the row's own organization, never another one. A row stored with no organization is a global grant and is not judged | `packages/plugins/plugin-security/src/grant-holder-membership-refusal.ts#onInsert`, `#onUpdate` | | 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `packages/objectql/src/engine.ts#buildDriverOptions` | diff --git a/packages/core/src/security/index.ts b/packages/core/src/security/index.ts index 0a2181eb0ca..d32a892f2f2 100644 --- a/packages/core/src/security/index.ts +++ b/packages/core/src/security/index.ts @@ -156,6 +156,10 @@ export { // [#20580] The session arm's membership check (#15409 ruling B), so the // permission explainer resolves the user it explains through the same one. vetOrganizationClaim, + // [ADR-0049, ADR-0131 D3, ADR-0126 §4] The one deactivation read of the + // security catalog — the `sys_metadata_activation` ledger, never a catalog + // row's `active` column — shared with the explainer and the security plugin. + readDisabledCatalogNames, resolveLocalizationContext, type ResolvedAuthzContext, type ResolveAuthzInput, @@ -232,7 +236,8 @@ export { isRowActive, type ActivatableRow } from './row-active.js'; // ADR-0131 D2–D4 — the ONE by-name read of the security catalog (positions, // permission sets, capabilities) over the engine registry and the metadata // service. It says a definition EXISTS under a name, never that it is in -// effect: the row `active` flag above stays the authority for that. +// effect: the activation ledger is the authority for that +// (`readDisabledCatalogNames`, exported with the resolver below). export { createSecurityCatalogReader, bindSecurityCatalogReader, diff --git a/packages/core/src/security/resolve-authz-context.ts b/packages/core/src/security/resolve-authz-context.ts index cc8039bc6f2..6fbbb9d4852 100644 --- a/packages/core/src/security/resolve-authz-context.ts +++ b/packages/core/src/security/resolve-authz-context.ts @@ -771,8 +771,13 @@ function ledgerUnregistered(ql: any): boolean { * engine answers "object not found" rather than a missing table — so the * registry is asked first, as `ObjectQLPlugin`'s own ledger hydration does, * and the read is not issued. An engine with no registry to ask is read. + * + * Exported so every other reader of "is this switched off" — the permission + * explainer's dropped-grant report, the security plugin's position-name fold — + * asks this one read instead of a second copy, and explain and enforce cannot + * disagree about deactivation. */ -async function readDisabledCatalogNames( +export async function readDisabledCatalogNames( ql: any, positions: readonly string[], sets: readonly string[], diff --git a/packages/core/src/security/security-catalog.ts b/packages/core/src/security/security-catalog.ts index 5fc24ee51be..368e25846f3 100644 --- a/packages/core/src/security/security-catalog.ts +++ b/packages/core/src/security/security-catalog.ts @@ -67,13 +67,14 @@ * * ## What this read does NOT answer * - * - ⛔ **Whether an item is in effect.** Deactivation lives on the catalog - * ROW (`sys_position.active`, `sys_permission_set.active`; standing ruling: - * the row flag stays authoritative), and no definition carries it. An entry - * here says a definition EXISTS under that name — never that it grants. A - * caller that drops the row flag because it now reads definitions would let - * a deactivated set grant again: the one widening this card must never - * open. Read the flag with `isRowActive` (`row-active.ts`) as before. + * - ⛔ **Whether an item is in effect.** No definition carries the switch: + * deactivation is the activation ledger's (`sys_metadata_activation`, one + * row per `(metadata_type, name)`; ADR-0126 §3 regime C as ADR-0131 D6 + * amends it), read through `readDisabledCatalogNames` + * (`resolve-authz-context.ts`) — never a catalog row's `active` column. An + * entry here says a definition EXISTS under that name — never that it + * grants. A caller that reads a definition and skips the ledger would let a + * deactivated set grant again: the one widening this read must never open. * - **The position → permission-set binding as a separate fact.** It is part * of the position definition (`PositionSchema.permissionSets`, ADR-0131 * D3/D4); the resolver reads it from the entry this module answers, and no diff --git a/packages/plugins/plugin-security/src/__tests__/security-catalog.testkit.ts b/packages/plugins/plugin-security/src/__tests__/security-catalog.testkit.ts index 74913a4b457..da4eea9f4e6 100644 --- a/packages/plugins/plugin-security/src/__tests__/security-catalog.testkit.ts +++ b/packages/plugins/plugin-security/src/__tests__/security-catalog.testkit.ts @@ -54,7 +54,7 @@ export function catalogFromTables(tables: Record d.name === row.name)) continue; const definition: Definition = { name: row.name }; - for (const [column, key] of [['system_permissions', 'systemPermissions'], ['tab_permissions', 'tabPermissions'], ['object_permissions', 'objects']] as const) { + for (const [column, key] of [['system_permissions', 'systemPermissions'], ['tab_permissions', 'tabPermissions'], ['object_permissions', 'objects'], ['admin_scope', 'adminScope']] as const) { const value = parsed(row[column] ?? row[key]); if (value !== undefined && value !== null) definition[key] = value; } @@ -65,9 +65,16 @@ export function catalogFromTables(tables: Record + positionRows.some((r) => r.name === name && (r.delegatable === true || r.delegatable === 1 || r.delegatable === '1')); return { permissions, - positions: positions.flatMap((f) => (f.fate === 'conflicting' ? [] : [{ name: f.name, permissionSets: [...f.permissionSets] }])), + positions: positions.flatMap((f) => (f.fate === 'conflicting' + ? [] + : [{ name: f.name, permissionSets: [...f.permissionSets], ...(delegatable(f.name) ? { delegatable: true } : {}) }])), }; } diff --git a/packages/plugins/plugin-security/src/builtin-positions.boot.test.ts b/packages/plugins/plugin-security/src/builtin-positions.boot.test.ts index 144125ec4ee..5621c734116 100644 --- a/packages/plugins/plugin-security/src/builtin-positions.boot.test.ts +++ b/packages/plugins/plugin-security/src/builtin-positions.boot.test.ts @@ -397,15 +397,21 @@ async function tenantPositionWrites(engine: ObjectQL): Promise = { - 'assign a position no catalog row carries': { + 'assign a position the security catalog does not hold': { ok: false, code: 'VALIDATION_FAILED', fields: ['position:reference_not_found'], }, - 'control: create a position': { + 'control: create a position row': { ok: true, }, - 'control: assign it': { + 'assign the row-only position': { + ok: false, + code: 'VALIDATION_FAILED', + fields: ['position:reference_not_found'], + }, + 'control: assign a stack-declared position (no row needed)': { ok: true, }, 'delete the everyone row': { diff --git a/packages/plugins/plugin-security/src/delegated-admin-gate-catalog.test.ts b/packages/plugins/plugin-security/src/delegated-admin-gate-catalog.test.ts new file mode 100644 index 00000000000..0e0092aee66 --- /dev/null +++ b/packages/plugins/plugin-security/src/delegated-admin-gate-catalog.test.ts @@ -0,0 +1,166 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0090 D12 × ADR-0131 D3/D4] The delegated-administration gate judges a + * position by its DEFINITION in the security catalog. + * + * The authorization resolver grants a held position the sets its definition's + * `permissionSets` names, read from the environment registry, and reads no + * `sys_position_permission_set` junction row. The gate's containment check and + * the assignable-positions listing must judge exactly that binding, or a + * delegate could hand out (and a picker could offer) a position whose real + * grant the gate never saw. These pins hold the gate to the catalog: + * + * - a binding declared only on the definition is judged; + * - a junction row binds nothing, so it neither refuses nor launders; + * - a position declared only in the registry (no `sys_position` row) is + * assignable and listed; + * - a set name the definition carries but the catalog does not hold yet must + * be allowlisted too (it starts granting the moment it is authored); + * - a catalog read that fails refuses the write instead of approving it as + * "distributes nothing". + */ + +import { describe, it, expect } from 'vitest'; +import { AuthzStoreUnavailableError, bindSecurityCatalogReader } from '@objectstack/core'; +import { DelegatedAdminGate } from './delegated-admin-gate.js'; +import { bindTestSecurityCatalog } from './__tests__/security-catalog.testkit.js'; + +const EAST_SCOPE = { + businessUnit: 'east', + includeSubtree: true, + manageAssignments: true, + manageBindings: false, + authorEnvironmentSets: false, + assignablePermissionSets: ['sales_user'], +}; + +const SETS: Record = { + tenant_admin: [{ name: 'admin_full', objects: { '*': { allowRead: true, modifyAllRecords: true } } }], + delegate: [{ name: 'east_admin', objects: {}, adminScope: EAST_SCOPE }], +}; + +const CATALOG = { + positions: [ + // Declared only on the definition: no junction row binds anything below. + { name: 'rep', permissionSets: ['sales_user'] }, + { name: 'mixed', permissionSets: ['sales_user', 'finance_admin'] }, + // No `sys_position` row carries this one. + { name: 'registry_only', permissionSets: ['sales_user'] }, + // Names a set nobody has authored yet. + { name: 'pending', permissionSets: ['sales_user', 'not_yet_authored'] }, + { name: 'everyone', permissionSets: ['member_default'] }, + ], + permissions: [{ name: 'sales_user' }, { name: 'finance_admin' }, { name: 'member_default' }], +}; + +function makeQl(tables: Record = {}) { + const all: Record = { + sys_business_unit: [ + { id: 'bu_hq', name: 'hq', parent_business_unit_id: null }, + { id: 'bu_east', name: 'east', parent_business_unit_id: 'bu_hq' }, + { id: 'bu_west', name: 'west', parent_business_unit_id: 'bu_hq' }, + ], + // Rows say something else on purpose: `rep` is bound to the set NOT on + // the allowlist, and `mixed` to nothing at all. + sys_position: [ + { id: 'pos_rep', name: 'rep' }, + { id: 'pos_mixed', name: 'mixed' }, + { id: 'pos_pending', name: 'pending' }, + ], + sys_permission_set: [ + { id: 'ps_sales', name: 'sales_user' }, + { id: 'ps_fin', name: 'finance_admin' }, + ], + sys_position_permission_set: [{ id: 'b1', position_id: 'pos_rep', permission_set_id: 'ps_fin' }], + sys_user_position: [], + sys_business_unit_member: [], + sys_user: [], + ...tables, + }; + const matches = (row: any, where: any): boolean => + Object.entries(where ?? {}).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + if (v && typeof v === 'object' && Array.isArray((v as any).$in)) return (v as any).$in.includes(row[k]); + if (v && typeof v === 'object') throw new Error(`fake driver: unsupported condition on ${k}`); + return row[k] === v; + }); + return { + async find(object: string, opts: any) { + const rows = (all[object] ?? []).filter((r) => matches(r, opts?.where)); + return typeof opts?.limit === 'number' ? rows.slice(0, opts.limit) : rows; + }, + async findOne(object: string, opts: any) { + return (all[object] ?? []).filter((r) => matches(r, opts?.where))[0] ?? null; + }, + } as any; +} + +const gateOver = (ql: any) => + new DelegatedAdminGate({ ql, resolveSets: async (ctx: any) => SETS[ctx?.principal ?? ''] ?? [] }); + +const assign = (gate: DelegatedAdminGate, position: string) => + gate.assert({ + object: 'sys_user_position', + operation: 'insert', + data: { user_id: 'u_east_1', position, business_unit_id: 'bu_east' }, + context: { principal: 'delegate', userId: 'u_delegate' }, + }); + +async function refusal(p: Promise): Promise { + const err: any = await p.then(() => null, (e) => e); + expect(err, 'expected a refusal').not.toBeNull(); + return err; +} + +describe('DelegatedAdminGate — a position distributes what its definition declares', () => { + it('a binding declared only on the definition is judged: a non-allowlisted set refuses the assignment', async () => { + const err = await refusal(assign(gateOver(bindTestSecurityCatalog(makeQl(), CATALOG)), 'mixed')); + expect(err).toMatchObject({ code: 'PERMISSION_DENIED', statusCode: 403 }); + expect(String(err.message)).toMatch(/position 'mixed' distributes permission set 'finance_admin', which is not in the scope's allowlist/); + }); + + it("a junction row binds nothing: `rep`'s row binding finance_admin neither refuses nor counts", async () => { + await expect(assign(gateOver(bindTestSecurityCatalog(makeQl(), CATALOG)), 'rep')).resolves.toBeUndefined(); + }); + + it('a position declared only in the registry (no sys_position row) is assignable', async () => { + await expect(assign(gateOver(bindTestSecurityCatalog(makeQl(), CATALOG)), 'registry_only')).resolves.toBeUndefined(); + }); + + it('a set name the definition carries but the catalog does not hold yet must be allowlisted too', async () => { + const err = await refusal(assign(gateOver(bindTestSecurityCatalog(makeQl(), CATALOG)), 'pending')); + expect(String(err.message)).toMatch(/position 'pending' distributes permission set 'not_yet_authored', which is not in the scope's allowlist/); + }); + + it('a catalog read that fails refuses the write; it is never read as "distributes nothing"', async () => { + const ql = makeQl(); + bindSecurityCatalogReader(ql, { + resolve: async () => { + throw new AuthzStoreUnavailableError('security catalog: registry (position)'); + }, + list: async () => [], + }); + const err = await refusal(assign(gateOver(ql), 'rep')); + expect(err).toBeInstanceOf(AuthzStoreUnavailableError); + }); +}); + +describe('DelegatedAdminGate — describeDelegableScope lists the catalog positions', () => { + it('a delegate is offered exactly the positions whose declared sets are all allowlisted (anchors excluded)', async () => { + const gate = gateOver(bindTestSecurityCatalog(makeQl(), CATALOG)); + const report = await gate.describeDelegableScope(SETS.delegate, { userId: 'u_delegate' }); + expect(report.assignablePositions.sort()).toEqual(['registry_only', 'rep']); + }); + + it('a tenant administrator is offered every position the catalog holds, a registry-only one included', async () => { + const gate = gateOver(bindTestSecurityCatalog(makeQl(), CATALOG)); + const report = await gate.describeDelegableScope(SETS.tenant_admin, { userId: 'u_admin' }); + expect(report.assignablePositions.sort()).toEqual(['mixed', 'pending', 'registry_only', 'rep']); + }); + + it('with no catalog bound, nothing is offered (fail closed) — a row is not a position', async () => { + const report = await gateOver(makeQl()).describeDelegableScope(SETS.tenant_admin, { userId: 'u_admin' }); + expect(report.assignablePositions).toEqual([]); + }); +}); diff --git a/packages/plugins/plugin-security/src/delegated-admin-gate-holding-organization.test.ts b/packages/plugins/plugin-security/src/delegated-admin-gate-holding-organization.test.ts index cd0a550efb3..ea1fd91b215 100644 --- a/packages/plugins/plugin-security/src/delegated-admin-gate-holding-organization.test.ts +++ b/packages/plugins/plugin-security/src/delegated-admin-gate-holding-organization.test.ts @@ -36,6 +36,7 @@ import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import type { PermissionSet } from '@objectstack/spec/security'; import { DelegatedAdminGate } from './delegated-admin-gate.js'; +import { bindTestSecurityCatalog } from './__tests__/security-catalog.testkit.js'; const ORG_A = 'org_a_acme'; /** Sorts BEFORE every `org_a` / `*_a_*` id, like the sibling suites. */ @@ -155,6 +156,12 @@ async function hold( } function gateOn(engine: ObjectQL) { + // [ADR-0131 D3/D4] The position and the set are the security catalog's + // definitions (environment-level); the rows above are each organization's. + bindTestSecurityCatalog(engine, { + positions: [{ name: 'field_lead', delegatable: true }], + permissions: [{ name: 'sales_user' }], + }); return new DelegatedAdminGate({ ql: engine, resolveSets: async (context: any) => (context?.principal === 'binder' ? BINDER_SETS : []), diff --git a/packages/plugins/plugin-security/src/delegated-admin-gate-position-organization.test.ts b/packages/plugins/plugin-security/src/delegated-admin-gate-position-organization.test.ts deleted file mode 100644 index 142e9be88cf..00000000000 --- a/packages/plugins/plugin-security/src/delegated-admin-gate-position-organization.test.ts +++ /dev/null @@ -1,414 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * The delegated-admin gate resolves a position NAME inside the caller's own - * organization — pinned over a real engine, one witness per authority read. - * - * ## The defect this exists to keep closed - * - * `sys_position` is a per-organization catalog upserted by - * `(name, organization_id)`, and `name` carries no installation-wide - * uniqueness, so in a single-database multi-org posture — ADR-0105 D1 - * `group` / `isolated`, which ADR-0132 ships open — two organizations may each - * hold a position of the same name. Two gate reads looked the position up by - * NAME alone, `limit: 1`, under a bare `{ isSystem: true }` context carrying no - * tenant, so whichever id the driver ordered first answered: - * - * - `positionIsDelegatable` — may this position be SELF-DELEGATED - * (ADR-0091 D3 rule 5)? - * - `setsBoundToPosition` — which permission sets does it distribute (the - * allowlist and containment tests of a delegated assignment, and D3 rule 6)? - * - * The sibling half — a scope's business-unit anchor — is pinned in - * `delegated-admin-gate-cross-organization.test.ts`; this file reuses its - * harness and its id ordering: the OTHER organization's ids sort first - * (`*_0_*` before `*_a_*`), and the SQL driver's `limit` read is - * `ORDER BY id ASC`, so under the old resolution every organization-A answer - * below was organization B's. - * - * Fail closed: a position name with no row in the caller's organization is not - * delegatable and distributes no permission sets — never another - * organization's row, never an unscoped read. That matches the runtime - * resolver, which reads position-bound sets through the caller's tenant. - */ - -import { describe, it, expect, afterEach } from 'vitest'; -import { ObjectQL } from '@objectstack/objectql'; -import { SqlDriver } from '@objectstack/driver-sql'; -import type { PermissionSet } from '@objectstack/spec/security'; -import { DelegatedAdminGate } from './delegated-admin-gate.js'; - -const ORG_A = 'org_a_acme'; -/** Deliberately sorts BEFORE every `org_a` / `*_a_*` id — the side the old read picked. */ -const ORG_B = 'org_0_globex'; - -const text = (name: string) => ({ name, type: 'text' as const }); -const object = (name: string, fields: string[]) => ({ - name, - label: name, - fields: { - id: { name: 'id', type: 'text' as const, primaryKey: true }, - organization_id: text('organization_id'), - ...Object.fromEntries(fields.map((f) => [f, text(f)])), - }, -}); - -const OBJECTS = [ - object('sys_business_unit', ['name', 'parent_business_unit_id']), - object('sys_position', ['name', 'delegatable']), - object('sys_permission_set', ['name', 'admin_scope']), - object('sys_position_permission_set', ['position_id', 'permission_set_id']), - object('sys_user_position', [ - 'user_id', 'position', 'business_unit_id', 'granted_by', 'delegated_from', 'valid_until', 'reason', - ]), - object('sys_business_unit_member', ['user_id', 'business_unit_id']), - object('sys_user', ['primary_business_unit_id']), -]; - -/** The delegate's scope: anchored at the unit NAMED `sales`, handing out `sales_user` only. */ -const SALES_SCOPE = { - businessUnit: 'sales', - includeSubtree: true, - manageAssignments: true, - manageBindings: false, - authorEnvironmentSets: false, - assignablePermissionSets: ['sales_user'], -}; - -const DELEGATE_SETS = [ - { - name: 'sales_admin', - objects: { sys_user_position: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } }, - adminScope: SALES_SCOPE, - }, -] as unknown as PermissionSet[]; - -const engines: ObjectQL[] = []; -afterEach(async () => { - while (engines.length) { - try { await engines.pop()?.destroy(); } catch { /* noop */ } - } -}); - -const sysCtx = (organizationId: string) => ({ isSystem: true, tenantId: organizationId }); - -async function bootEngine() { - const engine = new ObjectQL(); - engine.registerDriver( - new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), - true, - ); - await engine.init(); - engine.registerApp({ - id: 'com.objectstack.qa.delegated-admin-position-organization', - name: 'Delegated admin position organization scope', - version: '1.0.0', - type: 'plugin', - scope: 'system', - objects: OBJECTS, - } as any); - await engine.syncSchemas(); - engines.push(engine); - return engine; -} - -/** How one organization configured one position name. */ -interface PositionSpec { - name: string; - delegatable: boolean; - /** Permission-set NAMES the position distributes in this organization. */ - sets: string[]; - /** Those of `sets` that carry an `adminScope` in this organization. */ - adminSets?: string[]; -} - -/** - * One organization's slice: a unit named `sales` with one child, one member, - * a holder who DIRECTLY holds every position (for self-delegation), and the - * positions exactly as this organization configured them. `idPrefix` decides - * how this organization's ids sort. - */ -async function seedOrganization( - engine: ObjectQL, - organizationId: string, - idPrefix: string, - positions: PositionSpec[], -) { - const ctx = sysCtx(organizationId); - const ids = { - sales: `bu_${idPrefix}_sales`, - salesEast: `bu_${idPrefix}_sales_east`, - member: `usr_${idPrefix}_member`, - holder: `usr_${idPrefix}_holder`, - positionId: (name: string) => `pos_${idPrefix}_${name}`, - }; - await engine.insert('sys_business_unit', [ - { id: ids.sales, name: 'sales', parent_business_unit_id: null }, - { id: ids.salesEast, name: 'sales_east', parent_business_unit_id: ids.sales }, - ], { context: ctx } as any); - await engine.insert('sys_user', [ - { id: ids.member, primary_business_unit_id: ids.salesEast }, - { id: ids.holder, primary_business_unit_id: ids.salesEast }, - ], { context: ctx } as any); - await engine.insert('sys_business_unit_member', [ - { id: `mem_${idPrefix}_member`, user_id: ids.member, business_unit_id: ids.salesEast }, - { id: `mem_${idPrefix}_holder`, user_id: ids.holder, business_unit_id: ids.salesEast }, - ], { context: ctx } as any); - - const setNames = [...new Set(positions.flatMap((p) => p.sets))]; - const adminSets = new Set(positions.flatMap((p) => p.adminSets ?? [])); - if (setNames.length > 0) { - await engine.insert('sys_permission_set', setNames.map((n) => ({ - id: `ps_${idPrefix}_${n}`, - name: n, - admin_scope: adminSets.has(n) ? JSON.stringify(SALES_SCOPE) : null, - })), { context: ctx } as any); - } - for (const p of positions) { - await engine.insert('sys_position', [ - { id: ids.positionId(p.name), name: p.name, delegatable: p.delegatable ? '1' : '0' }, - ], { context: ctx } as any); - if (p.sets.length > 0) { - await engine.insert('sys_position_permission_set', p.sets.map((n) => ({ - id: `bind_${idPrefix}_${p.name}_${n}`, - position_id: ids.positionId(p.name), - permission_set_id: `ps_${idPrefix}_${n}`, - })), { context: ctx } as any); - } - // The holder holds it directly — self-delegation rule 4 is satisfied, so - // rules 5 and 6 are what decide. - await engine.insert('sys_user_position', [ - { id: `up_${idPrefix}_holder_${p.name}`, user_id: ids.holder, position: p.name, business_unit_id: ids.salesEast }, - ], { context: ctx } as any); - } - return ids; -} - -function gateOn(engine: ObjectQL) { - return new DelegatedAdminGate({ - ql: engine, - resolveSets: async (context: any) => (context?.principal === 'delegate' ? DELEGATE_SETS : []), - }); -} - -const DAY = 24 * 60 * 60 * 1000; - -/** A holder delegates their OWN position to a colleague, inside their organization. */ -const selfDelegate = ( - gate: DelegatedAdminGate, - organizationId: string, - ids: { holder: string; member: string }, - position: string, -) => - gate.assert({ - object: 'sys_user_position', - operation: 'insert', - data: { - user_id: ids.member, - position, - delegated_from: ids.holder, - valid_until: new Date(Date.now() + 7 * DAY).toISOString(), - reason: 'annual leave', - }, - context: { principal: 'holder', userId: ids.holder, tenantId: organizationId }, - }); - -/** A delegated administrator assigns a position inside their `sales` subtree. */ -const assignAsDelegate = ( - gate: DelegatedAdminGate, - organizationId: string, - ids: { salesEast: string; member: string }, - position: string, -) => - gate.assert({ - object: 'sys_user_position', - operation: 'insert', - data: { user_id: ids.member, position, business_unit_id: ids.salesEast }, - context: { principal: 'delegate', userId: 'usr_delegate', tenantId: organizationId }, - }); - -// ── positionIsDelegatable ───────────────────────────────────────────────── - -describe('DelegatedAdminGate — whether a position is delegatable is answered by the caller\'s own organization', () => { - /** Same name, opposite answers: only org B opted `field_lead` in to delegation. */ - const seedBoth = async (engine: ObjectQL) => { - const b = await seedOrganization(engine, ORG_B, '0', [{ name: 'field_lead', delegatable: true, sets: [] }]); - const a = await seedOrganization(engine, ORG_A, 'a', [{ name: 'field_lead', delegatable: false, sets: [] }]); - return { a, b }; - }; - - it('ground truth: both organizations hold `field_lead`, org B\'s row sorts first and is the delegatable one', async () => { - const engine = await bootEngine(); - const { a, b } = await seedBoth(engine); - - const driver: any = (engine as any).getDriver('sys_position'); - const raw = await driver.knex('sys_position').where({ name: 'field_lead' }).orderBy('id', 'asc'); - expect(raw.map((r: any) => r.id)).toEqual([b.positionId('field_lead'), a.positionId('field_lead')]); - expect(raw.map((r: any) => [r.organization_id, r.delegatable])).toEqual([[ORG_B, '1'], [ORG_A, '0']]); - }); - - it('org A, which did NOT opt in, refuses the self-delegation — org B\'s opt-in does not answer for it', async () => { - const engine = await bootEngine(); - const { a } = await seedBoth(engine); - - await expect(selfDelegate(gateOn(engine), ORG_A, a, 'field_lead')).rejects.toThrow( - /position 'field_lead' is not delegatable/, - ); - }); - - it('org B, which opted in, approves the self-delegation', async () => { - const engine = await bootEngine(); - const { b } = await seedBoth(engine); - - await expect(selfDelegate(gateOn(engine), ORG_B, b, 'field_lead')).resolves.toBeUndefined(); - }); - - it('a position with NO row in the caller\'s organization is not delegatable (fail closed)', async () => { - const engine = await bootEngine(); - // `night_shift` exists — delegatable — in org B only. The org A holder's - // assignment row names it, but org A's catalog has no such position. - await seedOrganization(engine, ORG_B, '0', [{ name: 'night_shift', delegatable: true, sets: [] }]); - const a = await seedOrganization(engine, ORG_A, 'a', []); - await engine.insert('sys_user_position', [ - { id: 'up_a_holder_night_shift', user_id: a.holder, position: 'night_shift', business_unit_id: a.salesEast }, - ], { context: sysCtx(ORG_A) } as any); - - await expect(selfDelegate(gateOn(engine), ORG_A, a, 'night_shift')).rejects.toThrow( - /position 'night_shift' is not delegatable/, - ); - }); -}); - -// ── setsBoundToPosition ─────────────────────────────────────────────────── - -describe('DelegatedAdminGate — the sets a position distributes are the caller\'s own organization\'s bindings', () => { - /** - * Two names, each bound differently per organization. The delegate's scope - * allowlists `sales_user` only, in both organizations. - * - `lead` — org A binds `sales_user` (allowlisted); org B binds `finance_user` (not). - * - `closer` — org A binds `finance_user` (not); org B binds `sales_user` (allowlisted). - */ - const seedBoth = async (engine: ObjectQL) => { - const b = await seedOrganization(engine, ORG_B, '0', [ - { name: 'lead', delegatable: false, sets: ['finance_user'] }, - { name: 'closer', delegatable: false, sets: ['sales_user'] }, - ]); - const a = await seedOrganization(engine, ORG_A, 'a', [ - { name: 'lead', delegatable: false, sets: ['sales_user'] }, - { name: 'closer', delegatable: false, sets: ['finance_user'] }, - ]); - return { a, b }; - }; - - it('org A judges `lead` by its OWN binding (allowlisted) — approved', async () => { - const engine = await bootEngine(); - const { a } = await seedBoth(engine); - - await expect(assignAsDelegate(gateOn(engine), ORG_A, a, 'lead')).resolves.toBeUndefined(); - }); - - it('org A judges `closer` by its OWN binding (not allowlisted) — refused, org B\'s binding does not launder it', async () => { - const engine = await bootEngine(); - const { a } = await seedBoth(engine); - - await expect(assignAsDelegate(gateOn(engine), ORG_A, a, 'closer')).rejects.toThrow( - /position 'closer' distributes permission set 'finance_user', which is not in the scope's allowlist/, - ); - }); - - it('org B gets the mirror-image answers from its own bindings', async () => { - const engine = await bootEngine(); - const { b } = await seedBoth(engine); - const gate = gateOn(engine); - - await expect(assignAsDelegate(gate, ORG_B, b, 'closer')).resolves.toBeUndefined(); - await expect(assignAsDelegate(gate, ORG_B, b, 'lead')).rejects.toThrow( - /position 'lead' distributes permission set 'finance_user', which is not in the scope's allowlist/, - ); - }); - - it('describeDelegableScope offers each organization only the positions its OWN bindings allow', async () => { - const engine = await bootEngine(); - await seedBoth(engine); - const gate = gateOn(engine); - - const reportA = await gate.describeDelegableScope(DELEGATE_SETS, { userId: 'usr_delegate', tenantId: ORG_A }); - const reportB = await gate.describeDelegableScope(DELEGATE_SETS, { userId: 'usr_delegate', tenantId: ORG_B }); - expect(reportA.assignablePositions).toEqual(['lead']); - expect(reportB.assignablePositions).toEqual(['closer']); - }); - - it('self-delegation rule 6 reads the caller\'s own bindings — an admin set bound only in org B does not block org A', async () => { - const engine = await bootEngine(); - // Both opted `relief` in; only org B's `relief` distributes an ADMIN set. - const b = await seedOrganization(engine, ORG_B, '0', [ - { name: 'relief', delegatable: true, sets: ['relief_admin'], adminSets: ['relief_admin'] }, - ]); - const a = await seedOrganization(engine, ORG_A, 'a', [{ name: 'relief', delegatable: true, sets: ['sales_user'] }]); - const gate = gateOn(engine); - - await expect(selfDelegate(gate, ORG_A, a, 'relief')).resolves.toBeUndefined(); - await expect(selfDelegate(gate, ORG_B, b, 'relief')).rejects.toThrow( - /distributes the admin set 'relief_admin' — administration cannot be self-delegated/, - ); - }); - - it('a position with NO row in the caller\'s organization distributes no sets — org B\'s bindings never judge org A', async () => { - const engine = await bootEngine(); - // `ghost` exists only in org B, bound to a set org A's scope does not allowlist. - await seedOrganization(engine, ORG_B, '0', [{ name: 'ghost', delegatable: false, sets: ['finance_user'] }]); - const a = await seedOrganization(engine, ORG_A, 'a', []); - const gate = gateOn(engine); - - // Nothing to offer: org A's catalog has no `ghost`. - const report = await gate.describeDelegableScope(DELEGATE_SETS, { userId: 'usr_delegate', tenantId: ORG_A }); - expect(report.assignablePositions).toEqual([]); - // And the write is judged against the empty binding set org A actually has — - // org B's `finance_user` is not what refuses or approves it. The assignment - // grants nothing at runtime either: the resolver reads position-bound sets - // through the caller's tenant. - await expect(assignAsDelegate(gate, ORG_A, a, 'ghost')).resolves.toBeUndefined(); - }); -}); - -// ── Controls ────────────────────────────────────────────────────────────── - -describe('DelegatedAdminGate — position reads: controls', () => { - it('control (dark) — one organization: its own delegatable + bound answers stand', async () => { - const engine = await bootEngine(); - const a = await seedOrganization(engine, ORG_A, 'a', [ - { name: 'field_lead', delegatable: true, sets: [] }, - { name: 'lead', delegatable: false, sets: ['sales_user'] }, - { name: 'closer', delegatable: false, sets: ['finance_user'] }, - ]); - const gate = gateOn(engine); - - await expect(selfDelegate(gate, ORG_A, a, 'field_lead')).resolves.toBeUndefined(); - await expect(assignAsDelegate(gate, ORG_A, a, 'lead')).resolves.toBeUndefined(); - await expect(assignAsDelegate(gate, ORG_A, a, 'closer')).rejects.toThrow(/not in the scope's allowlist/); - }); - - it('a `single`-posture caller (no organization in context) keeps the by-name answer', async () => { - const engine = await bootEngine(); - const a = await seedOrganization(engine, ORG_A, 'a', [ - { name: 'field_lead', delegatable: true, sets: [] }, - { name: 'closer', delegatable: false, sets: ['finance_user'] }, - ]); - const gate = gateOn(engine); - - await expect(gate.assert({ - object: 'sys_user_position', - operation: 'insert', - data: { - user_id: a.member, - position: 'field_lead', - delegated_from: a.holder, - valid_until: new Date(Date.now() + 7 * DAY).toISOString(), - reason: 'annual leave', - }, - context: { principal: 'holder', userId: a.holder }, - })).resolves.toBeUndefined(); - const report = await gate.describeDelegableScope(DELEGATE_SETS, { userId: 'usr_delegate' }); - expect(report.assignablePositions).toEqual(['field_lead']); - }); -}); diff --git a/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts b/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts index 8cb7674ebe3..08ac0a49020 100644 --- a/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts +++ b/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts @@ -3,6 +3,7 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { DelegatedAdminGate } from './delegated-admin-gate'; +import { bindCatalogFromTables, bindTestSecurityCatalog } from './__tests__/security-catalog.testkit.js'; /** * Fixture topology @@ -83,6 +84,9 @@ function makeHarness() { return rows[0] ?? null; }, } as any; + // [ADR-0131 D3/D4] The gate reads positions, their bindings and the set + // bodies from the security catalog: the one these rows convert to. + bindCatalogFromTables(ql, tables); // Resolved permission sets per principal — mirrors what // resolvePermissionSetsForContext would return for each context. @@ -376,9 +380,9 @@ describe('DelegatedAdminGate — direct grants (sys_user_permission_set)', () => await expect(remove()).rejects.toThrow(/names no permission set/); }); - it('a grant naming a set no catalog row carries is refused', async () => { + it('a grant naming a set the security catalog does not hold is refused', async () => { storedGrant({ permission_set_id: 'ps_sales', permission_set: 'sales_user_retired' }); - await expect(remove()).rejects.toThrow(/has no catalog row/); + await expect(remove()).rejects.toThrow(/is not in the security catalog/); }); it('an update keeping the set is judged by the stored name; one re-pointing the id by the new id', async () => { @@ -502,6 +506,7 @@ function makeDelegationHarness(nowMs = T0) { return (tables[object] ?? []).filter((r) => matches(r, opts?.where))[0] ?? null; }, } as any; + bindCatalogFromTables(ql, tables); const gate = new DelegatedAdminGate({ ql, resolveSets: async () => [{ name: 'member_default', objects: {} }], @@ -653,6 +658,7 @@ function makeAnchoredDelegationHarness(nowMs = T0) { return (tables[object] ?? []).filter((r) => matches(r, opts?.where))[0] ?? null; }, } as any; + bindCatalogFromTables(ql, tables); const gate = new DelegatedAdminGate({ ql, resolveSets: async () => [{ name: 'member_default', objects: {} }], @@ -925,16 +931,14 @@ describe('DelegatedAdminGate — the anchor is picked by organization, not by ar }); }); -// ── A position NAME is resolved to the caller's own organization's row ───── -// The companion of `delegated-admin-gate-position-organization.test.ts` (a -// real engine), on the same context-ignoring double as the block above: every -// organization's `sys_position` rows — and an organization-less residue row — -// are handed to the gate, listed in id order, so the row-level selection is -// the only thing standing between another organization's row and the two -// position-keyed authority decisions (delegatable; bound sets). -describe('DelegatedAdminGate — a position is picked by organization, not by arrival order', () => { +// ── A position is the security catalog's definition, not an organization's row ─ +// [ADR-0131 D3/D4] The two position-keyed authority decisions (delegatable; +// bound sets) read the position's DEFINITION from the environment catalog by +// name. Every organization's `sys_position` rows and junction rows are handed +// to the gate on a context-ignoring double, and none of them is what decides: +// the catalog is one per deployment, and a row is not a position. +describe('DelegatedAdminGate — a position is the catalog definition, not an organization row', () => { const ORG_A = 'org_a_acme'; - /** Sorts first, and is listed first — the row an unscoped `limit: 1` read answered with. */ const ORG_B = 'org_0_globex'; const FUTURE = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000).toISOString(); @@ -954,24 +958,23 @@ describe('DelegatedAdminGate — a position is picked by organization, not by ar sys_business_unit: [ { id: 'bu_a_sales', name: 'sales', parent_business_unit_id: null, organization_id: ORG_A }, ], + // Rows that SAY otherwise: org B's `closer` is delegatable and bound to + // the allowlisted set, and `night_shift` exists only as a row. sys_position: [ - // An organization-less residue row: invalid state under a walled - // posture, so it answers for no organization's caller. { id: 'pos_', name: 'night_shift', delegatable: true, organization_id: null }, { id: 'pos_0_closer', name: 'closer', delegatable: true, organization_id: ORG_B }, - { id: 'pos_a_closer', name: 'closer', delegatable: false, organization_id: ORG_A }, + { id: 'pos_0_greeter', name: 'greeter', delegatable: false, organization_id: ORG_B }, ], sys_permission_set: [ { id: 'ps_0_sales_user', name: 'sales_user', organization_id: ORG_B }, - { id: 'ps_a_finance_user', name: 'finance_user', organization_id: ORG_A }, ], sys_position_permission_set: [ { id: 'bind_0', position_id: 'pos_0_closer', permission_set_id: 'ps_0_sales_user', organization_id: ORG_B }, - { id: 'bind_a', position_id: 'pos_a_closer', permission_set_id: 'ps_a_finance_user', organization_id: ORG_A }, ], sys_user_position: [ { id: 'up_1', user_id: 'usr_holder', position: 'closer', organization_id: ORG_A }, { id: 'up_2', user_id: 'usr_holder', position: 'night_shift', organization_id: ORG_A }, + { id: 'up_3', user_id: 'usr_holder', position: 'greeter', organization_id: ORG_A }, ], sys_business_unit_member: [], sys_user: [], @@ -984,7 +987,7 @@ describe('DelegatedAdminGate — a position is picked by organization, not by ar } return row[k] === v; }); - return { + const ql = { async find(object: string, opts: any) { const rows = (tables[object] ?? []).filter((r) => matches(r, opts?.where)); return typeof opts?.limit === 'number' ? rows.slice(0, opts.limit) : rows; @@ -993,6 +996,15 @@ describe('DelegatedAdminGate — a position is picked by organization, not by ar return (tables[object] ?? []).filter((r) => matches(r, opts?.where))[0] ?? null; }, } as any; + // The catalog: `closer` is NOT delegatable and distributes `finance_user`; + // `greeter` IS delegatable and distributes `sales_user`; no `night_shift`. + return bindTestSecurityCatalog(ql, { + positions: [ + { name: 'closer', permissionSets: ['finance_user'] }, + { name: 'greeter', delegatable: true, permissionSets: ['sales_user'] }, + ], + permissions: [{ name: 'sales_user' }, { name: 'finance_user' }], + }); } const makeGate = () => @@ -1008,15 +1020,19 @@ describe('DelegatedAdminGate — a position is picked by organization, not by ar context: { principal: 'holder', userId: 'usr_holder', tenantId: ORG_A }, }); - it("refuses a self-delegation org A never opted in to, whatever org B's first row says", async () => { + it('refuses a self-delegation the definition never opted in to, whatever a row says', async () => { await expect(selfDelegate('closer')).rejects.toThrow(/position 'closer' is not delegatable/); }); - it('an organization-less residue row does not make a position delegatable for a walled caller', async () => { + it('a position that exists only as a row is not delegatable', async () => { await expect(selfDelegate('night_shift')).rejects.toThrow(/position 'night_shift' is not delegatable/); }); - it("judges a delegated assignment by org A's own bindings, not org B's first row", async () => { + it('CONTROL: the definition opts in, a row saying otherwise does not refuse it', async () => { + await expect(selfDelegate('greeter')).resolves.toBeUndefined(); + }); + + it("judges a delegated assignment by the definition's permissionSets, not a junction row", async () => { await expect(makeGate().assert({ object: 'sys_user_position', operation: 'insert', diff --git a/packages/plugins/plugin-security/src/delegated-admin-gate.ts b/packages/plugins/plugin-security/src/delegated-admin-gate.ts index b3b72839991..cd23afebc5d 100644 --- a/packages/plugins/plugin-security/src/delegated-admin-gate.ts +++ b/packages/plugins/plugin-security/src/delegated-admin-gate.ts @@ -37,7 +37,7 @@ * reconciliation are unaffected). */ -import { isGrantActive } from '@objectstack/core'; +import { isGrantActive, securityCatalogReaderOf, type SecurityCatalogReader } from '@objectstack/core'; import type { AdminScope, AdminScopeParsed, PermissionSet } from '@objectstack/spec/security'; import { PermissionDeniedError } from './errors.js'; import { @@ -45,13 +45,11 @@ import { rowOrganizationId, seedCtx as organizationScopedCtx, } from './organization-scope.js'; -import { GRANT_SET_ID_FIELD, grantSetNameOf, readGrantSetRows } from './grant-permission-set-name.js'; +import { GRANT_SET_ID_FIELD, grantSetNameOf } from './grant-permission-set-name.js'; const SYSTEM_CTX = { isSystem: true } as const; /** - * Max by-name candidates read when resolving a scope's business-unit anchor, - * or a position's own `sys_position` row (the same shape: `sys_position.name` - * is per-organization too — see `resolveOwnPosition`). + * Max by-name candidates read when resolving a scope's business-unit anchor. * * `sys_business_unit.name` carries NO uniqueness — the object's only unique * index is `(code, organization_id)` — so under a walled posture two @@ -76,11 +74,12 @@ const DEFAULT_DELEGATION_CEILING_MS = 30 * 24 * 60 * 60 * 1000; * active organization, never a scan across all of them. * * The same spelling as `SecurityPlugin.callerOrganizationId`, and deliberately - * so: the `adminScope` this gate resolves arrives on a permission set the - * plugin loaded through `dbLoaderFor(callerOrganizationId(context))`, i.e. out - * of THAT organization's catalog. Resolving the scope's business-unit anchor in - * any other organization pairs an authority minted in one tenant with a tree - * owned by another — which is what an unscoped by-name read did. + * so: the `adminScope` this gate resolves arrives on a permission set resolved + * for THAT organization's caller. Resolving the scope's business-unit anchor in + * any other organization pairs an authority held in one tenant with a tree + * owned by another — which is what an unscoped by-name read did. (The set + * itself is the security catalog's, which is environment-level — ADR-0131 D3; + * the business units are not.) * * Under the `single` posture this is the Default Organization: since ADR-0131 * C1 it exists from boot, and every session on a stock `single` deployment — @@ -237,6 +236,12 @@ interface HeldScope { subtree: Set; } +/** A permission set a position distributes, as the containment check reads it. */ +interface BoundSet { + name: string; + adminScope?: unknown; +} + function rowsOf(opCtx: any): any[] { const d = opCtx?.data; if (Array.isArray(d)) return d; @@ -391,7 +396,7 @@ export class DelegatedAdminGate { switch (opCtx.object) { case 'sys_user_position': - return this.assertAssignmentWrite(opCtx, held, organizationId); + return this.assertAssignmentWrite(opCtx, held); case 'sys_user_permission_set': return this.assertDirectGrantWrite(opCtx, held); case 'sys_position_permission_set': @@ -465,18 +470,20 @@ export class DelegatedAdminGate { ): Promise { const ql = this.deps.ql; const organizationId = callerOrganizationId(callerContext); + // [ADR-0131 D3/D4] Every position the security catalog holds — the + // environment registry, one namespace per deployment — never the + // per-organization `sys_position` rows: a position declared only in the + // registry is assignable, and a row with no definition grants nothing + // (the resolver reads the catalog), so it is not offered. const allPositions = async (): Promise => { - if (!ql?.find) return []; + const catalog = this.catalog(); + if (!catalog) return []; try { - const rows = await ql.find('sys_position', { - limit: 1000, - context: organizationScopedCtx(organizationId), - }); - return (Array.isArray(rows) ? rows : []) - .map((r: any) => String(r?.name ?? '')) + return (await catalog.list('position')) + .map((entry) => entry.name) .filter((n) => n && !ANCHOR_POSITIONS.has(n)); } catch { - return []; + return []; // a picker over an unreadable catalog offers nothing (fail closed) } }; @@ -530,7 +537,12 @@ export class DelegatedAdminGate { const assignablePositions: string[] = []; if (placing.length > 0) { for (const positionName of await allPositions()) { - const boundSets = await this.setsBoundToPosition(positionName, organizationId); + let boundSets: BoundSet[]; + try { + boundSets = await this.setsBoundToPosition(positionName); + } catch { + continue; // a position whose binding cannot be read is not offered + } const ok = placing.some((s) => boundSets.every( (bound) => @@ -675,12 +687,12 @@ export class DelegatedAdminGate { } // 5. The position must opt in to delegation. - if (!(await this.positionIsDelegatable(positionName, organizationId))) { + if (!(await this.positionIsDelegatable(positionName))) { deny(`position '${positionName}' is not delegatable — set delegatable: true on the position to allow it`, { position: positionName }); } // 6. A delegatable position must not distribute administration. - const boundSets = await this.setsBoundToPosition(positionName, organizationId); + const boundSets = await this.setsBoundToPosition(positionName); for (const b of boundSets) { if (parseMaybeJson((b as any).admin_scope ?? (b as any).adminScope)) { deny(`position '${positionName}' distributes the admin set '${b.name}' — administration cannot be self-delegated (D12 containment)`, { position: positionName, permissionSet: b.name }); @@ -728,49 +740,23 @@ export class DelegatedAdminGate { })); } - /** - * Resolve a position NAME to the caller's own organization's `sys_position` - * row — the one read both position-keyed authority decisions below stand on. - * - * `sys_position` is a per-organization catalog upserted by - * `(name, organization_id)`, and `name` carries no installation-wide - * uniqueness, so under a walled posture two organizations may each hold a - * position of the same name. A `limit: 1` read under a bare system context - * answered whichever row the driver ordered first — an id ordering, not an - * organization — and that row then decided whether the position may be - * self-delegated and which permission sets it distributes. - * - * The same two arms {@link resolveSubtree} uses for a business-unit anchor, - * and for the same reasons: the read carries the caller's organization so - * the driver composes a tenant predicate, and {@link resolveOwnOrganizationRow} - * reduces what came back to the caller's OWN row (a driver's compatibility - * arm also returns organization-less rows; a driver with no tenant scoping - * returns every organization's). - * - * Fail closed: a name with no row in the caller's organization resolves to - * `null` — never to another organization's row, never to an unscoped read. - * An organization-less caller (`single` posture) keeps the by-name answer — - * there is no other organization for it to cross into. - */ - private async resolveOwnPosition(positionName: string, organizationId?: string): Promise { - const ql = this.deps.ql; - if (!ql?.find || !positionName) return null; - const rows = await ql.find('sys_position', { - where: { name: positionName }, - limit: ANCHOR_CANDIDATE_LIMIT, - context: organizationScopedCtx(organizationId), - }); - return resolveOwnOrganizationRow(Array.isArray(rows) ? rows : [], organizationId).own; + /** The security catalog bound to this gate's engine (ADR-0131 D3), or `undefined` when none was bound. */ + private catalog(): SecurityCatalogReader | undefined { + return securityCatalogReaderOf(this.deps.ql); } - /** [ADR-0091 D3 rule 5] Does the caller's OWN organization's row for this - * position opt in to delegation? A position with no row there is not - * delegatable (fail closed — see {@link resolveOwnPosition}). */ - private async positionIsDelegatable(positionName: string, organizationId?: string): Promise { + /** + * [ADR-0091 D3 rule 5] Does the position's DEFINITION opt in to delegation + * (`PositionSchema.delegatable`)? Read from the security catalog by name + * (ADR-0131 D3/D4) — the environment registry, one definition per name per + * deployment — never from a per-organization `sys_position` row. A name the + * catalog does not hold, an unbound or unreadable catalog: not delegatable + * (fail closed). + */ + private async positionIsDelegatable(positionName: string): Promise { try { - const pos = await this.resolveOwnPosition(positionName, organizationId); - const v = (pos as any)?.delegatable; - return v === true || v === 1 || v === '1'; + const entry = await this.catalog()?.resolve('position', positionName); + return entry?.definition.delegatable === true; } catch { return false; } @@ -778,16 +764,12 @@ export class DelegatedAdminGate { // ── sys_user_position: user ↔ position assignments ────────────────── - private async assertAssignmentWrite( - opCtx: any, - held: HeldScope[], - organizationId?: string, - ): Promise { + private async assertAssignmentWrite(opCtx: any, held: HeldScope[]): Promise { const targets = await this.materializeTargets(opCtx, 'sys_user_position'); for (const t of targets) { const buId = t.next?.business_unit_id ?? null; const positionName = String(t.next?.position ?? t.prev?.position ?? ''); - const boundSets = positionName ? await this.setsBoundToPosition(positionName, organizationId) : []; + const boundSets = positionName ? await this.setsBoundToPosition(positionName) : []; const failure = this.firstApprovalFailure(held, (s) => { if (!s.scope.manageAssignments) return 'the scope does not grant manageAssignments'; @@ -1232,39 +1214,39 @@ export class DelegatedAdminGate { return [{ next: null, prev }]; } - /** The permission sets the caller's OWN organization's row for this position - * distributes. A position with no row there distributes nothing (fail - * closed — see {@link resolveOwnPosition}); the bindings and sets are then - * read by the resolved row's id, which no other organization shares. */ - private async setsBoundToPosition( - positionName: string, - organizationId?: string, - ): Promise> { - const ql = this.deps.ql; - if (!ql?.find) return []; - try { - const pos = await this.resolveOwnPosition(positionName, organizationId); - if (!pos?.id) return []; - const bindings = await ql.find('sys_position_permission_set', { - where: { position_id: pos.id }, - limit: 1000, - context: SYSTEM_CTX, - }); - const setIds = (Array.isArray(bindings) ? bindings : []) - .map((b: any) => b?.permission_set_id) - .filter(Boolean); - if (setIds.length === 0) return []; - const setRows = await ql.find('sys_permission_set', { - where: { id: { $in: setIds } }, - limit: setIds.length, - context: SYSTEM_CTX, - }); - return (Array.isArray(setRows) ? setRows : []) - .map((r: any) => ({ name: String(r?.name ?? ''), admin_scope: r?.admin_scope })) - .filter((r: any) => r.name); - } catch { - return []; + /** + * [ADR-0131 D3/D4] The permission sets a position distributes: the names its + * DEFINITION carries (`PositionSchema.permissionSets`), each with the + * `adminScope` of the catalog's definition of that name — the binding the + * authorization resolver grants through, read from the same security + * catalog, never from a `sys_position_permission_set` junction row (which + * grants nothing since the resolver stopped reading it). So the D12 + * containment and the assignable-positions listing judge exactly what an + * assignment will grant. + * + * A name the definition carries that the catalog does not hold is still + * returned, with no `adminScope`: it grants nothing today, but the binding is + * declared, and a set authored under that name later starts granting with no + * further write — so a delegate must have it allowlisted too (refuses more, + * never less). A position the catalog does not hold distributes nothing. + * + * ⛔ A catalog that cannot be read is not "distributes nothing": that would + * approve every assignment's allowlist check. The read's own loud failure + * (`AuthzStoreUnavailableError`) propagates, and the write is refused with it. + */ + private async setsBoundToPosition(positionName: string): Promise { + const catalog = this.catalog(); + if (!catalog || !positionName) return []; + const position = await catalog.resolve('position', positionName); + const names = Array.isArray(position?.definition.permissionSets) + ? (position.definition.permissionSets as unknown[]).filter((n): n is string => typeof n === 'string' && n !== '') + : []; + const out: BoundSet[] = []; + for (const name of new Set(names)) { + const set = await catalog.resolve('permission', name); + out.push({ name, ...(set?.definition.adminScope ? { adminScope: set.definition.adminScope } : {}) }); } + return out; } /** @@ -1272,15 +1254,17 @@ export class DelegatedAdminGate { * * - **The grant's own set** — a delete, or an update that keeps the grant's * set: read BY NAME from the grant's `permission_set` (the pre-image's, or - * the name the update supplies), through {@link readGrantSetRows}. A grant - * that names nothing, or names a set with no catalog row it applies to, - * is not one a delegate can be shown to hold authority over, so the write - * is refused (`unresolved`) — fail closed; a tenant admin is not judged - * here at all. + * the name the update supplies), in the security catalog (ADR-0131 D3) — + * the definition the resolver grants. A grant that names nothing, or names + * a set the catalog does not hold, is not one a delegate can be shown to + * hold authority over, so the write is refused (`unresolved`) — fail + * closed; a tenant admin is not judged here at all. * - **The set the write points the grant AT** — an insert, or an update that - * re-points `permission_set_id`: the caller's own reference, read by that - * id as before. The name is not the caller's to supply (the platform - * derives it from that id after this gate), so there is no name to read. + * re-points `permission_set_id`: the caller's own reference. The name is + * not the caller's to supply (the platform derives it from that id after + * this gate), so the id is read to its row's NAME — the grant id lookup, + * which goes with the `permission_set_id` column — and the body judged is + * the catalog's definition of that name. */ private async directGrantSet( t: { next: any | null; prev: any | null }, @@ -1289,8 +1273,10 @@ export class DelegatedAdminGate { const repointed = !!t.next && (!t.prev || String(t.next[GRANT_SET_ID_FIELD] ?? '') !== String(t.prev[GRANT_SET_ID_FIELD] ?? '')); if (repointed) { - const setRow = await this.loadSetRowById(row[GRANT_SET_ID_FIELD]); - return { setRow, setName: String(setRow?.name ?? row[GRANT_SET_ID_FIELD] ?? '') }; + const idRow = await this.loadSetRowById(row[GRANT_SET_ID_FIELD]); + const idName = typeof idRow?.name === 'string' && idRow.name !== '' ? idRow.name : undefined; + const setRow = idName ? await this.catalogSet(idName) : null; + return { setRow, setName: idName ?? String(row[GRANT_SET_ID_FIELD] ?? '') }; } const name = grantSetNameOf(row); if (!name) { @@ -1300,23 +1286,28 @@ export class DelegatedAdminGate { unresolved: 'the grant names no permission set (it is not yet named, or could not be) — only a tenant admin may change it', }; } - const setRow = await this.loadSetRowForGrant(row); + const setRow = await this.catalogSet(name); if (!setRow) { return { setRow: null, setName: name, - unresolved: `permission set '${name}' has no catalog row this grant applies to — only a tenant admin may change it`, + unresolved: `permission set '${name}' is not in the security catalog, so this grant confers nothing — only a tenant admin may change it`, }; } - return { setRow, setName: String(setRow.name ?? name) }; + return { setRow, setName: name }; } - /** The catalog row a stored grant names, by its name (`null` when none applies or the read fails). */ - private async loadSetRowForGrant(grant: any): Promise { - if (!this.deps.ql?.find) return null; + /** + * The security catalog's definition of the permission set `name`, as the + * containment check reads it (`{ name, adminScope }`), or `null` when the + * catalog holds no such set, none is bound, or the read fails (fail closed: + * the caller refuses a delegate's write it cannot judge). + */ + private async catalogSet(name: string): Promise { try { - const setRowOf = await readGrantSetRows(this.deps.ql, [grant]); - return setRowOf(grant) ?? null; + const entry = await this.catalog()?.resolve('permission', name); + if (!entry) return null; + return { name: entry.name, ...(entry.definition.adminScope ? { adminScope: entry.definition.adminScope } : {}) }; } catch { return null; } diff --git a/packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts b/packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts index 0fda95893b0..e47e483edc3 100644 --- a/packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts +++ b/packages/plugins/plugin-security/src/engine-find-bare-array.pin.test.ts @@ -1,7 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#15598] What `ObjectQL.find` actually resolves at the seven seams that used + * [#15598] What `ObjectQL.find` actually resolves at the seams that used * to normalize it — one pin per seam, each driven against a REAL engine. * * ## The class, and why a type was not allowed to settle it @@ -17,7 +17,7 @@ * Every seam answered `[object Array]`, with no own `records` key, on a * populated page and an empty one alike. * - * ⚠️ These are seven pins, not one pin repeated. A single reading of "the engine + * ⚠️ These are six pins, not one pin repeated. A single reading of "the engine * returns an array" would say nothing about whether a given seam is even * reachable — six of them sit behind a trigger (a truncated page, a refused * bulk write, a publish-materializer call path), and a seam nothing reaches is @@ -25,15 +25,12 @@ * through the real function and asserts the answer that block produced, and * {@link assertBareArrayPage} then reads back the value the engine handed it. * - * ## The seventh is the opposite defect, and its legs say so - * - * `security-plugin.ts`'s `sys_permission_set` loader was not merely carrying a - * dead limb: it swallowed a THROWN read into `[]` and mapped an unreadable - * result to `[]` as well, so three different facts left as one value. On the - * enforcement plane that is a silent withdrawal of grants that exist. Its legs - * below pin the repaired direction — the fault PROPAGATES, the refusal carries - * an envelope, and `PermissionEvaluator`'s #2565 warn (unreachable while the - * loader swallowed) fires while the request stays fail-closed. + * The seventh seam, `security-plugin.ts`'s `sys_permission_set` loader, is + * gone: the plugin takes a set's body from the security catalog + * (ADR-0131 D3/D4) and reads no `sys_permission_set` row, so it has no + * engine page to pin. Its fault direction (a read that did not answer + * propagates, and `PermissionEvaluator`'s #2565 warn fires) is pinned on the + * catalog loader in `security-readers-catalog.test.ts`. */ import { describe, it, expect, afterEach } from 'vitest'; @@ -58,8 +55,6 @@ import { upsertPackagePermissionSet } from './bootstrap-declared-permissions.js' import { reconcileOrgAdminGrant } from './auto-org-admin-grant.js'; import { claimSeedOwnership } from './claim-seed-ownership.js'; import { normalizeManagedByVocab } from './normalize-managed-by.js'; -import { SecurityPlugin } from './security-plugin.js'; -import { PermissionEvaluator } from './permission-evaluator.js'; const SYS = { context: { isSystem: true } } as any; const ORG = 'org_a'; @@ -342,124 +337,3 @@ describe('[#15598] block 6 — normalize-managed-by tryFind', () => { expect(rows[0].managed_by).toBe('platform'); }, 120_000); }); - -describe('[#15598] block 7 — security-plugin sys_permission_set loader (the DROP shape)', () => { - /** Boot the real plugin against `engine`, and hand back its private loader. */ - async function loaderOver(engine: any, seen: Seen, findOverride?: (o: string, q?: any, opt?: any) => Promise) { - const plugin = new SecurityPlugin(); - const svc = observed(engine, seen, findOverride ? { find: findOverride } : {}); - const { warns, logger } = recordingLogger(); - const ctx: any = { - logger, - registerService: () => { /* noop */ }, - registerMiddleware: () => { /* noop */ }, - getService: (n: string) => { - if (n === 'objectql') return svc; - if (n === 'metadata') return { list: async () => [] }; - if (n === 'manifest') return { register: () => { /* noop */ } }; - return undefined; - }, - }; - await plugin.init(ctx); - await plugin.start(ctx); - const loader = (plugin as any).dbLoaderFor?.(ORG); - expect(typeof loader, 'the loader was never built — the boot bailed out').toBe('function'); - return { loader: loader as (names: string[]) => Promise, warns }; - } - - it('loads the DB-authored set out of one bare array', async () => { - const engine = await boot(); - await (engine as any).insert( - 'sys_permission_set', - { name: 'ps_db', label: 'DB authored', managed_by: 'admin', organization_id: ORG, active: true }, - SYS, - ); - const seen: Seen = []; - const { loader } = await loaderOver(engine, seen); - seen.length = 0; // isolate the loader's OWN read - - const sets = await loader(['ps_db']); - - assertBareArrayPage(seen, 'dbLoaderFor'); - expect(sets.map((s: any) => s.name)).toEqual(['ps_db']); - }, 120_000); - - it('PROPAGATES a thrown read instead of swallowing it into "no permission sets"', async () => { - const engine = await boot(); - const seen: Seen = []; - const outage = Object.assign(new Error("Datasource 'primary' is declared but not connected"), { - code: 'ERR_DATASOURCE_UNAVAILABLE', - }); - const { loader } = await loaderOver(engine, seen, async (o: string, q?: any, opt?: any) => { - if (o === 'sys_permission_set' && q?.where?.name?.$in) throw outage; - return engine.find(o, q, opt); - }); - - // The whole repair: this used to resolve `[]`. An outage and an empty - // catalog are different facts and must not leave by the same door. - await expect(loader(['ps_db'])).rejects.toBe(outage); - }, 120_000); - - it('REFUSES a non-array result with an envelope, rather than inventing an empty page', async () => { - const engine = await boot(); - const seen: Seen = []; - const { loader } = await loaderOver(engine, seen, async (o: string, q?: any, opt?: any) => { - // The #13706 shape, planted deliberately: a `find()` that resolves an - // ENVELOPE. It is what the removed limb claimed to handle, and the point - // of the repair is that it is refused rather than silently normalized. - if (o === 'sys_permission_set' && q?.where?.name?.$in) return { records: [{ name: 'ps_db' }] } as any; - return engine.find(o, q, opt); - }); - - // ⛔ Not `toThrow()` alone — the envelope is the assertion. A bare "it threw" - // passes against any accident on this path. - await expect(loader(['ps_db'])).rejects.toMatchObject({ - code: 'DATABASE_ERROR', - status: 500, - name: 'PermissionSetReadUnansweredError', - }); - }, 120_000); - - it('REFUSES a page carrying a non-row, which the trailing filter used to drop in silence', async () => { - const engine = await boot(); - const seen: Seen = []; - const { loader } = await loaderOver(engine, seen, async (o: string, q?: any, opt?: any) => { - if (o === 'sys_permission_set' && q?.where?.name?.$in) return ['ps_db'] as any; - return engine.find(o, q, opt); - }); - - await expect(loader(['ps_db'])).rejects.toMatchObject({ - code: 'DATABASE_ERROR', - status: 500, - name: 'PermissionSetReadUnansweredError', - }); - }, 120_000); - - it('stays FAIL-CLOSED through the evaluator, and re-arms the #2565 warn the swallow had made unreachable', async () => { - const engine = await boot(); - const seen: Seen = []; - const outage = Object.assign(new Error("Datasource 'primary' is declared but not connected"), { - code: 'ERR_DATASOURCE_UNAVAILABLE', - }); - const { loader } = await loaderOver(engine, seen, async (o: string, q?: any, opt?: any) => { - if (o === 'sys_permission_set' && q?.where?.name?.$in) throw outage; - return engine.find(o, q, opt); - }); - - const warns: Array<{ msg: string; meta: any }> = []; - const resolved = await new PermissionEvaluator().resolvePermissionSets( - ['ps_db'], - { list: async () => [] }, - [], - loader, - { logger: { warn: (msg: string, meta?: any) => { warns.push({ msg, meta }); } } }, - ); - - // Enforcement is UNCHANGED — the unresolved set still grants nothing. - expect(resolved).toEqual([]); - // What changed is that the loss is now sayable. While the loader swallowed - // its own read failure this warn could never fire, so a DB outage and an - // empty catalog produced identical, undiagnosable 403s. - expect(warns.map((w) => w.msg).join('\n')).toContain('db lookup failed'); - }, 120_000); -}); diff --git a/packages/plugins/plugin-security/src/errors.test.ts b/packages/plugins/plugin-security/src/errors.test.ts index a1ff1f2badd..70f3c1be9ae 100644 --- a/packages/plugins/plugin-security/src/errors.test.ts +++ b/packages/plugins/plugin-security/src/errors.test.ts @@ -40,9 +40,9 @@ const FLOOR = [ 'DetailRecordNotFoundError', 'MasterReferenceMissingError', 'MaskedValueWriteError', - 'PermissionSetReadUnansweredError', 'ExplainObjectNotFoundError', 'PermissionSetNameConflictError', + 'PositionNameConflictError', ]; /** diff --git a/packages/plugins/plugin-security/src/errors.ts b/packages/plugins/plugin-security/src/errors.ts index 99d282ed9fd..70006b01035 100644 --- a/packages/plugins/plugin-security/src/errors.ts +++ b/packages/plugins/plugin-security/src/errors.ts @@ -263,61 +263,6 @@ export class MaskedValueWriteError extends Error { } } -/** - * The DB-authored permission-set read did not answer → `500 DATABASE_ERROR`. - * - * ## Why this exists at all — a read that did not happen is not an empty answer - * - * `SecurityPlugin`'s `sys_permission_set` loader is the ENFORCEMENT plane's door - * to DB-authored sets. It used to map three different facts onto one output, the - * empty list: the read succeeded and the catalog is empty; the read THREW; the - * read resolved something the loader could not read. The last two are invented - * answers, and the invention is not benign — "this principal has no permission - * sets" silently withdraws grants that exist, while every request keeps looking - * completely normal. That is the failure the loader's own neighbouring comment - * calls out as the thing not to do: dropping a row "revokes standing access with - * no signal at the moment of loss". - * - * ## Why REFUSING is the right direction, and not this seam's own invention - * - * The consumer already declares the handling. `PermissionEvaluator - * .resolvePermissionSets` catches a throwing loader, keeps the request - * fail-closed (the unresolved sets grant nothing — unchanged), and NAMES the - * failure in a warn, because "without the warn, a transient DB error makes - * custom permission sets silently vanish and the resulting 403s are - * undiagnosable" (#2565). A loader that swallowed its own read failure made - * that warn unreachable: the diagnostic the repo had already built could never - * fire. Throwing restores it. It also matches the maintainer's 2026-08-11 - * store-fault ruling for this plugin — FAIL-CLOSED, and a fault PROPAGATES; - * absent means absent and only absent. - * - * ⚠️ Enforcement is unchanged in both directions: before and after, a read that - * did not answer grants nothing. What changes is that it is now DISTINGUISHABLE - * from an empty catalog. - * - * `DATABASE_ERROR` comes from ADR-0112's closed vocabulary rather than a new - * spelling — the condition is exactly "a database operation did not answer". - * The status is declared for consistency with this file's other classes, not - * because the error is expected to reach a transport: its only consumer catches - * it one frame up. - */ -export class PermissionSetReadUnansweredError extends Error { - readonly code = 'DATABASE_ERROR'; - readonly status = 500; - readonly statusCode = 500; - /** The set names the unanswered read was asked for. */ - readonly names: readonly string[]; - constructor(names: readonly string[], detail: string) { - super( - `[Security] The sys_permission_set read for [${names.join(', ')}] did not answer: ${detail}. ` - + `Refusing rather than reporting an empty catalog — an unanswered read reported as "no permission ` - + `sets" silently withdraws grants that exist while every request still looks normal.`, - ); - this.name = 'PermissionSetReadUnansweredError'; - this.names = [...names]; - } -} - /** * [#18253] `explain` was asked about an object name that no declaration on this * runtime answers to → `404 OBJECT_NOT_FOUND`. @@ -465,6 +410,37 @@ export class PermissionSetNameConflictError extends Error { } } +/** + * [ruling Q (a), ADR-0131 D3] A Setup create of a position, or a rename into a + * name, that the security catalog already holds a definition of under the + * `single` posture: a second holder of one position name per deployment → + * `409 UNIQUE_VIOLATION` (`position-write-through.ts`). + * + * The wire identity is the one this platform already answers for a second + * `sys_position` row of a name in ONE organization (the engine's unique index, + * reported as `409 UNIQUE_VIOLATION`), and the one + * {@link PermissionSetNameConflictError} answers for the sibling catalog — one + * condition, one envelope, whichever layer catches it. It stamps the same + * named constants, so it adds no second emitter spelling of the code. + */ +export class PositionNameConflictError extends Error { + readonly code = PERMISSION_SET_NAME_CONFLICT_CODE; + readonly status = PERMISSION_SET_NAME_CONFLICT_STATUS; + readonly statusCode = PERMISSION_SET_NAME_CONFLICT_STATUS; + /** The position name that is already held. */ + readonly positionName: string; + constructor(positionName: string, holder?: string) { + super( + `[Security] a position named '${positionName}' already exists in this deployment` + + `${holder ? ` (declared by '${holder}')` : ''}: under the single tenancy posture a position name ` + + 'is unique per deployment, and two holders of one name are never merged. Choose another name, ' + + 'or edit the existing position.', + ); + this.name = 'PositionNameConflictError'; + this.positionName = positionName; + } +} + export function isPermissionDeniedError(e: unknown): e is PermissionDeniedError { if (!e || typeof e !== 'object') return false; const anyE = e as any; diff --git a/packages/plugins/plugin-security/src/explain-engine.test.ts b/packages/plugins/plugin-security/src/explain-engine.test.ts index 743f5d43c80..87d23d8cb9c 100644 --- a/packages/plugins/plugin-security/src/explain-engine.test.ts +++ b/packages/plugins/plugin-security/src/explain-engine.test.ts @@ -873,10 +873,10 @@ function makeGrantQl(tables: Rows) { return bindCatalogFromTables({ async find(object: string, opts: any) { const where = opts?.where ?? {}; - // [ADR-0131 D3] The resolver reads deactivation from the activation - // ledger: a fixture's `active: false` rows reach it as the ledger rows the - // upgrade ceremony converts them to. The explainer's own "deactivated" - // annotation still reads the row (stage 2 moves it). + // [ADR-0131 D3] The resolver AND the explainer's "deactivated" + // annotation read deactivation from the activation ledger: a fixture's + // `active: false` rows reach both as the ledger rows the upgrade ceremony + // converts them to, unless the fixture states its ledger itself. const stored = object === 'sys_metadata_activation' && !tables[object] ? ledgerFromTables(tables) : tables[object]; return (stored ?? []).filter((row) => Object.entries(where).every(([key, cond]) => { if (key.startsWith('$')) throw new Error(`fake driver: unsupported operator ${key}`); @@ -1087,6 +1087,60 @@ describe('buildContextForUser', () => { ]); }); + // [ADR-0131 D3, ADR-0126 §4] Explain and enforce read ONE switch: the + // activation ledger, through the resolver's own `readDisabledCatalogNames`. + // Each case below states the ledger and the rows apart, so the two disagree, + // and the explainer must side with what the resolver enforces. + it('a set and a position the LEDGER switched off are reported deactivated, whatever their rows say', async () => { + const ql = makeGrantQl({ + sys_user_position: [{ user_id: 'u2', position: 'field_auditor' }], + sys_position: [{ id: 'pos_fa', name: 'field_auditor', active: true }], + sys_user_permission_set: [{ user_id: 'u2', permission_set_id: 'psOff', permission_set: 'crm_full' }], + sys_permission_set: [{ id: 'psOff', name: 'crm_full', active: true }], + sys_metadata_activation: [ + { metadata_type: 'position', name: 'field_auditor', active: false }, + { metadata_type: 'permission', name: 'crm_full', active: 0 }, + ], + }); + const ctx = await buildContextForUser(ql, 'u2', NOW); + // Enforcement dropped both… + expect(ctx.positions).toEqual(['everyone']); + expect(ctx.permissions).toEqual([]); + // …and explain says why, from the same ledger. + expect(ctx.droppedGrants).toEqual([ + { kind: 'position', name: 'field_auditor', state: 'deactivated' }, + { kind: 'permission_set', name: 'crm_full', state: 'deactivated' }, + ]); + }); + + it("a catalog ROW's `active: false` with no ledger row is in effect, and explain reports nothing dropped", async () => { + const ql = makeGrantQl({ + sys_user_position: [{ user_id: 'u2', position: 'field_auditor' }], + sys_position: [{ id: 'pos_fa', name: 'field_auditor', active: false }], + sys_user_permission_set: [{ user_id: 'u2', permission_set_id: 'psOff', permission_set: 'crm_full' }], + sys_permission_set: [{ id: 'psOff', name: 'crm_full', active: false }], + sys_metadata_activation: [], + }); + const ctx = await buildContextForUser(ql, 'u2', NOW); + expect(ctx.positions).toEqual(['field_auditor', 'everyone']); + expect(ctx.permissions).toEqual(['crm_full']); + expect(ctx.droppedGrants).toEqual([]); + }); + + it('a grant naming a set the catalog does not hold is reported neither expired nor deactivated — there is no set to lose', async () => { + // No `sys_permission_set` row, so the catalog these tables convert to holds + // neither name; the ledger switches one of them off all the same. + const ql = makeGrantQl({ + sys_user_permission_set: [ + { user_id: 'u2', permission_set_id: 'ps_gone', permission_set: 'gone_set', valid_until: '2026-06-01T00:00:00Z' }, + { user_id: 'u2', permission_set_id: 'ps_gone2', permission_set: 'gone_off' }, + ], + sys_metadata_activation: [{ metadata_type: 'permission', name: 'gone_off', active: false }], + }); + const ctx = await buildContextForUser(ql, 'u2', NOW); + expect(ctx.droppedGrants).toEqual([]); + }); + it('an absent `active` column keeps granting and is NOT reported — absent is ACTIVE (ADR-0049)', async () => { const qlAbsent = makeGrantQl({ sys_user_position: [{ user_id: 'u2', position: 'hr_specialist' }], diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index 1d8f2ff143a..3e958635bfb 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -22,9 +22,10 @@ import { isGrantActive, isGrantExpired, - isRowActive, derivePosture as deriveAdminPosture, + readDisabledCatalogNames, resolveUserAuthzGrants, + securityCatalogReaderOf, } from '@objectstack/core'; import { crossFieldClassRefusalCarriedBy, @@ -66,7 +67,7 @@ import { unresolvedPostureExplainDetail, type UnresolvedPostureCause, } from './unresolved-posture.js'; -import { grantSetNameOf, readGrantSetRows } from './grant-permission-set-name.js'; +import { grantSetNameOf } from './grant-permission-set-name.js'; const SYSTEM_CTX = { isSystem: true } as const; @@ -450,7 +451,7 @@ function untilOfGrantRow(r: any): string | undefined { * `contributors[].state`): a grant row the principal HOLDS that the resolver * fail-closed DROPPED, with the closed reason enumeration naming why — * `expired` (ADR-0091 D2 validity window) or `deactivated` (ADR-0049 / #8613 - * catalogue `active` switch). `until` is the window bound, present only for + * switch, read from the `sys_metadata_activation` ledger). `until` is the window bound, present only for * the expired reason. One array, one reason discriminant — deliberately NOT a * sibling array per cause (maintainer ruling 2026-08-18). */ @@ -477,20 +478,25 @@ export interface DroppedGrant { * "held until … — expired" instead of silently omitting a grant the admin * knows they granted. This is "why did access DISAPPEAR", and only a dropped * row can answer it. - * - **deactivated** (commit 42b05af89) — a row whose CATALOGUE entry - * (`sys_permission_set.active` / `sys_position.active`) is switched off - * (ADR-0049 / #8613), so the grant stopped resolving for everyone holding - * it. Deactivation is an incident-response control with no date on the + * - **deactivated** (commit 42b05af89) — a row whose position or permission + * set is switched off (ADR-0049 / #8613), so the grant stopped resolving + * for everyone holding it. The switch is the activation ledger's + * (`sys_metadata_activation`, ADR-0126 §3 regime C as ADR-0131 D6 amends + * it), asked through the resolver's own read (`readDisabledCatalogNames`), + * so explain and enforce cannot disagree; a catalog row's `active` column + * is not read. Deactivation is an incident-response control with no date on the * user's own grant row, so without this state the set is simply ABSENT and * explain answers like the grant never existed — the exact silence the * contributor-attribution feature exists to prevent. * - **delegated** — the `delegated_from` provenance of a row that DID resolve, * so a position can be attributed "via delegation from X, until Y". * - * All verdicts come from the SAME shared predicate modules the resolver uses - * (`isGrantActive` / `isGrantExpired` for the ADR-0091 window, `isRowActive` - * for the ADR-0049 switch — all `@objectstack/core`) — one implementation of - * each rule, consulted twice, never re-derived here. + * All verdicts come from the SAME shared reads the resolver uses + * (`isGrantActive` / `isGrantExpired` for the ADR-0091 window, + * `readDisabledCatalogNames` for the ADR-0049 switch, the security catalog + * (`securityCatalogReaderOf`) for whether a set exists — all + * `@objectstack/core`) — one implementation of each rule, consulted twice, + * never re-derived here. * * A row that is BOTH expired and deactivated reports `expired`: the window * verdict is judged on the user's own grant row, the same order the resolver @@ -500,9 +506,8 @@ export interface DroppedGrant { * re-read for the grants this pass already walks — `sys_user_position` rows * and direct `sys_user_permission_set` grants. A deactivated position held via * role projection, or a deactivated SET reached only through an active - * position's `sys_position_permission_set` linkage, is not re-derived here — - * that would replicate the resolver's aggregation, which is the design this - * pass exists to avoid. + * position's `permissionSets`, is not re-derived here — that would replicate + * the resolver's aggregation, which is the design this pass exists to avoid. */ async function collectGrantProvenance( ql: any, @@ -535,21 +540,14 @@ async function collectGrantProvenance( droppedGrants.push({ kind: 'position', name: p, state: 'expired', until: untilOfGrantRow(r) }); } } - // [commit 42b05af89 / ADR-0049] A held position whose `sys_position` catalogue row is - // explicitly deactivated was dropped by the resolver (step 6a) — report it - // instead of letting it vanish. A name with no row has no flag to read and - // is untouched, matching the resolver. + // [commit 42b05af89 / ADR-0049] A held position the activation ledger + // switched off was dropped by the resolver (step 6a) — report it instead of + // letting it vanish. The resolver's own ledger read, so a name with no + // ledger row is in effect here exactly as it is there. if (heldPositionNames.size > 0) { - const posRows = await ql.find('sys_position', { - where: { name: { $in: Array.from(heldPositionNames) } }, - limit: heldPositionNames.size, - context: SYSTEM_CTX, - }); - for (const row of Array.isArray(posRows) ? posRows : []) { - const n = String((row as any)?.name ?? ''); - if (n && heldPositionNames.has(n) && !isRowActive(row)) { - droppedGrants.push({ kind: 'position', name: n, state: 'deactivated' }); - } + const disabled = await readDisabledCatalogNames(ql, Array.from(heldPositionNames), []); + for (const n of heldPositionNames) { + if (disabled.positions.has(n)) droppedGrants.push({ kind: 'position', name: n, state: 'deactivated' }); } } } catch { /* table unavailable → no provenance to report */ } @@ -564,27 +562,30 @@ async function collectGrantProvenance( // deactivated — the one remaining reason the resolver drops them (step 6b). const activeRows = grantRows.filter((g: any) => isGrantActive(g, nowMs)); if (expiredRows.length > 0 || activeRows.length > 0) { - // [commit 42b05af89] One `sys_permission_set` read serves both reasons: - // the set row an expired grant names, and the ADR-0049 `active` flag of - // the one a held grant names. [ADR-0131 D4] Read by the grant's NAME — - // its own organization's row, else the organization-less one - // ({@link readGrantSetRows}); deactivation is still the row's flag. - const setRowOf = await readGrantSetRows(ql, [...expiredRows, ...activeRows]); - for (const g of expiredRows) { - const s = setRowOf(g); - const n = (s as any)?.name; - if (n) droppedGrants.push({ kind: 'permission_set', name: String(n), state: 'expired', until: untilOfGrantRow(g) }); + // [ADR-0131 D3/D4] A grant's set is the security catalog's definition of + // the name it carries — the one the resolver grants (step 6). A name the + // catalog does not hold resolves nothing either way, so it is reported + // neither as expired nor as deactivated: there is no set to lose. + const catalog = securityCatalogReaderOf(ql); + const held = new Set(); + for (const g of [...expiredRows, ...activeRows]) { + const n = grantSetNameOf(g) as string; + if (!held.has(n) && catalog && (await catalog.resolve('permission', n))) held.add(n); } - const deactivatedNames = new Set(); - for (const g of activeRows) { - const s = setRowOf(g); - const n = (s as any)?.name; - // Dedupe: several grant rows can point at one deactivated set; the - // catalogue-row fact is reported once. - if (n && !isRowActive(s)) deactivatedNames.add(String(n)); + for (const g of expiredRows) { + const n = grantSetNameOf(g) as string; + if (held.has(n)) droppedGrants.push({ kind: 'permission_set', name: n, state: 'expired', until: untilOfGrantRow(g) }); } - for (const n of deactivatedNames) { - droppedGrants.push({ kind: 'permission_set', name: n, state: 'deactivated' }); + // [commit 42b05af89 / ADR-0049] Whether a held set is switched off is the + // activation ledger's answer, read once for every name a window-active + // grant carries — the resolver's own read. Several grant rows can name + // one deactivated set; the fact is reported once. + const activeNames = [...new Set(activeRows.map((g: any) => grantSetNameOf(g) as string))].filter((n) => held.has(n)); + if (activeNames.length > 0) { + const disabled = await readDisabledCatalogNames(ql, [], activeNames); + for (const n of activeNames) { + if (disabled.permissions.has(n)) droppedGrants.push({ kind: 'permission_set', name: n, state: 'deactivated' }); + } } } } catch { /* table unavailable → no provenance to report */ } diff --git a/packages/plugins/plugin-security/src/get-effective-object-permissions.test.ts b/packages/plugins/plugin-security/src/get-effective-object-permissions.test.ts index 844f14aa909..c8714ba0bc6 100644 --- a/packages/plugins/plugin-security/src/get-effective-object-permissions.test.ts +++ b/packages/plugins/plugin-security/src/get-effective-object-permissions.test.ts @@ -103,19 +103,6 @@ const SCHEMAS: Record = { report: { name: 'report', label: 'Report', enable: { apiMethods: ['get', 'list'] }, fields: { id: { name: 'id' } } }, }; -/** `where` matcher: scalar equality plus the `$in` form the resolver sends; any other operator REFUSES. */ -function matches(row: Record, where: Record | undefined): boolean { - return Object.entries(where ?? {}).every(([key, cond]) => { - if (key.startsWith('$')) throw new Error(`fake engine: unsupported operator ${key}`); - const value = row[key] ?? null; - if (cond && typeof cond === 'object' && Array.isArray((cond as { $in?: unknown }).$in)) { - return ((cond as { $in: unknown[] }).$in).includes(value); - } - if (cond && typeof cond === 'object') throw new Error(`fake engine: unsupported condition on ${key}`); - return value === (cond ?? null); - }); -} - function bootPlugin( opts: { dbRows?: Array>; engineSeam?: boolean; schemas?: Record } = {}, ) { @@ -123,17 +110,33 @@ function bootPlugin( const schemas = opts.schemas ?? SCHEMAS; const permissionSetReads: string[][] = []; const registered: Array<(context: unknown) => Promise> = []; + // [ADR-0131 D3/D4] The sets are the security catalog's definitions — each + // row above as the registry holds it once saved through the metadata door — + // read live, so a set taken away is gone from the next resolution. + const definitionOf = (row: Record) => ({ + name: row.name, + label: row.label, + objects: JSON.parse(String(row.object_permissions ?? '{}')), + fields: JSON.parse(String(row.field_permissions ?? '{}')), + systemPermissions: JSON.parse(String(row.system_permissions ?? '[]')), + tabPermissions: JSON.parse(String(row.tab_permissions ?? '{}')), + }); const ql: any = { registerMiddleware: () => {}, - registry: { getAllObjects: () => Object.values(schemas) }, - getSchema: (name: string) => schemas[name] ?? null, - find: async (object: string, query: any) => { - const tables: Record>> = { sys_permission_set: dbRows }; - if (object === 'sys_permission_set') permissionSetReads.push(query?.where?.name?.$in ?? []); - const rows = (tables[object] ?? []).filter((r) => matches(r, query?.where)); - // Hold the caller's bound (`check:objectql-double-limit`). - return typeof query?.limit === 'number' ? rows.slice(0, query.limit) : rows; + registry: { + getAllObjects: () => Object.values(schemas), + getItem: (type: string, name: string) => { + if (type !== 'permission') return undefined; + permissionSetReads.push([name]); + const row = dbRows.find((r) => r.name === name); + return row ? definitionOf(row) : undefined; + }, + listItems: (type: string) => (type === 'permission' ? dbRows.map(definitionOf) : []), + isPackageDisabled: () => false, }, + getSchema: (name: string) => schemas[name] ?? null, + // No table answers here: every set is the catalog's (above). + find: async () => [], }; if (opts.engineSeam !== false) { ql.registerEffectiveObjectPermissionsResolver = (fn: (context: unknown) => Promise) => registered.push(fn); diff --git a/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts b/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts index 47673f34582..f38ca35784c 100644 --- a/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts +++ b/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts @@ -113,6 +113,12 @@ interface BootOptions { members?: boolean | 'unprovisioned'; } +/** The positions the fixture's assignment rows name, as their definitions. */ +const QA_POSITIONS = [ + { name: 'qa_rep', label: 'Rep' }, + { name: 'qa_lead', label: 'Lead' }, +]; + async function boot(opts: BootOptions = {}) { const withMembers = opts.members === undefined || opts.members === true; const engine = new ObjectQL(); @@ -143,9 +149,12 @@ async function boot(opts: BootOptions = {}) { const services: Record = { manifest: { register: vi.fn() }, objectql: engine, + // [ADR-0131 D3/D4] The two positions the rows name are declared, as the + // security catalog an assignment is judged against reads them. metadata: { - get: async (_type: string, name: string) => engine.getSchema(name) ?? null, - list: async () => [MEMBER_DEFAULT, QA_ADMIN, QA_READ_ONLY_ON_GRANTS], + get: async (type: string, name: string) => + (type === 'position' ? QA_POSITIONS.find((p) => p.name === name) : engine.getSchema(name)) ?? null, + list: async (type: string) => (type === 'position' ? QA_POSITIONS : [MEMBER_DEFAULT, QA_ADMIN, QA_READ_ONLY_ON_GRANTS]), }, ...(opts.walled ? { 'org-scoping': { name: 'com.objectstack.org-scoping' }, tenancy: { posture: 'isolated' } } diff --git a/packages/plugins/plugin-security/src/grant-permission-set-name.ts b/packages/plugins/plugin-security/src/grant-permission-set-name.ts index b2fbdc89864..647783c1b46 100644 --- a/packages/plugins/plugin-security/src/grant-permission-set-name.ts +++ b/packages/plugins/plugin-security/src/grant-permission-set-name.ts @@ -13,9 +13,10 @@ * resolve (D10). Until then the grant carries both, and this module is what * keeps them saying the same thing. Rows written before the column existed are * rewritten by the backfill stage, never here. The grant readers of - * `plugin-security` and `plugin-auth` key on the name ({@link grantSetNameOf}, - * {@link readGrantSetRows}); the authorization resolver in `@objectstack/core` - * still reads the id until its own stage switches it. + * `plugin-security` and `plugin-auth`, and the authorization resolver in + * `@objectstack/core`, key on the name ({@link grantSetNameOf}) and take the + * set itself from the security catalog (ADR-0131 D3), never from a + * `sys_permission_set` row. * * ## The invariant: the two columns never disagree * @@ -257,86 +258,6 @@ export function grantOrganizationOf(row: unknown): string | null { return String(value); } -/** The engine surface {@link readGrantSetRows} reads through. */ -export interface GrantSetRowEngine { - find(object: string, options: Record): Promise; -} - -/** - * [ADR-0131 D4] The `sys_permission_set` row each of `grants` reads its set - * from, BY NAME — for a reader that needs the row's own columns (the ADR-0049 - * `active` flag, a delegated-administration scope), not only the name. - * - * The catalog is still materialized per organization, so one name can carry - * several rows. A grant reads the row of its OWN organization, else the - * organization-less one — the same two rows its id may point at (a set row - * applies to a grant of its own organization, an organization-less row to - * every grant). An organization's rows are read with that organization - * threaded into the context, so the driver's tenant scope decides what is - * visible; an organization-less grant reads organization-less rows only. - * - * A grant that names nothing ({@link grantSetNameOf}) resolves to no row, and - * so does a name no applicable row carries. A read that fails throws: the - * caller decides what a failed read means for its own answer. - */ -export async function readGrantSetRows( - engine: GrantSetRowEngine, - grants: readonly unknown[], -): Promise<(grant: unknown) => Record | undefined> { - const namesByOrganization = new Map>(); - const allNames = new Set(); - for (const grant of grants) { - const name = grantSetNameOf(grant); - if (!name) continue; - allNames.add(name); - const organizationId = grantOrganizationOf(grant); - if (organizationId === null) continue; - let names = namesByOrganization.get(organizationId); - if (!names) { - names = new Set(); - namesByOrganization.set(organizationId, names); - } - names.add(name); - } - const rowsOf = (result: unknown): Array & { name: string }> => - (Array.isArray(result) ? result : []).filter( - (r): r is Record & { name: string } => - !!r && typeof r === 'object' && typeof (r as Record).name === 'string', - ); - const organizationLess = new Map>(); - if (allNames.size > 0) { - const names = [...allNames]; - const rows = rowsOf(await engine.find(PERMISSION_SET_CATALOG_OBJECT, { - where: { name: { $in: names }, organization_id: null }, - limit: names.length * 2 + 1, - context: { isSystem: true }, - })); - for (const row of rows) { - if (grantOrganizationOf(row) === null && !organizationLess.has(row.name)) organizationLess.set(row.name, row); - } - } - const own = new Map>>(); - for (const [organizationId, nameSet] of namesByOrganization) { - const names = [...nameSet]; - const rows = rowsOf(await engine.find(PERMISSION_SET_CATALOG_OBJECT, { - where: { name: { $in: names } }, - limit: names.length * 2 + 1, - context: { isSystem: true, tenantId: organizationId }, - })); - const byName = new Map>(); - for (const row of rows) { - if (grantOrganizationOf(row) === organizationId && !byName.has(row.name)) byName.set(row.name, row); - } - own.set(organizationId, byName); - } - return (grant: unknown) => { - const name = grantSetNameOf(grant); - if (!name) return undefined; - const organizationId = grantOrganizationOf(grant); - return (organizationId !== null ? own.get(organizationId)?.get(name) : undefined) ?? organizationLess.get(name); - }; -} - /** The set row an id points at, as far as a grant's name is concerned. */ interface CatalogRow { name: string; diff --git a/packages/plugins/plugin-security/src/grant-readers-by-name.golden.test.ts b/packages/plugins/plugin-security/src/grant-readers-by-name.golden.test.ts index dec8f180673..5e0267e2c92 100644 --- a/packages/plugins/plugin-security/src/grant-readers-by-name.golden.test.ts +++ b/packages/plugins/plugin-security/src/grant-readers-by-name.golden.test.ts @@ -39,6 +39,7 @@ import { SqlDriver } from '@objectstack/driver-sql'; import { resetPlatformAdminEmailMemo } from '@objectstack/core'; import type { PermissionSet } from '@objectstack/spec/security'; import { SysUser, SysAccount, SysMember, SysOrganization } from '@objectstack/platform-objects/identity'; +import { SysMetadataActivation } from '@objectstack/platform-objects/system'; import { SecurityPlugin } from './security-plugin.js'; import { bootstrapPlatformAdmin, findExistingPlatformAdmin } from './bootstrap-platform-admin.js'; @@ -51,6 +52,9 @@ import { SysPositionPermissionSet } from './objects/sys-position-permission-set. import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; +/** The environment-authored set the member holds a grant of, as the security catalog declares it. */ +const LEGACY_REPORTS = { name: 'legacy_reports', label: 'Legacy reports' }; + const SYS = { isSystem: true } as const; const POSTURE_ENV = 'OS_TENANCY_POSTURE'; const OWNER_ENV = 'OS_PLATFORM_OWNER_EMAIL'; @@ -128,6 +132,8 @@ async function readersByPrincipal(posture: Posture): Promise type === 'permission' - ? ([...defaultPermissionSets, QA_ADMIN] as Array<{ name?: string }>).find((s) => s?.name === name) ?? null + ? ([...defaultPermissionSets, QA_ADMIN, LEGACY_REPORTS] as Array<{ name?: string }>).find((s) => s?.name === name) ?? null : engine.getSchema(name) ?? null, - list: async () => [...defaultPermissionSets, QA_ADMIN], + list: async () => [...defaultPermissionSets, QA_ADMIN, LEGACY_REPORTS], }, ...(walled ? { 'org-scoping': { name: 'com.objectstack.org-scoping' }, tenancy: { posture } } @@ -186,7 +192,14 @@ async function readersByPrincipal(posture: Posture): Promise = [...defaultPermissionSets, { name: 'legacy_reports', label: 'Legacy' }]; const ORG = 'org_un'; const orgCtx = { isSystem: true, tenantId: ORG }; @@ -77,6 +79,8 @@ async function boot(): Promise { objects: [ SysUser, SysAccount, SysMember, SysOrganization, SysPosition, SysUserPosition, SysPermissionSet, SysPositionPermissionSet, SysUserPermissionSet, + // The activation ledger the explainer reads deactivation from (ADR-0131 D3). + SysMetadataActivation, ], } as any); await engine.syncSchemas(); @@ -84,9 +88,12 @@ async function boot(): Promise { const services: Record = { manifest: { register: vi.fn() }, objectql: engine, + // [ADR-0131 D3/D4] The security catalog: the platform's sets plus the + // environment-authored `legacy_reports` the explain case grants. metadata: { - get: async (_type: string, name: string) => engine.getSchema(name) ?? null, - list: async () => [...defaultPermissionSets], + get: async (type: string, name: string) => + (type === 'permission' ? CATALOG_SETS.find((p) => p.name === name) : engine.getSchema(name)) ?? null, + list: async () => [...CATALOG_SETS], }, }; const ctx: any = { @@ -137,7 +144,11 @@ describe('[ADR-0131 D4] a grant that names nothing — explain', () => { const engine = await boot(); await bootstrapPlatformAdmin(engine, defaultPermissionSets); await user(engine, 'usr_member', '2025-03-01T00:00:00.000Z'); - await engine.insert('sys_permission_set', { id: 'ps_off', name: 'legacy_reports', label: 'Legacy', active: false }, { context: SYS } as any); + await engine.insert('sys_permission_set', { id: 'ps_off', name: 'legacy_reports', label: 'Legacy' }, { context: SYS } as any); + // Switched off where the resolver and the explainer read it: the ledger. + await engine.insert('sys_metadata_activation', { + metadata_type: 'permission', name: 'legacy_reports', package_id: 'com.objectstack.qa.grant-readers-unnamed', active: false, + }, { context: SYS } as any); await engine.insert('sys_user_permission_set', [ { id: 'g_off', user_id: 'usr_member', permission_set_id: 'ps_off', organization_id: ORG }, { diff --git a/packages/plugins/plugin-security/src/granted-by-writer-provenance.test.ts b/packages/plugins/plugin-security/src/granted-by-writer-provenance.test.ts index 6a6185b8c19..e4591e8beb1 100644 --- a/packages/plugins/plugin-security/src/granted-by-writer-provenance.test.ts +++ b/packages/plugins/plugin-security/src/granted-by-writer-provenance.test.ts @@ -163,6 +163,12 @@ async function boot(opts: { security: boolean } = { security: true }): Promise { expect(home.setNames).not.toContain('org_member'); }, 120_000); - it('CONTROL · the authoring organization\'s own member, with it active, does resolve its set — the set is resolvable at all', async () => { + // [ADR-0131 D3/D4, #15196 Q3 = A] REVERSED: the plugin no longer reads + // `sys_permission_set` rows at all, so a set an organization authored as a + // row with no catalog definition resolves for nobody — its own members, + // with it active, included. That is the walled-row loss the ruling accepts + // (loud at boot, promoted to the environment catalog by its operator); the + // global sets above resolve because the catalog declares them. + it('a set only an organization ROW carries resolves for nobody, its own active members included', async () => { const r = await rig(); const other = await resolve(r, USER_OTHER, ORG_OTHER); - expect(other.byName.get('org_member')?.systemPermissions).toEqual(['manage_metadata', 'probe.other_org_member']); - expect(other.ledger).toMatchObject({ viewAllRecords: true, modifyAllRecords: true }); + expect(other.byName.get('org_member')).toBeUndefined(); + expect(other.ledger).toBeNull(); + expect(other.loaderRows).toEqual([]); }, 120_000); }); diff --git a/packages/plugins/plugin-security/src/permission-evaluator.ts b/packages/plugins/plugin-security/src/permission-evaluator.ts index 1db23dbe989..cc4ad2b9129 100644 --- a/packages/plugins/plugin-security/src/permission-evaluator.ts +++ b/packages/plugins/plugin-security/src/permission-evaluator.ts @@ -481,8 +481,8 @@ export class PermissionEvaluator { bootstrapPermissionSets: PermissionSet[] = [], /** * Optional async loader for permission set names that aren't found in - * metadata or bootstrap. Lets callers query user-defined permission - * sets persisted in `sys_permission_set`. Failures are swallowed + * metadata or bootstrap — the security plugin passes its catalog loader + * (the security catalog's definitions, ADR-0131 D3/D4). Failures are swallowed * (fail-closed: unresolvable sets grant nothing) but SURFACED via * `options.logger` — see #2565: without the warn, a transient DB error * makes custom permission sets silently vanish and the resulting 403s @@ -533,10 +533,10 @@ export class PermissionEvaluator { } } - // Last-resort: query user-defined permission sets from the database. - // Without this, custom permission sets (created via the admin UI as - // `sys_permission_set` rows) would be silently ignored both for CRUD - // enforcement and for field-level masking. + // Last source: the loader the caller supplies — the security plugin's + // catalog loader, which answers environment-authored and package sets the + // metadata list does not (ADR-0131 D3/D4). Without it those sets would be + // silently ignored both for CRUD enforcement and for field-level masking. if (dbLoader) { const unresolved = identifiers.filter((n) => !seen.has(n)); if (unresolved.length > 0) { @@ -555,7 +555,7 @@ export class PermissionEvaluator { // DB error silently drops custom permission sets and the // resulting 403s point nowhere near the cause (#2565). options.logger?.warn?.( - '[security] sys_permission_set db lookup failed — unresolved sets grant nothing this request', + '[security] permission-set catalog lookup failed — unresolved sets grant nothing this request', { unresolved, error: (e as Error)?.message }, ); } diff --git a/packages/plugins/plugin-security/src/permission-set-active.test.ts b/packages/plugins/plugin-security/src/permission-set-active.test.ts index 673a48df5dd..9bc5df16606 100644 --- a/packages/plugins/plugin-security/src/permission-set-active.test.ts +++ b/packages/plugins/plugin-security/src/permission-set-active.test.ts @@ -1,25 +1,26 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#8613 / ADR-0049] `sys_permission_set.active`, at the DB loader. + * [#8613 / ADR-0049] A deactivated permission set, at the security plugin's + * own set resolution. * - * The dominant enforcement point for this flag is `resolveAuthzContext` in - * `@objectstack/core`, which drops a deactivated set before `context.permissions` - * is built. This loader is the SECOND reader, and it is not defence in depth - * that nothing reaches — the reachability case below is the reason it exists: + * The dominant enforcement point is `resolveUserAuthzGrants` in + * `@objectstack/core`, which drops a deactivated set before + * `context.permissions` is built. The plugin's resolution is the SECOND + * reader, and it is not defence in depth that nothing reaches — the + * reachability case below is the reason it exists: * * `resolvePermissionSetsForContext` requests `context.positions` as - * permission-set NAMES too (a position name is commonly reused as a set name, - * which the evaluator's own doc comment calls out). So an ACTIVE position - * whose name matches a DEACTIVATED `sys_permission_set` row arrives here with - * that name still standing — core filtered the position catalogue and the - * sets reached by id, and neither of those judged THIS row. + * permission-set NAMES too (a position name is commonly reused as a set + * name). So an ACTIVE position whose name matches a DEACTIVATED set arrives + * with that name still standing: core judged the position, not a set of + * the same name. * - * The filter runs in memory over rows the loader already fetched rather than as - * an `active: true` `where` predicate: `true` would also drop rows whose column - * is NULL (rows predating the field — a silent mass revocation on deployed - * data), and boolean `where` coercion differs per driver. `isRowActive` is the - * same predicate core and the break-glass guard use. + * [ADR-0131 D3, ADR-0126 §4] The switch is the activation ledger's + * (`sys_metadata_activation`, type `permission`), read through core's own + * `readDisabledCatalogNames` — the read the resolver takes — and the set's + * body is the security catalog's definition. A `sys_permission_set` row is not + * read at all, its `active` column included. */ import { describe, it, expect, vi } from 'vitest'; @@ -37,39 +38,46 @@ const MEMBER_DEFAULT: PermissionSet = { systemPermissions: [], } as any; -/** A DB-authored set, as it sits in `sys_permission_set`. `active` varies per case. */ -const salesManagerRow = (active: unknown) => ({ +/** An environment-authored set, as the security catalog holds it. */ +const SALES_MANAGER: PermissionSet = { name: 'sales_manager', label: 'Sales Manager', - object_permissions: JSON.stringify({ deal: { allowRead: true, allowEdit: true } }), - system_permissions: JSON.stringify(['setup.access']), - ...(active === undefined ? {} : { active }), -}); + objects: { deal: { allowRead: true, allowEdit: true } }, + systemPermissions: ['setup.access'], +} as any; -/** - * Resolve the security service the way a cross-package consumer does. The fake - * engine matches on `where.name.$in` ONLY — deliberately, because that is the - * predicate the loader has always sent. A filter expressed as an extra `where` - * key would be invisible to this fixture (and to any driver that ignores an - * unknown column), so the assertion below measures the product's own judgment. - */ -async function locateSecurityService( - dbRows: Array>, -): Promise> { +interface Fixture { + /** `sys_metadata_activation` rows. */ + ledger?: Array>; + /** `sys_permission_set` rows — present to prove they are not read. */ + rows?: Array>; +} + +/** Resolve the security service the way a cross-package consumer does, and count the ledger reads. */ +async function locate(fixture: Fixture = {}): Promise<{ svc: Partial; ledgerReads: () => number }> { const schema: any = { name: 'deal', label: 'Deal', systemFields: false, fields: { id: { name: 'id' }, amount: { name: 'amount' } }, }; + let ledgerReads = 0; const ql: any = { registerMiddleware: () => {}, getSchema: (name: string) => (name === 'deal' ? schema : null), findOne: async (object: string, query?: EngineFindOneQueryInput) => { assertEngineFindOnePredicate(object, query); return null; }, find: async (object: string, query: any) => { - if (object !== 'sys_permission_set') return []; - const wanted: string[] = query?.where?.name?.$in ?? []; - return dbRows.filter((r) => wanted.includes(String(r.name))); + if (object === 'sys_permission_set') return fixture.rows ?? []; + if (object !== 'sys_metadata_activation') return []; + ledgerReads += 1; + const types: string[] = query?.where?.metadata_type?.$in ?? []; + const names: string[] = query?.where?.name?.$in ?? []; + return (fixture.ledger ?? []).filter((r) => types.includes(String(r.metadata_type)) && names.includes(String(r.name))); + }, + registry: { + getItem: (type: string, name: string) => (type === 'permission' && name === SALES_MANAGER.name ? SALES_MANAGER : undefined), + listItems: (type: string) => (type === 'permission' ? [SALES_MANAGER] : []), + isPackageDisabled: () => false, }, }; const metadata: any = { @@ -88,64 +96,71 @@ async function locateSecurityService( const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' } as any); await plugin.init(ctx); await plugin.start(ctx); - return ctx.registerService.mock.calls.find((c: any[]) => c[0] === 'security')?.[1]; + ledgerReads = 0; // boot-time reads are not the resolution under test + return { + svc: ctx.registerService.mock.calls.find((c: any[]) => c[0] === 'security')?.[1], + ledgerReads: () => ledgerReads, + }; } -const namesFor = async ( - rows: Array>, - context: Record, -): Promise => { - const svc = await locateSecurityService(rows); +const namesFor = async (fixture: Fixture, context: Record): Promise => { + const { svc } = await locate(fixture); const sets = await svc.resolvePermissionSetsForContext?.({ userId: 'u1', ...context } as any); return (sets ?? []).map((s) => s.name); }; -describe('[#8613] the DB loader drops DEACTIVATED permission sets', () => { - it('a deactivated set requested by name resolves to nothing', async () => { - const names = await namesFor([salesManagerRow(false)], { permissions: ['sales_manager'] }); +const switchedOff = (active: unknown) => ({ metadata_type: 'permission', name: 'sales_manager', active }); + +describe('[#8613] a position name reaching a set the activation ledger switched off', () => { + it('THE REACHABILITY CASE: the folded name resolves nothing; the baseline still applies', async () => { + const names = await namesFor({ ledger: [switchedOff(false)] }, { positions: ['sales_manager'], permissions: [] }); expect(names).not.toContain('sales_manager'); - // The baseline still applies — deactivating one set is not a blanket denial. expect(names).toContain('member_default'); }); - it('an ACTIVE set still resolves, whole', async () => { - const svc = await locateSecurityService([salesManagerRow(true)]); - const sets = await svc.resolvePermissionSetsForContext?.({ - userId: 'u1', - permissions: ['sales_manager'], - } as any); + it('…and the same request with no ledger row does resolve, whole — the case is real, not vacuous', async () => { + const { svc } = await locate(); + const sets = await svc.resolvePermissionSetsForContext?.({ userId: 'u1', positions: ['sales_manager'], permissions: [] } as any); const found: any = (sets ?? []).find((s) => s.name === 'sales_manager'); - expect(found).toBeDefined(); - expect(found.objects).toEqual({ deal: { allowRead: true, allowEdit: true } }); - expect(found.systemPermissions).toEqual(['setup.access']); + expect(found?.objects).toEqual({ deal: { allowRead: true, allowEdit: true } }); + expect(found?.systemPermissions).toEqual(['setup.access']); }); - it('a row with NO `active` column still resolves — deployed rows are not mass-revoked', async () => { - const names = await namesFor([salesManagerRow(undefined)], { permissions: ['sales_manager'] }); - expect(names).toContain('sales_manager'); + it('the 0/1 storage shape of the ledger is judged too, not only a literal `false`', async () => { + expect(await namesFor({ ledger: [switchedOff(0)] }, { positions: ['sales_manager'] })).not.toContain('sales_manager'); + expect(await namesFor({ ledger: [switchedOff(1)] }, { positions: ['sales_manager'] })).toContain('sales_manager'); }); - it('the 0/1 storage shape is judged too, not only a literal `false`', async () => { - expect(await namesFor([salesManagerRow(0)], { permissions: ['sales_manager'] })).not.toContain( - 'sales_manager', + it("a ledger row of ANOTHER type under the same name switches nothing off (a position's row is not the set's)", async () => { + const names = await namesFor( + { ledger: [{ metadata_type: 'position', name: 'sales_manager', active: false }] }, + { positions: ['sales_manager'] }, ); - expect(await namesFor([salesManagerRow(1)], { permissions: ['sales_manager'] })).toContain( - 'sales_manager', + expect(names).toContain('sales_manager'); + }); + + it("a catalog ROW's `active: false` is not read — only the ledger switches a set off", async () => { + const names = await namesFor( + { rows: [{ name: 'sales_manager', active: false }] }, + { positions: ['sales_manager'] }, ); + expect(names).toContain('sales_manager'); }); +}); - it('THE REACHABILITY CASE: a POSITION name reaching a deactivated set of the same name', async () => { - // Core cannot judge this row: it filtered the `sys_position` catalogue and - // the sets it reached by id, and this name arrives as a POSITION the caller - // legitimately holds. Without this loader's filter the deactivated set - // would grant `deal` edit rights through the name-reuse path — the wall - // that looks enforced and is not. - const names = await namesFor([salesManagerRow(false)], { positions: ['sales_manager'] }); - expect(names).not.toContain('sales_manager'); +describe('[ADR-0049] names that arrive already judged are not judged again', () => { + it('a set the resolver granted (`context.permissions`) issues no ledger read', async () => { + const { svc, ledgerReads } = await locate({ ledger: [switchedOff(false)] }); + const sets = await svc.resolvePermissionSetsForContext?.({ userId: 'u1', permissions: ['sales_manager'] } as any); + // The resolver dropped a switched-off set before building `permissions`; + // a name it hands over is in effect, so no second read is spent on it. + expect((sets ?? []).map((s) => s.name)).toContain('sales_manager'); + expect(ledgerReads()).toBe(0); }); - it('…and the same request with the set ACTIVE does resolve — the case is real, not vacuous', async () => { - const names = await namesFor([salesManagerRow(true)], { positions: ['sales_manager'] }); - expect(names).toContain('sales_manager'); + it('a request with no folded collision issues no ledger read', async () => { + const { svc, ledgerReads } = await locate(); + await svc.resolvePermissionSetsForContext?.({ userId: 'u1', positions: ['everyone', 'org_member'], permissions: [] } as any); + expect(ledgerReads()).toBe(0); }); }); diff --git a/packages/plugins/plugin-security/src/permission-set-resolution-memo.test.ts b/packages/plugins/plugin-security/src/permission-set-resolution-memo.test.ts index 8db79d17317..af6cc1ec5f1 100644 --- a/packages/plugins/plugin-security/src/permission-set-resolution-memo.test.ts +++ b/packages/plugins/plugin-security/src/permission-set-resolution-memo.test.ts @@ -44,7 +44,7 @@ import type { PermissionSet } from '@objectstack/spec/security'; import { SecurityPlugin } from './security-plugin.js'; import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core'; -/** Resolvable from metadata, so only `custom_role` below reaches the db loader. */ +/** Resolvable from metadata, so only `custom_role` below reaches the catalog loader. */ const baselineSet: PermissionSet = { name: 'member_default', label: 'Member', @@ -52,8 +52,11 @@ const baselineSet: PermissionSet = { } as never; /** - * The db-loader harness. `find` is the SOLE observable: every call it records is - * one `sys_permission_set` round trip the deployment would really have made. + * The catalog-loader harness. The engine registry's by-name read of + * `custom_role` is the SOLE observable: every call it records is one catalog + * resolution of that set the deployment would really have made (ADR-0131 + * D3/D4 — the loader reads the security catalog, never a `sys_permission_set` + * row). */ const makeHarness = () => { const fields: Record = {}; @@ -69,11 +72,17 @@ const makeHarness = () => { ? [{ id: 'ps-1', name: 'custom_role', label: 'Custom', object_permissions: '{}' }] : [], ); + const getItem = vi.fn((type: string, name: string) => + type === 'permission' && name === 'custom_role' + ? { name: 'custom_role', label: 'Custom', objects: {} } + : undefined, + ); const ql = { registerMiddleware: (mw: never) => { if (!middleware) middleware = mw; }, getSchema: () => baseSchema, + registry: { getItem, listItems: () => [], isPackageDisabled: () => false }, find, findOne: vi.fn(async (object: string, query?: EngineFindOneQueryInput) => { assertEngineFindOnePredicate(object, query); return null; }), }; @@ -94,21 +103,13 @@ const makeHarness = () => { ctx, find, /** - * How many reads THE DB LOADER has issued so far. - * - * Matched on the loader's own predicate shape (`name: { $in: [...] }`) - * rather than on the object name: several unrelated probes read - * `sys_permission_set` by single name on this path (the ADR-0095 - * auto-org-admin grant, the ADR-0090 audience-binding suggestions), and - * counting those would make the number mean "reads of a table" instead of - * "resolutions of this context" — the quantity under test. + * How many catalog set resolutions THE LOADER has issued so far — the + * registry's by-name reads of type `permission` (the baseline set answers + * from metadata and never reaches it), so a read of another type does not + * count: the number means "resolutions of this context", the quantity + * under test. */ - permissionSetReads: () => - find.mock.calls.filter((c) => { - if (c[0] !== 'sys_permission_set') return false; - const where = (c[1] as { where?: { name?: unknown } } | undefined)?.where; - return Array.isArray((where?.name as { $in?: unknown[] } | undefined)?.$in); - }).length, + permissionSetReads: () => getItem.mock.calls.filter((c) => c[0] === 'permission').length, run: async (opCtx: unknown) => { if (!middleware) throw new Error('middleware never registered'); await middleware(opCtx, async () => {}); diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts index a8681ab8a59..2147861691a 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts @@ -1,8 +1,8 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * A `sys_user_position` write whose `position` names no `sys_position` row is - * refused — `400 VALIDATION_FAILED`, `reference_not_found` at `position` — + * A `sys_user_position` write whose `position` names no position the security + * catalog holds (ADR-0131 D3/D4) is refused — `400 VALIDATION_FAILED`, `reference_not_found` at `position` — * instead of answering 201 over an assignment that resolves to nothing. * * Measured on a REAL `ObjectQL` engine over a real SQL driver with the REAL @@ -312,6 +312,16 @@ describe('a position spelled as the catalog row ID is refused, and the account i }); describe('the accepted half — a catalog NAME, active or deactivated', () => { + it('a position declared only in the registry, with no sys_position row, is accepted', async () => { + const h = await boot(); + h.engine.registry.registerItem('position', { name: 'qa_registry_only', label: 'Registry only' } as any, 'name' as any, 'com.objectstack.qa.position-catalog-refusal'); + expect(await h.engine.find('sys_position', { where: { name: 'qa_registry_only' }, context: SYS })).toHaveLength(0); + const created = await h.engine.insert( + 'sys_user_position', { user_id: 'u_reg', position: 'qa_registry_only' }, { context: ADMIN } as any, + ); + expect(created).toMatchObject({ position: 'qa_registry_only' }); + }); + it('a catalog name is accepted and resolves: the holder reads rows', async () => { const h = await boot(); const created = await h.engine.insert( @@ -491,8 +501,8 @@ describe('a non-string position is judged by its string form', () => { expect([env.code, env.status], text).toEqual(['VALIDATION_FAILED', 400]); expect(env.fields[0], text).toMatchObject({ field: 'position', code: 'reference_not_found', value: text }); } - // A catalog row that really carries such a name is found, so the predicate holds. - await h.engine.insert('sys_position', { id: 'pos_lit', name: '{lit_pos}', label: 'Literal', active: true }, { context: SYS } as any); + // A catalog position that really carries such a name is found, so the predicate holds. + h.engine.registry.registerItem('position', { name: '{lit_pos}', label: 'Literal' } as any, 'name' as any, 'com.objectstack.qa.position-catalog-refusal'); const created = await h.engine.insert( 'sys_user_position', { user_id: 'u_tok_ok', position: '{lit_pos}' }, { context: ADMIN } as any, ); @@ -548,7 +558,11 @@ describe('scope — the stand-downs are declared, not accidental', () => { // A walled, two-organization posture // --------------------------------------------------------------------------- -describe("walled posture, two organizations — the predicate reads the WRITER's catalog", () => { +describe('walled posture, two organizations — the predicate reads the environment catalog, never an organization row', () => { + // [ADR-0131 D3/D4] Each organization holds a `sys_position` row; only + // `qa_a_own` also has a definition in the security catalog. The catalog is + // environment-level, so a row alone — whichever organization holds it — + // is not a position an assignment can take effect under. async function bootTwoOrgs() { const h = await boot({ walled: true }); await h.engine.insert('sys_position', @@ -557,6 +571,7 @@ describe("walled posture, two organizations — the predicate reads the WRITER's await h.engine.insert('sys_position', { id: 'pos_a_own', name: 'qa_a_own', label: 'A own', active: true }, { context: { isSystem: true, tenantId: 'org_a' } } as any); + h.engine.registry.registerItem('position', { name: 'qa_a_own', label: 'A own' } as any, 'name' as any, 'com.objectstack.qa.position-catalog-refusal'); return h; } const ORG_A_ADMIN = { ...ADMIN, tenantId: 'org_a' }; @@ -570,12 +585,11 @@ describe("walled posture, two organizations — the predicate reads the WRITER's expect(env.fields[0]).toMatchObject({ field: 'position', code: 'reference_not_found', value: 'nope_position' }); }); - it("a name only ANOTHER organization's catalog carries is REFUSED, answered exactly like a name no organization carries", async () => { - // REVERSED pin (#20297). The catalog is read the engine lookup probe's way, - // `{ ...context, isSystem: true }`: the writer's organization plus - // organization-less rows. org_b's `qa_b_only` is invisible from org_a, so - // the write is refused — and with the answer a name that exists nowhere - // gets, so the refusal is no oracle for "some other organization has it". + it("a name only an organization's ROW carries is REFUSED, answered exactly like a name nothing carries", async () => { + // [ADR-0131 D3/D4] org_b's `qa_b_only` row has no definition in the + // security catalog, so it grants nothing anywhere and the write is refused + // — with the answer a name that exists nowhere gets, so the refusal is no + // oracle for "some organization has a row of it" (#20297). const h = await bootTwoOrgs(); const foreign = envelopeOf(await refusalOf(() => h.engine.insert( 'sys_user_position', { user_id: 'u_wb', position: 'qa_b_only' }, { context: ORG_A_ADMIN } as any, @@ -610,7 +624,7 @@ describe("walled posture, two organizations — the predicate reads the WRITER's expect((await assignmentsOf(h, 'u_wm'))[0]?.position).toBe('qa_a_own'); }); - it("the writer's own organization's name is accepted (control for the scoped read)", async () => { + it('a name the catalog holds is accepted (control)', async () => { const h = await bootTwoOrgs(); await addMember(h, 'u_wo', 'org_a'); const created = await h.engine.insert( @@ -619,15 +633,13 @@ describe("walled posture, two organizations — the predicate reads the WRITER's expect(created).toMatchObject({ position: 'qa_a_own', organization_id: 'org_a' }); }); - it('the catalog read is scoped by the writer context — a context naming no organization reads every organization', async () => { - // A platform-level writer (no tenant in context) reads every organization, - // as the engine probe does. The full chain cannot carry one here: on the - // isolated posture the security middleware refuses a write with no active - // organization (403) before this refusal runs, so the read is pinned at - // the function, beside the org-bound reading of the same names. + it('the catalog read takes no organization — the same verdict for a writer in an organization and one in none', async () => { + // Pinned at the function: on the isolated posture the security middleware + // refuses a write with no active organization (403) before this refusal + // runs, so the full chain cannot carry a writer naming none. const h = await bootTwoOrgs(); const names = ['qa_b_only', 'qa_a_own', 'nope_position']; - expect(await namesWithoutCatalogRow({ ql: h.engine }, names, ADMIN)).toEqual(['nope_position']); + expect(await namesWithoutCatalogRow({ ql: h.engine }, names, ADMIN)).toEqual(['qa_b_only', 'nope_position']); expect(await namesWithoutCatalogRow({ ql: h.engine }, names, ORG_A_ADMIN)).toEqual(['qa_b_only', 'nope_position']); }); @@ -687,7 +699,7 @@ describe("walled posture, two organizations — the predicate reads the WRITER's expect(env.fields[0]).toMatchObject({ field: 'position', code: 'reference_not_found' }); expect(env.fields[0].message).toBe(positionNotInCatalogMessage('pos_b_only')); expect(env.fields[0].message).not.toContain('qa_b_only'); - // … while its own organization's row is named. + // … while its own organization's row, whose name the catalog holds, is named. const own = envelopeOf(await refusalOf(() => h.engine.insert( 'sys_user_position', { user_id: 'u_wc', position: 'pos_a_own' }, { context: ORG_A_ADMIN } as any, ))); diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.ts index 92f5addcf1c..1ffa2986d5f 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.ts @@ -1,64 +1,57 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * `sys_user_position.position` must name a `sys_position` catalog row — the - * write-path refusal for an assignment that names nothing. + * `sys_user_position.position` must name a position the security catalog + * holds — the write-path refusal for an assignment that names nothing. * * ## The hole * - * `position` is declared `Field.text` and references `sys_position.name` by - * convention only, while every sibling reference column on the same row - * (`user_id`, `organization_id`, `business_unit_id`, `granted_by`, - * `delegated_from`) is a `Field.lookup` the engine already refuses when it - * names no row (`reference_not_found`, `assertReferencesResolve` in - * `@objectstack/objectql`). So a value naming no catalog row was stored and - * answered `201`, and then resolved to nothing: the runtime resolver joins - * `sys_position` BY NAME, so the assignment granted no permission set, reached - * no sharing rule and put nothing in `current_user.positions` that any reader - * could act on. The holder signed in to an app that reads nothing, and no layer - * said why. The measured spelling is the position's record ID written where its - * NAME belongs — the neighbouring `user_id` is an id, and the analogous + * `position` is declared `Field.text` and references a position by NAME, while + * every sibling reference column on the same row (`user_id`, `organization_id`, + * `business_unit_id`, `granted_by`, `delegated_from`) is a `Field.lookup` the + * engine already refuses when it names no row (`reference_not_found`, + * `assertReferencesResolve` in `@objectstack/objectql`). So a value naming no + * position was stored and answered `201`, and then resolved to nothing: the + * assignment granted no permission set, reached no sharing rule and put + * nothing in `current_user.positions` that any reader could act on. The holder + * signed in to an app that reads nothing, and no layer said why. The measured + * spelling is the position's record ID written where its NAME belongs — the + * neighbouring `user_id` is an id, and the analogous * `sys_user_permission_set.permission_set_id` is genuinely id-typed, so the id * is what an author reaches for by analogy. * - * ## The predicate — exactly "no catalog row carries this name" + * ## The predicate — exactly "the name resolves to no position" * * Ruled on the card that filed the hole (#16712, maintainer-confirmed), * verbatim: «⛔ The predicate is exactly "no catalog row carries this name" — * never "this assignment cannot take effect": ADR-0049 keeps a deactivated - * position's assignment while it stops granting». So a DEACTIVATED position - * (`active: false`) is still a catalog row, and an assignment naming it is - * accepted — it stops granting, it is not refused. + * position's assignment while it stops granting». So a DEACTIVATED position is + * still a position, and an assignment naming it is accepted — it stops + * granting, it is not refused. (The switch is the activation ledger's, and + * this refusal does not read it.) * - * ## Whose catalog — the writer's organization plus organization-less rows + * ## Whose catalog — the security catalog, one per deployment * - * The ruling fixes the predicate; #20297 fixes the catalog it reads, by the - * platform's standing tenancy rule rather than by a new one. Every catalog read - * here runs under {@link catalogReadContext} — `{ ...context, isSystem: true }`, - * the `sudo()`-shaped elevation of the engine's own lookup probe - * (`assertReferencesResolve` in `@objectstack/objectql`, #19808), never a bare - * `{ isSystem: true }`: + * [ADR-0131 D3/D4] The catalog is the environment registry: positions + * declared by a package, the platform's built-ins, and every position an + * environment author saved through the metadata door, read BY NAME through + * the reader the security plugin binds to its engine + * (`securityCatalogReaderOf`). It is the one source the authorization + * resolver grants a held position through, so this refusal accepts exactly + * the names an assignment can take effect under, and refuses exactly the + * names that resolve nowhere: * - * - the elevation is about VISIBILITY: existence is a fact about the database, - * not about the writer's row-level reach, so RBAC, RLS and FLS are bypassed; - * - it is never about TENANCY: the writer's context is spread first, so the - * engine forwards its `tenantId` to the driver, which reads the writer's - * organization plus organization-less rows (`organization_id IS NULL`). + * - a position declared only in the registry (no `sys_position` row) is + * ACCEPTED — before the cutover the per-organization row table was the + * catalog read here, and such an assignment was refused `400`; + * - a name only a `sys_position` row carries (an organization's Setup row + * under a wall with no definition, a row whose definition was never saved) + * is REFUSED: the resolver grants nothing through it (#15196 Q3 = A). * - * So a name only ANOTHER organization's catalog carries is refused exactly like - * a name no organization carries — the same envelope and the same message — - * and the accept-or-refuse answer tells an organization admin nothing about any - * other organization's catalog. The bare spelling read every organization, - * which accepted such a name (it resolves nothing in the writer's organization) - * and answered "some other organization has this position" apart from "no one - * has it": the cross-tenant existence oracle the engine probe (#19808) and the - * delegated-admin gate's position-name reads (#19819, #19860) were closed - * against. Under a `single` posture the declared catalog is seeded with no - * organization, so when the deployment holds one organization every writer - * reads the whole catalog; a `single` deployment holding several (reported at - * `error` at boot, #17010) gives each writer its active organization's - * positions plus the organization-less ones. A writer whose context names no - * organization reads every organization's rows, as the engine probe does. + * The catalog is environment-level and takes no organization (ADR-0131 D3), + * so every writer reads the same catalog and the verdict says nothing about + * any organization's own data — there is no per-organization catalog left to + * leak the existence of. * * ## Which writes it judges * @@ -89,10 +82,11 @@ * write stores it with `201` (measured over SQLite: `123`, `true`, `{}`, * `{ a: 1 }` and `['x']` all stored). So those are judged by their string * form — a scalar by `String(value)`, an object or array by its JSON text — - * and refused like any name no catalog row carries. The stored text is the - * driver's, not that string form (SQLite stores `123` as `'123.0'`), so a - * non-string whose string form happens to equal a catalog name is accepted - * and resolves nothing: the engine's `text` leniency, outside this refusal; + * and refused like any name that resolves to no position. The stored text is + * the driver's, not that string form (SQLite stores `123` as `'123.0'`), so + * a non-string whose string form happens to equal a position name is + * accepted and resolves nothing: the engine's `text` leniency, outside this + * refusal; * - an update the engine refuses on its own dispatch predicate. * * ## Where it runs @@ -101,8 +95,7 @@ * INSIDE it: the delegated-admin gate and the object CRUD check have both * passed before the catalog is consulted. A caller who may not write this * table is refused `403` on authority, identically whatever the value names, - * and never sees the catalog verdict. The tenant boundary is the catalog - * read's own (above), not this placement's. + * and never sees the catalog verdict. * * The placement is also what lets the by-id PRE-IMAGE read ({@link SYSTEM_CTX}) * stay bare: that read happens only after the security middleware has admitted @@ -120,22 +113,22 @@ * code is the registered ADR-0112 `VALIDATION_FAILED` (built by * `validationFailure`, the shared constructor both HTTP doors map to 400), and * the field-level code is a member of the closed ADR-0114 catalog. The message - * names the value, says the column takes the catalog NAME, and names the fix — - * the name itself when the value is the record id of a position the writer's - * catalog holds (read the same way as the check, so never another - * organization's). + * names the value, says the column takes the position's NAME, and names the + * fix — the name itself when the value is the record id of a `sys_position` + * row the writer's organization holds and the catalog resolves that row's + * name (the id is a row fact, read under {@link catalogReadContext}, so never + * another organization's). * * ## Fails open * - * A catalog that cannot be read (the object is not registered in this - * composition, or the read throws) refuses nothing — an integrity check that - * cannot run must not invent a rejection, the stance the engine's own lookup - * probe takes. A failed read is reported at `warn`: the write proceeds, and the - * operator is told the check did not run. A name the query layer would read as - * a filter placeholder (`{…}`) is not a failed read: it is compared literally - * (`catalogCarries`), so its verdict never falls open. + * A catalog that cannot be read (no reader is bound to this engine, or the + * read throws) refuses nothing — an integrity check that cannot run must not + * invent a rejection, the stance the engine's own lookup probe takes. A failed + * read is reported at `warn`: the write proceeds, and the operator is told the + * check did not run. */ +import { securityCatalogReaderOf } from '@objectstack/core'; import { resolveEngineUpdateDispatch, type EngineUpdateDispatchData } from '@objectstack/metadata-core'; import type { FieldErrorCode } from '@objectstack/spec/api'; import { @@ -149,8 +142,10 @@ import { SysUserPosition } from './objects/sys-user-position.object.js'; /** The assignment object this refusal is registered on. */ export const POSITION_ASSIGNMENT_OBJECT = 'sys_user_position'; -/** The catalog a `position` value must name a row of. */ +/** The row table the id-spelling hint reads a record id in — never the verdict. */ export const POSITION_CATALOG_OBJECT = 'sys_position'; +/** The security catalog type a `position` value must name an item of. */ +const POSITION_CATALOG_TYPE = 'position'; /** The column judged, on {@link POSITION_ASSIGNMENT_OBJECT}. */ export const POSITION_FIELD = 'position'; @@ -158,18 +153,18 @@ export const POSITION_FIELD = 'position'; * The by-id PRE-IMAGE read only (is the stored `position` being changed?) — * never a catalog read. Bare, because the security middleware has already * refused, identically, every row id the writer cannot update (see "Where it - * runs"); every catalog read goes through {@link catalogReadContext}. + * runs"); the id-spelling hint's row read goes through {@link catalogReadContext}. */ const SYSTEM_CTX = { isSystem: true } as const; /** - * The context every `sys_position` read runs under: the writer's own context - * with `isSystem` set — the engine lookup probe's `sudo()`-shaped spelling. The - * spread carries the writer's `tenantId`, so the read sees the writer's - * organization plus organization-less rows; a context naming no organization - * reads every organization. ⛔ Never a bare `{ isSystem: true }` here: that - * spans every organization and makes the refusal a cross-tenant existence - * oracle (module note, "Whose catalog"). + * The context the id-spelling hint's `sys_position` read runs under: the + * writer's own context with `isSystem` set — the engine lookup probe's + * `sudo()`-shaped spelling. The spread carries the writer's `tenantId`, so the + * read sees the writer's organization plus organization-less rows; a context + * naming no organization reads every organization. ⛔ Never a bare + * `{ isSystem: true }` here: that spans every organization, and a hint naming + * another organization's row would be a cross-tenant existence oracle. */ function catalogReadContext(context: unknown): Record { const own = context && typeof context === 'object' ? (context as Record) : {}; @@ -342,41 +337,34 @@ export async function writtenPositionNames(ql: any, opCtx: any): Promise { const { ql, logger } = deps; if (names.length === 0) return []; - if (!ql || typeof ql.find !== 'function') return null; - // An unregistered catalog is a composition without one — nothing to judge - // against, the same silent stand-down the engine's own lookup probe takes. - if (typeof ql.getSchema === 'function' && !ql.getSchema(POSITION_CATALOG_OBJECT)) return null; + const catalog = securityCatalogReaderOf(ql); + // No catalog bound — nothing to judge against, the same silent stand-down + // the engine's own lookup probe takes for an unregistered target. + if (!catalog) return null; - const readCtx = catalogReadContext(context); const missing: string[] = []; for (const name of names) { let carried: boolean; try { - carried = await catalogCarries(ql, name, readCtx); + carried = (await catalog.resolve(POSITION_CATALOG_TYPE, name)) !== undefined; } catch (e) { logger?.warn?.( - `[security] the ${POSITION_CATALOG_OBJECT} catalog could not be read, so a ` + - `${POSITION_ASSIGNMENT_OBJECT} write was NOT checked for a position name that resolves ` + - `to no catalog row — it proceeds unchecked`, + `[security] the security catalog could not be read, so a ${POSITION_ASSIGNMENT_OBJECT} write was NOT ` + + 'checked for a position name that resolves to no position — it proceeds unchecked', { object: POSITION_ASSIGNMENT_OBJECT, error: (e as Error)?.message ?? String(e) }, ); return null; @@ -387,34 +375,12 @@ export async function namesWithoutCatalogRow( } /** - * Does the catalog, read under `readCtx`, hold a row whose `name` is exactly - * `name`? A name the query layer would read as a filter PLACEHOLDER (a - * fully-wrapped `{…}`, recognised by the spec's `classifyFilterToken`, the - * predicate `@objectstack/core`'s `resolveFilterTokens` applies to every `where` - * comparand) cannot go in `where` as itself: an unknown token throws - * `FILTER_TOKEN_UNKNOWN` and a known one is replaced by its value, so the read - * would fail open or judge a different name. Such a name is compared here - * instead, against the catalog names that share its first character. - */ -async function catalogCarries(ql: any, name: string, readCtx: Record): Promise { - if (classifyFilterToken(name) === null) { - const rows = await ql.find(POSITION_CATALOG_OBJECT, { where: { name }, limit: 1, context: readCtx }); - return Array.isArray(rows) && rows.length > 0; - } - const rows = await ql.find(POSITION_CATALOG_OBJECT, { - where: { name: { $startsWith: name.charAt(0) } }, - fields: ['name'], - context: readCtx, - }); - return Array.isArray(rows) && rows.some((row: any) => row?.name === name); -} - -/** - * For each missing value that is the record ID of a position the writer's - * catalog holds, that position's NAME — the exact fix. Read under the same - * {@link catalogReadContext} as the check itself, so the hint can name only a - * position the check could have accepted, never another organization's. A read - * that fails yields no hint. + * For each missing value that is the record ID of a `sys_position` row the + * writer's organization holds, that row's NAME — the exact fix, offered only + * when the security catalog resolves that name, so the hint never names a + * position the check would refuse. The id is a row fact, read under + * {@link catalogReadContext} (never another organization's row). A read that + * fails yields no hint. */ export async function idSpellingHints( deps: PositionCatalogRefusalDeps, @@ -424,16 +390,18 @@ export async function idSpellingHints( const hints = new Map(); const { ql } = deps; if (!ql || typeof ql.find !== 'function') return hints; + const catalog = securityCatalogReaderOf(ql); + if (!catalog) return hints; const readCtx = catalogReadContext(context); for (const value of values) { // A placeholder-shaped value is never a record id, and in `where` it would - // be resolved as a filter token rather than compared (see catalogCarries). + // be resolved as a filter token rather than compared. if (classifyFilterToken(value) !== null) continue; try { const rows = await ql.find(POSITION_CATALOG_OBJECT, { where: { id: value }, limit: 1, context: readCtx }); const row = Array.isArray(rows) ? rows[0] : null; const name = row && typeof row.name === 'string' && row.name !== '' ? row.name : null; - if (!name) continue; + if (!name || !(await catalog.resolve(POSITION_CATALOG_TYPE, name))) continue; hints.set(value, name); } catch { // No hint — the refusal still stands and still names the column's contract. @@ -447,14 +415,15 @@ export function positionNotInCatalogMessage(value: string, idOfPosition?: string if (idOfPosition) { return ( `${POSITION_LABEL}: no position is named '${value}' — that is the record id of the position ` + - `'${idOfPosition}'. ${POSITION_ASSIGNMENT_OBJECT}.${POSITION_FIELD} takes the position's machine name ` + - `(${POSITION_CATALOG_OBJECT}.name), never its id: write '${idOfPosition}'.` + `'${idOfPosition}'. ${POSITION_ASSIGNMENT_OBJECT}.${POSITION_FIELD} takes the position's machine name, ` + + `never its id: write '${idOfPosition}'.` ); } return ( `${POSITION_LABEL}: no position is named '${value}'. ${POSITION_ASSIGNMENT_OBJECT}.${POSITION_FIELD} ` + - `takes the machine name of a position in the catalog (${POSITION_CATALOG_OBJECT}.name), never its record ` + - `id: write the name of an existing position, or create the position first.` + `takes the machine name of a position in the security catalog (a package's position, a built-in, or one ` + + `saved through the metadata door), never its record id: write the name of an existing position, or ` + + `declare the position first.` ); } diff --git a/packages/plugins/plugin-security/src/position-write-through.test.ts b/packages/plugins/plugin-security/src/position-write-through.test.ts index 0cf67903d2e..6de62903f68 100644 --- a/packages/plugins/plugin-security/src/position-write-through.test.ts +++ b/packages/plugins/plugin-security/src/position-write-through.test.ts @@ -43,6 +43,7 @@ import { SysMetadataAuditObject, } from '@objectstack/metadata-core'; import { createSecurityCatalogReader, resetPlatformAdminEmailMemo } from '@objectstack/core'; +import { mapDataError } from '@objectstack/types'; import type { PermissionSet } from '@objectstack/spec/security'; import { DATA_MIGRATION_FLAG_OBJECT } from '@objectstack/spec/system'; import { SysUser, SysAccount, SysMember, SysOrganization } from '@objectstack/platform-objects/identity'; @@ -622,6 +623,80 @@ describe('[C2 stage S10, ADR-0048 addendum N.2/N.3] one namespace — a create o }); }); +describe('[ruling Q (a)] one holder per position name per deployment — a create or a rename into a name the environment catalog holds is refused', () => { + /** + * The wire envelope the REST door answers (`mapDataError`) — the one a + * second row of a name in ONE organization already answers, pinned as the + * control below. + */ + const DUPLICATE = { code: 'UNIQUE_VIOLATION', status: 409 }; + const httpEnvelope = (e: unknown) => { + const r = mapDataError(e, 'sys_position'); + return { code: (r.body as { code?: unknown }).code, status: r.status }; + }; + const OTHER_ORG = 'org_pw_other'; + /** An administrator of a SECOND organization of the same `single` deployment. */ + const otherAdmin = (b: Booted) => ({ ...b.admin, tenantId: OTHER_ORG, accessible_org_ids: [OTHER_ORG] }); + const bootTwo = async () => { + const b = await boot(); + await b.engine.insert('sys_organization', { id: OTHER_ORG, name: 'PW Other', slug: 'pw-other' }, { context: SYS } as any); + return b; + }; + + it("a second organization's create of a name the first organization's position holds is refused 409; the first definition is untouched", async () => { + const b = await bootTwo(); + await create(b, { name: 'shift_lead', label: 'Shift lead', description: 'First holder' }); + const refusal = await refusalOf(b.engine.insert('sys_position', { name: 'shift_lead', label: 'Theirs' }, { context: otherAdmin(b) } as any)); + expect(httpEnvelope(refusal)).toEqual(DUPLICATE); + // One dialect: a second holder in the SAME organization answers the same. + expect(httpEnvelope(await refusalOf(create(b, { name: 'shift_lead', label: 'Again' })))).toEqual(DUPLICATE); + expect((await b.rows('shift_lead')).map((r) => r.label)).toEqual(['Shift lead']); + expect((await b.envRows('shift_lead')).map((r) => r.body)).toEqual([ + { name: 'shift_lead', label: 'Shift lead', description: 'First holder', delegatable: false }, + ]); + }); + + it("a create of a name saved only through the metadata door is refused 409; the definition keeps its permissionSets", async () => { + const b = await boot(); + await b.protocol.saveMetaItem({ + type: 'position', name: 'door_lead', item: { name: 'door_lead', label: 'Door lead', permissionSets: ['viewer_readonly'] }, + }); + const refusal = await refusalOf(create(b, { name: 'door_lead', label: 'Setup copy' })); + expect(httpEnvelope(refusal)).toEqual(DUPLICATE); + expect(await b.rows('door_lead')).toEqual([]); + expect((await b.envRows('door_lead')).map((r) => r.body)).toEqual([ + { name: 'door_lead', label: 'Door lead', permissionSets: ['viewer_readonly'] }, + ]); + }); + + it('a rename into a name the environment catalog holds is refused 409; the row keeps its old name', async () => { + const b = await bootTwo(); + await create(b, { name: 'dock_chief', label: 'Dock chief' }); + const mine: any = await b.engine.insert('sys_position', { name: 'yard_chief', label: 'Yard chief' }, { context: otherAdmin(b) } as any); + const refusal = await refusalOf(b.engine.update('sys_position', { id: mine.id, name: 'dock_chief' }, { context: otherAdmin(b) } as any)); + expect(httpEnvelope(refusal)).toEqual(DUPLICATE); + expect((await b.rows('yard_chief')).map((r) => r.id)).toEqual([mine.id]); + expect((await b.envRows('dock_chief')).map((r) => r.body.label)).toEqual(['Dock chief']); + }); + + it('control: a name no definition holds is created, and an edit that keeps a held name is no second holder', async () => { + const b = await boot(); + const created: any = await create(b, { name: 'free_lead', label: 'Free lead' }); + await patch(b, created.id, { label: 'Free lead (relabelled)' }); + expect((await b.envRows('free_lead')).map((r) => r.body.label)).toEqual(['Free lead (relabelled)']); + }); + + it('a catalog that cannot be read refuses the create — accepting would merge blind', async () => { + const b = await boot(); + const unreadable = Object.assign(new Error('catalog unreadable'), { code: 'AUTHZ_STORE_UNAVAILABLE', status: 503 }); + vi.spyOn((b.engine as any).registry, 'getItem').mockImplementation(() => { throw unreadable; }); + const refusal: any = await refusalOf(create(b, { name: 'blind_lead', label: 'Blind lead' })); + expect(refusal).not.toBeNull(); + vi.restoreAllMocks(); + expect(await b.rows('blind_lead')).toEqual([]); + }); +}); + describe('[ADR-0131 D3] Q3 = A — a package cannot register a name a Setup position holds in the environment ledger', () => { it('the registry item seam refuses it 422 NAMESPACE_CONFLICT, naming both holders', async () => { const b = await boot(); diff --git a/packages/plugins/plugin-security/src/position-write-through.ts b/packages/plugins/plugin-security/src/position-write-through.ts index ac350354b74..f97c47663d6 100644 --- a/packages/plugins/plugin-security/src/position-write-through.ts +++ b/packages/plugins/plugin-security/src/position-write-through.ts @@ -67,6 +67,27 @@ * `packagedBaseRefusal`) keeps the stand-down below, as the `/automation` * doors keep theirs. * + * ## One namespace — a name the environment catalog already holds is not taken either (ruling Q (a)) + * + * Under `single` a position name is unique per DEPLOYMENT (the cutover's + * ruling Q (a): "the write door refuses a second holder"), while the row's own + * index is unique per organization only. So a Setup **create**, or a **rename + * into** a name, that the security catalog already resolves to a definition + * no package or built-in holds (S10 above answers those) — an environment + * definition another organization's row wrote, one an author saved through + * the metadata door, or one an application stack declares in memory — is + * refused, and the row write is undone. Before this, the definition save that + * follows overwrote the existing definition with this row's label and + * description and dropped its `permissionSets`: two holders silently merged + * into one. The refusal is `409 UNIQUE_VIOLATION` (`PositionNameConflictError`) + * — the answer a second row of the same name in one organization already gets + * over HTTP — so an author meets one dialect for "this name is taken". The read is the + * catalog the resolver grants through (`securityCatalogReaderOf`), asked + * before any definition is saved; a catalog that cannot be read refuses (the + * row goes and that failure is the answer), because accepting would merge + * blind. The upgrade ceremony reports a pair that already exists as + * `conflicting` and never merges it (`convertPositionBindingRows`). + * * ## Where it stands down — the write proceeds exactly as before * * - **a system write** (`isSystem`): the seeders and the package door write @@ -100,9 +121,11 @@ * because the row is still the row the data door wrote. */ +import { securityCatalogReaderOf } from '@objectstack/core'; import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/security'; import { METADATA_ITEM_NAME_PATTERN } from '@objectstack/spec/shared'; import { PositionSchema } from '@objectstack/spec/identity'; +import { PositionNameConflictError } from './errors.js'; /** The catalog object this module is registered on. */ export const POSITION_OBJECT = 'sys_position'; @@ -273,6 +296,24 @@ async function nameStillCarried(ql: any, name: string): Promise { return Array.isArray(rows) && rows.length > 0; } +/** + * [ruling Q (a)] The refusal of a create, or a rename into `name`, when the + * security catalog already holds a definition of that name (module note, "a + * name the environment catalog already holds"). `null` when it holds none, + * when the name is a package's or a built-in's (S10's + * {@link heldPositionNameRefusal} answers those, first), or when no catalog is + * bound to this engine. A catalog read that fails throws its own loud error, + * and the caller undoes the write. + */ +async function environmentHeldPositionNameRefusal(ql: any, name: unknown): Promise { + if (typeof name !== 'string' || name === '' || packageHoldsPosition(ql, name)) return null; + const catalog = securityCatalogReaderOf(ql); + if (!catalog) return null; + const held = await catalog.resolve(POSITION_METADATA_TYPE, name); + if (!held) return null; + return new PositionNameConflictError(name, held.packageId); +} + const errorText = (e: unknown): string => (e instanceof Error ? e.message : String(e)); function logError(logger: PositionWriteThroughLogger | undefined, message: string, cause: unknown, meta: Record): void { @@ -409,6 +450,12 @@ export function createPositionWriteThrough( const refusal = heldPositionNameRefusal(door, row.name); if (refusal) throw refusal; } + // [ruling Q (a)] …nor a name the environment catalog already holds: + // one holder per name per deployment, never a silent merge. + for (const row of createdRows) { + const refusal = await environmentHeldPositionNameRefusal(ql, row.name); + if (refusal) throw refusal; + } } catch (e) { await undoInsert(written); throw e; @@ -458,6 +505,12 @@ export function createPositionWriteThrough( const refusal = pre.name !== post.name ? heldPositionNameRefusal(door, post.name) : null; if (refusal) throw refusal; } + // [ruling Q (a)] A rename into a name the environment catalog holds is a + // second holder too. + for (const { pre, post } of posts) { + const refusal = pre.name !== post.name ? await environmentHeldPositionNameRefusal(ql, post.name) : null; + if (refusal) throw refusal; + } } catch (e) { await undoUpdate(targets, patch); throw e; diff --git a/packages/plugins/plugin-security/src/resolve-permission-sets-for-context.pin.test.ts b/packages/plugins/plugin-security/src/resolve-permission-sets-for-context.pin.test.ts index d843b19b132..4b073e59a41 100644 --- a/packages/plugins/plugin-security/src/resolve-permission-sets-for-context.pin.test.ts +++ b/packages/plugins/plugin-security/src/resolve-permission-sets-for-context.pin.test.ts @@ -22,9 +22,9 @@ * `systemPermissions`, `tabPermissions` — because a consumer that must MERGE * the caller's grants cannot reach any of those four from * `resolvePermissionSetNames`. Each is asserted through the located handle, on - * BOTH authoring paths a set can arrive by (declared in metadata, and authored - * in `sys_permission_set` through the DB loader), because the loader is where - * a column silently goes missing. + * BOTH paths a set can arrive by (the metadata list, and the security + * catalog's definition through the plugin's catalog loader — ADR-0131 D3/D4), + * because a loader is where a column silently goes missing. */ import { describe, it, expect, vi } from 'vitest'; @@ -44,20 +44,24 @@ const MEMBER_DEFAULT: PermissionSet = { } as any; /** - * A DB-authored set, as it sits in `sys_permission_set` — snake_case columns, - * JSON-encoded payloads. This is the row shape the plugin's `dbLoader` parses, - * and the shape `/me/apps` reads `tab_permissions` off today in its own copy. + * An environment-authored set, as the security catalog holds it — the engine + * registry's definition (hydrated from `sys_metadata`), which the plugin's + * catalog loader hands back whole. */ -const SALES_MANAGER_ROW = { +const SALES_MANAGER: PermissionSet = { name: 'sales_manager', label: 'Sales Manager', - object_permissions: JSON.stringify({ deal: { allowRead: true, allowEdit: true } }), - field_permissions: JSON.stringify({ 'deal.amount': { readable: true, editable: true } }), - system_permissions: JSON.stringify(['setup.access']), - tab_permissions: JSON.stringify({ app_crm: 'visible' }), -}; + objects: { deal: { allowRead: true, allowEdit: true } }, + fields: { 'deal.amount': { readable: true, editable: true } }, + systemPermissions: ['setup.access'], + tabPermissions: { app_crm: 'visible' }, +} as any; -function bootPlugin(dbRows: Array> = []) { +/** The `sys_permission_set` reads for a set the resolution asked about — there must be none. */ +let permissionSetRowReads = 0; + +function bootPlugin(catalog: PermissionSet[] = [], dbRows: Array> = []) { + permissionSetRowReads = 0; const schema: any = { name: 'deal', label: 'Deal', systemFields: false, fields: { id: { name: 'id' }, amount: { name: 'amount' } } }; const ql: any = { registerMiddleware: () => {}, @@ -65,9 +69,19 @@ function bootPlugin(dbRows: Array> = []) { findOne: async (object: string, query?: EngineFindOneQueryInput) => { assertEngineFindOnePredicate(object, query); return null; }, find: async (object: string, query: any) => { if (object !== 'sys_permission_set') return []; + // Only a read asking for a set the resolution asked about counts: the + // boot's own background passes (the org-admin backfill, the baseline + // binder) read their own names and race the request. + if (/legacy_auditor|sales_manager/.test(JSON.stringify(query?.where ?? {}))) permissionSetRowReads += 1; const wanted: string[] = query?.where?.name?.$in ?? []; return dbRows.filter((r) => wanted.includes(String(r.name))); }, + // The engine registry the security catalog reads first (ADR-0131 D3). + registry: { + getItem: (type: string, name: string) => (type === 'permission' ? catalog.find((d) => d.name === name) : undefined), + listItems: (type: string) => (type === 'permission' ? catalog : []), + isPackageDisabled: () => false, + }, }; const metadata: any = { get: async (_type: string, name: string) => (name === 'deal' ? schema : null), @@ -92,10 +106,15 @@ function bootPlugin(dbRows: Array> = []) { * what makes the feature detection below the same expression the endpoints * will write. */ -async function locateSecurityService(dbRows: Array> = []): Promise> { - const { plugin, ctx } = bootPlugin(dbRows); +async function locateSecurityService( + catalog: PermissionSet[] = [], + dbRows: Array> = [], +): Promise> { + const { plugin, ctx } = bootPlugin(catalog, dbRows); await plugin.init(ctx); await plugin.start(ctx); + // Boot-time probes are not the resolution under test. + permissionSetRowReads = 0; const registered = ctx.registerService.mock.calls.find((c: any[]) => c[0] === 'security')?.[1]; return registered as Partial; } @@ -117,7 +136,7 @@ describe('[#7616] resolvePermissionSetsForContext is reachable through the servi }); it('is CALLABLE through that handle and returns the sets whole', async () => { - const svc = await locateSecurityService([SALES_MANAGER_ROW]); + const svc = await locateSecurityService([SALES_MANAGER]); const sets = await svc.resolvePermissionSetsForContext?.({ userId: 'u1', @@ -129,16 +148,15 @@ describe('[#7616] resolvePermissionSetsForContext is reachable through the servi const byName = new Map((sets ?? []).map((s) => [s.name, s])); expect([...byName.keys()].sort()).toEqual(['member_default', 'sales_manager']); - // All four columns the names surface cannot reach, on the DB-authored set — - // the path where a column goes missing, since the loader projects the row - // by hand. + // All four members the names surface cannot reach, on the catalog-loaded + // set — the path where a member could go missing. const dbAuthored: any = byName.get('sales_manager'); expect(dbAuthored.objects).toEqual({ deal: { allowRead: true, allowEdit: true } }); expect(dbAuthored.fields).toEqual({ 'deal.amount': { readable: true, editable: true } }); expect(dbAuthored.systemPermissions).toEqual(['setup.access']); - // The column `/me/apps` filters its app list with. Dropped by this loader - // until #7616 — which would have made the published contract false for - // every DB-authored set the moment a consumer trusted it. + // The member `/me/apps` filters its app list with (#7616): dropped, it + // would make the published contract false for every environment-authored + // set the moment a consumer trusted it. expect(dbAuthored.tabPermissions).toEqual({ app_crm: 'visible' }); // …and on the metadata-declared set, which arrives by the other path. @@ -149,7 +167,7 @@ describe('[#7616] resolvePermissionSetsForContext is reachable through the servi }); it('is the SAME resolution the names surface reports — baseline additive, no cliff', async () => { - const svc = await locateSecurityService([SALES_MANAGER_ROW]); + const svc = await locateSecurityService([SALES_MANAGER]); const context = { userId: 'u1', permissions: ['sales_manager'] } as any; const names = await svc.resolvePermissionSetNames?.(context); @@ -189,84 +207,48 @@ describe('[#7616] resolvePermissionSetsForContext is reachable through the servi }); /** - * [#11121 residue] An ORGANIZATION-LESS `sys_permission_set` row still grants. + * [ADR-0131 D3/D4] A set is the security catalog's definition; no + * `sys_permission_set` row is read. * - * #11121 made this loader tenant-scoped so two organizations holding a row for - * the same name stop answering each other's requests. Its comment promised the - * other half — "an organization-less leftover only where it does not [have its - * own]" — and the code read `.own` alone, which by - * `resolveOwnOrganizationRow`'s documented contract is NEVER a residue once an - * organization is supplied. That helper is written for SEEDERS, where refusing - * to see a residue as "already seeded" is the entire point; enforcement wants - * the opposite reading. - * - * The consequence was a silent revocation on upgrade: every walled deployment - * carrying pre-#11121 rows (or any row authored without a tenant — the admin - * UI, a hand-run seed) kept its `system_permissions` and `tab_permissions`, - * because that read is unscoped and by id, while its `object_permissions` and - * `admin_scope` stopped applying. One row, two enforcement planes, opposite - * verdicts, no signal at the moment of loss. + * The plugin's third source used to be a per-organization row loader (the + * caller's own row, else an organization-less one). The authorization + * resolver already took a set's body from the catalog and granted nothing + * through a row-only set (#15196 Q3 = A); a row fallback here applied that + * set's object map anyway — two sources with opposite verdicts on one set. + * The loader now reads the catalog, which is environment-level: the same + * definition for a caller in any organization or none. */ -const RESIDUE_ROW = { +const ROW_ONLY = { name: 'legacy_auditor', label: 'Legacy Auditor', - organization_id: null, - object_permissions: JSON.stringify({ deal: { allowRead: true, allowDelete: true } }), - field_permissions: JSON.stringify({}), - system_permissions: JSON.stringify([]), - tab_permissions: JSON.stringify({}), -}; - -/** The SAME name, stamped with the caller's organization — this one must win. */ -const OWN_ROW = { - name: 'legacy_auditor', - label: 'Legacy Auditor (this organization)', organization_id: 'org_a', - object_permissions: JSON.stringify({ deal: { allowRead: true, allowDelete: false } }), - field_permissions: JSON.stringify({}), - system_permissions: JSON.stringify([]), - tab_permissions: JSON.stringify({}), + object_permissions: JSON.stringify({ deal: { allowRead: true, allowDelete: true } }), + active: true, }; -describe('[#11121] the per-organization loader does not silently revoke organization-less grants', () => { - it('resolves an organization-LESS row for a caller who has an organization', async () => { - const svc = await locateSecurityService([RESIDUE_ROW]); - - const sets = await svc.resolvePermissionSetsForContext?.({ - userId: 'u1', - organizationId: 'org_a', - permissions: ['legacy_auditor'], - } as any); - - const found = (sets ?? []).find((s) => s.name === 'legacy_auditor'); - expect(found, 'the grant vanished for a caller in an organization — this is the silent revocation').toBeTruthy(); - // Whole, not merely present: the columns that went missing are the point. - expect((found as any)?.objects?.deal?.allowDelete).toBe(true); - }); - - it('still prefers THIS organization\'s own row when both exist — the cross-tenant bleed stays closed', async () => { - const svc = await locateSecurityService([RESIDUE_ROW, OWN_ROW]); - - const sets = await svc.resolvePermissionSetsForContext?.({ - userId: 'u1', - organizationId: 'org_a', - permissions: ['legacy_auditor'], - } as any); - - const found = (sets ?? []).find((s) => s.name === 'legacy_auditor'); - expect(found?.label).toBe('Legacy Auditor (this organization)'); - // The residue's broader grant must NOT leak in behind the own row. - expect((found as any)?.objects?.deal?.allowDelete).toBe(false); - }); - - it('resolves the same row for a caller with NO organization (single posture, unchanged)', async () => { - const svc = await locateSecurityService([RESIDUE_ROW]); - - const sets = await svc.resolvePermissionSetsForContext?.({ - userId: 'u1', - permissions: ['legacy_auditor'], - } as any); - - expect((sets ?? []).some((s) => s.name === 'legacy_auditor')).toBe(true); - }); +describe('[ADR-0131 D3/D4] the catalog loader reads definitions, never sys_permission_set rows', () => { + for (const [label, organizationId] of [['in an organization', 'org_a'], ['with no organization', undefined]] as const) { + it(`a set only a row carries resolves nothing for a caller ${label}, and no row is read`, async () => { + const svc = await locateSecurityService([], [ROW_ONLY, { ...ROW_ONLY, organization_id: null }]); + const sets = await svc.resolvePermissionSetsForContext?.({ + userId: 'u1', + ...(organizationId ? { organizationId } : {}), + permissions: ['legacy_auditor'], + } as any); + expect((sets ?? []).map((s) => s.name)).toEqual(['member_default']); + expect(permissionSetRowReads).toBe(0); + }); + + it(`CONTROL: the catalog's definition resolves whole for a caller ${label}`, async () => { + const svc = await locateSecurityService([SALES_MANAGER], [ROW_ONLY]); + const sets = await svc.resolvePermissionSetsForContext?.({ + userId: 'u1', + ...(organizationId ? { organizationId } : {}), + permissions: ['sales_manager'], + } as any); + const found: any = (sets ?? []).find((s) => s.name === 'sales_manager'); + expect(found?.objects).toEqual({ deal: { allowRead: true, allowEdit: true } }); + expect(permissionSetRowReads).toBe(0); + }); + } }); diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index a49f9ed47b3..a6ef33c204d 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -2950,7 +2950,7 @@ describe('PermissionEvaluator', () => { expect(result).toEqual([psAdmin]); // ...but the swallow is surfaced, naming the unresolved sets expect(warns.length).toBe(1); - expect(warns[0].msg).toContain('db lookup failed'); + expect(warns[0].msg).toContain('catalog lookup failed'); expect(warns[0].meta?.unresolved).toEqual(['custom_sales']); expect(warns[0].meta?.error).toBe('db down'); }); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 058cd617bd7..d85d8d8b554 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -2,7 +2,7 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import { declaredHttpStatus } from '@objectstack/types'; -import { Plugin, PluginContext, POSTURE_LADDER, isRowActive, buildEffectiveObjectPermissions, recordNotFoundError, withoutOperationPrivateKeys } from '@objectstack/core'; +import { Plugin, PluginContext, POSTURE_LADDER, buildEffectiveObjectPermissions, recordNotFoundError, withoutOperationPrivateKeys } from '@objectstack/core'; import type { EffectiveObjectPermission, PermissionSet, RowLevelSecurityPolicy, TenantLayer0Verdict } from '@objectstack/spec/security'; import { describeHighPrivilegeBits, describeAnchorForbiddenBits, PUBLIC_FORM_SERVER_MANAGED_FIELDS } from '@objectstack/spec/security'; import type { AnchorBindingContext } from '@objectstack/spec/security'; @@ -101,7 +101,7 @@ import { PLATFORM_OWNER_WALL_BYPASS_EVENT, isVerifiedPlatformOwnerRow, } from './platform-owner-wall-bypass.js'; -import { isConfiguredPlatformAdminEmail, resolvePlatformAdminEmails, vetOrganizationClaim, createSecurityCatalogReader, bindSecurityCatalogReader } from '@objectstack/core'; +import { isConfiguredPlatformAdminEmail, resolvePlatformAdminEmails, vetOrganizationClaim, createSecurityCatalogReader, bindSecurityCatalogReader, securityCatalogReaderOf, readDisabledCatalogNames } from '@objectstack/core'; import { isPlatformTenantPolicy, isAuthoredTenantPolicy } from './platform-tenant-policies.js'; import { isPlatformOwnershipFloorPolicy, @@ -157,7 +157,6 @@ import { DetailRecordNotFoundError, MasterReferenceMissingError, MaskedValueWriteError, - PermissionSetReadUnansweredError, } from './errors.js'; import { assertEngineOwnedWriteAllowed } from './system-write-guard.js'; import { bootstrapPlatformAdmin, findExistingPlatformAdmin, shouldReplayBootstrapFor } from './bootstrap-platform-admin.js'; @@ -1025,53 +1024,6 @@ function userFacingDenialMessage( return renderOperationMessage({ messageKey }, { locale, translate }); } -/** - * The `sys_permission_set` page the enforcement-plane loader asked for, or a - * REFUSAL — never a different empty. - * - * ## The shape, driven rather than declared - * - * `ObjectQL.find` resolves a BARE array of row objects. That is MEASURED, not - * read off `IDataEngine.find`'s declared `Promise`: a declared type is - * not proof here, because this repo also carries a `find()` that resolves a - * `QueryResult` envelope and never an array. `engine-find-bare-array.pin.test.ts` - * boots a real `ObjectQL` over a real `SqlDriver`, starts this plugin against - * it, and pins what this very loader receives — on a populated page and an - * empty one. The `{ records }` limb this function replaced was therefore - * unreachable: dead code that read as a contract. - * - * ## Why anything else REFUSES instead of returning `[]` - * - * This is the enforcement plane. A value this loader cannot read is a read that - * did not answer, and the one thing it must never become is "this principal has - * no permission sets" — a statement that silently withdraws grants that exist - * while the request keeps looking normal. `PermissionEvaluator - * .resolvePermissionSets` catches the refusal one frame up, keeps the request - * fail-closed exactly as before, and reports it once (#2565's warn, which the - * old swallow made unreachable). - * - * A non-object ELEMENT refuses for the same reason, and that half is the - * repair's other direction: it used to be dropped in silence by the trailing - * `r?.name === name` filter, which is the same invention one row at a time. - */ -function permissionSetPageOrRefuse(rows: unknown, names: readonly string[]): any[] { - if (!Array.isArray(rows)) { - throw new PermissionSetReadUnansweredError( - names, - `the engine resolved ${rows === null ? 'null' : typeof rows}, not an array of rows`, - ); - } - for (const row of rows) { - if (!row || typeof row !== 'object') { - throw new PermissionSetReadUnansweredError( - names, - `the page carried a ${row === null ? 'null' : typeof row} where a row object was contracted`, - ); - } - } - return rows; -} - /** * [ADR-0058 D4] The policies whose predicate the write post-image check * compiles, out of the policies that apply to this principal, object and write @@ -1381,37 +1333,20 @@ export class SecurityPlugin implements Plugin { */ private readonly objectSecurityMetaCache = new Map(); /** - * Permission-set loader FACTORY, parameterised by the caller's organization. - * A factory rather than a bare loader because the read is organization-scoped - * and the organization is a per-request fact — see where it is built for why - * an unscoped by-name read stops being correct once the catalog is - * materialized per organization. - */ - private dbLoaderFor?: (organizationId?: string) => (names: string[]) => Promise; - - /** - * The organization a permission-set resolution runs in — the caller's own - * active organization, never a scan across all of them. - * - * Undefined for a `single`-posture caller (and for any context carrying no - * organization at all), which lands on the organization-less surface that - * posture correctly has. Both spellings are read for the same reason the - * sharing service reads both: `tenantId` is the declared envelope field and - * `organizationId` is the cast one, and a hand-built context may carry - * either. + * [ADR-0131 D3/D4] The permission-set loader for names the metadata list and + * the bootstrap sets did not answer: the security catalog's definitions, by + * name. Environment-level — the catalog takes no organization — so one loader + * serves every caller. See where it is built. */ - private callerOrganizationId(context: any): string | undefined { - const id = (context?.organizationId ?? context?.tenantId); - return typeof id === 'string' && id !== '' ? id : undefined; - } + private catalogSetLoader?: (names: string[]) => Promise; /** * The permission-set loader for ONE caller. `undefined` when no engine is - * wired, exactly as the un-parameterised loader used to be, so - * `resolvePermissionSets` keeps its "no db source" branch. + * wired, so `resolvePermissionSets` keeps its "no third source" branch. The + * caller does not choose the answer: the catalog is environment-level. */ - private dbLoaderForContext(context: any): ((names: string[]) => Promise) | undefined { - return this.dbLoaderFor?.(this.callerOrganizationId(context)); + private dbLoaderForContext(_context: any): ((names: string[]) => Promise) | undefined { + return this.catalogSetLoader; } /** * [#10757] Per-EXECUTION-CONTEXT memo for @@ -2022,156 +1957,42 @@ export class SecurityPlugin implements Plugin { ); } - // Construct a dbLoader FACTORY once that lets resolvePermissionSets - // surface user-defined permission sets from `sys_permission_set` - // (created via the admin UI) in addition to plugin-registered ones. + // [ADR-0131 D3/D4] The third source `resolvePermissionSets` asks, for the + // names the metadata list and the bootstrap sets did not answer: the + // security catalog's definition of each name (`securityCatalogReaderOf`, + // the reader `bindSecurityCatalogReader` attached to this engine in + // `start()`) — the environment registry, where every package manifest's + // sets and every environment-authored set hydrated from `sys_metadata` + // live. It is the same read the authorization resolver takes a set's body + // from, so enforcement and resolution read one source. // - // Scoped to the CALLER's organization rather than unscoped. Once the - // catalog is materialized per organization, several organizations hold a - // row for the same name, and a by-name `$in` under a bare - // `{ isSystem: true }` resolves whichever copy the driver ordered first — - // one tenant's grants answering another tenant's request. Threading the - // organization routes the read through `SqlDriver.applyTenantScope` (the - // governed chokepoint, unchanged by this work), and - // `resolveOwnOrganizationRow` then prefers this organization's own row over - // any organization-less one still standing. + // ⛔ No `sys_permission_set` row is read. A set that exists only as a row + // (an organization's Setup-authored set under a wall, a row whose + // definition was never saved) has no registry home, and the resolver + // already grants nothing through it (#15196 Q3 = A); a row fallback here + // would grant its object map anyway — a second source the cutover retires + // (#22601 → B). // - // The `limit` moves with it. `names.length` was exactly right while one row - // existed per name, and is a TRUNCATION the moment copies exist: the driver - // returns this organization's rows AND any organization-less ones, so the - // cap must admit both or a caller silently loses sets. It stays bounded — a - // multiple of the names asked for, never unbounded. - const dbLoaderFor = ql - ? (organizationId?: string) => async (names: string[]) => { - // ⛔ NO `catch` here, and ⛔ no invented empty below — this seam is - // the DROP-shaped half of this cleanup and is repaired in the OPPOSITE - // direction from the dead limbs elsewhere in this plugin. - // - // Three distinct facts used to leave here as one value, `[]`: the read - // succeeded on an empty catalog; the read THREW; the read resolved - // something this loader could not read. Only the first is an answer. - // The other two were inventions, and this is the enforcement plane — - // "no permission sets" silently withdraws grants that EXIST while - // every request still looks completely normal. The residue comment - // below already names that failure ("revokes standing access with no - // signal at the moment of loss"); the swallow above it produced the - // same loss for a whole page at once. - // - // The refusal is not this seam's invention: `resolvePermissionSets` - // ALREADY declares how a throwing loader is handled — it catches, the - // unresolved sets grant nothing (fail closed, unchanged), and the - // failure is named in a warn, because without it "a transient DB error - // makes custom permission sets silently vanish and the resulting 403s - // are undiagnosable" (#2565). Swallowing here made that warn - // UNREACHABLE. Letting the read fault propagate is what re-arms a - // diagnostic this repo had already built, and it is the direction the - // 2026-08-11 store-fault ruling settles: a fault propagates. - // [#20555] With NO active organization the read asks for the - // organization-less rows ONLY. `seedCtx(undefined)` carries no - // tenant, and `applyTenantScope` reads "no tenant" as an unscoped - // path, so a bare by-name read returned every organization's row of - // each name — and `resolveOwnOrganizationRow` answers an undefined - // organization with whichever row came first. The names asked for - // include the caller's POSITIONS (the fold in - // `resolvePermissionSetsForContextUnmemoized`), and with no active - // organization those still carry every membership's role plus the - // `everyone` anchor. Measured over a real `SqlDriver`: a member of - // one organization, with none active, resolved a set ANOTHER - // organization had authored under the name `org_member` — its - // `systemPermissions` and its object map, view/modify-all included — - // and a GLOBAL grant resolved another organization's same-named copy - // in place of the global row it named. - // - // The rule is the one `resolveUserAuthzGrants` applies to grant rows, - // and ADR-0123 D2's "tenant-scoped reads resolve to nothing": a row - // scoped to an organization applies only while that organization is - // active; an organization-less row applies everywhere. With a tenant - // the driver's scope already returns exactly those two classes. With - // none, the predicate below is that same scope, pushed into the read - // rather than filtered after it — after the `limit`, other - // organizations' copies could crowd out the global row this caller - // does hold. - const where = organizationId - ? { name: { $in: names } } - : { name: { $in: names }, organization_id: null }; - const rows = await ql.find( - 'sys_permission_set', - { where, limit: Math.max(names.length * 4, 20) }, - { context: seedCtx(organizationId) }, - ); - const fetched = permissionSetPageOrRefuse(rows, names); - // One row per NAME: this organization's own where it has one, an - // organization-less leftover only where it does not. - // - // The residue arm is load-bearing and was missing (#11121 shipped the - // comment without it, and its suite covers seeding rather than this - // loader). `resolveOwnOrganizationRow` is written for SEEDERS, where - // refusing to see a residue as "already seeded" is the whole point — - // so it never returns one as `own`. ENFORCEMENT wants the opposite - // reading: an organization-less row is still a row this principal was - // granted, and dropping it revokes standing access with no signal at - // the moment of loss — the failure mode this catalog's own header, - // and `resolve-authz-context`'s `sys_position` read, both call out as - // the thing not to do. The asymmetry was observable on a single row: - // its `system_permissions` and `tab_permissions` kept applying (that - // read is unscoped by id) while its `object_permissions` and - // `admin_scope` silently stopped. - // - // Preference order is unchanged and still closes the cross-tenant - // bleed #11121 fixed: this organization's own row WINS wherever it - // exists, and a leftover is consulted only in its absence. With no - // active organization every row here is organization-less (the read - // above asked for nothing else), so `own` is one of those. - const byName = new Map(); - for (const name of new Set(names)) { - const { own, organizationLessResidue } = resolveOwnOrganizationRow( - fetched.filter((r: any) => r?.name === name), - organizationId, - ); - const row = own ?? organizationLessResidue; - if (row) byName.set(name, row); - } - const all = Array.from(byName.values()); - // [ADR-0049] A DEACTIVATED set grants nothing. Not defence in depth - // that nothing reaches: `resolvePermissionSetsForContext` requests - // `context.positions` as permission-set NAMES too (a position name is - // commonly reused as a set name), so an ACTIVE position whose name - // matches a DEACTIVATED `sys_permission_set` row arrives here with - // that name still standing — this loader is the only place that read - // is judged. Filtered in memory rather than by a `where` predicate: - // `active: true` would also drop rows whose column is NULL (rows - // predating the field), and boolean `where` coercion differs per - // driver. `isRowActive` is the same predicate the core resolver and - // the break-glass guard use, so the three cannot drift. - const list = all.filter((r: any) => isRowActive(r)); - const parseJson = (v: any, fallback: any) => { - if (typeof v !== 'string') return v ?? fallback; - try { return JSON.parse(v || JSON.stringify(fallback)); } catch { return fallback; } - }; - return list.map((r: any) => ({ - name: r.name, - label: r.label, - objects: parseJson(r.object_permissions, {}), - fields: parseJson(r.field_permissions, {}), - systemPermissions: parseJson(r.system_permissions, []), - // [#7616] Hydrate the tab column too. Nothing on the DATA plane - // reads `tabPermissions` (the evaluator never mentions it), so this - // is inert for enforcement — but `resolvePermissionSetsForContext` - // is published on the `security` service as returning the sets - // WHOLE, and a loader that dropped this column would make that - // declaration false for every DB-authored set: the UI-plane copy in - // `/me/apps` reads exactly `tab_permissions` off the same row. - // Declared ≠ delivered is the failure this contract exists to - // prevent, so the column is loaded where the promise is made. The - // row is already fetched in full — no extra query, one JSON parse. - tabPermissions: parseJson(r.tab_permissions, {}), - // [ADR-0090 D12] Hydrate the delegated-admin scope so the gate can - // resolve a DB-authored delegate's authority. Null column → absent. - ...(r.admin_scope ? { adminScope: parseJson(r.admin_scope, undefined) } : {}), - })); - } - : undefined; - this.dbLoaderFor = dbLoaderFor; + // ⛔ No `catch` either. A catalog read that did not happen throws the + // reader's own `AuthzStoreUnavailableError`, and `resolvePermissionSets` + // catches it one frame up: the unresolved sets grant nothing (fail closed) + // and the failure is named in its warn (#2565). Swallowing it here would + // report "no such set" for a read that never answered. + // + // Whether a set is switched off is not this loader's question: the names + // it is asked for arrive already judged by the resolver's ledger read, + // except a POSITION name folded into the request, which + // `resolvePermissionSetsForContextUnmemoized` judges itself. + this.catalogSetLoader = async (names: string[]) => { + const catalog = securityCatalogReaderOf(ql); + if (!catalog) return []; + const out: PermissionSet[] = []; + for (const name of new Set(names)) { + const entry = await catalog.resolve('permission', name); + if (entry) out.push(entry.definition as unknown as PermissionSet); + } + return out; + }; // [ADR-0090 D12] Delegated-admin gate shares the SAME permission-set // resolution as the CRUD middleware, so a delegate's authority and their @@ -7146,6 +6967,7 @@ export class SecurityPlugin implements Plugin { this.dbLoaderForContext(context), { logger: this.logger }, ); + permissionSets = await this.dropDeactivatedFoldedSets(positions, explicitPermissionSets, baselineApplied, permissionSets); // [#13419 执行要点 3] Report — never alter — a grant that exists ONLY because // this method folded a POSITION name into the permission-set request above. this.reportNameFoldCollisions(positions, explicitPermissionSets, baselineApplied, permissionSets); @@ -7170,6 +6992,43 @@ export class SecurityPlugin implements Plugin { return permissionSets; } + /** + * [ADR-0049, ADR-0131 D3] Drop a set that reached the resolution ONLY because + * a POSITION of the same name was folded into the request, when the + * activation ledger switched that set off. + * + * Every other name arrives already judged: `context.permissions` is the + * resolver's output, which drops a deactivated set (§6b), and the baseline + * is the deployment's configuration, applied regardless. A folded position + * name is the one name nobody judged as a SET — the position is in effect, + * but the set that shares its name may not be. The row loader this replaced + * judged it with the catalog row's `active` column; the switch now lives in + * `sys_metadata_activation`, read through the resolver's own + * `readDisabledCatalogNames`, so this path and the resolver cannot disagree. + * + * The ledger is read only when a folded name actually resolved, so a request + * with no such collision issues no read. A read that fails throws + * (`AuthzStoreUnavailableError`), and the request fails closed. + */ + private async dropDeactivatedFoldedSets( + positions: unknown, + explicitPermissionSets: unknown, + baselineApplied: string[], + resolved: PermissionSet[], + ): Promise { + if (!Array.isArray(positions) || positions.length === 0 || resolved.length === 0) return resolved; + const judged = new Set(baselineApplied); + for (const n of Array.isArray(explicitPermissionSets) ? explicitPermissionSets : []) { + if (typeof n === 'string') judged.add(n); + } + const resolvedNames = new Set(resolved.map((ps) => (ps as any)?.name).filter((n: unknown): n is string => typeof n === 'string')); + const folded = positions.filter((n): n is string => typeof n === 'string' && resolvedNames.has(n) && !judged.has(n)); + if (folded.length === 0) return resolved; + const disabled = await readDisabledCatalogNames(this.ql, [], [...new Set(folded)]); + if (disabled.permissions.size === 0) return resolved; + return resolved.filter((ps) => !disabled.permissions.has((ps as any)?.name)); + } + /** * [#13419 执行要点 3, warning half] Report a permission-set name that collides * with a POSITION name and has NO junction row — the ungoverned half of the diff --git a/packages/plugins/plugin-security/src/security-readers-catalog.test.ts b/packages/plugins/plugin-security/src/security-readers-catalog.test.ts new file mode 100644 index 00000000000..e52fa0e2dc5 --- /dev/null +++ b/packages/plugins/plugin-security/src/security-readers-catalog.test.ts @@ -0,0 +1,173 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0131 D3/D4] The security plugin's third permission-set source — the + * catalog loader (`SecurityPlugin.catalogSetLoader`) — pinned the way the row + * loader it replaced was pinned (`engine-find-bare-array.pin.test.ts` block 7, + * #15598), against the post-cutover read. + * + * The loader answers the names the metadata list and the bootstrap sets did + * not: each name's definition in the security catalog the plugin binds to its + * engine. Three facts must never leave it as one value: + * + * - the catalog holds the set → the definition, WHOLE; + * - the catalog holds no set of that name (or no catalog is bound) → nothing; + * - the catalog read did NOT answer → a THROW (`AuthzStoreUnavailableError`), + * never "no such set". `PermissionEvaluator.resolvePermissionSets` catches it + * one frame up: the unresolved sets grant nothing (fail closed) and the + * #2565 warn names the failure, once. + * + * And it reads no `sys_permission_set` row at all: a set only a row carries + * resolves nothing, whatever the row says. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { + AUTHZ_STORE_UNAVAILABLE_CODE, + AUTHZ_STORE_UNAVAILABLE_STATUS, + AuthzStoreUnavailableError, +} from '@objectstack/core'; +import type { PermissionSet } from '@objectstack/spec/security'; +import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core'; + +import { SecurityPlugin } from './security-plugin.js'; +import { PermissionEvaluator } from './permission-evaluator.js'; + +/** An environment-authored set, as the engine registry holds it. */ +const CATALOG_SET: PermissionSet = { + name: 'ps_catalog', + label: 'Catalog authored', + objects: { deal: { allowRead: true, allowEdit: true } }, + fields: { 'deal.amount': { readable: true, editable: false } }, + systemPermissions: ['setup.access'], + tabPermissions: { app_crm: 'visible' }, +} as any; + +interface Fixture { + /** What the registry's by-name read does: answer from `CATALOG_SET`, or throw. */ + registryThrows?: Error; + /** `sys_permission_set` rows — present to prove they are never read. */ + rows?: Array>; + /** Boot with no registry on the engine: no catalog gets bound. */ + noRegistry?: boolean; +} + +/** Boot the real plugin over a double engine and hand back its private catalog loader. */ +async function loaderOver(fixture: Fixture = {}) { + let rowReads = 0; + const ql: any = { + registerMiddleware: () => {}, + getSchema: () => null, + findOne: async (object: string, query?: EngineFindOneQueryInput) => { assertEngineFindOnePredicate(object, query); return null; }, + // No table answers here: every set is the catalog's. Only a read that + // asks for a set the loader was asked about counts — the boot's own + // background passes (the org-admin backfill, the baseline binder) read + // their own names and race the call. + find: async (object: string, query?: unknown) => { + if (object === 'sys_permission_set' && /ps_catalog|ps_row_only/.test(JSON.stringify(query ?? {}))) rowReads += 1; + return []; + }, + }; + if (!fixture.noRegistry) { + ql.registry = { + getItem: (type: string, name: string) => { + if (fixture.registryThrows) throw fixture.registryThrows; + return type === 'permission' && name === CATALOG_SET.name ? CATALOG_SET : undefined; + }, + listItems: (type: string) => (type === 'permission' ? [CATALOG_SET] : []), + isPackageDisabled: () => false, + }; + } + const warn = vi.fn(); + const ctx: any = { + logger: { info: vi.fn(), warn, error: vi.fn(), debug: vi.fn() }, + registerService: vi.fn(), + getService: (name: string) => { + if (name === 'objectql') return ql; + if (name === 'metadata') return { get: async () => null, list: async () => [] }; + if (name === 'manifest') return { register: vi.fn() }; + throw new Error(`service not registered: ${name}`); + }, + }; + const plugin = new SecurityPlugin(); + await plugin.init(ctx); + await plugin.start(ctx); + const loader = (plugin as any).catalogSetLoader as ((names: string[]) => Promise) | undefined; + expect(typeof loader, 'the catalog loader was never built — the boot bailed out').toBe('function'); + rowReads = 0; // boot-time reads are not the loader's + return { loader: loader!, rowReads: () => rowReads }; +} + +describe('[ADR-0131 D3/D4] the catalog loader — what it answers', () => { + it('a set the catalog holds comes back WHOLE, and no sys_permission_set row is read', async () => { + const { loader, rowReads } = await loaderOver(); + const sets = await loader(['ps_catalog']); + expect(sets).toEqual([CATALOG_SET]); + expect(rowReads()).toBe(0); + }); + + it('a name the catalog does not hold answers nothing — an EMPTY answer is a real answer', async () => { + const { loader } = await loaderOver(); + await expect(loader(['ps_nowhere'])).resolves.toEqual([]); + }); + + it('a set only a sys_permission_set ROW carries resolves nothing, and the row is never read', async () => { + const { loader, rowReads } = await loaderOver({ + rows: [{ name: 'ps_row_only', object_permissions: JSON.stringify({ deal: { allowDelete: true } }) }], + }); + await expect(loader(['ps_row_only'])).resolves.toEqual([]); + expect(rowReads()).toBe(0); + }); + + it('a name asked twice is answered once', async () => { + const { loader } = await loaderOver(); + expect((await loader(['ps_catalog', 'ps_catalog'])).map((s) => s.name)).toEqual(['ps_catalog']); + }); + + it('an engine with no catalog bound answers nothing, never a guess', async () => { + const { loader } = await loaderOver({ noRegistry: true }); + await expect(loader(['ps_catalog'])).resolves.toEqual([]); + }); +}); + +describe('[ADR-0131 D3/D4, #2565] a catalog read that did not answer', () => { + it('PROPAGATES as the reader\'s own loud failure instead of reading as "no such set"', async () => { + const { loader } = await loaderOver({ registryThrows: new Error("Datasource 'primary' is declared but not connected") }); + // ⛔ Not `toThrow()` alone: the envelope is the assertion. + const err = await loader(['ps_catalog']).then(() => null, (e: unknown) => e); + expect(err).toBeInstanceOf(AuthzStoreUnavailableError); + expect(err).toMatchObject({ code: AUTHZ_STORE_UNAVAILABLE_CODE, status: AUTHZ_STORE_UNAVAILABLE_STATUS }); + }); + + it('stays FAIL-CLOSED through the evaluator, and its #2565 warn fires exactly once', async () => { + const { loader } = await loaderOver({ registryThrows: new Error('registry unreadable') }); + const warns: Array<{ msg: string; meta: any }> = []; + const resolved = await new PermissionEvaluator().resolvePermissionSets( + ['ps_catalog'], + { list: async () => [] }, + [], + loader, + { logger: { warn: (msg: string, meta?: any) => { warns.push({ msg, meta }); } } }, + ); + // The unresolved set grants nothing — fail closed, never a widening. + expect(resolved).toEqual([]); + // The loss is sayable: one warn, naming the catalog and the names it could not answer. + expect(warns).toHaveLength(1); + expect(warns[0].msg).toContain('catalog lookup failed'); + expect(warns[0].meta?.unresolved).toEqual(['ps_catalog']); + }); + + it('CONTROL: the same request with a catalog that answers resolves the set and warns nothing', async () => { + const { loader } = await loaderOver(); + const warns: string[] = []; + const resolved = await new PermissionEvaluator().resolvePermissionSets( + ['ps_catalog'], + { list: async () => [] }, + [], + loader, + { logger: { warn: (msg: string) => { warns.push(msg); } } }, + ); + expect(resolved.map((s) => s.name)).toEqual(['ps_catalog']); + expect(warns).toEqual([]); + }); +}); diff --git a/packages/plugins/plugin-security/src/write-preview-field-gate-parity.test.ts b/packages/plugins/plugin-security/src/write-preview-field-gate-parity.test.ts index 0bc3244267e..8bf66a26a47 100644 --- a/packages/plugins/plugin-security/src/write-preview-field-gate-parity.test.ts +++ b/packages/plugins/plugin-security/src/write-preview-field-gate-parity.test.ts @@ -321,6 +321,9 @@ async function boot(sets: PermissionSet[], opts: { orgScoping?: boolean } = {}) engine.registry.registerObject(ENG_LOG_AMENDABLE as any, 'test-package'); engine.registry.registerObject(USER_POSITION as any, 'test-package'); engine.registry.registerObject(ORG_TASK as any, 'test-package'); + // [ADR-0131 D3/D4] The position the assignment rows name, declared in the + // security catalog the assignment refusal reads. + engine.registry.registerItem('position', { name: 'sales', label: 'Sales' } as any, 'name' as any, 'test-package'); d.storeFor('crm_account').set('acc_p', { id: 'acc_p', name: 'P', type: 'partner' }); d.storeFor('crm_account').set('acc_d', { id: 'acc_d', name: 'D', type: 'direct' }); diff --git a/packages/qa/dogfood/test/delegation-of-duty.dogfood.test.ts b/packages/qa/dogfood/test/delegation-of-duty.dogfood.test.ts index 22ad78f88a5..f77aeb41cf1 100644 --- a/packages/qa/dogfood/test/delegation-of-duty.dogfood.test.ts +++ b/packages/qa/dogfood/test/delegation-of-duty.dogfood.test.ts @@ -65,7 +65,7 @@ describe('delegation of duty (ADR-0091 D3) — end to end', () => { fallbackPermissionSet: 'deleg_member', }), }); - await stack.signIn(); + const adminToken = await stack.signIn(); delegatorToken = await stack.signUp(DELEGATOR); await stack.signUp(DELEGATE); ql = await stack.kernel.getServiceAsync('objectql'); @@ -81,10 +81,16 @@ describe('delegation of duty (ADR-0091 D3) — end to end', () => { expect(org?.id, 'the boot created the Default Organization').toBeTruthy(); orgId = String(org.id); - // A delegatable position + a non-delegatable one (system inserts sidestep - // authoring rules). The delegator DIRECTLY holds the delegatable one. - await ql.insert('sys_position', { id: 'pos_vac_appr', name: 'vacation_approver', label: 'Vacation Approver', delegatable: true, active: true, organization_id: orgId }, SYS); - await ql.insert('sys_position', { id: 'pos_locked', name: 'locked_duty', label: 'Locked Duty', delegatable: false, active: true, organization_id: orgId }, SYS); + // A delegatable position + a non-delegatable one, declared where the gate + // and the resolver read a position (ADR-0131 D3/D4): the security catalog, + // through the metadata door. The delegator DIRECTLY holds the delegatable one. + for (const [name, label, delegatable] of [ + ['vacation_approver', 'Vacation Approver', true], + ['locked_duty', 'Locked Duty', false], + ] as const) { + const saved = await stack.apiAs(adminToken, 'PUT', `/meta/position/${name}`, { name, label, delegatable }); + expect(saved.status, await saved.clone().text().catch(() => '')).toBe(200); + } await ql.insert('sys_user_position', { id: 'hold_boss', user_id: delegatorId, position: 'vacation_approver', organization_id: orgId }, SYS); await ql.insert('sys_user_position', { id: 'hold_boss_locked', user_id: delegatorId, position: 'locked_duty', organization_id: orgId }, SYS); }, 90_000); diff --git a/packages/qa/dogfood/test/showcase-scope-depth-fallback.dogfood.test.ts b/packages/qa/dogfood/test/showcase-scope-depth-fallback.dogfood.test.ts index 33d70d40866..52d006c86d3 100644 --- a/packages/qa/dogfood/test/showcase-scope-depth-fallback.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-scope-depth-fallback.dogfood.test.ts @@ -10,7 +10,7 @@ // `objectstack dev`/`serve`/`start` actually take: the app declares the default // profile in METADATA, the CLI computes its name with `appDefaultPermissionSetName` // and passes only that NAME as `fallbackPermissionSet`, and the SecurityPlugin -// resolves the full set (incl. `readScope`) from `sys_permission_set` at request +// resolves the full set (incl. `readScope`) from the security catalog at request // time. The bug this guards: the artifact-serve path used to drop `permissions[]` // from the stack config, so `appDefaultPermissionSetName` saw nothing, the fallback // silently degraded to the built-in owner-only `member_default`, and a grant-less @@ -47,19 +47,19 @@ interface World { stack: VerifyStack; tokens: Record; } // Same BU world as the reference test: bu_parent ⊃ bu_child (sibling bu_other). // alice+carol ∈ bu_parent, bob ∈ bu_child, dave ∈ bu_other. Each owns one note. -// The scope profile is NOT passed as a bootstrap permission set — it is seeded -// into `sys_permission_set` (the runtime home of an app-declared `permission`) -// and reached only by NAME via `fallbackPermissionSet`. +// The scope profile is NOT passed as a bootstrap permission set — it is saved +// through the metadata door into the security catalog (ADR-0131 D3, the one +// home of a permission set) and reached only by NAME via `fallbackPermissionSet`. async function bootFallbackWorld(withResolver = true): Promise { const stack = await bootShowcase({ // Mirror the CLI exactly: the app's isDefault profile name, computed off the // declared `permissions[]`, handed to SecurityPlugin as the fallback. No - // `defaultPermissionSets` carry the scope profile — it must resolve from DB. + // `defaultPermissionSets` carry the scope profile — it must resolve from the catalog. security: new SecurityPlugin({ fallbackPermissionSet: appDefaultPermissionSetName([DEFAULT_PROFILE_METADATA]), }), }); - await stack.signIn(); + const adminToken = await stack.signIn(); const tokens = {} as Record; for (const who of WHO) tokens[who] = await stack.signUp(`fb-${who}@verify.test`); @@ -91,16 +91,16 @@ async function bootFallbackWorld(withResolver = true): Promise { }; if (withResolver) (stack.kernel as any).registerService('hierarchy-scope-resolver', refResolver); - // Seed the app-declared profile into `sys_permission_set` — this is what an - // app `permission` metadata becomes at runtime, and what the named fallback - // resolves through the SecurityPlugin dbLoader. `readScope` rides inside - // object_permissions JSON. - await sys('sys_permission_set', { + // Save the app-declared profile through the metadata door — the security + // catalog the named fallback resolves through (the SecurityPlugin's catalog + // loader, ADR-0131 D3/D4; a `sys_permission_set` row is not read). + // `readScope` rides inside `objects`. + const saved = await stack.apiAs(adminToken, 'PUT', `/meta/permission/${PROFILE_NAME}`, { name: PROFILE_NAME, label: DEFAULT_PROFILE_METADATA.label, - active: true, - object_permissions: JSON.stringify(DEFAULT_PROFILE_METADATA.objects), + objects: DEFAULT_PROFILE_METADATA.objects, }); + expect(saved.status, await saved.clone().text().catch(() => '')).toBe(200); const uid = async (who: Who) => (await ql.findOne('sys_user', { where: { email: `fb-${who}@verify.test` }, context: { isSystem: true } }))?.id; diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index daa9b00bc62..b30be9122b5 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -3411,6 +3411,11 @@ "verb": "update", "pinned": 2 }, + { + "file": "packages/plugins/plugin-security/src/security-readers-catalog.test.ts", + "verb": "findOne", + "pinned": 1 + }, { "file": "packages/plugins/plugin-security/src/seed-write-refusal.test.ts", "verb": "update", diff --git a/scripts/objectql-double-limit.baseline.json b/scripts/objectql-double-limit.baseline.json index 4349aff2929..bd41848d76d 100644 --- a/scripts/objectql-double-limit.baseline.json +++ b/scripts/objectql-double-limit.baseline.json @@ -618,9 +618,6 @@ "packages/plugins/plugin-security/src/permission-denied-user-copy.test.ts": { "blind": 1 }, - "packages/plugins/plugin-security/src/permission-set-active.test.ts": { - "unjudged": 1 - }, "packages/plugins/plugin-security/src/permission-set-drift.test.ts": { "unjudged": 1 },