fix(plugin-security)!: a write from a caller the read door refuses is refused, whatever rows it addresses

.changeset/sec-hardening-p3-write-refused-read-door.md

@@ -0,0 +1,16 @@
+---
+'@objectstack/plugin-security': patch
+---
+
+fix(plugin-security)!: a write from a caller the read door refuses is refused, whatever rows it addresses
+
+Clause-②: no (narrowing) — a predicate update or delete (`multi: true`, addressed by a `where`) from a caller whose read of the object is refused is refused with that read refusal before the write runs, where it was admitted before; nothing is widened.
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) no metadata an author writes changes and no stored row is rewritten: a runtime write from a caller the object's read refuses is refused -->
+
+**BREAKING** for a caller whose read of an object is refused while its write is admitted, and that updates or deletes that object's records by predicate. With the permission model's own grants that caller exists in one shape only: an object whose `requiredPermissions` map gates `read` and not `update` or `delete` (ADR-0066 ⑤), and a caller holding the write grant without the capability the read requires.
+
+- **The rule.** A caller the read door refuses writes none of the object's records, by id or by predicate. The predicate update and delete such a caller addresses — a flow's `update_record` or `delete_record` node with `multi: true` run as that user, or an engine `update` / `delete` with `multi: true` under that caller's context — now answer the read door's own refusal (the `PERMISSION_DENIED` envelope that caller's read gets), and nothing is written. The by-id update and delete are refused too.
+- **Unchanged.** A caller who reads the object keeps its predicate write, scoped to the records it reads. A caller the object's CRUD grants refuse is refused as before. A write the platform issues under the caller's context (a hook's or a cascade's own write, the referential clear of a deleted record's lookups) and a system write keep their answers. A store fault on the read propagates as before.
+- **Who moves.** A deployment that relies on such a caller writing by predicate grants it the capability the object's read requires, or gates the write verbs on a capability that caller holds.

content/docs/permissions/authorization.mdx

@@ -632,6 +632,10 @@ The complete, prioritized gap map lives in issue **#2561** (the production
   object's `requiredPermissions` may be a `string[]` (gates all CRUD) or a
   `{ read, create, update, delete }` map (read-open / write-gated), enforced
   per operation by plugin-security (`security-plugin.ts` capability AND-gate).
+  A map that gates `read` and not `update` or `delete` does not let a caller
+  without the read capability change records it cannot read: an update or
+  delete from a caller the read door refuses is refused, by id or by
+  predicate.
 - **Secure-by-default rollout** (ADR-0066 ④) — **system-object slice landed**:
   the raw secret/credential stores (`sys_secret`, `sys_jwks`,
   `sys_verification`, `sys_oauth_access_token`, `sys_oauth_refresh_token`,

content/docs/permissions/permissions-matrix.mdx

@@ -370,7 +370,7 @@ flowchart TD
 | 7 | **Field-Level Security** | Which fields is the user allowed to see/edit? |
 
 <Callout type="tip">
-**Performance:** For reads, steps 2–6 are compiled into a query filter (owner-match ∪ materialized shares, AND-ed with RLS) at query time, not evaluated record-by-record. By-id writes are verified with a pre-image check: the target row is re-read through the write-scope filter before the mutation, and a row the caller cannot read answers exactly what a missing id answers (`404 RECORD_NOT_FOUND`). A predicate (`multi: true`) update or delete matches only the rows the caller can read: a row the read door would not return is not written, not counted and not refused, and above 10 000 such rows the write is refused (`400 INVALID_FILTER`) rather than narrowed by a cut-off list. This keeps security checks efficient even on tables with millions of rows.
+**Performance:** For reads, steps 2–6 are compiled into a query filter (owner-match ∪ materialized shares, AND-ed with RLS) at query time, not evaluated record-by-record. By-id writes are verified with a pre-image check: the target row is re-read through the write-scope filter before the mutation, and a row the caller cannot read answers exactly what a missing id answers (`404 RECORD_NOT_FOUND`). A predicate (`multi: true`) update or delete matches only the rows the caller can read: a row the read door would not return is not written, not counted and not refused, and above 10 000 such rows the write is refused (`400 INVALID_FILTER`) rather than narrowed by a cut-off list. A caller whose read of the object is refused writes none of its records: its by-id and predicate updates and deletes are refused, whatever rows they address. This keeps security checks efficient even on tables with millions of rows.
 </Callout>
 
 ## See also

packages/plugins/plugin-security/src/predicate-write-unreadable-not-matched.test.ts

@@ -16,13 +16,25 @@
  *  3. A caller who can read a matched row but may not write it keeps the 403 a
  *     per-row gate answers, as before.
  *  4. A visible, writable match is still written (control).
- *  5. The read question's three outcomes: a read the read door REFUSES keeps
- *     the write's previous answer; a store fault propagates; a readable set
+ *  5. The read question's three outcomes: a read the read door REFUSES is the
+ *     write's answer (8, below); a store fault propagates; a readable set
  *     larger than one write may name is refused, nothing written.
  *  6. A predicate write the platform issues under the caller's context (a
  *     hook's own write) is not addressed by the caller and keeps the answer it
  *     had before.
  *  7. The by-id doors keep their answers (regression).
+ *  8. A write from a caller the read door refuses is refused, whatever rows it
+ *     addresses: the predicate update and delete that caller addresses are
+ *     answered the read door's own refusal before the write runs, with
+ *     row-level policies and without, and nothing changes. The by-id leg
+ *     refuses the same caller too; with no row-level policy it gives the same
+ *     answer. A caller who reads keeps its scoped
+ *     write, and a write the platform issues under the caller's context, or a
+ *     system write, keeps the answer it had. (The read here is refused by a
+ *     capability the object requires for a read and not for a write, ADR-0066
+ *     ⑤: since an object permission that edits or deletes a record also reads
+ *     it, the one shape in which the write door admits a caller the read door
+ *     refuses.)
  *
  * ## The rig
  *
@@ -86,7 +98,23 @@ function outcome(p: Promise<unknown>): Promise<Outcome> {
   );
 }
 
-async function boot(opts: { bulkRows?: number } = {}): Promise<Rig> {
+/**
+ * The rig's knobs. Defaults are the rig the first suites run on; the
+ * read-refused caller's suite (8) gates the child's read on a capability, and
+ * runs with and without the guard set's row-level policies.
+ */
+interface BootOptions {
+  /** Rows on the bulk object, for the readable-set cap. */
+  bulkRows?: number;
+  /** `requiredPermissions` on the child object (ADR-0066 D3/⑤). */
+  childRequiredPermissions?: Record<string, string[]>;
+  /** Keep the guard set's row-level policies (the default) or run with none. */
+  rls?: boolean;
+  /** Capabilities the guard set grants. */
+  capabilities?: string[];
+}
+
+async function boot(opts: BootOptions = {}): Promise<Rig> {
   const engine = new ObjectQL();
   engine.registerDriver(
     new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) as never,
@@ -114,7 +142,13 @@ async function boot(opts: { bulkRows?: number } = {}): Promise<Rig> {
         sharingModel: 'public_read_write',
         fields: { id: { name: 'id', type: 'text', primaryKey: true }, name: { name: 'name', type: 'text' } },
       },
-      { name: CHILD, label: 'Child', sharingModel: 'public_read_write', fields: childFields },
+      {
+        name: CHILD,
+        label: 'Child',
+        sharingModel: 'public_read_write',
+        ...(opts.childRequiredPermissions ? { requiredPermissions: opts.childRequiredPermissions } : {}),
+        fields: childFields,
+      },
       { name: BULK, label: 'Bulk', sharingModel: 'public_read_write', fields: childFields },
     ],
   } as never);
@@ -151,8 +185,9 @@ async function boot(opts: { bulkRows?: number } = {}): Promise<Rig> {
   ];
   const guard = PermissionSetSchema.parse({
     name: 'qa_pw_guard',
+    ...(opts.capabilities ? { systemPermissions: opts.capabilities } : {}),
     objects: { [PARENT]: crud, [CHILD]: crud, [BULK]: crud },
-    rowLevelSecurity: [...rls(CHILD), ...rls(BULK)],
+    rowLevelSecurity: opts.rls === false ? [] : [...rls(CHILD), ...rls(BULK)],
   });
   // Edit and delete on the child, and no read: the read door refuses this
   // caller outright, which is not a hidden row.
@@ -366,3 +401,93 @@ describe('the by-id doors keep their answers', () => {
     expect(got).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403, message: GATE_SENTENCE });
   });
 });
