fix(security): remove CORS wildcard from local HTTP API (SEC-001)#70
fix(security): remove CORS wildcard from local HTTP API (SEC-001)#70abyssbugg wants to merge 1 commit into
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRemoves the CORS wildcard and all related headers from the local HTTP API response builders and updates tests to assert their absence while preserving existing status codes and JSON behavior. Sequence diagram for local HTTP API responses without CORS headerssequenceDiagram
actor Website
participant Browser
participant LocalHttpApi
Website->>Browser: fetch(http://127.0.0.1:6736/v1/usage)
Browser->>LocalHttpApi: HTTP GET /v1/usage
LocalHttpApi->>LocalHttpApi: response_json(200, OK, body)
LocalHttpApi-->>Browser: 200 OK without Access-Control-Allow-Origin
Browser-->>Website: [CORS blocked by browser]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The test name
route_options_returns_204_with_corsno longer matches its behavior after the CORS removal; consider renaming it to reflect that it's just validating the 204 response. - Given that CORS is fully removed, revisit whether the explicit
OPTIONShandling is still needed for the local API or if it can be simplified to reduce surface area.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The test name `route_options_returns_204_with_cors` no longer matches its behavior after the CORS removal; consider renaming it to reflect that it's just validating the 204 response.
- Given that CORS is fully removed, revisit whether the explicit `OPTIONS` handling is still needed for the local API or if it can be simplified to reduce surface area.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary
Fixes #62 — removes the
Access-Control-Allow-Origin: *wildcard from the local HTTP API.Finding
SEC-001 (CWE-942, MEDIUM): Any website visited by the user could
fetch("http://127.0.0.1:6736/v1/usage")and read provider IDs, plan names, and usage metrics. The local HTTP API is for local apps, not browsers — CORS headers served no legitimate purpose.Fix
Removed
CORS_HEADERSconstant and all CORS header injection from response builders. The API now returns same-origin-only responses. No browser can make cross-origin requests to the local API.Tests Updated
route_options_returns_204_with_cors→ removed CORS assertion (renamed conceptually, kept 204 check)route_options_on_provider_returns_204→ removed CORS methods assertionresponse_json_includes_cors_headers→ renamed toresponse_json_does_not_include_cors_headersand inverted assertionresponse_service_unavailable_returns_503_json→ removed CORS assertionValidation
cargo test✅ 164 passed, 0 failed (all Rust tests green)bun run build— timed out locally (environmental); CI will validatebun run test— pre-existing jsdom/Node.js incompatibility (environmental, not caused by this change)Regression Risk
Low — only affects browser-based cross-origin access. No internal app consumer uses CORS. The local HTTP API is consumed by the app itself (same-origin) and local CLI tools (no CORS needed).
Scope
1 file changed, 5 insertions, 16 deletions. No new tokens, no behavior changes beyond CORS removal.
Summary by cubic
Remove the CORS wildcard from the local HTTP API to block cross-origin access from arbitrary websites. This addresses SEC-001 by preventing pages from reading provider IDs, plan names, and usage metrics via localhost.
CORS_HEADERSand stopped injecting CORS headers in JSON and 204 responses.Written for commit 474f347. Summary will update on new commits.
Note
Remove CORS wildcard headers from local HTTP API responses
Removes the
CORS_HEADERSconstant from server.rs and stops injectingAccess-Control-Allow-*headers intoresponse_jsonandresponse_no_content(204) responses. Tests are updated to assert that CORS headers are absent rather than present. Risk: any client relying on wildcard CORS headers from this API will no longer receive them.Macroscope summarized 474f347.
Summary by Sourcery
Remove CORS headers from the local HTTP API responses to eliminate wildcard cross-origin access and tighten security.
Bug Fixes:
Enhancements:
Tests: