feat: add honeypots and (mostly) invisible PoW captchas to highly spam prone forms#3488
feat: add honeypots and (mostly) invisible PoW captchas to highly spam prone forms#3488
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements a comprehensive anti-spam system for PubPub by adding honeypots and proof-of-work (PoW) captchas to highly spam-prone forms. The system uses the Altcha library for invisible PoW challenges and honeypot fields to catch automated bots.
Changes:
- Added Altcha-based proof-of-work captcha system with server-side HMAC verification
- Implemented honeypot fields across user-facing forms (signup, login, community/pub creation, discussions)
- Enhanced spam tag tracking with honeypot triggers, manual markings, and automated status updates
- Added comprehensive Slack and email notifications for spam events (new tags, bans, lifts)
- Created UI components for spam management (SpamStatusMenu, user profile spam indicators)
Reviewed changes
Copilot reviewed 64 out of 68 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| server/utils/captcha.ts | New utility for Altcha PoW captcha verification with HMAC |
| server/utils/honeypot.ts | Honeypot detection and spam tag creation logic |
| server/spamTag/userQueries.ts | Enhanced user spam tag management with notifications and session deletion |
| server/spamTag/userDashboard.ts | New spam user dashboard queries with affiliation tracking |
| server/utils/slack.ts | Expanded Slack notifications for spam events (bans, lifts, new tags) |
| server/utils/email/spam.ts | Email notifications for spam bans and lifts to users and dev team |
| server/community/api.ts | Community creation with captcha/honeypot verification |
| server/pub/api.ts | Pub creation endpoints with spam protection |
| server/user/api.ts | User creation/update with captcha and honeypot checks |
| server/discussion/api.ts | Discussion creation with spam protection |
| server/threadComment/api.ts | Thread comment creation with spam protection |
| server/login/api.ts | Login endpoint with captcha verification |
| server/signup/api.ts | Signup with captcha and honeypot protection |
| server/captcha/api.ts | Challenge generation endpoint for Altcha |
| client/components/Altcha/Altcha.tsx | React component for Altcha widget integration |
| client/components/Honeypot/Honeypot.tsx | Hidden honeypot field component with dev mode visibility |
| client/components/SpamStatusMenu/SpamStatusMenu.tsx | UI for managing spam status of users |
| client/containers/User/UserHeader.tsx | Display spam status badges for super admins |
| types/spam.ts | New type definitions for honeypot triggers and user spam tag fields |
| utils/api/schemas/community.ts | Schema updates for captcha/honeypot fields |
| utils/api/contracts/pub.ts | New createFromForm endpoint contract |
| utils/api/contracts/auth.ts | New loginFromForm endpoint contract |
| package.json | Added altcha and altcha-lib dependencies |
| .test/setup-env.js | Added BYPASS_CAPTCHA=true for tests |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!w) return; | ||
| const handleStateChange = (ev: Event) => { | ||
| const e = ev as CustomEvent<{ payload?: string; state: string }>; | ||
| console.log('state changed', e.detail); |
There was a problem hiding this comment.
The console.log statement should be removed before merging to production. Debug logging statements can add unnecessary noise to production logs and may expose implementation details about the captcha state machine.
| console.log('state changed', e.detail); |
| accentColorLight: inputValues.accentColorLight ?? '#ffffff', | ||
| accentColorDark: inputValues.accentColorDark ?? '#000000', |
There was a problem hiding this comment.
The default values for accentColorLight and accentColorDark (white and black) may not provide good visual contrast or branding. Consider using more distinctive default colors that better represent PubPub's brand, or making these fields required in the schema to force users to make an intentional choice.
| const subdomain = req.body.subdomain ?? 'community'; | ||
| return { | ||
| body: `https://${subdomain}.pubpub.org`, | ||
| status: 201, | ||
| }; |
There was a problem hiding this comment.
When the honeypot is triggered, the API returns a 201 success status with a fake community URL. While this is good security practice (it doesn't reveal to bots that they've been detected), the URL is constructed from user-provided input (req.body.subdomain) which could potentially be used to probe the system. Consider using a fixed fake subdomain or sanitizing this value before returning it.
| environment: | ||
| NODE_ENV: development | ||
| PORT: 3000 | ||
| # disable this |
There was a problem hiding this comment.
The comment "# disable this" is unclear and provides no context about why IS_DUQDUQ should be disabled in development. Consider updating this to explain the rationale, e.g., "# Disable DuqDuq mode in dev to use standard PubPub behavior" or similar.
| # disable this | |
| # Disable DuqDuq mode in dev to use standard PubPub behavior |
| onStateChange={(state) => { | ||
| if (!isLoading) { | ||
| setIsLoading(true); | ||
| } | ||
| // @ts-ignore | ||
| if (state.detail.state === 'verified') { | ||
| setIsLoading(false); | ||
| } | ||
| }} |
There was a problem hiding this comment.
There's a logic issue with the loading state management. Lines 52-54 set isLoading to true whenever onStateChange fires and isLoading is false, but line 21 also sets it to true. This creates a race condition where the loading state might be incorrectly set. The onStateChange handler should only manage loading state for the captcha verification itself, not for the entire form submission. Consider removing the isLoading management from onStateChange or using separate state variables for captcha loading vs. form submission loading.
| router.get('/api/captcha/challenge', async (_req, res) => { | ||
| const hmacKey = getAltchaHmacKey(); | ||
| const challenge = await createChallenge({ | ||
| hmacKey, | ||
| maxNumber: MAX_NUMBER, | ||
| }); | ||
| return res.json(challenge); | ||
| }); |
There was a problem hiding this comment.
The /api/captcha/challenge endpoint has no rate limiting. This could allow an attacker to generate unlimited challenge requests, potentially using up server resources or gathering information about the HMAC key through timing attacks. Consider adding rate limiting (e.g., 10 requests per IP per minute) to prevent abuse.
| const handlePostThreadComment = async (evt: React.FormEvent<HTMLFormElement>) => { | ||
| evt.preventDefault(); | ||
| const formData = new FormData(evt.currentTarget); | ||
| const honeypot = (formData.get('title') as string) ?? ''; | ||
| setIsLoading(true); | ||
| const outputData = await apiFetch('/api/threadComment', { | ||
| method: 'POST', | ||
| body: JSON.stringify({ | ||
| accessHash: locationData.query.access, | ||
| parentId: discussionData.id, | ||
| threadId: discussionData.thread.id, | ||
| pubId: pubData.id, | ||
| communityId: communityData.id, | ||
| content: getJSON(changeObject?.view), | ||
| text: getText(changeObject?.view) || '', | ||
| commentAccessHash: pubData.commentHash, | ||
| commenterName, | ||
| }), | ||
| }); | ||
|
|
||
| updateLocalData('pub', { | ||
| discussions: pubData.discussions.map((disc) => { | ||
| if (disc.thread!.id === outputData.threadId) { | ||
| return { | ||
| ...disc, | ||
| thread: { | ||
| ...disc.thread, | ||
| comments: [...disc.thread!.comments, outputData], | ||
| }, | ||
| }; | ||
| } | ||
| return disc; | ||
| }), | ||
| }); | ||
|
|
||
| if (isPubBottomInput) { | ||
| try { | ||
| const altchaPayload = await altchaRef.current?.verify(); | ||
| const outputData = await apiFetch('/api/threadComment/fromForm', { | ||
| method: 'POST', | ||
| body: JSON.stringify({ | ||
| ...postThreadCommentBody(), | ||
| _honeypot: honeypot, | ||
| altcha: altchaPayload, | ||
| }), | ||
| }); | ||
| updateLocalData('pub', { | ||
| discussions: pubData.discussions.map((disc) => { | ||
| if (disc.thread!.id === outputData.threadId) { | ||
| return { | ||
| ...disc, | ||
| thread: { | ||
| ...disc.thread, | ||
| comments: [...disc.thread!.comments, outputData], | ||
| }, | ||
| }; | ||
| } | ||
| return disc; | ||
| }), | ||
| }); | ||
| } finally { | ||
| setIsLoading(false); | ||
| setEditorKey(Date.now()); | ||
| if (isPubBottomInput) setEditorKey(Date.now()); | ||
| } | ||
| }; | ||
|
|
||
| const handlePostDiscussion = async () => { | ||
| const handlePostDiscussion = async (evt: React.FormEvent<HTMLFormElement>) => { | ||
| evt.preventDefault(); | ||
| const formData = new FormData(evt.currentTarget); | ||
| const honeypot = (formData.get('title') as string) ?? ''; | ||
| setIsLoading(true); | ||
| const initAnchorData = getLocalHighlightText(pubView, discussionData.id); | ||
| const outputData = await apiFetch('/api/discussions', { | ||
| method: 'POST', | ||
| body: JSON.stringify({ | ||
| accessHash: locationData.query.access, | ||
| discussionId: discussionData.id, | ||
| pubId: pubData.id, | ||
| historyKey: historyData.currentKey, | ||
| communityId: communityData.id, | ||
| content: getJSON(changeObject?.view), | ||
| text: getText(changeObject?.view) || '', | ||
| initAnchorData, | ||
| visibilityAccess: pubData.isRelease ? 'public' : 'members', | ||
| commentAccessHash: pubData.commentHash, | ||
| commenterName, | ||
| }), | ||
| }); | ||
| updateLocalData('pub', { | ||
| discussions: [...pubData.discussions, outputData], | ||
| }); | ||
|
|
||
| if (isPubBottomInput) { | ||
| try { | ||
| const altchaPayload = await altchaRef.current?.verify(); | ||
| const outputData = await apiFetch('/api/discussions/fromForm', { | ||
| method: 'POST', | ||
| body: JSON.stringify({ | ||
| ...postDiscussionBody(), | ||
| _honeypot: honeypot, | ||
| altcha: altchaPayload, | ||
| }), | ||
| }); | ||
| updateLocalData('pub', { | ||
| discussions: [...pubData.discussions, outputData], | ||
| }); | ||
| if (!isPubBottomInput) convertLocalHighlightToDiscussion(pubView, outputData.id); | ||
| } finally { | ||
| setIsLoading(false); | ||
| setEditorKey(Date.now()); | ||
| } else { | ||
| convertLocalHighlightToDiscussion(pubView, outputData.id); | ||
| if (isPubBottomInput) setEditorKey(Date.now()); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The error handling is missing in the try-catch blocks for discussion and thread comment posting. If apiFetch throws an error (e.g., from failed captcha verification or honeypot detection), the error will not be caught or displayed to the user. Users will see an indefinite loading state with no feedback.
Consider adding error handling to show users when their submission fails, for example:
} catch (err) {
console.error('Failed to post discussion:', err);
// Show error to user via toast or state update
} finally {
setIsLoading(false);
| body: JSON.stringify({ | ||
| try { | ||
| const altchaPayload = await altchaRef.current?.verify(); | ||
| console.log('altchaPayload', altchaPayload); |
There was a problem hiding this comment.
The console.log statement should be removed before merging to production. Debug logging statements like this can expose sensitive data (the captcha payload) to the browser console and should not be present in production code.
| console.log('altchaPayload', altchaPayload); |
Issue(s) Resolved
Test Plan
Screenshots (if applicable)
Optional
Notes/Context/Gotchas
Supporting Docs