From b9920faa85b9b0b2f6b09e9fe88683e5557ecd0e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 13:31:34 +0000 Subject: [PATCH 01/10] wip(plugin-audit): the parent-record read gates emit one ReadableIds leaf per parent object; D5's classes keep the probe Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- .../src/activity-read-visibility.ts | 74 +-- .../src/audit-log-read-visibility.ts | 74 ++- .../src/parent-record-read-gate.ts | 498 +++++++++++++----- 3 files changed, 472 insertions(+), 174 deletions(-) diff --git a/packages/plugins/plugin-audit/src/activity-read-visibility.ts b/packages/plugins/plugin-audit/src/activity-read-visibility.ts index 740eeb360c6..990edd2bc6e 100644 --- a/packages/plugins/plugin-audit/src/activity-read-visibility.ts +++ b/packages/plugins/plugin-audit/src/activity-read-visibility.ts @@ -12,38 +12,47 @@ * module a principal holding object-level `sys_activity` read read every row * of its environment, including rows about records it cannot open. * - * This is `installCommentReadVisibility` (`comment-access-hooks.ts`) applied to - * the activity stream, and the two gates share ONE answer to "can this caller - * read that record": {@link resolveReadableParentIds}, a caller-scoped engine - * read per parent object, so the parent's own OWD/sharing, RLS and object-level - * CRUD decide. ⛔ Nothing here derives readability a second way. + * The rule is the comment gate's (`comment-access-hooks.ts`) applied to the + * activity stream, and it is decided where every parent-record gate decides + * it: the engine and the security service answer "can this caller read that + * record", so the parent's own OWD/sharing, RLS and object-level CRUD decide. + * ⛔ Nothing here derives readability a second way. * - * Mechanism, as for comments, and shared with the compliance ledger's gate + * Mechanism, shared with the compliance ledger's gate * (`audit-log-read-visibility.ts`) through `parent-record-read-gate.ts`: a data * middleware (not a find-hook) on `find`, `findOne`, `count` and `aggregate`, - * so a list's `total` and a grouped count - * are narrowed exactly like its rows. For each read it pre-scans the parent - * pairs the query would touch under SYSTEM context, asks which of those parents - * the caller can read (one read per parent object, never one per row), and ANDs - * one branch per parent object into `ctx.ast.where`: + * so a list's `total` and a grouped count are narrowed exactly like its rows. + * For each read it ANDs into `ctx.ast.where` one branch per parent object the + * registry holds (ADR-0139 D4): * - * { $or: [ { object_name: OBJECT, record_id: { $in: READABLE_IDS } }, ... ] } + * { $or: [ { object_name: P, record_id: { $in: ReadableIds.of(P) } }, ... ] } + * + * 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). * * Everything fails CLOSED, row classes exactly as the comment gate treats its * threads: * - a row naming no parent (no `object_name` or no `record_id`, or an * `object_name` that is not a machine name) is excluded; - * - a row whose parent record no longer exists is excluded: the parent read - * finds nothing, so there is nothing to judge readability on; - * - a row whose parent object the caller cannot read, or that the engine does - * not know, is excluded: that parent read refuses or throws; - * - a row naming `sys_activity` itself is excluded: probing it would re-enter - * this gate. + * - a row whose parent record no longer exists is excluded: the subquery + * finds no such id; + * - a row whose parent object the caller cannot read, or that the registry + * does not hold, is excluded: the engine answers that leaf `$in: []`, or no + * leaf is written; + * - a row naming `sys_activity` itself is excluded: no leaf is written for the + * gated object. * A platform object is a parent like any other: its rows are kept exactly when * the caller can read that platform record. The stream declares no row class * outside the gate: its one platform producer is the CRUD mirror, and every * row it writes is about a record. * + * [ADR-0139 D3] A composition or driver refusal of a leaf is this read's own + * failure: it reaches the caller and says why, and this gate does not turn it + * into its deny-all sentinel. + * * System-context reads (the audit writer, engine self-reads) and context-less * programmatic calls on bare kernels are not narrowed, as for comments: every * real transport carries a context. @@ -55,10 +64,15 @@ * the security service's field answer, so it lives with the field redaction * (`activity-field-redaction.ts`, `computeWithheldUpdateFilter`), whose * middleware `AuditPlugin` registers after this one: its pre-scan reads the - * WHERE this gate has already narrowed. + * WHERE this gate has already narrowed. ⚠️ It reads it as SYSTEM, so a + * `ReadableIds` leaf in that WHERE is composed for the system there (every id + * of its parent), not for the reader: its own 2,000-row window still bounds a + * broad read of this stream until that rule becomes a predicate (ADR-0139 E6). + * The rows it lets through are still ANDed with this gate's leaves, composed + * for the reader. */ -import type { CommentAccessEngine, CommentAccessLogger, CommentReadMiddlewareCtx } from './comment-access-hooks.js'; +import type { CommentAccessLogger, CommentReadMiddlewareCtx } from './comment-access-hooks.js'; import { PARENT_GATE_READ_OPS, andIntoWhere, @@ -68,6 +82,7 @@ import { parseParentRecord, type ParentRecord, type ParentRecordGate, + type ParentRecordGateEngine, } from './parent-record-read-gate.js'; const ACTIVITY_OBJECT = 'sys_activity'; @@ -124,7 +139,7 @@ export function parseActivityParent(row: Record | null | undefi * an access attempt"). */ export function installActivityReadVisibility( - engine: CommentAccessEngine, + engine: ParentRecordGateEngine, logger: CommentAccessLogger, getAdmission: ParentObjectReadAdmissionSource, ): void { @@ -137,16 +152,13 @@ export function installActivityReadVisibility( if (!PARENT_GATE_READ_OPS.has(ctx.operation) || !ctx.ast || !ctx.context || ctx.context.isSystem) { return next(); } - try { - const filter = await computeParentRecordFilter(engine, ctx, logger, ACTIVITY_GATE, getAdmission); - if (filter) andIntoWhere(ctx, filter); - } catch (err) { - // A filter-compute failure must never fall open into a leak. - logger.warn( - `[audit] activity read visibility: filter failed, denying all (${(err as Error)?.message ?? err})`, - ); - andIntoWhere(ctx, ACTIVITY_GATE.denyAll); - } + // [ADR-0139 D3] No catch: the gate answers its own probe's failure with + // the deny-all sentinel, and a `ReadableIds` leaf is composed by the + // engine after `next()`, so a composition or driver refusal is this + // read's own failure and reaches the caller saying why — ⛔ never + // turned into the deny-all sentinel here. + const filter = await computeParentRecordFilter(engine, ctx, logger, ACTIVITY_GATE, getAdmission); + if (filter) andIntoWhere(ctx, filter); return next(); }, { object: ACTIVITY_OBJECT }, 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 a3f1e46a084..21c4620c200 100644 --- a/packages/plugins/plugin-audit/src/audit-log-read-visibility.ts +++ b/packages/plugins/plugin-audit/src/audit-log-read-visibility.ts @@ -21,9 +21,12 @@ * * The activity stream's gate (`activity-read-visibility.ts`), applied to the * ledger through the SAME mechanism, `parent-record-read-gate.ts`: a row naming - * a record is served exactly when the caller's own engine read of that record - * finds it (`resolveReadableParentIds`, shared with the comment and activity - * gates). ⛔ Nothing here derives row scope a second way. + * 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 + * driver refusal is the read's own failure (D3): it reaches the caller, never + * this gate's deny-all sentinel. * * ## The ledger's row classes, measured per writer * @@ -31,7 +34,9 @@ * mirror's `create` / `update` / `delete` (`audit-writers.ts`), record-view * `read` rows (`read-audit.ts`), plugin-auth's administrative `create` / * `update` on a `sys_user`, and the auth-event sink's `login` / `logout`, - * which name the session (`auth-event-audit.ts`). Judged by the record gate. + * which name the session (`auth-event-audit.ts`). Judged by the record gate; + * the session is in the audit writer's `AUDIT_EXCLUDED_OBJECTS`, so the gate + * names it ({@link LEDGER_NAMED_PARENTS}) to keep judging it. * A record that no longer exists is read by no caller, so its rows are * excluded for every caller that is not system context and does not hold * the audit capability below: every `delete` row, every other row about a @@ -44,7 +49,8 @@ * There is no record to judge, and none carries a record's field values * (the per-writer measurement in `audit-log-field-redaction.ts`), so they * are OUTSIDE this gate's class and are served under the ledger's own grant - * exactly as before. This is where the ledger differs from the activity + * exactly as before — through a predicate on the ledger's own columns + * ({@link ledgerRowAboutNoRecordWhere}), so a broad read is served whole. This is where the ledger differs from the activity * stream, which excludes every row naming no parent: the stream has no * platform producer of such rows, while these have three, and the shipped * `config_changes` list view reads two of them through the data door. @@ -64,8 +70,7 @@ * * A caller holding {@link LEDGER_AUDIT_CAPABILITY} is not narrowed by this * gate: it is served the deletion and sign-out trail, the rows about records - * it cannot open, and a broad read whole (the gate's pre-scan, and so its - * bound, never runs for it). It still needs the ledger's own object grant, + * it cannot open, and a broad read whole (nothing of the gate runs for it). It still needs the ledger's own object grant, * and the field redaction still narrows every snapshot it is served. Platform * administrators hold it by default (`ADMIN_FULL_ACCESS_CAPABILITIES` in * `@objectstack/spec`); every other position only by explicit grant. The @@ -78,7 +83,8 @@ * context. */ -import type { CommentAccessEngine, CommentAccessLogger } from './comment-access-hooks.js'; +import { SystemObjectName } from '@objectstack/spec/system'; +import type { CommentAccessLogger } from './comment-access-hooks.js'; import { LEDGER_RECORD_WRITE_ACTIONS } from './audit-log-field-redaction.js'; import { READ_AUDIT_ACTION } from './read-audit.js'; import { @@ -87,6 +93,7 @@ import { computeParentRecordFilter, type ParentObjectReadAdmissionSource, type ParentRecordGate, + type ParentRecordGateEngine, } from './parent-record-read-gate.js'; const LEDGER_OBJECT = 'sys_audit_log'; @@ -114,6 +121,26 @@ export function isLedgerRowAboutNoRecord(row: Record): boolean return typeof action === 'string' && action !== '' && !RECORD_ACTIONS.has(action); } +/** + * [ADR-0139 D4] {@link isLedgerRowAboutNoRecord} as a WHERE on the ledger's own + * columns — the outside class the gate ORs in beside its parent branches, so + * the driver evaluates it, unbounded. A fresh tree per call. The two are one + * statement in two forms, pinned row for row against a real driver in + * `audit-log-read-visibility.integration.test.ts`. + */ +export function ledgerRowAboutNoRecordWhere(): Record { + return { + $and: [ + // No record id: absent, null or empty. + { $or: [{ record_id: null }, { record_id: '' }] }, + // An action, and not a record action. `$ne: null` first, so the + // membership test is never asked of a null. + { action: { $ne: null } }, + { action: { $nin: ['', ...LEDGER_RECORD_ACTIONS] } }, + ], + }; +} + /** * The compliance ledger's audit capability: its holder is exempt from this * gate. Declared in `PLATFORM_CAPABILITIES` (`@objectstack/spec/security`) and @@ -122,12 +149,22 @@ export function isLedgerRowAboutNoRecord(row: Record): boolean */ export const LEDGER_AUDIT_CAPABILITY = 'view_all_audit_log'; +/** + * The parent the ledger's auth-event sink names (`auth-event-audit.ts`: a + * `login` / `logout` row is about the session). It is in the audit writer's + * `AUDIT_EXCLUDED_OBJECTS` — the CRUD mirror writes no row about a session — + * so the gate names it, and a row about a session is served exactly when that + * session is readable, as before ADR-0139 E5. + */ +export const LEDGER_NAMED_PARENTS: readonly string[] = [SystemObjectName.SESSION]; + const LEDGER_GATE: ParentRecordGate = { object: LEDGER_OBJECT, seam: 'audit log read visibility', denyAll: { id: '__audit_log_parent_denied__' }, - outsideClass: { fields: ['action'], test: isLedgerRowAboutNoRecord }, + outsideClass: { where: ledgerRowAboutNoRecordWhere }, exemptCapability: LEDGER_AUDIT_CAPABILITY, + namedParents: LEDGER_NAMED_PARENTS, }; /** @@ -140,7 +177,7 @@ const LEDGER_GATE: ParentRecordGate = { * an access attempt"). */ export function installAuditLogReadVisibility( - engine: CommentAccessEngine, + engine: ParentRecordGateEngine, logger: CommentAccessLogger, getAdmission: ParentObjectReadAdmissionSource, ): void { @@ -153,16 +190,13 @@ export function installAuditLogReadVisibility( if (!PARENT_GATE_READ_OPS.has(ctx.operation) || !ctx.ast || !ctx.context || ctx.context.isSystem) { return next(); } - try { - const filter = await computeParentRecordFilter(engine, ctx, logger, LEDGER_GATE, getAdmission); - if (filter) andIntoWhere(ctx, filter); - } catch (err) { - // A filter-compute failure must never fall open into a leak. - logger.warn( - `[audit] audit log read visibility: filter failed, denying all (${(err as Error)?.message ?? err})`, - ); - andIntoWhere(ctx, LEDGER_GATE.denyAll); - } + // [ADR-0139 D3] No catch: the gate answers its own probe's failure with + // the deny-all sentinel, and a `ReadableIds` leaf is composed by the + // engine after `next()`, so a composition or driver refusal is this + // read's own failure and reaches the caller saying why — ⛔ never + // turned into the deny-all sentinel here. + const filter = await computeParentRecordFilter(engine, ctx, logger, LEDGER_GATE, getAdmission); + if (filter) andIntoWhere(ctx, filter); return next(); }, { object: LEDGER_OBJECT }, 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 3cee7dd179d..21f7643ce30 100644 --- a/packages/plugins/plugin-audit/src/parent-record-read-gate.ts +++ b/packages/plugins/plugin-audit/src/parent-record-read-gate.ts @@ -14,46 +14,99 @@ * row, so neither OWD/sharing nor RLS can narrow them. * * ⛔ Nothing here derives readability. "Can this caller read that record" is - * {@link resolveReadableParentIds} (`@objectstack/core`): one - * caller-scoped engine read per parent object, so the parent's own OWD/sharing, - * RLS and object-level CRUD decide. This module only chooses WHICH rows are - * judged that way and turns the answer into a WHERE. + * answered by the engine and the security service, through one of two + * mechanisms this module chooses between per parent object, up front. * - * Mechanism: for one read, pre-scan the parent pairs the query would touch - * under SYSTEM context (bounded, in the caller's own order), ask which of those - * parent OBJECTS the caller may read at all (the security service's - * object-level read admission — see "A skipped object is not an access - * attempt" below), ask which of those parents the caller can read (one read - * per admitted parent object, never one per row), and return one branch per - * parent object for the gate to AND into `ctx.ast.where`: + * ## The leaf (ADR-0139 D4): one branch per parent object, no pre-scan * - * { $or: [ { object_name: OBJECT, record_id: { $in: READABLE_IDS } }, ... ] } + * The parent objects are enumerated from the metadata registry: every + * registered object outside the audit writer's `AUDIT_EXCLUDED_OBJECTS` (the + * CRUD mirror writes no row about one) and outside the gated object itself, + * plus the parents a gate declares its other producers name + * ({@link ParentRecordGate.namedParents}). For each, one branch: * - * Fails CLOSED. A row that names no parent this module can authorize is - * excluded: - * - no `object_name` or no `record_id`, or an `object_name` that is not a - * machine name; - * - a parent record that no longer exists: the parent read finds nothing, so - * there is nothing to judge readability on; - * - a parent object the caller cannot read, or that the engine does not know: - * the object-level read admission refuses it, or that parent read refuses - * or throws; - * - a row naming the gated object itself: probing it would re-enter the gate. + * { object_name: P, record_id: { $in: ReadableIds.of(P) } } + * + * ORed together and ANDed into the read's `where`. The leaf means "the ids of + * P this caller may read"; the engine composes it at its executor boundary + * from the security service's `canReadObject` and `getReadFilter` for P + * (ADR-0139 D2), and the driver evaluates it as one subquery against P's + * table inside its own tenant wall (D3). So a list, its `total` and a grouped + * count are one statement each, over every row the caller may read, on any + * table size: there is no candidate window. + * + * Rows the leaf excludes, exactly as the probe below does: a row naming no + * parent or a non-machine name, an object this gate does not enumerate, the + * gated object itself, or a parent that no longer exists (the subquery finds + * no such id). A parent object the caller may not read is answered `$in: []` + * by the engine's own `canReadObject` admission, before anything of that + * 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 + * + * The engine refuses to compose a leaf for three 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. + * + * An engine that cannot answer those questions (no registry view, no + * `hasObjectMiddleware`, no `resolveEffectiveDatasource` — each optional on + * its contract) cannot tell a composable parent from a refused one, so the + * whole read is the probe there too, and the gate says so once. + * + * The probe, for the parents it is given (or for every row, for a whole-read + * probe): pre-scan the parent pairs the query would touch under SYSTEM + * context (bounded at {@link PARENT_GATE_SCAN_LIMIT}, in the caller's own + * order), ask which of those parent OBJECTS the caller may read at all (the + * security service's object-level read admission — see "A skipped object is + * not an access attempt" below), ask which of those parents the caller can read + * ({@link resolveReadableParentIds}: one caller-scoped read per admitted + * parent object, never one per row), and emit one branch per parent object: + * + * { object_name: OBJECT, record_id: { $in: READABLE_IDS } } + * + * 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 + * rows naming those parents. + * + * ## What the read's own refusal does (ADR-0139 D3) + * + * A leaf is composed AFTER this gate hands the read on (`next()`), at the + * engine's executor boundary, so a composition or driver refusal is the read's + * own failure: it propagates to the caller and says why. ⛔ No consumer of + * this module catches it into its deny-all sentinel; this module makes no + * engine call for a leaf branch, so nothing it does can meet one. + * + * ## The outside class and the exemption * * The one declared exception is a gate's {@link ParentRecordGate.outsideClass}: - * rows that name NO record and that the gate states are not about one. Those - * are kept by their stored id, so the branch can only match rows the pre-scan - * saw. A gate that declares none (the activity stream) excludes them too. + * rows that name NO record and that the gate states are not about one. It is a + * plain predicate on the gated object's own columns, ORed in beside the parent + * branches, so it is evaluated by the driver like any filter, unbounded. A + * gate that declares none (the activity stream) excludes those rows. * * The one declared exemption is a gate's * {@link ParentRecordGate.exemptCapability}: a caller holding that capability - * is not narrowed by the gate at all — no pre-scan, so no pre-scan bound, and - * nothing ANDed in. Only this gate is skipped: the object's own grant, the other - * read seams on it (a field redaction, a query guard) and every other gate - * still apply. Held means the caller's resolved `systemPermissions`, the one - * capability set the request's authorization resolver stamps on the execution - * context and every other capability check reads. A gate that declares none - * (the activity stream) exempts no caller. + * is not narrowed by the gate at all, decided before anything else — nothing + * is enumerated and nothing ANDed in. Only this gate is skipped: the object's + * own grant, the other read seams on it (a field redaction, a query guard) and + * every other gate still apply. Held means the caller's resolved + * `systemPermissions`, the one capability set the request's authorization + * resolver stamps on the execution context and every other capability check + * reads. A gate that declares none (the activity stream) exempts no caller. * * ## A skipped object is not an access attempt * @@ -65,30 +118,42 @@ * activity feed. Those lines are not access attempts — the gate excluding * those rows is the expected answer — and they buried the real denials. * - * So before any parent object is read as the caller, the gate asks the - * security service's `canReadObject` (`ISecurityService`, `@objectstack/spec`) - * — the middleware's own object-level read gate, arm for arm (its capability - * AND-gate, its CRUD grant, its fail-closed posture and principal arms), which - * answers without logging or throwing — and reads only the objects it admits. - * An object it refuses contributes no readable id, exactly as its refused - * engine read did, so the emitted WHERE and the rows served are unchanged; it - * is just never read. ⛔ It is asked with the caller's own envelope (minus the + * A leaf never reads its parent as the caller: the engine asks + * `canReadObject` and answers a refused object `$in: []` without a read. The + * probe asks the same question first, through the security service's + * `canReadObject` (`ISecurityService`, `@objectstack/spec`) — the middleware's + * own object-level read gate, arm for arm (its capability AND-gate, its CRUD + * grant, its fail-closed posture and principal arms), which answers without + * logging or throwing — and reads only the objects it admits. An object it + * refuses contributes no readable id, exactly as its refused engine read did, + * so the emitted WHERE and the rows served are unchanged; it is just never + * read. ⛔ It is asked with the caller's own envelope (minus the * operation-private keys, as {@link resolveReadableParentIds} sends it): no * second derivation of authority, and no muting of the middleware's log — a * caller's own read of that object still logs its denial once. * - * The admission only ever NARROWS what is read: an admitted object is still - * read as the caller, and that read still decides row scope, sharing, RLS and - * every gate the middleware runs after its object-level arms. Fails CLOSED: - * an admission that throws, or answers anything but `true`, refuses that - * object. With no security service composed, or one whose contract omits the - * optional `canReadObject`, there is no admission to ask, and every parent - * object is read as the caller: the engine's middleware still decides, so the - * absence can leave the answer only as it is, never widen it. + * The admission only ever NARROWS what the probe reads: an admitted object is + * still read as the caller, and that read still decides row scope, sharing, + * RLS and every gate the middleware runs after its object-level arms. Fails + * CLOSED: an admission that throws, or answers anything but `true`, refuses + * that object. With no security service composed, or one whose contract omits + * the optional `canReadObject`, there is no admission to ask, and every probed + * parent object is read as the caller: the engine's middleware still decides, + * so the absence can leave the answer only as it is, never widen it. (A leaf + * with no usable security service is answered `$in: []` by the engine: fail + * closed, said once by the engine.) */ import { resolveReadableParentIds, withoutOperationPrivateKeys } from '@objectstack/core'; -import type { ISecurityService, SecurityContext } from '@objectstack/spec/contracts'; +import type { + EngineSchemaRegistryView, + IDataEngine, + IObjectQLEngine, + ISecurityService, + SecurityContext, +} from '@objectstack/spec/contracts'; +import { ReadableIds } from '@objectstack/spec/data'; +import { AUDIT_EXCLUDED_OBJECTS } from './audit-writers.js'; import { type CommentAccessEngine, type CommentAccessLogger, @@ -96,7 +161,7 @@ import { } from './comment-access-hooks.js'; /** - * The single member of the security service's contract the gate consults: the + * The single member of the security service's contract the probe consults: the * object-level read admission. OPTIONAL on the contract, so the gate handles * its absence by construction (see the module header). */ @@ -109,19 +174,63 @@ export type ParentObjectReadAdmission = Pick; */ export type ParentObjectReadAdmissionSource = () => ParentObjectReadAdmission | null | undefined; +/** + * The engine a parent-record gate reads through: the comment gate's slice, + * plus the three questions that decide, per parent object, between a leaf and + * the probe (ADR-0139 D5). Each is optional on its contract; without all + * three the gate cannot tell a composable parent from a refused one. + */ +export type ParentRecordGateEngine = CommentAccessEngine & { + /** The registry the parent objects are enumerated from. */ + registry?: Pick; +} & Partial> & + Partial>; + /** The engine reads a parent-record read gate answers for. */ export const PARENT_GATE_READ_OPS: ReadonlySet = new Set(['find', 'findOne', 'count', 'aggregate']); -/** Bound on the per-read candidate pre-scan — the comment gate's bound. Beyond - * it the filter fails CLOSED (the un-scanned rows are excluded, never leaked). */ +/** Bound on the probe's per-read candidate pre-scan — the comment gate's bound. + * Beyond it the probe fails CLOSED (the un-scanned rows are excluded, never + * leaked). The leaf has no bound. */ 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. + */ +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. */ + | 'federated-parent' + /** The engine exposes no registry view, `hasObjectMiddleware` or + * `resolveEffectiveDatasource`: the whole read is the probe. */ + | 'engine-cannot-classify'; + +/** + * 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 whole read to the probe. + */ +export type ParentReadPlan = + | { probeWholeRead: 'delegated-caller' | 'engine-cannot-classify' } + | { + probeWholeRead: null; + /** One `ReadableIds` leaf each, in name order. */ + leaves: string[]; + /** Read through the probe, each with its D5 class, in name order. */ + probed: Map; + }; + const SYSTEM_CTX = { isSystem: true } as const; /** Object machine-name shape (`ObjectSchema.name` in packages/spec). */ const OBJECT_NAME_RE = /^[a-z_][a-z0-9_]*$/; -/** The columns every gate pre-scans: the parent pair. */ +/** The columns the probe pre-scans: the parent pair. */ const PARENT_COLUMNS = ['object_name', 'record_id'] as const; /** The record a row is about. */ @@ -135,10 +244,11 @@ export interface ParentRecord { * the record gate has nothing to judge them by. */ export interface OutsideClassRows { - /** The columns {@link test} reads, beyond the parent pair. */ - fields: readonly string[]; - /** Called only for a row that names no parent record. */ - test(row: Record): boolean; + /** + * The class as a WHERE on the gated object's own columns. A factory: every + * read gets a fresh tree, so nothing downstream can edit a shared one. + */ + where(): Record; } /** What one gate is: its object, its log label, its sentinel and its class. */ @@ -157,6 +267,13 @@ export interface ParentRecordGate { * deliberately; the ledger's is pinned against `PLATFORM_CAPABILITIES`. */ exemptCapability?: string; + /** + * Parent objects inside `AUDIT_EXCLUDED_OBJECTS` that a producer of this + * gate's rows OTHER than the CRUD mirror names, measured per writer. They are + * enumerated like any parent, so a row about one is served exactly when its + * record is readable. Absent: the gate's rows come from the mirror alone. + */ + namedParents?: readonly string[]; } /** @@ -200,6 +317,86 @@ 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 parent objects one gate enumerates, in name order: every registered + * object outside `AUDIT_EXCLUDED_OBJECTS` and outside the gated object, plus + * the gate's {@link ParentRecordGate.namedParents} — `null` when the engine + * exposes no registry view to enumerate. + */ +export function enumerateParentObjects(engine: ParentRecordGateEngine, gate: ParentRecordGate): string[] | null { + const getAllObjects = engine.registry?.getAllObjects; + if (typeof getAllObjects !== 'function') return null; + // Read at call time, never at module load: `audit-writers.ts` sits on an + // import cycle with this module's consumers. + const excluded = new Set(AUDIT_EXCLUDED_OBJECTS); + for (const named of gate.namedParents ?? []) excluded.delete(named); + const names = new Set(); + for (const schema of getAllObjects.call(engine.registry) ?? []) { + const name = (schema as { name?: unknown } | null | undefined)?.name; + if (typeof name !== 'string' || !OBJECT_NAME_RE.test(name)) continue; + if (name === gate.object || excluded.has(name)) continue; + names.add(name); + } + return [...names].sort(); +} + +/** + * [ADR-0139 D4, D5] How one read of `gate.object` by `context` reaches its + * parents. Reads the context, the registry, the middleware registrations and + * the datasource bindings only — it asks no security service and reads no + * row, so it decides the probe up front, never by catching a refusal. + */ +export function planParentReads( + engine: ParentRecordGateEngine, + gate: ParentRecordGate, + context: CommentReadMiddlewareCtx['context'], +): ParentReadPlan { + if (isDelegatedCaller(context)) return { probeWholeRead: 'delegated-caller' }; + const parents = enumerateParentObjects(engine, gate); + const hasObjectMiddleware = engine.hasObjectMiddleware; + const datasourceOf = engine.resolveEffectiveDatasource; + if (parents === null || typeof hasObjectMiddleware !== 'function' || typeof datasourceOf !== 'function') { + return { probeWholeRead: 'engine-cannot-classify' }; + } + // `undefined` is "rides the default driver", for the gated object and a + // parent alike: two `undefined`s are one datasource, and an explicit binding + // to the default driver's own name reads as another one, which sends that + // parent to the probe — the bound, never a refused read. + const outer = datasourceOf.call(engine, gate.object); + const leaves: string[] = []; + const probed = new Map(); + for (const parent of parents) { + if (hasObjectMiddleware.call(engine, parent)) probed.set(parent, 'object-middleware'); + else if (datasourceOf.call(engine, parent) !== outer) probed.set(parent, 'federated-parent'); + else leaves.push(parent); + } + return { probeWholeRead: null, leaves, probed }; +} + +/** The engines a gate has already said "cannot classify" for, per seam. */ +const unclassifiableReported = new WeakMap>(); + +function reportUnclassifiableOnce(engine: object, logger: CommentAccessLogger, gate: ParentRecordGate): void { + let seams = unclassifiableReported.get(engine); + if (!seams) unclassifiableReported.set(engine, (seams = new Set())); + if (seams.has(gate.seam)) return; + seams.add(gate.seam); + logger.warn( + `[audit] ${gate.seam}: the engine exposes no registry view, hasObjectMiddleware() or ` + + 'resolveEffectiveDatasource(), so it cannot say which parent objects a ReadableIds leaf can be composed ' + + `for: every read is answered by the bounded probe (at most ${PARENT_GATE_SCAN_LIMIT} candidate rows, ` + + 'fail-closed). Reported once for this engine.', + ); +} + /** * The parent objects of `objects` the caller may read at all, by the security * service's object-level read admission — or `null` when none is composed (no @@ -243,58 +440,43 @@ async function admittedParentObjects( return admitted; } -/** AND `filter` into the read's WHERE, never replacing the caller's own. */ -export function andIntoWhere(ctx: CommentReadMiddlewareCtx, filter: unknown): void { - if (!ctx.ast) return; - ctx.ast.where = ctx.ast.where ? { $and: [ctx.ast.where, filter] } : filter; -} - /** - * The parent-visibility WHERE for one read of `gate.object`: `null` when the - * caller holds the gate's exempt capability or the query matches no rows - * (nothing to narrow), one `$in` branch per parent object holding the readable - * ids (plus the outside-class rows' ids), or the gate's deny-all sentinel. - * - * `getAdmission` resolves the security service whose object-level read - * admission is asked before any parent object is read as the caller (module - * header, "A skipped object is not an access attempt"). + * [ADR-0139 D5] The bounded probe's branches for one read: one `$in` of the + * readable ids per parent object, for the rows naming one of `restrictTo` — or + * for every row when `restrictTo` is `null` (a whole-read probe). */ -export async function computeParentRecordFilter( +async function probeBranches( engine: CommentAccessEngine, ctx: CommentReadMiddlewareCtx, logger: CommentAccessLogger, gate: ParentRecordGate, getAdmission: ParentObjectReadAdmissionSource, -): Promise { - // 0. The gate's declared exemption, decided before anything is scanned, so a - // holder's broad read never meets the pre-scan bound. - if (gate.exemptCapability && callerHoldsCapability(ctx.context, gate.exemptCapability)) return null; - - // 1. The parent pairs the query would touch, read under SYSTEM context (the - // caller may not see the rows yet; that is what is being decided). The - // caller's own order rides along, so on a table larger than the scan - // bound the window scanned is the window the caller pages through. + restrictTo: { parents: string[]; why: string } | null, +): Promise>> { + // The parent pairs the query would touch, read under SYSTEM context (the + // caller may not see the rows yet; that is what is being decided). The + // caller's own order rides along, so on a table larger than the scan bound + // the window scanned is the window the caller pages through. const orderBy = (ctx.ast as { orderBy?: unknown } | undefined)?.orderBy; - const fields: string[] = [...PARENT_COLUMNS]; - if (gate.outsideClass) { - for (const f of ['id', ...gate.outsideClass.fields]) if (!fields.includes(f)) fields.push(f); - } + const callerWhere = ctx.ast?.where as Record | undefined; + const onlyParents = restrictTo ? { object_name: { $in: [...restrictTo.parents] } } : undefined; + const where = onlyParents ? (callerWhere ? { $and: [callerWhere, onlyParents] } : onlyParents) : (callerWhere ?? {}); const candidates = await engine.find(gate.object, { - where: (ctx.ast?.where as Record) ?? {}, - fields, + where, + fields: [...PARENT_COLUMNS], ...(Array.isArray(orderBy) && orderBy.length > 0 ? { orderBy } : {}), limit: PARENT_GATE_SCAN_LIMIT, context: { ...SYSTEM_CTX }, }); - if (!candidates.length) return null; + if (!candidates.length) return []; if (candidates.length >= PARENT_GATE_SCAN_LIMIT) { // Not silent (fail-closed truncation): rows beyond the scan window are // excluded, so a very broad read may omit rows the caller could see. A // record timeline scopes by `object_name` + `record_id` and never hits it. logger.warn( - `[audit] ${gate.seam}: candidate pre-scan hit the ${PARENT_GATE_SCAN_LIMIT}-row cap; ` + - 'the visibility filter for this broad read is fail-closed and may omit visible rows — ' + - 'scope the query by object_name and record_id', + `[audit] ${gate.seam}: the bounded probe (${restrictTo ? restrictTo.why : 'the whole read'}) hit its ` + + `${PARENT_GATE_SCAN_LIMIT}-row candidate cap; the visibility filter for these rows is fail-closed and ` + + 'may omit visible rows — scope the query by object_name and record_id', ); } @@ -302,49 +484,119 @@ export async function computeParentRecordFilter( * value is what the emitted filter carries, so it can only match rows the * pre-scan actually saw spelled that way. */ const byObject = new Map>(); - /** Stored ids of the rows declared outside the gate's class. */ - const outside: unknown[] = []; + const allowed = restrictTo ? new Set(restrictTo.parents) : null; for (const row of candidates) { const parent = parseParentRecord(row, gate.object); - if (!parent) { - const id = row.id; - if (gate.outsideClass && (typeof id === 'string' || typeof id === 'number') && String(id) !== '' && - gate.outsideClass.test(row)) { - outside.push(id); - } - continue; - } + if (!parent || (allowed && !allowed.has(parent.object))) continue; let ids = byObject.get(parent.object); if (!ids) byObject.set(parent.object, (ids = new Map())); if (!ids.has(parent.recordId)) ids.set(parent.recordId, row.record_id); } - if (byObject.size === 0 && outside.length === 0) return gate.denyAll; + if (byObject.size === 0) return []; + + // The one readability answer — one caller-scoped read per parent object the + // caller may read at all. An object the object-level admission refuses is + // not read: it contributes no readable id, as its refused read did, and it + // logs no denial for a skip the caller never asked for. + const callerEnvelope = withoutOperationPrivateKeys((ctx.context ?? {}) as Record); + const admitted = await admittedParentObjects(byObject.keys(), callerEnvelope, getAdmission, logger, gate); + const idSets = new Map>(); + for (const [object, ids] of byObject) { + if (admitted && !admitted.has(object)) continue; + idSets.set(object, new Set(ids.keys())); + } + const readable = idSets.size > 0 ? await resolveReadableParentIds(engine, ctx.context, idSets) : new Map>(); - // 2. The one readability answer — one caller-scoped read per parent object - // the caller may read at all. An object the object-level admission - // refuses is not read: it contributes no readable id, as its refused read - // did, and it logs no denial for a skip the caller never asked for. + // One branch per parent object that kept at least one readable record. const branches: Array> = []; - if (byObject.size > 0) { - const callerEnvelope = withoutOperationPrivateKeys((ctx.context ?? {}) as Record); - const admitted = await admittedParentObjects(byObject.keys(), callerEnvelope, getAdmission, logger, gate); - const idSets = new Map>(); - for (const [object, ids] of byObject) { - if (admitted && !admitted.has(object)) continue; - idSets.set(object, new Set(ids.keys())); + for (const [object, ids] of byObject) { + const visible = readable.get(object); + if (!visible) continue; + const kept: unknown[] = []; + for (const [recordId, stored] of ids) if (visible.has(recordId)) kept.push(stored); + if (kept.length) branches.push({ object_name: object, record_id: { $in: kept } }); + } + return branches; +} + +/** "the bounded probe's parents, by class" — how a restricted probe names itself. */ +function describeProbed(probed: ReadonlyMap): string { + const byClass = new Map(); + for (const [parent, why] of probed) { + const list = byClass.get(why) ?? []; + list.push(parent); + byClass.set(why, list); + } + return [...byClass].map(([why, parents]) => `${why}: ${parents.join(', ')}`).join('; '); +} + +/** AND `filter` into the read's WHERE, never replacing the caller's own. */ +export function andIntoWhere(ctx: CommentReadMiddlewareCtx, filter: unknown): void { + if (!ctx.ast) return; + ctx.ast.where = ctx.ast.where ? { $and: [ctx.ast.where, filter] } : filter; +} + +/** + * The parent-visibility WHERE for one read of `gate.object`: `null` only when + * the caller holds the gate's exempt capability (nothing to narrow); + * otherwise one branch per enumerated parent object — a `ReadableIds` leaf, or + * the bounded probe's `$in` of readable ids for D5's classes — plus the + * outside class's predicate, ORed; or the gate's deny-all sentinel when no + * branch remains, or when the probe fails. + * + * `getAdmission` resolves the security service whose object-level read + * admission the probe asks before any parent object is read as the caller + * (module header, "A skipped object is not an access attempt"). + * + * Never throws for a fault of its own: the probe's failure is answered with + * the deny-all sentinel and said. A leaf is composed by the engine after this + * returns, so its refusal is the read's (module header, ADR-0139 D3). + */ +export async function computeParentRecordFilter( + engine: ParentRecordGateEngine, + ctx: CommentReadMiddlewareCtx, + logger: CommentAccessLogger, + gate: ParentRecordGate, + getAdmission: ParentObjectReadAdmissionSource, +): Promise { + // 0. The gate's declared exemption, decided before anything else. + if (gate.exemptCapability && callerHoldsCapability(ctx.context, gate.exemptCapability)) return null; + + // 1. Leaf or probe, per parent object, before any branch is written. + const plan = planParentReads(engine, gate, ctx.context); + if (plan.probeWholeRead === 'engine-cannot-classify') reportUnclassifiableOnce(engine, logger, gate); + + const branches: Array> = []; + // 2. One ReadableIds leaf per composable parent object (ADR-0139 D4). + if (plan.probeWholeRead === null) { + for (const parent of plan.leaves) { + branches.push({ object_name: parent, record_id: { $in: ReadableIds.of(parent) } }); } - const readable = idSets.size > 0 ? await resolveReadableParentIds(engine, ctx.context, idSets) : new Map>(); - - // 3. One branch per parent object that kept at least one readable record. - for (const [object, ids] of byObject) { - const visible = readable.get(object); - if (!visible) continue; - const kept: unknown[] = []; - for (const [recordId, stored] of ids) if (visible.has(recordId)) kept.push(stored); - if (kept.length) branches.push({ object_name: object, record_id: { $in: kept } }); + } + + // 3. The bounded probe for D5's classes (ADR-0139 D5), fail-closed. + const restrictTo = + plan.probeWholeRead !== null + ? null + : plan.probed.size > 0 + ? { parents: [...plan.probed.keys()], why: describeProbed(plan.probed) } + : undefined; + if (restrictTo !== undefined) { + try { + branches.push(...(await probeBranches(engine, ctx, logger, gate, getAdmission, restrictTo))); + } catch (err) { + // A probe failure must never fall open into a leak. + logger.warn( + `[audit] ${gate.seam}: the bounded probe ` + + `(${restrictTo ? restrictTo.why : `the whole read: ${plan.probeWholeRead}`}) failed, denying all ` + + `(${(err as Error)?.message ?? err})`, + ); + return gate.denyAll; } } - if (outside.length) branches.push({ id: { $in: outside } }); + + // 4. The rows declared outside the gate's class, as a predicate. + if (gate.outsideClass) branches.push(gate.outsideClass.where()); if (branches.length === 0) return gate.denyAll; return branches.length === 1 ? branches[0] : { $or: branches }; } From a367e0c1c995817a7a49cb0fb2c9edf72b910b97 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 13:38:25 +0000 Subject: [PATCH 02/10] wip(plugin-audit): the gate tests pin the leaf path, D5's routing and the no-catch consumers; the stand-in security doubles answer both read halves Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- ...ctivity-milestone-kind.integration.test.ts | 11 +- .../src/activity-read-visibility.test.ts | 255 +++++++++++++++++- ...tivity-withheld-update.integration.test.ts | 11 +- .../src/audit-log-read-visibility.test.ts | 144 ++++++++-- ...-record-read-admission.integration.test.ts | 43 ++- .../src/parent-record-read-gate.ts | 7 +- 6 files changed, 429 insertions(+), 42 deletions(-) diff --git a/packages/plugins/plugin-audit/src/activity-milestone-kind.integration.test.ts b/packages/plugins/plugin-audit/src/activity-milestone-kind.integration.test.ts index 86115633c9e..aebd28e285f 100644 --- a/packages/plugins/plugin-audit/src/activity-milestone-kind.integration.test.ts +++ b/packages/plugins/plugin-audit/src/activity-milestone-kind.integration.test.ts @@ -105,7 +105,7 @@ describe('[#22771] a fired milestone is served with metadata.kind, judged like t const allFields = (object: string): string[] => Object.keys(((engine as any).getSchema(object)?.fields ?? {}) as Record); - /** The double: the two contract members, answered per reader. No field is served masked. */ + /** The double: the two field members, answered per reader, and the two read halves (below). No field is served masked. */ const security = { async getReadableFields(object: string, context?: any): Promise { const all = allFields(object); @@ -116,6 +116,15 @@ describe('[#22771] a fired milestone is served with metadata.kind, judged like t async getQueryableFields(object: string, context?: any): Promise { return security.getReadableFields(object, context); }, + // [ADR-0139 E5] Every reader reads every parent record here: the two halves + // of a read the engine composes the parent-record gate's ReadableIds + // leaves from — the object-level admission and the read scope (none). + async canReadObject(): Promise { + return true; + }, + async getReadFilter(): Promise | undefined> { + return undefined; + }, }; /** The console's activity read: one record's rows, as `context`. */ 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 6364e85e18e..69274b766f8 100644 --- a/packages/plugins/plugin-audit/src/activity-read-visibility.test.ts +++ b/packages/plugins/plugin-audit/src/activity-read-visibility.test.ts @@ -2,16 +2,28 @@ /** * The `sys_activity` read gate's branches that a real engine cannot easily be - * made to take — a failing pre-scan, a failing parent read, the scan bound — - * plus the shape of what it emits. What the emitted filter SELECTS is pinned - * against a real driver in `activity-read-visibility.integration.test.ts`. + * made to take, plus the shape of what it emits: + * + * - 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; + * - 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. + * + * What the emitted filter SELECTS is pinned against a real engine and driver + * in `parent-record-read-leaf.integration.test.ts` (the leaf) and + * `activity-read-visibility.integration.test.ts` (the probe). */ import { describe, it, expect, vi } from 'vitest'; +import { isReadableIds } from '@objectstack/spec/data'; import { installActivityReadVisibility, parseActivityParent } from './activity-read-visibility.js'; import { LEDGER_AUDIT_CAPABILITY } from './audit-log-read-visibility.js'; +import { AUDIT_EXCLUDED_OBJECTS } from './audit-writers.js'; import type { CommentAccessEngine, CommentReadMiddlewareCtx } from './comment-access-hooks.js'; -import type { ParentObjectReadAdmissionSource } from './parent-record-read-gate.js'; +import type { ParentObjectReadAdmissionSource, ParentRecordGateEngine } from './parent-record-read-gate.js'; const DENY = { id: '__activity_parent_denied__' }; const silentLogger = () => ({ info: vi.fn(), warn: vi.fn(), debug: vi.fn() }); @@ -125,9 +137,9 @@ describe('installActivityReadVisibility', () => { expect(calls.probes).toEqual([]); }); - it('leaves a read that matches nothing alone', async () => { + it('a read the probe finds no row for is still narrowed, so a row written meanwhile is judged', async () => { const { mw } = install({ scan: () => [] }); - expect((await read(mw)).where).toBeUndefined(); + expect((await read(mw)).where).toEqual(DENY); }); it('a parent object whose read throws contributes nothing (fail closed)', async () => { @@ -328,4 +340,235 @@ describe('installActivityReadVisibility', () => { const engine: CommentAccessEngine = { registerHook: () => {}, find: async () => [], findOne: async () => null }; expect(() => installActivityReadVisibility(engine, silentLogger(), () => undefined)).not.toThrow(); }); + + it('an engine that cannot classify a parent sends the whole read to the probe, and says so once', async () => { + const logger = silentLogger(); + const { mw, calls } = install({ scan: () => [{ object_name: 'crm_case', record_id: 'c1' }], readable: { crm_case: ['c1'] }, logger }); + await read(mw); + await read(mw); + expect(calls.scans).toHaveLength(2); + const said = logger.warn.mock.calls.filter(([line]) => String(line).includes('cannot say which parent objects')); + expect(said).toHaveLength(1); + }); +}); + +/** A stand-in engine that answers the three questions the gate classifies a parent by. */ +function installClassifying(opts: { + objects: string[]; + /** Parent objects an object-scoped read middleware narrows. */ + middleware?: string[]; + /** object → datasource; absent: the default driver (`undefined`). */ + datasource?: Record; + scan?: (options: any) => Array>; + readable?: Record; + admission?: ParentObjectReadAdmissionSource; + logger?: ReturnType; + registry?: unknown; +}) { + let mw!: Mw; + const calls = { scans: [] as any[], probes: [] as Array<{ object: string; ids: string[]; context: any }> }; + const engine: ParentRecordGateEngine = { + registerHook: () => {}, + registerMiddleware: (fn) => { + mw = fn as Mw; + }, + find: async (object: string, options: any) => { + if (object === 'sys_activity') { + calls.scans.push(options); + return (opts.scan?.(options) ?? []) as any; + } + const ids: string[] = (options?.where?.id?.$in ?? []).map(String); + calls.probes.push({ object, ids, context: options?.context }); + const list = opts.readable?.[object] ?? []; + return ids.filter((id) => list.includes(id)).map((id) => ({ id })) as any; + }, + findOne: async () => null, + registry: (opts.registry ?? { getAllObjects: () => opts.objects.map((name) => ({ name })) }) as any, + hasObjectMiddleware: (object: string) => (opts.middleware ?? []).includes(object), + resolveEffectiveDatasource: (object: string) => opts.datasource?.[object], + }; + installActivityReadVisibility(engine, opts.logger ?? silentLogger(), opts.admission ?? (() => undefined)); + return { mw, calls }; +} + +/** The `$or` branches of a gate WHERE (or the one branch). */ +const branchesOf = (where: any): any[] => (where?.$or ? where.$or : [where]); +const leafObjectOf = (branch: any): string | null => { + const comparand = branch?.record_id?.$in; + return isReadableIds(comparand) ? comparand.object : null; +}; + +describe('[ADR-0139 D4] the leaf path: one ReadableIds leaf per parent object, and no pre-scan', () => { + it('writes one branch per registered parent object, in name order, each a branded leaf on record_id', async () => { + const { mw, calls } = installClassifying({ objects: ['crm_deal', 'crm_case'] }); + const { where, ran } = await read(mw); + expect(ran).toBe(true); + const branches = branchesOf(where); + expect(branches.map((b) => b.object_name)).toEqual(['crm_case', 'crm_deal']); + expect(branches.map(leafObjectOf)).toEqual(['crm_case', 'crm_deal']); + // Nothing scanned, nothing read as the caller. + expect(calls.scans).toEqual([]); + expect(calls.probes).toEqual([]); + }); + + it('enumerates outside the audit writer’s excluded objects, the gated object and non-machine names', async () => { + const excluded = AUDIT_EXCLUDED_OBJECTS.filter((o) => o !== 'sys_activity').slice(0, 3); + expect(excluded.length).toBeGreaterThan(0); + const { mw } = installClassifying({ objects: ['crm_case', 'sys_activity', 'Not A Name', ...excluded] }); + expect(branchesOf((await read(mw)).where).map(leafObjectOf)).toEqual(['crm_case']); + }); + + it('asks the security service nothing: the engine composes each leaf, so a refused object is never read', async () => { + const canReadObject = vi.fn(async () => false); + const { mw, calls } = installClassifying({ objects: ['crm_case', 'crm_secret'], admission: () => ({ canReadObject }) }); + expect(branchesOf((await read(mw)).where).map(leafObjectOf)).toEqual(['crm_case', 'crm_secret']); + expect(canReadObject).not.toHaveBeenCalled(); + expect(calls.probes).toEqual([]); + }); + + it('ANDs onto the caller’s where, on all four reads, and leaves writes, system and context-less reads alone', async () => { + const existing = { type: 'updated' }; + for (const operation of ['find', 'findOne', 'count', 'aggregate'] as const) { + const { mw } = installClassifying({ objects: ['crm_case'] }); + const { where } = await read(mw, { operation, ast: { object: 'sys_activity', where: existing } }); + expect(where.$and[0]).toBe(existing); + expect(leafObjectOf(where.$and[1])).toBe('crm_case'); + } + const { mw } = installClassifying({ objects: ['crm_case'] }); + expect((await read(mw, { operation: 'insert' })).where).toBeUndefined(); + expect((await read(mw, { context: { isSystem: true } })).where).toBeUndefined(); + expect((await read(mw, { context: undefined })).where).toBeUndefined(); + }); + + it('a fresh leaf per read: nothing is shared between two reads', async () => { + const { mw } = installClassifying({ objects: ['crm_case'] }); + const first = (await read(mw)).where as any; + const second = (await read(mw)).where as any; + expect(first.record_id.$in).not.toBe(second.record_id.$in); + }); + + it('no parent object to enumerate: the deny-all sentinel', async () => { + const { mw } = installClassifying({ objects: ['sys_activity'] }); + expect((await read(mw)).where).toEqual(DENY); + }); + + it('[#21260] a holder of the ledger’s audit capability is narrowed here exactly like any caller', async () => { + const { mw } = installClassifying({ objects: ['crm_case'] }); + const holder = { userId: 'u1', systemPermissions: [LEDGER_AUDIT_CAPABILITY] }; + expect(leafObjectOf((await read(mw, { context: holder })).where)).toBe('crm_case'); + }); +}); + +describe('[ADR-0139 D5] three classes keep the bounded probe, by name, chosen before any leaf is written', () => { + 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'], + middleware: ['sys_attachment'], + scan: () => [ + { object_name: 'sys_attachment', record_id: 'f1' }, + { object_name: 'sys_attachment', record_id: 'f2' }, + ], + readable: { sys_attachment: ['f2'] }, + }); + const branches = branchesOf((await read(mw)).where); + expect(branches.map(leafObjectOf)).toEqual(['crm_case', null]); + expect(branches[1]).toEqual({ object_name: 'sys_attachment', record_id: { $in: ['f2'] } }); + expect(calls.scans).toEqual([ + { + where: { object_name: { $in: ['sys_attachment'] } }, + fields: ['object_name', 'record_id'], + limit: 2000, + context: { isSystem: true }, + }, + ]); + expect(calls.probes.map((p) => p.object)).toEqual(['sys_attachment']); + }); + + it('a parent bound to another datasource than the stream is probed', async () => { + const { mw, calls } = installClassifying({ + objects: ['crm_case', 'ext_ticket'], + datasource: { ext_ticket: 'warehouse' }, + scan: () => [{ object_name: 'ext_ticket', record_id: 't1' }], + readable: { ext_ticket: ['t1'] }, + }); + const branches = branchesOf((await read(mw)).where); + expect(branches.map(leafObjectOf)).toEqual(['crm_case', null]); + expect(branches[1]).toEqual({ object_name: 'ext_ticket', record_id: { $in: ['t1'] } }); + expect(calls.scans[0].where).toEqual({ object_name: { $in: ['ext_ticket'] } }); + }); + + it('every parent federated (a telemetry-split stream): no leaf, the probe answers every row', async () => { + const { mw } = installClassifying({ + objects: ['crm_case', 'crm_deal'], + datasource: { sys_activity: 'telemetry' }, + scan: () => [{ object_name: 'crm_case', record_id: 'c1' }], + readable: { crm_case: ['c1'] }, + }); + 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 () => { + 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({}); + expect(calls.probes[0].context).toMatchObject({ userId: 'u_agent', onBehalfOf }); + }); + + it('the probe’s cap warning names the class it was probing for', async () => { + const logger = silentLogger(); + const rows = Array.from({ length: 2000 }, (_, i) => ({ object_name: 'sys_attachment', record_id: `f${i}` })); + const { mw } = installClassifying({ objects: ['crm_case', 'sys_attachment'], middleware: ['sys_attachment'], scan: () => rows, logger }); + await read(mw); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('(object-middleware: sys_attachment) hit the 2000-row cap')); + }); + + it('a probe that fails denies the whole read, and says so', async () => { + const logger = silentLogger(); + const { mw } = installClassifying({ + objects: ['crm_case', 'sys_attachment'], + middleware: ['sys_attachment'], + scan: () => { + throw new Error('pre-scan exploded'); + }, + logger, + }); + expect((await read(mw)).where).toEqual(DENY); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('failed, denying all')); + }); +}); + +describe('[ADR-0139 D3] the gate catches nothing the read itself raises', () => { + it('a refusal the read raises after the gate hands it on reaches the caller unchanged', async () => { + const { mw } = installClassifying({ objects: ['crm_case'] }); + const refusal = Object.assign(new Error('[driver-x] where.$or[0].record_id.$in holds a ReadableIds leaf'), { + code: 'INTERNAL_ERROR', + status: 500, + }); + const ctx: CommentReadMiddlewareCtx = { object: 'sys_activity', operation: 'find', ast: { object: 'sys_activity' }, context: { userId: 'u1' } }; + await expect(mw(ctx, async () => { + throw refusal; + })).rejects.toBe(refusal); + }); + + it('a fault in deciding how to read the parents fails the read: it is never served as the deny-all sentinel', async () => { + const registry = { + getAllObjects: () => { + throw new Error('registry exploded'); + }, + }; + const { mw } = installClassifying({ objects: [], registry }); + let ran = false; + const ctx: CommentReadMiddlewareCtx = { object: 'sys_activity', operation: 'find', ast: { object: 'sys_activity' }, context: { userId: 'u1' } }; + await expect(mw(ctx, async () => { + ran = true; + })).rejects.toThrow('registry exploded'); + expect(ran).toBe(false); + expect(ctx.ast?.where).toBeUndefined(); + }); }); diff --git a/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts b/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts index eb99484e462..1762f402a50 100644 --- a/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts +++ b/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts @@ -97,7 +97,7 @@ describe('[#21388] an update whose every recorded change is withheld from the re const allFields = (object: string): string[] => Object.keys(((engine as any).getSchema(object)?.fields ?? {}) as Record); - /** The double: the two contract members, answered per reader. */ + /** The double: the two field members, answered per reader, and the two read halves (below). */ const security = { async getReadableFields(object: string, context?: any): Promise { const all = allFields(object); @@ -110,6 +110,15 @@ describe('[#21388] an update whose every recorded change is withheld from the re async getQueryableFields(object: string, context?: any): Promise { return security.getReadableFields(object, context); }, + // [ADR-0139 E5] Every reader reads every parent record here: the two halves + // of a read the engine composes the parent-record gate's ReadableIds + // leaves from — the object-level admission and the read scope (none). + async canReadObject(): Promise { + return true; + }, + async getReadFilter(): Promise | undefined> { + return undefined; + }, }; const scoped = (object: string, record: string) => ({ object_name: object, record_id: record }); diff --git a/packages/plugins/plugin-audit/src/audit-log-read-visibility.test.ts b/packages/plugins/plugin-audit/src/audit-log-read-visibility.test.ts index 5ab2bd32911..c6c4fe6eed6 100644 --- a/packages/plugins/plugin-audit/src/audit-log-read-visibility.test.ts +++ b/packages/plugins/plugin-audit/src/audit-log-read-visibility.test.ts @@ -2,21 +2,29 @@ /** * [#21175] The `sys_audit_log` read gate's row classes and fail-closed - * branches, against a stand-in engine — plus the shape of what it emits. What - * the emitted filter SELECTS is pinned against a real driver in + * branches, against stand-in engines — plus the shape of what it emits: the + * leaf path (ADR-0139 D4) against an engine that answers the gate's three + * classifying questions, and the bounded probe against one that answers none + * of them. What the emitted filter SELECTS is pinned against a real driver in + * `parent-record-read-leaf.integration.test.ts` and * `audit-log-read-visibility.integration.test.ts`. */ import { describe, it, expect, vi } from 'vitest'; import { ADMIN_FULL_ACCESS_CAPABILITIES } from '@objectstack/spec'; +import { isReadableIds } from '@objectstack/spec/data'; import { PLATFORM_CAPABILITIES } from '@objectstack/spec/security'; +import { SystemObjectName } from '@objectstack/spec/system'; import { LEDGER_AUDIT_CAPABILITY, + LEDGER_NAMED_PARENTS, installAuditLogReadVisibility, isLedgerRowAboutNoRecord, + ledgerRowAboutNoRecordWhere, } from './audit-log-read-visibility.js'; +import { AUDIT_EXCLUDED_OBJECTS } from './audit-writers.js'; import type { CommentAccessEngine, CommentReadMiddlewareCtx } from './comment-access-hooks.js'; -import type { ParentObjectReadAdmissionSource } from './parent-record-read-gate.js'; +import type { ParentObjectReadAdmissionSource, ParentRecordGateEngine } from './parent-record-read-gate.js'; const DENY = { id: '__audit_log_parent_denied__' }; const silentLogger = () => ({ info: vi.fn(), warn: vi.fn(), debug: vi.fn() }); @@ -90,8 +98,24 @@ describe('isLedgerRowAboutNoRecord', () => { }); }); -describe('installAuditLogReadVisibility', () => { - it('keeps rows about a readable record by their parent pair, as stored', async () => { +/** The outside class's predicate, as the gate ORs it in. */ +const OUTSIDE = ledgerRowAboutNoRecordWhere(); + +describe('ledgerRowAboutNoRecordWhere', () => { + it('is the outside class as a predicate on the ledger’s own columns, a fresh tree per call', () => { + expect(OUTSIDE).toEqual({ + $and: [ + { $or: [{ record_id: null }, { record_id: '' }] }, + { action: { $ne: null } }, + { action: { $nin: ['', 'create', 'update', 'delete', 'read'] } }, + ], + }); + expect(ledgerRowAboutNoRecordWhere()).not.toBe(ledgerRowAboutNoRecordWhere()); + }); +}); + +describe('installAuditLogReadVisibility — the bounded probe (an engine that cannot classify a parent)', () => { + it('keeps rows about a readable record by their parent pair, as stored, beside the outside class', async () => { const { mw } = install({ scan: () => [ { id: 'l1', action: 'create', object_name: 'crm_case', record_id: 'c1' }, @@ -106,11 +130,12 @@ describe('installAuditLogReadVisibility', () => { $or: [ { object_name: 'crm_case', record_id: { $in: ['c1'] } }, { object_name: 'sys_session', record_id: { $in: ['s1'] } }, + OUTSIDE, ], }); }); - it('keeps the rows about no record by their stored id, beside the record branches', async () => { + it('the rows about no record are kept by the predicate, never by a stored id the scan saw', async () => { const { mw, calls } = install({ scan: () => [ { id: 'l1', action: 'create', object_name: 'crm_case', record_id: 'c1' }, @@ -119,7 +144,7 @@ describe('installAuditLogReadVisibility', () => { ], readable: { crm_case: [] }, }); - expect((await read(mw)).where).toEqual({ id: { $in: ['l2', 'l3'] } }); + expect((await read(mw)).where).toEqual(OUTSIDE); expect(calls.probes.map((p) => p.object)).toEqual(['crm_case']); }); @@ -142,9 +167,7 @@ describe('installAuditLogReadVisibility', () => { const canReadObject = vi.fn(async (object: string) => object !== 'sys_migration'); const { mw, calls } = install({ scan, readable, admission: () => ({ canReadObject }) }); expect((await read(mw)).where).toEqual(before); - expect(before).toEqual({ - $or: [{ object_name: 'crm_case', record_id: { $in: ['c1'] } }, { id: { $in: ['l3'] } }], - }); + expect(before).toEqual({ $or: [{ object_name: 'crm_case', record_id: { $in: ['c1'] } }, OUTSIDE] }); expect(calls.probes.map((p) => p.object)).toEqual(['crm_case']); expect(canReadObject.mock.calls.map(([object]) => object).sort()).toEqual(['crm_case', 'sys_migration']); }); @@ -165,11 +188,12 @@ describe('installAuditLogReadVisibility', () => { const { mw, calls } = install({ scan: () => [{ id: 'l1', action: 'update', object_name: 'crm_case', record_id: null }], }); - expect((await read(mw)).where).toEqual(DENY); + expect((await read(mw)).where).toEqual(OUTSIDE); + expect(isLedgerRowAboutNoRecord({ id: 'l1', action: 'update', object_name: 'crm_case', record_id: null })).toBe(false); expect(calls.probes).toEqual([]); }); - it('denies all when no scanned record is readable and no row is about no record', async () => { + it('no scanned record readable: only the outside class remains', async () => { const { mw } = install({ scan: () => [ { id: 'l1', action: 'delete', object_name: 'crm_case', record_id: 'c1' }, @@ -179,7 +203,7 @@ describe('installAuditLogReadVisibility', () => { ], readable: { crm_case: [] }, }); - expect((await read(mw)).where).toEqual(DENY); + expect((await read(mw)).where).toEqual(OUTSIDE); }); it('a parent object whose read throws contributes nothing (fail closed)', async () => { @@ -195,10 +219,10 @@ describe('installAuditLogReadVisibility', () => { }, }, }); - expect((await read(mw)).where).toEqual({ object_name: 'crm_case', record_id: { $in: ['c1'] } }); + expect((await read(mw)).where).toEqual({ $or: [{ object_name: 'crm_case', record_id: { $in: ['c1'] } }, OUTSIDE] }); }); - it('denies all, and says so, when the pre-scan itself fails', async () => { + it('denies all, and says so, when the pre-scan itself fails — the outside class included', async () => { const logger = silentLogger(); const { mw } = install({ scan: () => { @@ -209,17 +233,18 @@ describe('installAuditLogReadVisibility', () => { const { where, ran } = await read(mw); expect(where).toEqual(DENY); expect(ran).toBe(true); - expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('audit log read visibility: filter failed, denying all')); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('audit log read visibility: the bounded probe')); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('failed, denying all')); }); - it('scans under SYSTEM context with the caller’s where and order, the class columns included, bounded', async () => { + it('scans under SYSTEM context with the caller’s where and order, the parent pair only, bounded', async () => { const { mw, calls } = install({ scan: () => [] }); const where = { action: 'update' }; const orderBy = [{ field: 'created_at', order: 'desc' }]; const { where: after } = await read(mw, { ast: { object: 'sys_audit_log', where, orderBy } }); - expect(after).toEqual(where); + expect(after).toEqual({ $and: [where, OUTSIDE] }); expect(calls.scans).toEqual([ - { where, fields: ['object_name', 'record_id', 'id', 'action'], orderBy, limit: 2000, context: { isSystem: true } }, + { where, fields: ['object_name', 'record_id'], orderBy, limit: 2000, context: { isSystem: true } }, ]); }); @@ -230,7 +255,7 @@ describe('installAuditLogReadVisibility', () => { }); const existing = { action: 'create' }; const { where } = await read(mw, { ast: { object: 'sys_audit_log', where: existing } }); - expect(where).toEqual({ $and: [existing, { object_name: 'crm_case', record_id: { $in: ['c1'] } }] }); + expect(where).toEqual({ $and: [existing, { $or: [{ object_name: 'crm_case', record_id: { $in: ['c1'] } }, OUTSIDE] }] }); }); it('warns when the scan hits its bound', async () => { @@ -238,14 +263,15 @@ describe('installAuditLogReadVisibility', () => { const rows = Array.from({ length: 2000 }, (_, i) => ({ id: `l${i}`, action: 'update', object_name: 'crm_case', record_id: `c${i}` })); const { mw } = install({ scan: () => rows, readable: { crm_case: ['c1'] }, logger }); await read(mw); - expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('audit log read visibility: candidate pre-scan hit the 2000-row cap')); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining("audit log read visibility: the bounded probe's candidate pre-scan")); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('hit the 2000-row cap')); }); it('narrows count, aggregate and findOne, and leaves writes, system and context-less reads alone', async () => { const scan = () => [{ id: 'l1', action: 'create', object_name: 'crm_case', record_id: 'c1' }]; for (const operation of ['count', 'aggregate', 'findOne'] as const) { const { mw } = install({ scan, readable: { crm_case: [] } }); - expect((await read(mw, { operation })).where).toEqual(DENY); + expect((await read(mw, { operation })).where).toEqual(OUTSIDE); } const { mw, calls } = install({ scan, readable: { crm_case: [] } }); expect((await read(mw, { operation: 'insert' })).where).toBeUndefined(); @@ -260,6 +286,76 @@ describe('installAuditLogReadVisibility', () => { }); }); +/** A stand-in engine that answers the three questions the gate classifies a parent by. */ +function installClassifying(objects: string[]) { + let mw!: Mw; + const calls = { scans: [] as any[] }; + const engine: ParentRecordGateEngine = { + registerHook: () => {}, + registerMiddleware: (fn) => { + mw = fn as Mw; + }, + find: async (object: string, options: any) => { + calls.scans.push({ object, options }); + return []; + }, + findOne: async () => null, + registry: { getAllObjects: () => objects.map((name) => ({ name })) } as any, + hasObjectMiddleware: () => false, + resolveEffectiveDatasource: () => undefined, + }; + installAuditLogReadVisibility(engine, silentLogger(), () => undefined); + return { mw, calls }; +} + +describe('[ADR-0139 D4] the ledger on the leaf path', () => { + const leafOf = (branch: any): string | null => { + const comparand = branch?.record_id?.$in; + return isReadableIds(comparand) ? comparand.object : null; + }; + + it('one leaf per parent object, the outside class as a predicate beside them, nothing scanned', async () => { + const { mw, calls } = installClassifying(['crm_case', 'crm_deal', 'sys_audit_log']); + const where = (await read(mw)).where as any; + expect(where.$or.map(leafOf)).toEqual(['crm_case', 'crm_deal', null]); + expect(where.$or[2]).toEqual(OUTSIDE); + expect(calls.scans).toEqual([]); + }); + + it('names the session its auth-event sink writes about, which the audit writer excludes', async () => { + expect(LEDGER_NAMED_PARENTS).toEqual([SystemObjectName.SESSION]); + expect(AUDIT_EXCLUDED_OBJECTS).toContain(SystemObjectName.SESSION); + const others = AUDIT_EXCLUDED_OBJECTS.filter((o) => o !== SystemObjectName.SESSION && o !== 'sys_audit_log'); + const { mw } = installClassifying(['crm_case', SystemObjectName.SESSION, ...others]); + const where = (await read(mw)).where as any; + expect(where.$or.map(leafOf)).toEqual(['crm_case', SystemObjectName.SESSION, null]); + }); + + it('the exemption is decided before anything is enumerated', async () => { + const getAllObjects = vi.fn(() => [{ name: 'crm_case' }]); + let mw!: Mw; + const engine: ParentRecordGateEngine = { + registerHook: () => {}, + registerMiddleware: (fn) => { + mw = fn as Mw; + }, + find: async () => [], + findOne: async () => null, + registry: { getAllObjects } as any, + hasObjectMiddleware: () => false, + resolveEffectiveDatasource: () => undefined, + }; + installAuditLogReadVisibility(engine, silentLogger(), () => undefined); + const existing = { action: 'delete' }; + const { where } = await read(mw, { + context: { userId: 'u1', systemPermissions: [LEDGER_AUDIT_CAPABILITY] }, + ast: { object: 'sys_audit_log', where: existing }, + }); + expect(where).toBe(existing); + expect(getAllObjects).not.toHaveBeenCalled(); + }); +}); + describe('[#21260] the ledger audit capability', () => { const unreadable = () => [ { id: 'l1', action: 'delete', object_name: 'crm_case', record_id: 'c1' }, @@ -284,7 +380,7 @@ describe('[#21260] the ledger audit capability', () => { } }); - it('its holder’s broad read never meets the pre-scan bound', async () => { + it('its holder’s broad read never meets the probe’s bound', async () => { const logger = silentLogger(); const rows = Array.from({ length: 2000 }, (_, i) => ({ id: `l${i}`, action: 'update', object_name: 'crm_case', record_id: `c${i}` })); const { mw, calls } = install({ scan: () => rows, readable: { crm_case: [] }, logger }); @@ -296,7 +392,7 @@ describe('[#21260] the ledger audit capability', () => { it('a caller without it gets exactly the parent-record gate: other capabilities, a non-list, or none', async () => { for (const systemPermissions of [['setup.access', 'manage_users'], LEDGER_AUDIT_CAPABILITY, undefined]) { const { mw, calls } = install({ scan: unreadable, readable: { crm_case: [], sys_session: [] } }); - expect((await read(mw, { context: { userId: 'u1', systemPermissions } })).where).toEqual(DENY); + expect((await read(mw, { context: { userId: 'u1', systemPermissions } })).where).toEqual(OUTSIDE); expect(calls.scans).toHaveLength(1); } }); diff --git a/packages/plugins/plugin-audit/src/parent-record-read-admission.integration.test.ts b/packages/plugins/plugin-audit/src/parent-record-read-admission.integration.test.ts index f4c9fe4201c..fc49aa9786f 100644 --- a/packages/plugins/plugin-audit/src/parent-record-read-admission.integration.test.ts +++ b/packages/plugins/plugin-audit/src/parent-record-read-admission.integration.test.ts @@ -28,6 +28,16 @@ * The stand-in service can withdraw `canReadObject` — the contract's * optional-method state — so every case compares the gate WITH the admission * against the gate as it reads without one, on the same kernel and rows. + * + * [ADR-0139 E5] Two readers of that admission now. The vault carries an + * object-scoped read middleware (the stand-in for plugin-security's + * refusal), so it is one of D5's classes and the gate reads it through its + * bounded probe, which asks `canReadObject` first — the behaviour pinned here. + * Every other parent object is a `ReadableIds` leaf, which the ENGINE + * composes from `canReadObject` (or, when that is withdrawn, `explain`) and + * `getReadFilter`, without reading the parent as the caller; the stand-in + * answers both halves, so withdrawing `canReadObject` withdraws the probe's + * admission alone. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -63,11 +73,25 @@ const refusals: Array<{ object: string; operation: string; userId?: string }> = /** The object-level admission's questions: object and the envelope it was asked with. */ const asked: Array<{ object: string; context: any }> = []; -/** The stand-in `security` service; `canReadObject` is withdrawn by deleting it. */ -const security: { canReadObject?: (object: string, context?: any) => Promise } = {}; +/** The admission's verdict, unrecorded. */ +const admits = (object: string, context?: any) => object !== VAULT || isAdmin(context); + +/** + * The stand-in `security` service; `canReadObject` is withdrawn by deleting it. + * `explain` (the engine's fallback when it is withdrawn) and `getReadFilter` + * (no row restriction) are the halves the engine composes a leaf from. + */ +const security: { + canReadObject?: (object: string, context?: any) => Promise; + explain: (request: { object: string }, context?: any) => Promise<{ allowed: boolean }>; + getReadFilter: (object: string, context?: any) => Promise | undefined>; +} = { + explain: async ({ object }, context) => ({ allowed: admits(object, context) }), + getReadFilter: async () => undefined, +}; const admit = async (object: string, context?: any) => { asked.push({ object, context }); - return object !== VAULT || isAdmin(context); + return admits(object, context); }; function securityStandIn(): Plugin { @@ -179,13 +203,18 @@ describe('parent-record read gates: the object-level read admission', () => { })); expect(after.value).toEqual(before.value); expect(after.refusals).toEqual([]); - // Asked once per parent object per read, with the member's envelope minus - // the operation-private keys — the envelope the caller-scoped read gets. - expect(after.asked.map((a) => a.object).sort()).toEqual([OPEN, OPEN, VAULT, VAULT].sort()); - for (const a of after.asked) { + // The gate's probe asks about the vault once per read (rows, total), with + // the member's envelope minus the operation-private keys — the envelope + // the caller-scoped read gets. + const probeAsked = after.asked.filter((a) => a.object === VAULT); + expect(probeAsked).toHaveLength(2); + for (const a of probeAsked) { expect(a.context).toMatchObject({ userId: MEMBER.userId, positions: MEMBER.positions }); expect(a.context).not.toHaveProperty('__readScope'); } + // The board is a ReadableIds leaf: the engine asks about it, once per + // read, while composing — and never reads it as the member. + expect(after.asked.filter((a) => a.object === OPEN)).toHaveLength(2); }); } 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 21f7643ce30..211bc4e1d41 100644 --- a/packages/plugins/plugin-audit/src/parent-record-read-gate.ts +++ b/packages/plugins/plugin-audit/src/parent-record-read-gate.ts @@ -474,9 +474,10 @@ async function probeBranches( // excluded, so a very broad read may omit rows the caller could see. A // record timeline scopes by `object_name` + `record_id` and never hits it. logger.warn( - `[audit] ${gate.seam}: the bounded probe (${restrictTo ? restrictTo.why : 'the whole read'}) hit its ` + - `${PARENT_GATE_SCAN_LIMIT}-row candidate cap; the visibility filter for these rows is fail-closed and ` + - 'may omit visible rows — scope the query by object_name and record_id', + `[audit] ${gate.seam}: the bounded probe's candidate pre-scan ` + + `(${restrictTo ? restrictTo.why : 'the whole read'}) hit the ${PARENT_GATE_SCAN_LIMIT}-row cap; the ` + + 'visibility filter for these rows is fail-closed and may omit visible rows — scope the query by ' + + 'object_name and record_id', ); } From fff15afe150d18d24a9239214d2c1cde224a1a2c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 13:42:52 +0000 Subject: [PATCH 03/10] =?UTF-8?q?test(plugin-audit):=20the=20leaf=20path?= =?UTF-8?q?=20through=20a=20real=20engine=20and=20SQL=20driver=20=E2=80=94?= =?UTF-8?q?=20past=20the=20window,=20a=20true=20total,=20D5's=20classes=20?= =?UTF-8?q?on=20the=20probe,=20a=20refusing=20driver's=20read=20fails?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- ...arent-record-read-leaf.integration.test.ts | 446 ++++++++++++++++++ 1 file changed, 446 insertions(+) create mode 100644 packages/plugins/plugin-audit/src/parent-record-read-leaf.integration.test.ts 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 new file mode 100644 index 00000000000..9a573e5862d --- /dev/null +++ b/packages/plugins/plugin-audit/src/parent-record-read-leaf.integration.test.ts @@ -0,0 +1,446 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [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. + * + * ## Why a real engine, a real driver and the real plugin + * + * The leaf is composed by the engine at its executor boundary (ADR-0139 D2) + * and evaluated by the driver as one subquery against the parent's table (D3). + * Neither half exists in a stand-in engine, so what the gate emits is only + * measured here: `AuditPlugin` mounts both gates at `kernel:ready`, the rows + * are stored by `SqliteWasmDriver`, and every read goes through the real + * middleware chain. + * + * ## The one stand-in, and why it is the smallest honest one + * + * plugin-security. A leaf is composed from the security service's + * `canReadObject` and `getReadFilter`; a probe reads the parent as the caller + * through the engine middleware. plugin-security keeps those two answers in + * parity (`getReadFilter`'s contract: "the same filter the engine middleware + * ANDs into every find"), and so does the stand-in: one rule per parent + * object, served both as `getReadFilter` and by one GLOBAL engine middleware + * — global as plugin-security's is, so it narrows no object by name and makes + * no parent one of D5's middleware-gated class: + * + * - `e5_doc`: a member reads the records it owns; the admin reads them all; + * - `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. + * + * `getReadableFields` is deliberately absent, so the withheld-update rule + * (`activity-field-redaction.ts`, ADR-0139 E6's) stays inert and what is + * measured is this gate alone; that rule's own bound is E6's card. + * + * Bulk rows are written through the engine as the system — the `e5_doc` + * records through the real CRUD mirror (its `created` activity row and its + * `create` ledger row), the ledger's rows about no record directly, as their + * producers write them. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectKernel, findMisplacedReadableIds, refuseReadableIdsOnce } 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'; + +const ACTIVITY = 'sys_activity'; +const LEDGER = 'sys_audit_log'; +/** Owner-scoped: a member reads its own records. */ +const DOC = 'e5_doc'; +/** No member may read it. */ +const VAULT = 'e5_vault'; +/** Narrowed by an object-scoped read middleware: D5's second class. */ +const GATED = 'e5_gated'; +/** Bound to another datasource: D5's third class. */ +const FAR = 'e5_far'; +const HARNESS_PACKAGE = 'com.objectstack.audit.test.parent-record-leaf'; + +const SYS = { isSystem: true } as const; +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 HOLDER = { userId: 'u_holder', tenantId: 'org_1', positions: ['org_member'], systemPermissions: [LEDGER_AUDIT_CAPABILITY] }; +const DELEGATED_A = { ...MEMBER_A, onBehalfOf: { userId: 'u_delegator' } }; + +/** Past the 2,000-row candidate window the leaf replaces. */ +const DOCS_OF_A = 2_300; +const DOCS_OF_B = 300; +/** The ledger's rows about no record, also past the window. */ +const ROWS_ABOUT_NO_RECORD = 2_100; +const BATCH = 200; + +const READ_OPS = new Set(['find', 'findOne', 'count', 'aggregate']); +const isAdmin = (context: any) => context?.userId === ADMIN.userId; + +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; +} +const mayReadObject = (object: string, context: any) => context?.isSystem || object !== VAULT || isAdmin(context); + +/** Reads of a parent object as a caller (not the system), by object: the probe's footprint. */ +const callerReads: Array<{ object: string; operation: string; userId?: string }> = []; +/** Direct reads of the vault the stand-in refused. */ +const refusals: Array<{ object: string; userId?: string }> = []; + +const security = { + async canReadObject(object: string, context?: any): Promise { + return mayReadObject(object, context); + }, + async getReadFilter(object: string, context?: any): Promise | undefined> { + return readScopeOf(object, context); + }, +}; + +class WarehouseDriver extends SqliteWasmDriver { + public override readonly name: string = 'warehouse'; +} + +const objectOf = (name: string, extra: Record = {}) => ({ + name, + label: name, + fields: { + name: { name: 'name', label: 'Name', type: 'text' as const }, + owner: { name: 'owner', label: 'Owner', type: 'text' as const }, + }, + ...extra, +}); + +async function bootKernel(driver: SqliteWasmDriver) { + const kernel = new ObjectKernel({ logger: { level: 'silent' } }); + await kernel.use(new ObjectQLPlugin()); + await kernel.use(new AuditPlugin()); + await kernel.use({ + name: 'test.security-stand-in', + version: '0.0.0', + init: async (ctx: any) => ctx.registerService('security', security), + start: async () => {}, + } as any); + await kernel.bootstrap(); + const engine = kernel.getService('objectql'); + await driver.connect(); + engine.registerDriver(driver, true); + return { kernel, engine }; +} + +/** plugin-security's middleware, as the stand-in plays it: global, the same rule as `getReadFilter`. */ +function installSecurityMiddleware(engine: ObjectQL): void { + engine.registerMiddleware(async (op: any, next: () => Promise) => { + if (READ_OPS.has(op.operation) && op.context && !op.context.isSystem && op.object !== ACTIVITY && op.object !== LEDGER) { + callerReads.push({ object: op.object, operation: op.operation, userId: op.context.userId }); + if (!mayReadObject(op.object, op.context)) { + refusals.push({ object: op.object, userId: op.context.userId }); + throw Object.assign(new Error(`[Security] Access denied: operation '${op.operation}' on object '${op.object}'`), { + code: 'PERMISSION_DENIED', + statusCode: 403, + }); + } + const scope = readScopeOf(op.object, op.context); + if (scope && op.ast) op.ast.where = op.ast.where ? { $and: [op.ast.where, scope] } : scope; + } + return next(); + }); +} + +async function insertInBatches(engine: ObjectQL, object: string, rows: Row[]): Promise { + for (let i = 0; i < rows.length; i += BATCH) { + await engine.insert(object, rows.slice(i, i + BATCH) as any, { context: SYS }); + } +} + +describe('[ADR-0139 E5] the parent-record read gates on the leaf path', () => { + let kernel: ObjectKernel; + let engine: ObjectQL; + /** The WHERE each gated read reached the driver boundary with, by object. */ + const seenWhere: Record = { [ACTIVITY]: [], [LEDGER]: [] }; + const ids: Record = {}; + + /** Every row at rest, and the ground truth of who may read which, judged by the stand-in's own rules. */ + const truth = async (object: string, reader: any): Promise => { + const atRest: Row[] = await engine.find(object, { limit: 100_000, context: SYS }); + const live = new Map>(); + for (const parent of [DOC, VAULT, GATED, FAR]) { + const records: Row[] = await engine.find(parent, { limit: 100_000, context: SYS }); + live.set(parent, new Map(records.map((r) => [String(r.id), r]))); + } + const holder = Array.isArray(reader?.systemPermissions) && reader.systemPermissions.includes(LEDGER_AUDIT_CAPABILITY); + return atRest + .filter((row) => { + if (object === LEDGER && holder) return true; + if (object === LEDGER && (row.record_id === null || row.record_id === '') && !['create', 'update', 'delete', 'read'].includes(row.action)) { + return typeof row.action === 'string' && row.action !== ''; + } + const parent = live.get(row.object_name)?.get(String(row.record_id)); + if (!parent) return false; + if (isAdmin(reader)) return true; + if (row.object_name === VAULT) return false; + if (row.object_name === DOC || row.object_name === GATED) return parent.owner === reader.userId; + return true; + }) + .map((r) => String(r.id)) + .sort(); + }; + const served = async (object: string, context: any, extra: Record = {}) => + ((await engine.find(object, { limit: 100_000, ...extra, context })) as Row[]).map((r) => String(r.id)).sort(); + + beforeAll(async () => { + ({ kernel, engine } = await bootKernel(new SqliteWasmDriver({ filename: ':memory:' }))); + const warehouse = new WarehouseDriver({ filename: ':memory:' }); + await warehouse.connect(); + engine.registerDriver(warehouse); + engine.registry.registerObject(objectOf(DOC) as any, HARNESS_PACKAGE); + engine.registry.registerObject(objectOf(VAULT) as any, HARNESS_PACKAGE); + engine.registry.registerObject(objectOf(GATED) as any, HARNESS_PACKAGE); + engine.registry.registerObject(objectOf(FAR, { datasource: 'warehouse' }) as any, HARNESS_PACKAGE); + await engine.syncSchemas(); + installSecurityMiddleware(engine); + // D5's second class: an object-scoped read middleware narrows the gated parent. + engine.registerMiddleware(async (op: any, next: () => Promise) => { + if (READ_OPS.has(op.operation) && op.context && !op.context.isSystem && !isAdmin(op.context) && op.ast) { + const mine = { owner: op.context.userId ?? '__nobody__' }; + op.ast.where = op.ast.where ? { $and: [op.ast.where, mine] } : mine; + } + return next(); + }, { object: GATED }); + // Registered last, so it sees each gated read's WHERE as the gates left it. + for (const object of [ACTIVITY, LEDGER]) { + engine.registerMiddleware(async (op: any, next: () => Promise) => { + if (READ_OPS.has(op.operation) && op.context && !op.context.isSystem) seenWhere[object].push(op.ast?.where); + return next(); + }, { object }); + } + + // The bulk, through the real CRUD mirror: one activity row and one ledger row per record. + const docs: Row[] = []; + for (let i = 0; i < DOCS_OF_A; i++) docs.push({ name: `a${i}`, owner: MEMBER_A.userId }); + for (let i = 0; i < DOCS_OF_B; i++) docs.push({ name: `b${i}`, owner: MEMBER_B.userId }); + await insertInBatches(engine, DOC, docs); + ids.gone = (await engine.insert(DOC, { name: 'gone', owner: MEMBER_A.userId }, { context: SYS })).id; + await engine.delete(DOC, { where: { id: ids.gone }, context: SYS }); + ids.vault = (await engine.insert(VAULT, { name: 'vault' }, { context: SYS })).id; + ids.gatedA = (await engine.insert(GATED, { name: 'gated a', owner: MEMBER_A.userId }, { context: SYS })).id; + ids.gatedB = (await engine.insert(GATED, { name: 'gated b', owner: MEMBER_B.userId }, { context: SYS })).id; + ids.far = (await engine.insert(FAR, { name: 'far' }, { context: SYS })).id; + + // The ledger's rows about no record, as their producers write them (the settings service's config_change). + const now = new Date().toISOString(); + const outside: Row[] = []; + for (let i = 0; i < ROWS_ABOUT_NO_RECORD; i++) { + outside.push({ action: 'config_change', object_name: 'sys_setting', record_id: null, actor: 'svc:fixture', created_at: now }); + } + await insertInBatches(engine, LEDGER, outside); + }, 120_000); + + afterAll(async () => { + if (kernel) await Promise.race([kernel.shutdown(), new Promise((r) => setTimeout(r, 10_000))]); + }, 30_000); + + it('control: the fixture is past the window, and every row came from its producer', async () => { + const activity: Row[] = await engine.find(ACTIVITY, { limit: 100_000, context: SYS }); + expect(activity.length).toBeGreaterThan(PARENT_GATE_SCAN_LIMIT); + expect(activity.filter((r) => r.object_name === DOC && r.type === 'created').length).toBe(DOCS_OF_A + DOCS_OF_B + 1); + expect(activity.some((r) => r.object_name === DOC && r.record_id === ids.gone && r.type === 'deleted')).toBe(true); + expect((await truth(ACTIVITY, MEMBER_A)).length).toBeGreaterThan(PARENT_GATE_SCAN_LIMIT); + const ledger: Row[] = await engine.find(LEDGER, { limit: 100_000, context: SYS }); + expect(ledger.filter((r) => r.action === 'config_change').length).toBe(ROWS_ABOUT_NO_RECORD); + }); + + it('a member is served every activity row it may read past the old window, and the total agrees', async () => { + const expected = await truth(ACTIVITY, MEMBER_A); + expect(await served(ACTIVITY, MEMBER_A)).toEqual(expected); + expect(await engine.count(ACTIVITY, {}, { context: MEMBER_A })).toBe(expected.length); + const expectedB = await truth(ACTIVITY, MEMBER_B); + expect(await served(ACTIVITY, MEMBER_B)).toEqual(expectedB); + }); + + it('an administrator’s site-wide read (newest first, 20) is served 20 rows and a true total, not the window', async () => { + const page: Row[] = await engine.find(ACTIVITY, { + orderBy: [{ field: 'timestamp', order: 'desc' }], + limit: 20, + context: ADMIN, + }); + expect(page).toHaveLength(20); + const total = await engine.count(ACTIVITY, {}, { context: ADMIN }); + expect(total).toBe((await truth(ACTIVITY, ADMIN)).length); + expect(total).toBeGreaterThan(PARENT_GATE_SCAN_LIMIT); + }); + + it('no row is served whose parent the reader may not read, or that no longer exists', async () => { + const all: Row[] = await engine.find(ACTIVITY, { limit: 100_000, context: SYS }); + const byId = new Map(all.map((r) => [String(r.id), r])); + for (const id of await served(ACTIVITY, MEMBER_A)) { + const row = byId.get(id)!; + expect(row.object_name).not.toBe(VAULT); + expect(row.record_id).not.toBe(ids.gone); + if (row.object_name === DOC) expect(byId.get(id)).toBeTruthy(); + } + expect((await served(ACTIVITY, ADMIN)).some((id) => byId.get(id)!.record_id === ids.gone)).toBe(false); + expect((await served(ACTIVITY, ADMIN)).some((id) => byId.get(id)!.object_name === VAULT)).toBe(true); + }); + + it('the leaf reads no parent as the caller: a member’s broad read logs no refusal and reads neither the doc nor the vault', async () => { + callerReads.length = 0; + refusals.length = 0; + await served(ACTIVITY, MEMBER_A); + await engine.count(ACTIVITY, {}, { context: MEMBER_A }); + expect(refusals).toEqual([]); + expect(callerReads.filter((r) => r.object === DOC || r.object === VAULT)).toEqual([]); + }); + + it('one leaf per enumerated parent; D5’s middleware-gated and federated parents are probed, beside them', async () => { + seenWhere[ACTIVITY].length = 0; + await served(ACTIVITY, MEMBER_A); + const where = seenWhere[ACTIVITY][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).toEqual(expect.arrayContaining([DOC, VAULT])); + expect(leaves).not.toContain(GATED); + expect(leaves).not.toContain(FAR); + expect(leaves).not.toContain(ACTIVITY); + const probed = branches.filter((b) => Array.isArray(b?.record_id?.$in)); + expect(probed).toEqual(expect.arrayContaining([ + { object_name: GATED, record_id: { $in: [ids.gatedA] } }, + { object_name: FAR, record_id: { $in: [ids.far] } }, + ])); + }); + + 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('ledger: a non-holder is served every row about a readable record and every row about no record, past the window', async () => { + const expected = await truth(LEDGER, MEMBER_A); + expect(expected.length).toBeGreaterThan(DOCS_OF_A + ROWS_ABOUT_NO_RECORD - 1); + expect(await served(LEDGER, MEMBER_A)).toEqual(expected); + expect(await engine.count(LEDGER, {}, { context: MEMBER_A })).toBe(expected.length); + }); + + it('ledger: the outside class is a predicate — every row about no record, none named by a stored id', async () => { + seenWhere[LEDGER].length = 0; + const where = { action: 'config_change' }; + expect(await engine.count(LEDGER, { where }, { context: MEMBER_B })).toBe(ROWS_ABOUT_NO_RECORD); + const seen = JSON.stringify(seenWhere[LEDGER][0]); + expect(seen).not.toContain('"id":{"$in"'); + }); + + it('ledger: the audit capability’s holder is not narrowed — the gate writes nothing for it', async () => { + seenWhere[LEDGER].length = 0; + const expected = await truth(LEDGER, HOLDER); + expect(await served(LEDGER, HOLDER)).toEqual(expected); + expect(await engine.count(LEDGER, {}, { context: HOLDER })).toBe(expected.length); + expect(seenWhere[LEDGER].every((w) => w === undefined)).toBe(true); + // ...and the activity stream's gate still narrows it. + expect(await served(ACTIVITY, HOLDER)).toEqual(await truth(ACTIVITY, HOLDER)); + }); + + it('[#22788 note 1] branch count and query time on this fixture, recorded', async () => { + seenWhere[ACTIVITY].length = 0; + const time = async (run: () => Promise) => { + const t0 = performance.now(); + await run(); + return Math.round((performance.now() - t0) * 10) / 10; + }; + const reading = { + rowsAtRest: (await engine.find(ACTIVITY, { limit: 100_000, context: SYS })).length, + leafMs: { + memberFind: await time(() => served(ACTIVITY, MEMBER_A)), + 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 })), + }, + branches: 0, + leaves: 0, + }; + const where = seenWhere[ACTIVITY][0] as any; + reading.branches = where.$or.length; + reading.leaves = where.$or.filter((b: any) => b?.record_id?.$in && !Array.isArray(b.record_id.$in)).length; + // A reading, never a verdict on a duration (AGENTS.md). + console.log(`E5-READING ${JSON.stringify(reading)}`); + expect(reading.leaves).toBeGreaterThan(0); + }); +}); + +/** A driver that does not compile the composed leaf, refusing it with D3's envelope, as `driver-mongodb` does. */ +class RefusingDriver extends SqliteWasmDriver { + private readonly refusalState = { logged: false }; + private refuse(query: any): void { + const site = findMisplacedReadableIds(query?.where, false); + if (site) throw refuseReadableIdsOnce('driver-e5-refusing', site, undefined, this.refusalState, 'this test driver compiles no subquery'); + } + override async find(object: string, query: any, options?: any) { + this.refuse(query); + return super.find(object, query, options); + } + override async findOne(object: string, query: any, options?: any) { + this.refuse(query); + return super.findOne(object, query, options); + } + override async count(object: string, query?: any, options?: any) { + this.refuse(query); + return super.count(object, query, options); + } +} + +describe('[ADR-0139 D3] on a driver that refuses the leaf, the read fails and says why', () => { + let kernel: ObjectKernel; + let engine: ObjectQL; + + beforeAll(async () => { + ({ kernel, engine } = await bootKernel(new RefusingDriver({ filename: ':memory:' }))); + engine.registry.registerObject(objectOf(DOC) as any, HARNESS_PACKAGE); + await engine.syncSchemas(); + await engine.insert(DOC, { name: 'one', owner: MEMBER_A.userId }, { context: SYS }); + }, 120_000); + + afterAll(async () => { + if (kernel) await Promise.race([kernel.shutdown(), new Promise((r) => setTimeout(r, 10_000))]); + }, 30_000); + + it('control: the rows are there, and a system read is served them', async () => { + expect((await engine.find(ACTIVITY, { context: SYS })).length).toBeGreaterThan(0); + expect((await engine.find(LEDGER, { context: SYS })).length).toBeGreaterThan(0); + }); + + for (const object of [ACTIVITY, LEDGER]) { + it(`${object}: a member’s list and total are refused with D3’s envelope — never served as the deny-all sentinel`, async () => { + for (const read of [ + () => engine.find(object, { context: MEMBER_A }), + () => engine.count(object, {}, { context: MEMBER_A }), + ]) { + const outcome = await read().then( + (value) => ({ value }), + (error) => ({ error }), + ); + expect(outcome).toHaveProperty('error'); + const error = (outcome as { error: any }).error; + expect({ code: error.code, status: error.status }).toEqual({ code: 'INTERNAL_ERROR', status: 500 }); + expect(String(error.message)).toContain('[driver-e5-refusing]'); + } + }); + } +}); From 9e2ebf748a2a5f7fed108f82904a1f4addae05d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 14:03:19 +0000 Subject: [PATCH 04/10] =?UTF-8?q?fix(plugin-audit):=20a=20leaf=20needs=20t?= =?UTF-8?q?he=20object-level=20admission=20=E2=80=94=20without=20canReadOb?= =?UTF-8?q?ject=20the=20whole=20read=20is=20the=20bounded=20probe?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The engine admits a leaf's parent by explain() when the security service answers no canReadObject(), and explain() refuses a SYSTEM context. The withheld-update rule re-reads this gate's WHERE as SYSTEM, so a leaf there selected nothing and that rule served update rows it never judged (measured on the real security plugin with canReadObject withdrawn). Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- .../src/activity-read-visibility.test.ts | 38 +++++++- .../src/audit-log-read-visibility.test.ts | 2 +- ...-record-read-admission.integration.test.ts | 31 +++---- .../src/parent-record-read-gate.ts | 89 ++++++++++++++----- 4 files changed, 117 insertions(+), 43 deletions(-) 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 69274b766f8..7911536893d 100644 --- a/packages/plugins/plugin-audit/src/activity-read-visibility.test.ts +++ b/packages/plugins/plugin-audit/src/activity-read-visibility.test.ts @@ -343,7 +343,8 @@ describe('installActivityReadVisibility', () => { it('an engine that cannot classify a parent sends the whole read to the probe, and says so once', async () => { const logger = silentLogger(); - const { mw, calls } = install({ scan: () => [{ object_name: 'crm_case', record_id: 'c1' }], readable: { crm_case: ['c1'] }, logger }); + const admission = () => ({ canReadObject: async () => true }); + const { mw, calls } = install({ scan: () => [{ object_name: 'crm_case', record_id: 'c1' }], readable: { crm_case: ['c1'] }, logger, admission }); await read(mw); await read(mw); expect(calls.scans).toHaveLength(2); @@ -387,7 +388,13 @@ function installClassifying(opts: { hasObjectMiddleware: (object: string) => (opts.middleware ?? []).includes(object), resolveEffectiveDatasource: (object: string) => opts.datasource?.[object], }; - installActivityReadVisibility(engine, opts.logger ?? silentLogger(), opts.admission ?? (() => undefined)); + // A security service that answers `canReadObject` unless the case says otherwise: + // without it the gate writes no leaf (see the `no-object-admission` case). + installActivityReadVisibility( + engine, + opts.logger ?? silentLogger(), + opts.admission ?? (() => ({ canReadObject: async () => true })), + ); return { mw, calls }; } @@ -520,6 +527,33 @@ describe('[ADR-0139 D5] three classes keep the bounded probe, by name, chosen be expect(calls.probes[0].context).toMatchObject({ userId: 'u_agent', onBehalfOf }); }); + it('no object-level admission (no canReadObject, or no security service): no leaf, the whole read is the probe, said once', async () => { + // A leaf would then be admitted by explain(), which refuses the SYSTEM + // re-read of this WHERE the withheld-update rule makes (gate module header). + const sources: ParentObjectReadAdmissionSource[] = [ + () => undefined, + () => ({}), + () => { + throw new Error("service 'security' not registered"); + }, + ]; + for (const admission of sources) { + const logger = silentLogger(); + const { mw, calls } = installClassifying({ + objects: ['crm_case', 'crm_deal'], + scan: () => [{ object_name: 'crm_case', record_id: 'c1' }], + readable: { crm_case: ['c1'] }, + admission, + logger, + }); + expect((await read(mw)).where).toEqual({ object_name: 'crm_case', record_id: { $in: ['c1'] } }); + await read(mw); + expect(calls.scans.map((s) => s.where)).toEqual([{}, {}]); + const said = logger.warn.mock.calls.filter(([line]) => String(line).includes('answers no canReadObject()')); + expect(said).toHaveLength(1); + } + }); + it('the probe’s cap warning names the class it was probing for', async () => { const logger = silentLogger(); const rows = Array.from({ length: 2000 }, (_, i) => ({ object_name: 'sys_attachment', record_id: `f${i}` })); diff --git a/packages/plugins/plugin-audit/src/audit-log-read-visibility.test.ts b/packages/plugins/plugin-audit/src/audit-log-read-visibility.test.ts index c6c4fe6eed6..1f34de88ee6 100644 --- a/packages/plugins/plugin-audit/src/audit-log-read-visibility.test.ts +++ b/packages/plugins/plugin-audit/src/audit-log-read-visibility.test.ts @@ -304,7 +304,7 @@ function installClassifying(objects: string[]) { hasObjectMiddleware: () => false, resolveEffectiveDatasource: () => undefined, }; - installAuditLogReadVisibility(engine, silentLogger(), () => undefined); + installAuditLogReadVisibility(engine, silentLogger(), () => ({ canReadObject: async () => true })); return { mw, calls }; } diff --git a/packages/plugins/plugin-audit/src/parent-record-read-admission.integration.test.ts b/packages/plugins/plugin-audit/src/parent-record-read-admission.integration.test.ts index fc49aa9786f..be695280d81 100644 --- a/packages/plugins/plugin-audit/src/parent-record-read-admission.integration.test.ts +++ b/packages/plugins/plugin-audit/src/parent-record-read-admission.integration.test.ts @@ -29,15 +29,15 @@ * optional-method state — so every case compares the gate WITH the admission * against the gate as it reads without one, on the same kernel and rows. * - * [ADR-0139 E5] Two readers of that admission now. The vault carries an - * object-scoped read middleware (the stand-in for plugin-security's - * refusal), so it is one of D5's classes and the gate reads it through its - * bounded probe, which asks `canReadObject` first — the behaviour pinned here. - * Every other parent object is a `ReadableIds` leaf, which the ENGINE - * composes from `canReadObject` (or, when that is withdrawn, `explain`) and - * `getReadFilter`, without reading the parent as the caller; the stand-in - * answers both halves, so withdrawing `canReadObject` withdraws the probe's - * admission alone. + * [ADR-0139 E5] Two readers of that admission now. With it, the vault — which + * carries an object-scoped read middleware (the stand-in for plugin-security's + * refusal), one of D5's classes — is read through the gate's bounded probe, + * which asks `canReadObject` first: the behaviour pinned here. Every other + * parent object is a `ReadableIds` leaf, which the ENGINE composes from + * `canReadObject` and `getReadFilter` without reading the parent as the + * caller. Without it, the gate writes no leaf at all and reads every parent + * through the probe, as the caller (`parent-record-read-gate.ts`: a leaf needs + * the object-level admission) — the no-admission comparison below. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -73,25 +73,20 @@ const refusals: Array<{ object: string; operation: string; userId?: string }> = /** The object-level admission's questions: object and the envelope it was asked with. */ const asked: Array<{ object: string; context: any }> = []; -/** The admission's verdict, unrecorded. */ -const admits = (object: string, context?: any) => object !== VAULT || isAdmin(context); - /** * The stand-in `security` service; `canReadObject` is withdrawn by deleting it. - * `explain` (the engine's fallback when it is withdrawn) and `getReadFilter` - * (no row restriction) are the halves the engine composes a leaf from. + * `getReadFilter` (no row restriction) is the other half the engine composes a + * leaf from. */ const security: { canReadObject?: (object: string, context?: any) => Promise; - explain: (request: { object: string }, context?: any) => Promise<{ allowed: boolean }>; getReadFilter: (object: string, context?: any) => Promise | undefined>; } = { - explain: async ({ object }, context) => ({ allowed: admits(object, context) }), getReadFilter: async () => undefined, }; const admit = async (object: string, context?: any) => { asked.push({ object, context }); - return admits(object, context); + return object !== VAULT || isAdmin(context); }; function securityStandIn(): Plugin { @@ -213,7 +208,7 @@ describe('parent-record read gates: the object-level read admission', () => { expect(a.context).not.toHaveProperty('__readScope'); } // The board is a ReadableIds leaf: the engine asks about it, once per - // read, while composing — and never reads it as the member. + // read, while composing it. expect(after.asked.filter((a) => a.object === OPEN)).toHaveLength(2); }); } 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 211bc4e1d41..401e22bf757 100644 --- a/packages/plugins/plugin-audit/src/parent-record-read-gate.ts +++ b/packages/plugins/plugin-audit/src/parent-record-read-gate.ts @@ -61,10 +61,23 @@ * datasource, that is every parent of a ledger the lifecycle split routes * there. * - * An engine that cannot answer those questions (no registry view, no - * `hasObjectMiddleware`, no `resolveEffectiveDatasource` — each optional on - * its contract) cannot tell a composable parent from a refused one, so the - * whole read is the probe there too, and the gate says so once. + * Two more states send the WHOLE read to the probe, up front, and the gate + * says so once for each: + * + * - an engine that cannot answer those questions (no registry view, no + * `hasObjectMiddleware`, no `resolveEffectiveDatasource` — each optional on + * its contract) cannot tell a composable parent from a refused one; + * - a security service that answers no `canReadObject` (optional on + * `ISecurityService`), or no security service at all. The engine then + * admits a leaf's parent by `explain`, which answers a SYSTEM context "not + * allowed" (it explains a principal's access) where `canReadObject` admits + * it. This stream's withheld-update rule (`activity-field-redaction.ts`) + * re-reads the WHERE this gate writes as SYSTEM, so a leaf there would + * select nothing, and that rule would serve the update rows it never saw — + * rows whose every recorded change is withheld from the reader (measured + * on the real security plugin with `canReadObject` withdrawn). The probe's + * literal id lists mean the same thing under every context. With no + * security service, a leaf would select nothing for any caller. * * The probe, for the parents it is given (or for every row, for a whole-read * probe): pre-scan the parent pairs the query would touch under SYSTEM @@ -208,7 +221,10 @@ export type ParentProbeClass = | 'federated-parent' /** The engine exposes no registry view, `hasObjectMiddleware` or * `resolveEffectiveDatasource`: the whole read is the probe. */ - | 'engine-cannot-classify'; + | 'engine-cannot-classify' + /** The security service answers no `canReadObject`, or none is composed: + * the whole read is the probe (module header). */ + | 'no-object-admission'; /** * How one read reaches its parents: the parent objects written as leaves, and @@ -216,7 +232,7 @@ export type ParentProbeClass = * the whole read to the probe. */ export type ParentReadPlan = - | { probeWholeRead: 'delegated-caller' | 'engine-cannot-classify' } + | { probeWholeRead: 'delegated-caller' | 'engine-cannot-classify' | 'no-object-admission' } | { probeWholeRead: null; /** One `ReadableIds` leaf each, in name order. */ @@ -350,16 +366,20 @@ export function enumerateParentObjects(engine: ParentRecordGateEngine, gate: Par /** * [ADR-0139 D4, D5] How one read of `gate.object` by `context` reaches its - * parents. Reads the context, the registry, the middleware registrations and - * the datasource bindings only — it asks no security service and reads no - * row, so it decides the probe up front, never by catching a refusal. + * parents. Reads the context, whether the security service answers + * `canReadObject` (`objectAdmission`, resolved by the caller), 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. */ export function planParentReads( engine: ParentRecordGateEngine, gate: ParentRecordGate, context: CommentReadMiddlewareCtx['context'], + objectAdmission: boolean, ): ParentReadPlan { if (isDelegatedCaller(context)) return { probeWholeRead: 'delegated-caller' }; + if (!objectAdmission) return { probeWholeRead: 'no-object-admission' }; const parents = enumerateParentObjects(engine, gate); const hasObjectMiddleware = engine.hasObjectMiddleware; const datasourceOf = engine.resolveEffectiveDatasource; @@ -381,22 +401,45 @@ export function planParentReads( return { probeWholeRead: null, leaves, probed }; } -/** The engines a gate has already said "cannot classify" for, per seam. */ -const unclassifiableReported = new WeakMap>(); +/** The whole-read probe states a gate has already reported, per engine and seam. */ +const wholeReadProbeReported = new WeakMap>(); + +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', + 'no-object-admission': + 'the security service answers no canReadObject() (or none is registered), so a ReadableIds leaf would be ' + + 'admitted by explain(), which refuses the SYSTEM re-read of this WHERE that the withheld-update rule makes', +}; -function reportUnclassifiableOnce(engine: object, logger: CommentAccessLogger, gate: ParentRecordGate): void { - let seams = unclassifiableReported.get(engine); - if (!seams) unclassifiableReported.set(engine, (seams = new Set())); - if (seams.has(gate.seam)) return; - seams.add(gate.seam); +function reportWholeReadProbeOnce( + engine: object, + logger: CommentAccessLogger, + gate: ParentRecordGate, + why: 'engine-cannot-classify' | 'no-object-admission', +): void { + let reported = wholeReadProbeReported.get(engine); + if (!reported) wholeReadProbeReported.set(engine, (reported = new Set())); + const key = `${gate.seam} :: ${why}`; + if (reported.has(key)) return; + reported.add(key); logger.warn( - `[audit] ${gate.seam}: the engine exposes no registry view, hasObjectMiddleware() or ` + - 'resolveEffectiveDatasource(), so it cannot say which parent objects a ReadableIds leaf can be composed ' + - `for: every read is answered by the bounded probe (at most ${PARENT_GATE_SCAN_LIMIT} candidate rows, ` + - 'fail-closed). Reported once for this engine.', + `[audit] ${gate.seam}: ${WHOLE_READ_PROBE_WHY[why]}: every read is answered by the bounded probe (at most ` + + `${PARENT_GATE_SCAN_LIMIT} candidate rows, fail-closed). Reported once for this engine.`, ); } +/** Whether the security service `getAdmission` resolves answers `canReadObject`. */ +function hasObjectAdmission(getAdmission: ParentObjectReadAdmissionSource): boolean { + try { + return typeof getAdmission()?.canReadObject === 'function'; + } catch { + // `getService` throws on an empty slot: no security service is composed. + return false; + } +} + /** * The parent objects of `objects` the caller may read at all, by the security * service's object-level read admission — or `null` when none is composed (no @@ -564,8 +607,10 @@ export async function computeParentRecordFilter( if (gate.exemptCapability && callerHoldsCapability(ctx.context, gate.exemptCapability)) return null; // 1. Leaf or probe, per parent object, before any branch is written. - const plan = planParentReads(engine, gate, ctx.context); - if (plan.probeWholeRead === 'engine-cannot-classify') reportUnclassifiableOnce(engine, logger, gate); + const plan = planParentReads(engine, gate, ctx.context, hasObjectAdmission(getAdmission)); + if (plan.probeWholeRead === 'engine-cannot-classify' || plan.probeWholeRead === 'no-object-admission') { + reportWholeReadProbeOnce(engine, logger, gate, plan.probeWholeRead); + } const branches: Array> = []; // 2. One ReadableIds leaf per composable parent object (ADR-0139 D4). From 6cb81f3b889079464376e8e8bcd1974d969578d8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 14:10:47 +0000 Subject: [PATCH 05/10] test(dogfood): a ledger non-holder's broad read is exactly the rows about the record it can open, newest first past the old window included Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- ...audit-log-audit-capability.dogfood.test.ts | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/qa/dogfood/test/audit-log-audit-capability.dogfood.test.ts b/packages/qa/dogfood/test/audit-log-audit-capability.dogfood.test.ts index c53721fb665..0849c218dc6 100644 --- a/packages/qa/dogfood/test/audit-log-audit-capability.dogfood.test.ts +++ b/packages/qa/dogfood/test/audit-log-audit-capability.dogfood.test.ts @@ -28,7 +28,7 @@ // - a private record the writer owns, which neither member can open; // - a third member's second session, signed out (its sign-in row about an // ended session, and its sign-out row); -// - a bulk of rows past the gate's pre-scan bound, inserted as the system +// - a bulk of rows past the old pre-scan bound, inserted as the system // (the writers insert as the system too), about records that do not exist. // // `@objectstack/plugin-audit` resolves through its BUILT output here (a @@ -54,7 +54,7 @@ const ACTIVITY = 'sys_activity'; const OBJ = 'alc_item'; const PRIVATE = 'alc_private'; const SYS = { isSystem: true } as const; -/** The parent-record gate's pre-scan bound (`PARENT_GATE_SCAN_LIMIT` in plugin-audit). */ +/** The parent-record gate's old pre-scan bound, the bounded probe's still (`PARENT_GATE_SCAN_LIMIT` in plugin-audit). */ const SCAN_BOUND = 2_000; const BULK = SCAN_BOUND + 50; const PASSWORD = 'Alc-Fixture-Pass-21260'; @@ -146,8 +146,8 @@ describe('[#21260] the ledger audit capability exempts its holder from the paren }; const servedById = async (who: keyof typeof token, rowId: string) => (await stack.apiAs(token[who], 'GET', `/data/${LEDGER}/${rowId}`)).status; - const servedTotal = async (who: keyof typeof token, where: Record) => { - const res = await stack.apiAs(token[who], 'GET', `/data/${LEDGER}?$filter=${filterOf(where)}&limit=1`); + const servedTotal = async (who: keyof typeof token, where: Record, sort?: string) => { + const res = await stack.apiAs(token[who], 'GET', `/data/${LEDGER}?$filter=${filterOf(where)}&limit=1${sort ? `&sort=${sort}` : ''}`); expect(res.status).toBe(200); return Number(((await res.json()) as any).total); }; @@ -341,14 +341,21 @@ describe('[#21260] the ledger audit capability exempts its holder from the paren expect(await servedList('admin', ACTIVITY, { object_name: OBJ, record_id: id.gone })).toEqual([]); }); - it('non-holder: gets exactly the parent-record gate — none of these rows, and a broad read bounded', async () => { + it('non-holder: gets exactly the parent-record gate — none of these rows, and a broad read is exactly the rows about the record it can open', async () => { expect(await servedList('reader', LEDGER, { object_name: OBJ, record_id: id.gone })).toEqual([]); for (const rowId of [rowIds.goneDelete, rowIds.signOut, rowIds.endedSignIn, rowIds.privateCreate]) { expect(await servedById('reader', rowId), `ledger row ${rowId}`).toBe(404); } - const total = await servedTotal('reader', { object_name: OBJ }); - expect(total).toBeLessThan(SCAN_BOUND); - expect(total).toBeLessThan(bulkAtRest); + // [ADR-0139 E5] One ReadableIds leaf per parent object, no candidate + // window: the total is the rows about the live record, whatever the order. + // Newest first, the bulk (about records that do not exist) fills the + // newest 2,000 rows, so the rows about the live record lie past the old + // pre-scan window — which answered none of them. + const exact = (await ledgerAtRest({ object_name: OBJ, record_id: id.live })).length; + expect(exact).toBeGreaterThan(0); + expect(await servedTotal('reader', { object_name: OBJ })).toBe(exact); + expect(await servedTotal('reader', { object_name: OBJ }, '-created_at')).toBe(exact); + expect(bulkAtRest).toBeGreaterThan(SCAN_BOUND); }); it('non-holder control: is served the rows about the record it can open', async () => { From 8b14e7854a3be61c3182652997b0d0c4b7a1c874 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 14:19:58 +0000 Subject: [PATCH 06/10] =?UTF-8?q?test(plugin-audit):=20without=20canReadOb?= =?UTF-8?q?ject=20the=20withheld-update=20rule=20still=20withholds=20?= =?UTF-8?q?=E2=80=94=20the=20gate=20writes=20no=20leaf=20for=20its=20SYSTE?= =?UTF-8?q?M=20re-read?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- ...tivity-withheld-update.integration.test.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts b/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts index 1762f402a50..c4d4d96229a 100644 --- a/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts +++ b/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts @@ -119,6 +119,12 @@ describe('[#21388] an update whose every recorded change is withheld from the re async getReadFilter(): Promise | undefined> { return undefined; }, + // The engine's fallback admission when `canReadObject` is absent. Like + // plugin-security's (measured), it explains a principal's access and + // answers a SYSTEM context "not allowed". + async explain(_request: { object: string }, context?: any): Promise<{ allowed: boolean }> { + return { allowed: !context?.isSystem }; + }, }; const scoped = (object: string, record: string) => ({ object_name: object, record_id: record }); @@ -264,6 +270,23 @@ describe('[#21388] an update whose every recorded change is withheld from the re expect(walked).toEqual((await read(MEMBER)).map((r) => String(r.id))); }); + it('[ADR-0139 E5] with no canReadObject on the service, the row is still withheld: no leaf is written for the rule to re-read as SYSTEM', async () => { + // The rule pre-scans this read's WHERE as SYSTEM. A ReadableIds leaf there + // would be admitted by `explain`, which refuses SYSTEM, so it would select + // nothing, and the rule would serve the rows it never saw. + const own = Object.getOwnPropertyDescriptor(security, 'canReadObject'); + expect(own, 'the double carries canReadObject as its own member').toBeTruthy(); + delete (security as { canReadObject?: unknown }).canReadObject; + try { + const rows = await read(MEMBER); + expect(rows.map((r) => r.id)).not.toContain(rowIds.withheld); + // Armed: the member is still served the rows it is served with the admission. + expect(rows.map((r) => r.id).sort()).toEqual([rowIds.created, rowIds.mixed, rowIds.empty, rowIds.served].sort()); + } finally { + Object.defineProperty(security, 'canReadObject', own!); + } + }); + it('findOne: the withheld row is absent by id for the member and present for the admin', async () => { expect(await engine.findOne(ACTIVITY, { where: { id: rowIds.withheld }, context: MEMBER })).toBeNull(); expect((await engine.findOne(ACTIVITY, { where: { id: rowIds.mixed }, context: MEMBER }))?.id).toBe(rowIds.mixed); From 6d171682d496c6d22551948ea4a2bbd85cdf5da5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 14:34:53 +0000 Subject: [PATCH 07/10] test(dogfood): the non-holder's newest-first ledger page carries the rows about a record the old pre-scan window never reached Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- ...audit-log-audit-capability.dogfood.test.ts | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/packages/qa/dogfood/test/audit-log-audit-capability.dogfood.test.ts b/packages/qa/dogfood/test/audit-log-audit-capability.dogfood.test.ts index 0849c218dc6..8217f488c54 100644 --- a/packages/qa/dogfood/test/audit-log-audit-capability.dogfood.test.ts +++ b/packages/qa/dogfood/test/audit-log-audit-capability.dogfood.test.ts @@ -133,21 +133,21 @@ describe('[#21260] the ledger audit capability exempts its holder from the paren let stack: VerifyStack; let ql: any; const token: Record<'admin' | 'holder' | 'reader' | 'writer', string> = { admin: '', holder: '', reader: '', writer: '' }; - const id = { live: '', gone: '', private: '', endedSession: '' }; + const id = { live: '', early: '', gone: '', private: '', endedSession: '' }; const rowIds = { goneCreate: '', goneDelete: '', privateCreate: '', signOut: '', endedSignIn: '' }; let bulkAtRest = 0; const ledgerAtRest = (where: Record): Promise => ql.find(LEDGER, { where, limit: 5_000, context: { ...SYS } }); - const servedList = async (who: keyof typeof token, object: string, where: Record) => { - const res = await stack.apiAs(token[who], 'GET', `/data/${object}?$filter=${filterOf(where)}&limit=200`); + const servedList = async (who: keyof typeof token, object: string, where: Record, sort?: string) => { + const res = await stack.apiAs(token[who], 'GET', `/data/${object}?$filter=${filterOf(where)}&limit=200${sort ? `&sort=${sort}` : ''}`); expect(res.status).toBe(200); return rowsOf(await res.json()); }; const servedById = async (who: keyof typeof token, rowId: string) => (await stack.apiAs(token[who], 'GET', `/data/${LEDGER}/${rowId}`)).status; - const servedTotal = async (who: keyof typeof token, where: Record, sort?: string) => { - const res = await stack.apiAs(token[who], 'GET', `/data/${LEDGER}?$filter=${filterOf(where)}&limit=1${sort ? `&sort=${sort}` : ''}`); + const servedTotal = async (who: keyof typeof token, where: Record) => { + const res = await stack.apiAs(token[who], 'GET', `/data/${LEDGER}?$filter=${filterOf(where)}&limit=1`); expect(res.status).toBe(200); return Number(((await res.json()) as any).total); }; @@ -199,6 +199,9 @@ describe('[#21260] the ledger audit capability exempts its holder from the paren return String(j.id ?? j.record?.id ?? j.data?.id); }; id.live = await idOf(await stack.apiAs(token.writer, 'POST', `/data/${OBJ}`, { name: 'live' })); + // A readable record nothing touches again: every row about it is older + // than the bulk below. + id.early = await idOf(await stack.apiAs(token.writer, 'POST', `/data/${OBJ}`, { name: 'early' })); id.gone = await idOf(await stack.apiAs(token.writer, 'POST', `/data/${OBJ}`, { name: 'gone', alc_plain: VALUE.plain, alc_withheld: VALUE.withheld, })); @@ -347,14 +350,23 @@ describe('[#21260] the ledger audit capability exempts its holder from the paren expect(await servedById('reader', rowId), `ledger row ${rowId}`).toBe(404); } // [ADR-0139 E5] One ReadableIds leaf per parent object, no candidate - // window: the total is the rows about the live record, whatever the order. - // Newest first, the bulk (about records that do not exist) fills the - // newest 2,000 rows, so the rows about the live record lie past the old - // pre-scan window — which answered none of them. - const exact = (await ledgerAtRest({ object_name: OBJ, record_id: id.live })).length; - expect(exact).toBeGreaterThan(0); - expect(await servedTotal('reader', { object_name: OBJ })).toBe(exact); - expect(await servedTotal('reader', { object_name: OBJ }, '-created_at')).toBe(exact); + // window: the reader is served the rows about the records it can open, + // and the total says so, whatever the order. Newest first, the bulk (about + // records that do not exist, written after the early record) fills the + // newest 2,000 rows, so no row about the early record is in the old + // pre-scan's window — which therefore served none of them. + const readable = new Set([id.live, id.early]); + const atRest = await ledgerAtRest({ object_name: OBJ }); + const exact = atRest.filter((r) => readable.has(String(r.record_id))).map((r) => String(r.id)).sort(); + const early = atRest.filter((r) => String(r.record_id) === id.early); + expect(early.length).toBeGreaterThan(0); + expect(await servedTotal('reader', { object_name: OBJ })).toBe(exact.length); + const newestFirst = await servedList('reader', LEDGER, { object_name: OBJ }, '-created_at'); + // Armed: the page really is newest first. + const stamps = newestFirst.map((r) => String(r.created_at)); + expect([...stamps].sort().reverse()).toEqual(stamps); + expect(newestFirst.map((r) => String(r.id)).sort()).toEqual(exact); + for (const row of early) expect(newestFirst.map((r) => String(r.id))).toContain(String(row.id)); expect(bulkAtRest).toBeGreaterThan(SCAN_BOUND); }); From 601e1483b03c613d9f0c8602da191cf81be2d6c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 14:37:45 +0000 Subject: [PATCH 08/10] =?UTF-8?q?chore(changeset):=20plugin-audit=20minor?= =?UTF-8?q?=20=E2=80=94=20the=20parent-record=20read=20gates=20switch=20to?= =?UTF-8?q?=20the=20ReadableIds=20leaf?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- .changeset/23673-parent-gate-readable-ids.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 .changeset/23673-parent-gate-readable-ids.md diff --git a/.changeset/23673-parent-gate-readable-ids.md b/.changeset/23673-parent-gate-readable-ids.md new file mode 100644 index 00000000000..cc6ff5106ba --- /dev/null +++ b/.changeset/23673-parent-gate-readable-ids.md @@ -0,0 +1,18 @@ +--- +'@objectstack/plugin-audit': minor +--- + +feat(plugin-audit): the `sys_activity` and `sys_audit_log` read gates write one `ReadableIds` leaf per parent object and do no pre-scan, so a broad read is judged over every row, not a 2,000-row window (ADR-0139 D4, D5) + +Clause-②: no + +Both gates serve a row only when the caller may read the record it is about. Until now each read first scanned at most 2,000 candidate rows as the system, then asked which of those rows' parent records the caller could read. Past that window the gate failed closed: the unscanned rows were left out, the list's `total` agreed with the short answer, and a warning said so. + +Each gate now writes one branch per object the metadata registry holds (outside the audit writer's excluded objects and the gated object itself): `{ object_name: P, record_id: { $in: ReadableIds.of(P) } }`. The engine composes the leaf from the security service's `canReadObject` and `getReadFilter` for `P`, and the SQL drivers evaluate it as one subquery against `P`'s table, inside their own tenant wall. A list, its `total` and a grouped count are each one statement over every row the caller may read. A parent object the caller may not read is answered with no rows, without reading it as the caller, so no denial is logged for it. + +What a reader sees: + +- `sys_audit_log`: a broad read is served every row about a record the caller may read, and every row about no record, with a true `total`, whatever the table size. The rows about no record (`config_change`, `import`, `platform_admin_standing_change`, an auth event with no session) are now a filter on the ledger's own columns rather than a list of ids the scan saw. A holder of `view_all_audit_log` is still not narrowed. A row about a session is still served when that session is readable. +- `sys_activity`: the gate itself has no window any more, but the stream's withheld-update rule still pre-scans 2,000 rows as the system, so a broad read stays bounded by that rule until it becomes a predicate (ADR-0139 E6). A read about one record is unchanged. +- Three classes still read through the bounded probe, chosen before any leaf is written: a delegated (on-behalf-of) caller; a parent an object-scoped read middleware narrows (for example `sys_attachment`, `sys_comment_reaction`, `sys_approval_request` and the settings objects); and a parent bound to another datasource than the gated object, which covers every parent on a deployment that registers the `telemetry` datasource. The whole read also stays on the probe when the security service offers no `canReadObject`, or when none is registered, as before. +- On a driver that does not compile the leaf (`driver-mongodb`), a read of either object now fails with `INTERNAL_ERROR` / 500 naming the driver and the leaf, instead of serving a capped list. The gates do not turn that refusal into an empty answer. This is ADR-0139's ruled behaviour until a pipeline form is measured for that driver. From 0123e557131eea82fd577be2885bb35674ebd8d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 14:45:28 +0000 Subject: [PATCH 09/10] test(plugin-audit): type the gate WHERE the leaf-path case reads Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- .../plugins/plugin-audit/src/activity-read-visibility.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 7911536893d..93141482f70 100644 --- a/packages/plugins/plugin-audit/src/activity-read-visibility.test.ts +++ b/packages/plugins/plugin-audit/src/activity-read-visibility.test.ts @@ -437,7 +437,9 @@ describe('[ADR-0139 D4] the leaf path: one ReadableIds leaf per parent object, a const existing = { type: 'updated' }; for (const operation of ['find', 'findOne', 'count', 'aggregate'] as const) { const { mw } = installClassifying({ objects: ['crm_case'] }); - const { where } = await read(mw, { operation, ast: { object: 'sys_activity', where: existing } }); + const where = (await read(mw, { operation, ast: { object: 'sys_activity', where: existing } })).where as { + $and: unknown[]; + }; expect(where.$and[0]).toBe(existing); expect(leafObjectOf(where.$and[1])).toBe('crm_case'); } From 3ecca941d6c4aa571cf213bf757449b02b3c8e11 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 15:14:04 +0000 Subject: [PATCH 10/10] test(plugin-audit): the classifying engine doubles override the comment-gate double; the refusing driver keeps only the reads the suite drives Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- .../src/activity-read-visibility.test.ts | 7 ++- .../src/audit-log-read-visibility.test.ts | 45 ++++++++++--------- ...arent-record-read-leaf.integration.test.ts | 4 -- 3 files changed, 28 insertions(+), 28 deletions(-) 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 93141482f70..1b964e1d780 100644 --- a/packages/plugins/plugin-audit/src/activity-read-visibility.test.ts +++ b/packages/plugins/plugin-audit/src/activity-read-visibility.test.ts @@ -368,7 +368,7 @@ function installClassifying(opts: { }) { let mw!: Mw; const calls = { scans: [] as any[], probes: [] as Array<{ object: string; ids: string[]; context: any }> }; - const engine: ParentRecordGateEngine = { + const base: CommentAccessEngine = { registerHook: () => {}, registerMiddleware: (fn) => { mw = fn as Mw; @@ -384,10 +384,13 @@ function installClassifying(opts: { return ids.filter((id) => list.includes(id)).map((id) => ({ id })) as any; }, findOne: async () => null, + }; + // The classifying questions, over the comment-gate double above. + const engine: ParentRecordGateEngine = Object.assign(base, { registry: (opts.registry ?? { getAllObjects: () => opts.objects.map((name) => ({ name })) }) as any, hasObjectMiddleware: (object: string) => (opts.middleware ?? []).includes(object), resolveEffectiveDatasource: (object: string) => opts.datasource?.[object], - }; + }); // A security service that answers `canReadObject` unless the case says otherwise: // without it the gate writes no leaf (see the `no-object-admission` case). installActivityReadVisibility( diff --git a/packages/plugins/plugin-audit/src/audit-log-read-visibility.test.ts b/packages/plugins/plugin-audit/src/audit-log-read-visibility.test.ts index 1f34de88ee6..8dd1db6f9db 100644 --- a/packages/plugins/plugin-audit/src/audit-log-read-visibility.test.ts +++ b/packages/plugins/plugin-audit/src/audit-log-read-visibility.test.ts @@ -287,23 +287,31 @@ describe('installAuditLogReadVisibility — the bounded probe (an engine that ca }); /** A stand-in engine that answers the three questions the gate classifies a parent by. */ -function installClassifying(objects: string[]) { - let mw!: Mw; - const calls = { scans: [] as any[] }; - const engine: ParentRecordGateEngine = { +/** The comment-gate engine double, recording its middleware and its reads. */ +function baseEngine(onMiddleware: (fn: Mw) => void, onFind: (object: string, options: any) => void): CommentAccessEngine { + return { registerHook: () => {}, - registerMiddleware: (fn) => { - mw = fn as Mw; - }, + registerMiddleware: (fn) => onMiddleware(fn as Mw), find: async (object: string, options: any) => { - calls.scans.push({ object, options }); + onFind(object, options); return []; }, findOne: async () => null, - registry: { getAllObjects: () => objects.map((name) => ({ name })) } as any, - hasObjectMiddleware: () => false, - resolveEffectiveDatasource: () => undefined, }; +} + +function installClassifying(objects: string[]) { + let mw!: Mw; + const calls = { scans: [] as any[] }; + // The classifying questions, over the comment-gate double. + const engine: ParentRecordGateEngine = Object.assign( + baseEngine((fn) => (mw = fn), (object, options) => calls.scans.push({ object, options })), + { + registry: { getAllObjects: () => objects.map((name) => ({ name })) } as any, + hasObjectMiddleware: () => false, + resolveEffectiveDatasource: () => undefined, + }, + ); installAuditLogReadVisibility(engine, silentLogger(), () => ({ canReadObject: async () => true })); return { mw, calls }; } @@ -334,17 +342,10 @@ describe('[ADR-0139 D4] the ledger on the leaf path', () => { it('the exemption is decided before anything is enumerated', async () => { const getAllObjects = vi.fn(() => [{ name: 'crm_case' }]); let mw!: Mw; - const engine: ParentRecordGateEngine = { - registerHook: () => {}, - registerMiddleware: (fn) => { - mw = fn as Mw; - }, - find: async () => [], - findOne: async () => null, - registry: { getAllObjects } as any, - hasObjectMiddleware: () => false, - resolveEffectiveDatasource: () => undefined, - }; + const engine: ParentRecordGateEngine = Object.assign( + baseEngine((fn) => (mw = fn), () => {}), + { registry: { getAllObjects } as any, hasObjectMiddleware: () => false, resolveEffectiveDatasource: () => undefined }, + ); installAuditLogReadVisibility(engine, silentLogger(), () => undefined); const existing = { action: 'delete' }; const { where } = await read(mw, { 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 9a573e5862d..bfcb4e5bc0f 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 @@ -396,10 +396,6 @@ class RefusingDriver extends SqliteWasmDriver { this.refuse(query); return super.find(object, query, options); } - override async findOne(object: string, query: any, options?: any) { - this.refuse(query); - return super.findOne(object, query, options); - } override async count(object: string, query?: any, options?: any) { this.refuse(query); return super.count(object, query, options);