Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(core): Port security config #11440

Merged
merged 2 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading