diff --git a/.changeset/25009-readable-ids-delegated-compose.md b/.changeset/25009-readable-ids-delegated-compose.md new file mode 100644 index 00000000000..e2af54e0452 --- /dev/null +++ b/.changeset/25009-readable-ids-delegated-compose.md @@ -0,0 +1,19 @@ +--- +'@objectstack/objectql': minor +'@objectstack/plugin-security': minor +--- + +feat(objectql, plugin-security): the engine composes its `ReadableIds` leaf for a delegated (on-behalf-of) caller, scoped to the delegator intersection, instead of refusing it (ADR-0139 D5.1 as amended; O5 answered) + +Clause-②: no + +**Before.** A read carrying an on-behalf-of link whose final `where` held a `ReadableIds` leaf was refused `INTERNAL_ERROR` / 500 before any driver was asked (ADR-0139 D5.1 as accepted), and `getReadFilter` answered the deny sentinel for every delegated context, so the leaf's consumers routed such a caller to their bounded, fail-closed probe. + +**Now.** + +- `@objectstack/objectql`: the executor boundary composes the leaf for a delegated caller exactly as for any other. It asks `canReadObject` and `getReadFilter` with the delegated context as it stands (`onBehalfOf` intact) and composes what the security service answers; it derives no intersection of its own. The two remaining D5 classes, a parent an object-scoped read middleware narrows and a parent bound to another datasource, are still refused by name with the same envelope. +- `@objectstack/plugin-security`: `getReadFilter` answers a delegated context the ADR-0090 D10 intersection, the six layers the engine middleware ANDs into a delegated `find` of the same object: the principal's row-level security and `controlled_by_parent` derivation, the delegator's two over the delegator's own rebuilt context, and the sharing visibility filter under each identity at its own depth. A delegated read through the leaf serves exactly the rows a delegated direct read of the parent serves. It fails closed as the chain does: a delegator who does not exist, a delegator the object does not admit, a principal or a delegator that resolves no permission set, and any resolution failure answer the deny sentinel, and the leaf is `$in: []`. + +**What a reader sees.** No shipped consumer writes a leaf for a delegated caller yet (each still routes it to its probe; that routing retires per consumer on its own card), so a delegated read of `sys_activity`, `sys_audit_log`, `sys_comment`, `sys_comment_reaction` or `sys_attachment` is served as before. A door that binds `getReadFilter` directly, such as the analytics raw-SQL path, scopes a delegated context to the intersection now; as accepted it denied that context outright. + +**Unchanged.** `controlled_by_parent`'s own read scope (ADR-0055 §2 mechanism (b), the pre-resolved id set) and the nested master chain, which keep their own cards; the delegated `find`, `findOne`, `count` and `aggregate` through the engine middleware, which already applied the intersection. 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 e3b436c0379..aed87191b9e 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 @@ -5,8 +5,14 @@ D2's position sentence names the executor boundary — after the middleware chain and the before-hooks, immediately before each driver call — in place of the seam where `lowerRelationConditions` runs, which precedes the chain D4's emitters write into (ruling [`6121741984`](https://github.com/objectstack-ai/objectstack/issues/22861#issuecomment-6121741984) -on [#22861](https://github.com/objectstack-ai/objectstack/issues/22861), letter A). **This record declares; it -implements nothing.** It became **Accepted** on the maintainer's approval (`docs/adr/**` is Tier H, Prime +on [#22861](https://github.com/objectstack-ai/objectstack/issues/22861), letter A). **Amended** (2026-10-21): O5 is +answered — `controlled_by_parent` becomes the seventh consumer, in the end-state form — and D5.1 is amended from a +refusal to a composition: the engine composes the leaf for a delegated (on-behalf-of) caller, the scope being the +ADR-0090 D10 delegator intersection `getReadFilter` now answers for that context (ruling +[`6303721609`](https://github.com/objectstack-ai/objectstack/issues/24932#issuecomment-6303721609) on +[#24932](https://github.com/objectstack-ai/objectstack/issues/24932), letter B; the engine card +[#25009](https://github.com/objectstack-ai/objectstack/issues/25009)). D6 and the execution plan carry the rest of that +ruling. **This record declares; it implements nothing.** It became **Accepted** on the maintainer's approval (`docs/adr/**` is Tier H, Prime Directive #14); the execution cards under [Execution plan](#execution-plan-after-acceptance) are cut after acceptance and ⛔ never before it. Until then the platform stayed in the ruling's letter D state, and letter C (a located 400 at the bound, after objectui#12081 item 8) is the `domain:services` lane's interim on @@ -26,7 +32,8 @@ keys on), [ADR-0090](./0090-permission-model-v2-concept-convergence.md) D10 (del D5 (principal-less refusal), [ADR-0049](./0049-no-unenforced-security-properties.md) (enforce-or-remove: why no capability bit returns), [ADR-0112](./0112-error-code-vocabulary-and-ledger.md) (the refusal envelope), [ADR-0057](./0057-system-data-lifecycle-and-retention.md) (the `telemetry` datasource split that makes a parent federated). -**Leaves unchanged**: ADR-0055 §2's chosen mechanism (b) for `controlled_by_parent`, its `rlsMembership` IN-form and +**Leaves unchanged**: ADR-0055 §2's chosen mechanism (b) for `controlled_by_parent` (until the plugin-security switch +the 2026-10-21 amendment of D6 orders lands, in its own Tier H edit of that record), its `rlsMembership` IN-form and the fail-closed RLS compiler and its closed pushdown form set (historically four, broader today); ADR-0053 D-D1 (drivers receive lowered input) for every form but this one leaf. **Consumers**: `@objectstack/spec` (the leaf's declaration), `@objectstack/objectql` (composition and push-down), @@ -198,16 +205,31 @@ restriction". `managedBy: 'append-only'`, so the backfill goes through the migration door, never the CRUD write path). A row the backfill cannot judge keeps the fail-closed answer it has today. -### D5 — Three classes stay on the bounded fail-closed probe, by name - -Each is recognised by the engine, which **refuses to compose a leaf for it** (loud, as D3's envelope), and routed by the -consumer to today's probe before the leaf is written. ⛔ No silent fallback: the probe is chosen up front, never -reached by catching a refusal. - -1. **A delegated (on-behalf-of) caller** (ADR-0090 D10). `packages/plugins/plugin-security/src/security-plugin.ts#getReadFilter` - answers the deny sentinel for such a context because the delegator intersection is not threaded through the - read-scope path, while the CRUD middleware intersects it; a composed leaf would serve a delegated reader nothing. - The probe, which reads through the middleware, stays until the intersection reaches `getReadFilter`. +### D5 — Two classes stay on the bounded fail-closed probe, by name; the delegated caller composes (amended 2026-10-21) + +Each of the two remaining classes is recognised by the engine, which **refuses to compose a leaf for it** (loud, as D3's +envelope), and routed by the consumer to today's probe before the leaf is written. ⛔ No silent fallback: the probe is +chosen up front, never reached by catching a refusal. + +1. **A delegated (on-behalf-of) caller** (ADR-0090 D10) — **composed, not refused** (amended 2026-10-21; as accepted + this class was refused "until the intersection reaches `getReadFilter`", and it has). The engine's boundary + (`packages/objectql/src/engine.ts#composeReadableIdsAtBoundary`) asks both halves — `canReadObject` and + `getReadFilter` — with the delegated context exactly as it stands, `onBehalfOf` intact, and composes what it is + answered; it reads no delegation link of its own + (`packages/objectql/src/engine.ts#refuseUncomposableReadableIdsClasses` judges the two classes below and nothing + about the caller). The security service answers each half for the delegated principal, which under ADR-0090 D10 is + the INTERSECTION of the principal's leg and the delegator's: `canReadObject` already intersected the object half; + `packages/plugins/plugin-security/src/security-plugin.ts#getReadFilter` now answers the row half through + `packages/plugins/plugin-security/src/security-plugin.ts#computeDelegatedReadFilter`, the six layers the engine + middleware chain ANDs into a delegated `find` of P — the principal's RLS and `controlled_by_parent` derivation, the + delegator's two (over the context `packages/plugins/plugin-security/src/explain-engine.ts#resolveDelegatorContext` + rebuilds), and plugin-sharing's visibility filter under each identity at its own depth. So a delegated read through + the leaf serves exactly the rows a delegated direct read of P serves (measured: `find`, `count`, one statement per + read, the parent inside the subquery). The closed doors are the chain's: a delegator who does not exist, one the + parent object does not admit, a principal that resolves no set, and any resolution failure answer `false` or the + deny sentinel, and the leaf is `$in: []`. ⛔ The engine derives no second intersection: it has one context, the + delegated one, and the delegator's grants are the security service's to resolve. The six consumers' up-front + `'delegated-caller'` probe routing (D4) is now a residue of the refusal, retired per consumer on its own card. 2. **A parent whose rows are narrowed by an object-scoped read middleware** (`packages/objectql/src/engine.ts#registerMiddleware` with an `object`): `sys_attachment`, `sys_comment`, `sys_comment_reaction`, `sys_approval_request`, and the two gated objects themselves. Their WHERE is built at read time and is no part of `getReadFilter`, so a leaf over them @@ -226,11 +248,28 @@ compiled output of the RLS compiler, whose closed pushdown form set (historicall 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 is ⛔ not +`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. 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. +**Amended 2026-10-21 — O5 is decided, by the maintainer's ruling +[`6303721609`](https://github.com/objectstack-ai/objectstack/issues/24932#issuecomment-6303721609) on +[#24932](https://github.com/objectstack-ai/objectstack/issues/24932) (letter B), recorded here verbatim:** + +> O5 is answered yes, in the end-state form. `controlled_by_parent` becomes the seventh consumer of the engine-internal +> semi-join leaf, once for every shape: the engine first composes the leaf for a delegated (on-behalf-of) caller (the +> exit condition D5.1 itself names) and for a nested master chain; then plugin-security switches the caller, the +> delegator intersection and `getReadFilter` in one change, and the id-list pre-scan retires. + +The order the ruling binds: the delegated composition (D5.1 as amended, this revision's card), then the nested master +chain (a leaf inside a composed scope is composed; its own `domain:engine` card), then the plugin-security switch, +"which in the same Tier H edit corrects ADR-0055's 'cached within the request' sentence to what the code does". Two +things the ruling ⛔ does not take: an interim with two spellings of one scope, and "interim removal of the predicate for +an unrestricted master scope: it would expose rows with a null or dangling foreign key (measured)". ADR-0055 §2's +mechanism (b) therefore stands until the switch lands, and ADR-0055's own text moves only in that switch's edit, never +here. + ## Consequences - **Positive.** A reader gets every row they may see and a `total` that is true, on any table size; the pre-scan, @@ -269,7 +308,8 @@ bullet; nothing else in either moves. - **O3** — `driver-mongodb`: the cost of a pipeline form is unprototyped; it refuses until measured. - **O4** — `packages/plugins/plugin-sharing/src/sharing-service.ts` reads a caller's record-share grants at `limit: 5000` with no warning; that scope rides into the subquery unchanged. A separate finding, not this record's. -- **O5** — whether `controlled_by_parent`'s id set becomes a seventh consumer (D6). +- **O5** — whether `controlled_by_parent`'s id set becomes a seventh consumer (D6). **Answered** (2026-10-21): the + ruling is recorded verbatim in D6, as amended. ## Acceptance criteria @@ -284,7 +324,7 @@ and none changes `packages/spec` before it. | Card | Decisions | What lands | Pins (red before, green after) | Order | |:--|:--|:--|:--|:--| | **E1** — the leaf | D1 | `ReadableIds` in `packages/spec/src/data/`; the comparand door admits the branded value and refuses the unbranded one | a JSON round-trip of a `where` holding the leaf is refused; the plain-object spelling is refused 400 | first | -| **E2** — composition | D2, D5 | the engine seam composes the leaf, refuses it on write verbs and for D5's three classes, marks the scope `'policy'`; the conformance case-set | `canReadObject` false ⇒ no row; deny sentinel ⇒ no row; a delegated, middleware-gated or federated parent ⇒ the loud refusal | after E1 | +| **E2** — composition | D2, D5 | the engine seam composes the leaf, refuses it on write verbs and for D5's three classes as accepted (D5.1 amended 2026-10-21: the delegated caller composes), marks the scope `'policy'`; the conformance case-set | `canReadObject` false ⇒ no row; deny sentinel ⇒ no row; a delegated caller composes (both halves asked with the delegation link intact, the closed doors still `$in: []`); a middleware-gated or federated parent ⇒ the loud refusal, a delegated context included | after E1 | | **E3** — SQL family | D3 | `driver-sql` compiles the leaf inside `packages/drivers/driver-sql/src/sql-driver.ts#applyTenantScope`; wasm and local Turso inherit; the remote transport compiles it | a reader with more than 2,000 readable candidates gets every visible row; an unreadable row stays hidden; a deleted parent is excluded; `count` equals the rows served | after E2 | | **E4** — memory and mongodb | D3 | memory implements; mongodb refuses with the envelope | the conformance table records both | with E3 | | **E5** — the activity and ledger switch | D4, D5 | `packages/plugins/plugin-audit/src/parent-record-read-gate.ts#computeParentRecordFilter` emits leaves, no pre-scan; D5's classes keep the probe; letter C's refusal retired from these two | the measured scenario answers 1,808 of 1,808; the exemption capability unchanged; the outside class as a predicate | after E3 | diff --git a/packages/objectql/src/engine-readable-ids-composition.test.ts b/packages/objectql/src/engine-readable-ids-composition.test.ts index 0e05c00cbec..5ce354099c0 100644 --- a/packages/objectql/src/engine-readable-ids-composition.test.ts +++ b/packages/objectql/src/engine-readable-ids-composition.test.ts @@ -15,8 +15,12 @@ * * Refused by name, `INTERNAL_ERROR` / 500, before any driver is asked: a write * verb, `having`, an aggregation's filter, any position but a `$in` - * comparand, a composed leaf the engine did not compose, and D5's three - * classes (a delegated caller, a middleware-gated parent, a federated parent). + * comparand, a composed leaf the engine did not compose, and D5's two + * remaining classes (a middleware-gated parent, a federated parent). A + * delegated (on-behalf-of) caller composes like any other (D5.1 as amended): + * both halves are asked with the delegated context as it stands, and what the + * security service answers for that context — under ADR-0090 D10 the + * intersection of the two legs — is the scope. * * The recording driver is this package's witness shape (the in-memory * driver's test consumers are a closed census); evaluating the composed leaf @@ -87,18 +91,27 @@ const leafOf = (object = 'rid_parent') => ReadableIds.of(object); const branch = (object = 'rid_parent') => ({ object_name: object, record_id: { $in: leafOf(object) } }); const COUNT_AGG = [{ function: 'count', alias: 'n' }]; -/** A security service double that records which half it was asked, per object. */ +/** A security service double that records which half it was asked, per object, and the context each half received. */ function makeSecurity(opts: { - canRead?: (object: string) => boolean | Promise; - scope?: (object: string) => unknown; + canRead?: (object: string, context?: unknown) => boolean | Promise; + scope?: (object: string, context?: unknown) => unknown; } = {}) { const asked: string[] = []; + const contexts: unknown[] = []; const svc: any = { - async canReadObject(object: string) { asked.push(`canReadObject:${object}`); return opts.canRead ? opts.canRead(object) : true; }, - async getReadFilter(object: string) { asked.push(`getReadFilter:${object}`); return opts.scope ? opts.scope(object) : undefined; }, + async canReadObject(object: string, context?: unknown) { + asked.push(`canReadObject:${object}`); + contexts.push(context); + return opts.canRead ? opts.canRead(object, context) : true; + }, + async getReadFilter(object: string, context?: unknown) { + asked.push(`getReadFilter:${object}`); + contexts.push(context); + return opts.scope ? opts.scope(object, context) : undefined; + }, async explain() { asked.push('explain'); return { allowed: false }; }, }; - return { svc, asked }; + return { svc, asked, contexts }; } /** The `$in` comparand the driver received at `where.record_id`, under any `$and` the chain wrapped around it. */ @@ -344,6 +357,133 @@ describe('[ADR-0139 D2] the executor boundary composes the ReadableIds leaf', () }); }); +describe('[ADR-0139 D5.1, as amended] a delegated (on-behalf-of) caller composes — both halves asked with the delegated context as it stands', () => { + let engine: ObjectQL; + let calls: SeenCall[]; + /** The shape the OAuth door assembles: `userId` stays the human, the link names them as the delegator. */ + const DELEGATED = { userId: 'usr_owner', principalKind: 'agent', onBehalfOf: { userId: 'usr_owner', principalKind: 'human' } }; + const delegated = () => ({ ...DELEGATED, onBehalfOf: { ...DELEGATED.onBehalfOf } }) as any; + + beforeEach(async () => { + const rec = makeRecordingDriver(); + const farRec = makeRecordingDriver('far'); + calls = rec.calls; + engine = new ObjectQL(); + engine.registerDriver(rec.driver, true); + engine.registerDriver(farRec.driver); + await engine.init(); + for (const def of [CHILD, PARENT, OTHER, GATED, FAR]) engine.registry.registerObject(def as any, 'test'); + engine.registerMiddleware(async (_ctx, next) => next(), { object: 'rid_gated' }); + calls.length = 0; + }); + + const delegatedReads = (where: Record) => [ + ['find', () => engine.find('rid_child', { where, context: delegated() })], + ['findOne', () => engine.findOne('rid_child', { where, context: delegated() })], + ['count', () => engine.count('rid_child', { where, context: delegated() })], + ['aggregate', () => engine.aggregate('rid_child', { where, aggregations: COUNT_AGG as any, context: delegated() })], + ] as const; + + it('each read verb asks both halves with the delegation link intact and hands its driver ONE call carrying the composed leaf — scope by reference, marked policy', async () => { + // What the service answers for the delegated context stands for the + // ADR-0090 D10 intersection; the engine composes it, it derives nothing. + const scope = { $and: [{ owner: 'usr_owner' }, { title: { $ne: 'secret' } }] }; + const { svc, asked, contexts } = makeSecurity({ scope: () => scope }); + wire(engine, () => svc); + for (const [verb, run] of delegatedReads(branch())) { + calls.length = 0; + asked.length = 0; + contexts.length = 0; + await run(); + expect(calls.map((c) => c.verb), verb).toEqual([verb]); + expect(asked, verb).toEqual(['canReadObject:rid_parent', 'getReadFilter:rid_parent']); + // Both halves received the caller's context as it stands: the link the + // service intersects on is there, and the principal is the delegated one + // — never a context the engine rebuilt for the delegator. + for (const ctx of contexts as Array>) { + expect(ctx.onBehalfOf, verb).toEqual(DELEGATED.onBehalfOf); + expect(ctx.userId, verb).toBe(DELEGATED.userId); + expect(ctx.principalKind, verb).toBe('agent'); + } + const composed = inComparandOf(calls[0].ast.where) as any; + expect(isComposedReadableIds(composed), verb).toBe(true); + expect(composed.object, verb).toBe('rid_parent'); + expect(composed.scope, verb).toBe(scope); + } + expect(filterSubtreeProvenanceOf(scope)).toBe('policy'); + }); + + it('a leaf written by an object-scoped middleware on the child is composed for a delegated caller too', async () => { + const scope = { owner: 'usr_owner' }; + const { svc, contexts } = makeSecurity({ scope: () => scope }); + wire(engine, () => svc); + engine.registerMiddleware(async (ctx, next) => { + if (ctx.ast && ['find', 'findOne', 'count', 'aggregate'].includes(ctx.operation)) { + ctx.ast.where = ctx.ast.where ? { $and: [ctx.ast.where, branch()] } : branch(); + } + await next(); + }, { object: 'rid_child' }); + for (const [verb, run] of delegatedReads({ note: 'x' })) { + calls.length = 0; + contexts.length = 0; + await run(); + const composed = inComparandOf(calls[0].ast.where) as any; + expect(isComposedReadableIds(composed), verb).toBe(true); + expect(composed.scope, verb).toBe(scope); + expect(contexts.map((c) => (c as any)?.onBehalfOf?.userId), verb).toEqual(['usr_owner', 'usr_owner']); + } + }); + + it('the closed doors stay closed under a delegated context: canReadObject false, the deny sentinel, a throw on either half ⇒ $in: []', async () => { + const doors: ReadonlyArray]> = [ + ['canReadObject false', makeSecurity({ canRead: () => false, scope: () => ({ owner: 'usr_owner' }) })], + ['the deny sentinel, by identity', makeSecurity({ scope: () => RLS_DENY_FILTER })], + ['canReadObject throws', (() => { + const s = makeSecurity({ scope: () => ({ owner: 'usr_owner' }) }); + s.svc.canReadObject = async () => { throw new Error('canReadObject is down'); }; + return s; + })()], + ['getReadFilter throws', (() => { + const s = makeSecurity(); + s.svc.getReadFilter = async () => { throw new Error('getReadFilter is down'); }; + return s; + })()], + ]; + for (const [label, { svc }] of doors) { + wire(engine, () => svc); + for (const [verb, run] of delegatedReads(branch())) { + calls.length = 0; + await run(); + expect(calls.map((c) => c.verb), `${label}: ${verb}`).toEqual([verb]); + expect(inComparandOf(calls[0].ast.where), `${label}: ${verb}`).toEqual([]); + } + } + }); + + it('D5.2 and D5.3 keep their refusal under a delegated context: code and status, no driver call, the service unasked', async () => { + const { svc, asked } = makeSecurity(); + wire(engine, () => svc); + const gated = await refusalOf(engine.count('rid_child', { where: branch('rid_gated'), context: delegated() })); + expect(envelopeOf(gated)).toEqual(REFUSED); + expect(gated!.message).toContain("'rid_gated', whose rows an object-scoped read middleware narrows"); + const far = await refusalOf(engine.aggregate('rid_child', { where: branch('rid_far'), aggregations: COUNT_AGG as any, context: delegated() })); + expect(envelopeOf(far)).toEqual(REFUSED); + expect(far!.message).toContain("'rid_far', which is bound to datasource 'far'"); + expect(calls).toHaveLength(0); + expect(asked).toEqual([]); + }); + + it('CONTROL — the same read without the delegation link composes the same way, so the link is not what the engine keys on', async () => { + const scope = { owner: 'usr_owner' }; + const { svc, asked, contexts } = makeSecurity({ scope: () => scope }); + wire(engine, () => svc); + await engine.find('rid_child', { where: branch(), context: { userId: 'usr_owner', principalKind: 'human' } as any }); + expect(asked).toEqual(['canReadObject:rid_parent', 'getReadFilter:rid_parent']); + expect(contexts.map((c) => (c as any)?.onBehalfOf)).toEqual([undefined, undefined]); + expect((inComparandOf(calls[0].ast.where) as any).scope).toBe(scope); + }); +}); + describe('[ADR-0139 D1, D5] what the engine never composes is refused by name — no driver call', () => { let engine: ObjectQL; let calls: SeenCall[]; @@ -365,17 +505,6 @@ describe('[ADR-0139 D1, D5] what the engine never composes is refused by name calls.length = 0; }); - it('D5 — a delegated (on-behalf-of) caller', async () => { - const err = await refusalOf(engine.find('rid_child', { - where: branch(), - context: { userId: 'usr_agent', onBehalfOf: { userId: 'usr_owner' } } as any, - })); - expect(envelopeOf(err)).toEqual(REFUSED); - expect(err!.message).toContain('on-behalf-of'); - expect(calls).toHaveLength(0); - expect(asked, 'D5 is decided before the security service is asked').toEqual([]); - }); - it('D5 — a parent an object-scoped read middleware narrows', async () => { const err = await refusalOf(engine.count('rid_child', { where: branch('rid_gated') })); expect(envelopeOf(err)).toEqual(REFUSED); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index dd71823aff7..2879fdae7f2 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -6140,11 +6140,11 @@ export class ObjectQL implements IObjectQLEngine { * * Per parent object `P`, once per call, in this order: * - * 1. **D5's three classes are refused, by name** (`INTERNAL_ERROR` / 500), - * before the security service is asked anything: a delegated - * (on-behalf-of) caller; a parent an object-scoped read middleware - * narrows ({@link hasObjectMiddleware}); a parent bound to another - * datasource than `object` ({@link resolveDatasourceBinding}). + * 1. **D5's two remaining classes are refused, by name** (`INTERNAL_ERROR` + * / 500), before the security service is asked anything: a parent an + * object-scoped read middleware narrows ({@link hasObjectMiddleware}); a + * parent bound to another datasource than `object` + * ({@link resolveDatasourceBinding}). * 2. **A parent the registry does not hold** ⇒ `$in: []`, unasked. * 3. **The security service** — absent or unusable ⇒ `$in: []`. * 4. **`canReadObject(P)`** (its `explain` fallback when the method is @@ -6155,6 +6155,17 @@ export class ObjectQL implements IObjectQLEngine { * `'policy'` and otherwise passed exactly as `P`'s own read passes its * injected scope. The tenant wall is the driver's (D3). * + * [ADR-0139 D5.1, as amended] A **delegated (on-behalf-of) caller** is + * composed, not refused: both halves are asked with the caller's context + * exactly as it stands — `onBehalfOf` intact — and the security service + * answers each half for the delegated principal, which under ADR-0090 D10 + * is the INTERSECTION of the principal's leg and the delegator's. The + * engine composes what it is answered and derives no second intersection + * of its own: it has one context, the delegated one, and the delegator's + * grants are the security service's to resolve. A service that cannot + * answer that context answers the deny sentinel, and step 5 turns it into + * `$in: []`, closed. + * * Nothing is memoised across calls. `aggregate`'s `having` and its * aggregations' own filters are judged here too, so a leaf a middleware * wrote there is refused rather than evaluated in memory. @@ -6174,7 +6185,7 @@ export class ObjectQL implements IObjectQLEngine { } const sites = collectComposableReadableIds(object, operation, ast.where, 'where'); if (sites.length === 0) return ast; - const parents = this.refuseUncomposableReadableIdsClasses(object, operation, sites, context); + const parents = this.refuseUncomposableReadableIdsClasses(object, operation, sites); const security = this.resolveReadableIdsSecurity(); if (security.kind !== 'usable' && !this.readableIdsSecurityReported.has(security.kind)) { @@ -6202,17 +6213,18 @@ export class ObjectQL implements IObjectQLEngine { } /** - * [ADR-0139 D5] Refuse D5's three classes, by name, and return the distinct - * parent objects of `sites` the security service is asked about, in walk - * order (every one but a parent the registry does not hold). Reads the - * context, the registry and the middleware registrations only — no security - * call. + * [ADR-0139 D5] Refuse D5's two remaining classes (D5.2 and D5.3), by name, + * and return the distinct parent objects of `sites` the security service is + * asked about, in walk order (every one but a parent the registry does not + * hold). Reads the registry and the middleware registrations only — no + * security call, and no reading of the caller's context: a delegated + * (on-behalf-of) caller is no longer a class of its own (D5.1, as amended), + * it is composed like any other through the security service's answers. */ private refuseUncomposableReadableIdsClasses( object: string, operation: string, sites: readonly ComposableReadableIdsSite[], - context: ExecutionContext | undefined, ): string[] { const firstSite = new Map(); for (const site of sites) if (!firstSite.has(site.leaf.object)) firstSite.set(site.leaf.object, site); @@ -6222,11 +6234,6 @@ export class ObjectQL implements IObjectQLEngine { `such leaf ${why}. The query was NOT run and no driver was asked. Read this class through its bounded ` + 'probe, chosen before the leaf is written.', ); - const delegator = (context as { onBehalfOf?: { userId?: unknown } } | undefined)?.onBehalfOf?.userId; - if (delegator) { - throw refuse(sites[0], 'for a delegated (on-behalf-of) caller: the parent\'s read scope does not carry the ' + - 'delegator intersection, so the composed leaf would not answer what a direct read answers'); - } const outer = this.resolveDatasourceBinding(object)?.datasource; const composable: string[] = []; for (const [parent, site] of firstSite) { 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 0466fda8c59..cac1868247c 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 @@ -665,11 +665,13 @@ describe('[#5815] getReadFilter enforces the same read scope as the engine middl expect(reads).not.toContain('crm_contact'); }); - it('a delegated (on-behalf-of) read still denies outright — unchanged by the added layer', async () => { - // [#2852] The D10 delegator intersection is not implemented on this path, - // so it fails closed BEFORE any layer is composed. Pinned here because the + it('a delegated (on-behalf-of) read whose delegator does not exist still denies outright — unchanged by the added layer', async () => { + // [ADR-0090 D10] getReadFilter composes the delegator intersection for a + // delegated context (ADR-0139 D5.1 as amended). This fixture stores no + // `sys_user` row, so the link names a delegator who does not exist and the + // read fails closed BEFORE any layer is composed. Pinned here because the // added layer must not become a reason to compute a scope for a context - // this method refuses to scope at all. + // whose delegator cannot be resolved. const h = await boot({ shareLevel: 'edit' }); const delegated = { ...h.repContext(), onBehalfOf: { userId: OTHER } }; expect(await h.analyticsVisibleContacts(delegated)).toEqual([]); diff --git a/packages/plugins/plugin-security/src/default-report-sink.test.ts b/packages/plugins/plugin-security/src/default-report-sink.test.ts index 11cd651754e..a31d907dccb 100644 --- a/packages/plugins/plugin-security/src/default-report-sink.test.ts +++ b/packages/plugins/plugin-security/src/default-report-sink.test.ts @@ -22,8 +22,10 @@ * Case 1 drives a REAL report site on a bare instance, so the pin is about what * an operator sees rather than about a field's contents. `getReadFilter` with an * on-behalf-of context is the one refusal reachable with no `ql`, no `metadata` - * and no `start()`: it fails closed on the unimplemented D10 delegator - * intersection (#2852) and reports through `this.logger.error?.()`. + * and no `start()`: the ADR-0090 D10 intersection it composes (ADR-0139 D5.1 as + * amended) needs the delegator's `sys_user` row, which a bare instance cannot + * read, so the link names a delegator who does not exist and the read fails + * closed on that dangling link, reporting through `this.logger.error?.()`. * * Case 2 pins the channel the LEDGER row is about. `error` stays optional by * #9754's own ruling (hosts inject reduced sinks); `warn` is the channel a diff --git a/packages/plugins/plugin-security/src/readable-ids-delegated-compose.test.ts b/packages/plugins/plugin-security/src/readable-ids-delegated-compose.test.ts new file mode 100644 index 00000000000..042c9a6325d --- /dev/null +++ b/packages/plugins/plugin-security/src/readable-ids-delegated-compose.test.ts @@ -0,0 +1,399 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0139 D5.1, as amended / ADR-0090 D10] The engine composes its + * `ReadableIds` leaf for a DELEGATED (on-behalf-of) caller, and the scope it is + * composed with is the delegator intersection: `getReadFilter` answers the same + * six layers the engine middleware chain ANDs into a delegated `find` of the + * parent — the principal's RLS and `controlled_by_parent` derivation, the + * delegator's two, and plugin-sharing's visibility filter for each identity at + * its own depth — so a child read through the leaf serves exactly the children + * whose parent BOTH legs admit. + * + * Measured on the real stack: `ObjectQL` over better-sqlite3, the real + * `SecurityPlugin` with the real `SharingService` behind it, and the delegated + * context assembled by `resolveAuthzContext` and `assembleExecutionContext` as + * the OAuth door assembles it — `userId` the human, the link naming them as the + * delegator, the agent's ceiling set as its only grant. + * + * The two legs are made to disagree on purpose: + * + * - the CEILING admits a parent row by its content (`name == 'open'`) and is + * silent on depth, so its leg carries no owner narrowing (the agent leg's + * depth is `org` for a silent ceiling, `intersectDelegatedScope`); + * - the DELEGATOR's own set is silent on rows, so their leg is plugin-sharing's + * owner-match on a `private` parent plus the one record shared to them. + * + * So `p_caller_only` (open, owned by someone else) is admitted by the caller + * leg alone, `p_del_only` (secret, the delegator's own) by the delegator leg + * alone, and only `p_both` and `p_shared` by both. The closed doors are + * measured beside it: a delegator who does not exist, a delegator the parent + * object does not admit, and the two D5 classes that still refuse. + */ + +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 { ReadableIds, RLS_DENY_FILTER } 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'; + +const SYS = { context: { isSystem: true } } as never; +const ORG = 'org_rid_del'; + +/** The parent the leaf is over: `private`, so plugin-sharing's owner-match scopes it per identity. */ +const PARENT = 'qa_rid_del_parent'; +/** A child pointing at the parent; a consumer-shaped middleware writes the leaf into its reads. */ +const CHILD = 'qa_rid_del_child'; +/** A parent an object-scoped read middleware narrows: D5.2, still refused. */ +const GATED = 'qa_rid_del_gated'; + +/** The delegator: the human an agent acts for, and the delegated context's own `userId`. */ +const HUMAN = 'u_rid_del_human'; +/** Owns the rows the delegator does not. */ +const OTHER = 'u_rid_del_other'; +/** A delegator whose grants do not admit the parent object. */ +const NO_PARENT_READ = 'u_rid_del_no_parent_read'; + +const SET = { + /** The agent's ceiling: object read on both, a content rule on the parent's rows, silent on depth. */ + ceiling: 'qa_rid_del_ceiling', + /** The delegator's own set: object read on both, silent on rows. */ + human: 'qa_rid_del_human', + /** A set that admits the child but not the parent object. */ + childOnly: 'qa_rid_del_child_only', +} as const; + +type Outcome = { kind: 'landed' } | { kind: 'refused'; code?: string; status?: number }; + +const outcomeOf = (p: Promise): Promise => + p.then( + () => ({ kind: 'landed' as const }), + (e: unknown) => { + const x = e as { code?: unknown; status?: unknown; statusCode?: 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), + }; + }, + ); + +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 leafBranch = () => ({ parent: { $in: ReadableIds.of(PARENT) } }); + +async function boot() { + const engine = new ObjectQL(); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) as never, + true, + ); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.qa.readable-ids-delegated-compose', + name: 'The ReadableIds leaf composes for a delegated caller with the delegator intersection', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + SysPosition, SysPermissionSet, SysPositionPermissionSet, SysUserPosition, SysUserPermissionSet, + SysOrganization, SysUser, SysMember, SysRecordShare, + { + name: PARENT, + label: 'Parent', + sharingModel: 'private', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + name: { name: 'name', type: 'text' }, + owner_id: { name: 'owner_id', type: 'lookup', reference: 'sys_user' }, + }, + }, + { + name: CHILD, + label: 'Child', + sharingModel: 'public_read_write', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + note: { name: 'note', type: 'text' }, + parent: { name: 'parent', type: 'text' }, + }, + }, + { + name: GATED, + label: 'Gated parent', + sharingModel: 'public_read_write', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + name: { name: 'name', type: 'text' }, + }, + }, + ], + } as never); + await engine.syncSchemas(); + + const e = engine as unknown as { + insert: (o: string, row: Record, opts: never) => Promise; + getDriver: (o: string) => { create: (o: string, row: Record) => Promise; knex: { 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, NO_PARENT_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 sets = [ + ...defaultPermissionSets, + PermissionSetSchema.parse({ + name: SET.ceiling, + objects: { [PARENT]: READ, [CHILD]: READ, [GATED]: READ }, + rowLevelSecurity: [ + { name: 'qa_rid_del_ceiling_open_only', object: PARENT, operation: 'select', using: "record.name == 'open'" }, + ], + }), + PermissionSetSchema.parse({ name: SET.human, objects: { [PARENT]: READ, [CHILD]: READ, [GATED]: READ } }), + PermissionSetSchema.parse({ name: SET.childOnly, objects: { [CHILD]: READ } }), + ]; + // The delegators hold their sets through stored grants, as a user does. + for (const name of [SET.human, SET.childOnly]) { + 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.human], [OTHER, SET.human], [NO_PARENT_READ, SET.childOnly]] as const) { + await e.insert('sys_user_permission_set', { id: `ups_${user}`, user_id: user, permission_set: set, organization_id: ORG }, SYS); + } + + // The parent rows, named for the leg(s) that admit them. + await e.insert(PARENT, { id: 'p_both', name: 'open', owner_id: HUMAN }, SYS); + await e.insert(PARENT, { id: 'p_del_only', name: 'secret', owner_id: HUMAN }, SYS); + await e.insert(PARENT, { id: 'p_caller_only', name: 'open', owner_id: OTHER }, SYS); + await e.insert(PARENT, { id: 'p_neither', name: 'secret', owner_id: OTHER }, SYS); + await e.insert(PARENT, { id: 'p_shared', name: 'open', owner_id: OTHER }, SYS); + await e.insert('sys_record_share', { + id: 'shr_rid_del_1', object_name: PARENT, record_id: 'p_shared', recipient_type: 'user', recipient_id: HUMAN, access_level: 'read', + }, SYS); + for (const [id, parent] of [ + ['c_both', 'p_both'], ['c_del_only', 'p_del_only'], ['c_caller_only', 'p_caller_only'], + ['c_neither', 'p_neither'], ['c_shared', 'p_shared'], ['c_dangling', 'p_missing'], ['c_null', null], + ] as const) { + await e.insert(CHILD, { id, note: '', parent }, SYS); + } + await e.insert(GATED, { id: 'g_1', name: 'open' }, SYS); + + 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 ctx = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + 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. The + // key is `Symbol.for`, the one slot every bundle of the engine names. + (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 CHILD writes + // the leaf over the parent into every read, after the security chain. + engine.registerMiddleware(async (opCtx, next) => { + if (opCtx.ast && ['find', 'findOne', 'count', 'aggregate'].includes(opCtx.operation)) { + opCtx.ast.where = opCtx.ast.where ? { $and: [opCtx.ast.where, leafBranch()] } : leafBranch(); + } + await next(); + }, { object: CHILD }); + // D5.2's shape: an object-scoped read middleware on a parent. + engine.registerMiddleware(async (_opCtx, next) => next(), { object: GATED }); + 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, + onBehalfOf, + read: async (object: string, context: unknown, where?: Record) => + ids(await engine.find(object, { ...(where ? { where } : {}), 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)), + /** The ids `getReadFilter`'s scope admits, read with it as the only predicate under a system context. */ + 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)); + }, + 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 ?? '')); }; + const knex = e.getDriver(CHILD).knex; + 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); + }, + }; +} + +type Rig = Awaited>; + +const member = (userId: string, set: string) => ({ userId, positions: [], permissions: [set], posture: 'MEMBER' }); + +describe('[ADR-0139 D5.1 as amended] a delegated read through the leaf serves the delegator intersection', () => { + let rig: Rig; + beforeAll(async () => { rig = await boot(); }); + afterAll(async () => { await rig.teardown(); }); + + it("the legs, measured apart: the delegator's own read, and the ceiling's rule on a principal who owns the other rows", async () => { + // The delegator's own leg: owner-match plus the one share, every name. + expect(await rig.read(PARENT, member(HUMAN, SET.human))).toEqual(['p_both', 'p_del_only', 'p_shared']); + // The ceiling's content rule, held directly by the owner of the other rows: + // `open` only. Its leg under delegation carries no owner narrowing at all + // (a silent ceiling's depth is `org`), so it admits `p_caller_only` too. + expect(await rig.read(PARENT, member(OTHER, SET.ceiling))).toEqual(['p_caller_only', 'p_shared']); + }); + + it("the delegated read of the parent itself is the intersection: the delegator's rows the ceiling admits", async () => { + const context = await rig.onBehalfOf(HUMAN); + expect(await rig.read(PARENT, context)).toEqual(['p_both', 'p_shared']); + }); + + it('a delegated read through the leaf serves exactly the children whose parent both legs admit — find, count', async () => { + const context = await rig.onBehalfOf(HUMAN); + // `p_caller_only` is admitted by the caller leg alone, `p_del_only` by the + // delegator leg alone: neither child is served. A dangling or null pointer + // is excluded as before (the subquery finds no such id). + expect(await rig.read(CHILD, context)).toEqual(['c_both', 'c_shared']); + expect(await rig.count(CHILD, context)).toBe(2); + }); + + it("CONTROL — the delegator's own read through the leaf serves their own rows' children, the ceiling not applied", async () => { + expect(await rig.read(CHILD, member(HUMAN, SET.human))).toEqual(['c_both', 'c_del_only', 'c_shared']); + }); + + it('getReadFilter answers the delegated context the same scope the delegated find runs under (parity, by rows)', async () => { + const context = await rig.onBehalfOf(HUMAN); + expect(await rig.readThroughReadFilter(PARENT, context)).toEqual(['p_both', 'p_shared']); + expect(await rig.readThroughReadFilter(PARENT, context)).toEqual(await rig.read(PARENT, context)); + }); + + it('one statement touches the parent table per delegated child find: the subquery, no pre-scan of the parent', async () => { + const context = await rig.onBehalfOf(HUMAN); + const statements = await rig.statementsDuring(() => rig.engine.find(CHILD, { context: { ...context } } as never)); + const touchingParent = statements.filter((sql) => sql.includes(PARENT)); + expect(touchingParent, statements.join('\n')).toHaveLength(1); + // The one statement is the child's own select, the parent inside its subquery. + expect(touchingParent[0]).toMatch(new RegExp(`from .${CHILD}.`, 'i')); + expect(touchingParent[0]).toMatch(/in \(select/i); + }); +}); + +describe('[ADR-0139 D5.1 as amended] the closed doors stay closed on a delegated read', () => { + let rig: Rig; + beforeAll(async () => { rig = await boot(); }); + afterAll(async () => { await rig.teardown(); }); + + it('a delegator who does not exist: every read of the principal is refused at admission, and both halves of the parent deny', async () => { + const context = { ...(await rig.onBehalfOf(HUMAN)), onBehalfOf: { userId: 'u_rid_del_missing', principalKind: 'human' } }; + // The chain refuses the principal before any leaf is written (the child + // read included), `canReadObject` answers false, and `getReadFilter` is + // the deny sentinel BY IDENTITY — the answer the engine turns into `$in: []`. + expect(await rig.readOutcome(PARENT, context)).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); + expect(await rig.readOutcome(CHILD, context)).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); + expect(await rig.canReadObject(PARENT, context)).toBe(false); + expect(await rig.readFilter(PARENT, context)).toBe(RLS_DENY_FILTER); + }); + + it('a delegator the parent object does not admit: the parent read is refused, canReadObject is false, the leaf serves nothing', async () => { + const context = await rig.onBehalfOf(NO_PARENT_READ); + expect(await rig.readOutcome(PARENT, context)).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 }); + // The object half decides (D2 step 1): the leaf is `$in: []` before + // `getReadFilter` is asked, which — as for a non-delegated principal — + // answers rows, not object admission. + expect(await rig.canReadObject(PARENT, context)).toBe(false); + expect(await rig.read(CHILD, context)).toEqual([]); + expect(await rig.count(CHILD, context)).toBe(0); + }); + + it('D5.2 — a leaf over a parent an object-scoped read middleware narrows is still refused, INTERNAL_ERROR / 500, under a delegated context', async () => { + const context = await rig.onBehalfOf(HUMAN); + const got = await outcomeOf(rig.engine.find(CHILD, { + where: { parent: { $in: ReadableIds.of(GATED) } }, + context: { ...context }, + } as never)); + expect(got).toMatchObject({ kind: 'refused', code: 'INTERNAL_ERROR', status: 500 }); + }); + + it('CONTROL — a system context reads every child, the leaf composed with no restriction', async () => { + expect(await rig.read(CHILD, { isSystem: true })).toEqual(['c_both', 'c_caller_only', 'c_del_only', 'c_neither', 'c_shared']); + }); +}); diff --git a/packages/plugins/plugin-security/src/rls-deny-sentinel-identity.test.ts b/packages/plugins/plugin-security/src/rls-deny-sentinel-identity.test.ts index d59eb539f98..7062744ccfc 100644 --- a/packages/plugins/plugin-security/src/rls-deny-sentinel-identity.test.ts +++ b/packages/plugins/plugin-security/src/rls-deny-sentinel-identity.test.ts @@ -155,7 +155,11 @@ describe('[ADR-0139 D2 step 2] every getReadFilter deny return is the spec senti expect(logger.error).toHaveBeenCalled(); }); - it('an on-behalf-of context (the D10 intersection is not on this path)', async () => { + it('an on-behalf-of context whose delegator does not exist (the D10 intersection fails closed on a dangling link)', async () => { + // This rig's engine double answers no `sys_user`, so the delegator the link + // names is `missing`: the intersection cannot be composed, and the deny is + // the sentinel by reference. A delegator who exists is scoped to the + // intersection instead (`readable-ids-delegated-compose.test.ts`). const { plugin } = await boot({ sets: [OPEN_SET] }); expect(await plugin.getReadFilter('task', { ...MEMBER, onBehalfOf: { userId: 'u_delegator' } })).toBe(SPEC_RLS_DENY_FILTER); }); diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index 2ab67120d06..e60fb3a3503 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -1657,12 +1657,14 @@ describe('SecurityPlugin', () => { expect(filter).toEqual(RLS_DENY_FILTER); }); - it('[#2852] fail-closed on an on-behalf-of (delegated) context — no D10 intersection on this path', async () => { - // getReadFilter resolves only the CALLER's ceiling; the delegator RLS - // intersection the engine middleware applies is absent here. A delegated - // read must DENY rather than silently return the agent's (wider) own - // scope. Latent today (no agent surface reaches analytics) but the - // invariant is enforced regardless. + it('[ADR-0090 D10] fail-closed on an on-behalf-of (delegated) context whose delegator does not exist', async () => { + // getReadFilter composes the D10 intersection for a delegated context + // (ADR-0139 D5.1 as amended; `readable-ids-delegated-compose.test.ts` + // measures it on the real stack). This harness answers no `sys_user` + // row, so the link names a delegator who does not exist: there is no + // leg to intersect against, and the read DENIES rather than serve the + // agent's own (wider) scope — the dangling-link rule every D10 gate + // shares, logged as the failure it is. const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); const harness = makeMiddlewareCtx({ permissionSets: [tenantPolicySet], orgScoping: true }); await plugin.init(harness.ctx); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 4cb6e250600..ae6ea4afab5 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -5421,6 +5421,14 @@ export class SecurityPlugin implements Plugin { object: string, context: any, resolvedSets?: PermissionSet[], + /** + * [ADR-0090 D10] The depth to stamp instead of the one `getEffectiveScope` + * answers for `resolvedSets` — what step 2.6 stamps on a DELEGATED read, + * where the agent leg carries the intersected depth and the delegator leg + * its own ({@link computeDelegatedReadFilter}). Absent on every + * non-delegated call, which resolves the depth here as before. + */ + depth?: string, ): Promise | null> { const sharing = this.resolveKernelService?.('sharing') as | { buildReadFilter?: (o: string, c: any) => Promise } @@ -5432,23 +5440,25 @@ export class SecurityPlugin implements Plugin { // [#5386] `resolvedSets` lets an in-middleware caller (the // controlled_by_parent derivation) pass the sets it already resolved for // THIS identity, instead of re-resolving them from the context. - let readScope: string | undefined; - try { - const permissionSets = resolvedSets ?? (await this.resolvePermissionSetsForContext(context)); - if (permissionSets.length > 0) { - const meta = await this.getObjectSecurityMeta(object); - readScope = this.permissionEvaluator.getEffectiveScope( - 'read', - object, - permissionSets, - { isPrivate: meta.isPrivate }, - ); + let readScope: string | undefined = depth; + if (readScope === undefined) { + try { + const permissionSets = resolvedSets ?? (await this.resolvePermissionSetsForContext(context)); + if (permissionSets.length > 0) { + const meta = await this.getObjectSecurityMeta(object); + readScope = this.permissionEvaluator.getEffectiveScope( + 'read', + object, + permissionSets, + { isPrivate: meta.isPrivate }, + ); + } + } catch { + // Depth is a WIDENING input: failing to resolve it leaves the owner-match + // at its narrowest ('own'), which is the safe direction. The sharing call + // below still runs — and its own failure still denies. + readScope = undefined; } - } catch { - // Depth is a WIDENING input: failing to resolve it leaves the owner-match - // at its narrowest ('own'), which is the safe direction. The sharing call - // below still runs — and its own failure still denies. - readScope = undefined; } const filter = await sharing.buildReadFilter(object, { ...context, @@ -5861,6 +5871,14 @@ export class SecurityPlugin implements Plugin { * 3. {@link resolveSharingReadFilter} — plugin-sharing's OWD / record-share * visibility filter, contributed by the sibling middleware. * + * [ADR-0090 D10] On a DELEGATED (on-behalf-of) context the chain composes + * each of the three twice — once for the delegated principal's leg, once for + * the delegator's — and so does this method ({@link computeDelegatedReadFilter}): + * the delegated read is scoped to the intersection, exactly as a delegated + * `find` is. ADR-0139 D5.1 named that intersection reaching this method as + * the condition under which the engine composes its `ReadableIds` leaf for a + * delegated caller; it does, so the engine does. + * * Each layer has been missing here at some point, and each absence had the * same shape: a caller who cannot read a row through `/data` could still * `COUNT(*)` / `GROUP BY` it through `/analytics`. Layer 3 was #4467; layer 2 @@ -5899,6 +5917,28 @@ export class SecurityPlugin implements Plugin { // refusal, mirrored: zero rows, read off the same predicate, before any // layer resolves. This used to answer the sharing predicate alone. if (isPrincipalLessContext(context)) return RLS_DENY_FILTER; + // [ADR-0090 D10 / ADR-0139 D5.1 as amended] A DELEGATED (on-behalf-of) + // context is scoped to the intersection of its own leg and its delegator's, + // composed as the engine middleware chain composes a delegated find + // ({@link computeDelegatedReadFilter}). This path used to answer the deny + // sentinel for every such context, because the intersection was threaded + // through the chain only; the engine's `ReadableIds` composition asks this + // method with the delegated context as it stands, so the intersection + // reaches here now. Fails CLOSED on every throw: a delegated read is never + // scoped wider than its delegator's own, and never served with a layer + // dropped. + if (context?.onBehalfOf?.userId) { + try { + return await this.computeDelegatedReadFilter(object, context); + } catch (e) { + this.logger.error?.( + `[security] getReadFilter failed for object '${object}' (principal ${context?.userId ?? 'unknown'} ` + + `on behalf of ${context.onBehalfOf.userId}) — denying (fail-closed)`, + e instanceof Error ? e : new Error(String(e)), + ); + return RLS_DENY_FILTER; + } + } // [#4467] The OWD/sharing predicate is resolved for EVERY non-system caller, // ahead of the RLS branches below, because it is a SEPARATE middleware in // the chain this method mirrors: none of the RLS stand-downs below is a @@ -5916,28 +5956,6 @@ export class SecurityPlugin implements Plugin { ); return RLS_DENY_FILTER; } - // [#2852] D10 delegator intersection is NOT implemented on this path. - // The engine middleware (find/count/aggregate) intersects an on-behalf-of - // read with the DELEGATOR's own RLS (resolveDelegatorContext, ~L689), but - // getReadFilter — the read-scope provider bound by the analytics/raw-SQL - // path — resolves only the CALLER's ceiling. Computing a filter here for a - // delegated (agent) context would therefore SILENTLY WIDEN the read past - // the delegator's scope (the confused-deputy widening D10 prevents on the - // CRUD path). Until the intersection is threaded through computeRlsFilter - // (tracked with #2920 B1 / ADR-0095 D1), FAIL CLOSED: a delegated read on - // this path denies rather than under-scopes. System on-behalf-of already - // returned above; today no agent surface reaches analytics, so this is a - // latent-invariant guard, not a live-traffic change. - if (context?.onBehalfOf?.userId) { - this.logger.error?.( - `[security] getReadFilter received an on-behalf-of context for object ` + - `'${object}' (agent ${context?.userId ?? 'unknown'} on behalf of ` + - `${context.onBehalfOf.userId}) — the D10 delegator intersection is not ` + - `implemented on the read-scope path; denying (fail-closed: a delegated read is never scoped ` + - `wider than its delegator's own)`, - ); - return RLS_DENY_FILTER; - } try { const permissionSets = await this.resolvePermissionSetsForContext(context); // [#21079] ADR-0056 D2 — the deny baseline at the row scope. A caller who @@ -5978,6 +5996,88 @@ export class SecurityPlugin implements Plugin { } } + /** + * [ADR-0090 D10 / ADR-0139 D5.1 as amended] The read scope of a DELEGATED + * (on-behalf-of) principal: the INTERSECTION of the principal's own leg and + * the delegator's — never the union — composed here exactly as the engine + * middleware chain composes a delegated `find` of `object`, layer for layer: + * + * 1. {@link computeRlsFilter} over the principal's sets, with the delegated + * context AS IT STANDS — {@link collectRLSPolicies} reads that context as + * the delegated principal's own leg (`isDelegatedPrincipalLeg`), so the + * ceiling's silence contributes no object default row scope; + * 2. {@link computeControlledByParentFilter} for the principal; + * 3. {@link computeRlsFilter} over the DELEGATOR's sets, with the context + * {@link resolveDelegatorContext} rebuilds for them (the middleware's + * D10 half, step 3); + * 4. {@link computeControlledByParentFilter} for the delegator; + * 5. plugin-sharing's visibility filter for the principal's identity, at + * the agent-leg depth step 2.6 stamps as `__readScope` + * (`intersectDelegatedScope` of the ceiling's DECLARED depth and the + * delegator's effective one); + * 6. the same filter re-run under the DELEGATOR's identity at the + * delegator's own depth — plugin-sharing's second call, with `userId` + * swapped to the delegator and no delegation link. + * + * Fails CLOSED where the chain refuses, with the deny sentinel by identity: + * a principal that resolves no set (ADR-0056 D2); a delegator the link names + * who does not exist (the chain's `PERMISSION_DENIED`; `canReadObject`'s + * `false`); a delegator who resolves no set (object admission refuses them); + * a sharing scope that cannot be resolved. A throw anywhere propagates to the + * caller's catch, which denies. + * + * ⛔ Not a second intersection rule: every leg is the primitive the + * middleware calls, in the middleware's spelling, and the delegator's context + * comes from the one resolver the middleware and `canReadObject` share. + */ + private async computeDelegatedReadFilter( + object: string, + context: any, + ): Promise | undefined> { + const permissionSets = await this.resolvePermissionSetsForContext(context); + if (permissionSets.length === 0) return RLS_DENY_FILTER; + const del = await resolveDelegatorContext(this.ql, context); + if (del.kind !== 'resolved') { + this.logger.error?.( + `[security] getReadFilter received an on-behalf-of context for object '${object}' whose delegator ` + + `'${String(context?.onBehalfOf?.userId)}' does not exist — denying (fail-closed: a delegated read is ` + + `never scoped wider than its delegator's own) — with no delegator to intersect against it is not ` + + `scoped at all (ADR-0090 D10)`, + ); + return RLS_DENY_FILTER; + } + const delegatorContext = del.context; + const delegatorSets = await this.resolvePermissionSetsForContext(delegatorContext); + if (delegatorSets.length === 0) return RLS_DENY_FILTER; + + // Step 2.6's two depth stamps, from the same two evaluator calls. + const meta = await this.getObjectSecurityMeta(object); + const delegatorScope = this.permissionEvaluator.getEffectiveScope('read', object, delegatorSets, { isPrivate: meta.isPrivate }); + const declared = this.permissionEvaluator.getDeclaredScope('read', object, permissionSets, { isPrivate: meta.isPrivate }); + const agentLegScope = intersectDelegatedScope(declared, delegatorScope).agentLegScope; + + // Layers 5 and 6 — the sibling middleware's two calls. Resolved first, as + // the non-delegated path resolves its one sharing layer first: a sharing + // outage denies before any RLS layer is composed. + const sharingFilter = await this.resolveSharingReadFilter(object, context, permissionSets, agentLegScope); + const delegatorSharingFilter = await this.resolveSharingReadFilter( + object, + { ...context, userId: delegatorContext.userId, onBehalfOf: undefined }, + delegatorSets, + delegatorScope, + ); + + // 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 delFilter = await this.computeRlsFilter(delegatorSets, object, 'find', delegatorContext); + const delCbpFilter = await this.computeControlledByParentFilter(delegatorSets, object, delegatorContext); + + const securityLayers = andComposeLayers(andComposeLayers(filter, cbpFilter), andComposeLayers(delFilter, delCbpFilter)); + const sharingLayers = andComposeLayers(sharingFilter, delegatorSharingFilter); + return andComposeLayers(securityLayers, sharingLayers) ?? undefined; + } + /** * [#3547] Query surface: the field names the caller MAY READ on `object` * under `context`. This is the authoritative column projection for a @@ -8643,9 +8743,10 @@ export class SecurityPlugin implements Plugin { context: any, /** * [ADR-0055 amendment] The `controlled_by_parent` objects already being resolved on - * this branch of the walk, outermost first. Empty at every real call site - * — the four are the CRUD middleware (caller and D10 delegator) and - * `getReadFilter` — and grown by one on each recursive hop. + * 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> { diff --git a/packages/plugins/plugin-security/src/start-logger-binding.test.ts b/packages/plugins/plugin-security/src/start-logger-binding.test.ts index f24cdf9a4b9..117e5be3491 100644 --- a/packages/plugins/plugin-security/src/start-logger-binding.test.ts +++ b/packages/plugins/plugin-security/src/start-logger-binding.test.ts @@ -77,11 +77,13 @@ const sinkOf = (plugin: SecurityPlugin) => (plugin as any).logger; * Drive a REAL report through `this.logger` and return whether it landed. * * `getReadFilter` is a public instance method whose on-behalf-of refusal - * (`this.logger.error?.(…)`, the ADR-0095/#2852 fail-closed branch) depends on - * NONE of the handles captured below the bail-outs — no `this.ql`, no - * `this.metadata`. That makes it the one report site that is genuinely - * reachable on a bailed-out instance, which is what makes it the right probe - * for "the sink RECEIVES something". + * (`this.logger.error?.(…)`, the ADR-0090 D10 fail-closed branch for a + * delegator who does not exist — on a bailed-out instance the delegator's + * `sys_user` row cannot be read, so the link dangles) depends on NONE of the + * handles captured below the bail-outs — no `this.ql`, no `this.metadata`. + * That makes it the one report site that is genuinely reachable on a + * bailed-out instance, which is what makes it the right probe for "the sink + * RECEIVES something". * * Pre-fix this call is silent: `{}.error` is `undefined` and `?.()` no-ops. */