From c611c8fd0276aa7d02f8b3231a006689c79afc2a Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 21 Jul 2026 15:02:16 -0400 Subject: [PATCH] chore(core,express): post-release audit follow-ups Bound the refresh-result cache: entries were keyed by the rotating refresh cookie and never looked up again, so the map grew unbounded and retained tokens for the process lifetime. Sweep expired entries (throttled off the hot path) and cap total size as a backstop. Memoize the JWKS key set per auth-server URL in verifySignedAuthResponse. It was rebuilt inside the function on every call, so jose's key cache and refetch cooldown never engaged and every verification made an extra request to the auth server's JWKS endpoint. Make SeamlessAuthUser.email optional and phone nullable to match the cookie payload and the upstream users/me shape. Export redactSensitiveText from core and use it to mask tokens and secrets before the Express router logs an unhandled 5xx error. The utility was tested but wired nowhere, which left a redaction safety net that did not exist. Reorder the magic-link/check cookie requirement out from under the magic-link prefix, throw on a missing route parameter instead of forwarding the literal string undefined, correct the cookieSecret error message that named a removed environment variable, and drop a redundant res.json(...).end() call. --- .changeset/post-release-followups.md | 12 +++++++++ packages/core/README.md | 1 + packages/core/src/ensureCookies.ts | 6 +++-- packages/core/src/index.ts | 1 + packages/core/src/refreshAccessToken.ts | 26 ++++++++++++++++++- packages/core/src/verifySignedAuthResponse.ts | 16 +++++++++++- packages/core/tests/publicExports.test.js | 2 ++ packages/express/README.md | 4 +-- packages/express/src/createServer.ts | 23 ++++++++++++---- packages/express/src/handlers/finishLogin.ts | 2 +- packages/express/src/internal/cookie.ts | 2 +- .../express/tests/routeErrorHandling.test.js | 19 +++++++++++--- 12 files changed, 98 insertions(+), 16 deletions(-) create mode 100644 .changeset/post-release-followups.md diff --git a/.changeset/post-release-followups.md b/.changeset/post-release-followups.md new file mode 100644 index 0000000..bfd170a --- /dev/null +++ b/.changeset/post-release-followups.md @@ -0,0 +1,12 @@ +--- +"@seamless-auth/core": minor +"@seamless-auth/express": minor +--- + +Post-release follow-up cleanups from the pre-release audit. + +- Bound the refresh-result cache in core. Entries were keyed by the rotating refresh cookie and never revisited, so the map grew without limit and retained tokens for the process lifetime. It now sweeps expired entries (throttled) and caps total size. +- Memoize the JWKS key set per auth-server URL in `verifySignedAuthResponse`. It was rebuilt on every call, so jose's key cache and refetch cooldown never engaged and every verification made an extra request to `/.well-known/jwks.json`. +- `SeamlessAuthUser.email` is now optional and `phone` is `string | null`, matching the cookie payload and the upstream `/users/me` shape. This is a type-level change: consumers that treated `phone` as a non-null `string` will need to handle `null`. +- Export `redactSensitiveText` from core and use it to mask tokens and secrets before the Express router logs an unhandled error. +- Reorder the `/magic-link/check` cookie requirement so it is no longer shadowed by `/magic-link`, throw a clear error when a route parameter is missing instead of forwarding the literal string `"undefined"`, correct the `Missing cookieSecret` message that named a removed environment variable, and drop a redundant terminal `.end()` after `res.json(...)`. diff --git a/packages/core/README.md b/packages/core/README.md index 3676893..ce4b0aa 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -91,6 +91,7 @@ Key exports include: - `getSeamlessUser(...)` – resolves the hydrated user, typed as `SeamlessUser | null` - `hasScopedRole(...)` – checks scoped role grants such as `admin:read` - `assertSecretStrength(...)` / `assertSecrets(...)` – enforce the minimum secret length +- `redactSensitiveText(...)` – masks tokens, bearer values, and secrets before logging - `listOAuthProvidersHandler(...)` – retrieves public OAuth provider metadata - `startOAuthLoginHandler(...)` – starts an OAuth authorization-code login - `finishOAuthLoginHandler(...)` – finishes OAuth login and returns cookie instructions diff --git a/packages/core/src/ensureCookies.ts b/packages/core/src/ensureCookies.ts index 60836d8..78ed5a1 100644 --- a/packages/core/src/ensureCookies.ts +++ b/packages/core/src/ensureCookies.ts @@ -108,11 +108,13 @@ const COOKIE_REQUIREMENTS: Record< name: "preAuthCookieName", required: false, }, - "/magic-link": { + // Must precede "/magic-link" for the same prefix-match reason; otherwise this + // entry is shadowed and would silently stop applying if its config diverged. + "/magic-link/check": { name: "preAuthCookieName", required: true, }, - "/magic-link/check": { + "/magic-link": { name: "preAuthCookieName", required: true, }, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a304ec1..4a8bdf0 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -7,6 +7,7 @@ export * from "./getSeamlessUser.js"; export * from "./createServiceToken.js"; export * from "./validateSecrets.js"; export * from "./scopedRoles.js"; +export * from "./redaction.js"; export * from "./handlers/login.js"; export * from "./handlers/finishLogin.js"; diff --git a/packages/core/src/refreshAccessToken.ts b/packages/core/src/refreshAccessToken.ts index beaa9f5..fefb388 100644 --- a/packages/core/src/refreshAccessToken.ts +++ b/packages/core/src/refreshAccessToken.ts @@ -32,6 +32,28 @@ const recentRefreshResults = new Map< { result: RefreshAccessTokenResult; expiresAt: number } >(); const RECENT_REFRESH_RESULT_TTL_MS = 5000; +// Refresh cookies rotate, so a completed entry's key is never looked up again and +// would live forever without this. Sweep expired entries (throttled so it stays off +// the hot path) and cap total size as a backstop against a burst of distinct cookies. +const MAX_RECENT_REFRESH_RESULTS = 10_000; +let lastPruneAt = 0; + +function pruneRecentRefreshResults(now: number): void { + if (now - lastPruneAt >= RECENT_REFRESH_RESULT_TTL_MS) { + for (const [key, entry] of recentRefreshResults) { + if (entry.expiresAt <= now) { + recentRefreshResults.delete(key); + } + } + lastPruneAt = now; + } + + while (recentRefreshResults.size > MAX_RECENT_REFRESH_RESULTS) { + const oldest = recentRefreshResults.keys().next().value; + if (oldest === undefined) break; + recentRefreshResults.delete(oldest); + } +} export async function refreshAccessToken( refreshCookie: string, @@ -82,9 +104,11 @@ export async function refreshAccessToken( try { const result = await refreshPromise; if (result) { + const insertedAt = Date.now(); + pruneRecentRefreshResults(insertedAt); recentRefreshResults.set(refreshCookie, { result, - expiresAt: Date.now() + RECENT_REFRESH_RESULT_TTL_MS, + expiresAt: insertedAt + RECENT_REFRESH_RESULT_TTL_MS, }); } return result; diff --git a/packages/core/src/verifySignedAuthResponse.ts b/packages/core/src/verifySignedAuthResponse.ts index c80e9e7..3a54ecd 100644 --- a/packages/core/src/verifySignedAuthResponse.ts +++ b/packages/core/src/verifySignedAuthResponse.ts @@ -1,5 +1,19 @@ import { createRemoteJWKSet, jwtVerify } from "jose"; +// jose caches keys and applies a refetch cooldown per JWKS instance, so the instance +// must outlive a single call. Memoize per JWKS URL (one per auth server, so the map +// stays tiny) instead of building a fresh, empty-cache instance on every verification. +const jwksByUrl = new Map>(); + +function getJwks(jwksUrl: string): ReturnType { + let jwks = jwksByUrl.get(jwksUrl); + if (!jwks) { + jwks = createRemoteJWKSet(new URL(jwksUrl)); + jwksByUrl.set(jwksUrl, jwks); + } + return jwks; +} + export async function verifySignedAuthResponse( token: string, authServerUrl: string, @@ -7,7 +21,7 @@ export async function verifySignedAuthResponse( ): Promise { try { const jwksUrl = new URL("/.well-known/jwks.json", authServerUrl).toString(); - const JWKS = createRemoteJWKSet(new URL(jwksUrl)); + const JWKS = getJwks(jwksUrl); const { payload } = await jwtVerify(token, JWKS, { algorithms: ["RS256"], diff --git a/packages/core/tests/publicExports.test.js b/packages/core/tests/publicExports.test.js index 4c1665c..97aa943 100644 --- a/packages/core/tests/publicExports.test.js +++ b/packages/core/tests/publicExports.test.js @@ -9,6 +9,7 @@ import { getSeamlessUser, hasScopedRole, listOAuthProvidersHandler, + redactSensitiveText, refreshAccessToken, startOAuthLoginHandler, verifyCookieJwt, @@ -23,6 +24,7 @@ const DOCUMENTED_EXPORTS = { getSeamlessUser, hasScopedRole, listOAuthProvidersHandler, + redactSensitiveText, refreshAccessToken, startOAuthLoginHandler, verifyCookieJwt, diff --git a/packages/express/README.md b/packages/express/README.md index 275cd58..b59f6d4 100644 --- a/packages/express/README.md +++ b/packages/express/README.md @@ -491,8 +491,8 @@ app.get("/api/profile", guard, (req, res) => { { id: string; roles: string[]; - email: string; - phone: string; + email?: string; + phone?: string | null; iat?: number; exp?: number; token?: string; diff --git a/packages/express/src/createServer.ts b/packages/express/src/createServer.ts index 938d83f..eb0705f 100644 --- a/packages/express/src/createServer.ts +++ b/packages/express/src/createServer.ts @@ -26,7 +26,11 @@ import { startOAuthLogin, } from "./handlers/oauth"; import * as admin from "./handlers/admin"; -import { authFetch, AuthFetchOptions } from "@seamless-auth/core"; +import { + authFetch, + AuthFetchOptions, + redactSensitiveText, +} from "@seamless-auth/core"; import { buildProxyServiceAuthorization, buildServiceAuthorization, @@ -99,8 +103,8 @@ export type SeamlessAuthServerOptions = { export interface SeamlessAuthUser { id: string; roles: string[]; - email: string; - phone: string; + email?: string; + phone?: string | null; iat?: number; exp?: number; token?: string; @@ -129,7 +133,11 @@ function buildProxyQueryString(queryInput: Request["query"]): string { function routeParam(req: Request, name: string): string { const value = req.params[name]; - return Array.isArray(value) ? value[0] : value; + const resolved = Array.isArray(value) ? value[0] : value; + if (typeof resolved !== "string") { + throw new Error(`Missing route parameter "${name}"`); + } + return resolved; } function clientErrorStatus(err: unknown): number | null { @@ -683,7 +691,12 @@ export function createSeamlessAuthServer( const status = clientErrorStatus(err) ?? 500; if (status >= 500) { - console.error("[SEAMLESS-AUTH-EXPRESS] - Unhandled route error.", err); + const detail = + err instanceof Error ? (err.stack ?? err.message) : String(err); + console.error( + "[SEAMLESS-AUTH-EXPRESS] - Unhandled route error.", + redactSensitiveText(detail), + ); res.status(500).json({ error: "internal_error" }); return; } diff --git a/packages/express/src/handlers/finishLogin.ts b/packages/express/src/handlers/finishLogin.ts index 92e84c0..0270fba 100644 --- a/packages/express/src/handlers/finishLogin.ts +++ b/packages/express/src/handlers/finishLogin.ts @@ -52,5 +52,5 @@ export async function finishLogin( return res.status(result.status).json(result.error); } - res.status(result.status).json(result.body).end(); + res.status(result.status).json(result.body); } diff --git a/packages/express/src/internal/cookie.ts b/packages/express/src/internal/cookie.ts index bf8c97d..cc6db89 100644 --- a/packages/express/src/internal/cookie.ts +++ b/packages/express/src/internal/cookie.ts @@ -35,7 +35,7 @@ export function buildCookieSigner( opts: CookieSecurityOptions, ): CookieSignerOptions { if (!opts.cookieSecret) { - throw new Error("Missing COOKIE_SIGNING_KEY"); + throw new Error("Missing cookieSecret"); } return { diff --git a/packages/express/tests/routeErrorHandling.test.js b/packages/express/tests/routeErrorHandling.test.js index 406c59f..f7b1208 100644 --- a/packages/express/tests/routeErrorHandling.test.js +++ b/packages/express/tests/routeErrorHandling.test.js @@ -115,8 +115,7 @@ describe("route error handling", () => { }); it("logs the underlying error server side", async () => { - const cause = new Error("network down"); - global.fetch.mockRejectedValue(cause); + global.fetch.mockRejectedValue(new Error("network down")); await request(createApp()) .get("/auth/magic-link/check") @@ -124,7 +123,21 @@ describe("route error handling", () => { expect(errorSpy).toHaveBeenCalledWith( "[SEAMLESS-AUTH-EXPRESS] - Unhandled route error.", - cause, + expect.stringContaining("network down"), ); }); + + it("redacts sensitive values in the logged error", async () => { + global.fetch.mockRejectedValue( + new Error("upstream rejected Bearer sk-secret-token-value"), + ); + + await request(createApp()) + .get("/auth/magic-link/check") + .set("Cookie", createPreAuthCookie()); + + const [, logged] = errorSpy.mock.calls[0]; + expect(logged).not.toContain("sk-secret-token-value"); + expect(logged).toContain("[REDACTED]"); + }); });