[HYPER-11] fix: WebSocket origin checker rejects cross-origin connections when ALLOWED_ORIGINS is unset#30
Merged
Conversation
Without this patch, the WebSocket origin checker only allowed all origins when ALLOWED_ORIGINS was explicitly set to "*". If ALLOWED_ORIGINS was unset or empty, the checker would fall through to the restrictive logic that rejects origins not in the (empty) allowlist. This is a problem because it's inconsistent with the CORS middleware behavior, which allows all origins when ALLOWED_ORIGINS is not configured. Users expect WebSocket connections to work out of the box in development. This patch solves the problem by treating empty/nil ALLOWED_ORIGINS the same as "*" - allowing all origins. The logging now distinguishes between "not configured" vs explicitly set to "*" for better observability. Changes: - Handle empty/nil ALLOWED_ORIGINS as permissive (allow all) - Add distinct log messages for unconfigured vs wildcard cases - Add comprehensive test coverage with 9 test cases Co-authored-by: Claude Code <claude-code@noreply.anthropic.com>
11 config vars from config.go were undocumented: ALLOWED_ORIGINS, TRUST_PROXY_HEADERS, DOMAIN_DID, LEXICON_DIR, JETSTREAM_URL, JETSTREAM_COLLECTIONS, BACKFILL_ON_START, BACKFILL_COLLECTIONS, BACKFILL_PDS_CONCURRENCY, BACKFILL_REPO_TIMEOUT, PLC_DIRECTORY_URL
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
🚅 Environment hyperindex-pr-30 in hypercerts has no services deployed. |
aspiers
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ALLOWED_ORIGINSis not configured, the CORS middleware allowed all origins but the WebSocketCheckOriginrejected all cross-origin connections. Now both default to allow-all, matching pre-security-fix behavior. Closes [Security] WebSocket CheckOrigin accepts all origins (CSWSH risk) #3..env.example:ALLOWED_ORIGINS,TRUST_PROXY_HEADERS,DOMAIN_DID,LEXICON_DIR,JETSTREAM_URL,JETSTREAM_COLLECTIONS,BACKFILL_ON_START,BACKFILL_COLLECTIONS,BACKFILL_PDS_CONCURRENCY,BACKFILL_REPO_TIMEOUT,PLC_DIRECTORY_URL.Root Cause
PR #19 (security fixes) tightened
makeOriginCheckerinsubscription/handler.goto iterate over configured origins. WhenALLOWED_ORIGINSis empty (the default), the list isnil, so the loop matches nothing and every cross-origin WebSocket upgrade gets 403 Forbidden.The CORS middleware has the opposite default — empty origins means allow all (
Access-Control-Allow-Origin: *). This inconsistency meant HTTP GraphQL queries worked from any origin but WebSocket subscriptions were silently blocked.Testing
handler_test.gowith table-driven tests covering: nil origins, empty origins, wildcard, matching, non-matching, and multi-origin scenarios.hyperindex.test.certified.app:Origin: https://impactindexer.orgreturned 403 before the fix.