feat: replace reCAPTCHA with Cloudflare Turnstile for human verification#371
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR comprehensively replaces Google reCAPTCHA with Cloudflare Turnstile for human verification across the application. It introduces a new ChangesreCAPTCHA to Turnstile Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/api/routers/mobile/listings.ts (1)
148-156:⚠️ Potential issue | 🟠 Major | ⚡ Quick winForward
humanVerificationTokeninto spam-check instead of discarding it.Line 148 strips the token into
_humanVerificationToken, but Lines 151-156 never pass it tocheckSpamContent. This can break human-verification challenge handling for mobile listing creation.🔧 Proposed fix
- const { humanVerificationToken: _humanVerificationToken, ...payload } = input + const { humanVerificationToken, ...payload } = input const repository = new ListingsRepository(ctx.prisma) await checkSpamContent({ prisma: ctx.prisma, userId: ctx.session.user.id, content: payload.notes || '', entityType: 'listing', + challengeMode: 'challenge', + humanVerificationToken, + headers: ctx.headers, })🤖 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 `@src/server/api/routers/mobile/listings.ts` around lines 148 - 156, The code currently destructures humanVerificationToken into _humanVerificationToken and then calls checkSpamContent without passing it; update the call so the human verification token is forwarded: keep destructuring (humanVerificationToken or _humanVerificationToken) from input and include that token as an argument to checkSpamContent (e.g., pass humanVerificationToken or humanVerificationToken: _humanVerificationToken) so checkSpamContent receives the token along with prisma, userId, content, and entityType; locate this in the mobile listing creation flow around ListingsRepository and the checkSpamContent invocation.
🧹 Nitpick comments (6)
src/schemas/listing.ts (1)
14-14: ⚡ Quick winCentralize
humanVerificationTokenvalidation to prevent schema drift.The same token rule is duplicated across multiple schema files. Extract a shared Zod schema (e.g.,
HumanVerificationTokenSchema) and reuse it to keep validation behavior consistent during future changes.♻️ Proposed refactor
- humanVerificationToken: z.string().max(HUMAN_VERIFICATION_TOKEN_MAX_LENGTH).optional(), + humanVerificationToken: HumanVerificationTokenSchema.optional(),// e.g. in a shared schema module export const HumanVerificationTokenSchema = z .string() .max(HUMAN_VERIFICATION_TOKEN_MAX_LENGTH)Also applies to: 123-123
🤖 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 `@src/schemas/listing.ts` at line 14, The humanVerificationToken rule is duplicated across schemas; add a shared Zod export (e.g., HumanVerificationTokenSchema) that encapsulates z.string().max(HUMAN_VERIFICATION_TOKEN_MAX_LENGTH) and replace inline uses (like the humanVerificationToken entry in the Listing schema) with HumanVerificationTokenSchema.optional() to centralize validation; ensure the new symbol is exported from a shared schema module and imported where needed so all schemas reference the same HumanVerificationTokenSchema.src/schemas/listing.test.ts (1)
247-279: ⚡ Quick winAdd explicit tests for
humanVerificationTokenvalidation.This suite removed old token assertions, but it doesn’t add coverage for the new token field (valid value + over-max-length rejection). Adding those cases will protect the migration contract.
✅ Suggested test additions
+ it('should accept humanVerificationToken within max length', () => { + const validInput = { + listingId: '123e4567-e89b-12d3-a456-426614174000', + content: 'This is a comment', + humanVerificationToken: 'token-value', + } + expect(CreateCommentSchema.safeParse(validInput).success).toBe(true) + }) + + it('should reject humanVerificationToken above max length', () => { + const invalidInput = { + listingId: '123e4567-e89b-12d3-a456-426614174000', + content: 'This is a comment', + humanVerificationToken: 'a'.repeat(/* HUMAN_VERIFICATION_TOKEN_MAX_LENGTH + 1 */), + } + expect(CreateCommentSchema.safeParse(invalidInput).success).toBe(false) + })🤖 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 `@src/schemas/listing.test.ts` around lines 247 - 279, Add explicit tests for the new humanVerificationToken field in the CreateCommentSchema test suite: add one test that includes a valid humanVerificationToken value (within allowed length/format) alongside listingId and content and assert CreateCommentSchema.safeParse(...) returns success, and add another test that provides a humanVerificationToken exceeding the maximum allowed length and assert safeParse returns failure; reference CreateCommentSchema and place tests inside the existing describe('CreateCommentSchema', ...) block so they run with the other comment validations.src/server/api/trpc.ts (1)
149-160: ⚡ Quick winReuse
createInnerTRPCContextfor App Router context construction.This currently duplicates context assembly logic. Reusing the shared helper reduces drift risk if context fields evolve.
♻️ Proposed refactor
export const createAppRouterTRPCContext = async (opts?: FetchCreateContextFnOptions) => { const { userId } = await auth() let session: Nullable<Session> = null if (userId) session = await createSessionFromClerkUserId(userId) - return { - session, - prisma, - headers: opts?.req.headers ?? new Headers(), - } + return createInnerTRPCContext({ + session, + headers: opts?.req.headers ?? new Headers(), + }) }🤖 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 `@src/server/api/trpc.ts` around lines 149 - 160, The function createAppRouterTRPCContext duplicates context assembly; replace its manual construction with a call to the shared helper createInnerTRPCContext to avoid drift. Update createAppRouterTRPCContext to await and return createInnerTRPCContext(opts) (or call it with the same FetchCreateContextFnOptions it receives) so session, prisma, headers, and any future fields are built in one place; ensure you remove the duplicated auth/createSessionFromClerkUserId logic here and rely on createInnerTRPCContext instead.src/server/utils/spam-check.ts (2)
10-10: 💤 Low valueFix import order per ESLint rule.
@orm/clientshould be imported before@/features/...imports.🤖 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 `@src/server/utils/spam-check.ts` at line 10, Move the external package import for PrismaClient (import { type PrismaClient } from '`@orm/client`') above any local feature imports (anything under '`@/features/`...') in src/server/utils/spam-check.ts so the import order satisfies ESLint: ensure the '`@orm/client`' import appears before the '`@/features/`...' imports (keep the same named import symbol PrismaClient).
64-77: ⚡ Quick winAppError.humanVerification already throw (
never), so thereturn AppError.*pattern won’t bypass verification.*
AppError.humanVerificationRequired(),AppError.humanVerificationFailed(), andAppError.humanVerificationUnavailable()are typed to returnneverand each implementation immediatelythrow new TRPCError(...)(seesrc/lib/errors.tsaround lines 182–208). Therefore,enforceHumanVerificationwill not “silently pass” if a token is missing/invalid—those code paths always terminate.Optional readability: you could switch to
throw AppError.*instead ofreturn AppError.*, but it’s not functionally required.🤖 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 `@src/server/utils/spam-check.ts` around lines 64 - 77, The current checks call AppError.humanVerificationRequired/Failed/Unavailable using "return", but those functions are typed to throw (never), so change each "return AppError.humanVerificationRequired()", "return AppError.humanVerificationFailed()", and "return AppError.humanVerificationUnavailable()" to "throw AppError.humanVerificationRequired()", "throw AppError.humanVerificationFailed()", and "throw AppError.humanVerificationUnavailable()" respectively (or alternatively invoke them without returning) inside the enforceHumanVerification flow so the intent is explicit and the control flow is correct around the token, token length, and isTurnstileConfigured checks and before verifyTurnstileToken.src/features/human-verification/server/providers/turnstile.ts (1)
46-52: ⚡ Quick winConsider adding a timeout to the Turnstile verification request.
The
fetchcall lacks a timeout. If Cloudflare's siteverify endpoint is slow or unresponsive, requests could hang indefinitely, impacting user experience.⏱️ Proposed fix with AbortSignal timeout
const response = await fetch(TURNSTILE_SITEVERIFY_URL, { method: 'POST', body, headers: { 'content-type': 'application/x-www-form-urlencoded', }, + signal: AbortSignal.timeout(5000), })You may also want to catch
AbortErrorand return a specific error code like'siteverify-timeout'.🤖 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 `@src/features/human-verification/server/providers/turnstile.ts` around lines 46 - 52, Add a timeout to the Turnstile verification fetch by using an AbortController and setTimeout around the request to abort after a reasonable interval; update the code that calls fetch(TURNSTILE_SITEVERIFY_URL, ...) (and any enclosing function handling verification) to create an AbortController, pass its signal into fetch, clear the timer on successful response, and abort on timeout. Also catch the abort case (check for an AbortError) and return or throw the specific error code 'siteverify-timeout' so callers can distinguish timeout failures from other errors. Ensure any created timers are cleaned up to avoid leaks.
🤖 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 `@src/features/human-verification/client/HumanVerificationProvider.tsx`:
- Around line 63-77: The requestVerification implementation overwrites
pendingRequestRef.current and setPendingRequest with a new in-flight request,
which can leave the original caller's Promise unresolved; before creating a new
Promise in requestVerification, check pendingRequestRef.current (and/or
pendingRequest state) and if a pending request exists, immediately return
Promise.reject(new Error('Verification already in progress.')) (or otherwise
reject/resolve the existing pending request) to avoid leaving callers hanging;
update the logic in requestVerification to perform this guard and only set
pendingRequestRef.current and call setPendingRequest when there is no existing
pending request.
In `@src/server/api/routers/mobile/pcListings.ts`:
- Around line 157-162: The mobile POST handler calls checkSpamContent without
passing challengeMode, humanVerificationToken, or headers, so flagged spam will
be blocked with no recovery option; update the call in pcListings.ts to forward
challengeMode, humanVerificationToken (from input), and headers (from
ctx.req.headers) to checkSpamContent, and update CreatePcListingSchema to
include humanVerificationToken as an optional string so the mobile endpoint can
pass it through like the web routers (see comments.ts/core.ts for the same
parameter names and usage).
---
Outside diff comments:
In `@src/server/api/routers/mobile/listings.ts`:
- Around line 148-156: The code currently destructures humanVerificationToken
into _humanVerificationToken and then calls checkSpamContent without passing it;
update the call so the human verification token is forwarded: keep destructuring
(humanVerificationToken or _humanVerificationToken) from input and include that
token as an argument to checkSpamContent (e.g., pass humanVerificationToken or
humanVerificationToken: _humanVerificationToken) so checkSpamContent receives
the token along with prisma, userId, content, and entityType; locate this in the
mobile listing creation flow around ListingsRepository and the checkSpamContent
invocation.
---
Nitpick comments:
In `@src/features/human-verification/server/providers/turnstile.ts`:
- Around line 46-52: Add a timeout to the Turnstile verification fetch by using
an AbortController and setTimeout around the request to abort after a reasonable
interval; update the code that calls fetch(TURNSTILE_SITEVERIFY_URL, ...) (and
any enclosing function handling verification) to create an AbortController, pass
its signal into fetch, clear the timer on successful response, and abort on
timeout. Also catch the abort case (check for an AbortError) and return or throw
the specific error code 'siteverify-timeout' so callers can distinguish timeout
failures from other errors. Ensure any created timers are cleaned up to avoid
leaks.
In `@src/schemas/listing.test.ts`:
- Around line 247-279: Add explicit tests for the new humanVerificationToken
field in the CreateCommentSchema test suite: add one test that includes a valid
humanVerificationToken value (within allowed length/format) alongside listingId
and content and assert CreateCommentSchema.safeParse(...) returns success, and
add another test that provides a humanVerificationToken exceeding the maximum
allowed length and assert safeParse returns failure; reference
CreateCommentSchema and place tests inside the existing
describe('CreateCommentSchema', ...) block so they run with the other comment
validations.
In `@src/schemas/listing.ts`:
- Line 14: The humanVerificationToken rule is duplicated across schemas; add a
shared Zod export (e.g., HumanVerificationTokenSchema) that encapsulates
z.string().max(HUMAN_VERIFICATION_TOKEN_MAX_LENGTH) and replace inline uses
(like the humanVerificationToken entry in the Listing schema) with
HumanVerificationTokenSchema.optional() to centralize validation; ensure the new
symbol is exported from a shared schema module and imported where needed so all
schemas reference the same HumanVerificationTokenSchema.
In `@src/server/api/trpc.ts`:
- Around line 149-160: The function createAppRouterTRPCContext duplicates
context assembly; replace its manual construction with a call to the shared
helper createInnerTRPCContext to avoid drift. Update createAppRouterTRPCContext
to await and return createInnerTRPCContext(opts) (or call it with the same
FetchCreateContextFnOptions it receives) so session, prisma, headers, and any
future fields are built in one place; ensure you remove the duplicated
auth/createSessionFromClerkUserId logic here and rely on createInnerTRPCContext
instead.
In `@src/server/utils/spam-check.ts`:
- Line 10: Move the external package import for PrismaClient (import { type
PrismaClient } from '`@orm/client`') above any local feature imports (anything
under '`@/features/`...') in src/server/utils/spam-check.ts so the import order
satisfies ESLint: ensure the '`@orm/client`' import appears before the
'`@/features/`...' imports (keep the same named import symbol PrismaClient).
- Around line 64-77: The current checks call
AppError.humanVerificationRequired/Failed/Unavailable using "return", but those
functions are typed to throw (never), so change each "return
AppError.humanVerificationRequired()", "return
AppError.humanVerificationFailed()", and "return
AppError.humanVerificationUnavailable()" to "throw
AppError.humanVerificationRequired()", "throw
AppError.humanVerificationFailed()", and "throw
AppError.humanVerificationUnavailable()" respectively (or alternatively invoke
them without returning) inside the enforceHumanVerification flow so the intent
is explicit and the control flow is correct around the token, token length, and
isTurnstileConfigured checks and before verifyTurnstileToken.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 458f628c-209b-4c63-81fe-731fa1633673
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (52)
.env.docker.example.env.example.env.test.exampledocs/DEVELOPMENT_SETUP.mdeslint.config.mjsnext.config.tspackage.jsonsrc/app/listings/[id]/components/CommentForm.tsxsrc/app/listings/[id]/components/CommentThread.tsxsrc/app/listings/[id]/components/VoteButtons.tsxsrc/app/listings/components/shared/details/utils/logVoteError.test.tssrc/app/listings/new/NewListingPage.tsxsrc/app/pc-listings/[id]/components/PcCommentForm.tsxsrc/app/pc-listings/[id]/components/PcCommentThread.tsxsrc/app/pc-listings/[id]/components/PcVoteButtons.tsxsrc/app/pc-listings/new/NewPcListingPage.tsxsrc/components/Providers.tsxsrc/components/comments/GenericCommentForm.tsxsrc/components/comments/GenericCommentThread.tsxsrc/features/human-verification/client/HumanVerificationProvider.tsxsrc/features/human-verification/client/getHumanVerificationRequest.test.tssrc/features/human-verification/client/getHumanVerificationRequest.tssrc/features/human-verification/client/index.tssrc/features/human-verification/client/useSubmitWithHumanVerification.tssrc/features/human-verification/server/providers/turnstile.test.tssrc/features/human-verification/server/providers/turnstile.tssrc/features/human-verification/shared/constants.tssrc/lib/app-error-cause.tssrc/lib/captcha/config.tssrc/lib/captcha/hooks.tssrc/lib/captcha/verify.test.tssrc/lib/captcha/verify.tssrc/lib/errors.tssrc/lib/trpc-client-errors.test.tssrc/schemas/listing.test.tssrc/schemas/listing.tssrc/schemas/mobile.tssrc/schemas/pcListing.tssrc/server/api/mobileContext.tssrc/server/api/routers/listings/comments.test.tssrc/server/api/routers/listings/comments.tssrc/server/api/routers/listings/core.test.tssrc/server/api/routers/listings/core.tssrc/server/api/routers/mobile/listings.tssrc/server/api/routers/mobile/pcListings.tssrc/server/api/routers/pcListings.test.tssrc/server/api/routers/pcListings.tssrc/server/api/trpc.tssrc/server/utils/spam-check.test.tssrc/server/utils/spam-check.tssrc/server/utils/spamDetection.test.tssrc/server/utils/spamDetection.ts
💤 Files with no reviewable changes (9)
- src/components/comments/GenericCommentThread.tsx
- src/lib/captcha/verify.ts
- src/lib/captcha/config.ts
- src/app/listings/[id]/components/VoteButtons.tsx
- src/lib/captcha/verify.test.ts
- src/lib/captcha/hooks.ts
- src/app/pc-listings/[id]/components/PcVoteButtons.tsx
- package.json
- next.config.ts
Comment on lines
+157
to
+162
| await checkSpamContent({ | ||
| prisma: ctx.prisma, | ||
| userId: ctx.session.user.id, | ||
| content: input.notes ?? '', | ||
| entityType: 'pcListing', | ||
| }) |
There was a problem hiding this comment.
Mobile endpoint lacks challenge mode — spam detection will block without recovery option.
Unlike the web routers (comments.ts, core.ts), this mobile endpoint doesn't pass challengeMode, humanVerificationToken, or headers. Flagged spam will be blocked outright with no challenge opportunity.
If this is intentional (mobile app doesn't support Turnstile), consider documenting this. Otherwise, add the params for parity:
🔧 Suggested fix for challenge mode support
await checkSpamContent({
prisma: ctx.prisma,
userId: ctx.session.user.id,
content: input.notes ?? '',
entityType: 'pcListing',
+ challengeMode: 'challenge',
+ humanVerificationToken: input.humanVerificationToken,
+ headers: ctx.headers,
})This also requires updating CreatePcListingSchema to accept humanVerificationToken.
🤖 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 `@src/server/api/routers/mobile/pcListings.ts` around lines 157 - 162, The
mobile POST handler calls checkSpamContent without passing challengeMode,
humanVerificationToken, or headers, so flagged spam will be blocked with no
recovery option; update the call in pcListings.ts to forward challengeMode,
humanVerificationToken (from input), and headers (from ctx.req.headers) to
checkSpamContent, and update CreatePcListingSchema to include
humanVerificationToken as an optional string so the mobile endpoint can pass it
through like the web routers (see comments.ts/core.ts for the same parameter
names and usage).
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/features/human-verification/client/components/HumanVerificationProvider.tsx (1)
55-93:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject/cleanup pending verification on provider unmount.
If the provider unmounts while a challenge is pending, the caller promise can hang indefinitely because no reject path runs on unmount. Add unmount cleanup to reject the pending request and remove any mounted widget.
🛠️ Proposed fix
const closeDialog = useCallback(() => { @@ setPendingRequest(null) }, []) + + useEffect(() => { + return () => { + if (pendingRequestRef.current) { + pendingRequestRef.current.reject( + new Error('Human verification was interrupted. Please try again.'), + ) + pendingRequestRef.current = null + } + + if (widgetIdRef.current && window.turnstile) { + window.turnstile.remove(widgetIdRef.current) + widgetIdRef.current = null + } + } + }, [])Also applies to: 95-130
🤖 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 `@src/features/human-verification/client/components/HumanVerificationProvider.tsx` around lines 55 - 93, Add an unmount cleanup in HumanVerificationProvider that rejects any pending verification promise and removes the mounted widget: in a useEffect with no deps add a cleanup function that if pendingRequestRef.current exists calls its reject(new Error('Human verification aborted due to unmount')), clears pendingRequestRef.current and setPendingRequest(null), and if widgetIdRef.current and window.turnstile exist calls window.turnstile.remove(widgetIdRef.current) and nulls widgetIdRef.current (you can reuse/extend closeDialog logic to ensure both the widget is removed and the pending request is rejected on unmount).src/features/human-verification/server/providers/turnstile.ts (1)
66-67:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle non-JSON/invalid JSON
siteverifybodies without throwing.
response.json()can throw and currently bypasses your structuredTurnstileVerificationResultflow. Convert parse failures intoinvalid-siteverify-response.🛡️ Suggested fix
- const parsed = TurnstileResponseSchema.safeParse(await response.json()) + let siteverifyPayload: unknown + try { + siteverifyPayload = await response.json() + } catch { + return { success: false, errorCodes: ['invalid-siteverify-response'] } + } + + const parsed = TurnstileResponseSchema.safeParse(siteverifyPayload) if (!parsed.success) return { success: false, errorCodes: ['invalid-siteverify-response'] }🤖 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 `@src/features/human-verification/server/providers/turnstile.ts` around lines 66 - 67, Wrap the call to response.json() in a try/catch and treat any JSON parse failure as an invalid siteverify response: try to read the body (e.g. const body = await response.json()), catch any error and immediately return { success: false, errorCodes: ['invalid-siteverify-response'] }, then run TurnstileResponseSchema.safeParse(body) as before (the existing parsed handling and return shape/TurnstileVerificationResult should remain unchanged).
🧹 Nitpick comments (1)
src/scripts/api/generate-api-docs.ts (1)
1245-1248: ⚡ Quick winRestore an entrypoint guard to avoid import-time side effects.
Line 1245 runs the script on every import, which can unexpectedly write files or call
process.exit(1)in tooling/tests that import this module. Keep execution gated to direct CLI invocation.💡 Proposed fix
-main().catch((err) => { - console.error('Script failed:', err) - process.exit(1) -}) +function run() { + return main().catch((err) => { + console.error('Script failed:', err) + process.exit(1) + }) +} + +if (import.meta.url === `file://${process.argv[1]}`) { + void run() +}🤖 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 `@src/scripts/api/generate-api-docs.ts` around lines 1245 - 1248, The script currently invokes main() at import time which can cause side effects; wrap the call and error handler in a CLI-entry guard so main() only runs when the file is executed directly (e.g., check require.main === module for CommonJS or import.meta.main for ESM) and keep the existing error handling (the console.error and process.exit(1)) inside that guarded block; update the invocation around main() to use that guard so importing the module does not trigger execution.
🤖 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 `@public/api-docs/mobile-openapi.json`:
- Line 12316: Replace the generic summary fields like "get - preferences" with
concise, human-friendly descriptions (e.g., "Get user preferences") to improve
API docs readability; search for the summary key values at the listed locations
(the exact summary strings found in the diff) and update each to a clear,
descriptive phrase that reflects the endpoint action and resource (use the
corresponding operationId or path to craft the text), ensuring all occurrences
at the indicated locations are changed consistently.
- Around line 762-769: Update all $ref targets that currently use
"`#/definitions/`..." to use the OpenAPI 3.0 correct path
"`#/components/schemas/`..." so they resolve; specifically change references
inside the CreateListingSchema (e.g., customFieldValues -> anyOf -> 0 -> items
-> properties -> value and the additionalProperties entry) and any other
occurrences (there are 8 total) to point to "`#/components/schemas/`..." instead
of "`#/definitions/`...".
In `@src/schemas/mobile.ts`:
- Line 151: The mobile create alias CreateListingSchema currently re-exports
CreateListingBaseSchema but drops the humanVerificationToken field; update the
alias so mobile create flows can pass Turnstile tokens by extending or
rebuilding CreateListingSchema from CreateListingBaseSchema to include
humanVerificationToken (e.g., add a humanVerificationToken optional string
field) and apply the same change to the other mobile create alias referenced at
the other location so both mobile create schemas include humanVerificationToken.
In `@src/schemas/pcListing.ts`:
- Around line 3-7: The import/order lint failure is due to the '`@orm`' import
being placed after other external imports; move the import that brings in
ApprovalStatus, PcOs, ReportReason, ReportStatus (the import from '`@orm`') so it
appears before external imports like zod in the top import block of
src/schemas/pcListing.ts; reorder the import statements so the '`@orm`' import
precedes zod and other external modules (keeping existing named symbols and
formatting intact) to satisfy the rule.
In `@src/server/api/routers/mobile/listings.ts`:
- Around line 150-155: The create listing call to checkSpamContent is missing
the Forward Turnstile challenge context; update the call in the create handler
to pass challengeMode, humanVerificationToken from the input (e.g.,
input.challengeMode, input.humanVerificationToken) and the request headers
(ctx.req.headers) along with the existing prisma, userId, content, and
entityType so checkSpamContent receives the full verification context; locate
the call to checkSpamContent in the create path of the listings router and add
these three parameters to the argument object.
---
Outside diff comments:
In
`@src/features/human-verification/client/components/HumanVerificationProvider.tsx`:
- Around line 55-93: Add an unmount cleanup in HumanVerificationProvider that
rejects any pending verification promise and removes the mounted widget: in a
useEffect with no deps add a cleanup function that if pendingRequestRef.current
exists calls its reject(new Error('Human verification aborted due to unmount')),
clears pendingRequestRef.current and setPendingRequest(null), and if
widgetIdRef.current and window.turnstile exist calls
window.turnstile.remove(widgetIdRef.current) and nulls widgetIdRef.current (you
can reuse/extend closeDialog logic to ensure both the widget is removed and the
pending request is rejected on unmount).
In `@src/features/human-verification/server/providers/turnstile.ts`:
- Around line 66-67: Wrap the call to response.json() in a try/catch and treat
any JSON parse failure as an invalid siteverify response: try to read the body
(e.g. const body = await response.json()), catch any error and immediately
return { success: false, errorCodes: ['invalid-siteverify-response'] }, then run
TurnstileResponseSchema.safeParse(body) as before (the existing parsed handling
and return shape/TurnstileVerificationResult should remain unchanged).
---
Nitpick comments:
In `@src/scripts/api/generate-api-docs.ts`:
- Around line 1245-1248: The script currently invokes main() at import time
which can cause side effects; wrap the call and error handler in a CLI-entry
guard so main() only runs when the file is executed directly (e.g., check
require.main === module for CommonJS or import.meta.main for ESM) and keep the
existing error handling (the console.error and process.exit(1)) inside that
guarded block; update the invocation around main() to use that guard so
importing the module does not trigger execution.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 51a528a3-3678-44c6-93c9-1612a85b8947
📒 Files selected for processing (26)
docs/MOBILE_API.mdpublic/api-docs/mobile-openapi.jsonsrc/features/human-verification/client/components/HumanVerificationProvider.test.tsxsrc/features/human-verification/client/components/HumanVerificationProvider.tsxsrc/features/human-verification/client/hooks/useSubmitWithHumanVerification.tssrc/features/human-verification/client/index.tssrc/features/human-verification/client/utils/getHumanVerificationRequest.test.tssrc/features/human-verification/client/utils/getHumanVerificationRequest.tssrc/features/human-verification/server/providers/turnstile.test.tssrc/features/human-verification/server/providers/turnstile.tssrc/features/human-verification/shared/schema.tssrc/lib/env.tssrc/lib/errors.tssrc/lib/trust/service.tssrc/schemas/listing.test.tssrc/schemas/listing.tssrc/schemas/listingCreate.tssrc/schemas/mobile.tssrc/schemas/pcListing.tssrc/scripts/api/generate-api-docs.tssrc/server/api/routers/mobile/listings.tssrc/server/api/routers/mobile/pcListings.tssrc/server/api/routers/trust.tssrc/server/api/trpc.tssrc/server/utils/spam-check.test.tssrc/server/utils/spam-check.ts
✅ Files skipped from review due to trivial changes (5)
- src/lib/trust/service.ts
- src/features/human-verification/client/utils/getHumanVerificationRequest.ts
- src/features/human-verification/shared/schema.ts
- docs/MOBILE_API.md
- src/server/api/routers/trust.ts
Comment on lines
+762
to
+769
| "$ref": "#/definitions/CreateListingSchema/properties/customFieldValues/anyOf/0/items/properties/value" | ||
| } | ||
| }, | ||
| "value": { | ||
| "type": "number" | ||
| { | ||
| "type": "object", | ||
| "additionalProperties": { | ||
| "$ref": "#/definitions/CreateListingSchema/properties/customFieldValues/anyOf/0/items/properties/value" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json
path = "public/api-docs/mobile-openapi.json"
doc = json.load(open(path))
refs = []
def walk(node, p="$"):
if isinstance(node, dict):
for k, v in node.items():
np = f"{p}.{k}"
if k == "$ref" and isinstance(v, str):
refs.append((np, v))
walk(v, np)
elif isinstance(node, list):
for i, v in enumerate(node):
walk(v, f"{p}[{i}]")
walk(doc)
bad = [(p, r) for p, r in refs if r.startswith("`#/definitions/`")]
print(f"OpenAPI version: {doc.get('openapi')}")
print(f"Root has 'definitions': {'definitions' in doc}")
print(f"Refs using `#/definitions/`: {len(bad)}")
for p, r in bad:
print(f"{p} -> {r}")
PYRepository: Producdevity/EmuReady
Length of output: 1941
Fix OpenAPI 3 $ref targets using #/definitions (update to #/components/schemas)
public/api-docs/mobile-openapi.jsonis OpenAPI3.0.0and has no rootdefinitions.- 8
$refvalues still point to#/definitions/...(includingcustomFieldValuesvalueandadditionalProperties), so validators/codegen won’t resolve them.
♻️ Proposed fix pattern
- "$ref": "`#/definitions/CreateListingSchema/properties/customFieldValues/anyOf/0/items/properties/value`"
+ "$ref": "`#/components/schemas/CreateListingSchema/properties/customFieldValues/anyOf/0/items/properties/value`"
- "$ref": "`#/definitions/UpdateListingSchema/properties/customFieldValues/items/anyOf/1/properties/value`"
+ "$ref": "`#/components/schemas/UpdateListingSchema/properties/customFieldValues/items/anyOf/1/properties/value`"
- "$ref": "`#/definitions/CreatePcListingSchema/properties/customFieldValues/items/properties/value`"
+ "$ref": "`#/components/schemas/CreatePcListingSchema/properties/customFieldValues/items/properties/value`"
- "$ref": "`#/definitions/UpdatePcListingSchema/properties/customFieldValues/items/anyOf/1/properties/value`"
+ "$ref": "`#/components/schemas/UpdatePcListingSchema/properties/customFieldValues/items/anyOf/1/properties/value`"📝 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.
Suggested change
| "$ref": "#/definitions/CreateListingSchema/properties/customFieldValues/anyOf/0/items/properties/value" | |
| } | |
| }, | |
| "value": { | |
| "type": "number" | |
| { | |
| "type": "object", | |
| "additionalProperties": { | |
| "$ref": "#/definitions/CreateListingSchema/properties/customFieldValues/anyOf/0/items/properties/value" | |
| } | |
| "$ref": "`#/components/schemas/CreateListingSchema/properties/customFieldValues/anyOf/0/items/properties/value`" | |
| } | |
| }, | |
| { | |
| "type": "object", | |
| "additionalProperties": { | |
| "$ref": "`#/components/schemas/CreateListingSchema/properties/customFieldValues/anyOf/0/items/properties/value`" | |
| } |
🤖 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 `@public/api-docs/mobile-openapi.json` around lines 762 - 769, Update all $ref
targets that currently use "`#/definitions/`..." to use the OpenAPI 3.0 correct
path "`#/components/schemas/`..." so they resolve; specifically change references
inside the CreateListingSchema (e.g., customFieldValues -> anyOf -> 0 -> items
-> properties -> value and the additionalProperties entry) and any other
occurrences (there are 8 total) to point to "`#/components/schemas/`..." instead
of "`#/definitions/`...".
| "get": { | ||
| "summary": "Get user preferences", | ||
| "description": "Get user preferences", | ||
| "summary": "get - preferences", |
There was a problem hiding this comment.
Preference endpoint summaries lost descriptive value.
These summary strings are now generic ("get - preferences", etc.), which makes generated API docs harder to scan.
Also applies to: 12447-12447, 12590-12590, 12735-12735, 12880-12880, 13028-13028, 13176-13176, 13307-13307, 13449-13449
🤖 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 `@public/api-docs/mobile-openapi.json` at line 12316, Replace the generic
summary fields like "get - preferences" with concise, human-friendly
descriptions (e.g., "Get user preferences") to improve API docs readability;
search for the summary key values at the listed locations (the exact summary
strings found in the diff) and update each to a clear, descriptive phrase that
reflects the endpoint action and resource (use the corresponding operationId or
path to craft the text), ensuring all occurrences at the indicated locations are
changed consistently.
| .array(z.union([CustomFieldValueSchema, SimplifiedCustomFieldValueSchema])) | ||
| .optional(), | ||
| }) | ||
| export const CreateListingSchema = CreateListingBaseSchema |
There was a problem hiding this comment.
Add humanVerificationToken to mobile create schemas.
These aliases drop the verification token field, so mobile create flows can’t pass Turnstile tokens to spam-check challenge paths.
Suggested fix
import { z } from 'zod'
import { JsonValueSchema } from '`@/schemas/common`'
import { CreateListingBaseSchema, CreatePcListingBaseSchema } from '`@/schemas/listingCreate`'
+import { HumanVerificationTokenSchema } from '`@/features/human-verification/shared/schema`'
...
-export const CreateListingSchema = CreateListingBaseSchema
+export const CreateListingSchema = CreateListingBaseSchema.extend({
+ humanVerificationToken: HumanVerificationTokenSchema.optional(),
+})
...
-export const CreatePcListingSchema = CreatePcListingBaseSchema
+export const CreatePcListingSchema = CreatePcListingBaseSchema.extend({
+ humanVerificationToken: HumanVerificationTokenSchema.optional(),
+})Also applies to: 385-385
🤖 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 `@src/schemas/mobile.ts` at line 151, The mobile create alias
CreateListingSchema currently re-exports CreateListingBaseSchema but drops the
humanVerificationToken field; update the alias so mobile create flows can pass
Turnstile tokens by extending or rebuilding CreateListingSchema from
CreateListingBaseSchema to include humanVerificationToken (e.g., add a
humanVerificationToken optional string field) and apply the same change to the
other mobile create alias referenced at the other location so both mobile create
schemas include humanVerificationToken.
Comment on lines
+3
to
7
| import { HumanVerificationTokenSchema } from '@/features/human-verification/shared/schema' | ||
| import { JsonValueSchema } from '@/schemas/common' | ||
| import { CreatePcListingBaseSchema } from '@/schemas/listingCreate' | ||
| import { REVIEW_RISK_FILTERS, ReviewRiskFilterSchema } from '@/schemas/submissionRisk' | ||
| import { ApprovalStatus, PcOs, ReportReason, ReportStatus } from '@orm' |
There was a problem hiding this comment.
Resolve the import/order violation in the import block.
@orm should be ordered before zod in the external-import group to satisfy the lint rule consistently.
🔧 Suggested import reorder
-import { z } from 'zod'
+import { ApprovalStatus, PcOs, ReportReason, ReportStatus } from '`@orm`'
+import { z } from 'zod'
import { PAGINATION, CHAR_LIMITS } from '`@/data/constants`'
import { HumanVerificationTokenSchema } from '`@/features/human-verification/shared/schema`'
import { JsonValueSchema } from '`@/schemas/common`'
import { CreatePcListingBaseSchema } from '`@/schemas/listingCreate`'
import { REVIEW_RISK_FILTERS, ReviewRiskFilterSchema } from '`@/schemas/submissionRisk`'
-import { ApprovalStatus, PcOs, ReportReason, ReportStatus } from '`@orm`'🧰 Tools
🪛 ESLint
[error] 7-7: @orm import should occur before import of zod
(import/order)
🤖 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 `@src/schemas/pcListing.ts` around lines 3 - 7, The import/order lint failure
is due to the '`@orm`' import being placed after other external imports; move the
import that brings in ApprovalStatus, PcOs, ReportReason, ReportStatus (the
import from '`@orm`') so it appears before external imports like zod in the top
import block of src/schemas/pcListing.ts; reorder the import statements so the
'`@orm`' import precedes zod and other external modules (keeping existing named
symbols and formatting intact) to satisfy the rule.
Comment on lines
150
to
155
| await checkSpamContent({ | ||
| prisma: ctx.prisma, | ||
| userId: ctx.session.user.id, | ||
| content: payload.notes || '', | ||
| content: input.notes || '', | ||
| entityType: 'listing', | ||
| }) |
There was a problem hiding this comment.
Forward Turnstile challenge context in listing creation spam checks.
The create path still runs in block-only mode because it doesn’t pass challengeMode, humanVerificationToken, and request headers to checkSpamContent.
Suggested fix
await checkSpamContent({
prisma: ctx.prisma,
userId: ctx.session.user.id,
content: input.notes || '',
entityType: 'listing',
+ challengeMode: 'challenge',
+ humanVerificationToken: input.humanVerificationToken,
+ headers: ctx.headers,
})📝 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.
Suggested change
| await checkSpamContent({ | |
| prisma: ctx.prisma, | |
| userId: ctx.session.user.id, | |
| content: payload.notes || '', | |
| content: input.notes || '', | |
| entityType: 'listing', | |
| }) | |
| await checkSpamContent({ | |
| prisma: ctx.prisma, | |
| userId: ctx.session.user.id, | |
| content: input.notes || '', | |
| entityType: 'listing', | |
| challengeMode: 'challenge', | |
| humanVerificationToken: input.humanVerificationToken, | |
| headers: ctx.headers, | |
| }) |
🤖 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 `@src/server/api/routers/mobile/listings.ts` around lines 150 - 155, The create
listing call to checkSpamContent is missing the Forward Turnstile challenge
context; update the call in the create handler to pass challengeMode,
humanVerificationToken from the input (e.g., input.challengeMode,
input.humanVerificationToken) and the request headers (ctx.req.headers) along
with the existing prisma, userId, content, and entityType so checkSpamContent
receives the full verification context; locate the call to checkSpamContent in
the create path of the listings router and add these three parameters to the
argument object.
This was referenced May 26, 2026
Merged
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
1 participant
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.
Description
Replaces the old reCAPTCHA flow with Cloudflare Turnstile for human verification.
This adds the shared human-verification feature, wires Turnstile challenges into listing and comment submissions, updates spam checks to support the new
humanVerificationToken, and removes the reCAPTCHA client dependency/config. It also updates the example env files, CSP setup, and development docs for the new Turnstile keys.Fixes the current submission verification failures caused by the reCAPTCHA flow.
Type of change
How Has This Been Tested?
Screenshots (if applicable)
N/A
Checklist
Notes for reviewers
Production needs
NEXT_PUBLIC_TURNSTILE_SITE_KEYandTURNSTILE_SECRET_KEYset before this is deployed.The e2e check is currently failing on this PR and should be reviewed before merge.
Summary by CodeRabbit
New Features
Refactor
Chores