[WIP] Added Captcha settings for self-hosters #22757
Conversation
Issues with this commit currently: * No design review, copy needs addressing for the new fields in Ghost Admin * The change to using `hostSettings:captcha` for siteKey & secretKey needs to be actioned for Ghost (Pro)
WalkthroughThis pull request introduces new functionality to manage hCaptcha credentials. In the admin interface, the Possibly related PRs
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/admin-x-settings/src/components/settings/advanced/SpamFilters.tsxOops! Something went wrong! :( ESLint: 8.44.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "/apps/admin-x-settings".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in "apps/admin-x-settings/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
the
migration
|
It looks like this PR contains a migration 👀 General requirements
Schema changes
Data changes
|
![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
🧹 Nitpick comments (5)
ghost/core/core/server/data/schema/default-settings/default-settings.json (1)
331-337: Added captcha settings to default settings schema.Good addition of the
captcha_sitekeyandcaptcha_secretproperties to the members section of the default settings schema, with appropriate null default values.Consider adding appropriate flags to these properties. In particular, the
captcha_secretis sensitive and should probably not be exposed publicly. You might want to add theRO(read-only) flag to it to prevent it from being changed through public APIs."captcha_secret": { "defaultValue": null, - "type": "string" + "type": "string", + "flags": "RO" }ghost/core/core/server/api/endpoints/settings-public.js (1)
9-17: Consider using optional chaining for safer property accessThe current implementation assumes that
hostSettingsandcaptchaobjects exist in the config. For more robust code, consider using optional chaining or checking for the existence of these properties before accessing them.- const siteKey = config.get('hostSettings:captcha:siteKey'); + const siteKey = config.get('hostSettings:captcha:siteKey') || config.get('hostSettings:captcha')?.siteKey;apps/admin-x-settings/src/components/settings/advanced/SpamFilters.tsx (3)
118-136: Simplify conditional rendering and remove unnecessary fragmentThe fragment (
<>...</>) is unnecessary since it contains only one child. Additionally, the conditional logic could be more explicit for better readability.- {config?.hostSettings?.captcha || (<> - <SettingGroupContent> - <TextField - hint="TODO Change: Unique identifier for your site" - maxLength={36} // UUIDv4 format - placeholder="hCaptcha sitekey" - title="hCaptcha sitekey" - value={captchaSitekey} - onChange={handleSitekeyChange} - /> - <TextField - hint="TODO Change:Private secret key used to verify hCaptcha responses" - maxLength={100} - placeholder="hCaptcha secret" - title="hCaptcha secret" - value={captchaSecret} - onChange={handleSecretChange} /> - </SettingGroupContent> - </>)} + {!config?.hostSettings?.captcha && ( + <SettingGroupContent> + <TextField + hint="TODO Change: Unique identifier for your site" + maxLength={36} // UUIDv4 format + placeholder="hCaptcha sitekey" + title="hCaptcha sitekey" + value={captchaSitekey} + onChange={handleSitekeyChange} + /> + <TextField + hint="TODO Change: Private secret key used to verify hCaptcha responses" + maxLength={100} + placeholder="hCaptcha secret" + title="hCaptcha secret" + value={captchaSecret} + onChange={handleSecretChange} /> + </SettingGroupContent> + )}🧰 Tools
🪛 Biome (1.9.4)
[error] 118-136: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
128-135: Consider adding validation for the hCaptcha secret keyThe secret key input lacks validation. Consider adding basic validation to ensure users enter a properly formatted secret key.
120-135: Consider adding a help link to hCaptcha documentationIt would be helpful to provide a direct link to hCaptcha's documentation on how to obtain these credentials, similar to the link in the captchaHint. This would make it easier for self-hosters to set up hCaptcha.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/admin-x-settings/src/components/settings/advanced/SpamFilters.tsx(3 hunks)ghost/core/core/server/api/endpoints/settings-public.js(1 hunks)ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js(1 hunks)ghost/core/core/server/data/migrations/versions/5.116/2025-03-28-17-58-29-add-captcha-selfhoster-settings.js(1 hunks)ghost/core/core/server/data/schema/default-settings/default-settings.json(1 hunks)ghost/core/core/server/services/members/api.js(1 hunks)ghost/core/core/shared/settings-cache/CacheManager.js(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apps/admin-x-settings/src/components/settings/advanced/SpamFilters.tsx (1)
ghost/core/core/server/api/endpoints/settings-public.js (1)
config(4-4)
🪛 Biome (1.9.4)
apps/admin-x-settings/src/components/settings/advanced/SpamFilters.tsx
[error] 118-136: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Regression tests (Node 20.11.1, mysql8)
🔇 Additional comments (5)
ghost/core/core/server/api/endpoints/utils/serializers/input/settings.js (1)
80-81: Added captcha configuration settings appropriately.Good addition of the
captcha_sitekeyandcaptcha_secretsettings to the editable settings list, which is necessary for allowing self-hosters to configure captcha functionality through the admin interface.ghost/core/core/server/services/members/api.js (1)
247-247: Added appropriate fallback for captcha secret key.Good implementation of a fallback mechanism for the captcha secret key. This ensures the system can get the key from host settings (for Ghost Pro) or from the settings cache (for self-hosters), fulfilling the PR objective.
ghost/core/core/shared/settings-cache/CacheManager.js (1)
63-64:✅ Verification successful
Added captcha settings to PublicSettingsCache typedef.
Good addition of the
captcha_sitekeyandcaptcha_secretproperties to the PublicSettingsCache typedef, with clear JSDoc comments explaining their purpose.Verify that the
captcha_secretis not exposed through public APIs, as it's a sensitive value:
🏁 Script executed:
#!/bin/bash # Check if captcha_secret is exposed in public settings API endpoints rg -A 10 -B 10 "getPublic|publicSettings" --glob="*.js"Length of output: 45974
Captcha settings addition approved
The new properties (
captcha_sitekeyandcaptcha_secret) are clearly documented in the PublicSettingsCache typedef and inserted in a logical location. I ran a grep search on public API endpoints and found no evidence that the sensitivecaptcha_secretis being exposed. That said, please ensure that any merging functions (such as those in getCaptchaSettings) continue to filter out or handlecaptcha_secretsecurely.ghost/core/core/server/data/migrations/versions/5.116/2025-03-28-17-58-29-add-captcha-selfhoster-settings.js (1)
1-16: LGTM! Well-structured migration for adding captcha settingsThe migration correctly adds two new settings (
captcha_sitekeyandcaptcha_secret) for self-hosters to configure hCaptcha. The use ofcombineTransactionalMigrationsensures these settings are added atomically.apps/admin-x-settings/src/components/settings/advanced/SpamFilters.tsx (1)
120-127:✅ Verification successful
Consider adding validation for the hCaptcha site key
The site key input lacks validation. hCaptcha site keys typically follow a specific format. Consider adding validation to ensure users enter a valid site key format.
<TextField hint="TODO Change: Unique identifier for your site" maxLength={36} // UUIDv4 format placeholder="hCaptcha sitekey" title="hCaptcha sitekey" value={captchaSitekey} + error={captchaSitekey && !/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/.test(captchaSitekey) ? "Please enter a valid hCaptcha site key" : undefined} onChange={handleSitekeyChange} />
🌐 Web query:
What is the format of an hCaptcha site key?💡 Result:
hCaptcha site keys follow a standardized format consisting of 36 alphanumeric characters structured as eight hexadecimal digits followed by three groups of four and a final group of twelve, all separated by hyphens. This creates a pattern that resembles a UUID:
XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX.Examples from documentation include[4]:
20000000-ffff-ffff-ffff-000000000002(Enterprise account test key)10000000-ffff-ffff-ffff-000000000001(Publisher/Pro account test key)Key characteristics:
- Hexadecimal composition: Uses characters 0-9 and a-f/A-F
- Fixed segments: Always contains four hyphen-separated groups in an 8-4-4-4-12 arrangement
- Case-insensitive: Works with uppercase or lowercase letters
- Unique identifiers: Designed to be globally unique per integration
Developers implement these keys in HTML using the
data-sitekeyattribute:<div class="h-captcha" data-sitekey="your-site-key-here"></div>Test keys provided in hCaptcha's documentation maintain this format while using placeholder values (like repeated
fcharacters) to simulate valid keys[4][7].Citations:
- 1: https://wpforms.com/docs/how-to-set-up-and-use-hcaptcha-in-wpforms/
- 2: https://docs.netacea.com/captcha/custom-hcaptcha-page-guide
- 3: https://www.tassos.gr/docs/convert-forms/security/hcaptcha-to-your-form
- 4: https://docs.hcaptcha.com
- 5: https://lists.bgsu.edu/scripts/wa.exe?SHOWTPL=HELP-SYSCFG&S=WWW_HCAPTCHA_SITEKEY
- 6: https://loginizer.com/docs/configuration-and-settings/setting-up-hcapcha
- 7: https://docs.web3forms.com/getting-started/customizations/spam-protection/hcaptcha
- 8: https://docs.hcaptcha.com/switch/
- 9: https://www.phpbb.com/customise/db/extension/hcaptcha/support/topic/244255
- 10: https://docs.hcaptcha.com/configuration/
hCaptcha Site Key Validation Update
The hCaptcha site keys should follow a UUID format (8-4-4-4-12 hexadecimal digits) and are case-insensitive. To enforce this, please update the
<TextField>component by validating the input against this format. One suggestion is to use the following diff snippet, which also adds the case-insensitive flag (/i) for extra robustness:<TextField hint="TODO Change: Unique identifier for your site" maxLength={36} // UUID-like format placeholder="hCaptcha sitekey" title="hCaptcha sitekey" value={captchaSitekey} + error={captchaSitekey && !/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(captchaSitekey) ? "Please enter a valid hCaptcha site key" : undefined} onChange={handleSitekeyChange} />This change ensures users are prompted with an error message if the provided site key does not match the expected UUID format.
| hint="TODO Change: Unique identifier for your site" | ||
| maxLength={36} // UUIDv4 format |
There was a problem hiding this comment.
Update placeholder TODO hint text
The "TODO Change" in the hint text indicates that the copy needs to be updated before the PR is finalized. Consider providing a more descriptive hint that explains what a site key is and how to obtain it from hCaptcha.
| hint="TODO Change:Private secret key used to verify hCaptcha responses" | ||
| maxLength={100} |
There was a problem hiding this comment.
Fix typo in hint text and update the placeholder
There's a missing space after the colon in "TODO Change:Private", and the hint text needs to be updated. Consider providing clear instructions on how to obtain the secret key from hCaptcha.
- hint="TODO Change:Private secret key used to verify hCaptcha responses"
+ hint="TODO Change: Private secret key used to verify hCaptcha responses"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| hint="TODO Change:Private secret key used to verify hCaptcha responses" | |
| maxLength={100} | |
| hint="TODO Change: Private secret key used to verify hCaptcha responses" | |
| maxLength={100} |
ref BAE-403
Issues with this PR currently:
hostSettings:captchafor siteKey & secretKey needs to be actioned for Ghost (Pro)