diff --git a/.changeset/flow-lookup-expand.md b/.changeset/flow-lookup-expand.md new file mode 100644 index 000000000..d1c42136c --- /dev/null +++ b/.changeset/flow-lookup-expand.md @@ -0,0 +1,27 @@ +--- +"@objectstack/service-automation": minor +"@objectstack/lint": patch +--- + +feat(automation): opt-in single-hop lookup expansion for record-change flow templates (#3475) + +A record-change flow can now declare `expand: ['', …]` on its start +node config so node templates resolve `{record..}` (e.g. +`{record.account.name}` in a notify title, closing the #3426 gap for lookups). + +The engine re-reads the declared relations AFTER identity resolution, as the +run's OWN principal — `resolveRunDataContext` honors `runAs`, so a `runAs:'user'` +run reads the referenced object as the **triggering user** (its RLS/FLS enforced) +rather than system-elevated. This is what made expansion unsafe to do in the +trigger's re-read (which has no resolved grants) and is why it lives in the +engine (new `AutomationEngine.setRecordExpander`, bridged by the plugin to the +same data engine the CRUD nodes use). + +Only the declared relation keys are grafted onto the run record, so bare lookup +ids and `multiple` lookup arrays (#1872) on other relations — and the formula +fields the trigger already hydrated — are untouched. Opt-in ⇒ zero cost when +unused; best-effort ⇒ a re-read failure leaves the record unexpanded and never +breaks the flow. + +The `os validate` lint rule `flow-template-lookup-traversal` (#3426/#3472) is now +suppressed for a relation once the flow declares it in `config.expand`. diff --git a/packages/lint/src/validate-flow-template-paths.test.ts b/packages/lint/src/validate-flow-template-paths.test.ts index d8af2953c..b133b9416 100644 --- a/packages/lint/src/validate-flow-template-paths.test.ts +++ b/packages/lint/src/validate-flow-template-paths.test.ts @@ -186,6 +186,49 @@ describe('validateFlowTemplatePaths', () => { expect(findings[0].rule).toBe(FLOW_TEMPLATE_UNKNOWN_FIELD); }); + it('does NOT flag a lookup traversal when the start config declares expand (#3475)', () => { + const findings = validateFlowTemplatePaths({ + objects: [LEAD_OBJECT], + flows: [ + { + name: 'expand_ok', + type: 'record_change', + nodes: [ + { + id: 'start', + type: 'start', + config: { objectName: 'crm_lead', triggerType: 'record-created', expand: ['crm_account'] }, + }, + { id: 'n1', type: 'notify', notify: { title: 'From {record.crm_account.name}', body: 'x' } }, + ], + }, + ], + }); + expect(findings).toHaveLength(0); + }); + + it('still flags a lookup traversal NOT covered by the declared expand (#3475)', () => { + const findings = validateFlowTemplatePaths({ + objects: [LEAD_OBJECT], + flows: [ + { + name: 'expand_partial', + type: 'record_change', + nodes: [ + { + id: 'start', + type: 'start', + config: { objectName: 'crm_lead', triggerType: 'record-created', expand: ['target_channels'] }, + }, + { id: 'n1', type: 'notify', notify: { title: 'From {record.crm_account.name}', body: 'x' } }, + ], + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(FLOW_TEMPLATE_LOOKUP_TRAVERSAL); + }); + it('returns empty when there are no flows', () => { expect(validateFlowTemplatePaths({ objects: [LEAD_OBJECT] })).toHaveLength(0); }); diff --git a/packages/lint/src/validate-flow-template-paths.ts b/packages/lint/src/validate-flow-template-paths.ts index 5417366db..32c89a741 100644 --- a/packages/lint/src/validate-flow-template-paths.ts +++ b/packages/lint/src/validate-flow-template-paths.ts @@ -189,6 +189,22 @@ function boundObjectOf(flow: AnyRec): string | undefined { return fromConfig ?? fromTyped; } +/** + * The lookup relations a record-change flow opted IN to expand, from the start + * node's `config.expand` (#3475). A `{record..}` hop through one of + * these IS resolved at run time — the engine re-reads it as the run's identity — + * so the traversal warning is suppressed for those relations. Accepts a `string` + * or `string[]`; anything else yields the empty set. + */ +function declaredExpandOf(flow: AnyRec): Set { + const nodes = Array.isArray(flow.nodes) ? (flow.nodes as AnyRec[]) : []; + const start = nodes.find((n) => n?.type === 'start'); + const raw = ((start?.config ?? {}) as AnyRec).expand; + if (typeof raw === 'string') return new Set(raw ? [raw] : []); + if (Array.isArray(raw)) return new Set(raw.filter((r): r is string => typeof r === 'string' && r.length > 0)); + return new Set(); +} + /** * Validate `{record.}` template references across every record-change * flow. Pure and dependency-free; safe on pre- or post-parse stacks. @@ -218,6 +234,7 @@ export function validateFlowTemplatePaths(stack: AnyRec): FlowTemplatePathFindin if (!obj) return; const fieldTypes = fieldTypesOf(obj); + const expandSet = declaredExpandOf(flow); nodes.forEach((node, nodeIndex) => { if (typeof node !== 'object' || !node) return; @@ -264,7 +281,7 @@ export function validateFlowTemplatePaths(stack: AnyRec): FlowTemplatePathFindin if (nextIsIdentifier) { const headType = fieldTypes.get(head) ?? ''; - if (RELATION_TYPES.has(headType)) { + if (RELATION_TYPES.has(headType) && !expandSet.has(head)) { const key = rest.join('.'); if (seenTraversal.has(key)) continue; seenTraversal.add(key); @@ -278,9 +295,9 @@ export function validateFlowTemplatePaths(stack: AnyRec): FlowTemplatePathFindin `'${head}' — the flow record carries '${head}' as a scalar id, not an expanded object, so ` + `this resolves to an empty string at runtime (silently).`, hint: - `Single-hop lookup traversal in templates is not resolved yet (tracked on #3426). ` + - `Reference the foreign-key id directly ('{record.${head}}'), or add a formula field on ` + - `'${objectName}' that projects the related value and reference that instead.`, + `Opt in to resolve it: add '${head}' to the start node's config.expand (#3475) and the ` + + `engine re-reads it as the run's identity. Otherwise reference the foreign-key id directly ` + + `('{record.${head}}'), or project the value via a formula field on '${objectName}'.`, }); } // STRUCTURED_TYPES + any other scalar `.sub` access is left alone: diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 00b50c8eb..1c50c8309 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -289,6 +289,24 @@ export type FlowUserGrantsResolver = ( tenantId: string | undefined, ) => Promise | FlowUserGrants | undefined; +/** + * Re-reads the specified single-hop lookup relations of a record-change flow's + * triggering record so `{record..}` templates can traverse them + * (#3475). Injected by the host — the automation plugin bridges it to a + * data-engine `findOne(..., { expand, context })` scoped by the run's identity + * ({@link resolveRunDataContext}), so the referenced object's RLS/FLS are + * enforced as the RUN (never system-elevated for a `runAs:'user'` run). Returns + * the re-read record (with the requested relations expanded to objects) or + * `undefined`; only the declared relation keys are grafted onto the run record. + * When unwired, lookup traversal stays unresolved (the pre-#3475 behavior). + */ +export type FlowRecordExpander = ( + objectName: string, + id: unknown, + expandFields: readonly string[], + runContext: AutomationContext, +) => Promise | undefined> | Record | undefined; + /** * A designer-facing view of one connector action — identity + its JSON-Schema * input/output. The runtime handler is intentionally omitted; this is metadata. @@ -667,6 +685,9 @@ export class AutomationEngine implements IAutomationService { /** 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; + /** Bridge to a host data read that expands declared lookup relations on the + * run record (#3475), if wired. See {@link FlowRecordExpander}. */ + private recordExpander: FlowRecordExpander | null = null; private executionLogs: ExecutionLogEntry[] = []; private readonly maxLogSize: number; private logger: Logger; @@ -1136,6 +1157,16 @@ export class AutomationEngine implements IAutomationService { this.userGrantsResolver = resolver; } + /** + * Wire the record lookup-expander (#3475). The automation plugin bridges it + * to a `findOne(..., { expand, context })` on the same data engine the CRUD + * nodes use, scoped by the run's identity. Passing `null` detaches it (lookup + * traversal in templates then stays unresolved). + */ + setRecordExpander(expander: FlowRecordExpander | null): void { + this.recordExpander = expander; + } + /** * 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 @@ -1570,9 +1601,69 @@ export class AutomationEngine implements IAutomationService { `(ADR-0049, #1888).`, ); } + + // #3475 — opt-in single-hop lookup expansion, AFTER identity resolution so + // the re-read runs as this run's own principal (see helper). + await this.expandDeclaredLookups(flow, runContext); return runContext; } + /** + * Enrich `runContext.record` with opt-in single-hop lookup expansions the + * start node declares as `config.expand: string[]` (#3475). Re-reads just + * those relations through the injected {@link setRecordExpander} — which the + * host wires to a data-engine read scoped by {@link resolveRunDataContext}, + * so the referenced object's RLS/FLS are enforced as the RUN's identity (the + * triggering user for `runAs:'user'`, elevated for `runAs:'system'`). Grafts + * ONLY the declared relation keys, and only when the re-read actually returned + * an object/array, so bare lookup ids and #1872 multi-lookup arrays on other + * relations — and the formula fields the trigger already hydrated — stay + * untouched. Mutates `record` in place (the same object the run's variable map + * already references). Best-effort: any failure leaves `record` unexpanded + * (the template then renders the scalar id) — expansion must never break the + * flow it feeds. + */ + private async expandDeclaredLookups(flow: FlowParsed, runContext: AutomationContext): Promise { + if (!this.recordExpander) return; + const record = runContext.record as Record | undefined; + const id = record?.id; + const object = runContext.object; + if (!record || id == null || id === '' || !object) return; + + const startNode = flow.nodes.find((n) => n.type === 'start'); + const raw = (startNode?.config as { expand?: unknown } | undefined)?.expand; + const expandFields = + typeof raw === 'string' + ? raw + ? [raw] + : [] + : Array.isArray(raw) + ? raw.filter((f): f is string => typeof f === 'string' && f.length > 0) + : []; + if (expandFields.length === 0) return; + + try { + const full = await this.recordExpander(object, id, expandFields, runContext); + if (full && typeof full === 'object') { + for (const field of expandFields) { + const expanded = (full as Record)[field]; + // Graft only a genuinely expanded relation (object/array); a + // scalar means the field is not a resolvable lookup — leave the + // raw id in place rather than overwrite it. + if (expanded !== null && typeof expanded === 'object') { + record[field] = expanded; + } + } + } + } catch (err) { + this.logger.warn( + `[expand] flow '${flow.name}' could not expand lookups [${expandFields.join(', ')}] on ` + + `'${object}#${String(id)}': ${(err as Error)?.message ?? String(err)}. Templates referencing ` + + `these relations resolve to the scalar id.`, + ); + } + } + async execute(flowName: string, context?: AutomationContext): Promise { const startTime = Date.now(); const flow = this.flows.get(flowName); diff --git a/packages/services/service-automation/src/plugin.ts b/packages/services/service-automation/src/plugin.ts index 1c602724a..383a30fd1 100644 --- a/packages/services/service-automation/src/plugin.ts +++ b/packages/services/service-automation/src/plugin.ts @@ -12,6 +12,7 @@ import type { import { isConnectorUpstreamUnavailable } from '@objectstack/spec/integration'; import { AutomationEngine } from './engine.js'; import { installBuiltinNodes, rearmSuspendedWaitTimers } from './builtin/index.js'; +import { resolveRunDataContext } from './runtime-identity.js'; import { SysAutomationRun } from './sys-automation-run.object.js'; import { ObjectStoreSuspendedRunStore, @@ -520,6 +521,34 @@ export class AutomationServicePlugin implements Plugin { }; }); ctx.logger.debug('[Automation] runAs:user grant resolver bridged to @objectstack/core resolveUserAuthzGrants (#3356)'); + + // #3475 — bridge the lookup expander for record-change flow + // templates. Re-reads the relations a flow declares in its start + // node's `config.expand` via the run's own identity + // (`resolveRunDataContext` honors runAs), so `{record..}` + // resolves WITHOUT bypassing the triggering user's RLS/FLS on the + // referenced object. Same engine the CRUD nodes and grant resolver + // use; a no-op when it exposes no `findOne`. + const expandQl = engineQl as { + findOne?: ( + obj: string, + q: Record, + ) => Promise | null | undefined>; + }; + if (typeof expandQl.findOne === 'function') { + this.engine.setRecordExpander(async (objectName, id, expandFields, runContext) => { + const dataCtx = resolveRunDataContext(runContext); + const expand: Record = {}; + for (const f of expandFields) expand[f] = {}; + const query: Record = { where: { id }, expand }; + // Omit `context` for the identity-less case (matches the CRUD + // nodes), so the data engine applies its no-identity default. + if (dataCtx) query.context = dataCtx; + const full = await expandQl.findOne!(objectName, query); + return full && typeof full === 'object' ? (full as Record) : undefined; + }); + ctx.logger.debug('[Automation] record-change lookup expander bridged (#3475)'); + } } else { ctx.logger.debug('[Automation] objectql not present — runAs:user runs keep the trigger-supplied identity'); } diff --git a/packages/services/service-automation/src/record-lookup-expand.integration.test.ts b/packages/services/service-automation/src/record-lookup-expand.integration.test.ts new file mode 100644 index 000000000..968923c32 --- /dev/null +++ b/packages/services/service-automation/src/record-lookup-expand.integration.test.ts @@ -0,0 +1,153 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #3475 — end-to-end proof that a record-change flow can opt in to expanding a +// single-hop lookup (`{record.account.name}`) SAFELY. The engine re-reads the +// declared relation AFTER identity resolution, as the RUN's own principal +// (resolveRunDataContext honors runAs), and grafts it onto the run record so +// every node's `{record..}` template resolves — without bypassing +// the triggering user's RLS/FLS on the referenced object. +// +// Boots AutomationServicePlugin on a LiteKernel with a fake objectql that +// records the `context` + `expand` each findOne receives and returns a +// pre-expanded lead row (standing in for ObjectQL's own expand). A downstream +// update_record interpolates `{record.account.name}`, so the fake's captured +// update fields prove the record was enriched and the template resolved. + +import { describe, it, expect } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +import { AutomationServicePlugin } from './plugin.js'; +import { AutomationEngine } from './engine.js'; + +interface CrudCall { op: string; obj: string; ctx: any; expand?: any; fields?: any } + +/** + * A fake ObjectQL engine. `findOne('lead', …)` returns the lead row with + * `account` already expanded to an object (as real ObjectQL would when passed + * `expand`), and records the `context`/`expand` it was called with. `update` + * records the interpolated `fields`. Registered under both `objectql` and `data`. + * `throwOnFindOne` simulates a read failure for the fail-safe test. + */ +function fakeObjectQl(opts: { throwOnFindOne?: boolean } = {}) { + const crud: CrudCall[] = []; + const LEAD = { id: 'lead1', company: 'Acme', account: { id: 'acc1', name: 'Acme Corp' }, owner: 'u9' }; + const engine: any = { + async find(object: string, o: any) { crud.push({ op: 'find', obj: object, ctx: o?.context }); return []; }, + async findOne(object: string, o: any) { + crud.push({ op: 'findOne', obj: object, ctx: o?.context, expand: o?.expand }); + if (opts.throwOnFindOne) throw new Error('boom'); + return object === 'lead' ? { ...LEAD } : undefined; + }, + async insert(object: string, _f: any, o: any) { crud.push({ op: 'insert', obj: object, ctx: o?.context }); return { id: `${object}_1` }; }, + async update(object: string, fields: any, o: any) { crud.push({ op: 'update', obj: object, ctx: o?.context, fields }); return { ok: true }; }, + async delete(object: string, o: any) { crud.push({ op: 'delete', obj: object, ctx: o?.context }); return { ok: true }; }, + }; + return { engine, crud }; +} + +/** record-change flow: start(expand) → update_record('audit', { … }) → end. */ +function expandFlow(name: string, runAs: 'system' | 'user', expand: string[], fields: Record) { + return { + name, label: name, type: 'record_change', runAs, + variables: [{ name: 'noteId', type: 'text', isInput: true }], + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: { objectName: 'lead', triggerType: 'record-created', expand } }, + { id: 'up', type: 'update_record', label: 'Up', config: { objectName: 'audit', filter: { id: '{noteId}' }, fields } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'up' }, { id: 'e2', source: 'up', target: 'end' }], + }; +} + +async function boot(ql: any): Promise { + const kernel = new LiteKernel({ logger: { level: 'silent' } } as never); + kernel.use(new AutomationServicePlugin({ suspendedRunStore: 'memory' })); + kernel.use({ + 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() {}, + } as never); + await kernel.bootstrap(); + return kernel; +} + +/** The seeded trigger record: lookups are scalar FK ids, as the hook row carries them. */ +const SEED = { id: 'lead1', company: 'Acme', account: 'acc1', owner: 'u9' }; + +describe('record-change lookup expansion (#3475)', () => { + it("expands a declared lookup as the triggering user and resolves {record.account.name}", async () => { + const { engine: ql, crud } = fakeObjectQl(); + const kernel = await boot(ql); + const automation = kernel.getService('automation'); + automation.registerFlow('u', expandFlow('u', 'user', ['account'], { note: '{record.account.name}' }) as never); + + const res = await automation.execute('u', { + object: 'lead', record: { ...SEED }, userId: 'u1', params: { noteId: 'n1' }, + }); + expect(res.success, `run failed: ${JSON.stringify(res)}`).toBe(true); + + // The expander re-read the lead with an expand map, as the USER (not system). + const read = crud.find((c) => c.op === 'findOne' && c.obj === 'lead'); + expect(read, 'expander never re-read the record').toBeTruthy(); + expect(read!.expand).toHaveProperty('account'); + expect(read!.ctx?.isSystem).toBe(false); + expect(read!.ctx?.userId).toBe('u1'); + + // The record was enriched, so the downstream template resolved. + const upd = crud.find((c) => c.op === 'update' && c.obj === 'audit'); + expect(upd!.fields.note).toBe('Acme Corp'); + + await kernel.shutdown(); + }); + + it("runAs:'system' expands with an elevated context", async () => { + const { engine: ql, crud } = fakeObjectQl(); + const kernel = await boot(ql); + const automation = kernel.getService('automation'); + automation.registerFlow('s', expandFlow('s', 'system', ['account'], { note: '{record.account.name}' }) as never); + + await automation.execute('s', { object: 'lead', record: { ...SEED }, userId: 'u1', params: { noteId: 'n1' } }); + + const read = crud.find((c) => c.op === 'findOne' && c.obj === 'lead'); + expect(read!.ctx?.isSystem).toBe(true); + expect(read!.ctx?.userId).toBeUndefined(); + + await kernel.shutdown(); + }); + + it('grafts ONLY the declared relation — a non-declared lookup stays a scalar id', async () => { + const { engine: ql, crud } = fakeObjectQl(); + const kernel = await boot(ql); + const automation = kernel.getService('automation'); + // Declare only `account`; `owner` is left un-expanded. + automation.registerFlow('sel', expandFlow('sel', 'user', ['account'], { + acc: '{record.account.name}', own: '{record.owner}', + }) as never); + + await automation.execute('sel', { object: 'lead', record: { ...SEED }, userId: 'u1', params: { noteId: 'n1' } }); + + const upd = crud.find((c) => c.op === 'update' && c.obj === 'audit'); + expect(upd!.fields.acc).toBe('Acme Corp'); // declared → expanded object → resolved + expect(upd!.fields.own).toBe('u9'); // not declared → scalar id preserved (#1872) + + await kernel.shutdown(); + }); + + it('is fail-safe: a re-read error leaves the record unexpanded and the flow still runs', async () => { + const { engine: ql, crud } = fakeObjectQl({ throwOnFindOne: true }); + const kernel = await boot(ql); + const automation = kernel.getService('automation'); + automation.registerFlow('fs', expandFlow('fs', 'user', ['account'], { note: '{record.account.name}' }) as never); + + const res = await automation.execute('fs', { object: 'lead', record: { ...SEED }, userId: 'u1', params: { noteId: 'n1' } }); + expect(res.success, `run failed: ${JSON.stringify(res)}`).toBe(true); + + // account stayed the scalar id 'acc1', so the single-token `{record.account.name}` + // could not resolve (undefined — the interpolator's unresolved single-token value) + // — but the run completed rather than throwing. + const upd = crud.find((c) => c.op === 'update' && c.obj === 'audit'); + expect(upd).toBeTruthy(); + expect(upd!.fields.note).toBeUndefined(); + + await kernel.shutdown(); + }); +});