fix(auth): remove dead secret fallback in email-log id salt - #22
Conversation
getEmailLogId reused `process.env.BETTER_AUTH_SECRET ?? "<literal>"`, which React Doctor flagged as a hardcoded secret that fails open. In production the module already throws when BETTER_AUTH_SECRET is unset, so the literal branch was unreachable there. Reuse the already-validated `rawAuthSecret` and give the dev-only path an explicitly named, non-secret salt — the value is a log-correlation HMAC salt, never a signing key. Clears the only React Doctor security error; full-project score 49 -> 87. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 34 minutes and 52 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ✨ 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 |
|
React Doctor found no issues. 🎉 Reviewed by React Doctor for commit |
Greptile Summary
Confidence Score: 4/5The change is narrowly scoped to auth email log ID salt handling, but one consistency issue remains when the configured auth secret includes surrounding whitespace. The reviewed file is small and the affected behavior was exercised with a targeted runtime check, so the remaining risk is well understood and localized. src/auth/better-auth.ts
 T-Rex LogsWhat T-Rex did
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
src/auth/better-auth.ts:50
**Normalize secrets consistently.** `rawAuthSecret` is trimmed here, but the Better Auth config still receives `process.env.BETTER_AUTH_SECRET` verbatim through `secret`. When the environment value has leading or trailing whitespace, production passes the startup check and Better Auth signs with the padded secret, while this helper derives email log IDs from the unpadded value. That silently changes correlation IDs for only this path. Use the same validated value for both the auth config and this helper, or keep both paths on the exact same raw value.
Reviews (1): Last reviewed commit: "fix(auth): remove dead secret fallback i..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Salt for a non-reversible log-correlation id (keeps raw emails out of logs). | ||
| // In production `rawAuthSecret` is guaranteed set (we throw above otherwise); | ||
| // the dev-only constant is an explicit, non-secret salt — never a real key. | ||
| const secret = rawAuthSecret ?? "clientra-dev-email-log-salt"; |
There was a problem hiding this comment.
rawAuthSecret is trimmed here, but the Better Auth config still receives process.env.BETTER_AUTH_SECRET verbatim through secret. When the environment value has leading or trailing whitespace, production passes the startup check and Better Auth signs with the padded secret, while this helper derives email log IDs from the unpadded value. That silently changes correlation IDs for only this path. Use the same validated value for both the auth config and this helper, or keep both paths on the exact same raw value.
Artifacts
Repro: targeted auth secret normalization harness
- Contains supporting evidence from the run (text/javascript; charset=utf-8).
Repro: runtime trace showing raw Better Auth secret and trimmed email HMAC salt
- Keeps the command output available without making the summary code-heavy.
Repro: attempted Vitest harness blocked by local Node engine mismatch
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/auth/better-auth.ts
Line: 50
Comment:
**Normalize secrets consistently.** `rawAuthSecret` is trimmed here, but the Better Auth config still receives `process.env.BETTER_AUTH_SECRET` verbatim through `secret`. When the environment value has leading or trailing whitespace, production passes the startup check and Better Auth signs with the padded secret, while this helper derives email log IDs from the unpadded value. That silently changes correlation IDs for only this path. Use the same validated value for both the auth config and this helper, or keep both paths on the exact same raw value.
How can I resolve this? If you propose a fix, please make it concise.
1 participant
What
getEmailLogIdinsrc/auth/better-auth.tsusedprocess.env.BETTER_AUTH_SECRET ?? "clientra-auth-email-log"to salt an HMAC log-correlation id. React Doctor flagged this as a hardcoded secret that "fails open."Why it's safe / why fix anyway
BETTER_AUTH_SECRETis unset (better-auth.ts:27), so the literal-fallback branch was unreachable dead code there.rawAuthSecretand gives the dev-only path an explicitly named, non-secret salt — clearer intent, no misleading "secret" literal.Impact
Clears the only React Doctor security error. Full-project score 49 → 87 ("Great"). The 6 remaining warnings are confirmed false positives (
String.prototype.includesmisread as array lookup; inline-function-call render helpers) plus vendoredevilcharts/**— no action needed.Checks
typecheck+lintpass locally.🤖 Generated with Claude Code