fix(core): Add timeout to external secrets provider update to prevent startup hang (#29682)

packages/cli/src/modules/external-secrets.ee/__tests__/external-secrets-manager.ee.test.ts

@@ -1,6 +1,8 @@
-import { mockLogger } from '@n8n/backend-test-utils';
+import { Logger } from '@n8n/backend-common';
+import { mockInstance, mockLogger } from '@n8n/backend-test-utils';
 import type { Settings, SettingsRepository } from '@n8n/db';
 import { captor, mock } from 'jest-mock-extended';
+import { OperationalError } from 'n8n-workflow';
 
 import type { License } from '@/license';
 import {
@@ -8,11 +10,12 @@ import {
 	DummyProvider,
 	ErrorProvider,
 	FailedProvider,
+	HangingUpdateProvider,
 	MockProviders,
 } from '@test/external-secrets/utils';
 import { mockCipher } from '@test/mocking';
 
-import { EXTERNAL_SECRETS_DB_KEY } from '../constants';
+import { EXTERNAL_SECRETS_DB_KEY, EXTERNAL_SECRETS_PROVIDER_UPDATE_TIMEOUT_MS } from '../constants';
 import { ExternalSecretsManager } from '../external-secrets-manager.ee';
 import type { ExternalSecretsSettings } from '../types';
 
@@ -168,6 +171,21 @@ describe('External Secrets Manager', () => {
 			expect(result).toBe(true);
 		});
 
+		test('should return false when provider update hangs until timeout', async () => {
+			mockProvidersInstance.setProviders({
+				dummy: HangingUpdateProvider,
+			});
+
+			const initPromise = manager.init();
+			await jest.advanceTimersByTimeAsync(EXTERNAL_SECRETS_PROVIDER_UPDATE_TIMEOUT_MS);
+			await initPromise;
+
+			const updatePromise = manager.updateProvider('dummy');
+			await jest.advanceTimersByTimeAsync(EXTERNAL_SECRETS_PROVIDER_UPDATE_TIMEOUT_MS);
+
+			await expect(updatePromise).resolves.toBe(false);
+		});
+
 		test('should return false if provider is not connected', async () => {
 			mockProvidersInstance.setProviders({
 				dummy: ErrorProvider,
@@ -191,6 +209,56 @@ describe('External Secrets Manager', () => {
 		});
 	});
 
+	describe('updateSecrets timeout', () => {
+		test('should complete refresh when provider update never resolves', async () => {
+			mockProvidersInstance.setProviders({
+				dummy: HangingUpdateProvider,
+			});
+
+			const initPromise = manager.init();
+			await jest.advanceTimersByTimeAsync(EXTERNAL_SECRETS_PROVIDER_UPDATE_TIMEOUT_MS);
+			await initPromise;
+
+			expect(manager.initialized).toBe(true);
+
+			const updatePromise = manager.updateSecrets();
+			await jest.advanceTimersByTimeAsync(EXTERNAL_SECRETS_PROVIDER_UPDATE_TIMEOUT_MS);
+			await updatePromise;
+		});
+
+		test('should log OperationalError when provider update times out', async () => {
+			mockProvidersInstance.setProviders({
+				dummy: HangingUpdateProvider,
+			});
+
+			const scopedLogger = mock<Logger>();
+			const rootLogger = mockInstance(Logger);
+			rootLogger.scoped.mockReturnValue(scopedLogger);
+
+			manager = new ExternalSecretsManager(
+				rootLogger,
+				mock(),
+				settingsRepo,
+				license,
+				mockProvidersInstance,
+				cipher,
+				mock(),
+				mock(),
+			);
+
+			const initPromise = manager.init();
+			await jest.advanceTimersByTimeAsync(EXTERNAL_SECRETS_PROVIDER_UPDATE_TIMEOUT_MS);
+			await initPromise;
+
+			expect(scopedLogger.error).toHaveBeenCalledWith(
+				expect.stringContaining('Error updating secrets provider'),
+				expect.objectContaining({
+					error: expect.any(OperationalError),
+				}),
+			);
+		});
+	});
+
 	describe('reloadAllProviders', () => {
 		test('should reload all providers', async () => {
 			await manager.init();

packages/cli/src/modules/external-secrets.ee/constants.ts

@@ -3,6 +3,7 @@ import type { INodeProperties } from 'n8n-workflow';
 export const EXTERNAL_SECRETS_DB_KEY = 'feature.externalSecrets';
 export const EXTERNAL_SECRETS_INITIAL_BACKOFF = 10 * 1000;
 export const EXTERNAL_SECRETS_MAX_BACKOFF = 5 * 60 * 1000;
+export const EXTERNAL_SECRETS_PROVIDER_UPDATE_TIMEOUT_MS = 20_000;
 
 export const EXTERNAL_SECRETS_NAME_REGEX = /^[a-zA-Z0-9\-\_\/]+$/;
 

packages/cli/src/modules/external-secrets.ee/external-secrets-manager.ee.ts

@@ -3,12 +3,19 @@ import { SettingsRepository } from '@n8n/db';
 import { OnPubSubEvent } from '@n8n/decorators';
 import { Service } from '@n8n/di';
 import { Cipher, type IExternalSecretsManager } from 'n8n-core';
-import { jsonParse, type IDataObject, ensureError, UnexpectedError } from 'n8n-workflow';
+import {
+	jsonParse,
+	type IDataObject,
+	ensureError,
+	OperationalError,
+	UnexpectedError,
+} from 'n8n-workflow';
 
 import {
 	EXTERNAL_SECRETS_DB_KEY,
 	EXTERNAL_SECRETS_INITIAL_BACKOFF,
 	EXTERNAL_SECRETS_MAX_BACKOFF,
+	EXTERNAL_SECRETS_PROVIDER_UPDATE_TIMEOUT_MS,
 } from './constants';
 import { ExternalSecretsProviders } from './external-secrets-providers.ee';
 import { ExternalSecretsConfig } from './external-secrets.config';
@@ -191,6 +198,31 @@ export class ExternalSecretsManager implements IExternalSecretsManager {
 		}, currentBackoff);
 	}
 
+	private async refreshProviderSecrets(provider: SecretsProvider): Promise<void> {
+		const timeoutMs = EXTERNAL_SECRETS_PROVIDER_UPDATE_TIMEOUT_MS;
+		let timeoutId: NodeJS.Timeout | undefined;
+		try {
+			await Promise.race([
+				provider.update(),
+				new Promise<never>((_, reject) => {
+					timeoutId = setTimeout(
+						() =>
+							reject(
+								new OperationalError(
+									`External secrets provider "${provider.displayName}" (${provider.name}) timed out after ${timeoutMs}ms`,
+								),
+							),
+						timeoutMs,
+					);
+				}),
+			]);
+		} finally {
+			if (timeoutId !== undefined) {
+				clearTimeout(timeoutId);
+			}
+		}
+	}
+
 	async updateSecrets() {
 		if (!this.license.isExternalSecretsEnabled()) {
 			return;
@@ -199,10 +231,12 @@ export class ExternalSecretsManager implements IExternalSecretsManager {
 			Object.entries(this.providers).map(async ([k, p]) => {
 				try {
 					if (this.cachedSettings[k].connected && p.state === 'connected') {
-						await p.update();
+						await this.refreshProviderSecrets(p);
 					}
-				} catch {
-					this.logger.error(`Error updating secrets provider ${p.displayName} (${p.name}).`);
+				} catch (error) {
+					this.logger.error(`Error updating secrets provider ${p.displayName} (${p.name}).`, {
+						error: ensureError(error),
+					});
 				}
 			}),
 		);
@@ -402,7 +436,7 @@ export class ExternalSecretsManager implements IExternalSecretsManager {
 			return false;
 		}
 		try {
-			await this.providers[provider].update();
+			await this.refreshProviderSecrets(this.providers[provider]);
 			this.broadcastReloadExternalSecretsProviders();
 			this.logger.debug(`External secrets manager updated provider ${provider}`);
 			return true;

packages/cli/test/shared/external-secrets/utils.ts

@@ -100,6 +100,13 @@ export class AnotherDummyProvider extends DummyProvider {
 	name = 'another_dummy';
 }
 
+/** Simulates a store whose `update()` never settles (e.g. hung HTTP client). */
+export class HangingUpdateProvider extends DummyProvider {
+	async update(): Promise<void> {
+		await new Promise(() => {});
+	}
+}
+
 export class ErrorProvider extends SecretsProvider {
 	secrets: Record<string, string> = {};