refactor(core): Port security config (#11440)
Some checks are pending
Test Master / install-and-build (push) Waiting to run
Test Master / Unit tests (18.x) (push) Blocked by required conditions
Test Master / Unit tests (20.x) (push) Blocked by required conditions
Test Master / Unit tests (22.4) (push) Blocked by required conditions
Test Master / Lint (push) Blocked by required conditions
Test Master / Notify Slack on failure (push) Blocked by required conditions

This commit is contained in:
Iván Ovejero 2024-10-29 08:52:07 +01:00 committed by GitHub
parent c152a3ac56
commit 097f93564c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 69 additions and 39 deletions

View file

@ -0,0 +1,27 @@
import { Config, Env } from '../decorators';
@Config
export class SecurityConfig {
/**
* Which directories to limit n8n's access to. Separate multiple dirs with semicolon `;`.
*
* @example N8N_RESTRICT_FILE_ACCESS_TO=/home/user/.n8n;/home/user/n8n-data
*/
@Env('N8N_RESTRICT_FILE_ACCESS_TO')
restrictFileAccessTo: string = '';
/**
* Whether to block access to all files at:
* - the ".n8n" directory,
* - the static cache dir at ~/.cache/n8n/public, and
* - user-defined config files.
*/
@Env('N8N_BLOCK_FILE_ACCESS_TO_N8N_FILES')
blockFileAccessToN8nFiles: boolean = true;
/**
* In a [security audit](https://docs.n8n.io/hosting/securing/security-audit/), how many days for a workflow to be considered abandoned if not executed.
*/
@Env('N8N_SECURITY_AUDIT_DAYS_ABANDONED_WORKFLOW')
daysAbandonedWorkflow: number = 90;
}

View file

@ -13,6 +13,7 @@ import { NodesConfig } from './configs/nodes.config';
import { PublicApiConfig } from './configs/public-api.config'; import { PublicApiConfig } from './configs/public-api.config';
import { TaskRunnersConfig } from './configs/runners.config'; import { TaskRunnersConfig } from './configs/runners.config';
import { ScalingModeConfig } from './configs/scaling-mode.config'; import { ScalingModeConfig } from './configs/scaling-mode.config';
import { SecurityConfig } from './configs/security.config';
import { SentryConfig } from './configs/sentry.config'; import { SentryConfig } from './configs/sentry.config';
import { TemplatesConfig } from './configs/templates.config'; import { TemplatesConfig } from './configs/templates.config';
import { UserManagementConfig } from './configs/user-management.config'; import { UserManagementConfig } from './configs/user-management.config';
@ -22,6 +23,7 @@ import { Config, Env, Nested } from './decorators';
export { Config, Env, Nested } from './decorators'; export { Config, Env, Nested } from './decorators';
export { TaskRunnersConfig } from './configs/runners.config'; export { TaskRunnersConfig } from './configs/runners.config';
export { SecurityConfig } from './configs/security.config';
export { LOG_SCOPES } from './configs/logging.config'; export { LOG_SCOPES } from './configs/logging.config';
export type { LogScope } from './configs/logging.config'; export type { LogScope } from './configs/logging.config';
@ -106,4 +108,7 @@ export class GlobalConfig {
@Nested @Nested
license: LicenseConfig; license: LicenseConfig;
@Nested
security: SecurityConfig;
} }

View file

@ -265,6 +265,11 @@ describe('GlobalConfig', () => {
tenantId: 1, tenantId: 1,
cert: '', cert: '',
}, },
security: {
restrictFileAccessTo: '',
blockFileAccessToN8nFiles: true,
daysAbandonedWorkflow: 90,
},
}; };
it('should use all default values when no env variables are defined', () => { it('should use all default values when no env variables are defined', () => {

View file

@ -1,8 +1,8 @@
import { SecurityConfig } from '@n8n/config';
import { Flags } from '@oclif/core'; import { Flags } from '@oclif/core';
import { ApplicationError } from 'n8n-workflow'; import { ApplicationError } from 'n8n-workflow';
import { Container } from 'typedi'; import { Container } from 'typedi';
import config from '@/config';
import { RISK_CATEGORIES } from '@/security-audit/constants'; import { RISK_CATEGORIES } from '@/security-audit/constants';
import { SecurityAuditService } from '@/security-audit/security-audit.service'; import { SecurityAuditService } from '@/security-audit/security-audit.service';
import type { Risk } from '@/security-audit/types'; import type { Risk } from '@/security-audit/types';
@ -26,7 +26,7 @@ export class SecurityAudit extends BaseCommand {
}), }),
'days-abandoned-workflow': Flags.integer({ 'days-abandoned-workflow': Flags.integer({
default: config.getEnv('security.audit.daysAbandonedWorkflow'), default: Container.get(SecurityConfig).daysAbandonedWorkflow,
description: 'Days for a workflow to be considered abandoned if not executed', description: 'Days for a workflow to be considered abandoned if not executed',
}), }),
}; };

View file

