diff --git a/.changeset/true-banks-pump.md b/.changeset/true-banks-pump.md new file mode 100644 index 000000000..11a4283ae --- /dev/null +++ b/.changeset/true-banks-pump.md @@ -0,0 +1,20 @@ +--- +'@hono/zod-openapi': patch +--- + +Fix critical security vulnerability in body validation + +Fixes a security issue where request body validation could be bypassed by omitting the Content-Type header (introduced in v0.15.2). + +Security Impact: +- Previously, requests without Content-Type headers would skip validation entirely, allowing unvalidated data to reach handlers +- This could lead to type safety violations, unexpected behavior, and potential security vulnerabilities + +Changes: +- Made validation strict by default (when required is not specified) +- Requests without Content-Type are now validated instead of bypassing validation +- Added proper handling for multiple content-type scenarios +- Return 400 errors for unsupported content-types + +Breaking Change: +To allow optional body validation (previous behavior), explicitly set required: false in the route configuration. \ No newline at end of file diff --git a/packages/zod-openapi/README.md b/packages/zod-openapi/README.md index d02172613..e8d4d53f1 100644 --- a/packages/zod-openapi/README.md +++ b/packages/zod-openapi/README.md @@ -117,8 +117,21 @@ You can start your app just like you would with Hono. For Cloudflare Workers and export default app ``` -> [!IMPORTANT] -> The request must have the proper `Content-Type` to ensure the validation. For example, if you want to validate a JSON body, the request must have the `Content-Type` to `application/json` in the request. Otherwise, the value of `c.req.valid('json')` will be `{}`. +> [!IMPORTANT] > **Security Notice**: Body validation is now strict by default to prevent validation bypass vulnerabilities. +> +> ### Request Body Validation Behavior +> +> The validation behavior depends on the `required` field in `request.body`: +> +> | `required` value | Content-Type present | Content-Type missing | Wrong Content-Type | +> | ----------------------- | -------------------- | -------------------- | ------------------ | +> | Not specified (default) | ✅ Validates | ✅ Validates | ❌ Returns 400 | +> | `true` | ✅ Validates | ✅ Validates | ❌ Returns 400 | +> | `false` | ✅ Validates | ⚠️ Allows empty body | ❌ Returns 400 | +> +> ### Default Behavior (Secure) +> +> By default, requests are always validated to prevent security vulnerabilities: > > ```ts > import { createRoute, z, OpenAPIHono } from '@hono/zod-openapi' @@ -135,6 +148,7 @@ export default app > }), > }, > }, +> // required is not specified - defaults to strict validation > }, > }, > responses: { @@ -148,20 +162,22 @@ export default app > > app.openapi(route, (c) => { > const validatedBody = c.req.valid('json') -> return c.json(validatedBody) // validatedBody is {} +> return c.json(validatedBody) > }) > +> // Request without Content-Type will be validated (returns 400 for invalid data) > const res = await app.request('/books', { > method: 'POST', -> body: JSON.stringify({ title: 'foo' }), -> // The Content-Type header is lacking. +> body: JSON.stringify({ title: '' }), // Invalid: empty title +> // No Content-Type header > }) > -> const data = await res.json() -> console.log(data) // {} +> console.log(res.status) // 400 - Validation error > ``` > -> If you want to force validation of requests that do not have the proper `Content-Type`, set the value of `request.body.required` to `true`. +> ### Optional Body Validation +> +> To allow requests without a body (but still validate when body is present), explicitly set `required: false`: > > ```ts > const route = createRoute({ @@ -176,11 +192,50 @@ export default app > }), > }, > }, -> required: true, // <== add +> required: false, // Explicitly allow empty body +> }, +> }, +> }) +> +> // Request without Content-Type and body is allowed +> const res1 = await app.request('/books', { +> method: 'POST', +> }) +> console.log(res1.status) // 200 - Empty body allowed +> +> // But if Content-Type is present, body is validated +> const res2 = await app.request('/books', { +> method: 'POST', +> body: JSON.stringify({ title: '' }), // Invalid +> headers: { 'Content-Type': 'application/json' }, +> }) +> console.log(res2.status) // 400 - Validation error +> ``` +> +> ### Multiple Content-Type Support +> +> When multiple content types are supported, the appropriate validator is used based on the Content-Type header: +> +> ```ts +> const route = createRoute({ +> method: 'post', +> path: '/data', +> request: { +> body: { +> content: { +> 'application/json': { +> schema: z.object({ jsonField: z.string() }), +> }, +> 'application/x-www-form-urlencoded': { +> schema: z.object({ formField: z.string() }), +> }, +> }, > }, > }, > }) > ``` +> +> Unsupported content types will return a 400 error with a descriptive message. ### Handling Validation Errors diff --git a/packages/zod-openapi/src/index.test.ts b/packages/zod-openapi/src/index.test.ts index 0ab351831..bd7f3f78b 100644 --- a/packages/zod-openapi/src/index.test.ts +++ b/packages/zod-openapi/src/index.test.ts @@ -491,12 +491,12 @@ describe('JSON', () => { expect(res.status).toBe(400) }) - it('Should return 200 response without a content-type', async () => { + it('Should return 400 response without a content-type (security fix)', async () => { const req = new Request('http://localhost/posts', { method: 'POST', }) const res = await app.request(req) - expect(res.status).toBe(200) + expect(res.status).toBe(400) // Now validates by default for security }) }) @@ -683,12 +683,12 @@ describe('Form', () => { }) }) - it('Should return 200 response without a content-type', async () => { + it('Should return 400 response without a content-type (security fix)', async () => { const req = new Request('http://localhost/posts', { method: 'POST', }) const res = await app.request(req) - expect(res.status).toBe(200) + expect(res.status).toBe(400) // Now validates by default for security }) describe('required', () => { diff --git a/packages/zod-openapi/src/index.ts b/packages/zod-openapi/src/index.ts index b8ce3e2b9..babb1797d 100644 --- a/packages/zod-openapi/src/index.ts +++ b/packages/zod-openapi/src/index.ts @@ -501,6 +501,13 @@ export class OpenAPIHono< const bodyContent = route.request?.body?.content if (bodyContent) { + // Check if multiple content types are supported + const supportedContentTypes = Object.keys(bodyContent).filter((mediaType) => { + const schema = (bodyContent[mediaType] as ZodMediaTypeObject)?.['schema'] + return schema instanceof ZodType + }) + const hasMultipleContentTypes = supportedContentTypes.length > 1 + for (const mediaType of Object.keys(bodyContent)) { if (!bodyContent[mediaType]) { continue @@ -513,39 +520,179 @@ export class OpenAPIHono< // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore we can ignore the type error since Zod Validator's types are not used const validator = zValidator('json', schema, hook) - if (route.request?.body?.required) { - validators.push(validator) + if (route.request?.body?.required === false) { + // Only bypass validation if explicitly set to false + const mw: MiddlewareHandler = async (c, next) => { + const contentType = c.req.header('content-type') + + if (!contentType) { + // No content-type header - allow empty body only if required is explicitly false + c.req.addValidatedData('json', {}) + await next() + } else if (isJSONContentType(contentType)) { + // Content-type matches - always validate + return await validator(c, next) + } else if (!hasMultipleContentTypes) { + // Content-type doesn't match and no other content types - return 400 + return c.json( + { + success: false, + error: { + message: 'Invalid content-type. Expected application/json', + }, + }, + 400 + ) + } else { + // Multiple content types supported - skip this validator + await next() + } + } + validators.push(mw) } else { + // Default behavior: validate strictly but check content-type const mw: MiddlewareHandler = async (c, next) => { - if (c.req.header('content-type')) { - if (isJSONContentType(c.req.header('content-type')!)) { - return await validator(c, next) - } + const contentType = c.req.header('content-type') + + if (!contentType || isJSONContentType(contentType)) { + // No content-type or matching content-type - validate + return await validator(c, next) + } else if (!hasMultipleContentTypes) { + // Content-type doesn't match and no other content types - return 400 + return c.json( + { + success: false, + error: { + message: 'Invalid content-type. Expected application/json', + }, + }, + 400 + ) + } else { + // Multiple content types supported - skip this validator + await next() } - c.req.addValidatedData('json', {}) - await next() } validators.push(mw) } } if (isFormContentType(mediaType)) { const validator = zValidator('form', schema, hook as any) - if (route.request?.body?.required) { - validators.push(validator) + if (route.request?.body?.required === false) { + // Only bypass validation if explicitly set to false + const mw: MiddlewareHandler = async (c, next) => { + const contentType = c.req.header('content-type') + + if (!contentType) { + // No content-type header - allow empty body only if required is explicitly false + c.req.addValidatedData('form', {}) + await next() + } else if (isFormContentType(contentType)) { + // Content-type matches - always validate + return await validator(c, next) + } else if (!hasMultipleContentTypes) { + // Content-type doesn't match and no other content types - return 400 + return c.json( + { + success: false, + error: { + message: + 'Invalid content-type. Expected multipart/form-data or application/x-www-form-urlencoded', + }, + }, + 400 + ) + } else { + // Multiple content types supported - skip this validator + await next() + } + } + validators.push(mw) } else { + // Default behavior: validate strictly but check content-type const mw: MiddlewareHandler = async (c, next) => { - if (c.req.header('content-type')) { - if (isFormContentType(c.req.header('content-type')!)) { - return await validator(c, next) - } + const contentType = c.req.header('content-type') + + if (!contentType || isFormContentType(contentType)) { + // No content-type or matching content-type - validate + return await validator(c, next) + } else if (!hasMultipleContentTypes) { + // Content-type doesn't match and no other content types - return 400 + return c.json( + { + success: false, + error: { + message: + 'Invalid content-type. Expected multipart/form-data or application/x-www-form-urlencoded', + }, + }, + 400 + ) + } else { + // Multiple content types supported - skip this validator + await next() } - c.req.addValidatedData('form', {}) - await next() } validators.push(mw) } } } + + // Add a final validator to handle cases where no validator matched (for multiple content-types) + if (hasMultipleContentTypes) { + const finalCheck: MiddlewareHandler = async (c, next) => { + const contentType = c.req.header('content-type') + + // Check if any validator has added validated data + // We check the internal validated data store + // @ts-expect-error accessing internal property + const validatedData = c.req.validatedData + const hasValidatedData = + validatedData && (validatedData.json !== undefined || validatedData.form !== undefined) + + if (!hasValidatedData) { + if (contentType) { + // Check if content-type matches any supported type + const isSupported = supportedContentTypes.some((type) => { + if (isJSONContentType(type) && isJSONContentType(contentType)) return true + if (isFormContentType(type) && isFormContentType(contentType)) return true + return false + }) + + if (!isSupported) { + // Has content-type but it's not supported + const supportedTypes = supportedContentTypes.join(', ') + return c.json( + { + success: false, + error: { + message: `Invalid content-type. Expected one of: ${supportedTypes}`, + }, + }, + 400 + ) + } + } else if (route.request?.body?.required !== false) { + // No content-type and body is required (default or explicitly true) + // Validate with the first available validator + const firstMediaType = supportedContentTypes[0] + const schema = (bodyContent[firstMediaType] as ZodMediaTypeObject)['schema'] + + if (isJSONContentType(firstMediaType)) { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const validator = zValidator('json', schema, hook) + return await validator(c, next) + } else if (isFormContentType(firstMediaType)) { + const validator = zValidator('form', schema as any, hook as any) + return await validator(c, next) + } + } + } + await next() + } + validators.push(finalCheck) + } } const middleware = routeMiddleware diff --git a/packages/zod-openapi/src/validation-security.test.ts b/packages/zod-openapi/src/validation-security.test.ts new file mode 100644 index 000000000..842233211 --- /dev/null +++ b/packages/zod-openapi/src/validation-security.test.ts @@ -0,0 +1,511 @@ +import { describe, expect, it } from 'vitest' +import { OpenAPIHono, createRoute, z } from './index' + +describe('Validation Security - Body Required Behavior', () => { + const PostSchema = z.object({ + title: z.string().min(5), + content: z.string(), + }) + + describe('JSON body validation', () => { + describe('when body.required is not specified (default behavior)', () => { + const route = createRoute({ + method: 'post', + path: '/posts', + request: { + body: { + content: { + 'application/json': { + schema: PostSchema, + }, + }, + // required is not specified - should default to strict validation + }, + }, + responses: { + 200: { + description: 'Success', + content: { + 'application/json': { + schema: z.object({ success: z.boolean() }), + }, + }, + }, + }, + }) + + const app = new OpenAPIHono() + app.openapi(route, async (c) => { + const body = c.req.valid('json') + return c.json({ success: true }, 200) + }) + + it('should validate when content-type is application/json', async () => { + const res = await app.request('/posts', { + method: 'POST', + body: JSON.stringify({ title: 'Valid Title', content: 'Content' }), + headers: { 'Content-Type': 'application/json' }, + }) + expect(res.status).toBe(200) + }) + + it('should reject invalid body when content-type is application/json', async () => { + const res = await app.request('/posts', { + method: 'POST', + body: JSON.stringify({ title: 'Bad' }), // title too short + headers: { 'Content-Type': 'application/json' }, + }) + expect(res.status).toBe(400) + }) + + it('should reject request without content-type (security fix)', async () => { + const res = await app.request('/posts', { + method: 'POST', + body: JSON.stringify({ title: 'Bad' }), + }) + expect(res.status).toBe(400) // Should validate by default + }) + + it('should return 400 for wrong content-type', async () => { + const res = await app.request('/posts', { + method: 'POST', + body: JSON.stringify({ title: 'Valid Title', content: 'Content' }), + headers: { 'Content-Type': 'text/plain' }, + }) + expect(res.status).toBe(400) + const json = await res.json() + expect(json).toHaveProperty('error') + expect(json.error.message).toContain('Invalid content-type') + }) + }) + + describe('when body.required is explicitly true', () => { + const route = createRoute({ + method: 'post', + path: '/posts', + request: { + body: { + content: { + 'application/json': { + schema: PostSchema, + }, + }, + required: true, + }, + }, + responses: { + 200: { + description: 'Success', + content: { + 'application/json': { + schema: z.object({ success: z.boolean() }), + }, + }, + }, + }, + }) + + const app = new OpenAPIHono() + app.openapi(route, async (c) => { + const body = c.req.valid('json') + return c.json({ success: true }, 200) + }) + + it('should always validate the request body', async () => { + const res = await app.request('/posts', { + method: 'POST', + body: JSON.stringify({ title: 'Bad' }), + headers: { 'Content-Type': 'application/json' }, + }) + expect(res.status).toBe(400) + }) + + it('should reject request without content-type', async () => { + const res = await app.request('/posts', { + method: 'POST', + }) + expect(res.status).toBe(400) + }) + }) + + describe('when body.required is explicitly false', () => { + const route = createRoute({ + method: 'post', + path: '/posts', + request: { + body: { + content: { + 'application/json': { + schema: PostSchema, + }, + }, + required: false, + }, + }, + responses: { + 200: { + description: 'Success', + content: { + 'application/json': { + schema: z.object({ success: z.boolean() }), + }, + }, + }, + }, + }) + + const app = new OpenAPIHono() + app.openapi(route, async (c) => { + const body = c.req.valid('json') + return c.json({ success: true, body }, 200) + }) + + it('should validate when content-type is application/json', async () => { + const res = await app.request('/posts', { + method: 'POST', + body: JSON.stringify({ title: 'Bad' }), // Invalid + headers: { 'Content-Type': 'application/json' }, + }) + expect(res.status).toBe(400) + }) + + it('should allow empty body when no content-type is provided', async () => { + const res = await app.request('/posts', { + method: 'POST', + }) + expect(res.status).toBe(200) + const json = await res.json() + expect(json.body).toEqual({}) + }) + + it('should return 400 for wrong content-type', async () => { + const res = await app.request('/posts', { + method: 'POST', + body: 'plain text', + headers: { 'Content-Type': 'text/plain' }, + }) + expect(res.status).toBe(400) + const json = await res.json() + expect(json.error.message).toContain('Invalid content-type') + }) + }) + }) + + describe('Form body validation', () => { + const FormSchema = z.object({ + username: z.string().min(3), + password: z.string().min(8), + }) + + describe('when body.required is not specified (default behavior)', () => { + const route = createRoute({ + method: 'post', + path: '/login', + request: { + body: { + content: { + 'application/x-www-form-urlencoded': { + schema: FormSchema, + }, + }, + }, + }, + responses: { + 200: { + description: 'Success', + content: { + 'application/json': { + schema: z.object({ success: z.boolean() }), + }, + }, + }, + }, + }) + + const app = new OpenAPIHono() + app.openapi(route, async (c) => { + const body = c.req.valid('form') + return c.json({ success: true }, 200) + }) + + it('should validate when content-type is form-urlencoded', async () => { + const params = new URLSearchParams() + params.append('username', 'john') + params.append('password', 'password123') + + const res = await app.request('/login', { + method: 'POST', + body: params, + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + }) + expect(res.status).toBe(200) + }) + + it('should reject invalid form data', async () => { + const params = new URLSearchParams() + params.append('username', 'jo') // Too short + params.append('password', 'pass') // Too short + + const res = await app.request('/login', { + method: 'POST', + body: params, + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + }) + expect(res.status).toBe(400) + }) + + it('should reject request without content-type (security fix)', async () => { + const params = new URLSearchParams() + params.append('username', 'jo') + + const res = await app.request('/login', { + method: 'POST', + body: params, + }) + expect(res.status).toBe(400) + }) + + it('should return 400 for wrong content-type', async () => { + const res = await app.request('/login', { + method: 'POST', + body: JSON.stringify({ username: 'john', password: 'password123' }), + headers: { 'Content-Type': 'application/json' }, + }) + expect(res.status).toBe(400) + const json = await res.json() + expect(json.error.message).toContain('Invalid content-type') + }) + }) + + describe('when body.required is explicitly false', () => { + const route = createRoute({ + method: 'post', + path: '/login', + request: { + body: { + content: { + 'multipart/form-data': { + schema: FormSchema, + }, + }, + required: false, + }, + }, + responses: { + 200: { + description: 'Success', + content: { + 'application/json': { + schema: z.object({ success: z.boolean() }), + }, + }, + }, + }, + }) + + const app = new OpenAPIHono() + app.openapi(route, async (c) => { + const body = c.req.valid('form') + return c.json({ success: true, body }, 200) + }) + + it('should allow empty body when no content-type is provided', async () => { + const res = await app.request('/login', { + method: 'POST', + }) + expect(res.status).toBe(200) + const json = await res.json() + expect(json.body).toEqual({}) + }) + + it('should return 400 for wrong content-type', async () => { + const res = await app.request('/login', { + method: 'POST', + body: JSON.stringify({ username: 'john' }), + headers: { 'Content-Type': 'application/json' }, + }) + expect(res.status).toBe(400) + const json = await res.json() + expect(json.error.message).toContain('Invalid content-type') + expect(json.error.message).toContain('multipart/form-data') + }) + }) + }) + + describe('Multiple content-type support scenarios', () => { + const PostSchema = z.object({ + title: z.string().min(5), + content: z.string(), + }) + + const FormSchema = z.object({ + username: z.string().min(3), + password: z.string().min(8), + }) + + describe('when both JSON and Form content-types are supported', () => { + const route = createRoute({ + method: 'post', + path: '/posts', + request: { + body: { + content: { + 'application/json': { + schema: PostSchema, + }, + 'application/x-www-form-urlencoded': { + schema: FormSchema, + }, + }, + }, + }, + responses: { + 200: { + description: 'Success', + content: { + 'application/json': { + schema: z.object({ success: z.boolean() }), + }, + }, + }, + }, + }) + + const app = new OpenAPIHono() + app.openapi(route, async (c) => { + const jsonBody = c.req.valid('json') + const formBody = c.req.valid('form') + return c.json({ success: true, jsonBody, formBody }, 200) + }) + + it('should validate JSON when content-type is application/json', async () => { + const res = await app.request('/posts', { + method: 'POST', + body: JSON.stringify({ title: 'Valid Title', content: 'Content' }), + headers: { 'Content-Type': 'application/json' }, + }) + expect(res.status).toBe(200) + }) + + it('should validate Form when content-type is application/x-www-form-urlencoded', async () => { + const params = new URLSearchParams() + params.append('username', 'validuser') + params.append('password', 'password123') + + const res = await app.request('/posts', { + method: 'POST', + body: params, + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + }) + expect(res.status).toBe(200) + }) + + it('should reject invalid JSON data', async () => { + const res = await app.request('/posts', { + method: 'POST', + body: JSON.stringify({ title: 'Bad' }), // title too short + headers: { 'Content-Type': 'application/json' }, + }) + expect(res.status).toBe(400) + }) + + it('should reject invalid Form data', async () => { + const params = new URLSearchParams() + params.append('username', 'ab') // username too short + params.append('password', 'short') // password too short + + const res = await app.request('/posts', { + method: 'POST', + body: params, + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + }) + expect(res.status).toBe(400) + }) + + it('should return 400 for unsupported content-type even with multiple content-types', async () => { + const res = await app.request('/posts', { + method: 'POST', + body: JSON.stringify({ title: 'Valid Title', content: 'Content' }), + headers: { 'Content-Type': 'text/plain' }, + }) + expect(res.status).toBe(400) + const json = await res.json() + expect(json).toHaveProperty('error') + expect(json.error.message).toContain('content-type') + }) + + it('should reject when no content-type is provided even with valid data (security test)', async () => { + const res = await app.request('/posts', { + method: 'POST', + body: JSON.stringify({ title: 'Valid Title', content: 'Valid Content' }), // Valid data + }) + expect(res.status).toBe(400) + }) + }) + + describe('when multiple content-types are supported with required: false', () => { + const route = createRoute({ + method: 'post', + path: '/posts', + request: { + body: { + content: { + 'application/json': { + schema: PostSchema, + }, + 'multipart/form-data': { + schema: FormSchema, + }, + }, + required: false, + }, + }, + responses: { + 200: { + description: 'Success', + content: { + 'application/json': { + schema: z.object({ success: z.boolean() }), + }, + }, + }, + }, + }) + + const app = new OpenAPIHono() + app.openapi(route, async (c) => { + const jsonBody = c.req.valid('json') + const formBody = c.req.valid('form') + return c.json({ success: true, jsonBody, formBody }, 200) + }) + + it('should allow empty body when no content-type is provided', async () => { + const res = await app.request('/posts', { + method: 'POST', + }) + expect(res.status).toBe(200) + }) + + it('should validate when matching content-type is provided', async () => { + const res = await app.request('/posts', { + method: 'POST', + body: JSON.stringify({ title: 'Bad' }), // Invalid + headers: { 'Content-Type': 'application/json' }, + }) + expect(res.status).toBe(400) // Should validate + }) + + it('should return 400 for unrecognized content-type even with required: false and multiple content-types', async () => { + const res = await app.request('/posts', { + method: 'POST', + body: 'plain text', + headers: { 'Content-Type': 'text/plain' }, + }) + expect(res.status).toBe(400) + const json = await res.json() + expect(json).toHaveProperty('error') + expect(json.error.message).toContain('content-type') + }) + }) + }) +})