Harden path canonicalization, middleware consistency, and hot paths

.github/workflows/tests.yaml

@@ -5,18 +5,18 @@ jobs:
   testing:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
 
       - name: Setup Bun
-        uses: oven-sh/setup-bun@v1
+        uses: oven-sh/setup-bun@v2
         with:
           bun-version: latest
 
       - name: Install dependencies
         run: bun install
 
       - name: Linting
-        run: bun run format
+        run: bun run lint
 
       - name: Run tests
         run: bun test

README.md

@@ -363,7 +363,7 @@ router.get('/api/risky', async (req: ZeroRequest) => {
 - **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)
+- **Route Filter Bypass Prevention**: URL path normalization (double-slash collapse, URI decoding, `%2F` preservation, `.` / `..` resolution)
 - **Frozen Route Params**: Parameterless routes receive an immutable `Object.freeze({})` to prevent cross-request data leakage
 
 #### **Error Handling**
@@ -499,7 +499,7 @@ router.use(
 - **Fast parameter parsing**: Optimized URL parameter extraction with caching
 - **Query string parsing**: Uses `fast-querystring` for optimal performance
 - **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)
+- **URL normalization**: Single-pass URL parsing with path normalization (double-slash collapse, URI decoding, `.` / `..` resolution)
 
 ### Benchmark Results
 
@@ -607,6 +607,39 @@ _Benchmarks run on Bun v1.2.2 with simple JSON response routes. Results may vary
 
 ## Changelog
 
+### v1.3.1 — Follow-up Hardening
+
+Adversarial review of the v1.3.0 hardening pass. Remaining gaps in path handling, middleware consistency, and hot-path cost are addressed.
+
+#### Security
+
+- **Dot-segment resolution** — `%2e%2e` / `..` / `.` are now resolved after URI decoding so routing and `excludePaths` cannot disagree (the previous pass collapsed slashes and decoded, but left `..` in the path).
+- **Shared canonical path** — JWT, rate-limit, logger, and Prometheus `excludePaths` now use the same `req.path` the router computed, instead of `new URL(req.url).pathname`.
+- **Logger / Prometheus prefix matching** — `excludePaths` now uses exact-or-boundary matching (same as JWT and rate-limit). `/health` no longer skips `/healthcheck`.
+- **CORS preflight `Vary: Origin`** — set for static string origins as well as function/array origins.
+- **CORS `null` origin** — rejected for all non-wildcard configs, including `origin: 'null'`.
+- **Logger request IDs** — header-supplied IDs are stripped of control characters and capped at 128 chars.
+- **Logger response headers** — `Set-Cookie`, `Authorization`, `Cookie`, and `Proxy-Authorization` are redacted in the default serializer.
+- **JWT optional mode** — `req.ctx.authError` is a generic message, not the raw jose error.
+- **MemoryStore bounds** — `maxKeys` (default 10,000) plus amortized cleanup. When full, **new keys fail closed** (429) instead of evicting live counters.
+- **Write-once canonical path** — security middleware reads `Symbol.for('0http.canonicalPath')` or re-parses `req.url`; mutable `req.path` is ignored.
+
+#### Performance
+
+- Router skips decode / slash-collapse / dot-resolution when the path does not need them.
+- Reused frozen empty query object; single-pass param copy.
+- Middleware no longer allocates `new URL()` on every request when `req.path` is set.
+- JSON nesting scan short-circuits at the depth limit; body reader uses tracked byte length.
+- Custom `jsonTypes` parsers are cached instead of being created per request.
+
+#### Ergonomics
+
+- Types now include `req.path`, `req.body`, `req.files`, logger options (`level`, `requestIdHeader`, `generateRequestId`), `errorHandler(err, req)`, `extended` on body parser, and `maxKeys` on rate limit.
+- `ParsedFile.data` is `Uint8Array` (matches the implementation). `req.jwt.token` removed from types (removed in 1.3.0).
+- CORS origin validators receive `(origin, req)` as documented.
+- CORS preflight methods are compared case-insensitively.
+- CI runs `bun run lint` (check) instead of `format` (write). Bench script points at `bench.ts`.
+
 ### 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.

SECURITY_REVIEWS.md

@@ -3,10 +3,98 @@
 > 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.**
+>
+> Follow-up adversarial review (2026-09-05) against `0http-bun@1.3.0` found additional
+> gaps that the first pass claimed to have closed. Those are tracked and fixed below
+> as **R2-*** (review 2).
 
 ## Remediation Progress
 
-> **Fixed:** 6/6 Critical, 13/13 High, 13/13 Medium, 7/7 Low, 4/4 Info = **43/43 vulnerabilities resolved** ✅
+> **Review 1 (v1.3.0):** 6/6 Critical, 13/13 High, 13/13 Medium, 7/7 Low, 4/4 Info = **43/43 vulnerabilities resolved** ✅
+>
+> **Review 2 (v1.3.1):** 1/1 High, 5/5 Medium, 3/3 Low = **9/9 follow-up findings resolved** ✅
+>
+> **Review 2 adversarial pass:** FIFO `maxKeys` eviction and trusted `req.path` were rejected and replaced with fail-closed admission and a write-once canonical-path symbol.
+
+---
+
+## REVIEW 2 — Adversarial follow-up (v1.3.0 → v1.3.1)
+
+The v1.3.0 pass fixed the issues it named, but several fixes were incomplete or inconsistent across modules. These were found by re-reading the claimed remediations against the actual code.
+
+### ✅ R2-H1: Path normalization did not resolve `.` / `..` (auth / route filter bypass)
+
+- **Status:** FIXED
+- **Files:** `lib/path.js`, `lib/router/sequential.js`, JWT / rate-limit / logger / prometheus
+- **Issue:** M-2 claimed to prevent bypass via `%2e%2e`, but after `decodeURIComponent` the path still contained literal `..`. Middleware `excludePaths` used `new URL(req.url).pathname` (which *does* resolve dots) while the router did not. A request to `/admin/../health` could skip JWT (`pathname === '/health'`) while routing on `/admin/../health`.
+- **Fix applied:** Shared `normalizePathname()` collapses slashes, decodes (preserving `%2F`), then resolves `.` / `..`. All middleware reads `req.path` when present.
+
+### ✅ R2-M1: Logger and Prometheus `excludePaths` still used prefix matching
+
+- **Status:** FIXED
+- **Files:** `lib/middleware/logger.js`, `lib/middleware/prometheus.js`
+- **Issue:** H-7 / M-11 were fixed for JWT and rate-limit. Logger and Prometheus still used `pathname.startsWith(path)`, so `/health` skipped `/healthcheck`.
+- **Fix applied:** Shared `isExcludedPath()` — exact or `path + '/'` boundary.
+
+### ✅ R2-M2: CORS preflight omitted `Vary: Origin` for static string origins
+
+- **Status:** FIXED
+- **File:** `lib/middleware/cors.js`
+- **Issue:** L-7 said Vary is set for all non-wildcard origins. Actual requests did; preflight only set Vary for function/array origins. CDNs could cache a preflight for the wrong origin.
+- **Fix applied:** `applyVaryOrigin()` on every non-wildcard preflight and response.
+
+### ✅ R2-M3: `origin: 'null'` string config allowed sandboxed iframes
+
+- **Status:** FIXED
+- **File:** `lib/middleware/cors.js`
+- **Issue:** Null-origin rejection ran only for array/function configs. A string origin of `'null'` would reflect `Origin: null`.
+- **Fix applied:** Missing/`null` origins rejected for every non-wildcard configuration.
+
+### ✅ R2-M4: MemoryStore had no `maxKeys` and scanned all keys on every request
+
+- **Status:** FIXED
+- **File:** `lib/middleware/rate-limit.js`
+- **Issue:** H-10 bounded the sliding-window limiter. The default fixed-window `MemoryStore` still grew without limit (worse after I-1 unique unknown keys) and ran O(n) cleanup on every increment.
+- **Fix applied:** `maxKeys` (default 10,000) and amortized cleanup every 100 increments.
+
+### ✅ R2-M5: Client-supplied request IDs and default response logs leaked secrets
+
+- **Status:** FIXED
+- **File:** `lib/middleware/logger.js`
+- **Issue:** `requestIdHeader` values were copied verbatim (CRLF injection / log forging). Default response logging dumped all headers, including `Set-Cookie`.
+- **Fix applied:** Sanitize request IDs (strip controls, max 128 chars). Redact `Set-Cookie`, `Authorization`, `Cookie`, `Proxy-Authorization`.
+
+### ✅ R2-L1: Optional JWT mode stored raw `error.message` on `req.ctx`
+
+- **Status:** FIXED
+- **File:** `lib/middleware/jwt-auth.js`
+- **Fix applied:** `req.ctx.authError` is now `'Invalid or expired token'`.
+
+### ✅ R2-L2: CORS origin validator ignored the documented `req` argument
+
+- **Status:** FIXED
+- **File:** `lib/middleware/cors.js`
+- **Fix applied:** Validators are called as `origin(requestOrigin, req)`.
+
+### ✅ R2-H2 (adversarial): FIFO `maxKeys` eviction reset victim counters
+
+- **Status:** FIXED
+- **File:** `lib/middleware/rate-limit.js`
+- **Issue:** First-pass `maxKeys` deleted the oldest Map entry, which an attacker could use to rotate keys and reset a victim's window.
+- **Fix applied:** When the store is full, *new* keys fail closed (`totalHits` treated as over limit). Existing keys still increment. Same policy on the sliding-window limiter.
+
+### ✅ R2-M6 (adversarial): `getRequestPath()` trusted mutable `req.path`
+
+- **Status:** FIXED
+- **File:** `lib/path.js`
+- **Issue:** Security middleware preferred `req.path`, so application code (or a confused middleware) could set `req.path = '/health'` and skip JWT / rate-limit exclusions.
+- **Fix applied:** Router stores the canonical path on a write-once `Symbol.for('0http.canonicalPath')`. `getRequestPath()` uses that symbol or re-parses `req.url` — never `req.path`.
+
+### ✅ R2-L3: CORS preflight method check was case-sensitive
+
+- **Status:** FIXED
+- **File:** `lib/middleware/cors.js`
+- **Fix applied:** Methods compared case-insensitively.
 
 ---
 

common.d.ts

@@ -4,22 +4,30 @@ import {Logger} from 'pino'
 export interface IRouterConfig {
   cacheSize?: number
   defaultRoute?: RequestHandler
-  errorHandler?: (err: Error) => Response | Promise<Response>
+  errorHandler?: (err: Error, req?: ZeroRequest) => Response | Promise<Response>
   port?: number
 }
 
 export type StepFunction = (error?: unknown) => Response | Promise<Response>
 
 export interface ParsedFile {
+  filename?: string
+  originalName?: string
   name: string
   size: number
   type: string
-  data: File
+  mimetype?: string
+  data: Uint8Array
 }
 
 export type ZeroRequest = Request & {
+  // Canonical pathname set by the router (slash-collapsed, decoded, dot-resolved)
+  path?: string
   params: Record<string, string>
   query: Record<string, string>
+  // Parsed body / files (set by body-parser middleware)
+  body?: any
+  files?: Record<string, ParsedFile | ParsedFile[]>
   // Connection-level IP address (set via Bun.serve's server.requestIP or upstream middleware)
   ip?: string
   remoteAddress?: string
@@ -38,19 +46,22 @@ export type ZeroRequest = Request & {
   jwt?: {
     payload: any
     header: any
-    token: string
   }
   apiKey?: string
+  log?: Logger
+  requestId?: string
   // Context object for middleware data
   ctx?: {
     log?: Logger
+    requestId?: string
     user?: any
     jwt?: {
       payload: any
       header: any
-      token: string
     }
     apiKey?: string
+    authError?: string
+    authAttempted?: boolean
     rateLimit?: {
       limit: number
       used: number

lib/middleware/README.md

@@ -575,6 +575,7 @@ router.use(
       },
     },
     logBody: false,
+    // exact or boundary match — '/health' does not skip '/healthcheck'
     excludePaths: ['/health', '/metrics'],
   }),
 )
@@ -660,7 +661,7 @@ const prometheus = createPrometheusIntegration({
   // Control default Node.js metrics collection
   collectDefaultMetrics: true,
 
-  // Exclude paths from metrics collection (optimized for performance)
+  // Exclude paths from metrics collection (exact or boundary match)
   excludePaths: ['/health', '/ping', '/favicon.ico'],
 
   // Skip certain HTTP methods