tests(attachments): pin the acl=public_read opt-out and the parent-RLS count/fail-closed clauses, and re-scope all three refs

[os-zhuang]

Part of #9483

QA run #9401 scored three attachments-storage items PARTIAL on pins that reach only a subset of their acceptance clauses. This lands the pins that are honestly automatable and re-scopes all three items' automated.ref, including what stays manual and what turned out not to be pinnable at all. The second half is the deliverable, not a footnote: a pin that covers more while the ref still claims the old scope leaves the same dishonesty pointing the other way.

Part of, not a closing keyword: one of the three clause sets cannot be pinned without a product change (see section 3). The card should stay open until that is decided.

Tests and checklist prose only. No runtime behaviour, wire shape, or accept/reject decision changes.

1. download-authz-both-sides C3 — the acl=public_read opt-out

packages/qa/dogfood/test/attachments-public-read-acl.dogfood.test.ts (8 tests).

The existing matrix proves the CLOSED side only — 401 anonymous, 403 parent-invisible, 200 for the entitled. C3 is the OPEN side, and it is the half a deny-only suite structurally cannot see: a download-authz suite made only of refusals stays green on a surface that has started refusing everything.

Everything is asserted on ONE attachments-scope file attached to a private parent, so the flip is the only variable:

• 401 before, 302 afteracl='public_read' — and the redirect is followed anonymously and the served bytes compared, because a 302 into a wall satisfies a status-only assertion;
• the /url sibling reopens too, in its declared { success, data: { url } } envelope, and that URL also serves the bytes;
• flipping the acl back to private re-closes both routes — which is what shows the acl VALUE is the cause, rather than state accrued by the earlier requests;
• a sibling file on the same parent, same uploader, same scope keeps its 401/403, so the opt-out is per file and not a global fail-open.

The TTL is the assertion I would keep if I could keep only one.authorizeDownload returns presignedTtl on the public_read early return and downloadTtl after an allow verdict, and no status code exposes which branch ran. The pin decodes the expiry out of the minted capability URL: measured 3600s anonymous versus 300s for an authorized gated grant. A widened gate that admitted the anonymous caller through the AUTHORIZED path would still answer 302 with working bytes — and would be caught here.

The field-owned gating class (ref_object/ref_id, ADR-0104 D3 wave 2) is covered on both sides as well, including its distinct FILE_DOWNLOAD_DENIED code, which is the evidence that the field-owned arm rather than the attachments arm is the one that ran.

Stated as manual: C4, the friendly denial copy in objectui's RecordAttachmentsPanel, is a screenshot oracle over a console bundle this repo does not build. No pin, must be hand-driven, and the ref says so.

2. read-inherits-parent-rls C1 and C3 — the count, and failing closed

packages/qa/dogfood/test/attachments-parent-rls-count-parity.dogfood.test.ts (7 tests) and attachments-parent-rls-scan-cap.dogfood.test.ts (3 tests).

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. A suite reading only records cannot tell a filtered count from an unfiltered one.

The trap that would have made this vacuous, and the reason 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 to exceed the rows on the page, proving the count call happened instead of assuming it.

Pinned: the restricted member's 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, never a hand-built principal) agrees; a count narrowed to the invisible parent answers 0 for the member and the true number for admin — the sharpest shape of the leak, where rows are empty either way and only the count distinguishes; and paging the member all the way through yields exactly the reported total, so rows-vs-count parity is one asserted fact rather than two numbers that happen to agree.

The scan-cap file pays the volume the branch needs (2480 candidate join rows) and asserts the warning fires, that visible rows are DROPPED rather than the un-scanned remainder being admitted, that the invisible-parent row never appears, that every returned row has a genuinely resolvable visible parent, and that a row the broad read omitted is returned by a scoped read that does not hit the cap — the contrast without which "missing" is equally explained by "invisible anyway".

The seed shape is load-bearing. The pre-scan has no ORDER BY, and the driver was measured NOT to return insertion order — a first version seeded the probe row last, assumed it fell outside the window, and its control caught that it did not. Visible rows now sit at BOTH ENDS of the insertion sequence with the filler bulk between, so one group is outside the window under either end-ordering, and the test picks an actually-excluded row at runtime rather than predicting one.

Stated as not reached: the unknown parent-OBJECT arm has no reachable fixture from this lane — the #2727enable.files gate refuses to create a sys_attachment row against a non-files object even under system context, so such a row cannot be seeded (this is what the first draft of that test discovered). The middleware-level outer catch is not provokable through the API either, since a where malformed enough to break the system pre-scan also fails the caller's own read and surfaces as a 4xx. Both are named in the ref.

3. attach-requires-parent-edit C3 — measured, and NOT pinned

packages/qa/dogfood/test/attachments-unscoped-delete-gate.dogfood.test.ts (5 tests).

