fix(plugin-security): an OAuth MCP client's list_objects resolves its delegator once per request

.changeset/33179-list-objects-round-trips.md

@@ -0,0 +1,14 @@
+---
+'@objectstack/plugin-security': patch
+---
+
+fix(plugin-security): an OAuth MCP client's `list_objects` resolves the person it acts for once per request, not once per object
+
+Clause-②: no
+Metadata-protocol: none
+
+An MCP client signed in with OAuth acts on behalf of a person (ADR-0090 D10). Every object-schema read such a caller makes is the intersection of the client's grants and that person's. `list_objects` asks two questions of every object in the environment: may this caller read the object, and which of its fields. Each question re-read the person's user row, memberships, grants and positions, so one listing cost about 22 database statements per object. Measured on a real kernel, 150 objects took 4620 statements. An environment owner with about 170 objects on a remote database waited tens of seconds, close to a client's 60-second timeout.
+
+The security service now resolves that person once per request and reuses the answer for every object, so the same listing costs 11 statements whatever the object count. `describe_object` drops from 22 statements to 11. The answers are byte for byte the same for every caller: the same objects, field counts and order. Signed-in sessions and API keys act for nobody and were not affected.
+
+The reuse lasts only as long as the request's context and only until the next write. Revoking a grant or removing a membership retires it at once, inside the same request too. One request now judges the person's time-limited grants at one instant, its first resolution, instead of at each object, so one listing is coherent. No answer moves except one whose grant window closes inside that request.

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

