Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/25034-delegated-caller-reads-through-leaf.md
Original file line number Diff line number Diff line change
@@ -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.
16 changes: 11 additions & 5 deletions docs/qa/platform-checklist/areas/attachments-storage.json

Large diffs are not rendered by default.

41 changes: 27 additions & 14 deletions packages/plugins/plugin-audit/src/activity-read-visibility.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -558,20 +559,21 @@ 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' }],
readable: { crm_o05: ['c1'] },
});
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'],
Expand Down Expand Up @@ -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 () => {
Expand Down
8 changes: 5 additions & 3 deletions packages/plugins/plugin-audit/src/activity-read-visibility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
13 changes: 7 additions & 6 deletions packages/plugins/plugin-audit/src/comment-access-hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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;
Expand All @@ -62,8 +67,10 @@ type Row = Record<string, any>;

function readScopeOf(object: string, context: any): Record<string, unknown> | 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);

Expand Down Expand Up @@ -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;
})
Expand Down Expand Up @@ -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<string, Set<string>>();
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 () => {
Expand Down
16 changes: 12 additions & 4 deletions packages/plugins/plugin-audit/src/comment-thread-leaf.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Loading
Loading