diff --git a/.changeset/runas-user-grant-resolution-3356.md b/.changeset/runas-user-grant-resolution-3356.md new file mode 100644 index 0000000000..4f5be47aad --- /dev/null +++ b/.changeset/runas-user-grant-resolution-3356.md @@ -0,0 +1,49 @@ +--- +"@objectstack/core": minor +"@objectstack/service-automation": minor +"@objectstack/trigger-record-change": patch +--- + +fix(service-automation): `runAs:'user'` runs data ops with the triggering user's +real permission sets + positions, not a bare member fallback (#3356, follow-up to +#1888) + +Since #1888 the automation engine honours `flow.runAs` (`system` elevates), but +the `runAs:'user'` credential propagation was hollow. A record-change-triggered +`runAs:'user'` flow ran its data nodes (`update_record`, …) with a **zero-grant** +principal — only the `member`/`everyone` baseline — even when the triggering user +was fully authorized. Two faces by object config: a `private` object 403'd the +in-flow write (`not permitted for positions [org_member, everyone]` — the user's +permission sets were invisible); a `public_read_write` object let the write +through but **silently stripped** readonly/FLS-gated fields. The root cause: the +ObjectQL record-change hook session carries only a `userId` — never the writer's +positions/permission sets — and nothing in between resolved them, so the comment +promising "enforces RLS exactly as the user who made the change" never held. + +The fix resolves the triggering user's **actual** authorization at run setup, from +the same tables a direct REST request resolves through: + +- **`@objectstack/core`** factors the userId-driven core of `resolveAuthzContext` + into a new exported `resolveUserAuthzGrants(ql, userId, opts)` — the single place + that reads `sys_member` / `sys_user_position` / `sys_*_permission_set` and + derives positions, permission-set names, `platform_admin`, and posture. The + HTTP resolver now delegates to it (behaviour byte-identical; the full contract + suite still passes), so a non-HTTP surface that already knows the user id builds + the SAME envelope instead of re-implementing the reads. +- **`@objectstack/service-automation`** gains `AutomationEngine.setUserGrantsResolver`, + wired by the plugin to `resolveUserAuthzGrants` over the objectql/data engine. + For a `runAs:'user'` run whose trigger left the authz envelope unresolved (no + `permissions`), the engine now resolves the user's positions + permission sets + once at run setup and threads them into every data node's ObjectQL context — + so the run enforces RLS/FLS exactly as that user. Contexts that already carry + `permissions` are left untouched (a REST trigger, and notably an ADR-0090 agent + ceiling acting on-behalf-of a user — always non-empty — so a deliberately + narrowed identity is never re-broadened). `runAs:'system'` is unchanged, and a + resolver error fails safe (warns, keeps the bare user — never elevates). +- **`@objectstack/trigger-record-change`** stops forwarding the misleading + half-populated `positions` (empty in practice, and never `permissions`) from the + hook session; it forwards `userId` + tenant only and lets the engine resolve the + full grants authoritatively. + +When no ObjectQL engine is present (bare engine / tests) the resolver is unwired +and run identity is unchanged from before. diff --git a/packages/core/src/security/index.ts b/packages/core/src/security/index.ts index e7da5a3528..cf9f330a9b 100644 --- a/packages/core/src/security/index.ts +++ b/packages/core/src/security/index.ts @@ -82,9 +82,12 @@ export { export { resolveAuthzContext, + resolveUserAuthzGrants, resolveLocalizationContext, type ResolvedAuthzContext, type ResolveAuthzInput, + type UserAuthzGrants, + type ResolveUserAuthzGrantsOptions, type ResolveLocalizationInput, } from './resolve-authz-context.js'; diff --git a/packages/core/src/security/resolve-authz-context.test.ts b/packages/core/src/security/resolve-authz-context.test.ts index c9e8a5961d..ffee6d90e3 100644 --- a/packages/core/src/security/resolve-authz-context.test.ts +++ b/packages/core/src/security/resolve-authz-context.test.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { resolveAuthzContext, resolveLocalizationContext } from './resolve-authz-context.js'; +import { resolveAuthzContext, resolveUserAuthzGrants, resolveLocalizationContext } from './resolve-authz-context.js'; import { POSTURE_RANK } from './posture-ladder.js'; import type { AuthzPosture } from '@objectstack/spec/security'; @@ -375,3 +375,94 @@ describe('resolveAuthzContext — posture ladder (ADR-0095 D2/D3)', () => { }); }); +/** + * #3356 — the userId-driven core, callable WITHOUT an HTTP request. A + * `runAs:'user'` automation run knows the triggering user's id (the record-change + * hook session carries only that) and must build the SAME positions/permissions + * envelope a direct REST request from that user would resolve, so its data ops + * enforce RLS as that user — not the bare member/everyone fallback. + */ +describe('resolveUserAuthzGrants — userId-driven authz for non-HTTP surfaces (#3356)', () => { + it("resolves a known user's positions + permission-set names from the DB", async () => { + const ql = makeQl({ + sys_user: [{ id: 'u1', email: 'ada@x.com' }], + sys_member: [{ user_id: 'u1', role: 'admin', organization_id: 'o1' }], + sys_user_position: [{ user_id: 'u1', position: 'approver', organization_id: null }], + sys_user_permission_set: [{ user_id: 'u1', permission_set_id: 'psA', organization_id: null }], + sys_permission_set: [{ id: 'psA', name: 'ehr_all', system_permissions: ['cap_ehr'] }], + }); + const grants = await resolveUserAuthzGrants(ql, 'u1', { tenantId: 'o1' }); + expect(grants.positions).toContain('org_admin'); // sys_member owner/admin normalized + expect(grants.positions).toContain('approver'); // sys_user_position + expect(grants.positions).toContain('everyone'); // implicit audience anchor + expect(grants.permissions).toContain('ehr_all'); // user-scoped permission set + expect(grants.systemPermissions).toContain('cap_ehr'); + expect(grants.email).toBe('ada@x.com'); + }); + + it('matches resolveAuthzContext for the same user — one resolver, one envelope', async () => { + const tables = { + sys_user: [{ id: 'u1', email: 'ada@x.com' }], + sys_member: [], + sys_user_position: [{ user_id: 'u1', position: 'contributor', organization_id: null }], + sys_user_permission_set: [{ user_id: 'u1', permission_set_id: 'ps1', organization_id: null }], + sys_position: [{ id: 'r1', name: 'contributor' }], + sys_position_permission_set: [{ position_id: 'r1', permission_set_id: 'ps1' }], + sys_permission_set: [{ id: 'ps1', name: 'contributor_ps', system_permissions: ['cap_x'] }], + }; + const viaHttp = await resolveAuthzContext({ ql: makeQl(tables), headers: H(), getSession: session('u1') }); + const viaUser = await resolveUserAuthzGrants(makeQl(tables), 'u1'); + expect([...viaUser.positions].sort()).toEqual([...viaHttp.positions].sort()); + expect([...viaUser.permissions].sort()).toEqual([...viaHttp.permissions].sort()); + expect([...viaUser.systemPermissions].sort()).toEqual([...viaHttp.systemPermissions].sort()); + expect(viaUser.posture).toBe(viaHttp.posture); + }); + + it('seeds caller-supplied permissions FIRST, then appends resolved set names', async () => { + const ql = makeQl({ + sys_user: [{ id: 'u1' }], + sys_member: [], + sys_user_position: [], + sys_user_permission_set: [{ user_id: 'u1', permission_set_id: 'ps1', organization_id: null }], + sys_permission_set: [{ id: 'ps1', name: 'sales_ps' }], + }); + const grants = await resolveUserAuthzGrants(ql, 'u1', { seedPermissions: ['api:scope'] }); + expect(grants.permissions[0]).toBe('api:scope'); + expect(grants.permissions).toContain('sales_ps'); + }); + + it('a caller-supplied email wins over the sys_user read', async () => { + const ql = makeQl({ sys_user: [{ id: 'u1', email: 'db@x.com' }], sys_member: [], sys_user_position: [], sys_user_permission_set: [] }); + const grants = await resolveUserAuthzGrants(ql, 'u1', { seedEmail: 'session@x.com' }); + expect(grants.email).toBe('session@x.com'); + }); + + it('a user with no grants gets the implicit everyone anchor, empty permissions (never null)', async () => { + const ql = makeQl({ sys_user: [{ id: 'u1' }], sys_member: [], sys_user_position: [], sys_user_permission_set: [] }); + const grants = await resolveUserAuthzGrants(ql, 'u1'); + expect(grants.positions).toEqual(['everyone']); + expect(grants.permissions).toEqual([]); + expect(grants.org_user_ids).toEqual(['u1']); + }); + + it('fail-closed: no data engine yields an empty-but-valid envelope and never throws', async () => { + const grants = await resolveUserAuthzGrants(undefined, 'u1', { seedPermissions: ['api:scope'] }); + expect(grants.positions).toEqual([]); + expect(grants.permissions).toEqual(['api:scope']); + expect(grants.org_user_ids).toEqual(['u1']); + }); + + it('drops permission-set grants outside their validity window (ADR-0091)', async () => { + const past = new Date(Date.now() - 86_400_000).toISOString(); + const ql = makeQl({ + sys_user: [{ id: 'u1' }], + sys_member: [], + sys_user_position: [], + sys_user_permission_set: [{ user_id: 'u1', permission_set_id: 'psA', organization_id: null, valid_until: past }], + sys_permission_set: [{ id: 'psA', name: 'expired_ps' }], + }); + const grants = await resolveUserAuthzGrants(ql, 'u1'); + expect(grants.permissions).not.toContain('expired_ps'); + }); +}); + diff --git a/packages/core/src/security/resolve-authz-context.ts b/packages/core/src/security/resolve-authz-context.ts index 84840c6164..9a2e26cc3c 100644 --- a/packages/core/src/security/resolve-authz-context.ts +++ b/packages/core/src/security/resolve-authz-context.ts @@ -135,6 +135,97 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise; + posture?: AuthzPosture; + /** The user's unique email (`sys_user`), for `current_user.email` owner RLS. */ + email?: string; +} + +export interface ResolveUserAuthzGrantsOptions { + /** Active org/tenant id — scopes org-bound grants (a null-org row is global). */ + tenantId?: string; + /** Clock injection for grant validity windows (tests). */ + nowMs?: number; + /** + * Permission names the CALLER already resolved (e.g. API-key scopes) to seed + * `permissions` BEFORE permission-set names are appended, so a mixed + * API-key+session principal keeps every scope and the ordering is preserved. + * Copied, never mutated. + */ + seedPermissions?: string[]; + /** A caller-supplied email (e.g. from the session) that wins over the `sys_user` read. */ + seedEmail?: string; +} + +/** + * resolveUserAuthzGrants — the userId-driven core of {@link resolveAuthzContext}. + * + * Given a KNOWN user id, aggregate the authorization grants that user holds: + * org-admin positions (`sys_member`), platform-RBAC positions + * (`sys_user_position`), user- and position-bound permission sets + * (`sys_user_permission_set` / `sys_position_permission_set` → + * `sys_permission_set`), the derived `platform_admin` built-in + posture rung, + * fellow-org peers for identity-table RLS, and the env-side `ai_seat`. + * + * Factored out of `resolveAuthzContext` so a surface that already knows WHO the + * principal is — with no HTTP request to resolve it from — can build the SAME + * envelope through the ONE resolver, instead of re-reading `sys_member` / + * `sys_user_position` / `sys_*_permission_set` itself. The motivating consumer + * is a `runAs:'user'` automation run resolving the triggering user's grants + * (#3356): the record-change hook session carries only a `userId`, so the + * automation engine calls this to run the flow's data ops exactly as that user + * — not the bare member/everyone fallback the missing grants used to leave it. + * + * Fail-closed like its parent: every read is defensive, a missing engine/table + * yields an empty-but-valid envelope, and it never throws. + */ +export async function resolveUserAuthzGrants( + ql: any, + userId: string, + opts: ResolveUserAuthzGrantsOptions = {}, +): Promise { + const { tenantId } = opts; + const grants: UserAuthzGrants = { + positions: [], + permissions: Array.isArray(opts.seedPermissions) ? [...opts.seedPermissions] : [], + systemPermissions: [], + org_user_ids: [userId], + }; + if (opts.seedEmail) grants.email = opts.seedEmail; + if (!ql || typeof ql.find !== 'function') return grants; + // sys_user is needed for both the `current_user.email` fallback (API-key auth, // where the session didn't supply an email) and the ai_seat synthesis below. // Read the row at most once per resolution — the two reads were a duplicate @@ -151,10 +242,10 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise s.trim()).filter(Boolean)) { const r = mapMembershipRole(raw); - if (!ctx.positions.includes(r)) ctx.positions.push(r); + if (!grants.positions.includes(r)) grants.positions.push(r); } } } @@ -180,7 +271,7 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise typeof v === 'string' && v.length > 0), ); ids.add(userId); - ctx.org_user_ids = Array.from(ids); - } else { - ctx.org_user_ids = [userId]; + grants.org_user_ids = Array.from(ids); } // 6. Permission sets — user-scoped grants (null org = global, else active org). @@ -236,12 +325,12 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise 0) { - const positionRows = await tryFind(ql, 'sys_position', { name: { $in: ctx.positions } }, 100); + if (grants.positions.length > 0) { + const positionRows = await tryFind(ql, 'sys_position', { name: { $in: grants.positions } }, 100); const positionIds = positionRows.map((r) => r.id).filter(Boolean); if (positionIds.length > 0) { const rpsRows = await tryFind(ql, 'sys_position_permission_set', { position_id: { $in: positionIds } }, 500); @@ -252,21 +341,21 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise 0) { const psRows = await tryFind(ql, 'sys_permission_set', { id: { $in: Array.from(psIds) } }, 500); const tabRank: Record = { hidden: 0, default_off: 1, default_on: 2, visible: 3 }; const mergedTabs: Record = {}; for (const ps of psRows) { - if (ps.name && !ctx.permissions.includes(ps.name)) ctx.permissions.push(ps.name); + if (ps.name && !grants.permissions.includes(ps.name)) grants.permissions.push(ps.name); if (ps.name === ADMIN_FULL_ACCESS && unscopedUserPsIds.has(ps.id)) hasPlatformAdminGrant = true; const sysPerms = typeof ps.system_permissions === 'string' ? safeJsonParse(ps.system_permissions, []) : (ps.system_permissions ?? ps.systemPermissions); if (Array.isArray(sysPerms)) { for (const p of sysPerms) { - if (typeof p === 'string' && !ctx.systemPermissions.includes(p)) ctx.systemPermissions.push(p); + if (typeof p === 'string' && !grants.systemPermissions.includes(p)) grants.systemPermissions.push(p); } } const tabs = typeof ps.tab_permissions === 'string' @@ -282,12 +371,12 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise 0) ctx.tabPermissions = mergedTabs; + if (Object.keys(mergedTabs).length > 0) grants.tabPermissions = mergedTabs; } // 6c. Project the derived platform_admin built-in role (leads the list). - if (hasPlatformAdminGrant && !ctx.positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN)) { - ctx.positions.unshift(BUILTIN_IDENTITY_PLATFORM_ADMIN); + if (hasPlatformAdminGrant && !grants.positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN)) { + grants.positions.unshift(BUILTIN_IDENTITY_PLATFORM_ADMIN); } // 6d. [ADR-0095 D2/D3] Resolve the posture rung ONCE, from held CAPABILITY @@ -300,19 +389,19 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise { + it('resolves positions + permission sets and threads them to EVERY data op', async () => { + const engine = new AutomationEngine(makeLogger()); + const { data, calls } = fakeData(); + registerCrudNodes(engine, ctxWith(data)); + engine.registerFlow('usr', allOpsFlow('usr', 'user')); + + const seen: Array<{ userId: string; tenantId?: string }> = []; + engine.setUserGrantsResolver((userId, tenantId) => { + seen.push({ userId, tenantId }); + return { positions: ['approver', 'everyone'], permissions: ['ehr_all'], tenantId: 'org1' }; + }); + + // The record-change hook shape: ONLY a userId (no positions/permissions). + const res = await engine.execute('usr', { userId: 'u1' }); + expect(res.success).toBe(true); + + // Resolver invoked exactly once per run, for the triggering user. + expect(seen).toEqual([{ userId: 'u1', tenantId: undefined }]); + + for (const c of calls) { + expect(c.ctx.isSystem, `${c.op} wrongly elevated`).toBe(false); + expect(c.ctx.userId).toBe('u1'); + expect(c.ctx.positions).toEqual(['approver', 'everyone']); + expect(c.ctx.permissions, `${c.op} lost the resolved permission sets`).toEqual(['ehr_all']); + expect(c.ctx.tenantId).toBe('org1'); + } + }); + + it('does NOT re-resolve when the trigger already carried permissions (agent/REST envelope preserved)', async () => { + const engine = new AutomationEngine(makeLogger()); + const { data, calls } = fakeData(); + registerCrudNodes(engine, ctxWith(data)); + engine.registerFlow('usr', allOpsFlow('usr', 'user')); + + let called = 0; + engine.setUserGrantsResolver(() => { + called++; + return { positions: ['SHOULD_NOT_APPLY'], permissions: ['SHOULD_NOT_APPLY'] }; + }); + + // An already-resolved envelope — e.g. an ADR-0090 agent ceiling acting + // on-behalf-of a user (always non-empty). Must be honored verbatim, NOT + // re-broadened to the human's full grants. + await engine.execute('usr', { userId: 'u1', positions: ['agent'], permissions: ['mcp_agent_read'], tenantId: 't1' }); + expect(called).toBe(0); + for (const c of calls) { + expect(c.ctx.permissions).toEqual(['mcp_agent_read']); + expect(c.ctx.positions).toEqual(['agent']); + } + }); + + it("does NOT resolve for runAs:'system' (explicit elevation wins)", async () => { + const engine = new AutomationEngine(makeLogger()); + const { data, calls } = fakeData(); + registerCrudNodes(engine, ctxWith(data)); + engine.registerFlow('sys', allOpsFlow('sys', 'system')); + let called = 0; + engine.setUserGrantsResolver(() => { called++; return { positions: [], permissions: [] }; }); + await engine.execute('sys', { userId: 'u1' }); + expect(called).toBe(0); + for (const c of calls) expect(c.ctx.isSystem).toBe(true); + }); + + it('does NOT resolve when there is no trigger user (stays the unscoped fail-open)', async () => { + const engine = new AutomationEngine(makeLogger()); + const { data, calls } = fakeData(); + registerCrudNodes(engine, ctxWith(data)); + engine.registerFlow('sched', allOpsFlow('sched')); // default user, no userId + let called = 0; + engine.setUserGrantsResolver(() => { called++; return { positions: [], permissions: [] }; }); + await engine.execute('sched', { event: 'schedule' }); + expect(called).toBe(0); + for (const c of calls) expect(c.ctx, `${c.op} should stay unscoped`).toBeUndefined(); + }); + + it('fail-safe: a resolver error warns and keeps the bare user — never elevates', async () => { + const { logger, warns } = recordingLogger(); + const engine = new AutomationEngine(logger); + const { data, calls } = fakeData(); + registerCrudNodes(engine, ctxWith(data)); + engine.registerFlow('usr', allOpsFlow('usr', 'user')); + engine.setUserGrantsResolver(() => { throw new Error('db down'); }); + + const res = await engine.execute('usr', { userId: 'u1' }); + expect(res.success).toBe(true); + // The degraded resolution is AUDIBLE, and the run is NOT elevated. + expect(runAsWarns(warns).some((w) => w.includes('could not resolve grants'))).toBe(true); + for (const c of calls) { + expect(c.ctx.isSystem, `${c.op} wrongly elevated on resolver failure`).toBe(false); + expect(c.ctx.userId).toBe('u1'); + expect(c.ctx.permissions).toEqual([]); // unresolved → data middleware applies its baseline + } + }); +}); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 6341c8a1c2..a26b033894 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -250,6 +250,36 @@ export type FlowObjectSchemaResolver = ( objectName: string, ) => { fields?: readonly string[]; fieldTypes?: Record } | undefined; +/** + * The authorization envelope a `runAs:'user'` run needs to enforce its data + * ops as the triggering user (#3356): the user's resolved position names and + * permission-set names (the two lists the data security middleware keys on), + * plus their tenant. Built by {@link FlowUserGrantsResolver}. + */ +export interface FlowUserGrants { + positions: string[]; + permissions: string[]; + tenantId?: string; +} + +/** + * Resolves the authorization grants held by the triggering user of a + * `runAs:'user'` run, so its data nodes enforce RLS exactly as that user rather + * than the bare member/everyone fallback (#3356, follow-up to #1888). Injected + * by the host — the automation plugin bridges it to `@objectstack/core`'s + * `resolveUserAuthzGrants`, which reads `sys_member` / `sys_user_position` / + * `sys_*_permission_set` — so the engine stays decoupled from the identity + * store. Returns `undefined` (or throws, tolerated) when grants can't be + * resolved; the run then keeps whatever identity the trigger already carried + * (see {@link AutomationEngine.resolveRunContext}). When unwired, run identity + * is unchanged from the pre-#3356 behavior (the trigger-supplied context is + * used verbatim), so a bare engine in tests is unaffected. + */ +export type FlowUserGrantsResolver = ( + userId: string, + tenantId: string | undefined, +) => Promise | FlowUserGrants | undefined; + /** * A designer-facing view of one connector action — identity + its JSON-Schema * input/output. The runtime handler is intentionally omitted; this is metadata. @@ -617,6 +647,9 @@ export class AutomationEngine implements IAutomationService { /** Bridge to the host object registry for schema-aware condition validation at * registration (#1928), if wired. Advisory-only — see {@link FlowObjectSchemaResolver}. */ private objectSchemaResolver: FlowObjectSchemaResolver | null = null; + /** Bridge to the host authz resolver so a `runAs:'user'` run enforces the + * triggering user's real grants (#3356), if wired. See {@link FlowUserGrantsResolver}. */ + private userGrantsResolver: FlowUserGrantsResolver | null = null; private executionLogs: ExecutionLogEntry[] = []; private readonly maxLogSize: number; private logger: Logger; @@ -1072,6 +1105,20 @@ export class AutomationEngine implements IAutomationService { this.objectSchemaResolver = resolver; } + /** + * Wire the engine to the host's authorization resolver (#3356) so a + * `runAs:'user'` run resolves the TRIGGERING user's real positions + + * permission sets at run setup — the record-change hook session carries + * only a `userId`, so without this the run's data ops fell back to a bare + * member/everyone principal even when the triggering user was fully + * authorized. The automation plugin bridges it to `@objectstack/core`'s + * `resolveUserAuthzGrants`. Passing `null` detaches the bridge (run identity + * reverts to whatever the trigger supplied). + */ + setUserGrantsResolver(resolver: FlowUserGrantsResolver | null): void { + this.userGrantsResolver = resolver; + } + /** * Resolve a named function for a `script` node. Returns `undefined` when no * resolver is wired or the name is unregistered — the node then fails the @@ -1455,8 +1502,49 @@ export class AutomationEngine implements IAutomationService { * `runAs:'system'` to make scheduled elevation explicit (the build-time lint * `flow-schedule-runas-unscoped` flags the same shape earlier). */ - private resolveRunContext(flow: FlowParsed, context?: AutomationContext): AutomationContext { + private async resolveRunContext(flow: FlowParsed, context?: AutomationContext): Promise { const runContext: AutomationContext = { ...(context ?? {}), runAs: flow.runAs ?? 'user' }; + + // #3356 (follow-up to #1888) — a `runAs:'user'` run must enforce its data + // ops as the TRIGGERING user's real authorization. Most trigger surfaces + // (REST action / trigger endpoint) already resolve the full envelope and + // forward `permissions`; the ObjectQL record-change hook does NOT — its + // session carries only a `userId`, so the run used to fall back to a bare + // member/everyone principal (403 on private objects; silent field strips + // on public ones) even when the triggering user was fully authorized. + // When a grants resolver is wired and the trigger left the authz envelope + // unresolved (no `permissions`), resolve the user's real positions + + // permission sets here — the single point where every trigger type's run + // identity is established. Contexts that ALREADY carry `permissions` are + // left untouched: that includes an ADR-0090 agent principal acting + // on-behalf-of a user (its scope-derived ceiling is always non-empty), so + // this never re-broadens a deliberately narrowed identity. + if ( + runContext.runAs !== 'system' && + runContext.userId && + !Array.isArray(runContext.permissions) && + this.userGrantsResolver + ) { + try { + const grants = await this.userGrantsResolver(runContext.userId, runContext.tenantId); + if (grants) { + runContext.positions = Array.isArray(grants.positions) ? grants.positions : []; + runContext.permissions = Array.isArray(grants.permissions) ? grants.permissions : []; + if (grants.tenantId && !runContext.tenantId) runContext.tenantId = grants.tenantId; + } + } catch (err) { + // Fail-safe, never fail-open: on a resolution error the run keeps + // the trigger's (unresolved) identity — the data middleware applies + // its baseline member fallback, NOT elevation — and we warn loudly + // so the degraded authorization is audible rather than silent. + this.logger.warn( + `[runAs] flow '${flow.name}' could not resolve grants for triggering user ` + + `'${runContext.userId}': ${(err as Error)?.message ?? String(err)}. Its data ops fall ` + + `back to baseline member permissions (not elevated).`, + ); + } + } + if (runIsUnscopedUserMode(runContext) && flowTouchesData(flow)) { this.logger.warn( `[runAs] flow '${flow.name}' executes with runAs:'user' but its trigger carries no user ` + @@ -1537,8 +1625,9 @@ export class AutomationEngine implements IAutomationService { // ADR-0049 / #1888 — establish the run's effective execution identity // from flow.runAs (a COPY, never mutating the caller's context, so the // elevation is scoped to this run and the caller's identity is restored - // when execute() returns). Surfaces the user-less fail-open (see helper). - const runContext = this.resolveRunContext(flow, context); + // when execute() returns). Surfaces the user-less fail-open (see helper) + // and resolves the triggering user's real grants for `runAs:'user'` (#3356). + const runContext = await this.resolveRunContext(flow, context); try { // Find the start node @@ -2871,7 +2960,7 @@ export class AutomationEngine implements IAutomationService { // ADR-0049 / #1888 — establish the run's effective execution identity // from flow.runAs (see execute() / resolveRunContext); threaded below. - const runContext = this.resolveRunContext(flow, context); + const runContext = await this.resolveRunContext(flow, context); try { const startNode = flow.nodes.find(n => n.type === 'start'); diff --git a/packages/services/service-automation/src/plugin.ts b/packages/services/service-automation/src/plugin.ts index 1215b3d62b..1c602724a7 100644 --- a/packages/services/service-automation/src/plugin.ts +++ b/packages/services/service-automation/src/plugin.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import type { Plugin, PluginContext } from '@objectstack/core'; +import { resolveUserAuthzGrants } from '@objectstack/core'; import type { IJobService } from '@objectstack/spec/contracts'; import type { Connector, @@ -491,6 +492,41 @@ export class AutomationServicePlugin implements Plugin { ctx.logger.debug('[Automation] objectql registry not present — flow-condition checks limited to syntax'); } + // #3356 (follow-up to #1888) — bridge the shared authz resolver so a + // `runAs:'user'` run enforces the TRIGGERING user's REAL grants. The + // record-change hook session carries only a `userId` (never the writer's + // positions/permission sets), so without this the run's data ops fell + // back to a bare member/everyone principal — a 403 on private objects and + // silent field strips on public ones — even when the triggering user was + // fully authorized. `resolveUserAuthzGrants` reads the same + // sys_member / sys_user_position / sys_*_permission_set tables a REST + // request resolves through, so a flow "as the user" matches that user's + // own direct request. Wired off the same objectql/data engine the CRUD + // nodes use. Best-effort: without an ObjectQL engine there are no grants + // to resolve and run identity is unchanged (the trigger context is used + // verbatim, the pre-#3356 behavior). + try { + let ql: { find?: unknown } | undefined; + try { ql = ctx.getService<{ find?: unknown }>('objectql'); } + catch { try { ql = ctx.getService<{ find?: unknown }>('data'); } catch { ql = undefined; } } + if (ql && typeof ql.find === 'function') { + const engineQl = ql; + this.engine.setUserGrantsResolver(async (userId, tenantId) => { + const grants = await resolveUserAuthzGrants(engineQl, userId, { tenantId }); + return { + positions: grants.positions, + permissions: grants.permissions, + ...(tenantId ? { tenantId } : {}), + }; + }); + ctx.logger.debug('[Automation] runAs:user grant resolver bridged to @objectstack/core resolveUserAuthzGrants (#3356)'); + } else { + ctx.logger.debug('[Automation] objectql not present — runAs:user runs keep the trigger-supplied identity'); + } + } catch (err) { + ctx.logger.debug(`[Automation] runAs:user grant resolver not wired: ${(err as Error).message}`); + } + // Pull flow definitions from the ObjectQL schema registry. AppPlugin.init() // calls manifest.register(payload), which routes to ql.registerApp() and // stores each inline flow under type 'flow'. By the time start() runs, diff --git a/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts b/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts new file mode 100644 index 0000000000..6bc6556c96 --- /dev/null +++ b/packages/services/service-automation/src/runas-grant-resolution.integration.test.ts @@ -0,0 +1,126 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #3356 (follow-up to #1888) — end-to-end WIRING proof for the runAs:'user' +// credential propagation. It is not enough that the engine CAN resolve grants +// (a bare-engine unit tests that in crud-runas.test.ts) — the plugin's start() +// must actually BRIDGE the shared authz resolver to the objectql/data service, +// or a real deployment still runs a runAs:'user' flow with the bare +// member/everyone fallback (the hollow-credential bug this issue reports). +// +// This boots AutomationServicePlugin on a LiteKernel with a fake objectql that +// (a) serves the sys_member / sys_user_position / sys_*_permission_set tables +// `resolveUserAuthzGrants` reads, and (b) records the ObjectQL `context` each +// CRUD op receives. A runAs:'user' run triggered with ONLY a userId (the +// record-change hook shape) must reach its update_record with the TRIGGERING +// user's resolved positions + permission-set names — not an empty envelope. + +import { describe, it, expect } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +import { AutomationServicePlugin } from './plugin.js'; +import { AutomationEngine } from './engine.js'; + +/** + * A fake ObjectQL engine that both serves the authz tables (for grant + * resolution) and records the `context` every CRUD op is called with. Registered + * under BOTH `objectql` and `data` so the resolver (checks objectql first) and + * the CRUD nodes (check data first) resolve the same instance. + */ +function fakeObjectQl(tables: Record) { + const crud: Array<{ op: string; obj: string; ctx: any }> = []; + const match = (object: string, where: any): any[] => + (tables[object] ?? []).filter((r) => + Object.entries(where ?? {}).every(([k, v]) => + v && typeof v === 'object' && '$in' in (v as any) ? (v as any).$in.includes(r[k]) : r[k] === v, + ), + ); + const engine: any = { + async find(object: string, opts: any) { crud.push({ op: 'find', obj: object, ctx: opts?.context }); return match(object, opts?.where); }, + async findOne(object: string, opts: any) { crud.push({ op: 'findOne', obj: object, ctx: opts?.context }); return match(object, opts?.where)[0]; }, + async insert(object: string, _f: any, opts: any) { crud.push({ op: 'insert', obj: object, ctx: opts?.context }); return { id: `${object}_1` }; }, + async update(object: string, _f: any, opts: any) { crud.push({ op: 'update', obj: object, ctx: opts?.context }); return { ok: true }; }, + async delete(object: string, opts: any) { crud.push({ op: 'delete', obj: object, ctx: opts?.context }); return { ok: true }; }, + }; + return { engine, crud }; +} + +/** start → update_record('runas_thing') → end, parameterized by runAs. */ +function updateFlow(name: string, runAs: 'system' | 'user') { + return { + name, label: name, type: 'autolaunched', runAs, + variables: [{ name: 'noteId', type: 'text', isInput: true }], + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'up', type: 'update_record', label: 'Update', config: { objectName: 'runas_thing', filter: { id: '{noteId}' }, fields: { status: 'x' } } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'up' }, { id: 'e2', source: 'up', target: 'end' }], + }; +} + +/** Authz tables granting user 'u1' the unscoped `ehr_all` set + the `approver` position. */ +const AUTHZ_TABLES: Record = { + sys_user: [{ id: 'u1', email: 'u1@x.com' }], + sys_member: [], + sys_user_position: [{ user_id: 'u1', position: 'approver', organization_id: null }], + sys_user_permission_set: [{ user_id: 'u1', permission_set_id: 'psE', organization_id: null }], + sys_permission_set: [{ id: 'psE', name: 'ehr_all', system_permissions: ['cap_ehr'] }], + sys_position: [], + sys_position_permission_set: [], +}; + +async function bootWithObjectQl(ql: any): Promise { + const kernel = new LiteKernel({ logger: { level: 'silent' } } as never); + kernel.use(new AutomationServicePlugin({ suspendedRunStore: 'memory' })); + const harness = { + name: 'test.harness', type: 'standard' as const, version: '1.0.0', dependencies: [] as string[], + async init(ctx: any) { + ctx.registerService('objectql', ql); + ctx.registerService('data', ql); + }, + async start() {}, + }; + kernel.use(harness as never); + await kernel.bootstrap(); + return kernel; +} + +describe("AutomationServicePlugin bridges the runAs:'user' grant resolver (#3356)", () => { + it("resolves the triggering user's positions + permission sets into the data op context", async () => { + const { engine: ql, crud } = fakeObjectQl(AUTHZ_TABLES); + const kernel = await bootWithObjectQl(ql); + const automation = kernel.getService('automation'); + automation.registerFlow('usr', updateFlow('usr', 'user') as never); + + // The record-change hook shape: ONLY a userId — no positions/permissions. + const res = await automation.execute('usr', { userId: 'u1', params: { noteId: 'n1' } }); + expect(res.success, `run failed: ${JSON.stringify(res)}`).toBe(true); + + const update = crud.find((c) => c.op === 'update' && c.obj === 'runas_thing'); + expect(update, 'update_record never reached the data engine').toBeTruthy(); + expect(update!.ctx, 'runAs:user data op ran with NO identity (security skipped)').toBeTruthy(); + expect(update!.ctx.isSystem).toBe(false); + expect(update!.ctx.userId).toBe('u1'); + expect(update!.ctx.positions).toContain('approver'); // sys_user_position + expect(update!.ctx.positions).toContain('everyone'); // implicit anchor + expect( + update!.ctx.permissions, + "the triggering user's permission set was not propagated — runAs:'user' ran with the bare member fallback (#3356)", + ).toContain('ehr_all'); + + await kernel.shutdown(); + }); + + it("runAs:'system' still elevates — the resolver is not consulted", async () => { + const { engine: ql, crud } = fakeObjectQl(AUTHZ_TABLES); + const kernel = await bootWithObjectQl(ql); + const automation = kernel.getService('automation'); + automation.registerFlow('sys', updateFlow('sys', 'system') as never); + + await automation.execute('sys', { userId: 'u1', params: { noteId: 'n1' } }); + const update = crud.find((c) => c.op === 'update' && c.obj === 'runas_thing'); + expect(update!.ctx.isSystem).toBe(true); + expect(update!.ctx.userId).toBeUndefined(); + + await kernel.shutdown(); + }); +}); diff --git a/packages/triggers/trigger-record-change/src/record-change-trigger.ts b/packages/triggers/trigger-record-change/src/record-change-trigger.ts index 03b7455264..c0a1e26da4 100644 --- a/packages/triggers/trigger-record-change/src/record-change-trigger.ts +++ b/packages/triggers/trigger-record-change/src/record-change-trigger.ts @@ -225,7 +225,7 @@ export class RecordChangeTrigger implements FlowTrigger { { ...(inputDoc ?? {}), ...after } : inputDoc ?? (previous && typeof previous === 'object' ? previous : {}); - const session = (ctx.session ?? {}) as { userId?: string; organizationId?: string; positions?: string[] }; + const session = (ctx.session ?? {}) as { userId?: string; organizationId?: string }; return { record, @@ -233,13 +233,19 @@ export class RecordChangeTrigger implements FlowTrigger { object: binding.object ?? ctx.object, event: binding.event, userId: session.userId, - // Forward the writer's roles/org so a `runAs:'user'` flow enforces - // RLS exactly as the user who made the change, not a member fallback - // (#1888). The engine elevates only for `runAs:'system'`. The hook - // session exposes the active org as `organizationId` (the deprecated - // `session.tenantId` alias was removed in v11, #3290); it feeds the - // automation context's driver-layer `tenantId` field unchanged. - ...(Array.isArray(session.positions) && session.positions.length ? { positions: session.positions } : {}), + // Forward the writer's identity so a `runAs:'user'` flow enforces RLS + // exactly as the user who made the change (#1888). We forward the + // `userId` (+ the active org as `tenantId`) ONLY: the ObjectQL hook + // session does NOT carry the writer's positions / permission sets, so + // the automation engine resolves the triggering user's FULL grants + // from this `userId` at run setup (#3356). Forwarding a half-populated + // `positions` here (empty in practice, and never `permissions`) was the + // hollow-credential bug #3356 fixed — an incomplete, misleading + // duplicate of what the engine now resolves authoritatively. The + // engine elevates only for `runAs:'system'`. The hook session exposes + // the active org as `organizationId` (the deprecated `session.tenantId` + // alias was removed in v11, #3290); it feeds the automation context's + // driver-layer `tenantId` field unchanged. ...(session.organizationId ? { tenantId: session.organizationId } : {}), // Expose the record as params too, so flows with named `isInput` // variables matching record fields get them seeded.