From cd75b5c81b70b9848cb249c297437cde7b34e2b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriele=20Vigan=C3=B2?= Date: Fri, 25 Sep 2026 15:33:22 +0000 Subject: [PATCH 1/2] fix: keep one Better Auth instance so provider sign-in returns to the app Signing in to an application with Microsoft (or Google) left the user on the auth site's own page instead of sending them back: the sign-in itself worked, but the OpenID Connect authorization it was supposed to complete was dropped, so the application never received its code. Clearing site data appeared to fix it only because a second attempt reuses the session cookie and skips the sign-in step entirely. Better Auth carries the pending authorization across the provider redirect in per-request state, keyed by module-private tokens `defineRequestState()` mints once per module evaluation. The server build inlined `better-auth` into the SSR bundle and gave each `@better-auth/*` plugin its own chunk with a second copy inlined, so the OAuth provider plugin stored the pending request under one set of keys while Better Auth read another and found nothing. Neither half of the resume ran, and nothing logged an error. Bundle the Better Auth packages together so one module instance, and one set of keys, serves the whole flow. Because the failure is silent and the cause is a bundler heuristic that shifts with dependency upgrades, the build now also fails if either module that owns per-request state is emitted more than once. Co-Authored-By: Claude Opus 5 (1M context) --- package.json | 2 +- scripts/check-server-bundle.mjs | 63 +++++++++++++++++++++++++++++++++ vite.config.ts | 17 +++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 scripts/check-server-bundle.mjs diff --git a/package.json b/package.json index 424a208..fac29b7 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "scripts": { "dev": "dotenv -e .env.local -- env NODE_OPTIONS='--import ./instrument.server.mjs' vp dev", "generate-routes": "tsr generate", - "build": "vp build && cp instrument.server.mjs .output/server", + "build": "vp build && node scripts/check-server-bundle.mjs && cp instrument.server.mjs .output/server", "preview": "vp preview", "start": "node --import ./.output/server/instrument.server.mjs scripts/start.mjs", "db:generate": "drizzle-kit generate", diff --git a/scripts/check-server-bundle.mjs b/scripts/check-server-bundle.mjs new file mode 100644 index 0000000..57a42d9 --- /dev/null +++ b/scripts/check-server-bundle.mjs @@ -0,0 +1,63 @@ +import { readFile, readdir } from "node:fs/promises"; +import { fileURLToPath } from "node:url"; + +/** + * Better Auth keeps the in-flight OpenID Connect request in per-request state, + * keyed by module-private tokens `defineRequestState()` mints once per module + * evaluation. Two copies of a module in one bundle means two sets of keys: the + * OAuth provider plugin stores the pending authorization under one, Better Auth + * looks for it under the other, and the sign-in completes without ever resuming + * the authorization. The user lands back on the sign-in page, signed in, while + * the application that sent them there waits for a code that never arrives. + * + * Nothing about that failure is loud, so this guards the shape of the bundle + * instead: the modules that own per-request state must each appear exactly once. + * `vite.config.ts` keeps them in one chunk through `ssr.noExternal`. + */ +const stateModules = [ + { marker: "No request state found", module: "@better-auth/core request state" }, + { marker: "addOAuthServerContext", module: "better-auth OAuth request state" }, +]; + +const serverDir = fileURLToPath(new URL("../.output/server", import.meta.url)); + +async function serverChunks(directory) { + const entries = await readdir(directory, { withFileTypes: true }); + const files = await Promise.all( + entries.map((entry) => { + const path = `${directory}/${entry.name}`; + if (entry.isDirectory()) return serverChunks(path); + return entry.name.endsWith(".mjs") ? [path] : []; + }), + ); + return files.flat(); +} + +const chunks = await serverChunks(serverDir); +const sources = await Promise.all( + chunks.map(async (path) => ({ path, code: await readFile(path, "utf8") })), +); + +const problems = []; +for (const { marker, module } of stateModules) { + const holders = sources.filter(({ code }) => code.includes(marker)); + if (holders.length === 0) { + problems.push( + `${module}: marker "${marker}" is gone from the build, so this check no longer guards anything. Update the marker.`, + ); + } else if (holders.length > 1) { + const where = holders.map(({ path }) => path.slice(serverDir.length + 1)).join(", "); + problems.push( + `${module}: bundled ${holders.length} times (${where}). Signing in through an` + + " application would succeed without ever returning an authorization code." + + " Keep every better-auth package in `ssr.noExternal` in vite.config.ts.", + ); + } +} + +if (problems.length) { + console.error(`Server bundle check failed:\n- ${problems.join("\n- ")}`); + process.exit(1); +} + +console.info("Server bundle check passed: per-request state is not duplicated."); diff --git a/vite.config.ts b/vite.config.ts index c63f5f0..74600fb 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -19,6 +19,23 @@ const config = defineConfig({ options: { typeAware: true, typeCheck: true }, }, resolve: { tsconfigPaths: true }, + // Better Auth carries the in-flight OpenID Connect request across the upstream + // provider redirect in per-request state, keyed by module-private tokens that + // `defineRequestState()` mints once per module evaluation. Left alone, the server + // build inlines `better-auth` into the SSR bundle while giving each + // `@better-auth/*` plugin its own chunk with a second copy inlined, so the plugin + // writes the pending request under one key and Better Auth reads another. Nothing + // fails loudly: sign-in succeeds, the authorization is silently dropped, and the + // application that sent the user here never receives its code. Bundling them + // together keeps one module instance, and one set of keys. + ssr: { + noExternal: [ + "better-auth", + "@better-auth/core", + "@better-auth/oauth-provider", + "@better-auth/passkey", + ], + }, plugins: lazyPlugins(() => process.env.VITEST ? [] From 0360e0ab8ce7a30c5179979c7b3ce837cff50325 Mon Sep 17 00:00:00 2001 From: Lorenzo Corallo Date: Fri, 25 Sep 2026 17:52:48 +0200 Subject: [PATCH 2/2] fix: guard every Better Auth request state, not a fixed list The fix only kept four named Better Auth packages together, and the build check only looked for two hand-picked text markers. Adding another `@better-auth/*` plugin, or a Better Auth upgrade that adds new request state, could bring the lost-authorization bug back without the check noticing. Match the packages by name pattern instead, and make the check find every `defineRequestState()` call in the built server and fail if any of them appears more than once. The check now has tests. Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/check-server-bundle.mjs | 90 ++++++++++++++++++---------- scripts/check-server-bundle.test.mjs | 42 +++++++++++++ vite.config.ts | 11 ++-- 3 files changed, 106 insertions(+), 37 deletions(-) create mode 100644 scripts/check-server-bundle.test.mjs diff --git a/scripts/check-server-bundle.mjs b/scripts/check-server-bundle.mjs index 57a42d9..91bc43f 100644 --- a/scripts/check-server-bundle.mjs +++ b/scripts/check-server-bundle.mjs @@ -11,15 +11,51 @@ import { fileURLToPath } from "node:url"; * the application that sent them there waits for a code that never arrives. * * Nothing about that failure is loud, so this guards the shape of the bundle - * instead: the modules that own per-request state must each appear exactly once. + * instead: every `defineRequestState()` call must appear exactly once, and so + * must the core module that owns the request state. Checking every call, rather + * than a fixed list, also covers state added by future Better Auth releases. * `vite.config.ts` keeps them in one chunk through `ssr.noExternal`. */ -const stateModules = [ - { marker: "No request state found", module: "@better-auth/core request state" }, - { marker: "addOAuthServerContext", module: "better-auth OAuth request state" }, -]; +const stateCall = /\b(?:var|let|const)\s+([^=;]+?)\s*=\s*defineRequestState(?:\$\d+)?\(/g; +const coreMarker = "No request state found"; -const serverDir = fileURLToPath(new URL("../.output/server", import.meta.url)); +/** @param {{ path: string, code: string }[]} sources */ +export function findBundleProblems(sources) { + const calls = new Map(); + for (const { path, code } of sources) { + for (const [, binding] of code.matchAll(stateCall)) { + const name = binding.replace(/\s+/g, " ").trim(); + calls.set(name, [...(calls.get(name) ?? []), path]); + } + } + + const problems = []; + if (calls.size === 0) { + problems.push( + "no `defineRequestState()` call found in the build, so this check no longer guards anything. Update it.", + ); + } + for (const [name, paths] of calls) { + if (paths.length > 1) { + problems.push( + `request state \`${name}\` is bundled ${paths.length} times (${paths.join(", ")}).`, + ); + } + } + + const coreHolders = sources.filter(({ code }) => code.includes(coreMarker)); + if (coreHolders.length === 0) { + problems.push( + `marker "${coreMarker}" is gone from the build, so this check no longer guards anything. Update the marker.`, + ); + } else if (coreHolders.length > 1) { + const where = coreHolders.map(({ path }) => path).join(", "); + problems.push( + `@better-auth/core request state is bundled ${coreHolders.length} times (${where}).`, + ); + } + return problems; +} async function serverChunks(directory) { const entries = await readdir(directory, { withFileTypes: true }); @@ -27,37 +63,31 @@ async function serverChunks(directory) { entries.map((entry) => { const path = `${directory}/${entry.name}`; if (entry.isDirectory()) return serverChunks(path); - return entry.name.endsWith(".mjs") ? [path] : []; + return /\.[cm]?js$/.test(entry.name) ? [path] : []; }), ); return files.flat(); } -const chunks = await serverChunks(serverDir); -const sources = await Promise.all( - chunks.map(async (path) => ({ path, code: await readFile(path, "utf8") })), -); +if (process.argv[1] === fileURLToPath(import.meta.url)) { + const serverDir = fileURLToPath(new URL("../.output/server", import.meta.url)); + const chunks = await serverChunks(serverDir); + const sources = await Promise.all( + chunks.map(async (path) => ({ + path: path.slice(serverDir.length + 1), + code: await readFile(path, "utf8"), + })), + ); -const problems = []; -for (const { marker, module } of stateModules) { - const holders = sources.filter(({ code }) => code.includes(marker)); - if (holders.length === 0) { - problems.push( - `${module}: marker "${marker}" is gone from the build, so this check no longer guards anything. Update the marker.`, - ); - } else if (holders.length > 1) { - const where = holders.map(({ path }) => path.slice(serverDir.length + 1)).join(", "); - problems.push( - `${module}: bundled ${holders.length} times (${where}). Signing in through an` + - " application would succeed without ever returning an authorization code." + - " Keep every better-auth package in `ssr.noExternal` in vite.config.ts.", + const problems = findBundleProblems(sources); + if (problems.length) { + console.error( + `Server bundle check failed:\n- ${problems.join("\n- ")}\n` + + "Signing in through an application would succeed without ever returning an" + + " authorization code. Keep every better-auth package in `ssr.noExternal` in vite.config.ts.", ); + process.exit(1); } -} -if (problems.length) { - console.error(`Server bundle check failed:\n- ${problems.join("\n- ")}`); - process.exit(1); + console.info("Server bundle check passed: per-request state is not duplicated."); } - -console.info("Server bundle check passed: per-request state is not duplicated."); diff --git a/scripts/check-server-bundle.test.mjs b/scripts/check-server-bundle.test.mjs new file mode 100644 index 0000000..5ab04e8 --- /dev/null +++ b/scripts/check-server-bundle.test.mjs @@ -0,0 +1,42 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { findBundleProblems } from "./check-server-bundle.mjs"; + +const core = `function defineRequestState(initFn) {} +throw new Error("No request state found. Please make sure...");`; +const betterAuth = `var { get: getOAuthServerContext, set: setOAuthServerContext } = defineRequestState(() => null);`; +const oauthProvider = `var oAuthState = defineRequestState$1(() => null);`; + +describe("server bundle check", () => { + it("passes when every piece of request state is bundled once", () => { + const sources = [{ path: "_ssr/router.mjs", code: `${core}\n${betterAuth}\n${oauthProvider}` }]; + + expect(findBundleProblems(sources)).toEqual([]); + }); + + it("fails when a module that defines request state is bundled twice", () => { + const sources = [ + { path: "_ssr/router.mjs", code: `${core}\n${betterAuth}` }, + { path: "_libs/oauth-provider.mjs", code: `${betterAuth}\n${oauthProvider}` }, + ]; + + expect(findBundleProblems(sources)).toEqual([ + "request state `{ get: getOAuthServerContext, set: setOAuthServerContext }` is bundled 2 times (_ssr/router.mjs, _libs/oauth-provider.mjs).", + ]); + }); + + it("fails when the core request state module is bundled twice", () => { + const sources = [ + { path: "_ssr/router.mjs", code: `${core}\n${betterAuth}` }, + { path: "_libs/core.mjs", code: core }, + ]; + + expect(findBundleProblems(sources)).toEqual([ + "@better-auth/core request state is bundled 2 times (_ssr/router.mjs, _libs/core.mjs).", + ]); + }); + + it("fails when it can no longer find what it guards", () => { + expect(findBundleProblems([{ path: "_ssr/router.mjs", code: "" }])).toHaveLength(2); + }); +}); diff --git a/vite.config.ts b/vite.config.ts index 74600fb..1b534ef 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -27,14 +27,11 @@ const config = defineConfig({ // writes the pending request under one key and Better Auth reads another. Nothing // fails loudly: sign-in succeeds, the authorization is silently dropped, and the // application that sent the user here never receives its code. Bundling them - // together keeps one module instance, and one set of keys. + // together keeps one module instance, and one set of keys. Match by pattern so a + // newly installed `@better-auth/*` plugin is covered without editing this list; + // `scripts/check-server-bundle.mjs` fails the build if anything is duplicated anyway. ssr: { - noExternal: [ - "better-auth", - "@better-auth/core", - "@better-auth/oauth-provider", - "@better-auth/passkey", - ], + noExternal: [/^better-auth(\/|$)/, /^@better-auth\//], }, plugins: lazyPlugins(() => process.env.VITEST