From 166d5f1c705fb8ed6c8e32e0e62dc58c2d45c87c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 17:49:00 +0000 Subject: [PATCH 01/13] wip(plugin-security): refuse a sys_user_position write whose position names no sys_position row Claude-Session: https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ Co-authored-by: Claude --- .../src/position-catalog-refusal.ts | 323 ++++++++++++++++++ .../plugin-security/src/security-plugin.ts | 14 + 2 files changed, 337 insertions(+) create mode 100644 packages/plugins/plugin-security/src/position-catalog-refusal.ts 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..d3dab21f16b --- /dev/null +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.ts @@ -0,0 +1,323 @@ +// 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 (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; + * - the catalog is read WITHOUT a tenant scope ({@link SYSTEM_CTX}): a name + * that some `sys_position` row carries, in any organization, is accepted. + * Under a `single` posture every catalog row is organization-less and the + * two readings are the same set. Under a walled posture they differ for a + * name only ANOTHER organization's catalog carries — accepted here, and it + * resolves nothing in the writer's organization. That narrower reading is a + * question for the ruling, not a choice this module makes on its own. + * + * ## Which writes it judges + * + * - a non-system INSERT, one row or a batch — every row's `position`; + * - a non-system UPDATE whose payload carries `position`: by id, only when the + * value differs from the one the row already stores (a form that echoes an + * unchanged value back 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: not a string, empty, or longer than the + * column (`required` / `invalid_type` / `max_length`, one condition, one + * code); + * - 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 on authority and never sees the catalog verdict, so the + * verdict is not an existence probe for them — which matters because the + * catalog read above is not tenant-scoped. + * + * ## 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 + * own organization can see. + * + * ## 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. + */ + +import { resolveEngineUpdateDispatch, type EngineUpdateDispatchData } from '@objectstack/metadata-core'; +import type { FieldErrorCode } from '@objectstack/spec/api'; +import { validationFailure } from '@objectstack/types'; +import { SysUserPosition } from './objects/sys-user-position.object.js'; +import { rowOrganizationId } from './per-organization-catalog.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'; + +/** Existence is a fact about the database, so the catalog is read elevated and unscoped. */ +const SYSTEM_CTX = { isSystem: true } as const; + +/** The field-level code the sibling lookup columns already answer with. */ +const REFERENCE_NOT_FOUND: FieldErrorCode = 'reference_not_found'; + +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 (system-context catalog and pre-image reads). */ + ql: any; + logger?: { warn?: (msg: string, meta?: any) => void }; +} + +/** The organization the WRITER acts in — the one whose catalog an id hint may name. */ +function callerOrganizationId(context: any): string | undefined { + const id = context?.organizationId ?? context?.tenantId; + return typeof id === 'string' && id !== '' ? id : undefined; +} + +function rowsOf(data: unknown): any[] { + if (Array.isArray(data)) return data; + if (data && typeof data === 'object') return [data]; + return []; +} + +/** A value this refusal judges: a non-blank string within the column's bound. */ +function isJudgedValue(value: unknown): value is string { + return typeof value === 'string' && value.trim() !== '' && value.length <= POSITION_MAX_LENGTH; +} + +/** + * 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) => { + if (isJudgedValue(value) && !out.includes(value)) out.push(value); + }; + + 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]; + if (!isJudgedValue(next)) 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; + if (prev[POSITION_FIELD] === next) return out; + add(next); + return out; +} + +/** + * The names among `names` that NO `sys_position` row carries — or `null` when + * the catalog could not be read and nothing may be concluded. + * + * One bounded read per distinct name, never one `$in` read under a row limit: + * a walled posture seeds the same name once per organization, 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[], +): 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 missing: string[] = []; + for (const name of names) { + let rows: unknown; + try { + rows = await ql.find(POSITION_CATALOG_OBJECT, { where: { name }, limit: 1, context: SYSTEM_CTX }); + } 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 (!Array.isArray(rows) || rows.length === 0) missing.push(name); + } + return missing; +} + +/** + * For each missing value that is the record ID of a position the writer's own + * organization can see, that position's NAME — the exact fix. Scoped to the + * writer's organization (plus organization-less rows) so the hint never names + * another organization's position. A read that fails yields no hint. + */ +export async function idSpellingHints( + deps: PositionCatalogRefusalDeps, + values: readonly string[], + organizationId?: string, +): Promise> { + const hints = new Map(); + const { ql } = deps; + if (!ql || typeof ql.find !== 'function') return hints; + for (const value of values) { + try { + const rows = await ql.find(POSITION_CATALOG_OBJECT, { where: { id: value }, limit: 1, context: SYSTEM_CTX }); + const row = Array.isArray(rows) ? rows[0] : null; + const name = row && typeof row.name === 'string' && row.name !== '' ? row.name : null; + if (!name) continue; + const owner = rowOrganizationId(row); + if (organizationId && owner && owner !== organizationId) 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) => ({ + 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); + if (!missing || missing.length === 0) return; + const hints = await idSpellingHints(deps, missing, callerOrganizationId(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 844f8f7923c..12c1790692b 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, @@ -3770,6 +3771,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. From b226d338bc7cae181254116230d80f4587c06a63 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 17:58:57 +0000 Subject: [PATCH 02/13] test(plugin-security): pin the position catalog refusal on a real engine, with the ruling's control leg Claude-Session: https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ Co-authored-by: Claude --- .../src/position-catalog-refusal.test.ts | 461 ++++++++++++++++++ 1 file changed, 461 insertions(+) create mode 100644 packages/plugins/plugin-security/src/position-catalog-refusal.test.ts 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..710796a91d3 --- /dev/null +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts @@ -0,0 +1,461 @@ +// 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 { 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 }, + } as any); + 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 } as any); + 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); + }); +}); + +// --------------------------------------------------------------------------- +// 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' }); + }); +}); + +// --------------------------------------------------------------------------- +// 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 literal predicate, measured', () => { + 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 ACCEPTED — the ruled predicate is 'no row at all'", async () => { + // The two readings differ exactly here. An organization-scoped predicate + // would refuse this write (it resolves nothing in org_a); the ruled + // literal one accepts it. Pinned so a change of reading is a visible, + // deliberate edit — the question is recorded on the card. + const h = await bootTwoOrgs(); + const created = await h.engine.insert( + 'sys_user_position', { user_id: 'u_wb', position: 'qa_b_only' }, { context: ORG_A_ADMIN } as any, + ); + expect(created).toMatchObject({ position: '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.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.fields[0].message).toBe(positionNotInCatalogMessage('pos_a_own', 'qa_a_own')); + }); +}); + From a09391769047eee87137d1aa3a513211af9fea5c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 18:05:48 +0000 Subject: [PATCH 03/13] chore(changeset): plugin-security minor, BREAKING accept-set narrowing on sys_user_position.position Claude-Session: https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ Co-authored-by: Claude --- .changeset/16712-position-catalog-refusal.md | 54 ++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 .changeset/16712-position-catalog-refusal.md diff --git a/.changeset/16712-position-catalog-refusal.md b/.changeset/16712-position-catalog-refusal.md new file mode 100644 index 00000000000..96902c6c2c0 --- /dev/null +++ b/.changeset/16712-position-catalog-refusal.md @@ -0,0 +1,54 @@ +--- +'@objectstack/plugin-security': minor +--- + +fix(plugin-security)!: a `sys_user_position` write whose `position` names no `sys_position` catalog row is refused, instead of answering 201 over an assignment that resolves to nothing (#16712) + +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). + +**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. + +**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. +- The catalog is read across every organization: a name that ANY `sys_position` + row carries is accepted. On a single-organization posture that is the same as + the writer's own catalog. + +**Who is affected.** A client, script or AI author that writes a position's id, +a misspelled name, or a name not yet created. The fix is the one the refusal +names: write the position's `name`, or create the position first. From cbdd0e70b7ab740749cec6edb796789a1f875d9e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 18:23:27 +0000 Subject: [PATCH 04/13] fix(plugin-security): census the refusal's isSystem stand-down, stamp the field code where the casing gate owns it, type the test's query options Claude-Session: https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ Co-authored-by: Claude --- content/docs/permissions/system-context.mdx | 19 ++++++++++--------- .../src/position-catalog-refusal.test.ts | 4 ++-- .../src/position-catalog-refusal.ts | 19 +++++++++++++++---- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 656e0cc6320..8ec02b2eed9 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; every non-system insert, and every non-system update that changes `position`, is refused `400 VALIDATION_FAILED` / `reference_not_found` instead — the same stand-down the engine's referential-integrity check takes 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 | 87 | ✅ | +| Files containing at least one elevation read | 45 | ✅ | +| — the distinct symbols those reads live in — what this page anchors | 88 | ✅ | | — 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 index 710796a91d3..c98532a5583 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts @@ -191,7 +191,7 @@ async function rowsReadBy(h: Harness, userId: string): Promise { try { const rows = await h.engine.find('qa_inquiry', { context: { userId, positions: g.positions, permissions: g.permissions }, - } as any); + }); return Array.isArray(rows) ? rows.length : 0; } catch (e: any) { // An account holding nothing on the object is refused the read outright; @@ -217,7 +217,7 @@ function envelopeOf(e: unknown) { } async function assignmentsOf(h: Harness, userId: string): Promise { - const rows = await h.engine.find('sys_user_position', { where: { user_id: userId }, context: SYS } as any); + const rows = await h.engine.find('sys_user_position', { where: { user_id: userId }, context: SYS }); return Array.isArray(rows) ? rows : []; } diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.ts index d3dab21f16b..bfe123d631e 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.ts @@ -107,8 +107,19 @@ export const POSITION_FIELD = 'position'; /** Existence is a fact about the database, so the catalog is read elevated and unscoped. */ const SYSTEM_CTX = { isSystem: true } as const; -/** The field-level code the sibling lookup columns already answer with. */ -const REFERENCE_NOT_FOUND: FieldErrorCode = 'reference_not_found'; +/** + * 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. */ @@ -280,9 +291,9 @@ export function positionNotInCatalogError( values: readonly string[], hints: ReadonlyMap = new Map(), ): Error { - const fields = values.map((value) => ({ + const fields = values.map((value): PositionFieldError => ({ field: POSITION_FIELD, - code: REFERENCE_NOT_FOUND, + code: 'reference_not_found', message: positionNotInCatalogMessage(value, hints.get(value)), label: POSITION_LABEL, value, From 75804ad6a8d768cbd79b3f2c9a7eb3a9d73a8c47 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 20:34:37 +0000 Subject: [PATCH 05/13] docs(system-context): re-derive the census over the merged tree (symbols 88 -> 89) main moved a read into a new symbol while this branch added one; the text merge kept "88" from both sides. gen:system-context-census re-derives it. Claude-Session: https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ Co-authored-by: Claude --- content/docs/permissions/system-context.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index c55e1a3ecab..32e587d82fb 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -363,7 +363,7 @@ still holds equal to the census on every pull request: | — 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 | 45 | ✅ | -| — the distinct symbols those reads live in — what this page anchors | 88 | ✅ | +| — 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 From c8b533d2a2001c4a75b1e06beafb4ea04c401b47 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 20:45:13 +0000 Subject: [PATCH 06/13] fix(plugin-security): read the position catalog as the writer's organization plus organization-less rows The refusal's catalog reads (the name check and the id hint) now run under { ...context, isSystem: true }, the engine lookup probe's sudo()-shaped spelling, instead of a bare { isSystem: true } that spanned every organization. A name only another organization's catalog carries is refused with the envelope and message a name that exists nowhere gets. The walled pin that accepted it is reversed; the by-id pre-image read stays bare, pinned beside the measurement that the security middleware refuses a foreign row id and a nowhere id identically first. Claude-Session: https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ Co-authored-by: Claude --- .../src/position-catalog-refusal.test.ts | 108 +++++++++++++-- .../src/position-catalog-refusal.ts | 128 ++++++++++++------ 2 files changed, 186 insertions(+), 50 deletions(-) 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 c98532a5583..b194bc7e7b9 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts @@ -36,7 +36,7 @@ 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 { positionNotInCatalogMessage } from './position-catalog-refusal.js'; +import { namesWithoutCatalogRow, positionNotInCatalogMessage } from './position-catalog-refusal.js'; // --------------------------------------------------------------------------- // Fixture @@ -294,6 +294,23 @@ describe('the accepted half — a catalog NAME, active or deactivated', () => { // 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 the organization-less catalog', async () => { + // Every `single`-posture catalog row carries no organization. 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' }); + }); }); // --------------------------------------------------------------------------- @@ -394,7 +411,7 @@ describe('scope — the stand-downs are declared, not accidental', () => { // A walled, two-organization posture // --------------------------------------------------------------------------- -describe('walled posture, two organizations — the literal predicate, measured', () => { +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', @@ -416,16 +433,89 @@ describe('walled posture, two organizations — the literal predicate, measured' expect(env.fields[0]).toMatchObject({ field: 'position', code: 'reference_not_found', value: 'nope_position' }); }); - it("a name only ANOTHER organization's catalog carries is ACCEPTED — the ruled predicate is 'no row at all'", async () => { - // The two readings differ exactly here. An organization-scoped predicate - // would refuse this write (it resolves nothing in org_a); the ruled - // literal one accepts it. Pinned so a change of reading is a visible, - // deliberate edit — the question is recorded on the card. + 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 created = await h.engine.insert( + 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_b_only' }); + 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 } as any); + 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 () => { diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.ts index bfe123d631e..9d2fe6496a6 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.ts @@ -23,20 +23,39 @@ * * ## The predicate — exactly "no catalog row carries this name" * - * Ruled on the card that filed the hole (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: + * 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. * - * - a DEACTIVATED position (`active: false`) is still a catalog row, and an - * assignment naming it is accepted — it stops granting, it is not refused; - * - the catalog is read WITHOUT a tenant scope ({@link SYSTEM_CTX}): a name - * that some `sys_position` row carries, in any organization, is accepted. - * Under a `single` posture every catalog row is organization-less and the - * two readings are the same set. Under a walled posture they differ for a - * name only ANOTHER organization's catalog carries — accepted here, and it - * resolves nothing in the writer's organization. That narrower reading is a - * question for the ruling, not a choice this module makes on its own. + * ## 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 every catalog row is organization-less and + * every writer reads the whole catalog. A writer whose context names no + * organization reads every organization's rows, as the engine probe does. * * ## Which writes it judges * @@ -65,9 +84,16 @@ * 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 on authority and never sees the catalog verdict, so the - * verdict is not an existence probe for them — which matters because the - * catalog read above is not tenant-scoped. + * 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 * @@ -80,7 +106,8 @@ * 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 - * own organization can see. + * catalog holds (read the same way as the check, so never another + * organization's). * * ## Fails open * @@ -95,7 +122,6 @@ import { resolveEngineUpdateDispatch, type EngineUpdateDispatchData } from '@obj import type { FieldErrorCode } from '@objectstack/spec/api'; import { validationFailure } from '@objectstack/types'; import { SysUserPosition } from './objects/sys-user-position.object.js'; -import { rowOrganizationId } from './per-organization-catalog.js'; /** The assignment object this refusal is registered on. */ export const POSITION_ASSIGNMENT_OBJECT = 'sys_user_position'; @@ -104,9 +130,28 @@ export const POSITION_CATALOG_OBJECT = 'sys_position'; /** The column judged, on {@link POSITION_ASSIGNMENT_OBJECT}. */ export const POSITION_FIELD = 'position'; -/** Existence is a fact about the database, so the catalog is read elevated and unscoped. */ +/** + * 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 @@ -128,17 +173,11 @@ const POSITION_LABEL: string = typeof positionDef.label === 'string' ? positionD const POSITION_MAX_LENGTH: number = typeof positionDef.maxLength === 'number' ? positionDef.maxLength : 100; export interface PositionCatalogRefusalDeps { - /** ObjectQL engine handle (system-context catalog and pre-image reads). */ + /** ObjectQL engine handle (elevated catalog and pre-image reads). */ ql: any; logger?: { warn?: (msg: string, meta?: any) => void }; } -/** The organization the WRITER acts in — the one whose catalog an id hint may name. */ -function callerOrganizationId(context: any): string | undefined { - const id = context?.organizationId ?? context?.tenantId; - return typeof id === 'string' && id !== '' ? id : undefined; -} - function rowsOf(data: unknown): any[] { if (Array.isArray(data)) return data; if (data && typeof data === 'object') return [data]; @@ -200,16 +239,22 @@ export async function writtenPositionNames(ql: any, opCtx: any): Promise { const { ql, logger } = deps; if (names.length === 0) return []; @@ -218,11 +263,12 @@ export async function namesWithoutCatalogRow( // 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 rows: unknown; try { - rows = await ql.find(POSITION_CATALOG_OBJECT, { where: { name }, limit: 1, context: SYSTEM_CTX }); + rows = await ql.find(POSITION_CATALOG_OBJECT, { where: { name }, limit: 1, context: readCtx }); } catch (e) { logger?.warn?.( `[security] the ${POSITION_CATALOG_OBJECT} catalog could not be read, so a ` + @@ -238,27 +284,27 @@ export async function namesWithoutCatalogRow( } /** - * For each missing value that is the record ID of a position the writer's own - * organization can see, that position's NAME — the exact fix. Scoped to the - * writer's organization (plus organization-less rows) so the hint never names - * another organization's position. A read that fails yields no hint. + * 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[], - organizationId?: 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) { try { - const rows = await ql.find(POSITION_CATALOG_OBJECT, { where: { id: value }, limit: 1, context: SYSTEM_CTX }); + 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; - const owner = rowOrganizationId(row); - if (organizationId && owner && owner !== organizationId) continue; hints.set(value, name); } catch { // No hint — the refusal still stands and still names the column's contract. @@ -314,9 +360,9 @@ export async function assertPositionNamesCatalogRow( if (opCtx?.context?.isSystem) return; const names = await writtenPositionNames(deps.ql, opCtx); if (names.length === 0) return; - const missing = await namesWithoutCatalogRow(deps, names); + const missing = await namesWithoutCatalogRow(deps, names, opCtx.context); if (!missing || missing.length === 0) return; - const hints = await idSpellingHints(deps, missing, callerOrganizationId(opCtx.context)); + const hints = await idSpellingHints(deps, missing, opCtx.context); throw positionNotInCatalogError(missing, hints); } From 418b4411cc2e4c5f04b0fa8ad27e57b9fee03173 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 20:48:32 +0000 Subject: [PATCH 07/13] docs(plugin-security): the changeset and system-context row 23b state the writer's catalog The catalog is the writer's organization's positions plus the organization-less ones; a name only another organization carries is refused like any unknown name. The "read across every organization" line is gone. Claude-Session: https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ Co-authored-by: Claude --- .changeset/16712-position-catalog-refusal.md | 26 ++++++++++++++------ content/docs/permissions/system-context.mdx | 2 +- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/.changeset/16712-position-catalog-refusal.md b/.changeset/16712-position-catalog-refusal.md index 96902c6c2c0..7776412e121 100644 --- a/.changeset/16712-position-catalog-refusal.md +++ b/.changeset/16712-position-catalog-refusal.md @@ -2,16 +2,18 @@ '@objectstack/plugin-security': minor --- -fix(plugin-security)!: a `sys_user_position` write whose `position` names no `sys_position` catalog row is refused, instead of answering 201 over an assignment that resolves to nothing (#16712) +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). +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 @@ -34,6 +36,15 @@ 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 posture every position is +organization-less, 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 @@ -45,10 +56,9 @@ sees the catalog verdict. 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. -- The catalog is read across every organization: a name that ANY `sys_position` - row carries is accepted. On a single-organization posture that is the same as - the writer's own catalog. **Who is affected.** A client, script or AI author that writes a position's id, -a misspelled name, or a name not yet created. The fix is the one the refusal -names: write the position's `name`, or create the position first. +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 32e587d82fb..aaaabcc5690 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -125,7 +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; every non-system insert, and every non-system update that changes `position`, is refused `400 VALIDATION_FAILED` / `reference_not_found` instead — the same stand-down the engine's referential-integrity check takes 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 `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` | From cb1d5be9d86e8c8c208db9f3cbdb17c66afa2b8e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 21:02:22 +0000 Subject: [PATCH 08/13] test(plugin-security): type the pre-image pin's read options The new find in the foreign-row-id pin cast its options to any, growing the query-options-erasure test surface 236 -> 237. The options are on-contract, so they are typed, as assignmentsOf already types the same read. Claude-Session: https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ Co-authored-by: Claude --- .../plugin-security/src/position-catalog-refusal.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b194bc7e7b9..c113ba5ea43 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts @@ -514,7 +514,7 @@ describe("walled posture, two organizations — the predicate reads the WRITER's 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 } as any); + const [row] = await h.engine.find('sys_user_position', { where: { id: 'upb_foreign' }, context: SYS }); expect(row?.position).toBe('qa_b_only'); }); From 0f4bf26e93228a7726a09a89b00b939057ae67f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 22:07:15 +0000 Subject: [PATCH 09/13] docs(plugin-security): state the single-posture catalog premise the code supports The changeset, the module docblock and one test's name and comment said every catalog row on a single posture is organization-less. The declared catalog is seeded without an organization, but a position created through the data door by a session with an active organization is stamped with it. The conclusion stands (one organization: the scoped read covers the whole catalog); the premise is reworded. No behaviour or assertion change. Claude-Session: https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ Co-authored-by: Claude --- .changeset/16712-position-catalog-refusal.md | 5 +++-- .../plugin-security/src/position-catalog-refusal.test.ts | 9 +++++---- .../plugin-security/src/position-catalog-refusal.ts | 9 ++++++--- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/.changeset/16712-position-catalog-refusal.md b/.changeset/16712-position-catalog-refusal.md index 7776412e121..d0c49a9f57f 100644 --- a/.changeset/16712-position-catalog-refusal.md +++ b/.changeset/16712-position-catalog-refusal.md @@ -41,8 +41,9 @@ 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 posture every position is -organization-less, so every writer sees the whole catalog. A writer whose +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.** 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 c113ba5ea43..b77e9c5e6d5 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts @@ -295,10 +295,11 @@ describe('the accepted half — a catalog NAME, active or deactivated', () => { expect(await rowsReadBy(h, 'u_retired')).toBe(0); }); - it('single posture: an organization-bound writer still reads the organization-less catalog', async () => { - // Every `single`-posture catalog row carries no organization. The scoped - // read (`{ ...context, isSystem: true }`) forwards the writer's tenant, and - // the driver's `organization_id IS NULL` term keeps those rows visible. + 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( diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.ts index 9d2fe6496a6..3800b3dfce0 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.ts @@ -53,9 +53,12 @@ * 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 every catalog row is organization-less and - * every writer reads the whole catalog. A writer whose context names no - * organization reads every organization's rows, as the engine probe does. + * against. Under a `single` posture the declared catalog is seeded with no + * organization, and a position a session creates through the data door is + * stamped with the session's active organization — the deployment's one + * organization — or with none; either way every writer reads the whole + * catalog. A writer whose context names no organization reads every + * organization's rows, as the engine probe does. * * ## Which writes it judges * From ebf00fd8f7fa14cd581a4754b76a34b9814c2067 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 22:30:13 +0000 Subject: [PATCH 10/13] docs(plugin-security): make the single-posture whole-catalog sentence conditional on one organization A single-posture deployment holding several organizations still boots (it is reported at error at boot); there each writer reads its active organization's positions plus the organization-less ones. The docblock now says so instead of assuming one organization. Prose only. Claude-Session: https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ Co-authored-by: Claude --- .../plugin-security/src/position-catalog-refusal.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.ts index 3800b3dfce0..02303607afd 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.ts @@ -54,11 +54,11 @@ * 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, and a position a session creates through the data door is - * stamped with the session's active organization — the deployment's one - * organization — or with none; either way every writer reads the whole - * catalog. A writer whose context names no organization reads every - * organization's rows, as the engine probe does. + * 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 * From 6e1ef6aa82b2ef7d6748e42a587a27c510da63a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 23:17:02 +0000 Subject: [PATCH 11/13] fix(plugin-security): judge a non-string position by the text it is stored as The refusal judged strings only, and its docblock credited the engine with an invalid_type refusal it does not make: a text column's validation reads String(value), and a number, boolean, object or array was stored with 201 (measured over SQLite). Such a value is now judged by its stored text, a scalar by String(value) and an object or array by its JSON, and refused reference_not_found like any name no catalog row carries. The by-id unchanged-value comparison reads stored text, so 123 echoed over a stored '123' stays unjudged. The id-hint pin now asserts the envelope code, status, field and field code. Claude-Session: https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ Co-authored-by: Claude --- .../src/position-catalog-refusal.test.ts | 55 ++++++++++++ .../src/position-catalog-refusal.ts | 85 ++++++++++++++++--- 2 files changed, 128 insertions(+), 12 deletions(-) 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 b77e9c5e6d5..0e1a5927795 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts @@ -373,6 +373,57 @@ describe('every non-system write that stores a new position name is judged', () }); }); +// --------------------------------------------------------------------------- +// A position that is not a string is judged by the text it is stored as +// --------------------------------------------------------------------------- + +describe('a non-string position is judged by the text it would be stored as', () => { + // The engine's `text` validation refuses none of these, and the write stores + // each one as text with 201, so they are this refusal's to judge. + + it('insert: a number, a boolean, an object and an array are refused 400, reference_not_found at their stored text', 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("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 // --------------------------------------------------------------------------- @@ -540,12 +591,16 @@ describe("walled posture, two organizations — the predicate reads the WRITER's 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 index 02303607afd..7e1aff345cf 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.ts @@ -62,10 +62,13 @@ * * ## Which writes it judges * - * - a non-system INSERT, one row or a batch — every row's `position`; + * - a non-system INSERT, one row or a batch — every row's `position`, + * whatever its JSON type (see the stand-downs below for the text it is + * judged as); * - a non-system UPDATE whose payload carries `position`: by id, only when the - * value differs from the one the row already stores (a form that echoes an - * unchanged value back is not writing a new name); by predicate + * value's text differs from the one 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: @@ -77,9 +80,14 @@ * 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: not a string, empty, or longer than the - * column (`required` / `invalid_type` / `max_length`, one condition, one - * code); + * - a value the engine answers itself: `null` or a blank string (`required`), + * or one whose `String()` form is longer than the column (`max_length`). + * Nothing else is the engine's: its `text` validation reads `String(value)` + * and refuses no number, boolean, object or array, and the write stores it + * as text with `201` (measured over SQLite: `123`, `true`, `{}` and `['x']` + * all stored). So those are judged by the text they are stored as — a + * scalar by `String(value)`, an object or array by its JSON — and refused + * like any name no catalog row carries; * - an update the engine refuses on its own dispatch predicate. * * ## Where it runs @@ -187,9 +195,59 @@ function rowsOf(data: unknown): any[] { return []; } -/** A value this refusal judges: a non-blank string within the column's bound. */ -function isJudgedValue(value: unknown): value is string { - return typeof value === 'string' && value.trim() !== '' && value.length <= POSITION_MAX_LENGTH; +/** + * The text a `position` value is stored as, and so the name it is judged by: a + * string as itself, a number, bigint or boolean as `String(value)`, an object + * or array as its JSON text — the form a `text` column stores it in (`{}`, + * `["x"]`), never `String(['x'])`, which reads `'x'` and would accept an array + * naming a real position that then resolves nothing. `undefined` for a value + * that is no JSON value at all. + */ +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; + } +} + +/** + * The name this refusal judges a `position` value by, or `undefined` for a + * value the engine answers itself: `null` and a blank string (`required`), and + * a value whose `String()` form is longer than the column (`max_length`, which + * the engine reads on `String(value)` for every type). 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; + let engineForm: string; + try { + engineForm = String(value); + } catch { + return undefined; + } + if (engineForm.length > POSITION_MAX_LENGTH) return undefined; + return stringForm(value); } /** @@ -200,7 +258,8 @@ function isJudgedValue(value: unknown): value is string { export async function writtenPositionNames(ql: any, opCtx: any): Promise { const out: string[] = []; const add = (value: unknown) => { - if (isJudgedValue(value) && !out.includes(value)) out.push(value); + const name = judgedName(value); + if (name !== undefined && !out.includes(name)) out.push(name); }; if (opCtx?.operation === 'insert') { @@ -213,7 +272,8 @@ export async function writtenPositionNames(ql: any, opCtx: any): Promise)[POSITION_FIELD]; - if (!isJudgedValue(next)) return out; + const nextName = judgedName(next); + if (nextName === undefined) return out; let route: ReturnType; try { @@ -236,7 +296,8 @@ export async function writtenPositionNames(ql: any, opCtx: any): Promise Date: Sun, 27 Sep 2026 23:40:58 +0000 Subject: [PATCH 12/13] docs(plugin-security): a non-string position is judged by its string form, not its stored text The docblock and two test names said a non-string is judged by the text it is stored as. The code judges String(value) for a scalar and the JSON text for an object or array; the stored text is the driver's (SQLite stores 123 as '123.0'). The prose now says string form and names that gap. No code or assertion change. Claude-Session: https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ Co-authored-by: Claude --- .../src/position-catalog-refusal.test.ts | 9 ++--- .../src/position-catalog-refusal.ts | 35 ++++++++++--------- 2 files changed, 24 insertions(+), 20 deletions(-) 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 0e1a5927795..e4a1435ab82 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts @@ -374,14 +374,15 @@ describe('every non-system write that stores a new position name is judged', () }); // --------------------------------------------------------------------------- -// A position that is not a string is judged by the text it is stored as +// A position that is not a string is judged by its string form // --------------------------------------------------------------------------- -describe('a non-string position is judged by the text it would be stored as', () => { +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 as text with 201, so they are this refusal's to judge. + // 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 stored text', async () => { + 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'], diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.ts index 7e1aff345cf..2e0b2642fb9 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.ts @@ -63,12 +63,12 @@ * ## 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 text it is - * judged as); + * 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 text differs from the one 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 + * 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: @@ -84,10 +84,13 @@ * or one whose `String()` form is longer than the column (`max_length`). * Nothing else is the engine's: its `text` validation reads `String(value)` * and refuses no number, boolean, object or array, and the write stores it - * as text with `201` (measured over SQLite: `123`, `true`, `{}` and `['x']` - * all stored). So those are judged by the text they are stored as — a - * scalar by `String(value)`, an object or array by its JSON — and refused - * like any name no catalog row carries; + * with `201` (measured over SQLite: `123`, `true`, `{}` 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 @@ -196,12 +199,12 @@ function rowsOf(data: unknown): any[] { } /** - * The text a `position` value is stored as, and so the name it is judged by: a - * string as itself, a number, bigint or boolean as `String(value)`, an object - * or array as its JSON text — the form a `text` column stores it in (`{}`, - * `["x"]`), never `String(['x'])`, which reads `'x'` and would accept an array - * naming a real position that then resolves nothing. `undefined` for a value - * that is no JSON value at all. + * 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` for a value that is no JSON value at all. */ function stringForm(value: unknown): string | undefined { switch (typeof value) { @@ -296,7 +299,7 @@ export async function writtenPositionNames(ql: any, opCtx: any): Promise Date: Mon, 28 Sep 2026 00:21:37 +0000 Subject: [PATCH 13/13] fix(plugin-security): leave an operator object to the engine, and look a placeholder-shaped name up literally Measured on 0e5f4c79: an operator object ({ $in: [...] }) already answered the engine's invalid_type, but only because the catalog read of its JSON text threw FILTER_TOKEN_UNKNOWN (a fully-wrapped {...} comparand is resolved as a filter placeholder) and the refusal failed open. The same fail-open let { a: 1 }, { $foo: 1 } and '{nope_tok}' be stored unchecked with 201. judgedName now stands down on an operator object, mirroring the engine's #5922 predicate from the same spec inputs (isPlainRecord, ALL_OPERATORS, the retired operators), so invalid_type stays the one answer. A name that classifyFilterToken reads as a placeholder is compared literally against the catalog names sharing its first character, so it is judged, never resolved or failed open; the id hint skips such names. Claude-Session: https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ Co-authored-by: Claude --- .../src/position-catalog-refusal.test.ts | 46 ++++++++ .../src/position-catalog-refusal.ts | 102 ++++++++++++++---- 2 files changed, 128 insertions(+), 20 deletions(-) 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 e4a1435ab82..f45666f9480 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts @@ -416,6 +416,52 @@ describe('a non-string position is judged by its string form', () => { 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. diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.ts index 2e0b2642fb9..92f5addcf1c 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.ts @@ -80,17 +80,19 @@ * 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: `null` or a blank string (`required`), - * or one whose `String()` form is longer than the column (`max_length`). - * Nothing else is the engine's: its `text` validation reads `String(value)` - * and refuses no number, boolean, object or array, and the write stores it - * with `201` (measured over SQLite: `123`, `true`, `{}` 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; + * - 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 @@ -129,11 +131,19 @@ * 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. + * 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'; @@ -204,7 +214,8 @@ function rowsOf(data: unknown): any[] { * 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` for a value that is no JSON value at all. + * `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) { @@ -233,16 +244,41 @@ function stringForm(value: unknown): string | 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`), and - * a value whose `String()` form is longer than the column (`max_length`, which - * the engine reads on `String(value)` for every type). Every other value is - * judged, strings or not (module note, "Which writes it judges"). + * 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); @@ -333,9 +369,9 @@ export async function namesWithoutCatalogRow( const readCtx = catalogReadContext(context); const missing: string[] = []; for (const name of names) { - let rows: unknown; + let carried: boolean; try { - rows = await ql.find(POSITION_CATALOG_OBJECT, { where: { name }, limit: 1, context: readCtx }); + carried = await catalogCarries(ql, name, readCtx); } catch (e) { logger?.warn?.( `[security] the ${POSITION_CATALOG_OBJECT} catalog could not be read, so a ` + @@ -345,11 +381,34 @@ export async function namesWithoutCatalogRow( ); return null; } - if (!Array.isArray(rows) || rows.length === 0) missing.push(name); + 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 @@ -367,6 +426,9 @@ export async function idSpellingHints( 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;