mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-18 05:59:42 +02:00
Compare commits
8 Commits
master
...
n8n@2.34.2
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
970da944b9 | ||
|
|
38b6dca1b5 | ||
|
|
ed4298acfd | ||
|
|
e2279d07cd | ||
|
|
f2a28826ab | ||
|
|
170cfc7870 | ||
|
|
a93a2c8fb0 | ||
|
|
9eb299b857 |
18
CHANGELOG.md
18
CHANGELOG.md
|
|
@ -1,3 +1,21 @@
|
|||
## [2.34.2](https://github.com/n8n-io/n8n/compare/n8n@2.34.1...n8n@2.34.2) (2026-08-06)
|
||||
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* **editor:** Focus Markdown editor input before toolbar ([#35605](https://github.com/n8n-io/n8n/issues/35605)) ([ed4298a](https://github.com/n8n-io/n8n/commit/ed4298acfd7734f74df6183f0aa735feeaea48bc))
|
||||
* **editor:** Show agent tool credentials above configuration ([#35612](https://github.com/n8n-io/n8n/issues/35612)) ([38b6dca](https://github.com/n8n-io/n8n/commit/38b6dca1b5dd5ac3cee5ce3f9b01de0f8f126012))
|
||||
|
||||
|
||||
## [2.34.1](https://github.com/n8n-io/n8n/compare/n8n@2.34.0...n8n@2.34.1) (2026-08-05)
|
||||
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* **core:** Recover unresponsive task runners ([#35527](https://github.com/n8n-io/n8n/issues/35527)) ([a93a2c8](https://github.com/n8n-io/n8n/commit/a93a2c8fb017c187c6bd34ad847ef751933f5349))
|
||||
* **editor:** Fix janky spacing between agent capability chips with long labels ([#35571](https://github.com/n8n-io/n8n/issues/35571)) ([f2a2882](https://github.com/n8n-io/n8n/commit/f2a28826ab3c6b01568f812888e56d4d33dfa48c))
|
||||
|
||||
|
||||
# [2.34.0](https://github.com/n8n-io/n8n/compare/n8n@2.33.0...n8n@2.34.0) (2026-08-04)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "n8n-monorepo",
|
||||
"version": "2.34.0",
|
||||
"version": "2.34.2",
|
||||
"private": true,
|
||||
"engines": {
|
||||
"node": ">=22.22",
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/agents",
|
||||
"version": "0.19.0",
|
||||
"version": "0.19.1",
|
||||
"description": "AI agent SDK for n8n's code-first execution engine",
|
||||
"engines": {
|
||||
"node": ">=22.22"
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/ai-node-sdk",
|
||||
"version": "0.24.0",
|
||||
"version": "0.24.1",
|
||||
"description": "SDK for building AI nodes in n8n",
|
||||
"types": "dist/esm/index.d.ts",
|
||||
"module": "dist/esm/index.js",
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/ai-utilities",
|
||||
"version": "0.27.0",
|
||||
"version": "0.27.1",
|
||||
"description": "Utilities for building AI nodes in n8n",
|
||||
"types": "dist/esm/index.d.ts",
|
||||
"module": "dist/esm/index.js",
|
||||
|
|
|
|||
|
|
@ -172,6 +172,24 @@ describe('searxngSearch', () => {
|
|||
expect(url).not.toContain('http://searxng:8080//search');
|
||||
});
|
||||
|
||||
it('uses an injected fetch implementation instead of the global one', async () => {
|
||||
const injectedFetch = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => MOCK_SEARXNG_RESPONSE,
|
||||
});
|
||||
|
||||
const result = await searxngSearch(
|
||||
'http://searxng:8080',
|
||||
'stripe webhooks',
|
||||
{},
|
||||
injectedFetch as unknown as typeof fetch,
|
||||
);
|
||||
|
||||
expect(injectedFetch).toHaveBeenCalledTimes(1);
|
||||
expect(mockFetch).not.toHaveBeenCalled();
|
||||
expect(result.results).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('defaults to 5 results when maxResults is not specified', async () => {
|
||||
const manyResults = {
|
||||
results: Array.from({ length: 10 }, (_, i) => ({
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ export async function searxngSearch(
|
|||
baseUrl: string,
|
||||
query: string,
|
||||
options: WebSearchOptions,
|
||||
fetchImpl: typeof globalThis.fetch = globalThis.fetch,
|
||||
): Promise<WebSearchResponse> {
|
||||
let searchQuery = query;
|
||||
|
||||
|
|
@ -43,7 +44,7 @@ export async function searxngSearch(
|
|||
pageno: '1',
|
||||
});
|
||||
|
||||
const response = await fetch(`${normalizedUrl}/search?${params}`, {
|
||||
const response = await fetchImpl(`${normalizedUrl}/search?${params}`, {
|
||||
headers: {
|
||||
Accept: 'application/json',
|
||||
},
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/ai-workflow-builder",
|
||||
"version": "1.34.0",
|
||||
"version": "1.34.1",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
"typecheck": "tsc --noEmit",
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/api-types",
|
||||
"version": "1.34.0",
|
||||
"version": "1.34.1",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
"dev": "pnpm watch",
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ describe('UpdateOidcConfigurationDto', () => {
|
|||
prompt: 'consent',
|
||||
authenticationContextClassReference: ['mfa'],
|
||||
additionalScopes: 'groups',
|
||||
emailVerifiedRequired: true,
|
||||
rpInitiatedLogoutEnabled: true,
|
||||
});
|
||||
expect(result.success).toBe(true);
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ export class OidcConfigDto extends Z.class({
|
|||
prompt: z.enum(OIDC_PROMPT_VALUES).optional().default('select_account'),
|
||||
authenticationContextClassReference: z.array(z.string()).default([]),
|
||||
additionalScopes: z.string().default(''),
|
||||
emailVerifiedRequired: z.boolean().optional(),
|
||||
rpInitiatedLogoutEnabled: z.boolean().optional().default(false),
|
||||
}) {}
|
||||
|
||||
|
|
@ -25,5 +26,6 @@ export class UpdateOidcConfigurationDto extends OidcConfigDto.extend({
|
|||
prompt: z.enum(OIDC_PROMPT_VALUES),
|
||||
authenticationContextClassReference: z.array(z.string()),
|
||||
additionalScopes: z.string(),
|
||||
emailVerifiedRequired: z.boolean(),
|
||||
rpInitiatedLogoutEnabled: z.boolean(),
|
||||
}) {}
|
||||
|
|
|
|||
|
|
@ -1,11 +1,14 @@
|
|||
import { z } from 'zod';
|
||||
import { assignableGlobalRoleSchema } from '@n8n/permissions';
|
||||
|
||||
import { Z } from '../../zod-class';
|
||||
|
||||
/**
|
||||
* Query DTO for deleting a role. When the role still has users assigned,
|
||||
* `reassignRoleSlug` selects the role those users are moved to before deletion.
|
||||
*
|
||||
* Reassignment is only supported for global roles, and the target must be an
|
||||
* assignable global role — the owner role is never a valid target.
|
||||
*/
|
||||
export class RoleDeleteQueryDto extends Z.class({
|
||||
reassignRoleSlug: z.string().min(1).optional(),
|
||||
reassignRoleSlug: assignableGlobalRoleSchema.optional(),
|
||||
}) {}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/backend-common",
|
||||
"version": "1.34.0",
|
||||
"version": "1.34.1",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
"dev": "pnpm watch",
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/backend-network",
|
||||
"version": "1.8.0",
|
||||
"version": "1.8.1",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
"dev": "pnpm watch",
|
||||
|
|
|
|||
|
|
@ -210,9 +210,9 @@ describe('OutboundHttp.requests requestLegacy', () => {
|
|||
|
||||
describe('SSRF policy', () => {
|
||||
const baseUrl = 'https://example.test';
|
||||
const otherUrl = 'https://other.test';
|
||||
|
||||
it('validates the URL through the provided bridge', async () => {
|
||||
nock(baseUrl).get('/ok').reply(200, 'ok');
|
||||
function makeBridge() {
|
||||
const validateUrl = vi.fn().mockResolvedValue({ ok: true, result: undefined });
|
||||
const bridge = {
|
||||
validateUrl,
|
||||
|
|
@ -220,12 +220,57 @@ describe('OutboundHttp.requests requestLegacy', () => {
|
|||
validateRedirectSync: vi.fn(),
|
||||
createSecureLookup: vi.fn().mockReturnValue(vi.fn()),
|
||||
} as unknown as SsrfBridge;
|
||||
return { bridge, validateUrl };
|
||||
}
|
||||
|
||||
it('validates the URL through the provided bridge', async () => {
|
||||
nock(baseUrl).get('/ok').reply(200, 'ok');
|
||||
const { bridge, validateUrl } = makeBridge();
|
||||
const client = makeFacade().requests({ ssrf: bridge });
|
||||
|
||||
await client.requestLegacy({ baseURL: baseUrl, url: '/ok' });
|
||||
|
||||
expect(validateUrl).toHaveBeenCalledWith(new URL(`${baseUrl}/ok`));
|
||||
});
|
||||
|
||||
it('validates the url the request is sent to, not the uri alias', async () => {
|
||||
const scope = nock(otherUrl).get('/target').reply(200, 'ok');
|
||||
const { bridge, validateUrl } = makeBridge();
|
||||
const client = makeFacade().requests({ ssrf: bridge });
|
||||
|
||||
await client.requestLegacy({ uri: `${baseUrl}/ok`, url: `${otherUrl}/target` });
|
||||
|
||||
expect(validateUrl).toHaveBeenCalledTimes(1);
|
||||
expect(validateUrl).toHaveBeenCalledWith(new URL(`${otherUrl}/target`));
|
||||
expect(scope.isDone()).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects a blocked url even when the uri alias is allowed', async () => {
|
||||
const scope = nock(otherUrl).get('/target').reply(200, 'ok');
|
||||
const { bridge, validateUrl } = makeBridge();
|
||||
validateUrl.mockImplementation((url: URL) =>
|
||||
url.href === `${otherUrl}/target`
|
||||
? { ok: false, error: new Error('Blocked') }
|
||||
: { ok: true, result: undefined },
|
||||
);
|
||||
const client = makeFacade().requests({ ssrf: bridge });
|
||||
|
||||
await expect(
|
||||
client.requestLegacy({ uri: `${baseUrl}/ok`, url: `${otherUrl}/target` }),
|
||||
).rejects.toThrow('Blocked');
|
||||
expect(scope.isDone()).toBe(false);
|
||||
});
|
||||
|
||||
it('validates a baseURL-only target', async () => {
|
||||
const scope = nock(baseUrl).get('/only').reply(200, 'ok');
|
||||
const { bridge, validateUrl } = makeBridge();
|
||||
const client = makeFacade().requests({ ssrf: bridge });
|
||||
|
||||
await client.requestLegacy({ baseURL: `${baseUrl}/only` });
|
||||
|
||||
expect(validateUrl).toHaveBeenCalledWith(new URL(`${baseUrl}/only`));
|
||||
expect(scope.isDone()).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('domain allowlist', () => {
|
||||
|
|
|
|||
|
|
@ -33,6 +33,25 @@ describe('buildAxiosConfigFromLegacyRequest', () => {
|
|||
);
|
||||
});
|
||||
|
||||
test('should target the uri when no url is set', async () => {
|
||||
const axiosOptions = await buildAxiosConfigFromLegacyRequest({
|
||||
uri: 'https://example.com/from-uri',
|
||||
method: 'GET',
|
||||
});
|
||||
|
||||
expect(axiosOptions.url).toBe('https://example.com/from-uri');
|
||||
});
|
||||
|
||||
test('should target the url when both url and uri are set', async () => {
|
||||
const axiosOptions = await buildAxiosConfigFromLegacyRequest({
|
||||
uri: 'https://uri.example.com/path',
|
||||
url: 'https://url.example.com/path',
|
||||
method: 'GET',
|
||||
});
|
||||
|
||||
expect(axiosOptions.url).toBe('https://url.example.com/path');
|
||||
});
|
||||
|
||||
test('should set default User-Agent when none provided', async () => {
|
||||
const axiosOptions = await buildAxiosConfigFromLegacyRequest({
|
||||
url: 'https://example.com',
|
||||
|
|
|
|||
|
|
@ -19,6 +19,8 @@ import {
|
|||
isIgnoreStatusErrorConfig,
|
||||
isProxyPotentiallyActive,
|
||||
isRedirectStatus,
|
||||
resolveLegacyRequestTarget,
|
||||
resolveLegacyRequestUrl,
|
||||
searchForHeader,
|
||||
setAxiosAgents,
|
||||
tryParseUrl,
|
||||
|
|
@ -438,11 +440,16 @@ describe('buildTargetUrl', () => {
|
|||
expect(buildTargetUrl('/path', 'https://example.com')).toBe('https://example.com/path');
|
||||
});
|
||||
|
||||
it('should return undefined for falsy url', () => {
|
||||
it('should return undefined for falsy url without a baseURL', () => {
|
||||
expect(buildTargetUrl(undefined)).toBeUndefined();
|
||||
expect(buildTargetUrl('')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should fall back to baseURL for a falsy url', () => {
|
||||
expect(buildTargetUrl(undefined, 'https://example.com/path')).toBe('https://example.com/path');
|
||||
expect(buildTargetUrl('', 'https://example.com/path')).toBe('https://example.com/path');
|
||||
});
|
||||
|
||||
it('should return undefined for invalid URL combination', () => {
|
||||
expect(buildTargetUrl('not valid', 'also not valid')).toBeUndefined();
|
||||
});
|
||||
|
|
@ -454,6 +461,56 @@ describe('buildTargetUrl', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('resolveLegacyRequestTarget', () => {
|
||||
it('should return the url', () => {
|
||||
expect(resolveLegacyRequestTarget({ url: 'https://example.com/path' })).toBe(
|
||||
'https://example.com/path',
|
||||
);
|
||||
});
|
||||
|
||||
it('should return the uri when no url is set', () => {
|
||||
expect(resolveLegacyRequestTarget({ uri: 'https://example.com/path' })).toBe(
|
||||
'https://example.com/path',
|
||||
);
|
||||
});
|
||||
|
||||
it('should prefer url over uri when both are set', () => {
|
||||
expect(
|
||||
resolveLegacyRequestTarget({
|
||||
uri: 'https://uri.example.com/path',
|
||||
url: 'https://url.example.com/path',
|
||||
}),
|
||||
).toBe('https://url.example.com/path');
|
||||
});
|
||||
|
||||
it('should return undefined when neither url nor uri is set', () => {
|
||||
expect(resolveLegacyRequestTarget({})).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveLegacyRequestUrl', () => {
|
||||
it('should prefer url over uri when both are set', () => {
|
||||
expect(
|
||||
resolveLegacyRequestUrl({
|
||||
uri: 'https://uri.example.com/path',
|
||||
url: 'https://url.example.com/path',
|
||||
}),
|
||||
).toBe('https://url.example.com/path');
|
||||
});
|
||||
|
||||
it('should resolve a relative target against the baseURL', () => {
|
||||
expect(resolveLegacyRequestUrl({ url: '/path', baseURL: 'https://example.com' })).toBe(
|
||||
'https://example.com/path',
|
||||
);
|
||||
});
|
||||
|
||||
it('should resolve to the baseURL when no target is set', () => {
|
||||
expect(resolveLegacyRequestUrl({ baseURL: 'https://example.com/path' })).toBe(
|
||||
'https://example.com/path',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('setAxiosAgents', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ import {
|
|||
getBeforeRedirectFn,
|
||||
getHostFromRequestObject,
|
||||
isFormDataInstance,
|
||||
resolveLegacyRequestTarget,
|
||||
searchForHeader,
|
||||
setAxiosAgents,
|
||||
} from './utils';
|
||||
|
|
@ -165,12 +166,9 @@ export async function buildAxiosConfigFromLegacyRequest(
|
|||
}
|
||||
}
|
||||
|
||||
if (requestObject.uri !== undefined) {
|
||||
axiosConfig.url = requestObject.uri?.toString();
|
||||
}
|
||||
|
||||
if (requestObject.url !== undefined) {
|
||||
axiosConfig.url = requestObject.url?.toString();
|
||||
const target = resolveLegacyRequestTarget(requestObject);
|
||||
if (target !== undefined) {
|
||||
axiosConfig.url = target;
|
||||
}
|
||||
|
||||
if (requestObject.baseURL !== undefined) {
|
||||
|
|
|
|||
|
|
@ -345,11 +345,17 @@ export function getUrlFromProxyConfig(
|
|||
}
|
||||
}
|
||||
|
||||
/** Resolves `url` against an optional `baseURL`, returning the absolute href. */
|
||||
/**
|
||||
* Resolves `url` against an optional `baseURL`, returning the absolute href.
|
||||
*
|
||||
* Mirrors how axios composes the request target, including the case of an absent
|
||||
* or empty `url`, where the request goes to `baseURL` on its own.
|
||||
*/
|
||||
export function buildTargetUrl(url?: string, baseURL?: string): string | undefined {
|
||||
if (!url) return undefined;
|
||||
|
||||
try {
|
||||
if (!url) {
|
||||
return baseURL ? new URL(baseURL).href : undefined;
|
||||
}
|
||||
return baseURL ? new URL(url, baseURL).href : url;
|
||||
} catch {
|
||||
return undefined;
|
||||
|
|
@ -395,8 +401,22 @@ export async function validateUrlSsrf(
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves the raw target of a legacy request object, i.e. the value the axios
|
||||
* config carries as its `url`.
|
||||
*
|
||||
* `url` wins over its `uri` alias whenever it is set, mirroring the precedence of
|
||||
* the `request` library. Single source of truth for the legacy target so the
|
||||
* pre-flight SSRF check and the dispatched request can never read different
|
||||
* fields.
|
||||
*/
|
||||
export function resolveLegacyRequestTarget(requestObject: IRequestOptions): string | undefined {
|
||||
return (requestObject.url ?? requestObject.uri)?.toString();
|
||||
}
|
||||
|
||||
/** Resolves the absolute target of a legacy request object, honouring `baseURL`. */
|
||||
export function resolveLegacyRequestUrl(requestObject: IRequestOptions): string | undefined {
|
||||
const rawUrl = requestObject.uri?.toString() ?? requestObject.url?.toString();
|
||||
const rawUrl = resolveLegacyRequestTarget(requestObject);
|
||||
const baseURL = requestObject.baseURL?.toString();
|
||||
return buildTargetUrl(rawUrl, baseURL) ?? rawUrl;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/backend-test-utils",
|
||||
"version": "1.34.0",
|
||||
"version": "1.34.1",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
"dev": "pnpm watch",
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/blob-storage",
|
||||
"version": "1.2.0",
|
||||
"version": "1.2.1",
|
||||
"description": "Blob storage layer for n8n, with filesystem, S3, and Azure Blob backends",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/chat-hub",
|
||||
"version": "1.27.0",
|
||||
"version": "1.27.1",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
"dev": "pnpm watch",
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/client-oauth2",
|
||||
"version": "1.16.0",
|
||||
"version": "1.16.1",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
"dev": "pnpm watch",
|
||||
|
|
|
|||
|
|
@ -1,5 +1,10 @@
|
|||
/* eslint-disable @typescript-eslint/no-explicit-any */
|
||||
import { createHttpsProxyAgent } from '@n8n/backend-network/proxy';
|
||||
import type { SsrfBridge } from '@n8n/backend-network';
|
||||
import {
|
||||
createHttpProxyAgent,
|
||||
createHttpsProxyAgent,
|
||||
resolveProxyUrl,
|
||||
} from '@n8n/backend-network/proxy';
|
||||
import axios from 'axios';
|
||||
import type { AxiosRequestConfig, AxiosResponse } from 'axios';
|
||||
import * as qs from 'querystring';
|
||||
|
|
@ -15,7 +20,7 @@ import type {
|
|||
OAuth2AuthenticationMethod,
|
||||
OAuth2ClientCredentialType,
|
||||
} from './types';
|
||||
import { getAuthError } from './utils';
|
||||
import { getAuthError, tryParseUrl } from './utils';
|
||||
|
||||
export interface ClientOAuth2RequestObject {
|
||||
url: string;
|
||||
|
|
@ -44,6 +49,12 @@ export interface ClientOAuth2Options {
|
|||
query?: qs.ParsedUrlQuery;
|
||||
resource?: string;
|
||||
ignoreSSLIssues?: boolean;
|
||||
/**
|
||||
* When provided, token endpoint requests are validated against the host's
|
||||
* outbound network policy before dispatch, at DNS resolution time, and on
|
||||
* every redirect. Omit to leave the request unchecked.
|
||||
*/
|
||||
ssrfBridge?: SsrfBridge;
|
||||
}
|
||||
|
||||
export class ResponseError extends Error {
|
||||
|
|
@ -109,15 +120,41 @@ export class ClientOAuth2 {
|
|||
proxy: false,
|
||||
};
|
||||
|
||||
if (options.ignoreSSLIssues) {
|
||||
// Build a proxy-aware agent that relaxes TLS verification, so ignoring SSL
|
||||
// issues still routes through the env proxy (HTTPS_PROXY / NO_PROXY) instead
|
||||
// of connecting directly and bypassing the proxy.
|
||||
const { ssrfBridge } = this.options;
|
||||
|
||||
if (ssrfBridge) {
|
||||
const parsed = tryParseUrl(url);
|
||||
if (parsed) {
|
||||
const result = await ssrfBridge.validateUrl(parsed);
|
||||
if (!result.ok) throw result.error;
|
||||
}
|
||||
|
||||
requestConfig.beforeRedirect = (redirected) => {
|
||||
ssrfBridge.validateRedirectSync(String(redirected.href));
|
||||
};
|
||||
}
|
||||
|
||||
// Resolution is re-checked on the agent, so a hostname that resolves to a
|
||||
// different address between validation and connect is still caught. Only for
|
||||
// direct connections though: through a proxy the agent resolves the proxy host,
|
||||
// not the final target, so applying the lookup there would check the wrong host.
|
||||
const lookup = resolveProxyUrl(url) ? undefined : ssrfBridge?.createSecureLookup();
|
||||
|
||||
// Agents are built here rather than left to the global ones so the per-request
|
||||
// `lookup` and relaxed TLS apply. Both factories are proxy-aware, so ignoring SSL
|
||||
// issues still routes through the env proxy (HTTP(S)_PROXY / NO_PROXY) instead of
|
||||
// connecting directly and bypassing it.
|
||||
if (options.ignoreSSLIssues || lookup) {
|
||||
requestConfig.httpsAgent = createHttpsProxyAgent(url, undefined, {
|
||||
rejectUnauthorized: false,
|
||||
...(options.ignoreSSLIssues ? { rejectUnauthorized: false } : {}),
|
||||
...(lookup ? { lookup } : {}),
|
||||
});
|
||||
}
|
||||
|
||||
if (lookup) {
|
||||
requestConfig.httpAgent = createHttpProxyAgent(url, undefined, { lookup });
|
||||
}
|
||||
|
||||
const response = await axios.request(requestConfig);
|
||||
|
||||
if (response.status >= 400) {
|
||||
|
|
|
|||
|
|
@ -45,6 +45,17 @@ export function getAuthError(body: {
|
|||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse a string as a URL, returning `null` when it is not parseable.
|
||||
*/
|
||||
export function tryParseUrl(url: string): URL | null {
|
||||
try {
|
||||
return new URL(url);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensure a value is a string.
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -1,7 +1,10 @@
|
|||
import type { SsrfBridge } from '@n8n/backend-network';
|
||||
import axios from 'axios';
|
||||
import { Agent as HttpAgent, createServer, type Server } from 'http';
|
||||
import { Agent as HttpsAgent } from 'https';
|
||||
import { HttpsProxyAgent } from 'https-proxy-agent';
|
||||
import nock from 'nock';
|
||||
import type { AddressInfo, LookupFunction } from 'node:net';
|
||||
|
||||
import { ClientOAuth2, ResponseError } from '@/client-oauth2';
|
||||
import { ERROR_RESPONSES } from '@/constants';
|
||||
|
|
@ -371,6 +374,200 @@ describe('ClientOAuth2', () => {
|
|||
expect(requestConfig.proxy).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('ssrfBridge', () => {
|
||||
const lookup = vi.fn() as unknown as LookupFunction;
|
||||
|
||||
const agentLookupOf = (agent: unknown) =>
|
||||
(agent as { options: { lookup?: LookupFunction } }).options.lookup;
|
||||
|
||||
const makeSsrfBridge = (overrides?: Partial<SsrfBridge>): SsrfBridge => ({
|
||||
validateUrl: vi.fn().mockResolvedValue({ ok: true, result: undefined }),
|
||||
validateIp: vi.fn().mockReturnValue({ ok: true, result: undefined }),
|
||||
validateConnectionHost: vi.fn().mockReturnValue({ ok: true, result: undefined }),
|
||||
validateRedirectSync: vi.fn(),
|
||||
createSecureLookup: vi.fn().mockReturnValue(lookup),
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const makeBridgedCall = async (ssrfBridge: SsrfBridge, ignoreSSLIssues?: boolean) =>
|
||||
await new ClientOAuth2({
|
||||
clientId: config.clientId,
|
||||
clientSecret: config.clientSecret,
|
||||
accessTokenUri: config.accessTokenUri,
|
||||
authentication: 'header',
|
||||
ssrfBridge,
|
||||
}).accessTokenRequest({
|
||||
url: config.accessTokenUri,
|
||||
method: 'POST',
|
||||
headers: { Authorization: authHeader, Accept: 'application/json' },
|
||||
body: { refresh_token: 'test', grant_type: 'refresh_token' },
|
||||
ignoreSSLIssues,
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
nock.cleanAll();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('should reject before dispatching when the target is not allowed', async () => {
|
||||
const blocked = new Error('Address not allowed');
|
||||
const ssrfBridge = makeSsrfBridge({
|
||||
validateUrl: vi.fn().mockResolvedValue({ ok: false, error: blocked }),
|
||||
});
|
||||
const tokenScope = mockTokenResponse({
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ access_token: config.accessToken }),
|
||||
});
|
||||
|
||||
await expect(makeBridgedCall(ssrfBridge)).rejects.toBe(blocked);
|
||||
|
||||
expect(ssrfBridge.validateUrl).toHaveBeenCalledWith(new URL(config.accessTokenUri));
|
||||
expect(tokenScope.isDone()).toBe(false);
|
||||
});
|
||||
|
||||
it('should resolve the target through the provided lookup and guard redirects', async () => {
|
||||
const ssrfBridge = makeSsrfBridge();
|
||||
mockTokenResponse({
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ access_token: config.accessToken }),
|
||||
});
|
||||
|
||||
const axiosSpy = vi.spyOn(axios, 'request');
|
||||
|
||||
await makeBridgedCall(ssrfBridge);
|
||||
|
||||
const requestConfig = axiosSpy.mock.calls[0][0];
|
||||
expect(requestConfig.httpAgent).toBeInstanceOf(HttpAgent);
|
||||
expect(requestConfig.httpsAgent).toBeInstanceOf(HttpsAgent);
|
||||
expect(agentLookupOf(requestConfig.httpAgent)).toBe(lookup);
|
||||
expect(agentLookupOf(requestConfig.httpsAgent)).toBe(lookup);
|
||||
|
||||
requestConfig.beforeRedirect?.(
|
||||
{ href: 'http://redirected.example/token' },
|
||||
{} as never,
|
||||
{} as never,
|
||||
);
|
||||
expect(ssrfBridge.validateRedirectSync).toHaveBeenCalledWith(
|
||||
'http://redirected.example/token',
|
||||
);
|
||||
});
|
||||
|
||||
it('should keep the lookup when ignoreSSLIssues also relaxes TLS', async () => {
|
||||
const ssrfBridge = makeSsrfBridge();
|
||||
mockTokenResponse({
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ access_token: config.accessToken }),
|
||||
});
|
||||
|
||||
const axiosSpy = vi.spyOn(axios, 'request');
|
||||
|
||||
await makeBridgedCall(ssrfBridge, true);
|
||||
|
||||
const httpsAgent = axiosSpy.mock.calls[0][0].httpsAgent as HttpsAgent;
|
||||
expect(httpsAgent.options.rejectUnauthorized).toBe(false);
|
||||
expect(agentLookupOf(httpsAgent)).toBe(lookup);
|
||||
});
|
||||
|
||||
describe('behind an env proxy', () => {
|
||||
const PROXY_ENV_VARS = ['HTTPS_PROXY', 'https_proxy', 'NO_PROXY', 'no_proxy'] as const;
|
||||
let savedProxyEnv: Record<string, string | undefined>;
|
||||
|
||||
beforeEach(() => {
|
||||
savedProxyEnv = {};
|
||||
for (const key of PROXY_ENV_VARS) {
|
||||
savedProxyEnv[key] = process.env[key];
|
||||
delete process.env[key];
|
||||
}
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
for (const key of PROXY_ENV_VARS) {
|
||||
if (savedProxyEnv[key] === undefined) {
|
||||
delete process.env[key];
|
||||
} else {
|
||||
process.env[key] = savedProxyEnv[key];
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
const proxiedTokenResponse = () =>
|
||||
vi.spyOn(axios, 'request').mockResolvedValue({
|
||||
status: 200,
|
||||
headers: {},
|
||||
data: JSON.stringify({ access_token: config.accessToken }),
|
||||
});
|
||||
|
||||
it('should leave resolution to the proxy instead of checking the proxy host', async () => {
|
||||
process.env.HTTPS_PROXY = 'http://fake-proxy.example';
|
||||
const ssrfBridge = makeSsrfBridge();
|
||||
const axiosSpy = proxiedTokenResponse();
|
||||
|
||||
await makeBridgedCall(ssrfBridge);
|
||||
|
||||
// The lookup would resolve the proxy, not the target, so the target policy
|
||||
// must not be applied to it — the proxy reaches the target on our behalf.
|
||||
expect(ssrfBridge.createSecureLookup).not.toHaveBeenCalled();
|
||||
const requestConfig = axiosSpy.mock.calls[0][0];
|
||||
expect(requestConfig.httpAgent).toBeUndefined();
|
||||
expect(requestConfig.httpsAgent).toBeUndefined();
|
||||
// The pre-flight check on the target still runs.
|
||||
expect(ssrfBridge.validateUrl).toHaveBeenCalledWith(new URL(config.accessTokenUri));
|
||||
});
|
||||
|
||||
it('should relax TLS through the proxy agent without a lookup when ignoreSSLIssues is set', async () => {
|
||||
process.env.HTTPS_PROXY = 'http://fake-proxy.example';
|
||||
const ssrfBridge = makeSsrfBridge();
|
||||
const axiosSpy = proxiedTokenResponse();
|
||||
|
||||
await makeBridgedCall(ssrfBridge, true);
|
||||
|
||||
const httpsAgent = axiosSpy.mock.calls[0][0].httpsAgent as HttpsProxyAgent<string>;
|
||||
expect(httpsAgent).toBeInstanceOf(HttpsProxyAgent);
|
||||
expect(httpsAgent.connectOpts.rejectUnauthorized).toBe(false);
|
||||
expect(httpsAgent.connectOpts.lookup).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should apply the lookup when NO_PROXY excludes the target', async () => {
|
||||
process.env.HTTPS_PROXY = 'http://fake-proxy.example';
|
||||
process.env.NO_PROXY = new URL(config.baseUrl).hostname;
|
||||
const ssrfBridge = makeSsrfBridge();
|
||||
mockTokenResponse({
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ access_token: config.accessToken }),
|
||||
});
|
||||
|
||||
const axiosSpy = vi.spyOn(axios, 'request');
|
||||
|
||||
await makeBridgedCall(ssrfBridge);
|
||||
|
||||
const requestConfig = axiosSpy.mock.calls[0][0];
|
||||
expect(agentLookupOf(requestConfig.httpAgent)).toBe(lookup);
|
||||
expect(agentLookupOf(requestConfig.httpsAgent)).toBe(lookup);
|
||||
});
|
||||
});
|
||||
|
||||
it('should leave the request unchecked when no bridge is provided', async () => {
|
||||
mockTokenResponse({
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ access_token: config.accessToken }),
|
||||
});
|
||||
|
||||
const axiosSpy = vi.spyOn(axios, 'request');
|
||||
|
||||
await makeTokenCall();
|
||||
|
||||
const requestConfig = axiosSpy.mock.calls[0][0];
|
||||
expect(requestConfig.httpAgent).toBeUndefined();
|
||||
expect(requestConfig.httpsAgent).toBeUndefined();
|
||||
expect(requestConfig.beforeRedirect).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('RFC 8707 resource parameter', () => {
|
||||
|
|
@ -503,3 +700,87 @@ describe('ClientOAuth2', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
// Runs against a real loopback server rather than a nock interceptor: the secure
|
||||
// lookup lives on the request's http agent, so only an actual socket connection
|
||||
// proves it is consulted.
|
||||
describe('ClientOAuth2 over a real connection', () => {
|
||||
let server: Server;
|
||||
let port: number;
|
||||
let hits: number;
|
||||
|
||||
beforeAll(async () => {
|
||||
nock.cleanAll();
|
||||
nock.enableNetConnect();
|
||||
hits = 0;
|
||||
server = createServer((_req, res) => {
|
||||
hits++;
|
||||
res.writeHead(200, { 'Content-Type': 'application/json' });
|
||||
res.end(JSON.stringify({ access_token: 'real-token' }));
|
||||
});
|
||||
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
|
||||
port = (server.address() as AddressInfo).port;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await new Promise<void>((resolve, reject) =>
|
||||
server.close((error) => (error ? reject(error) : resolve())),
|
||||
);
|
||||
nock.disableNetConnect();
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
hits = 0;
|
||||
});
|
||||
|
||||
const makeCall = async (ssrfBridge?: SsrfBridge) => {
|
||||
const url = `http://localhost:${port}/token`;
|
||||
return await new ClientOAuth2({
|
||||
clientId: 'id',
|
||||
clientSecret: 'secret',
|
||||
accessTokenUri: url,
|
||||
authentication: 'header',
|
||||
ssrfBridge,
|
||||
}).accessTokenRequest({
|
||||
url,
|
||||
method: 'POST',
|
||||
headers: { Accept: 'application/json' },
|
||||
body: { grant_type: 'refresh_token', refresh_token: 'x' },
|
||||
});
|
||||
};
|
||||
|
||||
const bridgeWithLookup = (lookup: LookupFunction): SsrfBridge => ({
|
||||
validateUrl: vi.fn().mockResolvedValue({ ok: true, result: undefined }),
|
||||
validateIp: vi.fn().mockReturnValue({ ok: true, result: undefined }),
|
||||
validateConnectionHost: vi.fn().mockReturnValue({ ok: true, result: undefined }),
|
||||
validateRedirectSync: vi.fn(),
|
||||
createSecureLookup: () => lookup,
|
||||
});
|
||||
|
||||
it('should not open a connection when the lookup rejects the resolved address', async () => {
|
||||
const lookup: LookupFunction = (hostname, options, onResult) => {
|
||||
onResult(new Error(`lookup rejected ${hostname}`), options.all ? [] : '', undefined);
|
||||
};
|
||||
|
||||
await expect(makeCall(bridgeWithLookup(lookup))).rejects.toThrow('lookup rejected localhost');
|
||||
expect(hits).toBe(0);
|
||||
});
|
||||
|
||||
it('should complete the request when the lookup allows the resolved address', async () => {
|
||||
const lookup: LookupFunction = (_hostname, options, onResult) => {
|
||||
onResult(null, options.all ? [{ address: '127.0.0.1', family: 4 }] : '127.0.0.1', 4);
|
||||
};
|
||||
|
||||
const token = await makeCall(bridgeWithLookup(lookup));
|
||||
|
||||
expect(token.access_token).toBe('real-token');
|
||||
expect(hits).toBe(1);
|
||||
});
|
||||
|
||||
it('should complete the request when no bridge is provided', async () => {
|
||||
const token = await makeCall();
|
||||
|
||||
expect(token.access_token).toBe('real-token');
|
||||
expect(hits).toBe(1);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/create-node",
|
||||
"version": "0.42.0",
|
||||
"version": "0.42.1",
|
||||
"description": "Official CLI to create new community nodes for n8n",
|
||||
"bin": {
|
||||
"create-node": "bin/create-node.cjs"
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/db",
|
||||
"version": "1.34.0",
|
||||
"version": "1.34.1",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo .tmp-schema-docs",
|
||||
"dev": "pnpm watch",
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/decorators",
|
||||
"version": "1.34.0",
|
||||
"version": "1.34.1",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
"dev": "pnpm watch",
|
||||
|
|
|
|||
|
|
@ -444,6 +444,7 @@ export const baseConfig = tseslint.config(
|
|||
'@typescript-eslint/unbound-method': 'off',
|
||||
'n8n-local-rules/no-skipped-tests': process.env.NODE_ENV === 'development' ? 'warn' : 'error',
|
||||
'n8n-local-rules/no-error-instance-in-to-throw': 'error',
|
||||
'n8n-local-rules/no-dynamic-regexp': 'off',
|
||||
},
|
||||
},
|
||||
);
|
||||
|
|
|
|||
|
|
@ -28,6 +28,7 @@ export const localRulesPlugin = {
|
|||
'n8n-local-rules/no-type-only-import-in-di': 'error',
|
||||
'n8n-local-rules/no-aws-credential-discovery-imports': 'error',
|
||||
'n8n-local-rules/no-application-error': 'error',
|
||||
'n8n-local-rules/no-dynamic-regexp': 'warn',
|
||||
'n8n-local-rules/no-restricted-sleep-definition': 'error',
|
||||
'n8n-local-rules/no-restricted-sleep-import': 'error',
|
||||
},
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ import { NoErrorInstanceInToThrowRule } from './no-error-instance-in-to-throw.js
|
|||
import { NoAwsCredentialDiscoveryImportsRule } from './no-aws-credential-discovery-imports.js';
|
||||
import { NoUncentralizedHttpRule } from './no-uncentralized-http.js';
|
||||
import { NoApplicationErrorRule } from './no-application-error.js';
|
||||
import { NoDynamicRegExpRule } from './no-dynamic-regexp.js';
|
||||
import { ProjectOwnedEntityTransferRule } from './project-owned-entity-transfer.js';
|
||||
import { NoRestrictedSleepDefinitionRule } from './no-restricted-sleep-definition.js';
|
||||
import { NoRestrictedSleepImportRule } from './no-restricted-sleep-import.js';
|
||||
|
|
@ -53,6 +54,7 @@ export const rules = {
|
|||
'no-aws-credential-discovery-imports': NoAwsCredentialDiscoveryImportsRule,
|
||||
'no-uncentralized-http': NoUncentralizedHttpRule,
|
||||
'no-application-error': NoApplicationErrorRule,
|
||||
'no-dynamic-regexp': NoDynamicRegExpRule,
|
||||
'project-owned-entity-transfer': ProjectOwnedEntityTransferRule,
|
||||
'no-restricted-sleep-definition': NoRestrictedSleepDefinitionRule,
|
||||
'no-restricted-sleep-import': NoRestrictedSleepImportRule,
|
||||
|
|
|
|||
114
packages/@n8n/eslint-config/src/rules/no-dynamic-regexp.test.ts
Normal file
114
packages/@n8n/eslint-config/src/rules/no-dynamic-regexp.test.ts
Normal file
|
|
@ -0,0 +1,114 @@
|
|||
import { RuleTester } from '@typescript-eslint/rule-tester';
|
||||
|
||||
import { NoDynamicRegExpRule } from './no-dynamic-regexp.js';
|
||||
|
||||
const ruleTester = new RuleTester({
|
||||
languageOptions: {
|
||||
parser: require('@typescript-eslint/parser'),
|
||||
parserOptions: {
|
||||
ecmaVersion: 2022,
|
||||
sourceType: 'module',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const repeatedStaticPattern = [
|
||||
"const pattern0 = 'x';",
|
||||
...Array.from(
|
||||
{ length: 30 },
|
||||
(_, index) => `const pattern${index + 1} = pattern${index} + pattern${index};`,
|
||||
),
|
||||
'new RegExp(pattern30);',
|
||||
].join('\n');
|
||||
|
||||
ruleTester.run('no-dynamic-regexp', NoDynamicRegExpRule, {
|
||||
valid: [
|
||||
"new RegExp('^foo$');",
|
||||
'new RegExp();',
|
||||
'new OtherRegExp(dynamicPattern);',
|
||||
`
|
||||
const name = 'foo';
|
||||
new RegExp(\`^\${name}$\`);
|
||||
`,
|
||||
`
|
||||
const prefix = 'foo';
|
||||
const pattern = '^' + prefix + '$';
|
||||
new RegExp(pattern);
|
||||
`,
|
||||
`
|
||||
const source = /foo/.source;
|
||||
const pattern = source.replace('foo', 'bar').toUpperCase();
|
||||
new RegExp(pattern);
|
||||
`,
|
||||
`
|
||||
const source = 'foo';
|
||||
new RegExp(\`^\${source}:\${source}$\`);
|
||||
`,
|
||||
`
|
||||
import { RUNTIME_SKILL_NAME_PATTERN } from '@n8n/agents';
|
||||
|
||||
const skillNameSource = RUNTIME_SKILL_NAME_PATTERN.source.replace(/^\\^|\\$$/g, '');
|
||||
new RegExp(\`^(?:\${skillNameSource}:)?\${skillNameSource}$\`);
|
||||
`,
|
||||
`
|
||||
const pattern = '^foo$' as const;
|
||||
new RegExp(pattern satisfies string);
|
||||
`,
|
||||
repeatedStaticPattern,
|
||||
// very deep expression, don't report
|
||||
`new RegExp(${Array.from({ length: 101 }, () => "'x'").join(' + ')});`,
|
||||
],
|
||||
invalid: [
|
||||
{
|
||||
code: `
|
||||
function createPattern(name: string) {
|
||||
return new RegExp(\`^\${name}$\`);
|
||||
}
|
||||
`,
|
||||
errors: [{ messageId: 'noDynamicRegExp' }],
|
||||
},
|
||||
{
|
||||
code: `
|
||||
let pattern = '^foo$';
|
||||
new RegExp(pattern);
|
||||
`,
|
||||
errors: [{ messageId: 'noDynamicRegExp' }],
|
||||
},
|
||||
{
|
||||
code: `
|
||||
const pattern = getPattern();
|
||||
new RegExp(pattern);
|
||||
`,
|
||||
errors: [{ messageId: 'noDynamicRegExp' }],
|
||||
},
|
||||
{
|
||||
code: `
|
||||
function createPattern(options: { pattern: string }) {
|
||||
return new RegExp(options.pattern);
|
||||
}
|
||||
`,
|
||||
errors: [{ messageId: 'noDynamicRegExp' }],
|
||||
},
|
||||
{
|
||||
code: `
|
||||
function createPattern(suffix: string) {
|
||||
const source = 'foo';
|
||||
return new RegExp(source.replace('foo', suffix));
|
||||
}
|
||||
`,
|
||||
errors: [{ messageId: 'noDynamicRegExp' }],
|
||||
},
|
||||
{
|
||||
code: `
|
||||
function createPattern(authCookieName = 'n8n-auth') {
|
||||
return new RegExp(\`(^| )\${authCookieName}=(?<token>[^;]+)\`);
|
||||
}
|
||||
`,
|
||||
errors: [{ messageId: 'noDynamicRegExp' }],
|
||||
},
|
||||
{
|
||||
code: 'new RegExp(unknownPattern);',
|
||||
errors: [{ messageId: 'noDynamicRegExp' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
215
packages/@n8n/eslint-config/src/rules/no-dynamic-regexp.ts
Normal file
215
packages/@n8n/eslint-config/src/rules/no-dynamic-regexp.ts
Normal file
|
|
@ -0,0 +1,215 @@
|
|||
import { ESLintUtils, type TSESTree } from '@typescript-eslint/utils';
|
||||
import type { RuleContext } from '@typescript-eslint/utils/ts-eslint';
|
||||
|
||||
type MessageIds = 'noDynamicRegExp';
|
||||
type Options = [];
|
||||
|
||||
type Variable = {
|
||||
defs: Array<{
|
||||
type: string;
|
||||
node?: {
|
||||
init?: TSESTree.Expression | null;
|
||||
};
|
||||
name?: TSESTree.Identifier;
|
||||
parent?: {
|
||||
kind?: string;
|
||||
};
|
||||
}>;
|
||||
references: Array<{
|
||||
identifier?: TSESTree.Identifier;
|
||||
isReadOnly: () => boolean;
|
||||
}>;
|
||||
};
|
||||
|
||||
type Scope = {
|
||||
set: Map<string, unknown>;
|
||||
upper: Scope | null;
|
||||
};
|
||||
|
||||
type SourceCodeWithScope = {
|
||||
getScope(node: TSESTree.Node): unknown;
|
||||
};
|
||||
|
||||
const MAX_STATIC_EXPRESSION_DEPTH = 100;
|
||||
const EVALUATING = Symbol('evaluating');
|
||||
|
||||
const STATIC_METHODS = new Set([
|
||||
'concat',
|
||||
'replace',
|
||||
'replaceAll',
|
||||
'slice',
|
||||
'substring',
|
||||
'toLowerCase',
|
||||
'toUpperCase',
|
||||
'trim',
|
||||
]);
|
||||
|
||||
function hasGetScope(sourceCode: unknown): sourceCode is SourceCodeWithScope {
|
||||
return (
|
||||
typeof sourceCode === 'object' &&
|
||||
sourceCode !== null &&
|
||||
typeof Reflect.get(sourceCode, 'getScope') === 'function'
|
||||
);
|
||||
}
|
||||
|
||||
function getScope(context: Readonly<RuleContext<MessageIds, Options>>) {
|
||||
const sourceCode = context.sourceCode ?? context.getSourceCode();
|
||||
return (node: TSESTree.Node) =>
|
||||
(hasGetScope(sourceCode) ? sourceCode.getScope(node) : context.getScope()) as Scope;
|
||||
}
|
||||
|
||||
function isVariable(value: unknown): value is Variable {
|
||||
return (
|
||||
typeof value === 'object' &&
|
||||
value !== null &&
|
||||
Array.isArray(Reflect.get(value, 'defs')) &&
|
||||
Array.isArray(Reflect.get(value, 'references'))
|
||||
);
|
||||
}
|
||||
|
||||
function findVariable(scope: Scope, name: string): Variable | undefined {
|
||||
let currentScope: Scope | null = scope;
|
||||
|
||||
while (currentScope) {
|
||||
const variable = currentScope.set.get(name);
|
||||
if (isVariable(variable)) return variable;
|
||||
currentScope = currentScope.upper;
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function isStaticExpression(
|
||||
node: TSESTree.Node,
|
||||
getNodeScope: (node: TSESTree.Node) => Scope,
|
||||
cache: WeakMap<TSESTree.Node, boolean | typeof EVALUATING>,
|
||||
depth = 0,
|
||||
): boolean {
|
||||
// Stop and don't report if the expression is too deep.
|
||||
if (depth >= MAX_STATIC_EXPRESSION_DEPTH) return true;
|
||||
|
||||
// Cache shared initializers and mark active nodes to reject circular references.
|
||||
const cached = cache.get(node);
|
||||
if (cached !== undefined) return cached === EVALUATING ? false : cached;
|
||||
|
||||
cache.set(node, EVALUATING);
|
||||
const scope = getNodeScope(node);
|
||||
const nextDepth = depth + 1;
|
||||
|
||||
const isStatic = (() => {
|
||||
switch (node.type) {
|
||||
// A string, number, boolean, null, bigint, or regex written directly in source.
|
||||
case 'Literal':
|
||||
return true;
|
||||
|
||||
// A template string whose interpolated expressions must all be static.
|
||||
case 'TemplateLiteral':
|
||||
return node.expressions.every((expression) =>
|
||||
isStaticExpression(expression, getNodeScope, cache, nextDepth),
|
||||
);
|
||||
|
||||
// String concatenation, provided both operands are static.
|
||||
case 'BinaryExpression':
|
||||
return (
|
||||
node.operator === '+' &&
|
||||
isStaticExpression(node.left, getNodeScope, cache, nextDepth) &&
|
||||
isStaticExpression(node.right, getNodeScope, cache, nextDepth)
|
||||
);
|
||||
|
||||
// A named binding, such as an import or an immutable const variable.
|
||||
case 'Identifier': {
|
||||
const variable = findVariable(scope, node.name);
|
||||
if (!variable || variable.defs.length !== 1) return false;
|
||||
|
||||
const [definition] = variable.defs;
|
||||
if (definition.type === 'ImportBinding') return true;
|
||||
|
||||
const isConstDeclaration = definition.parent?.kind === 'const';
|
||||
if (definition.type !== 'Variable' || !isConstDeclaration || !definition.node?.init) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const isNeverWrittenAfterInit = variable.references.every(
|
||||
(reference) => reference.isReadOnly() || reference.identifier === definition.name,
|
||||
);
|
||||
return (
|
||||
isNeverWrittenAfterInit &&
|
||||
isStaticExpression(definition.node.init, getNodeScope, cache, nextDepth)
|
||||
);
|
||||
}
|
||||
|
||||
// Property access on a static object, including a static computed property.
|
||||
case 'MemberExpression':
|
||||
return (
|
||||
isStaticExpression(node.object, getNodeScope, cache, nextDepth) &&
|
||||
(!node.computed || isStaticExpression(node.property, getNodeScope, cache, nextDepth))
|
||||
);
|
||||
|
||||
// A whitelisted deterministic method called on static data with static arguments.
|
||||
case 'CallExpression': {
|
||||
if (node.callee.type !== 'MemberExpression' || node.callee.computed) return false;
|
||||
if (node.callee.property.type !== 'Identifier') return false;
|
||||
if (!STATIC_METHODS.has(node.callee.property.name)) return false;
|
||||
|
||||
return (
|
||||
isStaticExpression(node.callee.object, getNodeScope, cache, nextDepth) &&
|
||||
node.arguments.every(
|
||||
(argument) =>
|
||||
argument.type !== 'SpreadElement' &&
|
||||
isStaticExpression(argument, getNodeScope, cache, nextDepth),
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
// TypeScript-only wrappers that do not change the expression's runtime value.
|
||||
case 'TSAsExpression':
|
||||
case 'TSSatisfiesExpression':
|
||||
case 'TSNonNullExpression':
|
||||
case 'TSInstantiationExpression':
|
||||
return isStaticExpression(node.expression, getNodeScope, cache, nextDepth);
|
||||
|
||||
// All other expression types may depend on runtime data.
|
||||
default:
|
||||
return false;
|
||||
}
|
||||
})();
|
||||
|
||||
cache.set(node, isStatic);
|
||||
return isStatic;
|
||||
}
|
||||
|
||||
export const NoDynamicRegExpRule = ESLintUtils.RuleCreator.withoutDocs<Options, MessageIds>({
|
||||
meta: {
|
||||
type: 'problem',
|
||||
docs: {
|
||||
description:
|
||||
'Disallow RegExp constructors from dynamic values; use safeRegex for user-controlled patterns.',
|
||||
},
|
||||
schema: [],
|
||||
messages: {
|
||||
noDynamicRegExp:
|
||||
'Use safeRegex for dynamic regular expressions. Only literal or statically resolvable RegExp patterns are allowed.',
|
||||
},
|
||||
},
|
||||
defaultOptions: [],
|
||||
create(context) {
|
||||
const getNodeScope = getScope(context);
|
||||
const staticExpressionCache = new WeakMap<TSESTree.Node, boolean | typeof EVALUATING>();
|
||||
|
||||
return {
|
||||
NewExpression(node: TSESTree.NewExpression) {
|
||||
if (node.callee.type !== 'Identifier' || node.callee.name !== 'RegExp') return;
|
||||
|
||||
const [pattern] = node.arguments;
|
||||
if (!pattern || pattern.type === 'SpreadElement') return;
|
||||
|
||||
if (isStaticExpression(pattern, getNodeScope, staticExpressionCache)) return;
|
||||
|
||||
context.report({
|
||||
node,
|
||||
messageId: 'noDynamicRegExp',
|
||||
});
|
||||
},
|
||||
};
|
||||
},
|
||||
});
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/instance-ai",
|
||||
"version": "1.19.0",
|
||||
"version": "1.19.1",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
"typecheck": "tsc --noEmit && tsc -p evaluations/tsconfig.json",
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/mcp-apps",
|
||||
"version": "0.11.0",
|
||||
"version": "0.11.2",
|
||||
"description": "MCP Apps UI resources and server helpers for n8n",
|
||||
"main": "dist/server/index.js",
|
||||
"types": "dist/server/index.d.ts",
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/node-cli",
|
||||
"version": "0.43.0",
|
||||
"version": "0.43.1",
|
||||
"description": "Official CLI for developing community nodes for n8n",
|
||||
"bin": {
|
||||
"n8n-node": "bin/n8n-node.mjs"
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/node-engine-compatibility",
|
||||
"version": "0.2.0",
|
||||
"version": "0.2.1",
|
||||
"description": "Adapts v1 nodes and workflows to the Engine 2.0 execution model",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ export default defineConfig(
|
|||
// TODO: remove all the following rules
|
||||
eqeqeq: 'warn',
|
||||
'id-denylist': 'warn',
|
||||
'n8n-local-rules/no-dynamic-regexp': 'error',
|
||||
'no-case-declarations': 'warn',
|
||||
'no-extra-boolean-cast': 'warn',
|
||||
'no-empty': 'warn',
|
||||
|
|
@ -168,6 +169,7 @@ export default defineConfig(
|
|||
rules: {
|
||||
'import-x/no-extraneous-dependencies': 'warn',
|
||||
'n8n-local-rules/no-uncaught-json-parse': 'warn',
|
||||
'n8n-local-rules/no-dynamic-regexp': 'off',
|
||||
},
|
||||
},
|
||||
);
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
// Source: https://github.com/openai/openai-guardrails-js/blob/b9b99b4fb454f02a362c2836aec6285176ec40a8/src/checks/keywords.ts
|
||||
import { safeRegex } from 'n8n-workflow';
|
||||
|
||||
import type { CreateCheckFn, GuardrailResult } from '../types';
|
||||
|
||||
interface KeywordsConfig {
|
||||
|
|
@ -63,14 +65,12 @@ const keywordsCheck = (text: string, config: KeywordsConfig): GuardrailResult =>
|
|||
});
|
||||
|
||||
const patternText = `(?:${keywordPatterns.join('|')})`;
|
||||
const pattern = new RegExp(patternText, 'giu'); // case-insensitive, global, unicode aware
|
||||
|
||||
const matches: string[] = [];
|
||||
let match;
|
||||
const seen = new Set<string>();
|
||||
|
||||
// Find all matches and collect unique ones (case-insensitive)
|
||||
while ((match = pattern.exec(text)) !== null) {
|
||||
for (const match of safeRegex.matchAll(patternText, text, 'giu')) {
|
||||
const matchedText = match[0];
|
||||
if (!seen.has(matchedText.toLowerCase())) {
|
||||
matches.push(matchedText);
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@
|
|||
* schema for entity selection, output/result structures, and the async guardrail
|
||||
* check_fn for runtime enforcement.
|
||||
*/
|
||||
import { safeRegex } from 'n8n-workflow';
|
||||
|
||||
import { parseRegex } from '../../helpers/common';
|
||||
import type { CreateCheckFn, CustomRegex } from '../types';
|
||||
|
|
@ -222,12 +223,13 @@ function detectPii(text: string, config: PIIConfig): PiiDetectionResult {
|
|||
const grouped: Record<string, string[]> = {};
|
||||
const analyzerResults: PiiAnalyzerResult[] = [];
|
||||
|
||||
const matchAgainstPattern = (name: string, pattern: RegExp) => {
|
||||
const matchAgainstPattern = (name: string, literal: { source: string; flags: string }) => {
|
||||
// make sure to add the global flag to the regex, otherwise while() will never end
|
||||
const flags = pattern.flags.includes('g') ? pattern.flags : pattern.flags + 'g';
|
||||
const regex = new RegExp(pattern.source, flags);
|
||||
let match;
|
||||
while ((match = regex.exec(text)) !== null) {
|
||||
const flags = literal.flags.includes('g') ? literal.flags : literal.flags + 'g';
|
||||
const matches = safeRegex.matchAll(literal.source, text, flags);
|
||||
for (const match of matches) {
|
||||
if (match.index === undefined) continue;
|
||||
|
||||
const entityType = name;
|
||||
const start = match.index;
|
||||
const end = match.index + match[0].length;
|
||||
|
|
|
|||
|
|
@ -6,6 +6,8 @@
|
|||
* recognition, and a guardrail check_fn for runtime enforcement.
|
||||
*/
|
||||
|
||||
import { safeRegex } from 'n8n-workflow';
|
||||
|
||||
import type { CreateCheckFn, GuardrailResult } from '../types';
|
||||
|
||||
export type SecretKeysConfig = {
|
||||
|
|
@ -175,6 +177,7 @@ function containsAllowedPattern(text: string): boolean {
|
|||
}
|
||||
|
||||
// Regex for allowed file extensions - must end with the extension
|
||||
// eslint-disable-next-line n8n-local-rules/no-dynamic-regexp -- static pattern
|
||||
const extPattern = new RegExp(
|
||||
`^[^\\s]*(${ALLOWED_EXTENSIONS.map((ext) => ext.replace('.', '\\.')).join('|')})$`,
|
||||
'i',
|
||||
|
|
@ -194,8 +197,7 @@ function isSecretCandidate(
|
|||
if (customRegex) {
|
||||
for (const pattern of customRegex) {
|
||||
try {
|
||||
const regex = new RegExp(pattern);
|
||||
if (regex.test(s)) {
|
||||
if (safeRegex.test(pattern, s)) {
|
||||
return true;
|
||||
}
|
||||
} catch {
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
import { parseRegexLiteral } from 'n8n-workflow';
|
||||
|
||||
export const splitByComma = (str: string) => {
|
||||
return str
|
||||
.split(',')
|
||||
|
|
@ -6,16 +8,5 @@ export const splitByComma = (str: string) => {
|
|||
};
|
||||
|
||||
export const parseRegex = (input: string) => {
|
||||
const regexMatch = (input || '').toString().match(new RegExp('^/(.*?)/([gimusy]*)$'));
|
||||
|
||||
let regex: RegExp;
|
||||
if (!regexMatch) {
|
||||
regex = new RegExp((input || '').toString());
|
||||
} else if (regexMatch.length === 1) {
|
||||
regex = new RegExp(regexMatch[1]);
|
||||
} else {
|
||||
regex = new RegExp(regexMatch[1], regexMatch[2]);
|
||||
}
|
||||
|
||||
return regex;
|
||||
return parseRegexLiteral((input || '').toString());
|
||||
};
|
||||
|
|
|
|||
|
|
@ -25,4 +25,14 @@ describe('pii guardrail', () => {
|
|||
expect(result.info?.maskEntities).toEqual({});
|
||||
expect(result.info?.analyzerResults).toEqual([]);
|
||||
});
|
||||
|
||||
it('uses custom regex patterns to detect configured entities', async () => {
|
||||
const result = await createPiiCheckFn({
|
||||
entities: [],
|
||||
customRegex: [{ name: 'ORDER_ID', value: '/ORD-\\d{4}/g' }],
|
||||
})('Process ORD-1234 and ORD-5678');
|
||||
|
||||
expect(result.tripwireTriggered).toBe(true);
|
||||
expect(result.info?.maskEntities?.ORDER_ID).toEqual(['ORD-1234', 'ORD-5678']);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -17,4 +17,16 @@ describe('secretKeys guardrail', () => {
|
|||
'7b9fcd0a-9188-4e36-8c65-bc915192b2375',
|
||||
]);
|
||||
});
|
||||
|
||||
it('detects custom regex secret patterns', () => {
|
||||
const config: SecretKeysConfig = {
|
||||
threshold: 'balanced',
|
||||
customRegex: ['custom-secret-[0-9]+'],
|
||||
};
|
||||
|
||||
const result = secretKeysCheck('Token: custom-secret-1234', config);
|
||||
|
||||
expect(result.tripwireTriggered).toBe(true);
|
||||
expect(result.info?.maskEntities?.SECRET).toContain('custom-secret-1234');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -78,18 +78,14 @@ describe('common helper', () => {
|
|||
const input = '/test/gi';
|
||||
const result = parseRegex(input);
|
||||
|
||||
expect(result).toBeInstanceOf(RegExp);
|
||||
expect(result.source).toBe('test');
|
||||
expect(result.flags).toBe('gi');
|
||||
expect(result).toEqual({ source: 'test', flags: 'gi' });
|
||||
});
|
||||
|
||||
it('should parse regex with forward slashes but no flags', () => {
|
||||
const input = '/test/';
|
||||
const result = parseRegex(input);
|
||||
|
||||
expect(result).toBeInstanceOf(RegExp);
|
||||
expect(result.source).toBe('test');
|
||||
expect(result.flags).toBe('');
|
||||
expect(result).toEqual({ source: 'test', flags: '' });
|
||||
});
|
||||
|
||||
it('should parse regex with different flags', () => {
|
||||
|
|
@ -138,9 +134,7 @@ describe('common helper', () => {
|
|||
const input = 'test';
|
||||
const result = parseRegex(input);
|
||||
|
||||
expect(result).toBeInstanceOf(RegExp);
|
||||
expect(result.source).toBe('test');
|
||||
expect(result.flags).toBe('');
|
||||
expect(result).toEqual({ source: 'test', flags: '' });
|
||||
});
|
||||
|
||||
it('should handle string with special characters without forward slashes', () => {
|
||||
|
|
@ -155,63 +149,49 @@ describe('common helper', () => {
|
|||
const input = '';
|
||||
const result = parseRegex(input);
|
||||
|
||||
expect(result).toBeInstanceOf(RegExp);
|
||||
expect(result.source).toBe('(?:)'); // Empty string becomes non-capturing group
|
||||
expect(result.flags).toBe('');
|
||||
expect(result).toEqual({ source: '', flags: '' });
|
||||
});
|
||||
|
||||
it('should handle null input', () => {
|
||||
const input = null as unknown as string;
|
||||
const result = parseRegex(input);
|
||||
|
||||
expect(result).toBeInstanceOf(RegExp);
|
||||
expect(result.source).toBe('(?:)'); // null becomes empty string, then non-capturing group
|
||||
expect(result.flags).toBe('');
|
||||
expect(result).toEqual({ source: '', flags: '' });
|
||||
});
|
||||
|
||||
it('should handle undefined input', () => {
|
||||
const input = undefined as unknown as string;
|
||||
const result = parseRegex(input);
|
||||
|
||||
expect(result).toBeInstanceOf(RegExp);
|
||||
expect(result.source).toBe('(?:)'); // undefined becomes empty string, then non-capturing group
|
||||
expect(result.flags).toBe('');
|
||||
expect(result).toEqual({ source: '', flags: '' });
|
||||
});
|
||||
|
||||
it('should handle malformed regex with only opening slash', () => {
|
||||
const input = '/test';
|
||||
const result = parseRegex(input);
|
||||
|
||||
expect(result).toBeInstanceOf(RegExp);
|
||||
expect(result.source).toBe('\\/test'); // Forward slash gets escaped
|
||||
expect(result.flags).toBe('');
|
||||
expect(result).toEqual({ source: '/test', flags: '' });
|
||||
});
|
||||
|
||||
it('should handle malformed regex with only closing slash', () => {
|
||||
const input = 'test/';
|
||||
const result = parseRegex(input);
|
||||
|
||||
expect(result).toBeInstanceOf(RegExp);
|
||||
expect(result.source).toBe('test\\/'); // Forward slash gets escaped
|
||||
expect(result.flags).toBe('');
|
||||
expect(result).toEqual({ source: 'test/', flags: '' });
|
||||
});
|
||||
|
||||
it('should handle regex with empty pattern', () => {
|
||||
const input = '//g';
|
||||
const result = parseRegex(input);
|
||||
|
||||
expect(result).toBeInstanceOf(RegExp);
|
||||
expect(result.source).toBe('(?:)'); // Empty pattern becomes non-capturing group
|
||||
expect(result.flags).toBe('g');
|
||||
expect(result).toEqual({ source: '', flags: 'g' });
|
||||
});
|
||||
|
||||
it('should handle regex with only slashes', () => {
|
||||
const input = '//';
|
||||
const result = parseRegex(input);
|
||||
|
||||
expect(result).toBeInstanceOf(RegExp);
|
||||
expect(result.source).toBe('(?:)'); // Empty pattern becomes non-capturing group
|
||||
expect(result.flags).toBe('');
|
||||
expect(result).toEqual({ source: '', flags: '' });
|
||||
});
|
||||
|
||||
it('should handle complex regex patterns', () => {
|
||||
|
|
|
|||
|
|
@ -1,22 +1,22 @@
|
|||
import type { SqlCreatePromptArgs } from '@langchain/classic/agents/toolkits/sql';
|
||||
import { SqlToolkit, createSqlAgent } from '@langchain/classic/agents/toolkits/sql';
|
||||
import { SqlDatabase } from '@langchain/classic/sql_db';
|
||||
import type { BaseChatMemory } from '@langchain/community/memory/chat_memory';
|
||||
import type { BaseLanguageModel } from '@langchain/core/language_models/base';
|
||||
import { DynamicTool } from '@langchain/core/tools';
|
||||
import type { DataSource } from '@n8n/typeorm';
|
||||
import type { SqlCreatePromptArgs } from '@langchain/classic/agents/toolkits/sql';
|
||||
import { SqlToolkit, createSqlAgent } from '@langchain/classic/agents/toolkits/sql';
|
||||
import { SqlDatabase } from '@langchain/classic/sql_db';
|
||||
import { isRecord } from '@n8n/utils/is-record';
|
||||
import { getPromptInputByType, serializeChatHistory } from '@utils/helpers';
|
||||
import type { TracingMetadataEntry } from '@utils/tracing';
|
||||
import { buildTracingMetadata, getTracingConfig } from '@utils/tracing';
|
||||
import {
|
||||
type IDataObject,
|
||||
type IExecuteFunctions,
|
||||
type INodeExecutionData,
|
||||
NodeConnectionTypes,
|
||||
NodeOperationError,
|
||||
type IDataObject,
|
||||
} from 'n8n-workflow';
|
||||
|
||||
import { getPromptInputByType, serializeChatHistory } from '@utils/helpers';
|
||||
import type { TracingMetadataEntry } from '@utils/tracing';
|
||||
import { buildTracingMetadata, getTracingConfig } from '@utils/tracing';
|
||||
|
||||
import { getMysqlDataSource } from './other/handlers/mysql';
|
||||
import { getPostgresDataSource } from './other/handlers/postgres';
|
||||
import { getSqliteDataSource } from './other/handlers/sqlite';
|
||||
|
|
@ -28,6 +28,17 @@ const parseTablesString = (tablesString: string) =>
|
|||
.map((table) => table.trim())
|
||||
.filter((table) => table.length > 0);
|
||||
|
||||
function getErrorMessage(error: unknown): unknown {
|
||||
if (error instanceof Error) return error.message;
|
||||
if (isRecord(error) && 'message' in error) return error.message;
|
||||
return error;
|
||||
}
|
||||
|
||||
function stringifyErrorMessage(error: unknown): string {
|
||||
const message = getErrorMessage(error);
|
||||
return typeof message === 'string' ? message : JSON.stringify(message);
|
||||
}
|
||||
|
||||
const BLOCKED_SQL_OPERATIONS = new Set([
|
||||
'INSERT',
|
||||
'UPDATE',
|
||||
|
|
@ -47,6 +58,7 @@ export function detectBlockedSqlKeyword(sql: string): string | undefined {
|
|||
.replace(/'(?:[^'\\]|\\.)*'/g, "''")
|
||||
.replace(/"(?:[^"\\]|\\.)*"/g, '""');
|
||||
|
||||
// eslint-disable-next-line n8n-local-rules/no-dynamic-regexp -- static pattern
|
||||
const pattern = new RegExp(`\\b(${[...BLOCKED_SQL_OPERATIONS].join('|')})\\b`, 'i');
|
||||
const match = stripped.match(pattern);
|
||||
return match ? match[1].toUpperCase() : undefined;
|
||||
|
|
@ -186,17 +198,20 @@ export async function sqlAgentAgentExecute(
|
|||
chatHistory,
|
||||
});
|
||||
} catch (error) {
|
||||
if ((error.message as IDataObject)?.output) {
|
||||
response = error.message as IDataObject;
|
||||
const errorMessage = getErrorMessage(error);
|
||||
if (isRecord(errorMessage) && errorMessage.output) {
|
||||
response = errorMessage as IDataObject;
|
||||
} else {
|
||||
throw new NodeOperationError(this.getNode(), error.message as string, { itemIndex: i });
|
||||
throw new NodeOperationError(this.getNode(), stringifyErrorMessage(error), {
|
||||
itemIndex: i,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
returnData.push({ json: response });
|
||||
} catch (error) {
|
||||
if (this.continueOnFail()) {
|
||||
returnData.push({ json: { error: error.message }, pairedItem: { item: i } });
|
||||
returnData.push({ json: { error: stringifyErrorMessage(error) }, pairedItem: { item: i } });
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { SearxngSearch } from '@langchain/community/tools/searxng_search';
|
||||
import { DynamicTool } from '@langchain/core/tools';
|
||||
import type {
|
||||
IExecuteFunctions,
|
||||
INode,
|
||||
|
|
@ -9,142 +9,239 @@ import { mock } from 'vitest-mock-extended';
|
|||
|
||||
import { ToolSearXng } from './ToolSearXng.node';
|
||||
|
||||
describe('ToolSearXng', () => {
|
||||
describe('supplyData', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
});
|
||||
const SEARXNG_RESPONSE = {
|
||||
results: [
|
||||
{
|
||||
title: 'First result',
|
||||
url: 'https://example.com/1',
|
||||
content: 'First snippet',
|
||||
},
|
||||
{
|
||||
title: 'Second result',
|
||||
url: 'https://example.com/2',
|
||||
content: 'Second snippet',
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
it('should return SearXNG tool instance', async () => {
|
||||
function createContext(
|
||||
overrides: {
|
||||
apiUrl?: string;
|
||||
options?: Record<string, unknown>;
|
||||
httpResponse?: unknown;
|
||||
inputData?: INodeExecutionData[];
|
||||
} = {},
|
||||
) {
|
||||
const httpRequest = vi.fn().mockResolvedValue(overrides.httpResponse ?? SEARXNG_RESPONSE);
|
||||
const ctx = mock<IExecuteFunctions>({
|
||||
getInputData: vi.fn(() => overrides.inputData ?? [{ json: { input: 'test' } }]),
|
||||
getNode: vi.fn(() => mock<INode>({ name: 'test searxng' })),
|
||||
getCredentials: vi
|
||||
.fn()
|
||||
.mockResolvedValue({ apiUrl: overrides.apiUrl ?? 'https://searx.example.com' }),
|
||||
getNodeParameter: vi.fn().mockReturnValue(overrides.options ?? {}),
|
||||
addInputData: vi.fn().mockReturnValue({ index: 0 }),
|
||||
addOutputData: vi.fn(),
|
||||
});
|
||||
ctx.helpers.httpRequest = httpRequest;
|
||||
return { ctx, httpRequest };
|
||||
}
|
||||
|
||||
describe('ToolSearXng', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
});
|
||||
|
||||
describe('supplyData', () => {
|
||||
it('should return a search tool that requests through the node request helper', async () => {
|
||||
const node = new ToolSearXng();
|
||||
const { ctx, httpRequest } = createContext();
|
||||
|
||||
const supplyDataResult = await node.supplyData.call(
|
||||
mock<ISupplyDataFunctions>({
|
||||
getNode: vi.fn(() => mock<INode>({ name: 'test searxng' })),
|
||||
getCredentials: vi.fn().mockResolvedValue({ apiUrl: 'https://searx.example.com' }),
|
||||
getNodeParameter: vi.fn().mockReturnValue({}),
|
||||
}),
|
||||
ctx as unknown as ISupplyDataFunctions,
|
||||
0,
|
||||
);
|
||||
|
||||
expect(supplyDataResult.response).toBeInstanceOf(SearxngSearch);
|
||||
const tool = supplyDataResult.response as DynamicTool;
|
||||
expect(tool).toBeInstanceOf(DynamicTool);
|
||||
expect(tool.name).toBe('searxng-search');
|
||||
|
||||
await tool.invoke('current events');
|
||||
|
||||
expect(httpRequest).toHaveBeenCalledWith({
|
||||
method: 'POST',
|
||||
url: 'https://searx.example.com/search',
|
||||
qs: {
|
||||
q: 'current events',
|
||||
format: 'json',
|
||||
pageno: 1,
|
||||
safesearch: 0,
|
||||
},
|
||||
headers: { Accept: 'application/json' },
|
||||
json: true,
|
||||
timeout: 15_000,
|
||||
});
|
||||
});
|
||||
|
||||
it('should apply configured options as query parameters', async () => {
|
||||
const node = new ToolSearXng();
|
||||
const { ctx, httpRequest } = createContext({
|
||||
options: { numResults: 1, pageNumber: 3, language: 'fr', safesearch: 2 },
|
||||
});
|
||||
|
||||
const supplyDataResult = await node.supplyData.call(
|
||||
ctx as unknown as ISupplyDataFunctions,
|
||||
0,
|
||||
);
|
||||
const tool = supplyDataResult.response as DynamicTool;
|
||||
const response = await tool.invoke('test');
|
||||
|
||||
expect(httpRequest).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
qs: {
|
||||
q: 'test',
|
||||
format: 'json',
|
||||
pageno: 3,
|
||||
language: 'fr',
|
||||
safesearch: 2,
|
||||
},
|
||||
}),
|
||||
);
|
||||
// numResults slices client-side
|
||||
expect(response).toBe(
|
||||
JSON.stringify({
|
||||
title: 'First result',
|
||||
link: 'https://example.com/1',
|
||||
snippet: 'First snippet',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should trim trailing slashes from the configured API URL', async () => {
|
||||
const node = new ToolSearXng();
|
||||
const { ctx, httpRequest } = createContext({ apiUrl: 'https://searx.example.com//' });
|
||||
|
||||
const supplyDataResult = await node.supplyData.call(
|
||||
ctx as unknown as ISupplyDataFunctions,
|
||||
0,
|
||||
);
|
||||
await (supplyDataResult.response as DynamicTool).invoke('test');
|
||||
|
||||
expect(httpRequest).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ url: 'https://searx.example.com/search' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('should map results to comma-joined JSON strings', async () => {
|
||||
const node = new ToolSearXng();
|
||||
const { ctx } = createContext();
|
||||
|
||||
const supplyDataResult = await node.supplyData.call(
|
||||
ctx as unknown as ISupplyDataFunctions,
|
||||
0,
|
||||
);
|
||||
const response = await (supplyDataResult.response as DynamicTool).invoke('test');
|
||||
|
||||
expect(response).toBe(
|
||||
[
|
||||
JSON.stringify({
|
||||
title: 'First result',
|
||||
link: 'https://example.com/1',
|
||||
snippet: 'First snippet',
|
||||
}),
|
||||
JSON.stringify({
|
||||
title: 'Second result',
|
||||
link: 'https://example.com/2',
|
||||
snippet: 'Second snippet',
|
||||
}),
|
||||
].join(','),
|
||||
);
|
||||
});
|
||||
|
||||
it('should fall back to answers, infoboxes and suggestions', async () => {
|
||||
const node = new ToolSearXng();
|
||||
|
||||
const cases: Array<{ httpResponse: unknown; expected: string }> = [
|
||||
{ httpResponse: { results: [], answers: ['42'] }, expected: '42' },
|
||||
{
|
||||
httpResponse: { results: [], answers: [], infoboxes: [{ content: '<b>info</b> box' }] },
|
||||
expected: 'info box',
|
||||
},
|
||||
{
|
||||
httpResponse: { results: [], answers: [], infoboxes: [], suggestions: ['a', 'b'] },
|
||||
expected: 'Suggestions: a, b',
|
||||
},
|
||||
{ httpResponse: {}, expected: 'No good results found.' },
|
||||
];
|
||||
|
||||
for (const { httpResponse, expected } of cases) {
|
||||
const { ctx } = createContext({ httpResponse });
|
||||
const supplyDataResult = await node.supplyData.call(
|
||||
ctx as unknown as ISupplyDataFunctions,
|
||||
0,
|
||||
);
|
||||
const response = await (supplyDataResult.response as DynamicTool).invoke('test');
|
||||
expect(response).toBe(expected);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('execute', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
});
|
||||
|
||||
it('should execute SearXNG search and return result', async () => {
|
||||
it('should execute a search per input item and return results', async () => {
|
||||
const node = new ToolSearXng();
|
||||
const inputData: INodeExecutionData[] = [
|
||||
{
|
||||
json: { query: 'artificial intelligence' },
|
||||
},
|
||||
];
|
||||
|
||||
const mockExecute = mock<IExecuteFunctions>({
|
||||
getInputData: vi.fn(() => inputData),
|
||||
getNode: vi.fn(() => mock<INode>({ name: 'test searxng' })),
|
||||
getCredentials: vi.fn().mockResolvedValue({ apiUrl: 'https://searx.example.com' }),
|
||||
getNodeParameter: vi.fn().mockReturnValue({}),
|
||||
const { ctx, httpRequest } = createContext({
|
||||
inputData: [{ json: { input: 'machine learning' } }, { json: { input: 'deep learning' } }],
|
||||
});
|
||||
|
||||
// Mock the SearxngSearch.invoke method
|
||||
const mockResult = 'Search results for artificial intelligence...';
|
||||
SearxngSearch.prototype.invoke = vi.fn().mockResolvedValue(mockResult);
|
||||
const result = await node.execute.call(ctx);
|
||||
|
||||
const result = await node.execute.call(mockExecute);
|
||||
expect(httpRequest).toHaveBeenCalledTimes(2);
|
||||
expect(httpRequest).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.objectContaining({ qs: expect.objectContaining({ q: 'machine learning' }) }),
|
||||
);
|
||||
expect(httpRequest).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.objectContaining({ qs: expect.objectContaining({ q: 'deep learning' }) }),
|
||||
);
|
||||
|
||||
const expectedResponse = [
|
||||
JSON.stringify({
|
||||
title: 'First result',
|
||||
link: 'https://example.com/1',
|
||||
snippet: 'First snippet',
|
||||
}),
|
||||
JSON.stringify({
|
||||
title: 'Second result',
|
||||
link: 'https://example.com/2',
|
||||
snippet: 'Second snippet',
|
||||
}),
|
||||
].join(',');
|
||||
|
||||
expect(result).toEqual([
|
||||
[
|
||||
{
|
||||
json: {
|
||||
response: mockResult,
|
||||
},
|
||||
pairedItem: {
|
||||
item: 0,
|
||||
},
|
||||
},
|
||||
{ json: { response: expectedResponse }, pairedItem: { item: 0 } },
|
||||
{ json: { response: expectedResponse }, pairedItem: { item: 1 } },
|
||||
],
|
||||
]);
|
||||
expect(SearxngSearch.prototype.invoke).toHaveBeenCalledWith({
|
||||
query: 'artificial intelligence',
|
||||
});
|
||||
});
|
||||
|
||||
it('should handle multiple input items', async () => {
|
||||
it('should throw when an input item has no query', async () => {
|
||||
const node = new ToolSearXng();
|
||||
const inputData: INodeExecutionData[] = [
|
||||
{
|
||||
json: { query: 'machine learning' },
|
||||
},
|
||||
{
|
||||
json: { query: 'deep learning' },
|
||||
},
|
||||
];
|
||||
const { ctx, httpRequest } = createContext({ inputData: [{ json: {} }] });
|
||||
|
||||
const mockExecute = mock<IExecuteFunctions>({
|
||||
getInputData: vi.fn(() => inputData),
|
||||
getNode: vi.fn(() => mock<INode>({ name: 'test searxng' })),
|
||||
getCredentials: vi.fn().mockResolvedValue({ apiUrl: 'https://searx.example.com' }),
|
||||
getNodeParameter: vi.fn().mockReturnValue({}),
|
||||
});
|
||||
|
||||
// Mock the SearxngSearch.invoke method
|
||||
SearxngSearch.prototype.invoke = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce('Machine learning search results')
|
||||
.mockResolvedValueOnce('Deep learning search results');
|
||||
|
||||
const result = await node.execute.call(mockExecute);
|
||||
|
||||
expect(result).toEqual([
|
||||
[
|
||||
{
|
||||
json: {
|
||||
response: 'Machine learning search results',
|
||||
},
|
||||
pairedItem: {
|
||||
item: 0,
|
||||
},
|
||||
},
|
||||
{
|
||||
json: {
|
||||
response: 'Deep learning search results',
|
||||
},
|
||||
pairedItem: {
|
||||
item: 1,
|
||||
},
|
||||
},
|
||||
],
|
||||
]);
|
||||
expect(SearxngSearch.prototype.invoke).toHaveBeenCalledTimes(2);
|
||||
await expect(node.execute.call(ctx)).rejects.toThrow('Input item is missing');
|
||||
expect(httpRequest).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should handle credentials and options correctly', async () => {
|
||||
it('should read credentials and options per item', async () => {
|
||||
const node = new ToolSearXng();
|
||||
const inputData: INodeExecutionData[] = [
|
||||
{
|
||||
json: { query: 'test query' },
|
||||
},
|
||||
];
|
||||
const { ctx } = createContext({ inputData: [{ json: { input: 'test query' } }] });
|
||||
|
||||
const testOptions = { engines: ['google'], safesearch: 1 };
|
||||
const mockExecute = mock<IExecuteFunctions>({
|
||||
getInputData: vi.fn(() => inputData),
|
||||
getNode: vi.fn(() => mock<INode>({ name: 'test searxng' })),
|
||||
getCredentials: vi.fn().mockResolvedValue({ apiUrl: 'https://searx.test.com' }),
|
||||
getNodeParameter: vi.fn().mockReturnValue(testOptions),
|
||||
});
|
||||
await node.execute.call(ctx);
|
||||
|
||||
SearxngSearch.prototype.invoke = vi.fn().mockResolvedValue('test result');
|
||||
|
||||
await node.execute.call(mockExecute);
|
||||
|
||||
expect(mockExecute.getCredentials).toHaveBeenCalledWith('searXngApi');
|
||||
expect(mockExecute.getNodeParameter).toHaveBeenCalledWith('options', 0);
|
||||
expect(ctx.getCredentials).toHaveBeenCalledWith('searXngApi');
|
||||
expect(ctx.getNodeParameter).toHaveBeenCalledWith('options', 0);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { SearxngSearch } from '@langchain/community/tools/searxng_search';
|
||||
import { NodeConnectionTypes } from 'n8n-workflow';
|
||||
import { DynamicTool } from '@langchain/core/tools';
|
||||
import { NodeConnectionTypes, NodeOperationError } from 'n8n-workflow';
|
||||
import type {
|
||||
IDataObject,
|
||||
IExecuteFunctions,
|
||||
INodeExecutionData,
|
||||
INodeType,
|
||||
|
|
@ -12,22 +13,84 @@ import type {
|
|||
import { logWrapper, getConnectionHintNoticeField } from '@n8n/ai-utilities';
|
||||
|
||||
type Options = {
|
||||
numResults: number;
|
||||
pageNumber: number;
|
||||
language: string;
|
||||
safesearch: 0 | 1 | 2;
|
||||
numResults?: number;
|
||||
pageNumber?: number;
|
||||
language?: string;
|
||||
safesearch?: 0 | 1 | 2;
|
||||
};
|
||||
|
||||
type SearXngResponse = {
|
||||
results?: Array<{ title?: string; url?: string; content?: string }>;
|
||||
answers?: string[];
|
||||
infoboxes?: Array<{ content?: string }>;
|
||||
suggestions?: string[];
|
||||
};
|
||||
|
||||
function stripHtml(html: string): string {
|
||||
let content = html;
|
||||
let previous: string;
|
||||
do {
|
||||
previous = content;
|
||||
content = content.replace(/<[^>]+>/gi, '');
|
||||
} while (content !== previous);
|
||||
return content;
|
||||
}
|
||||
|
||||
function formatResponse(data: SearXngResponse, numResults: number): string {
|
||||
const results = data.results ?? [];
|
||||
if (results.length) {
|
||||
return results
|
||||
.slice(0, numResults)
|
||||
.map((result) =>
|
||||
JSON.stringify({
|
||||
title: result.title ?? '',
|
||||
link: result.url ?? '',
|
||||
snippet: result.content ?? '',
|
||||
}),
|
||||
)
|
||||
.join(',');
|
||||
}
|
||||
|
||||
if (data.answers?.length) return data.answers[0];
|
||||
if (data.infoboxes?.length) return stripHtml(data.infoboxes[0]?.content ?? '');
|
||||
if (data.suggestions?.length) return `Suggestions: ${data.suggestions.join(', ')}`;
|
||||
|
||||
return 'No good results found.';
|
||||
}
|
||||
|
||||
async function getTool(ctx: ISupplyDataFunctions | IExecuteFunctions, itemIndex: number) {
|
||||
const credentials = await ctx.getCredentials<{ apiUrl: string }>('searXngApi');
|
||||
const options = ctx.getNodeParameter('options', itemIndex) as Options;
|
||||
|
||||
return new SearxngSearch({
|
||||
apiBase: credentials.apiUrl,
|
||||
headers: {
|
||||
Accept: 'application/json',
|
||||
const apiBase = credentials.apiUrl.replace(/\/+$/, '');
|
||||
const numResults = options.numResults ?? 10;
|
||||
|
||||
const qs: IDataObject = {
|
||||
format: 'json',
|
||||
pageno: options.pageNumber ?? 1,
|
||||
safesearch: options.safesearch ?? 0,
|
||||
};
|
||||
if (options.language) qs.language = options.language;
|
||||
|
||||
return new DynamicTool({
|
||||
name: 'searxng-search',
|
||||
description:
|
||||
'A meta search engine. Useful for when you need to answer questions about current events. Input should be a search query. Output is a JSON array of the query results',
|
||||
func: async (query: string) => {
|
||||
// Routed through the node request helper so outbound policy applies.
|
||||
const response = (await ctx.helpers.httpRequest({
|
||||
method: 'POST',
|
||||
url: `${apiBase}/search`,
|
||||
qs: { q: query, ...qs },
|
||||
headers: { Accept: 'application/json' },
|
||||
json: true,
|
||||
// Search responses are short-lived; don't let a hung instance
|
||||
// stall the agent turn for the transport default (5 minutes).
|
||||
timeout: 15_000,
|
||||
})) as SearXngResponse;
|
||||
|
||||
return formatResponse(response, numResults);
|
||||
},
|
||||
params: options,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -132,9 +195,17 @@ export class ToolSearXng implements INodeType {
|
|||
for (let i = 0; i < input.length; i++) {
|
||||
const item = input[i];
|
||||
const tool = await getTool(this, i);
|
||||
|
||||
const query = item.json.input;
|
||||
if (typeof query !== 'string' || query === '') {
|
||||
throw new NodeOperationError(this.getNode(), 'Input item is missing', {
|
||||
itemIndex: i,
|
||||
});
|
||||
}
|
||||
|
||||
result.push({
|
||||
json: {
|
||||
response: await tool.invoke(item.json),
|
||||
response: await tool.invoke(query),
|
||||
},
|
||||
pairedItem: {
|
||||
item: i,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/n8n-nodes-langchain",
|
||||
"version": "2.34.0",
|
||||
"version": "2.34.1",
|
||||
"description": "",
|
||||
"main": "index.js",
|
||||
"exports": {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/scheduler",
|
||||
"version": "1.3.0",
|
||||
"version": "1.3.1",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
"dev": "pnpm watch",
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/task-runner",
|
||||
"version": "2.34.0",
|
||||
"version": "2.34.1",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
"start": "node dist/start.js",
|
||||
|
|
|
|||
|
|
@ -40,6 +40,7 @@ import { isErrorLike } from './errors/error-like';
|
|||
import { ExecutionError } from './errors/execution-error';
|
||||
import { makeSerializable } from './errors/serializable-error';
|
||||
import { TimeoutError } from './errors/timeout-error';
|
||||
import { freezeGlobals } from './prototype-hardening';
|
||||
import type { RequireResolver } from './require-resolver';
|
||||
import { createRequireResolver } from './require-resolver';
|
||||
import { DataRequestResponseReconstruct } from '../data-request/data-request-response-reconstruct';
|
||||
|
|
@ -88,10 +89,6 @@ export interface JsTaskData {
|
|||
additionalData: PartialAdditionalData;
|
||||
}
|
||||
|
||||
type GlobalFunctionWithPrototype = ((...args: unknown[]) => unknown) & {
|
||||
prototype?: object;
|
||||
};
|
||||
|
||||
type CustomConsole = {
|
||||
log: (...args: unknown[]) => void;
|
||||
};
|
||||
|
|
@ -190,17 +187,8 @@ export class JsTaskRunner extends TaskRunner {
|
|||
Buffer.allocUnsafe = safeAlloc as typeof Buffer.allocUnsafe;
|
||||
Buffer.allocUnsafeSlow = safeAlloc as typeof Buffer.allocUnsafeSlow;
|
||||
|
||||
// Freeze globals, except in tests because Vitest needs to be able to mutate prototypes
|
||||
if (process.env.NODE_ENV !== 'test') {
|
||||
Object.getOwnPropertyNames(globalThis)
|
||||
.map((name) => Reflect.get(globalThis, name) as unknown)
|
||||
.filter((value): value is GlobalFunctionWithPrototype => typeof value === 'function')
|
||||
.forEach((fn) => {
|
||||
if (typeof fn.prototype === 'object') Object.freeze(fn.prototype);
|
||||
Object.freeze(fn);
|
||||
});
|
||||
|
||||
[Reflect, JSON, Math].forEach(Object.freeze);
|
||||
freezeGlobals();
|
||||
}
|
||||
|
||||
// Freeze internal classes
|
||||
|
|
|
|||
|
|
@ -0,0 +1,66 @@
|
|||
import { EventEmitter } from 'node:events';
|
||||
|
||||
type FunctionWithPrototype = ((...args: unknown[]) => unknown) & {
|
||||
prototype?: object;
|
||||
};
|
||||
|
||||
const isFreezable = (value: unknown): value is object =>
|
||||
typeof value === 'object' || typeof value === 'function';
|
||||
|
||||
/** Freeze a function and its prototype. */
|
||||
const freezeFunctionAndPrototype = (fn: FunctionWithPrototype) => {
|
||||
if (isFreezable(fn.prototype)) {
|
||||
Object.freeze(fn.prototype);
|
||||
}
|
||||
Object.freeze(fn);
|
||||
};
|
||||
|
||||
/**
|
||||
* `EventEmitter.prototype` members that user code must not override.
|
||||
* `process` inherits these from `EventEmitter.prototype`,
|
||||
* which lives in an internal module and never appears on `globalThis`,
|
||||
* so the global freeze misses it.
|
||||
*/
|
||||
const PROTECTED_EVENT_EMITTER_METHODS = [
|
||||
'emit',
|
||||
'on',
|
||||
'once',
|
||||
'addListener',
|
||||
'prependListener',
|
||||
'prependOnceListener',
|
||||
] as const;
|
||||
|
||||
/**
|
||||
* Lock {@link PROTECTED_EVENT_EMITTER_METHODS} against reassignment.
|
||||
* Locked individually rather than by freezing the prototype,
|
||||
* because Node lazily copies these names onto stream subclass prototypes (`net`, `tls`, `http`, etc)
|
||||
* and a wholesale freeze breaks those assignments.
|
||||
* Kept enumerable to match Node's default, so hiding them from `for...in`,
|
||||
* `Object.keys` and spreads does not alter every emitter in the process.
|
||||
*/
|
||||
const lockEventEmitterMethods = () => {
|
||||
for (const method of PROTECTED_EVENT_EMITTER_METHODS) {
|
||||
Object.defineProperty(EventEmitter.prototype, method, {
|
||||
value: EventEmitter.prototype[method],
|
||||
writable: false,
|
||||
configurable: false,
|
||||
enumerable: true,
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Freeze the globals and shared prototypes reachable from sandboxed code so it
|
||||
* cannot mutate them to escape secure mode.
|
||||
* Skip in tests, where Vitest needs to mutate prototypes.
|
||||
*/
|
||||
export const freezeGlobals = () => {
|
||||
Object.getOwnPropertyNames(globalThis)
|
||||
.map((name) => Reflect.get(globalThis, name) as unknown)
|
||||
.filter((value): value is FunctionWithPrototype => typeof value === 'function')
|
||||
.forEach(freezeFunctionAndPrototype);
|
||||
|
||||
[Reflect, JSON, Math].forEach(Object.freeze);
|
||||
|
||||
lockEventEmitterMethods();
|
||||
};
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/telemetry",
|
||||
"version": "1.2.0",
|
||||
"version": "1.2.1",
|
||||
"scripts": {
|
||||
"clean": "rimraf dist .turbo",
|
||||
"dev": "pnpm watch",
|
||||
|
|
|
|||
|
|
@ -12,4 +12,15 @@ export const PLATFORM_TELEMETRY = defineTelemetryEvents({
|
|||
variant: z.union([z.string(), z.boolean()]).describe('Variant assigned to the user'),
|
||||
}),
|
||||
},
|
||||
TASK_RUNNER_DISCONNECTED: {
|
||||
name: 'Task runner disconnected',
|
||||
description:
|
||||
'The broker tore down a task runner connection because the runner was unhealthy: it failed a heartbeat check, or was reported unresponsive after it stopped acknowledging or offering tasks. Normal shutdowns are not reported.',
|
||||
properties: z.object({
|
||||
reason: z
|
||||
.enum(['failed-heartbeat-check', 'runner-unresponsive'])
|
||||
.describe('Which detection reported the runner as unhealthy'),
|
||||
mode: z.enum(['internal', 'external']).describe('Task runners deployment mode'),
|
||||
}),
|
||||
},
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@n8n/workflow-sdk",
|
||||
"version": "0.27.0",
|
||||
"version": "0.27.1",
|
||||
"description": "TypeScript SDK for programmatically creating n8n workflows",
|
||||
"exports": {
|
||||
".": {
|
||||
|
|
|
|||
|
|
@ -1,3 +1,7 @@
|
|||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync, symlinkSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import * as path from 'path';
|
||||
|
||||
import {
|
||||
validateNodeConfig,
|
||||
loadSchema,
|
||||
|
|
@ -165,6 +169,126 @@ describe('schema-validator', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('path component handling', () => {
|
||||
it.each([
|
||||
'n8n-nodes-base.foo/bar',
|
||||
'n8n-nodes-base.foo\\bar',
|
||||
'foo/bar.set',
|
||||
'n8n-nodes-base.foo\0bar',
|
||||
])('returns null for node types containing path separators (%s)', (nodeType) => {
|
||||
setSchemaBaseDirs(originalBaseDirs);
|
||||
expect(loadSchema(nodeType, 1)).toBeNull();
|
||||
});
|
||||
|
||||
it.each([
|
||||
// "base..." -> nodeName "..", "base.." -> nodeName "."
|
||||
'n8n-nodes-base...',
|
||||
'n8n-nodes-base..',
|
||||
])('returns null for node types with parent/current-dir components (%s)', (nodeType) => {
|
||||
setSchemaBaseDirs(originalBaseDirs);
|
||||
expect(loadSchema(nodeType, 1)).toBeNull();
|
||||
});
|
||||
|
||||
it('does not resolve a module from outside the schema base directory', () => {
|
||||
const tmpRoot = mkdtempSync(path.join(tmpdir(), 'sdk-schema-'));
|
||||
try {
|
||||
const baseDir = path.join(tmpRoot, 'base');
|
||||
mkdirSync(baseDir, { recursive: true });
|
||||
|
||||
// Plant a module where a "../" walk from <baseDir>/nodes/x would land.
|
||||
writeFileSync(
|
||||
path.join(tmpRoot, 'v1.schema.js'),
|
||||
'module.exports = function () { throw new Error("EXECUTED"); };',
|
||||
);
|
||||
|
||||
setSchemaBaseDirs([baseDir]);
|
||||
|
||||
// Count the "../" steps needed to climb from <baseDir>/nodes/x back to tmpRoot,
|
||||
// then build the type the same way an attacker would ("x." + "../" * n).
|
||||
const relative = path.relative(path.join(baseDir, 'nodes', 'x'), tmpRoot);
|
||||
const steps = relative.split(path.sep).filter((segment) => segment === '..').length;
|
||||
const nodeType = 'x.' + '../'.repeat(steps);
|
||||
|
||||
expect(loadSchema(nodeType, 1)).toBeNull();
|
||||
// The planted module must never be required or invoked.
|
||||
const result = validateNodeConfig(nodeType, 1, { parameters: {} });
|
||||
expect(result.valid).toBe(true);
|
||||
expect(result.errors).toEqual([]);
|
||||
} finally {
|
||||
rmSync(tmpRoot, { recursive: true, force: true });
|
||||
setSchemaBaseDirs(originalBaseDirs);
|
||||
}
|
||||
});
|
||||
|
||||
it('does not load a module through a symlinked directory in the schema tree', () => {
|
||||
const tmpRoot = mkdtempSync(path.join(tmpdir(), 'sdk-schema-'));
|
||||
try {
|
||||
const nodesDir = path.join(tmpRoot, 'base', 'nodes');
|
||||
mkdirSync(nodesDir, { recursive: true });
|
||||
|
||||
// Plant a module outside the schema tree, then point a symlinked
|
||||
// package directory inside `nodes/` at it.
|
||||
const outside = path.join(tmpRoot, 'outside', 'evil');
|
||||
mkdirSync(path.join(outside, 'set'), { recursive: true });
|
||||
writeFileSync(
|
||||
path.join(outside, 'set', 'v1.schema.js'),
|
||||
'module.exports = function () { throw new Error("EXECUTED"); };',
|
||||
);
|
||||
|
||||
try {
|
||||
symlinkSync(outside, path.join(nodesDir, 'evil'), 'dir');
|
||||
} catch {
|
||||
return; // Platform doesn't permit symlink creation (e.g. Windows CI) — skip.
|
||||
}
|
||||
|
||||
setSchemaBaseDirs([path.join(tmpRoot, 'base')]);
|
||||
|
||||
// "evil.set" would resolve to nodes/evil/set/v1.schema — inside the tree
|
||||
// lexically, but only reachable by following the symlink out.
|
||||
expect(loadSchema('evil.set', 1)).toBeNull();
|
||||
const result = validateNodeConfig('evil.set', 1, { parameters: {} });
|
||||
expect(result.valid).toBe(true);
|
||||
expect(result.errors).toEqual([]);
|
||||
} finally {
|
||||
rmSync(tmpRoot, { recursive: true, force: true });
|
||||
setSchemaBaseDirs(originalBaseDirs);
|
||||
}
|
||||
});
|
||||
|
||||
it('does not load a module through a symlinked schema file when all parent dirs are real', () => {
|
||||
const tmpRoot = mkdtempSync(path.join(tmpdir(), 'sdk-schema-leaf-'));
|
||||
try {
|
||||
// Every directory in the chain is real — mimics a legitimately installed node package.
|
||||
const nodeDir = path.join(tmpRoot, 'base', 'nodes', 'n8n-nodes-base', 'httpRequest');
|
||||
mkdirSync(nodeDir, { recursive: true });
|
||||
|
||||
// Attacker-controlled file, living outside the schema tree entirely.
|
||||
const outsideFile = path.join(tmpRoot, 'outside.schema.js');
|
||||
writeFileSync(
|
||||
outsideFile,
|
||||
'module.exports = function () { throw new Error("EXECUTED-VIA-LEAF-SYMLINK"); };',
|
||||
);
|
||||
|
||||
// Only the leaf FILE is a symlink — nothing above it is.
|
||||
try {
|
||||
symlinkSync(outsideFile, path.join(nodeDir, 'v1.schema.js'), 'file');
|
||||
} catch {
|
||||
return; // Platform doesn't permit symlink creation (e.g. Windows CI) — skip.
|
||||
}
|
||||
|
||||
setSchemaBaseDirs([path.join(tmpRoot, 'base')]);
|
||||
|
||||
expect(loadSchema('n8n-nodes-base.httpRequest', 1)).toBeNull();
|
||||
const result = validateNodeConfig('n8n-nodes-base.httpRequest', 1, { parameters: {} });
|
||||
expect(result.valid).toBe(true);
|
||||
expect(result.errors).toEqual([]);
|
||||
} finally {
|
||||
rmSync(tmpRoot, { recursive: true, force: true });
|
||||
setSchemaBaseDirs(originalBaseDirs);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('setSchemaBaseDirs', () => {
|
||||
it('allows setting custom schema base dirs', () => {
|
||||
const customDirs = ['/custom/path/to/schemas'];
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@
|
|||
* Schemas are loaded from `nodeDefinitionDirs` configured via `setSchemaBaseDirs()`.
|
||||
*/
|
||||
|
||||
import { realpathSync } from 'fs';
|
||||
import * as path from 'path';
|
||||
import type { ZodSchema, ZodIssue } from 'zod';
|
||||
|
||||
|
|
@ -64,6 +65,31 @@ function nodeTypeToPathComponents(nodeType: string): { pkg: string; nodeName: st
|
|||
return { pkg, nodeName };
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a node-type-derived path component is a single, safe path segment.
|
||||
* A legitimate `pkg`/`nodeName` never contains separators or parent references.
|
||||
*/
|
||||
function isSafePathComponent(component: string): boolean {
|
||||
if (component.includes('\0')) return false;
|
||||
if (component.includes('/') || component.includes('\\')) return false;
|
||||
if (component === '.' || component === '..') return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Real (symlink-resolved) path of `child` if it lands on `parent` itself or
|
||||
* somewhere beneath it, otherwise null. Comparing fully resolved paths means
|
||||
* neither a `..` sequence nor a symlinked directory or file can point outside
|
||||
* `parent`. Both paths must already exist. Callers should use the returned path
|
||||
* so the resolved real file is what gets loaded.
|
||||
*/
|
||||
function realPathWithin(parent: string, child: string): string | null {
|
||||
const realParent = realpathSync(parent);
|
||||
const realChild = realpathSync(child);
|
||||
const contained = realChild === realParent || realChild.startsWith(realParent + path.sep);
|
||||
return contained ? realChild : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Convert version number to version string for file path
|
||||
*
|
||||
|
|
@ -132,13 +158,23 @@ function buildExpectedFactoryName(
|
|||
}
|
||||
|
||||
/**
|
||||
* Try to load a schema module from a given path
|
||||
* @returns The module if found, null otherwise
|
||||
* Try to load a schema module from a given path, but only if the file `require()`
|
||||
* would actually open resolves inside `nodesRoot`. Resolving the module first
|
||||
* (with its real extension, following any symlinks) and checking containment on
|
||||
* the realpath closes both `..` traversal and symlinked directory/leaf escapes.
|
||||
* @returns The module if found and contained, null otherwise
|
||||
*/
|
||||
function tryLoadSchemaModule(schemaPath: string): Record<string, unknown> | null {
|
||||
function tryLoadSchemaModule(
|
||||
schemaPath: string,
|
||||
nodesRoot: string,
|
||||
): Record<string, unknown> | null {
|
||||
try {
|
||||
const safePath = realPathWithin(nodesRoot, require.resolve(schemaPath));
|
||||
if (!safePath) {
|
||||
return null;
|
||||
}
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports -- Dynamic module loading requires CommonJS require
|
||||
return require(schemaPath) as Record<string, unknown>;
|
||||
return require(safePath) as Record<string, unknown>;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
|
|
@ -214,15 +250,25 @@ function tryLoadSchemaForNodeType(nodeType: string, version: number): SchemaOrFa
|
|||
const versionStr = versionToString(version);
|
||||
const isLangchain = pkg === 'n8n-nodes-langchain';
|
||||
|
||||
for (const baseDir of schemaBaseDirs) {
|
||||
// Try flat structure first: nodes/{pkg}/{nodeName}/{version}.schema.js
|
||||
const flatSchemaPath = path.join(baseDir, 'nodes', pkg, nodeName, `${versionStr}.schema`);
|
||||
// Try split structure: nodes/{pkg}/{nodeName}/{version}/index.schema.js
|
||||
const splitSchemaPath = path.join(baseDir, 'nodes', pkg, nodeName, versionStr, 'index.schema');
|
||||
// Reject node types whose path components could escape the schema directory.
|
||||
// Only built-in nodes have generated schemas, so a rejected type would resolve
|
||||
// to nothing anyway; returning null falls back to skip-validation.
|
||||
if (!isSafePathComponent(pkg) || !isSafePathComponent(nodeName)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Try flat structure first, then split structure
|
||||
for (const baseDir of schemaBaseDirs) {
|
||||
const nodesRoot = path.resolve(baseDir, 'nodes');
|
||||
// Try flat structure first: nodes/{pkg}/{nodeName}/{version}.schema.js
|
||||
const flatSchemaPath = path.join(nodesRoot, pkg, nodeName, `${versionStr}.schema`);
|
||||
// Try split structure: nodes/{pkg}/{nodeName}/{version}/index.schema.js
|
||||
const splitSchemaPath = path.join(nodesRoot, pkg, nodeName, versionStr, 'index.schema');
|
||||
|
||||
// Try flat structure first, then split structure. `tryLoadSchemaModule`
|
||||
// refuses to require() a file that resolves outside the schema root.
|
||||
const schemaModule =
|
||||
tryLoadSchemaModule(flatSchemaPath) ?? tryLoadSchemaModule(splitSchemaPath);
|
||||
tryLoadSchemaModule(flatSchemaPath, nodesRoot) ??
|
||||
tryLoadSchemaModule(splitSchemaPath, nodesRoot);
|
||||
|
||||
if (!schemaModule) {
|
||||
continue;
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "n8n",
|
||||
"version": "2.34.0",
|
||||
"version": "2.34.2",
|
||||
"description": "n8n Workflow Automation Tool",
|
||||
"main": "dist/index",
|
||||
"types": "dist/index.d.ts",
|
||||
|
|
|
|||
|
|
@ -77,16 +77,105 @@ describe('RoleController', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('should pass the reassignment role through to the service', async () => {
|
||||
const request = managerRequest();
|
||||
roleService.getRole.mockResolvedValue({ roleType: 'global' } as Role);
|
||||
roleService.removeCustomRole.mockResolvedValue({ slug: 'custom-editor' } as Role);
|
||||
it('should pass the reassignment role through to the service for an entitled caller', async () => {
|
||||
const request = mock<AuthenticatedRequest>({
|
||||
user: {
|
||||
id: '123',
|
||||
role: {
|
||||
slug: 'global:admin',
|
||||
scopes: [{ slug: 'role:manage' }, { slug: 'user:changeRole' }],
|
||||
},
|
||||
},
|
||||
});
|
||||
roleService.getRole.mockResolvedValue({
|
||||
slug: 'global:custom-editor',
|
||||
roleType: 'global',
|
||||
} as Role);
|
||||
roleService.removeCustomRole.mockResolvedValue({ slug: 'global:custom-editor' } as Role);
|
||||
|
||||
await controller.deleteRole(request, mock(), 'custom-editor', {
|
||||
await controller.deleteRole(request, mock(), 'global:custom-editor', {
|
||||
reassignRoleSlug: 'global:member',
|
||||
});
|
||||
|
||||
expect(roleService.removeCustomRole).toHaveBeenCalledWith('custom-editor', 'global:member');
|
||||
expect(roleService.removeCustomRole).toHaveBeenCalledWith(
|
||||
'global:custom-editor',
|
||||
'global:member',
|
||||
);
|
||||
});
|
||||
|
||||
it('should ignore the reassignment role when the caller lacks user:changeRole', async () => {
|
||||
const request = mock<AuthenticatedRequest>({
|
||||
user: {
|
||||
id: '123',
|
||||
role: { slug: 'global:admin', scopes: [{ slug: 'role:manage' }] },
|
||||
},
|
||||
});
|
||||
roleService.getRole.mockResolvedValue({
|
||||
slug: 'global:custom-editor',
|
||||
roleType: 'global',
|
||||
} as Role);
|
||||
roleService.removeCustomRole.mockResolvedValue({ slug: 'global:custom-editor' } as Role);
|
||||
|
||||
await controller.deleteRole(request, mock(), 'global:custom-editor', {
|
||||
reassignRoleSlug: 'global:member',
|
||||
});
|
||||
|
||||
expect(roleService.removeCustomRole).toHaveBeenCalledWith(
|
||||
'global:custom-editor',
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it('should ignore the reassignment role when the caller holds the role being deleted', async () => {
|
||||
const request = mock<AuthenticatedRequest>({
|
||||
user: {
|
||||
id: '123',
|
||||
role: {
|
||||
slug: 'global:custom-editor',
|
||||
scopes: [{ slug: 'role:manage' }, { slug: 'user:changeRole' }],
|
||||
},
|
||||
},
|
||||
});
|
||||
roleService.getRole.mockResolvedValue({
|
||||
slug: 'global:custom-editor',
|
||||
roleType: 'global',
|
||||
} as Role);
|
||||
roleService.removeCustomRole.mockResolvedValue({ slug: 'global:custom-editor' } as Role);
|
||||
|
||||
await controller.deleteRole(request, mock(), 'global:custom-editor', {
|
||||
reassignRoleSlug: 'global:member',
|
||||
});
|
||||
|
||||
expect(roleService.removeCustomRole).toHaveBeenCalledWith(
|
||||
'global:custom-editor',
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it('should ignore the reassignment role for project roles', async () => {
|
||||
const request = mock<AuthenticatedRequest>({
|
||||
user: {
|
||||
id: '123',
|
||||
role: {
|
||||
slug: 'global:admin',
|
||||
scopes: [{ slug: 'role:manage' }, { slug: 'user:changeRole' }],
|
||||
},
|
||||
},
|
||||
});
|
||||
roleService.getRole.mockResolvedValue({
|
||||
slug: 'project:custom-editor',
|
||||
roleType: 'project',
|
||||
} as Role);
|
||||
roleService.removeCustomRole.mockResolvedValue({ slug: 'project:custom-editor' } as Role);
|
||||
|
||||
await controller.deleteRole(request, mock(), 'project:custom-editor', {
|
||||
reassignRoleSlug: 'project:admin',
|
||||
});
|
||||
|
||||
expect(roleService.removeCustomRole).toHaveBeenCalledWith(
|
||||
'project:custom-editor',
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { Logger } from '@n8n/backend-common';
|
||||
import type { SsrfBridge } from '@n8n/backend-network';
|
||||
import { mockInstance } from '@n8n/backend-test-utils';
|
||||
import type { ClientOAuth2 } from '@n8n/client-oauth2';
|
||||
import { type CredentialsEntity, type User } from '@n8n/db';
|
||||
|
|
@ -150,6 +151,86 @@ describe('OAuth2CredentialController', () => {
|
|||
expect(externalHooks.run).toHaveBeenCalledWith('oauth2.callback', expect.any(Array));
|
||||
});
|
||||
|
||||
describe('outbound network policy', () => {
|
||||
const runCallback = async () => {
|
||||
const { ClientOAuth2 } = await import('@n8n/client-oauth2');
|
||||
const mockGetToken = vi.fn().mockResolvedValue({
|
||||
data: { access_token: 'new_token', refresh_token: 'refresh_token' },
|
||||
});
|
||||
vi.mocked(ClientOAuth2).mockImplementation(function () {
|
||||
return { code: { getToken: mockGetToken } } as unknown as ClientOAuth2;
|
||||
});
|
||||
|
||||
oauthService.resolveCredential.mockResolvedValueOnce([
|
||||
mock<CredentialsEntity>({ id: '1' }),
|
||||
{ csrfSecret: 'csrf-secret' },
|
||||
{
|
||||
clientId: 'client_id',
|
||||
clientSecret: 'client_secret',
|
||||
authUrl: 'https://example.domain/oauth2/auth',
|
||||
accessTokenUrl: 'https://example.domain/oauth2/token',
|
||||
scope: 'openid',
|
||||
grantType: 'authorizationCode',
|
||||
authentication: 'header',
|
||||
},
|
||||
{
|
||||
token: 'token',
|
||||
cid: '1',
|
||||
userId: '123',
|
||||
origin: 'static-credential' as const,
|
||||
createdAt: timestamp,
|
||||
data: 'encrypted-data',
|
||||
},
|
||||
{ csrfSecret: 'csrf-secret' },
|
||||
]);
|
||||
oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential');
|
||||
externalHooks.run.mockResolvedValue(undefined);
|
||||
|
||||
const req = mock<OAuthRequest.OAuth2Credential.Callback>({
|
||||
query: { code: 'auth_code', state: validState },
|
||||
originalUrl: '/oauth2-credential/callback?code=auth_code&state=state',
|
||||
});
|
||||
|
||||
await controller.handleCallback(req, res);
|
||||
|
||||
return ClientOAuth2;
|
||||
};
|
||||
|
||||
it('should hand the bridge to the client when the service supplies one', async () => {
|
||||
const bridge = mock<SsrfBridge>();
|
||||
oauthService.getSsrfBridge.mockReturnValue(bridge);
|
||||
|
||||
const ClientOAuth2 = await runCallback();
|
||||
|
||||
expect(ClientOAuth2).toHaveBeenCalledWith(expect.objectContaining({ ssrfBridge: bridge }));
|
||||
expect(res.render).toHaveBeenCalledWith('oauth-callback');
|
||||
});
|
||||
|
||||
it('should build the client without a bridge when the service supplies none', async () => {
|
||||
oauthService.getSsrfBridge.mockReturnValue(undefined);
|
||||
|
||||
const ClientOAuth2 = await runCallback();
|
||||
|
||||
expect(ClientOAuth2).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ ssrfBridge: undefined }),
|
||||
);
|
||||
// The exchange must still complete, so instances with the guard off are unaffected.
|
||||
expect(res.render).toHaveBeenCalledWith('oauth-callback');
|
||||
});
|
||||
|
||||
it('should keep the bridge out of the external hook payload', async () => {
|
||||
oauthService.getSsrfBridge.mockReturnValue(mock<SsrfBridge>());
|
||||
|
||||
await runCallback();
|
||||
|
||||
const callbackCall = externalHooks.run.mock.calls.find(
|
||||
([hookName]) => hookName === 'oauth2.callback',
|
||||
);
|
||||
const [hookOptions] = callbackCall![1] as unknown as unknown[];
|
||||
expect(hookOptions).not.toHaveProperty('ssrfBridge');
|
||||
});
|
||||
});
|
||||
|
||||
it('should build the client with a certificate when certificate authentication is selected', async () => {
|
||||
const { ClientOAuth2 } = await import('@n8n/client-oauth2');
|
||||
const mockGetToken = vi.fn().mockResolvedValue({
|
||||
|
|
|
|||
|
|
@ -84,7 +84,12 @@ export class OAuth2CredentialController {
|
|||
|
||||
await this.externalHooks.run('oauth2.callback', [oAuthOptions]);
|
||||
|
||||
const oAuthObj = new ClientOAuth2(oAuthOptions);
|
||||
// The bridge is attached here rather than in the options object so it stays out of
|
||||
// the payload handed to external hooks.
|
||||
const oAuthObj = new ClientOAuth2({
|
||||
...oAuthOptions,
|
||||
ssrfBridge: this.oauthService.getSsrfBridge(),
|
||||
});
|
||||
|
||||
const queryParameters = req.originalUrl.split('?').splice(1, 1).join('');
|
||||
|
||||
|
|
|
|||
|
|
@ -47,6 +47,23 @@ export class RoleController {
|
|||
throw new ForbiddenError(RESPONSE_ERROR_MESSAGES.MISSING_SCOPE);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reassigning a deleted role's users is effectively a bulk instance-role change,
|
||||
* so it is only honored for callers entitled to change users' instance roles
|
||||
* (the same scopes the dedicated role-change endpoint requires), never for
|
||||
* project roles, and never for the caller's own role. When it is not honored the
|
||||
* reassignment target is ignored, so a role that still has users cannot be
|
||||
* deleted.
|
||||
*/
|
||||
private canReassignUsers(user: User, role: RoleDTO): boolean {
|
||||
return (
|
||||
role.roleType === 'global' &&
|
||||
user.role.slug !== role.slug &&
|
||||
hasGlobalScope(user, 'role:manage') &&
|
||||
hasGlobalScope(user, 'user:changeRole')
|
||||
);
|
||||
}
|
||||
|
||||
@Get('/')
|
||||
async getAllRoles(
|
||||
_req: AuthenticatedRequest,
|
||||
|
|
@ -137,7 +154,10 @@ export class RoleController {
|
|||
): Promise<RoleDTO> {
|
||||
const role = await this.roleService.getRole(slug);
|
||||
this.assertCanManageRoleType(req.user, role.roleType);
|
||||
const result = await this.roleService.removeCustomRole(slug, query.reassignRoleSlug);
|
||||
const reassignRoleSlug = this.canReassignUsers(req.user, role)
|
||||
? query.reassignRoleSlug
|
||||
: undefined;
|
||||
const result = await this.roleService.removeCustomRole(slug, reassignRoleSlug);
|
||||
this.eventService.emit('custom-role-deleted', {
|
||||
userId: req.user.id,
|
||||
roleSlug: result.slug,
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ import type { License } from '@/license';
|
|||
import { OtelConfig } from '@/modules/otel/otel.config';
|
||||
import type { NodeTypes } from '@/node-types';
|
||||
import type { Telemetry } from '@/telemetry';
|
||||
import { TELEMETRY_EVENT } from '@n8n/telemetry';
|
||||
|
||||
const flushPromises = async () => await new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
|
|
@ -3753,4 +3754,23 @@ describe('TelemetryEventRelay', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('runner events', () => {
|
||||
it('should track on `runner-disconnected` event', () => {
|
||||
const event: RelayEventMap['runner-disconnected'] = {
|
||||
reason: 'failed-heartbeat-check',
|
||||
mode: 'internal',
|
||||
};
|
||||
|
||||
eventService.emit('runner-disconnected', event);
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
TELEMETRY_EVENT.PLATFORM.TASK_RUNNER_DISCONNECTED,
|
||||
{
|
||||
reason: 'failed-heartbeat-check',
|
||||
mode: 'internal',
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -993,6 +993,11 @@ export type RelayEventMap = {
|
|||
executionId: string;
|
||||
};
|
||||
|
||||
'runner-disconnected': {
|
||||
reason: 'failed-heartbeat-check' | 'runner-unresponsive';
|
||||
mode: 'internal' | 'external';
|
||||
};
|
||||
|
||||
// #endregion
|
||||
|
||||
// #region queue
|
||||
|
|
|
|||
|
|
@ -41,6 +41,7 @@ import { NodeTypes } from '@/node-types';
|
|||
|
||||
import { EventRelay } from './event-relay';
|
||||
import { Telemetry } from '../../telemetry';
|
||||
import { TELEMETRY_EVENT } from '@n8n/telemetry';
|
||||
|
||||
// Max size for node_graph_string to avoid exceeding telemetry payload limits (32 KB), leaving room for other fields
|
||||
const MAX_NODE_GRAPH_STRING_SIZE = 24 * 1024;
|
||||
|
|
@ -213,6 +214,7 @@ export class TelemetryEventRelay extends EventRelay {
|
|||
'instance-ai-mcp-registry-connection-deleted': (event) =>
|
||||
this.instanceAiMcpRegistryConnectionDeleted(event),
|
||||
'hitl-response-actioned': (event) => this.hitlResponseActioned(event),
|
||||
'runner-disconnected': (event) => this.runnerDisconnected(event),
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -1675,6 +1677,10 @@ export class TelemetryEventRelay extends EventRelay {
|
|||
this.telemetry.track('User instance stopped');
|
||||
}
|
||||
|
||||
private runnerDisconnected({ reason, mode }: RelayEventMap['runner-disconnected']) {
|
||||
this.telemetry.track(TELEMETRY_EVENT.PLATFORM.TASK_RUNNER_DISCONNECTED, { reason, mode });
|
||||
}
|
||||
|
||||
private async instanceOwnerSetup({ userId }: RelayEventMap['instance-owner-setup']) {
|
||||
// Attach owner to instance group on telemetry
|
||||
this.telemetry.groupIdentify({
|
||||
|
|
|
|||
|
|
@ -845,6 +845,42 @@ describe('buildFromJson()', () => {
|
|||
expect(getLocalToolNames(agent)).toContain('web_search');
|
||||
});
|
||||
|
||||
it('routes fallback SearXNG search through the injected webSearchFetch', async () => {
|
||||
const webSearchFetch = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({ results: [] }),
|
||||
});
|
||||
const credentialProvider = {
|
||||
resolve: vi.fn().mockResolvedValue({ apiUrl: 'http://searxng.internal:8080' }),
|
||||
list: vi.fn().mockResolvedValue([]),
|
||||
};
|
||||
|
||||
const agent = await buildFromJson(
|
||||
makeConfig({
|
||||
model: 'deepseek/deepseek-chat',
|
||||
config: { webSearch: { enabled: true, provider: 'searxng', credential: 'searxng-url' } },
|
||||
}),
|
||||
{},
|
||||
{
|
||||
toolExecutor: makeMockToolExecutor(),
|
||||
credentialProvider,
|
||||
memoryFactory: makeMockMemoryFactory(),
|
||||
webSearchFetch: webSearchFetch as unknown as typeof fetch,
|
||||
},
|
||||
);
|
||||
|
||||
const webSearchTool = (agent as unknown as { tools?: BuiltTool[] }).tools?.find(
|
||||
(tool) => tool.name === 'web_search',
|
||||
);
|
||||
expect(webSearchTool).toBeDefined();
|
||||
|
||||
await webSearchTool!.handler!({ query: 'test' }, {} as never);
|
||||
|
||||
expect(webSearchFetch).toHaveBeenCalledTimes(1);
|
||||
const [requestUrl] = webSearchFetch.mock.calls[0] as [string];
|
||||
expect(requestUrl).toContain('http://searxng.internal:8080/search');
|
||||
});
|
||||
|
||||
it('uses native web search when native provider is explicitly configured', async () => {
|
||||
const agent = await buildFromJson(
|
||||
makeConfig({
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@ import { userHasScopes } from '@/permissions.ee/check-access';
|
|||
import { AiService } from '@/services/ai.service';
|
||||
import { ProxyTokenManager } from '@/services/proxy-token-manager';
|
||||
import { UrlService } from '@/services/url.service';
|
||||
import { createAiMcpFetch, createAiProxyFetch } from '@/utils/ai-proxy-fetch';
|
||||
import { createAiMcpFetch, createAiProxyFetch, createWebSearchFetch } from '@/utils/ai-proxy-fetch';
|
||||
import { WorkflowRunner } from '@/workflow-runner';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
|
||||
|
|
@ -374,6 +374,13 @@ export class AgentRuntimeReconstructionService {
|
|||
instrumentation?.mcpFetch ??
|
||||
createAiMcpFetch(this.outboundHttp, this.ssrfConfig, this.ssrfProtectionService);
|
||||
|
||||
// Transport for fallback web-search calls
|
||||
const webSearchFetch = createWebSearchFetch(
|
||||
this.outboundHttp,
|
||||
this.ssrfConfig,
|
||||
this.ssrfProtectionService,
|
||||
);
|
||||
|
||||
const buildMcpClient = async (server: AgentJsonMcpServerConfig) =>
|
||||
await buildMcpClientForServer(server, {
|
||||
credentialProvider,
|
||||
|
|
@ -413,6 +420,7 @@ export class AgentRuntimeReconstructionService {
|
|||
fallbackWebSearch: instrumentation?.webSearch,
|
||||
// Only the mock MCP transport makes attaching auth-pending servers safe.
|
||||
attachAuthPendingMcpServers: instrumentation?.mcpFetch !== undefined,
|
||||
webSearchFetch,
|
||||
});
|
||||
|
||||
await this.injectRuntimeDependencies({
|
||||
|
|
|
|||
|
|
@ -109,6 +109,8 @@ export interface BuildFromJsonOptions {
|
|||
resolveManagedEmbeddingProviderOptions?: ManagedEmbeddingProviderOptionsResolver;
|
||||
/** Proxy-aware `fetch` for the agent's model calls (see `createAiProxyFetch`). */
|
||||
modelFetch?: FetchFn;
|
||||
/** Policy-aware `fetch` for fallback web-search calls (see `createWebSearchFetch`). */
|
||||
webSearchFetch?: FetchFn;
|
||||
/**
|
||||
* Replaces the live Brave/SearXNG call behind the fallback `web_search`
|
||||
* tool. When set, the tool is attached without requiring a search provider
|
||||
|
|
@ -204,6 +206,7 @@ export async function buildFromJson(
|
|||
const fallbackWebSearchTool = buildFallbackWebSearchTool(
|
||||
config,
|
||||
options.credentialProvider,
|
||||
options.webSearchFetch,
|
||||
options.fallbackWebSearch,
|
||||
);
|
||||
if (fallbackWebSearchTool) {
|
||||
|
|
@ -298,6 +301,7 @@ export function buildProviderToolsForModel(
|
|||
function buildFallbackWebSearchTool(
|
||||
config: AgentJsonConfig,
|
||||
credentialProvider: CredentialProvider,
|
||||
webSearchFetch?: FetchFn,
|
||||
fallbackWebSearch?: FallbackWebSearchHandler,
|
||||
): BuiltTool | null {
|
||||
const webSearchConfig = config.config?.webSearch;
|
||||
|
|
@ -345,11 +349,16 @@ function buildFallbackWebSearchTool(
|
|||
if (typeof credential.apiUrl !== 'string') {
|
||||
throw new Error('SearXNG credential is missing an API URL.');
|
||||
}
|
||||
return await searxngSearch(credential.apiUrl, args.query, {
|
||||
maxResults: args.maxResults,
|
||||
includeDomains: args.includeDomains,
|
||||
excludeDomains: args.excludeDomains,
|
||||
});
|
||||
return await searxngSearch(
|
||||
credential.apiUrl,
|
||||
args.query,
|
||||
{
|
||||
maxResults: args.maxResults,
|
||||
includeDomains: args.includeDomains,
|
||||
excludeDomains: args.excludeDomains,
|
||||
},
|
||||
webSearchFetch,
|
||||
);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -257,6 +257,78 @@ describe('validateWorkflowCredentialReferences', () => {
|
|||
expect(result.error).toContain("credential 'cred-foreign' is not usable");
|
||||
});
|
||||
|
||||
test('validates every credential when nodeCredentialType is an expression', async () => {
|
||||
vi.spyOn(NodeHelpers, 'displayParameter').mockReturnValue(false);
|
||||
const { credentialsService, nodeTypes } = createMocks({
|
||||
usableCredentials: [],
|
||||
getOneImpl: async (id) => ({ id, name: 'GitHub account', type: 'githubApi' }),
|
||||
nodeTypeDescriptions: new Map([
|
||||
[
|
||||
'n8n-nodes-base.httpRequest',
|
||||
makeNodeTypeDescription({ name: 'n8n-nodes-base.httpRequest', credentials: [] }),
|
||||
],
|
||||
]),
|
||||
});
|
||||
|
||||
const result = await validateWorkflowCredentialReferences(
|
||||
[
|
||||
makeNode({
|
||||
name: 'Fetch',
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
parameters: {
|
||||
authentication: 'predefinedCredentialType',
|
||||
nodeCredentialType: '={{ "githubApi" }}',
|
||||
},
|
||||
credentials: { githubApi: { id: 'cred-foreign', name: 'GitHub account' } },
|
||||
}),
|
||||
],
|
||||
user,
|
||||
credentialsService,
|
||||
nodeTypes,
|
||||
projectId,
|
||||
);
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) throw new Error('unreachable');
|
||||
expect(result.error).toContain("credential 'cred-foreign' is not usable");
|
||||
});
|
||||
|
||||
test('validates every credential when genericAuthType is an expression', async () => {
|
||||
vi.spyOn(NodeHelpers, 'displayParameter').mockReturnValue(false);
|
||||
const { credentialsService, nodeTypes } = createMocks({
|
||||
usableCredentials: [],
|
||||
getOneImpl: async (id) => ({ id, name: 'Header auth', type: 'httpHeaderAuth' }),
|
||||
nodeTypeDescriptions: new Map([
|
||||
[
|
||||
'n8n-nodes-base.httpRequest',
|
||||
makeNodeTypeDescription({ name: 'n8n-nodes-base.httpRequest', credentials: [] }),
|
||||
],
|
||||
]),
|
||||
});
|
||||
|
||||
const result = await validateWorkflowCredentialReferences(
|
||||
[
|
||||
makeNode({
|
||||
name: 'Fetch',
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
parameters: {
|
||||
authentication: 'genericCredentialType',
|
||||
genericAuthType: '={{ "httpHeaderAuth" }}',
|
||||
},
|
||||
credentials: { httpHeaderAuth: { id: 'cred-foreign', name: 'Header auth' } },
|
||||
}),
|
||||
],
|
||||
user,
|
||||
credentialsService,
|
||||
nodeTypes,
|
||||
projectId,
|
||||
);
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) throw new Error('unreachable');
|
||||
expect(result.error).toContain("credential 'cred-foreign' is not usable");
|
||||
});
|
||||
|
||||
test('validates every credential when the node type cannot be resolved', async () => {
|
||||
const { credentialsService, nodeTypes } = createMocks({
|
||||
usableCredentials: [],
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import type { User } from '@n8n/db';
|
||||
import type { INode, INodeTypeDescription, IWorkflowBase } from 'n8n-workflow';
|
||||
import { NodeHelpers } from 'n8n-workflow';
|
||||
import { isExpression, NodeHelpers } from 'n8n-workflow';
|
||||
|
||||
import type { CredentialsService } from '@/credentials/credentials.service';
|
||||
import { NotFoundError } from '@/errors/response-errors/not-found.error';
|
||||
|
|
@ -243,6 +243,10 @@ function computeActiveCredentialTypes(node: INode, nodeTypes: NodeTypes): Set<st
|
|||
// static credentials array.
|
||||
const { nodeCredentialType } = node.parameters;
|
||||
if (typeof nodeCredentialType === 'string' && nodeCredentialType) {
|
||||
// An expression only resolves to its real type at execution time, so the
|
||||
// static filter can't know which credential it activates. Check all
|
||||
// referenced credentials rather than trust the unresolved literal.
|
||||
if (isExpression(nodeCredentialType)) return null;
|
||||
activeTypes.add(nodeCredentialType);
|
||||
}
|
||||
|
||||
|
|
@ -250,6 +254,7 @@ function computeActiveCredentialTypes(node: INode, nodeTypes: NodeTypes): Set<st
|
|||
// authentication=genericCredentialType) live in genericAuthType.
|
||||
const { genericAuthType } = node.parameters;
|
||||
if (typeof genericAuthType === 'string' && genericAuthType) {
|
||||
if (isExpression(genericAuthType)) return null;
|
||||
activeTypes.add(genericAuthType);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -355,6 +355,24 @@ describe('OidcService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('should persist emailVerifiedRequired through updateConfig', async () => {
|
||||
settingsRepository.save = vi.fn().mockResolvedValue(mockConfigFromDB);
|
||||
settingsRepository.findByKey = vi.fn().mockResolvedValue(mockConfigFromDB);
|
||||
vi.mocked(client.discovery).mockResolvedValue({} as client.Configuration);
|
||||
|
||||
await oidcService.updateConfig({
|
||||
...mockOidcConfig,
|
||||
emailVerifiedRequired: true,
|
||||
} as any as OidcConfigDto);
|
||||
|
||||
expect(settingsRepository.save).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
key: OIDC_PREFERENCES_DB_KEY,
|
||||
value: expect.stringContaining('"emailVerifiedRequired":true'),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should not publish in single main setup', async () => {
|
||||
(instanceSettings as any).isMultiMain = false;
|
||||
|
||||
|
|
@ -954,4 +972,98 @@ describe('OidcService', () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
const mockAuthCallbackWith = (userInfo: Record<string, unknown>) => {
|
||||
oidcService.verifyState = vi.fn().mockReturnValue('valid-state');
|
||||
oidcService.verifyNonce = vi.fn().mockReturnValue('valid-nonce');
|
||||
// @ts-expect-error - getOidcConfiguration is private and only accessible within class 'OidcService'
|
||||
oidcService.getOidcConfiguration = vi.fn().mockResolvedValue({} as client.Configuration);
|
||||
// @ts-expect-error - applySsoProvisioning is private and only accessible within class 'OidcService'
|
||||
oidcService.applySsoProvisioning = vi.fn().mockResolvedValue(undefined);
|
||||
vi.mocked(client.authorizationCodeGrant).mockResolvedValue({
|
||||
access_token: 'valid-access-token',
|
||||
token_type: 'bearer',
|
||||
claims: () => ({ sub: 'valid-subject' }),
|
||||
} as unknown as client.TokenEndpointResponse & client.TokenEndpointResponseHelpers);
|
||||
vi.spyOn(client, 'fetchUserInfo').mockResolvedValue(userInfo as any);
|
||||
};
|
||||
|
||||
const setEmailVerifiedRequired = () => {
|
||||
// Replace (not mutate) the config so the shared default constant is untouched.
|
||||
// @ts-expect-error - oidcConfig is private and only accessible within class 'OidcService'
|
||||
oidcService.oidcConfig = { ...oidcService.oidcConfig, emailVerifiedRequired: true };
|
||||
};
|
||||
|
||||
const login = async () => {
|
||||
const callbackUrl = new URL('https://example.com/callback');
|
||||
return await oidcService.loginUser(
|
||||
callbackUrl,
|
||||
oidcService.generateState().signed,
|
||||
oidcService.generateNonce().signed,
|
||||
);
|
||||
};
|
||||
|
||||
it('should reject linking to an existing user when email is not verified', async () => {
|
||||
mockAuthCallbackWith({ email_verified: false, email: 'john.doe@test.com' });
|
||||
userRepository.findOne = vi.fn().mockResolvedValue({ email: 'john.doe@test.com' } as any);
|
||||
|
||||
await expect(login()).rejects.toThrow(BadRequestError);
|
||||
await expect(login()).rejects.toThrow('Email address is not verified by the identity provider');
|
||||
expect(authIdentityRepository.save).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject when email_verified is the string "false" (no boolean coercion bypass)', async () => {
|
||||
mockAuthCallbackWith({ email_verified: 'false', email: 'john.doe@test.com' });
|
||||
userRepository.findOne = vi.fn().mockResolvedValue({ email: 'john.doe@test.com' } as any);
|
||||
|
||||
await expect(login()).rejects.toThrow(BadRequestError);
|
||||
expect(authIdentityRepository.save).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should link to an existing user when email_verified is absent (default, permissive)', async () => {
|
||||
mockAuthCallbackWith({ email: 'john.doe@test.com' });
|
||||
userRepository.findOne = vi
|
||||
.fn()
|
||||
.mockResolvedValue({ id: 'user-1', email: 'john.doe@test.com' } as any);
|
||||
|
||||
const user = await login();
|
||||
|
||||
expect(user.user.email).toEqual('john.doe@test.com');
|
||||
expect(authIdentityRepository.save).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject when email_verified is absent and enforcement is enabled', async () => {
|
||||
setEmailVerifiedRequired();
|
||||
mockAuthCallbackWith({ email: 'john.doe@test.com' });
|
||||
userRepository.findOne = vi.fn().mockResolvedValue({ email: 'john.doe@test.com' } as any);
|
||||
|
||||
await expect(login()).rejects.toThrow(BadRequestError);
|
||||
expect(authIdentityRepository.save).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should link when email_verified is true and enforcement is enabled', async () => {
|
||||
setEmailVerifiedRequired();
|
||||
mockAuthCallbackWith({ email_verified: true, email: 'john.doe@test.com' });
|
||||
userRepository.findOne = vi
|
||||
.fn()
|
||||
.mockResolvedValue({ id: 'user-1', email: 'john.doe@test.com' } as any);
|
||||
|
||||
const user = await login();
|
||||
|
||||
expect(user.user.email).toEqual('john.doe@test.com');
|
||||
expect(authIdentityRepository.save).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not re-check email_verified for an already-linked identity', async () => {
|
||||
// Enforcement on + unverified email, but the identity is already bound by `sub`.
|
||||
setEmailVerifiedRequired();
|
||||
mockAuthCallbackWith({ email_verified: false, email: 'john.doe@test.com' });
|
||||
authIdentityRepository.findOne = vi
|
||||
.fn()
|
||||
.mockResolvedValue({ user: { email: 'john.doe@test.com' } } as any);
|
||||
|
||||
const user = await login();
|
||||
|
||||
expect(user.user.email).toEqual('john.doe@test.com');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -54,6 +54,7 @@ type OidcRuntimeConfig = Pick<
|
|||
| 'prompt'
|
||||
| 'authenticationContextClassReference'
|
||||
| 'additionalScopes'
|
||||
| 'emailVerifiedRequired'
|
||||
| 'rpInitiatedLogoutEnabled'
|
||||
> & {
|
||||
discoveryEndpoint: URL;
|
||||
|
|
@ -345,6 +346,10 @@ export class OidcService {
|
|||
});
|
||||
|
||||
if (foundUser) {
|
||||
// Linking to an existing account uses the email as the key, so only do it
|
||||
// when the IdP says the email is verified.
|
||||
this.assertEmailVerified(userInfo.email_verified);
|
||||
|
||||
this.logger.debug(
|
||||
`OIDC login: User with email ${userInfo.email} already exists, linking OIDC identity.`,
|
||||
);
|
||||
|
|
@ -451,6 +456,19 @@ export class OidcService {
|
|||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Throws if the email is not verified. By default only an explicit `false` is
|
||||
* rejected; `emailVerifiedRequired` also rejects an absent claim.
|
||||
*/
|
||||
private assertEmailVerified(emailVerified: unknown): void {
|
||||
const isVerified = emailVerified === true || emailVerified === 'true';
|
||||
const isExplicitlyUnverified = emailVerified === false || emailVerified === 'false';
|
||||
|
||||
if (isExplicitlyUnverified || (this.oidcConfig.emailVerifiedRequired && !isVerified)) {
|
||||
throw new BadRequestError('Email address is not verified by the identity provider');
|
||||
}
|
||||
}
|
||||
|
||||
async generateTestLoginUrl(): Promise<{ url: URL; state: string; nonce: string }> {
|
||||
await this.loadOpenIdClient();
|
||||
const config = await this.loadConfig(true);
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { Logger } from '@n8n/backend-common';
|
|||
import { OutboundHttp, SsrfProtectionService, type HttpRequestClient } from '@n8n/backend-network';
|
||||
import { mockInstance } from '@n8n/backend-test-utils';
|
||||
import type { OAuth2CredentialData } from '@n8n/client-oauth2';
|
||||
import { AuthError as OAuth2AuthError } from '@n8n/client-oauth2';
|
||||
import { GlobalConfig, SsrfProtectionConfig } from '@n8n/config';
|
||||
import { Time } from '@n8n/constants';
|
||||
import type { AuthenticatedRequest, CredentialsEntity, ICredentialsDb, User } from '@n8n/db';
|
||||
|
|
@ -203,13 +204,46 @@ describe('OauthService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('getSsrfBridge', () => {
|
||||
afterEach(() => {
|
||||
ssrfProtectionConfig.enabled = false;
|
||||
});
|
||||
|
||||
it('should return the protection service when the guard is enabled', () => {
|
||||
ssrfProtectionConfig.enabled = true;
|
||||
|
||||
expect(service.getSsrfBridge()).toBe(ssrfProtectionService);
|
||||
});
|
||||
|
||||
it('should return undefined when the guard is disabled', () => {
|
||||
ssrfProtectionConfig.enabled = false;
|
||||
|
||||
expect(service.getSsrfBridge()).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('extractCallbackErrorReason', () => {
|
||||
it('should return the stringified body when the error has one', () => {
|
||||
const error = Object.assign(new Error('request failed'), {
|
||||
body: { error: 'invalid_grant' },
|
||||
it('should return only the error code for an OAuth2 error response', () => {
|
||||
const error = new OAuth2AuthError('request failed', {
|
||||
error: 'invalid_grant',
|
||||
error_description: 'some free-form detail from the authorization server',
|
||||
});
|
||||
|
||||
expect(service.extractCallbackErrorReason(error)).toBe('{"error":"invalid_grant"}');
|
||||
expect(service.extractCallbackErrorReason(error)).toBe('invalid_grant');
|
||||
});
|
||||
|
||||
it('should return undefined for an OAuth2 error response without an error code', () => {
|
||||
const error = new OAuth2AuthError('request failed', { error_description: 'detail' });
|
||||
|
||||
expect(service.extractCallbackErrorReason(error)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should ignore the body of a non-OAuth2 error response', () => {
|
||||
const error = Object.assign(new Error('HTTP status 400'), {
|
||||
body: '{"internalField":"detail"}',
|
||||
});
|
||||
|
||||
expect(service.extractCallbackErrorReason(error)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should surface the wrapped cause chain when there is no body', () => {
|
||||
|
|
@ -5339,6 +5373,68 @@ describe('OauthService', () => {
|
|||
expect(mockToken.refresh).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
describe('outbound network policy', () => {
|
||||
const captureRefreshClientOptions = async () => {
|
||||
const { ClientOAuth2 } = await import('@n8n/client-oauth2');
|
||||
let capturedOptions: Record<string, unknown> = {};
|
||||
const mockToken = {
|
||||
refresh: vi.fn().mockResolvedValue({
|
||||
data: { access_token: 'new-token', token_type: 'bearer' },
|
||||
accessToken: 'new-token',
|
||||
}),
|
||||
client: {},
|
||||
};
|
||||
vi.mocked(ClientOAuth2).mockImplementation(function (options) {
|
||||
capturedOptions = options as unknown as Record<string, unknown>;
|
||||
return { createToken: vi.fn().mockReturnValue(mockToken) } as never;
|
||||
});
|
||||
|
||||
credentialsRepository.findOne.mockResolvedValue(
|
||||
makeCredential({ isGlobal: true }) as never,
|
||||
);
|
||||
vi.spyOn(service, 'getOAuthCredentials').mockResolvedValue({
|
||||
clientId: 'id',
|
||||
clientSecret: 'secret',
|
||||
accessTokenUrl: 'https://example.com/token',
|
||||
grantType: 'authorizationCode',
|
||||
authentication: 'header',
|
||||
oauthTokenData: {
|
||||
access_token: 'stale',
|
||||
refresh_token: 'refresh-tok',
|
||||
token_type: 'bearer',
|
||||
},
|
||||
} as unknown as OAuth2CredentialData);
|
||||
vi.spyOn(service, 'encryptAndSaveData').mockResolvedValue(undefined);
|
||||
|
||||
const result = await service.refreshOAuth2CredentialById(credentialId, projectId);
|
||||
|
||||
return { capturedOptions, result };
|
||||
};
|
||||
|
||||
afterEach(() => {
|
||||
ssrfProtectionConfig.enabled = false;
|
||||
});
|
||||
|
||||
it('should build the refresh client with the bridge when the guard is enabled', async () => {
|
||||
ssrfProtectionConfig.enabled = true;
|
||||
|
||||
const { capturedOptions, result } = await captureRefreshClientOptions();
|
||||
|
||||
expect(capturedOptions.ssrfBridge).toBe(ssrfProtectionService);
|
||||
expect(result).toEqual({ Authorization: 'Bearer new-token' });
|
||||
});
|
||||
|
||||
it('should build the refresh client without a bridge when the guard is disabled', async () => {
|
||||
ssrfProtectionConfig.enabled = false;
|
||||
|
||||
const { capturedOptions, result } = await captureRefreshClientOptions();
|
||||
|
||||
expect(capturedOptions.ssrfBridge).toBeUndefined();
|
||||
// The refresh must still succeed, so instances that leave the guard off are unaffected.
|
||||
expect(result).toEqual({ Authorization: 'Bearer new-token' });
|
||||
});
|
||||
});
|
||||
|
||||
it('builds the client with a certificate when certificate authentication is selected', async () => {
|
||||
const { ClientOAuth2 } = await import('@n8n/client-oauth2');
|
||||
let capturedOptions: unknown;
|
||||
|
|
|
|||
|
|
@ -1,5 +1,10 @@
|
|||
import { Logger } from '@n8n/backend-common';
|
||||
import { OutboundHttp, SsrfProtectionService, type HttpRequestClient } from '@n8n/backend-network';
|
||||
import {
|
||||
OutboundHttp,
|
||||
SsrfProtectionService,
|
||||
type HttpRequestClient,
|
||||
type SsrfBridge,
|
||||
} from '@n8n/backend-network';
|
||||
import { GlobalConfig, SsrfProtectionConfig } from '@n8n/config';
|
||||
import type { AuthenticatedRequest, CredentialsEntity, ICredentialsDb } from '@n8n/db';
|
||||
import { CredentialsRepository } from '@n8n/db';
|
||||
|
|
@ -8,7 +13,7 @@ import Csrf from 'csrf';
|
|||
import type { Request, Response } from 'express';
|
||||
import { Credentials, Cipher } from 'n8n-core';
|
||||
import type { ICredentialDataDecryptedObject, IWorkflowExecuteAdditionalData } from 'n8n-workflow';
|
||||
import { jsonParse, jsonStringify, OperationalError, UnexpectedError } from 'n8n-workflow';
|
||||
import { jsonParse, OperationalError, UnexpectedError } from 'n8n-workflow';
|
||||
|
||||
import {
|
||||
GENERIC_OAUTH2_CREDENTIALS_WITH_EDITABLE_SCOPE,
|
||||
|
|
@ -25,6 +30,7 @@ import { validateOAuthUrl } from '@/oauth/validate-oauth-url';
|
|||
import { UrlService } from '@/services/url.service';
|
||||
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
|
||||
import {
|
||||
AuthError as OAuth2AuthError,
|
||||
ClientOAuth2,
|
||||
resolveClientAuthOptions,
|
||||
type ClientOAuth2Options,
|
||||
|
|
@ -124,8 +130,8 @@ export class OauthService {
|
|||
private readonly eventService: EventService,
|
||||
private readonly cacheService: CacheService,
|
||||
outboundHttp: OutboundHttp,
|
||||
ssrfProtectionService: SsrfProtectionService,
|
||||
ssrfProtectionConfig: SsrfProtectionConfig,
|
||||
private readonly ssrfProtectionService: SsrfProtectionService,
|
||||
private readonly ssrfProtectionConfig: SsrfProtectionConfig,
|
||||
) {
|
||||
// Unlike most OutboundHttp callsites, here we opt into SSRF protection (when the environment enables it) because the attack risk is higher:
|
||||
// these URLs can be user-, instance- or remote-server-supplied (discovery / dynamic client registration),
|
||||
|
|
@ -133,13 +139,22 @@ export class OauthService {
|
|||
// Self-hosted users with an internal OAuth/MCP server are accommodated via the SSRF allowlist config, not by disabling the guard.
|
||||
// In the future, enabling SSRF "per feature" could be refined through configuration.
|
||||
this.http = outboundHttp.requests({
|
||||
ssrf: ssrfProtectionConfig.enabled ? ssrfProtectionService : 'disabled',
|
||||
ssrf: this.getSsrfBridge() ?? 'disabled',
|
||||
timeout: OAUTH_REQUEST_TIMEOUT_MS,
|
||||
});
|
||||
}
|
||||
|
||||
private readonly http: HttpRequestClient;
|
||||
|
||||
/**
|
||||
* The bridge to hand to `ClientOAuth2` so token endpoint requests are subject to the
|
||||
* same outbound network policy as discovery / dynamic client registration.
|
||||
* `undefined` when the guard is disabled for this instance.
|
||||
*/
|
||||
getSsrfBridge(): SsrfBridge | undefined {
|
||||
return this.ssrfProtectionConfig.enabled ? this.ssrfProtectionService : undefined;
|
||||
}
|
||||
|
||||
private oauthFlowCacheKey(token: string): string {
|
||||
return `${OAUTH_FLOW_CACHE_PREFIX}${token}`;
|
||||
}
|
||||
|
|
@ -642,13 +657,19 @@ export class OauthService {
|
|||
|
||||
/**
|
||||
* Derive a human-readable reason for the OAuth callback error page.
|
||||
* Prefers a structured HTTP `body`, then falls back to the wrapped `cause`
|
||||
* chain so errors like {@link CredentialStorageError} surface their root
|
||||
* cause instead of rendering an empty "More details" section.
|
||||
* Surfaces the fixed OAuth2 error code when the authorization server returned one,
|
||||
* then falls back to the wrapped `cause` chain so errors like
|
||||
* {@link CredentialStorageError} surface their root cause instead of rendering an
|
||||
* empty "More details" section.
|
||||
*
|
||||
* The reason is rendered into a page, so it is limited to values n8n produces or a
|
||||
* known OAuth2 code — never free-form content read back from the token endpoint.
|
||||
* Callers log the full error before rendering.
|
||||
*/
|
||||
extractCallbackErrorReason(error: Error): string | undefined {
|
||||
if ('body' in error && error.body) {
|
||||
return jsonStringify(error.body, { replaceCircularRefs: true });
|
||||
if (error instanceof OAuth2AuthError) {
|
||||
const errorCode = (error.body as { error?: unknown } | undefined)?.error;
|
||||
return typeof errorCode === 'string' && errorCode.length > 0 ? errorCode : undefined;
|
||||
}
|
||||
|
||||
const causes: string[] = [];
|
||||
|
|
@ -720,6 +741,7 @@ export class OauthService {
|
|||
...(resource ? { resource } : {}),
|
||||
ignoreSSLIssues: oauthCredentials.ignoreSSLIssues,
|
||||
authentication: oauthCredentials.authentication ?? 'header',
|
||||
ssrfBridge: this.getSsrfBridge(),
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ required:
|
|||
- prompt
|
||||
- authenticationContextClassReference
|
||||
- additionalScopes
|
||||
- emailVerifiedRequired
|
||||
- rpInitiatedLogoutEnabled
|
||||
properties:
|
||||
clientId:
|
||||
|
|
@ -54,6 +55,12 @@ properties:
|
|||
Additional scopes to request, space separated. n8n always requests `openid`, `profile`
|
||||
and `email`. Use an empty string when unused.
|
||||
example: groups roles
|
||||
emailVerifiedRequired:
|
||||
type: boolean
|
||||
description: >
|
||||
Whether the identity provider must assert that the user's email address is verified before
|
||||
the login is accepted. When disabled, only an explicit negative assertion is rejected.
|
||||
example: false
|
||||
rpInitiatedLogoutEnabled:
|
||||
type: boolean
|
||||
description: >
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ required:
|
|||
- prompt
|
||||
- authenticationContextClassReference
|
||||
- additionalScopes
|
||||
- emailVerifiedRequired
|
||||
- rpInitiatedLogoutEnabled
|
||||
properties:
|
||||
clientId:
|
||||
|
|
@ -48,6 +49,12 @@ properties:
|
|||
Additional scopes to request, space separated. n8n always requests `openid`, `profile`
|
||||
and `email`.
|
||||
example: groups roles
|
||||
emailVerifiedRequired:
|
||||
type: boolean
|
||||
description: >
|
||||
Whether the identity provider must assert that the user's email address is verified before
|
||||
the login is accepted. When disabled, only an explicit negative assertion is rejected.
|
||||
example: false
|
||||
rpInitiatedLogoutEnabled:
|
||||
type: boolean
|
||||
description: >
|
||||
|
|
|
|||
|
|
@ -10,5 +10,8 @@ export function toOidcConfigurationResponse(config: OidcRuntimeConfig): OidcConf
|
|||
...config,
|
||||
discoveryEndpoint: config.discoveryEndpoint.toString(),
|
||||
clientSecret: config.clientSecret ? OIDC_CLIENT_SECRET_REDACTED_VALUE : config.clientSecret,
|
||||
// The PUT body requires every writable field, so never omit this one — a GET
|
||||
// response has to stay usable as a PUT body.
|
||||
emailVerifiedRequired: config.emailVerifiedRequired ?? false,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,9 @@
|
|||
import { OutboundHttp } from '@n8n/backend-network';
|
||||
import type { HttpRequestClient, SsrfBridge } from '@n8n/backend-network';
|
||||
import { Container } from '@n8n/di';
|
||||
import { RoutingNode } from 'n8n-core';
|
||||
import type {
|
||||
ICredentialTestFunctions,
|
||||
ICredentialType,
|
||||
INode,
|
||||
INodeType,
|
||||
|
|
@ -161,6 +165,40 @@ describe('CredentialsTester', () => {
|
|||
expect(redactedMessage.message).toBe('Test failed for apiKey *****key');
|
||||
});
|
||||
|
||||
// A node-defined credential test function may issue an outbound request to
|
||||
// a credential-supplied host. `testCredentials` must hand it a context
|
||||
// carrying the execution's egress policy, so that when SSRF protection is
|
||||
// enabled the test honours the same restrictions as node execution. The
|
||||
// legacy `this.helpers.request` helper routes through
|
||||
// `OutboundHttp.requests({ ssrf })`; asserting that argument proves the
|
||||
// bridge reaches the test function.
|
||||
it('forwards the SSRF bridge from getBase to a function-based credential test', async () => {
|
||||
const requestLegacy = vi.fn().mockResolvedValue('ok');
|
||||
const requests = vi.fn().mockReturnValue(mock<HttpRequestClient>({ requestLegacy }));
|
||||
Container.set(OutboundHttp, mock<OutboundHttp>({ requests }));
|
||||
|
||||
const ssrfBridge = mock<SsrfBridge>();
|
||||
vi.spyOn(WorkflowExecuteAdditionalData, 'getBase').mockResolvedValue(
|
||||
mock<IWorkflowExecuteAdditionalData>({ ssrfBridge }),
|
||||
);
|
||||
|
||||
mockTestFunction.mockImplementation(async function (this: ICredentialTestFunctions) {
|
||||
await this.helpers.request({ uri: 'http://internal-service.local/api' });
|
||||
return { status: 'OK', message: 'ok' };
|
||||
});
|
||||
credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue({ baseUrl: 'http://host' });
|
||||
|
||||
await credentialsTester.testCredentials('user-id', 'testCredentials', {
|
||||
id: 'credential-id',
|
||||
name: 'credential-name',
|
||||
type: 'oAuth2Api',
|
||||
data: { baseUrl: 'http://host' },
|
||||
});
|
||||
|
||||
expect(mockTestFunction).toHaveBeenCalled();
|
||||
expect(requests).toHaveBeenCalledWith({ ssrf: ssrfBridge });
|
||||
});
|
||||
|
||||
it('should keep function-based tests working with the real routing engine untouched', async () => {
|
||||
mockTestFunction.mockResolvedValue({ status: 'OK', message: 'fine' });
|
||||
credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue({});
|
||||
|
|
|
|||
|
|
@ -1,9 +1,13 @@
|
|||
import { Logger } from '@n8n/backend-common';
|
||||
import { OutboundHttp } from '@n8n/backend-network';
|
||||
import type { HttpRequestClient, SsrfBridge } from '@n8n/backend-network';
|
||||
import { mockInstance } from '@n8n/backend-test-utils';
|
||||
import { SharedWorkflowRepository } from '@n8n/db';
|
||||
import type { User } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import { RoutingNode } from 'n8n-core';
|
||||
import {
|
||||
type ILoadOptionsFunctions,
|
||||
type INodeParameters,
|
||||
type INodeType,
|
||||
type IWorkflowExecuteAdditionalData,
|
||||
|
|
@ -215,6 +219,125 @@ describe('DynamicNodeParametersService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
// The method-name branch runs a node's own loadOptions/listSearch/etc. method,
|
||||
// which may issue outbound requests to a credential-supplied host. Those
|
||||
// requests must carry the execution's egress policy so that, when SSRF
|
||||
// protection is enabled, they honour the same restrictions as node execution.
|
||||
// Many such methods use the legacy `this.helpers.request` helper (e.g. nodes
|
||||
// that build the request and set auth by hand), which routes through
|
||||
// `OutboundHttp.requests({ ssrf })` — asserting that argument proves the
|
||||
// bridge is forwarded end to end.
|
||||
describe('egress policy for method-name requests', () => {
|
||||
const requestLegacy = vi.fn();
|
||||
const requests = vi.fn();
|
||||
const outboundHttp = mock<OutboundHttp>({ requests });
|
||||
|
||||
beforeEach(() => {
|
||||
requestLegacy.mockResolvedValue([]);
|
||||
requests.mockReturnValue(mock<HttpRequestClient>({ requestLegacy }));
|
||||
Container.set(OutboundHttp, outboundHttp);
|
||||
});
|
||||
|
||||
/** A node method that issues one legacy `this.helpers.request` to an internal host. */
|
||||
const requestingMethod = () =>
|
||||
vi.fn(async function (this: ILoadOptionsFunctions) {
|
||||
await this.helpers.request({
|
||||
uri: 'http://internal-service.local/api',
|
||||
json: true,
|
||||
});
|
||||
return [];
|
||||
});
|
||||
|
||||
type Scenario = {
|
||||
name: string;
|
||||
register: (method: ReturnType<typeof requestingMethod>) => void;
|
||||
invoke: (additionalData: IWorkflowExecuteAdditionalData) => Promise<unknown>;
|
||||
};
|
||||
|
||||
const nodeTypeAndVersion = { name: 'TestNode', version: 1 };
|
||||
|
||||
const registerMethod = (
|
||||
type: 'loadOptions' | 'listSearch' | 'resourceMapping' | 'actionHandler',
|
||||
method: ReturnType<typeof requestingMethod>,
|
||||
) => {
|
||||
nodeTypes.getByNameAndVersion.mockReturnValue(
|
||||
mock<INodeType>({
|
||||
description: { properties: [] },
|
||||
methods: { [type]: { run: method } },
|
||||
}),
|
||||
);
|
||||
};
|
||||
|
||||
const scenarios: Scenario[] = [
|
||||
{
|
||||
name: 'loadOptions (getOptionsViaMethodName)',
|
||||
register: (method) => registerMethod('loadOptions', method),
|
||||
invoke: async (additionalData) =>
|
||||
await service.getOptionsViaMethodName('run', '', additionalData, nodeTypeAndVersion, {}),
|
||||
},
|
||||
{
|
||||
name: 'listSearch (getResourceLocatorResults)',
|
||||
register: (method) => registerMethod('listSearch', method),
|
||||
invoke: async (additionalData) =>
|
||||
await service.getResourceLocatorResults(
|
||||
'run',
|
||||
'',
|
||||
additionalData,
|
||||
nodeTypeAndVersion,
|
||||
{},
|
||||
),
|
||||
},
|
||||
{
|
||||
name: 'resourceMapping (getResourceMappingFields)',
|
||||
register: (method) => registerMethod('resourceMapping', method),
|
||||
invoke: async (additionalData) =>
|
||||
await service.getResourceMappingFields('run', '', additionalData, nodeTypeAndVersion, {}),
|
||||
},
|
||||
{
|
||||
name: 'actionHandler (getActionResult)',
|
||||
register: (method) => registerMethod('actionHandler', method),
|
||||
invoke: async (additionalData) =>
|
||||
await service.getActionResult(
|
||||
'run',
|
||||
'',
|
||||
additionalData,
|
||||
nodeTypeAndVersion,
|
||||
{},
|
||||
undefined,
|
||||
),
|
||||
},
|
||||
];
|
||||
|
||||
it.each(scenarios)(
|
||||
'forwards the SSRF bridge to requests made from the $name method',
|
||||
async ({ register, invoke }) => {
|
||||
const method = requestingMethod();
|
||||
register(method);
|
||||
const ssrfBridge = mock<SsrfBridge>();
|
||||
const additionalData = mock<IWorkflowExecuteAdditionalData>({ ssrfBridge });
|
||||
|
||||
await invoke(additionalData);
|
||||
|
||||
expect(method).toHaveBeenCalled();
|
||||
expect(requests).toHaveBeenCalledWith({ ssrf: ssrfBridge });
|
||||
},
|
||||
);
|
||||
|
||||
it.each(scenarios)(
|
||||
'disables SSRF for requests from the $name method when no bridge is attached',
|
||||
async ({ register, invoke }) => {
|
||||
const method = requestingMethod();
|
||||
register(method);
|
||||
const additionalData = mock<IWorkflowExecuteAdditionalData>({ ssrfBridge: undefined });
|
||||
|
||||
await invoke(additionalData);
|
||||
|
||||
expect(method).toHaveBeenCalled();
|
||||
expect(requests).toHaveBeenCalledWith({ ssrf: 'disabled' });
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe('getOptionsViaLoadOptions', () => {
|
||||
it('should throw BadRequestError when the node type has no requestDefaults.baseURL', async () => {
|
||||
nodeTypes.getByNameAndVersion.mockReturnValue(
|
||||
|
|
|
|||
|
|
@ -2,12 +2,14 @@ import { Logger } from '@n8n/backend-common';
|
|||
import { mockInstance } from '@n8n/backend-test-utils';
|
||||
import { TaskRunnersConfig } from '@n8n/config';
|
||||
import type { ChildProcess, SpawnOptions } from 'node:child_process';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
import { EventEmitter } from 'node:events';
|
||||
import { mock, type MockProxy } from 'vitest-mock-extended';
|
||||
|
||||
import type { TaskBrokerAuthService } from '@/task-runners/task-broker/auth/task-broker-auth.service';
|
||||
import { JsTaskRunnerProcess } from '@/task-runners/task-runner-process-js';
|
||||
|
||||
import type { TaskRunnerLifecycleEvents } from '../task-runner-lifecycle-events';
|
||||
import { TaskRunnerLifecycleEvents } from '../task-runner-lifecycle-events';
|
||||
import { restartRetryDelay } from '../task-runner-process-base';
|
||||
|
||||
// Source imports `spawn` from `node:child_process` as an ESM binding, so mutating
|
||||
// `require('child_process').spawn` does not intercept it — mock the module instead.
|
||||
|
|
@ -27,15 +29,25 @@ describe('TaskRunnerProcess', () => {
|
|||
stderr: { pipe: vi.fn() },
|
||||
}),
|
||||
);
|
||||
logger.scoped.mockReturnValue(logger);
|
||||
});
|
||||
|
||||
const logger = mockInstance(Logger);
|
||||
logger.scoped.mockReturnValue(logger);
|
||||
const runnerConfig = mockInstance(TaskRunnersConfig);
|
||||
runnerConfig.mode = 'internal';
|
||||
runnerConfig.insecureMode = false;
|
||||
const authService = mock<TaskBrokerAuthService>();
|
||||
let taskRunnerProcess = new JsTaskRunnerProcess(logger, runnerConfig, authService, mock());
|
||||
|
||||
const createChildProcess = (pid?: number) =>
|
||||
Object.assign(new EventEmitter(), {
|
||||
pid,
|
||||
stdout: new EventEmitter(),
|
||||
stderr: new EventEmitter(),
|
||||
kill: vi.fn(),
|
||||
}) as unknown as ChildProcess;
|
||||
|
||||
afterEach(async () => {
|
||||
spawnMock.mockClear();
|
||||
});
|
||||
|
|
@ -68,6 +80,22 @@ describe('TaskRunnerProcess', () => {
|
|||
expect.any(Function),
|
||||
);
|
||||
});
|
||||
|
||||
it('should scope the logger to the runner', () => {
|
||||
new JsTaskRunnerProcess(logger, runnerConfig, authService, mock());
|
||||
|
||||
expect(logger.scoped).toHaveBeenCalledWith('task-runner-js');
|
||||
});
|
||||
|
||||
it('should register listener for `runner:unresponsive` event', () => {
|
||||
const runnerLifecycleEvents = mock<TaskRunnerLifecycleEvents>();
|
||||
new JsTaskRunnerProcess(logger, runnerConfig, authService, runnerLifecycleEvents);
|
||||
|
||||
expect(runnerLifecycleEvents.on).toHaveBeenCalledWith(
|
||||
'runner:unresponsive',
|
||||
expect.any(Function),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('start', () => {
|
||||
|
|
@ -206,4 +234,312 @@ describe('TaskRunnerProcess', () => {
|
|||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('relaunch on unexpected exit', () => {
|
||||
let auth: MockProxy<TaskBrokerAuthService>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
auth = mock<TaskBrokerAuthService>();
|
||||
auth.createGrantToken.mockResolvedValue('grantToken');
|
||||
taskRunnerProcess = new JsTaskRunnerProcess(logger, runnerConfig, auth, mock());
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it('should relaunch the runner after it exits', async () => {
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
|
||||
child.emit('exit', 1);
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
|
||||
expect(spawnMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should keep retrying when a relaunch attempt fails', async () => {
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
auth.createGrantToken.mockRejectedValueOnce(new Error('grant token unavailable'));
|
||||
|
||||
child.emit('exit', 1);
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(5_000);
|
||||
expect(spawnMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should back off between failed relaunch attempts', async () => {
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
auth.createGrantToken
|
||||
.mockRejectedValueOnce(new Error('grant token unavailable'))
|
||||
.mockRejectedValueOnce(new Error('grant token unavailable'));
|
||||
|
||||
child.emit('exit', 1);
|
||||
await vi.advanceTimersByTimeAsync(5_000);
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(5_000);
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(5_000);
|
||||
expect(spawnMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should keep retrying when a relaunch fails after the spawn', async () => {
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
|
||||
const unwirableChild = Object.assign(new EventEmitter(), {
|
||||
pid: 43,
|
||||
stdout: {
|
||||
on: () => {
|
||||
throw new Error('stream closed');
|
||||
},
|
||||
},
|
||||
stderr: new EventEmitter(),
|
||||
kill: vi.fn(),
|
||||
}) as unknown as ChildProcess;
|
||||
spawnMock.mockReturnValueOnce(unwirableChild);
|
||||
|
||||
child.emit('exit', 1);
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
expect(unwirableChild.kill).toHaveBeenCalled();
|
||||
|
||||
await vi.advanceTimersByTimeAsync(5_000);
|
||||
|
||||
expect(spawnMock).toHaveBeenCalledTimes(3);
|
||||
expect(taskRunnerProcess.isRunning).toBe(true);
|
||||
});
|
||||
|
||||
it('should keep retrying when setting up process monitoring fails', async () => {
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
|
||||
const unmonitorableChild = createChildProcess(43);
|
||||
spawnMock.mockReturnValueOnce(unmonitorableChild);
|
||||
vi.spyOn(taskRunnerProcess, 'setupProcessMonitoring').mockImplementationOnce(() => {
|
||||
throw new Error('failed to attach monitoring');
|
||||
});
|
||||
|
||||
child.emit('exit', 1);
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
expect(unmonitorableChild.kill).toHaveBeenCalled();
|
||||
|
||||
await vi.advanceTimersByTimeAsync(5_000);
|
||||
|
||||
expect(spawnMock).toHaveBeenCalledTimes(3);
|
||||
expect(taskRunnerProcess.isRunning).toBe(true);
|
||||
});
|
||||
|
||||
it('should relaunch the runner when it errors without exiting', async () => {
|
||||
const failedSpawn = createChildProcess(undefined);
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValueOnce(failedSpawn).mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
|
||||
failedSpawn.emit('error', new Error('spawn EAGAIN'));
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
|
||||
expect(spawnMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should relaunch only once when a failed spawn emits both error and exit', async () => {
|
||||
const failedSpawn = createChildProcess(undefined);
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValueOnce(failedSpawn).mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
|
||||
failedSpawn.emit('error', new Error('spawn EAGAIN'));
|
||||
failedSpawn.emit('exit', 1);
|
||||
await vi.advanceTimersByTimeAsync(5_000);
|
||||
|
||||
expect(spawnMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should not treat an error from a spawned process as an exit', async () => {
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
|
||||
child.emit('error', new Error('kill EPERM'));
|
||||
await vi.advanceTimersByTimeAsync(5_000);
|
||||
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should not relaunch after stop', async () => {
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
|
||||
const stopPromise = taskRunnerProcess.stop();
|
||||
child.emit('exit', 0);
|
||||
await stopPromise;
|
||||
await vi.advanceTimersByTimeAsync(5_000);
|
||||
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should relaunch on exit again after a stop and an explicit restart', async () => {
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
|
||||
const stopPromise = taskRunnerProcess.stop();
|
||||
child.emit('exit', 0);
|
||||
await stopPromise;
|
||||
|
||||
await taskRunnerProcess.start();
|
||||
child.emit('exit', 1);
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
|
||||
expect(spawnMock).toHaveBeenCalledTimes(3);
|
||||
// the exit during the stop must not have left a retry loop behind
|
||||
expect(vi.getTimerCount()).toBe(0);
|
||||
});
|
||||
|
||||
it('should kill a runner spawned by a relaunch that was in flight when stop began', async () => {
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
|
||||
let releaseGrantToken!: (token: string) => void;
|
||||
auth.createGrantToken.mockImplementationOnce(
|
||||
async () =>
|
||||
await new Promise<string>((resolve) => {
|
||||
releaseGrantToken = resolve;
|
||||
}),
|
||||
);
|
||||
child.emit('exit', 1);
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
|
||||
const relaunchedChild = createChildProcess(43);
|
||||
spawnMock.mockReturnValue(relaunchedChild);
|
||||
const stopPromise = taskRunnerProcess.stop();
|
||||
releaseGrantToken('grantToken');
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
|
||||
expect(relaunchedChild.kill).toHaveBeenCalled();
|
||||
relaunchedChild.emit('exit', 0);
|
||||
await stopPromise;
|
||||
|
||||
expect(taskRunnerProcess.isRunning).toBe(false);
|
||||
await vi.advanceTimersByTimeAsync(10_000);
|
||||
expect(spawnMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should stop retrying once shutdown begins', async () => {
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
auth.createGrantToken.mockRejectedValueOnce(new Error('grant token unavailable'));
|
||||
|
||||
child.emit('exit', 1);
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
await taskRunnerProcess.stop();
|
||||
await vi.advanceTimersByTimeAsync(10_000);
|
||||
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('restart on runner report', () => {
|
||||
let runnerLifecycleEvents: TaskRunnerLifecycleEvents;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
const auth = mock<TaskBrokerAuthService>();
|
||||
auth.createGrantToken.mockResolvedValue('grantToken');
|
||||
runnerLifecycleEvents = new TaskRunnerLifecycleEvents();
|
||||
taskRunnerProcess = new JsTaskRunnerProcess(
|
||||
logger,
|
||||
runnerConfig,
|
||||
auth,
|
||||
runnerLifecycleEvents,
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
const RESTART_EVENTS = [
|
||||
'runner:failed-heartbeat-check',
|
||||
'runner:timed-out-during-task',
|
||||
'runner:unresponsive',
|
||||
] as const;
|
||||
|
||||
const report = (event: (typeof RESTART_EVENTS)[number], taskTypes: string[]) => {
|
||||
runnerLifecycleEvents.emit(event, { runnerId: 'runner1', taskTypes });
|
||||
};
|
||||
|
||||
const reportUnresponsive = (taskTypes: string[]) => {
|
||||
report('runner:unresponsive', taskTypes);
|
||||
};
|
||||
|
||||
it.each(RESTART_EVENTS)(
|
||||
'should force-kill and relaunch on %s for a runner of its own task type',
|
||||
async (event) => {
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
|
||||
report(event, ['javascript']);
|
||||
|
||||
// a runner with a blocked event loop never exits on its own
|
||||
expect(child.kill).toHaveBeenCalledWith('SIGKILL');
|
||||
|
||||
child.emit('exit', null);
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
|
||||
expect(spawnMock).toHaveBeenCalledTimes(2);
|
||||
},
|
||||
);
|
||||
|
||||
it('should ignore a report once shutdown has begun', async () => {
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
|
||||
const stopPromise = taskRunnerProcess.stop();
|
||||
reportUnresponsive(['javascript']);
|
||||
|
||||
expect(child.kill).not.toHaveBeenCalledWith('SIGKILL');
|
||||
|
||||
child.emit('exit', 0);
|
||||
await stopPromise;
|
||||
await vi.advanceTimersByTimeAsync(10_000);
|
||||
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it.each(RESTART_EVENTS)('should ignore %s for a runner of another task type', async (event) => {
|
||||
const child = createChildProcess(42);
|
||||
spawnMock.mockReturnValue(child);
|
||||
await taskRunnerProcess.start();
|
||||
|
||||
report(event, ['python']);
|
||||
|
||||
expect(child.kill).not.toHaveBeenCalled();
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('restartRetryDelay', () => {
|
||||
it('should double the delay up to a ceiling', () => {
|
||||
expect([1, 2, 3, 4, 5, 6].map(restartRetryDelay)).toEqual([
|
||||
5_000, 10_000, 20_000, 30_000, 30_000, 30_000,
|
||||
]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -4,9 +4,11 @@ import { mock } from 'vitest-mock-extended';
|
|||
import type WebSocket from 'ws';
|
||||
|
||||
import { WsStatusCodes } from '@/constants';
|
||||
import type { EventService } from '@/events/event.service';
|
||||
import type { DefaultTaskRunnerDisconnectAnalyzer } from '@/task-runners/default-task-runner-disconnect-analyzer';
|
||||
import { TaskBrokerWsServer } from '@/task-runners/task-broker/task-broker-ws-server';
|
||||
import type { TaskBroker } from '@/task-runners/task-broker/task-broker.service';
|
||||
import { TaskRunnerLifecycleEvents } from '@/task-runners/task-runner-lifecycle-events';
|
||||
|
||||
const globalConfig = mock<GlobalConfig>({ generic: { gracefulShutdownTimeout: 30 } });
|
||||
|
||||
|
|
@ -24,22 +26,37 @@ const mockWs = (readyState = WS_OPEN, isAlive = true) => {
|
|||
return ws;
|
||||
};
|
||||
|
||||
type KnownRunner = NonNullable<ReturnType<ReturnType<TaskBroker['getKnownRunners']>['get']>>;
|
||||
|
||||
const mockTaskBroker = (runners: Array<{ id: string; taskTypes: string[] }> = []) => {
|
||||
const taskBroker = mock<TaskBroker>();
|
||||
taskBroker.getKnownRunners.mockReturnValue(
|
||||
new Map(runners.map((runner) => [runner.id, { runner } as KnownRunner])),
|
||||
);
|
||||
return taskBroker;
|
||||
};
|
||||
|
||||
const createServer = ({
|
||||
taskBroker = mock<TaskBroker>(),
|
||||
taskBroker = mockTaskBroker(),
|
||||
disconnectAnalyzer = mock<DefaultTaskRunnerDisconnectAnalyzer>(),
|
||||
heartbeatInterval = 30,
|
||||
runnerLifecycleEvents = mock<TaskRunnerLifecycleEvents>(),
|
||||
eventService = mock<EventService>(),
|
||||
}: {
|
||||
taskBroker?: TaskBroker;
|
||||
disconnectAnalyzer?: DefaultTaskRunnerDisconnectAnalyzer;
|
||||
heartbeatInterval?: number;
|
||||
runnerLifecycleEvents?: TaskRunnerLifecycleEvents;
|
||||
eventService?: EventService;
|
||||
} = {}) =>
|
||||
new TaskBrokerWsServer(
|
||||
mock(),
|
||||
taskBroker,
|
||||
disconnectAnalyzer,
|
||||
mock<TaskRunnersConfig>({ path: '/runners', heartbeatInterval }),
|
||||
mock(),
|
||||
mock<TaskRunnersConfig>({ path: '/runners', heartbeatInterval, mode: 'internal' }),
|
||||
runnerLifecycleEvents,
|
||||
globalConfig,
|
||||
eventService,
|
||||
);
|
||||
|
||||
const wsMessage = (message: unknown) => Buffer.from(JSON.stringify(message));
|
||||
|
|
@ -222,9 +239,12 @@ describe('TaskBrokerWsServer', () => {
|
|||
describe('heartbeat timer', () => {
|
||||
const DEAD = false;
|
||||
|
||||
const runHeartbeatCheck = async (...connections: WebSocket[]) => {
|
||||
const runHeartbeatCheck = async (
|
||||
connections: WebSocket[],
|
||||
options: Parameters<typeof createServer>[0] = {},
|
||||
) => {
|
||||
vi.useFakeTimers();
|
||||
const server = createServer();
|
||||
const server = createServer(options);
|
||||
|
||||
connections.forEach((ws, i) => server.runnerConnections.set(`runner-${i}`, ws));
|
||||
|
||||
|
|
@ -264,7 +284,7 @@ describe('TaskBrokerWsServer', () => {
|
|||
it('should close connection with protocol error code when heartbeat check fails', async () => {
|
||||
const ws = mockWs(WS_OPEN, DEAD);
|
||||
|
||||
await runHeartbeatCheck(ws);
|
||||
await runHeartbeatCheck([ws]);
|
||||
|
||||
expect(ws.close).toHaveBeenCalledWith(WsStatusCodes.CloseProtocolError);
|
||||
});
|
||||
|
|
@ -273,7 +293,7 @@ describe('TaskBrokerWsServer', () => {
|
|||
const deadWs1 = mockWs(WS_OPEN, DEAD);
|
||||
const deadWs2 = mockWs(WS_OPEN, DEAD);
|
||||
|
||||
await runHeartbeatCheck(deadWs1, deadWs2);
|
||||
await runHeartbeatCheck([deadWs1, deadWs2]);
|
||||
|
||||
expect(deadWs1.close).toHaveBeenCalledWith(WsStatusCodes.CloseProtocolError);
|
||||
expect(deadWs2.close).toHaveBeenCalledWith(WsStatusCodes.CloseProtocolError);
|
||||
|
|
@ -282,12 +302,100 @@ describe('TaskBrokerWsServer', () => {
|
|||
it('should keep pinging live connections listed after a dead one', async () => {
|
||||
const liveWs = mockWs();
|
||||
|
||||
await runHeartbeatCheck(mockWs(WS_OPEN, DEAD), liveWs);
|
||||
await runHeartbeatCheck([mockWs(WS_OPEN, DEAD), liveWs]);
|
||||
|
||||
expect(liveWs.ping).toHaveBeenCalled();
|
||||
expect(liveWs.isAlive).toBe(false);
|
||||
expect(liveWs.close).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should report the dead runner with its task types', async () => {
|
||||
const runnerLifecycleEvents = mock<TaskRunnerLifecycleEvents>();
|
||||
const taskBroker = mockTaskBroker([
|
||||
{ id: 'runner-0', taskTypes: ['python'] },
|
||||
{ id: 'runner-1', taskTypes: ['javascript'] },
|
||||
]);
|
||||
|
||||
await runHeartbeatCheck([mockWs(WS_OPEN, DEAD), mockWs()], {
|
||||
taskBroker,
|
||||
runnerLifecycleEvents,
|
||||
});
|
||||
|
||||
expect(runnerLifecycleEvents.emit).toHaveBeenCalledExactlyOnceWith(
|
||||
'runner:failed-heartbeat-check',
|
||||
{ runnerId: 'runner-0', taskTypes: ['python'] },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('unresponsive runners', () => {
|
||||
it('should disconnect a runner reported unresponsive', async () => {
|
||||
const runnerLifecycleEvents = new TaskRunnerLifecycleEvents();
|
||||
const disconnectAnalyzer = mock<DefaultTaskRunnerDisconnectAnalyzer>();
|
||||
const server = createServer({ disconnectAnalyzer, runnerLifecycleEvents });
|
||||
const ws = mockWs();
|
||||
server.runnerConnections.set('test-runner', ws);
|
||||
server.start();
|
||||
|
||||
runnerLifecycleEvents.emit('runner:unresponsive', {
|
||||
runnerId: 'test-runner',
|
||||
taskTypes: ['javascript'],
|
||||
});
|
||||
|
||||
expect(ws.close).toHaveBeenCalledWith(WsStatusCodes.CloseProtocolError);
|
||||
expect(disconnectAnalyzer.toDisconnectError).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ runnerId: 'test-runner', reason: 'runner-unresponsive' }),
|
||||
);
|
||||
|
||||
await server.stop();
|
||||
});
|
||||
|
||||
it('should stop listening for unresponsive runners on stop', async () => {
|
||||
const runnerLifecycleEvents = new TaskRunnerLifecycleEvents();
|
||||
const server = createServer({ runnerLifecycleEvents });
|
||||
server.start();
|
||||
await server.stop();
|
||||
|
||||
const ws = mockWs();
|
||||
server.runnerConnections.set('test-runner', ws);
|
||||
runnerLifecycleEvents.emit('runner:unresponsive', {
|
||||
runnerId: 'test-runner',
|
||||
taskTypes: ['javascript'],
|
||||
});
|
||||
|
||||
expect(ws.close).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('disconnect reporting', () => {
|
||||
it.each(['failed-heartbeat-check', 'runner-unresponsive'] as const)(
|
||||
'should report a runner disconnected for reason %s',
|
||||
async (reason) => {
|
||||
const eventService = mock<EventService>();
|
||||
const server = createServer({ eventService });
|
||||
server.runnerConnections.set('test-runner', mockWs());
|
||||
|
||||
await server.removeConnection('test-runner', { reason });
|
||||
|
||||
expect(eventService.emit).toHaveBeenCalledWith('runner-disconnected', {
|
||||
reason,
|
||||
mode: 'internal',
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.each(['shutting-down', 'unknown'] as const)(
|
||||
'should not report a runner disconnected for reason %s',
|
||||
async (reason) => {
|
||||
const eventService = mock<EventService>();
|
||||
const server = createServer({ eventService });
|
||||
server.runnerConnections.set('test-runner', mockWs());
|
||||
|
||||
await server.removeConnection('test-runner', { reason });
|
||||
|
||||
expect(eventService.emit).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe('sendMessage', () => {
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ import type { TaskRunnersConfig } from '@n8n/config';
|
|||
import { Time } from '@n8n/constants';
|
||||
import type { BrokerMessage, RunnerMessage, TaskResultData } from '@n8n/task-runner';
|
||||
import { type INodeTypeBaseDescription } from 'n8n-workflow';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
import { mock, type MockProxy } from 'vitest-mock-extended';
|
||||
|
||||
import type { TaskRunnerLifecycleEvents } from '@/task-runners/task-runner-lifecycle-events';
|
||||
|
||||
|
|
@ -12,10 +12,21 @@ import { TaskRequesterAcceptTimeoutError } from '../errors/task-requester-accept
|
|||
import { TaskRunnerExecutionTimeoutError } from '../errors/task-runner-execution-timeout.error';
|
||||
import { TaskRunnerUnreachableError } from '../errors/task-runner-unreachable.error';
|
||||
import { TaskBroker } from '../task-broker.service';
|
||||
import type { TaskOffer, TaskRequest, TaskRunner } from '../task-broker.service';
|
||||
import type {
|
||||
RequesterMessageCallback,
|
||||
TaskOffer,
|
||||
TaskRequest,
|
||||
TaskRunner,
|
||||
} from '../task-broker.service';
|
||||
|
||||
const createValidUntil = (ms: number) => process.hrtime.bigint() + BigInt(ms * 1_000_000);
|
||||
|
||||
const createRunner = (id: string, taskTypes: string[]): TaskRunner => ({
|
||||
id,
|
||||
taskTypes,
|
||||
lastSeen: new Date(),
|
||||
});
|
||||
|
||||
describe('TaskBroker', () => {
|
||||
let taskBroker: TaskBroker;
|
||||
|
||||
|
|
@ -1161,7 +1172,8 @@ describe('TaskBroker', () => {
|
|||
const taskId = 'task1';
|
||||
const runnerId = 'runner1';
|
||||
const requesterId = 'requester1';
|
||||
const runner = mock<TaskRunner>({ id: runnerId });
|
||||
// a literal rather than a mock: `mock` proxies the array, breaking deep equality
|
||||
const runner: TaskRunner = { id: runnerId, taskTypes: ['test'], lastSeen: new Date() };
|
||||
const runnerCallback = vi.fn();
|
||||
const requesterCallback = vi.fn();
|
||||
|
||||
|
|
@ -1178,7 +1190,10 @@ describe('TaskBroker', () => {
|
|||
|
||||
await Promise.resolve();
|
||||
|
||||
expect(runnerLifecycleEvents.emit).toHaveBeenCalledWith('runner:timed-out-during-task');
|
||||
expect(runnerLifecycleEvents.emit).toHaveBeenCalledWith('runner:timed-out-during-task', {
|
||||
runnerId,
|
||||
taskTypes: ['test'],
|
||||
});
|
||||
|
||||
await Promise.resolve();
|
||||
|
||||
|
|
@ -1397,7 +1412,7 @@ describe('TaskBroker', () => {
|
|||
|
||||
const config = mock<TaskRunnersConfig>({ taskRequestTimeout: 60, taskAcceptTimeout: 2 });
|
||||
taskBroker = new TaskBroker(mock(), config, mock(), mock());
|
||||
taskBroker.registerRunner(mock<TaskRunner>({ id: 'runner1' }), vi.fn());
|
||||
taskBroker.registerRunner(mock<TaskRunner>({ id: 'runner1', taskTypes: [] }), vi.fn());
|
||||
|
||||
const requesterCallback = vi.fn();
|
||||
taskBroker.registerRequester('requester1', requesterCallback);
|
||||
|
|
@ -1489,6 +1504,282 @@ describe('TaskBroker', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('unresponsive runner detection', () => {
|
||||
const ACCEPT_TIMEOUT_MS = 2100;
|
||||
|
||||
let lifecycleEvents: MockProxy<TaskRunnerLifecycleEvents>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
lifecycleEvents = mock<TaskRunnerLifecycleEvents>();
|
||||
taskBroker = new TaskBroker(
|
||||
mock(),
|
||||
mock<TaskRunnersConfig>({ taskRequestTimeout: 60, taskTimeout: 60, taskAcceptTimeout: 2 }),
|
||||
lifecycleEvents,
|
||||
mock(),
|
||||
);
|
||||
taskBroker.registerRunner(createRunner('runner1', ['taskType1']), vi.fn());
|
||||
});
|
||||
|
||||
// a failing assertion must not leak fake timers into later tests
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
const offerFrom = (runnerId: string): TaskOffer => ({
|
||||
offerId: 'offer1',
|
||||
runnerId,
|
||||
taskType: 'taskType1',
|
||||
validFor: 10_000,
|
||||
validUntil: createValidUntil(10_000),
|
||||
});
|
||||
|
||||
const requestFor = (): TaskRequest => ({
|
||||
requestId: 'request1',
|
||||
requesterId: 'requester1',
|
||||
taskType: 'taskType1',
|
||||
});
|
||||
|
||||
const timeOutAcceptance = async (runnerId = 'runner1') => {
|
||||
const acceptPromise = taskBroker.acceptOffer(offerFrom(runnerId), requestFor());
|
||||
vi.advanceTimersByTime(ACCEPT_TIMEOUT_MS);
|
||||
await acceptPromise;
|
||||
};
|
||||
|
||||
const answerAcceptance = async (respond: (taskId: string) => void) => {
|
||||
const acceptPromise = taskBroker.acceptOffer(offerFrom('runner1'), requestFor());
|
||||
const [taskId] = taskBroker.getRunnerAcceptRejects().keys();
|
||||
respond(taskId);
|
||||
await acceptPromise;
|
||||
};
|
||||
|
||||
it('should report a runner unresponsive exactly once at the timeout threshold', async () => {
|
||||
await timeOutAcceptance();
|
||||
await timeOutAcceptance();
|
||||
expect(lifecycleEvents.emit).not.toHaveBeenCalled();
|
||||
|
||||
await timeOutAcceptance();
|
||||
expect(lifecycleEvents.emit).toHaveBeenCalledTimes(1);
|
||||
expect(lifecycleEvents.emit).toHaveBeenCalledWith('runner:unresponsive', {
|
||||
runnerId: 'runner1',
|
||||
taskTypes: ['taskType1'],
|
||||
});
|
||||
|
||||
await timeOutAcceptance();
|
||||
expect(lifecycleEvents.emit).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should reset the count when the runner acknowledges an acceptance', async () => {
|
||||
await timeOutAcceptance();
|
||||
await timeOutAcceptance();
|
||||
await answerAcceptance((taskId) => taskBroker.handleRunnerAccept(taskId));
|
||||
|
||||
await timeOutAcceptance();
|
||||
await timeOutAcceptance();
|
||||
|
||||
expect(lifecycleEvents.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reset the count when the runner rejects a task', async () => {
|
||||
await timeOutAcceptance();
|
||||
await timeOutAcceptance();
|
||||
await answerAcceptance((taskId) => taskBroker.handleRunnerReject(taskId, 'at capacity'));
|
||||
|
||||
await timeOutAcceptance();
|
||||
await timeOutAcceptance();
|
||||
|
||||
expect(lifecycleEvents.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reset the count when the runner defers a task', async () => {
|
||||
await timeOutAcceptance();
|
||||
await timeOutAcceptance();
|
||||
await answerAcceptance((taskId) => taskBroker.handleRunnerDeferred(taskId));
|
||||
|
||||
await timeOutAcceptance();
|
||||
await timeOutAcceptance();
|
||||
|
||||
expect(lifecycleEvents.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should clear the count when the runner deregisters', async () => {
|
||||
await timeOutAcceptance();
|
||||
await timeOutAcceptance();
|
||||
|
||||
taskBroker.deregisterRunner('runner1', new Error('connection lost'));
|
||||
taskBroker.registerRunner(createRunner('runner1', ['taskType1']), vi.fn());
|
||||
|
||||
await timeOutAcceptance();
|
||||
await timeOutAcceptance();
|
||||
|
||||
expect(lifecycleEvents.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should count acknowledgment timeouts per runner', async () => {
|
||||
taskBroker.registerRunner(createRunner('runner2', ['taskType1']), vi.fn());
|
||||
|
||||
await timeOutAcceptance('runner1');
|
||||
await timeOutAcceptance('runner1');
|
||||
await timeOutAcceptance('runner2');
|
||||
await timeOutAcceptance('runner2');
|
||||
expect(lifecycleEvents.emit).not.toHaveBeenCalled();
|
||||
|
||||
await timeOutAcceptance('runner2');
|
||||
expect(lifecycleEvents.emit).toHaveBeenCalledTimes(1);
|
||||
expect(lifecycleEvents.emit).toHaveBeenCalledWith('runner:unresponsive', {
|
||||
runnerId: 'runner2',
|
||||
taskTypes: ['taskType1'],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('silent runner detection', () => {
|
||||
const REQUEST_TIMEOUT_MS = 60_000;
|
||||
|
||||
let lifecycleEvents: MockProxy<TaskRunnerLifecycleEvents>;
|
||||
let requesterCallback: ReturnType<typeof vi.fn<RequesterMessageCallback>>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
lifecycleEvents = mock<TaskRunnerLifecycleEvents>();
|
||||
taskBroker = new TaskBroker(
|
||||
mock(),
|
||||
mock<TaskRunnersConfig>({ taskRequestTimeout: 60, taskTimeout: 60, taskAcceptTimeout: 2 }),
|
||||
lifecycleEvents,
|
||||
mock(),
|
||||
);
|
||||
requesterCallback = vi.fn<RequesterMessageCallback>();
|
||||
taskBroker.registerRequester('requester1', requesterCallback);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
const registerRunner = (isRunnerReachable?: () => boolean) => {
|
||||
taskBroker.registerRunner(
|
||||
createRunner('runner1', ['taskType1', 'taskType2']),
|
||||
vi.fn(),
|
||||
isRunnerReachable,
|
||||
);
|
||||
};
|
||||
|
||||
const letRequestExpire = () => {
|
||||
taskBroker.taskRequested({
|
||||
requestId: 'request1',
|
||||
requesterId: 'requester1',
|
||||
taskType: 'taskType1',
|
||||
timeout: taskBroker['createRequestTimeout']('request1'),
|
||||
});
|
||||
vi.advanceTimersByTime(REQUEST_TIMEOUT_MS);
|
||||
};
|
||||
|
||||
it('should report a reachable runner that sent no offers while a request expired', () => {
|
||||
registerRunner();
|
||||
|
||||
letRequestExpire();
|
||||
|
||||
expect(lifecycleEvents.emit).toHaveBeenCalledTimes(1);
|
||||
expect(lifecycleEvents.emit).toHaveBeenCalledWith('runner:unresponsive', {
|
||||
runnerId: 'runner1',
|
||||
taskTypes: ['taskType1', 'taskType2'],
|
||||
});
|
||||
expect(requesterCallback).toHaveBeenCalledWith({
|
||||
type: 'broker:requestexpired',
|
||||
requestId: 'request1',
|
||||
reason: 'timeout',
|
||||
});
|
||||
});
|
||||
|
||||
it('should not report a runner with an in-flight task', () => {
|
||||
registerRunner();
|
||||
taskBroker.setTasks({
|
||||
task1: {
|
||||
id: 'task1',
|
||||
runnerId: 'runner1',
|
||||
requesterId: 'requester1',
|
||||
taskType: 'taskType1',
|
||||
},
|
||||
});
|
||||
|
||||
letRequestExpire();
|
||||
|
||||
expect(lifecycleEvents.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not report a runner with a pending task offer', () => {
|
||||
registerRunner();
|
||||
taskBroker.setPendingTaskOffers([
|
||||
{
|
||||
offerId: 'offer1',
|
||||
runnerId: 'runner1',
|
||||
taskType: 'taskType2',
|
||||
validFor: 300_000,
|
||||
validUntil: createValidUntil(300_000),
|
||||
},
|
||||
]);
|
||||
|
||||
letRequestExpire();
|
||||
|
||||
expect(lifecycleEvents.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not report a runner with a non-expiring launcher offer', () => {
|
||||
registerRunner();
|
||||
taskBroker.setPendingTaskOffers([
|
||||
{
|
||||
offerId: 'offer1',
|
||||
runnerId: 'runner1',
|
||||
taskType: 'taskType2',
|
||||
validFor: -1,
|
||||
validUntil: 0n,
|
||||
},
|
||||
]);
|
||||
|
||||
letRequestExpire();
|
||||
|
||||
expect(lifecycleEvents.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not report a runner with an acceptance in progress', () => {
|
||||
registerRunner();
|
||||
taskBroker.setRunnerAcceptRejects({
|
||||
task1: { accept: vi.fn(), reject: vi.fn(), runnerId: 'runner1' },
|
||||
});
|
||||
|
||||
letRequestExpire();
|
||||
|
||||
expect(lifecycleEvents.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not report an unreachable runner', () => {
|
||||
registerRunner(() => false);
|
||||
|
||||
letRequestExpire();
|
||||
|
||||
expect(lifecycleEvents.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not report a runner that does not support the task type', () => {
|
||||
taskBroker.registerRunner(createRunner('runner1', ['other']), vi.fn());
|
||||
|
||||
letRequestExpire();
|
||||
|
||||
expect(lifecycleEvents.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should expire the request without reporting when no runner is registered', () => {
|
||||
letRequestExpire();
|
||||
|
||||
expect(lifecycleEvents.emit).not.toHaveBeenCalled();
|
||||
expect(requesterCallback).toHaveBeenCalledWith({
|
||||
type: 'broker:requestexpired',
|
||||
requestId: 'request1',
|
||||
reason: 'timeout',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('acceptOffer', () => {
|
||||
// a failing assertion must not leak fake timers into later tests
|
||||
afterEach(() => {
|
||||
|
|
|
|||
|
|
@ -17,7 +17,11 @@ export interface TaskBrokerServerInitRequest
|
|||
|
||||
export type TaskBrokerServerInitResponse = Response & { req: TaskBrokerServerInitRequest };
|
||||
|
||||
export type DisconnectReason = 'shutting-down' | 'failed-heartbeat-check' | 'unknown';
|
||||
export type DisconnectReason =
|
||||
| 'shutting-down'
|
||||
| 'failed-heartbeat-check'
|
||||
| 'runner-unresponsive'
|
||||
| 'unknown';
|
||||
|
||||
export type DisconnectErrorOptions = {
|
||||
runnerId?: TaskRunner['id'];
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import { jsonStringify, UserError } from 'n8n-workflow';
|
|||
import type WebSocket from 'ws';
|
||||
|
||||
import { WsStatusCodes } from '@/constants';
|
||||
import { EventService } from '@/events/event.service';
|
||||
import { DefaultTaskRunnerDisconnectAnalyzer } from '@/task-runners/default-task-runner-disconnect-analyzer';
|
||||
import type {
|
||||
DisconnectAnalyzer,
|
||||
|
|
@ -15,7 +16,10 @@ import type {
|
|||
TaskBrokerServerInitRequest,
|
||||
TaskBrokerServerInitResponse,
|
||||
} from '@/task-runners/task-broker/task-broker-types';
|
||||
import { TaskRunnerLifecycleEvents } from '@/task-runners/task-runner-lifecycle-events';
|
||||
import {
|
||||
TaskRunnerLifecycleEvents,
|
||||
type TaskRunnerLifecycleEventMap,
|
||||
} from '@/task-runners/task-runner-lifecycle-events';
|
||||
|
||||
import { TaskBroker, type MessageCallback, type TaskRunner } from './task-broker.service';
|
||||
|
||||
|
|
@ -53,12 +57,23 @@ export class TaskBrokerWsServer {
|
|||
private readonly taskRunnersConfig: TaskRunnersConfig,
|
||||
private readonly runnerLifecycleEvents: TaskRunnerLifecycleEvents,
|
||||
private readonly globalConfig: GlobalConfig,
|
||||
private readonly eventService: EventService,
|
||||
) {}
|
||||
|
||||
start() {
|
||||
this.startHeartbeatChecks();
|
||||
this.runnerLifecycleEvents.on('runner:unresponsive', this.onRunnerUnresponsive);
|
||||
}
|
||||
|
||||
private readonly onRunnerUnresponsive = ({
|
||||
runnerId,
|
||||
}: TaskRunnerLifecycleEventMap['runner:unresponsive']) => {
|
||||
void this.removeConnection(runnerId, {
|
||||
reason: 'runner-unresponsive',
|
||||
code: WsStatusCodes.CloseProtocolError,
|
||||
});
|
||||
};
|
||||
|
||||
private startHeartbeatChecks() {
|
||||
const { heartbeatInterval } = this.taskRunnersConfig;
|
||||
|
||||
|
|
@ -73,28 +88,30 @@ export class TaskBrokerWsServer {
|
|||
}
|
||||
|
||||
private checkConnectionLiveness() {
|
||||
let anyDead = false;
|
||||
|
||||
for (const [runnerId, connection] of this.runnerConnections) {
|
||||
if (connection.isAlive) {
|
||||
connection.isAlive = false;
|
||||
connection.ping();
|
||||
} else {
|
||||
anyDead = true;
|
||||
const taskTypes = this.taskBroker.getKnownRunners().get(runnerId)?.runner.taskTypes ?? [];
|
||||
|
||||
void this.removeConnection(runnerId, {
|
||||
reason: 'failed-heartbeat-check',
|
||||
code: WsStatusCodes.CloseProtocolError,
|
||||
expectedConnection: connection,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
if (anyDead) {
|
||||
this.runnerLifecycleEvents.emit('runner:failed-heartbeat-check');
|
||||
this.runnerLifecycleEvents.emit('runner:failed-heartbeat-check', {
|
||||
runnerId,
|
||||
taskTypes,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async stop() {
|
||||
this.runnerLifecycleEvents.off('runner:unresponsive', this.onRunnerUnresponsive);
|
||||
|
||||
if (this.heartbeatTimer) {
|
||||
clearInterval(this.heartbeatTimer);
|
||||
this.heartbeatTimer = undefined;
|
||||
|
|
@ -195,6 +212,13 @@ export class TaskBrokerWsServer {
|
|||
this.runnerConnections.delete(id);
|
||||
connection.close(code);
|
||||
|
||||
if (reason === 'failed-heartbeat-check' || reason === 'runner-unresponsive') {
|
||||
this.eventService.emit('runner-disconnected', {
|
||||
reason,
|
||||
mode: this.taskRunnersConfig.mode,
|
||||
});
|
||||
}
|
||||
|
||||
const inFlightTaskIds = this.taskBroker.getInFlightTaskIds(id);
|
||||
|
||||
const disconnectError = await this.disconnectAnalyzer.toDisconnectError({
|
||||
|
|
|
|||
|
|
@ -46,6 +46,8 @@ export interface TaskOffer {
|
|||
|
||||
const MAX_REQUEST_TIMEOUT_REFRESHES = 3;
|
||||
|
||||
const MAX_CONSECUTIVE_ACCEPT_TIMEOUTS = 3;
|
||||
|
||||
export interface TaskRequest {
|
||||
requestId: string;
|
||||
requesterId: string;
|
||||
|
|
@ -98,6 +100,8 @@ export class TaskBroker {
|
|||
{ accept: RequesterAcceptCallback; reject: TaskRejectCallback }
|
||||
> = new Map();
|
||||
|
||||
private consecutiveAcceptTimeouts: Map<TaskRunner['id'], number> = new Map();
|
||||
|
||||
private pendingTaskOffers: TaskOffer[] = [];
|
||||
|
||||
private pendingTaskRequests: TaskRequest[] = [];
|
||||
|
|
@ -150,6 +154,9 @@ export class TaskBroker {
|
|||
if (requestIndex === -1) return;
|
||||
|
||||
const request = this.pendingTaskRequests[requestIndex];
|
||||
|
||||
this.flagSilentRunners(request.taskType);
|
||||
|
||||
this.pendingTaskRequests.splice(requestIndex, 1);
|
||||
this.mismatchWarned.delete(requestId);
|
||||
|
||||
|
|
@ -207,6 +214,7 @@ export class TaskBroker {
|
|||
|
||||
deregisterRunner(runnerId: string, error: Error) {
|
||||
this.knownRunners.delete(runnerId);
|
||||
this.consecutiveAcceptTimeouts.delete(runnerId);
|
||||
|
||||
this.discardOffersFrom(runnerId);
|
||||
|
||||
|
|
@ -320,6 +328,7 @@ export class TaskBroker {
|
|||
handleRunnerAccept(taskId: Task['id']) {
|
||||
const acceptReject = this.runnerAcceptRejects.get(taskId);
|
||||
if (acceptReject) {
|
||||
this.consecutiveAcceptTimeouts.delete(acceptReject.runnerId);
|
||||
acceptReject.accept();
|
||||
this.runnerAcceptRejects.delete(taskId);
|
||||
}
|
||||
|
|
@ -328,6 +337,7 @@ export class TaskBroker {
|
|||
handleRunnerReject(taskId: Task['id'], reason: string) {
|
||||
const acceptReject = this.runnerAcceptRejects.get(taskId);
|
||||
if (acceptReject) {
|
||||
this.consecutiveAcceptTimeouts.delete(acceptReject.runnerId);
|
||||
acceptReject.reject(new TaskRejectError(reason));
|
||||
this.runnerAcceptRejects.delete(taskId);
|
||||
}
|
||||
|
|
@ -336,6 +346,7 @@ export class TaskBroker {
|
|||
handleRunnerDeferred(taskId: Task['id']) {
|
||||
const acceptReject = this.runnerAcceptRejects.get(taskId);
|
||||
if (acceptReject) {
|
||||
this.consecutiveAcceptTimeouts.delete(acceptReject.runnerId);
|
||||
acceptReject.reject(new TaskDeferredError());
|
||||
this.runnerAcceptRejects.delete(taskId);
|
||||
}
|
||||
|
|
@ -578,7 +589,10 @@ export class TaskBroker {
|
|||
if (!task) return;
|
||||
|
||||
if (this.taskRunnersConfig.mode === 'internal') {
|
||||
this.taskRunnerLifecycleEvents.emit('runner:timed-out-during-task');
|
||||
this.taskRunnerLifecycleEvents.emit('runner:timed-out-during-task', {
|
||||
runnerId: task.runnerId,
|
||||
taskTypes: this.knownRunners.get(task.runnerId)?.runner.taskTypes ?? [],
|
||||
});
|
||||
} else if (this.taskRunnersConfig.mode === 'external') {
|
||||
await this.messageRunner(task.runnerId, {
|
||||
type: 'broker:taskcancel',
|
||||
|
|
@ -673,6 +687,7 @@ export class TaskBroker {
|
|||
// A runner missing the acknowledgement window may be gone without its transport
|
||||
// having reported it yet, so its remaining offers cannot be trusted either.
|
||||
this.discardOffersFrom(offer.runnerId);
|
||||
this.flagRunnerIfUnresponsive(offer.runnerId);
|
||||
this.settleTasks();
|
||||
return;
|
||||
}
|
||||
|
|
@ -752,6 +767,74 @@ export class TaskBroker {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Counts consecutive acknowledgement timeouts per runner.
|
||||
* Any application-level reply (accept, reject, defer) proves the channel is alive and resets the count.
|
||||
* At the threshold, the runner is reported unresponsive exactly once,
|
||||
* so its transport can be torn down and the runner restarted.
|
||||
*/
|
||||
private flagRunnerIfUnresponsive(runnerId: TaskRunner['id']) {
|
||||
const failures = (this.consecutiveAcceptTimeouts.get(runnerId) ?? 0) + 1;
|
||||
|
||||
if (failures < MAX_CONSECUTIVE_ACCEPT_TIMEOUTS) {
|
||||
this.consecutiveAcceptTimeouts.set(runnerId, failures);
|
||||
} else {
|
||||
this.consecutiveAcceptTimeouts.delete(runnerId);
|
||||
this.reportUnresponsive(
|
||||
runnerId,
|
||||
`failed to acknowledge ${MAX_CONSECUTIVE_ACCEPT_TIMEOUTS} consecutive task acceptances`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Reports as unresponsive every reachable runner for `taskType` with no sign of life:
|
||||
* no pending offer, no in-flight task, no acceptance in progress.
|
||||
*
|
||||
* A healthy runner with spare capacity keeps offers pending,
|
||||
* so a request expiring next to a silent runner means the runner's offer loop has stalled
|
||||
* and its transport should be torn down so the runner can be restarted.
|
||||
*
|
||||
* A no-op when no runner is registered, which is normal while a runner is still starting up.
|
||||
*/
|
||||
private flagSilentRunners(taskType: string) {
|
||||
this.expireTasks();
|
||||
|
||||
[...this.knownRunners.values()]
|
||||
.filter(({ runner }) => runner.taskTypes.includes(taskType))
|
||||
.filter(({ isRunnerReachable }) => isRunnerReachable())
|
||||
.filter(({ runner }) => this.isSilent(runner.id))
|
||||
.forEach(({ runner }) => {
|
||||
this.reportUnresponsive(runner.id, 'sent no task offers while a task request expired');
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Reports a runner as unresponsive, so its transport can be torn down and, in internal
|
||||
* mode, its process force-restarted. A no-op for a runner that is no longer registered,
|
||||
* as it has nothing left to tear down.
|
||||
*/
|
||||
private reportUnresponsive(runnerId: TaskRunner['id'], cause: string) {
|
||||
const runner = this.knownRunners.get(runnerId)?.runner;
|
||||
if (runner) {
|
||||
this.logger.warn(`Runner (${runnerId}) ${cause}, reporting it as unresponsive`);
|
||||
this.taskRunnerLifecycleEvents.emit('runner:unresponsive', {
|
||||
runnerId,
|
||||
taskTypes: runner.taskTypes,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
private isSilent(runnerId: TaskRunner['id']) {
|
||||
const hasPendingOffer = this.pendingTaskOffers.some((offer) => offer.runnerId === runnerId);
|
||||
const hasInFlightTask = this.getInFlightTaskIds(runnerId).length > 0;
|
||||
const hasAcceptanceInProgress = [...this.runnerAcceptRejects.values()].some(
|
||||
(acceptReject) => acceptReject.runnerId === runnerId,
|
||||
);
|
||||
|
||||
return !hasPendingOffer && !hasInFlightTask && !hasAcceptanceInProgress;
|
||||
}
|
||||
|
||||
// Find matching task offers and requests, then let the runner
|
||||
// know that an offer has been accepted
|
||||
//
|
||||
|
|
|
|||
|
|
@ -1,9 +1,12 @@
|
|||
import { TypedEmitter } from '@n8n/backend-common';
|
||||
import { Service } from '@n8n/di';
|
||||
|
||||
type TaskRunnerLifecycleEventMap = {
|
||||
'runner:failed-heartbeat-check': never;
|
||||
'runner:timed-out-during-task': never;
|
||||
type RunnerIdentity = { runnerId: string; taskTypes: string[] };
|
||||
|
||||
export type TaskRunnerLifecycleEventMap = {
|
||||
'runner:failed-heartbeat-check': RunnerIdentity;
|
||||
'runner:timed-out-during-task': RunnerIdentity;
|
||||
'runner:unresponsive': RunnerIdentity;
|
||||
};
|
||||
|
||||
@Service()
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { Logger, TypedEmitter } from '@n8n/backend-common';
|
|||
import { LogScope, TaskRunnersConfig } from '@n8n/config';
|
||||
import { OnShutdown } from '@n8n/decorators';
|
||||
import { Service } from '@n8n/di';
|
||||
import { sleep } from '@n8n/utils/sleep';
|
||||
import assert from 'node:assert/strict';
|
||||
import { spawn } from 'node:child_process';
|
||||
|
||||
|
|
@ -11,6 +12,13 @@ import { TaskRunnerLifecycleEvents } from './task-runner-lifecycle-events';
|
|||
|
||||
export type ChildProcess = ReturnType<typeof spawn>;
|
||||
|
||||
const RESTART_RETRY_BASE_DELAY_MS = 5_000;
|
||||
const RESTART_RETRY_MAX_DELAY_MS = 30_000;
|
||||
|
||||
export function restartRetryDelay(attempt: number): number {
|
||||
return Math.min(RESTART_RETRY_BASE_DELAY_MS * 2 ** (attempt - 1), RESTART_RETRY_MAX_DELAY_MS);
|
||||
}
|
||||
|
||||
export type ExitReason = 'unknown' | 'oom';
|
||||
|
||||
export type TaskRunnerProcessEventMap = {
|
||||
|
|
@ -23,32 +31,51 @@ export type TaskRunnerProcessEventMap = {
|
|||
export abstract class TaskRunnerProcessBase extends TypedEmitter<TaskRunnerProcessEventMap> {
|
||||
protected readonly name: string;
|
||||
|
||||
protected readonly loggerScope: LogScope;
|
||||
protected readonly taskType: string;
|
||||
|
||||
protected process: ChildProcess | null = null;
|
||||
|
||||
protected _runPromise: Promise<void> | null = null;
|
||||
|
||||
private startPromise: Promise<void> | null = null;
|
||||
|
||||
protected isShuttingDown = false;
|
||||
|
||||
constructor(
|
||||
loggerScope: LogScope,
|
||||
protected readonly logger: Logger,
|
||||
protected readonly runnerConfig: TaskRunnersConfig,
|
||||
protected readonly authService: TaskBrokerAuthService,
|
||||
protected readonly runnerLifecycleEvents: TaskRunnerLifecycleEvents,
|
||||
) {
|
||||
super();
|
||||
this.logger = logger.scoped(this.loggerScope);
|
||||
this.logger = logger.scoped(loggerScope);
|
||||
|
||||
this.runnerLifecycleEvents.on('runner:failed-heartbeat-check', () => {
|
||||
this.logger.warn('Task runner failed heartbeat check, restarting...');
|
||||
void this.forceRestart();
|
||||
this.runnerLifecycleEvents.on('runner:failed-heartbeat-check', ({ taskTypes }) => {
|
||||
this.restartIfOwnRunner(taskTypes, 'failed heartbeat check');
|
||||
});
|
||||
|
||||
this.runnerLifecycleEvents.on('runner:timed-out-during-task', () => {
|
||||
this.logger.warn('Task runner timed out during task, restarting...');
|
||||
void this.forceRestart();
|
||||
this.runnerLifecycleEvents.on('runner:timed-out-during-task', ({ taskTypes }) => {
|
||||
this.restartIfOwnRunner(taskTypes, 'timed out during task');
|
||||
});
|
||||
|
||||
this.runnerLifecycleEvents.on('runner:unresponsive', ({ taskTypes }) => {
|
||||
this.restartIfOwnRunner(taskTypes, 'was reported unresponsive');
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Force-restarts this process if the reported runner is the one it spawned.
|
||||
*
|
||||
* Internal mode runs one process per task type, so the task type is what says
|
||||
* which process to restart.
|
||||
* The runner ID cannot: runners mint their own, so it is never tied to a process n8n spawned.
|
||||
*/
|
||||
private restartIfOwnRunner(taskTypes: string[], cause: string) {
|
||||
if (taskTypes.includes(this.taskType)) {
|
||||
this.logger.warn(`Task runner ${cause}, restarting...`);
|
||||
void this.forceRestart();
|
||||
}
|
||||
}
|
||||
|
||||
get isRunning() {
|
||||
|
|
@ -69,31 +96,54 @@ export abstract class TaskRunnerProcessBase extends TypedEmitter<TaskRunnerProce
|
|||
|
||||
async start() {
|
||||
assert(!this.process, `${this.name} already running`);
|
||||
this.isShuttingDown = false;
|
||||
|
||||
this.startPromise = this.spawnAndMonitor();
|
||||
try {
|
||||
await this.startPromise;
|
||||
} finally {
|
||||
this.startPromise = null;
|
||||
}
|
||||
}
|
||||
|
||||
private async spawnAndMonitor() {
|
||||
const grantToken = await this.authService.createGrantToken();
|
||||
const taskBrokerUri = `http://127.0.0.1:${this.runnerConfig.port}`;
|
||||
const runnerProcess = this.startProcess(grantToken, taskBrokerUri);
|
||||
|
||||
this.process = await this.startProcess(grantToken, taskBrokerUri);
|
||||
forwardToLogger(this.logger, this.process, `[${this.name}] `);
|
||||
this.monitorProcess(this.process);
|
||||
try {
|
||||
forwardToLogger(this.logger, runnerProcess, `[${this.name}] `);
|
||||
this.monitorProcess(runnerProcess);
|
||||
} catch (error) {
|
||||
runnerProcess.kill();
|
||||
throw error;
|
||||
}
|
||||
|
||||
this.process = runnerProcess;
|
||||
}
|
||||
|
||||
/** Stops the runner and prevents any further relaunch. */
|
||||
@OnShutdown()
|
||||
async stop() {
|
||||
if (!this.process) return;
|
||||
|
||||
this.isShuttingDown = true;
|
||||
this.process.kill();
|
||||
await this._runPromise;
|
||||
this.isShuttingDown = false;
|
||||
|
||||
await this.startPromise?.catch(() => {});
|
||||
|
||||
if (this.process) {
|
||||
this.process.kill();
|
||||
await this._runPromise;
|
||||
}
|
||||
}
|
||||
|
||||
/** Force-restart a task runner process suspected of being unresponsive. */
|
||||
/**
|
||||
* Force-restart a task runner process suspected of being unresponsive.
|
||||
* A no-op once shutdown has begun, so a late report cannot revive the runner.
|
||||
*/
|
||||
protected async forceRestart() {
|
||||
if (!this.process) return;
|
||||
|
||||
this.process.kill('SIGKILL');
|
||||
await this._runPromise;
|
||||
if (!this.isShuttingDown && this.process) {
|
||||
this.process.kill('SIGKILL');
|
||||
await this._runPromise;
|
||||
}
|
||||
}
|
||||
|
||||
protected onProcessExit(code: number | null, resolveFn: () => void) {
|
||||
|
|
@ -102,20 +152,59 @@ export abstract class TaskRunnerProcessBase extends TypedEmitter<TaskRunnerProce
|
|||
this.emit('exit', exitReason);
|
||||
resolveFn();
|
||||
if (!this.isShuttingDown) {
|
||||
setImmediate(async () => await this.start());
|
||||
setImmediate(async () => await this.startWithRetries());
|
||||
}
|
||||
}
|
||||
|
||||
private async startWithRetries() {
|
||||
let started = false;
|
||||
let failedAttempts = 0;
|
||||
|
||||
while (!this.isShuttingDown && !started) {
|
||||
try {
|
||||
await this.start();
|
||||
started = true;
|
||||
} catch (error) {
|
||||
failedAttempts++;
|
||||
const delayMs = restartRetryDelay(failedAttempts);
|
||||
this.logger.error(`Failed to start ${this.name}, retrying in ${delayMs}ms`, { error });
|
||||
await sleep(delayMs);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
protected monitorProcess(taskRunnerProcess: ChildProcess) {
|
||||
this.setupProcessMonitoring?.(taskRunnerProcess);
|
||||
|
||||
this._runPromise = new Promise((resolve) => {
|
||||
this.setupProcessMonitoring?.(taskRunnerProcess);
|
||||
taskRunnerProcess.on('exit', (code) => {
|
||||
this.onProcessExit(code, resolve);
|
||||
let exited = false;
|
||||
const onExit = (code: number | null) => {
|
||||
if (!exited) {
|
||||
exited = true;
|
||||
this.onProcessExit(code, resolve);
|
||||
}
|
||||
};
|
||||
|
||||
taskRunnerProcess.on('exit', onExit);
|
||||
|
||||
// A process that failed to spawn (no pid) emits `error` and may never emit `exit`.
|
||||
taskRunnerProcess.on('error', (error) => {
|
||||
this.logger.error(`${this.name} process errored`, { error });
|
||||
if (taskRunnerProcess.pid === undefined) {
|
||||
onExit(null);
|
||||
}
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
abstract startProcess(grantToken: string, taskBrokerUri: string): Promise<ChildProcess>;
|
||||
/**
|
||||
* Spawns the runner process and returns it.
|
||||
*
|
||||
* Synchronous by contract: monitoring is attached to the returned process, and
|
||||
* an `exit` emitted before that attachment is lost, leaving the runner dead
|
||||
* with no relaunch. Any async preparation belongs before the spawn.
|
||||
*/
|
||||
abstract startProcess(grantToken: string, taskBrokerUri: string): ChildProcess;
|
||||
setupProcessMonitoring?(process: ChildProcess): void;
|
||||
analyzeExitReason?(code: number | null): { reason: ExitReason };
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,24 +16,24 @@ import { ChildProcess, ExitReason, TaskRunnerProcessBase } from './task-runner-p
|
|||
*/
|
||||
@Service()
|
||||
export class JsTaskRunnerProcess extends TaskRunnerProcessBase {
|
||||
readonly name = 'runnner:js';
|
||||
protected readonly name = 'runner:js';
|
||||
|
||||
readonly loggerScope = 'task-runner-js';
|
||||
protected readonly taskType = 'javascript';
|
||||
|
||||
private oomDetector: NodeProcessOomDetector | null = null;
|
||||
|
||||
constructor(
|
||||
readonly logger: Logger,
|
||||
readonly runnerConfig: TaskRunnersConfig,
|
||||
readonly authService: TaskBrokerAuthService,
|
||||
readonly runnerLifecycleEvents: TaskRunnerLifecycleEvents,
|
||||
logger: Logger,
|
||||
runnerConfig: TaskRunnersConfig,
|
||||
authService: TaskBrokerAuthService,
|
||||
runnerLifecycleEvents: TaskRunnerLifecycleEvents,
|
||||
) {
|
||||
super(logger, runnerConfig, authService, runnerLifecycleEvents);
|
||||
super('task-runner-js', logger, runnerConfig, authService, runnerLifecycleEvents);
|
||||
|
||||
assert(this.isInternal, `${this.constructor.name} cannot be used in external mode`);
|
||||
}
|
||||
|
||||
async startProcess(grantToken: string, taskBrokerUri: string): Promise<ChildProcess> {
|
||||
startProcess(grantToken: string, taskBrokerUri: string): ChildProcess {
|
||||
const startScript = require.resolve('@n8n/task-runner/start');
|
||||
const flags = this.runnerConfig.insecureMode
|
||||
? []
|
||||
|
|
|
|||
|
|
@ -20,18 +20,18 @@ const asyncExec = promisify(exec);
|
|||
export class PyTaskRunnerProcess extends TaskRunnerProcessBase {
|
||||
protected readonly name = 'runner:py';
|
||||
|
||||
protected readonly loggerScope = 'task-runner-py';
|
||||
protected readonly taskType = 'python';
|
||||
|
||||
constructor(
|
||||
readonly logger: Logger,
|
||||
readonly runnerConfig: TaskRunnersConfig,
|
||||
readonly authService: TaskBrokerAuthService,
|
||||
readonly runnerLifecycleEvents: TaskRunnerLifecycleEvents,
|
||||
logger: Logger,
|
||||
runnerConfig: TaskRunnersConfig,
|
||||
authService: TaskBrokerAuthService,
|
||||
runnerLifecycleEvents: TaskRunnerLifecycleEvents,
|
||||
) {
|
||||
super(logger, runnerConfig, authService, runnerLifecycleEvents);
|
||||
super('task-runner-py', logger, runnerConfig, authService, runnerLifecycleEvents);
|
||||
}
|
||||
|
||||
async startProcess(grantToken: string, taskBrokerUri: string) {
|
||||
startProcess(grantToken: string, taskBrokerUri: string) {
|
||||
const pythonDir = path.join(__dirname, '../../../@n8n/task-runner-python');
|
||||
const venvPath = PyTaskRunnerProcess.getVenvPath();
|
||||
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import {
|
|||
AI_REQUEST_TIMEOUT_MS,
|
||||
createAiMcpFetch,
|
||||
createAiProxyFetch,
|
||||
createWebSearchFetch,
|
||||
} from '@/utils/ai-proxy-fetch';
|
||||
|
||||
describe('AI outbound fetch helpers', () => {
|
||||
|
|
@ -69,4 +70,38 @@ describe('AI outbound fetch helpers', () => {
|
|||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('creates web-search fetch with SSRF protection enabled when configured', () => {
|
||||
const fetch = vi.fn() as typeof global.fetch;
|
||||
const transport = mock<HttpTransport>();
|
||||
transport.asCustomFetch.mockReturnValue(fetch);
|
||||
const outboundHttp = mock<OutboundHttp>();
|
||||
outboundHttp.transport.mockReturnValue(transport);
|
||||
const ssrfConfig = { enabled: true } as SsrfProtectionConfig;
|
||||
const ssrfProtectionService = mock<SsrfProtectionService>();
|
||||
|
||||
expect(createWebSearchFetch(outboundHttp, ssrfConfig, ssrfProtectionService)).toBe(fetch);
|
||||
|
||||
expect(outboundHttp.transport).toHaveBeenCalledWith({
|
||||
proxy: 'env',
|
||||
ssrf: ssrfProtectionService,
|
||||
});
|
||||
});
|
||||
|
||||
it('creates web-search fetch with SSRF protection disabled when configured', () => {
|
||||
const fetch = vi.fn() as typeof global.fetch;
|
||||
const transport = mock<HttpTransport>();
|
||||
transport.asCustomFetch.mockReturnValue(fetch);
|
||||
const outboundHttp = mock<OutboundHttp>();
|
||||
outboundHttp.transport.mockReturnValue(transport);
|
||||
const ssrfConfig = { enabled: false } as SsrfProtectionConfig;
|
||||
const ssrfProtectionService = mock<SsrfProtectionService>();
|
||||
|
||||
expect(createWebSearchFetch(outboundHttp, ssrfConfig, ssrfProtectionService)).toBe(fetch);
|
||||
|
||||
expect(outboundHttp.transport).toHaveBeenCalledWith({
|
||||
proxy: 'env',
|
||||
ssrf: 'disabled',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -65,3 +65,20 @@ export function createAiMcpFetch(
|
|||
})
|
||||
.asCustomFetch();
|
||||
}
|
||||
|
||||
/**
|
||||
* A proxy-aware fetch for web-search providers (e.g. SearXNG) whose base URL
|
||||
* is user-configured, so follow the configured SSRF policy.
|
||||
*/
|
||||
export function createWebSearchFetch(
|
||||
outboundHttp: OutboundHttp,
|
||||
ssrfConfig: SsrfProtectionConfig,
|
||||
ssrfProtectionService: SsrfProtectionService,
|
||||
): CustomFetch {
|
||||
return outboundHttp
|
||||
.transport({
|
||||
proxy: 'env',
|
||||
ssrf: ssrfConfig.enabled ? ssrfProtectionService : 'disabled',
|
||||
})
|
||||
.asCustomFetch();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ import { In, type EntityManager } from '@n8n/typeorm';
|
|||
import omit from 'lodash/omit';
|
||||
import type { INode, IWorkflowBase, WorkflowId } from 'n8n-workflow';
|
||||
import {
|
||||
EXECUTE_WORKFLOW_NODE_TYPE,
|
||||
isNodeWithWorkflowSelector,
|
||||
jsonParse,
|
||||
NodeOperationError,
|
||||
PROJECT_ROOT,
|
||||
|
|
@ -276,34 +276,47 @@ export class EnterpriseWorkflowService {
|
|||
|
||||
/**
|
||||
* Collect every credential id a node references. Besides the node's own
|
||||
* `credentials`, an Execute Sub-workflow node with an inline source embeds a
|
||||
* whole workflow — including its nodes' credentials — inside a single string
|
||||
* `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.
|
||||
*
|
||||
* An inline sub-workflow may itself contain inline sub-workflows, so the walk
|
||||
* is iterative over an explicit stack: every referenced credential is
|
||||
* inspected regardless of nesting depth, with no silent cut-off that could let
|
||||
* a deeply nested reference escape validation. Nesting is inherently bounded by
|
||||
* the request size (each level embeds its child as literal escaped JSON) and
|
||||
* cannot cycle, so the stack cannot grow unbounded.
|
||||
*/
|
||||
private getCredentialIdsUsedByNode(node: INode): string[] {
|
||||
const credentialIds: string[] = [];
|
||||
const stack: INode[] = [node];
|
||||
|
||||
if (node.credentials) {
|
||||
for (const nodeCred of Object.values(node.credentials)) {
|
||||
const id = nodeCred.id?.toString();
|
||||
if (id) credentialIds.push(id);
|
||||
while (stack.length > 0) {
|
||||
const current = stack.pop()!;
|
||||
|
||||
if (current.credentials) {
|
||||
for (const nodeCred of Object.values(current.credentials)) {
|
||||
const id = nodeCred.id?.toString();
|
||||
if (id) credentialIds.push(id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (node.type === EXECUTE_WORKFLOW_NODE_TYPE) {
|
||||
credentialIds.push(...this.getCredentialIdsFromInlineWorkflow(node.parameters?.workflowJson));
|
||||
if (isNodeWithWorkflowSelector(current)) {
|
||||
stack.push(...this.getInlineWorkflowNodes(current.parameters?.workflowJson));
|
||||
}
|
||||
}
|
||||
|
||||
return credentialIds;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the credential ids referenced by the nodes of an inline sub-workflow
|
||||
* passed as the `workflowJson` parameter of an Execute Sub-workflow node.
|
||||
* Non-string or unparseable values reference no resolvable credentials and are
|
||||
* ignored (the node would fail at run time).
|
||||
* Parse the nodes of an inline sub-workflow passed as the `workflowJson`
|
||||
* parameter of a workflow-selector node. Non-string or unparseable values
|
||||
* reference no resolvable credentials and yield no nodes (the node would fail
|
||||
* at run time).
|
||||
*/
|
||||
private getCredentialIdsFromInlineWorkflow(workflowJson: unknown): string[] {
|
||||
private getInlineWorkflowNodes(workflowJson: unknown): INode[] {
|
||||
if (typeof workflowJson !== 'string') return [];
|
||||
|
||||
let parsed: Partial<IWorkflowBase>;
|
||||
|
|
@ -314,15 +327,7 @@ export class EnterpriseWorkflowService {
|
|||
}
|
||||
if (!parsed || !Array.isArray(parsed.nodes)) return [];
|
||||
|
||||
const credentialIds: string[] = [];
|
||||
for (const subNode of parsed.nodes) {
|
||||
if (!subNode?.credentials) continue;
|
||||
for (const nodeCred of Object.values(subNode.credentials)) {
|
||||
const id = nodeCred?.id?.toString();
|
||||
if (id) credentialIds.push(id);
|
||||
}
|
||||
}
|
||||
return credentialIds;
|
||||
return parsed.nodes.filter((subNode): subNode is INode => Boolean(subNode));
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
import { createTeamProject, linkUserToProject, testDb } from '@n8n/backend-test-utils';
|
||||
import { SsrfProtectionService } from '@n8n/backend-network';
|
||||
import { SsrfProtectionConfig } from '@n8n/config';
|
||||
import type { CredentialsEntity, User } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import { response as Response } from 'express';
|
||||
|
|
@ -487,4 +489,179 @@ describe('OAuth2 API', () => {
|
|||
expect(credentials.oauthTokenData).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('access token exchange network restrictions', () => {
|
||||
let ssrfProtectionConfig: SsrfProtectionConfig;
|
||||
let originalEnabled: boolean;
|
||||
|
||||
beforeAll(() => {
|
||||
ssrfProtectionConfig = Container.get(SsrfProtectionConfig);
|
||||
originalEnabled = ssrfProtectionConfig.enabled;
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
nock.cleanAll();
|
||||
ssrfProtectionConfig.enabled = true;
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
ssrfProtectionConfig.enabled = originalEnabled;
|
||||
});
|
||||
|
||||
it('should apply the same network access restrictions as other outbound OAuth requests', async () => {
|
||||
const restrictedAccessTokenUrl = 'http://169.254.169.254/latest/meta-data/token';
|
||||
const restrictedCredential = await saveCredential(
|
||||
{
|
||||
name: 'Restricted target',
|
||||
type: 'testOAuth2Api',
|
||||
data: { ...credentialData, accessTokenUrl: restrictedAccessTokenUrl },
|
||||
},
|
||||
{ user: owner, role: 'credential:owner' },
|
||||
);
|
||||
|
||||
const oauthService = Container.get(OauthService);
|
||||
const csrfSpy = vi.spyOn(oauthService, 'createCsrfState').mockClear();
|
||||
const renderSpy = vi.spyOn(Response, 'render').mockImplementation(function (this: any) {
|
||||
this.end();
|
||||
return this;
|
||||
});
|
||||
|
||||
await ownerAgent
|
||||
.get('/oauth2-credential/auth')
|
||||
.query({ id: restrictedCredential.id })
|
||||
.expect(200);
|
||||
const [, state] = await csrfSpy.mock.results[0].value;
|
||||
|
||||
const validateUrlSpy = vi.spyOn(Container.get(SsrfProtectionService), 'validateUrl');
|
||||
|
||||
const tokenScope = nock('http://169.254.169.254')
|
||||
.post('/latest/meta-data/token')
|
||||
.reply(400, { error: 'invalid_client', error_description: 'internal-response-marker' });
|
||||
|
||||
await ownerAgent
|
||||
.get('/oauth2-credential/callback')
|
||||
.query({ code: 'auth_code', state })
|
||||
.expect(200);
|
||||
|
||||
expect(validateUrlSpy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ href: restrictedAccessTokenUrl }),
|
||||
);
|
||||
expect(tokenScope.isDone()).toBe(false);
|
||||
expect(renderSpy).not.toHaveBeenCalledWith(
|
||||
'oauth-error-callback',
|
||||
expect.objectContaining({
|
||||
error: expect.objectContaining({
|
||||
reason: expect.stringContaining('internal-response-marker'),
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should complete the exchange against any reachable endpoint when restrictions are off', async () => {
|
||||
// An instance that leaves the guard off must keep working against endpoints the
|
||||
// enabled configuration would reject, e.g. a self-hosted OAuth server on the LAN.
|
||||
ssrfProtectionConfig.enabled = false;
|
||||
|
||||
const localAccessTokenUrl = 'http://10.20.30.40/oauth2/token';
|
||||
const localCredential = await saveCredential(
|
||||
{
|
||||
name: 'Self-hosted target',
|
||||
type: 'testOAuth2Api',
|
||||
data: { ...credentialData, accessTokenUrl: localAccessTokenUrl },
|
||||
},
|
||||
{ user: owner, role: 'credential:owner' },
|
||||
);
|
||||
|
||||
const oauthService = Container.get(OauthService);
|
||||
const csrfSpy = vi.spyOn(oauthService, 'createCsrfState').mockClear();
|
||||
const renderSpy = vi.spyOn(Response, 'render').mockImplementation(function (this: any) {
|
||||
this.end();
|
||||
return this;
|
||||
});
|
||||
|
||||
await ownerAgent.get('/oauth2-credential/auth').query({ id: localCredential.id }).expect(200);
|
||||
const [, state] = await csrfSpy.mock.results[0].value;
|
||||
|
||||
const validateUrlSpy = vi.spyOn(Container.get(SsrfProtectionService), 'validateUrl');
|
||||
|
||||
const tokenScope = nock('http://10.20.30.40')
|
||||
.post('/oauth2/token')
|
||||
.reply(200, { access_token: 'self_hosted_token' });
|
||||
|
||||
await ownerAgent
|
||||
.get('/oauth2-credential/callback')
|
||||
.query({ code: 'auth_code', state })
|
||||
.expect(200);
|
||||
|
||||
expect(tokenScope.isDone()).toBe(true);
|
||||
expect(validateUrlSpy).not.toHaveBeenCalled();
|
||||
expect(renderSpy).toHaveBeenCalledWith('oauth-callback');
|
||||
|
||||
const stored = await getCredentialById(localCredential.id);
|
||||
const decrypted = (await decryptCredentialData(stored!)) as Record<string, unknown>;
|
||||
expect(decrypted.oauthTokenData).toEqual(
|
||||
expect.objectContaining({ access_token: 'self_hosted_token' }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('callback error detail', () => {
|
||||
// Earlier tests leave unconsumed token-endpoint interceptors behind, which would
|
||||
// otherwise answer these requests with a success response.
|
||||
beforeEach(() => {
|
||||
nock.cleanAll();
|
||||
});
|
||||
|
||||
const startFlowAndSpyOnRender = async () => {
|
||||
const oauthService = Container.get(OauthService);
|
||||
const csrfSpy = vi.spyOn(oauthService, 'createCsrfState').mockClear();
|
||||
const renderSpy = vi.spyOn(Response, 'render').mockImplementation(function (this: any) {
|
||||
this.end();
|
||||
return this;
|
||||
});
|
||||
|
||||
await ownerAgent.get('/oauth2-credential/auth').query({ id: credential.id }).expect(200);
|
||||
const [, state] = await csrfSpy.mock.results[0].value;
|
||||
|
||||
return { renderSpy, state };
|
||||
};
|
||||
|
||||
it('should render the OAuth2 error code without the accompanying description', async () => {
|
||||
const { renderSpy, state } = await startFlowAndSpyOnRender();
|
||||
|
||||
nock('https://test.domain')
|
||||
.post('/oauth2/token')
|
||||
.reply(400, { error: 'invalid_client', error_description: 'upstream-response-marker' });
|
||||
|
||||
await ownerAgent
|
||||
.get('/oauth2-credential/callback')
|
||||
.query({ code: 'auth_code', state })
|
||||
.expect(200);
|
||||
|
||||
expect(renderSpy).toHaveBeenCalledWith(
|
||||
'oauth-error-callback',
|
||||
expect.objectContaining({
|
||||
error: expect.objectContaining({ reason: 'invalid_client' }),
|
||||
}),
|
||||
);
|
||||
expect(JSON.stringify(renderSpy.mock.calls)).not.toContain('upstream-response-marker');
|
||||
});
|
||||
|
||||
it('should not render the token endpoint response body when it is not an OAuth2 error', async () => {
|
||||
const { renderSpy, state } = await startFlowAndSpyOnRender();
|
||||
|
||||
nock('https://test.domain')
|
||||
.post('/oauth2/token')
|
||||
.reply(400, { internalField: 'upstream-response-marker' });
|
||||
|
||||
await ownerAgent
|
||||
.get('/oauth2-credential/callback')
|
||||
.query({ code: 'auth_code', state })
|
||||
.expect(200);
|
||||
|
||||
expect(renderSpy).toHaveBeenCalledWith('oauth-error-callback', {
|
||||
error: { message: 'HTTP status 400', reason: undefined },
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1128,12 +1128,12 @@ describe('RoleController', () => {
|
|||
expect(roleService.removeCustomRole).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should forward the reassignRoleSlug query param to the service', async () => {
|
||||
it('should forward the reassignRoleSlug query param to the service for an entitled caller', async () => {
|
||||
//
|
||||
// ARRANGE
|
||||
//
|
||||
const roleSlug = 'global:test-role';
|
||||
roleService.removeCustomRole.mockResolvedValue({
|
||||
const globalRole: Role = {
|
||||
slug: roleSlug,
|
||||
displayName: 'Deleted Role',
|
||||
description: null,
|
||||
|
|
@ -1141,7 +1141,9 @@ describe('RoleController', () => {
|
|||
roleType: 'global',
|
||||
scopes: [],
|
||||
licensed: true,
|
||||
});
|
||||
};
|
||||
roleService.getRole.mockResolvedValue(globalRole);
|
||||
roleService.removeCustomRole.mockResolvedValue(globalRole);
|
||||
|
||||
//
|
||||
// ACT
|
||||
|
|
@ -1154,6 +1156,43 @@ describe('RoleController', () => {
|
|||
expect(roleService.removeCustomRole).toHaveBeenCalledWith(roleSlug, 'global:member');
|
||||
});
|
||||
|
||||
it('should reject reassignment to the owner role', async () => {
|
||||
//
|
||||
// ACT & ASSERT — global:owner is not an assignable target (DTO validation)
|
||||
//
|
||||
await ownerAgent.delete('/roles/global:test-role?reassignRoleSlug=global:owner').expect(400);
|
||||
|
||||
expect(roleService.removeCustomRole).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should ignore the reassignment target for project roles', async () => {
|
||||
//
|
||||
// ARRANGE
|
||||
//
|
||||
const roleSlug = 'project:test-role';
|
||||
const projectRole: Role = {
|
||||
slug: roleSlug,
|
||||
displayName: 'Project Role',
|
||||
description: null,
|
||||
systemRole: false,
|
||||
roleType: 'project',
|
||||
scopes: ['workflow:read'],
|
||||
licensed: true,
|
||||
};
|
||||
roleService.getRole.mockResolvedValue(projectRole);
|
||||
roleService.removeCustomRole.mockResolvedValue(projectRole);
|
||||
|
||||
//
|
||||
// ACT — reassignment is not supported for project roles, so the target is dropped
|
||||
//
|
||||
await ownerAgent.delete(`/roles/${roleSlug}?reassignRoleSlug=project:admin`).expect(200);
|
||||
|
||||
//
|
||||
// ASSERT
|
||||
//
|
||||
expect(roleService.removeCustomRole).toHaveBeenCalledWith(roleSlug, undefined);
|
||||
});
|
||||
|
||||
it('should handle service errors gracefully', async () => {
|
||||
//
|
||||
// ARRANGE
|
||||
|
|
|
|||
|
|
@ -761,6 +761,51 @@ describe('OIDC service', () => {
|
|||
expect(user.email).toEqual('user3@example.com');
|
||||
});
|
||||
|
||||
it('should reject linking to an existing local account when email_verified is false', async () => {
|
||||
await createUser({ email: 'link-target@example.com' });
|
||||
|
||||
const state = oidcService.generateState();
|
||||
const nonce = oidcService.generateNonce();
|
||||
const callbackUrl = new URL(
|
||||
`http://localhost:5678/rest/sso/oidc/callback?code=valid-code&state=${state.plaintext}`,
|
||||
);
|
||||
|
||||
const mockTokens: mocked_oidc_client.TokenEndpointResponse &
|
||||
mocked_oidc_client.TokenEndpointResponseHelpers = {
|
||||
access_token: 'mock-access-token-unverified-link',
|
||||
id_token: 'mock-id-token-unverified-link',
|
||||
token_type: 'bearer',
|
||||
claims: () => {
|
||||
return {
|
||||
sub: 'attacker-subject-unverified',
|
||||
iss: 'https://example.com/auth/realms/n8n',
|
||||
aud: 'test-client-id',
|
||||
iat: Math.floor(Date.now() / 1000) - 1000,
|
||||
exp: Math.floor(Date.now() / 1000) + 3600,
|
||||
} as mocked_oidc_client.IDToken;
|
||||
},
|
||||
expiresIn: () => 3600,
|
||||
} as mocked_oidc_client.TokenEndpointResponse &
|
||||
mocked_oidc_client.TokenEndpointResponseHelpers;
|
||||
|
||||
authorizationCodeGrantMock.mockResolvedValueOnce(mockTokens);
|
||||
fetchUserInfoMock.mockResolvedValueOnce({
|
||||
email_verified: false,
|
||||
email: 'link-target@example.com',
|
||||
});
|
||||
|
||||
await expect(
|
||||
oidcService.loginUser(callbackUrl, state.signed, nonce.signed),
|
||||
).rejects.toThrowError(BadRequestError);
|
||||
|
||||
// The attacker's OIDC identity must not be linked to the existing account.
|
||||
const victim = await userRepository.findOne({
|
||||
where: { email: 'link-target@example.com' },
|
||||
relations: ['authIdentities'],
|
||||
});
|
||||
expect(victim!.authIdentities).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should throw `BadRequestError` if OIDC Idp does not provide an email', async () => {
|
||||
const state = oidcService.generateState();
|
||||
const nonce = oidcService.generateNonce();
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ const validConfig = {
|
|||
prompt: 'consent' as const,
|
||||
authenticationContextClassReference: ['mfa'],
|
||||
additionalScopes: 'groups',
|
||||
emailVerifiedRequired: false,
|
||||
rpInitiatedLogoutEnabled: false,
|
||||
};
|
||||
|
||||
|
|
@ -103,6 +104,7 @@ describe('OIDC SSO configuration in Public API', () => {
|
|||
'clientId',
|
||||
'clientSecret',
|
||||
'discoveryEndpoint',
|
||||
'emailVerifiedRequired',
|
||||
'loginEnabled',
|
||||
'prompt',
|
||||
'rpInitiatedLogoutEnabled',
|
||||
|
|
@ -225,6 +227,32 @@ describe('OIDC SSO configuration in Public API', () => {
|
|||
expect(read.body.rpInitiatedLogoutEnabled).toBe(true);
|
||||
});
|
||||
|
||||
it('persists emailVerifiedRequired', async () => {
|
||||
testServer.license.enable('feat:oidc');
|
||||
|
||||
const response = await testServer
|
||||
.publicApiAgentFor(owner)
|
||||
.put('/settings/sso/oidc')
|
||||
.send({ ...validConfig, emailVerifiedRequired: true });
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body.emailVerifiedRequired).toBe(true);
|
||||
|
||||
const read = await testServer.publicApiAgentFor(owner).get('/settings/sso/oidc');
|
||||
expect(read.body.emailVerifiedRequired).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects a partial body missing emailVerifiedRequired with 400', async () => {
|
||||
testServer.license.enable('feat:oidc');
|
||||
const { emailVerifiedRequired: _emailVerifiedRequired, ...partial } = validConfig;
|
||||
|
||||
const response = await testServer
|
||||
.publicApiAgentFor(owner)
|
||||
.put('/settings/sso/oidc')
|
||||
.send(partial);
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
});
|
||||
|
||||
it('keeps the stored secret when the redacted sentinel is submitted', async () => {
|
||||
testServer.license.enable('feat:oidc');
|
||||
await testServer.publicApiAgentFor(owner).put('/settings/sso/oidc').send(validConfig);
|
||||
|
|
|
|||
|
|
@ -1,5 +1,9 @@
|
|||
import { WorkflowEntity, WorkflowHistory } from '@n8n/db';
|
||||
import { EXECUTE_WORKFLOW_NODE_TYPE, type INode } from 'n8n-workflow';
|
||||
import {
|
||||
EXECUTE_WORKFLOW_NODE_TYPE,
|
||||
WORKFLOW_TOOL_LANGCHAIN_NODE_TYPE,
|
||||
type INode,
|
||||
} from 'n8n-workflow';
|
||||
|
||||
export const FIRST_CREDENTIAL_ID = '1';
|
||||
export const SECOND_CREDENTIAL_ID = '2';
|
||||
|
|
@ -9,6 +13,9 @@ const NODE_WITH_NO_CRED = '0133467b-df4a-473d-9295-fdd9d01fa45a';
|
|||
const NODE_WITH_ONE_CRED = '4673f869-f2dc-4a33-b053-ca3193bc5226';
|
||||
const NODE_WITH_TWO_CRED = '9b4208bd-8f10-4a6a-ad3b-da47a326f7da';
|
||||
const NODE_WITH_INLINE_SUBWORKFLOW_CRED = 'a1f8c2e0-1b2c-4d3e-9f0a-1234567890ab';
|
||||
const NODE_WITH_WORKFLOW_TOOL_INLINE_CRED = 'b2e9d3f1-2c3d-4e5f-a0b1-234567890abc';
|
||||
const NODE_WITH_NESTED_INLINE_SUBWORKFLOW_CRED = 'c3f0e4a2-3d4e-5f60-b1c2-34567890abcd';
|
||||
const NODE_WITH_DEEPLY_NESTED_INLINE_CRED = 'd4a1f5b3-4e5f-6071-c2d3-4567890abcde';
|
||||
|
||||
const nodeWithNoCredentials: INode = {
|
||||
id: NODE_WITH_NO_CRED,
|
||||
|
|
@ -82,11 +89,106 @@ const nodeWithInlineSubworkflowCredential: INode = {
|
|||
},
|
||||
};
|
||||
|
||||
// Builds the inline `workflowJson` of a sub-workflow containing a single
|
||||
// HTTP Request node that references `credentialId`.
|
||||
const inlineWorkflowJson = (credentialId: string) =>
|
||||
JSON.stringify({
|
||||
nodes: [
|
||||
{
|
||||
name: 'Steal',
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
typeVersion: 4.2,
|
||||
parameters: {},
|
||||
credentials: {
|
||||
httpHeaderAuth: { id: credentialId, name: 'Inline fake credential' },
|
||||
},
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
});
|
||||
|
||||
// Workflow Tool (LangChain) node whose inline `workflowJson` embeds a
|
||||
// sub-workflow referencing a credential (FIRST_CREDENTIAL_ID) not exposed via
|
||||
// the node's own top-level `credentials`. Same inline-executor path as the
|
||||
// Execute Sub-workflow node, so it must be inspected too.
|
||||
const nodeWithWorkflowToolInlineCredential: INode = {
|
||||
id: NODE_WITH_WORKFLOW_TOOL_INLINE_CRED,
|
||||
name: 'Workflow Tool',
|
||||
typeVersion: 2,
|
||||
type: WORKFLOW_TOOL_LANGCHAIN_NODE_TYPE,
|
||||
position: [0, 0],
|
||||
parameters: {
|
||||
source: 'parameter',
|
||||
workflowJson: inlineWorkflowJson(FIRST_CREDENTIAL_ID),
|
||||
},
|
||||
};
|
||||
|
||||
// Execute Sub-workflow node whose inline sub-workflow itself contains another
|
||||
// Execute Sub-workflow node with an inline sub-workflow referencing
|
||||
// SECOND_CREDENTIAL_ID — exercises the recursive walk into nested inline
|
||||
// workflows.
|
||||
const nodeWithNestedInlineSubworkflowCredential: INode = {
|
||||
id: NODE_WITH_NESTED_INLINE_SUBWORKFLOW_CRED,
|
||||
name: 'Execute Sub-workflow (nested)',
|
||||
typeVersion: 1.2,
|
||||
type: EXECUTE_WORKFLOW_NODE_TYPE,
|
||||
position: [0, 0],
|
||||
parameters: {
|
||||
source: 'parameter',
|
||||
workflowJson: JSON.stringify({
|
||||
nodes: [
|
||||
{
|
||||
name: 'Inner Execute Sub-workflow',
|
||||
type: EXECUTE_WORKFLOW_NODE_TYPE,
|
||||
typeVersion: 1.2,
|
||||
parameters: {
|
||||
source: 'parameter',
|
||||
workflowJson: inlineWorkflowJson(SECOND_CREDENTIAL_ID),
|
||||
},
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
}),
|
||||
},
|
||||
};
|
||||
|
||||
// Execute Sub-workflow node whose inline sub-workflow nests `depth` further
|
||||
// inline Execute Sub-workflow nodes, with the credential reference buried at the
|
||||
// deepest level. Used to prove the credential collector inspects every level and
|
||||
// never silently stops before reaching the bottom.
|
||||
const buildDeeplyNestedInlineNode = (depth: number): INode => {
|
||||
let workflowJson = inlineWorkflowJson(FIRST_CREDENTIAL_ID);
|
||||
for (let i = 0; i < depth; i++) {
|
||||
workflowJson = JSON.stringify({
|
||||
nodes: [
|
||||
{
|
||||
name: `Nested Execute Sub-workflow ${i}`,
|
||||
type: EXECUTE_WORKFLOW_NODE_TYPE,
|
||||
typeVersion: 1.2,
|
||||
parameters: { source: 'parameter', workflowJson },
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
});
|
||||
}
|
||||
return {
|
||||
id: NODE_WITH_DEEPLY_NESTED_INLINE_CRED,
|
||||
name: 'Execute Sub-workflow (deeply nested)',
|
||||
typeVersion: 1.2,
|
||||
type: EXECUTE_WORKFLOW_NODE_TYPE,
|
||||
position: [0, 0],
|
||||
parameters: { source: 'parameter', workflowJson },
|
||||
};
|
||||
};
|
||||
|
||||
export function getWorkflow(options?: {
|
||||
addNodeWithoutCreds?: boolean;
|
||||
addNodeWithOneCred?: boolean;
|
||||
addNodeWithTwoCreds?: boolean;
|
||||
addNodeWithInlineSubworkflowCred?: boolean;
|
||||
addNodeWithWorkflowToolInlineCred?: boolean;
|
||||
addNodeWithNestedInlineSubworkflowCred?: boolean;
|
||||
addNodeWithDeeplyNestedInlineCred?: boolean;
|
||||
}) {
|
||||
const workflow = new WorkflowEntity();
|
||||
|
||||
|
|
@ -108,6 +210,19 @@ export function getWorkflow(options?: {
|
|||
workflow.nodes.push(nodeWithInlineSubworkflowCredential);
|
||||
}
|
||||
|
||||
if (options?.addNodeWithWorkflowToolInlineCred) {
|
||||
workflow.nodes.push(nodeWithWorkflowToolInlineCredential);
|
||||
}
|
||||
|
||||
if (options?.addNodeWithNestedInlineSubworkflowCred) {
|
||||
workflow.nodes.push(nodeWithNestedInlineSubworkflowCredential);
|
||||
}
|
||||
|
||||
if (options?.addNodeWithDeeplyNestedInlineCred) {
|
||||
// 15 levels deep — beyond any small fixed traversal budget.
|
||||
workflow.nodes.push(buildDeeplyNestedInlineNode(15));
|
||||
}
|
||||
|
||||
return workflow;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -233,6 +233,38 @@ describe('JS TaskRunner execution on internal mode', () => {
|
|||
}),
|
||||
});
|
||||
});
|
||||
|
||||
// The host's `Function.prototype` is reachable from user code
|
||||
// via `$input.constructor.constructor.prototype`, so it must be frozen
|
||||
// like the other host prototypes to keep its members immutable.
|
||||
it('should freeze the host Function.prototype so it cannot be mutated', async () => {
|
||||
// Act
|
||||
const result = await runTaskWithCode(`
|
||||
const hostFunctionPrototype = $input.constructor.constructor.prototype;
|
||||
const originalApply = hostFunctionPrototype.apply;
|
||||
let applyReassigned = false;
|
||||
let propertyAdded = false;
|
||||
try { hostFunctionPrototype.apply = function () {}; } catch (e) {}
|
||||
applyReassigned = hostFunctionPrototype.apply !== originalApply;
|
||||
try { hostFunctionPrototype.injected = 'value'; } catch (e) {}
|
||||
propertyAdded = hostFunctionPrototype.injected === 'value';
|
||||
return {
|
||||
frozen: Object.isFrozen(hostFunctionPrototype),
|
||||
applyReassigned,
|
||||
propertyAdded,
|
||||
};
|
||||
`);
|
||||
|
||||
// Assert
|
||||
expect(result).toEqual({
|
||||
ok: true,
|
||||
result: {
|
||||
frozen: true,
|
||||
applyReassigned: false,
|
||||
propertyAdded: false,
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('Internal and external libs', () => {
|
||||
|
|
@ -308,5 +340,92 @@ describe('JS TaskRunner execution on internal mode', () => {
|
|||
}),
|
||||
});
|
||||
});
|
||||
|
||||
// A host module object exposes the host's own reflection via its
|
||||
// constructor (`require('crypto').constructor.getPrototypeOf`), which
|
||||
// bypasses the sandbox's shimmed `Object.getPrototypeOf` and lets user
|
||||
// code walk a host object's prototype chain up to the internal
|
||||
// `EventEmitter` prototype.
|
||||
const walkToEventEmitterPrototype = `
|
||||
const crypto = require('crypto');
|
||||
const hostGetProto = crypto.constructor.getPrototypeOf;
|
||||
const hostGetOwnDesc = crypto.constructor.getOwnPropertyDescriptor;
|
||||
|
||||
let proto = crypto.createHash('sha256');
|
||||
let emitterProto = null;
|
||||
for (let i = 0; i < 12; i++) {
|
||||
proto = hostGetProto(proto);
|
||||
if (!proto) break;
|
||||
if (hostGetOwnDesc(proto, 'emit') && hostGetOwnDesc(proto, 'on')) {
|
||||
emitterProto = proto;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
const methods = ['emit', 'on', 'once', 'addListener', 'prependListener', 'prependOnceListener'];
|
||||
`;
|
||||
|
||||
// `process` inherits `emit` (and the listener registration methods) from
|
||||
// the internal `EventEmitter` prototype, so overriding one would
|
||||
// intercept a host `process.emit(...)` call, hand user code a reference
|
||||
// to `process`, and allow requiring arbitrary modules. Those members
|
||||
// must therefore stay immutable.
|
||||
it('should keep EventEmitter members reachable from host modules immutable', async () => {
|
||||
// Act
|
||||
const result = await runTaskWithCode(`
|
||||
${walkToEventEmitterPrototype}
|
||||
|
||||
const overridden = [];
|
||||
for (const method of methods) {
|
||||
const original = emitterProto[method];
|
||||
try { emitterProto[method] = function () {}; } catch (e) {}
|
||||
if (emitterProto[method] !== original) overridden.push(method);
|
||||
}
|
||||
|
||||
return {
|
||||
found: emitterProto !== null,
|
||||
overridden,
|
||||
};
|
||||
`);
|
||||
|
||||
// Assert
|
||||
expect(result).toEqual({
|
||||
ok: true,
|
||||
result: {
|
||||
found: true,
|
||||
overridden: [],
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
// `EventEmitter.prototype` is shared by the whole process, so locking
|
||||
// these members must not make them non-enumerable: that would hide them
|
||||
// from `for...in`, `Object.keys` and spreads on every emitter, changing
|
||||
// behaviour host code and third-party modules already rely on.
|
||||
it('should keep locked EventEmitter members enumerable', async () => {
|
||||
// Act
|
||||
const result = await runTaskWithCode(`
|
||||
${walkToEventEmitterPrototype}
|
||||
|
||||
const nonEnumerable = [];
|
||||
for (const method of methods) {
|
||||
if (!hostGetOwnDesc(emitterProto, method).enumerable) nonEnumerable.push(method);
|
||||
}
|
||||
|
||||
return {
|
||||
found: emitterProto !== null,
|
||||
nonEnumerable,
|
||||
};
|
||||
`);
|
||||
|
||||
// Assert
|
||||
expect(result).toEqual({
|
||||
ok: true,
|
||||
result: {
|
||||
found: true,
|
||||
nonEnumerable: [],
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Some files were not shown because too many files have changed in this diff Show More
Loading…
Reference in New Issue
Block a user