Clause C3 says an unscoped multi-delete — no id AND no where — is refused outright (#4757). Driven end to end, that refusal never fires. The engine's predicate path dispatches beforeDelete per row (dispatchPerRowBeforeHooks builds input: { id: rowId, options }), so the handler always takes its by-id branch and never reaches the where === undefined check.

Measured on a real stack, member holding the sys_attachment delete bit:

case	before	result	after
uploader of one row, not entitled to the other	2	throws ATTACHMENT_DELETE_DENIED — but with the per-row message, not the unscoped one	2
uploader of every matched row	2	resolves, deletes both	0

Filed as #9719. attachment-access-hooks.test.ts pins the refusal by calling the handler directly with an operation-level context the engine no longer produces on this path, so that unit green is not evidence about the wired behaviour.

C3 is deliberately left unpinned rather than pinned against current behaviour — pinning today's outcome would turn the eventual fix red. What this file pins is the half that holds and is worth guarding: an unscoped AST cannot slip past the per-row gate and removes nothing — not even the one row the caller WAS entitled to, so the refusal cannot land as a partial delete; where: {} behaves identically; a predicate matching nothing is allowed through (the live half of the #4757 reasoning); and the same caller's scoped delete of their own row succeeds, without which every assertion here would be satisfied by a delete path that is simply broken. The delete grant is asserted before any of it, because otherwise RBAC refuses ahead of the attachment hook and the file would be green for a reason it is not about.

Also carried into the ledger

The QA run's finding 5, as a marker on all three items so runs stop re-deriving it: the cross-tenant clauses are structurally unprovable in the OSS workspace. @objectstack/organizations is cloud-private, the matrix's cross-tenant block skips by design, and that is blocked(dependency) — never a pass and never a defect. OS_TEST_MULTI_ORG_ENABLED=1 turns the skip into a hard failure for runs that ship the package.

Red without the behaviour — predicted vs actual

Every ablation was rebuilt into dist/ and proven live with scripts/ablation-dist-preflight.mjs before its colour was allowed to count (dogfood resolves through built artifacts; an unrebuilt ablation stays green and would certify a vacuous test). Each was reverted with --absent confirming no marker survives.

#	ablation	predicted	actual
A	authorizeDownload stops honouring acl === 'public_read'	4 red / 4 green — the two AFTER cases, the TTL case, the field-owned case	4 red / 4 green, exactly those four
B	'count' dropped from READ_OPS	5 red / 2 green — every count assertion, no row assertion	5 red / 2 green, exactly those five
C	the cap's logger.warn removed	1 red — the log oracle only	1 red / 2 green
D	the cap fails OPEN (no filter past the bound)	2 red — the fail-closed case and the dropped-rows contrast	first run 1 red / 2 green — prediction MISSED; see below
E	the parent-edit delete gate always allows	2 red — the two unscoped refusals	3 red / 2 green — the third is a CASCADE, not an independent detection

D is the one that changed the code. The dropped-rows contrast stayed green because it read the broad list with $top=200: with the filter removed the page fills with unfiltered rows, every visible row falls off the end, and "absent from the page" — which the test read as "excluded by the cap" — was really "paged out". The test would have gone green while the surface leaked the whole table. It now asserts the page is not full before drawing that conclusion, and on re-run leg D is 2 red / 1 green as predicted, with the second red a genuine detection. This is the vacuity the ablation existed to find, and it was in my own test.

E's extra red is a cascade and is not scored as a detection. With the gate open, the earlier unscoped delete actually removes the rows, so the later scoped-delete test fails for want of its fixture rather than by detecting anything. A matching count is not a passing verification unless each red fails for the reason predicted — here two did and the third did not.

Verification

All of the following on the pushed head 7e99100df, after merging origin/main (which carries #9664, the sibling card's landing):

• union — the four new pins plus the existing attachments-permission-matrix: Test Files 5 passed (5), Tests 38 passed | 1 skipped (39). The single skip is the cross-tenant block described above, skipped by design.
• pnpm --filter @objectstack/dogfood typecheck — tsc --noEmit, exit 0 (script name echoed, so not a zero-match filter).
• check:type-check-debt --re-measure — 33 ledger entries re-measured, 1926 raw errors, "surplus: none", nothing above its recorded number.
• green: check:platform-checklist (15 areas, 190 items), check:nul-bytes (6219 files), check:test-source-alias, check:type-source-resolution, check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:type-check-coverage, check:cross-package-test-inputs, check-affected-docs, and spec's check:empty-state / check:liveness / check:strictness-ledger / check:variant-docs.

check:cross-package-test-inputs was not assumed to cover these files. Three injection attempts using untracked spellings passed, which proved nothing; injecting the detector's own escaping-binding shape into one of the new tests turned it red, and restoring it turned it green — so the gate demonstrably reaches this file. The pins add no cross-package read of their own, so nothing new is declared.

Notes for review

• No changeset, skip-changeset instead.@objectstack/dogfood is private: true so it cannot appear in one, and scripts/check-empty-changeset.mjs rejects an empty-frontmatter changeset. Tests plus checklist prose release nothing.
• One issue filed: The #4757 unscoped multi-delete refusal on sys_attachment never fires through ObjectQL.delete — per-row hook dispatch bypasses it #9719 — the product gap in section 3. Not addressed by this PR.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/dogfood, touching 9 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/plugins/development.mdx(via rootDir (symbol))
• content/docs/protocol/kernel/metadata-service.mdx(via rootDir (symbol))

What this run could not see

• 1 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 2 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9bfd54105853bef3af39685502454bb8c7ce3bef → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 9ac2cb737e5c606e0c25f468cecb857487204780 — the merge of head 7e99100df8f8b7c9b1c2b4bcf324383e4e6fef0c into base 9bfd54105853bef3af39685502454bb8c7ce3bef, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9ac2cb737e5c606e0c25f468cecb857487204780 && git checkout 9ac2cb737e5c606e0c25f468cecb857487204780
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9bfd54105853bef3af39685502454bb8c7ce3bef 7e99100df8f8b7c9b1c2b4bcf324383e4e6fef0c && git checkout -B drift-repro 9bfd54105853bef3af39685502454bb8c7ce3bef && git merge --no-ff 7e99100df8f8b7c9b1c2b4bcf324383e4e6fef0c
node scripts/docs-audit/affected-docs.mjs --json 9bfd54105853bef3af39685502454bb8c7ce3bef

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 9bfd54105853bef3af39685502454bb8c7ce3bef → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[os-zhuang]

PM review — accepted, ready + auto-merge armed. ⭐ The best self-catch of this shift is in leg D.

domain:cli seat, session session_012WKSnqAaoqtW3QX7SSf1Vk. Dev returned status: rework — correctly: the card needs rework, this diff does not. #9483 is now pm:blocked + Blocked-by: #9719, paired and read back.

⚠️ Context worth recording: this dev died mid-run on an API server error (a platform incident that took two agents minutes apart) and was resumed. Its work at that moment existed only in a local worktree with the remote branch carrying zero commits of its own. It pushed first on resume, then finished. Everything below is post-recovery work.

⭐ Leg D — a prediction that MISSED, and the miss found a vacuous test

Predicted 2 red. First run: 1 red / 2 green. Rather than shrug at an off-by-one, the dev asked why — and found the fault was in its own test:

the dropped-rows contrast read the broad list with $top=200, so with the filter removed the page filled with unfiltered rows and "absent from the page" meant "paged out", not "excluded by the cap"

⇒ That assertion would have stayed green while the surface leaked the whole table. It was fixed by asserting the page is not full before drawing the conclusion; on re-run leg D is 2 red / 1 green as predicted, and the second red is now a genuine detection.

This is the single most valuable thing in the report. A wrong prediction is cheap; a test that passes for the wrong reason is expensive and silent. The prediction is what surfaced it — a leg that had merely "looked right" would have shipped the hole.

⭐ Leg E — a matching-ish count, correctly refused as a detection

Predicted 2 red / 3 green; actual 3 red / 2 green. The third red is a cascade — the ablated unscoped delete removes the rows the later scoped-delete test needs as fixture. The dev names it and explicitly does not score it as a detection.

"A matching or near-matching count is not a passing verification unless each red fails for the predicted reason."

Two did; the third did not; it is counted as two. That is the rule this lane has been circling all day, stated cleanly and applied against its own result.

The gate was proven to reach these files, not assumed

check:cross-package-test-inputs — three injections with untracked spellings passed, proving nothing, and only the detector's own escaping-binding shape turned it red, green again on restore. A positive control that distinguishes "the gate is satisfied" from "the gate cannot see this file at all", which is the zero-match trap in its gate-shaped form.

Ablations A/B/C each rebuilt into dist/ with scripts/ablation-dist-preflight.mjs proving the mutation was live before its colour counted, each reverted with --absent confirming no marker survived. A: predicted 4/4, actual 4/4, the predicted four. B: predicted 5/2, actual 5/2 — count assertions red while every row assertion stays green, which is precisely the clause.

Verified by me

• Part-of PR must not also close its card — green, so the Part of choice is settled mechanically as well as by rule.
• ⛔ content/docs/releases/ untouched — no content/docs/ touched at all. The drift check's two rootDir rows are the symbol appearing in test code, not a documentation change.
• The installAttachmentAccessHooks does not authorize an UNSCOPED multi-delete: no id + no where reads as "nothing to authorize" and deleteMany runs over the whole table #4757 unreachability claim, structurally: the guard's doc comment at attachment-access-hooks.ts:31-34 declares the property; the handler at :202 opens const ids = asIdList(ctx?.input?.id); if (ids), so with input.id bound per row the refusal arm cannot be reached. Its own unit test is green on a context shape the engine no longer produces.

⛔ #9719's A/B/C is not this seat's to rule — B changes reject behaviour for every guarded object (Clause ②), C retires a declared safety property. Human floor; filed for triage and the maintainer.

---

Generated by Claude Code