Skip to content

Commit

Permalink
refactor(core): Port security config (#11440)
Browse files Browse the repository at this point in the history
  • Loading branch information
ivov authored Oct 29, 2024
1 parent c152a3a commit 097f935
Show file tree
Hide file tree
Showing 13 changed files with 69 additions and 39 deletions.
27 changes: 27 additions & 0 deletions packages/@n8n/config/src/configs/security.config.ts
Original file line number Diff line number Diff line change
@@ -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;
}
5 changes: 5 additions & 0 deletions packages/@n8n/config/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { NodesConfig } from './configs/nodes.config';
import { PublicApiConfig } from './configs/public-api.config';
import { TaskRunnersConfig } from './configs/runners.config';
import { ScalingModeConfig } from './configs/scaling-mode.config';
import { SecurityConfig } from './configs/security.config';
import { SentryConfig } from './configs/sentry.config';
import { TemplatesConfig } from './configs/templates.config';
import { UserManagementConfig } from './configs/user-management.config';
Expand All @@ -22,6 +23,7 @@ import { Config, Env, Nested } from './decorators';

export { Config, Env, Nested } from './decorators';
export { TaskRunnersConfig } from './configs/runners.config';
export { SecurityConfig } from './configs/security.config';
export { LOG_SCOPES } from './configs/logging.config';
export type { LogScope } from './configs/logging.config';

Expand Down Expand Up @@ -106,4 +108,7 @@ export class GlobalConfig {

@Nested
license: LicenseConfig;

@Nested
security: SecurityConfig;
}
5 changes: 5 additions & 0 deletions packages/@n8n/config/test/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,11 @@ describe('GlobalConfig', () => {
tenantId: 1,
cert: '',
},
security: {
restrictFileAccessTo: '',
blockFileAccessToN8nFiles: true,
daysAbandonedWorkflow: 90,
},
};

it('should use all default values when no env variables are defined', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/commands/audit.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { SecurityConfig } from '@n8n/config';
import { Flags } from '@oclif/core';
import { ApplicationError } from 'n8n-workflow';
import { Container } from 'typedi';

import config from '@/config';
import { RISK_CATEGORIES } from '@/security-audit/constants';
import { SecurityAuditService } from '@/security-audit/security-audit.service';
import type { Risk } from '@/security-audit/types';
Expand All @@ -26,7 +26,7 @@ export class SecurityAudit extends BaseCommand {
}),