@@ -847,13 +847,24 @@ describe('[#25107] every reader in this package that resolves a delegator asks t
   const SRC = dirname(fileURLToPath(import.meta.url));
   const calls = (text: string, name: string) =>
     (text.match(new RegExp(`\\b${name}\\(`, 'g')) ?? []).length - (text.match(new RegExp(`function ${name}\\(`, 'g')) ?? []).length;
+  // The per-context memo (`SecurityPlugin.resolveDelegatorForContext`) resolves
+  // in ONE place for the readers that ask it, and asks nothing itself: each of
+  // its askers asks the check beside its own call, so an ask of the memo counts
+  // as a resolution in the asker's file, and the memo's one resolution counts
+  // in none.
+  const MEMO_FILE = 'security-plugin.ts';
+  const memoAsks = (text: string) => (text.match(/\bthis\.resolveDelegatorForContext\(context\b/g) ?? []).length;
 
   it('per source file, the asks equal the resolutions — and the seven files that resolve one are all counted', () => {
     const counted: Record<string, [number, number]> = {};
     for (const file of readdirSync(SRC, { recursive: true }) as string[]) {
       if (!file.endsWith('.ts') || file.endsWith('.test.ts') || file.includes('.testkit.')) continue;
       const text = readFileSync(join(SRC, file), 'utf8');
-      const resolutions = calls(text, 'resolveDelegatorContext');
+      let resolutions = calls(text, 'resolveDelegatorContext') + memoAsks(text);
+      if (file === MEMO_FILE) {
+        expect(calls(text, 'resolveDelegatorContext'), 'the memo resolves in exactly one place').toBe(1);
+        resolutions -= 1;
+      }
       if (resolutions === 0) continue;
       counted[file] = [resolutions, calls(text, 'delegatedOrganizationRefusal')];
     }

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

@@ -67,7 +67,7 @@
 import type { SecurityPlugin } from './security-plugin.js';
 import type { PermissionSet } from '@objectstack/spec/security';
 import type { FieldMaskingRule } from '@objectstack/spec/data';
-import { delegatedOrganizationRefusal, intersectFieldMasks, resolveDelegatorContext } from './explain-engine.js';
+import { delegatedOrganizationRefusal, intersectFieldMasks } from './explain-engine.js';
 
 /**
  * The `SecurityPlugin` members the functions below read — every one of them,
@@ -89,6 +89,7 @@ export interface FieldReadersHost {
   getObjectFieldNames: SecurityPlugin['getObjectFieldNames'];
   getObjectSecurityMeta: SecurityPlugin['getObjectSecurityMeta'];
   resolveCallerPosture: SecurityPlugin['resolveCallerPosture'];
+  resolveDelegatorForContext: SecurityPlugin['resolveDelegatorForContext'];
   resolveFallbackPermissionSets: SecurityPlugin['resolveFallbackPermissionSets'];
   resolvePermissionSetsForContext: SecurityPlugin['resolvePermissionSetsForContext'];
   resolveProjectionFieldMask: SecurityPlugin['resolveProjectionFieldMask'];
@@ -134,7 +135,7 @@ export async function computeReadableFields(
 
 /** The body of `SecurityPlugin#resolveProjectionFieldMask`; the method's docblock there is its contract. */
 export async function resolveProjectionFieldMask(
-  this: Pick<FieldReadersHost, 'computeForbiddenFieldWrites' | 'computeQueryGuardFieldPerms' | 'computeReadPartialMaskRules' | 'foldFieldRequiredPermissions' | 'getObjectFieldNames' | 'metadata' | 'permissionEvaluator' | 'ql' | 'resolveCallerPosture' | 'resolveFallbackPermissionSets' | 'resolvePermissionSetsForContext' | 'tenancyPosture'>,
+  this: Pick<FieldReadersHost, 'computeForbiddenFieldWrites' | 'computeQueryGuardFieldPerms' | 'computeReadPartialMaskRules' | 'foldFieldRequiredPermissions' | 'getObjectFieldNames' | 'metadata' | 'permissionEvaluator' | 'ql' | 'resolveCallerPosture' | 'resolveDelegatorForContext' | 'resolveFallbackPermissionSets' | 'resolvePermissionSetsForContext' | 'tenancyPosture'>,
   object: string,
   context: any,
   options: { fallbackOnEmptySets: boolean },
@@ -208,7 +209,9 @@ export async function resolveProjectionFieldMask(
   let delBasePerms: Record<string, { readable: boolean; editable: boolean }> | null = null;
   let delegatorSets: PermissionSet[] | null = null;
   if (permissionSets.length > 0 && context?.onBehalfOf?.userId) {
-    const del = await resolveDelegatorContext(this.ql, context);
+    // Once per context while no write intervenes (`resolveDelegatorForContext`):
+    // a schema-serving door asks this of every object it serves.
+    const del = await this.resolveDelegatorForContext(context);
     if (del.kind === 'missing') return { kind: 'answer', fields: [] };
     if (del.kind === 'resolved') {
       if (delegatedOrganizationRefusal(context, del.context, this.tenancyPosture) !== undefined) {

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

@@ -91,12 +91,13 @@ export interface ObjectAdmissionHost {
   getObjectSecurityMeta: SecurityPlugin['getObjectSecurityMeta'];
   objectReadAdmission: SecurityPlugin['objectReadAdmission'];
   organizationWallRefusal: SecurityPlugin['organizationWallRefusal'];
+  resolveDelegatorForContext: SecurityPlugin['resolveDelegatorForContext'];
   resolvePermissionSetsForContext: SecurityPlugin['resolvePermissionSetsForContext'];
 }
 
 /** The body of `SecurityPlugin#canReadObject`; the method's docblock there is its contract. */
 export async function canReadObject(
-  this: Pick<ObjectAdmissionHost, 'delegatedAdminGate' | 'logger' | 'objectReadAdmission' | 'ql' | 'resolvePermissionSetsForContext' | 'tenancyPosture'>,
+  this: Pick<ObjectAdmissionHost, 'delegatedAdminGate' | 'logger' | 'objectReadAdmission' | 'ql' | 'resolveDelegatorForContext' | 'resolvePermissionSetsForContext' | 'tenancyPosture'>,
   object: string,
   context?: any,
 ): Promise<boolean> {
@@ -121,7 +122,9 @@ export async function canReadObject(
     // ({@link delegatedOrganizationRefusal}).
     let delegatorSets: PermissionSet[] | null = null;
     if (context?.onBehalfOf?.userId) {
-      const del = await resolveDelegatorContext(this.ql, context);
+      // Once per context while no write intervenes (`resolveDelegatorForContext`):
+      // a schema-serving door asks this of every object it serves.
+      const del = await this.resolveDelegatorForContext(context);
       if (del.kind === 'missing') return false;
       if (del.kind === 'resolved') {
         if (delegatedOrganizationRefusal(context, del.context, this.tenancyPosture) !== undefined) return false;

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

@@ -191,6 +191,9 @@ import {
   objectReadAdmission,
   type ObjectAdmissionHost,
 } from './security-plugin-object-admission.js';
+// The ADR-0090 D10 delegator resolution the two object-schema read sites ask
+// through {@link SecurityPlugin.resolveDelegatorForContext}.
+import { resolveDelegatorContext, type DelegatorResolution } from './explain-engine.js';
 // [#25223 cut 10] The `everyone` anchor's declarations: the anchor's
 // definition a started kernel derives, and the built-in positions `init()`
 // declares; the methods of the same names below hand each body the members it
@@ -3343,7 +3346,7 @@ export class SecurityPlugin implements Plugin {
     const readPermissionEvaluator = () => this.permissionEvaluator;
     const readQl = () => this.ql;
     const readTenancyPosture = () => this.tenancyPosture;
-    const host: Pick<FieldReadersHost, 'computeForbiddenFieldWrites' | 'computeQueryGuardFieldPerms' | 'computeReadPartialMaskRules' | 'foldFieldRequiredPermissions' | 'getObjectFieldNames' | 'metadata' | 'permissionEvaluator' | 'ql' | 'resolveCallerPosture' | 'resolveFallbackPermissionSets' | 'resolvePermissionSetsForContext' | 'tenancyPosture'> = {
+    const host: Pick<FieldReadersHost, 'computeForbiddenFieldWrites' | 'computeQueryGuardFieldPerms' | 'computeReadPartialMaskRules' | 'foldFieldRequiredPermissions' | 'getObjectFieldNames' | 'metadata' | 'permissionEvaluator' | 'ql' | 'resolveCallerPosture' | 'resolveDelegatorForContext' | 'resolveFallbackPermissionSets' | 'resolvePermissionSetsForContext' | 'tenancyPosture'> = {
       computeForbiddenFieldWrites: (...a) => this.computeForbiddenFieldWrites(...a),
       computeQueryGuardFieldPerms: (...a) => this.computeQueryGuardFieldPerms(...a),
       computeReadPartialMaskRules: (...a) => this.computeReadPartialMaskRules(...a),
@@ -3353,6 +3356,7 @@ export class SecurityPlugin implements Plugin {
       get permissionEvaluator() { return readPermissionEvaluator(); },
       get ql() { return readQl(); },
       resolveCallerPosture: (...a) => this.resolveCallerPosture(...a),
+      resolveDelegatorForContext: (...a) => this.resolveDelegatorForContext(...a),
       resolveFallbackPermissionSets: (...a) => this.resolveFallbackPermissionSets(...a),
       resolvePermissionSetsForContext: (...a) => this.resolvePermissionSetsForContext(...a),
       get tenancyPosture() { return readTenancyPosture(); },
@@ -3476,11 +3480,12 @@ export class SecurityPlugin implements Plugin {
     const readLogger = () => this.logger;
     const readQl = () => this.ql;
     const readTenancyPosture = () => this.tenancyPosture;
-    const host: Pick<ObjectAdmissionHost, 'delegatedAdminGate' | 'logger' | 'objectReadAdmission' | 'ql' | 'resolvePermissionSetsForContext' | 'tenancyPosture'> = {
+    const host: Pick<ObjectAdmissionHost, 'delegatedAdminGate' | 'logger' | 'objectReadAdmission' | 'ql' | 'resolveDelegatorForContext' | 'resolvePermissionSetsForContext' | 'tenancyPosture'> = {
       get delegatedAdminGate() { return readDelegatedAdminGate(); },
       get logger() { return readLogger(); },
       objectReadAdmission: (...a) => this.objectReadAdmission(...a),
       get ql() { return readQl(); },
+      resolveDelegatorForContext: (...a) => this.resolveDelegatorForContext(...a),
       resolvePermissionSetsForContext: (...a) => this.resolvePermissionSetsForContext(...a),
       get tenancyPosture() { return readTenancyPosture(); },
     };
@@ -3763,6 +3768,80 @@ export class SecurityPlugin implements Plugin {
     return permissionSetMemoKey(context);
   }
 
+  /**
+   * The ADR-0090 D10 delegator resolutions {@link resolveDelegatorForContext}
+   * has made, per live execution context — the {@link permissionSetMemo}
+   * discipline, for the second question an on-behalf-of read asks.
+   */
+  private readonly delegatorMemo = new WeakMap<
+    object,
+    { epoch: number; key: string; resolution: Promise<DelegatorResolution> }
+  >();
+
+  /**
+   * [ADR-0090 D10] The delegator behind an on-behalf-of `context`
+   * ({@link resolveDelegatorContext}), resolved once per context while no
+   * write intervenes. Asked by the two object-schema read questions —
+   * {@link canReadObject} and the field projection
+   * ({@link resolveProjectionFieldMask}) — which a schema-serving door asks
+   * once PER OBJECT: `/meta` lists and MCP `list_objects` ask both of every
+   * object in the environment.
+   *
+   * ## The duplicate it removes
+   *
+   * Each ask re-read the delegator (`sys_user`, memberships, grants,
+   * positions) and handed back a context object nobody had seen, so the
+   * delegator's sets missed {@link permissionSetMemo} too: one OAuth client's
+   * `list_objects` issued 22 statements per object (two questions x 11) for
+   * one unchanged answer, measured on a real kernel. Against a database a
+   * network hop away that is the listing's whole latency.
+   *
+   * ## Why it is not a cache
+   *
+   * Exactly the {@link permissionSetMemo} argument, unchanged: keyed on the
+   * context OBJECT, which a door builds once per request (the `/mcp` door,
+   * REST's per-`req` memo), so nothing outlives the caller it was resolved
+   * for; reused only while {@link epoch} stands, so any write in this process
+   * — a grant revoked, a membership removed — retires it; and keyed on the
+   * three members the resolution reads off the context (`onBehalfOf.userId`,
+   * `tenantId`, `org_user_ids`), so a context edited in place resolves again.
+   * Every kind is kept (`none`, `missing`, `resolved`); a rejection is not.
+   *
+   * One consequence, stated: `resolveDelegatorContext`'s clock is read at the
+   * first resolution, so one request judges the delegator's time-limited
+   * grants (ADR-0091) at one instant rather than at each object's — which
+   * makes one listing coherent, and moves no answer except one whose grant
+   * window closes inside the request.
+   *
+   * The resolved context is FROZEN (shallowly, with its arrays): every asker
+   * shares it, and the two sites only read it — the organization check
+   * ({@link delegatedOrganizationRefusal}, still asked at each site, every
+   * call) and the delegator's sets. ⛔ Not for a site that carries the
+   * delegator context onward into a filter or a write (the read filter, the
+   * operation prologue, explain): those keep resolving their own.
+   */
+  private resolveDelegatorForContext(context: any): Promise<DelegatorResolution> {
+    // A primitive/absent context has no identity to key on: resolved, not kept.
+    const keyed = !!context && typeof context === 'object';
+    const key = keyed
+      ? JSON.stringify([context.onBehalfOf?.userId ?? null, context.tenantId ?? null, context.org_user_ids ?? null])
+      : '';
+    const hit = keyed ? this.delegatorMemo.get(context) : undefined;
+    if (hit && hit.epoch === this.epoch.current && hit.key === key) return hit.resolution;
+    const resolution = resolveDelegatorContext(this.ql, context).then((resolved) => {
+      if (resolved.kind !== 'resolved') return Object.freeze(resolved);
+      for (const value of Object.values(resolved.context)) if (Array.isArray(value)) Object.freeze(value);
+      return Object.freeze({ kind: 'resolved' as const, context: Object.freeze(resolved.context) });
+    });
+    if (!keyed) return resolution;
+    const entry = { epoch: this.epoch.current, key, resolution };
+    this.delegatorMemo.set(context, entry);
+    resolution.catch(() => {
+      if (this.delegatorMemo.get(context) === entry) this.delegatorMemo.delete(context);
+    });
+    return resolution;
+  }
+
   /**
    * [#18783] `ISecurityService.getEffectiveObjectPermissions` — the subject's
    * EFFECTIVE object-permission map, object name -> `EffectiveObjectPermission`.