Skip to content

fix: harden CORS proxy with timeout, size limit, and encoded path check #145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rsbh wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: harden CORS proxy with timeout, size limit, and encoded path check #145

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
40a19d0
fix: harden CORS proxy with timeout, size limit, and encoded path check
rsbh Jun 3, 2026
71cb3b8
fix: increase proxy body size limit to 10MB
rsbh Jun 3, 2026
32ff460
fix: increase proxy upstream timeout to 2 minutes
rsbh Jun 3, 2026
e1a8382
refactor: use StatusCodes from http-status-codes package
rsbh Jun 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 32 additions & 7 deletions packages/chronicle/src/server/api/apis-proxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { defineHandler, HTTPError } from 'nitro';
import { StatusCodes } from 'http-status-codes';
import { loadConfig } from '@/lib/config';
import { loadApiSpecs } from '@/lib/openapi';

const MAX_BODY_SIZE = 10_485_760; // 10 MB
const UPSTREAM_TIMEOUT_MS = 120_000;

interface ProxyRequest {
specName: string;
method: string;
Expand All @@ -10,17 +14,34 @@ interface ProxyRequest {
body?: unknown;
}

function isPathSafe(p: string): boolean {
let decoded: string;
try {
decoded = decodeURIComponent(p);
} catch {
return false;
}
if (/^[a-z]+:\/\//i.test(decoded)) return false;
const normalized = new URL(decoded, 'http://localhost').pathname;
return !normalized.split('/').includes('..');
}
Comment on lines +17 to +27
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Path traversal check is bypassed due to URL normalization resolving .. segments.

The new URL(decoded, 'http://localhost').pathname normalizes the path by resolving .. segments before the check runs. For example, /../../../etc/passwd becomes /etc/passwd after normalization, so split('/').includes('..') returns false and the path is deemed safe. However, line 60 then concatenates the original path (containing ..) with spec.server.url, allowing the traversal to reach the upstream server.

Check the raw decoded path for .. segments before normalization:

🐛 Proposed fix 
 function isPathSafe(p: string): boolean {
   let decoded: string;
   try {
     decoded = decodeURIComponent(p);
   } catch {
     return false;
   }
   if (/^[a-z]+:\/\//i.test(decoded)) return false;
-  const normalized = new URL(decoded, 'http://localhost').pathname;
-  return !normalized.split('/').includes('..');
+  // Check raw decoded path for traversal before any normalization
+  if (decoded.split('/').includes('..')) return false;
+  // Also reject protocol-relative URLs (//evil.com)
+  if (decoded.startsWith('//')) return false;
+  return true;
 }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/chronicle/src/server/api/apis-proxy.ts` around lines 16 - 26, The
isPathSafe function currently normalizes the path with new URL(decoded,
'http://localhost').pathname which resolves `..` before the safety check; change
the logic to first inspect the raw decoded variable for any `..` path segments
(e.g., check decoded.split('/').includes('..') or a regex) and reject if
present, then proceed to validate scheme (the /^[a-z]+:\/\//i test) and use
normalized (new URL(...).pathname) only afterwards; ensure the function returns
false when the raw decoded contains `..` so concatenation later with
spec.server.url cannot be exploited.


export default defineHandler(async event => {
if (event.req.method !== 'POST') {
throw new HTTPError({ status: 405, message: 'Method not allowed' });
throw new HTTPError({ status: StatusCodes.METHOD_NOT_ALLOWED, message: 'Method not allowed' });
}

const contentLength = parseInt(event.req.headers.get('content-length') ?? '0', 10);
if (contentLength > MAX_BODY_SIZE) {
throw new HTTPError({ status: StatusCodes.CONTENT_TOO_LARGE, message: `Request body too large (max ${MAX_BODY_SIZE} bytes)` });
}

const { specName, method, path, headers, body } =
(await event.req.json()) as ProxyRequest;

if (!specName || !method || !path) {
throw new HTTPError({
status: 400,
status: StatusCodes.BAD_REQUEST,
message: 'Missing specName, method, or path'
});
}
Expand All @@ -30,11 +51,11 @@ export default defineHandler(async event => {
const spec = specs.find(s => s.name === specName);

if (!spec) {
throw new HTTPError({ status: 404, message: `Unknown spec: ${specName}` });
throw new HTTPError({ status: StatusCodes.NOT_FOUND, message: `Unknown spec: ${specName}` });
}

if (/^[a-z]+:\/\//i.test(path) || path.includes('..')) {
throw new HTTPError({ status: 400, message: 'Invalid path' });
if (!isPathSafe(path)) {
throw new HTTPError({ status: StatusCodes.BAD_REQUEST, message: 'Invalid path' });
}

const url = spec.server.url + path;
Expand All @@ -43,7 +64,8 @@ export default defineHandler(async event => {
const response = await fetch(url, {
method,
headers,
body: body ? JSON.stringify(body) : undefined
body: body ? JSON.stringify(body) : undefined,
signal: AbortSignal.timeout(UPSTREAM_TIMEOUT_MS),
});

const contentType = response.headers.get('content-type') ?? '';
Expand All @@ -64,12 +86,15 @@ export default defineHandler(async event => {
headers: responseHeaders
});
} catch (error) {
if (error instanceof DOMException && error.name === 'TimeoutError') {
throw new HTTPError({ status: StatusCodes.GATEWAY_TIMEOUT, message: `Upstream request timed out after ${UPSTREAM_TIMEOUT_MS}ms` });
}
const message =
error instanceof Error
? `${error.message}${error.cause ? `: ${(error.cause as Error).message}` : ''}`
: 'Request failed';
throw new HTTPError({
status: 502,
status: StatusCodes.BAD_GATEWAY,
message: `Could not reach ${url}\n${message}`
});
}
Expand Down
Loading