'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',
}),
};
Expand Down
23 changes: 0 additions & 23 deletions packages/cli/src/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,29 +187,6 @@ export const schema = {
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: {
format: Boolean,
default: false,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { SecurityConfig } from '@n8n/config';
import type { IWorkflowBase } from 'n8n-workflow';
import { Service } from 'typedi';

import config from '@/config';
import type { WorkflowEntity } from '@/databases/entities/workflow-entity';
import { CredentialsRepository } from '@/databases/repositories/credentials.repository';
import { ExecutionDataRepository } from '@/databases/repositories/execution-data.repository';
Expand All @@ -15,10 +15,11 @@ export class CredentialsRiskReporter implements RiskReporter {
private readonly credentialsRepository: CredentialsRepository,
private readonly executionRepository: ExecutionRepository,
private readonly executionDataRepository: ExecutionDataRepository,
private readonly securityConfig: SecurityConfig,
) {}

async report(workflows: WorkflowEntity[]) {
const days = config.getEnv('security.audit.daysAbandonedWorkflow');
const days = this.securityConfig.daysAbandonedWorkflow;

const allExistingCreds = await this.getAllExistingCreds();
const { credsInAnyUse, credsInActiveUse } = await this.getAllCredsInUse(workflows);
Expand Down
8 changes: 6 additions & 2 deletions packages/cli/src/security-audit/security-audit.service.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { SecurityConfig } from '@n8n/config';
import Container, { Service } from 'typedi';

import config from '@/config';
Expand All @@ -8,7 +9,10 @@ import { toReportTitle } from '@/security-audit/utils';

@Service()
export class SecurityAuditService {
constructor(private readonly workflowRepository: WorkflowRepository) {}
constructor(
private readonly workflowRepository: WorkflowRepository,
private readonly securityConfig: SecurityConfig,
) {}

private reporters: {
[name: string]: RiskReporter;
Expand All @@ -19,7 +23,7 @@ export class SecurityAuditService {

await this.initReporters(categories);

const daysFromEnv = config.getEnv('security.audit.daysAbandonedWorkflow');
const daysFromEnv = this.securityConfig.daysAbandonedWorkflow;

if (daysAbandonedWorkflow) {
config.set('security.audit.daysAbandonedWorkflow', daysAbandonedWorkflow);
Expand Down
5 changes: 3 additions & 2 deletions packages/cli/src/services/frontend.service.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { FrontendSettings, ITelemetrySettings } from '@n8n/api-types';
import { GlobalConfig } from '@n8n/config';
import { GlobalConfig, SecurityConfig } from '@n8n/config';
import { createWriteStream } from 'fs';
import { mkdir } from 'fs/promises';
import uniq from 'lodash/uniq';
Expand Down Expand Up @@ -46,6 +46,7 @@ export class FrontendService {
private readonly mailer: UserManagementMailer,
private readonly instanceSettings: InstanceSettings,
private readonly urlService: UrlService,
private readonly securityConfig: SecurityConfig,
) {
loadNodesAndCredentials.addPostProcessor(async () => await this.generateTypes());
void this.generateTypes();
Expand Down Expand Up @@ -225,7 +226,7 @@ export class FrontendService {
maxCount: config.getEnv('executions.pruneDataMaxCount'),
},
security: {
blockFileAccessToN8nFiles: config.getEnv('security.blockFileAccessToN8nFiles'),
blockFileAccessToN8nFiles: this.securityConfig.blockFileAccessToN8nFiles,
},
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import type { SecurityConfig } from '@n8n/config';
import { mock } from 'jest-mock-extended';
import Container from 'typedi';
import { v4 as uuid } from 'uuid';

import config from '@/config';
import { CredentialsRepository } from '@/databases/repositories/credentials.repository';
import { ExecutionDataRepository } from '@/databases/repositories/execution-data.repository';
import { ExecutionRepository } from '@/databases/repositories/execution.repository';
Expand All @@ -15,10 +16,15 @@ import * as testDb from '../shared/test-db';

let securityAuditService: SecurityAuditService;

const securityConfig = mock<SecurityConfig>({ daysAbandonedWorkflow: 90 });

beforeAll(async () => {
await testDb.init();

securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository));
securityAuditService = new SecurityAuditService(
Container.get(WorkflowRepository),
securityConfig,
);
});

beforeEach(async () => {
Expand Down Expand Up @@ -154,7 +160,7 @@ test('should report credential in not recently executed workflow', async () => {
const workflow = await Container.get(WorkflowRepository).save(workflowDetails);

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({
finished: true,
Expand Down Expand Up @@ -223,7 +229,7 @@ test('should not report credentials in recently executed workflow', async () =>
const workflow = await Container.get(WorkflowRepository).save(workflowDetails);

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({
finished: true,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { mock } from 'jest-mock-extended';
import Container from 'typedi';
import { v4 as uuid } from 'uuid';

Expand All @@ -18,7 +19,7 @@ let securityAuditService: SecurityAuditService;
beforeAll(async () => {
await testDb.init();

securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository));
securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository), mock());
});

beforeEach(async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { mock } from 'jest-mock-extended';
import Container from 'typedi';
import { v4 as uuid } from 'uuid';

Expand All @@ -13,7 +14,7 @@ let securityAuditService: SecurityAuditService;
beforeAll(async () => {
await testDb.init();

securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository));
securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository), mock());
});

beforeEach(async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { mock } from 'jest-mock-extended';
import { NodeConnectionType } from 'n8n-workflow';
import Container from 'typedi';
import { v4 as uuid } from 'uuid';
Expand All @@ -23,7 +24,7 @@ let securityAuditService: SecurityAuditService;
beforeAll(async () => {
await testDb.init();

securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository));
securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository), mock());

simulateUpToDateInstance();
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { mock } from 'jest-mock-extended';
import { Container } from 'typedi';
import { v4 as uuid } from 'uuid';

Expand All @@ -24,7 +25,7 @@ let securityAuditService: SecurityAuditService;
beforeAll(async () => {
await testDb.init();

securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository));
securityAuditService = new SecurityAuditService(Container.get(WorkflowRepository), mock());
});

beforeEach(async () => {
Expand Down

0 comments on commit 097f935

Please sign in to comment.