fix(objectql): a runAs:'user' hook sees its organization's members in sys_user, as the user's own request does

.changeset/24497-hook-runas-org-peers.md

@@ -0,0 +1,18 @@
+---
+'@objectstack/objectql': patch
+---
+
+fix(objectql): a `runAs: 'user'` hook now sees the acting user's organization members in `sys_user`, as the same user's own request does
+
+A hook declaring `runAs: 'user'` is pinned to the triggering user. When the triggering write carried a hand-built system context such as `{ isSystem: true, userId, tenantId }` (the approvals writes are the shipped producers), the pinned context carried no `org_user_ids`, the peer list the request path resolves for every user. The shipped `sys_user_org_members` policy (`id in current_user.org_user_ids`) therefore dropped out, and the hook's `ctx.api` read of `sys_user` returned the acting user's own row only, while the same user's request returned their organization's members too.
+
+The hook wrapper now seeds the pinned context through the resolver the request path uses (`resolveUserAuthzGrants` from `@objectstack/core`):
+
+- The peer list is the members of the context's organization (`tenantId`) plus the acting user. A context with no organization gets the acting user alone, which is what an organization-less request gets. A user who belongs only to other organizations is never in it.
+- A context with no `permissions` also takes the user's resolved positions and permission sets, so the hook acts with the user's real grants rather than the security plugin's member baseline. Positions and permission sets the context already carries stay exactly as they were, so a narrowed ceiling is not re-broadened; only the peer list is added.
+- A context that already carries a peer list, such as a request-resolved one, resolves nothing and is handed through as before.
+- If the resolver fails, the hook runs on the unseeded pin, as before, and a warning says so. Policies that read `org_user_ids` then drop out: the hook sees less, never more.
+
+`runAs: 'inherit'` (the default) and `runAs: 'system'` are unchanged, and a `runAs: 'user'` hook whose trigger names no user is still refused with `HOOK_UNSCOPED_DATA_ACCESS`. A `runAs: 'user'` hook fired from a context that lacks either list now pays one grants resolution per dispatch.
+
+Clause-②: no (a runtime producer fix: a `runAs: 'user'` hook reads what the same user's own request reads. Nothing `@objectstack/spec` declares moves; owes the seat's review of record before the queue)

packages/objectql/src/engine.ts

@@ -146,6 +146,9 @@ import {
   // [ADR-0131 D8] The ONE tenant read predicate, shared with plugin-security's
   // Layer 0 — see {@link buildDriverOptions}.
   computeTenantLayer0Verdict,
+  // [#24497] The ONE grants resolver, the one the request path seeds a
+  // context's grants and peer list through — see `ScopedContext[SEED_USER_GRANTS]`.
+  resolveUserAuthzGrants,
 } from '@objectstack/core';
 import { WriteEpoch, isWriteEpochOperation } from './write-epoch.js';
 import { bridgeAuthzInvalidation } from './authz-invalidation-bridge.js';
@@ -310,7 +313,7 @@ import {
   divergingHookPayloadKeys,
   MultiUpdateHookKeyDivergenceError,
 } from './multi-update-hook-key-divergence.js';
-import { UnscopedHookApi, type HookRunAs, type HookRunAsRef, type RunAsDerivableApi } from './hook-run-as.js';
+import { UnscopedHookApi, SEED_USER_GRANTS, type HookRunAs, type HookRunAsRef, type RunAsDerivableApi } from './hook-run-as.js';
 import type { HookWriteRecording } from './hook-write-provenance.js';
 import { resolveMasterDetailRelation } from './master-detail.js';
 // [#19911] The settlement's question "does judging the FK's own lock need the
@@ -21776,6 +21779,61 @@ export class ScopedContext implements IScopedContext, RunAsDerivableApi {
     );
   }
 
