From d5481616326b304a76937b104d24326a93f6f565 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 5 Jan 2023 13:28:40 +0100 Subject: [PATCH] feat(core): Security audit (#5034) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * :sparkles: Implement security audit * :zap: Use logger * :test_tube: Fix test * :zap: Switch logger with stdout * :art: Set new logo * :zap: Fill out Public API schema * :pencil2: Fix typo * :zap: Break dependency cycle * :zap: Add security settings values * :test_tube: Test security settings * :zap: Add publicly accessible instance warning * :zap: Add metric to CLI command * :pencil2: Fix typo * :fire: Remove unneeded path alias * :blue_book: Add type import * :fire: Remove inferrable output type * :zap: Set description at correct level * :zap: Rename constant for consistency * :zap: Sort URLs * :zap: Rename local var * :zap: Shorten name * :pencil2: Improve phrasing * :zap: Improve naming * :zap: Fix casing * :pencil2: Add docline * :pencil2: Relocate comment * :zap: Add singular/plurals * :fire: Remove unneeded await * :pencil2: Improve test description * :zap: Optimize with sets * :zap: Adjust post master merge * :pencil2: Improve naming * :zap: Adjust in spy * :test_tube: Fix outdated instance test * :test_tube: Make diagnostics check consistent * :zap: Refactor `getAllExistingCreds` * :zap: Create helper `getNodeTypes` * :bug: Fix `InternalHooksManager` call * :truck: Rename `execution` to `nodes` risk * :zap: Add options to CLI command * :zap: Make days configurable * :revert: Undo changes to `BaseCommand` * :zap: Improve CLI command UX * :zap: Change no-report return value Empty array to trigger empty state on FE. * :zap: Add empty check to `reportInstanceRisk` * :test_tube: Extend Jest `expect` * :blue_book: Augment `jest.Matchers` * :test_tube: Set extend as setup file * :wrench: Override lint rule for `.d.ts` * :zap: Use new matcher * :zap: Update check * :blue_book: Improve typings * :zap: Adjust instance risk check * :pencil2: Rename `execution` → `nodes` in Public API schema * :pencil2: Add clarifying comment * :pencil2: Fix typo * :zap: Validate categories in CLI command * :pencil2: Improve naming * :pencil2: Make audit reference consistent * :blue_book: Fix typing * :zap: Use `finally` in CLI command --- packages/@n8n_io/eslint-config/base.js | 11 +- packages/cli/jest.config.js | 2 +- packages/cli/src/InternalHooks.ts | 7 + packages/cli/src/LoadNodesAndCredentials.ts | 18 +- packages/cli/src/PublicApi/types.d.ts | 14 + .../v1/handlers/audit/audit.handler.ts | 22 ++ .../v1/handlers/audit/spec/paths/audit.yml | 36 +++ .../v1/handlers/audit/spec/schemas/audit.yml | 105 ++++++++ packages/cli/src/PublicApi/v1/openapi.yml | 6 +- .../v1/shared/spec/schemas/_index.yml | 2 + packages/cli/src/audit/constants.ts | 121 +++++++++ packages/cli/src/audit/index.ts | 68 +++++ .../cli/src/audit/risks/credentials.risk.ts | 133 +++++++++ packages/cli/src/audit/risks/database.risk.ts | 106 ++++++++ .../cli/src/audit/risks/filesystem.risk.ts | 35 +++ packages/cli/src/audit/risks/instance.risk.ts | 220 +++++++++++++++ packages/cli/src/audit/risks/nodes.risk.ts | 111 ++++++++ packages/cli/src/audit/types.ts | 86 ++++++ packages/cli/src/audit/utils.ts | 26 ++ packages/cli/src/commands/audit.ts | 106 ++++++++ packages/cli/src/config/schema.ts | 8 + packages/cli/src/constants.ts | 8 +- packages/cli/src/jest.d.ts | 5 + packages/cli/src/utils.ts | 9 + packages/cli/test/extend-expect.ts | 12 + .../audit/credentials.risk.test.ts | 225 ++++++++++++++++ .../integration/audit/database.risk.test.ts | 187 +++++++++++++ .../integration/audit/filesystem.risk.test.ts | 76 ++++++ .../integration/audit/instance.risk.test.ts | 255 ++++++++++++++++++ .../test/integration/audit/nodes.risk.test.ts | 99 +++++++ packages/cli/test/integration/audit/utils.ts | 130 +++++++++ .../nodes-base/nodes/N8n/AuditDescription.ts | 90 +++++++ packages/nodes-base/nodes/N8n/N8n.node.ts | 10 +- packages/nodes-base/nodes/N8n/n8n.svg | 4 +- 34 files changed, 2335 insertions(+), 18 deletions(-) create mode 100644 packages/cli/src/PublicApi/v1/handlers/audit/audit.handler.ts create mode 100644 packages/cli/src/PublicApi/v1/handlers/audit/spec/paths/audit.yml create mode 100644 packages/cli/src/PublicApi/v1/handlers/audit/spec/schemas/audit.yml create mode 100644 packages/cli/src/audit/constants.ts create mode 100644 packages/cli/src/audit/index.ts create mode 100644 packages/cli/src/audit/risks/credentials.risk.ts create mode 100644 packages/cli/src/audit/risks/database.risk.ts create mode 100644 packages/cli/src/audit/risks/filesystem.risk.ts create mode 100644 packages/cli/src/audit/risks/instance.risk.ts create mode 100644 packages/cli/src/audit/risks/nodes.risk.ts create mode 100644 packages/cli/src/audit/types.ts create mode 100644 packages/cli/src/audit/utils.ts create mode 100644 packages/cli/src/commands/audit.ts create mode 100644 packages/cli/src/jest.d.ts create mode 100644 packages/cli/test/extend-expect.ts create mode 100644 packages/cli/test/integration/audit/credentials.risk.test.ts create mode 100644 packages/cli/test/integration/audit/database.risk.test.ts create mode 100644 packages/cli/test/integration/audit/filesystem.risk.test.ts create mode 100644 packages/cli/test/integration/audit/instance.risk.test.ts create mode 100644 packages/cli/test/integration/audit/nodes.risk.test.ts create mode 100644 packages/cli/test/integration/audit/utils.ts create mode 100644 packages/nodes-base/nodes/N8n/AuditDescription.ts diff --git a/packages/@n8n_io/eslint-config/base.js b/packages/@n8n_io/eslint-config/base.js index e1fec4db29..cadc748b5c 100644 --- a/packages/@n8n_io/eslint-config/base.js +++ b/packages/@n8n_io/eslint-config/base.js @@ -228,7 +228,7 @@ const config = (module.exports = { }, { selector: 'property', - format: ['camelCase', 'snake_case'], + format: ['camelCase', 'snake_case', 'UPPER_CASE'], leadingUnderscore: 'allowSingleOrDouble', trailingUnderscore: 'allowSingleOrDouble', }, @@ -404,6 +404,15 @@ const config = (module.exports = { */ 'import/prefer-default-export': 'off', }, + + overrides: [ + { + files: ['**/*.d.ts'], + rules: { + '@typescript-eslint/no-unused-vars': 'off', + }, + }, + ], }); if ('ESLINT_PLUGIN_DIFF_COMMIT' in process.env) { diff --git a/packages/cli/jest.config.js b/packages/cli/jest.config.js index 11a96c384c..993c4301f4 100644 --- a/packages/cli/jest.config.js +++ b/packages/cli/jest.config.js @@ -6,7 +6,7 @@ module.exports = { }, globalSetup: '/test/setup.ts', globalTeardown: '/test/teardown.ts', - setupFilesAfterEnv: ['/test/setup-mocks.ts'], + setupFilesAfterEnv: ['/test/setup-mocks.ts', '/test/extend-expect.ts'], moduleNameMapper: { '^@/(.*)$': '/src/$1', '^@db/(.*)$': '/src/databases/$1', diff --git a/packages/cli/src/InternalHooks.ts b/packages/cli/src/InternalHooks.ts index 94f75d98a3..9eeed21458 100644 --- a/packages/cli/src/InternalHooks.ts +++ b/packages/cli/src/InternalHooks.ts @@ -869,4 +869,11 @@ export class InternalHooksClass implements IInternalHooksClass { async onLicenseRenewAttempt(data: { success: boolean }): Promise { await this.telemetry.track('Instance attempted to refresh license', data); } + + /** + * Audit + */ + async onAuditGeneratedViaCli() { + return this.telemetry.track('Instance generated security audit via CLI command'); + } } diff --git a/packages/cli/src/LoadNodesAndCredentials.ts b/packages/cli/src/LoadNodesAndCredentials.ts index bcdbdab6a2..1d0c5262b9 100644 --- a/packages/cli/src/LoadNodesAndCredentials.ts +++ b/packages/cli/src/LoadNodesAndCredentials.ts @@ -142,21 +142,19 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials { } } - async loadNodesFromCustomDirectories(): Promise { - // Read nodes and credentials from custom directories - const customDirectories = []; + getCustomDirectories(): string[] { + const customDirectories = [UserSettings.getUserN8nFolderCustomExtensionPath()]; - // Add "custom" folder in user-n8n folder - customDirectories.push(UserSettings.getUserN8nFolderCustomExtensionPath()); - - // Add folders from special environment variable if (process.env[CUSTOM_EXTENSION_ENV] !== undefined) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const customExtensionFolders = process.env[CUSTOM_EXTENSION_ENV]!.split(';'); + const customExtensionFolders = process.env[CUSTOM_EXTENSION_ENV].split(';'); customDirectories.push(...customExtensionFolders); } - for (const directory of customDirectories) { + return customDirectories; + } + + async loadNodesFromCustomDirectories(): Promise { + for (const directory of this.getCustomDirectories()) { await this.runDirectoryLoader(CustomDirectoryLoader, directory); } } diff --git a/packages/cli/src/PublicApi/types.d.ts b/packages/cli/src/PublicApi/types.d.ts index 6a739bc2d5..f6353dea9e 100644 --- a/packages/cli/src/PublicApi/types.d.ts +++ b/packages/cli/src/PublicApi/types.d.ts @@ -9,6 +9,8 @@ import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import * as UserManagementMailer from '@/UserManagement/email/UserManagementMailer'; +import type { Risk } from '@/audit/types'; + export type ExecutionStatus = 'error' | 'running' | 'success' | 'waiting' | null; export type AuthlessRequest< @@ -162,3 +164,15 @@ export interface IJsonSchema { allOf?: IDependency[]; required: string[]; } + +// ---------------------------------- +// /audit +// ---------------------------------- + +export declare namespace AuditRequest { + type Generate = AuthenticatedRequest< + {}, + {}, + { additionalOptions?: { categories?: Risk.Category[]; daysAbandonedWorkflow?: number } } + >; +} diff --git a/packages/cli/src/PublicApi/v1/handlers/audit/audit.handler.ts b/packages/cli/src/PublicApi/v1/handlers/audit/audit.handler.ts new file mode 100644 index 0000000000..89e4479944 --- /dev/null +++ b/packages/cli/src/PublicApi/v1/handlers/audit/audit.handler.ts @@ -0,0 +1,22 @@ +import { authorize } from '@/PublicApi/v1/shared/middlewares/global.middleware'; +import { audit } from '@/audit'; +import type { Response } from 'express'; +import type { AuditRequest } from '@/PublicApi/types'; + +export = { + generateAudit: [ + authorize(['owner']), + async (req: AuditRequest.Generate, res: Response): Promise => { + try { + const result = await audit( + req.body?.additionalOptions?.categories, + req.body?.additionalOptions?.daysAbandonedWorkflow, + ); + + return res.json(result); + } catch (error) { + return res.status(500).json(error); + } + }, + ], +}; diff --git a/packages/cli/src/PublicApi/v1/handlers/audit/spec/paths/audit.yml b/packages/cli/src/PublicApi/v1/handlers/audit/spec/paths/audit.yml new file mode 100644 index 0000000000..2721b3d790 --- /dev/null +++ b/packages/cli/src/PublicApi/v1/handlers/audit/spec/paths/audit.yml @@ -0,0 +1,36 @@ +post: + x-eov-operation-id: generateAudit + x-eov-operation-handler: v1/handlers/audit/audit.handler + tags: + - Audit + summary: Generate an audit + description: Generate a security audit for your n8n instance. + requestBody: + required: false + content: + application/json: + schema: + type: object + properties: + additionalOptions: + type: object + properties: + daysAbandonedWorkflow: + type: integer + description: Days for a workflow to be considered abandoned if not executed + categories: + type: array + items: + type: string + enum: ['credentials', 'database', 'nodes', 'filesystem', 'instance'] + responses: + '200': + description: Operation successful. + content: + application/json: + schema: + $ref: '../schemas/audit.yml' + '401': + $ref: '../../../../shared/spec/responses/unauthorized.yml' + '500': + description: Internal server error. diff --git a/packages/cli/src/PublicApi/v1/handlers/audit/spec/schemas/audit.yml b/packages/cli/src/PublicApi/v1/handlers/audit/spec/schemas/audit.yml new file mode 100644 index 0000000000..5ee8fe8fcd --- /dev/null +++ b/packages/cli/src/PublicApi/v1/handlers/audit/spec/schemas/audit.yml @@ -0,0 +1,105 @@ +type: object +properties: + Credentials Risk Report: + type: object + example: + risk: credentials + sections: + [ + { + title: Credentials not used in any workflow, + description: These credentials are not used in any workflow. Keeping unused credentials in your instance is an unneeded security risk., + recommendation: Consider deleting these credentials if you no longer need them., + location: [{ kind: credential, id: '1', name: My Test Account }], + }, + ] + Database Risk Report: + type: object + example: + risk: database + sections: + [ + { + title: Expressions in "Execute Query" fields in SQL nodes, + description: This SQL node has an expression in the "Query" field of an "Execute Query" operation. Building a SQL query with an expression may lead to a SQL injection attack., + recommendation: Consider using the "Query Parameters" field to pass parameters to the query, + or validating the input of the expression in the "Query" field., + location: + [ + { + kind: node, + workflowId: '1', + workflowName: 'My Workflow', + nodeId: 51eb5852-ce0b-4806-b4ff-e41322a4041a, + nodeName: 'MySQL', + nodeType: n8n-nodes-base.mySql, + }, + ], + }, + ] + Filesystem Risk Report: + type: object + example: + risk: filesystem + sections: + [ + { + title: Nodes that interact with the filesystem, + description: This node reads from and writes to any accessible file in the host filesystem. Sensitive file content may be manipulated through a node operation., + recommendation: Consider protecting any sensitive files in the host filesystem, + or refactoring the workflow so that it does not require host filesystem interaction., + location: + [ + { + kind: node, + workflowId: '1', + workflowName: 'My Workflow', + nodeId: 51eb5852-ce0b-4806-b4ff-e41322a4041a, + nodeName: 'Ready Binary file', + nodeType: n8n-nodes-base.readBinaryFile, + }, + ], + }, + ] + Nodes Risk Report: + type: object + example: + risk: nodes + sections: + [ + { + title: Community nodes, + description: This node is sourced from the community. Community nodes are not vetted by the n8n team and have full access to the host system., + recommendation: Consider reviewing the source code in any community nodes installed in this n8n instance, + and uninstalling any community nodes no longer used., + location: + [ + { + kind: community, + nodeType: n8n-nodes-test.test, + packageUrl: https://www.npmjs.com/package/n8n-nodes-test, + }, + ], + }, + ] + Instance Risk Report: + type: object + example: + risk: execution + sections: + [ + { + title: Unprotected webhooks in instance, + description: These webhook nodes have the "Authentication" field set to "None" and are not directly connected to a node to validate the payload. Every unprotected webhook allows your workflow to be called by any third party who knows the webhook URL., + recommendation: Consider setting the "Authentication" field to an option other than "None", + or validating the payload with one of the following nodes., + location: + [ + { + kind: community, + nodeType: n8n-nodes-test.test, + packageUrl: https://www.npmjs.com/package/n8n-nodes-test, + }, + ], + }, + ] diff --git a/packages/cli/src/PublicApi/v1/openapi.yml b/packages/cli/src/PublicApi/v1/openapi.yml index f0b12c5c55..216ae07521 100644 --- a/packages/cli/src/PublicApi/v1/openapi.yml +++ b/packages/cli/src/PublicApi/v1/openapi.yml @@ -9,13 +9,15 @@ info: license: name: Sustainable Use License url: https://github.com/n8n-io/n8n/blob/master/packages/cli/LICENSE.md - version: 1.0.0 + version: 1.1.0 externalDocs: description: n8n API documentation url: https://docs.n8n.io/api/ servers: - url: /api/v1 tags: + - name: Audit + description: Operations about security audit - name: Execution description: Operations about executions - name: Workflow @@ -24,6 +26,8 @@ tags: description: Operations about credentials paths: + /audit: + $ref: './handlers/audit/spec/paths/audit.yml' /credentials: $ref: './handlers/credentials/spec/paths/credentials.yml' /credentials/{id}: diff --git a/packages/cli/src/PublicApi/v1/shared/spec/schemas/_index.yml b/packages/cli/src/PublicApi/v1/shared/spec/schemas/_index.yml index a8b150a5ff..d3c3d128dc 100644 --- a/packages/cli/src/PublicApi/v1/shared/spec/schemas/_index.yml +++ b/packages/cli/src/PublicApi/v1/shared/spec/schemas/_index.yml @@ -18,3 +18,5 @@ Credential: $ref: './../../../handlers/credentials/spec/schemas/credential.yml' CredentialType: $ref: './../../../handlers/credentials/spec/schemas/credentialType.yml' +Audit: + $ref: './../../../handlers/audit/spec/schemas/audit.yml' diff --git a/packages/cli/src/audit/constants.ts b/packages/cli/src/audit/constants.ts new file mode 100644 index 0000000000..dae6fd44c5 --- /dev/null +++ b/packages/cli/src/audit/constants.ts @@ -0,0 +1,121 @@ +import type { Risk } from '@/audit/types'; + +/** + * Risk categories + */ + +export const RISK_CATEGORIES: Risk.Category[] = [ + 'credentials', + 'database', + 'nodes', + 'instance', + 'filesystem', +]; + +/** + * Node types + */ + +export const SQL_NODE_TYPES_WITH_QUERY_PARAMS = new Set([ + 'n8n-nodes-base.postgres', + 'n8n-nodes-base.crateDb', + 'n8n-nodes-base.questDb', + 'n8n-nodes-base.timescaleDb', +]); + +export const SQL_NODE_TYPES = new Set([ + ...SQL_NODE_TYPES_WITH_QUERY_PARAMS, + 'n8n-nodes-base.mySql', + 'n8n-nodes-base.microsoftSql', + 'n8n-nodes-base.snowflake', +]); + +export const WEBHOOK_NODE_TYPE = 'n8n-nodes-base.webhook'; + +export const WEBHOOK_VALIDATOR_NODE_TYPES = new Set([ + 'n8n-nodes-base.if', + 'n8n-nodes-base.switch', + 'n8n-nodes-base.code', + 'n8n-nodes-base.function', + 'n8n-nodes-base.functionItem', +]); + +export const FILESYSTEM_INTERACTION_NODE_TYPES = new Set([ + 'n8n-nodes-base.readPdf', + 'n8n-nodes-base.readBinaryFile', + 'n8n-nodes-base.readBinaryFiles', + 'n8n-nodes-base.spreadsheetFile', + 'n8n-nodes-base.writeBinaryFile', +]); + +export const OFFICIAL_RISKY_NODE_TYPES = new Set([ + 'n8n-nodes-base.executeCommand', + 'n8n-nodes-base.code', + 'n8n-nodes-base.function', + 'n8n-nodes-base.functionItem', + 'n8n-nodes-base.httpRequest', + 'n8n-nodes-base.ssh', + 'n8n-nodes-base.ftp', +]); + +/** + * Risk reports + */ + +export const DATABASE_REPORT = { + RISK: 'database', + SECTIONS: { + EXPRESSIONS_IN_QUERIES: 'Expressions in "Execute Query" fields in SQL nodes', + EXPRESSIONS_IN_QUERY_PARAMS: 'Expressions in "Query Parameters" fields in SQL nodes', + UNUSED_QUERY_PARAMS: 'Unused "Query Parameters" fields in SQL nodes', + }, +} as const; + +export const CREDENTIALS_REPORT = { + RISK: 'credentials', + SECTIONS: { + CREDS_NOT_IN_ANY_USE: 'Credentials not used in any workflow', + CREDS_NOT_IN_ACTIVE_USE: 'Credentials not used in any active workflow', + CREDS_NOT_RECENTLY_EXECUTED: 'Credentials not used in recently executed workflows', + }, +} as const; + +export const FILESYSTEM_REPORT = { + RISK: 'filesystem', + SECTIONS: { + FILESYSTEM_INTERACTION_NODES: 'Nodes that interact with the filesystem', + }, +} as const; + +export const NODES_REPORT = { + RISK: 'nodes', + SECTIONS: { + OFFICIAL_RISKY_NODES: 'Official risky nodes', + COMMUNITY_NODES: 'Community nodes', + CUSTOM_NODES: 'Custom nodes', + }, +} as const; + +export const INSTANCE_REPORT = { + RISK: 'instance', + SECTIONS: { + UNPROTECTED_WEBHOOKS: 'Unprotected webhooks in instance', + OUTDATED_INSTANCE: 'Outdated instance', + SECURITY_SETTINGS: 'Security settings', + }, +} as const; + +/** + * URLs + */ + +export const ENV_VARS_DOCS_URL = 'https://docs.n8n.io/reference/environment-variables.html'; + +export const DB_QUERY_PARAMS_DOCS_URL = + 'https://docs.n8n.io/integrations/builtin/app-nodes/n8n-nodes-base.postgres#use-query-parameters'; + +export const COMMUNITY_NODES_RISKS_URL = 'https://docs.n8n.io/integrations/community-nodes/risks'; + +export const SELF_HOSTED_AUTH_DOCS_URL = 'https://docs.n8n.io/hosting/authentication'; + +export const NPM_PACKAGE_URL = 'https://www.npmjs.com/package'; diff --git a/packages/cli/src/audit/index.ts b/packages/cli/src/audit/index.ts new file mode 100644 index 0000000000..a8c3e79e18 --- /dev/null +++ b/packages/cli/src/audit/index.ts @@ -0,0 +1,68 @@ +import * as Db from '@/Db'; +import { separate } from '@/utils'; +import config from '@/config'; +import { RISK_CATEGORIES } from '@/audit/constants'; +import { toReportTitle } from '@/audit/utils'; +import { reportCredentialsRisk } from '@/audit/risks/credentials.risk'; +import { reportDatabaseRisk } from '@/audit/risks/database.risk'; +import { reportNodesRisk } from '@/audit/risks/nodes.risk'; +import { reportFilesystemRisk } from '@/audit/risks/filesystem.risk'; +import { reportInstanceRisk } from '@/audit/risks/instance.risk'; +import type { Risk } from '@/audit/types'; + +export const SYNC_MAP: Record = { + database: reportDatabaseRisk, + filesystem: reportFilesystemRisk, +}; + +export const ASYNC_MAP: Record = { + credentials: reportCredentialsRisk, + nodes: reportNodesRisk, + instance: reportInstanceRisk, +}; + +export const isAsync = (c: Risk.Category) => Object.keys(ASYNC_MAP).includes(c); + +export async function audit( + categories: Risk.Category[] = RISK_CATEGORIES, + daysAbandonedWorkflow?: number, +) { + if (categories.length === 0) categories = RISK_CATEGORIES; + + const daysFromEnv = config.getEnv('security.audit.daysAbandonedWorkflow'); + + if (daysAbandonedWorkflow) { + config.set('security.audit.daysAbandonedWorkflow', daysAbandonedWorkflow); + } + + const workflows = await Db.collections.Workflow.find({ + select: ['id', 'name', 'active', 'nodes', 'connections'], + }); + + const [asyncCategories, syncCategories] = separate(categories, isAsync); + + const reports: Risk.Report[] = []; + + if (asyncCategories.length > 0) { + const promises = asyncCategories.map(async (c) => ASYNC_MAP[c](workflows)); + const asyncReports = await Promise.all(promises); + asyncReports.forEach((r) => r !== null && reports.push(r)); + } + + if (syncCategories.length > 0) { + const syncReports = syncCategories.map((c) => SYNC_MAP[c](workflows)); + syncReports.forEach((r) => r !== null && reports.push(r)); + } + + if (daysAbandonedWorkflow) { + config.set('security.audit.daysAbandonedWorkflow', daysFromEnv); // restore env + } + + if (reports.length === 0) return []; // trigger empty state + + return reports.reduce((acc, cur) => { + acc[toReportTitle(cur.risk)] = cur; + + return acc; + }, {}); +} diff --git a/packages/cli/src/audit/risks/credentials.risk.ts b/packages/cli/src/audit/risks/credentials.risk.ts new file mode 100644 index 0000000000..d3c03162cf --- /dev/null +++ b/packages/cli/src/audit/risks/credentials.risk.ts @@ -0,0 +1,133 @@ +import { MoreThanOrEqual } from 'typeorm'; +import { DateUtils } from 'typeorm/util/DateUtils'; +import * as Db from '@/Db'; +import config from '@/config'; +import { CREDENTIALS_REPORT } from '@/audit/constants'; +import type { WorkflowEntity } from '@/databases/entities/WorkflowEntity'; +import type { Risk } from '@/audit/types'; + +async function getAllCredsInUse(workflows: WorkflowEntity[]) { + const credsInAnyUse = new Set(); + const credsInActiveUse = new Set(); + + workflows.forEach((workflow) => { + workflow.nodes.forEach((node) => { + if (!node.credentials) return; + + Object.values(node.credentials).forEach((cred) => { + if (!cred?.id) return; + + credsInAnyUse.add(cred.id); + + if (workflow.active) credsInActiveUse.add(cred.id); + }); + }); + }); + + return { + credsInAnyUse, + credsInActiveUse, + }; +} + +async function getAllExistingCreds() { + const credentials = await Db.collections.Credentials.find({ select: ['id', 'name'] }); + + return credentials.map(({ id, name }) => ({ kind: 'credential' as const, id, name })); +} + +async function getExecutionsInPastDays(days: number) { + const date = new Date(); + + date.setDate(date.getDate() - days); + + const utcDate = DateUtils.mixedDateToUtcDatetimeString(date) as string; + + return Db.collections.Execution.find({ + select: ['workflowData'], + where: { + startedAt: MoreThanOrEqual(utcDate), + }, + }); +} + +/** + * Return IDs of credentials in workflows executed in the past n days. + */ +async function getCredsInRecentlyExecutedWorkflows(days: number) { + const recentExecutions = await getExecutionsInPastDays(days); + + return recentExecutions.reduce>((acc, execution) => { + execution.workflowData?.nodes.forEach((node) => { + if (node.credentials) { + Object.values(node.credentials).forEach((c) => { + if (c.id) acc.add(c.id); + }); + } + }); + + return acc; + }, new Set()); +} + +export async function reportCredentialsRisk(workflows: WorkflowEntity[]) { + const days = config.getEnv('security.audit.daysAbandonedWorkflow'); + + const allExistingCreds = await getAllExistingCreds(); + const { credsInAnyUse, credsInActiveUse } = await getAllCredsInUse(workflows); + const recentlyExecutedCreds = await getCredsInRecentlyExecutedWorkflows(days); + + const credsNotInAnyUse = allExistingCreds.filter((c) => !credsInAnyUse.has(c.id)); + const credsNotInActiveUse = allExistingCreds.filter((c) => !credsInActiveUse.has(c.id)); + const credsNotRecentlyExecuted = allExistingCreds.filter((c) => !recentlyExecutedCreds.has(c.id)); + + const issues = [credsNotInAnyUse, credsNotInActiveUse, credsNotRecentlyExecuted]; + + if (issues.every((i) => i.length === 0)) return null; + + const report: Risk.StandardReport = { + risk: CREDENTIALS_REPORT.RISK, + sections: [], + }; + + const hint = 'Keeping unused credentials in your instance is an unneeded security risk.'; + const recommendation = 'Consider deleting these credentials if you no longer need them.'; + + const sentenceStart = ({ length }: { length: number }) => + length > 1 ? 'These credentials are' : 'This credential is'; + + if (credsNotInAnyUse.length > 0) { + report.sections.push({ + title: CREDENTIALS_REPORT.SECTIONS.CREDS_NOT_IN_ANY_USE, + description: [sentenceStart(credsNotInAnyUse), 'not used in any workflow.', hint].join(' '), + recommendation, + location: credsNotInAnyUse, + }); + } + + if (credsNotInActiveUse.length > 0) { + report.sections.push({ + title: CREDENTIALS_REPORT.SECTIONS.CREDS_NOT_IN_ACTIVE_USE, + description: [sentenceStart(credsNotInActiveUse), 'not used in active workflows.', hint].join( + ' ', + ), + recommendation, + location: credsNotInActiveUse, + }); + } + + if (credsNotRecentlyExecuted.length > 0) { + report.sections.push({ + title: CREDENTIALS_REPORT.SECTIONS.CREDS_NOT_RECENTLY_EXECUTED, + description: [ + sentenceStart(credsNotRecentlyExecuted), + `not used in recently executed workflows, i.e. workflows executed in the past ${days} days.`, + hint, + ].join(' '), + recommendation, + location: credsNotRecentlyExecuted, + }); + } + + return report; +} diff --git a/packages/cli/src/audit/risks/database.risk.ts b/packages/cli/src/audit/risks/database.risk.ts new file mode 100644 index 0000000000..06e36f47f8 --- /dev/null +++ b/packages/cli/src/audit/risks/database.risk.ts @@ -0,0 +1,106 @@ +import { toFlaggedNode } from '@/audit/utils'; +import { + SQL_NODE_TYPES, + DATABASE_REPORT, + DB_QUERY_PARAMS_DOCS_URL, + SQL_NODE_TYPES_WITH_QUERY_PARAMS, +} from '@/audit/constants'; +import type { WorkflowEntity as Workflow } from '@/databases/entities/WorkflowEntity'; +import type { Risk } from '@/audit/types'; + +function getIssues(workflows: Workflow[]) { + return workflows.reduce<{ [sectionTitle: string]: Risk.NodeLocation[] }>( + (acc, workflow) => { + workflow.nodes.forEach((node) => { + if (!SQL_NODE_TYPES.has(node.type)) return; + if (node.parameters === undefined) return; + if (node.parameters.operation !== 'executeQuery') return; + + if (typeof node.parameters.query === 'string' && node.parameters.query.startsWith('=')) { + acc.expressionsInQueries.push(toFlaggedNode({ node, workflow })); + } + + if (!SQL_NODE_TYPES_WITH_QUERY_PARAMS.has(node.type)) return; + + if (!node.parameters.additionalFields) { + acc.unusedQueryParams.push(toFlaggedNode({ node, workflow })); + } + + if (typeof node.parameters.additionalFields !== 'object') return; + if (node.parameters.additionalFields === null) return; + + if (!('queryParams' in node.parameters.additionalFields)) { + acc.unusedQueryParams.push(toFlaggedNode({ node, workflow })); + } + + if ( + 'queryParams' in node.parameters.additionalFields && + typeof node.parameters.additionalFields.queryParams === 'string' && + node.parameters.additionalFields.queryParams.startsWith('=') + ) { + acc.expressionsInQueryParams.push(toFlaggedNode({ node, workflow })); + } + }); + + return acc; + }, + { expressionsInQueries: [], expressionsInQueryParams: [], unusedQueryParams: [] }, + ); +} + +export function reportDatabaseRisk(workflows: Workflow[]) { + const { expressionsInQueries, expressionsInQueryParams, unusedQueryParams } = + getIssues(workflows); + + const issues = [expressionsInQueries, expressionsInQueryParams, unusedQueryParams]; + + if (issues.every((i) => i.length === 0)) return null; + + const report: Risk.StandardReport = { + risk: DATABASE_REPORT.RISK, + sections: [], + }; + + const sentenceStart = ({ length }: { length: number }) => + length > 1 ? 'These SQL nodes have' : 'This SQL node has'; + + if (expressionsInQueries.length > 0) { + report.sections.push({ + title: DATABASE_REPORT.SECTIONS.EXPRESSIONS_IN_QUERIES, + description: [ + sentenceStart(expressionsInQueries), + 'an expression in the "Query" field of an "Execute Query" operation. Building a SQL query with an expression may lead to a SQL injection attack.', + ].join(' '), + recommendation: + 'Consider using the "Query Parameters" field to pass parameters to the query, or validating the input of the expression in the "Query" field.', + location: expressionsInQueries, + }); + } + + if (expressionsInQueryParams.length > 0) { + report.sections.push({ + title: DATABASE_REPORT.SECTIONS.EXPRESSIONS_IN_QUERY_PARAMS, + description: [ + sentenceStart(expressionsInQueryParams), + 'an expression in the "Query Parameters" field of an "Execute Query" operation. Building a SQL query with an expression may lead to a SQL injection attack.', + ].join(' '), + recommendation: + 'Consider validating the input of the expression in the "Query Parameters" field.', + location: expressionsInQueryParams, + }); + } + + if (unusedQueryParams.length > 0) { + report.sections.push({ + title: DATABASE_REPORT.SECTIONS.UNUSED_QUERY_PARAMS, + description: [ + sentenceStart(unusedQueryParams), + 'no "Query Parameters" field in the "Execute Query" operation. Building a SQL query with unsanitized data may lead to a SQL injection attack.', + ].join(' '), + recommendation: `Consider using the "Query Parameters" field to sanitize parameters passed to the query. See: ${DB_QUERY_PARAMS_DOCS_URL}`, + location: unusedQueryParams, + }); + } + + return report; +} diff --git a/packages/cli/src/audit/risks/filesystem.risk.ts b/packages/cli/src/audit/risks/filesystem.risk.ts new file mode 100644 index 0000000000..177358abb4 --- /dev/null +++ b/packages/cli/src/audit/risks/filesystem.risk.ts @@ -0,0 +1,35 @@ +import { getNodeTypes } from '@/audit/utils'; +import { FILESYSTEM_INTERACTION_NODE_TYPES, FILESYSTEM_REPORT } from '@/audit/constants'; +import type { WorkflowEntity } from '@/databases/entities/WorkflowEntity'; +import type { Risk } from '@/audit/types'; + +export function reportFilesystemRisk(workflows: WorkflowEntity[]) { + const fsInteractionNodeTypes = getNodeTypes(workflows, (node) => + FILESYSTEM_INTERACTION_NODE_TYPES.has(node.type), + ); + + if (fsInteractionNodeTypes.length === 0) return null; + + const report: Risk.StandardReport = { + risk: FILESYSTEM_REPORT.RISK, + sections: [], + }; + + const sentenceStart = ({ length }: { length: number }) => + length > 1 ? 'These nodes read from and write to' : 'This node reads from and writes to'; + + if (fsInteractionNodeTypes.length > 0) { + report.sections.push({ + title: FILESYSTEM_REPORT.SECTIONS.FILESYSTEM_INTERACTION_NODES, + description: [ + sentenceStart(fsInteractionNodeTypes), + 'any accessible file in the host filesystem. Sensitive file content may be manipulated through a node operation.', + ].join(' '), + recommendation: + 'Consider protecting any sensitive files in the host filesystem, or refactoring the workflow so that it does not require host filesystem interaction.', + location: fsInteractionNodeTypes, + }); + } + + return report; +} diff --git a/packages/cli/src/audit/risks/instance.risk.ts b/packages/cli/src/audit/risks/instance.risk.ts new file mode 100644 index 0000000000..f0edbe595e --- /dev/null +++ b/packages/cli/src/audit/risks/instance.risk.ts @@ -0,0 +1,220 @@ +import axios from 'axios'; +import { UserSettings } from 'n8n-core'; +import config from '@/config'; +import { toFlaggedNode } from '@/audit/utils'; +import { separate } from '@/utils'; +import { + SELF_HOSTED_AUTH_DOCS_URL, + ENV_VARS_DOCS_URL, + INSTANCE_REPORT, + WEBHOOK_NODE_TYPE, + WEBHOOK_VALIDATOR_NODE_TYPES, +} from '@/audit/constants'; +import { getN8nPackageJson, inDevelopment } from '@/constants'; +import type { WorkflowEntity } from '@/databases/entities/WorkflowEntity'; +import type { Risk, n8n } from '@/audit/types'; + +function getSecuritySettings() { + if (config.getEnv('deployment.type') === 'cloud') return null; + + const userManagementEnabled = !config.getEnv('userManagement.disabled'); + const basicAuthActive = config.getEnv('security.basicAuth.active'); + const jwtAuthActive = config.getEnv('security.jwtAuth.active'); + + const isInstancePubliclyAccessible = !userManagementEnabled && !basicAuthActive && !jwtAuthActive; + + const settings: Record = {}; + + if (isInstancePubliclyAccessible) { + settings.publiclyAccessibleInstance = + 'Important! Your n8n instance is publicly accessible. Any third party who knows your instance URL can access your data.'.toUpperCase(); + } + + settings.features = { + communityPackagesEnabled: config.getEnv('nodes.communityPackages.enabled'), + versionNotificationsEnabled: config.getEnv('versionNotifications.enabled'), + templatesEnabled: config.getEnv('templates.enabled'), + publicApiEnabled: !config.getEnv('publicApi.disabled'), + userManagementEnabled, + }; + + settings.auth = { + authExcludeEndpoints: config.getEnv('security.excludeEndpoints') || 'none', + basicAuthActive, + jwtAuthActive, + }; + + settings.nodes = { + nodesExclude: config.getEnv('nodes.exclude') ?? 'none', + nodesInclude: config.getEnv('nodes.include') ?? 'none', + }; + + settings.telemetry = { + diagnosticsEnabled: config.getEnv('diagnostics.enabled'), + }; + + return settings; +} + +/** + * Whether a webhook node has a direct child assumed to validate its payload. + */ +function hasValidatorChild({ + node, + workflow, +}: { + node: WorkflowEntity['nodes'][number]; + workflow: WorkflowEntity; +}) { + const childNodeNames = workflow.connections[node.name]?.main[0].map((i) => i.node); + + if (!childNodeNames) return false; + + return childNodeNames.some((name) => + workflow.nodes.find((n) => n.name === name && WEBHOOK_VALIDATOR_NODE_TYPES.has(n.type)), + ); +} + +function getUnprotectedWebhookNodes(workflows: WorkflowEntity[]) { + return workflows.reduce((acc, workflow) => { + if (!workflow.active) return acc; + + workflow.nodes.forEach((node) => { + if ( + node.type === WEBHOOK_NODE_TYPE && + node.parameters.authentication === undefined && + !hasValidatorChild({ node, workflow }) + ) { + acc.push(toFlaggedNode({ node, workflow })); + } + }); + + return acc; + }, []); +} + +async function getNextVersions(currentVersionName: string) { + const BASE_URL = config.getEnv('versionNotifications.endpoint'); + const instanceId = await UserSettings.getInstanceId(); + + const response = await axios.get(BASE_URL + currentVersionName, { + // eslint-disable-next-line @typescript-eslint/naming-convention + headers: { 'n8n-instance-id': instanceId }, + }); + + return response.data; +} + +function removeIconData(versions: n8n.Version[]) { + return versions.map((version) => { + if (version.nodes.length === 0) return version; + + version.nodes.forEach((node) => delete node.iconData); + + return version; + }); +} + +function classify(versions: n8n.Version[], currentVersionName: string) { + const [pass, fail] = separate(versions, (v) => v.name === currentVersionName); + + return { currentVersion: pass[0], nextVersions: fail }; +} + +export async function getOutdatedState() { + let versions = []; + + const localVersion = getN8nPackageJson().version; + + try { + versions = await getNextVersions(localVersion).then(removeIconData); + } catch (error) { + if (inDevelopment) { + console.error('Failed to fetch n8n versions. Skipping outdated instance report...'); + } + return null; + } + + const { currentVersion, nextVersions } = classify(versions, localVersion); + + const nextVersionsNumber = nextVersions.length; + + if (nextVersionsNumber === 0) return null; + + const description = [ + `This n8n instance is outdated. Currently at version ${ + currentVersion.name + }, missing ${nextVersionsNumber} ${nextVersionsNumber > 1 ? 'updates' : 'update'}.`, + ]; + + const upcomingSecurityUpdates = nextVersions.some((v) => v.hasSecurityIssue || v.hasSecurityFix); + + if (upcomingSecurityUpdates) description.push('Newer versions contain security updates.'); + + return { + description: description.join(' '), + nextVersions, + }; +} + +export async function reportInstanceRisk(workflows: WorkflowEntity[]) { + const unprotectedWebhooks = getUnprotectedWebhookNodes(workflows); + const outdatedState = await getOutdatedState(); + const securitySettings = getSecuritySettings(); + + if (unprotectedWebhooks.length === 0 && outdatedState === null && securitySettings === null) { + return null; + } + + const report: Risk.InstanceReport = { + risk: INSTANCE_REPORT.RISK, + sections: [], + }; + + if (unprotectedWebhooks.length > 0) { + const sentenceStart = ({ length }: { length: number }) => + length > 1 ? 'These webhook nodes have' : 'This webhook node has'; + + const recommendedValidators = [...WEBHOOK_VALIDATOR_NODE_TYPES] + .filter((nodeType) => !nodeType.endsWith('function') || !nodeType.endsWith('functionItem')) + .join(','); + + report.sections.push({ + title: INSTANCE_REPORT.SECTIONS.UNPROTECTED_WEBHOOKS, + description: [ + sentenceStart(unprotectedWebhooks), + `the "Authentication" field set to "None" and ${ + unprotectedWebhooks.length > 1 ? 'are' : 'is' + } not directly connected to a node to validate the payload. Every unprotected webhook allows your workflow to be called by any third party who knows the webhook URL.`, + ].join(' '), + recommendation: `Consider setting the "Authentication" field to an option other than "None", or validating the payload with one of the following nodes: ${recommendedValidators}.`, + location: unprotectedWebhooks, + }); + } + + if (outdatedState !== null) { + report.sections.push({ + title: INSTANCE_REPORT.SECTIONS.OUTDATED_INSTANCE, + description: outdatedState.description, + recommendation: + 'Consider updating this n8n instance to the latest version to prevent security vulnerabilities.', + nextVersions: outdatedState.nextVersions, + }); + } + + if (securitySettings !== null) { + report.sections.push({ + title: INSTANCE_REPORT.SECTIONS.SECURITY_SETTINGS, + description: 'This n8n instance has the following security settings.', + recommendation: securitySettings.publiclyAccessibleInstance + ? [ + 'Important! Your n8n instance is publicly accessible. Set up user management or basic/JWT auth to protect access to your n8n instance.'.toUpperCase(), + `See: ${SELF_HOSTED_AUTH_DOCS_URL}`, + ].join(' ') + : `Consider adjusting the security settings for your n8n instance based on your needs. See: ${ENV_VARS_DOCS_URL}`, + settings: securitySettings, + }); + } + + return report; +} diff --git a/packages/cli/src/audit/risks/nodes.risk.ts b/packages/cli/src/audit/risks/nodes.risk.ts new file mode 100644 index 0000000000..94f041c1f3 --- /dev/null +++ b/packages/cli/src/audit/risks/nodes.risk.ts @@ -0,0 +1,111 @@ +import * as path from 'path'; +import glob from 'fast-glob'; +import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials'; +import { getNodeTypes } from '@/audit/utils'; +import { getAllInstalledPackages } from '@/CommunityNodes/packageModel'; +import { + OFFICIAL_RISKY_NODE_TYPES, + ENV_VARS_DOCS_URL, + NODES_REPORT, + COMMUNITY_NODES_RISKS_URL, + NPM_PACKAGE_URL, +} from '@/audit/constants'; +import type { WorkflowEntity } from '@/databases/entities/WorkflowEntity'; +import type { Risk } from '@/audit/types'; + +async function getCommunityNodeDetails() { + const installedPackages = await getAllInstalledPackages(); + + return installedPackages.reduce((acc, pkg) => { + pkg.installedNodes.forEach((node) => + acc.push({ + kind: 'community', + nodeType: node.type, + packageUrl: [NPM_PACKAGE_URL, pkg.packageName].join('/'), + }), + ); + + return acc; + }, []); +} + +async function getCustomNodeDetails() { + const customNodeTypes: Risk.CustomNodeDetails[] = []; + + for (const customDir of LoadNodesAndCredentials().getCustomDirectories()) { + const customNodeFiles = await glob('**/*.node.js', { cwd: customDir, absolute: true }); + + for (const nodeFile of customNodeFiles) { + const [fileName] = path.parse(nodeFile).name.split('.'); + customNodeTypes.push({ + kind: 'custom', + nodeType: ['CUSTOM', fileName].join('.'), + filePath: nodeFile, + }); + } + } + + return customNodeTypes; +} + +export async function reportNodesRisk(workflows: WorkflowEntity[]) { + const officialRiskyNodes = getNodeTypes(workflows, (node) => + OFFICIAL_RISKY_NODE_TYPES.has(node.type), + ); + + const [communityNodes, customNodes] = await Promise.all([ + getCommunityNodeDetails(), + getCustomNodeDetails(), + ]); + + const issues = [officialRiskyNodes, communityNodes, customNodes]; + + if (issues.every((i) => i.length === 0)) return null; + + const report: Risk.StandardReport = { + risk: NODES_REPORT.RISK, + sections: [], + }; + + const sentenceStart = (length: number) => (length > 1 ? 'These nodes are' : 'This node is'); + + if (officialRiskyNodes.length > 0) { + report.sections.push({ + title: NODES_REPORT.SECTIONS.OFFICIAL_RISKY_NODES, + description: [ + sentenceStart(officialRiskyNodes.length), + "part of n8n's official nodes and may be used to fetch and run any arbitrary code in the host system. This may lead to exploits such as remote code execution.", + ].join(' '), + recommendation: `Consider reviewing the parameters in these nodes, replacing them with app nodes where possible, and not loading unneeded node types with the NODES_EXCLUDE environment variable. See: ${ENV_VARS_DOCS_URL}`, + location: officialRiskyNodes, + }); + } + + if (communityNodes.length > 0) { + report.sections.push({ + title: NODES_REPORT.SECTIONS.COMMUNITY_NODES, + description: [ + sentenceStart(communityNodes.length), + `sourced from the n8n community. Community nodes are not vetted by the n8n team and have full access to the host system. See: ${COMMUNITY_NODES_RISKS_URL}`, + ].join(' '), + recommendation: + 'Consider reviewing the source code in any community nodes installed in this n8n instance, and uninstalling any community nodes no longer in use.', + location: communityNodes, + }); + } + + if (customNodes.length > 0) { + report.sections.push({ + title: NODES_REPORT.SECTIONS.CUSTOM_NODES, + description: [ + sentenceStart(communityNodes.length), + 'unpublished and located in the host system. Custom nodes are not vetted by the n8n team and have full access to the host system.', + ].join(' '), + recommendation: + 'Consider reviewing the source code in any custom node installed in this n8n instance, and removing any custom nodes no longer in use.', + location: customNodes, + }); + } + + return report; +} diff --git a/packages/cli/src/audit/types.ts b/packages/cli/src/audit/types.ts new file mode 100644 index 0000000000..73c1e5e044 --- /dev/null +++ b/packages/cli/src/audit/types.ts @@ -0,0 +1,86 @@ +import type { WorkflowEntity as Workflow } from '@/databases/entities/WorkflowEntity'; + +export namespace Risk { + export type Category = 'database' | 'credentials' | 'nodes' | 'instance' | 'filesystem'; + + type CredLocation = { + kind: 'credential'; + id: string; + name: string; + }; + + export type NodeLocation = { + kind: 'node'; + workflowId: string; + workflowName: string; + nodeId: string; + nodeName: string; + nodeType: string; + }; + + export type CommunityNodeDetails = { + kind: 'community'; + nodeType: string; + packageUrl: string; + }; + + export type CustomNodeDetails = { + kind: 'custom'; + nodeType: string; + filePath: string; + }; + + type SectionBase = { + title: string; + description: string; + recommendation: string; + }; + + export type Report = StandardReport | InstanceReport; + + export type StandardSection = SectionBase & { + location: NodeLocation[] | CredLocation[] | CommunityNodeDetails[] | CustomNodeDetails[]; + }; + + export type InstanceSection = SectionBase & { + location?: NodeLocation[]; + settings?: Record; + nextVersions?: n8n.Version[]; + }; + + export type StandardReport = { + risk: Exclude; + sections: StandardSection[]; + }; + + export type InstanceReport = { + risk: 'instance'; + sections: InstanceSection[]; + }; + + export type Audit = { + [reportTitle: string]: Report; + }; + + export type SyncReportFn = (workflows: Workflow[]) => StandardReport | null; + + export type AsyncReportFn = (workflows: Workflow[]) => Promise; +} + +export namespace n8n { + export type Version = { + name: string; + nodes: Array< + Workflow['nodes'][number] & { + iconData?: { type: string; fileBuffer: string }; // removed to declutter report + } + >; + createdAt: string; + description: string; + documentationUrl: string; + hasBreakingChange: boolean; + hasSecurityFix: boolean; + hasSecurityIssue: boolean; + securityIssueFixVersion: string; + }; +} diff --git a/packages/cli/src/audit/utils.ts b/packages/cli/src/audit/utils.ts new file mode 100644 index 0000000000..3a4f1a798d --- /dev/null +++ b/packages/cli/src/audit/utils.ts @@ -0,0 +1,26 @@ +import type { WorkflowEntity as Workflow } from '@/databases/entities/WorkflowEntity'; +import type { Risk } from '@/audit/types'; + +type Node = Workflow['nodes'][number]; + +export const toFlaggedNode = ({ node, workflow }: { node: Node; workflow: Workflow }) => ({ + kind: 'node' as const, + workflowId: workflow.id, + workflowName: workflow.name, + nodeId: node.id, + nodeName: node.name, + nodeType: node.type, +}); + +export const toReportTitle = (riskCategory: Risk.Category) => + riskCategory.charAt(0).toUpperCase() + riskCategory.slice(1) + ' Risk Report'; + +export function getNodeTypes(workflows: Workflow[], test: (element: Node) => boolean) { + return workflows.reduce((acc, workflow) => { + workflow.nodes.forEach((node) => { + if (test(node)) acc.push(toFlaggedNode({ node, workflow })); + }); + + return acc; + }, []); +} diff --git a/packages/cli/src/commands/audit.ts b/packages/cli/src/commands/audit.ts new file mode 100644 index 0000000000..7c3c8a5881 --- /dev/null +++ b/packages/cli/src/commands/audit.ts @@ -0,0 +1,106 @@ +import Command, { flags } from '@oclif/command'; +import { LoggerProxy } from 'n8n-workflow'; +import { UserSettings } from 'n8n-core'; +import { getLogger, Logger } from '@/Logger'; +import { audit } from '@/audit'; +import { RISK_CATEGORIES } from '@/audit/constants'; +import { CredentialTypes } from '@/CredentialTypes'; +import { NodeTypes } from '@/NodeTypes'; +import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials'; +import { InternalHooksManager } from '@/InternalHooksManager'; +import config from '@/config'; +import * as Db from '@/Db'; +import type { Risk } from '@/audit/types'; + +export class SecurityAudit extends Command { + static description = 'Generate a security audit report for this n8n instance'; + + static examples = [ + '$ n8n audit', + '$ n8n audit --categories=database,credentials', + '$ n8n audit --days-abandoned-workflow=10', + ]; + + static flags = { + help: flags.help({ char: 'h' }), + categories: flags.string({ + default: RISK_CATEGORIES.join(','), + description: 'Comma-separated list of categories to include in the audit', + }), + // eslint-disable-next-line @typescript-eslint/naming-convention + 'days-abandoned-workflow': flags.integer({ + default: config.getEnv('security.audit.daysAbandonedWorkflow'), + description: 'Days for a workflow to be considered abandoned if not executed', + }), + }; + + logger: Logger; + + async run() { + await this.init(); + + const { flags: auditFlags } = this.parse(SecurityAudit); + + const categories = + auditFlags.categories?.split(',').filter((c): c is Risk.Category => c !== '') ?? + RISK_CATEGORIES; + + const invalidCategories = categories.filter((c) => !RISK_CATEGORIES.includes(c)); + + if (invalidCategories.length > 0) { + const message = + invalidCategories.length > 1 + ? `Invalid categories received: ${invalidCategories.join(', ')}` + : `Invalid category received: ${invalidCategories[0]}`; + + const hint = `Valid categories are: ${RISK_CATEGORIES.join(', ')}`; + + throw new Error([message, hint].join('. ')); + } + + const result = await audit(categories, auditFlags['days-abandoned-workflow']); + + if (Array.isArray(result) && result.length === 0) { + this.logger.info('No security issues found'); + } else { + process.stdout.write(JSON.stringify(result, null, 2)); + } + + void InternalHooksManager.getInstance().onAuditGeneratedViaCli(); + } + + async init() { + await Db.init(); + + this.initLogger(); + + await this.initInternalHooksManager(); + } + + initLogger() { + this.logger = getLogger(); + LoggerProxy.init(this.logger); + } + + async initInternalHooksManager(): Promise { + const loadNodesAndCredentials = LoadNodesAndCredentials(); + await loadNodesAndCredentials.init(); + + const nodeTypes = NodeTypes(loadNodesAndCredentials); + CredentialTypes(loadNodesAndCredentials); + + const instanceId = await UserSettings.getInstanceId(); + await InternalHooksManager.init(instanceId, nodeTypes); + } + + async catch(error: Error) { + this.logger.error('Failed to generate security audit'); + this.logger.error(error.message); + + this.exit(1); + } + + async finally() { + this.exit(); + } +} diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 858bd5b67c..cd43c3a170 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -469,6 +469,14 @@ export const schema = { }, security: { + 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', + }, + }, excludeEndpoints: { doc: 'Additional endpoints to exclude auth checks. Multiple endpoints can be separated by colon (":")', format: String, diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index 9679e0ee8d..d8a0f994c6 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -22,9 +22,11 @@ export const NODES_BASE_DIR = join(CLI_DIR, '..', 'nodes-base'); export const GENERATED_STATIC_DIR = join(UserSettings.getUserHome(), '.cache/n8n/public'); export const EDITOR_UI_DIST_DIR = join(dirname(require.resolve('n8n-editor-ui')), 'dist'); -export const N8N_VERSION = jsonParse( - readFileSync(join(CLI_DIR, 'package.json'), 'utf8'), -).version; +export function getN8nPackageJson() { + return jsonParse(readFileSync(join(CLI_DIR, 'package.json'), 'utf8')); +} + +export const N8N_VERSION = getN8nPackageJson().version; export const NODE_PACKAGE_PREFIX = 'n8n-nodes-'; diff --git a/packages/cli/src/jest.d.ts b/packages/cli/src/jest.d.ts new file mode 100644 index 0000000000..dcb4e4e7bf --- /dev/null +++ b/packages/cli/src/jest.d.ts @@ -0,0 +1,5 @@ +namespace jest { + interface Matchers { + toBeEmptyArray(): T; + } +} diff --git a/packages/cli/src/utils.ts b/packages/cli/src/utils.ts index 487442dae2..e7059e1159 100644 --- a/packages/cli/src/utils.ts +++ b/packages/cli/src/utils.ts @@ -41,3 +41,12 @@ export const alphabetizeKeys = (obj: INode) => }), {}, ); + +export const separate = (array: T[], test: (element: T) => boolean) => { + const pass: T[] = []; + const fail: T[] = []; + + array.forEach((i) => (test(i) ? pass : fail).push(i)); + + return [pass, fail]; +}; diff --git a/packages/cli/test/extend-expect.ts b/packages/cli/test/extend-expect.ts new file mode 100644 index 0000000000..328daf2b0b --- /dev/null +++ b/packages/cli/test/extend-expect.ts @@ -0,0 +1,12 @@ +expect.extend({ + toBeEmptyArray(this: jest.MatcherContext, actual: unknown) { + const pass = Array.isArray(actual) && actual.length === 0; + + return { + pass, + message: pass + ? () => `Expected ${actual} to be an empty array` + : () => `Expected ${actual} not to be an empty array`, + }; + }, +}); diff --git a/packages/cli/test/integration/audit/credentials.risk.test.ts b/packages/cli/test/integration/audit/credentials.risk.test.ts new file mode 100644 index 0000000000..cfd8f9e416 --- /dev/null +++ b/packages/cli/test/integration/audit/credentials.risk.test.ts @@ -0,0 +1,225 @@ +import { v4 as uuid } from 'uuid'; +import * as Db from '@/Db'; +import config from '@/config'; +import { audit } from '@/audit'; +import { CREDENTIALS_REPORT } from '@/audit/constants'; +import { getRiskSection } from './utils'; +import * as testDb from '../shared/testDb'; + +let testDbName = ''; + +beforeAll(async () => { + const initResult = await testDb.init(); + testDbName = initResult.testDbName; +}); + +beforeEach(async () => { + await testDb.truncate(['Workflow', 'Credentials', 'Execution'], testDbName); +}); + +afterAll(async () => { + await testDb.terminate(testDbName); +}); + +test('should report credentials not in any use', async () => { + const credentialDetails = { + name: 'My Slack Credential', + data: 'U2FsdGVkX18WjITBG4IDqrGB1xE/uzVNjtwDAG3lP7E=', + type: 'slackApi', + nodesAccess: [{ nodeType: 'n8n-nodes-base.slack', date: '2022-12-21T11:23:00.561Z' }], + }; + + const workflowDetails = { + name: 'My Test Workflow', + active: false, + connections: {}, + nodeTypes: {}, + nodes: [ + { + id: uuid(), + name: 'My Node', + type: 'n8n-nodes-base.slack', + typeVersion: 1, + position: [0, 0] as [number, number], + }, + ], + }; + + await Promise.all([ + Db.collections.Credentials.save(credentialDetails), + Db.collections.Workflow.save(workflowDetails), + ]); + + const testAudit = await audit(['credentials']); + + const section = getRiskSection( + testAudit, + CREDENTIALS_REPORT.RISK, + CREDENTIALS_REPORT.SECTIONS.CREDS_NOT_IN_ANY_USE, + ); + + expect(section.location).toHaveLength(1); + expect(section.location[0]).toMatchObject({ + id: '1', + name: 'My Slack Credential', + }); +}); + +test('should report credentials not in active use', async () => { + const credentialDetails = { + name: 'My Slack Credential', + data: 'U2FsdGVkX18WjITBG4IDqrGB1xE/uzVNjtwDAG3lP7E=', + type: 'slackApi', + nodesAccess: [{ nodeType: 'n8n-nodes-base.slack', date: '2022-12-21T11:23:00.561Z' }], + }; + + const credential = await Db.collections.Credentials.save(credentialDetails); + + const workflowDetails = { + name: 'My Test Workflow', + active: false, + connections: {}, + nodeTypes: {}, + nodes: [ + { + id: uuid(), + name: 'My Node', + type: 'n8n-nodes-base.slack', + typeVersion: 1, + position: [0, 0] as [number, number], + }, + ], + }; + + await Db.collections.Workflow.save(workflowDetails); + + const testAudit = await audit(['credentials']); + + const section = getRiskSection( + testAudit, + CREDENTIALS_REPORT.RISK, + CREDENTIALS_REPORT.SECTIONS.CREDS_NOT_IN_ACTIVE_USE, + ); + + expect(section.location).toHaveLength(1); + expect(section.location[0]).toMatchObject({ + id: credential.id, + name: 'My Slack Credential', + }); +}); + +test('should report credential in not recently executed workflow', async () => { + const credentialDetails = { + name: 'My Slack Credential', + data: 'U2FsdGVkX18WjITBG4IDqrGB1xE/uzVNjtwDAG3lP7E=', + type: 'slackApi', + nodesAccess: [{ nodeType: 'n8n-nodes-base.slack', date: '2022-12-21T11:23:00.561Z' }], + }; + + const credential = await Db.collections.Credentials.save(credentialDetails); + + const workflowDetails = { + name: 'My Test Workflow', + active: false, + connections: {}, + nodeTypes: {}, + nodes: [ + { + id: uuid(), + name: 'My Node', + type: 'n8n-nodes-base.slack', + typeVersion: 1, + position: [0, 0] as [number, number], + credentials: { + slackApi: { + id: credential.id, + name: credential.name, + }, + }, + }, + ], + }; + + const workflow = await Db.collections.Workflow.save(workflowDetails); + + const date = new Date(); + date.setDate(date.getDate() - config.getEnv('security.audit.daysAbandonedWorkflow') - 1); + + await Db.collections.Execution.save({ + data: '[]', + finished: true, + mode: 'manual', + startedAt: date, + stoppedAt: date, + workflowData: workflow, + workflowId: workflow.id, + waitTill: null, + }); + + const testAudit = await audit(['credentials']); + + const section = getRiskSection( + testAudit, + CREDENTIALS_REPORT.RISK, + CREDENTIALS_REPORT.SECTIONS.CREDS_NOT_RECENTLY_EXECUTED, + ); + + expect(section.location).toHaveLength(1); + expect(section.location[0]).toMatchObject({ + id: credential.id, + name: credential.name, + }); +}); + +test('should not report credentials in recently executed workflow', async () => { + const credentialDetails = { + name: 'My Slack Credential', + data: 'U2FsdGVkX18WjITBG4IDqrGB1xE/uzVNjtwDAG3lP7E=', + type: 'slackApi', + nodesAccess: [{ nodeType: 'n8n-nodes-base.slack', date: '2022-12-21T11:23:00.561Z' }], + }; + + const credential = await Db.collections.Credentials.save(credentialDetails); + + const workflowDetails = { + name: 'My Test Workflow', + active: true, + connections: {}, + nodeTypes: {}, + nodes: [ + { + id: uuid(), + name: 'My Node', + type: 'n8n-nodes-base.slack', + typeVersion: 1, + position: [0, 0] as [number, number], + credentials: { + slackApi: { + id: credential.id, + name: credential.name, + }, + }, + }, + ], + }; + + const workflow = await Db.collections.Workflow.save(workflowDetails); + + const date = new Date(); + date.setDate(date.getDate() - config.getEnv('security.audit.daysAbandonedWorkflow') + 1); + + await Db.collections.Execution.save({ + data: '[]', + finished: true, + mode: 'manual', + startedAt: date, + stoppedAt: date, + workflowData: workflow, + workflowId: workflow.id, + waitTill: null, + }); + + const testAudit = await audit(['credentials']); + + expect(testAudit).toBeEmptyArray(); +}); diff --git a/packages/cli/test/integration/audit/database.risk.test.ts b/packages/cli/test/integration/audit/database.risk.test.ts new file mode 100644 index 0000000000..037cd8d6c9 --- /dev/null +++ b/packages/cli/test/integration/audit/database.risk.test.ts @@ -0,0 +1,187 @@ +import { v4 as uuid } from 'uuid'; +import * as Db from '@/Db'; +import { audit } from '@/audit'; +import { + DATABASE_REPORT, + SQL_NODE_TYPES, + SQL_NODE_TYPES_WITH_QUERY_PARAMS, +} from '@/audit/constants'; +import { getRiskSection, saveManualTriggerWorkflow } from './utils'; +import * as testDb from '../shared/testDb'; + +let testDbName = ''; + +beforeAll(async () => { + const initResult = await testDb.init(); + testDbName = initResult.testDbName; +}); + +beforeEach(async () => { + await testDb.truncate(['Workflow'], testDbName); +}); + +afterAll(async () => { + await testDb.terminate(testDbName); +}); + +test('should report expressions in queries', async () => { + const map = [...SQL_NODE_TYPES].reduce<{ [nodeType: string]: string }>((acc, cur) => { + return (acc[cur] = uuid()), acc; + }, {}); + + const promises = Object.entries(map).map(async ([nodeType, nodeId]) => { + const details = { + name: 'My Test Workflow', + active: false, + connections: {}, + nodeTypes: {}, + nodes: [ + { + id: nodeId, + name: 'My Node', + type: nodeType, + parameters: { + operation: 'executeQuery', + query: '=SELECT * FROM {{ $json.table }}', + additionalFields: {}, + }, + typeVersion: 1, + position: [0, 0] as [number, number], + }, + ], + }; + + return Db.collections.Workflow.save(details); + }); + + await Promise.all(promises); + + const testAudit = await audit(['database']); + + const section = getRiskSection( + testAudit, + DATABASE_REPORT.RISK, + DATABASE_REPORT.SECTIONS.EXPRESSIONS_IN_QUERIES, + ); + + expect(section.location).toHaveLength(SQL_NODE_TYPES.size); + + for (const loc of section.location) { + if (loc.kind === 'node') { + expect(loc.nodeId).toBe(map[loc.nodeType]); + } + } +}); + +test('should report expressions in query params', async () => { + const map = [...SQL_NODE_TYPES_WITH_QUERY_PARAMS].reduce<{ [nodeType: string]: string }>( + (acc, cur) => { + return (acc[cur] = uuid()), acc; + }, + {}, + ); + + const promises = Object.entries(map).map(async ([nodeType, nodeId]) => { + const details = { + name: 'My Test Workflow', + active: false, + connections: {}, + nodeTypes: {}, + nodes: [ + { + id: nodeId, + name: 'My Node', + type: nodeType, + parameters: { + operation: 'executeQuery', + query: 'SELECT * FROM users WHERE id = $1;', + additionalFields: { + queryParams: '={{ $json.userId }}', + }, + }, + typeVersion: 1, + position: [0, 0] as [number, number], + }, + ], + }; + + return Db.collections.Workflow.save(details); + }); + + await Promise.all(promises); + + const testAudit = await audit(['database']); + + const section = getRiskSection( + testAudit, + DATABASE_REPORT.RISK, + DATABASE_REPORT.SECTIONS.EXPRESSIONS_IN_QUERY_PARAMS, + ); + + expect(section.location).toHaveLength(SQL_NODE_TYPES_WITH_QUERY_PARAMS.size); + + for (const loc of section.location) { + if (loc.kind === 'node') { + expect(loc.nodeId).toBe(map[loc.nodeType]); + } + } +}); + +test('should report unused query params', async () => { + const map = [...SQL_NODE_TYPES_WITH_QUERY_PARAMS].reduce<{ [nodeType: string]: string }>( + (acc, cur) => { + return (acc[cur] = uuid()), acc; + }, + {}, + ); + + const promises = Object.entries(map).map(async ([nodeType, nodeId]) => { + const details = { + name: 'My Test Workflow', + active: false, + connections: {}, + nodeTypes: {}, + nodes: [ + { + id: nodeId, + name: 'My Node', + type: nodeType, + parameters: { + operation: 'executeQuery', + query: 'SELECT * FROM users WHERE id = 123;', + }, + typeVersion: 1, + position: [0, 0] as [number, number], + }, + ], + }; + + return Db.collections.Workflow.save(details); + }); + + await Promise.all(promises); + + const testAudit = await audit(['database']); + + const section = getRiskSection( + testAudit, + DATABASE_REPORT.RISK, + DATABASE_REPORT.SECTIONS.UNUSED_QUERY_PARAMS, + ); + + expect(section.location).toHaveLength(SQL_NODE_TYPES_WITH_QUERY_PARAMS.size); + + for (const loc of section.location) { + if (loc.kind === 'node') { + expect(loc.nodeId).toBe(map[loc.nodeType]); + } + } +}); + +test('should not report non-database node', async () => { + await saveManualTriggerWorkflow(); + + const testAudit = await audit(['database']); + + expect(testAudit).toBeEmptyArray(); +}); diff --git a/packages/cli/test/integration/audit/filesystem.risk.test.ts b/packages/cli/test/integration/audit/filesystem.risk.test.ts new file mode 100644 index 0000000000..88a8115138 --- /dev/null +++ b/packages/cli/test/integration/audit/filesystem.risk.test.ts @@ -0,0 +1,76 @@ +import { v4 as uuid } from 'uuid'; +import * as Db from '@/Db'; +import { audit } from '@/audit'; +import { FILESYSTEM_INTERACTION_NODE_TYPES, FILESYSTEM_REPORT } from '@/audit/constants'; +import { getRiskSection, saveManualTriggerWorkflow } from './utils'; +import * as testDb from '../shared/testDb'; + +let testDbName = ''; + +beforeAll(async () => { + const initResult = await testDb.init(); + testDbName = initResult.testDbName; +}); + +beforeEach(async () => { + await testDb.truncate(['Workflow'], testDbName); +}); + +afterAll(async () => { + await testDb.terminate(testDbName); +}); + +test('should report filesystem interaction nodes', async () => { + const map = [...FILESYSTEM_INTERACTION_NODE_TYPES].reduce<{ [nodeType: string]: string }>( + (acc, cur) => { + return (acc[cur] = uuid()), acc; + }, + {}, + ); + + const promises = Object.entries(map).map(async ([nodeType, nodeId]) => { + const details = { + name: 'My Test Workflow', + active: false, + connections: {}, + nodeTypes: {}, + nodes: [ + { + id: nodeId, + name: 'My Node', + type: nodeType, + typeVersion: 1, + position: [0, 0] as [number, number], + }, + ], + }; + + return Db.collections.Workflow.save(details); + }); + + await Promise.all(promises); + + const testAudit = await audit(['filesystem']); + + const section = getRiskSection( + testAudit, + FILESYSTEM_REPORT.RISK, + FILESYSTEM_REPORT.SECTIONS.FILESYSTEM_INTERACTION_NODES, + ); + + expect(section.location).toHaveLength(FILESYSTEM_INTERACTION_NODE_TYPES.size); + + for (const loc of section.location) { + if (loc.kind === 'node') { + expect(loc.nodeId).toBe(map[loc.nodeType]); + } + } +}); + +test('should not report non-filesystem-interaction node', async () => { + await saveManualTriggerWorkflow(); + + const testAudit = await audit(['filesystem']); + + expect(testAudit).toBeEmptyArray(); +}); diff --git a/packages/cli/test/integration/audit/instance.risk.test.ts b/packages/cli/test/integration/audit/instance.risk.test.ts new file mode 100644 index 0000000000..2e00653e71 --- /dev/null +++ b/packages/cli/test/integration/audit/instance.risk.test.ts @@ -0,0 +1,255 @@ +import { v4 as uuid } from 'uuid'; +import * as Db from '@/Db'; +import { audit } from '@/audit'; +import { INSTANCE_REPORT, WEBHOOK_VALIDATOR_NODE_TYPES } from '@/audit/constants'; +import { + getRiskSection, + saveManualTriggerWorkflow, + MOCK_09990_N8N_VERSION, + simulateOutdatedInstanceOnce, + simulateUpToDateInstance, +} from './utils'; +import * as testDb from '../shared/testDb'; +import { toReportTitle } from '@/audit/utils'; +import config from '@/config'; + +let testDbName = ''; + +beforeAll(async () => { + const initResult = await testDb.init(); + testDbName = initResult.testDbName; + + simulateUpToDateInstance(); +}); + +beforeEach(async () => { + await testDb.truncate(['Workflow'], testDbName); +}); + +afterAll(async () => { + await testDb.terminate(testDbName); +}); + +test('should report webhook lacking authentication', async () => { + const targetNodeId = uuid(); + + const details = { + name: 'My Test Workflow', + active: true, + nodeTypes: {}, + connections: {}, + nodes: [ + { + parameters: { + path: uuid(), + options: {}, + }, + id: targetNodeId, + name: 'Webhook', + type: 'n8n-nodes-base.webhook', + typeVersion: 1, + position: [0, 0] as [number, number], + webhookId: uuid(), + }, + ], + }; + + await Db.collections.Workflow.save(details); + + const testAudit = await audit(['instance']); + + const section = getRiskSection( + testAudit, + INSTANCE_REPORT.RISK, + INSTANCE_REPORT.SECTIONS.UNPROTECTED_WEBHOOKS, + ); + + if (!section.location) { + fail('Expected section to have locations'); + } + + expect(section.location).toHaveLength(1); + + expect(section.location[0].nodeId).toBe(targetNodeId); +}); + +test('should not report webhooks having basic or header auth', async () => { + const promises = ['basicAuth', 'headerAuth'].map(async (authType) => { + const details = { + name: 'My Test Workflow', + active: true, + nodeTypes: {}, + connections: {}, + nodes: [ + { + parameters: { + path: uuid(), + authentication: authType, + options: {}, + }, + id: uuid(), + name: 'Webhook', + type: 'n8n-nodes-base.webhook', + typeVersion: 1, + position: [0, 0] as [number, number], + webhookId: uuid(), + }, + ], + }; + + return Db.collections.Workflow.save(details); + }); + + await Promise.all(promises); + + const testAudit = await audit(['instance']); + + const report = testAudit?.[toReportTitle('instance')]; + + if (!report) { + fail('Expected test audit to have instance risk report'); + } + + for (const section of report.sections) { + expect(section.title).not.toBe(INSTANCE_REPORT.SECTIONS.UNPROTECTED_WEBHOOKS); + } +}); + +test('should not report webhooks validated by direct children', async () => { + const promises = [...WEBHOOK_VALIDATOR_NODE_TYPES].map(async (nodeType) => { + const details = { + name: 'My Test Workflow', + active: true, + nodeTypes: {}, + nodes: [ + { + parameters: { + path: uuid(), + options: {}, + }, + id: uuid(), + name: 'Webhook', + type: 'n8n-nodes-base.webhook', + typeVersion: 1, + position: [0, 0] as [number, number], + webhookId: uuid(), + }, + { + id: uuid(), + name: 'My Node', + type: nodeType, + typeVersion: 1, + position: [0, 0] as [number, number], + }, + ], + connections: { + Webhook: { + main: [ + [ + { + node: 'My Node', + type: 'main', + index: 0, + }, + ], + ], + }, + }, + }; + + return Db.collections.Workflow.save(details); + }); + + await Promise.all(promises); + + const testAudit = await audit(['instance']); + + const report = testAudit?.[toReportTitle('instance')]; + + if (!report) { + fail('Expected test audit to have instance risk report'); + } + + for (const section of report.sections) { + expect(section.title).not.toBe(INSTANCE_REPORT.SECTIONS.UNPROTECTED_WEBHOOKS); + } +}); + +test('should not report non-webhook node', async () => { + await saveManualTriggerWorkflow(); + + const testAudit = await audit(['instance']); + + const report = testAudit?.[toReportTitle('instance')]; + + if (!report) { + fail('Expected test audit to have instance risk report'); + } + + for (const section of report.sections) { + expect(section.title).not.toBe(INSTANCE_REPORT.SECTIONS.UNPROTECTED_WEBHOOKS); + } +}); + +test('should report outdated instance when outdated', async () => { + simulateOutdatedInstanceOnce(); + + const testAudit = await audit(['instance']); + + const section = getRiskSection( + testAudit, + INSTANCE_REPORT.RISK, + INSTANCE_REPORT.SECTIONS.OUTDATED_INSTANCE, + ); + + if (!section.nextVersions) { + fail('Expected section to have next versions'); + } + + expect(section.nextVersions).toHaveLength(1); + + expect(section.nextVersions[0].name).toBe(MOCK_09990_N8N_VERSION.name); +}); + +test('should not report outdated instance when up to date', async () => { + const testAudit = await audit(['instance']); + + const report = testAudit?.[toReportTitle('instance')]; + + if (!report) { + fail('Expected test audit to have instance risk report'); + } + + for (const section of report.sections) { + expect(section.title).not.toBe(INSTANCE_REPORT.SECTIONS.OUTDATED_INSTANCE); + } +}); + +test('should report security settings', async () => { + config.set('diagnostics.enabled', true); + + const testAudit = await audit(['instance']); + + const section = getRiskSection( + testAudit, + INSTANCE_REPORT.RISK, + INSTANCE_REPORT.SECTIONS.SECURITY_SETTINGS, + ); + + expect(section.settings).toMatchObject({ + features: { + communityPackagesEnabled: true, + versionNotificationsEnabled: true, + templatesEnabled: true, + publicApiEnabled: false, + userManagementEnabled: true, + }, + auth: { + authExcludeEndpoints: 'none', + basicAuthActive: false, + jwtAuthActive: false, + }, + nodes: { nodesExclude: 'none', nodesInclude: 'none' }, + telemetry: { diagnosticsEnabled: true }, + }); +}); diff --git a/packages/cli/test/integration/audit/nodes.risk.test.ts b/packages/cli/test/integration/audit/nodes.risk.test.ts new file mode 100644 index 0000000000..7be91a2330 --- /dev/null +++ b/packages/cli/test/integration/audit/nodes.risk.test.ts @@ -0,0 +1,99 @@ +import { v4 as uuid } from 'uuid'; +import * as Db from '@/Db'; +import { audit } from '@/audit'; +import * as packageModel from '@/CommunityNodes/packageModel'; +import { OFFICIAL_RISKY_NODE_TYPES, NODES_REPORT } from '@/audit/constants'; +import { getRiskSection, MOCK_PACKAGE, saveManualTriggerWorkflow } from './utils'; +import * as testDb from '../shared/testDb'; +import { toReportTitle } from '@/audit/utils'; + +let testDbName = ''; + +beforeAll(async () => { + const initResult = await testDb.init(); + testDbName = initResult.testDbName; +}); + +beforeEach(async () => { + await testDb.truncate(['Workflow'], testDbName); +}); + +afterAll(async () => { + await testDb.terminate(testDbName); +}); + +test('should report risky official nodes', async () => { + const map = [...OFFICIAL_RISKY_NODE_TYPES].reduce<{ [nodeType: string]: string }>((acc, cur) => { + return (acc[cur] = uuid()), acc; + }, {}); + + const promises = Object.entries(map).map(async ([nodeType, nodeId]) => { + const details = { + name: 'My Test Workflow', + active: false, + connections: {}, + nodeTypes: {}, + nodes: [ + { + id: nodeId, + name: 'My Node', + type: nodeType, + typeVersion: 1, + position: [0, 0] as [number, number], + }, + ], + }; + + return Db.collections.Workflow.save(details); + }); + + await Promise.all(promises); + + const testAudit = await audit(['nodes']); + + const section = getRiskSection( + testAudit, + NODES_REPORT.RISK, + NODES_REPORT.SECTIONS.OFFICIAL_RISKY_NODES, + ); + + expect(section.location).toHaveLength(OFFICIAL_RISKY_NODE_TYPES.size); + + for (const loc of section.location) { + if (loc.kind === 'node') { + expect(loc.nodeId).toBe(map[loc.nodeType]); + } + } +}); + +test('should not report non-risky official nodes', async () => { + await saveManualTriggerWorkflow(); + + const testAudit = await audit(['nodes']); + + const report = testAudit?.[toReportTitle('nodes')]; + + if (!report) return; + + for (const section of report.sections) { + expect(section.title).not.toBe(NODES_REPORT.SECTIONS.OFFICIAL_RISKY_NODES); + } +}); + +test('should report community nodes', async () => { + jest.spyOn(packageModel, 'getAllInstalledPackages').mockResolvedValueOnce(MOCK_PACKAGE); + + const testAudit = await audit(['nodes']); + + const section = getRiskSection( + testAudit, + NODES_REPORT.RISK, + NODES_REPORT.SECTIONS.COMMUNITY_NODES, + ); + + expect(section.location).toHaveLength(1); + + if (section.location[0].kind === 'community') { + expect(section.location[0].nodeType).toBe(MOCK_PACKAGE[0].installedNodes[0].type); + } +}); diff --git a/packages/cli/test/integration/audit/utils.ts b/packages/cli/test/integration/audit/utils.ts new file mode 100644 index 0000000000..8669010361 --- /dev/null +++ b/packages/cli/test/integration/audit/utils.ts @@ -0,0 +1,130 @@ +import nock from 'nock'; +import config from '@/config'; +import { v4 as uuid } from 'uuid'; +import * as Db from '@/Db'; +import { toReportTitle } from '@/audit/utils'; +import * as constants from '@/constants'; +import type { Risk } from '@/audit/types'; +import type { InstalledNodes } from '@/databases/entities/InstalledNodes'; +import type { InstalledPackages } from '@/databases/entities/InstalledPackages'; + +type GetSectionKind = C extends 'instance' + ? Risk.InstanceSection + : Risk.StandardSection; + +export function getRiskSection( + testAudit: Risk.Audit | never[], + riskCategory: C, + sectionTitle: string, +): GetSectionKind { + if (Array.isArray(testAudit)) { + throw new Error('Expected test audit not to be an array'); + } + + const report = testAudit[toReportTitle(riskCategory)]; + + if (!report) throw new Error(`Expected risk "${riskCategory}"`); + + for (const section of report.sections) { + if (section.title === sectionTitle) { + return section as GetSectionKind; + } + } + + throw new Error(`Expected section "${sectionTitle}" for risk "${riskCategory}"`); +} + +export async function saveManualTriggerWorkflow() { + const details = { + id: '1', + name: 'My Test Workflow', + active: false, + connections: {}, + nodeTypes: {}, + nodes: [ + { + id: uuid(), + name: 'My Node', + type: 'n8n-nodes-base.manualTrigger', + typeVersion: 1, + position: [0, 0] as [number, number], + }, + ], + }; + + return Db.collections.Workflow.save(details); +} + +export const MOCK_09990_N8N_VERSION = { + name: '0.999.0', + nodes: [ + { + name: 'n8n-nodes-base.testNode', + displayName: 'Test Node', + icon: 'file:testNode.svg', + defaults: { + name: 'Test Node', + }, + }, + ], + createdAt: '2022-11-11T11:11:11.111Z', + description: + 'Includes new nodes, node enhancements, core functionality and bug fixes', + documentationUrl: 'https://docs.n8n.io/reference/release-notes/#n8n09990', + hasBreakingChange: false, + hasSecurityFix: false, + hasSecurityIssue: false, + securityIssueFixVersion: null, +}; + +export const MOCK_01110_N8N_VERSION = { + name: '0.111.0', + nodes: [], + createdAt: '2022-01-01T00:00:00.000Z', + description: + 'Includes new nodes, node enhancements, core functionality and bug fixes', + documentationUrl: 'https://docs.n8n.io/reference/release-notes/#n8n01110', + hasBreakingChange: false, + hasSecurityFix: false, + hasSecurityIssue: false, + securityIssueFixVersion: null, +}; + +export const MOCK_PACKAGE: InstalledPackages[] = [ + { + createdAt: new Date(), + updatedAt: new Date(), + packageName: 'n8n-nodes-test', + installedVersion: '1.1.2', + authorName: 'test', + authorEmail: 'test@test.com', + setUpdateDate: () => {}, + installedNodes: [ + { + name: 'My Test Node', + type: 'myTestNode', + latestVersion: '1', + } as InstalledNodes, + ], + }, +]; + +export function simulateOutdatedInstanceOnce(versionName = MOCK_01110_N8N_VERSION.name) { + const baseUrl = config.getEnv('versionNotifications.endpoint') + '/'; + + jest + .spyOn(constants, 'getN8nPackageJson') + .mockReturnValueOnce({ name: 'n8n', version: versionName }); + + nock(baseUrl).get(versionName).reply(200, [MOCK_01110_N8N_VERSION, MOCK_09990_N8N_VERSION]); +} + +export function simulateUpToDateInstance(versionName = MOCK_09990_N8N_VERSION.name) { + const baseUrl = config.getEnv('versionNotifications.endpoint') + '/'; + + jest + .spyOn(constants, 'getN8nPackageJson') + .mockReturnValueOnce({ name: 'n8n', version: versionName }); + + nock(baseUrl).persist().get(versionName).reply(200, [MOCK_09990_N8N_VERSION]); +} diff --git a/packages/nodes-base/nodes/N8n/AuditDescription.ts b/packages/nodes-base/nodes/N8n/AuditDescription.ts new file mode 100644 index 0000000000..4a03705030 --- /dev/null +++ b/packages/nodes-base/nodes/N8n/AuditDescription.ts @@ -0,0 +1,90 @@ +import type { INodeProperties } from 'n8n-workflow'; + +export const auditOperations: INodeProperties[] = [ + { + displayName: 'Operation', + name: 'operation', + type: 'options', + noDataExpression: true, + default: 'get', + displayOptions: { + show: { + resource: ['audit'], + }, + }, + options: [ + { + name: 'Generate', + value: 'generate', + action: 'Generate a security audit', + description: 'Generate a security audit for this n8n instance', + routing: { + request: { + method: 'POST', + url: '/audit', + }, + }, + }, + ], + }, +]; + +export const auditFields: INodeProperties[] = [ + { + displayName: 'Additional Options', + name: 'additionalOptions', + type: 'collection', + placeholder: 'Add Filter', + displayOptions: { + show: { + resource: ['audit'], + }, + }, + routing: { + request: { + body: { + additionalOptions: '={{ $value }}', + }, + }, + }, + default: {}, + options: [ + { + displayName: 'Categories', + name: 'categories', + description: 'Risk categories to include in the audit', + type: 'multiOptions', + default: [], + options: [ + { + name: 'Credentials', + value: 'credentials', + }, + { + name: 'Database', + value: 'database', + }, + { + name: 'Filesystem', + value: 'filesystem', + }, + { + name: 'Instance', + value: 'instance', + }, + { + name: 'Nodes', + value: 'nodes', + }, + ], + }, + { + displayName: 'Days Abandoned Workflow', + name: 'daysAbandonedWorkflow', + description: 'Days for a workflow to be considered abandoned if not executed', + type: 'number', + default: 90, + }, + ], + }, +]; diff --git a/packages/nodes-base/nodes/N8n/N8n.node.ts b/packages/nodes-base/nodes/N8n/N8n.node.ts index 2525e4ee5d..0d9c36cff4 100644 --- a/packages/nodes-base/nodes/N8n/N8n.node.ts +++ b/packages/nodes-base/nodes/N8n/N8n.node.ts @@ -1,4 +1,5 @@ -import { INodeType, INodeTypeDescription } from 'n8n-workflow'; +import type { INodeType, INodeTypeDescription } from 'n8n-workflow'; +import { auditFields, auditOperations } from './AuditDescription'; import { credentialFields, credentialOperations } from './CredentialDescription'; import { executionFields, executionOperations } from './ExecutionDescription'; import { workflowFields, workflowOperations } from './WorkflowDescription'; @@ -44,6 +45,10 @@ export class N8n implements INodeType { type: 'options', noDataExpression: true, options: [ + { + name: 'Audit', + value: 'audit', + }, { name: 'Credential', value: 'credential', @@ -60,6 +65,9 @@ export class N8n implements INodeType { default: 'workflow', }, + ...auditOperations, + ...auditFields, + ...credentialOperations, ...credentialFields, diff --git a/packages/nodes-base/nodes/N8n/n8n.svg b/packages/nodes-base/nodes/N8n/n8n.svg index e2e67c20fb..17ba54479a 100644 --- a/packages/nodes-base/nodes/N8n/n8n.svg +++ b/packages/nodes-base/nodes/N8n/n8n.svg @@ -1 +1,3 @@ - \ No newline at end of file + + +