@ -187,29 +187,6 @@ export const schema = {
doc: 'Public URL where the editor is accessible. Also used for emails sent from n8n.', doc: 'Public URL where the editor is accessible. Also used for emails sent from n8n.',
}, },
security: {
restrictFileAccessTo: {
doc: 'If set only files in that directories can be accessed. Multiple directories can be separated by semicolon (";").',
format: String,
default: '',
env: 'N8N_RESTRICT_FILE_ACCESS_TO',
},
blockFileAccessToN8nFiles: {
doc: 'If set to true it will block access to all files in the ".n8n" directory, the static cache dir at ~/.cache/n8n/public, and user defined config files.',
format: Boolean,
default: true,
env: 'N8N_BLOCK_FILE_ACCESS_TO_N8N_FILES',
},
audit: {
daysAbandonedWorkflow: {
doc: 'Days for a workflow to be considered abandoned if not executed',
format: Number,
default: 90,
env: 'N8N_SECURITY_AUDIT_DAYS_ABANDONED_WORKFLOW',
},
},
},
workflowTagsDisabled: { workflowTagsDisabled: {
format: Boolean, format: Boolean,
default: false, default: false,

View file

@ -1,7 +1,7 @@
import { SecurityConfig } from '@n8n/config';
import type { IWorkflowBase } from 'n8n-workflow'; import type { IWorkflowBase } from 'n8n-workflow';
import { Service } from 'typedi'; import { Service } from 'typedi';
import config from '@/config';
import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; import type { WorkflowEntity } from '@/databases/entities/workflow-entity';
import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository';
import { ExecutionDataRepository } from '@/databases/repositories/execution-data.repository'; import { ExecutionDataRepository } from '@/databases/repositories/execution-data.repository';
@ -15,10 +15,11 @@ export class CredentialsRiskReporter implements RiskReporter {
private readonly credentialsRepository: CredentialsRepository, private readonly credentialsRepository: CredentialsRepository,
private readonly executionRepository: ExecutionRepository, private readonly executionRepository: ExecutionRepository,
private readonly executionDataRepository: ExecutionDataRepository, private readonly executionDataRepository: ExecutionDataRepository,
private readonly securityConfig: SecurityConfig,
) {} ) {}
async report(workflows: WorkflowEntity[]) { async report(workflows: WorkflowEntity[]) {
const days = config.getEnv('security.audit.daysAbandonedWorkflow'); const days = this.securityConfig.daysAbandonedWorkflow;
const allExistingCreds = await this.getAllExistingCreds(); const allExistingCreds = await this.getAllExistingCreds();
const { credsInAnyUse, credsInActiveUse } = await this.getAllCredsInUse(workflows); const { credsInAnyUse, credsInActiveUse } = await this.getAllCredsInUse(workflows);

View file

@ -1,3 +1,4 @@
import { SecurityConfig } from '@n8n/config';
import Container, { Service } from 'typedi'; import Container, { Service } from 'typedi';
import config from '@/config'; import config from '@/config';
@ -8,7 +9,10 @@ import { toReportTitle } from '@/security-audit/utils';
@Service() @Service()
export class SecurityAuditService { export class SecurityAuditService {
constructor(private readonly workflowRepository: WorkflowRepository) {} constructor(
private readonly workflowRepository: WorkflowRepository,
private readonly securityConfig: SecurityConfig,
) {}
private reporters: { private reporters: {
[name: string]: RiskReporter; [name: string]: RiskReporter;
@ -19,7 +23,7 @@ export class SecurityAuditService {
await this.initReporters(categories); await this.initReporters(categories);
const daysFromEnv = config.getEnv('security.audit.daysAbandonedWorkflow'); const daysFromEnv = this.securityConfig.daysAbandonedWorkflow;
if (daysAbandonedWorkflow) { if (daysAbandonedWorkflow) {
config.set('security.audit.daysAbandonedWorkflow', daysAbandonedWorkflow); config.set('security.audit.daysAbandonedWorkflow', daysAbandonedWorkflow);

View file

@ -1,5 +1,5 @@
import type { FrontendSettings, ITelemetrySettings } from '@n8n/api-types'; import type { FrontendSettings, ITelemetrySettings } from '@n8n/api-types';
import { GlobalConfig } from '@n8n/config'; import { GlobalConfig, SecurityConfig } from '@n8n/config';
import { createWriteStream } from 'fs'; import { createWriteStream } from 'fs';
import { mkdir } from 'fs/promises'; import { mkdir } from 'fs/promises';
import uniq from 'lodash/uniq'; import uniq from 'lodash/uniq';
@ -46,6 +46,7 @@ export class FrontendService {
private readonly mailer: UserManagementMailer, private readonly mailer: UserManagementMailer,
private readonly instanceSettings: InstanceSettings, private readonly instanceSettings: InstanceSettings,
private readonly urlService: UrlService, private readonly urlService: UrlService,
private readonly securityConfig: SecurityConfig,
) { ) {
loadNodesAndCredentials.addPostProcessor(async () => await this.generateTypes()); loadNodesAndCredentials.addPostProcessor(async () => await this.generateTypes());
void this.generateTypes(); void this.generateTypes();
@ -225,7 +226,7 @@ export class FrontendService {
maxCount: config.getEnv('executions.pruneDataMaxCount'), maxCount: config.getEnv('executions.pruneDataMaxCount'),
}, },
security: { security: {
blockFileAccessToN8nFiles: config.getEnv('security.blockFileAccessToN8nFiles'), blockFileAccessToN8nFiles: this.securityConfig.blockFileAccessToN8nFiles,
}, },
}; };
} }

View file

@ -1,7 +1,8 @@
import type { SecurityConfig } from '@n8n/config';
import { mock } from 'jest-mock-extended';
import Container from 'typedi'; import Container from 'typedi';
import { v4 as uuid } from 'uuid'; import { v4 as uuid } from 'uuid';
import config from '@/config';
import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository';
import { ExecutionDataRepository } from '@/databases/repositories/execution-data.repository'; import { ExecutionDataRepository } from '@/databases/repositories/execution-data.repository';
import { ExecutionRepository } from '@/databases/repositories/execution.repository'; import { ExecutionRepository } from '@/databases/repositories/execution.repository';
@ -15,10 +16,15 @@ import * as testDb from '../shared/test-db';
let securityAuditService: SecurityAuditService; let securityAuditService: SecurityAuditService;
const securityConfig = mock<SecurityConfig>({ daysAbandonedWorkflow: 90 });
beforeAll(async () => { beforeAll(async () => {
await testDb.init(); await testDb.init();
securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository)); securityAuditService = new SecurityAuditService(
Container.get(WorkflowRepository),
securityConfig,
);
}); });
beforeEach(async () => { beforeEach(async () => {
@ -154,7 +160,7 @@ test('should report credential in not recently executed workflow', async () => {
const workflow = await Container.get(WorkflowRepository).save(workflowDetails); const workflow = await Container.get(WorkflowRepository).save(workflowDetails);
const date = new Date(); const date = new Date();
date.setDate(date.getDate() - config.getEnv('security.audit.daysAbandonedWorkflow') - 1); date.setDate(date.getDate() - securityConfig.daysAbandonedWorkflow - 1);
const savedExecution = await Container.get(ExecutionRepository).save({ const savedExecution = await Container.get(ExecutionRepository).save({
finished: true, finished: true,
@ -223,7 +229,7 @@ test('should not report credentials in recently executed workflow', async () =>
const workflow = await Container.get(WorkflowRepository).save(workflowDetails); const workflow = await Container.get(WorkflowRepository).save(workflowDetails);
const date = new Date(); const date = new Date();
date.setDate(date.getDate() - config.getEnv('security.audit.daysAbandonedWorkflow') + 1); date.setDate(date.getDate() - securityConfig.daysAbandonedWorkflow + 1);
const savedExecution = await Container.get(ExecutionRepository).save({ const savedExecution = await Container.get(ExecutionRepository).save({
finished: true, finished: true,

View file

@ -1,3 +1,4 @@
import { mock } from 'jest-mock-extended';
import Container from 'typedi'; import Container from 'typedi';
import { v4 as uuid } from 'uuid'; import { v4 as uuid } from 'uuid';
@ -18,7 +19,7 @@ let securityAuditService: SecurityAuditService;
beforeAll(async () => { beforeAll(async () => {
await testDb.init(); await testDb.init();
securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository)); securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository), mock());
}); });
beforeEach(async () => { beforeEach(async () => {

View file

@ -1,3 +1,4 @@
import { mock } from 'jest-mock-extended';
import Container from 'typedi'; import Container from 'typedi';
import { v4 as uuid } from 'uuid'; import { v4 as uuid } from 'uuid';
@ -13,7 +14,7 @@ let securityAuditService: SecurityAuditService;
beforeAll(async () => { beforeAll(async () => {
await testDb.init(); await testDb.init();
securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository)); securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository), mock());
}); });
beforeEach(async () => { beforeEach(async () => {

View file

@ -1,3 +1,4 @@
import { mock } from 'jest-mock-extended';
import { NodeConnectionType } from 'n8n-workflow'; import { NodeConnectionType } from 'n8n-workflow';
import Container from 'typedi'; import Container from 'typedi';
import { v4 as uuid } from 'uuid'; import { v4 as uuid } from 'uuid';
@ -23,7 +24,7 @@ let securityAuditService: SecurityAuditService;
beforeAll(async () => { beforeAll(async () => {
await testDb.init(); await testDb.init();
securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository)); securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository), mock());
simulateUpToDateInstance(); simulateUpToDateInstance();
}); });

View file

@ -1,3 +1,4 @@
import { mock } from 'jest-mock-extended';
import { Container } from 'typedi'; import { Container } from 'typedi';
import { v4 as uuid } from 'uuid'; import { v4 as uuid } from 'uuid';
@ -24,7 +25,7 @@ let securityAuditService: SecurityAuditService;
beforeAll(async () => { beforeAll(async () => {
await testDb.init(); await testDb.init();
securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository)); securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository), mock());
}); });
beforeEach(async () => { beforeEach(async () => {