Skip to content

fix(cors): split public and admin GraphQL origins#71

Open
Kzoeps wants to merge 1 commit into
mainfrom
fix/split-graphql-admin-cors
Open

fix(cors): split public and admin GraphQL origins#71
Kzoeps wants to merge 1 commit into
mainfrom
fix/split-graphql-admin-cors

Conversation

@Kzoeps

@Kzoeps Kzoeps commented Jun 3, 2026

Copy link
Copy Markdown
Member

Summary

  • allow public GraphQL HTTP and WebSocket APIs from all browser origins by default
  • restrict admin GraphQL CORS to explicit ADMIN_ALLOWED_ORIGINS values
  • deprecate ALLOWED_ORIGINS as a fallback and document PUBLIC_ALLOWED_ORIGINS / ADMIN_ALLOWED_ORIGINS
  • include admin proxy headers in admin CORS preflight responses

Verification

  • DATABASE_URL=sqlite::memory: go test -race ./...
  • DATABASE_URL=sqlite::memory: go test ./...
  • go build -v ./...
  • make lint
  • local browser stack with agent-browser:
    • public /graphql works from allowed and disallowed browser origins
    • admin /admin/graphql works from http://127.0.0.1:13000
    • admin /admin/graphql is browser-blocked from http://127.0.0.1:13001

Screenshots

Local verification screenshots were generated at:

  • /tmp/hyperindex-cors-test/screens/allowed-client-origin.png
  • /tmp/hyperindex-cors-test/screens/disallowed-admin-origin.png

Summary by CodeRabbit

  • New Features

    • Separated CORS handling for public and admin GraphQL endpoints with independent origin configurations.
  • Configuration Changes

    • New environment variables: PUBLIC_ALLOWED_ORIGINS (defaults to *) and ADMIN_ALLOWED_ORIGINS (requires explicit origins).
    • ALLOWED_ORIGINS deprecated but supported as fallback for admin origins.
  • Documentation

    • Updated configuration guidance for new CORS environment variables.

@vercel

vercel Bot commented Jun 3, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
hyperindex-atproto-client Ready Ready Preview, Comment Jun 3, 2026 9:39am
hyperindex-client Ready Ready Preview, Comment Jun 3, 2026 9:39am

Request Review

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

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

More reviews will be available in 57 minutes and 55 seconds. 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: 5b755b39-4d4f-4202-a2df-c25c61517062

📥 Commits

Reviewing files that changed from the base of the PR and between debd085 and 786e390.

📒 Files selected for processing (15)
  • .changes/unreleased/split-graphql-admin-cors.yaml
  • .env.example
  • AGENTS.md
  • README.md
  • cmd/hyperindex/main.go
  • cmd/hyperindex/main_test.go
  • internal/config/config.go
  • internal/config/config_test.go
  • internal/graphql/admin/handler.go
  • internal/graphql/handler.go
  • internal/graphql/handler_test.go
  • internal/graphql/subscription/handler.go
  • internal/graphql/subscription/handler_test.go
  • internal/server/cors.go
  • internal/server/cors_test.go
📝 Walkthrough

Walkthrough

This PR splits CORS origin handling for GraphQL endpoints into two separate allowlists. Public GraphQL endpoints (/graphql, /graphql/ws) default to allowing all origins; admin endpoints (/admin/graphql) require explicitly configured trusted origins and reject wildcard settings. CORS enforcement moves from router-level to per-endpoint middleware.

Changes

CORS Origin Split for GraphQL

Layer / File(s) Summary
Configuration Schema & Origin Parsing
internal/config/config.go, internal/config/config_test.go, .env.example, README.md
Config gains PublicAllowedOrigins and AdminAllowedOrigins fields with env-var loading, validation rejects admin wildcard, and helper methods PublicAllowedOriginList(), AdminAllowedOriginList() provide parsed origin lists with fallback to deprecated ALLOWED_ORIGINS.
CORS Middleware Refactoring
internal/server/cors.go, internal/server/cors_test.go
CORSMiddleware is refactored to normalize origins, compute allowed headers with deduplication, add admin API key header support, and centralize Vary: Origin handling; setting Access-Control-Allow-Origin is gated on both non-empty request origin and membership in the allowed set.
WebSocket Subscription Origin Validation
internal/graphql/subscription/handler.go, internal/graphql/subscription/handler_test.go
WebSocket handler gains helpers to normalize allowed origins and detect same-origin requests; makeOriginChecker now allows missing or same-origin requests, checks explicit allowlist membership, and rejects mismatched cross-origin requests.
Per-Endpoint CORS Middleware Integration
cmd/hyperindex/main.go, cmd/hyperindex/main_test.go
Router setup removes global CORS middleware and adds per-endpoint wrappers: public /graphql routes use PublicAllowedOriginList(), admin /admin/graphql routes use AdminAllowedOriginList() with admin API key support; integration tests verify restricted admin origins and permissive public origins.
Documentation & CORS Comment Updates
internal/graphql/admin/handler.go, internal/graphql/handler.go, internal/graphql/handler_test.go
Environment example and README document the new PUBLIC_ALLOWED_ORIGINS and ADMIN_ALLOWED_ORIGINS variables and mark ALLOWED_ORIGINS as deprecated; code comments are updated to reference route-level rather than router-level CORS middleware.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • GainForest/hyperindex#24: Both PRs touch the same CORS wiring for admin GraphQL—internal/server/cors.go and its tests/middleware config around AllowAdminAPIKeyAuth/X-Admin-API-Key (while the main PR additionally splits public vs admin origin allowlists).

Poem

🐰 A CORS tale in two parts unfolds,
Public paths where all are bold,
Admin gates with origins known,
No wildcards near the throne!
Routes divide what once was whole,
Middleware guards each endpoint's role. 🔐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.32% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(cors): split public and admin GraphQL origins' accurately and concisely describes the main change: separating CORS origin handling for public and admin GraphQL endpoints.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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/split-graphql-admin-cors

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

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: debd085ace

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread cmd/hyperindex/main.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant