From 9bda02122d354ebbd4db8036d442cfef8b58303e Mon Sep 17 00:00:00 2001 From: "n8n-cat-bot[bot]" <283985454+n8n-cat-bot[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 12:35:38 +0000 Subject: [PATCH] =?UTF-8?q?chore:=20Schema-First=20migration=20=E2=80=94?= =?UTF-8?q?=20audit=20(#39207)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: n8n-cat-bot[bot] Co-authored-by: Sam Wooler Co-authored-by: Claude Sonnet 5 --- .../src/dto/audit/audit-public.dto.ts | 123 ++++ .../src/dto/audit/audit-public.openapi.ts | 117 ++++ packages/@n8n/api-types/src/dto/index.ts | 8 + packages/cli/eslint.config.mjs | 1 - packages/cli/oxlint.config.mts | 1 - packages/cli/src/public-api/types.ts | 13 - .../v1/controllers/audit.public.controller.ts | 39 ++ .../src/public-api/v1/controllers/index.ts | 1 + .../v1/handlers/audit/audit.handler.ts | 27 - .../v1/handlers/audit/spec/paths/audit.yml | 37 - .../spec/paths/generateAudit.generated.yml | 643 ++++++++++++++++++ .../v1/handlers/audit/spec/schemas/audit.yml | 105 --- .../v1/openapi-gen/decorator-routes.ts | 2 + .../v1/openapi.decorator-routes.generated.yml | 3 + packages/cli/src/public-api/v1/openapi.yml | 2 - .../v1/shared/spec/responses/_index.yml | 2 + .../spec/responses/internalServerError.yml | 1 + .../v1/shared/spec/schemas/_index.yml | 2 - .../spec/schemas/auditVersion.generated.yml | 35 + packages/cli/src/security-audit/constants.ts | 10 +- packages/cli/src/security-audit/types.ts | 10 +- .../test/integration/public-api/audit.test.ts | 127 ++++ 22 files changed, 1109 insertions(+), 200 deletions(-) create mode 100644 packages/@n8n/api-types/src/dto/audit/audit-public.dto.ts create mode 100644 packages/@n8n/api-types/src/dto/audit/audit-public.openapi.ts create mode 100644 packages/cli/src/public-api/v1/controllers/audit.public.controller.ts delete mode 100644 packages/cli/src/public-api/v1/handlers/audit/audit.handler.ts delete mode 100644 packages/cli/src/public-api/v1/handlers/audit/spec/paths/audit.yml create mode 100644 packages/cli/src/public-api/v1/handlers/audit/spec/paths/generateAudit.generated.yml delete mode 100644 packages/cli/src/public-api/v1/handlers/audit/spec/schemas/audit.yml create mode 100644 packages/cli/src/public-api/v1/shared/spec/responses/internalServerError.yml create mode 100644 packages/cli/src/public-api/v1/shared/spec/schemas/auditVersion.generated.yml create mode 100644 packages/cli/test/integration/public-api/audit.test.ts diff --git a/packages/@n8n/api-types/src/dto/audit/audit-public.dto.ts b/packages/@n8n/api-types/src/dto/audit/audit-public.dto.ts new file mode 100644 index 00000000000..cf7c4879624 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/audit/audit-public.dto.ts @@ -0,0 +1,123 @@ +import '../../openapi-extend'; + +import { z } from 'zod'; + +import { auditReportFieldDocs, auditRequestFieldDocs } from './audit-public.openapi'; +import { Z } from '../../zod-class'; + +export const SECURITY_AUDIT_CATEGORIES = [ + 'credentials', + 'database', + 'nodes', + 'filesystem', + 'instance', +] as const; + +export class GenerateAuditPublicDto extends Z.class({ + additionalOptions: z + .object({ + daysAbandonedWorkflow: z + .number() + .int() + .optional() + .openapi(auditRequestFieldDocs.daysAbandonedWorkflow), + categories: z.array(z.enum(SECURITY_AUDIT_CATEGORIES)).optional(), + }) + .optional(), +}) { + // Every field is optional, so an omitted body should generate the default audit + static override safeParse(data: unknown) { + return super.safeParse(data ?? {}); + } + + static override parse(data: unknown) { + return super.parse(data ?? {}); + } +} + +const riskLocationSchema = z.discriminatedUnion('kind', [ + z.object({ + kind: z.literal('node'), + workflowId: z.string(), + workflowName: z.string(), + nodeId: z.string(), + nodeName: z.string(), + nodeType: z.string(), + }), + z.object({ + kind: z.literal('credential'), + id: z.string(), + name: z.string(), + }), + z.object({ + kind: z.literal('community'), + nodeType: z.string(), + packageUrl: z.string(), + }), + z.object({ + kind: z.literal('custom'), + nodeType: z.string(), + filePath: z.string(), + }), +]); + +const versionNodeArraySchema = z.array(z.unknown()); + +// Registered under a refId so the generator emits it once as a shared component instead of +// inlining it under each of the five named risk reports below. +const versionSchema = z + .object({ + name: z.string(), + nodes: versionNodeArraySchema, + createdAt: z.string(), + description: z.string(), + documentationUrl: z.string(), + hasBreakingChange: z.boolean(), + // The version-notifications API returns `null`, not `false`/`''`, when a release has no + // security fix or issue to report. + hasSecurityFix: z.boolean().nullable(), + hasSecurityIssue: z.boolean().nullable(), + securityIssueFixVersion: z.string().nullable(), + }) + .openapi('AuditVersion'); + +const riskReportSectionSchema = z.object({ + title: z.string(), + description: z.string(), + recommendation: z.string(), + location: z.array(riskLocationSchema).optional(), + settings: z.record(z.string(), z.unknown()).optional(), + nextVersions: z.array(versionSchema).optional(), +}); + +const riskReportSchema = z.object({ + risk: z.enum(SECURITY_AUDIT_CATEGORIES), + sections: z.array(riskReportSectionSchema), +}); + +const auditReportsPublicSchema = z.object({ + 'Credentials Risk Report': riskReportSchema.openapi(auditReportFieldDocs.credentials).optional(), + 'Database Risk Report': riskReportSchema.openapi(auditReportFieldDocs.database).optional(), + 'Filesystem Risk Report': riskReportSchema.openapi(auditReportFieldDocs.filesystem).optional(), + 'Nodes Risk Report': riskReportSchema.openapi(auditReportFieldDocs.nodes).optional(), + 'Instance Risk Report': riskReportSchema.openapi(auditReportFieldDocs.instance).optional(), +}); + +export const auditPublicSchema = z.union([ + auditReportsPublicSchema, + z.array(z.unknown()).length(0).openapi({ maxItems: 0 }), +]); + +export type AuditPublic = z.infer; + +export class AuditPublicDto { + static schema = auditPublicSchema; + + static safeParse(data: unknown) { + return auditPublicSchema.safeParse(data); + } + + static parse(data: unknown): AuditPublic { + return auditPublicSchema.parse(data); + } +} diff --git a/packages/@n8n/api-types/src/dto/audit/audit-public.openapi.ts b/packages/@n8n/api-types/src/dto/audit/audit-public.openapi.ts new file mode 100644 index 00000000000..bcb67ba3aa0 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/audit/audit-public.openapi.ts @@ -0,0 +1,117 @@ +import type { ZodOpenAPIMetadata } from '@asteasolutions/zod-to-openapi'; + +export const auditRequestFieldDocs = { + daysAbandonedWorkflow: { + description: 'Days for a workflow to be considered abandoned if not executed', + }, +} as const satisfies Record; + +export const auditReportFieldDocs: Record = { + credentials: { + 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: { + 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: { + 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: { + 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: { + example: { + risk: 'instance', + 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: 'node', + workflowId: '1', + workflowName: 'My Workflow', + nodeId: '51eb5852-ce0b-4806-b4ff-e41322a4041a', + nodeName: 'Webhook', + nodeType: 'n8n-nodes-base.webhook', + }, + ], + }, + ], + }, + }, +}; diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index b0035683101..232ea0ba1e2 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -54,6 +54,14 @@ export type { DiscoverDataPublic } from './discover/discover-public.dto'; export { BreakingChangeReportQueryDto } from './breaking-changes/breaking-change-report-query.dto'; +export { + AuditPublicDto, + GenerateAuditPublicDto, + SECURITY_AUDIT_CATEGORIES, + auditPublicSchema, + type AuditPublic, +} from './audit/audit-public.dto'; + export { LoginRequestDto } from './auth/login-request.dto'; export { ResolveSignupTokenQueryDto } from './auth/resolve-signup-token-query.dto'; export { EmbedLoginQueryDto } from './auth/embed-login-query.dto'; diff --git a/packages/cli/eslint.config.mjs b/packages/cli/eslint.config.mjs index aafb79f3b2e..6513203e2ba 100644 --- a/packages/cli/eslint.config.mjs +++ b/packages/cli/eslint.config.mjs @@ -137,7 +137,6 @@ export default defineConfig( // `@PublicApiController` classes (API-70). NEVER add to this list — a new tuple handler // must fail CI. Entries are removed as each handler becomes a controller. files: [ - './src/public-api/v1/handlers/audit/audit.handler.ts', './src/public-api/v1/handlers/data-tables/data-tables.columns.handler.ts', './src/public-api/v1/handlers/data-tables/data-tables.rows.handler.ts', './src/public-api/v1/handlers/ldap/ldap.handler.ts', diff --git a/packages/cli/oxlint.config.mts b/packages/cli/oxlint.config.mts index a4e07be2bcd..cd35f2948ce 100644 --- a/packages/cli/oxlint.config.mts +++ b/packages/cli/oxlint.config.mts @@ -141,7 +141,6 @@ export default defineConfig({ // `@PublicApiController` classes (API-70). NEVER add to this list — a new tuple handler // must fail CI. Entries are removed as each handler becomes a controller. files: [ - './src/public-api/v1/handlers/audit/audit.handler.ts', './src/public-api/v1/handlers/data-tables/data-tables.columns.handler.ts', './src/public-api/v1/handlers/data-tables/data-tables.rows.handler.ts', './src/public-api/v1/handlers/evaluations/evaluations.handler.ts', diff --git a/packages/cli/src/public-api/types.ts b/packages/cli/src/public-api/types.ts index f73a7f3e678..b4628c6e4f9 100644 --- a/packages/cli/src/public-api/types.ts +++ b/packages/cli/src/public-api/types.ts @@ -15,7 +15,6 @@ import type { import type { AuthenticatedRequest } from '@n8n/db'; import type { AuthlessRequest } from '@/requests'; -import type { Risk } from '@/security-audit/types'; export type PaginatedRequest = AuthenticatedRequest< {}, @@ -185,18 +184,6 @@ export declare namespace CommunityPackageRequest { type Uninstall = AuthenticatedRequest<{ name: string }>; } -// ---------------------------------- -// /audit -// ---------------------------------- - -export declare namespace AuditRequest { - type Generate = AuthenticatedRequest< - {}, - {}, - { additionalOptions?: { categories?: Risk.Category[]; daysAbandonedWorkflow?: number } } - >; -} - export declare namespace LogStreamingRequest { type GetEventTypes = AuthenticatedRequest; type GetDestinations = AuthenticatedRequest; diff --git a/packages/cli/src/public-api/v1/controllers/audit.public.controller.ts b/packages/cli/src/public-api/v1/controllers/audit.public.controller.ts new file mode 100644 index 00000000000..8cfe45d1e26 --- /dev/null +++ b/packages/cli/src/public-api/v1/controllers/audit.public.controller.ts @@ -0,0 +1,39 @@ +import { AuditPublicDto, GenerateAuditPublicDto, type AuditPublic } from '@n8n/api-types'; +import type { AuthenticatedRequest } from '@n8n/db'; +import { + ApiDescription, + ApiErrorResponse, + ApiKeyScope, + ApiResponse, + ApiSummary, + ApiTags, + Body, + Post, + PublicApiController, +} from '@n8n/decorators'; +import type { Response } from 'express'; + +import { SecurityAuditService } from '@/security-audit/security-audit.service'; + +@PublicApiController('/audit') +export class AuditPublicController { + constructor(private readonly securityAuditService: SecurityAuditService) {} + + @Post('/') + @ApiKeyScope('securityAudit:generate') + @ApiSummary('Generate an audit') + @ApiDescription('Generate a security audit for your n8n instance.') + @ApiTags(['Audit']) + @ApiResponse(200, AuditPublicDto) + @ApiErrorResponse(500) + async generateAudit( + _req: AuthenticatedRequest, + _res: Response, + @Body body: GenerateAuditPublicDto, + ): Promise { + return await this.securityAuditService.run( + body.additionalOptions?.categories, + body.additionalOptions?.daysAbandonedWorkflow, + ); + } +} diff --git a/packages/cli/src/public-api/v1/controllers/index.ts b/packages/cli/src/public-api/v1/controllers/index.ts index f5dcc5a9de8..8b8cc2574ca 100644 --- a/packages/cli/src/public-api/v1/controllers/index.ts +++ b/packages/cli/src/public-api/v1/controllers/index.ts @@ -3,6 +3,7 @@ * decorator metadata is registered before PublicApiControllerRegistry / * scope-parity / discover run. */ +import './audit.public.controller'; import './community-packages.public.controller'; import './credential-type-policies.public.controller'; import './credentials.public.controller'; diff --git a/packages/cli/src/public-api/v1/handlers/audit/audit.handler.ts b/packages/cli/src/public-api/v1/handlers/audit/audit.handler.ts deleted file mode 100644 index 228b9815caf..00000000000 --- a/packages/cli/src/public-api/v1/handlers/audit/audit.handler.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { Container } from '@n8n/di'; - -import type { AuditRequest } from '@/public-api/types'; - -import type { PublicAPIEndpoint } from '../../shared/handler.types'; -import { apiKeyHasScopeWithGlobalScopeFallback } from '../../shared/middlewares/global.middleware'; - -type AuditHandlers = { - generateAudit: PublicAPIEndpoint; -}; - -const auditHandlers: AuditHandlers = { - generateAudit: [ - apiKeyHasScopeWithGlobalScopeFallback({ scope: 'securityAudit:generate' }), - async (req, res) => { - const { SecurityAuditService } = await import('@/security-audit/security-audit.service.js'); - const result = await Container.get(SecurityAuditService).run( - req.body?.additionalOptions?.categories, - req.body?.additionalOptions?.daysAbandonedWorkflow, - ); - - return res.json(result); - }, - ], -}; - -export = auditHandlers; diff --git a/packages/cli/src/public-api/v1/handlers/audit/spec/paths/audit.yml b/packages/cli/src/public-api/v1/handlers/audit/spec/paths/audit.yml deleted file mode 100644 index 610587e0ccd..00000000000 --- a/packages/cli/src/public-api/v1/handlers/audit/spec/paths/audit.yml +++ /dev/null @@ -1,37 +0,0 @@ -post: - x-eov-operation-id: generateAudit - x-required-scope: securityAudit:generate - 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/public-api/v1/handlers/audit/spec/paths/generateAudit.generated.yml b/packages/cli/src/public-api/v1/handlers/audit/spec/paths/generateAudit.generated.yml new file mode 100644 index 00000000000..c9bd8281f75 --- /dev/null +++ b/packages/cli/src/public-api/v1/handlers/audit/spec/paths/generateAudit.generated.yml @@ -0,0 +1,643 @@ +operationId: generateAudit +tags: + - Audit +summary: Generate an audit +description: Generate a security audit for your n8n instance. +x-required-scope: securityAudit:generate +x-eov-operation-id: unreachable +x-eov-operation-handler: v1/handlers/decorator-routed.handler +x-decorator-routed: true +requestBody: + 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: + anyOf: + - type: object + properties: + Credentials Risk Report: + type: object + properties: + risk: + type: string + enum: + - credentials + - database + - nodes + - filesystem + - instance + sections: + type: array + items: + type: object + properties: + title: + type: string + description: + type: string + recommendation: + type: string + location: + type: array + items: + oneOf: + - type: object + properties: + kind: + type: string + enum: + - node + workflowId: + type: string + workflowName: + type: string + nodeId: + type: string + nodeName: + type: string + nodeType: + type: string + required: + - kind + - workflowId + - workflowName + - nodeId + - nodeName + - nodeType + - type: object + properties: + kind: + type: string + enum: + - credential + id: + type: string + name: + type: string + required: + - kind + - id + - name + - type: object + properties: + kind: + type: string + enum: + - community + nodeType: + type: string + packageUrl: + type: string + required: + - kind + - nodeType + - packageUrl + - type: object + properties: + kind: + type: string + enum: + - custom + nodeType: + type: string + filePath: + type: string + required: + - kind + - nodeType + - filePath + settings: + type: object + additionalProperties: + nullable: true + nextVersions: + type: array + items: + $ref: ../../../../shared/spec/schemas/auditVersion.generated.yml + required: + - title + - description + - recommendation + required: + - risk + - sections + 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 + properties: + risk: + type: string + enum: + - credentials + - database + - nodes + - filesystem + - instance + sections: + type: array + items: + type: object + properties: + title: + type: string + description: + type: string + recommendation: + type: string + location: + type: array + items: + oneOf: + - type: object + properties: + kind: + type: string + enum: + - node + workflowId: + type: string + workflowName: + type: string + nodeId: + type: string + nodeName: + type: string + nodeType: + type: string + required: + - kind + - workflowId + - workflowName + - nodeId + - nodeName + - nodeType + - type: object + properties: + kind: + type: string + enum: + - credential + id: + type: string + name: + type: string + required: + - kind + - id + - name + - type: object + properties: + kind: + type: string + enum: + - community + nodeType: + type: string + packageUrl: + type: string + required: + - kind + - nodeType + - packageUrl + - type: object + properties: + kind: + type: string + enum: + - custom + nodeType: + type: string + filePath: + type: string + required: + - kind + - nodeType + - filePath + settings: + type: object + additionalProperties: + nullable: true + nextVersions: + type: array + items: + $ref: ../../../../shared/spec/schemas/auditVersion.generated.yml + required: + - title + - description + - recommendation + required: + - risk + - sections + 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 + properties: + risk: + type: string + enum: + - credentials + - database + - nodes + - filesystem + - instance + sections: + type: array + items: + type: object + properties: + title: + type: string + description: + type: string + recommendation: + type: string + location: + type: array + items: + oneOf: + - type: object + properties: + kind: + type: string + enum: + - node + workflowId: + type: string + workflowName: + type: string + nodeId: + type: string + nodeName: + type: string + nodeType: + type: string + required: + - kind + - workflowId + - workflowName + - nodeId + - nodeName + - nodeType + - type: object + properties: + kind: + type: string + enum: + - credential + id: + type: string + name: + type: string + required: + - kind + - id + - name + - type: object + properties: + kind: + type: string + enum: + - community + nodeType: + type: string + packageUrl: + type: string + required: + - kind + - nodeType + - packageUrl + - type: object + properties: + kind: + type: string + enum: + - custom + nodeType: + type: string + filePath: + type: string + required: + - kind + - nodeType + - filePath + settings: + type: object + additionalProperties: + nullable: true + nextVersions: + type: array + items: + $ref: ../../../../shared/spec/schemas/auditVersion.generated.yml + required: + - title + - description + - recommendation + required: + - risk + - sections + 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 + properties: + risk: + type: string + enum: + - credentials + - database + - nodes + - filesystem + - instance + sections: + type: array + items: + type: object + properties: + title: + type: string + description: + type: string + recommendation: + type: string + location: + type: array + items: + oneOf: + - type: object + properties: + kind: + type: string + enum: + - node + workflowId: + type: string + workflowName: + type: string + nodeId: + type: string + nodeName: + type: string + nodeType: + type: string + required: + - kind + - workflowId + - workflowName + - nodeId + - nodeName + - nodeType + - type: object + properties: + kind: + type: string + enum: + - credential + id: + type: string + name: + type: string + required: + - kind + - id + - name + - type: object + properties: + kind: + type: string + enum: + - community + nodeType: + type: string + packageUrl: + type: string + required: + - kind + - nodeType + - packageUrl + - type: object + properties: + kind: + type: string + enum: + - custom + nodeType: + type: string + filePath: + type: string + required: + - kind + - nodeType + - filePath + settings: + type: object + additionalProperties: + nullable: true + nextVersions: + type: array + items: + $ref: ../../../../shared/spec/schemas/auditVersion.generated.yml + required: + - title + - description + - recommendation + required: + - risk + - sections + 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 + properties: + risk: + type: string + enum: + - credentials + - database + - nodes + - filesystem + - instance + sections: + type: array + items: + type: object + properties: + title: + type: string + description: + type: string + recommendation: + type: string + location: + type: array + items: + oneOf: + - type: object + properties: + kind: + type: string + enum: + - node + workflowId: + type: string + workflowName: + type: string + nodeId: + type: string + nodeName: + type: string + nodeType: + type: string + required: + - kind + - workflowId + - workflowName + - nodeId + - nodeName + - nodeType + - type: object + properties: + kind: + type: string + enum: + - credential + id: + type: string + name: + type: string + required: + - kind + - id + - name + - type: object + properties: + kind: + type: string + enum: + - community + nodeType: + type: string + packageUrl: + type: string + required: + - kind + - nodeType + - packageUrl + - type: object + properties: + kind: + type: string + enum: + - custom + nodeType: + type: string + filePath: + type: string + required: + - kind + - nodeType + - filePath + settings: + type: object + additionalProperties: + nullable: true + nextVersions: + type: array + items: + $ref: ../../../../shared/spec/schemas/auditVersion.generated.yml + required: + - title + - description + - recommendation + required: + - risk + - sections + example: + risk: instance + 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: node + workflowId: '1' + workflowName: My Workflow + nodeId: 51eb5852-ce0b-4806-b4ff-e41322a4041a + nodeName: Webhook + nodeType: n8n-nodes-base.webhook + - type: array + items: + nullable: true + maxItems: 0 + '400': + $ref: ../../../../shared/spec/responses/badRequest.yml + '401': + $ref: ../../../../shared/spec/responses/unauthorized.yml + '403': + $ref: ../../../../shared/spec/responses/forbidden.yml + '415': + $ref: ../../../../shared/spec/responses/unsupportedMediaType.yml + '500': + $ref: ../../../../shared/spec/responses/internalServerError.yml diff --git a/packages/cli/src/public-api/v1/handlers/audit/spec/schemas/audit.yml b/packages/cli/src/public-api/v1/handlers/audit/spec/schemas/audit.yml deleted file mode 100644 index 5ee8fe8fcd9..00000000000 --- a/packages/cli/src/public-api/v1/handlers/audit/spec/schemas/audit.yml +++ /dev/null @@ -1,105 +0,0 @@ -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/public-api/v1/openapi-gen/decorator-routes.ts b/packages/cli/src/public-api/v1/openapi-gen/decorator-routes.ts index 7260ffa72aa..99178bb4866 100644 --- a/packages/cli/src/public-api/v1/openapi-gen/decorator-routes.ts +++ b/packages/cli/src/public-api/v1/openapi-gen/decorator-routes.ts @@ -39,6 +39,7 @@ export const ERROR_RESPONSE_REFS = { 409: { $ref: '../../../../shared/spec/responses/conflict.yml' }, 415: { $ref: '../../../../shared/spec/responses/unsupportedMediaType.yml' }, 422: { $ref: '../../../../shared/spec/responses/unprocessableEntity.yml' }, + 500: { $ref: '../../../../shared/spec/responses/internalServerError.yml' }, 503: { $ref: '../../../../shared/spec/responses/serviceUnavailable.yml' }, } as const satisfies Record; @@ -61,6 +62,7 @@ export const ERROR_RESPONSE_DESCRIPTIONS: Record 409: 'Conflict', 415: 'Unsupported media type.', 422: 'Unprocessable Entity', + 500: 'Internal server error.', 503: 'The requested service is temporarily unavailable.', }; diff --git a/packages/cli/src/public-api/v1/openapi.decorator-routes.generated.yml b/packages/cli/src/public-api/v1/openapi.decorator-routes.generated.yml index bc1e02db630..d201dbefffd 100644 --- a/packages/cli/src/public-api/v1/openapi.decorator-routes.generated.yml +++ b/packages/cli/src/public-api/v1/openapi.decorator-routes.generated.yml @@ -3,6 +3,9 @@ info: title: decorator-routes version: 0.0.0 paths: + /audit: + post: + $ref: ./handlers/audit/spec/paths/generateAudit.generated.yml /community-packages: get: $ref: ./handlers/community-packages/spec/paths/getInstalledPackages.generated.yml diff --git a/packages/cli/src/public-api/v1/openapi.yml b/packages/cli/src/public-api/v1/openapi.yml index 09a51929d6c..fe27f37e9dc 100644 --- a/packages/cli/src/public-api/v1/openapi.yml +++ b/packages/cli/src/public-api/v1/openapi.yml @@ -79,8 +79,6 @@ tags: description: Operations about workflows paths: - /audit: - $ref: './handlers/audit/spec/paths/audit.yml' /settings/ldap: $ref: './handlers/ldap/spec/paths/settings.ldap.yml' /settings/ldap/sync: diff --git a/packages/cli/src/public-api/v1/shared/spec/responses/_index.yml b/packages/cli/src/public-api/v1/shared/spec/responses/_index.yml index 3cf9c6b75e8..4773a9ddc12 100644 --- a/packages/cli/src/public-api/v1/shared/spec/responses/_index.yml +++ b/packages/cli/src/public-api/v1/shared/spec/responses/_index.yml @@ -14,5 +14,7 @@ UnsupportedMediaType: $ref: './unsupportedMediaType.yml' UnprocessableEntity: $ref: './unprocessableEntity.yml' +InternalServerError: + $ref: './internalServerError.yml' ServiceUnavailable: $ref: './serviceUnavailable.yml' diff --git a/packages/cli/src/public-api/v1/shared/spec/responses/internalServerError.yml b/packages/cli/src/public-api/v1/shared/spec/responses/internalServerError.yml new file mode 100644 index 00000000000..9f67f8b46ad --- /dev/null +++ b/packages/cli/src/public-api/v1/shared/spec/responses/internalServerError.yml @@ -0,0 +1 @@ +description: Internal server error. diff --git a/packages/cli/src/public-api/v1/shared/spec/schemas/_index.yml b/packages/cli/src/public-api/v1/shared/spec/schemas/_index.yml index 151d64476bf..c580ffb0bfc 100644 --- a/packages/cli/src/public-api/v1/shared/spec/schemas/_index.yml +++ b/packages/cli/src/public-api/v1/shared/spec/schemas/_index.yml @@ -12,5 +12,3 @@ WorkflowSettings: $ref: './../../../handlers/workflows/spec/schemas/workflowSettings.yml' WorkflowList: $ref: './../../../handlers/workflows/spec/schemas/workflowList.yml' -Audit: - $ref: './../../../handlers/audit/spec/schemas/audit.yml' diff --git a/packages/cli/src/public-api/v1/shared/spec/schemas/auditVersion.generated.yml b/packages/cli/src/public-api/v1/shared/spec/schemas/auditVersion.generated.yml new file mode 100644 index 00000000000..61e3aa873b4 --- /dev/null +++ b/packages/cli/src/public-api/v1/shared/spec/schemas/auditVersion.generated.yml @@ -0,0 +1,35 @@ +type: object +properties: + name: + type: string + nodes: + type: array + items: + nullable: true + createdAt: + type: string + description: + type: string + documentationUrl: + type: string + hasBreakingChange: + type: boolean + hasSecurityFix: + type: boolean + nullable: true + hasSecurityIssue: + type: boolean + nullable: true + securityIssueFixVersion: + type: string + nullable: true +required: + - name + - nodes + - createdAt + - description + - documentationUrl + - hasBreakingChange + - hasSecurityFix + - hasSecurityIssue + - securityIssueFixVersion diff --git a/packages/cli/src/security-audit/constants.ts b/packages/cli/src/security-audit/constants.ts index f1ad153e4c7..19b9e7701ba 100644 --- a/packages/cli/src/security-audit/constants.ts +++ b/packages/cli/src/security-audit/constants.ts @@ -1,16 +1,10 @@ -import type { Risk } from '@/security-audit/types'; +import { SECURITY_AUDIT_CATEGORIES } from '@n8n/api-types'; /** * Risk categories */ -export const RISK_CATEGORIES: Risk.Category[] = [ - 'credentials', - 'database', - 'nodes', - 'instance', - 'filesystem', -]; +export const RISK_CATEGORIES = [...SECURITY_AUDIT_CATEGORIES]; /** * Node types diff --git a/packages/cli/src/security-audit/types.ts b/packages/cli/src/security-audit/types.ts index 593e49abadb..8b9673e1a32 100644 --- a/packages/cli/src/security-audit/types.ts +++ b/packages/cli/src/security-audit/types.ts @@ -1,7 +1,9 @@ import type { IWorkflowBase } from 'n8n-workflow'; +import type { RISK_CATEGORIES } from '@/security-audit/constants'; + export namespace Risk { - export type Category = 'database' | 'credentials' | 'nodes' | 'instance' | 'filesystem'; + export type Category = (typeof RISK_CATEGORIES)[number]; type CredLocation = { kind: 'credential'; @@ -79,9 +81,9 @@ export namespace n8n { description: string; documentationUrl: string; hasBreakingChange: boolean; - hasSecurityFix: boolean; - hasSecurityIssue: boolean; - securityIssueFixVersion: string; + hasSecurityFix: boolean | null; + hasSecurityIssue: boolean | null; + securityIssueFixVersion: string | null; }; } diff --git a/packages/cli/test/integration/public-api/audit.test.ts b/packages/cli/test/integration/public-api/audit.test.ts new file mode 100644 index 00000000000..930f9530524 --- /dev/null +++ b/packages/cli/test/integration/public-api/audit.test.ts @@ -0,0 +1,127 @@ +import { createWorkflow, testDb } from '@n8n/backend-test-utils'; +import type { User } from '@n8n/db'; +import nock from 'nock'; +import { randomUUID } from 'node:crypto'; + +import { createOwnerWithApiKey } from '../shared/db/users'; +import { simulateUpToDateInstance } from '../security-audit/utils'; +import * as utils from '../shared/utils'; + +const testServer = utils.setupTestServer({ + endpointGroups: ['publicApi'], + modules: ['community-packages'], +}); + +let scopedOwner: User; +let unscopedOwner: User; + +beforeAll(async () => { + scopedOwner = await createOwnerWithApiKey({ scopes: ['securityAudit:generate'] }); + unscopedOwner = await createOwnerWithApiKey({ scopes: ['workflow:list'] }); +}); + +beforeEach(async () => { + await testDb.truncate(['WorkflowEntity']); + nock.cleanAll(); +}); + +describe('POST /audit', () => { + test('returns the requested security audit reports', async () => { + await createWorkflow({ + nodes: [ + { + id: randomUUID(), + name: 'Read file', + type: 'n8n-nodes-base.readBinaryFile', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + ], + }); + + const response = await testServer + .publicApiAgentFor(scopedOwner) + .post('/audit') + .send({ additionalOptions: { categories: ['filesystem'] } }) + .expect(200); + + expect(response.body).toEqual({ + 'Filesystem Risk Report': { + risk: 'filesystem', + sections: [ + expect.objectContaining({ + title: 'Nodes that interact with the filesystem', + location: [ + expect.objectContaining({ + kind: 'node', + nodeName: 'Read file', + nodeType: 'n8n-nodes-base.readBinaryFile', + }), + ], + }), + ], + }, + }); + }); + + test('returns an empty array when the audit finds no risks', async () => { + const response = await testServer + .publicApiAgentFor(scopedOwner) + .post('/audit') + .send({ additionalOptions: { categories: ['filesystem'] } }) + .expect(200); + + expect(response.body).toEqual([]); + }); + + test('generates the default audit when no body is sent', async () => { + simulateUpToDateInstance(); + + const response = await testServer.publicApiAgentFor(scopedOwner).post('/audit').expect(200); + + expect(response.body).toEqual({ + 'Instance Risk Report': { + risk: 'instance', + sections: [expect.objectContaining({ title: 'Security settings' })], + }, + }); + }); + + test('accepts a zero daysAbandonedWorkflow', async () => { + const response = await testServer + .publicApiAgentFor(scopedOwner) + .post('/audit') + .send({ additionalOptions: { daysAbandonedWorkflow: 0, categories: ['filesystem'] } }) + .expect(200); + + expect(response.body).toEqual([]); + }); + + test('ignores an unknown request field', async () => { + simulateUpToDateInstance(); + + const response = await testServer + .publicApiAgentFor(scopedOwner) + .post('/audit') + .send({ unknown: true }) + .expect(200); + + expect(response.body).toEqual({ + 'Instance Risk Report': { + risk: 'instance', + sections: [expect.objectContaining({ title: 'Security settings' })], + }, + }); + }); + + test('rejects an API key without the securityAudit:generate scope', async () => { + const response = await testServer + .publicApiAgentFor(unscopedOwner) + .post('/audit') + .send({}) + .expect(403); + + expect(response.body).toEqual({ message: 'Forbidden' }); + }); +});