From 61b32ec647d2e0acdb5669c4ae394798152a6e8c Mon Sep 17 00:00:00 2001 From: Ben Senescu <44480372+bensenescu@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:37:46 -0400 Subject: [PATCH] fix: extend OAuth state cookie lifetime to match the 10-minute state TTL (#544) --- src/lib/auth-config.ts | 10 ++++++++++ src/lib/auth.ts | 25 +++++++++++++++++++------ src/server/mcp/api-key-auth.ts | 1 + src/server/workflows/rankCheckPaths.ts | 2 +- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/lib/auth-config.ts b/src/lib/auth-config.ts index f818126..989ff9d 100644 --- a/src/lib/auth-config.ts +++ b/src/lib/auth-config.ts @@ -15,6 +15,16 @@ export function createBaseAuthConfig() { // /api/auth endpoint. Header lookup is case-insensitive. ipAddressHeaders: ["cf-connecting-ip"], }, + // Better Auth writes the OAuth state verification row with a 10-minute + // expiry but sets the matching signed cookie with maxAge 300, and + // parseGenericState checks the cookie before the row's expiresAt — so the + // intended 10-minute window is unreachable. The GSC and GA4 providers + // below force `select_account consent`, a two-screen Google flow, so a + // user who takes more than 5 minutes returns with a live verification row + // and a dead cookie and fails with "State mismatch: State not persisted + // correctly". The row's expiresAt still enforces the real 10-minute + // window and the state stays single-use, so this is not a weakening. + cookies: { state: { attributes: { maxAge: 600 } } }, }, account: { // Encrypt OAuth access/refresh tokens at rest in D1. Also covers the diff --git a/src/lib/auth.ts b/src/lib/auth.ts index e10b372..565540b 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -68,14 +68,27 @@ function createAuth() { const auth = betterAuth({ baseURL: baseUrl, secret: getHostedSecret(), - // The api-key plugin logs every verification failure at error level — a - // stale key or a throttled caller included. The /mcp handler already logs - // the response it returns at the right level (debug for 401, warn for 429), - // so drop the duplicate. logger: { - log: (level, message, ...args) => { + log: (level, message, ...args: unknown[]) => { + // The api-key plugin logs every verification failure at error level — a + // stale key or a throttled caller included. The /mcp handler already logs + // the response it returns at the right level (debug for 401, warn for 429), + // so drop the duplicate. if (message.startsWith("Failed to validate API key")) return; - console[level](`[better-auth] ${message}`, ...args); + // "Failed to parse state" is user/browser behavior: a replayed OAuth + // callback URL (back button, restored tab), or a consent screen left + // open past the state's lifetime. The request already redirects the + // user to an error page; nothing here is on-call actionable. + const effectiveLevel = + level === "error" && message === "Failed to parse state" + ? "warn" + : level; + // Also drops Better Auth's ISO-timestamp prefix, which makes every log + // line fingerprint as its own error group in observability. + console[effectiveLevel === "debug" ? "log" : effectiveLevel]( + `[better-auth] ${message}`, + ...args, + ); }, }, ...baseAuthConfig, diff --git a/src/server/mcp/api-key-auth.ts b/src/server/mcp/api-key-auth.ts index 0de319d..31cd333 100644 --- a/src/server/mcp/api-key-auth.ts +++ b/src/server/mcp/api-key-auth.ts @@ -103,6 +103,7 @@ export async function handleMcpApiKeyRequest( // clients (see lib/auth-api-key.ts). Cloudflare's counter is per-colo // best-effort, which is all this needs to be: credits bound spend, this // bounds runaway request volume. + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- the binding is declared in alchemy.run.ts; env stays unknown through the MCP handler chain const rateLimit = (env as { MCP_RATE_LIMIT?: RateLimit }).MCP_RATE_LIMIT; if (rateLimit) { const { success } = await rateLimit.limit({ key: userId }); diff --git a/src/server/workflows/rankCheckPaths.ts b/src/server/workflows/rankCheckPaths.ts index 2c211cd..0503876 100644 --- a/src/server/workflows/rankCheckPaths.ts +++ b/src/server/workflows/rankCheckPaths.ts @@ -106,7 +106,7 @@ async function checkBatchLive( results.push(outcome.value); return; } - const reason = outcome.reason; + const reason: unknown = outcome.reason; const code = reason instanceof AppError ? reason.code : "UNKNOWN"; const message = reason instanceof Error ? reason.message : String(reason); // DataForSEO erring on its own side is a provider flake, not our bug: the