Auto-provision the admin-login Turnstile widget (security F1 — wizard half) - #207
Conversation
Mirror the F5 WAF provisioning pattern for finding F1 (admin-login brute-force protection): a unit-testable turnstile-lib that idempotently creates (or reuses, by stable name) an account-level Turnstile widget for the fork's custom domain and returns its sitekey + secret. setup.ts wires the public sitekey as the TURNSTILE_SITEKEY Pages var and the secret as the TURNSTILE_SECRET Pages secret, non-fatal when the token lacks Account - Turnstile - Edit and skipped for pages.dev-only forks. Adds the Turnstile scope to the token recipe + README table (WAF scope is added by the F5 branch). The widget secret is never logged.
WIDGET_NAME was the sole reconciliation key, but one Cloudflare account can hold several forks and every fork's widget carries that same name. `.find()` then returns whichever was listed first, so provisioning a second fork adopts the first fork's sitekey — a widget scoped to the wrong domain. Every siteverify fails, and since F1 gates login on both keys being set (fail-closed), the admin login locks rather than falling back to the throttle. Match on name AND `domains` containing the fork's host. A widget with no domains field is treated as not ours, so the failure direction is always a duplicate widget (harmless, already the documented tradeoff) rather than a wrong-domain reuse. Found while provisioning widgets by hand across four forks: sparky.ink and akito.dog live in one account, which is exactly the colliding case. Three regression tests, each verified to fail against the old matcher. Existing reuse fixtures gain the `domains` the real API always returns.
sparkyfen
added
admin
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds an idempotent Cloudflare Turnstile widget reconciler, tests its creation, reuse, error, and secret-handling paths, and integrates provisioning and credential wiring into custom-domain setup. ChangesCustom-domain Turnstile setup
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Setup as scripts/setup.ts
participant Provisioner as provisionTurnstileWidget
participant Cloudflare as Cloudflare API
participant Pages as Pages API
Setup->>Provisioner: Provision widget for custom domain
Provisioner->>Cloudflare: List and reconcile account widgets
Cloudflare-->>Provisioner: Status, sitekey, and secret
Provisioner-->>Setup: Return TurnstileResult
Setup->>Pages: Write sitekey and secret
Pages-->>Setup: Complete Pages configuration
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/setup.ts`:
- Around line 605-612: Update the Turnstile reporting around turnstileStatus to
track successful sitekey and secret writes independently, rather than treating
provisioning status as configuration. Ensure failed Pages PATCH or putSecret
operations keep the integration unconfigured, add TURNSTILE_SECRET to the
initial CI secret wiring/deploy sync, and print the enabled/configured message
only when both credentials were successfully written.
In `@scripts/turnstile-lib.ts`:
- Line 111: Update the widget lookup around the api call in the Turnstile setup
flow to paginate through all challenge widgets, filtering results by the
sona-admin-login name before deciding to create one. Continue requesting
subsequent pages until the full result set is searched, and only invoke the
existing creation path when no matching widget is found.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 855430ab-9fbb-4d22-a3e1-6bb8fc5488bd
📒 Files selected for processing (4)
README.mdscripts/setup.tsscripts/turnstile-lib.test.tsscripts/turnstile-lib.ts
| // Admin-login Turnstile (finding F1). 'error' = token lacked the scope, so the | ||
| // login has no bot check; otherwise the sitekey/secret are wired and enforced. | ||
| if (turnstileStatus === 'error') { | ||
| console.log(' • Admin-login bot check: NOT set (token lacks Account · Turnstile · Edit).'); | ||
| console.log(' Add that permission to the token and re-run setup to protect /admin/login.'); | ||
| } else if (turnstileStatus) { | ||
| console.log(` • Admin-login bot check: Turnstile ${turnstileStatus} for ${host}`); | ||
| console.log(' (TURNSTILE_SITEKEY var + TURNSTILE_SECRET secret set; enforced once deployed).'); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not report Turnstile as enabled before its credentials are actually wired.
turnstileStatus reflects provisioning only (Line 336). A failed Pages PATCH or swallowed putSecret failure still reaches this success message, and the GitHub-secret list omits TURNSTILE_SECRET for the first CI deployment. Track successful sitekey and secret writes separately, include the secret in CI wiring/deploy sync, and print “configured” only when both succeed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/setup.ts` around lines 605 - 612, Update the Turnstile reporting
around turnstileStatus to track successful sitekey and secret writes
independently, rather than treating provisioning status as configuration. Ensure
failed Pages PATCH or putSecret operations keep the integration unconfigured,
add TURNSTILE_SECRET to the initial CI secret wiring/deploy sync, and print the
enabled/configured message only when both credentials were successfully written.
| if (!host) return { status: 'error', detail: 'no domain given' }; | ||
|
|
||
| // 1. List existing widgets; reconcile against ours by stable name. | ||
| const listRes = await api(cfToken, `/accounts/${accountId}/challenges/widgets?per_page=50`); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant file outline =="
ast-grep outline scripts/turnstile-lib.ts --view expanded || true
echo
echo "== surrounding lines =="
nl -ba scripts/turnstile-lib.ts | sed -n '1,220p'
echo
echo "== search for turnstile widget reconciliation usages =="
rg -n "widgets\?per_page|challenges/widgets|provisionTurnstileWidget|turnstile" scripts -SRepository: sona-fast/sona
Length of output: 867
🌐 Web query:
Cloudflare Turnstile widgets list API pagination name filter per_page 1000
💡 Result:
The Cloudflare Turnstile API for listing widgets supports pagination and filtering through query parameters [1][2]. Pagination The API supports pagination via the following query parameters: - page: The page number of paginated results [1][2]. - per_page: The number of items per page [1][2]. According to technical documentation, the maximum value supported for per_page is 1000 [3]. Filtering You can filter the list of widgets using the filter query parameter [1][4]. - Format: The filter is applied using a field:value string format (e.g., filter=name:my-widget) [2][4]. - Supported Fields: - name: Filters by widget name (case-insensitive substring matching) [1][4]. - sitekey: Filters by sitekey [2][4]. - Behavior: Providing an empty filter value will return all results [2][4]. The API will return a 400 Bad Request error if an unsupported field or invalid format is used [2][4]. Example Usage To list widgets with a custom page size, you would append the query parameters to your request: curl "https://api.cloudflare.com/client/v4/accounts/$ACCOUNT_ID/challenges/widgets?per_page=1000&filter=name:example" \ -H "Authorization: Bearer $CLOUDFLARE_API_TOKEN"
Citations:
- 1: https://developers.cloudflare.com/api/resources/turnstile/subresources/widgets/methods/list/
- 2: https://developers.cloudflare.com/api/python/resources/turnstile/subresources/widgets/
- 3: https://developers.cloudflare.com/api/python/resources/turnstile/subresources/widgets/methods/list/
- 4: https://developers.cloudflare.com/api/resources/turnstile/subresources/widgets/methods/list
🏁 Script executed:
#!/bin/bash
set -euo pipefail
grep -n "" scripts/turnstile-lib.ts | sed -n '80,150p'Repository: sona-fast/sona
Length of output: 3554
Paginate the widget lookup. The current call only reads the first 50 widgets, so a rerun can miss an existing sona-admin-login widget and create a duplicate. Filter by name and page through the full result set before creating a new widget.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/turnstile-lib.ts` at line 111, Update the widget lookup around the
api call in the Turnstile setup flow to paginate through all challenge widgets,
filtering results by the sona-admin-login name before deciding to create one.
Continue requesting subsequent pages until the full result set is searched, and
only invoke the existing creation path when no matching widget is found.
#192 touched the same three places in setup.ts. Both features are additive and independent, so every conflict keeps both sides: the two lib imports, the two provisioning calls, the two status variables, and the two Next-steps blocks. Nesting matters and is preserved — the WAF apply stays inside the resolved-zone block, Turnstile stays outside it (account-scoped, needs no zone) but inside the token check. TOKEN_RECIPE now lists both new scopes, with Turnstile in the Account group rather than appended after the Zone ones. Dropped the note telling the reader not to add Zone · WAF · Edit here — that was a merge-coordination marker for exactly this merge, and it is now stale.
1 participant
Supplies the credentials that #189 consumes. #189 makes
/admin/loginenforce Turnstile whenTURNSTILE_SITEKEY+TURNSTILE_SECRETare both set; nothing yet creates the widget or sets those keys. This adds that half.What's here
scripts/turnstile-lib.ts— idempotentprovisionTurnstileWidget, mirroringwaf-lib.tsfrom Download-beacon rate limiting (WAF applier + wizard) (security F5) #192: injectablecfApi, unit-testable, token used only as a Bearer header and never logged, widget secret never placed in a printabledetail.scripts/setup.ts— wizard hook. Non-fatal: a token withoutAccount · Turnstile · Editwarns and setup continues, matching how Download-beacon rate limiting (WAF applier + wizard) (security F5) #192 handles a missingZone · WAF · Edit.README+TOKEN_RECIPE— the new token permission row.Turnstile is account-scoped and needs no zone, so this is gated only on the fork having a custom domain, not on zone resolution.
Second commit: a real bug, found the hard way
WIDGET_NAMEalone was the reconciliation key. One Cloudflare account can hold several forks, and every fork's widget carries that same name — so.find()returns whichever is listed first, and provisioning a second fork adopts the first fork's sitekey. A widget scoped to the wrong domain fails every siteverify, and because #189 is fail-closed on both keys, the result is a locked admin login, not a fallback to the throttle.Matching now requires
domainsto contain the fork's host. A widget with nodomainsis treated as not ours, keeping the failure direction at duplicate widget (harmless, and already the documented tradeoff) rather than wrong-domain reuse.Surfaced while provisioning widgets by hand across four forks — two of them share one account, which is exactly the colliding case. Three regression tests, each verified to fail against the old matcher.
Merge order
Land this before #189. Reversed, a fork gets a login page expecting keys nothing has set — safe, since enforcement needs both, but it means F1 ships doing nothing.
Known gap, not in this PR
The wizard writes straight to Pages, which covers new forks only. Already-deployed forks need a
deploy.ymlstep syncing the repo secret/var to Pages (same reasonCRON_SECRET/SETUP_TOKENhave one — Pages secrets bind at deploy time). Left out deliberately: #205 already modifiesdeploy.yml, so that step should land inside or after it. A standalone backfill runner (the Turnstile analogue ofapply-download-ratelimit.ts) is also still missing.Verification
npm test1062/1062 ·npm run check0 errorsSummary by CodeRabbit
New Features
Documentation