fix(editor): Clarify external secret expression previews (#36064)

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Nour Alhadi Mahmoud
2026-08-13 13:51:15 +00:00
committed by GitHub
parent c0278b07a0
commit 7792fdbbc7
11 changed files with 339 additions and 16 deletions
@@ -1672,6 +1672,8 @@
"expressionEditor.uncalledFunction": "[this is a function, please add ()]",
"expressionEditor.deprecated.getPairedItem": "$getPairedItem is deprecated and will be removed",
"expressionModalInput.empty": "[empty]",
"expressionModalInput.evaluatedDuringExecution": "[evaluated during execution]",
"expressionModalInput.secretNotFound": "[secret not found]",
"expressionModalInput.undefined": "[undefined]",
"expressionModalInput.null": "null",
"expressionTip.noExecutionData": "Execute previous nodes to use input data",
@@ -85,6 +85,34 @@ describe('useResolvedExpression', () => {
expect(toValue(resolvedExpressionString)).toBe('[ERROR: Test error]');
});
it('should defer transformed credential secret previews until execution', async () => {
mockResolveExpression().mockResolvedValue(undefined);
const { resolvedExpressionString } = await renderTestComponent({
expression: "={{ JSON.parse($secrets.aws['preview-test']).password }}",
isForCredential: true,
additionalData: {
$secrets: { aws: { 'preview-test': '*********' } },
},
});
await nextTick();
expect(toValue(resolvedExpressionString)).toBe('[evaluated during execution]');
});
it('should not defer credential secret previews with an unknown secret reference', async () => {
mockResolveExpression().mockResolvedValue(undefined);
const { resolvedExpressionString } = await renderTestComponent({
expression: "={{ JSON.parse($secrets.aws['name-with-typo']).password }}",
isForCredential: true,
additionalData: {
$secrets: { aws: { 'preview-test': '*********' } },
},
});
await nextTick();
expect(toValue(resolvedExpressionString)).toBe('[secret not found]');
});
it('should debounce updates', async () => {
const resolveExpressionSpy = mockResolveExpression().mockResolvedValue(4);
const expression = ref('={{ testValue }}');
@@ -1,6 +1,7 @@
import { useNDVStore } from '@/features/ndv/shared/ndv.store';
import { injectWorkflowExecutionStateStore } from '@/app/stores/workflowExecutionState.store';
import {
getExternalSecretPreview,
isExpression as isExpressionUtil,
stringifyExpressionResult,
} from '@/app/utils/expressions';
@@ -108,11 +109,22 @@ export function useResolvedExpression({
if (currentInvocation !== updateExpressionInvocation) return;
resolvedExpression.value = resolved.ok ? resolved.result : null;
resolvedExpressionString.value = stringifyExpressionResult(
resolved,
workflowDocumentStore.value.getPinDataSnapshot(),
hasRunData.value,
);
const expressionString = toValue(expression);
const secretPreview =
resolved.ok &&
resolved.result === undefined &&
toValue(isForCredential) &&
typeof expressionString === 'string'
? getExternalSecretPreview(expressionString, toValue(additionalData)?.$secrets)
: undefined;
resolvedExpressionString.value =
secretPreview?.text ??
stringifyExpressionResult(
resolved,
workflowDocumentStore.value.getPinDataSnapshot(),
hasRunData.value,
);
} else {
resolvedExpression.value = null;
resolvedExpressionString.value = '';
@@ -1,6 +1,7 @@
import { ExpressionError } from 'n8n-workflow';
import {
completeExpressionSyntax,
getExternalSecretPreview,
shouldConvertToExpression,
removeExpressionPrefix,
stringifyExpressionResult,
@@ -9,6 +10,65 @@ import {
import { executionRetryMessage } from '@/features/execution/executions/executions.utils';
describe('Utils: Expressions', () => {
describe('getExternalSecretPreview()', () => {
const secrets = {
vault: {
key: '*********',
'json/path': '*********',
},
};
it.each([
'={{ $secrets.vault.key }}',
"={{ JSON.parse($secrets.vault['json/path']).password }}",
'={{ JSON.parse($secrets . vault ["json/path"]).password }}',
'={{ $secrets.vault.key + $secrets.vault["json/path"] }}',
'={{ $secrets[\'vault\']["key"] }}',
])('should defer an existing secret in "%s"', (expression) => {
expect(getExternalSecretPreview(expression, secrets)).toEqual({
text: '[evaluated during execution]',
exists: true,
});
});
it.each([
'={{ $secrets.invalidVault.key }}',
'={{ $secrets.vault.nameWithTypo }}',
'={{ $secrets.vault.key + $secrets.vault.nameWithTypo }}',
'={{ $secrets.invalidVault[$vars.key] }}',
])('should report a secret that does not exist in "%s"', (expression) => {
expect(getExternalSecretPreview(expression, secrets)).toEqual({
text: '[secret not found]',
exists: false,
});
});
it.each([
'={{ $secrets.vault.key.nameWithTypo }}',
'={{ $secrets.vault.key?.nameWithTypo }}',
'={{ $secrets[provider].key }}',
'={{ $secrets.vault[$vars.secret] }}',
])('should not judge a secret path it cannot read in "%s"', (expression) => {
expect(getExternalSecretPreview(expression, secrets)).toBeUndefined();
});
it.each(['={{ $json.password }}', 'plain $secrets.vault.key'])(
'should ignore non-expression secret references in "%s"',
(expression) => {
expect(getExternalSecretPreview(expression, secrets)).toBeUndefined();
},
);
it.each([undefined, {}, { vault: {} }])(
'should not claim a missing secret when metadata is %s',
(unavailableSecrets) => {
expect(
getExternalSecretPreview('={{ $secrets.vault.key }}', unavailableSecrets),
).toBeUndefined();
},
);
});
describe('stringifyExpressionResult()', () => {
it('should return empty string for non-critical errors', () => {
expect(
@@ -2,9 +2,101 @@ import { i18n } from '@n8n/i18n';
import type { ResolvableState } from '@/app/types/expressions';
import type { Result } from '@n8n/utils/result';
import { ExpressionError, ExpressionParser, isExpression, type IPinData } from 'n8n-workflow';
import { isObject } from '@/app/utils/objectUtils';
export { isExpression };
type ExternalSecretReferenceState = 'none' | 'known' | 'missing' | 'unknown';
const SECRET_REFERENCE = /\$secrets\b/;
/** The only key forms we can read at edit time: `.key`, `['key']`, `["key"]`. */
const LITERAL_KEY_ACCESS =
/^\s*(?:\.\s*(?<dotKey>[a-zA-Z_$][\w$]*)|\[\s*(?<quote>['"])(?<quotedKey>[^\\]*?)\k<quote>\s*\])/;
/** Any further access, e.g. `[$vars.key]`, `?.key`, `['a\'b']`. */
const UNREAD_ACCESS = /^\s*(?:\?\.|\.|\[)/;
/**
* Reads the keys directly after a `$secrets` occurrence, so `.vault['key']` yields
* `['vault', 'key']`. `hasUnreadAccess` flags a chain that goes on in some other form.
*/
const readLiteralKeys = (afterReference: string): { keys: string[]; hasUnreadAccess: boolean } => {
const keys: string[] = [];
let rest = afterReference;
let access = LITERAL_KEY_ACCESS.exec(rest);
while (access) {
keys.push(access.groups?.dotKey ?? access.groups?.quotedKey ?? '');
rest = rest.slice(access[0].length);
access = LITERAL_KEY_ACCESS.exec(rest);
}
return { keys, hasUnreadAccess: UNREAD_ACCESS.test(rest) };
};
/**
* Walks the keys through the masked secrets metadata: `['vault', 'key']` in
* `{ vault: { key: '***' } }` is `known`, an absent key is `missing`, anything else `unknown`.
*/
const lookUpKeys = (
keys: string[],
secrets: unknown,
): Exclude<ExternalSecretReferenceState, 'none'> => {
let value: unknown = secrets;
for (const key of keys) {
if (!isObject(value)) return 'unknown';
// Metadata that never loaded is empty, which must not read as a wrong path.
if (Object.keys(value).length === 0) return 'unknown';
if (!Object.hasOwn(value, key)) return 'missing';
value = value[key];
}
return typeof value === 'string' ? 'known' : 'unknown';
};
/**
* Looks up every `$secrets` reference in the expression's code against the secrets metadata,
* reporting `known` only when each one reads in full and lands on an existing secret.
*/
const getExternalSecretReferenceState = (
expression: string,
secrets: unknown,
): ExternalSecretReferenceState => {
let state: ExternalSecretReferenceState = 'none';
for (const { type, text: code } of ExpressionParser.splitExpression(expression)) {
if (type === 'text') continue;
const [, ...afterReferences] = code.split(SECRET_REFERENCE);
for (const afterReference of afterReferences) {
const { keys, hasUnreadAccess } = readLiteralKeys(afterReference);
const keyState = lookUpKeys(keys, secrets);
const referenceState = hasUnreadAccess && keyState === 'known' ? 'unknown' : keyState;
if (referenceState !== 'known') return referenceState;
state = 'known';
}
}
return state;
};
export const getExternalSecretPreview = (
expression: string,
secrets: unknown,
): { text: string; exists: boolean } | undefined => {
switch (getExternalSecretReferenceState(expression, secrets)) {
case 'known':
return { text: i18n.baseText('expressionModalInput.evaluatedDuringExecution'), exists: true };
case 'missing':
return { text: i18n.baseText('expressionModalInput.secretNotFound'), exists: false };
default:
return undefined;
}
};
export const isEmptyExpression = (expr: string) => {
return /\{\{\s*\}\}/.test(expr);
};
@@ -6,6 +6,9 @@ import { setActivePinia, type Pinia } from 'pinia';
import { defaultSettings } from '@/__tests__/defaults';
import { useSettingsStore } from '@n8n/stores/settings.store';
import { createTestNodeProperties } from '@/__tests__/mocks';
import { useUIStore } from '@/app/stores/ui.store';
import { CREDENTIAL_EDIT_MODAL_KEY } from '@/features/credentials/credentials.constants';
import { Expression } from 'n8n-workflow';
vi.mock('vue-router', () => {
const push = vi.fn();
@@ -82,6 +85,29 @@ describe('ExpressionEditModal', () => {
});
});
it('previews external secrets with the data passed by the credential modal', async () => {
// The evaluator returns undefined for a transformed secret, which its type does not admit.
vi.spyOn(Expression, 'resolveWithoutWorkflow').mockReturnValue(undefined as unknown as string);
useUIStore().modalsById[CREDENTIAL_EDIT_MODAL_KEY].open = true;
const { getByTestId } = renderModal({
pinia,
props: {
parameter: createTestNodeProperties({ name: 'foo', type: 'string' }),
path: '',
modelValue: "={{ JSON.parse($secrets.vault['json/path']).password }}",
dialogVisible: true,
additionalExpressionData: { $secrets: { vault: { 'json/path': '*********' } } },
},
});
await waitFor(() => {
expect(getByTestId('expression-modal-output')).toHaveTextContent(
'[evaluated during execution]',
);
});
});
describe('output render mode', () => {
it('renders all three render mode options', async () => {
const { getByRole } = renderModal({
@@ -10,7 +10,7 @@ import { createExpressionTelemetryPayload } from '@/app/utils/telemetryUtils';
import { useTelemetry } from '@n8n/composables/useTelemetry';
import type { Segment } from '@/app/types/expressions';
import type { INodeProperties } from 'n8n-workflow';
import type { IDataObject, INodeProperties } from 'n8n-workflow';
import { NodeConnectionTypes } from 'n8n-workflow';
import { outputTheme } from './ExpressionEditorModal/theme';
import ExpressionOutput from '@/features/shared/editors/components/InlineExpressionEditor/ExpressionOutput.vue';
@@ -44,6 +44,7 @@ type Props = {
eventSource?: string;
redactValues?: boolean;
isReadOnly?: boolean;
additionalExpressionData?: IDataObject;
};
const props = withDefaults(defineProps<Props>(), {
@@ -51,6 +52,7 @@ const props = withDefaults(defineProps<Props>(), {
dialogVisible: false,
redactValues: false,
isReadOnly: false,
additionalExpressionData: () => ({}),
});
const emit = defineEmits<{
'update:model-value': [value: string];
@@ -223,6 +225,7 @@ const onResizeThrottle = useThrottleFn(onResize, 10);
:model-value="modelValue"
:is-read-only="isReadOnly"
:path="path"
:additional-data="additionalExpressionData"
:class="[
$style.editor,
{
@@ -16,17 +16,20 @@ import { mappingDropCursor } from '@/features/shared/editors/plugins/codemirror/
import { editorKeymap } from '@/features/shared/editors/plugins/codemirror/keymap';
import { expressionCloseBrackets } from '@/features/shared/editors/plugins/codemirror/expressionCloseBrackets';
import type { TargetNodeParameterContext } from '@/Interface';
import type { IDataObject } from 'n8n-workflow';
type Props = {
modelValue: string;
path: string;
targetNodeParameterContext?: TargetNodeParameterContext;
isReadOnly?: boolean;
additionalData?: IDataObject;
};
const props = withDefaults(defineProps<Props>(), {
isReadOnly: false,
targetNodeParameterContext: undefined,
additionalData: () => ({}),
});
const emit = defineEmits<{
@@ -60,6 +63,7 @@ const { segments, readEditorValue, editor, hasFocus, focus } = useExpressionEdit
parameterPath: props.path,
},
targetNodeParameterContext: props.targetNodeParameterContext,
additionalData: () => props.additionalData,
});
watch(
@@ -1497,6 +1497,7 @@ onUpdated(async () => {
:event-source="eventSource || 'ndv'"
:is-read-only="isReadOnly"
:redact-values="shouldRedactValue"
:additional-expression-data="additionalExpressionData"
@close-dialog="closeExpressionEditDialog"
@update:model-value="expressionUpdated"
/>
@@ -476,5 +476,74 @@ describe('useExpressionEditor', () => {
);
expect(resolveExpressionMock).not.toHaveBeenCalled();
});
test('should defer previewing transformed external secrets until execution', async () => {
vi.spyOn(completionUtils, 'isCredentialsModalOpen').mockReturnValueOnce(true);
const {
expressionEditor: { segments },
} = await renderExpressionEditor({
editorValue: "{{ JSON.parse($secrets.awsSecretsManager['cred']).password }}",
extensions: [n8nLang()],
additionalData: {
$secrets: { awsSecretsManager: { cred: '*********' } },
},
});
await waitFor(() => {
expect(toValue(segments.resolvable)).toEqual([
expect.objectContaining({
resolved: '[evaluated during execution]',
state: 'pending',
}),
]);
});
});
test('should keep unknown external secret references invalid', async () => {
vi.spyOn(completionUtils, 'isCredentialsModalOpen').mockReturnValueOnce(true);
const {
expressionEditor: { segments },
} = await renderExpressionEditor({
editorValue: "{{ JSON.parse($secrets.awsSecretsManager['name-with-typo']).password }}",
extensions: [n8nLang()],
additionalData: {
$secrets: { awsSecretsManager: { cred: '*********' } },
},
});
await waitFor(() => {
expect(toValue(segments.resolvable)).toEqual([
expect.objectContaining({
resolved: '[secret not found]',
state: 'invalid',
}),
]);
});
});
test('should leave secret previews to the credential modal', async () => {
mockResolveExpression();
const {
expressionEditor: { segments },
} = await renderExpressionEditor({
editorValue: "{{ JSON.parse($secrets.awsSecretsManager['cred']).password }}",
extensions: [n8nLang()],
additionalData: {
$secrets: { awsSecretsManager: { cred: '*********' } },
},
});
await waitFor(() => {
expect(toValue(segments.resolvable)).toEqual([
expect.objectContaining({
resolved: '[undefined]',
state: 'invalid',
}),
]);
});
});
});
});
@@ -30,8 +30,19 @@ import {
} from '@/app/composables/useWorkflowHelpers';
import { highlighter } from '../plugins/codemirror/resolvableHighlighter';
import { closeCursorInfoBox } from '../plugins/codemirror/tooltips/InfoBoxTooltip';
import type { Html, Plaintext, RawSegment, Resolvable, Segment } from '@/app/types/expressions';
import { getExpressionErrorMessage, getResolvableState } from '@/app/utils/expressions';
import type {
Html,
Plaintext,
RawSegment,
Resolvable,
ResolvableState,
Segment,
} from '@/app/types/expressions';
import {
getExpressionErrorMessage,
getExternalSecretPreview,
getResolvableState,
} from '@/app/utils/expressions';
import { isCredentialsModalOpen } from '../plugins/codemirror/completions/utils';
import { usesDeprecatedExpressionFunction } from '../plugins/codemirror/expressionDeprecations';
import { closeCompletion, completionStatus } from '@codemirror/autocomplete';
@@ -144,7 +155,7 @@ export const useExpressionEditor = ({
const { from, to, text, token } = segment;
if (token === 'Resolvable') {
const { resolved, error, fullError } = await resolve(text, targetItem.value);
const { resolved, error, fullError, state } = await resolve(text, targetItem.value);
return {
kind: 'resolvable' as const,
from,
@@ -154,7 +165,8 @@ export const useExpressionEditor = ({
// For some reason, expressions that resolve to a number 0 are breaking preview in the SQL editor
// This fixes that but as as TODO we should figure out why this is happening
resolved: String(resolved),
state: getResolvableState(fullError ?? error, autocompleteStatus.value !== null),
state:
state ?? getResolvableState(fullError ?? error, autocompleteStatus.value !== null),
error: fullError,
};
}
@@ -378,11 +390,17 @@ export const useExpressionEditor = ({
}
async function resolve(resolvable: string, target: TargetItem | null) {
const result: { resolved: unknown; error: boolean; fullError: Error | null } = {
const result: {
resolved: unknown;
error: boolean;
fullError: Error | null;
state?: ResolvableState;
} = {
resolved: undefined,
error: false,
fullError: null,
};
const isCredentialModal = !expressionLocalResolveContext.value && isCredentialsModalOpen();
try {
// Deprecated functions still resolve on the backend, but we surface them
@@ -397,7 +415,7 @@ export const useExpressionEditor = ({
additionalKeys: toValue(additionalData),
});
} else if (
isCredentialsModalOpen() ||
isCredentialModal ||
(!ndvStore.value.activeNode && toValue(targetNodeParameterContext) === undefined)
) {
// e.g. credential modal
@@ -435,11 +453,19 @@ export const useExpressionEditor = ({
}
if (result.resolved === undefined) {
result.resolved = isUncalledExpressionExtension(resolvable)
? i18n.baseText('expressionEditor.uncalledFunction')
: i18n.baseText('expressionModalInput.undefined');
const secretPreview = isCredentialModal
? getExternalSecretPreview(resolvable, toValue(additionalData).$secrets)
: undefined;
result.error = true;
if (secretPreview) {
result.resolved = secretPreview.text;
result.state = secretPreview.exists ? 'pending' : 'invalid';
} else {
result.resolved = isUncalledExpressionExtension(resolvable)
? i18n.baseText('expressionEditor.uncalledFunction')
: i18n.baseText('expressionModalInput.undefined');
result.error = true;
}
}
return result;