opensource/n8n
3
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-09-01 15:47:41 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(core): Fix missing isolate acquisition and VM globals in expression engine (#27895)

Browse Source
This commit is contained in:
Iván Ovejero
2026-04-02 11:22:14 +02:00
committed by GitHub
parent f025a786e7
commit e5208484aa
7 changed files with 214 additions and 142 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/@n8n/expression-runtime/src/bridge/isolated-vm-bridge.ts
+20 -2
View File
@@ -362,6 +362,12 @@ export class IsolatedVmBridge implements RuntimeBridge {
value = (itemFn as (i: number) => unknown)(itemIndex);
startIndex = 2;
}
} else {
const dollarFn = (data as Record<string, unknown>).$;
if (path.length >= 2 && path[0] === '$' && typeof dollarFn === 'function') {
value = (dollarFn as (name: string) => unknown)(path[1]);
startIndex = 2;
}
}
for (let i = startIndex; i < path.length; i++) {
value = (value as Record<string, unknown>)?.[path[i]];
@@ -419,6 +425,12 @@ export class IsolatedVmBridge implements RuntimeBridge {
arr = (itemFn as (i: number) => unknown)(itemIndex);
startIndex = 2;
}
} else {
const dollarFn = (data as Record<string, unknown>).$;
if (path.length >= 2 && path[0] === '$' && typeof dollarFn === 'function') {
arr = (dollarFn as (name: string) => unknown)(path[1]);
startIndex = 2;
}
}
for (let i = startIndex; i < path.length; i++) {
arr = (arr as Record<string, unknown>)?.[path[i]];
@@ -462,9 +474,15 @@ export class IsolatedVmBridge implements RuntimeBridge {
// Navigate to function, tracking parent to preserve `this` context
let fn: unknown = data;
let parent: unknown = undefined;
for (const key of path) {
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>)?.[key];
fn = (fn as Record<string, unknown>)?.[path[i]];
}
if (typeof fn !== 'function') {
packages/@n8n/expression-runtime/src/runtime/index.ts
+9
View File
@@ -51,6 +51,15 @@ declare global {
var $now: import('luxon').DateTime;
var $today: import('luxon').DateTime;
var $items: unknown;
var $execution: unknown;
var $vars: unknown;
var $secrets: unknown;
var $executionId: string | undefined;
var $resumeWebhookUrl: string | undefined;
var $webhookId: string | undefined;
var $nodeId: string | undefined;
var $nodeVersion: number | undefined;
var $: (nodeName: string) => unknown;
}
}
packages/@n8n/expression-runtime/src/runtime/reset.ts
+34 -19
View File
@@ -18,6 +18,17 @@ const SafeURIError = createSafeErrorSubclass(URIError);
// Reset Function for Data Proxies
// ============================================================================
function fetchPrimitive(key: string): unknown {
try {
return globalThis.__getValueAtPath.applySync(null, [[key]], {
arguments: { copy: true },
result: { copy: true },
});
} catch {
return undefined;
}
}
/**
* Reset workflow data proxies before each evaluation.
*
@@ -73,6 +84,9 @@ export function resetDataProxies(timezone?: string): void {
globalThis.__data.$data = createDeepLazyProxy(['$data']);
globalThis.__data.$env = createDeepLazyProxy(['$env']);
globalThis.__data.process = createDeepLazyProxy(['process']);
globalThis.__data.$execution = createDeepLazyProxy(['$execution']);
globalThis.__data.$vars = createDeepLazyProxy(['$vars']);
globalThis.__data.$secrets = createDeepLazyProxy(['$secrets']);
// -------------------------------------------------------------------------
// Create DateTime values inside the isolate (not lazy-loaded from host,
@@ -88,25 +102,13 @@ export function resetDataProxies(timezone?: string): void {
// Fetch primitives directly (no lazy loading needed for simple values)
// -------------------------------------------------------------------------
try {
globalThis.__data.$runIndex = globalThis.__getValueAtPath.applySync(null, [['$runIndex']], {
arguments: { copy: true },
result: { copy: true },
});
} catch (error) {
// Property doesn't exist - set to undefined
globalThis.__data.$runIndex = undefined;
}
try {
globalThis.__data.$itemIndex = globalThis.__getValueAtPath.applySync(null, [['$itemIndex']], {
arguments: { copy: true },
result: { copy: true },
});
} catch (error) {
// Property doesn't exist - set to undefined
globalThis.__data.$itemIndex = undefined;
}
globalThis.__data.$runIndex = fetchPrimitive('$runIndex');
globalThis.__data.$itemIndex = fetchPrimitive('$itemIndex');
globalThis.__data.$executionId = fetchPrimitive('$executionId');
globalThis.__data.$resumeWebhookUrl = fetchPrimitive('$resumeWebhookUrl');
globalThis.__data.$webhookId = fetchPrimitive('$webhookId');
globalThis.__data.$nodeId = fetchPrimitive('$nodeId');
globalThis.__data.$nodeVersion = fetchPrimitive('$nodeVersion');
// -------------------------------------------------------------------------
// Expose workflow data to globalThis for expression access
@@ -125,6 +127,14 @@ export function resetDataProxies(timezone?: string): void {
globalThis.$env = globalThis.__data.$env;
globalThis.$now = globalThis.__data.$now as DateTime;
globalThis.$today = globalThis.__data.$today as DateTime;
globalThis.$execution = globalThis.__data.$execution;
globalThis.$vars = globalThis.__data.$vars;
globalThis.$secrets = globalThis.__data.$secrets;
globalThis.$executionId = globalThis.__data.$executionId as string | undefined;
globalThis.$resumeWebhookUrl = globalThis.__data.$resumeWebhookUrl as string | undefined;
globalThis.$webhookId = globalThis.__data.$webhookId as string | undefined;
globalThis.$nodeId = globalThis.__data.$nodeId as string | undefined;
globalThis.$nodeVersion = globalThis.__data.$nodeVersion as number | undefined;
// Expose standalone functions (min, max, average, numberList, zip, $ifEmpty, etc.)
Object.assign(globalThis.__data, extendedFunctions);
@@ -189,6 +199,11 @@ export function resetDataProxies(timezone?: string): void {
$binary: createDeepLazyProxy(['$item', indexStr, '$binary']),
};
};
globalThis.$ = function (nodeName: string) {
return createDeepLazyProxy(['$', nodeName]);
};
globalThis.__data.$ = globalThis.$;
}
// Matches initializeGlobalContext() lines 262-318 in packages/workflow/src/expression.ts
packages/cli/src/active-workflow-manager.ts
+35 -23
View File
@@ -273,10 +273,20 @@ export class ActiveWorkflowManager {
workflowSettings: workflowData.settings,
});
const webhooks = WebhookHelpers.getWorkflowWebhooks(workflow, additionalData, undefined, true);
await workflow.expression.acquireIsolate();
try {
const webhooks = WebhookHelpers.getWorkflowWebhooks(
workflow,
additionalData,
undefined,
true,
);
for (const webhookData of webhooks) {
await this.webhookService.deleteWebhook(workflow, webhookData, mode, 'update');
for (const webhookData of webhooks) {
await this.webhookService.deleteWebhook(workflow, webhookData, mode, 'update');
}
} finally {
await workflow.expression.releaseIsolate();
}
await this.workflowStaticDataService.saveStaticData(workflow);
@@ -671,21 +681,29 @@ export class ActiveWorkflowManager {
workflowSettings: dbWorkflow.settings,
});
if (shouldAddWebhooks) {
added.webhooks = await this.addWebhooks(
workflow,
additionalData,
'trigger',
activationMode,
);
}
let triggerCount = 0;
await workflow.expression.acquireIsolate();
try {
if (shouldAddWebhooks) {
added.webhooks = await this.addWebhooks(
workflow,
additionalData,
'trigger',
activationMode,
);
}
if (shouldAddTriggersAndPollers) {
added.triggersAndPollers = await this.addTriggersAndPollers(dbWorkflow, workflow, {
activationMode,
executionMode: 'trigger',
additionalData,
});
if (shouldAddTriggersAndPollers) {
added.triggersAndPollers = await this.addTriggersAndPollers(dbWorkflow, workflow, {
activationMode,
executionMode: 'trigger',
additionalData,
});
}
triggerCount = this.countTriggers(workflow, additionalData);
} finally {
await workflow.expression.releaseIsolate();
}
// Workflow got now successfully activated so make sure nothing is left in the queue
@@ -693,7 +711,6 @@ export class ActiveWorkflowManager {
await this.activationErrorsService.deregister(workflowId);
const triggerCount = this.countTriggers(workflow, additionalData);
await this.workflowRepository.updateWorkflowTriggerCount(workflow.id, triggerCount);
} catch (e) {
const error = e instanceof Error ? e : new Error(`${e}`);
@@ -776,11 +793,6 @@ export class ActiveWorkflowManager {
/**
* Count all triggers in the workflow, excluding Manual Trigger and other n8n-internal triggers.
*
* TODO: This method calls getWorkflowWebhooks, which evaluates webhook description expressions
* (path, httpMethod, etc.) that may reference user-authored expressions via $parameter. It
* should acquire an isolate before calling getWorkflowWebhooks, but countTriggers is sync.
* addWebhooks and removeWorkflow are async and can be fixed straightforwardly.
*/
private countTriggers(workflow: Workflow, additionalData: IWorkflowExecuteAdditionalData) {
const triggerFilter = (nodeType: INodeType) =>
packages/cli/src/webhooks/__tests__/test-webhooks.test.ts
+9 -5
View File
@@ -9,6 +9,7 @@ import type {
IWorkflowExecuteAdditionalData,
Workflow,
IHttpRequestMethods,
WorkflowExpression,
} from 'n8n-workflow';
import { v4 as uuid } from 'uuid';
@@ -71,7 +72,7 @@ describe('TestWebhooks', () => {
};
test('if webhook is needed, should register then create webhook and return true', async () => {
const workflow = mock<Workflow>();
const workflow = mock<Workflow>({ expression: mock<WorkflowExpression>() });
jest.spyOn(testWebhooks, 'toWorkflow').mockReturnValueOnce(workflow);
jest.spyOn(WebhookHelpers, 'getWorkflowWebhooks').mockReturnValue([webhook]);
@@ -107,7 +108,7 @@ describe('TestWebhooks', () => {
});
test('returns false if a triggerToStartFrom with triggerData is given', async () => {
const workflow = mock<Workflow>();
const workflow = mock<Workflow>({ expression: mock<WorkflowExpression>() });
jest.spyOn(testWebhooks, 'toWorkflow').mockReturnValueOnce(workflow);
jest.spyOn(WebhookHelpers, 'getWorkflowWebhooks').mockReturnValue([webhook]);
@@ -124,7 +125,7 @@ describe('TestWebhooks', () => {
test('returns true, registers and then creates webhook if triggerToStartFrom is given with no triggerData', async () => {
// ARRANGE
const workflow = mock<Workflow>();
const workflow = mock<Workflow>({ expression: mock<WorkflowExpression>() });
const webhook2 = mock<IWebhookData>({
node: 'trigger',
httpMethod,
@@ -161,6 +162,7 @@ describe('TestWebhooks', () => {
name: 'chatTriggerNode',
},
},
expression: mock<WorkflowExpression>(),
});
const chatSessionId = 'test-session-123';
const chatWebhook = mock<IWebhookData>({
@@ -200,6 +202,7 @@ describe('TestWebhooks', () => {
name: 'chatTriggerNode',
},
},
expression: mock<WorkflowExpression>(),
});
const chatWebhook = mock<IWebhookData>({
node: 'chatTriggerNode',
@@ -230,6 +233,7 @@ describe('TestWebhooks', () => {
name: 'webhookNode',
},
},
expression: mock<WorkflowExpression>(),
});
const chatSessionId = 'test-session-123';
const regularWebhook = mock<IWebhookData>({
@@ -256,7 +260,7 @@ describe('TestWebhooks', () => {
test('should handle destinationNode parameter correctly', async () => {
// ARRANGE
const workflow = mock<Workflow>();
const workflow = mock<Workflow>({ expression: mock<WorkflowExpression>() });
const destinationNodeObj = { nodeName: 'DestinationNode', mode: 'inclusive' as const };
webhook.webhookDescription = {
restartWebhook: false,
@@ -291,7 +295,7 @@ describe('TestWebhooks', () => {
}>)(
'handles single webhook trigger when workflowIsActive=%s',
async ({ published: workflowIsActive, withSingleWebhookTrigger, shouldThrow }) => {
const workflow = mock<Workflow>();
const workflow = mock<Workflow>({ expression: mock<WorkflowExpression>() });
const regularWebhook = mock<IWebhookData>({
node: 'Webhook',
httpMethod,
packages/cli/src/webhooks/test-webhooks.ts
+101 -92
View File
@@ -309,119 +309,128 @@ export class TestWebhooks implements IWebhookManager {
const workflow = this.toWorkflow(workflowEntity);
let webhooks = WebhookHelpers.getWorkflowWebhooks(
workflow,
additionalData,
destinationNode,
true,
);
// If we have a preferred trigger with data, we don't have to listen for a
// webhook.
if (triggerToStartFrom?.data) {
return false;
}
// If we have a preferred trigger without data we only want to listen for
// that trigger, not the other ones.
if (triggerToStartFrom) {
webhooks = webhooks.filter((w) => w.node === triggerToStartFrom.name);
}
if (!webhooks.some((w) => w.webhookDescription.restartWebhook !== true)) {
return false; // no webhooks found to start a workflow
}
const timeoutDuration = TEST_WEBHOOK_TIMEOUT;
// Check if any webhook is a single webhook trigger and workflow is active
if (workflowIsActive) {
const singleWebhookTrigger = webhooks.find((w) =>
SINGLE_WEBHOOK_TRIGGERS.includes(workflow.getNode(w.node)?.type ?? ''),
await workflow.expression.acquireIsolate();
let webhooks: IWebhookData[];
try {
webhooks = WebhookHelpers.getWorkflowWebhooks(
workflow,
additionalData,
destinationNode,
true,
);
if (singleWebhookTrigger) {
throw new SingleWebhookTriggerError(
workflow.getNode(singleWebhookTrigger.node)?.name ?? '',
);
}
}
const timeout = setTimeout(async () => await this.cancelWebhook(workflow.id), timeoutDuration);
for (const webhook of webhooks) {
webhook.path = removeTrailingSlash(webhook.path);
// Use sessionId-based path for ChatTrigger nodes when sessionId is provided
// IMPORTANT: This must happen BEFORE key generation
if (
chatSessionId &&
webhook.node &&
workflow.nodes[webhook.node]?.type === '@n8n/n8n-nodes-langchain.chatTrigger'
) {
// Generate predictable path using workflowId and sessionId (without leading slash to match lookup format)
webhook.path = `${workflow.id}/${chatSessionId}`;
}
const key = this.registrations.toKey(webhook);
const registrationByKey = await this.registrations.get(key);
if (runData && webhook.node in runData) {
// If we have a preferred trigger with data, we don't have to listen for a
// webhook.
if (triggerToStartFrom?.data) {
return false;
}
// if registration already exists and is not a test webhook created by this user in this workflow throw an error
if (
registrationByKey &&
!webhook.webhookId &&
!registrationByKey.webhook.isTest &&
registrationByKey.webhook.userId !== userId &&
registrationByKey.webhook.workflowId !== workflow.id
) {
throw new WebhookPathTakenError(webhook.node);
// If we have a preferred trigger without data we only want to listen for
// that trigger, not the other ones.
if (triggerToStartFrom) {
webhooks = webhooks.filter((w) => w.node === triggerToStartFrom.name);
}
webhook.isTest = true;
if (!webhooks.some((w) => w.webhookDescription.restartWebhook !== true)) {
return false; // no webhooks found to start a workflow
}
/**
* Additional data cannot be cached because of circular refs.
* Hence store the `userId` and recreate additional data when needed.
*/
const { workflowExecuteAdditionalData: _, ...cacheableWebhook } = webhook;
const timeoutDuration = TEST_WEBHOOK_TIMEOUT;
cacheableWebhook.userId = userId;
// Check if any webhook is a single webhook trigger and workflow is active
if (workflowIsActive) {
const singleWebhookTrigger = webhooks.find((w) =>
SINGLE_WEBHOOK_TRIGGERS.includes(workflow.getNode(w.node)?.type ?? ''),
);
if (singleWebhookTrigger) {
throw new SingleWebhookTriggerError(
workflow.getNode(singleWebhookTrigger.node)?.name ?? '',
);
}
}
const registration: TestWebhookRegistration = {
version: 1,
pushRef,
workflowEntity,
destinationNode,
webhook: cacheableWebhook as IWebhookData,
};
const timeout = setTimeout(
async () => await this.cancelWebhook(workflow.id),
timeoutDuration,
);
for (const webhook of webhooks) {
webhook.path = removeTrailingSlash(webhook.path);
// Use sessionId-based path for ChatTrigger nodes when sessionId is provided
// IMPORTANT: This must happen BEFORE key generation
if (
chatSessionId &&
webhook.node &&
workflow.nodes[webhook.node]?.type === '@n8n/n8n-nodes-langchain.chatTrigger'
) {
// Generate predictable path using workflowId and sessionId (without leading slash to match lookup format)
webhook.path = `${workflow.id}/${chatSessionId}`;
}
const key = this.registrations.toKey(webhook);
const registrationByKey = await this.registrations.get(key);
if (runData && webhook.node in runData) {
return false;
}
// if registration already exists and is not a test webhook created by this user in this workflow throw an error
if (
registrationByKey &&
!webhook.webhookId &&
!registrationByKey.webhook.isTest &&
registrationByKey.webhook.userId !== userId &&
registrationByKey.webhook.workflowId !== workflow.id
) {
throw new WebhookPathTakenError(webhook.node);
}
webhook.isTest = true;
try {
/**
* Register the test webhook _before_ creation at third-party service
* in case service sends a confirmation request immediately on creation.
* Additional data cannot be cached because of circular refs.
* Hence store the `userId` and recreate additional data when needed.
*/
await this.registrations.register(registration);
const { workflowExecuteAdditionalData: _, ...cacheableWebhook } = webhook;
await this.webhookService.createWebhookIfNotExists(workflow, webhook, 'manual', 'manual');
cacheableWebhook.userId = userId;
cacheableWebhook.staticData = workflow.staticData;
const registration: TestWebhookRegistration = {
version: 1,
pushRef,
workflowEntity,
destinationNode,
webhook: cacheableWebhook as IWebhookData,
};
await this.registrations.register(registration);
try {
/**
* Register the test webhook _before_ creation at third-party service
* in case service sends a confirmation request immediately on creation.
*/
await this.registrations.register(registration);
this.timeouts[key] = timeout;
} catch (error) {
await this.deactivateWebhooks(workflow);
await this.webhookService.createWebhookIfNotExists(workflow, webhook, 'manual', 'manual');
delete this.timeouts[key];
cacheableWebhook.staticData = workflow.staticData;
throw error;
await this.registrations.register(registration);
this.timeouts[key] = timeout;
} catch (error) {
await this.deactivateWebhooks(workflow);
delete this.timeouts[key];
throw error;
}
}
}
return true;
return true;
} finally {
await workflow.expression.releaseIsolate();
}
}
async cancelWebhook(workflowId: string) {
packages/cli/src/workflows/workflow.service.ts
+6 -1
View File
@@ -592,7 +592,12 @@ export class WorkflowService {
workflowId: workflow.id,
});
return await this.webhookService.findWebhookConflicts(workflow, additionalData);
await workflow.expression.acquireIsolate();
try {
return await this.webhookService.findWebhookConflicts(workflow, additionalData);
} finally {
await workflow.expression.releaseIsolate();
}
}
private async _detectWebhookConflicts(