diff --git a/README.md b/README.md index 6dcf4d4..f958cf8 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,8 @@ A high-performance, minimalist HTTP framework for [Bun](https://bun.sh/), inspir > Landing page: [0http-bun.21no.de](https://0http-bun.21no.de) +> **v1.3.0** includes a comprehensive security hardening release. See the [Changelog](#changelog) and [Migration Guide](#migrating-to-v130) sections below. + ## ✨ Why Choose 0http-bun? 0http-bun combines the simplicity of Express with the raw performance of Bun's runtime, delivering a framework that's both **blazingly fast** and **secure by design**. Perfect for everything from quick prototypes to production-grade APIs. @@ -71,6 +73,32 @@ Bun.serve({ }) ``` +### Enabling Client IP for Rate Limiting + +Bun's standard `Request` object does not expose the client IP address. To enable the default rate limiter key generator (and any middleware that reads `req.ip`), use `server.requestIP()` in the `Bun.serve` fetch handler: + +```typescript +import http from '0http-bun' +import {createRateLimit} from '0http-bun/lib/middleware' + +const {router} = http() + +router.use(createRateLimit({windowMs: 15 * 60 * 1000, max: 100})) + +router.get('/', () => new Response('Hello World!')) + +// Populate req.ip from Bun's server.requestIP before passing to the router +Bun.serve({ + port: 3000, + fetch(req, server) { + req.ip = server.requestIP(req)?.address + return router.fetch(req) + }, +}) +``` + +> **Why is this needed?** The Fetch API `Request` type does not include connection-level properties like `ip`. Bun provides client IP via `server.requestIP(req)` in the fetch handler's second argument. Setting `req.ip` before calling `router.fetch` ensures the rate limiter (and other middleware) can identify clients correctly. Without this, the default key generator falls back to unique per-request keys, which effectively disables rate limiting. + ### With TypeScript Types ```typescript @@ -116,9 +144,12 @@ interface IRouterConfig { defaultRoute?: RequestHandler // Custom 404 handler errorHandler?: (err: Error) => Response | Promise // Error handler port?: number // Port number (for reference) + cacheSize?: number // Max entries per HTTP method in the route cache (default: 1000) } ``` +> **Note on `cacheSize`:** The router uses an LRU-style cache for resolved routes. When the cache exceeds `cacheSize` entries for a given HTTP method, the oldest entry is evicted. The default of `1000` is suitable for most applications. Increase it for APIs with many dynamic routes; decrease it to save memory in constrained environments. + ### Request Object The `ZeroRequest` extends the standard `Request` with additional properties: @@ -273,6 +304,10 @@ router.use('/api/*', createJWTAuth({secret: process.env.JWT_SECRET})) ### Error Handling +The default error handler returns a generic `"Internal Server Error"` response (HTTP 500) and logs the full error to `console.error`. This prevents leaking stack traces or internal details to clients. + +You can provide a custom `errorHandler` for more control: + ```typescript import http, {ZeroRequest} from '0http-bun' @@ -295,77 +330,78 @@ const {router} = http({ }, }) -// Route that might throw an error -router.get('/api/risky', (req: ZeroRequest) => { - if (Math.random() > 0.5) { - const error = new Error('Random failure') - error.name = 'ValidationError' - throw error +// Errors thrown in both sync and async handlers are caught automatically +router.get('/api/risky', async (req: ZeroRequest) => { + const data = await fetchExternalData() // async errors are caught too + if (!data) { + throw new Error('Data not found') } - - return Response.json({success: true}) + return Response.json(data) }) ``` -## 🛡️ Security +> **Async error handling:** Errors thrown or rejected in async middleware/handlers are automatically caught and forwarded to the `errorHandler`. You do not need to wrap every handler in try/catch. -0http-bun is designed with **security-first principles** and includes comprehensive protection against common web vulnerabilities. Our middleware and core framework have been thoroughly penetration-tested to ensure production-ready security. +## Security -### 🔒 Built-in Security Features +0http-bun is designed with **security-first principles** and includes comprehensive protection against common web vulnerabilities. The core framework and middleware have been thoroughly penetration-tested and hardened. + +### Built-in Security Features #### **Input Validation & Sanitization** - **Size Limits**: Configurable limits prevent memory exhaustion attacks - **ReDoS Protection**: Restrictive regex patterns prevent Regular Expression DoS -- **JSON Security**: Nesting depth limits and safe parsing practices +- **JSON Security**: String-aware nesting depth validation and safe parsing - **Parameter Validation**: Maximum parameter counts and length restrictions +- **Filename Sanitization**: Multipart file uploads are sanitized to prevent path traversal (directory separators, `..`, null bytes are stripped; `originalName` preserved for reference) #### **Attack Prevention** -- **Prototype Pollution Protection**: Filters dangerous keys (`__proto__`, `constructor`, `prototype`) -- **Safe Property Access**: Uses `Object.prototype.hasOwnProperty.call()` for secure property access -- **Memory Exhaustion Prevention**: Strict size limits and cleanup mechanisms -- **DoS Mitigation**: Rate limiting and request throttling capabilities +- **Prototype Pollution Protection**: Filters dangerous keys (`__proto__`, `constructor`, `prototype`) in body parser, multipart parser, and URL-encoded parser using `Object.create(null)` for safe objects +- **Timing Attack Prevention**: API key comparisons use `crypto.timingSafeEqual()` +- **Algorithm Confusion Prevention**: JWT middleware rejects mixed symmetric/asymmetric algorithm configurations +- **Cache Exhaustion Prevention**: LRU-style route cache with configurable `cacheSize` limit (default: 1000) +- **Memory Exhaustion Prevention**: Strict size limits, sliding window rate limiter with `maxKeys` eviction, and automatic cleanup intervals +- **Route Filter Bypass Prevention**: URL path normalization (double-slash collapse, URI decoding, `%2F` preservation) +- **Frozen Route Params**: Parameterless routes receive an immutable `Object.freeze({})` to prevent cross-request data leakage #### **Error Handling** -- **Sanitized Error Messages**: Prevents information disclosure in production -- **Custom Error Handlers**: Flexible error handling with type-based responses -- **Secure Defaults**: Safe 404 and 500 responses without stack traces +- **Generic Error Responses**: Default error handler returns `"Internal Server Error"` without exposing stack traces or `err.message` +- **Server-Side Logging**: Full error details logged via `console.error` for debugging +- **Async Error Catching**: Rejected promises from async middleware are automatically caught and forwarded to the error handler +- **Unified JWT Errors**: JWT signature/expiry/claim validation failures return `"Invalid or expired token"` regardless of the specific failure reason #### **Authentication & Authorization** -- **JWT Security**: Proper token validation with comprehensive error handling -- **API Key Authentication**: Secure key validation with custom validator support -- **Path Exclusion**: Flexible authentication bypass for public routes -- **Token Extraction**: Secure token retrieval from headers and query parameters +- **JWT Security**: Default algorithms restricted to `['HS256']`; algorithm confusion prevented +- **Timing-Safe API Keys**: All API key comparisons use constant-time comparison +- **Path Exclusion Security**: Uses exact match or path boundary checking (`path + '/'`) instead of prefix matching +- **Optional Mode Visibility**: When `optional: true`, invalid tokens set `req.ctx.authError` and `req.ctx.authAttempted` for downstream inspection +- **Minimal Token Exposure**: Raw JWT token is no longer stored on the request context #### **Rate Limiting** -- **Sliding Window**: Precise rate limiting with memory-efficient implementation -- **IP-based Keys**: Smart key generation with proxy support -- **Memory Cleanup**: Automatic cleanup of expired entries -- **Configurable Limits**: Flexible rate limiting with skip functions +- **Secure Key Generation**: Default key generator uses `req.ip || req.remoteAddress || req.socket?.remoteAddress || 'unknown'` — proxy headers are **not trusted** by default +- **No Store Injection**: Rate limit store is always the constructor-configured instance (no `req.rateLimitStore` override) +- **Bounded Memory**: Sliding window rate limiter enforces `maxKeys` (default: 10,000) with periodic cleanup +- **Synchronous Increment**: `MemoryStore.increment` is synchronous to eliminate TOCTOU race conditions +- **Configurable Limits**: Flexible rate limiting with skip functions and path exclusion #### **CORS Security** -- **Origin Validation**: Dynamic origin checking with proper Vary headers +- **Null Origin Rejection**: `null` and missing origins are rejected before calling validator functions or checking arrays, preventing sandboxed iframe bypass +- **Conditional Headers**: CORS headers (methods, allowed headers, credentials, exposed headers) are only set when the origin is actually allowed +- **Vary Header**: `Vary: Origin` is set for all non-wildcard origins to prevent CDN cache poisoning - **Credential Safety**: Prevents wildcard origins with credentials -- **Preflight Handling**: Comprehensive OPTIONS request processing -- **Header Security**: Proper CORS header management - -### 📊 Security Assessment - -- **✅ No Critical Vulnerabilities** -- **✅ No High-Risk Issues** -- **✅ No Medium-Risk Vulnerabilities** -- **✅ Dependencies Audit Passed** -### 🛠️ Security Best Practices +### Security Best Practices ```typescript // Secure server configuration const {router} = http({ + cacheSize: 500, // Limit route cache for constrained environments errorHandler: (err: Error) => { // Never expose stack traces in production return Response.json({error: 'Internal server error'}, {status: 500}) @@ -378,7 +414,7 @@ const {router} = http({ // Apply security middleware stack router.use( createCORS({ - origin: ['https://yourdomain.com'], // Restrict origins + origin: ['https://yourdomain.com'], // Restrict origins — null origins are rejected credentials: true, }), ) @@ -386,7 +422,9 @@ router.use( router.use( createRateLimit({ windowMs: 15 * 60 * 1000, // 15 minutes - max: 100, // Limit each IP to 100 requests per window + max: 100, + // Behind a reverse proxy? Provide a custom keyGenerator: + // keyGenerator: (req) => req.headers.get('x-forwarded-for') || req.ip || 'unknown', }), ) @@ -394,7 +432,10 @@ router.use( '/api/*', createJWTAuth({ secret: process.env.JWT_SECRET, - algorithms: ['HS256'], + algorithms: ['HS256'], // Default — explicit for clarity + // For RS256, use jwksUri instead of secret: + // jwksUri: 'https://your-idp.com/.well-known/jwks.json', + // algorithms: ['RS256'], }), ) @@ -408,7 +449,7 @@ router.use( ) ``` -### 🔍 Security Monitoring +### Security Monitoring 0http-bun provides built-in security monitoring capabilities: @@ -420,7 +461,8 @@ router.use( req: (req) => ({ method: req.method, url: req.url, - ip: req.headers.get('x-forwarded-for') || 'unknown', + ip: + req.ip || req.remoteAddress || req.socket?.remoteAddress || 'unknown', userAgent: req.headers.get('user-agent'), }), }, @@ -436,15 +478,17 @@ router.use( ) ``` -### 🚨 Security Recommendations +### Security Recommendations 1. **Environment Variables**: Store secrets in environment variables, never in code 2. **HTTPS Only**: Always use HTTPS in production with proper TLS configuration -3. **Input Validation**: Validate and sanitize all user inputs -4. **Regular Updates**: Keep dependencies updated and run security audits -5. **Monitoring**: Implement logging and monitoring for security events +3. **Reverse Proxy Configuration**: If behind a reverse proxy, always provide a custom `keyGenerator` for rate limiting that reads the appropriate header (e.g., `X-Forwarded-For`) +4. **Algorithm Explicitness**: Always explicitly set JWT `algorithms` — do not rely on defaults +5. **Input Validation**: Validate and sanitize all user inputs +6. **Regular Updates**: Keep dependencies updated and run security audits +7. **Monitoring**: Implement logging and monitoring for security events -> 📖 **Security is a continuous process**. While 0http-bun provides strong security foundations, always follow security best practices and conduct regular security assessments for your applications. +> **Security is a continuous process**. While 0http-bun provides strong security foundations, always follow security best practices and conduct regular security assessments for your applications. ## Performance @@ -454,7 +498,8 @@ router.use( - **Efficient routing**: Based on the proven `trouter` library - **Fast parameter parsing**: Optimized URL parameter extraction with caching - **Query string parsing**: Uses `fast-querystring` for optimal performance -- **Memory efficient**: Route caching and object reuse to minimize allocations +- **Memory efficient**: LRU-style route caching with configurable `cacheSize` limit, immutable shared objects, and minimal allocations +- **URL normalization**: Single-pass URL parsing with path normalization (double-slash collapse, URI decoding) ### Benchmark Results @@ -527,10 +572,10 @@ const typedHandler = (req: ZeroRequest): Response => { ### 🔧 **Developer Tools** - **TypeScript Support**: Full type definitions and IntelliSense -- **Error Handling**: Comprehensive error management with custom handlers +- **Error Handling**: Comprehensive error management with custom handlers; async errors caught automatically - **Request Context**: Flexible context system for middleware data sharing - **Parameter Parsing**: Automatic URL parameter and query string parsing -- **Route Caching**: Intelligent caching for optimal performance +- **Route Caching**: LRU-style caching with configurable `cacheSize` for bounded memory usage ### 🚀 **Deployment Ready** @@ -560,6 +605,214 @@ _Benchmarks run on Bun v1.2.2 with simple JSON response routes. Results may vary - **💬 Community Discussions**: GitHub Discussions for questions and ideas - **🎯 Production Proven**: Used in production by companies worldwide +## Changelog + +### v1.3.0 — Security Hardening Release + +This release addresses **43 vulnerabilities** (6 Critical, 13 High, 13 Medium, 7 Low, 4 Info) identified in a comprehensive penetration test. All 43 issues have been resolved. + +#### Breaking Changes + +| Change | Old Behavior | New Behavior | +| ----------------------------- | ----------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------- | +| **Rate limit key generator** | Trusted `X-Forwarded-For`, `X-Real-IP`, `CF-Connecting-IP` proxy headers | Uses `req.ip \|\| req.remoteAddress \|\| 'unknown'` only. Supply a custom `keyGenerator` if behind a reverse proxy. | +| **JWT default algorithms** | `['HS256', 'RS256']` | `['HS256']` only. Mixed symmetric + asymmetric algorithms throw an error. Explicitly set `algorithms: ['RS256']` if using RSA keys. | +| **Default error handler** | Returned `err.message` to the client in the response body | Returns generic `"Internal Server Error"`. Full error logged server-side via `console.error`. | +| **`parseLimit` validation** | Silently returned 1MB default for `false`, `null`, objects | Throws `TypeError` for unexpected types. | +| **JWT token in context** | `req.jwt.token` and `req.ctx.jwt.token` contained the raw JWT string | Raw token no longer stored on the request context. Only `payload` and `header` are available. | +| **JWT module exports** | Exported internal functions (`extractToken`, `handleAuthError`, `extractTokenFromHeader`, etc.) | Only exports `createJWTAuth`, `createAPIKeyAuth`, and `API_KEY_SYMBOL`. Internal helpers are no longer exposed. | +| **Rate limit store override** | `req.rateLimitStore` could override the configured store at runtime | Always uses the constructor-configured store. `req.rateLimitStore` has no effect. | +| **API key in context** | `req.apiKey` / `req.ctx.apiKey` contained the raw API key | Now stores a masked version (`xxxx****xxxx`). Raw key available via `req[API_KEY_SYMBOL]`. | +| **Empty JSON body** | Empty/whitespace JSON body silently returned `{}` | Now sets `req.body` to `undefined`. Applications must handle `undefined` explicitly. | + +#### Core Router + +- **LRU route cache** with configurable `cacheSize` (default: 1000) — prevents unbounded memory growth from cache poisoning attacks. +- **URL path normalization** — double slashes collapsed, URI-decoded (preserving `%2F`), preventing route filter bypass via `//admin` or `%2e%2e` paths. +- **Immutable empty params** — `Object.freeze({})` prevents cross-request data leakage on parameterless routes. +- **`router.use()` return fix** — `return this` in arrow function corrected to `return router` for proper chaining. +- **Query prototype pollution protection** — dangerous keys (`__proto__`, `constructor`, `prototype`) are filtered from parsed query strings, mirroring existing route params protection. + +#### Middleware Chain + +- **Async error handling** — rejected promises from async middleware are now caught via `.catch()` and forwarded to the `errorHandler`. Previously, they were unhandled. + +#### Body Parser + +- **String-aware JSON nesting depth scanner** — brace characters inside JSON strings no longer count toward nesting depth, fixing a bypass where `{"key": "{{{..."}` could evade the depth check. +- **Custom `jsonParser` enforces size limits** — size validation runs before the custom parser function is called. +- **Prototype pollution protection for multipart** — uses `Object.create(null)` for body/files objects and blocklists dangerous property names. +- **Multipart filename sanitization** — strips `..`, path separators (`/`, `\`), null bytes, and leading dots. The original filename is preserved in `file.originalName`. +- **Strict content-type matching** — JSON parser matches `application/json` only (was `application/` which matched `application/xml`, `application/octet-stream`, etc.). +- **`parseLimit` type validation** — throws `TypeError` on unexpected input types instead of silently defaulting. +- **Empty JSON body handling** — empty or whitespace-only JSON bodies now set `req.body` to `undefined` instead of silently returning `{}`. _(Breaking change)_ +- **Raw body via Symbol** — raw body text is now stored via `Symbol.for('0http.rawBody')` (`RAW_BODY_SYMBOL`) instead of `req._rawBodyText`, preventing accidental serialization/logging. The symbol is exported from the body parser module. + +#### JWT Authentication + +- **Timing-safe API key comparison** — all API key comparisons use `crypto.timingSafeEqual()` with constant-time length mismatch handling. +- **Algorithm confusion prevention** — throws if both symmetric (`HS*`) and asymmetric (`RS*`/`ES*`/`PS*`) algorithms are configured. +- **Secure path exclusion** — exact match or `path + '/'` boundary checking replaces prefix matching (prevents `/healthcheck` bypassing `/health` exclusion). +- **Optional mode transparency** — when `optional: true` and a token is invalid, sets `req.ctx.authError` and `req.ctx.authAttempted = true` instead of silently proceeding. +- **Unified error messages** — JWT signature/expiry/claim validation failures return `"Invalid or expired token"` to prevent oracle attacks. Distinct messages are used for missing tokens and API key failures. +- **Safe option merging** — `...jwtOptions` spread applied first; security-critical options (`algorithms`, `audience`, `issuer`) override after. +- **Validator call signature** — always calls `apiKeyValidator(apiKey, req)` regardless of `Function.length` arity. +- **Reduced export surface** — only `createJWTAuth` and `createAPIKeyAuth` are exported. +- **Token type validation** — new `requiredTokenType` option validates the JWT `typ` header claim (case-insensitive). Rejects tokens with missing or incorrect type when configured. +- **API key via Symbol** — raw API key available via `req[API_KEY_SYMBOL]` (exported `Symbol.for('0http.apiKey')`). `req.apiKey` and `req.ctx.apiKey` now store a masked version (`xxxx****xxxx`). _(Breaking change)_ +- **Error logging in auth handler** — empty `catch` blocks in `handleAuthError` now log errors via `console.error` for debugging visibility. + +#### Rate Limiting + +- **Secure default key generator** — uses `req.ip || req.remoteAddress || 'unknown'` instead of trusting proxy headers. +- **Sliding window memory bounds** — `maxKeys` option (default: 10,000) with periodic cleanup via `setInterval` + `unref()`. +- **Synchronous `MemoryStore.increment`** — eliminates TOCTOU race condition in the fixed-window store. +- **Exact path exclusion** — `excludePaths` uses exact or boundary matching. +- **Configurable header disclosure** — `standardHeaders` now accepts `true` (full headers, default), `false` (no headers), or `'minimal'` (only `Retry-After` on 429 responses) to control rate limit information disclosure. +- **Unique unknown keys** — when no IP is available, the default key generator now creates unique per-request keys instead of sharing a single `'unknown'` bucket, preventing shared-bucket DoS. + +#### CORS + +- **Null origin rejection** — `null`/missing origins rejected before calling validator functions or checking arrays, preventing sandboxed iframe bypass. +- **Conditional CORS headers** — headers only set when the origin is actually allowed (previously leaked method/header lists even on rejected origins). +- **`Vary: Origin`** — set for all non-wildcard origins (previously only set for function/array origins). +- **Single allowedHeaders resolution** — `allowedHeaders` function is now resolved once per preflight request instead of multiple times, preventing inconsistency. + +### v1.2.2 + +- Middleware dependencies (`jose`, `pino`, `prom-client`) made optional with lazy loading. +- Prometheus metrics middleware added. +- Logger middleware added. + +--- + +## Migrating to v1.3.0 + +### Rate Limiting Behind a Reverse Proxy + +The default `keyGenerator` no longer reads proxy headers. If your application runs behind a reverse proxy (nginx, Cloudflare, AWS ALB, etc.), you **must** provide a custom `keyGenerator`: + +```typescript +router.use( + createRateLimit({ + windowMs: 15 * 60 * 1000, + max: 100, + keyGenerator: (req) => { + // Trust the header your reverse proxy sets + return ( + req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() || + req.ip || + 'unknown' + ) + }, + }), +) +``` + +### JWT Algorithm Configuration + +If you were relying on the default `['HS256', 'RS256']` algorithm list, you must now be explicit: + +```typescript +// For HMAC (symmetric) secrets: +createJWTAuth({ + secret: process.env.JWT_SECRET, + algorithms: ['HS256'], // This is now the default +}) + +// For RSA (asymmetric) keys: +createJWTAuth({ + jwksUri: 'https://your-idp.com/.well-known/jwks.json', + algorithms: ['RS256'], // Must be explicit — mixing with HS256 will throw +}) +``` + +### Error Handler + +If you relied on `err.message` being returned to clients from the default error handler, provide a custom `errorHandler`: + +```typescript +const {router} = http({ + errorHandler: (err: Error) => { + // Old behavior (NOT recommended for production): + return new Response(err.message, {status: 500}) + }, +}) +``` + +### `parseLimit` Validation + +If your code passes non-string/non-number values to `parseLimit` (e.g., `false`, `null`, objects), update it to pass a valid value: + +```typescript +// Before (silently defaulted to 1MB): +createBodyParser({json: {limit: someConfig.limit}}) // someConfig.limit might be null + +// After (throws TypeError): +createBodyParser({json: {limit: someConfig.limit || '1mb'}}) // Provide a fallback +``` + +### JWT Token Context + +If you accessed the raw JWT token string via `req.jwt.token` or `req.ctx.jwt.token`, note that only `payload` and `header` are now available: + +```typescript +// Before: +const rawToken = req.jwt.token // No longer available + +// After: +const payload = req.jwt.payload // Decoded payload +const header = req.jwt.header // Protected header +``` + +### API Key Access + +If you accessed the raw API key via `req.apiKey` or `req.ctx.apiKey`, note that these now contain a masked version. Use the exported `API_KEY_SYMBOL` for programmatic access: + +```typescript +import {API_KEY_SYMBOL} from '0http-bun/lib/middleware/jwt-auth' + +// Before: +const rawKey = req.apiKey // Was the raw API key, now masked (xxxx****xxxx) + +// After: +const rawKey = req[API_KEY_SYMBOL] // Symbol.for('0http.apiKey') +const maskedKey = req.apiKey // 'xxxx****xxxx' (safe for logging) +``` + +### Empty JSON Body + +If your code relied on empty JSON request bodies being parsed as `{}`, update it to handle `undefined`: + +```typescript +// Before (empty body → {}): +router.post('/api/data', (req) => { + const keys = Object.keys(req.body) // Always worked +}) + +// After (empty body → undefined): +router.post('/api/data', (req) => { + const body = req.body || {} + const keys = Object.keys(body) // Handle undefined +}) +``` + +### Raw Body Access + +If you accessed raw body text via `req._rawBodyText`, use the exported `RAW_BODY_SYMBOL` instead: + +```typescript +import {RAW_BODY_SYMBOL} from '0http-bun/lib/middleware/body-parser' + +// Before: +const rawBody = req._rawBodyText // Public string property + +// After: +const rawBody = req[RAW_BODY_SYMBOL] // Symbol.for('0http.rawBody') +``` + +--- + ## License MIT diff --git a/SECURITY_REVIEWS.md b/SECURITY_REVIEWS.md new file mode 100644 index 0000000..ac39794 --- /dev/null +++ b/SECURITY_REVIEWS.md @@ -0,0 +1,361 @@ +# Security Remediation TODO + +> Penetration test conducted on 2025-02-07 against `0http-bun@1.2.2`. +> **43 vulnerabilities found** — 6 Critical, 13 High, 13 Medium, 7 Low, 4 Info. +> **Overall Security Grade: D → B+ — All identified vulnerabilities resolved.** + +## Remediation Progress + +> **Fixed:** 6/6 Critical, 13/13 High, 13/13 Medium, 7/7 Low, 4/4 Info = **43/43 vulnerabilities resolved** ✅ + +--- + +## CRITICAL (Must fix immediately) + +### ✅ CRIT-1: Unbounded Route Cache — Memory Exhaustion DoS + +- **Status:** FIXED +- **File:** `lib/router/sequential.js` +- **Fix applied:** Added LRU-style cache eviction with configurable `config.cacheSize` (default 1000). When cache exceeds max size, oldest entries are evicted. + +--- + +### ✅ CRIT-2: JSON Nesting Depth Check Bypass via String Content + +- **Status:** FIXED +- **File:** `lib/middleware/body-parser.js` +- **Fix applied:** Nesting depth scanner now tracks `inString`/`escape` state to correctly ignore braces inside JSON string literals. + +--- + +### ✅ CRIT-3: Rate Limit Bypass via Spoofable IP Headers + +- **Status:** FIXED — ⚠️ BREAKING CHANGE +- **File:** `lib/middleware/rate-limit.js` +- **Fix applied:** `defaultKeyGenerator` now uses `req.ip || req.remoteAddress || 'unknown'` — no proxy headers trusted by default. Users behind reverse proxies must supply a custom `keyGenerator`. + +--- + +### ✅ CRIT-4: Rate Limit Store Injection via `req.rateLimitStore` + +- **Status:** FIXED +- **File:** `lib/middleware/rate-limit.js` +- **Fix applied:** Removed `req.rateLimitStore` backdoor. Always uses constructor-configured store. + +--- + +### ✅ CRIT-5: API Key Timing Side-Channel Attack + +- **Status:** FIXED +- **File:** `lib/middleware/jwt-auth.js` +- **Fix applied:** Added `crypto.timingSafeEqual()` for all API key comparisons with constant-time length mismatch handling. + +--- + +### ✅ CRIT-6: JWT Default Algorithms Enable Algorithm Confusion + +- **Status:** FIXED — ⚠️ BREAKING CHANGE +- **File:** `lib/middleware/jwt-auth.js` +- **Fix applied:** Default algorithms changed from `['HS256', 'RS256']` to `['HS256']`. Throws error if mixed symmetric+asymmetric algorithms are configured. + +--- + +## HIGH (Must fix before next release) + +### ✅ H-1: Default Error Handler Leaks `err.message` to Clients + +- **Status:** FIXED +- **File:** `lib/router/sequential.js` +- **Fix applied:** Default error handler now returns generic `"Internal Server Error"` and logs full error via `console.error`. + +--- + +### ✅ H-2: `this` Binding Bug in `router.use()` Breaks Middleware Chaining + +- **Status:** FIXED +- **File:** `lib/router/sequential.js` +- **Fix applied:** Changed `return this` → `return router` in arrow function `router.use()`. + +--- + +### ✅ H-3: Content-Length Bypass — Full Body Read Before Size Enforcement + +- **Status:** FIXED +- **File:** `lib/middleware/body-parser.js` +- **Fix applied:** Added `readBodyWithLimit()` streaming helper that reads body chunks incrementally and aborts immediately when cumulative size exceeds the limit. Replaces `await req.text()` + post-hoc size check in JSON, text, URL-encoded, and custom JSON parser paths. + +--- + +### ✅ H-4: Custom `jsonParser` Bypasses All Security Controls + +- **Status:** FIXED +- **File:** `lib/middleware/body-parser.js` +- **Fix applied:** Custom `jsonParser` now enforces size limits before calling the custom parser function. + +--- + +### ✅ H-5: Multipart Parser Reads Entire Body Before Validation + Double Memory + +- **Status:** FIXED +- **File:** `lib/middleware/body-parser.js` +- **Fix applied:** Merged the validation and extraction loops into a single pass. Field count, size limits, filename sanitization, and prototype pollution checks all run in one iteration of `formData.entries()` instead of two. + +--- + +### ✅ H-6: Multipart Parser Has No Prototype Pollution Protection + +- **Status:** FIXED +- **File:** `lib/middleware/body-parser.js` +- **Fix applied:** Uses `Object.create(null)` for body/files objects and added prototype pollution blocklist for field names. + +--- + +### ✅ H-7: JWT Path Exclusion Bypass via Prefix Collision + +- **Status:** FIXED +- **File:** `lib/middleware/jwt-auth.js` +- **Fix applied:** Path exclusion now uses exact match or `path + '/'` boundary checking. + +--- + +### ✅ H-8: JWT Token in Query Parameters Leaks via Logs/Referrer + +- **Status:** FIXED +- **File:** `lib/middleware/jwt-auth.js` +- **Fix applied:** Emits a `console.warn` deprecation warning at middleware construction time when `tokenQuery` is configured, advising migration to Authorization header or custom `getToken`. + +--- + +### ✅ H-9: Optional JWT Mode Silently Swallows Invalid/Forged Tokens + +- **Status:** FIXED +- **File:** `lib/middleware/jwt-auth.js` +- **Fix applied:** Now sets `req.ctx.authError` and `req.ctx.authAttempted = true` on invalid tokens in optional mode. + +--- + +### ✅ H-10: Sliding Window Rate Limiter — Unbounded Map Growth + +- **Status:** FIXED +- **File:** `lib/middleware/rate-limit.js` +- **Fix applied:** Added `maxKeys` (default 10000), periodic cleanup with `setInterval` + `unref()`, and eviction of stale entries. + +--- + +### ✅ H-11: CORS `null` Origin Bypass via Sandboxed Iframe + +- **Status:** FIXED +- **File:** `lib/middleware/cors.js` +- **Fix applied:** `null`/missing origins are now rejected before calling validator function or checking array. + +--- + +### ✅ H-12: Rate Limiter TOCTOU Race Condition + +- **Status:** FIXED +- **File:** `lib/middleware/rate-limit.js` +- **Fix applied:** `MemoryStore.increment` changed from `async` to synchronous. + +--- + +### ✅ H-13: Async Errors Bypass Custom Error Handler + +- **Status:** FIXED +- **File:** `lib/next.js` +- **Fix applied:** Async middleware errors now caught via `.catch()` on returned promises and forwarded to `errorHandler`. + +--- + +## MEDIUM (Should fix soon) + +### ✅ M-1: Shared Mutable `emptyParams` — Cross-Request Data Leakage + +- **Status:** FIXED +- **File:** `lib/router/sequential.js` +- **Fix applied:** `emptyParams` now uses `Object.freeze({})`. + +--- + +### ✅ M-2: No URL Path Normalization — Route Filter Bypass + +- **Status:** FIXED +- **File:** `lib/router/sequential.js` +- **Fix applied:** Added URL path normalization (double-slash collapse, `decodeURIComponent`, preserves `%2F`). + +--- + +### ✅ M-3: Multipart Filename Not Sanitized for Path Traversal + +- **Status:** FIXED +- **File:** `lib/middleware/body-parser.js` +- **Fix applied:** Filenames sanitized (strips `..`, path separators, null bytes; preserves `originalName`). + +--- + +### ✅ M-4: Content-Type Broad Matching Routes Non-JSON to JSON Parser + +- **Status:** FIXED +- **File:** `lib/middleware/body-parser.js` +- **Fix applied:** Internal JSON parser type changed from `'application/'` to `'application/json'`. + +--- + +### ✅ M-5: `parseLimit` Silently Defaults for Unexpected Types + +- **Status:** FIXED — ⚠️ BREAKING CHANGE +- **File:** `lib/middleware/body-parser.js` +- **Fix applied:** Now throws `TypeError` on unexpected types instead of silently defaulting. + +--- + +### ✅ M-6: Raw JWT Token Stored on Request Context + +- **Status:** FIXED +- **File:** `lib/middleware/jwt-auth.js` +- **Fix applied:** Raw JWT token removed from `req.ctx.jwt` and `req.jwt`. + +--- + +### ✅ M-7: API Key Stored in Plain Text on Request Context + +- **Status:** FIXED — ⚠️ BREAKING CHANGE +- **File:** `lib/middleware/jwt-auth.js` +- **Fix applied:** `req.ctx.apiKey` and `req.apiKey` now store a masked version (`xxxx****xxxx`). Raw key is available only via `req[API_KEY_SYMBOL]` (exported `Symbol.for('0http.apiKey')`) to prevent accidental serialization/logging. + +--- + +### ✅ M-8: JWKS URI Not Validated (Allows HTTP) + +- **Status:** FIXED +- **File:** `lib/middleware/jwt-auth.js` +- **Fix applied:** JWKS URI is validated at construction time. Throws an error if non-HTTPS URI is used when `NODE_ENV=production`. Emits a `console.warn` in non-production environments. + +--- + +### ✅ M-9: JWT Error Messages Reveal Validation Oracle + +- **Status:** FIXED +- **File:** `lib/middleware/jwt-auth.js` +- **Fix applied:** Unified JWT error messages to `'Invalid or expired token'`. + +--- + +### ✅ M-10: `jwtOptions` Spread Overrides Security-Critical Defaults + +- **Status:** FIXED +- **File:** `lib/middleware/jwt-auth.js` +- **Fix applied:** `...jwtOptions` spread now applied first, security-critical options override after. + +--- + +### ✅ M-11: Rate Limit `excludePaths` Uses Prefix Matching + +- **Status:** FIXED +- **File:** `lib/middleware/rate-limit.js` +- **Fix applied:** `excludePaths` uses exact or boundary matching. + +--- + +### ✅ M-12: CORS Headers Leak Method/Header Lists When Origin Rejected + +- **Status:** FIXED +- **File:** `lib/middleware/cors.js` +- **Fix applied:** CORS headers (methods, allowed headers, credentials, exposed headers) only set when origin is allowed. + +--- + +### ✅ M-13: `apiKeyValidator.length` Arity Check Is Unreliable + +- **Status:** FIXED +- **File:** `lib/middleware/jwt-auth.js` +- **Fix applied:** Removed unreliable `Function.length` arity check; always calls validators with `(apiKey, req)`. + +--- + +## LOW (Consider fixing) + +### ✅ L-1: `fast-querystring` `__proto__` in Query Can Pollute Downstream + +- **Status:** FIXED +- **File:** `lib/router/sequential.js` +- **Fix applied:** After parsing query string, dangerous keys (`__proto__`, `constructor`, `prototype`) are deleted from `req.query`, mirroring the existing protection on `req.params`. + +### ✅ L-2: Empty JSON Body Silently Becomes `{}` + +- **Status:** FIXED — ⚠️ BREAKING CHANGE +- **File:** `lib/middleware/body-parser.js` +- **Fix applied:** Empty or whitespace-only JSON bodies now set `req.body` to `undefined` instead of `{}`, letting the application decide the semantics. + +### ✅ L-3: `_rawBodyText` Publicly Accessible + +- **Status:** FIXED +- **File:** `lib/middleware/body-parser.js` +- **Fix applied:** Raw body text now stored via `Symbol.for('0http.rawBody')` (`RAW_BODY_SYMBOL`) instead of `req._rawBodyText`, preventing accidental serialization/logging. Symbol is exported for programmatic access. + +### ✅ L-4: Missing JWT Token Type (`typ`) Header Validation + +- **Status:** FIXED +- **File:** `lib/middleware/jwt-auth.js` +- **Fix applied:** Added `requiredTokenType` option to `createJWTAuth`. After successful JWT verification, validates the `typ` header claim (case-insensitive). Rejects tokens with missing or incorrect type when configured. + +### ✅ L-5: Internal Functions Exported, Expanding Attack Surface + +- **Status:** FIXED +- **File:** `lib/middleware/jwt-auth.js` +- **Fix applied:** Internal functions removed from module exports. + +### ✅ L-6: Rate Limit Headers Disclose Exact Config/Counters + +- **Status:** FIXED +- **File:** `lib/middleware/rate-limit.js` +- **Fix applied:** `standardHeaders` now accepts `true` (full headers — default, backward compatible), `false` (no headers), or `'minimal'` (only `Retry-After` on 429 responses). Minimal mode prevents disclosing exact rate limit configuration and usage counters. + +### ✅ L-7: Missing `Vary: Origin` for Static String CORS Origins + +- **Status:** FIXED +- **File:** `lib/middleware/cors.js` +- **Fix applied:** `Vary: Origin` now set for all non-wildcard origins (not just function/array). + +--- + +## INFO (Awareness) + +- ✅ **I-1:** `'unknown'` fallback in rate limiter now generates unique per-request keys to prevent shared bucket DoS; logs a security warning on first occurrence (`rate-limit.js`) — FIXED +- ✅ **I-2:** `allowedHeaders` function now resolved once per preflight request instead of 3 times, preventing inconsistency and wasted resources (`cors.js`) — FIXED +- ✅ **I-3:** Pointless `try/catch` blocks that only re-throw in body parser — FIXED (removed) +- ✅ **I-4:** Empty `catch` blocks in JWT `handleAuthError` now log errors via `console.error` to aid debugging (`jwt-auth.js`) — FIXED + +--- + +## Positive Observations + +- Prototype pollution protection on route params is well-implemented with `hasOwnProperty.call()` guard +- `fast-querystring` uses null-prototype objects for query results +- Security test suite exists for prototype pollution scenarios +- Body parser has comprehensive limits (parameter counts, field counts, key lengths) +- JWT error messages are partially sanitized (enumerated messages, not raw errors) +- CORS correctly blocks `credentials: true` + `origin: '*'` combination +- Lazy loading of heavy dependencies reduces startup attack surface +- `parseLimit` regex is anchored and bounded (not vulnerable to ReDoS) + +--- + +## Breaking Changes Summary + +| Vulnerability | Breaking Change | Impact | +| ------------- | ----------------------------------------------------------------------- | ------------------------------------------------------------------- | +| CRIT-3 | Default rate limit key generator no longer trusts proxy headers | Users behind reverse proxies must supply custom `keyGenerator` | +| CRIT-6 | Default JWT algorithms changed from `['HS256', 'RS256']` to `['HS256']` | Users relying on RS256 must explicitly configure algorithms | +| M-5 | `parseLimit` throws `TypeError` on invalid types | Code passing `false`, `null`, or objects to `parseLimit` will throw | +| M-7 | `req.ctx.apiKey` / `req.apiKey` now stores masked value | Code reading raw API key must use `req[API_KEY_SYMBOL]` instead | +| L-2 | Empty JSON body now sets `req.body` to `undefined` instead of `{}` | Code checking `req.body` for empty JSON must handle `undefined` | + +--- + +## Suggested Next Steps + +1. ~~**Fix remaining HIGH issues:** H-3 (streaming body read), H-5 (single-pass multipart), H-8 (query token deprecation)~~ ✅ All HIGH issues fixed +2. ~~**Fix remaining MEDIUM issues:** M-7 (mask API key in context), M-8 (JWKS HTTPS validation)~~ ✅ All MEDIUM issues fixed +3. ~~**Address LOW/INFO issues** as part of ongoing maintenance~~ ✅ All LOW and INFO issues fixed +4. **Write security regression tests** for each fix ✅ Completed — 483 tests passing +5. **Bump version to 1.3.0** with security changelog diff --git a/bench.ts b/bench.ts index 9917572..ee92bbf 100644 --- a/bench.ts +++ b/bench.ts @@ -1,16 +1,19 @@ import {run, bench, group} from 'mitata' -import httpNext from './index' +import httpNext, {IRouter} from './index' import httpPrevious from '0http-bun' -function setupRouter(router) { - router.use((req, next) => { +const silentErrorHandler = () => + new Response('Internal Server Error', {status: 500}) + +function setupRouter(router: any) { + router.use((req: any, next: () => any) => { return next() }) router.get('/', () => { return new Response() }) - router.get('/:id', async (req) => { + router.get('/:id', async (req: {params: Record}) => { return new Response(req.params.id) }) router.get('/:id/error', () => { @@ -18,35 +21,30 @@ function setupRouter(router) { }) } -const {router} = httpNext() -setupRouter(router) +function benchRouter(name: string, router: any) { + group(name, () => { + bench('Parameter URL', async () => { + await router.fetch(new Request(new URL('http://localhost/0'))) + }).gc('inner') + bench('Not Found URL', async () => { + await router.fetch(new Request(new URL('http://localhost/0/404'))) + }).gc('inner') + bench('Error URL', async () => { + await router.fetch(new Request(new URL('http://localhost/0/error'))) + }).gc('inner') + }) +} -const {router: routerPrevious} = httpPrevious() -setupRouter(routerPrevious) +const {router} = httpNext({errorHandler: silentErrorHandler}) +setupRouter(router) -group('Next Router', () => { - bench('Parameter URL', () => { - router.fetch(new Request(new URL('http://localhost/0'))) - }).gc('inner') - bench('Not Found URL', () => { - router.fetch(new Request(new URL('http://localhost/0/404'))) - }).gc('inner') - bench('Error URL', () => { - router.fetch(new Request(new URL('http://localhost/0/error'))) - }).gc('inner') +const {router: routerPrevious} = httpPrevious({ + errorHandler: silentErrorHandler, }) +setupRouter(routerPrevious) -group('Previous Router', () => { - bench('Parameter URL', () => { - routerPrevious.fetch(new Request(new URL('http://localhost/0'))) - }).gc('inner') - bench('Not Found URL', () => { - routerPrevious.fetch(new Request(new URL('http://localhost/0/404'))) - }).gc('inner') - bench('Error URL', () => { - routerPrevious.fetch(new Request(new URL('http://localhost/0/error'))) - }).gc('inner') -}) +benchRouter('Next Router', router) +benchRouter('Previous Router', routerPrevious) run({ colors: true, diff --git a/common.d.ts b/common.d.ts index 0d7ccd4..fe27523 100644 --- a/common.d.ts +++ b/common.d.ts @@ -2,6 +2,7 @@ import {Pattern, Methods} from 'trouter' import {Logger} from 'pino' export interface IRouterConfig { + cacheSize?: number defaultRoute?: RequestHandler errorHandler?: (err: Error) => Response | Promise port?: number @@ -19,6 +20,19 @@ export interface ParsedFile { export type ZeroRequest = Request & { params: Record query: Record + // Connection-level IP address (set via Bun.serve's server.requestIP or upstream middleware) + ip?: string + remoteAddress?: string + socket?: { + remoteAddress?: string + } + // Rate limit info (set by rate-limit middleware) + rateLimit?: { + limit: number + remaining: number + current: number + reset: Date + } // Legacy compatibility properties (mirrored from ctx) user?: any jwt?: { @@ -42,6 +56,8 @@ export type ZeroRequest = Request & { used: number remaining: number resetTime: Date + current: number + reset: Date } body?: any files?: Record diff --git a/lib/middleware/README.md b/lib/middleware/README.md index a3c20d9..a7ee14e 100644 --- a/lib/middleware/README.md +++ b/lib/middleware/README.md @@ -166,12 +166,50 @@ router.use(createBodyParser(bodyParserOptions)) **Supported Content Types:** -- `application/json` - Parsed as JSON +- `application/json` - Parsed as JSON (strict matching; `application/xml` and other `application/*` types are no longer routed to the JSON parser) - `application/x-www-form-urlencoded` - Parsed as form data - `multipart/form-data` - Parsed as FormData - `text/*` - Parsed as plain text - `application/octet-stream` - Parsed as ArrayBuffer +**Security Features:** + +- **JSON nesting depth limit** (max 100) with string-aware scanning — brace characters inside JSON strings do not count toward the depth limit +- **Prototype pollution protection** — multipart body and files objects use `Object.create(null)`, and dangerous property names (`__proto__`, `constructor`, `prototype`, etc.) are blocked +- **Multipart filename sanitization** — strips `..`, path separators (`/`, `\`), null bytes, and leading dots. The original filename is preserved in `file.originalName` +- **Size limit validation** — `parseLimit()` throws `TypeError` for unexpected types (e.g., `null`, `false`, objects) instead of silently defaulting to 1MB +- **Custom `jsonParser` safety** — when a custom `jsonParser` function is provided, size limits are enforced before the parser is called +- **Empty body handling** — empty or whitespace-only JSON bodies set `req.body` to `undefined` instead of silently returning `{}` +- **Raw body via Symbol** — raw body text is stored via a Symbol (`RAW_BODY_SYMBOL`) instead of a public string property, preventing accidental serialization/logging + +**Accessing Raw Body:** + +```javascript +import {RAW_BODY_SYMBOL} from '0http-bun/lib/middleware/body-parser' + +router.post('/webhook', (req) => { + const rawBody = req[RAW_BODY_SYMBOL] // Symbol.for('0http.rawBody') + // Use rawBody for signature verification, etc. +}) +``` + +**Verify Function:** + +When using the `verify` option with `createBodyParser`, the raw body is available via the same symbol: + +```javascript +const bodyParser = createBodyParser({ + verify: (req, rawBody) => { + // rawBody is the raw string before parsing + const signature = req.headers.get('x-signature') + if (!verifySignature(rawBody, signature)) { + throw new Error('Invalid signature') + } + }, + // Note: any JSON parsing behavior (such as deferNext) is handled internally when verify is set. +}) +``` + ### CORS Cross-Origin Resource Sharing middleware with flexible configuration. @@ -199,10 +237,13 @@ router.use( ) // Dynamic origin validation +// NOTE: null and missing origins are automatically rejected before calling the +// validator function, preventing bypass via sandboxed iframes. router.use( createCORS({ origin: (origin, req) => { // Custom logic to validate origin + // `origin` is guaranteed to be a non-null, non-'null' string here return ( origin?.endsWith('.mycompany.com') || origin === 'http://localhost:3000' ) @@ -211,6 +252,13 @@ router.use( ) ``` +**Security behavior:** + +- When an origin is **not allowed**, CORS headers (methods, allowed headers, credentials, exposed headers) are **not set** on the response. Only `Vary: Origin` is added. +- `null` origins (from sandboxed iframes, `file://` URLs, etc.) are **rejected** for array and function origin configurations. +- `Vary: Origin` is set for all non-wildcard origin configurations to prevent CDN cache poisoning. +- Wildcard (`*`) origins with `credentials: true` are blocked (CORS spec requirement). + **TypeScript Usage:** ```typescript @@ -329,7 +377,9 @@ router.use( jwksUri: process.env.JWKS_URI, // JWT verification options - algorithms: ['HS256', 'RS256'], + // IMPORTANT: Do NOT mix symmetric (HS*) and asymmetric (RS*/ES*/PS*) algorithms. + // Use HS256 with static secrets, or RS256/ES256 with JWKS URIs. + algorithms: ['HS256'], issuer: 'your-app', audience: 'your-users', clockTolerance: 10, // Clock skew tolerance (seconds) @@ -341,14 +391,13 @@ router.use( // Try multiple sources return ( req.headers.get('x-auth-token') || - req.headers.get('authorization')?.replace('Bearer ', '') || - new URL(req.url).searchParams.get('token') + req.headers.get('authorization')?.replace('Bearer ', '') ) }, // Alternative token sources tokenHeader: 'x-custom-token', // Custom header name - tokenQuery: 'access_token', // Query parameter name + tokenQuery: 'access_token', // Query parameter name (see security note below) // Error handling onError: (err, req) => { @@ -377,14 +426,22 @@ router.use( }, // Optional authentication (proceed even without token) + // When optional: true, invalid tokens set req.ctx.authError and req.ctx.authAttempted = true optional: false, - // Exclude certain paths + // Exclude certain paths (uses exact match or path boundary, NOT prefix matching) + // e.g., '/health' excludes '/health' and '/health/...' but NOT '/healthcheck' excludePaths: ['/health', '/metrics', '/api/public'], + + // Token type validation (validates the JWT 'typ' header claim) + // Case-insensitive comparison. Rejects tokens with missing or incorrect type. + requiredTokenType: 'JWT', // or 'at+jwt' for access tokens, etc. }), ) ``` +> **Security Note on `tokenQuery`:** Passing JWTs via query parameters exposes tokens in server logs, browser history, and HTTP `Referer` headers. Prefer header-based token extraction in production. + #### API Key Authentication The JWT middleware also supports API key authentication as an alternative or fallback: @@ -399,6 +456,14 @@ router.use( }), ) +// Access the raw API key (req.apiKey is masked for security) +import {API_KEY_SYMBOL} from '0http-bun/lib/middleware/jwt-auth' + +router.get('/api/data', (req) => { + console.log(req.apiKey) // 'xxxx****xxxx' (masked, safe for logging) + console.log(req[API_KEY_SYMBOL]) // Raw API key via Symbol.for('0http.apiKey') +}) + // API key with custom validation router.use( '/api/*', @@ -457,15 +522,22 @@ router.use('/api/protected/*', createJWTAuth(jwtConfig)) ```typescript // Access decoded token in route handlers router.get('/api/profile', (req) => { - // Multiple ways to access user data + // Decoded JWT payload console.log(req.user) // Decoded JWT payload console.log(req.ctx.user) // Same as req.user - console.log(req.jwt) // Full JWT info (payload, header, token) + + // JWT header and payload (raw token is NOT included for security) + console.log(req.jwt) // { payload, header } console.log(req.ctx.jwt) // Same as req.jwt // API key authentication data (if used) - console.log(req.apiKey) // API key value - console.log(req.ctx.apiKey) // Same as req.apiKey + console.log(req.apiKey) // Masked API key (xxxx****xxxx) + console.log(req.ctx.apiKey) // Same as req.apiKey (masked) + + // Optional mode: check if auth was attempted but failed + if (req.ctx.authAttempted && req.ctx.authError) { + console.log('Auth failed:', req.ctx.authError) + } return Response.json({ user: req.user, @@ -618,9 +690,7 @@ const prometheus = createPrometheusIntegration({ #### Custom Business Metrics ```javascript -const { - createPrometheusIntegration, -} = require('0http-bun/lib/middleware') +const {createPrometheusIntegration} = require('0http-bun/lib/middleware') // Get the prometheus client from the integration const prometheus = createPrometheusIntegration() @@ -783,8 +853,16 @@ router.use( windowMs: 60 * 1000, // 1 minute max: 20, // Max requests keyGenerator: (req) => { - // Custom key generation (default: IP address) - return req.headers.get('x-user-id') || req.headers.get('x-forwarded-for') + // Custom key generation + // Default uses: req.ip || req.remoteAddress || req.socket?.remoteAddress || 'unknown' + // NOTE: Proxy headers are NOT trusted by default. If behind a + // reverse proxy, you MUST provide a custom keyGenerator: + return ( + req.headers.get('x-user-id') || + req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() || + req.ip || + 'unknown' + ) }, skip: (req) => { // Skip rate limiting for certain requests @@ -802,6 +880,12 @@ router.use( ) }, standardHeaders: true, // Send X-RateLimit-* headers + // standardHeaders options: + // true - full headers (X-RateLimit-Limit, Remaining, Reset, Used) — default + // false - no rate limit headers + // 'minimal' - only Retry-After on 429 responses (hides exact config/counters) + // excludePaths uses exact match or boundary matching (NOT prefix) + // e.g., '/health' matches '/health' and '/health/...' but NOT '/healthcheck' excludePaths: ['/health', '/metrics'], }), ) @@ -811,6 +895,8 @@ router.use( createRateLimit({ store: new MemoryStore(), // Built-in memory store // Or implement custom store with increment() method + // Note: The store is always the constructor-configured instance. + // It cannot be overridden at runtime via the request object. }), ) ``` @@ -825,10 +911,12 @@ const rateLimitOptions: RateLimitOptions = { windowMs: 15 * 60 * 1000, // 15 minutes max: 100, keyGenerator: (req) => { + // Default: req.ip || req.remoteAddress || req.socket?.remoteAddress || 'unknown' + // Custom: read from proxy-set header if behind a reverse proxy return ( - req.headers.get('x-user-id') || - req.headers.get('x-forwarded-for') || - 'anonymous' + req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() || + req.ip || + 'unknown' ) }, standardHeaders: true, @@ -839,6 +927,8 @@ router.use(createRateLimit(rateLimitOptions)) // Custom store implementation class CustomStore implements RateLimitStore { + // Note: For MemoryStore, increment is synchronous to prevent TOCTOU races. + // Custom stores may be async if using external backends (e.g., Redis). async increment( key: string, windowMs: number, @@ -861,11 +951,15 @@ router.use( createSlidingWindowRateLimit({ windowMs: 60 * 1000, // 1 minute sliding window max: 10, // Max 10 requests per minute - keyGenerator: (req) => req.headers.get('x-forwarded-for') || 'default', + maxKeys: 10000, // Maximum tracked keys (default: 10000) — prevents unbounded memory growth + keyGenerator: (req) => + req.ip || req.remoteAddress || req.socket?.remoteAddress || 'unknown', }), ) ``` +> **Memory safety:** The sliding window rate limiter enforces a `maxKeys` limit (default: 10,000). When the limit is exceeded, the oldest entry is evicted. A periodic cleanup interval runs at most every 60 seconds (or `windowMs`, whichever is shorter) and uses `unref()` so it doesn't prevent process exit. + **TypeScript Usage:** ```typescript @@ -875,7 +969,8 @@ import type {RateLimitOptions} from '0http-bun/lib/middleware' const slidingOptions: RateLimitOptions = { windowMs: 60 * 1000, // 1 minute max: 10, // 10 requests max - keyGenerator: (req) => req.user?.id || req.headers.get('x-forwarded-for'), + maxKeys: 10000, // Max tracked keys (default: 10000) + keyGenerator: (req) => req.user?.id || req.ip || 'unknown', handler: (req, hits, max, resetTime) => { return Response.json( { @@ -920,7 +1015,8 @@ router.use( createSlidingWindowRateLimit({ windowMs: 3600 * 1000, // 1 hour max: 3, // 3 accounts per IP per hour - keyGenerator: (req) => req.headers.get('x-forwarded-for'), + keyGenerator: (req) => + req.ip || req.remoteAddress || req.socket?.remoteAddress || 'unknown', }), ) @@ -937,8 +1033,9 @@ router.use( **Performance Considerations:** -- **Memory Usage**: Higher than fixed window (stores timestamp arrays) +- **Memory Usage**: Higher than fixed window (stores timestamp arrays), bounded by `maxKeys` (default: 10,000) - **Time Complexity**: O(n) per request where n = requests in window +- **Cleanup**: Automatic periodic cleanup runs at most every 60 seconds; interval uses `unref()` to not prevent process exit - **Best For**: Critical APIs, financial transactions, user-facing features - **Use Fixed Window For**: High-volume APIs where approximate limiting is acceptable @@ -966,6 +1063,8 @@ Both rate limiters send the following headers when `standardHeaders: true`: - `X-RateLimit-Reset` - Reset time (Unix timestamp) - `X-RateLimit-Used` - Used requests +When `standardHeaders: 'minimal'`, only `Retry-After` is sent on 429 responses. This prevents disclosing exact rate limit configuration and usage counters to clients. + **Error Handling:** Rate limiting middleware allows errors to bubble up as proper HTTP 500 responses. If your `keyGenerator` function or custom `store.increment()` method throws an error, it will not be caught and masked - the error will propagate up the middleware chain for proper error handling. @@ -1102,10 +1201,12 @@ router.use( router.use(createLogger({format: 'combined'})) // 3. Rate limiting (protect against abuse) +// NOTE: Default key generator uses req.ip — provide keyGenerator if behind a proxy router.use( createRateLimit({ windowMs: 15 * 60 * 1000, max: 1000, + // keyGenerator: (req) => req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() || req.ip || 'unknown', }), ) @@ -1113,11 +1214,11 @@ router.use( router.use(createBodyParser({limit: '10mb'})) // 5. Authentication (protect API routes) +// Default algorithms: ['HS256'] — specify ['RS256'] for asymmetric keys router.use( '/api/*', createJWTAuth({ secret: process.env.JWT_SECRET, - skip: (req) => req.url.includes('/api/public/'), }), ) diff --git a/lib/middleware/body-parser.js b/lib/middleware/body-parser.js index 2b81da1..b32310a 100644 --- a/lib/middleware/body-parser.js +++ b/lib/middleware/body-parser.js @@ -12,6 +12,87 @@ * - Error message sanitization to prevent information leakage */ +/** + * Symbol key for storing raw body text to prevent accidental serialization/logging (L-3 fix) + */ +const RAW_BODY_SYMBOL = Symbol.for('0http.rawBody') + +/** + * Keys that must be blocked to prevent prototype pollution attacks. + * Shared across all parsers (URL-encoded, multipart, nested key parsing). + */ +const PROTOTYPE_POLLUTION_KEYS = new Set([ + '__proto__', + 'constructor', + 'prototype', + 'hasOwnProperty', + 'isPrototypeOf', + 'propertyIsEnumerable', + 'valueOf', + 'toString', +]) + +/** + * Reads request body as text with inline size enforcement. + * Streams the body and aborts if the accumulated size exceeds the limit, + * preventing full memory allocation for oversized payloads (H-3 fix). + * @param {Request} req - Request object + * @param {number} maxBytes - Maximum allowed body size in bytes + * @returns {Promise} Body text + * @throws {Object} Object with `status` and `message` on limit exceeded + */ +async function readBodyWithLimit(req, maxBytes) { + // If the request has no body stream, fall back to req.text() + if (!req.body || typeof req.body.getReader !== 'function') { + const text = await req.text() + const byteLength = new TextEncoder().encode(text).length + if (byteLength > maxBytes) { + const err = new Error('Request body size exceeded') + err.status = 413 + throw err + } + return text + } + + const reader = req.body.getReader() + const chunks = [] + let totalBytes = 0 + + try { + while (true) { + const {done, value} = await reader.read() + if (done) break + + totalBytes += value.byteLength + if (totalBytes > maxBytes) { + reader.cancel() + const err = new Error('Request body size exceeded') + err.status = 413 + throw err + } + chunks.push(value) + } + } catch (e) { + // Re-throw size limit errors + if (e.status === 413) throw e + // Wrap other stream errors + const err = new Error('Failed to read request body') + err.status = 400 + throw err + } + + // Concatenate chunks and decode to string + const totalLength = chunks.reduce((sum, chunk) => sum + chunk.byteLength, 0) + const combined = new Uint8Array(totalLength) + let offset = 0 + for (const chunk of chunks) { + combined.set(chunk, offset) + offset += chunk.byteLength + } + + return new TextDecoder().decode(combined) +} + /** * Parses size limit strings with suffixes (e.g. '500b', '1kb', '2mb') * @param {number|string} limit - Size limit @@ -64,7 +145,9 @@ function parseLimit(limit) { return Math.min(bytes, 1024 * 1024 * 1024) // Max 1GB } - return limit || 1024 * 1024 // Default 1MB + throw new TypeError( + `Invalid limit type: expected number or string, got ${typeof limit}`, + ) } /** @@ -113,19 +196,7 @@ function parseNestedKey(obj, key, value, depth = 0) { throw new Error('Maximum nesting depth exceeded') } - // Protect against prototype pollution - const prototypePollutionKeys = [ - '__proto__', - 'constructor', - 'prototype', - 'hasOwnProperty', - 'isPrototypeOf', - 'propertyIsEnumerable', - 'valueOf', - 'toString', - ] - - if (prototypePollutionKeys.includes(key)) { + if (PROTOTYPE_POLLUTION_KEYS.has(key)) { return // Silently ignore dangerous keys } @@ -138,7 +209,7 @@ function parseNestedKey(obj, key, value, depth = 0) { const [, baseKey, indexKey, remaining] = match // Protect against prototype pollution on base key - if (prototypePollutionKeys.includes(baseKey)) { + if (PROTOTYPE_POLLUTION_KEYS.has(baseKey)) { return } @@ -161,7 +232,7 @@ function parseNestedKey(obj, key, value, depth = 0) { } } else { // Protect against prototype pollution on index key - if (!prototypePollutionKeys.includes(indexKey)) { + if (!PROTOTYPE_POLLUTION_KEYS.has(indexKey)) { obj[baseKey][indexKey] = value } } @@ -194,90 +265,104 @@ function createJSONParser(options = {}) { return deferNext ? null : next() } - try { - const contentLength = req.headers.get('content-length') - if (contentLength) { - const length = parseInt(contentLength) - if (isNaN(length) || length < 0) { - return new Response('Invalid content-length header', {status: 400}) - } - if (length > parsedLimit) { - return new Response('Request body size exceeded', {status: 413}) - } + const contentLength = req.headers.get('content-length') + if (contentLength) { + const length = parseInt(contentLength) + if (isNaN(length) || length < 0) { + return new Response('Invalid content-length header', {status: 400}) } - - // Check if the request has a null body (no body was provided) - if (req.body === null) { - Object.defineProperty(req, 'body', { - value: undefined, - writable: true, - enumerable: true, - configurable: true, - }) - return deferNext ? null : next() + if (length > parsedLimit) { + return new Response('Request body size exceeded', {status: 413}) } + } - const text = await req.text() - // Store raw body text for verification - req._rawBodyText = text + // Check if the request has a null body (no body was provided) + if (req.body === null) { + Object.defineProperty(req, 'body', { + value: undefined, + writable: true, + enumerable: true, + configurable: true, + }) + return deferNext ? null : next() + } - // Validate text length to prevent memory exhaustion - const textLength = new TextEncoder().encode(text).length - if (textLength > parsedLimit) { + let text + try { + text = await readBodyWithLimit(req, parsedLimit) + } catch (e) { + if (e.status === 413) { return new Response('Request body size exceeded', {status: 413}) } - - // Additional protection against excessively deep nesting - if (text.length > 0) { - // Count nesting levels to prevent stack overflow during parsing - let nestingLevel = 0 - let maxNesting = 0 - for (let i = 0; i < text.length; i++) { - if (text[i] === '{' || text[i] === '[') { - nestingLevel++ - maxNesting = Math.max(maxNesting, nestingLevel) - } else if (text[i] === '}' || text[i] === ']') { - nestingLevel-- - } + return new Response('Failed to read request body', {status: 400}) + } + // Store raw body text for verification (L-3: use Symbol to prevent accidental serialization) + req[RAW_BODY_SYMBOL] = text + + // Additional protection against excessively deep nesting + if (text.length > 0) { + // Count nesting levels with string-awareness to prevent bypass via string content + let nestingLevel = 0 + let maxNesting = 0 + let inString = false + let escape = false + for (let i = 0; i < text.length; i++) { + const ch = text[i] + if (escape) { + escape = false + continue } - if (maxNesting > 100) { - return new Response('JSON nesting too deep', {status: 400}) + if (ch === '\\' && inString) { + escape = true + continue + } + if (ch === '"') { + inString = !inString + continue + } + if (inString) continue + if (ch === '{' || ch === '[') { + nestingLevel++ + maxNesting = Math.max(maxNesting, nestingLevel) + } else if (ch === '}' || ch === ']') { + nestingLevel-- } } - - // Handle empty string body (becomes empty object) - if (text === '' || text.trim() === '') { - Object.defineProperty(req, 'body', { - value: {}, - writable: true, - enumerable: true, - configurable: true, - }) - return deferNext ? null : next() - } - - let body - try { - body = JSON.parse(text, reviver) - } catch (parseError) { - throw new Error(`Invalid JSON: ${parseError.message}`) - } - - if (strict && typeof body !== 'object') { - throw new Error('JSON body must be an object or array') + if (maxNesting > 100) { + return new Response('JSON nesting too deep', {status: 400}) } + } + // Handle empty string body — return undefined to distinguish from actual data (L-2 fix) + if (text === '' || text.trim() === '') { Object.defineProperty(req, 'body', { - value: body, + value: undefined, writable: true, enumerable: true, configurable: true, }) - return deferNext ? null : next() - } catch (error) { - throw error } + + let body + try { + body = JSON.parse(text, reviver) + } catch (parseError) { + throw new Error(`Invalid JSON: ${parseError.message}`) + } + + if (strict && typeof body !== 'object') { + throw new Error('JSON body must be an object or array') + } + + Object.defineProperty(req, 'body', { + value: body, + writable: true, + enumerable: true, + configurable: true, + }) + + return deferNext ? null : next() } } @@ -299,38 +384,37 @@ function createTextParser(options = {}) { return deferNext ? null : next() } - try { - const contentLength = req.headers.get('content-length') - if (contentLength) { - const length = parseInt(contentLength) - if (isNaN(length) || length < 0) { - return new Response('Invalid content-length header', {status: 400}) - } - if (length > parsedLimit) { - return new Response('Request body size exceeded', {status: 413}) - } + const contentLength = req.headers.get('content-length') + if (contentLength) { + const length = parseInt(contentLength) + if (isNaN(length) || length < 0) { + return new Response('Invalid content-length header', {status: 400}) } + if (length > parsedLimit) { + return new Response('Request body size exceeded', {status: 413}) + } + } - const text = await req.text() - // Store raw body text for verification - req._rawBodyText = text - - const textLength = new TextEncoder().encode(text).length - if (textLength > parsedLimit) { + let text + try { + text = await readBodyWithLimit(req, parsedLimit) + } catch (e) { + if (e.status === 413) { return new Response('Request body size exceeded', {status: 413}) } + return new Response('Failed to read request body', {status: 400}) + } + // Store raw body text for verification (L-3: use Symbol to prevent accidental serialization) + req[RAW_BODY_SYMBOL] = text - Object.defineProperty(req, 'body', { - value: text, - writable: true, - enumerable: true, - configurable: true, - }) + Object.defineProperty(req, 'body', { + value: text, + writable: true, + enumerable: true, + configurable: true, + }) - return deferNext ? null : next() - } catch (error) { - throw error - } + return deferNext ? null : next() } } @@ -338,8 +422,8 @@ function createTextParser(options = {}) { * Creates a URL-encoded form parser middleware * @param {Object} options - Body parser configuration * @param {number|string} options.limit - Maximum body size in bytes - * @param {boolean} options.extended - Use extended query string parsing - * @param {boolean} options.parseNestedObjects - Parse nested object notation + * @param {boolean} options.extended - Enable rich parsing: nested objects, arrays, and duplicate key merging (default: true). When false, only flat key-value pairs are returned. + * @param {boolean} options.parseNestedObjects - Parse bracket notation (e.g. a[b]=1) into nested objects. Only applies when extended=true (default: true) * @param {boolean} options.deferNext - If true, don't call next() and let caller handle it * @returns {Function} Middleware function */ @@ -361,92 +445,87 @@ function createURLEncodedParser(options = {}) { return deferNext ? null : next() } - try { - const contentLength = req.headers.get('content-length') - if (contentLength) { - const length = parseInt(contentLength) - if (isNaN(length) || length < 0) { - return new Response('Invalid content-length header', {status: 400}) - } - if (length > parsedLimit) { - return new Response('Request body size exceeded', {status: 413}) - } + const contentLength = req.headers.get('content-length') + if (contentLength) { + const length = parseInt(contentLength) + if (isNaN(length) || length < 0) { + return new Response('Invalid content-length header', {status: 400}) } + if (length > parsedLimit) { + return new Response('Request body size exceeded', {status: 413}) + } + } - const text = await req.text() - // Store raw body text for verification - req._rawBodyText = text - - const textLength = new TextEncoder().encode(text).length - if (textLength > parsedLimit) { + let text + try { + text = await readBodyWithLimit(req, parsedLimit) + } catch (e) { + if (e.status === 413) { return new Response('Request body size exceeded', {status: 413}) } + return new Response('Failed to read request body', {status: 400}) + } + // Store raw body text for verification (L-3: use Symbol to prevent accidental serialization) + req[RAW_BODY_SYMBOL] = text - const body = {} - const params = new URLSearchParams(text) + const body = Object.create(null) + const params = new URLSearchParams(text) - // Prevent DoS through excessive parameters - let paramCount = 0 - const maxParams = 1000 // Reasonable limit for URL-encoded parameters + // Prevent DoS through excessive parameters + let paramCount = 0 + const maxParams = 1000 // Reasonable limit for URL-encoded parameters - for (const [key, value] of params.entries()) { - paramCount++ - if (paramCount > maxParams) { - return new Response('Too many parameters', {status: 400}) - } + for (const [key, value] of params.entries()) { + paramCount++ + if (paramCount > maxParams) { + return new Response('Too many parameters', {status: 400}) + } - // Validate key and value lengths to prevent memory exhaustion - if (key.length > 1000 || value.length > 10000) { - return new Response('Parameter too long', {status: 400}) - } + // Validate key and value lengths to prevent memory exhaustion + if (key.length > 1000 || value.length > 10000) { + return new Response('Parameter too long', {status: 400}) + } - if (parseNestedObjects) { - try { - parseNestedKey(body, key, value) - } catch (parseError) { - return new Response( - `Invalid parameter structure: ${parseError.message}`, - {status: 400}, - ) - } - } else { - // Protect against prototype pollution even when parseNestedObjects is false - const prototypePollutionKeys = [ - '__proto__', - 'constructor', - 'prototype', - 'hasOwnProperty', - 'isPrototypeOf', - 'propertyIsEnumerable', - 'valueOf', - 'toString', - ] - - if (!prototypePollutionKeys.includes(key)) { - if (body[key] !== undefined) { - if (Array.isArray(body[key])) { - body[key].push(value) - } else { - body[key] = [body[key], value] - } + if (extended && parseNestedObjects) { + // Extended + nested: parse bracket notation into nested objects/arrays + // (parseNestedKey has its own prototype pollution guards via PROTOTYPE_POLLUTION_KEYS) + try { + parseNestedKey(body, key, value) + } catch (parseError) { + return new Response( + `Invalid parameter structure: ${parseError.message}`, + {status: 400}, + ) + } + } else if (extended) { + // Extended but no nested parsing: flat keys with duplicate key merging into arrays + if (!PROTOTYPE_POLLUTION_KEYS.has(key)) { + if (body[key] !== undefined) { + if (Array.isArray(body[key])) { + body[key].push(value) } else { - body[key] = value + body[key] = [body[key], value] } + } else { + body[key] = value } } + } else { + // Simple mode: flat key-value pairs, last value wins + if (!PROTOTYPE_POLLUTION_KEYS.has(key)) { + body[key] = value + } } + } - Object.defineProperty(req, 'body', { - value: body, - writable: true, - enumerable: true, - configurable: true, - }) + Object.defineProperty(req, 'body', { + value: body, + writable: true, + enumerable: true, + configurable: true, + }) - return deferNext ? null : next() - } catch (error) { - throw error - } + return deferNext ? null : next() } } @@ -468,109 +547,141 @@ function createMultipartParser(options = {}) { return deferNext ? null : next() } - try { - const contentLength = req.headers.get('content-length') - if (contentLength) { - const length = parseInt(contentLength) - if (isNaN(length) || length < 0) { - return new Response('Invalid content-length header', {status: 400}) - } - if (length > parsedLimit) { - return new Response('Request body size exceeded', {status: 413}) - } + const contentLength = req.headers.get('content-length') + if (contentLength) { + const length = parseInt(contentLength) + if (isNaN(length) || length < 0) { + return new Response('Invalid content-length header', {status: 400}) + } + if (length > parsedLimit) { + return new Response('Request body size exceeded', {status: 413}) } + } - const formData = await req.formData() + const formData = await req.formData() - // Calculate actual size of form data and validate - let totalSize = 0 - let fieldCount = 0 - const maxFields = 100 // Reasonable limit for form fields + // Single-pass: validate and extract simultaneously (H-5 fix) + // Merges the validation and extraction loops to avoid double iteration + let totalSize = 0 + let fieldCount = 0 + const maxFields = 100 // Reasonable limit for form fields - for (const [key, value] of formData.entries()) { - fieldCount++ - if (fieldCount > maxFields) { - return new Response('Too many form fields', {status: 400}) - } + const body = Object.create(null) + const files = Object.create(null) - // Validate field name length - if (key.length > 1000) { - return new Response('Field name too long', {status: 400}) - } + for (const [key, value] of formData.entries()) { + fieldCount++ + if (fieldCount > maxFields) { + return new Response('Too many form fields', {status: 400}) + } - if (value instanceof File) { - totalSize += value.size - // Validate file name length for security - if (value.name && value.name.length > 255) { - return new Response('Filename too long', {status: 400}) - } - // Validate file size individually - if (value.size > parsedLimit) { - return new Response('File too large', {status: 413}) - } - } else { - const valueSize = new TextEncoder().encode(value).length - totalSize += valueSize - // Validate field value length - if (valueSize > 100000) { - // 100KB per field - return new Response('Field value too long', {status: 400}) - } + // Validate field name length + if (key.length > 1000) { + return new Response('Field name too long', {status: 400}) + } + + // Skip prototype pollution keys + if (PROTOTYPE_POLLUTION_KEYS.has(key)) { + continue + } + + if (value instanceof File) { + totalSize += value.size + // Validate file name length for security + if (value.name && value.name.length > 255) { + return new Response('Filename too long', {status: 400}) } + // Validate file size individually + if (value.size > parsedLimit) { + return new Response('File too large', {status: 413}) + } + totalSize += new TextEncoder().encode(key).length - // Check total size periodically to prevent memory exhaustion + // Check total size to prevent memory exhaustion if (totalSize > parsedLimit) { return new Response('Request body size exceeded', {status: 413}) } - } - const body = {} - const files = {} - - for (const [key, value] of formData.entries()) { - if (value instanceof File) { - const mimetype = value.type?.split(';')[0] || value.type - const fileData = new Uint8Array(await value.arrayBuffer()) + const mimetype = value.type?.split(';')[0] || value.type + const fileData = new Uint8Array(await value.arrayBuffer()) + // Sanitize filename to prevent path traversal + let sanitizedFilename = value.name + if (sanitizedFilename) { + const originalName = sanitizedFilename + // Remove null bytes + sanitizedFilename = sanitizedFilename.replace(/\0/g, '') + // Remove path separators and directory components + sanitizedFilename = sanitizedFilename.replace(/\.\./g, '') + sanitizedFilename = sanitizedFilename.replace(/[/\\]/g, '') + // Remove leading dots (hidden files) + sanitizedFilename = sanitizedFilename.replace(/^\.+/, '') + // If nothing is left after sanitization, use a default name + if (!sanitizedFilename) { + sanitizedFilename = 'upload' + } files[key] = { - filename: value.name, - name: value.name, + filename: sanitizedFilename, + originalName: originalName, + name: sanitizedFilename, size: value.size, type: value.type, mimetype: mimetype, data: fileData, } } else { - if (body[key] !== undefined) { - if (Array.isArray(body[key])) { - body[key].push(value) - } else { - body[key] = [body[key], value] - } - } else { - body[key] = value + files[key] = { + filename: value.name, + name: value.name, + size: value.size, + type: value.type, + mimetype: mimetype, + data: fileData, } } - } + } else { + const valueSize = new TextEncoder().encode(value).length + totalSize += valueSize + // Validate field value length + if (valueSize > 100000) { + // 100KB per field + return new Response('Field value too long', {status: 400}) + } - Object.defineProperty(req, 'body', { - value: body, - writable: true, - enumerable: true, - configurable: true, - }) + totalSize += new TextEncoder().encode(key).length - Object.defineProperty(req, 'files', { - value: files, - writable: true, - enumerable: true, - configurable: true, - }) + // Check total size to prevent memory exhaustion + if (totalSize > parsedLimit) { + return new Response('Request body size exceeded', {status: 413}) + } - return deferNext ? null : next() - } catch (error) { - throw error + if (body[key] !== undefined) { + if (Array.isArray(body[key])) { + body[key].push(value) + } else { + body[key] = [body[key], value] + } + } else { + body[key] = value + } + } } + + Object.defineProperty(req, 'body', { + value: body, + writable: true, + enumerable: true, + configurable: true, + }) + + Object.defineProperty(req, 'files', { + value: files, + writable: true, + enumerable: true, + configurable: true, + }) + + return deferNext ? null : next() } } @@ -586,6 +697,7 @@ function createMultipartParser(options = {}) { * @param {Function} options.onError - Custom error handler * @param {Function} options.verify - Body verification function * @param {boolean} options.parseNestedObjects - Parse nested object notation (for compatibility) + * @param {boolean} options.extended - Enable rich URL-encoded parsing (for compatibility, forwarded to urlencoded.extended) * @param {string|number} options.jsonLimit - JSON size limit (backward compatibility) * @param {string|number} options.textLimit - Text size limit (backward compatibility) * @param {string|number} options.urlencodedLimit - URL-encoded size limit (backward compatibility) @@ -603,6 +715,7 @@ function createBodyParser(options = {}) { onError, verify, parseNestedObjects = true, + extended, // Backward compatibility for direct limit options jsonLimit, textLimit, @@ -626,6 +739,12 @@ function createBodyParser(options = {}) { urlencoded.urlencodedLimit || urlencoded.limit || '1mb', + extended: + urlencoded.extended !== undefined + ? urlencoded.extended + : extended !== undefined + ? extended + : true, parseNestedObjects: urlencoded.parseNestedObjects !== undefined ? urlencoded.parseNestedObjects @@ -640,7 +759,7 @@ function createBodyParser(options = {}) { // Create parsers with custom types consideration const jsonParserMiddleware = createJSONParser({ ...jsonOptions, - type: 'application/', // Broad match for JSON types + type: 'application/json', // Match only JSON content types deferNext: !!verify, // Defer next if verification is enabled }) const textParserMiddleware = createTextParser({ @@ -686,7 +805,26 @@ function createBodyParser(options = {}) { // Custom JSON parser handling for custom JSON types (case-insensitive) if (jsonParser && isJsonType(contentType, jsonTypes)) { - const text = await req.text() + const contentLength = req.headers.get('content-length') + const jsonParsedLimit = parseLimit(jsonOptions.limit) + if (contentLength) { + const length = parseInt(contentLength) + if (isNaN(length) || length < 0) { + return new Response('Invalid content-length header', {status: 400}) + } + if (length > jsonParsedLimit) { + return new Response('Request body size exceeded', {status: 413}) + } + } + let text + try { + text = await readBodyWithLimit(req, jsonParsedLimit) + } catch (e) { + if (e.status === 413) { + return new Response('Request body size exceeded', {status: 413}) + } + return new Response('Failed to read request body', {status: 400}) + } const body = jsonParser(text) Object.defineProperty(req, 'body', { value: body, @@ -699,7 +837,15 @@ function createBodyParser(options = {}) { } else { // Check if content type matches any JSON types first (including custom ones) if (isJsonType(contentType, jsonTypes)) { - result = await jsonParserMiddleware(req, next) + // Use a JSON parser that matches the detected custom type + const customTypeJsonParser = createJSONParser({ + ...jsonOptions, + type: jsonTypes.find((t) => + contentType.toLowerCase().includes(t.toLowerCase()), + ), + deferNext: !!verify, + }) + result = await customTypeJsonParser(req, next) } else { // Route to appropriate parser based on content type (case-insensitive) const lowerContentType = contentType.toLowerCase() @@ -735,10 +881,10 @@ function createBodyParser(options = {}) { if (verify && req.body !== undefined) { try { // For verification, we need to pass the raw body text - // Get the original text/data that was parsed + // Get the original text/data that was parsed (L-3: read from Symbol) let rawBody = '' - if (req._rawBodyText) { - rawBody = req._rawBodyText + if (req[RAW_BODY_SYMBOL]) { + rawBody = req[RAW_BODY_SYMBOL] } verify(req, rawBody) } catch (verifyError) { @@ -777,6 +923,7 @@ module.exports = { hasBody, shouldParse, parseLimit, + RAW_BODY_SYMBOL, } // Default export is the main body parser diff --git a/lib/middleware/cors.js b/lib/middleware/cors.js index e8f13e4..14588ef 100644 --- a/lib/middleware/cors.js +++ b/lib/middleware/cors.js @@ -28,8 +28,8 @@ function createCORS(options = {}) { const allowedOrigin = getAllowedOrigin(origin, requestOrigin, req) const addCorsHeaders = (response) => { - // Add Vary header for dynamic origins (regardless of whether origin is allowed) - if (typeof origin === 'function' || Array.isArray(origin)) { + // Add Vary header for non-wildcard origins to prevent CDN cache poisoning + if (origin !== '*') { const existingVary = response.headers.get('Vary') if (existingVary) { if (!existingVary.includes('Origin')) { @@ -42,32 +42,51 @@ function createCORS(options = {}) { if (allowedOrigin !== false) { response.headers.set('Access-Control-Allow-Origin', allowedOrigin) - } - // Don't allow wildcard origin with credentials - if (credentials && allowedOrigin !== '*') { - response.headers.set('Access-Control-Allow-Credentials', 'true') - } + // Don't allow wildcard origin with credentials + if (credentials && allowedOrigin !== '*') { + response.headers.set('Access-Control-Allow-Credentials', 'true') + } - // Handle exposedHeaders (can be string or array) - const exposedHeadersList = Array.isArray(exposedHeaders) - ? exposedHeaders - : typeof exposedHeaders === 'string' - ? [exposedHeaders] - : [] - if (exposedHeadersList.length > 0) { + // Handle exposedHeaders (can be string or array) + const exposedHeadersList = Array.isArray(exposedHeaders) + ? exposedHeaders + : typeof exposedHeaders === 'string' + ? [exposedHeaders] + : [] + if (exposedHeadersList.length > 0) { + response.headers.set( + 'Access-Control-Expose-Headers', + exposedHeadersList.join(', '), + ) + } + + // Add method and header info + response.headers.set( + 'Access-Control-Allow-Methods', + (Array.isArray(methods) ? methods : [methods]).join(', '), + ) + + const resolvedAllowedHeaders = + typeof allowedHeaders === 'function' + ? allowedHeaders(req) + : allowedHeaders + const allowedHeadersList = Array.isArray(resolvedAllowedHeaders) + ? resolvedAllowedHeaders + : typeof resolvedAllowedHeaders === 'string' + ? [resolvedAllowedHeaders] + : [] response.headers.set( - 'Access-Control-Expose-Headers', - exposedHeadersList.join(', '), + 'Access-Control-Allow-Headers', + allowedHeadersList.join(', '), ) } - // Add method and header info for all requests (not just OPTIONS) - response.headers.set( - 'Access-Control-Allow-Methods', - (Array.isArray(methods) ? methods : [methods]).join(', '), - ) + return response + } + if (req.method === 'OPTIONS') { + // I-2: Resolve allowedHeaders once for the entire preflight handling const resolvedAllowedHeaders = typeof allowedHeaders === 'function' ? allowedHeaders(req) @@ -77,15 +96,7 @@ function createCORS(options = {}) { : typeof resolvedAllowedHeaders === 'string' ? [resolvedAllowedHeaders] : [] - response.headers.set( - 'Access-Control-Allow-Headers', - allowedHeadersList.join(', '), - ) - return response - } - - if (req.method === 'OPTIONS') { // Handle preflight request const requestMethod = req.headers.get('access-control-request-method') const requestHeaders = req.headers.get('access-control-request-headers') @@ -98,13 +109,6 @@ function createCORS(options = {}) { // Check if requested headers are allowed if (requestHeaders) { const requestedHeaders = requestHeaders.split(',').map((h) => h.trim()) - const resolvedAllowedHeaders = - typeof allowedHeaders === 'function' - ? allowedHeaders(req) - : allowedHeaders - const allowedHeadersList = Array.isArray(resolvedAllowedHeaders) - ? resolvedAllowedHeaders - : [] const hasDisallowedHeaders = requestedHeaders.some( (header) => @@ -127,31 +131,24 @@ function createCORS(options = {}) { if (typeof origin === 'function' || Array.isArray(origin)) { response.headers.set('Vary', 'Origin') } - } - // Don't allow wildcard origin with credentials - if (credentials && allowedOrigin !== '*') { - response.headers.set('Access-Control-Allow-Credentials', 'true') - } + // Don't allow wildcard origin with credentials + if (credentials && allowedOrigin !== '*') { + response.headers.set('Access-Control-Allow-Credentials', 'true') + } - response.headers.set( - 'Access-Control-Allow-Methods', - (Array.isArray(methods) ? methods : [methods]).join(', '), - ) + response.headers.set( + 'Access-Control-Allow-Methods', + (Array.isArray(methods) ? methods : [methods]).join(', '), + ) - const resolvedAllowedHeaders = - typeof allowedHeaders === 'function' - ? allowedHeaders(req) - : allowedHeaders - const allowedHeadersList = Array.isArray(resolvedAllowedHeaders) - ? resolvedAllowedHeaders - : [] - response.headers.set( - 'Access-Control-Allow-Headers', - allowedHeadersList.join(', '), - ) + response.headers.set( + 'Access-Control-Allow-Headers', + allowedHeadersList.join(', '), + ) - response.headers.set('Access-Control-Max-Age', maxAge.toString()) + response.headers.set('Access-Control-Max-Age', maxAge.toString()) + } if (preflightContinue) { const result = next() @@ -193,10 +190,13 @@ function getAllowedOrigin(origin, requestOrigin, req) { } if (Array.isArray(origin)) { + if (!requestOrigin || requestOrigin === 'null') return false return origin.includes(requestOrigin) ? requestOrigin : false } if (typeof origin === 'function') { + // Reject null/missing origins to prevent bypass via sandboxed iframes + if (!requestOrigin || requestOrigin === 'null') return false const result = origin(requestOrigin) return result === true ? requestOrigin : result || false } diff --git a/lib/middleware/index.d.ts b/lib/middleware/index.d.ts index 2eaa5ae..567569b 100644 --- a/lib/middleware/index.d.ts +++ b/lib/middleware/index.d.ts @@ -65,6 +65,8 @@ export interface JWTAuthOptions { audience?: string | string[] issuer?: string algorithms?: string[] + // Token type validation + requiredTokenType?: string // Custom response and error handling unauthorizedResponse?: | Response @@ -95,29 +97,14 @@ export interface TokenExtractionOptions { export function createJWTAuth(options?: JWTAuthOptions): RequestHandler export function createAPIKeyAuth(options: APIKeyAuthOptions): RequestHandler export function extractTokenFromHeader(req: ZeroRequest): string | null -export function extractToken( - req: ZeroRequest, - options?: TokenExtractionOptions, -): string | null -export function validateApiKeyInternal( - apiKey: string, - apiKeys: JWTAuthOptions['apiKeys'], - apiKeyValidator: JWTAuthOptions['apiKeyValidator'], - req: ZeroRequest, -): Promise -export function handleAuthError( - error: Error, - handlers: { - unauthorizedResponse?: JWTAuthOptions['unauthorizedResponse'] - onError?: JWTAuthOptions['onError'] - }, - req: ZeroRequest, -): Response +export const API_KEY_SYMBOL: symbol +export function maskApiKey(key: string): string // Rate limiting middleware types export interface RateLimitOptions { windowMs?: number max?: number + message?: string keyGenerator?: (req: ZeroRequest) => Promise | string handler?: ( req: ZeroRequest, @@ -126,7 +113,7 @@ export interface RateLimitOptions { resetTime: Date, ) => Promise | Response store?: RateLimitStore - standardHeaders?: boolean + standardHeaders?: boolean | 'minimal' excludePaths?: string[] skip?: (req: ZeroRequest) => boolean } @@ -169,7 +156,7 @@ export interface CORSOptions { | boolean | ((origin: string, req: ZeroRequest) => boolean | string) methods?: string[] - allowedHeaders?: string[] + allowedHeaders?: string[] | ((req: ZeroRequest) => string[]) exposedHeaders?: string[] credentials?: boolean maxAge?: number @@ -187,25 +174,30 @@ export function getAllowedOrigin( // Body parser middleware types export interface JSONParserOptions { - limit?: number + limit?: number | string reviver?: (key: string, value: any) => any strict?: boolean type?: string + deferNext?: boolean } export interface TextParserOptions { - limit?: number + limit?: number | string type?: string defaultCharset?: string + deferNext?: boolean } export interface URLEncodedParserOptions { - limit?: number + limit?: number | string extended?: boolean + parseNestedObjects?: boolean + deferNext?: boolean } export interface MultipartParserOptions { - limit?: number + limit?: number | string + deferNext?: boolean } export interface BodyParserOptions { @@ -213,6 +205,15 @@ export interface BodyParserOptions { text?: TextParserOptions urlencoded?: URLEncodedParserOptions multipart?: MultipartParserOptions + jsonTypes?: string[] + jsonParser?: (text: string) => any + onError?: (error: Error, req: ZeroRequest, next: () => any) => any + verify?: (req: ZeroRequest, rawBody: string) => void + parseNestedObjects?: boolean + jsonLimit?: number | string + textLimit?: number | string + urlencodedLimit?: number | string + multipartLimit?: number | string } export interface ParsedFile { @@ -233,6 +234,8 @@ export function createMultipartParser( export function createBodyParser(options?: BodyParserOptions): RequestHandler export function hasBody(req: ZeroRequest): boolean export function shouldParse(req: ZeroRequest, type: string): boolean +export function parseLimit(limit: number | string): number +export const RAW_BODY_SYMBOL: symbol // Prometheus metrics middleware types export interface PrometheusMetrics { diff --git a/lib/middleware/index.js b/lib/middleware/index.js index 19585b0..ac6ee63 100644 --- a/lib/middleware/index.js +++ b/lib/middleware/index.js @@ -23,6 +23,8 @@ module.exports = { createJWTAuth: jwtAuthModule.createJWTAuth, createAPIKeyAuth: jwtAuthModule.createAPIKeyAuth, extractTokenFromHeader: jwtAuthModule.extractTokenFromHeader, + API_KEY_SYMBOL: jwtAuthModule.API_KEY_SYMBOL, + maskApiKey: jwtAuthModule.maskApiKey, // Rate limiting middleware createRateLimit: rateLimitModule.createRateLimit, @@ -44,6 +46,8 @@ module.exports = { createBodyParser: bodyParserModule.createBodyParser, hasBody: bodyParserModule.hasBody, shouldParse: bodyParserModule.shouldParse, + parseLimit: bodyParserModule.parseLimit, + RAW_BODY_SYMBOL: bodyParserModule.RAW_BODY_SYMBOL, // Prometheus metrics middleware createPrometheusMiddleware: prometheusModule.createPrometheusMiddleware, diff --git a/lib/middleware/jwt-auth.js b/lib/middleware/jwt-auth.js index 52de88a..a81f925 100644 --- a/lib/middleware/jwt-auth.js +++ b/lib/middleware/jwt-auth.js @@ -13,6 +13,40 @@ function loadJose() { return joseLib } +const crypto = require('crypto') + +/** + * Symbol key for storing raw API key to prevent accidental serialization (M-7 fix) + */ +const API_KEY_SYMBOL = Symbol.for('0http.apiKey') + +/** + * Masks an API key for safe storage/logging. + * Shows first 4 and last 4 characters, masks the rest. + * @param {string} key - Raw API key + * @returns {string} Masked key + */ +function maskApiKey(key) { + if (!key || typeof key !== 'string') return '****' + if (key.length <= 8) return '****' + return key.slice(0, 4) + '****' + key.slice(-4) +} + +/** + * Constant-time string comparison to prevent timing attacks + */ +function timingSafeCompare(a, b) { + if (typeof a !== 'string' || typeof b !== 'string') return false + const aBuf = Buffer.from(a) + const bBuf = Buffer.from(b) + if (aBuf.length !== bBuf.length) { + // Still perform comparison to maintain constant time + crypto.timingSafeEqual(aBuf, aBuf) + return false + } + return crypto.timingSafeEqual(aBuf, bBuf) +} + /** * Creates JWT authentication middleware * @param {Object} options - JWT configuration options @@ -53,6 +87,7 @@ function createJWTAuth(options = {}) { audience, issuer, algorithms, + requiredTokenType, } = options // API key mode doesn't require JWT secret @@ -61,6 +96,35 @@ function createJWTAuth(options = {}) { throw new Error('JWT middleware requires either secret or jwksUri') } + // H-8: Warn about security risk of JWT tokens in query parameters + if (tokenQuery) { + console.warn( + `[0http-bun] SECURITY WARNING: JWT tokenQuery ("${tokenQuery}") is deprecated. ` + + 'Tokens in query parameters are logged in server access logs, browser history, and Referer headers. ' + + 'Use Authorization header or a custom getToken function instead.', + ) + } + + // M-8: Validate JWKS URI uses HTTPS in production + if (jwksUri) { + const parsedUri = new URL(jwksUri) + if ( + parsedUri.protocol !== 'https:' && + process.env.NODE_ENV === 'production' + ) { + throw new Error( + 'JWT middleware: JWKS URI must use HTTPS in production to prevent MitM key substitution. ' + + `Got: ${parsedUri.protocol}// Set NODE_ENV to a non-production value to bypass this check.`, + ) + } + if (parsedUri.protocol !== 'https:') { + console.warn( + `[0http-bun] SECURITY WARNING: JWKS URI "${jwksUri}" uses ${parsedUri.protocol} instead of https:. ` + + 'This is insecure and will be rejected in production (NODE_ENV=production).', + ) + } + } + // Setup key resolver for JWT let keyLike if (jwks) { @@ -82,18 +146,36 @@ function createJWTAuth(options = {}) { } // Default JWT verification options + const resolvedAlgorithms = algorithms || jwtOptions.algorithms || ['HS256'] + + // Prevent algorithm confusion attacks + const hasSymmetric = resolvedAlgorithms.some((alg) => alg.startsWith('HS')) + const hasAsymmetric = resolvedAlgorithms.some( + (alg) => + alg.startsWith('RS') || alg.startsWith('ES') || alg.startsWith('PS'), + ) + if (hasSymmetric && hasAsymmetric) { + throw new Error( + 'JWT middleware: mixing symmetric (HS*) and asymmetric (RS*/ES*/PS*) algorithms is not allowed. This prevents algorithm confusion attacks.', + ) + } + const defaultJwtOptions = { - algorithms: algorithms || ['HS256', 'RS256'], + ...jwtOptions, + algorithms: resolvedAlgorithms, audience, issuer, - ...jwtOptions, } return async function jwtAuthMiddleware(req, next) { const url = new URL(req.url) // Skip authentication for excluded paths - if (excludePaths.some((path) => url.pathname.startsWith(path))) { + if ( + excludePaths.some( + (path) => url.pathname === path || url.pathname.startsWith(path + '/'), + ) + ) { return next() } @@ -109,18 +191,19 @@ function createJWTAuth(options = {}) { req, ) if (validationResult !== false) { - // Set API key context + // Set API key context — store masked version to prevent leakage (M-7 fix) req.ctx = req.ctx || {} - req.ctx.apiKey = apiKey + req.ctx.apiKey = maskApiKey(apiKey) + req[API_KEY_SYMBOL] = apiKey // Raw key via Symbol for programmatic access only // If validation result is an object, use it as user data, otherwise default const userData = validationResult && typeof validationResult === 'object' ? validationResult - : {apiKey} + : {apiKey: maskApiKey(apiKey)} req.ctx.user = userData - req.apiKey = apiKey + req.apiKey = maskApiKey(apiKey) req.user = userData return next() } else { @@ -164,24 +247,40 @@ function createJWTAuth(options = {}) { defaultJwtOptions, ) + // L-4: Validate JWT token type header if configured + if (requiredTokenType) { + const tokenType = protectedHeader.typ + if ( + !tokenType || + tokenType.toLowerCase() !== requiredTokenType.toLowerCase() + ) { + return handleAuthError( + new Error('Invalid token type'), + {unauthorizedResponse, onError}, + req, + ) + } + } + // Add user info to request context req.ctx = req.ctx || {} req.ctx.user = payload req.ctx.jwt = { payload, header: protectedHeader, - token, } req.user = payload // Mirror to root for compatibility req.jwt = { payload, header: protectedHeader, - token, } return next() } catch (error) { if (optional && (!hasApiKeyMode || !req.headers.get(apiKeyHeader))) { + req.ctx = req.ctx || {} + req.ctx.authError = error.message + req.ctx.authAttempted = true return next() } @@ -200,12 +299,6 @@ function createJWTAuth(options = {}) { */ async function validateApiKeyInternal(apiKey, apiKeys, apiKeyValidator, req) { if (apiKeyValidator) { - // Check if this is the simplified validateApiKey function (expects only key) - if (apiKeyValidator.length === 1) { - const result = await apiKeyValidator(apiKey) - return result || false - } - // Otherwise call with both key and req const result = await apiKeyValidator(apiKey, req) return result || false } @@ -216,10 +309,10 @@ async function validateApiKeyInternal(apiKey, apiKeys, apiKeyValidator, req) { } if (Array.isArray(apiKeys)) { - return apiKeys.includes(apiKey) + return apiKeys.some((key) => timingSafeCompare(key, apiKey)) } - return apiKeys === apiKey + return timingSafeCompare(apiKeys, apiKey) } /** @@ -271,7 +364,8 @@ function handleAuthError(error, handlers = {}, req) { return result } } catch (handlerError) { - // Fall back to default handling if custom handler fails + // I-4: Log errors from custom handlers to aid debugging + console.error('[0http-bun] Custom onError handler threw:', handlerError) } } @@ -303,7 +397,11 @@ function handleAuthError(error, handlers = {}, req) { } } } catch (responseError) { - // Fall back to default response if custom response fails + // I-4: Log errors from custom response handlers to aid debugging + console.error( + '[0http-bun] Custom unauthorizedResponse handler threw:', + responseError, + ) } } @@ -317,19 +415,10 @@ function handleAuthError(error, handlers = {}, req) { message = 'Invalid API key' } else if (error.message === 'JWT verification not configured') { message = 'JWT verification not configured' + } else if (error.message === 'Invalid token type') { + message = 'Invalid token type' } else { - const {errors} = loadJose() - if (error instanceof errors.JWTExpired) { - message = 'Token expired' - } else if (error instanceof errors.JWTInvalid) { - message = 'Invalid token format' - } else if (error instanceof errors.JWKSNoMatchingKey) { - message = 'Token signature verification failed' - } else if (error.message.includes('audience')) { - message = 'Invalid token audience' - } else if (error.message.includes('issuer')) { - message = 'Invalid token issuer' - } + message = 'Invalid or expired token' } return new Response(JSON.stringify({error: message}), { @@ -376,7 +465,10 @@ function createAPIKeyAuth(options = {}) { const validateKey = typeof keys === 'function' ? keys - : (key) => (Array.isArray(keys) ? keys.includes(key) : keys === key) + : (key) => + Array.isArray(keys) + ? keys.some((k) => timingSafeCompare(k, key)) + : timingSafeCompare(keys, key) return async function apiKeyAuthMiddleware(req, next) { try { @@ -400,9 +492,10 @@ function createAPIKeyAuth(options = {}) { }) } - // Add API key info to context + // Add API key info to context — store masked version (M-7 fix) req.ctx = req.ctx || {} - req.ctx.apiKey = apiKey + req.ctx.apiKey = maskApiKey(apiKey) + req[API_KEY_SYMBOL] = apiKey // Raw key via Symbol for programmatic access only return next() } catch (error) { @@ -418,7 +511,6 @@ module.exports = { createJWTAuth, createAPIKeyAuth, extractTokenFromHeader, - extractToken, - validateApiKeyInternal, - handleAuthError, + API_KEY_SYMBOL, + maskApiKey, } diff --git a/lib/middleware/rate-limit.js b/lib/middleware/rate-limit.js index 76fa9f9..dbd687c 100644 --- a/lib/middleware/rate-limit.js +++ b/lib/middleware/rate-limit.js @@ -8,7 +8,7 @@ class MemoryStore { this.resetTimes = new Map() } - async increment(key, windowMs) { + increment(key, windowMs) { const now = Date.now() const windowStart = Math.floor(now / windowMs) * windowMs const storeKey = `${key}:${windowStart}` @@ -82,27 +82,40 @@ function createRateLimit(options = {}) { * @returns {Response} Response with headers added */ const addRateLimitHeaders = (response, totalHits, resetTime) => { - if (standardHeaders && response && response.headers) { - response.headers.set('X-RateLimit-Limit', max.toString()) - response.headers.set( - 'X-RateLimit-Remaining', - Math.max(0, max - totalHits).toString(), - ) - response.headers.set( - 'X-RateLimit-Reset', - Math.ceil(resetTime.getTime() / 1000).toString(), - ) - response.headers.set('X-RateLimit-Used', totalHits.toString()) + if (!standardHeaders || !response?.headers) return response + + if (standardHeaders === 'minimal') { + // L-6: Only add Retry-After on 429 responses to avoid disclosing config + if (response.status === 429) { + const retryAfter = Math.ceil((resetTime.getTime() - Date.now()) / 1000) + response.headers.set('Retry-After', Math.max(0, retryAfter).toString()) + } + return response } + + // Full headers (standardHeaders === true) + response.headers.set('X-RateLimit-Limit', max.toString()) + response.headers.set( + 'X-RateLimit-Remaining', + Math.max(0, max - totalHits).toString(), + ) + response.headers.set( + 'X-RateLimit-Reset', + Math.ceil(resetTime.getTime() / 1000).toString(), + ) + response.headers.set('X-RateLimit-Used', totalHits.toString()) return response } return async function rateLimitMiddleware(req, next) { - // Allow test to inject a fresh store for isolation - const activeStore = req && req.rateLimitStore ? req.rateLimitStore : store + const activeStore = store const url = new URL(req.url) - if (excludePaths.some((path) => url.pathname.startsWith(path))) { + if ( + excludePaths.some( + (path) => url.pathname === path || url.pathname.startsWith(path + '/'), + ) + ) { return next() } @@ -159,22 +172,30 @@ function createRateLimit(options = {}) { } /** - * Default key generator - uses IP address + * Default key generator - uses connection-level IP address + * Checks req.ip, req.remoteAddress, and req.socket?.remoteAddress in order. + * NOTE: If behind a reverse proxy, provide a custom keyGenerator that + * reads the appropriate header after configuring your proxy to set it. * @param {Request} req - Request object * @returns {string} Rate limit key */ +let _unknownKeyWarned = false + function defaultKeyGenerator(req) { - // Try to get real IP from common headers - const forwarded = req.headers.get('x-forwarded-for') - const realIp = req.headers.get('x-real-ip') - const cfConnectingIp = req.headers.get('cf-connecting-ip') - - return ( - cfConnectingIp || - realIp || - (forwarded && forwarded.split(',')[0].trim()) || - 'unknown' - ) + // Use connection-level IP if available (set by Bun's server or upstream middleware) + const ip = req.ip || req.remoteAddress || req.socket?.remoteAddress + if (ip) return ip + + // I-1: Generate unique key per request to avoid shared bucket DoS + if (!_unknownKeyWarned) { + console.warn( + '[0http-bun] SECURITY WARNING: Rate limiter cannot determine client IP. ' + + 'Each request without an IP gets a unique key (no shared bucket). ' + + 'Configure a custom keyGenerator for proper rate limiting behind proxies.', + ) + _unknownKeyWarned = true + } + return `unknown:${Date.now()}:${Math.random().toString(36).slice(2)}` } /** @@ -216,10 +237,32 @@ function createSlidingWindowRateLimit(options = {}) { max = 100, keyGenerator = defaultKeyGenerator, handler = defaultHandler, + maxKeys = 10000, } = options const requests = new Map() // key -> array of timestamps + // Periodic cleanup of stale entries + const cleanupInterval = setInterval( + () => { + const now = Date.now() + for (const [key, timestamps] of requests.entries()) { + const filtered = timestamps.filter((ts) => now - ts < windowMs) + if (filtered.length === 0) { + requests.delete(key) + } else { + requests.set(key, filtered) + } + } + }, + Math.min(windowMs, 60000), + ) // Clean up at most every minute + + // Allow garbage collection if reference is lost + if (cleanupInterval.unref) { + cleanupInterval.unref() + } + return async function slidingWindowRateLimitMiddleware(req, next) { // Generate key and record the request immediately - let errors bubble up const key = await keyGenerator(req) @@ -248,6 +291,13 @@ function createSlidingWindowRateLimit(options = {}) { userRequests.push(now) requests.set(key, userRequests) + // Enforce max keys to prevent unbounded memory growth + if (requests.size > maxKeys) { + // Remove oldest entry + const firstKey = requests.keys().next().value + requests.delete(firstKey) + } + // Add rate limit info to context req.ctx = req.ctx || {} req.ctx.rateLimit = { diff --git a/lib/next.js b/lib/next.js index 374491b..9b73cd9 100644 --- a/lib/next.js +++ b/lib/next.js @@ -15,12 +15,19 @@ module.exports = function next( const nextIndex = index + 1 try { - return middleware(req, (err) => { + const result = middleware(req, (err) => { if (err) { return errorHandler(err, req) } return next(middlewares, req, nextIndex, defaultRoute, errorHandler) }) + + // Catch rejected promises from async middleware + if (result && typeof result.catch === 'function') { + return result.catch((err) => errorHandler(err, req)) + } + + return result } catch (err) { return errorHandler(err, req) } diff --git a/lib/router/sequential.js b/lib/router/sequential.js index 09fcaec..1e00ab4 100644 --- a/lib/router/sequential.js +++ b/lib/router/sequential.js @@ -11,6 +11,7 @@ const STATUS_500 = { module.exports = (config = {}) => { const cache = new Map() + const cacheSize = config.cacheSize || 1000 // Pre-create default responses to avoid object creation overhead const default404Response = new Response(null, STATUS_404) @@ -18,10 +19,14 @@ module.exports = (config = {}) => { // Cache default functions to avoid closure creation const defaultRouteHandler = config.defaultRoute || (() => default404Response) const errorHandlerFn = - config.errorHandler || ((err) => new Response(err.message, STATUS_500)) + config.errorHandler || + ((err) => { + console.error(err) + return new Response('Internal Server Error', STATUS_500) + }) - // Optimize empty params object reuse - const emptyParams = {} + // Optimize empty params object reuse (frozen to prevent cross-request mutation) + const emptyParams = Object.freeze({}) const router = new Trouter() router.port = config.port || 3000 @@ -35,7 +40,7 @@ module.exports = (config = {}) => { } _use.call(router, prefix, middlewares) - return this + return router } router.fetch = (req) => { @@ -65,8 +70,25 @@ module.exports = (config = {}) => { const path = pathStart < pathEnd ? url.substring(pathStart, pathEnd) : '/' - req.path = path + // Normalize path: collapse double slashes and decode URI components + // Preserve encoded slashes (%2F/%2f) to maintain path structure + let normalizedPath = path.replace(/\/\/+/g, '/') + try { + normalizedPath = decodeURIComponent( + normalizedPath.replace(/%2[fF]/g, '%252F'), + ) + } catch (_) { + // Malformed URI — use the collapsed path as-is + } + + req.path = normalizedPath req.query = queryString ? qs.parse(queryString) : {} + // L-1: Filter dangerous keys from query to prevent prototype pollution downstream + if (queryString) { + delete req.query['__proto__'] + delete req.query['constructor'] + delete req.query['prototype'] + } // Optimized cache lookup with method-based Map structure const method = req.method @@ -74,14 +96,23 @@ module.exports = (config = {}) => { let match_result if (methodCache) { - match_result = methodCache.get(path) + match_result = methodCache.get(normalizedPath) if (match_result === undefined) { - match_result = router.find(method, path) - methodCache.set(path, match_result) + match_result = router.find(method, normalizedPath) + methodCache.set(normalizedPath, match_result) + // LRU eviction: remove oldest entry when cache exceeds max size + if (methodCache.size > cacheSize) { + const firstKey = methodCache.keys().next().value + methodCache.delete(firstKey) + } + } else { + // LRU refresh: move accessed entry to end so it's evicted last + methodCache.delete(normalizedPath) + methodCache.set(normalizedPath, match_result) } } else { - match_result = router.find(method, path) - methodCache = new Map([[path, match_result]]) + match_result = router.find(method, normalizedPath) + methodCache = new Map([[normalizedPath, match_result]]) cache.set(method, methodCache) } diff --git a/package.json b/package.json index 08a5ea7..7c81c38 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "0http-bun", - "version": "1.2.2", + "version": "1.3.0", "description": "0http for Bun", "main": "index.js", "scripts": { @@ -26,13 +26,13 @@ "lib/" ], "devDependencies": { - "0http-bun": "^1.2.1", - "bun-types": "^1.2.16", + "0http-bun": "^1.2.2", + "bun-types": "^1.3.8", "mitata": "^1.0.34", - "prettier": "^3.5.3", - "typescript": "^5.8.3", - "jose": "^6.0.11", - "pino": "^9.7.0", + "prettier": "^3.8.1", + "typescript": "^5.9.3", + "jose": "^6.1.3", + "pino": "^9.14.0", "prom-client": "^15.1.3" }, "keywords": [ diff --git a/test-types.ts b/test-types.ts index 5bcf498..0afef0c 100644 --- a/test-types.ts +++ b/test-types.ts @@ -122,6 +122,20 @@ const testRequestTypes = async (req: ZeroRequest): Promise => { const params: Record = req.params const query: Record = req.query + // Test connection-level IP properties + const ip: string | undefined = req.ip + const remoteAddress: string | undefined = req.remoteAddress + const socketAddress: string | undefined = req.socket?.remoteAddress + + // Test top-level rate limit info (set by rate-limit middleware) + const topLevelRateLimit = req.rateLimit + if (topLevelRateLimit) { + const limit: number = topLevelRateLimit.limit + const remaining: number = topLevelRateLimit.remaining + const current: number = topLevelRateLimit.current + const reset: Date = topLevelRateLimit.reset + } + // Test context object const ctx = req.ctx const log = ctx?.log diff --git a/test/integration/router.test.js b/test/integration/router.test.js index 785bf89..501dfa2 100644 --- a/test/integration/router.test.js +++ b/test/integration/router.test.js @@ -164,9 +164,12 @@ describe('Router Integration Tests', () => { describe('Error Handling', () => { it('should return a 500 response for a route that throws an error', async () => { + const originalError = console.error + console.error = () => {} const response = await router.fetch(createTestRequest('GET', '/error')) + console.error = originalError expect(response.status).toBe(500) - expect(await response.text()).toEqual('Unexpected error') + expect(await response.text()).toEqual('Internal Server Error') }) it('should return a 404 response for a non-existent route', async () => { @@ -346,12 +349,12 @@ describe('Router Integration Tests', () => { const testCases = [ { url: '/search/hello%20world?filter=test%20value', - expectedTerm: 'hello%20world', + expectedTerm: 'hello world', expectedQuery: {filter: 'test value'}, // Query parser decodes spaces }, { url: '/search/café?type=beverage', - expectedTerm: 'caf%C3%A9', // URL parameters are URL-encoded + expectedTerm: 'café', expectedQuery: {type: 'beverage'}, }, { @@ -426,9 +429,14 @@ describe('Router Integration Tests', () => { return {success: true} }) + const originalError = console.error + console.error = () => {} + const req = createTestRequest('GET', '/api/error/test') const result = await router.fetch(req) + console.error = originalError + // Should get error response with error handling expect(result.status).toBe(500) expect(req.step1).toBe(true) diff --git a/test/performance/regression.test.js b/test/performance/regression.test.js index f98ba48..56f191e 100644 --- a/test/performance/regression.test.js +++ b/test/performance/regression.test.js @@ -214,6 +214,10 @@ describe('Performance Regression Tests', () => { throw new Error('Test error') }) + // Suppress console.error during perf test (default error handler logs errors) + const originalError = console.error + console.error = () => {} + const iterations = 500 const {time} = await measureTime(async () => { for (let i = 0; i < iterations; i++) { @@ -221,6 +225,8 @@ describe('Performance Regression Tests', () => { } }) + console.error = originalError + const avgTime = time / iterations expect(avgTime).toBeLessThan(3) // Error handling should not be too slow }) diff --git a/test/security/prototype-pollution.test.js b/test/security/prototype-pollution.test.js index 95b042b..680d882 100644 --- a/test/security/prototype-pollution.test.js +++ b/test/security/prototype-pollution.test.js @@ -207,4 +207,77 @@ describe('Prototype Pollution Security Tests', () => { expect({}.polluted).toBeUndefined() }) }) + + describe('Query Prototype Pollution Protection (L-1)', () => { + it('should filter __proto__ from query parameters', async () => { + routerInstance.get('/search', (req) => { + return Response.json({query: req.query}) + }) + + const testReq = { + method: 'GET', + url: 'http://localhost/search?__proto__[polluted]=true&safe=value', + headers: {}, + } + + await routerInstance.fetch(testReq) + + // __proto__ key should be filtered from query + expect(testReq.query['__proto__']).toBeUndefined() + expect(testReq.query.safe).toBe('value') + expect({}.polluted).toBeUndefined() + }) + + it('should filter constructor from query parameters', async () => { + routerInstance.get('/search', (req) => { + return Response.json({query: req.query}) + }) + + const testReq = { + method: 'GET', + url: 'http://localhost/search?constructor=malicious&name=test', + headers: {}, + } + + await routerInstance.fetch(testReq) + + expect(testReq.query['constructor']).toBeUndefined() + expect(testReq.query.name).toBe('test') + }) + + it('should filter prototype from query parameters', async () => { + routerInstance.get('/search', (req) => { + return Response.json({query: req.query}) + }) + + const testReq = { + method: 'GET', + url: 'http://localhost/search?prototype=bad&ok=yes', + headers: {}, + } + + await routerInstance.fetch(testReq) + + expect(testReq.query['prototype']).toBeUndefined() + expect(testReq.query.ok).toBe('yes') + }) + + it('should still allow normal query parameters', async () => { + routerInstance.get('/search', (req) => { + return Response.json({query: req.query}) + }) + + const testReq = { + method: 'GET', + url: 'http://localhost/search?page=1&limit=10&filter=active', + headers: {}, + } + + await routerInstance.fetch(testReq) + + expect(testReq.query.page).toBe('1') + expect(testReq.query.limit).toBe('10') + expect(testReq.query.filter).toBe('active') + }) + }) }) diff --git a/test/unit/body-parser.test.js b/test/unit/body-parser.test.js index 9e22b1b..3793065 100644 --- a/test/unit/body-parser.test.js +++ b/test/unit/body-parser.test.js @@ -395,7 +395,7 @@ describe('Body Parser Middleware', () => { const middleware = bodyParser() await middleware(req, next) - expect(req.body).toEqual({}) + expect(req.body).toBeUndefined() expect(next).toHaveBeenCalled() }) @@ -899,8 +899,8 @@ describe('Body Parser Middleware', () => { }) it('should handle non-string, non-number limit values', () => { - expect(parseLimit(null)).toBe(1024 * 1024) // Default 1MB - expect(parseLimit(undefined)).toBe(1024 * 1024) // Default 1MB + expect(() => parseLimit(null)).toThrow(TypeError) + expect(() => parseLimit(undefined)).toThrow(TypeError) }) it('should handle custom JSON parser in main body parser', async () => { @@ -992,6 +992,132 @@ describe('Body Parser Middleware', () => { expect(req.body.name).toEqual(['John', 'Jane', 'Bob']) }) + it('should use simple mode when extended is false (last value wins)', async () => { + const formData = 'name=John&name=Jane&name=Bob' + req = createTestRequest('POST', '/api/form', { + headers: {'Content-Type': 'application/x-www-form-urlencoded'}, + body: formData, + }) + + const middleware = bodyParser({urlencoded: {extended: false}}) + await middleware(req, next) + + // In simple mode, last value wins (no array merging) + expect(req.body.name).toBe('Bob') + }) + + it('should not parse bracket notation when extended is false', async () => { + const formData = 'user[name]=John&user[age]=30' + req = createTestRequest('POST', '/api/form', { + headers: {'Content-Type': 'application/x-www-form-urlencoded'}, + body: formData, + }) + + const middleware = bodyParser({urlencoded: {extended: false}}) + await middleware(req, next) + + // In simple mode, brackets are treated as literal key characters + expect(req.body['user[name]']).toBe('John') + expect(req.body['user[age]']).toBe('30') + expect(req.body.user).toBeUndefined() + }) + + it('should merge duplicate keys into arrays when extended is true but parseNestedObjects is false', async () => { + const formData = 'color=red&color=blue&color=green' + req = createTestRequest('POST', '/api/form', { + headers: {'Content-Type': 'application/x-www-form-urlencoded'}, + body: formData, + }) + + const middleware = bodyParser({ + urlencoded: {extended: true, parseNestedObjects: false}, + }) + await middleware(req, next) + + expect(Array.isArray(req.body.color)).toBe(true) + expect(req.body.color).toEqual(['red', 'blue', 'green']) + }) + + it('should parse nested objects when extended is true and parseNestedObjects is true', async () => { + const formData = 'user[name]=John&user[age]=30' + req = createTestRequest('POST', '/api/form', { + headers: {'Content-Type': 'application/x-www-form-urlencoded'}, + body: formData, + }) + + const middleware = bodyParser({ + urlencoded: {extended: true, parseNestedObjects: true}, + }) + await middleware(req, next) + + expect(req.body.user).toEqual({name: 'John', age: '30'}) + }) + + it('should protect against prototype pollution when extended is false', async () => { + const formData = '__proto__=polluted&constructor=polluted&name=safe' + req = createTestRequest('POST', '/api/form', { + headers: {'Content-Type': 'application/x-www-form-urlencoded'}, + body: formData, + }) + + const middleware = bodyParser({urlencoded: {extended: false}}) + await middleware(req, next) + + expect({}.toString).toBeTypeOf('function') + expect(req.body.name).toBe('safe') + expect(req.body.__proto__).toBeUndefined() + expect(req.body.constructor).toBeUndefined() + }) + + it('should protect against prototype pollution when extended=true, parseNestedObjects=false', async () => { + const formData = '__proto__=a&__proto__=b&constructor=c&name=safe' + req = createTestRequest('POST', '/api/form', { + headers: {'Content-Type': 'application/x-www-form-urlencoded'}, + body: formData, + }) + + const middleware = bodyParser({ + urlencoded: {extended: true, parseNestedObjects: false}, + }) + await middleware(req, next) + + expect(req.body.name).toBe('safe') + expect(req.body.__proto__).toBeUndefined() + expect(req.body.constructor).toBeUndefined() + }) + + it('should treat brackets as literal keys when extended=true but parseNestedObjects=false', async () => { + const formData = 'user[name]=John&user[name]=Jane' + req = createTestRequest('POST', '/api/form', { + headers: {'Content-Type': 'application/x-www-form-urlencoded'}, + body: formData, + }) + + const middleware = bodyParser({ + urlencoded: {extended: true, parseNestedObjects: false}, + }) + await middleware(req, next) + + // Brackets treated as literal key characters, duplicate keys merged into array + expect(req.body['user[name]']).toEqual(['John', 'Jane']) + expect(req.body.user).toBeUndefined() + }) + + it('should forward top-level extended option to urlencoded parser', async () => { + const formData = 'name=John&name=Jane' + req = createTestRequest('POST', '/api/form', { + headers: {'Content-Type': 'application/x-www-form-urlencoded'}, + body: formData, + }) + + // Top-level extended=false should be forwarded to urlencoded parser + const middleware = bodyParser({extended: false}) + await middleware(req, next) + + // Simple mode: last value wins + expect(req.body.name).toBe('Jane') + }) + it('should handle multipart files with no type', async () => { const boundary = 'boundary123' const multipartBody = [ @@ -1261,9 +1387,9 @@ describe('Body Parser Middleware', () => { const {createTextParser} = require('../../lib/middleware/body-parser') const middleware = createTextParser() - await expect(middleware(mockReq, () => {})).rejects.toThrow( - 'Text parsing failed', - ) + const response = await middleware(mockReq, () => {}) + expect(response.status).toBe(400) + expect(await response.text()).toBe('Failed to read request body') }) test('should handle invalid content-length in URL-encoded parser', async () => { @@ -1322,9 +1448,9 @@ describe('Body Parser Middleware', () => { createURLEncodedParser, } = require('../../lib/middleware/body-parser') const middleware = createURLEncodedParser() - await expect(middleware(mockReq, () => {})).rejects.toThrow( - 'URL-encoded parsing failed', - ) + const response = await middleware(mockReq, () => {}) + expect(response.status).toBe(400) + expect(await response.text()).toBe('Failed to read request body') }) test('should handle invalid content-length in multipart parser', async () => { @@ -1607,9 +1733,9 @@ describe('Body Parser Middleware', () => { const {createTextParser} = require('../../lib/middleware/body-parser') const middleware = createTextParser() - await expect(middleware(mockReq, () => {})).rejects.toThrow( - 'Text reading failed', - ) + const response = await middleware(mockReq, () => {}) + expect(response.status).toBe(400) + expect(await response.text()).toBe('Failed to read request body') }) test('should handle URL-encoded early return for non-matching content type (lines 360-361)', async () => { @@ -1701,10 +1827,7 @@ describe('Body Parser Middleware', () => { // This might not be possible due to the restrictive regex expect(() => parseLimit('0.b')).toThrow('Invalid limit format') } catch (e) { - // If this doesn't work, the line might be unreachable - console.log( - 'Line 40 may be mathematically unreachable due to regex constraints', - ) + // Line 40 may be mathematically unreachable due to regex constraints } }) @@ -1720,7 +1843,6 @@ describe('Body Parser Middleware', () => { ['content-type', 'application/x-www-form-urlencoded'], ]), text: async () => 'simpleKey=simpleValue', // Simple key without brackets - _rawBodyText: 'simpleKey=simpleValue', } const urlEncodedParser = createURLEncodedParser({ @@ -1741,7 +1863,6 @@ describe('Body Parser Middleware', () => { method: 'POST', headers: new Map([['content-type', 'application/json']]), text: async () => '', // Empty string should be handled - _rawBodyText: '', } const jsonParser = createJSONParser() @@ -1815,7 +1936,6 @@ describe('Body Parser Middleware', () => { ['content-length', bodyContent.length.toString()], ]), text: async () => bodyContent, - _rawBodyText: bodyContent, } const urlEncodedParser = createURLEncodedParser({limit: 500}) // Small limit @@ -1870,3 +1990,67 @@ describe('Body Parser Middleware', () => { }) }) }) + +describe('RAW_BODY_SYMBOL Security (L-3)', () => { + const { + createJSONParser, + createTextParser, + createURLEncodedParser, + RAW_BODY_SYMBOL, + } = require('../../lib/middleware/body-parser') + const {createTestRequest} = require('../helpers') + + it('should export RAW_BODY_SYMBOL', () => { + expect(RAW_BODY_SYMBOL).toBeDefined() + expect(typeof RAW_BODY_SYMBOL).toBe('symbol') + expect(RAW_BODY_SYMBOL.toString()).toBe('Symbol(0http.rawBody)') + }) + + it('should store raw body via Symbol in JSON parser', async () => { + const req = createTestRequest('POST', '/api/data', { + headers: {'Content-Type': 'application/json'}, + body: '{"key": "value"}', + }) + + const parser = createJSONParser() + const next = jest.fn() + await parser(req, next) + + // Raw body should be accessible via Symbol + expect(req[RAW_BODY_SYMBOL]).toBe('{"key": "value"}') + // Raw body should NOT be accessible as a regular string property + expect(req._rawBodyText).toBeUndefined() + }) + + it('should store raw body via Symbol in text parser', async () => { + const req = createTestRequest('POST', '/api/data', { + headers: {'Content-Type': 'text/plain'}, + body: 'hello world', + }) + + const parser = createTextParser() + const next = jest.fn() + await parser(req, next) + + expect(req[RAW_BODY_SYMBOL]).toBe('hello world') + expect(req._rawBodyText).toBeUndefined() + }) + + it('should not expose raw body as enumerable property', async () => { + const req = createTestRequest('POST', '/api/data', { + headers: {'Content-Type': 'application/json'}, + body: '{"secret": "password123"}', + }) + + const parser = createJSONParser() + const next = jest.fn() + await parser(req, next) + + // Symbol-keyed properties should not appear in Object.keys + const keys = Object.keys(req) + expect(keys).not.toContain('_rawBodyText') + // The raw body should only be accessible via Symbol + expect(req._rawBodyText).toBeUndefined() + expect(req[RAW_BODY_SYMBOL]).toBe('{"secret": "password123"}') + }) +}) diff --git a/test/unit/config.test.js b/test/unit/config.test.js index f591893..ffb0b78 100644 --- a/test/unit/config.test.js +++ b/test/unit/config.test.js @@ -96,11 +96,16 @@ describe('Router Configuration', () => { throw new Error('Test error') }) + const originalError = console.error + console.error = () => {} + const response = await router.fetch( new Request('http://localhost:3000/test-error', { method: 'GET', }), ) + + console.error = originalError expect(response.status).toBe(500) }) }) diff --git a/test/unit/cors.test.js b/test/unit/cors.test.js index 9ca1695..49fc8c9 100644 --- a/test/unit/cors.test.js +++ b/test/unit/cors.test.js @@ -667,7 +667,10 @@ describe('CORS Middleware', () => { expect(response.status).toBe(204) expect(headersFunction).toHaveBeenCalledWith(req) - expect(response.headers.get('Access-Control-Allow-Headers')).toBe('') + // I-2: string return values are now correctly handled as single-element arrays + expect(response.headers.get('Access-Control-Allow-Headers')).toBe( + 'Content-Type', + ) expect(next).not.toHaveBeenCalled() }) @@ -680,12 +683,48 @@ describe('CORS Middleware', () => { const middleware = cors({ origin: false, // Explicitly set to false + credentials: true, }) const response = await middleware(req, next) expect(response.status).toBe(204) expect(response.headers.get('Access-Control-Allow-Origin')).toBeNull() + // Should not leak CORS policy headers to disallowed origins + expect(response.headers.get('Access-Control-Allow-Methods')).toBeNull() + expect(response.headers.get('Access-Control-Allow-Headers')).toBeNull() + expect(response.headers.get('Access-Control-Max-Age')).toBeNull() + expect( + response.headers.get('Access-Control-Allow-Credentials'), + ).toBeNull() + expect(next).not.toHaveBeenCalled() + }) + + it('should not leak CORS headers in preflight for rejected origins', async () => { + req = createTestRequest('OPTIONS', '/api/test') + req.headers = new Headers({ + Origin: 'https://evil.com', + 'Access-Control-Request-Method': 'POST', + }) + + const middleware = cors({ + origin: ['https://trusted.com'], + credentials: true, + maxAge: 3600, + methods: ['GET', 'POST'], + allowedHeaders: ['Content-Type', 'Authorization'], + }) + + const response = await middleware(req, next) + + expect(response.status).toBe(204) + expect(response.headers.get('Access-Control-Allow-Origin')).toBeNull() + expect(response.headers.get('Access-Control-Allow-Methods')).toBeNull() + expect(response.headers.get('Access-Control-Allow-Headers')).toBeNull() + expect(response.headers.get('Access-Control-Max-Age')).toBeNull() + expect( + response.headers.get('Access-Control-Allow-Credentials'), + ).toBeNull() expect(next).not.toHaveBeenCalled() }) @@ -701,4 +740,49 @@ describe('CORS Middleware', () => { ) }) }) + + describe('Preflight allowedHeaders Function Caching (I-2)', () => { + it('should call allowedHeaders function only once during preflight', async () => { + req = createTestRequest('OPTIONS', '/api/test') + req.headers = new Headers({ + Origin: 'https://example.com', + 'Access-Control-Request-Method': 'POST', + 'Access-Control-Request-Headers': 'Content-Type', + }) + + const headersFunction = jest.fn(() => ['Content-Type', 'Authorization']) + + const middleware = cors({ + origin: 'https://example.com', + allowedHeaders: headersFunction, + }) + + const response = await middleware(req, next) + + expect(response.status).toBe(204) + // I-2: should be called exactly once, not 3 times + expect(headersFunction).toHaveBeenCalledTimes(1) + expect(response.headers.get('Access-Control-Allow-Headers')).toBe( + 'Content-Type, Authorization', + ) + }) + + it('should still call allowedHeaders function for non-preflight requests', async () => { + req.headers = new Headers({Origin: 'https://example.com'}) + + const headersFunction = jest.fn(() => ['Content-Type']) + + const middleware = cors({ + origin: 'https://example.com', + allowedHeaders: headersFunction, + }) + + const response = await middleware(req, next) + + expect(headersFunction).toHaveBeenCalledTimes(1) + expect(response.headers.get('Access-Control-Allow-Headers')).toBe( + 'Content-Type', + ) + }) + }) }) diff --git a/test/unit/edge-cases.test.js b/test/unit/edge-cases.test.js index b44f3b9..0e3ff0b 100644 --- a/test/unit/edge-cases.test.js +++ b/test/unit/edge-cases.test.js @@ -286,9 +286,14 @@ describe('Router Edge Cases and Boundary Conditions', () => { // This should not break the router router.get('/test', null, (req) => ({message: 'success'})) + const originalError = console.error + console.error = () => {} + const req = createTestRequest('GET', '/test') const result = await router.fetch(req) + console.error = originalError + // Should handle gracefully expect(typeof result).toBe('object') }) diff --git a/test/unit/jwt-auth.test.js b/test/unit/jwt-auth.test.js index 1085476..d7e56b9 100644 --- a/test/unit/jwt-auth.test.js +++ b/test/unit/jwt-auth.test.js @@ -152,8 +152,9 @@ describe('JWT Authentication Middleware', () => { const response = await middleware(req, next) expect(response.status).toBe(200) - expect(req.apiKey).toBe(validApiKey) - expect(req.user).toEqual({apiKey: validApiKey}) + // M-7 fix: API key is now masked in req.apiKey and req.ctx.apiKey + expect(req.apiKey).toBe('test****-123') + expect(req.user).toEqual({apiKey: 'test****-123'}) expect(next).toHaveBeenCalled() }) @@ -193,7 +194,7 @@ describe('JWT Authentication Middleware', () => { const response = await middleware(req, next) expect(response.status).toBe(200) - expect(customValidator).toHaveBeenCalledWith(customApiKey) + expect(customValidator).toHaveBeenCalledWith(customApiKey, req) expect(req.user).toEqual({userId: '123', role: 'user'}) expect(next).toHaveBeenCalled() }) @@ -506,7 +507,7 @@ describe('JWT Authentication Middleware', () => { const response = await middleware(req, next) expect(response.status).toBe(200) - expect(singleParamValidator).toHaveBeenCalledWith(validApiKey) + expect(singleParamValidator).toHaveBeenCalledWith(validApiKey, req) }) it('should handle validateApiKey function with two parameters', async () => { @@ -779,7 +780,7 @@ describe('JWT Authentication Middleware', () => { const response = await middleware(req, next) expect(response.status).toBe(401) const errorData = await response.json() - expect(errorData.error).toBe('Token expired') + expect(errorData.error).toBe('Invalid or expired token') }) it('should handle JWT with malformed token', async () => { @@ -795,10 +796,8 @@ describe('JWT Authentication Middleware', () => { const response = await middleware(req, next) expect(response.status).toBe(401) const errorData = await response.json() - // This will trigger JWTInvalid error and get "Invalid token format" message - expect(['Invalid token format', 'Invalid token']).toContain( - errorData.error, - ) + // All JWT errors now return a uniform message to prevent validation oracle + expect(errorData.error).toBe('Invalid or expired token') }) it('should handle audience validation error', async () => { @@ -822,10 +821,8 @@ describe('JWT Authentication Middleware', () => { const response = await middleware(req, next) expect(response.status).toBe(401) const errorData = await response.json() - // The error message will contain "audience" which triggers the specific handler - expect(['Invalid token audience', 'Invalid token']).toContain( - errorData.error, - ) + // All JWT errors now return a uniform message to prevent validation oracle + expect(errorData.error).toBe('Invalid or expired token') }) it('should handle issuer validation error', async () => { @@ -849,10 +846,8 @@ describe('JWT Authentication Middleware', () => { const response = await middleware(req, next) expect(response.status).toBe(401) const errorData = await response.json() - // The error message will contain "issuer" which triggers the specific handler - expect(['Invalid token issuer', 'Invalid token']).toContain( - errorData.error, - ) + // All JWT errors now return a uniform message to prevent validation oracle + expect(errorData.error).toBe('Invalid or expired token') }) }) @@ -940,7 +935,7 @@ describe('JWT Authentication Middleware', () => { const response = await middleware(req, next) expect(response.status).toBe(401) const errorData = await response.json() - expect(errorData.error).toBe('Token signature verification failed') + expect(errorData.error).toBe('Invalid or expired token') }) it('should handle JWTInvalid error type', async () => { @@ -956,10 +951,8 @@ describe('JWT Authentication Middleware', () => { const response = await middleware(req, next) expect(response.status).toBe(401) const errorData = await response.json() - // Should get either "Invalid token format" or "Invalid token" - expect(['Invalid token format', 'Invalid token']).toContain( - errorData.error, - ) + // All JWT errors now return a uniform message to prevent validation oracle + expect(errorData.error).toBe('Invalid or expired token') }) }) }) @@ -1075,7 +1068,8 @@ describe('API Key Authentication Middleware (createAPIKeyAuth)', () => { const response = await middleware(req, next) expect(response.status).toBe(200) - expect(req.ctx.apiKey).toBe('valid-key-2') + // M-7 fix: API key is now masked + expect(req.ctx.apiKey).toBe('vali****ey-2') }) it('should validate single API key string', async () => { @@ -1224,7 +1218,7 @@ describe('Final Coverage Tests for Remaining Lines', () => { const response = await middleware(req, next) expect(response.status).toBe(401) const errorData = await response.json() - expect(errorData.error).toBe('Invalid token') + expect(errorData.error).toBe('Invalid or expired token') expect(throwingUnauthorizedResponse).toHaveBeenCalled() }) @@ -1247,7 +1241,7 @@ describe('Final Coverage Tests for Remaining Lines', () => { const response = await middleware(req, next) expect(response.status).toBe(401) const errorData = await response.json() - expect(errorData.error).toBe('Invalid token audience') + expect(errorData.error).toBe('Invalid or expired token') }) it('should handle JWT issuer validation error (line 314)', async () => { @@ -1269,6 +1263,124 @@ describe('Final Coverage Tests for Remaining Lines', () => { const response = await middleware(req, next) expect(response.status).toBe(401) const errorData = await response.json() - expect(errorData.error).toBe('Invalid token issuer') + expect(errorData.error).toBe('Invalid or expired token') + }) +}) + +describe('JWT Token Type Validation (L-4)', () => { + const {jwtAuth} = require('../../lib/middleware') + const {createTestRequest} = require('../helpers') + const {SignJWT, importJWK} = require('jose') + let req, next, testKey + + beforeEach(async () => { + req = createTestRequest('GET', '/protected') + next = jest.fn(() => new Response('Protected resource')) + + testKey = await importJWK({ + kty: 'oct', + k: 'AyM1SysPpbyDfgZld3umj1qzKObwVMkoqQ-EstJQLr_T-1qS0gZH75aKtMN3Yj0iPS4hcgUuTwjAzZr1Z9CAow', + }) + }) + + it('should accept token with correct typ header', async () => { + const jwt = await new SignJWT({sub: 'user123'}) + .setProtectedHeader({alg: 'HS256', typ: 'JWT'}) + .setIssuedAt() + .setExpirationTime('1h') + .sign(testKey) + + req.headers = new Headers({Authorization: `Bearer ${jwt}`}) + + const middleware = jwtAuth({ + secret: testKey, + algorithms: ['HS256'], + requiredTokenType: 'JWT', + }) + + const response = await middleware(req, next) + expect(response.status).toBe(200) + expect(next).toHaveBeenCalled() + }) + + it('should reject token with wrong typ header', async () => { + const jwt = await new SignJWT({sub: 'user123'}) + .setProtectedHeader({alg: 'HS256', typ: 'at+jwt'}) + .setIssuedAt() + .setExpirationTime('1h') + .sign(testKey) + + req.headers = new Headers({Authorization: `Bearer ${jwt}`}) + + const middleware = jwtAuth({ + secret: testKey, + algorithms: ['HS256'], + requiredTokenType: 'JWT', + }) + + const response = await middleware(req, next) + expect(response.status).toBe(401) + const errorData = await response.json() + expect(errorData.error).toBe('Invalid token type') + }) + + it('should reject token with missing typ header when required', async () => { + const jwt = await new SignJWT({sub: 'user123'}) + .setProtectedHeader({alg: 'HS256'}) + .setIssuedAt() + .setExpirationTime('1h') + .sign(testKey) + + req.headers = new Headers({Authorization: `Bearer ${jwt}`}) + + const middleware = jwtAuth({ + secret: testKey, + algorithms: ['HS256'], + requiredTokenType: 'JWT', + }) + + const response = await middleware(req, next) + expect(response.status).toBe(401) + const errorData = await response.json() + expect(errorData.error).toBe('Invalid token type') + }) + + it('should perform case-insensitive typ comparison', async () => { + const jwt = await new SignJWT({sub: 'user123'}) + .setProtectedHeader({alg: 'HS256', typ: 'jwt'}) + .setIssuedAt() + .setExpirationTime('1h') + .sign(testKey) + + req.headers = new Headers({Authorization: `Bearer ${jwt}`}) + + const middleware = jwtAuth({ + secret: testKey, + algorithms: ['HS256'], + requiredTokenType: 'JWT', + }) + + const response = await middleware(req, next) + expect(response.status).toBe(200) + expect(next).toHaveBeenCalled() + }) + + it('should skip typ validation when requiredTokenType not set', async () => { + const jwt = await new SignJWT({sub: 'user123'}) + .setProtectedHeader({alg: 'HS256'}) + .setIssuedAt() + .setExpirationTime('1h') + .sign(testKey) + + req.headers = new Headers({Authorization: `Bearer ${jwt}`}) + + const middleware = jwtAuth({ + secret: testKey, + algorithms: ['HS256'], + }) + + const response = await middleware(req, next) + expect(response.status).toBe(200) + expect(next).toHaveBeenCalled() }) }) diff --git a/test/unit/middleware.test.js b/test/unit/middleware.test.js index 4ff5642..ae5b6c5 100644 --- a/test/unit/middleware.test.js +++ b/test/unit/middleware.test.js @@ -266,9 +266,10 @@ describe('Middleware Next Function Unit Tests', () => { const nextFn = createNextFunction(middlewares, final, errorHandler) - // Async errors in middleware are not currently caught by the next function - // This is a limitation of the current implementation - await expect(nextFn({})).rejects.toThrow('Async middleware error') + // Async errors are now caught by the next function + const result = await nextFn({}) + expect(result.errorHandled).toBe(true) + expect(result.errorMessage).toBe('Async middleware error') }) it('should handle errors passed to next function', async () => { diff --git a/test/unit/rate-limit.test.js b/test/unit/rate-limit.test.js index 6c7d170..9bf1eb2 100644 --- a/test/unit/rate-limit.test.js +++ b/test/unit/rate-limit.test.js @@ -8,6 +8,7 @@ describe('Rate Limit Middleware', () => { beforeEach(() => { req = createTestRequest('GET', '/api/test') + req.ip = '127.0.0.1' req.socket = {remoteAddress: '127.0.0.1'} next = jest.fn(() => new Response('Success')) }) @@ -273,6 +274,7 @@ describe('Rate Limit Middleware', () => { const promises = [] for (let i = 0; i < 5; i++) { const newReq = createTestRequest('GET', '/api/test') + newReq.ip = '127.0.0.1' newReq.socket = {remoteAddress: '127.0.0.1'} promises.push(middleware(newReq, next)) } @@ -448,7 +450,7 @@ describe('Rate Limit Middleware', () => { expect(customStore.increment).toHaveBeenCalled() }) - it('should use injected store from request', async () => { + it('should use constructor-injected store (not request-level injection)', async () => { const customStore = { increment: jest.fn().mockResolvedValue({ totalHits: 1, @@ -456,12 +458,11 @@ describe('Rate Limit Middleware', () => { }), } - // Inject store into request - req.rateLimitStore = customStore - + // Use constructor-level dependency injection (secure approach) const middleware = rateLimit({ windowMs: 60000, max: 5, + store: customStore, }) await middleware(req, next) @@ -473,12 +474,13 @@ describe('Rate Limit Middleware', () => { describe('Default Key Generator', () => { const {defaultKeyGenerator} = require('../../lib/middleware/rate-limit') - it('should use CF-Connecting-IP header when available', () => { + it('should use req.ip when available (connection-level IP)', () => { const testReq = { + ip: '1.2.3.4', headers: new Headers([ - ['cf-connecting-ip', '1.2.3.4'], - ['x-real-ip', '5.6.7.8'], - ['x-forwarded-for', '9.10.11.12, 13.14.15.16'], + ['cf-connecting-ip', '5.6.7.8'], + ['x-real-ip', '9.10.11.12'], + ['x-forwarded-for', '13.14.15.16'], ]), } @@ -486,11 +488,12 @@ describe('Rate Limit Middleware', () => { expect(key).toBe('1.2.3.4') }) - it('should use X-Real-IP header when CF-Connecting-IP not available', () => { + it('should use req.remoteAddress when req.ip not available', () => { const testReq = { + remoteAddress: '5.6.7.8', headers: new Headers([ - ['x-real-ip', '5.6.7.8'], - ['x-forwarded-for', '9.10.11.12, 13.14.15.16'], + ['x-real-ip', '9.10.11.12'], + ['x-forwarded-for', '13.14.15.16'], ]), } @@ -498,22 +501,57 @@ describe('Rate Limit Middleware', () => { expect(key).toBe('5.6.7.8') }) - it('should use first IP from X-Forwarded-For header', () => { + it('should use req.socket.remoteAddress as last IP fallback', () => { const testReq = { - headers: new Headers([['x-forwarded-for', '9.10.11.12, 13.14.15.16']]), + socket: {remoteAddress: '10.0.0.1'}, + headers: new Headers([['x-forwarded-for', '13.14.15.16']]), } const key = defaultKeyGenerator(testReq) - expect(key).toBe('9.10.11.12') + expect(key).toBe('10.0.0.1') }) - it('should return unknown when no IP headers available', () => { + it('should prefer req.ip over req.socket.remoteAddress', () => { const testReq = { + ip: '1.2.3.4', + socket: {remoteAddress: '10.0.0.1'}, headers: new Headers(), } const key = defaultKeyGenerator(testReq) - expect(key).toBe('unknown') + expect(key).toBe('1.2.3.4') + }) + + it('should prefer req.remoteAddress over req.socket.remoteAddress', () => { + const testReq = { + remoteAddress: '5.6.7.8', + socket: {remoteAddress: '10.0.0.1'}, + headers: new Headers(), + } + + const key = defaultKeyGenerator(testReq) + expect(key).toBe('5.6.7.8') + }) + + it('should not trust proxy headers by default', () => { + const testReq = { + headers: new Headers([['x-forwarded-for', '9.10.11.12, 13.14.15.16']]), + } + + const key = defaultKeyGenerator(testReq) + expect(key.startsWith('unknown:')).toBe(true) + }) + + it('should return unique key when no IP headers available', () => { + const testReq = { + headers: new Headers(), + } + + const key1 = defaultKeyGenerator(testReq) + const key2 = defaultKeyGenerator(testReq) + expect(key1.startsWith('unknown:')).toBe(true) + expect(key2.startsWith('unknown:')).toBe(true) + expect(key1).not.toBe(key2) // Each call should produce a unique key }) }) @@ -632,4 +670,59 @@ describe('Rate Limit Middleware', () => { expect(await response.text()).toBe('Custom string response') }) }) + + describe('Minimal Standard Headers Mode (L-6)', () => { + it('should only add Retry-After on 429 with minimal mode', async () => { + const middleware = rateLimit({ + windowMs: 60000, + max: 1, + standardHeaders: 'minimal', + }) + + // First request should pass without rate limit headers + const response1 = await middleware(req, next) + expect(response1.headers.get('X-RateLimit-Limit')).toBeNull() + expect(response1.headers.get('X-RateLimit-Remaining')).toBeNull() + expect(response1.headers.get('X-RateLimit-Reset')).toBeNull() + expect(response1.headers.get('X-RateLimit-Used')).toBeNull() + expect(response1.headers.get('Retry-After')).toBeNull() + + jest.clearAllMocks() + + // Second request should be rate limited with only Retry-After + const response2 = await middleware(req, next) + expect(response2.status).toBe(429) + expect(response2.headers.get('Retry-After')).toBeTruthy() + expect(response2.headers.get('X-RateLimit-Limit')).toBeNull() + expect(response2.headers.get('X-RateLimit-Remaining')).toBeNull() + expect(response2.headers.get('X-RateLimit-Used')).toBeNull() + }) + + it('should add full headers when standardHeaders is true', async () => { + const middleware = rateLimit({ + windowMs: 60000, + max: 5, + standardHeaders: true, + }) + + const response = await middleware(req, next) + expect(response.headers.get('X-RateLimit-Limit')).toBe('5') + expect(response.headers.get('X-RateLimit-Remaining')).toBe('4') + expect(response.headers.get('X-RateLimit-Used')).toBe('1') + expect(response.headers.get('X-RateLimit-Reset')).toBeTruthy() + }) + + it('should add no headers when standardHeaders is false', async () => { + const middleware = rateLimit({ + windowMs: 60000, + max: 5, + standardHeaders: false, + }) + + const response = await middleware(req, next) + expect(response.headers.get('X-RateLimit-Limit')).toBeNull() + expect(response.headers.get('X-RateLimit-Remaining')).toBeNull() + expect(response.headers.get('X-RateLimit-Used')).toBeNull() + }) + }) }) diff --git a/test/unit/router.test.js b/test/unit/router.test.js index 96f1014..911e97b 100644 --- a/test/unit/router.test.js +++ b/test/unit/router.test.js @@ -237,12 +237,17 @@ describe('Sequential Router Unit Tests', () => { throw new Error('Test error') }) + const originalError = console.error + console.error = () => {} + const response = await routerInstance.fetch( createTestRequest('GET', '/error'), ) + console.error = originalError + expect(response.status).toBe(500) - expect(await response.text()).toBe('Test error') + expect(await response.text()).toBe('Internal Server Error') }) it('should use custom error handler', async () => { @@ -269,22 +274,18 @@ describe('Sequential Router Unit Tests', () => { throw new Error('Async error') }) - // Test that the router can handle the request, even if the handler throws - // The actual error handling behavior may vary based on the router implementation - try { - const response = await routerInstance.fetch( - createTestRequest('GET', '/async-error'), - ) - - // If the router handles the error, it should return a 500 response - if (response && response.status) { - expect(response.status).toBe(500) - expect(await response.text()).toBe('Async error') - } - } catch (error) { - // If the error is not caught by the router, verify it's the expected error - expect(error.message).toBe('Async error') - } + const originalError = console.error + console.error = () => {} + + // Async errors are now caught by the router and handled by the error handler + const response = await routerInstance.fetch( + createTestRequest('GET', '/async-error'), + ) + + console.error = originalError + + expect(response.status).toBe(500) + expect(await response.text()).toBe('Internal Server Error') }) })