From ccf42e3213251ec99ffbf1c3dda1d410ec76fa72 Mon Sep 17 00:00:00 2001 From: Bernhard Wittmann Date: Wed, 14 Oct 2026 06:24:28 +0000 Subject: [PATCH] fix(ai-builder): Make browser credential-setup evals grade the right thing (no-changelog) (#40858) --- packages/@n8n/api-types/src/index.ts | 1 + .../src/schemas/instance-ai.schema.ts | 12 ++ ...ld-workflow-credential-test-target.test.ts | 125 ++++++++++++++++++ ...ild-workflow-single-turn-questions.test.ts | 106 +++++++++++++++ .../credential-setup-lane-stand-in.test.ts | 46 +++++++ .../__tests__/fixture-server.test.ts | 10 +- .../evaluations/clients/n8n-client.ts | 11 ++ .../fixtures/providers/slack/manifest.json | 7 +- .../evaluations/harness/build-workflow.ts | 36 ++++- .../instance-ai.adapter.service.test.ts | 35 +++++ .../__tests__/instance-ai.controller.test.ts | 65 +++++++++ ...hread-credential-allowlist.service.test.ts | 41 ++++++ .../thread-credential-allowlist.service.ts | 27 +++- .../instance-ai.adapter.service.ts | 20 ++- .../instance-ai/instance-ai.controller.ts | 27 ++++ .../instance-ai/instance-ai.service.ts | 2 + .../credentials/SlackApi.credentials.ts | 9 +- .../test/SlackApi.credentials.test.ts | 27 ++++ 18 files changed, 594 insertions(+), 13 deletions(-) create mode 100644 packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-credential-test-target.test.ts create mode 100644 packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-single-turn-questions.test.ts create mode 100644 packages/@n8n/instance-ai/evaluations/__tests__/credential-setup-lane-stand-in.test.ts create mode 100644 packages/nodes-base/credentials/test/SlackApi.credentials.test.ts diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index eb0558f76c2..9a9626aabcf 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -502,6 +502,7 @@ export { InstanceAiEvalExecutionRequest, InstanceAiEvalAgentExecutionRequest, InstanceAiEvalCredentialAllowlistRequest, + InstanceAiEvalCredentialTestTargetRequest, INSTANCE_AI_MEMORY_TASK_WAIT_TIMEOUT_MS, AI_GATEWAY_MANAGED_TAG, InstanceAiEvalRestoreThreadRequest, diff --git a/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts b/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts index 221b56482ba..858037dacd1 100644 --- a/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts +++ b/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts @@ -3042,6 +3042,18 @@ export class InstanceAiEvalCredentialAllowlistRequest extends Z.class({ bypassCredentialTest: z.array(z.string().min(1)).max(50).optional(), }) {} +/** + * Aims the thread's connection tests for one credential type at a provider + * stand-in, so the eval checks the captured secret without the real provider. + */ +export class InstanceAiEvalCredentialTestTargetRequest extends Z.class({ + threadId: z.string().uuid(), + credentialType: z.string().min(1), + baseUrl: z.string().url(), + /** Credential field the stand-in URL is written into. */ + urlField: z.string().min(1), +}) {} + /** The id an authored seed gives a data table, agent or folder. ≥8 chars: the * restore remaps ids by whole-document string replace, and a short id would risk * corrupting unrelated substrings — so the restore path refuses shorter ids. diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-credential-test-target.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-credential-test-target.test.ts new file mode 100644 index 00000000000..95ced070aaa --- /dev/null +++ b/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-credential-test-target.test.ts @@ -0,0 +1,125 @@ +import { vi } from 'vitest'; + +import { N8nApiError, type N8nClient } from '../clients/n8n-client'; +import { buildWorkflow } from '../harness/build-workflow'; +import { + resolveCredentialSetupFixture, + startCredentialSetupLane, + type CredentialSetupLane, +} from '../harness/credential-setup-lane'; +import type { EvalLogger } from '../harness/logger'; + +vi.mock('../harness/chat-loop', async (importActual) => { + const actual = await importActual(); + return { + SSE_SETTLE_DELAY_MS: 0, + startSseConnection: vi.fn().mockResolvedValue(undefined), + waitForAllActivity: vi.fn().mockResolvedValue(undefined), + runMultiTurnConversation: vi.fn().mockResolvedValue(undefined), + recordUserTurn: vi.fn(), + buildAutoApprovePayload: actual.buildAutoApprovePayload, + }; +}); + +vi.mock('../harness/credential-setup-lane', async (importActual) => ({ + ...(await importActual()), + startCredentialSetupLane: vi.fn(), +})); + +vi.mock('../outcome/workflow-discovery', () => ({ + buildAgentOutcome: vi.fn().mockResolvedValue({ + workflowsCreated: [], + executionsRun: [], + dataTablesCreated: [], + finalText: 'done', + workflowJsons: [], + }), + extractWorkflowIdsFromMessages: vi.fn().mockReturnValue([]), +})); + +const silentLogger: EvalLogger = { + info: () => {}, + verbose: () => {}, + success: () => {}, + warn: () => {}, + error: () => {}, + isVerbose: false, +}; + +function makeClient(): N8nClient { + return { + getPersonalProjectId: vi.fn().mockResolvedValue('project-1'), + ensureThread: vi.fn().mockResolvedValue(undefined), + setThreadCredentialAllowlist: vi.fn().mockResolvedValue(undefined), + setThreadCredentialTestTarget: vi.fn().mockResolvedValue(undefined), + listCredentialIds: vi.fn().mockResolvedValue([]), + listCredentials: vi.fn().mockResolvedValue([]), + sendMessage: vi.fn().mockResolvedValue({ runId: 'run-1' }), + getThreadMessages: vi.fn().mockResolvedValue({ messages: [] }), + listWorkflows: vi.fn().mockResolvedValue([]), + } as unknown as N8nClient; +} + +async function buildWithLane( + lane: Partial, + client = makeClient(), +): Promise { + vi.mocked(startCredentialSetupLane).mockResolvedValue({ + local: false, + close: vi.fn().mockResolvedValue(undefined), + ...lane, + } as CredentialSetupLane); + await buildWorkflow({ + client, + skipWorkflowChecks: true, + preRunWorkflowIds: new Set(), + claimedWorkflowIds: new Set(), + logger: silentLogger, + conversation: [{ role: 'user', text: 'create a Slack credential' }], + credentialSetupSelection: await resolveCredentialSetupFixture({ credentialFixture: 'slack' }), + }); + return client; +} + +describe('buildWorkflow credential test target', () => { + it('aims the thread at the fixture stand-in before the first message', async () => { + const client = await buildWithLane({ + credentialType: 'slackApi', + verifyBaseUrl: 'http://stand-in.test:4100', + credentialUrlField: 'url', + }); + + expect(client.setThreadCredentialTestTarget).toHaveBeenCalledWith(expect.any(String), { + credentialType: 'slackApi', + baseUrl: 'http://stand-in.test:4100', + urlField: 'url', + }); + expect( + vi.mocked(client.setThreadCredentialTestTarget).mock.invocationCallOrder[0], + ).toBeLessThan(vi.mocked(client.sendMessage).mock.invocationCallOrder[0]); + }); + + it('still builds against a backend without the endpoint', async () => { + const client = makeClient(); + vi.mocked(client.setThreadCredentialTestTarget).mockRejectedValue( + new N8nApiError('Not found', 404), + ); + + await buildWithLane( + { + credentialType: 'slackApi', + verifyBaseUrl: 'http://stand-in.test:4100', + credentialUrlField: 'url', + }, + client, + ); + + expect(client.sendMessage).toHaveBeenCalled(); + }); + + it('sets no target when the fixture ships no stand-in', async () => { + const client = await buildWithLane({ credentialType: 'slackApi', credentialUrlField: 'url' }); + + expect(client.setThreadCredentialTestTarget).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-single-turn-questions.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-single-turn-questions.test.ts new file mode 100644 index 00000000000..51aff8835bd --- /dev/null +++ b/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-single-turn-questions.test.ts @@ -0,0 +1,106 @@ +import { vi } from 'vitest'; + +import type { N8nClient } from '../clients/n8n-client'; +import { buildWorkflow } from '../harness/build-workflow'; +import { waitForAllActivity, type ConfirmationStrategy } from '../harness/chat-loop'; +import type { EvalLogger } from '../harness/logger'; +import type { CapturedEvent } from '../types'; + +const proxyAnswer = { + kind: 'questions', + answers: [{ questionId: 'key_choice', selectedOptions: ['Create a new API key'] }], +}; + +vi.mock('../harness/chat-loop', async (importActual) => { + const actual = await importActual(); + return { + SSE_SETTLE_DELAY_MS: 0, + startSseConnection: vi.fn().mockResolvedValue(undefined), + waitForAllActivity: vi.fn().mockResolvedValue(undefined), + runMultiTurnConversation: vi.fn().mockResolvedValue(undefined), + recordUserTurn: vi.fn(), + buildAutoApprovePayload: actual.buildAutoApprovePayload, + }; +}); + +vi.mock('../utils/user-proxy', () => ({ + UserProxyLlm: class { + respondToConfirmation = vi.fn().mockResolvedValue(proxyAnswer); + ingestEvents = vi.fn(); + decideFollowUp = vi.fn().mockResolvedValue({ kind: 'done' }); + getDecisionStats = vi.fn().mockReturnValue({}); + }, +})); + +vi.mock('../outcome/workflow-discovery', () => ({ + buildAgentOutcome: vi.fn().mockResolvedValue({ + workflowsCreated: [], + executionsRun: [], + dataTablesCreated: [], + finalText: 'done', + workflowJsons: [], + }), + extractWorkflowIdsFromMessages: vi.fn().mockReturnValue([]), +})); + +const silentLogger: EvalLogger = { + info: () => {}, + verbose: () => {}, + success: () => {}, + warn: () => {}, + error: () => {}, + isVerbose: false, +}; + +function makeClient(): N8nClient { + return { + getPersonalProjectId: vi.fn().mockResolvedValue('project-1'), + ensureThread: vi.fn().mockResolvedValue(undefined), + setThreadCredentialAllowlist: vi.fn().mockResolvedValue(undefined), + sendMessage: vi.fn().mockResolvedValue({ runId: 'run-1' }), + getThreadMessages: vi.fn().mockResolvedValue({ messages: [] }), + listWorkflows: vi.fn().mockResolvedValue([]), + } as unknown as N8nClient; +} + +function confirmationEvent(payload: Record): CapturedEvent { + return { + type: 'confirmation-request', + data: { type: 'confirmation-request', payload: { requestId: 'req-1', ...payload } }, + } as unknown as CapturedEvent; +} + +async function singleTurnStrategy(): Promise { + await buildWorkflow({ + client: makeClient(), + skipWorkflowChecks: true, + preRunWorkflowIds: new Set(), + claimedWorkflowIds: new Set(), + logger: silentLogger, + conversation: [{ role: 'user', text: 'create a Gemini credential' }], + }); + const strategy = vi.mocked(waitForAllActivity).mock.calls[0]?.[0].confirmationStrategy; + if (!strategy) throw new Error('single-turn build passed no confirmation strategy'); + return strategy; +} + +describe('buildWorkflow single-turn confirmations', () => { + beforeEach(() => vi.mocked(waitForAllActivity).mockClear()); + + it('answers ask-user questions through the user proxy instead of leaving them blank', async () => { + const strategy = await singleTurnStrategy(); + const response = await strategy( + confirmationEvent({ + inputType: 'questions', + questions: [{ id: 'key_choice', question: 'Which key?', type: 'single' }], + }), + ); + expect(response).toEqual(proxyAnswer); + }); + + it('keeps auto-approving every other confirmation', async () => { + const strategy = await singleTurnStrategy(); + const response = await strategy(confirmationEvent({ message: 'Open aistudio.google.com' })); + expect(response).toEqual({ kind: 'approval', approved: true }); + }); +}); diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/credential-setup-lane-stand-in.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/credential-setup-lane-stand-in.test.ts new file mode 100644 index 00000000000..bc181518ce2 --- /dev/null +++ b/packages/@n8n/instance-ai/evaluations/__tests__/credential-setup-lane-stand-in.test.ts @@ -0,0 +1,46 @@ +import { vi } from 'vitest'; + +import type { N8nClient } from '../clients/n8n-client'; +import { startBrowserRuntime } from '../harness/browser-runtime'; +import { + resolveCredentialSetupFixture, + startCredentialSetupLane, +} from '../harness/credential-setup-lane'; +import type { EvalLogger } from '../harness/logger'; + +vi.mock('../harness/browser-runtime', () => ({ + startBrowserRuntime: vi.fn(), + attachToRunningBrowser: vi.fn(), +})); + +const silentLogger: EvalLogger = { + info: () => {}, + verbose: () => {}, + success: () => {}, + warn: () => {}, + error: () => {}, + isVerbose: false, +}; + +describe('startCredentialSetupLane stand-in', () => { + it('exposes the shipped fixture stand-in and its credential field', async () => { + vi.mocked(startBrowserRuntime).mockResolvedValue({ + close: vi.fn().mockResolvedValue(undefined), + } as unknown as Awaited>); + const selection = await resolveCredentialSetupFixture({ credentialFixture: 'slack' }); + + const lane = await startCredentialSetupLane({ + client: {} as N8nClient, + selection, + logger: silentLogger, + }); + + try { + expect(lane?.credentialType).toBe('slackApi'); + expect(lane?.credentialUrlField).toBe('url'); + expect(lane?.verifyBaseUrl).toMatch(/^http:\/\/.+:\d+$/); + } finally { + await lane?.close(); + } + }); +}); diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/fixture-server.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/fixture-server.test.ts index 5ea5cf9ea43..9bf7f1b15e0 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/fixture-server.test.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/fixture-server.test.ts @@ -91,9 +91,13 @@ describe('provider fixtures', () => { urlField: 'host', verify: { path: '/v1beta/models', query: 'key' }, }, - // No `verify`: slackApi's test hardcodes its baseURL, so the stand-in can - // never be aimed at it. The value check reports itself unverifiable. - { type: 'slackApi', id: 'slack', host: 'api.slack.com', prefix: 'xoxb-' }, + { + type: 'slackApi', + id: 'slack', + host: 'api.slack.com', + prefix: 'xoxb-', + verify: { path: '/api/users.profile.get', header: 'Authorization', scheme: 'Bearer' }, + }, ]; it.each(SHIPPED)('ships the $id fixture keyed on $type', async (row) => { diff --git a/packages/@n8n/instance-ai/evaluations/clients/n8n-client.ts b/packages/@n8n/instance-ai/evaluations/clients/n8n-client.ts index baf0f714b31..ff1b9522e56 100644 --- a/packages/@n8n/instance-ai/evaluations/clients/n8n-client.ts +++ b/packages/@n8n/instance-ai/evaluations/clients/n8n-client.ts @@ -1000,6 +1000,17 @@ export class N8nClient { }); } + /** Send the thread's connection tests for one credential type to a provider stand-in. */ + async setThreadCredentialTestTarget( + threadId: string, + target: { credentialType: string; baseUrl: string; urlField: string }, + ): Promise { + await this.fetch('/rest/instance-ai/eval/thread-credential-test-target', { + method: 'POST', + body: { threadId, ...target }, + }); + } + /** * Seed an existing thread with a previously exported conversation: the * referenced workflows are recreated (node credentials stripped server-side), diff --git a/packages/@n8n/instance-ai/evaluations/fixtures/providers/slack/manifest.json b/packages/@n8n/instance-ai/evaluations/fixtures/providers/slack/manifest.json index fc636b7e530..4fe9c78cb66 100644 --- a/packages/@n8n/instance-ai/evaluations/fixtures/providers/slack/manifest.json +++ b/packages/@n8n/instance-ai/evaluations/fixtures/providers/slack/manifest.json @@ -8,5 +8,10 @@ "/apps/:appId/install-on-team": "install.html", "/apps/:appId/oauth": "install.html" }, - "defaultRoute": "/apps" + "defaultRoute": "/apps", + "verify": { + "path": "/api/users.profile.get", + "header": "Authorization", + "scheme": "Bearer" + } } diff --git a/packages/@n8n/instance-ai/evaluations/harness/build-workflow.ts b/packages/@n8n/instance-ai/evaluations/harness/build-workflow.ts index 8f29b3d6170..f7384ba3dba 100644 --- a/packages/@n8n/instance-ai/evaluations/harness/build-workflow.ts +++ b/packages/@n8n/instance-ai/evaluations/harness/build-workflow.ts @@ -26,6 +26,7 @@ import { waitForAllActivity, runMultiTurnConversation, recordUserTurn, + buildAutoApprovePayload, type ConfirmationStrategy, } from './chat-loop'; import { runWorkflowChecks, summarizeMissingWorkflowError } from './cleanup'; @@ -95,6 +96,7 @@ import { lastAgentText, userTurnsAsText, } from '../utils/conversation-text'; +import { getEventPayload } from '../utils/confirmation-payload'; import { UserProxyLlm, type ProxyDecisionStats } from '../utils/user-proxy'; // --------------------------------------------------------------------------- @@ -871,12 +873,31 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise { + if (getEventPayload(event).inputType !== 'questions') { + return buildAutoApprovePayload(event); + } + questionProxy.ingestEvents(events); + return await questionProxy.respondToConfirmation(event); + }, }); } catch (error: unknown) { if (!(error instanceof RunTimeoutError)) throw error; diff --git a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts index 608f4984ab3..6aa56b89f1e 100644 --- a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts +++ b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts @@ -8261,6 +8261,41 @@ describe('createCredentialAdapter', () => { await expect(credentialService.test('cred-1')).rejects.toBe(refusal); expect(test).not.toHaveBeenCalled(); }); + + it('writes the stand-in URL into the credential field an eval target names', async () => { + const decryptForUse = vi.fn().mockResolvedValue({ apiKey: 'secret' }); + const test = vi.fn().mockResolvedValue({ status: 'OK', message: 'ok' }); + const { service, mockUser } = adapterWith(decryptForUse, test); + const { credentialService } = service.createContext(mockUser, { + getCredentialTestTarget: () => ({ + credentialType: 'slackApi', + baseUrl: 'http://stand-in.test:4100', + urlField: 'url', + }), + }); + + await credentialService.test('cred-1'); + + expect(test).toHaveBeenCalledWith( + mockUser.id, + expect.objectContaining({ + data: { apiKey: 'secret', url: 'http://stand-in.test:4100' }, + }), + ); + }); + + it('tests the stored data unchanged when the thread has no target', async () => { + const decryptForUse = vi.fn().mockResolvedValue({ apiKey: 'secret' }); + const test = vi.fn().mockResolvedValue({ status: 'OK', message: 'ok' }); + const { credentialService, mockUser } = adapterWith(decryptForUse, test); + + await credentialService.test('cred-1'); + + expect(test).toHaveBeenCalledWith( + mockUser.id, + expect.objectContaining({ data: { apiKey: 'secret' } }), + ); + }); }); describe('getCredentialFillState', () => { diff --git a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.controller.test.ts b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.controller.test.ts index b874220ddb4..81dcce4a100 100644 --- a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.controller.test.ts +++ b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.controller.test.ts @@ -46,6 +46,7 @@ import type { InstanceAiAdminSettingsUpdateRequest, InstanceAiEvalAgentExecutionResult, InstanceAiEvalCredentialAllowlistRequest, + InstanceAiEvalCredentialTestTargetRequest, InstanceAiEvalExecutionResult, InstanceAiEvalLlmUsage, InstanceAiEvalRestoreThreadRequest, @@ -1106,6 +1107,70 @@ describe('InstanceAiController', () => { }); }); + describe('setThreadCredentialTestTarget', () => { + const target = { + credentialType: 'slackApi', + baseUrl: 'http://stand-in.test:4100', + urlField: 'url', + }; + const payload = { threadId: THREAD_ID, ...target }; + + it('should require instanceAi:eval scope', () => { + expect(scopeOf('setThreadCredentialTestTarget')).toEqual({ + scope: 'instanceAi:eval', + globalOnly: true, + }); + }); + + beforeEach(() => { + globalConfig.instanceAi.evalInstance = true; + }); + + afterEach(() => { + globalConfig.instanceAi.evalInstance = false; + }); + + it('should set the test target for an owned thread', async () => { + memoryService.checkThreadOwnership.mockResolvedValue('owned'); + + const result = await controller.setThreadCredentialTestTarget( + req, + res, + payload as InstanceAiEvalCredentialTestTargetRequest, + ); + + expect(result).toEqual({ ok: true }); + expect(evalCredentialAllowlists.getTestTarget(THREAD_ID, 'slackApi')).toEqual(target); + }); + + it('should reject a thread that does not exist', async () => { + memoryService.checkThreadOwnership.mockResolvedValue('not_found'); + + await expect( + controller.setThreadCredentialTestTarget( + req, + res, + payload as InstanceAiEvalCredentialTestTargetRequest, + ), + ).rejects.toThrow(NotFoundError); + }); + + it('should refuse the target on an instance that is not an eval instance', async () => { + globalConfig.instanceAi.evalInstance = false; + evalCredentialAllowlists.clearThread(THREAD_ID); + memoryService.checkThreadOwnership.mockResolvedValue('owned'); + + await expect( + controller.setThreadCredentialTestTarget( + req, + res, + payload as InstanceAiEvalCredentialTestTargetRequest, + ), + ).rejects.toThrow(NotFoundError); + expect(evalCredentialAllowlists.getTestTarget(THREAD_ID, 'slackApi')).toBeUndefined(); + }); + }); + describe('getEvalThreadMemory', () => { const memory: InstanceAiEvalThreadMemoryResponse = { observations: [{ marker: 'critical', text: 'Posting via HTTP Request', tokenCount: 7 }], diff --git a/packages/cli/src/modules/instance-ai/eval/__tests__/thread-credential-allowlist.service.test.ts b/packages/cli/src/modules/instance-ai/eval/__tests__/thread-credential-allowlist.service.test.ts index c71404e821a..f15372c01b0 100644 --- a/packages/cli/src/modules/instance-ai/eval/__tests__/thread-credential-allowlist.service.test.ts +++ b/packages/cli/src/modules/instance-ai/eval/__tests__/thread-credential-allowlist.service.test.ts @@ -55,3 +55,44 @@ describe('EvalThreadCredentialAllowlistService', () => { expect(service.get('thread-1')).toBeUndefined(); }); }); + +describe('EvalThreadCredentialAllowlistService test targets', () => { + let service: EvalThreadCredentialAllowlistService; + const target = { + credentialType: 'slackApi', + baseUrl: 'http://stand-in.test:4100', + urlField: 'url', + }; + + beforeEach(() => { + service = new EvalThreadCredentialAllowlistService(); + }); + + it('returns the target for its credential type only', () => { + service.setTestTarget('thread-1', target); + + expect(service.getTestTarget('thread-1', 'slackApi')).toEqual(target); + expect(service.getTestTarget('thread-1', 'openAiApi')).toBeUndefined(); + expect(service.getTestTarget('thread-2', 'slackApi')).toBeUndefined(); + }); + + it('keeps a target for each credential type', () => { + const openAi = { + credentialType: 'openAiApi', + baseUrl: 'http://stand-in.test:4200', + urlField: 'url', + }; + service.setTestTarget('thread-1', target); + service.setTestTarget('thread-1', openAi); + + expect(service.getTestTarget('thread-1', 'slackApi')).toEqual(target); + expect(service.getTestTarget('thread-1', 'openAiApi')).toEqual(openAi); + }); + + it('forgets the target when the thread is cleared', () => { + service.setTestTarget('thread-1', target); + service.clearThread('thread-1'); + + expect(service.getTestTarget('thread-1', 'slackApi')).toBeUndefined(); + }); +}); diff --git a/packages/cli/src/modules/instance-ai/eval/thread-credential-allowlist.service.ts b/packages/cli/src/modules/instance-ai/eval/thread-credential-allowlist.service.ts index 064f6420409..9b609757e69 100644 --- a/packages/cli/src/modules/instance-ai/eval/thread-credential-allowlist.service.ts +++ b/packages/cli/src/modules/instance-ai/eval/thread-credential-allowlist.service.ts @@ -1,10 +1,19 @@ import { Service } from '@n8n/di'; +/** Where the thread's connection tests for one credential type go instead of + * the provider. See `InstanceAiEvalCredentialTestTargetRequest`. */ +export interface EvalCredentialTestTarget { + credentialType: string; + baseUrl: string; + urlField: string; +} + /** - * Per-thread credential visibility for evaluation runs. The eval harness + * Per-thread credential settings for evaluation runs. The eval harness * declares which credentials a build thread may see; the builder context's * credential `list()` is filtered to that set. Threads without an entry see - * the unfiltered instance listing. + * the unfiltered instance listing. The harness can also bypass a credential's + * connection test, or aim it at a provider stand-in. */ @Service() export class EvalThreadCredentialAllowlistService { @@ -13,6 +22,8 @@ export class EvalThreadCredentialAllowlistService { * without contacting the provider — see `bypassCredentialTest` on * `InstanceAiEvalCredentialAllowlistRequest`. */ private readonly testBypassByThread = new Map(); + /** Thread id → credential type → target. */ + private readonly testTargetsByThread = new Map>(); set(threadId: string, credentialIds: string[], bypassCredentialTest?: string[]): void { this.byThread.set(threadId, [...credentialIds]); @@ -41,8 +52,20 @@ export class EvalThreadCredentialAllowlistService { return this.testBypassByThread.get(threadId)?.includes(credentialId) ?? false; } + /** Replaces an earlier target for the same credential type only. */ + setTestTarget(threadId: string, target: EvalCredentialTestTarget): void { + const targets = this.testTargetsByThread.get(threadId) ?? new Map(); + targets.set(target.credentialType, { ...target }); + this.testTargetsByThread.set(threadId, targets); + } + + getTestTarget(threadId: string, credentialType: string): EvalCredentialTestTarget | undefined { + return this.testTargetsByThread.get(threadId)?.get(credentialType); + } + clearThread(threadId: string): void { this.byThread.delete(threadId); this.testBypassByThread.delete(threadId); + this.testTargetsByThread.delete(threadId); } } diff --git a/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts b/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts index 310e54cb345..4d757d32741 100644 --- a/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts +++ b/packages/cli/src/modules/instance-ai/instance-ai.adapter.service.ts @@ -171,6 +171,7 @@ import { scopeCredentialProvider, type AgentCredentialProvider, } from './eval/scoped-credential-provider'; +import type { EvalCredentialTestTarget } from './eval/thread-credential-allowlist.service'; import { InstanceAiBuilderDelegateAdapterService } from '@/modules/agents/instance-ai-builder-delegate.adapter'; import { DataTableRepository } from '@/modules/data-table/data-table.repository'; import { DataTableService } from '@/modules/data-table/data-table.service'; @@ -490,6 +491,9 @@ export class InstanceAiAdapterService { * contacting the provider. A predicate rather than a list because the * harness registers bypasses mid-run, after this context is built. */ shouldBypassCredentialTest?: (credentialId: string) => boolean; + /** Eval-only: the provider stand-in for a credential type's connection test. + * A getter for the same reason as the bypass predicate. */ + getCredentialTestTarget?: (credentialType: string) => EvalCredentialTestTarget | undefined; /** Pre-bound agent for the build-existing-agent flow. When omitted, the * assistant can create one via the build-agent tool. */ agentId?: string; @@ -532,6 +536,7 @@ export class InstanceAiAdapterService { getCredentialIdAllowlist, resumeAgentBuild = false, shouldBypassCredentialTest, + getCredentialTestTarget, agentId, configEvalsEnabled, setupPanelVariant, @@ -557,6 +562,7 @@ export class InstanceAiAdapterService { projectId, getCredentialIdAllowlist, shouldBypassCredentialTest, + getCredentialTestTarget, ); return { userId: user.id, @@ -2758,6 +2764,7 @@ export class InstanceAiAdapterService { boundProjectId?: string, getCredentialIdAllowlist?: () => string[] | undefined, shouldBypassCredentialTest?: (credentialId: string) => boolean, + getCredentialTestTarget?: (credentialType: string) => EvalCredentialTestTarget | undefined, ): InstanceAiCredentialService { const { credentialsService, @@ -2867,15 +2874,18 @@ export class InstanceAiAdapterService { return { success: true, message: 'Connection tested successfully' }; } + const data = await credentialsService.decryptForUse( + credential, + { kind: 'user', user }, + boundProjectId, + ); + // Eval-only: aim the test at the thread's stand-in. + const target = getCredentialTestTarget?.(credential.type); const credentialsToTest: ICredentialsDecrypted = { id: credential.id, name: credential.name, type: credential.type, - data: await credentialsService.decryptForUse( - credential, - { kind: 'user', user }, - boundProjectId, - ), + data: target ? { ...data, [target.urlField]: target.baseUrl } : data, }; const result = await credentialsService.test(user.id, credentialsToTest); diff --git a/packages/cli/src/modules/instance-ai/instance-ai.controller.ts b/packages/cli/src/modules/instance-ai/instance-ai.controller.ts index a6abe0cbaad..2b20cd46fb8 100644 --- a/packages/cli/src/modules/instance-ai/instance-ai.controller.ts +++ b/packages/cli/src/modules/instance-ai/instance-ai.controller.ts @@ -24,6 +24,7 @@ import { InstanceAiEvalExecutionRequest, InstanceAiEvalAgentExecutionRequest, InstanceAiEvalCredentialAllowlistRequest, + InstanceAiEvalCredentialTestTargetRequest, InstanceAiEvalRestoreThreadRequest, InstanceAiEvalSeedDataTableRowsRequest, findSeedFolderIssues, @@ -104,6 +105,8 @@ const KEEP_ALIVE_INTERVAL_MS = 15_000; export class InstanceAiController { private readonly gatewayApiKey: string; + private readonly instanceAiConfig: GlobalConfig['instanceAi']; + constructor( private readonly instanceAiService: InstanceAiService, private readonly gatewayService: InstanceAiGatewayService, @@ -133,6 +136,7 @@ export class InstanceAiController { private readonly threadTabsService: InstanceAiThreadTabsService, ) { this.gatewayApiKey = globalConfig.instanceAi.gatewayApiKey; + this.instanceAiConfig = globalConfig.instanceAi; } private requireInstanceAiEnabled(): void { @@ -1208,6 +1212,29 @@ export class InstanceAiController { return { ok: true }; } + /** + * Send the thread's connection tests for one credential type to a provider + * stand-in, so the agent's own test checks the captured secret. + */ + @Post('/eval/thread-credential-test-target') + @GlobalScope('instanceAi:eval') + async setThreadCredentialTestTarget( + req: AuthenticatedRequest, + _res: Response, + @Body payload: InstanceAiEvalCredentialTestTargetRequest, + ) { + this.requireInstanceAiEnabled(); + // Redirecting a credential test sends its secret elsewhere, so only a + // throwaway eval instance offers it. + if (!this.instanceAiConfig.evalInstance) { + throw new NotFoundError('Credential test targets are only available on an eval instance'); + } + await this.assertThreadAccess(req.user.id, payload.threadId); + const { threadId, ...target } = payload; + this.evalCredentialAllowlists.setTestTarget(threadId, target); + return { ok: true }; + } + /** * Observational memory for a thread, for a context eval to assert on. * diff --git a/packages/cli/src/modules/instance-ai/instance-ai.service.ts b/packages/cli/src/modules/instance-ai/instance-ai.service.ts index c291b9e0a73..b92a311ba28 100644 --- a/packages/cli/src/modules/instance-ai/instance-ai.service.ts +++ b/packages/cli/src/modules/instance-ai/instance-ai.service.ts @@ -2692,6 +2692,8 @@ export class InstanceAiService { getCredentialIdAllowlist: () => this.evalCredentialAllowlists.get(threadId), shouldBypassCredentialTest: (credentialId: string) => this.evalCredentialAllowlists.shouldBypassTest(threadId, credentialId), + getCredentialTestTarget: (credentialType: string) => + this.evalCredentialAllowlists.getTestTarget(threadId, credentialType), configEvalsEnabled, setupPanelVariant, mcpConnectionsAvailable, diff --git a/packages/nodes-base/credentials/SlackApi.credentials.ts b/packages/nodes-base/credentials/SlackApi.credentials.ts index 50bec0a4666..2a5008f85bd 100644 --- a/packages/nodes-base/credentials/SlackApi.credentials.ts +++ b/packages/nodes-base/credentials/SlackApi.credentials.ts @@ -57,6 +57,13 @@ export class SlackApi implements ICredentialType { type: 'hidden', default: '', }, + { + // Base URL of the connection test only, so an eval can aim the test at a stand-in. Node requests ignore it + displayName: 'Base URL', + name: 'url', + type: 'hidden', + default: 'https://slack.com', + }, { displayName: 'We strongly recommend setting up a signing secret to ensure the authenticity of requests.', @@ -82,7 +89,7 @@ export class SlackApi implements ICredentialType { test: ICredentialTestRequest = { request: { - baseURL: 'https://slack.com', + baseURL: '={{$credentials.url}}', url: '/api/users.profile.get', }, rules: [ diff --git a/packages/nodes-base/credentials/test/SlackApi.credentials.test.ts b/packages/nodes-base/credentials/test/SlackApi.credentials.test.ts new file mode 100644 index 00000000000..1c4147b8e75 --- /dev/null +++ b/packages/nodes-base/credentials/test/SlackApi.credentials.test.ts @@ -0,0 +1,27 @@ +import { NodeHelpers } from 'n8n-workflow'; + +import { SlackApi } from '../SlackApi.credentials'; + +describe('SlackApi Credential', () => { + const credential = new SlackApi(); + + it('should read the connection test base URL from the credential', () => { + expect(credential.test.request.baseURL).toBe('={{$credentials.url}}'); + expect(credential.test.request.url).toBe('/api/users.profile.get'); + }); + + // Credentials saved before the `url` field existed store no value for it. + // The tester fills defaults in with getNodeParameters before it resolves the request. + it('should test a stored credential without a url against slack.com', () => { + const data = NodeHelpers.getNodeParameters( + credential.properties, + { accessToken: 'xoxb-stored' }, + true, + false, + null, + null, + ); + + expect(data).toMatchObject({ accessToken: 'xoxb-stored', url: 'https://slack.com' }); + }); +});