fix(core): Validate AI builder credential IDs before save (#30070)
Some checks are pending
Build: Benchmark Image / build (push) Waiting to run
CI: Master (Build, Test, Lint) / Build for Github Cache (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (22.x) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (24.14.1) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (25.x) (push) Waiting to run
CI: Master (Build, Test, Lint) / Lint (push) Waiting to run
CI: Master (Build, Test, Lint) / Performance (push) Waiting to run
CI: Master (Build, Test, Lint) / Notify Slack on failure (push) Blocked by required conditions
Util: Sync API Docs / sync-public-api (push) Waiting to run

This commit is contained in:
Albert Alises 2026-05-08 13:29:12 +02:00 committed by GitHub
parent afe119be14
commit ceaebc6cbe
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 393 additions and 45 deletions

View File

@ -52,7 +52,11 @@ import {
import type { BuilderWorkspace } from '../../workspace/builder-sandbox-factory';
import { readFileViaSandbox } from '../../workspace/sandbox-fs';
import { getWorkspaceRoot } from '../../workspace/sandbox-setup';
import { buildCredentialMap, type CredentialMap } from '../workflows/resolve-credentials';
import {
buildCredentialSnapshot,
type CredentialEntry,
type CredentialMap,
} from '../workflows/resolve-credentials';
import { createIdentityEnforcedSubmitWorkflowTool } from '../workflows/submit-workflow-identity';
import {
type SubmitWorkflowAttempt,
@ -612,9 +616,12 @@ export async function startBuildWorkflowAgentTask(
let builderTools: ToolsInput;
let prompt = BUILDER_AGENT_PROMPT;
let credMap: CredentialMap | undefined;
let availableCredentials: CredentialEntry[] | undefined;
if (useSandbox) {
credMap = await buildCredentialMap(domainContext.credentialService);
const credentialSnapshot = await buildCredentialSnapshot(domainContext.credentialService);
credMap = credentialSnapshot.map;
availableCredentials = credentialSnapshot.list;
const toolNames = [
'nodes',
@ -824,6 +831,7 @@ export async function startBuildWorkflowAgentTask(
context: domainContext,
workspace,
credentialMap: credMap,
availableCredentials,
root,
currentRunId: context.runId,
getWorkflowLoopState: async () =>

View File

@ -1,7 +1,11 @@
import type { WorkflowJSON } from '@n8n/workflow-sdk';
import type { InstanceAiContext } from '../../../types';
import { resolveCredentials, type CredentialMap } from '../resolve-credentials';
import {
resolveCredentials,
type CredentialEntry,
type CredentialMap,
} from '../resolve-credentials';
// ---------------------------------------------------------------------------
// Helpers
@ -232,6 +236,172 @@ describe('resolveCredentials', () => {
});
});
describe('raw credential validation against snapshot', () => {
const availableCredentials: CredentialEntry[] = [
{ id: 'slack-1', name: 'Team Slack', type: 'slackApi' },
{ id: 'slack-2', name: 'Backup Slack', type: 'slackApi' },
{ id: 'gmail-1', name: 'Gmail', type: 'gmailOAuth2Api' },
];
it('keeps a raw credential id that exists in the snapshot for the same type', async () => {
const json = makeWorkflow({
nodes: [
{
id: '1',
name: 'Slack',
type: 'n8n-nodes-base.slack',
typeVersion: 2,
position: [0, 0],
credentials: { slackApi: { id: 'slack-1', name: 'Team Slack' } },
},
],
});
const credMap: CredentialMap = new Map([
['slackApi', { id: 'slack-2', name: 'Backup Slack' }],
]);
const result = await resolveCredentials(
json,
undefined,
createMockContext(),
credMap,
availableCredentials,
);
expect(result.mockedNodeNames).toEqual([]);
expect(json.nodes[0].credentials).toEqual({
slackApi: { id: 'slack-1', name: 'Team Slack' },
});
});
it('mocks a synthesized raw credential id instead of replacing it with the type-map fallback', async () => {
const json = makeWorkflow({
nodes: [
{
id: '1',
name: 'Slack',
type: 'n8n-nodes-base.slack',
typeVersion: 2,
position: [0, 0],
credentials: { slackApi: { id: 'WHATSAPP_CREDENTIAL_ID', name: 'WhatsApp' } },
},
],
});
const credMap: CredentialMap = new Map([['slackApi', { id: 'slack-1', name: 'Team Slack' }]]);
const result = await resolveCredentials(
json,
undefined,
createMockContext(),
credMap,
availableCredentials,
);
expect(result.mockedNodeNames).toEqual(['Slack']);
expect(result.mockedCredentialTypes).toEqual(['slackApi']);
expect(result.mockedCredentialsByNode).toEqual({ Slack: ['slackApi'] });
expect(json.nodes[0].credentials).toEqual({});
expect(result.verificationPinData).toEqual({
Slack: [{ _mockedCredential: 'slackApi' }],
});
});
it('mocks a mock-* raw credential id that is absent from the snapshot', async () => {
const json = makeWorkflow({
nodes: [
{
id: '1',
name: 'Gmail',
type: 'n8n-nodes-base.gmail',
typeVersion: 2,
position: [0, 0],
credentials: { gmailOAuth2Api: { id: 'mock-gmail-oauth2', name: 'Gmail' } },
},
],
});
const result = await resolveCredentials(
json,
undefined,
createMockContext(),
new Map(),
availableCredentials,
);
expect(result.mockedNodeNames).toEqual(['Gmail']);
expect(result.mockedCredentialTypes).toEqual(['gmailOAuth2Api']);
expect(json.nodes[0].credentials).toEqual({});
});
it('mocks a real id when it belongs to a different credential type', async () => {
const json = makeWorkflow({
nodes: [
{
id: '1',
name: 'Slack',
type: 'n8n-nodes-base.slack',
typeVersion: 2,
position: [0, 0],
credentials: { slackApi: { id: 'gmail-1', name: 'Gmail' } },
},
],
});
const result = await resolveCredentials(
json,
undefined,
createMockContext(),
new Map(),
availableCredentials,
);
expect(result.mockedNodeNames).toEqual(['Slack']);
expect(result.mockedCredentialTypes).toEqual(['slackApi']);
expect(json.nodes[0].credentials).toEqual({});
});
it('restores the existing workflow credential on edit when the builder emits an invalid raw id', async () => {
const json = makeWorkflow({
nodes: [
{
id: '1',
name: 'Slack',
type: 'n8n-nodes-base.slack',
typeVersion: 2,
position: [0, 0],
credentials: { slackApi: { id: 'WHATSAPP_CREDENTIAL_ID', name: 'WhatsApp' } },
},
],
});
const existingWorkflow = makeWorkflow({
nodes: [
{
id: '1',
name: 'Slack',
type: 'n8n-nodes-base.slack',
typeVersion: 2,
position: [0, 0],
credentials: { slackApi: { id: 'existing-slack', name: 'Existing Slack' } },
},
],
});
const result = await resolveCredentials(
json,
'wf-123',
createMockContext(existingWorkflow),
new Map(),
[{ id: 'existing-slack', name: 'Existing Slack', type: 'slackApi' }],
);
expect(result.mockedNodeNames).toEqual([]);
expect(json.nodes[0].credentials).toEqual({
slackApi: { id: 'existing-slack', name: 'Existing Slack' },
});
});
});
describe('existing workflow takes priority over credential map', () => {
it('preserves the existing credential on an edit even when the map has a different credential of the same type', async () => {
const json = makeWorkflow({

View File

@ -14,9 +14,21 @@ jest.mock('@mastra/core/tools', () => ({
createTool: jest.fn((config: Record<string, unknown>) => config),
}));
jest.mock('@n8n/workflow-sdk', () => ({
validateWorkflow: jest.fn(() => ({ errors: [], warnings: [] })),
}));
jest.mock(
'@n8n/utils',
() => ({
hasPlaceholderDeep: jest.fn(() => false),
}),
{ virtual: true },
);
jest.mock(
'@n8n/workflow-sdk',
() => ({
validateWorkflow: jest.fn(() => ({ errors: [], warnings: [] })),
}),
{ virtual: true },
);
// `require` (rather than `import`) is needed because `submit-workflow.tool`
// transitively pulls in @mastra/core (ESM-only); the require call here runs
@ -218,6 +230,34 @@ describe('createSubmitWorkflowTool — permission enforcement', () => {
});
describe('classifySubmitFailure', () => {
it('routes credential access save failures to setup instead of code remediation', () => {
const remediation = classifySubmitFailure(
['Workflow save failed: You do not have access to the credentials "mock-gmail-oauth2"'],
'workflow_save_failed',
);
expect(remediation).toMatchObject({
category: 'needs_setup',
shouldEdit: false,
reason: 'workflow_save_failed',
});
expect(remediation.guidance).toContain('credential');
});
it('routes missing credential save failures to setup instead of code remediation', () => {
const remediation = classifySubmitFailure(
['Workflow save failed: Credentials not found for id WHATSAPP_CREDENTIAL_ID'],
'workflow_save_failed',
);
expect(remediation).toMatchObject({
category: 'needs_setup',
shouldEdit: false,
reason: 'workflow_save_failed',
});
expect(remediation.guidance).toContain('credential');
});
it('treats workflow save failures as terminal blockers', () => {
const remediation = classifySubmitFailure(
['Workflow save failed: database unavailable'],

View File

@ -2,7 +2,7 @@ import { createTool } from '@mastra/core/tools';
import { generateWorkflowCode } from '@n8n/workflow-sdk';
import { z } from 'zod';
import { buildCredentialMap, resolveCredentials } from './resolve-credentials';
import { buildCredentialSnapshot, resolveCredentials } from './resolve-credentials';
import { stripStaleCredentialsFromWorkflow } from './setup-workflow.service';
import { ensureWebhookIds } from './submit-workflow.tool';
import type { InstanceAiContext } from '../../types';
@ -163,8 +163,14 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
// Resolve undefined/null credentials before saving.
// newCredential() produces NewCredentialImpl which serializes to undefined.
const credentialMap = await buildCredentialMap(context.credentialService);
await resolveCredentials(json, workflowId, context, credentialMap);
const credentialSnapshot = await buildCredentialSnapshot(context.credentialService);
await resolveCredentials(
json,
workflowId,
context,
credentialSnapshot.map,
credentialSnapshot.list,
);
// Strip credential entries that are no longer valid for the current
// parameters. Resolution above (and the LLM itself) can re-emit stale

View File

@ -15,6 +15,44 @@ import type { InstanceAiContext } from '../../types';
*/
export type CredentialMap = Map<string, { id: string; name: string }>;
/** Flat credential entry — preserves duplicates of the same type. */
export interface CredentialEntry {
id: string;
name: string;
type: string;
}
/**
* Paired credential snapshot produced from a single `credentialService.list()`
* call: a type-keyed map for fallback resolution AND a flat list for
* validating raw credential ids without losing duplicates of the same type.
*/
export interface CredentialSnapshot {
map: CredentialMap;
list: CredentialEntry[];
}
/**
* Build a paired credential snapshot from all available credentials.
* Non-fatal returns empty structures if listing fails.
*/
export async function buildCredentialSnapshot(
credentialService: Pick<InstanceAiContext['credentialService'], 'list'>,
): Promise<CredentialSnapshot> {
const map: CredentialMap = new Map();
const list: CredentialEntry[] = [];
try {
const allCreds = await credentialService.list();
for (const cred of allCreds) {
map.set(cred.type, { id: cred.id, name: cred.name });
list.push({ id: cred.id, name: cred.name, type: cred.type });
}
} catch {
// Non-fatal — credentials will be unresolved
}
return { map, list };
}
/**
* Build a credential map from all available credentials.
* Non-fatal returns an empty map if listing fails.
@ -22,15 +60,7 @@ export type CredentialMap = Map<string, { id: string; name: string }>;
export async function buildCredentialMap(
credentialService: Pick<InstanceAiContext['credentialService'], 'list'>,
): Promise<CredentialMap> {
const map: CredentialMap = new Map();
try {
const allCreds = await credentialService.list();
for (const cred of allCreds) {
map.set(cred.type, { id: cred.id, name: cred.name });
}
} catch {
// Non-fatal — credentials will be unresolved
}
const { map } = await buildCredentialSnapshot(credentialService);
return map;
}
@ -63,6 +93,7 @@ export async function resolveCredentials(
workflowId: string | undefined,
ctx: InstanceAiContext,
credentialMap: CredentialMap,
availableCredentials?: CredentialEntry[],
): Promise<CredentialResolutionResult> {
const mockedNodeNames: string[] = [];
const mockedCredentialTypesSet = new Set<string>();
@ -93,15 +124,53 @@ export async function resolveCredentials(
let nodeMocked = false;
for (const [key, value] of Object.entries(creds)) {
if (value !== undefined && value !== null) continue;
// Try 1: restore from existing workflow (preserves the user's chosen credential
// when the LLM drops the id during an edit — e.g., emits newCredential('name')
// without the id, which serializes to undefined).
const existingCreds = node.name ? existingCredsByNode.get(node.name) : undefined;
if (existingCreds?.[key]) {
const restoreExistingCredential = () => {
if (!existingCreds?.[key]) return false;
creds[key] = existingCreds[key];
cleanupMockPinData(json, node.name);
return true;
};
const mockCredential = () => {
const nodeName = node.name ?? '';
delete creds[key];
mockedCredentialTypesSet.add(key);
nodeMocked = true;
if (nodeName) {
// Track which credential types were mocked on this node
mockedCredentialsByNode[nodeName] ??= [];
mockedCredentialsByNode[nodeName].push(key);
// Produce sidecar verification pin data (never saved to workflow).
// If the workflow already has real pinData for this node, skip — the
// existing pinData will suffice for execution skipping.
if (!(json.pinData && nodeName in json.pinData)) {
verificationPinData[nodeName] ??= [];
if (verificationPinData[nodeName].length === 0) {
verificationPinData[nodeName].push({ _mockedCredential: key });
}
}
}
};
if (value !== undefined && value !== null) {
if (isKnownCredentialForType(value, key, availableCredentials)) {
cleanupMockPinData(json, node.name);
continue;
}
if (restoreExistingCredential()) {
continue;
}
mockCredential();
continue;
}
if (restoreExistingCredential()) {
continue;
}
@ -119,26 +188,7 @@ export async function resolveCredentials(
// The credential key is deleted so the saved workflow doesn't reference a
// non-existent credential. Verification pin data is produced so the execution
// engine can skip this node during test runs.
const nodeName = node.name ?? '';
delete creds[key];
mockedCredentialTypesSet.add(key);
nodeMocked = true;
if (nodeName) {
// Track which credential types were mocked on this node
mockedCredentialsByNode[nodeName] ??= [];
mockedCredentialsByNode[nodeName].push(key);
// Produce sidecar verification pin data (never saved to workflow).
// If the workflow already has real pinData for this node, skip — the
// existing pinData will suffice for execution skipping.
if (!(json.pinData && nodeName in json.pinData)) {
verificationPinData[nodeName] ??= [];
if (verificationPinData[nodeName].length === 0) {
verificationPinData[nodeName].push({ _mockedCredential: key });
}
}
}
mockCredential();
}
if (nodeMocked && node.name) {
@ -154,6 +204,30 @@ export async function resolveCredentials(
};
}
function getCredentialId(value: unknown): string | undefined {
if (typeof value !== 'object' || value === null || !('id' in value)) return undefined;
const { id } = value;
if (typeof id !== 'string' || id.trim() === '') return undefined;
return id;
}
function isKnownCredentialForType(
value: unknown,
credentialType: string,
availableCredentials: CredentialEntry[] | undefined,
): boolean {
if (!availableCredentials) return true;
const id = getCredentialId(value);
if (!id) return false;
return availableCredentials.some(
(credential) => credential.id === id && credential.type === credentialType,
);
}
/**
* Legacy cleanup: remove mock pinData markers from workflows saved before the
* sidecar verification data refactor. New builds never write `_mockedCredential`

View File

@ -17,7 +17,7 @@
import { createTool } from '@mastra/core/tools';
import type { Workspace } from '@mastra/core/workspace';
import type { CredentialMap } from './resolve-credentials';
import type { CredentialEntry, CredentialMap } from './resolve-credentials';
import {
createSubmitWorkflowTool,
resolveSandboxWorkflowFilePath,
@ -215,6 +215,7 @@ export function createIdentityEnforcedSubmitWorkflowTool(args: {
context: InstanceAiContext;
workspace: Workspace;
credentialMap?: CredentialMap;
availableCredentials?: CredentialEntry[];
onAttempt: (attempt: SubmitWorkflowAttempt) => Promise<void> | void;
root: string;
currentRunId?: string;
@ -229,6 +230,7 @@ export function createIdentityEnforcedSubmitWorkflowTool(args: {
async (attempt) => {
await args.onAttempt(budgetTracker.recordAttempt(attempt));
},
args.availableCredentials,
);
const underlyingExecute = underlying.execute as SubmitExecute | undefined;

View File

@ -14,7 +14,11 @@ import { validateWorkflow } from '@n8n/workflow-sdk';
import { createHash, randomUUID } from 'node:crypto';
import { z } from 'zod';
import { resolveCredentials, type CredentialMap } from './resolve-credentials';
import {
resolveCredentials,
type CredentialEntry,
type CredentialMap,
} from './resolve-credentials';
import { stripStaleCredentialsFromWorkflow } from './setup-workflow.service';
import type { InstanceAiContext } from '../../types';
import type { ValidationWarning } from '../../workflow-builder';
@ -235,6 +239,17 @@ export function classifySubmitFailure(
errors: string[],
reason = 'submit_failed',
): RemediationMetadata {
const text = errors.join('\n').toLowerCase();
if (isCredentialSaveFailure(text)) {
return createRemediation({
category: 'needs_setup',
shouldEdit: false,
reason,
guidance:
'Workflow submission failed because a credential is missing or inaccessible. Stop editing and route the user to credential setup.',
});
}
if (reason === 'workflow_save_failed') {
return createRemediation({
category: 'blocked',
@ -245,7 +260,6 @@ export function classifySubmitFailure(
});
}
const text = errors.join('\n').toLowerCase();
if (
text.includes('blocked by admin') ||
text.includes('read-only') ||
@ -274,6 +288,7 @@ export function createSubmitWorkflowTool(
workspace: Workspace,
credentialMap: CredentialMap = new Map(),
onAttempt?: (attempt: SubmitWorkflowAttempt) => void | Promise<void>,
availableCredentials?: CredentialEntry[],
) {
return createTool({
id: 'submit-workflow',
@ -419,7 +434,13 @@ export function createSubmitWorkflowTool(
// For updates: restore from the existing workflow's resolved credentials.
// For new nodes: look up credentials by name from the credential service.
// Unresolved credentials are mocked via pinned data when available.
const mockResult = await resolveCredentials(json, workflowId, context, credentialMap);
const mockResult = await resolveCredentials(
json,
workflowId,
context,
credentialMap,
availableCredentials,
);
// Strip credential entries that are no longer valid for the current
// parameters. Resolution above (and the LLM itself) can re-emit stale
@ -520,3 +541,18 @@ export function createSubmitWorkflowTool(
},
});
}
function isCredentialSaveFailure(text: string): boolean {
if (!text.includes('credential')) return false;
return (
text.includes('not found') ||
text.includes('missing') ||
text.includes('not accessible') ||
text.includes('no access') ||
text.includes('do not have access') ||
text.includes("don't have access") ||
text.includes('not shared') ||
text.includes('permission')
);
}

View File

@ -0,0 +1,10 @@
import { WORKFLOW_RULES } from './workflow-rules';
describe('WORKFLOW_RULES', () => {
it('forbids synthesized credential ids and treats availableCredentials as an allow-list', () => {
expect(WORKFLOW_RULES).toContain('availableCredentials');
expect(WORKFLOW_RULES).toContain('allow-list');
expect(WORKFLOW_RULES).toContain('Never synthesize credential IDs');
expect(WORKFLOW_RULES).toContain('mock-*');
});
});

View File

@ -11,6 +11,8 @@ export const WORKFLOW_RULES = `Follow these rules strictly when generating workf
1. **Always use newCredential() for authentication**
- When a node needs credentials, always use \`newCredential('Name')\` in the credentials config
- NEVER use placeholder strings, fake API keys, or hardcoded auth values
- Never synthesize credential IDs. Do not invent raw IDs such as \`WHATSAPP_CREDENTIAL_ID\`, \`mock-gmail-oauth2\`, or any \`mock-*\` value
- If \`availableCredentials\` is provided, treat it as an allow-list: copy an existing credential ID exactly or use \`newCredential('Name')\` without an ID
- Example: \`credentials: { slackApi: newCredential('Slack Bot') }\`
- The credential type must match what the node expects