mirror of
https://github.com/n8n-io/n8n.git
synced 2026-09-21 04:37:50 +08:00
fix(core): Accept placeholder() inside node credentials slot (#29691)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
1d9548c81f
commit
dc6bd68de3
@@ -432,7 +432,7 @@ export interface WorkflowContext {
|
||||
*/
|
||||
export interface NodeConfig<TParams = IDataObject> {
|
||||
parameters?: TParams;
|
||||
credentials?: Record<string, CredentialReference | NewCredentialValue>;
|
||||
credentials?: Record<string, CredentialReference | NewCredentialValue | PlaceholderValue>;
|
||||
name?: string;
|
||||
position?: [number, number];
|
||||
webhookId?: string;
|
||||
|
||||
@@ -445,6 +445,84 @@ describe('Node Builder', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('placeholder() inside credentials slot', () => {
|
||||
it('normalizes placeholder() to a __newCredential marker carrying the hint as name', () => {
|
||||
const n = node({
|
||||
type: 'n8n-nodes-base.slack',
|
||||
version: 2.2,
|
||||
config: {
|
||||
parameters: { channel: '#general' },
|
||||
credentials: { slackApi: placeholder('Slack Bot') },
|
||||
},
|
||||
});
|
||||
|
||||
const stored = n.config.credentials?.slackApi;
|
||||
expect(stored).toBeDefined();
|
||||
expect((stored as { __newCredential?: boolean }).__newCredential).toBe(true);
|
||||
expect((stored as { name?: string }).name).toBe('Slack Bot');
|
||||
expect((stored as { id?: string }).id).toBeUndefined();
|
||||
// The original __placeholder marker is gone — credentials maps never carry it.
|
||||
expect((stored as { __placeholder?: boolean }).__placeholder).toBeUndefined();
|
||||
});
|
||||
|
||||
it('serializes a placeholder() credential to undefined (omitted from JSON)', () => {
|
||||
const n = node({
|
||||
type: 'n8n-nodes-base.slack',
|
||||
version: 2.2,
|
||||
config: {
|
||||
credentials: { slackApi: placeholder('Slack Bot') },
|
||||
},
|
||||
});
|
||||
|
||||
// Same shape as newCredential() without id: toJSON returns undefined
|
||||
// so JSON.stringify drops the slot entirely.
|
||||
expect(JSON.stringify(n.config.credentials)).toBe('{}');
|
||||
});
|
||||
|
||||
it('does not leak the <__PLACEHOLDER_VALUE__*> string into serialized credentials', () => {
|
||||
const n = node({
|
||||
type: 'n8n-nodes-base.slack',
|
||||
version: 2.2,
|
||||
config: {
|
||||
credentials: { slackApi: placeholder('Slack Bot') },
|
||||
},
|
||||
});
|
||||
|
||||
expect(JSON.stringify(n.config.credentials)).not.toContain('__PLACEHOLDER_VALUE__');
|
||||
});
|
||||
|
||||
it('normalizes only the placeholder slot, leaving other credentials untouched', () => {
|
||||
const n = node({
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
version: 4.2,
|
||||
config: {
|
||||
credentials: {
|
||||
httpBasicAuth: { id: 'existing-123', name: 'Existing Auth' },
|
||||
httpHeaderAuth: placeholder('Header Auth'),
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const creds = n.config.credentials!;
|
||||
expect(creds.httpBasicAuth).toEqual({ id: 'existing-123', name: 'Existing Auth' });
|
||||
expect((creds.httpHeaderAuth as { __newCredential?: boolean }).__newCredential).toBe(true);
|
||||
expect((creds.httpHeaderAuth as { name?: string }).name).toBe('Header Auth');
|
||||
});
|
||||
|
||||
it('also normalizes when credentials are supplied via update()', () => {
|
||||
const n = node({
|
||||
type: 'n8n-nodes-base.slack',
|
||||
version: 2.2,
|
||||
config: { parameters: { channel: '#general' } },
|
||||
});
|
||||
|
||||
const updated = n.update({ credentials: { slackApi: placeholder('Slack Bot') } });
|
||||
const stored = updated.config.credentials?.slackApi;
|
||||
expect((stored as { __newCredential?: boolean }).__newCredential).toBe(true);
|
||||
expect((stored as { name?: string }).name).toBe('Slack Bot');
|
||||
});
|
||||
});
|
||||
|
||||
describe('then() with multiple targets (fan-out)', () => {
|
||||
it('should connect a node to multiple targets with array syntax', () => {
|
||||
const source = node({
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
type StickyNoteConfig,
|
||||
type PlaceholderValue,
|
||||
type NewCredentialValue,
|
||||
type CredentialReference,
|
||||
type DeclaredConnection,
|
||||
type NodeChain,
|
||||
type InputTarget,
|
||||
@@ -98,6 +99,35 @@ function generateNodeName(type: string): string {
|
||||
.replace(/Gcp/g, 'GCP');
|
||||
}
|
||||
|
||||
/**
|
||||
* Collapse `placeholder('hint')` markers inside a credentials map into
|
||||
* `newCredential('hint')`. The two have identical intent in this slot —
|
||||
* "a credential is required, no real one is bound yet" — so we normalize at
|
||||
* config ingest. Downstream code (resolveCredentials, hasNewCredential, the
|
||||
* `__newCredential` toJSON path) only ever sees `__newCredential` markers in
|
||||
* credential slots, never `__placeholder` ones.
|
||||
*
|
||||
* Returns a new config object when any normalization happens; otherwise a
|
||||
* shallow copy (matching the previous `{ ...config }` semantics).
|
||||
*/
|
||||
export function normalizeNodeConfig(config: NodeConfig): NodeConfig {
|
||||
const creds = config?.credentials;
|
||||
if (!creds) return { ...config };
|
||||
|
||||
let normalizedCreds:
|
||||
| Record<string, CredentialReference | NewCredentialValue | PlaceholderValue>
|
||||
| undefined;
|
||||
for (const [key, value] of Object.entries(creds)) {
|
||||
if (value && typeof value === 'object' && '__placeholder' in value) {
|
||||
normalizedCreds ??= { ...creds };
|
||||
normalizedCreds[key] = new NewCredentialImpl(value.hint);
|
||||
}
|
||||
}
|
||||
|
||||
if (!normalizedCreds) return { ...config };
|
||||
return { ...config, credentials: normalizedCreds };
|
||||
}
|
||||
|
||||
/**
|
||||
* Internal node instance implementation
|
||||
*/
|
||||
@@ -122,7 +152,7 @@ class NodeInstanceImpl<TType extends string, TVersion extends string, TOutput =
|
||||
) {
|
||||
this.type = type;
|
||||
this.version = version;
|
||||
this.config = { ...config };
|
||||
this.config = normalizeNodeConfig(config);
|
||||
this.id = id ?? uuid();
|
||||
this.name = name ?? config?.name ?? generateNodeName(type);
|
||||
this._connections = connections ?? [];
|
||||
@@ -1146,6 +1176,11 @@ class PlaceholderImpl implements PlaceholderValue {
|
||||
* Placeholders are used to mark values that need to be filled in
|
||||
* when a workflow template is instantiated.
|
||||
*
|
||||
* Inside a node's `credentials` slot, `placeholder(hint)` is normalized to
|
||||
* `newCredential(hint)` at config ingest — the two have identical intent
|
||||
* there ("a credential is required, no real one bound yet"). Outside the
|
||||
* credentials slot the original placeholder semantics are unchanged.
|
||||
*
|
||||
* @param hint - Description shown to users (e.g., 'Enter Channel')
|
||||
* @returns A placeholder value that serializes to the placeholder format
|
||||
*
|
||||
|
||||
@@ -32,6 +32,7 @@
|
||||
|
||||
import { v4 as uuid } from 'uuid';
|
||||
|
||||
import { normalizeNodeConfig } from './node-builder';
|
||||
import { createFromAIExpression } from '../../expression';
|
||||
import type {
|
||||
NodeConfig,
|
||||
@@ -103,7 +104,7 @@ class SubnodeInstanceImpl<
|
||||
) {
|
||||
this.type = type;
|
||||
this.version = version;
|
||||
this.config = { ...config };
|
||||
this.config = normalizeNodeConfig(config);
|
||||
this.id = id ?? uuid();
|
||||
this.name = name ?? config.name ?? generateNodeName(type);
|
||||
this._subnodeType = subnodeType;
|
||||
|
||||
@@ -10,6 +10,13 @@ import type { NodeInstance } from '../types/base';
|
||||
/**
|
||||
* Check if a node or any of its subnodes have a newCredential() marker.
|
||||
* Nodes with new credentials need pin data to avoid execution errors.
|
||||
*
|
||||
* Note: by the time a NodeInstance reaches this code, credentials slots only
|
||||
* ever contain `CredentialReference` or `__newCredential` markers — never
|
||||
* `__placeholder` markers. `placeholder()` values supplied for credentials
|
||||
* are normalized to `__newCredential` at config ingest in
|
||||
* `node-builder.ts#normalizeNodeConfig`, so we don't need a second check
|
||||
* here.
|
||||
*/
|
||||
export function hasNewCredential(node: NodeInstance<string, string, unknown>): boolean {
|
||||
// Check main node credentials
|
||||
|
||||
+16
-2
@@ -103,8 +103,22 @@ function serializeNode(
|
||||
|
||||
// Add optional properties
|
||||
if (config.credentials) {
|
||||
// Serialize credentials to ensure newCredential() markers are converted to JSON
|
||||
n8nNode.credentials = deepCopy(config.credentials);
|
||||
if (typeof config.credentials !== 'object') {
|
||||
// Real workflows occasionally carry credentials as a primitive (e.g. the
|
||||
// post-redaction string `"[REDACTED]"`). Pass through unchanged.
|
||||
n8nNode.credentials = deepCopy(config.credentials);
|
||||
} else {
|
||||
// `NodeConfig.credentials` is typed wide (also accepts PlaceholderValue)
|
||||
// at the public API. By this point `normalizeNodeConfig` has rewritten any
|
||||
// placeholder() markers to newCredential() markers, so no __placeholder
|
||||
// values remain at runtime. Narrow the value type for the serializer.
|
||||
const resolvable: NonNullable<NodeJSON['credentials']> = {};
|
||||
for (const [key, value] of Object.entries(config.credentials)) {
|
||||
if (value && typeof value === 'object' && '__placeholder' in value) continue;
|
||||
resolvable[key] = value;
|
||||
}
|
||||
n8nNode.credentials = deepCopy(resolvable);
|
||||
}
|
||||
}
|
||||
if (config.disabled) {
|
||||
n8nNode.disabled = config.disabled;
|
||||
|
||||
Reference in New Issue
Block a user