Skip to content

Add realtime compiler dashboard CSRF protection#1454

Merged
emmadesilva merged 17 commits intomasterfrom
realtime-compiler-security
Nov 13, 2023
Merged

Add realtime compiler dashboard CSRF protection#1454
emmadesilva merged 17 commits intomasterfrom
realtime-compiler-security

Conversation

@emmadesilva
Copy link
Member

@emmadesilva emmadesilva commented Nov 12, 2023

While we make it clear that the realtime compiler is only intended to be used for local development (as it uses the built in PHP server, which is not designed for public use, as per the manual), since it does allow access to the file system I felt it responsible to at least add some CSRF protection to the dashboard forms. Also extracts a base class while I'm at it.

I ran benchmarks, and adding the session has a negligible performance impact of about 2.5 seconds for 10 000 requests, and that's only for the dashboard page.

@codecov
Copy link

codecov bot commented Nov 12, 2023

Codecov Report

Merging #1454 (abca991) into master (ddd0c75) will not change coverage.
The diff coverage is n/a.

@@              Coverage Diff              @@
##              master     #1454     +/-   ##
=============================================
  Coverage     100.00%   100.00%             
- Complexity      1728      3456   +1728     
=============================================
  Files            180       360    +180     
  Lines           4693      9386   +4693     
=============================================
+ Hits            4693      9386   +4693     

see 180 files with indirect coverage changes

@emmadesilva emmadesilva changed the title Realtime compiler security Add realtime compiler dashboard CSRF protection Nov 12, 2023
@emmadesilva emmadesilva force-pushed the realtime-compiler-security branch from e8b7ffd to 3f3cc54 Compare November 12, 2023 20:18
@emmadesilva emmadesilva force-pushed the realtime-compiler-security branch from 400bb2d to f06ff3a Compare November 13, 2023 10:25
This reverts commit 8ee7f57af9d94f9feb00ed4993174954081fc623.
These expire with the session anyways, and expiring them manually means only one dashboard action works per page reload which is not helpful.
We probably don't need this
Makes it more readable but has same semantics
@emmadesilva emmadesilva marked this pull request as ready for review November 13, 2023 14:13
@emmadesilva emmadesilva merged commit 7c58d74 into master Nov 13, 2023
@emmadesilva emmadesilva deleted the realtime-compiler-security branch November 13, 2023 14:14
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.

2 participants