diff --git a/.changeset/24932-cbp-leaf-switch.md b/.changeset/24932-cbp-leaf-switch.md new file mode 100644 index 00000000000..131a44c6998 --- /dev/null +++ b/.changeset/24932-cbp-leaf-switch.md @@ -0,0 +1,16 @@ +--- +'@objectstack/plugin-security': patch +'@objectstack/service-analytics': patch +'@objectstack/driver-turso': patch +'@objectstack/spec': patch +--- + +fix(plugin-security): a `controlled_by_parent` detail's read scope is the engine's `ReadableIds` leaf over its master — one statement per read, no master id list, 200 past the dialect's bind limit + +Clause-②: no +Metadata-protocol: none + +- **`@objectstack/plugin-security`.** The read derivation for a `controlled_by_parent` object answers `{ masterFK: { $in: ReadableIds.of(master) } }` (ADR-0139 D6, the leaf's seventh consumer) in place of a master id list it pre-scanned under a system context and bound into `$in`. The engine composes the leaf at its executor boundary from this service's own `canReadObject` / `getReadFilter` of the master — recursively down a master chain — and the driver compiles one statement with the master inside a subquery. Measured before: 2 statements per find and 4 per find + count on a detail, and `DATABASE_ERROR` / 500 past ~32,800 masters (`too many SQL variables`). Now: 1 and 2, no scan of the master, and 200 with the right rows at 33,000 masters. Every row set is unchanged: plain, restricted, unrestricted (`viewAllRecords`), delegated (the ADR-0090 D10 intersection), two-level chains, null and dangling master pointers (still excluded), and a caller the master object does not admit (still nothing). A metadata cycle and a chain past the depth bound still answer zero rows, now from a registry-only walk that pre-empts the engine's own refusal. A write verb's predicate carries no leaf (ADR-0139 D1): a predicate write of a detail is narrowed by the read door's answer and judged per master, as before. +- **`@objectstack/service-analytics`.** The engine-bound merges (`ObjectQLStrategy.withReadScope`, its FK attribute map, and the plugin's record-label pass) admit the leaf and hand the scope to the engine, which composes it as a find does. `NativeSQLStrategy.canHandle` declines a query whose base or joined object's read scope carries the leaf, so a `controlled_by_parent` object's analytics query routes to the ObjectQL strategy and serves the find path's rows. `compileScopedFilterToSql` refuses a scope carrying the leaf by name, in its existing `READ_SCOPE_COMPILE_FAILED` / 500 envelope: the `/api/v1/analytics/sql` dry-run echo of a `controlled_by_parent` object now refuses where it rendered the bound id list — it runs no composition and shows no statement it would not run. A filter an author writes keeps its judgement on every face. +- **`@objectstack/driver-turso`.** The remote face resolves a composed leaf nested inside a composed leaf's scope through `toRemoteWhere` with the outer read's options, so a two-level `controlled_by_parent` chain on a remote Turso datasource answers rows instead of `INTERNAL_ERROR` / 500. +- **`@objectstack/spec`.** One nested-leaf row in `READABLE_IDS_CASES`, so every driver's conformance suite measures the shape, and the comparand-shape door's docblock names both callers that pass `admitReadableIds`. No schema, accept set or export of a metadata key moves. diff --git a/docs/adr/0055-master-detail-controlled-by-parent.md b/docs/adr/0055-master-detail-controlled-by-parent.md index 77acf56b423..e992098b057 100644 --- a/docs/adr/0055-master-detail-controlled-by-parent.md +++ b/docs/adr/0055-master-detail-controlled-by-parent.md @@ -2,6 +2,7 @@ **Status**: Accepted (2026-06-19) — implemented in this PR (P0–P2) · **Amended** (2026-09-07, #11082 / PR #11183 — the "Single-level only in v1" limit under Consequences and the "Transitive nested master-detail chains" **Non-goal** are both **reversed**: since `@objectstack/plugin-security` 17.3.0 (merged 2026-08-23) the read derivation and the write gate compose `controlled_by_parent` across a chain, bounded by a cycle guard and a cost ceiling that both fail **closed**. Nothing else in this record changes — §1–§4, the mechanism choice, the set-size limit and the share-table non-goal all stand. See **"Amendment (2026-09-07, #11082): transitive chains compose — the single-level non-goal is reversed"** at the end.) · **Revisited in part** (2026-10-10, [ADR-0139](./0139-engine-internal-semi-join-leaf-readable-parent-ids.md), Proposed) — §2's alternatives row (a) is reversed for ONE engine-internal, non-authorable leaf the engine composes from the compiler's output; the RLS compiler still has no subquery form, and §2's chosen mechanism (b), its `rlsMembership` IN-form and the set-size limit all stand. +· **Amended** (2026-10-22, #24932, ruling B `6303721609` and Q1 C / Q2 A `6329027469`) — **§2's chosen mechanism (b) is retired for `controlled_by_parent`, and the set-size limit with it**: the read derivation (`packages/plugins/plugin-security/src/security-plugin.ts#computeControlledByParentFilter`) answers ADR-0139's `ReadableIds` leaf over the master, the engine composes it from this plugin's own `canReadObject` / `getReadFilter` of the master, and the driver compiles one statement with the master inside a subquery — the seventh consumer of that leaf, in the end-state form. No master id set is pre-resolved any more. The 2026-10-10 line above reads as of its date. What still stands: §1 (the one required `master_detail`), §3 (the write gate), the cycle guard and the depth bound (now the engine's own cut, judged first over the registry), and the RLS compiler's `rlsMembership` IN-form, which serves `current_user.` memberships and never served this derivation. The limit that replaces the set-size ceiling is ADR-0139 D5's two master shapes, which the boot refuses (#25144). **Deciders**: ObjectStack Protocol Architects **Builds on**: [ADR-0049](./0049-no-unenforced-security-properties.md) (enforce-or-remove), [ADR-0054](./0054-runtime-proof-for-authorable-surface.md) (prove-it-runs) **Surfaced by**: an audit of master-detail permission semantics — `OWDModel.controlled_by_parent` is **declared but unenforced** (zero runtime consumers; not reachable through the object's `sharingModel` enum; the RLS compiler is relationship-blind). This is *false compliance* (ADR-0049) and *unproven liveness* (ADR-0054). @@ -72,7 +73,9 @@ A `@objectstack/dogfood` RLS proof, bound in the liveness ledger on `object.shar - **Positive.** Master-detail finally carries permission inheritance (not just cascade/expand); a declared-but-dead OWD value becomes enforced with a permanent runtime guard; zero RLS-compiler changes (lowest-risk path through a security-critical subsystem); composes with tenant/owner/role RLS and analytics; the `OWDModel`↔`sharingModel` inconsistency is resolved. - **Negative / limits (honest).** - **Set-size ceiling.** A user with very many accessible masters produces a large `IN (...)`. Acceptable for typical cardinalities; **large-tenant scale (≫ thousands of masters) is a known limit** — a future share-table/join mechanism would replace the id list. v1 documents this, not solves it. + ⚠️ **Amended 2026-10-22 (#24932) — retired.** There is no id list and no `IN (...)` of master ids any more: the read derivation answers the engine's `ReadableIds` leaf and the driver compiles `masterFK IN (SELECT id FROM master WHERE …)`, so nothing is bound per master and the statement does not grow with the master table. The ceiling this bullet booked was measured before it went (`DATABASE_ERROR` / 500, `too many SQL variables`, past ~32,800 masters on SQLite; 200 with the right rows at 33,000 after). The bullet is kept because it was true for mechanism (b); ⛔ it is no longer a statement of a limit. - **Per-request resolution cost.** One extra master-id query per controlled_by_parent object per request (cached within the request). + ⚠️ **Amended 2026-10-22 (#24932) — retired, and corrected.** There is no extra query: the master is a subquery inside the detail's one statement (one per find, one per count). Before the switch the extra query ran **twice** per request — once for `find`, once more for `count` — and was never cached within the request; the parenthesis described an intent the code did not implement. - **Single-level only in v1.** Nested master-detail chains (a detail whose master is itself a detail) are **not** traversed transitively in v1. ⚠️ **Amended 2026-09-07 — this limit was lifted on 2026-08-23** by #11082 (PR #11183), released in `@objectstack/plugin-security` 17.3.0. The bullet is kept rather than deleted because it was true, and correctly recorded, **for v1**; what changed is the version it describes. ⛔ It is no longer a statement of the **enforced boundary** — both halves walk the chain now. Read the Amendment at the end before deriving any invariant from this line. @@ -118,9 +121,11 @@ So a two-level chain was enforced at level one and open at level two, under meta ### What both halves do now - **Read.** `packages/plugins/plugin-security/src/security-plugin.ts#computeControlledByParentFilter` AND-composes the master's **own** `controlled_by_parent` derivation as a third layer, resolved through **that same method**, so the recursive answer cannot drift from the top-level one. The master set is therefore point-for-point equal to what a direct read of the master returns **at every level** — the equality §2 established for one. + ⚠️ **Amended 2026-10-22 (#24932).** The derivation no longer composes the master's layers itself: it answers the `ReadableIds` leaf over the master, and the **engine** composes the master's scope by asking this plugin's public doors — `canReadObject(master)` and `getReadFilter(master)`, the latter being this same derivation for a master that is itself `controlled_by_parent`, so the chain composes recursively, one nested subquery per hop, inside one statement (ADR-0139 D6 as amended). The equality holds for the same reason, one level up: the scope the engine composes IS the scope a direct read of the master runs under. - **Write.** `packages/plugins/plugin-security/src/security-plugin.ts#assertControlledByParentWrite` walks the chain hop by hop, running the same three master-edit legs — extracted verbatim as `packages/plugins/plugin-security/src/security-plugin.ts#assertMasterRowEditable` — on every hop until it reaches a master that governs its own rows. Each added refusal keeps the `403 PERMISSION_DENIED` envelope named for the **caller's own** object and operation, never for an ancestor the caller never asked about. **Cost.** The per-request resolution cost booked under Consequences is now paid **per hop** — one extra master-id query per `controlled_by_parent` object on the chain, bounded as below. +⚠️ **Amended 2026-10-22 (#24932).** No extra query per hop: each hop is one nested subquery inside the detail's one statement, and the depth bound caps the statement's nesting, not a count of queries. The write gate's per-hop master-row probe is unchanged. ### The two guards, and what the depth bound is NOT diff --git a/docs/adr/0139-engine-internal-semi-join-leaf-readable-parent-ids.md b/docs/adr/0139-engine-internal-semi-join-leaf-readable-parent-ids.md index aed87191b9e..c3e4dcea5f3 100644 --- a/docs/adr/0139-engine-internal-semi-join-leaf-readable-parent-ids.md +++ b/docs/adr/0139-engine-internal-semi-join-leaf-readable-parent-ids.md @@ -241,6 +241,21 @@ chosen up front, never reached by catching a refusal. (ADR-0057 §3.6, `packages/objectql/src/engine.ts#LIFECYCLE_DATASOURCE`): there the leaf serves consumer 1 nothing, and letter C's loud bound is what that deployment class has. Recorded as a limit, not solved. +**The seventh consumer routes nothing to a probe (2026-10-22, #24932 — the plugin-security switch, ruling Q1 C / Q2 A +[`6329027469`](https://github.com/objectstack-ai/objectstack/issues/24932#issuecomment-6329027469)).** +`controlled_by_parent`'s read derivation (`packages/plugins/plugin-security/src/security-plugin.ts#computeControlledByParentFilter`) +answers the leaf for EVERY `controlled_by_parent` object, with no up-front routing to a bounded id list, because the two +classes above cannot occur for its master: the boot **refuses** a `controlled_by_parent` detail whose master is bound to +another datasource (D5.3) or narrowed by an object-scoped read middleware (D5.2) — `ObjectQLPlugin`, on +`kernel:bootstrapped`, `INVALID_METADATA` / 422 ([#25144](https://github.com/objectstack-ai/objectstack/issues/25144), +PR [#25166](https://github.com/objectstack-ai/objectstack/pull/25166)). "Refuses at boot" is the accurate form: the +refusal judges the artifacts present at boot, and an artifact registered later (the `manifest` service, +`metadata:reloaded`) is admitted, its detail then answering this record's D5 refusal at read, never a wrong row set; no +public entry reaches that path today, and the engine lane re-measures it now that the leaf is written on every read. The +plugin's own two guards stay: a metadata cycle and a chain past `CBP_MAX_CHAIN_DEPTH` answer zero rows from a +registry-only walk, made before the leaf is written, so this record's cycle and depth refusals are never reached for a +declared chain; the two bounds are one cut. + ### D6 — What ADR-0055 and ADR-0056 now say ADR-0055's row (a) is reversed **for this one leaf and nothing else**: the leaf is composed by the engine from the @@ -249,7 +264,9 @@ IN-form and deliberate lack of a subquery form are unchanged (`packages/plugins/plugin-security/src/rls-compiler.ts#RLSCompiler`, `#compileExpression`: a subquery shape compiles to `null` and fails closed). §2's mechanism (b) for `controlled_by_parent` stands, set-size ceiling included; whether that id set becomes a seventh consumer was ⛔ not -decided here. ADR-0056's non-goal stands as written for the compiler and is revisited only as the engine-side leaf. +decided here (as accepted; decided 2026-10-21 by ruling B, and landed 2026-10-22 by the plugin-security switch, +#24932 — ADR-0055's own amendment of that date retires its mechanism (b) and the ceiling; D5's seventh-consumer note +records the routing). ADR-0056's non-goal stands as written for the compiler and is revisited only as the engine-side leaf. Each record carries one pointer to this one, landed with this record — ADR-0055's status line, ADR-0056's non-goal bullet; nothing else in either moves. diff --git a/packages/drivers/driver-turso/src/turso-driver.ts b/packages/drivers/driver-turso/src/turso-driver.ts index ad933a4d535..934aafff2b5 100644 --- a/packages/drivers/driver-turso/src/turso-driver.ts +++ b/packages/drivers/driver-turso/src/turso-driver.ts @@ -3383,6 +3383,16 @@ export class TursoDriver extends SqlDriver { * TENANT)` and decides nothing about the wall, so the two faces answer one * row set. An uncomposed leaf, or a composed one outside a `$in`, is refused * first — INTERNAL_ERROR / 500, logged at `error` once per driver. + * + * [ADR-0139 D6 as amended — the nested master chain] A composed leaf's + * scope may itself carry a composed leaf (a `controlled_by_parent` master + * that is `controlled_by_parent` too), so the scope is resolved through + * THIS method, with the OUTER read's options: the nested parent's semi-join + * and its tenant wall are compiled the same way, one level down, and the + * transport nests the subqueries. `replaceComposedReadableIds` returns a + * composed leaf unentered, so without the recursion the nested leaf reached + * the transport unresolved and the remote face refused it (500) where the + * local face answered. */ private toRemoteWhere(object: string, where: unknown, options: DriverOptions | undefined): unknown { const misplaced = findMisplacedReadableIds(where, true); @@ -3390,7 +3400,7 @@ export class TursoDriver extends SqlDriver { const resolved = replaceComposedReadableIds(where, (leaf) => remoteReadableIdsSemiJoin({ object: leaf.object, table: this.remoteTableFor(leaf.object, 'ReadableIds'), - scope: leaf.scope === undefined ? undefined : (this.toRemoteFilter(leaf.object, leaf.scope) as FilterCondition), + scope: leaf.scope === undefined ? undefined : (this.toRemoteWhere(leaf.object, leaf.scope, options) as FilterCondition), tenant: this.remoteTenantScope(leaf.object, options), })); return this.toRemoteFilter(object, resolved); diff --git a/packages/plugins/plugin-security/src/check-only-write-scope.test.ts b/packages/plugins/plugin-security/src/check-only-write-scope.test.ts index ca9dbb5911d..43155d3aebe 100644 --- a/packages/plugins/plugin-security/src/check-only-write-scope.test.ts +++ b/packages/plugins/plugin-security/src/check-only-write-scope.test.ts @@ -63,6 +63,7 @@ import type { PermissionSet } from '@objectstack/spec/security'; import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system'; import { SecurityPlugin } from './security-plugin.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; +import { composeReadableIdsInMemory } from './readable-ids-in-memory.testkit.js'; // ── metadata: the showcase invoice graph, exactly as it ships ────────────── @@ -446,7 +447,15 @@ async function makeStack(opts: { orgScoping?: boolean } = {}): Promise { let result: any[] = []; await securityMw(opCtx, async () => { await sharingMw(opCtx, async () => { - result = await engine.find(object, { where: opCtx.ast.where }); + // [ADR-0139 D6] A controlled_by_parent read carries the engine's + // leaf over the master; this double is no engine, so the composition + // is done for it (`readable-ids-in-memory.testkit.ts`). + const where = await composeReadableIdsInMemory(opCtx.ast.where, opCtx.context, { + rows: (o) => engine._tables[o], + readAdmission: (o, c) => security.canReadObject(o, c), + readScope: (o, c) => security.getReadFilter(o, c), + }); + result = await engine.find(object, { where }); }); }); return result; diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-chain-middle-master.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-chain-middle-master.test.ts index 6ccc8a5f5cc..d9d24235d0b 100644 --- a/packages/plugins/plugin-security/src/controlled-by-parent-chain-middle-master.test.ts +++ b/packages/plugins/plugin-security/src/controlled-by-parent-chain-middle-master.test.ts @@ -67,6 +67,7 @@ import { defaultPermissionSets } from './objects/default-permission-sets.js'; import { SysUserPosition } from './objects/sys-user-position.object.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { relaxEveryRegisteredTenantColumn } from './pre-18-tenant-column.testkit.js'; +import { wireEngineSecuritySource } from './engine-security-source.testkit.js'; const SYS = { context: { isSystem: true } } as never; const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; @@ -281,6 +282,8 @@ beforeAll(async () => { const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); await plugin.init(ctx as never); await plugin.start(ctx as never); + // The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2). + wireEngineSecuritySource(engine, () => plugin); sharing = new SharingService({ engine: engine as never, securityService: () => security as never }); engine.registerMiddleware(buildSharingMiddleware(sharing, ctx.logger) as never, { object: '*' }); }); diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-chain.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-chain.test.ts index b1f1a0efeae..b0017828238 100644 --- a/packages/plugins/plugin-security/src/controlled-by-parent-chain.test.ts +++ b/packages/plugins/plugin-security/src/controlled-by-parent-chain.test.ts @@ -54,6 +54,7 @@ import { SharingService, type SharingEngine } from '@objectstack/plugin-sharing' import { matchesFilterCondition } from '@objectstack/formula'; import type { PermissionSet } from '@objectstack/spec/security'; import { assertEngineFindOnePredicate } from '@objectstack/metadata-core'; +import { composeReadableIdsInMemory } from './readable-ids-in-memory.testkit.js'; const REP = 'usr_rep'; const OTHER = 'usr_other'; @@ -377,8 +378,23 @@ async function boot(options: BootOptions = {}) { const repContext = () => ({ userId: REP, tenantId: 'org-1', positions: [], permissions: [] }); - /** The CRUD READ face: run the middleware, then apply the filter it injected. */ - const visible = async (object: string, context?: any): Promise => { + /** + * [ADR-0139 D6] The derivation answers the engine's `ReadableIds` leaf over + * the master, which the engine composes at its executor boundary from the + * security service's `canReadObject` / `getReadFilter` of the master. This + * double is no engine, so the composition is done for it, the same way + * (`readable-ids-in-memory.testkit.ts`), with the real service answering + * the master's two halves at every level of the chain. + */ + const compose = (where: unknown, context: any) => + composeReadableIdsInMemory(where, context, { + rows: (object) => store.rows[object], + readAdmission: (object, c) => plugin.canReadObject(object, c), + readScope: (object, c) => plugin.getReadFilter(object, c), + }); + + /** Run the middleware for a find of `object` and return the `where` it injected, uncomposed. */ + const inject = async (object: string, context?: any): Promise => { const opCtx: any = { object, operation: 'find', @@ -387,20 +403,29 @@ async function boot(options: BootOptions = {}) { context: context ?? repContext(), }; await middleware(opCtx, async () => {}); + return opCtx.ast.where ?? null; + }; + + /** The CRUD READ face: run the middleware, compose the leaf it injected, then apply the filter. */ + const visible = async (object: string, context?: any): Promise => { + const where = await compose(await inject(object, context), context ?? repContext()); return (store.rows[object] ?? []) - .filter((r) => matchesFilterCondition(r, opCtx.ast.where ?? null)) + .filter((r) => matchesFilterCondition(r, where as any)) .map((r) => String(r.id)); }; /** - * The ANALYTICS read face — the scope `getReadFilter` hands the raw-SQL path. - * No middleware runs here; this method IS the whole enforcement on that - * surface, and it is the THIRD call site of the derivation. + * The ANALYTICS read face — the scope `getReadFilter` hands the analytics + * service, which hands it to the engine (the raw-SQL strategy declines a + * scope carrying the leaf). No middleware runs here; this method IS the + * whole enforcement on that surface, and it is the THIRD call site of the + * derivation. */ const analyticsVisible = async (object: string): Promise => { const filter = await plugin.getReadFilter(object, repContext()); + const where = await compose((filter ?? null) as unknown, repContext()); return (store.rows[object] ?? []) - .filter((r) => matchesFilterCondition(r, (filter ?? null) as any)) + .filter((r) => matchesFilterCondition(r, where as any)) .map((r) => String(r.id)); }; @@ -420,8 +445,9 @@ async function boot(options: BootOptions = {}) { context: { ...repContext(), onBehalfOf: { userId: delegator } }, }; await middleware(opCtx, async () => {}); + const where = await compose(opCtx.ast.where ?? null, opCtx.context); return (store.rows[object] ?? []) - .filter((r) => matchesFilterCondition(r, opCtx.ast.where ?? null)) + .filter((r) => matchesFilterCondition(r, where as any)) .map((r) => String(r.id)); }; @@ -450,7 +476,7 @@ async function boot(options: BootOptions = {}) { return out; }; - return { store, ctx, plugin, repContext, visible, analyticsVisible, delegatedVisible, update, writable }; + return { store, ctx, plugin, repContext, inject, visible, analyticsVisible, delegatedVisible, update, writable }; } // ───────────────────────────────────────────────────────────────────────────── @@ -492,17 +518,34 @@ describe("[#11082] controlled_by_parent composes across a chain — READ", () => it('the DELEGATOR call site composes the chain too — not only the caller\'s', async () => { // The derivation has FOUR consumers (the CRUD middleware's caller leg, its - // D10 delegator leg, and `getReadFilter`); the recursion lives inside the + // D10 delegator leg, and `getReadFilter`); the chain walk lives inside the // shared helper so all of them inherit it. This pins the delegator leg // specifically, because it is the one a fix aimed at "the" call site misses. // - // Agent REP reaches {quote_us (grant), quote_own (ownership)}; delegator - // OTHER owns acct_us and acct_jp, so reaches {quote_us, quote_jp}. The - // delegated read is the INTERSECTION — `line_us` alone. Before the fix the - // delegator's own derivation was org-wide, so the intersection collapsed to - // the agent's set and `line_own` came back too. + // [ADR-0139 D6] Both legs write the leaf over the master, and the engine + // composes it ONCE with the delegated context as it stands, asking + // `getReadFilter(master)` — the ADR-0090 D10 delegated spelling: the + // principal's leg at the ceiling's DECLARED depth ('org' when the ceiling + // declares none, the D10 ruling's rule), the delegator's leg at the + // delegator's own effective depth. So the detail reads exactly the rows a + // delegated direct read of its master reads. Here the ceiling (`crm_rep`) + // declares no depth, so the principal's leg scopes nothing and the + // delegator OTHER's accounts {acct_us, acct_jp} decide: `line_us` and + // `line_jp`. Before the switch the id-list derivation resolved the + // principal's half at the agent's own EFFECTIVE depth ('own' on a private + // master) and answered `line_us` alone — narrower than the delegated direct + // read of the master, the drift ADR-0055 §2's equality forbids. The shape + // is this rig's hand-built one (two different humans as agent and + // delegator); the assembled on-behalf-of context (`@objectstack/core`'s + // `assembleExecutionContext`) makes the agent run AS the delegator, where + // the two spellings agree — pinned on the real stack in + // `controlled-by-parent-leaf-switch.test.ts`. The leak this case guards + // is unchanged: `line_own` (the agent's own account, which the delegator + // cannot read) stays out. const h = await boot(); - expect(await h.delegatedVisible('crm_quote_line_item', OTHER)).toEqual(['line_us']); + const rows = await h.delegatedVisible('crm_quote_line_item', OTHER); + expect(rows).toEqual(['line_us', 'line_jp']); + expect(rows).not.toContain('line_own'); }); it('CONTROL: a detail under a `private` master is untouched — `[]` throughout', async () => { @@ -632,12 +675,15 @@ describe('[#11082] the walk is bounded — both guards fail CLOSED', () => { }); }); - it('the bound caps WORK: resolving the two-level chain costs a bounded number of finds', async () => { - // Guards the cost limit ADR-0055 books, and would redden if the walk ever - // re-entered an ancestor without the visited set catching it. + it('the walk does NO store work: the derivation pre-scans no master, at any level of the chain', async () => { + // [ADR-0139 D6] The derivation answers the leaf and walks the chain over + // the registry alone; the master's rows are read by the engine, inside + // the one statement it composes (here, by the testkit's composition). A + // `find` of a master during the middleware run is the retired pre-scan. const h = await boot(); h.store.findCalls.length = 0; - await h.visible('crm_quote_line_item'); - expect(h.store.findCalls.length).toBeLessThanOrEqual(8); + const injected = await h.inject('crm_quote_line_item'); + expect(h.store.findCalls.filter((o) => o === 'crm_quote' || o === 'crm_account' || o === 'crm_quote_line_item')).toEqual([]); + expect(injected).toBeTruthy(); }); }); diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-every-master-edit.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-every-master-edit.test.ts index d978a2bb9ab..1324e38dee3 100644 --- a/packages/plugins/plugin-security/src/controlled-by-parent-every-master-edit.test.ts +++ b/packages/plugins/plugin-security/src/controlled-by-parent-every-master-edit.test.ts @@ -45,6 +45,7 @@ import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.j import { SecurityPlugin } from './security-plugin.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; +import { wireEngineSecuritySource } from './engine-security-source.testkit.js'; const SYS = { context: { isSystem: true } } as never; const ORG = 'org_cbp_eme'; @@ -240,6 +241,8 @@ async function boot() { const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); await plugin.init(ctx as never); await plugin.start(ctx as never); + // The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2). + wireEngineSecuritySource(engine, () => plugin); sharing = new SharingService({ engine: engine as never, securityService: () => security as never }); engine.registerMiddleware(buildSharingMiddleware(sharing, ctx.logger) as never, { object: '*' }); // The row gates log the writes they refuse. diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts index 6fc3f2f8467..23b686ffb82 100644 --- a/packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts +++ b/packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts @@ -62,6 +62,7 @@ import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.j import { SecurityPlugin } from './security-plugin.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; +import { wireEngineSecuritySource } from './engine-security-source.testkit.js'; const SYS = { context: { isSystem: true } } as never; const ORG = 'org_cbp_erm'; @@ -246,6 +247,8 @@ async function boot() { const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); await plugin.init(ctx as never); await plugin.start(ctx as never); + // The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2). + wireEngineSecuritySource(engine, () => plugin); sharing = new SharingService({ engine: engine as never, securityService: () => security as never }); engine.registerMiddleware(buildSharingMiddleware(sharing, ctx.logger) as never, { object: '*' }); // The row gates log the writes they refuse. diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-leaf-switch.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-leaf-switch.test.ts new file mode 100644 index 00000000000..dc600244428 --- /dev/null +++ b/packages/plugins/plugin-security/src/controlled-by-parent-leaf-switch.test.ts @@ -0,0 +1,632 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0055 · ADR-0139 D6] `controlled_by_parent` is the seventh consumer of + * the engine's `ReadableIds` leaf: the read derivation answers + * `{ masterFK: { $in: ReadableIds.of(master) } }` for every + * `controlled_by_parent` object, the engine composes it at its executor + * boundary from the security service's own `canReadObject` / `getReadFilter` + * of the master, and the driver runs ONE statement with the master inside a + * subquery. No master id list is materialised, nothing is bound per master. + * + * Measured on the real stack: `ObjectQL` over better-sqlite3, the real + * `SecurityPlugin` with the real `SharingService` behind it, direct member + * contexts and the delegated context `resolveAuthzContext` and + * `assembleExecutionContext` assemble. + * + * ## What the id-list form answered before this switch, on this rig + * + * The derivation pre-scanned the master under a system context + * (`select id from [where …]`) and bound every readable id into + * `$in` — once per find and once more per count, with no limit. So a detail + * read ran 2 statements per find (the scan, then the read with the ids + * bound), 4 per find + count, and past the dialect's variable limit + * (33,000 masters here; better-sqlite3's limit is 32,766) both verbs + * answered `DATABASE_ERROR` / 500, `too many SQL variables`. + * + * ## The pins + * + * - every caller shape keeps its row set: plain (restricted by ownership and a + * share), unrestricted (`viewAllRecords`), delegated (the ADR-0090 D10 + * intersection), two-level chain, null and dangling master pointers + * (excluded — ruling B item 4), and a caller the master object does not + * admit (the deny door, `$in: []`); + * - statements per request: find = 1, find + count = 2, no scan of the master + * on its own, the master inside the child's subquery; + * - past the variable limit: 200 with exactly the right rows; + * - a metadata cycle and a chain past the depth bound answer zero rows from + * the plugin's registry walk, never the engine's 500 — and the cut is the + * engine's own: a chain of eight derived objects composes (eight nested + * subqueries in one statement), the ninth is the empty set; + * - a write verb's predicate carries no leaf (ADR-0139 D1): a bulk update of a + * detail lands, narrowed to the rows the caller may read and edit; + * - the scope `getReadFilter` answers carries the branded leaf — one for a + * direct caller, one per leg for a delegated one — so the analytics faces + * receive the same spelling the find path runs. + */ + +import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; +import { AsyncLocalStorage } from 'node:async_hooks'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { resolveAuthzContext, assembleExecutionContext } from '@objectstack/core'; +import { isReadableIds } from '@objectstack/spec/data'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { SysOrganization, SysUser, SysMember } from '@objectstack/platform-objects/identity'; +import { SharingService, buildSharingMiddleware, SysRecordShare } from '@objectstack/plugin-sharing'; + +import { SysPosition, SysPermissionSet, SysPositionPermissionSet } from './__tests__/legacy-catalog-objects.testkit.js'; +import { SysUserPosition } from './objects/sys-user-position.object.js'; +import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; +import { SecurityPlugin } from './security-plugin.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; +import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; + +/** + * The dialects this rig runs on: SQLite always; live PostgreSQL when + * `OS_TEST_POSTGRES_URL` names one (the ruling's measurement condition for + * the switch: driver-sql's readable-ids conformance on PostgreSQL, and this + * rig's row sets and statements on it). The PostgreSQL cell builds its tables + * in a schema of its own and drops it on teardown. PostgreSQL binds up to + * 65,535 parameters, so the bind-limit case below does not discriminate the + * id-list form there; its row set is still read. + */ +interface Cell { readonly label: 'sqlite' | 'postgres'; readonly url?: string } +const PG_URL = process.env.OS_TEST_POSTGRES_URL; +const CELLS: readonly Cell[] = [{ label: 'sqlite' }, ...(PG_URL ? [{ label: 'postgres' as const, url: PG_URL }] : [])]; + +const SYS = { context: { isSystem: true } } as never; +const ORG = 'org_cbp_leaf'; + +/** The master: `private`, owner-scoped by plugin-sharing, with one share. */ +const ACCOUNT = 'qa_cbp_leaf_account'; +/** A one-hop detail of ACCOUNT. */ +const CONTACT = 'qa_cbp_leaf_contact'; +/** A derived master: `controlled_by_parent` of ACCOUNT. */ +const QUOTE = 'qa_cbp_leaf_quote'; +/** The two-level chain's leaf: `controlled_by_parent` of QUOTE. */ +const LINE = 'qa_cbp_leaf_line'; +/** The bind-limit pair: a master with more rows than the dialect binds, and its detail. */ +const BIG_MASTER = 'qa_cbp_leaf_big_master'; +const BIG_CHILD = 'qa_cbp_leaf_big_child'; +/** Past better-sqlite3's `SQLITE_MAX_VARIABLE_NUMBER` (32,766). */ +const BIG_MASTERS = 33_000; +/** A two-object metadata cycle. */ +const CYC_A = 'qa_cbp_leaf_cyc_a'; +const CYC_B = 'qa_cbp_leaf_cyc_b'; +/** A generated chain `deep_0 → … → deep_9` of derived objects over a `private` root `deep_10`. */ +const DEEP = 'qa_cbp_leaf_deep'; +const DEEP_LINKS = 10; + +const HUMAN = 'u_cbp_leaf_human'; +const OTHER = 'u_cbp_leaf_other'; +const VIEW_ALL = 'u_cbp_leaf_view_all'; +const NO_ACCOUNT_READ = 'u_cbp_leaf_no_account_read'; + +const SET = { + /** Read on every object, edit on the account and its contacts; silent on rows. */ + member: 'qa_cbp_leaf_member', + /** The same, with `viewAllRecords` on the account: an unrestricted master scope. */ + viewAll: 'qa_cbp_leaf_view_all', + /** Read on the details, not on the account. */ + noAccount: 'qa_cbp_leaf_no_account', + /** The agent's ceiling: read everywhere, a content rule on the account's rows. */ + ceiling: 'qa_cbp_leaf_ceiling', +} as const; + +type Outcome = { kind: 'landed'; value: unknown } | { kind: 'refused'; code?: string; status?: number; message?: string }; + +const outcomeOf = (p: Promise): Promise => + p.then( + (value) => ({ kind: 'landed' as const, value }), + (e: unknown) => { + const x = e as { code?: unknown; status?: unknown; statusCode?: unknown; message?: unknown } | null; + const status = x?.statusCode ?? x?.status; + return { + kind: 'refused' as const, + code: x?.code == null ? undefined : String(x.code), + status: status == null ? undefined : Number(status), + message: x?.message == null ? undefined : String(x.message), + }; + }, + ); + +const releaseAsyncContexts = (...owners: object[]): void => { + for (const owner of owners) { + for (const value of Object.values(owner)) if (value instanceof AsyncLocalStorage) value.disable(); + } +}; + +const ids = (rows: unknown): string[] => + (Array.isArray(rows) ? rows : [rows]) + .filter(Boolean) + .map((r) => String((r as { id: unknown }).id)) + .sort(); + +const TEXT = (name: string) => ({ name, type: 'text' }); +const OWNER = { name: 'owner_id', type: 'lookup', reference: 'sys_user' }; +const ID = { name: 'id', type: 'text', primaryKey: true }; +const masterOf = (field: string, master: string) => ({ name: field, type: 'master_detail', reference: master, required: true }); + +function deepChain(): Record[] { + const out: Record[] = []; + for (let k = 0; k < DEEP_LINKS; k++) { + out.push({ + name: `${DEEP}_${k}`, + label: `Deep ${k}`, + sharingModel: 'controlled_by_parent', + fields: { id: ID, up: masterOf('up', `${DEEP}_${k + 1}`) }, + }); + } + out.push({ name: `${DEEP}_${DEEP_LINKS}`, label: 'Deep root', sharingModel: 'private', fields: { id: ID, owner_id: OWNER } }); + return out; +} + +async function boot(cell: Cell) { + const engine = new ObjectQL(); + let dropSchema: (() => Promise) | undefined; + if (cell.label === 'postgres') { + const schema = `qa_cbp_leaf_${Date.now().toString(36)}_${Math.floor(Math.random() * 1e6).toString(36)}`; + const admin = new SqlDriver({ client: 'pg', connection: cell.url, pool: { min: 0, max: 1 } } as never); + await admin.execute(`create schema "${schema}"`); + dropSchema = async () => { + try { await admin.execute(`drop schema "${schema}" cascade`); } finally { await admin.disconnect?.(); } + }; + engine.registerDriver(new SqlDriver({ client: 'pg', connection: cell.url, searchPath: [schema] } as never) as never, true); + } else { + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) as never, + true, + ); + } + await engine.init(); + const everyDeep = Array.from({ length: DEEP_LINKS + 1 }, (_, k) => `${DEEP}_${k}`); + engine.registerApp({ + id: 'com.objectstack.qa.controlled-by-parent-leaf-switch', + name: 'controlled_by_parent answers the ReadableIds leaf', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + SysPosition, SysPermissionSet, SysPositionPermissionSet, SysUserPosition, SysUserPermissionSet, + SysOrganization, SysUser, SysMember, SysRecordShare, + { name: ACCOUNT, label: 'Account', sharingModel: 'private', fields: { id: ID, name: TEXT('name'), owner_id: OWNER } }, + { + name: CONTACT, label: 'Contact', sharingModel: 'controlled_by_parent', + fields: { id: ID, note: TEXT('note'), account: masterOf('account', ACCOUNT) }, + }, + { + name: QUOTE, label: 'Quote', sharingModel: 'controlled_by_parent', + fields: { id: ID, name: TEXT('name'), account: masterOf('account', ACCOUNT) }, + }, + { + name: LINE, label: 'Quote line', sharingModel: 'controlled_by_parent', + fields: { id: ID, note: TEXT('note'), quote: masterOf('quote', QUOTE) }, + }, + { name: BIG_MASTER, label: 'Big master', sharingModel: 'private', fields: { id: ID, name: TEXT('name'), owner_id: OWNER } }, + { + name: BIG_CHILD, label: 'Big child', sharingModel: 'controlled_by_parent', + fields: { id: ID, note: TEXT('note'), master: masterOf('master', BIG_MASTER) }, + }, + { name: CYC_A, label: 'Cycle A', sharingModel: 'controlled_by_parent', fields: { id: ID, b: masterOf('b', CYC_B) } }, + { name: CYC_B, label: 'Cycle B', sharingModel: 'controlled_by_parent', fields: { id: ID, a: masterOf('a', CYC_A) } }, + ...deepChain(), + ], + } as never); + await engine.syncSchemas(); + // The organization-less permission-set catalogue row below exists only on a + // store built before 18.x (ADR-0131 D1 makes a fresh tenant column NOT + // NULL): that table is this store's. + await relaxTenantColumns(engine as never, ['sys_permission_set']); + + const e = engine as unknown as { + insert: (o: string, row: Record, opts: never) => Promise; + getDriver: (o: string) => { + create: (o: string, row: Record) => Promise; + knex: ((table: string) => { insert: (rows: Record[]) => Promise }) + & { on: (ev: string, fn: (q: { sql?: unknown }) => void) => void; removeListener: (ev: string, fn: (q: { sql?: unknown }) => void) => void }; + }; + logger: { warn: () => void }; + }; + await e.insert('sys_organization', { id: ORG, name: ORG, slug: ORG }, SYS); + for (const id of [HUMAN, OTHER, VIEW_ALL, NO_ACCOUNT_READ]) { + await e.insert('sys_user', { id, name: id, email: `${id}@example.test` }, SYS); + await e.insert('sys_member', { id: `mem_${id}`, user_id: id, organization_id: ORG, role: 'member' }, SYS); + } + const READ = { allowRead: true } as const; + const READ_EDIT = { allowRead: true, allowEdit: true } as const; + const detailObjects = [CONTACT, QUOTE, LINE, BIG_CHILD, CYC_A, CYC_B, ...everyDeep]; + const readAll = Object.fromEntries([ACCOUNT, BIG_MASTER, ...detailObjects].map((o) => [o, READ])); + const sets = [ + ...defaultPermissionSets, + PermissionSetSchema.parse({ name: SET.member, objects: { ...readAll, [ACCOUNT]: READ_EDIT, [CONTACT]: READ_EDIT } }), + PermissionSetSchema.parse({ name: SET.viewAll, objects: { ...readAll, [ACCOUNT]: { ...READ, viewAllRecords: true } } }), + PermissionSetSchema.parse({ name: SET.noAccount, objects: Object.fromEntries(detailObjects.map((o) => [o, READ])) }), + PermissionSetSchema.parse({ + name: SET.ceiling, + objects: readAll, + rowLevelSecurity: [ + { name: 'qa_cbp_leaf_ceiling_open_only', object: ACCOUNT, operation: 'select', using: "record.name == 'open'" }, + ], + }), + ]; + // The delegators hold their sets through stored grants, as a user does. + for (const name of [SET.member, SET.noAccount]) { + await e.getDriver('sys_permission_set').create('sys_permission_set', { + id: `ps_${name}`, name, label: name, organization_id: null, managed_by: 'admin', active: true, + object_permissions: JSON.stringify({}), system_permissions: JSON.stringify([]), + }); + } + for (const [user, set] of [[HUMAN, SET.member], [OTHER, SET.member], [NO_ACCOUNT_READ, SET.noAccount]] as const) { + await e.insert('sys_user_permission_set', { id: `ups_${user}`, user_id: user, permission_set: set, organization_id: ORG }, SYS); + } + + // The accounts: two of HUMAN's (one 'open', one 'secret' — the ceiling's + // rule tells them apart), one of OTHER's, and one of OTHER's shared to + // HUMAN for read. + await e.insert(ACCOUNT, { id: 'a_open', name: 'open', owner_id: HUMAN }, SYS); + await e.insert(ACCOUNT, { id: 'a_secret', name: 'secret', owner_id: HUMAN }, SYS); + await e.insert(ACCOUNT, { id: 'a_other', name: 'open', owner_id: OTHER }, SYS); + await e.insert(ACCOUNT, { id: 'a_shared', name: 'open', owner_id: OTHER }, SYS); + await e.insert('sys_record_share', { + id: 'shr_cbp_leaf_1', object_name: ACCOUNT, record_id: 'a_shared', recipient_type: 'user', recipient_id: HUMAN, access_level: 'read', + }, SYS); + // The details, one per account, plus a null and a dangling master pointer + // (written through the driver: the required master is what the authoring + // door refuses, and these rows exist to be excluded). A driver write is not + // stamped, so it names the rig's organization itself. + const direct = async (object: string, rows: Record[]) => { + for (const row of rows) await e.getDriver(object).create(object, { ...row, organization_id: ORG }); + }; + await direct(CONTACT, [ + { id: 'c_open', note: '', account: 'a_open' }, { id: 'c_secret', note: '', account: 'a_secret' }, + { id: 'c_other', note: '', account: 'a_other' }, { id: 'c_shared', note: '', account: 'a_shared' }, + { id: 'c_null', note: '', account: null }, { id: 'c_dangling', note: '', account: 'a_missing' }, + ]); + await direct(QUOTE, [ + { id: 'q_open', name: 'q', account: 'a_open' }, { id: 'q_other', name: 'q', account: 'a_other' }, + { id: 'q_null', name: 'q', account: null }, { id: 'q_dangling', name: 'q', account: 'a_missing' }, + ]); + await direct(LINE, [ + { id: 'l_open', note: '', quote: 'q_open' }, { id: 'l_other', note: '', quote: 'q_other' }, + { id: 'l_via_null', note: '', quote: 'q_null' }, { id: 'l_via_dangling', note: '', quote: 'q_dangling' }, + { id: 'l_null', note: '', quote: null }, { id: 'l_dangling', note: '', quote: 'q_missing' }, + ]); + // The bind-limit population, straight through the driver's connection: + // BIG_MASTERS masters HUMAN owns, one OTHER owns, and three children. + const knex = e.getDriver(BIG_MASTER).knex; + const bigRows: Record[] = []; + for (let i = 0; i < BIG_MASTERS; i++) { + bigRows.push({ id: `m${String(i).padStart(5, '0')}`, name: 'open', owner_id: HUMAN, organization_id: ORG }); + } + bigRows.push({ id: 'm_other', name: 'open', owner_id: OTHER, organization_id: ORG }); + for (let i = 0; i < bigRows.length; i += 500) await knex(BIG_MASTER).insert(bigRows.slice(i, i + 500)); + await direct(BIG_CHILD, [ + { id: 'bc1', note: '', master: 'm00001' }, { id: 'bc2', note: '', master: 'm00002' }, { id: 'bc3', note: '', master: 'm_other' }, + ]); + // The cycle, and the deep chain whose root HUMAN owns: any empty answer on + // it is the bound talking, not the fixture. + await direct(CYC_A, [{ id: 'cyc_a1', b: 'cyc_b1' }]); + await direct(CYC_B, [{ id: 'cyc_b1', a: 'cyc_a1' }]); + for (let k = 0; k < DEEP_LINKS; k++) await direct(`${DEEP}_${k}`, [{ id: `${DEEP}_${k}_r`, up: `${DEEP}_${k + 1}_r` }]); + await direct(`${DEEP}_${DEEP_LINKS}`, [{ id: `${DEEP}_${DEEP_LINKS}_r`, owner_id: HUMAN }]); + + let security: { + getReadFilter: (o: string, c: unknown) => Promise | undefined>; + canReadObject: (o: string, c: unknown) => Promise; + } | undefined; + let sharing: SharingService | undefined; + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (type: string, name: string) => + type === 'permission' ? sets.find((s: { name?: string }) => s.name === name) ?? null : engine.getSchema(name) ?? null, + list: async () => sets, + }, + tenancy: { posture: 'single' }, + get sharing() { return sharing; }, + }; + const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + const ctx = { + logger, + registerService: (name: string, svc: unknown) => { + if (name === 'security') security = svc as typeof security; + else services[name] = svc; + }, + hook: () => undefined, + 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 as never); + await plugin.start(ctx as never); + // What `ObjectQLPlugin.init` does on a kernel: put the SOURCE of the + // `security` service in the engine's slot, read at every composition. + (engine as unknown as Record)[Symbol.for('objectstack.objectql.securityServiceSource')] = () => security; + sharing = new SharingService({ engine: engine as never, securityService: () => security as never }); + engine.registerMiddleware(buildSharingMiddleware(sharing, ctx.logger) as never, { object: '*' }); + vi.spyOn(e.logger, 'warn').mockImplementation(() => undefined); + + /** A delegated context for `userId`, as the OAuth door assembles it, the agent holding `ceiling` as its only grant. */ + const onBehalfOf = async (userId: string, ceiling: string = SET.ceiling) => { + const authz = await resolveAuthzContext({ + userAuthGates: undefined, + apiKeyLifecycle: undefined, + ql: engine, + headers: {}, + getSession: async () => ({ + user: { id: userId }, + session: { id: `sess_${userId}`, userId, activeOrganizationId: ORG }, + }), + tenancyPosture: 'single', + } as never); + return assembleExecutionContext({ + authz, + oauth: { userId, scopes: ['data:read'], clientId: 'qa_agent_client', scopePermissions: [ceiling], delegatesActions: false }, + localization: undefined, + requestLocale: undefined, + accessToken: undefined, + } as never)!; + }; + + return { + engine, + logger, + onBehalfOf, + read: async (object: string, context: unknown) => + ids(await engine.find(object, { context: { ...(context as object) } } as never)), + count: (object: string, context: unknown) => engine.count(object, { context: { ...(context as object) } } as never), + readOutcome: (object: string, context: unknown) => outcomeOf(engine.find(object, { context: { ...(context as object) } } as never)), + countOutcome: (object: string, context: unknown) => outcomeOf(engine.count(object, { context: { ...(context as object) } } as never)), + readFilter: (object: string, context: unknown) => security!.getReadFilter(object, { ...(context as object) }), + canReadObject: (object: string, context: unknown) => security!.canReadObject(object, { ...(context as object) }), + /** The SQL statements the driver ran during `run`, as their text. */ + statementsDuring: async (run: () => Promise): Promise => { + const seen: string[] = []; + const onQuery = (q: { sql?: unknown }) => { seen.push(String(q?.sql ?? '')); }; + knex.on('query', onQuery); + try { await run(); } finally { knex.removeListener('query', onQuery); } + return seen; + }, + teardown: async () => { + try { await engine.destroy(); } catch { /* noop */ } + releaseAsyncContexts(engine, plugin); + await dropSchema?.(); + }, + }; +} + +type Rig = Awaited>; + +const member = (userId: string, set: string) => ({ userId, positions: [], permissions: [set], posture: 'MEMBER' }); +/** The table a statement's FIRST `from` names — the table it reads, not one inside a subquery of it (SQLite backticks, PostgreSQL double quotes). */ +const fromTable = (sql: string): string | undefined => /^select .*? from ["`]?([a-z0-9_]+)["`]?/i.exec(sql)?.[1]; +/** A statement that reads `table` — not one that reads another table with `table` inside a subquery. */ +const readsTable = (table: string) => (sql: string) => fromTable(sql) === table; +/** The leaves a scope carries, as the objects they name, in walk order. */ +function leavesIn(node: unknown, out: string[] = []): string[] { + if (node === null || typeof node !== 'object') return out; + if (Array.isArray(node)) { for (const item of node) leavesIn(item, out); return out; } + for (const [key, value] of Object.entries(node as Record)) { + if (key === '$in' && isReadableIds(value)) out.push(value.object); + else leavesIn(value, out); + } + return out; +} + +for (const cell of CELLS) describe(`on ${cell.label}`, () => { + describe('[ADR-0139 D6] controlled_by_parent answers the leaf: every caller shape keeps its row set', () => { + let rig: Rig; + beforeAll(async () => { rig = await boot(cell); }, 120_000); + afterAll(async () => { await rig.teardown(); }); + + it('the accounts, measured apart: the owner-match plus the one share for each identity, and every row for view-all', async () => { + expect(await rig.read(ACCOUNT, member(HUMAN, SET.member))).toEqual(['a_open', 'a_secret', 'a_shared']); + expect(await rig.read(ACCOUNT, member(OTHER, SET.member))).toEqual(['a_other', 'a_shared']); + expect(await rig.read(ACCOUNT, member(VIEW_ALL, SET.viewAll))).toEqual(['a_open', 'a_other', 'a_secret', 'a_shared']); + }); + + it('a restricted caller reads the contacts of the accounts it may read — find and count, both identities', async () => { + for (const [user, expected] of [ + [HUMAN, ['c_open', 'c_secret', 'c_shared']], + [OTHER, ['c_other', 'c_shared']], + ] as const) { + const context = member(user, SET.member); + expect(await rig.read(CONTACT, context), user).toEqual([...expected]); + expect(await rig.count(CONTACT, context), user).toBe(expected.length); + } + }); + + it('an unrestricted master scope (viewAllRecords) reads every contact whose account exists — never the null or dangling pointer (ruling B item 4)', async () => { + const context = member(VIEW_ALL, SET.viewAll); + expect(await rig.read(CONTACT, context)).toEqual(['c_open', 'c_other', 'c_secret', 'c_shared']); + expect(await rig.count(CONTACT, context)).toBe(4); + }); + + it('a caller the master object does not admit reads no contact: canReadObject(ACCOUNT) is false, the leaf is $in: []', async () => { + const context = member(NO_ACCOUNT_READ, SET.noAccount); + expect(await rig.readOutcome(ACCOUNT, context)).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); + expect(await rig.canReadObject(ACCOUNT, context)).toBe(false); + expect(await rig.read(CONTACT, context)).toEqual([]); + expect(await rig.count(CONTACT, context)).toBe(0); + }); + + it("a delegated caller reads the intersection: the delegator's accounts the ceiling admits", async () => { + const context = await rig.onBehalfOf(HUMAN); + expect(await rig.read(ACCOUNT, context)).toEqual(['a_open', 'a_shared']); + expect(await rig.read(CONTACT, context)).toEqual(['c_open', 'c_shared']); + expect(await rig.count(CONTACT, context)).toBe(2); + }); + + it('the two-level chain composes: a line is readable where its quote is, and a quote where its account is', async () => { + expect(await rig.read(QUOTE, member(HUMAN, SET.member))).toEqual(['q_open']); + expect(await rig.read(LINE, member(HUMAN, SET.member))).toEqual(['l_open']); + expect(await rig.read(LINE, member(OTHER, SET.member))).toEqual(['l_other']); + expect(await rig.read(LINE, member(VIEW_ALL, SET.viewAll))).toEqual(['l_open', 'l_other']); + expect(await rig.read(LINE, await rig.onBehalfOf(HUMAN))).toEqual(['l_open']); + expect(await rig.count(LINE, member(VIEW_ALL, SET.viewAll))).toBe(2); + }); + + it('CONTROL — a system context reads every detail whose master pointer is set, the chain composed with no restriction', async () => { + expect(await rig.read(CONTACT, { isSystem: true })).toEqual(['c_dangling', 'c_null', 'c_open', 'c_other', 'c_secret', 'c_shared']); + }); + }); + + describe('[ADR-0139 D6] statements per request: one per find, two per find + count, no scan of the master', () => { + let rig: Rig; + beforeAll(async () => { rig = await boot(cell); }, 120_000); + afterAll(async () => { await rig.teardown(); }); + + const shapes = (): Array<[string, string, unknown | Promise, string]> => [ + ['plain', CONTACT, member(HUMAN, SET.member), ACCOUNT], + ['unrestricted', CONTACT, member(VIEW_ALL, SET.viewAll), ACCOUNT], + ['two-level chain', LINE, member(HUMAN, SET.member), QUOTE], + ]; + + it.each(shapes())('%s: the find is one statement, the master inside its subquery, and find + count is two', async (_shape, object, contextOrPromise, master) => { + const context = await contextOrPromise; + const reading = (statements: string[]) => statements.filter((sql) => readsTable(object)(sql) || readsTable(master)(sql) || readsTable(ACCOUNT)(sql)); + const find = await rig.statementsDuring(() => rig.engine.find(object, { context: { ...(context as object) } } as never)); + const findAll = `find:\n${find.join('\n')}`; + expect(reading(find), findAll).toHaveLength(1); + expect(reading(find)[0], findAll).toMatch(new RegExp(`^select .* from .${object}. where .*in \\(select .id. from .${master}.`, 'i')); + // No statement reads the master on its own: the leaf stands where the + // pre-resolved id list stood, so the master is never pre-scanned. + expect(find.filter(readsTable(master)), findAll).toHaveLength(0); + expect(find.filter(readsTable(ACCOUNT)), findAll).toHaveLength(0); + const both = await rig.statementsDuring(async () => { + await rig.engine.find(object, { context: { ...(context as object) } } as never); + await rig.engine.count(object, { context: { ...(context as object) } } as never); + }); + const bothAll = `find + count:\n${both.join('\n')}`; + expect(reading(both), bothAll).toHaveLength(2); + for (const sql of reading(both)) expect(sql, bothAll).toMatch(/in \(select/i); + }); + + it('a delegated find is one statement too: both halves write the leaf, the engine composes the master once, the subquery is carried twice', async () => { + const context = await rig.onBehalfOf(HUMAN); + const find = await rig.statementsDuring(() => rig.engine.find(CONTACT, { context: { ...context } } as never)); + const all = `delegated find:\n${find.join('\n')}`; + const reads = find.filter((sql) => readsTable(CONTACT)(sql) || readsTable(ACCOUNT)(sql)); + expect(reads, all).toHaveLength(1); + expect(reads[0].match(new RegExp(`in \\(select .id. from .${ACCOUNT}.`, 'gi')) ?? [], all).toHaveLength(2); + expect(find.filter(readsTable(ACCOUNT)), all).toHaveLength(0); + }); + + it('the two-level chain is one statement with the subqueries nested: quote inside the line, account inside the quote', async () => { + const find = await rig.statementsDuring(() => rig.engine.find(LINE, { context: member(HUMAN, SET.member) } as never)); + const reads = find.filter(readsTable(LINE)); + expect(reads, find.join('\n')).toHaveLength(1); + expect(reads[0]).toMatch(new RegExp(`in \\(select .id. from .${QUOTE}. where .*in \\(select .id. from .${ACCOUNT}.`, 'i')); + }); + }); + + describe('[ADR-0139 D6] past the dialect\'s variable limit', () => { + let rig: Rig; + beforeAll(async () => { rig = await boot(cell); }, 180_000); + afterAll(async () => { await rig.teardown(); }); + + it(`a detail under ${BIG_MASTERS} readable masters answers 200 with exactly the right rows, and count agrees`, async () => { + const context = member(HUMAN, SET.member); + expect(await rig.read(BIG_MASTER, { isSystem: true })).toHaveLength(BIG_MASTERS + 1); + const find = await rig.readOutcome(BIG_CHILD, context); + expect(find.kind, JSON.stringify(find)).toBe('landed'); + expect(ids((find as { value: unknown }).value)).toEqual(['bc1', 'bc2']); + const count = await rig.countOutcome(BIG_CHILD, context); + expect(count).toEqual({ kind: 'landed', value: 2 }); + }); + + it('…in one statement that binds the owner, never a master id', async () => { + const find = await rig.statementsDuring(() => rig.engine.find(BIG_CHILD, { context: member(HUMAN, SET.member) } as never)); + const reads = find.filter(readsTable(BIG_CHILD)); + expect(reads, find.join('\n')).toHaveLength(1); + expect(reads[0]).toMatch(new RegExp(`in \\(select .id. from .${BIG_MASTER}. where`, 'i')); + expect(find.filter(readsTable(BIG_MASTER))).toHaveLength(0); + }); + }); + + describe('[ADR-0055 amendment] the two guards answer zero rows from the registry walk, never the engine\'s refusal', () => { + let rig: Rig; + beforeAll(async () => { rig = await boot(cell); }, 120_000); + afterAll(async () => { await rig.teardown(); }); + + it('a metadata cycle: zero rows, 200, reported as a cycle, and no statement scans either object', async () => { + rig.logger.error.mockClear(); + const statements = await rig.statementsDuring(async () => { + expect(await rig.readOutcome(CYC_A, member(HUMAN, SET.member))).toMatchObject({ kind: 'landed', value: [] }); + expect(await rig.countOutcome(CYC_B, member(HUMAN, SET.member))).toEqual({ kind: 'landed', value: 0 }); + }); + const messages = rig.logger.error.mock.calls.map((c: unknown[]) => String(c[0])); + expect(messages.some((m) => m.includes('CYCLE') && m.includes(CYC_A))).toBe(true); + expect(statements.filter((sql) => /in \(select/i.test(sql))).toHaveLength(0); + }); + + it('a chain past the depth bound: zero rows, 200, reported naming the bound', async () => { + rig.logger.error.mockClear(); + expect(await rig.readOutcome(`${DEEP}_0`, member(HUMAN, SET.member))).toMatchObject({ kind: 'landed', value: [] }); + expect(await rig.readOutcome(`${DEEP}_1`, member(HUMAN, SET.member))).toMatchObject({ kind: 'landed', value: [] }); + const messages = rig.logger.error.mock.calls.map((c: unknown[]) => String(c[0])); + expect(messages.some((m) => m.includes('depth bound') && m.includes(`${DEEP}_0`))).toBe(true); + }); + + it("the cut is the engine's: eight derived objects compose into one statement with eight nested subqueries, and the row is served", async () => { + // `deep_2 → deep_3 → … → deep_9` are eight derived objects over the + // `private` root HUMAN owns. The engine composes a chain at most eight + // scopes deep; the plugin's bound refuses the ninth (`deep_1`, above). + const context = member(HUMAN, SET.member); + const statements = await rig.statementsDuring(async () => { + expect(await rig.read(`${DEEP}_2`, context)).toEqual([`${DEEP}_2_r`]); + }); + const reads = statements.filter(readsTable(`${DEEP}_2`)); + expect(reads, statements.join('\n')).toHaveLength(1); + expect(reads[0].match(/in \(select/gi) ?? []).toHaveLength(8); + expect(await rig.read(`${DEEP}_${DEEP_LINKS - 1}`, context)).toEqual([`${DEEP}_${DEEP_LINKS - 1}_r`]); + }); + }); + + describe('[ADR-0139 D1] a write verb\'s predicate carries no leaf', () => { + let rig: Rig; + beforeAll(async () => { rig = await boot(cell); }, 120_000); + afterAll(async () => { await rig.teardown(); }); + + it('a bulk update of the contacts lands, narrowed to the rows whose account the caller may read and edit', async () => { + const context = member(OTHER, SET.member); + const outcome = await outcomeOf(rig.engine.update(CONTACT, { note: 'bulk' }, { where: {}, multi: true, context: { ...context } } as never)); + expect(outcome.kind, JSON.stringify(outcome)).toBe('landed'); + const after = (await rig.engine.find(CONTACT, { context: { isSystem: true } } as never)) as Array<{ id: string; note: string }>; + const noted = after.filter((r) => r.note === 'bulk').map((r) => r.id).sort(); + expect(noted).toEqual(['c_other', 'c_shared']); + }); + }); + + describe('[ADR-0139 D6] the scope getReadFilter answers carries the leaf — the spelling the analytics faces receive', () => { + let rig: Rig; + beforeAll(async () => { rig = await boot(cell); }, 120_000); + afterAll(async () => { await rig.teardown(); }); + + it('a direct caller: one leaf over the master, and nothing bound per master', async () => { + const scope = await rig.readFilter(CONTACT, member(HUMAN, SET.member)); + expect(leavesIn(scope)).toEqual([ACCOUNT]); + expect(JSON.stringify(scope)).not.toMatch(/a_open|a_secret|a_shared/); + }); + + it('a delegated caller: one leaf per leg, both over the master', async () => { + const scope = await rig.readFilter(CONTACT, await rig.onBehalfOf(HUMAN)); + expect(leavesIn(scope)).toEqual([ACCOUNT, ACCOUNT]); + }); + + it("a derived master's scope carries the leaf over ITS master, so the chain composes inside the engine", async () => { + expect(leavesIn(await rig.readFilter(QUOTE, member(HUMAN, SET.member)))).toEqual([ACCOUNT]); + expect(leavesIn(await rig.readFilter(LINE, member(HUMAN, SET.member)))).toEqual([QUOTE]); + }); + + it('a caller the master object does not admit still receives the leaf; the engine answers it $in: [] (canReadObject false)', async () => { + const context = member(NO_ACCOUNT_READ, SET.noAccount); + expect(leavesIn(await rig.readFilter(CONTACT, context))).toEqual([ACCOUNT]); + expect(await rig.canReadObject(ACCOUNT, context)).toBe(false); + }); + }); +}); diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-master-object-read.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-master-object-read.test.ts index bf6a3ae9d24..9a06ff2e102 100644 --- a/packages/plugins/plugin-security/src/controlled-by-parent-master-object-read.test.ts +++ b/packages/plugins/plugin-security/src/controlled-by-parent-master-object-read.test.ts @@ -43,8 +43,10 @@ import { SysPosition, SysPermissionSet, SysPositionPermissionSet } from './__tes import { SysUserPosition } from './objects/sys-user-position.object.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { SecurityPlugin } from './security-plugin.js'; +import { isReadableIds } from '@objectstack/spec/data'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; +import { wireEngineSecuritySource } from './engine-security-source.testkit.js'; const SYS = { context: { isSystem: true } } as never; const ORG = 'org_cbp_mor'; @@ -237,6 +239,8 @@ async function boot() { const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); await plugin.init(ctx as never); await plugin.start(ctx as never); + // The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2). + wireEngineSecuritySource(engine, () => plugin); // The row gates log the writes they refuse. vi.spyOn(e.logger, 'warn').mockImplementation(() => undefined); @@ -277,10 +281,19 @@ async function boot() { ids(await engine.findOne(object, { where: { id }, context: { ...(context as object) } } as never)), count: (object: string, context: unknown) => engine.count(object, { context: { ...(context as object) } } as never), readMaster: (context: unknown) => outcomeOf(engine.find(MASTER, { context: { ...(context as object) } } as never)), - /** The ids `getReadFilter`'s scope admits, read with it as the only predicate. */ + /** + * The ids `getReadFilter`'s scope admits for `context`. [ADR-0139 D6] The + * scope carries the engine's leaf over the master, which the engine + * composes for the context of the READ — so it is run under the caller's + * own context, as the analytics engine-bound face runs it, never under a + * system one (where the leaf composes as "no restriction" for the system). + * The find is also scoped by the middleware; what this measures is that + * the scope carries the leaf and the composed read admits what it should. + */ readThroughReadFilter: async (object: string, context: unknown) => { const scope = await security!.getReadFilter(object, { ...(context as object) }); - return ids(await engine.find(object, { where: scope ?? {}, context: { isSystem: true } } as never)); + expect(leavesIn(scope)).toEqual([MASTER]); + return ids(await engine.find(object, { where: scope ?? {}, context: { ...(context as object) } } as never)); }, stored: async (object: string, id: string) => ((await engine.findOne(object, { where: { id }, context: { isSystem: true } } as never)) ?? null) as Record | null, @@ -295,6 +308,16 @@ async function boot() { type Rig = Awaited>; const member = (userId: string, set: string) => ({ userId, positions: [], permissions: [set], posture: 'MEMBER' }); +/** The leaves a scope carries, as the objects they name. */ +function leavesIn(node: unknown, out: string[] = []): string[] { + if (node === null || typeof node !== 'object') return out; + if (Array.isArray(node)) { for (const item of node) leavesIn(item, out); return out; } + for (const [key, value] of Object.entries(node as Record)) { + if (key === '$in' && isReadableIds(value)) out.push(value.object); + else leavesIn(value, out); + } + return out; +} /** Two principals the master's own read door refuses at the object gate. */ const UNADMITTED = [ diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-master-object-write.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-master-object-write.test.ts index 96a980ff414..2a1cc86d6b8 100644 --- a/packages/plugins/plugin-security/src/controlled-by-parent-master-object-write.test.ts +++ b/packages/plugins/plugin-security/src/controlled-by-parent-master-object-write.test.ts @@ -39,6 +39,7 @@ import { SysUserPosition } from './objects/sys-user-position.object.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { SecurityPlugin } from './security-plugin.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; +import { wireEngineSecuritySource } from './engine-security-source.testkit.js'; const SYS = { context: { isSystem: true } } as never; const ORG = 'org_cbp_mow'; @@ -211,6 +212,8 @@ async function boot() { const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); await plugin.init(ctx as never); await plugin.start(ctx as never); + // The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2). + wireEngineSecuritySource(engine, () => plugin); // The row gates log the writes they refuse. vi.spyOn(e.logger, 'warn').mockImplementation(() => undefined); diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-sharing.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-sharing.test.ts index cac1868247c..078234a4a67 100644 --- a/packages/plugins/plugin-security/src/controlled-by-parent-sharing.test.ts +++ b/packages/plugins/plugin-security/src/controlled-by-parent-sharing.test.ts @@ -49,6 +49,7 @@ import { matchesFilterCondition } from '@objectstack/formula'; import { validateRecord, assertEngineFindOnePredicate } from '@objectstack/objectql'; import type { PermissionSet } from '@objectstack/spec/security'; import { ObjectSchema } from '@objectstack/spec/data'; +import { composeReadableIdsInMemory } from './readable-ids-in-memory.testkit.js'; const REP = 'usr_rep'; const OTHER = 'usr_other'; @@ -396,8 +397,23 @@ async function boot(options: BootOptions = {}) { const repContext = () => ({ userId: REP, tenantId: 'org-1', positions: [], permissions: [] }); - /** The READ face: run the middleware, then apply the filter it injected. */ - const visibleContacts = async (): Promise => { + /** + * [ADR-0139 D6] The derivation answers the engine's `ReadableIds` leaf over + * the master, composed by the engine at its executor boundary from this + * service's own `canReadObject` / `getReadFilter` of the master. This double + * is no engine, so the composition is done for it the same way + * (`readable-ids-in-memory.testkit.ts`), with the real service — and the + * real sharing service behind it — answering the master's scope. + */ + const compose = (where: unknown, context: any) => + composeReadableIdsInMemory(where, context, { + rows: (object) => store.rows[object], + readAdmission: (object, c) => plugin.canReadObject(object, c), + readScope: (object, c) => plugin.getReadFilter(object, c), + }); + + /** Run the middleware for a find of the contacts and return the `where` it injected, uncomposed. */ + const injectedForContacts = async (): Promise => { const opCtx: any = { object: 'crm_contact', operation: 'find', @@ -406,20 +422,29 @@ async function boot(options: BootOptions = {}) { context: repContext(), }; await middleware(opCtx, async () => {}); + return opCtx.ast.where ?? null; + }; + + /** The READ face: run the middleware, compose the leaf it injected, then apply the filter. */ + const visibleContacts = async (): Promise => { + const where = await compose(await injectedForContacts(), repContext()); return (store.rows.crm_contact ?? []) - .filter((r) => matchesFilterCondition(r, opCtx.ast.where ?? null)) + .filter((r) => matchesFilterCondition(r, where as any)) .map((r) => String(r.id)); }; /** * [#5815] The ANALYTICS read face: the scope `getReadFilter` hands the - * raw-SQL path, applied to the same rows. No middleware runs here — this - * method IS the whole enforcement on that surface. + * analytics service, which hands it to the engine (the raw-SQL strategy + * declines a scope carrying the leaf), applied to the same rows. No + * middleware runs here — this method IS the whole enforcement on that + * surface. */ const analyticsVisibleContacts = async (context?: any): Promise => { const filter = await plugin.getReadFilter('crm_contact', context ?? repContext()); + const where = await compose((filter ?? null) as unknown, context ?? repContext()); return (store.rows.crm_contact ?? []) - .filter((r) => matchesFilterCondition(r, (filter ?? null) as any)) + .filter((r) => matchesFilterCondition(r, where as any)) .map((r) => String(r.id)); }; @@ -478,6 +503,8 @@ async function boot(options: BootOptions = {}) { ctx, plugin, repContext, + injectedForContacts, + compose, visibleContacts, analyticsVisibleContacts, updateContact, @@ -552,18 +579,23 @@ describe('[#5386] controlled_by_parent folds the master\'s ownership and share g }); // ── boundary conditions ────────────────────────────────────────────────── - it('the fold reads the grant table, resolves the master ONCE, and never re-enters the detail', async () => { + it('the derivation does NO store work; the grant table is read when the master\'s scope is composed, and the detail is never re-entered', async () => { const h = await boot({ shareLevel: 'edit' }); h.store.find.mockClear(); - await h.visibleContacts(); + // [ADR-0139 D6] The middleware writes the leaf and walks the chain over + // the registry: no pre-scan of the master, no read of anything. + const injected = await h.injectedForContacts(); + const during = h.store.find.mock.calls.map((c: any[]) => String(c[0])); + expect(during).not.toContain('crm_account'); + expect(during).not.toContain('crm_contact'); + // The master's scope is composed by the engine (here, by the testkit): + // THAT is where the grant table is consulted — direct evidence the sharing + // half runs inside the composition and not merely that some filter came + // back — and the detail itself is never read to answer its own scope. + await h.compose(injected, h.repContext()); const reads = h.store.find.mock.calls.map((c: any[]) => String(c[0])); - // The grant table was consulted — direct evidence the sharing half ran and - // not merely that some filter came back. expect(reads).toContain('sys_record_share'); - // The master id set is resolved by ONE system-context read. More than one - // would mean the derivation re-entered the middleware (v1 is single-level, - // ADR-0055 — the master's own controlled_by_parent is not walked). - expect(reads.filter((o) => o === 'crm_account')).toHaveLength(1); + expect(reads).not.toContain('crm_account'); expect(reads).not.toContain('crm_contact'); }); @@ -619,10 +651,16 @@ describe('[#5815] getReadFilter enforces the same read scope as the engine middl const h = await boot({ shareLevel: 'edit' }); const filter = await h.plugin.getReadFilter('crm_contact', h.repContext()); // Before the fix this was `undefined` (RLS null AND sharing null), which - // the raw-SQL path spreads into a WHERE that constrains nothing. + // the raw-SQL path spreads into a WHERE that constrains nothing. The + // predicate is the engine's leaf over the master (ADR-0139 D6): it names + // the master and binds no master id, and composed it admits exactly the + // contacts under the accounts the caller may read. expect(filter).toBeDefined(); - expect(JSON.stringify(filter)).toContain('acct_own'); - expect(JSON.stringify(filter)).not.toContain('acct_eu'); + expect(JSON.stringify(filter)).toContain('"object":"crm_account"'); + expect(JSON.stringify(filter)).not.toMatch(/acct_own|acct_eu|acct_us/); + const composed = await h.compose(filter, h.repContext()); + expect(JSON.stringify(composed)).toContain('acct_own'); + expect(JSON.stringify(composed)).not.toContain('acct_eu'); }); it('PARITY with no grant at all: both surfaces narrow to the caller-owned master', async () => { @@ -653,15 +691,23 @@ describe('[#5815] getReadFilter enforces the same read scope as the engine middl expect(await h.analyticsVisibleContacts()).toEqual(await h.visibleContacts()); }); - it('fail-closed: the derivation resolves the master ONCE per call, never re-entering the detail', async () => { + it('getReadFilter does NO store work for the derivation: the master is read only when the engine composes the leaf', async () => { const h = await boot({ shareLevel: 'edit' }); h.store.find.mockClear(); - await h.analyticsVisibleContacts(); + const filter = await h.plugin.getReadFilter('crm_contact', h.repContext()); + // [ADR-0139 D6] `getReadFilter(contact)` resolves the contact's OWN + // sharing half (the grant table) and writes the leaf for the master; it + // pre-scans no master and never re-enters the detail. + const before = h.store.find.mock.calls.map((c: any[]) => String(c[0])); + expect(before).not.toContain('crm_account'); + expect(before).not.toContain('crm_contact'); + // Composing the leaf asks `getReadFilter(account)`, whose sharing half + // reads the grant table — direct evidence the derivation runs on THIS + // path rather than a filter merely coming back from somewhere. + h.store.find.mockClear(); + await h.compose(filter ?? null, h.repContext()); const reads = h.store.find.mock.calls.map((c: any[]) => String(c[0])); - // Direct evidence the derivation (and its sharing half) ran on THIS path - // rather than a filter merely coming back from somewhere. expect(reads).toContain('sys_record_share'); - expect(reads.filter((o) => o === 'crm_account')).toHaveLength(1); expect(reads).not.toContain('crm_contact'); }); diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-write-shape-master-edit.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-write-shape-master-edit.test.ts index cb17c0d56c7..f415d78c15b 100644 --- a/packages/plugins/plugin-security/src/controlled-by-parent-write-shape-master-edit.test.ts +++ b/packages/plugins/plugin-security/src/controlled-by-parent-write-shape-master-edit.test.ts @@ -45,6 +45,7 @@ import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.j import { SecurityPlugin } from './security-plugin.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; +import { wireEngineSecuritySource } from './engine-security-source.testkit.js'; const SYS = { context: { isSystem: true } } as never; const ORG = 'org_cbp_wsme'; @@ -229,6 +230,8 @@ async function boot() { const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); await plugin.init(ctx as never); await plugin.start(ctx as never); + // The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2). + wireEngineSecuritySource(engine, () => plugin); sharing = new SharingService({ engine: engine as never, securityService: () => security as never }); engine.registerMiddleware(buildSharingMiddleware(sharing, ctx.logger) as never, { object: '*' }); // The row gates log the writes they refuse. diff --git a/packages/plugins/plugin-security/src/delegated-write-never-exceeds-delegator.test.ts b/packages/plugins/plugin-security/src/delegated-write-never-exceeds-delegator.test.ts index 4a9330a9643..56f0d29bada 100644 --- a/packages/plugins/plugin-security/src/delegated-write-never-exceeds-delegator.test.ts +++ b/packages/plugins/plugin-security/src/delegated-write-never-exceeds-delegator.test.ts @@ -46,6 +46,7 @@ import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.j import { SecurityPlugin } from './security-plugin.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; import { relaxEveryRegisteredTenantColumn } from './pre-18-tenant-column.testkit.js'; +import { wireEngineSecuritySource } from './engine-security-source.testkit.js'; const SYS = { context: { isSystem: true } } as never; const ALPHA = 'org_alpha'; @@ -216,6 +217,8 @@ async function boot(posture: TenancyPosture) { const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); await plugin.init(ctx as never); await plugin.start(ctx as never); + // The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2). + wireEngineSecuritySource(engine, () => plugin); sharing = new SharingService({ engine: engine as never, securityService: () => security as never }); engine.registerMiddleware(buildSharingMiddleware(sharing, ctx.logger) as never, { object: '*' }); // The row gates log the writes they refuse. diff --git a/packages/plugins/plugin-security/src/engine-security-source.testkit.ts b/packages/plugins/plugin-security/src/engine-security-source.testkit.ts new file mode 100644 index 00000000000..20db0168be5 --- /dev/null +++ b/packages/plugins/plugin-security/src/engine-security-source.testkit.ts @@ -0,0 +1,26 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0139 D2] What `ObjectQLPlugin.init` does on a kernel, for a rig that + * boots a bare `ObjectQL` beside the `SecurityPlugin`: put the SOURCE of the + * `security` service in the engine's slot, read at every `ReadableIds` + * composition and never cached. + * + * Since the `controlled_by_parent` derivation answers the engine's leaf + * (ADR-0139 D6), every read of a `controlled_by_parent` object on a real + * engine composes it from the security service — `canReadObject` and + * `getReadFilter` of the master — and an engine with an EMPTY slot answers + * every leaf `$in: []`, fail-closed. On a kernel the slot is always filled; + * a rig that wires the engine by hand fills it here, or every detail read, + * and every write whose probe reads the detail as the caller (the by-id + * pre-image, the predicate write's readable rows), sees no row. + * + * The key is `Symbol.for`, the one slot every bundle of the engine names + * (`SECURITY_SERVICE_SOURCE_SLOT` in `@objectstack/objectql`'s engine, which + * is deliberately not re-exported). + */ +export const ENGINE_SECURITY_SERVICE_SOURCE_SLOT: unique symbol = Symbol.for('objectstack.objectql.securityServiceSource'); + +export function wireEngineSecuritySource(engine: object, source: () => unknown): void { + (engine as Record)[ENGINE_SECURITY_SERVICE_SOURCE_SLOT] = source; +} diff --git a/packages/plugins/plugin-security/src/explain-controlled-by-parent-master-modify-all.test.ts b/packages/plugins/plugin-security/src/explain-controlled-by-parent-master-modify-all.test.ts index d1f14ac117f..5d90c60d06a 100644 --- a/packages/plugins/plugin-security/src/explain-controlled-by-parent-master-modify-all.test.ts +++ b/packages/plugins/plugin-security/src/explain-controlled-by-parent-master-modify-all.test.ts @@ -55,6 +55,7 @@ import { PermissionSetSchema } from '@objectstack/spec/security'; import type { ExplainDecision } from '@objectstack/spec/security'; import { SecurityPlugin } from './security-plugin.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; +import { wireEngineSecuritySource } from './engine-security-source.testkit.js'; const SYS = { context: { isSystem: true } } as never; const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; @@ -211,6 +212,8 @@ beforeAll(async () => { plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); await plugin.init(ctx as never); await plugin.start(ctx as never); + // The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2). + wireEngineSecuritySource(engine, () => plugin); sharing = new SharingService({ engine: engine as never, securityService: () => security as never }); engine.registerMiddleware(buildSharingMiddleware(sharing, ctx.logger) as never, { object: '*' }); }); diff --git a/packages/plugins/plugin-security/src/explain-enforce-parity.test.ts b/packages/plugins/plugin-security/src/explain-enforce-parity.test.ts index 4af40d1f223..4cca44d237c 100644 --- a/packages/plugins/plugin-security/src/explain-enforce-parity.test.ts +++ b/packages/plugins/plugin-security/src/explain-enforce-parity.test.ts @@ -91,6 +91,7 @@ import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.j import { SecurityPlugin } from './security-plugin.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; import { relaxEveryRegisteredTenantColumn } from './pre-18-tenant-column.testkit.js'; +import { wireEngineSecuritySource } from './engine-security-source.testkit.js'; const SYS = { context: { isSystem: true } } as never; const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; @@ -366,6 +367,8 @@ async function bootRls(predicate: string, opts: { grantCrud?: boolean } = {}) { const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); await plugin.init(ctx as never); await plugin.start(ctx as never); + // The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2). + wireEngineSecuritySource(engine, () => plugin); // The driver logs the refused comparison it withholds from the caller. vi.spyOn((engine as unknown as { logger: { warn: () => void } }).logger, 'warn').mockImplementation(() => undefined); @@ -552,6 +555,8 @@ async function bootPrincipal(source: PostureSource) { const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); await plugin.init(ctx as never); await plugin.start(ctx as never); + // The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2). + wireEngineSecuritySource(engine, () => plugin); const security = services.security as { resolvePermissionSetsForContext: (c: unknown) => Promise> }; /** The posture admission hands the resolver: the `tenancy` service's, or none when it is not registered. */ const admissionPosture = 'tenancy' in source ? source.tenancy : undefined; @@ -724,6 +729,8 @@ async function bootSharing(opts: { faultReadFilter?: boolean; authored?: boolean const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); await plugin.init(ctx as never); await plugin.start(ctx as never); + // The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2). + wireEngineSecuritySource(engine, () => plugin); sharing = new SharingService({ engine: engine as never, securityService: () => security as never }); if (opts.faultReadFilter) { sharing.buildReadFilter = async () => { throw new Error('share store unavailable'); }; @@ -931,6 +938,8 @@ async function bootDelegated(posture: TenancyPosture) { const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); await plugin.init(ctx as never); await plugin.start(ctx as never); + // The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2). + wireEngineSecuritySource(engine, () => plugin); sharing = new SharingService({ engine: engine as never, securityService: () => security as never }); engine.registerMiddleware(buildSharingMiddleware(sharing, ctx.logger) as never, { object: '*' }); // The pre-image gate logs the rows it refuses. @@ -1249,7 +1258,15 @@ function delegatedRows(posture: TenancyPosture, label: string, walled: boolean): [D_ORG, true, 'update', 'PARENT', 'p_org_mine', 'its own row', 'admitted'], [D_ORG, true, 'update', 'PARENT', 'p_beta', "its own row in another organization", walled ? REFUSED_NOT_FOUND : 'admitted'], [D_ORG, true, 'update', 'DETAIL', 'd_under_org', 'a detail under its own master', 'admitted'], - [D_ORG, true, 'update', 'DETAIL', 'd_under_theirs', "a detail under someone else's master", REFUSED_NOT_FOUND], + // [ADR-0139 D6] The detail follows its master's DELEGATED read: `m_theirs` + // is readable to the agent acting for the `org` writer (as `p_theirs` is, + // one row below the parent cells: refused at the write gate, not absent), + // so its detail is read and the update is refused by the master-edit + // gate, 403. Before the switch the id-list derivation scoped the agent's + // half of the master set at the ceiling's EFFECTIVE depth ('own'), so the + // detail was absent to a caller who could read its master — the drift + // ADR-0055 §2's equality forbids — and the update answered 404. + [D_ORG, true, 'update', 'DETAIL', 'd_under_theirs', "a detail under someone else's master", REFUSED_DENIED], [D_OWN, true, 'update', 'PARENT', 'p_own_mine', 'its own row', 'admitted'], [D_OWN, true, 'update', 'PARENT', 'p_theirs', 'a row it cannot read', REFUSED_NOT_FOUND], // The delegators themselves. diff --git a/packages/plugins/plugin-security/src/explain-read-absent-record-story.test.ts b/packages/plugins/plugin-security/src/explain-read-absent-record-story.test.ts index 73985629001..789071dbd6c 100644 --- a/packages/plugins/plugin-security/src/explain-read-absent-record-story.test.ts +++ b/packages/plugins/plugin-security/src/explain-read-absent-record-story.test.ts @@ -49,6 +49,7 @@ import { defaultPermissionSets } from './objects/default-permission-sets.js'; import { explainAccess, type ExplainEngineDeps } from './explain-engine.js'; import { PermissionEvaluator } from './permission-evaluator.js'; import { installDefaultOrganization } from './default-organization.testkit.js'; +import { wireEngineSecuritySource } from './engine-security-source.testkit.js'; const SYS = { context: { isSystem: true } } as never; const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; @@ -182,6 +183,8 @@ beforeAll(async () => { plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); await plugin.init(ctx as never); await plugin.start(ctx as never); + // The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2). + wireEngineSecuritySource(engine, () => plugin); sharing = new SharingService({ engine: engine as never, securityService: () => security as never }); engine.registerMiddleware(buildSharingMiddleware(sharing, ctx.logger) as never, { object: '*' }); }); diff --git a/packages/plugins/plugin-security/src/readable-ids-in-memory.testkit.ts b/packages/plugins/plugin-security/src/readable-ids-in-memory.testkit.ts new file mode 100644 index 00000000000..5f8ed411fb0 --- /dev/null +++ b/packages/plugins/plugin-security/src/readable-ids-in-memory.testkit.ts @@ -0,0 +1,137 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0139 D2] The engine's `ReadableIds` composition, IN MEMORY, for the + * suites in this package that drive the security middleware over an engine + * double and evaluate the filter it injected with `matchesFilterCondition`. + * + * Since the `controlled_by_parent` derivation answers the engine's leaf + * (ADR-0139 D6, the seventh consumer), the filter the middleware writes for a + * detail read is `{ masterFK: { $in: ReadableIds.of(master) } }` — a value no + * in-memory matcher can evaluate, because the leaf stands for a scope the + * engine composes at its executor boundary from the security service's own + * two halves of a read of the master. A double that evaluates the injected + * filter therefore needs that composition done for it, and this is it: the + * same five steps `ObjectQL#composeReadableIdsAtBoundary` runs, over the + * double's rows — + * + * 1. the object-level read admission of P false or a throw ⇒ `$in: []`; + * 2. the read scope of P: `undefined` ⇒ every row of P; the deny sentinel, by + * identity, or a throw ⇒ `$in: []`; any other scope ⇒ the ids of P's rows + * it matches, the leaves IT carries composed first (the nested master + * chain, ADR-0139 D6 as amended), under the same caller context; + * 3. a parent already on the branch is a cycle, and a chain deeper than the + * engine's bound is refused, both by a throw — the engine's own answer, + * so a suite that reaches either here is measuring a chain the plugin's + * registry walk should have closed first; + * 4. a parent the double holds no rows for answers `$in: []`, as a parent + * the registry does not hold does in the engine. + * + * ⛔ Not a second composer for production: D5's two classes (a master on + * another datasource, a master an object-scoped read middleware narrows) are + * refused at boot and are not modelled; the tenant wall is the driver's and a + * double has none. The real composition is pinned on the real stack in this + * package (`controlled-by-parent-leaf-switch.test.ts`, + * `readable-ids-nested-compose.test.ts`, `readable-ids-delegated-compose.test.ts`) + * and in `@objectstack/objectql`'s own suites; what this testkit lets a + * fake-engine suite keep pinning is WHICH ROWS the derivation admits, with + * the master's scope answered by the real security service. + */ + +import { RLS_DENY_FILTER, isComposedReadableIds, isReadableIds } from '@objectstack/spec/data'; +import { matchesFilterCondition } from '@objectstack/formula'; + +/** The engine's `READABLE_IDS_MAX_CHAIN_DEPTH`: scopes composed above the read's own `where`. */ +const MAX_CHAIN_DEPTH = 8; + +/** + * The two halves of a read of the parent the engine asks the security + * service (ADR-0139 D2 steps 1 and 5), handed in by the suite as predicates: + * this testkit is no door of its own and names no service member — the suite + * wires the real service's object admission and read scope to them. + */ +export interface InMemoryReadableIdsSecurity { + /** The rows of object `P` the double holds — `undefined` for an object it does not hold. */ + readonly rows: (object: string) => readonly Record[] | undefined; + /** D2 step 1: the object-level read admission of `P` for the caller. */ + readonly readAdmission: (object: string, context: unknown) => Promise; + /** D2 step 5: the row scope of `P` for the caller — `undefined`, the deny sentinel, or a filter. */ + readonly readScope: (object: string, context: unknown) => Promise; +} + +/** + * `where` with every `$in: ReadableIds.of(P)` leaf replaced by the ids of P's + * rows the caller may read, copy-on-write; a subtree holding no leaf is + * returned by reference. `null` and `undefined` pass through. + */ +export async function composeReadableIdsInMemory( + where: T, + context: unknown, + security: InMemoryReadableIdsSecurity, +): Promise { + const answers = new Map(); + const resolve = async (parent: string, ancestors: readonly string[]): Promise => { + const known = answers.get(parent); + if (known) return known; + if (ancestors.includes(parent)) { + throw new Error(`[readable-ids testkit] the chain of read scopes ${[...ancestors, parent].join(' -> ')} returns to '${parent}', a cycle`); + } + if (ancestors.length >= MAX_CHAIN_DEPTH) { + throw new Error(`[readable-ids testkit] the chain of read scopes ${[...ancestors, parent].join(' -> ')} is deeper than ${MAX_CHAIN_DEPTH}`); + } + const deny = (): unknown[] => { + answers.set(parent, []); + return []; + }; + const rows = security.rows(parent); + if (!rows) return deny(); + let admitted: boolean; + try { + admitted = (await security.readAdmission(parent, context)) === true; + } catch { + return deny(); + } + if (!admitted) return deny(); + let scope: unknown; + try { + scope = await security.readScope(parent, context); + } catch { + return deny(); + } + if (scope === RLS_DENY_FILTER) return deny(); + let matched: readonly Record[]; + if (scope === undefined || scope === null) { + matched = rows; + } else { + const composed = await rewrite(scope, [...ancestors, parent]); + matched = rows.filter((row) => matchesFilterCondition(row, composed as never)); + } + const ids = matched.map((row) => row.id).filter((id) => id !== undefined && id !== null); + answers.set(parent, ids); + return ids; + }; + const rewrite = async (value: unknown, ancestors: readonly string[]): Promise => { + if (value === null || typeof value !== 'object') return value; + if (isComposedReadableIds(value)) { + throw new Error('[readable-ids testkit] a composed leaf reached the composition: the engine refuses one it did not compose'); + } + if (Array.isArray(value)) { + let out: unknown[] | undefined; + for (let i = 0; i < value.length; i++) { + const next = await rewrite(value[i], ancestors); + if (next !== value[i]) (out ??= [...value])[i] = next; + } + return out ?? value; + } + let out: Record | undefined; + for (const key of Object.keys(value)) { + const item = (value as Record)[key]; + const next = key === '$in' && isReadableIds(item) + ? await resolve(item.object, ancestors) + : await rewrite(item, ancestors); + if (next !== item) (out ??= { ...(value as Record) })[key] = next; + } + return out ?? value; + }; + return (await rewrite(where, [])) as T; +} diff --git a/packages/plugins/plugin-security/src/readable-ids-nested-compose.test.ts b/packages/plugins/plugin-security/src/readable-ids-nested-compose.test.ts index 5ebdc57d1fd..e40a545218c 100644 --- a/packages/plugins/plugin-security/src/readable-ids-nested-compose.test.ts +++ b/packages/plugins/plugin-security/src/readable-ids-nested-compose.test.ts @@ -5,7 +5,7 @@ * that sits INSIDE a composed scope is composed by the engine too, so a * two-level `controlled_by_parent` chain — a child whose master is itself * `controlled_by_parent` — compiles to ONE statement with the subqueries - * nested, and serves exactly the rows today's id-list path serves. + * nested, and serves exactly the rows the id-list form served before the switch. * * Measured on the real stack: `ObjectQL` over better-sqlite3, the real * `SecurityPlugin` with the real `SharingService` behind it, direct member @@ -14,26 +14,24 @@ * child → PARENT (`controlled_by_parent`, its master GRAND) → GRAND (`private`, * owner-scoped by plugin-sharing), read two ways over the SAME rows: * - * - **today's id-list path**: a `controlled_by_parent` child of PARENT, read - * as the security middleware chain scopes it — `computeControlledByParentFilter` - * pre-scans GRAND and then PARENT under a system context and hands the - * driver `parent IN [ids]` (ADR-0055 §2 mechanism (b)); - * - **the leaf path**: a child of the same shape whose reads carry - * `{ parent: { $in: ReadableIds.of(PARENT) } }` (a consumer-shaped - * middleware writes it), with the engine's security source wrapped so that - * `getReadFilter(PARENT)` answers the real scope with its pre-resolved - * master id list replaced by the leaf `ReadableIds.of(GRAND)` — the - * spelling the plugin-security switch (ruling 6303721609, card (c)) will - * answer. The wrapper stands in for that switch and touches nothing else: - * `getReadFilter(GRAND)` and both `canReadObject` halves are the real - * service's, asked by the engine with the caller's context at each level. + * - **the switch's path**: a `controlled_by_parent` child of PARENT, read as + * the security middleware chain scopes it — `computeControlledByParentFilter` + * answers `{ parent: { $in: ReadableIds.of(PARENT) } }` (ADR-0139 D6, the + * seventh consumer), and `getReadFilter(PARENT)` answers PARENT's scope + * with the leaf `ReadableIds.of(GRAND)` inside it; + * - **a consumer's path**: a child of the same shape whose reads carry the + * same leaf over PARENT written by a consumer-shaped object-scoped + * middleware (the shape the six earlier consumers emit), composed by the + * engine from the same security answers. * - * Before the engine composed the nested leaf, the leaf path answered - * `INTERNAL_ERROR` / 500: driver-sql refused the uncomposed leaf inside the - * parent's subquery (the #24932 dev report's measurement, reproduced here on - * the base before the change). The closed doors are measured beside the - * parity: a caller the grandparent object does not admit, a delegator who - * does not exist, and a delegator the grandparent object does not admit. + * The two paths must agree row for row, and the first must run ONE statement. + * Before the switch, the first path pre-scanned GRAND and then PARENT under a + * system context and bound `parent IN [ids]`; before the engine composed the + * nested leaf, the second answered `INTERNAL_ERROR` / 500 (driver-sql refused + * the uncomposed leaf inside the parent's subquery). The closed doors are + * measured beside the parity: a caller the grandparent object does not admit, + * a delegator who does not exist, and a delegator the grandparent object does + * not admit. */ import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; @@ -41,7 +39,7 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { resolveAuthzContext, assembleExecutionContext } from '@objectstack/core'; -import { ReadableIds, RLS_DENY_FILTER } from '@objectstack/spec/data'; +import { ReadableIds, isReadableIds } from '@objectstack/spec/data'; import { PermissionSetSchema } from '@objectstack/spec/security'; import { SysOrganization, SysUser, SysMember } from '@objectstack/platform-objects/identity'; import { SharingService, buildSharingMiddleware, SysRecordShare } from '@objectstack/plugin-sharing'; @@ -60,9 +58,9 @@ const ORG = 'org_rid_nest'; const GRAND = 'qa_rid_nest_grand'; /** The middle: `controlled_by_parent`, its master GRAND — its read scope is derived from GRAND's. */ const PARENT = 'qa_rid_nest_parent'; -/** Today's path: a `controlled_by_parent` child of PARENT, scoped by the id-list pre-scan. */ +/** The switch's path: a `controlled_by_parent` child of PARENT, scoped by the leaf the derivation answers. */ const CHILD_CBP = 'qa_rid_nest_child_cbp'; -/** The leaf path: the same rows, read through a leaf over PARENT that a consumer-shaped middleware writes. */ +/** A consumer's path: the same rows, read through a leaf over PARENT that a consumer-shaped middleware writes. */ const CHILD_LEAF = 'qa_rid_nest_child_leaf'; /** The delegator: the human an agent acts for, and the owner of two grandparents. */ @@ -111,29 +109,16 @@ const ids = (rows: unknown): string[] => const leafBranch = () => ({ parent: { $in: ReadableIds.of(PARENT) } }); -/** - * The real scope with every pre-resolved master id list on `fk` replaced by - * the leaf over `master` — `{ fk: { $in: [ids] } }` becomes - * `{ fk: { $in: ReadableIds.of(master) } }`, copy-on-write — and how many - * were replaced. The spelling the plugin-security switch will answer. - */ -function withLeafForMaster(scope: unknown, fk: string, master: string): { scope: unknown; replaced: number } { - let replaced = 0; - const rewrite = (node: unknown): unknown => { - if (node === null || typeof node !== 'object') return node; - if (Array.isArray(node)) return node.map(rewrite); - const out: Record = {}; - for (const [key, value] of Object.entries(node as Record)) { - if (key === fk && value !== null && typeof value === 'object' && Array.isArray((value as { $in?: unknown }).$in)) { - replaced++; - out[key] = { ...(value as Record), $in: ReadableIds.of(master) }; - } else { - out[key] = rewrite(value); - } - } - return out; - }; - return { scope: rewrite(scope), replaced }; +/** The leaves a scope carries on `fk`, as the objects they name. */ +function leavesOn(scope: unknown, fk: string, out: string[] = []): string[] { + if (scope === null || typeof scope !== 'object') return out; + if (Array.isArray(scope)) { for (const item of scope) leavesOn(item, fk, out); return out; } + for (const [key, value] of Object.entries(scope as Record)) { + const comparand = key === fk && value !== null && typeof value === 'object' ? (value as { $in?: unknown }).$in : undefined; + if (isReadableIds(comparand)) out.push(comparand.object); + else leavesOn(value, fk, out); + } + return out; } async function boot() { @@ -296,22 +281,8 @@ async function boot() { await plugin.init(ctx as never); await plugin.start(ctx as never); // The engine's security source (what `ObjectQLPlugin.init` puts in the - // slot): the real service, with PARENT's read scope answered in the - // spelling the plugin-security switch will answer — the pre-resolved master - // id list replaced by the leaf over GRAND. Every other answer is the real - // service's, untouched. - const replacedPerCall: number[] = []; - const wrapped = { - canReadObject: (object: string, context: unknown) => security!.canReadObject(object, context), - async getReadFilter(object: string, context: unknown) { - const real = await security!.getReadFilter(object, context); - if (object !== PARENT || real === undefined || real === RLS_DENY_FILTER) return real; - const { scope, replaced } = withLeafForMaster(real, 'grand', GRAND); - replacedPerCall.push(replaced); - return scope; - }, - }; - (engine as unknown as Record)[Symbol.for('objectstack.objectql.securityServiceSource')] = () => wrapped; + // slot): the real service, read at every composition. + (engine as unknown as Record)[Symbol.for('objectstack.objectql.securityServiceSource')] = () => security; sharing = new SharingService({ engine: engine as never, securityService: () => security as never }); engine.registerMiddleware(buildSharingMiddleware(sharing, ctx.logger) as never, { object: '*' }); // The consumer shape: an object-scoped read middleware on the leaf-path @@ -349,7 +320,7 @@ async function boot() { return { engine, onBehalfOf, - replacedPerCall, + readFilter: (object: string, context: unknown) => security!.getReadFilter(object, { ...(context as object) }), read: async (object: string, context: unknown) => ids(await engine.find(object, { context: { ...(context as object) } } as never)), count: (object: string, context: unknown) => engine.count(object, { context: { ...(context as object) } } as never), @@ -379,7 +350,7 @@ const fromTable = (sql: string): string | undefined => /^select .*? from `?([a-z /** A statement that reads `table` — not one that reads another table with `table` inside a subquery. */ const readsTable = (table: string) => (sql: string) => fromTable(sql) === table; -describe('[ADR-0139 D6 as amended] a two-level controlled_by_parent chain composes and serves the rows the id-list path serves', () => { +describe('[ADR-0139 D6 as amended] a two-level controlled_by_parent chain composes, and the switch\'s path serves the rows a consumer\'s leaf serves', () => { let rig: Rig; beforeAll(async () => { rig = await boot(); }); afterAll(async () => { await rig.teardown(); }); @@ -389,7 +360,7 @@ describe('[ADR-0139 D6 as amended] a two-level controlled_by_parent chain compos expect(await rig.read(GRAND, member(OTHER, SET.human))).toEqual(['g_caller_only', 'g_neither', 'g_shared']); }); - it("a direct caller: the leaf path serves exactly the rows today's id-list path serves — find and count, both identities", async () => { + it("a direct caller: the switch's path serves exactly the rows a consumer's leaf serves — find and count, both identities", async () => { for (const [user, expected] of [ [HUMAN, ['c_both', 'c_del_only', 'c_shared']], [OTHER, ['c_caller_only', 'c_neither', 'c_shared']], @@ -408,7 +379,7 @@ describe('[ADR-0139 D6 as amended] a two-level controlled_by_parent chain compos expect(await rig.read(CHILD_LEAF, member(HUMAN, SET.human))).not.toContain('c_via_null'); }); - it("a delegated caller: the leaf path serves exactly the rows today's path serves — the delegator intersection at both levels", async () => { + it("a delegated caller: the switch's path serves exactly the rows a consumer's leaf serves — the delegator intersection at both levels", async () => { const context = await rig.onBehalfOf(HUMAN); const today = await rig.read(CHILD_CBP, context); expect(today).toEqual(['c_both', 'c_shared']); @@ -417,53 +388,40 @@ describe('[ADR-0139 D6 as amended] a two-level controlled_by_parent chain compos expect(await rig.count(CHILD_CBP, context)).toBe(2); }); - it("the wrapper replaced PARENT's pre-resolved master id list with the leaf on every composed read: once per leg", async () => { + it("PARENT's read scope carries the leaf over GRAND — one for a direct caller, one per leg for a delegated one — and no master id", async () => { // The direct answer carries one `controlled_by_parent` predicate; the - // delegated answer carries one per leg (ADR-0090 D10). Either way, every - // list was replaced — none reached the driver as a bound id list. - expect(rig.replacedPerCall.length).toBeGreaterThan(0); - expect(rig.replacedPerCall.every((n) => n >= 1)).toBe(true); - expect(new Set(rig.replacedPerCall)).toEqual(new Set([1, 2])); + // delegated answer carries one per leg (ADR-0090 D10). Either way it is + // the leaf: no grandparent id is pre-resolved into the scope. + const direct = await rig.readFilter(PARENT, member(HUMAN, SET.human)); + expect(leavesOn(direct, 'grand')).toEqual([GRAND]); + expect(JSON.stringify(direct)).not.toMatch(/g_both|g_del_only|g_shared/); + const delegated = await rig.readFilter(PARENT, await rig.onBehalfOf(HUMAN)); + expect(leavesOn(delegated, 'grand')).toEqual([GRAND, GRAND]); }); - it('one statement per find and per count on the leaf path: the child\'s select with the parent and the grandparent inside nested subqueries, and no pre-scan of the parent — against the pre-scans of today\'s path', async () => { + it('one statement per find and per count on BOTH paths: the child\'s select with the parent and the grandparent inside nested subqueries, and no pre-scan of the parent or the grandparent', async () => { const context = member(HUMAN, SET.human); - for (const [verb, run] of [ - ['find', () => rig.engine.find(CHILD_LEAF, { context: { ...context } } as never)], - ['count', () => rig.engine.count(CHILD_LEAF, { context: { ...context } } as never)], - ] as const) { - const statements = await rig.statementsDuring(run); - const all = `${verb}:\n${statements.join('\n')}`; - // The read is ONE statement: the child's, the parent inside its subquery - // and the grandparent inside that one. - const reads = statements.filter(readsTable(CHILD_LEAF)); - expect(reads, all).toHaveLength(1); - expect(reads[0].match(/in \(select/gi) ?? [], all).toHaveLength(2); - expect(reads[0], all).toMatch(new RegExp(`in \\(select .id. from .${PARENT}. where .grand. in \\(select .id. from .${GRAND}.`, 'i')); - // No statement reads the parent on its own: the leaf stands where the - // pre-resolved id list stood, so the parent is never pre-scanned. - expect(statements.filter(readsTable(PARENT)), all).toHaveLength(0); - // What else the driver ran is the security service's own answering, not - // the read: the real `getReadFilter(PARENT)` still derives PARENT's - // scope through `computeControlledByParentFilter`, whose grandparent - // pre-scan this rig's wrapper then REPLACES with the leaf (it retires - // with the plugin-security switch, card (c)), and plugin-sharing reads - // the share rows to answer the grandparent's scope. Neither is a read - // of the child, the parent or — as a read of its own — the grandparent - // by the composed statement. - for (const sql of statements.filter(readsTable(GRAND))) expect(sql, all).toMatch(/^select .id. from/i); + for (const child of [CHILD_CBP, CHILD_LEAF]) { + for (const [verb, run] of [ + ['find', () => rig.engine.find(child, { context: { ...context } } as never)], + ['count', () => rig.engine.count(child, { context: { ...context } } as never)], + ] as const) { + const statements = await rig.statementsDuring(run); + const all = `${child} ${verb}:\n${statements.join('\n')}`; + // The read is ONE statement: the child's, the parent inside its subquery + // and the grandparent inside that one. + const reads = statements.filter(readsTable(child)); + expect(reads, all).toHaveLength(1); + expect(reads[0].match(/in \(select/gi) ?? [], all).toHaveLength(2); + expect(reads[0], all).toMatch(new RegExp(`in \\(select .id. from .${PARENT}. where .grand. in \\(select .id. from .${GRAND}.`, 'i')); + // No statement reads the parent or the grandparent on its own: the + // leaf stands where the pre-resolved id list stood at every level, so + // neither is pre-scanned. What else the driver ran is plugin-sharing + // reading the share rows to answer the grandparent's scope. + expect(statements.filter(readsTable(PARENT)), all).toHaveLength(0); + expect(statements.filter(readsTable(GRAND)), all).toHaveLength(0); + } } - // Today's path: the grandparent and then the parent are pre-scanned under - // a system context (`select id from …`), and the child's own statement - // binds the parent ids as a list — no subquery anywhere. - const today = await rig.statementsDuring(() => rig.engine.find(CHILD_CBP, { context: { ...context } } as never)); - const todayAll = `today:\n${today.join('\n')}`; - expect(today.filter(readsTable(GRAND)), todayAll).toHaveLength(1); - expect(today.filter(readsTable(PARENT)), todayAll).toHaveLength(1); - const todayReads = today.filter(readsTable(CHILD_CBP)); - expect(todayReads, todayAll).toHaveLength(1); - expect(todayReads[0], todayAll).not.toMatch(/in \(select/i); - expect(today.some((sql) => /in \(select/i.test(sql)), todayAll).toBe(false); }); }); @@ -487,7 +445,7 @@ describe('[ADR-0139 D6 as amended] the closed doors stay closed at every level o expect(await rig.readOutcome(CHILD_LEAF, context)).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); }); - it('a delegator the grandparent object does not admit: the leaf path and today\'s path both serve nothing', async () => { + it('a delegator the grandparent object does not admit: both paths serve nothing', async () => { const context = await rig.onBehalfOf(NO_GRAND_READ); expect(await rig.canReadObject(GRAND, context)).toBe(false); expect(await rig.read(CHILD_CBP, context)).toEqual([]); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 130693ab657..54b78ccf74d 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -12,7 +12,7 @@ import { MCP_AGENT_PERMISSION_SET_RESTRICTED } from '@objectstack/spec/ai'; // [#21829] `MAX_BULK_PER_ROW_HOOK_ROWS`: the platform's row ceiling for one // predicate write, which also bounds the id list a predicate write's read // question composes (see PREDICATE_WRITE_READABLE_ID_CAP). -import { markFilterSubtreeProvenance, FieldMaskingRuleSchema, MAX_BULK_PER_ROW_HOOK_ROWS, type FieldMaskingRule } from '@objectstack/spec/data'; +import { markFilterSubtreeProvenance, FieldMaskingRuleSchema, MAX_BULK_PER_ROW_HOOK_ROWS, ReadableIds, type FieldMaskingRule } from '@objectstack/spec/data'; import { StandardErrorCode } from '@objectstack/spec/api'; import type { NumberComparandDoorFieldMeta } from '@objectstack/spec/data'; // [#7414] The SHARED operation-message catalog #7307 built for the data path's @@ -525,16 +525,22 @@ interface RlsFilterOptions { * {@link SecurityPlugin.resolveCbpRelation} and cached. */ /** - * [ADR-0055 amendment] How many `controlled_by_parent` hops the master-set derivation and - * the master-write gate will walk before they fail CLOSED. + * [ADR-0055 amendment] How many `controlled_by_parent` hops the read derivation's + * chain walk and the master-write gate will walk before they fail CLOSED. * * ⚠️ This is a COST ceiling, not a semantic rule, and it is deliberately not a * "supported chain length". Termination is already guaranteed without it — both * walks carry the set of objects already visited on the branch and refuse to - * re-enter one, over a finite schema registry. What the bound caps is WORK: one - * extra master-id `find` per hop per `controlled_by_parent` object per request, - * which is the per-request cost ADR-0055 already books as a known limit. A - * runaway generated schema should pay a bounded price, not an unbounded one. + * re-enter one, over a finite schema registry. What the bound caps is WORK. On + * the write side, one master-row probe per hop. On the read side the + * derivation answers the engine's `ReadableIds` leaf (ADR-0139 D6), which the + * engine composes into ONE statement with one nested subquery per hop; the + * engine refuses a chain past its own `READABLE_IDS_MAX_CHAIN_DEPTH` by name + * (INTERNAL_ERROR / 500), and this bound is the SAME cut, judged here first + * over the registry alone, so a chain the engine would refuse answers zero + * rows from this plugin and the engine never meets it. ⛔ The two values must + * stay equal: a plugin bound above the engine's turns an over-depth chain from + * zero rows into the engine's 500; one below it is over-denial. * * HOW THE VALUE WAS CHOSEN — no value is derivable from the tree, so the * derivation is stated rather than implied. Measured on this repo: every @@ -545,15 +551,28 @@ interface RlsFilterOptions { * never derived. The consumer that motivated the ADR-0055 amendment needs **two** * (`crm_quote_line_item` → `crm_quote` → `crm_account`). 8 is four times the * deepest chain any consumer has asked for, so it cannot be reached by - * authoring that means anything, and it still caps the walk at 8 queries. + * authoring that means anything, and it still caps the walk at 8 hops. * - * AT THE BOUND: the read derivation returns the EMPTY master set and the write - * gate DENIES, each logging the chain it refused. ⛔ Never "no restriction" — - * that is precisely the failure the ADR-0055 amendment closed, and a bound that widened on - * overflow would reintroduce it at depth 9 instead of depth 2. + * AT THE BOUND: the read derivation answers the EMPTY master set (`$in: []`) + * and the write gate DENIES, each logging the chain it refused. ⛔ Never "no + * restriction" — that is precisely the failure the ADR-0055 amendment closed, + * and a bound that widened on overflow would reintroduce it at depth 9 + * instead of depth 2. */ const CBP_MAX_CHAIN_DEPTH = 8; +/** + * [ADR-0139 D1] The engine verbs whose `where` may carry a `ReadableIds` + * leaf — the engine's own `READABLE_IDS_READ_VERBS`. The read derivation + * answers its leaf for these and nothing for a write verb's predicate: a + * predicate write is narrowed to the rows the read door returns for the + * caller's own predicate ({@link SecurityPlugin.readableRowsScopeForPredicateWrite}, + * where the leaf composes) and judged per master by the 2.8 gate + * ({@link SecurityPlugin.assertControlledByParentWrite}, a verdict, never a + * scope), so no master set is composed into a write's predicate. + */ +const CBP_LEAF_READ_VERBS: ReadonlySet = new Set(['find', 'findOne', 'count', 'aggregate']); + /** * [ADR-0055] Which leg of the master-detail write check a refusal came from, * keyed by the refusal itself. Written at the one place a refusal is built @@ -4461,12 +4480,12 @@ export class SecurityPlugin implements Plugin { if (rlsFilter) extra.push(rlsFilter); let tenantLayer0Verdict = layered.tenantLayer0Verdict; // ADR-0055: a controlled_by_parent object derives its read scope from the - // master record — `masterFK IN (accessible master ids)`, AND-ed in. - const cbpFilter = await this.computeControlledByParentFilter( - permissionSets, - opCtx.object, - opCtx.context, - ); + // master record — `masterFK IN ()`, + // AND-ed in as the engine's `ReadableIds` leaf over the master + // (ADR-0139 D6), which the engine composes at its executor boundary + // from this very service's `canReadObject` / `getReadFilter` of the + // master. Nothing for a write verb's predicate (ADR-0139 D1). + const cbpFilter = this.computeControlledByParentFilter(opCtx.object, opCtx.operation, opCtx.context); if (cbpFilter) extra.push(cbpFilter); // [ADR-0090 D10] AND the delegator's read RLS (and CBP) into the same // where — the delegated principal sees only rows BOTH may see. Computed @@ -4480,7 +4499,12 @@ export class SecurityPlugin implements Plugin { // recorded verdict is their intersection — what the composed wall // decided, not the caller's half alone. tenantLayer0Verdict = intersectTenantLayer0Verdicts(tenantLayer0Verdict, delLayered.tenantLayer0Verdict); - const delCbp = await this.computeControlledByParentFilter(delegatorSets, opCtx.object, delegatorContext); + // The delegator's half writes the same leaf over the master; the + // engine composes it with the request's delegated context, whose + // scope `getReadFilter` answers as the ADR-0090 D10 intersection + // (ADR-0139 D5.1 as amended), so both halves carry one subquery + // twice — correct rows, measured, and left as it is. + const delCbp = this.computeControlledByParentFilter(opCtx.object, opCtx.operation, delegatorContext); if (delCbp) extra.push(delCbp); } // [#21829] The write doors' rule on the PREDICATE door: a row the @@ -5926,7 +5950,11 @@ export class SecurityPlugin implements Plugin { * * 1. {@link computeRlsFilter} — tenant Layer 0 + RLS policies; * 2. {@link computeControlledByParentFilter} — ADR-0055, `masterFK IN - * (accessible master ids)`; + * ()`, spelled as the engine's + * `ReadableIds` leaf over the master (ADR-0139 D6): the engine composes + * it from this service's own answers about the master before any + * driver sees it, and the analytics faces hand it to the engine or + * decline it (`NativeSQLStrategy.canHandle`), never compile it; * 3. {@link resolveSharingReadFilter} — plugin-sharing's OWD / record-share * visibility filter, contributed by the sibling middleware. * @@ -6027,17 +6055,13 @@ export class SecurityPlugin implements Plugin { if (permissionSets.length === 0) return RLS_DENY_FILTER; const filter = await this.computeRlsFilter(permissionSets, object, 'find', context); // [#5815] ADR-0055 — the SECOND thing the middleware ANDs into `ast.where` - // for a read. Resolved from the sets already resolved above, exactly as - // the middleware feeds it its own, so the derived master id set is - // computed for this identity once and never re-resolved. It stands down - // (null) for any object that is not controlled_by_parent, and it is - // internally fail-closed; a THROW propagates to the catch below, which + // for a read: the leaf over the master, the same answer the middleware + // writes for a find. It stands down (null) for any object that is not + // controlled_by_parent, and it is internally fail-closed (the deny + // sentinel for an unresolvable relation, `$in: []` for a chain the + // registry walk refuses); a THROW propagates to the catch below, which // denies — the same posture as the surrounding layers. - const cbpFilter = await this.computeControlledByParentFilter( - permissionSets, - object, - context, - ); + const cbpFilter = this.computeControlledByParentFilter(object, 'find', context); // [#4467] RLS AND controlled-by-parent AND sharing — the same // AND-composition the middlewares achieve by all writing into `ast.where`. // Any layer may be absent; `andComposeLayers` returns the others, or null @@ -6152,9 +6176,9 @@ export class SecurityPlugin implements Plugin { // Layers 1 to 4 — the security middleware's own four, in its order. const filter = await this.computeRlsFilter(permissionSets, object, 'find', context); - const cbpFilter = await this.computeControlledByParentFilter(permissionSets, object, context); + const cbpFilter = this.computeControlledByParentFilter(object, 'find', context); const delFilter = await this.computeRlsFilter(delegatorSets, object, 'find', delegatorContext); - const delCbpFilter = await this.computeControlledByParentFilter(delegatorSets, object, delegatorContext); + const delCbpFilter = this.computeControlledByParentFilter(object, 'find', delegatorContext); const securityLayers = andComposeLayers(andComposeLayers(filter, cbpFilter), andComposeLayers(delFilter, delCbpFilter)); const sharingLayers = andComposeLayers(sharingFilter, delegatorSharingFilter); @@ -8750,195 +8774,171 @@ export class SecurityPlugin implements Plugin { } /** - * ADR-0055 — master-detail "controlled by parent" READ derivation. + * ADR-0055 — master-detail "controlled by parent" READ derivation, answered + * as the engine's `ReadableIds` leaf (ADR-0139 D6: `controlled_by_parent` + * is the leaf's seventh consumer, in the end-state form). * * For an object whose `sharingModel` is `controlled_by_parent`, access is - * derived from the master: return a filter `masterFK IN ()`. The id set is resolved under a system context — no middleware - * re-entry, so no recursion — against BOTH halves of the master's own read - * scope, the same two the engine ANDs into a direct `find` of the master: - * - * 1. the master's read RLS (`computeRlsFilter` — tenant Layer 0 + policies), and - * 2. plugin-sharing's OWD / record-share visibility filter - * (`resolveSharingReadFilter` — the owner-match widened by READ depth, - * OR-ed with the caller's `sys_record_share` grants). - * - * [#5386] Half 2 used to be missing, and its absence was not a narrow gap: an - * app that authors NO `rowLevelSecurity` on the master got an UNRESTRICTED - * master id set, so the declared narrowing restricted nothing at all — every - * detail row was readable by any holder of object-level read. Authoring RLS on - * the master was no workaround either, since RLS is ANDed with (not OR-ed - * into) the sharing filter and so cuts off the very rows a grant shared in. - * Which half applies is decided by the MASTER's own effective sharing model — - * `buildReadFilter` returns null for a non-`private` master — so the derived - * set stays point-for-point equal to what a direct find of the master returns. - * - * [ADR-0055 §3] Both halves scope ROWS. Ahead of them the master's OBJECT-level - * read admission is asked ({@link objectReadAdmission}, the predicate - * `canReadObject` asks), for the principal whose sets this call was handed: a - * principal the master's own read door refuses at the object gate gets the - * empty master set, so the equality with a direct find of the master holds for - * that principal too, and with the engine's ReadableIds leaf, which asks - * `canReadObject` before it scopes a parent's rows. - * - * An empty set yields `{ masterFK: { $in: [] } }`, which matches no rows (fail - * closed), as does a failure to resolve the sharing half. A misconfigured - * object (not exactly one required master_detail to derive from, ADR-0055 §1) - * denies all reads (defense-in-depth; the spec's parse refuses authoring it - * since #23100). Returns null when the object is not controlled_by_parent. - * - * [ADR-0055 amendment] The derivation COMPOSES ACROSS A CHAIN. It used to resolve the - * master set from the two halves above and nothing else, which made a master - * that is ITSELF `controlled_by_parent` resolve to "no restriction" on both: - * its RLS half is `null` (a derived object authors no policy — that is the - * whole point of the declaration) and its sharing half is `null` too, because - * `effectiveSharingModel` maps `controlled_by_parent` to `public` and - * `buildReadFilter` opts out of every non-`private` model. Composed: `null`, - * so `find(master, {})` ran as SYSTEM and returned EVERY master row. A - * two-level chain was therefore enforced at level one and org-wide at level - * two — read and write — with metadata that reads as if it were narrowed. - * - * So the master's own derivation is now the THIRD half, AND-ed in with the - * other two: the master set is exactly the set a direct read of the master - * returns, at every level, which is the same equality #5386 established for - * one level. ⛔ This is NOT a blanket refusal for chained declarations — that - * would deny the single-level case #5386 fixed and that measurement shows - * correct today. A detail under a reachable master stays reachable. - * - * ⚠️ ADR-0055 records "single-level only in v1" as an honest limit and lists - * transitive chains under Non-goals. That scope line is what this closes; the - * ADR itself is a governed surface and is amended separately. - * - * Two guards bound the walk, both fail-CLOSED (an empty master set), never - * "no restriction" — the direction that made this a security defect: - * - * - **Cycle protection.** `ancestors` carries the objects already being - * resolved on this branch. Re-entering one is a metadata cycle (`A`'s - * master is `B`, `B`'s master is `A`, or an object mastered by itself), - * and it denies. Termination does not depend on the depth bound: the - * ancestor set is strictly growing over a finite schema registry. + * derived from the master: the answer is `{ masterFK: { $in: + * ReadableIds.of(master) } }` — "the ids of the master's records this + * caller may read". The engine replaces the leaf at its executor boundary, + * immediately before the driver call, with a scope it composes from THIS + * service's own two halves of a read of the master, asked with the caller's + * context exactly as it stands (`ObjectQL#composeReadableIdsAtBoundary`): + * + * 1. `canReadObject(master)` — the object-level read admission + * ({@link objectReadAdmission}); refused ⇒ `$in: []`, no detail is read; + * 2. `getReadFilter(master)` — the master's row scope: its read RLS + * (tenant Layer 0 + policies), plugin-sharing's OWD / record-share + * visibility filter, and the master's OWN `controlled_by_parent` + * derivation, which is this method again and so another leaf — the + * chain composes recursively, one nested subquery per hop, inside the + * one statement the driver runs. The deny sentinel ⇒ `$in: []`. + * + * The driver compiles the composed leaf as `masterFK IN (SELECT id FROM + * master WHERE scope AND tenant)` (ADR-0139 D3), so a detail read is one + * statement whatever the master's cardinality: no master id list is + * materialised, nothing is bound per master, and the dialect's variable + * limit is never reached. Before this, the derivation pre-scanned the + * master under a system context and bound every readable id into `$in` — + * twice per request (find, then count), unbounded — so a child read answered + * `DATABASE_ERROR` / 500 past ~32,800 masters (the dialect's bind limit), and + * its CPU grew with the master table. + * + * [#5386] The master's scope is the SAME scope a direct find of the master + * runs under, point for point: both halves above are the ones the engine + * middleware chain ANDs into a direct read of the master, asked of the same + * service, so the derived set cannot drift from the direct one. That + * equality used to be re-established here by composing the halves by hand + * (RLS, sharing, the master's own derivation) around a system-context scan; + * it now follows from the engine asking this service's public doors. + * + * [ADR-0090 D10] A DELEGATED (on-behalf-of) caller is composed, not routed: + * the engine asks both halves with the delegated context intact, and + * `getReadFilter(master)` answers the ADR-0090 D10 intersection through + * {@link computeDelegatedReadFilter} (ADR-0139 D5.1 as amended). The + * middleware's delegator half writes the same leaf, so a delegated read + * carries one subquery twice; correct rows, measured, left as it is. + * + * [ADR-0139 D1] The leaf is a READ scope and a write's predicate never + * carries one: `null` for a write verb ({@link CBP_LEAF_READ_VERBS}). A + * predicate write of a detail is narrowed to the rows the read door returns + * for the caller's own predicate ({@link readableRowsScopeForPredicateWrite}, + * where this leaf composes) and judged per master by the 2.8 gate + * ({@link assertControlledByParentWrite}, a verdict, never a scope). + * + * [ADR-0139 D5.2 / D5.3] The two master shapes the engine composes no leaf + * over — a master on another datasource, a master an object-scoped read + * middleware narrows — are refused at boot (`ObjectQLPlugin`, on + * `kernel:bootstrapped`), so no up-front routing to a bounded list is made + * here: every `controlled_by_parent` object answers the leaf. + * + * A misconfigured object (not exactly one required master_detail to derive + * from, ADR-0055 §1) answers the deny sentinel by reference (the engine + * answers `$in: []` for it when it is a master's scope; a direct read of the + * object matches no row). Returns null when the object is not + * controlled_by_parent, or when the caller is the system. + * + * [ADR-0055 amendment] The derivation COMPOSES ACROSS A CHAIN — a master + * that is itself `controlled_by_parent` is scoped by ITS master, at every + * level — and two guards bound the chain, both fail-CLOSED (`$in: []`, the + * empty master set), never "no restriction", the direction that made the + * unbounded form a security defect. Both are judged here over the REGISTRY + * alone, before the leaf is written and before any store work + * ({@link controlledByParentChainRefusal}), so the engine's own cycle and + * depth refusals (`INTERNAL_ERROR` / 500, `READABLE_IDS_MAX_CHAIN_DEPTH`) + * are never reached for a declared chain: + * + * - **Cycle protection.** The walk carries the objects already on the + * chain and a chain that re-enters one (`A`'s master is `B`, `B`'s + * master is `A`, or an object mastered by itself) denies. Termination + * does not depend on the depth bound: the visited set is strictly + * growing over a finite schema registry. * - **Depth bound** ({@link CBP_MAX_CHAIN_DEPTH}). A COST ceiling, not a - * semantic rule — see the constant for how the value was chosen. + * semantic rule, and the engine's own cut — see the constant. */ - private async computeControlledByParentFilter( - permissionSets: PermissionSet[], + private computeControlledByParentFilter( object: string, + /** The engine verb the answer is composed onto; a write verb answers `null` (ADR-0139 D1). */ + operation: string, context: any, - /** - * [ADR-0055 amendment] The `controlled_by_parent` objects already being resolved on - * this branch of the walk, outermost first. Empty at five of the six call - * sites — the CRUD middleware (caller and D10 delegator), `getReadFilter` - * and `computeDelegatedReadFilter` (principal and delegator) — and grown by - * one at the sixth, the recursive hop. - */ - ancestors: readonly string[] = [], - ): Promise | null> { + ): Record | null { if (!this.ql || !context?.userId) return null; const schema = typeof this.ql.getSchema === 'function' ? this.ql.getSchema(object) : null; const sharingModel = schema?.sharingModel ?? schema?.security?.sharingModel; if (sharingModel !== 'controlled_by_parent') return null; + if (!CBP_LEAF_READ_VERBS.has(operation)) return null; const rel = this.resolveCbpRelation(object); // No resolvable master: the deny sentinel, by reference (ADR-0139 D2 step 2). if (!rel) return RLS_DENY_FILTER; - // [ADR-0055 amendment] Chain guards, BEFORE any store work. Both answer with the empty - // master set — the same shape the #5386 sharing-resolution failure answers - // with, and the same posture: a chain this derivation cannot resolve denies, - // because the alternative ("no restriction") is the defect being fixed. - if (ancestors.includes(object)) { + // [ADR-0055 amendment] Chain guards, over the registry, BEFORE the leaf is + // written. Both answer with the empty master set — the same shape the + // #5386 sharing-resolution failure answered with, and the same posture: a + // chain this derivation cannot resolve denies, because the alternative + // ("no restriction") is the defect being fixed, and the other alternative + // (the engine's refusal of the same chain) is a 500 for a declared shape. + const refusal = this.controlledByParentChainRefusal(object, rel); + if (refusal?.kind === 'cycle') { this.logger.error?.( `[security] controlled_by_parent derivation found a CYCLE resolving '${object}' ` + - `(chain: ${[...ancestors, object].join(' -> ')}) — denying (fail-closed: a chain the derivation ` + + `(chain: ${refusal.chain.join(' -> ')}) — denying (fail-closed: a chain the derivation ` + `cannot resolve admits no child rather than leaving it unrestricted)`, ); return { [rel.fk]: { $in: [] } }; } - if (ancestors.length >= CBP_MAX_CHAIN_DEPTH) { + if (refusal?.kind === 'depth') { this.logger.error?.( `[security] controlled_by_parent derivation exceeded the chain depth bound ` + `(${CBP_MAX_CHAIN_DEPTH}) resolving '${object}' ` + - `(chain: ${[...ancestors, object].join(' -> ')}) — denying (fail-closed: a chain the derivation ` + + `(chain: ${refusal.chain.join(' -> ')}) — denying (fail-closed: a chain the derivation ` + `cannot resolve admits no child rather than leaving it unrestricted)`, ); return { [rel.fk]: { $in: [] } }; } + return { [rel.fk]: { $in: ReadableIds.of(rel.master) } }; + } - // [ADR-0055 §3] "Reading a detail requires READ access to its master", and - // the OBJECT-level read admission is the first half of that access — the - // half the master's own read door asks before any row filter, and the half - // the engine's ADR-0139 ReadableIds leaf asks (`canReadObject`). Asked here - // through the one predicate both read ({@link objectReadAdmission}), for - // the principal whose sets this call was handed, before the master's rows - // are scoped below under a system context. A principal the master's own - // door does not admit reads no master, so the master set is empty and no - // detail is read; at every hop of a chain alike, since the recursion below - // asks the same question of the next master. A throw denies too - // (fail-closed), as the sharing half's resolution failure does. - let masterObjectReadable: boolean; - try { - masterObjectReadable = await this.objectReadAdmission(rel.master, permissionSets, null); - } catch (e) { - this.logger.error?.( - `[security] controlled_by_parent derivation could not resolve the object-level read admission ` + - `of master '${rel.master}' for '${object}' (user ${context?.userId ?? 'unknown'}) ` + - `— denying (fail-closed: a child is readable only where its master is, so a master whose ` + - `admission cannot be resolved admits no child)`, - e instanceof Error ? e : new Error(String(e)), - ); - return { [rel.fk]: { $in: [] } }; - } - if (!masterObjectReadable) return { [rel.fk]: { $in: [] } }; - - const masterRlsFilter = await this.computeRlsFilter(permissionSets, rel.master, 'find', context); - // [#5386] The OWD / record-share half, resolved through the SAME helper - // `getReadFilter` uses, so the derived path and the direct path cannot - // drift. A resolution failure denies (empty master set) rather than - // silently widening the children back to everyone. - let masterSharingFilter: Record | null; - try { - masterSharingFilter = await this.resolveSharingReadFilter(rel.master, context, permissionSets); - } catch (e) { - this.logger.error?.( - `[security] controlled_by_parent derivation could not resolve the sharing (OWD) read ` + - `scope of master '${rel.master}' for '${object}' (user ${context?.userId ?? 'unknown'}) ` + - `— denying (fail-closed: a child is readable only where its master is, so a master scope that ` + - `cannot be resolved admits no child)`, - e instanceof Error ? e : new Error(String(e)), - ); - return { [rel.fk]: { $in: [] } }; - } - // [ADR-0055 amendment] The THIRD half — the master's OWN `controlled_by_parent` - // derivation, resolved through this very method so the recursion cannot - // drift from the top-level answer. `null` for a master that is not derived - // (the single-level case, unchanged), and internally fail-closed at every - // level. This is the whole fix on the read side: without it both halves - // above are `null` for a derived master and the system `find` below - // returned every row. - const masterCbpFilter = await this.computeControlledByParentFilter( - permissionSets, - rel.master, - context, - [...ancestors, object], - ); - const masterFilter = andComposeLayers( - andComposeLayers(masterRlsFilter, masterSharingFilter), - masterCbpFilter, - ); - let masterIds: string[] = []; - try { - const rows = await this.ql.find(rel.master, { - where: masterFilter ?? {}, - fields: ['id'], - context: { isSystem: true }, - }); - masterIds = (Array.isArray(rows) ? rows : []) - .map((r: any) => r?.id) - .filter((id: any) => id != null); - } catch { - masterIds = []; + /** + * [ADR-0055 amendment] Walk the `controlled_by_parent` chain above `object` + * through the registry — no store work, no security call — and name the + * first guard it trips: a cycle, or the depth bound. `null` when the chain + * is one the engine composes whole. + * + * The walk follows exactly the chain the engine's composition will follow: + * each master that is itself `controlled_by_parent` contributes another leaf + * to its own read scope, so the chain is the run of derived masters, and it + * ends at the first master that governs its own rows, at a master the + * registry does not hold, or at a derived master with no resolvable + * relation (whose own derivation answers the deny sentinel, which the engine + * answers `$in: []` — closed, and no refusal). `chain` is the objects on the + * branch outermost first, the re-entered or over-depth master last — what + * the refusal log prints. + * + * The depth cut is the engine's: a chain of {@link CBP_MAX_CHAIN_DEPTH} + * derived objects composes (the read's object and up to seven derived + * masters above it), and the next derived master is refused — here as the + * empty master set, in the engine as its loud refusal of the same chain, + * which this walk therefore pre-empts. + */ + private controlledByParentChainRefusal( + object: string, + rel: CbpRelation, + ): { kind: 'cycle' | 'depth'; chain: string[] } | null { + const chain: string[] = [object]; + let master = rel.master; + for (;;) { + if (chain.includes(master)) return { kind: 'cycle', chain: [...chain, master] }; + const schema = typeof this.ql?.getSchema === 'function' ? this.ql.getSchema(master) : null; + const model = schema?.sharingModel ?? schema?.security?.sharingModel; + if (model !== 'controlled_by_parent') return null; + if (chain.length >= CBP_MAX_CHAIN_DEPTH) return { kind: 'depth', chain: [...chain, master] }; + chain.push(master); + const next = this.resolveCbpRelation(master); + if (!next) return null; + master = next.master; } - return { [rel.fk]: { $in: masterIds } }; } /** diff --git a/packages/services/service-analytics/src/__tests__/controlled-by-parent-leaf-three-faces.test.ts b/packages/services/service-analytics/src/__tests__/controlled-by-parent-leaf-three-faces.test.ts new file mode 100644 index 00000000000..61a26ab81a4 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/controlled-by-parent-leaf-three-faces.test.ts @@ -0,0 +1,308 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0139 D6 · ruling Q2 A on #24932] A read scope carrying the engine's + * `ReadableIds` leaf — what `getReadFilter` answers for a `controlled_by_parent` + * object since plugin-security's derivation switched to the leaf — on the three + * analytics faces: + * + * - **the engine-bound face** (`ObjectQLStrategy.withReadScope` → + * `engine.aggregate`) ADMITS the leaf and the engine composes it at its + * boundary, so the aggregate answers the find path's rows; + * - **the native face** (`NativeSQLStrategy`) DECLINES the query in + * `canHandle`, so it routes to the ObjectQL strategy and the raw-SQL seam is + * never called — the content-based decline the #7598 ruling accepted; + * - **the `/analytics/sql` echo** (`ObjectQLStrategy.generateSql`, through + * `compileScopedFilterToSql`) REFUSES in the withheld + * `READ_SCOPE_COMPILE_FAILED` / 500 envelope: it runs no composition and + * renders no statement it would not run. The one visible change, on a + * debugging aid, named in the PR as a deviation. + * + * And what must not move: a filter an AUTHOR writes keeps today's judgement + * (an unbranded `{ object }` in a `$in` slot is refused as the non-list + * comparand it always was, with or without a leaf-carrying scope), and a + * caller the master object does not admit — the deny sentinel as the scope — + * keeps serving nothing on every face. + * + * Before this change (measured on the base, the dev report on #24932): every + * face answered `READ_SCOPE_COMPILE_FAILED` / 500 for a leaf-carrying scope, + * the engine-bound one from `assertReadScopeComparandsRunnable` refusing the + * branded leaf as a non-list comparand before `engine.aggregate` could + * compose it. + */ + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import { declaredRefusalMessage, resolveThrownHttpError, serverFaultProvenance } from '@objectstack/types'; +import { ReadableIds, RLS_DENY_FILTER } from '@objectstack/spec/data'; +import type { Cube, FilterCondition } from '@objectstack/spec/data'; +import type { AnalyticsQuery, StrategyContext } from '@objectstack/spec/contracts'; + +import { AnalyticsService } from '../analytics-service.js'; +import { ObjectQLStrategy } from '../strategies/objectql-strategy.js'; +import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js'; +import type { DatasetScopedStrategyContext } from '../strategies/types.js'; +import { compileScopedFilterToSql, findReadableIdsLeaf } from '../read-scope-sql.js'; + +const MASTER = 'cbp_leaf_account'; +const DETAIL = 'cbp_leaf_contact'; + +const MASTER_FIELDS: Record> = { + id: { type: 'text', name: 'id' }, + owner: { type: 'text', name: 'owner' }, +}; +const DETAIL_FIELDS: Record> = { + id: { type: 'text', name: 'id' }, + account: { type: 'text', name: 'account' }, + region: { type: 'text', name: 'region' }, +}; + +const MASTERS = [ + { id: 'a_mine', owner: 'u_me' }, + { id: 'a_mine2', owner: 'u_me' }, + { id: 'a_theirs', owner: 'u_other' }, +]; +const DETAILS = [ + { id: 'c1', account: 'a_mine', region: 'emea' }, + { id: 'c2', account: 'a_mine2', region: 'apac' }, + { id: 'c3', account: 'a_theirs', region: 'emea' }, + { id: 'c4', account: null, region: 'emea' }, + { id: 'c5', account: 'a_gone', region: 'amer' }, +]; +/** The rows the find path serves u_me: the details of the masters u_me owns. */ +const MINE = ['c1', 'c2']; +const ALL = DETAILS.map((d) => d.id); + +const CUBE: Cube = { + name: 'contacts', + sql: DETAIL, + measures: { n: { sql: '*', type: 'count', title: 'n' } }, + dimensions: Object.fromEntries(['id', 'region'].map((n) => [n, { label: n, type: 'string', sql: n }])), + public: true, +} as unknown as Cube; + +const QUERY = { cube: 'contacts', dimensions: ['id'], measures: ['n'] } as AnalyticsQuery; + +/** The scope plugin-security's derivation answers for DETAIL: the leaf over MASTER. A factory: a fresh leaf each time. */ +const leafScope = (): FilterCondition => ({ account: { $in: ReadableIds.of(MASTER) } }) as unknown as FilterCondition; +/** What an author could write that LOOKS like the leaf: an unbranded object in a list slot. */ +const authoredLookalike = { account: { $in: { object: MASTER } } }; + +interface WireBearingError extends Error { + code?: unknown; + status?: unknown; +} + +type Outcome = { refusal?: WireBearingError; admitted?: string[] }; + +const quiet = { + debug() {}, + info() {}, + warn() {}, + error() {}, + child() { + return quiet; + }, +}; + +describe('[ADR-0139 D6] a read scope carrying the ReadableIds leaf on the three analytics faces', () => { + let driver: SqliteWasmDriver; + let engine: ObjectQL; + /** Swapped per case; the `getReadScope` contract filled by hand. */ + let readScope: unknown = null; + /** Statements the NATIVE face handed to the database. */ + let rawStatements = 0; + /** The master's scope the engine's composition is answered, per call. */ + const securityCalls: string[] = []; + + const runRawSql = async (sql: string, params: unknown[]): Promise[]> => { + rawStatements += 1; + const result = await driver.execute(sql.replace(/\$\d+/g, '?'), params as unknown[]); + if (Array.isArray(result)) return result as Record[]; + if (result && typeof result === 'object' && 'rows' in (result as Record)) { + return (result as { rows: Record[] }).rows; + } + return []; + }; + + beforeAll(async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + (driver as unknown as { logger: unknown }).logger = quiet; + await driver.initObjects([{ name: MASTER, fields: MASTER_FIELDS }, { name: DETAIL, fields: DETAIL_FIELDS }] as never); + for (const row of MASTERS) await driver.create(MASTER, { ...row }); + for (const row of DETAILS) await driver.create(DETAIL, { ...row }); + + engine = new ObjectQL({ logger: quiet }); + engine.registerDriver(driver as never, true); + await engine.init(); + engine.registerObject({ name: MASTER, label: 'Account', fields: MASTER_FIELDS } as never); + engine.registerObject({ name: DETAIL, label: 'Contact', fields: DETAIL_FIELDS } as never); + // The engine's security source (what `ObjectQLPlugin.init` puts in the + // slot): the master's two halves for u_me — admitted, the rows u_me owns. + const security = { + canReadObject: async (object: string) => { securityCalls.push(`canReadObject:${object}`); return true; }, + getReadFilter: async (object: string) => { + securityCalls.push(`getReadFilter:${object}`); + return object === MASTER ? { owner: 'u_me' } : undefined; + }, + }; + (engine as unknown as Record)[Symbol.for('objectstack.objectql.securityServiceSource')] = () => security; + }); + + afterAll(async () => { + await driver?.disconnect?.(); + }); + + const executeAggregate = async (objectName: string, options: any) => + (await engine.aggregate(objectName, { + where: options.filter, + groupBy: options.groupBy, + aggregations: options.aggregations?.map((a: any) => ({ function: a.method, field: a.field, alias: a.alias })), + context: options.context, + } as never)) as Record[]; + + /** The service with the engine path only — the three-faces rig's shape. */ + const engineService = () => new AnalyticsService({ + cubes: [CUBE], + logger: quiet, + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + getReadScope: () => (readScope ?? undefined) as FilterCondition | undefined, + executeAggregate, + }); + + /** The service with native SQL wired, as a raw-SQL deployment composes it. */ + const nativeService = () => { + const rawSql = vi.fn(runRawSql); + const service = new AnalyticsService({ + cubes: [CUBE], + logger: quiet, + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: true, inMemory: false }), + getReadScope: () => (readScope ?? undefined) as FilterCondition | undefined, + executeAggregate, + executeRawSql: (_object: string, sql: string, params: unknown[]) => rawSql(sql, params), + hasObjectMiddleware: () => false, + }); + return { service, rawSql }; + }; + + /** A hand-filled `StrategyContext` for driving one strategy directly. */ + const directCtx = (nativeSql: boolean): StrategyContext => + ({ + getCube: (name: string) => (name === 'contacts' ? CUBE : undefined), + queryCapabilities: () => ({ nativeSql, objectqlAggregate: !nativeSql, inMemory: false }), + getReadScope: () => (readScope ?? undefined) as FilterCondition | undefined, + executeRawSql: (_object: string, sql: string, params: unknown[]) => runRawSql(sql, params), + executeAggregate, + hasObjectMiddleware: () => false, + }) as unknown as StrategyContext; + + const ids = (rows: Array>): string[] => rows.map((r) => String(r.id)).sort(); + + const outcomeOf = async (run: () => Promise>>): Promise => { + try { + return { admitted: ids(await run()) }; + } catch (e) { + return { refusal: e as WireBearingError }; + } + }; + + const expectWithheldRefusal = (face: string, o: Outcome): void => { + expect(o.admitted, `${face}: expected a refusal, got rows`).toBeUndefined(); + expect(o.refusal, `${face}: expected an Error`).toBeInstanceOf(Error); + expect(o.refusal?.code, `${face}: envelope code`).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(o.refusal?.status, `${face}: envelope status`).toBe(500); + expect(serverFaultProvenance(resolveThrownHttpError(o.refusal, 500)), `${face}: provenance`).toBe('declared'); + expect(declaredRefusalMessage(o.refusal), `${face}: relayed prose`).toBeUndefined(); + }; + + it('CONTROL: with no scope, the engine-bound face serves the whole fixture', async () => { + readScope = null; + expect(await outcomeOf(async () => (await engineService().query(QUERY)).rows)).toEqual({ admitted: ALL }); + }); + + it('the engine-bound face composes the leaf and answers the find path\'s rows: the details of the masters u_me owns', async () => { + readScope = leafScope(); + securityCalls.length = 0; + expect(await outcomeOf(async () => (await engineService().query(QUERY)).rows)).toEqual({ admitted: MINE }); + // The composition asked the security service about the MASTER, as a find does. + expect(securityCalls).toContain(`canReadObject:${MASTER}`); + expect(securityCalls).toContain(`getReadFilter:${MASTER}`); + // …and a `where` of the caller's own composes with it. + expect(await outcomeOf(async () => (await engineService().query({ ...QUERY, where: { region: 'emea' } } as AnalyticsQuery)).rows)) + .toEqual({ admitted: ['c1'] }); + }); + + it('the native face declines in canHandle, and through the service the raw-SQL seam is never called: the engine path serves the rows', async () => { + readScope = leafScope(); + expect(new NativeSQLStrategy().canHandle(QUERY, directCtx(true))).toBe(false); + // The decline reads the scope of every object the statement scopes — here + // the base object — through the context's resolved set, as the door + // resolves it. + const scopedCtx = { ...directCtx(true), readScopedObjects: [DETAIL] } as unknown as DatasetScopedStrategyContext; + expect(new NativeSQLStrategy().canHandle(QUERY, scopedCtx)).toBe(false); + const { service, rawSql } = nativeService(); + rawStatements = 0; + expect(await outcomeOf(async () => (await service.query(QUERY)).rows)).toEqual({ admitted: MINE }); + expect(rawSql).not.toHaveBeenCalled(); + expect(rawStatements).toBe(0); + }); + + it('CONTROL: the native face still serves a scope with no leaf natively — the decline is content-based', async () => { + readScope = { region: 'emea' }; + expect(new NativeSQLStrategy().canHandle(QUERY, directCtx(true))).toBe(true); + const { service, rawSql } = nativeService(); + expect(await outcomeOf(async () => (await service.query(QUERY)).rows)).toEqual({ admitted: ['c1', 'c3', 'c4'] }); + expect(rawSql).toHaveBeenCalledTimes(1); + }); + + it('the /analytics/sql echo refuses a leaf-carrying scope in the withheld READ_SCOPE_COMPILE_FAILED envelope, before any statement', async () => { + readScope = leafScope(); + rawStatements = 0; + const direct = await outcomeOf(async () => { + const { sql, params } = await new ObjectQLStrategy().generateSql(QUERY, directCtx(false)); + return runRawSql(sql, params); + }); + expectWithheldRefusal('echo (strategy)', direct); + const viaService = await outcomeOf(async () => { + const { sql, params } = await engineService().generateSql(QUERY); + return runRawSql(sql, params); + }); + expectWithheldRefusal('echo (service)', viaService); + expect(rawStatements).toBe(0); + // The compiler names the leaf and the object it stands for, for the + // operator's log — the prose the wire withholds. + expect(() => compileScopedFilterToSql(leafScope(), DETAIL)).toThrow(/ReadableIds leaf over "cbp_leaf_account" at readScope\.account\.\$in/); + expect(findReadableIdsLeaf(leafScope())).toEqual({ object: MASTER, path: 'account.$in' }); + expect(findReadableIdsLeaf({ $and: [{ region: 'emea' }, leafScope()] })).toEqual({ object: MASTER, path: '$and[1].account.$in' }); + expect(findReadableIdsLeaf({ region: 'emea' })).toBeNull(); + expect(findReadableIdsLeaf(authoredLookalike)).toBeNull(); + }); + + it("a filter an AUTHOR writes keeps today's judgement: an unbranded look-alike in a $in slot is refused as a non-list comparand, with or without the leaf-carrying scope", async () => { + const authored = { ...QUERY, where: authoredLookalike } as AnalyticsQuery; + for (const [name, scope] of [['no scope', null], ['the leaf-carrying scope', leafScope()]] as const) { + readScope = scope; + const engineFace = await outcomeOf(async () => (await engineService().query(authored)).rows); + expect(engineFace.admitted, `engine face, ${name}`).toBeUndefined(); + expect(engineFace.refusal?.code, `engine face, ${name}`).toBe('INVALID_FILTER'); + expect(engineFace.refusal?.status, `engine face, ${name}`).toBe(400); + } + // …and the same refusal where the native face serves (no leaf in the scope). + readScope = null; + const { service, rawSql } = nativeService(); + const nativeFace = await outcomeOf(async () => (await service.query(authored)).rows); + expect(nativeFace.refusal?.code).toBe('INVALID_FILTER'); + expect(nativeFace.refusal?.status).toBe(400); + expect(rawSql).not.toHaveBeenCalled(); + }); + + it('a caller the master object does not admit keeps the deny sentinel: no rows on the engine face, and the native face still serves it natively as nothing', async () => { + readScope = RLS_DENY_FILTER; + expect(await outcomeOf(async () => (await engineService().query(QUERY)).rows)).toEqual({ admitted: [] }); + expect(new NativeSQLStrategy().canHandle(QUERY, directCtx(true))).toBe(true); + const { service, rawSql } = nativeService(); + expect(await outcomeOf(async () => (await service.query(QUERY)).rows)).toEqual({ admitted: [] }); + expect(rawSql).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 529668b7862..ac44380aac7 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -1075,7 +1075,10 @@ export class AnalyticsServicePlugin implements Plugin { // catches it and renders raw ids, so there the refusal only reaches the // log. Same envelope as the other engine-bound merges, withheld on the // wire; the display pass's catch is untouched. - if (scope) assertReadScopeComparandsRunnable(scope, targetObject); + // [ADR-0139 D6] The engine's `ReadableIds` leaf admitted, as at the + // two merges this door mirrors: the `$and` below hands the scope to + // `executeAggregate` — the engine — whose boundary composes it. + if (scope) assertReadScopeComparandsRunnable(scope, targetObject, { admitReadableIds: true }); if (scope) assertReadScopePlaceholdersResolvable(scope, targetObject, context); if (scope) assertReadScopeAdmittedByEngine(scope, targetObject, context, { judgeFilter }); // #3680 — the sort-key pass hands over the PRE-window id set (every diff --git a/packages/services/service-analytics/src/read-scope-sql.ts b/packages/services/service-analytics/src/read-scope-sql.ts index 62f59da19f0..cc9ff7037d3 100644 --- a/packages/services/service-analytics/src/read-scope-sql.ts +++ b/packages/services/service-analytics/src/read-scope-sql.ts @@ -5,6 +5,11 @@ import type { FilterCondition } from '@objectstack/spec/data'; // at the ObjectQL merge sites by {@link assertReadScopeComparandsRunnable}, and // [#20018] at the end of {@link compileScopedFilterToSql} by the same function. import { assertListComparandShapes, normalizeFilterComparandTypes } from '@objectstack/spec/data'; +// [ADR-0139 D6] The engine's `ReadableIds` leaf, which a read scope carries for +// a `controlled_by_parent` object since the derivation answers it: admitted at +// the engine-bound merges ({@link assertReadScopeComparandsRunnable} with +// `admitReadableIds`), refused at the entry of {@link compileScopedFilterToSql}. +import { isReadableIds } from '@objectstack/spec/data'; // [ADR-0053 D-D1, amended 2026-09-30 — #5930 step 3] The shared lowering, run // at the entry of {@link compileScopedFilterToSql}, after the placeholders // resolve and before any clause is compiled. @@ -827,6 +832,23 @@ export function compileScopedFilterToSql( options: ReadScopeCompileOptions = {}, ): { sql: string; params: unknown[] } { const quotedAlias = quoteIdent(alias, 'alias'); + // [ADR-0139 D6] A scope carrying the engine's `ReadableIds` leaf is refused + // here, first and by name: this compiler runs no composition, so it cannot + // render the semi-join the engine composes for the leaf, and rendering the + // scope without it would show a statement the platform never runs. + // `NativeSQLStrategy.canHandle` declines such a query before this point and + // the ObjectQL strategy serves it; the `/analytics/sql` dry-run echo reaches + // here and refuses in the module's envelope — the one consistent, loud + // answer, no half-rendering (the #7598 ruling's own words for this face). + const leaf = findReadableIdsLeaf(filter); + if (leaf) { + throw readScopeCompileError( + `[read-scope-sql] read scope for "${alias}" carries the engine's ReadableIds leaf over "${leaf.object}" at ` + + `readScope.${leaf.path}: the engine composes that leaf at its executor boundary, and this compiler runs no ` + + 'composition, so the scope cannot be rendered here (fail-closed). The query is served by the ObjectQL ' + + 'strategy; the dry-run echo cannot show a statement it would not run.', + ); + } // [#20075] Resolve the scope's placeholders with the engine's own resolver // BEFORE the lowering, because the lowering binds values: what is bound, and // what the echo prints, is the value the ObjectQL face's engine resolves. A @@ -1002,6 +1024,50 @@ export function assertReadScopeCannotVacate(scope: unknown, objectName: string): ); } +/** [ADR-0139 D6] What an engine-bound merge tells {@link assertReadScopeComparandsRunnable}. */ +export interface ReadScopeComparandOptions { + /** + * Admit the engine's branded `ReadableIds` leaf as a `$in` comparand: the + * scope is handed to `engine.aggregate`, whose boundary composes it. Passed + * by the engine-bound merges only; `compileScopedFilterToSql` never passes it. + */ + readonly admitReadableIds?: boolean; +} + +/** + * [ADR-0139 D6] The first `ReadableIds` leaf `scope` carries as a `$in` + * comparand — the object it names and the key path it sits at — or `null`. + * Exported for `NativeSQLStrategy.canHandle`, which declines a query whose + * base or joined object's scope carries one, so the scope goes to the engine + * and never to a compiler that cannot compose it. + */ +export function findReadableIdsLeaf(scope: unknown): { object: string; path: string } | null { + const onPath = new Set(); + const visit = (value: unknown, path: string): { object: string; path: string } | null => { + if (value === null || typeof value !== 'object') return null; + if (onPath.has(value)) return null; + onPath.add(value); + try { + if (Array.isArray(value)) { + for (let i = 0; i < value.length; i++) { + const found = visit(value[i], `${path}[${i}]`); + if (found) return found; + } + return null; + } + for (const [key, item] of Object.entries(value as Record)) { + if (key === '$in' && isReadableIds(item)) return { object: item.object, path: `${path}.${key}` }; + const found = visit(item, path === '' ? key : `${path}.${key}`); + if (found) return found; + } + return null; + } finally { + onPath.delete(value); + } + }; + return visit(scope, ''); +} + /** * [#19995] Refuse, in this module's envelope, a read scope the ENGINE would * refuse for one of its comparands — judged on the scope ALONE, before it is @@ -1068,14 +1134,39 @@ export function assertReadScopeCannotVacate(scope: unknown, objectName: string): * {@link compileScopedFilterToSql} the `$icontains` arm of * {@link compileOperator} refuses the same scope first, in its own sentence. * + * ## …and the engine's `ReadableIds` leaf (ADR-0139 D6) + * + * A `controlled_by_parent` object's read scope carries the engine's branded + * `ReadableIds` leaf over its master — "the ids of the master this caller may + * read" — since the security service's derivation answers it in place of a + * pre-resolved id list. The list-shape face refuses the leaf as a non-list + * comparand unless its caller passes `admitReadableIds: true`, the option the + * engine's own ingress door passes (`@objectstack/objectql`'s filter seam). + * The two ENGINE-bound merges and the engine-bound label pass pass it here + * (`options.admitReadableIds`): what they compose is handed to + * `engine.aggregate`, whose executor boundary composes the leaf from the + * security service exactly as a `find` does, so admitting it is the one + * verdict that agrees with the find path. {@link compileScopedFilterToSql} + * passes nothing and keeps refusing: it compiles the scope itself, runs no + * composition, and `NativeSQLStrategy.canHandle` declines such a query before + * it is reached. The leaf is admitted only where the scope came from + * `getReadScope`; a filter an author writes never reaches this door with one + * (the brand is a symbol key no JSON carries), and the engine's door keeps + * refusing it there as the non-list comparand it always was. + * * @param scope the `StrategyContext.getReadScope` output, exactly as returned * @param objectName the object the scope was requested for — for the operator's * log only; withheld from the response by the `READ_SCOPE_COMPILE_FAILED` / * 500 declaration, like every message in this module. + * @param options `admitReadableIds` — passed by the engine-bound merges only. */ -export function assertReadScopeComparandsRunnable(scope: unknown, objectName: string): void { +export function assertReadScopeComparandsRunnable( + scope: unknown, + objectName: string, + options?: ReadScopeComparandOptions, +): void { try { - assertListComparandShapes(scope, undefined, 'readScope'); + assertListComparandShapes(scope, undefined, 'readScope', options?.admitReadableIds === true ? { admitReadableIds: true } : undefined); normalizeFilterComparandTypes(scope, undefined, 'readScope'); } catch (e) { throw readScopeCompileError( diff --git a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts index 7d015231e03..d255ba69917 100644 --- a/packages/services/service-analytics/src/strategies/native-sql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/native-sql-strategy.ts @@ -36,7 +36,7 @@ import { numberComparandRefusalMessage, } from '@objectstack/spec/data'; import { findCrossFieldComparand, findUninterpretableTemporalMember } from '../comparand-shape.js'; -import { assertReadScopeCannotVacate, compileScopedFilterToSql } from '../read-scope-sql.js'; +import { assertReadScopeCannotVacate, compileScopedFilterToSql, findReadableIdsLeaf } from '../read-scope-sql.js'; import { nonTextColumnResolver, textOperatorPolarity } from '../non-text-column.js'; import { declaredValueShapeResolver, whereEmptyLeafSql } from '../empty-operator-sql.js'; // [#20986] The one resolver of the object a relationship-path hop reads. @@ -736,10 +736,47 @@ export class NativeSQLStrategy implements AnalyticsStrategy { // is the native fast path for every gated object, and on an engine that // cannot answer, for every object. See {@link readsObjectWithEngineMiddleware}. if (this.readsObjectWithEngineMiddleware(query, ctx)) return false; + // ── [ADR-0139 D6] DECLINE a read scope carrying the engine's leaf ────── + // + // A `controlled_by_parent` object's read scope carries the engine's + // `ReadableIds` leaf over its master — "the ids of the master this caller + // may read" — since the security service's derivation answers it in + // place of a pre-resolved id list (the id list answered 500 past the + // dialect's bind limit, and this strategy bound it as `IN ($1, …)`). The + // leaf is composed only at the engine's executor boundary, from the + // security service's own `canReadObject` / `getReadFilter` of the master, + // recursively for a master chain; this strategy runs raw SQL through the + // driver and composes nothing, and `compileScopedFilterToSql` refuses the + // leaf by name. So the query routes to the ObjectQL strategy, whose + // `withReadScope` admits the leaf and hands it to `engine.aggregate`, + // where it composes as it does on a find. The content-based decline the + // #7598 ruling accepted for a `{ $field }` comparand, for the same + // reason: one owner of the composition, no second copy here. Every object + // the statement scopes is read, as the cross-field decline reads them. + if (this.readScopeCarriesReadableIds(query, ctx)) return false; const caps = ctx.queryCapabilities(query.cube); return caps.nativeSql && typeof ctx.executeRawSql === 'function'; } + /** + * [ADR-0139 D6] Does the read scope of any object this statement scopes + * carry the engine's `ReadableIds` leaf? See the decline at + * {@link canHandle}. The objects are the set the door scoped + * (`readScopedObjects`), or the cube's base object and declared joins for a + * context built without it, as {@link crossFieldComparisonIn} reads them. A + * context with no `getReadScope` scopes nothing and carries no leaf. + */ + private readScopeCarriesReadableIds(query: AnalyticsQuery, ctx: StrategyContext): boolean { + if (typeof ctx.getReadScope !== 'function') return false; + const cube = query.cube ? ctx.getCube(query.cube) : undefined; + if (!cube) return false; + const scoped = (ctx as DatasetScopedStrategyContext).readScopedObjects; + const objects = scoped + ? [...scoped] + : [this.extractObjectName(cube), ...Object.keys(cube.joins ?? {}).map((alias) => cube.joins?.[alias]?.name ?? alias)]; + return objects.some((object) => findReadableIdsLeaf(ctx.getReadScope!(object)) !== null); + } + /** * [#21080] Does this query read an object the engine holds a middleware * for — or one it cannot answer about? See the decline at {@link canHandle}. diff --git a/packages/services/service-analytics/src/strategies/objectql-strategy.ts b/packages/services/service-analytics/src/strategies/objectql-strategy.ts index 160a2c268da..2de62845ff2 100644 --- a/packages/services/service-analytics/src/strategies/objectql-strategy.ts +++ b/packages/services/service-analytics/src/strategies/objectql-strategy.ts @@ -897,7 +897,12 @@ export class ObjectQLStrategy implements AnalyticsStrategy { // same scope in the withheld `READ_SCOPE_COMPILE_FAILED` / 500 (#5367). // Judged on the scope ALONE, for the attribution reason above; the // caller's own `where` keeps reaching the engine's doors and its 400. - assertReadScopeComparandsRunnable(scope, objectName); + // [ADR-0139 D6] With the engine's `ReadableIds` leaf admitted: this merge + // hands the scope to `engine.aggregate`, whose executor boundary composes + // the leaf from the security service exactly as a find does. The option + // is this merge's and the engine's; `compileScopedFilterToSql` keeps + // refusing the leaf, and the caller's own `where` never carries one. + assertReadScopeComparandsRunnable(scope, objectName, { admitReadableIds: true }); // [#19995] …and the placeholder half. The engine resolves `{…}` values on // the COMPOSED `where`, so a placeholder in the scope it cannot resolve // came back as its `FILTER_TOKEN_*` / 400 with the token relayed. Judged @@ -1419,7 +1424,10 @@ export class ObjectQLStrategy implements AnalyticsStrategy { // [#19995] …and the comparand half, as at `withReadScope`: a comparand the // engine refuses would come back as its relayed 400, naming the referenced // object's policy. Before the mark, for the same reason as the line above. - if (scope != null) assertReadScopeComparandsRunnable(scope, refObject); + // [ADR-0139 D6] The engine's `ReadableIds` leaf admitted, as at + // `withReadScope`: this is an engine-bound merge, and the engine composes + // the leaf at its boundary. + if (scope != null) assertReadScopeComparandsRunnable(scope, refObject, { admitReadableIds: true }); // [#19995] …and the placeholder half, as at `withReadScope`, with the // context forwarded to `executeAggregate` below. if (scope != null) assertReadScopePlaceholdersResolvable(scope, refObject, ctx.context); diff --git a/packages/spec/src/data/filter-comparand-shape.ts b/packages/spec/src/data/filter-comparand-shape.ts index 8ddde5bcac8..4c102ba8ea4 100644 --- a/packages/spec/src/data/filter-comparand-shape.ts +++ b/packages/spec/src/data/filter-comparand-shape.ts @@ -440,10 +440,14 @@ * non-enumerable brand; the copy is refused here as a non-list, ⛔ never * dropped (dropping the `$in` would serve every row the rest of the branch * matches). - * - **Every caller but the engine's filter seam refuses it.** This door is - * also the authoring lowering's ({@link parseFilterAST}), the save door's, - * the analytics doors' and the RLS compiler's, and none of them composes the - * leaf; they pass no option and keep refusing it. + * - **Every caller that composes nothing refuses it.** The callers that pass + * the option are the engine's filter seam, which composes the leaf at the + * executor boundary, and the analytics service's engine-bound merges, which + * judge the scope the engine will then run and hand it to the engine's own + * aggregate boundary to compose (ADR-0139 D6, the seventh consumer). This + * door is also the authoring lowering's ({@link parseFilterAST}), the save + * door's, the analytics raw-SQL compiler's and the RLS compiler's, and none + * of those composes the leaf; they pass no option and keep refusing it. * * ## Refusal envelope * diff --git a/packages/spec/src/data/readable-ids-conformance.ts b/packages/spec/src/data/readable-ids-conformance.ts index 57c95972c5d..96e01dc306e 100644 --- a/packages/spec/src/data/readable-ids-conformance.ts +++ b/packages/spec/src/data/readable-ids-conformance.ts @@ -258,6 +258,18 @@ export const READABLE_IDS_CASES: readonly ReadableIdsCase[] = [ note: 'The branch shape ADR-0139 D4 has each consumer emit per parent object. c7 carries an id that ' + 'rid_parent also carries, under another object name.', }, + { + name: 'a composed leaf nested in a composed leaf\'s scope: the subqueries nest', + where: () => ({ record_id: { $in: composed({ id: { $in: composed({ owner: 'u1' }) } }) } }), + verdict: 'matches', + holdsComposedLeaf: true, + expected: ['c1', 'c3', 'c6', 'c7'], + note: 'ADR-0139 D6 as amended, the nested master chain: a parent whose own read scope is derived from ITS ' + + 'master (a controlled_by_parent master that is controlled_by_parent too) is composed with a leaf inside ' + + 'its scope, and the driver compiles one statement with the subqueries nested. The inner scope admits ' + + 'the parents u1 owns, so the rows are the narrowing case\'s. A driver that resolves only the outer leaf ' + + 'hands the inner one to its compiler unresolved and refuses it, or evaluates it as no restriction.', + }, { name: 'an uncomposed leaf is refused, never evaluated as no restriction', where: () => ({ record_id: { $in: ReadableIds.of(P) } }),