chore: Bundle/2.x (#41035)

Co-authored-by: Matsu <matias.huhta@n8n.io>
Co-authored-by: Bernhard Wittmann <bernhard.wittmann@n8n.io>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Elias Meire <elias@meire.dev>
Co-authored-by: n8n-assistant[bot] <n8n-assistant[bot]@users.noreply.github.com>
Co-authored-by: João Almeida <jg.almeida56@gmail.com>
Co-authored-by: Savelii <savelii.sychov@n8n.io>
Co-authored-by: Rick Gale <richardgale90@gmail.com>
Co-authored-by: Emilia <100027345+sovietspaceship@users.noreply.github.com>
Co-authored-by: oleg <me@olegivaniv.com>
Co-authored-by: RomanDavydchuk <roman.davydchuk@n8n.io>
Co-authored-by: Stephen Wright <sjw948@gmail.com>
Co-authored-by: Yuliia Pominchuk <31064937+yuliia-pominchuk@users.noreply.github.com>
Co-authored-by: Shireen Missi <94372015+ShireenMissi@users.noreply.github.com>
Co-authored-by: Miguel Ángel Moreno <1411774+miguelsaddress@users.noreply.github.com>
Co-authored-by: Anne Aguirre <anne.aguirre@n8n.io>
Co-authored-by: Dimitri Lavrenük <20122620+dlavrenuek@users.noreply.github.com>
Co-authored-by: Kai <kai.hartlage@n8n.io>
Co-authored-by: Luigi Gubello <27034386+luigigubello@users.noreply.github.com>
Co-authored-by: Danny Martini <danny@n8n.io>
Co-authored-by: MHMD <45968454+dou-mhmd@users.noreply.github.com>
Co-authored-by: Mutasem Aldmour <4711238+mutdmour@users.noreply.github.com>
Co-authored-by: Iván Ovejero <ivov.src@gmail.com>
Co-authored-by: Charlie Kolb <charlie@n8n.io>
This commit is contained in:
n8n-assistant[bot] 2026-10-14 06:33:56 +00:00 committed by GitHub
parent ccf42e3213
commit 46761a60a9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
139 changed files with 6276 additions and 905 deletions

View File

@ -0,0 +1,68 @@
import { isChatHubToolTypeAllowed } from '../chat-hub';
describe('isChatHubToolTypeAllowed', () => {
it.each(['@n8n/n8n-nodes-langchain.chat', '@n8n/n8n-nodes-langchain.chatHitlTool'])(
'blocks the %s variant of the Chat tool for all users',
(type) => {
expect(isChatHubToolTypeAllowed(type, { isChatUser: false })).toBe(false);
},
);
it('blocks the Data Table HITL tool for chat users', () => {
expect(isChatHubToolTypeAllowed('n8n-nodes-base.dataTableHitlTool', { isChatUser: true })).toBe(
false,
);
});
it('blocks the Chat tool for all users', () => {
expect(
isChatHubToolTypeAllowed('@n8n/n8n-nodes-langchain.chatTool', { isChatUser: false }),
).toBe(false);
expect(
isChatHubToolTypeAllowed('@n8n/n8n-nodes-langchain.chatTool', { isChatUser: true }),
).toBe(false);
});
it.each([
'@n8n/n8n-nodes-langchain.toolWorkflow',
'n8n-nodes-base.dataTableTool',
'n8n-nodes-base.executeCommand',
'n8n-nodes-base.executeCommandTool',
'n8n-nodes-base.code',
'@n8n/n8n-nodes-langchain.toolCode',
'n8n-nodes-base.git',
'n8n-nodes-base.gitTool',
'n8n-nodes-base.messageAnAgentTool',
'n8n-nodes-base.executeCommandHitlTool',
'n8n-nodes-base.codeHitlTool',
'n8n-nodes-base.readWriteFileHitlTool',
'n8n-nodes-base.executeWorkflowHitlTool',
'n8n-nodes-base.n8nHitlTool',
])('blocks %s for chat users', (type) => {
expect(isChatHubToolTypeAllowed(type, { isChatUser: true })).toBe(false);
});
it.each(['n8n-nodes-base.executeCommandTool', '@n8n/n8n-nodes-langchain.toolCode'])(
'allows %s for users who can build workflows',
(type) => {
expect(isChatHubToolTypeAllowed(type, { isChatUser: false })).toBe(true);
},
);
it.each([
'n8n-nodes-base.slackTool',
'n8n-nodes-base.gmailTool',
'n8n-nodes-base.httpRequestTool',
'@n8n/n8n-nodes-langchain.toolHttpRequest',
'n8n-nodes-base.graphqlTool',
'n8n-nodes-base.rssFeedReadTool',
'@n8n/n8n-nodes-langchain.mcpClientTool',
'n8n-nodes-base.sshTool',
'n8n-nodes-base.ftpHitlTool',
'n8n-nodes-base.jenkinsTool',
'n8n-nodes-base.rundeckTool',
'n8n-nodes-base.awsLambdaTool',
])('allows %s for chat users', (type) => {
expect(isChatHubToolTypeAllowed(type, { isChatUser: true })).toBe(true);
});
});

View File

@ -2,7 +2,10 @@ import type { Scope } from '@n8n/permissions';
import {
CHAT_TOOL_NODE_TYPE,
type ChunkType,
CODE_NODE_TYPE,
CODE_TOOL_LANGCHAIN_NODE_TYPE,
DATA_TABLE_TOOL_NODE_TYPE,
EXECUTE_WORKFLOW_NODE_TYPE,
type INode,
INodeSchema,
WORKFLOW_TOOL_LANGCHAIN_NODE_TYPE,
@ -709,12 +712,46 @@ export interface ChatHubToolDto {
/** Tool types blocked for ALL users in the Chat Hub. */
export const ALWAYS_BLOCKED_CHAT_HUB_TOOL_TYPES: string[] = [CHAT_TOOL_NODE_TYPE];
/** Additional tool types blocked for chat-only users (global:chatUser). */
/**
* Additional tool types blocked for chat-only users (global:chatUser).
* These tools run code or commands on the n8n instance, or access its files,
* workflows, data tables or agents. Tools that act on a remote system through
* a credential stay allowed: the credential limits what they can do.
* Chat users cannot build workflows, so they cannot add these tools either.
* A `*Tool` or `*HitlTool` variant of a listed node type is blocked too.
* Any node can have a `*HitlTool` variant, so the list also holds node types
* that are not usable as tools.
*/
export const CHAT_USER_BLOCKED_CHAT_HUB_TOOL_TYPES: string[] = [
WORKFLOW_TOOL_LANGCHAIN_NODE_TYPE,
DATA_TABLE_TOOL_NODE_TYPE,
CODE_NODE_TYPE,
CODE_TOOL_LANGCHAIN_NODE_TYPE,
EXECUTE_WORKFLOW_NODE_TYPE,
'n8n-nodes-base.executeCommand',
'n8n-nodes-base.messageAnAgent',
'n8n-nodes-base.git',
'n8n-nodes-base.readWriteFile',
'n8n-nodes-base.n8n',
];
/** Matches the node type registry, which resolves both suffixes to the base node type. */
function withoutToolSuffix(type: string): string {
return type.replace(/HitlTool$/, '').replace(/Tool$/, '');
}
/** Compares base node types, so an entry also covers the base, `*Tool` and `*HitlTool` forms. */
function isListed(list: string[], type: string): boolean {
const baseType = withoutToolSuffix(type);
return list.some((entry) => withoutToolSuffix(entry) === baseType);
}
/** Returns whether a user can add or run a tool of this type in the Chat Hub. */
export function isChatHubToolTypeAllowed(type: string, user: { isChatUser: boolean }): boolean {
if (isListed(ALWAYS_BLOCKED_CHAT_HUB_TOOL_TYPES, type)) return false;
return !(user.isChatUser && isListed(CHAT_USER_BLOCKED_CHAT_HUB_TOOL_TYPES, type));
}
/**
* Request schema for creating a chat hub tool
*/

View File

@ -246,6 +246,11 @@ export { GetWorkflowQueryDto } from './workflows/get-workflow-query.dto';
export { UpdateWorkflowPublicDto } from './workflows/update-workflow-public.dto';
export { UpdateWorkflowQueryDto } from './workflows/update-workflow-query.dto';
export { ListWorkflowsQueryDto } from './workflows/list-workflows-query.dto';
export {
ListCallableSubworkflowsQueryDto,
type CallableSubworkflow,
type CallableSubworkflowsResponse,
} from './workflows/list-callable-subworkflows-query.dto';
export {
WorkflowPublicDto,
CreatedWorkflowPublicDto,

View File

@ -212,6 +212,11 @@ describe('CreateWorkflowDto', () => {
request: { name: 'a'.repeat(129), nodes: [], connections: {} },
expectedErrorPath: ['name'],
},
...['__proto__', 'constructor', 'prototype'].map((id) => ({
name: `reserved id ${id}`,
request: { id, name: 'My Workflow', nodes: [], connections: {} },
expectedErrorPath: ['id'],
})),
{
name: 'missing nodes',
request: { name: 'Test', connections: {} },

View File

@ -1,9 +1,13 @@
import { z } from 'zod';
import { baseWorkflowShape } from './base-workflow.dto';
import { isSafeObjectKey } from '../../schemas/instance-ai.schema';
import { Z } from '../../zod-class';
export const workflowIdSchema = z.string();
// Workflow ids are used as object keys, so `__proto__`, `constructor` and `prototype` are not allowed
export const workflowIdSchema = z
.string()
.refine(isSafeObjectKey, { message: 'Invalid workflow id' });
export class CreateWorkflowDto extends Z.class({
// Spread base fields (name, nodes, connections, settings, etc.)

View File

@ -0,0 +1,28 @@
import { z } from 'zod';
import { Z } from '../../zod-class';
import { paginationSchema } from '../pagination/pagination.dto';
/**
* Query for the list of workflows that a parent workflow can call as
* sub-workflows. Only name search and pagination are accepted, so callers
* cannot probe workflow contents.
*/
export class ListCallableSubworkflowsQueryDto extends Z.class({
...paginationSchema,
query: z.string().trim().max(256).optional(),
}) {}
export type CallableSubworkflow = {
id: string;
name: string;
active: boolean;
isArchived: boolean;
/** Set only when the workflow belongs to a team project. */
projectName?: string;
};
export type CallableSubworkflowsResponse = {
count: number;
data: CallableSubworkflow[];
};

View File

@ -96,6 +96,7 @@ export {
ChatHubUpdateToolRequest,
ALWAYS_BLOCKED_CHAT_HUB_TOOL_TYPES,
CHAT_USER_BLOCKED_CHAT_HUB_TOOL_TYPES,
isChatHubToolTypeAllowed,
chatHubVectorStoreProviderSchema,
type ChatHubVectorStoreProvider,
VECTOR_STORE_PROVIDER_CREDENTIAL_TYPE_MAP,

View File

@ -1,8 +1,8 @@
import type { HttpsProxyAgent } from 'https-proxy-agent';
import type http from 'node:http';
import http from 'node:http';
import { makeSsrfBridge } from '../../ssrf/__tests__/mock-ssrf-bridge';
import { installProxyConnectionGuard } from '../connection-guards';
import { installConnectionGuard, installProxyConnectionGuard } from '../connection-guards';
type ConnectOpts = Parameters<HttpsProxyAgent<string>['connect']>[1];
@ -76,3 +76,78 @@ describe('installProxyConnectionGuard', () => {
expect(bridge.validateConnectionHost).not.toHaveBeenCalled();
});
});
describe('installConnectionGuard', () => {
function guarded() {
const createConnection = vi.fn().mockReturnValue('SOCKET');
const target = { createConnection };
return { target, createConnection };
}
it('validates the target host and delegates when the policy allows it', () => {
const bridge = makeSsrfBridge();
const { target, createConnection } = guarded();
installConnectionGuard(target, bridge);
expect(target.createConnection({ host: 'target.example.com' })).toBe('SOCKET');
expect(bridge.validateConnectionHost).toHaveBeenCalledWith('target.example.com');
expect(createConnection).toHaveBeenCalled();
});
it('rejects a connection to a socket path without validating the unused host', () => {
const bridge = makeSsrfBridge();
const { target, createConnection } = guarded();
installConnectionGuard(target, bridge);
expect(() =>
target.createConnection({ host: 'target.example.com', socketPath: '/var/run/docker.sock' }),
).toThrow('Cannot open a connection to a socket path');
expect(createConnection).not.toHaveBeenCalled();
expect(bridge.validateConnectionHost).not.toHaveBeenCalled();
});
it('reports a socket path through the callback when the caller passes one', () => {
const bridge = makeSsrfBridge();
const { target, createConnection } = guarded();
installConnectionGuard(target, bridge);
const onConnect = vi.fn();
expect(
target.createConnection({ socketPath: '/var/run/docker.sock' }, onConnect),
).toBeUndefined();
expect(onConnect).toHaveBeenCalledWith(
expect.objectContaining({ message: 'Cannot open a connection to a socket path' }),
);
expect(createConnection).not.toHaveBeenCalled();
});
});
describe('installConnectionGuard on a real http.Agent', () => {
async function get(agent: http.Agent): Promise<string> {
return await new Promise((resolve, reject) => {
const req = http.get('http://target.example.com/x', { agent, timeout: 3000 }, (res) => {
let data = '';
res.on('data', (chunk) => (data += String(chunk)));
res.on('end', () => resolve(data));
});
req.on('error', reject);
req.on('timeout', () => {
req.destroy();
reject(new Error('timeout'));
});
});
}
it.each([
['ssrf active', () => makeSsrfBridge()],
['ssrf disabled', () => 'disabled' as const],
])('refuses a socket path Node puts in the connect options (%s)', async (_label, ssrf) => {
const agent = new http.Agent({ socketPath: '/tmp/backend-network.sock' } as http.AgentOptions);
installConnectionGuard(agent as unknown as { createConnection: never }, ssrf());
await expect(get(agent)).rejects.toThrow('Cannot open a connection to a socket path');
});
});

View File

@ -15,7 +15,7 @@ import { installConnectionGuard, UnknownConnectionHostError } from '../connectio
import { EnvProxyHttpAgent } from '../env-proxy-http-agent';
import { EnvProxyHttpsAgent } from '../env-proxy-https-agent';
import { type LocalServer, startServer } from '../local-server';
import { buildNodeAgents, type ProxyUrl } from '../node-agents';
import { buildNodeAgents, type NodeAgentOptions, type ProxyUrl } from '../node-agents';
// HttpsProxyAgent stores `lookup` in `connectOpts` rather than `options`
// (unlike http.Agent and HttpProxyAgent which use `options`).
@ -32,6 +32,10 @@ function getAgentOptions(agent: http.Agent | https.Agent): { keepAlive?: boolean
return (agent as unknown as { options?: { keepAlive?: boolean } }).options ?? {};
}
function withSocketPath(socketPath: string): NodeAgentOptions {
return { socketPath } as unknown as NodeAgentOptions;
}
// ---------------------------------------------------------------------------
// buildNodeAgents — shared builder (single source of truth for the undici
// factory, the axios transport layer, and the global proxy agents)
@ -111,6 +115,29 @@ describe('buildNodeAgents', () => {
});
});
describe('rejects a caller-provided socketPath (not a validated connection target)', () => {
const socketPath = '/tmp/backend-network.sock';
it.each([
['ssrf disabled', 'disabled' as const],
['ssrf active', makeSsrfBridge()],
])('throws when agentOptions.socketPath is set (%s)', (_label, ssrf) => {
expect(() => buildNodeAgents(false, ssrf, withSocketPath(socketPath))).toThrow(
'`agentOptions.socketPath` is not supported',
);
});
it.each([
['direct', false as const],
['env proxy', 'env' as const],
['explicit proxy', 'http://proxy.internal:3128' as ProxyUrl],
])('throws on the %s path too', (_label, proxy) => {
expect(() => buildNodeAgents(proxy, makeSsrfBridge(), withSocketPath(socketPath))).toThrow(
'`agentOptions.socketPath` is not supported',
);
});
});
describe('SSRF lookup placement', () => {
const modes: Array<[string, false | 'env' | ProxyUrl]> = [
['proxy: false', false],

View File

@ -14,8 +14,18 @@ export class UnknownConnectionHostError extends UnexpectedError {
}
}
export class SocketPathConnectionError extends UnexpectedError {
constructor() {
super('Cannot open a connection to a socket path');
}
}
/** Subset of an agent's connection options we read to find the target host. */
type ConnectionOptions = { host?: string | null; hostname?: string | null };
type ConnectionOptions = {
host?: string | null;
hostname?: string | null;
socketPath?: string;
};
/**
* Runtime-only `createConnection` method of Node's http(s) agents.
@ -59,6 +69,16 @@ export function installProxyConnectionGuard(agent: ProxyConnectionAgent, ssrf: S
};
}
function connectionError(
ssrf: SsrfBridge | 'disabled',
options: ConnectionOptions,
): Error | undefined {
if (options.socketPath) {
return new SocketPathConnectionError();
}
return ssrf === 'disabled' ? undefined : connectionHostError(ssrf, options);
}
/**
* Wraps an agent's `createConnection` to validate the connection target before the socket opens.
*
@ -68,11 +88,11 @@ export function installProxyConnectionGuard(agent: ProxyConnectionAgent, ssrf: S
*/
export function installConnectionGuard(
target: { createConnection: CreateConnection },
ssrf: SsrfBridge,
ssrf: SsrfBridge | 'disabled',
): void {
const createConnection = target.createConnection.bind(target);
target.createConnection = (options, onConnect) => {
const error = connectionHostError(ssrf, options);
const error = connectionError(ssrf, options);
if (error) {
if (onConnect) {
onConnect(error);

View File

@ -1,4 +1,4 @@
import { UnexpectedError } from 'n8n-workflow';
import { UserError } from 'n8n-workflow';
import http from 'node:http';
import https from 'node:https';
import type { LookupFunction } from 'node:net';
@ -81,6 +81,10 @@ export type NodeAgentOptions = https.AgentOptions;
* therefore has no effect and is rejected to avoid a false sense of control over
* DNS resolution.
*
* `agentOptions.socketPath` is rejected for a related reason: it replaces the
* connection target, leaving the request's host and port unused, so nothing the
* policy decides applies to the socket that opens.
*
* `agentOptions` describes the connection to the **target**. Behind a proxy its TLS
* options travel with the tunnelled session rather than the proxy handshake
* (see `proxy/proxied-agents.ts`).
@ -91,11 +95,17 @@ export function buildNodeAgents(
agentOptions?: NodeAgentOptions,
): { httpAgent: http.Agent; httpsAgent: https.Agent } {
if (agentOptions?.lookup) {
throw new UnexpectedError(
throw new UserError(
'`agentOptions.lookup` is not supported: DNS resolution is managed by the SSRF policy. Remove it from `agentOptions`.',
);
}
if (agentOptions !== undefined && 'socketPath' in agentOptions && agentOptions.socketPath) {
throw new UserError(
'`agentOptions.socketPath` is not supported. Remove it from `agentOptions`.',
);
}
const lookup: LookupFunction | undefined =
ssrf !== 'disabled' ? ssrf.createSecureLookup() : undefined;
@ -132,15 +142,16 @@ export function buildNodeAgents(
}
/**
* Installs {@link installConnectionGuard} on a direct-path agent pair when SSRF protection is active.
* Installs {@link installConnectionGuard} on a direct-path agent pair.
*
* It goes on whatever the SSRF setting, because it also refuses a socket path, which
* is not an SSRF decision.
*/
function applyConnectionGuard(
agents: { httpAgent: http.Agent; httpsAgent: https.Agent },
ssrf: SsrfOption,
): { httpAgent: http.Agent; httpsAgent: https.Agent } {
if (ssrf !== 'disabled') {
installConnectionGuard(agents.httpAgent, ssrf);
installConnectionGuard(agents.httpsAgent, ssrf);
}
installConnectionGuard(agents.httpAgent, ssrf);
installConnectionGuard(agents.httpsAgent, ssrf);
return agents;
}

View File

@ -35,14 +35,17 @@ describe('WorkflowProjectService', () => {
});
});
it('loads the owner project when the cached value is malformed', async () => {
const project = { id: 'project-1' } as Project;
cacheService.getHashValue.mockResolvedValue('invalid' as never);
sharedWorkflowRepository.findOneOrFail.mockResolvedValue({ project } as never);
it.each(['invalid', {}])(
'loads the owner project when the cached value is malformed',
async (cached) => {
const project = { id: 'project-1' } as Project;
cacheService.getHashValue.mockResolvedValue(cached as never);
sharedWorkflowRepository.findOneOrFail.mockResolvedValue({ project } as never);
await expect(service.getWorkflowProjectCached('workflow-1')).resolves.toBe(project);
expect(sharedWorkflowRepository.findOneOrFail).toHaveBeenCalledOnce();
});
await expect(service.getWorkflowProjectCached('workflow-1')).resolves.toBe(project);
expect(sharedWorkflowRepository.findOneOrFail).toHaveBeenCalledOnce();
},
);
it('sets the cached owner project', async () => {
const project = { id: 'project-1' } as Project;

View File

@ -338,6 +338,27 @@ for (const backend of ['memory', 'redis'] as const) {
await expect(cacheService.getHashValue('key', 'field1')).resolves.toBe('value1');
});
if (backend === 'memory') {
test('should not return inherited properties as hash field values', async () => {
await cacheService.setHash('key', { field1: { id: 'value1' } });
await expect(cacheService.getHashValue('key', '__proto__')).resolves.toBeUndefined();
await expect(cacheService.getHashValue('key', 'constructor')).resolves.toBeUndefined();
});
test('should not store a `__proto__` hash field', async () => {
await cacheService.setHash('key', { field1: { id: 'value1' } });
// A computed key creates an own `__proto__` field, as a hash keyed by a workflow id does
await cacheService.setHash('key', { ['__proto__']: { id: 'value2' } });
expect(Object.getPrototypeOf(await cacheService.get('key'))).toBe(Object.prototype);
await expect(cacheService.getHashValue('key', '__proto__')).resolves.toBeUndefined();
await expect(cacheService.getHashValue('key', 'field1')).resolves.toEqual({
id: 'value1',
});
});
}
});
});
}

View File

@ -151,7 +151,15 @@ export class CacheService extends TypedEmitter<CacheEvents> {
const hashObject: Hash = (await this.get(key)) ?? {};
Object.assign(hashObject, hash);
for (const [field, value] of Object.entries(hash)) {
// Skip `__proto__`. On a plain object, `hash['__proto__'] = value` does not add a field.
// It changes the object's prototype.
if (field === '__proto__') {
continue;
}
hashObject[field] = value;
}
await this.set(key, hashObject);
}
@ -309,7 +317,10 @@ export class CacheService extends TypedEmitter<CacheEvents> {
} else {
const hashObject = await this.cache.store.get<Hash<T>>(cacheKey);
hashValue = hashObject?.[hashKey] as MaybeHash<T>;
hashValue =
hashObject && Object.hasOwn(hashObject, hashKey)
? (hashObject[hashKey] as MaybeHash<T>)
: undefined;
}
if (hashValue !== undefined) {

View File

@ -10,7 +10,9 @@ export class WorkflowProjectService {
) {}
private reconstructProject(project: Partial<Project>): Project | undefined {
if (typeof project !== 'object' || project === null) return undefined;
if (typeof project !== 'object' || project === null || typeof project.id !== 'string') {
return undefined;
}
return Object.assign(new Project(), project);
}

View File

@ -1,5 +1,6 @@
import { GlobalConfig } from '@n8n/config';
import { In, IsNull, MoreThan, Not, type SelectQueryBuilder } from '@n8n/typeorm';
import type { IWorkflowSettings } from 'n8n-workflow';
import type { Mock, Mocked } from 'vitest';
import { mock } from 'vitest-mock-extended';
@ -840,4 +841,270 @@ describe('WorkflowRepository', () => {
expect(mockExecute).toHaveBeenCalled();
});
});
describe('caller allowlist purge', () => {
let writes: Array<{ id: string; callerIds: string }>;
let writtenSettings: Array<IWorkflowSettings | undefined>;
let selectQb: { where: Mock; andWhere: Mock; setLock: Mock };
const stubCandidates = (candidates: Array<Partial<WorkflowEntity>>) => {
writes = [];
writtenSettings = [];
const select = {
select: vi.fn(),
where: vi.fn(),
andWhere: vi.fn(),
setLock: vi.fn(),
getMany: vi.fn().mockResolvedValue(candidates),
};
select.select.mockReturnValue(select);
select.where.mockReturnValue(select);
select.andWhere.mockReturnValue(select);
select.setLock.mockReturnValue(select);
let pendingId = '';
let pendingCallerIds = '';
let pendingSettings: IWorkflowSettings | undefined;
const update = {
update: vi.fn(),
set: vi.fn(),
where: vi.fn(),
setParameter: vi.fn(),
execute: vi.fn(async () => {
writes.push({ id: pendingId, callerIds: pendingCallerIds });
writtenSettings.push(pendingSettings);
}),
};
update.update.mockReturnValue(update);
// Postgres writes the whole document, SQLite the one key through a parameter.
update.set.mockImplementation((values: { settings: unknown }) => {
if (typeof values.settings === 'object' && values.settings !== null) {
pendingSettings = values.settings as IWorkflowSettings;
pendingCallerIds = pendingSettings.callerIds ?? '';
}
return update;
});
update.where.mockImplementation((_sql: string, parameters: { workflowId: string }) => {
pendingId = parameters.workflowId;
return update;
});
update.setParameter.mockImplementation((_name: string, value: string) => {
pendingCallerIds = value;
return update;
});
vi.mocked(entityManager.createQueryBuilder).mockImplementation(((...args: unknown[]) =>
args.length > 0 ? select : update) as never);
transactionRunner.run.mockImplementation(
async (ctx: unknown, fn: (c: unknown) => Promise<unknown>) => await fn(ctx),
);
selectQb = select;
return select;
};
it('removes the entry and keeps the rest in order, whatever whitespace separates them', async () => {
stubCandidates([
{
id: 'subworkflow-1',
settings: { callerPolicy: 'workflowsFromAList', callerIds: 'first,\n second , ,\tthird' },
},
]);
await workflowRepository.removeFromCallerAllowlists(['second'], {});
expect(writes).toEqual([{ id: 'subworkflow-1', callerIds: 'first,third' }]);
});
it('rewrites only the allowlists that hold the id as an entry of its own', async () => {
stubCandidates([
{
id: 'subworkflow-1',
settings: { callerPolicy: 'workflowsFromAList', callerIds: 'abc,abcdef' },
},
{
id: 'subworkflow-2',
settings: { callerPolicy: 'workflowsFromAList', callerIds: 'abcdef' },
},
{ id: 'subworkflow-3', settings: { callerPolicy: 'workflowsFromAList' } },
]);
await workflowRepository.removeFromCallerAllowlists(['abc'], {});
expect(writes).toEqual([{ id: 'subworkflow-1', callerIds: 'abcdef' }]);
});
it('leaves an allowlist alone when the id only appears inside another entry', async () => {
stubCandidates([
{
id: 'subworkflow-1',
settings: { callerPolicy: 'workflowsFromAList', callerIds: 'abc-1, abc-2' },
},
]);
await workflowRepository.removeFromCallerAllowlists(['abc'], {});
expect(writes).toEqual([]);
});
it('rewrites every workflow that names the id', async () => {
stubCandidates([
{
id: 'subworkflow-1',
settings: { callerPolicy: 'workflowsFromAList', callerIds: 'first,second' },
},
{
id: 'subworkflow-2',
settings: { callerPolicy: 'workflowsFromAList', callerIds: 'second,third' },
},
]);
await workflowRepository.removeFromCallerAllowlists(['second'], {});
expect(writes).toEqual([
{ id: 'subworkflow-1', callerIds: 'first' },
{ id: 'subworkflow-2', callerIds: 'third' },
]);
});
it('takes every id out of one allowlist in a single write', async () => {
stubCandidates([
{
id: 'subworkflow-1',
settings: { callerPolicy: 'workflowsFromAList', callerIds: 'first,second,third' },
},
]);
await workflowRepository.removeFromCallerAllowlists(['first', 'third'], {});
expect(writes).toEqual([{ id: 'subworkflow-1', callerIds: 'second' }]);
});
it('reads the candidates once, whatever the batch size, and locks only the ones it rewrites', async () => {
const select = stubCandidates([
{
id: 'subworkflow-1',
settings: { callerPolicy: 'workflowsFromAList', callerIds: 'one,first' },
},
{
id: 'subworkflow-2',
settings: { callerPolicy: 'workflowsFromAList', callerIds: 'first' },
},
]);
await workflowRepository.removeFromCallerAllowlists(['one', 'two'], {});
expect(select.where).toHaveBeenCalledTimes(2);
expect(select.where).toHaveBeenNthCalledWith(1, expect.stringContaining('callerIds'));
expect(select.where).toHaveBeenNthCalledWith(2, 'workflow.id IN (:...ids)', {
ids: ['subworkflow-1'],
});
expect(select.setLock).toHaveBeenCalledTimes(1);
expect(select.setLock).toHaveBeenCalledWith('pessimistic_write');
expect(writes).toEqual([{ id: 'subworkflow-1', callerIds: 'first' }]);
});
it('locks nothing when no allowlist names a removed id', async () => {
const select = stubCandidates([
{
id: 'subworkflow-1',
settings: { callerPolicy: 'workflowsFromAList', callerIds: 'first' },
},
]);
await workflowRepository.removeFromCallerAllowlists(['one'], {});
expect(select.setLock).not.toHaveBeenCalled();
expect(writes).toEqual([]);
});
it('matches candidates on the stored document, never on a value read out of it', async () => {
const select = stubCandidates([]);
await workflowRepository.removeFromCallerAllowlists(['one'], {});
expect(select.where).toHaveBeenCalledWith('workflow.settings::text LIKE \'%"callerIds"%\'');
});
it('keeps a value the binary JSON form has no room for', async () => {
const noBinaryJsonForm = String.fromCharCode(0);
stubCandidates([
{
id: 'subworkflow-1',
settings: {
callerPolicy: 'workflowsFromAList',
callerIds: 'first,second',
errorWorkflow: `error-workflow${noBinaryJsonForm}`,
},
},
]);
await workflowRepository.removeFromCallerAllowlists(['second'], {});
expect(writtenSettings).toEqual([
{
callerPolicy: 'workflowsFromAList',
callerIds: 'first',
errorWorkflow: `error-workflow${noBinaryJsonForm}`,
},
]);
});
it('purges a row that exists at an id being removed', async () => {
stubCandidates([
{
id: 'recreated-1',
settings: { callerPolicy: 'workflowsFromAList', callerIds: 'first,second' },
},
]);
await workflowRepository.removeFromCallerAllowlists(['recreated-1', 'second'], {});
expect(writes).toEqual([{ id: 'recreated-1', callerIds: 'first' }]);
});
it('writes nothing when given no ids', async () => {
stubCandidates([]);
await workflowRepository.removeFromCallerAllowlists([], {});
expect(selectQb.where).not.toHaveBeenCalled();
expect(writes).toEqual([]);
});
it('removes only the entries naming an id without a row', async () => {
stubCandidates([
{
id: 'subworkflow-1',
settings: { callerPolicy: 'workflowsFromAList', callerIds: 'deleted,survivor,other' },
},
]);
vi.mocked(entityManager.find).mockResolvedValue([{ id: 'survivor' }, { id: 'other' }]);
await workflowRepository.removeMissingFromCallerAllowlists({});
expect(entityManager.find).toHaveBeenCalledWith(WorkflowEntity, {
select: ['id'],
where: { id: In(['deleted', 'survivor', 'other']) },
});
expect(writes).toEqual([{ id: 'subworkflow-1', callerIds: 'survivor,other' }]);
});
it('locks and writes nothing when every named id has a row', async () => {
const select = stubCandidates([
{
id: 'subworkflow-1',
settings: { callerPolicy: 'workflowsFromAList', callerIds: 'first,second' },
},
]);
vi.mocked(entityManager.find).mockResolvedValue([{ id: 'first' }, { id: 'second' }]);
await workflowRepository.removeMissingFromCallerAllowlists({});
expect(select.setLock).not.toHaveBeenCalled();
expect(writes).toEqual([]);
});
});
});

View File

@ -454,6 +454,136 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
}
}
/** Remove every id in `workflowIds` from the `callerIds` allowlist of every other workflow. */
async removeFromCallerAllowlists(workflowIds: string[], ctx: OperationContext): Promise<void> {
if (workflowIds.length === 0) return;
await this.runInTransaction(ctx, async (tx) => {
const candidates = await this.findCallerAllowlistCandidates(tx);
await this.purgeCallerAllowlists(candidates, new Set(workflowIds), tx);
});
}
/** Remove every id that has no workflow row from every `callerIds` allowlist. */
async removeMissingFromCallerAllowlists(ctx: OperationContext): Promise<void> {
await this.runInTransaction(ctx, async (tx) => {
const candidates = await this.findCallerAllowlistCandidates(tx);
const namedIds = [
...new Set(candidates.flatMap((workflow) => this.callerIdEntries(workflow))),
];
const existingIds = new Set<string>();
for (const chunk of chunkIds(namedIds)) {
const existing = await tx.find(WorkflowEntity, {
select: ['id'],
where: { id: In(chunk) },
});
for (const { id } of existing) existingIds.add(id);
}
const missingIds = new Set(namedIds.filter((id) => !existingIds.has(id)));
if (missingIds.size > 0) await this.purgeCallerAllowlists(candidates, missingIds, tx);
});
}
/** Rewrite the allowlists among `candidates` that name any of `removedIds`, under a row lock. */
private async purgeCallerAllowlists(
candidates: WorkflowEntity[],
removedIds: Set<string>,
tx: EntityManager,
): Promise<void> {
const affected = candidates.filter(
(workflow) => this.callerIdsWithout(workflow, removedIds) !== undefined,
);
for (const workflow of await this.lockCallerAllowlists(affected, tx)) {
const remaining = this.callerIdsWithout(workflow, removedIds);
if (remaining !== undefined) await this.writeCallerIds(workflow, remaining, tx);
}
}
/** The non-empty, trimmed entries of the `callerIds` allowlist of `workflow`. */
private callerIdEntries(workflow: WorkflowEntity): string[] {
const callerIds = workflow.settings?.callerIds;
if (typeof callerIds !== 'string') return [];
return callerIds
.split(',')
.map((callerId) => callerId.trim())
.filter((callerId) => callerId !== '');
}
/** The allowlist of `workflow` without `removedIds`, or `undefined` when it names none of them. */
private callerIdsWithout(workflow: WorkflowEntity, removedIds: Set<string>): string | undefined {
const entries = this.callerIdEntries(workflow);
if (!entries.some((callerId) => removedIds.has(callerId))) return undefined;
return entries.filter((callerId) => !removedIds.has(callerId)).join(',');
}
/** Find every workflow that has a `callerIds` allowlist. */
private async findCallerAllowlistCandidates(tx: EntityManager): Promise<WorkflowEntity[]> {
// On Postgres, match the raw text rather than extract: the `json` column accepts characters,
// such as an escaped NUL, that extracting `callerIds` rejects with `unsupported Unicode
// escape sequence`, which fails the query for every row.
const hasAllowlist =
this.globalConfig.database.type === 'postgresdb'
? 'workflow.settings::text LIKE \'%"callerIds"%\''
: `${this.settingsTextValue('workflow.settings', 'callerIds')} IS NOT NULL`;
return await tx
.createQueryBuilder(WorkflowEntity, 'workflow')
.select(['workflow.id', 'workflow.settings'])
.where(hasAllowlist)
.getMany();
}
/** Lock `workflows` until the transaction ends and re-read their settings under the lock. */
private async lockCallerAllowlists(
workflows: WorkflowEntity[],
tx: EntityManager,
): Promise<WorkflowEntity[]> {
// SQLite serialises writes, so what it read is already current.
if (this.globalConfig.database.type !== 'postgresdb') return workflows;
const locked: WorkflowEntity[] = [];
for (const chunk of chunkIds(workflows.map(({ id }) => id))) {
locked.push(
...(await tx
.createQueryBuilder(WorkflowEntity, 'workflow')
.select(['workflow.id', 'workflow.settings'])
.where('workflow.id IN (:...ids)', { ids: chunk })
.setLock('pessimistic_write')
.getMany()),
);
}
return locked;
}
/** Store `callerIds` back on one workflow, keeping its other settings. */
private async writeCallerIds(
workflow: WorkflowEntity,
callerIds: string,
tx: EntityManager,
): Promise<void> {
const update = tx
.createQueryBuilder()
.update(WorkflowEntity)
.where('id = :workflowId', { workflowId: workflow.id });
if (this.globalConfig.database.type === 'postgresdb') {
// `jsonb_set` rejects characters the `json` column accepts, so rewrite the whole document
// read under the row lock.
await update.set({ settings: { ...workflow.settings, callerIds } }).execute();
return;
}
await update
.set({ settings: () => "JSON_SET(COALESCE(settings, '{}'), '$.callerIds', :callerIds)" })
.setParameter('callerIds', callerIds)
.execute();
}
async findById(workflowId: string) {
return await this.findOne({
where: { id: workflowId },
@ -864,7 +994,6 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
onlySharedWithMe?: boolean;
},
options: WorkflowListQueryOptions = {},
callableForParentWorkflowId?: string,
) {
if (
options.filter?.parentFolderId &&
@ -884,18 +1013,8 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
}
const [workflowsAndFolders, count] = await Promise.all([
this.getWorkflowsAndFoldersUnionWithSharingSubquery(
user,
sharingOptions,
options,
callableForParentWorkflowId,
),
this.getWorkflowsAndFoldersCountWithSharingSubquery(
user,
sharingOptions,
options,
callableForParentWorkflowId,
),
this.getWorkflowsAndFoldersUnionWithSharingSubquery(user, sharingOptions, options),
this.getWorkflowsAndFoldersCountWithSharingSubquery(user, sharingOptions, options),
]);
const isArchived =
@ -922,15 +1041,9 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
onlySharedWithMe?: boolean;
},
options: WorkflowListQueryOptions = {},
callableForParentWorkflowId?: string,
) {
const { baseQuery, sortByColumn, sortByDirection } =
this.buildBaseUnionQueryWithSharingSubquery(
user,
sharingOptions,
options,
callableForParentWorkflowId,
);
this.buildBaseUnionQueryWithSharingSubquery(user, sharingOptions, options);
const query = this.buildUnionQuery(baseQuery, {
sortByColumn,
@ -956,7 +1069,6 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
onlySharedWithMe?: boolean;
},
options: WorkflowListQueryOptions = {},
callableForParentWorkflowId?: string,
) {
const { skip, take, ...baseQueryParameters } = options;
@ -964,7 +1076,6 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
user,
sharingOptions,
baseQueryParameters,
callableForParentWorkflowId,
);
const response = await baseQuery
@ -986,7 +1097,6 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
onlySharedWithMe?: boolean;
},
options: WorkflowListQueryOptions = {},
callableForParentWorkflowId?: string,
) {
// Common fields for both folders and workflows
const commonFields = {
@ -1036,7 +1146,6 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
user,
sharingOptions,
workflowQueryParameters,
callableForParentWorkflowId,
).addSelect("'workflow'", 'resource');
const qb = this.manager.createQueryBuilder();
@ -1155,14 +1264,8 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
onlySharedWithMe?: boolean;
},
options: WorkflowListQueryOptions = {},
callableForParentWorkflowId?: string,
): Promise<WorkflowListResult> {
const query = this.getManyQueryWithSharingSubquery(
user,
sharingOptions,
options,
callableForParentWorkflowId,
);
const query = this.getManyQueryWithSharingSubquery(user, sharingOptions, options);
const [workflows, count] = await query.getManyAndCount();
return { workflows, count };
@ -1182,7 +1285,6 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
onlySharedWithMe?: boolean;
},
options: WorkflowListQueryOptions = {},
callableForParentWorkflowId?: string,
): SelectQueryBuilder<WorkflowEntity> {
const qb = this.createQueryBuilder('workflow');
@ -1200,21 +1302,9 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
sharingOptionsWithProjectId,
);
if (callableForParentWorkflowId) {
// Union: workflows readable by the user OR callable by the parent workflow.
const callableSubquery = this.buildCallablePolicySubquery(callableForParentWorkflowId);
qb.andWhere(
`(workflow.id IN (${sharedWorkflowSubquery.getQuery()}) OR workflow.id IN (${callableSubquery.getQuery()}))`,
);
qb.setParameters({
...sharedWorkflowSubquery.getParameters(),
...callableSubquery.getParameters(),
});
} else {
// Apply the sharing filter using the subquery
qb.andWhere(`workflow.id IN (${sharedWorkflowSubquery.getQuery()})`);
qb.setParameters(sharedWorkflowSubquery.getParameters());
}
// Apply the sharing filter using the subquery
qb.andWhere(`workflow.id IN (${sharedWorkflowSubquery.getQuery()})`);
qb.setParameters(sharedWorkflowSubquery.getParameters());
// Apply other filters
// For personal project and shared-with-me cases, projectId is already handled in the subquery
@ -1238,70 +1328,98 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
}
/**
* Build a subquery returning IDs of workflows whose callerPolicy permits the
* given `parentWorkflowId` to call them.
* List the workflows that `parentWorkflowId` can call as sub-workflows. The
* list holds the workflows the user can read, plus the workflows whose
* caller policy names the parent explicitly (`any`, or an allowlist that
* contains the parent id).
*
* Only a minimal set of columns is loaded, because the callable rows can
* belong to projects that the user has no access to.
*/
async findCallableSubworkflows(
user: User,
sharingOptions: { scopes: Scope[]; projectRoles: string[]; workflowRoles: string[] },
parentWorkflowId: string,
options: { query?: string; triggerNodeTypes: string[]; skip: number; take: number },
): Promise<{ workflows: WorkflowEntity[]; count: number }> {
const sharedWorkflowSubquery = this.buildSharedWorkflowIdsSubquery(user, sharingOptions);
const callableSubquery = this.buildCallablePolicySubquery(parentWorkflowId);
const qb = this.createQueryBuilder('workflow')
.select([
'workflow.id',
'workflow.name',
'workflow.activeVersionId',
'workflow.isArchived',
'workflow.updatedAt',
])
.leftJoin('workflow.shared', 'owner', 'owner.role = :callableOwnerRole', {
callableOwnerRole: 'workflow:owner',
})
.leftJoin('owner.project', 'ownerProject')
.addSelect([
'owner.workflowId',
'owner.projectId',
'owner.role',
'ownerProject.id',
'ownerProject.type',
'ownerProject.name',
])
.where(
`(workflow.id IN (${sharedWorkflowSubquery.getQuery()}) OR workflow.id IN (${callableSubquery.getQuery()}))`,
)
.setParameters({
...sharedWorkflowSubquery.getParameters(),
...callableSubquery.getParameters(),
});
// Match names only: the list can hold workflows the user cannot read, so
// their descriptions must not affect the result.
const searchWords = this.parseSearchWords(options.query);
if (searchWords.length > 0) {
const { conditions, parameters } = this.buildSearchConditions(
searchWords,
'LOWER(workflow.name)',
);
qb.andWhere(`(${conditions.join(' AND ')})`, parameters);
}
this.applyTriggerNodeTypesFilter(qb, options.triggerNodeTypes);
qb.orderBy('workflow.updatedAt', 'DESC').skip(options.skip).take(options.take);
const [workflows, count] = await qb.getManyAndCount();
return { workflows, count };
}
/**
* Build a subquery returning IDs of workflows whose callerPolicy names the
* given `parentWorkflowId` explicitly.
*
* The `workflowsFromSameOwner` policy is left out on purpose: for a user who
* is not the owner, it would list workflows that the owner did not share.
* The owner can already read those workflows through the sharing subquery.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private buildCallablePolicySubquery(parentWorkflowId: string): SelectQueryBuilder<any> {
const subquery = this.manager
.createQueryBuilder()
.select('cpw.id')
.from(WorkflowEntity, 'cpw')
.leftJoin(
SharedWorkflow,
'sw_sub',
"sw_sub.workflowId = cpw.id AND sw_sub.role = 'workflow:owner'",
)
.leftJoin(
SharedWorkflow,
'sw_par',
"sw_par.workflowId = :cpParentWorkflowId AND sw_par.role = 'workflow:owner'",
);
const { conditions, params } = this.buildCallablePolicyConditions(parentWorkflowId);
subquery.where('(' + conditions.join(' OR ') + ')', params);
return subquery;
}
private buildCallablePolicyConditions(parentWorkflowId: string): {
conditions: string[];
params: Record<string, string>;
} {
const defaultPolicy = this.globalConfig.workflows.callerPolicyDefaultOption;
const callerPolicy = this.settingsTextValue('cpw.settings', 'callerPolicy');
const callerIds = this.settingsTextValue('cpw.settings', 'callerIds');
const conditions: string[] = [];
const params: Record<string, string> = {
cpParentWorkflowId: parentWorkflowId,
cpCallerIdMembership: `%,${escapeLike(parentWorkflowId)},%`,
};
// Branch 1: callerPolicy = 'any'
conditions.push(`${callerPolicy} = 'any'`);
// Branch 2: callerPolicy = 'workflowsFromAList' and the allowlist contains parentWorkflowId as a whole ID.
conditions.push(
const conditions = [
`${callerPolicy} = 'any'`,
// The allowlist must contain parentWorkflowId as a whole ID.
`(${callerPolicy} = 'workflowsFromAList' AND (',' || REPLACE(${callerIds}, ' ', '') || ',') LIKE :cpCallerIdMembership ${LIKE_ESCAPE_CLAUSE})`,
);
// Branch 3: callerPolicy = 'workflowsFromSameOwner' (or NULL when default is 'workflowsFromSameOwner').
const sameOwnerPolicyClauses = [`${callerPolicy} = 'workflowsFromSameOwner'`];
if (defaultPolicy === 'workflowsFromSameOwner') {
sameOwnerPolicyClauses.push(`${callerPolicy} IS NULL`);
}
conditions.push(
`((${sameOwnerPolicyClauses.join(' OR ')}) AND sw_sub.projectId = sw_par.projectId AND sw_par.projectId IS NOT NULL)`,
);
// Handle NULL callerPolicy when default is 'any'
];
if (defaultPolicy === 'any') {
conditions.push(`${callerPolicy} IS NULL`);
}
return { conditions, params };
return this.manager
.createQueryBuilder()
.select('cpw.id')
.from(WorkflowEntity, 'cpw')
.where(`(${conditions.join(' OR ')})`, {
cpCallerIdMembership: `%,${escapeLike(parentWorkflowId)},%`,
});
}
private settingsTextValue(field: string, key: string): string {
@ -1467,14 +1585,15 @@ export class WorkflowRepository extends BaseRepository<WorkflowEntity> {
/**
* Builds search conditions and parameters for matching all of the search words
*/
private buildSearchConditions(searchWords: string[]): {
private buildSearchConditions(
searchWords: string[],
fieldExpression = this.getFieldConcatExpression(),
): {
conditions: string[];
parameters: Record<string, string>;
} {
const concatExpression = this.getFieldConcatExpression();
const conditions = searchWords.map((_, index) => {
return `${concatExpression} LIKE :searchWord${index}`;
return `${fieldExpression} LIKE :searchWord${index}`;
});
const parameters: Record<string, string> = {};

View File

@ -1098,6 +1098,10 @@ export class ChatTrigger extends Node {
// By header as well as by the iframe's attribute, so the document has no
// origin even if the attribute is ever stripped.
res.setHeader('Content-Security-Policy', `sandbox ${CHAT_FRAME_SANDBOX}`);
} else {
// The webhook route sandboxes every response. This page keeps its origin,
// because the widget keeps the session in `localStorage`.
res.removeHeader('Content-Security-Policy');
}
const page = createPage({

View File

@ -984,6 +984,17 @@ describe('ChatTrigger Node', () => {
expect(renderedView().sandbox).not.toContain('allow-popups-to-escape-sandbox');
});
it('removes the route sandbox from the public chat page', async () => {
await renderSetupPage('none');
expect(mockResponse.removeHeader).toHaveBeenCalledWith('Content-Security-Policy');
expect(mockResponse.setHeader).not.toHaveBeenCalledWith(
'Content-Security-Policy',
expect.anything(),
);
expect(renderedPage()).toContain('createChat');
});
it('renders the author chat for the frame own request', async () => {
mockRequest.query = { n8nShellInner: '1' };
mockRequest.headers = {
@ -998,6 +1009,7 @@ describe('ChatTrigger Node', () => {
'Content-Security-Policy',
'sandbox allow-scripts allow-forms allow-modals allow-popups',
);
expect(mockResponse.removeHeader).not.toHaveBeenCalled();
expect(renderedPage()).toContain('createChat');
// Identity injected server-side, and a header token for the messages that follow.
expect(renderedPage()).toContain('"email":"visitor@example.com"');

View File

@ -1,5 +1,6 @@
import { MongoClient } from 'mongodb';
import type { ILoadOptionsFunctions, ISupplyDataFunctions } from 'n8n-workflow';
import { NodeOperationError } from 'n8n-workflow';
import { mock } from 'vitest-mock-extended';
import {
@ -144,6 +145,30 @@ describe('VectorStoreMongoDBAtlas', () => {
expect(mockClient.connect).toHaveBeenCalledTimes(1);
expect(client).toBe(mockClient);
});
it('should not include the connection string in the error when the client cannot be created', async () => {
MockMongoClient.mockImplementation(function () {
throw new Error(
'Protocol and host list are required in "mongodb://testuser:secret-pass@?x:27017"',
);
});
mockContext.getCredentials.mockResolvedValue({
configurationType: 'values',
host: '?x',
user: 'testuser',
password: 'secret-pass',
port: 27017,
database: 'testdb',
});
const error = await createMongoClient(mockContext, 1.1).catch((e: unknown) => e);
expect(error).toBeInstanceOf(NodeOperationError);
expect((error as Error).message).toBe(
'MongoDB connection error: Protocol and host list are required in "mongodb://[REDACTED]"',
);
expect(JSON.stringify(error)).not.toContain('secret-pass');
expect((error as Error).stack).not.toContain('secret-pass');
});
});
describe('.getCollections', () => {

View File

@ -11,7 +11,10 @@ import {
} from 'n8n-workflow';
import { metadataFilterField, createVectorStoreNode } from '@n8n/ai-utilities';
import { validateAndResolveMongoCredentials } from 'n8n-nodes-base/dist/nodes/MongoDb/GenericFunctions';
import {
sanitizeMongoUriInMessage,
validateAndResolveMongoCredentials,
} from 'n8n-nodes-base/dist/nodes/MongoDb/GenericFunctions';
/**
* Constants for the name of the credentials and Node parameters.
@ -169,15 +172,22 @@ export async function createMongoClient(
const node = context.getNode();
const { connectionString } = validateAndResolveMongoCredentials(node, credentials);
const client = new MongoClient(connectionString, {
appName: 'devrel.integration.n8n_vector_integ',
driverInfo: {
name: 'n8n_vector',
version: version.toString(),
},
});
await client.connect();
return client;
try {
const client = new MongoClient(connectionString, {
appName: 'devrel.integration.n8n_vector_integ',
driverInfo: {
name: 'n8n_vector',
version: version.toString(),
},
});
await client.connect();
return client;
} catch (error) {
throw new NodeOperationError(
node,
`MongoDB connection error: ${sanitizeMongoUriInMessage(error, connectionString)}`,
);
}
}
/**

View File

@ -4,7 +4,7 @@ import type { Embeddings } from '@langchain/core/embeddings';
import type { VectorStore } from '@langchain/core/vectorstores';
import type { VectorStoreNodeConstructorArgs } from '@n8n/ai-utilities';
import { ConnectionPoolManager } from 'n8n-nodes-base/dist/utils/connection-pool-manager';
import type { IExecuteFunctions } from 'n8n-workflow';
import type { IExecuteFunctions, INode } from 'n8n-workflow';
import type { Pool } from 'oracledb';
import type { Mocked } from 'vitest';
@ -323,9 +323,12 @@ vi.mock('@n8n/ai-utilities', () => {
import { VectorStoreOracleDB } from './VectorStoreOracleDB.node';
describe('VectorStoreOracleDB.node', () => {
const node = { name: 'Oracle Database Vector Store', type: 'vectorStoreOracleDBVector' } as INode;
const context = {
getNodeParameter: vi.fn(),
getCredentials: vi.fn(),
getNode: vi.fn(() => node),
logger: {
debug: vi.fn(),
error: vi.fn(),
@ -594,4 +597,104 @@ describe('VectorStoreOracleDB.node', () => {
expect(connectionCloseMocks[1]).toHaveBeenCalledTimes(1);
expect(poolCloseMocks[0]).not.toHaveBeenCalled();
});
describe('parameter validation', () => {
const withTableName = (tableName: string) => (name: string) => {
if (name === 'tableName') return tableName;
if (name === 'options.distanceStrategy') return DistanceStrategyMock.DOT_PRODUCT;
return undefined;
};
it('rejects a metadata filter name that is not a plain identifier', async () => {
const node = createNode();
await expect(
node.getVectorStoreClient(context, { "userId'": 'alice' } as OracleFilter, embeddings, 0),
).rejects.toThrow('Invalid metadata filter key');
expect(initializeSpy).not.toHaveBeenCalled();
});
it('rejects a metadata filter name supplied at search time', async () => {
const node = createNode();
const vectorStore = await node.getVectorStoreClient(context, undefined, embeddings, 0);
await expect(
vectorStore.similaritySearchVectorWithScore([0.1], 3, {
'userId?(@ == "bob")': 'alice',
} as OracleFilter),
).rejects.toThrow('Invalid metadata filter key');
expect(similaritySearchSpy).not.toHaveBeenCalled();
});
it('rejects a metadata filter name nested under $and at search time', async () => {
const node = createNode();
const vectorStore = await node.getVectorStoreClient(context, undefined, embeddings, 0);
await expect(
vectorStore.similaritySearchVectorWithScore([0.1], 3, {
$and: [{ project: 'n8n' }, { 'a.b': 'x' }],
} as OracleFilter),
).rejects.toThrow('Invalid metadata filter key');
expect(similaritySearchSpy).not.toHaveBeenCalled();
});
it('accepts a filter whose names are plain identifiers', async () => {
const node = createNode();
const filter = { userId: 'alice', user_id: 'alice' } as OracleFilter;
const vectorStore = await node.getVectorStoreClient(context, filter, embeddings, 0);
await vectorStore.similaritySearchVectorWithScore([0.1], 3, undefined);
expect(similaritySearchSpy).toHaveBeenCalledWith([0.1], 3, filter);
});
it('rejects a table name that is not a plain identifier', async () => {
const node = createNode();
context.getNodeParameter.mockImplementation(withTableName('n8n_vectors; SELECT 1'));
await expect(node.getVectorStoreClient(context, undefined, embeddings, 0)).rejects.toThrow(
'Invalid table name',
);
context.getNodeParameter.mockImplementation(defaultGetNodeParameter);
});
it('rejects a table name that is not a plain identifier when inserting', async () => {
const node = createNode();
context.getNodeParameter.mockImplementation(withTableName('n8n_vectors; SELECT 1'));
await expect(node.populateVectorStore(context, embeddings, documents, 0)).rejects.toThrow(
'Invalid table name',
);
expect(fromDocumentsSpy).not.toHaveBeenCalled();
context.getNodeParameter.mockImplementation(defaultGetNodeParameter);
});
it('accepts a schema-qualified table name', async () => {
const node = createNode();
context.getNodeParameter.mockImplementation(withTableName('app.n8n_vectors'));
await node.getVectorStoreClient(context, undefined, embeddings, 0);
const lastCall = initializeSpy.mock.calls.at(-1);
if (!lastCall) throw new Error('initializeSpy should have been called');
expect(lastCall[1]?.tableName).toBe('app.n8n_vectors');
context.getNodeParameter.mockImplementation(defaultGetNodeParameter);
});
it('falls back to the default table name when the parameter resolves to empty', async () => {
const node = createNode();
context.getNodeParameter.mockImplementation(withTableName(''));
await node.getVectorStoreClient(context, undefined, embeddings, 0);
const lastCall = initializeSpy.mock.calls.at(-1);
if (!lastCall) throw new Error('initializeSpy should have been called');
expect(lastCall[1]?.tableName).toBe('n8n_vectors');
context.getNodeParameter.mockImplementation(defaultGetNodeParameter);
});
});
});

View File

@ -4,9 +4,15 @@ import { createVectorStoreNode, metadataFilterField } from '@n8n/ai-utilities';
import { DistanceStrategy, OracleVS, type OracleDBVSArgs } from '@oracle/langchain-oracledb';
import type { OracleDBNodeCredentials } from 'n8n-nodes-base/dist/nodes/Oracle/Sql/helpers/interfaces';
import { configureOracleDB } from 'n8n-nodes-base/dist/nodes/Oracle/Sql/transport';
import type { IExecuteFunctions, INodeProperties, ISupplyDataFunctions } from 'n8n-workflow';
import type { INode, IExecuteFunctions, INodeProperties, ISupplyDataFunctions } from 'n8n-workflow';
import { NodeOperationError } from 'n8n-workflow';
import type oracledb from 'oracledb';
import {
findUnsupportedJsonPathFilterKey,
isSafeQualifiedSqlIdentifier,
} from '@utils/sqlIdentifier';
const sharedFields: INodeProperties[] = [
{
displayName: 'Table Name',
@ -20,6 +26,40 @@ const sharedFields: INodeProperties[] = [
const DEFAULT_INITIALIZATION_TEXT = 'n8n vector store initialization text';
const DEFAULT_TABLE_NAME = 'n8n_vectors';
/** Reads the Table Name, falling back to the default when an expression resolves to empty. */
function getTableName(
context: IExecuteFunctions | ISupplyDataFunctions,
itemIndex: number,
): string {
const tableName =
(context.getNodeParameter('tableName', itemIndex, '', { extractValue: true }) as string) ||
DEFAULT_TABLE_NAME;
if (!isSafeQualifiedSqlIdentifier(tableName)) {
throw new NodeOperationError(context.getNode(), `Invalid table name \"${tableName}\"`, {
itemIndex,
description:
'The table name must be a plain name, or a schema and name separated by a dot. Letters, digits and underscores only.',
});
}
return tableName;
}
/** Rejects a metadata filter the Oracle store cannot resolve into a SQL/JSON path. */
function assertSupportedFilter(node: INode, filter: unknown, itemIndex?: number): void {
const unsupported = findUnsupportedJsonPathFilterKey(filter);
if (unsupported === undefined) return;
throw new NodeOperationError(node, `Invalid metadata filter key \"${unsupported}\"`, {
itemIndex,
description:
'A metadata filter name must start with a letter or underscore and contain only letters, digits and underscores.',
});
}
const NO_ROWS_FOUND_ERROR_MESSAGE = 'No rows found.';
const isNoRowsFoundError = (error: unknown): error is Error =>
@ -146,11 +186,15 @@ const createLazyOraclePool = (
* with any ad-hoc filter provided at runtime.
*/
class ExtendedOracleDBVectorStore extends OracleVS {
n8nNode!: INode;
static async initialize(
embeddings: EmbeddingsInterface,
args: OracleDBVSArgs,
args: OracleDBVSArgs & { n8nNode: INode },
): Promise<ExtendedOracleDBVectorStore> {
const oracleDBVectorStore = new this(embeddings, args);
const { n8nNode, ...rest } = args;
const oracleDBVectorStore = new this(embeddings, rest);
oracleDBVectorStore.n8nNode = n8nNode;
await oracleDBVectorStore.initialize();
return oracleDBVectorStore;
@ -162,6 +206,9 @@ class ExtendedOracleDBVectorStore extends OracleVS {
filter?: OracleVS['FilterType'],
) {
const mergedFilter = mergeFilters(this.filter, filter);
// Checked here as well as at construction, because load and both
// retrieve-as-tool modes supply the filter at search time.
assertSupportedFilter(this.n8nNode, mergedFilter);
try {
return await super.similaritySearchVectorWithScore(query, k, mergedFilter);
@ -199,17 +246,17 @@ export class VectorStoreOracleDB extends createVectorStoreNode<ExtendedOracleDBV
embeddings: EmbeddingsInterface,
itemIndex: number,
): Promise<ExtendedOracleDBVectorStore> {
const tableName = context.getNodeParameter('tableName', itemIndex, '', {
extractValue: true,
}) as string;
const tableName = getTableName(context, itemIndex);
assertSupportedFilter(context.getNode(), filter, itemIndex);
const credentials = (await context.getCredentials('oracleDBApi')) as OracleDBNodeCredentials;
const client = createLazyOraclePool(context, credentials);
const query = DEFAULT_INITIALIZATION_TEXT;
const config: OracleDBVSArgs = {
const config: OracleDBVSArgs & { n8nNode: INode } = {
client,
tableName,
query,
filter: filter as ExtendedOracleDBVectorStore['FilterType'] | undefined,
n8nNode: context.getNode(),
};
config.distanceStrategy = context.getNodeParameter(
@ -227,9 +274,7 @@ export class VectorStoreOracleDB extends createVectorStoreNode<ExtendedOracleDBV
documents: Array<Document<Record<string, unknown>>>,
itemIndex: number,
): Promise<void> {
const tableName = context.getNodeParameter('tableName', itemIndex, '', {
extractValue: true,
}) as string;
const tableName = getTableName(context, itemIndex);
const credentials = (await context.getCredentials('oracleDBApi')) as OracleDBNodeCredentials;
const client = createLazyOraclePool(context, credentials);
const query = DEFAULT_INITIALIZATION_TEXT;

View File

@ -2,6 +2,7 @@ import {
escapeQualifiedSqlIdentifier,
escapeSqlIdentifier,
findUnsafeMetadataFilterKey,
findUnsupportedJsonPathFilterKey,
isSafeQualifiedSqlIdentifier,
isSafeSqlIdentifier,
} from './sqlIdentifier';
@ -97,4 +98,56 @@ describe('sqlIdentifier', () => {
expect(findUnsafeMetadataFilterKey({ ok: '1', 'a\\b': '2' })).toBe('a\\b');
});
});
describe('findUnsupportedJsonPathFilterKey', () => {
it.each([
['a plain name', { userId: 'alice' }],
['an underscored name', { user_id: 'alice' }],
['a leading underscore', { _internal: true }],
['mixed case', { UserID: 'alice' }],
['trailing digits', { field2: 'x' }],
['several names at once', { project: 'n8n', category: 'movies', author: 'a' }],
['an array value', { author: ['a', 'b'] }],
['an operator in value position', { rating: { $gte: 4.5 } }],
['an operator the store does not know', { nested: { flag: true } }],
['names nested under $and', { $and: [{ project: 'n8n' }, { language: 'en' }] }],
['names nested under $or', { $or: [{ project: 'n8n' }] }],
['no filter at all', undefined],
])('accepts %s', (_name, filter) => {
expect(findUnsupportedJsonPathFilterKey(filter)).toBeUndefined();
});
it.each([
['a quote', "x'"],
['a backslash', 'a\\b'],
['a double quote', 'a"b'],
['a path predicate', 'userId?(@ == "bob")'],
['a dot', 'user.id'],
['a hyphen', 'file-name'],
['a space', 'my key'],
['a dollar sign', 'a$b'],
['a leading digit', '1field'],
['an empty name', ''],
])('rejects %s', (_name, key) => {
expect(findUnsupportedJsonPathFilterKey({ [key]: 'value' })).toBe(key);
});
it('reports a name nested under $and', () => {
const filter = { $and: [{ project: 'n8n' }, { "x'": 'alice' }] };
expect(findUnsupportedJsonPathFilterKey(filter)).toBe("x'");
});
it('reports a name nested under $or inside $and', () => {
const filter = { $and: [{ $or: [{ ok: 1 }, { 'a.b': 2 }] }] };
expect(findUnsupportedJsonPathFilterKey(filter)).toBe('a.b');
});
it('reports the first unsupported name when several are present', () => {
const filter = { good: 1, "bad'one": 2, 'bad.two': 3 };
expect(findUnsupportedJsonPathFilterKey(filter)).toBe("bad'one");
});
});
});

View File

@ -1,3 +1,4 @@
import { isRecord } from '@n8n/utils/is-record';
import pg from 'pg';
/**
@ -64,3 +65,39 @@ export function findUnsafeMetadataFilterKey(
if (!filter) return undefined;
return Object.keys(filter).find((key) => key.includes("'") || key.includes('\\'));
}
/**
* Matches a field name that Oracle accepts as an unquoted SQL/JSON path step.
* Oracle documents the unquoted form as a leading letter followed by letters and
* digits; underscore is allowed here because it is inert in both the path
* grammar and the surrounding string literal, and is common in existing keys.
*/
const SAFE_JSON_PATH_FIELD = /^[A-Za-z_][A-Za-z0-9_]*$/;
/**
* Returns the first metadata-filter key that the Oracle store cannot resolve as
* an unquoted SQL/JSON path step, or undefined when every key is usable.
*
* Walks `$and` and `$or` because the store expands them before building the
* path, so a nested key reaches the same place a top-level one does.
*/
export function findUnsupportedJsonPathFilterKey(filter: unknown): string | undefined {
if (!isRecord(filter)) return undefined;
for (const [key, value] of Object.entries(filter)) {
if (key === '$and' || key === '$or') {
if (!Array.isArray(value)) continue;
for (const condition of value) {
const unsupported = findUnsupportedJsonPathFilterKey(condition);
if (unsupported !== undefined) return unsupported;
}
continue;
}
// Only the field name reaches the path. Operator names in value position
// are matched against a fixed set by the store, and values are bound.
if (!SAFE_JSON_PATH_FIELD.test(key)) return key;
}
return undefined;
}

View File

@ -193,6 +193,34 @@ describe('JsTaskRunner', () => {
expect(outcome.result).toEqual([wrapIntoJson({ allZeros: true })]);
});
it('should give small buffers their own backing memory', async () => {
const outcome = await executeForAllItems({
code: `
const fromString = Buffer.from('.');
const fromArray = Buffer.from([1, 2]);
return [{ json: {
fromString: fromString.buffer.byteLength,
fromArray: fromArray.buffer.byteLength,
} }]
`,
inputItems: [{ a: 1 }],
});
expect(outcome.result).toEqual([wrapIntoJson({ fromString: 1, fromArray: 2 })]);
});
it('should prevent guest code from re-enabling the buffer pool', async () => {
const outcome = await executeForAllItems({
code: `
Buffer.poolSize = 8192;
return [{ json: { poolSize: Buffer.poolSize } }]
`,
inputItems: [{ a: 1 }],
});
expect(outcome.result).toEqual([wrapIntoJson({ poolSize: 0 })]);
});
it('should support Buffer.from and Buffer.concat', async () => {
const outcome = await executeForAllItems({
code: `

View File

@ -186,6 +186,9 @@ export class JsTaskRunner extends TaskRunner {
const safeAlloc = Buffer.alloc.bind(Buffer);
Buffer.allocUnsafe = safeAlloc;
Buffer.allocUnsafeSlow = safeAlloc;
// Disable the shared buffer pool so small buffers get their own backing memory.
// Use defineProperty so the value is read-only: sandboxed code must not re-enable the pool.
Object.defineProperty(Buffer, 'poolSize', { value: 0, writable: false, configurable: false });
if (process.env.NODE_ENV !== 'test') {
freezeGlobals();

View File

@ -7,6 +7,16 @@ import { mock } from 'vitest-mock-extended';
import { AbstractServer } from '@/abstract-server';
import { ExternalHooks } from '@/external-hooks';
import { LiveWebhooks } from '@/webhooks/live-webhooks';
import { SlackInteractionWebhooks } from '@/webhooks/slack-interaction-webhooks';
import { TelegramInteractionWebhooks } from '@/webhooks/telegram-interaction-webhooks';
import { WaitingForms } from '@/webhooks/waiting-forms';
import { WaitingWebhooks } from '@/webhooks/waiting-webhooks';
import { createWebhookHandlerFor } from '@/webhooks/webhook-request-handler';
vi.mock('@/webhooks/webhook-request-handler', () => ({
createWebhookHandlerFor: vi.fn(),
}));
const mockServer = mock<http.Server>();
@ -87,6 +97,21 @@ describe('AbstractServer health endpoints', () => {
await testServer.init();
});
describe('webhook route registration', () => {
it('should register the waiting-forms route with the form sandbox family', async () => {
const waitingForms = mock<WaitingForms>();
Container.set(LiveWebhooks, mock<LiveWebhooks>());
Container.set(WaitingForms, waitingForms);
Container.set(WaitingWebhooks, mock<WaitingWebhooks>());
Container.set(SlackInteractionWebhooks, mock<SlackInteractionWebhooks>());
Container.set(TelegramInteractionWebhooks, mock<TelegramInteractionWebhooks>());
await testServer.start();
expect(createWebhookHandlerFor).toHaveBeenCalledWith(waitingForms, undefined, 'form');
});
});
describe('/healthz (liveness)', () => {
it('should return 200 regardless of fullyReady flag', () => {
const res = mock<express.Response>();

View File

@ -1,4 +1,6 @@
import { mockInstance } from '@n8n/backend-test-utils';
import { SecurityConfig } from '@n8n/config';
import { Container } from '@n8n/di';
import type { Request, Response } from 'express';
import { ErrorReporter } from 'n8n-core';
import { UserError } from 'n8n-workflow';
@ -8,6 +10,7 @@ import { ConflictError, ForbiddenError, InternalServerError, NotFoundError } fro
import { LicenseEulaRequiredError } from '@/errors/response-errors/license-eula-required.error';
import { PolicyViolationError } from '@/policy/policy-violation.error';
import { reportError, sendErrorResponse } from '@/response-helper';
import { setRouteSandboxSetting } from '@/webhooks/webhook-response-headers';
describe('sendErrorResponse', () => {
let mockResponse: Response;
@ -56,10 +59,34 @@ describe('sendErrorResponse', () => {
);
});
it('should declare a JSON content type on error responses', () => {
// Simulate an earlier handler having declared an HTML content type.
const res = mock<Response>({
status: vi.fn().mockReturnThis(),
json: vi.fn().mockReturnThis(),
get: vi.fn().mockReturnValue('text/html; charset=utf-8'),
});
sendErrorResponse(res, new Error('Regular error'));
expect(res.setHeader).toHaveBeenCalledWith(
'Content-Type',
expect.stringContaining('application/json'),
);
expect(res.status).toHaveBeenCalledWith(500);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({
code: 0,
message: 'Regular error',
}),
);
});
describe('form pages', () => {
const responseFor = (originalUrl: string) =>
mock<Response>({
req: mock<Request>({ originalUrl }),
locals: {},
status: vi.fn().mockReturnThis(),
json: vi.fn().mockReturnThis(),
render: vi.fn().mockReturnThis(),
@ -88,6 +115,45 @@ describe('sendErrorResponse', () => {
expect.stringContaining('sandbox'),
);
});
describe('when only form sandboxing is disabled', () => {
const securityConfig = Container.get(SecurityConfig);
beforeEach(() => {
securityConfig.disableFormHtmlSandboxing = true;
});
afterEach(() => {
securityConfig.disableFormHtmlSandboxing = false;
});
it('should keep the webhook sandbox on a webhook path that looks like a form path', () => {
const res = responseFor('/webhook/n8n-form-demo');
setRouteSandboxSetting(res, 'webhook');
sendErrorResponse(res, new NotFoundError('not found'));
expect(res.render).toHaveBeenCalledWith('form-trigger-404', { isTestWebhook: false });
expect(res.setHeader).toHaveBeenCalledWith(
'Content-Security-Policy',
expect.stringContaining('sandbox'),
);
expect(res.removeHeader).not.toHaveBeenCalled();
});
it('should not sandbox the form 404 page on a form route', () => {
const res = responseFor('/form/does-not-exist');
setRouteSandboxSetting(res, 'form');
sendErrorResponse(res, new NotFoundError('not found'));
expect(res.render).toHaveBeenCalledWith('form-trigger-404', { isTestWebhook: false });
expect(res.setHeader).not.toHaveBeenCalledWith(
'Content-Security-Policy',
expect.stringContaining('sandbox'),
);
});
});
});
});

View File

@ -379,15 +379,16 @@ describe('WorkflowExecuteAdditionalData', () => {
parameters: {},
},
];
const subWorkflowData = () =>
mock<IWorkflowBase>({
// Plain object: inline code is structure-validated, which a deep mock fails.
const subWorkflowData = (): IWorkflowBase =>
({
id: 'sub-id',
name: 'Sub Workflow',
nodes: structuredClone(nodes),
connections: {},
staticData: {},
settings: {},
});
}) as IWorkflowBase;
beforeEach(() => {
nodeTypes.getByNameAndVersion.mockReturnValue(nodeType);
@ -398,7 +399,7 @@ describe('WorkflowExecuteAdditionalData', () => {
it('checks credentials against the triggering user for an inline sub-workflow', async () => {
await executeWorkflow(
mock<IExecuteWorkflowInfo>({ id: undefined, code: subWorkflowData() }),
{ code: subWorkflowData() },
// `rootExecutionMode` explicit: a deep mock auto-stubs every
// property, so leaving it out would hand `??` a truthy proxy.
mock<IWorkflowExecuteAdditionalData>({
@ -415,11 +416,24 @@ describe('WorkflowExecuteAdditionalData', () => {
expect(credentialsPermissionChecker.check).not.toHaveBeenCalled();
});
it('rejects an inline sub-workflow with a non-boolean disabled flag', async () => {
const code = subWorkflowData();
code.nodes = code.nodes.map((n) => ({ ...n, disabled: 1 as unknown as boolean }));
await expect(
executeWorkflow({ code }, mock<IWorkflowExecuteAdditionalData>({ userId: 'user-1' }), {
parentWorkflowId: 'parent-1',
executionMode: 'manual',
}),
).rejects.toThrow('Workflow structure is invalid');
expect(credentialsPermissionChecker.checkForUser).not.toHaveBeenCalled();
});
// A sub-workflow's own WorkflowExecute runs as 'integrated', so the
// original mode has to come from the root.
it('checks against the triggering user through a nested inline sub-workflow', async () => {
await executeWorkflow(
mock<IExecuteWorkflowInfo>({ id: undefined, code: subWorkflowData() }),
{ code: subWorkflowData() },
mock<IWorkflowExecuteAdditionalData>({
userId: 'user-1',
rootExecutionMode: 'manual',
@ -442,7 +456,7 @@ describe('WorkflowExecuteAdditionalData', () => {
// access to it has since changed.
it('keeps the project check for an inline sub-workflow in a triggered run', async () => {
await executeWorkflow(
mock<IExecuteWorkflowInfo>({ id: undefined, code: subWorkflowData() }),
{ code: subWorkflowData() },
mock<IWorkflowExecuteAdditionalData>({
userId: 'publisher-1',
rootExecutionMode: undefined,
@ -467,7 +481,7 @@ describe('WorkflowExecuteAdditionalData', () => {
'checks against the acting user for an inline sub-workflow in a %s run',
async (executionMode) => {
await executeWorkflow(
mock<IExecuteWorkflowInfo>({ id: undefined, code: subWorkflowData() }),
{ code: subWorkflowData() },
mock<IWorkflowExecuteAdditionalData>({
userId: 'user-1',
rootExecutionMode: undefined,
@ -502,7 +516,7 @@ describe('WorkflowExecuteAdditionalData', () => {
it('falls back to the project check for an inline sub-workflow without a triggering user', async () => {
const workflowData = subWorkflowData();
await executeWorkflow(
mock<IExecuteWorkflowInfo>({ id: undefined, code: workflowData }),
{ code: workflowData },
mock<IWorkflowExecuteAdditionalData>({ userId: undefined }),
{ parentWorkflowId: 'parent-1', executionMode: 'webhook' },
);
@ -555,7 +569,7 @@ describe('WorkflowExecuteAdditionalData', () => {
it('preserves parent static-data persistence for inline definitions without an id', async () => {
await executeWorkflow(
{ code: mock<IWorkflowBase>({ ...subWorkflowData(), id: undefined }) },
{ code: { ...subWorkflowData(), id: undefined } as unknown as IWorkflowBase },
mock<IWorkflowExecuteAdditionalData>({ userId: undefined }),
{ parentWorkflowId: 'parent-1', executionMode: 'webhook' },
);
@ -579,7 +593,7 @@ describe('WorkflowExecuteAdditionalData', () => {
it('preserves the triggering user in the sub-workflow additional data for inline sub-workflows so nested inline calls stay scoped to that user', async () => {
await executeWorkflow(
mock<IExecuteWorkflowInfo>({ id: undefined, code: subWorkflowData() }),
{ code: subWorkflowData() },
mock<IWorkflowExecuteAdditionalData>({ userId: 'user-1' }),
mock<ExecuteWorkflowOptions>({
loadedWorkflowData: subWorkflowData(),
@ -1352,22 +1366,22 @@ describe('WorkflowExecuteAdditionalData', () => {
});
it('should use provided workflow code when id is not provided', async () => {
const workflowCode = mock<IWorkflowBase>({
const workflowCode = {
id: 'code-workflow',
name: 'Code Workflow',
active: false,
nodes: [
mock<INode>({
{
id: 'node1',
type: 'n8n-nodes-base.set',
name: 'Node 1',
typeVersion: 1,
parameters: {},
position: [250, 300],
}),
} as INode,
],
connections: {},
});
} as IWorkflowBase;
const result = await getPublishedWorkflowData({ code: workflowCode }, 'parent-workflow-id');

View File

@ -269,7 +269,7 @@ export abstract class AbstractServer {
// Register a handler for waiting forms (excluded from metrics to avoid double-counting)
this.app.all(
`/${this.endpointFormWaiting}/:path{/:suffix}`,
createWebhookHandlerFor(Container.get(WaitingForms)),
createWebhookHandlerFor(Container.get(WaitingForms), undefined, 'form'),
);
// Register a handler for waiting webhooks (excluded from metrics to avoid double-counting)

View File

@ -0,0 +1,11 @@
import type { INode } from 'n8n-workflow';
import { NodeOperationError } from 'n8n-workflow';
export class UninspectableSubworkflowSourceError extends NodeOperationError {
constructor(node: INode) {
super(
node,
`The '${node.name}' node loads its sub-workflow from a source that is resolved only when the workflow runs, so its credential access cannot be checked on save. Use the "Database" or "Define Below" source instead.`,
);
}
}

View File

@ -810,6 +810,77 @@ describe('CredentialsPermissionChecker', () => {
});
});
describe('with a default-valued authentication parameter', () => {
const credentialId = 'password-cred';
const description = {
credentials: [
{
name: 'passwordApi',
required: true,
displayOptions: { show: { authentication: ['password'] } },
},
{
name: 'tokenApi',
required: true,
displayOptions: { show: { authentication: ['token'] } },
},
],
properties: [
{
displayName: 'Authentication',
name: 'authentication',
type: 'options',
options: [
{ name: 'Password', value: 'password' },
{ name: 'Token', value: 'token' },
],
default: 'password',
},
],
};
// Saved nodes omit parameters left at their default value
const node: INode = {
id: 'node-6',
name: 'Service',
type: 'n8n-nodes-base.service',
typeVersion: 1,
position: [0, 0],
parameters: { resource: 'entry' },
credentials: { passwordApi: { id: credentialId, name: 'Password' } },
};
beforeEach(() => {
nodeTypes.getByNameAndVersion.mockReturnValue({ description } as never);
credentialsRepository.findGlobalProjectCredentialIds.mockResolvedValue([]);
});
it('should check the credential selected by the default value', async () => {
sharedCredentialsRepository.getFilteredAccessibleCredentials.mockResolvedValue([]);
await expect(permissionChecker.check(workflowId, [node])).rejects.toThrow(
'Node "Service" does not have access to the credential',
);
expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).toHaveBeenCalledWith(
[teamProject.id],
[credentialId],
);
});
it('should let the run through when the project can use the credential', async () => {
sharedCredentialsRepository.getFilteredAccessibleCredentials.mockResolvedValue([
credentialId,
]);
await expect(permissionChecker.check(workflowId, [node])).resolves.not.toThrow();
});
it('should return the credential id for the node', () => {
expect(permissionChecker.getCredentialIdsForNodes([node])).toEqual([credentialId]);
});
});
it('should fall back to checking all credentials if node type cannot be resolved', async () => {
nodeTypes.getByNameAndVersion.mockImplementation(() => {
throw new Error('Unknown node type');
@ -874,6 +945,12 @@ describe('CredentialsPermissionChecker', () => {
expect(permissionChecker.getCredentialIdsForNodes([disabledNode])).toEqual([]);
});
it.each([1, 'true', {}])('does not skip a node whose disabled flag is %p', (disabled) => {
const flaggedNode = { ...node, disabled } as unknown as INode;
expect(permissionChecker.getCredentialIdsForNodes([flaggedNode])).toEqual([credentialId]);
});
it('skips a credential reference with no id instead of throwing', () => {
const nodeWithNoId: INode = {
...node,

View File

@ -480,7 +480,7 @@ export class CredentialsPermissionChecker {
getCredentialIdsForNodes(nodes: INode[]): string[] {
const ids = new Set<string>();
for (const node of nodes) {
if (node.disabled || !node.credentials) continue;
if (node.disabled === true || !node.credentials) continue;
const activeCredTypes = this.getActiveCredentialTypes(node);
@ -495,7 +495,7 @@ export class CredentialsPermissionChecker {
private mapCredIdsToNodes(nodes: INode[]) {
return nodes.reduce<{ [credentialId: string]: INode[] }>((map, node) => {
if (node.disabled || !node.credentials) return map;
if (node.disabled === true || !node.credentials) return map;
const activeCredTypes = this.getActiveCredentialTypes(node);

View File

@ -23,7 +23,7 @@ describe('List query middleware', () => {
vi.clearAllMocks();
mockReq = { baseUrl: '/rest/workflows' } as ListQuery.Request;
mockRes = { status: () => ({ json: vi.fn() }) } as unknown as Response;
mockRes = { status: () => ({ json: vi.fn() }), setHeader: vi.fn() } as unknown as Response;
args = [mockReq, mockRes, nextFn];
sendErrorResponse = vi.spyOn(ResponseHelper, 'sendErrorResponse');

View File

@ -76,25 +76,6 @@ export class WorkflowFilter extends BaseFilter {
@Expose()
executionBlockedBy?: WorkflowExecutionBlockCause[];
/**
* When `true`, the list is widened to include workflows that the workflow
* identified by `parentWorkflowId` is permitted to call, even if those
* workflows are not directly shared with the requesting user.
*/
@IsBoolean()
@IsOptional()
@Expose()
includeCallableSubworkflows?: boolean;
/**
* ID of the workflow that will act as the caller.
* Used together with `includeCallableSubworkflows` to evaluate `callerPolicy`.
*/
@IsString()
@IsOptional()
@Expose()
parentWorkflowId?: string;
static async fromString(rawFilter: string) {
return await this.toFilter(rawFilter, WorkflowFilter);
}

View File

@ -43,6 +43,9 @@ vi.mock('../agent-eval-instructions-rewrite', () => ({ rewriteAgentInstructions:
vi.mock('../agent-eval-case-generation.service', () => ({
AgentEvalCaseGenerationService: class AgentEvalCaseGenerationService {},
}));
vi.mock('@/permissions.ee/check-access', () => ({
userHasScopes: vi.fn().mockResolvedValue(true),
}));
const AGENT_ID = 'agent-1';
const PROJECT_ID = 'proj-1';
@ -132,6 +135,7 @@ describe('AgentEvalService', () => {
runRepository.findByIdAndAgentId.mockResolvedValue(makeRun());
runRepository.findAndCountByDatasetIdAndAgentId.mockResolvedValue([[], 0]);
resultRepository.findAndCountByRunId.mockResolvedValue([[], 0]);
vi.mocked(userHasScopes).mockReset().mockResolvedValue(true);
resultRepository.findById.mockResolvedValue(makeResult());
service = new AgentEvalService(
@ -181,7 +185,10 @@ describe('AgentEvalService', () => {
['previewRun', async () => await service.previewRun(user, AGENT_ID, PROJECT_ID, {})],
['startRun', async () => await service.startRun(user, AGENT_ID, PROJECT_ID, 'ds-1', {})],
['listRuns', async () => await service.listRuns(AGENT_ID, PROJECT_ID, 'ds-1', PAGE)],
['getRunDetail', async () => await service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-1', PAGE)],
[
'getRunDetail',
async () => await service.getRunDetail(user, AGENT_ID, PROJECT_ID, 'run-1', PAGE),
],
['getRunSummary', async () => await service.getRunSummary(AGENT_ID, PROJECT_ID, 'run-1')],
['cancelRun', async () => await service.cancelRun(AGENT_ID, PROJECT_ID, 'run-1')],
[
@ -255,9 +262,9 @@ describe('AgentEvalService', () => {
it('404s a run belonging to another agent, and reads no results for it', async () => {
runRepository.findByIdAndAgentId.mockResolvedValue(null);
await expect(service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-other', PAGE)).rejects.toThrow(
NotFoundError,
);
await expect(
service.getRunDetail(user, AGENT_ID, PROJECT_ID, 'run-other', PAGE),
).rejects.toThrow(NotFoundError);
expect(resultRepository.findAndCountByRunId).not.toHaveBeenCalled();
});
@ -956,6 +963,77 @@ describe('AgentEvalService', () => {
createdById: 'user-1',
});
});
it('checks the creating user can read the referenced data table', async () => {
datasetRepository.createDataset.mockResolvedValue(makeDataset());
await service.createDataset(user, AGENT_ID, PROJECT_ID, {
name: 'cases',
agentId: AGENT_ID,
datasetSource: 'data_table',
datasetRef: { dataTableId: 'dt-1' },
});
expect(userHasScopes).toHaveBeenCalledWith(user, ['dataTable:readRow'], false, {
dataTableId: 'dt-1',
});
});
it('refuses a data table the creating user cannot read', async () => {
vi.mocked(userHasScopes).mockResolvedValue(false);
await expect(
service.createDataset(user, AGENT_ID, PROJECT_ID, {
name: 'cases',
agentId: AGENT_ID,
datasetSource: 'data_table',
datasetRef: { dataTableId: 'dt-other' },
}),
).rejects.toThrow(ForbiddenError);
expect(datasetRepository.createDataset).not.toHaveBeenCalled();
});
});
// Results hold copies of the source rows, and the table can live in another
// project than the agent, so `agent:read` alone must not open them.
describe('getRunDetail data table access', () => {
it('checks the reader can read the run dataset’s data table', async () => {
await service.getRunDetail(user, AGENT_ID, PROJECT_ID, 'run-1', PAGE);
expect(datasetRepository.findByIdAndAgentId).toHaveBeenCalledWith('ds-1', AGENT_ID);
expect(userHasScopes).toHaveBeenCalledWith(user, ['dataTable:readRow'], false, {
dataTableId: 'dt-1',
});
});
it('refuses a reader without access to the data table, and reads no results', async () => {
vi.mocked(userHasScopes).mockResolvedValue(false);
await expect(service.getRunDetail(user, AGENT_ID, PROJECT_ID, 'run-1', PAGE)).rejects.toThrow(
ForbiddenError,
);
expect(resultRepository.findAndCountByRunId).not.toHaveBeenCalled();
});
it('reads no results when the run’s dataset does not resolve', async () => {
datasetRepository.findByIdAndAgentId.mockResolvedValue(null);
await expect(service.getRunDetail(user, AGENT_ID, PROJECT_ID, 'run-1', PAGE)).rejects.toThrow(
NotFoundError,
);
expect(resultRepository.findAndCountByRunId).not.toHaveBeenCalled();
});
it('refuses a data-table dataset whose ref carries no table id', async () => {
datasetRepository.findByIdAndAgentId.mockResolvedValue(
makeDataset({ datasetRef: { credentialId: 'c-1', spreadsheetId: 's-1', sheetName: 'S' } }),
);
await expect(service.getRunDetail(user, AGENT_ID, PROJECT_ID, 'run-1', PAGE)).rejects.toThrow(
/without a table id/,
);
expect(resultRepository.findAndCountByRunId).not.toHaveBeenCalled();
});
});
describe('deleteDataset', () => {
@ -1039,7 +1117,7 @@ describe('AgentEvalService', () => {
describe('response mapping', () => {
it('serializes dates as ISO strings and keeps run coordination columns off the wire', async () => {
const detail = await service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-1', PAGE);
const detail = await service.getRunDetail(user, AGENT_ID, PROJECT_ID, 'run-1', PAGE);
expect(detail.runAt).toBe('2026-01-03T00:00:00.000Z');
expect(detail.createdAt).toBe('2026-01-03T00:00:00.000Z');
@ -1106,7 +1184,7 @@ describe('AgentEvalService', () => {
1,
]);
const detail = await service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-1', PAGE);
const detail = await service.getRunDetail(user, AGENT_ID, PROJECT_ID, 'run-1', PAGE);
expect(detail.results.data).toHaveLength(1);
expect(detail.results.data[0]).toMatchObject({
@ -1138,7 +1216,7 @@ describe('AgentEvalService', () => {
it('passes the run-detail window to the repository and reports the case total', async () => {
resultRepository.findAndCountByRunId.mockResolvedValue([[], 500]);
const detail = await service.getRunDetail(AGENT_ID, PROJECT_ID, 'run-1', {
const detail = await service.getRunDetail(user, AGENT_ID, PROJECT_ID, 'run-1', {
take: 25,
skip: 50,
});

View File

@ -485,7 +485,7 @@ describe('AgentEvalsController', () => {
await controller.getRun(runReq(), undefined, query);
expect(service.getRunDetail).toHaveBeenCalledWith(AGENT_ID, PROJECT_ID, 'run-1', {
expect(service.getRunDetail).toHaveBeenCalledWith(user, AGENT_ID, PROJECT_ID, 'run-1', {
take: 25,
skip: 50,
});
@ -507,7 +507,7 @@ describe('AgentEvalsController', () => {
it('defaults the run-detail window wider than the shared list default', async () => {
await controller.getRun(runReq(), undefined, AgentEvalRunDetailQueryDto.parse({}));
expect(service.getRunDetail).toHaveBeenCalledWith(AGENT_ID, PROJECT_ID, 'run-1', {
expect(service.getRunDetail).toHaveBeenCalledWith(user, AGENT_ID, PROJECT_ID, 'run-1', {
take: AGENT_EVAL_RESULTS_DEFAULT_TAKE,
skip: 0,
});

View File

@ -27,6 +27,7 @@ import {
AgentEvalRunRepository,
} from '@n8n/db';
import { Service } from '@n8n/di';
import { UnexpectedError } from 'n8n-workflow';
import pLimit from 'p-limit';
import { BadRequestError, ForbiddenError, NotFoundError, OperationalError } from '@n8n/errors';
@ -110,6 +111,10 @@ export class AgentEvalService {
);
}
if (payload.datasetSource === 'data_table') {
await this.assertDataTableReadable(user, payload.datasetRef.dataTableId);
}
const dataset = await this.datasetRepository.createDataset({
name: payload.name,
description: payload.description ?? null,
@ -323,8 +328,13 @@ export class AgentEvalService {
return { count, data: runs.map(toRunRecord) };
}
/** A run with a page of its per-case results — the "open a run" view. */
/**
* A run with a page of its per-case results — the "open a run" view. Results
* hold copies of the source rows, so the reader needs the same table access
* the runner required of whoever started the run.
*/
async getRunDetail(
user: User,
agentId: string,
projectId: string,
runId: string,
@ -332,6 +342,7 @@ export class AgentEvalService {
): Promise<AgentEvalRunDetail> {
await this.assertAgentInProject(agentId, projectId);
const run = await this.resolveRun(agentId, runId);
await this.assertDatasetSourceReadable(user, await this.resolveDataset(agentId, run.datasetId));
const [results, count] = await this.resultRepository.findAndCountByRunId(runId, page);
return { ...toRunRecord(run), results: { count, data: results.map(toResultRecord) } };
}
@ -541,6 +552,24 @@ export class AgentEvalService {
return dataset;
}
// The table may live in another project than the agent, so `@ProjectScope` on
// the agent's project says nothing about it. Only data-table datasets are
// runnable today, so no other source has stored rows to protect.
private async assertDatasetSourceReadable(user: User, dataset: AgentEvalDataset): Promise<void> {
if (dataset.datasetSource !== 'data_table') return;
if (!('dataTableId' in dataset.datasetRef)) {
throw new UnexpectedError(
`Agent eval dataset ${dataset.id} has a 'data_table' source without a table id.`,
);
}
await this.assertDataTableReadable(user, dataset.datasetRef.dataTableId);
}
private async assertDataTableReadable(user: User, dataTableId: string): Promise<void> {
const allowed = await userHasScopes(user, ['dataTable:readRow'], false, { dataTableId });
if (!allowed) throw new ForbiddenError('You do not have access to this dataset.');
}
private async resolveRun(agentId: string, runId: string): Promise<AgentEvalRun> {
const run = await this.runRepository.findByIdAndAgentId(runId, agentId);
if (!run) throw new NotFoundError(`Agent eval run ${runId} not found.`);

View File

@ -255,7 +255,7 @@ export class AgentEvalsController {
): Promise<AgentEvalRunDetail> {
await this.flagGate.assertEnabled(req.user);
const { agentId, projectId, runId } = req.params;
return await this.service.getRunDetail(agentId, projectId, runId, query);
return await this.service.getRunDetail(req.user, agentId, projectId, runId, query);
}
/** Counts only — cheap enough for the UI to poll while a run is in flight. */

View File

@ -21,11 +21,11 @@ describe('AgentsListController', () => {
const res = mock<Response>();
const query = { skip: 0, take: 50, sortBy: 'updatedAt:desc' } as never;
agentsService.findByUserMembershipPaginated.mockResolvedValue(response);
agentsService.findListableByUserPaginated.mockResolvedValue(response);
await controller.list(req, res, query);
expect(agentsService.findByUserMembershipPaginated).toHaveBeenCalledWith(user, query);
expect(agentsService.findListableByUserPaginated).toHaveBeenCalledWith(user, query);
expect(agentsService.findChatReachableByUserPaginated).not.toHaveBeenCalled();
expect(res.json).toHaveBeenCalledWith(response);
});
@ -35,11 +35,11 @@ describe('AgentsListController', () => {
const res = mock<Response>();
const query = { skip: 0, take: 10, filter: { query: 'support' } } as never;
agentsService.findByUserMembershipPaginated.mockResolvedValue({ count: 0, data: [] } as never);
agentsService.findListableByUserPaginated.mockResolvedValue({ count: 0, data: [] } as never);
await controller.list(req, res, query);
expect(agentsService.findByUserMembershipPaginated).toHaveBeenCalledWith(user, query);
expect(agentsService.findListableByUserPaginated).toHaveBeenCalledWith(user, query);
});
it.each([true, false])('serves the chat list when availableInChat is %s', async (value) => {
@ -53,7 +53,7 @@ describe('AgentsListController', () => {
await controller.list(req, res, query);
expect(agentsService.findChatReachableByUserPaginated).toHaveBeenCalledWith(user, query);
expect(agentsService.findByUserMembershipPaginated).not.toHaveBeenCalled();
expect(agentsService.findListableByUserPaginated).not.toHaveBeenCalled();
expect(res.json).toHaveBeenCalledWith(response);
});
@ -63,7 +63,7 @@ describe('AgentsListController', () => {
const query = { skip: 0, take: 50 } as never;
const emptyResponse = { count: 0, data: [] } as never;
agentsService.findByUserMembershipPaginated.mockResolvedValue(emptyResponse);
agentsService.findListableByUserPaginated.mockResolvedValue(emptyResponse);
await controller.list(req, res, query);

View File

@ -1148,28 +1148,36 @@ describe('AgentsService', () => {
});
});
describe('findByUserMembershipPaginated', () => {
describe('findListableByUserPaginated', () => {
const user = { id: 'user-1' } as unknown as User;
it('uses bare project membership and does not consult the scope service', async () => {
it('scopes to agent:list, the scope the project list route requires', async () => {
const { service, agentRepository, projectRelationRepository, projectScopeService } =
makeService();
projectRelationRepository.findAllByUser.mockResolvedValue([
{ projectId: 'project-1' },
{ projectId: 'project-2' },
] as never);
projectScopeService.getProjectIds.mockResolvedValue(['project-1', 'project-2']);
agentRepository.findByProjectIdsPaginated.mockResolvedValue({ count: 0, data: [] });
const options = { skip: 0, take: 10 } as never;
await service.findByUserMembershipPaginated(user, options);
await service.findListableByUserPaginated(user, options);
expect(projectRelationRepository.findAllByUser).toHaveBeenCalledWith('user-1');
expect(projectScopeService.getProjectIds).not.toHaveBeenCalled();
expect(projectScopeService.getProjectIds).toHaveBeenCalledWith(user, ['agent:list']);
expect(projectRelationRepository.findAllByUser).not.toHaveBeenCalled();
expect(agentRepository.findByProjectIdsPaginated).toHaveBeenCalledWith(
['project-1', 'project-2'],
options,
);
});
it('passes a null project scope through, so a global role sees every project', async () => {
const { service, agentRepository, projectScopeService } = makeService();
projectScopeService.getProjectIds.mockResolvedValue(null);
agentRepository.findByProjectIdsPaginated.mockResolvedValue({ count: 0, data: [] });
const options = { skip: 0, take: 10 } as never;
await service.findListableByUserPaginated(user, options);
expect(agentRepository.findByProjectIdsPaginated).toHaveBeenCalledWith(null, options);
});
});
describe('getCapabilitySummary', () => {

View File

@ -11,8 +11,8 @@ import { parseThreadListLimit } from './utils/parse-thread-list-limit';
* Global (cross-project) agents list endpoint.
*
* It serves two audiences, and `availableInChat` picks between them:
* - the overview page, which wants every agent in the user's projects and the
* full entity to render its cards;
* - the overview page, which wants every agent in the projects where the user
* holds `agent:list` and the full entity to render its cards;
* - the n8n chat page, which wants the agents whose published config carries
* the n8n Chat channel, scoped to `agent:execute` and answered with the
* narrow `AgentChatListItem` so a chat-only member gets no config.
@ -30,7 +30,7 @@ export class AgentsListController {
async list(req: AuthenticatedRequest, res: Response, @Query query: ListAgentsQueryDto) {
res.json(
query.filter?.availableInChat === undefined
? await this.agentsService.findByUserMembershipPaginated(req.user, query)
? await this.agentsService.findListableByUserPaginated(req.user, query)
: await this.agentsService.findChatReachableByUserPaginated(req.user, query),
);
}

View File

@ -470,13 +470,12 @@ export class AgentsService {
return await this.agentExecutionService.findN8nChatThreadForAgents(user.id, agentIds, threadId);
}
/** The agents overview list: every project the user belongs to, no scope check. */
async findByUserMembershipPaginated(
/** The agents overview list: every project where the user holds `agent:list`. */
async findListableByUserPaginated(
user: User,
options: ListAgentsQueryDto,
): Promise<AgentListResult> {
const projectRelations = await this.projectRelationRepository.findAllByUser(user.id);
const projectIds = projectRelations.map((pr) => pr.projectId);
const projectIds = await this.projectScopeService.getProjectIds(user, ['agent:list']);
return await this.agentRepository.findByProjectIdsPaginated(projectIds, options);
}

View File

@ -1,6 +1,7 @@
import type { ChatHubAgentKnowledgeItem } from '@n8n/api-types';
import type { Logger } from '@n8n/backend-common';
import type { User } from '@n8n/db';
import type { EntityManager } from '@n8n/typeorm';
import type { IRunExecutionData, IWorkflowBase } from 'n8n-workflow';
import { v4 as uuid } from 'uuid';
import { mock } from 'vitest-mock-extended';
@ -80,6 +81,7 @@ describe('ChatHubAgentService', () => {
const dynamicNodeParametersService = mock<DynamicNodeParametersService>();
const toolService = mock<ChatHubToolService>();
const mockUser = mock<User>({ id: mockUserId });
const mockTrx = mock<EntityManager>();
let service: ChatHubAgentService;
@ -99,6 +101,11 @@ describe('ChatHubAgentService', () => {
dynamicNodeParametersService,
toolService,
);
agentRepository.runInTransaction.mockImplementation(
async (ctx: unknown, fn: (em: never, ctx: unknown) => Promise<unknown>) =>
await fn(mockTrx as never, ctx),
);
});
describe('createAgent', () => {
@ -129,6 +136,7 @@ describe('ChatHubAgentService', () => {
model: 'gpt-4',
ownerId: mockUserId,
}),
mockTrx,
);
expect(dto.id).toBe(agent.id);
expect(dto.name).toBe('Test Agent');
@ -153,6 +161,7 @@ describe('ChatHubAgentService', () => {
expect(agentRepository.createAgent).toHaveBeenCalledWith(
expect.objectContaining({ description: null }),
mockTrx,
);
});
@ -171,10 +180,34 @@ describe('ChatHubAgentService', () => {
toolIds,
});
expect(toolService.setAgentTools).toHaveBeenCalledWith(expect.any(String), toolIds);
expect(toolService.setAgentTools).toHaveBeenCalledWith(
expect.any(String),
mockUserId,
toolIds,
mockTrx,
);
expect(dto.toolIds).toEqual(toolIds);
});
it('should create the agent and set its tools in one transaction', async () => {
agentRepository.createAgent.mockResolvedValue(makeAgent());
toolService.setAgentTools.mockRejectedValue(new NotFoundError('Chat hub tool not found'));
await expect(
service.createAgent(mockUser, {
name: 'Test Agent',
systemPrompt: 'You are helpful',
icon: { type: 'emoji', value: '🤖' },
credentialId: 'cred-1',
provider: 'openai',
model: 'gpt-4',
toolIds: [uuid()],
}),
).rejects.toThrow(NotFoundError);
expect(agentRepository.createAgent).toHaveBeenCalledWith(expect.anything(), mockTrx);
});
it('should not call setAgentTools when toolIds is empty', async () => {
const agent = makeAgent();
agentRepository.createAgent.mockResolvedValue(agent);
@ -210,6 +243,7 @@ describe('ChatHubAgentService', () => {
expect(agentRepository.createAgent).toHaveBeenCalledWith(
expect.objectContaining({ suggestedPrompts: prompts }),
mockTrx,
);
expect(dto.suggestedPrompts).toEqual(prompts);
});
@ -230,6 +264,7 @@ describe('ChatHubAgentService', () => {
expect(agentRepository.createAgent).toHaveBeenCalledWith(
expect.objectContaining({ suggestedPrompts: [] }),
mockTrx,
);
});
});
@ -273,7 +308,11 @@ describe('ChatHubAgentService', () => {
await service.updateAgent(agent.id, mockUser, { name: 'New Name' });
expect(agentRepository.updateAgent).toHaveBeenCalledWith(agent.id, { name: 'New Name' });
expect(agentRepository.updateAgent).toHaveBeenCalledWith(
agent.id,
{ name: 'New Name' },
mockTrx,
);
});
it('should update multiple fields at once', async () => {
@ -294,11 +333,15 @@ describe('ChatHubAgentService', () => {
systemPrompt: 'New prompt',
});
expect(agentRepository.updateAgent).toHaveBeenCalledWith(agent.id, {
name: 'Updated',
description: 'New desc',
systemPrompt: 'New prompt',
});
expect(agentRepository.updateAgent).toHaveBeenCalledWith(
agent.id,
{
name: 'Updated',
description: 'New desc',
systemPrompt: 'New prompt',
},
mockTrx,
);
expect(dto.name).toBe('Updated');
});
@ -311,7 +354,24 @@ describe('ChatHubAgentService', () => {
await service.updateAgent(agent.id, mockUser, { toolIds });
expect(toolService.setAgentTools).toHaveBeenCalledWith(agent.id, toolIds);
expect(toolService.setAgentTools).toHaveBeenCalledWith(
agent.id,
mockUserId,
toolIds,
mockTrx,
);
});
it('should not update the agent fields when the tools are rejected', async () => {
const agent = makeAgent();
agentRepository.getOneById.mockResolvedValue(agent);
toolService.setAgentTools.mockRejectedValue(new NotFoundError('Chat hub tool not found'));
await expect(
service.updateAgent(agent.id, mockUser, { name: 'New Name', toolIds: [uuid()] }),
).rejects.toThrow(NotFoundError);
expect(agentRepository.updateAgent).not.toHaveBeenCalled();
});
it('should not set tools when toolIds is not provided', async () => {
@ -339,6 +399,7 @@ describe('ChatHubAgentService', () => {
expect(agentRepository.updateAgent).toHaveBeenCalledWith(
agent.id,
expect.objectContaining({ suggestedPrompts: prompts }),
mockTrx,
);
expect(dto.suggestedPrompts).toEqual(prompts);
});
@ -354,6 +415,7 @@ describe('ChatHubAgentService', () => {
expect(agentRepository.updateAgent).toHaveBeenCalledWith(
agent.id,
expect.not.objectContaining({ suggestedPrompts: expect.anything() }),
mockTrx,
);
});
});

View File

@ -0,0 +1,307 @@
import { mockInstance, testDb, testModules } from '@n8n/backend-test-utils';
import type { User } from '@n8n/db';
import { Container } from '@n8n/di';
import { NotFoundError } from '@n8n/errors';
import { BinaryDataService } from 'n8n-core';
import type { INode } from 'n8n-workflow';
import { saveCredential } from '@test-integration/db/credentials';
import { createMember } from '@test-integration/db/users';
import { WorkflowExecutionService } from '../../../workflows/workflow-execution.service';
import { ChatHubAgent } from '../chat-hub-agent.entity';
import { ChatHubAgentRepository } from '../chat-hub-agent.repository';
import { ChatHubAgentService } from '../chat-hub-agent.service';
import { ChatHubCredentialsService } from '../chat-hub-credentials.service';
import { ChatHubSession } from '../chat-hub-session.entity';
import type { ChatHubTool } from '../chat-hub-tool.entity';
import { ChatHubToolRepository } from '../chat-hub-tool.repository';
import { ChatHubToolService } from '../chat-hub-tool.service';
import { ChatHubService } from '../chat-hub.service';
import { ChatHubSessionRepository } from '../chat-session.repository';
mockInstance(BinaryDataService);
mockInstance(WorkflowExecutionService);
mockInstance(ChatHubCredentialsService);
beforeAll(async () => {
await testModules.loadModules(['chat-hub']);
await testDb.init();
});
beforeEach(async () => {
await testDb.truncate([
'ChatHubSession',
'ChatHubAgent',
'ChatHubTool',
'SharedCredentials',
'CredentialsEntity',
]);
});
afterAll(async () => {
await testDb.terminate();
});
describe('chat hub tool references', () => {
let chatHubService: ChatHubService;
let agentService: ChatHubAgentService;
let toolService: ChatHubToolService;
let toolRepository: ChatHubToolRepository;
let sessionRepository: ChatHubSessionRepository;
let agentRepository: ChatHubAgentRepository;
let owner: User;
let otherUser: User;
beforeAll(() => {
chatHubService = Container.get(ChatHubService);
agentService = Container.get(ChatHubAgentService);
toolService = Container.get(ChatHubToolService);
toolRepository = Container.get(ChatHubToolRepository);
sessionRepository = Container.get(ChatHubSessionRepository);
agentRepository = Container.get(ChatHubAgentRepository);
});
beforeEach(async () => {
owner = await createMember();
otherUser = await createMember();
});
async function createTool(ownerId: string) {
const id = crypto.randomUUID();
const definition: INode = {
id,
name: `Tool ${id}`,
type: 'n8n-nodes-base.httpRequestTool',
typeVersion: 4.4,
position: [0, 0],
parameters: {},
};
return await toolRepository.createTool({
id,
name: definition.name,
type: definition.type,
typeVersion: definition.typeVersion,
ownerId,
definition,
enabled: true,
});
}
async function createSession(ownerId: string) {
return await sessionRepository.createChatSession({
id: crypto.randomUUID(),
ownerId,
title: 'session',
lastMessageAt: new Date(),
});
}
async function createAgent(ownerId: string) {
return await agentRepository.createAgent({
id: crypto.randomUUID(),
name: 'agent',
systemPrompt: 'prompt',
ownerId,
credentialId: null,
provider: 'openai',
model: 'gpt-4',
files: [],
});
}
/** Reads every stored link row, without the owner filter that the services apply */
async function storedLinkIds(entity: typeof ChatHubSession | typeof ChatHubAgent, id: string) {
const tools = await toolRepository.manager
.createQueryBuilder()
.relation(entity, 'tools')
.of(id)
.loadMany<ChatHubTool>();
return tools.map((t) => t.id).sort();
}
describe('updateSession', () => {
it('should set tools that the user owns', async () => {
const session = await createSession(owner.id);
const tool = await createTool(owner.id);
await chatHubService.updateSession(owner, session.id, { toolIds: [tool.id] });
expect(await toolService.getToolIdsForSession(session.id)).toEqual([tool.id]);
});
it('should reject tools that another user owns and keep the session unchanged', async () => {
const session = await createSession(owner.id);
const ownTool = await createTool(owner.id);
const foreignTool = await createTool(otherUser.id);
await chatHubService.updateSession(owner, session.id, { toolIds: [ownTool.id] });
await expect(
chatHubService.updateSession(owner, session.id, {
title: 'renamed',
toolIds: [ownTool.id, foreignTool.id],
}),
).rejects.toThrow(NotFoundError);
expect(await toolService.getToolIdsForSession(session.id)).toEqual([ownTool.id]);
const stored = await sessionRepository.findOneByOrFail({ id: session.id });
expect(stored.title).toBe('session');
});
it('should accept duplicate IDs of an owned tool', async () => {
const session = await createSession(owner.id);
const tool = await createTool(owner.id);
await chatHubService.updateSession(owner, session.id, { toolIds: [tool.id, tool.id] });
expect(await toolService.getToolIdsForSession(session.id)).toEqual([tool.id]);
});
it('should keep the tool links unchanged when the session update fails', async () => {
const session = await createSession(owner.id);
const tool = await createTool(owner.id);
await expect(
chatHubService.updateSession(owner, session.id, {
credentialId: crypto.randomUUID(),
toolIds: [tool.id],
}),
).rejects.toThrow();
expect(await storedLinkIds(ChatHubSession, session.id)).toEqual([]);
});
it('should only return owned tool IDs and remove other links on the next save', async () => {
const session = await createSession(owner.id);
const ownTool = await createTool(owner.id);
const foreignTool = await createTool(otherUser.id);
// Write the link rows directly to simulate rows stored before validation existed
await toolRepository.setSessionTools(session.id, [ownTool.id, foreignTool.id]);
const { session: dto } = await chatHubService.getConversation(owner.id, session.id);
expect(dto.toolIds).toEqual([ownTool.id]);
await chatHubService.updateSession(owner, session.id, { toolIds: dto.toolIds });
expect(await storedLinkIds(ChatHubSession, session.id)).toEqual([ownTool.id]);
});
});
describe('createAgent', () => {
async function agentRequest() {
const credential = await saveCredential(
{ name: 'OpenAI', type: 'openAiApi', data: { apiKey: 'test-api-key' } },
{ user: owner, role: 'credential:owner' },
);
return {
name: 'agent',
systemPrompt: 'prompt',
icon: { type: 'emoji' as const, value: '🤖' },
credentialId: credential.id,
provider: 'openai' as const,
model: 'gpt-4',
};
}
it('should set tools that the user owns', async () => {
const tool = await createTool(owner.id);
const agent = await agentService.createAgent(owner, {
...(await agentRequest()),
toolIds: [tool.id],
});
expect(await toolService.getToolIdsForAgent(agent.id)).toEqual([tool.id]);
});
it('should reject tools that another user owns and not create the agent', async () => {
const foreignTool = await createTool(otherUser.id);
await expect(
agentService.createAgent(owner, { ...(await agentRequest()), toolIds: [foreignTool.id] }),
).rejects.toThrow(NotFoundError);
expect(await agentRepository.getManyByUserId(owner.id)).toHaveLength(0);
});
it('should return each tool ID once when the request repeats an ID', async () => {
const tool = await createTool(owner.id);
const agent = await agentService.createAgent(owner, {
...(await agentRequest()),
toolIds: [tool.id, tool.id],
});
expect(agent.toolIds).toEqual([tool.id]);
});
});
describe('updateAgent', () => {
it('should reject tools that another user owns and keep the agent unchanged', async () => {
const agent = await createAgent(owner.id);
const foreignTool = await createTool(otherUser.id);
await expect(
agentService.updateAgent(agent.id, owner, { name: 'renamed', toolIds: [foreignTool.id] }),
).rejects.toThrow(NotFoundError);
expect(await toolService.getToolIdsForAgent(agent.id)).toEqual([]);
const stored = await agentRepository.findOneByOrFail({ id: agent.id });
expect(stored.name).toBe('agent');
});
it('should keep the tool links unchanged when the agent update fails', async () => {
const agent = await createAgent(owner.id);
const tool = await createTool(owner.id);
await expect(
agentService.updateAgent(agent.id, owner, {
credentialId: crypto.randomUUID(),
toolIds: [tool.id],
}),
).rejects.toThrow();
expect(await storedLinkIds(ChatHubAgent, agent.id)).toEqual([]);
});
it('should only return owned tool IDs and remove other links on the next save', async () => {
const agent = await createAgent(owner.id);
const ownTool = await createTool(owner.id);
const foreignTool = await createTool(otherUser.id);
await toolRepository.setAgentTools(agent.id, [ownTool.id, foreignTool.id]);
const dto = await agentService.getAgentByIdAsDto(agent.id, owner.id);
expect(dto.toolIds).toEqual([ownTool.id]);
await agentService.updateAgent(agent.id, owner, { toolIds: dto.toolIds });
expect(await storedLinkIds(ChatHubAgent, agent.id)).toEqual([ownTool.id]);
});
});
describe('tool definitions at run time', () => {
it('should only return session tools that the session owner owns', async () => {
const session = await createSession(owner.id);
const ownTool = await createTool(owner.id);
const foreignTool = await createTool(otherUser.id);
// Write the link rows directly to simulate rows stored before validation existed
await toolRepository.setSessionTools(session.id, [ownTool.id, foreignTool.id]);
const definitions = await toolService.getToolDefinitionsForSession(session.id);
expect(definitions.map((d) => d.id)).toEqual([ownTool.id]);
});
it('should only return agent tools that the agent owner owns', async () => {
const agent = await createAgent(owner.id);
const ownTool = await createTool(owner.id);
const foreignTool = await createTool(otherUser.id);
await toolRepository.setAgentTools(agent.id, [ownTool.id, foreignTool.id]);
const definitions = await toolService.getToolDefinitionsForAgent(agent.id);
expect(definitions.map((d) => d.id)).toEqual([ownTool.id]);
});
});
});

View File

@ -136,6 +136,85 @@ describe('ChatHubToolService', () => {
});
});
describe('setSessionTools', () => {
it('should set tools that the user owns', async () => {
const tool = makeTool();
chatToolRepository.getByIds.mockResolvedValue([tool]);
await service.setSessionTools(mockSessionId, mockUserId, [tool.id]);
expect(chatToolRepository.setSessionTools).toHaveBeenCalledWith(
mockSessionId,
[tool.id],
undefined,
);
});
it('should not set tools when the user does not own one of them', async () => {
chatToolRepository.getByIds.mockResolvedValue([]);
await expect(service.setSessionTools(mockSessionId, mockUserId, [uuid()])).rejects.toThrow(
NotFoundError,
);
expect(chatToolRepository.setSessionTools).not.toHaveBeenCalled();
});
it('should not set tools when the user owns only some of them', async () => {
const ownTool = makeTool();
chatToolRepository.getByIds.mockResolvedValue([ownTool]);
await expect(
service.setSessionTools(mockSessionId, mockUserId, [ownTool.id, uuid()]),
).rejects.toThrow(NotFoundError);
expect(chatToolRepository.setSessionTools).not.toHaveBeenCalled();
});
it('should check and store each tool once when IDs repeat', async () => {
const tool = makeTool();
chatToolRepository.getByIds.mockResolvedValue([tool]);
await service.setSessionTools(mockSessionId, mockUserId, [tool.id, tool.id]);
expect(chatToolRepository.getByIds).toHaveBeenCalledWith([tool.id], mockUserId, undefined);
expect(chatToolRepository.setSessionTools).toHaveBeenCalledWith(
mockSessionId,
[tool.id],
undefined,
);
});
it('should clear the tools without a lookup when the list is empty', async () => {
await service.setSessionTools(mockSessionId, mockUserId, []);
expect(chatToolRepository.getByIds).not.toHaveBeenCalled();
expect(chatToolRepository.setSessionTools).toHaveBeenCalledWith(mockSessionId, [], undefined);
});
});
describe('setAgentTools', () => {
it('should set tools that the user owns', async () => {
const tool = makeTool();
chatToolRepository.getByIds.mockResolvedValue([tool]);
await service.setAgentTools(mockAgentId, mockUserId, [tool.id]);
expect(chatToolRepository.setAgentTools).toHaveBeenCalledWith(
mockAgentId,
[tool.id],
undefined,
);
});
it('should not set tools when the user does not own one of them', async () => {
chatToolRepository.getByIds.mockResolvedValue([]);
await expect(service.setAgentTools(mockAgentId, mockUserId, [uuid()])).rejects.toThrow(
NotFoundError,
);
expect(chatToolRepository.setAgentTools).not.toHaveBeenCalled();
});
});
const deprecatedCodeTool: INode = {
...mockDefinition,
type: '@n8n/n8n-nodes-langchain.code',

View File

@ -1,6 +1,7 @@
import type { ChatHubConversationModel, ChatProviderSettingsDto } from '@n8n/api-types';
import type { PolicyCleared } from '@n8n/decorators';
import type { Logger } from '@n8n/backend-common';
import type { WorkflowRepository, SharedWorkflowRepository, User } from '@n8n/db';
import type { Project, WorkflowRepository, SharedWorkflowRepository, User } from '@n8n/db';
import type { EntityManager } from '@n8n/typeorm';
import type { Cipher, BinaryDataService } from 'n8n-core';
import { type IBinaryData, type INode, CHAT_TRIGGER_NODE_TYPE } from 'n8n-workflow';
@ -1409,7 +1410,7 @@ describe('ChatHubWorkflowService', () => {
vi.spyOn(serviceWithRepo, 'getSystemMessageMetadata').mockReturnValue('__metadata__');
const result = await serviceWithRepo.prepareReplyWorkflow(
{ id: 'user-123' } as User,
{ id: 'user-123', role: { slug: 'global:member' } } as User,
'session-456' as any,
{},
{ provider: 'custom-agent', agentId: 'agent-1' } as any,
@ -1508,4 +1509,78 @@ __metadata__"
`);
});
});
describe('tool type authorization', () => {
const toolNode = (name: string, type: string): INode => ({
id: `id-${name}`,
name,
type,
typeVersion: 1,
position: [0, 0],
parameters: {},
});
const storedTools = [
toolNode('Shell', 'n8n-nodes-base.executeCommandTool'),
toolNode('Code', '@n8n/n8n-nodes-langchain.toolCode'),
toolNode('Slack', 'n8n-nodes-base.slackTool'),
];
beforeEach(() => {
chatHubSettingsService.ensureModelIsAllowed.mockResolvedValue(undefined);
chatHubSettingsService.getProviderSettings.mockResolvedValue(mock<ChatProviderSettingsDto>());
chatHubSettingsService.getSemanticSearchOptions.mockResolvedValue(null);
chatHubCredentialsService.findPersonalProject.mockResolvedValue(
mock<Project>({ id: 'project-789' }),
);
});
async function getToolNodeNames(user: User, model: ChatHubConversationModel) {
const result = await service.prepareReplyWorkflow(
user,
'session-456',
{ openAiApi: { id: 'cred-1', name: '' } },
model,
[],
'Hello',
storedTools,
[],
'UTC',
{},
defaultExecutionMetadata,
);
const toolNames = new Set(storedTools.map((t) => t.name));
return result.workflowData.nodes.map((n) => n.name).filter((name) => toolNames.has(name));
}
const chatUser = { id: 'user-123', role: { slug: 'global:chatUser' } } as User;
const member = { id: 'user-123', role: { slug: 'global:member' } } as User;
const baseModel: ChatHubConversationModel = { provider: 'openai', model: 'gpt-4' };
it('should drop stored tools that a chat user is not allowed to run', async () => {
expect(await getToolNodeNames(chatUser, baseModel)).toEqual(['Slack']);
});
it('should keep all stored tools for a member', async () => {
expect(await getToolNodeNames(member, baseModel)).toEqual(['Shell', 'Code', 'Slack']);
});
it('should drop blocked tools from a chat user agent', async () => {
chatHubAgentRepository.getOneById.mockResolvedValue(
mock<ChatHubAgent>({
id: 'agent-1',
provider: 'openai',
model: 'gpt-4',
credentialId: 'cred-1',
systemPrompt: '',
files: [],
}),
);
chatHubToolService.getToolDefinitionsForAgent.mockResolvedValue(storedTools);
expect(
await getToolNodeNames(chatUser, { provider: 'custom-agent', agentId: 'agent-1' }),
).toEqual(['Slack']);
});
});
});

View File

@ -1,11 +1,15 @@
import type { ChatHubCreateToolRequest, ChatHubUpdateToolRequest } from '@n8n/api-types';
import type { ModuleRegistry } from '@n8n/backend-common';
import type { AuthenticatedRequest } from '@n8n/db';
import type { ModuleSettings } from '@n8n/decorators';
import type { NextFunction, Request, Response } from 'express';
import type { INode } from 'n8n-workflow';
import { mock } from 'vitest-mock-extended';
import { ForbiddenError } from '@n8n/errors';
import { BadRequestError, ForbiddenError } from '@n8n/errors';
import { sendErrorResponse } from '@/response-helper';
import type { ChatHubToolService } from '../chat-hub-tool.service';
import { ChatHubController } from '../chat-hub.controller';
vi.mock('@/response-helper');
@ -13,7 +17,15 @@ vi.mock('@/response-helper');
describe('ChatHubController', () => {
const settingsMap = new Map<string, ModuleSettings>();
const moduleRegistry = { settings: settingsMap } as unknown as ModuleRegistry;
const controller = new ChatHubController(mock(), mock(), mock(), mock(), mock(), moduleRegistry);
const chatToolService = mock<ChatHubToolService>();
const controller = new ChatHubController(
mock(),
mock(),
mock(),
chatToolService,
mock(),
moduleRegistry,
);
const req = mock<Request>();
const res = mock<Response>();
@ -51,4 +63,81 @@ describe('ChatHubController', () => {
expect(sendErrorResponse).toHaveBeenCalledWith(res, expect.any(ForbiddenError));
});
});
describe('tool type authorization', () => {
const chatUserReq = mock<AuthenticatedRequest>({
user: { id: 'chat-user', role: { slug: 'global:chatUser' } },
});
const memberReq = mock<AuthenticatedRequest>({
user: { id: 'member', role: { slug: 'global:member' } },
});
const toolDefinition = (type: string): INode => ({
id: '00000000-0000-4000-8000-000000000001',
name: 'Tool',
type,
typeVersion: 1,
position: [0, 0],
parameters: {},
});
const createPayload = (type: string) =>
({ definition: toolDefinition(type) }) as ChatHubCreateToolRequest;
const updatePayload = (type: string) =>
({ definition: toolDefinition(type) }) as ChatHubUpdateToolRequest;
beforeEach(() => {
chatToolService.createTool.mockResolvedValue(mock({ definition: toolDefinition('x') }));
chatToolService.updateTool.mockResolvedValue(mock({ definition: toolDefinition('x') }));
});
it.each([
'n8n-nodes-base.executeCommandTool',
'n8n-nodes-base.gitTool',
'@n8n/n8n-nodes-langchain.toolCode',
'n8n-nodes-base.messageAnAgentTool',
])('should reject creating a %s tool for a chat user', async (type) => {
await expect(controller.createTool(chatUserReq, res, createPayload(type))).rejects.toThrow(
BadRequestError,
);
expect(chatToolService.createTool).not.toHaveBeenCalled();
});
it('should reject updating a tool to a blocked type for a chat user', async () => {
await expect(
controller.updateTool(
chatUserReq,
res,
'tool-1',
updatePayload('n8n-nodes-base.executeCommandTool'),
),
).rejects.toThrow(BadRequestError);
expect(chatToolService.updateTool).not.toHaveBeenCalled();
});
it.each(['n8n-nodes-base.slackTool', 'n8n-nodes-base.httpRequestTool'])(
'should allow creating a %s tool for a chat user',
async (type) => {
await controller.createTool(chatUserReq, res, createPayload(type));
expect(chatToolService.createTool).toHaveBeenCalled();
},
);
it('should allow creating a Code tool for a member', async () => {
await controller.createTool(
memberReq,
res,
createPayload('@n8n/n8n-nodes-langchain.toolCode'),
);
expect(chatToolService.createTool).toHaveBeenCalled();
});
it('should reject creating a Chat tool for a member', async () => {
await expect(
controller.createTool(memberReq, res, createPayload('@n8n/n8n-nodes-langchain.chatTool')),
).rejects.toThrow(BadRequestError);
});
});
});

View File

@ -98,26 +98,34 @@ export class ChatHubAgentService {
const id = uuidv4();
const agent = await this.chatAgentRepository.createAgent({
id,
name: data.name,
description: data.description ?? null,
icon: data.icon,
suggestedPrompts: data.suggestedPrompts ?? [],
systemPrompt: data.systemPrompt,
ownerId: user.id,
credentialId: data.credentialId,
provider: data.provider,
model: data.model,
files: [],
// One transaction: a rejected tool removes the new agent again
const agent = await this.chatAgentRepository.runInTransaction({}, async (trx) => {
const created = await this.chatAgentRepository.createAgent(
{
id,
name: data.name,
description: data.description ?? null,
icon: data.icon,
suggestedPrompts: data.suggestedPrompts ?? [],
systemPrompt: data.systemPrompt,
ownerId: user.id,
credentialId: data.credentialId,
provider: data.provider,
model: data.model,
files: [],
},
trx,
);
if (data.toolIds.length > 0) {
await this.chatHubToolService.setAgentTools(id, user.id, data.toolIds, trx);
}
return created;
});
if (data.toolIds.length > 0) {
await this.chatHubToolService.setAgentTools(id, data.toolIds);
}
this.logger.debug(`Chat agent created: ${id} by user ${user.id}`);
return this.toDto(agent, data.toolIds);
return this.toDto(agent, [...new Set(data.toolIds)]);
}
async updateAgent(
@ -148,11 +156,14 @@ export class ChatHubAgentService {
if (updates.provider !== undefined) updateData.provider = updates.provider;
if (updates.model !== undefined) updateData.model = updates.model ?? null;
const agent = await this.chatAgentRepository.updateAgent(id, updateData);
if (updates.toolIds !== undefined) {
await this.chatHubToolService.setAgentTools(id, updates.toolIds);
}
// One transaction: a rejected tool or a failed field update leaves the agent unchanged
const { toolIds: newToolIds } = updates;
const agent = await this.chatAgentRepository.runInTransaction({}, async (trx) => {
if (newToolIds !== undefined) {
await this.chatHubToolService.setAgentTools(id, user.id, newToolIds, trx);
}
return await this.chatAgentRepository.updateAgent(id, updateData, trx);
});
this.logger.debug(`Chat agent updated: ${id} by user ${user.id}`);
const toolIds = await this.chatHubToolService.getToolIdsForAgent(agent.id);

View File

@ -69,7 +69,7 @@ export class ChatHubToolRepository extends Repository<ChatHubTool> {
const em = trx ?? this.manager;
const session = await em
.createQueryBuilder(ChatHubSession, 'session')
.innerJoinAndSelect('session.tools', 'tool')
.innerJoinAndSelect('session.tools', 'tool', 'tool.ownerId = session.ownerId')
.where('session.id = :sessionId', { sessionId })
.orderBy('tool.createdAt', 'ASC')
.getOne();
@ -80,7 +80,7 @@ export class ChatHubToolRepository extends Repository<ChatHubTool> {
const em = trx ?? this.manager;
const agent = await em
.createQueryBuilder(ChatHubAgent, 'agent')
.innerJoinAndSelect('agent.tools', 'tool')
.innerJoinAndSelect('agent.tools', 'tool', 'tool.ownerId = agent.ownerId')
.where('agent.id = :agentId', { agentId })
.orderBy('tool.createdAt', 'ASC')
.getOne();
@ -91,7 +91,7 @@ export class ChatHubToolRepository extends Repository<ChatHubTool> {
const em = trx ?? this.manager;
const rows = await em
.createQueryBuilder(ChatHubSession, 'session')
.innerJoin('session.tools', 'tool')
.innerJoin('session.tools', 'tool', 'tool.ownerId = session.ownerId')
.select('tool.id', 'toolId')
.where('session.id = :sessionId', { sessionId })
.getRawMany<{ toolId: string }>();
@ -102,7 +102,7 @@ export class ChatHubToolRepository extends Repository<ChatHubTool> {
const em = trx ?? this.manager;
const rows = await em
.createQueryBuilder(ChatHubAgent, 'agent')
.innerJoin('agent.tools', 'tool')
.innerJoin('agent.tools', 'tool', 'tool.ownerId = agent.ownerId')
.select('tool.id', 'toolId')
.where('agent.id = :agentId', { agentId })
.getRawMany<{ toolId: string }>();
@ -110,22 +110,28 @@ export class ChatHubToolRepository extends Repository<ChatHubTool> {
}
async setSessionTools(sessionId: string, toolIds: string[], trx?: EntityManager): Promise<void> {
const em = trx ?? this.manager;
const currentToolIds = await this.getToolIdsForSession(sessionId, em);
await em
.createQueryBuilder()
.relation(ChatHubSession, 'tools')
.of(sessionId)
.addAndRemove(toolIds, currentToolIds);
await this.replaceLinkedTools(ChatHubSession, sessionId, toolIds, trx ?? this.manager);
}
async setAgentTools(agentId: string, toolIds: string[], trx?: EntityManager): Promise<void> {
const em = trx ?? this.manager;
const currentToolIds = await this.getToolIdsForAgent(agentId, em);
await em
.createQueryBuilder()
.relation(ChatHubAgent, 'tools')
.of(agentId)
.addAndRemove(toolIds, currentToolIds);
await this.replaceLinkedTools(ChatHubAgent, agentId, toolIds, trx ?? this.manager);
}
/**
* Replaces every link row, including rows to tools that the owner does not own,
* so that a save also removes those rows.
*/
private async replaceLinkedTools(
entity: typeof ChatHubSession | typeof ChatHubAgent,
id: string,
toolIds: string[],
em: EntityManager,
): Promise<void> {
const relation = em.createQueryBuilder().relation(entity, 'tools').of(id);
const currentTools = await relation.loadMany<ChatHubTool>();
await relation.addAndRemove(
toolIds,
currentTools.map((t) => t.id),
);
}
}

View File

@ -53,12 +53,41 @@ export class ChatHubToolService {
return await this.chatToolRepository.getToolIdsForAgent(agentId, trx);
}
async setSessionTools(sessionId: string, toolIds: string[], trx?: EntityManager): Promise<void> {
await this.chatToolRepository.setSessionTools(sessionId, toolIds, trx);
async setSessionTools(
sessionId: string,
ownerId: string,
toolIds: string[],
trx?: EntityManager,
): Promise<void> {
await this.ensureToolsOwnedBy(ownerId, toolIds, trx);
await this.chatToolRepository.setSessionTools(sessionId, [...new Set(toolIds)], trx);
}
async setAgentTools(agentId: string, toolIds: string[], trx?: EntityManager): Promise<void> {
await this.chatToolRepository.setAgentTools(agentId, toolIds, trx);
async setAgentTools(
agentId: string,
ownerId: string,
toolIds: string[],
trx?: EntityManager,
): Promise<void> {
await this.ensureToolsOwnedBy(ownerId, toolIds, trx);
await this.chatToolRepository.setAgentTools(agentId, [...new Set(toolIds)], trx);
}
/**
* Throws `NotFoundError` unless the user owns every tool in `toolIds`.
*/
private async ensureToolsOwnedBy(
userId: string,
toolIds: string[],
trx?: EntityManager,
): Promise<void> {
const uniqueIds = [...new Set(toolIds)];
if (uniqueIds.length === 0) return;
const ownedTools = await this.chatToolRepository.getByIds(uniqueIds, userId, trx);
if (ownedTools.length !== uniqueIds.length) {
throw new NotFoundError('Chat hub tool not found');
}
}
private validateToolExpressions(definition: INode) {

View File

@ -5,6 +5,7 @@ import {
type ChatHubBaseLLMModel,
type ChatProviderSettingsDto,
type ChatHubAgentKnowledgeItem,
isChatHubToolTypeAllowed,
} from '@n8n/api-types';
import { Logger } from '@n8n/backend-common';
import {
@ -1361,7 +1362,7 @@ Respond the title only:`,
credentials,
model,
systemMessage,
tools,
this.filterAllowedTools(user, tools),
timeZone,
vectorStoreSearch,
executionMetadata,
@ -1371,6 +1372,22 @@ Respond the title only:`,
});
}
/**
* Stored tools can predate a tool type rule or a role change, so check them
* again before each run.
*/
private filterAllowedTools(user: User, tools: INode[]): INode[] {
const isChatUser = user.role.slug === 'global:chatUser';
return tools.filter((tool) => {
if (isChatHubToolTypeAllowed(tool.type, { isChatUser })) return true;
this.logger.warn('Skipped a Chat Hub tool that the user is not allowed to run', {
userId: user.id,
toolType: tool.type,
});
return false;
});
}
private async prepareChatAgentWorkflow(
agentId: string,
user: User,

View File

@ -20,8 +20,7 @@ import {
type ChatSendMessageResponse,
type ChatReconnectResponse,
ChatReconnectRequest,
ALWAYS_BLOCKED_CHAT_HUB_TOOL_TYPES,
CHAT_USER_BLOCKED_CHAT_HUB_TOOL_TYPES,
isChatHubToolTypeAllowed,
} from '@n8n/api-types';
import { ModuleRegistry } from '@n8n/backend-common';
import { AuthenticatedRequest } from '@n8n/db';
@ -518,13 +517,10 @@ export class ChatHubController {
}
private assertToolTypeAllowed(type: string, user: AuthenticatedRequest['user']) {
if (ALWAYS_BLOCKED_CHAT_HUB_TOOL_TYPES.includes(type)) {
if (!isChatHubToolTypeAllowed(type, { isChatUser: false })) {
throw new BadRequestError(`Tool type "${type}" is not supported in the Chat Hub`);
}
if (
user.role.slug === 'global:chatUser' &&
CHAT_USER_BLOCKED_CHAT_HUB_TOOL_TYPES.includes(type)
) {
if (!isChatHubToolTypeAllowed(type, { isChatUser: user.role.slug === 'global:chatUser' })) {
throw new BadRequestError(`Tool type "${type}" is not available for your role`);
}
}

View File

@ -240,7 +240,7 @@ export class ChatHubService {
const enabledTools = await this.chatHubToolService.getEnabledTools(user.id, trx);
const toolIds = enabledTools.map((t) => t.id);
if (toolIds.length > 0) {
await this.chatHubToolService.setSessionTools(sessionId, toolIds, trx);
await this.chatHubToolService.setSessionTools(sessionId, user.id, toolIds, trx);
}
return session;
@ -378,11 +378,14 @@ export class ChatHubService {
if (updates.title !== undefined) sessionUpdates.title = updates.title;
if (updates.credentialId !== undefined) sessionUpdates.credentialId = updates.credentialId;
await this.sessionRepository.updateChatSession(sessionId, sessionUpdates);
if (updates.toolIds !== undefined) {
await this.chatHubToolService.setSessionTools(sessionId, updates.toolIds);
}
// One transaction: a rejected tool or a failed field update leaves the session unchanged
const { toolIds } = updates;
await this.sessionRepository.runInTransaction({}, async (trx) => {
if (toolIds !== undefined) {
await this.chatHubToolService.setSessionTools(sessionId, user.id, toolIds, trx);
}
await this.sessionRepository.updateChatSession(sessionId, sessionUpdates, trx);
});
}
/**

View File

@ -1,6 +1,7 @@
import type { ListDataTableQueryDto } from '@n8n/api-types';
import type { Logger } from '@n8n/backend-common';
import { createTeamProject, testDb, testModules } from '@n8n/backend-test-utils';
import { CacheService } from '@n8n/backend-services';
import { createTeamProject, createWorkflow, testDb, testModules } from '@n8n/backend-test-utils';
import type { Project, User } from '@n8n/db';
import { Container } from '@n8n/di';
import type {
@ -17,7 +18,7 @@ import { mock } from 'vitest-mock-extended';
import type { InstanceWriteAccessService } from '@n8n/backend-services';
import * as checkAccess from '@/permissions.ee/check-access';
import type { OwnershipService } from '@/services/ownership.service';
import { OwnershipService } from '@/services/ownership.service';
import type { DataTableAggregateService } from '../data-table-aggregate.service';
import { DataTableProxyService } from '../data-table-proxy.service';
@ -299,7 +300,15 @@ describe('DataTableProxyService (with database)', () => {
const workflow = mock<Workflow>({ id: 'workflow-id' });
afterEach(async () => {
await testDb.truncate(['DataTable', 'DataTableColumn', 'Project', 'ProjectRelation']);
await testDb.truncate([
'DataTable',
'DataTableColumn',
'SharedWorkflow',
'WorkflowEntity',
'Project',
'ProjectRelation',
]);
await Container.get(CacheService).delete('workflow-project');
});
it('resolves the proxy when the data table exists in the project', async () => {
@ -332,6 +341,29 @@ describe('DataTableProxyService (with database)', () => {
await expect(promise).rejects.toBeInstanceOf(NodeOperationError);
await expect(promise).rejects.toThrow(table.id);
});
it('rejects a table from another project for a workflow with the id `__proto__`', async () => {
const workflowProject = await createTeamProject();
const otherProject = await createTeamProject();
const table = await Container.get(DataTableService).createDataTable(otherProject.id, {
name: 'Other Project Table',
columns: [],
});
await createWorkflow({ id: '__proto__' }, workflowProject);
// The ownership cache must already exist, as it does on any instance that ran a workflow
await Container.get(CacheService).setHash('workflow-project', {
'other-workflow': Container.get(OwnershipService).copyProject(workflowProject),
});
const promise = Container.get(DataTableProxyService).getDataTableProxy(
mock<Workflow>({ id: '__proto__' }),
node,
table.id,
);
await expect(promise).rejects.toBeInstanceOf(NodeOperationError);
await expect(promise).rejects.toThrow(table.id);
});
});
describe('makeDataTableOperationsForUser', () => {

View File

@ -0,0 +1,127 @@
import {
createTeamProject,
createWorkflow,
linkUserToProject,
testDb,
} from '@n8n/backend-test-utils';
import { ExecutionRepository } from '@n8n/db';
import type { User, WorkflowEntity } from '@n8n/db';
import { Container } from '@n8n/di';
import { mock } from 'vitest-mock-extended';
import { ExecutionListService } from '@/executions/execution-list.service';
import { ExecutionRedactionServiceProxy } from '@/executions/execution-redaction-proxy.service';
import type { Telemetry } from '@/telemetry';
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
import { createSuccessfulExecution } from '@test-integration/db/executions';
import { createCustomRoleWithScopeSlugs } from '@test-integration/db/roles';
import { createMember, createOwner } from '@test-integration/db/users';
import { createGetExecutionTool } from '../tools/get-execution.tool';
import { createSearchExecutionsTool } from '../tools/search-executions.tool';
let workflow: WorkflowEntity;
let executionId: string;
let workflowReader: User;
let executionReader: User;
beforeAll(async () => {
await testDb.init();
const owner = await createOwner();
const project = await createTeamProject('MCP execution scopes', owner);
workflow = await createWorkflow({ settings: { availableInMCP: true } }, project);
executionId = (await createSuccessfulExecution(workflow)).id;
const workflowOnlyRole = await createCustomRoleWithScopeSlugs(['workflow:read'], {
roleType: 'project',
});
workflowReader = await createMember();
await linkUserToProject(workflowReader, project, workflowOnlyRole.slug);
executionReader = await createMember();
await linkUserToProject(executionReader, project, 'project:viewer');
});
afterAll(async () => {
await testDb.terminate();
});
const getExecution = async (user: User, includeData: boolean) =>
await createGetExecutionTool(
user,
Container.get(ExecutionRepository),
Container.get(WorkflowFinderService),
mock<Telemetry>(),
Container.get(ExecutionRedactionServiceProxy),
).handler(
{
workflowId: workflow.id,
executionId,
includeData,
nodeNames: undefined,
truncateData: undefined,
},
{} as never,
);
const searchExecutions = async (user: User, workflowId?: string) =>
await createSearchExecutionsTool(
user,
Container.get(ExecutionListService),
Container.get(WorkflowFinderService),
mock<Telemetry>(),
).handler({ workflowId }, {} as never);
describe('get_workflow_execution project role scopes', () => {
test.each([true, false])(
'returns no execution for a role without execution:read (includeData: %s)',
async (includeData) => {
const result = await getExecution(workflowReader, includeData);
expect(result.structuredContent).toMatchObject({
execution: null,
error: "Workflow not found or you don't have permission to access it.",
});
expect(result.structuredContent).not.toHaveProperty('data');
},
);
test('returns the execution for a role with execution:read', async () => {
const result = await getExecution(executionReader, false);
expect(result.structuredContent).toMatchObject({
execution: { id: executionId, workflowId: workflow.id },
});
});
});
describe('search_workflow_executions project role scopes', () => {
test('returns an access error for a role without execution:read when filtering by workflow', async () => {
const result = await searchExecutions(workflowReader, workflow.id);
expect(result.isError).toBe(true);
expect(result.structuredContent).toMatchObject({
data: [],
error: "Workflow not found or you don't have permission to access it.",
});
});
test('returns no executions for a role without execution:read', async () => {
const result = await searchExecutions(workflowReader);
expect(result.structuredContent).toMatchObject({ data: [], count: 0 });
expect(result.structuredContent).not.toHaveProperty('error');
});
test.each([undefined, 'workflow'] as const)(
'returns the execution for a role with execution:read (filter: %s)',
async (filter) => {
const result = await searchExecutions(executionReader, filter ? workflow.id : undefined);
expect(result.structuredContent).toMatchObject({
data: [expect.objectContaining({ id: executionId, workflowId: workflow.id })],
});
},
);
});

View File

@ -57,6 +57,74 @@ describe('get-execution MCP tool', () => {
});
describe('handler tests', () => {
describe('access checks', () => {
test.each([true, false])(
'requires workflow:read and execution:read on the workflow (includeData: %s)',
async (includeData) => {
const tool = createGetExecutionTool(
user,
executionRepository,
workflowFinderService,
telemetry,
executionRedactionServiceProxy,
);
(workflowFinderService.findWorkflowForUser as Mock).mockResolvedValue(mockWorkflow);
(executionPersistence.findWithUnflattenedData as Mock).mockResolvedValue(null);
(executionRepository.findIfAccessible as Mock).mockResolvedValue(null);
await tool.handler(
{
workflowId: 'workflow-1',
executionId: 'execution-1',
includeData,
nodeNames: undefined,
truncateData: undefined,
},
{} as never,
);
expect(workflowFinderService.findWorkflowForUser).toHaveBeenCalledWith(
'workflow-1',
user,
['workflow:read', 'execution:read'],
{},
);
},
);
test.each([true, false])(
'does not load the execution when the workflow lookup fails (includeData: %s)',
async (includeData) => {
const tool = createGetExecutionTool(
user,
executionRepository,
workflowFinderService,
telemetry,
executionRedactionServiceProxy,
);
(workflowFinderService.findWorkflowForUser as Mock).mockResolvedValue(null);
const result = await tool.handler(
{
workflowId: 'workflow-1',
executionId: 'execution-1',
includeData,
nodeNames: undefined,
truncateData: undefined,
},
{} as never,
);
expect(executionPersistence.findWithUnflattenedData).not.toHaveBeenCalled();
expect(executionRepository.findIfAccessible).not.toHaveBeenCalled();
expect(result.structuredContent).toMatchObject({ execution: null });
expect(result.structuredContent).not.toHaveProperty('data');
},
);
});
describe('execution retrieval', () => {
test('returns generic error when execution is not found for workflow', async () => {
const tool = createGetExecutionTool(

View File

@ -35,7 +35,7 @@ describe('search-executions MCP tool', () => {
beforeEach(() => {
executionListService = mockInstance(ExecutionListService, {
buildSharingOptions: vi.fn().mockResolvedValue({
scopes: ['workflow:read'],
scopes: ['execution:read'],
projectRoles: ['project:editor'],
workflowRoles: ['workflow:editor'],
}),
@ -123,7 +123,7 @@ describe('search-executions MCP tool', () => {
expect(workflowFinderService.findWorkflowForUser).toHaveBeenCalledWith(
'wf-1',
user,
['workflow:read'],
['workflow:read', 'execution:read'],
{ includeActiveVersion: undefined },
);
@ -203,10 +203,32 @@ describe('search-executions MCP tool', () => {
});
});
test('returns an access error and runs no query when the workflow lookup fails', async () => {
(workflowFinderService.findWorkflowForUser as Mock).mockResolvedValue(null);
const result = await createTool().handler({ workflowId: 'wf-1' } as never, {} as never);
expect(result.isError).toBe(true);
expect(result.structuredContent).toEqual({
data: [],
count: 0,
estimated: false,
nextCursor: null,
error: "Workflow not found or you don't have permission to access it.",
});
expect(executionListService.findPageWithCount).not.toHaveBeenCalled();
});
test('delegates sharing options to the list service', async () => {
await createTool().handler({} as never, {} as never);
expect(executionListService.buildSharingOptions).toHaveBeenCalledWith('workflow:read');
expect(executionListService.buildSharingOptions).toHaveBeenCalledWith('execution:read');
const query = (executionListService.findPageWithCount as Mock).mock.calls[0][0];
expect(query.sharingOptions).toEqual({
scopes: ['execution:read'],
projectRoles: ['project:editor'],
workflowRoles: ['workflow:editor'],
});
});
test('tracks telemetry on success', async () => {

View File

@ -98,7 +98,12 @@ export const createGetExecutionTool = (
};
try {
await getMcpWorkflow(workflowId, user, ['workflow:read'], workflowFinderService);
await getMcpWorkflow(
workflowId,
user,
['workflow:read', 'execution:read'],
workflowFinderService,
);
// Use lightweight metadata query when data isn't needed;
// split into branches so TypeScript can narrow each return type.

View File

@ -113,11 +113,16 @@ export const createSearchExecutionsTool = (
try {
// Validate workflow access if workflowId is provided
if (workflowId) {
await getMcpWorkflow(workflowId, user, ['workflow:read'], workflowFinderService);
await getMcpWorkflow(
workflowId,
user,
['workflow:read', 'execution:read'],
workflowFinderService,
);
}
const safeLimit = Math.min(Math.max(1, limit), MAX_RESULTS);
const sharingOptions = await executionListService.buildSharingOptions('workflow:read');
const sharingOptions = await executionListService.buildSharingOptions('execution:read');
const parsedCursor = parseExecutionCursor(cursor);
const query = {

View File

@ -4,6 +4,7 @@ import type { PolicyCleared, PolicyViolation } from '@n8n/decorators';
import {
type Variables,
type VariablesRepository,
Folder,
type FolderRepository,
CredentialsEntity,
type CredentialsRepository,
@ -99,6 +100,7 @@ describe('SourceControlImportService', () => {
const executionPersistence = mock<ExecutionPersistence>();
const credentialsService = mock<CredentialsService>();
const transactionManager = mock<EntityManager>();
const cascadeManager = mock<EntityManager>();
const globalAdminContext = new SourceControlContext(
Object.assign(new User(), { role: GLOBAL_ADMIN_ROLE }),
@ -166,6 +168,14 @@ describe('SourceControlImportService', () => {
transactionManager.delete.mockImplementation(
async (_entity, criteria) => await sharedCredentialsRepository.delete(criteria as never),
);
cascadeManager.delete.mockImplementation(async (entity, criteria) =>
entity === Folder
? await folderRepository.delete(criteria as never)
: await projectRepository.delete(criteria as never),
);
workflowRepository.runInTransaction.mockImplementation(
async (ctx, fn) => await fn(cascadeManager, ctx),
);
credentialsRepositoryManager.transaction.mockImplementation(async (...args) => {
const callback = args.find((arg) => typeof arg === 'function');
if (!callback) throw new Error('Transaction callback is required');
@ -3463,6 +3473,48 @@ describe('SourceControlImportService', () => {
).toBeGreaterThan(folderRepository.delete.mock.invocationCallOrder[0]);
});
it('should purge caller allowlists in the folder delete transaction', async () => {
const candidates = [mock<SourceControlledFile>({ id: 'folder1' })];
const straggler = Object.assign(new WorkflowEntity(), {
id: 'wf-1',
activeVersionId: null,
});
folderRepository.getAllFolderIdsInHierarchy.mockResolvedValueOnce([]);
workflowRepository.find.mockResolvedValueOnce([straggler]);
workflowRepository.findOne.mockResolvedValueOnce(straggler);
await service.deleteFoldersNotInWorkfolder(candidates);
expect(workflowRepository.runInTransaction).toHaveBeenCalledTimes(1);
expect(workflowRepository.removeMissingFromCallerAllowlists).toHaveBeenCalledWith({});
expect(
workflowRepository.removeMissingFromCallerAllowlists.mock.invocationCallOrder[0],
).toBeGreaterThan(folderRepository.delete.mock.invocationCallOrder[0]);
expect(
workflowRepository.removeMissingFromCallerAllowlists.mock.invocationCallOrder[0],
).toBeLessThan(workflowMutationHooks.afterWorkflowsDeleted.mock.invocationCallOrder[0]);
});
it('should fail the folder deletion when the caller allowlist purge fails', async () => {
const candidates = [mock<SourceControlledFile>({ id: 'folder1' })];
const straggler = Object.assign(new WorkflowEntity(), {
id: 'wf-1',
activeVersionId: null,
});
folderRepository.getAllFolderIdsInHierarchy.mockResolvedValueOnce([]);
workflowRepository.find.mockResolvedValueOnce([straggler]);
workflowRepository.findOne.mockResolvedValueOnce(straggler);
workflowRepository.removeMissingFromCallerAllowlists.mockRejectedValueOnce(
new Error('statement timeout'),
);
await expect(service.deleteFoldersNotInWorkfolder(candidates)).rejects.toThrow(
'statement timeout',
);
expect(workflowMutationHooks.afterWorkflowsDeleted).not.toHaveBeenCalled();
});
it('should not fire the sweep when the deleted folders contained no workflows', async () => {
const candidates = [mock<SourceControlledFile>({ id: 'folder1' })];
folderRepository.getAllFolderIdsInHierarchy.mockResolvedValueOnce([]);
@ -4132,6 +4184,32 @@ describe('SourceControlImportService', () => {
).toBeGreaterThan(projectRepository.delete.mock.invocationCallOrder[0]);
});
it('should purge caller allowlists in the project delete transaction', async () => {
const candidates = [mock<SourceControlledFile>({ id: 'project-1' })];
sharedWorkflowRepository.find.mockResolvedValueOnce([
{ workflowId: 'wf-in-folder' },
{ workflowId: 'wf-at-project-root' },
] as SharedWorkflow[]);
workflowRepository.findOne
.mockResolvedValueOnce(
Object.assign(new WorkflowEntity(), { id: 'wf-in-folder', activeVersionId: null }),
)
.mockResolvedValueOnce(
Object.assign(new WorkflowEntity(), {
id: 'wf-at-project-root',
activeVersionId: null,
}),
);
await service.deleteTeamProjectsNotInWorkfolder(user, candidates, []);
expect(workflowRepository.runInTransaction).toHaveBeenCalledTimes(1);
expect(workflowRepository.removeMissingFromCallerAllowlists).toHaveBeenCalledWith({});
expect(
workflowRepository.removeMissingFromCallerAllowlists.mock.invocationCallOrder[0],
).toBeGreaterThan(projectRepository.delete.mock.invocationCallOrder[0]);
});
it('should not fire the sweep when the deleted projects contained no workflows', async () => {
const candidates = [mock<SourceControlledFile>({ id: 'project-1' })];
sharedWorkflowRepository.find.mockResolvedValueOnce([]);

View File

@ -1,16 +1,11 @@
import type { SourceControlledFile, WorkflowPublishBlockedDetails } from '@n8n/api-types';
import { Logger } from '@n8n/backend-common';
import type {
FindOptionsWhere,
Project,
TagEntity,
User,
Variables,
WorkflowEntity,
} from '@n8n/db';
import type { FindOptionsWhere, TagEntity, User, Variables, WorkflowEntity } from '@n8n/db';
import {
CredentialsRepository,
Folder,
FolderRepository,
Project,
ProjectRelationRepository,
ProjectRepository,
SharedCredentialsRepository,
@ -1916,8 +1911,8 @@ export class SourceControlImportService {
workflows.map((workflow) => workflow.id),
);
await this.folderRepository.delete({
id: In(candidateIds),
await this.deleteWithCallerAllowlistPurge(async (trx) => {
await trx.delete(Folder, { id: In(candidateIds) });
});
await this.sweepAfterWorkflowCascade(cascadedWorkflowIds);
@ -1967,8 +1962,8 @@ export class SourceControlImportService {
ownedWorkflows.map((sw) => sw.workflowId),
);
await this.projectRepository.delete({
id: In(candidateIds),
await this.deleteWithCallerAllowlistPurge(async (trx) => {
await trx.delete(Project, { id: In(candidateIds) });
});
await this.sweepAfterWorkflowCascade(cascadedWorkflowIds);
@ -2080,6 +2075,19 @@ export class SourceControlImportService {
await this.workflowMutationHooks.afterWorkflowsDeleted(cascadedWorkflowIds);
}
/**
* Run a folder/project row delete and drop every id left without a row from the `callerIds`
* allowlists, in one transaction, so no new workflow can take a freed id first.
*/
private async deleteWithCallerAllowlistPurge(deleteRows: (trx: EntityManager) => Promise<void>) {
await this.workflowRepository.runInTransaction({}, async (trx, ctx) => {
await deleteRows(trx);
// Purge by what is gone rather than by the ids collected before the delete: the cascade
// also removes a workflow moved into a deleted folder since.
await this.workflowRepository.removeMissingFromCallerAllowlists(ctx);
});
}
/** Contextual error for a failed deletion during pull, so the operator learns which resource to look at. */
private deletionError(
type: 'workflow' | 'folder' | 'project',

View File

@ -14,7 +14,7 @@ import { FORM_TRIGGER_PATH_IDENTIFIER, NodeApiError } from 'n8n-workflow';
import { Readable } from 'node:stream';
import picocolors from 'picocolors';
import { applyFormSandboxCSP } from './webhooks/webhook-response-headers';
import { applySandboxCSP } from './webhooks/webhook-response-headers';
export function sendSuccessResponse(
res: Response,
@ -65,7 +65,8 @@ export function sendErrorResponse(res: Response, error: Error) {
if (isFormTrigger || isLegacyFormTrigger) {
const isTestWebhook = basePath.includes('test');
res.status(404);
applyFormSandboxCSP(res);
// The setting of the route decides: a webhook path can look like a form path.
applySandboxCSP(res, 'form');
return res.render('form-trigger-404', { isTestWebhook });
}
}
@ -73,7 +74,7 @@ export function sendErrorResponse(res: Response, error: Error) {
if (error.errorCode === 409 && originalUrl && originalUrl.includes('form-waiting')) {
//codes other than 200 breaks redirection to form-waiting page from form trigger
//render form page instead of json
applyFormSandboxCSP(res);
applySandboxCSP(res, 'form');
return res.render('form-trigger-409', {
message: error.message,
});
@ -95,6 +96,8 @@ export function sendErrorResponse(res: Response, error: Error) {
response.stacktrace = error.stack;
}
// res.json() does not override a Content-Type set earlier; force JSON here.
res.setHeader('Content-Type', 'application/json');
res.status(status).json(response);
}

View File

@ -70,6 +70,20 @@ describe('OwnershipService', () => {
expect(returnedProject).toBe(mockProject);
});
test('should ignore a cached project without an id and read the project from the database', async () => {
const mockProject = Object.assign(new Project(), { id: 'project-id' });
// A cached value without an id, as a lookup of the key `__proto__` used to return
cacheService.getHashValue.mockResolvedValueOnce({});
sharedWorkflowRepository.findOneOrFail.mockResolvedValueOnce(
Object.assign(new SharedWorkflow(), { role: 'workflow:owner', project: mockProject }),
);
const returnedProject = await ownershipService.getWorkflowProjectCached('some-workflow-id');
expect(returnedProject).toBe(mockProject);
expect(sharedWorkflowRepository.findOneOrFail).toHaveBeenCalled();
});
test('should throw if no workflow owner project found', async () => {
sharedWorkflowRepository.findOneOrFail.mockRejectedValue(new Error());

View File

@ -34,6 +34,23 @@ export class OwnershipService {
private settingsRepository: SettingsRepository,
) {}
// To make use of the cache service we should store POJOs, these
// methods should be used to create copies of the entities
// converting them into plain JavaScript objects
// Ideally our data entities wouldn't be classes, but plain
// interfaces and therefore pojos adhering to these interfaces
copyProject(project: Project): Partial<Project> {
return {
...project,
};
}
reconstructProject(project: Partial<Project>): Project | undefined {
if (typeof project?.id !== 'string') {
return undefined;
}
return Object.assign(new Project(), project);
}
copyUser(user: User): Partial<User> {
return {
...user,

View File

@ -1,7 +1,11 @@
import { Logger } from '@n8n/backend-common';
import { mockInstance } from '@n8n/backend-test-utils';
import { type Response } from 'express';
import { isWebhookHtmlSandboxingDisabled, getHtmlSandboxCSP } from 'n8n-core';
import {
isFormHtmlSandboxingDisabled,
isWebhookHtmlSandboxingDisabled,
getHtmlSandboxCSP,
} from 'n8n-core';
import { OperationalError, randomString } from 'n8n-workflow';
import type { IHttpRequestMethods } from 'n8n-workflow';
import { mock } from 'vitest-mock-extended';
@ -18,6 +22,7 @@ import type {
vi.mock('n8n-core', async () => ({
...(await vi.importActual<typeof import('n8n-core')>('n8n-core')),
isWebhookHtmlSandboxingDisabled: vi.fn().mockReturnValue(false),
isFormHtmlSandboxingDisabled: vi.fn().mockReturnValue(false),
getHtmlSandboxCSP: vi.fn().mockReturnValue('sandbox allow-downloads allow-forms allow-modals'),
}));
@ -335,7 +340,7 @@ describe('WebhookRequestHandler', () => {
});
it('should not set CSP sandbox header when sandboxing is disabled', async () => {
vi.mocked(isWebhookHtmlSandboxingDisabled).mockReturnValueOnce(true);
vi.mocked(isWebhookHtmlSandboxingDisabled).mockReturnValue(true);
const req = mock<WebhookRequest>({
path: '/',
@ -354,7 +359,174 @@ describe('WebhookRequestHandler', () => {
await handler(req, res);
expect(res.setHeader).not.toHaveBeenCalledWith('Content-Security-Policy', expect.anything());
expect(res.setHeader).not.toHaveBeenCalledWith(
'Content-Security-Policy',
getHtmlSandboxCSP(),
);
});
describe('on every exit path', () => {
const sandboxCSP = 'sandbox allow-scripts';
const cspSetBefore = (res: ReturnType<typeof mock<Response>>, writer: 'json' | 'send') => {
const cspCall = res.setHeader.mock.calls.findIndex(
([name]) => name === 'Content-Security-Policy',
);
expect(cspCall).not.toBe(-1);
expect(res.setHeader.mock.calls[cspCall][1]).toBe(sandboxCSP);
expect(res.setHeader.mock.invocationCallOrder[cspCall]).toBeLessThan(
res[writer].mock.invocationCallOrder[0],
);
};
beforeEach(() => {
vi.mocked(getHtmlSandboxCSP).mockReturnValue(sandboxCSP);
});
it('should set the sandbox header before an error response is sent', async () => {
const req = mock<WebhookRequest>({ path: '/', method: 'GET', params: { path: 'test' } });
const res = mock<Response>();
res.status.mockReturnValue(res);
webhookManager.executeWebhook.mockRejectedValueOnce(new OperationalError('Test error'));
await handler(req, res);
expect(res.status).toHaveBeenCalledWith(500);
cspSetBefore(res, 'json');
});
it('should set the sandbox header on a legacy response without headers', async () => {
const req = mock<WebhookRequest>({ path: '/', method: 'GET', params: { path: 'test' } });
const res = mock<Response>();
// A node can pass a plain string body to `sendResponse()`, without a headers object.
webhookManager.executeWebhook.mockResolvedValueOnce({
responseCode: 200,
data: '<p>text</p>',
} as unknown as IWebhookResponseCallbackData);
await handler(req, res);
expect(res.send).toHaveBeenCalledWith('<p>text</p>');
cspSetBefore(res, 'send');
});
it('should set the sandbox header for an unsupported method', async () => {
const req = mock<WebhookRequest>({
path: '/',
method: 'CONNECT' as IHttpRequestMethods,
});
const res = mock<Response>();
res.status.mockReturnValue(res);
await handler(req, res);
cspSetBefore(res, 'json');
});
it('should set the sandbox header when the node writes the response', async () => {
const req = mock<WebhookRequest>({ path: '/', method: 'GET', params: { path: 'test' } });
const res = mock<Response>();
webhookManager.executeWebhook.mockImplementationOnce(async (_req, response) => {
response.send('<p>text</p>');
return { noWebhookResponse: true };
});
await handler(req, res);
cspSetBefore(res, 'send');
});
});
describe('sandbox setting of the route', () => {
const formHandlers = [
['a form route', createWebhookHandlerFor(webhookManager, 'form')],
[
'a route that selects the form setting',
createWebhookHandlerFor(webhookManager, undefined, 'form'),
],
] as const;
test.each(formHandlers)('should follow the form setting on %s', async (_, formHandler) => {
vi.mocked(isFormHtmlSandboxingDisabled).mockReturnValue(true);
vi.mocked(isWebhookHtmlSandboxingDisabled).mockReturnValue(false);
vi.mocked(getHtmlSandboxCSP).mockReturnValue('sandbox allow-scripts');
const req = mock<WebhookRequest>({ path: '/', method: 'GET', params: { path: 'test' } });
const res = mock<Response>();
webhookManager.executeWebhook.mockResolvedValueOnce({ noWebhookResponse: true });
await (formHandler as typeof handler)(req, res);
expect(res.setHeader).not.toHaveBeenCalledWith(
'Content-Security-Policy',
'sandbox allow-scripts',
);
});
it('should not sandbox a form response when only form sandboxing is disabled', async () => {
vi.mocked(isFormHtmlSandboxingDisabled).mockReturnValue(true);
vi.mocked(isWebhookHtmlSandboxingDisabled).mockReturnValue(false);
vi.mocked(getHtmlSandboxCSP).mockReturnValue('sandbox allow-scripts');
const formHandler = createWebhookHandlerFor(webhookManager, 'form') as typeof handler;
const req = mock<WebhookRequest>({ path: '/', method: 'GET', params: { path: 'test' } });
const res = mock<Response>();
webhookManager.executeWebhook.mockResolvedValueOnce({
responseCode: 200,
data: '<p>text</p>',
headers: { 'content-type': 'text/html' },
} as unknown as IWebhookResponseCallbackData);
await formHandler(req, res);
expect(res.setHeaders).toHaveBeenCalled();
expect(res.setHeader).not.toHaveBeenCalledWith(
'Content-Security-Policy',
'sandbox allow-scripts',
);
expect(res.send).toHaveBeenCalledWith('<p>text</p>');
});
it('should keep the form sandbox on a form response when webhook sandboxing is disabled', async () => {
vi.mocked(isFormHtmlSandboxingDisabled).mockReturnValue(false);
vi.mocked(isWebhookHtmlSandboxingDisabled).mockReturnValue(true);
vi.mocked(getHtmlSandboxCSP).mockReturnValue('sandbox allow-scripts');
const formHandler = createWebhookHandlerFor(webhookManager, 'form') as typeof handler;
const req = mock<WebhookRequest>({ path: '/', method: 'GET', params: { path: 'test' } });
const res = mock<Response>();
webhookManager.executeWebhook.mockResolvedValueOnce({
responseCode: 200,
data: { ok: true },
headers: { 'content-type': 'text/html' },
});
await formHandler(req, res);
expect(res.setHeader).toHaveBeenCalledWith(
'Content-Security-Policy',
'sandbox allow-scripts',
);
expect(res.removeHeader).not.toHaveBeenCalled();
expect(res.json).toHaveBeenCalledWith({ ok: true });
});
it('should follow the webhook setting on other routes', async () => {
vi.mocked(isFormHtmlSandboxingDisabled).mockReturnValue(true);
vi.mocked(isWebhookHtmlSandboxingDisabled).mockReturnValue(false);
vi.mocked(getHtmlSandboxCSP).mockReturnValue('sandbox allow-scripts');
const req = mock<WebhookRequest>({ path: '/', method: 'GET', params: { path: 'test' } });
const res = mock<Response>();
webhookManager.executeWebhook.mockResolvedValueOnce({ noWebhookResponse: true });
await handler(req, res);
expect(res.setHeader).toHaveBeenCalledWith(
'Content-Security-Policy',
'sandbox allow-scripts',
);
});
});
});
});

View File

@ -6,7 +6,12 @@ import type { Response } from 'express';
import { getHtmlSandboxCSP } from 'n8n-core';
import { mock } from 'vitest-mock-extended';
import { applyFormSandboxCSP, WebhookResponseHeaders } from '@/webhooks/webhook-response-headers';
import {
applyFormSandboxCSP,
applySandboxCSP,
setRouteSandboxSetting,
WebhookResponseHeaders,
} from '@/webhooks/webhook-response-headers';
describe('WebhookResponseHeaders', () => {
const logger = mockInstance(Logger);
@ -353,13 +358,75 @@ describe('WebhookResponseHeaders', () => {
expect(res.setHeader).toHaveBeenCalledWith('Content-Security-Policy', getHtmlSandboxCSP());
});
it('should set no policy when form sandboxing is disabled', () => {
it('should remove the policy when form sandboxing is disabled', () => {
securityConfig.disableFormHtmlSandboxing = true;
const res = mock<Response>();
applyFormSandboxCSP(res);
expect(res.setHeader).not.toHaveBeenCalled();
expect(res.removeHeader).toHaveBeenCalledWith('Content-Security-Policy');
});
});
describe('applySandboxCSP', () => {
const securityConfig = Container.get(SecurityConfig);
afterEach(() => {
securityConfig.disableWebhookHtmlSandboxing = false;
});
it('should set the sandbox policy', () => {
const res = mock<Response>();
applySandboxCSP(res);
expect(res.setHeader).toHaveBeenCalledWith('Content-Security-Policy', getHtmlSandboxCSP());
});
it('should leave the policy unchanged when webhook sandboxing is disabled', () => {
securityConfig.disableWebhookHtmlSandboxing = true;
const res = mock<Response>();
applySandboxCSP(res);
expect(res.setHeader).not.toHaveBeenCalled();
expect(res.removeHeader).not.toHaveBeenCalled();
});
describe('on a form route', () => {
afterEach(() => {
securityConfig.disableFormHtmlSandboxing = false;
});
it('should follow the form setting instead of the webhook setting', () => {
securityConfig.disableFormHtmlSandboxing = true;
const res = mock<Response>({ locals: {} });
setRouteSandboxSetting(res, 'form');
applySandboxCSP(res);
expect(res.setHeader).not.toHaveBeenCalled();
});
it('should set the policy when only webhook sandboxing is disabled', () => {
securityConfig.disableWebhookHtmlSandboxing = true;
const res = mock<Response>({ locals: {} });
setRouteSandboxSetting(res, 'form');
applySandboxCSP(res);
expect(res.setHeader).toHaveBeenCalledWith('Content-Security-Policy', getHtmlSandboxCSP());
});
});
it('should use the fallback setting outside webhook and form routes', () => {
securityConfig.disableWebhookHtmlSandboxing = true;
const res = mock<Response>({ locals: {} });
applySandboxCSP(res, 'form');
expect(res.setHeader).toHaveBeenCalledWith('Content-Security-Policy', getHtmlSandboxCSP());
});
});
});

View File

@ -21,7 +21,12 @@ import {
isWebhookResponse,
isWebhookStreamResponse,
} from '@/webhooks/webhook-response';
import { applySandboxCSP, WebhookResponseHeaders } from '@/webhooks/webhook-response-headers';
import {
applySandboxCSP,
setRouteSandboxSetting,
WebhookResponseHeaders,
type WebhookSandboxSetting,
} from '@/webhooks/webhook-response-headers';
import type {
IWebhookManager,
WebhookOptionsRequest,
@ -34,6 +39,7 @@ class WebhookRequestHandler {
constructor(
private readonly webhookManager: IWebhookManager,
private readonly expectedNodeType?: ExpectedWebhookNodeType,
private readonly sandboxSetting: WebhookSandboxSetting = 'webhook',
) {}
/**
@ -43,6 +49,12 @@ class WebhookRequestHandler {
async handleRequest(req: WebhookRequest | WebhookOptionsRequest, res: express.Response) {
const method = req.method;
// Set the sandbox before any writer runs, so that every response of the route has it,
// for all status codes and content types. Only an n8n page that must keep its origin
// replaces or removes it. Later writers follow the same setting through `res.locals`.
setRouteSandboxSetting(res, this.sandboxSetting);
applySandboxCSP(res);
if (method !== 'OPTIONS' && !WEBHOOK_METHODS.includes(method)) {
return ResponseHelper.sendErrorResponse(
res,
@ -280,12 +292,16 @@ function trackWebhookMetrics(
* recorded on each response: webhook requests via `observeWebhookRequest`,
* form submissions (POST only) via `observeFormSubmission`. No metrics are
* emitted for other node types.
*
* `sandboxSetting` selects the operator setting for the sandbox CSP. It defaults to the
* form setting for form routes and to the webhook setting for all other routes.
*/
export function createWebhookHandlerFor(
webhookManager: IWebhookManager,
expectedNodeType?: ExpectedWebhookNodeType,
sandboxSetting: WebhookSandboxSetting = expectedNodeType === 'form' ? 'form' : 'webhook',
): express.RequestHandler {
const handler = new WebhookRequestHandler(webhookManager, expectedNodeType);
const handler = new WebhookRequestHandler(webhookManager, expectedNodeType, sandboxSetting);
const metricsService = Container.get(PrometheusWebhookAndFormMetricsService);
return async (req, res) => {

View File

@ -86,17 +86,43 @@ export class WebhookResponseHeaders {
}
}
/** Set the sandbox CSP header on a webhook response, unless explicitly disabled. */
export function applySandboxCSP(res: Response): void {
if (isWebhookHtmlSandboxingDisabled()) return;
/** The operator setting that controls the sandbox CSP on the responses of a route. */
export type WebhookSandboxSetting = 'webhook' | 'form';
const SANDBOX_SETTING_LOCAL = 'webhookSandboxSetting';
/** Record which operator setting controls the sandbox CSP on this response. */
export function setRouteSandboxSetting(res: Response, setting: WebhookSandboxSetting): void {
res.locals[SANDBOX_SETTING_LOCAL] = setting;
}
function getRouteSandboxSetting(res: Response): WebhookSandboxSetting | undefined {
const setting: unknown = res.locals?.[SANDBOX_SETTING_LOCAL];
return setting === 'webhook' || setting === 'form' ? setting : undefined;
}
/**
* Set the sandbox CSP header on a webhook or form route response, unless an operator
* disabled it. The setting of the route decides, so every writer of a route follows the
* same setting. `fallback` applies when the response is not on a webhook or form route.
*/
export function applySandboxCSP(res: Response, fallback: WebhookSandboxSetting = 'webhook'): void {
const setting = getRouteSandboxSetting(res) ?? fallback;
const disabled =
setting === 'form' ? isFormHtmlSandboxingDisabled() : isWebhookHtmlSandboxingDisabled();
if (disabled) return;
res.setHeader('Content-Security-Policy', getHtmlSandboxCSP());
}
/**
* Set the sandbox CSP header on a form page response, unless an operator disabled it.
* Set the sandbox CSP header on a form page response. If an operator disabled it, remove
* the header instead, because the request handler sets it for the whole route.
* Call this for every HTML page a form endpoint serves, error and status pages included.
*/
export function applyFormSandboxCSP(res: Response): void {
if (isFormHtmlSandboxingDisabled()) return;
if (isFormHtmlSandboxingDisabled()) {
res.removeHeader('Content-Security-Policy');
return;
}
res.setHeader('Content-Security-Policy', getHtmlSandboxCSP());
}

View File

@ -162,6 +162,7 @@ async function fetchWorkflowData(
workflowData.nodes ?? [],
parentWorkflowId,
);
WorkflowHelpers.validateWorkflowStructure(workflowData);
// An inline sub-workflow is part of the parent that embeds it, not a
// workflow of its own, so it runs under the parent workflow's id.
workflowData.id = parentWorkflowId;

View File

@ -10,11 +10,13 @@ import type {
} from '@n8n/db';
import { WorkflowEntity } from '@n8n/db';
import type { PolicyCleared } from '@n8n/decorators';
import type { INode } from 'n8n-workflow';
import type { MockProxy } from 'vitest-mock-extended';
import { mock } from 'vitest-mock-extended';
import type { CredentialsFinderService } from '@n8n/backend-services';
import { BadRequestError, ForbiddenError, NotFoundError } from '@n8n/errors';
import { UninspectableSubworkflowSourceError } from '@/errors/uninspectable-subworkflow-source.error';
import type { ExternalHooks, WorkflowLifecycleHookActor } from '@/external-hooks';
import type { McpSettingsService } from '@/modules/mcp/mcp.settings.service';
import type { InstanceRedactionEnforcementService } from '@/modules/redaction/instance-redaction-enforcement.service';
@ -81,6 +83,7 @@ describe('WorkflowCreationService', () => {
enterpriseWorkflowServiceMock.collectCredentialReferences.mockReturnValue({
ids: new Set(),
hasUnresolved: false,
hasUninspectableSource: false,
});
// Default: no active floor. Tests opt into a floor explicitly.
@ -139,6 +142,7 @@ describe('WorkflowCreationService', () => {
enterpriseWorkflowServiceMock.collectCredentialReferences.mockReturnValue({
ids: new Set(['source-credential']),
hasUnresolved: false,
hasUninspectableSource: false,
});
const context = await workflowCreationService.prepareBatchContext(
@ -328,6 +332,7 @@ describe('WorkflowCreationService', () => {
enterpriseWorkflowServiceMock.collectCredentialReferences.mockReturnValue({
ids: new Set(['credential-1']),
hasUnresolved: false,
hasUninspectableSource: false,
});
credentialsFinderServiceMock.findCredentialIdsWithScopeForUser.mockResolvedValue(
new Set(['credential-1']),
@ -384,6 +389,7 @@ describe('WorkflowCreationService', () => {
enterpriseWorkflowServiceMock.collectCredentialReferences.mockReturnValue({
ids: new Set(['credential-1']),
hasUnresolved: true,
hasUninspectableSource: false,
});
credentialsFinderServiceMock.findCredentialIdsWithScopeForUser.mockResolvedValue(
new Set(['credential-1']),
@ -403,6 +409,58 @@ describe('WorkflowCreationService', () => {
'The workflow you are trying to save contains credentials that are not shared with you',
);
});
it('should pass full credential access to the source check', async () => {
const user = mock<User>();
const newWorkflow = new WorkflowEntity();
licenseStateMock.isSharingLicensed.mockReturnValue(true);
projectServiceMock.getProjectWithScope.mockResolvedValue({ id: 'project-1' } as never);
enterpriseWorkflowServiceMock.collectCredentialReferences.mockReturnValue({
ids: new Set(),
hasUnresolved: false,
hasUninspectableSource: true,
});
enterpriseWorkflowServiceMock.canUseEveryCredential.mockReturnValue(true);
enterpriseWorkflowServiceMock.validateCredentialPermissionsToUser.mockImplementation(() => {
throw new Error('Stopping for test');
});
await expect(
workflowCreationService.createWorkflow(user, newWorkflow, { projectId: 'project-1' }),
).rejects.toThrow();
expect(enterpriseWorkflowServiceMock.canUseEveryCredential).toHaveBeenCalledWith(user);
expect(
enterpriseWorkflowServiceMock.validateCredentialPermissionsToUser,
).toHaveBeenCalledWith(newWorkflow, new Set(), { allowUninspectableSource: true });
});
it('should preserve the source error message for a restricted user', async () => {
const user = mock<User>();
const newWorkflow = new WorkflowEntity();
const node = { name: 'Sub' } as INode;
licenseStateMock.isSharingLicensed.mockReturnValue(true);
projectServiceMock.getProjectWithScope.mockResolvedValue({ id: 'project-1' } as never);
enterpriseWorkflowServiceMock.collectCredentialReferences.mockReturnValue({
ids: new Set(),
hasUnresolved: false,
hasUninspectableSource: true,
});
enterpriseWorkflowServiceMock.canUseEveryCredential.mockReturnValue(false);
enterpriseWorkflowServiceMock.validateCredentialPermissionsToUser.mockImplementation(() => {
throw new UninspectableSubworkflowSourceError(node);
});
await expect(
workflowCreationService.createWorkflow(user, newWorkflow, { projectId: 'project-1' }),
).rejects.toThrow(
/'Sub' node loads its sub-workflow from a source that is resolved only when the workflow runs/,
);
expect(
enterpriseWorkflowServiceMock.validateCredentialPermissionsToUser,
).toHaveBeenCalledWith(newWorkflow, new Set(), { allowUninspectableSource: false });
});
});
it('should throw BadRequestError when user lacks access to credentials in workflow', async () => {

View File

@ -17,6 +17,7 @@ import { mock } from 'vitest-mock-extended';
import type { ActiveWorkflowManager } from '@/active-workflow-manager';
import type { CredentialsFinderService } from '@n8n/backend-services';
import type { CredentialsService } from '@/credentials/credentials.service';
import type { NodeTypes } from '@/node-types';
import type { CredentialsPermissionChecker } from '@/executions/pre-execution-checks/credentials-permission-checker';
import type { PolicyEnforcementService } from '@/policy/policy-enforcement.service';
import type { OwnershipService } from '@/services/ownership.service';
@ -44,10 +45,12 @@ describe('EnterpriseWorkflowService', () => {
const credentialsService = mock<CredentialsService>();
const ownershipService = mock<OwnershipService>();
const credentialsFinderService = mock<CredentialsFinderService>();
const nodeTypes = mock<NodeTypes>();
const credentialsPermissionChecker = mock<CredentialsPermissionChecker>();
beforeEach(() => {
vi.clearAllMocks();
nodeTypes.getByNameAndVersion.mockReset();
flags.credSharingEnabled = false;
service = new EnterpriseWorkflowService(
mock(), // logger
@ -65,6 +68,7 @@ describe('EnterpriseWorkflowService', () => {
workflowPublishHistoryRepository,
workflowMutationHooks,
policyEnforcementService,
nodeTypes,
credentialsPermissionChecker,
);
});
@ -401,6 +405,216 @@ describe('EnterpriseWorkflowService', () => {
});
});
describe('validateWorkflowCredentialUsage() - sub-workflow source', () => {
const URL_SOURCE = { source: 'url', workflowUrl: 'https://example.com/workflow.json' };
const subworkflowNode = (parameters: Record<string, unknown>, overrides: Partial<INode> = {}) =>
({
id: 'sub-1',
name: 'Sub',
type: 'n8n-nodes-base.executeWorkflow',
typeVersion: 1.2,
position: [0, 0],
parameters,
...overrides,
}) as unknown as INode;
const save = (
newNodes: INode[],
previousNodes: INode[] = [],
options?: { allowUninspectableSource?: boolean },
) =>
service.validateWorkflowCredentialUsage(
{ nodes: newNodes } as unknown as IWorkflowBase,
{ nodes: previousNodes } as unknown as IWorkflowBase,
[],
[],
options,
);
it.each([
['"url"', URL_SOURCE],
['"localFile"', { source: 'localFile', workflowPath: '/data/workflow.json' }],
['an expression', { source: '={{ $json.source }}' }],
['not a string', { source: 42 }],
])('rejects a new Execute Sub-workflow node whose source is %s', (_label, parameters) => {
expect(() => save([subworkflowNode(parameters)])).toThrow(/cannot be checked/);
});
it.each([
['absent', {}],
['"database"', { source: 'database', workflowId: { __rl: true, value: 'wf-1', mode: 'id' } }],
[
'"parameter"',
{ source: 'parameter', workflowJson: JSON.stringify({ nodes: [], connections: {} }) },
],
])('accepts a new Execute Sub-workflow node whose source is %s', (_label, parameters) => {
expect(() => save([subworkflowNode(parameters)])).not.toThrow();
});
it('rejects a Workflow Tool node whose source is neither "database" nor "parameter"', () => {
const node = subworkflowNode(URL_SOURCE, {
type: '@n8n/n8n-nodes-langchain.toolWorkflow',
typeVersion: 2,
});
expect(() => save([node])).toThrow(/cannot be checked/);
});
it('rejects the synthetic HITL tool variant of a url-sourced Execute Sub-workflow node', () => {
const node = subworkflowNode(URL_SOURCE, { type: 'n8n-nodes-base.executeWorkflowHitlTool' });
expect(() => save([node])).toThrow(/cannot be checked/);
});
it('rejects a url-sourced node nested inside an inline sub-workflow', () => {
const inner = subworkflowNode(URL_SOURCE, { id: 'inner', name: 'Inner' });
const outer = subworkflowNode({
source: 'parameter',
workflowJson: JSON.stringify({ nodes: [inner], connections: {} }),
});
expect(() => save([outer])).toThrow(/cannot be checked/);
});
it('rejects a url-sourced Execute Sub-workflow tool inside an inline agent parameter', () => {
const agent = {
id: 'agent-1',
name: 'Agent',
type: 'n8n-nodes-base.messageAnAgent',
typeVersion: 2,
position: [0, 0],
parameters: {
agentSource: 'inline',
inlineAgent: {
config: {
tools: [
{ nodeType: 'n8n-nodes-base.executeWorkflowTool', nodeParameters: URL_SOURCE },
],
},
},
},
} as unknown as INode;
expect(() => save([agent])).toThrow(/cannot be checked/);
});
it('rejects an existing editable node that is changed to a url source', () => {
const previous = subworkflowNode({ source: 'database', workflowId: 'wf-1' });
expect(() => save([subworkflowNode(URL_SOURCE)], [previous])).toThrow(/cannot be checked/);
});
it('allows renaming, moving and disabling a stored URL node', () => {
const previous = subworkflowNode(URL_SOURCE);
const edited = { ...previous, name: 'Renamed', position: [10, 10], disabled: true } as INode;
const result = save([edited], [previous]);
expect(result.nodes[0]).toEqual(edited);
});
it('rejects URL changes when the node description is unavailable', () => {
const previous = subworkflowNode(URL_SOURCE);
const changed = subworkflowNode({
source: 'url',
workflowUrl: 'https://example.com/other.json',
});
expect(() => save([changed], [previous])).toThrow(/cannot be checked/);
});
it('rejects retry setting changes on a stored URL node', () => {
const previous = subworkflowNode(URL_SOURCE);
const changed = { ...previous, retryOnFail: true } as INode;
expect(() => save([changed], [previous])).toThrow(/cannot be checked/);
});
describe('with the node type description available', () => {
beforeEach(() => {
nodeTypes.getByNameAndVersion.mockReturnValue({
description: {
properties: [
{ displayName: 'Source', name: 'source', type: 'options', default: 'database' },
{
displayName: 'Workflow',
name: 'workflowId',
type: 'string',
default: '',
displayOptions: { show: { source: ['database'] } },
},
{
displayName: 'URL',
name: 'workflowUrl',
type: 'string',
default: '',
displayOptions: { show: { source: ['url'] } },
},
{ displayName: 'Mode', name: 'mode', type: 'options', default: 'once' },
],
},
} as never);
});
it('accepts omitted parameter defaults', () => {
// A node stored through the API may carry a default the editor strips on save.
const stored = subworkflowNode({ ...URL_SOURCE, mode: 'once' });
const fromEditor = { ...subworkflowNode(URL_SOURCE), name: 'Renamed' } as INode;
const result = save([fromEditor], [stored]);
expect(result.nodes[0]).toEqual({ ...stored, name: 'Renamed' });
});
it('accepts omitted hidden parameters', () => {
// The editor drops the old database workflow ID when the source is URL.
const stored = subworkflowNode({ ...URL_SOURCE, workflowId: 'old-workflow' });
const fromEditor = { ...subworkflowNode(URL_SOURCE), name: 'Renamed' } as INode;
const result = save([fromEditor], [stored]);
expect(result.nodes[0]).toEqual({ ...stored, name: 'Renamed' });
});
it('rejects URL changes after parameter normalization', () => {
const stored = subworkflowNode({ ...URL_SOURCE, mode: 'once' });
const changed = subworkflowNode({
source: 'url',
workflowUrl: 'https://example.com/other.json',
});
expect(() => save([changed], [stored])).toThrow(/cannot be checked/);
});
});
it('accepts url-sourced nodes when the caller allows uninspectable sources', () => {
const previous = subworkflowNode(URL_SOURCE);
const changed = subworkflowNode({
source: 'url',
workflowUrl: 'https://example.com/other.json',
});
const options = { allowUninspectableSource: true };
expect(() => save([subworkflowNode(URL_SOURCE)], [], options)).not.toThrow();
expect(save([changed], [previous], options).nodes[0]).toEqual(changed);
});
it('checks source permissions when validating a new workflow', () => {
const workflow = { nodes: [subworkflowNode(URL_SOURCE)] } as unknown as IWorkflowBase;
expect(() => service.validateCredentialPermissionsToUser(workflow, [])).toThrow(
/cannot be checked/,
);
expect(() =>
service.validateCredentialPermissionsToUser(workflow, [], {
allowUninspectableSource: true,
}),
).not.toThrow();
});
it('reports runtime-loaded sources even when there are no credential IDs', () => {
const workflow = { nodes: [subworkflowNode(URL_SOURCE)] } as unknown as IWorkflowBase;
expect(service.collectCredentialReferences(workflow)).toEqual({
ids: new Set(),
hasUnresolved: false,
hasUninspectableSource: true,
});
});
});
describe('validateWorkflowCredentialUsage() - credential added to an existing node', () => {
const httpNode = (
credentials?: INode['credentials'],

View File

@ -58,7 +58,7 @@ import type { WorkflowPublicationStatusService } from '@/workflows/publication/w
import type { WorkflowMutationHooksProxy } from '@/workflows/workflow-mutation-hooks-proxy.service';
import type { WorkflowPublishGuardProxy } from '@/workflows/workflow-publish-guard-proxy.service';
import type { WorkflowValidationService } from '@/workflows/workflow-validation.service';
import { NodeGroupRulesFlagGate } from '@/workflows/node-group-rules-flag-gate';
import type { NodeGroupRulesFlagGate } from '@/workflows/node-group-rules-flag-gate';
import type { RestrictedNodeTypesProviderProxy } from '@/workflows/restricted-node-types-provider-proxy.service';
import { WorkflowService } from '@/workflows/workflow.service';
import { ALL_RULES_RELAXED, NO_RULES_RELAXED } from './node-group-rules.test-data';
@ -73,6 +73,7 @@ describe('WorkflowService', () => {
let workflowRepositoryMock: MockProxy<{
getManyAndCountWithSharingSubquery: Mock;
getWorkflowsAndFoldersWithCountWithSharingSubquery: Mock;
findCallableSubworkflows: Mock;
}>;
let roleServiceMock: MockProxy<RoleService>;
let webhookServiceMock: MockProxy<WebhookService>;
@ -94,8 +95,6 @@ describe('WorkflowService', () => {
webhookServiceMock = mock<WebhookService>();
workflowFinderServiceMock = mock<WorkflowFinderService>();
// By default the requester can read the supplied parent workflow.
workflowFinderServiceMock.findWorkflowForUser.mockResolvedValue(mock<WorkflowEntity>());
globalConfigMock = mock<GlobalConfig>({
workflows: mock<WorkflowsConfig>({ useWorkflowPublicationService: false }),
@ -163,7 +162,6 @@ describe('WorkflowService', () => {
workflowRoles: expect.any(Array),
}),
undefined,
undefined, // callableForParentWorkflowId
);
});
@ -183,7 +181,6 @@ describe('WorkflowService', () => {
workflowRoles: expect.any(Array),
}),
undefined,
undefined, // callableForParentWorkflowId
);
});
@ -203,7 +200,6 @@ describe('WorkflowService', () => {
workflowRoles: expect.any(Array),
}),
undefined,
undefined, // callableForParentWorkflowId
);
});
@ -223,110 +219,82 @@ describe('WorkflowService', () => {
workflowRoles: expect.any(Array),
}),
undefined,
undefined, // callableForParentWorkflowId
);
});
describe('callableForParentWorkflowId', () => {
test('should pass parentWorkflowId when includeCallableSubworkflows is true', async () => {
describe('getCallableSubworkflows', () => {
test('should query with read roles and the Execute Workflow Trigger node type', async () => {
const user = mock<User>();
const options = {
filter: {
includeCallableSubworkflows: true,
parentWorkflowId: 'parent-wf-id',
},
};
workflowRepositoryMock.findCallableSubworkflows.mockResolvedValue({
workflows: [],
count: 0,
});
await workflowService.getMany(user, options);
await workflowService.getCallableSubworkflows(user, 'parent-wf-id', {
query: 'search',
skip: 10,
take: 5,
});
expect(workflowRepositoryMock.getManyAndCountWithSharingSubquery).toHaveBeenCalledWith(
expect(roleServiceMock.rolesWithScope).toHaveBeenCalledWith('project', ['workflow:read']);
expect(roleServiceMock.rolesWithScope).toHaveBeenCalledWith('workflow', ['workflow:read']);
expect(workflowRepositoryMock.findCallableSubworkflows).toHaveBeenCalledWith(
user,
expect.any(Object),
options,
{
scopes: ['workflow:read'],
projectRoles: ['project:viewer'],
workflowRoles: ['project:viewer'],
},
'parent-wf-id',
{
query: 'search',
triggerNodeTypes: ['n8n-nodes-base.executeWorkflowTrigger'],
skip: 10,
take: 5,
},
);
});
test('should pass undefined when includeCallableSubworkflows is false', async () => {
const user = mock<User>();
const options = {
filter: {
includeCallableSubworkflows: false,
parentWorkflowId: 'parent-wf-id',
},
};
test('should return minimal rows with the project name for team projects only', async () => {
workflowRepositoryMock.findCallableSubworkflows.mockResolvedValue({
count: 2,
workflows: [
{
id: 'team-wf',
name: 'Team',
activeVersionId: 'version-1',
isArchived: false,
nodes: [{ type: 'n8n-nodes-base.code' }],
shared: [{ project: { type: 'team', name: 'Team Project' } }],
},
{
id: 'personal-wf',
name: 'Personal',
activeVersionId: null,
isArchived: true,
shared: [{ project: { type: 'personal', name: 'Jane <jane@example.com>' } }],
},
],
});
await workflowService.getMany(user, options);
const result = await workflowService.getCallableSubworkflows(mock<User>(), 'parent-wf-id', {
skip: 0,
take: 10,
});
expect(workflowRepositoryMock.getManyAndCountWithSharingSubquery).toHaveBeenCalledWith(
user,
expect.any(Object),
options,
undefined,
);
});
test('should pass undefined when includeCallableSubworkflows is true but parentWorkflowId is missing', async () => {
const user = mock<User>();
const options = {
filter: {
includeCallableSubworkflows: true,
},
};
await workflowService.getMany(user, options);
expect(workflowRepositoryMock.getManyAndCountWithSharingSubquery).toHaveBeenCalledWith(
user,
expect.any(Object),
options,
undefined,
);
});
test('should pass undefined when includeCallableSubworkflows is true but parentWorkflowId is not a string', async () => {
const user = mock<User>();
const options = {
filter: {
includeCallableSubworkflows: true,
parentWorkflowId: 123, // invalid type
},
};
await workflowService.getMany(user, options);
expect(workflowRepositoryMock.getManyAndCountWithSharingSubquery).toHaveBeenCalledWith(
user,
expect.any(Object),
options,
undefined,
);
});
test('should pass undefined when the requester cannot read the parent workflow', async () => {
const user = mock<User>();
const options = {
filter: {
includeCallableSubworkflows: true,
parentWorkflowId: 'parent-wf-id',
},
};
// Requester has no read access to the supplied parent workflow.
workflowFinderServiceMock.findWorkflowForUser.mockResolvedValue(null);
await workflowService.getMany(user, options);
expect(workflowFinderServiceMock.findWorkflowForUser).toHaveBeenCalledWith(
'parent-wf-id',
user,
['workflow:read'],
);
expect(workflowRepositoryMock.getManyAndCountWithSharingSubquery).toHaveBeenCalledWith(
user,
expect.any(Object),
options,
undefined,
);
expect(result).toEqual({
count: 2,
data: [
{
id: 'team-wf',
name: 'Team',
active: true,
isArchived: false,
projectName: 'Team Project',
},
{ id: 'personal-wf', name: 'Personal', active: false, isArchived: true },
],
});
});
});
@ -461,12 +429,10 @@ describe('WorkflowService', () => {
expect(result).toMatchObject({ workflows: [folder], count: 1 });
expect(
workflowRepositoryMock.getWorkflowsAndFoldersWithCountWithSharingSubquery,
).toHaveBeenCalledWith(
expect.anything(),
expect.anything(),
{ ...restrictedNodeFilter, workflowIdsIn: null },
undefined,
);
).toHaveBeenCalledWith(expect.anything(), expect.anything(), {
...restrictedNodeFilter,
workflowIdsIn: null,
});
});
test('limits the list query to the restricted workflow ids', async () => {
@ -479,7 +445,6 @@ describe('WorkflowService', () => {
expect.anything(),
expect.anything(),
{ ...restrictedNodeFilter, workflowIdsIn },
undefined,
);
});
});

View File

@ -17,6 +17,7 @@ import { v4 as uuid } from 'uuid';
import { BadRequestError, ForbiddenError, InternalServerError, NotFoundError } from '@n8n/errors';
import { WorkflowValidationError } from '@/errors/response-errors/workflow-validation.error';
import { UninspectableSubworkflowSourceError } from '@/errors/uninspectable-subworkflow-source.error';
import type { WorkflowActionSource } from '@/events/maps/relay.event-map';
import { ExternalHooks, toWorkflowLifecycleHookActor } from '@/external-hooks';
import { validateEntity } from '@/generic-helpers';
@ -239,8 +240,11 @@ export class WorkflowCreationService {
if (this.licenseState.isSharingLicensed()) {
// This is a new workflow, so we simply check if the user has access to
// all used credentials
const { ids: credentialIds, hasUnresolved } =
this.enterpriseWorkflowService.collectCredentialReferences(newWorkflow);
const {
ids: credentialIds,
hasUnresolved,
hasUninspectableSource,
} = this.enterpriseWorkflowService.collectCredentialReferences(newWorkflow);
if (batchContext) {
const uncheckedIds = [...credentialIds].filter(
(id) => !batchContext.checkedCredentialIds.has(id),
@ -270,8 +274,15 @@ export class WorkflowCreationService {
this.enterpriseWorkflowService.validateCredentialPermissionsToUser(
newWorkflow,
hasUnresolved ? new Set() : accessibleCredentialIds,
{
allowUninspectableSource:
hasUninspectableSource && this.enterpriseWorkflowService.canUseEveryCredential(user),
},
);
} catch (error) {
if (error instanceof UninspectableSubworkflowSourceError) {
throw new BadRequestError(error.message);
}
throw new BadRequestError(
'The workflow you are trying to save contains credentials that are not shared with you',
);

View File

@ -21,10 +21,12 @@ import { hasGlobalScope } from '@n8n/permissions';
import { isRecord } from '@n8n/utils/is-record';
import { In, type EntityManager } from '@n8n/typeorm';
import isEqual from 'lodash/isEqual';
import type { INode, IWorkflowBase, WorkflowId } from 'n8n-workflow';
import omit from 'lodash/omit';
import type { INode, INodeTypeDescription, IWorkflowBase, WorkflowId } from 'n8n-workflow';
import {
isNodeWithWorkflowSelector,
jsonParse,
NodeHelpers,
NodeOperationError,
PROJECT_ROOT,
UserError,
@ -40,14 +42,46 @@ import { CredentialsPermissionChecker } from '@/executions/pre-execution-checks/
import { FolderNotFoundError } from '@/errors/folder-not-found.error';
import { BadRequestError, NotFoundError } from '@n8n/errors';
import { TransferWorkflowError } from '@/errors/response-errors/transfer-workflow.error';
import { UninspectableSubworkflowSourceError } from '@/errors/uninspectable-subworkflow-source.error';
import { extractAgentCredentialIds } from '@/modules/agents/utils/extract-agent-credential-ids';
import { NodeTypes } from '@/node-types';
import { PolicyEnforcementService } from '@/policy/policy-enforcement.service';
import { OwnershipService } from '@/services/ownership.service';
import { ProjectService } from '@/services/project.service.ee';
import { stripToolSuffix } from '@/utils';
import { WorkflowFinderService } from './workflow-finder.service';
import { WorkflowMutationHooksProxy } from './workflow-mutation-hooks-proxy.service';
/**
* Database workflows use their own project permissions at runtime.
* Parameter workflows contain JSON we can inspect before saving.
*/
const INSPECTABLE_SUBWORKFLOW_SOURCES = new Set(['database', 'parameter']);
/** Exclude allowed edits and temporary editor state from the definition comparison. */
const FREELY_EDITABLE_NODE_FIELDS = ['name', 'position', 'disabled', 'color', 'issues', 'status'];
/** Tool and HITL tool variants use the same source rules as their base node. */
function isWorkflowSelectorNode(node: INode): boolean {
if (typeof node.type !== 'string') return false;
return isNodeWithWorkflowSelector({ ...node, type: stripToolSuffix(node.type) });
}
type CredentialReferences = {
/** Resolvable credential ids. */
ids: string[];
/** A non-managed credential carrying no id, i.e. only a name. */
hasUnresolved: boolean;
/** A sub-workflow source whose content is not available at save time. */
hasUninspectableSource: boolean;
};
export type CredentialGuardOptions = {
/** Allow content loaded at runtime only for users who can use every credential. */
allowUninspectableSource?: boolean;
};
@Service()
export class EnterpriseWorkflowService {
constructor(
@ -66,6 +100,7 @@ export class EnterpriseWorkflowService {
private readonly workflowPublishHistoryRepository: WorkflowPublishHistoryRepository,
private readonly workflowMutationHooks: WorkflowMutationHooksProxy,
private readonly policyEnforcementService: PolicyEnforcementService,
private readonly nodeTypes: NodeTypes,
private readonly credentialsPermissionChecker: CredentialsPermissionChecker,
) {}
@ -185,6 +220,7 @@ export class EnterpriseWorkflowService {
validateCredentialPermissionsToUser(
workflow: IWorkflowBase,
allowedCredentials: Array<{ id: string }> | ReadonlySet<string>,
options: CredentialGuardOptions = {},
) {
// Reuse the shared collector so inline sub-workflow credentials (Execute
// Sub-workflow, Workflow Tool, Workflow Retriever) and unresolved name-only
@ -192,8 +228,16 @@ export class EnterpriseWorkflowService {
const allowedCredentialIds: ReadonlySet<string> = Array.isArray(allowedCredentials)
? new Set(allowedCredentials.map(({ id }) => id))
: allowedCredentials;
const inaccessibleNodes = this.getNodesWithInaccessibleCreds(workflow, allowedCredentialIds);
const inaccessibleNodes = this.getNodesWithInaccessibleCreds(
workflow,
allowedCredentialIds,
options,
);
if (inaccessibleNodes.length > 0) {
const uninspectableNode = inaccessibleNodes.find((node) =>
this.isBlockedByUninspectableSource(node, options),
);
if (uninspectableNode) throw new UninspectableSubworkflowSourceError(uninspectableNode);
throw new UserError('The workflow contains credentials that you do not have access to');
}
}
@ -224,6 +268,7 @@ export class EnterpriseWorkflowService {
previousVersion,
allCredentials,
restoredNodes,
{ allowUninspectableSource: this.canUseEveryCredential(user) },
);
} catch (error) {
if (error instanceof NodeOperationError) {
@ -240,20 +285,12 @@ export class EnterpriseWorkflowService {
previousWorkflowVersion: IWorkflowBase,
credentialsUserHasAccessTo: Array<{ id: string }>,
restoredNodes: INode[] = [],
options: CredentialGuardOptions = {},
) {
/**
* We only need to check nodes that use credentials the current user cannot access,
* since these can be 2 possibilities:
* - It matches exactly one previous node that already used such a credential: it's a
* read only node and therefore cannot be changed
* - It's a new node, or an editable node that newly references such a credential,
* which indicates tampering and therefore must fail saving
*/
const allowedCredentialIds = credentialsUserHasAccessTo.map((cred) => cred.id);
const nodesWithCredentialsUserDoesNotHaveAccessTo = new Set(
this.getNodesWithInaccessibleCreds(newWorkflowVersion, allowedCredentialIds),
this.getNodesWithInaccessibleCreds(newWorkflowVersion, allowedCredentialIds, options),
);
// If there are no nodes with credentials the user does not have access to we can skip the rest
@ -265,9 +302,11 @@ export class EnterpriseWorkflowService {
// the user could edit that now carries a credential they cannot use fails the
// save like a new node does; restoring it would drop the other edits silently.
const readOnlyNodeIds = new Set(
this.getNodesWithInaccessibleCreds(previousWorkflowVersion, allowedCredentialIds).map(
(node) => node.id,
),
this.getNodesWithInaccessibleCreds(
previousWorkflowVersion,
allowedCredentialIds,
options,
).map((node) => node.id),
);
// Node ids are meant to be unique. Edge case: where stored data still repeats one, match the
@ -295,6 +334,7 @@ export class EnterpriseWorkflowService {
// 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;
const blockedBySource = this.isBlockedByUninspectableSource(node, options);
// A restore brings back a node exactly as a version from this workflow's history
// stored it. Any change to it makes it a new edit, which the checks below judge.
@ -308,12 +348,22 @@ export class EnterpriseWorkflowService {
nodeId: node.id,
nodeCredentials: node.credentials,
});
if (blockedBySource) throw new UninspectableSubworkflowSourceError(node);
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.`,
);
}
// The editor allows these source settings to be edited. Reject changes
// instead of silently discarding them when we restore the stored node.
if (
blockedBySource &&
!isEqual(this.getNodeDefinition(node), this.getNodeDefinition(previousNode))
) {
throw new UninspectableSubworkflowSourceError(node);
}
this.logger.debug('Replacing node with previous version when saving updated workflow', {
nodeType: node.type,
nodeName: node.name,
@ -335,62 +385,120 @@ export class EnterpriseWorkflowService {
}
/**
* Get all nodes in a workflow whose credentials the user cannot use: either a
* referenced credential id is not accessible, or the node carries an unresolved
* non-managed reference (id null/empty) that could resolve by name to a
* credential the user does not own. Inline sub-workflow credentials are included.
* Find nodes with inaccessible credentials, unresolved references, or blocked sources.
* Include credentials and sources in nested workflows and agents.
*/
getNodesWithInaccessibleCreds(workflow: IWorkflowBase, userCredIds: Iterable<string>) {
getNodesWithInaccessibleCreds(
workflow: IWorkflowBase,
userCredIds: Iterable<string>,
options: CredentialGuardOptions = {},
) {
if (!workflow.nodes) {
return [];
}
const allowedCredentialIds = userCredIds instanceof Set ? userCredIds : new Set(userCredIds);
return workflow.nodes.filter((node) => {
const { ids, hasUnresolved } = this.getNodeCredentialRefs(node);
return hasUnresolved || ids.some((credId) => !allowedCredentialIds.has(credId));
const { ids, hasUnresolved, hasUninspectableSource } = this.getNodeCredentialRefs(node);
return (
hasUnresolved ||
(hasUninspectableSource && !options.allowUninspectableSource) ||
ids.some((credId) => !allowedCredentialIds.has(credId))
);
});
}
collectCredentialReferences(workflow: IWorkflowBase): {
ids: Set<string>;
hasUnresolved: boolean;
hasUninspectableSource: boolean;
} {
const ids = new Set<string>();
let hasUnresolved = false;
let hasUninspectableSource = false;
for (const node of workflow.nodes ?? []) {
const references = this.getNodeCredentialRefs(node);
for (const id of references.ids) ids.add(id);
hasUnresolved ||= references.hasUnresolved;
hasUninspectableSource ||= references.hasUninspectableSource;
}
return { ids, hasUnresolved };
return { ids, hasUnresolved, hasUninspectableSource };
}
canUseEveryCredential(user: User): boolean {
return hasGlobalScope(user, 'credential:use');
}
private isBlockedByUninspectableSource(node: INode, options: CredentialGuardOptions): boolean {
if (options.allowUninspectableSource) return false;
return this.getNodeCredentialRefs(node).hasUninspectableSource;
}
/**
* Collect the credential references a node uses. Besides the node's own
* `credentials`, a node with an inline workflow selector (Execute
* Sub-workflow, Workflow Tool, Workflow Retriever) embeds a whole workflow —
* including its nodes' credentials — inside its `workflowJson` string
* parameter, so those references are walked as well.
*
* Returns `ids` (resolvable credential ids) and `hasUnresolved` (a non-managed
* credential carrying no id, i.e. only a name). A name-only reference can be
* resolved by name to a credential the user cannot access, so callers gating a
* save must reject it rather than treat the node as credential-free.
* `__aiGatewayManaged` credentials with a null id are resolved at execution and
* are exempt.
*
* An inline sub-workflow may itself contain inline sub-workflows, so the walk
* is iterative over an explicit stack: every referenced credential is
* inspected regardless of nesting depth, with no silent cut-off that could let
* a deeply nested reference escape validation. Nesting is inherently bounded by
* the request size (each level embeds its child as literal escaped JSON) and
* cannot cycle, so the stack cannot grow unbounded.
* Compare node behavior without treating editor formatting or allowed edits as changes.
*/
private getNodeCredentialRefs(node: INode): { ids: string[]; hasUnresolved: boolean } {
private getNodeDefinition(node: INode): Record<string, unknown> {
const definition: Record<string, unknown> = omit(node, FREELY_EDITABLE_NODE_FIELDS);
definition.parameters = this.getDisplayedParameters(node);
const credentials = node.credentials ?? {};
if (Object.keys(credentials).length === 0) delete definition.credentials;
// The editor persists these only when they differ from their default.
if (!node.continueOnFail) delete definition.continueOnFail;
if (!node.onError || node.onError === 'stopWorkflow') delete definition.onError;
if (!node.notes) delete definition.notes;
return definition;
}
/**
* The editor omits default values and hidden parameters on save.
* Compare displayed parameters with defaults filled in so an unchanged node still matches.
*/
private getDisplayedParameters(node: INode) {
const parameters = node.parameters ?? {};
const description = this.getNodeTypeDescription(node);
if (!description) return parameters;
return (
NodeHelpers.getNodeParameters(
description.properties,
parameters,
true,
false,
node,
description,
) ?? {}
);
}
private getNodeTypeDescription(node: INode): INodeTypeDescription | null {
try {
return this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion)?.description ?? null;
} catch {
return null;
}
}
/**
* A missing source defaults to Database. Expressions are not resolved until runtime.
*/
private hasUninspectableSubworkflowSource(node: INode): boolean {
const source = node.parameters?.source;
if (source === undefined) return false;
return !(typeof source === 'string' && INSPECTABLE_SUBWORKFLOW_SOURCES.has(source));
}
/**
* Inspect the node and its embedded workflows and agents at every nesting level.
* Report name-only credentials and runtime-loaded sources separately from known IDs;
* neither means the node uses no credentials. Managed Gateway credentials are exempt.
* Use a stack to avoid a call-stack limit. Serialized input bounds the nesting depth.
*/
private getNodeCredentialRefs(node: INode): CredentialReferences {
const ids: string[] = [];
let hasUnresolved = false;
let hasUninspectableSource = false;
const stack: INode[] = [node];
while (stack.length > 0) {
@ -409,7 +517,8 @@ export class EnterpriseWorkflowService {
}
}
if (isNodeWithWorkflowSelector(current)) {
if (isWorkflowSelectorNode(current)) {
hasUninspectableSource ||= this.hasUninspectableSubworkflowSource(current);
stack.push(...this.getInlineWorkflowNodes(current.parameters?.workflowJson));
}
@ -420,7 +529,7 @@ export class EnterpriseWorkflowService {
}
}
return { ids, hasUnresolved };
return { ids, hasUnresolved, hasUninspectableSource };
}
/**
@ -452,9 +561,8 @@ export class EnterpriseWorkflowService {
const nested = Object.values(config).flatMap((value) => this.getAgentToolNodes(value));
if (typeof config.nodeType !== 'string') return nested;
// A node runs as a tool under its `…Tool` variant, which carries the same
// parameters as the node itself.
const type = config.nodeType.replace(/Tool$/, '');
// Tool variants keep the base node's parameters.
const type = stripToolSuffix(config.nodeType);
const parameters = isRecord(config.nodeParameters) ? config.nodeParameters : {};
return [...nested, { type, parameters } as INode];

View File

@ -1,5 +1,10 @@
import { UpdateWorkflowHistoryVersionDto } from '@n8n/api-types';
import type { WorkflowListPublicationStatus, WorkflowExecutionBlockCause } from '@n8n/api-types';
import type {
CallableSubworkflowsResponse,
ListCallableSubworkflowsQueryDto,
WorkflowExecutionBlockCause,
WorkflowListPublicationStatus,
} from '@n8n/api-types';
import { LicenseState, Logger } from '@n8n/backend-common';
import { EventService, RoleService } from '@n8n/backend-services';
import { GlobalConfig } from '@n8n/config';
@ -38,7 +43,13 @@ import { ensureError } from '@n8n/utils/errors/ensure-error';
import isEqual from 'lodash/isEqual';
import pick from 'lodash/pick';
import type { INode, INodes, IWorkflowSettings, JsonValue, IConnections } from 'n8n-workflow';
import { PROJECT_ROOT, Workflow, assert, calculateWorkflowChecksum } from 'n8n-workflow';
import {
EXECUTE_WORKFLOW_TRIGGER_NODE_TYPE,
PROJECT_ROOT,
Workflow,
assert,
calculateWorkflowChecksum,
} from 'n8n-workflow';
import { v4 as uuid } from 'uuid';
import {
@ -303,11 +314,6 @@ export class WorkflowService {
sharingOptions.workflowRoles = workflowRoles;
}
const callableForParentWorkflowId = await this.resolveCallableForParentWorkflowId(
user,
options,
);
const workflowIdsIn = await this.resolveRestrictedWorkflowIds(options);
const listOptions = workflowIdsIn === undefined ? options : { ...options, workflowIdsIn };
@ -318,7 +324,6 @@ export class WorkflowService {
user,
sharingOptions,
listOptions,
callableForParentWorkflowId,
);
workflows = workflowsAndFolders.filter((wf) => wf.resource === 'workflow');
@ -327,7 +332,6 @@ export class WorkflowService {
user,
sharingOptions,
listOptions,
callableForParentWorkflowId,
));
}
@ -380,25 +384,42 @@ export class WorkflowService {
};
}
private async resolveCallableForParentWorkflowId(
/**
* List the workflows that `parentWorkflowId` can call as sub-workflows, for
* the sub-workflow picker. The caller must check that the user can update
* the parent workflow. Each row holds only the fields the picker shows.
*/
async getCallableSubworkflows(
user: User,
options?: ListQuery.Options,
): Promise<string | undefined> {
if (options?.filter?.includeCallableSubworkflows !== true) return undefined;
parentWorkflowId: string,
{ query, skip, take }: ListCallableSubworkflowsQueryDto,
): Promise<CallableSubworkflowsResponse> {
const requiredScopes: Scope[] = ['workflow:read'];
const [projectRoles, workflowRoles] = await Promise.all([
this.roleService.rolesWithScope('project', requiredScopes),
this.roleService.rolesWithScope('workflow', requiredScopes),
]);
const parentWorkflowId =
typeof options.filter.parentWorkflowId === 'string'
? options.filter.parentWorkflowId
: undefined;
if (!parentWorkflowId) return undefined;
const parentWorkflow = await this.workflowFinderService.findWorkflowForUser(
parentWorkflowId,
const { workflows, count } = await this.workflowRepository.findCallableSubworkflows(
user,
['workflow:read'],
{ scopes: requiredScopes, projectRoles, workflowRoles },
parentWorkflowId,
{ query, triggerNodeTypes: [EXECUTE_WORKFLOW_TRIGGER_NODE_TYPE], skip, take },
);
return parentWorkflow ? parentWorkflowId : undefined;
const data = workflows.map((workflow) => {
const project = workflow.shared?.[0]?.project;
return {
id: workflow.id,
name: workflow.name,
active: workflow.activeVersionId !== null,
isArchived: workflow.isArchived,
// Personal project names hold user names and emails, so only team names are returned.
...(project?.type === 'team' ? { projectName: project.name } : {}),
};
});
return { count, data };
}
private async resolveRestrictedWorkflowIds(
@ -1731,14 +1752,18 @@ export class WorkflowService {
// workflow cannot be deleted and unpublishing deprovisions them. This covers
// the paths that never published and anything an interrupted unpublish left.
//
// Both writes commit together. A deprovision of its own would leave a workflow
// All three writes commit together. A deprovision of its own would leave a workflow
// whose delete then failed alive with its schedules stripped.
await this.workflowRepository.runInTransaction({}, async (trx) => {
await this.workflowRepository.runInTransaction({}, async (trx, ctx) => {
await this.durableJobProvisioner.deprovisionOwnerInTransaction(
trx,
this.workflowScheduledJobOwner.ref(workflowId),
);
// A new workflow can be created at this id once the row is gone, so allowlists lose
// the entry in the same transaction.
await this.workflowRepository.removeFromCallerAllowlists([workflowId], ctx);
await trx.delete(WorkflowEntity, { id: workflowId });
});

View File

@ -5,9 +5,11 @@ import {
DeactivateWorkflowDto,
ExecutionRedactionQueryDtoSchema,
ImportWorkflowFromUrlDto,
ListCallableSubworkflowsQueryDto,
ManualRunDto,
TransferWorkflowBodyDto,
UpdateWorkflowDto,
type CallableSubworkflowsResponse,
type WorkflowPublicationStatus,
} from '@n8n/api-types';
import { Logger } from '@n8n/backend-common';
@ -717,6 +719,22 @@ export class WorkflowsController {
return await this.workflowPublicationStatusService.getStatus(workflowId);
}
/**
* Lists the workflows that this workflow can call as sub-workflows. Only
* editors of the parent can add a sub-workflow call, so this requires
* `workflow:update` on the parent.
*/
@Get('/:workflowId/callable-subworkflows')
@ProjectScope('workflow:update')
async getCallableSubworkflows(
req: AuthenticatedRequest,
_res: unknown,
@Param('workflowId') workflowId: string,
@Query query: ListCallableSubworkflowsQueryDto,
): Promise<CallableSubworkflowsResponse> {
return await this.workflowService.getCallableSubworkflows(req.user, workflowId, query);
}
@Post('/with-node-types')
async getWorkflowsWithNodesIncluded(req: AuthenticatedRequest, res: express.Response) {
try {

View File

@ -280,6 +280,9 @@ describe('Dynamic Credentials API', () => {
});
});
// `authSource=cookie` makes the caller's own n8n session token the identity the
// resolver is asked to validate. The resolver here keys on an external subject
// (`credential-resolver.oauth2-1.0`), so it must not be handed that identity.
describe('when the identity is taken from the session cookie', () => {
it('should refuse to authorize against a resolver that does not resolve n8n users', async () => {
const response = await testServer

View File

@ -112,7 +112,7 @@ function minimalNodeType(name: string, properties: INodeProperties[] = []): INod
};
}
function buildDefaultNodes(): INodeTypeData {
export function buildDefaultNodes(): INodeTypeData {
ScheduleTrigger.prototype.trigger = async () => ({});
return {
'n8n-nodes-base.manualTrigger': {

View File

@ -43,6 +43,7 @@ describe('EnterpriseWorkflowService', () => {
mock(), // workflowPublishHistoryRepository
mock(), // workflowMutationHooks
mock(), // policyEnforcementService
mock(), // nodeTypes
mock(), // credentialsPermissionChecker
);
});

View File

@ -29,6 +29,8 @@ import { v4 as uuid } from 'uuid';
import { mock } from 'vitest-mock-extended';
import { ActiveWorkflowManager } from '@/active-workflow-manager';
import { SubworkflowPolicyDenialError } from '@/errors/subworkflow-policy-denial.error';
import { SubworkflowPolicyChecker } from '@/executions/pre-execution-checks/subworkflow-policy-checker';
import type { ExternalHooks } from '@/external-hooks';
import { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus';
import { NodeTypes } from '@/node-types';
@ -1459,3 +1461,227 @@ describe('publishAsSystem()', () => {
expect(workflowPublicationNotifier.requestDrain).not.toHaveBeenCalled();
});
});
describe('delete() and the workflowsFromAList caller policy', () => {
test('denies a caller whose allowlist entry no longer refers to a live workflow', async () => {
const owner = await createOwner();
const member = await createMember();
const allowlistedCaller = await createWorkflow({}, owner);
const subworkflow = await createWorkflow(
{
settings: { callerPolicy: 'workflowsFromAList', callerIds: allowlistedCaller.id },
},
owner,
);
await workflowService.archive(owner, allowlistedCaller.id);
await workflowService.delete(owner, allowlistedCaller.id);
const reusedIdWorkflow = await createWorkflow({ id: allowlistedCaller.id }, member);
const storedSubworkflow = await workflowRepository.findOneByOrFail({ id: subworkflow.id });
await expect(
Container.get(SubworkflowPolicyChecker).check(
{ id: storedSubworkflow.id, settings: storedSubworkflow.settings },
reusedIdWorkflow.id,
),
).rejects.toThrowError(SubworkflowPolicyDenialError);
});
test('keeps the allowlist entry while the caller is only archived', async () => {
const owner = await createOwner();
const allowlistedCaller = await createWorkflow({}, owner);
const callerIds = `keepMe,\n${allowlistedCaller.id}`;
const subworkflow = await createWorkflow(
{ settings: { callerPolicy: 'workflowsFromAList', callerIds } },
owner,
);
await workflowService.archive(owner, allowlistedCaller.id);
const afterArchive = await workflowRepository.findOneByOrFail({ id: subworkflow.id });
expect(afterArchive.settings).toEqual({ callerPolicy: 'workflowsFromAList', callerIds });
await workflowService.delete(owner, allowlistedCaller.id);
const afterDelete = await workflowRepository.findOneByOrFail({ id: subworkflow.id });
expect(afterDelete.settings).toEqual({
callerPolicy: 'workflowsFromAList',
callerIds: 'keepMe',
});
});
test('allows a live allowlisted caller owned by another project', async () => {
const owner = await createOwner();
const member = await createMember();
const allowlistedCaller = await createWorkflow({}, member);
const subworkflow = await createWorkflow(
{
settings: { callerPolicy: 'workflowsFromAList', callerIds: allowlistedCaller.id },
},
owner,
);
const storedSubworkflow = await workflowRepository.findOneByOrFail({ id: subworkflow.id });
await expect(
Container.get(SubworkflowPolicyChecker).check(
{ id: storedSubworkflow.id, settings: storedSubworkflow.settings },
allowlistedCaller.id,
),
).resolves.toBeUndefined();
});
test('keeps the other settings of an allowlist it rewrites', async () => {
const owner = await createOwner();
const caller = await createWorkflow({}, owner);
const subworkflow = await createWorkflow(
{
settings: {
callerPolicy: 'workflowsFromAList',
callerIds: `${caller.id},another-caller`,
executionOrder: 'v1',
timezone: 'Europe/Berlin',
},
},
owner,
);
await workflowService.archive(owner, caller.id);
await workflowService.delete(owner, caller.id);
const after = await workflowRepository.findOneByOrFail({ id: subworkflow.id });
expect(after.settings).toEqual({
callerPolicy: 'workflowsFromAList',
callerIds: 'another-caller',
executionOrder: 'v1',
timezone: 'Europe/Berlin',
});
});
test('leaves an allowlist alone when the deleted id only appears inside another entry', async () => {
const owner = await createOwner();
const caller = await createWorkflow({}, owner);
const subworkflow = await createWorkflow(
{
settings: {
callerPolicy: 'workflowsFromAList',
callerIds: `${caller.id}-suffix, ${caller.id}-other`,
},
},
owner,
);
const before = await workflowRepository.findOneByOrFail({ id: subworkflow.id });
await workflowService.archive(owner, caller.id);
await workflowService.delete(owner, caller.id);
const after = await workflowRepository.findOneByOrFail({ id: subworkflow.id });
expect(after.settings).toEqual(before.settings);
expect(after.updatedAt).toEqual(before.updatedAt);
});
test('takes several deleted ids out of one allowlist', async () => {
const owner = await createOwner();
const first = await createWorkflow({}, owner);
const second = await createWorkflow({}, owner);
const subworkflow = await createWorkflow(
{
settings: {
callerPolicy: 'workflowsFromAList',
callerIds: `${first.id},keepMe,${second.id}`,
},
},
owner,
);
await workflowRepository.removeFromCallerAllowlists([first.id, second.id], {});
const after = await workflowRepository.findOneByOrFail({ id: subworkflow.id });
expect(after.settings).toEqual({ callerPolicy: 'workflowsFromAList', callerIds: 'keepMe' });
});
test('leaves every allowlist untouched when the deleted workflow is on none of them', async () => {
const owner = await createOwner();
const allowlistedCaller = await createWorkflow({}, owner);
const subworkflow = await createWorkflow(
{
settings: {
callerPolicy: 'workflowsFromAList',
callerIds: `${allowlistedCaller.id},another-caller`,
},
},
owner,
);
const before = await workflowRepository.findOneByOrFail({ id: subworkflow.id });
const unrelatedWorkflow = await createWorkflow({}, owner);
await workflowService.archive(owner, unrelatedWorkflow.id);
await workflowService.delete(owner, unrelatedWorkflow.id);
const after = await workflowRepository.findOneByOrFail({ id: subworkflow.id });
expect(after.settings).toEqual(before.settings);
expect(after.updatedAt).toEqual(before.updatedAt);
});
// A character the `json` column keeps but the binary JSON form cannot hold.
const NO_BINARY_JSON_FORM = String.fromCharCode(0);
test('deletes a workflow while an allowlist elsewhere holds a value json alone can carry', async () => {
const owner = await createOwner();
const subworkflow = await createWorkflow(
{
settings: {
callerPolicy: 'workflowsFromAList',
callerIds: `another-caller${NO_BINARY_JSON_FORM}`,
},
},
owner,
);
const before = await workflowRepository.findOneByOrFail({ id: subworkflow.id });
const unrelatedWorkflow = await createWorkflow({}, owner);
await workflowService.archive(owner, unrelatedWorkflow.id);
await workflowService.delete(owner, unrelatedWorkflow.id);
await expect(workflowRepository.findOneBy({ id: unrelatedWorkflow.id })).resolves.toBeNull();
const after = await workflowRepository.findOneByOrFail({ id: subworkflow.id });
expect(after.settings).toEqual(before.settings);
});
test('rewrites an allowlist that holds a value json alone can carry', async () => {
const owner = await createOwner();
const caller = await createWorkflow({}, owner);
const subworkflow = await createWorkflow(
{
settings: {
callerPolicy: 'workflowsFromAList',
callerIds: `${caller.id},another-caller`,
errorWorkflow: `error-workflow${NO_BINARY_JSON_FORM}`,
},
},
owner,
);
await workflowService.archive(owner, caller.id);
await workflowService.delete(owner, caller.id);
const after = await workflowRepository.findOneByOrFail({ id: subworkflow.id });
expect(after.settings).toEqual({
callerPolicy: 'workflowsFromAList',
callerIds: 'another-caller',
errorWorkflow: `error-workflow${NO_BINARY_JSON_FORM}`,
});
});
});

View File

@ -38,6 +38,7 @@ import {
calculateWorkflowChecksum,
type INode,
} from 'n8n-workflow';
import { ExecuteWorkflow } from 'n8n-nodes-base/nodes/ExecuteWorkflow/ExecuteWorkflow/ExecuteWorkflow.node';
import { v4 as uuid } from 'uuid';
import { ActiveWorkflowManager } from '@/active-workflow-manager';
@ -103,7 +104,11 @@ beforeAll(async () => {
saveCredential = affixRoleToSaveCredential('credential:owner');
await utils.initNodeTypes();
// Use the real node description to test parameter defaults and visibility.
await utils.initNodeTypes({
...utils.buildDefaultNodes(),
'n8n-nodes-base.executeWorkflow': { type: new ExecuteWorkflow(), sourcePath: '' },
});
});
beforeEach(async () => {
@ -1359,6 +1364,131 @@ describe('PATCH /workflows/:workflowId', () => {
},
);
describe('sub-workflow loaded from a URL or file', () => {
const UNINSPECTABLE_SOURCE_MESSAGE =
'The \'Sub\' node loads its sub-workflow from a source that is resolved only when the workflow runs, so its credential access cannot be checked on save. Use the "Database" or "Define Below" source instead.';
const remoteSubworkflowNode = (workflowUrl = 'https://example.com/workflow.json') => ({
id: 'uuid-sub-1',
name: 'Sub',
type: 'n8n-nodes-base.executeWorkflow',
typeVersion: 1.1,
position: [0, 0] as [number, number],
parameters: { source: 'url', workflowUrl },
});
it('rejects a member adding such a node to a shared workflow', async () => {
const workflow = await createWorkflow({}, owner);
await shareWorkflowWithUsers(workflow, [member]);
const response = await authMemberAgent.patch(`/workflows/${workflow.id}`).send({
versionId: workflow.versionId,
nodes: [remoteSubworkflowNode()],
});
expect(response.statusCode).toBe(400);
expect(response.body.message).toBe(UNINSPECTABLE_SOURCE_MESSAGE);
});
it('rejects a member creating a workflow with such a node', async () => {
const response = await authMemberAgent.post('/workflows').send({
name: 'test',
active: false,
connections: {},
nodes: [remoteSubworkflowNode()],
});
expect(response.statusCode).toBe(400);
expect(response.body.message).toBe(UNINSPECTABLE_SOURCE_MESSAGE);
});
it('rejects a member adding the HITL tool variant of such a node', async () => {
const workflow = await createWorkflow({}, owner);
await shareWorkflowWithUsers(workflow, [member]);
const response = await authMemberAgent.patch(`/workflows/${workflow.id}`).send({
versionId: workflow.versionId,
nodes: [{ ...remoteSubworkflowNode(), type: 'n8n-nodes-base.executeWorkflowHitlTool' }],
});
expect(response.statusCode).toBe(400);
expect(response.body.message).toBe(UNINSPECTABLE_SOURCE_MESSAGE);
});
it('rejects a member changing an execution setting of an existing node', async () => {
const workflow = await createWorkflow({ nodes: [remoteSubworkflowNode()] }, owner);
await shareWorkflowWithUsers(workflow, [member]);
const response = await authMemberAgent.patch(`/workflows/${workflow.id}`).send({
versionId: workflow.versionId,
nodes: [{ ...remoteSubworkflowNode(), retryOnFail: true }],
});
expect(response.statusCode).toBe(400);
expect(response.body.message).toBe(UNINSPECTABLE_SOURCE_MESSAGE);
});
it('preserves a renamed URL node when the editor omits default and hidden parameters', async () => {
// The editor omits the default mode and the old database workflow ID.
const stored = {
...remoteSubworkflowNode(),
parameters: {
...remoteSubworkflowNode().parameters,
mode: 'once',
workflowId: { __rl: true, value: 'old-workflow', mode: 'id' },
},
};
const workflow = await createWorkflow({ nodes: [stored] }, owner);
await shareWorkflowWithUsers(workflow, [member]);
const response = await authMemberAgent.patch(`/workflows/${workflow.id}`).send({
versionId: workflow.versionId,
nodes: [{ ...remoteSubworkflowNode(), name: 'Renamed' }],
});
expect(response.statusCode).toBe(200);
const saved = await workflowRepository.findOneByOrFail({ id: workflow.id });
expect(saved.nodes).toEqual([{ ...stored, name: 'Renamed' }]);
});
it('rejects a member changing a stored URL and leaves the node unchanged', async () => {
const workflow = await createWorkflow({ nodes: [remoteSubworkflowNode()] }, owner);
await shareWorkflowWithUsers(workflow, [member]);
const changed = await authMemberAgent.patch(`/workflows/${workflow.id}`).send({
versionId: workflow.versionId,
nodes: [remoteSubworkflowNode('https://example.com/other.json')],
});
expect(changed.statusCode).toBe(400);
expect(changed.body.message).toBe(UNINSPECTABLE_SOURCE_MESSAGE);
const saved = await workflowRepository.findOneByOrFail({ id: workflow.id });
expect(saved.nodes).toEqual([remoteSubworkflowNode()]);
});
it('lets the owner create and change such a node', async () => {
const createResponse = await authOwnerAgent.post('/workflows').send({
name: 'test',
active: false,
connections: {},
nodes: [remoteSubworkflowNode()],
});
expect(createResponse.statusCode).toBe(200);
const { id, versionId } = createResponse.body.data;
const response = await authOwnerAgent.patch(`/workflows/${id}`).send({
versionId,
nodes: [remoteSubworkflowNode('https://example.com/other.json')],
});
expect(response.statusCode).toBe(200);
expect(response.body.data.nodes[0].parameters.workflowUrl).toBe(
'https://example.com/other.json',
);
});
});
describe('restoring a history version', () => {
const nodeWith = (credential: { id: string; name: string }, url = 'https://v1.test') => ({
id: 'uuid-restore-1',
@ -1468,6 +1598,60 @@ describe('PATCH /workflows/:workflowId', () => {
});
});
describe('sub-workflow with an inspectable source', () => {
const databaseSubworkflowNode = (workflowId: string) => ({
id: 'uuid-sub-db',
name: 'Sub',
type: 'n8n-nodes-base.executeWorkflow',
typeVersion: 1.2,
position: [0, 0] as [number, number],
parameters: {
source: 'database',
workflowId: { __rl: true, value: workflowId, mode: 'id' },
},
});
it('lets a member change a stored node that loads its sub-workflow from the database', async () => {
const workflow = await createWorkflow(
{ nodes: [databaseSubworkflowNode('wf-old')] },
owner,
);
await shareWorkflowWithUsers(workflow, [member]);
const response = await authMemberAgent.patch(`/workflows/${workflow.id}`).send({
versionId: workflow.versionId,
nodes: [databaseSubworkflowNode('wf-new')],
});
expect(response.statusCode).toBe(200);
expect(response.body.data.nodes[0].parameters.workflowId.value).toBe('wf-new');
});
it('lets a member create a workflow with a node that defines its sub-workflow inline', async () => {
const response = await authMemberAgent.post('/workflows').send({
name: 'test',
active: false,
connections: {},
nodes: [
{
id: 'uuid-sub-inline',
name: 'Sub',
type: 'n8n-nodes-base.executeWorkflow',
typeVersion: 1.2,
position: [0, 0],
parameters: {
source: 'parameter',
workflowJson: JSON.stringify({ nodes: [], connections: {} }),
},
},
],
});
expect(response.statusCode).toBe(200);
expect(response.body.data.nodes[0].parameters.source).toBe('parameter');
});
});
describe('credentials in an agent node parameter', () => {
// The agent node keeps the model credential and its tool credentials in
// the hidden `inlineAgent` parameter, not in `node.credentials`.

View File

@ -23,6 +23,7 @@ import type {
Role,
WorkflowHistory,
WorkflowEntity,
Project,
} from '@n8n/db';
import {
ProjectRepository,
@ -1968,185 +1969,26 @@ describe('GET /workflows', () => {
expect(response.body.data).toHaveLength(1); // Should return all workflows when nodeTypes is empty
});
describe('includeCallableSubworkflows', () => {
const executeWorkflowTriggerNode = (id = uuid()) => ({
id,
name: 'When Executed by Another Workflow',
type: 'n8n-nodes-base.executeWorkflowTrigger',
parameters: {},
typeVersion: 1,
position: [0, 0] as [number, number],
test('should ignore callable sub-workflow filter keys', async () => {
const parentWorkflow = await createWorkflow({ name: 'Parent' }, member);
const anyPolicyWorkflow = await createWorkflow(
{ name: 'Any Policy', settings: { callerPolicy: 'any' } },
owner,
);
const filter = JSON.stringify({
includeCallableSubworkflows: true,
parentWorkflowId: parentWorkflow.id,
});
test('should include workflows callable by the parent workflow based on callerPolicy', async () => {
const parentWorkflow = await createWorkflow({ name: 'Parent' }, member);
const response = await authMemberAgent
.get('/workflows')
.query(`filter=${filter}`)
.expect(200);
// callerPolicy: 'any' — callable by everyone
const anyPolicyWorkflow = await createWorkflow(
{
name: 'Any Policy',
nodes: [executeWorkflowTriggerNode()],
settings: { callerPolicy: 'any' },
},
owner,
);
// callerPolicy: 'workflowsFromAList' — parentWorkflow is explicitly listed
const fromListWorkflow = await createWorkflow(
{
name: 'From List Policy',
nodes: [executeWorkflowTriggerNode()],
settings: {
callerPolicy: 'workflowsFromAList',
callerIds: parentWorkflow.id,
},
},
owner,
);
// callerPolicy: 'none' — callable by nobody
await createWorkflow(
{
name: 'None Policy',
nodes: [executeWorkflowTriggerNode()],
settings: { callerPolicy: 'none' },
},
owner,
);
const filter = JSON.stringify({
triggerNodeTypes: ['n8n-nodes-base.executeWorkflowTrigger'],
includeCallableSubworkflows: true,
parentWorkflowId: parentWorkflow.id,
});
const response = await authMemberAgent
.get('/workflows')
.query(`filter=${filter}`)
.expect(200);
const returnedIds = response.body.data.map((w: { id: string }) => w.id) as string[];
expect(returnedIds).toContain(anyPolicyWorkflow.id);
expect(returnedIds).toContain(fromListWorkflow.id);
expect(returnedIds).not.toContain(
(
await createWorkflow(
{
name: 'None Policy Check',
nodes: [executeWorkflowTriggerNode()],
settings: { callerPolicy: 'none' },
},
owner,
)
).id,
);
});
test('should not include callable workflows when includeCallableSubworkflows is false', async () => {
const parentWorkflow = await createWorkflow({ name: 'Parent' }, member);
const anyPolicyWorkflow = await createWorkflow(
{
name: 'Any Policy',
nodes: [executeWorkflowTriggerNode()],
settings: { callerPolicy: 'any' },
},
owner,
);
const filter = JSON.stringify({
triggerNodeTypes: ['n8n-nodes-base.executeWorkflowTrigger'],
includeCallableSubworkflows: false,
parentWorkflowId: parentWorkflow.id,
});
const response = await authMemberAgent
.get('/workflows')
.query(`filter=${filter}`)
.expect(200);
const returnedIds = response.body.data.map((w: { id: string }) => w.id) as string[];
// member doesn't own anyPolicyWorkflow, so without expansion it should not be visible
expect(returnedIds).not.toContain(anyPolicyWorkflow.id);
});
test('should not expand the list when the requester cannot read the parent workflow', async () => {
// Parent workflow owned by `owner`, inaccessible to `member`.
const parentWorkflow = await createWorkflow({ name: 'Owner Parent' }, owner);
// Subworkflow owned by the same owner, callable by same-owner workflows.
const sameOwnerSubworkflow = await createWorkflow(
{
name: 'Same Owner Sub',
nodes: [executeWorkflowTriggerNode()],
settings: { callerPolicy: 'workflowsFromSameOwner' },
},
owner,
);
// Subworkflow that explicitly allowlists the inaccessible parent id.
const fromListSubworkflow = await createWorkflow(
{
name: 'From List Sub',
nodes: [executeWorkflowTriggerNode()],
settings: {
callerPolicy: 'workflowsFromAList',
callerIds: parentWorkflow.id,
},
},
owner,
);
const filter = JSON.stringify({
triggerNodeTypes: ['n8n-nodes-base.executeWorkflowTrigger'],
includeCallableSubworkflows: true,
parentWorkflowId: parentWorkflow.id,
});
const response = await authMemberAgent
.get('/workflows')
.query(`filter=${filter}`)
.expect(200);
const returnedIds = response.body.data.map((w: { id: string }) => w.id) as string[];
// member cannot read the supplied parent workflow, so callable
// subworkflows must not be exposed regardless of caller policy.
expect(returnedIds).not.toContain(sameOwnerSubworkflow.id);
expect(returnedIds).not.toContain(fromListSubworkflow.id);
});
test('should match callerIds by whole id, not by substring', async () => {
const parentWorkflow = await createWorkflow({ name: 'Parent' }, member);
// Allowlist contains the parent id only as a substring of a longer id.
const substringSubworkflow = await createWorkflow(
{
name: 'Substring Allowlist',
nodes: [executeWorkflowTriggerNode()],
settings: {
callerPolicy: 'workflowsFromAList',
callerIds: `${parentWorkflow.id}-extra-suffix`,
},
},
owner,
);
const filter = JSON.stringify({
triggerNodeTypes: ['n8n-nodes-base.executeWorkflowTrigger'],
includeCallableSubworkflows: true,
parentWorkflowId: parentWorkflow.id,
});
const response = await authMemberAgent
.get('/workflows')
.query(`filter=${filter}`)
.expect(200);
const returnedIds = response.body.data.map((w: { id: string }) => w.id) as string[];
// parentWorkflow.id is only a substring of the allowlisted id, so it must
// not be treated as an allowed caller.
expect(returnedIds).not.toContain(substringSubworkflow.id);
});
const returnedIds = response.body.data.map((w: { id: string }) => w.id) as string[];
expect(returnedIds).toEqual([parentWorkflow.id]);
expect(returnedIds).not.toContain(anyPolicyWorkflow.id);
});
});
@ -5851,6 +5693,157 @@ describe('GET /workflows/:workflowId/executions/last-successful', () => {
});
});
describe('GET /workflows/:workflowId/callable-subworkflows', () => {
const executeWorkflowTriggerNode = () => ({
id: uuid(),
name: 'When Executed by Another Workflow',
type: 'n8n-nodes-base.executeWorkflowTrigger',
parameters: {},
typeVersion: 1,
position: [0, 0] as [number, number],
});
const createSubworkflow = async (
name: string,
settings: WorkflowEntity['settings'],
userOrProject: User | Project,
) =>
await createWorkflow({ name, nodes: [executeWorkflowTriggerNode()], settings }, userOrProject);
const getCallableIds = async (agent: SuperAgentTest, parentWorkflowId: string) => {
const response = await agent
.get(`/workflows/${parentWorkflowId}/callable-subworkflows`)
.expect(200);
return (response.body.data.data as Array<{ id: string }>).map((w) => w.id);
};
test('should list readable workflows and workflows that allow the parent explicitly', async () => {
const parentWorkflow = await createWorkflow({ name: 'Parent' }, member);
const ownSubworkflow = await createSubworkflow('Own', undefined, member);
const anyPolicyWorkflow = await createSubworkflow('Any', { callerPolicy: 'any' }, owner);
const fromListWorkflow = await createSubworkflow(
'From List',
{ callerPolicy: 'workflowsFromAList', callerIds: `other-id, ${parentWorkflow.id}` },
owner,
);
const nonePolicyWorkflow = await createSubworkflow('None', { callerPolicy: 'none' }, owner);
const substringListWorkflow = await createSubworkflow(
'Substring List',
{ callerPolicy: 'workflowsFromAList', callerIds: `${parentWorkflow.id}-extra-suffix` },
owner,
);
// Readable, but has no Execute Workflow Trigger node.
await createWorkflow({ name: 'No Trigger' }, member);
// Callable by the parent, but has no Execute Workflow Trigger node.
const callableNoTriggerWorkflow = await createWorkflow(
{ name: 'Any Policy No Trigger', settings: { callerPolicy: 'any' } },
owner,
);
const returnedIds = await getCallableIds(authMemberAgent, parentWorkflow.id);
expect(returnedIds.sort()).toEqual(
[ownSubworkflow.id, anyPolicyWorkflow.id, fromListWorkflow.id].sort(),
);
expect(returnedIds).not.toContain(nonePolicyWorkflow.id);
expect(returnedIds).not.toContain(substringListWorkflow.id);
expect(returnedIds).not.toContain(callableNoTriggerWorkflow.id);
});
test('should not list unshared workflows of the user who shared the parent', async () => {
const sharedParent = await createWorkflow({ name: 'Shared Parent' }, owner);
await shareWorkflowWithUsers(sharedParent, [member]);
const sameOwnerWorkflow = await createSubworkflow(
'Same Owner',
{ callerPolicy: 'workflowsFromSameOwner' },
owner,
);
const defaultPolicyWorkflow = await createSubworkflow('Default Policy', undefined, owner);
const returnedIds = await getCallableIds(authMemberAgent, sharedParent.id);
expect(returnedIds).not.toContain(sameOwnerWorkflow.id);
expect(returnedIds).not.toContain(defaultPolicyWorkflow.id);
});
test('should return only the minimal fields and the project name for team projects', async () => {
const parentWorkflow = await createWorkflow({ name: 'Parent' }, owner);
const teamProject = await createTeamProject('Team Project', owner);
const teamWorkflow = await createSubworkflow('Team Sub', undefined, teamProject);
const personalWorkflow = await createSubworkflow('Personal Sub', undefined, owner);
const response = await authOwnerAgent
.get(`/workflows/${parentWorkflow.id}/callable-subworkflows`)
.query({ select: '["nodes"]', filter: '{"nodeTypes":["n8n-nodes-base.code"]}' })
.expect(200);
expect(response.body.data.count).toBe(2);
expect(response.body.data.data).toEqual(
arrayContaining([
{
id: teamWorkflow.id,
name: 'Team Sub',
active: false,
isArchived: false,
projectName: 'Team Project',
},
{ id: personalWorkflow.id, name: 'Personal Sub', active: false, isArchived: false },
]),
);
});
test('should filter by name and paginate', async () => {
const parentWorkflow = await createWorkflow({ name: 'Parent' }, member);
await createSubworkflow('Alpha One', undefined, member);
await createSubworkflow('Alpha Two', undefined, member);
await createSubworkflow('Beta', undefined, member);
const response = await authMemberAgent
.get(`/workflows/${parentWorkflow.id}/callable-subworkflows`)
.query({ query: 'alpha', take: 1 })
.expect(200);
expect(response.body.data.count).toBe(2);
expect(response.body.data.data).toHaveLength(1);
expect(response.body.data.data[0].name).toMatch(/^Alpha/);
});
test('should match the search query against workflow names only', async () => {
const parentWorkflow = await createWorkflow({ name: 'Parent' }, member);
await createWorkflow(
{
name: 'Callable',
description: 'contains uniqueterm',
nodes: [executeWorkflowTriggerNode()],
settings: { callerPolicy: 'any' },
},
owner,
);
const response = await authMemberAgent
.get(`/workflows/${parentWorkflow.id}/callable-subworkflows`)
.query({ query: 'uniqueterm' })
.expect(200);
expect(response.body.data).toEqual({ count: 0, data: [] });
});
test('should forbid users who cannot update the parent workflow', async () => {
const teamProject = await createTeamProject('Team Project', owner);
await linkUserToProject(member, teamProject, 'project:viewer');
const parentWorkflow = await createWorkflow({ name: 'Parent' }, teamProject);
await authMemberAgent.get(`/workflows/${parentWorkflow.id}/callable-subworkflows`).expect(403);
});
test('should forbid users who cannot access the parent workflow', async () => {
const parentWorkflow = await createWorkflow({ name: 'Parent' }, owner);
await createSubworkflow('Allows Parent', { callerPolicy: 'any' }, owner);
await authMemberAgent.get(`/workflows/${parentWorkflow.id}/callable-subworkflows`).expect(403);
});
});
describe('POST /workflows/with-node-types', () => {
const NODE_TYPE = 'n8n-nodes-base.set';

View File

@ -1,5 +1,11 @@
import { AxiosError, AxiosHeaders } from 'axios';
import get from 'lodash/get';
import { Workflow, createEmptyRunExecutionData } from 'n8n-workflow';
import {
NodeApiError,
NodeOperationError,
Workflow,
createEmptyRunExecutionData,
} from 'n8n-workflow';
import type {
DeclarativeRestApiSettings,
ICredentialDataDecryptedObject,
@ -2428,9 +2434,11 @@ describe('RoutingNode', () => {
const createRoutingNode = ({
continueOnFail,
requestErrorUrl,
requestError = new Error('Request failed'),
}: {
continueOnFail: boolean;
requestErrorUrl?: string;
requestError?: Error;
}) => {
const items: INodeExecutionData[] = [
{ json: { id: 'first' } },
@ -2483,7 +2491,7 @@ describe('RoutingNode', () => {
context.helpers.httpRequest = vi.fn(async (requestOptions: IHttpRequestOptions) => {
requestUrls.push(requestOptions.url);
if (requestOptions.url === requestErrorUrl) {
throw new Error('Request failed');
throw requestError;
}
return { body: { url: requestOptions.url } };
});
@ -2541,6 +2549,41 @@ describe('RoutingNode', () => {
expect(result?.[0]?.[1]?.error).toMatchObject({ message: 'Request failed' });
expect(result?.[0]?.[2]?.json).toEqual({ url: '/items/third' });
});
test('returns a request error without request headers when continue on fail is enabled', async () => {
const requestError = new AxiosError(
'Request failed with status code 500',
'ERR_BAD_RESPONSE',
{ headers: new AxiosHeaders({ Authorization: 'Bearer secret-token' }) },
);
const { run } = createRoutingNode({
continueOnFail: true,
requestErrorUrl: '/items/second',
requestError,
});
const result = await run;
const error = result?.[0]?.[1]?.error;
expect(error).toBeInstanceOf(NodeApiError);
expect(error).toMatchObject({ context: { itemIndex: 1, runIndex: 0 } });
expect(JSON.stringify(error)).not.toContain('secret-token');
});
test('keeps a node operation error when continue on fail is enabled', async () => {
const requestError = new NodeOperationError(mock<INode>(), 'Post-receive action failed');
const { run } = createRoutingNode({
continueOnFail: true,
requestErrorUrl: '/items/second',
requestError,
});
const result = await run;
const error = result?.[0]?.[1]?.error;
expect(error).toBe(requestError);
expect(error).toMatchObject({ context: { itemIndex: 1, runIndex: 0 } });
});
});
describe('itemIndex', () => {

View File

@ -311,24 +311,27 @@ export class RoutingNode {
const error = responseData.reason;
const nodeError =
error instanceof NodeApiError || error instanceof NodeOperationError
? error
: new NodeApiError(node, error as JsonObject, {
runIndex,
itemIndex,
message: error?.message,
description: error?.description,
httpCode:
error.isAxiosError && error.response ? String(error.response?.status) : 'none',
});
set(nodeError, 'context.itemIndex', itemIndex);
set(nodeError, 'context.runIndex', runIndex);
if (itemContext[itemIndex].thisArgs?.continueOnFail()) {
returnData.push({ json: {}, error: error as NodeApiError });
returnData.push({ json: {}, error: nodeError });
continue;
}
if (error instanceof NodeApiError) {
set(error, 'context.itemIndex', itemIndex);
set(error, 'context.runIndex', runIndex);
throw error;
}
throw new NodeApiError(node, error as JsonObject, {
runIndex,
itemIndex,
message: error?.message,
description: error?.description,
httpCode: error.isAxiosError && error.response ? String(error.response?.status) : 'none',
});
throw nodeError;
}
if (itemContext[itemIndex].requestData.maxResults) {

View File

@ -681,6 +681,7 @@
"chatHub.toolsManager.add": "Add",
"chatHub.toolsManager.addTool": "Add tool",
"chatHub.toolsManager.remove": "Remove",
"chatHub.toolsManager.roleBlocked": "Your role cannot use this tool. Remove it, or ask an admin for a different role.",
"chatHub.toolsManager.configure": "Configure",
"chatHub.toolsManager.manageTools": "Manage tools",
"chatHub.toolsManager.enableTool": "Enable tool",

View File

@ -1,4 +1,9 @@
import { getLastSuccessfulExecution, getNewWorkflowData, getWorkflows } from './workflows';
import {
getCallableSubworkflows,
getLastSuccessfulExecution,
getNewWorkflowData,
getWorkflows,
} from './workflows';
import { DEFAULT_NEW_WORKFLOW_NAME, DEFAULT_SETTINGS } from '@/app/constants/workflows';
import * as apiUtils from '@n8n/rest-api-client';
import type { IRestApiContext } from '@n8n/rest-api-client';
@ -46,6 +51,38 @@ describe('API: workflows', () => {
});
});
describe('getCallableSubworkflows', () => {
const mockContext = {
baseUrl: 'http://test-base-url',
sessionId: 'test-session',
pushRef: 'test-ref',
} as IRestApiContext;
it('should call the parent-specific endpoint with the search and page params', async () => {
const response = {
count: 1,
data: [{ id: 'sub-1', name: 'Sub', active: false, isArchived: false }],
};
const makeRestApiRequestSpy = vi
.spyOn(apiUtils, 'makeRestApiRequest')
.mockResolvedValue(response);
const result = await getCallableSubworkflows(mockContext, 'parent-id', {
query: 'sub',
skip: 40,
take: 20,
});
expect(makeRestApiRequestSpy).toHaveBeenCalledWith(
mockContext,
'GET',
'/workflows/parent-id/callable-subworkflows',
{ query: 'sub', skip: 40, take: 20 },
);
expect(result).toEqual(response);
});
});
describe('getLastSuccessfulExecution', () => {
let mockContext: IRestApiContext;
let makeRestApiRequestSpy: MockInstance;

View File

@ -12,7 +12,7 @@ import type {
} from '@/features/execution/executions/executions.types';
import { DEFAULT_NEW_WORKFLOW_NAME, DEFAULT_SETTINGS } from '@/app/constants';
import { isEmpty } from '@/app/utils/typesUtils';
import type { ExecutionRedactionQueryDto } from '@n8n/api-types';
import type { CallableSubworkflowsResponse, ExecutionRedactionQueryDto } from '@n8n/api-types';
import type { IRestApiContext } from '@n8n/rest-api-client';
import type {
ExecutionFilters,
@ -120,6 +120,19 @@ export async function getWorkflowsAndFolders(
});
}
export async function getCallableSubworkflows(
context: IRestApiContext,
parentWorkflowId: string,
params: { query?: string; skip: number; take: number },
) {
return await makeRestApiRequest<CallableSubworkflowsResponse>(
context,
'GET',
`/workflows/${parentWorkflowId}/callable-subworkflows`,
params,
);
}
export async function getActiveWorkflows(context: IRestApiContext) {
return await makeRestApiRequest<string[]>(context, 'GET', '/active-workflows');
}

View File

@ -4,12 +4,14 @@ import { useWorkflowsListStore } from '@/app/stores/workflowsList.store';
import type { IWorkflowDb, WorkflowListResource, WorkflowResource } from '@/Interface';
import * as apiUtils from '@n8n/rest-api-client';
import { createTestWorkflow } from '@/__tests__/mocks';
import { DEFAULT_WORKFLOW_PAGE_SIZE } from '@/app/constants';
import type { WorkflowHistory } from '@n8n/rest-api-client';
vi.mock('@/app/api/workflows', () => ({
getWorkflows: vi.fn(),
getWorkflow: vi.fn(),
getWorkflowsAndFolders: vi.fn(),
getCallableSubworkflows: vi.fn(),
getWorkflowsWithNodesIncluded: vi.fn(),
getActiveWorkflows: vi.fn(),
workflowExists: vi.fn(),
@ -328,6 +330,44 @@ describe('useWorkflowsListStore', () => {
});
});
describe('fetchCallableSubworkflowsPage', () => {
it('should request the page and keep the rows out of the workflows cache', async () => {
const response = {
count: 1,
data: [{ id: 'sub-1', name: 'Sub', active: false, isArchived: false }],
};
vi.mocked(workflowsApi).getCallableSubworkflows.mockResolvedValue(response);
const result = await workflowsListStore.fetchCallableSubworkflowsPage(
'parent-id',
2,
10,
'sub',
);
expect(workflowsApi.getCallableSubworkflows).toHaveBeenCalledWith(
expect.anything(),
'parent-id',
{ query: 'sub', skip: 10, take: 10 },
);
expect(result).toEqual(response);
expect(workflowsListStore.workflowsById).toEqual({});
expect(workflowsListStore.totalWorkflowCount).toBe(0);
});
it('should request the first page with the default page size and no query', async () => {
vi.mocked(workflowsApi).getCallableSubworkflows.mockResolvedValue({ count: 0, data: [] });
await workflowsListStore.fetchCallableSubworkflowsPage('parent-id');
expect(workflowsApi.getCallableSubworkflows).toHaveBeenCalledWith(
expect.anything(),
'parent-id',
{ query: undefined, skip: 0, take: DEFAULT_WORKFLOW_PAGE_SIZE },
);
});
});
describe('searchWorkflows', () => {
it('should search workflows with filters', async () => {
const mockWorkflows = [{ id: '1', name: 'Test Workflow' }] as IWorkflowDb[];

View File

@ -18,8 +18,6 @@ export type WorkflowListFilters = {
parentFolderId?: string;
availableInMCP?: boolean;
triggerNodeTypes?: string[];
includeCallableSubworkflows?: boolean;
parentWorkflowId?: string;
};
const ALL_PROJECTS_KEY = '__all_projects__';
@ -184,6 +182,21 @@ export const useWorkflowsListStore = defineStore(STORES.WORKFLOWS_LIST, () => {
return data;
}
// Callable rows can belong to workflows the user cannot read, so they are not
// added to the workflows cache.
async function fetchCallableSubworkflowsPage(
parentWorkflowId: string,
page = 1,
pageSize = DEFAULT_WORKFLOW_PAGE_SIZE,
query?: string,
) {
return await workflowsApi.getCallableSubworkflows(rootStore.restApiContext, parentWorkflowId, {
query,
skip: (page - 1) * pageSize,
take: pageSize,
});
}
async function searchWorkflows({
projectId,
ids,
@ -329,6 +342,7 @@ export const useWorkflowsListStore = defineStore(STORES.WORKFLOWS_LIST, () => {
// Fetching
fetchWorkflowsPageWithCount,
fetchWorkflowsPage,
fetchCallableSubworkflowsPage,
searchWorkflows,
fetchAllWorkflows,
fetchWorkflow,

View File

@ -18,6 +18,8 @@ const props = defineProps<{
installing?: boolean;
/** Non-admin cannot install; button is disabled with contact-admin tooltip. */
installDisabled?: boolean;
/** The user's role cannot use this tool type, so only removal is possible. */
roleBlocked?: boolean;
}>();
const emit = defineEmits<{
@ -62,7 +64,10 @@ const restriction = computed(() => getNodeItemRestriction(props.nodeType.name));
ref="rowRef"
:class="[
$style.item,
{ [$style.configured]: mode === 'configured', [$style.restricted]: !!restriction },
{
[$style.configured]: mode === 'configured',
[$style.restricted]: !!restriction || roleBlocked,
},
]"
>
<div :class="$style.iconWrapper">
@ -101,8 +106,23 @@ const restriction = computed(() => getNodeItemRestriction(props.nodeType.name));
:anchor="rowRef"
/>
<N8nTooltip
v-if="roleBlocked"
:content="i18n.baseText('chatHub.toolsManager.roleBlocked')"
placement="top"
>
<span
:class="$style.marker"
tabindex="0"
role="img"
:aria-label="i18n.baseText('chatHub.toolsManager.roleBlocked')"
>
<N8nIcon icon="lock" size="small" data-test-id="chat-tool-role-blocked-icon" />
</span>
</N8nTooltip>
<template v-if="mode === 'configured'">
<N8nTooltip :content="i18n.baseText('chatHub.toolsManager.configure')">
<N8nTooltip v-if="!roleBlocked" :content="i18n.baseText('chatHub.toolsManager.configure')">
<N8nIconButton
icon="settings"
variant="ghost"
@ -123,6 +143,7 @@ const restriction = computed(() => getNodeItemRestriction(props.nodeType.name));
</N8nTooltip>
<N8nTooltip
v-if="!roleBlocked"
:content="
enabled
? i18n.baseText('chatHub.toolsManager.disableTool')
@ -203,6 +224,20 @@ const restriction = computed(() => getNodeItemRestriction(props.nodeType.name));
}
}
// Matches the lock marker of the availability policy popover.
.marker {
display: inline-flex;
align-items: center;
justify-content: center;
padding: var(--spacing--3xs);
color: var(--color--text--tint-1);
&:focus-visible {
outline: var(--focus--border-width) solid var(--focus--border-color);
outline-offset: 2px;
}
}
.iconWrapper {
flex-shrink: 0;
width: 32px;

View File

@ -1,4 +1,4 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { createComponentRenderer } from '@/__tests__/render';
import { createTestingPinia } from '@pinia/testing';
import { flushPromises } from '@vue/test-utils';
@ -16,6 +16,7 @@ import userEvent from '@testing-library/user-event';
import { MODAL_CONFIRM } from '@/app/constants';
import type { ChatHubToolDto } from '@n8n/api-types';
import { createToolSettingsStub } from '../__tests__/toolSettingsStub';
import { hasRole } from '@/app/utils/rbac/checks/hasRole';
vi.mock('virtual:node-popularity-data', () => ({
default: [
@ -289,6 +290,119 @@ describe('ToolsManagerModal', () => {
expect(queryByText('Tool With Inputs')).toBeNull();
});
describe('role restrictions', () => {
const toolType = (name: string, displayName: string): INodeTypeDescription => ({
...TOOL_NODE_TYPE_A,
name,
displayName,
});
const CHAT_USER_BLOCKED_TOOLS = [
toolType('n8n-nodes-base.executeCommandTool', 'Execute Command Tool'),
toolType('n8n-nodes-base.gitTool', 'Git Tool'),
toolType('@n8n/n8n-nodes-langchain.toolCode', 'Code Tool'),
toolType('n8n-nodes-base.messageAnAgentTool', 'Message an Agent Tool'),
];
const CHAT_TOOL = toolType('@n8n/n8n-nodes-langchain.chatTool', 'Chat Tool');
const ROLE_TEST_TOOLS = [...CHAT_USER_BLOCKED_TOOLS, CHAT_TOOL];
beforeEach(() => {
const getDefaultNodeType = nodeTypesStore.getNodeType;
nodeTypesStore.getNodeType = vi
.fn()
.mockImplementation(
(name: string) =>
ROLE_TEST_TOOLS.find((tool) => tool.name === name) ?? getDefaultNodeType(name),
);
nodeTypesStore.visibleNodeTypesByOutputConnectionTypeNames = {
[NodeConnectionTypes.AiTool]: [
'n8n-nodes-base.toolA',
...ROLE_TEST_TOOLS.map((tool) => tool.name),
],
};
});
afterEach(() => {
vi.mocked(hasRole).mockReturnValue(false);
});
it('should hide tools that chat users are not allowed to add', () => {
vi.mocked(hasRole).mockReturnValue(true);
const { container, getByText } = renderComponent({ props: defaultProps() });
expect(getByText('Tool A')).toBeTruthy();
expect(getAvailableItems(container)).toHaveLength(1);
});
it('should show those tools to users who can build workflows', () => {
vi.mocked(hasRole).mockReturnValue(false);
const { getByText } = renderComponent({ props: defaultProps() });
for (const tool of CHAT_USER_BLOCKED_TOOLS) {
expect(getByText(tool.displayName)).toBeTruthy();
}
});
it('should hide the Chat tool for users who can build workflows', () => {
vi.mocked(hasRole).mockReturnValue(false);
const { queryByText } = renderComponent({ props: defaultProps() });
expect(queryByText('Chat Tool')).toBeNull();
});
describe('configured tool that the role blocks', () => {
beforeEach(() => {
chatStore.configuredTools = [
createMockToolDto({
name: 'Shell',
type: 'n8n-nodes-base.executeCommandTool',
}),
];
});
it('should mark the tool and keep only the remove action for a chat user', () => {
vi.mocked(hasRole).mockReturnValue(true);
const { container } = renderComponent({ props: defaultProps() });
const [configured] = getConfiguredItems(container);
expect(
configured.querySelector('[data-test-id="chat-tool-role-blocked-icon"]'),
).toBeTruthy();
expect(getActionButtons(configured)).toHaveLength(1);
expect(getToggle(configured)).toBeNull();
});
it('should remove the tool after confirmation for a chat user', async () => {
vi.mocked(hasRole).mockReturnValue(true);
mockConfirm.mockResolvedValue(MODAL_CONFIRM);
const { container } = renderComponent({ props: defaultProps() });
const [removeBtn] = getActionButtons(getConfiguredItems(container)[0]);
await userEvent.click(removeBtn);
await waitFor(() => {
expect(chatStore.removeConfiguredTool).toHaveBeenCalledWith('tool-1');
});
});
it('should show the tool as usual for users who can build workflows', () => {
vi.mocked(hasRole).mockReturnValue(false);
const { container } = renderComponent({ props: defaultProps() });
const [configured] = getConfiguredItems(container);
expect(configured.querySelector('[data-test-id="chat-tool-role-blocked-icon"]')).toBeNull();
expect(getActionButtons(configured)).toHaveLength(2);
expect(getToggle(configured)).toBeTruthy();
});
});
});
describe('tool actions', () => {
it('should switch to settings view when adding a tool', async () => {
const { getAllByText, getByTestId } = renderComponent({ props: defaultProps() });

View File

@ -16,10 +16,7 @@ import {
import { useI18n } from '@n8n/i18n';
import { useDebounceFn } from '@vueuse/core';
import { NodeConnectionTypes, type INode, type INodeTypeDescription } from 'n8n-workflow';
import {
ALWAYS_BLOCKED_CHAT_HUB_TOOL_TYPES,
CHAT_USER_BLOCKED_CHAT_HUB_TOOL_TYPES,
} from '@n8n/api-types';
import { isChatHubToolTypeAllowed } from '@n8n/api-types';
import type { ChatHubToolDto } from '@n8n/api-types';
import { computed, onMounted, ref, watch } from 'vue';
import { getDebounceTime } from '@n8n/composables/useDebounce';
@ -121,13 +118,11 @@ const settingsIsValid = ref(false);
const tools = computed<ChatHubToolDto[]>(() => chatStore.configuredTools);
const excludedToolTypes = computed(() => {
const blocked = [...ALWAYS_BLOCKED_CHAT_HUB_TOOL_TYPES];
if (hasRole(['global:chatUser'])) {
blocked.push(...CHAT_USER_BLOCKED_CHAT_HUB_TOOL_TYPES);
}
return blocked;
});
const isChatUser = computed(() => hasRole(['global:chatUser']));
function isToolTypeExcluded(type: string): boolean {
return !isChatHubToolTypeAllowed(type, { isChatUser: isChatUser.value });
}
function resolveToolNodeType(name: string): INodeTypeDescription | null {
return (
@ -159,9 +154,7 @@ const availableToolTypes = computed<INodeTypeDescription[]>(() => {
.map((name) => resolveToolNodeType(name))
.filter(
(nodeType): nodeType is INodeTypeDescription =>
nodeType !== null &&
!excludedToolTypes.value.includes(nodeType.name) &&
!hasInputs(nodeType),
nodeType !== null && !isToolTypeExcluded(nodeType.name) && !hasInputs(nodeType),
)
.sort((a, b) => {
const popA = nodePopularityMap.get(a.name) ?? 0;
@ -213,7 +206,7 @@ const matchingAvailableTools = computed(() => {
if (
!resolved ||
seen.has(resolved.name) ||
excludedToolTypes.value.includes(resolved.name) ||
isToolTypeExcluded(resolved.name) ||
hasInputs(resolved)
) {
continue;
@ -443,6 +436,7 @@ function handleSettingsChangeName(name: string) {
:node-type="getNodeType(tool)!"
:configured-node="tool.definition"
:enabled="agentToolIds ? agentToolIds.includes(tool.definition.id) : tool.enabled"
:role-blocked="isToolTypeExcluded(tool.definition.type)"
mode="configured"
@configure="handleConfigureTool(tool)"
@remove="handleRemoveTool(tool.definition.id)"

View File

@ -98,6 +98,7 @@ const {
searchFilter,
onSearchFilter,
getWorkflowName,
getWorkflowBaseName,
applyDefaultExecuteWorkflowNodeName,
populateNextWorkflowsPage,
setWorkflowsResources,
@ -184,8 +185,7 @@ function onInputChange(workflowId: NodeParameterValue): void {
cachedResultUrl: getWorkflowUrl(workflowId),
};
if (isListMode.value) {
const resource = workflowsListStore.getWorkflowById(workflowId);
if (resource?.name) {
if (getWorkflowBaseName(workflowId)) {
params.cachedResultName = getWorkflowName(workflowId);
}
}

View File

@ -12,6 +12,7 @@ import { type MockedStore, mockedStore } from '@/__tests__/utils';
import { createTestWorkflow } from '@/__tests__/mocks';
import type { IWorkflowDb } from '@/Interface';
import type { Router } from 'vue-router';
import type { Scope } from '@n8n/permissions';
import { createTestingPinia } from '@pinia/testing';
const useCanvasOperations = vi.hoisted(() => vi.fn());
@ -20,6 +21,22 @@ vi.mock('@/app/composables/useCanvasOperations', () => ({
useCanvasOperations,
}));
const documentScopes = vi.hoisted(() => ({ value: [] as string[] }));
// The document store exposes `scopes` as read-only, so the tests provide a stand-in.
vi.mock('@/app/stores/workflowDocument.store', async (importOriginal) => {
const actual = await importOriginal<typeof import('@/app/stores/workflowDocument.store')>();
const { computed } = await import('vue');
return {
...actual,
injectWorkflowDocumentStore: () =>
computed(() => ({
documentId: actual.createWorkflowDocumentId(''),
scopes: documentScopes.value,
})),
};
});
describe('useWorkflowResourcesLocator', () => {
let workflowsListStoreMock: MockedStore<typeof useWorkflowsListStore>;
let workflowsStoreMock: MockedStore<typeof useWorkflowsStore>;
@ -32,6 +49,7 @@ describe('useWorkflowResourcesLocator', () => {
beforeEach(() => {
vi.clearAllMocks();
documentScopes.value = [];
createTestingPinia();
workflowsListStoreMock = mockedStore(useWorkflowsListStore);
@ -238,40 +256,86 @@ describe('useWorkflowResourcesLocator', () => {
expect(hasMoreWorkflowsToLoad.value).toBe(false);
});
it('should include callable subworkflow filter when inside an open workflow', async () => {
workflowsListStoreMock.fetchWorkflowsPage.mockResolvedValue([]);
workflowsStoreMock.workflowId = 'parent-wf-id';
describe('callable sub-workflows', () => {
const setUpParentWorkflow = (scopes: Scope[]) => {
workflowsStoreMock.workflowId = 'parent-wf-id';
workflowsStoreMock.isNewWorkflow = false;
documentScopes.value = scopes;
};
const { populateNextWorkflowsPage } = useWorkflowResourcesLocator(routerMock);
await populateNextWorkflowsPage();
it('should list callable sub-workflows when the user can update a saved parent', async () => {
setUpParentWorkflow(['workflow:read', 'workflow:update']);
workflowsListStoreMock.fetchCallableSubworkflowsPage.mockResolvedValue({
count: 3,
data: [
{ id: '1', name: 'Team Sub', active: true, isArchived: false, projectName: 'Team' },
{ id: '2', name: 'Personal Sub', active: false, isArchived: true },
],
});
expect(workflowsListStoreMock.fetchWorkflowsPage).toHaveBeenCalledWith(
undefined,
1,
40,
'updatedAt:desc',
{
triggerNodeTypes: ['n8n-nodes-base.executeWorkflowTrigger'],
includeCallableSubworkflows: true,
parentWorkflowId: 'parent-wf-id',
},
);
});
const { onSearchFilter, workflowsResources, hasMoreWorkflowsToLoad, getWorkflowName } =
useWorkflowResourcesLocator(routerMock);
await onSearchFilter('sub');
it('should not include callable subworkflow filter when no workflow is open', async () => {
workflowsListStoreMock.fetchWorkflowsPage.mockResolvedValue([]);
workflowsStoreMock.workflowId = '';
expect(workflowsListStoreMock.fetchCallableSubworkflowsPage).toHaveBeenCalledWith(
'parent-wf-id',
1,
40,
'sub',
);
expect(workflowsListStoreMock.fetchWorkflowsPage).not.toHaveBeenCalled();
expect(workflowsResources.value).toEqual([
{
name: 'Team — Team Sub',
value: '1',
url: expect.any(String) as string,
isArchived: false,
active: true,
},
{
name: 'Personal Sub',
value: '2',
url: expect.any(String) as string,
isArchived: true,
active: false,
},
]);
expect(hasMoreWorkflowsToLoad.value).toBe(true);
expect(getWorkflowName('1')).toBe('Team — Team Sub');
// Callable rows can belong to workflows the user cannot read, so they
// must stay out of the workflows cache.
expect(workflowsListStoreMock.addWorkflow).not.toHaveBeenCalled();
expect(workflowsListStoreMock.setWorkflows).not.toHaveBeenCalled();
});
const { populateNextWorkflowsPage } = useWorkflowResourcesLocator(routerMock);
await populateNextWorkflowsPage();
it('should list readable workflows when the user cannot update the parent', async () => {
setUpParentWorkflow(['workflow:read']);
workflowsListStoreMock.fetchWorkflowsPage.mockResolvedValue([]);
expect(workflowsListStoreMock.fetchWorkflowsPage).toHaveBeenCalledWith(
undefined,
1,
40,
'updatedAt:desc',
{ triggerNodeTypes: ['n8n-nodes-base.executeWorkflowTrigger'] },
);
const { populateNextWorkflowsPage } = useWorkflowResourcesLocator(routerMock);
await populateNextWorkflowsPage();
expect(workflowsListStoreMock.fetchCallableSubworkflowsPage).not.toHaveBeenCalled();
expect(workflowsListStoreMock.fetchWorkflowsPage).toHaveBeenCalledWith(
undefined,
1,
40,
'updatedAt:desc',
{ triggerNodeTypes: ['n8n-nodes-base.executeWorkflowTrigger'] },
);
});
it('should list readable workflows when the parent is not saved', async () => {
setUpParentWorkflow(['workflow:read', 'workflow:update']);
workflowsStoreMock.isNewWorkflow = true;
workflowsListStoreMock.fetchWorkflowsPage.mockResolvedValue([]);
const { populateNextWorkflowsPage } = useWorkflowResourcesLocator(routerMock);
await populateNextWorkflowsPage();
expect(workflowsListStoreMock.fetchCallableSubworkflowsPage).not.toHaveBeenCalled();
expect(workflowsListStoreMock.fetchWorkflowsPage).toHaveBeenCalled();
});
});
it('should handle multiple page loads correctly', async () => {

View File

@ -1,4 +1,6 @@
import { ref, computed } from 'vue';
import type { CallableSubworkflow } from '@n8n/api-types';
import { getResourcePermissions } from '@n8n/permissions';
import { useWorkflowsListStore, type WorkflowListFilters } from '@/app/stores/workflowsList.store';
import { useWorkflowsStore } from '@/app/stores/workflows.store';
import type { Router } from 'vue-router';
@ -25,6 +27,9 @@ export function useWorkflowResourcesLocator(router: Router) {
const currentPage = ref(0);
const PAGE_SIZE = 40;
const totalCount = ref(0);
// Rows from the callable sub-workflows endpoint. Some of them can belong to
// workflows the user cannot read, so they stay out of the workflows cache.
const callableWorkflowsById = ref<Record<string, CallableSubworkflow>>({});
const hasMoreWorkflowsToLoad = computed(() => totalCount.value > workflowsResources.value.length);
@ -37,16 +42,24 @@ export function useWorkflowResourcesLocator(router: Router) {
return workflow.name;
}
function constructCallableName(workflow: CallableSubworkflow) {
return workflow.projectName ? `${workflow.projectName} — ${workflow.name}` : workflow.name;
}
function getWorkflowName(id: string): string {
const workflow = workflowsListStore.getWorkflowById(id);
if (workflow) {
return constructName(workflow);
}
const callableWorkflow = callableWorkflowsById.value[id];
if (callableWorkflow) {
return constructCallableName(callableWorkflow);
}
return id;
}
function getWorkflowBaseName(id: string): string | null {
const workflow = workflowsListStore.getWorkflowById(id);
const workflow = workflowsListStore.getWorkflowById(id) ?? callableWorkflowsById.value[id];
if (workflow) {
return workflow.name;
}
@ -68,25 +81,46 @@ export function useWorkflowResourcesLocator(router: Router) {
};
}
async function populateNextWorkflowsPage(reset = false) {
if (reset) {
currentPage.value = 0;
function callableWorkflowToResourceMapper(workflow: CallableSubworkflow) {
return {
name: constructCallableName(workflow),
value: workflow.id,
url: getWorkflowUrl(workflow.id),
isArchived: workflow.isArchived,
active: workflow.active,
};
}
// The callable list also holds workflows whose caller policy allows this
// parent. The backend serves it only to users who can edit a saved parent.
function canListCallableSubworkflows() {
return (
!workflowsStore.isNewWorkflow &&
getResourcePermissions(workflowDocumentStore.value.scopes).workflow.update === true
);
}
async function fetchNextResources() {
if (canListCallableSubworkflows()) {
const { count, data } = await workflowsListStore.fetchCallableSubworkflowsPage(
workflowsStore.workflowId,
currentPage.value,
PAGE_SIZE,
searchFilter.value || undefined,
);
for (const workflow of data) {
callableWorkflowsById.value[workflow.id] = workflow;
}
totalCount.value = count;
return data.map(callableWorkflowToResourceMapper);
}
currentPage.value++;
const parentWorkflowId = workflowsStore.workflowId;
const filters: WorkflowListFilters = {
triggerNodeTypes: ['n8n-nodes-base.executeWorkflowTrigger'],
};
if (searchFilter.value) {
filters.query = searchFilter.value;
}
if (parentWorkflowId) {
filters.includeCallableSubworkflows = true;
filters.parentWorkflowId = parentWorkflowId;
}
const workflows = await workflowsListStore.fetchWorkflowsPage(
undefined,
@ -96,11 +130,22 @@ export function useWorkflowResourcesLocator(router: Router) {
filters,
);
totalCount.value = workflowsListStore.totalWorkflowCount;
return workflows.map(workflowDbToResourceMapper);
}
async function populateNextWorkflowsPage(reset = false) {
if (reset) {
currentPage.value = 0;
}
currentPage.value++;
const resources = await fetchNextResources();
if (reset) {
workflowsResources.value = workflows.map(workflowDbToResourceMapper);
workflowsResources.value = resources;
} else {
workflowsResources.value.push(...workflows.map(workflowDbToResourceMapper));
workflowsResources.value.push(...resources);
}
}
@ -140,6 +185,7 @@ export function useWorkflowResourcesLocator(router: Router) {
getWorkflowUrl,
onSearchFilter,
getWorkflowName,
getWorkflowBaseName,
applyDefaultExecuteWorkflowNodeName,
populateNextWorkflowsPage,
setWorkflowsResources,

View File

@ -1,13 +1,30 @@
import type {
ICredentialDataDecryptedObject,
ICredentialTestRequest,
ICredentialType,
IHttpRequestOptions,
INodeProperties,
import {
getCredentialOwnRequestAllowedDomains,
UserError,
type ICredentialDataDecryptedObject,
type ICredentialTestRequest,
type ICredentialType,
type IHttpRequestOptions,
type INodeProperties,
} from 'n8n-workflow';
import { getTokenRequestClient, TOKEN_REQUEST_TIMEOUT } from './common/token-request';
// `region` is interpolated into the token-exchange host, so the stored value must stay
// inside this fixed set. The options field and the runtime check both read it from here.
const REGION_OPTIONS = [
{ name: 'Asia Pacific, Japan, and China', value: 'apjc.amp' },
{ name: 'Europe', value: 'eu.amp' },
{ name: 'North America', value: 'amp' },
] as const;
type Region = (typeof REGION_OPTIONS)[number]['value'];
function assertSupportedRegion(region: unknown): asserts region is Region {
if (typeof region !== 'string' || !REGION_OPTIONS.some((option) => option.value === region)) {
throw new UserError('Unsupported Cisco Secure Endpoint region');
}
}
export class CiscoSecureEndpointApi implements ICredentialType {
name = 'ciscoSecureEndpointApi';
@ -28,20 +45,7 @@ export class CiscoSecureEndpointApi implements ICredentialType {
displayName: 'Region',
name: 'region',
type: 'options',
options: [
{
name: 'Asia Pacific, Japan, and China',
value: 'apjc.amp',
},
{
name: 'Europe',
value: 'eu.amp',
},
{
name: 'North America',
value: 'amp',
},
],
options: [...REGION_OPTIONS],
default: 'amp',
},
{
@ -69,10 +73,14 @@ export class CiscoSecureEndpointApi implements ICredentialType {
): Promise<IHttpRequestOptions> {
const clientId = credentials.clientId as string;
const clientSecret = credentials.clientSecret as string;
const region = credentials.region as string;
const region = credentials.region;
assertSupportedRegion(region);
// `region` is interpolated into the request host, so gate the token POSTs on SSRF protection.
const http = getTokenRequestClient('user-controlled');
// Apply the credential's own allow-list to the token requests it issues.
const allowedDomains = getCredentialOwnRequestAllowedDomains(credentials);
const secureXToken = (await http.request({
url: `https://visibility.${region}.cisco.com/iroh/oauth2/token`,
@ -90,6 +98,7 @@ export class CiscoSecureEndpointApi implements ICredentialType {
},
json: true,
timeout: TOKEN_REQUEST_TIMEOUT,
allowedDomains,
})) as { access_token: string };
const secureEndpointToken = (await http.request({
@ -105,6 +114,7 @@ export class CiscoSecureEndpointApi implements ICredentialType {
},
json: true,
timeout: TOKEN_REQUEST_TIMEOUT,
allowedDomains,
})) as { access_token: string };
const requestOptionsWithAuth: IHttpRequestOptions = {

View File

@ -108,3 +108,37 @@ describe('CiscoSecureEndpointApi Credential (integration)', () => {
expect(result.headers?.Authorization).toBe('Bearer secure-endpoint-token');
});
});
/**
* Drives the real `OutboundHttp` client (no host rewrite) so the credential's own
* allow-list is enforced against the actual token host. The restriction gate rejects
* before any socket opens, so no request reaches the network.
*/
describe('CiscoSecureEndpointApi Credential (integration) allow-list enforcement', () => {
const credential = new CiscoSecureEndpointApi();
beforeEach(() => {
const config = new SsrfProtectionConfig();
Object.assign(config, { enabled: true, allowedIpRanges: ['127.0.0.0/8'] });
Container.set(SsrfProtectionConfig, config);
});
afterEach(() => {
Container.reset();
});
test('refuses the token exchange when the allow-list excludes the token host', async () => {
await expect(
credential.authenticate(
{
region: 'eu.amp',
clientId: 'client-id',
clientSecret: 'client-secret',
allowedHttpRequestDomains: 'domains',
allowedDomains: 'example.com',
},
{ headers: {}, method: 'GET', url: 'https://api.eu.amp.cisco.com/v3/organizations' },
),
).rejects.toThrow(/Domain not allowed/);
});
});

View File

@ -85,6 +85,41 @@ describe('CiscoSecureEndpointApi Credential', () => {
expect(requestsMock).toHaveBeenCalledWith();
});
it('rejects a region outside the supported list without issuing a request', async () => {
await expect(
credential.authenticate({ ...baseCredentials, region: 'not-a-region' }, requestOptions),
).rejects.toThrow('Unsupported Cisco Secure Endpoint region');
expect(requestMock).not.toHaveBeenCalled();
});
it("applies the credential's allowed domains to the token requests", async () => {
await credential.authenticate(
{
...baseCredentials,
allowedHttpRequestDomains: 'domains',
allowedDomains: '*.amp.cisco.com',
},
requestOptions,
);
expect(requestMock).toHaveBeenCalledTimes(2);
for (const [options] of requestMock.mock.calls) {
expect(options).toEqual(expect.objectContaining({ allowedDomains: '*.amp.cisco.com' }));
}
});
it('rejects a domains restriction with an empty allow-list before issuing a request', async () => {
await expect(
credential.authenticate(
{ ...baseCredentials, allowedHttpRequestDomains: 'domains', allowedDomains: '' },
requestOptions,
),
).rejects.toThrow('No allowed domains specified');
expect(requestMock).not.toHaveBeenCalled();
});
it('attaches the Secure Endpoint access token to the outgoing request', async () => {
const result = await credential.authenticate(baseCredentials, requestOptions);

Some files were not shown because too many files have changed in this diff Show More