mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
parent
955c99dc26
commit
eeaf84f756
|
|
@ -0,0 +1,29 @@
|
|||
import { CommunityPackageRequestDto } from '../community-package-request.dto';
|
||||
|
||||
describe('CommunityPackageRequestDto', () => {
|
||||
it.each([
|
||||
{ name: 'n8n-nodes-test' },
|
||||
{ name: 'n8n-nodes-test', version: '1.0.0' },
|
||||
{ name: '@scope/n8n-nodes-test@1.0.0' },
|
||||
])('accepts %o', (request) => {
|
||||
expect(CommunityPackageRequestDto.safeParse(request).success).toBe(true);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ name: 'missing name', request: {} },
|
||||
{ name: 'empty name', request: { name: ' ' } },
|
||||
{ name: 'empty version', request: { name: 'n8n-nodes-test', version: '' } },
|
||||
])('rejects $name', ({ request }) => {
|
||||
expect(CommunityPackageRequestDto.safeParse(request).success).toBe(false);
|
||||
});
|
||||
|
||||
it('drops unknown fields', () => {
|
||||
const result = CommunityPackageRequestDto.safeParse({
|
||||
name: 'n8n-nodes-test',
|
||||
checksum: 'sha512-abc',
|
||||
verify: true,
|
||||
});
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.success && result.data).toEqual({ name: 'n8n-nodes-test' });
|
||||
});
|
||||
});
|
||||
|
|
@ -47,10 +47,8 @@ export class ListCommunityPackagesQueryDto extends Z.class({}, { strict: true })
|
|||
export class InstallCommunityPackagePublicDto extends Z.class({
|
||||
name: z.string().openapi(installCommunityPackageFieldDocs.name),
|
||||
version: z.string().optional().openapi(installCommunityPackageFieldDocs.version),
|
||||
verify: z.boolean().optional().openapi(installCommunityPackageFieldDocs.verify),
|
||||
}) {}
|
||||
|
||||
export class UpdateCommunityPackagePublicDto extends Z.class({
|
||||
version: z.string().optional().openapi(updateCommunityPackageFieldDocs.version),
|
||||
verify: z.boolean().optional().openapi(updateCommunityPackageFieldDocs.verify),
|
||||
}) {}
|
||||
|
|
|
|||
|
|
@ -6,20 +6,14 @@ export const installCommunityPackageFieldDocs = {
|
|||
'npm package name. Unscoped names start with n8n-nodes-. Scoped names use the @[author]/n8n-nodes- form.',
|
||||
},
|
||||
version: {
|
||||
description: 'Semver version or npm dist-tag, such as latest or beta',
|
||||
},
|
||||
verify: {
|
||||
description:
|
||||
'Whether to verify the package against the n8n-vetted package list. Required when the instance has N8N_UNVERIFIED_PACKAGES_ENABLED=false.',
|
||||
'Semver version or npm dist-tag, such as latest or beta. When unverified packages are disabled, only versions verified by n8n can be installed.',
|
||||
},
|
||||
} as const satisfies Record<string, ZodOpenAPIMetadata>;
|
||||
|
||||
export const updateCommunityPackageFieldDocs = {
|
||||
version: {
|
||||
description: 'Semver version or npm dist-tag to update to, such as latest or beta',
|
||||
},
|
||||
verify: {
|
||||
description:
|
||||
'Whether to verify the package against the n8n-vetted package list. Setting to false will allow installing or updating to an unverified version. Default is true.',
|
||||
'Semver version or npm dist-tag to update to, such as latest or beta. When unverified packages are disabled, only versions verified by n8n can be installed.',
|
||||
},
|
||||
} as const satisfies Record<string, ZodOpenAPIMetadata>;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,9 @@
|
|||
import { z } from 'zod';
|
||||
|
||||
import { Z } from '../../zod-class';
|
||||
|
||||
/** Body of `POST` and `PATCH /community-packages`. */
|
||||
export class CommunityPackageRequestDto extends Z.class({
|
||||
name: z.string().trim().min(1),
|
||||
version: z.string().trim().min(1).optional(),
|
||||
}) {}
|
||||
|
|
@ -465,6 +465,7 @@ export {
|
|||
IMPORT_PACKAGE_SELECTION_REQUEST_FORM_FIELDS,
|
||||
} from './packages/import-package-request.dto';
|
||||
export { ExportPackageRequestDto } from './packages/export-package-request.dto';
|
||||
export { CommunityPackageRequestDto } from './community-packages/community-package-request.dto';
|
||||
|
||||
export * from './evaluations';
|
||||
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import {
|
|||
createRefreshingAuthFetch,
|
||||
makeN8nLlmFailedAttemptHandler,
|
||||
getProxyAgent,
|
||||
aiClientFetch,
|
||||
} from '@n8n/ai-utilities';
|
||||
import { DATABRICKS_PARTNER_USER_AGENT } from 'n8n-nodes-base/dist/nodes/Databricks/constants';
|
||||
import { createMockExecuteFunction } from 'n8n-nodes-base/test/nodes/Helpers';
|
||||
|
|
@ -28,6 +29,7 @@ const mockedMakeN8nLlmFailedAttemptHandler = vi.mocked(makeN8nLlmFailedAttemptHa
|
|||
const mockedGetProxyAgent = vi.mocked(getProxyAgent);
|
||||
const mockedGetDatabricksTokenProvider = vi.mocked(getDatabricksTokenProvider);
|
||||
const mockedCreateRefreshingAuthFetch = vi.mocked(createRefreshingAuthFetch);
|
||||
const mockedAiClientFetch = vi.mocked(aiClientFetch);
|
||||
|
||||
const mockTokenProvider = {
|
||||
getToken: vi.fn(async () => 'test-token'),
|
||||
|
|
@ -132,6 +134,16 @@ describe('LmChatDatabricks', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should build the refreshing fetch on top of the bounded aiClientFetch', async () => {
|
||||
const ctx = setupMockContext();
|
||||
|
||||
await node.supplyData.call(ctx, 0);
|
||||
|
||||
expect(mockedCreateRefreshingAuthFetch).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ baseFetch: mockedAiClientFetch }),
|
||||
);
|
||||
});
|
||||
|
||||
it('should wire the refreshing fetch into ChatOpenAI', async () => {
|
||||
const ctx = setupMockContext();
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { ChatOpenAI, type ClientOptions } from '@langchain/openai';
|
||||
import {
|
||||
getProxyAgent,
|
||||
aiClientFetch,
|
||||
makeN8nLlmFailedAttemptHandler,
|
||||
N8nLlmTracing,
|
||||
getConnectionHintNoticeField,
|
||||
|
|
@ -397,7 +398,7 @@ export class LmChatOpenRouter implements INodeType {
|
|||
const timeout = options.timeout;
|
||||
const configuration: ClientOptions = {
|
||||
baseURL: credentials.url,
|
||||
fetch: createOpenRouterFetch(globalThis.fetch),
|
||||
fetch: createOpenRouterFetch(aiClientFetch),
|
||||
fetchOptions: {
|
||||
dispatcher: getProxyAgent(
|
||||
credentials.url,
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@
|
|||
|
||||
/* eslint-disable @typescript-eslint/unbound-method */
|
||||
import { ChatOpenAI } from '@langchain/openai';
|
||||
import { makeN8nLlmFailedAttemptHandler, getProxyAgent } from '@n8n/ai-utilities';
|
||||
import { makeN8nLlmFailedAttemptHandler, getProxyAgent, aiClientFetch } from '@n8n/ai-utilities';
|
||||
import { createMockExecuteFunction } from 'n8n-nodes-base/test/nodes/Helpers';
|
||||
import type { INode, ISupplyDataFunctions } from 'n8n-workflow';
|
||||
import type { Mock, Mocked } from 'vitest';
|
||||
|
|
@ -17,6 +17,7 @@ vi.mock('@n8n/ai-utilities');
|
|||
const MockedChatOpenAI = vi.mocked(ChatOpenAI);
|
||||
const mockedMakeN8nLlmFailedAttemptHandler = vi.mocked(makeN8nLlmFailedAttemptHandler);
|
||||
const mockedGetProxyAgent = vi.mocked(getProxyAgent);
|
||||
const mockedAiClientFetch = vi.mocked(aiClientFetch);
|
||||
|
||||
describe('LmChatOpenRouter', () => {
|
||||
let node: LmChatOpenRouter;
|
||||
|
|
@ -50,6 +51,11 @@ describe('LmChatOpenRouter', () => {
|
|||
|
||||
mockedMakeN8nLlmFailedAttemptHandler.mockReturnValue(vi.fn());
|
||||
mockedGetProxyAgent.mockReturnValue({} as any);
|
||||
// aiClientFetch wraps global fetch in production; mirror that so the
|
||||
// OpenRouter adapter drives the test's globalThis.fetch stub.
|
||||
mockedAiClientFetch.mockImplementation(
|
||||
async (...args) => await (globalThis.fetch as typeof fetch)(...args),
|
||||
);
|
||||
return ctx;
|
||||
};
|
||||
|
||||
|
|
@ -303,6 +309,17 @@ describe('LmChatOpenRouter', () => {
|
|||
return MockedChatOpenAI.mock.calls[0][0]?.configuration?.fetch as typeof fetch;
|
||||
}
|
||||
|
||||
it('should delegate to the bounded aiClientFetch under the wrapper', async () => {
|
||||
const mockFetch = vi.fn().mockResolvedValue(jsonResponse({ ok: true }));
|
||||
|
||||
const wrappedFetch = await setupFetchWrapper(mockFetch);
|
||||
await wrappedFetch('https://openrouter.ai/api/v1/models');
|
||||
|
||||
// The wrapper must route through aiClientFetch, not call fetch directly,
|
||||
// so the outbound response stays bounded.
|
||||
expect(mockedAiClientFetch).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each<{ input: unknown; expected: string; label: string }>([
|
||||
{ input: '', expected: '{}', label: 'empty string' },
|
||||
{ input: ' ', expected: '{}', label: 'whitespace-only string' },
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import type { ClientOptions } from '@langchain/openai';
|
||||
import { getProxyAgent } from '@n8n/ai-utilities';
|
||||
import { aiClientFetch, getProxyAgent } from '@n8n/ai-utilities';
|
||||
import type { ISupplyDataFunctions } from 'n8n-workflow';
|
||||
|
||||
import type { RefreshingTokenSource } from '../oauth2-token-provider';
|
||||
|
|
@ -29,7 +29,7 @@ export function createDatabricksGatewayConfig(
|
|||
const { fetch: authFetch, tokenSource } = createDatabricksAuthFetch(ctx, credential, {
|
||||
endpointUrl: baseURL,
|
||||
egressFilter,
|
||||
baseFetch: fetch,
|
||||
baseFetch: aiClientFetch,
|
||||
});
|
||||
|
||||
const configuration: ClientOptions = {
|
||||
|
|
|
|||
|
|
@ -3002,7 +3002,7 @@ describe('CredentialsHelper', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('getDecrypted - credentialDecrypt policy enforcement', () => {
|
||||
describe('getDecrypted', () => {
|
||||
const nodeCredentials: INodeCredentialsDetails = {
|
||||
id: 'cred-policy',
|
||||
name: 'Policy Test Credential',
|
||||
|
|
@ -3017,6 +3017,19 @@ describe('CredentialsHelper', () => {
|
|||
usageScope: 'project',
|
||||
} as CredentialsEntity;
|
||||
|
||||
const executeDataFor = (nodeType: string): IExecuteData => ({
|
||||
node: {
|
||||
id: nodeType,
|
||||
name: nodeType,
|
||||
type: nodeType,
|
||||
typeVersion: 1,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
data: {},
|
||||
source: null,
|
||||
});
|
||||
|
||||
let helper: CredentialsHelper;
|
||||
|
||||
beforeEach(() => {
|
||||
|
|
@ -3036,161 +3049,222 @@ describe('CredentialsHelper', () => {
|
|||
);
|
||||
});
|
||||
|
||||
test('calls enforceCredentialDecrypt with the credential, consumer and project context', async () => {
|
||||
const executeData = {
|
||||
node: {
|
||||
name: 'Slack1',
|
||||
type: 'n8n-nodes-base.slack',
|
||||
typeVersion: 1,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
data: {},
|
||||
source: null,
|
||||
} as IExecuteData;
|
||||
describe('managed credential access', () => {
|
||||
const useManagedCredential = (type: string) => {
|
||||
credentialsRepository.findOneByOrFail.mockResolvedValue(
|
||||
mock<CredentialsEntity>({
|
||||
...credentialEntity,
|
||||
type,
|
||||
isManaged: true,
|
||||
}),
|
||||
);
|
||||
};
|
||||
|
||||
const additionalData = mock<IWorkflowExecuteAdditionalData>({
|
||||
projectId: 'proj-1',
|
||||
executionId: 'exec-1',
|
||||
userId: 'user-1',
|
||||
test('does not resolve managed OpenAI credentials for full-access nodes', async () => {
|
||||
useManagedCredential('openAiApi');
|
||||
|
||||
await expect(
|
||||
helper.getDecrypted(
|
||||
mock<IWorkflowExecuteAdditionalData>(),
|
||||
nodeCredentials,
|
||||
'openAiApi',
|
||||
'manual',
|
||||
executeDataFor('n8n-nodes-base.httpRequest'),
|
||||
true,
|
||||
),
|
||||
).rejects.toThrow('Managed credentials are not supported by this node');
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
await helper.getDecrypted(
|
||||
additionalData,
|
||||
nodeCredentials,
|
||||
'testApi',
|
||||
'manual',
|
||||
executeData,
|
||||
true,
|
||||
);
|
||||
test.each([
|
||||
['other managed credential types', 'slackApi', 'n8n-nodes-base.httpRequest'],
|
||||
['managed OpenAI credentials in regular nodes', 'openAiApi', 'n8n-nodes-base.openAi'],
|
||||
])('resolves %s', async (_scenario, credentialType, nodeType) => {
|
||||
useManagedCredential(credentialType);
|
||||
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledExactlyOnceWith(
|
||||
{
|
||||
credentialType: 'testApi',
|
||||
credentialId: 'cred-policy',
|
||||
consumer: { nodeType: 'n8n-nodes-base.slack' },
|
||||
const result = await helper.getDecrypted(
|
||||
mock<IWorkflowExecuteAdditionalData>({
|
||||
projectId: 'proj-1',
|
||||
executionId: undefined,
|
||||
userId: undefined,
|
||||
}),
|
||||
nodeCredentials,
|
||||
credentialType,
|
||||
'manual',
|
||||
executeDataFor(nodeType),
|
||||
true,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ apiKey: 'test' });
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledExactlyOnceWith(
|
||||
{
|
||||
credentialType,
|
||||
credentialId: 'cred-policy',
|
||||
consumer: { nodeType },
|
||||
projectId: 'proj-1',
|
||||
},
|
||||
{ kind: 'system', reason: 'execution' },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('credentialDecrypt policy enforcement', () => {
|
||||
test('calls enforceCredentialDecrypt with the credential, consumer and project context', async () => {
|
||||
const executeData = {
|
||||
node: {
|
||||
name: 'Slack1',
|
||||
type: 'n8n-nodes-base.slack',
|
||||
typeVersion: 1,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
data: {},
|
||||
source: null,
|
||||
} as IExecuteData;
|
||||
|
||||
const additionalData = mock<IWorkflowExecuteAdditionalData>({
|
||||
projectId: 'proj-1',
|
||||
},
|
||||
{ kind: 'system', reason: 'execution', executionId: 'exec-1' },
|
||||
);
|
||||
});
|
||||
executionId: 'exec-1',
|
||||
userId: 'user-1',
|
||||
});
|
||||
|
||||
test('names the user when the decrypt is outside a run, e.g. an OAuth flow', async () => {
|
||||
const additionalData = mock<IWorkflowExecuteAdditionalData>({
|
||||
projectId: undefined,
|
||||
executionId: undefined,
|
||||
userId: 'user-1',
|
||||
await helper.getDecrypted(
|
||||
additionalData,
|
||||
nodeCredentials,
|
||||
'testApi',
|
||||
'manual',
|
||||
executeData,
|
||||
true,
|
||||
);
|
||||
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledExactlyOnceWith(
|
||||
{
|
||||
credentialType: 'testApi',
|
||||
credentialId: 'cred-policy',
|
||||
consumer: { nodeType: 'n8n-nodes-base.slack' },
|
||||
projectId: 'proj-1',
|
||||
},
|
||||
{ kind: 'system', reason: 'execution', executionId: 'exec-1' },
|
||||
);
|
||||
});
|
||||
|
||||
await helper.getDecrypted(
|
||||
additionalData,
|
||||
nodeCredentials,
|
||||
'testApi',
|
||||
'internal',
|
||||
undefined,
|
||||
true,
|
||||
);
|
||||
test('names the user when the decrypt is outside a run, e.g. an OAuth flow', async () => {
|
||||
const additionalData = mock<IWorkflowExecuteAdditionalData>({
|
||||
projectId: undefined,
|
||||
executionId: undefined,
|
||||
userId: 'user-1',
|
||||
});
|
||||
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledExactlyOnceWith(
|
||||
expect.anything(),
|
||||
{ kind: 'user', user: { id: 'user-1' } },
|
||||
);
|
||||
});
|
||||
await helper.getDecrypted(
|
||||
additionalData,
|
||||
nodeCredentials,
|
||||
'testApi',
|
||||
'internal',
|
||||
undefined,
|
||||
true,
|
||||
);
|
||||
|
||||
test('uses the actor the caller names over the derived one', async () => {
|
||||
const additionalData = mock<IWorkflowExecuteAdditionalData>({
|
||||
projectId: undefined,
|
||||
executionId: undefined,
|
||||
userId: undefined,
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledExactlyOnceWith(
|
||||
expect.anything(),
|
||||
{ kind: 'user', user: { id: 'user-1' } },
|
||||
);
|
||||
});
|
||||
|
||||
await helper.getDecrypted(
|
||||
additionalData,
|
||||
nodeCredentials,
|
||||
'testApi',
|
||||
'internal',
|
||||
undefined,
|
||||
true,
|
||||
undefined,
|
||||
{ actor: { kind: 'system', reason: 'log-streaming' } },
|
||||
);
|
||||
test('uses the actor the caller names over the derived one', async () => {
|
||||
const additionalData = mock<IWorkflowExecuteAdditionalData>({
|
||||
projectId: undefined,
|
||||
executionId: undefined,
|
||||
userId: undefined,
|
||||
});
|
||||
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledExactlyOnceWith(
|
||||
expect.anything(),
|
||||
{ kind: 'system', reason: 'log-streaming' },
|
||||
);
|
||||
});
|
||||
await helper.getDecrypted(
|
||||
additionalData,
|
||||
nodeCredentials,
|
||||
'testApi',
|
||||
'internal',
|
||||
undefined,
|
||||
true,
|
||||
undefined,
|
||||
{ actor: { kind: 'system', reason: 'log-streaming' } },
|
||||
);
|
||||
|
||||
test('passes a null consumer when no node is asking, e.g. a credential test', async () => {
|
||||
const additionalData = mock<IWorkflowExecuteAdditionalData>({
|
||||
projectId: undefined,
|
||||
executionId: undefined,
|
||||
userId: undefined,
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledExactlyOnceWith(
|
||||
expect.anything(),
|
||||
{ kind: 'system', reason: 'log-streaming' },
|
||||
);
|
||||
});
|
||||
|
||||
await helper.getDecrypted(
|
||||
additionalData,
|
||||
nodeCredentials,
|
||||
'testApi',
|
||||
'manual',
|
||||
undefined,
|
||||
true,
|
||||
);
|
||||
test('passes a null consumer when no node is asking, e.g. a credential test', async () => {
|
||||
const additionalData = mock<IWorkflowExecuteAdditionalData>({
|
||||
projectId: undefined,
|
||||
executionId: undefined,
|
||||
userId: undefined,
|
||||
});
|
||||
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledExactlyOnceWith(
|
||||
expect.objectContaining({ consumer: null, projectId: null }),
|
||||
{ kind: 'system', reason: 'execution' },
|
||||
);
|
||||
});
|
||||
await helper.getDecrypted(
|
||||
additionalData,
|
||||
nodeCredentials,
|
||||
'testApi',
|
||||
'manual',
|
||||
undefined,
|
||||
true,
|
||||
);
|
||||
|
||||
test('resolves the credential before enforcing the policy check', async () => {
|
||||
const callOrder: string[] = [];
|
||||
credentialsRepository.findOneByOrFail.mockImplementation(async () => {
|
||||
callOrder.push('findOneByOrFail');
|
||||
return credentialEntity;
|
||||
});
|
||||
policyEnforcementService.enforceCredentialDecrypt.mockImplementation(async () => {
|
||||
callOrder.push('enforceCredentialDecrypt');
|
||||
return await mock();
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledExactlyOnceWith(
|
||||
expect.objectContaining({ consumer: null, projectId: null }),
|
||||
{ kind: 'system', reason: 'execution' },
|
||||
);
|
||||
});
|
||||
|
||||
await helper.getDecrypted(
|
||||
mock<IWorkflowExecuteAdditionalData>(),
|
||||
nodeCredentials,
|
||||
'testApi',
|
||||
'manual',
|
||||
undefined,
|
||||
true,
|
||||
);
|
||||
test('resolves the credential before enforcing the policy check', async () => {
|
||||
const callOrder: string[] = [];
|
||||
credentialsRepository.findOneByOrFail.mockImplementation(async () => {
|
||||
callOrder.push('findOneByOrFail');
|
||||
return credentialEntity;
|
||||
});
|
||||
policyEnforcementService.enforceCredentialDecrypt.mockImplementation(async () => {
|
||||
callOrder.push('enforceCredentialDecrypt');
|
||||
return await mock();
|
||||
});
|
||||
|
||||
expect(callOrder).toEqual(['findOneByOrFail', 'enforceCredentialDecrypt']);
|
||||
});
|
||||
|
||||
test('blocks decryption when the policy check throws', async () => {
|
||||
const violation = new Error('blocked by policy');
|
||||
policyEnforcementService.enforceCredentialDecrypt.mockRejectedValueOnce(violation);
|
||||
|
||||
await expect(
|
||||
helper.getDecrypted(
|
||||
await helper.getDecrypted(
|
||||
mock<IWorkflowExecuteAdditionalData>(),
|
||||
nodeCredentials,
|
||||
'testApi',
|
||||
'manual',
|
||||
),
|
||||
).rejects.toThrow(violation);
|
||||
});
|
||||
undefined,
|
||||
true,
|
||||
);
|
||||
|
||||
test('decryption behavior is unchanged when the policy check clears', async () => {
|
||||
const result = await helper.getDecrypted(
|
||||
mock<IWorkflowExecuteAdditionalData>(),
|
||||
nodeCredentials,
|
||||
'testApi',
|
||||
'manual',
|
||||
undefined,
|
||||
true,
|
||||
);
|
||||
expect(callOrder).toEqual(['findOneByOrFail', 'enforceCredentialDecrypt']);
|
||||
});
|
||||
|
||||
expect(result).toEqual({ apiKey: 'test' });
|
||||
test('blocks decryption when the policy check throws', async () => {
|
||||
const violation = new Error('blocked by policy');
|
||||
policyEnforcementService.enforceCredentialDecrypt.mockRejectedValueOnce(violation);
|
||||
|
||||
await expect(
|
||||
helper.getDecrypted(
|
||||
mock<IWorkflowExecuteAdditionalData>(),
|
||||
nodeCredentials,
|
||||
'testApi',
|
||||
'manual',
|
||||
),
|
||||
).rejects.toThrow(violation);
|
||||
});
|
||||
|
||||
test('decryption behavior is unchanged when the policy check clears', async () => {
|
||||
const result = await helper.getDecrypted(
|
||||
mock<IWorkflowExecuteAdditionalData>(),
|
||||
nodeCredentials,
|
||||
'testApi',
|
||||
'manual',
|
||||
undefined,
|
||||
true,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ apiKey: 'test' });
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ import type {
|
|||
IRun,
|
||||
INodeExecutionData,
|
||||
INode,
|
||||
INodeType,
|
||||
ITaskData,
|
||||
WorkflowExecuteMode,
|
||||
ExecuteAgentWorkflowContext,
|
||||
|
|
@ -42,6 +43,7 @@ import { ExternalHooks } from '@/external-hooks';
|
|||
import { hashAgentSandboxPrincipal } from '@/modules/agents/agent-sandbox-principal';
|
||||
import { AgentWorkflowExecutionService } from '@/modules/agents/agent-workflow-execution.service';
|
||||
import { DataTableProxyService } from '@/modules/data-table/data-table-proxy.service';
|
||||
import { NodeTypes } from '@/node-types';
|
||||
import { OwnershipService } from '@/services/ownership.service';
|
||||
import { WorkflowStatisticsService } from '@/services/workflow-statistics.service';
|
||||
import { Telemetry } from '@/telemetry';
|
||||
|
|
@ -58,6 +60,7 @@ import {
|
|||
import * as WorkflowHelpers from '@/workflow-helpers';
|
||||
import { WorkflowHookContextService } from '@/workflow-hook-context.service';
|
||||
import { WorkflowPublishedDataService } from '@/workflows/workflow-published-data.service';
|
||||
import { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service';
|
||||
|
||||
const EXECUTION_ID = '123';
|
||||
const LAST_NODE_EXECUTED = 'Last node executed';
|
||||
|
|
@ -344,17 +347,31 @@ describe('WorkflowExecuteAdditionalData', () => {
|
|||
});
|
||||
|
||||
describe('credential permission check routing', () => {
|
||||
const nodeTypes = mockInstance(NodeTypes);
|
||||
const workflowStaticDataService = mockInstance(WorkflowStaticDataService);
|
||||
const nodeType = mock<INodeType>({ description: { properties: [] } });
|
||||
const nodes: INode[] = [
|
||||
{
|
||||
id: 'trigger',
|
||||
name: 'Trigger',
|
||||
type: 'n8n-nodes-base.executeWorkflowTrigger',
|
||||
typeVersion: 1.1,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
];
|
||||
const subWorkflowData = () =>
|
||||
mock<IWorkflowBase>({
|
||||
id: 'sub-id',
|
||||
name: 'Sub Workflow',
|
||||
nodes: [],
|
||||
nodes,
|
||||
connections: {},
|
||||
staticData: {},
|
||||
settings: {},
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
nodeTypes.getByNameAndVersion.mockReturnValue(nodeType);
|
||||
vi.mocked(credentialsPermissionChecker.check).mockClear();
|
||||
vi.mocked(credentialsPermissionChecker.checkForUser).mockClear();
|
||||
vi.mocked(WorkflowExecute).mockClear();
|
||||
|
|
@ -369,12 +386,7 @@ describe('WorkflowExecuteAdditionalData', () => {
|
|||
userId: 'user-1',
|
||||
rootExecutionMode: undefined,
|
||||
}),
|
||||
mock<ExecuteWorkflowOptions>({
|
||||
loadedWorkflowData: subWorkflowData(),
|
||||
doNotWaitToFinish: false,
|
||||
parentWorkflowId: 'parent-1',
|
||||
executionMode: 'manual',
|
||||
}),
|
||||
{ parentWorkflowId: 'parent-1', executionMode: 'manual' },
|
||||
);
|
||||
|
||||
expect(credentialsPermissionChecker.checkForUser).toHaveBeenCalledTimes(1);
|
||||
|
|
@ -472,15 +484,61 @@ describe('WorkflowExecuteAdditionalData', () => {
|
|||
await executeWorkflow(
|
||||
mock<IExecuteWorkflowInfo>({ id: undefined, code: subWorkflowData() }),
|
||||
mock<IWorkflowExecuteAdditionalData>({ userId: undefined }),
|
||||
mock<ExecuteWorkflowOptions>({
|
||||
loadedWorkflowData: subWorkflowData(),
|
||||
doNotWaitToFinish: false,
|
||||
parentWorkflowId: 'parent-1',
|
||||
}),
|
||||
{ parentWorkflowId: 'parent-1', executionMode: 'webhook' },
|
||||
);
|
||||
|
||||
expect(credentialsPermissionChecker.check).toHaveBeenCalled();
|
||||
expect(credentialsPermissionChecker.check).toHaveBeenCalledWith('parent-1', nodes);
|
||||
expect(credentialsPermissionChecker.checkForUser).not.toHaveBeenCalled();
|
||||
expect(activeExecutions.add).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
workflowData: expect.objectContaining({ id: 'parent-1' }),
|
||||
}),
|
||||
);
|
||||
const [integratedAdditionalData] = vi.mocked(WorkflowExecute).mock.calls[0];
|
||||
expect(integratedAdditionalData.workflowId).toBe('parent-1');
|
||||
});
|
||||
|
||||
it('keeps the parent identity for nested inline sub-workflows', async () => {
|
||||
await executeWorkflow(
|
||||
{ code: subWorkflowData() },
|
||||
mock<IWorkflowExecuteAdditionalData>({ userId: undefined }),
|
||||
{ parentWorkflowId: 'parent-1', executionMode: 'webhook' },
|
||||
);
|
||||
const [integratedAdditionalData] = vi.mocked(WorkflowExecute).mock.calls[0];
|
||||
const parentWorkflowId = integratedAdditionalData.workflowId;
|
||||
if (!parentWorkflowId) throw new Error('Expected a parent workflow ID');
|
||||
|
||||
await executeWorkflow({ code: subWorkflowData() }, integratedAdditionalData, {
|
||||
parentWorkflowId,
|
||||
executionMode: 'webhook',
|
||||
});
|
||||
|
||||
expect(credentialsPermissionChecker.check).toHaveBeenNthCalledWith(1, 'parent-1', nodes);
|
||||
expect(credentialsPermissionChecker.check).toHaveBeenNthCalledWith(2, 'parent-1', nodes);
|
||||
});
|
||||
|
||||
it('preserves parent static-data persistence for inline definitions without an id', async () => {
|
||||
await executeWorkflow(
|
||||
{ code: mock<IWorkflowBase>({ ...subWorkflowData(), id: undefined }) },
|
||||
mock<IWorkflowExecuteAdditionalData>({ userId: undefined }),
|
||||
{ parentWorkflowId: 'parent-1', executionMode: 'webhook' },
|
||||
);
|
||||
const [integratedAdditionalData, , runExecutionData] =
|
||||
vi.mocked(WorkflowExecute).mock.calls[0];
|
||||
if (!integratedAdditionalData.hooks || !runExecutionData) {
|
||||
throw new Error('Expected sub-workflow hooks and run data');
|
||||
}
|
||||
const staticData = { global: { lastProcessedId: 123 } };
|
||||
|
||||
await integratedAdditionalData.hooks.runHook('workflowExecuteAfter', [
|
||||
{ ...runWithData, data: runExecutionData, mode: 'integrated' },
|
||||
staticData,
|
||||
]);
|
||||
|
||||
expect(workflowStaticDataService.saveStaticDataById).toHaveBeenCalledWith(
|
||||
'parent-1',
|
||||
staticData,
|
||||
);
|
||||
});
|
||||
|
||||
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 () => {
|
||||
|
|
@ -1103,6 +1161,23 @@ describe('WorkflowExecuteAdditionalData', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe.each([
|
||||
['draft', getDraftWorkflowData],
|
||||
['published', getPublishedWorkflowData],
|
||||
])('inline %s workflow data', (_version, loadWorkflow) => {
|
||||
// Regression guard for inline workflow identity (IAM-1105).
|
||||
it.each(['embedded-id', undefined])(
|
||||
'uses the parent identity with inline id %s',
|
||||
async (id) => {
|
||||
const workflowCode = mock<IWorkflowBase>({ id, nodes: [], connections: {} });
|
||||
|
||||
const result = await loadWorkflow({ code: workflowCode }, 'parent-workflow-id');
|
||||
|
||||
expect(result.id).toBe('parent-workflow-id');
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe('getPublishedWorkflowData', () => {
|
||||
beforeEach(() => {
|
||||
workflowRepository.get.mockClear();
|
||||
|
|
@ -1249,7 +1324,8 @@ describe('WorkflowExecuteAdditionalData', () => {
|
|||
|
||||
const result = await getPublishedWorkflowData({ code: workflowCode }, 'parent-workflow-id');
|
||||
|
||||
expect(result).toEqual(workflowCode);
|
||||
expect(result.id).toBe('parent-workflow-id');
|
||||
expect(result.nodes).toEqual(workflowCode.nodes);
|
||||
expect(workflowRepository.get).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ import {
|
|||
SecretsProviderConnectionRepository,
|
||||
} from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import { Credentials, getAdditionalKeys } from 'n8n-core';
|
||||
import { Credentials, FULL_ACCESS_NODE_TYPES, getAdditionalKeys } from 'n8n-core';
|
||||
import type {
|
||||
CredentialInformation,
|
||||
ICredentialDataDecryptedObject,
|
||||
|
|
@ -30,6 +30,7 @@ import type {
|
|||
import {
|
||||
ICredentialsHelper,
|
||||
NodeHelpers,
|
||||
OPEN_AI_API_CREDENTIAL_TYPE,
|
||||
Workflow,
|
||||
UnexpectedError,
|
||||
UserError,
|
||||
|
|
@ -585,6 +586,16 @@ export class CredentialsHelper extends ICredentialsHelper {
|
|||
|
||||
const credentialsEntity = await this.getCredentialsEntity(nodeCredentials, type);
|
||||
|
||||
// Managed OpenAI credentials are unavailable to nodes that can request undeclared types.
|
||||
if (
|
||||
credentialsEntity.isManaged &&
|
||||
type === OPEN_AI_API_CREDENTIAL_TYPE &&
|
||||
consumerNode &&
|
||||
FULL_ACCESS_NODE_TYPES.has(consumerNode.type)
|
||||
) {
|
||||
throw new UserError('Managed credentials are not supported by this node');
|
||||
}
|
||||
|
||||
// Validate against the executing project's policy before any decryption happens.
|
||||
await this.policyEnforcementService.enforceCredentialDecrypt(
|
||||
{
|
||||
|
|
|
|||
|
|
@ -2035,6 +2035,7 @@ describe('CredentialsService', () => {
|
|||
id: 'credential-id',
|
||||
name: 'Stored Credential',
|
||||
type: 'githubApi',
|
||||
isManaged: false,
|
||||
});
|
||||
const decryptedData = { accessToken: 'stored-token' } as ICredentialDataDecryptedObject;
|
||||
const unredactedData = { accessToken: 'live-token' } as ICredentialDataDecryptedObject;
|
||||
|
|
@ -2071,11 +2072,50 @@ describe('CredentialsService', () => {
|
|||
expect(result).toEqual(testResult);
|
||||
});
|
||||
|
||||
it('uses stored values when testing a managed credential', async () => {
|
||||
const storedCredential = mock<CredentialsEntity>({
|
||||
id: 'credential-id',
|
||||
name: 'Stored Credential',
|
||||
type: 'openAiApi',
|
||||
isManaged: true,
|
||||
});
|
||||
const decryptedData = {
|
||||
apiKey: 'stored-key',
|
||||
url: 'https://saved.example/v1',
|
||||
} as ICredentialDataDecryptedObject;
|
||||
const testResult = { status: 'OK', message: 'Credential tested successfully' } as const;
|
||||
|
||||
credentialsFinderService.findCredentialForUser.mockResolvedValue(storedCredential);
|
||||
sharedCredentialsRepository.findCredentialOwningProject.mockResolvedValue(undefined);
|
||||
vi.spyOn(service, 'decrypt').mockResolvedValue(decryptedData);
|
||||
credentialsTester.testCredentials.mockResolvedValue(testResult);
|
||||
|
||||
const result = await service.testWithCredentials(ownerUser, {
|
||||
id: storedCredential.id,
|
||||
name: 'Draft Credential',
|
||||
type: 'githubApi',
|
||||
data: {
|
||||
apiKey: CREDENTIAL_BLANKING_VALUE,
|
||||
url: 'https://draft.example/v1',
|
||||
},
|
||||
});
|
||||
|
||||
expect(credentialsTester.testCredentials).toHaveBeenCalledWith(ownerUser.id, 'openAiApi', {
|
||||
id: storedCredential.id,
|
||||
name: storedCredential.name,
|
||||
type: storedCredential.type,
|
||||
data: decryptedData,
|
||||
homeProject: undefined,
|
||||
});
|
||||
expect(result).toEqual(testResult);
|
||||
});
|
||||
|
||||
it('discards a caller-supplied homeProject and resolves the owning project from storage', async () => {
|
||||
const storedCredential = mock<CredentialsEntity>({
|
||||
id: 'credential-id',
|
||||
name: 'Stored Credential',
|
||||
type: 'githubApi',
|
||||
isManaged: false,
|
||||
});
|
||||
const decryptedData = { accessToken: 'stored-token' } as ICredentialDataDecryptedObject;
|
||||
const testResult = { status: 'OK', message: 'Credential tested successfully' } as const;
|
||||
|
|
@ -2131,6 +2171,7 @@ describe('CredentialsService', () => {
|
|||
id: 'credential-id',
|
||||
name: 'Stored Credential',
|
||||
type: 'githubApi',
|
||||
isManaged: false,
|
||||
});
|
||||
const decryptedData = { accessToken: 'stored-token' } as ICredentialDataDecryptedObject;
|
||||
const testResult = { status: 'OK', message: 'Credential tested successfully' } as const;
|
||||
|
|
|
|||
|
|
@ -1451,7 +1451,7 @@ export class CredentialsService {
|
|||
const mergedCredentials = await this.prepareCredentialsForTest({
|
||||
storedCredential,
|
||||
user,
|
||||
credentialsToTest: credentials,
|
||||
credentialsToTest: storedCredential.isManaged ? undefined : credentials,
|
||||
});
|
||||
|
||||
return await this.test(user.id, mergedCredentials);
|
||||
|
|
|
|||
|
|
@ -208,4 +208,73 @@ describe('executeErrorWorkflow', () => {
|
|||
expect(workflowExecutionService.executeErrorWorkflow).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('execution context handed to the error workflow', () => {
|
||||
const runtimeData = {
|
||||
version: 1 as const,
|
||||
establishedAt: 2000000000,
|
||||
source: 'webhook' as const,
|
||||
credentials: 'sealed-identity-carrier',
|
||||
secureArtifacts: 'stripped-trigger-values',
|
||||
redaction: { version: 2 as const, production: true, manual: false },
|
||||
executedByUserId: 'user-who-ran-the-failed-workflow',
|
||||
usesDynamicCredentials: true,
|
||||
};
|
||||
|
||||
const runFailedWorkflow = async () => {
|
||||
const workflowData = mock<IWorkflowBase>({
|
||||
id: 'workflow-123',
|
||||
name: 'Failing workflow',
|
||||
settings: { errorWorkflow: 'error-workflow-456' },
|
||||
nodes: [],
|
||||
});
|
||||
const data = createRunExecutionData({
|
||||
resultData: {
|
||||
error: new NodeOperationError(
|
||||
mockNode,
|
||||
'Test error',
|
||||
) as IRun['data']['resultData']['error'],
|
||||
lastNodeExecuted: 'TestNode',
|
||||
runData: {},
|
||||
},
|
||||
executionData: { nodeExecutionStack: [] },
|
||||
});
|
||||
data.executionData!.runtimeData = runtimeData;
|
||||
|
||||
ownershipService.getWorkflowProjectCached.mockResolvedValue({ id: 'project-1' } as never);
|
||||
|
||||
executeErrorWorkflow(
|
||||
workflowData,
|
||||
{ data, mode: 'trigger', startedAt: new Date(), storedAt: 'db', status: 'error' },
|
||||
'trigger',
|
||||
'execution-1',
|
||||
);
|
||||
await new Promise(process.nextTick);
|
||||
|
||||
const [, workflowErrorData] =
|
||||
workflowExecutionService.executeErrorWorkflow.mock.calls.at(0) ?? [];
|
||||
return workflowErrorData!.execution!.executionContext;
|
||||
};
|
||||
|
||||
it('omits the identity carrier and the secure artifacts', async () => {
|
||||
const executionContext = await runFailedWorkflow();
|
||||
|
||||
expect(executionContext).toBeDefined();
|
||||
expect(executionContext).not.toHaveProperty('credentials');
|
||||
expect(executionContext).not.toHaveProperty('secureArtifacts');
|
||||
});
|
||||
|
||||
// This same payload becomes the error workflow's start-item `$json`, so a
|
||||
// carrier left on it would be readable workflow data, not just inherited state.
|
||||
it('passes only the non-identity fields', async () => {
|
||||
expect(await runFailedWorkflow()).toEqual({
|
||||
version: 1,
|
||||
establishedAt: 2000000000,
|
||||
source: 'webhook',
|
||||
redaction: { version: 2, production: true, manual: false },
|
||||
executedByUserId: 'user-who-ran-the-failed-workflow',
|
||||
usesDynamicCredentials: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import { GlobalConfig } from '@n8n/config';
|
|||
import { Container } from '@n8n/di';
|
||||
import { ErrorReporter } from 'n8n-core';
|
||||
import type { IRun, IWorkflowBase, WorkflowExecuteMode } from 'n8n-workflow';
|
||||
import { toErrorWorkflowContext } from 'n8n-workflow';
|
||||
|
||||
import type { IWorkflowErrorData } from '@/interfaces';
|
||||
import { isPolicyRefusal } from '@/policy/policy-violation.error';
|
||||
|
|
@ -74,7 +75,7 @@ export function executeErrorWorkflow(
|
|||
lastNodeExecuted: fullRunData.data.resultData.lastNodeExecuted!,
|
||||
mode,
|
||||
retryOf,
|
||||
executionContext: fullRunData.data.executionData?.runtimeData,
|
||||
executionContext: toErrorWorkflowContext(fullRunData.data.executionData?.runtimeData),
|
||||
},
|
||||
workflow: {
|
||||
id: workflowId,
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import { Service } from '@n8n/di';
|
|||
import { ensureError } from '@n8n/utils/errors/ensure-error';
|
||||
import { z } from 'zod';
|
||||
|
||||
import { selectVettedVersion } from '@/modules/community-packages/community-node-types-utils';
|
||||
import { CommunityNodeTypesService } from '@/modules/community-packages/community-node-types.service';
|
||||
import { CommunityPackagesConfig } from '@/modules/community-packages/community-packages.config';
|
||||
import {
|
||||
|
|
@ -200,19 +201,9 @@ export class CommunityPackagesInstanceSettingsLoader {
|
|||
);
|
||||
}
|
||||
|
||||
let resolvedVersion = item.version;
|
||||
let checksum: string | undefined;
|
||||
|
||||
if (vetted) {
|
||||
if (item.version === undefined) {
|
||||
resolvedVersion = vetted.npmVersion;
|
||||
checksum = vetted.checksum;
|
||||
} else if (vetted.npmVersion === item.version) {
|
||||
checksum = vetted.checksum;
|
||||
} else {
|
||||
checksum = vetted.nodeVersions?.find((v) => v.npmVersion === item.version)?.checksum;
|
||||
}
|
||||
}
|
||||
const { version: resolvedVersion, checksum } = vetted
|
||||
? selectVettedVersion(vetted, item.version)
|
||||
: { version: item.version, checksum: undefined };
|
||||
|
||||
if (!checksum && !this.communityPackagesConfig.unverifiedEnabled) {
|
||||
const ref = resolvedVersion ? `'${item.name}@${resolvedVersion}'` : `'${item.name}'`;
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import type { CommunityNodeType } from '@n8n/api-types';
|
||||
import type { CommunityNodeType, CommunityPackageRequestDto } from '@n8n/api-types';
|
||||
import type { Logger } from '@n8n/backend-common';
|
||||
import type { EventService } from '@n8n/backend-services';
|
||||
import type { InstanceSettingsLoaderConfig } from '@n8n/config';
|
||||
|
|
@ -6,7 +6,7 @@ import type { InstanceSettings } from 'n8n-core';
|
|||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import type { Push } from '@/push';
|
||||
import type { NodeRequest } from '@/requests';
|
||||
import type { AuthenticatedRequest } from '@n8n/db';
|
||||
|
||||
import type { CommunityNodeTypesService } from '../community-node-types.service';
|
||||
import type { CommunityPackagesConfig } from '../community-packages.config';
|
||||
|
|
@ -23,7 +23,7 @@ describe('CommunityPackagesController', () => {
|
|||
const communityNodeTypesService = mock<CommunityNodeTypesService>();
|
||||
const instanceSettings = mock<InstanceSettings>();
|
||||
(instanceSettings as any).nodesDownloadDir = '/tmp/n8n-nodes-download';
|
||||
const communityPackagesConfig = mock<CommunityPackagesConfig>();
|
||||
const communityPackagesConfig = mock<CommunityPackagesConfig>({ unverifiedEnabled: true });
|
||||
const instanceSettingsLoaderConfig = mock<InstanceSettingsLoaderConfig>({
|
||||
communityPackagesManagedByEnv: false,
|
||||
});
|
||||
|
|
@ -40,19 +40,24 @@ describe('CommunityPackagesController', () => {
|
|||
);
|
||||
|
||||
const controller = new CommunityPackagesController(lifecycle);
|
||||
const req = mock<AuthenticatedRequest>({ user: { id: 'user1' } });
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
communityPackagesConfig.unverifiedEnabled = true;
|
||||
});
|
||||
|
||||
describe('installPackage', () => {
|
||||
it('should throw error if verify in options but no checksum', async () => {
|
||||
const request = mock<NodeRequest.Post>({
|
||||
user: { id: 'user123' },
|
||||
body: { name: 'n8n-nodes-test', verify: true, version: '1.0.0' },
|
||||
it('should reject an install when unverified packages are disabled and the package is not vetted', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
const body = { name: 'n8n-nodes-test', version: '1.0.0' };
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue({
|
||||
rawString: 'n8n-nodes-test',
|
||||
packageName: 'n8n-nodes-test',
|
||||
version: undefined,
|
||||
});
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(undefined);
|
||||
await expect(controller.installPackage(request)).rejects.toThrow(
|
||||
await expect(controller.installPackage(req, {}, body)).rejects.toThrow(
|
||||
'Package n8n-nodes-test is not vetted for installation',
|
||||
);
|
||||
});
|
||||
|
|
@ -60,21 +65,16 @@ describe('CommunityPackagesController', () => {
|
|||
it.each(['echo "hello"', '1.a.b', '0.1.29#;ls'])(
|
||||
'should throw error if version is invalid',
|
||||
async (version) => {
|
||||
const request = mock<NodeRequest.Post>({
|
||||
user: { id: 'user123' },
|
||||
body: { name: 'n8n-nodes-test', verify: true, version },
|
||||
});
|
||||
await expect(controller.installPackage(request)).rejects.toThrow(
|
||||
const body = { name: 'n8n-nodes-test', version };
|
||||
await expect(controller.installPackage(req, {}, body)).rejects.toThrow(
|
||||
`Invalid version: ${version}`,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it('should have correct version', async () => {
|
||||
const request = mock<NodeRequest.Post>({
|
||||
user: { id: 'user123' },
|
||||
body: { name: 'n8n-nodes-test', verify: true, version: '1.0.0' },
|
||||
});
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
const body = { name: 'n8n-nodes-test', version: '1.0.0' };
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(
|
||||
mock<CommunityNodeType>({
|
||||
npmVersion: '1.1.1',
|
||||
|
|
@ -99,7 +99,7 @@ describe('CommunityPackagesController', () => {
|
|||
}),
|
||||
);
|
||||
|
||||
await controller.installPackage(request);
|
||||
await controller.installPackage(req, {}, body);
|
||||
|
||||
expect(communityPackagesService.installPackage).toHaveBeenCalledWith(
|
||||
'n8n-nodes-test',
|
||||
|
|
@ -116,15 +116,12 @@ describe('CommunityPackagesController', () => {
|
|||
});
|
||||
|
||||
describe('updatePackage', () => {
|
||||
it('should use the version from the request body when updating a package', async () => {
|
||||
const req = mock<NodeRequest.Update>({
|
||||
body: {
|
||||
name: 'n8n-nodes-test',
|
||||
version: '2.0.0',
|
||||
checksum: 'a893hfdsy7399',
|
||||
},
|
||||
user: { id: 'user1' },
|
||||
});
|
||||
it('should use the version from the request body and ignore a checksum in the body', async () => {
|
||||
const body = {
|
||||
name: 'n8n-nodes-test',
|
||||
version: '2.0.0',
|
||||
checksum: 'a893hfdsy7399',
|
||||
} as CommunityPackageRequestDto;
|
||||
|
||||
const previouslyInstalledPackage = mock<InstalledPackages>({
|
||||
installedNodes: [{ type: 'testNode', latestVersion: 1, name: 'testNode' }],
|
||||
|
|
@ -147,25 +144,59 @@ describe('CommunityPackagesController', () => {
|
|||
version: undefined,
|
||||
});
|
||||
|
||||
const result = await controller.updatePackage(req);
|
||||
const result = await controller.updatePackage(req, {}, body);
|
||||
|
||||
expect(communityPackagesService.updatePackage).toHaveBeenCalledWith(
|
||||
'n8n-nodes-test',
|
||||
previouslyInstalledPackage,
|
||||
'2.0.0',
|
||||
'a893hfdsy7399',
|
||||
undefined,
|
||||
);
|
||||
|
||||
expect(result).toBe(newInstalledPackage);
|
||||
});
|
||||
|
||||
it('should resolve the checksum from the vetted list when unverified packages are disabled', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
const body = {
|
||||
name: 'n8n-nodes-test',
|
||||
version: '2.0.0',
|
||||
checksum: 'a893hfdsy7399',
|
||||
} as CommunityPackageRequestDto;
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(
|
||||
mock<CommunityNodeType>({ npmVersion: '2.0.0', checksum: 'vetted-checksum' }),
|
||||
);
|
||||
const previouslyInstalledPackage = mock<InstalledPackages>({
|
||||
installedNodes: [],
|
||||
installedVersion: '1.0.0',
|
||||
});
|
||||
communityPackagesService.findInstalledPackage.mockResolvedValue(previouslyInstalledPackage);
|
||||
communityPackagesService.updatePackage.mockResolvedValue(
|
||||
mock<InstalledPackages>({ installedNodes: [], installedVersion: '2.0.0' }),
|
||||
);
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue({
|
||||
rawString: 'n8n-nodes-test',
|
||||
packageName: 'n8n-nodes-test',
|
||||
version: undefined,
|
||||
});
|
||||
|
||||
await controller.updatePackage(req, {}, body);
|
||||
|
||||
expect(communityPackagesService.updatePackage).toHaveBeenCalledWith(
|
||||
'n8n-nodes-test',
|
||||
previouslyInstalledPackage,
|
||||
'2.0.0',
|
||||
'vetted-checksum',
|
||||
);
|
||||
});
|
||||
|
||||
it.each(['echo "hello"', '1.a.b', '0.1.29#;ls'])(
|
||||
'should throw error if version is invalid',
|
||||
async (version) => {
|
||||
const req = mock<NodeRequest.Update>({
|
||||
body: { name: 'n8n-nodes-test', version, checksum: 'a893hfdsy7399' },
|
||||
});
|
||||
await expect(controller.updatePackage(req)).rejects.toThrow(`Invalid version: ${version}`);
|
||||
const body = { name: 'n8n-nodes-test', version };
|
||||
await expect(controller.updatePackage(req, {}, body)).rejects.toThrow(
|
||||
`Invalid version: ${version}`,
|
||||
);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -249,6 +249,8 @@ describe('PATCH /community-packages', () => {
|
|||
});
|
||||
|
||||
test('should reject if package is not installed', async () => {
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue(parsedNpmPackageName);
|
||||
|
||||
const {
|
||||
body: { message },
|
||||
} = await authAgent.patch('/community-packages').send({ name: mockPackageName() }).expect(400);
|
||||
|
|
|
|||
|
|
@ -63,15 +63,22 @@ describe('CommunityPackagesLifecycleService', () => {
|
|||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
communityPackagesConfig.unverifiedEnabled = true;
|
||||
communityPackagesService.withLoadStatus.mockImplementation((packages) => packages);
|
||||
});
|
||||
|
||||
describe('install', () => {
|
||||
it('should throw error if verify in options but no checksum', async () => {
|
||||
it('should reject an install when unverified packages are disabled and the package is not vetted', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue({
|
||||
rawString: 'n8n-nodes-test',
|
||||
packageName: 'n8n-nodes-test',
|
||||
version: undefined,
|
||||
});
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(undefined);
|
||||
|
||||
await expect(
|
||||
lifecycle.install({ name: 'n8n-nodes-test', verify: true, version: '1.0.0' }, user, 'ui'),
|
||||
lifecycle.install({ name: 'n8n-nodes-test', version: '1.0.0' }, user, 'ui'),
|
||||
).rejects.toThrow('Package n8n-nodes-test is not vetted for installation');
|
||||
|
||||
expect(communityNodeTypesService.findVetted).toHaveBeenCalledWith('n8n-nodes-test');
|
||||
|
|
@ -81,12 +88,35 @@ describe('CommunityPackagesLifecycleService', () => {
|
|||
'should throw error if version is invalid',
|
||||
async (version) => {
|
||||
await expect(
|
||||
lifecycle.install({ name: 'n8n-nodes-test', verify: true, version }, user, 'ui'),
|
||||
lifecycle.install({ name: 'n8n-nodes-test', version }, user, 'ui'),
|
||||
).rejects.toThrow(`Invalid version: ${version}`);
|
||||
},
|
||||
);
|
||||
|
||||
it('should install with checksum when verify is true', async () => {
|
||||
it('should install the requested version without a checksum when unverified packages are enabled', async () => {
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue({
|
||||
rawString: 'n8n-nodes-test',
|
||||
packageName: 'n8n-nodes-test',
|
||||
version: undefined,
|
||||
});
|
||||
communityPackagesService.findInstalledPackage.mockResolvedValue(null);
|
||||
communityPackagesService.checkNpmPackageStatus.mockResolvedValue({ status: 'OK' });
|
||||
communityPackagesService.installPackage.mockResolvedValue(
|
||||
mock<InstalledPackages>({ installedNodes: [] }),
|
||||
);
|
||||
|
||||
await lifecycle.install({ name: 'n8n-nodes-test', version: '1.5.0' }, user, 'ui');
|
||||
|
||||
expect(communityNodeTypesService.findVetted).not.toHaveBeenCalled();
|
||||
expect(communityPackagesService.installPackage).toHaveBeenCalledWith(
|
||||
'n8n-nodes-test',
|
||||
'1.5.0',
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it('should install a vetted version with its checksum when unverified packages are disabled', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(
|
||||
mock<CommunityNodeType>({
|
||||
npmVersion: '1.1.1',
|
||||
|
|
@ -111,11 +141,7 @@ describe('CommunityPackagesLifecycleService', () => {
|
|||
}),
|
||||
);
|
||||
|
||||
await lifecycle.install(
|
||||
{ name: 'n8n-nodes-test', verify: true, version: '1.0.0' },
|
||||
user,
|
||||
'ui',
|
||||
);
|
||||
await lifecycle.install({ name: 'n8n-nodes-test', version: '1.0.0' }, user, 'ui');
|
||||
|
||||
expect(communityPackagesService.installPackage).toHaveBeenCalledWith(
|
||||
'n8n-nodes-test',
|
||||
|
|
@ -130,6 +156,85 @@ describe('CommunityPackagesLifecycleService', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should install the latest vetted version when unverified packages are disabled and no version is given', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(
|
||||
mock<CommunityNodeType>({ checksum: 'vetted-checksum', npmVersion: '1.2.3' }),
|
||||
);
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue({
|
||||
rawString: 'n8n-nodes-test',
|
||||
packageName: 'n8n-nodes-test',
|
||||
version: undefined,
|
||||
});
|
||||
communityPackagesService.findInstalledPackage.mockResolvedValue(null);
|
||||
communityPackagesService.checkNpmPackageStatus.mockResolvedValue({ status: 'OK' });
|
||||
communityPackagesService.installPackage.mockResolvedValue(
|
||||
mock<InstalledPackages>({ installedNodes: [] }),
|
||||
);
|
||||
|
||||
await lifecycle.install({ name: 'n8n-nodes-test' }, user, 'ui');
|
||||
|
||||
expect(communityPackagesService.installPackage).toHaveBeenCalledWith(
|
||||
'n8n-nodes-test',
|
||||
'1.2.3',
|
||||
'vetted-checksum',
|
||||
);
|
||||
});
|
||||
|
||||
it('should resolve a scoped package with a version suffix against the vetted list', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue({
|
||||
rawString: '@scope/n8n-nodes-test@1.0.0',
|
||||
packageName: '@scope/n8n-nodes-test',
|
||||
scope: '@scope',
|
||||
version: '1.0.0',
|
||||
});
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(
|
||||
mock<CommunityNodeType>({
|
||||
checksum: 'latest-checksum',
|
||||
npmVersion: '2.0.0',
|
||||
nodeVersions: [{ npmVersion: '1.0.0', checksum: 'v1-checksum' }],
|
||||
}),
|
||||
);
|
||||
communityPackagesService.findInstalledPackage.mockResolvedValue(null);
|
||||
communityPackagesService.checkNpmPackageStatus.mockResolvedValue({ status: 'OK' });
|
||||
communityPackagesService.installPackage.mockResolvedValue(
|
||||
mock<InstalledPackages>({ installedNodes: [] }),
|
||||
);
|
||||
|
||||
await lifecycle.install({ name: '@scope/n8n-nodes-test@1.0.0' }, user, 'ui');
|
||||
|
||||
expect(communityNodeTypesService.findVetted).toHaveBeenCalledWith('@scope/n8n-nodes-test');
|
||||
expect(communityPackagesService.installPackage).toHaveBeenCalledWith(
|
||||
'@scope/n8n-nodes-test',
|
||||
'1.0.0',
|
||||
'v1-checksum',
|
||||
);
|
||||
});
|
||||
|
||||
it('should reject an install of a version that is not in the vetted list when unverified packages are disabled', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue({
|
||||
rawString: 'n8n-nodes-test',
|
||||
packageName: 'n8n-nodes-test',
|
||||
version: undefined,
|
||||
});
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(
|
||||
mock<CommunityNodeType>({
|
||||
checksum: 'latest-checksum',
|
||||
npmVersion: '2.0.0',
|
||||
nodeVersions: [{ npmVersion: '1.0.0', checksum: 'v1-checksum' }],
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(
|
||||
lifecycle.install({ name: 'n8n-nodes-test', version: '1.5.0' }, user, 'ui'),
|
||||
).rejects.toThrow(
|
||||
'Version 1.5.0 of n8n-nodes-test is not verified by n8n. Latest verified version is 2.0.0',
|
||||
);
|
||||
expect(communityPackagesService.installPackage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject install when the package is already installed and loaded', async () => {
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue({
|
||||
rawString: 'n8n-nodes-test',
|
||||
|
|
@ -207,7 +312,7 @@ describe('CommunityPackagesLifecycleService', () => {
|
|||
),
|
||||
);
|
||||
|
||||
const promise = lifecycle.install({ name: 'n8n-nodes-test', verify: true }, user, 'ui');
|
||||
const promise = lifecycle.install({ name: 'n8n-nodes-test' }, user, 'ui');
|
||||
await expect(promise).rejects.toBeInstanceOf(IncompatibleNodesApiVersionError);
|
||||
await expect(promise).rejects.toMatchObject({
|
||||
httpStatusCode: 400,
|
||||
|
|
@ -334,7 +439,7 @@ describe('CommunityPackagesLifecycleService', () => {
|
|||
});
|
||||
|
||||
describe('update', () => {
|
||||
it('should use the version from the request when updating a package', async () => {
|
||||
it('should update without a checksum when unverified packages are enabled', async () => {
|
||||
const previouslyInstalledPackage = mockPackage('1.0.0');
|
||||
const newInstalledPackage = mockPackage('2.0.0');
|
||||
|
||||
|
|
@ -347,20 +452,17 @@ describe('CommunityPackagesLifecycleService', () => {
|
|||
});
|
||||
|
||||
const result = await lifecycle.update(
|
||||
{
|
||||
name: 'n8n-nodes-test',
|
||||
version: '2.0.0',
|
||||
checksum: 'a893hfdsy7399',
|
||||
},
|
||||
{ name: 'n8n-nodes-test', version: '2.0.0' },
|
||||
user,
|
||||
'badRequest',
|
||||
);
|
||||
|
||||
expect(communityNodeTypesService.findVetted).not.toHaveBeenCalled();
|
||||
expect(communityPackagesService.updatePackage).toHaveBeenCalledWith(
|
||||
'n8n-nodes-test',
|
||||
previouslyInstalledPackage,
|
||||
'2.0.0',
|
||||
'a893hfdsy7399',
|
||||
undefined,
|
||||
);
|
||||
|
||||
expect(result).toBe(newInstalledPackage);
|
||||
|
|
@ -370,28 +472,13 @@ describe('CommunityPackagesLifecycleService', () => {
|
|||
'should throw error if version is invalid',
|
||||
async (version) => {
|
||||
await expect(
|
||||
lifecycle.update(
|
||||
{ name: 'n8n-nodes-test', version, checksum: 'a893hfdsy7399' },
|
||||
user,
|
||||
'badRequest',
|
||||
),
|
||||
lifecycle.update({ name: 'n8n-nodes-test', version }, user, 'badRequest'),
|
||||
).rejects.toThrow(`Invalid version: ${version}`);
|
||||
},
|
||||
);
|
||||
|
||||
it('should throw error if verify is true but package is not vetted', async () => {
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(undefined);
|
||||
|
||||
await expect(
|
||||
lifecycle.update(
|
||||
{ name: 'n8n-nodes-test', version: '2.0.0', verify: true },
|
||||
user,
|
||||
'badRequest',
|
||||
),
|
||||
).rejects.toThrow('Package n8n-nodes-test is not vetted for installation');
|
||||
});
|
||||
|
||||
it('should update with checksum when verify is true and version matches latest', async () => {
|
||||
it('should update with the vetted checksum when unverified packages are disabled and the version matches latest', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(
|
||||
mock<CommunityNodeType>({
|
||||
checksum: 'vetted-checksum',
|
||||
|
|
@ -409,11 +496,7 @@ describe('CommunityPackagesLifecycleService', () => {
|
|||
version: undefined,
|
||||
});
|
||||
|
||||
await lifecycle.update(
|
||||
{ name: 'n8n-nodes-test', version: '2.0.0', verify: true },
|
||||
user,
|
||||
'badRequest',
|
||||
);
|
||||
await lifecycle.update({ name: 'n8n-nodes-test', version: '2.0.0' }, user, 'badRequest');
|
||||
|
||||
expect(communityPackagesService.updatePackage).toHaveBeenCalledWith(
|
||||
'n8n-nodes-test',
|
||||
|
|
@ -424,6 +507,7 @@ describe('CommunityPackagesLifecycleService', () => {
|
|||
});
|
||||
|
||||
it('should use version-specific checksum from nodeVersions when targeting older version', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(
|
||||
mock<CommunityNodeType>({
|
||||
checksum: 'latest-checksum',
|
||||
|
|
@ -441,11 +525,7 @@ describe('CommunityPackagesLifecycleService', () => {
|
|||
version: undefined,
|
||||
});
|
||||
|
||||
await lifecycle.update(
|
||||
{ name: 'n8n-nodes-test', version: '1.0.0', verify: true },
|
||||
user,
|
||||
'badRequest',
|
||||
);
|
||||
await lifecycle.update({ name: 'n8n-nodes-test', version: '1.0.0' }, user, 'badRequest');
|
||||
|
||||
expect(communityPackagesService.updatePackage).toHaveBeenCalledWith(
|
||||
'n8n-nodes-test',
|
||||
|
|
@ -455,6 +535,91 @@ describe('CommunityPackagesLifecycleService', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should update to the latest vetted version when unverified packages are disabled and no version is given', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(
|
||||
mock<CommunityNodeType>({ checksum: 'vetted-checksum', npmVersion: '2.0.0' }),
|
||||
);
|
||||
const previouslyInstalledPackage = mockPackage('1.0.0');
|
||||
communityPackagesService.findInstalledPackage.mockResolvedValue(previouslyInstalledPackage);
|
||||
communityPackagesService.updatePackage.mockResolvedValue(mockPackage('2.0.0'));
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue({
|
||||
rawString: 'n8n-nodes-test',
|
||||
packageName: 'n8n-nodes-test',
|
||||
version: undefined,
|
||||
});
|
||||
|
||||
await lifecycle.update({ name: 'n8n-nodes-test' }, user, 'badRequest');
|
||||
|
||||
expect(communityPackagesService.updatePackage).toHaveBeenCalledWith(
|
||||
'n8n-nodes-test',
|
||||
previouslyInstalledPackage,
|
||||
'2.0.0',
|
||||
'vetted-checksum',
|
||||
);
|
||||
});
|
||||
|
||||
it('should resolve a scoped package with a version suffix against the vetted list', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValueOnce({
|
||||
rawString: '@scope/n8n-nodes-test@1.0.0',
|
||||
packageName: '@scope/n8n-nodes-test',
|
||||
scope: '@scope',
|
||||
version: '1.0.0',
|
||||
});
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(
|
||||
mock<CommunityNodeType>({
|
||||
checksum: 'latest-checksum',
|
||||
npmVersion: '2.0.0',
|
||||
nodeVersions: [{ npmVersion: '1.0.0', checksum: 'v1-checksum' }],
|
||||
}),
|
||||
);
|
||||
const previouslyInstalledPackage = mockPackage('0.5.0');
|
||||
communityPackagesService.findInstalledPackage.mockResolvedValue(previouslyInstalledPackage);
|
||||
communityPackagesService.updatePackage.mockResolvedValue(mockPackage('1.0.0'));
|
||||
|
||||
await lifecycle.update({ name: '@scope/n8n-nodes-test@1.0.0' }, user, 'badRequest');
|
||||
|
||||
expect(communityNodeTypesService.findVetted).toHaveBeenCalledWith('@scope/n8n-nodes-test');
|
||||
expect(communityPackagesService.findInstalledPackage).toHaveBeenCalledWith(
|
||||
'@scope/n8n-nodes-test',
|
||||
);
|
||||
expect(communityPackagesService.updatePackage).toHaveBeenCalledWith(
|
||||
'@scope/n8n-nodes-test',
|
||||
previouslyInstalledPackage,
|
||||
'1.0.0',
|
||||
'v1-checksum',
|
||||
);
|
||||
});
|
||||
|
||||
it('should reject an update when unverified packages are disabled and the package is not vetted', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(undefined);
|
||||
|
||||
await expect(
|
||||
lifecycle.update({ name: 'n8n-nodes-test', version: '2.0.0' }, user, 'badRequest'),
|
||||
).rejects.toThrow('Package n8n-nodes-test is not vetted for installation');
|
||||
expect(communityPackagesService.updatePackage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject an update to a version that is not in the vetted list when unverified packages are disabled', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(
|
||||
mock<CommunityNodeType>({
|
||||
checksum: 'latest-checksum',
|
||||
npmVersion: '2.0.0',
|
||||
nodeVersions: [{ npmVersion: '1.0.0', checksum: 'v1-checksum' }],
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(
|
||||
lifecycle.update({ name: 'n8n-nodes-test', version: '1.5.0' }, user, 'badRequest'),
|
||||
).rejects.toThrow(
|
||||
'Version 1.5.0 of n8n-nodes-test is not verified by n8n. Latest verified version is 2.0.0',
|
||||
);
|
||||
expect(communityPackagesService.updatePackage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should throw NotFoundError when package missing and whenMissing is notFound', async () => {
|
||||
communityPackagesService.findInstalledPackage.mockResolvedValue(null);
|
||||
|
||||
|
|
|
|||
|
|
@ -132,6 +132,20 @@ describe('CommunityPackagesService', () => {
|
|||
},
|
||||
);
|
||||
|
||||
test.each([
|
||||
['n8n-nodes-test', 'n8n-nodes-test', undefined, undefined],
|
||||
['n8n-nodes-test@1.2.3', 'n8n-nodes-test', undefined, '1.2.3'],
|
||||
['@scope/n8n-nodes-test', '@scope/n8n-nodes-test', '@scope', undefined],
|
||||
['@scope/n8n-nodes-test@1.2.3', '@scope/n8n-nodes-test', '@scope', '1.2.3'],
|
||||
])('should parse "%s"', (rawString, packageName, scope, version) => {
|
||||
expect(communityPackagesService.parseNpmPackageName(rawString)).toEqual({
|
||||
rawString,
|
||||
packageName,
|
||||
scope,
|
||||
version,
|
||||
});
|
||||
});
|
||||
|
||||
test.each(['n8n-nodes-base', '@n8n/n8n-nodes-langchain'])(
|
||||
'should reject reserved package name %s',
|
||||
(name) => {
|
||||
|
|
|
|||
|
|
@ -23,6 +23,23 @@ export type StrapiCommunityNodeType = {
|
|||
aiNodeSdkVersion?: number;
|
||||
};
|
||||
|
||||
/**
|
||||
* Picks the version to install from a vetted entry and the checksum that belongs to it.
|
||||
* No requested version means the latest vetted one. `checksum` is undefined when the
|
||||
* requested version is not vetted.
|
||||
*/
|
||||
export function selectVettedVersion(
|
||||
vetted: Pick<StrapiCommunityNodeType, 'npmVersion' | 'checksum' | 'nodeVersions'>,
|
||||
requestedVersion: string | undefined,
|
||||
): { version: string; checksum: string | undefined } {
|
||||
const version = requestedVersion ?? vetted.npmVersion;
|
||||
const checksum =
|
||||
version === vetted.npmVersion
|
||||
? vetted.checksum
|
||||
: vetted.nodeVersions?.find((v) => v.npmVersion === version)?.checksum;
|
||||
return { version, checksum };
|
||||
}
|
||||
|
||||
export type CommunityNodesMetadata = Pick<
|
||||
StrapiCommunityNodeType,
|
||||
'id' | 'name' | 'npmVersion' | 'updatedAt'
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
import { Delete, Get, Patch, Post, RestController, GlobalScope } from '@n8n/decorators';
|
||||
import { CommunityPackageRequestDto } from '@n8n/api-types';
|
||||
import type { AuthenticatedRequest } from '@n8n/db';
|
||||
import { Body, Delete, Get, Patch, Post, RestController, GlobalScope } from '@n8n/decorators';
|
||||
|
||||
import { NodeRequest } from '@/requests';
|
||||
import type { NodeRequest } from '@/requests';
|
||||
|
||||
import { CommunityPackagesLifecycleService } from './community-packages.lifecycle.service';
|
||||
|
||||
|
|
@ -10,10 +12,12 @@ export class CommunityPackagesController {
|
|||
|
||||
@Post('/')
|
||||
@GlobalScope('communityPackage:install')
|
||||
async installPackage(req: NodeRequest.Post) {
|
||||
const { name, verify, version } = req.body;
|
||||
|
||||
return await this.communityPackagesLifecycle.install({ name, verify, version }, req.user, 'ui');
|
||||
async installPackage(
|
||||
req: AuthenticatedRequest,
|
||||
_res: unknown,
|
||||
@Body body: CommunityPackageRequestDto,
|
||||
) {
|
||||
return await this.communityPackagesLifecycle.install(body, req.user, 'ui');
|
||||
}
|
||||
|
||||
@Get('/')
|
||||
|
|
@ -32,13 +36,11 @@ export class CommunityPackagesController {
|
|||
|
||||
@Patch('/')
|
||||
@GlobalScope('communityPackage:update')
|
||||
async updatePackage(req: NodeRequest.Update) {
|
||||
const { name, version, checksum } = req.body;
|
||||
|
||||
return await this.communityPackagesLifecycle.update(
|
||||
{ name, version, checksum },
|
||||
req.user,
|
||||
'badRequest',
|
||||
);
|
||||
async updatePackage(
|
||||
req: AuthenticatedRequest,
|
||||
_res: unknown,
|
||||
@Body body: CommunityPackageRequestDto,
|
||||
) {
|
||||
return await this.communityPackagesLifecycle.update(body, req.user, 'badRequest');
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ import { IncompatibleNodesApiVersionError } from '@/errors/response-errors/incom
|
|||
import type { UserLike } from '@/events/maps/relay.event-map';
|
||||
import { Push } from '@/push';
|
||||
|
||||
import { selectVettedVersion } from './community-node-types-utils';
|
||||
import { CommunityNodeTypesService } from './community-node-types.service';
|
||||
import { CommunityPackagesConfig } from './community-packages.config';
|
||||
import { CommunityPackagesService, isValidVersionSpecifier } from './community-packages.service';
|
||||
|
|
@ -63,34 +64,31 @@ export class CommunityPackagesLifecycleService {
|
|||
}
|
||||
|
||||
/**
|
||||
* Version and checksum the registry vetted for this package, in one lookup.
|
||||
* The pair must come from the same catalog read: two separate reads can
|
||||
* straddle a catalog refresh and pair a pinned version with the checksum of
|
||||
* a newer release, failing a legitimate install. No requested version means
|
||||
* the latest vetted one.
|
||||
* Resolves the version and checksum to install. In verified-only mode both come from the
|
||||
* vetted list. The checksum is what lets a package through `checkInstallPermissions`, so
|
||||
* only the instance config decides this, never the caller.
|
||||
*/
|
||||
private async resolveVetted(
|
||||
name: string,
|
||||
version?: string,
|
||||
): Promise<{ version: string; checksum: string }> {
|
||||
requestedVersion: string | undefined,
|
||||
): Promise<{ version: string | undefined; checksum: string | undefined }> {
|
||||
if (this.communityPackagesConfig.unverifiedEnabled) {
|
||||
return { version: requestedVersion, checksum: undefined };
|
||||
}
|
||||
|
||||
const vettedPackage = await this.communityNodeTypesService.findVetted(name);
|
||||
if (!vettedPackage) {
|
||||
throw new BadRequestError(`Package ${name} is not vetted for installation`);
|
||||
}
|
||||
|
||||
const resolvedVersion = version ?? vettedPackage.npmVersion;
|
||||
const checksum =
|
||||
resolvedVersion === vettedPackage.npmVersion
|
||||
? vettedPackage.checksum
|
||||
: vettedPackage.nodeVersions?.find((v) => v.npmVersion === resolvedVersion)?.checksum;
|
||||
|
||||
const { version, checksum } = selectVettedVersion(vettedPackage, requestedVersion);
|
||||
if (!checksum) {
|
||||
throw new BadRequestError(
|
||||
`Version ${resolvedVersion} of ${name} is not verified by n8n. Latest verified version is ${vettedPackage.npmVersion}`,
|
||||
`Version ${version} of ${name} is not verified by n8n. Latest verified version is ${vettedPackage.npmVersion}`,
|
||||
);
|
||||
}
|
||||
|
||||
return { version: resolvedVersion, checksum };
|
||||
return { version, checksum };
|
||||
}
|
||||
|
||||
async listInstalledPackages(): Promise<PublicInstalledPackage[] | InstalledPackages[]> {
|
||||
|
|
@ -130,29 +128,19 @@ export class CommunityPackagesLifecycleService {
|
|||
}
|
||||
|
||||
async install(
|
||||
args: { name: string | undefined; version?: string; verify?: boolean },
|
||||
args: { name: string | undefined; version?: string },
|
||||
user: UserLike,
|
||||
presentation: CommunityPackageInstallPresentation,
|
||||
): Promise<InstalledPackages> {
|
||||
this.assertNotManagedByEnv();
|
||||
const { name, verify, version } = args;
|
||||
const { name } = args;
|
||||
|
||||
if (!name) {
|
||||
throw new BadRequestError(PACKAGE_NAME_NOT_PROVIDED);
|
||||
}
|
||||
|
||||
if (version && !isValidVersionSpecifier(version)) {
|
||||
throw new BadRequestError(`Invalid version: ${version}`);
|
||||
}
|
||||
|
||||
let checksum: string | undefined;
|
||||
let resolvedVersion = version;
|
||||
|
||||
if (verify) {
|
||||
// Pins the vetted version too: without it, npm would install its own
|
||||
// `latest` while the checksum describes the registry's latest, and the
|
||||
// two can differ while a new release awaits vetting.
|
||||
({ version: resolvedVersion, checksum } = await this.resolveVetted(name, version));
|
||||
if (args.version && !isValidVersionSpecifier(args.version)) {
|
||||
throw new BadRequestError(`Invalid version: ${args.version}`);
|
||||
}
|
||||
|
||||
let parsed: CommunityPackages.ParsedPackageName;
|
||||
|
|
@ -165,6 +153,12 @@ export class CommunityPackagesLifecycleService {
|
|||
);
|
||||
}
|
||||
|
||||
// The vetted list is keyed by bare package name; `name` may carry a `@version` suffix.
|
||||
const { version: packageVersion, checksum } = await this.resolveVetted(
|
||||
parsed.packageName,
|
||||
args.version ?? parsed.version,
|
||||
);
|
||||
|
||||
if (parsed.packageName === STARTER_TEMPLATE_NAME) {
|
||||
const templateMessage =
|
||||
presentation === 'ui'
|
||||
|
|
@ -197,7 +191,6 @@ export class CommunityPackagesLifecycleService {
|
|||
throw new BadRequestError(`Package "${name}" is banned so it cannot be installed`);
|
||||
}
|
||||
|
||||
const packageVersion = resolvedVersion ?? parsed.version;
|
||||
let installedPackage: InstalledPackages;
|
||||
|
||||
try {
|
||||
|
|
@ -256,29 +249,39 @@ export class CommunityPackagesLifecycleService {
|
|||
}
|
||||
|
||||
async update(
|
||||
args: { name: string | undefined; version?: string; checksum?: string; verify?: boolean },
|
||||
args: { name: string | undefined; version?: string },
|
||||
user: UserLike,
|
||||
whenMissing: MissingInstalledPackageBehavior,
|
||||
): Promise<InstalledPackages> {
|
||||
this.assertNotManagedByEnv();
|
||||
const { name, version, verify } = args;
|
||||
|
||||
let checksum = args.checksum;
|
||||
|
||||
if (verify) {
|
||||
({ checksum } = await this.resolveVetted(name ?? '', version));
|
||||
}
|
||||
const { name } = args;
|
||||
|
||||
if (!name) {
|
||||
throw new BadRequestError(PACKAGE_NAME_NOT_PROVIDED);
|
||||
}
|
||||
|
||||
if (version && !isValidVersionSpecifier(version)) {
|
||||
throw new BadRequestError(`Invalid version: ${version}`);
|
||||
if (args.version && !isValidVersionSpecifier(args.version)) {
|
||||
throw new BadRequestError(`Invalid version: ${args.version}`);
|
||||
}
|
||||
|
||||
let parsed: CommunityPackages.ParsedPackageName;
|
||||
|
||||
try {
|
||||
parsed = this.communityPackagesService.parseNpmPackageName(name);
|
||||
} catch (error) {
|
||||
throw new BadRequestError(
|
||||
error instanceof Error ? error.message : 'Failed to parse package name',
|
||||
);
|
||||
}
|
||||
|
||||
const { packageName } = parsed;
|
||||
const { version, checksum } = await this.resolveVetted(
|
||||
packageName,
|
||||
args.version ?? parsed.version,
|
||||
);
|
||||
|
||||
const previouslyInstalledPackage =
|
||||
await this.communityPackagesService.findInstalledPackage(name);
|
||||
await this.communityPackagesService.findInstalledPackage(packageName);
|
||||
|
||||
if (!previouslyInstalledPackage) {
|
||||
if (whenMissing === 'notFound') {
|
||||
|
|
@ -289,7 +292,7 @@ export class CommunityPackagesLifecycleService {
|
|||
|
||||
try {
|
||||
const newInstalledPackage = await this.communityPackagesService.updatePackage(
|
||||
this.communityPackagesService.parseNpmPackageName(name).packageName,
|
||||
packageName,
|
||||
previouslyInstalledPackage,
|
||||
version,
|
||||
checksum,
|
||||
|
|
@ -317,7 +320,7 @@ export class CommunityPackagesLifecycleService {
|
|||
|
||||
this.eventService.emit('community-package-updated', {
|
||||
user,
|
||||
packageName: name,
|
||||
packageName,
|
||||
packageVersionCurrent: previouslyInstalledPackage.installedVersion,
|
||||
packageVersionNew: newInstalledPackage.installedVersion,
|
||||
packageNodeNames: newInstalledPackage.installedNodes.map((n) => n.name),
|
||||
|
|
|
|||
|
|
@ -1,21 +1,23 @@
|
|||
import { mockInstance } from '@n8n/backend-test-utils';
|
||||
import { ExecutionRepository, User } from '@n8n/db';
|
||||
import { createEmptyRunExecutionData } from 'n8n-workflow';
|
||||
import { createEmptyRunExecutionData, type IRunExecutionData } from 'n8n-workflow';
|
||||
import type { Mock } from 'vitest';
|
||||
|
||||
import { ExecutionPersistence } from '@/executions/execution-persistence';
|
||||
import { Telemetry } from '@/telemetry';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
|
||||
import { USER_CALLED_MCP_TOOL_EVENT } from '../mcp.constants';
|
||||
import { createGetExecutionTool } from '../tools/get-execution.tool';
|
||||
|
||||
import { ExecutionPersistence } from '@/executions/execution-persistence';
|
||||
import { ExecutionRedactionServiceProxy } from '@/executions/execution-redaction-proxy.service';
|
||||
import { Telemetry } from '@/telemetry';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
|
||||
describe('get-execution MCP tool', () => {
|
||||
const user = Object.assign(new User(), { id: 'user-1' });
|
||||
let executionRepository: ExecutionRepository;
|
||||
let executionPersistence: ExecutionPersistence;
|
||||
let workflowFinderService: WorkflowFinderService;
|
||||
let telemetry: Telemetry;
|
||||
let executionRedactionServiceProxy: ExecutionRedactionServiceProxy;
|
||||
|
||||
const mockWorkflow = {
|
||||
id: 'workflow-1',
|
||||
|
|
@ -31,6 +33,7 @@ describe('get-execution MCP tool', () => {
|
|||
telemetry = mockInstance(Telemetry, {
|
||||
track: vi.fn(),
|
||||
});
|
||||
executionRedactionServiceProxy = mockInstance(ExecutionRedactionServiceProxy);
|
||||
});
|
||||
|
||||
describe('smoke tests', () => {
|
||||
|
|
@ -40,6 +43,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
expect(tool.name).toBe('get_workflow_execution');
|
||||
|
|
@ -60,6 +64,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
(workflowFinderService.findWorkflowForUser as Mock).mockResolvedValue(mockWorkflow);
|
||||
|
|
@ -92,6 +97,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const mockExecution = {
|
||||
|
|
@ -137,6 +143,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const mockExecutionData = createEmptyRunExecutionData();
|
||||
|
|
@ -210,6 +217,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const mockExecutionData = createEmptyRunExecutionData();
|
||||
|
|
@ -282,6 +290,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const mockExecutionData = createEmptyRunExecutionData();
|
||||
|
|
@ -337,6 +346,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const mockExecutionData = createEmptyRunExecutionData();
|
||||
|
|
@ -402,6 +412,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const mockExecutionData = createEmptyRunExecutionData();
|
||||
|
|
@ -456,6 +467,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const mockExecutionData = createEmptyRunExecutionData();
|
||||
|
|
@ -514,6 +526,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const mockExecutionData = createEmptyRunExecutionData();
|
||||
|
|
@ -582,6 +595,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const mockExecution = {
|
||||
|
|
@ -623,6 +637,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const mockExecution = {
|
||||
|
|
@ -663,6 +678,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const mockExecution = {
|
||||
|
|
@ -706,6 +722,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const mockExecutionData = createEmptyRunExecutionData();
|
||||
|
|
@ -768,6 +785,7 @@ describe('get-execution MCP tool', () => {
|
|||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const waitTillDate = new Date('2025-01-01T12:00:00.000Z');
|
||||
|
|
@ -806,5 +824,206 @@ describe('get-execution MCP tool', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('redaction', () => {
|
||||
const mockExecution = () => ({
|
||||
id: 'execution-1',
|
||||
workflowId: 'workflow-1',
|
||||
mode: 'manual' as const,
|
||||
status: 'success' as const,
|
||||
startedAt: new Date('2025-01-01T00:00:00.000Z'),
|
||||
stoppedAt: new Date('2025-01-01T00:01:00.000Z'),
|
||||
retryOf: null,
|
||||
retrySuccessId: null,
|
||||
waitTill: null,
|
||||
data: {
|
||||
...createEmptyRunExecutionData(),
|
||||
resultData: {
|
||||
runData: {
|
||||
'HTTP Request': [
|
||||
{
|
||||
startTime: 1,
|
||||
executionIndex: 0,
|
||||
executionTime: 1,
|
||||
source: [],
|
||||
data: { main: [[{ json: { secret: 'leaked' } }]] },
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
} as IRunExecutionData,
|
||||
});
|
||||
|
||||
test('passes the execution through redaction when includeData is true', async () => {
|
||||
const tool = createGetExecutionTool(
|
||||
user,
|
||||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const execution = mockExecution();
|
||||
|
||||
(workflowFinderService.findWorkflowForUser as Mock).mockResolvedValue(mockWorkflow);
|
||||
(executionPersistence.findWithUnflattenedData as Mock).mockResolvedValue(execution);
|
||||
|
||||
await tool.handler(
|
||||
{
|
||||
workflowId: 'workflow-1',
|
||||
executionId: 'execution-1',
|
||||
includeData: true,
|
||||
nodeNames: undefined,
|
||||
truncateData: undefined,
|
||||
},
|
||||
{} as never,
|
||||
);
|
||||
|
||||
expect(executionRedactionServiceProxy.processExecution).toHaveBeenCalledTimes(1);
|
||||
expect(executionRedactionServiceProxy.processExecution).toHaveBeenCalledWith(execution, {
|
||||
user,
|
||||
});
|
||||
});
|
||||
|
||||
test('does not pass redactExecutionData, so the workflow policy applies', async () => {
|
||||
const tool = createGetExecutionTool(
|
||||
user,
|
||||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
(workflowFinderService.findWorkflowForUser as Mock).mockResolvedValue(mockWorkflow);
|
||||
(executionPersistence.findWithUnflattenedData as Mock).mockResolvedValue(mockExecution());
|
||||
|
||||
await tool.handler(
|
||||
{
|
||||
workflowId: 'workflow-1',
|
||||
executionId: 'execution-1',
|
||||
includeData: true,
|
||||
nodeNames: undefined,
|
||||
truncateData: undefined,
|
||||
},
|
||||
{} as never,
|
||||
);
|
||||
|
||||
// `true` would redact regardless of policy and `false` is the reveal path,
|
||||
// which the MCP surface must never take.
|
||||
const [, options] = (executionRedactionServiceProxy.processExecution as Mock).mock.calls[0];
|
||||
expect(options).not.toHaveProperty('redactExecutionData');
|
||||
});
|
||||
|
||||
test('returns the redacted data', async () => {
|
||||
const tool = createGetExecutionTool(
|
||||
user,
|
||||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
const execution = mockExecution();
|
||||
|
||||
(workflowFinderService.findWorkflowForUser as Mock).mockResolvedValue(mockWorkflow);
|
||||
(executionPersistence.findWithUnflattenedData as Mock).mockResolvedValue(execution);
|
||||
// The real service redacts in place, so mirror that here.
|
||||
(executionRedactionServiceProxy.processExecution as Mock).mockImplementation(
|
||||
async (target: typeof execution) => {
|
||||
target.data.resultData.runData = {
|
||||
'HTTP Request': [
|
||||
{
|
||||
startTime: 1,
|
||||
executionIndex: 0,
|
||||
executionTime: 1,
|
||||
source: [],
|
||||
data: {
|
||||
main: [
|
||||
[
|
||||
{
|
||||
json: {},
|
||||
redaction: { redacted: true, reason: 'workflow_redaction_policy' },
|
||||
},
|
||||
],
|
||||
],
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
target.data.redactionInfo = {
|
||||
isRedacted: true,
|
||||
reason: 'workflow_redaction_policy',
|
||||
canReveal: false,
|
||||
};
|
||||
return target;
|
||||
},
|
||||
);
|
||||
|
||||
const result = await tool.handler(
|
||||
{
|
||||
workflowId: 'workflow-1',
|
||||
executionId: 'execution-1',
|
||||
includeData: true,
|
||||
nodeNames: undefined,
|
||||
truncateData: undefined,
|
||||
},
|
||||
{} as never,
|
||||
);
|
||||
|
||||
expect(result.structuredContent).toMatchObject({
|
||||
data: {
|
||||
resultData: {
|
||||
runData: {
|
||||
'HTTP Request': [
|
||||
{
|
||||
data: {
|
||||
main: [
|
||||
[
|
||||
{
|
||||
json: {},
|
||||
redaction: { redacted: true, reason: 'workflow_redaction_policy' },
|
||||
},
|
||||
],
|
||||
],
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
redactionInfo: {
|
||||
isRedacted: true,
|
||||
reason: 'workflow_redaction_policy',
|
||||
canReveal: false,
|
||||
},
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
test('skips redaction when includeData is false', async () => {
|
||||
const tool = createGetExecutionTool(
|
||||
user,
|
||||
executionRepository,
|
||||
workflowFinderService,
|
||||
telemetry,
|
||||
executionRedactionServiceProxy,
|
||||
);
|
||||
|
||||
(workflowFinderService.findWorkflowForUser as Mock).mockResolvedValue(mockWorkflow);
|
||||
(executionRepository.findIfAccessible as Mock).mockResolvedValue(mockExecution());
|
||||
|
||||
await tool.handler(
|
||||
{
|
||||
workflowId: 'workflow-1',
|
||||
executionId: 'execution-1',
|
||||
includeData: false,
|
||||
nodeNames: undefined,
|
||||
truncateData: undefined,
|
||||
},
|
||||
{} as never,
|
||||
);
|
||||
|
||||
expect(executionRedactionServiceProxy.processExecution).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ import { CollaborationService } from '@/collaboration/collaboration.service';
|
|||
import { CredentialsService } from '@/credentials/credentials.service';
|
||||
import { ExecutionService } from '@/executions/execution.service';
|
||||
import { ExecutionListService } from '@/executions/execution-list.service';
|
||||
import { SubworkflowPolicyChecker } from '@/executions/pre-execution-checks/subworkflow-policy-checker';
|
||||
import { ExecutionRedactionServiceProxy } from '@/executions/execution-redaction-proxy.service';
|
||||
import { DataTableProxyService } from '@/modules/data-table/data-table-proxy.service';
|
||||
import { NodeCatalogService } from '@/node-catalog';
|
||||
import { NodeTypes } from '@/node-types';
|
||||
|
|
@ -26,6 +26,7 @@ import { TagService } from '@/services/tag.service';
|
|||
import { Telemetry } from '@/telemetry';
|
||||
import { WorkflowRunner } from '@/workflow-runner';
|
||||
import { WorkflowCreationService } from '@/workflows/workflow-creation.service';
|
||||
import { ErrorWorkflowValidationService } from '@/workflows/error-workflow-validation.service';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
import { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service';
|
||||
import { WorkflowPublishedDataService } from '@/workflows/workflow-published-data.service';
|
||||
|
|
@ -135,7 +136,7 @@ describe('install_community_node registration', () => {
|
|||
mockInstance(WorkflowHistoryService),
|
||||
mockInstance(WorkflowsConfig),
|
||||
mockInstance(WorkflowPublishedDataService),
|
||||
mockInstance(SubworkflowPolicyChecker),
|
||||
mockInstance(ErrorWorkflowValidationService),
|
||||
mockInstance(AiGatewayService, {
|
||||
isAvailable: vi.fn().mockResolvedValue({ available: false }),
|
||||
}),
|
||||
|
|
@ -147,6 +148,7 @@ describe('install_community_node registration', () => {
|
|||
mockInstance(FolderService),
|
||||
mockInstance(AiPreferenceService),
|
||||
mockInstance(McpConfig, { communityNodeDiscoveryEnabled: true }),
|
||||
mockInstance(ExecutionRedactionServiceProxy),
|
||||
);
|
||||
|
||||
beforeEach(() => {
|
||||
|
|
|
|||
|
|
@ -98,11 +98,7 @@ describe('install_community_node MCP tool', () => {
|
|||
test('installs the package the node type belongs to', async () => {
|
||||
const structured = await call();
|
||||
|
||||
expect(lifecycleService.install).toHaveBeenCalledWith(
|
||||
{ name: PACKAGE, verify: true },
|
||||
user,
|
||||
'mcp',
|
||||
);
|
||||
expect(lifecycleService.install).toHaveBeenCalledWith({ name: PACKAGE }, user, 'mcp');
|
||||
expect(structured).toMatchObject({
|
||||
installed: true,
|
||||
packageName: PACKAGE,
|
||||
|
|
@ -116,15 +112,11 @@ describe('install_community_node MCP tool', () => {
|
|||
expect(structured.nodeTypes).toEqual([NODE_TYPE, `${PACKAGE}.firecrawlTool`]);
|
||||
});
|
||||
|
||||
test('delegates version pinning to install(), which always verifies the checksum', async () => {
|
||||
// No version is passed on purpose: install() resolves the latest vetted
|
||||
// version and its checksum from one catalog lookup, so the pair can
|
||||
// never straddle a catalog refresh.
|
||||
test('delegates version and verification policy to install()', async () => {
|
||||
await call();
|
||||
|
||||
const [args] = lifecycleService.install.mock.calls[0];
|
||||
expect(args.version).toBeUndefined();
|
||||
expect(args.verify).toBe(true);
|
||||
expect(args).toEqual({ name: PACKAGE });
|
||||
|
||||
expect(communityNodeTypesService.findVetted).not.toHaveBeenCalled();
|
||||
});
|
||||
|
|
@ -299,11 +291,7 @@ describe('install_community_node MCP tool', () => {
|
|||
|
||||
await call(DOTTED_NODE);
|
||||
|
||||
expect(lifecycleService.install).toHaveBeenCalledWith(
|
||||
{ name: DOTTED_PACKAGE, verify: true },
|
||||
user,
|
||||
'mcp',
|
||||
);
|
||||
expect(lifecycleService.install).toHaveBeenCalledWith({ name: DOTTED_PACKAGE }, user, 'mcp');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -30,8 +30,8 @@ import { ActiveExecutions } from '@/active-executions';
|
|||
import { CollaborationService } from '@/collaboration/collaboration.service';
|
||||
import { CredentialsService } from '@/credentials/credentials.service';
|
||||
import { ExecutionListService } from '@/executions/execution-list.service';
|
||||
import { ExecutionRedactionServiceProxy } from '@/executions/execution-redaction-proxy.service';
|
||||
import { ExecutionService } from '@/executions/execution.service';
|
||||
import { SubworkflowPolicyChecker } from '@/executions/pre-execution-checks/subworkflow-policy-checker';
|
||||
import { DataTableProxyService } from '@/modules/data-table/data-table-proxy.service';
|
||||
import { InstanceContextService } from '@/modules/instance-ai/instance-context.service';
|
||||
import {
|
||||
|
|
@ -56,6 +56,7 @@ import { WorkflowRunner } from '@/workflow-runner';
|
|||
import { WorkflowCreationService } from '@/workflows/workflow-creation.service';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
import { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service';
|
||||
import { ErrorWorkflowValidationService } from '@/workflows/error-workflow-validation.service';
|
||||
import { WorkflowPublishedDataService } from '@/workflows/workflow-published-data.service';
|
||||
import { WorkflowService } from '@/workflows/workflow.service';
|
||||
|
||||
|
|
@ -203,7 +204,7 @@ describe('McpService scope enforcement', () => {
|
|||
mockInstance(WorkflowHistoryService),
|
||||
mockInstance(WorkflowsConfig),
|
||||
mockInstance(WorkflowPublishedDataService),
|
||||
mockInstance(SubworkflowPolicyChecker),
|
||||
mockInstance(ErrorWorkflowValidationService),
|
||||
mockInstance(AiGatewayService, {
|
||||
isAvailable: vi.fn().mockResolvedValue({ available: false }),
|
||||
}),
|
||||
|
|
@ -217,6 +218,7 @@ describe('McpService scope enforcement', () => {
|
|||
mockInstance(FolderService),
|
||||
mockInstance(AiPreferenceService),
|
||||
mockInstance(McpConfig),
|
||||
mockInstance(ExecutionRedactionServiceProxy),
|
||||
);
|
||||
|
||||
beforeEach(() => {
|
||||
|
|
|
|||
|
|
@ -30,8 +30,8 @@ import { ActiveExecutions } from '@/active-executions';
|
|||
import { CollaborationService } from '@/collaboration/collaboration.service';
|
||||
import { CredentialsService } from '@/credentials/credentials.service';
|
||||
import { ExecutionListService } from '@/executions/execution-list.service';
|
||||
import { ExecutionRedactionServiceProxy } from '@/executions/execution-redaction-proxy.service';
|
||||
import { ExecutionService } from '@/executions/execution.service';
|
||||
import { SubworkflowPolicyChecker } from '@/executions/pre-execution-checks/subworkflow-policy-checker';
|
||||
import { DataTableProxyService } from '@/modules/data-table/data-table-proxy.service';
|
||||
import { NodeCatalogService } from '@/node-catalog';
|
||||
import { NodeTypes } from '@/node-types';
|
||||
|
|
@ -49,6 +49,7 @@ import { WorkflowRunner } from '@/workflow-runner';
|
|||
import { WorkflowCreationService } from '@/workflows/workflow-creation.service';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
import { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service';
|
||||
import { ErrorWorkflowValidationService } from '@/workflows/error-workflow-validation.service';
|
||||
import { WorkflowPublishedDataService } from '@/workflows/workflow-published-data.service';
|
||||
import { WorkflowService } from '@/workflows/workflow.service';
|
||||
|
||||
|
|
@ -134,7 +135,7 @@ describe('McpService', () => {
|
|||
mockInstance(WorkflowHistoryService),
|
||||
mockInstance(WorkflowsConfig),
|
||||
mockInstance(WorkflowPublishedDataService),
|
||||
mockInstance(SubworkflowPolicyChecker),
|
||||
mockInstance(ErrorWorkflowValidationService),
|
||||
mockAiGatewayService(),
|
||||
mockInstance(McpPostSaveMetricsService),
|
||||
mockInstance(ModuleRegistry),
|
||||
|
|
@ -142,6 +143,7 @@ describe('McpService', () => {
|
|||
mockInstance(FolderService),
|
||||
aiPreferenceService,
|
||||
mockInstance(McpConfig),
|
||||
mockInstance(ExecutionRedactionServiceProxy),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -190,7 +192,7 @@ describe('McpService', () => {
|
|||
mockInstance(WorkflowHistoryService),
|
||||
mockInstance(WorkflowsConfig),
|
||||
mockInstance(WorkflowPublishedDataService),
|
||||
mockInstance(SubworkflowPolicyChecker),
|
||||
mockInstance(ErrorWorkflowValidationService),
|
||||
mockAiGatewayService(),
|
||||
mockInstance(McpPostSaveMetricsService),
|
||||
mockInstance(ModuleRegistry),
|
||||
|
|
@ -198,6 +200,7 @@ describe('McpService', () => {
|
|||
mockInstance(FolderService),
|
||||
mockInstance(AiPreferenceService),
|
||||
mockInstance(McpConfig),
|
||||
mockInstance(ExecutionRedactionServiceProxy),
|
||||
);
|
||||
|
||||
expect(queueMcpService.isQueueMode).toBe(true);
|
||||
|
|
@ -400,7 +403,7 @@ describe('McpService', () => {
|
|||
mockInstance(WorkflowHistoryService),
|
||||
mockInstance(WorkflowsConfig),
|
||||
mockInstance(WorkflowPublishedDataService),
|
||||
mockInstance(SubworkflowPolicyChecker),
|
||||
mockInstance(ErrorWorkflowValidationService),
|
||||
mockAiGatewayService(),
|
||||
mockInstance(McpPostSaveMetricsService),
|
||||
mockInstance(ModuleRegistry),
|
||||
|
|
@ -408,6 +411,7 @@ describe('McpService', () => {
|
|||
mockInstance(FolderService),
|
||||
mockInstance(AiPreferenceService),
|
||||
mockInstance(McpConfig),
|
||||
mockInstance(ExecutionRedactionServiceProxy),
|
||||
);
|
||||
|
||||
const user = Object.assign(new User(), { id: 'user-1', role: GLOBAL_MEMBER_ROLE });
|
||||
|
|
@ -1202,7 +1206,7 @@ describe('McpService', () => {
|
|||
mockInstance(WorkflowHistoryService),
|
||||
mockInstance(WorkflowsConfig),
|
||||
mockInstance(WorkflowPublishedDataService),
|
||||
mockInstance(SubworkflowPolicyChecker),
|
||||
mockInstance(ErrorWorkflowValidationService),
|
||||
mockAiGatewayService(),
|
||||
mockInstance(McpPostSaveMetricsService),
|
||||
mockInstance(ModuleRegistry),
|
||||
|
|
@ -1210,6 +1214,7 @@ describe('McpService', () => {
|
|||
mockInstance(FolderService),
|
||||
mockInstance(AiPreferenceService),
|
||||
mockInstance(McpConfig),
|
||||
mockInstance(ExecutionRedactionServiceProxy),
|
||||
);
|
||||
|
||||
const server = await service.getServer(user, mcpFeatureFlags());
|
||||
|
|
@ -1260,7 +1265,7 @@ describe('McpService', () => {
|
|||
mockInstance(WorkflowHistoryService),
|
||||
mockInstance(WorkflowsConfig),
|
||||
mockInstance(WorkflowPublishedDataService),
|
||||
mockInstance(SubworkflowPolicyChecker),
|
||||
mockInstance(ErrorWorkflowValidationService),
|
||||
mockAiGatewayService(),
|
||||
mockInstance(McpPostSaveMetricsService),
|
||||
mockInstance(ModuleRegistry),
|
||||
|
|
@ -1268,6 +1273,7 @@ describe('McpService', () => {
|
|||
mockInstance(FolderService),
|
||||
mockInstance(AiPreferenceService),
|
||||
mockInstance(McpConfig),
|
||||
mockInstance(ExecutionRedactionServiceProxy),
|
||||
);
|
||||
|
||||
const server = await service.getServer(user, mcpFeatureFlags());
|
||||
|
|
@ -1343,7 +1349,7 @@ describe('McpService', () => {
|
|||
mockInstance(WorkflowHistoryService),
|
||||
mockInstance(WorkflowsConfig),
|
||||
mockInstance(WorkflowPublishedDataService),
|
||||
mockInstance(SubworkflowPolicyChecker),
|
||||
mockInstance(ErrorWorkflowValidationService),
|
||||
mockAiGatewayService(),
|
||||
mockInstance(McpPostSaveMetricsService),
|
||||
mockInstance(ModuleRegistry),
|
||||
|
|
@ -1351,6 +1357,7 @@ describe('McpService', () => {
|
|||
mockInstance(FolderService),
|
||||
mockInstance(AiPreferenceService),
|
||||
mockInstance(McpConfig),
|
||||
mockInstance(ExecutionRedactionServiceProxy),
|
||||
);
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ import { TagService } from '@/services/tag.service';
|
|||
import { UrlService } from '@n8n/backend-services';
|
||||
import { Telemetry } from '@/telemetry';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
import { ErrorWorkflowValidationService } from '@/workflows/error-workflow-validation.service';
|
||||
import { WorkflowPublishedDataService } from '@/workflows/workflow-published-data.service';
|
||||
import { WorkflowService } from '@/workflows/workflow.service';
|
||||
|
||||
|
|
@ -201,6 +202,20 @@ describe('update-workflow MCP tool', () => {
|
|||
incrementPostSaveFailure: vi.fn(),
|
||||
});
|
||||
|
||||
/**
|
||||
* Built per tool, not once in `beforeEach`: a test may swap `globalConfig`
|
||||
* (custom error-trigger type) before building its tool, and the service reads
|
||||
* the config it was constructed with.
|
||||
*/
|
||||
const buildErrorWorkflowValidationService = () =>
|
||||
new ErrorWorkflowValidationService(
|
||||
globalConfig,
|
||||
nodeTypes,
|
||||
workflowFinderService,
|
||||
workflowPublishedDataService,
|
||||
subworkflowPolicyChecker,
|
||||
);
|
||||
|
||||
const createTool = () =>
|
||||
createUpdateWorkflowTool(
|
||||
user,
|
||||
|
|
@ -215,8 +230,7 @@ describe('update-workflow MCP tool', () => {
|
|||
dataTableOps as never,
|
||||
tagService,
|
||||
globalConfig,
|
||||
subworkflowPolicyChecker,
|
||||
workflowPublishedDataService,
|
||||
buildErrorWorkflowValidationService(),
|
||||
aiGatewayService,
|
||||
{},
|
||||
logger,
|
||||
|
|
@ -2347,6 +2361,26 @@ describe('update-workflow MCP tool', () => {
|
|||
expect(saved.settings).toEqual(expect.objectContaining({ errorWorkflow: 'DEFAULT' }));
|
||||
});
|
||||
|
||||
// Nothing evaluates `settings.errorWorkflow`; `executeErrorWorkflow` uses it
|
||||
// as a literal workflow id. Saving an expression would look like it worked
|
||||
// and then silently never run a handler.
|
||||
test('rejects an expression instead of saving a reference that never resolves', async () => {
|
||||
const result = await callHandler({
|
||||
workflowId: 'wf-1',
|
||||
operations: [
|
||||
{
|
||||
type: 'setWorkflowSettings',
|
||||
settings: { errorWorkflow: '={{ $json.handlerId }}' },
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
const response = parseResult(result);
|
||||
expect(result.isError).toBe(true);
|
||||
expect(response.error).toContain('does not accept expressions');
|
||||
expect(workflowService.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('does not attach settings for node-only edits', async () => {
|
||||
await callHandler({
|
||||
workflowId: 'wf-1',
|
||||
|
|
@ -2525,8 +2559,7 @@ describe('update-workflow MCP tool', () => {
|
|||
dataTableOps as never,
|
||||
tagService,
|
||||
globalConfig,
|
||||
subworkflowPolicyChecker,
|
||||
workflowPublishedDataService,
|
||||
buildErrorWorkflowValidationService(),
|
||||
aiGatewayService,
|
||||
{},
|
||||
logger,
|
||||
|
|
@ -3887,8 +3920,7 @@ describe('update-workflow MCP tool', () => {
|
|||
dataTableOps as never,
|
||||
tagService,
|
||||
globalConfig,
|
||||
subworkflowPolicyChecker,
|
||||
workflowPublishedDataService,
|
||||
buildErrorWorkflowValidationService(),
|
||||
aiGatewayService,
|
||||
{},
|
||||
logger,
|
||||
|
|
@ -3927,8 +3959,7 @@ describe('update-workflow MCP tool', () => {
|
|||
dataTableOps as never,
|
||||
tagService,
|
||||
globalConfig,
|
||||
subworkflowPolicyChecker,
|
||||
workflowPublishedDataService,
|
||||
buildErrorWorkflowValidationService(),
|
||||
aiGatewayService,
|
||||
{},
|
||||
logger,
|
||||
|
|
@ -3967,8 +3998,7 @@ describe('update-workflow MCP tool', () => {
|
|||
dataTableOps as never,
|
||||
tagService,
|
||||
globalConfig,
|
||||
subworkflowPolicyChecker,
|
||||
workflowPublishedDataService,
|
||||
buildErrorWorkflowValidationService(),
|
||||
aiGatewayService,
|
||||
{},
|
||||
logger,
|
||||
|
|
|
|||
|
|
@ -28,8 +28,8 @@ import { CollaborationService } from '@/collaboration/collaboration.service';
|
|||
import { N8N_VERSION } from '@/constants';
|
||||
import { CredentialsService } from '@/credentials/credentials.service';
|
||||
import { ExecutionListService } from '@/executions/execution-list.service';
|
||||
import { ExecutionRedactionServiceProxy } from '@/executions/execution-redaction-proxy.service';
|
||||
import { ExecutionService } from '@/executions/execution.service';
|
||||
import { SubworkflowPolicyChecker } from '@/executions/pre-execution-checks/subworkflow-policy-checker';
|
||||
import { DataTableProxyService } from '@/modules/data-table/data-table-proxy.service';
|
||||
import { NodeCatalogService } from '@/node-catalog';
|
||||
import { NodeTypes } from '@/node-types';
|
||||
|
|
@ -44,6 +44,7 @@ import { RoleService } from '@/services/role.service';
|
|||
import { TagService } from '@/services/tag.service';
|
||||
import { Telemetry } from '@/telemetry';
|
||||
import { WorkflowRunner } from '@/workflow-runner';
|
||||
import { ErrorWorkflowValidationService } from '@/workflows/error-workflow-validation.service';
|
||||
import { WorkflowCreationService } from '@/workflows/workflow-creation.service';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
import { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service';
|
||||
|
|
@ -262,7 +263,7 @@ export class McpService {
|
|||
private readonly workflowHistoryService: WorkflowHistoryService,
|
||||
private readonly workflowsConfig: WorkflowsConfig,
|
||||
private readonly workflowPublishedDataService: WorkflowPublishedDataService,
|
||||
private readonly subworkflowPolicyChecker: SubworkflowPolicyChecker,
|
||||
private readonly errorWorkflowValidationService: ErrorWorkflowValidationService,
|
||||
private readonly aiGatewayService: AiGatewayService,
|
||||
private readonly postSaveMetrics: McpPostSaveMetricsService,
|
||||
private readonly moduleRegistry: ModuleRegistry,
|
||||
|
|
@ -270,6 +271,7 @@ export class McpService {
|
|||
private readonly folderService: FolderService,
|
||||
private readonly aiPreferenceService: AiPreferenceService,
|
||||
private readonly mcpConfig: McpConfig,
|
||||
private readonly executionRedactionServiceProxy: ExecutionRedactionServiceProxy,
|
||||
) {}
|
||||
|
||||
/** Resolves user experience flags and the shared activity gate. */
|
||||
|
|
@ -521,6 +523,7 @@ export class McpService {
|
|||
this.executionRepository,
|
||||
this.workflowFinderService,
|
||||
this.telemetry,
|
||||
this.executionRedactionServiceProxy,
|
||||
);
|
||||
registerIfAllowed(getExecutionTool);
|
||||
|
||||
|
|
@ -1055,8 +1058,7 @@ export class McpService {
|
|||
dataTableOps,
|
||||
this.tagService,
|
||||
this.globalConfig,
|
||||
this.subworkflowPolicyChecker,
|
||||
this.workflowPublishedDataService,
|
||||
this.errorWorkflowValidationService,
|
||||
this.aiGatewayService,
|
||||
uninstalledNodeOptions,
|
||||
this.logger,
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { type ExecutionRepository, type User } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import type { IRunExecutionData, IRunData, ITaskDataConnections, IPinData } from 'n8n-workflow';
|
||||
import { ensureError } from '@n8n/utils/errors/ensure-error';
|
||||
import type { IPinData, IRunData, IRunExecutionData, ITaskDataConnections } from 'n8n-workflow';
|
||||
import { jsonStringify, replaceCircularReferences } from 'n8n-workflow';
|
||||
import z from 'zod';
|
||||
|
||||
|
|
@ -11,6 +11,7 @@ import type { ToolDefinition, UserCalledMCPToolEventPayload } from '../mcp.types
|
|||
import { getMcpWorkflow } from './workflow-validation.utils';
|
||||
|
||||
import { ExecutionPersistence } from '@/executions/execution-persistence';
|
||||
import type { ExecutionRedactionServiceProxy } from '@/executions/execution-redaction-proxy.service';
|
||||
import type { Telemetry } from '@/telemetry';
|
||||
import type { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
|
||||
|
|
@ -67,6 +68,7 @@ export const createGetExecutionTool = (
|
|||
executionRepository: ExecutionRepository,
|
||||
workflowFinderService: WorkflowFinderService,
|
||||
telemetry: Telemetry,
|
||||
executionRedactionServiceProxy: ExecutionRedactionServiceProxy,
|
||||
): ToolDefinition<typeof inputSchema.shape> => ({
|
||||
name: 'get_workflow_execution',
|
||||
config: {
|
||||
|
|
@ -107,6 +109,12 @@ export const createGetExecutionTool = (
|
|||
executionId,
|
||||
[workflowId],
|
||||
);
|
||||
|
||||
if (fullExecution) {
|
||||
// Redacts in place, modifying fullExecution internally
|
||||
await executionRedactionServiceProxy.processExecution(fullExecution, { user });
|
||||
}
|
||||
|
||||
execution = fullExecution;
|
||||
executionData = fullExecution?.data ?? null;
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -200,11 +200,8 @@ export const createInstallCommunityNodeTool = (
|
|||
|
||||
let installedPackage: Awaited<ReturnType<CommunityPackagesLifecycleService['install']>>;
|
||||
try {
|
||||
// No version: install() pins the latest vetted version and its
|
||||
// checksum from a single catalog lookup, so the pair can never
|
||||
// straddle a catalog refresh.
|
||||
installedPackage = await communityPackagesLifecycleService.install(
|
||||
{ name: packageName, verify: true },
|
||||
{ name: packageName },
|
||||
user,
|
||||
'mcp',
|
||||
);
|
||||
|
|
|
|||
|
|
@ -4,13 +4,11 @@ import type { GlobalConfig } from '@n8n/config';
|
|||
import { type User, type SharedWorkflowRepository, WorkflowEntity } from '@n8n/db';
|
||||
import { hasGlobalScope } from '@n8n/permissions';
|
||||
import isEqual from 'lodash/isEqual';
|
||||
import { Workflow, type INode, type IWorkflowSettings } from 'n8n-workflow';
|
||||
import type { IWorkflowSettings } from 'n8n-workflow';
|
||||
import { z } from 'zod';
|
||||
|
||||
import type { CollaborationService } from '@/collaboration/collaboration.service';
|
||||
import type { CredentialsService } from '@/credentials/credentials.service';
|
||||
import { SubworkflowPolicyDenialError } from '@/errors/subworkflow-policy-denial.error';
|
||||
import type { SubworkflowPolicyChecker } from '@/executions/pre-execution-checks/subworkflow-policy-checker';
|
||||
import type { DataTableUserOperations } from '@/modules/data-table/data-table-proxy.service';
|
||||
import type { McpPostSaveMetricsService } from '@/modules/mcp/mcp-post-save-metrics.service';
|
||||
import type { NodeTypes } from '@/node-types';
|
||||
|
|
@ -24,8 +22,12 @@ import {
|
|||
removeDefaultValues,
|
||||
resolveNodeWebhookIds,
|
||||
} from '@/workflow-helpers';
|
||||
import {
|
||||
isExpressionErrorWorkflowId,
|
||||
staticErrorWorkflowId,
|
||||
type ErrorWorkflowValidationService,
|
||||
} from '@/workflows/error-workflow-validation.service';
|
||||
import type { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
import type { WorkflowPublishedDataService } from '@/workflows/workflow-published-data.service';
|
||||
import type { WorkflowService } from '@/workflows/workflow.service';
|
||||
|
||||
import { buildInvalidAiToolSourceErrorResponse } from './connection-structure-check';
|
||||
|
|
@ -366,119 +368,67 @@ type UpdateWorkflowOutput = z.infer<z.ZodObject<typeof outputSchema>>;
|
|||
|
||||
/**
|
||||
* Validates a freshly-set `errorWorkflow` reference. Throws a teaching-oriented
|
||||
* error when the target does not exist / is inaccessible, has no active Error
|
||||
* Trigger node, or cannot be called by this workflow due to its sub-workflow
|
||||
* caller policy — each of which would otherwise silently prevent the error
|
||||
* workflow from running on failure. A 'DEFAULT' / cleared value skips the check.
|
||||
* An archived target, or one that is not available in MCP, is rejected like in
|
||||
* the by-id MCP tools.
|
||||
* error when the value is an expression, or when the target does not exist / is
|
||||
* inaccessible, has no active Error Trigger node, or cannot be called by this
|
||||
* workflow due to its sub-workflow caller policy — each of which would otherwise
|
||||
* silently prevent the error workflow from running on failure. A 'DEFAULT' /
|
||||
* cleared value skips the check.
|
||||
*/
|
||||
async function assertErrorWorkflowIsUsable({
|
||||
errorWorkflowId,
|
||||
parentWorkflowId,
|
||||
user,
|
||||
workflowFinderService,
|
||||
workflowPublishedDataService,
|
||||
useWorkflowPublicationService,
|
||||
nodeTypes,
|
||||
subworkflowPolicyChecker,
|
||||
errorTriggerType,
|
||||
errorWorkflowValidationService,
|
||||
}: {
|
||||
errorWorkflowId: string | undefined;
|
||||
parentWorkflowId: string;
|
||||
user: User;
|
||||
workflowFinderService: WorkflowFinderService;
|
||||
workflowPublishedDataService: WorkflowPublishedDataService;
|
||||
useWorkflowPublicationService: boolean;
|
||||
nodeTypes: NodeTypes;
|
||||
subworkflowPolicyChecker: SubworkflowPolicyChecker;
|
||||
errorTriggerType: string;
|
||||
errorWorkflowValidationService: ErrorWorkflowValidationService;
|
||||
}): Promise<void> {
|
||||
if (!errorWorkflowId || errorWorkflowId === 'DEFAULT') {
|
||||
return;
|
||||
// The setting is never evaluated — an expression is used as a literal workflow
|
||||
// id, so the handler would never run. Caught before `staticErrorWorkflowId`,
|
||||
// which reports "no id to validate" for an expression and a cleared value alike.
|
||||
if (isExpressionErrorWorkflowId(errorWorkflowId)) {
|
||||
throw new Error(
|
||||
`errorWorkflow does not accept expressions, and '${errorWorkflowId}' would be stored as a literal workflow ID that never matches, so the error workflow would never run. Pass a plain workflow ID (find one with search_workflows), or "DEFAULT" to clear it.`,
|
||||
);
|
||||
}
|
||||
|
||||
// Read access is required intentionally, mirroring the editor UI (the error
|
||||
// workflow picker only lists workflows the user can read). Resolving the
|
||||
// target without an access check would let callers probe arbitrary workflow
|
||||
// IDs and learn their name / published / trigger / policy state from the
|
||||
// validation errors below. Runtime not requiring read access is separate: it
|
||||
// runs the error workflow under the owner project's context, gated by caller
|
||||
// policy, which is about execution — not about who may configure the link.
|
||||
const errorWorkflow = await workflowFinderService.findWorkflowForUser(
|
||||
errorWorkflowId,
|
||||
const staticId = staticErrorWorkflowId(errorWorkflowId);
|
||||
if (!staticId) return;
|
||||
|
||||
const errorWorkflow = await workflowFinderService.findWorkflowForUser(staticId, user, [
|
||||
'workflow:read',
|
||||
]);
|
||||
if (errorWorkflow) validateMcpWorkflow(errorWorkflow);
|
||||
|
||||
const problem = await errorWorkflowValidationService.findProblem({
|
||||
errorWorkflowId: staticId,
|
||||
parentWorkflowId,
|
||||
user,
|
||||
['workflow:read'],
|
||||
// activeVersion is only the published source of truth when the publication
|
||||
// service is off; otherwise we read it from the service below.
|
||||
{ includeActiveVersion: !useWorkflowPublicationService },
|
||||
);
|
||||
|
||||
if (!errorWorkflow) {
|
||||
throw new Error(
|
||||
`Error workflow '${errorWorkflowId}' was not found or you do not have access to it. Find a valid workflow ID with search_workflows, or create an error-handler workflow first.`,
|
||||
);
|
||||
}
|
||||
validateMcpWorkflow(errorWorkflow);
|
||||
|
||||
// Runtime runs the PUBLISHED version of the error workflow, not its draft, and
|
||||
// resolves it differently depending on the publication service flag — mirror
|
||||
// WorkflowExecutionService.loadErrorWorkflowData exactly so we neither reject a
|
||||
// workflow runtime would run nor accept a version runtime will not use.
|
||||
let publishedNodes: INode[] | undefined;
|
||||
|
||||
if (useWorkflowPublicationService) {
|
||||
const published = await workflowPublishedDataService.getPublishedWorkflowData(errorWorkflowId);
|
||||
publishedNodes = published?.publishedVersion.nodes;
|
||||
} else if (errorWorkflow.activeVersionId && errorWorkflow.activeVersion) {
|
||||
publishedNodes = errorWorkflow.activeVersion.nodes ?? [];
|
||||
}
|
||||
|
||||
if (!publishedNodes) {
|
||||
throw new Error(
|
||||
`Error workflow '${errorWorkflow.name}' (${errorWorkflowId}) has no published version, so n8n cannot run it when this workflow fails. Publish that workflow first (publish_workflow), then set it as the error workflow.`,
|
||||
);
|
||||
}
|
||||
|
||||
const hasErrorTrigger = publishedNodes.some(
|
||||
(node) => node.type === errorTriggerType && node.disabled !== true,
|
||||
);
|
||||
|
||||
if (!hasErrorTrigger) {
|
||||
throw new Error(
|
||||
`The published version of workflow '${errorWorkflow.name}' (${errorWorkflowId}) has no active Error Trigger node, so it would never run when this workflow fails. Add an Error Trigger node (${errorTriggerType}) and publish it, pick a different error workflow, or create a new error-handler workflow.`,
|
||||
);
|
||||
}
|
||||
|
||||
// Runtime blocks the error workflow if this workflow may not call it as a
|
||||
// sub-workflow (see WorkflowExecutionService.executeErrorWorkflow). The
|
||||
// policy checker only reads the target's id + settings, so an empty-node
|
||||
// Workflow instance is sufficient.
|
||||
const errorWorkflowInstance = new Workflow({
|
||||
id: errorWorkflow.id,
|
||||
name: errorWorkflow.name,
|
||||
nodeTypes,
|
||||
nodes: [],
|
||||
connections: {},
|
||||
active: false,
|
||||
settings: errorWorkflow.settings ?? {},
|
||||
});
|
||||
|
||||
try {
|
||||
await subworkflowPolicyChecker.check(
|
||||
errorWorkflowInstance,
|
||||
parentWorkflowId,
|
||||
undefined,
|
||||
user.id,
|
||||
);
|
||||
} catch (error) {
|
||||
if (error instanceof SubworkflowPolicyDenialError) {
|
||||
throw new Error(
|
||||
`Error workflow '${errorWorkflow.name}' (${errorWorkflowId}) cannot be called by this workflow because of its caller policy, so n8n would block it at runtime. Update that workflow's settings ("This workflow can be called by …") to allow this one — set it to any workflow, or add this workflow to its allowlist — or pick a different error workflow.`,
|
||||
);
|
||||
}
|
||||
if (!problem) return;
|
||||
|
||||
throw error;
|
||||
switch (problem.reason) {
|
||||
case 'not-found':
|
||||
throw new Error(
|
||||
`Error workflow '${staticId}' was not found or you do not have access to it. Find a valid workflow ID with search_workflows, or create an error-handler workflow first.`,
|
||||
);
|
||||
case 'not-published':
|
||||
throw new Error(
|
||||
`Error workflow '${problem.name}' (${staticId}) has no published version, so n8n cannot run it when this workflow fails. Publish that workflow first (publish_workflow), then set it as the error workflow.`,
|
||||
);
|
||||
case 'no-error-trigger':
|
||||
throw new Error(
|
||||
`The published version of workflow '${problem.name}' (${staticId}) has no active Error Trigger node, so it would never run when this workflow fails. Add an Error Trigger node (${problem.errorTriggerType}) and publish it, pick a different error workflow, or create a new error-handler workflow.`,
|
||||
);
|
||||
case 'caller-policy':
|
||||
throw new Error(
|
||||
`Error workflow '${problem.name}' (${staticId}) cannot be called by this workflow because of its caller policy, so n8n would block it at runtime. Update that workflow's settings ("This workflow can be called by …") to allow this one — set it to any workflow, or add this workflow to its allowlist — or pick a different error workflow.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -536,11 +486,9 @@ function assertExecutionTimeoutWithinMax(
|
|||
*/
|
||||
type WorkflowSettingsGuardDependencies = {
|
||||
user: User;
|
||||
nodeTypes: NodeTypes;
|
||||
globalConfig: GlobalConfig;
|
||||
workflowFinderService: WorkflowFinderService;
|
||||
workflowPublishedDataService: WorkflowPublishedDataService;
|
||||
subworkflowPolicyChecker: SubworkflowPolicyChecker;
|
||||
errorWorkflowValidationService: ErrorWorkflowValidationService;
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
@ -565,11 +513,9 @@ async function assertWorkflowSettingsValid(
|
|||
},
|
||||
{
|
||||
user,
|
||||
nodeTypes,
|
||||
globalConfig,
|
||||
workflowFinderService,
|
||||
workflowPublishedDataService,
|
||||
subworkflowPolicyChecker,
|
||||
errorWorkflowValidationService,
|
||||
}: WorkflowSettingsGuardDependencies,
|
||||
): Promise<void> {
|
||||
// Validate a freshly-set error workflow so the agent can self-correct in
|
||||
|
|
@ -585,11 +531,7 @@ async function assertWorkflowSettingsValid(
|
|||
parentWorkflowId: workflowId,
|
||||
user,
|
||||
workflowFinderService,
|
||||
workflowPublishedDataService,
|
||||
useWorkflowPublicationService: globalConfig.workflows.useWorkflowPublicationService,
|
||||
nodeTypes,
|
||||
subworkflowPolicyChecker,
|
||||
errorTriggerType: globalConfig.nodes.errorTriggerType,
|
||||
errorWorkflowValidationService,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -1116,8 +1058,7 @@ export const createUpdateWorkflowTool = (
|
|||
dataTableOps: DataTableUserOperations,
|
||||
tagService: TagService,
|
||||
globalConfig: GlobalConfig,
|
||||
subworkflowPolicyChecker: SubworkflowPolicyChecker,
|
||||
workflowPublishedDataService: WorkflowPublishedDataService,
|
||||
errorWorkflowValidationService: ErrorWorkflowValidationService,
|
||||
aiGatewayService: AiGatewayService,
|
||||
options: {
|
||||
/**
|
||||
|
|
@ -1136,11 +1077,9 @@ export const createUpdateWorkflowTool = (
|
|||
// what is being validated.
|
||||
const settingsGuardDependencies: WorkflowSettingsGuardDependencies = {
|
||||
user,
|
||||
nodeTypes,
|
||||
globalConfig,
|
||||
workflowFinderService,
|
||||
workflowPublishedDataService,
|
||||
subworkflowPolicyChecker,
|
||||
errorWorkflowValidationService,
|
||||
};
|
||||
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -51,7 +51,7 @@ export const workflowSettingsObjectSchema = z.object({
|
|||
errorWorkflow: z
|
||||
.string()
|
||||
.describe(
|
||||
'ID of a SEPARATE workflow to run whenever THIS workflow fails — the common best-practice way to send failure alerts (email, Slack, etc.) or log errors via a shared, reusable handler. The referenced workflow must contain an Error Trigger node; find its ID with search_workflows. Pass "DEFAULT" to clear it. There are two ways to handle failures: (a) a dedicated/shared error workflow set here, or (b) an Error Trigger node placed directly inside THIS workflow (n8n fires it automatically on failure, no setting needed). When the user asks for error handling, ask which pattern they prefer before choosing. When errorWorkflow is set, it takes precedence over a same-workflow Error Trigger for the failing run. Failure handling fires for production executions only, not manual/test runs. Distinct from per-node onError/retry (setNodeSettings).',
|
||||
'ID of a SEPARATE workflow to run whenever THIS workflow fails — the common best-practice way to send failure alerts (email, Slack, etc.) or log errors via a shared, reusable handler. The referenced workflow must contain an Error Trigger node; find its ID with search_workflows. Pass "DEFAULT" to clear it. Must be a plain ID — this setting is never evaluated, so an expression would be stored as a literal ID and the handler would never run. There are two ways to handle failures: (a) a dedicated/shared error workflow set here, or (b) an Error Trigger node placed directly inside THIS workflow (n8n fires it automatically on failure, no setting needed). When the user asks for error handling, ask which pattern they prefer before choosing. When errorWorkflow is set, it takes precedence over a same-workflow Error Trigger for the failing run. Failure handling fires for production executions only, not manual/test runs. Distinct from per-node onError/retry (setNodeSettings).',
|
||||
)
|
||||
.optional(),
|
||||
timezone: z
|
||||
|
|
|
|||
|
|
@ -27,7 +27,12 @@ import {
|
|||
import { executeWorkflowNode } from './utils/test-builders';
|
||||
|
||||
type ImportPackageParams = Pick<ImportPackageRequest, 'user' | 'packageBuffer'> &
|
||||
Partial<Pick<ImportPackageRequest, 'workflowIdPolicy' | 'workflowPublishingPolicy'>>;
|
||||
Partial<
|
||||
Pick<
|
||||
ImportPackageRequest,
|
||||
'workflowIdPolicy' | 'workflowPublishingPolicy' | 'workflowConflictPolicy'
|
||||
>
|
||||
>;
|
||||
|
||||
async function importPackage(params: ImportPackageParams) {
|
||||
return await Container.get(N8nPackagesService).importPackage(importPackageRequest(params));
|
||||
|
|
@ -191,6 +196,40 @@ describe('Package import of workflows with sub-workflows', () => {
|
|||
expect(importedMain.settings?.errorWorkflow).toBe(importedError.id);
|
||||
});
|
||||
|
||||
// A later package version can add an error handler for a workflow that is
|
||||
// already imported. The parent is then an update that points at a handler this
|
||||
// same import creates, so the handler is neither present nor published while
|
||||
// the parent is written — publishing is a package-wide sweep that runs after.
|
||||
it('remaps `settings.errorWorkflow` when the parent is an update and the handler is new', async () => {
|
||||
const owner = await createOwner();
|
||||
|
||||
const v1 = serializedWorkflow({ id: 'CHEDDAR', name: 'Main' });
|
||||
await importPackage({
|
||||
user: owner,
|
||||
packageBuffer: await buildSubWorkflowPackage([v1]),
|
||||
workflowIdPolicy: WorkflowIdPolicy.Source,
|
||||
});
|
||||
|
||||
const v2 = serializedWorkflow({
|
||||
id: 'CHEDDAR',
|
||||
name: 'Main',
|
||||
settings: { errorWorkflow: 'BRIE' },
|
||||
});
|
||||
const errorHandler = serializedWorkflow({ id: 'BRIE', name: 'Error handler' });
|
||||
|
||||
const result = await importPackage({
|
||||
user: owner,
|
||||
packageBuffer: await buildSubWorkflowPackage([v2, errorHandler]),
|
||||
workflowIdPolicy: WorkflowIdPolicy.Source,
|
||||
workflowConflictPolicy: 'new-version',
|
||||
});
|
||||
|
||||
const importedMain = await findImported(result, 'CHEDDAR');
|
||||
const importedError = await findImported(result, 'BRIE');
|
||||
|
||||
expect(importedMain.settings?.errorWorkflow).toBe(importedError.id);
|
||||
});
|
||||
|
||||
it('does not crash when a package carries a non-string settings.callerIds', async () => {
|
||||
const owner = await createOwner();
|
||||
|
||||
|
|
|
|||
|
|
@ -123,7 +123,12 @@ describe('WorkflowImporter.apply', () => {
|
|||
user,
|
||||
expect.any(WorkflowEntity),
|
||||
'existing-update',
|
||||
{ publicApi: true, source: 'import', allowArchivedUpdate: false },
|
||||
{
|
||||
publicApi: true,
|
||||
source: 'import',
|
||||
allowArchivedUpdate: false,
|
||||
allowUnresolvedErrorWorkflow: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -291,6 +296,7 @@ describe('WorkflowImporter.apply', () => {
|
|||
publicApi: true,
|
||||
source: 'import',
|
||||
allowArchivedUpdate: true,
|
||||
allowUnresolvedErrorWorkflow: true,
|
||||
});
|
||||
expect(archive).not.toHaveBeenCalled();
|
||||
expect(unarchive).not.toHaveBeenCalled();
|
||||
|
|
|
|||
|
|
@ -286,6 +286,11 @@ export class WorkflowImporter {
|
|||
publicApi: true,
|
||||
source: 'import',
|
||||
allowArchivedUpdate: item.existing.isArchived,
|
||||
// `settings.errorWorkflow` is a package reference this importer rebinds. The
|
||||
// handler it now points at can still be unwritten (later in this loop) and is
|
||||
// unpublished until the package-wide sweep, so the write-time check on that
|
||||
// reference cannot apply here.
|
||||
allowUnresolvedErrorWorkflow: true,
|
||||
...(tagIds !== undefined ? { tagIds } : {}),
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -298,6 +298,73 @@ describe('OAuthServerService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
const buildServiceWithQueryIgnoringResolver = () => {
|
||||
const registry = new ProtectedResourceRegistry(mock<Logger>());
|
||||
// A static resource would not reproduce the bug. Needs Resolver
|
||||
registry.registerResolver({
|
||||
id: 'form-path-only',
|
||||
scopes: [],
|
||||
resolveByUrl: async (url) =>
|
||||
new URL(url).pathname.replace(/\/$/, '') === '/form/abc'
|
||||
? {
|
||||
id: 'form-abc',
|
||||
isFirstParty: true,
|
||||
getResourceUrl: () => FIRST_PARTY_URL,
|
||||
getAudiences: () => [FIRST_PARTY_URL],
|
||||
scopes: [],
|
||||
authorize: async () => true,
|
||||
}
|
||||
: undefined,
|
||||
resolveByPath: async () => undefined,
|
||||
});
|
||||
|
||||
return new OAuthServerService(
|
||||
logger,
|
||||
mockInstance(GlobalConfig),
|
||||
oauthSessionService,
|
||||
oauthClientRepository,
|
||||
tokenService,
|
||||
authorizationCodeService,
|
||||
userConsentRepository,
|
||||
registry,
|
||||
mailer,
|
||||
urlServiceMock,
|
||||
mock<EventService>(),
|
||||
mock<AuthService>(),
|
||||
mock<OAuthConsentService>(),
|
||||
);
|
||||
};
|
||||
|
||||
it.each([
|
||||
['a query string', `${FIRST_PARTY_URL}?x=1`],
|
||||
['a trailing slash', `${FIRST_PARTY_URL}/`],
|
||||
['an oversized query string', `${FIRST_PARTY_URL}?z=${'a'.repeat(2048)}`],
|
||||
])(
|
||||
'returns undefined and does not upsert when the client_id is the resource URL with %s',
|
||||
async (_, clientId) => {
|
||||
oauthClientRepository.findOneBy.mockResolvedValue(null);
|
||||
|
||||
const result =
|
||||
await buildServiceWithQueryIgnoringResolver().clientsStore.getClient(clientId);
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(oauthClientRepository.upsert).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it('upserts the virtual client when the client_id is the canonical resource URL', async () => {
|
||||
oauthClientRepository.findOneBy.mockResolvedValue(null);
|
||||
|
||||
const result =
|
||||
await buildServiceWithQueryIgnoringResolver().clientsStore.getClient(FIRST_PARTY_URL);
|
||||
|
||||
expect(result).toMatchObject({ client_id: FIRST_PARTY_URL });
|
||||
expect(oauthClientRepository.upsert).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: FIRST_PARTY_URL, redirectUris: [FIRST_PARTY_URL] }),
|
||||
['id'],
|
||||
);
|
||||
});
|
||||
|
||||
it('returns undefined and does not upsert when the resolved resource is not first-party', async () => {
|
||||
oauthClientRepository.findOneBy.mockResolvedValue(null);
|
||||
|
||||
|
|
|
|||
|
|
@ -266,7 +266,7 @@ export class OAuthServerService implements OAuthServerProvider {
|
|||
// base URL, so a client_id that isn't can never resolve to one. Skip the resolver
|
||||
// sweep + lazy upsert for anything else, so the unauthenticated /authorize path
|
||||
// can't be used to fan out DB lookups on arbitrary client_ids.
|
||||
if (!this.isTriggerResourceClientId(clientId)) {
|
||||
if (clientId.length > MAX_REDIRECT_URI_LENGTH || !this.isTriggerResourceClientId(clientId)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
|
|
@ -275,6 +275,12 @@ export class OAuthServerService implements OAuthServerProvider {
|
|||
return undefined;
|
||||
}
|
||||
|
||||
// The lookup ignores the query string, so without this check each `?x=N` variant adds a new row.
|
||||
const resourceUrls = resource.getResourceUrls?.() ?? [resource.getResourceUrl()];
|
||||
if (!resourceUrls.includes(clientId)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
await this.oauthClientRepository.upsert(
|
||||
{
|
||||
id: clientId,
|
||||
|
|
|
|||
|
|
@ -62,7 +62,7 @@ export class CommunityPackagesPublicController {
|
|||
@Body body: InstallCommunityPackagePublicDto,
|
||||
): Promise<CommunityPackagePublicDto> {
|
||||
const installedPackage = await this.communityPackagesLifecycleService.install(
|
||||
{ name: body.name, version: body.version, verify: body.verify ?? true },
|
||||
{ name: body.name, version: body.version },
|
||||
req.user,
|
||||
'publicApi',
|
||||
);
|
||||
|
|
@ -84,7 +84,7 @@ export class CommunityPackagesPublicController {
|
|||
@Body body: UpdateCommunityPackagePublicDto,
|
||||
): Promise<CommunityPackagePublicDto> {
|
||||
const updated = await this.communityPackagesLifecycleService.update(
|
||||
{ name, version: body.version, verify: body.verify ?? true },
|
||||
{ name, version: body.version },
|
||||
req.user,
|
||||
'notFound',
|
||||
);
|
||||
|
|
|
|||
|
|
@ -19,10 +19,7 @@ requestBody:
|
|||
description: npm package name. Unscoped names start with n8n-nodes-. Scoped names use the @[author]/n8n-nodes- form.
|
||||
version:
|
||||
type: string
|
||||
description: Semver version or npm dist-tag, such as latest or beta
|
||||
verify:
|
||||
type: boolean
|
||||
description: Whether to verify the package against the n8n-vetted package list. Required when the instance has N8N_UNVERIFIED_PACKAGES_ENABLED=false.
|
||||
description: Semver version or npm dist-tag, such as latest or beta. When unverified packages are disabled, only versions verified by n8n can be installed.
|
||||
required:
|
||||
- name
|
||||
responses:
|
||||
|
|
|
|||
|
|
@ -22,10 +22,7 @@ requestBody:
|
|||
properties:
|
||||
version:
|
||||
type: string
|
||||
description: Semver version or npm dist-tag to update to, such as latest or beta
|
||||
verify:
|
||||
type: boolean
|
||||
description: Whether to verify the package against the n8n-vetted package list. Setting to false will allow installing or updating to an unverified version. Default is true.
|
||||
description: Semver version or npm dist-tag to update to, such as latest or beta. When unverified packages are disabled, only versions verified by n8n can be installed.
|
||||
responses:
|
||||
'200':
|
||||
description: Operation successful.
|
||||
|
|
|
|||
|
|
@ -211,15 +211,7 @@ export declare namespace AnnotationTagsRequest {
|
|||
export declare namespace NodeRequest {
|
||||
type GetAll = AuthenticatedRequest;
|
||||
|
||||
type Post = AuthenticatedRequest<
|
||||
{},
|
||||
{},
|
||||
{ name?: string; verify?: boolean; version?: string; checksum?: string }
|
||||
>;
|
||||
|
||||
type Delete = AuthenticatedRequest<{}, {}, {}, { name: string }>;
|
||||
|
||||
type Update = Post;
|
||||
}
|
||||
|
||||
// ----------------------------------
|
||||
|
|
|
|||
|
|
@ -156,9 +156,9 @@ async function fetchWorkflowData(
|
|||
} else {
|
||||
const workflowData = workflowInfo.code;
|
||||
if (workflowData) {
|
||||
if (!workflowData.id) {
|
||||
workflowData.id = parentWorkflowId;
|
||||
}
|
||||
// An inline sub-workflow is part of the parent that embeds it, not a
|
||||
// workflow of its own, so it runs under the parent workflow's id.
|
||||
workflowData.id = parentWorkflowId;
|
||||
workflowData.settings ??= parentWorkflowSettings;
|
||||
}
|
||||
return workflowData;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,186 @@
|
|||
import { mockInstance } from '@n8n/backend-test-utils';
|
||||
import type { GlobalConfig } from '@n8n/config';
|
||||
import type { User, WorkflowEntity, WorkflowHistory } from '@n8n/db';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import { SubworkflowPolicyDenialError } from '@/errors/subworkflow-policy-denial.error';
|
||||
import type { SubworkflowPolicyChecker } from '@/executions/pre-execution-checks/subworkflow-policy-checker';
|
||||
import { NodeTypes } from '@/node-types';
|
||||
import {
|
||||
ErrorWorkflowValidationService,
|
||||
staticErrorWorkflowId,
|
||||
} from '@/workflows/error-workflow-validation.service';
|
||||
import type { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
import type {
|
||||
PublishedWorkflowData,
|
||||
WorkflowPublishedDataService,
|
||||
} from '@/workflows/workflow-published-data.service';
|
||||
|
||||
const ERROR_TRIGGER_TYPE = 'n8n-nodes-base.errorTrigger';
|
||||
|
||||
describe('staticErrorWorkflowId', () => {
|
||||
it('returns the id for a concrete reference', () => {
|
||||
expect(staticErrorWorkflowId('wf-1')).toBe('wf-1');
|
||||
});
|
||||
|
||||
it.each([undefined, '', 'DEFAULT', '={{ $json.handler }}'])(
|
||||
'returns undefined for %s, which names no concrete workflow',
|
||||
(value) => {
|
||||
expect(staticErrorWorkflowId(value)).toBeUndefined();
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe('ErrorWorkflowValidationService', () => {
|
||||
const user = mock<User>({ id: 'user-1' });
|
||||
const nodeTypes = mockInstance(NodeTypes);
|
||||
|
||||
let workflowFinderService: WorkflowFinderService;
|
||||
let workflowPublishedDataService: WorkflowPublishedDataService;
|
||||
let subworkflowPolicyChecker: SubworkflowPolicyChecker;
|
||||
let service: ErrorWorkflowValidationService;
|
||||
|
||||
const buildErrorWorkflow = (overrides: Partial<WorkflowEntity> = {}) =>
|
||||
mock<WorkflowEntity>({
|
||||
id: 'err-wf',
|
||||
name: 'Error Handler',
|
||||
settings: {},
|
||||
activeVersionId: 'err-wf-v1',
|
||||
activeVersion: mock<WorkflowHistory>({
|
||||
nodes: [{ type: ERROR_TRIGGER_TYPE, disabled: false }] as INode[],
|
||||
}),
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const buildService = (useWorkflowPublicationService = false) => {
|
||||
const globalConfig = mock<GlobalConfig>({
|
||||
nodes: { errorTriggerType: ERROR_TRIGGER_TYPE },
|
||||
workflows: { useWorkflowPublicationService },
|
||||
});
|
||||
return new ErrorWorkflowValidationService(
|
||||
globalConfig,
|
||||
nodeTypes,
|
||||
workflowFinderService,
|
||||
workflowPublishedDataService,
|
||||
subworkflowPolicyChecker,
|
||||
);
|
||||
};
|
||||
|
||||
const findProblem = async (svc = service) =>
|
||||
await svc.findProblem({ errorWorkflowId: 'err-wf', parentWorkflowId: 'wf-1', user });
|
||||
|
||||
beforeEach(() => {
|
||||
workflowFinderService = mock<WorkflowFinderService>();
|
||||
workflowPublishedDataService = mock<WorkflowPublishedDataService>();
|
||||
subworkflowPolicyChecker = mock<SubworkflowPolicyChecker>();
|
||||
service = buildService();
|
||||
});
|
||||
|
||||
it('accepts a published, callable workflow with an active Error Trigger', async () => {
|
||||
vi.mocked(workflowFinderService.findWorkflowForUser).mockResolvedValue(buildErrorWorkflow());
|
||||
|
||||
expect(await findProblem()).toBeUndefined();
|
||||
});
|
||||
|
||||
it('requires read access, so a caller cannot probe ids outside their projects', async () => {
|
||||
vi.mocked(workflowFinderService.findWorkflowForUser).mockResolvedValue(null);
|
||||
|
||||
expect(await findProblem()).toEqual({ reason: 'not-found' });
|
||||
expect(workflowFinderService.findWorkflowForUser).toHaveBeenCalledWith(
|
||||
'err-wf',
|
||||
user,
|
||||
['workflow:read'],
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it('reports a workflow with no published version', async () => {
|
||||
vi.mocked(workflowFinderService.findWorkflowForUser).mockResolvedValue(
|
||||
buildErrorWorkflow({ activeVersionId: null, activeVersion: null }),
|
||||
);
|
||||
|
||||
expect(await findProblem()).toEqual({ reason: 'not-published', name: 'Error Handler' });
|
||||
});
|
||||
|
||||
it('reports a published version whose Error Trigger is disabled', async () => {
|
||||
vi.mocked(workflowFinderService.findWorkflowForUser).mockResolvedValue(
|
||||
buildErrorWorkflow({
|
||||
activeVersion: mock<WorkflowHistory>({
|
||||
nodes: [{ type: ERROR_TRIGGER_TYPE, disabled: true }] as INode[],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
|
||||
expect(await findProblem()).toEqual({
|
||||
reason: 'no-error-trigger',
|
||||
name: 'Error Handler',
|
||||
errorTriggerType: ERROR_TRIGGER_TYPE,
|
||||
});
|
||||
});
|
||||
|
||||
it('reports a target whose caller policy excludes the parent workflow', async () => {
|
||||
vi.mocked(workflowFinderService.findWorkflowForUser).mockResolvedValue(buildErrorWorkflow());
|
||||
vi.mocked(subworkflowPolicyChecker.check).mockRejectedValue(
|
||||
new SubworkflowPolicyDenialError({
|
||||
subworkflowId: 'err-wf',
|
||||
subworkflowProject: mock(),
|
||||
instanceUrl: 'http://localhost:5678',
|
||||
hasReadAccess: false,
|
||||
}),
|
||||
);
|
||||
|
||||
expect(await findProblem()).toEqual({ reason: 'caller-policy', name: 'Error Handler' });
|
||||
});
|
||||
|
||||
it('checks the policy against the parent workflow and the acting user', async () => {
|
||||
vi.mocked(workflowFinderService.findWorkflowForUser).mockResolvedValue(buildErrorWorkflow());
|
||||
|
||||
await findProblem();
|
||||
|
||||
expect(subworkflowPolicyChecker.check).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: 'err-wf' }),
|
||||
'wf-1',
|
||||
undefined,
|
||||
'user-1',
|
||||
);
|
||||
});
|
||||
|
||||
it('does not swallow an unexpected failure from the policy checker', async () => {
|
||||
vi.mocked(workflowFinderService.findWorkflowForUser).mockResolvedValue(buildErrorWorkflow());
|
||||
vi.mocked(subworkflowPolicyChecker.check).mockRejectedValue(new Error('db is down'));
|
||||
|
||||
await expect(findProblem()).rejects.toThrow('db is down');
|
||||
});
|
||||
|
||||
describe('with the workflow publication service on', () => {
|
||||
// Runtime reads the published version from the service in this mode, so
|
||||
// validation must read it from the same place or it would accept a version
|
||||
// runtime will never run.
|
||||
it('reads the published nodes from the publication service', async () => {
|
||||
const withPublicationService = buildService(true);
|
||||
vi.mocked(workflowFinderService.findWorkflowForUser).mockResolvedValue(buildErrorWorkflow());
|
||||
vi.mocked(workflowPublishedDataService.getPublishedWorkflowData).mockResolvedValue(
|
||||
mock<PublishedWorkflowData>({
|
||||
publishedVersion: mock<WorkflowHistory>({
|
||||
nodes: [{ type: ERROR_TRIGGER_TYPE }] as INode[],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
|
||||
expect(await findProblem(withPublicationService)).toBeUndefined();
|
||||
expect(workflowPublishedDataService.getPublishedWorkflowData).toHaveBeenCalledWith('err-wf');
|
||||
});
|
||||
|
||||
it('reports not-published when the service has no published version', async () => {
|
||||
const withPublicationService = buildService(true);
|
||||
vi.mocked(workflowFinderService.findWorkflowForUser).mockResolvedValue(buildErrorWorkflow());
|
||||
vi.mocked(workflowPublishedDataService.getPublishedWorkflowData).mockResolvedValue(null);
|
||||
|
||||
expect(await findProblem(withPublicationService)).toEqual({
|
||||
reason: 'not-published',
|
||||
name: 'Error Handler',
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -37,6 +37,7 @@ import type { ExternalHooks, WorkflowLifecycleHookActor } from '@/external-hooks
|
|||
import type { RedactionEnforcementService } from '@/modules/redaction/redaction-enforcement.service';
|
||||
import type { PolicyCleared } from '@n8n/decorators';
|
||||
import { userHasScopes } from '@/permissions.ee/check-access';
|
||||
import type { ErrorWorkflowValidationService } from '@/workflows/error-workflow-validation.service';
|
||||
import type { PolicyEnforcementService } from '@/policy/policy-enforcement.service';
|
||||
import { PolicyViolationError } from '@/policy/policy-violation.error';
|
||||
import type { DurableJobProvisioner } from '@/scheduling/durable-job-provisioner';
|
||||
|
|
@ -136,6 +137,7 @@ describe('WorkflowService', () => {
|
|||
mock(), // policyEnforcementService
|
||||
workflowPublicationStatusServiceMock, // workflowPublicationStatusService
|
||||
mock(), // nodeGroupRulesFlagGate
|
||||
mock(), // errorWorkflowValidationService
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -510,6 +512,7 @@ describe('WorkflowService', () => {
|
|||
mock(), // policyEnforcementService
|
||||
mock(), // workflowPublicationStatusService
|
||||
nodeGroupRulesFlagGateMock, // nodeGroupRulesFlagGate
|
||||
mock(), // errorWorkflowValidationService
|
||||
);
|
||||
|
||||
vi.clearAllMocks();
|
||||
|
|
@ -1354,6 +1357,7 @@ describe('WorkflowService', () => {
|
|||
policyEnforcementServiceMock, // policyEnforcementService
|
||||
mock(), // workflowPublicationStatusService
|
||||
mock(), // nodeGroupRulesFlagGate
|
||||
mock(), // errorWorkflowValidationService
|
||||
);
|
||||
|
||||
// Bypass validation internals
|
||||
|
|
@ -2114,6 +2118,7 @@ describe('WorkflowService', () => {
|
|||
mock(), // policyEnforcementService
|
||||
mock(), // workflowPublicationStatusService
|
||||
mock(), // nodeGroupRulesFlagGate
|
||||
mock(), // errorWorkflowValidationService
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -2255,6 +2260,7 @@ describe('WorkflowService', () => {
|
|||
mock(), // policyEnforcementService
|
||||
mock(), // workflowPublicationStatusService
|
||||
mock(), // nodeGroupRulesFlagGate
|
||||
mock(), // errorWorkflowValidationService
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -2561,6 +2567,7 @@ describe('WorkflowService', () => {
|
|||
mock(), // policyEnforcementService
|
||||
mock(), // workflowPublicationStatusService
|
||||
mock(), // nodeGroupRulesFlagGate
|
||||
mock(), // errorWorkflowValidationService
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -2731,6 +2738,7 @@ describe('WorkflowService', () => {
|
|||
policyEnforcementServiceMock, // policyEnforcementService
|
||||
mock(), // workflowPublicationStatusService
|
||||
mock(), // nodeGroupRulesFlagGate
|
||||
mock(), // errorWorkflowValidationService
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -2918,6 +2926,7 @@ describe('WorkflowService', () => {
|
|||
mock(), // policyEnforcementService
|
||||
mock(), // workflowPublicationStatusService
|
||||
mock(), // nodeGroupRulesFlagGate
|
||||
mock(), // errorWorkflowValidationService
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -3021,6 +3030,7 @@ describe('WorkflowService', () => {
|
|||
mock(), // policyEnforcementService
|
||||
mock(), // workflowPublicationStatusService
|
||||
mock(), // nodeGroupRulesFlagGate
|
||||
mock(), // errorWorkflowValidationService
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -3064,4 +3074,179 @@ describe('WorkflowService', () => {
|
|||
).rejects.toThrow('Some tags not found');
|
||||
});
|
||||
});
|
||||
|
||||
describe('update() error workflow reference', () => {
|
||||
let workflowService: WorkflowService;
|
||||
let workflowFinderServiceMock: MockProxy<WorkflowFinderService>;
|
||||
let errorWorkflowValidationServiceMock: MockProxy<ErrorWorkflowValidationService>;
|
||||
let workflowRepositoryMock: MockProxy<{ update: Mock; updateContent: Mock; findOne: Mock }>;
|
||||
|
||||
const user = mock<User>({ id: 'user-1' });
|
||||
|
||||
beforeEach(() => {
|
||||
workflowFinderServiceMock = mock<WorkflowFinderService>();
|
||||
errorWorkflowValidationServiceMock = mock<ErrorWorkflowValidationService>();
|
||||
errorWorkflowValidationServiceMock.findProblem.mockResolvedValue(undefined);
|
||||
workflowRepositoryMock = mock();
|
||||
|
||||
const ownershipServiceMock = mock<OwnershipService>();
|
||||
ownershipServiceMock.getWorkflowProjectCached.mockResolvedValue(
|
||||
mock<Project>({ id: 'project-1' }),
|
||||
);
|
||||
|
||||
workflowService = new WorkflowService(
|
||||
mock(), // logger
|
||||
mock(), // sharedWorkflowRepository
|
||||
workflowRepositoryMock as never, // workflowRepository
|
||||
mock(), // workflowTagMappingRepository
|
||||
ownershipServiceMock, // ownershipService
|
||||
mock(), // tagService
|
||||
mock(), // workflowHistoryService
|
||||
mock(), // externalHooks
|
||||
mock(), // activeWorkflowManager
|
||||
mock(), // roleService
|
||||
mock(), // projectService
|
||||
mock(), // executionPersistence
|
||||
mock(), // eventService
|
||||
mock(), // globalConfig
|
||||
mock(), // folderRepository
|
||||
workflowFinderServiceMock, // workflowFinderService
|
||||
mock(), // workflowPublishHistoryRepository
|
||||
mock(), // outboxRepository
|
||||
Object.assign(mock<WorkflowValidationService>(), {
|
||||
validateCredentialNodeRestrictions: () => ({ isValid: true }),
|
||||
}), // workflowValidationService
|
||||
mock(), // nodeTypes
|
||||
mock(), // webhookService
|
||||
mock(), // licenseState
|
||||
mock(), // projectRepository
|
||||
mock(), // redactionEnforcementService
|
||||
mock(), // workflowPublicationNotifier
|
||||
mock(), // scheduleTriggerJobRegistrar
|
||||
mock(), // pollTriggerJobRegistrar
|
||||
mock(), // workflowScheduledJobOwner
|
||||
mock(), // durableJobProvisioner
|
||||
mock(), // workflowPublishedVersionRepository
|
||||
mock(), // workflowHookContextService
|
||||
mock(), // workflowPublishGuard
|
||||
mock(), // workflowMutationHooks
|
||||
mock(), // policyEnforcementService
|
||||
mock(), // workflowPublicationStatusService
|
||||
mock(), // nodeGroupRulesFlagGate
|
||||
errorWorkflowValidationServiceMock, // errorWorkflowValidationService
|
||||
);
|
||||
|
||||
vi.mocked(WorkflowHelpers.removeDefaultValues).mockImplementation((settings) => settings);
|
||||
});
|
||||
|
||||
const setupExistingWorkflow = (settings: Record<string, unknown> = {}) => {
|
||||
const existingWorkflow = mock<WorkflowEntity>({
|
||||
id: 'workflow-1',
|
||||
isArchived: false,
|
||||
versionId: 'v1',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
settings,
|
||||
activeVersionId: undefined as unknown as string,
|
||||
tags: [],
|
||||
});
|
||||
workflowFinderServiceMock.findWorkflowForUser.mockResolvedValue(existingWorkflow);
|
||||
workflowRepositoryMock.findOne.mockResolvedValue(existingWorkflow);
|
||||
};
|
||||
|
||||
const update = async (
|
||||
settings: Record<string, unknown>,
|
||||
options: { allowUnresolvedErrorWorkflow?: boolean } = {},
|
||||
) =>
|
||||
await workflowService.update(user, { settings } as unknown as WorkflowEntity, 'workflow-1', {
|
||||
forceSave: true,
|
||||
...options,
|
||||
});
|
||||
|
||||
it('validates a newly set reference against the acting user', async () => {
|
||||
setupExistingWorkflow();
|
||||
|
||||
await update({ errorWorkflow: 'err-wf' });
|
||||
|
||||
expect(errorWorkflowValidationServiceMock.findProblem).toHaveBeenCalledWith({
|
||||
errorWorkflowId: 'err-wf',
|
||||
parentWorkflowId: 'workflow-1',
|
||||
user,
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
['not-found', 'does not exist or you do not have access to it'],
|
||||
['not-published', 'has no published version'],
|
||||
['no-error-trigger', 'no active Error Trigger node'],
|
||||
['caller-policy', 'does not allow this workflow to call it'],
|
||||
] as const)('rejects the write when the reference is %s', async (reason, message) => {
|
||||
setupExistingWorkflow();
|
||||
errorWorkflowValidationServiceMock.findProblem.mockResolvedValue({
|
||||
reason,
|
||||
name: 'Error Handler',
|
||||
errorTriggerType: 'n8n-nodes-base.errorTrigger',
|
||||
} as never);
|
||||
|
||||
await expect(update({ errorWorkflow: 'err-wf' })).rejects.toThrow(BadRequestError);
|
||||
await expect(update({ errorWorkflow: 'err-wf' })).rejects.toThrow(message);
|
||||
expect(workflowRepositoryMock.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// The message must read the same whether the workflow is missing or merely
|
||||
// unreadable, otherwise the endpoint reports which ids exist.
|
||||
it('does not distinguish a missing workflow from an unreadable one', async () => {
|
||||
setupExistingWorkflow();
|
||||
errorWorkflowValidationServiceMock.findProblem.mockResolvedValue({ reason: 'not-found' });
|
||||
|
||||
await expect(update({ errorWorkflow: 'err-wf' })).rejects.toThrow(
|
||||
'The selected error workflow does not exist or you do not have access to it.',
|
||||
);
|
||||
});
|
||||
|
||||
// The setting is unversioned, so an existing workflow whose handler was since
|
||||
// archived or restricted must stay saveable.
|
||||
it('does not re-check a reference the write leaves unchanged', async () => {
|
||||
setupExistingWorkflow({ errorWorkflow: 'err-wf' });
|
||||
|
||||
await update({ errorWorkflow: 'err-wf', timezone: 'UTC' });
|
||||
|
||||
expect(errorWorkflowValidationServiceMock.findProblem).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
['no settings key', {}],
|
||||
['a cleared reference', { errorWorkflow: 'DEFAULT' }],
|
||||
['an expression', { errorWorkflow: '={{ $json.handler }}' }],
|
||||
])('skips validation for %s', async (_label, settings) => {
|
||||
setupExistingWorkflow();
|
||||
|
||||
await update(settings);
|
||||
|
||||
expect(errorWorkflowValidationServiceMock.findProblem).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('validates a reference that replaces a different one', async () => {
|
||||
setupExistingWorkflow({ errorWorkflow: 'old-err-wf' });
|
||||
|
||||
await update({ errorWorkflow: 'new-err-wf' });
|
||||
|
||||
expect(errorWorkflowValidationServiceMock.findProblem).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ errorWorkflowId: 'new-err-wf' }),
|
||||
);
|
||||
});
|
||||
|
||||
// A package import points at a handler the same import still has to create and
|
||||
// publish, so none of the four problems is decidable while it writes.
|
||||
it('skips validation for a package import', async () => {
|
||||
setupExistingWorkflow();
|
||||
errorWorkflowValidationServiceMock.findProblem.mockResolvedValue({ reason: 'not-found' });
|
||||
|
||||
await expect(
|
||||
update({ errorWorkflow: 'err-wf' }, { allowUnresolvedErrorWorkflow: true }),
|
||||
).resolves.toBeDefined();
|
||||
|
||||
expect(errorWorkflowValidationServiceMock.findProblem).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
167
packages/cli/src/workflows/error-workflow-validation.service.ts
Normal file
167
packages/cli/src/workflows/error-workflow-validation.service.ts
Normal file
|
|
@ -0,0 +1,167 @@
|
|||
import { GlobalConfig } from '@n8n/config';
|
||||
import type { User } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import type { IWorkflowSettings } from 'n8n-workflow';
|
||||
import { Workflow } from 'n8n-workflow';
|
||||
|
||||
import { WorkflowFinderService } from './workflow-finder.service';
|
||||
import { WorkflowPublishedDataService } from './workflow-published-data.service';
|
||||
|
||||
import { SubworkflowPolicyDenialError } from '@/errors/subworkflow-policy-denial.error';
|
||||
import { SubworkflowPolicyChecker } from '@/executions/pre-execution-checks/subworkflow-policy-checker';
|
||||
import { NodeTypes } from '@/node-types';
|
||||
|
||||
/**
|
||||
* Why a workflow cannot serve as another workflow's error handler. Callers turn
|
||||
* this into their own copy — the MCP tool teaches an agent how to self-correct,
|
||||
* the REST path states the problem — so the rules stay in one place while the
|
||||
* wording stays fit for each audience.
|
||||
*/
|
||||
export type ErrorWorkflowProblem =
|
||||
| { reason: 'not-found' }
|
||||
| { reason: 'not-published'; name: string }
|
||||
| { reason: 'no-error-trigger'; name: string; errorTriggerType: string }
|
||||
| { reason: 'caller-policy'; name: string };
|
||||
|
||||
/**
|
||||
* True when the setting holds an n8n expression.
|
||||
*
|
||||
* Nothing ever evaluates it. `executeErrorWorkflow` reads `settings.errorWorkflow`
|
||||
* and passes it to the error-workflow lookup verbatim, so a stored `=…` string is
|
||||
* used as a literal workflow id and can never match one — the handler silently
|
||||
* never runs. The editor's setting is a workflow picker and cannot produce this;
|
||||
* only a caller that writes settings directly (API, MCP) can. Such callers should
|
||||
* reject it rather than save a reference that is dead on arrival.
|
||||
*/
|
||||
export function isExpressionErrorWorkflowId(
|
||||
errorWorkflow: IWorkflowSettings['errorWorkflow'],
|
||||
): boolean {
|
||||
return typeof errorWorkflow === 'string' && errorWorkflow.startsWith('=');
|
||||
}
|
||||
|
||||
/**
|
||||
* The workflow id `settings.errorWorkflow` actually points at, or undefined when
|
||||
* the setting names no concrete workflow: cleared (`DEFAULT`), or an expression.
|
||||
*
|
||||
* An expression yields undefined because there is no id to validate, NOT because
|
||||
* it is a usable value — see {@link isExpressionErrorWorkflowId}.
|
||||
*/
|
||||
export function staticErrorWorkflowId(
|
||||
errorWorkflow: IWorkflowSettings['errorWorkflow'],
|
||||
): string | undefined {
|
||||
if (
|
||||
typeof errorWorkflow !== 'string' ||
|
||||
errorWorkflow === '' ||
|
||||
errorWorkflow === 'DEFAULT' ||
|
||||
isExpressionErrorWorkflowId(errorWorkflow)
|
||||
) {
|
||||
return undefined;
|
||||
}
|
||||
return errorWorkflow;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates a `settings.errorWorkflow` reference at write time.
|
||||
*
|
||||
* Runtime runs the named workflow with the failed run's context and checks only
|
||||
* the target's own caller policy, so nothing downstream establishes that the
|
||||
* person who wrote the setting may point at that workflow at all. Checking on
|
||||
* write also catches the three ways a link silently never fires: the target is
|
||||
* gone, unpublished, or has no Error Trigger.
|
||||
*
|
||||
* Callers must only run this when the reference actually changes, so a workflow
|
||||
* that already carries a stale link stays saveable.
|
||||
*/
|
||||
@Service()
|
||||
export class ErrorWorkflowValidationService {
|
||||
constructor(
|
||||
private readonly globalConfig: GlobalConfig,
|
||||
private readonly nodeTypes: NodeTypes,
|
||||
private readonly workflowFinderService: WorkflowFinderService,
|
||||
private readonly workflowPublishedDataService: WorkflowPublishedDataService,
|
||||
private readonly subworkflowPolicyChecker: SubworkflowPolicyChecker,
|
||||
) {}
|
||||
|
||||
/** The problem with the reference, or undefined when it is usable. */
|
||||
async findProblem({
|
||||
errorWorkflowId,
|
||||
parentWorkflowId,
|
||||
user,
|
||||
}: {
|
||||
errorWorkflowId: string;
|
||||
parentWorkflowId: string;
|
||||
user: User;
|
||||
}): Promise<ErrorWorkflowProblem | undefined> {
|
||||
const { useWorkflowPublicationService } = this.globalConfig.workflows;
|
||||
const { errorTriggerType } = this.globalConfig.nodes;
|
||||
|
||||
// Read access is required intentionally, mirroring the editor UI (the error
|
||||
// workflow picker only lists workflows the user can read). Resolving the
|
||||
// target without an access check would let callers probe arbitrary workflow
|
||||
// IDs and learn their name / published / trigger / policy state from the
|
||||
// problems below. Runtime not requiring read access is separate: it runs the
|
||||
// error workflow under the owner project's context, gated by caller policy,
|
||||
// which is about execution — not about who may configure the link.
|
||||
const errorWorkflow = await this.workflowFinderService.findWorkflowForUser(
|
||||
errorWorkflowId,
|
||||
user,
|
||||
['workflow:read'],
|
||||
// activeVersion is only the published source of truth when the publication
|
||||
// service is off; otherwise we read it from the service below.
|
||||
{ includeActiveVersion: !useWorkflowPublicationService },
|
||||
);
|
||||
|
||||
if (!errorWorkflow) return { reason: 'not-found' };
|
||||
|
||||
// Runtime runs the PUBLISHED version of the error workflow, not its draft, and
|
||||
// resolves it differently depending on the publication service flag — mirror
|
||||
// WorkflowExecutionService.loadErrorWorkflowData exactly so we neither reject a
|
||||
// workflow runtime would run nor accept a version runtime will not use.
|
||||
const publishedNodes = useWorkflowPublicationService
|
||||
? (await this.workflowPublishedDataService.getPublishedWorkflowData(errorWorkflowId))
|
||||
?.publishedVersion.nodes
|
||||
: errorWorkflow.activeVersionId && errorWorkflow.activeVersion
|
||||
? (errorWorkflow.activeVersion.nodes ?? [])
|
||||
: undefined;
|
||||
|
||||
if (!publishedNodes) return { reason: 'not-published', name: errorWorkflow.name };
|
||||
|
||||
const hasErrorTrigger = publishedNodes.some(
|
||||
(node) => node.type === errorTriggerType && node.disabled !== true,
|
||||
);
|
||||
|
||||
if (!hasErrorTrigger) {
|
||||
return { reason: 'no-error-trigger', name: errorWorkflow.name, errorTriggerType };
|
||||
}
|
||||
|
||||
// Runtime blocks the error workflow if this workflow may not call it as a
|
||||
// sub-workflow (see WorkflowExecutionService.executeErrorWorkflow). The
|
||||
// policy checker only reads the target's id + settings, so an empty-node
|
||||
// Workflow instance is sufficient.
|
||||
const errorWorkflowInstance = new Workflow({
|
||||
id: errorWorkflow.id,
|
||||
name: errorWorkflow.name,
|
||||
nodeTypes: this.nodeTypes,
|
||||
nodes: [],
|
||||
connections: {},
|
||||
active: false,
|
||||
settings: errorWorkflow.settings ?? {},
|
||||
});
|
||||
|
||||
try {
|
||||
await this.subworkflowPolicyChecker.check(
|
||||
errorWorkflowInstance,
|
||||
parentWorkflowId,
|
||||
undefined,
|
||||
user.id,
|
||||
);
|
||||
} catch (error) {
|
||||
if (error instanceof SubworkflowPolicyDenialError) {
|
||||
return { reason: 'caller-policy', name: errorWorkflow.name };
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
|
@ -30,6 +30,11 @@ import type { INode, INodes, IWorkflowSettings, JsonValue, IConnections } from '
|
|||
import { PROJECT_ROOT, Workflow, assert, calculateWorkflowChecksum } from 'n8n-workflow';
|
||||
import { v4 as uuid } from 'uuid';
|
||||
|
||||
import {
|
||||
ErrorWorkflowValidationService,
|
||||
staticErrorWorkflowId,
|
||||
type ErrorWorkflowProblem,
|
||||
} from './error-workflow-validation.service';
|
||||
import { WorkflowPublicationNotifier } from './publication/workflow-publication-notifier';
|
||||
import { WorkflowPublicationStatusService } from './publication/workflow-publication-status.service';
|
||||
import { NodeGroupRulesFlagGate } from './node-group-rules-flag-gate';
|
||||
|
|
@ -129,8 +134,66 @@ export class WorkflowService {
|
|||
private readonly policyEnforcementService: PolicyEnforcementService,
|
||||
private readonly workflowPublicationStatusService: WorkflowPublicationStatusService,
|
||||
private readonly nodeGroupRulesFlagGate: NodeGroupRulesFlagGate,
|
||||
private readonly errorWorkflowValidationService: ErrorWorkflowValidationService,
|
||||
) {}
|
||||
|
||||
/**
|
||||
* Rejects a write that points `settings.errorWorkflow` at a workflow the
|
||||
* acting user may not link to, or one that could never handle the failure.
|
||||
*
|
||||
* Only a *changed* reference is checked: the setting is unversioned and takes
|
||||
* effect on the next failure, so this is the one moment authorization can be
|
||||
* applied — but re-checking an unchanged value would make a workflow whose
|
||||
* handler was since archived or restricted impossible to save at all.
|
||||
*
|
||||
* This is a check on *authoring* a reference (UI, API, MCP), which is why a
|
||||
* package import opts out with `allowUnresolvedErrorWorkflow`. An import writes
|
||||
* package content whose references it rebinds itself, and mid-import none of
|
||||
* these four problems is decidable: the handler may be created later in the same
|
||||
* apply loop, it is published only by the package-wide sweep that runs after every
|
||||
* write, and its own `callerIds` are rebound as part of that same content. Nor
|
||||
* would enforcing it here buy anything — the create path has no such check, so a
|
||||
* package reaches the same end state by importing the parent as a new workflow.
|
||||
*/
|
||||
private async assertErrorWorkflowChangeAllowed(
|
||||
user: User,
|
||||
workflowId: string,
|
||||
currentSettings: IWorkflowSettings | undefined,
|
||||
updateSettings: IWorkflowSettings | undefined,
|
||||
) {
|
||||
const nextId = staticErrorWorkflowId(updateSettings?.errorWorkflow);
|
||||
if (!nextId || nextId === staticErrorWorkflowId(currentSettings?.errorWorkflow)) return;
|
||||
|
||||
const problem = await this.errorWorkflowValidationService.findProblem({
|
||||
errorWorkflowId: nextId,
|
||||
parentWorkflowId: workflowId,
|
||||
user,
|
||||
});
|
||||
|
||||
if (!problem) return;
|
||||
|
||||
this.logger.warn('Rejected an error workflow reference the user may not set', {
|
||||
workflowId,
|
||||
errorWorkflowId: nextId,
|
||||
userId: user.id,
|
||||
reason: problem.reason,
|
||||
});
|
||||
|
||||
// `not-found` covers "no such workflow" and "you cannot read it" alike, so
|
||||
// the message must not distinguish them — telling them apart would turn
|
||||
// this into a probe for workflow ids outside the user's projects.
|
||||
const messages: Record<ErrorWorkflowProblem['reason'], string> = {
|
||||
'not-found': 'The selected error workflow does not exist or you do not have access to it.',
|
||||
'not-published': 'The selected error workflow has no published version.',
|
||||
'no-error-trigger':
|
||||
'The published version of the selected error workflow has no active Error Trigger node.',
|
||||
'caller-policy':
|
||||
'The selected error workflow does not allow this workflow to call it. Update its "This workflow can be called by" setting.',
|
||||
};
|
||||
|
||||
throw new BadRequestError(messages[problem.reason]);
|
||||
}
|
||||
|
||||
async getMany(
|
||||
user: User,
|
||||
options?: ListQuery.Options,
|
||||
|
|
@ -412,6 +475,11 @@ export class WorkflowService {
|
|||
versionDescription?: string;
|
||||
/** Allows a package import to update archived content. */
|
||||
allowArchivedUpdate?: boolean;
|
||||
/**
|
||||
* Skips the `settings.errorWorkflow` write-time check for a package import.
|
||||
* See {@link assertErrorWorkflowChangeAllowed}.
|
||||
*/
|
||||
allowUnresolvedErrorWorkflow?: boolean;
|
||||
} = {},
|
||||
): Promise<WorkflowEntity> {
|
||||
const {
|
||||
|
|
@ -428,6 +496,7 @@ export class WorkflowService {
|
|||
versionName,
|
||||
versionDescription,
|
||||
allowArchivedUpdate = false,
|
||||
allowUnresolvedErrorWorkflow = false,
|
||||
} = options;
|
||||
const workflow = await this.workflowFinderService.findWorkflowForUser(workflowId, user, [
|
||||
'workflow:update',
|
||||
|
|
@ -452,6 +521,15 @@ export class WorkflowService {
|
|||
workflowUpdateData.settings?.redactionPolicy,
|
||||
);
|
||||
|
||||
if (!allowUnresolvedErrorWorkflow) {
|
||||
await this.assertErrorWorkflowChangeAllowed(
|
||||
user,
|
||||
workflowId,
|
||||
workflow.settings,
|
||||
workflowUpdateData.settings,
|
||||
);
|
||||
}
|
||||
|
||||
if (!forceSave && expectedChecksum) {
|
||||
await this._detectConflicts(workflow, expectedChecksum);
|
||||
}
|
||||
|
|
|
|||
151
packages/cli/test/integration/mcp/get-execution.tool.test.ts
Normal file
151
packages/cli/test/integration/mcp/get-execution.tool.test.ts
Normal file
|
|
@ -0,0 +1,151 @@
|
|||
import { LicenseState } from '@n8n/backend-common';
|
||||
import {
|
||||
createWorkflow,
|
||||
shareWorkflowWithUsers,
|
||||
mockInstance,
|
||||
testDb,
|
||||
testModules,
|
||||
} from '@n8n/backend-test-utils';
|
||||
import { ExecutionRepository, type User } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import { stringify } from 'flatted';
|
||||
import { createEmptyRunExecutionData, type IRunExecutionData } from 'n8n-workflow';
|
||||
|
||||
import { createExecution } from '../shared/db/executions';
|
||||
import { createMember, createOwner } from '../shared/db/users';
|
||||
|
||||
import { ExecutionRedactionServiceProxy } from '@/executions/execution-redaction-proxy.service';
|
||||
import { createGetExecutionTool } from '@/modules/mcp/tools/get-execution.tool';
|
||||
import { RedactionModule } from '@/modules/redaction/redaction.module';
|
||||
import { Telemetry } from '@/telemetry';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
|
||||
beforeAll(async () => {
|
||||
await testModules.loadModules(['mcp', 'redaction']);
|
||||
await testDb.init();
|
||||
// `loadModules` registers modules but never runs their `init`, which is what
|
||||
// hands the real implementation to `ExecutionRedactionServiceProxy`.
|
||||
await Container.get(RedactionModule).init();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await testDb.terminate();
|
||||
});
|
||||
|
||||
const telemetry = mockInstance(Telemetry, { track: vi.fn() });
|
||||
|
||||
/**
|
||||
* Data redaction is licensed-gated, and `resolvePolicy` returns 'none' without it.
|
||||
*/
|
||||
const licenseState = mockInstance(LicenseState, {
|
||||
isDataRedactionLicensed: () => true,
|
||||
});
|
||||
|
||||
const SECRET = 'super-secret-value';
|
||||
|
||||
const runExecutionData = (): IRunExecutionData => ({
|
||||
...createEmptyRunExecutionData(),
|
||||
resultData: {
|
||||
runData: {
|
||||
'HTTP Request': [
|
||||
{
|
||||
startTime: Date.now(),
|
||||
executionIndex: 0,
|
||||
executionTime: 1,
|
||||
executionStatus: 'success',
|
||||
source: [],
|
||||
data: { main: [[{ json: { secret: SECRET } }]] },
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const callTool = async (user: User, workflowId: string, executionId: string) => {
|
||||
const tool = createGetExecutionTool(
|
||||
user,
|
||||
Container.get(ExecutionRepository),
|
||||
Container.get(WorkflowFinderService),
|
||||
telemetry,
|
||||
Container.get(ExecutionRedactionServiceProxy),
|
||||
);
|
||||
|
||||
return await tool.handler(
|
||||
{
|
||||
workflowId,
|
||||
executionId,
|
||||
includeData: true,
|
||||
nodeNames: undefined,
|
||||
truncateData: undefined,
|
||||
},
|
||||
{} as never,
|
||||
);
|
||||
};
|
||||
|
||||
const itemOf = (result: Awaited<ReturnType<typeof callTool>>) => {
|
||||
const data = (result.structuredContent as { data: IRunExecutionData }).data;
|
||||
return data.resultData.runData['HTTP Request'][0].data?.main[0]?.[0];
|
||||
};
|
||||
|
||||
const redactionInfoOf = (result: Awaited<ReturnType<typeof callTool>>) =>
|
||||
(result.structuredContent as { data: IRunExecutionData }).data.redactionInfo;
|
||||
|
||||
const setup = async ({ redact, owner }: { redact: boolean; owner: User }) => {
|
||||
const workflow = await createWorkflow(
|
||||
{
|
||||
settings: {
|
||||
availableInMCP: true,
|
||||
...(redact && { redactionPolicy: 'all' as const }),
|
||||
},
|
||||
},
|
||||
owner,
|
||||
);
|
||||
|
||||
const execution = await createExecution(
|
||||
{ status: 'success', data: stringify(runExecutionData()) },
|
||||
workflow,
|
||||
);
|
||||
|
||||
return { workflow, execution };
|
||||
};
|
||||
|
||||
describe('get_workflow_execution applies the workflow redaction policy', () => {
|
||||
let owner: User;
|
||||
let member: User;
|
||||
|
||||
beforeEach(async () => {
|
||||
await testDb.truncate(['ExecutionEntity', 'WorkflowEntity', 'SharedWorkflow', 'User']);
|
||||
owner = await createOwner();
|
||||
member = await createMember();
|
||||
expect(licenseState.isDataRedactionLicensed()).toBe(true);
|
||||
});
|
||||
|
||||
test('redacts for the owner, who may still reveal', async () => {
|
||||
const { workflow, execution } = await setup({ redact: true, owner });
|
||||
|
||||
const result = await callTool(owner, workflow.id, execution.id);
|
||||
|
||||
expect(itemOf(result)?.json).toEqual({});
|
||||
expect(redactionInfoOf(result)).toMatchObject({ isRedacted: true, canReveal: true });
|
||||
});
|
||||
|
||||
test('redacts for a shared editor, who may not reveal', async () => {
|
||||
const { workflow, execution } = await setup({ redact: true, owner });
|
||||
await shareWorkflowWithUsers(workflow, [member]);
|
||||
|
||||
const result = await callTool(member, workflow.id, execution.id);
|
||||
|
||||
// The `workflow:editor` sharing role carries no `execution:reveal` scope.
|
||||
expect(itemOf(result)?.json).toEqual({});
|
||||
expect(redactionInfoOf(result)).toMatchObject({ isRedacted: true, canReveal: false });
|
||||
});
|
||||
|
||||
test('leaves data untouched when the workflow has no redaction policy', async () => {
|
||||
const { workflow, execution } = await setup({ redact: false, owner });
|
||||
|
||||
const result = await callTool(owner, workflow.id, execution.id);
|
||||
|
||||
expect(itemOf(result)?.json).toEqual({ secret: SECRET });
|
||||
expect(redactionInfoOf(result)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
@ -9,12 +9,14 @@ vi.mock('@/modules/community-packages/npm-utils', async () => ({
|
|||
import type { CommunityNodeType } from '@n8n/api-types';
|
||||
import { mockInstance, testDb } from '@n8n/backend-test-utils';
|
||||
import type { User } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import type { ApiKeyScope } from '@n8n/permissions';
|
||||
import { OWNER_API_KEY_SCOPES } from '@n8n/permissions';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
|
||||
import { CommunityNodeTypesService } from '@/modules/community-packages/community-node-types.service';
|
||||
import { CommunityPackagesConfig } from '@/modules/community-packages/community-packages.config';
|
||||
import { CommunityPackagesService } from '@/modules/community-packages/community-packages.service';
|
||||
import { executeNpmCommand } from '@/modules/community-packages/npm-utils';
|
||||
|
||||
|
|
@ -52,6 +54,7 @@ const parsedNpmPackageName = {
|
|||
};
|
||||
|
||||
let owner: User;
|
||||
const communityPackagesConfig = Container.get(CommunityPackagesConfig);
|
||||
|
||||
describe('Community packages (Public API)', () => {
|
||||
beforeAll(async () => {
|
||||
|
|
@ -60,6 +63,7 @@ describe('Community packages (Public API)', () => {
|
|||
|
||||
beforeEach(async () => {
|
||||
vi.resetAllMocks();
|
||||
communityPackagesConfig.unverifiedEnabled = true;
|
||||
communityPackagesService.withLoadStatus.mockImplementation((packages) => packages);
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(mockedVettedPackage);
|
||||
await testDb.truncate(['User']);
|
||||
|
|
@ -248,12 +252,10 @@ describe('Community packages (Public API)', () => {
|
|||
createdAt: pkg.createdAt.toISOString(),
|
||||
updatedAt: pkg.updatedAt.toISOString(),
|
||||
});
|
||||
// No version requested, so install() pins the latest vetted version:
|
||||
// the same catalog lookup provides both it and the checksum.
|
||||
expect(communityPackagesService.installPackage).toHaveBeenCalledWith(
|
||||
parsedNpmPackageName.packageName,
|
||||
mockedVettedPackage.npmVersion,
|
||||
mockedVettedPackage.checksum,
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -272,12 +274,13 @@ describe('Community packages (Public API)', () => {
|
|||
expect(response.status).toBe(200);
|
||||
expect(communityPackagesService.installPackage).toHaveBeenCalledWith(
|
||||
parsedNpmPackageName.packageName,
|
||||
mockedVettedPackage.npmVersion,
|
||||
mockedVettedPackage.checksum,
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it('should install with verification disabled when verify is false', async () => {
|
||||
it('should install the latest vetted version when unverified packages are disabled', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
const pkg = mockPackage();
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue(parsedNpmPackageName);
|
||||
communityPackagesService.findInstalledPackage.mockResolvedValue(null);
|
||||
|
|
@ -287,18 +290,18 @@ describe('Community packages (Public API)', () => {
|
|||
const response = await testServer
|
||||
.publicApiAgentFor(owner)
|
||||
.post('/community-packages')
|
||||
.send({ name: mockPackageName(), verify: false });
|
||||
.send({ name: mockPackageName() });
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(communityNodeTypesService.findVetted).not.toHaveBeenCalled();
|
||||
expect(communityPackagesService.installPackage).toHaveBeenCalledWith(
|
||||
parsedNpmPackageName.packageName,
|
||||
undefined,
|
||||
undefined,
|
||||
mockedVettedPackage.npmVersion,
|
||||
mockedVettedPackage.checksum,
|
||||
);
|
||||
});
|
||||
|
||||
it('should return 400 when package is not vetted', async () => {
|
||||
it('should return 400 when unverified packages are disabled and the package is not vetted', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
communityNodeTypesService.findVetted.mockResolvedValue(undefined);
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue(parsedNpmPackageName);
|
||||
|
||||
|
|
@ -344,18 +347,32 @@ describe('Community packages (Public API)', () => {
|
|||
expect(communityPackagesService.updatePackage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return 400 when update options are invalid', async () => {
|
||||
it('should update without a checksum and ignore a verify field when unverified packages are enabled', async () => {
|
||||
const pkg = mockPackage();
|
||||
communityPackagesService.findInstalledPackage.mockResolvedValue(pkg);
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue({
|
||||
packageName: pkg.packageName,
|
||||
rawString: pkg.packageName,
|
||||
});
|
||||
communityPackagesService.updatePackage.mockResolvedValue(pkg);
|
||||
|
||||
const response = await testServer
|
||||
.publicApiAgentFor(owner)
|
||||
.patch(`/community-packages/${encodeURIComponent(mockPackageName())}`)
|
||||
.send({ verify: 'false' });
|
||||
.patch(`/community-packages/${encodeURIComponent(pkg.packageName)}`)
|
||||
.send({ version: COMMUNITY_PACKAGE_VERSION.UPDATED, verify: true });
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
expect(response.body.message).toContain('verify');
|
||||
expect(communityPackagesService.updatePackage).not.toHaveBeenCalled();
|
||||
expect(response.status).toBe(200);
|
||||
expect(communityNodeTypesService.findVetted).not.toHaveBeenCalled();
|
||||
expect(communityPackagesService.updatePackage).toHaveBeenCalledWith(
|
||||
pkg.packageName,
|
||||
pkg,
|
||||
COMMUNITY_PACKAGE_VERSION.UPDATED,
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it('should return 200 when package is updated successfully', async () => {
|
||||
it('should update with the vetted checksum when unverified packages are disabled', async () => {
|
||||
communityPackagesConfig.unverifiedEnabled = false;
|
||||
const packageName = mockPackageName();
|
||||
const pkg = mockPackage({ packageName });
|
||||
const updatedNode = mockNode(packageName);
|
||||
|
|
@ -406,6 +423,10 @@ describe('Community packages (Public API)', () => {
|
|||
it('should return 404 when package is not installed', async () => {
|
||||
const name = mockPackageName();
|
||||
communityPackagesService.findInstalledPackage.mockResolvedValue(null);
|
||||
communityPackagesService.parseNpmPackageName.mockReturnValue({
|
||||
packageName: name,
|
||||
rawString: name,
|
||||
});
|
||||
|
||||
const response = await testServer
|
||||
.publicApiAgentFor(owner)
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ import { WorkflowPublicationStatusService } from '@/workflows/publication/workfl
|
|||
import { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service';
|
||||
import type { WorkflowPublishGuardProxy } from '@/workflows/workflow-publish-guard-proxy.service';
|
||||
import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
|
||||
import { ErrorWorkflowValidationService } from '@/workflows/error-workflow-validation.service';
|
||||
import { WorkflowService } from '@/workflows/workflow.service';
|
||||
|
||||
import { createCustomRoleWithScopeSlugs, cleanupRolesAndScopes } from '../shared/db/roles';
|
||||
|
|
@ -129,6 +130,7 @@ beforeAll(async () => {
|
|||
Container.get(PolicyEnforcementService), // policyEnforcementService
|
||||
Container.get(WorkflowPublicationStatusService), // workflowPublicationStatusService
|
||||
Container.get(NodeGroupRulesFlagGate), // nodeGroupRulesFlagGate
|
||||
Container.get(ErrorWorkflowValidationService), // errorWorkflowValidationService
|
||||
);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import {
|
|||
createActiveWorkflow,
|
||||
setActiveVersion,
|
||||
createWorkflowWithHistory,
|
||||
createWorkflowHistory,
|
||||
shareWorkflowWithProjects,
|
||||
shareWorkflowWithUsers,
|
||||
randomCredentialPayload,
|
||||
|
|
@ -5732,3 +5733,126 @@ describe('POST /workflows/with-node-types', () => {
|
|||
expect(response.body.count).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('PATCH /workflows/:id - error workflow reference', () => {
|
||||
const ERROR_TRIGGER_TYPE = 'n8n-nodes-base.errorTrigger';
|
||||
|
||||
/** A published handler with an active Error Trigger, owned by `ownerUser`. */
|
||||
const createErrorHandler = async (ownerUser: User, name: string) => {
|
||||
const handler = await createWorkflow(
|
||||
{
|
||||
name,
|
||||
nodes: [
|
||||
{
|
||||
id: uuid(),
|
||||
name: 'Error Trigger',
|
||||
type: ERROR_TRIGGER_TYPE,
|
||||
typeVersion: 1,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
settings: { callerPolicy: 'any' },
|
||||
},
|
||||
ownerUser,
|
||||
);
|
||||
await createWorkflowHistory(handler, ownerUser);
|
||||
await setActiveVersion(handler.id, handler.versionId);
|
||||
return handler;
|
||||
};
|
||||
|
||||
/** A victim workflow the member may edit but does not own. */
|
||||
const createSharedVictimWorkflow = async () => {
|
||||
const victim = await createWorkflowWithHistory({ name: 'Victim' }, owner);
|
||||
await shareWorkflowWithUsers(victim, [member]);
|
||||
return victim;
|
||||
};
|
||||
|
||||
test('rejects pointing a workflow at an error workflow the caller cannot read', async () => {
|
||||
const victim = await createSharedVictimWorkflow();
|
||||
const foreignHandler = await createErrorHandler(anotherMember, 'Foreign Handler');
|
||||
|
||||
const response = await authMemberAgent
|
||||
.patch(`/workflows/${victim.id}`)
|
||||
.send({ settings: { errorWorkflow: foreignHandler.id } });
|
||||
|
||||
expect(response.statusCode).toBe(400);
|
||||
expect(response.body.message).toContain('does not exist or you do not have access to it');
|
||||
|
||||
const stored = await workflowRepository.findOneBy({ id: victim.id });
|
||||
expect(stored?.settings?.errorWorkflow).toBeUndefined();
|
||||
});
|
||||
|
||||
test('rejects an error workflow that has no published version', async () => {
|
||||
const victim = await createSharedVictimWorkflow();
|
||||
const draftHandler = await createWorkflow({ name: 'Draft Handler' }, member);
|
||||
|
||||
const response = await authMemberAgent
|
||||
.patch(`/workflows/${victim.id}`)
|
||||
.send({ settings: { errorWorkflow: draftHandler.id } });
|
||||
|
||||
expect(response.statusCode).toBe(400);
|
||||
expect(response.body.message).toContain('has no published version');
|
||||
});
|
||||
|
||||
test('rejects a published error workflow with no Error Trigger', async () => {
|
||||
const victim = await createSharedVictimWorkflow();
|
||||
const handler = await createWorkflowWithHistory({ name: 'Plain Handler' }, member);
|
||||
await setActiveVersion(handler.id, handler.versionId);
|
||||
|
||||
const response = await authMemberAgent
|
||||
.patch(`/workflows/${victim.id}`)
|
||||
.send({ settings: { errorWorkflow: handler.id } });
|
||||
|
||||
expect(response.statusCode).toBe(400);
|
||||
expect(response.body.message).toContain('no active Error Trigger node');
|
||||
});
|
||||
|
||||
test('accepts a published, readable error workflow with an Error Trigger', async () => {
|
||||
const victim = await createSharedVictimWorkflow();
|
||||
const handler = await createErrorHandler(member, 'Own Handler');
|
||||
|
||||
const response = await authMemberAgent
|
||||
.patch(`/workflows/${victim.id}`)
|
||||
.send({ settings: { errorWorkflow: handler.id } });
|
||||
|
||||
expect(response.statusCode).toBe(200);
|
||||
|
||||
const stored = await workflowRepository.findOneBy({ id: victim.id });
|
||||
expect(stored?.settings?.errorWorkflow).toBe(handler.id);
|
||||
});
|
||||
|
||||
// The setting is unversioned, so re-checking an unchanged value would make a
|
||||
// workflow whose handler was since archived or restricted impossible to save.
|
||||
test('still saves a workflow whose existing error workflow is no longer valid', async () => {
|
||||
const victim = await createWorkflowWithHistory(
|
||||
{ name: 'Victim', settings: { errorWorkflow: 'long-gone-workflow-id' } },
|
||||
owner,
|
||||
);
|
||||
await shareWorkflowWithUsers(victim, [member]);
|
||||
|
||||
const response = await authMemberAgent
|
||||
.patch(`/workflows/${victim.id}`)
|
||||
.send({ settings: { errorWorkflow: 'long-gone-workflow-id', timezone: 'UTC' } });
|
||||
|
||||
expect(response.statusCode).toBe(200);
|
||||
});
|
||||
|
||||
test('allows clearing the reference', async () => {
|
||||
const victim = await createWorkflowWithHistory(
|
||||
{ name: 'Victim', settings: { errorWorkflow: 'long-gone-workflow-id' } },
|
||||
owner,
|
||||
);
|
||||
await shareWorkflowWithUsers(victim, [member]);
|
||||
|
||||
const response = await authMemberAgent
|
||||
.patch(`/workflows/${victim.id}`)
|
||||
.send({ settings: { errorWorkflow: 'DEFAULT' } });
|
||||
|
||||
expect(response.statusCode).toBe(200);
|
||||
|
||||
const stored = await workflowRepository.findOneBy({ id: victim.id });
|
||||
expect(stored?.settings?.errorWorkflow).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -937,8 +937,9 @@ describe('establishExecutionContext', () => {
|
|||
|
||||
const errorContext = runExecutionData.executionData!.runtimeData!;
|
||||
|
||||
// Should inherit credentials from failed workflow
|
||||
expect(errorContext.credentials).toBe('original-workflow-credentials');
|
||||
// Must NOT inherit the failed run's identity carrier: the error workflow
|
||||
// reports the failure, it does not act as that run's user.
|
||||
expect(errorContext.credentials).toBeUndefined();
|
||||
|
||||
// Should have fresh establishedAt
|
||||
expect(errorContext.establishedAt).toBeGreaterThanOrEqual(beforeTimestamp);
|
||||
|
|
@ -1039,8 +1040,9 @@ describe('establishExecutionContext', () => {
|
|||
|
||||
const errorContext = runExecutionData.executionData!.runtimeData!;
|
||||
|
||||
// Should inherit root credentials
|
||||
expect(errorContext.credentials).toBe('root-workflow-credentials');
|
||||
// The carrier is dropped at every depth, so a failure nested under a
|
||||
// sub-workflow cannot hand the root run's identity on either.
|
||||
expect(errorContext.credentials).toBeUndefined();
|
||||
|
||||
// Should track the failed sub-workflow
|
||||
expect(errorContext.parentExecutionId).toBe('failed-sub-workflow-id');
|
||||
|
|
@ -1051,6 +1053,167 @@ describe('establishExecutionContext', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('error workflow identity isolation', () => {
|
||||
// `executeErrorWorkflow` populates BOTH `runExecutionData.parentExecution`
|
||||
// and the start item's metadata, and the first wins. Each case below drives
|
||||
// one of the two branches so neither can regress on its own.
|
||||
const sealedParentContext: IExecutionContext = {
|
||||
version: 1,
|
||||
establishedAt: 2000000000,
|
||||
source: 'webhook',
|
||||
credentials: 'sealed-identity-carrier',
|
||||
secureArtifacts: 'stripped-trigger-values',
|
||||
redaction: { version: 2, production: true, manual: false },
|
||||
executedByUserId: 'user-who-ran-the-failed-workflow',
|
||||
usesDynamicCredentials: true,
|
||||
};
|
||||
|
||||
const errorTriggerNode = mock<INode>({
|
||||
name: 'ErrorTrigger',
|
||||
type: 'n8n-nodes-base.errorTrigger',
|
||||
});
|
||||
|
||||
const buildErrorRun = (source: 'parentExecution' | 'startItemMetadata') => {
|
||||
const parentExecution: RelatedExecution = {
|
||||
executionId: 'failed-execution-id',
|
||||
workflowId: 'failed-workflow-id',
|
||||
executionContext: sealedParentContext,
|
||||
};
|
||||
|
||||
return createRunExecutionData({
|
||||
startData: {},
|
||||
resultData: { runData: {} },
|
||||
executionData: {
|
||||
contextData: {},
|
||||
nodeExecutionStack: [
|
||||
{
|
||||
node: errorTriggerNode,
|
||||
data: { main: [[{ json: { error: 'boom' } }]] },
|
||||
source: null,
|
||||
...(source === 'startItemMetadata' && { metadata: { parentExecution } }),
|
||||
},
|
||||
],
|
||||
metadata: {},
|
||||
waitingExecution: {},
|
||||
waitingExecutionSource: {},
|
||||
},
|
||||
...(source === 'parentExecution' && { parentExecution }),
|
||||
});
|
||||
};
|
||||
|
||||
it.each(['parentExecution', 'startItemMetadata'] as const)(
|
||||
'does not pass the identity carrier to an error workflow (via %s)',
|
||||
async (source) => {
|
||||
const runExecutionData = buildErrorRun(source);
|
||||
|
||||
await establishExecutionContext(
|
||||
mockWorkflow,
|
||||
runExecutionData,
|
||||
mockAdditionalData,
|
||||
'error',
|
||||
);
|
||||
|
||||
const errorContext = runExecutionData.executionData!.runtimeData!;
|
||||
|
||||
expect(errorContext.credentials).toBeUndefined();
|
||||
expect(errorContext.secureArtifacts).toBeUndefined();
|
||||
},
|
||||
);
|
||||
|
||||
it.each(['parentExecution', 'startItemMetadata'] as const)(
|
||||
'still inherits the fields the error execution needs (via %s)',
|
||||
async (source) => {
|
||||
const runExecutionData = buildErrorRun(source);
|
||||
|
||||
await establishExecutionContext(
|
||||
mockWorkflow,
|
||||
runExecutionData,
|
||||
mockAdditionalData,
|
||||
'error',
|
||||
);
|
||||
|
||||
const errorContext = runExecutionData.executionData!.runtimeData!;
|
||||
|
||||
// Redaction escalates top-down, so a failure of a redacted run must not
|
||||
// produce an unredacted error record.
|
||||
expect(errorContext.redaction).toEqual(sealedParentContext.redaction);
|
||||
expect(errorContext.usesDynamicCredentials).toBe(true);
|
||||
expect(errorContext.executedByUserId).toBe('user-who-ran-the-failed-workflow');
|
||||
expect(errorContext.parentExecutionId).toBe('failed-execution-id');
|
||||
expect(errorContext.source).toBe('error');
|
||||
},
|
||||
);
|
||||
|
||||
it('keeps passing the carrier to a sub-workflow, which is a different relationship', async () => {
|
||||
const runExecutionData = buildErrorRun('parentExecution');
|
||||
|
||||
// Same shape, but an 'integrated' run: a sub-workflow the parent's own
|
||||
// graph calls, not a handler named in an unversioned setting.
|
||||
await establishExecutionContext(
|
||||
mockWorkflow,
|
||||
runExecutionData,
|
||||
mockAdditionalData,
|
||||
'integrated',
|
||||
);
|
||||
|
||||
const childContext = runExecutionData.executionData!.runtimeData!;
|
||||
|
||||
expect(childContext.credentials).toBe('sealed-identity-carrier');
|
||||
expect(childContext.secureArtifacts).toBe('stripped-trigger-values');
|
||||
});
|
||||
|
||||
// The two branches above are skipped when the run data already carries a
|
||||
// context — a persisted error run that resumes after a Wait, or the second
|
||||
// establish call once the main process has populated `runtimeData`.
|
||||
it('strips a carrier from a context the error run already carries', async () => {
|
||||
const runExecutionData = buildErrorRun('parentExecution');
|
||||
runExecutionData.executionData!.runtimeData = { ...sealedParentContext };
|
||||
|
||||
await establishExecutionContext(mockWorkflow, runExecutionData, mockAdditionalData, 'error');
|
||||
|
||||
const errorContext = runExecutionData.executionData!.runtimeData!;
|
||||
|
||||
expect(errorContext.credentials).toBeUndefined();
|
||||
expect(errorContext.secureArtifacts).toBeUndefined();
|
||||
// The rest of the established context survives untouched — this is the
|
||||
// error run's own context, not an inheritance point.
|
||||
expect(errorContext.establishedAt).toBe(sealedParentContext.establishedAt);
|
||||
expect(errorContext.source).toBe('webhook');
|
||||
expect(errorContext.redaction).toEqual(sealedParentContext.redaction);
|
||||
expect(errorContext.executedByUserId).toBe('user-who-ran-the-failed-workflow');
|
||||
expect(errorContext.usesDynamicCredentials).toBe(true);
|
||||
});
|
||||
|
||||
it('leaves an already-established context alone for a non-error resume', async () => {
|
||||
const runExecutionData = buildErrorRun('parentExecution');
|
||||
runExecutionData.executionData!.runtimeData = { ...sealedParentContext };
|
||||
|
||||
await establishExecutionContext(
|
||||
mockWorkflow,
|
||||
runExecutionData,
|
||||
mockAdditionalData,
|
||||
'webhook',
|
||||
);
|
||||
|
||||
expect(runExecutionData.executionData!.runtimeData).toEqual(sealedParentContext);
|
||||
});
|
||||
|
||||
// `encryptedRunnerIdentity` is a carrier, so it cannot be filtered — and it
|
||||
// is spread onto the context AFTER the inheritance gate, so refusing it at
|
||||
// the source is the only place the gate holds.
|
||||
it('does not take the runner identity from additionalData on an error run', async () => {
|
||||
const runExecutionData = buildErrorRun('parentExecution');
|
||||
const additionalData = mock<IWorkflowExecuteAdditionalData>({
|
||||
encryptedRunnerIdentity: 'encrypted-credential-blob',
|
||||
executionId: undefined,
|
||||
});
|
||||
|
||||
await establishExecutionContext(mockWorkflow, runExecutionData, additionalData, 'error');
|
||||
|
||||
expect(runExecutionData.executionData!.runtimeData!.credentials).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('context propagation priority and edge cases', () => {
|
||||
it('should prefer runExecutionData.parentExecution over startItem.metadata', async () => {
|
||||
// Both sources provide context, runExecutionData.parentExecution should win
|
||||
|
|
|
|||
|
|
@ -1,6 +1,12 @@
|
|||
import { Logger } from '@n8n/backend-common';
|
||||
import { Container } from '@n8n/di';
|
||||
import { type IRunExecutionData, type Workflow, type WorkflowExecuteMode } from 'n8n-workflow';
|
||||
import {
|
||||
toErrorWorkflowContext,
|
||||
type IExecutionContext,
|
||||
type IRunExecutionData,
|
||||
type Workflow,
|
||||
type WorkflowExecuteMode,
|
||||
} from 'n8n-workflow';
|
||||
|
||||
import { assertExecutionDataExists, type PreExecutionAdditionalData } from '@/utils/assertions';
|
||||
|
||||
|
|
@ -27,9 +33,10 @@ import { ExecutionContextService } from './execution-context.service';
|
|||
* The function follows a priority-based approach to establish execution context:
|
||||
*
|
||||
* ### 1. Preserve Existing Context (Webhook Resume)
|
||||
* If `executionData.runtimeData` already exists, the function returns immediately without
|
||||
* modification. This preserves context when workflows resume from database (e.g., after
|
||||
* waiting for a webhook or manual continuation).
|
||||
* If `executionData.runtimeData` already exists, the function returns immediately, keeping
|
||||
* that context. This preserves context when workflows resume from database (e.g., after
|
||||
* waiting for a webhook or manual continuation). An error run still passes its context
|
||||
* through {@link contextAllowedFor}, which the branches below cannot reach.
|
||||
*
|
||||
* ### 2. Inherit from Parent Execution (Sub-workflows)
|
||||
* If `runExecutionData.parentExecution` exists, creates a new context by inheriting all
|
||||
|
|
@ -38,12 +45,12 @@ import { ExecutionContextService } from './execution-context.service';
|
|||
* - `source`: Set to current execution mode
|
||||
* - `parentExecutionId`: Tracks the parent execution ID
|
||||
*
|
||||
* This applies to sub-workflows invoked via "Execute Workflow" node.
|
||||
* This applies to sub-workflows invoked via "Execute Workflow" node, and to error
|
||||
* workflows — `executeErrorWorkflow` populates `parentExecution` as well.
|
||||
*
|
||||
* ### 3. Inherit from Start Node Metadata (Error Workflows)
|
||||
* ### 3. Inherit from Start Node Metadata
|
||||
* If `startItem.metadata.parentExecution.executionContext` exists, creates a new context
|
||||
* by inheriting from the parent context. This applies to error workflows that need to
|
||||
* preserve the original workflow's context.
|
||||
* by inheriting from the parent context. Reached only when branch 2 did not apply.
|
||||
*
|
||||
* ### 4. Create Fresh Context (New Executions)
|
||||
* For new root executions, creates a fresh context with:
|
||||
|
|
@ -56,7 +63,8 @@ import { ExecutionContextService } from './execution-context.service';
|
|||
*
|
||||
* ## Context Inheritance Pattern
|
||||
* When inheriting context, the strategy is:
|
||||
* 1. Spread all parent context fields (credentials, custom fields, etc.)
|
||||
* 1. Spread the inheritable parent context fields (see {@link contextAllowedFor}:
|
||||
* everything for a sub-workflow, non-identity fields only for an error workflow)
|
||||
* 2. Override `establishedAt` with current timestamp
|
||||
* 3. Override `source` with current execution mode
|
||||
* 4. Add `parentExecutionId` to track lineage
|
||||
|
|
@ -101,6 +109,35 @@ import { ExecutionContextService } from './execution-context.service';
|
|||
* @see IWorkflowExecuteAdditionalData for additional execution data
|
||||
* @see RelatedExecution for parent execution context propagation
|
||||
*/
|
||||
/**
|
||||
* Second gate on the identity carrier reaching an error workflow.
|
||||
*
|
||||
* `executeErrorWorkflow` already strips the carrier when it builds the error
|
||||
* payload. This refuses one even if a caller passes it, so the boundary does not
|
||||
* depend on every present and future call site getting it right. Every way a
|
||||
* context enters an error run passes through here: the two inheritance branches
|
||||
* below (`executeErrorWorkflow` populates both `parentExecution` and the start
|
||||
* item's metadata) and a context the run data already carries, which the early
|
||||
* return would otherwise hand on unfiltered.
|
||||
*
|
||||
* The remaining source, `additionalData.encryptedRunnerIdentity`, cannot be
|
||||
* filtered — it IS a carrier — so an error run must not read it at all.
|
||||
*/
|
||||
function contextAllowedFor(
|
||||
context: IExecutionContext,
|
||||
mode: WorkflowExecuteMode,
|
||||
): IExecutionContext;
|
||||
function contextAllowedFor(
|
||||
context: IExecutionContext | undefined,
|
||||
mode: WorkflowExecuteMode,
|
||||
): IExecutionContext | undefined;
|
||||
function contextAllowedFor(
|
||||
context: IExecutionContext | undefined,
|
||||
mode: WorkflowExecuteMode,
|
||||
): IExecutionContext | undefined {
|
||||
return mode === 'error' ? toErrorWorkflowContext(context) : context;
|
||||
}
|
||||
|
||||
export const establishExecutionContext = async (
|
||||
workflow: Workflow,
|
||||
runExecutionData: IRunExecutionData,
|
||||
|
|
@ -123,8 +160,12 @@ export const establishExecutionContext = async (
|
|||
// A retry reloads the original run's data under a NEW executionId, so its
|
||||
// id must join the sealed carrier's path (allowInherit) or resolution would
|
||||
// reject it. Resume keeps the same id, so the bind stays a no-op regardless.
|
||||
//
|
||||
// Filter first: an error run that already has a context (resumed from the
|
||||
// database, or established by an earlier call on the main process) skips
|
||||
// the inheritance branches below, so this is where its gate has to sit.
|
||||
executionData.runtimeData = await executionContextService.maybeBindExecutionId(
|
||||
executionData.runtimeData,
|
||||
contextAllowedFor(executionData.runtimeData, mode),
|
||||
additionalData?.executionId,
|
||||
{ allowInherit: mode === 'retry' },
|
||||
);
|
||||
|
|
@ -140,7 +181,10 @@ export const establishExecutionContext = async (
|
|||
source: mode,
|
||||
};
|
||||
|
||||
if (additionalData?.encryptedRunnerIdentity) {
|
||||
// An error workflow reports a failure; it never acts as the failed run's user.
|
||||
// A carrier handed in here would land on the context after the inheritance
|
||||
// branches spread it, so it has to be refused at the source.
|
||||
if (mode !== 'error' && additionalData?.encryptedRunnerIdentity) {
|
||||
executionData.runtimeData.credentials = additionalData.encryptedRunnerIdentity;
|
||||
if (executionData.runtimeData.credentials) {
|
||||
executionData.runtimeData = await executionContextService.maybeBindExecutionId(
|
||||
|
|
@ -157,7 +201,7 @@ export const establishExecutionContext = async (
|
|||
// and the mode in which it is running, while still retaining all other contextual information
|
||||
// from the parent execution.
|
||||
executionData.runtimeData = {
|
||||
...(runExecutionData.parentExecution.executionContext ?? {}),
|
||||
...(contextAllowedFor(runExecutionData.parentExecution.executionContext, mode) ?? {}),
|
||||
...executionData.runtimeData,
|
||||
parentExecutionId: runExecutionData.parentExecution.executionId,
|
||||
};
|
||||
|
|
@ -210,7 +254,7 @@ export const establishExecutionContext = async (
|
|||
// and can inherit context from there
|
||||
if (startItem.metadata?.parentExecution?.executionContext) {
|
||||
executionData.runtimeData = {
|
||||
...startItem.metadata.parentExecution.executionContext,
|
||||
...contextAllowedFor(startItem.metadata.parentExecution.executionContext, mode),
|
||||
...executionData.runtimeData,
|
||||
parentExecutionId: startItem.metadata.parentExecution.executionId,
|
||||
};
|
||||
|
|
@ -225,11 +269,10 @@ export const establishExecutionContext = async (
|
|||
|
||||
// Re-run the global sub-execution hooks against the child workflow so its own
|
||||
// record reflects context derived from itself (redaction policy, private-
|
||||
// credential flag), merged with the inherited parent context. An error
|
||||
// workflow reaches inheritance through this branch, so without this a
|
||||
// policy'd or private-credential error workflow called by a policy-less
|
||||
// parent would not redact its own record. Mirrors the
|
||||
// `runExecutionData.parentExecution` branch above.
|
||||
// credential flag), merged with the inherited parent context. Without this a
|
||||
// policy'd or private-credential child called by a policy-less parent would
|
||||
// not redact its own record. Mirrors the `runExecutionData.parentExecution`
|
||||
// branch above.
|
||||
executionData.runtimeData = await executionContextService.augmentSubExecutionContext(
|
||||
workflow,
|
||||
startItem,
|
||||
|
|
|
|||
|
|
@ -14,10 +14,9 @@ export async function getInstalledCommunityNodes(
|
|||
export async function installNewPackage(
|
||||
context: IRestApiContext,
|
||||
name: string,
|
||||
verify?: boolean,
|
||||
version?: string,
|
||||
): Promise<PublicInstalledPackage> {
|
||||
return await post(context.baseUrl, '/community-packages', { name, verify, version });
|
||||
return await post(context.baseUrl, '/community-packages', { name, version });
|
||||
}
|
||||
|
||||
export async function uninstallPackage(context: IRestApiContext, name: string): Promise<void> {
|
||||
|
|
@ -28,13 +27,8 @@ export async function updatePackage(
|
|||
context: IRestApiContext,
|
||||
name: string,
|
||||
version?: string,
|
||||
checksum?: string,
|
||||
): Promise<PublicInstalledPackage> {
|
||||
return await makeRestApiRequest(context, 'PATCH', '/community-packages', {
|
||||
name,
|
||||
version,
|
||||
checksum,
|
||||
});
|
||||
return await makeRestApiRequest(context, 'PATCH', '/community-packages', { name, version });
|
||||
}
|
||||
|
||||
export async function getAvailableCommunityPackageCount(): Promise<number> {
|
||||
|
|
|
|||
|
|
@ -57,17 +57,8 @@ export const useCommunityNodesStore = defineStore(STORES.COMMUNITY_NODES, () =>
|
|||
}, timeout);
|
||||
};
|
||||
|
||||
const installPackage = async (
|
||||
packageName: string,
|
||||
verify?: boolean,
|
||||
version?: string,
|
||||
): Promise<void> => {
|
||||
await communityNodesApi.installNewPackage(
|
||||
rootStore.restApiContext,
|
||||
packageName,
|
||||
verify,
|
||||
version,
|
||||
);
|
||||
const installPackage = async (packageName: string, version?: string): Promise<void> => {
|
||||
await communityNodesApi.installNewPackage(rootStore.restApiContext, packageName, version);
|
||||
await fetchInstalledPackages();
|
||||
};
|
||||
|
||||
|
|
@ -85,17 +76,12 @@ export const useCommunityNodesStore = defineStore(STORES.COMMUNITY_NODES, () =>
|
|||
installedPackages.value[newPackage.packageName] = newPackage;
|
||||
};
|
||||
|
||||
const updatePackage = async (
|
||||
packageName: string,
|
||||
version?: string,
|
||||
checksum?: string,
|
||||
): Promise<void> => {
|
||||
const updatePackage = async (packageName: string, version?: string): Promise<void> => {
|
||||
const packageToUpdate = installedPackages.value[packageName];
|
||||
const updatedPackage = await communityNodesApi.updatePackage(
|
||||
rootStore.restApiContext,
|
||||
packageToUpdate.packageName,
|
||||
version,
|
||||
checksum,
|
||||
);
|
||||
updatePackageObject(updatedPackage);
|
||||
};
|
||||
|
|
|
|||
|
|
@ -122,11 +122,7 @@ describe('CommunityPackageManageConfirmModal', () => {
|
|||
await flushPromises();
|
||||
await fireEvent.click(getByRole('button', { name: 'Confirm update' }));
|
||||
|
||||
expect(communityNodesStore.updatePackage).toHaveBeenCalledWith(
|
||||
'n8n-nodes-test',
|
||||
'2.0.5',
|
||||
'correct-checksum',
|
||||
);
|
||||
expect(communityNodesStore.updatePackage).toHaveBeenCalledWith('n8n-nodes-test', '2.0.5');
|
||||
});
|
||||
|
||||
it('should call nodeTypesStore methods and update latestVerifiedVersion on mount', async () => {
|
||||
|
|
|
|||
|
|
@ -160,11 +160,7 @@ const onUpdate = async () => {
|
|||
if (settingsStore.isUnverifiedPackagesEnabled) {
|
||||
await communityNodesStore.updatePackage(props.activePackageName);
|
||||
} else if (settingsStore.isCommunityNodesFeatureEnabled) {
|
||||
await communityNodesStore.updatePackage(
|
||||
props.activePackageName,
|
||||
updateVersion.value,
|
||||
nodeTypeStorePackage.value?.checksum,
|
||||
);
|
||||
await communityNodesStore.updatePackage(props.activePackageName, updateVersion.value);
|
||||
} else {
|
||||
throw new Error('Community nodes feature is not correctly enabled.');
|
||||
}
|
||||
|
|
|
|||
|
|
@ -163,7 +163,7 @@ describe('CommunityNodeDetails', () => {
|
|||
await waitFor(() => expect(removeNodeFromMergedNodes).toHaveBeenCalled());
|
||||
|
||||
expect(getCommunityNodeAttributes).toHaveBeenCalledWith('n8n-nodes-preview-test.OtherNode');
|
||||
expect(installPackage).toHaveBeenCalledWith('n8n-nodes-test', true, '1.0.0');
|
||||
expect(installPackage).toHaveBeenCalledWith('n8n-nodes-test', '1.0.0');
|
||||
expect(fetchCredentialTypes).toHaveBeenCalledWith(true);
|
||||
expect(getAllNodeCreateElements).toHaveBeenCalled();
|
||||
expect(popViewStack).toHaveBeenCalled();
|
||||
|
|
|
|||
|
|
@ -204,11 +204,7 @@ describe('useInstallNode', () => {
|
|||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(communityNodesStore.installPackage).toHaveBeenCalledWith(
|
||||
'test-package',
|
||||
true,
|
||||
'1.0.0',
|
||||
);
|
||||
expect(communityNodesStore.installPackage).toHaveBeenCalledWith('test-package', '1.0.0');
|
||||
});
|
||||
|
||||
it('should install verified node with npm version', async () => {
|
||||
|
|
@ -221,11 +217,7 @@ describe('useInstallNode', () => {
|
|||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(communityNodesStore.installPackage).toHaveBeenCalledWith(
|
||||
'test-package',
|
||||
true,
|
||||
'1.0.0',
|
||||
);
|
||||
expect(communityNodesStore.installPackage).toHaveBeenCalledWith('test-package', '1.0.0');
|
||||
expect(nodeTypesStore.getNodeTypes).toHaveBeenCalled();
|
||||
expect(nodeTypesStore.fetchCommunityNodePreviews).toHaveBeenCalled();
|
||||
expect(credentialsStore.fetchCredentialTypes).toHaveBeenCalledWith(true);
|
||||
|
|
@ -251,11 +243,7 @@ describe('useInstallNode', () => {
|
|||
|
||||
expect(result.success).toBe(true);
|
||||
expect(nodeTypesStore.getCommunityNodeAttributes).toHaveBeenCalledWith('test-node');
|
||||
expect(communityNodesStore.installPackage).toHaveBeenCalledWith(
|
||||
'test-package',
|
||||
true,
|
||||
'1.0.0',
|
||||
);
|
||||
expect(communityNodesStore.installPackage).toHaveBeenCalledWith('test-package', '1.0.0');
|
||||
});
|
||||
|
||||
it('should install verified node as latest when unverified packages are enabled', async () => {
|
||||
|
|
@ -444,11 +432,7 @@ describe('useInstallNode', () => {
|
|||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(communityNodesStore.installPackage).toHaveBeenCalledWith(
|
||||
'test-package',
|
||||
true,
|
||||
undefined,
|
||||
);
|
||||
expect(communityNodesStore.installPackage).toHaveBeenCalledWith('test-package', undefined);
|
||||
});
|
||||
|
||||
it('should handle getNpmVersion errors', async () => {
|
||||
|
|
@ -570,11 +554,7 @@ describe('useInstallNode', () => {
|
|||
});
|
||||
|
||||
expect(nodeTypesStore.getCommunityNodeAttributes).toHaveBeenCalledWith('test-node');
|
||||
expect(communityNodesStore.installPackage).toHaveBeenCalledWith(
|
||||
'test-package',
|
||||
true,
|
||||
'2.0.0',
|
||||
);
|
||||
expect(communityNodesStore.installPackage).toHaveBeenCalledWith('test-package', '2.0.0');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -80,7 +80,6 @@ export function useInstallNode() {
|
|||
if (props.type === 'verified' && !settingsStore.isUnverifiedPackagesEnabled) {
|
||||
await communityNodesStore.installPackage(
|
||||
props.packageName,
|
||||
true,
|
||||
await getNpmVersion(props.nodeType),
|
||||
);
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -266,6 +266,45 @@ export const ExecutionContextSchema = z
|
|||
*/
|
||||
export type IExecutionContext = z.output<typeof ExecutionContextSchema>;
|
||||
|
||||
/**
|
||||
* The subset of a failed run's context that an error workflow may inherit.
|
||||
*
|
||||
* An error workflow reports a failure; it does not continue the failed run's
|
||||
* session. So it must not act as that run's user: the failing workflow names its
|
||||
* handler in `settings.errorWorkflow`, and whoever can write that setting would
|
||||
* otherwise choose which workflow gets to borrow the identity.
|
||||
*
|
||||
* Built as an allow-list rather than by deleting known-secret keys, so a field
|
||||
* added to `IExecutionContext` later has to be listed here before it can cross
|
||||
* the boundary.
|
||||
*/
|
||||
export function toErrorWorkflowContext(context: IExecutionContext): IExecutionContext;
|
||||
export function toErrorWorkflowContext(
|
||||
context: IExecutionContext | undefined,
|
||||
): IExecutionContext | undefined;
|
||||
export function toErrorWorkflowContext(
|
||||
context: IExecutionContext | undefined,
|
||||
): IExecutionContext | undefined {
|
||||
if (!context) return undefined;
|
||||
|
||||
return {
|
||||
version: context.version,
|
||||
establishedAt: context.establishedAt,
|
||||
source: context.source,
|
||||
...(context.triggerNode !== undefined && { triggerNode: context.triggerNode }),
|
||||
...(context.parentExecutionId !== undefined && {
|
||||
parentExecutionId: context.parentExecutionId,
|
||||
}),
|
||||
...(context.redaction !== undefined && { redaction: context.redaction }),
|
||||
...(context.executedByUserId !== undefined && {
|
||||
executedByUserId: context.executedByUserId,
|
||||
}),
|
||||
...(context.usesDynamicCredentials !== undefined && {
|
||||
usesDynamicCredentials: context.usesDynamicCredentials,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Metadata shape for the `n8n-oauth` credential-context source.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -1,4 +1,8 @@
|
|||
import { toExecutionContext } from '../src/execution-context';
|
||||
import {
|
||||
toErrorWorkflowContext,
|
||||
toExecutionContext,
|
||||
type IExecutionContext,
|
||||
} from '../src/execution-context';
|
||||
|
||||
describe('toExecutionContext — redaction snapshot', () => {
|
||||
const baseContext = {
|
||||
|
|
@ -72,3 +76,73 @@ describe('toExecutionContext — redaction snapshot', () => {
|
|||
).toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('toErrorWorkflowContext', () => {
|
||||
const fullContext: IExecutionContext = {
|
||||
version: 1,
|
||||
establishedAt: 1234567890,
|
||||
source: 'webhook',
|
||||
triggerNode: { name: 'Webhook', type: 'n8n-nodes-base.webhook' },
|
||||
parentExecutionId: 'parent-1',
|
||||
credentials: 'encrypted-identity-carrier',
|
||||
secureArtifacts: 'encrypted-secure-artifacts',
|
||||
redaction: { version: 2, production: true, manual: false },
|
||||
executedByUserId: 'user-1',
|
||||
usesDynamicCredentials: true,
|
||||
};
|
||||
|
||||
it('drops the identity carrier and the secure artifacts', () => {
|
||||
const result = toErrorWorkflowContext(fullContext);
|
||||
|
||||
expect(result).not.toHaveProperty('credentials');
|
||||
expect(result).not.toHaveProperty('secureArtifacts');
|
||||
});
|
||||
|
||||
it('keeps the fields the error execution needs for its own record', () => {
|
||||
expect(toErrorWorkflowContext(fullContext)).toEqual({
|
||||
version: 1,
|
||||
establishedAt: 1234567890,
|
||||
source: 'webhook',
|
||||
triggerNode: { name: 'Webhook', type: 'n8n-nodes-base.webhook' },
|
||||
parentExecutionId: 'parent-1',
|
||||
redaction: { version: 2, production: true, manual: false },
|
||||
executedByUserId: 'user-1',
|
||||
usesDynamicCredentials: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('carries no key that the allow-list does not name', () => {
|
||||
// The point of the allow-list: a field added to IExecutionContext must be
|
||||
// listed before it can reach an error workflow. A deny-list would leak it.
|
||||
const withUnknownField = {
|
||||
...fullContext,
|
||||
someFutureSecret: 'must-not-cross',
|
||||
} as IExecutionContext;
|
||||
|
||||
expect(toErrorWorkflowContext(withUnknownField)).not.toHaveProperty('someFutureSecret');
|
||||
});
|
||||
|
||||
it('omits optional fields the parent did not set, rather than setting them undefined', () => {
|
||||
const result = toErrorWorkflowContext({
|
||||
version: 1,
|
||||
establishedAt: 1234567890,
|
||||
source: 'trigger',
|
||||
});
|
||||
|
||||
expect(Object.keys(result!)).toEqual(['version', 'establishedAt', 'source']);
|
||||
});
|
||||
|
||||
it('keeps `usesDynamicCredentials: false` rather than dropping it', () => {
|
||||
const result = toErrorWorkflowContext({ ...fullContext, usesDynamicCredentials: false });
|
||||
|
||||
expect(result).toHaveProperty('usesDynamicCredentials', false);
|
||||
});
|
||||
|
||||
it('returns undefined when there is no context', () => {
|
||||
expect(toErrorWorkflowContext(undefined)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('produces a context that still parses as an execution context', () => {
|
||||
expect(() => toExecutionContext(toErrorWorkflowContext(fullContext)!)).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user