fix(auth,atoms): required CALENDSO_ENCRYPTION_KEY and remove Stripe secrets screenshot - #76
Conversation
An unset key silently encoded to a zero-length secret in signJwt and the login getServerSideProps JWT verify (jose signs and verifies HS256 with a 0-byte key successfully), and degraded the email verification seed to a pure function of the victim email in verifyCodeUnAuthenticated (audit LO-04/LO-05). All three sites now throw, matching the required-env pattern already used in next.config.ts. Production containers already refuse to boot without the key (scripts/start.sh), so only misconfigured local setups see the new error.
…nt ID The 590KB upstream-inherited screenshot of a Stripe test-mode API keys page shipped partially legible key fragments and a fully visible account ID (audit MD-30). Nothing references the file. The test-mode keys shown should still be rotated on the Stripe dashboard.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
There was a problem hiding this comment.
Code Review
This pull request introduces explicit checks for the CALENDSO_ENCRYPTION_KEY environment variable across JWT verification, signing, and unauthenticated code verification to prevent security degradation or silent failures when the key is missing. The reviewer pointed out that in getServerSideProps.tsx, the check is placed inside a try-catch block, which would catch the thrown error and silently redirect the user instead of failing loudly as intended. It is recommended to move this check outside the try-catch block to properly surface the configuration issue.
| const verifyJwt = (jwt: string) => { | ||
| // A missing key would encode to a zero-length secret, which jose happily | ||
| // verifies against - fail loudly instead (same pattern as next.config.ts). | ||
| if (!process.env.CALENDSO_ENCRYPTION_KEY) throw new Error("Please set CALENDSO_ENCRYPTION_KEY"); |
There was a problem hiding this comment.
The check for CALENDSO_ENCRYPTION_KEY is placed inside verifyJwt, which is executed within a try-catch block inside getServerSideProps (lines 31-50). If the key is missing, the thrown error will be caught by the catch block and the user will be silently redirected to /auth/error?error=Invalid%20JWT%3A%20Please%20try%20again. This prevents the application from failing loudly as intended and masks a critical configuration issue as a user-facing authentication error.\n\nTo ensure the application fails loudly on misconfiguration, this check should be moved to the top of getServerSideProps (outside of the try-catch block).
What
Closes audit findings MD-30, LO-04 and LO-05:
signJwtand the logingetServerSidePropsJWT verify encoded it to a zero-length secret (jose happily signs/verifies HS256 with a 0-byte key), andverifyCodeUnAuthenticateddegraded its seed to a pure function of the victim email (|| ""). All three now throw, matching the required-env pattern already innext.config.ts. Production containers already refuse to boot without the key (scripts/start.sh), so only misconfigured local setups see the new error.packages/platform/atoms/static/Stripe secrets.png(test-mode API-keys screenshot with a fully visible Stripe account ID, upstream-inherited). Nothing references the file.Evidence
yarn test packages/features/auth).biome linton the changed files: warnings/infos only, same set that already exists on dev (CI lint gate stays green).Note for JOY
The test-mode Stripe keys visible in the removed screenshot should still be rotated on the Stripe dashboard (deleting the file does not un-publish the account ID from git history).
🤖 Generated by ZCode