+  /**
+   * [#24497] The second half of a hook's `runAs: 'user'` derivation: the
+   * user's grants and peer list, seeded through the one resolver the request
+   * path seeds them through (`resolveUserAuthzGrants`, `@objectstack/core`).
+   * The hook wrapper awaits it on the api {@link withRunAs} derived; it stays
+   * a separate, async step so `withRunAs` keeps its synchronous contract.
+   *
+   * A hook fired from a hand-built `{ isSystem: true, userId }` context (the
+   * approvals writes) was pinned with no `org_user_ids`, so every row policy
+   * reading `current_user.org_user_ids` compiled `unresolved-variable` and
+   * dropped out: the hook saw its own `sys_user` row only, while the same
+   * user's request saw their organisation's members too. Ruling A′ puts the
+   * contract on the producer: every user context carries `org_user_ids`.
+   *
+   * The same rule as a `runAs: 'user'` flow's run setup (service-automation
+   * `resolveRunContext`):
+   *  - positions and permission sets the context already carries stay
+   *    verbatim, so a narrowed ceiling is never re-broadened; only a context
+   *    with no `permissions` takes the resolved ones;
+   *  - a context that already carries a peer list keeps it;
+   *  - a context that needs neither resolves nothing and is returned as is.
+   *
+   * The peer list is the members of the context's `tenantId`, the user
+   * included; a context with no `tenantId` gets the user alone, as an
+   * organisation-less request does. A resolver failure (an unreadable
+   * permission store) is thrown to the caller, which keeps the unseeded api.
+   *
+   * Called on the api {@link withRunAs}`('user')` derived, which is never
+   * elevated, so it does not re-judge the elevation flag.
+   *
+   * Keyed on {@link SEED_USER_GRANTS}, a symbol the package entry does not
+   * export, so it is no nameable member of this class's published face (see
+   * the symbol's docblock in `hook-run-as.ts`).
+   */
+  async [SEED_USER_GRANTS](): Promise<ScopedContext> {
+    const ctx = this.executionContext;
+    if (!ctx.userId) return this;
+    const needsGrants = !Array.isArray(ctx.permissions);
+    const needsPeers = !Array.isArray(ctx.org_user_ids);
+    if (!needsGrants && !needsPeers) return this;
+    const grants = await resolveUserAuthzGrants(
+      this.engine,
+      ctx.userId,
+      ctx.tenantId ? { tenantId: ctx.tenantId } : {},
+    );
+    return new ScopedContext(
+      {
+        ...ctx,
+        ...(needsGrants ? { positions: grants.positions, permissions: grants.permissions } : {}),
+        ...(needsPeers ? { org_user_ids: grants.org_user_ids } : {}),
+      },
+      this.engine
+    );
+  }
+
   /**
    * Execute a callback within a database transaction.
    *

packages/objectql/src/hook-run-as.test.ts

@@ -182,7 +182,9 @@ describe('#14010 Hook.runAs — the identity a hook\'s ctx.api presents', () =>
     }, { logger: silentLogger });
     await wrapped(ctx);
 
-    const context = engine.seen[0]!.context as ExecutionContext;
+    // The hook's own write, not the grants resolution the pin now issues
+    // first (#24497, pinned in its own block below).
+    const context = engine.seen.find((e: { method: string }) => e.method === 'update')!.context as ExecutionContext;
     expect(context.isSystem).toBe(false);
     expect(context.userId).toBe('usr_operator');
   });
@@ -281,6 +283,114 @@ describe('#14010 Hook.runAs — the identity a hook\'s ctx.api presents', () =>
     });
   });
 
+  describe("[#24497] a 'user' pin carries the user's grants and peer list, through the request path's resolver", () => {
+    /**
+     * A recording engine that also answers the reads `resolveUserAuthzGrants`
+     * issues: organisation `org_1` holds `usr_operator` and `usr_peer`.
+     * `failReads` makes every read throw, as an unreadable permission store does.
+     */
+    function grantsEngine(opts: { failReads?: boolean } = {}) {
+      const engine = recordingEngine();
+      engine.find = async (object: string, query: any) => {
+        engine.seen.push({ method: 'find', context: query?.context, object, where: query?.where });
+        if (opts.failReads) throw new Error('permission store unreachable');
+        if (object === 'sys_member' && query?.where?.organization_id === 'org_1') {
+          return [
+            { id: 'm1', user_id: 'usr_operator', organization_id: 'org_1', role: 'member' },
+            { id: 'm2', user_id: 'usr_peer', organization_id: 'org_1', role: 'member' },
+          ];
+        }
+        return [];
+      };
+      return engine;
+    }
+
+    async function hookWrite(execCtx: ExecutionContext, engine: any, runAs = 'user', logger: any = silentLogger) {
+      const wrapped = wrapDeclarativeHook(hookOf(runAs), async (c) => {
+        await (c.api as any).object('account').update({ id: 'acct_1', current_grade: 'A' });
+      }, { logger });
+      await wrapped(makeCtx(execCtx, engine));
+      return engine.seen.find((e: { method: string }) => e.method === 'update')!.context as ExecutionContext;
+    }
+
+    const reads = (engine: any) => engine.seen.filter((e: { method: string }) => e.method === 'find');
+
+    it('a hand-built { isSystem: true, userId, tenantId } trigger: the pin carries the organisation\'s peers, the user included, and nothing beyond', async () => {
+      const engine = grantsEngine();
+      const context = await hookWrite({ isSystem: true, userId: 'usr_operator', tenantId: 'org_1' } as ExecutionContext, engine);
+      expect(context.isSystem).toBe(false);
+      expect([...(context.org_user_ids ?? [])].sort()).toEqual(['usr_operator', 'usr_peer']);
+      // Carried no permissions, so it takes the resolved ones (an array, as on a request).
+      expect(Array.isArray(context.permissions)).toBe(true);
+      expect(Array.isArray(context.positions)).toBe(true);
+      // The peer read is scoped to the context's organisation.
+      expect(reads(engine).some((r: any) => r.object === 'sys_member' && r.where?.organization_id === 'org_1')).toBe(true);
+    });
+
+    it('positions and permission sets already on the context stay VERBATIM; only the peer list is taken', async () => {
+      // The approvals writes' shape: `{ isSystem: true, positions: [], permissions: [], userId }` —
+      // and a narrowed ceiling a door forwarded is the same case: never re-broadened.
+      const engine = grantsEngine();
+      const context = await hookWrite({
+        isSystem: true, userId: 'usr_operator', tenantId: 'org_1', positions: ['narrow'], permissions: ['ceiling_only'],
+      } as ExecutionContext, engine);
+      expect(context.positions).toEqual(['narrow']);
+      expect(context.permissions).toEqual(['ceiling_only']);
+      expect([...(context.org_user_ids ?? [])].sort()).toEqual(['usr_operator', 'usr_peer']);
+    });
+
+    it('a context that already carries a peer list (a request-resolved one) resolves NOTHING and keeps its envelope', async () => {
+      const engine = grantsEngine();
+      const context = await hookWrite({ ...OPERATOR, org_user_ids: ['usr_operator', 'usr_x'] } as ExecutionContext, engine);
+      expect(reads(engine)).toEqual([]);
+      expect(context.org_user_ids).toEqual(['usr_operator', 'usr_x']);
+      expect(context.positions).toEqual(OPERATOR.positions);
+      expect(context.permissions).toEqual(OPERATOR.permissions);
+    });
+
+    it('a trigger with no organisation: the peer list is the user alone, as an organisation-less request\'s is', async () => {
+      const engine = grantsEngine();
+      const context = await hookWrite({ isSystem: true, userId: 'usr_operator' } as ExecutionContext, engine);
+      expect(context.org_user_ids).toEqual(['usr_operator']);
+    });
+
+    it("'system' and 'inherit' resolve nothing, and the user-less refusal does not move", async () => {
+      const sys = grantsEngine();
+      expect((await hookWrite({ isSystem: true, userId: 'usr_operator', tenantId: 'org_1' } as ExecutionContext, sys, 'system')).isSystem).toBe(true);
+      expect(reads(sys)).toEqual([]);
+
+      const inherit = grantsEngine();
+      await hookWrite(OPERATOR, inherit, 'inherit');
+      expect(reads(inherit)).toEqual([]);
+
+      const userless = grantsEngine();
+      const ctx = makeCtx(USERLESS, userless);
+      const wrapped = wrapDeclarativeHook(hookOf('user'), async (c) => {
+        await (c.api as any).object('account').update({ id: 'acct_1', current_grade: 'A' });
+      }, { logger: silentLogger });
+      await expect(wrapped(ctx)).rejects.toMatchObject({ code: HOOK_UNSCOPED_DATA_ACCESS_CODE, status: HOOK_UNSCOPED_DATA_ACCESS_STATUS });
+      expect(userless.seen).toEqual([]);
+    });
+
+    it('the seed is no nameable member of the published face: keyed on a symbol the package entry does not export', async () => {
+      const entry = await import('./index.js');
+      expect('SEED_USER_GRANTS' in entry).toBe(false);
+      expect(Object.getOwnPropertyNames(ScopedContext.prototype)).not.toContain('withUserGrantsSeeded');
+      expect(Object.getOwnPropertySymbols(ScopedContext.prototype)).toHaveLength(1);
+    });
+
+    it('a resolver failure keeps the UNSEEDED pin — narrower, never elevated — and says so once at warn', async () => {
+      const engine = grantsEngine({ failReads: true });
+      const warns: unknown[][] = [];
+      const logger = { ...silentLogger, warn: (...a: unknown[]) => { warns.push(a); } };
+      const context = await hookWrite({ isSystem: true, userId: 'usr_operator', tenantId: 'org_1' } as ExecutionContext, engine, 'user', logger);
+      expect(context.isSystem).toBe(false);
+      expect(context.org_user_ids).toBeUndefined();
+      expect(warns).toHaveLength(1);
+      expect(String(warns[0]![0])).toMatch(/current_user\.org_user_ids drops out/);
+    });
+  });
+
   it('a runAs value outside the enum is refused LOUDLY at bind time, never silently ignored', () => {
     // Declared ≠ enforced is the defect this key exists to end, so a hook that
     // reached the engine without going through `HookSchema` cannot declare an

packages/objectql/src/hook-run-as.ts

@@ -205,3 +205,40 @@ export function deriveHookApi(api: unknown, runAs: HookRunAs, ref: HookRunAsRef)
   }
   return derivable.withRunAs(runAs, ref);
 }
