Skip to content

Issue 17 add csrf protection to middleware#20

Merged
dankinder merged 3 commits intokatabole:mainfrom
jonyen:issue-17_add-csrf-protection-to-middleware
Mar 4, 2026
Merged

Issue 17 add csrf protection to middleware#20
dankinder merged 3 commits intokatabole:mainfrom
jonyen:issue-17_add-csrf-protection-to-middleware

Conversation

@jonyen
Copy link
Contributor

@jonyen jonyen commented Nov 13, 2025

We enable CSRF protection for prod environments (not needed for dev). Added some tests.

Updated go to 1.25.0 which supports CSRF protection.

Full disclosure: this code is all generated with AI. I have reviewed the code and have tested it manually to ensure that CSRF is functional.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds CSRF protection to the application by implementing cross-origin request validation in production environments. The protection is layered with CORS configuration to ensure secure handling of cross-origin requests while allowing legitimate same-origin operations.

Key changes:

  • Upgraded Go to version 1.25.0 to support CSRF protection features
  • Implemented environment-aware CSRF protection that is active in production but permissive in development
  • Added comprehensive test coverage for CSRF protection scenarios including origin validation, referer checking, and safe method handling

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.

File Description
go.mod Updated Go version to 1.25.0 and cleaned up unused dependencies
actions/csrf_test.go Added comprehensive test suite covering CSRF protection scenarios including origin/referer validation and safe methods
actions/app.go Integrated CSRF protection middleware with environment-aware configuration and updated CORS settings

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

req, err := http.NewRequest("POST", baseURL+"/users", body)
require.NoError(t, err)

req.Header.Set("Origin", conf.SiteURL)
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

Reference to undefined variable 'conf'. This should likely be 'fix.App.conf.SiteURL' to access the configuration from the test fixture.

Copilot uses AI. Check for mistakes.
req, err := http.NewRequest("POST", baseURL+"/users", body)
require.NoError(t, err)

req.Header.Set("Referer", conf.SiteURL+"/users/new")
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

Reference to undefined variable 'conf'. This should likely be 'fix.App.conf.SiteURL' to access the configuration from the test fixture.

Copilot uses AI. Check for mistakes.
go.mod Outdated
go 1.24.0

toolchain go1.24.1
go 1.25.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets do 1.25.4, the latest

Comment on lines +12 to +14
func TestCSRFProtection_BlocksUntrustedOrigins(t *testing.T) {
fix := NewFixture(t)
defer fix.Cleanup()
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets try to have it follow conventions used in our other tests. For example, the fixture is usually just called f. And for http requests use the helpers on f.Client if possible (I know for these tests since there are headers and such involved that may not be possible)

@dankinder
Copy link
Contributor

Hey @jonyen is this one good for more review?

@jonyen
Copy link
Contributor Author

jonyen commented Jan 31, 2026

Sorry for the delay. This is good for more review and I've recorded a video demonstrating that it works: https://app.airtimetools.com/recorder/s/z_S4j6PdjpfdsIIVTePBY0?t=0

@dankinder
Copy link
Contributor

Great thanks man, sorry for the delay

@dankinder dankinder merged commit 15e757b into katabole:main Mar 4, 2026
1 check 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