docs: add security-hardened proxy.ts example and README section#230
docs: add security-hardened proxy.ts example and README section#230apuodesu wants to merge 1 commit intocloudflare:mainfrom
Conversation
Add a proxy.ts to the app-router-cloudflare example with: - OWASP-recommended security response headers - Double-encoded path traversal detection - Explicit route matcher covering /api/* routes Add a 'Security hardening' section to the README pointing users to the example. This helps vinext users adopt production security defaults without having to discover best practices independently.
|
/bigbonk review |
commit: |
There was a problem hiding this comment.
Thanks for contributing this — having a security-hardening example is a great idea. I have concerns about the path traversal detection logic that I think should be addressed before merging, plus a few smaller suggestions.
Main issue: the double-encoded path traversal check is inaccurate and misleading
The path traversal detection on lines 16-19 has several problems that could confuse users who copy-paste this as a template:
-
request.urlis a serialized URL, not the raw HTTP request line. By the time the proxy runs, the URL has been parsed by theURLconstructor (inrunMiddlewareatmiddleware.ts:248) and re-serialized. Double-encoded sequences like%252fin the original HTTP request may not survive this round-trip in the same form. The check is testing something different from what the comment describes. -
The comment says
%2e%2e = encoded '..'but the regex matches%25xx(double-encoded sequences). Single-encoded..(%2e%2e) is already handled by vinext's built-innormalizePath+decodeURIComponentin the middleware runner (middleware.ts:253-259). The comment and the regex don't match. -
The
%5ccheck is too broad. It matches backslashes anywhere in the entire URL string, including query parameters and fragments where%5ccan be legitimate (e.g., Windows file paths in user input, regex patterns in query strings). -
False positives on
%25. The regex/%25[0-9a-fA-F]{2}/would block any URL containing a literal percent sign followed by two hex chars — e.g., a query parameter like?q=100%25(which encodes to?q=100%2525and round-trips as?q=100%25) or URLs with legitimate double-encoding in external API callbacks.
Given that vinext already handles path traversal via decodeURIComponent + normalizePath in the middleware runner, and returns 400 on malformed percent-encoding, I'd recommend either:
- Removing the path traversal check entirely and noting in the docstring that vinext handles this built-in, or
- Narrowing it to only the pathname (use
new URL(request.url).pathnameinstead ofrequest.url) and fixing the comment to accurately describe what's being matched
The security response headers section (lines 21-36) is clean and useful — that's the real value of this example.
| // %252f = double-encoded '/', %2e%2e = encoded '..', %5c = encoded '\' | ||
| // These can bypass route matching when the server decodes at different stages. | ||
| const rawUrl = request.url; | ||
| if (/%25[0-9a-fA-F]{2}/.test(rawUrl) || /%5[cC]/.test(rawUrl)) { |
There was a problem hiding this comment.
This regex tests against the full serialized request.url (including origin, query string, and fragment), not the raw HTTP request line. It will produce false positives for legitimate URLs — e.g., a query string containing %25 (a literal percent sign) or %5c (a backslash in user input).
Also, by the time this code runs, the URL has already been parsed and re-serialized through a URL constructor in vinext's runMiddleware, so the encoding may not match what was originally sent by the client.
Suggestion: either remove this check (vinext handles path traversal built-in via decodeURIComponent + normalizePath in the middleware runner) or scope it to just the pathname:
| if (/%25[0-9a-fA-F]{2}/.test(rawUrl) || /%5[cC]/.test(rawUrl)) { | |
| const { pathname } = new URL(request.url); | |
| if (/%25[0-9a-fA-F]{2}/.test(pathname) || /%5[cC]/.test(pathname)) { |
| */ | ||
| export default function proxy(request: NextRequest) { | ||
| // Block double-encoded path traversal attempts. | ||
| // %252f = double-encoded '/', %2e%2e = encoded '..', %5c = encoded '\' |
There was a problem hiding this comment.
The comment says %2e%2e = encoded '..' but the regex on line 17 doesn't match %2e%2e — it matches %25xx (double-encoded). These are different things. Single-encoded %2e%2e (..) is already handled by vinext's built-in normalizePath. The comment should be updated to accurately reflect what the regex catches, or removed if the check is removed.
| // %252f = double-encoded '/', %2e%2e = encoded '..', %5c = encoded '\' | |
| // Block double-encoded path traversal attempts (%25xx) and encoded backslashes (%5c). | |
| // Single-encoded sequences like %2e%2e are already handled by vinext's built-in normalization. |
| * | ||
| * See: https://owasp.org/www-project-secure-headers/ | ||
| */ | ||
| export default function proxy(request: NextRequest) { |
There was a problem hiding this comment.
Minor: vinext supports both export default and a named proxy export for proxy files (matching Next.js 16 convention). Since this is meant as a best-practice template, consider using the named export to be explicit about the Next.js 16 convention:
| export default function proxy(request: NextRequest) { | |
| export default function proxy(request: NextRequest) { |
Actually, export default is fine and works correctly — just noting that export function proxy(...) is also valid if you want to align with the named-export convention mentioned in the vinext middleware runner.
|
|
||
| ## Security hardening | ||
|
|
||
| vinext handles URL normalization, path traversal prevention, and internal header stripping automatically. For production deployments, we recommend adding a `proxy.ts` (Next.js 16) or `middleware.ts` (Next.js 15) with: |
There was a problem hiding this comment.
Nit: the phrase "we recommend" is a strong endorsement that implies this is necessary for security. Since the README sentence right before it says vinext already handles URL normalization, path traversal prevention, and internal header stripping automatically, users might wonder why they need to add a proxy.ts at all.
Consider softening to something like "you may want to add" and clarifying that the built-in protections handle the critical stuff, while these headers are best-practice additions:
| vinext handles URL normalization, path traversal prevention, and internal header stripping automatically. For production deployments, we recommend adding a `proxy.ts` (Next.js 16) or `middleware.ts` (Next.js 15) with: | |
| vinext handles URL normalization, path traversal prevention, and internal header stripping automatically. For production deployments, you may also want to add a `proxy.ts` (Next.js 16) or `middleware.ts` (Next.js 15) with: |
|
Review submitted successfully. Here's a summary of my review: Review: Request ChangesMain concern: The path traversal detection logic (lines 16-19 of Four specific issues identified:
Recommendation: Either remove the path traversal check entirely (vinext handles this built-in) or narrow it to only the pathname and fix the comments. The security response headers portion (lines 21-36) and the README section are clean and useful — that's the real value of this PR. |
Summary
Add a production-ready
proxy.tsexample toexamples/app-router-cloudflare/and a corresponding "Security hardening" section to the README.Motivation
vinext v0.0.18 has strong built-in protections (URL normalization, path traversal prevention, internal header stripping, image endpoint validation). However, there's no guidance for users on adding security response headers or configuring explicit proxy route matchers for production deployments.
After deploying a vinext app to production (soulo.ai), I found that users benefit from a ready-to-use proxy template that adds:
X-Content-Type-Options,X-Frame-Options,Referrer-Policy,Permissions-Policy%25xxand%5cpatterns as defense-in-depth/api/*Changes
examples/app-router-cloudflare/proxy.ts— New file. Documented, minimal, copy-pasteable security proxyREADME.md— New "Security hardening" section after "Known limitations" pointing to the exampleTesting
Deployed and verified on production (
soulo.ai):%2e%2epath traversal returns 400