chore: Bundle/1.x (#38767)

This commit is contained in:
n8n-assistant[bot] 2026-09-16 08:22:02 +03:00 committed by GitHub
parent 35ffb8881e
commit fff2c84a51
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
83 changed files with 2668 additions and 377 deletions

View File

@ -0,0 +1,11 @@
import { z } from 'zod';
import { Z } from 'zod-class';
export class TestCredentialRequestDto extends Z.class({
credentials: z.object({
id: z.string(),
name: z.string().default(''),
type: z.string(),
data: z.record(z.string(), z.unknown()).optional(),
}),
}) {}

View File

@ -49,6 +49,7 @@ export { PullWorkFolderRequestDto } from './source-control/pull-work-folder-requ
export { PushWorkFolderRequestDto } from './source-control/push-work-folder-request.dto';
export { CreateCredentialDto } from './credentials/create-credential.dto';
export { TestCredentialRequestDto } from './credentials/test-credential-request.dto';
export { VariableListRequestDto } from './variables/variables-list-request.dto';
export { CreateVariableRequestDto } from './variables/create-variable-request.dto';
export { UpdateVariableRequestDto } from './variables/update-variable-request.dto';

View File

@ -3,6 +3,7 @@ import { describe, it, expect } from 'vitest';
import { extendedFunctions } from '../function-extensions';
const zip = extendedFunctions.zip;
const toPathSegment = extendedFunctions.toPathSegment;
describe('zip', () => {
it('should pair keys with values', () => {
@ -21,3 +22,13 @@ describe('zip', () => {
expect(({} as Record<string, unknown>).marker).toBeUndefined();
});
});
describe('toPathSegment', () => {
it('encodes a value as one path segment', () => {
expect(toPathSegment('a/b c')).toBe('a%2Fb%20c');
});
it.each(['', '.', '..', null, undefined])('rejects %s', (input) => {
expect(() => toPathSegment(input)).toThrow('Invalid identifier');
});
});

View File

@ -52,6 +52,19 @@ const not = (value: unknown): boolean => {
return !value;
};
function toPathSegment(input: unknown): string {
if (input === null || input === undefined) {
throw new ExpressionExtensionError('Invalid identifier: a value is required');
}
const value = String(input);
if (value === '' || value === '.' || value === '..') {
throw new ExpressionExtensionError(`Invalid identifier: "${value}" is not allowed`);
}
return encodeURIComponent(value);
}
function ifEmpty<T, V>(value: V, defaultValue: T) {
if (arguments.length !== 2) {
// DIVERGENCE from packages/workflow/src/extensions/extended-functions.ts:
@ -95,6 +108,7 @@ export const extendedFunctions = {
average,
numberList,
zip,
toPathSegment,
$min: min,
$max: max,
$average: average,

View File

@ -8,7 +8,7 @@ import {
type SupplyData,
} from 'n8n-workflow';
import { getSessionId } from '@utils/helpers';
import { coerceSessionIdToString, getSessionId } from '@utils/helpers';
import { logWrapper } from '@utils/logWrapper';
import { getConnectionHintNoticeField } from '@utils/sharedFields';
@ -155,7 +155,11 @@ export class MemoryBufferWindow implements INodeType {
if (nodeVersion >= 1.2) {
sessionId = getSessionId(this, itemIndex);
} else {
sessionId = this.getNodeParameter('sessionKey', itemIndex) as string;
sessionId = coerceSessionIdToString(
this,
this.getNodeParameter('sessionKey', itemIndex),
itemIndex,
);
}
const memory = await memoryInstance.getMemory(`${workflowId}__${sessionId}`, {

View File

@ -7,7 +7,7 @@ import {
type SupplyData,
} from 'n8n-workflow';
import { getSessionId } from '@utils/helpers';
import { coerceSessionIdToString, getSessionId } from '@utils/helpers';
import { logWrapper } from '@utils/logWrapper';
import { getConnectionHintNoticeField } from '@utils/sharedFields';
@ -98,7 +98,11 @@ export class MemoryMotorhead implements INodeType {
if (nodeVersion >= 1.2) {
sessionId = getSessionId(this, itemIndex);
} else {
sessionId = this.getNodeParameter('sessionId', itemIndex) as string;
sessionId = coerceSessionIdToString(
this,
this.getNodeParameter('sessionId', itemIndex),
itemIndex,
);
}
const memory = new MotorheadMemory({

View File

@ -12,7 +12,7 @@ import {
import type { RedisClientOptions } from 'redis';
import { createClient } from 'redis';
import { getSessionId } from '@utils/helpers';
import { coerceSessionIdToString, getSessionId } from '@utils/helpers';
import { logWrapper } from '@utils/logWrapper';
import { getConnectionHintNoticeField } from '@utils/sharedFields';
@ -121,7 +121,11 @@ export class MemoryRedisChat implements INodeType {
if (nodeVersion >= 1.2) {
sessionId = getSessionId(this, itemIndex);
} else {
sessionId = this.getNodeParameter('sessionKey', itemIndex) as string;
sessionId = coerceSessionIdToString(
this,
this.getNodeParameter('sessionKey', itemIndex),
itemIndex,
);
}
const redisOptions: RedisClientOptions = {

View File

@ -9,7 +9,7 @@ import type {
SupplyData,
} from 'n8n-workflow';
import { getSessionId } from '@utils/helpers';
import { coerceSessionIdToString, getSessionId } from '@utils/helpers';
import { logWrapper } from '@utils/logWrapper';
import { getConnectionHintNoticeField } from '@utils/sharedFields';
@ -110,7 +110,11 @@ export class MemoryXata implements INodeType {
if (nodeVersion >= 1.2) {
sessionId = getSessionId(this, itemIndex);
} else {
sessionId = this.getNodeParameter('sessionId', itemIndex) as string;
sessionId = coerceSessionIdToString(
this,
this.getNodeParameter('sessionId', itemIndex),
itemIndex,
);
}
const xataClient = new BaseClient({

View File

@ -12,7 +12,7 @@ import {
NodeOperationError,
} from 'n8n-workflow';
import { getSessionId } from '@utils/helpers';
import { coerceSessionIdToString, getSessionId } from '@utils/helpers';
import { logWrapper } from '@utils/logWrapper';
import { getConnectionHintNoticeField } from '@utils/sharedFields';
@ -132,7 +132,11 @@ export class MemoryZep implements INodeType {
if (nodeVersion >= 1.2) {
sessionId = getSessionId(this, itemIndex);
} else {
sessionId = this.getNodeParameter('sessionId', itemIndex) as string;
sessionId = coerceSessionIdToString(
this,
this.getNodeParameter('sessionId', itemIndex),
itemIndex,
);
}
let memory: BaseChatMemory;

View File

@ -101,6 +101,26 @@ export function getPromptInputByType(options: {
return input;
}
// The session key can arrive straight from an untrusted request body (e.g. a
// public chat trigger). It is used to address stored data in the memory
// backend, so it must be a primitive value: an object or array would be
// interpreted as query structure by backends like MongoDB. Coerce numbers and
// booleans to strings and reject anything non-primitive.
export function coerceSessionIdToString(
ctx: ISupplyDataFunctions | IWebhookFunctions,
sessionId: unknown,
itemIndex: number,
): string {
if (typeof sessionId === 'string') return sessionId;
if (typeof sessionId === 'number' || typeof sessionId === 'boolean') {
return String(sessionId);
}
throw new NodeOperationError(ctx.getNode(), 'Invalid session ID', {
description: 'The session ID must be a text value',
itemIndex,
});
}
export function getSessionId(
ctx: ISupplyDataFunctions | IWebhookFunctions,
itemIndex: number,
@ -114,43 +134,48 @@ export function getSessionId(
if (selectorType === autoSelect) {
// If memory node is used in webhook like node(like chat trigger node), it doesn't have access to evaluateExpression
// so we try to extract sessionId from the bodyData
let rawSessionId: unknown;
if ('getBodyData' in ctx) {
const bodyData = ctx.getBodyData() ?? {};
sessionId = bodyData.sessionId as string;
rawSessionId = bodyData.sessionId;
} else {
sessionId = ctx.evaluateExpression('{{ $json.sessionId }}', itemIndex) as string;
rawSessionId = ctx.evaluateExpression('{{ $json.sessionId }}', itemIndex);
// try to get sessionId from chat trigger
if (!sessionId || sessionId === undefined) {
if (rawSessionId === '' || rawSessionId === undefined || rawSessionId === null) {
try {
const chatTrigger = ctx.getChatTrigger();
if (chatTrigger) {
sessionId = ctx.evaluateExpression(
rawSessionId = ctx.evaluateExpression(
`{{ $('${chatTrigger.name}').first().json.sessionId }}`,
itemIndex,
) as string;
);
}
} catch (error) {}
}
}
if (sessionId === '' || sessionId === undefined) {
if (rawSessionId === '' || rawSessionId === undefined || rawSessionId === null) {
throw new NodeOperationError(ctx.getNode(), 'No session ID found', {
description:
"Expected to find the session ID in an input field called 'sessionId' (this is what the chat trigger node outputs). To use something else, change the 'Session ID' parameter",
itemIndex,
});
}
sessionId = coerceSessionIdToString(ctx, rawSessionId, itemIndex);
} else {
sessionId = ctx.getNodeParameter(customKey, itemIndex, '') as string;
if (sessionId === '' || sessionId === undefined) {
const rawSessionId = ctx.getNodeParameter(customKey, itemIndex, '');
if (rawSessionId === '' || rawSessionId === undefined || rawSessionId === null) {
throw new NodeOperationError(ctx.getNode(), 'Key parameter is empty', {
description:
"Provide a key to use as session ID in the 'Key' parameter or use the 'Connected Chat Trigger Node' option to use the session ID from your Chat Trigger",
itemIndex,
});
}
sessionId = coerceSessionIdToString(ctx, rawSessionId, itemIndex);
}
return sessionId;

View File

@ -486,6 +486,55 @@ describe('getSessionId', () => {
expect(sessionId).toBe('12345');
});
it('should reject a non-primitive sessionId from bodyData', () => {
mockCtx.getBodyData = jest.fn();
mockCtx.getNodeParameter.mockReturnValue('fromInput');
mockCtx.getBodyData.mockReturnValue({ sessionId: { $ne: null } });
expect(() => getSessionId(mockCtx, 0)).toThrow(NodeOperationError);
});
it('should reject an array sessionId from bodyData', () => {
mockCtx.getBodyData = jest.fn();
mockCtx.getNodeParameter.mockReturnValue('fromInput');
mockCtx.getBodyData.mockReturnValue({ sessionId: [{ $regex: '^a' }] });
expect(() => getSessionId(mockCtx, 0)).toThrow(NodeOperationError);
});
it('should coerce a numeric sessionId from bodyData to a string', () => {
mockCtx.getBodyData = jest.fn();
mockCtx.getNodeParameter.mockReturnValue('fromInput');
mockCtx.getBodyData.mockReturnValue({ sessionId: 12345 });
const sessionId = getSessionId(mockCtx, 0);
expect(sessionId).toBe('12345');
});
it('should coerce a boolean sessionId from bodyData to a string', () => {
mockCtx.getBodyData = jest.fn();
mockCtx.getNodeParameter.mockReturnValue('fromInput');
mockCtx.getBodyData.mockReturnValue({ sessionId: false });
const sessionId = getSessionId(mockCtx, 0);
expect(sessionId).toBe('false');
});
it('should throw "No session ID found" for a null sessionId from bodyData', () => {
mockCtx.getBodyData = jest.fn();
mockCtx.getNodeParameter.mockReturnValue('fromInput');
mockCtx.getBodyData.mockReturnValue({ sessionId: null });
expect(() => getSessionId(mockCtx, 0)).toThrow(NodeOperationError);
});
it('should reject a non-primitive sessionId resolved from an expression', () => {
mockCtx.getNodeParameter.mockReturnValue('fromInput');
mockCtx.evaluateExpression.mockReturnValue({ $ne: null });
expect(() => getSessionId(mockCtx, 0)).toThrow(NodeOperationError);
});
it('should retrieve sessionId from chat trigger', () => {
mockCtx.getNodeParameter.mockReturnValue('fromInput');
mockCtx.evaluateExpression.mockReturnValueOnce(undefined);

View File

@ -52,6 +52,10 @@ class SecurityValidator(ast.NodeVisitor):
self._add_violation(
node.lineno, ERROR_DANGEROUS_NAME.format(name=alias.asname)
)
elif alias.asname and alias.asname in BLOCKED_ATTRIBUTES:
self._add_violation(
node.lineno, ERROR_DANGEROUS_ATTRIBUTE.format(attr=alias.asname)
)
self.generic_visit(node)
def visit_ImportFrom(self, node: ast.ImportFrom) -> None:
@ -67,10 +71,18 @@ class SecurityValidator(ast.NodeVisitor):
self._add_violation(
node.lineno, ERROR_DANGEROUS_NAME.format(name=alias.asname)
)
elif alias.asname and alias.asname in BLOCKED_ATTRIBUTES:
self._add_violation(
node.lineno, ERROR_DANGEROUS_ATTRIBUTE.format(attr=alias.asname)
)
elif alias.name in BLOCKED_NAMES:
self._add_violation(
node.lineno, ERROR_DANGEROUS_NAME.format(name=alias.name)
)
elif alias.name in BLOCKED_ATTRIBUTES:
self._add_violation(
node.lineno, ERROR_DANGEROUS_ATTRIBUTE.format(attr=alias.name)
)
self.generic_visit(node)

View File

@ -342,6 +342,33 @@ class TestImportAliasValidation(TestTaskAnalyzer):
analyzer.validate(code)
assert name in exc_info.value.description
def test_import_alias_using_blocked_attribute_rejected(
self, analyzer: TaskAnalyzer
) -> None:
for name in BLOCKED_ATTRIBUTES:
code = f"import json as {name}"
with pytest.raises(SecurityViolationError) as exc_info:
analyzer.validate(code)
assert name in exc_info.value.description
def test_from_import_alias_using_blocked_attribute_rejected(
self, analyzer: TaskAnalyzer
) -> None:
for name in BLOCKED_ATTRIBUTES:
code = f"from json import dumps as {name}"
with pytest.raises(SecurityViolationError) as exc_info:
analyzer.validate(code)
assert name in exc_info.value.description
def test_from_import_name_using_blocked_attribute_rejected(
self, analyzer: TaskAnalyzer
) -> None:
for name in BLOCKED_ATTRIBUTES:
code = f"from math import {name}"
with pytest.raises(SecurityViolationError) as exc_info:
analyzer.validate(code)
assert name in exc_info.value.description
def test_safe_import_aliases_allowed(self, analyzer: TaskAnalyzer) -> None:
safe = [
"import json as j",

View File

@ -1,3 +1,4 @@
import type { TestCredentialRequestDto } from '@n8n/api-types';
import type {
AuthenticatedRequest,
CredentialsEntity,
@ -131,6 +132,90 @@ describe('CredentialsController', () => {
});
});
describe('testCredentials', () => {
const storedCredential = mock<CredentialsEntity>({
id: 'credential-id',
name: 'Stored Credential',
type: 'githubApi',
});
const decryptedData = { accessToken: 'stored-token' };
const testResult = { status: 'OK', message: 'Credential tested successfully' } as any;
beforeEach(() => {
credentialsFinderService.findCredentialForUser.mockResolvedValue(storedCredential);
jest.mocked(credentialsService.decrypt).mockReturnValue(decryptedData);
jest
.spyOn(credentialsService, 'replaceCredentialContentsForSharee')
.mockResolvedValue(undefined);
jest.spyOn(credentialsService, 'unredact').mockReturnValue(decryptedData);
jest.spyOn(credentialsService, 'test').mockResolvedValue(testResult);
});
it('discards a caller-supplied homeProject and resolves the owning project from storage', async () => {
const owningProject = createRawProjectData({ id: 'real-owning-project-id', type: 'team' });
sharedCredentialsRepository.findCredentialOwningProject.mockResolvedValue(owningProject);
const payload = {
credentials: {
id: 'credential-id',
name: 'Stored Credential',
type: 'githubApi',
// A caller naming a project it doesn't own — this must never
// reach the credential test.
homeProject: {
id: 'attacker-chosen-project-id',
name: 'Attacker Project',
icon: null,
type: 'team',
createdAt: '2024-01-01T00:00:00.000Z',
updatedAt: '2024-01-01T00:00:00.000Z',
},
data: { accessToken: 'live-token' },
},
} as unknown as TestCredentialRequestDto;
await credentialsController.testCredentials(req, res, payload);
expect(sharedCredentialsRepository.findCredentialOwningProject).toHaveBeenCalledWith(
'credential-id',
);
expect(credentialsService.test).toHaveBeenCalledWith(
req.user.id,
expect.objectContaining({
homeProject: expect.objectContaining({ id: 'real-owning-project-id' }),
}),
);
});
it('does not fall back to a caller-supplied homeProject when no owning project is found', async () => {
sharedCredentialsRepository.findCredentialOwningProject.mockResolvedValue(undefined);
const payload = {
credentials: {
id: 'credential-id',
name: 'Stored Credential',
type: 'githubApi',
homeProject: {
id: 'attacker-chosen-project-id',
name: 'Attacker Project',
icon: null,
type: 'team',
createdAt: '2024-01-01T00:00:00.000Z',
updatedAt: '2024-01-01T00:00:00.000Z',
},
data: { accessToken: 'live-token' },
},
} as unknown as TestCredentialRequestDto;
await credentialsController.testCredentials(req, res, payload);
expect(credentialsService.test).toHaveBeenCalledWith(
req.user.id,
expect.objectContaining({ homeProject: undefined }),
);
});
});
describe('updateCredentials', () => {
const credentialId = 'cred-123';
const existingCredential = mock<CredentialsEntity>({

View File

@ -3,6 +3,7 @@ import {
CredentialsGetManyRequestQuery,
CredentialsGetOneRequestQuery,
GenerateCredentialNameRequestQuery,
TestCredentialRequestDto,
} from '@n8n/api-types';
import { Logger } from '@n8n/backend-common';
import { GlobalConfig } from '@n8n/config';
@ -29,7 +30,7 @@ import { hasGlobalScope, PROJECT_OWNER_ROLE_SLUG } from '@n8n/permissions';
// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
import { In } from '@n8n/typeorm';
import { deepCopy } from 'n8n-workflow';
import type { ICredentialDataDecryptedObject } from 'n8n-workflow';
import type { ICredentialDataDecryptedObject, ICredentialsDecrypted } from 'n8n-workflow';
import { z } from 'zod';
import { CredentialsFinderService } from './credentials-finder.service';
@ -135,8 +136,12 @@ export class CredentialsController {
// TODO: Write at least test cases for the failure paths.
@Post('/test')
async testCredentials(req: CredentialRequest.Test) {
const { credentials } = req.body;
async testCredentials(
req: AuthenticatedRequest,
_res: unknown,
@Body payload: TestCredentialRequestDto,
) {
const { credentials } = payload;
const storedCredential = await this.credentialsFinderService.findCredentialForUser(
credentials.id,
@ -148,9 +153,29 @@ export class CredentialsController {
throw new ForbiddenError();
}
const mergedCredentials = deepCopy(credentials);
const mergedCredentials: ICredentialsDecrypted = deepCopy({
...credentials,
data: credentials.data as ICredentialDataDecryptedObject | undefined,
});
const decryptedData = this.credentialsService.decrypt(storedCredential, true);
// Find the owning project to prevent leakage of other project data.
const owningProject = await this.sharedCredentialsRepository.findCredentialOwningProject(
storedCredential.id,
);
if (!owningProject) {
mergedCredentials.homeProject = undefined;
} else {
mergedCredentials.homeProject = {
id: owningProject.id,
name: owningProject.name,
icon: owningProject.icon,
type: owningProject.type,
createdAt: owningProject.createdAt.toISOString(),
updatedAt: owningProject.updatedAt.toISOString(),
};
}
// When a sharee (or project viewer) opens a credential, the fields and the
// credential data are missing so the payload will be empty
// We need to replace the credential contents with the db version if that's the case

View File

@ -0,0 +1,411 @@
import type { Logger } from '@n8n/backend-common';
import { mockInstance } from '@n8n/backend-test-utils';
import axios from 'axios';
import { mocked } from 'jest-mock';
import { mock } from 'jest-mock-extended';
import type { InstanceSettings, PackageDirectoryLoader } from 'n8n-core';
import { execFile } from 'node:child_process';
import { readFile, rm, writeFile } from 'node:fs/promises';
import path from 'node:path';
import { NPM_PACKAGE_STATUS_GOOD } from '@/constants';
import type { License } from '@/license';
import type { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
import type { Publisher } from '@/scaling/pubsub/publisher.service';
import type { CommunityPackagesConfig } from '../community-packages.config';
import { CommunityPackagesService } from '../community-packages.service';
import { InstalledNodesRepository } from '../installed-nodes.repository';
import type { InstalledPackages } from '../installed-packages.entity';
import { InstalledPackagesRepository } from '../installed-packages.repository';
import { checkIfVersionExistsOrThrow, verifyIntegrity } from '../npm-utils';
jest.mock('node:fs/promises');
jest.mock('node:child_process');
jest.mock('axios');
jest.mock('../community-node-types-utils', () => ({
getCommunityNodeTypes: jest.fn().mockResolvedValue([]),
}));
jest.mock('../npm-utils', () => ({
checkIfVersionExistsOrThrow: jest.fn().mockResolvedValue(true),
verifyIntegrity: jest.fn().mockResolvedValue(undefined),
}));
type ExecFileCallback = NonNullable<Parameters<typeof execFile>[3]>;
describe('CommunityPackagesService pubsub handlers', () => {
const PACKAGE_NAME = 'n8n-nodes-test';
const PACKAGE_VERSION = '1.0.0';
const REGISTRY = 'https://registry.npmjs.org';
const TARBALL_NAME = `${PACKAGE_NAME}-1.0.0.tgz`;
const nodesDownloadDir = path.join('tmp', 'n8n-jest-pubsub-downloads');
const instanceSettings = mock<InstanceSettings>({ nodesDownloadDir });
const logger = mock<Logger>();
const publisher = mock<Publisher>();
const license = mock<License>();
const loadNodesAndCredentials = mock<LoadNodesAndCredentials>();
mockInstance(InstalledNodesRepository);
const installedPackageRepository = mockInstance(InstalledPackagesRepository);
const createService = (configOverrides: Partial<CommunityPackagesConfig> = {}) => {
const config = mock<CommunityPackagesConfig>({
enabled: true,
preventLoading: false,
reinstallMissing: false,
registry: REGISTRY,
unverifiedEnabled: true,
...configOverrides,
});
return new CommunityPackagesService(
instanceSettings,
logger,
installedPackageRepository,
loadNodesAndCredentials,
publisher,
license,
config,
);
};
const execMock = ((...args: Parameters<typeof execFile>) => {
const command = args[0];
const cmdArgs = args[1];
const callback = args[args.length - 1] as ExecFileCallback;
if (command === 'npm' && cmdArgs?.[0] === 'pack') {
callback(null, { stdout: TARBALL_NAME } as never, '');
} else {
callback(null, 'Done', '');
}
}) as typeof execFile;
beforeEach(() => {
jest.resetAllMocks();
mocked(execFile).mockImplementation(execMock);
mocked(checkIfVersionExistsOrThrow).mockResolvedValue(true);
mocked(verifyIntegrity).mockResolvedValue(undefined);
mocked(readFile).mockResolvedValue(
JSON.stringify({ name: PACKAGE_NAME, version: PACKAGE_VERSION, dependencies: {} }),
);
mocked(writeFile).mockResolvedValue(undefined);
mocked(rm).mockResolvedValue(undefined);
mocked(axios.post).mockResolvedValue({ data: { status: NPM_PACKAGE_STATUS_GOOD } });
license.isCustomNpmRegistryEnabled.mockReturnValue(true);
loadNodesAndCredentials.loadPackage.mockResolvedValue(
mock<PackageDirectoryLoader>({
loadedNodes: [{ name: 'a-node-from-the-loader', version: 1 }],
}),
);
loadNodesAndCredentials.unloadPackage.mockResolvedValue(undefined);
loadNodesAndCredentials.postProcessLoaders.mockResolvedValue(undefined);
installedPackageRepository.findOne.mockResolvedValue(null);
});
const arrangeInstalledPackage = (
overrides: Partial<Pick<InstalledPackages, 'packageName' | 'installedVersion'>> = {},
) => {
installedPackageRepository.findOne.mockResolvedValue(
mock<InstalledPackages>({
packageName: PACKAGE_NAME,
installedVersion: PACKAGE_VERSION,
...overrides,
}),
);
};
const expectPack = (packageName: string, packageVersion: string) => {
expect(execFile).toHaveBeenCalledWith(
'npm',
['pack', `${packageName}@${packageVersion}`, `--registry=${REGISTRY}`, '--quiet'],
{ cwd: nodesDownloadDir },
expect.any(Function),
);
};
const expectNoDownloadOrLoad = () => {
expect(execFile).not.toHaveBeenCalled();
expect(loadNodesAndCredentials.loadPackage).not.toHaveBeenCalled();
};
const expectDownloadAndLoad = (packageName: string) => {
expectPack(packageName, PACKAGE_VERSION);
expect(loadNodesAndCredentials.loadPackage).toHaveBeenCalledWith(packageName);
expect(loadNodesAndCredentials.postProcessLoaders).toHaveBeenCalled();
};
test.each([PACKAGE_NAME, `@scope/${PACKAGE_NAME}`])(
'should download and load "%s" for a valid install event',
async (packageName) => {
const service = createService();
arrangeInstalledPackage({ packageName });
await service.handleInstallEvent({ packageName, packageVersion: PACKAGE_VERSION });
expectDownloadAndLoad(packageName);
},
);
test('should install a checksum-verified package when unverified packages are disabled', async () => {
const service = createService({ unverifiedEnabled: false });
arrangeInstalledPackage();
await service.handleInstallEvent({
packageName: PACKAGE_NAME,
packageVersion: PACKAGE_VERSION,
checksum: 'sha512-abc123',
});
expect(verifyIntegrity).toHaveBeenCalledWith(
PACKAGE_NAME,
PACKAGE_VERSION,
REGISTRY,
'sha512-abc123',
);
expectDownloadAndLoad(PACKAGE_NAME);
});
test('should not install a package whose checksum does not match', async () => {
const service = createService();
arrangeInstalledPackage();
mocked(verifyIntegrity).mockRejectedValue(new Error('Checksum mismatch'));
await expect(
service.handleInstallEvent({
packageName: PACKAGE_NAME,
packageVersion: PACKAGE_VERSION,
checksum: 'sha512-abc123',
}),
).resolves.toBeUndefined();
expectNoDownloadOrLoad();
});
test.each([
['leading segment is not a valid scope', `../${PACKAGE_NAME}`],
['required prefix is missing', 'some-other-package'],
['name is not a single word', 'n8n-nodes-With Space'],
])('should not install a package when the %s', async (_label, packageName) => {
const service = createService();
// Arranged so the stored-record guard passes and only the name guard can refuse.
arrangeInstalledPackage({ packageName });
await expect(
service.handleInstallEvent({ packageName, packageVersion: PACKAGE_VERSION }),
).resolves.toBeUndefined();
expectNoDownloadOrLoad();
});
test('should not install a package whose stored version specifier is invalid', async () => {
const service = createService();
arrangeInstalledPackage({ installedVersion: '1.a.b' });
await expect(
service.handleInstallEvent({ packageName: PACKAGE_NAME, packageVersion: '1.a.b' }),
).resolves.toBeUndefined();
expectNoDownloadOrLoad();
});
test('should not install an unverified package when unverified packages are disabled', async () => {
const service = createService({ unverifiedEnabled: false });
arrangeInstalledPackage();
await expect(
service.handleInstallEvent({ packageName: PACKAGE_NAME, packageVersion: PACKAGE_VERSION }),
).resolves.toBeUndefined();
expectNoDownloadOrLoad();
});
test('should not install a package rejected by the package status check', async () => {
const service = createService();
arrangeInstalledPackage();
mocked(axios.post).mockResolvedValue({ data: { status: 'Banned', reason: 'not allowed' } });
await expect(
service.handleInstallEvent({ packageName: PACKAGE_NAME, packageVersion: PACKAGE_VERSION }),
).resolves.toBeUndefined();
expectNoDownloadOrLoad();
});
test('should not install a version that does not exist in the registry', async () => {
const service = createService();
arrangeInstalledPackage({ installedVersion: '9.9.9' });
mocked(checkIfVersionExistsOrThrow).mockRejectedValue(new Error('Version not found'));
await expect(
service.handleInstallEvent({ packageName: PACKAGE_NAME, packageVersion: '9.9.9' }),
).resolves.toBeUndefined();
expectNoDownloadOrLoad();
});
test('should not install a package without a stored record', async () => {
const service = createService();
await expect(
service.handleInstallEvent({ packageName: PACKAGE_NAME, packageVersion: PACKAGE_VERSION }),
).resolves.toBeUndefined();
expectNoDownloadOrLoad();
});
test('should install the version from the stored record when the payload version differs', async () => {
const service = createService();
arrangeInstalledPackage({ installedVersion: '2.5.0' });
await service.handleInstallEvent({ packageName: PACKAGE_NAME, packageVersion: '9.9.9' });
expect(checkIfVersionExistsOrThrow).toHaveBeenCalledWith(PACKAGE_NAME, '2.5.0', REGISTRY);
expectPack(PACKAGE_NAME, '2.5.0');
expect(loadNodesAndCredentials.loadPackage).toHaveBeenCalledWith(PACKAGE_NAME);
});
test('should install without the payload checksum when the payload version differs', async () => {
const service = createService();
arrangeInstalledPackage({ installedVersion: '2.5.0' });
await service.handleInstallEvent({
packageName: PACKAGE_NAME,
packageVersion: '9.9.9',
checksum: 'sha512-abc123',
});
expect(verifyIntegrity).not.toHaveBeenCalled();
expectPack(PACKAGE_NAME, '2.5.0');
expect(loadNodesAndCredentials.loadPackage).toHaveBeenCalledWith(PACKAGE_NAME);
});
const disabledInstanceCases: Array<[string, Partial<CommunityPackagesConfig>]> = [
['community packages are disabled', { enabled: false }],
['package loading is disabled', { preventLoading: true }],
];
test.each(disabledInstanceCases)(
'should not install a package when %s',
async (_label, configOverrides) => {
const service = createService(configOverrides);
arrangeInstalledPackage();
await expect(
service.handleInstallEvent({ packageName: PACKAGE_NAME, packageVersion: PACKAGE_VERSION }),
).resolves.toBeUndefined();
expectNoDownloadOrLoad();
},
);
const expectNoRemoval = () => {
expect(rm).not.toHaveBeenCalled();
expect(loadNodesAndCredentials.unloadPackage).not.toHaveBeenCalled();
};
test('should delete and unload a package for a valid uninstall event', async () => {
const service = createService();
await service.handleUninstallEvent({ packageName: PACKAGE_NAME });
expect(rm).toHaveBeenCalledWith(`${nodesDownloadDir}/node_modules/${PACKAGE_NAME}`, {
recursive: true,
force: true,
});
expect(loadNodesAndCredentials.unloadPackage).toHaveBeenCalledWith(PACKAGE_NAME);
expect(loadNodesAndCredentials.postProcessLoaders).toHaveBeenCalled();
});
test.each(disabledInstanceCases)(
'should not uninstall a package when %s',
async (_label, configOverrides) => {
const service = createService(configOverrides);
await expect(
service.handleUninstallEvent({ packageName: PACKAGE_NAME }),
).resolves.toBeUndefined();
expectNoRemoval();
},
);
test('should not uninstall a package while a stored record exists', async () => {
const service = createService();
arrangeInstalledPackage();
await expect(
service.handleUninstallEvent({ packageName: PACKAGE_NAME }),
).resolves.toBeUndefined();
expectNoRemoval();
});
test.each([
`../${PACKAGE_NAME}`,
'../../etc',
'some-other-package',
`@scope/${PACKAGE_NAME}/..`,
`${PACKAGE_NAME}/../../node_modules/n8n-nodes-other`,
])('should not uninstall the package "%s"', async (packageName) => {
const service = createService();
await expect(service.handleUninstallEvent({ packageName })).resolves.toBeUndefined();
expectNoRemoval();
});
describe('logging', () => {
test('should log a refused command as skipped', async () => {
const service = createService();
await service.handleInstallEvent({
packageName: PACKAGE_NAME,
packageVersion: PACKAGE_VERSION,
});
expect(logger.warn).toHaveBeenCalledWith(
'Skipped installing community package',
expect.objectContaining({ packageName: PACKAGE_NAME }),
);
expect(logger.error).not.toHaveBeenCalled();
});
test('should log a command that failed while installing as an error', async () => {
const service = createService();
arrangeInstalledPackage();
mocked(execFile).mockImplementation(((...args: Parameters<typeof execFile>) => {
const callback = args[args.length - 1] as ExecFileCallback;
callback(new Error('npm exploded'), '', '');
}) as typeof execFile);
await service.handleInstallEvent({
packageName: PACKAGE_NAME,
packageVersion: PACKAGE_VERSION,
});
expect(logger.error).toHaveBeenCalledWith(
'Failed to install community package',
expect.objectContaining({ packageName: PACKAGE_NAME }),
);
});
test('should log a command that failed while uninstalling as an error', async () => {
const service = createService();
mocked(rm).mockRejectedValue(new Error('permission denied'));
await service.handleUninstallEvent({ packageName: PACKAGE_NAME });
expect(logger.error).toHaveBeenCalledWith(
'Failed to uninstall community package',
expect.objectContaining({ packageName: PACKAGE_NAME }),
);
});
});
});

View File

@ -402,6 +402,7 @@ describe('CommunityPackagesService', () => {
const PACKAGE_NAME = 'n8n-nodes-test';
const installedPackageForUpdateTest = mock<InstalledPackages>({
packageName: PACKAGE_NAME,
installedVersion: '1.0.0',
});
const packageDirectoryLoader = mock<PackageDirectoryLoader>({
@ -533,7 +534,7 @@ describe('CommunityPackagesService', () => {
expect(publisher.publishCommand).toHaveBeenCalledWith({
command: 'community-package-update',
payload: { packageName: PACKAGE_NAME, packageVersion: 'latest' },
payload: { packageName: PACKAGE_NAME, packageVersion: '1.0.0', checksum: undefined },
});
});
@ -554,6 +555,8 @@ describe('CommunityPackagesService', () => {
});
describe('installPackage', () => {
const PACKAGE_NAME = 'n8n-nodes-test';
test('should throw when installation of not vetted packages is forbidden', async () => {
config.unverifiedEnabled = false;
config.registry = 'https://registry.npmjs.org';
@ -561,6 +564,67 @@ describe('CommunityPackagesService', () => {
'Installation of unverified community packages is forbidden!',
);
});
test('should throw when the package is rejected by the package status check', async () => {
config.unverifiedEnabled = true;
config.registry = 'https://registry.npmjs.org';
mocked(axios.post).mockResolvedValue({ data: { status: 'Banned', reason: 'not allowed' } });
await expect(communityPackagesService.installPackage(PACKAGE_NAME, '1.0.0')).rejects.toThrow(
`Package "${PACKAGE_NAME}" is not allowed`,
);
expect(execFile).not.toHaveBeenCalled();
});
test('should throw for a package name that resolves outside its own directory', async () => {
config.unverifiedEnabled = true;
config.registry = 'https://registry.npmjs.org';
// `installPackage` does not parse the name, which is why the resolved directory is
// asserted to be the named one.
await expect(
communityPackagesService.installPackage(`${PACKAGE_NAME}/..`, '1.0.0'),
).rejects.toThrow('Invalid package name');
expect(mkdir).not.toHaveBeenCalled();
expect(rm).not.toHaveBeenCalled();
});
test('should publish the installed version rather than the requested specifier', async () => {
config.unverifiedEnabled = true;
config.registry = 'https://registry.npmjs.org';
mocked(execFile).mockImplementation(((...args: Parameters<typeof execFile>) => {
const cmdArgs = args[1];
const actualCallback = args[args.length - 1] as ExecFileCallback;
if (cmdArgs?.[0] === 'pack') {
actualCallback(null, { stdout: `${PACKAGE_NAME}-latest.tgz` } as never, '');
} else {
actualCallback(null, 'Done', '');
}
}) as typeof execFile);
mocked(readFile).mockResolvedValue(
JSON.stringify({ name: PACKAGE_NAME, version: '1.2.3', dependencies: {} }),
);
loadNodesAndCredentials.loadPackage.mockResolvedValue(
mock<PackageDirectoryLoader>({
loadedNodes: [{ name: 'a-node-from-the-loader', version: 1 }],
}),
);
installedPackageRepository.saveInstalledPackageWithNodes.mockResolvedValue(
mock<InstalledPackages>({ packageName: PACKAGE_NAME, installedVersion: '1.2.3' }),
);
await communityPackagesService.installPackage(PACKAGE_NAME);
expect(publisher.publishCommand).toHaveBeenCalledWith({
command: 'community-package-install',
payload: { packageName: PACKAGE_NAME, packageVersion: '1.2.3', checksum: undefined },
});
});
});
describe('ensurePackageJson', () => {

View File

@ -14,7 +14,7 @@ import {
} from 'n8n-workflow';
import { execFile } from 'node:child_process';
import { access, constants, mkdir, readFile, rm, writeFile } from 'node:fs/promises';
import { join } from 'node:path';
import { join, relative, resolve, sep } from 'node:path';
import { promisify } from 'node:util';
import {
@ -70,6 +70,11 @@ const INVALID_OR_SUSPICIOUS_PACKAGE_NAME = /[^0-9a-z@\-._/]/;
/** Built-in package names cannot be installed as community packages. */
const RESERVED_PACKAGE_NAMES = new Set<string>(BUILTIN_NODES_PACKAGES);
const NPM_PACKAGE_SCOPE_PATTERN = /^@[a-z0-9][a-z0-9\-._]*$/;
/** A package name holds no path separator, so it can never widen the resolved directory. */
const NPM_PACKAGE_NAME_PATTERN = /^[a-z0-9][a-z0-9\-._]*$/;
type PackageJson = {
name: 'installed-nodes';
private: true;
@ -146,6 +151,10 @@ export class CommunityPackagesService {
const scope = rawString.includes('/') ? rawString.split('/')[0] : undefined;
if (scope && !NPM_PACKAGE_SCOPE_PATTERN.test(scope)) {
throw new UnexpectedError(`Invalid package scope: ${scope}`);
}
const packageNameWithoutScope = scope ? rawString.replace(`${scope}/`, '') : rawString;
if (!packageNameWithoutScope.startsWith(NODE_PACKAGE_PREFIX)) {
@ -162,6 +171,12 @@ export class CommunityPackagesService {
const packageName = version ? rawString.replace(`@${version}`, '') : rawString;
const nameWithoutScopeOrVersion = scope ? packageName.replace(`${scope}/`, '') : packageName;
if (!NPM_PACKAGE_NAME_PATTERN.test(nameWithoutScopeOrVersion)) {
throw new UnexpectedError(`Invalid package name: ${packageName}`);
}
if (RESERVED_PACKAGE_NAMES.has(packageName)) {
throw new UserError(`Package name "${packageName}" is reserved for n8n built-in packages`);
}
@ -416,6 +431,24 @@ export class CommunityPackagesService {
}
}
private async runRegistryChecks(packageName: string, packageVersion: string, checksum?: string) {
this.checkInstallPermissions(Boolean(checksum));
const packageStatus = await this.checkNpmPackageStatus(packageName);
if (packageStatus.status !== NPM_PACKAGE_STATUS_GOOD) {
throw new UnexpectedError(`Package "${packageName}" is not allowed`);
}
const registry = this.getNpmRegistry();
if (checksum) {
await verifyIntegrity(packageName, packageVersion, registry, checksum);
}
await checkIfVersionExistsOrThrow(packageName, packageVersion, registry);
}
private async installOrUpdatePackage(
packageName: string,
options:
@ -425,14 +458,7 @@ export class CommunityPackagesService {
const isUpdate = 'installedPackage' in options;
const packageVersion = !options.version ? 'latest' : options.version;
const shouldValidateChecksum = 'checksum' in options && Boolean(options.checksum);
this.checkInstallPermissions(shouldValidateChecksum);
if (options.checksum) {
await verifyIntegrity(packageName, packageVersion, this.getNpmRegistry(), options.checksum);
}
await checkIfVersionExistsOrThrow(packageName, packageVersion, this.getNpmRegistry());
await this.runRegistryChecks(packageName, packageVersion, options.checksum);
try {
await this.downloadPackage(packageName, packageVersion);
@ -464,7 +490,11 @@ export class CommunityPackagesService {
const installedPackage = await this.persistInstalledPackage(loader);
void this.publisher.publishCommand({
command: isUpdate ? 'community-package-update' : 'community-package-install',
payload: { packageName, packageVersion },
payload: {
packageName,
packageVersion: installedPackage.installedVersion,
checksum: options.checksum,
},
});
await this.loadNodesAndCredentials.postProcessLoaders();
this.logger.info(`Community package installed: ${packageName}`);
@ -484,18 +514,107 @@ export class CommunityPackagesService {
}
}
private assertPackageChangesAllowed() {
if (!this.config.enabled) {
throw new UserError('Community packages are disabled on this instance');
}
if (this.config.preventLoading) {
throw new UserError('Community package loading is disabled on this instance');
}
}
// Payloads from other instances pass the same gates as a local install request. Not
// `communityPackagesManagedByEnv`: only the main runs the env loader, and it publishes
// these commands, so a follower that refused them would never get the packages.
// Failures are logged rather than rethrown, so one message cannot break the subscriber.
@OnPubSubEvent('community-package-install')
@OnPubSubEvent('community-package-update')
async handleInstallEvent({
packageName,
packageVersion,
}: { packageName: string; packageVersion: string }) {
await this.installOrUpdateNpmPackage(packageName, packageVersion);
checksum,
}: { packageName: string; packageVersion: string; checksum?: string }) {
let accepted = false;
try {
this.assertPackageChangesAllowed();
const parsed = this.parseNpmPackageName(packageName);
// The payload only says that something needs converging. What gets installed comes
// from the record the publishing instance persisted before it published.
const installedPackage = await this.findInstalledPackage(parsed.packageName);
if (!installedPackage) {
throw new UnexpectedError('No installed package record found', {
extra: { packageName: parsed.packageName },
});
}
const resolvedVersion = installedPackage.installedVersion;
if (!valid(resolvedVersion)) {
throw new UnexpectedError(`Invalid version: ${resolvedVersion}`);
}
const versionMatches = resolvedVersion === packageVersion;
if (!versionMatches) {
this.logger.warn('Community package version differs from the stored record', {
packageName: installedPackage.packageName,
packageVersion,
installedVersion: resolvedVersion,
});
}
// Integrity is recorded per version, so the payload checksum only applies when the
// payload names the version that is about to be installed.
await this.runRegistryChecks(
installedPackage.packageName,
resolvedVersion,
versionMatches ? checksum : undefined,
);
accepted = true;
await this.installOrUpdateNpmPackage(installedPackage.packageName, resolvedVersion);
} catch (error) {
const details = { packageName, reason: ensureError(error).message };
if (accepted) this.logger.error('Failed to install community package', details);
else this.logger.warn('Skipped installing community package', details);
}
}
@OnPubSubEvent('community-package-uninstall')
async handleUninstallEvent({ packageName }: { packageName: string }) {
await this.removeNpmPackage(packageName);
let accepted = false;
try {
this.assertPackageChangesAllowed();
const parsed = this.parseNpmPackageName(packageName);
// The mirror of the install path: the removed record is what says the package is
// meant to be gone, so a payload on its own never removes anything.
const installedPackage = await this.findInstalledPackage(parsed.packageName);
if (installedPackage) {
throw new UnexpectedError('Installed package record still present', {
extra: { packageName: parsed.packageName },
});
}
accepted = true;
await this.removeNpmPackage(parsed.packageName);
} catch (error) {
const details = { packageName, reason: ensureError(error).message };
if (accepted) this.logger.error('Failed to uninstall community package', details);
else this.logger.warn('Skipped uninstalling community package', details);
}
}
private async installOrUpdateNpmPackage(packageName: string, packageVersion: string) {
@ -513,7 +632,25 @@ export class CommunityPackagesService {
}
private resolvePackageDirectory(packageName: string) {
return `${this.downloadFolder}/node_modules/${packageName}`;
const nodeModulesDirectory = `${this.downloadFolder}/node_modules`;
const packageDirectory = `${nodeModulesDirectory}/${packageName}`;
// Not every caller validates the name, so confirm the resolved path is exactly the
// named directory. A prefix check alone still accepts a name like `@scope/pkg/..`,
// which resolves to the scope directory and stays under node_modules.
const relativePath = relative(resolve(nodeModulesDirectory), resolve(packageDirectory));
// An empty relative path is the download folder itself, and a leading `..` is outside
// it. Both of those compare equal to the name that produced them, so reject them first.
if (
relativePath === '' ||
relativePath.startsWith('..') ||
relativePath.split(sep).join('/') !== packageName
) {
throw new UserError('Invalid package name', { extra: { packageName } });
}
return packageDirectory;
}
private async downloadPackage(packageName: string, packageVersion: string): Promise<string> {

View File

@ -33,11 +33,13 @@ export type PubSubCommandMap = {
'community-package-install': {
packageName: string;
packageVersion: string;
checksum?: string;
};
'community-package-update': {
packageName: string;
packageVersion: string;
checksum?: string;
};
'community-package-uninstall': {

View File

@ -20,7 +20,6 @@ import {
import { Service } from '@n8n/di';
// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
import { In, type EntityManager } from '@n8n/typeorm';
import omit from 'lodash/omit';
import type { INode, IWorkflowBase, WorkflowId } from 'n8n-workflow';
import {
EXECUTE_WORKFLOW_NODE_TYPE,
@ -200,62 +199,70 @@ export class EnterpriseWorkflowService {
/**
* We only need to check nodes that use credentials the current user cannot access,
* since these can be 2 possibilities:
* - Same ID already exist: it's a read only node and therefore cannot be changed
* - It matches exactly one previous node: it's a read only node and therefore cannot be changed
* - It's a new node which indicates tampering and therefore must fail saving
*/
const allowedCredentialIds = credentialsUserHasAccessTo.map((cred) => cred.id);
const nodesWithCredentialsUserDoesNotHaveAccessTo = this.getNodesWithInaccessibleCreds(
newWorkflowVersion,
allowedCredentialIds,
const nodesWithCredentialsUserDoesNotHaveAccessTo = new Set(
this.getNodesWithInaccessibleCreds(newWorkflowVersion, allowedCredentialIds),
);
// If there are no nodes with credentials the user does not have access to we can skip the rest
if (nodesWithCredentialsUserDoesNotHaveAccessTo.length === 0) {
if (nodesWithCredentialsUserDoesNotHaveAccessTo.size === 0) {
return newWorkflowVersion;
}
const previouslyExistingNodeIds = previousWorkflowVersion.nodes.map((node) => node.id);
// Node ids are meant to be unique. Edge case: where stored data still repeats one, match the
// first node.
const previousNodesById = new Map<string, INode>();
for (const node of previousWorkflowVersion.nodes) {
if (!previousNodesById.has(node.id)) previousNodesById.set(node.id, node);
}
// If it's a new node we can't allow it to be saved
// since it uses creds the node doesn't have access
const isTamperingAttempt = (inaccessibleCredNodeId: string) =>
!previouslyExistingNodeIds.includes(inaccessibleCredNodeId);
const submittedIdCounts = new Map<string, number>();
for (const node of newWorkflowVersion.nodes) {
submittedIdCounts.set(node.id, (submittedIdCounts.get(node.id) ?? 0) + 1);
}
nodesWithCredentialsUserDoesNotHaveAccessTo.forEach((node) => {
if (isTamperingAttempt(node.id)) {
newWorkflowVersion.nodes = newWorkflowVersion.nodes.map((node) => {
if (!nodesWithCredentialsUserDoesNotHaveAccessTo.has(node)) return node;
// Only a node we can prove is the stored one may keep credentials the user cannot
// access. A missing id means the node is new; a repeated id makes the match
// ambiguous, and an ambiguous match is not a proof, so no claimant is trusted.
const previousNode = previousNodesById.get(node.id);
const idClaimedOnce = submittedIdCounts.get(node.id) === 1;
if (!previousNode || !idClaimedOnce) {
this.logger.warn('Blocked workflow update due to tampering attempt', {
nodeType: node.type,
nodeName: node.name,
nodeId: node.id,
nodeCredentials: node.credentials,
});
// Node is new, so this is probably a tampering attempt. Throw an error
throw new NodeOperationError(
node,
`You don't have access to the credentials in the '${node.name}' node. Ask the owner to share them with you.`,
);
}
// Replace the node with the previous version of the node
// Since it cannot be modified (read only node)
const nodeIdx = newWorkflowVersion.nodes.findIndex(
(newWorkflowNode) => newWorkflowNode.id === node.id,
);
this.logger.debug('Replacing node with previous version when saving updated workflow', {
nodeType: node.type,
nodeName: node.name,
nodeId: node.id,
});
const previousNodeVersion = previousWorkflowVersion.nodes.find(
(previousNode) => previousNode.id === node.id,
);
// Replace the whole node. Patching the previous fields over the submitted node
// would keep every field the previous node does not have.
// Allow changing only name, position and disabled status for read-only nodes
Object.assign(
newWorkflowVersion.nodes[nodeIdx],
omit(previousNodeVersion, ['name', 'position', 'disabled']),
);
const restoredNode = { ...previousNode };
if ('name' in node) restoredNode.name = node.name;
if ('position' in node) restoredNode.position = node.position;
if ('disabled' in node) restoredNode.disabled = node.disabled;
else delete restoredNode.disabled;
return restoredNode;
});
return newWorkflowVersion;

View File

@ -1622,8 +1622,10 @@ describe('POST /credentials/test', () => {
expect(mockCredentialsTester.testCredentials.mock.calls[0][1]).toBe(savedCredential.type);
expect(mockCredentialsTester.testCredentials.mock.calls[0][2]).toEqual({
id: savedCredential.id,
name: '',
type: savedCredential.type,
data: credential.data,
homeProject: expect.objectContaining({ id: ownerPersonalProject.id }),
});
});
@ -1652,8 +1654,10 @@ describe('POST /credentials/test', () => {
expect(mockCredentialsTester.testCredentials.mock.calls[0][1]).toBe(savedCredential.type);
expect(mockCredentialsTester.testCredentials.mock.calls[0][2]).toEqual({
id: savedCredential.id,
name: '',
type: savedCredential.type,
data: credential.data,
homeProject: expect.objectContaining({ id: ownerPersonalProject.id }),
});
});

View File

@ -3,10 +3,12 @@ import {
CredentialsEntity,
CredentialsRepository,
SharedWorkflowRepository,
WorkflowEntity,
WorkflowRepository,
} from '@n8n/db';
import { Container } from '@n8n/di';
import { mock } from 'jest-mock-extended';
import type { INode } from 'n8n-workflow';
import { Telemetry } from '@/telemetry';
import { EnterpriseWorkflowService } from '@/workflows/workflow.service.ee';
@ -59,6 +61,28 @@ describe('EnterpriseWorkflowService', () => {
return credentialEntity;
}
const STORED_NODE_ID = '4673f869-f2dc-4a33-b053-ca3193bc5226';
const inaccessibleCredential = {
test: { id: FIRST_CREDENTIAL_ID, name: 'First fake credential' },
};
const makeNode = (overrides: Partial<INode>): INode => ({
id: STORED_NODE_ID,
name: 'Node',
type: 'n8n-nodes-base.httpRequest',
typeVersion: 1,
position: [0, 0],
parameters: {},
...overrides,
});
const workflowWithNodes = (nodes: INode[]) => {
const workflow = new WorkflowEntity();
workflow.nodes = nodes;
return workflow;
};
it('Should throw error saving a workflow using credential without access', () => {
const newWorkflowVersion = getWorkflow({ addNodeWithOneCred: true });
const previousWorkflowVersion = getWorkflow();
@ -116,6 +140,59 @@ describe('EnterpriseWorkflowService', () => {
service.validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, []);
}).toThrow();
});
it('Should reject a repeated node id when both claimants use the credential', () => {
const previousWorkflowVersion = workflowWithNodes([
makeNode({ id: STORED_NODE_ID, name: 'First', credentials: inaccessibleCredential }),
]);
const newWorkflowVersion = workflowWithNodes([
makeNode({ id: STORED_NODE_ID, name: 'First', credentials: inaccessibleCredential }),
makeNode({
id: STORED_NODE_ID,
name: 'Second',
credentials: inaccessibleCredential,
parameters: { url: 'https://example.com/' },
}),
]);
expect(() => {
service.validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, []);
}).toThrow();
});
// Only one claimant is checked here, so the id count has to cover every submitted node.
it('Should reject a repeated node id when only one claimant uses the credential', () => {
const previousWorkflowVersion = workflowWithNodes([
makeNode({ id: STORED_NODE_ID, name: 'First', credentials: inaccessibleCredential }),
]);
const newWorkflowVersion = workflowWithNodes([
makeNode({ id: STORED_NODE_ID, name: 'First' }),
makeNode({
id: STORED_NODE_ID,
name: 'Second',
credentials: inaccessibleCredential,
parameters: { url: 'https://example.com/' },
}),
]);
expect(() => {
service.validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, []);
}).toThrow();
});
it('Should not keep submitted fields that the stored node does not have', () => {
const storedNode = makeNode({ id: STORED_NODE_ID, credentials: inaccessibleCredential });
delete (storedNode as Partial<INode>).parameters;
const previousWorkflowVersion = workflowWithNodes([storedNode]);
const newWorkflowVersion = workflowWithNodes([
{ ...storedNode, parameters: { url: 'https://example.com/' } },
]);
service.validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, []);
expect(newWorkflowVersion.nodes[0].parameters).toBeUndefined();
});
});
describe('getNodesWithInaccessibleCreds', () => {

View File

@ -2256,6 +2256,125 @@ describe('RoutingNode', () => {
}
});
describe('per-item errors', () => {
const createRoutingNode = ({
continueOnFail,
requestErrorUrl,
}: {
continueOnFail: boolean;
requestErrorUrl?: string;
}) => {
const items: INodeExecutionData[] = [
{ json: { id: 'first' } },
{ json: requestErrorUrl ? { id: 'second' } : {} },
{ json: { id: 'third' } },
];
const node: INode = {
parameters: {},
name: 'test',
type: 'test.set',
typeVersion: 1,
id: 'uuid-1234',
position: [0, 0],
};
const nodeType = mock<INodeType>();
nodeType.description = {
requestDefaults: {
baseURL: 'http://127.0.0.1:5678',
url: '=/items/{{ toPathSegment($json.id) }}',
},
properties: [],
} as unknown as INodeTypeDescription;
const runExecutionData = createEmptyRunExecutionData();
const workflow = new Workflow({
nodes: [node],
connections: {},
active: false,
nodeTypes,
});
const executeFunctions = mock<executionContexts.ExecuteContext>();
Object.assign(executeFunctions, {
executeData: { data: {}, node, source: null } as IExecuteData,
inputData: { main: [items] } as ITaskDataConnections,
runIndex: 0,
additionalData,
workflow,
node,
mode: 'internal',
connectionInputData: items,
runExecutionData,
});
executeFunctions.getNodeParameter.mockReturnValue({});
const requestUrls: string[] = [];
const contexts = items.map((_, itemIndex) => {
const context = getExecuteSingleFunctions(workflow, runExecutionData, 0, node, itemIndex);
// @ts-expect-error overwriting a method
context.getNodeParameter = () => ({});
context.continueOnFail.mockReturnValue(continueOnFail);
context.helpers.httpRequest = jest.fn(async (requestOptions: IHttpRequestOptions) => {
requestUrls.push(requestOptions.url);
if (requestOptions.url === requestErrorUrl) {
throw new Error('Request failed');
}
return { body: { url: requestOptions.url } };
});
return context;
});
let contextIndex = 0;
jest.spyOn(executionContexts, 'ExecuteSingleContext').mockImplementation(function (
this: executionContexts.ExecuteSingleContext,
) {
return contexts[contextIndex++] as never;
} as never);
return {
requestUrls,
run: new RoutingNode(executeFunctions, nodeType).runNode(),
};
};
test('returns a preparation error and processes later items when continue on fail is enabled', async () => {
const { requestUrls, run } = createRoutingNode({ continueOnFail: true });
const result = await run;
expect(requestUrls).toEqual(['/items/first', '/items/third']);
expect(result?.[0]?.[0]?.json).toEqual({ url: '/items/first' });
expect(result?.[0]?.[1]?.error).toMatchObject({
name: 'NodeOperationError',
message: 'Invalid identifier: a value is required',
context: { itemIndex: 1, runIndex: 0 },
});
expect(result?.[0]?.[2]?.json).toEqual({ url: '/items/third' });
});
test('throws a preparation error and does not process later items when continue on fail is disabled', async () => {
const { requestUrls, run } = createRoutingNode({ continueOnFail: false });
await expect(run).rejects.toMatchObject({
name: 'NodeOperationError',
message: 'Invalid identifier: a value is required',
context: { itemIndex: 1, runIndex: 0 },
});
expect(requestUrls).toEqual(['/items/first']);
});
test('returns a request error and processes later items when continue on fail is enabled', async () => {
const { requestUrls, run } = createRoutingNode({
continueOnFail: true,
requestErrorUrl: '/items/second',
});
const result = await run;
expect(requestUrls).toEqual(['/items/first', '/items/second', '/items/third']);
expect(result?.[0]?.[0]?.json).toEqual({ url: '/items/first' });
expect(result?.[0]?.[1]?.error).toMatchObject({ message: 'Request failed' });
expect(result?.[0]?.[2]?.json).toEqual({ url: '/items/third' });
});
});
describe('itemIndex', () => {
const tests: Array<{
description: string;

View File

@ -12,7 +12,9 @@ import type {
} from 'n8n-workflow';
import { CHAT_TRIGGER_NODE_TYPE, createRunExecutionData, NodeConnectionTypes } from 'n8n-workflow';
import { WAITING_TOKEN_QUERY_PARAM } from '@/constants';
import { InstanceSettings } from '@/instance-settings';
import { generateUrlSignature, prepareUrlForSigning } from '@/utils/signature-helpers';
import { NodeExecutionContext } from '../node-execution-context';
@ -403,5 +405,50 @@ describe('NodeExecutionContext', () => {
'http://localhost/waiting-webhook/123/node456?approved=true&signature=11c5efc97a0d6f2ea9045dba6e397596cba29dc24adb44a9ebd3d1272c991e9b',
);
});
describe('when the node id is not a plain path segment', () => {
const NODE_ID = '../43/dddd2020-0000-4000-8000-000000002099';
const signedUrlForNodeId = (nodeId: string) =>
new URL(
new TestContext(
workflow,
mock<INode>({ id: nodeId }),
mock<IWorkflowExecuteAdditionalData>({
executionId: '123',
webhookWaitingBaseUrl: 'http://localhost/waiting-webhook',
formWaitingBaseUrl: 'http://localhost/form-waiting',
}),
mode,
createRunExecutionData({ resultData: { runData: {} } }),
).getSignedResumeUrl({ approved: 'true' }),
);
it('should keep the node id inside its own path segment', () => {
expect(signedUrlForNodeId(NODE_ID).pathname).toBe(
'/waiting-webhook/123/..%2F43%2Fdddd2020-0000-4000-8000-000000002099',
);
});
it('should keep the signature scoped to the escaped path', () => {
const signature = signedUrlForNodeId(NODE_ID).searchParams.get('signature')!;
// Unescaped, the same ids normalise to a different path, so the signature over the
// escaped one must not validate against it.
const normalisedUrl = new URL(
`http://localhost/waiting-webhook/123/${NODE_ID}?approved=true&signature=${signature}`,
);
expect(normalisedUrl.pathname).toBe(
'/waiting-webhook/43/dddd2020-0000-4000-8000-000000002099',
);
normalisedUrl.searchParams.delete(WAITING_TOKEN_QUERY_PARAM);
const expectedSignature = generateUrlSignature(
prepareUrlForSigning(normalisedUrl),
instanceSettings.hmacSignatureSecret,
);
expect(signature).not.toBe(expectedSignature);
});
});
});
});

View File

@ -46,7 +46,11 @@ import { ensureType } from './utils/ensure-type';
import { extractValue } from './utils/extract-value';
import { getAdditionalKeys } from './utils/get-additional-keys';
import { validateValueAgainstSchema } from './utils/validate-value-against-schema';
import { generateUrlSignature, prepareUrlForSigning } from '../../utils/signature-helpers';
import {
buildResumeUrlSuffix,
generateUrlSignature,
prepareUrlForSigning,
} from '../../utils/signature-helpers';
export abstract class NodeExecutionContext implements Omit<FunctionsBase, 'getCredentials'> {
protected readonly instanceSettings = Container.get(InstanceSettings);
@ -238,7 +242,9 @@ export abstract class NodeExecutionContext implements Omit<FunctionsBase, 'getCr
throw new UnexpectedError('Execution id is missing');
}
const baseURL = new URL(`${webhookWaitingBaseUrl}/${executionId}/${this.node.id}`);
const baseURL = new URL(
`${webhookWaitingBaseUrl}${buildResumeUrlSuffix(executionId, this.node.id)}`,
);
for (const [key, value] of Object.entries(parameters)) {
baseURL.searchParams.set(key, value);

View File

@ -119,24 +119,41 @@ export class RoutingNode {
} as DeclarativeRestApiSettings.ResultOptions,
});
const { proxy, timeout, allowUnauthorizedCerts } = itemContext[
itemIndex
].thisArgs.getNodeParameter('requestOptions', 0, {}) as {
proxy: string;
timeout: number;
allowUnauthorizedCerts: boolean;
};
if (nodeType.description.requestOperations) {
itemContext[itemIndex].requestData.requestOperations = {
...nodeType.description.requestOperations,
try {
const { proxy, timeout, allowUnauthorizedCerts } = itemContext[
itemIndex
].thisArgs.getNodeParameter('requestOptions', 0, {}) as {
proxy: string;
timeout: number;
allowUnauthorizedCerts: boolean;
};
}
if (nodeType.description.requestDefaults) {
for (const key of Object.keys(nodeType.description.requestDefaults)) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let value = (nodeType.description.requestDefaults as Record<string, any>)[key];
if (nodeType.description.requestOperations) {
itemContext[itemIndex].requestData.requestOperations = {
...nodeType.description.requestOperations,
};
}
if (nodeType.description.requestDefaults) {
for (const key of Object.keys(nodeType.description.requestDefaults)) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let value = (nodeType.description.requestDefaults as Record<string, any>)[key];
// If the value is an expression resolve it
value = this.getParameterValue(
value,
itemIndex,
runIndex,
executeData,
{ $credentials: credentials, $version: node.typeVersion },
false,
) as string;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(itemContext[itemIndex].requestData.options as Record<string, any>)[key] = value;
}
}
for (const property of nodeType.description.properties) {
let value = get(node.parameters, property.name, []) as string | NodeParameterValue;
// If the value is an expression resolve it
value = this.getParameterValue(
value,
@ -145,109 +162,113 @@ export class RoutingNode {
executeData,
{ $credentials: credentials, $version: node.typeVersion },
false,
) as string;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(itemContext[itemIndex].requestData.options as Record<string, any>)[key] = value;
) as string | NodeParameterValue;
const tempOptions = this.getRequestOptionsFromParameters(
itemContext[itemIndex].thisArgs,
property,
itemIndex,
runIndex,
'',
{ $credentials: credentials, $value: value, $version: node.typeVersion },
);
this.mergeOptions(itemContext[itemIndex].requestData, tempOptions);
}
}
for (const property of nodeType.description.properties) {
let value = get(node.parameters, property.name, []) as string | NodeParameterValue;
// If the value is an expression resolve it
value = this.getParameterValue(
value,
itemIndex,
runIndex,
executeData,
{ $credentials: credentials, $version: node.typeVersion },
false,
) as string | NodeParameterValue;
if (proxy) {
const proxyParsed = url.parse(proxy);
const proxyProperties = ['host', 'port'];
const tempOptions = this.getRequestOptionsFromParameters(
itemContext[itemIndex].thisArgs,
property,
itemIndex,
runIndex,
'',
{ $credentials: credentials, $value: value, $version: node.typeVersion },
);
for (const property of proxyProperties) {
if (
!(property in proxyParsed) ||
proxyParsed[property as keyof typeof proxyParsed] === null
) {
throw new NodeOperationError(node, 'The proxy is not value', {
runIndex,
itemIndex,
description: `The proxy URL does not contain a valid value for "${property}"`,
});
}
}
this.mergeOptions(itemContext[itemIndex].requestData, tempOptions);
}
itemContext[itemIndex].requestData.options.proxy = {
host: proxyParsed.hostname as string,
port: parseInt(proxyParsed.port!),
protocol: proxyParsed.protocol?.replace(/:$/, '') || undefined,
};
if (proxy) {
const proxyParsed = url.parse(proxy);
const proxyProperties = ['host', 'port'];
for (const property of proxyProperties) {
if (
!(property in proxyParsed) ||
proxyParsed[property as keyof typeof proxyParsed] === null
) {
throw new NodeOperationError(node, 'The proxy is not value', {
runIndex,
itemIndex,
description: `The proxy URL does not contain a valid value for "${property}"`,
});
if (proxyParsed.auth) {
const [username, password] = proxyParsed.auth.split(':');
itemContext[itemIndex].requestData.options.proxy!.auth = {
username,
password,
};
}
}
itemContext[itemIndex].requestData.options.proxy = {
host: proxyParsed.hostname as string,
port: parseInt(proxyParsed.port!),
protocol: proxyParsed.protocol?.replace(/:$/, '') || undefined,
};
if (proxyParsed.auth) {
const [username, password] = proxyParsed.auth.split(':');
itemContext[itemIndex].requestData.options.proxy!.auth = {
username,
password,
};
if (allowUnauthorizedCerts) {
itemContext[itemIndex].requestData.options.skipSslCertificateValidation =
allowUnauthorizedCerts;
}
}
if (allowUnauthorizedCerts) {
itemContext[itemIndex].requestData.options.skipSslCertificateValidation =
allowUnauthorizedCerts;
}
if (timeout) {
itemContext[itemIndex].requestData.options.timeout = timeout;
} else {
// set default timeout to 5 minutes
itemContext[itemIndex].requestData.options.timeout = 300_000;
}
if (timeout) {
itemContext[itemIndex].requestData.options.timeout = timeout;
} else {
// set default timeout to 5 minutes
itemContext[itemIndex].requestData.options.timeout = 300_000;
}
if (credentials?.allowedHttpRequestDomains === 'none') {
throw new NodeOperationError(
node,
'This credential is configured to prevent use within an HTTP Request node',
);
}
if (credentials?.allowedHttpRequestDomains === 'none') {
throw new NodeOperationError(
node,
'This credential is configured to prevent use within an HTTP Request node',
const allowedDomains = getCredentialAllowedDomains(credentials);
if (credentials?.allowedHttpRequestDomains === 'domains' && !allowedDomains) {
throw new NodeOperationError(
node,
'No allowed domains specified. Configure allowed domains or change restriction setting.',
);
}
if (allowedDomains) {
itemContext[itemIndex].requestData.options.allowedDomains = allowedDomains;
}
requestPromises.push(
this.makeRequest(
itemContext[itemIndex].requestData,
itemContext[itemIndex].thisArgs,
itemIndex,
runIndex,
credentialDescription?.name,
itemContext[itemIndex].requestData.requestOperations,
credentialsDecrypted,
),
);
}
} catch (error) {
const nodeError =
error instanceof NodeApiError || error instanceof NodeOperationError
? error
: new NodeOperationError(
node,
error instanceof Error ? error : new Error(String(error)),
{ itemIndex, runIndex },
);
const allowedDomains = getCredentialAllowedDomains(credentials);
if (credentials?.allowedHttpRequestDomains === 'domains' && !allowedDomains) {
throw new NodeOperationError(
node,
'No allowed domains specified. Configure allowed domains or change restriction setting.',
);
}
if (allowedDomains) {
itemContext[itemIndex].requestData.options.allowedDomains = allowedDomains;
}
set(nodeError, 'context.itemIndex', itemIndex);
set(nodeError, 'context.runIndex', runIndex);
requestPromises.push(
this.makeRequest(
itemContext[itemIndex].requestData,
itemContext[itemIndex].thisArgs,
itemIndex,
runIndex,
credentialDescription?.name,
itemContext[itemIndex].requestData.requestOperations,
credentialsDecrypted,
),
);
if (itemContext[itemIndex].thisArgs.continueOnFail()) {
requestPromises.push(Promise.resolve([{ json: {}, error: nodeError }]));
continue;
}
throw nodeError;
}
}
const promisesResponses = await Promise.allSettled(requestPromises);

View File

@ -1,4 +1,4 @@
import { generateUrlSignature } from '../signature-helpers';
import { buildResumeUrlSuffix, generateUrlSignature } from '../signature-helpers';
describe('signature-helpers', () => {
const secret = 'test-secret';
@ -24,3 +24,32 @@ describe('signature-helpers', () => {
});
});
});
describe('buildResumeUrlSuffix', () => {
it('should leave a uuid node id unchanged', () => {
expect(buildResumeUrlSuffix('123', 'dddd2020-0000-4000-8000-000000002099')).toBe(
'/123/dddd2020-0000-4000-8000-000000002099',
);
});
it('should confine a node id to a single path segment', () => {
expect(buildResumeUrlSuffix('123', '../43/other')).toBe('/123/..%2F43%2Fother');
});
it('should confine a node id that opens a query string', () => {
expect(buildResumeUrlSuffix('123', 'x?approved=true')).toBe('/123/x%3Fapproved%3Dtrue');
});
describe('when the node id is a bare dot segment', () => {
// Encoding leaves `.` and `..` untouched, so the documented outcome is a link that
// collapses on parsing and no longer resolves. Pinned so the limitation stays visible.
it.each([
{ nodeId: '.', suffix: '/123/.', pathname: '/waiting-webhook/123/' },
{ nodeId: '..', suffix: '/123/..', pathname: '/waiting-webhook/' },
])('should collapse the path for a node id of "$nodeId"', ({ nodeId, suffix, pathname }) => {
expect(buildResumeUrlSuffix('123', nodeId)).toBe(suffix);
expect(new URL(`http://localhost/waiting-webhook${suffix}`).pathname).toBe(pathname);
});
});
});

View File

@ -8,6 +8,18 @@ export function generateUrlSignature(url: string, secret: string) {
return token;
}
/**
* Builds the `/<executionId>/<nodeId>` suffix of a waiting-webhook resume URL.
*
* The node id is client-supplied and stored verbatim, so it is percent-encoded to keep it
* within one path segment of the signed URL. Ids of exactly `.` or `..` are the exception —
* no encoding of a dot segment survives URL normalisation, so those collapse the path and
* yield a link that does not resolve. The execution id is server-generated and already URL-safe.
*/
export function buildResumeUrlSuffix(executionId: string, nodeId: string): string {
return `/${executionId}/${encodeURIComponent(nodeId)}`;
}
/**
* Prepare url for signing
*/

View File

@ -100,6 +100,62 @@ describe('ResourceLocatorDropdown', () => {
});
});
describe('opening a result link', () => {
async function clickCachedResultLink(cachedResultUrl: string) {
vi.useFakeTimers();
const cachedModelValue: INodeParameterResourceLocator = {
__rl: true,
value: 'workflow-cached',
mode: 'list',
cachedResultName: 'Cached Workflow',
cachedResultUrl,
};
renderComponent({
props: {
show: true,
resources: mockResources, // doesn't contain workflow-cached
modelValue: cachedModelValue,
},
});
const cachedItem = screen.getByText('Cached Workflow').closest('[data-test-id="rlc-item"]');
await fireEvent.mouseEnter(cachedItem as Element);
await vi.advanceTimersByTimeAsync(250);
const linkIcon = cachedItem?.querySelector('svg[data-icon="external-link"]');
expect(linkIcon).toBeInTheDocument();
await fireEvent.click(linkIcon as Element);
}
it('should open an http(s) result url with noopener,noreferrer', async () => {
const windowOpenSpy = vi.spyOn(window, 'open').mockImplementation(() => null);
await clickCachedResultLink('https://example.com/resource');
expect(windowOpenSpy).toHaveBeenCalledWith(
'https://example.com/resource',
'_blank',
'noopener,noreferrer',
);
windowOpenSpy.mockRestore();
});
it('should not open a result url with a disallowed scheme', async () => {
const windowOpenSpy = vi.spyOn(window, 'open').mockImplementation(() => null);
await clickCachedResultLink('javascript:alert(document.domain)');
expect(windowOpenSpy).not.toHaveBeenCalled();
windowOpenSpy.mockRestore();
});
});
describe('model value handling', () => {
it('should compare values correctly for selection highlighting', () => {
const modelValue: INodeParameterResourceLocator = {

View File

@ -7,6 +7,7 @@ import type { EventBus } from '@n8n/utils/event-bus';
import { createEventBus } from '@n8n/utils/event-bus';
import type { INodeParameterResourceLocator } from 'n8n-workflow';
import { computed, onBeforeUnmount, onMounted, ref, useCssModule, watch } from 'vue';
import { openSafeUrl } from '@/app/utils/htmlUtils';
const SEARCH_BAR_HEIGHT_PX = 40;
const SCROLL_MARGIN_PX = 10;
@ -135,7 +136,7 @@ function openUrl(event: MouseEvent, url: string) {
event.preventDefault();
event.stopPropagation();
window.open(url, '_blank');
openSafeUrl(url);
}
function onKeyDown(e: KeyboardEvent) {

View File

@ -16,6 +16,7 @@ import { useRootStore } from '@n8n/stores/useRootStore';
import { useTelemetry } from '@/app/composables/useTelemetry';
import ConfirmProvisioningDialog from '../provisioning/components/ConfirmProvisioningDialog.vue';
import { MODAL_CONFIRM } from '@/app/constants/modals';
import { openSafeUrl } from '@/app/utils/htmlUtils';
const i18n = useI18n();
const ssoStore = useSSOStore();
@ -245,7 +246,7 @@ const onTest = async () => {
const url = await ssoStore.testSamlConfig();
if (typeof window !== 'undefined') {
window.open(url, '_blank');
openSafeUrl(url);
}
} catch (error) {
toast.showError(error, 'error');

View File

@ -0,0 +1,30 @@
import { Adalo } from './Adalo.node';
function collectUrls(value: unknown): string[] {
if (Array.isArray(value)) return value.flatMap(collectUrls);
if (value === null || typeof value !== 'object') return [];
return Object.entries(value).flatMap(([key, child]) => {
if (key === 'url' && typeof child === 'string') return [child];
return collectUrls(child);
});
}
describe('Adalo node', () => {
it('encodes every dynamic request URL path segment', () => {
const dynamicUrls = collectUrls(new Adalo().description.properties).filter((url) =>
url.includes('$parameter'),
);
const dynamicExpressions = dynamicUrls.flatMap((url) =>
[...url.matchAll(/{{\s*(.*?)\s*}}/g)]
.map((match) => match[1])
.filter((expression) => expression.includes('$parameter')),
);
expect(dynamicUrls).toHaveLength(5);
expect(dynamicExpressions).toHaveLength(8);
for (const expression of dynamicExpressions) {
expect(expression).toMatch(/^toPathSegment\(.+\)$/);
}
});
});

View File

@ -75,7 +75,7 @@ export class Adalo implements INodeType {
},
request: {
method: 'POST',
url: '=/collections/{{$parameter["collectionId"]}}',
url: '=/collections/{{toPathSegment($parameter["collectionId"])}}',
},
},
action: 'Create a row',
@ -87,7 +87,7 @@ export class Adalo implements INodeType {
routing: {
request: {
method: 'DELETE',
url: '=/collections/{{$parameter["collectionId"]}}/{{$parameter["rowId"]}}',
url: '=/collections/{{toPathSegment($parameter["collectionId"])}}/{{toPathSegment($parameter["rowId"])}}',
},
output: {
postReceive: [
@ -109,7 +109,7 @@ export class Adalo implements INodeType {
routing: {
request: {
method: 'GET',
url: '=/collections/{{$parameter["collectionId"]}}/{{$parameter["rowId"]}}',
url: '=/collections/{{toPathSegment($parameter["collectionId"])}}/{{toPathSegment($parameter["rowId"])}}',
},
},
action: 'Retrieve a row',
@ -121,7 +121,7 @@ export class Adalo implements INodeType {
routing: {
request: {
method: 'GET',
url: '=/collections/{{$parameter["collectionId"]}}',
url: '=/collections/{{toPathSegment($parameter["collectionId"])}}',
qs: {
limit: '={{$parameter["limit"]}}',
},
@ -152,7 +152,7 @@ export class Adalo implements INodeType {
},
request: {
method: 'PUT',
url: '=/collections/{{$parameter["collectionId"]}}/{{$parameter["rowId"]}}',
url: '=/collections/{{toPathSegment($parameter["collectionId"])}}/{{toPathSegment($parameter["rowId"])}}',
},
},
action: 'Update a row',

View File

@ -7,9 +7,7 @@ import type {
INodeType,
INodeTypeDescription,
} from 'n8n-workflow';
import { NodeConnectionTypes, NodeOperationError } from 'n8n-workflow';
import { toPathSegment } from '@utils/url';
import { toPathSegment, NodeConnectionTypes, NodeOperationError } from 'n8n-workflow';
import {
caseCommentFields,

View File

@ -6,9 +6,7 @@ import type {
IRequestOptions,
IHttpRequestMethods,
} from 'n8n-workflow';
import { NodeApiError, NodeOperationError } from 'n8n-workflow';
import { toPathSegment } from '@utils/url';
import { toPathSegment, NodeApiError, NodeOperationError } from 'n8n-workflow';
import type { Connector, ElasticSecurityApiCredentials } from './types';

View File

@ -7,9 +7,7 @@ import type {
INodeTypeDescription,
JsonObject,
} from 'n8n-workflow';
import { NodeConnectionTypes, jsonParse, NodeApiError } from 'n8n-workflow';
import { toPathSegment } from '@utils/url';
import { NodeConnectionTypes, jsonParse, NodeApiError, toPathSegment } from 'n8n-workflow';
import { documentFields, documentOperations, indexFields, indexOperations } from './descriptions';
import {

View File

@ -5,9 +5,7 @@ import type {
IHttpRequestOptions,
IHttpRequestMethods,
} from 'n8n-workflow';
import { NodeApiError } from 'n8n-workflow';
import { toPathSegment } from '@utils/url';
import { toPathSegment, NodeApiError } from 'n8n-workflow';
import type { ElasticsearchApiCredentials } from './types';

View File

@ -27,6 +27,8 @@ const GPG_SSH_DEFAULT_KEY_COMMAND_CONFIG_KEY_PATTERN = /^gpg\.ssh\.defaultkeycom
const URL_REWRITE_CONFIG_KEY_PATTERN = /^url\.(.*)\.(?:insteadof|pushinsteadof)$/i;
const CREDENTIAL_URL_HELPER_CONFIG_KEY_PATTERN = /^credential\.(.*)\.helper$/i;
const KEY_BLACKLIST = [
FILTER_COMMAND_CONFIG_KEY_PATTERN,
MERGE_DRIVER_CONFIG_KEY_PATTERN,
@ -40,6 +42,7 @@ const KEY_BLACKLIST = [
SEQUENCE_EDITOR_CONFIG_KEY_PATTERN,
GPG_SSH_DEFAULT_KEY_COMMAND_CONFIG_KEY_PATTERN,
URL_REWRITE_CONFIG_KEY_PATTERN,
CREDENTIAL_URL_HELPER_CONFIG_KEY_PATTERN,
];
export function findBlacklistedKeys(

View File

@ -830,6 +830,7 @@ export class Git implements INodeType {
logOptions.file = options.file as string;
}
await validateGitConfig();
const log = await git.log(logOptions);
returnItems.push(

View File

@ -11,6 +11,7 @@ import { Git } from '../Git.node';
const mockGit = {
log: jest.fn(),
raw: jest.fn(),
listConfig: jest.fn(),
env: jest.fn().mockReturnThis(),
};
@ -71,6 +72,11 @@ describe('Git Node', () => {
mockGit.raw.mockImplementation(async (args: string[]) =>
args[0] === 'rev-parse' ? revParseOutput : '',
);
mockGit.listConfig.mockResolvedValue({
files: ['.git/config', 'command line:'],
values: { '.git/config': {}, 'command line:': {} },
all: {},
});
gitNode = new Git();
});
@ -84,7 +90,7 @@ describe('Git Node', () => {
expect(mockSimpleGit).toHaveBeenCalledWith(
expect.objectContaining({
config: ['safe.bareRepository=explicit'],
config: expect.arrayContaining(['safe.bareRepository=explicit']),
}),
);
});
@ -97,7 +103,7 @@ describe('Git Node', () => {
expect(mockSimpleGit).toHaveBeenCalledWith(
expect.objectContaining({
config: ['safe.bareRepository=explicit'],
config: expect.arrayContaining(['safe.bareRepository=explicit']),
}),
);
});
@ -110,7 +116,7 @@ describe('Git Node', () => {
expect(mockSimpleGit).toHaveBeenCalledWith(
expect.objectContaining({
config: ['safe.bareRepository=explicit'],
config: expect.arrayContaining(['safe.bareRepository=explicit']),
}),
);
});
@ -121,11 +127,8 @@ describe('Git Node', () => {
await gitNode.execute.call(executeFunctions);
expect(mockSimpleGit).toHaveBeenCalledWith(
expect.objectContaining({
config: [],
}),
);
const options = mockSimpleGit.mock.calls[0][0];
expect(options.config).not.toContain('safe.bareRepository=explicit');
});
});
@ -137,7 +140,7 @@ describe('Git Node', () => {
expect(mockSimpleGit).toHaveBeenCalledWith(
expect.objectContaining({
config: ['core.hooksPath=/dev/null'],
config: expect.arrayContaining(['core.hooksPath=/dev/null']),
}),
);
});
@ -147,11 +150,8 @@ describe('Git Node', () => {
await gitNode.execute.call(executeFunctions);
expect(mockSimpleGit).toHaveBeenCalledWith(
expect.objectContaining({
config: [],
}),
);
const options = mockSimpleGit.mock.calls[0][0];
expect(options.config).not.toContain('core.hooksPath=/dev/null');
});
});

View File

@ -1976,6 +1976,7 @@ describe('Git Node', () => {
'remote.origin.receivepack',
'gpg.openpgp.program',
'gpg.ssh.defaultKeyCommand',
'credential.https://example.invalid.helper',
])("rejects repository Git config key '%s'", async (configKey) => {
mockExecuteFunctions.getNodeParameter
.mockReturnValueOnce('add')
@ -2001,6 +2002,7 @@ describe('Git Node', () => {
{ operation: 'add', params: ['add', '/repo', {}, 'file.txt'], guard: 'add' },
{ operation: 'commit', params: ['commit', '/repo', {}, 'repro commit'], guard: 'commit' },
{ operation: 'fetch', params: ['fetch', '/repo', {}], guard: 'fetch' },
{ operation: 'log', params: ['log', '/repo', {}], guard: 'log' },
{ operation: 'pull', params: ['pull', '/repo', {}], guard: 'pull' },
{ operation: 'push', params: ['push', '/repo', {}], guard: 'push' },
{ operation: 'pushTags', params: ['pushTags', '/repo', {}], guard: 'pushTags' },
@ -2042,7 +2044,9 @@ describe('Git Node', () => {
for (const flag of expectedFlags) {
expect(unsafe?.[flag]).toBeUndefined();
}
expect(mockGit.listConfig).not.toHaveBeenCalled();
// The guard reads the merged config, then returns before the denylist check, so it
// never looks up the repository-local scope.
expect(mockGit.listConfig).not.toHaveBeenCalledWith('local');
});
});
});

View File

@ -0,0 +1,53 @@
import type { INodeTypeBaseDescription } from 'n8n-workflow';
import { HighLevelV1 } from './v1/HighLevelV1.node';
import { HighLevelV2 } from './v2/HighLevelV2.node';
const baseDescription: INodeTypeBaseDescription = {
displayName: 'HighLevel',
name: 'highLevel',
icon: 'file:highLevel.svg',
group: ['transform'],
description: 'Consume HighLevel API',
defaultVersion: 2,
};
function collectUrls(value: unknown): string[] {
if (Array.isArray(value)) return value.flatMap(collectUrls);
if (value === null || typeof value !== 'object') return [];
return Object.entries(value).flatMap(([key, child]) => {
if (key === 'url' && typeof child === 'string') return [child];
return collectUrls(child);
});
}
describe('HighLevel node', () => {
it.each([
{
version: 1,
description: new HighLevelV1(baseDescription).description,
},
{
version: 2,
description: new HighLevelV2(baseDescription).description,
},
])('encodes every dynamic request URL path segment in v$version', ({ description }) => {
const dynamicUrls = collectUrls(description.properties).filter((url) =>
url.includes('$parameter'),
);
expect(dynamicUrls.length).toBeGreaterThan(0);
for (const url of dynamicUrls) {
const dynamicExpressions = [...url.matchAll(/{{\s*(.*?)\s*}}/g)]
.map((match) => match[1])
.filter((expression) => expression.includes('$parameter'));
expect(url.replace(/{{\s*(.*?)\s*}}/g, '')).not.toContain('$parameter');
expect(dynamicExpressions.length).toBeGreaterThan(0);
for (const expression of dynamicExpressions) {
expect(expression).toMatch(/^toPathSegment\(.+\)$/);
}
}
});
});

View File

@ -17,7 +17,7 @@ import type {
IRequestOptions,
IWebhookFunctions,
} from 'n8n-workflow';
import { NodeApiError } from 'n8n-workflow';
import { NodeApiError, toPathSegment } from 'n8n-workflow';
const VALID_EMAIL_REGEX =
/^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
@ -162,7 +162,7 @@ export async function opportunityUpdatePreSendAction(
if (!body.status || !body.title) {
const pipelineId = this.getNodeParameter('pipelineId');
const opportunityId = this.getNodeParameter('opportunityId');
const resource = `/pipelines/${pipelineId}/opportunities/${opportunityId}`;
const resource = `/pipelines/${toPathSegment(pipelineId)}/opportunities/${toPathSegment(opportunityId)}`;
const responseData = await highLevelApiRequest.call(this, 'GET', resource);
body.status = body.status || responseData.status;
body.title = body.title || responseData.name;
@ -179,7 +179,7 @@ export async function taskUpdatePreSendAction(
if (!body.title || !body.dueDate) {
const contactId = this.getNodeParameter('contactId');
const taskId = this.getNodeParameter('taskId');
const resource = `/contacts/${contactId}/tasks/${taskId}`;
const resource = `/contacts/${toPathSegment(contactId)}/tasks/${toPathSegment(taskId)}`;
const responseData = await highLevelApiRequest.call(this, 'GET', resource);
body.title = body.title || responseData.title;
// the api response dueDate has to be formatted or it will error on update

View File

@ -44,7 +44,7 @@ export const contactOperations: INodeProperties[] = [
routing: {
request: {
method: 'DELETE',
url: '=/contacts/{{$parameter.contactId}}',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}',
},
output: {
postReceive: [
@ -65,7 +65,7 @@ export const contactOperations: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/contacts/{{$parameter.contactId}}',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}',
},
output: {
postReceive: [
@ -100,7 +100,7 @@ export const contactOperations: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/contacts/lookup?email={{$parameter.email}}&phone={{$parameter.phone}}',
url: '=/contacts/lookup?email={{toPathSegment($parameter.email)}}&phone={{toPathSegment($parameter.phone)}}',
},
output: {
postReceive: [
@ -121,7 +121,7 @@ export const contactOperations: INodeProperties[] = [
routing: {
request: {
method: 'PUT',
url: '=/contacts/{{$parameter.contactId}}',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}',
},
send: {
preSend: [validEmailAndPhonePreSendAction, splitTagsPreSendAction],

View File

@ -28,7 +28,7 @@ export const opportunityOperations: INodeProperties[] = [
},
request: {
method: 'POST',
url: '=/pipelines/{{$parameter.pipelineId}}/opportunities',
url: '=/pipelines/{{toPathSegment($parameter.pipelineId)}}/opportunities',
},
},
action: 'Create an opportunity',
@ -39,7 +39,7 @@ export const opportunityOperations: INodeProperties[] = [
routing: {
request: {
method: 'DELETE',
url: '=/pipelines/{{$parameter.pipelineId}}/opportunities/{{$parameter.opportunityId}}',
url: '=/pipelines/{{toPathSegment($parameter.pipelineId)}}/opportunities/{{toPathSegment($parameter.opportunityId)}}',
},
output: {
postReceive: [
@ -60,7 +60,7 @@ export const opportunityOperations: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/pipelines/{{$parameter.pipelineId}}/opportunities/{{$parameter.opportunityId}}',
url: '=/pipelines/{{toPathSegment($parameter.pipelineId)}}/opportunities/{{toPathSegment($parameter.opportunityId)}}',
},
},
action: 'Get an opportunity',
@ -71,7 +71,7 @@ export const opportunityOperations: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/pipelines/{{$parameter.pipelineId}}/opportunities',
url: '=/pipelines/{{toPathSegment($parameter.pipelineId)}}/opportunities',
},
send: {
paginate: true,
@ -85,7 +85,7 @@ export const opportunityOperations: INodeProperties[] = [
routing: {
request: {
method: 'PUT',
url: '=/pipelines/{{$parameter.pipelineId}}/opportunities/{{$parameter.opportunityId}}',
url: '=/pipelines/{{toPathSegment($parameter.pipelineId)}}/opportunities/{{toPathSegment($parameter.opportunityId)}}',
},
send: {
preSend: [opportunityUpdatePreSendAction, splitTagsPreSendAction],

View File

@ -24,7 +24,7 @@ export const taskOperations: INodeProperties[] = [
routing: {
request: {
method: 'POST',
url: '=/contacts/{{$parameter.contactId}}/tasks',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}/tasks',
},
output: {
postReceive: [taskPostReceiceAction],
@ -38,7 +38,7 @@ export const taskOperations: INodeProperties[] = [
routing: {
request: {
method: 'DELETE',
url: '=/contacts/{{$parameter.contactId}}/tasks/{{$parameter.taskId}}',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}/tasks/{{toPathSegment($parameter.taskId)}}',
},
output: {
postReceive: [
@ -59,7 +59,7 @@ export const taskOperations: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/contacts/{{$parameter.contactId}}/tasks/{{$parameter.taskId}}',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}/tasks/{{toPathSegment($parameter.taskId)}}',
},
output: {
postReceive: [taskPostReceiceAction],
@ -73,7 +73,7 @@ export const taskOperations: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/contacts/{{$parameter.contactId}}/tasks',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}/tasks',
},
output: {
postReceive: [
@ -95,7 +95,7 @@ export const taskOperations: INodeProperties[] = [
routing: {
request: {
method: 'PUT',
url: '=/contacts/{{$parameter.contactId}}/tasks/{{$parameter.taskId}}',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}/tasks/{{toPathSegment($parameter.taskId)}}',
},
send: {
preSend: [taskUpdatePreSendAction],

View File

@ -0,0 +1,55 @@
import type { IExecuteSingleFunctions, IHttpRequestOptions } from 'n8n-workflow';
import { opportunityUpdatePreSendAction, taskUpdatePreSendAction } from '../GenericFunctions';
describe('HighLevel v1 request paths', () => {
let context: Partial<IExecuteSingleFunctions>;
beforeEach(() => {
context = {
getNodeParameter: jest.fn(),
helpers: {
requestWithAuthentication: jest.fn().mockResolvedValue({}),
} as unknown as IExecuteSingleFunctions['helpers'],
};
});
it('encodes opportunity IDs as URL path segments', async () => {
(context.getNodeParameter as jest.Mock)
.mockReturnValueOnce('pipeline/id?x#y')
.mockReturnValueOnce('opportunity/id');
await opportunityUpdatePreSendAction.call(context as IExecuteSingleFunctions, {
url: 'https://example.com',
body: {},
});
expect(context.helpers?.requestWithAuthentication).toHaveBeenCalledWith(
'highLevelApi',
expect.objectContaining({
uri: 'https://rest.gohighlevel.com/v1/pipelines/pipeline%2Fid%3Fx%23y/opportunities/opportunity%2Fid',
}),
);
});
it('encodes task IDs as URL path segments', async () => {
(context.getNodeParameter as jest.Mock)
.mockReturnValueOnce('contact/id')
.mockReturnValueOnce('task/id?x#y');
await taskUpdatePreSendAction.call(
context as IExecuteSingleFunctions,
{
url: 'https://example.com',
body: {},
} as IHttpRequestOptions,
);
expect(context.helpers?.requestWithAuthentication).toHaveBeenCalledWith(
'highLevelApi',
expect.objectContaining({
uri: 'https://rest.gohighlevel.com/v1/contacts/contact%2Fid/tasks/task%2Fid%3Fx%23y',
}),
);
});
});

View File

@ -16,7 +16,7 @@ import type {
IPollFunctions,
IWebhookFunctions,
} from 'n8n-workflow';
import { ApplicationError, NodeApiError } from 'n8n-workflow';
import { ApplicationError, NodeApiError, toPathSegment } from 'n8n-workflow';
const VALID_EMAIL_REGEX =
/^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
@ -218,7 +218,13 @@ export const addNotePostReceiveAction = async function (
body: note,
};
await highLevelApiRequest.call(this, 'POST', `/contacts/${contactId}/notes`, requestBody, {});
await highLevelApiRequest.call(
this,
'POST',
`/contacts/${toPathSegment(contactId)}/notes`,
requestBody,
{},
);
return items;
};
@ -231,7 +237,7 @@ export async function taskUpdatePreSendAction(
if (!body.title || !body.dueDate) {
const contactId = this.getNodeParameter('contactId');
const taskId = this.getNodeParameter('taskId');
const resource = `/contacts/${contactId}/tasks/${taskId}`;
const resource = `/contacts/${toPathSegment(contactId)}/tasks/${toPathSegment(taskId)}`;
const responseData = await highLevelApiRequest.call(this, 'GET', resource);
body.title = body.title || responseData.title;
// the api response dueDate has to be formatted or it will error on update

View File

@ -8,7 +8,7 @@ import type {
INodeTypeBaseDescription,
INodeTypeDescription,
} from 'n8n-workflow';
import { NodeConnectionTypes } from 'n8n-workflow';
import { NodeConnectionTypes, toPathSegment } from 'n8n-workflow';
import { calendarFields, calendarOperations } from './description/CalendarDescription';
import { contactFields, contactNotes, contactOperations } from './description/ContactDescription';
@ -125,8 +125,7 @@ export class HighLevelV2 implements INodeType {
this,
'highLevelOAuth2Api',
{
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
url: `https://services.leadconnectorhq.com/locations/${locationId}/customFields?model=contact`,
url: `https://services.leadconnectorhq.com/locations/${toPathSegment(locationId)}/customFields?model=contact`,
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
@ -162,8 +161,7 @@ export class HighLevelV2 implements INodeType {
this,
'highLevelOAuth2Api',
{
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
url: `https://services.leadconnectorhq.com/locations/${locationId}/timezones`,
url: `https://services.leadconnectorhq.com/locations/${toPathSegment(locationId)}/timezones`,
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',

View File

@ -29,7 +29,7 @@ export const calendarOperations: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/calendars/{{$parameter.calendarId}}/free-slots',
url: '=/calendars/{{toPathSegment($parameter.calendarId)}}/free-slots',
},
},
},

View File

@ -56,7 +56,7 @@ export const contactOperations: INodeProperties[] = [
routing: {
request: {
method: 'DELETE',
url: '=/contacts/{{$parameter.contactId}}/',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}/',
},
output: {
postReceive: [
@ -77,7 +77,7 @@ export const contactOperations: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/contacts/{{$parameter.contactId}}/',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}/',
},
output: {
postReceive: [
@ -113,7 +113,7 @@ export const contactOperations: INodeProperties[] = [
routing: {
request: {
method: 'PUT',
url: '=/contacts/{{$parameter.contactId}}/',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}/',
},
send: {
preSend: [validEmailAndPhonePreSendAction, splitTagsPreSendAction],

View File

@ -38,7 +38,7 @@ export const opportunityOperations: INodeProperties[] = [
routing: {
request: {
method: 'DELETE',
url: '=/opportunities/{{$parameter.opportunityId}}',
url: '=/opportunities/{{toPathSegment($parameter.opportunityId)}}',
},
output: {
postReceive: [
@ -59,7 +59,7 @@ export const opportunityOperations: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/opportunities/{{$parameter.opportunityId}}',
url: '=/opportunities/{{toPathSegment($parameter.opportunityId)}}',
},
},
action: 'Get an opportunity',
@ -85,7 +85,7 @@ export const opportunityOperations: INodeProperties[] = [
routing: {
request: {
method: 'PUT',
url: '=/opportunities/{{$parameter.opportunityId}}',
url: '=/opportunities/{{toPathSegment($parameter.opportunityId)}}',
},
},
action: 'Update an opportunity',

View File

@ -24,7 +24,7 @@ export const taskOperations: INodeProperties[] = [
routing: {
request: {
method: 'POST',
url: '=/contacts/{{$parameter.contactId}}/tasks/',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}/tasks/',
},
output: {
postReceive: [taskPostReceiceAction],
@ -38,7 +38,7 @@ export const taskOperations: INodeProperties[] = [
routing: {
request: {
method: 'DELETE',
url: '=/contacts/{{$parameter.contactId}}/tasks/{{$parameter.taskId}}/',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}/tasks/{{toPathSegment($parameter.taskId)}}/',
},
output: {
postReceive: [
@ -59,7 +59,7 @@ export const taskOperations: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/contacts/{{$parameter.contactId}}/tasks/{{$parameter.taskId}}/',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}/tasks/{{toPathSegment($parameter.taskId)}}/',
},
output: {
postReceive: [taskPostReceiceAction],
@ -73,7 +73,7 @@ export const taskOperations: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/contacts/{{$parameter.contactId}}/tasks/',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}/tasks/',
},
output: {
postReceive: [
@ -95,7 +95,7 @@ export const taskOperations: INodeProperties[] = [
routing: {
request: {
method: 'PUT',
url: '=/contacts/{{$parameter.contactId}}/tasks/{{$parameter.taskId}}/',
url: '=/contacts/{{toPathSegment($parameter.contactId)}}/tasks/{{toPathSegment($parameter.taskId)}}/',
},
send: {
preSend: [taskUpdatePreSendAction],

View File

@ -1,4 +1,6 @@
import { highLevelApiRequest } from '../GenericFunctions';
import type { IExecuteSingleFunctions, IN8nHttpFullResponse } from 'n8n-workflow';
import { addNotePostReceiveAction, highLevelApiRequest } from '../GenericFunctions';
describe('GenericFunctions - highLevelApiRequest', () => {
let mockContext: any;
@ -114,3 +116,44 @@ describe('GenericFunctions - highLevelApiRequest', () => {
expect(result).toEqual(mockResponse);
});
});
describe('addNotePostReceiveAction', () => {
it('encodes the contact ID as one URL path segment', async () => {
const httpRequestWithAuthentication = jest.fn().mockResolvedValue({});
const context = {
getNodeParameter: jest.fn().mockReturnValue('A note'),
helpers: { httpRequestWithAuthentication },
};
await addNotePostReceiveAction.call(
context as unknown as IExecuteSingleFunctions,
[{ json: {} }],
{
body: { contact: { id: 'contact/id?x#y', locationId: 'location-id' } },
} as IN8nHttpFullResponse,
);
expect(httpRequestWithAuthentication).toHaveBeenCalledWith(
'highLevelOAuth2Api',
expect.objectContaining({
url: 'https://services.leadconnectorhq.com/contacts/contact%2Fid%3Fx%23y/notes',
}),
);
});
it('rejects an invalid contact ID before sending a request', async () => {
const httpRequestWithAuthentication = jest.fn();
const context = {
getNodeParameter: jest.fn().mockReturnValue('A note'),
helpers: { httpRequestWithAuthentication },
};
await expect(
addNotePostReceiveAction.call(context as unknown as IExecuteSingleFunctions, [{ json: {} }], {
body: { contact: { id: '..', locationId: 'location-id' } },
} as IN8nHttpFullResponse),
).rejects.toThrow('Invalid identifier');
expect(httpRequestWithAuthentication).not.toHaveBeenCalled();
});
});

View File

@ -32,7 +32,9 @@ describe('taskUpdatePreSendAction', () => {
});
it('should fetch missing title and dueDate from the API', async () => {
(mockThis.getNodeParameter as jest.Mock).mockReturnValueOnce('123').mockReturnValueOnce('456');
(mockThis.getNodeParameter as jest.Mock)
.mockReturnValueOnce('contact/id?x#y')
.mockReturnValueOnce('task/id');
const mockApiResponse = {
title: 'Fetched Task Title',
@ -60,6 +62,12 @@ describe('taskUpdatePreSendAction', () => {
title: 'Fetched Task Title',
dueDate: '2024-12-25T00:00:00+00:00',
});
expect(mockThis.helpers?.httpRequestWithAuthentication).toHaveBeenCalledWith(
'highLevelOAuth2Api',
expect.objectContaining({
url: 'https://services.leadconnectorhq.com/contacts/contact%2Fid%3Fx%23y/tasks/task%2Fid',
}),
);
});
it('should only fetch title if dueDate is provided', async () => {

View File

@ -33,7 +33,7 @@ export const credentialOperations: INodeProperties[] = [
routing: {
request: {
method: 'DELETE',
url: '=/credentials/{{ $parameter.credentialId }}',
url: '=/credentials/{{ toPathSegment($parameter.credentialId) }}',
},
},
},
@ -44,7 +44,7 @@ export const credentialOperations: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/credentials/schema/{{ $parameter.credentialTypeName }}',
url: '=/credentials/schema/{{ toPathSegment($parameter.credentialTypeName) }}',
},
},
},

View File

@ -23,7 +23,7 @@ export const executionOperations: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/executions/{{ $parameter.executionId }}',
url: '=/executions/{{ toPathSegment($parameter.executionId) }}',
},
},
},
@ -51,7 +51,7 @@ export const executionOperations: INodeProperties[] = [
routing: {
request: {
method: 'DELETE',
url: '=/executions/{{ $parameter.executionId }}',
url: '=/executions/{{ toPathSegment($parameter.executionId) }}',
},
},
},

View File

@ -94,7 +94,7 @@ const activateOperation: INodeProperties[] = [
routing: {
request: {
method: 'POST',
url: '=/workflows/{{ $value }}/activate',
url: '=/workflows/{{ toPathSegment($value) }}/activate',
},
},
},
@ -141,7 +141,7 @@ const deactivateOperation: INodeProperties[] = [
routing: {
request: {
method: 'POST',
url: '=/workflows/{{ $value }}/deactivate',
url: '=/workflows/{{ toPathSegment($value) }}/deactivate',
},
},
},
@ -160,7 +160,7 @@ const deleteOperation: INodeProperties[] = [
routing: {
request: {
method: 'DELETE',
url: '=/workflows/{{ $value }}',
url: '=/workflows/{{ toPathSegment($value) }}',
},
},
},
@ -303,7 +303,7 @@ const getOperation: INodeProperties[] = [
routing: {
request: {
method: 'GET',
url: '=/workflows/{{ $value }}',
url: '=/workflows/{{ toPathSegment($value) }}',
},
},
},
@ -322,7 +322,7 @@ const updateOperation: INodeProperties[] = [
routing: {
request: {
method: 'PUT',
url: '=/workflows/{{ $value }}',
url: '=/workflows/{{ toPathSegment($value) }}',
},
},
},

View File

@ -0,0 +1,32 @@
import { N8n } from '../N8n.node';
function collectUrls(value: unknown): string[] {
if (Array.isArray(value)) return value.flatMap(collectUrls);
if (value === null || typeof value !== 'object') return [];
return Object.entries(value).flatMap(([key, child]) => {
if (key === 'url' && typeof child === 'string') return [child];
return collectUrls(child);
});
}
describe('n8n Node Structure', () => {
it('encodes every dynamic request URL path segment', () => {
const dynamicUrls = collectUrls(new N8n().description.properties).filter(
(url) => url.includes('$value') || url.includes('$parameter'),
);
expect(dynamicUrls.length).toBeGreaterThan(0);
for (const url of dynamicUrls) {
const dynamicExpressions = [...url.matchAll(/{{\s*(.*?)\s*}}/g)]
.map((match) => match[1])
.filter((expression) => expression.includes('$value') || expression.includes('$parameter'));
expect(url.replace(/{{\s*(.*?)\s*}}/g, '')).not.toMatch(/\$(?:value|parameter)/);
expect(dynamicExpressions.length).toBeGreaterThan(0);
for (const expression of dynamicExpressions) {
expect(expression).toMatch(/^toPathSegment\(.+\)$/);
}
}
});
});

View File

@ -5,6 +5,7 @@ import {
type INodeExecutionData,
type INodeType,
type INodeTypeDescription,
toPathSegment,
NodeConnectionTypes,
} from 'n8n-workflow';
@ -198,7 +199,7 @@ export class NotionTrigger implements INodeType {
let { results: data } = await notionApiRequest.call(
this,
'POST',
`/databases/${databaseId}/query`,
`/databases/${toPathSegment(databaseId)}/query`,
body,
{},
'',
@ -221,7 +222,7 @@ export class NotionTrigger implements INodeType {
const { results, has_more, next_cursor } = await notionApiRequest.call(
this,
'POST',
`/databases/${databaseId}/query`,
`/databases/${toPathSegment(databaseId)}/query`,
body,
{},
'',

View File

@ -18,7 +18,7 @@ import type {
IRequestOptions,
JsonObject,
} from 'n8n-workflow';
import { NodeApiError, NodeOperationError, safeRegex } from 'n8n-workflow';
import { toPathSegment, NodeApiError, NodeOperationError, safeRegex } from 'n8n-workflow';
import { validate as uuidValidate } from 'uuid';
import { blockUrlExtractionRegexp, databasePageUrlValidationRegexp } from './constants';
@ -129,7 +129,7 @@ export async function notionApiRequestGetBlockChildrens(
this,
'results',
'GET',
`/blocks/${block.id}/children`,
`/blocks/${toPathSegment(block.id)}/children`,
);
childrens = (childrens || []).map((entry: IDataObject) => ({

View File

@ -9,6 +9,7 @@ import type {
INodeTypeBaseDescription,
INodeTypeDescription,
} from 'n8n-workflow';
import { toPathSegment } from 'n8n-workflow';
import { versionDescription } from './VersionDescription';
import type { SortData } from '../shared/GenericFunctions';
@ -46,7 +47,11 @@ export class NotionV1 implements INodeType {
const databaseId = this.getCurrentNodeParameter('databaseId', {
extractValue: true,
}) as string;
const { properties } = await notionApiRequest.call(this, 'GET', `/databases/${databaseId}`);
const { properties } = await notionApiRequest.call(
this,
'GET',
`/databases/${toPathSegment(databaseId)}`,
);
for (const key of Object.keys(properties as IDataObject)) {
//remove parameters that cannot be set from the API.
if (
@ -82,7 +87,11 @@ export class NotionV1 implements INodeType {
const databaseId = this.getCurrentNodeParameter('databaseId', {
extractValue: true,
}) as string;
const { properties } = await notionApiRequest.call(this, 'GET', `/databases/${databaseId}`);
const { properties } = await notionApiRequest.call(
this,
'GET',
`/databases/${toPathSegment(databaseId)}`,
);
for (const key of Object.keys(properties as IDataObject)) {
returnData.push({
name: `${key} - (${properties[key].type})`,
@ -110,7 +119,11 @@ export class NotionV1 implements INodeType {
}) as string;
const resource = this.getCurrentNodeParameter('resource') as string;
const operation = this.getCurrentNodeParameter('operation') as string;
const { properties } = await notionApiRequest.call(this, 'GET', `/databases/${databaseId}`);
const { properties } = await notionApiRequest.call(
this,
'GET',
`/databases/${toPathSegment(databaseId)}`,
);
if (resource === 'databasePage') {
if (['multi_select', 'select'].includes(type) && operation === 'getAll') {
return properties[name][type].options.map((option: IDataObject) => ({
@ -149,8 +162,12 @@ export class NotionV1 implements INodeType {
);
const {
parent: { database_id: databaseId },
} = await notionApiRequest.call(this, 'GET', `/pages/${pageId}`);
const { properties } = await notionApiRequest.call(this, 'GET', `/databases/${databaseId}`);
} = await notionApiRequest.call(this, 'GET', `/pages/${toPathSegment(pageId)}`);
const { properties } = await notionApiRequest.call(
this,
'GET',
`/databases/${toPathSegment(databaseId)}`,
);
for (const key of Object.keys(properties as IDataObject)) {
//remove parameters that cannot be set from the API.
if (
@ -190,8 +207,12 @@ export class NotionV1 implements INodeType {
const [name, type] = (this.getCurrentNodeParameter('&key') as string).split('|');
const {
parent: { database_id: databaseId },
} = await notionApiRequest.call(this, 'GET', `/pages/${pageId}`);
const { properties } = await notionApiRequest.call(this, 'GET', `/databases/${databaseId}`);
} = await notionApiRequest.call(this, 'GET', `/pages/${toPathSegment(pageId)}`);
const { properties } = await notionApiRequest.call(
this,
'GET',
`/databases/${toPathSegment(databaseId)}`,
);
return properties[name][type].options.map((option: IDataObject) => ({
name: option.name,
value: option.id,
@ -245,7 +266,7 @@ export class NotionV1 implements INodeType {
const block = await notionApiRequest.call(
this,
'PATCH',
`/blocks/${blockId}/children`,
`/blocks/${toPathSegment(blockId)}/children`,
body,
);
@ -268,7 +289,7 @@ export class NotionV1 implements INodeType {
this,
'results',
'GET',
`/blocks/${blockId}/children`,
`/blocks/${toPathSegment(blockId)}/children`,
{},
);
} else {
@ -276,7 +297,7 @@ export class NotionV1 implements INodeType {
responseData = await notionApiRequest.call(
this,
'GET',
`/blocks/${blockId}/children`,
`/blocks/${toPathSegment(blockId)}/children`,
{},
qs,
);
@ -298,7 +319,11 @@ export class NotionV1 implements INodeType {
const databaseId = extractDatabaseId(
this.getNodeParameter('databaseId', i, '', { extractValue: true }) as string,
);
responseData = await notionApiRequest.call(this, 'GET', `/databases/${databaseId}`);
responseData = await notionApiRequest.call(
this,
'GET',
`/databases/${toPathSegment(databaseId)}`,
);
const executionData = this.helpers.constructExecutionMetaData(
this.helpers.returnJsonArray(responseData as IDataObject),
@ -421,7 +446,7 @@ export class NotionV1 implements INodeType {
this,
'results',
'POST',
`/databases/${databaseId}/query`,
`/databases/${toPathSegment(databaseId)}/query`,
body,
{},
);
@ -430,7 +455,7 @@ export class NotionV1 implements INodeType {
responseData = await notionApiRequest.call(
this,
'POST',
`/databases/${databaseId}/query`,
`/databases/${toPathSegment(databaseId)}/query`,
body,
qs,
);
@ -466,7 +491,12 @@ export class NotionV1 implements INodeType {
if (properties.length !== 0) {
body.properties = mapProperties.call(this, properties, timezone) as IDataObject;
}
responseData = await notionApiRequest.call(this, 'PATCH', `/pages/${pageId}`, body);
responseData = await notionApiRequest.call(
this,
'PATCH',
`/pages/${toPathSegment(pageId)}`,
body,
);
if (simple) {
responseData = simplifyObjects(responseData, false, 1);
}
@ -484,7 +514,11 @@ export class NotionV1 implements INodeType {
if (operation === 'get') {
for (let i = 0; i < length; i++) {
const userId = this.getNodeParameter('userId', i) as string;
responseData = await notionApiRequest.call(this, 'GET', `/users/${userId}`);
responseData = await notionApiRequest.call(
this,
'GET',
`/users/${toPathSegment(userId)}`,
);
const executionData = this.helpers.constructExecutionMetaData(
this.helpers.returnJsonArray(responseData as IDataObject),
@ -555,7 +589,11 @@ export class NotionV1 implements INodeType {
for (let i = 0; i < length; i++) {
const pageId = extractPageId(this.getNodeParameter('pageId', i) as string);
const simple = this.getNodeParameter('simple', i) as boolean;
responseData = await notionApiRequest.call(this, 'GET', `/pages/${pageId}`);
responseData = await notionApiRequest.call(
this,
'GET',
`/pages/${toPathSegment(pageId)}`,
);
if (simple) {
responseData = simplifyObjects(responseData, false, 1);
}

View File

@ -6,7 +6,7 @@ import type {
INodeTypeBaseDescription,
INodeTypeDescription,
} from 'n8n-workflow';
import { jsonParse, NodeApiError } from 'n8n-workflow';
import { toPathSegment, jsonParse, NodeApiError } from 'n8n-workflow';
import { loadOptions } from './methods';
import { versionDescription } from './VersionDescription';
@ -75,7 +75,7 @@ export class NotionV2 implements INodeType {
const block = await notionApiRequest.call(
this,
'PATCH',
`/blocks/${blockId}/children`,
`/blocks/${toPathSegment(blockId)}/children`,
body,
);
@ -109,7 +109,7 @@ export class NotionV2 implements INodeType {
this,
'results',
'GET',
`/blocks/${blockId}/children`,
`/blocks/${toPathSegment(blockId)}/children`,
{},
);
@ -122,7 +122,7 @@ export class NotionV2 implements INodeType {
responseData = await notionApiRequest.call(
this,
'GET',
`/blocks/${blockId}/children`,
`/blocks/${toPathSegment(blockId)}/children`,
{},
qs,
);
@ -181,7 +181,11 @@ export class NotionV2 implements INodeType {
const databaseId = extractDatabaseId(
this.getNodeParameter('databaseId', i, '', { extractValue: true }) as string,
);
responseData = await notionApiRequest.call(this, 'GET', `/databases/${databaseId}`);
responseData = await notionApiRequest.call(
this,
'GET',
`/databases/${toPathSegment(databaseId)}`,
);
if (simple) {
responseData = simplifyObjects(responseData, download)[0];
}
@ -316,7 +320,11 @@ export class NotionV2 implements INodeType {
const databaseId = this.getNodeParameter('databaseId', 0, '', {
extractValue: true,
}) as string;
const { properties } = await notionApiRequest.call(this, 'GET', `/databases/${databaseId}`);
const { properties } = await notionApiRequest.call(
this,
'GET',
`/databases/${toPathSegment(databaseId)}`,
);
let titleKey = '';
for (const key of Object.keys(properties as IDataObject)) {
if (properties[key].type === 'title') {
@ -403,7 +411,11 @@ export class NotionV2 implements INodeType {
const pageId = getPageId.call(this, i);
const simple = this.getNodeParameter('simple', i) as boolean;
responseData = await notionApiRequest.call(this, 'GET', `/pages/${pageId}`);
responseData = await notionApiRequest.call(
this,
'GET',
`/pages/${toPathSegment(pageId)}`,
);
if (simple) {
responseData = simplifyObjects(responseData, download);
}
@ -479,7 +491,7 @@ export class NotionV2 implements INodeType {
this,
'results',
'POST',
`/databases/${databaseId}/query`,
`/databases/${toPathSegment(databaseId)}/query`,
body,
{},
);
@ -488,7 +500,7 @@ export class NotionV2 implements INodeType {
responseData = await notionApiRequest.call(
this,
'POST',
`/databases/${databaseId}/query`,
`/databases/${toPathSegment(databaseId)}/query`,
body,
qs,
);
@ -548,7 +560,12 @@ export class NotionV2 implements INodeType {
}
}
responseData = await notionApiRequest.call(this, 'PATCH', `/pages/${pageId}`, body);
responseData = await notionApiRequest.call(
this,
'PATCH',
`/pages/${toPathSegment(pageId)}`,
body,
);
if (simple) {
responseData = simplifyObjects(responseData, false);
}
@ -577,7 +594,11 @@ export class NotionV2 implements INodeType {
for (let i = 0; i < itemsLength; i++) {
try {
const userId = this.getNodeParameter('userId', i) as string;
responseData = await notionApiRequest.call(this, 'GET', `/users/${userId}`);
responseData = await notionApiRequest.call(
this,
'GET',
`/users/${toPathSegment(userId)}`,
);
const executionData = this.helpers.constructExecutionMetaData(
this.helpers.returnJsonArray(responseData as IDataObject),
@ -633,9 +654,14 @@ export class NotionV2 implements INodeType {
try {
const pageId = getPageId.call(this, i);
const simple = this.getNodeParameter('simple', i) as boolean;
responseData = await notionApiRequest.call(this, 'PATCH', `/pages/${pageId}`, {
archived: true,
});
responseData = await notionApiRequest.call(
this,
'PATCH',
`/pages/${toPathSegment(pageId)}`,
{
archived: true,
},
);
if (simple) {
responseData = simplifyObjects(responseData, download);
}

View File

@ -1,5 +1,6 @@
import moment from 'moment-timezone';
import type { IDataObject, ILoadOptionsFunctions, INodePropertyOptions } from 'n8n-workflow';
import { toPathSegment } from 'n8n-workflow';
import {
extractPageId,
@ -15,7 +16,11 @@ export async function getDatabaseProperties(
const databaseId = this.getCurrentNodeParameter('databaseId', {
extractValue: true,
}) as string;
const { properties } = await notionApiRequest.call(this, 'GET', `/databases/${databaseId}`);
const { properties } = await notionApiRequest.call(
this,
'GET',
`/databases/${toPathSegment(databaseId)}`,
);
for (const key of Object.keys(properties as IDataObject)) {
//remove parameters that cannot be set from the API.
if (
@ -53,7 +58,11 @@ export async function getFilterProperties(
const databaseId = this.getCurrentNodeParameter('databaseId', {
extractValue: true,
}) as string;
const { properties } = await notionApiRequest.call(this, 'GET', `/databases/${databaseId}`);
const { properties } = await notionApiRequest.call(
this,
'GET',
`/databases/${toPathSegment(databaseId)}`,
);
for (const key of Object.keys(properties as IDataObject)) {
returnData.push({
name: `${key}`,
@ -85,7 +94,11 @@ export async function getPropertySelectValues(
}) as string;
const resource = this.getCurrentNodeParameter('resource') as string;
const operation = this.getCurrentNodeParameter('operation') as string;
const { properties } = await notionApiRequest.call(this, 'GET', `/databases/${databaseId}`);
const { properties } = await notionApiRequest.call(
this,
'GET',
`/databases/${toPathSegment(databaseId)}`,
);
if (resource === 'databasePage') {
if (['multi_select', 'select', 'status'].includes(type) && operation === 'getAll') {
return properties[name][type].options.map((option: IDataObject) => ({
@ -131,8 +144,12 @@ export async function getDatabaseIdFromPage(
);
const {
parent: { database_id: databaseId },
} = await notionApiRequest.call(this, 'GET', `/pages/${pageId}`);
const { properties } = await notionApiRequest.call(this, 'GET', `/databases/${databaseId}`);
} = await notionApiRequest.call(this, 'GET', `/pages/${toPathSegment(pageId)}`);
const { properties } = await notionApiRequest.call(
this,
'GET',
`/databases/${toPathSegment(databaseId)}`,
);
for (const key of Object.keys(properties as IDataObject)) {
//remove parameters that cannot be set from the API.
if (
@ -172,8 +189,12 @@ export async function getDatabaseOptionsFromPage(
const [name, type] = (this.getCurrentNodeParameter('&key') as string).split('|');
const {
parent: { database_id: databaseId },
} = await notionApiRequest.call(this, 'GET', `/pages/${pageId}`);
const { properties } = await notionApiRequest.call(this, 'GET', `/databases/${databaseId}`);
} = await notionApiRequest.call(this, 'GET', `/pages/${toPathSegment(pageId)}`);
const { properties } = await notionApiRequest.call(
this,
'GET',
`/databases/${toPathSegment(databaseId)}`,
);
return properties[name][type].options.map((option: IDataObject) => ({
name: option.name,
value: option.name,

View File

@ -1,4 +1,4 @@
import type { INodeProperties } from 'n8n-workflow';
import type { INodeProperties, INodePropertyOptions } from 'n8n-workflow';
const stmtBatchOptions = [
{
@ -381,6 +381,47 @@ export const tableRLC: INodeProperties = {
],
};
// Single source of truth for the WHERE Operator dropdown; the query builder derives
// its allow-list from this so adding an operator here also permits it at runtime.
export const operatorOptions: INodePropertyOptions[] = [
{
name: 'Equal',
value: 'equal',
},
{
name: 'Not Equal',
value: '!=',
},
{
name: 'Like',
value: 'LIKE',
},
{
name: 'Greater Than',
value: '>',
},
{
name: 'Less Than',
value: '<',
},
{
name: 'Greater Than Or Equal',
value: '>=',
},
{
name: 'Less Than Or Equal',
value: '<=',
},
{
name: 'Is Null',
value: 'IS NULL',
},
{
name: 'Is Not Null',
value: 'IS NOT NULL',
},
];
export const whereFixedCollection: INodeProperties = {
displayName: 'Select Rows',
name: 'where',
@ -417,45 +458,7 @@ export const whereFixedCollection: INodeProperties = {
type: 'options',
description:
"The operator to check the column against. When using 'LIKE' operator percent sign ( %) matches zero or more characters, underscore ( _ ) matches any single character.",
// eslint-disable-next-line n8n-nodes-base/node-param-options-type-unsorted-items
options: [
{
name: 'Equal',
value: 'equal',
},
{
name: 'Not Equal',
value: '!=',
},
{
name: 'Like',
value: 'LIKE',
},
{
name: 'Greater Than',
value: '>',
},
{
name: 'Less Than',
value: '<',
},
{
name: 'Greater Than Or Equal',
value: '>=',
},
{
name: 'Less Than Or Equal',
value: '<=',
},
{
name: 'Is Null',
value: 'IS NULL',
},
{
name: 'Is Not Null',
value: 'IS NOT NULL',
},
],
options: operatorOptions,
default: 'equal',
},
{

View File

@ -17,6 +17,7 @@ import type {
} from '../../helpers/interfaces';
import {
quoteSqlIdentifier,
escapeSqlStringLiteral,
addWhereClauses,
getColumnMetaData,
getColumnMap,
@ -111,11 +112,13 @@ export async function execute(
const quotedTableName = quoteSqlIdentifier(schema) + '.' + quoteSqlIdentifier(table);
if (deleteCommand === 'drop') {
// Embedded in a single-quoted PL/SQL literal, so ' in the identifier must be doubled.
const dropTableName = escapeSqlStringLiteral(quotedTableName);
query = `DECLARE
e_table_missing EXCEPTION;
PRAGMA EXCEPTION_INIT(e_table_missing, -942);
BEGIN
EXECUTE IMMEDIATE ('DROP TABLE ${quotedTableName} PURGE');
EXECUTE IMMEDIATE ('DROP TABLE ${dropTableName} PURGE');
EXCEPTION
WHEN e_table_missing THEN NULL;
END;`;
@ -133,6 +136,8 @@ export async function execute(
whereClauses,
combineConditions,
columnMetaDataObject,
this.getNode(),
i,
);
} else {
throw new NodeOperationError(
@ -178,6 +183,8 @@ export async function execute(
whereClauses,
combineConditions,
columnMetaDataObject,
this.getNode(),
0,
true,
);

View File

@ -112,7 +112,14 @@ export async function execute(
const whereClauses =
((this.getNodeParameter('where', i, []) as IDataObject).values as WhereClause[]) || [];
const combineConditions = this.getNodeParameter('combineConditions', i, 'AND') as string;
[query, values] = addWhereClauses(query, whereClauses, combineConditions, columnMetaDataObject);
[query, values] = addWhereClauses(
query,
whereClauses,
combineConditions,
columnMetaDataObject,
this.getNode(),
i,
);
const sortRules =
((this.getNodeParameter('sort', i, []) as IDataObject).values as SortRule[]) || [];

View File

@ -25,6 +25,7 @@ import type {
ColumnMap,
OracleDBNodeOptions,
} from './interfaces';
import { operatorOptions } from '../actions/common.descriptions';
const n8nTypetoDBType: { [key: string]: oracledb.DbType } = {
boolean: oracledb.DB_TYPE_BOOLEAN,
@ -197,6 +198,10 @@ export function quoteSqlIdentifier(name: string): string {
* Invalid examples:
* my"column → contains an illegal ".
* my..column → double dot not allowed.
*
* A single quote is legal here: Oracle double-quoted identifiers may contain
* any character except " (e.g. "O'Brien"). Callers that embed the result in a
* string literal (the Drop path) must escape it via escapeSqlStringLiteral.
*/
const validateRegex = /^(?:"[^"]+"|[^".]+)(?:\.(?:"[^"]+"|[^".]+))*$/;
if (!validateRegex.test(name)) {
@ -217,6 +222,25 @@ export function quoteSqlIdentifier(name: string): string {
return quotedParts.join('.');
}
// Escapes a value for embedding inside an Oracle single-quoted text literal.
// Doubling ' is Oracle's only literal escape (no backslash processing), so a
// quoted identifier such as "O'Brien" cannot terminate the surrounding literal.
export function escapeSqlStringLiteral(value: string): string {
return value.replace(/'/g, "''");
}
// Operators are concatenated into the WHERE clause, so only a fixed set is allowed.
// Derived from the Operator dropdown (so a new dropdown entry is permitted automatically),
// plus the case/synonym variants Oracle accepts that arrive via an expression-driven value.
const VALID_WHERE_OPERATORS = new Set<string>([
...operatorOptions.map((option) =>
option.value === 'equal' ? '=' : String(option.value).toUpperCase(),
),
'<>',
'^=',
'NOT LIKE',
]);
export function addSortRules(query: string, rules: SortRule[]): string {
if (rules.length === 0) return query;
@ -654,6 +678,8 @@ export function addWhereClauses(
clauses: WhereClause[],
combineConditions: string,
schema: ColumnMap,
node: INode,
itemIndex: number,
isExecuteMany: boolean = false,
): [string, oracledb.BindParameter[] | oracledb.BindDefinition[]] {
if (clauses.length === 0) return [query, []];
@ -672,6 +698,22 @@ export function addWhereClauses(
clause.condition = '=';
}
// The operator can be expression-driven, so accept the case/whitespace variants
// Oracle itself tolerates, then check against the fixed set it is concatenated from.
// An expression can also yield a non-string; route that to the invalid-operator error.
const normalizedCondition =
typeof clause.condition === 'string'
? clause.condition.trim().replace(/\s+/g, ' ').toUpperCase()
: '';
if (!VALID_WHERE_OPERATORS.has(normalizedCondition)) {
throw new NodeOperationError(
node,
`Operator "${clause.condition}" is not valid. Supported operators: ${[...VALID_WHERE_OPERATORS].join(', ')}`,
{ itemIndex },
);
}
clause.condition = normalizedCondition;
// The condition value is json type, so convert to required type only
// if fixed expression is used instead of n8n expressions.
if (typeof clause.value === 'string') {

View File

@ -681,6 +681,34 @@ VALUES (
expect(runQueries).toHaveBeenCalledWith(queries, emptyInputItems, nodeOptions);
},
);
it('should neutralise an injected Drop table name by escaping the string literal', async () => {
const nodeParameters: IDataObject = {
operation: 'deleteTable',
schema: { __rl: true, mode: 'list', value: CONFIG.user },
table: {
__rl: true,
mode: 'list',
value: "X') PURGE; EXECUTE IMMEDIATE 'DROP TABLE SENSITIVE_DATA",
},
deleteCommand: 'drop',
options: {},
};
const mockThis = createMockExecuteFunction(nodeParameters);
const items = [{ json: {}, pairedItem: { item: 0, input: undefined } }];
const runQueries = getRunQueriesFn(mockThis, pool);
await deleteTable.execute.call(mockThis, runQueries, items, {}, pool);
expect(runQueries).toHaveBeenCalledTimes(1);
const { query } = runQueries.mock.calls[0][0][0];
// The injected quotes are doubled, so they stay inside the identifier and
// cannot terminate the EXECUTE IMMEDIATE literal.
expect(query).toContain(
".\"X'') PURGE; EXECUTE IMMEDIATE ''DROP TABLE SENSITIVE_DATA\" PURGE')",
);
expect(query).not.toContain("X') PURGE");
});
});
describe('Test execute operation', () => {

View File

@ -1,14 +1,145 @@
import { mock } from 'jest-mock-extended';
import { DateTime } from 'luxon';
import type { INode } from 'n8n-workflow';
import * as oracleDBTypes from 'oracledb';
import type { ExecuteOpBindParam } from '../helpers/interfaces';
import type { ColumnMap, ExecuteOpBindParam } from '../helpers/interfaces';
import {
addSortRules,
addWhereClauses,
escapeSqlStringLiteral,
getBindParameters,
getCompatibleValue,
getOutBindDefsForExecute,
quoteSqlIdentifier,
} from '../helpers/utils';
describe('Test addWhereClauses', () => {
const schema: ColumnMap = { ID: { type: 'NUMBER', nullable: true, maxSize: 0 } };
const node = mock<INode>();
const build = (condition: string, value: unknown = 1) =>
addWhereClauses(
'DELETE FROM "T"',
[{ column: 'ID', condition, value }],
'AND',
schema,
node,
0,
);
it('should normalise the "equal" operator to =', () => {
const [query] = build('equal');
expect(query).toEqual('DELETE FROM "T" WHERE "ID" = :0');
});
it('should keep the column and value out of the concatenated SQL', () => {
const [query, binds] = build('>', 5);
expect(query).toEqual('DELETE FROM "T" WHERE "ID" > :0');
expect(binds).toHaveLength(1);
});
it('should build value-less clauses for IS NULL / IS NOT NULL', () => {
const [query, binds] = build('IS NULL', undefined);
expect(query).toEqual('DELETE FROM "T" WHERE "ID" IS NULL');
expect(binds).toHaveLength(0);
});
it.each(['<>', '^=', 'NOT LIKE'])('should accept the %s operator synonym', (condition) => {
const [query] = build(condition);
expect(query).toEqual(`DELETE FROM "T" WHERE "ID" ${condition} :0`);
});
it('should accept a lowercase operator, normalising it to uppercase', () => {
const [query] = build('like');
expect(query).toEqual('DELETE FROM "T" WHERE "ID" LIKE :0');
});
it('should accept a lowercase "is null" and build a value-less clause', () => {
const [query, binds] = build('is null', undefined);
expect(query).toEqual('DELETE FROM "T" WHERE "ID" IS NULL');
expect(binds).toHaveLength(0);
});
it('should tolerate surrounding whitespace in the operator', () => {
const [query] = build(' = ');
expect(query).toEqual('DELETE FROM "T" WHERE "ID" = :0');
});
it('should reject an operator outside the allowed set', () => {
expect(() => build("= 'x' OR 1=1 --")).toThrow('is not valid');
});
it('should reject an empty operator', () => {
expect(() => build('')).toThrow('is not valid');
});
it.each([5, null, { op: '=' }])(
'should reject the non-string operator %o with the operator error',
(condition) => {
expect(() => build(condition as unknown as string)).toThrow('is not valid');
},
);
});
describe('Test quoteSqlIdentifier', () => {
it('should wrap a simple identifier in double quotes', () => {
expect(quoteSqlIdentifier('employees')).toEqual('"employees"');
});
it('should quote each part of a dot-separated identifier', () => {
expect(quoteSqlIdentifier('scott.employees')).toEqual('"scott"."employees"');
});
it('should preserve an already-quoted identifier', () => {
expect(quoteSqlIdentifier('"My Table"')).toEqual('"My Table"');
});
it('should trim surrounding whitespace before quoting', () => {
expect(quoteSqlIdentifier(' employees ')).toEqual('"employees"');
});
it('should reject an identifier containing a double quote', () => {
expect(() => quoteSqlIdentifier('my"table')).toThrow('is not valid');
});
it('should accept a single quote, which is legal in an Oracle identifier', () => {
expect(quoteSqlIdentifier("O'Brien")).toEqual('"O\'Brien"');
});
it('should preserve a single quote inside an already-quoted identifier', () => {
expect(quoteSqlIdentifier('"O\'Brien"')).toEqual('"O\'Brien"');
});
it('should quote (not reject) a value crafted to close a string literal', () => {
// The literal-context safety comes from escapeSqlStringLiteral, not from rejection here.
expect(quoteSqlIdentifier("X') PURGE; EXECUTE IMMEDIATE 'DROP TABLE SENSITIVE_DATA")).toEqual(
'"X\') PURGE; EXECUTE IMMEDIATE \'DROP TABLE SENSITIVE_DATA"',
);
});
it('should reject a double-dot identifier', () => {
expect(() => quoteSqlIdentifier('my..column')).toThrow('is not valid');
});
});
describe('Test escapeSqlStringLiteral', () => {
it('should double a single quote so it cannot terminate a literal', () => {
expect(escapeSqlStringLiteral('"O\'Brien"')).toEqual('"O\'\'Brien"');
});
it('should leave a value without single quotes unchanged', () => {
expect(escapeSqlStringLiteral('"scott"."employees"')).toEqual('"scott"."employees"');
});
it('should neutralise an identifier crafted to close the DROP literal', () => {
const quoted = quoteSqlIdentifier("X') PURGE; EXECUTE IMMEDIATE 'DROP TABLE SENSITIVE_DATA");
expect(escapeSqlStringLiteral(quoted)).toEqual(
"\"X'') PURGE; EXECUTE IMMEDIATE ''DROP TABLE SENSITIVE_DATA\"",
);
});
});
describe('Test addSortRules', () => {
it('should ORDER BY ASC', () => {
const query = 'SELECT * FROM "scott"."employees"';

View File

@ -302,6 +302,39 @@ export function getFilters(
},
default: '',
placeholder: 'name=eq.jhon',
hint: 'Use $1, $2, etc. and the parameters below to reference dynamic values, rather than building this string with an expression, to avoid PostgREST filter injection',
},
{
displayName: 'Filters (String) Parameters',
name: 'filterStringParameters',
type: 'fixedCollection',
typeOptions: {
multipleValues: true,
},
displayOptions: {
show: {
resource: resources,
operation: operations,
filterType: ['string'],
},
},
default: {},
placeholder: 'Add Parameter',
options: [
{
displayName: 'Values',
name: 'values',
values: [
{
displayName: 'Value',
name: 'value',
type: 'string',
default: '',
},
],
},
],
description: 'Values to substitute for $1, $2, etc. in the filter string above.',
},
];
}
@ -363,6 +396,37 @@ export const buildOrQuery = (value: IDataObject) =>
export const buildGetQuery = (query: Map<string, string>, value: IDataObject) =>
query.set(quotePostgrestComponent(value.keyName), `eq.${String(value.keyValue)}`);
export function applyFilterStringParameters(filterString: string, parameters: IDataObject[]) {
if (parameters.length === 0) return filterString;
return filterString.replace(/\$(\d+)/g, (_match, index: string) => {
const position = Number(index) - 1;
if (position < 0 || position >= parameters.length) {
throw new UserError(
`Filters (String) references parameter $${index}, but only ${parameters.length} parameter(s) were provided`,
);
}
return encodeURIComponent(quotePostgrestComponent(parameters[position].value));
});
}
export function appendFilterStringToEndpoint(
context: IExecuteFunctions,
endpoint: string,
itemIndex: number,
) {
const filterString = context.getNodeParameter('filterString', itemIndex) as string;
const filterStringParameters = context.getNodeParameter(
'filterStringParameters.values',
itemIndex,
[],
) as IDataObject[];
const encodedTemplate = encodeURI(filterString);
return `${endpoint}?${applyFilterStringParameters(encodedTemplate, filterStringParameters)}`;
}
export async function validateCredentials(
this: ICredentialTestFunctions,
decryptedCredentials: ICredentialDataDecryptedObject,

View File

@ -11,9 +11,10 @@ import type {
INodeType,
INodeTypeDescription,
} from 'n8n-workflow';
import { NodeConnectionTypes, NodeOperationError } from 'n8n-workflow';
import { NodeConnectionTypes, NodeOperationError, toPathSegment } from 'n8n-workflow';
import {
appendFilterStringToEndpoint,
buildGetQuery,
buildOrQuery,
buildQuery,
@ -175,7 +176,7 @@ export class Supabase implements INodeType {
const operation = this.getNodeParameter('operation', 0);
if (resource === 'row') {
const tableId = this.getNodeParameter('tableId', 0) as string;
const tableId = toPathSegment(this.getNodeParameter('tableId', 0));
if (operation === 'create') {
const records: IDataObject[] = [];
@ -265,8 +266,7 @@ export class Supabase implements INodeType {
}
if (filterType === 'string') {
const filterString = this.getNodeParameter('filterString', i) as string;
endpoint = `${endpoint}?${encodeURI(filterString)}`;
endpoint = appendFilterStringToEndpoint(this, endpoint, i);
}
let rows;
@ -367,8 +367,7 @@ export class Supabase implements INodeType {
}
if (filterType === 'string') {
const filterString = this.getNodeParameter('filterString', i) as string;
endpoint = `${endpoint}?${encodeURI(filterString)}`;
endpoint = appendFilterStringToEndpoint(this, endpoint, i);
}
if (!returnAll) {
@ -442,8 +441,7 @@ export class Supabase implements INodeType {
}
if (filterType === 'string') {
const filterString = this.getNodeParameter('filterString', i) as string;
endpoint = `${endpoint}?${encodeURI(filterString)}`;
endpoint = appendFilterStringToEndpoint(this, endpoint, i);
}
const record: IDataObject = {};

View File

@ -7,6 +7,7 @@ import {
type INodeExecutionData,
type IPairedItemData,
NodeOperationError,
UserError,
} from 'n8n-workflow';
import * as utils from '../GenericFunctions';
@ -135,6 +136,102 @@ describe('Test Supabase Node', () => {
});
});
describe('filter string parameters', () => {
it('should substitute $1 with a quoted value when it contains reserved characters', () => {
const resolved = utils.applyFilterStringParameters('or=(email.eq.$1)', [
{ value: 'nobody@x.com,id.gt.0' },
]);
expect(decodeURIComponent(resolved)).toBe('or=(email.eq."nobody@x.com,id.gt.0")');
});
it('should leave a plain substituted value unquoted', () => {
expect(utils.applyFilterStringParameters('id.eq.$1', [{ value: '42' }])).toBe('id.eq.42');
});
it('should substitute multiple parameters by position', () => {
const resolved = utils.applyFilterStringParameters('or=(a.eq.$1,b.eq.$2)', [
{ value: 'x,y' },
{ value: 'z' },
]);
expect(decodeURIComponent(resolved)).toBe('or=(a.eq."x,y",b.eq.z)');
});
it('should throw when the filter string references a parameter that was not provided', () => {
expect(() => utils.applyFilterStringParameters('email.eq.$2', [{ value: 'x' }])).toThrow(
'Filters (String) references parameter $2',
);
});
it('should leave the filter string untouched when no parameters are provided, even with a literal $N in it', () => {
expect(utils.applyFilterStringParameters('price.eq.$50', [])).toBe('price.eq.$50');
});
it('should escape & and = in a substituted value so it cannot open a new top-level query parameter', () => {
const resolved = utils.applyFilterStringParameters('or=(email.eq.$1)', [
{ value: 'x@y.com&select=*' },
]);
expect(Array.from(new URLSearchParams(resolved).keys())).toEqual(['or']);
});
it.each(['getAll', 'delete', 'update'])(
'should escape an injected parameter value for %s so it cannot append an extra condition',
async (operation) => {
const supabaseApiRequest = jest.spyOn(utils, 'supabaseApiRequest').mockResolvedValue([]);
const fakeExecuteFunction = createMockExecuteFunction({
resource: 'row',
operation,
returnAll: true,
filterType: 'string',
filterString: 'or=(email.eq.$1)',
filterStringParameters: { values: [{ value: 'nobody@x.com,id.gt.0' }] },
tableId: 'my_table',
dataToSend: 'defineBelow',
fieldsUi: { fieldValues: [] },
});
await node.execute.call(fakeExecuteFunction);
// the substituted value is percent-encoded (decodeURI alone leaves
// reserved-character escapes like %40/%2C untouched); the receiving
// server decodes it the same way before parsing the filter.
const endpointArg = supabaseApiRequest.mock.calls[0][1] as string;
expect(decodeURIComponent(endpointArg)).toBe(
'/my_table?or=(email.eq."nobody@x.com,id.gt.0")',
);
supabaseApiRequest.mockRestore();
},
);
it.each(['getAll', 'delete', 'update'])(
"should not swallow a $N parameter mismatch under continueOnFail for %s, matching manual mode's unconditional validation errors",
async (operation) => {
const fakeExecuteFunction = createMockExecuteFunction(
{
resource: 'row',
operation,
returnAll: true,
filterType: 'string',
filterString: 'email.eq.$2',
filterStringParameters: { values: [{ value: 'x' }] },
tableId: 'my_table',
dataToSend: 'defineBelow',
fieldsUi: { fieldValues: [] },
},
true,
);
await expect(node.execute.call(fakeExecuteFunction)).rejects.toThrow(
'Filters (String) references parameter $2',
);
},
);
});
it('should allow filtering on the same field multiple times', async () => {
const supabaseApiRequest = jest
.spyOn(utils, 'supabaseApiRequest')
@ -285,6 +382,92 @@ describe('Test Supabase Node', () => {
);
});
describe('table name path segment', () => {
const encodedNames = [
['a/b', '/a%2Fb'],
['a\\b', '/a%5Cb'],
['a?b=c', '/a%3Fb%3Dc'],
['a#b', '/a%23b'],
['../a', '/..%2Fa'],
['my table', '/my%20table'],
];
it.each(encodedNames)(
'should send the table name %s as a single path segment',
async (tableId, expectedEndpoint) => {
const supabaseApiRequest = jest
.spyOn(utils, 'supabaseApiRequest')
.mockResolvedValueOnce([]);
const fakeExecuteFunction = createMockExecuteFunction({
resource: 'row',
operation: 'getAll',
returnAll: false,
limit: 50,
tableId,
filterType: 'none',
orderBy: '',
});
await node.execute.call(fakeExecuteFunction);
expect(supabaseApiRequest).toHaveBeenCalledWith(
'GET',
expectedEndpoint,
expect.anything(),
expect.anything(),
undefined,
expect.anything(),
);
supabaseApiRequest.mockRestore();
},
);
const operations: Array<[string, IDataObject]> = [
['create', { dataToSend: 'defineBelow', fieldsUi: { fieldValues: [] } }],
['delete', { filterType: 'none' }],
['get', { filters: { conditions: [{ keyName: 'id', keyValue: '1' }] } }],
['update', { filterType: 'none', dataToSend: 'defineBelow', fieldsUi: { fieldValues: [] } }],
];
it.each(operations)(
'should encode the table name on the %s operation',
async (operation, parameters) => {
const supabaseApiRequest = jest.spyOn(utils, 'supabaseApiRequest').mockResolvedValue([]);
const fakeExecuteFunction = createMockExecuteFunction({
resource: 'row',
operation,
tableId: 'a/b',
...parameters,
});
await node.execute.call(fakeExecuteFunction);
expect(supabaseApiRequest.mock.calls[0][1]).toBe('/a%2Fb');
supabaseApiRequest.mockRestore();
},
);
it.each(['', '.', '..'])(
'should reject the table name "%s" before any request is made',
async (tableId) => {
const supabaseApiRequest = jest.spyOn(utils, 'supabaseApiRequest');
const fakeExecuteFunction = createMockExecuteFunction({
resource: 'row',
operation: 'getAll',
returnAll: false,
limit: 50,
tableId,
filterType: 'none',
orderBy: '',
});
await expect(node.execute.call(fakeExecuteFunction)).rejects.toThrow(UserError);
expect(supabaseApiRequest).not.toHaveBeenCalled();
supabaseApiRequest.mockRestore();
},
);
});
describe('getSchemaHeader function', () => {
const mockExecuteContext = {
getNodeParameter: jest.fn(),

View File

@ -10,6 +10,7 @@ import type {
import { NodeConnectionTypes } from 'n8n-workflow';
import { getSites, webflowApiRequest } from '../GenericFunctions';
import { verifySignature } from '../WebflowTriggerHelpers';
export class WebflowTriggerV2 implements INodeType {
description: INodeTypeDescription;
@ -154,6 +155,9 @@ export class WebflowTriggerV2 implements INodeType {
let responseData;
const webhookData = this.getWorkflowStaticData('node');
const siteId = this.getNodeParameter('site') as string;
if (!webhookData.webhookId) {
return true;
}
const endpoint = `/sites/${siteId}/webhooks/${webhookData.webhookId}`;
try {
responseData = await webflowApiRequest.call(this, 'DELETE', endpoint);
@ -171,6 +175,12 @@ export class WebflowTriggerV2 implements INodeType {
};
async webhook(this: IWebhookFunctions): Promise<IWebhookResponseData> {
if (!(await verifySignature.call(this))) {
const res = this.getResponseObject();
res.status(401).send('Unauthorized').end();
return { noWebhookResponse: true };
}
const req = this.getRequestObject();
return {
workflowData: [this.helpers.returnJsonArray(req.body as IDataObject[])],

View File

@ -0,0 +1,56 @@
import { createHmac, timingSafeEqual } from 'crypto';
import type { IWebhookFunctions } from 'n8n-workflow';
const MAX_TIMESTAMP_AGE_MS = 300_000;
export async function verifySignature(this: IWebhookFunctions): Promise<boolean> {
let secret: string;
try {
const credentials = await this.getCredentials('webflowOAuth2Api');
if (typeof credentials.clientSecret !== 'string' || credentials.clientSecret === '') {
return false;
}
secret = credentials.clientSecret;
} catch {
return false;
}
const req = this.getRequestObject();
const timestamp = req.header('x-webflow-timestamp');
const signature = req.header('x-webflow-signature');
const rawBody = req.rawBody;
if (
typeof timestamp !== 'string' ||
!/^\d+$/.test(timestamp) ||
typeof signature !== 'string' ||
(!Buffer.isBuffer(rawBody) && typeof rawBody !== 'string')
) {
return false;
}
const timestampMs = Number(timestamp);
if (
!Number.isSafeInteger(timestampMs) ||
Math.abs(Date.now() - timestampMs) > MAX_TIMESTAMP_AGE_MS
) {
return false;
}
try {
const hmac = createHmac('sha256', secret);
hmac.update(`${timestamp}:`);
hmac.update(rawBody);
const expectedSignature = Buffer.from(hmac.digest('hex'));
const actualSignature = Buffer.from(signature);
return (
expectedSignature.length === actualSignature.length &&
timingSafeEqual(expectedSignature, actualSignature)
);
} catch {
return false;
}
}

View File

@ -0,0 +1,64 @@
import type { IDataObject, INodeTypeBaseDescription, IWebhookFunctions } from 'n8n-workflow';
import { WebflowTriggerV2 } from '../V2/WebflowTriggerV2.node';
import { verifySignature } from '../WebflowTriggerHelpers';
jest.mock('../GenericFunctions', () => ({
getSites: jest.fn(),
}));
jest.mock('../WebflowTriggerHelpers', () => ({
verifySignature: jest.fn(),
}));
const baseDescription: INodeTypeBaseDescription = {
displayName: 'Webflow Trigger',
name: 'webflowTrigger',
icon: 'file:webflow.svg',
group: ['trigger'],
description: 'Handle Webflow events via webhooks',
defaultVersion: 2,
};
describe('WebflowTriggerV2', () => {
const trigger = new WebflowTriggerV2(baseDescription);
let response: { status: jest.Mock; send: jest.Mock; end: jest.Mock };
let context: IWebhookFunctions;
const body: IDataObject = { triggerType: 'form_submission' };
beforeEach(() => {
jest.clearAllMocks();
response = {
status: jest.fn().mockReturnThis(),
send: jest.fn().mockReturnThis(),
end: jest.fn().mockReturnThis(),
};
context = {
getRequestObject: jest.fn().mockReturnValue({ body }),
getResponseObject: jest.fn().mockReturnValue(response),
helpers: {
returnJsonArray: jest.fn().mockImplementation((data: IDataObject) => [data]),
},
} as unknown as IWebhookFunctions;
});
it('rejects a delivery when signature verification fails', async () => {
jest.mocked(verifySignature).mockResolvedValue(false);
const result = await trigger.webhook.call(context);
expect(response.status).toHaveBeenCalledWith(401);
expect(response.send).toHaveBeenCalledWith('Unauthorized');
expect(response.end).toHaveBeenCalled();
expect(result).toEqual({ noWebhookResponse: true });
});
it('starts the workflow when signature verification succeeds', async () => {
jest.mocked(verifySignature).mockResolvedValue(true);
const result = await trigger.webhook.call(context);
expect(result).toEqual({ workflowData: [[body]] });
expect(response.status).not.toHaveBeenCalled();
});
});

View File

@ -0,0 +1,110 @@
import { createHmac } from 'crypto';
import type { IWebhookFunctions } from 'n8n-workflow';
import { verifySignature } from '../WebflowTriggerHelpers';
describe('WebflowTriggerHelpers', () => {
const clientSecret = 'oauth-client-secret';
const rawBody = Buffer.from('{"triggerType":"form_submission"}');
function createSignature(secret: string, timestamp: string, body: Buffer | string = rawBody) {
const hmac = createHmac('sha256', secret);
hmac.update(`${timestamp}:`);
hmac.update(body);
return hmac.digest('hex');
}
function createContext(options: {
credentialSecret?: string;
timestamp?: string | null;
signature?: string | null;
body?: Buffer | string;
}): IWebhookFunctions {
const headers: Record<string, string | null> = {
'x-webflow-timestamp': options.timestamp ?? null,
'x-webflow-signature': options.signature ?? null,
};
return {
getCredentials: jest.fn().mockResolvedValue({ clientSecret: options.credentialSecret }),
getRequestObject: jest.fn().mockReturnValue({
header: (name: string) => headers[name] ?? null,
rawBody: options.body ?? rawBody,
}),
} as unknown as IWebhookFunctions;
}
it('accepts a valid OAuth delivery', async () => {
const timestamp = Date.now().toString();
const context = createContext({
credentialSecret: clientSecret,
timestamp,
signature: createSignature(clientSecret, timestamp),
});
await expect(verifySignature.call(context)).resolves.toBe(true);
});
it('accepts a valid delivery with a string raw body', async () => {
const timestamp = Date.now().toString();
const body = rawBody.toString();
const context = createContext({
credentialSecret: clientSecret,
timestamp,
signature: createSignature(clientSecret, timestamp, body),
body,
});
await expect(verifySignature.call(context)).resolves.toBe(true);
});
it('rejects a delivery with a missing signature', async () => {
const context = createContext({
credentialSecret: clientSecret,
timestamp: Date.now().toString(),
});
await expect(verifySignature.call(context)).resolves.toBe(false);
});
it('rejects a delivery with an invalid signature', async () => {
const context = createContext({
credentialSecret: clientSecret,
timestamp: Date.now().toString(),
signature: 'invalid',
});
await expect(verifySignature.call(context)).resolves.toBe(false);
});
it('rejects a delivery with an expired timestamp', async () => {
const timestamp = (Date.now() - 301_000).toString();
const context = createContext({
credentialSecret: clientSecret,
timestamp,
signature: createSignature(clientSecret, timestamp),
});
await expect(verifySignature.call(context)).resolves.toBe(false);
});
it('rejects a delivery with a malformed timestamp', async () => {
const timestamp = `${Date.now()}invalid`;
const context = createContext({
credentialSecret: clientSecret,
timestamp,
signature: createSignature(clientSecret, timestamp),
});
await expect(verifySignature.call(context)).resolves.toBe(false);
});
it('rejects an OAuth delivery when the client secret is unavailable', async () => {
const context = createContext({
timestamp: Date.now().toString(),
signature: 'signature',
});
await expect(verifySignature.call(context)).resolves.toBe(false);
});
});

View File

@ -1,32 +0,0 @@
import { UserError } from 'n8n-workflow';
import { toPathSegment } from './url';
describe('toPathSegment', () => {
it('should leave a plain id unchanged', () => {
expect(toPathSegment('my-index')).toBe('my-index');
});
it('should encode an id so it stays a single path segment', () => {
expect(toPathSegment('a/b/c')).toBe('a%2Fb%2Fc');
expect(toPathSegment('other-index/_doc/x')).toBe('other-index%2F_doc%2Fx');
});
it.each([
// A comma is percent-encoded, which Elasticsearch still reads as a multi-index
// separator, so `a,b` keeps resolving to both indices.
['a,b', 'a%2Cb'],
// A wildcard is left alone, so index patterns keep working.
['logs-*', 'logs-*'],
])('should preserve multi-value and wildcard syntax (%s)', (id, expected) => {
expect(toPathSegment(id)).toBe(expected);
});
it.each(['', '.', '..'])('should reject the identifier "%s"', (id) => {
expect(() => toPathSegment(id)).toThrow(UserError);
});
it.each([null, undefined])('should reject the nullish identifier %s', (id) => {
expect(() => toPathSegment(id)).toThrow(UserError);
});
});

View File

@ -1,6 +1,7 @@
import { average as aAverage } from './array-extensions';
import { ExpressionExtensionError } from '../errors/expression-extension.error';
import { ExpressionError } from '../errors/expression.error';
import { toPathSegment as toPathSegmentValue } from '../url';
// Define an own data field rather than assigning through an inherited setter.
function defineField(target: Record<string, unknown>, key: PropertyKey, value: unknown): void {
@ -72,6 +73,17 @@ function ifEmpty<T, V>(value: V, defaultValue: T) {
return value;
}
function toPathSegment(value: unknown): string {
try {
return toPathSegmentValue(value);
} catch (error) {
if (error instanceof Error) {
throw new ExpressionError(error.message);
}
throw error;
}
}
ifEmpty.doc = {
name: 'ifEmpty',
description:
@ -91,6 +103,7 @@ export const extendedFunctions = {
average,
numberList,
zip,
toPathSegment,
$min: min,
$max: max,
$average: average,

View File

@ -22,6 +22,7 @@ export * from './expressions/expression-helpers';
export * from './from-ai-parse-utils';
export * from './node-helpers';
export * from './tool-helpers';
export * from './url';
export * from './node-reference-parser-utils';
export * from './metadata-utils';
export * from './workflow';

View File

@ -1,4 +1,4 @@
import { UserError } from 'n8n-workflow';
import { UserError } from './errors';
/**
* Encode an id as a single URL path segment. `.`, `..`, and the empty string

View File

@ -170,6 +170,7 @@ describe('Expression — compatibility corpus', () => {
selectedCardId: 'c1',
dimensions: ['2026-08-24 10:30:00', 'DE'],
metrics: [10, 20],
pathId: 'a/b c',
jsonString: '{"k":[1,2,3]}',
key: 'dimensions',
zero: 0,
@ -809,6 +810,7 @@ describe('Expression — compatibility corpus', () => {
['$prevNode.outputIndex', '={{ $prevNode.outputIndex }}', 0],
['$execution.id', '={{ $execution.id }}', 'exec-1'],
['$workflow.active', '={{ $workflow.active }}', false],
['toPathSegment($json.pathId)', '={{ toPathSegment($json.pathId) }}', 'a%2Fb%20c'],
['`${$runIndex}:${$itemIndex}`', '={{ `${$runIndex}:${$itemIndex}` }}', '0:0'],
['$("Rates").first()?.json?.cardId', '={{ $("Rates").first()?.json?.cardId }}', 'c1'],
[
@ -862,6 +864,7 @@ describe('Expression — compatibility corpus', () => {
['"}}"', '={{ "}}" }}'],
['"{{" + "}}"', '={{ "{{" + "}}" }}'],
['literal {{ }} braces? {{ 1 + 1', '=literal {{ }} braces? {{ 1 + 1 }}'],
['dot URL path segment', '={{ toPathSegment("..") }}'],
];
it.each(REJECTED)('rejects: %s', (_name, expr) => {

View File

@ -0,0 +1,18 @@
import { toPathSegment, UserError } from '../src';
describe('toPathSegment', () => {
it.each([
['my-id', 'my-id'],
[123, '123'],
['a/b c', 'a%2Fb%20c'],
['a?b#c\\d', 'a%3Fb%23c%5Cd'],
['already%2Fencoded', 'already%252Fencoded'],
['café', 'caf%C3%A9'],
])('encodes %s as one path segment', (input, expected) => {
expect(toPathSegment(input)).toBe(expected);
});
it.each(['', '.', '..', null, undefined])('rejects %s', (input) => {
expect(() => toPathSegment(input)).toThrow(UserError);
});
});