Skip to content

Fix graceful handling of invalid anonymous JWT cookies#2585

Merged
matthewvolk merged 7 commits intocanaryfrom
copilot/fix-be616404-26cd-4aa0-a963-c07c99842991
Sep 8, 2025
Merged

Fix graceful handling of invalid anonymous JWT cookies#2585
matthewvolk merged 7 commits intocanaryfrom
copilot/fix-be616404-26cd-4aa0-a963-c07c99842991

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Sep 5, 2025

Problem

Catalyst was throwing ungraceful errors when users had invalid anonymous JWT cookies. This typically occurred when:

  • A user had a cookie from a previous Catalyst instance with a different AUTH_SECRET
  • The JWT token was corrupted or malformed
  • The secret key used to sign the JWT had changed

The error would propagate up and crash the request, providing a poor user experience.

Solution

This PR adds graceful error handling to the getAnonymousSession() function in core/auth/anonymous-session.ts. The changes:

  1. Wrap JWT decode in try-catch: The decode() call from next-auth/jwt is now wrapped in error handling
  2. Return null for graceful recovery: Returns null which triggers the middleware to create a new anonymous session that overwrites the invalid cookie

Implementation

try {
  const session = await decode({
    secret,
    salt: `${cookiePrefix}${anonymousCookieName}`,
    token: jwt.value,
  });
  return session;
} catch (err) {
  return null;
}

Behavior

  • Before: Invalid JWT cookies caused application errors
  • After: Invalid JWT cookies are silently handled, and a new anonymous session is automatically created by the middleware

The error handling is cleanly encapsulated within the auth module, leveraging existing middleware logic. When getAnonymousSession() returns null due to an invalid JWT, the middleware detects the missing anonymous session and creates a new one, which automatically overwrites the invalid cookie.

Testing

Added E2E test coverage to verify:

  • Invalid JWT cookies are handled gracefully
  • New anonymous sessions are created when no valid cookie exists
  • Application continues to function normally after invalid cookie cleanup

Additional Changes

  • Fixed .gitignore: Removed package-lock.json entry since this is a pnpm-only workspace

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@vercel
Copy link
Copy Markdown

vercel Bot commented Sep 5, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
catalyst-b2b Ready Ready Preview Comment Sep 5, 2025 7:53pm
catalyst-canary Ready Ready Preview Comment Sep 5, 2025 7:53pm
3 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
catalyst Ignored Ignored Sep 5, 2025 7:53pm
catalyst-au Ignored Ignored Sep 5, 2025 7:53pm
catalyst-uk Ignored Ignored Sep 5, 2025 7:53pm

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Sep 5, 2025

🦋 Changeset detected

Latest commit: 0efb7fd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@bigcommerce/catalyst-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copilot AI changed the title [WIP] Hey team! Bug report/feature request coming in. Catalyst seems to ungracefully throw an error if the user's anonymous jwt (in a cookie) is invalid. In this case, I had one generated from a previous instance of Catalyst with a different secret key than ... Fix graceful handling of invalid anonymous JWT cookies Sep 5, 2025
Copilot AI requested a review from bookernath September 5, 2025 16:23
@bookernath bookernath force-pushed the copilot/fix-be616404-26cd-4aa0-a963-c07c99842991 branch from 360ea5f to 5ee2775 Compare September 5, 2025 17:13
Co-Authored-By: bookernath <8922457+bookernath@users.noreply.github.com>
@bookernath
Copy link
Copy Markdown
Contributor

@copilot I've moved the approach to middleware because there were bugs with the initial approach, can you clean it up?

Copilot AI and others added 2 commits September 5, 2025 17:51
Co-authored-by: bookernath <8922457+bookernath@users.noreply.github.com>
Co-authored-by: bookernath <8922457+bookernath@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 5, 2025

@copilot I've moved the approach to middleware because there were bugs with the initial approach, can you clean it up?

I've cleaned up the implementation in commit 3655888. The approach is now properly contained within the getAnonymousSession() function:

  • Removed the special { invalid: true } return value hack
  • Invalid JWT cookies are automatically cleared when detected and null is returned
  • Middleware no longer needs special handling - it just treats invalid sessions like missing sessions
  • Error handling is properly encapsulated in the auth module

