Skip to content

Commit 3999156

Browse files
committed
docs(qa): the read-inherits-parent-rls checklist item names the limits left after the attachment leaf; the changeset declares Clause-② yes
The checklist item's knownGaps and automated.ref described the 2,000-row fail-closed cap for a broad read, which an ordinary parent no longer meets. They now name what remains: ADR-0139 D5's three classes keep the bounded probe, and driver-mongodb answers INTERNAL_ERROR / 500 until O3. The re-exported installAttachmentReadVisibility takes an optional third argument, so the changeset's declaration reads Clause-② yes (contract record on the PR). Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude <noreply@anthropic.com>
1 parent a664ab3 commit 3999156

2 files changed

Lines changed: 11 additions & 5 deletions

File tree

‎.changeset/23732-attachment-readable-ids.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
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)
66

7-
Clause-②: no
7+
Clause-②: yes
88

99
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.
1010

‎docs/qa/platform-checklist/areas/attachments-storage.json‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@
267267
"title": "sys_attachment reads inherit parent visibility: a restricted member sees neither rows nor counts for invisible parents",
268268
"since": "v15.1",
269269
"status": "active",
270-
"revision": 3,
270+
"revision": 4,
271271
"priority": "P1",
272272
"surface": "api",
273273
"personas": ["member who can read the parent record", "member who cannot read the parent record"],
@@ -283,7 +283,7 @@
283283
"knownGaps": [
284284
"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.",
285285
"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.",
286-
"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."
286+
"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."
287287
]
288288
},
289289
"steps": [
@@ -325,10 +325,10 @@
325325
"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)"
326326
],
327327
"traps": ["wrong-persona"],
328-
"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." },
328+
"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." },
329329
"source": [
330330
"docs/plans/release-15.1-test-plan.md §C3 (#2970)",
331-
"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)",
331+
"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)",
332332
"packages/services/service-storage/src/attachment-read-visibility.test.ts"
333333
],
334334
"history": [
@@ -339,6 +339,12 @@
339339
"date": "2026-08-18",
340340
"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.",
341341
"ref": "#9483"
342+
},
343+
{
344+
"revision": 4,
345+
"date": "2026-10-16",
346+
"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.",
347+
"ref": "#23767"
342348
}
343349
]
344350
},

0 commit comments

Comments
 (0)