From 1cd2c5fb22a906058cc083c3af2d561ce7f4332d Mon Sep 17 00:00:00 2001 From: "n8n-assistant[bot]" <100856346+n8n-assistant[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 09:03:28 +0200 Subject: [PATCH] chore: Bundle/1.x (#34679) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Charlie Kolb Co-authored-by: n8n-assistant[bot] <100856346+n8n-assistant[bot]@users.noreply.github.com> Co-authored-by: yehorkardash Co-authored-by: Mike Repeć Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Bernhard Wittmann Co-authored-by: RomanDavydchuk Co-authored-by: Yuliia Pominchuk <31064937+yuliia-pominchuk@users.noreply.github.com> Co-authored-by: Yuliia Pominchuk Co-authored-by: Guillaume Jacquart --- package.json | 3 +- .../src/__tests__/integration.test.ts | 66 +++ .../src/bridge/isolated-vm-bridge.ts | 13 + .../__tests__/require-resolver.test.ts | 196 ++++++- .../src/js-task-runner/js-task-runner.ts | 1 + .../src/js-task-runner/require-resolver.ts | 97 +++- .../workflow-execute-additional-data.test.ts | 98 +++- .../credentials-permission-checker.test.ts | 141 ++++- .../credentials-permission-checker.ts | 109 +++- .../src/workflow-execute-additional-data.ts | 24 +- .../cli/src/workflows/workflow.service.ee.ts | 72 ++- .../cli/test/integration/shared/workflow.ts | 36 +- .../workflows/workflow.service.ee.test.ts | 22 + .../__tests__/routing-node.test.ts | 72 +++ .../src/app/utils/parameterUtils.test.ts | 37 +- .../src/features/ndv/shared/ndv.utils.test.ts | 38 +- .../src/features/ndv/shared/ndv.utils.ts | 21 +- .../nodes/EditImage/EditImage.node.ts | 17 +- .../EditImage/test/EditImage.node.test.ts | 161 ++++++ .../test/v1/EmailSendV1.node.test.ts | 73 +++ .../EmailSend/test/v2/send.operation.test.ts | 65 +++ .../test/v2/sendAndWait.operation.test.ts | 4 +- .../nodes/EmailSend/test/v2/utils.test.ts | 32 ++ packages/nodes-base/nodes/EmailSend/utils.ts | 9 + .../nodes/EmailSend/v1/EmailSendV1.node.ts | 20 +- .../nodes/EmailSend/v2/send.operation.ts | 23 +- .../EmailSend/v2/sendAndWait.operation.ts | 5 +- .../nodes-base/nodes/EmailSend/v2/utils.ts | 5 +- .../nodes-base/nodes/Git/GenericFunctions.ts | 129 ++++- packages/nodes-base/nodes/Git/Git.node.ts | 97 ++-- .../Git/__test__/GenericFunctions.test.ts | 42 +- .../nodes/Git/__test__/Git.node.test.ts | 186 ------ .../nodes/Git/test/Git.node.test.ts | 544 +++++++++++++++++- .../Postgres/PostgresTrigger.functions.ts | 84 +-- .../nodes/Postgres/PostgresTrigger.node.ts | 54 +- .../Postgres/test/PostgresTrigger.test.ts | 276 +++++++++ .../Set/v2/helpers/__tests__/utils.test.ts | 23 + .../nodes/Snowflake/GenericFunctions.ts | 6 +- .../nodes/Snowflake/Snowflake.node.ts | 54 +- .../Snowflake/__tests__/executeQuery.test.ts | 156 +++++ .../__tests__/executeQuery.workflow.json | 64 +++ .../nodes-base/test/lodash-set.patch.test.ts | 46 ++ patches/lodash@4.18.0.patch | 26 + pnpm-lock.yaml | 159 ++--- 44 files changed, 2956 insertions(+), 450 deletions(-) create mode 100644 packages/nodes-base/nodes/EditImage/test/EditImage.node.test.ts create mode 100644 packages/nodes-base/nodes/EmailSend/test/v1/EmailSendV1.node.test.ts create mode 100644 packages/nodes-base/nodes/EmailSend/test/v2/send.operation.test.ts create mode 100644 packages/nodes-base/nodes/EmailSend/test/v2/utils.test.ts create mode 100644 packages/nodes-base/nodes/EmailSend/utils.ts delete mode 100644 packages/nodes-base/nodes/Git/__test__/Git.node.test.ts create mode 100644 packages/nodes-base/nodes/Postgres/test/PostgresTrigger.test.ts create mode 100644 packages/nodes-base/nodes/Snowflake/__tests__/executeQuery.test.ts create mode 100644 packages/nodes-base/nodes/Snowflake/__tests__/executeQuery.workflow.json create mode 100644 packages/nodes-base/test/lodash-set.patch.test.ts create mode 100644 patches/lodash@4.18.0.patch diff --git a/package.json b/package.json index b74d780869e..11c396d8bec 100644 --- a/package.json +++ b/package.json @@ -177,7 +177,8 @@ "minifaker": "patches/minifaker.patch", "z-vue-scan": "patches/z-vue-scan.patch", "@lezer/highlight": "patches/@lezer__highlight.patch", - "v-code-diff": "patches/v-code-diff.patch" + "v-code-diff": "patches/v-code-diff.patch", + "lodash@4.18.0": "patches/lodash@4.18.0.patch" } } } diff --git a/packages/@n8n/expression-runtime/src/__tests__/integration.test.ts b/packages/@n8n/expression-runtime/src/__tests__/integration.test.ts index 529e8d340b4..6e0b2512543 100644 --- a/packages/@n8n/expression-runtime/src/__tests__/integration.test.ts +++ b/packages/@n8n/expression-runtime/src/__tests__/integration.test.ts @@ -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).win).toBeUndefined(); + } finally { + delete (Object.prototype as Record).win; + } + }); + it('should preserve error name, message, and custom properties across isolate boundary', () => { const data = { $json: {} }; diff --git a/packages/@n8n/expression-runtime/src/bridge/isolated-vm-bridge.ts b/packages/@n8n/expression-runtime/src/bridge/isolated-vm-bridge.ts index 95e655651bb..2886952b22e 100644 --- a/packages/@n8n/expression-runtime/src/bridge/isolated-vm-bridge.ts +++ b/packages/@n8n/expression-runtime/src/bridge/isolated-vm-bridge.ts @@ -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) { diff --git a/packages/@n8n/task-runner/src/js-task-runner/__tests__/require-resolver.test.ts b/packages/@n8n/task-runner/src/js-task-runner/__tests__/require-resolver.test.ts index 36061b9cc77..4a1cb5f6b8d 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/__tests__/require-resolver.test.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/__tests__/require-resolver.test.ts @@ -1,7 +1,13 @@ +import type * as nodeCrypto from 'node:crypto'; + import { ExecutionError } from '@/js-task-runner/errors/execution-error'; import { DisallowedModuleError } from '../errors/disallowed-module.error'; -import { createRequireResolver, type RequireResolverOpts } from '../require-resolver'; +import { + createRequireResolver, + secureModuleExport, + type RequireResolverOpts, +} from '../require-resolver'; describe('require resolver', () => { let defaultOpts: RequireResolverOpts; @@ -61,6 +67,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; + + 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 = { existing: 1 }; + const secured = secureModuleExport(mod) as Record; + + 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).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); diff --git a/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts b/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts index 9ca330c7d1a..6778638719e 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts @@ -154,6 +154,7 @@ export class JsTaskRunner extends TaskRunner { this.requireResolver = createRequireResolver({ allowedBuiltInModules, allowedExternalModules, + secureModules: this.mode === 'secure', }); if (this.mode === 'secure') this.preventPrototypePollution(allowedExternalModules); diff --git a/packages/@n8n/task-runner/src/js-task-runner/require-resolver.ts b/packages/@n8n/task-runner/src/js-task-runner/require-resolver.ts index e1a3abab5c2..13a4ab98363 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/require-resolver.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/require-resolver.ts @@ -15,13 +15,105 @@ export type RequireResolverOpts = { * execution sandbox. `"*"` means all are allowed. */ allowedExternalModules: Set | '*'; + + /** + * 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(); +const targetByView = new WeakMap(); + +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 = { + 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 | '*', 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; }; } diff --git a/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts b/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts index e38953f404f..37b07c17fbb 100644 --- a/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts +++ b/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts @@ -4,7 +4,7 @@ import type { WorkflowEntity } from '@n8n/db'; import { ExecutionRepository, WorkflowRepository } from '@n8n/db'; import { Container } from '@n8n/di'; import { mock } from 'jest-mock-extended'; -import { ExternalSecretsProxy } from 'n8n-core'; +import { ExternalSecretsProxy, WorkflowExecute } from 'n8n-core'; import type { IWorkflowBase, IExecuteWorkflowInfo, @@ -103,7 +103,7 @@ describe('WorkflowExecuteAdditionalData', () => { mockInstance(Telemetry); const workflowRepository = mockInstance(WorkflowRepository); const activeExecutions = mockInstance(ActiveExecutions); - mockInstance(CredentialsPermissionChecker); + const credentialsPermissionChecker = mockInstance(CredentialsPermissionChecker); mockInstance(SubworkflowPolicyChecker); mockInstance(WorkflowStatisticsService); mockInstance(DataTableProxyService); @@ -228,6 +228,100 @@ describe('WorkflowExecuteAdditionalData', () => { expect(getVariablesSpy).toHaveBeenCalledWith(workflowId, undefined); }); + + describe('credential permission check routing', () => { + const subWorkflowData = () => + mock({ + id: 'sub-id', + name: 'Sub Workflow', + nodes: [], + connections: {}, + staticData: {}, + settings: {}, + }); + + beforeEach(() => { + jest.mocked(credentialsPermissionChecker.check).mockClear(); + jest.mocked(credentialsPermissionChecker.checkForUser).mockClear(); + jest.mocked(WorkflowExecute).mockClear(); + }); + + it('checks credentials against the triggering user for an inline sub-workflow', async () => { + await executeWorkflow( + mock({ id: undefined, code: subWorkflowData() }), + mock({ userId: 'user-1' }), + mock({ + loadedWorkflowData: subWorkflowData(), + doNotWaitToFinish: false, + parentWorkflowId: 'parent-1', + }), + ); + + expect(credentialsPermissionChecker.checkForUser).toHaveBeenCalledTimes(1); + expect(jest.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({ id: 'db-id', code: undefined }), + mock({ userId: 'user-1' }), + mock({ + 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({ id: undefined, code: subWorkflowData() }), + mock({ userId: undefined }), + mock({ + 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({ id: undefined, code: subWorkflowData() }), + mock({ userId: 'user-1' }), + mock({ + loadedWorkflowData: subWorkflowData(), + doNotWaitToFinish: false, + parentWorkflowId: 'parent-1', + }), + ); + + const integratedAdditionalData = jest.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({ id: 'db-id', code: undefined }), + mock({ userId: 'user-1' }), + mock({ + loadedWorkflowData: subWorkflowData(), + doNotWaitToFinish: false, + }), + ); + + const integratedAdditionalData = jest.mocked(WorkflowExecute).mock.calls[0][0]; + expect(integratedAdditionalData.userId).toBeUndefined(); + }); + }); }); describe('getRunData', () => { diff --git a/packages/cli/src/executions/pre-execution-checks/__tests__/credentials-permission-checker.test.ts b/packages/cli/src/executions/pre-execution-checks/__tests__/credentials-permission-checker.test.ts index 5681f7fb682..c5ad646cc2c 100644 --- a/packages/cli/src/executions/pre-execution-checks/__tests__/credentials-permission-checker.test.ts +++ b/packages/cli/src/executions/pre-execution-checks/__tests__/credentials-permission-checker.test.ts @@ -4,10 +4,13 @@ import { type SharedCredentialsRepository, type CredentialsRepository, type CredentialsEntity, + type UserRepository, GLOBAL_OWNER_ROLE, + GLOBAL_MEMBER_ROLE, } from '@n8n/db'; import { mock } from 'jest-mock-extended'; import type { INode } from 'n8n-workflow'; +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'; @@ -19,13 +22,18 @@ describe('CredentialsPermissionChecker', () => { const credentialsRepository = mock(); const ownershipService = mock(); const projectService = mock(); + const nodeTypes = mock(); + const userRepository = mock(); + const credentialsFinderService = mock(); const permissionChecker = new CredentialsPermissionChecker( sharedCredentialsRepository, credentialsRepository, ownershipService, projectService, + nodeTypes, + userRepository, + credentialsFinderService, ); - const nodeTypes = mock(); const workflowId = 'workflow123'; const credentialId = 'cred123'; @@ -256,6 +264,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'); @@ -276,4 +360,59 @@ 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 () => { + // A plain object (rather than `mock()`) is used here so the assertion below + // can rely on `expect.objectContaining`: a jest-mock-extended deep mock's Proxy + // auto-mocks any accessed property (including `asymmetricMatch`), which fools Jest's + // asymmetric-matcher detection and produces a false negative for the received value. + const memberUser = { id: userId, role: GLOBAL_MEMBER_ROLE } as User; + userRepository.findOne.mockResolvedValueOnce(memberUser); + 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({ id: userId, role: GLOBAL_MEMBER_ROLE }), + ); + credentialsFinderService.findCredentialsForUser.mockResolvedValueOnce([ + mock({ 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({ role: GLOBAL_OWNER_ROLE })); + + await expect(permissionChecker.checkForUser(userId, [node])).resolves.not.toThrow(); + expect(credentialsFinderService.findCredentialsForUser).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/cli/src/executions/pre-execution-checks/credentials-permission-checker.ts b/packages/cli/src/executions/pre-execution-checks/credentials-permission-checker.ts index a6ebf583eb5..ff3b4f5eec4 100644 --- a/packages/cli/src/executions/pre-execution-checks/credentials-permission-checker.ts +++ b/packages/cli/src/executions/pre-execution-checks/credentials-permission-checker.ts @@ -1,10 +1,12 @@ 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 { UserError } from 'n8n-workflow'; +import { displayParameter, isExpression, UserError } from 'n8n-workflow'; +import { NodeTypes } from '@/node-types'; +import { CredentialsFinderService } from '@/credentials/credentials-finder.service'; import { OwnershipService } from '@/services/ownership.service'; import { ProjectService } from '@/services/project.service.ee'; @@ -16,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' @@ -37,8 +48,49 @@ export class CredentialsPermissionChecker { private readonly credentialsRepository: CredentialsRepository, 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. */ @@ -99,13 +151,62 @@ export class CredentialsPermissionChecker { return nodes.reduce<{ [credentialId: string]: INode[] }>((map, node) => { if (node.disabled || !node.credentials) return map; - Object.values(node.credentials).forEach((cred) => { + const activeCredTypes = this.getActiveCredentialTypes(node); + + for (const [credType, cred] of Object.entries(node.credentials)) { if (!cred.id) throw new InvalidCredentialError(node); + // Skip credentials that are not actively used by the node's current configuration + if (activeCredTypes !== null && !activeCredTypes.has(credType)) continue; + map[cred.id] = map[cred.id] ? [...map[cred.id], node] : [node]; - }); + } return map; }, {}); } + + /** + * Determines which credential types are actively used by a node based on its + * current configuration. Returns null if the node type cannot be resolved, + * in which case all credentials should be checked as a safe fallback. + */ + private getActiveCredentialTypes(node: INode): Set | null { + try { + const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion); + const activeTypes = new Set(); + + // Check credentials defined in the node type description with display options + for (const credDef of nodeType.description.credentials ?? []) { + if (displayParameter(node.parameters, credDef, node, nodeType.description)) { + activeTypes.add(credDef.name); + } + } + + // For nodes using predefined credential type (e.g., HTTP Request node), + // 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); + } + + // For nodes using generic credential types (e.g., HTTP Request with + // authentication=genericCredentialType), the active credential type is + // specified by the genericAuthType parameter + const { genericAuthType } = node.parameters; + if (typeof genericAuthType === 'string' && genericAuthType) { + if (isExpression(genericAuthType)) return null; + activeTypes.add(genericAuthType); + } + + return activeTypes; + } catch { + // If we can't resolve the node type, fall back to checking all credentials + return null; + } + } } diff --git a/packages/cli/src/workflow-execute-additional-data.ts b/packages/cli/src/workflow-execute-additional-data.ts index 7ed962e80fb..d7bdd594a23 100644 --- a/packages/cli/src/workflow-execute-additional-data.ts +++ b/packages/cli/src/workflow-execute-additional-data.ts @@ -163,12 +163,18 @@ export async function executeWorkflow( const executionId = await activeExecutions.add(runData); + // 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) { @@ -184,6 +190,7 @@ async function startExecution( executionId: string, runData: IWorkflowExecutionDataProcess, workflowData: IWorkflowBase, + isInlineSubworkflow = false, ): Promise { const nodeTypes = Container.get(NodeTypes); const activeExecutions = Container.get(ActiveExecutions); @@ -212,7 +219,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, @@ -223,6 +240,11 @@ async function startExecution( // Create new additionalData to have different workflow loaded and to call // different webhooks 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, }); additionalDataIntegrated.hooks = getLifecycleHooksForSubExecutions( diff --git a/packages/cli/src/workflows/workflow.service.ee.ts b/packages/cli/src/workflows/workflow.service.ee.ts index a4eacfa8c5d..8ac98fd3cf6 100644 --- a/packages/cli/src/workflows/workflow.service.ee.ts +++ b/packages/cli/src/workflows/workflow.service.ee.ts @@ -21,8 +21,15 @@ import { Service } from '@n8n/di'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In, type EntityManager } from '@n8n/typeorm'; import omit from 'lodash/omit'; -import type { 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 { WorkflowFinderService } from './workflow-finder.service'; @@ -260,17 +267,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; + try { + parsed = jsonParse>(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; + } + async transferWorkflow( user: User, workflowId: string, diff --git a/packages/cli/test/integration/shared/workflow.ts b/packages/cli/test/integration/shared/workflow.ts index 362118e433d..fb9a8173792 100644 --- a/packages/cli/test/integration/shared/workflow.ts +++ b/packages/cli/test/integration/shared/workflow.ts @@ -1,5 +1,5 @@ import { WorkflowEntity } 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,5 +104,9 @@ export function getWorkflow(options?: { workflow.nodes.push(nodeWithTwoCredentials); } + if (options?.addNodeWithInlineSubworkflowCred) { + workflow.nodes.push(nodeWithInlineSubworkflowCredential); + } + return workflow; } diff --git a/packages/cli/test/integration/workflows/workflow.service.ee.test.ts b/packages/cli/test/integration/workflows/workflow.service.ee.test.ts index 7a001d84625..849ceb4a682 100644 --- a/packages/cli/test/integration/workflows/workflow.service.ee.test.ts +++ b/packages/cli/test/integration/workflows/workflow.service.ee.test.ts @@ -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); + }); }); }); diff --git a/packages/core/src/execution-engine/__tests__/routing-node.test.ts b/packages/core/src/execution-engine/__tests__/routing-node.test.ts index 05eabb2033a..402d4bbdcc3 100644 --- a/packages/core/src/execution-engine/__tests__/routing-node.test.ts +++ b/packages/core/src/execution-engine/__tests__/routing-node.test.ts @@ -773,6 +773,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(); + 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', () => { diff --git a/packages/frontend/editor-ui/src/app/utils/parameterUtils.test.ts b/packages/frontend/editor-ui/src/app/utils/parameterUtils.test.ts index 6d708b19d17..5259121988e 100644 --- a/packages/frontend/editor-ui/src/app/utils/parameterUtils.test.ts +++ b/packages/frontend/editor-ui/src/app/utils/parameterUtils.test.ts @@ -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'); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/features/ndv/shared/ndv.utils.test.ts b/packages/frontend/editor-ui/src/features/ndv/shared/ndv.utils.test.ts index 9320e30b358..1b29325282d 100644 --- a/packages/frontend/editor-ui/src/features/ndv/shared/ndv.utils.test.ts +++ b/packages/frontend/editor-ui/src/features/ndv/shared/ndv.utils.test.ts @@ -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, @@ -600,3 +600,39 @@ describe('setValue', () => { expect(nodeValues.value.newProperty).toBe('newValue'); }); }); + +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 = 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 = 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'); + }); +}); diff --git a/packages/frontend/editor-ui/src/features/ndv/shared/ndv.utils.ts b/packages/frontend/editor-ui/src/features/ndv/shared/ndv.utils.ts index 4c5db67ab80..a29f0067557 100644 --- a/packages/frontend/editor-ui/src/features/ndv/shared/ndv.utils.ts +++ b/packages/frontend/editor-ui/src/features/ndv/shared/ndv.utils.ts @@ -104,20 +104,13 @@ export function setValue( } } } else { - // Value should be set - if (typeof value === 'object') { - set( - get(nodeValues.value, nameParts.join('.')) as Record, - lastNamePart as string, - deepCopy(value), - ); - } else { - set( - get(nodeValues.value, nameParts.join('.')) as Record, - 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); } } diff --git a/packages/nodes-base/nodes/EditImage/EditImage.node.ts b/packages/nodes-base/nodes/EditImage/EditImage.node.ts index 006a1978204..747fb55b4f5 100644 --- a/packages/nodes-base/nodes/EditImage/EditImage.node.ts +++ b/packages/nodes-base/nodes/EditImage/EditImage.node.ts @@ -5,6 +5,7 @@ import type { IDataObject, IExecuteFunctions, ILoadOptionsFunctions, + INode, INodeExecutionData, INodeProperties, INodePropertyOptions, @@ -15,6 +16,19 @@ import { NodeOperationError, NodeConnectionTypes, deepCopy } from 'n8n-workflow' import { parse as pathParse } from 'path'; import { file } from 'tmp-promise'; +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', @@ -1278,7 +1292,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![dataPropertyName].fileExtension = options.format as string; newItem.binary![dataPropertyName].mimeType = `image/${options.format}`; const fileName = newItem.binary![dataPropertyName].fileName; diff --git a/packages/nodes-base/nodes/EditImage/test/EditImage.node.test.ts b/packages/nodes-base/nodes/EditImage/test/EditImage.node.test.ts new file mode 100644 index 00000000000..fa33bd14018 --- /dev/null +++ b/packages/nodes-base/nodes/EditImage/test/EditImage.node.test.ts @@ -0,0 +1,161 @@ +import { mockDeep } from 'jest-mock-extended'; +import type { IExecuteFunctions, INode, INodeExecutionData } from 'n8n-workflow'; + +import { EditImage } from '../EditImage.node'; + +const createTestBuffer = () => + Buffer.from([ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x06, 0x00, 0x00, 0x00, 0x1f, 0x15, 0xc4, + 0x89, 0x00, 0x00, 0x00, 0x0a, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x63, 0x00, 0x01, 0x00, 0x00, + 0x05, 0x00, 0x01, 0x0d, 0x0a, 0x2d, 0xb4, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, + 0x42, 0x60, 0x82, + ]); + +const mockGmInstance: any = { + background: jest.fn(function (this: any) { + return this; + }), + blur: jest.fn(function (this: any) { + return this; + }), + borderColor: jest.fn(function (this: any) { + return this; + }), + border: jest.fn(function (this: any) { + return this; + }), + compose: jest.fn(function (this: any) { + return this; + }), + geometry: jest.fn(function (this: any) { + return this; + }), + composite: jest.fn(function (this: any) { + return this; + }), + crop: jest.fn(function (this: any) { + return this; + }), + drawCircle: jest.fn(function (this: any) { + return this; + }), + drawLine: jest.fn(function (this: any) { + return this; + }), + drawRectangle: jest.fn(function (this: any) { + return this; + }), + fill: jest.fn(function (this: any) { + return this; + }), + font: jest.fn(function (this: any) { + return this; + }), + fontSize: jest.fn(function (this: any) { + return this; + }), + drawText: jest.fn(function (this: any) { + return this; + }), + identify: jest.fn(function (this: any, callback: any) { + callback(null, { width: 100, height: 100, format: 'PNG' }); + return this; + }), + quality: jest.fn(function (this: any) { + return this; + }), + resize: jest.fn(function (this: any) { + return this; + }), + rotate: jest.fn(function (this: any) { + return this; + }), + setFormat: jest.fn(function (this: any) { + return this; + }), + shear: jest.fn(function (this: any) { + return this; + }), + stream: jest.fn(function (this: any) { + return this; + }), + transparent: jest.fn(function (this: any) { + return this; + }), + toBuffer: jest.fn(function (this: any, callback: any) { + callback(null, createTestBuffer()); + return this; + }), + autoOrient: jest.fn(function (this: any) { + return this; + }), + out: jest.fn(function (this: any) { + return this; + }), +}; + +jest.mock('gm', () => ({ + default: jest.fn(() => mockGmInstance), + __esModule: true, +})); + +describe('EditImage Node', () => { + let editImageNode: EditImage; + const mockExecuteFunctions = mockDeep(); + let mockNode: INode; + + beforeEach(() => { + jest.clearAllMocks(); + editImageNode = new EditImage(); + mockNode = { + id: 'test-node-id', + name: 'EditImage', + type: 'n8n-nodes-base.editImage', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }; + + mockExecuteFunctions.getNode.mockReturnValue(mockNode); + mockExecuteFunctions.continueOnFail.mockReturnValue(false); + mockExecuteFunctions.helpers.assertBinaryData.mockReturnValue(undefined as any); + }); + + describe('format and quality options', () => { + 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(); + }); + }); +}); diff --git a/packages/nodes-base/nodes/EmailSend/test/v1/EmailSendV1.node.test.ts b/packages/nodes-base/nodes/EmailSend/test/v1/EmailSendV1.node.test.ts new file mode 100644 index 00000000000..2c1c54d5381 --- /dev/null +++ b/packages/nodes-base/nodes/EmailSend/test/v1/EmailSendV1.node.test.ts @@ -0,0 +1,73 @@ +import type { IExecuteFunctions, INodeTypeBaseDescription } from 'n8n-workflow'; +import type { MockProxy } from 'jest-mock-extended'; +import { mock } from 'jest-mock-extended'; + +import { EmailSendV1 } from '../../v1/EmailSendV1.node'; + +const mockTransporter = { sendMail: jest.fn() }; + +jest.mock('nodemailer', () => ({ + createTransport: jest.fn(() => mockTransporter), +})); + +describe('Test EmailSendV1', () => { + let emailSendV1: EmailSendV1; + let mockExecuteFunctions: MockProxy; + + beforeEach(() => { + emailSendV1 = new EmailSendV1({ + description: { + displayName: 'Send Email', + name: 'emailSend', + icon: 'fa:envelope', + group: ['output'], + description: 'Sends an Email', + version: 1, + defaults: { + name: 'Send Email', + color: '#00bb88', + }, + }, + } as unknown as INodeTypeBaseDescription); + + mockExecuteFunctions = mock(); + jest.clearAllMocks(); + }); + + it('should serialize object message values before sending', async () => { + mockExecuteFunctions.getInputData.mockReturnValue([{ json: { data: 'test' } }]); + 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' }); + mockTransporter.sendMail.mockResolvedValue({ messageId: 'test-id' }); + + await emailSendV1.execute.call(mockExecuteFunctions); + + expect(mockTransporter.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, + }), + ); + }); +}); diff --git a/packages/nodes-base/nodes/EmailSend/test/v2/send.operation.test.ts b/packages/nodes-base/nodes/EmailSend/test/v2/send.operation.test.ts new file mode 100644 index 00000000000..ec0386dab34 --- /dev/null +++ b/packages/nodes-base/nodes/EmailSend/test/v2/send.operation.test.ts @@ -0,0 +1,65 @@ +import type { IExecuteFunctions } from 'n8n-workflow'; +import type { MockProxy } from 'jest-mock-extended'; +import { mock } from 'jest-mock-extended'; + +import * as sendOperation from '../../v2/send.operation'; + +const mockTransporter = { sendMail: jest.fn() }; + +jest.mock('../../v2/utils', () => { + const originalModule = jest.requireActual('../../v2/utils'); + return { + ...originalModule, + configureTransport: jest.fn(() => mockTransporter), + }; +}); + +describe('Test EmailSendV2, send operation', () => { + let mockExecuteFunctions: MockProxy; + + beforeEach(() => { + mockExecuteFunctions = mock(); + jest.clearAllMocks(); + }); + + it('should request normalized text and HTML values before sending', async () => { + mockExecuteFunctions.getInputData.mockReturnValue([{ json: { data: 'test' } }]); + mockExecuteFunctions.getNode.mockReturnValue({ + typeVersion: 2.1, + } as ReturnType); + 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']); + mockTransporter.sendMail.mockResolvedValue({ messageId: 'test-id' }); + + await sendOperation.execute.call(mockExecuteFunctions); + + expect(mockTransporter.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', + }), + ); + }); +}); diff --git a/packages/nodes-base/nodes/EmailSend/test/v2/sendAndWait.operation.test.ts b/packages/nodes-base/nodes/EmailSend/test/v2/sendAndWait.operation.test.ts index 8f30ff36016..fb04ff6b389 100644 --- a/packages/nodes-base/nodes/EmailSend/test/v2/sendAndWait.operation.test.ts +++ b/packages/nodes-base/nodes/EmailSend/test/v2/sendAndWait.operation.test.ts @@ -28,7 +28,7 @@ describe('Test EmailSendV2, email => sendAndWait', () => { jest.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); @@ -57,7 +57,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({ diff --git a/packages/nodes-base/nodes/EmailSend/test/v2/utils.test.ts b/packages/nodes-base/nodes/EmailSend/test/v2/utils.test.ts new file mode 100644 index 00000000000..1657f208fb6 --- /dev/null +++ b/packages/nodes-base/nodes/EmailSend/test/v2/utils.test.ts @@ -0,0 +1,32 @@ +import { createTransport } from 'nodemailer'; + +import { configureTransport } from '../../v2/utils'; + +jest.mock('nodemailer', () => ({ + createTransport: jest.fn(), +})); + +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, + }, + ); + }); +}); diff --git a/packages/nodes-base/nodes/EmailSend/utils.ts b/packages/nodes-base/nodes/EmailSend/utils.ts new file mode 100644 index 00000000000..a1787879659 --- /dev/null +++ b/packages/nodes-base/nodes/EmailSend/utils.ts @@ -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); +} diff --git a/packages/nodes-base/nodes/EmailSend/v1/EmailSendV1.node.ts b/packages/nodes-base/nodes/EmailSend/v1/EmailSendV1.node.ts index 7f6e0adb47a..159ccb6c732 100644 --- a/packages/nodes-base/nodes/EmailSend/v1/EmailSendV1.node.ts +++ b/packages/nodes-base/nodes/EmailSend/v1/EmailSendV1.node.ts @@ -9,6 +9,7 @@ import type { import { NodeConnectionTypes } from 'n8n-workflow'; import { createTransport } from 'nodemailer'; import type SMTPTransport from 'nodemailer/lib/smtp-transport'; +import { toMailString } from '../utils'; const versionDescription: INodeTypeDescription = { displayName: 'Send Email', @@ -149,15 +150,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'); @@ -191,7 +193,9 @@ export class EmailSendV1 implements INodeType { subject, text, html, - replyTo: options.replyTo as string | undefined, + replyTo, + disableFileAccess: true, + disableUrlAccess: true, }; if (attachmentPropertyString && item.binary) { diff --git a/packages/nodes-base/nodes/EmailSend/v2/send.operation.ts b/packages/nodes-base/nodes/EmailSend/v2/send.operation.ts index ba94d270ef2..44dd99f64f2 100644 --- a/packages/nodes-base/nodes/EmailSend/v2/send.operation.ts +++ b/packages/nodes-base/nodes/EmailSend/v2/send.operation.ts @@ -12,6 +12,7 @@ import { createUtmCampaignLink, updateDisplayOptions } from '@utils/utilities'; import { fromEmailProperty, toEmailProperty } from './descriptions'; import { configureTransport, type EmailSendOptions } from './utils'; import { appendAttributionOption } from '../../../utils/descriptions'; +import { toMailString } from '../utils'; const properties: INodeProperties[] = [ // TODO: Add choice for text as text or html (maybe also from name) @@ -186,29 +187,35 @@ export async function execute(this: IExecuteFunctions): Promise { - 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[] = []; diff --git a/packages/nodes-base/nodes/EmailSend/v2/utils.ts b/packages/nodes-base/nodes/EmailSend/v2/utils.ts index ce680ab1abb..4646dda5031 100644 --- a/packages/nodes-base/nodes/EmailSend/v2/utils.ts +++ b/packages/nodes-base/nodes/EmailSend/v2/utils.ts @@ -44,7 +44,10 @@ export function configureTransport(credentials: IDataObject, options: EmailSendO }; } - return createTransport(connectionOptions); + return createTransport(connectionOptions, { + disableFileAccess: true, + disableUrlAccess: true, + }); } export async function smtpConnectionTest( diff --git a/packages/nodes-base/nodes/Git/GenericFunctions.ts b/packages/nodes-base/nodes/Git/GenericFunctions.ts index ef0e22c0b1a..21a6aef01a3 100644 --- a/packages/nodes-base/nodes/Git/GenericFunctions.ts +++ b/packages/nodes-base/nodes/Git/GenericFunctions.ts @@ -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, + remoteName: string, + value: string, +) { + const existingValues = remoteValuesByName.get(remoteName) ?? []; + existingValues.push(value); + remoteValuesByName.set(remoteName, existingValues); +} + +export function getConfiguredRemoteRepositories( + configValues: Record>, + node: INode, +): ConfiguredRemoteRepositories { + const sourceValidationTargets: string[] = []; + const targetValidationTargets: string[] = []; + const remoteOriginUrls: string[] = []; + const remoteOriginPushUrls: string[] = []; + const remoteUrlsByName = new Map(); + const remotePushUrlsByName = new Map(); + + 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 = ''; diff --git a/packages/nodes-base/nodes/Git/Git.node.ts b/packages/nodes-base/nodes/Git/Git.node.ts index b6a6c6a19e2..de397e7b526 100644 --- a/packages/nodes-base/nodes/Git/Git.node.ts +++ b/packages/nodes-base/nodes/Git/Git.node.ts @@ -31,7 +31,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+.-]*::/; @@ -295,8 +302,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, @@ -367,35 +386,23 @@ export class Git implements INodeType { } }; - const getRemoteOriginTargetRepositories = ( - configValues: Record>, - ) => { - const remoteOriginUrls: string[] = []; - const remoteOriginPushUrls: string[] = []; + const validateConfiguredRemoteRepositories = async ( + git: SimpleGit, + repositoryType: 'source' | 'target', + baseDir: string, + ): Promise => { + 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) => @@ -491,14 +498,20 @@ export class Git implements INodeType { const gitOptions: Partial = { baseDir: operation === 'clone' ? cloneStagingBase : resolvedRepositoryPath, config: gitConfig, + // simple-git blocks callers from setting `core.hooksPath` via `config` + // unless this flag is set. We set it deliberately as a mitigation, so + // opt in to keep that mitigation working. + ...(!enableHooks && { unsafe: { allowUnsafeHooksPath: true } }), }; const cleanEnv = Object.create(null) as Record; + 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') { @@ -540,6 +553,11 @@ export class Git implements INodeType { ); } + const repositoryType = getRepositoryTypeForRemoteConfigKey(key); + if (repositoryType) { + await assertRepositoryReferenceAllowed(value, repositoryType, resolvedRepositoryPath); + } + if (options.mode === 'append') { append = true; } @@ -641,6 +659,7 @@ export class Git implements INodeType { // fetch // ---------------------------------- + await validateConfiguredRemoteRepositories(git, 'source', resolvedRepositoryPath); await git.fetch(); returnItems.push({ json: { @@ -681,6 +700,7 @@ export class Git implements INodeType { // pull // ---------------------------------- + await validateConfiguredRemoteRepositories(git, 'source', resolvedRepositoryPath); await git.pull(); returnItems.push({ json: { @@ -713,19 +733,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'); @@ -751,6 +764,7 @@ export class Git implements INodeType { // pushTags // ---------------------------------- + await validateConfiguredRemoteRepositories(git, 'target', resolvedRepositoryPath); await git.pushTags(); returnItems.push({ json: { @@ -849,6 +863,7 @@ export class Git implements INodeType { // ---------------------------------- const name = this.getNodeParameter('name', itemIndex, '') as string; + validateGitTag(name, this.getNode()); await git.addTag(name); returnItems.push({ diff --git a/packages/nodes-base/nodes/Git/__test__/GenericFunctions.test.ts b/packages/nodes-base/nodes/Git/__test__/GenericFunctions.test.ts index c12a57d413c..4e9fba11421 100644 --- a/packages/nodes-base/nodes/Git/__test__/GenericFunctions.test.ts +++ b/packages/nodes-base/nodes/Git/__test__/GenericFunctions.test.ts @@ -1,7 +1,8 @@ import { mockDeep } from 'jest-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({ + 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()); + + 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({ + values: { + '.git/config': { + 'remote.upstream.url': '/outside/source-repo', + 'remote.origin.url': 'https://github.com/test/repo.git', + }, + }, + }); + + const result = getConfiguredRemoteRepositories(config.values, mockDeep()); + + expect(result.pushTarget).toBe('https://github.com/test/repo.git'); + }); + }); }); diff --git a/packages/nodes-base/nodes/Git/__test__/Git.node.test.ts b/packages/nodes-base/nodes/Git/__test__/Git.node.test.ts deleted file mode 100644 index 4b30fdb0922..00000000000 --- a/packages/nodes-base/nodes/Git/__test__/Git.node.test.ts +++ /dev/null @@ -1,186 +0,0 @@ -import { DeploymentConfig, SecurityConfig } from '@n8n/config'; -import { Container } from '@n8n/di'; -import { mock } from 'jest-mock-extended'; -import type { IExecuteFunctions } from 'n8n-workflow'; -import type { SimpleGit } from 'simple-git'; -import simpleGit from 'simple-git'; - -import { Git } from '../Git.node'; - -const mockGit = { - log: jest.fn(), - env: jest.fn().mockReturnThis(), -}; - -jest.mock('simple-git'); -const mockSimpleGit = simpleGit as jest.MockedFunction; -mockSimpleGit.mockReturnValue(mockGit as unknown as SimpleGit); - -describe('Git Node', () => { - let gitNode: Git; - let executeFunctions: jest.Mocked; - let deploymentConfig: jest.Mocked; - let securityConfig: jest.Mocked; - - beforeEach(() => { - jest.clearAllMocks(); - - deploymentConfig = mock({ - type: 'default', - }); - securityConfig = mock({ - disableBareRepos: false, - enableGitNodeHooks: true, - }); - Container.set(DeploymentConfig, deploymentConfig); - Container.set(SecurityConfig, securityConfig); - - executeFunctions = mock({ - getInputData: jest.fn().mockReturnValue([{ json: {} }]), - getNodeParameter: jest.fn(), - helpers: { - isFilePathBlocked: jest.fn(), - returnJsonArray: jest - .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 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: [], - }), - ); - }); - }); - - describe('Restricted file paths', () => { - it('should throw an error if the repository path is blocked', async () => { - (executeFunctions.helpers.isFilePathBlocked as jest.Mock).mockReturnValue(true); - (executeFunctions.helpers.resolvePath as jest.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 jest.Mock).mockResolvedValue(resolvedPath); - (executeFunctions.helpers.isFilePathBlocked as jest.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, - }), - ); - }); - }); -}); diff --git a/packages/nodes-base/nodes/Git/test/Git.node.test.ts b/packages/nodes-base/nodes/Git/test/Git.node.test.ts index d367ee7930d..9911aeb9f86 100644 --- a/packages/nodes-base/nodes/Git/test/Git.node.test.ts +++ b/packages/nodes-base/nodes/Git/test/Git.node.test.ts @@ -1,16 +1,19 @@ -import { SecurityConfig } from '@n8n/config'; +import { DeploymentConfig, SecurityConfig } from '@n8n/config'; import { Container } from '@n8n/di'; import { mock } from 'jest-mock-extended'; import type { IExecuteFunctions, ResolvedFilePath } from 'n8n-workflow'; import { mkdir, rename, rm } from 'node:fs/promises'; -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 { 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 = { @@ -54,9 +57,22 @@ const mockRm = jest.mocked(rm); describe('Git Node', () => { let gitNode: Git; let mockExecuteFunctions: jest.Mocked; + let deploymentConfig: jest.Mocked; + let securityConfig: jest.Mocked; beforeEach(() => { + jest.clearAllMocks(); gitNode = new Git(); + deploymentConfig = mock({ + type: 'default', + }); + securityConfig = mock({ + disableBareRepos: false, + enableGitNodeHooks: true, + enableGitNodeAllConfigKeys: false, + }); + Container.set(DeploymentConfig, deploymentConfig); + Container.set(SecurityConfig, securityConfig); mockExecuteFunctions = mock({ getInputData: jest.fn(() => [{ json: {} }]), getNodeParameter: jest.fn(), @@ -73,8 +89,22 @@ describe('Git Node', () => { pinDirectory: jest.fn(async () => null), }, }); - jest.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); @@ -106,6 +136,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 = jest.fn(() => true); + mockExecuteFunctions.helpers.resolvePath = jest.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 = jest.fn( + async () => resolvedPath as ResolvedFilePath, + ); + mockExecuteFunctions.helpers.isFilePathBlocked = jest.fn(() => false); + + await gitNode.execute.call(mockExecuteFunctions); + + expect(mockSimpleGit).toHaveBeenCalledWith( + expect.objectContaining({ + baseDir: resolvedPath, + }), + ); + }); + }); + describe('Branch switching', () => { const mockNodeParameters = (params: Record) => { mockExecuteFunctions.getNodeParameter.mockImplementation( @@ -764,6 +939,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 = jest.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({ + enableGitNodeAllConfigKeys: true, + }), + ); + mockExecuteFunctions.getNodeParameter + .mockReturnValueOnce('addConfig') + .mockReturnValueOnce('/repo') + .mockReturnValueOnce({}) + .mockReturnValueOnce('remote.origin.pushurl') + .mockReturnValueOnce('/outside/repo'); + mockExecuteFunctions.helpers.isFilePathBlocked = jest.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({ + enableGitNodeAllConfigKeys: true, + }), + ); + mockExecuteFunctions.getNodeParameter + .mockReturnValueOnce('addConfig') + .mockReturnValueOnce('/repo') + .mockReturnValueOnce({}) + .mockReturnValueOnce(key) + .mockReturnValueOnce('/outside/repo'); + mockExecuteFunctions.helpers.isFilePathBlocked = jest.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 = jest.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 = jest.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 = jest.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 = jest.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', @@ -783,7 +1178,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; @@ -796,11 +1193,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 } }]); }); @@ -834,7 +1231,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, [ @@ -914,19 +1313,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 = jest.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 = jest.fn( + async (path: string) => path as ResolvedFilePath, + ); + mockExecuteFunctions.helpers.isFilePathBlocked = jest.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'); }); @@ -948,6 +1389,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 = jest.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') @@ -1030,6 +1490,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; + 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') @@ -1188,6 +1664,48 @@ describe('Git Node', () => { }); }); + 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 diff --git a/packages/nodes-base/nodes/Postgres/PostgresTrigger.functions.ts b/packages/nodes-base/nodes/Postgres/PostgresTrigger.functions.ts index 0c1cfd9a781..57bdbf6d644 100644 --- a/packages/nodes-base/nodes/Postgres/PostgresTrigger.functions.ts +++ b/packages/nodes-base/nodes/Postgres/PostgresTrigger.functions.ts @@ -6,10 +6,36 @@ import type { INodeListSearchResult, INodeListSearchItems, } from 'n8n-workflow'; +import { 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 +43,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 ApplicationError('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 +72,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 ApplicationError('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 ApplicationError('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) { diff --git a/packages/nodes-base/nodes/Postgres/PostgresTrigger.node.ts b/packages/nodes-base/nodes/Postgres/PostgresTrigger.node.ts index 97f60386b55..533f3efbdb6 100644 --- a/packages/nodes-base/nodes/Postgres/PostgresTrigger.node.ts +++ b/packages/nodes-base/nodes/Postgres/PostgresTrigger.node.ts @@ -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, diff --git a/packages/nodes-base/nodes/Postgres/test/PostgresTrigger.test.ts b/packages/nodes-base/nodes/Postgres/test/PostgresTrigger.test.ts new file mode 100644 index 00000000000..23599b90cd6 --- /dev/null +++ b/packages/nodes-base/nodes/Postgres/test/PostgresTrigger.test.ts @@ -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'; + +jest.mock('../transport', () => ({ configurePostgres: jest.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 = jest.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: jest.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: jest.fn().mockResolvedValue(undefined), + any: jest.fn().mockResolvedValue([]), + query: jest.fn().mockResolvedValue([]), + done: jest.fn().mockResolvedValue(undefined), + client: { on: jest.fn(), removeListener: jest.fn() }, + }; + const db = { + connect: jest.fn().mockResolvedValue(connection), + any: jest.fn().mockResolvedValue([]), + }; + jest.mocked(configurePostgres).mockResolvedValue({ db, pgp: {} } as never); + + const params: IDataObject = { + triggerMode: 'createTrigger', + additionalFields, + options: {}, + schema: 'public', + tableName: 'users', + firesOn, + }; + const fns = { + getNodeParameter: jest.fn((name: string, _i?: unknown, fallback?: unknown) => + name in params ? params[name] : fallback, + ), + getCredentials: jest.fn().mockResolvedValue({}), + getNode: jest.fn().mockReturnValue({ id: 'node-1' }), + getMode: jest.fn().mockReturnValue('trigger'), + emit: jest.fn(), + helpers: { returnJsonArray: jest.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: jest.fn().mockResolvedValue(undefined), + client: { on: jest.fn(), removeListener: jest.fn() }, + }; + const db = { connect: jest.fn().mockResolvedValue(connection) }; + jest.mocked(configurePostgres).mockResolvedValue({ db, pgp: {} } as never); + + const params: IDataObject = { + triggerMode: 'listenTrigger', + additionalFields: {}, + options: {}, + channelName, + }; + const fns = { + getNodeParameter: jest.fn((name: string, _i?: unknown, fallback?: unknown) => + name in params ? params[name] : fallback, + ), + getCredentials: jest.fn().mockResolvedValue({}), + getNode: jest.fn().mockReturnValue({ id: 'node-1' }), + getMode: jest.fn().mockReturnValue('trigger'), + emit: jest.fn(), + helpers: { returnJsonArray: jest.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$'); + }); +}); diff --git a/packages/nodes-base/nodes/Set/v2/helpers/__tests__/utils.test.ts b/packages/nodes-base/nodes/Set/v2/helpers/__tests__/utils.test.ts index 8be358ec65c..3b280a1badf 100644 --- a/packages/nodes-base/nodes/Set/v2/helpers/__tests__/utils.test.ts +++ b/packages/nodes-base/nodes/Set/v2/helpers/__tests__/utils.test.ts @@ -49,6 +49,29 @@ describe('prepareReturnItem', () => { (mockContext.getNodeParameter as jest.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 = { diff --git a/packages/nodes-base/nodes/Snowflake/GenericFunctions.ts b/packages/nodes-base/nodes/Snowflake/GenericFunctions.ts index 2a09c0fb6d8..598b386eca5 100644 --- a/packages/nodes-base/nodes/Snowflake/GenericFunctions.ts +++ b/packages/nodes-base/nodes/Snowflake/GenericFunctions.ts @@ -111,11 +111,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((resolve, reject) => { conn.execute({ sqlText, diff --git a/packages/nodes-base/nodes/Snowflake/Snowflake.node.ts b/packages/nodes-base/nodes/Snowflake/Snowflake.node.ts index 45f8fddb9b3..3d0493abc98 100644 --- a/packages/nodes-base/nodes/Snowflake/Snowflake.node.ts +++ b/packages/nodes-base/nodes/Snowflake/Snowflake.node.ts @@ -5,7 +5,7 @@ import type { INodeType, INodeTypeDescription, } from 'n8n-workflow'; -import { NodeConnectionTypes } from 'n8n-workflow'; +import { NodeConnectionTypes, NodeOperationError } from 'n8n-workflow'; import snowflake from 'snowflake-sdk'; import { getResolvables } from '@utils/utilities'; @@ -89,7 +89,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…', + }, + ], }, // ---------------------------------- @@ -195,7 +221,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 } }, @@ -217,7 +261,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), @@ -256,7 +300,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( diff --git a/packages/nodes-base/nodes/Snowflake/__tests__/executeQuery.test.ts b/packages/nodes-base/nodes/Snowflake/__tests__/executeQuery.test.ts new file mode 100644 index 00000000000..1077313d4ca --- /dev/null +++ b/packages/nodes-base/nodes/Snowflake/__tests__/executeQuery.test.ts @@ -0,0 +1,156 @@ +import { NodeTestHarness } from '@nodes-testing/node-test-harness'; +import { NodeConnectionTypes, type WorkflowTestData } from 'n8n-workflow'; + +const mockExecute = jest.fn(); +const mockConnect = jest.fn(); +const mockDestroy = jest.fn(); +const mockConnection = { connect: mockConnect, execute: mockExecute, destroy: mockDestroy }; + +jest.mock('snowflake-sdk', () => ({ + configure: jest.fn(), + createConnection: jest.fn().mockReturnValue(mockConnection), +})); + +const snowflakeCredentials = { + authentication: 'password', + account: 'test-account', + database: 'TEST_DB', + schema: 'PUBLIC', + warehouse: 'WH', + role: 'SYSADMIN', + clientSessionKeepAlive: false, + username: 'user', + password: 'pass', +}; + +beforeEach(() => { + 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(() => jest.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 } }, + ); +}); diff --git a/packages/nodes-base/nodes/Snowflake/__tests__/executeQuery.workflow.json b/packages/nodes-base/nodes/Snowflake/__tests__/executeQuery.workflow.json new file mode 100644 index 00000000000..e96f7561066 --- /dev/null +++ b/packages/nodes-base/nodes/Snowflake/__tests__/executeQuery.workflow.json @@ -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": [] +} diff --git a/packages/nodes-base/test/lodash-set.patch.test.ts b/packages/nodes-base/test/lodash-set.patch.test.ts new file mode 100644 index 00000000000..fc0945459a1 --- /dev/null +++ b/packages/nodes-base/test/lodash-set.patch.test.ts @@ -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 = {}; + 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 = {}; + 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 = {}; + set(obj, 'a.b.c', 1); + set(obj, 'list[0].name', 'n'); + + expect(obj).toEqual({ a: { b: { c: 1 } }, list: [{ name: 'n' }] }); + }); +}); diff --git a/patches/lodash@4.18.0.patch b/patches/lodash@4.18.0.patch new file mode 100644 index 00000000000..a0f166d0e12 --- /dev/null +++ b/patches/lodash@4.18.0.patch @@ -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 902bfd4ee8f96898526f4776b061ce1d1c189ecc..c45042ae7e99ea0b900eb665989bcb72bf880b0f 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]) ? [] : {}); + } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5e14750211f..6ccae6c4cac 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -267,8 +267,8 @@ overrides: tmp: 0.2.7 ws: 8.21.0 axios: 1.18.0 - vue-i18n@<=11.4.4: 11.4.4 hono@<=4.12.27: 4.12.27 + vue-i18n@<=11.4.4: 11.4.4 linkify-it@<=5.0.1: 5.0.1 patchedDependencies: @@ -296,6 +296,9 @@ patchedDependencies: js-base64: hash: bb02fdf69495c7b0768791b60ab6e1a002053b8decd19a174f5755691e5c9500 path: patches/js-base64.patch + lodash@4.18.0: + hash: c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39 + path: patches/lodash@4.18.0.patch minifaker: hash: bc707e2c34a2464da2c9c93ead33e80fd9883a00434ef64907ddc45208a08b33 path: patches/minifaker.patch @@ -444,7 +447,7 @@ importers: version: 0.6.0(@opentelemetry/api@1.9.0)(@opentelemetry/sdk-trace-base@1.30.1(@opentelemetry/api@1.9.0))(openai@6.21.0(ws@8.21.0(bufferutil@4.0.9)(utf-8-validate@5.0.10))(zod@3.25.67))(ws@8.21.0(bufferutil@4.0.9)(utf-8-validate@5.0.10)) lodash: specifier: 4.18.0 - version: 4.18.0 + version: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) n8n-workflow: specifier: workspace:* version: link:../../workflow @@ -739,7 +742,7 @@ importers: version: 3.4.2 lodash: specifier: 4.18.0 - version: 4.18.0 + version: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) n8n-core: specifier: workspace:* version: link:../../core @@ -788,7 +791,7 @@ importers: version: link:../permissions lodash: specifier: 4.18.0 - version: 4.18.0 + version: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) n8n-workflow: specifier: workspace:* version: link:../../workflow @@ -952,7 +955,7 @@ importers: version: 3.3.1 lodash: specifier: 4.18.0 - version: 4.18.0 + version: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) luxon: specifier: 'catalog:' version: 3.4.4 @@ -1202,7 +1205,7 @@ importers: version: 1.0.1(@langchain/core@1.1.8(@opentelemetry/api@1.9.0)(@opentelemetry/sdk-trace-base@1.30.1(@opentelemetry/api@1.9.0))(openai@6.21.0(ws@8.21.0(bufferutil@4.0.9)(utf-8-validate@5.0.10))(zod@3.25.67))(ws@8.21.0(bufferutil@4.0.9)(utf-8-validate@5.0.10)))(encoding@0.1.13) '@langchain/community': specifier: 'catalog:' - version: 1.1.18(3cb93a0b6e411dc400a0c826af4a0e9e) + version: 1.1.18(2acfdd5f5cb130a81dbfd044a10b3ba6) '@langchain/core': specifier: 'catalog:' version: 1.1.8(@opentelemetry/api@1.9.0)(@opentelemetry/sdk-trace-base@1.30.1(@opentelemetry/api@1.9.0))(openai@6.21.0(ws@8.21.0(bufferutil@4.0.9)(utf-8-validate@5.0.10))(zod@3.25.67))(ws@8.21.0(bufferutil@4.0.9)(utf-8-validate@5.0.10)) @@ -1325,7 +1328,7 @@ importers: version: 1.2.3(@langchain/core@1.1.8(@opentelemetry/api@1.9.0)(@opentelemetry/sdk-trace-base@1.30.1(@opentelemetry/api@1.9.0))(openai@6.21.0(ws@8.21.0(bufferutil@4.0.9)(utf-8-validate@5.0.10))(zod@3.25.67))(ws@8.21.0(bufferutil@4.0.9)(utf-8-validate@5.0.10)))(@opentelemetry/api@1.9.0)(@opentelemetry/sdk-trace-base@1.30.1(@opentelemetry/api@1.9.0))(openai@6.21.0(ws@8.21.0(bufferutil@4.0.9)(utf-8-validate@5.0.10))(zod@3.25.67))(ws@8.21.0(bufferutil@4.0.9)(utf-8-validate@5.0.10))(zod-to-json-schema@3.23.3(zod@3.25.67)) lodash: specifier: 4.18.0 - version: 4.18.0 + version: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mammoth: specifier: 1.11.0 version: 1.11.0 @@ -1505,7 +1508,7 @@ importers: version: 8.3.4 lodash: specifier: 4.18.0 - version: 4.18.0 + version: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) luxon: specifier: 'catalog:' version: 3.4.4 @@ -1751,7 +1754,7 @@ importers: version: 4.2.6 lodash: specifier: 4.18.0 - version: 4.18.0 + version: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) luxon: specifier: 'catalog:' version: 3.4.4 @@ -2049,7 +2052,7 @@ importers: version: 9.0.2 lodash: specifier: 4.18.0 - version: 4.18.0 + version: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) luxon: specifier: 'catalog:' version: 3.4.4 @@ -2298,7 +2301,7 @@ importers: version: 0.0.5 lodash: specifier: 4.18.0 - version: 4.18.0 + version: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) markdown-it: specifier: ^13.0.2 version: 13.0.2 @@ -2791,7 +2794,7 @@ importers: version: 10.4.0 lodash: specifier: 4.18.0 - version: 4.18.0 + version: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) luxon: specifier: 'catalog:' version: 3.4.4 @@ -3111,7 +3114,7 @@ importers: version: 4.2.6 lodash: specifier: 4.18.0 - version: 4.18.0 + version: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) lossless-json: specifier: 1.0.5 version: 1.0.5 @@ -3213,7 +3216,7 @@ importers: version: 3.0.3 ts-ics: specifier: 1.2.2 - version: 1.2.2(date-fns@2.30.0)(lodash@4.18.0)(zod@4.4.0) + version: 1.2.2(date-fns@2.30.0)(lodash@4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39))(zod@4.4.0) uuid: specifier: 'catalog:' version: 11.1.1 @@ -3427,7 +3430,7 @@ importers: version: 3.3.1 lodash: specifier: 4.18.0 - version: 4.18.0 + version: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) luxon: specifier: 'catalog:' version: 3.4.4 @@ -17654,8 +17657,8 @@ packages: vue-component-type-helpers@2.2.12: resolution: {integrity: sha512-YbGqHZ5/eW4SnkPNR44mKVc6ZKQoRs/Rux1sxC6rdwXb4qpbOSYfDr9DsTHolOTGmIKgM9j141mZbBeg05R1pw==} - vue-component-type-helpers@3.3.5: - resolution: {integrity: sha512-Fe1jyPJoUGpJOYKOri44jduR7My4yYINOMJISuMAbmrs+L5LbIDUc8NTWZYY3EJLK0yPLuCmcd5zoCsE4k2/KA==} + vue-component-type-helpers@3.3.7: + resolution: {integrity: sha512-Skkhw9agYSgsWqv7bxSOGJZa9SaiJbZVGdXuFWnrzKaQYHnw9qbjD630rw6RyMqDbp54nfLCLw5SZA55if7JLg==} vue-demi@0.14.10: resolution: {integrity: sha512-nMZBOwuzabUO0nLgIcc6rycZEebF6eeUfaiQx9+WSk8e29IbLvPU9feI6tqW4kTo3hvoYAJkMh8n8D0fuISphg==} @@ -21010,7 +21013,7 @@ snapshots: istanbul-lib-coverage: 3.2.2 json-stringify-safe: 5.0.1 lil-http-terminator: 1.2.3 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) nanoid: 3.3.11 object-sizeof: 2.6.5 p-debounce: 2.1.0 @@ -21989,7 +21992,7 @@ snapshots: - aws-crt - encoding - '@langchain/community@1.1.18(3cb93a0b6e411dc400a0c826af4a0e9e)': + '@langchain/community@1.1.18(2acfdd5f5cb130a81dbfd044a10b3ba6)': dependencies: '@browserbasehq/stagehand': 1.9.0(@playwright/test@1.56.0)(bufferutil@4.0.9)(deepmerge@4.3.1)(dotenv@16.6.1)(encoding@0.1.13)(openai@6.21.0(ws@8.21.0(bufferutil@4.0.9)(utf-8-validate@5.0.10))(zod@3.25.67))(utf-8-validate@5.0.10)(zod@3.25.67) '@ibm-cloud/watsonx-ai': 1.1.2 @@ -22031,7 +22034,7 @@ snapshots: ignore: 5.2.4 ioredis: 5.3.2 jsdom: 23.0.1(bufferutil@4.0.9)(utf-8-validate@5.0.10) - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mammoth: 1.11.0 mongodb: 6.21.0(@aws-sdk/credential-providers@3.808.0)(gcp-metadata@5.3.0)(socks@2.8.3) pdf-parse: 1.1.1 @@ -22279,7 +22282,7 @@ snapshots: '@rushstack/terminal': 0.22.4(@types/node@20.19.21) '@rushstack/ts-command-line': 5.3.4(@types/node@20.19.21) diff: 8.0.2 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) minimatch: 10.2.3 resolve: 1.22.10 semver: 7.7.3 @@ -24482,7 +24485,7 @@ snapshots: storybook: 9.1.7(@testing-library/dom@10.4.0)(bufferutil@4.0.9)(prettier@3.6.2)(utf-8-validate@5.0.10)(vite@7.0.0(@types/node@20.19.21)(jiti@2.6.1)(lightningcss@1.30.2)(sass@1.89.2)(terser@5.16.1)(tsx@4.19.3)(yaml@2.8.3)) type-fest: 2.19.0 vue: 3.5.13(typescript@5.9.2) - vue-component-type-helpers: 3.3.5 + vue-component-type-helpers: 3.3.7 '@stylistic/eslint-plugin@5.0.0(eslint@9.29.0(jiti@2.6.1))': dependencies: @@ -24609,7 +24612,7 @@ snapshots: chalk: 3.0.0 css.escape: 1.5.1 dom-accessibility-api: 0.6.3 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) redent: 3.0.0 '@testing-library/user-event@14.6.1(@testing-library/dom@10.4.0)': @@ -25987,7 +25990,7 @@ snapshots: graceful-fs: 4.2.11 is-stream: 2.0.1 lazystream: 1.0.1 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) normalize-path: 3.0.0 readable-stream: 4.5.2 @@ -26047,7 +26050,7 @@ snapshots: array-hyper-unique@2.1.4: dependencies: deep-eql: 4.0.0 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) array-includes@3.1.9: dependencies: @@ -26241,6 +26244,16 @@ snapshots: - debug - supports-color + axios@1.18.0(debug@4.4.3): + dependencies: + follow-redirects: 1.16.0(debug@4.4.3) + form-data: 4.0.6 + https-proxy-agent: 5.0.1 + proxy-from-env: 2.1.0 + transitivePeerDependencies: + - debug + - supports-color + b4a@1.6.7: {} babel-jest@29.6.2(@babel/core@7.28.4): @@ -26600,7 +26613,7 @@ snapshots: cron-parser: 4.9.0 get-port: 5.1.1 ioredis: 5.3.2 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) msgpackr: 1.11.2 semver: 7.7.2 uuid: 11.1.1 @@ -26612,7 +26625,7 @@ snapshots: cron-parser: 4.9.0 get-port: 5.1.1 ioredis: 5.3.2 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) msgpackr: 1.11.2 semver: 7.7.3 uuid: 11.1.1 @@ -27119,7 +27132,7 @@ snapshots: dependencies: chalk: 4.1.2 date-fns: 2.30.0 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) rxjs: 7.8.1 shell-quote: 1.8.1 spawn-command: 0.0.2 @@ -27940,9 +27953,9 @@ snapshots: async-validator: 4.2.5 dayjs: 1.11.10 escape-html: 1.0.3 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) lodash-es: 4.18.1 - lodash-unified: 1.0.3(@types/lodash-es@4.17.12)(lodash-es@4.18.1)(lodash@4.18.0) + lodash-unified: 1.0.3(@types/lodash-es@4.17.12)(lodash-es@4.18.1)(lodash@4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39)) memoize-one: 6.0.0 normalize-wheel-es: 1.2.0 vue: 3.5.13(typescript@5.9.2) @@ -28399,7 +28412,7 @@ snapshots: eslint-plugin-lodash@8.0.0(eslint@9.29.0(jiti@2.6.1)): dependencies: eslint: 9.29.0(jiti@2.6.1) - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) eslint-plugin-n8n-nodes-base@1.16.3(eslint@9.29.0(jiti@2.6.1))(typescript@5.9.2): dependencies: @@ -29694,7 +29707,7 @@ snapshots: '@types/debug': 4.1.12 '@types/node': 20.19.21 '@types/tough-cookie': 4.0.5 - axios: 1.18.0(debug@4.4.1) + axios: 1.18.0(debug@4.4.3) camelcase: 6.3.0 debug: 4.4.3 dotenv: 16.6.1 @@ -29704,7 +29717,7 @@ snapshots: isstream: 0.1.2 jsonwebtoken: 9.0.2 mime-types: 2.1.35 - retry-axios: 2.6.0(axios@1.18.0(debug@4.4.3)) + retry-axios: 2.6.0(axios@1.18.0) tough-cookie: 4.1.4 transitivePeerDependencies: - supports-color @@ -29809,7 +29822,7 @@ snapshots: cli-width: 3.0.0 external-editor: 3.1.0 figures: 3.2.0 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mute-stream: 0.0.8 run-async: 2.4.1 rxjs: 6.6.7 @@ -31399,10 +31412,10 @@ snapshots: lodash-es@4.18.1: {} - lodash-unified@1.0.3(@types/lodash-es@4.17.12)(lodash-es@4.18.1)(lodash@4.18.0): + lodash-unified@1.0.3(@types/lodash-es@4.17.12)(lodash-es@4.18.1)(lodash@4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39)): dependencies: '@types/lodash-es': 4.17.12 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) lodash-es: 4.18.1 lodash.camelcase@4.3.0: {} @@ -31443,7 +31456,7 @@ snapshots: lodash.truncate@4.4.2: {} - lodash@4.18.0: {} + lodash@4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39): {} log-symbols@4.1.0: dependencies: @@ -31819,7 +31832,7 @@ snapshots: dependencies: '@miragejs/pretender-node-polyfill': 0.1.2 inflected: 2.1.0 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) pretender: 3.4.7 mitt@2.1.0: {} @@ -31827,7 +31840,7 @@ snapshots: mjml-accordion@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31835,7 +31848,7 @@ snapshots: mjml-body@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31843,7 +31856,7 @@ snapshots: mjml-button@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31851,7 +31864,7 @@ snapshots: mjml-carousel@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31863,7 +31876,7 @@ snapshots: glob: 10.5.0 html-minifier: 4.0.0 js-beautify: 1.14.9 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) minimatch: 9.0.9 mjml-core: 4.15.3(encoding@0.1.13) mjml-migrate: 4.15.3(encoding@0.1.13) @@ -31876,7 +31889,7 @@ snapshots: mjml-column@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31889,7 +31902,7 @@ snapshots: html-minifier: 4.0.0 js-beautify: 1.14.9 juice: 10.0.1(encoding@0.1.13) - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-migrate: 4.15.3(encoding@0.1.13) mjml-parser-xml: 4.15.3 mjml-validator: 4.15.3 @@ -31899,7 +31912,7 @@ snapshots: mjml-divider@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31907,7 +31920,7 @@ snapshots: mjml-group@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31915,7 +31928,7 @@ snapshots: mjml-head-attributes@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31923,7 +31936,7 @@ snapshots: mjml-head-breakpoint@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31931,7 +31944,7 @@ snapshots: mjml-head-font@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31939,7 +31952,7 @@ snapshots: mjml-head-html-attributes@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31947,7 +31960,7 @@ snapshots: mjml-head-preview@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31955,7 +31968,7 @@ snapshots: mjml-head-style@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31963,7 +31976,7 @@ snapshots: mjml-head-title@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31971,7 +31984,7 @@ snapshots: mjml-head@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31979,7 +31992,7 @@ snapshots: mjml-hero@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31987,7 +32000,7 @@ snapshots: mjml-image@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -31996,7 +32009,7 @@ snapshots: dependencies: '@babel/runtime': 7.26.10 js-beautify: 1.14.9 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) mjml-parser-xml: 4.15.3 yargs: 17.7.2 @@ -32006,7 +32019,7 @@ snapshots: mjml-navbar@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -32016,7 +32029,7 @@ snapshots: '@babel/runtime': 7.28.4 detect-node: 2.1.0 htmlparser2: 9.1.0 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-preset-core@4.15.3(encoding@0.1.13): dependencies: @@ -32052,7 +32065,7 @@ snapshots: mjml-raw@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -32060,7 +32073,7 @@ snapshots: mjml-section@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -32068,7 +32081,7 @@ snapshots: mjml-social@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -32076,7 +32089,7 @@ snapshots: mjml-spacer@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -32084,7 +32097,7 @@ snapshots: mjml-table@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -32092,7 +32105,7 @@ snapshots: mjml-text@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) transitivePeerDependencies: - encoding @@ -32104,7 +32117,7 @@ snapshots: mjml-wrapper@4.15.3(encoding@0.1.13): dependencies: '@babel/runtime': 7.28.4 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) mjml-core: 4.15.3(encoding@0.1.13) mjml-section: 4.15.3(encoding@0.1.13) transitivePeerDependencies: @@ -34061,7 +34074,7 @@ snapshots: onetime: 5.1.2 signal-exit: 3.0.7 - retry-axios@2.6.0(axios@1.18.0(debug@4.4.3)): + retry-axios@2.6.0(axios@1.18.0): dependencies: axios: 1.18.0(debug@4.4.1) @@ -35565,11 +35578,11 @@ snapshots: '@ts-graphviz/common': 2.1.5 '@ts-graphviz/core': 2.0.7 - ts-ics@1.2.2(date-fns@2.30.0)(lodash@4.18.0)(zod@4.4.0): + ts-ics@1.2.2(date-fns@2.30.0)(lodash@4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39))(zod@4.4.0): dependencies: date-fns: 2.30.0 date-fns-tz: 2.0.0(date-fns@2.30.0) - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) zod: 4.4.0 ts-interface-checker@0.1.13: {} @@ -36342,7 +36355,7 @@ snapshots: vue-component-type-helpers@2.2.12: {} - vue-component-type-helpers@3.3.5: {} + vue-component-type-helpers@3.3.7: {} vue-demi@0.14.10(vue@3.5.13(typescript@5.9.2)): dependencies: @@ -36372,7 +36385,7 @@ snapshots: eslint-visitor-keys: 4.2.1 espree: 10.4.0 esquery: 1.6.0 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) semver: 7.7.3 transitivePeerDependencies: - supports-color @@ -36880,7 +36893,7 @@ snapshots: dependencies: '@babel/runtime': 7.28.4 '@types/lodash': 4.17.17 - lodash: 4.18.0 + lodash: 4.18.0(patch_hash=c9c1aefdc3c11c84b56f0e446f4100ee59911491de392196faae26946b910b39) lodash-es: 4.18.1 nanoclone: 0.2.1 property-expr: 2.0.5