Uh oh!
There was an error while loading. Please reload this page.
test(dogfood): probe app-authored RLS wideners on the bulk write path (#6736) - #7274
Merged
Merged
Conversation
…#6736) End-to-end probe, not a fix. #6736's acceptance section names an end-to-end measurement as the first thing needed ("the mechanism above is a code read"), and the maintainer's 2026-08-08 ruling on #5493 (Q2 = A1) deferred the fix for this path with a stated reason: no measured pull. This supplies the missing measurement and pins today's behaviour so it cannot change unobserved in either direction. Measured on the real stack (bootStack + real SecurityPlugin + real SharingServicePlugin + real ObjectQL engine), app-authored RLS wideners on update and delete: update({multi}): declaration admits 3 rows, statement touches 1, no error delete({multi}): declaration admits 3 rows, statement removes 1, no error Both narrowed by the same predicate, `{ owner_id: <caller> }`, which buildWriteFilter contributes and the bulk branch ANDs into the AST. Zero WARN or ERROR lines in the whole run: the affected-row count is the only signal and it names no authority. Two discrimination controls keep the headline numbers meaningful: the same principal / row / widener SUCCEEDS on the by-id path (so the declaration is live, post-#5493), and a row the widener does not admit is still refused with the ADR-0112 envelope (so the widener has a real boundary). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckNo hand-written docs reference the 0 changed package(s). ✅ |
os-zhuang
marked this pull request as ready for review
August 10, 2026 04:41
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #6736 — probe only, deliberately NOT
Fixes.The fix for this path is deferred by the maintainer's 2026-08-08 ruling on #5493 (Q2 = A1), and #6736 exists precisely so that deferral cannot die inside a ruled thread (the #6216 failure mode). Auto-closing the card on merge would destroy the record this card was filed to keep. What the ruling did not defer is the card's own first acceptance step — "An end-to-end probe first (the mechanism above is a code read)". This PR is that probe, and nothing else.
What this adds
One dogfood test file,
packages/qa/dogfood/test/bulk-widener-probe.dogfood.test.ts. It pins today's behaviour, and today's behaviour is defective-by-declaration. The file says so in its own header, in those words. It is not a fix, contains no fix, and must not be read as one.The measurement
Real stack —
bootStackwith the realSecurityPlugin, the realSharingServicePluginand the real ObjectQL engine. A member holds an app-authored permission set declaring, in the author's own words, "any holder may UPDATE a note in stageopen, and DELETE a note in stagestale" — said about the row, never about its owner.update({multi})n_bob_open,n_alice_open_1,n_alice_open_2)n_bob_open)delete({multi})d_bob_stale,d_alice_stale_1,d_alice_stale_2)d_bob_stale)The claim in the card reproduces, on both verbs.
The narrowing predicate, read off the running system rather than inferred, is identical for both verbs:
deletecomposes the same way and for a documented reason, not by accident:buildWriteFilterreturns the owner-match alone for the delete verb (ADR-0111 D3 — a share widens rows, never verbs), so there is no share branch for a widener to have been OR-ed alongside either.What surfaces the discrepancy: nothing. The engine returns an affected-row count of
1, which is indistinguishable from "only one row matched the caller's predicate". The whole probe run emits zero WARN and zero ERROR lines. Contrast two nearby precedents that do speak: the by-id delete denial one block up in the same middleware logs a specific reason (ADR-0111 D10), and #6783 addedSYSTEM_WRITE_SKIP_NOTICEfor exactly the "configured but inert stops being silent" shape. This branch has neither.Why the fixture is a
public_readobjectDeliberate, and load-bearing.
buildReadFilterreturnsnullfor any non-privatemodel, so reads are open and cannot confound the measurement — on aprivateobject the cross-owner rows would be invisible to the caller and a bulk write that touched nothing would be explained by read scoping rather than by the write composition, which is the wrong thing to measure. MeanwhilebuildWriteFilterreturns the owner-match forprivateandreadalike (only a fullypublicobject is write-open). So on this object the sharing write filter is the only narrowing agent: the composition under test, isolated.Discrimination controls — why the headline number means anything
"1 out of 3" proves nothing on its own. An inert widener (never parsed, never applicable) and a fixture that accidentally granted ownership would each produce a confident-looking number. Two controls split those apart, and both hold:
sharing.checkEditanswersdeny,security.checkAuthoredRowWriteanswersadmit, the PATCH returns 2xx and the row really changes. An inert declaration would 403 here.FORBIDDEN/PERMISSION_DENIED), not as a bare throw. A fixture that leaked ownership, or a disabled gate, would let this through.Plus an explicit integrity case asserting the resolved principal, that the app-authored set actually reached the execution context, that the run is not
isSystem, and that the caller owns exactly one row of each fixture.Reverse verification — direction predicted in writing BEFORE the run
Manipulation: neutralise
composeAnd(ast.where, writeFilter)in the bulk branch only; revert immediately. Predicted, before running: the two MEASURE cases go red (touched 1 to 3; survivors 2 to 0), and the three other cases — integrity, C1, C2 — stay green, because the by-id path never enters that branch. Recorded reasoning: if C1 or C2 also moved, the probe would not be isolating the bulk branch and the headline count would not be attributable to it.Measured: exactly that.
2 failed | 3 passed, the two failures being the two MEASURE cases, withaffected1 to 3 on both verbs andsurvivors2 to 0.No delta to report honestly: the prediction and the measurement agree case for case.
Scope — what this PR deliberately does NOT do
Per the ruling and the dispatch: no filter-shaped authored-only write
FilterCondition, no second composition site, no change tobuildWriteFilter's composition, and no extension ofcheckAuthoredRowWrite(a per-row verdict has no seat in AST composition, which is exactly why the by-id surface cannot reach this path). Zero production files change;git diff origin/mainoverpackages/andexamples/is empty apart from the new test.Gates
pnpm --filter @objectstack/dogfood exec vitest run test/bulk-widener-probe.dogfood.test.ts— 5 passedpnpm --filter @objectstack/dogfood typecheck— cleaneslint packages/qa/dogfood/test/bulk-widener-probe.dogfood.test.ts --no-inline-config— cleancheck:nul-bytes,check:verify-stand-in,check:engine-double-contract,check:error-code-casing,check:route-envelope,check:query-options-erasure,check:authz-resolver— all greenNo changeset: test-only, releases nothing (
skip-changeset).Generated by Claude Code