Skip to content

Commit beed776

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-24531-8b0d-identity-mail-sibling
2 parents 344aeb8 + ba4afcc commit beed776

10 files changed

Lines changed: 671 additions & 34 deletions
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
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
6+
7+
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.
8+
9+
The hook wrapper now seeds the pinned context through the resolver the request path uses (`resolveUserAuthzGrants` from `@objectstack/core`):
10+
11+
- 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.
12+
- 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.
13+
- A context that already carries a peer list, such as a request-resolved one, resolves nothing and is handed through as before.
14+
- 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.
15+
16+
`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.
17+
18+
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‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,9 @@ import {
146146
// [ADR-0131 D8] The ONE tenant read predicate, shared with plugin-security's
147147
// Layer 0 — see {@link buildDriverOptions}.
148148
computeTenantLayer0Verdict,
149+
// [#24497] The ONE grants resolver, the one the request path seeds a
150+
// context's grants and peer list through — see `ScopedContext[SEED_USER_GRANTS]`.
151+
resolveUserAuthzGrants,
149152
} from '@objectstack/core';
150153
import { WriteEpoch, isWriteEpochOperation } from './write-epoch.js';
151154
import { bridgeAuthzInvalidation } from './authz-invalidation-bridge.js';
@@ -310,7 +313,7 @@ import {
310313
divergingHookPayloadKeys,
311314
MultiUpdateHookKeyDivergenceError,
312315
} from './multi-update-hook-key-divergence.js';
313-
import { UnscopedHookApi, type HookRunAs, type HookRunAsRef, type RunAsDerivableApi } from './hook-run-as.js';
316+
import { UnscopedHookApi, SEED_USER_GRANTS, type HookRunAs, type HookRunAsRef, type RunAsDerivableApi } from './hook-run-as.js';
314317
import type { HookWriteRecording } from './hook-write-provenance.js';
315318
import { resolveMasterDetailRelation } from './master-detail.js';
316319
// [#19911] The settlement's question "does judging the FK's own lock need the
@@ -21816,6 +21819,61 @@ export class ScopedContext implements IScopedContext, RunAsDerivableApi {
2181621819
);
2181721820
}
2181821821

21822+
/**
21823+
* [#24497] The second half of a hook's `runAs: 'user'` derivation: the
21824+
* user's grants and peer list, seeded through the one resolver the request
21825+
* path seeds them through (`resolveUserAuthzGrants`, `@objectstack/core`).
21826+
* The hook wrapper awaits it on the api {@link withRunAs} derived; it stays
21827+
* a separate, async step so `withRunAs` keeps its synchronous contract.
21828+
*
21829+
* A hook fired from a hand-built `{ isSystem: true, userId }` context (the
21830+
* approvals writes) was pinned with no `org_user_ids`, so every row policy
21831+
* reading `current_user.org_user_ids` compiled `unresolved-variable` and
21832+
* dropped out: the hook saw its own `sys_user` row only, while the same
21833+
* user's request saw their organisation's members too. Ruling A′ puts the
21834+
* contract on the producer: every user context carries `org_user_ids`.
21835+
*
21836+
* The same rule as a `runAs: 'user'` flow's run setup (service-automation
21837+
* `resolveRunContext`):
21838+
* - positions and permission sets the context already carries stay
21839+
* verbatim, so a narrowed ceiling is never re-broadened; only a context
21840+
* with no `permissions` takes the resolved ones;
21841+
* - a context that already carries a peer list keeps it;
21842+
* - a context that needs neither resolves nothing and is returned as is.
21843+
*
21844+
* The peer list is the members of the context's `tenantId`, the user
21845+
* included; a context with no `tenantId` gets the user alone, as an
21846+
* organisation-less request does. A resolver failure (an unreadable
21847+
* permission store) is thrown to the caller, which keeps the unseeded api.
21848+
*
21849+
* Called on the api {@link withRunAs}`('user')` derived, which is never
21850+
* elevated, so it does not re-judge the elevation flag.
21851+
*
21852+
* Keyed on {@link SEED_USER_GRANTS}, a symbol the package entry does not
21853+
* export, so it is no nameable member of this class's published face (see
21854+
* the symbol's docblock in `hook-run-as.ts`).
21855+
*/
21856+
async [SEED_USER_GRANTS](): Promise<ScopedContext> {
21857+
const ctx = this.executionContext;
21858+
if (!ctx.userId) return this;
21859+
const needsGrants = !Array.isArray(ctx.permissions);
21860+
const needsPeers = !Array.isArray(ctx.org_user_ids);
21861+
if (!needsGrants && !needsPeers) return this;
21862+
const grants = await resolveUserAuthzGrants(
21863+
this.engine,
21864+
ctx.userId,
21865+
ctx.tenantId ? { tenantId: ctx.tenantId } : {},
21866+
);
21867+
return new ScopedContext(
21868+
{
21869+
...ctx,
21870+
...(needsGrants ? { positions: grants.positions, permissions: grants.permissions } : {}),
21871+
...(needsPeers ? { org_user_ids: grants.org_user_ids } : {}),
21872+
},
21873+
this.engine
21874+
);
21875+
}
21876+
2181921877
/**
2182021878
* Execute a callback within a database transaction.
2182121879
*

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

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,9 @@ describe('#14010 Hook.runAs — the identity a hook\'s ctx.api presents', () =>
182182
}, { logger: silentLogger });
183183
await wrapped(ctx);
184184

185-
const context = engine.seen[0]!.context as ExecutionContext;
185+
// The hook's own write, not the grants resolution the pin now issues
186+
// first (#24497, pinned in its own block below).
187+
const context = engine.seen.find((e: { method: string }) => e.method === 'update')!.context as ExecutionContext;
186188
expect(context.isSystem).toBe(false);
187189
expect(context.userId).toBe('usr_operator');
188190
});
@@ -281,6 +283,114 @@ describe('#14010 Hook.runAs — the identity a hook\'s ctx.api presents', () =>
281283
});
282284
});
283285

286+
describe("[#24497] a 'user' pin carries the user's grants and peer list, through the request path's resolver", () => {
287+
/**
288+
* A recording engine that also answers the reads `resolveUserAuthzGrants`
289+
* issues: organisation `org_1` holds `usr_operator` and `usr_peer`.
290+
* `failReads` makes every read throw, as an unreadable permission store does.
291+
*/
292+
function grantsEngine(opts: { failReads?: boolean } = {}) {
293+
const engine = recordingEngine();
294+
engine.find = async (object: string, query: any) => {
295+
engine.seen.push({ method: 'find', context: query?.context, object, where: query?.where });
296+
if (opts.failReads) throw new Error('permission store unreachable');
297+
if (object === 'sys_member' && query?.where?.organization_id === 'org_1') {
298+
return [
299+
{ id: 'm1', user_id: 'usr_operator', organization_id: 'org_1', role: 'member' },
300+
{ id: 'm2', user_id: 'usr_peer', organization_id: 'org_1', role: 'member' },
301+
];
302+
}
303+
return [];
304+
};
305+
return engine;
306+
}
307+
308+
async function hookWrite(execCtx: ExecutionContext, engine: any, runAs = 'user', logger: any = silentLogger) {
309+
const wrapped = wrapDeclarativeHook(hookOf(runAs), async (c) => {
310+
await (c.api as any).object('account').update({ id: 'acct_1', current_grade: 'A' });
311+
}, { logger });
312+
await wrapped(makeCtx(execCtx, engine));
313+
return engine.seen.find((e: { method: string }) => e.method === 'update')!.context as ExecutionContext;
314+
}
315+
316+
const reads = (engine: any) => engine.seen.filter((e: { method: string }) => e.method === 'find');
317+
318+
it('a hand-built { isSystem: true, userId, tenantId } trigger: the pin carries the organisation\'s peers, the user included, and nothing beyond', async () => {
319+
const engine = grantsEngine();
320+
const context = await hookWrite({ isSystem: true, userId: 'usr_operator', tenantId: 'org_1' } as ExecutionContext, engine);
321+
expect(context.isSystem).toBe(false);
322+
expect([...(context.org_user_ids ?? [])].sort()).toEqual(['usr_operator', 'usr_peer']);
323+
// Carried no permissions, so it takes the resolved ones (an array, as on a request).
324+
expect(Array.isArray(context.permissions)).toBe(true);
325+
expect(Array.isArray(context.positions)).toBe(true);
326+
// The peer read is scoped to the context's organisation.
327+
expect(reads(engine).some((r: any) => r.object === 'sys_member' && r.where?.organization_id === 'org_1')).toBe(true);
328+
});
329+
330+
it('positions and permission sets already on the context stay VERBATIM; only the peer list is taken', async () => {
331+
// The approvals writes' shape: `{ isSystem: true, positions: [], permissions: [], userId }` —
332+
// and a narrowed ceiling a door forwarded is the same case: never re-broadened.
333+
const engine = grantsEngine();
334+
const context = await hookWrite({
335+
isSystem: true, userId: 'usr_operator', tenantId: 'org_1', positions: ['narrow'], permissions: ['ceiling_only'],
336+
} as ExecutionContext, engine);
337+
expect(context.positions).toEqual(['narrow']);
338+
expect(context.permissions).toEqual(['ceiling_only']);
339+
expect([...(context.org_user_ids ?? [])].sort()).toEqual(['usr_operator', 'usr_peer']);
340+
});
341+
342+
it('a context that already carries a peer list (a request-resolved one) resolves NOTHING and keeps its envelope', async () => {
343+
const engine = grantsEngine();
344+
const context = await hookWrite({ ...OPERATOR, org_user_ids: ['usr_operator', 'usr_x'] } as ExecutionContext, engine);
345+
expect(reads(engine)).toEqual([]);
346+
expect(context.org_user_ids).toEqual(['usr_operator', 'usr_x']);
347+
expect(context.positions).toEqual(OPERATOR.positions);
348+
expect(context.permissions).toEqual(OPERATOR.permissions);
349+
});
350+
351+
it('a trigger with no organisation: the peer list is the user alone, as an organisation-less request\'s is', async () => {
352+
const engine = grantsEngine();
353+
const context = await hookWrite({ isSystem: true, userId: 'usr_operator' } as ExecutionContext, engine);
354+
expect(context.org_user_ids).toEqual(['usr_operator']);
355+
});
356+
357+
it("'system' and 'inherit' resolve nothing, and the user-less refusal does not move", async () => {
358+
const sys = grantsEngine();
359+
expect((await hookWrite({ isSystem: true, userId: 'usr_operator', tenantId: 'org_1' } as ExecutionContext, sys, 'system')).isSystem).toBe(true);
360+
expect(reads(sys)).toEqual([]);
361+
362+
const inherit = grantsEngine();
363+
await hookWrite(OPERATOR, inherit, 'inherit');
364+
expect(reads(inherit)).toEqual([]);
365+
366+
const userless = grantsEngine();
367+
const ctx = makeCtx(USERLESS, userless);
368+
const wrapped = wrapDeclarativeHook(hookOf('user'), async (c) => {
369+
await (c.api as any).object('account').update({ id: 'acct_1', current_grade: 'A' });
370+
}, { logger: silentLogger });
371+
await expect(wrapped(ctx)).rejects.toMatchObject({ code: HOOK_UNSCOPED_DATA_ACCESS_CODE, status: HOOK_UNSCOPED_DATA_ACCESS_STATUS });
372+
expect(userless.seen).toEqual([]);
373+
});
374+
375+
it('the seed is no nameable member of the published face: keyed on a symbol the package entry does not export', async () => {
376+
const entry = await import('./index.js');
377+
expect('SEED_USER_GRANTS' in entry).toBe(false);
378+
expect(Object.getOwnPropertyNames(ScopedContext.prototype)).not.toContain('withUserGrantsSeeded');
379+
expect(Object.getOwnPropertySymbols(ScopedContext.prototype)).toHaveLength(1);
380+
});
381+
382+
it('a resolver failure keeps the UNSEEDED pin — narrower, never elevated — and says so once at warn', async () => {
383+
const engine = grantsEngine({ failReads: true });
384+
const warns: unknown[][] = [];
385+
const logger = { ...silentLogger, warn: (...a: unknown[]) => { warns.push(a); } };
386+
const context = await hookWrite({ isSystem: true, userId: 'usr_operator', tenantId: 'org_1' } as ExecutionContext, engine, 'user', logger);
387+
expect(context.isSystem).toBe(false);
388+
expect(context.org_user_ids).toBeUndefined();
389+
expect(warns).toHaveLength(1);
390+
expect(String(warns[0]![0])).toMatch(/current_user\.org_user_ids drops out/);
391+
});
392+
});
393+
284394
it('a runAs value outside the enum is refused LOUDLY at bind time, never silently ignored', () => {
285395
// Declared ≠ enforced is the defect this key exists to end, so a hook that
286396
// reached the engine without going through `HookSchema` cannot declare an

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,3 +205,40 @@ export function deriveHookApi(api: unknown, runAs: HookRunAs, ref: HookRunAsRef)
205205
}
206206
return derivable.withRunAs(runAs, ref);
207207
}
208+
209+
/**
210+
* The key the engine's `ScopedContext` carries the second half of a
211+
* `runAs: 'user'` derivation under (#24497): seed the user's grants and peer
212+
* list onto the api `withRunAs('user')` derived.
213+
*
214+
* A symbol, and ⛔ deliberately NOT re-exported from the package entry, so the
215+
* method is no NAMEABLE member of `ScopedContext`'s published face: only the
216+
* hook wrapper reaches it, through {@link seedHookUserGrants}. A public named
217+
* method would widen the published surface, and it would also be a derivation
218+
* the runtime's stored-metadata seams (`deriveThroughSeam`) do not know, so a
219+
* seam-served host api calling it would get back an unserved context.
220+
*/
221+
export const SEED_USER_GRANTS: unique symbol = Symbol('objectstack.objectql.hookSeedUserGrants');
222+
223+
/**
224+
* What the api a `runAs: 'user'` derivation produced must offer for the
225+
* user's grants and peer list to be seeded onto it (#24497). The engine's
226+
* `ScopedContext` implements it (`engine.ts`, under {@link SEED_USER_GRANTS});
227+
* the refusing {@link UnscopedHookApi} does not, and needs nothing seeded.
228+
*/
229+
interface UserGrantsSeedableApi {
230+
[SEED_USER_GRANTS](): Promise<IScopedContext>;
231+
}
232+
233+
/**
234+
* Seed the user's grants and peer list onto the api a `runAs: 'user'`
235+
* derivation produced, through the core resolver the request path uses
236+
* (#24497). Async, so it is a step of its own after {@link deriveHookApi}
237+
* rather than a change to that function's synchronous contract. An api that
238+
* cannot seed (the refusing {@link UnscopedHookApi}) is returned as is.
239+
*/
240+
export async function seedHookUserGrants(api: unknown): Promise<unknown> {
241+
const seedable = api as Partial<UserGrantsSeedableApi> | undefined;
242+
if (!seedable || typeof seedable[SEED_USER_GRANTS] !== 'function') return api;
243+
return seedable[SEED_USER_GRANTS]();
244+
}

‎packages/objectql/src/hook-wrappers.ts‎

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { noopHookMetricsRecorder, type HookMetricsRecorder, type HookMetricOutco
2323
import { materializeDeclaredFields } from './declared-fields.js';
2424
import { describeCelFault, type CelFault } from './cel-fault.js';
2525
import { nullPreviousReadDetail, ruleScope, type PreviousBinding } from './validation/rule-validator.js';
26-
import { hookRunAs, deriveHookApi, type HookRunAs } from './hook-run-as.js';
26+
import { hookRunAs, deriveHookApi, seedHookUserGrants, type HookRunAs } from './hook-run-as.js';
2727

2828
/**
2929
* The logger the hook layer writes its diagnostics to — the `Logger` CONTRACT
@@ -469,8 +469,8 @@ export function wrapDeclarativeHook(
469469
// engine's own context object is never touched (the condition gate above
470470
// already ran against it; the readonly strip and the session keep reading
471471
// it). Under `'inherit'` this is a no-op that leaves the engine-built api
472-
// in place by reference.
473-
const restoreApi = installRunAsApi(ctx, runAs, meta.name);
472+
// in place by reference — and, deliberately, without an extra `await`.
473+
const restoreApi = runAs === 'inherit' ? noRestore : await installRunAsApi(ctx, runAs, meta.name, logger);
474474
const startedAt = Date.now();
475475

476476
const recordOutcome = (err?: any) => {
@@ -537,19 +537,51 @@ export function wrapDeclarativeHook(
537537
* only the api holds the full triggering `ExecutionContext` — `ctx.session` is
538538
* a projection of it and drops the transaction handle, among other things.
539539
*/
540-
function installRunAsApi(ctx: HookContext, runAs: HookRunAs, hookName: string): () => void {
541-
if (runAs === 'inherit') return () => {};
540+
function installRunAsApi(
541+
ctx: HookContext,
542+
runAs: HookRunAs,
543+
hookName: string,
544+
logger: HookDiagnosticsLogger,
545+
): Promise<() => void> {
546+
if (runAs === 'inherit') return Promise.resolve(noRestore);
542547
const original = ctx.api;
543-
ctx.api = deriveHookApi(original, runAs, {
548+
const derived = deriveHookApi(original, runAs, {
544549
hook: hookName,
545550
object: ctx.object,
546551
event: ctx.event,
547-
}) as HookContext['api'];
548-
return () => {
549-
ctx.api = original;
552+
});
553+
const install = (api: unknown) => {
554+
ctx.api = api as HookContext['api'];
555+
return () => {
556+
ctx.api = original;
557+
};
550558
};
559+
if (runAs !== 'user') return Promise.resolve(install(derived));
560+
// [#24497] A `'user'` pin carries the user's grants and peer list, seeded
561+
// through the resolver the request path uses (`ScopedContext[SEED_USER_GRANTS]`, `hook-run-as.ts`).
562+
// Cost: one `resolveUserAuthzGrants` per dispatch whose triggering context
563+
// carries no `permissions` or no `org_user_ids` — the hand-built system
564+
// contexts; a request-resolved context carries both and resolves nothing.
565+
return seedHookUserGrants(derived).then(install, (err: unknown) => {
566+
// Fail-safe, never fail-open — the flow run setup's posture: the hook keeps
567+
// the unseeded pin, so its data ops run on the context's own grants (the
568+
// security plugin's member baseline when it carries none, never elevation),
569+
// and every row policy reading `current_user.org_user_ids` drops out. It
570+
// sees less, never more. FUNCTIONAL, so `warn`: nothing claimed-persisted
571+
// failed to land, and the symptom is the hook's own narrower reads.
572+
logger.warn(
573+
`[hook] runAs 'user': the triggering user's grants could not be resolved, so this hook's data ops run ` +
574+
`on the triggering context's own grants (not elevated), and every row policy reading ` +
575+
`current_user.org_user_ids drops out`,
576+
{ hook: hookName, object: ctx.object, event: ctx.event, error: (err as any)?.message },
577+
);
578+
return install(derived);
579+
});
551580
}
552581

582+
/** The restore of a hook whose `runAs` installed nothing (`'inherit'`). */
583+
const noRestore = (): void => {};
584+
553585
/**
554586
* Swap `ctx.input` in place for a Proxy that exposes a flat record view
555587
* over the engine's `{ data, options, id? }` wrapper. Returns a function

0 commit comments

Comments
 (0)