+
+/**
+ * The key the engine's `ScopedContext` carries the second half of a
+ * `runAs: 'user'` derivation under (#24497): seed the user's grants and peer
+ * list onto the api `withRunAs('user')` derived.
+ *
+ * A symbol, and ⛔ deliberately NOT re-exported from the package entry, so the
+ * method is no NAMEABLE member of `ScopedContext`'s published face: only the
+ * hook wrapper reaches it, through {@link seedHookUserGrants}. A public named
+ * method would widen the published surface, and it would also be a derivation
+ * the runtime's stored-metadata seams (`deriveThroughSeam`) do not know, so a
+ * seam-served host api calling it would get back an unserved context.
+ */
+export const SEED_USER_GRANTS: unique symbol = Symbol('objectstack.objectql.hookSeedUserGrants');
+
+/**
+ * What the api a `runAs: 'user'` derivation produced must offer for the
+ * user's grants and peer list to be seeded onto it (#24497). The engine's
+ * `ScopedContext` implements it (`engine.ts`, under {@link SEED_USER_GRANTS});
+ * the refusing {@link UnscopedHookApi} does not, and needs nothing seeded.
+ */
+interface UserGrantsSeedableApi {
+  [SEED_USER_GRANTS](): Promise<IScopedContext>;
+}
+
+/**
+ * Seed the user's grants and peer list onto the api a `runAs: 'user'`
+ * derivation produced, through the core resolver the request path uses
+ * (#24497). Async, so it is a step of its own after {@link deriveHookApi}
+ * rather than a change to that function's synchronous contract. An api that
+ * cannot seed (the refusing {@link UnscopedHookApi}) is returned as is.
+ */
+export async function seedHookUserGrants(api: unknown): Promise<unknown> {
+  const seedable = api as Partial<UserGrantsSeedableApi> | undefined;
+  if (!seedable || typeof seedable[SEED_USER_GRANTS] !== 'function') return api;
+  return seedable[SEED_USER_GRANTS]();
+}

