feat(core): Enforce config file permissions on startup (#11328)

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <netroy@users.noreply.github.com>

packages/@n8n/config/src/index.ts

@@ -19,6 +19,7 @@ import { UserManagementConfig } from './configs/user-management.config';
 import { VersionNotificationsConfig } from './configs/version-notifications.config';
 import { WorkflowsConfig } from './configs/workflows.config';
 import { Config, Env, Nested } from './decorators';
+export { Config, Env, Nested } from './decorators';
 
 export { LOG_SCOPES } from './configs/logging.config';
 export type { LogScope } from './configs/logging.config';

packages/cli/src/environments/source-control/__tests__/source-control.service.test.ts

@@ -6,7 +6,7 @@ import { SourceControlService } from '@/environments/source-control/source-contr
 
 describe('SourceControlService', () => {
 	const preferencesService = new SourceControlPreferencesService(
-		new InstanceSettings(),
+		new InstanceSettings(mock()),
 		mock(),
 		mock(),
 	);

packages/cli/src/executions/__tests__/execution-recovery.service.test.ts

@@ -22,7 +22,7 @@ import { setupMessages } from './utils';
 
 describe('ExecutionRecoveryService', () => {
 	const push = mockInstance(Push);
-	const instanceSettings = new InstanceSettings();
+	const instanceSettings = new InstanceSettings(mock());
 
 	let executionRecoveryService: ExecutionRecoveryService;
 	let executionRepository: ExecutionRepository;

packages/cli/test/integration/pruning.service.test.ts

@@ -22,7 +22,7 @@ import { mockInstance } from '../shared/mocking';
 
 describe('softDeleteOnPruningCycle()', () => {
 	let pruningService: PruningService;
-	const instanceSettings = new InstanceSettings();
+	const instanceSettings = new InstanceSettings(mock());
 	instanceSettings.markAsLeader();
 
 	const now = new Date();

packages/cli/test/setup-test-folder.ts

@@ -14,5 +14,8 @@ process.env.N8N_USER_FOLDER = testDir;
 writeFileSync(
 	join(testDir, '.n8n/config'),
 	JSON.stringify({ encryptionKey: 'test_key', instanceId: '123' }),
-	'utf-8',
+	{
+		encoding: 'utf-8',
+		mode: 0o600,
+	},
 );

packages/core/package.json

@@ -38,6 +38,7 @@
   "dependencies": {
     "@langchain/core": "catalog:",
     "@n8n/client-oauth2": "workspace:*",
+    "@n8n/config": "workspace:*",
     "aws4": "1.11.0",
     "axios": "catalog:",
     "concat-stream": "2.0.0",

packages/core/src/InstanceSettings.ts

@@ -1,10 +1,12 @@
 import { createHash, randomBytes } from 'crypto';
-import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'fs';
-import { ApplicationError, jsonParse, ALPHABET } from 'n8n-workflow';
+import { chmodSync, existsSync, mkdirSync, readFileSync, statSync, writeFileSync } from 'fs';
+import { ApplicationError, jsonParse, ALPHABET, toResult } from 'n8n-workflow';
 import { customAlphabet } from 'nanoid';
 import path from 'path';
 import { Service } from 'typedi';
 
+import { InstanceSettingsConfig } from './InstanceSettingsConfig';
+
 const nanoid = customAlphabet(ALPHABET, 16);
 
 interface ReadOnlySettings {
@@ -41,6 +43,8 @@ export class InstanceSettings {
 
 	private readonly settingsFile = path.join(this.n8nFolder, 'config');
 
+	readonly enforceSettingsFilePermissions = this.loadEnforceSettingsFilePermissionsFlag();
+
 	private settings = this.loadOrCreate();
 
 	/**
@@ -53,7 +57,7 @@ export class InstanceSettings {
 
 	readonly instanceType: InstanceType;
 
-	constructor() {
+	constructor(private readonly config: InstanceSettingsConfig) {
 		const command = process.argv[2];
 		this.instanceType = ['webhook', 'worker'].includes(command)
 			? (command as InstanceType)
@@ -126,6 +130,7 @@ export class InstanceSettings {
 	private loadOrCreate(): Settings {
 		if (existsSync(this.settingsFile)) {
 			const content = readFileSync(this.settingsFile, 'utf8');
+			this.ensureSettingsFilePermissions();
 
 			const settings = jsonParse<Settings>(content, {
 				errorMessage: `Error parsing n8n-config file "${this.settingsFile}". It does not seem to be valid JSON.`,
@@ -155,6 +160,7 @@ export class InstanceSettings {
 		if (!inTest && !process.env.N8N_ENCRYPTION_KEY) {
 			console.info(`No encryption key found - Auto-generated and saved to: ${this.settingsFile}`);
 		}
+		this.ensureSettingsFilePermissions();
 
 		return settings;
 	}
@@ -168,6 +174,93 @@ export class InstanceSettings {
 
 	private save(settings: Settings) {
 		this.settings = settings;
-		writeFileSync(this.settingsFile, JSON.stringify(settings, null, '\t'), 'utf-8');
+		writeFileSync(this.settingsFile, JSON.stringify(this.settings, null, '\t'), {
+			mode: this.enforceSettingsFilePermissions.enforce ? 0o600 : undefined,
+			encoding: 'utf-8',
+		});
+	}
+
+	private loadEnforceSettingsFilePermissionsFlag(): {
+		isSet: boolean;
+		enforce: boolean;
+	} {
+		const { enforceSettingsFilePermissions } = this.config;
+		const isEnvVarSet = !!process.env.N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS;
+		if (this.isWindows()) {
+			if (isEnvVarSet) {
+				console.warn(
+					'Ignoring N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS as it is not supported on Windows.',
+				);
+			}
+
+			return {
+				isSet: isEnvVarSet,
+				enforce: false,
+			};
+		}
+
+		return {
+			isSet: isEnvVarSet,
+			enforce: enforceSettingsFilePermissions,
+		};
+	}
+
+	/**
+	 * Ensures that the settings file has the r/w permissions only for the owner.
+	 */
+	private ensureSettingsFilePermissions() {
+		// If the flag is explicitly set to false, skip the check
+		if (this.enforceSettingsFilePermissions.isSet && !this.enforceSettingsFilePermissions.enforce) {
+			return;
+		}
+		if (this.isWindows()) {
+			// Ignore windows as it does not support chmod. We have already logged a warning
+			return;
+		}
+
+		const permissionsResult = toResult(() => {
+			const stats = statSync(this.settingsFile);
+			return stats.mode & 0o777;
+		});
+		// If we can't determine the permissions, log a warning and skip the check
+		if (!permissionsResult.ok) {
+			console.warn(
+				`Could not ensure settings file permissions: ${permissionsResult.error.message}. To skip this check, set N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS=false.`,
+			);
+			return;
+		}
+
+		const arePermissionsCorrect = permissionsResult.result === 0o600;
+		if (arePermissionsCorrect) {
+			return;
+		}
+
+		// If the permissions are incorrect and the flag is not set, log a warning
+		if (!this.enforceSettingsFilePermissions.isSet) {
+			console.warn(
+				`Permissions 0${permissionsResult.result.toString(8)} for n8n settings file ${this.settingsFile} are too wide. This is ignored for now, but in the future n8n will attempt to change the permissions automatically. To automatically enforce correct permissions now set N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS=true (recommended), or turn this check off set N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS=false.`,
+			);
+			// The default is false so we skip the enforcement for now
+			return;
+		}
+
+		if (this.enforceSettingsFilePermissions.enforce) {
+			console.warn(
+				`Permissions 0${permissionsResult.result.toString(8)} for n8n settings file ${this.settingsFile} are too wide. Changing permissions to 0600..`,
+			);
+			const chmodResult = toResult(() => chmodSync(this.settingsFile, 0o600));
+			if (!chmodResult.ok) {
+				// Some filesystems don't support permissions. In this case we log the
+				// error and ignore it. We might want to prevent the app startup in the
+				// future in this case.
+				console.warn(
+					`Could not enforce settings file permissions: ${chmodResult.error.message}. To skip this check, set N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS=false.`,
+				);
+			}
+		}
+	}
+
+	private isWindows() {
+		return process.platform === 'win32';
 	}
 }

packages/core/src/InstanceSettingsConfig.ts

@@ -0,0 +1,12 @@
+import { Config, Env } from '@n8n/config';
+
+@Config
+export class InstanceSettingsConfig {
+	/**
+	 * Whether to enforce that n8n settings file doesn't have overly wide permissions.
+	 * If set to true, n8n will check the permissions of the settings file and
+	 * attempt change them to 0600 (only owner has rw access) if they are too wide.
+	 */
+	@Env('N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS')
+	enforceSettingsFilePermissions: boolean = false;
+}

packages/core/test/InstanceSettings.test.ts

@@ -1,20 +1,35 @@
 import fs from 'fs';
 
-import { InstanceSettings } from '@/InstanceSettings';
+import { InstanceSettings } from '../src/InstanceSettings';
+import { InstanceSettingsConfig } from '../src/InstanceSettingsConfig';
 
 describe('InstanceSettings', () => {
 	process.env.N8N_USER_FOLDER = '/test';
 
 	const existSpy = jest.spyOn(fs, 'existsSync');
-	beforeEach(() => jest.resetAllMocks());
+	const statSpy = jest.spyOn(fs, 'statSync');
+	const chmodSpy = jest.spyOn(fs, 'chmodSync');
+
+	const createSettingsInstance = (opts?: Partial<InstanceSettingsConfig>) =>
+		new InstanceSettings({
+			...new InstanceSettingsConfig(),
+			...opts,
+		});
+
+	beforeEach(() => {
+		jest.resetAllMocks();
+		statSpy.mockReturnValue({ mode: 0o600 } as fs.Stats);
+	});
 
 	describe('If the settings file exists', () => {
 		const readSpy = jest.spyOn(fs, 'readFileSync');
-		beforeEach(() => existSpy.mockReturnValue(true));
+		beforeEach(() => {
+			existSpy.mockReturnValue(true);
+		});
 
 		it('should load settings from the file', () => {
 			readSpy.mockReturnValue(JSON.stringify({ encryptionKey: 'test_key' }));
-			const settings = new InstanceSettings();
+			const settings = createSettingsInstance();
 			expect(settings.encryptionKey).toEqual('test_key');
 			expect(settings.instanceId).toEqual(
 				'6ce26c63596f0cc4323563c529acfca0cccb0e57f6533d79a60a42c9ff862ae7',
@@ -23,13 +38,52 @@ describe('InstanceSettings', () => {
 
 		it('should throw error if settings file is not valid JSON', () => {
 			readSpy.mockReturnValue('{"encryptionKey":"test_key"');
-			expect(() => new InstanceSettings()).toThrowError();
+			expect(() => createSettingsInstance()).toThrowError();
 		});
 
 		it('should throw if the env and file keys do not match', () => {
 			readSpy.mockReturnValue(JSON.stringify({ encryptionKey: 'key_1' }));
 			process.env.N8N_ENCRYPTION_KEY = 'key_2';
-			expect(() => new InstanceSettings()).toThrowError();
+			expect(() => createSettingsInstance()).toThrowError();
+		});
+
+		it('should check if the settings file has the correct permissions', () => {
+			process.env.N8N_ENCRYPTION_KEY = 'test_key';
+			readSpy.mockReturnValueOnce(JSON.stringify({ encryptionKey: 'test_key' }));
+			statSpy.mockReturnValueOnce({ mode: 0o600 } as fs.Stats);
+			const settings = createSettingsInstance();
+			expect(settings.encryptionKey).toEqual('test_key');
+			expect(settings.instanceId).toEqual(
+				'6ce26c63596f0cc4323563c529acfca0cccb0e57f6533d79a60a42c9ff862ae7',
+			);
+			expect(statSpy).toHaveBeenCalledWith('/test/.n8n/config');
+		});
+
+		it('should check the permissions but not fix them if settings file has incorrect permissions by default', () => {
+			readSpy.mockReturnValueOnce(JSON.stringify({ encryptionKey: 'test_key' }));
+			statSpy.mockReturnValueOnce({ mode: 0o644 } as fs.Stats);
+			createSettingsInstance();
+			expect(statSpy).toHaveBeenCalledWith('/test/.n8n/config');
+			expect(chmodSpy).not.toHaveBeenCalled();
+		});
+
+		it("should not check the permissions if 'N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS' is false", () => {
+			process.env.N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS = 'false';
+			readSpy.mockReturnValueOnce(JSON.stringify({ encryptionKey: 'test_key' }));
+			createSettingsInstance();
+			expect(statSpy).not.toHaveBeenCalled();
+			expect(chmodSpy).not.toHaveBeenCalled();
+		});
+
+		it("should fix the permissions of the settings file if 'N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS' is true", () => {
+			process.env.N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS = 'true';
+			readSpy.mockReturnValueOnce(JSON.stringify({ encryptionKey: 'test_key' }));
+			statSpy.mockReturnValueOnce({ mode: 0o644 } as fs.Stats);
+			createSettingsInstance({
+				enforceSettingsFilePermissions: true,
+			});
+			expect(statSpy).toHaveBeenCalledWith('/test/.n8n/config');
+			expect(chmodSpy).toHaveBeenCalledWith('/test/.n8n/config', 0o600);
 		});
 	});
 
@@ -42,20 +96,58 @@ describe('InstanceSettings', () => {
 			writeFileSpy.mockReturnValue();
 		});
 
-		it('should create a new settings file', () => {
-			const settings = new InstanceSettings();
+		it('should create a new settings file without explicit permissions if N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS is not set', () => {
+			process.env.N8N_ENCRYPTION_KEY = 'key_2';
+			const settings = createSettingsInstance();
 			expect(settings.encryptionKey).not.toEqual('test_key');
 			expect(mkdirSpy).toHaveBeenCalledWith('/test/.n8n', { recursive: true });
 			expect(writeFileSpy).toHaveBeenCalledWith(
 				'/test/.n8n/config',
 				expect.stringContaining('"encryptionKey":'),
-				'utf-8',
+				{
+					encoding: 'utf-8',
+					mode: undefined,
+				},
+			);
+		});
+
+		it('should create a new settings file without explicit permissions if N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS=false', () => {
+			process.env.N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS = 'false';
+			process.env.N8N_ENCRYPTION_KEY = 'key_2';
+			const settings = createSettingsInstance();
+			expect(settings.encryptionKey).not.toEqual('test_key');
+			expect(mkdirSpy).toHaveBeenCalledWith('/test/.n8n', { recursive: true });
+			expect(writeFileSpy).toHaveBeenCalledWith(
+				'/test/.n8n/config',
+				expect.stringContaining('"encryptionKey":'),
+				{
+					encoding: 'utf-8',
+					mode: undefined,
+				},
+			);
+		});
+
+		it('should create a new settings file with explicit permissions if N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS=true', () => {
+			process.env.N8N_ENFORCE_SETTINGS_FILE_PERMISSIONS = 'true';
+			process.env.N8N_ENCRYPTION_KEY = 'key_2';
+			const settings = createSettingsInstance({
+				enforceSettingsFilePermissions: true,
+			});
+			expect(settings.encryptionKey).not.toEqual('test_key');
+			expect(mkdirSpy).toHaveBeenCalledWith('/test/.n8n', { recursive: true });
+			expect(writeFileSpy).toHaveBeenCalledWith(
+				'/test/.n8n/config',
+				expect.stringContaining('"encryptionKey":'),
+				{
+					encoding: 'utf-8',
+					mode: 0o600,
+				},
 			);
 		});
 
 		it('should pick up the encryption key from env var N8N_ENCRYPTION_KEY', () => {
 			process.env.N8N_ENCRYPTION_KEY = 'env_key';
-			const settings = new InstanceSettings();
+			const settings = createSettingsInstance();
 			expect(settings.encryptionKey).toEqual('env_key');
 			expect(settings.instanceId).toEqual(
 				'2c70e12b7a0646f92279f427c7b38e7334d8e5389cff167a1dc30e73f826b683',
@@ -65,7 +157,26 @@ describe('InstanceSettings', () => {
 			expect(writeFileSpy).toHaveBeenCalledWith(
 				'/test/.n8n/config',
 				expect.stringContaining('"encryptionKey":'),
-				'utf-8',
+				{
+					encoding: 'utf-8',
+					mode: undefined,
+				},
+			);
+		});
+
+		it("should not set the permissions of the settings file if 'N8N_IGNORE_SETTINGS_FILE_PERMISSIONS' is true", () => {
+			process.env.N8N_ENCRYPTION_KEY = 'key_2';
+			process.env.N8N_IGNORE_SETTINGS_FILE_PERMISSIONS = 'true';
+			const settings = createSettingsInstance();
+			expect(settings.encryptionKey).not.toEqual('test_key');
+			expect(mkdirSpy).toHaveBeenCalledWith('/test/.n8n', { recursive: true });
+			expect(writeFileSpy).toHaveBeenCalledWith(
+				'/test/.n8n/config',
+				expect.stringContaining('"encryptionKey":'),
+				{
+					encoding: 'utf-8',
+					mode: undefined,
+				},
 			);
 		});
 	});
@@ -77,7 +188,7 @@ describe('InstanceSettings', () => {
 			jest.spyOn(fs, 'existsSync').mockReturnValueOnce(true);
 			jest.spyOn(fs, 'readFileSync').mockReturnValueOnce(JSON.stringify({ encryptionKey }));
 
-			const settings = new InstanceSettings();
+			const settings = createSettingsInstance();
 
 			const [instanceType, nanoid] = settings.hostId.split('-');
 			expect(instanceType).toEqual('main');

packages/core/test/utils.ts

@@ -1,12 +1,11 @@
 import { mock } from 'jest-mock-extended';
 import { Duplex } from 'stream';
 import type { DeepPartial } from 'ts-essentials';
+import type { Constructable } from 'typedi';
 import { Container } from 'typedi';
 
-import type { Class } from '@/Interfaces';
-
 export const mockInstance = <T>(
-	constructor: Class<T>,
+	constructor: Constructable<T>,
 	data: DeepPartial<T> | undefined = undefined,
 ) => {
 	const instance = mock<T>(data);

packages/workflow/src/result.ts

@@ -1,3 +1,5 @@
+import { ensureError } from './errors';
+
 export type ResultOk<T> = { ok: true; result: T };
 export type ResultError<E> = { ok: false; error: E };
 export type Result<T, E> = ResultOk<T> | ResultError<E>;
@@ -11,3 +13,18 @@ export const createResultError = <E = unknown>(error: E): ResultError<E> => ({
 	ok: false,
 	error,
 });
+
+/**
+ * Executes the given function and converts it to a Result object.
+ *
+ * @example
+ * const result = toResult(() => fs.writeFileSync('file.txt', 'Hello, World!'));
+ */
+export const toResult = <T, E extends Error = Error>(fn: () => T): Result<T, E> => {
+	try {
+		return createResultOk<T>(fn());
+	} catch (e) {
+		const error = ensureError(e);
+		return createResultError<E>(error as E);
+	}
+};

pnpm-lock.yaml

@@ -1094,6 +1094,9 @@ importers:
       '@n8n/client-oauth2':
         specifier: workspace:*
         version: link:../@n8n/client-oauth2
+      '@n8n/config':
+        specifier: workspace:*
+        version: link:../@n8n/config
       aws4:
         specifier: 1.11.0
         version: 1.11.0