Skip to content

Conversation

@cfsmp3
Copy link
Collaborator

@cfsmp3 cfsmp3 commented Jan 19, 2026

Summary

  • Validate Origin header on state-changing requests (POST, PUT, DELETE)
  • Provide additional CSRF protection beyond SameSite cookies

Security Issue Addressed

No CSRF Protection on POST (Low) - Previously relied only on CORS headers for protection. This PR adds explicit Origin header validation on state-changing requests.

Changes

  • backend/controllers/app_handlers.go:
    • Add isOriginAllowed() function to validate request origins
    • Update EnableCORS() to reject POST/PUT/DELETE requests from disallowed origins
    • Allow localhost origins in development mode (ENV != "production")
    • Log rejected requests for security monitoring
    • Set dynamic CORS Access-Control-Allow-Origin based on request origin

CSRF Protection Layers

With this PR, the application now has multiple layers of CSRF protection:

  1. SameSite=Lax cookies (from PR fix: secure session cookies and validate WebSocket origin #405) - Prevents cross-site cookie sending
  2. Origin header validation (this PR) - Rejects requests from disallowed origins
  3. Session-based authentication - Requires valid session for protected endpoints

Test plan

  • Verify requests from allowed origin work correctly
  • Verify requests from disallowed origins are rejected (403 Forbidden)
  • Verify localhost works in development mode
  • Verify CORS preflight (OPTIONS) requests work correctly

🤖 Generated with Claude Code

Validate Origin header on state-changing requests (POST, PUT, DELETE)
to provide additional CSRF protection beyond SameSite cookies.

- Add isOriginAllowed() function to validate request origins
- Reject requests with invalid/disallowed Origin headers on POST/PUT/DELETE
- Allow localhost origins in development mode
- Log rejected requests for security monitoring
- Dynamic CORS header based on request origin

This complements SameSite=Lax cookies for comprehensive CSRF protection.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions
Copy link

Thank you for opening this PR!

Before a maintainer takes a look, it would be really helpful if you could walk through your changes using GitHub's review tools.

Please take a moment to:

  • Check the "Files changed" tab
  • Leave comments on any lines for functions, comments, etc. that are important, non-obvious, or may need attention
  • Clarify decisions you made or areas you might be unsure about and/or any future updates being considered.
  • Finally, submit all the comments!

More information on how to conduct a self review:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request

This helps make the review process smoother and gives us a clearer understanding of your thought process.

Once you've added your self-review, we'll continue from our side. Thank you!

Copy link
Collaborator

@its-me-abhishek its-me-abhishek left a comment

Choose a reason for hiding this comment

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

Can add w.Header().Add("Vary", "Origin") before the response, to prevent caching at all (if required) else looks okay to me.

Addresses review feedback to add the Vary header when responses
differ based on the Origin header. This prevents browsers and
CDNs from incorrectly caching CORS responses.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@its-me-abhishek its-me-abhishek merged commit 48e612f into main Jan 21, 2026
5 checks passed
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.

3 participants