Skip to content

fix(backend): Improve satellite auth redirect failures#8636

Open
jescalan wants to merge 2 commits into
mainfrom
je/satellite-auth-redirect-invalid-fixes
Open

fix(backend): Improve satellite auth redirect failures#8636
jescalan wants to merge 2 commits into
mainfrom
je/satellite-auth-redirect-invalid-fixes

Conversation

@jescalan

@jescalan jescalan commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Improves the debugging experience for production satellite-domain auth failures when the handshake redirect_url points at a host that is not associated with the instance, such as a dynamic preview deployment. FAPI now returns a dedicated host-scoped error for this case and renders a readable HTML response for document handshakes, while the backend SDK logs satellite-specific guidance when this condition turns into a redirect loop.

Changes in this repo

Updates the backend SDK handshake redirect-loop diagnostic to show satellite-domain and preview-deployment guidance for SatelliteCookieNeedsSyncing loops, with a focused authenticateRequest regression test.

Companion PRs

@vercel

vercel Bot commented May 22, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
clerk-js-sandbox Ready Ready Preview, Comment Jun 3, 2026 7:27pm

Request Review

@changeset-bot

changeset-bot Bot commented May 22, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b994202

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

This PR includes changesets to release 10 packages
Name Type
@clerk/backend Patch
@clerk/astro Patch
@clerk/express Patch
@clerk/fastify Patch
@clerk/hono Patch
@clerk/nextjs Patch
@clerk/nuxt Patch
@clerk/react-router Patch
@clerk/tanstack-react-start Patch
@clerk/testing 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

@jescalan jescalan changed the title Improve satellite auth redirect failures fix(backend): Improve satellite auth redirect failures May 22, 2026
@jescalan jescalan marked this pull request as ready for review May 22, 2026 22:12
@jescalan

Copy link
Copy Markdown
Contributor Author

@codex review

@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: a1007b13-cb3d-4a18-ba94-57b1dd8c24ea

📥 Commits

Reviewing files that changed from the base of the PR and between a6496aa and b994202.

📒 Files selected for processing (3)
  • .changeset/polite-satellites-wait.md
  • packages/backend/src/tokens/__tests__/request.test.ts
  • packages/backend/src/tokens/request.ts
✅ Files skipped from review due to trivial changes (1)
  • .changeset/polite-satellites-wait.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/backend/src/tokens/tests/request.test.ts
  • packages/backend/src/tokens/request.ts

📝 Walkthrough

Walkthrough

This PR extracts redirect-loop message generation into getHandshakeRedirectLoopMessage(reason) and uses it in authenticateRequest so the signed-out message is reason-aware (satellite-cookie-specific when reason === AuthErrorReason.SatelliteCookieNeedsSyncing, otherwise a generic message). It adds a vitest that simulates a satellite redirect loop asserting SignedOut with AuthErrorReason.SatelliteCookieNeedsSyncing and that console.log contains guidance substrings. The changeset documents a patch release for @clerk/backend noting improved diagnostics.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and concisely describes the main change: improving satellite auth redirect failures in the backend, which aligns with all three file changes.
Description check ✅ Passed The description is related to the changeset, explaining the satellite redirect-loop diagnostics improvements and test additions that match the file changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@pkg-pr-new

pkg-pr-new Bot commented May 22, 2026

Copy link
Copy Markdown

Open in StackBlitz

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@8636

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@8636

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@8636

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@8636

@clerk/expo

npm i https://pkg.pr.new/@clerk/expo@8636

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@8636

@clerk/express

npm i https://pkg.pr.new/@clerk/express@8636

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@8636

@clerk/hono

npm i https://pkg.pr.new/@clerk/hono@8636

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@8636

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@8636

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@8636

@clerk/react

npm i https://pkg.pr.new/@clerk/react@8636

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@8636

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@8636

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@8636

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@8636

@clerk/ui

npm i https://pkg.pr.new/@clerk/ui@8636

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@8636

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@8636

commit: b994202

@jescalan

Copy link
Copy Markdown
Contributor Author

@codex review
@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the PR details and diff
gh pr view 8636 --repo clerk/javascript --json title,body,files,additions,deletions,changedFiles

