From 7d7c358f31f5fadf51f3e3d1d76750637de9d87e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 09:20:57 +0000 Subject: [PATCH 1/2] chore(#7281): record reverse-verification predictions before the first mutation --- .claude-scratch/PREDICTIONS-7281.md | 50 +++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 .claude-scratch/PREDICTIONS-7281.md diff --git a/.claude-scratch/PREDICTIONS-7281.md b/.claude-scratch/PREDICTIONS-7281.md new file mode 100644 index 0000000000..3d541e4451 --- /dev/null +++ b/.claude-scratch/PREDICTIONS-7281.md @@ -0,0 +1,50 @@ +# #7281 — reverse-verification predictions, written BEFORE the first producer mutation + +Branch point: d53bd0ba9bbe603632e6e9f55c2f202f06a7d541 (origin/main, 2026-08-10 08:53:15 +0000) + +Re-derived anchors at that SHA: +- packages/plugins/plugin-security/src/security-plugin.ts:749/:754 — service registration of `checkAuthoredRowWrite` (triage anchor, still holds) +- packages/plugins/plugin-security/src/security-plugin.ts:2660 — `async checkAuthoredRowWrite(...)` +- packages/plugins/plugin-security/src/security-plugin.ts:2703-2705 — `parts` + the caller-context `findOne` (the defect) +- packages/plugins/plugin-security/src/security-plugin.ts:1301-1306 — the by-id write PRE-IMAGE gate, `findOne(..., context: opCtx.context)` (caller context too) +- packages/plugins/plugin-sharing/src/sharing-plugin.ts:933-939 — the deferral consumption (`probeAuthoredRowWrite` -> admit -> next()) +- packages/plugins/plugin-security/src/row-write-widener-composition.test.ts:521 — the fake-engine `resolves.toBe('admit')` + +## The planned mutation + +`checkAuthoredRowWrite`'s probe read moves from the caller's own execution context +to an elevated (`isSystem`) context, projected to `['id']`. Layer 0 (tenant wall) +and Layer 1 (authored predicate) stay AND-ed into the `where` — they are the +predicate, not the read scope, so the tenant wall does NOT move. + +## Predictions (direction decided before running) + +P1 probe_note (public_read), cross-owner row the widener admits, verdict: + admit BEFORE, admit AFTER. (no change — buildReadFilter returns null on non-private) +P2 probe_secret (private), SAME row shape, SAME widener, SAME principal, verdict: + **abstain BEFORE, admit AFTER** <= the red/green pin +P3 probe_secret, the caller's OWN admitted row, verdict: + admit BEFORE, admit AFTER (declaration is live either way — the control) +P4 NO-LEAK a: a row the declaration does NOT admit (stage 'closed'), both objects: + abstain BEFORE, abstain AFTER +P5 NO-LEAK b: a principal holding NO applicable widener, every row, both objects: + abstain BEFORE, abstain AFTER +P6 NO-LEAK c: end-to-end by-id PATCH of a non-admitted row, both objects: + 4xx with an ADR-0112 envelope BEFORE and AFTER; row unchanged +P7 NO-LEAK d: the elevated scope is confined — after the probe runs, the caller's + own `find` on probe_secret still returns only their own rows, and the caller's + context object is NOT mutated (no isSystem / no extra keys) +P8 NO-LEAK e: the probe returns a verdict string only; no row data reaches the caller +P9 Tenant wall: plugin-security's existing unit pin "abstain — the matching row in + ANOTHER tenant (Layer 0 stays AND-ed in)" stays GREEN after the change, because + layer0 is in the `where`. Deleting layer0 from `parts` must turn it RED. +P10 END-TO-END by-id PATCH of the cross-owner row the declaration ADMITS: + - probe_note (public_read): 2xx BEFORE and AFTER + - probe_secret (private): 403 BEFORE — and my prediction is **still 403 AFTER**, + refused by the security by-id write PRE-IMAGE gate (security-plugin.ts:1301), + which performs its OWN `findOne` under `opCtx.context` and is therefore blind + to the same cross-owner row. If it lands 2xx AFTER, my reading of the + pre-image gate is wrong and I must re-derive before claiming anything. + Record the exact refusal SHAPE on both sides (sharing `FORBIDDEN` vs security + `(row-level security)`), because the shape names which gate refused. +P11 Mutating the fix back out (restore `context`) must turn the P2 pin RED again. From 1c1f81eb7805a52190f4b2b6d7bd9533dbe0e414 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 09:45:02 +0000 Subject: [PATCH 2/2] fix(plugin-security): resolve checkAuthoredRowWrite's probe read under an elevated scope (#7281) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The verdict answers "does the declared, app-authored widener admit this row", which is a question about the row and the policy. It was resolved by re-reading the row through the caller's own execution context, so plugin-sharing's READ filter applied and a `private`-OWD cross-owner row was invisible to the question asked about it — `abstain` for a row the declaration names. The by-id widener surface was structurally dead on the posture #5493 built it for. Ruled by the maintainer on 2026-08-10 (reading 2): resolve the probe read under a scope that can see the row; the write decision stays with the pre-image gate. The predicate — {id} AND layer0(tenant wall) AND layer1(authored policies) — still carries the whole question and still compiles from the caller's own permission sets and tenant, so nothing about the ANSWER widens with the scope. The read is projected to `id`, so the probe can learn existence and nothing else. Test half, landed independently and first per the ruling: a real-stack pin (packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts) that measures the verdict on both OWD postures, plus six no-leak cases; and the fake-engine unit file's end-to-end assertion — which asserted a `private` cross-owner write LANDING, which the real stack refuses — removed, with the double's blind spot written into the file. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ --- .changeset/authored-row-write-probe-scope.md | 54 +++ .claude-scratch/PREDICTIONS-7281.md | 50 -- .../src/authored-row-write-verdict.test.ts | 115 +++++ .../src/row-write-widener-composition.test.ts | 75 ++- .../plugin-security/src/security-plugin.ts | 72 ++- .../src/authored-row-write-deferral.test.ts | 9 +- .../authored-row-write-scope.dogfood.test.ts | 451 ++++++++++++++++++ .../spec/src/contracts/security-service.ts | 20 +- 8 files changed, 761 insertions(+), 85 deletions(-) create mode 100644 .changeset/authored-row-write-probe-scope.md delete mode 100644 .claude-scratch/PREDICTIONS-7281.md create mode 100644 packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts diff --git a/.changeset/authored-row-write-probe-scope.md b/.changeset/authored-row-write-probe-scope.md new file mode 100644 index 0000000000..3dfcb96dfc --- /dev/null +++ b/.changeset/authored-row-write-probe-scope.md @@ -0,0 +1,54 @@ +--- +"@objectstack/plugin-security": patch +"@objectstack/spec": patch +--- + +fix(plugin-security): `checkAuthoredRowWrite` answers the declaration, not the caller's read scope (#7281) + +`ISecurityService.checkAuthoredRowWrite` asks one question — *does an +app-authored row-level widener admit this row for this write?* — and it resolved +that question by re-reading the row through the **caller's own** execution +context. That `findOne` re-enters the middleware chain, so `plugin-sharing`'s +READ filter applied: on a `private`-OWD object a cross-owner row is invisible to +the caller, the read answered null, and the verdict was `abstain` for a row the +declaration names by predicate. + +Measured on the real stack across two objects identical in every respect except +their OWD — same widener text, same principal, same cross-owner row shape: + +| OWD | verdict before | verdict after | +|---|---|---| +| `public_read` | `admit` | `admit` | +| `private` | **`abstain`** | **`admit`** | + +So the by-id widener surface was live on read-open objects and stood down on +read-closed ones, discriminated by a property the widener's author never +mentions — and `private` is the posture #5493 built that surface for. The +maintainer ruled it a defect (2026-08-10): the verdict is about the row and the +policy, not about what the caller may see. The probe read now resolves under an +elevated, principal-less scope. + +**This does not widen anything.** The predicate carries the whole of the +question and travels in the query rather than in the scope: `{id} AND +layer0(tenant wall) AND layer1(app-authored policies)`, both layers still +compiled from the caller's own permission sets and tenant before the read, and +the read is projected to `id` so the probe can only ever learn *that* a row +matches. A row in another tenant, a row no authored policy matches, and a caller +holding no authored policy at all all still answer `abstain` — pinned, including +by mutation: delete the tenant layer from the predicate and the cross-tenant case +goes red. `admit` also remains evidence and never authorization: the by-id write +pre-image gate still resolves the write under the caller's own context and +refuses on its own terms. + +One consequence is stated plainly rather than papered over: because that +pre-image gate performs the same caller-scoped read, a `private`-OWD cross-owner +by-id write is **still refused end-to-end** after this change — now by the +row-level gate (`PERMISSION_DENIED`, "…(row-level security)") rather than by the +sharing middleware's `FORBIDDEN`. Whether a write should reach a row the caller +cannot read is a separate contract question about that gate's read scope, and it +is not settled here. Both behaviours are pinned on the real stack. + +The `@objectstack/spec` half is documentation only: `ISecurityService`'s contract +listed "the row is unreadable" among the `abstain` cases, which is exactly the +conflation the ruling removed. No signature, shape or vocabulary changes, and the +method stays optional and fail-closed. diff --git a/.claude-scratch/PREDICTIONS-7281.md b/.claude-scratch/PREDICTIONS-7281.md deleted file mode 100644 index 3d541e4451..0000000000 --- a/.claude-scratch/PREDICTIONS-7281.md +++ /dev/null @@ -1,50 +0,0 @@ -# #7281 — reverse-verification predictions, written BEFORE the first producer mutation - -Branch point: d53bd0ba9bbe603632e6e9f55c2f202f06a7d541 (origin/main, 2026-08-10 08:53:15 +0000) - -Re-derived anchors at that SHA: -- packages/plugins/plugin-security/src/security-plugin.ts:749/:754 — service registration of `checkAuthoredRowWrite` (triage anchor, still holds) -- packages/plugins/plugin-security/src/security-plugin.ts:2660 — `async checkAuthoredRowWrite(...)` -- packages/plugins/plugin-security/src/security-plugin.ts:2703-2705 — `parts` + the caller-context `findOne` (the defect) -- packages/plugins/plugin-security/src/security-plugin.ts:1301-1306 — the by-id write PRE-IMAGE gate, `findOne(..., context: opCtx.context)` (caller context too) -- packages/plugins/plugin-sharing/src/sharing-plugin.ts:933-939 — the deferral consumption (`probeAuthoredRowWrite` -> admit -> next()) -- packages/plugins/plugin-security/src/row-write-widener-composition.test.ts:521 — the fake-engine `resolves.toBe('admit')` - -## The planned mutation - -`checkAuthoredRowWrite`'s probe read moves from the caller's own execution context -to an elevated (`isSystem`) context, projected to `['id']`. Layer 0 (tenant wall) -and Layer 1 (authored predicate) stay AND-ed into the `where` — they are the -predicate, not the read scope, so the tenant wall does NOT move. - -## Predictions (direction decided before running) - -P1 probe_note (public_read), cross-owner row the widener admits, verdict: - admit BEFORE, admit AFTER. (no change — buildReadFilter returns null on non-private) -P2 probe_secret (private), SAME row shape, SAME widener, SAME principal, verdict: - **abstain BEFORE, admit AFTER** <= the red/green pin -P3 probe_secret, the caller's OWN admitted row, verdict: - admit BEFORE, admit AFTER (declaration is live either way — the control) -P4 NO-LEAK a: a row the declaration does NOT admit (stage 'closed'), both objects: - abstain BEFORE, abstain AFTER -P5 NO-LEAK b: a principal holding NO applicable widener, every row, both objects: - abstain BEFORE, abstain AFTER -P6 NO-LEAK c: end-to-end by-id PATCH of a non-admitted row, both objects: - 4xx with an ADR-0112 envelope BEFORE and AFTER; row unchanged -P7 NO-LEAK d: the elevated scope is confined — after the probe runs, the caller's - own `find` on probe_secret still returns only their own rows, and the caller's - context object is NOT mutated (no isSystem / no extra keys) -P8 NO-LEAK e: the probe returns a verdict string only; no row data reaches the caller -P9 Tenant wall: plugin-security's existing unit pin "abstain — the matching row in - ANOTHER tenant (Layer 0 stays AND-ed in)" stays GREEN after the change, because - layer0 is in the `where`. Deleting layer0 from `parts` must turn it RED. -P10 END-TO-END by-id PATCH of the cross-owner row the declaration ADMITS: - - probe_note (public_read): 2xx BEFORE and AFTER - - probe_secret (private): 403 BEFORE — and my prediction is **still 403 AFTER**, - refused by the security by-id write PRE-IMAGE gate (security-plugin.ts:1301), - which performs its OWN `findOne` under `opCtx.context` and is therefore blind - to the same cross-owner row. If it lands 2xx AFTER, my reading of the - pre-image gate is wrong and I must re-derive before claiming anything. - Record the exact refusal SHAPE on both sides (sharing `FORBIDDEN` vs security - `(row-level security)`), because the shape names which gate refused. -P11 Mutating the fix back out (restore `context`) must turn the P2 pin RED again. diff --git a/packages/plugins/plugin-security/src/authored-row-write-verdict.test.ts b/packages/plugins/plugin-security/src/authored-row-write-verdict.test.ts index 9fc77044d5..b8e979adbd 100644 --- a/packages/plugins/plugin-security/src/authored-row-write-verdict.test.ts +++ b/packages/plugins/plugin-security/src/authored-row-write-verdict.test.ts @@ -477,3 +477,118 @@ describe('[#5493] fail-closed: every failure is `abstain`, and nothing throws ou ).resolves.toBe('abstain'); }); }); + +// ─────────────────────────────────────────────────────────────────────────── + +/** + * [#7281] The probe read runs ELEVATED — and the elevation is confined to it. + * + * An elevated read inside a permission check is exactly the shape that has to + * be proven not to widen anything, so these cases measure the confinement + * rather than asserting it. What a fake engine can legitimately pin here is the + * CALL SHAPE — which context each engine call carries, what the query asks for, + * and whether anything is written — and that is precisely what these cases pin. + * What it cannot pin is the effect of the elevation on a `private`-OWD object, + * because this file's engine has no middleware chain: that lives on the real + * stack, in `packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts`. + */ +describe('[#7281] the probe read is elevated, and the elevation is confined to the probe', () => { + /** Wrap every engine verb so the probe's own traffic is observable. */ + const instrument = (engine: any) => { + const calls: { op: string; object: string; options: any }[] = []; + for (const op of ['find', 'findOne', 'insert', 'update', 'delete'] as const) { + const original = engine[op].bind(engine); + engine[op] = async (object: string, a?: any, b?: any) => { + // `update` is (object, data, options); the rest carry options second. + calls.push({ op, object, options: op === 'update' ? b : a }); + return original(object, a, b); + }; + } + return calls; + }; + + it('the probe reads under an elevated, PRINCIPAL-LESS context — and reads only', async () => { + const stack = await makeStack(); + const calls = instrument(stack.engine); + + await expect( + stack.security.checkAuthoredRowWrite('crm_opportunity', OPP_OPEN.id, 'update', OUTSIDER_WIDENED_CTX), + ).resolves.toBe('admit'); + + const onTarget = calls.filter((c) => c.object === 'crm_opportunity'); + expect(onTarget.length, 'the probe touched the target object').toBeGreaterThan(0); + expect( + onTarget.filter((c) => c.op === 'insert' || c.op === 'update' || c.op === 'delete'), + 'a permission PROBE writes nothing, ever', + ).toEqual([]); + for (const call of onTarget) { + expect(call.options?.context?.isSystem, `${call.op} runs elevated`).toBe(true); + expect(call.options?.context?.userId, `${call.op} carries no principal`).toBeUndefined(); + expect( + call.options?.context, + 'the elevated context is the probe\'s own object, never the caller\'s', + ).not.toBe(OUTSIDER_WIDENED_CTX); + } + }); + + it('the elevated scope carries the WHOLE question in the query: id AND the tenant wall AND the authored predicate', async () => { + // This is why elevating the read cannot widen the answer. Layer 0 (the + // tenant wall) and Layer 1 (the app-authored policies) are compiled from + // the CALLER's permission sets and the CALLER's tenant BEFORE the read and + // travel in the `where`. Delete either from the composed `parts` and the + // cross-tenant / non-matching-row cases above go red — which is exactly + // what makes them pins rather than decoration. + const stack = await makeStack(); + const calls = instrument(stack.engine); + + await stack.security.checkAuthoredRowWrite('crm_opportunity', OPP_OPEN.id, 'update', OUTSIDER_WIDENED_CTX); + + const read = calls.find((c) => c.object === 'crm_opportunity' && c.options?.where); + const where = JSON.stringify(read?.options?.where ?? {}); + expect(where, 'the record id').toContain(OPP_OPEN.id); + expect(where, 'Layer 0 — the tenant wall, from the caller\'s own tenant').toContain('organization_id'); + expect(where, 'Layer 1 — the app-authored predicate').toContain('prospecting'); + expect( + read?.options?.fields, + 'projected to existence: the probe can learn THAT the row matches, never what is in it', + ).toEqual(['id']); + }); + + // ⚠️ GUARD, NOT A MEASUREMENT — this case is green against the pre-#7281 + // producer too, and says so on purpose. It cannot bite on the scope change + // itself (the old code passed the caller's context straight through and + // mutated nothing either); what it bites on is the WRONG WAY to implement + // the elevation — stamping `isSystem` onto the caller's own object, which + // would silently elevate every later use of that context. Reverse-verified + // by mutation, not by reverting the fix. + it('the caller\'s own context object is not mutated by the probe', async () => { + const stack = await makeStack(); + const before = JSON.parse(JSON.stringify(OUTSIDER_WIDENED_CTX)); + await stack.security.checkAuthoredRowWrite('crm_opportunity', OPP_OPEN.id, 'update', OUTSIDER_WIDENED_CTX); + expect(JSON.parse(JSON.stringify(OUTSIDER_WIDENED_CTX))).toEqual(before); + expect((OUTSIDER_WIDENED_CTX as any).isSystem, 'no elevation is stamped onto the caller').toBeUndefined(); + }); + + // ⚠️ GUARD, NOT A MEASUREMENT — green on both sides of #7281, deliberately. + // The write path never carried an elevated read before this change and must + // never carry one after it; the case exists so that a future widening of the + // probe's scope into the enforcement path goes red the day it is written, + // rather than the day someone measures a production leak. + it('the elevation does not reach the WRITE path: the by-id gate still resolves the row as the caller', async () => { + // The ruling leaves the write decision with the pre-image gate, and that + // gate reads as the CALLER. If the elevation ever leaked into it, a caller + // who cannot see a row would start writing it — so this case exists to go + // red the moment that happens. + const stack = await makeStack(); + const calls = instrument(stack.engine); + + await stack.update('crm_opportunity', OPP_OPEN.id, OUTSIDER_WIDENED_CTX); + + const reads = calls.filter((c) => c.object === 'crm_opportunity' && c.op !== 'update'); + expect(reads.length, 'the write path read the target row').toBeGreaterThan(0); + for (const call of reads) { + expect(call.options?.context?.isSystem, 'no elevated read on the write path').toBeFalsy(); + expect(call.options?.context?.userId, 'the write path reads as the caller').toBe('u_outsider'); + } + }); +}); diff --git a/packages/plugins/plugin-security/src/row-write-widener-composition.test.ts b/packages/plugins/plugin-security/src/row-write-widener-composition.test.ts index 372a1e5f52..813942d8d5 100644 --- a/packages/plugins/plugin-security/src/row-write-widener-composition.test.ts +++ b/packages/plugins/plugin-security/src/row-write-widener-composition.test.ts @@ -43,6 +43,22 @@ // `sharingModel: 'private'` but NO `access.default: 'private'` — because that // posture is exactly what withholds the ADR-0066 ① Layer-1 superuser // short-circuit, and it is the shape HotCRM ships. +// +// ⚠️ [#7281] WHAT THIS FILE CANNOT SEE — read before adding a case. +// `makeEngine()` below implements `find` as a direct row filter and registers +// no middleware of its own, so a nested re-read issued from INSIDE a middleware +// (the by-id write pre-image gate's `findOne`, and `checkAuthoredRowWrite`'s +// probe) is never scoped by `plugin-sharing`'s READ filter the way the real +// engine scopes it. On a `private`-OWD object that filter is the difference +// between a cross-owner row being visible and being invisible — and both of +// those nested reads decide their verdict on exactly that. So this double is +// LOOSER than the producer on the read-scope axis, and any assertion whose +// outcome depends on it is green here for the double's reason rather than the +// producer's. #7281 was filed because one such assertion shipped that way (a +// `private` cross-owner write asserted as landing, which the real stack refuses +// — see the case at the bottom of this file for the measurement). +// Anything read-scope-dependent belongs in the real-stack pin: +// `packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts`. import { describe, it, expect, vi, beforeEach } from 'vitest'; import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; import { SharingService, buildSharingMiddleware } from '@objectstack/plugin-sharing'; @@ -486,32 +502,43 @@ describe('[#5492] the platform ownership floor still stands where nothing replac }); }); -describe('[#5493] the sharing middleware DEFERS to an app-authored RLS widener instead of hard-refusing', () => { +describe('[#5493 / #7281] the two authorities’ verdicts on a row an app-authored widener admits', () => { let stack: Stack; beforeEach(async () => { stack = await makeStack(); }); - it('an APP-AUTHORED RLS update-widener passes the security gate AND the write now lands', async () => { - // ── what this case used to assert, and why it flipped ────────────────── - // #5492 (this file's subject) landed the composition and left #5493's - // symptom standing on purpose, so this case was written as a CONTROL of - // the old position: the security pre-image gate admitted the row and the - // SHARING middleware refused it first with `FORBIDDEN`. #5493 step 2 is - // the fix for exactly that, so the control flips — the two authorities are - // ONE composite determination (maintainer ruling, #5492 comment - // 5219846435; mirrored for this card in comment 5217346436), and this - // middleware may not hard-refuse a by-id write an app-authored widener - // admits by declaration. + it('sharing refuses on its own terms; the security service admits by DECLARATION', async () => { + // ⚠️ SCOPE-BLIND BY CONSTRUCTION — this case cannot bite on read scope. + // + // #7281 is what corrected it. The case used to end by driving the write + // through both middlewares and asserting `{ ok: true }` — "the write now + // lands" — on `crm_opportunity`, a `private`-OWD object, for a row owned + // and created by somebody else. The real stack does NOT do that. Measured + // end-to-end (`bootStack` + real SecurityPlugin + real SharingServicePlugin + // + real engine, two objects identical but for their OWD): + // + // public_read verdict admit PATCH 200, row changes + // private verdict abstain PATCH 403 "[Security] Access denied: not + // permitted to update this record + // (row-level security)" + // + // The 403 on `private` is the SECURITY pre-image gate's, thrown before the + // sharing middleware is even reached — so on that posture the deferral this + // block is named for never runs at all. Both halves of the old assertion + // were green here for one reason: `makeEngine()` implements `find` as a + // direct row filter with NO middleware chain, so no nested re-read in this + // file is ever scoped by `plugin-sharing`'s read filter. The double was + // looser than the producer on exactly the axis those verdicts turn on. // - // Everything the old case measured is still measured here, and the load - // is the same load: sharing STILL refuses on its own terms (`checkEdit` → - // `deny`, unchanged — nothing widened the sharing verdict), the security - // gate STILL admits the row through the app policy `stage == - // 'prospecting'` OR-combining past the platform ownership floor. What - // changed is the composition between them: the middleware consults - // `checkAuthoredRowWrite` before refusing, gets `admit`, and hands the row - // to the pre-image gate that makes the final decision. The write lands and - // the ROW REALLY CHANGES — a completed write with an unchanged row would - // mean the middleware chain was bypassed, not that the fix works. + // What survives is what a fake CAN legitimately pin: the two authorities' + // VERDICTS, composed by provenance against the real `member_default` seed. + // `checkAuthoredRowWrite` → `admit` is true on the real stack too (#7281 + // moved the probe read to an elevated scope, so the declaration is what + // answers) — but it is green HERE for the fake's own reason and would stay + // green if the producer regressed to caller-scoped reads. The pin that + // bites on that axis is the real-stack one: + // `packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts`. + // ⛔ Do not re-add an end-to-end write assertion to this file for a + // `private` object without giving this engine a real middleware chain. await expect( stack.sharing.checkEdit('crm_opportunity', OPP_THEIRS.id, WIDENED_CTX as any), ).resolves.toBe('deny'); @@ -520,9 +547,5 @@ describe('[#5493] the sharing middleware DEFERS to an app-authored RLS widener i await expect( stack.security.checkAuthoredRowWrite('crm_opportunity', OPP_THEIRS.id, 'update', WIDENED_CTX), ).resolves.toBe('admit'); - - const out = await stack.write('update', 'crm_opportunity', OPP_THEIRS.id, WIDENED_CTX); - expect(out, out.message).toMatchObject({ ok: true }); - expect(rowById(stack, 'crm_opportunity', OPP_THEIRS.id)?.next_step).toBe('updated'); }); }); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index b15c0ea552..e2054d5321 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -217,6 +217,36 @@ interface RlsFilterOptions { * The map value for each managed `managed_by` is the human owner label used in * the (business-message-only) deny text. */ +/** + * [#7281] The scope `checkAuthoredRowWrite`'s probe read runs under. + * + * The method asks ONE question — "does the declared, app-authored widener admit + * this row for this operation?" — and that question is about the row and the + * declaration, never about what the CALLER may see. Resolving it under the + * caller's own context folded a READ decision into a WRITE question: on a + * `private`-OWD object `plugin-sharing`'s read filter scopes every re-read to + * owner-match OR shares, so a cross-owner row was invisible to the probe and + * the verdict was `abstain` for a row the declaration names by predicate. The + * by-id widener surface was therefore structurally dead on `private` — the + * posture #5493 built it for (maintainer ruling, 2026-08-10: reading 2). + * + * Elevating the READ does not widen the ANSWER, because the predicate carries + * the whole of the question and travels in the `where`, not in the scope: + * `{ id } AND layer0(tenant wall) AND layer1(app-authored policies)`. Both + * layers are computed from the CALLER's permission sets and the CALLER's + * tenant BEFORE the read (see {@link SecurityPlugin.checkAuthoredRowWrite}), so + * a row in another tenant, a row no authored policy matches, and a caller + * holding no authored policy at all are all still `abstain` — measured, not + * asserted (`authored-row-write-verdict.test.ts`, and the real-stack pin + * `packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts`). + * + * Principal-less on purpose: it carries no `userId`, so nothing downstream can + * mistake it for the caller acting with more authority than they hold. It is + * SPREAD at the single call site rather than passed by reference, so no + * middleware can stamp state onto a shared object. + */ +const AUTHORED_ROW_WRITE_PROBE_CONTEXT = { isSystem: true, positions: [], permissions: [] } as const; + const SYSTEM_ROW_PROVENANCE: Record< string, { noun: string; pluralNoun: string; managed: Record } @@ -2652,10 +2682,31 @@ export class SecurityPlugin implements Plugin { * {@link hasWriteBypass} and {@link resolveWriteScope} fail closed on it), an * unresolvable probe and a thrown lookup all return `abstain`. * - * The pre-image read is the same `findOne` shape the by-id write gate uses, - * with the caller's own context — so a row the caller cannot READ is not - * "admitted by declaration" here either, which is the non-widening direction - * and matches what the enforcement path already does with the same read. + * **[#7281] The probe read is ELEVATED, and that is the whole of the fix + * this method received.** It used to run under the CALLER's own context, + * which re-entered the middleware chain and picked up `plugin-sharing`'s + * READ filter: on a `private`-OWD object that scopes every read to + * owner-match OR shares, so a cross-owner row was invisible and the verdict + * was `abstain` for a row the declaration names — measured on the real stack + * across two objects identical but for their OWD (#7281: `public_read` → + * `admit`, `private` → `abstain`, same widener, same principal, same row + * shape). That made the by-id widener structurally dead on exactly the + * posture #5493 built it for, and it did so by folding a READ decision into + * a question about a declaration. The maintainer ruled it (2026-08-10, + * reading 2): this method answers the declaration; the write decision stays + * with the pre-image gate. + * + * Nothing about the ANSWER widens with the scope: `{id} AND layer0 AND + * layer1` is still the entire predicate, both layers still compile from the + * caller's own permission sets and tenant, and `admit` is still evidence + * rather than authorization — the by-id write pre-image gate re-resolves the + * write under the caller's own context and refuses on its own terms. + * ⚠️ One consequence, measured and deliberately NOT papered over: because + * that gate performs the same caller-scoped `findOne`, a `private`-OWD + * cross-owner by-id write is still refused end-to-end after this fix, now by + * the row-level gate rather than by the sharing middleware. The verdict is + * correct; reviving the by-id widener on `private` end-to-end is a separate + * contract question about the pre-image gate's own read scope. */ async checkAuthoredRowWrite( object: string, @@ -2701,7 +2752,18 @@ export class SecurityPlugin implements Plugin { if (layer1 == null) return 'abstain'; const parts = [{ id: recordId }, ...(layer0 ? [layer0] : []), layer1]; - const row = await this.ql.findOne(object, { where: { $and: parts }, context }); + // [#7281] The predicate is the question; the scope is not. Read ELEVATED + // (see AUTHORED_ROW_WRITE_PROBE_CONTEXT) so the answer is decided by + // `{id} AND layer0 AND layer1` alone, and projected to `id` so the probe + // can only ever learn EXISTENCE — no column of a row the caller may not + // read is materialised, let alone returned. The verdict remains evidence, + // never authorization: the by-id write pre-image gate still resolves the + // write under the caller's own context. + const row = await this.ql.findOne(object, { + where: { $and: parts }, + fields: ['id'], + context: { ...AUTHORED_ROW_WRITE_PROBE_CONTEXT }, + }); return row ? 'admit' : 'abstain'; } catch (e) { this.logger.warn?.( diff --git a/packages/plugins/plugin-sharing/src/authored-row-write-deferral.test.ts b/packages/plugins/plugin-sharing/src/authored-row-write-deferral.test.ts index 83f0e80906..a6d90947ac 100644 --- a/packages/plugins/plugin-sharing/src/authored-row-write-deferral.test.ts +++ b/packages/plugins/plugin-sharing/src/authored-row-write-deferral.test.ts @@ -41,8 +41,13 @@ // `created_by` ownership floor is deliberately absent from `AUTHORED_POLICIES`, // which is the whole point of probe E-A — plus `abstain` for an unknown row, a // principal-less context and a delegated one. The REAL service driving the REAL -// composition is measured end-to-end in plugin-security's -// `row-write-widener-composition.test.ts` (#5493 control, flipped by this PR); +// composition is measured end-to-end on the REAL stack, in +// `packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts` +// ([#7281] — plugin-security's `row-write-widener-composition.test.ts` was named +// here until that card measured that its fake engine registers no middleware +// chain: nested re-reads there are never scoped by this plugin's READ filter, so +// it cannot see read scoping and is not an end-to-end measurement of anything +// that depends on it); // what THIS file owns is the consumer half: which outcomes widen, which do not, // and that everything that is not a literal `admit` leaves the refusal intact. import { describe, it, expect, beforeEach, vi } from 'vitest'; diff --git a/packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts b/packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts new file mode 100644 index 0000000000..2b14e0217d --- /dev/null +++ b/packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts @@ -0,0 +1,451 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#7281] `checkAuthoredRowWrite` answers the DECLARATION, not the caller's +// read scope — pinned on the REAL stack. +// +// ── Why this file exists at all ─────────────────────────────────────────── +// The unit file that was supposed to cover this — `plugin-security`'s +// `row-write-widener-composition.test.ts` — cannot see the behaviour under +// test. Its `makeEngine()` fake implements `find` as a direct row filter with +// NO middleware chain, so the re-read inside `checkAuthoredRowWrite` is never +// scoped by `plugin-sharing`'s read filter. The producer's verdict on a +// `private`-OWD object depends on precisely that scoping, so the double was +// looser than the producer on the one axis the verdict turns on, and the file +// was green over behaviour the real stack did not have (#7281, and the +// maintainer's 2026-08-10 ruling: "fix the unit test half — independently and +// first"). This file is the real-stack pin that ruling names: real `bootStack`, +// real `SecurityPlugin`, real `SharingServicePlugin`, real ObjectQL engine, +// real middleware chain — the harness idiom of `bulk-widener-probe.dogfood.test.ts`. +// +// ── The mechanism ───────────────────────────────────────────────────────── +// `checkAuthoredRowWrite` resolves "does an app-authored widener admit this +// row" by re-reading the row behind `{id} AND layer0(tenant) AND layer1(authored)`. +// That `findOne` re-enters the middleware chain. Under the CALLER's own context +// it therefore also picks up `plugin-sharing`'s READ filter, which on a +// `private` OWD scopes to owner-match OR shares — so a cross-owner row is +// invisible, `findOne` answers null, and the verdict is `abstain` for a row the +// declaration names. The by-id widener was structurally dead on `private`, the +// posture #5493's widener surface was built for. The ruled fix resolves that +// probe read under an ELEVATED scope, leaving `{id} AND layer0 AND layer1` as +// the whole of the predicate — the tenant wall included, since it lives in the +// `where` and not in the read scope. +// +// ── The discriminator, isolated ─────────────────────────────────────────── +// TWO objects, identical in every respect except the OWD; the SAME widener +// text, the SAME principal, the SAME cross-owner row shape. Whatever separates +// their verdicts is the OWD and nothing else. +// +// `wscope_note` sharingModel 'public_read' -> buildReadFilter returns null +// `wscope_secret` sharingModel 'private' -> buildReadFilter scopes to owner +// +// ── What the no-leak cases are for ──────────────────────────────────────── +// An elevated read inside a permission check is exactly the shape that has to +// be PROVEN not to widen anything. The maintainer's ruling states the safety +// argument ("No leak either way: the final write gate still enforces"); the +// `[no-leak N]` cases below are that argument measured rather than assumed — +// a row the declaration does not admit, a principal holding no widener at all, +// the caller's own read scope after the probe has run, and the end-to-end write +// on both postures. +// +// ── which cases MOVE, and which are green on both sides by design ───────── +// Exactly two assertions change with #7281: `[B private]` and the verdict +// assertion opening `[E2E private]` (both measured `abstain` against the +// pre-#7281 producer, both `admit` after). EVERY other case here — the two +// controls and all six `[no-leak N]` cases — is green on BOTH sides, and that +// invariance IS the safety claim rather than a gap in coverage: an elevated +// read inside a permission check is only safe if nothing else moves. They are +// reverse-verified by mutating what they guard, not by reverting the fix. +// +// ⚠️ READ THIS BEFORE TRUSTING A GREEN RUN ⚠️ +// `[E2E private]` pins that the by-id WRITE is still refused on `private` even +// once the verdict says `admit`, and that the refusal comes from the security +// by-id write PRE-IMAGE gate (`security-plugin.ts`, step 2.7) — which performs +// its OWN `findOne` under `opCtx.context` and is blind to the same cross-owner +// row for the same reason the probe used to be — BEFORE the sharing middleware +// is reached at all. The ruling moved the PROBE's scope and deliberately left +// the write decision with that gate, so #7281 fixes the VERDICT and its +// end-to-end consequence on `private` is nil. That is stated here, in the file, +// and not only in a PR body, because a reader who assumes otherwise will +// mis-read every case below. Whether that gate should also stop conflating +// "cannot read" with "cannot write" is a separate contract question and is NOT +// settled by this ruling. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { defineStack, definePermissionSet } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { resolveAuthzContext } from '@objectstack/core'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; + +// ── the two objects under probe ──────────────────────────────────────────── + +const OPEN = 'wscope_note'; // OWD public_read — reads are open +const CLOSED = 'wscope_secret'; // OWD private — reads are owner-scoped + +const commonFields = () => ({ + title: Field.text({ label: 'Title', required: true, maxLength: 160 }), + body: Field.text({ label: 'Body', maxLength: 2000 }), + stage: Field.text({ label: 'Stage', maxLength: 40 }), + owner_id: Field.lookup('sys_user', { label: 'Owner' }), +}); + +/** Read-open. `buildReadFilter` returns null for a non-`private` model. */ +const OpenNote = ObjectSchema.create({ + name: OPEN, + label: 'Widener Scope Note', + pluralLabel: 'Widener Scope Notes', + sharingModel: 'public_read', + fields: commonFields(), +}); + +/** + * Read-closed — the posture #5493's widener surface was built for, and the ONLY + * difference from `OpenNote`. Ordinary tenant business object (no + * `access.default: 'private'`), so the ADR-0066 ① superuser short-circuit is + * withheld and the app policy is really the thing being asked. + */ +const SecretNote = ObjectSchema.create({ + name: CLOSED, + label: 'Widener Scope Secret', + pluralLabel: 'Widener Scope Secrets', + sharingModel: 'private', + fields: commonFields(), +}); + +/** + * The app's declaration, in the author's own words: "any holder of this set may + * UPDATE a note in stage `open`" — said about the ROW, never about its owner, + * and said identically about both objects. App-authored (neither policy is the + * platform ownership floor `owner_only_writes`), which is the provenance + * `checkAuthoredRowWrite` filters on. + */ +const WidenerSet = definePermissionSet({ + name: 'wscope_widener', + label: 'Widener Scope — app-authored update widener', + objects: { + [OPEN]: { allowRead: true, allowCreate: true, allowEdit: true }, + [CLOSED]: { allowRead: true, allowCreate: true, allowEdit: true }, + }, + rowLevelSecurity: [ + { name: 'wscope_open_stage_updates_note', object: OPEN, operation: 'update', using: "stage == 'open'" }, + { name: 'wscope_open_stage_updates_secret', object: CLOSED, operation: 'update', using: "stage == 'open'" }, + ], +}); + +/** + * The no-widener control principal's set: byte-identical object CRUD, and NO + * row-level declaration at all. It exists so `[no-leak 2]` varies exactly one + * thing — the declaration — rather than varying CRUD rights and calling the + * difference a widener. + */ +const PlainSet = definePermissionSet({ + name: 'wscope_plain', + label: 'Widener Scope — same CRUD, no declaration', + objects: { + [OPEN]: { allowRead: true, allowCreate: true, allowEdit: true }, + [CLOSED]: { allowRead: true, allowCreate: true, allowEdit: true }, + }, +}); + +const probeApp = defineStack({ + manifest: { + id: 'com.example.authoredrowwritescope', + namespace: 'wscope', + version: '0.0.1', + type: 'app', + name: 'Authored Row-Write Scope Probe', + engines: { protocol: '^17' }, + }, + objects: [OpenNote, SecretNote], + permissions: [WidenerSet, PlainSet], +}); + +const SYS = { isSystem: true } as const; + +interface Row { id: string; title: string; stage: string; owner_id: string; body?: string } + +/** Row ids, per object — same shapes on both so the postures stay comparable. */ +const ids = (object: string) => ({ + mine: `${object}_bob_open`, + theirsAdmitted: `${object}_alice_open`, + theirsOutside: `${object}_alice_closed`, +}); + +describe('[#7281] checkAuthoredRowWrite answers the declaration, not the caller read scope', () => { + let stack: VerifyStack; + let ql: any; + let security: any; + let sharing: any; + let bobToken: string; + let carolToken: string; + let bobId: string; + let aliceId: string; + let carolId: string; + let bobCtx: any; + let carolCtx: any; + + /** The SAME authz context the REST entry point builds — never a hand-rolled principal. */ + const authzFor = async (token: string) => { + const authService: any = await stack.kernel.getServiceAsync('auth'); + let api: any = authService?.api; + if (!api && typeof authService?.getApi === 'function') api = await authService.getApi(); + const headers = new Headers({ authorization: `Bearer ${token}` }); + return resolveAuthzContext({ + ql, + headers, + getSession: async (h: any) => api?.getSession?.({ headers: h }), + }); + }; + + const seed = async (object: string, row: Row) => + ql.insert(object, { ...row }, { context: { ...SYS } }); + + const rowById = async (object: string, id: string): Promise => + (await ql.findOne(object, { where: { id }, context: { ...SYS } })) as Row | null; + + beforeAll(async () => { + stack = await bootStack(probeApp, { + security: new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, WidenerSet as any, PlainSet as any], + fallbackPermissionSet: 'member_default', + }), + }); + await stack.signIn(); // dev admin seed + bobToken = await stack.signUp('wscope-bob@verify.test'); // holds the widener + carolToken = await stack.signUp('wscope-carol@verify.test'); // same CRUD, no widener + await stack.signUp('wscope-alice@verify.test'); // row owner only + + ql = await stack.kernel.getServiceAsync('objectql'); + security = await stack.kernel.getServiceAsync('security'); + sharing = await stack.kernel.getServiceAsync('sharing'); + + const uid = async (email: string) => + (await ql.findOne('sys_user', { where: { email }, context: { ...SYS } }))?.id; + bobId = await uid('wscope-bob@verify.test'); + carolId = await uid('wscope-carol@verify.test'); + aliceId = await uid('wscope-alice@verify.test'); + expect(bobId).toBeTruthy(); + expect(carolId).toBeTruthy(); + expect(aliceId).toBeTruthy(); + + const bindSet = async (userId: string, name: string) => { + const setRow = await ql.findOne('sys_permission_set', { where: { name }, context: { ...SYS } }); + expect(setRow?.id, `the app-declared set '${name}' is seeded`).toBeTruthy(); + await ql.insert('sys_user_permission_set', + { user_id: userId, permission_set_id: setRow.id }, { context: { ...SYS } }); + }; + await bindSet(bobId, 'wscope_widener'); + await bindSet(carolId, 'wscope_plain'); + + for (const object of [OPEN, CLOSED]) { + const id = ids(object); + await seed(object, { id: id.mine, title: 'bob open', stage: 'open', owner_id: bobId, body: 'seed' }); + await seed(object, { id: id.theirsAdmitted, title: 'alice open', stage: 'open', owner_id: aliceId, body: 'seed' }); + await seed(object, { id: id.theirsOutside, title: 'alice closed', stage: 'closed', owner_id: aliceId, body: 'seed' }); + } + + bobCtx = await authzFor(bobToken); + carolCtx = await authzFor(carolToken); + }, 180_000); + + afterAll(async () => { await stack?.stop(); }); + + // ── integrity ───────────────────────────────────────────────────────────── + // + // Every number below is worthless if the fixture quietly handed Bob + // ownership, the widener never reached the resolver, or the two objects + // differ by more than their OWD. Assert all three BEFORE measuring. + + it('[integrity] the principals, the declaration, and the ONE difference between the two objects', async () => { + expect(bobCtx?.userId, 'the resolved principal is Bob').toBe(bobId); + expect(bobCtx?.permissions, 'the app-authored set resolved onto the context') + .toContain('wscope_widener'); + expect(bobCtx?.isSystem, 'the probe never runs as system').toBeFalsy(); + expect(carolCtx?.userId).toBe(carolId); + expect(carolCtx?.permissions).toContain('wscope_plain'); + expect(carolCtx?.permissions, 'the control principal holds NO widener').not.toContain('wscope_widener'); + + for (const object of [OPEN, CLOSED]) { + const id = ids(object); + expect((await rowById(object, id.mine))?.owner_id, `${object}: Bob owns exactly the one row`).toBe(bobId); + expect((await rowById(object, id.theirsAdmitted))?.owner_id).toBe(aliceId); + expect((await rowById(object, id.theirsOutside))?.owner_id).toBe(aliceId); + } + + // The discriminator, named rather than inferred: the sharing READ filter is + // the only thing that differs between the two objects for this caller. + expect( + await sharing.buildReadFilter(OPEN, bobCtx), + 'public_read: reads are open, so nothing scopes the probe re-read', + ).toBeNull(); + expect( + await sharing.buildReadFilter(CLOSED, bobCtx), + 'private: reads are owner-scoped, which is what removed the verdict', + ).toMatchObject({ owner_id: bobId }); + + // And it really bites on reads: Bob cannot see Alice's row on `private`. + const seen = await ql.find(CLOSED, { where: {}, context: bobCtx }); + expect(seen.map((r: Row) => r.id).sort(), 'Bob reads only his own row on the private object') + .toEqual([ids(CLOSED).mine]); + }); + + // ── the measurement ─────────────────────────────────────────────────────── + + it('[A public_read] the declaration admits the cross-owner row → admit', async () => { + await expect( + security.checkAuthoredRowWrite(OPEN, ids(OPEN).theirsAdmitted, 'update', bobCtx), + ).resolves.toBe('admit'); + }); + + it('⭐ [B private] the SAME declaration, the SAME row shape, the SAME principal → admit', async () => { + // THE pin. Against the pre-#7281 producer this case is RED, measuring + // `abstain`: the probe re-read ran under Bob's own context, the sharing + // read filter scoped it to `owner_id = bob`, and the row the declaration + // names by predicate was invisible to the question asked about it. + await expect( + security.checkAuthoredRowWrite(CLOSED, ids(CLOSED).theirsAdmitted, 'update', bobCtx), + ).resolves.toBe('admit'); + }); + + it('[control] on that same private object, Bob\'s OWN admitted row → admit (the declaration is live either way)', async () => { + // Rules out "the widener is inert / mis-parsed on this object". If this + // were `abstain` too, case B would be measuring a broken declaration + // rather than the read scope. + await expect( + security.checkAuthoredRowWrite(CLOSED, ids(CLOSED).mine, 'update', bobCtx), + ).resolves.toBe('admit'); + }); + + // ── no-leak: the elevated read must widen NOTHING ───────────────────────── + + it('[no-leak 1] a row the declaration does NOT admit stays `abstain` — on BOTH postures', async () => { + // The elevated read must not turn "invisible" into "permitted". `stage` + // is 'closed', so layer1 does not match and no read scope is involved in + // the answer at all. + for (const object of [OPEN, CLOSED]) { + await expect( + security.checkAuthoredRowWrite(object, ids(object).theirsOutside, 'update', bobCtx), + `${object}: outside the declaration`, + ).resolves.toBe('abstain'); + } + }); + + it('[no-leak 2] a principal with the SAME CRUD and NO declaration gains nothing — every row, both postures', async () => { + for (const object of [OPEN, CLOSED]) { + const id = ids(object); + for (const target of [id.mine, id.theirsAdmitted, id.theirsOutside]) { + await expect( + security.checkAuthoredRowWrite(object, target, 'update', carolCtx), + `${object}/${target}: no authored policy can admit anything`, + ).resolves.toBe('abstain'); + } + } + }); + + it('[no-leak 3] the verdict is the whole answer — no row data crosses the boundary', async () => { + const verdict = await security.checkAuthoredRowWrite( + CLOSED, ids(CLOSED).theirsAdmitted, 'update', bobCtx, + ); + expect(typeof verdict, 'a string from the closed vocabulary, never a row').toBe('string'); + expect(['admit', 'abstain']).toContain(verdict); + }); + + it('[no-leak 4] the elevation is confined to the probe: the caller context and the caller read scope are unchanged', async () => { + const before = JSON.parse(JSON.stringify(bobCtx)); + await security.checkAuthoredRowWrite(CLOSED, ids(CLOSED).theirsAdmitted, 'update', bobCtx); + await security.checkAuthoredRowWrite(OPEN, ids(OPEN).theirsAdmitted, 'update', bobCtx); + + expect(JSON.parse(JSON.stringify(bobCtx)), 'the caller context is not mutated by the probe') + .toEqual(before); + expect((bobCtx as any).isSystem, 'no elevation is stamped onto the caller').toBeFalsy(); + + // The caller's own read scope after the probe has run is what it was before. + const seen = await ql.find(CLOSED, { where: {}, context: bobCtx }); + expect(seen.map((r: Row) => r.id).sort(), 'still only his own row').toEqual([ids(CLOSED).mine]); + expect( + await ql.findOne(CLOSED, { where: { id: ids(CLOSED).theirsAdmitted }, context: bobCtx }), + 'the row the verdict admits is STILL invisible to the caller as a read', + ).toBeFalsy(); + }); + + it('[no-leak 5] a row outside the declaration is still REFUSED end-to-end, on both postures (ADR-0112 envelope)', async () => { + for (const object of [OPEN, CLOSED]) { + const target = ids(object).theirsOutside; + const res = await stack.apiAs(bobToken, 'PATCH', `/data/${object}/${target}`, { body: 'should-not-land' }); + expect(res.status, `${object}: a row outside the declaration must be refused`).toBeGreaterThanOrEqual(400); + const envelope: any = await res.json().catch(() => ({})); + expect( + JSON.stringify(envelope), + `${object}: the refusal carries a real error envelope, not a bare throw`, + ).toMatch(/FORBIDDEN|PERMISSION_DENIED/); + expect((await rowById(object, target))?.body, `${object}: the row is untouched`).toBe('seed'); + } + }); + + it('[E2E public_read] the widener is LIVE end-to-end where the caller can read the row', async () => { + // The posture on which #5493's by-id deferral actually functions, and the + // control that makes `[E2E private]` below mean something: an identical + // declaration on a read-open object lands the write. Sharing still refuses + // on its own terms; the deferral consults the verdict and stands down. + const target = ids(OPEN).theirsAdmitted; + await expect(sharing.checkEdit(OPEN, target, bobCtx)).resolves.toBe('deny'); + await expect(security.checkAuthoredRowWrite(OPEN, target, 'update', bobCtx)).resolves.toBe('admit'); + + const res = await stack.apiAs(bobToken, 'PATCH', `/data/${OPEN}/${target}`, { body: 'e2e-open' }); + expect(res.status, await res.text().catch(() => '')).toBeLessThan(300); + expect((await rowById(OPEN, target))?.body).toBe('e2e-open'); + + // restore, so ordering between cases cannot smuggle a result + await ql.update(OPEN, { body: 'seed' }, { where: { id: target }, context: { ...SYS } }); + }); + + it('⚠️ [E2E private] the verdict now admits — and the WRITE is still refused, by the pre-image gate', async () => { + // ── READ THIS BEFORE CHANGING THIS CASE ──────────────────────────────── + // This is NOT the widener being ignored, and it is NOT #7281 unfixed. The + // maintainer's ruling moved the PROBE's read scope and left the write + // decision with the pre-image gate. That gate — `security-plugin.ts` + // step 2.7 — resolves the write with its OWN `findOne` under `opCtx.context`, + // the caller's context, which on a `private` OWD is scoped by the sharing + // READ filter for exactly the reason the probe used to be. So it refuses, + // and it refuses FIRST: the 403 below carries the row-level gate's sentence + // and `PERMISSION_DENIED`, not the sharing middleware's `FORBIDDEN`, which + // means the deferral this verdict feeds is never even reached on this + // posture. Measured, both before and after #7281. + // + // Two consequences, stated so nobody has to re-derive them: + // • the verdict fix is real and pinned by case [B private] above — the + // contract question "does the declaration admit this row" is answered + // correctly now, and no consumer is misled by an `abstain` that only + // described the caller's eyesight; + // • whether a by-id write should LAND on `private` for a row the caller + // cannot read is a separate contract question about the pre-image + // gate's read scope ("may you write what you cannot read"), and it is + // NOT settled by #7281's ruling. This case pins the answer as it + // stands today so that changing it is a deliberate act with a red test + // to justify, rather than a silent side effect. + const target = ids(CLOSED).theirsAdmitted; + await expect(security.checkAuthoredRowWrite(CLOSED, target, 'update', bobCtx)).resolves.toBe('admit'); + + const res = await stack.apiAs(bobToken, 'PATCH', `/data/${CLOSED}/${target}`, { body: 'e2e-secret' }); + expect(res.status, 'still refused — the pre-image gate reads as the caller').toBe(403); + const envelope: any = await res.json().catch(() => ({})); + expect(envelope?.code, 'ADR-0112 error code').toBe('PERMISSION_DENIED'); + expect( + String(envelope?.error ?? ''), + 'the row-level gate refused, NOT the sharing middleware (that shape would be `FORBIDDEN: insufficient privileges`)', + ).toContain(`not permitted to update this '${CLOSED}' record (row-level security)`); + expect((await rowById(CLOSED, target))?.body, 'the row is untouched').toBe('seed'); + }); + + it('[no-leak 6] the no-declaration principal is still REFUSED end-to-end on the row the WIDENER admits', async () => { + for (const object of [OPEN, CLOSED]) { + const target = ids(object).theirsAdmitted; + const before = (await rowById(object, target))?.body; + const res = await stack.apiAs(carolToken, 'PATCH', `/data/${object}/${target}`, { body: 'carol-should-not-land' }); + expect(res.status, `${object}: Carol declares nothing and must be refused`).toBeGreaterThanOrEqual(400); + const envelope: any = await res.json().catch(() => ({})); + expect(JSON.stringify(envelope)).toMatch(/FORBIDDEN|PERMISSION_DENIED/); + expect((await rowById(object, target))?.body, `${object}: the row is untouched`).toBe(before); + } + }); +}); diff --git a/packages/spec/src/contracts/security-service.ts b/packages/spec/src/contracts/security-service.ts index 5707e46cff..79615cd57d 100644 --- a/packages/spec/src/contracts/security-service.ts +++ b/packages/spec/src/contracts/security-service.ts @@ -383,12 +383,28 @@ export interface ISecurityService { * **`admit` iff** at least one applicable, **non-floor** policy matches the * row for this operation. `abstain` in **every** other case, including: * the caller holds no authored policy for `(object, operation)`; the - * authored policies apply but none matches this row; the row is unreadable, - * absent, or in another tenant; the context carries no principal; the context + * authored policies apply but none matches this row; the row is absent, or + * in another tenant; the context carries no principal; the context * is on-behalf-of (ADR-0090 D10 — the delegator intersection is not computed * on this path, so an answer here would be resolved against the wrong * identity); or any internal probe fails. * + * **[#7281] The caller's READ scope is not one of those cases**, and the + * omission is the maintainer's 2026-08-10 ruling rather than an oversight. + * The question is "does the declaration admit this row", which is about the + * row and the policy; an implementation that resolves it through the + * caller's own visibility folds a READ decision into a WRITE question and + * silently answers `abstain` for every cross-owner row on a `private`-OWD + * object — the posture the widener surface exists for (measured: two objects + * identical but for their OWD, same widener, same principal, same row shape; + * `public_read` → `admit`, `private` → `abstain`). Implementations therefore + * resolve the row under a scope that can SEE it, with the tenant wall and + * the authored predicate carried in the query rather than in the scope. + * This does not widen anything: `admit` remains evidence and never + * authorization (see below), so a caller who may not read a row still may + * not write it — that refusal belongs to the write gate, which makes it on + * its own terms. + * * **Fail-closed by construction, in both halves.** The method itself never * throws outward — an internal failure becomes `abstain`. And the method is * OPTIONAL: a deployment whose security service predates it, or omits it,