packages/objectql/src/hook-wrappers.ts

@@ -23,7 +23,7 @@ import { noopHookMetricsRecorder, type HookMetricsRecorder, type HookMetricOutco
 import { materializeDeclaredFields } from './declared-fields.js';
 import { describeCelFault, type CelFault } from './cel-fault.js';
 import { nullPreviousReadDetail, ruleScope, type PreviousBinding } from './validation/rule-validator.js';
-import { hookRunAs, deriveHookApi, type HookRunAs } from './hook-run-as.js';
+import { hookRunAs, deriveHookApi, seedHookUserGrants, type HookRunAs } from './hook-run-as.js';
 
 /**
  * The logger the hook layer writes its diagnostics to — the `Logger` CONTRACT
@@ -469,8 +469,8 @@ export function wrapDeclarativeHook(
     // engine's own context object is never touched (the condition gate above
     // already ran against it; the readonly strip and the session keep reading
     // it). Under `'inherit'` this is a no-op that leaves the engine-built api
-    // in place by reference.
-    const restoreApi = installRunAsApi(ctx, runAs, meta.name);
+    // in place by reference — and, deliberately, without an extra `await`.
+    const restoreApi = runAs === 'inherit' ? noRestore : await installRunAsApi(ctx, runAs, meta.name, logger);
     const startedAt = Date.now();
 
     const recordOutcome = (err?: any) => {
@@ -537,19 +537,51 @@ export function wrapDeclarativeHook(
  * only the api holds the full triggering `ExecutionContext` — `ctx.session` is
  * a projection of it and drops the transaction handle, among other things.
  */
-function installRunAsApi(ctx: HookContext, runAs: HookRunAs, hookName: string): () => void {
-  if (runAs === 'inherit') return () => {};
+function installRunAsApi(
+  ctx: HookContext,
+  runAs: HookRunAs,
+  hookName: string,
+  logger: HookDiagnosticsLogger,
+): Promise<() => void> {
+  if (runAs === 'inherit') return Promise.resolve(noRestore);
   const original = ctx.api;
-  ctx.api = deriveHookApi(original, runAs, {
+  const derived = deriveHookApi(original, runAs, {
     hook: hookName,
     object: ctx.object,
     event: ctx.event,
-  }) as HookContext['api'];
-  return () => {
-    ctx.api = original;
+  });
+  const install = (api: unknown) => {
+    ctx.api = api as HookContext['api'];
+    return () => {
+      ctx.api = original;
+    };
   };
+  if (runAs !== 'user') return Promise.resolve(install(derived));
+  // [#24497] A `'user'` pin carries the user's grants and peer list, seeded
+  // through the resolver the request path uses (`ScopedContext[SEED_USER_GRANTS]`, `hook-run-as.ts`).
+  // Cost: one `resolveUserAuthzGrants` per dispatch whose triggering context
+  // carries no `permissions` or no `org_user_ids` — the hand-built system
+  // contexts; a request-resolved context carries both and resolves nothing.
+  return seedHookUserGrants(derived).then(install, (err: unknown) => {
+    // Fail-safe, never fail-open — the flow run setup's posture: the hook keeps
+    // the unseeded pin, so its data ops run on the context's own grants (the
+    // security plugin's member baseline when it carries none, never elevation),
+    // and every row policy reading `current_user.org_user_ids` drops out. It
+    // sees less, never more. FUNCTIONAL, so `warn`: nothing claimed-persisted
+    // failed to land, and the symptom is the hook's own narrower reads.
+    logger.warn(
+      `[hook] runAs 'user': the triggering user's grants could not be resolved, so this hook's data ops run ` +
+        `on the triggering context's own grants (not elevated), and every row policy reading ` +
+        `current_user.org_user_ids drops out`,
+      { hook: hookName, object: ctx.object, event: ctx.event, error: (err as any)?.message },
+    );
+    return install(derived);
+  });
 }
 
+/** The restore of a hook whose `runAs` installed nothing (`'inherit'`). */
+const noRestore = (): void => {};
+
 /**
  * Swap `ctx.input` in place for a Proxy that exposes a flat record view
  * over the engine's `{ data, options, id? }` wrapper. Returns a function