diff --git a/.agents/skills/public-api/SKILL.md b/.agents/skills/public-api/SKILL.md index 1c33a19650a..4e745fb89f5 100644 --- a/.agents/skills/public-api/SKILL.md +++ b/.agents/skills/public-api/SKILL.md @@ -112,7 +112,7 @@ model; reuse only what applies. Decorators, all from `@n8n/decorators`: | `@ApiResponse(status)` / `@ApiResponse(status, Dto)` | Success status + (optional) output DTO; registry `.parse()`s + strips the return value. Exactly one per route — a second `@ApiResponse` throws. `204` can't carry a DTO — throws. | | `@ApiErrorResponse(status)` | Declares an additional documented non-2xx status (e.g. `404`, `409`). Stack multiple for more than one. `400`/`401`/`403` are added automatically (body/query present, always, and `@ApiKeyScope` present, respectively) — don't declare those yourself. | | `@ApiSummary(text)` / `@ApiDescription(text)` / `@ApiTags([...])` | OpenAPI summary/description/tags. `@ApiTags` sorts alphabetically regardless of the order you pass. All optional but expected on every real route. | -| `@Query` / `@Body` / `@Param('name')` | Bind + validate via a `Z.class` DTO / path param. | +| `@Query` / `@Body` / `@Param('name')` | Bind + validate via a `Z.class` DTO / path param. `@Body` is JSON by default; `@Body({ mediaType: 'multipart/form-data', uploadLimits })` takes a `multipart/form-data` body instead — see [Request body media types](reference.md#request-body-media-types). | | `@Licensed('feat')` | Gates the route on a single `BooleanLicenseFeature`; `PublicApiControllerRegistry` runs its own license middleware (after auth/`@ApiKeyScope`/`@ProjectScope`|`@GlobalScope`, before the handler) and 403s unlicensed requests. Only takes one feature — if the gate is an any-of/all-of combination (e.g. `LicenseState.isProvisioningLicensed()`, which is `feat:saml` OR `feat:oidc`), `@Licensed` can't express that; check manually in the handler instead, same as the internal `provisioning.controller.ee.ts`/`role-mapping-rule.controller.ee.ts` do today (throwing `ForbiddenError` on failure). | ## Authorization (easy to get wrong) diff --git a/.agents/skills/public-api/reference.md b/.agents/skills/public-api/reference.md index 1647476d3a9..e67b24a13ae 100644 --- a/.agents/skills/public-api/reference.md +++ b/.agents/skills/public-api/reference.md @@ -103,6 +103,49 @@ scope, RBAC denial. Add whichever apply, matching the nearest existing tests: unchanged. Tests alone cannot show this — see [Verifying a migration](#verifying-a-migration). +## Request body media types + +`@Body` defaults to `application/json`; the body is already parsed by the +app-wide `bodyParser` before the registry sees it. Declaring +`@Body({ mediaType: 'multipart/form-data', uploadLimits })` instead takes a +`multipart/form-data` body: + +- `uploadLimits: () => MultipartUploadLimits` is a thunk, read once the route + handles its first request (not at startup) — read live config inside it + (e.g. `Container.get(GlobalConfig)`), don't inline a literal. +- The route's `@Body` DTO validates text fields merged with uploaded files, + not `req.body` alone. A text field and a file share the same flat object; + a file field uses `publicApiUploadedFileSchema` (`@n8n/api-types`) — a + multer-file-shaped schema that documents itself as `{ type: 'string', + format: 'binary' }`. Several files under the same field name become an + array. +- An unknown field (one the DTO doesn't declare, on a `{ strict: true }` DTO) + fails with `Unexpected form field ""` — not the default "unrecognized + keys" wording. +- Multer's own parsing errors map to the same statuses express-openapi-validator + used: `413` for a size/count limit, `400` for any other multer error, `500` + — unmasked, since this is a `ResponseError` — for anything else (a + malformed body; a missing boundary gives `400` with `multipart file(s) + required`). +- The body is parsed **after** every auth/scope/license/quota gate and + **before** controller/route middlewares — a caller those gates would reject + never has their (possibly huge) body read off the socket, and a middleware + that reads `req.body` sees it already parsed. +- `/discover` shows no request schema for a multipart route (same as a legacy + multipart route today) — a client can't assume a JSON schema it never gets. +- The generator documents the body under the `multipart/form-data` content + key (not `application/json`) and adds `413` alongside `415` to the route's + documented responses automatically. + +Everything above lives in `packages/cli/src/public-api/media-types/`, one +handler per media type (`REQUEST_BODY_HANDLERS` in +`media-types/request-body/index.ts`). Adding a further media type (e.g. +`application/octet-stream`) means: add it to `RequestBodyMediaOptions` in +`@n8n/decorators`'s `controller/types.ts`, write a handler implementing +`RequestBodyHandler` in `media-types/request-body/`, and register it in +`REQUEST_BODY_HANDLERS` — the registry, resolver, generator and `/discover` +need no change, since they all read the handler, not the media type. + ## Migrating legacy EOV endpoints Legacy `express-openapi-validator` endpoints live under @@ -146,8 +189,10 @@ not templates. entry from the `off` allowlists for `no-repository-in-public-api-handler` and `require-public-api-controller` in `packages/cli/eslint.config.mjs` (shrink-only — never extend them). -- For complex legacy-only, multipart, or non-standard endpoints, study the - nearest existing handler first. +- A `multipart/form-data` legacy endpoint migrates onto `@Body({ mediaType: + 'multipart/form-data', uploadLimits })` — see + [Request body media types](#request-body-media-types). For any other + non-standard endpoint, study the nearest existing handler first. - Keep each field in its original position when you extract a request shape shared by two routes, and destructure out the ones a route doesn't take. The generator emits properties in shape order, so a moved field rewrites the `*.generated.yml` diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index 86aa10da5ac..2419865b955 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -231,6 +231,7 @@ export { } from './schemas/source-controlled-file.schema'; export { policyViolationSchema, type PolicyViolation } from './schemas/policy-violation.schema'; +export { publicApiUploadedFileSchema } from './schemas/public-api-uploaded-file.schema'; export { policyCheckFailureSchema, diff --git a/packages/@n8n/api-types/src/schemas/__tests__/public-api-uploaded-file.schema.test.ts b/packages/@n8n/api-types/src/schemas/__tests__/public-api-uploaded-file.schema.test.ts new file mode 100644 index 00000000000..fa408022daf --- /dev/null +++ b/packages/@n8n/api-types/src/schemas/__tests__/public-api-uploaded-file.schema.test.ts @@ -0,0 +1,43 @@ +import { publicApiUploadedFileSchema } from '../public-api-uploaded-file.schema'; + +describe('publicApiUploadedFileSchema', () => { + const multerFile = { + fieldname: 'package', + originalname: 'export.n8np', + encoding: '7bit', + mimetype: 'application/gzip', + size: 1234, + buffer: Buffer.from('hello'), + destination: '', + filename: '', + path: '', + }; + + it('accepts a multer file object and strips extra keys', () => { + const result = publicApiUploadedFileSchema.safeParse(multerFile); + + assert(result.success, 'Expected multer file object to be valid'); + + expect(result.data).toStrictEqual({ + fieldname: 'package', + originalname: 'export.n8np', + mimetype: 'application/gzip', + size: 1234, + buffer: multerFile.buffer, + }); + }); + + it('produces an invalid_type issue with received undefined for a missing value', () => { + const result = publicApiUploadedFileSchema.safeParse(undefined); + + assert(!result.success, 'Expected undefined to be invalid'); + + expect(result.error.issues[0]).toStrictEqual({ + code: 'invalid_type', + expected: 'object', + message: 'Required', + path: [], + received: 'undefined', + }); + }); +}); diff --git a/packages/@n8n/api-types/src/schemas/public-api-uploaded-file.schema.ts b/packages/@n8n/api-types/src/schemas/public-api-uploaded-file.schema.ts new file mode 100644 index 00000000000..5d10a32525b --- /dev/null +++ b/packages/@n8n/api-types/src/schemas/public-api-uploaded-file.schema.ts @@ -0,0 +1,15 @@ +import '../openapi-extend'; +import { z } from 'zod'; + +/** + * Matches a multer file object where extra keys (e.g. `encoding`) are accepted and stripped. + */ +export const publicApiUploadedFileSchema = z + .object({ + fieldname: z.string(), + originalname: z.string(), + mimetype: z.string(), + size: z.number(), + buffer: z.instanceof(Uint8Array), + }) + .openapi({ type: 'string', format: 'binary' }); diff --git a/packages/@n8n/decorators/src/controller/__tests__/args.test.ts b/packages/@n8n/decorators/src/controller/__tests__/args.test.ts index a825de17316..8b9b1c92359 100644 --- a/packages/@n8n/decorators/src/controller/__tests__/args.test.ts +++ b/packages/@n8n/decorators/src/controller/__tests__/args.test.ts @@ -48,6 +48,56 @@ describe('Args Decorators', () => { }); }); + describe('@Body options', () => { + const resolve = (controllerClass: Controller) => + controllerRegistryMetadata.getRouteMetadata(controllerClass, 'testMethod').args[0]; + + it('sets no `required` or `media` for the bare form', () => { + class TestController { + testMethod(@Body _body: unknown) {} + } + + expect(resolve(TestController as Controller)).toEqual({ type: 'body' }); + }); + + it('sets `required` without `media` for `@Body({ required: true })`', () => { + class TestController { + testMethod(@Body({ required: true }) _body: unknown) {} + } + + expect(resolve(TestController as Controller)).toEqual({ type: 'body', required: true }); + }); + + it('sets `media` for `@Body({ mediaType: "multipart/form-data", uploadLimits })`', () => { + const uploadLimits = () => ({ fileSize: 1024 }); + + class TestController { + testMethod(@Body({ mediaType: 'multipart/form-data', uploadLimits }) _body: unknown) {} + } + + expect(resolve(TestController as Controller)).toEqual({ + type: 'body', + media: { mediaType: 'multipart/form-data', uploadLimits }, + }); + }); + + it('combines `required` with a declared media type', () => { + const uploadLimits = () => ({}); + + class TestController { + testMethod( + @Body({ required: true, mediaType: 'multipart/form-data', uploadLimits }) _body: unknown, + ) {} + } + + expect(resolve(TestController as Controller)).toEqual({ + type: 'body', + required: true, + media: { mediaType: 'multipart/form-data', uploadLimits }, + }); + }); + }); + describe('@Param decorator', () => { it('should set param arg with key at correct parameter index', () => { class TestController { diff --git a/packages/@n8n/decorators/src/controller/args.ts b/packages/@n8n/decorators/src/controller/args.ts index bd037559184..1ee04656f51 100644 --- a/packages/@n8n/decorators/src/controller/args.ts +++ b/packages/@n8n/decorators/src/controller/args.ts @@ -2,7 +2,7 @@ import { Container } from '@n8n/di'; import type { ZodTypeAny } from 'zod'; import { ControllerRegistryMetadata } from './controller-registry-metadata'; -import type { Arg, Controller } from './types'; +import type { Arg, Controller, RequestBodyMedia } from './types'; const ArgDecorator = (arg: Arg): ParameterDecorator => @@ -14,8 +14,15 @@ const ArgDecorator = routeMetadata.args[parameterIndex] = arg; }; -export interface BodyOptions { - required?: boolean; +export type BodyOptions = { required?: boolean } & (RequestBodyMedia | { mediaType?: undefined }); + +function stripExtraPropertiesFromRequestBodyMedia(options: RequestBodyMedia): RequestBodyMedia { + switch (options.mediaType) { + case 'application/json': + return { mediaType: 'application/json' }; + case 'multipart/form-data': + return { mediaType: 'multipart/form-data', uploadLimits: options.uploadLimits }; + } } /** Injects the request body into the handler */ @@ -35,12 +42,18 @@ export function Body( return ArgDecorator({ type: 'body' })(targetOrOptions as object, propertyKey, parameterIndex); } - // Factory form e.g. `@Body() body: MyDto` or `@Body({ required: true }) body: MyDto` + // Factory form e.g. `@Body() body: MyDto` or `@Body({ required: true })` const options = targetOrOptions as BodyOptions | undefined; - return ArgDecorator({ - type: 'body', - ...(options?.required !== undefined && { required: options.required }), - }); + const arg: Arg = { type: 'body' }; + + if (options?.required !== undefined) { + arg.required = options.required; + } + if (options?.mediaType !== undefined) { + arg.media = stripExtraPropertiesFromRequestBodyMedia(options); + } + + return ArgDecorator(arg); } /** Injects the request query into the handler */ diff --git a/packages/@n8n/decorators/src/controller/index.ts b/packages/@n8n/decorators/src/controller/index.ts index 07d22a488cc..75a5f78ad08 100644 --- a/packages/@n8n/decorators/src/controller/index.ts +++ b/packages/@n8n/decorators/src/controller/index.ts @@ -25,6 +25,10 @@ export type { ErrorResponse, HandlerName, Method, + MultipartUploadLimits, + RequestBodyMedia, + RequestBodyMediaOptions, + RequestBodyMediaType, ResponseDtoClass, RouteMetadata, StaticRouterMetadata, diff --git a/packages/@n8n/decorators/src/controller/types.ts b/packages/@n8n/decorators/src/controller/types.ts index ca454054eac..62d514bda35 100644 --- a/packages/@n8n/decorators/src/controller/types.ts +++ b/packages/@n8n/decorators/src/controller/types.ts @@ -24,8 +24,35 @@ export interface ErrorResponse { export type Method = 'get' | 'post' | 'put' | 'patch' | 'delete' | 'head' | 'options'; +/** Multer limits for a `multipart/form-data` `@Body`. Every key matches multer's own `limits` option. */ +export interface MultipartUploadLimits { + fieldNameSize?: number; + fieldSize?: number; + fields?: number; + fileSize?: number; + files?: number; + parts?: number; + headerPairs?: number; +} + +/** + * Accepted options for `@Body` media type. This is where supported media types are first + * registered. + */ +export interface RequestBodyMediaOptions { + 'application/json': object; + 'multipart/form-data': { uploadLimits: () => MultipartUploadLimits }; +} + +export type RequestBodyMediaType = keyof RequestBodyMediaOptions; + +/** One request-body media type, tagged with `mediaType` and carrying that type's own options. */ +export type RequestBodyMedia = { + [K in RequestBodyMediaType]: { mediaType: K } & RequestBodyMediaOptions[K]; +}[RequestBodyMediaType]; + export type Arg = - | { type: 'body'; required?: boolean } + | { type: 'body'; required?: boolean; media?: RequestBodyMedia } | { type: 'query' } | { type: 'param'; key: string; schema?: ZodTypeAny }; diff --git a/packages/cli/src/__tests__/controller.registry.body-media.test.ts b/packages/cli/src/__tests__/controller.registry.body-media.test.ts new file mode 100644 index 00000000000..d480202d4d9 --- /dev/null +++ b/packages/cli/src/__tests__/controller.registry.body-media.test.ts @@ -0,0 +1,54 @@ +import type { GlobalConfig } from '@n8n/config'; +import { Body, ControllerRegistryMetadata, Post, RestController } from '@n8n/decorators'; +import { Container } from '@n8n/di'; +import express from 'express'; +import { mock } from 'vitest-mock-extended'; + +import type { AuthService } from '@/auth/auth.service'; +import { ControllerRegistry } from '@/controller.registry'; +import type { License } from '@/license'; +import type { LastActiveAtService } from '@/services/last-active-at.service'; +import { RateLimitService } from '@/services/rate-limit.service'; + +/** + * Guard test suite ensuring Internal API controllers cannot declare a multipart `@Body`. + * This is only supported for Public API controllers. We may change that in future. + */ +describe('ControllerRegistry - @Body media guard', () => { + const authService = mock(); + const lastActiveAtService = mock(); + + function activate(): void { + authService.createAuthMiddleware.mockImplementation(() => async (_req, _res, next) => next()); + lastActiveAtService.middleware.mockImplementation(async (_req, _res, next) => next()); + + new ControllerRegistry( + mock(), + authService, + mock({ endpoints: { rest: 'rest' } }), + Container.get(ControllerRegistryMetadata), + lastActiveAtService, + new RateLimitService(), + ).activate(express()); + } + + beforeEach(() => { + Container.set(ControllerRegistryMetadata, new ControllerRegistryMetadata()); + }); + + it('throws for an internal route declaring a multipart @Body', () => { + @RestController('/test') + // @ts-expect-error tsc complains about unused class + class TestController { + @Post('/') + method(@Body({ mediaType: 'multipart/form-data', uploadLimits: () => ({}) }) _body: unknown) { + return {}; + } + } + + expect(() => activate()).toThrow( + "TestController.method declares @Body({ mediaType: 'multipart/form-data' }), which only " + + '@PublicApiController routes support.', + ); + }); +}); diff --git a/packages/cli/src/controller.registry.ts b/packages/cli/src/controller.registry.ts index bb332e0f462..1a9ecfdd5c3 100644 --- a/packages/cli/src/controller.registry.ts +++ b/packages/cli/src/controller.registry.ts @@ -29,6 +29,7 @@ import { AbstractServer } from './abstract-server'; import { CorsService } from './services/cors-service'; import { LastActiveAtService } from './services/last-active-at.service'; import { RateLimitService } from './services/rate-limit.service'; +import type { BodyArg } from './public-api/public-api-route-resolver'; @Service() export class ControllerRegistry { @@ -80,6 +81,22 @@ export class ControllerRegistry { // Register regular routes for (const [handlerName, route] of metadata.routes) { + // Guard against internal API using non-JSON body media types. + // We may support non-JSON media types in the future, but for now they are only supported + // on `@PublicApiController` routes. + const nonJsonBodyArg = route.args.find( + (arg): arg is BodyArg => + arg?.type === 'body' && + arg.media !== undefined && + arg.media.mediaType !== 'application/json', + ); + if (nonJsonBodyArg) { + throw new UnexpectedError( + `${controllerClass.name}.${handlerName} declares @Body({ mediaType: ` + + `'${nonJsonBodyArg.media?.mediaType}' }), which only @PublicApiController routes support.`, + ); + } + // Original handler logic for non-router routes const argTypes = Reflect.getMetadata( 'design:paramtypes', diff --git a/packages/cli/src/public-api/__tests__/public-api-controller.registry.test.ts b/packages/cli/src/public-api/__tests__/public-api-controller.registry.test.ts index d9abf174757..33c7a3fd63c 100644 --- a/packages/cli/src/public-api/__tests__/public-api-controller.registry.test.ts +++ b/packages/cli/src/public-api/__tests__/public-api-controller.registry.test.ts @@ -1,4 +1,4 @@ -import { Z } from '@n8n/api-types'; +import { publicApiUploadedFileSchema, Z } from '@n8n/api-types'; import { LicenseState } from '@n8n/backend-common'; import type { EventService } from '@n8n/backend-services'; import { UNLIMITED_LICENSE_QUOTA } from '@n8n/constants'; @@ -10,12 +10,13 @@ import { ControllerRegistryMetadata, Deprecated, Get, + Middleware, Param, Post, ProjectScope, RequiresUserQuota, } from '@n8n/decorators'; -import type { Controller } from '@n8n/decorators'; +import type { Controller, MultipartUploadLimits } from '@n8n/decorators'; import { Container, Service } from '@n8n/di'; import express from 'express'; import request from 'supertest'; @@ -311,61 +312,77 @@ describe('PublicApiControllerRegistry', () => { }); describe('request media type', () => { - function registerOptionalBodyRoute() { - @Service() - class WidgetsPublicController { - @Post('/') - @ApiResponse(200) - method(_req: unknown, _res: unknown, @Body body: OptionalWidgetBodyDto) { - return body; - } - } - markPublicApiController(WidgetsPublicController as Controller, '/widgets'); + const namesNoMediaType: Array<[string, string | undefined]> = [ + ['a request with no Content-Type', undefined], + ['a request with an empty Content-Type', ''], + ['a request with a whitespace Content-Type', ' '], + ]; + + function postWithContentType(header: string | undefined, path: string) { + const pending = request(activate()).post(`/api/v1/widgets/${path}`); + + return header === undefined ? pending : pending.set('Content-Type', header); } - function registerBodyRoute() { + class PackageBodyDto extends Z.class( + { package: publicApiUploadedFileSchema, workflowConflictPolicy: z.string() }, + { strict: true }, + ) {} + + function registerWidgetsController() { @Service() class WidgetsPublicController { - @Post('/') + @Post('/required') @ApiResponse(200) - method(_req: unknown, _res: unknown, @Body body: WidgetBodyDto) { + required(_req: unknown, _res: unknown, @Body body: WidgetBodyDto) { return body; } - } - markPublicApiController(WidgetsPublicController as Controller, '/widgets'); - } - function registerRequiredOptionalBodyRoute() { - @Service() - class WidgetsPublicController { - @Post('/') + @Post('/optional') @ApiResponse(200) - method( + optional(_req: unknown, _res: unknown, @Body body: OptionalWidgetBodyDto) { + return body; + } + + @Post('/required-optional') + @ApiResponse(200) + requiredOptional( _req: unknown, _res: unknown, @Body({ required: true }) body: OptionalWidgetBodyDto, ) { return body; } + + @Post('/upload') + @ApiResponse(200) + upload( + _req: unknown, + _res: unknown, + @Body({ mediaType: 'multipart/form-data', uploadLimits: () => ({}) }) + body: PackageBodyDto, + ) { + return { workflowConflictPolicy: body.workflowConflictPolicy, size: body.package.size }; + } } markPublicApiController(WidgetsPublicController as Controller, '/widgets'); } - it('accepts application/json', async () => { - registerBodyRoute(); + beforeEach(() => { + registerWidgetsController(); + }); + it('accepts application/json', async () => { await request(activate()) - .post('/api/v1/widgets') + .post('/api/v1/widgets/required') .set('Content-Type', 'application/json') .send({ name: 'a' }) .expect(200); }); it('accepts application/json with parameters', async () => { - registerBodyRoute(); - await request(activate()) - .post('/api/v1/widgets') + .post('/api/v1/widgets/required') .set('Content-Type', 'application/json; charset=utf-8') .send({ name: 'a' }) .expect(200); @@ -379,10 +396,8 @@ describe('PublicApiControllerRegistry', () => { ['text/PlAiN; charset=UTF-8', 'text/plain; charset=utf-8'], ['multipart/form-data; boundary=XYZ', 'multipart/form-data'], ])('rejects %s with 415', async (sent, reported) => { - registerBodyRoute(); - const response = await request(activate()) - .post('/api/v1/widgets') + .post('/api/v1/widgets/required') .set('Content-Type', sent) .send('a') .expect(415); @@ -391,10 +406,8 @@ describe('PublicApiControllerRegistry', () => { }); it('rejects a non-JSON media type when no body follows', async () => { - registerBodyRoute(); - const response = await request(activate()) - .post('/api/v1/widgets') + .post('/api/v1/widgets/required') .set('Content-Type', 'application/x-www-form-urlencoded') .expect(415); @@ -403,31 +416,15 @@ describe('PublicApiControllerRegistry', () => { ); }); - const namesNoMediaType: Array<[string, string | undefined]> = [ - ['a request with no Content-Type', undefined], - ['a request with an empty Content-Type', ''], - ['a request with a whitespace Content-Type', ' '], - ]; - - function postWithContentType(header: string | undefined) { - const pending = request(activate()).post('/api/v1/widgets'); - - return header === undefined ? pending : pending.set('Content-Type', header); - } - it.each(namesNoMediaType)( 'accepts %s when every body field is optional', async (_label, header) => { - registerOptionalBodyRoute(); - - await postWithContentType(header).expect(200); + await postWithContentType(header, 'optional').expect(200); }, ); it.each(namesNoMediaType)('rejects %s when the body is required', async (_label, header) => { - registerBodyRoute(); - - const response = await postWithContentType(header).expect(415); + const response = await postWithContentType(header, 'required').expect(415); expect(response.body.message).toBe('unsupported media type undefined'); }); @@ -435,33 +432,178 @@ describe('PublicApiControllerRegistry', () => { it.each(namesNoMediaType)( 'rejects %s when @Body({ required: true }) overrides an otherwise-optional DTO', async (_label, header) => { - registerRequiredOptionalBodyRoute(); - - const response = await postWithContentType(header).expect(415); + const response = await postWithContentType(header, 'required-optional').expect(415); expect(response.body.message).toBe('unsupported media type undefined'); }, ); it('accepts application/json with an empty object when @Body({ required: true }) is set', async () => { - registerRequiredOptionalBodyRoute(); - await request(activate()) - .post('/api/v1/widgets') + .post('/api/v1/widgets/required-optional') .set('Content-Type', 'application/json') .send({}) .expect(200); }); it('accepts application/json carrying an unrelated parameter', async () => { - registerBodyRoute(); - await request(activate()) - .post('/api/v1/widgets') + .post('/api/v1/widgets/required') .set('Content-Type', 'application/json; Foo=BAR') .send({ name: 'a' }) .expect(200); }); + + describe('multipart request bodies', () => { + it('parses a multipart body, merging the uploaded file with text fields', async () => { + const response = await request(activate()) + .post('/api/v1/widgets/upload') + .field('workflowConflictPolicy', 'new-version') + .attach('package', Buffer.from('hello'), 'export.n8np') + .expect(200); + + expect(response.body).toEqual({ workflowConflictPolicy: 'new-version', size: 5 }); + }); + + it('rejects a JSON body with 415 for a multipart-only route', async () => { + const response = await request(activate()) + .post('/api/v1/widgets/upload') + .set('Content-Type', 'application/json') + .send({ workflowConflictPolicy: 'new-version' }) + .expect(415); + + expect(response.body.message).toBe('unsupported media type application/json'); + }); + + it('rejects a request missing the required file with 400', async () => { + const response = await request(activate()) + .post('/api/v1/widgets/upload') + .field('workflowConflictPolicy', 'new-version') + .expect(400); + + expect(response.body.message).toBe("request/body must have required property 'package'"); + }); + + it('rejects an unknown form field with 400', async () => { + const response = await request(activate()) + .post('/api/v1/widgets/upload') + .field('workflowConflictPolicy', 'new-version') + .field('evil', 'x') + .attach('package', Buffer.from('hello'), 'export.n8np') + .expect(400); + + expect(response.body.message).toBe('Unexpected form field "evil"'); + }); + + function registerUploadLimitedRoute(uploadLimits: () => MultipartUploadLimits) { + @Service() + class WidgetsUploadLimitedPublicController { + @Post('/upload-limited') + @ApiResponse(200) + method( + _req: unknown, + _res: unknown, + @Body({ mediaType: 'multipart/form-data', uploadLimits }) body: PackageBodyDto, + ) { + return { workflowConflictPolicy: body.workflowConflictPolicy, size: body.package.size }; + } + } + markPublicApiController(WidgetsUploadLimitedPublicController as Controller, '/widgets'); + } + + it('rejects an oversized file with 413', async () => { + registerUploadLimitedRoute(() => ({ fileSize: 2 })); + + const response = await request(activate()) + .post('/api/v1/widgets/upload-limited') + .field('workflowConflictPolicy', 'new-version') + .attach('package', Buffer.from('hello'), 'export.n8np') + .expect(413); + + expect(response.body.message).toBe('File too large'); + }); + + it('keeps 401 ahead of multipart parsing for an unauthenticated caller', async () => { + authStrategyRegistry.authenticate.mockResolvedValue(false); + registerUploadLimitedRoute(() => ({ fileSize: 2 })); + + // The file is bigger than the configured limit; a 413 here would mean the body was + // parsed before authentication ran. + await request(activate()) + .post('/api/v1/widgets/upload-limited') + .field('workflowConflictPolicy', 'new-version') + .attach('package', Buffer.from('hello'), 'export.n8np') + .expect(401); + }); + + it('keeps the scope check ahead of multipart parsing', async () => { + authStrategyRegistry.authenticate.mockImplementation(async (req: AuthenticatedRequest) => { + req.user = authenticatedUser; + req.tokenGrant = { scopes: [], apiKeyScopes: [], subject: authenticatedUser }; + return true; + }); + + @Service() + class WidgetsUploadScopedPublicController { + @Post('/upload-scoped') + @ApiResponse(200) + @ApiKeyScope('workflow:import') + method( + _req: unknown, + _res: unknown, + @Body({ mediaType: 'multipart/form-data', uploadLimits: () => ({ fileSize: 2 }) }) + body: PackageBodyDto, + ) { + return body; + } + } + markPublicApiController(WidgetsUploadScopedPublicController as Controller, '/widgets'); + + // Same oversized file as above: a 413 here would mean the body was parsed before the + // scope check ran. + const response = await request(activate()) + .post('/api/v1/widgets/upload-scoped') + .field('workflowConflictPolicy', 'new-version') + .attach('package', Buffer.from('hello'), 'export.n8np') + .expect(403); + + expect(response.body).toEqual({ message: 'Forbidden' }); + }); + + it('hands the parsed body to a controller middleware, not only to the handler', async () => { + const middlewareSaw: unknown[] = []; + + @Service() + class WidgetsUploadMiddlewarePublicController { + @Middleware() + capture(req: express.Request, _res: express.Response, next: express.NextFunction) { + middlewareSaw.push(req.body); + next(); + } + + @Post('/upload-middleware') + @ApiResponse(200) + method( + _req: unknown, + _res: unknown, + @Body({ mediaType: 'multipart/form-data', uploadLimits: () => ({}) }) + body: PackageBodyDto, + ) { + return { ok: true, size: body.package.size }; + } + } + markPublicApiController(WidgetsUploadMiddlewarePublicController as Controller, '/widgets'); + + await request(activate()) + .post('/api/v1/widgets/upload-middleware') + .field('workflowConflictPolicy', 'new-version') + .attach('package', Buffer.from('hello'), 'export.n8np') + .expect(200); + + expect(middlewareSaw).toHaveLength(1); + expect(middlewareSaw[0]).toMatchObject({ workflowConflictPolicy: 'new-version' }); + }); + }); }); describe('@RequiresUserQuota', () => { diff --git a/packages/cli/src/public-api/__tests__/public-api-route-resolver.test.ts b/packages/cli/src/public-api/__tests__/public-api-route-resolver.test.ts index 2ee8db58bb9..65ca0a7b1f9 100644 --- a/packages/cli/src/public-api/__tests__/public-api-route-resolver.test.ts +++ b/packages/cli/src/public-api/__tests__/public-api-route-resolver.test.ts @@ -24,6 +24,8 @@ import { WidgetQueryDto, WidgetResponseDto, } from '@/public-api/__tests__/public-api-controller-test-utils'; +import { JSON_REQUEST_BODY_MEDIA } from '@/public-api/media-types/request-body'; +import { jsonRequestBody } from '@/public-api/media-types/request-body/json.request-body'; import { apiKeyScopesSatisfy, @@ -61,7 +63,14 @@ describe('public-api-route-resolver', () => { method(@Body _body: WidgetBodyDto) {} } - expect(resolve(TestController as Controller)).toEqual([{ type: 'body', dto: WidgetBodyDto }]); + expect(resolve(TestController as Controller)).toEqual([ + { + type: 'body', + dto: WidgetBodyDto, + media: JSON_REQUEST_BODY_MEDIA, + handler: jsonRequestBody, + }, + ]); }); it('resolves a query arg to its Zod DTO via design:paramtypes reflection', () => { @@ -80,7 +89,12 @@ describe('public-api-route-resolver', () => { } expect(resolve(TestController as Controller)).toEqual([ - { type: 'body', dto: WidgetArrayResponseDto }, + { + type: 'body', + dto: WidgetArrayResponseDto, + media: JSON_REQUEST_BODY_MEDIA, + handler: jsonRequestBody, + }, ]); }); @@ -95,7 +109,12 @@ describe('public-api-route-resolver', () => { expect(resolve(TestController as Controller)).toEqual([ { type: 'param', key: 'id' }, - { type: 'body', dto: WidgetBodyDto }, + { + type: 'body', + dto: WidgetBodyDto, + media: JSON_REQUEST_BODY_MEDIA, + handler: jsonRequestBody, + }, { type: 'query', dto: WidgetQueryDto }, ]); }); diff --git a/packages/cli/src/public-api/media-types/__tests__/content-type.test.ts b/packages/cli/src/public-api/media-types/__tests__/content-type.test.ts new file mode 100644 index 00000000000..bebbdd26316 --- /dev/null +++ b/packages/cli/src/public-api/media-types/__tests__/content-type.test.ts @@ -0,0 +1,25 @@ +import { UnsupportedMediaTypeError } from '@n8n/errors'; + +import { assertContentType } from '../content-type'; + +describe('assertContentType', () => { + it('ignores parameters and casing when matching', () => { + expect( + assertContentType({ + header: 'Application/JSON; charset=utf-8', + expected: 'application/json', + bodyRequired: true, + }), + ).toBe(true); + }); + + it('still throws for a mismatched media type when the body is optional', () => { + expect(() => + assertContentType({ + header: 'text/plain', + expected: 'multipart/form-data', + bodyRequired: false, + }), + ).toThrow(UnsupportedMediaTypeError); + }); +}); diff --git a/packages/cli/src/public-api/public-api-media-type.ts b/packages/cli/src/public-api/media-types/content-type.ts similarity index 59% rename from packages/cli/src/public-api/public-api-media-type.ts rename to packages/cli/src/public-api/media-types/content-type.ts index 4ed5fa5f4dd..10162bb1045 100644 --- a/packages/cli/src/public-api/public-api-media-type.ts +++ b/packages/cli/src/public-api/media-types/content-type.ts @@ -1,7 +1,6 @@ +import type { RequestBodyMediaType } from '@n8n/decorators'; import { UnsupportedMediaTypeError } from '@n8n/errors'; -const JSON_MEDIA_TYPE = 'application/json'; - /** * The media type, plus the string the legacy validator reports: lower-cased, parameters sorted by * name, `boundary` left out. @@ -11,12 +10,18 @@ function readMediaType(header: string): { mediaType: string; reported: string } const mediaType = rawMediaType.trim().toLowerCase(); const parameters = new Map(); + // Parameter sorting and returning of the reported string is kept only for parity with the + // EOV handler — we may be able to remove and simplify this method in future. for (const part of parameterParts) { const separator = part.indexOf('='); - if (separator === -1) continue; + if (separator === -1) { + continue; + } const name = part.slice(0, separator).trim().toLowerCase(); - if (name === 'boundary') continue; + if (name === 'boundary') { + continue; + } const value = part.slice(separator + 1); parameters.set(name, name === 'charset' ? value.toLowerCase() : value); @@ -30,19 +35,29 @@ function readMediaType(header: string): { mediaType: string; reported: string } } /** - * The legacy validator accepted only JSON. It reported a header that names no media type — absent, - * empty, or whitespace — as the literal `undefined`, and rejected it only when the body was - * required. Migrated routes keep both behaviours and the messages that came with them. + * Check a request's `Content-Type` against the media type `@Body` declares. */ -export function assertJsonContentType(header: string | undefined, bodyRequired: boolean): void { +export function assertContentType({ + header, + expected, + bodyRequired, +}: { + header: string | undefined; + expected: RequestBodyMediaType; + bodyRequired: boolean; +}): boolean { const { mediaType, reported } = readMediaType(header ?? ''); if (mediaType === '') { - if (bodyRequired) throw new UnsupportedMediaTypeError('unsupported media type undefined'); - return; + if (bodyRequired) { + throw new UnsupportedMediaTypeError('unsupported media type undefined'); + } + return false; } - if (mediaType !== JSON_MEDIA_TYPE) { + if (mediaType !== expected) { throw new UnsupportedMediaTypeError(`unsupported media type ${reported}`); } + + return true; } diff --git a/packages/cli/src/public-api/media-types/request-body/__tests__/json.request-body.test.ts b/packages/cli/src/public-api/media-types/request-body/__tests__/json.request-body.test.ts new file mode 100644 index 00000000000..763d56b46ad --- /dev/null +++ b/packages/cli/src/public-api/media-types/request-body/__tests__/json.request-body.test.ts @@ -0,0 +1,23 @@ +import type { Request } from 'express'; +import { z, ZodError } from 'zod'; + +import { jsonRequestBody } from '../json.request-body'; + +describe('jsonRequestBody', () => { + it('reads req.body unchanged', () => { + const req = { body: { a: 1 } } as unknown as Request; + + expect(jsonRequestBody.readInput(req)).toBe(req.body); + }); + + it('formats a validation error the same way the shared field-level formatter does', () => { + try { + z.object({ name: z.string() }).parse({}); + } catch (error) { + assert(error instanceof ZodError, 'expected a ZodError'); + expect(jsonRequestBody.formatValidationError(error)).toBe( + "request/body must have required property 'name'", + ); + } + }); +}); diff --git a/packages/cli/src/public-api/media-types/request-body/__tests__/multipart.request-body.test.ts b/packages/cli/src/public-api/media-types/request-body/__tests__/multipart.request-body.test.ts new file mode 100644 index 00000000000..0cc7c1e8206 --- /dev/null +++ b/packages/cli/src/public-api/media-types/request-body/__tests__/multipart.request-body.test.ts @@ -0,0 +1,155 @@ +import { BadRequestError, ContentTooLargeError, InternalServerError } from '@n8n/errors'; +import type { Request } from 'express'; +import { MulterError } from 'multer'; +import { mock } from 'vitest-mock-extended'; +import { z } from 'zod'; + +import { multipartRequestBody, toPublicApiError } from '../multipart.request-body'; + +function fakeFile(fieldname: string): Express.Multer.File { + return mock({ fieldname }); +} + +describe('toPublicApiError', () => { + it.each([ + ['LIMIT_FILE_SIZE', 'File too large'], + ['LIMIT_FILE_COUNT', 'Too many files'], + ['LIMIT_PART_COUNT', 'Too many parts'], + ] as const)('maps %s to a 413 ContentTooLargeError', (code, message) => { + const error = toPublicApiError(new MulterError(code)); + + expect(error).toBeInstanceOf(ContentTooLargeError); + expect(error.message).toBe(message); + }); + + it('maps LIMIT_UNEXPECTED_FILE to a 500 InternalServerError', () => { + const error = toPublicApiError(new MulterError('LIMIT_UNEXPECTED_FILE')); + + expect(error).toBeInstanceOf(InternalServerError); + expect(error.message).toBe('Unexpected field'); + }); + + it('maps any other multer error to a 400 BadRequestError', () => { + const error = toPublicApiError(new MulterError('LIMIT_FIELD_VALUE')); + + expect(error).toBeInstanceOf(BadRequestError); + expect(error.message).toBe('Field value too long'); + }); + + it('maps a missing-boundary error to the legacy 400 message', () => { + const error = toPublicApiError(new Error('Multipart: Boundary not found')); + + expect(error).toBeInstanceOf(BadRequestError); + expect(error.message).toBe('multipart file(s) required'); + }); + + it('wraps an unrecognized error in an unmasked 500 InternalServerError', () => { + const error = toPublicApiError(new Error('Unexpected end of form')); + + expect(error).toBeInstanceOf(InternalServerError); + expect(error.message).toBe('Unexpected end of form'); + }); + + it('stringifies a thrown non-Error value', () => { + const error = toPublicApiError('disk exploded'); + + expect(error).toBeInstanceOf(InternalServerError); + expect(error.message).toBe('disk exploded'); + }); +}); + +describe('multipartRequestBody', () => { + describe('readInput', () => { + it('merges text fields with files', () => { + const req = { + body: { workflowConflictPolicy: 'new-version' }, + files: [fakeFile('package')], + } as unknown as Request; + + const input = multipartRequestBody.readInput(req) as Record; + + expect(input.workflowConflictPolicy).toBe('new-version'); + expect(input.package).toMatchObject({ fieldname: 'package' }); + }); + + it('collects several files under the same field name into an array', () => { + const req = { + body: {}, + files: [fakeFile('selectedWorkflowIds'), fakeFile('selectedWorkflowIds')], + } as unknown as Request; + + const input = multipartRequestBody.readInput(req) as Record; + + expect(Array.isArray(input.selectedWorkflowIds)).toBe(true); + expect((input.selectedWorkflowIds as unknown[]).length).toBe(2); + }); + + it('merges a files map (req.files as a Record) the same way as an array', () => { + const req = { + body: {}, + files: { package: [fakeFile('package')] }, + } as unknown as Request; + + const input = multipartRequestBody.readInput(req) as Record; + + expect(input.package).toMatchObject({ fieldname: 'package' }); + }); + + it('handles a request with no files at all', () => { + const req = { body: { a: '1' }, files: undefined } as unknown as Request; + + expect(multipartRequestBody.readInput(req)).toEqual({ a: '1' }); + }); + + it('turns a part literally named __proto__ into an own key rather than the object prototype', () => { + // Matches how multer actually builds `req.body` (`Object.create(null)`, then an assignment + // per field) - a plain `{ __proto__: 'evil' }` literal wouldn't reproduce the same hazard, + // since object-literal syntax special-cases that key as a prototype-setter instead of a key. + const body: Record = Object.create(null) as Record; + body.__proto__ = 'evil'; + const req = { body, files: undefined } as unknown as Request; + + const input = multipartRequestBody.readInput(req) as Record; + + expect(Object.getPrototypeOf(input)).toBeNull(); + expect(Object.prototype.hasOwnProperty.call(input, '__proto__')).toBe(true); + expect(input.__proto__).toBe('evil'); + }); + + it('turns a file part named __proto__ into an own key as well', () => { + const req = { + body: {}, + files: [fakeFile('__proto__')], + } as unknown as Request; + + const input = multipartRequestBody.readInput(req) as Record; + + expect(Object.prototype.hasOwnProperty.call(input, '__proto__')).toBe(true); + expect(input.__proto__).toMatchObject({ fieldname: '__proto__' }); + }); + }); + + describe('formatValidationError', () => { + it('reports an unrecognized key as an unexpected form field', () => { + const schema = z.object({ workflowConflictPolicy: z.string() }).strict(); + const result = schema.safeParse({ workflowConflictPolicy: 'new-version', evil: 'x' }); + + assert(!result.success, 'expected an unrecognized-key error'); + + expect(multipartRequestBody.formatValidationError(result.error)).toBe( + 'Unexpected form field "evil"', + ); + }); + + it('falls back to the default formatter for any other issue', () => { + const schema = z.object({ workflowConflictPolicy: z.string() }).strict(); + const result = schema.safeParse({}); + + assert(!result.success, 'expected a required-property error'); + + expect(multipartRequestBody.formatValidationError(result.error)).toBe( + "request/body must have required property 'workflowConflictPolicy'", + ); + }); + }); +}); diff --git a/packages/cli/src/public-api/media-types/request-body/index.ts b/packages/cli/src/public-api/media-types/request-body/index.ts new file mode 100644 index 00000000000..012b756b3c7 --- /dev/null +++ b/packages/cli/src/public-api/media-types/request-body/index.ts @@ -0,0 +1,18 @@ +import type { RequestBodyMedia, RequestBodyMediaType } from '@n8n/decorators'; + +import { jsonRequestBody } from './json.request-body'; +import { multipartRequestBody } from './multipart.request-body'; +import type { RequestBodyHandler } from './types'; + +export type { RequestBodyHandler } from './types'; + +/** + * One handler per request-body media type `@Body` can declare. + */ +export const REQUEST_BODY_HANDLERS: Record = { + 'application/json': jsonRequestBody, + 'multipart/form-data': multipartRequestBody, +}; + +/** The media type every route had before `@Body` could declare one - the default for a route that doesn't. */ +export const JSON_REQUEST_BODY_MEDIA: RequestBodyMedia = { mediaType: 'application/json' }; diff --git a/packages/cli/src/public-api/media-types/request-body/json.request-body.ts b/packages/cli/src/public-api/media-types/request-body/json.request-body.ts new file mode 100644 index 00000000000..e7d888184ae --- /dev/null +++ b/packages/cli/src/public-api/media-types/request-body/json.request-body.ts @@ -0,0 +1,22 @@ +import type { ZodError } from 'zod'; + +import { formatValidationError } from '@/public-api/public-api-validation-error'; + +import type { RequestBodyHandler } from './types'; + +/** + * The default request body handler if a media type is not specified on a route in `@Body`. + */ +export const jsonRequestBody: RequestBodyHandler = { + mediaType: 'application/json', + errorStatuses: [415], + discoverable: true, + + readInput(req) { + return req.body; + }, + + formatValidationError(error: ZodError) { + return formatValidationError('body', error); + }, +}; diff --git a/packages/cli/src/public-api/media-types/request-body/multipart-parser.ts b/packages/cli/src/public-api/media-types/request-body/multipart-parser.ts new file mode 100644 index 00000000000..02f4b2775ba --- /dev/null +++ b/packages/cli/src/public-api/media-types/request-body/multipart-parser.ts @@ -0,0 +1,11 @@ +import type { MultipartUploadLimits } from '@n8n/decorators'; +import type { RequestHandler } from 'express'; + +/** + * Builds the multer middleware that parses a `multipart/form-data` body, mirroring + * express-openapi-validator's `fileUploader`. + */ +export async function loadMultipartParser(limits: MultipartUploadLimits): Promise { + const { default: multer } = await import('multer'); + return multer({ storage: multer.memoryStorage(), limits }).any(); +} diff --git a/packages/cli/src/public-api/media-types/request-body/multipart.request-body.ts b/packages/cli/src/public-api/media-types/request-body/multipart.request-body.ts new file mode 100644 index 00000000000..5ba23c812ef --- /dev/null +++ b/packages/cli/src/public-api/media-types/request-body/multipart.request-body.ts @@ -0,0 +1,122 @@ +import { + BadRequestError, + ContentTooLargeError, + InternalServerError, + UnexpectedError, +} from '@n8n/errors'; +import type { Request } from 'express'; +import type { MulterError } from 'multer'; +import type { ZodError } from 'zod'; + +import { formatValidationError } from '@/public-api/public-api-validation-error'; + +import { loadMultipartParser } from './multipart-parser'; +import type { RequestBodyHandler } from './types'; + +// Same regexes express-openapi-validator 5.5.3 used in legacy EOV handlers. +const PAYLOAD_TOO_BIG_CODE = /LIMIT_(FILE|PART)_(SIZE|COUNT)/; +const UNEXPECTED_FILE_CODE = /LIMIT_UNEXPECTED_FILE/; +const MISSING_BOUNDARY = /Multipart: Boundary not found/i; + +function isMulterError(error: unknown): error is MulterError { + return error instanceof Error && error.name === 'MulterError'; +} + +/** + * Maps a multer parsing error to a serializable public API error matching the legacy EOV handlers. + */ +export function toPublicApiError(error: unknown): Error { + if (isMulterError(error)) { + if (PAYLOAD_TOO_BIG_CODE.test(error.code)) { + return new ContentTooLargeError(error.message); + } + if (UNEXPECTED_FILE_CODE.test(error.code)) { + return new InternalServerError(error.message); + } + return new BadRequestError(error.message); + } + + const message = error instanceof Error ? error.message : String(error); + if (MISSING_BOUNDARY.test(message)) { + return new BadRequestError('multipart file(s) required'); + } + + return new InternalServerError(message); +} + +function listFiles(files: Request['files']): Express.Multer.File[] { + if (!files) return []; + return Array.isArray(files) ? files : Object.values(files).flat(); +} + +/** + * Merges a multipart body's text fields and uploaded files into one object for the route's `@Body` + * DTO to validate. A field with several files under the same name becomes an array; a file wins over + * a text field sent under the same name. + * + * Built with `Object.create(null)` so a field named `__proto__` becomes an enumerable key instead of + * reassigning the object's prototype - the strict DTO then rejects it, instead of it silently vanishing. + */ +function mergeMultipartInput(req: Request): Record { + const input: Record = Object.create(null); + + for (const [key, value] of Object.entries(req.body ?? {})) { + input[key] = value; + } + + const filesByField = new Map(); + + for (const file of listFiles(req.files)) { + const existing = filesByField.get(file.fieldname); + if (existing) { + existing.push(file); + } else { + filesByField.set(file.fieldname, [file]); + } + } + + for (const [fieldname, files] of filesByField) { + input[fieldname] = files.length === 1 ? files[0] : files; + } + + return input; +} + +export const multipartRequestBody: RequestBodyHandler = { + mediaType: 'multipart/form-data', + // 500 is reachable via `toPublicApiError` + errorStatuses: [413, 415, 500], + discoverable: false, + + async parseBody(media, req, res) { + if (media.mediaType !== 'multipart/form-data') { + throw new UnexpectedError( + 'multipartRequestBody.parseBody called with a non-multipart media type', + ); + } + + const parse = await loadMultipartParser(media.uploadLimits()); + + await new Promise((resolve, reject) => { + void parse(req, res, (error?: unknown) => { + if (error) { + reject(toPublicApiError(error)); + return; + } + resolve(); + }); + }); + }, + + readInput(req) { + return mergeMultipartInput(req); + }, + + formatValidationError(error: ZodError) { + const [issue] = error.errors; + if (issue?.code === 'unrecognized_keys' && issue.keys.length > 0) { + return `Unexpected form field "${issue.keys[0]}"`; + } + return formatValidationError('body', error); + }, +}; diff --git a/packages/cli/src/public-api/media-types/request-body/types.ts b/packages/cli/src/public-api/media-types/request-body/types.ts new file mode 100644 index 00000000000..4f06d246e10 --- /dev/null +++ b/packages/cli/src/public-api/media-types/request-body/types.ts @@ -0,0 +1,18 @@ +import type { RequestBodyMedia, RequestBodyMediaType } from '@n8n/decorators'; +import type { Request, Response } from 'express'; +import type { ZodError } from 'zod'; + +export interface RequestBodyHandler { + /** The OpenAPI content key, and the `Content-Type` a route declaring this handler must match. */ + readonly mediaType: RequestBodyMediaType; + /** Error statuses this media type can cause; documented on every route that declares it. */ + readonly errorStatuses: readonly number[]; + /** Whether `/discover` shows a request schema for a route declaring this media type. */ + readonly discoverable: boolean; + /** Parses the body into whatever shape `readInput` later reads back. **/ + parseBody?(media: RequestBodyMedia, req: Request, res: Response): Promise; + /** The value the route's `@Body` DTO validates. */ + readInput(req: Request): unknown; + /** The public message for a value that failed the `@Body` DTO's `.safeParse()`. */ + formatValidationError(error: ZodError): string; +} diff --git a/packages/cli/src/public-api/public-api-controller.registry.ts b/packages/cli/src/public-api/public-api-controller.registry.ts index b341d81b203..6c31c3a8114 100644 --- a/packages/cli/src/public-api/public-api-controller.registry.ts +++ b/packages/cli/src/public-api/public-api-controller.registry.ts @@ -4,7 +4,12 @@ import type { BooleanLicenseFeature } from '@n8n/constants'; import { UNLIMITED_LICENSE_QUOTA } from '@n8n/constants'; import type { AuthenticatedRequest } from '@n8n/db'; import { ControllerRegistryMetadata } from '@n8n/decorators'; -import type { AccessScope, ApiKeyScopeRequirement, Controller } from '@n8n/decorators'; +import type { + AccessScope, + ApiKeyScopeRequirement, + Controller, + RequestBodyMedia, +} from '@n8n/decorators'; import { Container, Service } from '@n8n/di'; import type { Request, RequestHandler, Response, Router } from 'express'; import { Router as createRouter } from 'express'; @@ -12,11 +17,12 @@ import { z } from 'zod'; import type { ZodTypeAny } from 'zod'; import { FeatureNotLicensedError } from '@/errors/feature-not-licensed.error'; -import { BadRequestError } from '@n8n/errors'; +import { BadRequestError, UnsupportedMediaTypeError } from '@n8n/errors'; import { License } from '@/license'; +import { assertContentType } from '@/public-api/media-types/content-type'; +import type { RequestBodyHandler } from '@/public-api/media-types/request-body'; import { userHasScopes } from '@/permissions.ee/check-access'; import { USER_QUOTA_FORBIDDEN_MESSAGE } from '@/public-api/constants'; -import { assertJsonContentType } from '@/public-api/public-api-media-type'; import type { ValidatedParamArg } from '@/public-api/public-api-route-resolver'; import { apiKeyScopesSatisfy, @@ -86,25 +92,31 @@ export class PublicApiControllerRegistry { ); const bodyArg = findBodyArg(resolvedArgs); - const bodyDto = bodyArg?.dto; - const bodyRequired = bodyDto ? (bodyArg?.required ?? isRequestBodyRequired(bodyDto)) : false; + const bodyRequired = bodyArg + ? (bodyArg.required ?? isRequestBodyRequired(bodyArg.dto)) + : false; const handler = async (req: Request, res: Response) => { - if (bodyDto) assertJsonContentType(req.headers['content-type'], bodyRequired); - const args: unknown[] = [req, res]; for (const arg of resolvedArgs) { if (arg.type === 'param') { args.push( arg.schema ? parsePathParam(arg.key, arg.schema, req.params) : req.params[arg.key], ); + continue; + } + + const isBodyArg = arg.type === 'body' && bodyArg; + const input = isBodyArg ? bodyArg.handler.readInput(req) : req[arg.type]; + + const output = arg.dto.safeParse(input); + if (output.success) { + args.push(output.data); } else { - const output = arg.dto.safeParse(req[arg.type]); - if (output.success) { - args.push(output.data); - } else { - throw new BadRequestError(formatValidationError(arg.type, output.error)); - } + const message = isBodyArg + ? bodyArg.handler.formatValidationError(output.error) + : formatValidationError(arg.type, output.error); + throw new BadRequestError(message); } } @@ -154,6 +166,11 @@ export class PublicApiControllerRegistry { middlewares.push(this.createUserQuotaMiddleware()); } + // After every access gate, the body's Content-Type is checked and then parsed. + if (bodyArg) { + middlewares.push(this.createBodyMiddleware(bodyArg.media, bodyRequired, bodyArg.handler)); + } + middlewares.push(...controllerMiddlewares, ...(route.middlewares ?? [])); const finalHandler: RequestHandler = async (req, res, next) => { @@ -237,6 +254,43 @@ export class PublicApiControllerRegistry { }; } + private createBodyMiddleware( + media: RequestBodyMedia, + bodyRequired: boolean, + handler: RequestBodyHandler, + ): RequestHandler { + return async (req, res, next) => { + let matched: boolean; + try { + matched = assertContentType({ + header: req.headers['content-type'], + expected: media.mediaType, + bodyRequired, + }); + } catch (error) { + sendPublicApiErrorResponse( + res, + error instanceof UnsupportedMediaTypeError ? error : new Error(String(error)), + ); + return; + } + + if (!matched || !handler.parseBody) { + next(); + return; + } + + try { + await handler.parseBody(media, req, res); + } catch (error) { + sendPublicApiErrorResponse(res, error instanceof Error ? error : new Error(String(error))); + return; + } + + next(); + }; + } + /** * Rejects a path param that breaks its `@Param` schema, ahead of the scope middlewares. The * handler parses the params again to bind its arguments; by then they are known to be valid. diff --git a/packages/cli/src/public-api/public-api-route-resolver.ts b/packages/cli/src/public-api/public-api-route-resolver.ts index 960d35100dc..0053e5b4ed2 100644 --- a/packages/cli/src/public-api/public-api-route-resolver.ts +++ b/packages/cli/src/public-api/public-api-route-resolver.ts @@ -7,6 +7,7 @@ import type { ErrorResponse, HandlerName, Method, + RequestBodyMedia, ResponseDtoClass, SuccessStatus, } from '@n8n/decorators'; @@ -16,6 +17,12 @@ import type { ApiKeyScope } from '@n8n/permissions'; import { UnexpectedError } from 'n8n-workflow'; import type { ZodTypeAny } from 'zod'; +import type { RequestBodyHandler } from '@/public-api/media-types/request-body'; +import { + JSON_REQUEST_BODY_MEDIA, + REQUEST_BODY_HANDLERS, +} from '@/public-api/media-types/request-body'; + export const HTTP_METHODS = [ 'get', 'post', @@ -29,7 +36,15 @@ export const HTTP_METHODS = [ export type HttpMethod = (typeof HTTP_METHODS)[number]; export type ParamArg = { type: 'param'; key: string; schema?: ZodTypeAny }; -type BodyArg = { type: 'body'; dto: ZodClass; required?: boolean }; +export type BodyArg = { + type: 'body'; + dto: ZodClass; + required?: boolean; + /** Defaults to JSON when `@Body` declares no media type. */ + media: RequestBodyMedia; + /** Resolved once here from `media`, so callers don't each look it up in `REQUEST_BODY_HANDLERS` themselves. */ + handler: RequestBodyHandler; +}; type QueryArg = { type: 'query'; dto: ZodClass }; /** A `ParamArg` whose `@Param` declared a schema, so its value is validated before the handler. */ @@ -62,6 +77,10 @@ export interface ResolvedPublicApiRoute { requestBodyDto?: ZodClass; /** Explicit `@Body({ required })` override; falls back to `isRequestBodyRequired` when unset. */ requestBodyRequired?: boolean; + /** The request body's media type; defaults to JSON when `@Body` declares none. Set only when `requestBodyDto` is. */ + requestBodyMedia?: RequestBodyMedia; + /** The request body's media-type handler, resolved from `requestBodyMedia`. Set only when `requestBodyDto` is. */ + requestBodyHandler?: RequestBodyHandler; requestQueryDto?: ZodClass; responseDto?: ResponseDtoClass; /** Success status declared via `@ApiResponse` - always present, see `resolveSuccessStatus`. */ @@ -126,10 +145,13 @@ export function resolveRouteArgs( } if (arg.type === 'body') { + const media = arg.media ?? JSON_REQUEST_BODY_MEDIA; resolved.push({ type: 'body', dto: paramType, ...(arg.required !== undefined && { required: arg.required }), + media, + handler: REQUEST_BODY_HANDLERS[media.mediaType], }); continue; } @@ -254,6 +276,8 @@ export function resolvePublicApiRoutes(): ResolvedPublicApiRoute[] { const requestBodyArg = findBodyArg(args); const requestBodyDto = requestBodyArg?.dto; const requestBodyRequired = requestBodyArg?.required; + const requestBodyMedia = requestBodyArg?.media; + const requestBodyHandler = requestBodyArg?.handler; const requestQueryDto = findQueryArg(args)?.dto; const joined = `${prefix}${route.path}`.replace(/\/+/g, '/'); @@ -268,6 +292,8 @@ export function resolvePublicApiRoutes(): ResolvedPublicApiRoute[] { args, requestBodyDto, requestBodyRequired, + requestBodyMedia, + requestBodyHandler, requestQueryDto, responseDto: route.responseDto, successStatus: resolveSuccessStatus(controllerClass.name, handlerName, route.successStatus), diff --git a/packages/cli/src/public-api/v1/handlers/discover/discover.service.ts b/packages/cli/src/public-api/v1/handlers/discover/discover.service.ts index c36d371e9db..08bb111e383 100644 --- a/packages/cli/src/public-api/v1/handlers/discover/discover.service.ts +++ b/packages/cli/src/public-api/v1/handlers/discover/discover.service.ts @@ -139,7 +139,11 @@ function buildDecoratorEndpoints(): EndpointInfo[] { operationId: route.handlerName, tag: route.tags?.[0] ?? 'Other', scope: route.apiKeyScope ?? null, - requestSchema: buildRequestBodyJsonSchema(route), + // A non-JSON body shows no request schema (`discoverable: false` on its handler), so a + // client doesn't assume one. + requestSchema: route.requestBodyHandler?.discoverable + ? buildRequestBodyJsonSchema(route) + : undefined, })); } diff --git a/packages/cli/src/public-api/v1/openapi-gen/__tests__/decorator-routes.test.ts b/packages/cli/src/public-api/v1/openapi-gen/__tests__/decorator-routes.test.ts index 91d62eb2ec1..bbd8467932c 100644 --- a/packages/cli/src/public-api/v1/openapi-gen/__tests__/decorator-routes.test.ts +++ b/packages/cli/src/public-api/v1/openapi-gen/__tests__/decorator-routes.test.ts @@ -260,6 +260,38 @@ describe('getDecoratorGeneratedOperations', () => { }); }); + it('documents a multipart body under its own content key, with 413, 415, and 500 responses', () => { + class WidgetsPublicController { + @Post('/') + @ApiResponse(200) + method( + _req: unknown, + _res: unknown, + @Body({ mediaType: 'multipart/form-data', uploadLimits: () => ({}) }) body: WidgetBodyDto, + ) { + return body; + } + } + markPublicApiController(WidgetsPublicController as Controller, '/widgets'); + + const [operation] = getDecoratorGeneratedOperations(); + + expect(operation.config.request?.body?.content).toEqual({ + 'multipart/form-data': { schema: WidgetBodyDto.schema }, + }); + expect(operation.config.responses[413]).toEqual({ + $ref: '../../../../shared/spec/responses/contentTooLarge.yml', + }); + expect(operation.config.responses[415]).toEqual({ + $ref: '../../../../shared/spec/responses/unsupportedMediaType.yml', + }); + // Multer's parsing errors can themselves 500 (an unmasked parse failure, or + // LIMIT_UNEXPECTED_FILE) - see multipart.request-body.ts's `toPublicApiError`. + expect(operation.config.responses[500]).toEqual({ + $ref: '../../../../shared/spec/responses/internalServerError.yml', + }); + }); + it('refs the shared response file for an error status declared without a body DTO', () => { class WidgetsPublicController { @Get('/') diff --git a/packages/cli/src/public-api/v1/openapi-gen/__tests__/request-body-schema-parity.test.ts b/packages/cli/src/public-api/v1/openapi-gen/__tests__/request-body-schema-parity.test.ts index 8dc42161f09..91e15acb3a9 100644 --- a/packages/cli/src/public-api/v1/openapi-gen/__tests__/request-body-schema-parity.test.ts +++ b/packages/cli/src/public-api/v1/openapi-gen/__tests__/request-body-schema-parity.test.ts @@ -8,7 +8,8 @@ import { buildRequestBodyJsonSchema, getDecoratorGeneratedOperations } from '../ /** * The build writes a request body schema into the spec. `/discover` builds the same schema at - * runtime, through `buildRequestBodyJsonSchema`. Nothing else checks that the two agree. + * runtime, through `buildRequestBodyJsonSchema`, for a route whose media type is `discoverable` + * (JSON; not multipart). Nothing else checks that the two agree. */ const V1_DIR = path.resolve(__dirname, '../..'); @@ -21,7 +22,7 @@ const SPEC_FILE_BY_HANDLER = new Map( ]), ); -function schemaInSpecFile(handlerName: string): unknown { +function schemaInSpecFile(handlerName: string, mediaType: string): unknown { const specFile = SPEC_FILE_BY_HANDLER.get(handlerName); if (!specFile) throw new Error(`The build generated no spec file for ${handlerName}`); @@ -29,13 +30,19 @@ function schemaInSpecFile(handlerName: string): unknown { requestBody?: { content?: Record }; }; - return spec.requestBody?.content?.['application/json']?.schema; + return spec.requestBody?.content?.[mediaType]?.schema; } describe('buildRequestBodyJsonSchema', () => { - const routesWithBody = resolvePublicApiRoutes().filter((route) => route.requestBodyDto); + const discoverableRoutesWithBody = resolvePublicApiRoutes().filter( + (route) => route.requestBodyDto && route.requestBodyHandler?.discoverable, + ); - it.each(routesWithBody)('$handlerName matches its committed spec file', (route) => { - expect(buildRequestBodyJsonSchema(route)).toEqual(schemaInSpecFile(route.handlerName)); + it.each(discoverableRoutesWithBody)('$handlerName matches its committed spec file', (route) => { + assert(route.requestBodyMedia, 'expected a route with a body to have a resolved media type'); + + expect(buildRequestBodyJsonSchema(route)).toEqual( + schemaInSpecFile(route.handlerName, route.requestBodyMedia.mediaType), + ); }); }); 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 958f06b8a64..2fb89547979 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 = { 403: { $ref: '../../../../shared/spec/responses/forbidden.yml' }, 404: { $ref: '../../../../shared/spec/responses/notFound.yml' }, 409: { $ref: '../../../../shared/spec/responses/conflict.yml' }, + 413: { $ref: '../../../../shared/spec/responses/contentTooLarge.yml' }, 415: { $ref: '../../../../shared/spec/responses/unsupportedMediaType.yml' }, 422: { $ref: '../../../../shared/spec/responses/unprocessableEntity.yml' }, 500: { $ref: '../../../../shared/spec/responses/internalServerError.yml' }, @@ -62,6 +63,7 @@ export const ERROR_RESPONSE_DESCRIPTIONS: Record 403: 'Forbidden', 404: 'The specified resource was not found.', 409: 'Conflict', + 413: 'Content too large.', 415: 'Unsupported media type.', 422: 'Unprocessable Entity', 500: 'Internal server error.', @@ -156,18 +158,20 @@ function buildPathParams(route: ResolvedPublicApiRoute): z.AnyZodObject | undefi return Object.keys(shape).length ? z.object(shape) : undefined; } -/** A route's request body, straight from its `@Body` DTO - no field-splitting needed like query has. */ function buildRequestBody( route: ResolvedPublicApiRoute, ): NonNullable['body'] { - if (!route.requestBodyDto) return undefined; + if (!route.requestBodyDto || !route.requestBodyHandler) { + return undefined; + } const required = route.requestBodyRequired ?? isRequestBodyRequired(route.requestBodyDto); + const handler = route.requestBodyHandler; return { ...(required ? { required: true } : {}), content: { - 'application/json': { + [handler.mediaType]: { schema: route.requestBodyDto.schema, }, }, @@ -229,8 +233,17 @@ function buildResponses( if (route.requestBodyDto ?? route.requestQueryDto) { responses[400] = ERROR_RESPONSE_REFS[400]; } - if (route.requestBodyDto) { - responses[415] = ERROR_RESPONSE_REFS[415]; + if (route.requestBodyDto && route.requestBodyHandler) { + const handler = route.requestBodyHandler; + for (const status of handler.errorStatuses) { + if (!isDocumentedErrorStatus(status)) { + throw new UnexpectedError( + `Request-body media type ${handler.mediaType} declares undocumented error status ` + + `${status} - add a shared response file and register it in ERROR_RESPONSE_REFS.`, + ); + } + responses[status] = ERROR_RESPONSE_REFS[status]; + } } responses[401] = ERROR_RESPONSE_REFS[401]; if (route.apiKeyScope) { 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 4773a9ddc12..d465ce964f3 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 @@ -12,6 +12,8 @@ PaymentRequired: $ref: './paymentRequired.yml' UnsupportedMediaType: $ref: './unsupportedMediaType.yml' +ContentTooLarge: + $ref: './contentTooLarge.yml' UnprocessableEntity: $ref: './unprocessableEntity.yml' InternalServerError: diff --git a/packages/cli/src/public-api/v1/shared/spec/responses/contentTooLarge.yml b/packages/cli/src/public-api/v1/shared/spec/responses/contentTooLarge.yml new file mode 100644 index 00000000000..be9c6530ee2 --- /dev/null +++ b/packages/cli/src/public-api/v1/shared/spec/responses/contentTooLarge.yml @@ -0,0 +1 @@ +description: Content too large.