Make CORS origins configurable instead of hardcoded#282
Make CORS origins configurable instead of hardcoded#282prempalhade3-code wants to merge 5 commits intoAOSSIE-Org:mainfrom
Conversation
📝 WalkthroughWalkthroughThe pull request updates backend startup and configuration: adds config validation and CORS AllowedOrigins, changes config load error handling, conditionally initializes Redis, simplifies router setup by removing various route registrations and WebSocket signaling endpoints, adjusts .gitignore, and removes two no-op loops in tests. Changes
Sequence Diagram(s)sequenceDiagram
participant DevClient as Client
participant Server as Backend Server
participant Mongo as MongoDB
participant Redis as Redis (optional)
participant Debate as debate package
DevClient->>Server: HTTP request / ws connect
Server->>Server: LoadConfig -> validateConfig
alt config invalid
Server-->>DevClient: Fail startup (log.Fatalf)
else config valid
Server->>Mongo: Connect to DB
Mongo-->>Server: DB OK
alt cfg.Redis.Addr set
Server->>Redis: Init connection (debate.InitRedis)
Redis-->>Server: OK / warn on failure
Server->>Debate: Enable Redis-backed features
else no Redis
Server-->>Server: Log Redis unavailable
end
Server->>Server: Configure CORS origins
Server->>Server: Register simplified routes and start room watcher
Server-->>DevClient: Serve responses / websockets
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.gitignore:
- Around line 3-4: The .gitignore entry '*.env.DS_Store' only matches files
literally ending with '.env.DS_Store'; replace it with a proper macOS ignore
like '.DS_Store' or '**/.DS_Store' so typical .DS_Store files are ignored (edit
the .gitignore line containing '*.env.DS_Store' to '.DS_Store' or
'**/.DS_Store').
In `@backend/config/config.go`:
- Around line 100-118: Extend validateConfig to validate CORS origins: for
cfg.CORS.AllowedOrigins iterate each origin (trim whitespace) and return an
error if any origin is empty, equals "*" while cfg.CORS.AllowCredentials is
true, or does not start with "http://" or "https://"; ensure you reference the
cfg.CORS.AllowedOrigins slice and cfg.CORS.AllowCredentials flag in the checks
so invalid configurations are rejected.
| *.env.DS_Store | ||
| *.zip |
There was a problem hiding this comment.
Potential typo in the .DS_Store ignore pattern.
*.env.DS_Store only matches filenames ending with .env.DS_Store. If the goal is to ignore macOS .DS_Store files, consider using .DS_Store (optionally with **/.DS_Store).
🔧 Proposed fix
-*.env.DS_Store
+.DS_Store📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| *.env.DS_Store | |
| *.zip | |
| .DS_Store | |
| *.zip |
🤖 Prompt for AI Agents
In @.gitignore around lines 3 - 4, The .gitignore entry '*.env.DS_Store' only
matches files literally ending with '.env.DS_Store'; replace it with a proper
macOS ignore like '.DS_Store' or '**/.DS_Store' so typical .DS_Store files are
ignored (edit the .gitignore line containing '*.env.DS_Store' to '.DS_Store' or
'**/.DS_Store').
| func validateConfig(cfg *Config) error { | ||
|
|
||
| if cfg.Server.Port == 0 { | ||
| return fmt.Errorf("server.port is required") | ||
| } | ||
| if cfg.Database.URI == "" { | ||
| return fmt.Errorf("database.uri is required") | ||
| } | ||
| if cfg.JWT.Secret == "" { | ||
| return fmt.Errorf("jwt.secret is required") | ||
| } | ||
| if cfg.Gemini.ApiKey == "" { | ||
| return fmt.Errorf("gemini.apiKey is required") | ||
| } | ||
| if cfg.GoogleOAuth.ClientID == "" { | ||
| return fmt.Errorf("googleOAuth.clientID is required") | ||
| } | ||
|
|
||
| return nil |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "config.go" -o -name "main.go" -o -name "router.go" | head -20Repository: AOSSIE-Org/DebateAI
Length of output: 182
🏁 Script executed:
git ls-files | grep -E "(config|router)" | grep "\.go$"Repository: AOSSIE-Org/DebateAI
Length of output: 87
🏁 Script executed:
cat -n backend/config/config.goRepository: AOSSIE-Org/DebateAI
Length of output: 3525
🏁 Script executed:
find . -type f -name "*.go" | xargs grep -l "gin-contrib/cors\|AllowCredentials" | head -10Repository: AOSSIE-Org/DebateAI
Length of output: 91
🏁 Script executed:
cat -n backend/cmd/server/main.goRepository: AOSSIE-Org/DebateAI
Length of output: 5875
🏁 Script executed:
grep -r "AllowCredentials" backend/ --include="*.go" -B 2 -A 2Repository: AOSSIE-Org/DebateAI
Length of output: 415
🌐 Web query:
CORS specification wildcard origin AllowCredentials true invalid
💡 Result:
Per the CORS/FETCH rules, you cannot combine:
Access-Control-Allow-Origin: *(wildcard) andAccess-Control-Allow-Credentials: true(allow cookies/HTTP auth/client certs)
If you do, browsers will treat the response as invalid for credentialed CORS and block access (you’ll typically see an error like “Credential is not supported if the CORS header ‘Access-Control-Allow-Origin’ is ‘*’”). [1], [2]
What to do instead
If you need credentials (cookies/auth):
- Return a single, explicit origin (often by echoing the request’s
Originif it’s in your allowlist), e.g.Access-Control-Allow-Origin: https://app.example - Keep
Access-Control-Allow-Credentials: true - Add
Vary: Originwhen you dynamically reflect origins to avoid cache issues. [2], [3]
If you want to allow any site (*):
- Do not send
Access-Control-Allow-Credentials: true, and ensure the client does not usecredentials: "include"/withCredentials=true. [1], [2]
Sources:
[1] MDN: “Credential is not supported if the CORS header ‘Access-Control-Allow-Origin’ is ‘*’” https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/CORS/Errors/CORSNotSupportingCredentials
[2] MDN: Access-Control-Allow-Origin (wildcard + credentials errors) https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin
[3] MDN: handling multiple origins + Vary: Origin guidance https://developer.mozilla.org/docs/Web/HTTP/Guides/CORS/Errors/CORSMissingAllowOrigin
🌐 Web query:
gin-contrib/cors AllowCredentials true wildcard "*" behavior compliance
💡 Result:
In CORS, you cannot use Access-Control-Allow-Origin: * together with Access-Control-Allow-Credentials: true. For credentialed requests (cookies / HTTP auth / client certs), the server must return an explicit origin (and typically Vary: Origin); if it returns *, browsers will block access to the response and Set-Cookie won’t work. (developer.mozilla.org)
What gin-contrib/cors does / expects
gin-contrib/cors documents this as a restriction:
AllowAllOrigins(effectively “*”) means credentials cannot be used / “allowing all origins disables cookies.” (github.com)AllowCredentialsis intended only when you use precise origins (not all origins / wildcard). (github.com)
Compliance-safe configuration
To be compliant while allowing credentials:
- Do not use
AllowAllOrigins = trueorAllowOrigins = []string{"*"}. - Instead, whitelist explicit origins (or implement
AllowOriginFunc) and keepAllowCredentials = true. (github.com)
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/CORS?utm_source=openai
- 2: https://github.com/gin-contrib/cors?utm_source=openai
- 3: https://github.com/gin-contrib/cors?utm_source=openai
- 4: https://github.com/gin-contrib/cors?utm_source=openai
Add CORS origin validation to prevent invalid configurations.
The router is configured with AllowCredentials: true (line 88, backend/cmd/server/main.go), but the config validator does not check cors.allowedOrigins. Per the CORS specification, combining a wildcard origin ("*") with credential support is invalid and will cause browsers to reject credentialed requests. Add validation to reject:
- Wildcard
"*"whenAllowCredentialsis true - Empty or whitespace-only origins
- Origins without a scheme (
http://orhttps://)
Proposed validation additions
import (
"fmt"
"os"
+ "strings"
"gopkg.in/yaml.v3"
) func validateConfig(cfg *Config) error {
if cfg.Server.Port == 0 {
return fmt.Errorf("server.port is required")
}
if cfg.Database.URI == "" {
return fmt.Errorf("database.uri is required")
}
if cfg.JWT.Secret == "" {
return fmt.Errorf("jwt.secret is required")
}
if cfg.Gemini.ApiKey == "" {
return fmt.Errorf("gemini.apiKey is required")
}
if cfg.GoogleOAuth.ClientID == "" {
return fmt.Errorf("googleOAuth.clientID is required")
}
+ for i, origin := range cfg.CORS.AllowedOrigins {
+ if strings.TrimSpace(origin) == "" {
+ return fmt.Errorf("cors.allowedOrigins[%d] is empty", i)
+ }
+ if origin == "*" {
+ return fmt.Errorf("cors.allowedOrigins cannot include '*' when allowCredentials is true")
+ }
+ if !strings.HasPrefix(origin, "http://") && !strings.HasPrefix(origin, "https://") {
+ return fmt.Errorf("cors.allowedOrigins[%d] must include scheme", i)
+ }
+ }
return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func validateConfig(cfg *Config) error { | |
| if cfg.Server.Port == 0 { | |
| return fmt.Errorf("server.port is required") | |
| } | |
| if cfg.Database.URI == "" { | |
| return fmt.Errorf("database.uri is required") | |
| } | |
| if cfg.JWT.Secret == "" { | |
| return fmt.Errorf("jwt.secret is required") | |
| } | |
| if cfg.Gemini.ApiKey == "" { | |
| return fmt.Errorf("gemini.apiKey is required") | |
| } | |
| if cfg.GoogleOAuth.ClientID == "" { | |
| return fmt.Errorf("googleOAuth.clientID is required") | |
| } | |
| return nil | |
| func validateConfig(cfg *Config) error { | |
| if cfg.Server.Port == 0 { | |
| return fmt.Errorf("server.port is required") | |
| } | |
| if cfg.Database.URI == "" { | |
| return fmt.Errorf("database.uri is required") | |
| } | |
| if cfg.JWT.Secret == "" { | |
| return fmt.Errorf("jwt.secret is required") | |
| } | |
| if cfg.Gemini.ApiKey == "" { | |
| return fmt.Errorf("gemini.apiKey is required") | |
| } | |
| if cfg.GoogleOAuth.ClientID == "" { | |
| return fmt.Errorf("googleOAuth.clientID is required") | |
| } | |
| for i, origin := range cfg.CORS.AllowedOrigins { | |
| if strings.TrimSpace(origin) == "" { | |
| return fmt.Errorf("cors.allowedOrigins[%d] is empty", i) | |
| } | |
| if origin == "*" { | |
| return fmt.Errorf("cors.allowedOrigins cannot include '*' when allowCredentials is true") | |
| } | |
| if !strings.HasPrefix(origin, "http://") && !strings.HasPrefix(origin, "https://") { | |
| return fmt.Errorf("cors.allowedOrigins[%d] must include scheme", i) | |
| } | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In `@backend/config/config.go` around lines 100 - 118, Extend validateConfig to
validate CORS origins: for cfg.CORS.AllowedOrigins iterate each origin (trim
whitespace) and return an error if any origin is empty, equals "*" while
cfg.CORS.AllowCredentials is true, or does not start with "http://" or
"https://"; ensure you reference the cfg.CORS.AllowedOrigins slice and
cfg.CORS.AllowCredentials flag in the checks so invalid configurations are
rejected.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/config/config.go`:
- Around line 118-127: The loop over cfg.CORS.AllowedOrigins currently trims
origin for the empty-value check but uses the untrimmed origin for the wildcard
and scheme checks; update the code to store the trimmed value (e.g.,
trimmedOrigin := strings.TrimSpace(origin)) and use trimmedOrigin for all
subsequent checks, and change the wildcard error text from "cors.allowedOrigins
cannot contain '*' when credentials are enabled" to remove the conditional
wording (e.g., "cors.allowedOrigins cannot contain '*'") since credentials are
always enabled.
| for _, origin := range cfg.CORS.AllowedOrigins { | ||
| if strings.TrimSpace(origin) == "" { | ||
| return fmt.Errorf("cors.allowedOrigins cannot contain empty values") | ||
| } | ||
| if origin == "*" { | ||
| return fmt.Errorf("cors.allowedOrigins cannot contain '*' when credentials are enabled") | ||
| } | ||
| if !strings.HasPrefix(origin, "http://") && !strings.HasPrefix(origin, "https://") { | ||
| return fmt.Errorf("cors.allowedOrigins must include scheme") | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for CORS-related configuration and AllowCredentials
rg -n "AllowCredentials|cors\\.Config|AllowedOrigins|CORSConfig|CORS" backend --type go -A 3 -B 3Repository: AOSSIE-Org/DebateAI
Length of output: 1933
Remove conditional language from wildcard validation error; align trimmed origin usage across all checks.
Credentials are hardcoded to AllowCredentials: true in the CORS middleware (backend/cmd/server/main.go:88), making the error message "when credentials are enabled" misleading—the condition is always true at runtime. Additionally, use the trimmed origin consistently for the scheme check to avoid false positives on whitespace-padded values.
🛠️ Suggested fix
for _, origin := range cfg.CORS.AllowedOrigins {
+ trimmed := strings.TrimSpace(origin)
- if strings.TrimSpace(origin) == "" {
+ if trimmed == "" {
return fmt.Errorf("cors.allowedOrigins cannot contain empty values")
}
- if origin == "*" {
- return fmt.Errorf("cors.allowedOrigins cannot contain '*' when credentials are enabled")
+ if trimmed == "*" {
+ return fmt.Errorf("cors.allowedOrigins cannot contain '*'")
}
- if !strings.HasPrefix(origin, "http://") && !strings.HasPrefix(origin, "https://") {
+ if !strings.HasPrefix(trimmed, "http://") && !strings.HasPrefix(trimmed, "https://") {
return fmt.Errorf("cors.allowedOrigins must include scheme")
}
}🤖 Prompt for AI Agents
In `@backend/config/config.go` around lines 118 - 127, The loop over
cfg.CORS.AllowedOrigins currently trims origin for the empty-value check but
uses the untrimmed origin for the wildcard and scheme checks; update the code to
store the trimmed value (e.g., trimmedOrigin := strings.TrimSpace(origin)) and
use trimmedOrigin for all subsequent checks, and change the wildcard error text
from "cors.allowedOrigins cannot contain '*' when credentials are enabled" to
remove the conditional wording (e.g., "cors.allowedOrigins cannot contain '*'")
since credentials are always enabled.
1 participant
This PR removes the hardcoded frontend origin and makes CORS origins
configurable via backend configuration.
Existing local development behavior is preserved by falling back to the
previous default when no configuration is provided.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.