fix(plugin-security)!: the raw-SQL read scope (getReadFilter) answers the deny sentinel where the delegated prologue refuses

.changeset/25106-read-scope-delegated-organization-membership.md

@@ -0,0 +1,17 @@
+---
+'@objectstack/plugin-security': patch
+---
+
+fix(plugin-security)!: the raw-SQL read scope (`getReadFilter`) answers the deny sentinel for a delegated context the operation prologue refuses — the delegator's membership must back the organization the read runs in, on a walled posture
+
+Clause-②: no (narrowing)
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, object definition or stored shape is added, removed, renamed or re-shaped, and no exported declaration is removed or narrowed, so there is nothing for `objectstack migrate meta` to rewrite. What changes is the read scope the security service answers an in-process caller that builds an on-behalf-of execution context in an organization the delegator does not belong to; the remedy is to build that context in an organization the delegator is a member of, never a rewrite of a package source or a stored row. -->
+
+**BREAKING** (the security service's read scope narrows for one context shape). ADR-0021 D-C makes the raw-SQL read scope the find path's scope, and the provider's own contract is agreement with the engine middleware. Under `isolated` or `group`, the delegated-principal prologue already refuses every operation of an on-behalf-of context whose active organization the delegator's membership does not back (ADR-0090 D10, `vetOrganizationClaim`). `getReadFilter` did not ask that question, so for the same context it answered a composed read scope where the find path refuses. It now asks it, once, beside the dangling-delegator refusal it already mirrored.
+
+- **FROM:** under `isolated` or `group`, `getReadFilter` for an execution context carrying `onBehalfOf` and an active organization the delegator is not a member of answered a read scope composed from the two principals' layers, where the engine's find refuses the same context.
+- **TO:** it answers `RLS_DENY_FILTER` itself (the spec's sentinel, by reference), so a door that bypasses the engine reads zero rows for that context, as the engine's find refuses it.
+- **Unchanged:** the `single` posture, an on-behalf-of context with no active organization, an on-behalf-of context in an organization the delegator is a member of, a context without `onBehalfOf`, a dangling delegator (still the deny sentinel) and system contexts. The `/mcp` OAuth door, the one producer of an on-behalf-of context in the platform, vets the request's organization against the same membership, so the contexts it builds keep their scope.
+- **The fix for an in-process producer:** code that builds an `ExecutionContext` with `onBehalfOf` by hand and hands it to the security service or to the analytics service must set its `tenantId` to an organization the delegating user is a member of, or leave it unset.

packages/plugins/plugin-security/src/delegated-organization-membership.test.ts

@@ -39,12 +39,24 @@
  *   without the delegation link; a delegated context with no active
  *   organization (it keeps ADR-0123 D2's refusal); a system context; and the
  *   `single` posture, which has no wall.
+ *
+ * ## The raw-SQL read scope answers the same rule [#25106 · ADR-0021 D-C]
+ *
+ * `getReadFilter` is the read scope a door that bypasses the engine ANDs into
+ * its query, and its contract is agreement with the find path. So where the
+ * prologue refuses a delegated context, the provider answers the deny
+ * sentinel (`RLS_DENY_FILTER` itself, by reference), on a public, a private
+ * and an organization-less object alike: the rule is about the context, not
+ * the object. Every context the prologue admits keeps the scope it had, and
+ * one parity cell reads the provider beside the engine's own find over the
+ * same contexts.
  */
 
 import { describe, it, expect, afterEach, vi } from 'vitest';
 import { ObjectQL } from '@objectstack/objectql';
 import { SqlDriver } from '@objectstack/driver-sql';
 import type { PermissionSet } from '@objectstack/spec/security';
+import { RLS_DENY_FILTER } from '@objectstack/spec/data';
 import { SecurityPlugin } from './security-plugin.js';
 import { SysOrganization, SysUser, SysMember } from '@objectstack/platform-objects/identity';
 import { SysPosition, SysPermissionSet, SysPositionPermissionSet } from './__tests__/legacy-catalog-objects.testkit.js';
@@ -65,6 +77,9 @@ const FIELDS = {
 const OBJECTS = [
   { name: 'qa_account', label: 'Account', fields: FIELDS },
   { name: 'qa_vault', label: 'Vault', access: { default: 'private' }, fields: FIELDS },
+  // [#25106] An organization-less object: no wall applies to it, so only the
+  // context rule can answer the deny sentinel for it.
+  { name: 'qa_catalog', label: 'Catalog', tenancy: { enabled: false }, fields: FIELDS },
 ];
 
 const MEMBER: PermissionSet = {
@@ -201,7 +216,7 @@ async function boot(posture: Posture) {
     const rows = await knex(object).select('id', 'name', 'organization_id');
     return [...rows].sort((a, b) => String(a.id).localeCompare(String(b.id)));
   };
-  return { engine, stored };
+  return { engine, plugin, stored };
 }
 
 interface Outcome { ok: boolean; value?: unknown; code?: string; status?: number; message?: string; developerMessage?: string }
@@ -305,3 +320,137 @@ describe('[#25063] what the rule leaves alone', () => {
     expect((await b.stored('qa_account')).find((r) => r.id === 'r1')).toEqual({ id: 'r1', name: 'one', organization_id: ORG_D });
   });
 });
+
+// ---------------------------------------------------------------------------
+// [#25106 · ADR-0021 D-C] The raw-SQL read scope (`getReadFilter`) answers the
+// same rule. Its contract is agreement with the find path: where the prologue
+// refuses a delegated context, the provider answers the deny sentinel.
+// ---------------------------------------------------------------------------
+
+const PROVIDER_OBJECTS = ['qa_account', 'qa_vault', 'qa_catalog'] as const;
+
+/** One row per tenant object in `org_a` and `org_c` (`org_d`'s is `boot`'s), and two catalog rows, which carry no organization. */
+async function seedForReads(engine: ObjectQL) {
+  for (const object of ['qa_account', 'qa_vault']) {
+    for (const [org, suffix] of [[ORG_A, 'a'], [ORG_C, 'c']] as const) {
+      await engine.insert(object, { id: `${object}_in_${suffix}`, name: 'seeded', organization_id: org }, { context: { ...SYS_CTX, tenantId: org } } as never);
+    }
+  }
+  for (const id of ['qa_catalog_1', 'qa_catalog_2']) {
+    await engine.insert('qa_catalog', { id, name: 'seeded' }, { context: SYS_CTX } as never);
+  }
+}
+
+const sortedIds = (rows: unknown) => (rows as Array<{ id: string }>).map((r) => r.id).sort();
+
+/** The ids a read scope admits, applied as the only predicate under a system context — as a door that bypasses the engine applies it. */
+const idsAdmittedBy = async (engine: ObjectQL, object: string, scope: Record<string, unknown> | undefined) =>
+  sortedIds(await engine.find(object, { where: scope ?? {}, context: { isSystem: true } } as never));
+
+describe('[#25106 · ADR-0021 D-C] getReadFilter answers the deny sentinel where the prologue refuses', () => {
+  for (const posture of ['group', 'isolated'] as const) {
+    for (const object of PROVIDER_OBJECTS) {
+      it(`\`${posture}\` · ${object}: a delegated context in an organization the delegator is not a member of answers RLS_DENY_FILTER itself`, async () => {
+        const b = await boot(posture);
+        await seedForReads(b.engine);
+        expect(await b.plugin.getReadFilter(object, operator(ORG_D))).toBe(RLS_DENY_FILTER);
+      });
+    }
+  }
+});
+
+describe('[#25106] what the rule leaves alone in the read scope', () => {
+  // Each answer below was read on the provider before this rule and after it,
+  // and is the same: the rule moves only the contexts the prologue refuses.
+  // A scope is pinned by the rows it admits (`idsAdmittedBy`).
+  const scopeRows = async (b: Awaited<ReturnType<typeof boot>>, object: string, context: Record<string, unknown>) => {
+    const scope = await b.plugin.getReadFilter(object, { ...context });
+    expect(scope, `${object}: a scope, not the deny sentinel`).not.toBe(RLS_DENY_FILTER);
+    return idsAdmittedBy(b.engine, object, scope);
+  };
+
+  const IN_SET: Record<'group' | 'isolated', Record<string, string[]>> = {
+    // `group` walls the delegator's leg by its own memberships (`org_a`, `org_c`);
+    // the operator crosses on the private object, so its rows come from both.
+    group: { qa_account: ['qa_account_in_a'], qa_vault: ['qa_vault_in_a', 'qa_vault_in_c'], qa_catalog: ['qa_catalog_1', 'qa_catalog_2'] },
+    isolated: { qa_account: ['qa_account_in_a'], qa_vault: ['qa_vault_in_a'], qa_catalog: ['qa_catalog_1', 'qa_catalog_2'] },
+  };
+
+  for (const posture of ['group', 'isolated'] as const) {
+    it(`\`${posture}\`: a delegated context in an organization the delegator belongs to keeps its scope`, async () => {
+      const b = await boot(posture);
+      await seedForReads(b.engine);
+      for (const object of PROVIDER_OBJECTS) {
+        expect(await scopeRows(b, object, operator(ORG_A)), object).toEqual(IN_SET[posture][object]);
+      }
+    });
+
+    it(`\`${posture}\`: the same caller in \`org_d\` WITHOUT the delegation link keeps its scope`, async () => {
+      const b = await boot(posture);
+      await seedForReads(b.engine);
+      expect(await scopeRows(b, 'qa_account', operator(ORG_D, false))).toEqual(['qa_account_in_d']);
+      expect(await scopeRows(b, 'qa_vault', operator(ORG_D, false))).toEqual(['qa_vault_in_a', 'qa_vault_in_c', 'qa_vault_in_d']);
+      expect(await scopeRows(b, 'qa_catalog', operator(ORG_D, false))).toEqual(['qa_catalog_1', 'qa_catalog_2']);
+    });
+
+    it(`\`${posture}\`: a delegated context with NO active organization is not this rule's — its scope is unchanged`, async () => {
+      // The wall decides the tenant object (no organization: no row), and the
+      // organization-less object keeps its rows. The private object is left
+      // out: the delegator holds its grant only inside an organization, so
+      // object admission refuses it (`canReadObject`'s half), not this rule.
+      const b = await boot(posture);
+      await seedForReads(b.engine);
+      expect(await scopeRows(b, 'qa_account', operator(undefined))).toEqual([]);
+      expect(await scopeRows(b, 'qa_catalog', operator(undefined))).toEqual(['qa_catalog_1', 'qa_catalog_2']);
+    });
+
+    it(`\`${posture}\`: a system context is unscoped`, async () => {
+      const b = await boot(posture);
+      for (const object of PROVIDER_OBJECTS) {
+        expect(await b.plugin.getReadFilter(object, { ...SYS_CTX, onBehalfOf: { userId: DELEGATOR } }), object).toBeUndefined();
+      }
+    });
+  }
+
+  it('`single`: there is no wall, so a delegated context in `org_d` keeps its scope', async () => {
+    const b = await boot('single');
+    await seedForReads(b.engine);
+    expect(await scopeRows(b, 'qa_account', operator(ORG_D))).toEqual(['qa_account_in_a', 'qa_account_in_c', 'qa_account_in_d']);
+    expect(await scopeRows(b, 'qa_vault', operator(ORG_D))).toEqual(['qa_vault_in_a', 'qa_vault_in_c', 'qa_vault_in_d']);
+    expect(await scopeRows(b, 'qa_catalog', operator(ORG_D))).toEqual(['qa_catalog_1', 'qa_catalog_2']);
+  });
+});
+
+describe('[#25106 · ADR-0021 D-C] parity: the read scope agrees with the engine find on the same contexts', () => {
+  for (const posture of ['group', 'isolated'] as const) {
+    it(`\`${posture}\`: refused by find ⇔ the deny sentinel; admitted ⇒ the scope admits exactly the rows find returns`, async () => {
+      const b = await boot(posture);
+      await seedForReads(b.engine);
+      const contexts: Record<string, Record<string, unknown>> = {
+        'delegated in org_d': operator(ORG_D),
+        'delegated in org_a': operator(ORG_A),
+        'undelegated in org_d': operator(ORG_D, false),
+      };
+      const refused: string[] = [];
+      for (const [label, context] of Object.entries(contexts)) {
+        for (const object of PROVIDER_OBJECTS) {
+          const cell = `${label} · ${object}`;
+          const scope = await b.plugin.getReadFilter(object, { ...context });
+          const found = await attempt(() => b.engine.find(object, { context: { ...context } } as never));
+          if (!found.ok) {
+            refused.push(cell);
+            expect(found.code, cell).toBe('PERMISSION_DENIED');
+            expect(found.status, cell).toBe(403);
+            expect(scope, cell).toBe(RLS_DENY_FILTER);
+          } else {
+            expect(scope, cell).not.toBe(RLS_DENY_FILTER);
+            expect(await idsAdmittedBy(b.engine, object, scope), cell).toEqual(sortedIds(found.value));
+          }
+        }
+      }
+      // The table is not vacuous on either side: find refuses exactly the
+      // delegated context the delegator's membership does not back.
+      expect(refused).toEqual(PROVIDER_OBJECTS.map((object) => `delegated in org_d · ${object}`));
+    });
+  }
+});

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

@@ -6081,7 +6081,10 @@ export class SecurityPlugin implements Plugin {
    * Fails CLOSED where the chain refuses, with the deny sentinel by identity:
    * a principal that resolves no set (ADR-0056 D2); a delegator the link names
    * who does not exist (the chain's `PERMISSION_DENIED`; `canReadObject`'s
-   * `false`); a delegator who resolves no set (object admission refuses them);
+   * `false`); [#25106] on a walled posture, a live active organization the
+   * delegator's own membership does not back (the prologue's
+   * `PERMISSION_DENIED`, asked of `vetOrganizationClaim` with the prologue's
+   * inputs); a delegator who resolves no set (object admission refuses them);
    * a sharing scope that cannot be resolved. A throw anywhere propagates to the
    * caller's catch, which denies.
    *
@@ -6106,6 +6109,27 @@ export class SecurityPlugin implements Plugin {
       return RLS_DENY_FILTER;
     }
     const delegatorContext = del.context;
+    // [#25106 · ADR-0090 D10] The prologue's organization-membership refusal
+    // (`resolveOperationPrincipals`), mirrored at the same point: after the
+    // dangling-delegator refusal, before the delegator's sets resolve. On a
+    // walled posture the find path refuses every operation of a delegated
+    // context whose live active organization the delegator's own membership
+    // does not back, so the scope that agrees with it is zero rows, on every
+    // object. One ask of the one spelling of the rule, with the inputs the
+    // prologue passes; under `single` the claim stands, and a context with no
+    // active organization is not this rule's (ADR-0123 D2).
+    const liveOrganization =
+      context.tenantId != null && context.tenantId !== '' ? String(context.tenantId) : undefined;
+    if (
+      liveOrganization !== undefined &&
+      vetOrganizationClaim(liveOrganization, delegatorContext?.accessible_org_ids ?? [], this.tenancyPosture) === undefined
+    ) {
+      this.logger.warn?.(
+        `[security] getReadFilter received an on-behalf-of context for object '${object}' whose delegator is not a ` +
+          `member of the organization it runs in — denying, as the operation prologue refuses it (ADR-0090 D10, fail-closed)`,
+      );
+      return RLS_DENY_FILTER;
+    }
     const delegatorSets = await this.resolvePermissionSetsForContext(delegatorContext);
     if (delegatorSets.length === 0) return RLS_DENY_FILTER;