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..3217934a710 --- /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 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. 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, 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/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; } /** 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..a8ce96ac542 --- /dev/null +++ b/packages/plugins/plugin-security/src/by-id-write-row-reread.posture.test.ts @@ -0,0 +1,441 @@ +// 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 { 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'; +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 } }); + +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 } }); + 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) }) + : 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) => + !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) }) + : engine.delete(INVOICE, { where: { id }, context: structuredClone(PLAIN) })); + } 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) }); + 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) })); + 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) })); + 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) }))) + .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) }); + 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 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 }); + 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) })); + 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 } }); + 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) })); + 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) })); + 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 } }), 'nothing stored').toHaveLength(0); + }); + } +}); + +// ── 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); + } + } + }); + } +}); 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 { 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]); + }); + } +});