fix(plugin-security): checkAuthoredRowWrite answers the declaration, not the caller's read scope (#7281)

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

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');
+    }
+  });
+});

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');
   });
 });