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

[rsbh]

Summary

• Decode path before traversal check to catch %2e%2e encoded variants
• Add AbortSignal.timeout(30s) to upstream fetch calls
• Reject request bodies larger than 1MB via Content-Length check
• Return 504 for upstream timeouts instead of generic 502

Closes #118

Test plan

• Send proxy request with %2e%2e in path → should get 400
• Send proxy request with body > 1MB → should get 413
• Send proxy request to unreachable host → should timeout with 504 after 30s
• Normal proxy requests still work

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
chronicle	Ready	Preview, Comment	Jun 3, 2026 8:04am

[coderabbitai]

Warning

Review limit reached

@rsbh, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 54 minutes. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 04676fce-5f3f-4154-86b1-b902dbcd9c50

📥 Commits

Reviewing files that changed from the base of the PR and between 71cb3b8 and e1a8382.

📒 Files selected for processing (1)

• packages/chronicle/src/server/api/apis-proxy.ts

📝 Walkthrough

Walkthrough

The API proxy handler in apis-proxy.ts is hardened against three attack vectors: oversized request bodies (HTTP 413), encoded path traversal (HTTP 400), and indefinite upstream hangs (HTTP 504). Constants define size and timeout limits; a strict isPathSafe validator decodes paths and rejects URL schemes and .. patterns; and the upstream fetch now includes a timeout signal with targeted error recovery.

Changes

Proxy request/response hardening

Layer / File(s)	Summary
Size and timeout configuration packages/chronicle/src/server/api/apis-proxy.ts	Module-level constants define maximum request body size (rejecting oversized POSTs with HTTP 413) and upstream fetch timeout duration.
Path safety validation and request size checks packages/chronicle/src/server/api/apis-proxy.ts	isPathSafe helper decodes the incoming path and rejects URL schemes plus .. traversal patterns; request handler validates content-length header and rejects oversized payloads; invalid paths trigger HTTP 400.
Upstream timeout and error handling packages/chronicle/src/server/api/apis-proxy.ts	Fetch request includes AbortSignal.timeout with configured duration; DOMException timeout errors are caught and mapped to HTTP 504.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and clearly summarizes the main hardening improvements: timeout, size limit, and encoded path check—all primary objectives of this changeset.
Description check	✅ Passed	The description is related to the changeset, providing specific implementation details (timeout value, size limit, path decoding) and a test plan aligned with the changes.
Linked Issues check	✅ Passed	The code implements all objectives from issue #118: decodes path before traversal check to catch encoded variants, adds 30s abort signal timeout to upstream fetch, rejects bodies >1MB via Content-Length validation, and returns 504 for upstream timeouts.
Out of Scope Changes check	✅ Passed	All changes are scoped to hardening the CORS proxy handler in apis-proxy.ts; no unrelated modifications detected outside the stated objectives.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/cors-proxy-hardening

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@packages/chronicle/src/server/api/apis-proxy.ts`:
- Around line 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.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 416145ef-a345-4ebd-ac5c-607291a2045a

📥 Commits

Reviewing files that changed from the base of the PR and between 86cbba0 and 71cb3b8.

📒 Files selected for processing (1)

• packages/chronicle/src/server/api/apis-proxy.ts

[coderabbitai]

⚠️ 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.