Skip to content

Fix/jwt hardcoded secret#383

Open
HassanKorey wants to merge 2 commits into
geevapp:mainfrom
HassanKorey:fix/jwt-hardcoded-secret
Open

Fix/jwt hardcoded secret#383
HassanKorey wants to merge 2 commits into
geevapp:mainfrom
HassanKorey:fix/jwt-hardcoded-secret

Conversation

@HassanKorey

Copy link
Copy Markdown

Pull Request Template

Description

Closes #351

This PR addresses the token forgery vulnerability by removing the insecure NEXTAUTH_SECRET fallback and deleting the legacy, vulnerable session route that blindly trusted cookies.

What Changed:

  • app/lib/jwt.ts: Removed the hardcoded secret fallback. The module now throws an error on import if NEXTAUTH_SECRET is missing, ensuring the app "fails fast at startup".
  • app/app/(auth)/session/route.ts: Completely deleted the deprecated endpoint.
  • Cookies: Removed the insecure token/auth-token cookie extraction path from getTokenFromRequest and cleaned up the auth-token clearing logic in logout/route.ts.
  • Tests: Added jwt-config.test.ts to test the new fail-fast behavior and updated auth.test.ts to match the deleted routes/cookies.
  • CI: Added a step in .github/workflows/test.yml to explicitly check for NEXTAUTH_SECRET in production config secrets.

(Security Note: This completely closes the legacy token forgery path. Developers must now provide a valid NEXTAUTH_SECRET environment variable for the application to start or build successfully).


Checklist

  • I have tested my changes locally
  • I have updated documentation as needed
  • I have run npx prisma generate after schema changes
  • I have run npx prisma migrate dev or npx prisma migrate deploy as appropriate

Post-Merge Steps for Maintainers

If this PR includes changes to the Prisma schema:
(N/A - No Prisma schema changes in this PR)

  1. Run the following command to apply the migration to your database:

    npx prisma migrate deploy

@drips-wave

drips-wave Bot commented Jun 26, 2026

Copy link
Copy Markdown

@HassanKorey Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@HassanKorey

Copy link
Copy Markdown
Author

Heads up on the CI failures:

Run Tests: Fails on the new Check production config secrets step because GitHub Actions doesn't pass repository secrets to fork PRs. The check is working exactly as intended, but it's restricted by the fork environment.
Contracts CI: Seems to be failing for an unrelated reason, as this PR only touches the frontend Next.js auth logic and doesn't modify anything in the contracts/ directory.

@3m1n3nc3

Copy link
Copy Markdown
Contributor

Heads up on the CI failures:

Run Tests: Fails on the new Check production config secrets step because GitHub Actions doesn't pass repository secrets to fork PRs. The check is working exactly as intended, but it's restricted by the fork environment. Contracts CI: Seems to be failing for an unrelated reason, as this PR only touches the frontend Next.js auth logic and doesn't modify anything in the contracts/ directory.

We had a PR that broke the contracts, but that has been fixed; just sync your branch, and the contract failures will be gone. But the Next.js failures are yours to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lib/jwt.ts falls back to a hardcoded secret and a live legacy session route trusts a bearer cookie — token forgery

2 participants