diff --git a/.changeset/23732-attachment-readable-ids.md b/.changeset/23732-attachment-readable-ids.md new file mode 100644 index 00000000000..94466de4346 --- /dev/null +++ b/.changeset/23732-attachment-readable-ids.md @@ -0,0 +1,20 @@ +--- +'@objectstack/service-storage': minor +--- + +feat(service-storage): the `sys_attachment` read gate writes one `ReadableIds` leaf per parent object and does no pre-scan, so a broad read is judged over every row, not a 2,000-row window (ADR-0139 D4, D5) + +Clause-②: yes + +The gate serves an attachment only when the caller may read the record it is attached to. 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. + +The gate now writes one branch per object the metadata registry holds (except `sys_attachment` itself): `{ parent_object: P, parent_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: + +- A broad read of `sys_attachment` is served every attachment whose parent record the caller may read, with a true `total`, whatever the table size. An attachment whose parent the caller may not read, whose parent no longer exists, or that names no parent is still never served. A read scoped to one record (the attachments panel) 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_comment`, `sys_comment_reaction`, `sys_activity` and `sys_audit_log`); and a parent bound to another datasource than `sys_attachment`. The whole read also stays on the probe when the security service offers no `canReadObject`, or when none is registered. +- A read whose probe finds no candidate row is now narrowed to no rows rather than left unfiltered. It served nothing before either. +- On a driver that does not compile the leaf (`driver-mongodb`), a read of `sys_attachment` now fails with `INTERNAL_ERROR` / 500 naming the driver and the leaf, instead of serving a capped list. The gate does not turn that refusal into an empty answer. This is ADR-0139's ruled behaviour until a pipeline form is measured for that driver. + +For a host that installs the gate itself: `installAttachmentReadVisibility(engine, logger, getAdmission?)` takes an optional third argument that resolves the security service on every read. `StorageServicePlugin` passes it. A call without it keeps every read on the bounded probe and says so once. diff --git a/docs/qa/platform-checklist/areas/attachments-storage.json b/docs/qa/platform-checklist/areas/attachments-storage.json index 95962e4f25a..56ad4afe2e5 100644 --- a/docs/qa/platform-checklist/areas/attachments-storage.json +++ b/docs/qa/platform-checklist/areas/attachments-storage.json @@ -267,7 +267,7 @@ "title": "sys_attachment reads inherit parent visibility: a restricted member sees neither rows nor counts for invisible parents", "since": "v15.1", "status": "active", - "revision": 3, + "revision": 4, "priority": "P1", "surface": "api", "personas": ["member who can read the parent record", "member who cannot read the parent record"], @@ -283,7 +283,7 @@ "knownGaps": [ "MARKER (QA run #9401 finding 5, carried here so runs stop re-deriving it): the CROSS-TENANT clauses of this area are structurally unprovable in the OSS workspace. `@objectstack/organizations` is open core since ADR-0132 (Apache-2.0, published on npm) — ⛔ it being closed-source is NOT the reason and has not been since #16215. The reason is ADR-0132's entitlement boundary: no framework package may declare `@objectstack/organizations` (pinned by `packages/plugins/organizations/src/no-framework-dependents.pin.test.ts` — apps declare it, packages do not), so `packages/qa/dogfood` cannot depend on it and the dogfood matrix's cross-tenant block is gated behind `describe.skipIf(!organizationsAvailable)` and skips BY DESIGN here. That is blocked(dependency), never a pass and never a defect. A run that is supposed to ship the package sets OS_TEST_MULTI_ORG_ENABLED=1, which turns the skip into a hard failure instead of a silent one.", "CLOSED by the qa-scratch-authz recipe (#7670): stock showcase's files-enabled parent (showcase_project) is public_read_write — every member sees every project, so the invisible-parent case is not demonstrable on stock seeds, and the pinned dogfood matrix proves it only on its own private fixture (att_secret). The recipe provisions the private files-enabled parent at runtime; landing one in the showcase seeds proper would retire this gap entirely, and remains the better long-term fix.", - "the READ_SCAN_LIMIT fail-closed branch (clause 4) needs a broad read past the 2000-candidate cap — run #7635 reached it by bulking to 2100 join rows on the scratch parent. Budget for that write volume, or the clause degrades to 'the warning never appeared', which is indistinguishable from a silent leak." + "a broad read over an ordinary parent no longer meets the 2000-candidate cap (ADR-0139 E7, #23767): the gate writes one ReadableIds leaf per parent object and pre-scans nothing, so the scratch parents' broad read is served every visible row with a true total and no '[storage] attachment read visibility' warning. The bounded 2000-row fail-closed probe (READ_SCAN_LIMIT, clause 4's warning) remains for ADR-0139 D5's three classes only: a delegated (on-behalf-of) caller, a parent narrowed by an object-scoped read middleware (sys_comment, sys_comment_reaction, sys_activity and sys_audit_log among them), and a parent bound to another datasource than sys_attachment. None of the three is provisioned by the qa-scratch-authz recipe, so clause 4's probe half is not reachable from a run here: it is pinned in packages/services/service-storage/src/attachment-read-leaf.integration.test.ts and attachment-read-leaf.test.ts, and a run cites them. On driver-mongodb, which compiles no ReadableIds leaf until ADR-0139's O3 is measured, a member's sys_attachment read fails INTERNAL_ERROR / 500 naming the driver instead of serving a short list: a ruled limit (ADR-0139 D3, Consequences), never a pass and never a leak." ] }, "steps": [ @@ -325,10 +325,10 @@ "rows filtered but `total` counting the raw table is equally a FAIL (the middleware-not-hook design note in attachment-access-hooks.ts exists because a find-hook leaves count() unfiltered)" ], "traps": ["wrong-persona"], - "automated": { "kind": "api", "ref": "packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts (clauses 0 and 2: the restricted member's scoped list is empty for an invisible parent while the entitled one is not, and a by-id read of the hidden row is 403/404) + packages/qa/dogfood/test/attachments-parent-rls-count-parity.dogfood.test.ts (clause 1, the COUNT: the restricted member's list `total` excludes invisible-parent rows while the entitled caller's includes them on the same request, engine.count() under the member's OWN resolved authz context (resolveAuthzContext, not a hand-built principal) is filtered identically, a count NARROWED to the invisible parent answers 0 for the member and the true number for admin, and paging the member all the way through yields exactly the reported total — so rows-vs-count parity is asserted as one fact rather than two numbers that happen to agree. ⚠ THE TRAP, and why every assertion passes $top: protocol.findData only calls engine.count() when the request carries a PAGE LIMIT; without one it sets total = records.length, which is trivially self-consistent and stays green with count() fully unfiltered because the count path never runs. Each assertion therefore also requires total > records.length, proving the count call happened rather than assuming it) + packages/qa/dogfood/test/attachments-parent-rls-scan-cap.dogfood.test.ts (clause 3, the READ_SCAN_LIMIT half: with >2000 candidate join rows the broad read's pre-scan is truncated, and the pin asserts the '[storage] attachment read visibility ... 2000-row cap ... fail-closed' warning fires (the clause's own log oracle — silence plus leaked rows is the failure), that visible rows are DROPPED rather than the un-scanned remainder being admitted, that the invisible-parent row never appears, that every row returned has a genuinely resolvable visible parent, and that a row the broad read omitted is still returned by a SCOPED read that does not hit the cap — the contrast without which 'missing' would be equally explained by 'invisible anyway'. The seed puts visible rows at BOTH ENDS of the insertion order with the filler bulk between, because the pre-scan has no ORDER BY and the driver was measured NOT to return insertion order; the control asserts the window is full and some visible parent is outside it rather than trusting either). ALSO PINNED, as the reachable half of clause 3's fail-compute arm: a row whose parent RECORD no longer resolves (a dangling parent_id, what a hard-deleted parent leaves behind) is excluded from both rows and count. NOT REACHED, and not to be read as covered: (a) the unknown parent_OBJECT arm (`catch { visible = [] }`) has no reachable fixture from this lane — the #2727 enable.files gate refuses to create a sys_attachment row against a non-files object even under system context (measured: \"File attachments are not enabled for object 'att_ghost_object'\"), so such a row cannot be seeded; (b) the middleware-level outer catch ('filter failed, denying all') is not provokable through the API, since a where malformed enough to break the system pre-scan also fails the caller's own read and surfaces as a 4xx instead." }, + "automated": { "kind": "api", "ref": "packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts (clauses 0 and 2: the restricted member's scoped list is empty for an invisible parent while the entitled one is not, and a by-id read of the hidden row is 403/404) + packages/qa/dogfood/test/attachments-parent-rls-count-parity.dogfood.test.ts (clause 1, the COUNT: the restricted member's list `total` excludes invisible-parent rows while the entitled caller's includes them on the same request, engine.count() under the member's OWN resolved authz context (resolveAuthzContext, not a hand-built principal) is filtered identically, a count NARROWED to the invisible parent answers 0 for the member and the true number for admin, and paging the member all the way through yields exactly the reported total — so rows-vs-count parity is asserted as one fact rather than two numbers that happen to agree. ⚠ THE TRAP, and why every assertion passes $top: protocol.findData only calls engine.count() when the request carries a PAGE LIMIT; without one it sets total = records.length, which is trivially self-consistent and stays green with count() fully unfiltered because the count path never runs. Each assertion therefore also requires total > records.length, proving the count call happened rather than assuming it) + packages/qa/dogfood/test/attachments-parent-rls-scan-cap.dogfood.test.ts (clause 3, the broad read past the old 2000-row pre-scan window, @proof attachments-parent-rls-scan-cap: since ADR-0139 E7 the gate writes one ReadableIds leaf per parent object and pre-scans nothing for an ordinary parent, so with >2000 candidate join rows the restricted member is served EVERY visible row on one page, `total` equals the rows served (the read passes $top, so total comes from engine.count()), no '-row cap' warning is logged, the invisible-parent row and every dangling-parent row stay excluded, and the owner is served the hidden row on the same read. The seed still puts visible rows at BOTH ENDS of the insertion order with the filler bulk between, and the control asserts that a 2000-row window would leave a visible row outside it, so the pin cannot pass on a fixture the old bound would have served whole) + packages/services/service-storage/src/attachment-read-leaf.integration.test.ts and packages/services/service-storage/src/attachment-read-leaf.test.ts (clause 3, the half that still bounds: ADR-0139 D5's three classes, a delegated caller, a parent narrowed by an object-scoped read middleware and a federated parent, keep the bounded 2000-row fail-closed probe, chosen before any leaf is written; past the cap the probe warns naming its class, and a probe failure denies all and says so. Also there: on a driver that compiles no leaf, as driver-mongodb until ADR-0139's O3, find and count fail { code: 'INTERNAL_ERROR', status: 500 } naming the driver, never the deny-all sentinel). ALSO PINNED, as the reachable half of clause 3's fail-compute arm: a row whose parent RECORD no longer resolves (a dangling parent_id, what a hard-deleted parent leaves behind) is excluded from both rows and count. NOT REACHED from this lane, and not to be read as covered by a run here: (a) an attachment naming a parent OBJECT the registry does not hold cannot be seeded — the #2727 enable.files gate refuses to create a sys_attachment row against a non-files object even under system context (measured: \"File attachments are not enabled for object 'att_ghost_object'\"); such a row names no enumerated parent, so no branch admits it, which attachment-read-leaf.integration.test.ts pins below the API; (b) the middleware no longer wraps the read in an outer catch (ADR-0139 D3): the probe's own failure ('the bounded probe (...) failed, denying all') is pinned in attachment-read-leaf.test.ts and is not provokable through the API, since a where malformed enough to break the system pre-scan also fails the caller's own read and surfaces as a 4xx instead." }, "source": [ "docs/plans/release-15.1-test-plan.md §C3 (#2970)", - "packages/services/service-storage/src/attachment-access-hooks.ts#installAttachmentReadVisibility (installAttachmentReadVisibility: middleware over find/findOne/count/aggregate, deny-all sentinel, READ_SCAN_LIMIT fail-closed cap)", + "packages/services/service-storage/src/attachment-access-hooks.ts#installAttachmentReadVisibility (installAttachmentReadVisibility: middleware over find/findOne/count/aggregate, one ReadableIds leaf per parent object (ADR-0139 D4), deny-all sentinel, the READ_SCAN_LIMIT fail-closed probe for ADR-0139 D5's three classes)", "packages/services/service-storage/src/attachment-read-visibility.test.ts" ], "history": [ @@ -339,6 +339,12 @@ "date": "2026-08-18", "change": "clauses 1 and 3 pinned — QA run #9401 scored this item partial with the COUNT and the fail-closed cap unpinned. A count leak is a real RLS leak: `total` comes from engine.count(), not the find path, which is exactly why the visibility rule is a data middleware over find/findOne/count/aggregate rather than a find hook, and a suite reading only `records` cannot tell the two apart. automated.ref now names the count-parity and scan-cap pins, records the $top trap that makes a count assertion vacuous without a page limit, and enumerates the two fail-closed arms that remain unreachable from this lane so neither is read as covered. Ablation-verified: dropping 'count' from READ_OPS turns 5 of the 7 count-parity tests red and leaves the row-only assertions green.", "ref": "#9483" + }, + { + "revision": 4, + "date": "2026-10-16", + "change": "clause 3's broad-read half re-pinned for ADR-0139 E7: the attachment gate writes one ReadableIds leaf per parent object and pre-scans nothing, so a broad read over an ordinary parent past 2000 candidates is served whole with a true total, and the scan-cap pin is rewritten for that side of the bound (its @proof tag kept). knownGaps and automated.ref now name the limits that remain: ADR-0139 D5's three classes keep the bounded 2000-row probe (pinned in service-storage), and driver-mongodb answers INTERNAL_ERROR / 500 until O3. The unknown-parent-object and outer-catch arms are restated for the new gate.", + "ref": "#23767" } ] }, diff --git a/packages/qa/dogfood/test/attachments-parent-rls-scan-cap.dogfood.test.ts b/packages/qa/dogfood/test/attachments-parent-rls-scan-cap.dogfood.test.ts index 069059ddbe8..cecdbf4d152 100644 --- a/packages/qa/dogfood/test/attachments-parent-rls-scan-cap.dogfood.test.ts +++ b/packages/qa/dogfood/test/attachments-parent-rls-scan-cap.dogfood.test.ts @@ -1,37 +1,37 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. // -// attachments-storage.read-inherits-parent-rls — clause C3, the 2000-row -// pre-scan cap. +// attachments-storage.read-inherits-parent-rls — clause C3, the broad read +// past the old 2,000-row pre-scan window. // -// The read-visibility middleware pre-scans the candidate (parent_object, -// parent_id) pairs a query would touch, bounded at READ_SCAN_LIMIT = 2000. -// Past that bound the filter is built from a TRUNCATED candidate set, and the -// clause is about which way the truncation falls: rows outside the scan window -// are EXCLUDED (the caller may lose rows they could see) rather than admitted -// unfiltered (the caller gains rows they may not). The cap also logs, because a -// silent truncation is indistinguishable from a leak — the item's own wording: -// "silence plus leaked rows is the failure". +// [ADR-0139 E7] The read-visibility middleware used to pre-scan the candidate +// (parent_object, parent_id) pairs a query would touch, bounded at 2,000 rows, +// and fail CLOSED past that bound: the un-scanned rows were excluded, a warning +// said so, and the list's `total` agreed with the short answer. It now writes +// one `ReadableIds` leaf per parent object and pre-scans nothing for an +// ordinary parent: the engine composes the leaf from the security service's +// `canReadObject` and `getReadFilter`, and the driver evaluates it as one +// subquery. So the clause this file pins is now the OTHER side of the bound: +// a broad read past 2,000 candidates serves EVERY row the member may read, +// its `total` is true, no truncation is announced, and the security direction +// holds exactly as before — no invisible-parent row and no dangling-parent row. +// (The bounded probe survives for ADR-0139 D5's three classes only, which this +// fixture's parents are not; those are pinned in service-storage's +// `attachment-read-leaf.integration.test.ts`.) // // ## Why the seed is shaped the way it is // -// The pre-scan takes 2000 rows in whatever order the driver returns them, with -// no ORDER BY of its own — so which rows land inside the window is NOT a -// property this test may assume. A first attempt seeded the probe row last and -// asserted it fell outside; measured, it fell INSIDE (the control caught it), -// so the driver is not returning insertion order here. -// -// The seed therefore puts visible rows at BOTH ENDS of the insertion sequence -// with the filler bulk between them. Any ordering that takes a contiguous run -// from either end leaves one of the two groups outside the window, so some -// visible row is excluded either way — and the test then picks an actually -// excluded row at RUNTIME rather than predicting which one it will be. The -// control asserts that premise (window full, at least one visible parent -// unscanned) instead of trusting it. +// The old pre-scan took 2,000 rows in whatever order the driver returned them, +// so the seed puts visible rows at BOTH ENDS of the insertion sequence with the +// filler bulk between them: any contiguous window from either end leaves one +// group outside it. The control asserts that premise (a 2,000-row window +// really does leave some visible row unscanned), so this pin cannot pass on a +// fixture the old bound would have served whole. // // Volume goes in through bulk system inserts: the clause is about the READ // path, and 2000 real presigned uploads would pay for nothing it tests. The // filler rows share ONE committed sys_file (ContentDocumentLink semantics — a -// file legitimately carries many join rows). +// file legitimately carries many join rows) and name parent ids that do not +// exist, which is what a hard-deleted parent record leaves behind. // // @proof: attachments-parent-rls-scan-cap @@ -47,9 +47,9 @@ import { const SYS = { isSystem: true } as const; -/** READ_SCAN_LIMIT in attachment-access-hooks.ts. Mirrored, not imported: the - * test asserts the SHIPPED bound, so a silent change to it must surface here - * as a failure rather than be followed automatically. */ +/** The old pre-scan window (READ_SCAN_LIMIT in attachment-access-hooks.ts, now + * the bounded probe's, for ADR-0139 D5's classes only). Mirrored, not + * imported: the fixture must stay past the bound the gate shipped with. */ const READ_SCAN_LIMIT = 2_000; /** Visible attachments seeded BEFORE the filler bulk, and again AFTER it. */ @@ -74,7 +74,7 @@ function captureLog() { }; } -describe('sys_attachment read visibility fails CLOSED past the pre-scan cap (#9483)', () => { +describe('[ADR-0139 E7] a broad sys_attachment read past the old pre-scan window is served whole', () => { let harness: AttachmentsHarness; let stack: VerifyStack; let ql: any; @@ -167,31 +167,27 @@ describe('sys_attachment read visibility fails CLOSED past the pre-scan cap (#94 await stopAttachmentsHarness(harness); }); - it('control: the window is full and some visible parent really is outside it', async () => { - // The premise, asserted. An unreached branch reads exactly like a satisfied - // one, so if the seed ever stopped crossing the bound this must fail loudly - // rather than let the clause pass on a code path it never entered. + it('control: the old window would be full and some visible row really is outside it', async () => { + // The premise, asserted. A fixture the old 2,000-row window would have + // served whole cannot tell the leaf from the bound, so if the seed ever + // stopped crossing it this must fail loudly. const total = await ql.count('sys_attachment', { context: { ...SYS } }); - expect(total, 'candidate rows must exceed READ_SCAN_LIMIT').toBeGreaterThan(READ_SCAN_LIMIT); + expect(total, 'candidate rows must exceed the old window').toBeGreaterThan(READ_SCAN_LIMIT); expect(visibleRows.size).toBe(VISIBLE_PER_GROUP * 2); - const scanned = await ql.find('sys_attachment', { + const window = await ql.find('sys_attachment', { where: {}, fields: ['id'], limit: READ_SCAN_LIMIT, context: { ...SYS }, }); - expect(scanned.length, 'the pre-scan window is exactly the cap').toBe(READ_SCAN_LIMIT); - - const scannedIds = new Set(scanned.map((r: any) => String(r.id))); - const unscannedVisible = [...visibleRows.keys()].filter((id) => !scannedIds.has(id)); - expect( - unscannedVisible.length, - 'at least one visible row must fall outside the window, or the clause is unreachable', - ).toBeGreaterThan(0); + expect(window.length, 'a 2,000-row window is full').toBe(READ_SCAN_LIMIT); + const inWindow = new Set(window.map((r: any) => String(r.id))); + const outside = [...visibleRows.keys()].filter((id) => !inWindow.has(id)); + expect(outside.length, 'at least one visible row falls outside a 2,000-row window').toBeGreaterThan(0); }); - it('C3 — the capped broad read warns AND fails closed: visible rows are dropped, the hidden one never appears', async () => { + it('C3 — the member is served every visible row, with a true total, and nothing is announced as truncated', async () => { const log = captureLog(); let body: any; try { @@ -202,73 +198,33 @@ describe('sys_attachment read visibility fails CLOSED past the pre-scan cap (#94 log.restore(); } - // 1. The truncation is LOUD. This is the clause's own oracle: silence plus - // leaked rows is the failure, so the warning is what makes a truncated - // answer distinguishable from a complete one. - const warned = log.matching('[storage] attachment read visibility'); - expect(warned.length, 'the cap must announce itself').toBeGreaterThan(0); - expect( - warned.some((l) => l.includes(`${READ_SCAN_LIMIT}-row cap`) && l.includes('fail-closed')), - 'the warning names the cap and the direction it falls', - ).toBe(true); - - // 2. It falls CLOSED: rows the member could otherwise see are excluded, - // rather than the un-scanned remainder being admitted unfiltered. + // 1. Every row the member may read, past the old window, on one page. const returned = new Set(body.records.map((r: any) => String(r.id))); - const returnedVisible = [...visibleRows.keys()].filter((id) => returned.has(id)); - expect( - returnedVisible.length, - 'a truncated scan must return FEWER visible rows than exist, not more', - ).toBeLessThan(visibleRows.size); + const missing = [...visibleRows.keys()].filter((id) => !returned.has(id)); + expect(missing, 'no visible row is dropped').toEqual([]); + expect(body.records.length, 'and nothing else is served').toBe(visibleRows.size); - // 3. And the security direction, which must hold under truncation too. - expect(returned.has(hiddenAttachmentId), 'an invisible parent’s row must never appear').toBe( - false, - ); - expect( - body.records.every((r: any) => r.parent_object !== 'att_secret'), - 'no att_secret row under any id', - ).toBe(true); + // 2. The total is the engine's count over the same WHERE, and it agrees. + expect(body.total, 'total equals the rows served').toBe(visibleRows.size); - // 4. Every row that DID come back is one the member may genuinely see — - // truncation must not admit a dangling-parent row either. - expect( - body.records.every((r: any) => visibleRows.has(String(r.id))), - 'no row with an unresolvable parent is admitted by the truncated filter', - ).toBe(true); + // 3. No truncation: the bounded probe never ran for this parent. + expect(log.matching('-row cap'), 'no pre-scan cap is announced').toEqual([]); }); - it('the dropped rows are genuinely visible: a scoped read returns one the broad read omitted', async () => { - // The contrast that makes assertion 2 above mean "excluded by the cap" - // rather than "invisible anyway". Without it, a surface that hid these rows - // from this member for some entirely different reason would satisfy the - // fail-closed assertion while proving nothing about the cap. - const broad = await stack.apiAs(memberTok, 'GET', '/data/sys_attachment?$top=200'); - const records = ((await broad.json()) as any).records as any[]; - - // The page must NOT be full, or "absent from the page" means paged out - // rather than filtered out and the conclusion below does not follow. This - // guard is not decoration: the fail-open ablation of the cap was caught by - // it and by nothing else — with the filter removed the page fills with - // unfiltered rows, every visible row falls off the end, and the test would - // otherwise have gone green while the surface leaked the whole table. - expect(records.length, 'the page is not full, so absence means excluded').toBeLessThan(200); - - const returned = new Set(records.map((r: any) => String(r.id))); - const dropped = [...visibleRows.entries()].find(([id]) => !returned.has(id)); - expect(dropped, 'the broad read dropped at least one visible row').toBeTruthy(); - - const [droppedId, parentId] = dropped as [string, string]; - const scoped = await stack.apiAs( - memberTok, - 'GET', - `/data/sys_attachment?$top=10&parent_object=att_case&parent_id=${parentId}`, - ); - expect(scoped.status).toBe(200); - const rows = ((await scoped.json()) as any).records; + it('the security direction holds past the old window: no invisible-parent row, no dangling-parent row', async () => { + const res = await stack.apiAs(memberTok, 'GET', '/data/sys_attachment?$top=200'); + expect(res.status).toBe(200); + const records = ((await res.json()) as any).records as any[]; + expect(records.some((r) => String(r.id) === hiddenAttachmentId), 'an invisible parent’s row never appears').toBe(false); + expect(records.filter((r) => r.parent_object === 'att_secret'), 'no att_secret row under any id').toEqual([]); expect( - rows.some((r: any) => String(r.id) === droppedId), - 'the same row, reachable when the read does not hit the cap', - ).toBe(true); + records.filter((r) => String(r.parent_id).startsWith('cap-ghost-')), + 'a row whose parent record does not exist is never served', + ).toEqual([]); + + // The paired side: the record's owner is served the hidden row on the same read. + const asAdmin = await stack.apiAs(adminTok, 'GET', '/data/sys_attachment?$top=200'); + const adminRecords = ((await asAdmin.json()) as any).records as any[]; + expect(adminRecords.some((r) => String(r.id) === hiddenAttachmentId), 'the owner reads it').toBe(true); }); }); diff --git a/packages/services/service-storage/src/attachment-access-hooks.ts b/packages/services/service-storage/src/attachment-access-hooks.ts index d6f749b45dd..3e8aaf2cdb1 100644 --- a/packages/services/service-storage/src/attachment-access-hooks.ts +++ b/packages/services/service-storage/src/attachment-access-hooks.ts @@ -2,7 +2,14 @@ import { resolveReadableParentIds, withoutOperationPrivateKeys } from '@objectstack/core'; import type { StandardErrorCode } from '@objectstack/spec/api'; -import type { ISecurityService, ISharingService } from '@objectstack/spec/contracts'; +import type { + EngineSchemaRegistryView, + IDataEngine, + IObjectQLEngine, + ISecurityService, + ISharingService, +} from '@objectstack/spec/contracts'; +import { ReadableIds } from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; import { renderOperationMessage, type ValidationMessageTranslator } from '@objectstack/spec/system'; @@ -678,14 +685,61 @@ export function installAttachmentAccessHooks( * `{ id: '__deny_all__' }` read-filter deny). */ const READ_DENY_ALL = { id: '__attachment_parent_denied__' } as const; -/** Bound on the per-read candidate pre-scan. Beyond this the filter fails - * CLOSED (excludes the un-scanned rows) rather than leaking them. */ +/** Bound on the bounded probe's per-read candidate pre-scan (ADR-0139 D5). + * Beyond it the probe fails CLOSED (the un-scanned rows are excluded, never + * leaked). The `ReadableIds` leaf has no bound. */ const READ_SCAN_LIMIT = 2_000; const READ_OPS = new Set(['find', 'findOne', 'count', 'aggregate']); +/** The gated object: never a parent of its own rows (see {@link attachmentParentOf}). */ +const ATTACHMENT_OBJECT = 'sys_attachment'; + +/** Object machine-name shape (`ObjectSchema.name` in packages/spec). */ +const OBJECT_NAME_RE = /^[a-z_][a-z0-9_]*$/; + +/** + * Resolves, on every read, the security service whose object-level read + * admission (`canReadObject`, OPTIONAL on `ISecurityService`) decides whether + * this gate may write a `ReadableIds` leaf at all. The security plugin may + * register after this one, so a service captured at install time would pin + * "none" for the life of the process. + */ +type AttachmentReadAdmissionSource = () => Pick | null | undefined; + +/** + * The engine the read gate reads through: the lifecycle seams' 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. + */ +type AttachmentReadGateEngine = AttachmentLifecycleEngine & { + /** The registry the parent objects are enumerated from. */ + registry?: Pick; +} & Partial> & + Partial>; + +/** Why the WHOLE read goes to the bounded probe, decided before any leaf. */ +type WholeReadProbe = 'delegated-caller' | 'engine-cannot-classify' | 'no-object-admission'; + /** - * sys_attachment READ visibility inheritance (#2755 follow-up, #2970 item 1). + * How one read reaches its parents: the parent objects written as leaves and + * the ones the probe reads, each with its D5 class — or the one state that + * sends the whole read to the probe. + */ +type AttachmentReadPlan = + | { probeWholeRead: WholeReadProbe } + | { + 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; + }; + +/** + * sys_attachment READ visibility inheritance (#2755 follow-up, #2970 item 1; + * ADR-0139 D4, D5). * * The create/delete hooks above gate writes, but a member could still LIST * `sys_attachment` rows (file_name, size, parent_id) pointing at records they @@ -700,16 +754,80 @@ const READ_OPS = new Set(['find', 'findOne', 'count', 'aggregate']); * the returned rows — a beforeFind/afterFind hook would leave `count()` * unfiltered and leak the true row count via `total`. * - * Mechanism (generalizes ADR-0055 `controlled_by_parent` to a polymorphic - * parent): for each read, pre-scan the candidate `(parent_object, parent_id)` - * pairs the query would touch (system context), resolve the visible parent - * ids per `parent_object` through the caller-scoped engine (RLS/OWD/sharing - * of the PARENT apply), and AND a `$or` of - * `{ parent_object, parent_id: { $in: } }` into `ctx.ast.where`. + * ⛔ Nothing here derives readability: the engine and the security service + * answer "can this caller read that record", through one of two mechanisms + * chosen per parent object, up front. + * + * ## The leaf (ADR-0139 D4): one branch per parent object, no pre-scan + * + * The parent objects are every object the metadata registry holds, except + * `sys_attachment` itself. For each, one branch over the row's own pointer + * pair: + * + * { parent_object: P, parent_id: { $in: ReadableIds.of(P) } } + * + * ORed together and ANDed into the read's `where`. The engine composes each + * leaf at its executor boundary from the security service's `canReadObject` + * and `getReadFilter` for P (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. The gate reads no row for a leaf, and no parent as + * the caller: a parent object the caller may not read is answered `$in: []` + * by the engine's own admission, without a logged denial. + * + * Rows the leaf excludes, as the probe does: a row naming no parent, an object + * the registry does not hold, `sys_attachment` itself, or a parent record that + * no longer exists (the subquery finds no such id). + * + * ## The bounded probe (ADR-0139 D5): three classes, by name, chosen up front + * + * The engine refuses to compose a leaf for three classes (`INTERNAL_ERROR` / + * 500). This gate routes each to the bounded probe BEFORE any leaf is written + * — ⛔ never by catching that refusal: + * + * 1. **A delegated (on-behalf-of) caller** (ADR-0090 D10), by the engine's own + * test (`onBehalfOf.userId`): the whole read is the probe. + * 2. **A parent an object-scoped read middleware narrows** + * (`hasObjectMiddleware`): `sys_comment`, `sys_comment_reaction`, the + * activity stream and the ledger among them. Their WHERE is built at read + * time and is no part of their read scope, so a leaf would be wider than a + * direct read. + * 3. **A federated parent**, bound to another datasource than + * `sys_attachment` (`resolveEffectiveDatasource`): no driver evaluates a + * subquery across two datasources. + * + * 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`), and a + * security service that answers no `canReadObject` — or none at all. The + * engine would then admit a leaf's parent by `explain`, which answers a SYSTEM + * context "not allowed" where `canReadObject` admits it, and with no service it + * answers every leaf `$in: []`, hiding every attachment from every caller. The + * probe's literal id lists mean the same thing under every context. + * + * The probe, for the parents it is given (or for every row, for a whole-read + * probe): pre-scan the candidate `(parent_object, parent_id)` pairs the query + * would touch (system context, bounded at {@link READ_SCAN_LIMIT}), resolve the + * visible parent ids per `parent_object` through the caller-scoped engine + * (RLS/OWD/sharing of the PARENT apply), and emit one + * `{ parent_object, parent_id: { $in: } }` branch per parent object. + * Past the bound it fails CLOSED and says so; a probe that throws denies the + * whole read, and says so. Restricted to the parents of classes 2 and 3, its + * window holds only the rows naming those parents. + * + * ## What the read's own refusal does (ADR-0139 D3) + * + * A leaf is composed AFTER this middleware hands the read on (`next()`), so a + * composition or driver refusal (`driver-mongodb` refuses every leaf until + * ADR-0139's O3 is measured) is the read's own failure: it reaches the caller + * and says why. ⛔ This middleware does not catch it into its deny-all + * sentinel, and it makes no engine call for a leaf branch, so nothing it does + * can meet one. */ export function installAttachmentReadVisibility( - engine: AttachmentLifecycleEngine, + engine: AttachmentReadGateEngine, logger: AttachmentLifecycleLogger, + getAdmission?: AttachmentReadAdmissionSource, ): void { if (typeof engine.registerMiddleware !== 'function') return; // engine lacks the seam const andIn = (ctx: AttachmentReadMiddlewareCtx, filter: unknown) => { @@ -724,65 +842,160 @@ export function installAttachmentReadVisibility( if (!READ_OPS.has(ctx.operation) || !ctx.ast || !ctx.context || ctx.context.isSystem) { return next(); } - try { - const filter = await computeParentVisibilityFilter(engine, ctx, logger); - if (filter) andIn(ctx, filter); - } catch (err) { - // A filter-compute failure must never fall open into a leak. - logger.warn( - `[storage] attachment read visibility: filter failed, denying all (${(err as Error)?.message ?? err})`, - ); - andIn(ctx, READ_DENY_ALL); - } + // [ADR-0139 D3] No catch: the probe's own failure is answered with the + // deny-all sentinel inside, and a `ReadableIds` leaf is composed by the + // engine after `next()`, so its refusal is this read's failure and + // reaches the caller saying why — ⛔ never turned into deny-all here. + andIn(ctx, await computeParentVisibilityFilter(engine, ctx, logger, getAdmission)); return next(); }, - { object: 'sys_attachment' }, + { object: ATTACHMENT_OBJECT }, ); } /** - * Resolve the parent-visibility WHERE predicate for one sys_attachment read. - * Returns `null` when the query matches no rows (nothing to narrow), a - * `$or` of visible-parent clauses, or the deny-all sentinel. + * 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. */ -async function computeParentVisibilityFilter( - engine: AttachmentLifecycleEngine, +function isDelegatedCaller(context: AttachmentReadMiddlewareCtx['context']): boolean { + return Boolean((context as { onBehalfOf?: { userId?: unknown } } | undefined)?.onBehalfOf?.userId); +} + +/** Whether the security service `getAdmission` resolves answers `canReadObject`. */ +function hasObjectAdmission(getAdmission: AttachmentReadAdmissionSource | undefined): boolean { + try { + return typeof getAdmission?.()?.canReadObject === 'function'; + } catch { + // `getService` throws on an empty slot: no security service is composed. + return false; + } +} + +/** + * [ADR-0139 D4, D5] How one read of `sys_attachment` by `context` reaches its + * parents. Reads the context, whether a `canReadObject` is composed, the + * registry, the middleware registrations and the datasource bindings only — it + * asks the security service nothing and reads no row, so it decides the probe + * up front, never by catching a refusal. + */ +function planAttachmentParentReads( + engine: AttachmentReadGateEngine, + context: AttachmentReadMiddlewareCtx['context'], + objectAdmission: boolean, +): AttachmentReadPlan { + if (isDelegatedCaller(context)) return { probeWholeRead: 'delegated-caller' }; + if (!objectAdmission) return { probeWholeRead: 'no-object-admission' }; + const getAllObjects = engine.registry?.getAllObjects; + const hasObjectMiddleware = engine.hasObjectMiddleware; + const datasourceOf = engine.resolveEffectiveDatasource; + if (typeof getAllObjects !== 'function' || typeof hasObjectMiddleware !== 'function' || typeof datasourceOf !== 'function') { + return { probeWholeRead: 'engine-cannot-classify' }; + } + const parents = 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) || name === ATTACHMENT_OBJECT) continue; + parents.add(name); + } + // `undefined` is "rides the default driver", for `sys_attachment` and a + // parent alike. An explicit binding to the default driver's own name reads + // as another datasource here, which sends that parent to the probe — the + // bound, never a refused read. + const outer = datasourceOf.call(engine, ATTACHMENT_OBJECT); + const leaves: string[] = []; + const probed = new Map(); + for (const parent of [...parents].sort()) { + 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 whole-read probe states already reported, per engine. */ +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() or answered $in: [] for every caller', +}; + +function reportWholeReadProbeOnce( + engine: object, + logger: AttachmentLifecycleLogger, + why: 'engine-cannot-classify' | 'no-object-admission', +): void { + let reported = wholeReadProbeReported.get(engine); + if (!reported) wholeReadProbeReported.set(engine, (reported = new Set())); + if (reported.has(why)) return; + reported.add(why); + logger.warn( + `[storage] attachment read visibility: ${WHOLE_READ_PROBE_WHY[why]}: every read is answered by the bounded ` + + `probe (at most ${READ_SCAN_LIMIT} candidate rows, fail-closed). Reported once for this engine.`, + ); +} + +/** "object-middleware: a, b; federated-parent: c" — how a restricted probe names itself. */ +function describeProbed(probed: ReadonlyMap): string { + const byClass = new Map(); + for (const [parent, why] of probed) byClass.set(why, [...(byClass.get(why) ?? []), parent]); + return [...byClass].map(([why, parents]) => `${why}: ${parents.join(', ')}`).join('; '); +} + +/** + * [ADR-0139 D5] The bounded probe's branches for one read: one `$in` of the + * visible parent ids per parent object, for the rows naming one of + * `restrictTo.parents` — or for every row when `restrictTo` is `null` (a + * whole-read probe). + */ +async function probeBranches( + engine: AttachmentReadGateEngine, ctx: AttachmentReadMiddlewareCtx, logger: AttachmentLifecycleLogger, -): Promise { + restrictTo: { parents: string[]; why: string } | null, +): Promise>> { // 1. Candidate (parent_object, parent_id) pairs the query would touch — // read under SYSTEM context (the caller may not see the rows yet; that // is exactly what we are deciding). Bypasses this middleware (isSystem). - const candidates = await engine.find('sys_attachment', { - where: (ctx.ast?.where as Record) ?? {}, + const callerWhere = ctx.ast?.where as Record | undefined; + const onlyParents = restrictTo ? { parent_object: { $in: [...restrictTo.parents] } } : undefined; + const where = onlyParents ? (callerWhere ? { $and: [callerWhere, onlyParents] } : onlyParents) : (callerWhere ?? {}); + const candidates = await engine.find(ATTACHMENT_OBJECT, { + where, fields: ['parent_object', 'parent_id'], limit: READ_SCAN_LIMIT, context: { ...SYSTEM_CTX }, }); - if (!candidates.length) return null; + if (!candidates.length) return []; if (candidates.length >= READ_SCAN_LIMIT) { // Not silent (fail-closed truncation): rows beyond the scan window are - // excluded from the visibility filter, so a very broad unscoped list may - // omit some rows the caller could see. The panel scopes to one parent so - // never hits this; a global list should paginate by parent_object. + // excluded from the visibility filter, so a very broad read may omit some + // rows the caller could see. The panel scopes to one parent so never hits + // this; a broad read past it should scope by parent_object. logger.warn( - `[storage] attachment read visibility: candidate pre-scan hit the ${READ_SCAN_LIMIT}-row cap; ` + - 'the visibility filter for this broad read is fail-closed and may omit visible rows — scope the query by parent_object', + `[storage] attachment read visibility: the bounded probe's candidate pre-scan ` + + `(${restrictTo ? restrictTo.why : 'the whole read'}) hit the ${READ_SCAN_LIMIT}-row cap; the visibility ` + + 'filter for these rows is fail-closed and may omit visible rows — scope the query by parent_object', ); } + const allowed = restrictTo ? new Set(restrictTo.parents) : null; const byObject = new Map>(); for (const row of candidates) { // Skip rows that name no readable parent — self-referential rows included // (no valid files-enabled target is sys_attachment), which also prevents // caller-scoped re-entry into this middleware during the probe below. const parent = attachmentParentOf(row); - if (!parent) continue; + if (!parent || (allowed && !allowed.has(parent.object))) continue; let ids = byObject.get(parent.object); if (!ids) byObject.set(parent.object, (ids = new Set())); ids.add(parent.id); } - if (byObject.size === 0) return READ_DENY_ALL; + if (byObject.size === 0) return []; // 2. Per parent_object, the visible id subset via the CALLER's context — // the parent object's own RLS/OWD/sharing applies. The write gate's @@ -793,14 +1006,65 @@ async function computeParentVisibilityFilter( ctx.context as Record | undefined, byObject, ); - const clauses: Array> = []; + const branches: Array> = []; for (const parentObject of byObject.keys()) { const visible = [...(visibleByObject.get(parentObject) ?? [])]; - if (visible.length) { - clauses.push({ parent_object: parentObject, parent_id: { $in: visible } }); + if (visible.length) branches.push({ parent_object: parentObject, parent_id: { $in: visible } }); + } + return branches; +} + +/** + * The parent-visibility WHERE for one sys_attachment read: one branch per + * enumerated parent object — a `ReadableIds` leaf, or the bounded probe's + * `$in` of visible ids for D5's classes — ORed; or the deny-all sentinel when + * no branch remains, or when the probe fails. + * + * Never throws for a fault of its probe: that 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 (ADR-0139 D3). + */ +async function computeParentVisibilityFilter( + engine: AttachmentReadGateEngine, + ctx: AttachmentReadMiddlewareCtx, + logger: AttachmentLifecycleLogger, + getAdmission: AttachmentReadAdmissionSource | undefined, +): Promise { + // 1. Leaf or probe, per parent object, before any branch is written. + const plan = planAttachmentParentReads(engine, ctx.context, hasObjectAdmission(getAdmission)); + if (plan.probeWholeRead === 'engine-cannot-classify' || plan.probeWholeRead === 'no-object-admission') { + reportWholeReadProbeOnce(engine, logger, plan.probeWholeRead); + } + + const 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({ parent_object: parent, parent_id: { $in: ReadableIds.of(parent) } }); + } + } + + // 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, restrictTo))); + } catch (err) { + // A probe failure must never fall open into a leak. + logger.warn( + `[storage] attachment read visibility: the bounded probe ` + + `(${restrictTo ? restrictTo.why : `the whole read: ${plan.probeWholeRead}`}) failed, denying all ` + + `(${(err as Error)?.message ?? err})`, + ); + return READ_DENY_ALL; } } - if (clauses.length === 0) return READ_DENY_ALL; - return clauses.length === 1 ? clauses[0] : { $or: clauses }; + if (branches.length === 0) return READ_DENY_ALL; + return branches.length === 1 ? branches[0] : { $or: branches }; } diff --git a/packages/services/service-storage/src/attachment-read-leaf.integration.test.ts b/packages/services/service-storage/src/attachment-read-leaf.integration.test.ts new file mode 100644 index 00000000000..3f82990ea50 --- /dev/null +++ b/packages/services/service-storage/src/attachment-read-leaf.integration.test.ts @@ -0,0 +1,409 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0139 E7] The `sys_attachment` read gate on the LEAF path, through a + * real engine and a real SQL driver: one `ReadableIds` leaf per parent object, + * no pre-scan, so a broad read of `sys_attachment` is served every row the + * caller may read, with a true total, past the old 2,000-row window — and + * D5's three classes still read through the bounded probe, by name. + * + * ## Why a real engine and a real driver + * + * 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: `ObjectQLPlugin` wires the security service into the + * composition, the rows are stored by `SqlDriver` (sqlite `:memory:`), and + * every read goes through the real middleware chain. The gate is installed + * exactly as `StorageServicePlugin` installs it, with the security service + * resolved per read. + * + * ## 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. The stand-in keeps those two answers in + * parity (`getReadFilter`'s contract: "the same filter the engine middleware + * ANDs into every find"): 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: + * + * - `e7_doc`: a member reads the records it owns; the admin reads them all; + * - `e7_vault`: no member may read it (`canReadObject` false; a direct read + * is refused and the refusal recorded); the admin may; + * - every other object: every caller reads every record. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectKernel, findMisplacedReadableIds, refuseReadableIdsOnce } from '@objectstack/core'; +import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SysAttachment } from '@objectstack/platform-objects'; +import type { ISecurityService } from '@objectstack/spec/contracts'; +import { installAttachmentReadVisibility } from './attachment-access-hooks.js'; + +const ATTACHMENT = 'sys_attachment'; +/** Owner-scoped: a member reads its own records. */ +const DOC = 'e7_doc'; +/** No member may read it. */ +const VAULT = 'e7_vault'; +/** Narrowed by an object-scoped read middleware: D5's second class. */ +const GATED = 'e7_gated'; +/** Bound to another datasource: D5's third class. */ +const FAR = 'e7_far'; +const HARNESS_PACKAGE = 'com.objectstack.service.storage.test.attachment-leaf'; + +/** The bounded probe's window (`READ_SCAN_LIMIT` in attachment-access-hooks.ts). Mirrored, not imported. */ +const SCAN_LIMIT = 2_000; + +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 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; +const BATCH = 200; + +const READ_OPS = new Set(['find', 'findOne', 'count', 'aggregate']); +const isAdmin = (context: any) => context?.userId === ADMIN.userId; + +type Row = Record; + +function readScopeOf(object: string, context: any): Record | undefined { + if (context?.isSystem || isAdmin(context)) return undefined; + if (object === DOC) return { owner: context?.userId ?? '__nobody__' }; + return undefined; +} +const mayReadObject = (object: string, context: any) => context?.isSystem || object !== VAULT || isAdmin(context); + +/** Reads of a parent object as a caller (not the system): 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); + }, +}; + +const sqlite = () => new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + +class WarehouseDriver extends SqlDriver { + 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, +}); + +const silent = { info() {}, warn() {}, debug() {} }; + +async function bootKernel(driver: SqlDriver) { + const kernel = new ObjectKernel({ logger: { level: 'silent' } }); + await kernel.use(new ObjectQLPlugin()); + 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 as any, true); + engine.registry.registerObject(SysAttachment as any, HARNESS_PACKAGE); + 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 !== ATTACHMENT) { + 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 { + const out: Row[] = []; + for (let i = 0; i < rows.length; i += BATCH) { + const made = await engine.insert(object, rows.slice(i, i + BATCH) as any, { context: SYS }); + out.push(...(made as unknown as Row[])); + } + return out; +} + +const attachmentTo = (parentObject: string, parentId: unknown, name = 'f.txt') => ({ + parent_object: parentObject, + parent_id: String(parentId), + file_id: 'file_shared', + file_name: name, +}); + +describe('[ADR-0139 E7] the attachment read gate on the leaf path', () => { + let kernel: ObjectKernel; + let engine: ObjectQL; + /** The WHERE each gated read reached the driver boundary with. */ + const seenWhere: unknown[] = []; + const ids: Record = {}; + + /** Ground truth of who may read which attachment, judged by the stand-in's own rules. */ + const truth = async (reader: any): Promise => { + const atRest: Row[] = await engine.find(ATTACHMENT, { 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]))); + } + return atRest + .filter((row) => { + const parent = live.get(row.parent_object)?.get(String(row.parent_id)); + if (!parent) return false; + if (isAdmin(reader)) return true; + if (row.parent_object === VAULT) return false; + if (row.parent_object === DOC || row.parent_object === GATED) return parent.owner === reader.userId; + return true; + }) + .map((r) => String(r.id)) + .sort(); + }; + const served = async (context: any, extra: Record = {}) => + ((await engine.find(ATTACHMENT, { limit: 100_000, ...extra, context })) as Row[]).map((r) => String(r.id)).sort(); + + beforeAll(async () => { + ({ kernel, engine } = await bootKernel(sqlite())); + const warehouse = new WarehouseDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + await warehouse.connect(); + engine.registerDriver(warehouse as any); + 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 }); + // The gate, as StorageServicePlugin installs it: the security service resolved per read. + installAttachmentReadVisibility(engine as any, silent, () => kernel.getService('security')); + // Registered after the gate, so it sees each gated read's WHERE as the gate left it. + engine.registerMiddleware(async (op: any, next: () => Promise) => { + if (READ_OPS.has(op.operation) && op.context && !op.context.isSystem) seenWhere.push(op.ast?.where); + return next(); + }, { object: ATTACHMENT }); + + // The parents, then one attachment per parent 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 }); + const madeDocs = await insertInBatches(engine, DOC, docs); + await insertInBatches(engine, ATTACHMENT, madeDocs.map((d) => attachmentTo(DOC, d.id))); + ids.gone = (await engine.insert(DOC, { name: 'gone', owner: MEMBER_A.userId }, { context: SYS })).id; + ids.goneAttachment = (await engine.insert(ATTACHMENT, attachmentTo(DOC, ids.gone), { 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.vaultAttachment = (await engine.insert(ATTACHMENT, attachmentTo(VAULT, ids.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.gatedAttachmentA = (await engine.insert(ATTACHMENT, attachmentTo(GATED, ids.gatedA), { context: SYS })).id; + ids.gatedAttachmentB = (await engine.insert(ATTACHMENT, attachmentTo(GATED, ids.gatedB), { context: SYS })).id; + ids.far = (await engine.insert(FAR, { name: 'far' }, { context: SYS })).id; + ids.farAttachment = (await engine.insert(ATTACHMENT, attachmentTo(FAR, ids.far), { context: SYS })).id; + // Rows naming no readable parent: a dangling id, an object the registry does not hold, the gated object itself. + ids.dangling = (await engine.insert(ATTACHMENT, attachmentTo(DOC, 'no_such_doc'), { context: SYS })).id; + ids.unregistered = (await engine.insert(ATTACHMENT, attachmentTo('e7_nowhere', 'x1'), { context: SYS })).id; + ids.selfRef = (await engine.insert(ATTACHMENT, attachmentTo(ATTACHMENT, ids.dangling), { context: SYS })).id; + }, 180_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, for the member and in total', async () => { + const atRest: Row[] = await engine.find(ATTACHMENT, { limit: 100_000, context: SYS }); + expect(atRest.length).toBeGreaterThan(SCAN_LIMIT); + expect((await truth(MEMBER_A)).length).toBeGreaterThan(SCAN_LIMIT); + expect((await truth(MEMBER_A)).length).toBe(DOCS_OF_A + 1 + 1); // own docs, own gated, the far one + }); + + it('a member is served every attachment it may read past the old window, and the total agrees', async () => { + const expected = await truth(MEMBER_A); + expect(await served(MEMBER_A)).toEqual(expected); + expect(await engine.count(ATTACHMENT, {}, { context: MEMBER_A })).toBe(expected.length); + const expectedB = await truth(MEMBER_B); + expect(await served(MEMBER_B)).toEqual(expectedB); + expect(await engine.count(ATTACHMENT, {}, { context: MEMBER_B })).toBe(expectedB.length); + }); + + it('an administrator’s page of 20 is 20 rows over a true total, not the window', async () => { + const page: Row[] = await engine.find(ATTACHMENT, { limit: 20, context: ADMIN }); + expect(page).toHaveLength(20); + const total = await engine.count(ATTACHMENT, {}, { context: ADMIN }); + expect(total).toBe((await truth(ADMIN)).length); + expect(total).toBeGreaterThan(SCAN_LIMIT); + }); + + it('no row is served whose parent the reader may not read, that no longer exists, or that names none', async () => { + const hidden = [ids.vaultAttachment, ids.goneAttachment, ids.dangling, ids.unregistered, ids.selfRef, ids.gatedAttachmentB]; + const asA = new Set(await served(MEMBER_A)); + for (const id of hidden) expect(asA.has(String(id))).toBe(false); + const asAdmin = new Set(await served(ADMIN)); + expect(asAdmin.has(String(ids.vaultAttachment))).toBe(true); + for (const id of [ids.goneAttachment, ids.dangling, ids.unregistered, ids.selfRef]) expect(asAdmin.has(String(id))).toBe(false); + }); + + 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(MEMBER_A); + await engine.count(ATTACHMENT, {}, { 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.length = 0; + await served(MEMBER_A); + const branches: any[] = (seenWhere[0] as any).$or; + const leaves = branches.filter((b) => b?.parent_id?.$in && !Array.isArray(b.parent_id.$in)).map((b) => b.parent_object); + expect(leaves).toEqual(expect.arrayContaining([DOC, VAULT])); + expect(leaves).not.toContain(GATED); + expect(leaves).not.toContain(FAR); + expect(leaves).not.toContain(ATTACHMENT); + const probed = branches.filter((b) => Array.isArray(b?.parent_id?.$in)); + expect(probed).toEqual( + expect.arrayContaining([ + { parent_object: GATED, parent_id: { $in: [String(ids.gatedA)] } }, + { parent_object: FAR, parent_id: { $in: [String(ids.far)] } }, + ]), + ); + expect(probed).toHaveLength(2); + }); + + it('D5: a delegated caller reads through the bounded probe — no leaf — and is not refused', async () => { + seenWhere.length = 0; + const rows = await served(DELEGATED_A); + const where = seenWhere[0] as any; + const branches: any[] = where?.$or ?? [where]; + expect(branches.every((b) => Array.isArray(b?.parent_id?.$in))).toBe(true); + // The probe's window: bounded, fail-closed, never wider than the truth. + expect(rows.length).toBeLessThanOrEqual(SCAN_LIMIT); + const expected = new Set(await truth(MEMBER_A)); + for (const id of rows) expect(expected.has(id)).toBe(true); + expect(await engine.count(ATTACHMENT, {}, { context: DELEGATED_A })).toBe(rows.length); + }); + + it('[ADR-0139 O2] the bound case, recorded: rows at rest, served, total, branches and time', async () => { + seenWhere.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(ATTACHMENT, { limit: 100_000, context: SYS })).length, + memberTruth: (await truth(MEMBER_A)).length, + memberServed: (await served(MEMBER_A)).length, + memberTotal: await engine.count(ATTACHMENT, {}, { context: MEMBER_A }), + adminTotal: await engine.count(ATTACHMENT, {}, { context: ADMIN }), + ms: { + memberFind: await time(() => served(MEMBER_A)), + memberCount: await time(() => engine.count(ATTACHMENT, {}, { context: MEMBER_A })), + adminTop20: await time(() => engine.find(ATTACHMENT, { limit: 20, context: ADMIN })), + delegatedFind: await time(() => served(DELEGATED_A)), + }, + branches: 0, + leaves: 0, + }; + const where = seenWhere[0] as any; + reading.branches = where?.$or?.length ?? 1; + reading.leaves = (where?.$or ?? [where]).filter((b: any) => b?.parent_id?.$in && !Array.isArray(b.parent_id.$in)).length; + // A reading, never a verdict on a duration (AGENTS.md). + console.log(`E7-ATTACHMENT-READING ${JSON.stringify(reading)}`); + expect(reading.rowsAtRest).toBeGreaterThan(SCAN_LIMIT); + }); +}); + +/** A driver that does not compile the composed leaf, refusing it with D3's envelope, as `driver-mongodb` does. */ +class RefusingDriver extends SqlDriver { + private readonly refusalState = { logged: false }; + private refuse(query: any): void { + const site = findMisplacedReadableIds(query?.where, false); + if (site) throw refuseReadableIdsOnce('driver-e7-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 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 attachment read fails and says why', () => { + let kernel: ObjectKernel; + let engine: ObjectQL; + + beforeAll(async () => { + ({ kernel, engine } = await bootKernel(new RefusingDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }))); + engine.registry.registerObject(objectOf(DOC) as any, HARNESS_PACKAGE); + await engine.syncSchemas(); + installAttachmentReadVisibility(engine as any, silent, () => kernel.getService('security')); + const doc = await engine.insert(DOC, { name: 'one', owner: MEMBER_A.userId }, { context: SYS }); + await engine.insert(ATTACHMENT, attachmentTo(DOC, doc.id), { 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 row is there, and a system read is served it', async () => { + expect((await engine.find(ATTACHMENT, { context: SYS })).length).toBe(1); + }); + + it('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(ATTACHMENT, { context: MEMBER_A }), + () => engine.count(ATTACHMENT, {}, { 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-e7-refusing]'); + } + }); +}); diff --git a/packages/services/service-storage/src/attachment-read-leaf.test.ts b/packages/services/service-storage/src/attachment-read-leaf.test.ts new file mode 100644 index 00000000000..482f7051405 --- /dev/null +++ b/packages/services/service-storage/src/attachment-read-leaf.test.ts @@ -0,0 +1,288 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0139 E7, D4, D5] The `sys_attachment` read gate's choice between a + * `ReadableIds` leaf and the bounded probe, per parent object, decided before + * any branch is written — against a stand-in engine that answers the three + * classifying questions (`registry.getAllObjects`, `hasObjectMiddleware`, + * `resolveEffectiveDatasource`) and records every read the gate makes. + * + * What a leaf SELECTS is the engine's and the driver's (composition and the + * subquery), so it is measured on a real engine and driver in + * `attachment-read-leaf.integration.test.ts`; this file pins which branch each + * parent gets, that a leaf costs no read at all, and the probe states. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { isReadableIds } from '@objectstack/spec/data'; +import { installAttachmentReadVisibility } from './attachment-access-hooks.js'; +import type { AttachmentReadMiddlewareCtx } from './attachment-lifecycle.js'; + +type Mw = (ctx: AttachmentReadMiddlewareCtx, next: () => Promise) => Promise; +type Row = { id: string; parent_object: string; parent_id: string }; + +const DENY_ALL = { id: '__attachment_parent_denied__' }; + +function recordingLogger() { + return { info: vi.fn(), warn: vi.fn(), debug: vi.fn() }; +} + +/** Filter Protocol subset the probe's pre-scan uses: AND of keys, `$and`, `$in`, equality. */ +function matches(row: Record, where: any): boolean { + if (!where) return true; + return Object.entries(where).every(([key, val]: [string, any]) => { + if (key === '$and') return (val as any[]).every((w) => matches(row, w)); + if (key === '$or') return (val as any[]).some((w) => matches(row, w)); + if (val && typeof val === 'object' && Array.isArray(val.$in)) return val.$in.map(String).includes(String(row[key])); + return String(row[key]) === String(val); + }); +} + +function boot(fixture: { + rows?: Row[]; + objects?: string[]; + /** Parents an object-scoped read middleware narrows (D5.2). */ + middlewareGated?: string[]; + /** Parent → its datasource (D5.3); absent rides the default driver. */ + datasources?: Record; + /** parent object → visible ids for any caller. */ + visible?: Record; + /** What the security source answers; `'throws'` = no service composed. */ + admission?: 'can-read-object' | 'explain-only' | 'none' | 'throws' | 'omitted'; + /** Drop one classifying question from the engine. */ + engineLacks?: 'registry' | 'hasObjectMiddleware' | 'resolveEffectiveDatasource'; + failPreScan?: boolean; +}) { + const rows = fixture.rows ?? []; + const objects = fixture.objects ?? ['att_case', 'att_secret', 'sys_attachment', 'sys_comment']; + const gated = new Set(fixture.middlewareGated ?? []); + const reads = { preScans: [] as any[], parentReads: [] as Array<{ object: string; ids: string[] }> }; + let mw!: Mw; + // Only the members the read gate calls: `find` (its probe) and the three + // classifying questions. The write verbs are not part of this gate. + const engine: any = { + registerHook: () => {}, + registerMiddleware: (fn: Mw) => { + mw = fn; + }, + find: async (object: string, query: any) => { + if (object === 'sys_attachment') { + reads.preScans.push(query); + if (fixture.failPreScan === true) throw new Error('driver down'); + const hits = rows.filter((r) => matches(r, query?.where)); + return typeof query?.limit === 'number' ? hits.slice(0, query.limit) : hits; + } + const ids: string[] = (query?.where?.id?.$in ?? []).map(String); + reads.parentReads.push({ object, ids }); + const visible = ids.filter((id) => (fixture.visible?.[object] ?? []).includes(id)).map((id) => ({ id })); + return typeof query?.limit === 'number' ? visible.slice(0, query.limit) : visible; + }, + registry: { getAllObjects: () => objects.map((name) => ({ name })) }, + hasObjectMiddleware: (name: string) => gated.has(name), + resolveEffectiveDatasource: (name: string) => fixture.datasources?.[name], + }; + if (fixture.engineLacks === 'registry') delete engine.registry; + if (fixture.engineLacks === 'hasObjectMiddleware') delete engine.hasObjectMiddleware; + if (fixture.engineLacks === 'resolveEffectiveDatasource') delete engine.resolveEffectiveDatasource; + + const admission = fixture.admission ?? 'can-read-object'; + const getAdmission = + admission === 'omitted' + ? undefined + : () => { + if (admission === 'throws') throw new Error("service 'security' not found"); + if (admission === 'none') return undefined; + if (admission === 'explain-only') return { explain: async () => ({ allowed: true }) } as any; + return { canReadObject: async () => true }; + }; + const logger = recordingLogger(); + installAttachmentReadVisibility(engine, logger, getAdmission); + return { mw, reads, logger }; +} + +async function read(mw: Mw, partial: Partial = {}, next?: () => Promise) { + const ctx: AttachmentReadMiddlewareCtx = { + object: 'sys_attachment', + operation: 'find', + ast: { object: 'sys_attachment', where: undefined }, + context: { userId: 'u1' }, + ...partial, + }; + await mw(ctx, next ?? (async () => {})); + return ctx.ast?.where as any; +} + +/** The branches of an emitted WHERE: its `$or`, or the one branch. */ +const branchesOf = (where: any): any[] => (where?.$or ? where.$or : [where]); +const leafBranches = (where: any) => branchesOf(where).filter((b) => isReadableIds(b?.parent_id?.$in)); +const literalBranches = (where: any) => branchesOf(where).filter((b) => Array.isArray(b?.parent_id?.$in)); + +describe('[ADR-0139 D4] the attachment read gate writes one ReadableIds leaf per parent object', () => { + it('one leaf per registered parent object, over the row’s own pointer pair, and no read at all', async () => { + const { mw, reads } = boot({}); + const where = await read(mw); + expect(where.$or).toHaveLength(3); + for (const [i, parent] of ['att_case', 'att_secret', 'sys_comment'].entries()) { + const branch = where.$or[i]; + expect(Object.keys(branch).sort()).toEqual(['parent_id', 'parent_object']); + expect(branch.parent_object).toBe(parent); + expect(isReadableIds(branch.parent_id.$in)).toBe(true); + expect(branch.parent_id.$in.object).toBe(parent); + } + // No pre-scan, no caller-scoped parent read: the engine composes the leaf. + expect(reads.preScans).toHaveLength(0); + expect(reads.parentReads).toHaveLength(0); + }); + + it('sys_attachment is never a parent of its own rows', async () => { + const { mw } = boot({}); + const where = await read(mw); + expect(branchesOf(where).map((b) => b.parent_object)).not.toContain('sys_attachment'); + }); + + it('a single parent object is a bare branch, ANDed onto the caller’s own where', async () => { + const { mw } = boot({ objects: ['att_case', 'sys_attachment'] }); + const own = { mime_type: 'application/pdf' }; + const where = await read(mw, { ast: { object: 'sys_attachment', where: own } }); + expect(where.$and[0]).toBe(own); + expect(where.$and[1].parent_object).toBe('att_case'); + expect(isReadableIds(where.$and[1].parent_id.$in)).toBe(true); + }); + + it('a list and its total get the same WHERE', async () => { + const { mw } = boot({}); + const list = JSON.stringify(branchesOf(await read(mw)).map((b) => [b.parent_object, b.parent_id.$in.object])); + const total = JSON.stringify( + branchesOf(await read(mw, { operation: 'count' })).map((b) => [b.parent_object, b.parent_id.$in.object]), + ); + expect(total).toBe(list); + }); + + it('a registry holding no parent object leaves deny-all, never an unfiltered read', async () => { + const { mw } = boot({ objects: ['sys_attachment'] }); + expect(await read(mw)).toEqual(DENY_ALL); + }); +}); + +describe('[ADR-0139 D5] three classes keep the bounded probe, chosen before any leaf is written', () => { + const rows: Row[] = [ + { id: 'a1', parent_object: 'sys_comment', parent_id: 'c1' }, + { id: 'a2', parent_object: 'sys_comment', parent_id: 'c2' }, + { id: 'a3', parent_object: 'att_far', parent_id: 'f1' }, + { id: 'a4', parent_object: 'att_case', parent_id: 'k1' }, + ]; + + it('D5.2: a parent an object-scoped read middleware narrows is probed, its pre-scan restricted to its rows', async () => { + const { mw, reads } = boot({ rows, middlewareGated: ['sys_comment'], visible: { sys_comment: ['c1'] } }); + const where = await read(mw); + expect(leafBranches(where).map((b) => b.parent_object)).toEqual(['att_case', 'att_secret']); + expect(literalBranches(where)).toEqual([{ parent_object: 'sys_comment', parent_id: { $in: ['c1'] } }]); + expect(reads.preScans).toHaveLength(1); + expect(reads.preScans[0].where).toEqual({ parent_object: { $in: ['sys_comment'] } }); + expect(reads.preScans[0].limit).toBe(2_000); + expect(reads.preScans[0].context).toEqual({ isSystem: true }); + expect(reads.parentReads).toEqual([{ object: 'sys_comment', ids: ['c1', 'c2'] }]); + }); + + it('D5.3: a parent bound to another datasource is probed beside the leaves', async () => { + const { mw, reads } = boot({ + rows, + objects: ['att_case', 'att_far', 'sys_attachment'], + datasources: { att_far: 'warehouse' }, + visible: { att_far: ['f1'] }, + }); + const where = await read(mw); + expect(leafBranches(where).map((b) => b.parent_object)).toEqual(['att_case']); + expect(literalBranches(where)).toEqual([{ parent_object: 'att_far', parent_id: { $in: ['f1'] } }]); + expect(reads.preScans[0].where).toEqual({ parent_object: { $in: ['att_far'] } }); + }); + + it('D5.3: sys_attachment on its own datasource makes every default-driver parent federated', async () => { + const { mw } = boot({ rows, objects: ['att_case', 'sys_attachment'], datasources: { sys_attachment: 'files' }, visible: { att_case: ['k1'] } }); + const where = await read(mw); + expect(leafBranches(where)).toEqual([]); + expect(where).toEqual({ parent_object: 'att_case', parent_id: { $in: ['k1'] } }); + }); + + it('the restricted probe keeps the caller’s own where in its pre-scan', async () => { + const { mw, reads } = boot({ rows, middlewareGated: ['sys_comment'], visible: { sys_comment: ['c1', 'c2'] } }); + const own = { file_name: 'x.txt' }; + await read(mw, { ast: { object: 'sys_attachment', where: own } }); + expect(reads.preScans[0].where).toEqual({ $and: [own, { parent_object: { $in: ['sys_comment'] } }] }); + }); + + it('D5.1: a delegated caller reads through the whole-read probe — no leaf at all', async () => { + const { mw, reads } = boot({ rows, visible: { att_case: ['k1'], sys_comment: ['c2'] } }); + const where = await read(mw, { context: { userId: 'agent_1', onBehalfOf: { userId: 'u1' } } as any }); + expect(leafBranches(where)).toEqual([]); + expect(where).toEqual({ + $or: [ + { parent_object: 'sys_comment', parent_id: { $in: ['c2'] } }, + { parent_object: 'att_case', parent_id: { $in: ['k1'] } }, + ], + }); + // The whole read's window: the caller's where, unrestricted. + expect(reads.preScans[0].where).toEqual({}); + }); + + it('a restricted probe that throws denies the whole read and says so — the leaves are not served alone', async () => { + const { mw, logger } = boot({ rows, middlewareGated: ['sys_comment'], failPreScan: true }); + expect(await read(mw)).toEqual(DENY_ALL); + expect(logger.warn.mock.calls.map((c) => String(c[0])).join('\n')).toMatch( + /bounded probe \(object-middleware: sys_comment\) failed, denying all \(driver down\)/, + ); + }); + + it('a probe past the bound says which class it was', async () => { + const many: Row[] = Array.from({ length: 2_000 }, (_u, i) => ({ id: `g${i}`, parent_object: 'sys_comment', parent_id: `c${i}` })); + const { mw, logger } = boot({ rows: many, middlewareGated: ['sys_comment'], visible: { sys_comment: ['c1'] } }); + await read(mw); + const warned = logger.warn.mock.calls.map((c) => String(c[0])); + expect(warned.some((l) => l.includes('(object-middleware: sys_comment) hit the 2000-row cap') && l.includes('fail-closed'))).toBe(true); + }); +}); + +describe('[ADR-0139 E5’s guard] no leaf without canReadObject; an engine that cannot classify probes', () => { + const rows: Row[] = [{ id: 'a4', parent_object: 'att_case', parent_id: 'k1' }]; + + for (const admission of ['explain-only', 'none', 'throws', 'omitted'] as const) { + it(`security source ${admission}: the whole read is the probe, said once for the engine`, async () => { + const { mw, logger, reads } = boot({ rows, admission, visible: { att_case: ['k1'] } }); + expect(await read(mw)).toEqual({ parent_object: 'att_case', parent_id: { $in: ['k1'] } }); + await read(mw); + expect(reads.preScans).toHaveLength(2); + const said = logger.warn.mock.calls.map((c) => String(c[0])).filter((l) => l.includes('answers no canReadObject()')); + expect(said).toHaveLength(1); + }); + } + + for (const lacks of ['registry', 'hasObjectMiddleware', 'resolveEffectiveDatasource'] as const) { + it(`an engine without ${lacks} cannot classify: the whole read is the probe, said once`, async () => { + const { mw, logger } = boot({ rows, engineLacks: lacks, visible: { att_case: ['k1'] } }); + expect(await read(mw)).toEqual({ parent_object: 'att_case', parent_id: { $in: ['k1'] } }); + await read(mw); + const said = logger.warn.mock.calls.map((c) => String(c[0])).filter((l) => l.includes('cannot say which parent objects')); + expect(said).toHaveLength(1); + }); + } +}); + +describe('[ADR-0139 D3] the read’s own refusal reaches the caller', () => { + it('a refusal raised by next() (the composition, the driver) propagates — the WHERE keeps its leaves', async () => { + const { mw } = boot({}); + const refusal = Object.assign(new Error('[driver-mongodb] compiles no ReadableIds leaf'), { code: 'INTERNAL_ERROR', status: 500 }); + const ctx: AttachmentReadMiddlewareCtx = { + object: 'sys_attachment', + operation: 'find', + ast: { object: 'sys_attachment', where: undefined }, + context: { userId: 'u1' }, + }; + await expect( + mw(ctx, async () => { + throw refusal; + }), + ).rejects.toBe(refusal); + expect(ctx.ast?.where).not.toEqual(DENY_ALL); + expect(leafBranches(ctx.ast?.where)).toHaveLength(3); + }); +}); diff --git a/packages/services/service-storage/src/attachment-read-visibility.test.ts b/packages/services/service-storage/src/attachment-read-visibility.test.ts index 84a14e954eb..cf002145fcd 100644 --- a/packages/services/service-storage/src/attachment-read-visibility.test.ts +++ b/packages/services/service-storage/src/attachment-read-visibility.test.ts @@ -266,10 +266,15 @@ describe('installAttachmentReadVisibility', () => { expect(where).toEqual({ id: 'a1' }); // unchanged }); - it('leaves an already-empty result set alone (no candidates → no filter)', async () => { - const { mw } = install({ attachments: [], visible: {} }); + it('a read the probe finds no candidate for is narrowed to deny-all, never left unfiltered', async () => { + // [ADR-0139 E7] The probe no longer answers "nothing to narrow" when its + // pre-scan finds no row: an unfiltered read is exactly what a row written + // between the pre-scan and the read would leak through. Narrowing only — + // the pre-scan found nothing, so nothing is lost. + const { mw, calls } = install({ attachments: [], visible: {} }); const { where } = await runRead(mw, {}); - expect(where).toBeUndefined(); + expect(where).toEqual({ id: '__attachment_parent_denied__' }); + expect(calls.parentFinds).toHaveLength(0); }); it('ignores self-referential (parent_object=sys_attachment) rows — prevents probe re-entry', async () => { diff --git a/packages/services/service-storage/src/storage-service-plugin.ts b/packages/services/service-storage/src/storage-service-plugin.ts index 2d72bc4509a..b7520fbaebb 100644 --- a/packages/services/service-storage/src/storage-service-plugin.ts +++ b/packages/services/service-storage/src/storage-service-plugin.ts @@ -475,8 +475,18 @@ export class StorageServicePlugin implements Plugin { // Parent-derived READ visibility (#2970 item 1) — list/find/count of // sys_attachment only returns rows whose parent record the caller can // read. Middleware (not a hook) so list `total` is filtered too. + // [ADR-0139 D4, D5] The gate writes a `ReadableIds` leaf per parent + // object only when the security service answers `canReadObject`; + // resolved on every read (the security plugin may start after this + // one), and without it every read takes the bounded probe. if (typeof (engine as any).registerMiddleware === 'function') { - installAttachmentReadVisibility(engine as any, ctx.logger); + installAttachmentReadVisibility(engine as any, ctx.logger, () => { + try { + return ctx.getService('security'); + } catch { + return undefined; + } + }); } // Field-reference ownership (ADR-0104 D3 wave 2) — keeps // sys_file.ref_object/ref_id/ref_field in step with what records hold,