+
+describe('a write from a caller the read door refuses is refused, whatever rows it addresses', () => {
+  // The child's read is gated on a capability the caller lacks, and its writes
+  // are not (ADR-0066 ⑤), so the write door admits a caller the read door
+  // refuses. The rig is a `single` install (its Default Organization), so every
+  // answer here is the `single` posture's.
+  const READ_CAP = 'qa_pw_read_cap';
+  const READ_GATED = { childRequiredPermissions: { read: [READ_CAP] } };
+  const EVERY_ROW = ids('c_own', 'c_hidden_locked', 'c_hidden_open', 'c_shared_locked', 'c_shared');
+
+  for (const verb of ['update', 'delete'] as const) {
+    const write = (r: Rig, where: Record<string, unknown>, context?: Record<string, unknown>) =>
+      verb === 'update' ? r.update(where, context) : r.remove(where, context);
+
+    for (const rls of [true, false]) {
+      const policies = rls ? 'under row-level policies' : 'with no row-level policy';
+      it(`${verb}, ${policies}: the caller gets the read door's refusal for every predicate, and nothing changes`, async () => {
+        const r = await boot({ ...READ_GATED, rls });
+        const read = await outcome(r.engine.find(CHILD, { where: EVERY_ROW, context: { ...CALLER } } as never));
+        expect(read).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 });
+        const before = await names(r);
+
+        const answers = [
+          await write(r, EVERY_ROW),
+          await write(r, ids('c_own')),
+          await write(r, ids('c_hidden_open')),
+          await write(r, { team: 'ops' }),
+          await write(r, ids('c_nowhere')),
+        ];
+
+        for (const got of answers) expect(got).toEqual(read);
+        // Refused means untouched, every row.
+        expect(await names(r)).toEqual(before);
+      });
+    }
+
+    it(`${verb}: the by-id leg gives that caller the same answer`, async () => {
+      const r = await boot({ ...READ_GATED, rls: false });
+      const read = await outcome(r.engine.find(CHILD, { where: ids('c_own'), context: { ...CALLER } } as never));
+      const predicate = await write(r, ids('c_own'));
+      const byId = await outcome(
+        verb === 'update'
+          ? r.engine.update(CHILD, { name: 'swept' }, { where: { id: 'c_own' }, context: { ...CALLER } } as never)
+          : r.engine.delete(CHILD, { where: { id: 'c_own' }, context: { ...CALLER } } as never),
+      );
+      expect(read).toMatchObject({ kind: 'refused', code: 'PERMISSION_DENIED', status: 403 });
+      expect(predicate).toEqual(read);
+      expect(byId).toEqual(read);
+      expect((await r.stored(CHILD, 'c_own'))?.name).toBe('mine');
+    });
+
+    it(`${verb}: a caller holding the capability the read asks for keeps its write, scoped to the rows it reads`, async () => {
+      const r = await boot({ ...READ_GATED, capabilities: [READ_CAP] });
+      expect(await outcome(r.engine.find(CHILD, { where: ids('c_own'), context: { ...CALLER } } as never)))
+        .toMatchObject({ kind: 'landed' });
+
+      const got = await write(r, ids('c_own', 'c_hidden_locked', 'c_hidden_open'));
+
+      expect(got).toEqual({ kind: 'landed', result: 1 });
+      const after = await names(r);
+      if (verb === 'update') expect(after.c_own).toBe('swept');
+      else expect(after).not.toHaveProperty('c_own');
+      expect(after.c_hidden_locked).toBe('theirs, hidden, locked');
+      expect(after.c_hidden_open).toBe('theirs, hidden, open');
+    });
+  }
+
+  it('a predicate write the platform issues under that caller\'s context, and a system write, keep the answers they had', async () => {
+    const r = await boot(READ_GATED);
+    r.engine.registerHook(
+      'beforeUpdate',
+      async (hookCtx: any) => {
+        await hookCtx.api.object(CHILD).update({ name: 'written by the hook' }, { where: ids('c_hidden_open'), multi: true });
+      },
+      { object: PARENT, packageId: PACKAGE },
+    );
+
+    const viaHook = await outcome(
+      r.engine.update(PARENT, { name: 'renamed' }, { where: { id: 'p_hook' }, context: { ...CALLER } } as never),
+    );
+    const system = await outcome(
+      r.engine.update(CHILD, { name: 'written by the system' }, { where: ids('c_shared'), multi: true, context: { isSystem: true } } as never),
+    );
+
+    expect(viaHook).toMatchObject({ kind: 'landed' });
+    expect((await r.stored(CHILD, 'c_hidden_open'))?.name).toBe('written by the hook');
+    expect(system).toEqual({ kind: 'landed', result: 1 });
+    expect((await r.stored(CHILD, 'c_shared'))?.name).toBe('written by the system');
+  });
+});

