WIP: Added CF Turnstile to members signup#29136
Open
sam-lord wants to merge 18 commits into
Open
Conversation
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
github-actions
Bot
added
the
migration
Contributor
|
It looks like this PR contains a migration 👀 General requirements
Schema changes
Data changes
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
WalkthroughThis PR adds Cloudflare Turnstile support across Ghost member signup/signin flows. It introduces new Turnstile settings and schema updates, a labs flag, backend verification middleware, Portal token acquisition and widget handling, frontend script injection, Admin settings UI for configuring the keys, updated tests, and new locale strings for Turnstile/security-verification errors. Possible related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 10m 23s | View ↗ |
nx run ghost:test:ci:integration |
✅ Succeeded | 2m 39s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 2m 39s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | <1s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 2m 59s | View ↗ |
nx run-many -t test:unit -p @tryghost/admin-x-s... |
✅ Succeeded | 2m 21s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 20s | View ↗ |
nx run ghost-admin:test |
✅ Succeeded | 2m 29s | View ↗ |
Additional runs (6) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-07 10:06:05 UTC
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🌐 Automated translation review
Verdict: ⏭️ Skipped — PR is too large for automated review
Reviewed 0 translations across 61 files.
This PR touches 61 translation files, which is beyond the automated reviewer's per-PR limit (max 15 files / 500 lines). A maintainer should review the translations manually.
Advisory review by i18n-review-bot. Non-blocking — a maintainer still owns the merge decision, and the bot cannot approve PRs on its own. Translator expertise wins where there is doubt.
There was a problem hiding this comment.
Pull request overview
This PR adds a Cloudflare Turnstile-based anti-bot verification flow for member signup/signin (via /members/api/send-magic-link), wiring it through Ghost core (settings, labs flag, middleware, ghost_head) and Portal (popup + [data-members-form] forms), with accompanying tests and i18n keys.
Changes:
- Add Turnstile settings (
turnstile_sitekey,turnstile_secret_key), migrations, public exposure of sitekey only, and labs flagturnstile. - Enforce Turnstile verification server-side on
send-magic-link, and inject Turnstile client script + sitekey attribute via{{ghost_head}}when active. - Implement Portal-side Turnstile token acquisition (overlay pattern) for popup flows and data-attribute forms, plus i18n strings and test coverage.
Reviewed changes
Copilot reviewed 105 out of 105 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ghost/i18n/locales/zh/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/zh-Hant/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/vi/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/uz/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/ur/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/uk/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/tr/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/th/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/ta/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/sw/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/sv/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/sr/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/sr-Cyrl/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/sq/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/sl/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/sk/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/si/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/ru/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/ro/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/pt/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/pt-BR/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/pl/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/pa/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/nn/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/nl/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/ne/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/nb/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/ms/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/mn/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/mk/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/lv/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/lt/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/kz/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/ko/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/ja/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/it/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/is/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/id/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/hu/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/hr/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/hi/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/he/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/gd/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/fr/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/fi/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/fa/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/eu/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/et/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/es/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/eo/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/en/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/el/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/de/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/de-CH/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/da/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/cs/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/ca/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/bs/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/bn/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/bg/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/ar/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/af/portal.json | Add Turnstile-related Portal translation keys |
| ghost/i18n/locales/context.json | Add context descriptions for new Turnstile-related strings |
| ghost/core/test/utils/fixtures/default-settings.json | Add Turnstile keys to test default settings fixture |
| ghost/core/test/unit/server/services/members/turnstile-service.test.js | Unit tests for TurnstileService middleware behavior |
| ghost/core/test/unit/server/data/schema/integrity.test.js | Update settings hash due to new settings |
| ghost/core/test/unit/server/data/exporter/index.test.js | Update expected settings keys count for exporter tests |
| ghost/core/test/unit/frontend/helpers/ghost-head.test.js | Add tests for Turnstile script/sitekey injection behavior |
| ghost/core/test/unit/frontend/helpers/snapshots/ghost-head.test.js.snap | Snapshot updates for {{ghost_head}} Turnstile injection |
| ghost/core/test/e2e-api/members/send-magic-link.test.js | E2E API coverage for Turnstile enforcement on send-magic-link |
| ghost/core/test/e2e-api/members/snapshots/send-magic-link.test.js.snap | Snapshot updates for Turnstile-related errors |
| ghost/core/test/e2e-api/content/settings.test.js | Ensure sitekey is public while secret key is not exposed |
| ghost/core/test/e2e-api/content/snapshots/settings.test.js.snap | Snapshot update for public settings including sitekey |
| ghost/core/test/e2e-api/admin/settings.test.js | Update settings count/index assumptions for new settings |
| ghost/core/test/e2e-api/admin/snapshots/settings.test.js.snap | Snapshot updates for admin settings responses including Turnstile keys |
| ghost/core/test/e2e-api/admin/snapshots/config.test.js.snap | Snapshot update to include turnstile labs flag |
| ghost/core/core/shared/settings-cache/public.js | Expose turnstile_sitekey in public settings cache map |
| ghost/core/core/shared/settings-cache/cache-manager.js | Document new settings in cache manager JSDoc |
| ghost/core/core/shared/labs.js | Add turnstile to private labs features list |
| ghost/core/core/shared/config/defaults.json | Add default Turnstile siteverify URL config |
| ghost/core/core/server/web/members/app.js | Mount Turnstile verification middleware on send-magic-link route |
| ghost/core/core/server/services/members/turnstile-service.js | Implement Turnstile token verification service + middleware |
| ghost/core/core/server/services/members/middleware.js | Wire TurnstileService into members middleware exports |
| ghost/core/core/server/data/schema/default-settings/default-settings.json | Add Turnstile settings to default settings schema |
| ghost/core/core/server/data/migrations/versions/6.51/2026-07-06-13-05-11-add-turnstile-sitekey-setting.js | Migration to add turnstile_sitekey setting |
| ghost/core/core/server/data/migrations/versions/6.51/2026-07-06-13-05-11-add-turnstile-secret-key-setting.js | Migration to add turnstile_secret_key setting |
| ghost/core/core/server/data/exporter/table-lists.js | Blocklist turnstile_secret_key from exports |
| ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js | Allow editing Turnstile settings via Admin settings API |
| ghost/core/core/frontend/helpers/ghost_head.js | Conditionally inject Turnstile client script + portal data attribute |
| docs/plans/cloudflare-turnstile.md | Design/implementation plan and progress log for Turnstile integration |
| apps/portal/test/utils/turnstile.test.js | Unit tests for Portal Turnstile overlay helper |
| apps/portal/test/utils/helpers.test.js | Tests for new helpers: Turnstile enabled + sitekey getters |
| apps/portal/test/signup-flow.test.jsx | Portal signup flow tests including Turnstile token submission |
| apps/portal/test/signin-flow.test.jsx | Portal signin flow tests including Turnstile token submission |
| apps/portal/test/data-attributes.test.jsx | Data-attribute forms tests including Turnstile token submission |
| apps/portal/src/utils/turnstile.js | Portal Turnstile overlay helper + token getter |
| apps/portal/src/utils/helpers.js | Add helper functions for Turnstile enablement + sitekey |
| apps/portal/src/utils/errors.js | Allowlist Turnstile-related error messages for translation/display |
| apps/portal/src/utils/api.js | Include turnstileToken in sendMagicLink request body |
| apps/portal/src/data-attributes.js | Add Turnstile token acquisition for [data-members-form] submissions |
| apps/portal/src/actions.js | Add Turnstile token acquisition for Portal popup signin/signup actions |
| apps/admin-x-settings/test/acceptance/advanced/spam-filters.test.ts | Acceptance tests for Turnstile UI in spam filters settings |
| apps/admin-x-settings/src/components/settings/membership/membership-settings.tsx | Add spam filters keywords and pass correct keyword set to SpamFilters |
| apps/admin-x-settings/src/components/settings/advanced/spam-filters.tsx | Add Turnstile configuration UI + validation + warning banner |
| apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx | Add Turnstile private labs toggle to Admin UI |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+504
to
+506
| export function hasTurnstileEnabled({site}) { | ||
| return !!(site?.labs?.turnstile && site?.turnstile_sitekey); | ||
| } |
Comment on lines
+107
to
+109
| <Banner color='yellow' data-testid='turnstile-warning'> | ||
| While Turnstile is enabled, custom or third-party signup forms that post to the members API directly will stop working, and signup forms embedded on other websites only work if those domains are added to the widget's hostname allowlist in the Cloudflare dashboard. | ||
| </Banner> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (7)
docs/plans/cloudflare-turnstile.md (2)
102-104: 📐 Maintainability & Code Quality | 🔵 TrivialDrop the
settings-public.jsstep here.The branch already exposes the sitekey through the public settings merge path, and the progress log says
settings-public.jsneeded no change. Keeping this in the plan sends readers to a dead-end edit.Suggested edit
- - Expose **only** the sitekey publicly: `ghost/core/core/shared/settings-cache/public.js` and the - members/public settings serializer (`api/endpoints/settings-public.js`) — mirror what the old - captcha commit did (in reverse). + - Expose **only** the sitekey publicly via `ghost/core/core/shared/settings-cache/public.js`. + The members/public settings serializer does not need a dedicated change because it already + merges `getPublic()`.🤖 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 `@docs/plans/cloudflare-turnstile.md` around lines 102 - 104, Remove the planned `settings-public.js` change from the Cloudflare Turnstile doc because the public sitekey is already exposed through the existing public settings merge path. Update the plan section that mentions `ghost/core/core/shared/settings-cache/public.js` and `api/endpoints/settings-public.js` so it only references the actual file that needs changes, keeping the guidance aligned with the branch’s current implementation and avoiding a dead-end edit.
105-118: 📐 Maintainability & Code Quality | 🔵 TrivialAlign the service path and wiring with the implementation.
This still points at
TurnstileService.jsand boot-time wiring inapi.js/members-api.js, but the branch usesturnstile-service.jsand constructsverifyTurnstileinservices/members/middleware.jswith per-request checks. The current wording will misdirect anyone following the plan.Suggested edit
- **TurnstileService** at `ghost/core/core/server/services/members/TurnstileService.js`, modeled on + **TurnstileService** at `ghost/core/core/server/services/members/turnstile-service.js`, modeled on @@ - - Wired up in `services/members/api.js` / `members-api.js` reading labs + settings, and mounted in - `ghost/core/core/server/web/members/app.js` on the `send-magic-link` route, after - `verifyIntegrityToken` and before the brute middleware (failing a bot before it consumes brute - allowance for a real email). Applies to **all** email types (decision 3). + - Constructed in `ghost/core/core/server/services/members/middleware.js` and mounted in + `ghost/core/core/server/web/members/app.js` on the `send-magic-link` route, after + `verifyIntegrityToken` and before the brute middleware. Applies to **all** email types + (decision 3).🤖 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 `@docs/plans/cloudflare-turnstile.md` around lines 105 - 118, The plan references the wrong service file and initialization flow for Turnstile. Update the documentation to point to turnstile-service.js and the verifyTurnstile middleware in services/members/middleware.js, and describe the per-request validation path instead of boot-time wiring in api.js/members-api.js. Keep the route placement guidance in app.js, but make the symbols match the actual implementation so readers can find the correct code quickly.apps/admin-x-settings/src/components/settings/advanced/spam-filters.tsx (1)
36-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
getSettingValuescall for the same keys.
turnstile_sitekey/turnstile_secret_keyare already fetched insideonValidate(line 23) using the same call. Consider computing this once and reusing it, to avoid duplicated lookups/logic drift.🤖 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 `@apps/admin-x-settings/src/components/settings/advanced/spam-filters.tsx` around lines 36 - 37, The same `getSettingValues` lookup for `turnstile_sitekey` and `turnstile_secret_key` is duplicated in `spam-filters.tsx`, causing redundant work and potential drift between `onValidate` and the local destructuring. Compute the pair once in `SpamFilters` (or reuse the existing `onValidate` result) and pass/derive it from that shared value so both the validation logic and the `turnstileSitekey`/`turnstileSecretKey` assignment use the same source.ghost/core/test/e2e-api/members/send-magic-link.test.js (1)
750-886: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd coverage for siteverify network failure/timeout.
The suite covers missing-token, accept, reject, signin-enforcement, and bypass paths, but not what happens when the Cloudflare siteverify call itself fails (network error, non-200, or times out). Per the
TurnstileServicesnippet, that path returns anInternalServerError(fail-closed), which is worth locking in with a test given this is a bot-protection gate on a public signup endpoint.Cloudflare's own guidance stresses setting explicit timeouts and having fallback behavior for Siteverify failures — worth confirming
externalRequest.postinturnstile-service.jshas a bounded timeout, since none is visible in the referenced call.🧪 Suggested additional test
+ it('Fails closed when siteverify errors', async function () { + nock('https://challenges.cloudflare.com') + .post('/turnstile/v0/siteverify') + .replyWithError('network error'); + + await membersAgent.post('/api/send-magic-link') + .body({ + email: 'turnstile-siteverify-error@test.com', + emailType: 'signup', + turnstileToken: 'XXXX.DUMMY.TOKEN.XXXX' + }) + .expectStatus(500); + + mockManager.assert.sentEmailCount(0); + });🤖 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 `@ghost/core/test/e2e-api/members/send-magic-link.test.js` around lines 750 - 886, Add an e2e test in the Turnstile verification suite that simulates a Cloudflare siteverify failure (network error, non-200, or timeout) for membersAgent.post('/api/send-magic-link') using mockSiteverify, and assert the request fails closed with InternalServerError and no email is sent. Use the existing TurnstileService/siteverify flow and the mockSiteverify helper to locate the path, and verify the failure behavior matches the TurnstileService contract when externalRequest.post cannot complete successfully. Also inspect turnstile-service.js where externalRequest.post is used and ensure it has an explicit timeout so the failure case is bounded.apps/portal/src/utils/turnstile.js (1)
63-104: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winOverlay lacks accessible modal semantics.
The full-page overlay shown during
before-interactive-callbackhas norole="dialog"/aria-modaland no focus trapping, so keyboard/screen-reader users could tab to background content while it's visible. This only surfaces for Cloudflare's rarer interactive challenge, so impact is limited, but it's still a real gap for that path.🤖 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 `@apps/portal/src/utils/turnstile.js` around lines 63 - 104, The full-page challenge overlay created in ensureOverlay currently lacks dialog semantics and keyboard containment. Update the overlay setup to add accessible modal attributes such as role="dialog" and aria-modal="true", and ensure showOverlay/hideOverlay also manage focus by moving focus into the widget container and trapping/restoring it while the overlay is visible. Use the existing ensureOverlay, showOverlay, and hideOverlay helpers to keep the behavior localized.apps/portal/src/actions.js (1)
125-131: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winParallelize independent token fetches to cut added latency.
In both
signinandsignup,getPopupTurnstileTokenandapi.member.getIntegrityTokenare awaited sequentially, but neither depends on the other's result. Running them concurrently reduces the added round-trip time from Turnstile verification before the magic-link request is sent.♻️ Proposed fix
- const turnstileToken = await getPopupTurnstileToken({state}); - const integrityToken = await api.member.getIntegrityToken(); + const [turnstileToken, integrityToken] = await Promise.all([ + getPopupTurnstileToken({state}), + api.member.getIntegrityToken() + ]);Apply the same pattern in
signup(lines 209-210).Also applies to: 209-211
🤖 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 `@apps/portal/src/actions.js` around lines 125 - 131, In `signin` and `signup`, `getPopupTurnstileToken` and `api.member.getIntegrityToken` are independent and should not be awaited one after the other. Update the token fetch flow in `actions.js` so both promises start together and are awaited together before building the payload, using the existing `signin`/`signup` action logic as the place to change. Apply the same concurrency pattern in both affected sections so the magic-link request isn’t delayed by sequential token fetches.apps/portal/src/data-attributes.js (1)
80-89: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider parallelizing the integrity-token fetch and Turnstile token acquisition.
The integrity token network request and the Turnstile token retrieval are independent and currently run sequentially, adding latency to form submission.
♻️ Proposed fix
- const integrityTokenRes = await fetch(`${siteUrl}/members/api/integrity-token/`, {method: 'GET'}); - const integrityToken = await integrityTokenRes.text(); - - if (turnstileSitekey) { - // Runs the widget in the main page's document; the overlay only - // appears if Cloudflare requires interaction - reqBody.turnstileToken = await getTurnstileToken({doc: document, sitekey: turnstileSitekey}); - } + const [integrityTokenRes, turnstileToken] = await Promise.all([ + fetch(`${siteUrl}/members/api/integrity-token/`, {method: 'GET'}), + // Runs the widget in the main page's document; the overlay only + // appears if Cloudflare requires interaction + turnstileSitekey ? getTurnstileToken({doc: document, sitekey: turnstileSitekey}) : Promise.resolve(undefined) + ]); + const integrityToken = await integrityTokenRes.text(); + if (turnstileToken) { + reqBody.turnstileToken = turnstileToken; + }🤖 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 `@apps/portal/src/data-attributes.js` around lines 80 - 89, The integrity-token request and Turnstile token acquisition in data-attributes.js are independent but currently awaited sequentially inside the form submission flow. Update the logic around the integrity token fetch and getTurnstileToken call to start both operations in parallel, then await their results before building reqBody so the submission still includes both values. Keep the existing conditional behavior for turnstileSitekey and preserve the current try/catch handling around this block.
🤖 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.
Nitpick comments:
In `@apps/admin-x-settings/src/components/settings/advanced/spam-filters.tsx`:
- Around line 36-37: The same `getSettingValues` lookup for `turnstile_sitekey`
and `turnstile_secret_key` is duplicated in `spam-filters.tsx`, causing
redundant work and potential drift between `onValidate` and the local
destructuring. Compute the pair once in `SpamFilters` (or reuse the existing
`onValidate` result) and pass/derive it from that shared value so both the
validation logic and the `turnstileSitekey`/`turnstileSecretKey` assignment use
the same source.
In `@apps/portal/src/actions.js`:
- Around line 125-131: In `signin` and `signup`, `getPopupTurnstileToken` and
`api.member.getIntegrityToken` are independent and should not be awaited one
after the other. Update the token fetch flow in `actions.js` so both promises
start together and are awaited together before building the payload, using the
existing `signin`/`signup` action logic as the place to change. Apply the same
concurrency pattern in both affected sections so the magic-link request isn’t
delayed by sequential token fetches.
In `@apps/portal/src/data-attributes.js`:
- Around line 80-89: The integrity-token request and Turnstile token acquisition
in data-attributes.js are independent but currently awaited sequentially inside
the form submission flow. Update the logic around the integrity token fetch and
getTurnstileToken call to start both operations in parallel, then await their
results before building reqBody so the submission still includes both values.
Keep the existing conditional behavior for turnstileSitekey and preserve the
current try/catch handling around this block.
In `@apps/portal/src/utils/turnstile.js`:
- Around line 63-104: The full-page challenge overlay created in ensureOverlay
currently lacks dialog semantics and keyboard containment. Update the overlay
setup to add accessible modal attributes such as role="dialog" and
aria-modal="true", and ensure showOverlay/hideOverlay also manage focus by
moving focus into the widget container and trapping/restoring it while the
overlay is visible. Use the existing ensureOverlay, showOverlay, and hideOverlay
helpers to keep the behavior localized.
In `@docs/plans/cloudflare-turnstile.md`:
- Around line 102-104: Remove the planned `settings-public.js` change from the
Cloudflare Turnstile doc because the public sitekey is already exposed through
the existing public settings merge path. Update the plan section that mentions
`ghost/core/core/shared/settings-cache/public.js` and
`api/endpoints/settings-public.js` so it only references the actual file that
needs changes, keeping the guidance aligned with the branch’s current
implementation and avoiding a dead-end edit.
- Around line 105-118: The plan references the wrong service file and
initialization flow for Turnstile. Update the documentation to point to
turnstile-service.js and the verifyTurnstile middleware in
services/members/middleware.js, and describe the per-request validation path
instead of boot-time wiring in api.js/members-api.js. Keep the route placement
guidance in app.js, but make the symbols match the actual implementation so
readers can find the correct code quickly.
In `@ghost/core/test/e2e-api/members/send-magic-link.test.js`:
- Around line 750-886: Add an e2e test in the Turnstile verification suite that
simulates a Cloudflare siteverify failure (network error, non-200, or timeout)
for membersAgent.post('/api/send-magic-link') using mockSiteverify, and assert
the request fails closed with InternalServerError and no email is sent. Use the
existing TurnstileService/siteverify flow and the mockSiteverify helper to
locate the path, and verify the failure behavior matches the TurnstileService
contract when externalRequest.post cannot complete successfully. Also inspect
turnstile-service.js where externalRequest.post is used and ensure it has an
explicit timeout so the failure case is bounded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cb02aa2b-b6a0-448a-8d3d-619718bbd541
⛔ Files ignored due to path filters (5)
ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snapis excluded by!**/*.snapghost/core/test/e2e-api/admin/__snapshots__/settings.test.js.snapis excluded by!**/*.snapghost/core/test/e2e-api/content/__snapshots__/settings.test.js.snapis excluded by!**/*.snapghost/core/test/e2e-api/members/__snapshots__/send-magic-link.test.js.snapis excluded by!**/*.snapghost/core/test/unit/frontend/helpers/__snapshots__/ghost-head.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (100)
apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsxapps/admin-x-settings/src/components/settings/advanced/spam-filters.tsxapps/admin-x-settings/src/components/settings/membership/membership-settings.tsxapps/admin-x-settings/test/acceptance/advanced/spam-filters.test.tsapps/portal/src/actions.jsapps/portal/src/data-attributes.jsapps/portal/src/utils/api.jsapps/portal/src/utils/errors.jsapps/portal/src/utils/helpers.jsapps/portal/src/utils/turnstile.jsapps/portal/test/data-attributes.test.jsxapps/portal/test/signin-flow.test.jsxapps/portal/test/signup-flow.test.jsxapps/portal/test/utils/helpers.test.jsapps/portal/test/utils/turnstile.test.jsdocs/plans/cloudflare-turnstile.mdghost/core/core/frontend/helpers/ghost_head.jsghost/core/core/server/api/endpoints/utils/serializers/input/settings.jsghost/core/core/server/data/exporter/table-lists.jsghost/core/core/server/data/migrations/versions/6.51/2026-07-06-13-05-11-add-turnstile-secret-key-setting.jsghost/core/core/server/data/migrations/versions/6.51/2026-07-06-13-05-11-add-turnstile-sitekey-setting.jsghost/core/core/server/data/schema/default-settings/default-settings.jsonghost/core/core/server/services/members/middleware.jsghost/core/core/server/services/members/turnstile-service.jsghost/core/core/server/web/members/app.jsghost/core/core/shared/config/defaults.jsonghost/core/core/shared/labs.jsghost/core/core/shared/settings-cache/cache-manager.jsghost/core/core/shared/settings-cache/public.jsghost/core/test/e2e-api/admin/settings.test.jsghost/core/test/e2e-api/content/settings.test.jsghost/core/test/e2e-api/members/send-magic-link.test.jsghost/core/test/unit/frontend/helpers/ghost-head.test.jsghost/core/test/unit/server/data/exporter/index.test.jsghost/core/test/unit/server/data/schema/integrity.test.jsghost/core/test/unit/server/services/members/turnstile-service.test.jsghost/core/test/utils/fixtures/default-settings.jsonghost/i18n/locales/af/portal.jsonghost/i18n/locales/ar/portal.jsonghost/i18n/locales/bg/portal.jsonghost/i18n/locales/bn/portal.jsonghost/i18n/locales/bs/portal.jsonghost/i18n/locales/ca/portal.jsonghost/i18n/locales/context.jsonghost/i18n/locales/cs/portal.jsonghost/i18n/locales/da/portal.jsonghost/i18n/locales/de-CH/portal.jsonghost/i18n/locales/de/portal.jsonghost/i18n/locales/el/portal.jsonghost/i18n/locales/en/portal.jsonghost/i18n/locales/eo/portal.jsonghost/i18n/locales/es/portal.jsonghost/i18n/locales/et/portal.jsonghost/i18n/locales/eu/portal.jsonghost/i18n/locales/fa/portal.jsonghost/i18n/locales/fi/portal.jsonghost/i18n/locales/fr/portal.jsonghost/i18n/locales/gd/portal.jsonghost/i18n/locales/he/portal.jsonghost/i18n/locales/hi/portal.jsonghost/i18n/locales/hr/portal.jsonghost/i18n/locales/hu/portal.jsonghost/i18n/locales/id/portal.jsonghost/i18n/locales/is/portal.jsonghost/i18n/locales/it/portal.jsonghost/i18n/locales/ja/portal.jsonghost/i18n/locales/ko/portal.jsonghost/i18n/locales/kz/portal.jsonghost/i18n/locales/lt/portal.jsonghost/i18n/locales/lv/portal.jsonghost/i18n/locales/mk/portal.jsonghost/i18n/locales/mn/portal.jsonghost/i18n/locales/ms/portal.jsonghost/i18n/locales/nb/portal.jsonghost/i18n/locales/ne/portal.jsonghost/i18n/locales/nl/portal.jsonghost/i18n/locales/nn/portal.jsonghost/i18n/locales/pa/portal.jsonghost/i18n/locales/pl/portal.jsonghost/i18n/locales/pt-BR/portal.jsonghost/i18n/locales/pt/portal.jsonghost/i18n/locales/ro/portal.jsonghost/i18n/locales/ru/portal.jsonghost/i18n/locales/si/portal.jsonghost/i18n/locales/sk/portal.jsonghost/i18n/locales/sl/portal.jsonghost/i18n/locales/sq/portal.jsonghost/i18n/locales/sr-Cyrl/portal.jsonghost/i18n/locales/sr/portal.jsonghost/i18n/locales/sv/portal.jsonghost/i18n/locales/sw/portal.jsonghost/i18n/locales/ta/portal.jsonghost/i18n/locales/th/portal.jsonghost/i18n/locales/tr/portal.jsonghost/i18n/locales/uk/portal.jsonghost/i18n/locales/ur/portal.jsonghost/i18n/locales/uz/portal.jsonghost/i18n/locales/vi/portal.jsonghost/i18n/locales/zh-Hant/portal.jsonghost/i18n/locales/zh/portal.json
sam-lord
added a commit
that referenced
this pull request
Jul 7, 2026
ref #29136 - v6.51.0 shipped while this branch was in review, so the two setting migrations moved from 6.51/ to 6.52/ to satisfy the migration integrity check - the spam-filters acceptance test asserted lastApiRequests.editSettings on a mock that never registered editSettings, which fails tsc; the mock is now registered (matching the pattern used elsewhere) and stays undefined when no request is made - the settings model test now expects 114 default settings to account for turnstile_sitekey and turnstile_secret_key
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Contributor
There was a problem hiding this comment.
🌐 Automated translation review
Verdict: ⏭️ Skipped — PR is too large for automated review
Reviewed 0 translations across 61 files.
This PR touches 61 translation files, which is beyond the automated reviewer's per-PR limit (max 15 files / 500 lines). A maintainer should review the translations manually.
Advisory review by i18n-review-bot. Non-blocking — a maintainer still owns the merge decision, and the bot cannot approve PRs on its own. Translator expertise wins where there is doubt.
no ref - spam signups need bot protection; Turnstile chosen over reviving hCaptcha as it is free, privacy-friendly, and needs no server-side SDK - captures the decisions already made (hard enforcement with admin warning, modal-only-when-needed widget, all send-magic-link types protected) so implementation sessions don't re-litigate them - points at the removed hCaptcha implementation (1091014) as the template for every integration point - includes a phased checklist designed to be worked through incrementally by /loop
…diff ref docs/plans/cloudflare-turnstile.md - first Phase 0 item: read the 1091014 captcha cleanup commit end to end before implementing - the plan missed several touchpoints that commit reveals: settings-cache CacheManager typedef, content settings + version API snapshots, exporter allowedKeysLength, and Portal's fixtures-generator site params - also recorded two prior-art divergences so later phases don't copy them: the old CaptchaService was config-driven and built once at boot (no restart-free enablement), and its middleware lived in members-api.js rather than web/members/app.js
ref docs/plans/cloudflare-turnstile.md - first scaffolding step for Cloudflare Turnstile signup protection: the flag gates every subsequent code path, so it lands first - private (developer experiments) rather than public beta because the feature is not functional yet - includes the labs settings toggle and the config API snapshot update
ref docs/plans/cloudflare-turnstile.md - scaffolding for Cloudflare Turnstile signup protection: the sitekey must reach public clients (Portal, data-attribute forms) and the secret key stays server-side for siteverify calls - only the sitekey is exposed via the public settings cache; the secret key name matches the settings API redaction pattern and is added to the exporter blocklist like every other secret setting - settings are also added to the pinned test fixture copy of default-settings.json, without which they would not exist in e2e test databases - includes a Content API test asserting the secret is never exposed publicly
ref docs/plans/cloudflare-turnstile.md - modeled on the CaptchaService removed in 1091014, but reads enabled/secretKey lazily (constructor accepts functions evaluated per request) so flipping the labs flag or editing keys in settings takes effect without a restart - verification failures return deliberately sparse client errors; Cloudflare error codes are only logged server-side to avoid leaking signal to bots - not yet wired to any route; middleware mounting is the next step
ref docs/plans/cloudflare-turnstile.md - the TurnstileService takes its siteverify endpoint from config so tests and self-hosters can point it elsewhere (e.g. a mock), with the production Cloudflare URL as the default
ref docs/plans/cloudflare-turnstile.md - covers all email types (signup, subscribe, signin) like the previous hCaptcha implementation, so bots cannot fall back to an unprotected path - mounted after the integrity token check and before the brute middleware so a failing bot doesn't consume brute allowance for a real member's email - only active when the turnstile labs flag is on and both keys are set, evaluated per request so enabling it doesn't need a restart - e2e tests mock the siteverify endpoint with nock: success, failure, missing token, signin enforcement, and flag-off/keys-unset regressions
ref docs/plans/cloudflare-turnstile.md - injects the Cloudflare api.js script (render=explicit) and a data-turnstile-sitekey attribute on the Portal script tag when Turnstile is active - the api.js copy on the main page is what data-attribute signup forms will drive; Portal reads the sitekey from the data attribute so it can act before the settings fetch - uses the same active check as the send-magic-link middleware (labs flag + both keys) so the widget is never injected when the server wouldn't enforce it
ref docs/plans/cloudflare-turnstile.md - confirmed in a real browser that a Turnstile widget renders, executes and returns tokens inside a srcDoc iframe like Portal's popup, so the planned in-iframe overlay design is viable and no postMessage fallback is needed - recorded two gotchas for the overlay helper: inject api.js only after the srcdoc document replaces the transient about:blank one, and size the overlay container ourselves because the widget lives in a closed shadow root
ref docs/plans/cloudflare-turnstile.md
- createTurnstile({doc, sitekey}) is document-bound so the same helper serves Portal's popup iframe and data-attribute forms on the main page, without a React dependency
- the widget runs invisibly and the overlay only appears while Cloudflare requires interaction, per the settled UX decision; the widget resets before each execution because tokens are single-use
- the overlay sizes its own box because Turnstile renders into a closed shadow root (confirmed in the srcDoc iframe spike)
- hasTurnstileEnabled requires both the labs flag and a sitekey on the site object so Portal stays inert when the server wouldn't enforce verification
ref docs/plans/cloudflare-turnstile.md - when Turnstile is active, both flows fetch a token before requesting the magic link and pass it as turnstileToken, matching what the send-magic-link middleware now expects - the verifier runs in the popup iframe's own document so the challenge overlay appears over the popup content, found by data-testid rather than threading a ref through App state like the old hCaptcha code - verification failures surface through the existing signin/signup failure notifications - flow tests cover token delivery on single/multi tier sites, overlay visibility around the interactive callbacks, and the flag-off regression
ref docs/plans/cloudflare-turnstile.md - theme forms posting via [data-members-form] now fetch a Turnstile token between the integrity token and the send-magic-link request when Turnstile is active - the widget runs in the main page's document using the api.js copy ghost_head injects, with the shared overlay helper showing a challenge only when Cloudflare requires interaction — never an inline widget in theme markup, per the settled decision - verification failures surface through the existing [data-members-error] elements
ref docs/plans/cloudflare-turnstile.md - Portal only shows specific error messages that are on the specialMessages allowlist in utils/errors.js, so the four Turnstile failure strings are registered there rather than wrapped in t() at the throw site - covers the server's sparse rejection message plus the client-side load/failure/expiry errors from the overlay helper - locale files updated via the i18n translate script, with context.json descriptions for translators
ref docs/plans/cloudflare-turnstile.md - site key and secret key fields appear behind the turnstile labs flag, with both-or-neither validation since a partial configuration would leave verification silently inactive - a warning banner explains that custom/third-party forms posting to the members API stop working while Turnstile is enabled, and that externally embedded signup forms need their domains allowlisted in the Cloudflare widget - SpamFilters gets its own search keywords (turnstile, captcha, bot) instead of borrowing the Access section's list
ref docs/plans/cloudflare-turnstile.md - everything Turnstile-related is green: portal, admin-x-settings unit + acceptance, all affected e2e-api suites, three workspace lints, and a clean i18n translate - the only ghost/core unit failures are pre-existing timezone-dependent automations tests that pass under TZ=UTC and fail under Europe/London; unrelated to this branch and flagged as a separate task - sweep checklist item deliberately left unchecked pending a human call on the unrelated failures
ref docs/plans/cloudflare-turnstile.md - main has since released 6.51, and migrations added to an already-shipped version folder risk being skipped on sites that have upgraded past it; migrate:create confirms 6.51 is the current target - also regenerated the admin settings snapshot content-length, which changed because main added settings of its own since the branch point
ref #29136 - v6.51.0 shipped while this branch was in review, so the two setting migrations moved from 6.51/ to 6.52/ to satisfy the migration integrity check - the spam-filters acceptance test asserted lastApiRequests.editSettings on a mock that never registered editSettings, which fails tsc; the mock is now registered (matching the pattern used elsewhere) and stays undefined when no request is made - the settings model test now expects 114 default settings to account for turnstile_sitekey and turnstile_secret_key - regenerated the admin settings snapshot content-length after rebasing onto latest main
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Contributor
There was a problem hiding this comment.
🌐 Automated translation review
Verdict: ⏭️ Skipped — PR is too large for automated review
Reviewed 0 translations across 61 files.
This PR touches 61 translation files, which is beyond the automated reviewer's per-PR limit (max 15 files / 500 lines). A maintainer should review the translations manually.
Advisory review by i18n-review-bot. Non-blocking — a maintainer still owns the merge decision, and the bot cannot approve PRs on its own. Translator expertise wins where there is doubt.
ref #29136 - the Turnstile setting migrations moved to the 6.52/ folder after v6.51.0 shipped, and migration folders must not exceed the package version or knex-migrator would skip them at boot - this is the same promote-to-next-minor-rc bump that ghost/core/bin/create-migration.js applies automatically, covering both ghost/core and ghost/admin
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Contributor
There was a problem hiding this comment.
🌐 Automated translation review
Verdict: ⏭️ Skipped — PR is too large for automated review
Reviewed 0 translations across 61 files.
This PR touches 61 translation files, which is beyond the automated reviewer's per-PR limit (max 15 files / 500 lines). A maintainer should review the translations manually.
Advisory review by i18n-review-bot. Non-blocking — a maintainer still owns the merge decision, and the bot cannot approve PRs on its own. Translator expertise wins where there is doubt.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #29136 +/- ##
==========================================
+ Coverage 74.26% 74.30% +0.04%
==========================================
Files 1565 1567 +2
Lines 135738 135853 +115
Branches 16452 16477 +25
==========================================
+ Hits 100805 100950 +145
+ Misses 33934 33902 -32
- Partials 999 1001 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
This is a spike to check whether we can simply add CF Turnstile to prevent spam member signups. The code is loosely based on the hCaptcha implementation, but is otherwise a one-shot prompt & will need to be thoroughly reviewed (by myself & others) before it can be merged.