fix: validate WebSocket Origin header against allow-list

[HimanshuPathak2725]

Type of change

• Bug fix
• New feature
• Refactor / cleanup
• Docs / CI

Checklist

• go build ./... passes locally
• go vet ./... passes locally
• I've tested the change manually
• README update — not needed, internal server security boundary,
  no user-facing/API change.

[HimanshuPathak2725]

Thanks for the review! Addressed the two genuinely useful points (case-insensitive hostname comparison, logging on parse failure) in the latest commit. On the rest:

• Scheme/port validation: the Origin header is set by the browser itself and can't be forged via JS (fetch/XHR treat it as a forbidden header), so there's no realistic bypass path here to close, adding scheme/port checks wouldn't stop an attacker who doesn't control the Origin header in the first place.
• Exporting IsAllowedOrigin: kept it unexported intentionally it's only used within package main, no other package needs it.
• Caching parsed URLs: /ws upgrades are already rate-limited to 10/min per IP ([Security] Implement IP Rate Limiting on WebSocket Relay #6/feat: add per-IP rate limiting on WebSocket relay #9), so url.Parse() isn't in a genuinely hot path at this scale.
• A couple of the other points (missing doc comment, missing empty-Origin handling) don't apply to the code as written both were already there before this review ran.

Let me know if you'd like any of the deferred ones addressed anyway.

[Copilot]

Pull request overview

Tightens the server’s WebSocket security boundary by validating the Origin header against an allow-list, replacing the previous unconditional CheckOrigin acceptance.

Changes:

• Add isAllowedOrigin and wire it into the WebSocket upgrader CheckOrigin callback.
• Add unit tests covering allow-listed and disallowed Origin values.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
cmd/server/main.go	Introduces Origin allow-list validation and applies it to WebSocket upgrade checks.
cmd/server/main_test.go	Adds test coverage for the new Origin allow-list logic.

Comments suppressed due to low confidence (1)

cmd/server/main.go:123

• guestURL uses strings.Contains(reqHost, "render.com"), which is broader than intended and doesn’t match the onrender.com allow-list used by isAllowedOrigin. It can also treat hosts like evilrender.com as “trusted” and switch to https. Consider normalizing the host (strip port, lower-case) and using exact/suffix matching for onrender.com (and the other allowed domains) instead of substring matching.

func guestURL(code string, reqHost string) string {
	scheme := "http"
	if strings.Contains(reqHost, "render.com") || strings.Contains(reqHost, "railway.app") || strings.Contains(reqHost, "ngrok") || strings.Contains(reqHost, "fly.dev") {
		scheme = "https"
	} else if strings.Contains(reqHost, "localhost") {

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[VishalRaut2106]

Hey @HimanshuPathak2725 , great work on this! The code and tests look solid.

One quick request: I recently set up a custom domain for the project (gatekeeper.vishalraut.me). Could you please add "vishalraut.me" to the allowed domains list in isAllowedOrigin() and the tests? Once that's added, I'll merge this right away!

[HimanshuPathak2725]

Thanks! Added support for the new vishalraut.me domain (including gatekeeper.vishalraut.me via the existing subdomain matching logic) and expanded the tests accordingly.

Ran:

• go test ./...
• go test -race ./...

Both pass successfully.

[github-actions]

Feedback on Code Changes

Bugs and Security Issues

1. Security Issue: Incomplete Origin Validation

   • The isAllowedOrigin function now validates the scheme and port, addressing the initial concern. However, ensure that the validation logic is consistent and handles all edge cases, such as URLs with non-standard ports or schemes. The current implementation only checks for standard ports (80 and 443), which might not cover all valid cases.
   • Action: Add validation for non-standard ports and ensure the logic is robust.

2. Security Issue: Missing Port Validation

   • The function now checks for standard ports (80 and 443) for the specified domains. Ensure that this logic is robust and handles all valid cases, including non-standard ports.
   • Action: Extend port validation to cover non-standard ports.

3. Security Issue: Case Sensitivity in Domain Validation

   • The function converts domains to lowercase before validation, ensuring case-insensitive comparison. This addresses the initial concern.
   • Action: Ensure all domain comparisons are case-insensitive.

Performance Bottlenecks

1. Performance Bottleneck: URL Parsing in Hot Path
   • The function parses the URL for every WebSocket upgrade request. While this is acceptable for most use cases, consider caching parsed URLs or using a more efficient method if performance becomes an issue under high load.
   • Action: Evaluate the need for URL parsing in the hot path and consider caching or optimizing the validation method.

Idiomatic Go Conventions

1. Idiomatic Go Convention: Error Handling

   • The function now logs URL parsing errors, providing better visibility into potential issues. This addresses the initial concern.
   • Action: Ensure all error handling is robust and provides sufficient visibility.

2. Idiomatic Go Convention: Variable Naming

   • The variable u has been renamed to parsedURL, improving readability and maintainability.
   • Action: Ensure all variable names are descriptive and follow Go conventions.

3. Idiomatic Go Convention: Function Naming

   • The function isAllowedOrigin has been kept unexported, which is fine for internal use.
   • Action: Ensure function names are descriptive and follow Go conventions.

Robust Error Handling

1. Robust Error Handling: URL Parsing

   • The function now logs URL parsing errors, providing better visibility into potential issues. This addresses the initial concern.
   • Action: Ensure all URL parsing errors are logged and handled appropriately.

2. Robust Error Handling: Origin Header

   • The function handles cases where the Origin header is missing or malformed by returning false. This ensures robustness.
   • Action: Ensure all edge cases for the Origin header are handled appropriately.

Additional Improvements

1. Additional Improvement: Test Coverage

   • The test cases for IsAllowedOrigin now cover more edge cases, including different schemes, ports, and malformed URLs. This ensures comprehensive coverage.
   • Action: Ensure all edge cases are covered in tests.

2. Additional Improvement: Documentation

   • The function IsAllowedOrigin now includes a comment explaining its purpose, parameters, and return values. This improves code readability and maintainability.
   • Action: Ensure all functions are well-documented.

Summary of Actions

1. Security:

   • Validate the scheme and port in the origin URL.
   • Convert domains to lowercase for case-insensitive comparison.
   • Handle URL parsing errors appropriately.

2. Performance:

   • Consider caching parsed URLs or using a more efficient validation method if performance becomes an issue.

3. Idiomatic Go:

   • Rename variables and functions to follow Go conventions.
   • Handle errors and edge cases robustly.

4. Testing:

   • Expand test cases to cover more edge cases.

5. Documentation:

   • Add comments to the IsAllowedOrigin function.

By addressing these points, the code will be more secure, performant, idiomatic, and robust.

[VishalRaut2106]

Hey @HimanshuPathak2725 , great work

[VishalRaut2106]

Hey Himanshu, thanks for wrapping up Finding #3!

The Ping/Pong heartbeat implementation is super clean, and putting a hard 32KB read limit on the WebSockets completely shuts down the OOM/DoS vector. Really appreciate the rigorous unit tests as well.

Merging this in now—thanks again for the massive security improvements across the board!

[HimanshuPathak2725]

@VishalRaut2106 Labels ig.....!!