chore: Bundle/2.x (backport to release-candidate/2.35.x) (#36561)

Co-authored-by: n8n-assistant[bot] <100856346+n8n-assistant[bot]@users.noreply.github.com>
Co-authored-by: Matsu <matias.huhta@n8n.io>
Co-authored-by: Sam Wooler <swooler592@gmail.com>
Co-authored-by: Dimitri Lavrenük <20122620+dlavrenuek@users.noreply.github.com>
Co-authored-by: Bernhard Wittmann <bernhard.wittmann@n8n.io>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Lorent Lempereur <looorent@users.noreply.github.com>
Co-authored-by: yehorkardash <yehor.kardash@n8n.io>
Co-authored-by: Danny Martini <danny@n8n.io>
Co-authored-by: mfsiega <93014743+mfsiega@users.noreply.github.com>
Co-authored-by: phyllis-noester <102315132+phyllis-noester@users.noreply.github.com>
Co-authored-by: n8n-assistant[bot] <n8n-assistant[bot]@users.noreply.github.com>
Co-authored-by: Emilia <100027345+sovietspaceship@users.noreply.github.com>
Co-authored-by: Robin Braumann <50590409+bjorger@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This commit is contained in:
n8n-assistant[bot]
2026-08-19 08:33:35 +03:00
committed by GitHub
parent fa781db207
commit 00aad642c2
81 changed files with 4505 additions and 831 deletions
@@ -0,0 +1,78 @@
import type { HttpsProxyAgent } from 'https-proxy-agent';
import type http from 'node:http';
import { makeSsrfBridge } from '../../ssrf/__tests__/mock-ssrf-bridge';
import { installProxyConnectionGuard } from '../connection-guards';
type ConnectOpts = Parameters<HttpsProxyAgent<string>['connect']>[1];
describe('installProxyConnectionGuard', () => {
function guarded(connectOpts: { host?: string | null; hostname?: string | null }) {
const connect = vi.fn().mockResolvedValue('SOCKET');
const agent = { connectOpts, connect };
return { agent, connect };
}
it.each([
['host', { host: 'proxy.internal' }],
['hostname', { hostname: 'proxy.internal' }],
])(
'reads the proxy host from `%s` and delegates when the policy allows it',
async (_label, connectOpts) => {
const bridge = makeSsrfBridge();
const { agent, connect } = guarded(connectOpts);
installProxyConnectionGuard(agent, bridge);
const req = {} as http.ClientRequest;
const opts = {} as ConnectOpts;
await expect(agent.connect(req, opts)).resolves.toBe('SOCKET');
expect(bridge.validateConnectionHost).toHaveBeenCalledWith('proxy.internal');
expect(connect).toHaveBeenCalledWith(req, opts);
},
);
it('rejects the connection when the policy denies the proxy host', async () => {
const error = new Error('blocked');
const bridge = makeSsrfBridge({
validateConnectionHost: vi.fn().mockReturnValue({ ok: false, error }),
});
const { agent, connect } = guarded({ host: '169.254.169.254' });
installProxyConnectionGuard(agent, bridge);
await expect(agent.connect({} as http.ClientRequest, {} as ConnectOpts)).rejects.toBe(error);
expect(connect).not.toHaveBeenCalled();
});
it('validates the proxy host on every connection, not once at installation', async () => {
const bridge = makeSsrfBridge();
const { agent } = guarded({ host: 'proxy.internal' });
installProxyConnectionGuard(agent, bridge);
await agent.connect({} as http.ClientRequest, {} as ConnectOpts);
agent.connectOpts.host = 'other-proxy.internal';
await agent.connect({} as http.ClientRequest, {} as ConnectOpts);
expect(bridge.validateConnectionHost).toHaveBeenNthCalledWith(1, 'proxy.internal');
expect(bridge.validateConnectionHost).toHaveBeenNthCalledWith(2, 'other-proxy.internal');
});
it.each([
['missing', {}],
['null', { host: null, hostname: null }],
['empty', { host: '' }],
['empty while `hostname` is set', { host: '', hostname: 'proxy.internal' }],
])('rejects the connection when the proxy host is %s', async (_label, connectOpts) => {
const bridge = makeSsrfBridge();
const { agent, connect } = guarded(connectOpts);
installProxyConnectionGuard(agent, bridge);
await expect(agent.connect({} as http.ClientRequest, {} as ConnectOpts)).rejects.toThrow(
'Cannot determine the host for this connection',
);
expect(connect).not.toHaveBeenCalled();
expect(bridge.validateConnectionHost).not.toHaveBeenCalled();
});
});
@@ -1,24 +1,47 @@
import http from 'node:http';
import type { LookupFunction } from 'node:net';
import type { MockInstance } from 'vitest';
import { makeLookupFn } from '../../ssrf/__tests__/mock-ssrf-bridge';
import { EnvProxyHttpAgent } from '../env-proxy-http-agent';
// Routing/caching is covered in env-proxy-router.test.ts; here we only assert
// the agent's wiring: delegate to the resolved proxy agent, else dispatch
// directly via `super.addRequest`. The proxy agent is mocked and `getProxyForUrl`
// drives which branch runs, so nothing hits the network.
const { getProxyForUrl, proxyAddRequest } = vi.hoisted(() => ({
const { getProxyForUrl, proxyAddRequest, proxyConnect, proxyAgents } = vi.hoisted(() => ({
getProxyForUrl: vi.fn<(url: string) => string>(),
proxyAddRequest: vi.fn(),
proxyConnect: vi.fn(),
proxyAgents: [] as unknown[],
}));
vi.mock('proxy-from-env', () => ({ getProxyForUrl }));
vi.mock('http-proxy-agent', () => ({
HttpProxyAgent: class {
connectOpts: { host: string };
addRequest = proxyAddRequest;
constructor(
proxyUrl: string,
readonly agentOptions?: { lookup?: unknown },
) {
this.connectOpts = { host: new URL(proxyUrl).hostname };
proxyAgents.push(this);
}
async connect(...args: unknown[]): Promise<unknown> {
return await proxyConnect(...args);
}
},
}));
type CreatedProxyAgent = {
agentOptions?: { lookup?: unknown };
connect: (req: unknown, opts: unknown) => Promise<unknown>;
};
const req = {} as http.ClientRequest;
const options = (o: Partial<http.RequestOptions>): http.RequestOptions => o as http.RequestOptions;
@@ -28,6 +51,9 @@ describe('EnvProxyHttpAgent', () => {
beforeEach(() => {
getProxyForUrl.mockReset();
proxyAddRequest.mockReset();
proxyConnect.mockReset();
proxyConnect.mockResolvedValue('SOCKET');
proxyAgents.length = 0;
// `super.addRequest` is the only path that would open a real socket.
// `addRequest` is an internal Agent method untyped on the public types.
superAddRequest = vi
@@ -57,4 +83,37 @@ describe('EnvProxyHttpAgent', () => {
expect(proxyAddRequest).not.toHaveBeenCalled();
expect(superAddRequest).toHaveBeenCalledWith(req, opts);
});
describe('SSRF scope of the resolved proxy agent', () => {
beforeEach(() => {
getProxyForUrl.mockReturnValue('http://proxy.internal:3128');
});
function resolvedProxyAgent(lookup?: LookupFunction): CreatedProxyAgent {
new EnvProxyHttpAgent(lookup).addRequest(req, options({ host: 'a.example', port: 80 }));
return proxyAgents[0] as CreatedProxyAgent;
}
it.each([
['a lookup is given', () => makeLookupFn()],
['no lookup is given', () => undefined],
])('opens the connection to the proxy unchecked when %s', async (_label, makeLookup) => {
const lookup = makeLookup();
const proxyAgent = resolvedProxyAgent(lookup);
await expect(proxyAgent.connect(req, {})).resolves.toBe('SOCKET');
expect(proxyAgent.agentOptions?.lookup).toBeUndefined();
});
it('keeps the secure lookup on its own direct pool', () => {
const lookupFn = makeLookupFn();
const agent = new EnvProxyHttpAgent(lookupFn) as unknown as {
options: { lookup?: unknown };
};
expect(agent.options.lookup).toBe(lookupFn);
});
});
});
@@ -1,25 +1,48 @@
import type http from 'node:http';
import https from 'node:https';
import type { LookupFunction } from 'node:net';
import type { MockInstance } from 'vitest';
import { makeLookupFn } from '../../ssrf/__tests__/mock-ssrf-bridge';
import { EnvProxyHttpsAgent } from '../env-proxy-https-agent';
// Routing/caching is covered in env-proxy-router.test.ts; here we only assert
// the agent's wiring: delegate to the resolved proxy agent, else dispatch
// directly via `super.addRequest`. The proxy agent is mocked and `getProxyForUrl`
// drives which branch runs, so nothing hits the network.
const { getProxyForUrl, proxyAddRequest } = vi.hoisted(() => ({
const { getProxyForUrl, proxyAddRequest, proxyConnect, proxyAgents } = vi.hoisted(() => ({
getProxyForUrl: vi.fn<(url: string) => string>(),
proxyAddRequest: vi.fn(),
proxyConnect: vi.fn(),
proxyAgents: [] as unknown[],
}));
vi.mock('proxy-from-env', () => ({ getProxyForUrl }));
vi.mock('https-proxy-agent', () => ({
HttpsProxyAgent: class {
connectOpts: { host: string };
addRequest = proxyAddRequest;
constructor(
proxyUrl: string,
readonly agentOptions?: { lookup?: unknown },
) {
this.connectOpts = { host: new URL(proxyUrl).hostname };
proxyAgents.push(this);
}
async connect(...args: unknown[]): Promise<unknown> {
return await proxyConnect(...args);
}
},
}));
type CreatedProxyAgent = {
agentOptions?: { lookup?: unknown };
connect: (req: unknown, opts: unknown) => Promise<unknown>;
};
const req = {} as http.ClientRequest;
const options = (o: Partial<https.RequestOptions>): https.RequestOptions =>
o as https.RequestOptions;
@@ -30,6 +53,9 @@ describe('EnvProxyHttpsAgent', () => {
beforeEach(() => {
getProxyForUrl.mockReset();
proxyAddRequest.mockReset();
proxyConnect.mockReset();
proxyConnect.mockResolvedValue('SOCKET');
proxyAgents.length = 0;
// `super.addRequest` is the only path that would open a real socket.
// `addRequest` is an internal Agent method untyped on the public types.
superAddRequest = vi
@@ -59,4 +85,37 @@ describe('EnvProxyHttpsAgent', () => {
expect(proxyAddRequest).not.toHaveBeenCalled();
expect(superAddRequest).toHaveBeenCalledWith(req, opts);
});
describe('SSRF scope of the resolved proxy agent', () => {
beforeEach(() => {
getProxyForUrl.mockReturnValue('http://proxy.internal:3128');
});
function resolvedProxyAgent(lookup?: LookupFunction): CreatedProxyAgent {
new EnvProxyHttpsAgent(lookup).addRequest(req, options({ host: 'a.example', port: 443 }));
return proxyAgents[0] as CreatedProxyAgent;
}
it.each([
['a lookup is given', () => makeLookupFn()],
['no lookup is given', () => undefined],
])('opens the connection to the proxy unchecked when %s', async (_label, makeLookup) => {
const lookup = makeLookup();
const proxyAgent = resolvedProxyAgent(lookup);
await expect(proxyAgent.connect(req, {})).resolves.toBe('SOCKET');
expect(proxyAgent.agentOptions?.lookup).toBeUndefined();
});
it('keeps the secure lookup on its own direct pool', () => {
const lookupFn = makeLookupFn();
const agent = new EnvProxyHttpsAgent(lookupFn) as unknown as {
options: { lookup?: unknown };
};
expect(agent.options.lookup).toBe(lookupFn);
});
});
});
@@ -1,13 +1,21 @@
import { HttpProxyAgent } from 'http-proxy-agent';
import { HttpsProxyAgent } from 'https-proxy-agent';
import dns from 'node:dns';
import http from 'node:http';
import https from 'node:https';
import type { LookupFunction } from 'node:net';
import type { SsrfBridge } from '../../ssrf';
import { makeLookupFn, makeSsrfBridge } from '../../ssrf/__tests__/mock-ssrf-bridge';
import {
makeDenyingLookup,
makeLookupFn,
makeSsrfBridge,
} from '../../ssrf/__tests__/mock-ssrf-bridge';
import { installConnectionGuard, UnknownConnectionHostError } from '../connection-guards';
import { EnvProxyHttpAgent } from '../env-proxy-http-agent';
import { EnvProxyHttpsAgent } from '../env-proxy-https-agent';
import { buildNodeAgents, installConnectionGuard } from '../node-agents';
import { type LocalServer, startServer } from '../local-server';
import { buildNodeAgents, type ProxyUrl } from '../node-agents';
// HttpsProxyAgent stores `lookup` in `connectOpts` rather than `options`
// (unlike http.Agent and HttpProxyAgent which use `options`).
@@ -103,39 +111,25 @@ describe('buildNodeAgents', () => {
});
});
describe('SSRF lookup placement (direct connections only)', () => {
it('proxy: false → injects the secure lookup on both agents', () => {
describe('SSRF lookup placement', () => {
const modes: Array<[string, false | 'env' | ProxyUrl]> = [
['proxy: false', false],
['proxy: env', 'env'],
['proxy: explicit URL', 'http://proxy.internal:3128'],
];
it.each(modes)('%s → injects the secure lookup on both agents', (_label, proxy) => {
const lookupFn = makeLookupFn();
const bridge = makeSsrfBridge({ createSecureLookup: vi.fn().mockReturnValue(lookupFn) });
const { httpAgent, httpsAgent } = buildNodeAgents(false, bridge);
const { httpAgent, httpsAgent } = buildNodeAgents(proxy, bridge);
expect(getAgentLookup(httpAgent)).toBe(lookupFn);
expect(getAgentLookup(httpsAgent)).toBe(lookupFn);
});
it('proxy: env → injects the secure lookup for the direct path', () => {
const lookupFn = makeLookupFn();
const bridge = makeSsrfBridge({ createSecureLookup: vi.fn().mockReturnValue(lookupFn) });
const { httpAgent, httpsAgent } = buildNodeAgents('env', bridge);
expect(getAgentLookup(httpAgent)).toBe(lookupFn);
expect(getAgentLookup(httpsAgent)).toBe(lookupFn);
});
it('proxy: explicit URL → does NOT inject the lookup (proxy validates the target)', () => {
const lookupFn = makeLookupFn();
const bridge = makeSsrfBridge({ createSecureLookup: vi.fn().mockReturnValue(lookupFn) });
const { httpAgent, httpsAgent } = buildNodeAgents('http://proxy.internal:3128', bridge);
expect(getAgentLookup(httpAgent)).toBeUndefined();
expect(getAgentLookup(httpsAgent)).toBeUndefined();
});
it('ssrf disabled → no lookup on the direct path', () => {
const { httpAgent, httpsAgent } = buildNodeAgents('env', 'disabled');
it.each(modes)('%s with SSRF disabled → no lookup on either agent', (_label, proxy) => {
const { httpAgent, httpsAgent } = buildNodeAgents(proxy, 'disabled');
expect(getAgentLookup(httpAgent)).toBeUndefined();
expect(getAgentLookup(httpsAgent)).toBeUndefined();
@@ -185,6 +179,24 @@ describe('buildNodeAgents', () => {
expect(socket).toBe('SOCKET');
});
it.each([
['missing', {}],
['null', { host: null, hostname: null }],
['empty', { host: '' }],
['empty while `hostname` is set', { host: '', hostname: 'target.example' }],
])('rejects the connection when the host is %s', (_label, options) => {
const bridge = makeSsrfBridge();
const { createConnection, original } = guarded(bridge);
const onCreate = vi.fn();
const result = createConnection(options, onCreate);
expect(onCreate).toHaveBeenCalledWith(expect.any(UnknownConnectionHostError));
expect(original).not.toHaveBeenCalled();
expect(result).toBeUndefined();
expect(bridge.validateConnectionHost).not.toHaveBeenCalled();
});
it('passes the raw host through to the bridge (normalization is the services job)', () => {
const bridge = makeSsrfBridge();
const { createConnection } = guarded(bridge);
@@ -214,4 +226,107 @@ describe('buildNodeAgents', () => {
},
);
});
describe('proxy host validation (explicit proxy URL)', () => {
let proxyServer: LocalServer;
let proxyByIp: ProxyUrl;
let proxyByHostname: ProxyUrl;
const realLookup = () => dns.lookup as unknown as LookupFunction;
async function getThroughAgent(agent: http.Agent): Promise<string> {
return await new Promise((resolve, reject) => {
const req = http.get('http://proxied-target.invalid/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'));
});
});
}
beforeEach(async () => {
proxyServer = await startServer((_req, res) => {
res.setHeader('Content-Type', 'text/plain');
res.end('proxied');
});
const { port } = new URL(proxyServer.url);
proxyByIp = `http://127.0.0.1:${port}`;
proxyByHostname = `http://localhost:${port}`;
});
afterEach(async () => {
await proxyServer.close();
});
it('rejects a proxy host the SSRF policy denies, without reaching the proxy', async () => {
const error = new Error(
'The request was blocked because it resolves to a restricted IP address',
);
const bridge = makeSsrfBridge({
createSecureLookup: realLookup,
validateConnectionHost: vi.fn().mockReturnValue({ ok: false, error }),
});
const { httpAgent } = buildNodeAgents(proxyByIp, bridge);
await expect(getThroughAgent(httpAgent)).rejects.toThrow(error.message);
expect(bridge.validateConnectionHost).toHaveBeenCalledWith('127.0.0.1');
expect(proxyServer.captured).toEqual([]);
});
it.each(['httpAgent', 'httpsAgent'] as const)(
'validates the proxy host before the %s opens its connection',
async (which) => {
const error = new Error('blocked');
const bridge = makeSsrfBridge({
validateConnectionHost: vi.fn().mockReturnValue({ ok: false, error }),
});
const agents = buildNodeAgents('http://proxy.internal:3128', bridge);
const agent = agents[which] as unknown as {
connect: (req: unknown, opts: unknown) => Promise<unknown>;
};
await expect(agent.connect({}, {})).rejects.toBe(error);
expect(bridge.validateConnectionHost).toHaveBeenCalledWith('proxy.internal');
},
);
it('rejects a proxy hostname that resolves to a restricted address', async () => {
const error = new Error(
'The request was blocked because it resolves to a restricted IP address',
);
const lookup = makeDenyingLookup(error);
const bridge = makeSsrfBridge({ createSecureLookup: () => lookup });
const { httpAgent } = buildNodeAgents(proxyByHostname, bridge);
await expect(getThroughAgent(httpAgent)).rejects.toThrow(error.message);
expect(lookup).toHaveBeenCalledWith('localhost', expect.any(Object), expect.any(Function));
expect(proxyServer.captured).toEqual([]);
});
it('connects through a proxy host the SSRF policy allows', async () => {
const bridge = makeSsrfBridge({ createSecureLookup: realLookup });
const { httpAgent } = buildNodeAgents(proxyByIp, bridge);
await expect(getThroughAgent(httpAgent)).resolves.toBe('proxied');
expect(bridge.validateConnectionHost).toHaveBeenCalledWith('127.0.0.1');
expect(proxyServer.captured).toEqual(['http://proxied-target.invalid/x']);
});
it('connects through the proxy unchanged when SSRF protection is disabled', async () => {
const { httpAgent } = buildNodeAgents(proxyByHostname, 'disabled');
await expect(getThroughAgent(httpAgent)).resolves.toBe('proxied');
expect(proxyServer.captured).toEqual(['http://proxied-target.invalid/x']);
});
});
});
@@ -1,8 +1,8 @@
import type { Logger } from '@n8n/backend-common';
import { HttpProxyAgent } from 'http-proxy-agent';
import { HttpsProxyAgent } from 'https-proxy-agent';
import http from 'node:http';
import https from 'node:https';
import type http from 'node:http';
import type https from 'node:https';
import { mock } from 'vitest-mock-extended';
import type { SsrfProtectionService } from '../../ssrf';
@@ -30,15 +30,9 @@ function getAgentLookup(agent: http.Agent | https.Agent): unknown {
// ---------------------------------------------------------------------------
describe('getNodeAgent', () => {
it('proxy: false → plain http/https.Agent (no proxy class)', () => {
const { httpAgent, httpsAgent } = makeFacade().transport({ proxy: false }).getNodeAgent();
expect(httpAgent).toBeInstanceOf(http.Agent);
expect(httpsAgent).toBeInstanceOf(https.Agent);
expect(httpAgent).not.toBeInstanceOf(HttpProxyAgent);
expect(httpsAgent).not.toBeInstanceOf(HttpsProxyAgent);
});
// Proxy-mode → agent-class matrix is owned by `node-agents.test.ts`
// (`buildNodeAgents` → `describe('agent classes per proxy mode')`); this is a
// representative case proving the facade forwards to it correctly.
it('proxy: explicit URL → HttpProxyAgent / HttpsProxyAgent', () => {
const { httpAgent, httpsAgent } = makeFacade()
.transport({ proxy: 'http://proxy.internal:3128' })
@@ -48,13 +42,6 @@ describe('getNodeAgent', () => {
expect(httpsAgent).toBeInstanceOf(HttpsProxyAgent);
});
it('proxy: env → custom env-routing agents (http/https.Agent subclasses)', () => {
const { httpAgent, httpsAgent } = makeFacade().transport({ proxy: 'env' }).getNodeAgent();
expect(httpAgent).toBeInstanceOf(http.Agent);
expect(httpsAgent).toBeInstanceOf(https.Agent);
});
it('returns the same agent instances on repeated calls', () => {
const client = makeFacade().transport();
const a1 = client.getNodeAgent();
@@ -79,62 +66,29 @@ describe('getNodeAgent', () => {
// ---------------------------------------------------------------------------
describe('getNodeAgent SSRF lookup injection', () => {
describe('proxy: false', () => {
it('injects createSecureLookup when SSRF is enabled', () => {
const lookupFn = makeLookupFn();
const bridge = makeSsrfBridge({
createSecureLookup: vi.fn().mockReturnValue(lookupFn),
});
const { httpAgent, httpsAgent } = makeFacade()
.transport({ ssrf: bridge, proxy: false })
.getNodeAgent();
expect(bridge.createSecureLookup).toHaveBeenCalledTimes(1);
expect(getAgentLookup(httpAgent)).toBe(lookupFn);
expect(getAgentLookup(httpsAgent)).toBe(lookupFn);
// The full proxy-mode matrix is owned by `node-agents.test.ts`
// (`buildNodeAgents` → `describe('SSRF lookup placement')`); this is a
// representative case proving the facade forwards to it correctly.
it('the agents carry the lookup created by the SSRF bridge', () => {
const lookupFn = makeLookupFn();
const bridge = makeSsrfBridge({
createSecureLookup: vi.fn().mockReturnValue(lookupFn),
});
it('does NOT inject lookup when SSRF is disabled', () => {
const { httpAgent, httpsAgent } = makeFacade()
.transport({ ssrf: 'disabled', proxy: false })
.getNodeAgent();
const { httpAgent, httpsAgent } = makeFacade()
.transport({ ssrf: bridge, proxy: false })
.getNodeAgent();
expect(getAgentLookup(httpAgent)).toBeUndefined();
expect(getAgentLookup(httpsAgent)).toBeUndefined();
});
expect(getAgentLookup(httpAgent)).toBe(lookupFn);
expect(getAgentLookup(httpsAgent)).toBe(lookupFn);
});
describe('proxy: explicit URL', () => {
it('does NOT inject lookup behind an explicit proxy (proxy validates the target)', () => {
const lookupFn = makeLookupFn();
const bridge = makeSsrfBridge({
createSecureLookup: vi.fn().mockReturnValue(lookupFn),
});
const { httpAgent, httpsAgent } = makeFacade()
.transport({ ssrf: bridge, proxy: 'http://proxy.internal:3128' })
.getNodeAgent();
it('no lookup when SSRF is disabled', () => {
const { httpAgent, httpsAgent } = makeFacade()
.transport({ ssrf: 'disabled', proxy: false })
.getNodeAgent();
// SSRF lookup is applied to direct connections only. Behind a proxy it
// would resolve the proxy host, not the final target, so it is omitted.
expect(getAgentLookup(httpAgent)).toBeUndefined();
expect(getAgentLookup(httpsAgent)).toBeUndefined();
});
});
describe('proxy: env', () => {
it('injects createSecureLookup when SSRF is enabled', () => {
const lookupFn = makeLookupFn();
const bridge = makeSsrfBridge({
createSecureLookup: vi.fn().mockReturnValue(lookupFn),
});
const { httpAgent, httpsAgent } = makeFacade()
.transport({ ssrf: bridge, proxy: 'env' })
.getNodeAgent();
expect(bridge.createSecureLookup).toHaveBeenCalledTimes(1);
// EnvProxy* agents inherit from http/https.Agent and pass lookup to super()
expect(getAgentLookup(httpAgent)).toBe(lookupFn);
expect(getAgentLookup(httpsAgent)).toBe(lookupFn);
});
expect(getAgentLookup(httpAgent)).toBeUndefined();
expect(getAgentLookup(httpsAgent)).toBeUndefined();
});
});
@@ -5,13 +5,18 @@ import type { Dispatcher } from 'undici';
import { mock } from 'vitest-mock-extended';
import type { SsrfBridge, SsrfProtectionService } from '../../ssrf';
import { makeLookupFn, makeSsrfBridge } from '../../ssrf/__tests__/mock-ssrf-bridge';
import {
makeDenyingLookup,
makeSsrfBridge,
useCleanProxyEnv,
} from '../../ssrf/__tests__/mock-ssrf-bridge';
import { type LocalServer, startServer } from '../local-server';
import { OutboundHttp } from '../outbound-http';
import { createSsrfInterceptor } from '../undici/transport';
// SSRF enforcement lives in a single place: the dispatcher interceptor.
// This file proves it at two levels:
// SSRF enforcement on the dispatch path lives in the dispatcher interceptor
// (an explicit proxy URI is additionally checked when the dispatcher is built).
// This file proves the interceptor at two levels:
// (a) a direct unit test of `createSsrfInterceptor`, and
// (b) end-to-end tests against a real local server (no mocked `fetch`), so the
// interceptor actually runs and we assert that a 30x cannot smuggle a
@@ -49,6 +54,8 @@ function makeOpts(path: string, origin?: string) {
return { path, origin } as unknown as Dispatcher.DispatchOptions;
}
useCleanProxyEnv();
describe('createSsrfInterceptor', () => {
it('validates the reconstructed target URL and dispatches when allowed', async () => {
const bridge = makeSsrfBridge();
@@ -89,7 +96,7 @@ describe('createSsrfInterceptor', () => {
expect(bridge.validateUrl).not.toHaveBeenCalled();
expect(innerDispatch).not.toHaveBeenCalled();
expect(handler.onResponseError).toHaveBeenCalled();
expect(handler.onResponseError).toHaveBeenCalledWith(null, expect.any(TypeError));
});
it('falls back to onError when onResponseError is unavailable', async () => {
@@ -309,14 +316,123 @@ describe('connect-time secure lookup (DNS rebinding)', () => {
await expect(fetchFn('http://rebind.example/')).rejects.toThrow();
});
it('does not derive a connect-time lookup behind an explicit proxy', () => {
const createSecureLookup = vi.fn(makeLookupFn);
const bridge = makeSsrfBridge({ createSecureLookup });
it('resolves the proxy host through the secure lookup behind an explicit proxy', async () => {
const denied = new Error('blocked: restricted IP address');
const lookupSpy = makeDenyingLookup(denied);
const bridge = makeSsrfBridge({ createSecureLookup: () => lookupSpy });
const fetchFn = makeTransport({
ssrf: bridge,
proxy: 'http://proxy.internal:3128',
}).asCustomFetch();
// Forces the lazy dispatcher to build. The proxy resolves the target, so
// the secure lookup must not be consulted on our side.
makeTransport({ ssrf: bridge, proxy: 'http://proxy.invalid:3128' }).getDispatcher();
const rejection = await fetchFn('http://target.invalid/x').catch((e: unknown) => e);
expect(createSecureLookup).not.toHaveBeenCalled();
expect(lookupSpy).toHaveBeenCalledWith('proxy.internal', expect.anything(), expect.anything());
expect(rootCauseMessage(rejection)).toBe(denied.message);
});
});
describe('proxy host validation', () => {
const realLookup = () => dns.lookup as unknown as LookupFunction;
function denyingBridge(error: Error) {
return makeSsrfBridge({
createSecureLookup: realLookup,
validateConnectionHost: vi.fn().mockReturnValue({ ok: false, error }),
});
}
function bridgeDenyingProxyHost(deniedHostname: string) {
const error = new Error('The proxy host is not permitted by policy');
const bridge = makeSsrfBridge({
createSecureLookup: realLookup,
validateUrl: vi.fn(async (url: string | URL) => {
const hostname = typeof url === 'string' ? new URL(url).hostname : url.hostname;
return await Promise.resolve(
hostname === deniedHostname
? { ok: false as const, error }
: { ok: true as const, result: undefined },
);
}),
});
return { bridge, error };
}
it('rejects an explicit proxy host the policy denies', () => {
const error = new Error('The proxy host is not permitted by policy');
const bridge = denyingBridge(error);
expect(() =>
makeTransport({ ssrf: bridge, proxy: 'http://127.0.0.1:3128' }).getDispatcher(),
).toThrow(error.message);
expect(bridge.validateConnectionHost).toHaveBeenCalledWith('127.0.0.1');
});
it('surfaces the rejection of an explicit proxy host through fetch', async () => {
const error = new Error('The proxy host is not permitted by policy');
const fetchFn = makeTransport({
ssrf: denyingBridge(error),
proxy: 'http://127.0.0.1:3128',
}).asCustomFetch();
await expect(fetchFn('http://target.invalid/x')).rejects.toThrow(error.message);
});
// The environment's proxies describe the deployment, so the policy that decides
// which targets a workflow may reach does not decide them.
it.each([
['HTTP_PROXY', 'http://target.invalid/x'],
['HTTPS_PROXY', 'https://target.invalid/x'],
] as const)('leaves a proxy configured through %s unchecked', async (envKey, target) => {
process.env[envKey] = 'http://127.0.0.1:3128';
const lookupSpy = makeDenyingLookup(new Error('blocked: restricted IP address'));
const { bridge } = bridgeDenyingProxyHost('127.0.0.1');
bridge.createSecureLookup = () => lookupSpy;
const transport = makeTransport({ ssrf: bridge, proxy: 'env' });
expect(() => transport.getDispatcher()).not.toThrow();
await transport
.asCustomFetch()(target)
.catch(() => undefined);
const validated = vi.mocked(bridge.validateUrl).mock.calls.map(([url]) => String(url));
expect(validated).toEqual([target]);
expect(bridge.validateConnectionHost).not.toHaveBeenCalled();
expect(lookupSpy).not.toHaveBeenCalled();
});
it('leaves an explicit proxy unchecked when SSRF protection is disabled', () => {
const bridge = denyingBridge(new Error('The proxy host is not permitted by policy'));
expect(() =>
makeTransport({ ssrf: 'disabled', proxy: 'http://127.0.0.1:3128' }).getDispatcher(),
).not.toThrow();
expect(bridge.validateConnectionHost).not.toHaveBeenCalled();
});
it('serves a target the environment exempts from the direct path', async () => {
const server = await startServer((_req, res) => {
res.writeHead(200, { 'content-type': 'text/plain' });
res.end('ok');
});
process.env.HTTP_PROXY = 'http://proxy.internal:3128';
process.env.NO_PROXY = '127.0.0.1';
const { bridge } = bridgeDenyingProxyHost('proxy.internal');
const fetchFn = makeTransport({ ssrf: bridge, proxy: 'env' }).asCustomFetch();
try {
const res = await fetchFn(`${server.url}/x`);
expect(res.status).toBe(200);
} finally {
await server.close();
}
});
it('leaves the dispatcher untouched when no proxy is configured in the environment', () => {
const bridge = makeSsrfBridge({ createSecureLookup: realLookup });
expect(() => makeTransport({ ssrf: bridge, proxy: 'env' }).getDispatcher()).not.toThrow();
expect(bridge.validateConnectionHost).not.toHaveBeenCalled();
});
});
@@ -4,7 +4,7 @@ import dns from 'node:dns';
import http from 'node:http';
import type { LookupFunction } from 'node:net';
import { makeSsrfBridge } from '../../ssrf/__tests__/mock-ssrf-bridge';
import { makeSsrfBridge, useCleanProxyEnv } from '../../ssrf/__tests__/mock-ssrf-bridge';
import { getBeforeRedirectFn, setAxiosAgents } from '../axios/utils';
import { type LocalServer, startServer } from '../local-server';
import { buildNodeAgents } from '../node-agents';
@@ -31,9 +31,10 @@ async function httpGetWithAgent(url: string, agent: http.Agent): Promise<string>
}
describe('outbound transport integration', () => {
useCleanProxyEnv();
let target: LocalServer;
let proxy: LocalServer;
const ORIGINAL_ENV = { ...process.env };
beforeAll(async () => {
target = await startServer((req, res) => {
@@ -58,18 +59,10 @@ describe('outbound transport integration', () => {
});
beforeEach(() => {
delete process.env.HTTP_PROXY;
delete process.env.HTTPS_PROXY;
delete process.env.NO_PROXY;
delete process.env.ALL_PROXY;
target.clear();
proxy.clear();
});
afterEach(() => {
process.env = { ...ORIGINAL_ENV };
});
describe('setAxiosAgents routing', () => {
it('routes through an explicit custom proxy', async () => {
const config: AxiosRequestConfig = { url: `${target.url}/x`, method: 'GET', proxy: false };
@@ -120,7 +113,7 @@ describe('outbound transport integration', () => {
});
});
describe('SSRF secure lookup is applied to direct connections only', () => {
describe('SSRF secure lookup placement', () => {
it('invokes the secure lookup for a direct (hostname) connection', async () => {
const lookupSpy = vi.fn((hostname: string, options: dns.LookupOptions, onResult: unknown) =>
dns.lookup(hostname, options, onResult as never),
@@ -142,8 +135,17 @@ describe('outbound transport integration', () => {
expect(lookupSpy).toHaveBeenCalledWith('localhost', expect.anything(), expect.anything());
});
it('does NOT invoke the secure lookup when routed through a proxy', async () => {
process.env.HTTP_PROXY = proxy.url;
// An explicit proxy URL can come from a request, so its host is resolved
// through the policy; the environment's is part of the deployment and is not.
it.each([
['leaves the env proxy host to the platform resolver', true],
['invokes the secure lookup for an explicit proxy host', false],
])('%s', async (_case, fromEnv) => {
const { port } = new URL(proxy.url);
const proxyUrl = `http://localhost:${port}`;
if (fromEnv) {
process.env.HTTP_PROXY = proxyUrl;
}
const lookupSpy = vi.fn((hostname: string, options: dns.LookupOptions, onResult: unknown) =>
dns.lookup(hostname, options, onResult as never),
);
@@ -156,14 +158,21 @@ describe('outbound transport integration', () => {
method: 'GET',
proxy: false,
};
setAxiosAgents(config, undefined, undefined, bridge);
setAxiosAgents(config, undefined, fromEnv ? undefined : proxyUrl, bridge);
const res = await axios<{ message: string }>(config);
expect(res.data.message).toBe('proxied');
// The proxy host is an IP (no lookup) and the proxy resolves the final
// target, so the secure lookup is never consulted on our side.
expect(lookupSpy).not.toHaveBeenCalled();
if (fromEnv) {
expect(lookupSpy).not.toHaveBeenCalled();
} else {
expect(lookupSpy).toHaveBeenCalledWith('localhost', expect.anything(), expect.anything());
}
expect(lookupSpy).not.toHaveBeenCalledWith(
'proxied-target.invalid',
expect.anything(),
expect.anything(),
);
});
});
@@ -1,8 +1,10 @@
import type { Logger } from '@n8n/backend-common';
import dns from 'node:dns';
import type { LookupFunction } from 'node:net';
import { mock } from 'vitest-mock-extended';
import type { SsrfBridge } from '../../../ssrf';
import { makeSsrfBridge } from '../../../ssrf/__tests__/mock-ssrf-bridge';
import { makeSsrfBridge, useCleanProxyEnv } from '../../../ssrf/__tests__/mock-ssrf-bridge';
import { executeLegacyRequest } from '../../legacy-request';
import { type LocalServer, startServer } from '../../local-server';
import { configureGlobalAxiosDefaults } from '../config';
@@ -16,8 +18,6 @@ import { httpRequest } from '../request';
configureGlobalAxiosDefaults();
const PROXY_ENV_KEYS = ['HTTP_PROXY', 'HTTPS_PROXY', 'NO_PROXY'] as const;
async function startRedirectServer(): Promise<LocalServer> {
let serverUrl = '';
const server = await startServer((req, res) => {
@@ -67,13 +67,11 @@ function makeBridge(blockedPath: string): { bridge: SsrfBridge; error: Error } {
}
describe('httpRequest manual redirect following with SSRF + proxy', () => {
useCleanProxyEnv();
let server: LocalServer;
const savedEnv: Record<string, string | undefined> = {};
beforeEach(async () => {
for (const key of PROXY_ENV_KEYS) {
savedEnv[key] = process.env[key];
}
// A proxy is configured (engages the manual follower), but loopback is
// exempt so the request connects directly to the local server.
process.env.HTTP_PROXY = 'http://127.0.0.1:1';
@@ -84,13 +82,6 @@ describe('httpRequest manual redirect following with SSRF + proxy', () => {
});
afterEach(async () => {
for (const key of PROXY_ENV_KEYS) {
if (savedEnv[key] === undefined) {
delete process.env[key];
} else {
process.env[key] = savedEnv[key];
}
}
await server.close();
});
@@ -218,6 +209,177 @@ describe('httpRequest manual redirect following with SSRF + proxy', () => {
});
});
describe('proxy host validation across redirect hops', () => {
let proxyServer: LocalServer;
const START_URL = 'http://redirect-target.invalid/start';
const INTERNAL_URL = 'http://redirect-target.invalid/internal';
beforeEach(async () => {
proxyServer = await startServer((req, res) => {
if (req.url === START_URL) {
res.writeHead(302, { Location: INTERNAL_URL });
res.end();
return;
}
res.writeHead(200, { 'content-type': 'text/plain' });
res.end('reached:proxied');
});
});
afterEach(async () => {
await proxyServer.close();
});
const proxyConfig = () => ({
host: 'localhost',
port: Number(new URL(proxyServer.url).port),
protocol: 'http',
});
it('follows the redirect through a proxy the policy allows', async () => {
const bridge = makeSsrfBridge({
createSecureLookup: () => dns.lookup as unknown as LookupFunction,
});
const response = await httpRequest(
{ method: 'GET', url: START_URL, proxy: proxyConfig() },
bridge,
);
expect(response).toBe('reached:proxied');
expect(proxyServer.captured).toEqual([START_URL, INTERNAL_URL]);
});
it('blocks the redirect hop when the proxy host is no longer allowed', async () => {
const denied = new Error(
'The request was blocked because it resolves to a restricted IP address',
);
const lookup = ((hostname: string, options: dns.LookupOptions, onResult: unknown) => {
if (proxyServer.captured.length > 0) {
(onResult as (error: Error | null, address?: unknown, family?: number) => void)(
denied,
options.all ? [] : '',
undefined,
);
return;
}
dns.lookup(hostname, options, onResult as never);
}) as unknown as LookupFunction;
const bridge = makeSsrfBridge({ createSecureLookup: () => lookup });
await expect(
httpRequest({ method: 'GET', url: START_URL, proxy: proxyConfig() }, bridge),
).rejects.toThrow(denied.message);
expect(proxyServer.captured).toEqual([START_URL]);
});
});
describe('proxy host validation before the request is sent', () => {
let proxyServer: LocalServer;
const TARGET_URL = 'http://proxied-target.invalid/x';
beforeEach(async () => {
proxyServer = await startServer((_req, res) => {
res.writeHead(200, { 'content-type': 'text/plain' });
res.end('reached:proxied');
});
});
afterEach(async () => {
await proxyServer.close();
});
const proxyUrl = () => `http://localhost:${new URL(proxyServer.url).port}`;
const proxyConfig = () => ({
host: 'localhost',
port: Number(new URL(proxyServer.url).port),
protocol: 'http',
});
function bridgeDenying(hostname: string, error: Error): SsrfBridge {
return makeSsrfBridge({
createSecureLookup: () => dns.lookup as unknown as LookupFunction,
validateUrl: vi.fn(async (url: string | URL) => {
const host = typeof url === 'string' ? new URL(url).hostname : url.hostname;
return await Promise.resolve(
host === hostname
? { ok: false as const, error }
: { ok: true as const, result: undefined },
);
}),
});
}
const requestPaths = [
[
'httpRequest',
async (bridge: SsrfBridge) =>
await httpRequest({ method: 'GET', url: TARGET_URL, proxy: proxyConfig() }, bridge),
],
[
'executeLegacyRequest',
async (bridge: SsrfBridge) =>
await executeLegacyRequest(
{ uri: TARGET_URL, proxy: proxyUrl() },
bridge,
mock<Logger>(),
),
],
] as const;
it.each(requestPaths)(
'blocks %s when the policy denies the proxy host',
async (_name, send) => {
const error = new Error('The proxy host is not permitted by policy');
await expect(send(bridgeDenying('localhost', error))).rejects.toBe(error);
expect(proxyServer.captured).toEqual([]);
},
);
it.each(requestPaths)('sends %s when the policy allows the proxy host', async (_name, send) => {
const bridge = bridgeDenying('never-matches.invalid', new Error('unused'));
expect(await send(bridge)).toBe('reached:proxied');
expect(bridge.validateUrl).toHaveBeenCalledWith(
expect.objectContaining({ hostname: 'localhost' }),
);
});
it('reports the target URL denial before checking the proxy host', async () => {
const targetError = new Error('The target host is not permitted by policy');
const bridge = makeSsrfBridge({
createSecureLookup: () => dns.lookup as unknown as LookupFunction,
validateUrl: vi.fn(async (url: string | URL) => {
const host = typeof url === 'string' ? new URL(url).hostname : url.hostname;
return await Promise.resolve({
ok: false as const,
error: host === 'localhost' ? new Error('proxy denied') : targetError,
});
}),
});
await expect(
httpRequest({ method: 'GET', url: TARGET_URL, proxy: proxyConfig() }, bridge),
).rejects.toBe(targetError);
});
it('sends the request through the proxy when SSRF protection is disabled', async () => {
const response = await httpRequest({
method: 'GET',
url: TARGET_URL,
proxy: proxyConfig(),
});
expect(response).toBe('reached:proxied');
expect(proxyServer.captured).toEqual([TARGET_URL]);
});
});
describe('credential headers across redirects', () => {
let origin: LocalServer;
let crossOrigin: LocalServer;
@@ -24,6 +24,7 @@ import {
searchForHeader,
setAxiosAgents,
tryParseUrl,
validateProxySsrf,
} from '../utils';
// Agent construction is owned by `buildNodeAgents` (./factory).
@@ -431,6 +432,66 @@ describe('getUrlFromProxyConfig', () => {
});
});
describe('validateProxySsrf', () => {
const denyingBridge = (deniedHostname: string, error: Error) =>
makeSsrfBridge({
validateUrl: vi.fn(async (url: string | URL) => {
const hostname = typeof url === 'string' ? new URL(url).hostname : url.hostname;
return await Promise.resolve(
hostname === deniedHostname
? { ok: false as const, error }
: { ok: true as const, result: undefined },
);
}),
});
it.each([
['a proxy URL string', 'http://proxy.example.com:8080'],
['a proxy config object', { host: 'proxy.example.com', port: 8080 }],
])('throws when the policy denies the host of %s', async (_label, proxyConfig) => {
const error = new Error('The proxy host is not permitted by policy');
await expect(
validateProxySsrf(proxyConfig, denyingBridge('proxy.example.com', error)),
).rejects.toBe(error);
});
it('validates the URL composed from a proxy config object', async () => {
const bridge = makeSsrfBridge();
await validateProxySsrf(
{
protocol: 'https',
host: 'proxy.example.com',
port: 9443,
auth: { username: 'user', password: 'pass' },
},
bridge,
);
expect(bridge.validateUrl).toHaveBeenCalledWith(
expect.objectContaining({ href: 'https://user:pass@proxy.example.com:9443/' }),
);
});
it.each([
['no proxy is configured', undefined],
['the proxy config has no host', { host: '', port: 8080 }],
['the proxy scheme is not supported', 'socks5://proxy.example.com:1080'],
['the proxy value is not a URL', 'not-a-url'],
])('does not consult the policy when %s', async (_label, proxyConfig) => {
const bridge = makeSsrfBridge();
await validateProxySsrf(proxyConfig, bridge);
expect(bridge.validateUrl).not.toHaveBeenCalled();
});
it('does not consult the policy when no bridge is provided', async () => {
await expect(validateProxySsrf('http://proxy.example.com:8080')).resolves.toBeUndefined();
});
});
describe('buildTargetUrl', () => {
it('should return url as-is when no baseURL', () => {
expect(buildTargetUrl('https://example.com/path')).toBe('https://example.com/path');
@@ -20,6 +20,7 @@ import {
searchForHeader,
setAxiosAgents,
throwIfDomainNotAllowed,
validateProxySsrf,
validateUrlSsrf,
} from './utils';
import type { SsrfBridge } from '../../ssrf';
@@ -187,6 +188,7 @@ export async function httpRequest(
const url = buildTargetUrl(requestOptions.url, requestOptions.baseURL) ?? requestOptions.url;
await validateUrlSsrf(url, ssrfBridge);
await validateProxySsrf(requestOptions.proxy, ssrfBridge);
const axiosRequest = convertN8nRequestToAxios(requestOptions, ssrfBridge);
if (
@@ -139,8 +139,6 @@ export const getBeforeRedirectFn =
const customProxyUrl = proxyConfig ? getUrlFromProxyConfig(proxyConfig) : null;
const proxy = resolveProxyOption(customProxyUrl);
// SSRF lookup is applied to direct connections only; behind a proxy the
// proxy validates the final target.
const targetUrl = redirectedRequest.href;
const { httpAgent, httpsAgent } = buildNodeAgents(proxy, ssrf, redirectAgentOptions);
@@ -397,8 +395,6 @@ export function setAxiosAgents(
const customProxyUrl = proxyConfig ? getUrlFromProxyConfig(proxyConfig) : null;
const proxy = resolveProxyOption(customProxyUrl);
// SSRF lookup is applied to direct connections only; behind a proxy the
// proxy validates the final target.
const { httpAgent, httpsAgent } = buildNodeAgents(proxy, ssrf, agentOptions);
config.httpAgent = httpAgent;
config.httpsAgent = httpsAgent;
@@ -420,6 +416,16 @@ export async function validateUrlSsrf(
}
}
export async function validateProxySsrf(
proxyConfig: IHttpRequestOptions['proxy'] | string | undefined,
ssrfBridge?: SsrfBridge,
): Promise<void> {
const proxyUrl = getUrlFromProxyConfig(proxyConfig);
if (!isSupportedProxyUrl(proxyUrl)) return;
await validateUrlSsrf(proxyUrl, ssrfBridge);
}
/**
* Resolves the raw target of a legacy request object, i.e. the value the axios
* config carries as its `url`.
@@ -0,0 +1,85 @@
import type { HttpsProxyAgent } from 'https-proxy-agent';
import { UnexpectedError } from 'n8n-workflow';
import type http from 'node:http';
import type net from 'node:net';
import type { SsrfBridge } from '../ssrf';
/** Per-hop connect options of a proxy agent, as `agent-base` hands them to `connect()`. */
type ProxyConnectOpts = Parameters<HttpsProxyAgent<string>['connect']>[1];
export class UnknownConnectionHostError extends UnexpectedError {
constructor() {
super('Cannot determine the host for this connection');
}
}
/** Subset of an agent's connection options we read to find the target host. */
type ConnectionOptions = { host?: string | null; hostname?: string | null };
/**
* Runtime-only `createConnection` method of Node's http(s) agents.
*/
type CreateConnection = (
options: ConnectionOptions,
onConnect?: (error: Error | null, stream?: unknown) => void,
) => unknown;
interface ProxyConnectionAgent {
connectOpts: ConnectionOptions;
connect(req: http.ClientRequest, opts: ProxyConnectOpts): Promise<net.Socket>;
}
function connectionHostError(ssrf: SsrfBridge, options: ConnectionOptions): Error | undefined {
const host = options.host ?? options.hostname ?? undefined;
if (typeof host !== 'string' || host.length === 0) {
return new UnknownConnectionHostError();
}
const result = ssrf.validateConnectionHost(host);
return result.ok ? undefined : result.error;
}
/**
* Validates the proxy's own host before the socket to it opens.
*
* `agent-base` opens that socket inside `connect()`, from `connectOpts`.
* The agent's `createConnection` receives the target's options and never sees the proxy.
* This guard decides IP literals only.
* A hostname proxy relies on the secure lookup in the agent's options, which validates
* the address that hostname resolves to.
*/
export function installProxyConnectionGuard(agent: ProxyConnectionAgent, ssrf: SsrfBridge): void {
const connect = agent.connect.bind(agent);
agent.connect = async (req, opts) => {
const error = connectionHostError(ssrf, agent.connectOpts);
if (error) {
throw error;
}
return await connect(req, opts);
};
}
/**
* Wraps an agent's `createConnection` to validate the connection target before the socket opens.
*
* Node invokes the custom `lookup` to resolve hostnames only.
* An IP-literal target therefore reaches the socket unresolved, and needs this check
* at connection time.
*/
export function installConnectionGuard(
target: { createConnection: CreateConnection },
ssrf: SsrfBridge,
): void {
const createConnection = target.createConnection.bind(target);
target.createConnection = (options, onConnect) => {
const error = connectionHostError(ssrf, options);
if (error) {
if (onConnect) {
onConnect(error);
return undefined;
}
throw error;
}
return createConnection(options, onConnect);
};
}
@@ -13,8 +13,10 @@ type HttpProxyReqOpts = HttpAddRequestArgs[1];
/**
* `http.Agent` that delegates per-request env-proxy routing and caching to a shared {@link EnvProxyRouter}.
*
* The optional SSRF `lookup` is applied to the direct path only
* (behind a proxy it would resolve the proxy host, so the proxy validates the target).
* The optional SSRF `lookup` is applied to the direct path only.
* A proxy named by the environment belongs to the deployment rather than to a request.
* The policy that decides which targets a workflow may reach does not decide such a
* proxy (see `buildNodeAgents`).
*
* Also backs `installGlobalProxyAgent` (http-proxy.ts), keeping a single env-proxy agent implementation.
*/
@@ -15,7 +15,12 @@ import type { SsrfBridge } from '../ssrf';
import { invokeAxios } from './axios/invoke';
import { buildAxiosConfigFromLegacyRequest, buildLegacyAgentOptions } from './axios/legacy';
import { followSsrfRedirects, shouldFollowRedirectsManually } from './axios/redirect';
import { resolveLegacyRequestUrl, throwIfDomainNotAllowed, validateUrlSsrf } from './axios/utils';
import {
resolveLegacyRequestUrl,
throwIfDomainNotAllowed,
validateProxySsrf,
validateUrlSsrf,
} from './axios/utils';
import { binaryToString } from './binary-string';
import { parseIncomingMessage } from './parse-incoming-message';
@@ -56,6 +61,7 @@ export async function executeLegacyRequest(
const url = resolveLegacyRequestUrl(requestObject);
await validateUrlSsrf(url, ssrfBridge);
await validateProxySsrf(requestObject.proxy, ssrfBridge);
axiosConfig = Object.assign(
axiosConfig,
@@ -3,6 +3,7 @@ import http from 'node:http';
import https from 'node:https';
import type { LookupFunction } from 'node:net';
import { installConnectionGuard, installProxyConnectionGuard } from './connection-guards';
import { EnvProxyHttpAgent } from './env-proxy-http-agent';
import { EnvProxyHttpsAgent } from './env-proxy-https-agent';
import { createProxiedHttpAgent, createProxiedHttpsAgent } from '../proxy/proxied-agents';
@@ -54,9 +55,25 @@ export type NodeAgentOptions = https.AgentOptions;
* undici factory (`undici/factory.ts`), the axios transport layer
* (`axios/utils.ts`) and the global proxy agents (`http-proxy.ts`).
*
* SSRF lookup is injected only for **direct** connections. Behind a proxy the
* lookup resolves the proxy host, not the final target, so it is omitted there
* and the proxy validates the final target.
* The SSRF policy covers every host this builder opens a socket to, with one
* exception: a proxy named by the environment.
* `HTTP_PROXY` / `HTTPS_PROXY` describe the deployment rather than a request, and
* such a proxy commonly sits on a private address the policy blocks for targets.
* Setting them already requires control of the process.
* An explicit {@link ProxyUrl} can come from a request, so the policy decides its
* host like any other.
*
* The lookup resolves the target on a direct connection, and the proxy's own host
* behind an explicit proxy.
* Behind any proxy the final target never reaches these agents, so validating it
* belongs to the caller:
* - the axios entry points (`httpRequest`, `executeLegacyRequest`) check it before
* the request, and on each hop with the manual redirect follower
* - callers taking the agents on their own (`HttpTransport.getNodeAgent`) get
* neither check, and validate the targets they send through them
*
* A pre-request check does not pin an address to the socket, so a target the proxy
* resolves stays open to a rebind between the check and the connection.
*
* The `lookup` is owned by this builder: it is derived from the SSRF policy and
* always overrides anything in `agentOptions`. Passing `agentOptions.lookup`
@@ -101,24 +118,18 @@ export function buildNodeAgents(
);
}
// Explicit proxy URL. No direct path, so no SSRF lookup is injected.
return {
httpAgent: createProxiedHttpAgent(proxy, agentOptions),
httpsAgent: createProxiedHttpsAgent(proxy, agentOptions),
// Explicit proxy URL. No direct path, so the lookup only guards the proxy host.
const agents = {
httpAgent: createProxiedHttpAgent(proxy, { ...agentOptions, lookup }),
httpsAgent: createProxiedHttpsAgent(proxy, { ...agentOptions, lookup }),
};
if (ssrf !== 'disabled') {
installProxyConnectionGuard(agents.httpAgent, ssrf);
installProxyConnectionGuard(agents.httpsAgent, ssrf);
}
return agents;
}
/** Subset of an agent's connection options we read to find the target host. */
type ConnectionOptions = { host?: string | null; hostname?: string | null };
/**
* Runtime-only `createConnection` method of Node's http(s) agents.
*/
type CreateConnection = (
options: ConnectionOptions,
onConnect?: (error: Error | null, stream?: unknown) => void,
) => unknown;
/**
* Installs {@link installConnectionGuard} on a direct-path agent pair when SSRF protection is active.
*/
@@ -132,28 +143,3 @@ function applyConnectionGuard(
}
return agents;
}
/**
* Wraps an agent's `createConnection` to validate the connection target before the socket opens.
* Node invokes the custom `lookup` only to resolve hostnames, so it also requires a check at connection time.
*/
export function installConnectionGuard(
target: { createConnection: CreateConnection },
ssrf: SsrfBridge,
): void {
const createConnection = target.createConnection.bind(target);
target.createConnection = (options, onConnect) => {
const host = options.host ?? options.hostname ?? undefined;
if (typeof host === 'string') {
const result = ssrf.validateConnectionHost(host);
if (!result.ok) {
if (onConnect) {
onConnect(result.error);
return undefined;
}
throw result.error;
}
}
return createConnection(options, onConnect);
};
}
@@ -139,8 +139,13 @@ export interface HttpRequestClient {
* SSRF coverage is identical for `asCustomFetch()` and `getDispatcher()` (same
* underlying dispatcher): every dispatched request — initial and each redirect
* hop — is validated, and direct connections also carry a connect-time secure
* DNS lookup that defeats DNS-rebinding (TOCTOU). `getNodeAgent()` enforces the
* same connect-time secure lookup for direct connections.
* DNS lookup that defeats DNS-rebinding (TOCTOU). `getNodeAgent()` carries that
* same lookup, applied to whichever host its socket opens to: the target on a
* direct connection, the proxy behind an explicit one.
* It does **not** validate the targets sent through it.
* Behind any proxy the proxy resolves the final target, so no client-side
* connect-time check of that target is possible.
* Callers taking the agents validate those targets themselves (see `buildNodeAgents`).
*/
export interface HttpTransport {
asCustomFetch(): CustomFetch;
@@ -108,11 +108,53 @@ function buildDispatcherFromProxy(
return new Agent({ ...agentOptions, ...secureConnect(ssrf) });
}
if (proxy === 'env') {
// The environment's proxies are part of the deployment, not of a request,
// so the policy does not decide them (see `buildNodeAgents`).
return new EnvHttpProxyAgent({ ...agentOptions, ...secureConnect(ssrf) });
}
// Explicit proxy URL: no direct path, so no connect-time lookup is injected —
// the proxy resolves the target. Mirrors `buildNodeAgents`.
return new ProxyAgent({ uri: proxy, ...agentOptions });
assertProxyHostAllowed(ssrf, proxy);
return new ProxyAgent({
uri: proxy,
...agentOptions,
...secureProxyConnect(ssrf),
});
}
/**
* A connect-time secure DNS lookup for the socket opened to a proxy.
*
* undici builds the connector for the proxy socket from `proxyTls`.
* A caller's `connect` does not reach it: `ProxyAgent` overwrites that with its own
* tunnel handshake.
*/
function secureProxyConnect(ssrf: SsrfOption) {
return ssrf === 'disabled' ? {} : { proxyTls: { lookup: ssrf.createSecureLookup() } };
}
/**
* Catches an IP-literal proxy host, which no lookup sees.
* Applies to an explicit proxy URI, the one form that can come from a request.
*/
function assertProxyHostAllowed(ssrf: SsrfOption, proxyUri: string): void {
if (ssrf === 'disabled') {
return;
}
const hostname = proxyHostname(proxyUri);
if (hostname === undefined) {
return;
}
const result = ssrf.validateConnectionHost(hostname);
if (!result.ok) {
throw result.error;
}
}
function proxyHostname(uri: string): string | undefined {
try {
return new URL(uri).hostname;
} catch {
return undefined;
}
}
/**
@@ -174,8 +216,11 @@ function lazyValue<T>(factory: () => T): () => T {
* `fetch` re-dispatches through this dispatcher for each redirect hop, so this
* validates the initial request **and** every redirect target (both hostname
* and direct-IP targets), unlike a connect-time DNS lookup which never fires for
* IP-literal targets. Validation runs against the request target, never the
* proxy, so it is proxy-agnostic.
* IP-literal targets.
*
* This interceptor does not validate a proxy's own host.
* That host is fixed for the dispatcher rather than per request, so
* {@link buildDispatcherFromProxy} decides it where the dispatcher is built.
*/
export function createSsrfInterceptor(bridge: SsrfBridge): Dispatcher.DispatcherComposeInterceptor {
return (dispatch) => (opts, handler) => {
@@ -1,5 +1,6 @@
import type dns from 'node:dns';
import type { LookupFunction } from 'node:net';
import { vi } from 'vitest';
import { afterEach, beforeEach, vi, type Mock } from 'vitest';
import type { SsrfBridge } from '..';
@@ -7,6 +8,50 @@ export function makeLookupFn(): LookupFunction {
return vi.fn() as unknown as LookupFunction;
}
/** A `lookup` that fails every resolution with `error`. */
export function makeDenyingLookup(error: Error): LookupFunction & Mock {
return vi.fn((_hostname: string, options: dns.LookupOptions, onResult: unknown) => {
(onResult as (error: Error | null, address?: unknown, family?: number) => void)(
error,
options.all ? [] : '',
undefined,
);
}) as unknown as LookupFunction & Mock;
}
const PROXY_ENV_KEYS = [
'HTTP_PROXY',
'http_proxy',
'HTTPS_PROXY',
'https_proxy',
'NO_PROXY',
'no_proxy',
'ALL_PROXY',
'all_proxy',
] as const;
/** Clears the proxy environment around each test and restores it afterwards. */
export function useCleanProxyEnv(): void {
const savedEnv: Record<string, string | undefined> = {};
beforeEach(() => {
for (const key of PROXY_ENV_KEYS) {
savedEnv[key] = process.env[key];
delete process.env[key];
}
});
afterEach(() => {
for (const key of PROXY_ENV_KEYS) {
if (savedEnv[key] === undefined) {
delete process.env[key];
} else {
process.env[key] = savedEnv[key];
}
}
});
}
export function makeSsrfBridge(overrides?: Partial<SsrfBridge>): SsrfBridge {
return {
validateUrl: vi.fn().mockResolvedValue({ ok: true, result: undefined }),
@@ -277,7 +277,7 @@ export class IsolatedVmBridge implements RuntimeBridge {
}
/**
* Create an ivm.Reference callback for getting value/metadata at a path.
* Create an ivm.Callback for getting value/metadata at a path.
*
* Used by createDeepLazyProxy when accessing properties. Returns metadata
* markers for arrays and objects, or the primitive value directly.
@@ -291,8 +291,8 @@ export class IsolatedVmBridge implements RuntimeBridge {
* @param data - Current workflow data to use for callback responses
* @private
*/
private createGetValueAtPathRef(data: WorkflowData): ivm.Reference {
return new (getIvm().Reference)((path: string[]) => {
private createGetValueAtPathRef(data: WorkflowData): ivm.Callback {
return new (getIvm().Callback)((path: string[]) => {
try {
// Navigate to value
// Special-case: paths starting with ['$item', index] call data.$item(index)
@@ -361,15 +361,15 @@ export class IsolatedVmBridge implements RuntimeBridge {
}
/**
* Create an ivm.Reference callback for getting array elements at an index.
* Create an ivm.Callback for getting array elements at an index.
*
* Used by array proxy when accessing numeric indices.
*
* @param data - Current workflow data to use for callback responses
* @private
*/
private createGetArrayElementRef(data: WorkflowData): ivm.Reference {
return new (getIvm().Reference)((path: string[], index: number) => {
private createGetArrayElementRef(data: WorkflowData): ivm.Callback {
return new (getIvm().Callback)((path: string[], index: number) => {
try {
// Navigate to array
// Special-case: paths starting with ['$item', index] call data.$item(index)
@@ -462,11 +462,17 @@ export class IsolatedVmBridge implements RuntimeBridge {
* in this switch; the `type` field selects a static branch in source,
* not a property lookup on a runtime object.
*
* Return-value note: handlers must return plain, structured-clone-able
* data. Results cross into the isolate through an ivm.Callback, which copies
* them via the structured-clone algorithm — return JSON-shaped values, not
* isolated-vm objects (`Reference`/`ExternalCopy`) or other non-cloneable
* values.
*
* @param data - Current workflow data
* @private
*/
private createCallHostRef(data: WorkflowData): ivm.Reference {
return new (getIvm().Reference)((rawMsg: unknown) => {
private createCallHostRef(data: WorkflowData): ivm.Callback {
return new (getIvm().Callback)((rawMsg: unknown) => {
try {
const msg = bridgeMessageSchema.parse(rawMsg);
switch (msg.type) {
@@ -695,7 +701,7 @@ export class IsolatedVmBridge implements RuntimeBridge {
* Execute JavaScript code in the isolated context.
*
* Flow:
* 1. Create three ivm.Reference callbacks scoped to the current data:
* 1. Create three ivm.Callback instances scoped to the current data:
* `getValueAtPath`, `getArrayElement`, `callHost`.
* 2. Use evalClosureSync to run the code in a closure where `$0`/`$1`/`$2`
* are the callback references — no global mutable state.
@@ -715,6 +721,10 @@ export class IsolatedVmBridge implements RuntimeBridge {
throw new Error('Bridge not initialized. Call initialize() first.');
}
// Host callbacks are ivm.Callback instances: inside the isolate they
// arrive as plain functions with structured-clone marshaling, so the
// runtime invokes them directly. Callbacks are GC-managed; there is no
// release() to call in `finally`.
const getValueAtPath = this.createGetValueAtPathRef(data);
const getArrayElement = this.createGetArrayElementRef(data);
const callHost = this.createCallHostRef(data);
@@ -796,10 +806,6 @@ try {
);
}
throw new Error(`Expression evaluation failed: ${errorMessage}`);
} finally {
getValueAtPath.release();
getArrayElement.release();
callHost.release();
}
}
@@ -1,9 +1,7 @@
import { buildContext } from '../context';
function makeRef(impl: (args: unknown[]) => unknown) {
return {
applySync: (_thisArg: unknown, args: unknown[]) => impl(args),
};
return (...args: unknown[]) => impl(args);
}
describe('buildContext proxy', () => {
@@ -5,8 +5,6 @@ import { createDeepLazyProxy, isLazyProxy, getProxyPath } from '../lazy-proxy';
// Helpers
// ---------------------------------------------------------------------------
const ivmCallOpts = { arguments: { copy: true }, result: { copy: true } };
function mockApplySync(returnValue: unknown = undefined) {
return vi.fn().mockReturnValue(returnValue);
}
@@ -22,8 +20,8 @@ function createMockCallbacks(
const getArrayElement = overrides.getArrayElement ?? mockApplySync();
const callbacks = {
getValueAtPath: { applySync: getValueAtPath },
getArrayElement: { applySync: getArrayElement },
getValueAtPath,
getArrayElement,
};
return { getValueAtPath, getArrayElement, callbacks };
@@ -231,7 +229,7 @@ describe('createDeepLazyProxy', () => {
const p = proxy(['data']);
mocks.getArrayElement.mockReturnValue('val');
p.list[3];
expect(mocks.getArrayElement).toHaveBeenCalledWith(null, [['data', 'list'], 3], ivmCallOpts);
expect(mocks.getArrayElement).toHaveBeenCalledWith(['data', 'list'], 3);
});
it('returns undefined for non-numeric non-length properties', () => {
@@ -272,15 +270,15 @@ describe('createDeepLazyProxy', () => {
// Each level triggers __getValueAtPath and creates a nested proxy
// a -> returns object metadata
const a = p.a;
expect(mocks.getValueAtPath).toHaveBeenLastCalledWith(null, [['a']], ivmCallOpts);
expect(mocks.getValueAtPath).toHaveBeenLastCalledWith(['a']);
// a.b -> returns object metadata
const b = a.b;
expect(mocks.getValueAtPath).toHaveBeenLastCalledWith(null, [['a', 'b']], ivmCallOpts);
expect(mocks.getValueAtPath).toHaveBeenLastCalledWith(['a', 'b']);
// a.b.c -> returns object metadata
const c = b.c;
expect(mocks.getValueAtPath).toHaveBeenLastCalledWith(null, [['a', 'b', 'c']], ivmCallOpts);
expect(mocks.getValueAtPath).toHaveBeenLastCalledWith(['a', 'b', 'c']);
expect(getProxyPath(c)).toEqual(['a', 'b', 'c']);
});
@@ -303,7 +301,7 @@ describe('createDeepLazyProxy', () => {
mocks.getValueAtPath.mockReturnValue('val');
const p = proxy(['$json']);
p.user;
expect(mocks.getValueAtPath).toHaveBeenCalledWith(null, [['$json', 'user']], ivmCallOpts);
expect(mocks.getValueAtPath).toHaveBeenCalledWith(['$json', 'user']);
});
it('nested proxies inherit full path', () => {
@@ -315,11 +313,7 @@ describe('createDeepLazyProxy', () => {
// Accessing a property on the nested proxy should build the full path
mocks.getValueAtPath.mockReturnValue('Alice');
user.name;
expect(mocks.getValueAtPath).toHaveBeenLastCalledWith(
null,
[['$json', 'user', 'name']],
ivmCallOpts,
);
expect(mocks.getValueAtPath).toHaveBeenLastCalledWith(['$json', 'user', 'name']);
});
});
@@ -369,7 +363,7 @@ describe('createDeepLazyProxy', () => {
mocks.getValueAtPath.mockReturnValue('val');
const p = proxy(['$json']);
'foo' in p;
expect(mocks.getValueAtPath).toHaveBeenCalledWith(null, [['$json', 'foo']], ivmCallOpts);
expect(mocks.getValueAtPath).toHaveBeenCalledWith(['$json', 'foo']);
});
});
@@ -461,7 +455,7 @@ describe('createDeepLazyProxy', () => {
const p = arrayProxy(['arr'], 3);
mocks.getArrayElement.mockReturnValue('first');
expect(p[0]).toBe('first');
expect(mocks.getArrayElement).toHaveBeenCalledWith(null, [['arr'], 0], ivmCallOpts);
expect(mocks.getArrayElement).toHaveBeenCalledWith(['arr'], 0);
});
it('nested array element returns an array-shaped child proxy', () => {
@@ -52,30 +52,23 @@ type InputRpcType = (typeof INPUT_RPC_TYPES)[keyof typeof INPUT_RPC_TYPES];
// ============================================================================
/**
* The subset of `ivm.Reference` shape the in-isolate runtime relies on.
* Declared locally rather than importing from `isolated-vm` because this
* module is bundled into the isolate IIFE, where the native module is
* unavailable. The host wires real `ivm.Reference` instances which
* structurally satisfy this interface.
* A bridge callback as seen from inside the isolate: a plain function.
* The host wires these as `ivm.Callback` instances, which arrive in the
* isolate as ordinary functions whose arguments and return value are copied
* via the structured-clone algorithm.
*/
interface BridgeCallback {
applySync(
thisArg: unknown,
args: unknown[],
options?: { arguments?: { copy?: boolean }; result?: { copy?: boolean } },
): unknown;
}
type BridgeCallback = (...args: unknown[]) => unknown;
/**
* Bridge callbacks the in-isolate runtime can invoke synchronously via
* `ivm.Reference.applySync`.
* Bridge callbacks the in-isolate runtime invokes synchronously as plain
* functions (host-side `ivm.Callback` instances).
*
* - `getValueAtPath`, `getArrayElement`: data-access primitives used by the
* lazy-proxy system. Hot path; one ivm.Reference each for minimum overhead.
* lazy-proxy system. Hot path; one ivm.Callback each for minimum overhead.
* - `callHost`: typed-RPC dispatcher. The in-isolate runtime constructs
* an envelope (e.g. `{ type: 'getNodeFirst', nodeName, ... }`) and the
* host-side dispatcher validates it with zod before routing to a handler.
* A single ivm.Reference covers every typed operation; new operations
* A single ivm.Callback covers every typed operation; new operations
* are new schemas in `bridge/bridge-messages.ts` + new cases in the
* dispatcher switch. The name reflects what this is: a synchronous
* host RPC, not a postMessage-style async send.
@@ -198,11 +191,7 @@ export function buildContext(
const lazyProxy = createDeepLazyProxy(['$', nodeName], undefined, callbacks);
const sendNodeMethod = (type: NodeRpcType) => {
return (branchIndex?: number, runIndex?: number) => {
const result = callbacks.callHost.applySync(
null,
[{ type, nodeName, branchIndex, runIndex }],
{ arguments: { copy: true }, result: { copy: true } },
);
const result = callbacks.callHost({ type, nodeName, branchIndex, runIndex });
throwIfErrorSentinel(result);
return result;
};
@@ -217,18 +206,12 @@ export function buildContext(
type: 'getNodePairedItem' | 'getNodeItemMatching',
itemIndex?: number,
) => {
const result = callbacks.callHost.applySync(null, [{ type, nodeName, itemIndex }], {
arguments: { copy: true },
result: { copy: true },
});
const result = callbacks.callHost({ type, nodeName, itemIndex });
throwIfErrorSentinel(result);
return result;
};
const sendGetNodeItem = () => {
const result = callbacks.callHost.applySync(null, [{ type: 'getNodeItem', nodeName }], {
arguments: { copy: true },
result: { copy: true },
});
const result = callbacks.callHost({ type: 'getNodeItem', nodeName });
throwIfErrorSentinel(result);
return result;
};
@@ -271,10 +254,7 @@ export function buildContext(
const lazyInputProxy = createDeepLazyProxy(['$input'], undefined, callbacks);
const sendInputMethod = (type: InputRpcType) => {
return () => {
const result = callbacks.callHost.applySync(null, [{ type }], {
arguments: { copy: true },
result: { copy: true },
});
const result = callbacks.callHost({ type });
throwIfErrorSentinel(result);
return result;
};
@@ -295,11 +275,7 @@ export function buildContext(
// args, and the host's `WorkflowDataProxy.$items` applies its own
// defaults when fields are undefined.
target.$items = (nodeName?: string, outputIndex?: number, runIndex?: number) => {
const result = callbacks.callHost.applySync(
null,
[{ type: 'getItems', nodeName, outputIndex, runIndex }],
{ arguments: { copy: true }, result: { copy: true } },
);
const result = callbacks.callHost({ type: 'getItems', nodeName, outputIndex, runIndex });
throwIfErrorSentinel(result);
return result;
};
@@ -316,11 +292,13 @@ export function buildContext(
valueType?: string,
defaultValue?: unknown,
) => {
const result = callbacks.callHost.applySync(
null,
[{ type: 'fromAi', name, description, valueType, defaultValue }],
{ arguments: { copy: true }, result: { copy: true } },
);
const result = callbacks.callHost({
type: 'fromAi',
name,
description,
valueType,
defaultValue,
});
throwIfErrorSentinel(result);
return result;
};
@@ -333,11 +311,7 @@ export function buildContext(
// Under the VM engine this re-enters the bridge on a fresh evaluation;
// the legacy engine handles it inline.
target.$evaluateExpression = (expression: string, itemIndex?: number) => {
const result = callbacks.callHost.applySync(
null,
[{ type: 'evaluateExpression', expression, itemIndex }],
{ arguments: { copy: true }, result: { copy: true } },
);
const result = callbacks.callHost({ type: 'evaluateExpression', expression, itemIndex });
throwIfErrorSentinel(result);
return result;
};
@@ -352,18 +326,12 @@ export function buildContext(
incomingSourceData: unknown,
initialPairedItem: unknown,
) => {
const result = callbacks.callHost.applySync(
null,
[
{
type: 'getPairedItem',
destinationNodeName,
incomingSourceData,
initialPairedItem,
},
],
{ arguments: { copy: true }, result: { copy: true } },
);
const result = callbacks.callHost({
type: 'getPairedItem',
destinationNodeName,
incomingSourceData,
initialPairedItem,
});
throwIfErrorSentinel(result);
return result;
};
@@ -374,7 +342,7 @@ export function buildContext(
// so each key is fetched at most once per evaluation.
// -------------------------------------------------------------------------
// Track keys we've already probed so we never call applySync twice
// Track keys we've already probed so we never call getValueAtPath twice
// for the same key — even if the host returned undefined.
const probedKeys = new Set<string>();
@@ -383,10 +351,7 @@ export function buildContext(
let value: unknown;
try {
value = callbacks.getValueAtPath.applySync(null, [[key]], {
arguments: { copy: true },
result: { copy: true },
});
value = callbacks.getValueAtPath([key]);
} catch {
// Don't mark as probed — the throw may be transient
// (e.g. host data not yet available) and a retry should be allowed.
@@ -106,7 +106,7 @@ export type ProxyMeta = { kind: 'object'; keys?: string[] } | { kind: 'array'; l
*
* @param basePath - Current path in object tree (e.g., ['$json', 'user'])
* @param meta - Optional shape descriptor (object/array + known keys/length)
* @param callbacks - ivm.Reference callbacks for cross-isolate communication
* @param callbacks - ivm.Callback functions for cross-isolate communication
* @returns Proxy object with lazy loading behavior
*/
export function createDeepLazyProxy(
@@ -133,10 +133,7 @@ export function createDeepLazyProxy(
function resolveObjectKeys(): string[] {
if (objectKeys) return objectKeys;
if (fetchedKeys) return fetchedKeys;
const value = getValueAtPath.applySync(null, [basePath], {
arguments: { copy: true },
result: { copy: true },
});
const value = getValueAtPath(basePath);
throwIfErrorSentinel(value);
if (isObjectMetadata(value)) {
fetchedKeys = value.__keys;
@@ -252,10 +249,7 @@ export function createDeepLazyProxy(
if (isArray) {
const idx = isInArrayBounds(prop);
if (idx === undefined) return undefined;
const element = getArrayElement.applySync(null, [basePath, idx], {
arguments: { copy: true },
result: { copy: true },
});
const element = getArrayElement(basePath, idx);
// Primitives (and null) skip `materializeChild`'s metadata checks.
if (element === null || typeof element !== 'object') {
targetObj[prop] = element;
@@ -269,12 +263,8 @@ export function createDeepLazyProxy(
// materializeChild rebuilds it only inside metadata branches.
const path = [...basePath, prop];
// Call back to host to get metadata/value
// Note: getValueAtPath is an ivm.Reference passed via callbacks
const value = getValueAtPath.applySync(null, [path], {
arguments: { copy: true },
result: { copy: true },
});
// Call back to host to get metadata/value.
const value = getValueAtPath(path);
targetObj[prop] = materializeChild(basePath, prop, value);
return targetObj[prop];
@@ -301,10 +291,7 @@ export function createDeepLazyProxy(
// Build path and check existence via callback
const path = [...basePath, prop];
const value = getValueAtPath.applySync(null, [path], {
arguments: { copy: true },
result: { copy: true },
});
const value = getValueAtPath(path);
// Handle errors serialized by host-side callbacks — reconstruct and throw
// so the isolate's outer try-catch can serialize them back via __reportError
@@ -137,10 +137,10 @@ export interface NodeProxy {
* type.
*
* Return types are `unknown` rather than `INodeExecutionData` / `[]`:
* results cross the isolate boundary via `applySync({ result: { copy: true } })`,
* which structured-clones the value and erases nominal types. The handlers
* pass the clone through verbatim, so a precise return type would be
* misleading. Matches the `NodeProxy` return type for the same reason.
* `ivm.Callback` structured-clones results as they pass between isolates,
* which erases nominal types. The handlers pass the clone through verbatim,
* so a precise return type would be misleading. Matches the `NodeProxy`
* return type for the same reason.
*/
export interface InputProxy {
first?: () => unknown;
+7 -1
View File
@@ -3,7 +3,10 @@ import type {
ExpressionKind,
PatternKind,
PropertyKind,
SpreadElementKind,
SpreadPropertyKind,
StatementKind,
SwitchCaseKind,
VariableDeclaratorKind,
} from 'ast-types/lib/gen/kinds';
@@ -23,4 +26,7 @@ export type ParentKind =
| PropertyKind
| PatternKind
| VariableDeclaratorKind
| CatchClauseKind;
| CatchClauseKind
| SpreadElementKind
| SpreadPropertyKind
| SwitchCaseKind;
@@ -80,6 +80,10 @@ const customPatches: Partial<Record<ParentKind['type'], CustomPatcher>> = {
}
},
Property(path, parent: namedTypes.Property, dataNode) {
if (parent.computed && parent.key === path.node) {
polyfillVar(path, dataNode);
return;
}
if (path.node !== parent.value) {
return;
}
@@ -117,6 +121,36 @@ const customPatches: Partial<Record<ParentKind['type'], CustomPatcher>> = {
polyfillVar(path, dataNode);
}
},
SpreadElement(path, parent: namedTypes.SpreadElement, dataNode) {
if (parent.argument === path.node) {
polyfillVar(path, dataNode);
}
},
SpreadProperty(path, parent: namedTypes.SpreadProperty, dataNode) {
if (parent.argument === path.node) {
polyfillVar(path, dataNode);
}
},
MethodDefinition(path, parent: namedTypes.MethodDefinition, dataNode) {
if (parent.computed && parent.key === path.node) {
polyfillVar(path, dataNode);
}
},
SwitchCase(path, parent: namedTypes.SwitchCase, dataNode) {
if (parent.test === path.node) {
polyfillVar(path, dataNode);
}
},
ClassDeclaration(path, parent: namedTypes.ClassDeclaration, dataNode) {
if (parent.superClass === path.node) {
polyfillVar(path, dataNode);
}
},
ClassExpression(path, parent: namedTypes.ClassExpression, dataNode) {
if (parent.superClass === path.node) {
polyfillVar(path, dataNode);
}
},
};
export const jsVariablePolyfill = (
@@ -143,6 +177,12 @@ export const jsVariablePolyfill = (
case 'OptionalMemberExpression':
case 'VariableDeclarator':
case 'ArrowFunctionExpression':
case 'SpreadElement':
case 'SpreadProperty':
case 'MethodDefinition':
case 'SwitchCase':
case 'ClassDeclaration':
case 'ClassExpression':
if (!customPatches[parent.type]) {
throw new Error(`Couldn't find custom patcher for parent type: ${parent.type}`);
}
@@ -211,12 +251,9 @@ export const jsVariablePolyfill = (
case 'RestElement':
case 'ArrayPattern':
case 'ObjectPattern':
case 'ClassExpression':
case 'RecordExpression':
case 'V8IntrinsicIdentifier':
case 'TopicReference':
case 'MethodDefinition':
case 'ClassDeclaration':
case 'ClassProperty':
case 'StaticBlock':
case 'ClassBody':
@@ -314,6 +351,7 @@ export const jsVariablePolyfill = (
// This is a simple type guard that guarantees we haven't missed
// a case. It'll result in a type error at compile time.
assertNever(parent);
polyfillVar(path, dataNode);
break;
}
},
@@ -0,0 +1,107 @@
import { Tournament } from '../src/index';
const evaluator = new Tournament((e) => {
throw e;
});
/**
* Every identifier that is a free read has to be resolved through the data
* context. One that is skipped resolves against the host scope instead, which
* is how `process` and friends become reachable from an expression.
*/
describe('jsVariablePolyfill', () => {
describe('free reads resolve through the data context', () => {
it.each([
['object spread', '{{ ({...value}).a }}'],
['nested object spread', '{{ ({...({...value})}).a }}'],
['computed object key', '{{ ({[key]: 1}).a }}'],
['computed class field', '{{ (() => { class X { [key] = 1; } return new X().a; })() }}'],
[
'computed class method',
'{{ (() => { class X { [key]() { return 1; } } return new X().a(); })() }}',
],
['switch case', '{{ (() => { switch (1) { case one: return 1; } })() }}'],
])('%s', (_, expression) => {
expect(evaluator.execute(expression, { value: { a: 1 }, key: 'a', one: 1 })).toBe(1);
});
it('array spread', () => {
expect(evaluator.execute('{{ [...value].length }}', { value: [1, 2, 3] })).toBe(3);
});
it('call argument spread', () => {
expect(evaluator.execute('{{ Math.max(...value) }}', { value: [1, 5, 3] })).toBe(5);
});
it('base class', () => {
class Base {
greet() {
return 'hello';
}
}
expect(
evaluator.execute('{{ (() => { class X extends Base {} return new X().greet(); })() }}', {
Base,
}),
).toBe('hello');
});
});
describe('unresolved free reads do not fall through to the host scope', () => {
it.each([
['object spread', '{{ ({...process}) }}'],
['nested object spread', '{{ ({...({...process})}) }}'],
['spread inside a function', '{{ (() => ({...process}))() }}'],
])('%s', (_, expression) => {
expect(evaluator.execute(expression, {})).toEqual({});
});
it.each([
['array spread', '{{ [...process] }}'],
['call argument spread', '{{ ((a) => a)(...process) }}'],
])('%s', (_, expression) => {
expect(() => evaluator.execute(expression, {})).toThrow(/is not iterable/);
});
it('base class', () => {
expect(() => evaluator.execute('{{ (() => { class X extends Buffer {} })() }}', {})).toThrow(
/is not a constructor or null/,
);
});
it('switch case', () => {
expect(
evaluator.execute(
'{{ (() => { switch (1) { case process: return "host"; } return "safe"; })() }}',
{},
),
).toBe('safe');
});
it.each([
['computed object key', '{{ Object.keys({[process]: 1})[0] }}'],
[
'computed class method key',
'{{ (() => { class X { [process]() {} } return Object.getOwnPropertyNames(X.prototype)[1]; })() }}',
],
])('%s', (_, expression) => {
expect(evaluator.execute(expression, { Object })).toBe('undefined');
});
});
describe('bindings are left alone', () => {
it.each([
['rest property', '{{ (() => { const {...rest} = value; return rest.a; })() }}'],
['rest element', '{{ (() => { const [...rest] = [1]; return rest[0]; })() }}'],
['rest parameter', '{{ ((...rest) => rest[0])(1) }}'],
[
'local shadowing a host global',
'{{ (() => { const process = value; return {...process}.a; })() }}',
],
['parameter shadowing a host global', '{{ ((process) => ({...process}).a)(value) }}'],
])('%s', (_, expression) => {
expect(evaluator.execute(expression, { value: { a: 1 } })).toBe(1);
});
});
});
@@ -35,7 +35,7 @@ afterAll(async () => {
describe('InsightsController', () => {
const insightsByPeriodRepository = mockInstance(InsightsByPeriodRepository);
const workflowSharingService = mockInstance(WorkflowSharingService);
mockInstance(WorkflowSharingService);
let controller: InsightsController;
const sevenDaysAgo = DateTime.now().minus({ days: 7 }).toJSDate();
const today = DateTime.now().toJSDate();
@@ -365,13 +365,6 @@ describe('InsightsController', () => {
},
];
beforeEach(() => {
// getSharedWorkflowIds returns all workflow IDs for the owner-like user
workflowSharingService.getSharedWorkflowIds.mockResolvedValue(
mockRows.map((row) => row.workflowId),
);
});
it('should return empty insights by workflow if no data', async () => {
// ARRANGE
insightsByPeriodRepository.getInsightsByWorkflow.mockResolvedValue({ count: 0, rows: [] });
@@ -2,12 +2,14 @@ import type { LicenseState } from '@n8n/backend-common';
import {
createTeamProject,
createWorkflow,
linkUserToProject,
mockLogger,
testDb,
testModules,
} from '@n8n/backend-test-utils';
import type { InstanceType } from '@n8n/constants';
import type { IWorkflowDb, Project, User, WorkflowEntity } from '@n8n/db';
import { WorkflowRepository } from '@n8n/db';
import { Container } from '@n8n/di';
import { DateTime } from 'luxon';
import type { InstanceSettings } from 'n8n-core';
@@ -16,9 +18,12 @@ import type { MockInstance, Mocked } from 'vitest';
import type { MockProxy } from 'vitest-mock-extended';
import { mock } from 'vitest-mock-extended';
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
import type { WorkflowSharingService } from '@/workflows/workflow-sharing.service';
import { createMember } from '@test-integration/db/users';
import { createCompactedInsightsEvent } from '../database/entities/__tests__/db-utils';
import type { InsightsByPeriod } from '../database/entities/insights-by-period';
import type { InsightsByPeriodRepository } from '../database/repositories/insights-by-period.repository';
import { InsightsCollectionService } from '../insights-collection.service';
import type { InsightsCompactionService } from '../insights-compaction.service';
@@ -36,8 +41,11 @@ describe('InsightsService (Integration)', () => {
'InsightsRaw',
'InsightsByPeriod',
'InsightsMetadata',
'SharedWorkflow',
'WorkflowEntity',
'ProjectRelation',
'Project',
'User',
]);
});
@@ -144,6 +152,10 @@ describe('InsightsService (Integration)', () => {
let project: Project;
let workflow: IWorkflowDb & WorkflowEntity;
const globalWorkflowReadUser = {
role: { scopes: [{ slug: 'workflow:read' }] },
} as unknown as User;
beforeEach(async () => {
project = await createTeamProject();
workflow = await createWorkflow({}, project);
@@ -198,6 +210,7 @@ describe('InsightsService (Integration)', () => {
// ACT
const summary = await insightsService.getInsightsSummary({
user: globalWorkflowReadUser,
startDate: startDate.toJSDate(),
endDate: endDate.toJSDate(),
});
@@ -227,6 +240,7 @@ describe('InsightsService (Integration)', () => {
// ACT
const summary = await insightsService.getInsightsSummary({
user: globalWorkflowReadUser,
startDate,
endDate: now.toJSDate(),
});
@@ -294,6 +308,7 @@ describe('InsightsService (Integration)', () => {
// ACT
const summary = await insightsService.getInsightsSummary({
user: globalWorkflowReadUser,
startDate: startDate.toJSDate(),
endDate: endDate.toJSDate(),
projectId: project.id,
@@ -308,13 +323,157 @@ describe('InsightsService (Integration)', () => {
total: { value: 10, unit: 'count', deviation: -5 },
});
});
describe('project scoping', () => {
let member: User;
let otherProject: Project;
let startDate: Date;
let endDate: Date;
let workflowInsights: InsightsByPeriod;
let otherWorkflowInsights: InsightsByPeriod;
beforeEach(async () => {
member = await createMember();
otherProject = await createTeamProject();
const otherWorkflow = await createWorkflow({}, otherProject);
const now = DateTime.utc();
startDate = now.minus({ days: 6 }).toJSDate();
endDate = now.toJSDate();
// 4 successes in `project`, 10 in `otherProject`
[workflowInsights, otherWorkflowInsights] = await Promise.all([
createCompactedInsightsEvent(workflow, {
type: 'success',
value: 4,
periodUnit: 'day',
periodStart: now.minus({ days: 1 }),
}),
createCompactedInsightsEvent(otherWorkflow, {
type: 'success',
value: 10,
periodUnit: 'day',
periodStart: now.minus({ days: 1 }),
}),
]);
});
test('should aggregate only accessible projects when no project is requested', async () => {
await linkUserToProject(member, project, 'project:viewer');
const summary = await insightsService.getInsightsSummary({
user: member,
startDate,
endDate,
});
expect(summary.total.value).toBe(workflowInsights.value);
});
test('should return no results for a user with no accessible projects', async () => {
const summary = await insightsService.getInsightsSummary({
user: member,
startDate,
endDate,
});
expect(summary.total.value).toBe(0);
expect(summary.failed.value).toBe(0);
});
test('should aggregate all projects for users with the global workflow read scope', async () => {
const summary = await insightsService.getInsightsSummary({
user: globalWorkflowReadUser,
startDate,
endDate,
});
expect(summary.total.value).toBe(workflowInsights.value + otherWorkflowInsights.value);
});
test('should aggregate the requested project when it is accessible', async () => {
await linkUserToProject(member, project, 'project:viewer');
const summary = await insightsService.getInsightsSummary({
user: member,
startDate,
endDate,
projectId: project.id,
});
expect(summary.total.value).toBe(workflowInsights.value);
});
test('should throw a forbidden error when the requested project is not accessible', async () => {
await linkUserToProject(member, project, 'project:viewer');
await expect(
insightsService.getInsightsSummary({
user: member,
startDate,
endDate,
projectId: otherProject.id,
}),
).rejects.toThrow(ForbiddenError);
});
test('should throw a forbidden error when the requested project does not exist', async () => {
await expect(
insightsService.getInsightsSummary({
user: member,
startDate,
endDate,
projectId: 'non-existing-project-id',
}),
).rejects.toThrow(ForbiddenError);
});
test('should retain history from deleted workflows for users with the global workflow read scope', async () => {
// Deleting a workflow nulls the insights metadata FK but keeps the row
await Container.get(WorkflowRepository).delete({ id: workflow.id });
const summary = await insightsService.getInsightsSummary({
user: globalWorkflowReadUser,
startDate,
endDate,
});
expect(summary.total.value).toBe(workflowInsights.value + otherWorkflowInsights.value);
});
test('should exclude deleted workflow history when scoped to the requested project', async () => {
await linkUserToProject(member, project, 'project:viewer');
const deletedWorkflow = await createWorkflow({}, project);
await createCompactedInsightsEvent(deletedWorkflow, {
type: 'success',
value: 5,
periodUnit: 'day',
periodStart: DateTime.utc().minus({ days: 1 }),
});
await Container.get(WorkflowRepository).delete({ id: deletedWorkflow.id });
const summary = await insightsService.getInsightsSummary({
user: member,
startDate,
endDate,
projectId: project.id,
});
// Only the live workflow's 4 successes; the deleted workflows are excluded
expect(summary.total.value).toBe(workflowInsights.value);
});
});
});
describe('getInsightsByWorkflow', () => {
let insightsService: InsightsService;
// Owner-like user with the global `workflow:read` scope, so every workflow is accessible
const owner = { role: { scopes: [{ slug: 'workflow:read' }] } } as unknown as User;
const globalWorkflowReadUser = {
role: { scopes: [{ slug: 'workflow:read' }] },
} as unknown as User;
beforeAll(() => {
insightsService = Container.get(InsightsService);
@@ -415,7 +574,7 @@ describe('InsightsService (Integration)', () => {
// ACT
const byWorkflow = await insightsService.getInsightsByWorkflow({
user: owner,
user: globalWorkflowReadUser,
startDate,
endDate,
});
@@ -482,7 +641,7 @@ describe('InsightsService (Integration)', () => {
// ACT
const byWorkflow = await insightsService.getInsightsByWorkflow({
user: owner,
user: globalWorkflowReadUser,
startDate,
endDate: now.toJSDate(),
sortBy: 'runTime:desc',
@@ -510,7 +669,7 @@ describe('InsightsService (Integration)', () => {
// ACT
const byWorkflow = await insightsService.getInsightsByWorkflow({
user: owner,
user: globalWorkflowReadUser,
startDate,
endDate: now.toJSDate(),
sortBy: 'succeeded:desc',
@@ -538,7 +697,7 @@ describe('InsightsService (Integration)', () => {
const startDate = now.minus({ days: 14 }).startOf('day').toJSDate();
const byWorkflow = await insightsService.getInsightsByWorkflow({
user: owner,
user: globalWorkflowReadUser,
startDate,
endDate: now.toJSDate(),
skip: 10,
@@ -608,7 +767,7 @@ describe('InsightsService (Integration)', () => {
// ACT
const byWorkflow = await insightsService.getInsightsByWorkflow({
user: owner,
user: globalWorkflowReadUser,
startDate,
endDate: now.toJSDate(),
projectId: project.id,
@@ -655,7 +814,7 @@ describe('InsightsService (Integration)', () => {
// ACT
const byWorkflow = await insightsService.getInsightsByWorkflow({
user: owner,
user: globalWorkflowReadUser,
startDate,
endDate: now.toJSDate(),
});
@@ -664,6 +823,148 @@ describe('InsightsService (Integration)', () => {
expect(byWorkflow.count).toEqual(0);
expect(byWorkflow.data).toHaveLength(0);
});
describe('project scoping', () => {
let member: User;
let startDate: Date;
let endDate: Date;
beforeEach(async () => {
member = await createMember();
const now = DateTime.utc();
startDate = now.minus({ days: 6 }).toJSDate();
endDate = now.toJSDate();
// workflow1 (in `project`) is accessible; workflow4 (in `project2`) is not
await createCompactedInsightsEvent(workflow1, {
type: 'success',
value: 4,
periodUnit: 'day',
periodStart: now.minus({ days: 1 }),
});
await createCompactedInsightsEvent(workflow4, {
type: 'success',
value: 10,
periodUnit: 'day',
periodStart: now.minus({ days: 1 }),
});
});
test('should list only workflows in projects the caller can read when no project is requested', async () => {
await linkUserToProject(member, project, 'project:viewer');
const byWorkflow = await insightsService.getInsightsByWorkflow({
user: member,
startDate,
endDate,
});
expect(byWorkflow.count).toBe(1);
expect(byWorkflow.data.map((row) => row.workflowId)).toEqual([workflow1.id]);
});
test('should list no workflows for a user with no accessible projects', async () => {
const byWorkflow = await insightsService.getInsightsByWorkflow({
user: member,
startDate,
endDate,
});
expect(byWorkflow.count).toBe(0);
expect(byWorkflow.data).toEqual([]);
});
test('should list workflows for the requested project when it is accessible', async () => {
await linkUserToProject(member, project, 'project:viewer');
const byWorkflow = await insightsService.getInsightsByWorkflow({
user: member,
startDate,
endDate,
projectId: project.id,
});
expect(byWorkflow.count).toBe(1);
expect(byWorkflow.data.map((row) => row.workflowId)).toEqual([workflow1.id]);
});
test('should throw a forbidden error when the requested project is not accessible', async () => {
await linkUserToProject(member, project, 'project:viewer');
await expect(
insightsService.getInsightsByWorkflow({
user: member,
startDate,
endDate,
projectId: project2.id,
}),
).rejects.toThrow(ForbiddenError);
});
test('should throw a forbidden error when the requested project does not exist', async () => {
await expect(
insightsService.getInsightsByWorkflow({
user: member,
startDate,
endDate,
projectId: 'non-existing-project-id',
}),
).rejects.toThrow(ForbiddenError);
});
test('should report a count consistent with the returned rows when results are scoped', async () => {
await linkUserToProject(member, project, 'project:viewer');
// A second accessible workflow, so pagination has something to page through
await createCompactedInsightsEvent(workflow2, {
type: 'success',
value: 1,
periodUnit: 'day',
periodStart: DateTime.utc().minus({ days: 1 }),
});
const byWorkflow = await insightsService.getInsightsByWorkflow({
user: member,
startDate,
endDate,
take: 1,
});
// The inaccessible workflow4 must not inflate the count
expect(byWorkflow.count).toBe(2);
expect(byWorkflow.data).toHaveLength(1);
});
test('should retain a row for a deleted workflow when the caller has the global workflow read scope', async () => {
await Container.get(WorkflowRepository).delete({ id: workflow4.id });
const byWorkflow = await insightsService.getInsightsByWorkflow({
user: globalWorkflowReadUser,
startDate,
endDate,
});
expect(byWorkflow.data.find((row) => row.workflowId === null)).toMatchObject({
total: 10,
hasReadAccess: false,
});
});
test('should exclude a row for a deleted workflow when the caller is scoped to its project', async () => {
await linkUserToProject(member, project2, 'project:viewer');
await Container.get(WorkflowRepository).delete({ id: workflow4.id });
const byWorkflow = await insightsService.getInsightsByWorkflow({
user: member,
startDate,
endDate,
projectId: project2.id,
});
expect(byWorkflow.data).toHaveLength(0);
});
});
});
describe('getInsightsByTime', () => {
@@ -672,6 +973,10 @@ describe('InsightsService (Integration)', () => {
insightsService = Container.get(InsightsService);
});
const globalWorkflowReadUser = {
role: { scopes: [{ slug: 'workflow:read' }] },
} as unknown as User;
let project: Project;
let otherProject: Project;
let workflow1: IWorkflowDb & WorkflowEntity;
@@ -691,6 +996,7 @@ describe('InsightsService (Integration)', () => {
const now = DateTime.utc();
const startDate = now.minus({ days: 14 }).toJSDate();
const byTime = await insightsService.getInsightsByTime({
user: globalWorkflowReadUser,
startDate,
endDate: now.toJSDate(),
});
@@ -709,6 +1015,7 @@ describe('InsightsService (Integration)', () => {
const startDate = now.minus({ days: 14 }).startOf('day').toJSDate();
const byTime = await insightsService.getInsightsByTime({
user: globalWorkflowReadUser,
startDate,
endDate: now.toJSDate(),
});
@@ -773,6 +1080,7 @@ describe('InsightsService (Integration)', () => {
// ACT
const byTime = await insightsService.getInsightsByTime({
user: globalWorkflowReadUser,
startDate,
endDate: now.toJSDate(),
});
@@ -851,6 +1159,7 @@ describe('InsightsService (Integration)', () => {
// ACT
const byTime = await insightsService.getInsightsByTime({
user: globalWorkflowReadUser,
startDate,
endDate: now.toJSDate(),
insightTypes: ['time_saved_min', 'failure'],
@@ -931,6 +1240,7 @@ describe('InsightsService (Integration)', () => {
// ACT
const byTime = await insightsService.getInsightsByTime({
user: globalWorkflowReadUser,
startDate,
endDate: now.toJSDate(),
projectId: project.id,
@@ -985,6 +1295,141 @@ describe('InsightsService (Integration)', () => {
]),
);
});
describe('project scoping', () => {
let member: User;
let startDate: Date;
let endDate: Date;
let workflowInsights: InsightsByPeriod;
let otherWorkflowInsights: InsightsByPeriod;
beforeEach(async () => {
member = await createMember();
const now = DateTime.utc();
startDate = now.minus({ days: 6 }).toJSDate();
endDate = now.toJSDate();
// 4 successes in `project`, 10 in `otherProject`
[workflowInsights, otherWorkflowInsights] = await Promise.all([
createCompactedInsightsEvent(workflow1, {
type: 'success',
value: 4,
periodUnit: 'day',
periodStart: now.minus({ days: 1 }),
}),
createCompactedInsightsEvent(workflow3, {
type: 'success',
value: 10,
periodUnit: 'day',
periodStart: now.minus({ days: 1 }),
}),
]);
});
test('should aggregate only accessible projects when no project is requested', async () => {
await linkUserToProject(member, project, 'project:viewer');
const byTime = await insightsService.getInsightsByTime({
user: member,
startDate,
endDate,
});
expect(byTime).toHaveLength(1);
expect(byTime[0].values.succeeded).toBe(workflowInsights.value);
});
test('should return no results for a user with no accessible projects', async () => {
const byTime = await insightsService.getInsightsByTime({
user: member,
startDate,
endDate,
});
expect(byTime).toHaveLength(0);
});
test('should aggregate the requested project when it is accessible', async () => {
await linkUserToProject(member, project, 'project:viewer');
const byTime = await insightsService.getInsightsByTime({
user: member,
startDate,
endDate,
projectId: project.id,
});
expect(byTime).toHaveLength(1);
expect(byTime[0].values.succeeded).toBe(workflowInsights.value);
});
test('should throw a forbidden error when the requested project is not accessible', async () => {
await linkUserToProject(member, project, 'project:viewer');
await expect(
insightsService.getInsightsByTime({
user: member,
startDate,
endDate,
projectId: otherProject.id,
}),
).rejects.toThrow(ForbiddenError);
});
test('should throw a forbidden error when the requested project does not exist', async () => {
await expect(
insightsService.getInsightsByTime({
user: member,
startDate,
endDate,
projectId: 'non-existing-project-id',
}),
).rejects.toThrow(ForbiddenError);
});
test('should aggregate all projects for users with the global workflow read scope', async () => {
const byTime = await insightsService.getInsightsByTime({
user: globalWorkflowReadUser,
startDate,
endDate,
});
expect(byTime).toHaveLength(1);
expect(byTime[0].values.succeeded).toBe(
workflowInsights.value + otherWorkflowInsights.value,
);
});
test('should retain deleted workflow history for users with the global workflow read scope', async () => {
await Container.get(WorkflowRepository).delete({ id: workflow3.id });
const byTime = await insightsService.getInsightsByTime({
user: globalWorkflowReadUser,
startDate,
endDate,
});
expect(byTime[0].values.succeeded).toBe(
workflowInsights.value + otherWorkflowInsights.value,
);
});
test('should exclude deleted workflow history when scoped to the requested project', async () => {
await linkUserToProject(member, otherProject, 'project:viewer');
await Container.get(WorkflowRepository).delete({ id: workflow3.id });
const byTime = await insightsService.getInsightsByTime({
user: member,
startDate,
endDate,
projectId: otherProject.id,
});
expect(byTime).toHaveLength(0);
});
});
});
describe('validateDateFiltersLicense', () => {
@@ -5,6 +5,7 @@ import type { InstanceSettings } from 'n8n-core';
import type { MockProxy } from 'vitest-mock-extended';
import { mock } from 'vitest-mock-extended';
import { userHasScopes } from '@/permissions.ee/check-access';
import type { WorkflowSharingService } from '@/workflows/workflow-sharing.service';
import { TypeToNumber } from '../database/entities/insights-shared';
@@ -13,6 +14,12 @@ import type { InsightsCompactionService } from '../insights-compaction.service';
import type { InsightsPruningService } from '../insights-pruning.service';
import { InsightsService } from '../insights.service';
vi.mock('@/permissions.ee/check-access', () => ({
userHasScopes: vi.fn(),
}));
const user = mock<User>({ id: 'user-1' });
describe('InsightsService', () => {
let insightsService: InsightsService;
@@ -32,6 +39,7 @@ describe('InsightsService', () => {
mockLicenseState = mock<LicenseState>();
mockInstanceSettings = mock<InstanceSettings>();
mockWorkflowSharingService = mock<WorkflowSharingService>();
vi.mocked(userHasScopes).mockResolvedValue(true);
insightsService = new InsightsService(
mockInsightsByPeriodRepository,
@@ -61,6 +69,7 @@ describe('InsightsService', () => {
]);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -194,6 +203,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -216,6 +226,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -238,6 +249,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -259,6 +271,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -283,6 +296,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -303,6 +317,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -324,6 +339,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -348,6 +364,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -374,6 +391,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -396,6 +414,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -420,6 +439,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -442,6 +462,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -464,6 +485,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -483,6 +505,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -505,6 +528,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -527,6 +551,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -550,6 +575,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -572,6 +598,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -594,6 +621,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -616,6 +644,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -639,6 +668,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -661,6 +691,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -683,6 +714,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -705,6 +737,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -728,6 +761,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -750,6 +784,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -771,6 +806,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -793,6 +829,7 @@ describe('InsightsService', () => {
);
const result = await insightsService.getInsightsSummary({
user,
startDate,
endDate,
});
@@ -802,6 +839,79 @@ describe('InsightsService', () => {
});
});
});
describe('project access', () => {
beforeEach(() => {
mockInsightsByPeriodRepository.getPreviousAndCurrentPeriodTypeAggregates.mockResolvedValue(
[],
);
});
it('should not check project access when no project is requested', async () => {
await insightsService.getInsightsSummary({ user, startDate, endDate });
expect(userHasScopes).not.toHaveBeenCalled();
});
it('should throw a forbidden error when the requested project is not accessible', async () => {
vi.mocked(userHasScopes).mockResolvedValue(false);
await expect(
insightsService.getInsightsSummary({ user, startDate, endDate, projectId: 'project-1' }),
).rejects.toThrow('You do not have access to insights for this project.');
expect(
mockInsightsByPeriodRepository.getPreviousAndCurrentPeriodTypeAggregates,
).not.toHaveBeenCalled();
});
it('should query the requested project when it is accessible', async () => {
await insightsService.getInsightsSummary({
user,
startDate,
endDate,
projectId: 'project-1',
});
expect(userHasScopes).toHaveBeenCalledWith(user, ['workflow:read'], false, {
projectId: 'project-1',
});
expect(
mockInsightsByPeriodRepository.getPreviousAndCurrentPeriodTypeAggregates,
).toHaveBeenCalledWith(expect.objectContaining({ projectId: 'project-1' }));
});
it('should query without an access filter for users with the global workflow read scope', async () => {
mockWorkflowSharingService.rolesGrantingScope.mockResolvedValue(undefined);
await insightsService.getInsightsSummary({ user, startDate, endDate });
expect(
mockInsightsByPeriodRepository.getPreviousAndCurrentPeriodTypeAggregates,
).toHaveBeenCalledWith(expect.objectContaining({ accessFilter: undefined }));
});
it('should query with an access filter for users without the global workflow read scope', async () => {
mockWorkflowSharingService.rolesGrantingScope.mockResolvedValue({
projectRoles: ['project:viewer'],
workflowRoles: ['workflow:owner'],
});
await insightsService.getInsightsSummary({ user, startDate, endDate });
expect(
mockInsightsByPeriodRepository.getPreviousAndCurrentPeriodTypeAggregates,
).toHaveBeenCalledWith(
expect.objectContaining({
accessFilter: {
user,
projectRoles: ['project:viewer'],
workflowRoles: ['workflow:owner'],
},
}),
);
});
});
});
describe('timeZone forwarding', () => {
@@ -813,7 +923,12 @@ describe('InsightsService', () => {
[],
);
await insightsService.getInsightsSummary({ startDate, endDate, timeZone: 'Europe/Berlin' });
await insightsService.getInsightsSummary({
user,
startDate,
endDate,
timeZone: 'Europe/Berlin',
});
expect(
mockInsightsByPeriodRepository.getPreviousAndCurrentPeriodTypeAggregates,
@@ -842,7 +957,12 @@ describe('InsightsService', () => {
it('forwards timeZone to getInsightsByTime', async () => {
mockInsightsByPeriodRepository.getInsightsByTime.mockResolvedValue([]);
await insightsService.getInsightsByTime({ startDate, endDate, timeZone: 'Europe/Berlin' });
await insightsService.getInsightsByTime({
user,
startDate,
endDate,
timeZone: 'Europe/Berlin',
});
expect(mockInsightsByPeriodRepository.getInsightsByTime).toHaveBeenCalledWith(
expect.objectContaining({ timeZone: 'Europe/Berlin' }),
@@ -871,55 +991,148 @@ describe('InsightsService', () => {
const makeUser = (scopes: string[]) =>
({ role: { scopes: scopes.map((slug) => ({ slug })) } }) as unknown as User;
it('sets hasReadAccess only for workflows in the shared set for non-admins', async () => {
beforeEach(() => {
mockInsightsByPeriodRepository.getInsightsByWorkflow.mockResolvedValue({
count: 0,
rows: [],
});
});
it('sets hasReadAccess true for a row with a workflowId', async () => {
const user = makeUser([]);
mockInsightsByPeriodRepository.getInsightsByWorkflow.mockResolvedValue({
count: 2,
rows: [makeRow('wf-accessible'), makeRow('wf-inaccessible')],
count: 1,
rows: [makeRow('wf-1')],
});
mockWorkflowSharingService.getSharedWorkflowIds.mockResolvedValue(['wf-accessible']);
const result = await insightsService.getInsightsByWorkflow({ user, startDate, endDate });
expect(mockWorkflowSharingService.getSharedWorkflowIds).toHaveBeenCalledWith(user, {
scopes: ['workflow:read'],
projectId: undefined,
});
expect(result.data.find((r) => r.workflowId === 'wf-accessible')?.hasReadAccess).toBe(true);
expect(result.data.find((r) => r.workflowId === 'wf-inaccessible')?.hasReadAccess).toBe(
false,
);
expect(result.data[0].hasReadAccess).toBe(true);
});
it('sets hasReadAccess true for all rows for admins', async () => {
it('sets hasReadAccess false for a row with a null workflowId (deleted workflow)', async () => {
const user = makeUser(['workflow:read']);
mockInsightsByPeriodRepository.getInsightsByWorkflow.mockResolvedValue({
count: 2,
rows: [makeRow('wf-1'), makeRow('wf-2')],
});
// getSharedWorkflowIds returns all workflow IDs for users with the global scope
mockWorkflowSharingService.getSharedWorkflowIds.mockResolvedValue(['wf-1', 'wf-2']);
const result = await insightsService.getInsightsByWorkflow({ user, startDate, endDate });
expect(mockWorkflowSharingService.getSharedWorkflowIds).toHaveBeenCalledWith(user, {
scopes: ['workflow:read'],
projectId: undefined,
});
expect(result.data.every((r) => r.hasReadAccess)).toBe(true);
});
it('sets hasReadAccess false for rows with a null workflowId (deleted workflow)', async () => {
const user = makeUser([]);
mockInsightsByPeriodRepository.getInsightsByWorkflow.mockResolvedValue({
count: 1,
rows: [makeRow(null)],
});
mockWorkflowSharingService.getSharedWorkflowIds.mockResolvedValue([]);
const result = await insightsService.getInsightsByWorkflow({ user, startDate, endDate });
expect(result.data[0].hasReadAccess).toBe(false);
});
describe('project access', () => {
it('should not check project access when no project is requested', async () => {
await insightsService.getInsightsByWorkflow({ user, startDate, endDate });
expect(userHasScopes).not.toHaveBeenCalled();
});
it('should throw a forbidden error when the requested project is not accessible', async () => {
vi.mocked(userHasScopes).mockResolvedValue(false);
await expect(
insightsService.getInsightsByWorkflow({
user,
startDate,
endDate,
projectId: 'project-1',
}),
).rejects.toThrow('You do not have access to insights for this project.');
expect(mockInsightsByPeriodRepository.getInsightsByWorkflow).not.toHaveBeenCalled();
});
it('should query without an access filter for users with the global workflow read scope', async () => {
mockWorkflowSharingService.rolesGrantingScope.mockResolvedValue(undefined);
await insightsService.getInsightsByWorkflow({ user, startDate, endDate });
expect(mockInsightsByPeriodRepository.getInsightsByWorkflow).toHaveBeenCalledWith(
expect.objectContaining({ accessFilter: undefined }),
);
});
it('should query with an access filter for users without the global workflow read scope', async () => {
mockWorkflowSharingService.rolesGrantingScope.mockResolvedValue({
projectRoles: ['project:viewer'],
workflowRoles: ['workflow:owner'],
});
await insightsService.getInsightsByWorkflow({ user, startDate, endDate });
expect(mockInsightsByPeriodRepository.getInsightsByWorkflow).toHaveBeenCalledWith(
expect.objectContaining({
accessFilter: {
user,
projectRoles: ['project:viewer'],
workflowRoles: ['workflow:owner'],
},
}),
);
});
});
});
describe('getInsightsByTime', () => {
const startDate = new Date('2024-01-01');
const endDate = new Date('2024-01-07');
beforeEach(() => {
mockInsightsByPeriodRepository.getInsightsByTime.mockResolvedValue([]);
});
describe('project access', () => {
it('should not check project access when no project is requested', async () => {
await insightsService.getInsightsByTime({ user, startDate, endDate });
expect(userHasScopes).not.toHaveBeenCalled();
});
it('should throw a forbidden error when the requested project is not accessible', async () => {
vi.mocked(userHasScopes).mockResolvedValue(false);
await expect(
insightsService.getInsightsByTime({
user,
startDate,
endDate,
projectId: 'project-1',
}),
).rejects.toThrow('You do not have access to insights for this project.');
expect(mockInsightsByPeriodRepository.getInsightsByTime).not.toHaveBeenCalled();
});
it('should query without an access filter for users with the global workflow read scope', async () => {
mockWorkflowSharingService.rolesGrantingScope.mockResolvedValue(undefined);
await insightsService.getInsightsByTime({ user, startDate, endDate });
expect(mockInsightsByPeriodRepository.getInsightsByTime).toHaveBeenCalledWith(
expect.objectContaining({ accessFilter: undefined }),
);
});
it('should query with an access filter for users without the global workflow read scope', async () => {
mockWorkflowSharingService.rolesGrantingScope.mockResolvedValue({
projectRoles: ['project:viewer'],
workflowRoles: ['workflow:owner'],
});
await insightsService.getInsightsByTime({ user, startDate, endDate });
expect(mockInsightsByPeriodRepository.getInsightsByTime).toHaveBeenCalledWith(
expect.objectContaining({
accessFilter: {
user,
projectRoles: ['project:viewer'],
workflowRoles: ['workflow:owner'],
},
}),
);
});
});
});
});
@@ -1,11 +1,22 @@
import { createTeamProject, createWorkflow, testDb, testModules } from '@n8n/backend-test-utils';
import { GlobalConfig } from '@n8n/config';
import {
createTeamProject,
createWorkflow,
linkUserToProject,
testDb,
testModules,
} from '@n8n/backend-test-utils';
import type { Project, User, WorkflowEntity } from '@n8n/db';
import { Container } from '@n8n/di';
import { DateTime } from 'luxon';
import { InsightsConfig } from '@/modules/insights/insights.config';
import { createMember } from '@test-integration/db/users';
import { createCompactedInsightsEvent, createMetadata } from '../../entities/__tests__/db-utils';
import type { InsightsByPeriod } from '../../entities/insights-by-period';
import { TypeToNumber } from '../../entities/insights-shared';
import type { InsightsAccessFilter } from '../insights-by-period.repository';
import { InsightsByPeriodRepository } from '../insights-by-period.repository';
const isPostgres = Container.get(GlobalConfig).database.type === 'postgresdb';
@@ -268,4 +279,152 @@ describe('InsightsByPeriodRepository', () => {
expect(transactionSpy).toHaveBeenCalledTimes(1);
});
});
describe('access filter', () => {
let member: User;
let accessibleProject: Project;
let accessibleWorkflow: WorkflowEntity;
let accessibleInsight: InsightsByPeriod;
let inaccessibleProject: Project;
let inaccessibleWorkflow: WorkflowEntity;
let inaccessibleInsight: InsightsByPeriod;
let accessFilter: InsightsAccessFilter;
let startDate: Date;
let endDate: Date;
beforeAll(async () => {
member = await createMember();
accessibleProject = await createTeamProject();
await linkUserToProject(member, accessibleProject, 'project:viewer');
accessibleWorkflow = await createWorkflow({}, accessibleProject);
inaccessibleProject = await createTeamProject();
inaccessibleWorkflow = await createWorkflow({}, inaccessibleProject);
const now = DateTime.utc();
startDate = now.minus({ days: 7 }).toJSDate();
endDate = now.toJSDate();
[accessibleInsight, inaccessibleInsight] = await Promise.all([
createCompactedInsightsEvent(accessibleWorkflow, {
type: 'success',
value: 4,
periodUnit: 'day',
periodStart: now.minus({ days: 1 }),
}),
createCompactedInsightsEvent(inaccessibleWorkflow, {
type: 'success',
value: 10,
periodUnit: 'day',
periodStart: now.minus({ days: 1 }),
}),
]);
accessFilter = {
user: member,
projectRoles: ['project:viewer'],
workflowRoles: ['workflow:owner'],
};
});
describe('getPreviousAndCurrentPeriodTypeAggregates', () => {
test('should aggregate both workflows when no access filter is applied', async () => {
const insightsByPeriodRepository = Container.get(InsightsByPeriodRepository);
const rows = await insightsByPeriodRepository.getPreviousAndCurrentPeriodTypeAggregates({
startDate,
endDate,
});
const currentSuccessTotal = rows.find(
(row) => row.period === 'current' && row.type === TypeToNumber.success,
)?.total_value;
expect(Number(currentSuccessTotal)).toBe(
accessibleInsight.value + inaccessibleInsight.value,
);
});
test('should exclude workflows outside the access filter', async () => {
const insightsByPeriodRepository = Container.get(InsightsByPeriodRepository);
const rows = await insightsByPeriodRepository.getPreviousAndCurrentPeriodTypeAggregates({
startDate,
endDate,
accessFilter,
});
const currentSuccessTotal = rows.find(
(row) => row.period === 'current' && row.type === TypeToNumber.success,
)?.total_value;
expect(Number(currentSuccessTotal)).toBe(accessibleInsight.value);
});
});
describe('getInsightsByWorkflow', () => {
test('should return both workflows when no access filter is applied', async () => {
const insightsByPeriodRepository = Container.get(InsightsByPeriodRepository);
const { count, rows } = await insightsByPeriodRepository.getInsightsByWorkflow({
startDate,
endDate,
});
expect(count).toBe(2);
expect(rows.map((row) => row.workflowId).sort()).toEqual(
[accessibleWorkflow.id, inaccessibleWorkflow.id].sort(),
);
});
test('should exclude workflows outside the access filter', async () => {
const insightsByPeriodRepository = Container.get(InsightsByPeriodRepository);
const { count, rows } = await insightsByPeriodRepository.getInsightsByWorkflow({
startDate,
endDate,
accessFilter,
});
expect(count).toBe(1);
expect(rows).toHaveLength(1);
expect(rows[0].workflowId).toBe(accessibleWorkflow.id);
expect(rows[0].succeeded).toBe(accessibleInsight.value);
});
});
describe('getInsightsByTime', () => {
test('should aggregate both workflows when no access filter is applied', async () => {
const insightsByPeriodRepository = Container.get(InsightsByPeriodRepository);
const rows = await insightsByPeriodRepository.getInsightsByTime({
startDate,
endDate,
periodUnit: 'day',
insightTypes: ['success'],
});
const totalSucceeded = rows.reduce((sum, row) => sum + (row.succeeded ?? 0), 0);
expect(totalSucceeded).toBe(accessibleInsight.value + inaccessibleInsight.value);
});
test('should exclude workflows outside the access filter', async () => {
const insightsByPeriodRepository = Container.get(InsightsByPeriodRepository);
const rows = await insightsByPeriodRepository.getInsightsByTime({
startDate,
endDate,
periodUnit: 'day',
insightTypes: ['success'],
accessFilter,
});
const totalSucceeded = rows.reduce((sum, row) => sum + (row.succeeded ?? 0), 0);
expect(totalSucceeded).toBe(accessibleInsight.value);
});
});
});
});
@@ -1,10 +1,12 @@
import { isValidTimeZone } from '@n8n/api-types';
import { GlobalConfig } from '@n8n/config';
import { sql } from '@n8n/db';
import type { User } from '@n8n/db';
import { sql, SharedWorkflowRepository } from '@n8n/db';
import { Container, Service } from '@n8n/di';
import type { SelectQueryBuilder } from '@n8n/typeorm';
import { DataSource, LessThanOrEqual, Repository } from '@n8n/typeorm';
import { DateTime } from 'luxon';
import { UnexpectedError } from 'n8n-workflow';
import { z } from 'zod';
import { getDateRangesCommonTableExpressionQuery } from './insights-by-period-query.helper';
@@ -73,11 +75,24 @@ const aggregatedInsightsByTimeParser = z
})
.array();
/**
* Identifies a caller whose insights must be limited to the workflows they can
* read.
*/
export type InsightsAccessFilter = {
user: User;
projectRoles: string[];
workflowRoles: string[];
};
@Service()
export class InsightsByPeriodRepository extends Repository<InsightsByPeriod> {
private isRunningCompaction = false;
constructor(dataSource: DataSource) {
constructor(
dataSource: DataSource,
private readonly sharedWorkflowRepository: SharedWorkflowRepository,
) {
super(InsightsByPeriod, dataSource.manager);
}
@@ -85,6 +100,30 @@ export class InsightsByPeriodRepository extends Repository<InsightsByPeriod> {
return this.manager.connection.driver.escape(fieldName);
}
/**
* Limits a query to insights for workflows the caller can read, by
* correlating against their workflow shares.
*/
private applyAccessFilter(
qb: SelectQueryBuilder<InsightsByPeriod>,
accessFilter: InsightsAccessFilter,
) {
// Ensure the metadata relation is joined, so we can correlate against workflowId
if (!qb.expressionMap.joinAttributes.some((join) => join.alias.name === 'metadata')) {
throw new UnexpectedError('The metadata relation must be joined before the access filter');
}
const subquery = this.sharedWorkflowRepository.buildSharedWorkflowIdsSubquery(
accessFilter.user,
{
projectRoles: accessFilter.projectRoles,
workflowRoles: accessFilter.workflowRoles,
},
);
subquery.andWhere('"sw"."workflowId" = metadata."workflowId"');
qb.andWhere(`EXISTS (${subquery.getQuery()})`).setParameters(subquery.getParameters());
}
private getPeriodFilterExpr(maxAgeInDays = 0) {
// Database-specific period start expression to filter out data to compact by days matching the periodUnit
let periodStartExpr = `date('now', '-${maxAgeInDays} days')`;
@@ -305,7 +344,14 @@ export class InsightsByPeriodRepository extends Repository<InsightsByPeriod> {
endDate,
projectId,
timeZone,
}: { projectId?: string; startDate: Date; endDate: Date; timeZone?: string }): Promise<
accessFilter,
}: {
projectId?: string;
startDate: Date;
endDate: Date;
accessFilter?: InsightsAccessFilter;
timeZone?: string;
}): Promise<
Array<{
period: 'previous' | 'current';
type: 0 | 1 | 2 | 3;
@@ -336,10 +382,17 @@ export class InsightsByPeriodRepository extends Repository<InsightsByPeriod> {
.groupBy('period')
.addGroupBy('insights.type');
// If we're filtering by projectId or accessFilter, we need a metadata join to access projectId and workflowId.
if (projectId || accessFilter) {
rawRowsQuery.innerJoin('insights.metadata', 'metadata');
}
if (projectId) {
rawRowsQuery
.innerJoin('insights.metadata', 'metadata')
.andWhere('metadata.projectId = :projectId', { projectId });
rawRowsQuery.andWhere('metadata.projectId = :projectId', { projectId });
}
if (accessFilter) {
this.applyAccessFilter(rawRowsQuery, accessFilter);
}
const rawRows = await rawRowsQuery.getRawMany();
@@ -373,6 +426,7 @@ export class InsightsByPeriodRepository extends Repository<InsightsByPeriod> {
sortBy = 'total:desc',
projectId,
timeZone,
accessFilter,
}: {
skip?: number;
take?: number;
@@ -381,6 +435,7 @@ export class InsightsByPeriodRepository extends Repository<InsightsByPeriod> {
startDate: Date;
endDate: Date;
timeZone?: string;
accessFilter?: InsightsAccessFilter;
}) {
const [sortField, sortOrder] = this.parseSortingParams(sortBy);
const sumOfExecutions = sql`SUM(CASE WHEN insights.type IN (${TypeToNumber.success.toString()}, ${TypeToNumber.failure.toString()}) THEN value ELSE 0 END)`;
@@ -422,6 +477,10 @@ export class InsightsByPeriodRepository extends Repository<InsightsByPeriod> {
rawRowsQuery.andWhere('metadata.projectId = :projectId', { projectId });
}
if (accessFilter) {
this.applyAccessFilter(rawRowsQuery, accessFilter);
}
const paginatedQuery = rawRowsQuery
.clone()
.orderBy(this.escapeField(sortField), sortOrder)
@@ -443,6 +502,7 @@ export class InsightsByPeriodRepository extends Repository<InsightsByPeriod> {
startDate,
endDate,
timeZone,
accessFilter,
}: {
periodUnit: PeriodUnit;
insightTypes: TypeUnit[];
@@ -450,6 +510,7 @@ export class InsightsByPeriodRepository extends Repository<InsightsByPeriod> {
startDate: Date;
endDate: Date;
timeZone?: string;
accessFilter?: InsightsAccessFilter;
}) {
const cte = getDateRangesCommonTableExpressionQuery({ dbType, startDate, endDate, timeZone });
@@ -474,10 +535,17 @@ export class InsightsByPeriodRepository extends Repository<InsightsByPeriod> {
.groupBy(periodStartExpr)
.orderBy(periodStartExpr, 'ASC');
// If we're filtering by projectId or accessFilter, we need a metadata join to access projectId and workflowId.
if (projectId || accessFilter) {
rawRowsQuery.innerJoin('insights.metadata', 'metadata');
}
if (projectId) {
rawRowsQuery
.innerJoin('insights.metadata', 'metadata')
.andWhere('metadata.projectId = :projectId', { projectId });
rawRowsQuery.andWhere('metadata.projectId = :projectId', { projectId });
}
if (accessFilter) {
this.applyAccessFilter(rawRowsQuery, accessFilter);
}
const rawRows = await rawRowsQuery.getRawMany();
@@ -24,13 +24,14 @@ export class InsightsController {
@Get('/summary')
@GlobalScope('insights:list')
async getInsightsSummary(
_req: AuthenticatedRequest,
req: AuthenticatedRequest,
_res: Response,
@Query query: InsightsDateFilterDto = {},
): Promise<InsightsSummary> {
const { startDate, endDate, timeZone } = this.prepareDateFilters(query);
return await this.insightsService.getInsightsSummary({
user: req.user,
startDate,
endDate,
timeZone,
@@ -64,7 +65,7 @@ export class InsightsController {
@GlobalScope('insights:list')
@Licensed('feat:insights:viewDashboard')
async getInsightsByTime(
_req: AuthenticatedRequest,
req: AuthenticatedRequest,
_res: Response,
@Query query: InsightsDateFilterDto,
): Promise<InsightsByTime[]> {
@@ -73,6 +74,7 @@ export class InsightsController {
// Cast to full insights by time type
// as the service returns all types by default
return (await this.insightsService.getInsightsByTime({
user: req.user,
projectId: query.projectId,
startDate,
endDate,
@@ -87,7 +89,7 @@ export class InsightsController {
@Get('/by-time/time-saved')
@GlobalScope('insights:list')
async getTimeSavedInsightsByTime(
_req: AuthenticatedRequest,
req: AuthenticatedRequest,
_res: Response,
@Query query: InsightsDateFilterDto,
): Promise<RestrictedInsightsByTime[]> {
@@ -96,6 +98,7 @@ export class InsightsController {
// Cast to restricted insights by time type
// as the service returns only time saved data
return (await this.insightsService.getInsightsByTime({
user: req.user,
insightTypes: ['time_saved_min'],
projectId: query.projectId,
startDate,
@@ -7,10 +7,13 @@ import { DateTime } from 'luxon';
import { InstanceSettings } from 'n8n-core';
import { UserError } from 'n8n-workflow';
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
import { userHasScopes } from '@/permissions.ee/check-access';
import { WorkflowSharingService } from '@/workflows/workflow-sharing.service';
import type { PeriodUnit, TypeUnit } from './database/entities/insights-shared';
import { NumberToType, TypeToNumber } from './database/entities/insights-shared';
import type { InsightsAccessFilter } from './database/repositories/insights-by-period.repository';
import { InsightsByPeriodRepository } from './database/repositories/insights-by-period.repository';
import { InsightsCompactionService } from './insights-compaction.service';
import { InsightsPruningService } from './insights-pruning.service';
@@ -70,22 +73,56 @@ export class InsightsService {
await this.stopCompactionAndPruningTimers();
}
/**
* Resolves what insights the caller may read. A requested project must be
* readable by them. When no specific project is requested, results are limited to the
* workflows they can read.
*
* Returns the filter to apply, or `undefined` when the caller's global role
* already grants access to every workflow.
*/
private async resolveAccessFilter(
user: User,
projectId?: string,
): Promise<InsightsAccessFilter | undefined> {
if (projectId) {
const userHasRequiredProjectScopes = await userHasScopes(user, ['workflow:read'], false, {
projectId,
});
if (!userHasRequiredProjectScopes) {
throw new ForbiddenError('You do not have access to insights for this project.');
}
}
const workflowReadRoles = await this.workflowSharingService.rolesGrantingScope(
user,
'workflow:read',
);
return workflowReadRoles && { user, ...workflowReadRoles };
}
async getInsightsSummary({
user,
startDate,
endDate,
projectId,
timeZone,
}: {
user: User;
projectId?: string;
startDate: Date;
endDate: Date;
timeZone?: string;
}): Promise<InsightsSummary> {
const accessFilter = await this.resolveAccessFilter(user, projectId);
const rows = await this.insightsByPeriodRepository.getPreviousAndCurrentPeriodTypeAggregates({
startDate,
endDate,
projectId,
timeZone,
accessFilter,
});
// Initialize data structures for both periods
@@ -186,6 +223,8 @@ export class InsightsService {
endDate: Date;
timeZone?: string;
}) {
const accessFilter = await this.resolveAccessFilter(user, projectId);
const { count, rows } = await this.insightsByPeriodRepository.getInsightsByWorkflow({
startDate,
endDate,
@@ -194,18 +233,13 @@ export class InsightsService {
sortBy,
projectId,
timeZone,
accessFilter,
});
const accessibleWorkflowIds = new Set(
await this.workflowSharingService.getSharedWorkflowIds(user, {
scopes: ['workflow:read'],
projectId,
}),
);
// A non-null means the caller can read it; null means the workflow has since been deleted.
const data = rows.map((row) => ({
...row,
hasReadAccess: row.workflowId !== null && accessibleWorkflowIds.has(row.workflowId),
hasReadAccess: row.workflowId !== null,
}));
return {
@@ -215,6 +249,7 @@ export class InsightsService {
}
async getInsightsByTime({
user,
// Default to all insight types
insightTypes = Object.keys(TypeToNumber) as TypeUnit[],
projectId,
@@ -222,12 +257,14 @@ export class InsightsService {
endDate,
timeZone,
}: {
user: User;
insightTypes?: TypeUnit[];
projectId?: string;
startDate: Date;
endDate: Date;
timeZone?: string;
}) {
const accessFilter = await this.resolveAccessFilter(user, projectId);
const periodUnit = this.getDateFiltersGranularity({ startDate, endDate });
const rows = await this.insightsByPeriodRepository.getInsightsByTime({
periodUnit,
@@ -236,6 +273,7 @@ export class InsightsService {
startDate,
endDate,
timeZone,
accessFilter,
});
return rows.map((r) => {
@@ -67,6 +67,7 @@ const insightsHandlers: InsightsHandlers = {
}
const summary = await Container.get(InsightsService).getInsightsSummary({
user: req.user,
startDate,
endDate,
projectId: query.data.projectId,
@@ -7,7 +7,7 @@ import type {
WorkflowRepository,
} from '@n8n/db';
import type { EntityManager, UpdateResult } from '@n8n/typeorm';
import type { IWorkflowBase } from 'n8n-workflow';
import type { INode, IWorkflowBase } from 'n8n-workflow';
import { WorkflowActivationError } from 'n8n-workflow';
import { mock } from 'vitest-mock-extended';
@@ -115,6 +115,189 @@ describe('EnterpriseWorkflowService', () => {
expect(() => service.validateCredentialPermissionsToUser(workflow, [])).not.toThrow();
});
it('should inspect credentials referenced inside an inline sub-workflow', () => {
const workflow = mock<IWorkflowBase>({
nodes: [
{
type: 'n8n-nodes-base.executeWorkflow',
parameters: {
source: 'parameter',
workflowJson: JSON.stringify({
nodes: [{ credentials: { spotifyApi: { id: 'cred-unknown', name: 'x' } } }],
connections: {},
}),
},
},
],
});
expect(() =>
service.validateCredentialPermissionsToUser(workflow, [
mock<CredentialsEntity>({ id: 'cred-1' }),
]),
).toThrow();
});
it('should pass when an inline sub-workflow credential is in the allowed list', () => {
const workflow = mock<IWorkflowBase>({
nodes: [
{
type: 'n8n-nodes-base.executeWorkflow',
parameters: {
source: 'parameter',
workflowJson: JSON.stringify({
nodes: [{ credentials: { spotifyApi: { id: 'cred-1', name: 'x' } } }],
connections: {},
}),
},
},
],
});
expect(() =>
service.validateCredentialPermissionsToUser(workflow, [
mock<CredentialsEntity>({ id: 'cred-1' }),
]),
).not.toThrow();
});
it('should reject a non-managed credential with a null id (name-only reference)', () => {
// Built as a real object: mock<IWorkflowBase> strips an explicit null id.
const workflow = {
nodes: [{ credentials: { spotifyApi: { id: null, name: 'Someone else prod' } } }],
} as unknown as IWorkflowBase;
expect(() =>
service.validateCredentialPermissionsToUser(workflow, [
mock<CredentialsEntity>({ id: 'cred-1' }),
]),
).toThrow();
});
it('should reject a non-managed credential with an empty-string id', () => {
const workflow = mock<IWorkflowBase>({
nodes: [{ credentials: { spotifyApi: { id: '', name: 'Someone else prod' } } }],
});
expect(() =>
service.validateCredentialPermissionsToUser(workflow, [
mock<CredentialsEntity>({ id: 'cred-1' }),
]),
).toThrow();
});
it('should reject a null-id credential hidden inside an inline sub-workflow', () => {
const workflow = mock<IWorkflowBase>({
nodes: [
{
type: 'n8n-nodes-base.executeWorkflow',
parameters: {
source: 'parameter',
workflowJson: JSON.stringify({
nodes: [{ credentials: { spotifyApi: { id: null, name: 'Someone else prod' } } }],
connections: {},
}),
},
},
],
});
expect(() =>
service.validateCredentialPermissionsToUser(workflow, [
mock<CredentialsEntity>({ id: 'cred-1' }),
]),
).toThrow();
});
it('should inspect credentials nested in a deeper inline sub-workflow', () => {
const inner = JSON.stringify({
nodes: [{ credentials: { spotifyApi: { id: 'cred-unknown', name: 'x' } } }],
connections: {},
});
const workflow = mock<IWorkflowBase>({
nodes: [
{
type: 'n8n-nodes-base.executeWorkflow',
parameters: {
source: 'parameter',
workflowJson: JSON.stringify({
nodes: [
{ type: 'n8n-nodes-base.executeWorkflow', parameters: { workflowJson: inner } },
],
connections: {},
}),
},
},
],
});
expect(() =>
service.validateCredentialPermissionsToUser(workflow, [
mock<CredentialsEntity>({ id: 'cred-1' }),
]),
).toThrow();
});
});
describe('validateWorkflowCredentialUsage() - unresolved credentials', () => {
// Real objects, not mock<IWorkflowBase>, so the explicit null id survives.
const nodeWithNullCred = (id: string, name: string) =>
({
id,
name,
type: 'n8n-nodes-base.httpRequest',
typeVersion: 4.2,
position: [0, 0],
parameters: {},
credentials: { httpHeaderAuth: { id: null, name: 'some name' } },
}) as unknown as INode;
it('rejects a new node carrying an unresolved (name-only) credential as tampering', () => {
const newVersion = {
nodes: [nodeWithNullCred('new-1', 'Steal')],
} as unknown as IWorkflowBase;
const previousVersion = { nodes: [] } as unknown as IWorkflowBase;
expect(() =>
service.validateWorkflowCredentialUsage(newVersion, previousVersion, []),
).toThrow();
});
it('rejects an unresolved credential smuggled into a new inline sub-workflow node', () => {
const inlineNode = {
id: 'new-inline',
name: 'Sub',
type: 'n8n-nodes-base.executeWorkflow',
typeVersion: 1.2,
position: [0, 0],
parameters: {
source: 'parameter',
workflowJson: JSON.stringify({
nodes: [{ credentials: { httpHeaderAuth: { id: null, name: 'y' } } }],
connections: {},
}),
},
} as unknown as INode;
const newVersion = { nodes: [inlineNode] } as unknown as IWorkflowBase;
const previousVersion = { nodes: [] } as unknown as IWorkflowBase;
expect(() =>
service.validateWorkflowCredentialUsage(newVersion, previousVersion, []),
).toThrow();
});
it('keeps an unresolved credential on a pre-existing (read-only) node without throwing', () => {
const existing = nodeWithNullCred('existing-1', 'Call');
const newVersion = {
nodes: [{ ...existing, name: 'Renamed' }],
} as unknown as IWorkflowBase;
const previousVersion = { nodes: [existing] } as unknown as IWorkflowBase;
expect(() =>
service.validateWorkflowCredentialUsage(newVersion, previousVersion, []),
).not.toThrow();
});
});
describe('attemptWorkflowReactivation', () => {
@@ -1686,6 +1686,7 @@ describe('WorkflowService', () => {
let externalHooksMock: MockProxy<ExternalHooks>;
let workflowPublishedVersionRepositoryMock: MockProxy<WorkflowPublishedVersionRepository>;
let workflowMutationHooksMock: MockProxy<WorkflowMutationHooksProxy>;
let ownershipServiceMock: MockProxy<OwnershipService>;
const WORKFLOW_ID = 'workflow-1';
@@ -1702,6 +1703,7 @@ describe('WorkflowService', () => {
beforeEach(() => {
workflowFinderServiceMock = mock<WorkflowFinderService>();
ownershipServiceMock = mock<OwnershipService>();
workflowRepositoryMock = mock();
executionPersistenceMock = mock();
activeWorkflowManagerMock = mock();
@@ -1718,7 +1720,7 @@ describe('WorkflowService', () => {
mock(), // sharedWorkflowRepository
workflowRepositoryMock, // workflowRepository
mock(), // workflowTagMappingRepository
mock(), // ownershipService
ownershipServiceMock, // ownershipService
mock(), // tagService
mock(), // workflowHistoryService
externalHooksMock, // externalHooks
@@ -1886,6 +1888,25 @@ describe('WorkflowService', () => {
).toBeLessThan(workflowRepositoryMock.delete.mock.invocationCallOrder[0]);
});
test('invalidates the cached project for the deleted workflow', async () => {
const workflow = makeWorkflowEntity({ isArchived: true, activeVersionId: null });
workflowFinderServiceMock.findWorkflowForUser.mockResolvedValue(workflow);
await workflowService.delete(mock<User>(), WORKFLOW_ID, true);
expect(ownershipServiceMock.invalidateWorkflowProjectCacheByIds).toHaveBeenCalledWith([
WORKFLOW_ID,
]);
});
test('does not invalidate the cached project when the workflow is not found', async () => {
workflowFinderServiceMock.findWorkflowForUser.mockResolvedValue(null);
await workflowService.delete(mock<User>(), WORKFLOW_ID, true);
expect(ownershipServiceMock.invalidateWorkflowProjectCacheByIds).not.toHaveBeenCalled();
});
test('forwards the acting user to the delete and afterDelete hooks', async () => {
const workflow = makeWorkflowEntity({ isArchived: true, activeVersionId: null });
workflowFinderServiceMock.findWorkflowForUser.mockResolvedValue(workflow);
@@ -146,4 +146,27 @@ export class WorkflowSharingService {
});
return sharedWorkflows.map(({ workflowId }) => workflowId);
}
/**
* Resolve the roles granting `scope`. Returns `undefined` when the user's
* global role already grants the scope, meaning no filtering is needed.
*/
async rolesGrantingScope(
user: User,
scope: Scope,
): Promise<{ projectRoles: string[]; workflowRoles: string[] } | undefined> {
if (hasGlobalScope(user, scope)) {
return undefined;
}
const [projectRoles, workflowRoles] = await Promise.all([
this.roleService.rolesWithScope('project', [scope]),
this.roleService.rolesWithScope('workflow', [scope]),
]);
return {
projectRoles,
workflowRoles,
};
}
}
@@ -154,21 +154,14 @@ export class EnterpriseWorkflowService {
workflow: IWorkflowBase,
allowedCredentials: CredentialsEntity[],
) {
workflow.nodes.forEach((node) => {
if (!node.credentials) {
return;
}
Object.keys(node.credentials).forEach((credentialType) => {
const credential = node.credentials?.[credentialType];
if (credential?.__aiGatewayManaged && credential?.id === null) return;
const credentialId = credential?.id;
if (credentialId === undefined) return;
const matchedCredential = allowedCredentials.find(({ id }) => id === credentialId);
if (!matchedCredential) {
throw new UserError('The workflow contains credentials that you do not have access to');
}
});
});
// Reuse the shared collector so inline sub-workflow credentials (Execute
// Sub-workflow, Workflow Tool, Workflow Retriever) and unresolved name-only
// references are inspected too, keeping this check aligned with the update path.
const allowedCredentialIds = allowedCredentials.map(({ id }) => id);
const inaccessibleNodes = this.getNodesWithInaccessibleCreds(workflow, allowedCredentialIds);
if (inaccessibleNodes.length > 0) {
throw new UserError('The workflow contains credentials that you do not have access to');
}
}
async preventTampering<T extends IWorkflowBase>(workflow: T, workflowId: string, user: User) {
@@ -264,23 +257,35 @@ export class EnterpriseWorkflowService {
return newWorkflowVersion;
}
/** Get all nodes in a workflow where the node credential is not accessible to the user. */
/**
* 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.
*/
getNodesWithInaccessibleCreds(workflow: IWorkflowBase, userCredIds: string[]) {
if (!workflow.nodes) {
return [];
}
return workflow.nodes.filter((node) => {
const usedCredentialIds = this.getCredentialIdsUsedByNode(node);
return usedCredentialIds.some((credId) => !userCredIds.includes(credId));
const { ids, hasUnresolved } = this.getNodeCredentialRefs(node);
return hasUnresolved || ids.some((credId) => !userCredIds.includes(credId));
});
}
/**
* Collect every credential id a node references. Besides the node's own
* 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 parsed out and returned as well.
* 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
@@ -289,8 +294,9 @@ export class EnterpriseWorkflowService {
* the request size (each level embeds its child as literal escaped JSON) and
* cannot cycle, so the stack cannot grow unbounded.
*/
private getCredentialIdsUsedByNode(node: INode): string[] {
const credentialIds: string[] = [];
private getNodeCredentialRefs(node: INode): { ids: string[]; hasUnresolved: boolean } {
const ids: string[] = [];
let hasUnresolved = false;
const stack: INode[] = [node];
while (stack.length > 0) {
@@ -299,7 +305,13 @@ export class EnterpriseWorkflowService {
if (current.credentials) {
for (const nodeCred of Object.values(current.credentials)) {
const id = nodeCred.id?.toString();
if (id) credentialIds.push(id);
if (id) {
ids.push(id);
} else if (nodeCred.__aiGatewayManaged && nodeCred.id === null) {
// Managed credential, resolved at execution — exempt.
} else if (nodeCred.id === null || nodeCred.id === '') {
hasUnresolved = true;
}
}
}
@@ -308,7 +320,7 @@ export class EnterpriseWorkflowService {
}
}
return credentialIds;
return { ids, hasUnresolved };
}
/**
@@ -1197,6 +1197,8 @@ export class WorkflowService {
await this.workflowRepository.delete(workflowId);
await this.ownershipService.invalidateWorkflowProjectCacheByIds([workflowId]);
// After the cascade, so it can see the rows the delete orphaned. Observes a
// committed delete, so it must not throw — the module swallows its own errors.
await this.workflowMutationHooks.afterWorkflowsDeleted([workflowId]);
@@ -1,11 +1,20 @@
import type { InsightsDateRange } from '@n8n/api-types';
import { mockInstance, createWorkflow, createTeamProject, testDb } from '@n8n/backend-test-utils';
import {
mockInstance,
createWorkflow,
createTeamProject,
linkUserToProject,
testDb,
} from '@n8n/backend-test-utils';
import type { Project } from '@n8n/db';
import { GLOBAL_ADMIN_ROLE, GLOBAL_MEMBER_ROLE, GLOBAL_OWNER_ROLE } from '@n8n/db';
import { DateTime } from 'luxon';
import { createCompactedInsightsEvent } from '@/modules/insights/database/entities/__tests__/db-utils';
import type { InsightsByPeriod } from '@/modules/insights/database/entities/insights-by-period';
import { Telemetry } from '@/telemetry';
import { createCustomRoleWithScopeSlugs } from '../shared/db/roles';
import { createUser } from '../shared/db/users';
import type { SuperAgentTest } from '../shared/types';
import * as utils from '../shared/utils';
@@ -20,6 +29,17 @@ const testServer = utils.setupTestServer({
modules: ['insights'],
});
const truncateDatabase = async () => {
await testDb.truncate([
'InsightsRaw',
'InsightsByPeriod',
'InsightsMetadata',
'SharedWorkflow',
'WorkflowEntity',
'Project',
]);
};
beforeAll(async () => {
const owner = await createUser({ role: GLOBAL_OWNER_ROLE });
const admin = await createUser({ role: GLOBAL_ADMIN_ROLE });
@@ -113,6 +133,87 @@ describe('GET /insights routes return 200 if date range inside license limits',
});
});
describe('GET /insights/summary', () => {
describe('scopes results by project access', () => {
let insightsViewer: SuperAgentTest;
let accessibleProject: Project;
let inaccessibleProject: Project;
let accessibleWorkflowInsights: InsightsByPeriod;
beforeAll(async () => {
testServer.license.setDefaults({
features: ['feat:insights:viewSummary', 'feat:insights:viewDashboard'],
quotas: { 'quota:insights:maxHistoryDays': 365 },
});
// A global role granting only the insights view scopes, and no workflow access
const insightsRole = await createCustomRoleWithScopeSlugs(
['insights:list', 'insights:read'],
{
roleType: 'global',
},
);
const viewer = await createUser({ role: insightsRole });
insightsViewer = testServer.authAgentFor(viewer);
accessibleProject = await createTeamProject();
inaccessibleProject = await createTeamProject();
await linkUserToProject(viewer, accessibleProject, 'project:viewer');
const accessibleWorkflow = await createWorkflow({}, accessibleProject);
const inaccessibleWorkflow = await createWorkflow({}, inaccessibleProject);
const periodStart = DateTime.utc().startOf('day');
accessibleWorkflowInsights = await createCompactedInsightsEvent(accessibleWorkflow, {
type: 'success',
value: 1,
periodUnit: 'day',
periodStart,
});
await createCompactedInsightsEvent(inaccessibleWorkflow, {
type: 'success',
value: 5,
periodUnit: 'day',
periodStart,
});
});
test('should return the summary for a project the user can read', async () => {
const response = await insightsViewer
.get('/insights/summary')
.query({ projectId: accessibleProject.id })
.expect(200);
expect(response.body.data.total.value).toBe(accessibleWorkflowInsights.value);
});
test('should return 403 for a project the user cannot read', async () => {
await insightsViewer
.get('/insights/summary')
.query({ projectId: inaccessibleProject.id })
.expect(403);
});
test('should respond the same for an unknown project id as for an inaccessible one', async () => {
await insightsViewer
.get('/insights/summary')
.query({ projectId: 'non-existing-project-id' })
.expect(403);
});
test('should return the summary when no project is requested', async () => {
const response = await insightsViewer.get('/insights/summary').expect(200);
expect(response.body.data.total.value).toBe(accessibleWorkflowInsights.value);
});
afterAll(async () => {
await truncateDatabase();
});
});
});
describe('GET /insights/by-workflow', () => {
beforeAll(() => {
testServer.license.setDefaults({
@@ -189,14 +290,7 @@ describe('GET /insights/by-workflow', () => {
describe('sorting order verification', () => {
afterEach(async () => {
await testDb.truncate([
'InsightsRaw',
'InsightsByPeriod',
'InsightsMetadata',
'SharedWorkflow',
'WorkflowEntity',
'Project',
]);
await truncateDatabase();
});
test('should return workflows sorted by total:desc', async () => {
@@ -325,4 +419,215 @@ describe('GET /insights/by-workflow', () => {
});
});
});
describe('scopes results by project access', () => {
let insightsViewer: SuperAgentTest;
let accessibleProject: Project;
let inaccessibleProject: Project;
let inaccessibleWorkflowName: string;
let accessibleWorkflowInsights: InsightsByPeriod;
beforeAll(async () => {
testServer.license.setDefaults({
features: ['feat:insights:viewSummary', 'feat:insights:viewDashboard'],
quotas: { 'quota:insights:maxHistoryDays': 365 },
});
// A global role granting only the insights view scopes, and no workflow access
const insightsRole = await createCustomRoleWithScopeSlugs(
['insights:list', 'insights:read'],
{
roleType: 'global',
},
);
const viewer = await createUser({ role: insightsRole });
insightsViewer = testServer.authAgentFor(viewer);
accessibleProject = await createTeamProject();
inaccessibleProject = await createTeamProject();
await linkUserToProject(viewer, accessibleProject, 'project:viewer');
const accessibleWorkflow = await createWorkflow({}, accessibleProject);
const inaccessibleWorkflow = await createWorkflow(
{ name: 'TOPSECRET Payroll Sync' },
inaccessibleProject,
);
inaccessibleWorkflowName = inaccessibleWorkflow.name;
const periodStart = DateTime.utc().startOf('day');
accessibleWorkflowInsights = await createCompactedInsightsEvent(accessibleWorkflow, {
type: 'success',
value: 2,
periodUnit: 'day',
periodStart,
});
await createCompactedInsightsEvent(inaccessibleWorkflow, {
type: 'success',
value: 6,
periodUnit: 'day',
periodStart,
});
});
test('should not include workflows from inaccessible projects when no project is requested', async () => {
const response = await insightsViewer.get('/insights/by-workflow').expect(200);
expect(response.body.data.count).toBe(1);
expect(response.body.data.data).toHaveLength(1);
const workflowNames = response.body.data.data.map(
(row: { workflowName: string }) => row.workflowName,
);
expect(workflowNames).not.toContain(inaccessibleWorkflowName);
expect(response.body.data.data[0].total).toBe(accessibleWorkflowInsights.value);
});
test('should return 403 for a project the user cannot read', async () => {
await insightsViewer
.get('/insights/by-workflow')
.query({ projectId: inaccessibleProject.id })
.expect(403);
});
test('should return workflows for a project the user can read', async () => {
const response = await insightsViewer
.get('/insights/by-workflow')
.query({ projectId: accessibleProject.id })
.expect(200);
expect(response.body.data.data).toHaveLength(1);
expect(response.body.data.data[0].total).toBe(accessibleWorkflowInsights.value);
});
afterAll(async () => {
await truncateDatabase();
});
});
});
describe('GET /insights/by-time', () => {
describe('scopes results by project access', () => {
let insightsViewer: SuperAgentTest;
let accessibleProject: Project;
let inaccessibleProject: Project;
let accessibleWorkflowInsights: InsightsByPeriod;
beforeAll(async () => {
testServer.license.setDefaults({
features: ['feat:insights:viewSummary', 'feat:insights:viewDashboard'],
quotas: { 'quota:insights:maxHistoryDays': 365 },
});
// A global role granting only the insights view scopes, and no workflow access
const insightsRole = await createCustomRoleWithScopeSlugs(
['insights:list', 'insights:read'],
{
roleType: 'global',
},
);
const viewer = await createUser({ role: insightsRole });
insightsViewer = testServer.authAgentFor(viewer);
accessibleProject = await createTeamProject();
inaccessibleProject = await createTeamProject();
await linkUserToProject(viewer, accessibleProject, 'project:viewer');
const accessibleWorkflow = await createWorkflow({}, accessibleProject);
const inaccessibleWorkflow = await createWorkflow({}, inaccessibleProject);
const periodStart = DateTime.utc().startOf('day');
accessibleWorkflowInsights = await createCompactedInsightsEvent(accessibleWorkflow, {
type: 'success',
value: 3,
periodUnit: 'day',
periodStart,
});
await createCompactedInsightsEvent(inaccessibleWorkflow, {
type: 'success',
value: 7,
periodUnit: 'day',
periodStart,
});
});
test('returns 403 for an inaccessible project and scoped totals otherwise', async () => {
await insightsViewer
.get('/insights/by-time')
.query({ projectId: inaccessibleProject.id })
.expect(403);
const response = await insightsViewer.get('/insights/by-time').expect(200);
expect(response.body.data[0].values.succeeded).toBe(accessibleWorkflowInsights.value);
});
afterAll(async () => {
await truncateDatabase();
});
});
});
describe('GET /insights/by-time/time-saved', () => {
describe('scopes results by project access', () => {
let insightsViewer: SuperAgentTest;
let accessibleProject: Project;
let inaccessibleProject: Project;
let accessibleWorkflowInsights: InsightsByPeriod;
beforeAll(async () => {
testServer.license.setDefaults({
features: ['feat:insights:viewSummary', 'feat:insights:viewDashboard'],
quotas: { 'quota:insights:maxHistoryDays': 365 },
});
// A global role granting only the insights view scopes, and no workflow access
const insightsRole = await createCustomRoleWithScopeSlugs(
['insights:list', 'insights:read'],
{
roleType: 'global',
},
);
const viewer = await createUser({ role: insightsRole });
insightsViewer = testServer.authAgentFor(viewer);
accessibleProject = await createTeamProject();
inaccessibleProject = await createTeamProject();
await linkUserToProject(viewer, accessibleProject, 'project:viewer');
const accessibleWorkflow = await createWorkflow({}, accessibleProject);
const inaccessibleWorkflow = await createWorkflow({}, inaccessibleProject);
const periodStart = DateTime.utc().startOf('day');
accessibleWorkflowInsights = await createCompactedInsightsEvent(accessibleWorkflow, {
type: 'time_saved_min',
value: 4,
periodUnit: 'day',
periodStart,
});
await createCompactedInsightsEvent(inaccessibleWorkflow, {
type: 'time_saved_min',
value: 8,
periodUnit: 'day',
periodStart,
});
});
afterAll(async () => {
await truncateDatabase();
});
test('returns 403 for an inaccessible project and scoped totals otherwise', async () => {
await insightsViewer
.get('/insights/by-time/time-saved')
.query({ projectId: inaccessibleProject.id })
.expect(403);
const response = await insightsViewer.get('/insights/by-time/time-saved').expect(200);
expect(response.body.data[0].values.timeSaved).toBe(accessibleWorkflowInsights.value);
});
});
});
@@ -1,11 +1,17 @@
import { insightsSummarySchema } from '@n8n/api-types';
import { createTeamProject, createWorkflow, testDb } from '@n8n/backend-test-utils';
import { type User } from '@n8n/db';
import {
createTeamProject,
createWorkflow,
linkUserToProject,
testDb,
} from '@n8n/backend-test-utils';
import { type Project, type User } from '@n8n/db';
import { DateTime } from 'luxon';
import { createCompactedInsightsEvent } from '@/modules/insights/database/entities/__tests__/db-utils';
import { createOwnerWithApiKey } from '../shared/db/users';
import { createCustomRoleWithScopeSlugs } from '../shared/db/roles';
import { addApiKey, createOwnerWithApiKey, createUser } from '../shared/db/users';
import type { SuperAgentTest } from '../shared/types';
import * as utils from '../shared/utils';
@@ -174,4 +180,69 @@ describe('GET /insights/summary', () => {
expect(response.body.total.value).toBe(4);
expect(response.body.failed.value).toBe(1);
});
describe('project access', () => {
let viewer: User;
let viewerAgent: SuperAgentTest;
let accessibleProject: Project;
let inaccessibleProject: Project;
let accessibleWorkflow: Awaited<ReturnType<typeof createWorkflow>>;
let inaccessibleWorkflow: Awaited<ReturnType<typeof createWorkflow>>;
beforeAll(async () => {
// A global role granting only the insights view scopes, and no workflow access
const insightsRole = await createCustomRoleWithScopeSlugs(
['insights:list', 'insights:read'],
{
roleType: 'global',
},
);
viewer = await createUser({ role: insightsRole });
viewer.apiKeys = [await addApiKey(viewer, { scopes: ['insights:read'] })];
accessibleProject = await createTeamProject();
inaccessibleProject = await createTeamProject();
await linkUserToProject(viewer, accessibleProject, 'project:viewer');
accessibleWorkflow = await createWorkflow({}, accessibleProject);
inaccessibleWorkflow = await createWorkflow({}, inaccessibleProject);
});
beforeEach(() => {
viewerAgent = testServer.publicApiAgentFor(viewer);
});
test('returns 403 for a project the API key holder cannot read', async () => {
await viewerAgent
.get('/insights/summary')
.query({ projectId: inaccessibleProject.id })
.expect(403);
});
test('returns the summary for a project the API key holder can read', async () => {
await viewerAgent
.get('/insights/summary')
.query({ projectId: accessibleProject.id })
.expect(200);
});
test('aggregates only accessible projects when no project is requested', async () => {
const accessibleSuccessfulExecutions = 3;
const inaccessibleSuccessfulExecutions = 5;
await createSummaryMetrics(accessibleWorkflow, { success: accessibleSuccessfulExecutions });
await createSummaryMetrics(inaccessibleWorkflow, {
success: inaccessibleSuccessfulExecutions,
});
const response = await viewerAgent
.get('/insights/summary')
.query({
startDate: DateTime.utc().minus({ days: 2 }).toISO(),
endDate: DateTime.utc().plus({ days: 1 }).toISO(),
})
.expect(200);
expect(response.body.total.value).toBe(accessibleSuccessfulExecutions);
});
});
});
@@ -116,4 +116,26 @@ describe('WorkflowSharingService', () => {
expect(sharedWorkflowIds).not.toContain(workflow2.id);
});
});
describe('rolesGrantingScope', () => {
it('should return no options for users holding the scope globally', async () => {
const options = await workflowSharingService.rolesGrantingScope(owner, 'workflow:read');
expect(options).toBeUndefined();
});
it('should return the roles granting the scope for other users', async () => {
const options = await workflowSharingService.rolesGrantingScope(member, 'workflow:read');
expect(options?.projectRoles).toContain('project:viewer');
expect(options?.workflowRoles).toContain('workflow:owner');
});
it('should return only the roles granting the requested scope', async () => {
const options = await workflowSharingService.rolesGrantingScope(member, 'workflow:update');
expect(options?.projectRoles).not.toContain('project:viewer');
expect(options?.projectRoles).toContain('project:admin');
});
});
});
@@ -80,7 +80,7 @@ describe('WorkflowExecute node error forwarding to ErrorReporter', () => {
async function runWorkflowThatThrows(
error: unknown,
additionalDataOverrides: Partial<IWorkflowExecuteAdditionalData> = {},
): Promise<void> {
): Promise<IRun> {
const nodeType = mock<INodeType>({
description: {
name: 'manualTrigger',
@@ -116,7 +116,23 @@ describe('WorkflowExecute node error forwarding to ErrorReporter', () => {
).mockImplementation(() => {});
await workflowExecute.run({ workflow, startNode: triggerNode });
await waitPromise.promise;
return await waitPromise.promise;
}
function createAxiosError() {
return Object.assign(new Error('connect ECONNREFUSED 127.0.0.1:443'), {
name: 'AxiosError',
isAxiosError: true,
code: 'ECONNREFUSED',
config: {
headers: { 'x-request-header': 'payload' },
data: '{"requestField":"payload"}',
},
options: {
headers: { 'x-request-header': 'payload' },
data: '{"requestField":"payload"}',
},
});
}
it('should report a Error instance as class + stack only, without its message', async () => {
@@ -174,6 +190,39 @@ describe('WorkflowExecute node error forwarding to ErrorReporter', () => {
expect(mockErrorReporter.error).not.toHaveBeenCalled();
});
it('should store unhandled axios errors as NodeApiError', async () => {
const run = await runWorkflowThatThrows(createAxiosError());
const nodeError = run.data.resultData.runData.ThrowingNode[0].error;
const resultError = run.data.resultData.error;
expect(nodeError?.name).toBe('NodeApiError');
expect(resultError?.name).toBe('NodeApiError');
});
it('should omit axios config from stored errors', async () => {
const run = await runWorkflowThatThrows(createAxiosError());
const nodeError = run.data.resultData.runData.ThrowingNode[0].error;
const resultError = run.data.resultData.error;
expect(nodeError).not.toHaveProperty('config');
expect(resultError).not.toHaveProperty('config');
});
it('should omit legacy request options from stored errors', async () => {
const run = await runWorkflowThatThrows(createAxiosError());
const nodeError = run.data.resultData.runData.ThrowingNode[0].error;
const resultError = run.data.resultData.error;
expect(nodeError).not.toHaveProperty('options');
expect(resultError).not.toHaveProperty('options');
});
it('should omit request values from serialized execution errors', async () => {
const run = await runWorkflowThatThrows(createAxiosError());
expect(JSON.stringify(run.data.resultData)).not.toContain('payload');
});
it('should not report an ApplicationError with no cause', async () => {
const plainAppError = new ApplicationError('plain operational error', { level: 'error' });
@@ -27,7 +27,6 @@ import type {
ITaskData,
ITaskDataConnections,
ITaskMetadata,
NodeApiError,
NodeOperationError,
Workflow,
IRunExecutionData,
@@ -39,6 +38,7 @@ import type {
INodeIssues,
INodeType,
ITaskStartedData,
JsonObject,
AiAgentRequest,
IWorkflowExecutionDataProcess,
EngineRequest,
@@ -55,6 +55,7 @@ import {
UnexpectedError,
UserError,
OperationalError,
NodeApiError,
TimeoutExecutionCancelledError,
ManualExecutionCancelledError,
createRunExecutionData,
@@ -105,6 +106,14 @@ interface RunWorkflowOptions {
additionalRunFilterNodes?: string[];
}
function normalizeUnhandledAxiosError(error: unknown, node: INode): ExecutionBaseError {
if (isAxiosError(error)) {
return new NodeApiError(node, error as JsonObject);
}
return error as ExecutionBaseError;
}
export class WorkflowExecute {
private status: ExecutionStatus = 'new';
@@ -2010,7 +2019,7 @@ export class WorkflowExecute {
});
}
const e = error as unknown as ExecutionBaseError;
const e = normalizeUnhandledAxiosError(error, executionNode);
executionError = { ...e, message: e.message, stack: e.stack };
@@ -6483,6 +6483,8 @@
"insights.dashboard.dataRangeAlert.dismiss": "Dismiss",
"insights.dashboard.paywall.title": "Upgrade to access more detailed insights",
"insights.dashboard.paywall.description": "Gain access to more granular, per-workflow insights and visual breakdown of production executions over different time periods.",
"insights.dashboard.error.forbidden.title": "Couldn't load insights",
"insights.dashboard.error.forbidden.message": "You don't have access to insights for this project",
"insights.banner.title.timeSaved.tooltip": "Total time saved calculated from your estimated time savings per execution across all workflows",
"insights.banner.queueMode.warning": "We identified and fixed an issue where insights execution counts were duplicated for queue mode users.",
"insights.banner.queueMode.warning.link.text": "Learn more",
@@ -23,6 +23,7 @@ import type {
} from '@n8n/api-types';
import { INSIGHT_TYPES } from '@/features/execution/insights/insights.constants';
import type { InsightsSummaryDisplay } from '@/features/execution/insights/insights.types';
import { ResponseError } from '@n8n/rest-api-client/utils';
import { vi } from 'vitest';
const { emitters, addEmitter } = useEmitters<'n8nDataTableServer'>();
@@ -71,6 +72,12 @@ const mockTelemetry = {
track: vi.fn(),
};
const showError = vi.fn();
vi.mock('@n8n/composables/useToast', () => ({
useToast: () => ({ showError }),
}));
vi.mock('@n8n/composables/useTelemetry', () => ({
useTelemetry: () => mockTelemetry,
}));
@@ -726,6 +733,89 @@ describe('InsightsDashboard', () => {
},
});
});
it('should show an error and revert to all projects when the selected project is forbidden', async () => {
insightsStore.charts.execute = vi.fn().mockImplementation(async (_delay, params) => {
insightsStore.charts.error = params?.projectId
? new ResponseError('You do not have access to insights for this project.', {
httpStatusCode: 403,
})
: null;
});
renderComponent({
props: { insightType: INSIGHT_TYPES.TOTAL },
});
await waitFor(() => {
expect(screen.getByTestId('project-sharing-select')).toBeInTheDocument();
});
await selectProject(teamProjects[0].name);
await waitAllPromises();
expect(showError).toHaveBeenCalledWith(
expect.objectContaining({
httpStatusCode: 403,
message: 'You do not have access to insights for this project.',
}),
"Couldn't load insights",
{ message: "You don't have access to insights for this project" },
);
await waitFor(() => {
expect(insightsStore.charts.execute).toHaveBeenLastCalledWith(0, {
...DEFAULT_DATE_RANGE,
projectId: undefined,
});
});
});
it('should not revert the current selection when a stale forbidden request resolves after a newer one', async () => {
let resolveForbiddenFetch: () => void = () => {};
const forbiddenFetchGate = new Promise<void>((resolve) => {
resolveForbiddenFetch = resolve;
});
insightsStore.charts.execute = vi.fn().mockImplementation(async (_delay, params) => {
if (params?.projectId === teamProjects[0].id) {
await forbiddenFetchGate;
insightsStore.charts.error = new ResponseError(
'You do not have access to insights for this project.',
{ httpStatusCode: 403 },
);
return;
}
insightsStore.charts.error = null;
});
renderComponent({
props: { insightType: INSIGHT_TYPES.TOTAL },
});
await waitFor(() => {
expect(screen.getByTestId('project-sharing-select')).toBeInTheDocument();
});
// Select the forbidden project — its request is held open below.
await selectProject(teamProjects[0].name);
// Switch to an accessible project before the first request resolves.
await selectProject(teamProjects[1].name);
await waitAllPromises();
vi.clearAllMocks();
// Let the stale (forbidden) request resolve now that a newer selection is active.
resolveForbiddenFetch();
await waitAllPromises();
expect(showError).not.toHaveBeenCalled();
expect(insightsStore.charts.execute).not.toHaveBeenCalledWith(
0,
expect.objectContaining({ projectId: undefined }),
);
});
});
describe('Default date range initialization', () => {
@@ -10,7 +10,9 @@ import { useInsightsStore } from '@/features/execution/insights/insights.store';
import type { DateValue } from '@internationalized/date';
import { getLocalTimeZone, parseDate, today } from '@internationalized/date';
import type { InsightsSummaryType } from '@n8n/api-types';
import { useToast } from '@n8n/composables/useToast';
import { useI18n } from '@n8n/i18n';
import { ResponseError } from '@n8n/rest-api-client/utils';
import {
computed,
defineAsyncComponent,
@@ -61,6 +63,7 @@ const props = defineProps<{
const route = useRoute();
const i18n = useI18n();
const toast = useToast();
const insightsStore = useInsightsStore();
const projectsStore = useProjectsStore();
@@ -167,33 +170,47 @@ const fetchPaginatedTableData = ({
});
};
let latestFetchId = 0;
watch(
() => [props.insightType, selectedProject.value, range.value],
() => {
async () => {
const fetchId = ++latestFetchId;
sortTableBy.value = [{ id: props.insightType, desc: true }];
const { startDate, endDate } = getFilteredRange();
const projectId = selectedProject.value?.id;
if (insightsStore.isSummaryEnabled) {
void insightsStore.summary.execute(0, {
startDate,
endDate,
projectId: selectedProject.value?.id,
});
void insightsStore.summary.execute(0, { startDate, endDate, projectId });
}
void insightsStore.charts.execute(0, {
startDate,
endDate,
projectId: selectedProject.value?.id,
});
const chartsPromise = insightsStore.charts.execute(0, { startDate, endDate, projectId });
if (insightsStore.isDashboardEnabled) {
fetchPaginatedTableData({
sortBy: sortTableBy.value,
projectId: selectedProject.value?.id,
projectId,
});
}
await chartsPromise;
// A newer selection/range change has superseded this request.
if (fetchId !== latestFetchId) {
return;
}
// Callers may receive HTTP 403 if they have no `workflow:read` permission for a project.
// Revert to "All projects" instead of leaving the dashboard stuck on an all-zero, misleading state.
const chartsError = insightsStore.charts.error;
if (projectId && chartsError instanceof ResponseError && chartsError.httpStatusCode === 403) {
toast.showError(chartsError, i18n.baseText('insights.dashboard.error.forbidden.title'), {
message: i18n.baseText('insights.dashboard.error.forbidden.message'),
});
selectedProject.value = null;
}
},
{
immediate: true,
@@ -131,7 +131,11 @@ export namespace BrevoNode {
}
function validateEmailStrings(input: ValidEmailFields): ValidatedEmail {
const composer = new MailComposer({ ...input });
const composer = new MailComposer({
...input,
disableFileAccess: true,
disableUrlAccess: true,
});
const addressFields = composer.compile().getAddresses();
const fieldFetcher = new Map<string, () => Email[] | Email>([
@@ -2,6 +2,26 @@ import type { IBinaryData, IExecuteSingleFunctions, IHttpRequestOptions } from '
import { BrevoNode } from '../GenericFunctions';
const { mailComposerOptions } = vi.hoisted(() => ({
mailComposerOptions: vi.fn(),
}));
vi.mock('nodemailer/lib/mail-composer', () => ({
default: class MailComposer {
constructor(options: unknown) {
mailComposerOptions(options);
}
compile() {
return {
getAddresses: () => ({
to: [{ address: 'recipient@example.com' }],
}),
};
}
},
}));
type AttachmentEntry = { content: string; name: string };
function makeContext(overrides: {
@@ -37,6 +57,29 @@ function makeContext(overrides: {
} as unknown as IExecuteSingleFunctions;
}
describe('Brevo - email validation', () => {
beforeEach(() => {
vi.clearAllMocks();
});
it('applies content access restrictions when validating addresses', async () => {
const context = {
getNodeParameter: vi.fn().mockReturnValue('recipient@example.com'),
} as unknown as IExecuteSingleFunctions;
await BrevoNode.Validators.validateAndCompileRecipientEmails.call(context, {
url: '',
body: {},
});
expect(mailComposerOptions).toHaveBeenCalledWith({
to: 'recipient@example.com',
disableFileAccess: true,
disableUrlAccess: true,
});
});
});
describe('Brevo - validateAndCompileAttachmentsData', () => {
const validate = BrevoNode.Validators.validateAndCompileAttachmentsData;
@@ -7,7 +7,9 @@ import type {
INodeTypeDescription,
JsonObject,
} from 'n8n-workflow';
import { NodeConnectionTypes, jsonParse, NodeApiError } from 'n8n-workflow';
import { NodeConnectionTypes, NodeApiError } from 'n8n-workflow';
import { parseAndResolveQueryParameters } from '@utils/query-parameters';
import { documentFields, documentOperations, indexFields, indexOperations } from './descriptions';
import {
@@ -146,11 +148,16 @@ export class Elasticsearch implements INodeType {
// const paginate = this.getNodeParameter('paginate', i) as boolean;
if (Object.keys(options).length) {
const { query, ...rest } = options;
const { query, queryParameters, ...rest } = options;
if (query) {
Object.assign(
body,
jsonParse(query, { errorMessage: "Invalid JSON in 'Query' option" }),
parseAndResolveQueryParameters(
query,
queryParameters ?? '[]',
this.getNode(),
i,
) as IDataObject,
);
}
Object.assign(qs, rest);
@@ -395,13 +395,28 @@ export const documentFields: INodeProperties[] = [
displayName: 'Query',
name: 'query',
description:
'Query in the <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl.html">Elasticsearch Query DSL</a>',
'Query in the <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl.html">Elasticsearch Query DSL</a>. Use $1, $2, and so on as complete values to reference Query Parameters below.',
type: 'json',
typeOptions: {
alwaysOpenEditWindow: true,
},
default: '',
placeholder: placeholders.query,
hint: 'Use query parameters for dynamic values instead of embedding expressions in the query',
},
{
displayName: 'Query Parameters',
name: 'queryParameters',
description:
'Array of values to use for $1, $2, and so on, in order. Values can be strings, numbers, booleans, null, or arrays of these values.',
type: 'json',
typeOptions: {
rows: 2,
},
default: '={{ [] }}',
placeholder: '{{ [$json.name, 30] }}',
validateType: 'array',
hint: 'Build the array inside the expression, for example {{ [$json.name, 30] }} sets $1 to the name and $2 to 30',
},
{
displayName: 'Request Cache',
@@ -30,7 +30,7 @@ export const aliases = `{
export const query = `{
"query": {
"term": {
"user.id": "john"
"user.id": "$1"
}
}
}`;
@@ -0,0 +1,69 @@
{
"name": "Elasticsearch Document GetAll Query Parameters Test",
"nodes": [
{
"parameters": {},
"type": "n8n-nodes-base.manualTrigger",
"typeVersion": 1,
"position": [0, 0],
"id": "trigger-id",
"name": "When clicking 'Execute Workflow'"
},
{
"parameters": {
"resource": "document",
"operation": "getAll",
"indexId": "my-index",
"returnAll": false,
"limit": 10,
"simple": false,
"options": {
"query": "{\"query\": {\"term\": {\"user.id\": \"$1\"}}}",
"queryParameters": "={{ ['john\", \"boost\": \"2'] }}"
}
},
"type": "n8n-nodes-base.elasticsearch",
"typeVersion": 1,
"position": [200, 0],
"id": "elasticsearch-getAll",
"name": "Elasticsearch",
"credentials": {
"elasticsearchApi": {
"id": "elasticsearch-cred-id",
"name": "Elasticsearch account"
}
}
}
],
"pinData": {
"Elasticsearch": [
{
"json": {
"_index": "my-index",
"_id": "doc1",
"_score": 1,
"_source": {
"user.id": "john"
}
}
}
]
},
"connections": {
"When clicking 'Execute Workflow'": {
"main": [
[
{
"node": "Elasticsearch",
"type": "main",
"index": 0
}
]
]
}
},
"active": false,
"settings": {
"executionOrder": "v1"
}
}
@@ -0,0 +1,41 @@
import { NodeTestHarness } from '@nodes-testing/node-test-harness';
import nock from 'nock';
describe('Elasticsearch', () => {
const credentials = {
elasticsearchApi: {
username: 'user',
password: 'password',
baseUrl: 'https://elastic.local',
ignoreSSLIssues: false,
},
};
beforeAll(() => {
// The interceptor only matches if the expression-supplied value lands as a single
// plain string value in the body, quotes and all, and if `queryParameters` is kept
// out of the request's query string.
nock('https://elastic.local')
.post('/my-index/_search', {
query: { term: { 'user.id': 'john", "boost": "2' } },
})
.query({ _source: 'true', size: '10' })
.reply(200, {
hits: {
hits: [
{
_index: 'my-index',
_id: 'doc1',
_score: 1,
_source: { 'user.id': 'john' },
},
],
},
});
});
new NodeTestHarness().setupTests({
credentials,
workflowFiles: ['document-getAll-queryParameters.workflow.json'],
});
});
@@ -18,6 +18,7 @@ export type DocumentGetAllOptions = Partial<{
max_concurrent_shard_requests: number;
pre_filter_shard_size: number;
query: string;
queryParameters: unknown;
request_cache: boolean;
routing: string;
search_type: 'query_then_fetch' | 'dfs_query_then_fetch';
@@ -2,6 +2,70 @@ import type { INode } from 'n8n-workflow';
import { NodeOperationError } from 'n8n-workflow';
import type { ConfigListSummary } from 'simple-git';
const FILTER_COMMAND_CONFIG_KEY_PATTERN = /^filter\.(.*)\.(?:clean|smudge|process)$/i;
const MERGE_DRIVER_CONFIG_KEY_PATTERN = /^merge\.(.*)\.driver$/i;
const REMOTE_PACK_COMMAND_CONFIG_KEY_PATTERN = /^remote\.(.*)\.(?:uploadpack|receivepack)$/i;
const GPG_FORMAT_PROGRAM_CONFIG_KEY_PATTERN = /^gpg\.(.*)\.program$/i;
const CORE_ASKPASS_CONFIG_KEY_PATTERN = /^core\.askpass$/i;
const CORE_EDITOR_CONFIG_KEY_PATTERN = /^core\.editor$/i;
const CORE_ALTERNATE_REFS_COMMAND_CONFIG_KEY_PATTERN = /^core\.alternaterefscommand$/i;
const GC_RECENT_OBJECTS_HOOK_CONFIG_KEY_PATTERN = /^gc\.recentobjectshook$/i;
const CONFIGURED_HOOK_COMMAND_CONFIG_KEY_PATTERN = /^hook\.(.*)\.command$/i;
const SEQUENCE_EDITOR_CONFIG_KEY_PATTERN = /^sequence\.editor$/i;
const GPG_SSH_DEFAULT_KEY_COMMAND_CONFIG_KEY_PATTERN = /^gpg\.ssh\.defaultkeycommand$/i;
const URL_REWRITE_CONFIG_KEY_PATTERN = /^url\.(.*)\.(?:insteadof|pushinsteadof)$/i;
const KEY_BLACKLIST = [
FILTER_COMMAND_CONFIG_KEY_PATTERN,
MERGE_DRIVER_CONFIG_KEY_PATTERN,
REMOTE_PACK_COMMAND_CONFIG_KEY_PATTERN,
GPG_FORMAT_PROGRAM_CONFIG_KEY_PATTERN,
CORE_ASKPASS_CONFIG_KEY_PATTERN,
CORE_EDITOR_CONFIG_KEY_PATTERN,
CORE_ALTERNATE_REFS_COMMAND_CONFIG_KEY_PATTERN,
GC_RECENT_OBJECTS_HOOK_CONFIG_KEY_PATTERN,
CONFIGURED_HOOK_COMMAND_CONFIG_KEY_PATTERN,
SEQUENCE_EDITOR_CONFIG_KEY_PATTERN,
GPG_SSH_DEFAULT_KEY_COMMAND_CONFIG_KEY_PATTERN,
URL_REWRITE_CONFIG_KEY_PATTERN,
];
export function findBlacklistedKeys(
config: ConfigListSummary,
localConfigFiles: string[],
): string[] {
const localConfigFileSet = new Set(localConfigFiles);
const localConfigIndex = config.files.findIndex((file) => localConfigFileSet.has(file));
if (localConfigIndex === -1) {
return [];
}
// Scan config sources from repository-local config onward.
const repositoryConfigFiles = config.files.slice(localConfigIndex);
const forbiddenKeys = new Set<string>();
for (const file of repositoryConfigFiles) {
for (const key of Object.keys(config.values[file] ?? {})) {
if (KEY_BLACKLIST.some((pattern) => pattern.test(key))) {
forbiddenKeys.add(key);
}
}
}
return Array.from(forbiddenKeys);
}
/**
* Shared safeguards for git references: block argument injection, path
* traversal, and control characters. The caller supplies the allowed-character
+48 -10
View File
@@ -16,7 +16,7 @@ import {
import { randomBytes } from 'node:crypto';
import { mkdir, rename, rm } from 'node:fs/promises';
import { basename, dirname, isAbsolute, join, resolve } from 'path';
import type { LogOptions, SimpleGit, SimpleGitOptions } from 'simple-git';
import type { ConfigListSummary, LogOptions, SimpleGit, SimpleGitOptions } from 'simple-git';
import simpleGit from 'simple-git';
import { URL, fileURLToPath } from 'url';
@@ -35,6 +35,7 @@ import {
import {
type ConfiguredRemoteRepositories,
getConfiguredRemoteRepositories,
findBlacklistedKeys,
getRepositoryTypeForRemoteConfigKey,
mapGitConfigList,
validateGitReference,
@@ -411,11 +412,10 @@ export class Git implements INodeType {
};
const validateConfiguredRemoteRepositories = async (
git: SimpleGit,
repositoryType: 'source' | 'target',
baseDir: string,
config: ConfigListSummary,
): Promise<ConfiguredRemoteRepositories> => {
const config = await git.listConfig();
const remoteRepositories = getConfiguredRemoteRepositories(config.values, this.getNode());
const validationTargets =
repositoryType === 'source'
@@ -540,7 +540,7 @@ export class Git implements INodeType {
...(Object.keys(unsafe).length > 0 && { unsafe }),
};
const cleanEnv = Object.create(null) as Record<string, unknown>;
const cleanEnv = Object.create(null) as Record<string, string>;
const isWriteOperation = operation === 'push' || operation === 'pushTags';
// Tell git not to ask for any information via the terminal like for
// example the username. As nobody will be able to answer it would
@@ -550,6 +550,25 @@ export class Git implements INodeType {
isWriteOperation && !enableHooks ? 'git:http:https:ssh' : 'file:git:http:https:ssh';
const git: SimpleGit = simpleGit(gitOptions).env(cleanEnv);
const validateGitConfig = async (): Promise<ConfigListSummary> => {
const repositoryConfig = await git.listConfig();
if (securityConfig.enableGitNodeAllConfigKeys) {
return repositoryConfig;
}
const localConfig = await git.listConfig('local');
const blacklistedConfigKeys = findBlacklistedKeys(repositoryConfig, localConfig.files);
if (blacklistedConfigKeys.length > 0) {
throw new NodeOperationError(
this.getNode(),
`Repository Git config key '${blacklistedConfigKeys[0]}' is not allowed`,
);
}
return repositoryConfig;
};
if (operation === 'add') {
// ----------------------------------
// add
@@ -562,6 +581,7 @@ export class Git implements INodeType {
.filter((p) => p.length > 0);
// Use -- separator to prevent argument injection
await validateGitConfig();
await git.add(['--', ...paths]);
returnItems.push({
@@ -658,6 +678,7 @@ export class Git implements INodeType {
// ----------------------------------
const message = this.getNodeParameter('message', itemIndex, '') as string;
await validateGitConfig();
const branch = options.branch;
if (branch !== undefined && branch !== '') {
assertParamIsString('branch', branch, this.getNode());
@@ -694,8 +715,12 @@ export class Git implements INodeType {
// ----------------------------------
// fetch
// ----------------------------------
await validateConfiguredRemoteRepositories(git, 'source', resolvedRepositoryPath);
const repositoryConfig = await validateGitConfig();
await validateConfiguredRemoteRepositories(
'source',
resolvedRepositoryPath,
repositoryConfig,
);
await git.fetch();
returnItems.push({
json: {
@@ -736,7 +761,12 @@ export class Git implements INodeType {
// pull
// ----------------------------------
await validateConfiguredRemoteRepositories(git, 'source', resolvedRepositoryPath);
const repositoryConfig = await validateGitConfig();
await validateConfiguredRemoteRepositories(
'source',
resolvedRepositoryPath,
repositoryConfig,
);
await git.pull();
returnItems.push({
json: {
@@ -751,6 +781,7 @@ export class Git implements INodeType {
// push
// ----------------------------------
const repositoryConfig = await validateGitConfig();
const branch = options.branch;
if (branch !== undefined && branch !== '') {
assertParamIsString('branch', branch, this.getNode());
@@ -770,9 +801,9 @@ export class Git implements INodeType {
} else {
const authentication = this.getNodeParameter('authentication', 0) as string;
const { pushTarget } = await validateConfiguredRemoteRepositories(
git,
'target',
resolvedRepositoryPath,
repositoryConfig,
);
if (authentication === 'gitPassword') {
@@ -799,8 +830,12 @@ export class Git implements INodeType {
// ----------------------------------
// pushTags
// ----------------------------------
await validateConfiguredRemoteRepositories(git, 'target', resolvedRepositoryPath);
const repositoryConfig = await validateGitConfig();
await validateConfiguredRemoteRepositories(
'target',
resolvedRepositoryPath,
repositoryConfig,
);
await git.pushTags();
returnItems.push({
json: {
@@ -883,6 +918,7 @@ export class Git implements INodeType {
// status
// ----------------------------------
await validateGitConfig();
const status = await git.status();
returnItems.push(
@@ -926,6 +962,7 @@ export class Git implements INodeType {
assertParamIsBoolean('force', force, this.getNode());
}
await validateGitConfig();
await checkoutBranch(git, {
branchName,
createBranch,
@@ -952,6 +989,7 @@ export class Git implements INodeType {
const name = this.getNodeParameter('name', itemIndex, '') as string;
validateGitTag(name, this.getNode());
await validateGitConfig();
await git.addTag(name);
returnItems.push({
json: {
@@ -2,9 +2,86 @@ import { mockDeep } from 'vitest-mock-extended';
import type { ConfigListSummary } from 'simple-git';
import type { INode } from 'n8n-workflow';
import { getConfiguredRemoteRepositories, mapGitConfigList } from '../GenericFunctions';
import {
getConfiguredRemoteRepositories,
findBlacklistedKeys,
mapGitConfigList,
} from '../GenericFunctions';
describe('GenericFunctions', () => {
describe('findBlacklistedKeys', () => {
it('should reject filter and merge commands from repository config only', () => {
const config = mockDeep<ConfigListSummary>({
files: ['global-config', '.git/config', 'command line:'],
values: {
'global-config': { 'filter.lfs.process': 'git-lfs filter-process' },
'.git/config': {
'filter.poc.clean': 'command',
'merge.poc.driver': 'command',
},
'command line:': { 'core.sshcommand': 'ssh' },
},
});
const result = findBlacklistedKeys(config, ['.git/config']);
expect(result).toEqual(['filter.poc.clean', 'merge.poc.driver']);
});
it('should reject other command-bearing repository config keys', () => {
const blacklistedKeys = [
'core.askPass',
'core.editor',
'core.alternateRefsCommand',
'gc.recentObjectsHook',
'hook.pre-commit.command',
'sequence.editor',
'remote.origin.uploadpack',
'remote.origin.receivepack',
'gpg.openpgp.program',
'gpg.x509.program',
'gpg.ssh.program',
'gpg.ssh.defaultKeyCommand',
];
const config = mockDeep<ConfigListSummary>({
files: ['.git/config'],
values: {
'.git/config': Object.fromEntries(blacklistedKeys.map((key) => [key, 'command'])),
},
});
expect(findBlacklistedKeys(config, ['.git/config'])).toEqual(blacklistedKeys);
});
it('should include config files included from repository config', () => {
const config = mockDeep<ConfigListSummary>({
files: ['global-config', '.git/config', 'included-config', 'command line:'],
values: {
'global-config': {},
'.git/config': { 'include.path': 'included-config' },
'included-config': { 'FILTER.POC.PROCESS': 'command' },
'command line:': {},
},
});
const result = findBlacklistedKeys(config, ['.git/config']);
expect(result).toEqual(['FILTER.POC.PROCESS']);
});
it('should return no keys when repository config origin is absent', () => {
const config = mockDeep<ConfigListSummary>({
files: ['global-config', 'command line:'],
values: {
'global-config': { 'filter.lfs.clean': 'git-lfs clean' },
'command line:': {},
},
});
expect(findBlacklistedKeys(config, ['.git/config'])).toEqual([]);
});
});
describe('mapGitConfigList', () => {
it('should map the git config list', () => {
const config = mockDeep<ConfigListSummary>({
@@ -1,6 +1,6 @@
import { DeploymentConfig, SecurityConfig } from '@n8n/config';
import { Container } from '@n8n/di';
import type { IExecuteFunctions, ResolvedFilePath } from 'n8n-workflow';
import type { IExecuteFunctions, NodeParameterValueType, ResolvedFilePath } from 'n8n-workflow';
import { execFileSync } from 'node:child_process';
import { existsSync, writeFileSync } from 'node:fs';
import { mkdtemp, rm } from 'node:fs/promises';
@@ -22,8 +22,13 @@ describe('Git Node command-config handling', () => {
let gitNode: Git;
let repoDir: string;
let marker: string;
let additionalDirs: string[];
const buildContext = (operation: string, repositoryPath: string): Mocked<IExecuteFunctions> => {
const buildContext = (
operation: string,
repositoryPath: string,
parameters: Record<string, NodeParameterValueType | object> = {},
): Mocked<IExecuteFunctions> => {
const ctx = mock<IExecuteFunctions>({
getInputData: vi.fn(() => [{ json: {} }]),
// Swallow the expected network failure so we can assert on the side effect.
@@ -36,15 +41,22 @@ describe('Git Node command-config handling', () => {
},
});
ctx.getNodeParameter.mockImplementation(
(name: string, _itemIndex: number, fallbackValue?: unknown) => {
(
name: string,
_itemIndex: number,
fallbackValue?: NodeParameterValueType,
): NodeParameterValueType | object => {
switch (name) {
case 'operation':
return operation;
case 'repositoryPath':
return repositoryPath;
case 'options':
return {};
return parameters.options ?? {};
default:
if (Object.hasOwn(parameters, name)) {
return parameters[name];
}
return fallbackValue ?? '';
}
},
@@ -65,15 +77,25 @@ describe('Git Node command-config handling', () => {
);
repoDir = await mkdtemp(join(tmpdir(), 'n8n-git-cfg-'));
marker = join(repoDir, 'command-ran');
execFileSync('git', ['init', '-q', repoDir]);
additionalDirs = [];
execFileSync('git', ['init', '-q', '-b', 'main', repoDir]);
gitConfig(repoDir, 'user.email', 'test@example.com');
gitConfig(repoDir, 'user.name', 'Test');
});
afterEach(async () => {
await rm(repoDir, { recursive: true, force: true });
await Promise.all(
[repoDir, ...additionalDirs].map(
async (dir) => await rm(dir, { recursive: true, force: true }),
),
);
});
const git = (...args: string[]) => execFileSync('git', ['-C', repoDir, ...args]);
const markerCommand = () =>
`node -e "require('node:fs').writeFileSync(process.argv[1], '')" ${JSON.stringify(marker)}`;
it('does not run command-bearing repo-local git config on fetch', async () => {
// A repository-local sshCommand that git would otherwise run when talking to an
// ssh remote, plus an ssh remote to trigger it.
@@ -117,4 +139,104 @@ describe('Git Node command-config handling', () => {
// status succeeded (no error), proving git ran and would have queried fsmonitor.
expect((result[0][0].json as { error?: unknown }).error).toBeUndefined();
});
it('rejects a repo-local clean filter before add', async () => {
writeFileSync(join(repoDir, '.gitattributes'), '*.txt filter=poc\n');
writeFileSync(join(repoDir, 'payload.txt'), 'content');
gitConfig(repoDir, 'filter.poc.clean', `${markerCommand()}; cat`);
const result = await gitNode.execute.call(
buildContext('add', repoDir, { pathsToAdd: 'payload.txt' }),
);
expect(existsSync(marker)).toBe(false);
expect((result[0][0].json as { error?: unknown }).error).toBeDefined();
});
it('rejects a clean filter included from repository config', async () => {
const includedConfig = join(repoDir, 'included-config');
writeFileSync(join(repoDir, '.gitattributes'), '*.txt filter=poc\n');
writeFileSync(join(repoDir, 'payload.txt'), 'content');
execFileSync('git', [
'config',
'--file',
includedConfig,
'filter.poc.clean',
`${markerCommand()}; cat`,
]);
gitConfig(repoDir, 'include.path', includedConfig);
const result = await gitNode.execute.call(
buildContext('add', repoDir, { pathsToAdd: 'payload.txt' }),
);
expect(existsSync(marker)).toBe(false);
expect((result[0][0].json as { error?: unknown }).error).toBeDefined();
});
it('rejects a repo-local smudge filter before switching branch', async () => {
writeFileSync(join(repoDir, '.gitattributes'), '*.txt filter=poc\n');
writeFileSync(join(repoDir, 'payload.txt'), 'main');
git('add', '.');
git('commit', '-q', '-m', 'main');
git('checkout', '-q', '-b', 'other');
writeFileSync(join(repoDir, 'payload.txt'), 'other');
git('commit', '-q', '-am', 'other');
git('checkout', '-q', 'main');
gitConfig(repoDir, 'filter.poc.smudge', `${markerCommand()}; cat`);
const result = await gitNode.execute.call(
buildContext('switchBranch', repoDir, {
branchName: 'other',
options: { createBranch: false },
}),
);
expect(existsSync(marker)).toBe(false);
expect((result[0][0].json as { error?: unknown }).error).toBeDefined();
});
it('rejects a repo-local process filter before add', async () => {
writeFileSync(join(repoDir, '.gitattributes'), '*.txt filter=poc\n');
writeFileSync(join(repoDir, 'payload.txt'), 'content');
gitConfig(repoDir, 'filter.poc.process', markerCommand());
const result = await gitNode.execute.call(
buildContext('add', repoDir, { pathsToAdd: 'payload.txt' }),
);
expect(existsSync(marker)).toBe(false);
expect((result[0][0].json as { error?: unknown }).error).toBeDefined();
});
it('rejects a repo-local merge driver before pull', async () => {
writeFileSync(join(repoDir, '.gitattributes'), '*.txt merge=poc\n');
writeFileSync(join(repoDir, 'payload.txt'), 'base');
git('add', '.');
git('commit', '-q', '-m', 'base');
const remoteDir = await mkdtemp(join(tmpdir(), 'n8n-git-remote-'));
const otherDir = await mkdtemp(join(tmpdir(), 'n8n-git-other-'));
additionalDirs.push(remoteDir, otherDir);
execFileSync('git', ['init', '--bare', '-q', '-b', 'main', remoteDir]);
git('remote', 'add', 'origin', remoteDir);
git('push', '-q', '-u', 'origin', 'main');
execFileSync('git', ['clone', '-q', remoteDir, otherDir]);
gitConfig(otherDir, 'user.email', 'test@example.com');
gitConfig(otherDir, 'user.name', 'Test');
writeFileSync(join(otherDir, 'payload.txt'), 'remote');
execFileSync('git', ['-C', otherDir, 'commit', '-q', '-am', 'remote']);
execFileSync('git', ['-C', otherDir, 'push', '-q']);
writeFileSync(join(repoDir, 'payload.txt'), 'local');
git('commit', '-q', '-am', 'local');
gitConfig(repoDir, 'pull.rebase', 'false');
gitConfig(repoDir, 'merge.poc.driver', `${markerCommand()}; exit 1`);
const result = await gitNode.execute.call(buildContext('pull', repoDir));
expect(existsSync(marker)).toBe(false);
expect((result[0][0].json as { error?: unknown }).error).toBeDefined();
});
});
@@ -105,8 +105,13 @@ describe('Git Node', () => {
}
},
);
mockGit.listConfig.mockResolvedValue({ values: {} } as any);
mockGit.listConfig.mockResolvedValue({
files: ['.git/config', 'command line:'],
values: { '.git/config': {}, 'command line:': {} },
all: {},
} as any);
mockGit.log.mockResolvedValue({ all: [] } as any);
mockGit.raw.mockResolvedValue('');
mockMkdir.mockResolvedValue(undefined);
mockRename.mockResolvedValue(undefined);
mockRm.mockResolvedValue(undefined);
@@ -439,6 +444,25 @@ describe('Git Node', () => {
expect(mockGit.push).not.toHaveBeenCalled();
});
it('should validate repository config before pushing to a custom target', async () => {
mockExecuteFunctions.getNodeParameter
.mockReturnValueOnce('push')
.mockReturnValueOnce('/repo')
.mockReturnValueOnce({
repository: true,
targetRepository: 'https://github.com/example/repo.git',
});
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config'],
values: { '.git/config': { 'core.askPass': 'command' } },
} as any);
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
"Repository Git config key 'core.askPass' is not allowed",
);
expect(mockGit.push).not.toHaveBeenCalled();
});
it('should reject push target repositories starting with a hyphen', async () => {
mockExecuteFunctions.getNodeParameter
.mockReturnValueOnce('push')
@@ -464,6 +488,7 @@ describe('Git Node', () => {
// Mock git config for push operation
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config'],
values: { '.git/config': { 'remote.origin.url': 'https://github.com/test/repo.git' } },
} as any);
@@ -480,6 +505,7 @@ describe('Git Node', () => {
.mockReturnValueOnce({})
.mockReturnValueOnce('gitPassword');
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config'],
values: { '.git/config': { 'remote.origin.url': '/blocked/target-repo' } },
} as any);
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn(
@@ -500,6 +526,7 @@ describe('Git Node', () => {
.mockReturnValueOnce({})
.mockReturnValueOnce('none');
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config'],
values: { '.git/config': { 'remote.origin.url': '/blocked/target-repo' } },
} as any);
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn(
@@ -520,6 +547,7 @@ describe('Git Node', () => {
.mockReturnValueOnce({})
.mockReturnValueOnce('none');
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config'],
values: {
'.git/config': {
'remote.origin.url': 'https://github.com/test/repo.git',
@@ -572,6 +600,7 @@ describe('Git Node', () => {
.mockReturnValueOnce({})
.mockReturnValueOnce('none');
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config'],
values: {
'.git/config': {
'remote.origin.url': [
@@ -596,6 +625,7 @@ describe('Git Node', () => {
.mockReturnValueOnce({})
.mockReturnValueOnce('none');
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config'],
values: {
'.git/config': {
'remote.origin.url': 'https://github.com/test/repo.git',
@@ -621,6 +651,7 @@ describe('Git Node', () => {
.mockReturnValueOnce({})
.mockReturnValueOnce('none');
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config', '.git/config.worktree'],
values: {
'.git/config': {
'remote.origin.url': 'https://github.com/test/repo.git',
@@ -1043,6 +1074,7 @@ describe('Git Node', () => {
.mockReturnValueOnce('/repo')
.mockReturnValueOnce({});
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config'],
values: {
'.git/config': {
'remote.origin.url': 'https://github.com/test/repo.git',
@@ -1070,6 +1102,7 @@ describe('Git Node', () => {
.mockReturnValueOnce('/repo')
.mockReturnValueOnce({});
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config'],
values: {
'.git/config': {
'remote.origin.url': 'https://github.com/test/repo.git',
@@ -1098,6 +1131,7 @@ describe('Git Node', () => {
.mockReturnValueOnce({})
.mockReturnValueOnce('none');
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config'],
values: {
'.git/config': {
'remote.origin.url': 'https://github.com/test/repo.git',
@@ -1123,6 +1157,7 @@ describe('Git Node', () => {
.mockReturnValueOnce({})
.mockReturnValueOnce('none');
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config'],
values: {
'.git/config': {
'remote.origin.url': '/outside/source-repo',
@@ -1612,6 +1647,7 @@ describe('Git Node', () => {
.mockReturnValueOnce({});
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config'],
values: { '.git/config': { 'user.name': 'test' } },
} as any);
@@ -1782,6 +1818,23 @@ invalid line without proper format`;
expect(mockGit.addTag).toHaveBeenCalledWith('v1.2.3+build.1');
});
it('should reject signing program config before creating a tag', async () => {
mockExecuteFunctions.getNodeParameter
.mockReturnValueOnce('tag')
.mockReturnValueOnce('/repo')
.mockReturnValueOnce({})
.mockReturnValueOnce('v1.0.0');
mockGit.listConfig.mockResolvedValueOnce({
files: ['.git/config'],
values: { '.git/config': { 'gpg.ssh.program': 'command' } },
} as any);
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
"Repository Git config key 'gpg.ssh.program' is not allowed",
);
expect(mockGit.addTag).not.toHaveBeenCalled();
});
it('should reject tag operation when name starts with a hyphen', async () => {
mockExecuteFunctions.getNodeParameter
.mockReturnValueOnce('tag')
@@ -1846,7 +1899,7 @@ invalid line without proper format`;
});
});
describe('Command config neutralization', () => {
describe('Command config protection', () => {
const expectedOverrides = [
'core.sshCommand=ssh',
'core.fsmonitor=false',
@@ -1883,6 +1936,73 @@ invalid line without proper format`;
}
});
it.each([
'filter.poc.clean',
'merge.poc.driver',
'core.askPass',
'core.editor',
'core.alternateRefsCommand',
'gc.recentObjectsHook',
'hook.pre-commit.command',
'sequence.editor',
'remote.origin.uploadpack',
'remote.origin.receivepack',
'gpg.openpgp.program',
'gpg.ssh.defaultKeyCommand',
])("rejects repository Git config key '%s'", async (configKey) => {
mockExecuteFunctions.getNodeParameter
.mockReturnValueOnce('add')
.mockReturnValueOnce('/repo')
.mockReturnValueOnce({})
.mockReturnValueOnce('file.txt');
mockGit.listConfig.mockResolvedValueOnce({
files: ['global-config', '.git/config', 'command line:'],
values: {
'global-config': { 'filter.lfs.process': 'git-lfs filter-process' },
'.git/config': { [configKey]: 'command' },
'command line:': {},
},
} as any);
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
`Repository Git config key '${configKey}' is not allowed`,
);
expect(mockGit.add).not.toHaveBeenCalled();
});
it.each([
{ operation: 'add', params: ['add', '/repo', {}, 'file.txt'], guard: 'add' },
{ operation: 'commit', params: ['commit', '/repo', {}, 'repro commit'], guard: 'commit' },
{ operation: 'fetch', params: ['fetch', '/repo', {}], guard: 'fetch' },
{ operation: 'pull', params: ['pull', '/repo', {}], guard: 'pull' },
{ operation: 'push', params: ['push', '/repo', {}], guard: 'push' },
{ operation: 'pushTags', params: ['pushTags', '/repo', {}], guard: 'pushTags' },
{ operation: 'status', params: ['status', '/repo', {}], guard: 'status' },
{
operation: 'switchBranch',
params: ['switchBranch', '/repo', {}, 'main'],
guard: 'checkout',
},
{ operation: 'tag', params: ['tag', '/repo', {}, 'v1.0.0'], guard: 'addTag' },
])('rejects repository Git config key before $operation', async ({ params, guard }) => {
for (const value of params) {
mockExecuteFunctions.getNodeParameter.mockReturnValueOnce(value);
}
mockGit.listConfig.mockResolvedValueOnce({
files: ['global-config', '.git/config', 'command line:'],
values: {
'global-config': { 'filter.lfs.process': 'git-lfs filter-process' },
'.git/config': { 'filter.poc.clean': 'command' },
'command line:': {},
},
} as any);
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
"Repository Git config key 'filter.poc.clean' is not allowed",
);
expect((mockGit as any)[guard]).not.toHaveBeenCalled();
});
it('does not pin command config when enableGitNodeAllConfigKeys is true', async () => {
securityConfig.enableGitNodeAllConfigKeys = true;
@@ -1895,6 +2015,7 @@ invalid line without proper format`;
for (const flag of expectedFlags) {
expect(unsafe?.[flag]).toBeUndefined();
}
expect(mockGit.listConfig).not.toHaveBeenCalled();
});
});
});
@@ -623,10 +623,32 @@ export const documentFields: INodeProperties[] = [
operation: ['query'],
},
},
description: 'JSON query to execute',
description:
'JSON query to execute. Use $1, $2, and so on as complete values to reference Query Parameters below.',
hint: 'Use query parameters for dynamic values instead of embedding expressions in the query',
required: true,
placeholder:
'{"structuredQuery": {"where": {"fieldFilter": {"field": {"fieldPath": "age"},"op": "EQUAL", "value": {"integerValue": 28}}}, "from": [{"collectionId": "users-collection"}]}}',
'{"structuredQuery": {"where": {"fieldFilter": {"field": {"fieldPath": "age"},"op": "EQUAL", "value": {"integerValue": "$1"}}}, "from": [{"collectionId": "users-collection"}]}}',
},
{
displayName: 'Query Parameters',
name: 'queryParameters',
type: 'json',
typeOptions: {
rows: 2,
},
default: '={{ [] }}',
displayOptions: {
show: {
resource: ['document'],
operation: ['query'],
},
},
description:
'Array of values to use for $1, $2, and so on, in order. Values can be strings, numbers, booleans, null, or arrays of these values.',
hint: 'Build the array inside the expression, for example {{ [$json.name, 30] }} sets $1 to the name and $2 to 30',
placeholder: '{{ [$json.name, 30] }}',
validateType: 'array',
},
{
displayName: 'Simplify',
@@ -7,7 +7,9 @@ import type {
INodeType,
INodeTypeDescription,
} from 'n8n-workflow';
import { NodeConnectionTypes, jsonParse } from 'n8n-workflow';
import { NodeConnectionTypes } from 'n8n-workflow';
import { parseAndResolveQueryParameters } from '@utils/query-parameters';
import { collectionFields, collectionOperations } from './CollectionDescription';
import { documentFields, documentOperations } from './DocumentDescription';
@@ -363,11 +365,18 @@ export class GoogleFirebaseCloudFirestore implements INodeType {
await Promise.all(
items.map(async (_: IDataObject, i: number) => {
const query = this.getNodeParameter('query', i) as string;
const queryParameters = this.getNodeParameter('queryParameters', i, '[]');
const body = parseAndResolveQueryParameters(
query,
queryParameters,
this.getNode(),
i,
) as IDataObject;
responseData = await googleApiRequest.call(
this,
'POST',
`/${projectId}/databases/${database}/documents:runQuery`,
jsonParse(query),
body,
);
responseData = responseData.map(
@@ -0,0 +1,65 @@
{
"name": "CloudFirestore Document Query Parameters Test",
"nodes": [
{
"parameters": {},
"type": "n8n-nodes-base.manualTrigger",
"typeVersion": 1,
"position": [0, 0],
"id": "trigger-id",
"name": "When clicking 'Execute Workflow'"
},
{
"parameters": {
"authentication": "googleFirebaseCloudFirestoreOAuth2Api",
"resource": "document",
"operation": "query",
"projectId": "test-project",
"database": "(default)",
"query": "{\"structuredQuery\": {\"where\": {\"fieldFilter\": {\"field\": {\"fieldPath\": \"name\"}, \"op\": \"EQUAL\", \"value\": {\"stringValue\": \"$1\"}}}, \"from\": [{\"collectionId\": \"users\"}]}}",
"queryParameters": "={{ ['hello \"there\", \"op\": \"GREATER_THAN'] }}"
},
"type": "n8n-nodes-base.googleFirebaseCloudFirestore",
"typeVersion": 1.1,
"position": [200, 0],
"id": "cloudFirestore-query",
"name": "Query Documents",
"credentials": {
"googleFirebaseCloudFirestoreOAuth2Api": {
"id": "cloudFirestore-oauth-id",
"name": "CloudFirestore OAuth2"
}
}
}
],
"pinData": {
"Query Documents": [
{
"json": {
"_name": "projects/test-project/databases/(default)/documents/users/user1",
"_id": "user1",
"_createTime": "2023-01-01T10:00:00.000Z",
"_updateTime": "2023-01-01T10:00:00.000Z",
"name": "John Doe"
}
}
]
},
"connections": {
"When clicking 'Execute Workflow'": {
"main": [
[
{
"node": "Query Documents",
"type": "main",
"index": 0
}
]
]
}
},
"active": false,
"settings": {
"executionOrder": "v1"
}
}
@@ -0,0 +1,48 @@
import { NodeTestHarness } from '@nodes-testing/node-test-harness';
import nock from 'nock';
describe('GoogleFirebaseCloudFirestore', () => {
const credentials = {
googleFirebaseCloudFirestoreOAuth2Api: {
oauthTokenData: {
access_token: 'test-access-token',
},
},
};
beforeAll(() => {
// The interceptor only matches if the expression-supplied value lands as a single
// plain string value, quotes and all, leaving the query structure untouched.
nock('https://firestore.googleapis.com')
.post('/v1/projects/test-project/databases/(default)/documents:runQuery', {
structuredQuery: {
where: {
fieldFilter: {
field: { fieldPath: 'name' },
op: 'EQUAL',
value: { stringValue: 'hello "there", "op": "GREATER_THAN' },
},
},
from: [{ collectionId: 'users' }],
},
})
.reply(200, [
{
document: {
name: 'projects/test-project/databases/(default)/documents/users/user1',
id: 'user1',
fields: {
name: { stringValue: 'John Doe' },
},
createTime: '2023-01-01T10:00:00.000Z',
updateTime: '2023-01-01T10:00:00.000Z',
},
},
]);
});
new NodeTestHarness().setupTests({
credentials,
workflowFiles: ['document-query-parameters.workflow.json'],
});
});
@@ -240,6 +240,9 @@ export async function encodeEmail(email: IEmail) {
mailOptions.attachments = attachments;
}
mailOptions.disableFileAccess = true;
mailOptions.disableUrlAccess = true;
const mail = new MailComposer(mailOptions).compile();
// by default the bcc headers are deleted when the mail is built.
@@ -0,0 +1,40 @@
import { encodeEmail } from '../GenericFunctions';
const { mailComposerOptions } = vi.hoisted(() => ({
mailComposerOptions: vi.fn(),
}));
vi.mock('nodemailer/lib/mail-composer', () => ({
default: class MailComposer {
constructor(options: unknown) {
mailComposerOptions(options);
}
compile() {
return {
keepBcc: false,
build: vi.fn().mockResolvedValue(Buffer.from('email')),
};
}
},
}));
describe('Gmail email encoding', () => {
beforeEach(() => {
vi.clearAllMocks();
});
it('applies content access restrictions to generated emails', async () => {
await encodeEmail({
subject: 'Test subject',
body: 'Test body',
});
expect(mailComposerOptions).toHaveBeenCalledWith(
expect.objectContaining({
disableFileAccess: true,
disableUrlAccess: true,
}),
);
});
});
@@ -1,11 +1,9 @@
import type { INode, IExecuteFunctions } from 'n8n-workflow';
import { Binary, ObjectId } from 'mongodb';
import {
parseAndResolveQueryParameters,
prepareItems,
serializeMongoItems,
} from './GenericFunctions';
import { parseAndResolveQueryParameters } from '@utils/query-parameters';
import { prepareItems, serializeMongoItems } from './GenericFunctions';
const mockNode = { name: 'MongoDB', type: 'n8n-nodes-base.mongoDb' } as INode;
@@ -2,7 +2,7 @@ import { formatPemBlock } from '@n8n/utils/format-pem-block';
import get from 'lodash/get';
import set from 'lodash/set';
import { Binary, MongoClient, ObjectId } from 'mongodb';
import { jsonParse, NodeOperationError } from 'n8n-workflow';
import { NodeOperationError } from 'n8n-workflow';
import type {
ICredentialDataDecryptedObject,
IDataObject,
@@ -13,6 +13,7 @@ import type {
import { createSecureContext } from 'tls';
import { routeBinaryProperties } from '@utils/binary';
import { isScalarValue } from '@utils/query-parameters';
import type {
IMongoCredentials,
@@ -83,116 +84,6 @@ export function validateAndResolveMongoCredentials(
}
}
type MongoQueryParameterScalar = string | number | boolean | bigint | Date | null;
type MongoQueryParameter = MongoQueryParameterScalar | MongoQueryParameterScalar[];
function isScalarValue(value: unknown): value is MongoQueryParameterScalar {
return (
value === null ||
typeof value === 'string' ||
typeof value === 'number' ||
typeof value === 'boolean' ||
typeof value === 'bigint' ||
value instanceof Date
);
}
function parseQueryParameters(
rawParameters: unknown,
node: INode,
itemIndex: number,
): MongoQueryParameter[] {
let parameters: unknown = rawParameters;
if (typeof parameters === 'string') {
try {
parameters = JSON.parse(parameters) as unknown;
} catch (error) {
throw new NodeOperationError(node, error as Error, {
itemIndex,
message: 'Query Parameters must be valid JSON',
description: 'Enter the parameters as a JSON array',
});
}
}
if (!Array.isArray(parameters)) {
throw new NodeOperationError(node, 'Query Parameters must be a JSON array', {
itemIndex,
description: 'Enter the parameters as a JSON array',
});
}
return parameters.map((parameter, index) => {
if (isScalarValue(parameter) || (Array.isArray(parameter) && parameter.every(isScalarValue))) {
return parameter;
}
throw new NodeOperationError(
node,
`Query parameter ${index + 1} must be a scalar or an array of scalars`,
{
itemIndex,
description: 'Objects and nested arrays are not supported',
},
);
});
}
export function parseAndResolveQueryParameters(
query: string,
rawParameters: unknown,
node: INode,
itemIndex: number,
): unknown {
const parsedQuery = jsonParse<unknown>(query);
const parameters = parseQueryParameters(rawParameters, node, itemIndex);
if (parameters.length === 0) return parsedQuery;
const usedParameters = new Set<number>();
const resolveValue = (value: unknown): unknown => {
if (typeof value === 'string') {
const match = /^\$(\d+)$/.exec(value);
if (!match) return value;
const parameterIndex = Number(match[1]) - 1;
if (parameterIndex < 0 || parameterIndex >= parameters.length) {
throw new NodeOperationError(node, `Query placeholder ${value} has no matching value`, {
itemIndex,
description: `Add a value for ${value} to Query Parameters`,
});
}
usedParameters.add(parameterIndex);
return parameters[parameterIndex];
}
if (Array.isArray(value)) return value.map(resolveValue);
if (value !== null && typeof value === 'object') {
return Object.fromEntries(
Object.entries(value).map(([key, entry]) => [key, resolveValue(entry)]),
);
}
return value;
};
const resolvedQuery = resolveValue(parsedQuery);
const unusedParameter = parameters.findIndex((_, index) => !usedParameters.has(index));
if (unusedParameter !== -1) {
throw new NodeOperationError(node, `Query parameter ${unusedParameter + 1} is not used`, {
itemIndex,
description: `Add $${unusedParameter + 1} to the query or remove the unused parameter`,
});
}
return resolvedQuery;
}
function describeUpdateKeyValueType(value: unknown): string {
if (value === null) return 'null';
if (Array.isArray(value)) return 'array';
@@ -19,10 +19,11 @@ import type {
IPairedItemData,
} from 'n8n-workflow';
import { parseAndResolveQueryParameters } from '@utils/query-parameters';
import {
buildParameterizedConnString,
connectMongoClient,
parseAndResolveQueryParameters,
prepareFields,
prepareItems,
serializeMongoItems,
@@ -0,0 +1,94 @@
import type { INode } from 'n8n-workflow';
import { parseAndResolveQueryParameters } from '../query-parameters';
const mockNode = { name: 'Test Node', type: 'n8n-nodes-base.test' } as INode;
describe('parseAndResolveQueryParameters', () => {
it('replaces placeholders with scalars and scalar arrays', () => {
const query = JSON.stringify({
name: '$1',
age: { gte: '$2' },
tags: { in: '$3' },
});
const result = parseAndResolveQueryParameters(
query,
'["Alice", 30, ["active", "admin"]]',
mockNode,
0,
);
expect(result).toEqual({
name: 'Alice',
age: { gte: 30 },
tags: { in: ['active', 'admin'] },
});
});
it('only replaces complete values, not keys or parts of strings', () => {
const query = JSON.stringify({ $1: 'key', exact: '$1', partial: 'user-$1' });
const result = parseAndResolveQueryParameters(query, ['Alice'], mockNode, 0);
expect(result).toEqual({ $1: 'key', exact: 'Alice', partial: 'user-$1' });
});
it('treats a parameter containing JSON as a plain string value', () => {
const query = JSON.stringify({ term: { user: '$1' } });
const result = parseAndResolveQueryParameters(
query,
['zzz", "match_all": {}, "boost": "2'],
mockNode,
0,
);
expect(result).toEqual({ term: { user: 'zzz", "match_all": {}, "boost": "2' } });
});
it('does not replace placeholders when parameters are empty', () => {
const result = parseAndResolveQueryParameters('{ "name": "$1" }', [], mockNode, 0);
expect(result).toEqual({ name: '$1' });
});
it('throws when a placeholder has no matching parameter', () => {
expect(() =>
parseAndResolveQueryParameters('{ "a": "$1", "b": "$2" }', ['Alice'], mockNode, 0),
).toThrow('Query placeholder $2 has no matching value');
});
it.each([{ parameters: [{ name: 'Alice' }] }, { parameters: [[['nested']]] }])(
'throws for unsupported parameter value $parameters',
({ parameters }) => {
expect(() =>
parseAndResolveQueryParameters('{ "name": "$1" }', parameters, mockNode, 0),
).toThrow(/must be a scalar or an array of scalars/);
},
);
it('throws when a parameter is not used', () => {
expect(() =>
parseAndResolveQueryParameters('{ "name": "$1" }', ['Alice', 30], mockNode, 0),
).toThrow('Query parameter 2 is not used');
});
it('throws when the parameters are not a JSON array', () => {
expect(() =>
parseAndResolveQueryParameters('{ "name": "$1" }', '{ "name": "Alice" }', mockNode, 0),
).toThrow('Query Parameters must be a JSON array');
});
it('throws when the parameters are not valid JSON', () => {
expect(() => parseAndResolveQueryParameters('{ "name": "$1" }', '[', mockNode, 0)).toThrow(
'Query Parameters must be valid JSON',
);
});
it('throws when the query is not valid JSON', () => {
expect(() => parseAndResolveQueryParameters('{ "name": ', ['Alice'], mockNode, 0)).toThrow(
"Invalid JSON in 'Query'",
);
});
});
@@ -0,0 +1,120 @@
import { jsonParse, NodeOperationError } from 'n8n-workflow';
import type { INode } from 'n8n-workflow';
type QueryParameterScalar = string | number | boolean | bigint | Date | null;
type QueryParameter = QueryParameterScalar | QueryParameterScalar[];
export function isScalarValue(value: unknown): value is QueryParameterScalar {
return (
value === null ||
typeof value === 'string' ||
typeof value === 'number' ||
typeof value === 'boolean' ||
typeof value === 'bigint' ||
value instanceof Date
);
}
function parseQueryParameters(
rawParameters: unknown,
node: INode,
itemIndex: number,
): QueryParameter[] {
let parameters: unknown = rawParameters;
if (typeof parameters === 'string') {
try {
parameters = JSON.parse(parameters) as unknown;
} catch (error) {
throw new NodeOperationError(node, error as Error, {
itemIndex,
message: 'Query Parameters must be valid JSON',
description: 'Enter the parameters as a JSON array',
});
}
}
if (!Array.isArray(parameters)) {
throw new NodeOperationError(node, 'Query Parameters must be a JSON array', {
itemIndex,
description: 'Enter the parameters as a JSON array',
});
}
return parameters.map((parameter, index) => {
if (isScalarValue(parameter) || (Array.isArray(parameter) && parameter.every(isScalarValue))) {
return parameter;
}
throw new NodeOperationError(
node,
`Query parameter ${index + 1} must be a scalar or an array of scalars`,
{
itemIndex,
description: 'Objects and nested arrays are not supported',
},
);
});
}
/**
* Parses a JSON query and substitutes `$1`, `$2`, ... placeholders with the given parameters.
*
* Placeholders are only substituted when they make up a complete string value, so a parameter
* can never contribute structure (keys, operators, extra clauses) to the resulting query.
*/
export function parseAndResolveQueryParameters(
query: string,
rawParameters: unknown,
node: INode,
itemIndex: number,
): unknown {
const parsedQuery = jsonParse<unknown>(query, {
errorMessage: "Invalid JSON in 'Query'",
});
const parameters = parseQueryParameters(rawParameters, node, itemIndex);
if (parameters.length === 0) return parsedQuery;
const usedParameters = new Set<number>();
const resolveValue = (value: unknown): unknown => {
if (typeof value === 'string') {
const match = /^\$(\d+)$/.exec(value);
if (!match) return value;
const parameterIndex = Number(match[1]) - 1;
if (parameterIndex < 0 || parameterIndex >= parameters.length) {
throw new NodeOperationError(node, `Query placeholder ${value} has no matching value`, {
itemIndex,
description: `Add a value for ${value} to Query Parameters`,
});
}
usedParameters.add(parameterIndex);
return parameters[parameterIndex];
}
if (Array.isArray(value)) return value.map(resolveValue);
if (value !== null && typeof value === 'object') {
return Object.fromEntries(
Object.entries(value).map(([key, entry]) => [key, resolveValue(entry)]),
);
}
return value;
};
const resolvedQuery = resolveValue(parsedQuery);
const unusedParameter = parameters.findIndex((_, index) => !usedParameters.has(index));
if (unusedParameter !== -1) {
throw new NodeOperationError(node, `Query parameter ${unusedParameter + 1} is not used`, {
itemIndex,
description: `Add $${unusedParameter + 1} to the query or remove the unused parameter`,
});
}
return resolvedQuery;
}
@@ -12,11 +12,7 @@
*/
import { bench } from 'vitest';
import { ExpressionEvaluator, IsolatedVmBridge } from '@n8n/expression-runtime';
import {
DollarSignValidator,
PrototypeSanitizer,
ThisSanitizer,
} from 'n8n-workflow/expression-sandboxing';
import { expressionSandboxHooks } from 'n8n-workflow/expression-sandboxing';
import { BENCH_OPTIONS } from '../bench-options';
@@ -24,10 +20,7 @@ import { BENCH_OPTIONS } from '../bench-options';
const evaluator = new ExpressionEvaluator({
createBridge: () => new IsolatedVmBridge({ timeout: 5000 }),
maxCodeCacheSize: 1024,
hooks: {
before: [ThisSanitizer],
after: [PrototypeSanitizer, DollarSignValidator],
},
hooks: expressionSandboxHooks,
});
await evaluator.initialize();
const caller = {};
@@ -1,15 +1,17 @@
import { Tournament } from '@n8n/tournament';
import { DollarSignValidator, ThisSanitizer, PrototypeSanitizer } from './expression-sandboxing';
import { expressionSandboxHooks } from './expression-sandboxing';
type Evaluator = (expr: string, data: unknown) => string | null | (() => unknown);
type ErrorHandler = (error: Error) => void;
const errorHandler: ErrorHandler = () => {};
const tournamentEvaluator = new Tournament(errorHandler, undefined, undefined, {
before: [ThisSanitizer],
after: [PrototypeSanitizer, DollarSignValidator],
});
const tournamentEvaluator = new Tournament(
errorHandler,
undefined,
undefined,
expressionSandboxHooks,
);
const evaluator: Evaluator = tournamentEvaluator.execute.bind(tournamentEvaluator);
export const setErrorHandler = (handler: ErrorHandler) => {
+66 -99
View File
@@ -1,4 +1,10 @@
import { type ASTAfterHook, type ASTBeforeHook, astBuilders as b, astVisit } from '@n8n/tournament';
import {
type ASTAfterHook,
type ASTBeforeHook,
type TournamentHooks,
astBuilders as b,
astVisit,
} from '@n8n/tournament';
import {
ExpressionClassExtensionError,
@@ -129,7 +135,51 @@ const isValidDollarPropertyAccess = (expr: unknown): boolean => {
const GLOBAL_IDENTIFIERS = new Set(['globalThis']);
const BLOCKED_SPREAD_GLOBALS = new Set(['process', 'global', 'globalThis', 'Buffer']);
/**
* Backstop, not the containment: the polyfill already resolves a free base class
* identifier through the data context. Listed here because a subclass inherits
* the static side of its base, so these are the costliest to ever let through.
*/
const blockedBaseClasses = new Set([
'Function',
'GeneratorFunction',
'AsyncFunction',
'AsyncGeneratorFunction',
'Buffer',
]);
/**
* Rejects `class X extends <expr>` unless the base class is a plain identifier
* that is not one of {@link blockedBaseClasses}.
*
* Must stay a before hook: the polyfill rewrites a free base class identifier
* into a data-context lookup, after which the two cases are indistinguishable.
*/
export const ClassExtensionValidator: ASTBeforeHook = (ast, _dataNode) => {
const validate = (superClass: unknown) => {
if (isAstNode(superClass)) {
if (superClass.type !== 'Identifier') {
throw new ExpressionError('Cannot use dynamic class extension due to security concerns');
}
if (typeof superClass.name === 'string' && blockedBaseClasses.has(superClass.name)) {
throw new ExpressionClassExtensionError(superClass.name);
}
}
};
astVisit(ast, {
visitClassDeclaration(path) {
this.traverse(path);
validate(path.node.superClass);
},
visitClassExpression(path) {
this.traverse(path);
validate(path.node.superClass);
},
});
};
/**
* Prevents regular functions from binding their `this` to the Node.js global.
@@ -301,58 +351,6 @@ export const DollarSignValidator: ASTAfterHook = (ast, _dataNode) => {
});
};
const blockedBaseClasses = new Set([
'Function',
'GeneratorFunction',
'AsyncFunction',
'AsyncGeneratorFunction',
]);
/**
* Builds an AST node that safely resolves a spread argument like `...process`.
*
* Tournament's VariablePolyfill rewrites plain identifiers (e.g. `process`)
* to look them up from the data context, but it does NOT handle identifiers
* inside SpreadElement / SpreadProperty nodes. Without this fix, `{...process}`
* would resolve to the real Node.js `process` object.
*
* The generated code checks the data context first, falling back to a throw:
*
* ("process" in data) ? data.process : (() => { throw new Error("...") })()
*
* - If the workflow has a variable called "process" spread that (safe, user-defined)
* - Otherwise throw at runtime, blocking access to the real global
*/
const buildSafeSpreadArg = (name: string, dataNode: Parameters<ASTAfterHook>[1]) => {
// "process" in ___n8n_data
const isInDataContext = b.binaryExpression('in', b.literal(name), dataNode);
// ___n8n_data.process
const readFromDataContext = b.memberExpression(dataNode, b.identifier(name));
// (() => { throw new Error('Cannot spread "process" ...') })()
//
// This is an IIFE because `throw` is a statement, not an expression,
// so it cannot appear directly inside a ternary's falsy branch.
const throwSecurityError = b.callExpression(
b.arrowFunctionExpression(
[],
b.blockStatement([
b.throwStatement(
b.newExpression(b.identifier('Error'), [
b.literal(`Cannot spread "${name}" due to security concerns`),
]),
),
]),
),
[],
);
// Full result:
// ("process" in ___n8n_data) ? ___n8n_data.process : (() => { throw ... })()
return b.conditionalExpression(isInDataContext, readFromDataContext, throwSecurityError);
};
export const PrototypeSanitizer: ASTAfterHook = (ast, dataNode) => {
astVisit(ast, {
visitVariableDeclarator(path) {
@@ -392,42 +390,19 @@ export const PrototypeSanitizer: ASTAfterHook = (ast, dataNode) => {
visitClassDeclaration(path) {
this.traverse(path);
const node = path.node;
const className = getReservedIdentifier(node.id);
if (className !== undefined) {
throw new ExpressionReservedVariableError(className);
}
if (node.superClass) {
if (node.superClass.type === 'Identifier') {
if (blockedBaseClasses.has(node.superClass.name)) {
throw new ExpressionClassExtensionError(node.superClass.name);
}
} else {
throw new ExpressionError('Cannot use dynamic class extension due to security concerns');
}
}
const className = getReservedIdentifier(path.node.id);
if (className === undefined) return;
throw new ExpressionReservedVariableError(className);
},
visitClassExpression(path) {
this.traverse(path);
const node = path.node;
const className = getReservedIdentifier(node.id);
const className = getReservedIdentifier(path.node.id);
if (className !== undefined) {
throw new ExpressionReservedVariableError(className);
}
if (node.superClass) {
if (node.superClass.type === 'Identifier') {
if (blockedBaseClasses.has(node.superClass.name)) {
throw new ExpressionClassExtensionError(node.superClass.name);
}
} else {
throw new ExpressionError('Cannot use dynamic class extension due to security concerns');
}
}
},
visitAssignmentExpression(path) {
@@ -530,30 +505,22 @@ export const PrototypeSanitizer: ASTAfterHook = (ast, dataNode) => {
}
},
visitSpreadElement(path) {
this.traverse(path);
const { argument } = path.node;
if (argument.type === 'Identifier' && BLOCKED_SPREAD_GLOBALS.has(argument.name)) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any
(path.node as any).argument = buildSafeSpreadArg(argument.name, dataNode);
}
},
visitSpreadProperty(path) {
this.traverse(path);
const { argument } = path.node;
if (argument.type === 'Identifier' && BLOCKED_SPREAD_GLOBALS.has(argument.name)) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any
(path.node as any).argument = buildSafeSpreadArg(argument.name, dataNode);
}
},
visitWithStatement() {
throw new ExpressionWithStatementError();
},
});
};
/**
* The complete set of AST hooks an expression evaluator must run. Evaluators take
* this object rather than assembling their own, so a hook added here cannot be
* missed by one of them.
*/
export const expressionSandboxHooks: TournamentHooks = {
before: [ClassExtensionValidator, ThisSanitizer],
after: [PrototypeSanitizer, DollarSignValidator],
};
export const sanitizer = (value: unknown): unknown => {
const propertyKey = String(value);
if (!isSafeObjectProperty(propertyKey)) {
+2 -11
View File
@@ -6,13 +6,7 @@ import { UnexpectedError, UserError } from './errors';
import { ExpressionExtensionError } from './errors/expression-extension.error';
import { ExpressionError } from './errors/expression.error';
import { evaluateExpression, setErrorHandler } from './expression-evaluator-proxy';
import {
DollarSignValidator,
PrototypeSanitizer,
ThisSanitizer,
sanitizer,
sanitizerName,
} from './expression-sandboxing';
import { expressionSandboxHooks, sanitizer, sanitizerName } from './expression-sandboxing';
import { isExpression } from './expressions/expression-helpers';
import * as LoggerProxy from './logger-proxy';
import { extend, extendOptional } from './extensions';
@@ -268,10 +262,7 @@ export class Expression {
maxCodeCacheSize: options.maxCodeCacheSize,
poolSize: options.poolSize,
idleTimeoutMs: options.idleTimeoutMs,
hooks: {
before: [ThisSanitizer],
after: [PrototypeSanitizer, DollarSignValidator],
},
hooks: expressionSandboxHooks,
logger: LoggerProxy,
observability: options.observability,
});
+28 -4
View File
@@ -33,7 +33,12 @@ import { createResultError, createResultOk } from '@n8n/utils/result';
import type { IRunExecutionData } from './run-execution-data/run-execution-data';
import { safeRegex } from './safe-regex';
import { isResourceLocatorValue } from './type-guards';
import { containsUnsafeObjectPropertyToken, deepCopy, isObjectEmpty } from './utils';
import {
containsUnsafeObjectPropertyToken,
deepCopy,
isObjectEmpty,
isSafeObjectProperty,
} from './utils';
import type { Workflow } from './workflow';
import type { EnvProviderState } from './workflow-data-proxy-env-provider';
import { createEnvProvider, createEnvProviderState } from './workflow-data-proxy-env-provider';
@@ -61,6 +66,14 @@ type PairedItemMethod = (typeof PAIRED_ITEM_METHOD)[keyof typeof PAIRED_ITEM_MET
* This is a process-wide invariant, so we probe once and cache the result.
*/
let codeGenerationAllowed: boolean | undefined;
// Reads a key from a placeholder source only when it is the source's own
// property, so a lookup can never resolve to a value reached through the
// prototype chain (e.g. `constructor` / `__proto__`).
const readOwnKey = (source: unknown, key: string): unknown => {
if (source === null || typeof source !== 'object') return undefined;
return Object.hasOwn(source, key) ? (source as Record<string, unknown>)[key] : undefined;
};
const isCodeGenerationAllowed = (): boolean => {
if (codeGenerationAllowed === undefined) {
try {
@@ -1116,6 +1129,15 @@ export class WorkflowDataProxy {
},
);
}
// Reserved keys resolve to inherited members (e.g. the object's
// constructor or prototype) rather than a placeholder value, so they
// are never valid placeholder names.
if (!isSafeObjectProperty(name)) {
throw new ExpressionError('Invalid parameter key', {
runIndex,
itemIndex,
});
}
const resultData =
that.runExecutionData?.resultData?.runData?.[that.activeNodeName]?.[runIndex];
@@ -1138,10 +1160,12 @@ export class WorkflowDataProxy {
type: 'no_execution_data',
});
}
// Resolve only own placeholder keys, never values reached through the
// prototype chain of the input data — including the `query` container
// itself, which is read as an own key for the same reason.
return (
// TS does not know that the key exists, we need to address this in refactor
(placeholdersDataInputData?.query as Record<string, unknown>)?.[name] ??
placeholdersDataInputData?.[name] ??
readOwnKey(readOwnKey(placeholdersDataInputData, 'query'), name) ??
readOwnKey(placeholdersDataInputData, name) ??
defaultValue
);
};
@@ -87,4 +87,22 @@ describe('Expression — nested $json shapes (engine parity)', () => {
list: [[1, 2], { k: 'v' }],
});
});
it('spreads $json into an object literal', () => {
expect(evaluate('={{ {...$json} }}', { name: 'alice', age: 30 })).toEqual({
name: 'alice',
age: 30,
});
});
it('overrides a property of a spread $json', () => {
expect(evaluate('={{ {...$json, age: 31} }}', { name: 'alice', age: 30 })).toEqual({
name: 'alice',
age: 31,
});
});
it('spreads a nested array from $json', () => {
expect(evaluate('={{ [...$json.rows] }}', { rows: [1, 2, 3] })).toEqual([1, 2, 3]);
});
});
@@ -10,13 +10,7 @@ import {
ExpressionError,
ExpressionWithStatementError,
} from '../src/errors';
import {
DollarSignValidator,
ThisSanitizer,
PrototypeSanitizer,
sanitizer,
DOLLAR_SIGN_ERROR,
} from '../src/expression-sandboxing';
import { expressionSandboxHooks, sanitizer, DOLLAR_SIGN_ERROR } from '../src/expression-sandboxing';
const tournament = new Tournament(
(e) => {
@@ -24,10 +18,7 @@ const tournament = new Tournament(
},
undefined,
undefined,
{
before: [ThisSanitizer],
after: [PrototypeSanitizer, DollarSignValidator],
},
expressionSandboxHooks,
);
const errorRegex = /^Cannot access ".*" due to security concerns$/;
@@ -477,6 +468,15 @@ describe('PrototypeSanitizer', () => {
}).toThrowError(ExpressionClassExtensionError);
});
it('should not allow class extending Buffer', () => {
expect(() => {
tournament.execute(
'{{ (() => { class Z extends Buffer {} return Z.allocUnsafe(32).length; })() }}',
{ __sanitize: sanitizer },
);
}).toThrow(ExpressionClassExtensionError);
});
it('should allow class extending safe classes', () => {
expect(() => {
tournament.execute(
@@ -647,98 +647,179 @@ describe('PrototypeSanitizer', () => {
});
});
describe('Spread-based global access', () => {
it('should not allow spreading process', () => {
describe('Spread of host globals', () => {
it.each([
'process',
'global',
'globalThis',
'Buffer',
'console',
'Error',
'crypto',
'navigator',
'performance',
'Intl',
'Atomics',
'URL',
'TextEncoder',
'TextDecoder',
'fetch',
'Headers',
'Request',
'Response',
'Blob',
'File',
'FormData',
'AbortController',
'Event',
'EventTarget',
'MessageChannel',
'SharedArrayBuffer',
'ReadableStream',
'WritableStream',
'WeakRef',
'FinalizationRegistry',
'AggregateError',
])('should not expose the host %s through a spread', (name) => {
expect(tournament.execute(`{{ ({...${name}}) }}`, { __sanitize: sanitizer })).toEqual({});
});
it.each([
['nested spread', '{{ ({...({...process})}) }}'],
['spread inside an arrow function', '{{ (() => ({...process}))() }}'],
['spread among other spreads', '{{ ({...{}, ...process}) }}'],
])('should not expose a host global through a %s', (_, expression) => {
expect(tournament.execute(expression, { __sanitize: sanitizer })).toEqual({});
});
it.each([
['process.env', '{{ typeof ({...process}).env }}'],
['process.pid', '{{ typeof ({...process}).pid }}'],
['Buffer.allocUnsafe', '{{ typeof ({...Buffer}).allocUnsafe }}'],
['console.log', '{{ typeof ({...console}).log }}'],
])('should not expose the host %s through a spread', (_, expression) => {
expect(tournament.execute(expression, { __sanitize: sanitizer })).toBe('undefined');
});
it.each([
['an array literal', '{{ [...process] }}'],
['call arguments', '{{ ((a) => a)(...process) }}'],
])('should not iterate the host process in %s', (_, expression) => {
expect(() => tournament.execute(expression, { __sanitize: sanitizer })).toThrow(
/is not iterable/,
);
});
it('should not hand out a reference to a host function', () => {
expect(() => {
tournament.execute('{{ ((g) => g.getBuiltinModule)(({...process})) }}', {
tournament.execute('{{ ({...console}).log.toString() }}', { __sanitize: sanitizer });
}).toThrow(/Cannot read properties of undefined/);
});
it('should not write to a host function', () => {
expect(() => {
tournament.execute('{{ Object.assign(({...console}).log, {written: 1}).name }}', {
__sanitize: sanitizer,
Object,
});
}).toThrowError(/due to security concerns/);
}).toThrow(/Cannot convert undefined or null to object/);
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any
expect((console.log as any).written).toBeUndefined();
});
it('should not allow spreading process in object literal', () => {
expect(() => {
tournament.execute('{{ ({...process}) }}', { __sanitize: sanitizer });
}).toThrowError(/Cannot spread "process" due to security concerns/);
it.each([
['an array literal', '{{ [...arguments][0] }}'],
['an object literal', '{{ ({...arguments}) }}'],
['a function body', '{{ (() => [...arguments][0])() }}'],
])("should not expose the evaluator's own arguments in %s", (_, expression) => {
expect(() => tournament.execute(expression, { __sanitize: sanitizer })).toThrow(errorRegex);
});
it('should not allow spreading process in array', () => {
expect(() => {
tournament.execute('{{ [...process] }}', { __sanitize: sanitizer });
}).toThrowError(/Cannot spread "process" due to security concerns/);
});
it('should not allow spreading global', () => {
expect(() => {
tournament.execute('{{ ({...global}) }}', { __sanitize: sanitizer });
}).toThrowError(/Cannot spread "global" due to security concerns/);
});
it('should not allow spreading Buffer', () => {
expect(() => {
tournament.execute('{{ ({...Buffer}) }}', { __sanitize: sanitizer });
}).toThrowError(/Cannot spread "Buffer" due to security concerns/);
});
it('should not allow the exact RCE PoC payload', () => {
it('should not reach a built-in module through a spread', () => {
expect(() => {
tournament.execute(
"{{ ((g) => g.getBuiltinModule('child_process').execSync('id').toString())({...process}) }}",
{ __sanitize: sanitizer },
);
}).toThrowError(/due to security concerns/);
}).toThrow(errorRegex);
});
it('should not allow spreading process in function call arguments', () => {
expect(() => {
tournament.execute('{{ ((a, b) => a)(...process) }}', { __sanitize: sanitizer });
}).toThrowError(/Cannot spread "process" due to security concerns/);
});
it('should not allow spreading process inside arrow function', () => {
expect(() => {
tournament.execute('{{ (() => ({...process}))() }}', { __sanitize: sanitizer });
}).toThrowError(/Cannot spread "process" due to security concerns/);
});
it('should not allow spreading process in nested spread', () => {
expect(() => {
tournament.execute('{{ ({...({...process})}) }}', { __sanitize: sanitizer });
}).toThrowError(/Cannot spread "process" due to security concerns/);
});
it('should not allow spreading process in template expression', () => {
expect(() => {
// eslint-disable-next-line n8n-local-rules/no-interpolation-in-regular-string
tournament.execute('{{ `${JSON.stringify({...process})}` }}', {
__sanitize: sanitizer,
});
}).toThrow();
});
it('should not allow spreading process among other spreads', () => {
expect(() => {
tournament.execute('{{ ({...{a:1}, ...process}) }}', { __sanitize: sanitizer });
}).toThrowError(/Cannot spread "process" due to security concerns/);
});
it('should resolve spread from data context process, not the real one', () => {
const result = tournament.execute('{{ ({...process}).safe }}', {
it('should not expose the host process through a template expression', () => {
// eslint-disable-next-line n8n-local-rules/no-interpolation-in-regular-string
const result = tournament.execute('{{ `${JSON.stringify({...process})}` }}', {
__sanitize: sanitizer,
process: { safe: true },
JSON,
});
expect(result).toBe(true);
expect(result).toBe('{}');
});
it('should not expose real process.version via spread', () => {
const result = tournament.execute('{{ typeof ({...process}).version }}', {
it('should not expose the host process as a computed key', () => {
const result = tournament.execute('{{ Object.keys({[process]: 1})[0] }}', {
__sanitize: sanitizer,
process: {},
Object,
});
expect(result).toBe('undefined');
});
it('should use data context pid via spread, not real pid', () => {
/**
* `Buffer` is additionally named in `blockedBaseClasses`, so these use a
* global that is not, to show that containment comes from the polyfill
* rather than from that list.
*/
it.each([
['Error', '{{ (() => { class X extends Error {} return X.captureStackTrace; })() }}'],
['Array', '{{ (() => { class X extends Array {} return X.from; })() }}'],
])('should not expose the host %s as a base class', (_, expression) => {
expect(() => tournament.execute(expression, { __sanitize: sanitizer })).toThrow(
/is not a constructor or null/,
);
});
it('should not expose the host process as a switch case', () => {
const result = tournament.execute(
'{{ (() => { switch (1) { case process: return "host"; } return "safe"; })() }}',
{ __sanitize: sanitizer },
);
expect(result).toBe('safe');
});
});
describe('Spread of data context values', () => {
it('should spread an object from the data context', () => {
const result = tournament.execute('{{ ({...$json}).greeting }}', {
__sanitize: sanitizer,
$json: { greeting: 'hello' },
});
expect(result).toBe('hello');
});
it('should merge a spread data context object with additional properties', () => {
const result = tournament.execute('{{ JSON.stringify({...$json, b: 2}) }}', {
__sanitize: sanitizer,
$json: { a: 1 },
JSON,
});
expect(result).toBe('{"a":1,"b":2}');
});
it('should spread an array from the data context', () => {
const result = tournament.execute('{{ [...$arr].length }}', {
__sanitize: sanitizer,
$arr: [1, 2, 3],
});
expect(result).toBe(3);
});
it('should spread a data context value into call arguments', () => {
const result = tournament.execute('{{ Math.max(...$arr) }}', {
__sanitize: sanitizer,
$arr: [1, 5, 3],
});
expect(result).toBe(5);
});
it('should prefer a data context value over the host global of the same name', () => {
const result = tournament.execute('{{ ({...process}).pid }}', {
__sanitize: sanitizer,
process: { pid: -1 },
@@ -746,42 +827,42 @@ describe('PrototypeSanitizer', () => {
expect(result).toBe(-1);
});
it('should use data context when spread is wrapped in arrow function', () => {
const result = tournament.execute('{{ ((g) => g.pid)({...process}) }}', {
__sanitize: sanitizer,
process: { pid: -1 },
});
expect(result).toBe(-1);
it.each([
['const', '{{ (() => { const process = { a: 1 }; return {...process}.a; })() }}'],
['function scope', '{{ (function(){ const Buffer = { a: 1 }; return {...Buffer}.a; })() }}'],
['parameter', '{{ ((process) => ({...process}).a)({ a: 1 }) }}'],
])('should spread a local variable declared in %s scope', (_, expression) => {
expect(tournament.execute(expression, { __sanitize: sanitizer })).toBe(1);
});
it('should not give access to real process.exit via spread', () => {
const result = tournament.execute('{{ typeof ({...process}).exit }}', {
it('should resolve a computed key from the data context', () => {
const result = tournament.execute('{{ ({[$key]: 1}).dynamic }}', {
__sanitize: sanitizer,
process: {},
$key: 'dynamic',
});
expect(result).not.toBe('function');
expect(result).toBe(1);
});
it('should not give access to real process.env via spread', () => {
const result = tournament.execute('{{ typeof ({...process}).env }}', {
__sanitize: sanitizer,
process: {},
});
expect(result).not.toBe('object');
});
it('should not give access to getBuiltinModule via spread', () => {
let result: unknown;
try {
result = tournament.execute('{{ typeof ({...process}).getBuiltinModule }}', {
__sanitize: sanitizer,
process: {},
});
} catch {
// Blocked by PrototypeSanitizer — also a valid outcome
return;
it('should resolve a base class from the data context', () => {
class Base {
greet() {
return 'hello';
}
}
expect(result).not.toBe('function');
const result = tournament.execute(
'{{ (() => { class X extends Base {} return new X().greet(); })() }}',
{ __sanitize: sanitizer, Base },
);
expect(result).toBe('hello');
});
it('should resolve a switch case from the data context', () => {
const result = tournament.execute(
'{{ (() => { switch ($key) { case $key: return "matched"; } return "unmatched"; })() }}',
{ __sanitize: sanitizer, $key: 'a' },
);
expect(result).toBe('matched');
});
});
@@ -1052,6 +1052,84 @@ describe('WorkflowDataProxy', () => {
expect(() => proxy.$fromAI('some_key')).toThrow(ExpressionError);
});
describe('key resolution is limited to own placeholder keys', () => {
const buildProxy = (json: unknown) => {
const workflow = new Workflow({
id: '123',
name: 'test workflow',
nodes: [
{
id: 'aiNode',
name: 'AI Node',
type: 'n8n-nodes-base.aiAgent',
typeVersion: 1,
position: [0, 0],
parameters: {},
},
],
connections: {},
active: false,
nodeTypes: Helpers.NodeTypes(),
});
const connectionInputData = [{ json: json as IDataObject, pairedItem: { item: 0 } }];
const dataProxy = new WorkflowDataProxy(
workflow,
null,
0,
0,
'AI Node',
connectionInputData,
{},
'manual',
{},
undefined,
);
return dataProxy.getDataProxy();
};
test('rejects reserved object keys (__proto__, constructor, prototype)', () => {
const proxy = buildProxy({ safe: 'ok' });
expect(proxy.$fromAI('safe')).toBe('ok');
expect(() => proxy.$fromAI('__proto__')).toThrow(ExpressionError);
expect(() => proxy.$fromAI('constructor')).toThrow(ExpressionError);
expect(() => proxy.$fromAI('prototype')).toThrow(ExpressionError);
});
test('ignores inherited (non-own) keys and returns the default', () => {
const proxy = buildProxy({ safe: 'ok' });
expect(proxy.$fromAI('toString', '', 'string', 'fallback')).toBe('fallback');
expect(proxy.$fromAI('hasOwnProperty', '', 'string', 'fallback')).toBe('fallback');
expect(proxy.$fromAI('valueOf', '', 'string', 'fallback')).toBe('fallback');
});
test('rejects a reserved key even when present as an own key from parsed JSON', () => {
const json = JSON.parse('{"__proto__": {"polluted": true}, "safe": "ok"}');
const proxy = buildProxy(json);
expect(() => proxy.$fromAI('__proto__')).toThrow(ExpressionError);
expect(proxy.$fromAI('safe')).toBe('ok');
});
test('returns the default when input json is a bare primitive', () => {
const proxy = buildProxy(1);
expect(() => proxy.$fromAI('constructor')).toThrow(ExpressionError);
expect(() => proxy.$fromAI('__proto__')).toThrow(ExpressionError);
expect(proxy.$fromAI('any_key', '', 'string', 'fallback')).toBe('fallback');
});
test('resolves a placeholder from an own query container', () => {
const proxy = buildProxy({ query: { full_name: 'Alice' } });
expect(proxy.$fromAI('full_name')).toBe('Alice');
});
test('does not read the query container through the prototype chain', () => {
const json = Object.create({ query: { evil: 'from-prototype' } });
json.safe = 'ok';
const proxy = buildProxy(json);
expect(proxy.$fromAI('safe')).toBe('ok');
expect(proxy.$fromAI('evil', '', 'string', 'fallback')).toBe('fallback');
});
});
});
describe('$tool', () => {
+6 -1
View File
@@ -35,7 +35,12 @@ const config = {
};
// Define backend patches to keep during deployment
const PATCHES_TO_KEEP = ['pdfjs-dist', 'pkce-challenge', 'bull'];
const PATCHES_TO_KEEP = [
'pdfjs-dist',
'pkce-challenge',
'bull',
'lodash'
];
// #endregion ===== Configuration =====