packages/plugins/plugin-security/src/security-plugin-addressed-writes.ts

@@ -136,7 +136,7 @@ export function addressedPredicateWrite(
 export async function readableRowsScopeForPredicateWrite(
   this: Pick<AddressedWritesHost, 'ql'>,
   opCtx: any,
-): Promise<Record<string, unknown> | null> {
+): Promise<Record<string, unknown>> {
   if (typeof this.ql?.find !== 'function') {
     // The read question cannot be asked, and a write that cannot be narrowed
     // must not run un-narrowed. Unreachable in a real deployment — `start()`
@@ -159,7 +159,11 @@ export async function readableRowsScopeForPredicateWrite(
       context: withoutOperationPrivateKeys((opCtx.context ?? {}) as Record<string, unknown>),
     }),
   );
-  if (answer.refused) return null;
+  // [#21829, ruling B] A caller the read door refuses writes none of the
+  // object's records: the read door's refusal is the predicate write's answer,
+  // before the write runs. The by-id leg refuses the same caller too (step
+  // 2.7).
+  if (answer.refused) throw answer.error;
   const rows: unknown[] = Array.isArray(answer.value) ? answer.value : [];
   if (rows.length > PREDICATE_WRITE_READABLE_ID_CAP) {
     throw predicateWriteReadableCapError(opCtx.object, opCtx.operation);

packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts

@@ -2380,9 +2380,12 @@ export function registerAuthzMiddleware(
       // writable. Composed here, before `next()`, so it binds the engine's
       // matched-row read, its per-row hook dispatch and the driver write
       // alike — a hidden row is not written, not counted and not refused.
+      // [#21829, ruling B] A caller the read door REFUSES is answered that
+      // refusal here, thrown by the read question itself before `next()`: it
+      // writes none of the object's records, whatever its predicate addresses.
+      // The by-id leg (step 2.7) refuses the same caller too.
       if (this.addressedPredicateWrite(opCtx, nestedInOperation, referentialFieldClearWrite)) {
-        const readable = await this.readableRowsScopeForPredicateWrite(opCtx);
-        if (readable) extra.push(readable);
+        extra.push(await this.readableRowsScopeForPredicateWrite(opCtx));
       }
       // [#15813 / ADR-0131 D8] RECORD the Layer 0 verdict on the operation —
       // the seam ruled on #15706 (option (i)): the wall states what it

packages/plugins/plugin-security/src/security-plugin.ts

@@ -819,7 +819,8 @@ export async function unreadableUnderCallerRead(read: () => Promise<unknown>): P
  *  - an answer: whatever the read door returned (no row, a row, a row set);
  *  - `refused`: a declared 4xx — the read itself was refused (the object's read
  *    grant withheld, a predicate the driver will not compile). Not a hidden
- *    row, so the write keeps the answer it has today;
+ *    row: a write door that asks this question throws it, carried in `error`,
+ *    and the write is refused;
  *  - anything else is a store fault and propagates as raised (#7505): an outage
  *    is neither an answer nor a refusal, and reporting it as either would
  *    relabel it.
@@ -4027,15 +4028,19 @@ export class SecurityPlugin implements Plugin {
    *  - an answer: the scope to compose. An empty answer composes the empty id
    *    list, so a predicate that matches only rows the caller cannot read
    *    matches nothing at all;
-   *  - `refused` (a declared 4xx — the read grant withheld, a predicate the
-   *    driver will not compile): `null`, and the write keeps the answer it has
-   *    today;
+   *  - `refused` (a declared 4xx — a capability the object requires for a read
+   *    and not for a write, a predicate the driver will not compile): that
+   *    refusal is THROWN, so the write is answered the read door's own refusal
+   *    before it runs and changes nothing ([#21829] ruling B). A caller the
+   *    read door refuses writes none of the object's records, whatever its
+   *    predicate addresses; the by-id leg refuses the same caller too (step
+   *    2.7);
    *  - a store fault propagates as raised.
    *
    * The id list is bounded by {@link PREDICATE_WRITE_READABLE_ID_CAP}; see
    * there for what happens above it.
    */
-  private readableRowsScopeForPredicateWrite(opCtx: any): Promise<Record<string, unknown> | null> {
+  private readableRowsScopeForPredicateWrite(opCtx: any): Promise<Record<string, unknown>> {
     // [#25223 cut 8] The body moved to `security-plugin-addressed-writes.ts` (see `AddressedWritesHost`).
     const readQl = () => this.ql;
     const host: Pick<AddressedWritesHost, 'ql'> = {