diff --git a/.changeset/16712-position-catalog-refusal.md b/.changeset/16712-position-catalog-refusal.md new file mode 100644 index 00000000000..d0c49a9f57f --- /dev/null +++ b/.changeset/16712-position-catalog-refusal.md @@ -0,0 +1,65 @@ +--- +'@objectstack/plugin-security': minor +--- + +fix(plugin-security)!: a `sys_user_position` write whose `position` names no `sys_position` row in the writer's catalog is refused (#16712, #20297), instead of answering 201 over an assignment that grants nothing + +Clause-②: yes + + + +**BREAKING** accept-set narrowing on the `sys_user_position` write path — +shipped as `minor` under the launch-window convention (`check-changeset-no-major` +refuses `major`; breaking-ness is carried by this banner and the ADR-0087 +disposition, not by the level). Maintainer-confirmed ruling on #16712 (option A), +with the catalog it reads settled on #20297: the writer's organization, under +the platform's standing tenancy rule. + +**What changed.** `sys_user_position.position` is the position's machine NAME +(`sys_position.name`), but it is declared `Field.text`, so a value naming no +catalog row — most often the position's record ID, written where its name +belongs — was stored with a `201` and then resolved to nothing: the holder got +no permission set, no sharing rule reached them, and they signed in to an app +that reads nothing, with no error anywhere. Such a write is now refused: + +- `400 VALIDATION_FAILED`, one `fields[]` entry per offending value at + `field: 'position'`, `code: 'reference_not_found'`, + `constraint: { target: 'sys_position', targetField: 'name' }` — the same + envelope a bad `user_id` or `organization_id` on the same row already gets. +- The message names the value and says the column takes the catalog NAME. When + the value is the record id of a position the writer's own organization can + see, it names that position and says to write its name. + +**Which writes.** Every non-system insert (one row or a batch, refused whole), +every non-system update by id that CHANGES `position`, and every predicate +update (`multi: true`) that sets it. The check runs after authorization: a +caller who may not write the table is still refused `403` on authority and never +sees the catalog verdict. + +**Whose catalog.** The writer's: the positions of the writer's own +organization plus the organization-less ones — the same reach the engine gives +its own lookup-reference check. A name that only ANOTHER organization's +catalog carries is refused exactly like any unknown name, with the same +envelope and the same message, so the answer says nothing about other +organizations. On a single-organization deployment the declared positions +carry no organization and any other position can only carry the one +organization there is, so every writer sees the whole catalog. A writer whose +context names no organization sees every organization's positions. + +**What did not change.** + +- A **deactivated** position is still a catalog row: an assignment naming it is + accepted and, as before, grants nothing (ADR-0049). +- **Stored rows** are untouched. An update that edits another column, or echoes + the unchanged `position` back, is not judged, so an existing row whose name + is no longer in the catalog stays editable. +- **System-context writes** are not judged — the seed loader (which on a fresh + single-organization boot writes `stack.data` before the declared position + catalog exists), invitation acceptance and the platform's own bootstraps. That + is the same stand-down the engine's lookup check takes. + +**Who is affected.** A client, script or AI author that writes a position's id, +a misspelled name, a name not yet created, or — on a deployment that walls +organizations off — a name only another organization has. The fix is the one +the refusal names: write the name of a position in the writer's own catalog, or +create the position there first. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 61d0057f684..aaaabcc5690 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -10,7 +10,7 @@ the seed loader replaying package fixtures, a plugin's boot reconciler, a service self-write, a migration. This page is **the authority** for what that flag actually does. It exists -because the flag is not one concept: it is a single boolean read at **105 +because the flag is not one concept: it is a single boolean read at **106 distinct sites across 19 packages**, and knowing three of those behaviours gives no hint that the other hundred-and-four exist. Every documented app-side bug traced to `isSystem` had the same shape — the metadata was complete and correct, @@ -125,6 +125,7 @@ that silently does not happen. | 21 | **`readonly` strip bypassed — INSERT** | objectql | Same, on create — one gate over BOTH create-side passes since the 2026-09-03 ruling moved the static-`readonly` strip in beside the runtime-owned one and deleted the DataProtocol ingress copy. `isSystem` is the **only** exemption on this path: `preserveAudit` is deliberately not read on create, so a non-system historical import is still stripped | `packages/objectql/src/engine.ts#insert` | | 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 | `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` | | 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` | | 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `packages/plugins/plugin-security/src/system-write-guard.ts#isUserContextWrite`, `#assertEngineOwnedWriteAllowed` | | 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `packages/plugins/plugin-auth/src/identity-write-guard.ts#isUserContextWrite` | @@ -136,7 +137,7 @@ that silently does not happen. ### 3. Sharing (`plugin-sharing`) -The largest single consumer — **17 of the 105 sites**. +The largest single consumer — **17 of the 106 sites**. | # | Behaviour when `isSystem` | What you get / what you lose | Anchor | |:--|:---|:---|:---| @@ -279,7 +280,7 @@ Ownership injection, `readonly` bypass and sharing materialisation are independent decisions, and a seed loader plausibly wants the first two but not the third. The concept is nevertheless **staying as one boolean**: -- **Shipped semantics.** `isSystem` is a published contract with 105 read sites +- **Shipped semantics.** `isSystem` is a published contract with 106 read sites in 19 packages. Splitting it is a breaking contract change across all of them. (The ruling was taken when the census read 80 sites in 18 packages; the count has grown, which strengthens rather than weakens the argument.) @@ -353,16 +354,16 @@ still holds equal to the census on every pull request: | Appearances of the bare identifier `isSystem` in non-test sources | 813 | — | | — parsed as a declaration | 22 | ✅ | | — parsed as an object-literal / type key (producers and option objects) | 310 | — | -| — parsed as a property **read** | 111 | ✅ | +| — parsed as a property **read** | 112 | ✅ | | — parsed in some other syntactic position (a local, a cast, a conditional) | 9 | ✅ | | — the remainder: text inside comments and string literals | 358 | — | | Of those reads: reads of one of the unrelated metadata fields | 6 | ✅ | -| Of those reads: reads of `ExecutionContext.isSystem` | **105** | ✅ | -| — behaviour-bearing (rows 1–61 above) | 102 | ✅ | +| Of those reads: reads of `ExecutionContext.isSystem` | **106** | ✅ | +| — behaviour-bearing (rows 1–61 above) | 103 | ✅ | | — carry the flag onward only (rows 62–64 above) | 3 | ✅ | | Packages containing at least one elevation read | **19** | ✅ | -| Files containing at least one elevation read | 44 | ✅ | -| — the distinct symbols those reads live in — what this page anchors | 88 | ✅ | +| Files containing at least one elevation read | 45 | ✅ | +| — the distinct symbols those reads live in — what this page anchors | 89 | ✅ | | — of those files, the ones holding more than one read in one symbol | 8 | ✅ | The six rows marked — are a **dated decomposition, not a live claim**: they were @@ -426,7 +427,7 @@ same resolver, and the same registration shape, that holds `docs/adr/**`. Renaming a symbol is now a loud red instead of a silent misdirection. ⚠️ **The precision that costs, priced here rather than buried.** A symbol anchor -cannot say WHICH read inside a function it means, and **8** of the **44** +cannot say WHICH read inside a function it means, and **8** of the **45** anchored files hold more than one read inside a single symbol. So the population check runs per file at symbol granularity: every file the census finds a read in must be anchored, and the set of symbols this page cites into that file must diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts new file mode 100644 index 00000000000..f45666f9480 --- /dev/null +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts @@ -0,0 +1,654 @@ +// 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` — + * 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 + * `SecurityPlugin` registered on it the way a kernel composition does, because + * the refusal is an engine middleware whose whole contract is WHERE it runs in + * that chain: inside the security middleware, after authorization. + * + * Every refusal is identified by its ADR-0112 envelope — `code` and `status`, + * read through `resolveThrownHttpError`, the resolver both HTTP doors answer + * with — never by a bare `toThrow()`: a throw-shaped assertion stays green when + * a DIFFERENT refusal fires first, and on this table the delegated-admin gate + * is always one step earlier. + * + * The ruling this executes keeps a CONTROL LEG, and so does this suite: the + * same account that an id-spelled assignment leaves reading nothing reads rows + * the moment it holds the same permission set through a direct + * `sys_user_permission_set` grant. Without that leg "the account reads nothing" + * and "the permission set is misconfigured" are one observation again. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { resolveUserAuthzGrants } from '@objectstack/core'; +import { resolveThrownHttpError } from '@objectstack/types'; +import type { PermissionSet } from '@objectstack/spec/security'; + +import { SecurityPlugin } from './security-plugin.js'; +import { SysPosition } from './objects/sys-position.object.js'; +import { SysUserPosition } from './objects/sys-user-position.object.js'; +import { SysPermissionSet } from './objects/sys-permission-set.object.js'; +import { SysPositionPermissionSet } from './objects/sys-position-permission-set.object.js'; +import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; +import { namesWithoutCatalogRow, positionNotInCatalogMessage } from './position-catalog-refusal.js'; + +// --------------------------------------------------------------------------- +// Fixture +// --------------------------------------------------------------------------- + +/** The fallback every authenticated caller resolves: it grants nothing here. */ +const MEMBER_DEFAULT = { + name: 'member_default', + label: 'Member', + objects: {}, +} as unknown as PermissionSet; + +/** A tenant-level administrator (ADR-0066 superuser wildcard): the gate admits it, CRUD admits it. */ +const QA_ADMIN = { + name: 'qa_admin', + label: 'QA Admin', + objects: { + '*': { + allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, + viewAllRecords: true, modifyAllRecords: true, + }, + }, +} as unknown as PermissionSet; + +/** + * The `organization_admin` shape: the superuser wildcard (so the delegated-admin + * gate admits it) with an EXPLICIT per-table deny on the assignment table (so + * the CRUD check refuses it). A caller the platform will not let write this + * table must never learn anything from the catalog. + */ +const QA_ORG_ADMIN_LIKE = { + name: 'qa_org_admin_like', + label: 'QA Org Admin (read-only on RBAC)', + objects: { + '*': { + allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, + viewAllRecords: true, modifyAllRecords: true, + }, + sys_user_position: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false }, + }, +} as unknown as PermissionSet; + +const QA_INQUIRY = { + name: 'qa_inquiry', + label: 'Inquiry', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + subject: { name: 'subject', type: 'text' }, + }, +}; + +const SYS = { isSystem: true } as const; +const ADMIN = { userId: 'u_admin', positions: [], permissions: ['qa_admin'] }; +const ORG_ADMIN_LIKE = { userId: 'u_org_admin', positions: [], permissions: ['qa_org_admin_like'] }; +const PLAIN_MEMBER = { userId: 'u_member', positions: [], permissions: [] }; + +/** The auditor position's catalog row — the row whose ID the measured authors wrote. */ +const AUDITOR_POSITION_ID = 'position_mtur8hz5jp7ptm3r'; + +const engines: ObjectQL[] = []; +afterEach(async () => { + vi.restoreAllMocks(); + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function boot(opts: { walled?: boolean } = {}) { + 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.position-catalog-refusal', + name: 'Position catalog refusal', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [SysPosition, SysUserPosition, SysPermissionSet, SysPositionPermissionSet, SysUserPermissionSet, QA_INQUIRY], + } as any); + await engine.syncSchemas(); + engines.push(engine); + + const warn = vi.fn(); + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (_type: string, name: string) => engine.getSchema(name) ?? null, + list: async () => [MEMBER_DEFAULT, QA_ADMIN, QA_ORG_ADMIN_LIKE], + }, + ...(opts.walled + ? { 'org-scoping': { name: 'com.objectstack.org-scoping' }, tenancy: { posture: 'isolated' } } + : {}), + }; + const ctx: any = { + logger: { info: vi.fn(), warn, error: vi.fn(), debug: vi.fn() }, + // Lifecycle hooks are collected and never fired: the fixture seeds every + // row it reads itself, and a boot-time bootstrap racing the suite's own + // writes (and its teardown) would only add noise to what is measured. + hook: vi.fn(), + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await plugin.init(ctx); + await plugin.start(ctx); + vi.spyOn((engine as any).logger, 'warn').mockImplementation(() => undefined); + + // A walled posture refuses an organization-less system write on a + // tenant-scoped object, so the fixture is seeded INTO org_a there. + const seed = opts.walled ? { isSystem: true, tenantId: 'org_a' } : SYS; + + // The catalog, the one DB-authored set it distributes, and three rows it reads. + await engine.insert('sys_permission_set', { + id: 'ps_auditor', + name: 'qa_auditor_set', + label: 'QA Auditor', + object_permissions: JSON.stringify({ qa_inquiry: { allowRead: true, viewAllRecords: true } }), + field_permissions: '{}', + system_permissions: '[]', + active: true, + }, { context: seed } as any); + await engine.insert('sys_position', [ + { id: AUDITOR_POSITION_ID, name: 'qa_auditor', label: 'Auditor', active: true }, + // ADR-0049 shape E: deactivated, still a catalog row, still bound. + { id: 'pos_retired', name: 'qa_retired', label: 'Retired', active: false }, + ], { context: seed } as any); + await engine.insert('sys_position_permission_set', [ + { id: 'pps_auditor', position_id: AUDITOR_POSITION_ID, permission_set_id: 'ps_auditor' }, + { id: 'pps_retired', position_id: 'pos_retired', permission_set_id: 'ps_auditor' }, + ], { context: seed } as any); + await engine.insert('qa_inquiry', [ + { id: 'inq_1', subject: 'one' }, + { id: 'inq_2', subject: 'two' }, + { id: 'inq_3', subject: 'three' }, + ], { context: seed } as any); + + return { engine, warn }; +} + +type Harness = Awaited>; + +/** What an account reads of `qa_inquiry`, through the real resolver and the real middleware. */ +async function rowsReadBy(h: Harness, userId: string): Promise { + const g = await resolveUserAuthzGrants(h.engine, userId); + try { + const rows = await h.engine.find('qa_inquiry', { + context: { userId, positions: g.positions, permissions: g.permissions }, + }); + return Array.isArray(rows) ? rows.length : 0; + } catch (e: any) { + // An account holding nothing on the object is refused the read outright; + // for this suite that is "reads nothing", the card's observation. + if (e?.code === 'PERMISSION_DENIED') return 0; + throw e; + } +} + +async function refusalOf(run: () => Promise): Promise { + try { + await run(); + } catch (e) { + return e; + } + throw new Error('expected the write to be refused, but it succeeded'); +} + +/** The ADR-0112 envelope an HTTP door answers this throw with. */ +function envelopeOf(e: unknown) { + const r = resolveThrownHttpError(e); + return { status: r.status, code: r.code, fields: (r.details?.fields ?? []) as any[] }; +} + +async function assignmentsOf(h: Harness, userId: string): Promise { + const rows = await h.engine.find('sys_user_position', { where: { user_id: userId }, context: SYS }); + return Array.isArray(rows) ? rows : []; +} + +// --------------------------------------------------------------------------- +// The refusal, and the ruling's control leg +// --------------------------------------------------------------------------- + +describe('a position spelled as the catalog row ID is refused, and the account is provably fine', () => { + it('refuses 400 VALIDATION_FAILED, reference_not_found at position, naming the value and the name to write', async () => { + const h = await boot(); + const err = await refusalOf(() => h.engine.insert( + 'sys_user_position', { user_id: 'u_holder', position: AUDITOR_POSITION_ID }, { context: ADMIN } as any, + )); + const env = envelopeOf(err); + expect(env.code).toBe('VALIDATION_FAILED'); + expect(env.status).toBe(400); + expect(env.fields).toHaveLength(1); + expect(env.fields[0]).toMatchObject({ + field: 'position', + code: 'reference_not_found', + value: AUDITOR_POSITION_ID, + constraint: { target: 'sys_position', targetField: 'name' }, + }); + // The fix is the exact name — the value is that position's record id. + expect(env.fields[0].message).toBe(positionNotInCatalogMessage(AUDITOR_POSITION_ID, 'qa_auditor')); + expect(env.fields[0].message).toContain(`write 'qa_auditor'`); + + // Nothing was stored, and the account reads nothing … + expect(await assignmentsOf(h, 'u_holder')).toHaveLength(0); + expect(await rowsReadBy(h, 'u_holder')).toBe(0); + + // … CONTROL LEG, same account: a direct grant of the set the position + // distributes reads rows at once, so the account, the set and the object + // are fine — the refused edge was the whole difference. + await h.engine.insert( + 'sys_user_permission_set', { user_id: 'u_holder', permission_set_id: 'ps_auditor' }, { context: ADMIN } as any, + ); + expect(await rowsReadBy(h, 'u_holder')).toBe(3); + }); + + it('refuses a name no catalog row carries, with the generic remedy', async () => { + const h = await boot(); + const err = await refusalOf(() => h.engine.insert( + 'sys_user_position', { user_id: 'u_holder', position: 'totally_not_a_position' }, { context: ADMIN } as any, + )); + const env = envelopeOf(err); + expect(env.code).toBe('VALIDATION_FAILED'); + expect(env.status).toBe(400); + expect(env.fields.map((f) => [f.field, f.code, f.value])).toEqual([ + ['position', 'reference_not_found', 'totally_not_a_position'], + ]); + expect(env.fields[0].message).toBe(positionNotInCatalogMessage('totally_not_a_position')); + expect(await assignmentsOf(h, 'u_holder')).toHaveLength(0); + }); +}); + +describe('the accepted half — a catalog NAME, active or deactivated', () => { + it('a catalog name is accepted and resolves: the holder reads rows', async () => { + const h = await boot(); + const created = await h.engine.insert( + 'sys_user_position', { user_id: 'u_named', position: 'qa_auditor' }, { context: ADMIN } as any, + ); + expect(created).toMatchObject({ user_id: 'u_named', position: 'qa_auditor' }); + expect(await rowsReadBy(h, 'u_named')).toBe(3); + }); + + it('ADR-0049 shape E: a DEACTIVATED position is still a catalog row — accepted, stored, and grants nothing', async () => { + const h = await boot(); + const created = await h.engine.insert( + 'sys_user_position', { user_id: 'u_retired', position: 'qa_retired' }, { context: ADMIN } as any, + ); + expect(created).toMatchObject({ position: 'qa_retired' }); + expect(await assignmentsOf(h, 'u_retired')).toHaveLength(1); + // It stops granting (the resolver drops the deactivated name) — it is not refused. + expect(await rowsReadBy(h, 'u_retired')).toBe(0); + }); + + it('single posture: an organization-bound writer still reads organization-less catalog rows', async () => { + // A `single` posture seeds its declared catalog with no organization, and + // this fixture seeds its rows the same way. The scoped read + // (`{ ...context, isSystem: true }`) forwards the writer's tenant, and the + // driver's `organization_id IS NULL` term keeps those rows visible. + const h = await boot(); + const orgBound = { ...ADMIN, tenantId: 'org_a' }; + const created = await h.engine.insert( + 'sys_user_position', { user_id: 'u_ob', position: 'qa_auditor' }, { context: orgBound } as any, + ); + expect(created).toMatchObject({ position: 'qa_auditor' }); + const env = envelopeOf(await refusalOf(() => h.engine.insert( + 'sys_user_position', { user_id: 'u_ob2', position: 'nope_position' }, { context: orgBound } as any, + ))); + expect([env.code, env.status]).toEqual(['VALIDATION_FAILED', 400]); + expect(env.fields[0]).toMatchObject({ field: 'position', code: 'reference_not_found', value: 'nope_position' }); + }); +}); + +// --------------------------------------------------------------------------- +// Every write shape that stores a new name +// --------------------------------------------------------------------------- + +describe('every non-system write that stores a new position name is judged', () => { + it('a batch insert with one bad row is refused whole, naming only the bad value; nothing is stored', async () => { + const h = await boot(); + const err = await refusalOf(() => h.engine.insert('sys_user_position', [ + { user_id: 'u_b1', position: 'qa_auditor' }, + { user_id: 'u_b2', position: 'nope_position' }, + ], { context: ADMIN } as any)); + const env = envelopeOf(err); + expect(env.code).toBe('VALIDATION_FAILED'); + expect(env.status).toBe(400); + expect(env.fields.map((f) => [f.field, f.code, f.value])).toEqual([ + ['position', 'reference_not_found', 'nope_position'], + ]); + expect(await assignmentsOf(h, 'u_b1')).toHaveLength(0); + expect(await assignmentsOf(h, 'u_b2')).toHaveLength(0); + }); + + it('an update by id that CHANGES position to an unknown name is refused; the stored row is untouched', async () => { + const h = await boot(); + await h.engine.insert('sys_user_position', { id: 'upa', user_id: 'u_up', position: 'qa_auditor' }, { context: ADMIN } as any); + const err = await refusalOf(() => h.engine.update( + 'sys_user_position', { id: 'upa', position: AUDITOR_POSITION_ID }, { context: ADMIN } as any, + )); + const env = envelopeOf(err); + expect(env.code).toBe('VALIDATION_FAILED'); + expect(env.status).toBe(400); + expect(env.fields[0]).toMatchObject({ field: 'position', code: 'reference_not_found', value: AUDITOR_POSITION_ID }); + expect((await assignmentsOf(h, 'u_up'))[0]?.position).toBe('qa_auditor'); + }); + + it('a predicate update (multi) setting an unknown name is refused', async () => { + const h = await boot(); + await h.engine.insert('sys_user_position', { id: 'upm', user_id: 'u_multi', position: 'qa_auditor' }, { context: ADMIN } as any); + const err = await refusalOf(() => h.engine.update( + 'sys_user_position', { position: 'nope_position' }, { where: { user_id: 'u_multi' }, multi: true, context: ADMIN } as any, + )); + const env = envelopeOf(err); + expect(env.code).toBe('VALIDATION_FAILED'); + expect(env.status).toBe(400); + expect(env.fields[0]).toMatchObject({ field: 'position', code: 'reference_not_found', value: 'nope_position' }); + expect((await assignmentsOf(h, 'u_multi'))[0]?.position).toBe('qa_auditor'); + }); + + it('an update that does not change position is not judged — a stored fossil stays editable', async () => { + const h = await boot(); + // A row that predates the refusal, written where it is not judged (a system write). + await h.engine.insert('sys_user_position', { id: 'fossil', user_id: 'u_fossil', position: 'gone_position' }, { context: SYS } as any); + // Editing another column, with and without the unchanged value echoed back. + await h.engine.update('sys_user_position', { id: 'fossil', reason: 'edited' }, { context: ADMIN } as any); + await h.engine.update('sys_user_position', { id: 'fossil', position: 'gone_position', reason: 'echoed' }, { context: ADMIN } as any); + const [row] = await assignmentsOf(h, 'u_fossil'); + expect(row).toMatchObject({ position: 'gone_position', reason: 'echoed' }); + }); +}); + +// --------------------------------------------------------------------------- +// A position that is not a string is judged by its string form +// --------------------------------------------------------------------------- + +describe('a non-string position is judged by its string form', () => { + // The engine's `text` validation refuses none of these, and the write stores + // each one with 201, so they are this refusal's to judge. The string form is + // String(value) for a scalar and the JSON text for an object or array. + + it('insert: a number, a boolean, an object and an array are refused 400, reference_not_found at their string form', async () => { + const h = await boot(); + for (const [position, text] of [ + [123, '123'], + [true, 'true'], + [{}, '{}'], + // Judged as its JSON text, never as String(['qa_auditor']) === 'qa_auditor', + // which would accept an array naming a real position that resolves nothing. + [['qa_auditor'], '["qa_auditor"]'], + ] as const) { + const env = envelopeOf(await refusalOf(() => h.engine.insert( + 'sys_user_position', { user_id: 'u_ns', position }, { context: ADMIN } as any, + ))); + expect([env.code, env.status], text).toEqual(['VALIDATION_FAILED', 400]); + expect(env.fields, text).toHaveLength(1); + expect(env.fields[0], text).toMatchObject({ field: 'position', code: 'reference_not_found', value: text }); + expect(env.fields[0].message, text).toBe(positionNotInCatalogMessage(text)); + } + expect(await assignmentsOf(h, 'u_ns')).toHaveLength(0); + }); + + it('update by id: a numeric and a boolean position are refused 400; the stored row is untouched', async () => { + const h = await boot(); + await h.engine.insert('sys_user_position', { id: 'ups', user_id: 'u_nsu', position: 'qa_auditor' }, { context: ADMIN } as any); + for (const [position, text] of [[123, '123'], [true, 'true']] as const) { + const env = envelopeOf(await refusalOf(() => h.engine.update( + 'sys_user_position', { id: 'ups', position }, { context: ADMIN } as any, + ))); + expect([env.code, env.status], text).toEqual(['VALIDATION_FAILED', 400]); + expect(env.fields[0], text).toMatchObject({ field: 'position', code: 'reference_not_found', value: text }); + } + expect((await assignmentsOf(h, 'u_nsu'))[0]?.position).toBe('qa_auditor'); + }); + + it('an operator object is the engine\'s to refuse: invalid_type (#5922), never reference_not_found', async () => { + const h = await boot(); + for (const position of [{ $in: ['x'] }, { $in: [], a: 1 }]) { + const label = JSON.stringify(position); + const env = envelopeOf(await refusalOf(() => h.engine.insert( + 'sys_user_position', { user_id: 'u_op', position }, { context: ADMIN } as any, + ))); + expect([env.code, env.status], label).toEqual(['VALIDATION_FAILED', 400]); + expect(env.fields[0], label).toMatchObject({ field: 'position', code: 'invalid_type' }); + } + expect(await assignmentsOf(h, 'u_op')).toHaveLength(0); + }); + + it('an object with no declared operator key is judged: { a: 1 } and { $foo: 1 } are refused reference_not_found', async () => { + const h = await boot(); + for (const [position, text] of [[{ a: 1 }, '{"a":1}'], [{ $foo: 1 }, '{"$foo":1}']] as const) { + const env = envelopeOf(await refusalOf(() => h.engine.insert( + 'sys_user_position', { user_id: 'u_plain', position }, { context: ADMIN } as any, + ))); + expect([env.code, env.status], text).toEqual(['VALIDATION_FAILED', 400]); + expect(env.fields[0], text).toMatchObject({ field: 'position', code: 'reference_not_found', value: text }); + expect(env.fields[0].message, text).toBe(positionNotInCatalogMessage(text)); + } + expect(await assignmentsOf(h, 'u_plain')).toHaveLength(0); + // Judged, not failed open: the catalog read never gave up on these names. + expect(h.warn.mock.calls.filter((c) => String(c[0]).includes('could not be read'))).toHaveLength(0); + }); + + it('a placeholder-shaped name is compared literally, never resolved as a filter token', async () => { + const h = await boot(); + for (const text of ['{nope_tok}', '{current_user_id}']) { + const env = envelopeOf(await refusalOf(() => h.engine.insert( + 'sys_user_position', { user_id: 'u_tok', position: text }, { context: ADMIN } as any, + ))); + 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); + const created = await h.engine.insert( + 'sys_user_position', { user_id: 'u_tok_ok', position: '{lit_pos}' }, { context: ADMIN } as any, + ); + expect(created).toMatchObject({ position: '{lit_pos}' }); + expect(h.warn.mock.calls.filter((c) => String(c[0]).includes('could not be read'))).toHaveLength(0); + }); + + it("update by id: 123 echoed over a stored '123' is an unchanged value, not judged", async () => { + const h = await boot(); + // A row written where it is not judged (a system write), whose text names no catalog row. + await h.engine.insert('sys_user_position', { id: 'upe', user_id: 'u_echo', position: '123' }, { context: SYS } as any); + await h.engine.update('sys_user_position', { id: 'upe', position: 123, reason: 'echoed' }, { context: ADMIN } as any); + expect((await assignmentsOf(h, 'u_echo'))[0]).toMatchObject({ reason: 'echoed' }); + }); +}); + +// --------------------------------------------------------------------------- +// Scope: what it does NOT judge, pinned so the stand-down stays deliberate +// --------------------------------------------------------------------------- + +describe('scope — the stand-downs are declared, not accidental', () => { + it('an isSystem write is not judged (seed loader, invitation acceptance, platform bootstraps)', async () => { + const h = await boot(); + const created = await h.engine.insert( + 'sys_user_position', { user_id: 'u_sys', position: 'not_in_catalog' }, { context: SYS } as any, + ); + expect(created).toMatchObject({ position: 'not_in_catalog' }); + }); + + it('authorization first: a caller who may not write the table gets 403, never the catalog verdict', async () => { + const h = await boot(); + for (const [caller, missingName] of [ + [PLAIN_MEMBER, 'nope_position'], + [ORG_ADMIN_LIKE, 'nope_position'], + ] as const) { + const err = await refusalOf(() => h.engine.insert( + 'sys_user_position', { user_id: 'u_x', position: missingName }, { context: caller } as any, + )); + const env = envelopeOf(err); + expect(env.code, `caller ${caller.userId}`).toBe('PERMISSION_DENIED'); + expect(env.status, `caller ${caller.userId}`).toBe(403); + // The identical write naming a REAL position is refused identically — + // the answer does not depend on the catalog. + const same = envelopeOf(await refusalOf(() => h.engine.insert( + 'sys_user_position', { user_id: 'u_x', position: 'qa_auditor' }, { context: caller } as any, + ))); + expect([same.code, same.status]).toEqual([env.code, env.status]); + } + }); +}); + +// --------------------------------------------------------------------------- +// A walled, two-organization posture +// --------------------------------------------------------------------------- + +describe("walled posture, two organizations — the predicate reads the WRITER's catalog", () => { + async function bootTwoOrgs() { + const h = await boot({ walled: true }); + await h.engine.insert('sys_position', + { id: 'pos_b_only', name: 'qa_b_only', label: 'B only', active: true }, + { context: { isSystem: true, tenantId: 'org_b' } } as any); + 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); + return h; + } + const ORG_A_ADMIN = { ...ADMIN, tenantId: 'org_a' }; + + it('a name that NO organization carries is refused from inside an organization', async () => { + const h = await bootTwoOrgs(); + const env = envelopeOf(await refusalOf(() => h.engine.insert( + 'sys_user_position', { user_id: 'u_wa', position: 'nope_position' }, { context: ORG_A_ADMIN } as any, + ))); + expect([env.code, env.status]).toEqual(['VALIDATION_FAILED', 400]); + 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". + 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, + ))); + expect([foreign.code, foreign.status]).toEqual(['VALIDATION_FAILED', 400]); + expect(foreign.fields).toHaveLength(1); + expect(foreign.fields[0]).toMatchObject({ + field: 'position', + code: 'reference_not_found', + value: 'qa_b_only', + constraint: { target: 'sys_position', targetField: 'name' }, + }); + // Byte-identical to the "exists nowhere" message for that value … + expect(foreign.fields[0].message).toBe(positionNotInCatalogMessage('qa_b_only')); + // … and the same envelope, key for key, as a name nobody carries. + const nowhere = envelopeOf(await refusalOf(() => h.engine.insert( + 'sys_user_position', { user_id: 'u_wb', position: 'nope_position' }, { context: ORG_A_ADMIN } as any, + ))); + const shape = (e: typeof foreign) => [e.code, e.status, e.fields.map((f) => [f.field, f.code, Object.keys(f).sort(), f.constraint])]; + expect(shape(foreign)).toEqual(shape(nowhere)); + expect(await assignmentsOf(h, 'u_wb')).toHaveLength(0); + + // A predicate update that sets it reads the same catalog, and is refused the same way. + await h.engine.insert('sys_user_position', { id: 'upw', user_id: 'u_wm', position: 'qa_a_own' }, { context: ORG_A_ADMIN } as any); + const multi = envelopeOf(await refusalOf(() => h.engine.update( + 'sys_user_position', { position: 'qa_b_only' }, { where: { user_id: 'u_wm' }, multi: true, context: ORG_A_ADMIN } as any, + ))); + expect([multi.code, multi.status]).toEqual(['VALIDATION_FAILED', 400]); + expect(multi.fields[0]).toMatchObject({ field: 'position', code: 'reference_not_found', value: 'qa_b_only' }); + expect(multi.fields[0].message).toBe(positionNotInCatalogMessage('qa_b_only')); + 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 () => { + const h = await bootTwoOrgs(); + const created = await h.engine.insert( + 'sys_user_position', { user_id: 'u_wo', position: 'qa_a_own' }, { context: ORG_A_ADMIN } as any, + ); + 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. + 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, ORG_A_ADMIN)).toEqual(['qa_b_only', 'nope_position']); + }); + + it("an update by id of ANOTHER organization's row answers exactly like an id that exists nowhere", async () => { + // The by-id pre-image read stays a bare system read; this is why that is + // safe: the security middleware refuses both ids, identically, before the + // catalog refusal runs, so neither the pre-image nor the catalog verdict + // is reachable for a row outside the writer's organization. + const h = await bootTwoOrgs(); + await h.engine.insert('sys_user_position', + { id: 'upb_foreign', user_id: 'u_bx', position: 'qa_b_only' }, + { context: { isSystem: true, tenantId: 'org_b' } } as any); + for (const position of ['nope_position', 'qa_a_own']) { + const foreignErr = await refusalOf(() => h.engine.update( + 'sys_user_position', { id: 'upb_foreign', position }, { context: ORG_A_ADMIN } as any, + )); + const nowhereErr = await refusalOf(() => h.engine.update( + 'sys_user_position', { id: 'up_nowhere', position }, { context: ORG_A_ADMIN } as any, + )); + const foreign = envelopeOf(foreignErr); + const nowhere = envelopeOf(nowhereErr); + expect([foreign.code, foreign.status], position).toEqual(['PERMISSION_DENIED', 403]); + expect([nowhere.code, nowhere.status], position).toEqual(['PERMISSION_DENIED', 403]); + expect(resolveThrownHttpError(foreignErr).message, position).toBe(resolveThrownHttpError(nowhereErr).message); + } + const [row] = await h.engine.find('sys_user_position', { where: { id: 'upb_foreign' }, context: SYS }); + expect(row?.position).toBe('qa_b_only'); + }); + + it('the org boundary holds for a VALID foreign organization — 403 at the wall, whatever the name', async () => { + // The leg the card carried as NOT MEASURED on a single-org posture: an + // organization_a writer stamping organization_b on the row. The tenant wall + // refuses it before the catalog is consulted, identically for a name the + // catalog carries, a name only organization_b carries, and a name nobody + // carries — so the catalog verdict leaks nothing across the wall. + const h = await bootTwoOrgs(); + for (const position of ['qa_auditor', 'qa_b_only', 'nope_position']) { + const env = envelopeOf(await refusalOf(() => h.engine.insert( + 'sys_user_position', { user_id: 'u_wf', position, organization_id: 'org_b' }, { context: ORG_A_ADMIN } as any, + ))); + expect([env.code, env.status], position).toEqual(['PERMISSION_DENIED', 403]); + } + expect(await assignmentsOf(h, 'u_wf')).toHaveLength(0); + }); + + it("the id hint never names another organization's position", async () => { + const h = await bootTwoOrgs(); + const env = envelopeOf(await refusalOf(() => h.engine.insert( + 'sys_user_position', { user_id: 'u_wc', position: 'pos_b_only' }, { context: ORG_A_ADMIN } as any, + ))); + expect([env.code, env.status]).toEqual(['VALIDATION_FAILED', 400]); + 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. + 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, + ))); + expect([own.code, own.status]).toEqual(['VALIDATION_FAILED', 400]); + expect(own.fields[0]).toMatchObject({ field: 'position', code: 'reference_not_found' }); + expect(own.fields[0].message).toBe(positionNotInCatalogMessage('pos_a_own', 'qa_a_own')); + }); +}); + diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.ts new file mode 100644 index 00000000000..92f5addcf1c --- /dev/null +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.ts @@ -0,0 +1,509 @@ +// 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. + * + * ## 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 + * `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" + * + * 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. + * + * ## Whose catalog — the writer's organization plus organization-less rows + * + * 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 }`: + * + * - 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`). + * + * 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. + * + * ## Which writes it judges + * + * - a non-system INSERT, one row or a batch — every row's `position`, + * whatever its JSON type (see the stand-downs below for the string form it + * is judged by); + * - a non-system UPDATE whose payload carries `position`: by id, only when the + * value's string form differs from the string form of the value the row + * already stores (a form that echoes an unchanged value back — `123` over a + * stored `'123'` included — is not writing a new name); by predicate + * (`multi: true`), always, because the value lands on every matched row. + * + * It stands down, deliberately, on: + * + * - every `isSystem` write — the stand-down the engine's own lookup refusal and + * this plugin's whole security middleware take. That covers the seed loader, + * which on a fresh `single`-posture boot writes `stack.data` from + * `AppPlugin.start()`, BEFORE `kernel:ready` seeds the declared catalog: a + * refusal there would turn every authored assignment seed into a failed boot + * that succeeds on the second one. It also covers invitation acceptance and + * the platform's own bootstraps; + * - a value the engine answers itself, and only those: `null` or a blank + * string (`required`); one whose `String()` form is longer than the column + * (`max_length`); and an operator object, a plain object carrying a declared + * filter operator as an own key such as `{ $in: [...] }` (`invalid_type`, + * #5922, mirrored from the engine's own predicate). The engine's `text` + * validation refuses no other number, boolean, object or array, and the + * 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; + * - an update the engine refuses on its own dispatch predicate. + * + * ## Where it runs + * + * Registered by `SecurityPlugin` AFTER its security middleware, so it runs + * 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. + * + * 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 + * the writer's update of that id. An update naming another organization's row + * id and one naming an id that exists nowhere are both refused there, `403 + * PERMISSION_DENIED` with one identical answer, before this refusal runs — + * measured on a two-organization walled posture and pinned beside it. + * + * ## The envelope + * + * `400 VALIDATION_FAILED` with one `fields[]` entry per offending value, at + * `field: 'position'`, `code: 'reference_not_found'` — the envelope the sibling + * lookup columns on this very row already answer with, so an author meets one + * dialect for "this reference names nothing". No new error code: the top-level + * 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). + * + * ## 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. + */ + +import { resolveEngineUpdateDispatch, type EngineUpdateDispatchData } from '@objectstack/metadata-core'; +import type { FieldErrorCode } from '@objectstack/spec/api'; +import { + ALL_OPERATORS, + RETIRED_FILTER_OPERATORS, + classifyFilterToken, + isPlainRecord, +} from '@objectstack/spec/data'; +import { validationFailure } from '@objectstack/types'; +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. */ +export const POSITION_CATALOG_OBJECT = 'sys_position'; +/** The column judged, on {@link POSITION_ASSIGNMENT_OBJECT}. */ +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}. + */ +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"). + */ +function catalogReadContext(context: unknown): Record { + const own = context && typeof context === 'object' ? (context as Record) : {}; + return { ...own, isSystem: true }; +} + +/** + * One `fields[]` entry of the refusal. `code` is typed to the closed ADR-0114 + * field-level catalog, so the literal written below is a member of it by + * construction — the field-addressed vocabulary, one level below `error.code`. + */ +interface PositionFieldError { + field: string; + code: FieldErrorCode; + message: string; + label: string; + value: string; + constraint: { target: string; targetField: string }; +} + +const positionDef = (SysUserPosition as any).fields?.[POSITION_FIELD] ?? {}; +/** The column's declared label — what the message names it by. */ +const POSITION_LABEL: string = typeof positionDef.label === 'string' ? positionDef.label : 'Position'; +/** The column's declared bound; a longer value is the engine's `max_length` to answer. */ +const POSITION_MAX_LENGTH: number = typeof positionDef.maxLength === 'number' ? positionDef.maxLength : 100; + +export interface PositionCatalogRefusalDeps { + /** ObjectQL engine handle (elevated catalog and pre-image reads). */ + ql: any; + logger?: { warn?: (msg: string, meta?: any) => void }; +} + +function rowsOf(data: unknown): any[] { + if (Array.isArray(data)) return data; + if (data && typeof data === 'object') return [data]; + return []; +} + +/** + * The string form a `position` value is judged by: a string as itself, a + * number, bigint or boolean as `String(value)`, an object or array as its JSON + * text (`{}`, `["x"]`) — never `String(['x'])`, which reads `'x'` and would + * accept an array naming a real position that then resolves nothing. It is not + * the stored text, which is the driver's (SQLite stores `123` as `'123.0'`). + * `undefined` only for `null`, `undefined`, a symbol, a function, and an object + * that can be neither serialised nor stringified; a bigint is `String(value)`. + */ +function stringForm(value: unknown): string | undefined { + switch (typeof value) { + case 'string': + return value; + case 'number': + case 'bigint': + case 'boolean': + return String(value); + case 'object': { + if (value === null) return undefined; + try { + const json = JSON.stringify(value); + if (typeof json === 'string') return json; + } catch { + // not JSON-serialisable: fall back to the engine's own reading below + } + try { + return String(value); + } catch { + return undefined; + } + } + default: + return undefined; + } +} + +/** + * [#5922] The filter-operator keys the engine refuses as a `text` value: the + * spec's `ALL_OPERATORS` plus the retired ones, the same two inputs as + * `FILTER_OPERATOR_KEYS` in `@objectstack/objectql`'s `record-validator.ts`. + */ +const FILTER_OPERATOR_KEYS: ReadonlySet = new Set([ + ...ALL_OPERATORS, + ...Object.keys(RETIRED_FILTER_OPERATORS), +]); + +/** + * [#5922] Is this an operator object — a `where` node pasted into the write — + * which the engine itself refuses on a `text` field (`invalid_type`)? A narrow + * mirror of `filterOperatorKeysIn` in `record-validator.ts`, module-private + * there: the spec's `isPlainRecord` test, no `Date`, and at least one own key in + * {@link FILTER_OPERATOR_KEYS}. ⛔ Not a `$`-prefix test: `{ $foo: 1 }` carries + * no declared operator, the engine stores it, and it is judged here. + */ +function isFilterOperatorObject(value: unknown): boolean { + if (!isPlainRecord(value) || value instanceof Date) return false; + return Object.keys(value).some((key) => FILTER_OPERATOR_KEYS.has(key)); +} + +/** + * The name this refusal judges a `position` value by, or `undefined` for a + * value the engine answers itself: `null` and a blank string (`required`), a + * value whose `String()` form is longer than the column (`max_length`, which + * the engine reads on `String(value)` for every type), and an operator object + * (`invalid_type`, #5922). Every other value is judged, strings or not (module + * note, "Which writes it judges"). + */ +function judgedName(value: unknown): string | undefined { + if (value === null || value === undefined) return undefined; + if (typeof value === 'string' && value.trim() === '') return undefined; + if (isFilterOperatorObject(value)) return undefined; + let engineForm: string; + try { + engineForm = String(value); + } catch { + return undefined; + } + if (engineForm.length > POSITION_MAX_LENGTH) return undefined; + return stringForm(value); +} + +/** + * The `position` values this write would STORE as new names, in first-seen + * order and without repeats. Empty when the write names none this refusal + * judges (see the module note for which writes those are). + */ +export async function writtenPositionNames(ql: any, opCtx: any): Promise { + const out: string[] = []; + const add = (value: unknown) => { + const name = judgedName(value); + if (name !== undefined && !out.includes(name)) out.push(name); + }; + + if (opCtx?.operation === 'insert') { + for (const row of rowsOf(opCtx.data)) add(row?.[POSITION_FIELD]); + return out; + } + if (opCtx?.operation !== 'update') return out; + + const data = opCtx.data; + if (!data || typeof data !== 'object' || Array.isArray(data)) return out; + if (!Object.prototype.hasOwnProperty.call(data, POSITION_FIELD)) return out; + const next = (data as Record)[POSITION_FIELD]; + const nextName = judgedName(next); + if (nextName === undefined) return out; + + let route: ReturnType; + try { + route = resolveEngineUpdateDispatch(data as EngineUpdateDispatchData, opCtx.options); + } catch { + return out; // the engine answers a malformed call itself + } + if (route.kind === 'multi') { + add(next); + return out; + } + if (route.kind !== 'by-id') return out; // the engine refuses it on its own predicate + + // By id: a new name only when it differs from the stored one. A missing row + // is the engine's to answer (not found), never a catalog verdict. + let prev: any = null; + try { + prev = await ql?.findOne?.(POSITION_ASSIGNMENT_OBJECT, { where: { id: route.id }, context: SYSTEM_CTX }); + } catch { + prev = null; + } + if (!prev) return out; + // Compared by string form, so `123` echoed over a stored `'123'` is unchanged. + if (stringForm(prev[POSITION_FIELD]) === nextName) return out; + add(next); + return out; +} + +/** + * The names among `names` that no `sys_position` row the WRITER's catalog + * holds carries — the writer's organization plus organization-less rows, or + * every organization for a context naming none ({@link catalogReadContext}) — + * or `null` when the catalog could not be read and nothing may be concluded. + * `context` is the writer's execution context, required so no caller can + * silently fall back to an unscoped read. + * + * One bounded read per distinct name, never one `$in` read under a row limit: + * a context naming no organization sees every organization's copy of a name, + * so a limited `$in` page can fill with copies of one name and report the + * others missing. + */ +export async function namesWithoutCatalogRow( + deps: PositionCatalogRefusalDeps, + names: readonly string[], + context: unknown, +): 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 readCtx = catalogReadContext(context); + const missing: string[] = []; + for (const name of names) { + let carried: boolean; + try { + carried = await catalogCarries(ql, name, readCtx); + } 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`, + { object: POSITION_ASSIGNMENT_OBJECT, error: (e as Error)?.message ?? String(e) }, + ); + return null; + } + if (!carried) missing.push(name); + } + return missing; +} + +/** + * 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. + */ +export async function idSpellingHints( + deps: PositionCatalogRefusalDeps, + values: readonly string[], + context: unknown, +): Promise> { + const hints = new Map(); + const { ql } = deps; + if (!ql || typeof ql.find !== 'function') 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). + 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; + hints.set(value, name); + } catch { + // No hint — the refusal still stands and still names the column's contract. + } + } + return hints; +} + +/** The sentence an author reads — names the value, the column's contract and the fix. */ +export function positionNotInCatalogMessage(value: string, idOfPosition?: string): 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}'.` + ); + } + 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.` + ); +} + +/** + * The refusal: `VALIDATION_FAILED` (400 at both HTTP doors) with one + * `reference_not_found` entry per offending value at `field: 'position'`. + */ +export function positionNotInCatalogError( + values: readonly string[], + hints: ReadonlyMap = new Map(), +): Error { + const fields = values.map((value): PositionFieldError => ({ + field: POSITION_FIELD, + code: 'reference_not_found', + message: positionNotInCatalogMessage(value, hints.get(value)), + label: POSITION_LABEL, + value, + constraint: { target: POSITION_CATALOG_OBJECT, targetField: 'name' }, + })); + return validationFailure(fields.map((f) => f.message).join('; '), fields); +} + +/** + * Refuse the write when it would store a `position` no catalog row carries. + * Resolves (does nothing) for every write outside the judged set. + */ +export async function assertPositionNamesCatalogRow( + deps: PositionCatalogRefusalDeps, + opCtx: any, +): Promise { + if (opCtx?.object !== POSITION_ASSIGNMENT_OBJECT) return; + if (opCtx?.context?.isSystem) return; + const names = await writtenPositionNames(deps.ql, opCtx); + if (names.length === 0) return; + const missing = await namesWithoutCatalogRow(deps, names, opCtx.context); + if (!missing || missing.length === 0) return; + const hints = await idSpellingHints(deps, missing, opCtx.context); + throw positionNotInCatalogError(missing, hints); +} + +/** + * The engine middleware `SecurityPlugin` registers on + * {@link POSITION_ASSIGNMENT_OBJECT}, after its security middleware. + */ +export function createPositionCatalogRefusal( + deps: PositionCatalogRefusalDeps, +): (opCtx: any, next: () => Promise) => Promise { + return async (opCtx, next) => { + await assertPositionNamesCatalogRow(deps, opCtx); + await next(); + }; +} diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index b0869bf98d2..a57b30ca417 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -27,6 +27,7 @@ import { INVITATION_PLACEMENT_SERVICE, createInvitationPlacementService, } from './invitation-placement.js'; +import { POSITION_ASSIGNMENT_OBJECT, createPositionCatalogRefusal } from './position-catalog-refusal.js'; import { explainAccess, buildContextForUser, @@ -3771,6 +3772,19 @@ export class SecurityPlugin implements Plugin { ctx.logger.info('Security middleware registered on ObjectQL engine'); + // [ADR-0057 D4 / ADR-0112] A `sys_user_position` write whose `position` + // names no `sys_position` catalog row is refused (`400 VALIDATION_FAILED`, + // `reference_not_found` at `position`) instead of answering 201 over an + // assignment that resolves to nothing. Registered AFTER the security + // middleware, so it runs INSIDE it: the delegated-admin gate and the CRUD + // check have both passed before the catalog is consulted, and a caller who + // may not write the table is refused on authority, never on the value. + // Scope, predicate and stand-downs: `position-catalog-refusal.ts`. + ql.registerMiddleware( + createPositionCatalogRefusal({ ql, logger: ctx.logger }), + { object: POSITION_ASSIGNMENT_OBJECT }, + ); + // [ADR-0094] Data-door write-through: every non-system CRUD write on // `sys_permission_set` is redirected into the metadata store (the ONE // authoritative store for definitions); the record is projector-owned.