fix(core): Filter stale credentials from setup wizard requests (#28478)

This commit is contained in:
Albert Alises 2026-04-20 14:37:51 +02:00 committed by GitHub
parent 2d0b231e31
commit 657bdf136f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 463 additions and 30 deletions

View file

@ -7,6 +7,7 @@ import {
applyNodeChanges,
buildCompletedReport,
createCredentialCache,
stripStaleCredentialsFromWorkflow,
} from '../setup-workflow.service';
// ---------------------------------------------------------------------------
@ -357,6 +358,102 @@ describe('buildSetupRequests', () => {
expect(context.credentialService.list).toHaveBeenCalledTimes(1);
});
it('does not generate credential request for HTTP Request with auth=none and stale node.credentials', async () => {
(context.nodeService as unknown as Record<string, unknown>).getNodeCredentialTypes = jest
.fn()
.mockResolvedValue([]);
(context.nodeService.getDescription as jest.Mock).mockResolvedValue({
group: [],
credentials: [
{
name: 'httpHeaderAuth',
displayOptions: { show: { authentication: ['genericCredentialType'] } },
},
],
});
(context.credentialService.list as jest.Mock).mockResolvedValue([]);
const node = makeNode({
name: 'HTTP Request',
type: 'n8n-nodes-base.httpRequest',
typeVersion: 4.4,
parameters: { authentication: 'none', url: 'https://api.example.com' },
credentials: { httpHeaderAuth: { id: 'old-cred', name: 'Stale Header Auth' } },
});
const result = await buildSetupRequests(context, node);
expect(result.find((r) => r.credentialType === 'httpHeaderAuth')).toBeUndefined();
});
it('fallback: displayOptions filtering takes priority over stale node.credentials', async () => {
// Remove getNodeCredentialTypes to force fallback path
delete (context.nodeService as unknown as Record<string, unknown>).getNodeCredentialTypes;
(context.nodeService.getDescription as jest.Mock).mockResolvedValue({
group: [],
credentials: [
{
name: 'httpHeaderAuth',
displayOptions: { show: { authentication: ['genericCredentialType'] } },
},
],
});
(context.credentialService.list as jest.Mock).mockResolvedValue([]);
const node = makeNode({
name: 'HTTP Request',
type: 'n8n-nodes-base.httpRequest',
typeVersion: 4.4,
parameters: { authentication: 'none', url: 'https://api.example.com' },
credentials: { httpHeaderAuth: { id: 'old-cred', name: 'Stale Header Auth' } },
});
const result = await buildSetupRequests(context, node);
expect(result.find((r) => r.credentialType === 'httpHeaderAuth')).toBeUndefined();
});
it('fallback: node with assigned credentials matching description is still detected', async () => {
(context.nodeService as unknown as Record<string, unknown>).getNodeCredentialTypes = jest
.fn()
.mockResolvedValue([]);
(context.nodeService.getDescription as jest.Mock).mockResolvedValue({
group: [],
credentials: [{ name: 'slackApi' }],
});
(context.credentialService.list as jest.Mock).mockResolvedValue([
{ id: 'cred-1', name: 'My Slack', updatedAt: '2025-01-01T00:00:00.000Z' },
]);
const node = makeNode({
credentials: { slackApi: { id: 'cred-1', name: 'My Slack' } },
});
const result = await buildSetupRequests(context, node);
expect(result.find((r) => r.credentialType === 'slackApi')).toBeDefined();
});
it('fallback: node.credentials with types not in description are excluded', async () => {
// Remove getNodeCredentialTypes to force fallback path
delete (context.nodeService as unknown as Record<string, unknown>).getNodeCredentialTypes;
(context.nodeService.getDescription as jest.Mock).mockResolvedValue({
group: [],
credentials: [{ name: 'slackApi' }],
});
(context.credentialService.list as jest.Mock).mockResolvedValue([
{ id: 'cred-1', name: 'My Slack', updatedAt: '2025-01-01T00:00:00.000Z' },
]);
const node = makeNode({
credentials: {
slackApi: { id: 'cred-1', name: 'My Slack' },
httpHeaderAuth: { id: 'stale', name: 'Stale Auth' },
},
});
const result = await buildSetupRequests(context, node);
expect(result.find((r) => r.credentialType === 'slackApi')).toBeDefined();
expect(result.find((r) => r.credentialType === 'httpHeaderAuth')).toBeUndefined();
});
it('treats placeholder values as parameter issues', async () => {
(context.nodeService.getDescription as jest.Mock).mockResolvedValue({
group: [],
@ -571,6 +668,101 @@ describe('applyNodeChanges', () => {
expect(result.failed).toHaveLength(1);
expect(result.failed[0].error).toContain('Failed to save workflow');
});
it('strips credentials not valid for the current parameters', async () => {
const node = makeNode({
name: 'HTTP Request',
type: 'n8n-nodes-base.httpRequest',
typeVersion: 4.4,
parameters: { authentication: 'none', url: 'https://api.example.com' },
credentials: { httpHeaderAuth: { id: 'stale', name: 'Stale Header Auth' } },
});
const wfJson = makeWorkflowJSON([node]);
(context.workflowService.getAsWorkflowJSON as jest.Mock).mockResolvedValue(wfJson);
(context.nodeService.getDescription as jest.Mock).mockResolvedValue({
group: [],
credentials: [
{
name: 'httpHeaderAuth',
displayOptions: { show: { authentication: ['genericCredentialType'] } },
},
],
});
(context.workflowService.updateFromWorkflowJSON as jest.Mock).mockResolvedValue(undefined);
await applyNodeChanges(context, 'wf-1');
const calls = (context.workflowService.updateFromWorkflowJSON as jest.Mock).mock.calls as Array<
[string, WorkflowJSON]
>;
const savedJson = calls[0][1];
const savedNode = savedJson.nodes.find((n) => n.name === 'HTTP Request');
expect(savedNode?.credentials).toBeUndefined();
});
it('preserves just-applied credentials even if description would exclude them', async () => {
const node = makeNode({
name: 'HTTP Request',
type: 'n8n-nodes-base.httpRequest',
typeVersion: 4.4,
parameters: { authentication: 'none' },
});
const wfJson = makeWorkflowJSON([node]);
(context.workflowService.getAsWorkflowJSON as jest.Mock).mockResolvedValue(wfJson);
(context.nodeService.getDescription as jest.Mock).mockResolvedValue({
group: [],
credentials: [],
});
(context.credentialService.get as jest.Mock).mockResolvedValue({
id: 'cred-1',
name: 'My Header Auth',
});
(context.workflowService.updateFromWorkflowJSON as jest.Mock).mockResolvedValue(undefined);
await applyNodeChanges(context, 'wf-1', {
'HTTP Request': { httpHeaderAuth: 'cred-1' },
});
const calls = (context.workflowService.updateFromWorkflowJSON as jest.Mock).mock.calls as Array<
[string, WorkflowJSON]
>;
const savedJson = calls[0][1];
const savedNode = savedJson.nodes.find((n) => n.name === 'HTTP Request');
expect(savedNode?.credentials).toEqual({
httpHeaderAuth: { id: 'cred-1', name: 'My Header Auth' },
});
});
it('keeps credentials matching description displayOptions', async () => {
const node = makeNode({
name: 'HTTP Request',
type: 'n8n-nodes-base.httpRequest',
typeVersion: 4.4,
parameters: {
authentication: 'genericCredentialType',
genericAuthType: 'httpHeaderAuth',
},
credentials: { httpHeaderAuth: { id: 'cred-1', name: 'Header Auth' } },
});
const wfJson = makeWorkflowJSON([node]);
(context.workflowService.getAsWorkflowJSON as jest.Mock).mockResolvedValue(wfJson);
(context.nodeService.getDescription as jest.Mock).mockResolvedValue({
group: [],
credentials: [],
});
(context.workflowService.updateFromWorkflowJSON as jest.Mock).mockResolvedValue(undefined);
await applyNodeChanges(context, 'wf-1');
const calls = (context.workflowService.updateFromWorkflowJSON as jest.Mock).mock.calls as Array<
[string, WorkflowJSON]
>;
const savedJson = calls[0][1];
const savedNode = savedJson.nodes.find((n) => n.name === 'HTTP Request');
expect(savedNode?.credentials).toEqual({
httpHeaderAuth: { id: 'cred-1', name: 'Header Auth' },
});
});
});
// ---------------------------------------------------------------------------
@ -607,3 +799,117 @@ describe('buildCompletedReport', () => {
expect(report).toHaveLength(0);
});
});
// ---------------------------------------------------------------------------
// stripStaleCredentialsFromWorkflow
// ---------------------------------------------------------------------------
describe('stripStaleCredentialsFromWorkflow', () => {
let context: InstanceAiContext;
beforeEach(() => {
context = createMockContext();
});
it('removes credential entries that no longer match the node parameters', async () => {
const node = makeNode({
name: 'HTTP Request',
type: 'n8n-nodes-base.httpRequest',
typeVersion: 4.4,
parameters: { authentication: 'none', url: 'https://api.example.com' },
credentials: { httpHeaderAuth: { id: 'stale', name: 'Stale Header Auth' } },
});
const wfJson = makeWorkflowJSON([node]);
(context.nodeService.getDescription as jest.Mock).mockResolvedValue({
group: [],
credentials: [
{
name: 'httpHeaderAuth',
displayOptions: { show: { authentication: ['genericCredentialType'] } },
},
],
});
await stripStaleCredentialsFromWorkflow(context, wfJson);
expect(wfJson.nodes[0].credentials).toBeUndefined();
});
it('keeps credential entries that match the current parameters', async () => {
const node = makeNode({
name: 'HTTP Request',
type: 'n8n-nodes-base.httpRequest',
typeVersion: 4.4,
parameters: {
authentication: 'genericCredentialType',
genericAuthType: 'httpHeaderAuth',
},
credentials: { httpHeaderAuth: { id: 'cred-1', name: 'Header Auth' } },
});
const wfJson = makeWorkflowJSON([node]);
(context.nodeService.getDescription as jest.Mock).mockResolvedValue({
group: [],
credentials: [],
});
await stripStaleCredentialsFromWorkflow(context, wfJson);
expect(wfJson.nodes[0].credentials).toEqual({
httpHeaderAuth: { id: 'cred-1', name: 'Header Auth' },
});
});
it('strips per-node — clean nodes are unaffected, stale nodes are scrubbed', async () => {
const cleanNode = makeNode({
name: 'OpenRouter',
type: 'n8n-nodes-base.httpRequest',
typeVersion: 4.4,
parameters: {
authentication: 'genericCredentialType',
genericAuthType: 'httpHeaderAuth',
},
credentials: { httpHeaderAuth: { id: 'cred-1', name: 'OpenRouter Auth' } },
});
const staleNode = makeNode({
name: 'Joke API',
id: 'node-2',
type: 'n8n-nodes-base.httpRequest',
typeVersion: 4.4,
parameters: { authentication: 'none', url: 'https://icanhazdadjoke.com/' },
credentials: { httpHeaderAuth: { id: 'cred-1', name: 'OpenRouter Auth' } },
});
const wfJson = makeWorkflowJSON([cleanNode, staleNode]);
(context.nodeService.getDescription as jest.Mock).mockResolvedValue({
group: [],
credentials: [
{
name: 'httpHeaderAuth',
displayOptions: { show: { authentication: ['genericCredentialType'] } },
},
],
});
await stripStaleCredentialsFromWorkflow(context, wfJson);
expect(wfJson.nodes[0].credentials).toEqual({
httpHeaderAuth: { id: 'cred-1', name: 'OpenRouter Auth' },
});
expect(wfJson.nodes[1].credentials).toBeUndefined();
});
it('is a no-op for nodes without credentials', async () => {
const node = makeNode({
parameters: { authentication: 'none' },
});
const wfJson = makeWorkflowJSON([node]);
(context.nodeService.getDescription as jest.Mock).mockResolvedValue({
group: [],
credentials: [],
});
await stripStaleCredentialsFromWorkflow(context, wfJson);
expect(wfJson.nodes[0].credentials).toBeUndefined();
expect(context.nodeService.getDescription).not.toHaveBeenCalled();
});
});

View file

@ -3,6 +3,7 @@ import { generateWorkflowCode, layoutWorkflowJSON } from '@n8n/workflow-sdk';
import { z } from 'zod';
import { buildCredentialMap, resolveCredentials } from './resolve-credentials';
import { stripStaleCredentialsFromWorkflow } from './setup-workflow.service';
import { ensureWebhookIds } from './submit-workflow.tool';
import type { InstanceAiContext } from '../../types';
import { parseAndValidate, partitionWarnings } from '../../workflow-builder';
@ -153,6 +154,12 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
const credentialMap = await buildCredentialMap(context.credentialService);
await resolveCredentials(json, workflowId, context, credentialMap);
// Strip credential entries that are no longer valid for the current
// parameters. Resolution above (and the LLM itself) can re-emit stale
// references between turns; without this, setup analysis would surface
// a credential request for a node that no longer needs one.
await stripStaleCredentialsFromWorkflow(context, json);
// Ensure webhook nodes have a webhookId so n8n registers clean paths
await ensureWebhookIds(json, workflowId, context);

View file

@ -6,7 +6,7 @@
* state machine, and this logic is testable independently.
*/
import { findPlaceholderDetails } from '@n8n/utils';
import type { IDataObject, NodeJSON, DisplayOptions } from '@n8n/workflow-sdk';
import type { IDataObject, NodeJSON, DisplayOptions, WorkflowJSON } from '@n8n/workflow-sdk';
import { matchesDisplayOptions } from '@n8n/workflow-sdk';
import { nanoid } from 'nanoid';
@ -31,6 +31,122 @@ export function createCredentialCache(): CredentialCache {
// ── Node analysis ───────────────────────────────────────────────────────────
/**
* Compute the set of credential types valid for a node given its current
* parameters. Mirrors the resolution in `buildSetupRequests`: consults the
* node service's dynamic resolver first, then falls back to the description's
* static credentials filtered by displayOptions, plus the dynamic types
* implied by `authentication: genericCredentialType | predefinedCredentialType`.
*/
export async function getValidCredentialTypes(
context: InstanceAiContext,
node: NodeJSON,
): Promise<Set<string>> {
const typeVersion = node.typeVersion ?? 1;
const parameters = (node.parameters as Record<string, unknown>) ?? {};
let nodeDesc: Awaited<ReturnType<typeof context.nodeService.getDescription>> | undefined;
try {
nodeDesc = await context.nodeService.getDescription(node.type, typeVersion);
} catch {
nodeDesc = undefined;
}
const types = new Set<string>();
if (context.nodeService.getNodeCredentialTypes) {
try {
const dynamic = await context.nodeService.getNodeCredentialTypes(
node.type,
typeVersion,
parameters,
node.credentials as Record<string, unknown> | undefined,
);
for (const t of dynamic) types.add(t);
} catch (error) {
// Falling through to description-based detection is safe, but the dynamic
// resolver isn't expected to throw — log so we can investigate if it does.
context.logger?.warn(
'[setup-workflow] getNodeCredentialTypes threw during credential validation',
{
nodeType: node.type,
typeVersion,
error: error instanceof Error ? error.message : String(error),
},
);
}
}
if (nodeDesc?.credentials) {
for (const c of nodeDesc.credentials as Array<{ name?: string; displayOptions?: unknown }>) {
if (!c.name) continue;
if (!c.displayOptions) {
types.add(c.name);
continue;
}
if (
matchesDisplayOptions(
{ parameters, nodeVersion: typeVersion },
c.displayOptions as DisplayOptions,
)
) {
types.add(c.name);
}
}
}
const authentication = parameters.authentication;
if (
authentication === 'genericCredentialType' &&
typeof parameters.genericAuthType === 'string'
) {
types.add(parameters.genericAuthType);
} else if (
authentication === 'predefinedCredentialType' &&
typeof parameters.nodeCredentialType === 'string'
) {
types.add(parameters.nodeCredentialType);
}
return types;
}
/**
* Drop credential entries from a node whose type is no longer valid for the
* node's current parameters (e.g. an `httpHeaderAuth` left over from an earlier
* builder revision after `authentication` was switched to `none`). Mutates
* `node.credentials` in place. Types in `preserveTypes` are kept regardless
* used by the apply path so a credential the user just assigned isn't stripped.
*/
export async function stripStaleCredentialsFromNode(
context: InstanceAiContext,
node: NodeJSON,
preserveTypes?: Set<string>,
): Promise<void> {
if (!node.credentials || Object.keys(node.credentials).length === 0) return;
const validTypes = await getValidCredentialTypes(context, node);
const cleaned: NonNullable<typeof node.credentials> = {};
for (const [credType, value] of Object.entries(node.credentials)) {
if (validTypes.has(credType) || preserveTypes?.has(credType)) {
cleaned[credType] = value;
}
}
node.credentials = Object.keys(cleaned).length > 0 ? cleaned : undefined;
}
/**
* Run {@link stripStaleCredentialsFromNode} over every node in a workflow.
* Intended to run after `resolveCredentials` in the builder save paths so the
* LLM can't persist stale credential references between turns.
*/
export async function stripStaleCredentialsFromWorkflow(
context: InstanceAiContext,
json: WorkflowJSON,
): Promise<void> {
await Promise.all(
(json.nodes ?? []).map(async (node) => await stripStaleCredentialsFromNode(context, node)),
);
}
/**
* Build setup request(s) from a single WorkflowJSON node.
* Detects credential types, auto-selects the most recent credential,
@ -120,27 +236,22 @@ export async function buildSetupRequests(
}
// Fallback: if dynamic detection returned nothing, check the node description's
// static credentials list and already-assigned credentials on the node.
// This catches cases where getNodeCredentialTypes fails silently (e.g. node
// lookup miss) or isn't available.
if (credentialTypes.length === 0) {
const nodeCredTypes = node.credentials ? Object.keys(node.credentials) : [];
if (nodeCredTypes.length > 0) {
credentialTypes = nodeCredTypes;
} else if (nodeDesc?.credentials) {
// Only include credentials whose displayOptions match the current parameters.
// Mirrors the evaluation in instance-ai.adapter.service.ts getNodeCredentialTypes().
credentialTypes = nodeDesc.credentials
.filter((c: { name?: string; displayOptions?: unknown }) => {
if (!c.displayOptions) return true;
return matchesDisplayOptions(
{ parameters, nodeVersion: typeVersion },
c.displayOptions as DisplayOptions,
);
})
.map((c: { name?: string }) => c.name)
.filter((n): n is string => n !== undefined);
}
// static credentials list with displayOptions filtering. This catches cases where
// getNodeCredentialTypes fails silently (e.g. node lookup miss) or isn't available.
// We intentionally do NOT fall back to node.credentials here — stale credential
// entries (e.g. httpHeaderAuth left over from an earlier builder revision when
// authentication was later changed to 'none') must not generate setup requests.
if (credentialTypes.length === 0 && nodeDesc?.credentials) {
credentialTypes = nodeDesc.credentials
.filter((c: { name?: string; displayOptions?: unknown }) => {
if (!c.displayOptions) return true;
return matchesDisplayOptions(
{ parameters, nodeVersion: typeVersion },
c.displayOptions as DisplayOptions,
);
})
.map((c: { name?: string }) => c.name)
.filter((n): n is string => n !== undefined);
}
// Dynamic credential resolution for nodes that use genericCredentialType
@ -559,6 +670,12 @@ export async function applyNodeChanges(
});
}
}
// Drop credential entries that are no longer valid for the node's current
// parameters. Entries just applied via credsMap are preserved so a user
// can assign an auxiliary credential without it being cleaned up.
const appliedTypes = new Set(credsMap ? Object.keys(credsMap) : []);
await stripStaleCredentialsFromNode(context, node, appliedTypes);
}
// Single save for all changes

View file

@ -15,6 +15,7 @@ import { createHash, randomUUID } from 'node:crypto';
import { z } from 'zod';
import { resolveCredentials, type CredentialMap } from './resolve-credentials';
import { stripStaleCredentialsFromWorkflow } from './setup-workflow.service';
import type { InstanceAiContext } from '../../types';
import type { ValidationWarning } from '../../workflow-builder';
import { partitionWarnings } from '../../workflow-builder';
@ -304,6 +305,12 @@ export function createSubmitWorkflowTool(
// Unresolved credentials are mocked via pinned data when available.
const mockResult = await resolveCredentials(json, workflowId, context, credentialMap);
// Strip credential entries that are no longer valid for the current
// parameters. Resolution above (and the LLM itself) can re-emit stale
// references between turns; without this, setup analysis would surface
// a credential request for a node that no longer needs one.
await stripStaleCredentialsFromWorkflow(context, json);
// Ensure webhook nodes have a webhookId so n8n registers clean paths
// (e.g., "{uuid}/dashboard" instead of "{workflowId}/{encodedNodeName}/dashboard").
// The SDK's toJSON() doesn't emit webhookId, so we inject it here.

View file

@ -532,6 +532,8 @@ export interface InstanceAiContext {
* Used to register `parse-file` and supply data to the parser.
*/
currentUserAttachments?: InstanceAiAttachment[];
/** Optional logger for diagnostics from domain tools. */
logger?: Logger;
}
// ── Task storage ─────────────────────────────────────────────────────────────

View file

@ -203,6 +203,7 @@ export class InstanceAiAdapterService {
webResearchService: this.createWebResearchAdapter(user, searchProxyConfig),
workspaceService: this.createWorkspaceAdapter(user),
licenseHints: this.buildLicenseHints(),
logger: this.logger,
};
}
@ -1689,7 +1690,7 @@ export class InstanceAiAdapterService {
return filteredIssues;
},
getNodeCredentialTypes: async (nodeType, typeVersion, parameters, existingCredentials) => {
getNodeCredentialTypes: async (nodeType, typeVersion, parameters, _existingCredentials) => {
const nodes = await getNodes();
const desc = findNodeByVersion(nodes, nodeType, typeVersion);
if (!desc) return [];
@ -1767,13 +1768,6 @@ export class InstanceAiAdapterService {
credentialTypes.add(parameters.nodeCredentialType as string);
}
// 4. Already-assigned credentials
if (existingCredentials) {
for (const credType of Object.keys(existingCredentials)) {
credentialTypes.add(credType);
}
}
return Array.from(credentialTypes);
},