The implementation is now cleaner and follows better separation of concerns.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • googlechromelabs.github.io
    • Triggering command: node install.mjs (dns block)
  • https://storage.googleapis.com/chrome-for-testing-public/140.0.7339.80/linux64/chrome-headless-shell-linux64.zip
    • Triggering command: node install.mjs (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Comment thread .gitignore Outdated
node_modules
dist
.turbo
package-lock.json
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use npm, only use pnpm

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Do not use npm, only use pnpm

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed package-lock.json from .gitignore since this is a pnpm-only workspace. Fixed in commit ebdef36.

Comment thread core/auth/anonymous-session.ts Outdated
const useSecureCookies = await shouldUseSecureCookie();
const cookiePrefix = useSecureCookies ? '__Secure-' : '';
// Clear the invalid cookie
await clearAnonymousSession();
Copy link
Copy Markdown
Contributor

@matthewvolk matthewvolk Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Can't this entire PR be simplified if we simply omit this line and just return null, and allow the anonymousSignIn() to trigger in this condition?

// Create the anonymous session if it doesn't exist
if (!req.auth && !anonymousSession) {
await anonymousSignIn();
}

We can rely on that condition because creating an anonymous session is independent of req.auth. When you create the anonymous session only sets the anonymous cookie and does not change the NextAuth session that drives req.auth. So if the jwt cannot be decoded and we simply return null, so long as the user is not actually signed in, it should trigger the logic above.

Suggested change
await clearAnonymousSession();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Can't this entire PR be simplified if we simply omit this line and just return null, and allow the anonymousSignIn() to trigger in this condition?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're absolutely right! Simplified the implementation by removing the explicit clearAnonymousSession() call. Now the error handler just returns null and lets the existing middleware logic at lines 21-24 handle creating a new anonymous session, which automatically overwrites the invalid cookie. This is much cleaner and avoids duplicating cookie management logic. Fixed in commit ebdef36.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified the error handling as suggested by removing the console.error line entirely. The function now just returns null when JWT decoding fails, which triggers the middleware logic at lines 21-24 to create a new anonymous session that overwrites the invalid cookie. This approach is cleaner and avoids duplicating logging logic. Fixed in commit 24eda3f.

Co-authored-by: bookernath <8922457+bookernath@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 5, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • googlechromelabs.github.io
    • Triggering command: node install.mjs (dns block)
  • https://storage.googleapis.com/chrome-for-testing-public/137.0.7151.55/linux64/chrome-headless-shell-linux64.zip
    • Triggering command: node install.mjs (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Co-authored-by: bookernath <8922457+bookernath@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 5, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • googlechromelabs.github.io
  • https://storage.googleapis.com/chrome-for-testing-public/137.0.7151.55/linux64/chrome-headless-shell-linux64.zip
    • Triggering command: node install.mjs (http block)
  • https://storage.googleapis.com/chrome-for-testing-public/137.0.7151.55/linux64/chrome-linux64.zip

If you need me to access, download, or install something from one of these locations, you can either:

Copy link
Copy Markdown
Contributor

@matthewvolk matthewvolk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran Playwright locally to verify the tests pass.
Screenshot 2025-09-05 at 3 42 34 PM

Only reason I'm not approving is because I contributed to this PR and don't want to approve my own.

@matthewvolk matthewvolk dismissed their stale review September 5, 2025 20:44

Now that I've contributed to this PR, I'd prefer that someone else gives final approval

@matthewvolk matthewvolk marked this pull request as ready for review September 5, 2025 20:45
@matthewvolk matthewvolk requested a review from a team September 5, 2025 20:45
@matthewvolk matthewvolk added this pull request to the merge queue Sep 8, 2025
Merged via the queue into canary with commit a40b96f Sep 8, 2025
13 checks passed
@matthewvolk matthewvolk deleted the copilot/fix-be616404-26cd-4aa0-a963-c07c99842991 branch September 8, 2025 18:31
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.

4 participants