refactor(core): Drop the legacy bridge function-dispatch channel (#31200)

This commit is contained in:
Danny Martini 2026-05-29 17:47:16 +02:00 committed by GitHub
parent 24e95b164a
commit 8debcef0cf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 71 additions and 182 deletions

View File

@ -29,9 +29,12 @@ Key functions exposed on `globalThis` inside the isolate:
Host-side callbacks registered by `IsolatedVmBridge` as `ivm.Reference` objects
(synchronous cross-isolate calls):
- `__getValueAtPath(path[])` — returns a primitive, array metadata, or object metadata
- `__getValueAtPath(path[])` — returns a primitive, array metadata, or object metadata.
Function-typed values are returned as `undefined`; callable bindings route through `callHost`.
- `__getArrayElement(path[], index)` — returns a single array element (or its metadata)
- `__callFunctionAtPath(path[], ...args)` — invokes a host-side function and returns the result
- `callHost(envelope)` — typed-RPC dispatcher; the isolate sends a schema-validated
envelope (e.g. `{ type: 'getItems', nodeName, ... }`) and the host dispatches
to the matching handler
## Usage
@ -50,7 +53,7 @@ From the expression's perspective it just sees normal objects:
// Inside an expression (runs in isolate):
$json.user.email // triggers getValueAtPath(['$json','user','email'])
$json.items[150].id // triggers getArrayElement(['$json','items'], 150)
$items() // triggers callFunctionAtPath(['$items'])
$items() // triggers callHost({ type: 'getItems' })
```
### Array metadata

View File

@ -7,12 +7,12 @@
* `target.<name>` (set in `runtime/context.ts`) and returns the in-isolate
* copy before any host lookup happens.
*
* Why this matters under the threat model: every host function reachable
* from `data` becomes a callable target via the bridge's `callFunctionAtPath`.
* Confirming that helpers resolve in-isolate lets us safely strip them from
* the VM-path `data` object (see `expression.ts` shouldUseVm guard) without
* losing functionality. Keeping the helpers shadowed is a load-bearing
* structural invariant of the security model.
* Why this matters under the threat model: function-typed bindings on
* `data` are now structurally unreachable `getValueAtPath` returns
* `undefined` for any function-typed value and there is no `__isFunction`
* wrapper path. Confirming that helpers resolve in-isolate also lets us
* safely strip them from the VM-path `data` object (see `expression.ts`
* `shouldUseVm` guard) without losing functionality.
*
* If this test ever fails, one of the following changed:
* - Tournament's polyfill no longer rewrites bare identifiers to context-first

View File

@ -2,12 +2,12 @@
* Regression suite for the typed-RPC routing pattern.
*
* The pattern: `$('Foo').first()` is routed through the dedicated
* `getNodeFirst` typed RPC rather than the generic `callFunctionAtPath`
* channel. The in-isolate runtime exposes a synthetic proxy on
* `target.$(...)` that intercepts `.first` and sends a typed envelope via
* `callHost`; the bridge handler reads the literal string `"first"`
* from the host-side proxy. The property name is compile-time fixed and
* cannot be influenced by expression input.
* `getNodeFirst` typed RPC over the `callHost` dispatcher. The in-isolate
* runtime exposes a synthetic proxy on `target.$(...)` that intercepts
* `.first` and sends a typed envelope via `callHost`; the bridge handler
* reads the literal string `"first"` from the host-side proxy. The
* property name is compile-time fixed and cannot be influenced by
* expression input.
*
* Each typed RPC should land with a test in this file
* confirming:

View File

@ -280,7 +280,13 @@ export class IsolatedVmBridge implements RuntimeBridge {
* Create an ivm.Reference callback for getting value/metadata at a path.
*
* Used by createDeepLazyProxy when accessing properties. Returns metadata
* markers for functions, arrays, and objects, or the primitive value directly.
* markers for arrays and objects, or the primitive value directly.
*
* Function-typed values are returned as `undefined` every callable on
* the host data surface (`$('Foo').first()`, `$items()`, `$fromAI()`,
* `$evaluateExpression()`, `$getPairedItem()`) is wired in-isolate via
* the typed-RPC dispatcher (`callHost`). No expression form should
* reach a function through this path.
*
* @param data - Current workflow data to use for callback responses
* @private
@ -314,14 +320,13 @@ export class IsolatedVmBridge implements RuntimeBridge {
}
}
// Handle functions - return metadata marker
// Functions are not reachable via the lazy-proxy data path —
// every callable on the host data surface routes through the
// typed-RPC dispatcher. Return undefined so any residual
// access surfaces as missing rather than as a stale metadata
// marker the runtime no longer knows how to interpret.
if (typeof value === 'function') {
const fnString = value.toString();
// Block native functions for security
if (fnString.includes('[native code]')) {
return undefined;
}
return { __isFunction: true, __name: path[path.length - 1] };
return undefined;
}
// Handle arrays - always lazy, only transfer length
@ -414,48 +419,6 @@ export class IsolatedVmBridge implements RuntimeBridge {
});
}
/**
* Create an ivm.Reference callback for calling functions at a path.
*
* Used when expressions invoke functions from workflow data.
*
* @param data - Current workflow data to use for callback responses
* @private
*/
private createCallFunctionAtPathRef(data: WorkflowData): ivm.Reference {
return new (getIvm().Reference)((path: string[], ...args: unknown[]) => {
try {
// Navigate to function, tracking parent to preserve `this` context
let fn: unknown = data;
let parent: unknown = undefined;
let startIndex = 0;
const dollarFn = (data as Record<string, unknown>).$;
if (path.length >= 2 && path[0] === '$' && typeof dollarFn === 'function') {
fn = (dollarFn as (name: string) => unknown)(path[1]);
startIndex = 2;
}
for (let i = startIndex; i < path.length; i++) {
parent = fn;
fn = (fn as Record<string, unknown>)?.[path[i]];
}
if (typeof fn !== 'function') {
throw new Error(`${path.join('.')} is not a function`);
}
// Block native functions for security (same check as getValueAtPath)
if (fn.toString().includes('[native code]')) {
throw new Error(`${path.join('.')} is a native function and cannot be called`);
}
// Execute function with parent as `this` to preserve method context
return (fn as (...fnArgs: unknown[]) => unknown).call(parent, ...args);
} catch (err) {
return serializeError(err);
}
});
}
/**
* Create the single typed-RPC dispatcher.
*
@ -707,9 +670,9 @@ export class IsolatedVmBridge implements RuntimeBridge {
* Execute JavaScript code in the isolated context.
*
* Flow:
* 1. Create four ivm.Reference callbacks scoped to the current data:
* `getValueAtPath`, `getArrayElement`, `callFunctionAtPath`, `callHost`.
* 2. Use evalClosureSync to run the code in a closure where `$0`/`$1`/`$2`/`$3`
* 1. Create three ivm.Reference callbacks scoped to the current data:
* `getValueAtPath`, `getArrayElement`, `callHost`.
* 2. Use evalClosureSync to run the code in a closure where `$0`/`$1`/`$2`
* are the callback references no global mutable state.
* 3. buildContext() inside the isolate creates a fresh evaluation context
* from the closure-scoped references.
@ -729,7 +692,6 @@ export class IsolatedVmBridge implements RuntimeBridge {
const getValueAtPath = this.createGetValueAtPathRef(data);
const getArrayElement = this.createGetArrayElementRef(data);
const callFunctionAtPath = this.createCallFunctionAtPathRef(data);
const callHost = this.createCallHostRef(data);
try {
@ -750,8 +712,7 @@ export class IsolatedVmBridge implements RuntimeBridge {
var __ctx = buildContext({
getValueAtPath: $0,
getArrayElement: $1,
callFunctionAtPath: $2,
callHost: $3,
callHost: $2,
}, ${timezone});
try {
var __result = (function() {
@ -776,7 +737,7 @@ try {
const result = this.context.evalClosureSync(
wrappedCode,
[getValueAtPath, getArrayElement, callFunctionAtPath, callHost],
[getValueAtPath, getArrayElement, callHost],
{ result: { copy: true }, timeout: this.config.timeout },
);
@ -813,7 +774,6 @@ try {
} finally {
getValueAtPath.release();
getArrayElement.release();
callFunctionAtPath.release();
callHost.release();
}
}

View File

@ -24,7 +24,6 @@ describe('buildContext proxy', () => {
const ctx = buildContext({
getValueAtPath,
getArrayElement: makeRef(() => undefined),
callFunctionAtPath: makeRef(() => undefined),
callHost: makeRef(() => undefined),
});
@ -37,7 +36,6 @@ describe('buildContext proxy', () => {
const ctx = buildContext({
getValueAtPath,
getArrayElement: makeRef(() => undefined),
callFunctionAtPath: makeRef(() => undefined),
callHost: makeRef(() => undefined),
});
@ -54,7 +52,6 @@ describe('buildContext proxy', () => {
const ctx = buildContext({
getValueAtPath,
getArrayElement: makeRef(() => undefined),
callFunctionAtPath: makeRef(() => undefined),
callHost: makeRef(() => undefined),
});

View File

@ -15,21 +15,18 @@ function mockApplySync(returnValue: unknown = undefined) {
function createMockCallbacks(
overrides: {
getValueAtPath?: ReturnType<typeof vi.fn>;
callFunctionAtPath?: ReturnType<typeof vi.fn>;
getArrayElement?: ReturnType<typeof vi.fn>;
} = {},
) {
const getValueAtPath = overrides.getValueAtPath ?? mockApplySync();
const callFunctionAtPath = overrides.callFunctionAtPath ?? mockApplySync();
const getArrayElement = overrides.getArrayElement ?? mockApplySync();
const callbacks = {
getValueAtPath: { applySync: getValueAtPath },
callFunctionAtPath: { applySync: callFunctionAtPath },
getArrayElement: { applySync: getArrayElement },
};
return { getValueAtPath, callFunctionAtPath, getArrayElement, callbacks };
return { getValueAtPath, getArrayElement, callbacks };
}
// ---------------------------------------------------------------------------
@ -161,36 +158,7 @@ describe('createDeepLazyProxy', () => {
});
// -----------------------------------------------------------------------
// 4. Function metadata
// -----------------------------------------------------------------------
describe('function metadata', () => {
it('creates a callable wrapper for function metadata', () => {
mocks.getValueAtPath.mockReturnValue({ __isFunction: true, __name: 'myFn' });
const p = proxy();
expect(typeof p.myFn).toBe('function');
});
it('invokes __callFunctionAtPath with correct args when called', () => {
mocks.getValueAtPath.mockReturnValue({ __isFunction: true, __name: 'myFn' });
mocks.callFunctionAtPath.mockReturnValue('result');
const p = proxy();
p.myFn('a', 1);
expect(mocks.callFunctionAtPath).toHaveBeenCalledWith(null, [['myFn'], 'a', 1], ivmCallOpts);
});
it('caches the function wrapper', () => {
mocks.getValueAtPath.mockReturnValue({ __isFunction: true, __name: 'myFn' });
const p = proxy();
const first = p.myFn;
const second = p.myFn;
expect(first).toBe(second);
expect(mocks.getValueAtPath).toHaveBeenCalledTimes(1);
});
});
// -----------------------------------------------------------------------
// 5. Array metadata (always lazy-loaded via array proxy)
// 4. Array metadata (always lazy-loaded via array proxy)
// -----------------------------------------------------------------------
describe('array metadata', () => {
@ -428,16 +396,6 @@ describe('createDeepLazyProxy', () => {
// -----------------------------------------------------------------------
describe('edge cases', () => {
it('plain object with __isFunction=false is treated as primitive', () => {
mocks.getValueAtPath.mockReturnValue({ __isFunction: false, other: 1 });
const p = proxy();
const val = p.prop;
// Not a function — falls through to "primitive" caching
expect(typeof val).toBe('object');
expect(val.__isFunction).toBe(false);
expect(val.other).toBe(1);
});
it('plain object with __isArray=false is treated as primitive', () => {
mocks.getValueAtPath.mockReturnValue({ __isArray: false, data: 'x' });
const p = proxy();
@ -452,14 +410,6 @@ describe('createDeepLazyProxy', () => {
p.arr[-1];
expect(mocks.getArrayElement).not.toHaveBeenCalled();
});
it('function wrapper on a basePath proxy passes full path', () => {
mocks.getValueAtPath.mockReturnValue({ __isFunction: true, __name: '$items' });
mocks.callFunctionAtPath.mockReturnValue([]);
const p = proxy(['$root']);
p.fn();
expect(mocks.callFunctionAtPath).toHaveBeenCalledWith(null, [['$root', 'fn']], ivmCallOpts);
});
});
// -----------------------------------------------------------------------

View File

@ -72,8 +72,6 @@ interface BridgeCallback {
*
* - `getValueAtPath`, `getArrayElement`: data-access primitives used by the
* lazy-proxy system. Hot path; one ivm.Reference each for minimum overhead.
* - `callFunctionAtPath`: legacy generic dispatch; to be removed once
* every consumer has migrated to typed messages.
* - `callHost`: typed-RPC dispatcher. The in-isolate runtime constructs
* an envelope (e.g. `{ type: 'getNodeFirst', nodeName, ... }`) and the
* host-side dispatcher validates it with zod before routing to a handler.
@ -82,14 +80,13 @@ interface BridgeCallback {
* dispatcher switch. The name reflects what this is: a synchronous
* host RPC, not a postMessage-style async send.
*
* The bridge wires all four callbacks unconditionally before invoking
* The bridge wires all three callbacks unconditionally before invoking
* `buildContext`, so the runtime treats them as present no defensive
* null/undefined checks at each call site.
*/
export interface BridgeCallbacks {
getValueAtPath: BridgeCallback;
getArrayElement: BridgeCallback;
callFunctionAtPath: BridgeCallback;
callHost: BridgeCallback;
}
@ -405,19 +402,6 @@ export function buildContext(
throwIfErrorSentinel(value);
// Function metadata — create a callable wrapper
if (value && typeof value === 'object' && (value as any).__isFunction) {
target[key] = function (...args: unknown[]) {
const result = callbacks.callFunctionAtPath.applySync(null, [[key], ...args], {
arguments: { copy: true },
result: { copy: true },
});
throwIfErrorSentinel(result);
return result;
};
return true;
}
// Object / array metadata — create a shape-matched lazy proxy for deep access
if (isArrayMetadata(value)) {
target[key] = createDeepLazyProxy(

View File

@ -38,9 +38,11 @@ const unsafeJmespathPropertyPattern = new RegExp(
*
* Mirrors the host-side wrapper in `packages/workflow/src/workflow-data-proxy.ts`
* so that expressions resolve `$jmespath` from the in-isolate runtime via
* Tournament's polyfill, instead of falling through to the bridge's
* `callFunctionAtPath` channel. This shrinks the host-callable surface
* exposed by the data object.
* Tournament's polyfill. `$jmespath` is a pure utility it takes its
* data as an argument and runs a query on it, with no need for host
* context. Keeping it in-isolate means it does not appear as a
* host-callable on the bridge, which shrinks the host-callable surface
* the isolate can reach.
*
* Behavioural parity with the host wrapper:
* - Throws `ExpressionError` (same name) when args are wrong.

View File

@ -96,10 +96,13 @@ export type ProxyMeta = { kind: 'object'; keys?: string[] } | { kind: 'array'; l
*
* Pattern:
* 1. When property accessed: Call getValueAtPath([path]) to get metadata
* 2. Metadata indicates type: primitive, object, array, or function
* 2. Metadata indicates type: primitive, object, or array
* 3. For objects/arrays: Create nested proxy (shape-matched) for lazy loading
* 4. For functions: Create wrapper that calls callFunctionAtPath
* 5. Cache all fetched values in target to avoid repeated callbacks
* 4. Cache all fetched values in target to avoid repeated callbacks
*
* Callable host bindings (`$('Foo').first()`, `$items()`, etc.) do not flow
* through this proxy they are wired as in-isolate stubs on `target` that
* route through the typed-RPC dispatcher (`callHost`).
*
* @param basePath - Current path in object tree (e.g., ['$json', 'user'])
* @param meta - Optional shape descriptor (object/array + known keys/length)
@ -112,13 +115,12 @@ export function createDeepLazyProxy(
callbacks?: {
getValueAtPath: any;
getArrayElement: any;
callFunctionAtPath: any;
},
): Record<string | symbol, unknown> {
if (!callbacks) {
throw new Error('createDeepLazyProxy requires callbacks parameter');
}
const { getValueAtPath, getArrayElement, callFunctionAtPath } = callbacks;
const { getValueAtPath, getArrayElement } = callbacks;
const isArray = meta?.kind === 'array';
const arrayLength = isArray ? meta.length : 0;
@ -153,19 +155,8 @@ export function createDeepLazyProxy(
function materializeChild(basePath: string[], propOrIdx: string, value: unknown): unknown {
if (value === undefined || value === null) return value;
throwIfErrorSentinel(value);
// `path = [...basePath, propOrIdx]` is built only inside the three branches
// `path = [...basePath, propOrIdx]` is built only inside the two branches
// that actually use it, so non-metadata objects don't pay for the spread.
if (value && typeof value === 'object' && (value as { __isFunction?: unknown }).__isFunction) {
const path = [...basePath, propOrIdx];
return function (...args: any[]) {
const result = callFunctionAtPath.applySync(null, [path, ...args], {
arguments: { copy: true },
result: { copy: true },
});
throwIfErrorSentinel(result);
return result;
};
}
if (isArrayMetadata(value)) {
const path = [...basePath, propOrIdx];
return createDeepLazyProxy(path, { kind: 'array', length: value.__length }, callbacks);

View File

@ -551,14 +551,14 @@ export class Expression {
// Expression extensions — only attached for the legacy engine.
//
// In the VM engine, every host function reachable from `data` becomes
// a callable target the isolate can reach via `callFunctionAtPath`.
// To minimise that surface we keep the VM-path data object as small as
// possible and let the in-isolate runtime resolve helpers itself
// (see packages/@n8n/expression-runtime/src/runtime/context.ts, where
// Tournament's polyfill rewrites bare `extend(...)` calls to the
// in-isolate copy on `target.extend`). Setting them on `data` in VM
// mode would be dead code AND an unnecessary host-callable.
// In the VM engine, function-typed bindings on `data` are
// structurally unreachable: the bridge's `getValueAtPath` returns
// `undefined` for any function-typed value, and the in-isolate
// runtime resolves helpers itself via Tournament's polyfill
// (see packages/@n8n/expression-runtime/src/runtime/context.ts,
// where bare `extend(...)` calls bind to the in-isolate copy on
// `target.extend`). Setting them on `data` in VM mode would be
// dead code.
if (!usingVm) {
data.extend = extend;
data.extendOptional = extendOptional;

View File

@ -234,16 +234,18 @@ describe('Expression Parser', () => {
expect(evaluate('={{ $if("a"==="b", 1, 2) }}')).toEqual(2);
expect(evaluate('={{ $if("a"==="a", 1) }}')).toEqual(1);
expect(evaluate('={{ $if("a"==="b", 1) }}')).toEqual(false);
expect(evaluate('={{ $if("a"==="a", 1, 2) }}')).toEqual(1);
expect(evaluate('={{ $if("a"==="b", 1, 2) }}')).toEqual(2);
// This will likely break when sandboxing is implemented but it works for now.
// If you're implementing sandboxing maybe provide a way to add functions to
// sandbox we can check instead?
const mockCallback = vi.fn(() => false);
evaluate('={{ $if("a"==="a", true, $data.cb()) }}', [{ cb: mockCallback }]);
expect(mockCallback.mock.calls.length).toEqual(0);
evaluate('={{ $if("a"==="b", true, $data.cb()) }}', [{ cb: mockCallback }]);
expect(mockCallback.mock.calls.length).toEqual(1);
// Short-circuit: $if is AST-rewritten to a JS ternary, so the
// unselected branch is not evaluated. Probe with a throwing IIFE
// in the else position — the throw is swallowed by the
// evaluator's E() handler, but a non-undefined result for the
// true case proves the IIFE didn't run, and the undefined
// result for the false case proves it did.
const throwingExpr = "(function(){ throw new Error('side effect'); })()";
expect(evaluate(`={{ $if(true, 1, ${throwingExpr}) }}`)).toEqual(1);
expect(evaluate(`={{ $if(false, 1, ${throwingExpr}) }}`)).toBeUndefined();
});
test('$not', () => {