From eeafb7d4e4eebeac39bf8c2ef906b27e0f3b98a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 04:00:22 +0000 Subject: [PATCH 1/5] fix(plugin-security): a by-id write's row re-read propagates a store fault, and judges the platform's own predicates without the caller-predicate guard The re-read classifies its outcome through readUnlessRefused: a declared 4xx refusal keeps the record-level 403, anything else propagates as raised. The write predicates the platform composes ride into the re-read under one fresh subtree marked 'policy' with the spec's filter-subtree provenance mark, and the field-level predicate guard does not judge a subtree so marked. The controlled-by-parent master read carries its master write predicates the same way. Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude --- .changeset/by-id-write-row-reread-rules.md | 15 + .../by-id-write-row-reread.posture.test.ts | 343 ++++++++++++++++++ .../src/predicate-guard.test.ts | 39 ++ .../plugin-security/src/predicate-guard.ts | 18 +- .../src/security-plugin-authz-middleware.ts | 40 +- .../src/security-plugin-cbp-write-gate.ts | 10 +- 6 files changed, 447 insertions(+), 18 deletions(-) create mode 100644 .changeset/by-id-write-row-reread-rules.md create mode 100644 packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts diff --git a/.changeset/by-id-write-row-reread-rules.md b/.changeset/by-id-write-row-reread-rules.md new file mode 100644 index 00000000000..6c6689cad33 --- /dev/null +++ b/.changeset/by-id-write-row-reread-rules.md @@ -0,0 +1,15 @@ +--- +'@objectstack/plugin-security': patch +--- + +fix(plugin-security): a by-id write's row re-read propagates a store fault, and judges the platform's own predicates without the caller-predicate guard + +Clause-②: yes (widening) +Metadata-protocol: none + +Before an update or delete by id, the security middleware re-reads the addressed row under the caller's context with the platform's write predicates for that row (the tenancy wall and the row-level write predicates). Two rules now hold for that re-read, and for the controlled-by-parent master read a detail write asks: + +- **A store fault propagates as raised.** A re-read the store cannot answer (an unavailable datasource, a driver failure) is the write's answer, unchanged — for `ERR_DATASOURCE_UNAVAILABLE` the transports answer `503` — and nothing is written. Only a declared refusal (a 4xx) keeps the record-level `403 PERMISSION_DENIED`, one answer whatever the id names. Before, every failure of the re-read was answered `403`. The controlled-by-parent master read keeps its `403` for every failure. +- **The platform's own predicates are not the caller's predicate.** The field-level predicate guard refuses a caller's filter on a field its field rules do not let it read; it no longer judges the predicates the platform composes onto the re-read. Under the `group` and `isolated` tenancy postures, a caller whose field rules hide the organization column now updates and deletes the records of its organization by id, and writes a controlled-by-parent detail under a master of its organization, where before each write was refused `403`. A record of another organization and an id that names no record answer that caller exactly as they answer any other caller. + +Unchanged: a caller's own filter on a field it cannot read is refused as before, on a read and on a predicate write; a record the caller reads but may not write keeps its `403`; a system write is not the gate's. diff --git a/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts b/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts new file mode 100644 index 00000000000..e040dde97c7 --- /dev/null +++ b/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts @@ -0,0 +1,343 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The by-id write's row re-read (step 2.7 of the authorization middleware) and + * the controlled-by-parent master read (step 2.8), measured on a REAL `ObjectQL` + * engine over a real SQL driver with the REAL `SecurityPlugin` registered on it. + * + * Two rules: + * + * 1. A store fault raised by the re-read propagates as raised; only a declared + * refusal (a 4xx) keeps the record-level answer — the classification + * `readUnlessRefused` gives every caller-context read the write doors ask. + * 2. The predicates the platform composes onto the re-read (the tenancy wall, + * the row-level write predicates) are not the caller's predicate, so the + * caller-predicate guard (step 2.9) does not judge them. The caller's own + * predicate is still judged. + * + * Nothing else about the re-read's answers moves: a row the caller cannot read + * and an id naming no row answer as they do for every other caller, a row the + * caller reads is written, and a system write is not the gate's. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { resolveThrownHttpError } from '@objectstack/types'; + +import { SecurityPlugin } from './security-plugin.js'; + +const ORG_A = 'org_a'; +const ORG_B = 'org_b'; +const HIDDEN = { readable: false, editable: false }; +const CRUD = { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }; + +const INVOICE = 'qa_reread_invoice'; +const MASTER = 'qa_reread_master'; +const DETAIL = 'qa_reread_detail'; + +const OBJECTS = [ + { + name: INVOICE, + label: 'Invoice', + fields: { id: { name: 'id', type: 'text', primaryKey: true }, status: { name: 'status', type: 'text' } }, + }, + { + name: MASTER, + label: 'Master', + fields: { id: { name: 'id', type: 'text', primaryKey: true }, name: { name: 'name', type: 'text' } }, + }, + { + name: DETAIL, + label: 'Detail', + sharingModel: 'controlled_by_parent', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + note: { name: 'note', type: 'text' }, + master: { name: 'master', type: 'master_detail', required: true, reference: MASTER }, + }, + }, +]; + +const PERMISSION_SETS = [ + { name: 'qa_reread_member', label: 'Member', objects: { [INVOICE]: CRUD, [MASTER]: CRUD, [DETAIL]: CRUD } }, + // Field rules alone: the organization column hidden from the holder's reads. + { name: 'qa_reread_org_hidden', label: 'Organization hidden', objects: {}, fields: { [`${INVOICE}.organization_id`]: HIDDEN } }, + { name: 'qa_reread_master_org_hidden', label: 'Master organization hidden', objects: {}, fields: { [`${MASTER}.organization_id`]: HIDDEN } }, +]; + +const member = (...extra: string[]) => ({ + userId: 'u_member', + positions: [] as string[], + permissions: ['qa_reread_member', ...extra], + tenantId: ORG_A, + accessible_org_ids: [ORG_A], +}); +const PLAIN = member(); +const ORG_HIDDEN = member('qa_reread_org_hidden'); +const MASTER_ORG_HIDDEN = member('qa_reread_master_org_hidden'); + +type Posture = 'single' | 'group' | 'isolated'; +const WALLED: Posture[] = ['group', 'isolated']; +const NOWHERE = 'zz_nowhere_0001'; + +const engines: ObjectQL[] = []; +afterEach(async () => { + vi.restoreAllMocks(); + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function boot(posture: Posture): Promise { + 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.by-id-write-row-reread', + name: 'By-id write row re-read', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: OBJECTS, + } as any); + await engine.syncSchemas(); + engines.push(engine); + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (type: string, name: string) => + (type === 'permission' ? PERMISSION_SETS.find((p) => p.name === name) + : type === 'position' || type === 'capability' ? undefined + : engine.getSchema(name)) ?? null, + list: async (type: string) => (type === 'permission' ? PERMISSION_SETS : []), + }, + ...(posture === 'single' ? {} : { 'org-scoping': { name: 'com.objectstack.org-scoping' }, tenancy: { posture } }), + }; + const ctx: any = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + 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: 'qa_reread_member' }); + await plugin.init(ctx); + await plugin.start(ctx); + vi.spyOn((engine as any).logger, 'warn').mockImplementation(() => undefined); + return engine; +} + +const inOrg = (organizationId: string) => ({ context: { isSystem: true, tenantId: organizationId } } as any); + +async function seedInvoice(engine: ObjectQL, id: string, organizationId: string) { + await engine.insert(INVOICE, { id, status: 'open', organization_id: organizationId }, inOrg(organizationId)); +} + +async function storedInvoice(engine: ObjectQL, id: string): Promise | undefined> { + const rows = await engine.find(INVOICE, { where: { id }, context: { isSystem: true } } as any); + return (Array.isArray(rows) ? rows : [])[0]; +} + +/** The answer a write gets: admitted, or the envelope both doors resolve, the error's name and sentence. */ +async function answerOf(run: () => Promise): Promise> { + try { + await run(); + return { status: 200 }; + } catch (e) { + const r = resolveThrownHttpError(e); + return { status: r.status, code: r.code, name: (e as Error).name, message: (e as Error).message }; + } +} + +const writeById = (engine: ObjectQL, op: 'update' | 'delete', id: string, context: object) => + answerOf(() => op === 'update' + ? engine.update(INVOICE, { status: 'closed' }, { where: { id }, context: structuredClone(context) } as any) + : engine.delete(INVOICE, { where: { id }, context: structuredClone(context) } as any)); + +/** The caller-context re-read step 2.7 asks: `{ id }` first, then the platform's predicates. */ +const isRowReRead = (query: any, id: string) => + !query?.context?.isSystem + && Array.isArray(query?.where?.$and) + && query.where.$and.length >= 2 + && query.where.$and[0]?.id === id; + +/** Fault the engine's `findOne` for the row re-read of `id` alone; every other read is served. */ +function faultReRead(engine: ObjectQL, id: string, error: Error) { + const original = engine.findOne.bind(engine); + return vi.spyOn(engine, 'findOne').mockImplementation(async (object: any, query: any, ...rest: any[]) => { + if (object === INVOICE && isRowReRead(query, id)) throw error; + return original(object, query, ...rest); + }); +} + +/** + * A driver outage shaped like `@objectstack/objectql`'s `DatasourceUnavailableError`: + * a `code` and a `name`, and no `status`, because that class declares none. + */ +function storeFault(): Error { + return Object.assign(new Error(`[ObjectQL] Datasource 'primary' configured for object '${INVOICE}' is declared but not connected.`), { + name: 'DatasourceUnavailableError', + code: 'ERR_DATASOURCE_UNAVAILABLE', + datasource: 'primary', + }); +} + +/** A declared refusal: an error carrying a 4xx status and its code. */ +function declaredRefusal(): Error { + return Object.assign(new Error('[Security] Access denied: the read is refused.'), { + name: 'PermissionDeniedError', + code: 'PERMISSION_DENIED', + status: 403, + }); +} + +describe("a by-id write's row re-read propagates a store fault as raised; a declared refusal keeps the record-level answer", () => { + for (const posture of WALLED) { + describe(`posture '${posture}'`, () => { + it('a store fault raised by the re-read is the write\'s answer, unchanged, and nothing is written', async () => { + const engine = await boot(posture); + for (const op of ['update', 'delete'] as const) { + const id = `fault_${op}`; + await seedInvoice(engine, id, ORG_A); + const fault = storeFault(); + const spy = faultReRead(engine, id, fault); + let raised: unknown; + try { + await (op === 'update' + ? engine.update(INVOICE, { status: 'closed' }, { where: { id }, context: structuredClone(PLAIN) } as any) + : engine.delete(INVOICE, { where: { id }, context: structuredClone(PLAIN) } as any)); + } catch (e) { + raised = e; + } + expect(raised, `${op}: the fault itself, as raised`).toBe(fault); + expect(spy.mock.calls.some(([object, query]) => object === INVOICE && isRowReRead(query, id)), `${op}: the re-read was asked`).toBe(true); + spy.mockRestore(); + expect(await storedInvoice(engine, id), `${op}: nothing written`).toMatchObject({ id, status: 'open' }); + } + }); + + it('a declared refusal raised by the re-read keeps the record-level 403, one answer whatever the id names', async () => { + const engine = await boot(posture); + await seedInvoice(engine, 'refused_row', ORG_A); + for (const op of ['update', 'delete'] as const) { + const answers: Record[] = []; + for (const id of ['refused_row', NOWHERE]) { + const refusal = declaredRefusal(); + const spy = faultReRead(engine, id, refusal); + const answer = await writeById(engine, op, id, PLAIN); + spy.mockRestore(); + expect([answer.status, answer.code, answer.name], `${op} ${id}: ${answer.message}`).toEqual([403, 'PERMISSION_DENIED', 'PermissionDeniedError']); + expect(answer.message, `${op} ${id}: the record-level refusal, not the read's own`).not.toBe(refusal.message); + answers.push(answer); + } + expect(answers[0], `${op}: a row and no row answer alike`).toEqual(answers[1]); + } + expect(await storedInvoice(engine, 'refused_row'), 'nothing written').toMatchObject({ status: 'open' }); + }); + + it('control: with the store answering, the same writes land', async () => { + const engine = await boot(posture); + await seedInvoice(engine, 'steady_update', ORG_A); + await seedInvoice(engine, 'steady_delete', ORG_A); + expect(await writeById(engine, 'update', 'steady_update', PLAIN)).toEqual({ status: 200 }); + expect(await storedInvoice(engine, 'steady_update')).toMatchObject({ status: 'closed' }); + expect(await writeById(engine, 'delete', 'steady_delete', PLAIN)).toEqual({ status: 200 }); + expect(await storedInvoice(engine, 'steady_delete')).toBeUndefined(); + }); + }); + } +}); + +describe("a by-id write's row re-read judges the platform's own predicates without the caller-predicate guard; the caller's own predicate is still judged", () => { + for (const posture of WALLED) { + describe(`posture '${posture}'`, () => { + it('a caller whose field rules hide the organization column updates and deletes a row of its organization by id', async () => { + const engine = await boot(posture); + await seedInvoice(engine, 'own_update', ORG_A); + await seedInvoice(engine, 'own_delete', ORG_A); + // The precondition: the caller reads the row, and the column is hidden from it. + const seen = await engine.findOne(INVOICE, { where: { id: 'own_update' }, context: structuredClone(ORG_HIDDEN) } as any); + expect(seen, 'the caller reads the row').toBeTruthy(); + expect((seen as Record).organization_id, 'the organization column is hidden from it').toBeUndefined(); + + expect(await writeById(engine, 'update', 'own_update', ORG_HIDDEN), 'update').toEqual({ status: 200 }); + expect(await storedInvoice(engine, 'own_update'), 'update stored').toMatchObject({ status: 'closed', organization_id: ORG_A }); + expect(await writeById(engine, 'delete', 'own_delete', ORG_HIDDEN), 'delete').toEqual({ status: 200 }); + expect(await storedInvoice(engine, 'own_delete'), 'delete removed the row').toBeUndefined(); + }); + + it("a row of another organization, and an id naming no row, answer that caller exactly as they answer a caller who reads the column; neither is written", async () => { + const engine = await boot(posture); + await seedInvoice(engine, 'other_org', ORG_B); + for (const op of ['update', 'delete'] as const) { + for (const id of ['other_org', NOWHERE]) { + const reads = await writeById(engine, op, id, PLAIN); + expect([reads.status, reads.code], `${op} ${id}, column read: ${reads.message}`).toEqual([404, 'RECORD_NOT_FOUND']); + expect(await writeById(engine, op, id, ORG_HIDDEN), `${op} ${id}, column hidden`).toEqual(reads); + } + } + expect(await storedInvoice(engine, 'other_org'), 'untouched').toMatchObject({ status: 'open', organization_id: ORG_B }); + }); + + it("control: the caller's own predicate on the hidden column is still refused, on a read and on a predicate write", async () => { + const engine = await boot(posture); + await seedInvoice(engine, 'own_predicate', ORG_A); + const read = await answerOf(() => engine.find(INVOICE, { where: { organization_id: ORG_A }, context: structuredClone(ORG_HIDDEN) } as any)); + expect([read.status, read.code], String(read.message)).toEqual([403, 'PERMISSION_DENIED']); + const write = await answerOf(() => engine.update(INVOICE, { status: 'closed' }, + { multi: true, where: { organization_id: ORG_A }, context: structuredClone(ORG_HIDDEN) } as any)); + expect([write.status, write.code], String(write.message)).toEqual([403, 'PERMISSION_DENIED']); + // ...and the same predicate from a caller who reads the column is answered. + expect(await answerOf(() => engine.find(INVOICE, { where: { organization_id: ORG_A }, context: structuredClone(PLAIN) } as any))) + .toEqual({ status: 200 }); + expect(await storedInvoice(engine, 'own_predicate'), 'untouched').toMatchObject({ status: 'open' }); + }); + + it('control: a system write by id is not the gate\'s, and lands', async () => { + const engine = await boot(posture); + await seedInvoice(engine, 'system_write', ORG_B); + await engine.update(INVOICE, { status: 'closed' }, { where: { id: 'system_write' }, ...inOrg(ORG_B) } as any); + expect(await storedInvoice(engine, 'system_write')).toMatchObject({ status: 'closed' }); + }); + }); + } + + it("control, 'single': no wall is composed, and the caller who cannot read the column writes as before", async () => { + const engine = await boot('single'); + await seedInvoice(engine, 'single_row', ORG_A); + expect(await writeById(engine, 'update', 'single_row', ORG_HIDDEN)).toEqual({ status: 200 }); + expect(await writeById(engine, 'delete', 'single_row', ORG_HIDDEN)).toEqual({ status: 200 }); + expect(await writeById(engine, 'update', NOWHERE, ORG_HIDDEN)).toEqual(await writeById(engine, 'update', NOWHERE, PLAIN)); + }); +}); + +describe("the controlled-by-parent master read judges the platform's own predicates on the master without the caller-predicate guard", () => { + for (const posture of WALLED) { + it(`posture '${posture}': a caller whose field rules hide the master's organization column writes a detail under a master of its organization; a master of another organization is refused as for a caller who reads the column`, async () => { + const engine = await boot(posture); + await engine.insert(MASTER, { id: 'master_a', name: 'A', organization_id: ORG_A }, inOrg(ORG_A)); + await engine.insert(MASTER, { id: 'master_b', name: 'B', organization_id: ORG_B }, inOrg(ORG_B)); + + const own = await answerOf(() => engine.insert(DETAIL, { id: 'detail_own', note: 'n', master: 'master_a' }, + { context: structuredClone(MASTER_ORG_HIDDEN) } as any)); + expect(own, 'a detail under a master of its organization').toEqual({ status: 200 }); + const stored = await engine.find(DETAIL, { where: { id: 'detail_own' }, context: { isSystem: true } } as any); + expect(stored, 'the detail is stored').toHaveLength(1); + + const reads = await answerOf(() => engine.insert(DETAIL, { id: 'detail_other_plain', note: 'n', master: 'master_b' }, + { context: structuredClone(PLAIN) } as any)); + expect(reads.status, `column read: ${reads.message}`).not.toBe(200); + const hidden = await answerOf(() => engine.insert(DETAIL, { id: 'detail_other_hidden', note: 'n', master: 'master_b' }, + { context: structuredClone(MASTER_ORG_HIDDEN) } as any)); + expect([hidden.status, hidden.code, hidden.name], `column hidden: ${hidden.message}`).toEqual([reads.status, reads.code, reads.name]); + expect(await engine.find(DETAIL, { where: { master: 'master_b' }, context: { isSystem: true } } as any), 'nothing stored').toHaveLength(0); + }); + } +}); diff --git a/packages/plugins/plugin-security/src/predicate-guard.test.ts b/packages/plugins/plugin-security/src/predicate-guard.test.ts index b88fbefd3f7..c4a0a3f4d6c 100644 --- a/packages/plugins/plugin-security/src/predicate-guard.test.ts +++ b/packages/plugins/plugin-security/src/predicate-guard.test.ts @@ -9,6 +9,7 @@ import { assertReadableQueryFields, } from './predicate-guard.js'; import { isPermissionDeniedError } from './errors.js'; +import { markFilterSubtreeProvenance } from '@objectstack/spec/data'; const HIDDEN_SALARY = { salary: { readable: false, editable: false } }; @@ -198,3 +199,41 @@ describe('a cross-field comparand is collected and judged like a condition key', } }); }); + +describe("the guard judges the caller's own predicate, never a subtree the platform composed and marked 'policy'", () => { + const policy = (subtree: T): T => markFilterSubtreeProvenance(subtree, 'policy'); + const refusal = (ast: Record) => { + try { + assertReadableQueryFields(ast, HIDDEN_SALARY, 'employee'); + return undefined; + } catch (e) { + return e as { code?: string; statusCode?: number }; + } + }; + + it("a hidden field inside a 'policy' subtree is not collected, nor anything beneath it", () => { + const where = { $and: [{ id: 'e1' }, policy({ $and: [{ salary: { $gte: 1 } }, { $or: [{ salary: 2 }] }] })] }; + expect([...collectConditionFields(where)]).toEqual(['id']); + expect(refusal({ where })).toBeUndefined(); + }); + + it("CONTROL the caller's own predicate on the hidden field beside a 'policy' subtree is still refused", () => { + const where = { $and: [{ salary: { $gte: 100000 } }, policy({ salary: { $gte: 1 } })] }; + const r = refusal({ where }); + expect(isPermissionDeniedError(r)).toBe(true); + expect([r?.code, r?.statusCode]).toEqual(['PERMISSION_DENIED', 403]); + }); + + it("CONTROL an unmarked subtree, an 'author' subtree, and a copy of a 'policy' subtree are judged as the caller's", () => { + const marked = policy({ salary: { $gte: 1 } }); + for (const subtree of [ + { salary: { $gte: 1 } }, + markFilterSubtreeProvenance({ salary: { $gte: 1 } }, 'author'), + { ...marked }, + JSON.parse(JSON.stringify(marked)) as Record, + ]) { + const r = refusal({ where: { $and: [{ id: 'e1' }, subtree] } }); + expect([r?.code, r?.statusCode]).toEqual(['PERMISSION_DENIED', 403]); + } + }); +}); diff --git a/packages/plugins/plugin-security/src/predicate-guard.ts b/packages/plugins/plugin-security/src/predicate-guard.ts index 21bb89938fd..5c6dfada09c 100644 --- a/packages/plugins/plugin-security/src/predicate-guard.ts +++ b/packages/plugins/plugin-security/src/predicate-guard.ts @@ -21,9 +21,20 @@ * Ordering contract: this guard MUST run against the CALLER-supplied AST, * before RLS filter injection — RLS policies legitimately reference fields * the caller cannot read (e.g. `owner_id`), and must not be rejected. + * + * Provenance contract: where a predicate the platform composed reaches a read + * INSIDE the caller's own `where` — the security middleware's by-id write + * re-read carries the tenancy wall and the row-level write predicates that + * way — the composing code marks that subtree `'policy'` with the spec's + * filter-subtree provenance mark (`markFilterSubtreeProvenance`, the mark the + * read-scope merge boundaries already stamp on every scope they inject), and + * this guard does not judge a subtree so marked: it is not the caller's + * predicate. The fail direction is closed: a subtree that carries no mark, or + * any other mark, is judged as the caller's — the mark never survives a copy, + * a serialization or the wire, so a request cannot carry one in. */ -import { FieldReferenceSchema } from '@objectstack/spec/data'; +import { FieldReferenceSchema, filterSubtreeProvenanceOf } from '@objectstack/spec/data'; import { PermissionDeniedError } from './errors.js'; interface FieldPermissionLike { @@ -43,9 +54,14 @@ const LOGICAL_KEYS = new Set(['$and', '$or', '$not']); * column of the same row"): the comparison reads the named field's value, so * row presence discloses it exactly as a filter on the field itself would. * Both are collected here, into the one set the one field rule judges. + * + * A subtree the platform composed and marked `'policy'` is not collected, + * nor anything beneath it (see the provenance contract above): its fields + * are the platform's predicate, not the caller's. */ export function collectConditionFields(condition: unknown, out: Set = new Set()): Set { if (!condition || typeof condition !== 'object' || Array.isArray(condition)) return out; + if (filterSubtreeProvenanceOf(condition) === 'policy') return out; for (const [key, value] of Object.entries(condition as Record)) { if (LOGICAL_KEYS.has(key)) { if (Array.isArray(value)) for (const sub of value) collectConditionFields(sub, out); diff --git a/packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts b/packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts index e74140ae2a3..f3a6b3d705a 100644 --- a/packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts +++ b/packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts @@ -102,7 +102,7 @@ import type { II18nService, IMetadataService, IObjectQLEngine, SharingWriteVerdi import { crossFieldClassRefusalCarriedBy, findCrossFieldClassRefusal } from '@objectstack/formula'; import { declaredHttpStatus } from '@objectstack/types'; import type { SecurityPlugin } from './security-plugin.js'; -import { isGuestEnvelope, requiredCapsForOperation } from './security-plugin.js'; +import { isGuestEnvelope, readUnlessRefused, requiredCapsForOperation } from './security-plugin.js'; import { assertFlowSystemIdentityWriteAllowed } from './flow-system-identity-write-refusal.js'; import { isUnreadableAddressedRowRefusal } from './delegated-admin-gate.js'; import { isRowWriteProbe, logGateRefusal, type AuthzMiddlewareFn } from './record-row-write-probe.js'; @@ -1161,24 +1161,32 @@ export function registerAuthzMiddleware( // may not write it keeps the 403 below; nothing is disclosed to them. const addressedId = this.addressedByIdWriteId(opCtx, nestedInOperation, referentialFieldClearWrite); if (writeParts.length > 0) { - let visible: unknown = null; - // A re-read that THREW is not an absent row: a store fault or a - // read-time policy refusal keeps today's denial below (#7505). - let reReadThrew = false; - try { - visible = await this.ql.findOne(opCtx.object, { - where: { $and: [{ id: targetId }, ...writeParts] }, + // The write predicates are the PLATFORM's, composed here — never the + // caller's own predicate. They ride into the caller-context re-read + // under one fresh `$and` marked `'policy'` (the filter-subtree + // provenance mark step 3 stamps on every scope it injects), so the + // re-read's caller-predicate guard (step 2.9) judges the caller's + // part, `{ id }`, and not a wall or row-level predicate on a field + // the caller cannot read. The mark goes on a node built here, so a + // frozen or shared filter object cannot leave it unmarked. + const platformWritePredicate = markFilterSubtreeProvenance({ $and: writeParts }, 'policy'); + // Three outcomes of the re-read, classified by `readUnlessRefused` + // (#7505): a row or no row; a declared 4xx refusal (the read door + // refuses this caller), which keeps the record-level denial below, + // one answer for every id; anything else is a store fault and + // propagates as raised — an outage is neither a hidden row nor a + // refusal, and answering it as either would relabel it. + const reRead = await readUnlessRefused(() => + this.ql.findOne(opCtx.object, { + where: { $and: [{ id: targetId }, platformWritePredicate] }, context: opCtx.context, - }); - } catch { - // A read denial (e.g. no read permission) is itself a "cannot - // touch this row" signal — fall through to the deny below. - visible = null; - reReadThrew = true; - } + }), + ); + const reReadRefused = reRead.refused; + const visible: unknown = reRead.refused ? null : reRead.value; if ( !visible && - !reReadThrew && + !reReadRefused && addressedId !== undefined && (await this.callerCannotReadAddressedRow(opCtx, addressedId)) ) { diff --git a/packages/plugins/plugin-security/src/security-plugin-cbp-write-gate.ts b/packages/plugins/plugin-security/src/security-plugin-cbp-write-gate.ts index abb007e2160..cb1c44f8566 100644 --- a/packages/plugins/plugin-security/src/security-plugin-cbp-write-gate.ts +++ b/packages/plugins/plugin-security/src/security-plugin-cbp-write-gate.ts @@ -83,6 +83,7 @@ import { type EngineUpdateDispatchData, } from '@objectstack/metadata-core'; import type { ControlledByParentWriteDenialLeg, ControlledByParentWriteOutcome } from '@objectstack/spec/contracts'; +import { markFilterSubtreeProvenance } from '@objectstack/spec/data'; import { PermissionDeniedError, MasterDetailRelationMissingError, @@ -870,9 +871,16 @@ export async function assertMasterRowEditable( // is what the 403 below says. The ruling names this probe (and the // sharing half beneath it) as the house fail-closed posture to match, // not as a site to change. + // + // The master's write predicates are the platform's, composed above — not + // the caller's own predicate — so they ride into this caller-context read + // under one fresh `$and` marked `'policy'`, exactly as step 2.7's by-id + // re-read carries its own: the read's caller-predicate guard (step 2.9) + // judges `{ id }` only, never a wall or row-level predicate on a master + // field the caller cannot read. try { visible = await this.ql.findOne(rel.master, { - where: { $and: [{ id: masterId }, ...masterWriteParts] }, + where: { $and: [{ id: masterId }, markFilterSubtreeProvenance({ $and: masterWriteParts }, 'policy')] }, context, }); } catch { From f89d12e09a78c267edb45828f5b4088843692566 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 04:18:54 +0000 Subject: [PATCH 2/5] test(plugin-security): the Layer 0 matrix gate reads the write predicates from the re-read's platform subtree Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude --- .../plugins/plugin-security/src/authz-matrix-gate.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-security/src/authz-matrix-gate.test.ts b/packages/plugins/plugin-security/src/authz-matrix-gate.test.ts index daa6bdf4bc3..8abc2b53ca8 100644 --- a/packages/plugins/plugin-security/src/authz-matrix-gate.test.ts +++ b/packages/plugins/plugin-security/src/authz-matrix-gate.test.ts @@ -244,7 +244,11 @@ async function writeFilter(cell: any, roleCtx: any): Promise { if (!reRead) { return threw ? `CRUD_DENY:${threw?.name ?? 'err'}` : 'BYPASS(no-write-filter)'; } - return reRead[1].where.$and.slice(1); + // The re-read is `{ $and: [{ id }, PLATFORM] }`: PLATFORM is one `$and` of + // the write predicates, marked as the platform's own (never the caller's + // predicate), and its members are the matrix this gate locks. + const [, platform] = reRead[1].where.$and; + return platform.$and; } /** From e611e6e21b40027824cb6266e772e641c85ca94b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 04:31:01 +0000 Subject: [PATCH 3/5] test(plugin-security): the row re-read pins pass typed engine options Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude --- .../by-id-write-row-reread.posture.test.ts | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts b/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts index e040dde97c7..6c06aa51f9b 100644 --- a/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts +++ b/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts @@ -134,14 +134,14 @@ async function boot(posture: Posture): Promise { return engine; } -const inOrg = (organizationId: string) => ({ context: { isSystem: true, tenantId: organizationId } } as any); +const inOrg = (organizationId: string) => ({ context: { isSystem: true, tenantId: organizationId } }); async function seedInvoice(engine: ObjectQL, id: string, organizationId: string) { await engine.insert(INVOICE, { id, status: 'open', organization_id: organizationId }, inOrg(organizationId)); } async function storedInvoice(engine: ObjectQL, id: string): Promise | undefined> { - const rows = await engine.find(INVOICE, { where: { id }, context: { isSystem: true } } as any); + const rows = await engine.find(INVOICE, { where: { id }, context: { isSystem: true } }); return (Array.isArray(rows) ? rows : [])[0]; } @@ -158,8 +158,8 @@ async function answerOf(run: () => Promise): Promise answerOf(() => op === 'update' - ? engine.update(INVOICE, { status: 'closed' }, { where: { id }, context: structuredClone(context) } as any) - : engine.delete(INVOICE, { where: { id }, context: structuredClone(context) } as any)); + ? engine.update(INVOICE, { status: 'closed' }, { where: { id }, context: structuredClone(context) }) + : engine.delete(INVOICE, { where: { id }, context: structuredClone(context) })); /** The caller-context re-read step 2.7 asks: `{ id }` first, then the platform's predicates. */ const isRowReRead = (query: any, id: string) => @@ -211,8 +211,8 @@ describe("a by-id write's row re-read propagates a store fault as raised; a decl let raised: unknown; try { await (op === 'update' - ? engine.update(INVOICE, { status: 'closed' }, { where: { id }, context: structuredClone(PLAIN) } as any) - : engine.delete(INVOICE, { where: { id }, context: structuredClone(PLAIN) } as any)); + ? engine.update(INVOICE, { status: 'closed' }, { where: { id }, context: structuredClone(PLAIN) }) + : engine.delete(INVOICE, { where: { id }, context: structuredClone(PLAIN) })); } catch (e) { raised = e; } @@ -263,7 +263,7 @@ describe("a by-id write's row re-read judges the platform's own predicates witho await seedInvoice(engine, 'own_update', ORG_A); await seedInvoice(engine, 'own_delete', ORG_A); // The precondition: the caller reads the row, and the column is hidden from it. - const seen = await engine.findOne(INVOICE, { where: { id: 'own_update' }, context: structuredClone(ORG_HIDDEN) } as any); + const seen = await engine.findOne(INVOICE, { where: { id: 'own_update' }, context: structuredClone(ORG_HIDDEN) }); expect(seen, 'the caller reads the row').toBeTruthy(); expect((seen as Record).organization_id, 'the organization column is hidden from it').toBeUndefined(); @@ -289,13 +289,13 @@ describe("a by-id write's row re-read judges the platform's own predicates witho it("control: the caller's own predicate on the hidden column is still refused, on a read and on a predicate write", async () => { const engine = await boot(posture); await seedInvoice(engine, 'own_predicate', ORG_A); - const read = await answerOf(() => engine.find(INVOICE, { where: { organization_id: ORG_A }, context: structuredClone(ORG_HIDDEN) } as any)); + const read = await answerOf(() => engine.find(INVOICE, { where: { organization_id: ORG_A }, context: structuredClone(ORG_HIDDEN) })); expect([read.status, read.code], String(read.message)).toEqual([403, 'PERMISSION_DENIED']); const write = await answerOf(() => engine.update(INVOICE, { status: 'closed' }, - { multi: true, where: { organization_id: ORG_A }, context: structuredClone(ORG_HIDDEN) } as any)); + { multi: true, where: { organization_id: ORG_A }, context: structuredClone(ORG_HIDDEN) })); expect([write.status, write.code], String(write.message)).toEqual([403, 'PERMISSION_DENIED']); // ...and the same predicate from a caller who reads the column is answered. - expect(await answerOf(() => engine.find(INVOICE, { where: { organization_id: ORG_A }, context: structuredClone(PLAIN) } as any))) + expect(await answerOf(() => engine.find(INVOICE, { where: { organization_id: ORG_A }, context: structuredClone(PLAIN) }))) .toEqual({ status: 200 }); expect(await storedInvoice(engine, 'own_predicate'), 'untouched').toMatchObject({ status: 'open' }); }); @@ -303,7 +303,7 @@ describe("a by-id write's row re-read judges the platform's own predicates witho it('control: a system write by id is not the gate\'s, and lands', async () => { const engine = await boot(posture); await seedInvoice(engine, 'system_write', ORG_B); - await engine.update(INVOICE, { status: 'closed' }, { where: { id: 'system_write' }, ...inOrg(ORG_B) } as any); + await engine.update(INVOICE, { status: 'closed' }, { where: { id: 'system_write' }, ...inOrg(ORG_B) }); expect(await storedInvoice(engine, 'system_write')).toMatchObject({ status: 'closed' }); }); }); @@ -326,18 +326,18 @@ describe("the controlled-by-parent master read judges the platform's own predica await engine.insert(MASTER, { id: 'master_b', name: 'B', organization_id: ORG_B }, inOrg(ORG_B)); const own = await answerOf(() => engine.insert(DETAIL, { id: 'detail_own', note: 'n', master: 'master_a' }, - { context: structuredClone(MASTER_ORG_HIDDEN) } as any)); + { context: structuredClone(MASTER_ORG_HIDDEN) })); expect(own, 'a detail under a master of its organization').toEqual({ status: 200 }); - const stored = await engine.find(DETAIL, { where: { id: 'detail_own' }, context: { isSystem: true } } as any); + const stored = await engine.find(DETAIL, { where: { id: 'detail_own' }, context: { isSystem: true } }); expect(stored, 'the detail is stored').toHaveLength(1); const reads = await answerOf(() => engine.insert(DETAIL, { id: 'detail_other_plain', note: 'n', master: 'master_b' }, - { context: structuredClone(PLAIN) } as any)); + { context: structuredClone(PLAIN) })); expect(reads.status, `column read: ${reads.message}`).not.toBe(200); const hidden = await answerOf(() => engine.insert(DETAIL, { id: 'detail_other_hidden', note: 'n', master: 'master_b' }, - { context: structuredClone(MASTER_ORG_HIDDEN) } as any)); + { context: structuredClone(MASTER_ORG_HIDDEN) })); expect([hidden.status, hidden.code, hidden.name], `column hidden: ${hidden.message}`).toEqual([reads.status, reads.code, reads.name]); - expect(await engine.find(DETAIL, { where: { master: 'master_b' }, context: { isSystem: true } } as any), 'nothing stored').toHaveLength(0); + expect(await engine.find(DETAIL, { where: { master: 'master_b' }, context: { isSystem: true } }), 'nothing stored').toHaveLength(0); }); } }); From b6b340a3db9bf6254c5941f56c410bc3fe85e74b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 05:17:42 +0000 Subject: [PATCH 4/5] test(plugin-security): an organization administrator whose field rules hide the organization column removes an in-boundary assignment by id Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude --- .../by-id-write-row-reread.posture.test.ts | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts b/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts index 6c06aa51f9b..5da616c58fd 100644 --- a/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts +++ b/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts @@ -25,7 +25,12 @@ import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { resolveThrownHttpError } from '@objectstack/types'; +import { SysUser, SysMember, SysBusinessUnit, SysBusinessUnitMember } from '@objectstack/platform-objects/identity'; + import { SecurityPlugin } from './security-plugin.js'; +import { SysUserPosition } from './objects/sys-user-position.object.js'; +import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; +import { defaultPermissionSets } from './objects/index.js'; const ORG_A = 'org_a'; const ORG_B = 'org_b'; @@ -341,3 +346,96 @@ describe("the controlled-by-parent master read judges the platform's own predica }); } }); + +// ── The organization administrator's assignment removal ───────────────────── + +/** An in-boundary set and position: no superuser bit, no `adminScope`, an organization capability only. */ +const IN_BOUNDARY_SET = { name: 'qa_reread_in_set', label: 'In boundary', objects: { sys_user: { allowRead: true } }, systemPermissions: ['manage_org_users'] }; +const IN_BOUNDARY_POSITION = { name: 'qa_reread_in_pos', label: 'In boundary', permissionSets: [IN_BOUNDARY_SET.name] }; +/** Field rules alone, held beside `organization_admin`: both assignment tables' organization column hidden from its reads. */ +const ASSIGNMENT_ORG_HIDDEN = { + name: 'qa_reread_assignment_org_hidden', + label: 'Assignment organization hidden', + objects: {}, + fields: { 'sys_user_position.organization_id': HIDDEN, 'sys_user_permission_set.organization_id': HIDDEN }, +}; +const ASSIGNMENT_SETS = [...defaultPermissionSets, IN_BOUNDARY_SET, ASSIGNMENT_ORG_HIDDEN]; +const ASSIGNMENT_ROWS = { + sys_user_position: { position: IN_BOUNDARY_POSITION.name }, + sys_user_permission_set: { permission_set: IN_BOUNDARY_SET.name }, +} as const; +type AssignmentTable = keyof typeof ASSIGNMENT_ROWS; +const orgAdmin = (...extra: string[]) => ({ + userId: 'u_org_admin', positions: [] as string[], permissions: ['organization_admin', ...extra], tenantId: ORG_A, accessible_org_ids: [ORG_A], +}); + +async function bootAssignments(posture: Posture): Promise { + 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.by-id-write-row-reread.assignments', + name: 'By-id write row re-read, assignments', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [SysUserPosition, SysUserPermissionSet, SysMember, SysBusinessUnit, SysBusinessUnitMember], + } as any); + await engine.syncSchemas(); + if (!engine.registry.getObject(SysUser.name)) engine.registry.registerObject(SysUser as never, 'qa.by-id-write-row-reread'); + engines.push(engine); + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (type: string, name: string) => + (type === 'position' ? (name === IN_BOUNDARY_POSITION.name ? IN_BOUNDARY_POSITION : undefined) + : type === 'permission' ? ASSIGNMENT_SETS.find((p) => p.name === name) + : type === 'capability' ? undefined + : engine.getSchema(name)) ?? null, + list: async (type: string) => (type === 'position' ? [IN_BOUNDARY_POSITION] : type === 'permission' ? ASSIGNMENT_SETS : []), + }, + 'org-scoping': { name: 'com.objectstack.org-scoping' }, + tenancy: { posture }, + }; + const ctx: any = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + 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); + for (const m of [ + { id: 'm_admin', user_id: 'u_org_admin', organization_id: ORG_A, role: 'admin' }, + { id: 'm_holder', user_id: 'u_holder', organization_id: ORG_A, role: 'member' }, + ]) { + await engine.insert('sys_member', m, inOrg(ORG_A)); + } + return engine; +} + +describe("an organization administrator whose field rules hide the organization column removes an in-boundary assignment by id", () => { + for (const posture of WALLED) { + it(`posture '${posture}': the removal is admitted, as for an administrator who reads the column`, async () => { + const engine = await bootAssignments(posture); + for (const table of Object.keys(ASSIGNMENT_ROWS) as AssignmentTable[]) { + for (const [label, context] of [['column read', orgAdmin()], ['column hidden', orgAdmin(ASSIGNMENT_ORG_HIDDEN.name)]] as const) { + const id = `${table}_${label.replace(' ', '_')}`; + await engine.insert(table, { id, user_id: 'u_holder', ...ASSIGNMENT_ROWS[table], organization_id: ORG_A }, inOrg(ORG_A)); + const answer = await answerOf(() => engine.delete(table, { where: { id }, context: structuredClone(context) })); + expect(answer, `${table}, ${label}: ${String(answer.message)}`).toEqual({ status: 200 }); + expect(await engine.find(table, { where: { id }, context: { isSystem: true } }), `${table}, ${label}: removed`).toHaveLength(0); + } + } + }); + } +}); From cc1fbac302c6bc1f9b1a4929b28512285b8619ab Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 06:15:05 +0000 Subject: [PATCH 5/5] fix(plugin-security): the row re-read changeset and pins state the rules only, and name every platform scope the predicate guard does not judge The changeset states the two rules without narrating earlier answers, and names where the provenance rule reaches: the by-id write re-read, the controlled-by-parent master read, the analytics read-scope merge handed to an aggregate under the caller's context, and a sharing read scope composed before the security middleware. A REST-level pin covers the analytics face. Claude-Session: https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj Co-authored-by: Claude --- .changeset/by-id-write-row-reread-rules.md | 8 +- .../by-id-write-row-reread.posture.test.ts | 2 +- ...ics-platform-scope-predicate-guard.test.ts | 153 ++++++++++++++++++ 3 files changed, 158 insertions(+), 5 deletions(-) create mode 100644 packages/rest/src/analytics-platform-scope-predicate-guard.test.ts diff --git a/.changeset/by-id-write-row-reread-rules.md b/.changeset/by-id-write-row-reread-rules.md index 6c6689cad33..3217934a710 100644 --- a/.changeset/by-id-write-row-reread-rules.md +++ b/.changeset/by-id-write-row-reread-rules.md @@ -7,9 +7,9 @@ fix(plugin-security): a by-id write's row re-read propagates a store fault, and Clause-②: yes (widening) Metadata-protocol: none -Before an update or delete by id, the security middleware re-reads the addressed row under the caller's context with the platform's write predicates for that row (the tenancy wall and the row-level write predicates). Two rules now hold for that re-read, and for the controlled-by-parent master read a detail write asks: +Before an update or delete by id, the security middleware re-reads the addressed row under the caller's context with the platform's write predicates for that row (the tenancy wall and the row-level write predicates). Two rules hold for that re-read, and the second for the controlled-by-parent master read a detail write asks as well: -- **A store fault propagates as raised.** A re-read the store cannot answer (an unavailable datasource, a driver failure) is the write's answer, unchanged — for `ERR_DATASOURCE_UNAVAILABLE` the transports answer `503` — and nothing is written. Only a declared refusal (a 4xx) keeps the record-level `403 PERMISSION_DENIED`, one answer whatever the id names. Before, every failure of the re-read was answered `403`. The controlled-by-parent master read keeps its `403` for every failure. -- **The platform's own predicates are not the caller's predicate.** The field-level predicate guard refuses a caller's filter on a field its field rules do not let it read; it no longer judges the predicates the platform composes onto the re-read. Under the `group` and `isolated` tenancy postures, a caller whose field rules hide the organization column now updates and deletes the records of its organization by id, and writes a controlled-by-parent detail under a master of its organization, where before each write was refused `403`. A record of another organization and an id that names no record answer that caller exactly as they answer any other caller. +- **A store fault propagates as raised.** A re-read the store cannot answer (an unavailable datasource, a driver failure) is the write's answer, unchanged — for `ERR_DATASOURCE_UNAVAILABLE` the transports answer `503` — and nothing is written. Only a declared refusal (a 4xx) keeps the record-level `403 PERMISSION_DENIED`, one answer whatever the id names. The controlled-by-parent master read keeps its `403` for every failure. +- **The platform's own predicates are not the caller's predicate.** The field-level predicate guard refuses a caller's filter on a field its field rules do not let it read, and judges only the caller's predicate: a predicate subtree the platform composed and marked as its own is not judged. That holds for every such subtree that reaches the guard inside a caller-context read — the by-id write re-read and the controlled-by-parent master read, the analytics read-scope merge handed to an aggregate under the caller's context, and a record-sharing read scope composed before the security middleware. Only platform-composed predicates are exempt; nothing the caller writes is. Under the `group` and `isolated` tenancy postures, a caller whose field rules hide the organization column updates and deletes the records of its organization by id, writes a controlled-by-parent detail under a master of its organization, and aggregates over the records of its organization. A record of another organization and an id that names no record answer that caller exactly as they answer any other caller. -Unchanged: a caller's own filter on a field it cannot read is refused as before, on a read and on a predicate write; a record the caller reads but may not write keeps its `403`; a system write is not the gate's. +Unchanged: a caller's own filter on a field it cannot read is refused, on a read, an analytics query and a predicate write; a record the caller reads but may not write keeps its `403`; a system write is not the gate's. diff --git a/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts b/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts index 5da616c58fd..a8ce96ac542 100644 --- a/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts +++ b/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts @@ -314,7 +314,7 @@ describe("a by-id write's row re-read judges the platform's own predicates witho }); } - it("control, 'single': no wall is composed, and the caller who cannot read the column writes as before", async () => { + it("control, 'single': no wall is composed, and the caller who cannot read the column writes by id", async () => { const engine = await boot('single'); await seedInvoice(engine, 'single_row', ORG_A); expect(await writeById(engine, 'update', 'single_row', ORG_HIDDEN)).toEqual({ status: 200 }); diff --git a/packages/rest/src/analytics-platform-scope-predicate-guard.test.ts b/packages/rest/src/analytics-platform-scope-predicate-guard.test.ts new file mode 100644 index 00000000000..8459f0b0b5e --- /dev/null +++ b/packages/rest/src/analytics-platform-scope-predicate-guard.test.ts @@ -0,0 +1,153 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The field-level predicate guard judges the caller's own predicate, never the + * read scope the platform composes — on the analytics face. + * + * The analytics strategy that aggregates through the engine merges the + * caller's read scope (here the tenancy wall on `organization_id`) into the + * filter it hands to `engine.aggregate` under the caller's context, marked as + * the platform's own. A caller whose field rules hide that column aggregates + * over the records of its organization; its own filter on the column is still + * refused. + * + * Real stack: `ObjectQL` over SQLite, the real `SecurityPlugin` under the + * `group` and `isolated` tenancy postures, the real `AnalyticsServicePlugin` + * with the engine-aggregate strategy. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SecurityPlugin } from '@objectstack/plugin-security'; +import { AnalyticsServicePlugin, type AnalyticsService } from '@objectstack/service-analytics'; + +const OBJECT = 'rest_an_scope_ledger'; +const ORG_A = 'org_a'; +const ORG_B = 'org_b'; +const SYS = { context: { isSystem: true, userId: 'usr_system' } }; + +const MEMBER_SET = PermissionSetSchema.parse({ + name: 'rest_an_scope_member', + label: 'Member', + objects: { [OBJECT]: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } }, +}); +/** Field rules alone: the organization column hidden from the holder's reads. */ +const ORG_HIDDEN_SET = PermissionSetSchema.parse({ + name: 'rest_an_scope_org_hidden', + label: 'Organization hidden', + objects: {}, + fields: { [`${OBJECT}.organization_id`]: { readable: false, editable: false } }, +}); + +const caller = (...extra: string[]) => ({ + userId: 'usr_member', + positions: [] as string[], + permissions: [MEMBER_SET.name, ...extra], + posture: 'MEMBER', + tenantId: ORG_A, + accessible_org_ids: [ORG_A], +}); +const READS_COLUMN = caller(); +const COLUMN_HIDDEN = caller(ORG_HIDDEN_SET.name); + +const quiet: any = { debug() {}, info() {}, warn() {}, error() {}, child() { return quiet; } }; + +const engines: ObjectQL[] = []; +afterEach(async () => { + vi.restoreAllMocks(); + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function boot(posture: 'group' | 'isolated'): Promise<{ engine: ObjectQL; service: AnalyticsService }> { + const engine = new ObjectQL({ logger: quiet } as any); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true } as any), + true, + ); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.qa.analytics-platform-scope-predicate-guard', + name: 'Analytics platform scope and the predicate guard', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [{ name: OBJECT, label: 'Ledger', fields: { title: { name: 'title', type: 'text' } } }], + } as never); + await engine.syncSchemas(); + engines.push(engine); + + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + data: engine, + metadata: { + get: async (type: string, name: string) => + (type === 'permission' ? [MEMBER_SET, ORG_HIDDEN_SET].find((p) => p.name === name) + : type === 'position' || type === 'capability' ? undefined + : engine.getSchema(name)) ?? null, + list: async (type: string) => (type === 'permission' ? [MEMBER_SET, ORG_HIDDEN_SET] : []), + }, + 'org-scoping': { name: 'com.objectstack.org-scoping' }, + tenancy: { posture }, + }; + const ctx: any = { + logger: quiet, + hook: () => {}, + registerService: (name: string, svc: unknown) => { services[name] = svc; }, + replaceService: (name: string, svc: unknown) => { services[name] = svc; }, + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const security = new SecurityPlugin({ fallbackPermissionSet: MEMBER_SET.name }); + await security.init(ctx); + await security.start(ctx); + vi.spyOn((engine as unknown as { logger: { warn: () => void } }).logger, 'warn').mockImplementation(() => undefined); + + await engine.insert(OBJECT, [ + { id: 'a1', title: 't', organization_id: ORG_A }, + { id: 'a2', title: 't', organization_id: ORG_A }, + ], { context: { ...SYS.context, tenantId: ORG_A } } as never); + await engine.insert(OBJECT, [{ id: 'b1', title: 't', organization_id: ORG_B }], { context: { ...SYS.context, tenantId: ORG_B } } as never); + + await new AnalyticsServicePlugin({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + }).init(ctx); + return { engine, service: services.analytics as AnalyticsService }; +} + +type Thrown = { code?: string; status?: number; statusCode?: number; message?: string } | null; +const answerOf = (run: () => Promise) => + run().then( + (r) => ({ answered: r as { rows?: Array> } }), + (e: Thrown) => ({ refused: { code: e?.code, status: e?.status ?? e?.statusCode, message: String(e?.message) } }), + ); +const countOf = (r: { answered?: { rows?: Array> } }) => + Number(Object.values(r.answered?.rows?.[0] ?? {})[0]); + +describe("the predicate guard judges the caller's own predicate, never the read scope the analytics merge composes", () => { + for (const posture of ['group', 'isolated'] as const) { + it(`posture '${posture}': a caller whose field rules hide the scoped column aggregates over the platform scope; its own filter on the column is refused`, async () => { + const { service } = await boot(posture); + const query = { cube: OBJECT, measures: ['count'] }; + + const reads = await answerOf(() => service.query(query as never, structuredClone(READS_COLUMN) as never)); + expect(reads, JSON.stringify(reads)).toHaveProperty('answered'); + expect(countOf(reads as never), 'the caller who reads the column counts its organization').toBe(2); + + const hidden = await answerOf(() => service.query(query as never, structuredClone(COLUMN_HIDDEN) as never)); + expect(hidden, JSON.stringify(hidden)).toHaveProperty('answered'); + expect(countOf(hidden as never), 'the caller who cannot read the column counts the same records').toBe(2); + + const own = await answerOf(() => service.query( + { ...query, where: { organization_id: ORG_A } } as never, structuredClone(COLUMN_HIDDEN) as never)); + expect(own, JSON.stringify(own)).toHaveProperty('refused'); + expect([(own as any).refused.code, (own as any).refused.status]).toEqual(['PERMISSION_DENIED', 403]); + }); + } +});