Combine turnstile and rack-attack#410
Open
JPrevost wants to merge 14 commits into
Open
Conversation
Coverage Report for CI Build 28115565855Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.003%) to 98.356%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions1 previously-covered line in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
❌ 2 blocking issues (2 total)
|
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds Rack::Attack-based protections for the expensive /results and /record endpoints and ties throttling into the existing Turnstile flow so legitimate users can recover from throttling via challenge verification.
Changes:
- Introduces a global RPS throttle plus a stricter per-IP throttle for
/resultsand/record. - Overrides Rack::Attack throttled responses on those endpoints to redirect to the Turnstile challenge instead of returning 429.
- After successful Turnstile verification, writes a short-lived “verified” cache key intended to suppress re-challenges for that IP.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| README.md | Documents new ENV knobs for the results/record throttles and Turnstile grace period. |
| config/initializers/rack_attack.rb | Adds new throttles and custom throttled response logic for /results and /record. |
| app/controllers/turnstile_controller.rb | Writes a cache-based grace-period marker after successful Turnstile verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jazairi
requested changes
Jun 22, 2026
jazairi
left a comment
jazairi
left a comment
Contributor
There was a problem hiding this comment.
I think all of Copilot's suggested feedback is worth implementing, particularly the fix for the potential feedback loop. Once that is in place, this is ready to merge from my perspective.
I was pondering adding unit tests for the grace period and throttle_response logic, but I couldn't figure a good way to do so that wouldn't just be testing implementation details. In this case, I think manual testing is sufficient. Anything more would likely have to be a system test, which would be new to us and well out of scope of this urgent change.
Also fixes up docs and adds tests
Also more tests and docs fixups
Comment on lines
+15
to
+17
| cache_key = "turnstile_verified:#{request.ip}" | ||
| grace_period = ENV.fetch('TURNSTILE_GRACE_PERIOD') { 15 }.to_i.minutes | ||
| Rails.cache.write(cache_key, true, expires_in: grace_period) |
Comment on lines
137
to
138
| - `REQUESTS_PER_PERIOD` - number of requests that can be made for general throttles per `REQUEST_PERIOD` | ||
| - `REQUEST_PERIOD` - time in minutes used along with `REQUESTS_PER_PERIOD` |
| - `RESULTS_THROTTLE_PERIOD` - time in minutes for `/results` and `/record` endpoint throttle (default 1 minute). Throttled requests are redirected to Turnstile for verification. | ||
| - `TURNSTILE_GRACE_PERIOD` - time in minutes that an IP is whitelisted from throttling after successfully passing Turnstile verification (default 15 minutes). This prevents users from being re-challenged repeatedly. | ||
| - `REDIRECT_REQUESTS_PER_PERIOD`- number of requests that can be made that the query string starts with our legacy redirect parameter to throttle per `REQUEST_PERIOD` | ||
| - `REDIRECT_REQUEST_PERIOD`- time in minutes used along with `REDIRECT_REQUEST_PERIOD` |
Comment on lines
+125
to
+127
| # IMPORTANT: Only redirect if the matched throttle is one that has a grace period cache check | ||
| # (results/global or req/ip/results). Other throttles (req/ip, etc.) don't have grace period | ||
| # exemptions, so redirecting would create an infinite loop. |
Comment on lines
+15
to
+17
| cache_key = "turnstile_verified:#{request.ip}" | ||
| grace_period = ENV.fetch('TURNSTILE_GRACE_PERIOD') { 15 }.to_i.minutes | ||
| Rails.cache.write(cache_key, true, expires_in: grace_period) |
Comment on lines
+51
to
+54
| post turnstile_verify_path, | ||
| params: { 'cf-turnstile-response' => 'mocked', return_to: '/results?q=test' }, | ||
| headers: { 'REMOTE_ADDR' => ip } | ||
|
|
Comment on lines
+67
to
+70
| post turnstile_verify_path, | ||
| params: { 'cf-turnstile-response' => 'mocked', return_to: '/results?q=test' }, | ||
| headers: { 'REMOTE_ADDR' => ip } | ||
|
|
Comment on lines
+90
to
+93
| post turnstile_verify_path, | ||
| params: { 'cf-turnstile-response' => 'mocked', return_to: '/results?q=test' }, | ||
| headers: { 'REMOTE_ADDR' => ip } | ||
|
|
Comment on lines
+112
to
+115
| post turnstile_verify_path, | ||
| params: { 'cf-turnstile-response' => 'mocked', return_to: '/results?q=test' }, | ||
| headers: { 'REMOTE_ADDR' => ip1 } | ||
|
|
Comment on lines
+117
to
+120
| post turnstile_verify_path, | ||
| params: { 'cf-turnstile-response' => 'mocked', return_to: '/results?q=test' }, | ||
| headers: { 'REMOTE_ADDR' => ip2 } | ||
|
|
Comment on lines
+134
to
+137
| post turnstile_verify_path, | ||
| params: { return_to: '/results?q=test' }, | ||
| headers: { 'REMOTE_ADDR' => ip } | ||
|
|
|
|
||
| test 'verify applies default grace period when TURNSTILE_GRACE_PERIOD env var not set' do | ||
| with_bot_detection_enabled do | ||
| default_grace_period = 15 # minutes (the default from the controller) |
| require 'test_helper' | ||
|
|
||
| class RackAttackBlocklistTest < ActionDispatch::IntegrationTest | ||
| def test_blocked_user_agent_returns_403 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Developer
Accessibility
New ENV
Approval beyond code review
Additional context needed to review
E.g., if the PR includes updated dependencies and/or data
migration, or how to confirm the feature is working.
Code Reviewer
Code
added technical debt.
Documentation
(not just this pull request message).
Testing