diff --git a/.changeset/25034-delegated-caller-reads-through-leaf.md b/.changeset/25034-delegated-caller-reads-through-leaf.md new file mode 100644 index 00000000000..804059c0377 --- /dev/null +++ b/.changeset/25034-delegated-caller-reads-through-leaf.md @@ -0,0 +1,13 @@ +--- +'@objectstack/plugin-audit': patch +'@objectstack/service-storage': patch +--- + +fix(plugin-audit,service-storage): a delegated (on-behalf-of) caller reads the parent-record gates through the ReadableIds leaf, no longer through the bounded 2000-row probe (ADR-0139 D5.1, as amended) + +Clause-②: no +Metadata-protocol: none + +The activity, ledger and comment-thread read gates (`@objectstack/plugin-audit`) and the attachment read gate (`@objectstack/service-storage`) routed every read by a delegated caller to the whole-read probe: a SYSTEM pre-scan of at most 2000 rows, then one caller-scoped read per parent object. Past 2000 candidate rows, a delegated caller was served a truncated list (fail-closed, never a leak) with a matching `total`. + +Since the engine composes a `ReadableIds` leaf for a delegated caller from the delegator intersection (ADR-0090 D10, #25009), the gates now write such a caller the same leaves as any caller. It is served every row its delegator intersection admits, with a true `total`. This is the same set the probe's own readability answer admits, without the window. ADR-0139 D5's two remaining classes are unchanged: a parent narrowed by an object-scoped read middleware, and a parent bound to another datasource. Both still take the bounded probe, read with the caller's delegated context. The gates no longer read the delegation link themselves, and the `delegated-caller` value is gone from the gates' internal whole-read probe reasons. Neither is exported. diff --git a/docs/qa/platform-checklist/areas/attachments-storage.json b/docs/qa/platform-checklist/areas/attachments-storage.json index c0956514d4b..a1185d53a72 100644 --- a/docs/qa/platform-checklist/areas/attachments-storage.json +++ b/docs/qa/platform-checklist/areas/attachments-storage.json @@ -343,7 +343,7 @@ "title": "sys_attachment reads inherit parent visibility: a restricted member sees neither rows nor counts for invisible parents", "since": "v15.1", "status": "active", - "revision": 6, + "revision": 7, "priority": "P1", "surface": "api", "personas": ["member who can read the parent record", "member who cannot read the parent record"], @@ -359,7 +359,7 @@ "knownGaps": [ "MARKER (QA run #9401 finding 5, carried here so runs stop re-deriving it): the CROSS-TENANT clauses of this area are structurally unprovable in the OSS workspace. `@objectstack/organizations` is open core since ADR-0132 (Apache-2.0, published on npm) — ⛔ it being closed-source is NOT the reason and has not been since #16215. The reason is ADR-0132's entitlement boundary: no framework package may declare `@objectstack/organizations` (pinned by `packages/plugins/organizations/src/no-framework-dependents.pin.test.ts` — apps declare it, packages do not), so `packages/qa/dogfood` cannot depend on it and the dogfood matrix's cross-tenant block is gated behind `describe.skipIf(!organizationsAvailable)` and skips BY DESIGN here. That is blocked(dependency), never a pass and never a defect. A run that is supposed to ship the package sets OS_TEST_MULTI_ORG_ENABLED=1, which turns the skip into a hard failure instead of a silent one.", "CLOSED by the qa-scratch-authz recipe (#7670): stock showcase's files-enabled parent (showcase_project) is public_read_write — every member sees every project, so the invisible-parent case is not demonstrable on stock seeds, and the pinned dogfood matrix proves it only on its own private fixture (att_secret). The recipe provisions the private files-enabled parent at runtime; landing one in the showcase seeds proper would retire this gap entirely, and remains the better long-term fix.", - "a broad read over an ordinary parent no longer meets the 2000-candidate cap (ADR-0139 E7, #23767): the gate writes one ReadableIds leaf per parent object and pre-scans nothing, so the scratch parents' broad read is served every visible row with a true total and no '[storage] attachment read visibility' warning. The bounded 2000-row fail-closed probe (READ_SCAN_LIMIT, clause 4's warning) remains for ADR-0139 D5's three classes only: a delegated (on-behalf-of) caller, a parent narrowed by an object-scoped read middleware (sys_comment, sys_comment_reaction, sys_activity and sys_audit_log among them), and a parent bound to another datasource than sys_attachment. None of the three is provisioned by the qa-scratch-authz recipe, so clause 4's probe half is not reachable from a run here: it is pinned in packages/services/service-storage/src/attachment-read-leaf.integration.test.ts and attachment-read-leaf.test.ts, and a run cites them. On driver-mongodb, which compiles no ReadableIds leaf until ADR-0139's O3 is measured, a member's sys_attachment read fails INTERNAL_ERROR / 500 naming the driver instead of serving a short list: a ruled limit (ADR-0139 D3, Consequences), never a pass and never a leak.", + "a broad read over an ordinary parent no longer meets the 2000-candidate cap (ADR-0139 E7, #23767): the gate writes one ReadableIds leaf per parent object and pre-scans nothing, so the scratch parents' broad read is served every visible row with a true total and no '[storage] attachment read visibility' warning. The bounded 2000-row fail-closed probe (READ_SCAN_LIMIT, clause 4's warning) remains for ADR-0139 D5's two remaining classes only: a parent narrowed by an object-scoped read middleware (sys_comment, sys_comment_reaction, sys_activity and sys_audit_log among them), and a parent bound to another datasource than sys_attachment. A delegated (on-behalf-of) caller is no class since D5.1's amendment (#25034): it is written the same leaves as any caller and the engine composes them from the delegator intersection. Neither class is provisioned by the qa-scratch-authz recipe, so clause 4's probe half is not reachable from a run here: it is pinned in packages/services/service-storage/src/attachment-read-leaf.integration.test.ts and attachment-read-leaf.test.ts, and a run cites them. On driver-mongodb, which compiles no ReadableIds leaf until ADR-0139's O3 is measured, a member's sys_attachment read fails INTERNAL_ERROR / 500 naming the driver instead of serving a short list: a ruled limit (ADR-0139 D3, Consequences), never a pass and never a leak.", "two more states send the WHOLE read to the bounded 2000-row probe, up front, and the gate warns once per engine for each: a security service that answers no canReadObject (or no security service at all), because no ReadableIds leaf is written without that admission; and an engine that cannot classify a parent (no registry view, no hasObjectMiddleware or no resolveEffectiveDatasource). A stock boot composes both, so neither is reachable from this lane; a run that sees the once-per-engine warning has met one of them and scores the broad-read clause against the 2000-row bound, not as a pass. On the API surface the driver-mongodb refusal names its driver in the server LOG only: the response body is the ADR-0112 envelope (INTERNAL_ERROR / 500) and does not carry the driver name, so a run reads the driver from the log, never from the body." ] }, @@ -391,7 +391,7 @@ "evidence": "the by-id response" }, { - "clause": "the filter fails CLOSED and never truncates an ordinary parent: since ADR-0139 D4 (step E7) the gate writes one ReadableIds leaf per parent object and pre-scans nothing, so a broad read past 2000 candidate rows serves every visible row with a true `total` and logs no cap warning. The bounded 2000-row probe remains only for ADR-0139 D5's three classes (a delegated caller, a parent an object-scoped read middleware narrows, a federated parent) and the two whole-read states knownGaps[3] names; there a pre-scan past the cap or a probe failure excludes rows rather than leaking them, and logs the '[storage] attachment read visibility' warning naming the fail-closed truncation or the denial", + "clause": "the filter fails CLOSED and never truncates an ordinary parent: since ADR-0139 D4 (step E7) the gate writes one ReadableIds leaf per parent object and pre-scans nothing, so a broad read past 2000 candidate rows serves every visible row with a true `total` and logs no cap warning. The bounded 2000-row probe remains only for ADR-0139 D5's two remaining classes (a parent an object-scoped read middleware narrows, a federated parent; a delegated caller reads through the leaf since D5.1's amendment) and the two whole-read states knownGaps[3] names; there a pre-scan past the cap or a probe failure excludes rows rather than leaking them, and logs the '[storage] attachment read visibility' warning naming the fail-closed truncation or the denial", "oracle": "log", "verify": "on a very broad unscoped list over the recipe's ordinary parents, every visible row is served, `total` (the read passes $top) equals the rows served, the invisible-parent row stays excluded, and no '-row cap' warning is logged. Leaked rows are the failure; a cap warning on an ordinary parent contradicts ADR-0139 D4 and is recorded as a finding. The probe half is not reachable from a run here (knownGaps[2]): cite attachment-read-leaf.integration.test.ts and attachment-read-leaf.test.ts", "evidence": "the broad read's row count, its total and the log around it" @@ -402,10 +402,10 @@ "rows filtered but `total` counting the raw table is equally a FAIL (the middleware-not-hook design note in attachment-access-hooks.ts exists because a find-hook leaves count() unfiltered)" ], "traps": ["wrong-persona"], - "automated": { "kind": "api", "ref": "packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts (clauses 0 and 2: the restricted member's scoped list is empty for an invisible parent while the entitled one is not, and a by-id read of the hidden row is 403/404) + packages/qa/dogfood/test/attachments-parent-rls-count-parity.dogfood.test.ts (clause 1, the COUNT: the restricted member's list `total` excludes invisible-parent rows while the entitled caller's includes them on the same request, engine.count() under the member's OWN resolved authz context (resolveAuthzContext, not a hand-built principal) is filtered identically, a count NARROWED to the invisible parent answers 0 for the member and the true number for admin, and paging the member all the way through yields exactly the reported total — so rows-vs-count parity is asserted as one fact rather than two numbers that happen to agree. ⚠ THE TRAP, and why every assertion passes $top: protocol.findData only calls engine.count() when the request carries a PAGE LIMIT; without one it sets total = records.length, which is trivially self-consistent and stays green with count() fully unfiltered because the count path never runs. Each assertion therefore also requires total > records.length, proving the count call happened rather than assuming it) + packages/qa/dogfood/test/attachments-parent-rls-scan-cap.dogfood.test.ts (clause 3, the broad read past the old 2000-row pre-scan window, @proof attachments-parent-rls-scan-cap: since ADR-0139 E7 the gate writes one ReadableIds leaf per parent object and pre-scans nothing for an ordinary parent, so with >2000 candidate join rows the restricted member is served EVERY visible row on one page, `total` equals the rows served (the read passes $top, so total comes from engine.count()), no '-row cap' warning is logged, the invisible-parent row and every dangling-parent row stay excluded, and the owner is served the hidden row on the same read. The seed still puts visible rows at BOTH ENDS of the insertion order with the filler bulk between, and the control asserts that a 2000-row window would leave a visible row outside it, so the pin cannot pass on a fixture the old bound would have served whole) + packages/services/service-storage/src/attachment-read-leaf.integration.test.ts and packages/services/service-storage/src/attachment-read-leaf.test.ts (clause 3, the half that still bounds: ADR-0139 D5's three classes, a delegated caller, a parent narrowed by an object-scoped read middleware and a federated parent, keep the bounded 2000-row fail-closed probe, chosen before any leaf is written; past the cap the probe warns naming its class, and a probe failure denies all and says so. Also there: on a driver that compiles no leaf, as driver-mongodb until ADR-0139's O3, find and count fail { code: 'INTERNAL_ERROR', status: 500 } naming the driver, never the deny-all sentinel). ALSO PINNED, as the reachable half of clause 3's fail-compute arm: a row whose parent RECORD no longer resolves (a dangling parent_id, what a hard-deleted parent leaves behind) is excluded from both rows and count. NOT REACHED from this lane, and not to be read as covered by a run here: (a) an attachment naming a parent OBJECT the registry does not hold cannot be seeded — the #2727 enable.files gate refuses to create a sys_attachment row against a non-files object even under system context (measured: \"File attachments are not enabled for object 'att_ghost_object'\"); such a row names no enumerated parent, so no branch admits it, which attachment-read-leaf.integration.test.ts pins below the API; (b) the middleware no longer wraps the read in an outer catch (ADR-0139 D3): the probe's own failure ('the bounded probe (...) failed, denying all') is pinned in attachment-read-leaf.test.ts and is not provokable through the API, since a where malformed enough to break the system pre-scan also fails the caller's own read and surfaces as a 4xx instead." }, + "automated": { "kind": "api", "ref": "packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts (clauses 0 and 2: the restricted member's scoped list is empty for an invisible parent while the entitled one is not, and a by-id read of the hidden row is 403/404) + packages/qa/dogfood/test/attachments-parent-rls-count-parity.dogfood.test.ts (clause 1, the COUNT: the restricted member's list `total` excludes invisible-parent rows while the entitled caller's includes them on the same request, engine.count() under the member's OWN resolved authz context (resolveAuthzContext, not a hand-built principal) is filtered identically, a count NARROWED to the invisible parent answers 0 for the member and the true number for admin, and paging the member all the way through yields exactly the reported total — so rows-vs-count parity is asserted as one fact rather than two numbers that happen to agree. ⚠ THE TRAP, and why every assertion passes $top: protocol.findData only calls engine.count() when the request carries a PAGE LIMIT; without one it sets total = records.length, which is trivially self-consistent and stays green with count() fully unfiltered because the count path never runs. Each assertion therefore also requires total > records.length, proving the count call happened rather than assuming it) + packages/qa/dogfood/test/attachments-parent-rls-scan-cap.dogfood.test.ts (clause 3, the broad read past the old 2000-row pre-scan window, @proof attachments-parent-rls-scan-cap: since ADR-0139 E7 the gate writes one ReadableIds leaf per parent object and pre-scans nothing for an ordinary parent, so with >2000 candidate join rows the restricted member is served EVERY visible row on one page, `total` equals the rows served (the read passes $top, so total comes from engine.count()), no '-row cap' warning is logged, the invisible-parent row and every dangling-parent row stay excluded, and the owner is served the hidden row on the same read. The seed still puts visible rows at BOTH ENDS of the insertion order with the filler bulk between, and the control asserts that a 2000-row window would leave a visible row outside it, so the pin cannot pass on a fixture the old bound would have served whole) + packages/services/service-storage/src/attachment-read-leaf.integration.test.ts and packages/services/service-storage/src/attachment-read-leaf.test.ts (clause 3, the half that still bounds: ADR-0139 D5's two remaining classes, a parent narrowed by an object-scoped read middleware and a federated parent, keep the bounded 2000-row fail-closed probe, chosen before any leaf is written, while a delegated caller is served through the leaf from its delegator intersection, in parity with the probe's own readability answer; past the cap the probe warns naming its class, and a probe failure denies all and says so. Also there: on a driver that compiles no leaf, as driver-mongodb until ADR-0139's O3, find and count fail { code: 'INTERNAL_ERROR', status: 500 } naming the driver, never the deny-all sentinel). ALSO PINNED, as the reachable half of clause 3's fail-compute arm: a row whose parent RECORD no longer resolves (a dangling parent_id, what a hard-deleted parent leaves behind) is excluded from both rows and count. NOT REACHED from this lane, and not to be read as covered by a run here: (a) an attachment naming a parent OBJECT the registry does not hold cannot be seeded — the #2727 enable.files gate refuses to create a sys_attachment row against a non-files object even under system context (measured: \"File attachments are not enabled for object 'att_ghost_object'\"); such a row names no enumerated parent, so no branch admits it, which attachment-read-leaf.integration.test.ts pins below the API; (b) the middleware no longer wraps the read in an outer catch (ADR-0139 D3): the probe's own failure ('the bounded probe (...) failed, denying all') is pinned in attachment-read-leaf.test.ts and is not provokable through the API, since a where malformed enough to break the system pre-scan also fails the caller's own read and surfaces as a 4xx instead." }, "source": [ "docs/plans/release-15.1-test-plan.md §C3 (#2970)", - "packages/services/service-storage/src/attachment-access-hooks.ts#installAttachmentReadVisibility (installAttachmentReadVisibility: middleware over find/findOne/count/aggregate, one ReadableIds leaf per parent object (ADR-0139 D4), deny-all sentinel, the READ_SCAN_LIMIT fail-closed probe for ADR-0139 D5's three classes)", + "packages/services/service-storage/src/attachment-access-hooks.ts#installAttachmentReadVisibility (installAttachmentReadVisibility: middleware over find/findOne/count/aggregate, one ReadableIds leaf per parent object (ADR-0139 D4), deny-all sentinel, the READ_SCAN_LIMIT fail-closed probe for ADR-0139 D5's two remaining classes)", "packages/services/service-storage/src/attachment-read-visibility.test.ts" ], "history": [ @@ -434,6 +434,12 @@ "date": "2026-10-20", "change": "acceptance[3] (clause, verify, evidence) says what the gate does now — checklist-accuracy finding of QA run #24779 (wave anchor #24579 round 12): the clause still named a 2000-row cap warning that the scan-cap pin says an ordinary parent no longer emits. Governing text ADR-0139 D4 and D5 (step E7, the attachment gate): one ReadableIds leaf per parent object and no pre-scan, the bounded fail-closed probe kept for D5's three classes (attachment-access-hooks.ts, READ_SCAN_LIMIT), as knownGaps[2]–[3] already record. Read at base 307ce4c4b4", "ref": "#24824" + }, + { + "revision": 7, + "date": "2026-10-22", + "change": "ADR-0139 D5.1 as amended (#25034): a delegated (on-behalf-of) caller is no longer a probe class. The attachment gate writes it the same ReadableIds leaves as any caller and the engine composes them from the delegator intersection (ADR-0090 D10), so knownGaps[2], acceptance[3] and both refs now name D5's two remaining classes (an object-scoped-middleware parent, a federated parent). The delegated read and its parity with the probe's own readability answer are pinned in attachment-read-leaf.integration.test.ts and attachment-read-leaf.test.ts. No step or run procedure changes.", + "ref": "#25034" } ] }, diff --git a/packages/plugins/plugin-audit/src/activity-read-visibility.test.ts b/packages/plugins/plugin-audit/src/activity-read-visibility.test.ts index 39dbc65c9d8..d55267d6787 100644 --- a/packages/plugins/plugin-audit/src/activity-read-visibility.test.ts +++ b/packages/plugins/plugin-audit/src/activity-read-visibility.test.ts @@ -7,7 +7,8 @@ * - the leaf path (ADR-0139 D4) against a stand-in engine that answers the * three questions the gate classifies by — one `ReadableIds` leaf per * enumerated parent object, nothing scanned, nothing asked; - * - D5's three classes routed to the bounded probe, by name, up front; + * - D5's two remaining classes routed to the bounded probe, by name, up + * front, and a delegated caller planned like any caller (D5.1, as amended); * - the bounded probe itself — a failing pre-scan, a failing parent read, * the scan bound, the object-level admission — against a stand-in engine * that answers none of the three, so the whole read is the probe. @@ -558,7 +559,7 @@ describe('[ADR-0139 D4] a read that names its parent objects enumerates only tho expect(leafOnly.calls.probes).toEqual([]); }); - it('a delegated caller’s whole-read probe is unchanged: it scans the caller’s own where', async () => { + it('a delegated caller’s read naming its parents is one leaf per named parent, and nothing is scanned', async () => { const { mw, calls } = installClassifying({ objects: OBJECTS, scan: () => [{ object_name: 'crm_o05', record_id: 'c1' }], @@ -566,12 +567,13 @@ describe('[ADR-0139 D4] a read that names its parent objects enumerates only tho }); const where = { object_name: { $in: NAMED } }; const ctxWhere = (await read(mw, { ast: { object: 'sys_activity', where }, context: { userId: 'u_agent', onBehalfOf: { userId: 'u_d' } } as any })).where as any; - expect(ctxWhere.$and[1]).toEqual({ object_name: 'crm_o05', record_id: { $in: ['c1'] } }); - expect(calls.scans.map((s) => s.where)).toEqual([where]); + expect(gateBranches(ctxWhere).map(leafObjectOf)).toEqual(NAMED); + expect(calls.scans).toEqual([]); + expect(calls.probes).toEqual([]); }); }); -describe('[ADR-0139 D5] three classes keep the bounded probe, by name, chosen before any leaf is written', () => { +describe('[ADR-0139 D5] two classes keep the bounded probe, by name, chosen before any leaf is written; a delegated caller is not one', () => { it('a parent an object-scoped read middleware narrows is probed — its rows only — beside the other parents’ leaves', async () => { const { mw, calls } = installClassifying({ objects: ['crm_case', 'sys_attachment'], @@ -619,17 +621,28 @@ describe('[ADR-0139 D5] three classes keep the bounded probe, by name, chosen be expect((await read(mw)).where).toEqual({ object_name: 'crm_case', record_id: { $in: ['c1'] } }); }); - it('a delegated caller: no leaf at all, the whole read is the probe, read with the caller’s envelope', async () => { + it('D5.1, as amended: a delegated caller gets the leaves any caller gets; only a D5.2 parent is probed, read with the delegated envelope', async () => { const onBehalfOf = { userId: 'u_delegator' }; - const { mw, calls } = installClassifying({ - objects: ['crm_case', 'crm_deal'], - scan: () => [{ object_name: 'crm_case', record_id: 'c1' }], - readable: { crm_case: ['c1'] }, - }); - const { where } = await read(mw, { context: { userId: 'u_agent', onBehalfOf } as any }); - expect(where).toEqual({ object_name: 'crm_case', record_id: { $in: ['c1'] } }); - expect(calls.scans[0].where).toEqual({}); + const opts = { + objects: ['crm_case', 'crm_deal', 'sys_attachment'], + middleware: ['sys_attachment'], + scan: () => [{ object_name: 'sys_attachment', record_id: 'f1' }], + readable: { sys_attachment: ['f1'] }, + }; + const { mw, calls } = installClassifying(opts); + const { where } = await read(mw, { context: { userId: 'u_agent', onBehalfOf, __readScope: 'org' } as any }); + const branches = branchesOf(where); + expect(branches.map(leafObjectOf)).toEqual(['crm_case', 'crm_deal', null]); + expect(branches[2]).toEqual({ object_name: 'sys_attachment', record_id: { $in: ['f1'] } }); + // The same plan as a caller acting for no one. + const plain = installClassifying(opts); + expect(branchesOf((await read(plain.mw, { context: { userId: 'u_agent' } })).where).map(leafObjectOf)).toEqual(['crm_case', 'crm_deal', null]); + // No whole-read window: the one pre-scan is the probed parent's, and its + // read carries the delegation on, minus the operation-private keys. + expect(calls.scans.map((s) => s.where)).toEqual([{ object_name: { $in: ['sys_attachment'] } }]); + expect(calls.probes.map((p) => p.object)).toEqual(['sys_attachment']); expect(calls.probes[0].context).toMatchObject({ userId: 'u_agent', onBehalfOf }); + expect(calls.probes[0].context).not.toHaveProperty('__readScope'); }); it('no object-level admission (no canReadObject, or no security service): no leaf, the whole read is the probe, said once', async () => { diff --git a/packages/plugins/plugin-audit/src/activity-read-visibility.ts b/packages/plugins/plugin-audit/src/activity-read-visibility.ts index fc5624d66b6..a4a6c66586a 100644 --- a/packages/plugins/plugin-audit/src/activity-read-visibility.ts +++ b/packages/plugins/plugin-audit/src/activity-read-visibility.ts @@ -30,9 +30,11 @@ * * which the engine composes from the caller's read scope of each `P` and the * driver evaluates as one subquery per branch: no pre-scan, no candidate - * window. D5's three classes — a delegated caller, a parent an object-scoped - * read middleware narrows, a parent on another datasource — are routed to the - * bounded probe up front, by name (the gate module's header). + * window — for a delegated caller too, whose read scope the security service + * answers as the delegator intersection (ADR-0139 D5.1, as amended). D5's two + * remaining classes — a parent an object-scoped read middleware narrows, a + * parent on another datasource — are routed to the bounded probe up front, by + * name (the gate module's header). * * Everything fails CLOSED, row classes exactly as the comment gate treats its * threads: diff --git a/packages/plugins/plugin-audit/src/audit-log-read-visibility.ts b/packages/plugins/plugin-audit/src/audit-log-read-visibility.ts index 592ed944867..1c40dc36796 100644 --- a/packages/plugins/plugin-audit/src/audit-log-read-visibility.ts +++ b/packages/plugins/plugin-audit/src/audit-log-read-visibility.ts @@ -23,8 +23,8 @@ * ledger through the SAME mechanism, `parent-record-read-gate.ts`: a row naming * a record is served exactly when the caller may read that record — one * `ReadableIds` leaf per parent object, composed by the engine from the - * caller's read scope (ADR-0139 D4), and the bounded probe for D5's three - * classes. ⛔ Nothing here derives row scope a second way. A composition or + * caller's read scope (ADR-0139 D4), and the bounded probe for D5's two + * remaining classes. ⛔ Nothing here derives row scope a second way. A composition or * driver refusal is the read's own failure (D3): it reaches the caller, never * this gate's deny-all sentinel. * diff --git a/packages/plugins/plugin-audit/src/comment-access-hooks.ts b/packages/plugins/plugin-audit/src/comment-access-hooks.ts index 153158903b5..b507db72897 100644 --- a/packages/plugins/plugin-audit/src/comment-access-hooks.ts +++ b/packages/plugins/plugin-audit/src/comment-access-hooks.ts @@ -1128,12 +1128,13 @@ export function installCommentAccessHooks( * this gate ({@link commentThreadGate}). * * That module decides up front, by name, what keeps the bounded probe (over the - * pair): ADR-0139 D5's three classes — a delegated caller (the whole read), a - * parent an object-scoped read middleware narrows (`sys_attachment`, - * `sys_comment_reaction`, `sys_approval_request`, …), a federated parent — and - * the two whole-read states it reports once: an engine that cannot classify, - * and a security service that answers no `canReadObject` (no leaf is written - * without it). ⛔ Never reached by catching a refusal. + * pair): ADR-0139 D5's two remaining classes — a parent an object-scoped read + * middleware narrows (`sys_attachment`, `sys_comment_reaction`, + * `sys_approval_request`, …), a federated parent — and the two whole-read + * states it reports once: an engine that cannot classify, and a security + * service that answers no `canReadObject` (no leaf is written without it). + * ⛔ Never reached by catching a refusal. A delegated caller is not one of + * them: it reads through the leaves (D5.1, as amended). * * ## A row the backfill has not reached keeps the probe — never a leaf * diff --git a/packages/plugins/plugin-audit/src/comment-parent-pair.integration.test.ts b/packages/plugins/plugin-audit/src/comment-parent-pair.integration.test.ts index fac39863728..23969c2ae5c 100644 --- a/packages/plugins/plugin-audit/src/comment-parent-pair.integration.test.ts +++ b/packages/plugins/plugin-audit/src/comment-parent-pair.integration.test.ts @@ -9,22 +9,25 @@ * at its executor boundary (ADR-0139 D2) and evaluated by the driver as one * subquery against the parent's table (D3), so a broad comment read is served * every comment the caller may read, with a true total, past the 2,000-row - * window the thread probe had; D5's classes still read through the bounded - * probe, by name; and a row the backfill has not reached is probed by its - * thread, never leafed. + * window the thread probe had — a delegated caller's included, with its + * delegator intersection (D5.1, as amended); D5's two remaining classes still + * read through the bounded probe, by name; and a row the backfill has not + * reached is probed by its thread, never leafed. * * The security stand-in is E5's (`parent-record-read-leaf.integration.test.ts`, * whose header says why it is the smallest honest one): one rule per parent * object, served as `getReadFilter` and by one GLOBAL engine middleware. * * - `e7_doc`: a member reads the records it owns; the admin reads them all; + * a delegated caller reads the INTERSECTION of its own leg and its + * delegator's (ADR-0090 D10), whose leg withholds a few of the member's; * - `e7_vault`: no member may read it; the admin may; * - `e7_gated`: narrowed by an object-scoped read middleware (D5.2); * - `e7_far`: bound to another datasource (D5.3). */ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; -import { ObjectKernel, findMisplacedReadableIds, refuseReadableIdsOnce } from '@objectstack/core'; +import { ObjectKernel, findMisplacedReadableIds, refuseReadableIdsOnce, resolveReadableParentIds } from '@objectstack/core'; import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; @@ -49,6 +52,8 @@ const MEMBER_A = { userId: 'u_a', tenantId: 'org_1', positions: ['org_member'] } const MEMBER_B = { userId: 'u_b', tenantId: 'org_1', positions: ['org_member'] }; const ADMIN = { userId: 'u_admin', tenantId: 'org_1', positions: ['org_admin'] }; const DELEGATED_A = { ...MEMBER_A, onBehalfOf: { userId: 'u_delegator' } }; +/** The delegator's own leg on `e7_doc` admits every record but these (by name). */ +const WITHHELD_FROM_DELEGATOR = ['a0', 'a1', 'a2']; /** Past the 2,000-row candidate window the leaf replaces. */ const DOCS_OF_A = 2_300; @@ -62,8 +67,10 @@ type Row = Record; function readScopeOf(object: string, context: any): Record | undefined { if (context?.isSystem || isAdmin(context)) return undefined; - if (object === DOC) return { owner: context?.userId ?? '__nobody__' }; - return undefined; + if (object !== DOC) return undefined; + const own = { owner: context?.userId ?? '__nobody__' }; + // ADR-0090 D10: a delegated caller's scope is its own leg AND its delegator's. + return context?.onBehalfOf?.userId ? { $and: [own, { name: { $nin: WITHHELD_FROM_DELEGATOR } }] } : own; } const mayReadObject = (object: string, context: any) => context?.isSystem || object !== VAULT || isAdmin(context); @@ -160,6 +167,7 @@ describe('[ADR-0139 E7] the comment-thread read gate over the stamped parent pai if (!target || !parent) return false; if (isAdmin(reader)) return true; if (target.object === VAULT) return false; + if (target.object === DOC && reader?.onBehalfOf?.userId && WITHHELD_FROM_DELEGATOR.includes(parent.name)) return false; if (target.object === DOC || target.object === GATED) return parent.owner === reader.userId; return true; }) @@ -308,14 +316,52 @@ describe('[ADR-0139 E7] the comment-thread read gate over the stamped parent pai ])); }); - it('D5: a delegated caller reads through the bounded probe — no leaf — never wider than the truth', async () => { + it('D5.1, as amended: a delegated caller reads through the leaf — every comment its delegator intersection admits, past the window, a true total', async () => { seenWhere.length = 0; const rows = await served(DELEGATED_A); - expect(JSON.stringify(seenWhere[0])).not.toMatch(/ReadableIds|__readable/i); - expect(rows.length).toBeLessThanOrEqual(WINDOW); - const expected = new Set(await truth(MEMBER_A)); - for (const id of rows) expect(expected.has(id)).toBe(true); + const stampedLeaves = findOr(seenWhere[0]).flatMap((b) => (b?.$or ? b.$or : [b])); + const leaves = stampedLeaves.filter((b) => b?.record_id?.$in && !Array.isArray(b.record_id.$in)).map((b) => b.object_name); + expect(leaves).toEqual(expect.arrayContaining([DOC, VAULT])); + for (const probed of [GATED, FAR]) expect(leaves, probed).not.toContain(probed); + // D5.2 and D5.3 keep the probe, read with the delegated context. + expect(stampedLeaves).toEqual(expect.arrayContaining([ + { object_name: GATED, record_id: { $in: [ids.gatedA] } }, + { object_name: FAR, record_id: { $in: [ids.far] } }, + ])); + const expected = await truth(DELEGATED_A); + expect(expected.length).toBeGreaterThan(WINDOW); + expect(rows).toEqual(expected); expect(await engine.count(COMMENT, {}, { context: DELEGATED_A })).toBe(rows.length); + // The intersection, never the principal's leg alone: exactly the comments on the records the delegator is withheld go. + const withheldThreads = ((await engine.find(DOC, { where: { name: { $in: WITHHELD_FROM_DELEGATOR } }, context: SYS })) as Row[]) + .map((d) => `${DOC}:${d.id}`); + const onWithheld = ((await engine.find(COMMENT, { where: { thread_id: { $in: withheldThreads } }, context: SYS })) as Row[]) + .map((r) => String(r.id)) + .sort(); + expect(onWithheld.length).toBeGreaterThanOrEqual(WITHHELD_FROM_DELEGATOR.length); + const asMember = new Set(await served(MEMBER_A)); + expect([...asMember].filter((id) => !rows.includes(id)).sort()).toEqual(onWithheld); + expect(rows.every((id) => asMember.has(id))).toBe(true); + }); + + it('D5.1, as amended: parity with the probe — the leaf serves a delegated caller exactly the comments the probe’s own readability answer admits, unbounded', async () => { + // The probe's answer is `resolveReadableParentIds` (one caller-scoped read + // per parent object, through the engine middleware chain), asked for every + // stamped pair at rest, so no window cuts it. + const atRest: Row[] = await engine.find(COMMENT, { limit: 100_000, context: SYS }); + const pairs = new Map>(); + for (const row of atRest) { + if (![DOC, VAULT, GATED, FAR].includes(row.object_name) || row.record_id == null) continue; + if (!pairs.has(row.object_name)) pairs.set(row.object_name, new Set()); + pairs.get(row.object_name)!.add(String(row.record_id)); + } + const readable = await resolveReadableParentIds(engine as any, DELEGATED_A, pairs); + const admittedByProbe = atRest + .filter((row) => readable.get(row.object_name)?.has(String(row.record_id)) ?? false) + .map((r) => String(r.id)) + .sort(); + expect(admittedByProbe.length).toBeGreaterThan(WINDOW); + expect(await served(DELEGATED_A)).toEqual(admittedByProbe); }); it('a row the backfill has not reached is probed by its thread, never leafed — then the backfill stamps it', async () => { diff --git a/packages/plugins/plugin-audit/src/comment-thread-leaf.test.ts b/packages/plugins/plugin-audit/src/comment-thread-leaf.test.ts index dfae9e62603..db3a9ae7659 100644 --- a/packages/plugins/plugin-audit/src/comment-thread-leaf.test.ts +++ b/packages/plugins/plugin-audit/src/comment-thread-leaf.test.ts @@ -145,12 +145,20 @@ describe('[ADR-0139 E7] the comment-thread gate: a leaf per parent object, the p expect(pairProbe?.options.fields).toEqual(['object_name', 'record_id']); }); - it('D5.1: a delegated caller writes no leaf — the whole read is the probe', async () => { - const s = standIn({ objects: ['crm_case'] }); + it('D5.1, as amended: a delegated caller is written the same leaves and the same restricted probe as any caller', async () => { + const opts = { objects: ['crm_case', 'crm_deal', 'sys_attachment', 'far_obj'], middleware: ['sys_attachment'], federated: ['far_obj'] }; + const plain = standIn(opts); + installCommentReadVisibility(plain.engine, silentLogger(), () => admission); + const s = standIn(opts); installCommentReadVisibility(s.engine, silentLogger(), () => admission); const { where } = await read(s.mw(), { userId: 'u1', onBehalfOf: { userId: 'u0' } }); - expect(leavesOf(where)).toEqual([]); - expect(where).toEqual({ id: '__comment_thread_denied__' }); + expect(leavesOf(where)).toEqual(['crm_case', 'crm_deal']); + expect(where).toEqual((await read(plain.mw())).where); + // No whole-read window: the stamped rows' only pre-scan is the D5.2 / D5.3 parents' pair probe. + expect(s.finds.filter((f) => f.object === 'sys_comment').map((f) => f.options.where)).toEqual([ + { object_name: { $in: ['far_obj', 'sys_attachment'] } }, + { object_name: { $null: true } }, + ]); }); it('no leaf without canReadObject: the whole read is the probe, said once', async () => { diff --git a/packages/plugins/plugin-audit/src/parent-record-read-gate.ts b/packages/plugins/plugin-audit/src/parent-record-read-gate.ts index ba05a61b0b0..236c2076df6 100644 --- a/packages/plugins/plugin-audit/src/parent-record-read-gate.ts +++ b/packages/plugins/plugin-audit/src/parent-record-read-gate.ts @@ -51,23 +51,30 @@ * object is read: no caller-scoped read of it happens, so its middleware * logs no denial for it. * - * ## The bounded probe (ADR-0139 D5): three classes, by name, chosen up front + * ## A delegated caller reads through the leaf (ADR-0139 D5.1, as amended) * - * The engine refuses to compose a leaf for three classes (`INTERNAL_ERROR` / + * A delegated (on-behalf-of) caller (ADR-0090 D10) is no class of its own: it + * gets the same leaves and the same probed parents as any caller. The engine + * composes each leaf with the delegated context exactly as it stands, and the + * security service answers both halves for it as the INTERSECTION of the + * principal's leg and the delegator's — the scope a delegated direct read of + * the parent is narrowed by, which is what the probe reads. ⛔ This module + * reads no delegation link: the context is passed on, never inspected. + * + * ## The bounded probe (ADR-0139 D5): two classes, by name, chosen up front + * + * The engine refuses to compose a leaf for two classes (`INTERNAL_ERROR` / * 500). This module routes each to the bounded probe BEFORE any leaf is * written — ⛔ never by catching that refusal: * - * 1. **A delegated (on-behalf-of) caller** (ADR-0090 D10): the security - * service's read scope does not carry the delegator intersection, so no - * leaf is written at all; the whole read is the probe. - * 2. **A parent an object-scoped read middleware narrows** - * (`hasObjectMiddleware`): its WHERE is built at read time and is no part - * of its read scope, so a leaf would be wider than a direct read. - * 3. **A federated parent**, bound to another datasource than the gated - * object (`resolveEffectiveDatasource`): no driver evaluates a subquery - * across two datasources. On a deployment that registers the `telemetry` - * datasource, that is every parent of a ledger the lifecycle split routes - * there. + * 1. **A parent an object-scoped read middleware narrows** + * (`hasObjectMiddleware`, D5.2): its WHERE is built at read time and is + * no part of its read scope, so a leaf would be wider than a direct read. + * 2. **A federated parent** (D5.3), bound to another datasource than the + * gated object (`resolveEffectiveDatasource`): no driver evaluates a + * subquery across two datasources. On a deployment that registers the + * `telemetry` datasource, that is every parent of a ledger the lifecycle + * split routes there. * * Two more states send the WHOLE read to the probe, up front, and the gate * says so once for each: @@ -100,7 +107,7 @@ * * Beyond the bound the probe fails CLOSED: the un-scanned rows are excluded, * and a warning says so. A probe that throws denies the whole read, and says - * so. Restricted to the parents of classes 2 and 3, its window holds only the + * so. Restricted to the parents of the two classes, its window holds only the * rows naming those parents. * * ## What the read's own refusal does (ADR-0139 D3) @@ -217,12 +224,12 @@ export const PARENT_GATE_SCAN_LIMIT = 2_000; /** * Why a parent object is read through the bounded probe and not as a - * `ReadableIds` leaf — ADR-0139 D5's three classes, by name, plus the engine - * that cannot tell. Every value is decided before any leaf is written. + * `ReadableIds` leaf — ADR-0139 D5's two remaining classes, by name, plus the + * two states in which the gate cannot write a leaf at all. Every value is + * decided before any leaf is written. A delegated caller is none of them + * (D5.1, as amended: the engine composes its leaf; module header). */ export type ParentProbeClass = - /** D5.1 — an on-behalf-of caller: the whole read is the probe. */ - | 'delegated-caller' /** D5.2 — an object-scoped read middleware narrows this parent. */ | 'object-middleware' /** D5.3 — this parent is bound to another datasource than the gated object. */ @@ -236,11 +243,11 @@ export type ParentProbeClass = /** * How one read reaches its parents: the parent objects written as leaves, and - * the ones the probe reads, each with its class — or the one class that sends + * the ones the probe reads, each with its class — or the one state that sends * the whole read to the probe. */ export type ParentReadPlan = - | { probeWholeRead: 'delegated-caller' | 'engine-cannot-classify' | 'no-object-admission' } + | { probeWholeRead: 'engine-cannot-classify' | 'no-object-admission' } | { probeWholeRead: null; /** One `ReadableIds` leaf each, in name order. */ @@ -341,14 +348,6 @@ export function parseParentRecord( return { object, recordId: String(recordId) }; } -/** - * Whether `context` reads on behalf of another user (ADR-0090 D10) — the - * engine's own test for D5's first class, so the two agree by construction. - */ -function isDelegatedCaller(context: CommentReadMiddlewareCtx['context']): boolean { - return Boolean((context as { onBehalfOf?: { userId?: unknown } } | undefined)?.onBehalfOf?.userId); -} - /** The machine names a comparand on `object_name` confines a row to, or `null` for a shape not read. */ function confinedObjectNames(comparand: unknown): string[] | null { const machineName = (v: unknown): v is string => typeof v === 'string' && OBJECT_NAME_RE.test(v); @@ -434,21 +433,21 @@ export function enumerateParentObjects( } /** - * [ADR-0139 D4, D5] How one read of `gate.object` by `context` reaches its - * parents. Reads the context, whether the security service answers - * `canReadObject` (`objectAdmission`, resolved by the caller), the read's own - * `where` (the parents it names), the registry, the middleware registrations - * and the datasource bindings only — it asks the security service nothing and - * reads no row, so it decides the probe up front, never by catching a refusal. + * [ADR-0139 D4, D5] How one read of `gate.object` reaches its parents. Reads + * whether the security service answers `canReadObject` (`objectAdmission`, + * resolved by the caller), the read's own `where` (the parents it names), the + * registry, the middleware registrations and the datasource bindings only — it + * asks the security service nothing and reads no row, so it decides the probe + * up front, never by catching a refusal. It reads nothing of the caller: the + * plan is the same for every caller, a delegated one included (D5.1, as + * amended). */ export function planParentReads( engine: ParentRecordGateEngine, gate: ParentRecordGate, - context: CommentReadMiddlewareCtx['context'], objectAdmission: boolean, where?: unknown, ): ParentReadPlan { - if (isDelegatedCaller(context)) return { probeWholeRead: 'delegated-caller' }; if (!objectAdmission) return { probeWholeRead: 'no-object-admission' }; const parents = enumerateParentObjects(engine, gate, where); const hasObjectMiddleware = engine.hasObjectMiddleware; @@ -679,10 +678,8 @@ export async function computeParentRecordFilter( // 1. Leaf or probe, per parent object the read can be about, before any // branch is written. The read's own `where`, as it reached this gate. - const plan = planParentReads(engine, gate, ctx.context, hasObjectAdmission(getAdmission), ctx.ast?.where); - if (plan.probeWholeRead === 'engine-cannot-classify' || plan.probeWholeRead === 'no-object-admission') { - reportWholeReadProbeOnce(engine, logger, gate, plan.probeWholeRead); - } + const plan = planParentReads(engine, gate, hasObjectAdmission(getAdmission), ctx.ast?.where); + if (plan.probeWholeRead !== null) reportWholeReadProbeOnce(engine, logger, gate, plan.probeWholeRead); const branches: Array> = []; // 2. One ReadableIds leaf per composable parent object (ADR-0139 D4). diff --git a/packages/plugins/plugin-audit/src/parent-record-read-leaf.integration.test.ts b/packages/plugins/plugin-audit/src/parent-record-read-leaf.integration.test.ts index 2b78169020e..88ac3cd9674 100644 --- a/packages/plugins/plugin-audit/src/parent-record-read-leaf.integration.test.ts +++ b/packages/plugins/plugin-audit/src/parent-record-read-leaf.integration.test.ts @@ -4,8 +4,9 @@ * [ADR-0139 E5] The parent-record read gates on the LEAF path, through a real * engine and a real SQL driver: one `ReadableIds` leaf per parent object, no * pre-scan, so a broad read of `sys_activity` or `sys_audit_log` is served - * every row the caller may read, with a true total, on any table size — and - * D5's three classes still read through the bounded probe, by name. + * every row the caller may read, with a true total, on any table size — a + * delegated caller's included (D5.1, as amended) — and D5's two remaining + * classes still read through the bounded probe, by name. * * ## Why a real engine, a real driver and the real plugin * @@ -28,6 +29,11 @@ * no parent one of D5's middleware-gated class: * * - `e5_doc`: a member reads the records it owns; the admin reads them all; + * a delegated caller (`onBehalfOf`) reads the INTERSECTION of its own leg + * and its delegator's (ADR-0090 D10, as plugin-security answers both + * halves for that context), and the delegator's leg withholds a few of + * the member's records — so a leaf composed without the delegation would + * serve rows a delegated read of `e5_doc` does not; * - `e5_vault`: no member may read it (`canReadObject` false; a direct read * is refused and the refusal recorded); the admin may; * - every other object: every caller reads every record. @@ -43,13 +49,13 @@ */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { ObjectKernel, findMisplacedReadableIds, refuseReadableIdsOnce } from '@objectstack/core'; +import { ObjectKernel, findMisplacedReadableIds, refuseReadableIdsOnce, resolveReadableParentIds } from '@objectstack/core'; import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; import { AuditPlugin } from './audit-plugin.js'; import { LEDGER_AUDIT_CAPABILITY } from './audit-log-read-visibility.js'; -import { PARENT_GATE_SCAN_LIMIT } from './parent-record-read-gate.js'; +import { PARENT_GATE_SCAN_LIMIT, parseParentRecord } from './parent-record-read-gate.js'; import { singlePostureTenancy } from './__tests__/single-posture-tenancy.js'; import { installDefaultOrganization } from './default-organization.testkit.js'; @@ -71,6 +77,8 @@ const MEMBER_B = { userId: 'u_b', tenantId: 'org_1', positions: ['org_member'] } const ADMIN = { userId: 'u_admin', tenantId: 'org_1', positions: ['org_admin'] }; const HOLDER = { userId: 'u_holder', tenantId: 'org_1', positions: ['org_member'], systemPermissions: [LEDGER_AUDIT_CAPABILITY] }; const DELEGATED_A = { ...MEMBER_A, onBehalfOf: { userId: 'u_delegator' } }; +/** The delegator's own leg on `e5_doc` admits every record but these (by name). */ +const WITHHELD_FROM_DELEGATOR = ['a0', 'a1', 'a2']; /** Past the 2,000-row candidate window the leaf replaces. */ const DOCS_OF_A = 2_300; @@ -87,8 +95,10 @@ type Row = Record; /** The stand-in's one rule per parent object — the read scope a leaf is composed with. */ function readScopeOf(object: string, context: any): Record | undefined { if (context?.isSystem || isAdmin(context)) return undefined; - if (object === DOC) return { owner: context?.userId ?? '__nobody__' }; - return undefined; + if (object !== DOC) return undefined; + const own = { owner: context?.userId ?? '__nobody__' }; + // ADR-0090 D10: a delegated caller's scope is its own leg AND its delegator's. + return context?.onBehalfOf?.userId ? { $and: [own, { name: { $nin: WITHHELD_FROM_DELEGATOR } }] } : own; } const mayReadObject = (object: string, context: any) => context?.isSystem || object !== VAULT || isAdmin(context); @@ -189,6 +199,7 @@ describe('[ADR-0139 E5] the parent-record read gates on the leaf path', () => { if (!parent) return false; if (isAdmin(reader)) return true; if (row.object_name === VAULT) return false; + if (row.object_name === DOC && reader?.onBehalfOf?.userId && WITHHELD_FROM_DELEGATOR.includes(parent.name)) return false; if (row.object_name === DOC || row.object_name === GATED) return parent.owner === reader.userId; return true; }) @@ -348,17 +359,57 @@ describe('[ADR-0139 E5] the parent-record read gates on the leaf path', () => { } }); - it('D5: a delegated caller reads through the bounded probe — no leaf — and is not refused', async () => { - seenWhere[ACTIVITY].length = 0; - const rows = await served(ACTIVITY, DELEGATED_A); - const where = seenWhere[ACTIVITY][0] as any; - const branches: any[] = where?.$or ?? [where]; - expect(branches.every((b) => Array.isArray(b?.record_id?.$in))).toBe(true); - // The probe's window: bounded, fail-closed, never wider than the truth. - expect(rows.length).toBeLessThanOrEqual(PARENT_GATE_SCAN_LIMIT); - const expected = new Set(await truth(ACTIVITY, MEMBER_A)); - for (const id of rows) expect(expected.has(id)).toBe(true); - expect(await engine.count(ACTIVITY, {}, { context: DELEGATED_A })).toBe(rows.length); + it('D5.1, as amended: a delegated caller reads through the leaf — every row its delegator intersection admits, past the window, a true total', async () => { + for (const object of [ACTIVITY, LEDGER]) { + seenWhere[object].length = 0; + const rows = await served(object, DELEGATED_A); + const where = seenWhere[object][0] as any; + const branches: any[] = where.$or; + const leaves = branches.filter((b) => b?.record_id?.$in && !Array.isArray(b.record_id.$in)).map((b) => b.object_name); + expect(leaves, object).toEqual(expect.arrayContaining([DOC, VAULT])); + expect(leaves, object).not.toContain(GATED); + expect(leaves, object).not.toContain(FAR); + // D5.2 and D5.3 keep the probe, read with the delegated context. + expect(branches.filter((b) => Array.isArray(b?.record_id?.$in)), object).toEqual(expect.arrayContaining([ + { object_name: GATED, record_id: { $in: [ids.gatedA] } }, + { object_name: FAR, record_id: { $in: [ids.far] } }, + ])); + const expected = await truth(object, DELEGATED_A); + expect(expected.length, object).toBeGreaterThan(PARENT_GATE_SCAN_LIMIT); + expect(rows, object).toEqual(expected); + expect(await engine.count(object, {}, { context: DELEGATED_A }), object).toBe(expected.length); + } + // The intersection, never the principal's leg alone: the rows about the records the delegator is withheld are not served. + const asMember = new Set(await served(ACTIVITY, MEMBER_A)); + const asDelegated = new Set(await served(ACTIVITY, DELEGATED_A)); + const withheld = [...asMember].filter((id) => !asDelegated.has(id)); + expect(withheld).toHaveLength(WITHHELD_FROM_DELEGATOR.length); + expect([...asDelegated].every((id) => asMember.has(id))).toBe(true); + }); + + it('D5.1, as amended: parity with the probe — the leaf serves a delegated caller exactly the rows the probe’s own readability answer admits, unbounded', async () => { + // The probe's answer is `resolveReadableParentIds`: one caller-scoped read + // per parent object through the engine middleware chain, with the + // delegated context. Asked here for every parent pair at rest, so no + // window cuts it; the leaf must serve exactly the rows it admits. + const atRest: Row[] = await engine.find(ACTIVITY, { limit: 100_000, context: SYS }); + const pairs = new Map>(); + for (const row of atRest) { + const parent = parseParentRecord(row, ACTIVITY); + if (!parent || ![DOC, VAULT, GATED, FAR].includes(parent.object)) continue; + if (!pairs.has(parent.object)) pairs.set(parent.object, new Set()); + pairs.get(parent.object)!.add(parent.recordId); + } + const readable = await resolveReadableParentIds(engine as any, DELEGATED_A, pairs); + const admittedByProbe = atRest + .filter((row) => { + const parent = parseParentRecord(row, ACTIVITY); + return parent !== null && (readable.get(parent.object)?.has(parent.recordId) ?? false); + }) + .map((r) => String(r.id)) + .sort(); + expect(admittedByProbe.length).toBeGreaterThan(PARENT_GATE_SCAN_LIMIT); + expect(await served(ACTIVITY, DELEGATED_A)).toEqual(admittedByProbe); }); it('ledger: a non-holder is served every row about a readable record and every row about no record, past the window', async () => { @@ -400,8 +451,6 @@ describe('[ADR-0139 E5] the parent-record read gates on the leaf path', () => { memberCount: await time(() => engine.count(ACTIVITY, {}, { context: MEMBER_A })), adminTop20: await time(() => engine.find(ACTIVITY, { orderBy: [{ field: 'timestamp', order: 'desc' }], limit: 20, context: ADMIN })), adminCount: await time(() => engine.count(ACTIVITY, {}, { context: ADMIN })), - }, - probeMs: { delegatedFind: await time(() => served(ACTIVITY, DELEGATED_A)), delegatedCount: await time(() => engine.count(ACTIVITY, {}, { context: DELEGATED_A })), }, diff --git a/packages/services/service-storage/src/attachment-access-hooks.ts b/packages/services/service-storage/src/attachment-access-hooks.ts index f78d8f6896c..c258cb47d12 100644 --- a/packages/services/service-storage/src/attachment-access-hooks.ts +++ b/packages/services/service-storage/src/attachment-access-hooks.ts @@ -853,8 +853,12 @@ type AttachmentReadGateEngine = AttachmentLifecycleEngine & { } & Partial> & Partial>; -/** Why the WHOLE read goes to the bounded probe, decided before any leaf. */ -type WholeReadProbe = 'delegated-caller' | 'engine-cannot-classify' | 'no-object-admission'; +/** + * Why the WHOLE read goes to the bounded probe, decided before any leaf. A + * delegated caller is not one of them (ADR-0139 D5.1, as amended): the engine + * composes its leaf. + */ +type WholeReadProbe = 'engine-cannot-classify' | 'no-object-admission'; /** * How one read reaches its parents: the parent objects written as leaves and @@ -913,20 +917,28 @@ type AttachmentReadPlan = * the registry does not hold, `sys_attachment` itself, or a parent record that * no longer exists (the subquery finds no such id). * - * ## The bounded probe (ADR-0139 D5): three classes, by name, chosen up front + * ## A delegated caller reads through the leaf (ADR-0139 D5.1, as amended) + * + * A delegated (on-behalf-of) caller (ADR-0090 D10) is no class of its own: it + * gets the same leaves and the same probed parents as any caller. The engine + * composes each leaf with the delegated context exactly as it stands, and the + * security service answers both halves for it as the INTERSECTION of the + * principal's leg and the delegator's — the scope a delegated direct read of + * the parent is narrowed by, which is what the probe reads. ⛔ This gate reads + * no delegation link: the context is passed on, never inspected. * - * The engine refuses to compose a leaf for three classes (`INTERNAL_ERROR` / + * ## The bounded probe (ADR-0139 D5): two classes, by name, chosen up front + * + * The engine refuses to compose a leaf for two classes (`INTERNAL_ERROR` / * 500). This gate routes each to the bounded probe BEFORE any leaf is written * — ⛔ never by catching that refusal: * - * 1. **A delegated (on-behalf-of) caller** (ADR-0090 D10), by the engine's own - * test (`onBehalfOf.userId`): the whole read is the probe. - * 2. **A parent an object-scoped read middleware narrows** - * (`hasObjectMiddleware`): `sys_comment`, `sys_comment_reaction`, the - * activity stream and the ledger among them. Their WHERE is built at read - * time and is no part of their read scope, so a leaf would be wider than a - * direct read. - * 3. **A federated parent**, bound to another datasource than + * 1. **A parent an object-scoped read middleware narrows** + * (`hasObjectMiddleware`, D5.2): `sys_comment`, `sys_comment_reaction`, + * the activity stream and the ledger among them. Their WHERE is built at + * read time and is no part of their read scope, so a leaf would be wider + * than a direct read. + * 2. **A federated parent** (D5.3), bound to another datasource than * `sys_attachment` (`resolveEffectiveDatasource`): no driver evaluates a * subquery across two datasources. * @@ -946,7 +958,7 @@ type AttachmentReadPlan = * (RLS/OWD/sharing of the PARENT apply), and emit one * `{ parent_object, parent_id: { $in: } }` branch per parent object. * Past the bound it fails CLOSED and says so; a probe that throws denies the - * whole read, and says so. Restricted to the parents of classes 2 and 3, its + * whole read, and says so. Restricted to the parents of the two classes, its * window holds only the rows naming those parents. * * ## What the read's own refusal does (ADR-0139 D3) @@ -987,14 +999,6 @@ export function installAttachmentReadVisibility( ); } -/** - * Whether `context` reads on behalf of another user (ADR-0090 D10) — the - * engine's own test for D5's first class, so the two agree by construction. - */ -function isDelegatedCaller(context: AttachmentReadMiddlewareCtx['context']): boolean { - return Boolean((context as { onBehalfOf?: { userId?: unknown } } | undefined)?.onBehalfOf?.userId); -} - /** Whether the security service `getAdmission` resolves answers `canReadObject`. */ function hasObjectAdmission(getAdmission: AttachmentReadAdmissionSource | undefined): boolean { try { @@ -1006,18 +1010,14 @@ function hasObjectAdmission(getAdmission: AttachmentReadAdmissionSource | undefi } /** - * [ADR-0139 D4, D5] How one read of `sys_attachment` by `context` reaches its - * parents. Reads the context, whether a `canReadObject` is composed, the - * registry, the middleware registrations and the datasource bindings only — it - * asks the security service nothing and reads no row, so it decides the probe - * up front, never by catching a refusal. + * [ADR-0139 D4, D5] How one read of `sys_attachment` reaches its parents. + * Reads whether a `canReadObject` is composed, the registry, the middleware + * registrations and the datasource bindings only — it asks the security + * service nothing and reads no row, so it decides the probe up front, never by + * catching a refusal. It reads nothing of the caller: the plan is the same for + * every caller, a delegated one included (D5.1, as amended). */ -function planAttachmentParentReads( - engine: AttachmentReadGateEngine, - context: AttachmentReadMiddlewareCtx['context'], - objectAdmission: boolean, -): AttachmentReadPlan { - if (isDelegatedCaller(context)) return { probeWholeRead: 'delegated-caller' }; +function planAttachmentParentReads(engine: AttachmentReadGateEngine, objectAdmission: boolean): AttachmentReadPlan { if (!objectAdmission) return { probeWholeRead: 'no-object-admission' }; const getAllObjects = engine.registry?.getAllObjects; const hasObjectMiddleware = engine.hasObjectMiddleware; @@ -1049,7 +1049,7 @@ function planAttachmentParentReads( /** The whole-read probe states already reported, per engine. */ const wholeReadProbeReported = new WeakMap>(); -const WHOLE_READ_PROBE_WHY: Readonly> = { +const WHOLE_READ_PROBE_WHY: Readonly> = { 'engine-cannot-classify': 'the engine exposes no registry view, hasObjectMiddleware() or resolveEffectiveDatasource(), so it cannot ' + 'say which parent objects a ReadableIds leaf can be composed for', @@ -1061,7 +1061,7 @@ const WHOLE_READ_PROBE_WHY: Readonly { // 1. Leaf or probe, per parent object, before any branch is written. - const plan = planAttachmentParentReads(engine, ctx.context, hasObjectAdmission(getAdmission)); - if (plan.probeWholeRead === 'engine-cannot-classify' || plan.probeWholeRead === 'no-object-admission') { - reportWholeReadProbeOnce(engine, logger, plan.probeWholeRead); - } + const plan = planAttachmentParentReads(engine, hasObjectAdmission(getAdmission)); + if (plan.probeWholeRead !== null) reportWholeReadProbeOnce(engine, logger, plan.probeWholeRead); const branches: Array> = []; // 2. One ReadableIds leaf per composable parent object (ADR-0139 D4). diff --git a/packages/services/service-storage/src/attachment-read-leaf.integration.test.ts b/packages/services/service-storage/src/attachment-read-leaf.integration.test.ts index 236f6d6e339..b42773f7681 100644 --- a/packages/services/service-storage/src/attachment-read-leaf.integration.test.ts +++ b/packages/services/service-storage/src/attachment-read-leaf.integration.test.ts @@ -4,8 +4,10 @@ * [ADR-0139 E7] The `sys_attachment` read gate on the LEAF path, through a * real engine and a real SQL driver: one `ReadableIds` leaf per parent object, * no pre-scan, so a broad read of `sys_attachment` is served every row the - * caller may read, with a true total, past the old 2,000-row window — and - * D5's three classes still read through the bounded probe, by name. + * caller may read, with a true total, past the old 2,000-row window — a + * delegated caller's included, with its delegator intersection (D5.1, as + * amended) — and D5's two remaining classes still read through the bounded + * probe, by name. * * ## Why a real engine and a real driver * @@ -30,13 +32,17 @@ * one of D5's middleware-gated class: * * - `e7_doc`: a member reads the records it owns; the admin reads them all; + * a delegated caller (`onBehalfOf`) reads the INTERSECTION of its own leg + * and its delegator's (ADR-0090 D10, as plugin-security answers both + * halves for that context), and the delegator's leg withholds a few of + * the member's records; * - `e7_vault`: no member may read it (`canReadObject` false; a direct read * is refused and the refusal recorded); the admin may; * - every other object: every caller reads every record. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { ObjectKernel, findMisplacedReadableIds, refuseReadableIdsOnce } from '@objectstack/core'; +import { ObjectKernel, findMisplacedReadableIds, refuseReadableIdsOnce, resolveReadableParentIds } from '@objectstack/core'; import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { SysAttachment } from '@objectstack/platform-objects'; @@ -63,6 +69,8 @@ const MEMBER_A = { userId: 'u_a', tenantId: 'org_1', positions: ['org_member'] } const MEMBER_B = { userId: 'u_b', tenantId: 'org_1', positions: ['org_member'] }; const ADMIN = { userId: 'u_admin', tenantId: 'org_1', positions: ['org_admin'] }; const DELEGATED_A = { ...MEMBER_A, onBehalfOf: { userId: 'u_delegator' } }; +/** The delegator's own leg on `e7_doc` admits every record but these (by name). */ +const WITHHELD_FROM_DELEGATOR = ['a0', 'a1', 'a2']; /** Past the 2,000-row candidate window the leaf replaces. */ const DOCS_OF_A = 2_300; @@ -76,8 +84,10 @@ type Row = Record; function readScopeOf(object: string, context: any): Record | undefined { if (context?.isSystem || isAdmin(context)) return undefined; - if (object === DOC) return { owner: context?.userId ?? '__nobody__' }; - return undefined; + if (object !== DOC) return undefined; + const own = { owner: context?.userId ?? '__nobody__' }; + // ADR-0090 D10: a delegated caller's scope is its own leg AND its delegator's. + return context?.onBehalfOf?.userId ? { $and: [own, { name: { $nin: WITHHELD_FROM_DELEGATOR } }] } : own; } const mayReadObject = (object: string, context: any) => context?.isSystem || object !== VAULT || isAdmin(context); @@ -186,6 +196,7 @@ describe('[ADR-0139 E7] the attachment read gate on the leaf path', () => { if (!parent) return false; if (isAdmin(reader)) return true; if (row.parent_object === VAULT) return false; + if (row.parent_object === DOC && reader?.onBehalfOf?.userId && WITHHELD_FROM_DELEGATOR.includes(parent.name)) return false; if (row.parent_object === DOC || row.parent_object === GATED) return parent.owner === reader.userId; return true; }) @@ -313,17 +324,57 @@ describe('[ADR-0139 E7] the attachment read gate on the leaf path', () => { expect(probed).toHaveLength(2); }); - it('D5: a delegated caller reads through the bounded probe — no leaf — and is not refused', async () => { + it('D5.1, as amended: a delegated caller reads through the leaf — every attachment its delegator intersection admits, past the window, a true total', async () => { seenWhere.length = 0; const rows = await served(DELEGATED_A); - const where = seenWhere[0] as any; - const branches: any[] = where?.$or ?? [where]; - expect(branches.every((b) => Array.isArray(b?.parent_id?.$in))).toBe(true); - // The probe's window: bounded, fail-closed, never wider than the truth. - expect(rows.length).toBeLessThanOrEqual(SCAN_LIMIT); - const expected = new Set(await truth(MEMBER_A)); - for (const id of rows) expect(expected.has(id)).toBe(true); + const branches: any[] = (seenWhere[0] as any).$or; + const leaves = branches.filter((b) => b?.parent_id?.$in && !Array.isArray(b.parent_id.$in)).map((b) => b.parent_object); + expect(leaves).toEqual(expect.arrayContaining([DOC, VAULT])); + expect(leaves).not.toContain(GATED); + expect(leaves).not.toContain(FAR); + // D5.2 and D5.3 keep the probe, read with the delegated context. + const probed = branches.filter((b) => Array.isArray(b?.parent_id?.$in)); + expect(probed).toEqual( + expect.arrayContaining([ + { parent_object: GATED, parent_id: { $in: [String(ids.gatedA)] } }, + { parent_object: FAR, parent_id: { $in: [String(ids.far)] } }, + ]), + ); + expect(probed).toHaveLength(2); + const expected = await truth(DELEGATED_A); + expect(expected.length).toBeGreaterThan(SCAN_LIMIT); + expect(rows).toEqual(expected); expect(await engine.count(ATTACHMENT, {}, { context: DELEGATED_A })).toBe(rows.length); + // The intersection, never the principal's leg alone: exactly the attachments on the records the delegator is withheld go. + const withheldDocs = ((await engine.find(DOC, { where: { name: { $in: WITHHELD_FROM_DELEGATOR } }, context: SYS })) as Row[]) + .map((d) => String(d.id)); + const onWithheld = ((await engine.find(ATTACHMENT, { where: { parent_object: DOC, parent_id: { $in: withheldDocs } }, context: SYS })) as Row[]) + .map((r) => String(r.id)) + .sort(); + expect(onWithheld).toHaveLength(WITHHELD_FROM_DELEGATOR.length); + const asMember = new Set(await served(MEMBER_A)); + expect([...asMember].filter((id) => !rows.includes(id)).sort()).toEqual(onWithheld); + expect(rows.every((id) => asMember.has(id))).toBe(true); + }); + + it('D5.1, as amended: parity with the probe — the leaf serves a delegated caller exactly the attachments the probe’s own readability answer admits, unbounded', async () => { + // The probe's answer is `resolveReadableParentIds` (one caller-scoped read + // per parent object, through the engine middleware chain), asked for every + // parent pair at rest, so no window cuts it. + const atRest: Row[] = await engine.find(ATTACHMENT, { limit: 100_000, context: SYS }); + const pairs = new Map>(); + for (const row of atRest) { + if (![DOC, VAULT, GATED, FAR].includes(row.parent_object)) continue; + if (!pairs.has(row.parent_object)) pairs.set(row.parent_object, new Set()); + pairs.get(row.parent_object)!.add(String(row.parent_id)); + } + const readable = await resolveReadableParentIds(engine as any, DELEGATED_A, pairs); + const admittedByProbe = atRest + .filter((row) => readable.get(row.parent_object)?.has(String(row.parent_id)) ?? false) + .map((r) => String(r.id)) + .sort(); + expect(admittedByProbe.length).toBeGreaterThan(SCAN_LIMIT); + expect(await served(DELEGATED_A)).toEqual(admittedByProbe); }); it('[ADR-0139 O2] the bound case, recorded: rows at rest, served, total, branches and time', async () => { diff --git a/packages/services/service-storage/src/attachment-read-leaf.test.ts b/packages/services/service-storage/src/attachment-read-leaf.test.ts index 482f7051405..8e33ff4f338 100644 --- a/packages/services/service-storage/src/attachment-read-leaf.test.ts +++ b/packages/services/service-storage/src/attachment-read-leaf.test.ts @@ -56,7 +56,12 @@ function boot(fixture: { const rows = fixture.rows ?? []; const objects = fixture.objects ?? ['att_case', 'att_secret', 'sys_attachment', 'sys_comment']; const gated = new Set(fixture.middlewareGated ?? []); - const reads = { preScans: [] as any[], parentReads: [] as Array<{ object: string; ids: string[] }> }; + const reads = { + preScans: [] as any[], + parentReads: [] as Array<{ object: string; ids: string[] }>, + /** The context each parent read was made with, in read order. */ + parentContexts: [] as any[], + }; let mw!: Mw; // Only the members the read gate calls: `find` (its probe) and the three // classifying questions. The write verbs are not part of this gate. @@ -74,6 +79,7 @@ function boot(fixture: { } const ids: string[] = (query?.where?.id?.$in ?? []).map(String); reads.parentReads.push({ object, ids }); + reads.parentContexts.push(query?.context); const visible = ids.filter((id) => (fixture.visible?.[object] ?? []).includes(id)).map((id) => ({ id })); return typeof query?.limit === 'number' ? visible.slice(0, query.limit) : visible; }, @@ -164,7 +170,7 @@ describe('[ADR-0139 D4] the attachment read gate writes one ReadableIds leaf per }); }); -describe('[ADR-0139 D5] three classes keep the bounded probe, chosen before any leaf is written', () => { +describe('[ADR-0139 D5] two classes keep the bounded probe, chosen before any leaf is written; a delegated caller is not one', () => { const rows: Row[] = [ { id: 'a1', parent_object: 'sys_comment', parent_id: 'c1' }, { id: 'a2', parent_object: 'sys_comment', parent_id: 'c2' }, @@ -211,18 +217,21 @@ describe('[ADR-0139 D5] three classes keep the bounded probe, chosen before any expect(reads.preScans[0].where).toEqual({ $and: [own, { parent_object: { $in: ['sys_comment'] } }] }); }); - it('D5.1: a delegated caller reads through the whole-read probe — no leaf at all', async () => { - const { mw, reads } = boot({ rows, visible: { att_case: ['k1'], sys_comment: ['c2'] } }); - const where = await read(mw, { context: { userId: 'agent_1', onBehalfOf: { userId: 'u1' } } as any }); - expect(leafBranches(where)).toEqual([]); - expect(where).toEqual({ - $or: [ - { parent_object: 'sys_comment', parent_id: { $in: ['c2'] } }, - { parent_object: 'att_case', parent_id: { $in: ['k1'] } }, - ], - }); - // The whole read's window: the caller's where, unrestricted. - expect(reads.preScans[0].where).toEqual({}); + it('D5.1, as amended: a delegated caller gets the leaves any caller gets; only a D5.2 parent is probed, read with the delegated envelope', async () => { + const onBehalfOf = { userId: 'u1' }; + const { mw, reads } = boot({ rows, middlewareGated: ['sys_comment'], visible: { att_case: ['k1'], sys_comment: ['c2'] } }); + const where = await read(mw, { context: { userId: 'agent_1', onBehalfOf, __readScope: 'org' } as any }); + expect(leafBranches(where).map((b) => b.parent_object)).toEqual(['att_case', 'att_secret']); + expect(literalBranches(where)).toEqual([{ parent_object: 'sys_comment', parent_id: { $in: ['c2'] } }]); + // The same plan as a caller acting for no one. + const plain = boot({ rows, middlewareGated: ['sys_comment'], visible: { att_case: ['k1'], sys_comment: ['c2'] } }); + expect(await read(plain.mw, { context: { userId: 'agent_1' } })).toEqual(where); + // No whole-read window: the one pre-scan is the probed parent's, and its + // read carries the delegation on, minus the operation-private keys. + expect(reads.preScans.map((q) => q.where)).toEqual([{ parent_object: { $in: ['sys_comment'] } }]); + expect(reads.parentReads).toEqual([{ object: 'sys_comment', ids: ['c1', 'c2'] }]); + expect(reads.parentContexts[0]).toMatchObject({ userId: 'agent_1', onBehalfOf }); + expect(reads.parentContexts[0]).not.toHaveProperty('__readScope'); }); it('a restricted probe that throws denies the whole read and says so — the leaves are not served alone', async () => {