# Get the diff
gh pr diff 8636 --repo clerk/javascript

Length of output: 5215



@jescalan: The changes look clean and focused. Here's my review:


packages/backend/src/tokens/request.ts

Correctness ✅ — The branching logic is correct. SatelliteCookieNeedsSyncing gets a targeted, actionable message while all other redirect-loop reasons fall back to the original generic message. No behavioral regression for non-satellite flows.

Minor: function placementgetHandshakeRedirectLoopMessage is declared (via hoisting) after its call site. While this is valid TypeScript, moving the function definition above its first use would improve readability:

// Define before use at line ~338
function getHandshakeRedirectLoopMessage(reason: string): string {
  if (reason === AuthErrorReason.SatelliteCookieNeedsSyncing) {
    return `Clerk: Satellite-domain authentication resulted in an infinite redirect loop. ...`;
  }
  return `Clerk: Refreshing the session token resulted in an infinite redirect loop. ...`;
}

const isRedirectLoop = handshakeService.checkAndTrackRedirectLoop(handshakeHeaders);
if (isRedirectLoop) {
  const msg = getHandshakeRedirectLoopMessage(reason);
  // ...
}

Optional: module-level extraction — Since the function doesn't close over any local variables, it could also be extracted to module level to avoid re-declaration on each authenticateRequest invocation. Low impact but cleaner.


packages/backend/src/tokens/__tests__/request.test.ts

Test coverage ✅ — The regression test is well-structured:

  • Correctly uses __clerk_redirect_count: '3' + __clerk_synced=false + isSatellite: true to trigger the satellite redirect loop
  • Asserts the full signed-out state (reason, isSatellite, domain, signInUrl)
  • Verifies all three key substrings of the satellite guidance message
  • Properly restores the spy with consoleSpy.mockRestore()

No issues with the test logic or assertions.


Summary

The PR achieves its goal with minimal surface area. The only actionable suggestion is readability-oriented (move the function definition above its call site). The test coverage is solid for the new branch. ✅

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

API Changes Report

Generated by Break Check on 2026-06-03T19:30:41.354Z

Summary

Metric Count
Packages analyzed 19
Packages with changes 1
🔴 Breaking changes 0
🟡 Non-breaking changes 1
🟢 Additions 0

🤖 This report was reviewed by claude-sonnet-4-6.

Note
Break Check could not snapshot 3 subpaths; the diff below excludes them.

  • @clerk/astro ./env: Internal Error: Unable to determine module for: /home/runner/_work/javascript/javascript/packages/astro/env.d.ts You have encountered a software defect. Please consider reporting the issue to the maintainers of this application.
  • @clerk/shared ./cookie: Internal Error: Unable to follow symbol for "Cookies" You have encountered a software defect. Please consider reporting the issue to the maintainers of this application.
  • @clerk/testing ./cypress: Symbol not found for identifier: Cypress

@clerk/shared

Current version: 4.15.0
Recommended bump: MINOR → 4.16.0

Subpath ./apiUrlFromPublishableKey

🟡 Non-breaking Changes (1)

Modified: apiUrlFromPublishableKey
- apiUrlFromPublishableKey: (publishableKey: string) => "https://api.lclclerk.com" | "https://api.clerkstage.dev" | "https://api.clerk.com"
+ apiUrlFromPublishableKey: (publishableKey: string) => "https://api.clerk.com" | "https://api.lclclerk.com" | "https://api.clerkstage.dev"

Static analyzer: Breaking change in function apiUrlFromPublishableKey: Return type changed: "https://api.lclclerk.com"|"https://api.clerkstage.dev"|"https://api.clerk.com""https://api.clerk.com"|"https://api.lclclerk.com"|"https://api.clerkstage.dev"

🤖 AI review (reclassified as non-breaking) (99%): The union type contains exactly the same three string literal members; only their order changed, and TypeScript union member order is irrelevant to assignability or type-checking for consumers.


Report generated by Break Check

Last ran on b994202. Pushes that change no tracked declarations (no API surface change vs. base) are skipped and don't update this comment.

@jescalan jescalan force-pushed the je/satellite-auth-redirect-invalid-fixes branch from a6496aa to b994202 Compare June 3, 2026 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant