diff --git a/packages/@n8n/config/src/index.ts b/packages/@n8n/config/src/index.ts index b76e5c455c..7b944eac85 100644 --- a/packages/@n8n/config/src/index.ts +++ b/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'; diff --git a/packages/cli/src/environments/source-control/__tests__/source-control.service.test.ts b/packages/cli/src/environments/source-control/__tests__/source-control.service.test.ts index 06dab4f97c..e1ebf0e56a 100644 --- a/packages/cli/src/environments/source-control/__tests__/source-control.service.test.ts +++ b/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(), ); diff --git a/packages/cli/src/executions/__tests__/execution-recovery.service.test.ts b/packages/cli/src/executions/__tests__/execution-recovery.service.test.ts index de7c27a8e8..115a1a52f6 100644 --- a/packages/cli/src/executions/__tests__/execution-recovery.service.test.ts +++ b/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; diff --git a/packages/cli/test/integration/pruning.service.test.ts b/packages/cli/test/integration/pruning.service.test.ts index af79640746..5a53d70315 100644 --- a/packages/cli/test/integration/pruning.service.test.ts +++ b/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(); diff --git a/packages/cli/test/setup-test-folder.ts b/packages/cli/test/setup-test-folder.ts index 94e45e6c90..997a0ec80f 100644 --- a/packages/cli/test/setup-test-folder.ts +++ b/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, + }, ); diff --git a/packages/core/package.json b/packages/core/package.json index 89b96d3c8c..8d6088ccba 100644 --- a/packages/core/package.json +++ b/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", diff --git a/packages/core/src/InstanceSettings.ts b/packages/core/src/InstanceSettings.ts index 4a050db121..7d38f21184 100644 --- a/packages/core/src/InstanceSettings.ts +++ b/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(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'; } } diff --git a/packages/core/src/InstanceSettingsConfig.ts b/packages/core/src/InstanceSettingsConfig.ts new file mode 100644 index 0000000000..60baf8b80f --- /dev/null +++ b/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; +} diff --git a/packages/core/test/InstanceSettings.test.ts b/packages/core/test/InstanceSettings.test.ts index 7bc572b168..22f8fadff8 100644 --- a/packages/core/test/InstanceSettings.test.ts +++ b/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) => + 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'); diff --git a/packages/core/test/utils.ts b/packages/core/test/utils.ts index 7f4862cabd..f1ed54dd03 100644 --- a/packages/core/test/utils.ts +++ b/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 = ( - constructor: Class, + constructor: Constructable, data: DeepPartial | undefined = undefined, ) => { const instance = mock(data); diff --git a/packages/workflow/src/result.ts b/packages/workflow/src/result.ts index 3ee3c18fb7..78dddfe343 100644 --- a/packages/workflow/src/result.ts +++ b/packages/workflow/src/result.ts @@ -1,3 +1,5 @@ +import { ensureError } from './errors'; + export type ResultOk = { ok: true; result: T }; export type ResultError = { ok: false; error: E }; export type Result = ResultOk | ResultError; @@ -11,3 +13,18 @@ export const createResultError = (error: E): ResultError => ({ 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 = (fn: () => T): Result => { + try { + return createResultOk(fn()); + } catch (e) { + const error = ensureError(e); + return createResultError(error as E); + } +}; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index dd1145c94b..fe88e86eef 100644 --- a/pnpm-lock.yaml +++ b/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