Draft
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
vercel
bot
temporarily deployed
to
Preview – better-captcha-better-auth-example
vercel
bot
temporarily deployed
to
Preview – better-captcha-better-auth-example
vercel
bot
temporarily deployed
to
Preview – better-captcha-better-auth-example
vercel
bot
temporarily deployed
to
Preview – better-captcha-better-auth-example
vercel
bot
temporarily deployed
to
Preview – better-captcha-better-auth-example
vercel
bot
temporarily deployed
to
Preview – better-captcha-better-auth-example
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
packages/server/README.md (1)
5-6: Minor grammar fix: Use hyphenated "production-ready".📝 Suggested fix
-> [!WARNING] -> This library is in early development and is not production ready yet. Expect breaking API changes while the provider surface stabilises. +> [!WARNING] +> This library is in early development and is not production-ready yet. Expect breaking API changes while the provider surface stabilises.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/README.md` around lines 5 - 6, Update the warning text in the README: change the phrase "production ready" to the hyphenated form "production-ready" inside the warning block (the line beginning with "[!WARNING]" that contains "This library is in early development and is not production ready yet."). Keep the rest of the sentence unchanged.apps/docs/content/docs/en/server.mdx (1)
21-33: Harden the quick-start handler for invalid JSON and missing token.Right now malformed JSON or absent
captchaTokencan surface as avoidable runtime errors in a copy-pasted implementation. Add lightweight guards in the example.💡 Suggested doc-safe handler pattern
export async function POST(request: Request) { - const body = await request.json(); - const token = body.captchaToken; + let body: unknown; + try { + body = await request.json(); + } catch { + return Response.json({ ok: false, error: "invalid_json" }, { status: 400 }); + } + + const token = + typeof body === "object" && body !== null && "captchaToken" in body + ? (body as { captchaToken?: unknown }).captchaToken + : undefined; + + if (typeof token !== "string" || token.length === 0) { + return Response.json({ ok: false, error: "missing_captcha_token" }, { status: 400 }); + } const result = await verifyToken("turnstile", { secret: process.env.TURNSTILE_SECRET_KEY!, response: token, expectedHostname: "example.com", });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/content/docs/en/server.mdx` around lines 21 - 33, The POST handler should be hardened against malformed JSON and missing captchaToken: wrap the request.json() call in a try/catch and return a 400 JSON error if parsing fails, then validate that body.captchaToken exists before calling verifyToken (return a 400 with a helpful error if it's missing); keep using verifyToken and TURNSTILE_SECRET_KEY but bail early on invalid input to avoid runtime exceptions and to surface clear error responses from the POST function.packages/server/tests/package-manifest.test.ts (1)
11-21: Consider explicit export-key existence checks for clearer failures.If an export key is removed, this currently fails via property access. Adding
toBeDefined()assertions first will make failures more actionable.🔍 Suggested test readability tweak
const rootExport = packageJson.exports["."]; +expect(rootExport).toBeDefined(); expect(rootExport.default).toMatch(/\.mjs$/); expect(rootExport.types).toMatch(/\.d\.mts$/); const providersExport = packageJson.exports["./providers/*"]; +expect(providersExport).toBeDefined(); expect(providersExport.default).toMatch(/\.mjs$/); expect(providersExport.types).toMatch(/\.d\.mts$/); const errorsExport = packageJson.exports["./errors"]; +expect(errorsExport).toBeDefined(); expect(errorsExport.default).toMatch(/\.mjs$/); expect(errorsExport.types).toMatch(/\.d\.mts$/);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/tests/package-manifest.test.ts` around lines 11 - 21, The test assumes export keys exist and will fail with unclear errors if they're removed; update the assertions to first assert the exports object and each specific export key are present (e.g., assert packageJson.exports is defined, then expect(packageJson.exports["."]).toBeDefined(), expect(packageJson.exports["./providers/*"]).toBeDefined(), expect(packageJson.exports["./errors"]).toBeDefined()) before accessing .default and .types on rootExport, providersExport, and errorsExport so failures report missing keys explicitly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/server/src/http.ts`:
- Around line 90-93: The JSON serialization in createPostRequestInit is
happening before the try/catch, so a thrown error escapes standardized wrapping;
move the call to createPostRequestInit (or at least the
JSON.stringify/serialization portion) into the existing try block that surrounds
the request flow (the same block that handles mergeAbortSignal/timeout), or wrap
the serialization in its own try/catch and rethrow a CaptchaServerError;
specifically update the code around effectiveTimeoutMs, mergeAbortSignal, and
createPostRequestInit so that any serialization failure is caught and converted
to a CaptchaServerError with the original error attached.
- Around line 59-61: The code calls AbortSignal.any(...) which throws on Node
20.0–20.2; implement a fallback merge instead of changing engines: add a helper
(e.g., manualAbortSignalMerge(signals)) that creates a new AbortController,
attaches 'abort' listeners to each input signal to call
controller.abort(reason), and returns controller.signal; update the return to
use signal: signals.length === 1 ? signals[0] : AbortSignal.any?.(signals) ??
manualAbortSignalMerge(signals); ensure the existing cleanup() removes the
listeners and aborts the controller if needed so no leaks (refer to symbols:
AbortSignal.any, signal, cleanup, manualAbortSignalMerge).
In `@packages/server/src/json.ts`:
- Around line 26-31: The error message in readRequiredBoolean is hardcoded to
mention "success"; change it to include the actual requested key name by using
the key parameter in the CaptchaServerError message so the thrown error reads
something like "Provider response is missing a boolean '<key>' field." Update
the call site in readRequiredBoolean (and keep provider in the meta) so the
message is dynamic based on the key argument rather than always saying
"success".
In `@packages/server/src/providers/friendly-captcha.ts`:
- Around line 52-53: The code uses options.endpoint (falling back to
DEFAULT_ENDPOINT) directly when building the HTTP request, which allows an empty
string to become a bad URL; before constructing the request (where url:
options.endpoint ?? DEFAULT_ENDPOINT and body: payload are set), validate
options.endpoint: if options.endpoint is defined but is an empty or
whitespace-only string, throw an error with code "invalid-argument" (or
otherwise return a validation error) instead of proceeding; alternatively,
validate that options.endpoint, when provided, is a well-formed URL (e.g.,
non-empty after trim and parses as a URL) and only then use it, otherwise use
DEFAULT_ENDPOINT or raise the invalid-argument error.
In `@packages/server/src/providers/hcaptcha.ts`:
- Around line 62-63: The code currently uses options.endpoint ??
DEFAULT_ENDPOINT which treats an empty-string endpoint as valid and causes a
network error; update the validation in the hcaptcha provider (before building
the request that uses options.endpoint and DEFAULT_ENDPOINT) to explicitly check
if options.endpoint is provided and non-empty (e.g., not === '' and is a valid
URL) and throw a clear argument/validation error when it is an empty string or
invalid; ensure this validation runs inside the function that constructs the
request (the hcaptcha provider's verification/request function) so the code uses
DEFAULT_ENDPOINT only when options.endpoint is undefined or null.
In `@packages/server/src/providers/recaptcha-compatible.ts`:
- Around line 43-47: The current merge order lets options.extraBody override
required verification fields; update the build of the form body so required keys
take precedence by either filtering out forbidden keys from options.extraBody
(secret, response, remoteip, sitekey) before merging or merging extraBody first
and then spreading the required fields last; locate the merge in
recaptcha-compatible where buildProviderFormBody(...) is called and ensure the
final object has the required fields (secret, response, remoteip, sitekey)
overriding any values from options.extraBody.
In `@packages/server/src/providers/recaptcha.ts`:
- Around line 73-84: The code currently calls getCommonMismatchCodes with a
possibly undefined score which lets requests pass when options.minScore is set
but the provider omitted score; update the logic around readNumber and
getCommonMismatchCodes so that if options.minScore is provided and score is
null/undefined you treat it as a mismatch (e.g., add a "score-missing" or reuse
"score-too-low" code) instead of passing null through; adjust the call site that
constructs mismatch inputs (the score variable and the getCommonMismatchCodes
invocation in recaptcha.ts) to inject this explicit mismatch when
options.minScore != null && score == null so the score gating cannot be
bypassed.
In `@packages/server/src/shared.ts`:
- Line 88: The condition currently gates the mismatch check using string
truthiness (check.expected && check.actual !== check.expected) which skips valid
empty-string expectations; change the guard to explicitly test for undefined
(e.g., check.expected !== undefined && check.actual !== check.expected) so
comparisons run when expected === "" — update the conditional that references
check.expected and check.actual accordingly.
In `@packages/server/tests/providers.test.ts`:
- Around line 25-36: The KEYS test fixture contains literal strings that
resemble real secrets (the KEYS constant and keys like turnstileSecret,
recaptchaSitekey, recaptchaSecret, hcaptchaSitekey, hcaptchaSecret,
captchaFoxSitekey, captchaFoxSecret, captchaFoxFailingSitekey,
captchaFoxFailingSecret, privateCaptchaApiKey) which trips secret scanners;
replace those values with clearly non-key placeholders (e.g.,
"TEST_TURNSTILE_SECRET", "TEST_RECAPTCHA_SITEKEY", "TEST_RECAPTCHA_SECRET",
etc.) or a uniform neutral pattern like "placeholder-<SERVICE>-value" to avoid
secret-like patterns, and adjust any tests that assert on specific string
content to either use the new placeholders or assert on behavior rather than
exact keys.
---
Nitpick comments:
In `@apps/docs/content/docs/en/server.mdx`:
- Around line 21-33: The POST handler should be hardened against malformed JSON
and missing captchaToken: wrap the request.json() call in a try/catch and return
a 400 JSON error if parsing fails, then validate that body.captchaToken exists
before calling verifyToken (return a 400 with a helpful error if it's missing);
keep using verifyToken and TURNSTILE_SECRET_KEY but bail early on invalid input
to avoid runtime exceptions and to surface clear error responses from the POST
function.
In `@packages/server/README.md`:
- Around line 5-6: Update the warning text in the README: change the phrase
"production ready" to the hyphenated form "production-ready" inside the warning
block (the line beginning with "[!WARNING]" that contains "This library is in
early development and is not production ready yet."). Keep the rest of the
sentence unchanged.
In `@packages/server/tests/package-manifest.test.ts`:
- Around line 11-21: The test assumes export keys exist and will fail with
unclear errors if they're removed; update the assertions to first assert the
exports object and each specific export key are present (e.g., assert
packageJson.exports is defined, then
expect(packageJson.exports["."]).toBeDefined(),
expect(packageJson.exports["./providers/*"]).toBeDefined(),
expect(packageJson.exports["./errors"]).toBeDefined()) before accessing .default
and .types on rootExport, providersExport, and errorsExport so failures report
missing keys explicitly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (38)
.github/workflows/build.ymlREADME.mdapps/docs/content/docs/en/index.mdxapps/docs/content/docs/en/provider/altcha.mdxapps/docs/content/docs/en/provider/cap-widget.mdxapps/docs/content/docs/en/provider/captcha-fox.mdxapps/docs/content/docs/en/provider/friendly-captcha.mdxapps/docs/content/docs/en/provider/hcaptcha.mdxapps/docs/content/docs/en/provider/private-captcha.mdxapps/docs/content/docs/en/provider/prosopo.mdxapps/docs/content/docs/en/provider/recaptcha-v3.mdxapps/docs/content/docs/en/provider/recaptcha.mdxapps/docs/content/docs/en/provider/turnstile.mdxapps/docs/content/docs/en/server.mdxapps/docs/content/docs/meta.jsonpackage.jsonpackages/server/CHANGELOG.mdpackages/server/README.mdpackages/server/package.jsonpackages/server/src/errors.tspackages/server/src/http.tspackages/server/src/index.tspackages/server/src/json.tspackages/server/src/providers/captcha-fox.tspackages/server/src/providers/friendly-captcha.tspackages/server/src/providers/hcaptcha.tspackages/server/src/providers/private-captcha.tspackages/server/src/providers/prosopo.tspackages/server/src/providers/recaptcha-compatible.tspackages/server/src/providers/recaptcha.tspackages/server/src/providers/turnstile.tspackages/server/src/result.tspackages/server/src/shared.tspackages/server/tests/package-manifest.test.tspackages/server/tests/providers.test.tspackages/server/tsconfig.jsonpackages/server/tsdown.config.tspackages/server/vitest.config.ts
Comment on lines
+59
to
+61
| return { | ||
| signal: signals.length === 1 ? signals[0] : AbortSignal.any(signals), | ||
| cleanup: () => { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Node engine constraints in repo package manifests:"
fd -a 'package.json' | while read -r file; do
echo "--- $file"
jq -r '.name + " :: " + (.engines.node // "<no engines.node declared>")' "$file"
done
echo
echo "AbortSignal.any usage sites:"
rg -nP '\bAbortSignal\.any\s*\('Repository: LuggaPugga/better-captcha
Length of output: 2337
🏁 Script executed:
#!/bin/bash
# Check the http.ts file context around line 60
echo "=== http.ts context (lines 50-70) ==="
sed -n '50,70p' packages/server/src/http.ts
echo
echo "=== Check `@better-captcha/server` package.json details ==="
cat packages/server/package.jsonRepository: LuggaPugga/better-captcha
Length of output: 2090
🌐 Web query:
AbortSignal.any Node.js version introduction when was it added
💡 Result:
AbortSignal.any() was added to Node.js in v20.3.0 (Current line) and backported to v18.17.0 (Node 18 LTS). [1] (MDN also lists Node.js support starting at 20.3.0.) [2]
Sources:
[1] Node.js API docs (Globals → AbortSignal.any(signals) “Added in: v20.3.0, v18.17.0”)
[2] MDN compatibility data for AbortSignal.any() (Node.js 20.3.0)
Add a compatibility fallback for AbortSignal.any to support Node 20.0–20.2.
AbortSignal.any() was added in Node 20.3.0 (and backported to v18.17.0). Since the root package declares engines.node: >=20 (which includes 20.0, 20.1, 20.2), code reaching line 60 with both timeout and external signals will throw TypeError: AbortSignal.any is not a function on unsupported versions.
Either:
- Add an explicit
engines.nodeconstraint topackages/server/package.jsonwith>=20.3.0, or - Implement a manual fallback for older runtimes:
signal: signals.length === 1
? signals[0]
: AbortSignal.any?.(signals) ?? manualAbortSignalMerge(signals)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/http.ts` around lines 59 - 61, The code calls
AbortSignal.any(...) which throws on Node 20.0–20.2; implement a fallback merge
instead of changing engines: add a helper (e.g.,
manualAbortSignalMerge(signals)) that creates a new AbortController, attaches
'abort' listeners to each input signal to call controller.abort(reason), and
returns controller.signal; update the return to use signal: signals.length === 1
? signals[0] : AbortSignal.any?.(signals) ?? manualAbortSignalMerge(signals);
ensure the existing cleanup() removes the listeners and aborts the controller if
needed so no leaks (refer to symbols: AbortSignal.any, signal, cleanup,
manualAbortSignalMerge).
Comment on lines
+90
to
+93
| const effectiveTimeoutMs = timeoutMs === undefined ? DEFAULT_TIMEOUT_MS : timeoutMs; | ||
| const abort = mergeAbortSignal(signal, effectiveTimeoutMs); | ||
| const requestInit = createPostRequestInit(body, abort.signal, headers); | ||
|
|
Contributor
There was a problem hiding this comment.
Serialization failures currently bypass your standardized error wrapping.
createPostRequestInit(...) executes before the try; if JSON serialization throws, callers get a raw error instead of CaptchaServerError.
Suggested fix
export async function postJson(options: PostJsonOptions): Promise<JsonObject> {
const { url, body, provider, fetcher, signal, timeoutMs, headers } = options;
const fetchImpl = getFetch(fetcher);
const effectiveTimeoutMs = timeoutMs === undefined ? DEFAULT_TIMEOUT_MS : timeoutMs;
const abort = mergeAbortSignal(signal, effectiveTimeoutMs);
- const requestInit = createPostRequestInit(body, abort.signal, headers);
try {
+ const requestInit = createPostRequestInit(body, abort.signal, headers);
const response = await fetchImpl(url, requestInit);📝 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
| const effectiveTimeoutMs = timeoutMs === undefined ? DEFAULT_TIMEOUT_MS : timeoutMs; | |
| const abort = mergeAbortSignal(signal, effectiveTimeoutMs); | |
| const requestInit = createPostRequestInit(body, abort.signal, headers); | |
| const effectiveTimeoutMs = timeoutMs === undefined ? DEFAULT_TIMEOUT_MS : timeoutMs; | |
| const abort = mergeAbortSignal(signal, effectiveTimeoutMs); | |
| try { | |
| const requestInit = createPostRequestInit(body, abort.signal, headers); | |
| const response = await fetchImpl(url, requestInit); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/http.ts` around lines 90 - 93, The JSON serialization in
createPostRequestInit is happening before the try/catch, so a thrown error
escapes standardized wrapping; move the call to createPostRequestInit (or at
least the JSON.stringify/serialization portion) into the existing try block that
surrounds the request flow (the same block that handles
mergeAbortSignal/timeout), or wrap the serialization in its own try/catch and
rethrow a CaptchaServerError; specifically update the code around
effectiveTimeoutMs, mergeAbortSignal, and createPostRequestInit so that any
serialization failure is caught and converted to a CaptchaServerError with the
original error attached.
Comment on lines
+26
to
+31
| export function readRequiredBoolean(object: JsonObject, key: string, provider: string): boolean { | ||
| const value = readBoolean(object, key); | ||
| if (value === undefined) { | ||
| throw new CaptchaServerError("invalid-response", "Provider response is missing a boolean success field.", { | ||
| provider, | ||
| }); |
Contributor
There was a problem hiding this comment.
Use the requested key name in the required-boolean error message.
readRequiredBoolean is generic, but the message always mentions success, which can mislead debugging for other keys.
Suggested fix
export function readRequiredBoolean(object: JsonObject, key: string, provider: string): boolean {
const value = readBoolean(object, key);
if (value === undefined) {
- throw new CaptchaServerError("invalid-response", "Provider response is missing a boolean success field.", {
+ throw new CaptchaServerError("invalid-response", `Provider response is missing a boolean "${key}" field.`, {
provider,
});
}
return 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
| export function readRequiredBoolean(object: JsonObject, key: string, provider: string): boolean { | |
| const value = readBoolean(object, key); | |
| if (value === undefined) { | |
| throw new CaptchaServerError("invalid-response", "Provider response is missing a boolean success field.", { | |
| provider, | |
| }); | |
| export function readRequiredBoolean(object: JsonObject, key: string, provider: string): boolean { | |
| const value = readBoolean(object, key); | |
| if (value === undefined) { | |
| throw new CaptchaServerError("invalid-response", `Provider response is missing a boolean "${key}" field.`, { | |
| provider, | |
| }); | |
| } | |
| return value; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/json.ts` around lines 26 - 31, The error message in
readRequiredBoolean is hardcoded to mention "success"; change it to include the
actual requested key name by using the key parameter in the CaptchaServerError
message so the thrown error reads something like "Provider response is missing a
boolean '<key>' field." Update the call site in readRequiredBoolean (and keep
provider in the meta) so the message is dynamic based on the key argument rather
than always saying "success".
Comment on lines
+52
to
+53
| url: options.endpoint ?? DEFAULT_ENDPOINT, | ||
| body: payload, |
Contributor
There was a problem hiding this comment.
Validate custom endpoint input before HTTP call.
A provided empty endpoint currently degrades into a network error instead of an invalid-argument error.
Proposed fix
export async function verifyFriendlyCaptcha(
options: FriendlyCaptchaVerifyOptions,
): Promise<FriendlyCaptchaVerificationResult> {
assertNonEmptyString(options.apiKey, "apiKey", PROVIDER);
assertNonEmptyString(options.response, "response", PROVIDER);
+ if (options.endpoint !== undefined) {
+ assertNonEmptyString(options.endpoint, "endpoint", PROVIDER);
+ }📝 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
| url: options.endpoint ?? DEFAULT_ENDPOINT, | |
| body: payload, | |
| export async function verifyFriendlyCaptcha( | |
| options: FriendlyCaptchaVerifyOptions, | |
| ): Promise<FriendlyCaptchaVerificationResult> { | |
| assertNonEmptyString(options.apiKey, "apiKey", PROVIDER); | |
| assertNonEmptyString(options.response, "response", PROVIDER); | |
| if (options.endpoint !== undefined) { | |
| assertNonEmptyString(options.endpoint, "endpoint", PROVIDER); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/providers/friendly-captcha.ts` around lines 52 - 53, The
code uses options.endpoint (falling back to DEFAULT_ENDPOINT) directly when
building the HTTP request, which allows an empty string to become a bad URL;
before constructing the request (where url: options.endpoint ?? DEFAULT_ENDPOINT
and body: payload are set), validate options.endpoint: if options.endpoint is
defined but is an empty or whitespace-only string, throw an error with code
"invalid-argument" (or otherwise return a validation error) instead of
proceeding; alternatively, validate that options.endpoint, when provided, is a
well-formed URL (e.g., non-empty after trim and parses as a URL) and only then
use it, otherwise use DEFAULT_ENDPOINT or raise the invalid-argument error.
Comment on lines
+62
to
+63
| url: options.endpoint ?? DEFAULT_ENDPOINT, | ||
| body, |
Contributor
There was a problem hiding this comment.
Validate custom endpoint input before HTTP call.
If options.endpoint is provided as an empty string, this currently fails later as a network error instead of a clear argument validation error.
Proposed fix
export async function verifyHCaptcha(options: HCaptchaVerifyOptions): Promise<HCaptchaVerificationResult> {
assertNonEmptyString(options.secret, "secret", PROVIDER);
assertNonEmptyString(options.response, "response", PROVIDER);
+ if (options.endpoint !== undefined) {
+ assertNonEmptyString(options.endpoint, "endpoint", PROVIDER);
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/providers/hcaptcha.ts` around lines 62 - 63, The code
currently uses options.endpoint ?? DEFAULT_ENDPOINT which treats an empty-string
endpoint as valid and causes a network error; update the validation in the
hcaptcha provider (before building the request that uses options.endpoint and
DEFAULT_ENDPOINT) to explicitly check if options.endpoint is provided and
non-empty (e.g., not === '' and is a valid URL) and throw a clear
argument/validation error when it is an empty string or invalid; ensure this
validation runs inside the function that constructs the request (the hcaptcha
provider's verification/request function) so the code uses DEFAULT_ENDPOINT only
when options.endpoint is undefined or null.
Comment on lines
+43
to
+47
| const body = buildProviderFormBody(options.secret, options.response, { | ||
| ...(options.extraBody ?? {}), | ||
| remoteip: options.remoteip, | ||
| sitekey: options.sitekey, | ||
| }); |
Contributor
There was a problem hiding this comment.
Prevent extraBody from overriding required verification fields.
extraBody can currently include secret/response and overwrite the validated values when building the final form body.
Proposed fix
- const body = buildProviderFormBody(options.secret, options.response, {
- ...(options.extraBody ?? {}),
- remoteip: options.remoteip,
- sitekey: options.sitekey,
- });
+ const extraBody = Object.fromEntries(
+ Object.entries(options.extraBody ?? {}).filter(([key]) => key !== "secret" && key !== "response"),
+ ) as Record<string, string>;
+
+ const body = buildProviderFormBody(options.secret, options.response, {
+ ...extraBody,
+ remoteip: options.remoteip,
+ sitekey: options.sitekey,
+ });📝 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
| const body = buildProviderFormBody(options.secret, options.response, { | |
| ...(options.extraBody ?? {}), | |
| remoteip: options.remoteip, | |
| sitekey: options.sitekey, | |
| }); | |
| const extraBody = Object.fromEntries( | |
| Object.entries(options.extraBody ?? {}).filter(([key]) => key !== "secret" && key !== "response"), | |
| ) as Record<string, string>; | |
| const body = buildProviderFormBody(options.secret, options.response, { | |
| ...extraBody, | |
| remoteip: options.remoteip, | |
| sitekey: options.sitekey, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/providers/recaptcha-compatible.ts` around lines 43 - 47,
The current merge order lets options.extraBody override required verification
fields; update the build of the form body so required keys take precedence by
either filtering out forbidden keys from options.extraBody (secret, response,
remoteip, sitekey) before merging or merging extraBody first and then spreading
the required fields last; locate the merge in recaptcha-compatible where
buildProviderFormBody(...) is called and ensure the final object has the
required fields (secret, response, remoteip, sitekey) overriding any values from
options.extraBody.
Comment on lines
+73
to
+84
| const score = readNumber(raw, "score"); | ||
| const mismatches = getCommonMismatchCodes<ReCaptchaErrorCode>({ | ||
| expectedHostname: options.expectedHostname, | ||
| hostname, | ||
| hostnameMismatchCode: "hostname-mismatch", | ||
| expectedAction: options.expectedAction, | ||
| action, | ||
| actionMismatchCode: "action-mismatch", | ||
| minScore: options.minScore, | ||
| score, | ||
| scoreTooLowCode: "score-too-low", | ||
| }); |
Contributor
There was a problem hiding this comment.
Fail closed when minScore is set but provider omits score.
When minScore is provided, a missing score currently produces no mismatch and can return success. That bypasses score gating.
🛡️ Suggested fix
const score = readNumber(raw, "score");
const mismatches = getCommonMismatchCodes<ReCaptchaErrorCode>({
expectedHostname: options.expectedHostname,
hostname,
hostnameMismatchCode: "hostname-mismatch",
expectedAction: options.expectedAction,
action,
actionMismatchCode: "action-mismatch",
minScore: options.minScore,
score,
scoreTooLowCode: "score-too-low",
});
+ if (typeof options.minScore === "number" && typeof score !== "number") {
+ mismatches.push("score-too-low");
+ }Also applies to: 86-92
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/providers/recaptcha.ts` around lines 73 - 84, The code
currently calls getCommonMismatchCodes with a possibly undefined score which
lets requests pass when options.minScore is set but the provider omitted score;
update the logic around readNumber and getCommonMismatchCodes so that if
options.minScore is provided and score is null/undefined you treat it as a
mismatch (e.g., add a "score-missing" or reuse "score-too-low" code) instead of
passing null through; adjust the call site that constructs mismatch inputs (the
score variable and the getCommonMismatchCodes invocation in recaptcha.ts) to
inject this explicit mismatch when options.minScore != null && score == null so
the score gating cannot be bypassed.
packages/server/src/shared.ts
Outdated
| ): TCode[] { | ||
| const mismatches: TCode[] = []; | ||
| for (const check of checks) { | ||
| if (check.expected && check.actual !== check.expected) { |
Contributor
There was a problem hiding this comment.
Don’t gate mismatch checks on string truthiness.
Using check.expected && ... skips comparison when expected is "". Compare against undefined instead.
Suggested fix
- if (check.expected && check.actual !== check.expected) {
+ if (check.expected !== undefined && check.actual !== check.expected) {
mismatches.push(check.mismatchCode);
}📝 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
| if (check.expected && check.actual !== check.expected) { | |
| if (check.expected !== undefined && check.actual !== check.expected) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/shared.ts` at line 88, The condition currently gates the
mismatch check using string truthiness (check.expected && check.actual !==
check.expected) which skips valid empty-string expectations; change the guard to
explicitly test for undefined (e.g., check.expected !== undefined &&
check.actual !== check.expected) so comparisons run when expected === "" —
update the conditional that references check.expected and check.actual
accordingly.
…TCHA compatible providers
… handling functions
…ndling in postJson, and enhance provider verification logic
… README for installation instructions
…r various providers
…server documentation
…and enhance error messages in verification providers
…ol branding and updating type check
…erification providers
vercel
bot
temporarily deployed
to
Preview – better-captcha-better-auth-example
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.
Todo
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests