From ec661819fa622cfbcaa5744cdfcaef8b3e584579 Mon Sep 17 00:00:00 2001 From: Praveen Sampath Date: Wed, 29 Jul 2026 07:50:36 +0000 Subject: [PATCH] fix(web): expose safe PostgreSQL error diagnostics --- web/src/hooks.server.ts | 7 +- web/src/lib/server/error-serializer.test.ts | 98 +++++++++++++++++++ web/src/lib/server/error-serializer.ts | 68 +++++++++++++ web/src/lib/server/logger.ts | 13 +-- web/src/lib/server/oauth/accountLinking.ts | 2 +- web/src/lib/server/oauth/sso-callback.ts | 23 +++-- .../(public)/auth/entra/callback/+server.ts | 5 +- .../(public)/auth/okta/callback/+server.ts | 5 +- 8 files changed, 194 insertions(+), 27 deletions(-) create mode 100644 web/src/lib/server/error-serializer.test.ts create mode 100644 web/src/lib/server/error-serializer.ts diff --git a/web/src/hooks.server.ts b/web/src/hooks.server.ts index f6023ca6e..e69f06834 100644 --- a/web/src/hooks.server.ts +++ b/web/src/hooks.server.ts @@ -107,11 +107,10 @@ const handleLogging: Handle = async ({ event, resolve }) => { logger.info('Request started', { method: event.request.method, - url: event.url.pathname + event.url.search, + url: event.url.pathname, userAgent: event.request.headers.get('user-agent'), ip: event.getClientAddress(), userId: event.locals.user?.id, - userEmail: event.locals.user?.email, }) const response = await resolve(event) @@ -120,7 +119,7 @@ const handleLogging: Handle = async ({ event, resolve }) => { logger.info('Request completed', { method: event.request.method, - url: event.url.pathname + event.url.search, + url: event.url.pathname, status: response.status, duration, userId: event.locals.user?.id, @@ -135,7 +134,7 @@ export const handleError: HandleServerError = ({ error, event }) => { const logger = event.locals.logger || new Logger('error') logger.error('Unhandled server error', error as Error, { - url: event.url.pathname + event.url.search, + url: event.url.pathname, method: event.request.method, userId: event.locals.user?.id, requestId: event.locals.requestId, diff --git a/web/src/lib/server/error-serializer.test.ts b/web/src/lib/server/error-serializer.test.ts new file mode 100644 index 000000000..ac630f25b --- /dev/null +++ b/web/src/lib/server/error-serializer.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from 'vitest' +import { serializeError } from './error-serializer' + +describe('serializeError', () => { + it('includes PostgreSQL failure metadata without messages or query values', () => { + const cause = Object.assign(new Error('database message with super-secret-value'), { + name: 'PostgresError', + code: '23514', + severity: 'ERROR', + schema_name: 'public', + table_name: 'user_oauth_credentials', + column_name: undefined, + type_name: undefined, + constraint_name: 'user_oauth_credentials_refresh_token_encrypted', + routine: 'ExecConstraints', + detail: 'Failing row contains super-secret-profile-data', + query: 'INSERT with super-secret-driver-query', + parameters: ['super-secret-token'], + }) + const error = Object.assign( + new Error('Failed query: INSERT with super-secret-query\nparams: super-secret-token'), + { + query: 'INSERT with super-secret-query', + params: ['super-secret-token'], + cause, + }, + ) + + const serialized = serializeError(error) + + expect(serialized).toMatchObject({ + type: 'Error', + message: 'Database query failed', + database: { + sqlState: '23514', + severity: 'ERROR', + schema: 'public', + table: 'user_oauth_credentials', + constraint: 'user_oauth_credentials_refresh_token_encrypted', + routine: 'ExecConstraints', + }, + }) + + const logged = JSON.stringify(serialized) + expect(logged).not.toContain('super-secret') + expect(logged).not.toContain('params') + expect(logged).not.toContain('detail') + }) + + it('sanitizes query errors without a PostgreSQL cause', () => { + const error = Object.assign(new Error('Failed query: SELECT $1\nparams: secret'), { + query: 'SELECT $1', + params: ['secret'], + cause: Object.assign(new Error('connection failed'), { code: 'ECONNRESET' }), + }) + + const serialized = serializeError(error) + + expect(serialized.message).toBe('Database query failed') + expect(serialized.database).toBeUndefined() + expect(JSON.stringify(serialized)).not.toContain('secret') + }) + + it('extracts metadata from direct PostgreSQL errors', () => { + const error = Object.assign(new Error('duplicate value: super-secret-email'), { + name: 'PostgresError', + code: '23505', + severity: 'ERROR', + table_name: 'users', + constraint_name: 'users_email_key', + }) + + const serialized = serializeError(error) + + expect(serialized).toMatchObject({ + type: 'PostgresError', + message: 'Database query failed', + database: { + sqlState: '23505', + severity: 'ERROR', + table: 'users', + constraint: 'users_email_key', + }, + }) + expect(JSON.stringify(serialized)).not.toContain('super-secret-email') + }) + + it('preserves ordinary error messages and stacks', () => { + const error = new Error('ordinary failure') + + expect(serializeError(error)).toEqual({ + type: 'Error', + message: 'ordinary failure', + stack: error.stack, + database: undefined, + }) + }) +}) diff --git a/web/src/lib/server/error-serializer.ts b/web/src/lib/server/error-serializer.ts new file mode 100644 index 000000000..c5480dbed --- /dev/null +++ b/web/src/lib/server/error-serializer.ts @@ -0,0 +1,68 @@ +export interface SerializedDatabaseError { + sqlState: string + severity?: string + schema?: string + table?: string + column?: string + dataType?: string + constraint?: string + routine?: string +} + +export interface SerializedError { + type: string + message: string + stack?: string + database?: SerializedDatabaseError +} + +type ErrorRecord = Record + +function asErrorRecord(value: unknown): ErrorRecord | null { + return typeof value === 'object' && value !== null ? (value as ErrorRecord) : null +} + +function getString(record: ErrorRecord, key: string): string | undefined { + const value = record[key] + return typeof value === 'string' ? value : undefined +} + +function serializeDatabaseError(cause: unknown): SerializedDatabaseError | undefined { + const record = asErrorRecord(cause) + if (!record) return undefined + + const sqlState = getString(record, 'code') + if (!sqlState || !/^[0-9A-Z]{5}$/.test(sqlState)) return undefined + + return { + sqlState, + severity: getString(record, 'severity'), + schema: getString(record, 'schema_name'), + table: getString(record, 'table_name'), + column: getString(record, 'column_name'), + dataType: getString(record, 'data_type_name') ?? getString(record, 'type_name'), + constraint: getString(record, 'constraint_name'), + routine: getString(record, 'routine'), + } +} + +function stackFrames(stack: string | undefined): string | undefined { + if (!stack) return undefined + + const frames = stack.split('\n').filter((line) => line.trimStart().startsWith('at ')) + return frames.length > 0 ? frames.join('\n') : undefined +} + +export function serializeError(error: Error): SerializedError { + const record = error as Error & ErrorRecord + const isQueryError = typeof record.query === 'string' && Array.isArray(record.params) + const database = serializeDatabaseError(record.cause) ?? serializeDatabaseError(record) + const isDatabaseError = isQueryError || database !== undefined + + return { + type: error.name, + message: isDatabaseError ? 'Database query failed' : error.message, + stack: isDatabaseError ? stackFrames(error.stack) : error.stack, + database, + } +} diff --git a/web/src/lib/server/logger.ts b/web/src/lib/server/logger.ts index a554d6e4f..3d45754f1 100644 --- a/web/src/lib/server/logger.ts +++ b/web/src/lib/server/logger.ts @@ -2,6 +2,7 @@ import pino, { type Logger as PinoLogger } from 'pino' import { env } from '$env/dynamic/private' import { dev } from '$app/environment' import { ulid } from 'ulid' +import { serializeError } from './error-serializer' const logLevel = env.LOG_LEVEL || (dev ? 'debug' : 'info') const logPretty = env.LOG_PRETTY === 'true' || dev @@ -30,21 +31,13 @@ const pinoConfig: pino.LoggerOptions = { }, }, serializers: { - error: (err: Error) => ({ - type: err.name, - message: err.message, - stack: err.stack, - }), + error: serializeError, request: (req: any) => ({ method: req.method, - url: req.url, - headers: req.headers, - query: req.query, - params: req.params, + url: typeof req.url === 'string' ? req.url.split('?')[0] : undefined, }), response: (res: any) => ({ statusCode: res.statusCode, - headers: res.headers, }), }, ...(transport && { transport }), diff --git a/web/src/lib/server/oauth/accountLinking.ts b/web/src/lib/server/oauth/accountLinking.ts index 963ae790c..56ea629c2 100644 --- a/web/src/lib/server/oauth/accountLinking.ts +++ b/web/src/lib/server/oauth/accountLinking.ts @@ -84,7 +84,7 @@ export class AccountLinkingService { if (!isDomainApproved) { throw new Error( - `Registration is not allowed for domain: ${profile.email.split('@')[1]}. ` + + 'Registration is not allowed for this email domain. ' + 'Please contact your administrator to approve your domain.', ) } diff --git a/web/src/lib/server/oauth/sso-callback.ts b/web/src/lib/server/oauth/sso-callback.ts index d4c07e8cf..4323170b1 100644 --- a/web/src/lib/server/oauth/sso-callback.ts +++ b/web/src/lib/server/oauth/sso-callback.ts @@ -4,7 +4,7 @@ import { app } from '$lib/server/config' import { OAuthStateManager } from '$lib/server/oauth/state' import { AccountLinkingService } from '$lib/server/oauth/accountLinking' import { createSession, generateSessionToken, setSessionTokenCookie } from '$lib/server/auth' -import { logger } from '$lib/server/logger' +import { logger, type Logger } from '$lib/server/logger' export interface SsoCallbackOptions { provider: string @@ -12,6 +12,7 @@ export interface SsoCallbackOptions { loadService: () => Promise createService: (ServiceClass: any, config: any, callbackUrl: string) => any callbackPath: string + logger?: Logger } function getErrorRedirect(error: unknown): string { @@ -34,19 +35,22 @@ export async function handleSsoCallback( options: SsoCallbackOptions, ): Promise { const { provider, loadConfig, loadService, createService, callbackPath } = options + const callbackLogger = options.logger ?? logger const code = url.searchParams.get('code') const state = url.searchParams.get('state') const error = url.searchParams.get('error') if (error) { - const errorDescription = url.searchParams.get('error_description') || 'Unknown OAuth error' - logger.error(`${provider} OAuth callback error:`, errorDescription) + callbackLogger.error(`${provider} OAuth callback rejected`, undefined, { + provider, + oauthError: error.slice(0, 100), + }) redirect(302, '/login?error=oauth_error') } if (!code || !state) { - logger.error('Missing required OAuth parameters') + callbackLogger.error('Missing required OAuth parameters', undefined, { provider }) redirect(302, '/login?error=invalid_oauth_response') } @@ -107,12 +111,15 @@ export async function handleSsoCallback( redirectUrl.searchParams.set('linked', provider) } - logger.info( - `${provider} OAuth authentication successful for user: ${user.email} (${user.id})`, - ) + callbackLogger.info(`${provider} OAuth authentication successful`, { + provider, + userId: user.id, + isNewUser, + isLinkedAccount, + }) successUrl = redirectUrl.toString() } catch (error) { - logger.error(`${provider} OAuth callback error:`, error) + callbackLogger.error(`${provider} OAuth callback error`, error, { provider }) redirect(302, getErrorRedirect(error)) } diff --git a/web/src/routes/(public)/auth/entra/callback/+server.ts b/web/src/routes/(public)/auth/entra/callback/+server.ts index e6fa973a8..18eec073d 100644 --- a/web/src/routes/(public)/auth/entra/callback/+server.ts +++ b/web/src/routes/(public)/auth/entra/callback/+server.ts @@ -3,8 +3,8 @@ import { loadEntraOAuthService } from '$lib/server/oauth/entra' import { handleSsoCallback } from '$lib/server/oauth/sso-callback' import type { RequestHandler } from './$types' -export const GET: RequestHandler = async ({ url, cookies }) => { - await handleSsoCallback(url, cookies, { +export const GET: RequestHandler = async ({ url, cookies, locals }) => { + return handleSsoCallback(url, cookies, { provider: 'entra', loadConfig: getEntraAuthConfig, loadService: loadEntraOAuthService, @@ -18,6 +18,7 @@ export const GET: RequestHandler = async ({ url, cookies }) => { callbackUrl, ), callbackPath: '/auth/entra/callback', + logger: locals.logger, }) } diff --git a/web/src/routes/(public)/auth/okta/callback/+server.ts b/web/src/routes/(public)/auth/okta/callback/+server.ts index f09e07bda..36c99fb37 100644 --- a/web/src/routes/(public)/auth/okta/callback/+server.ts +++ b/web/src/routes/(public)/auth/okta/callback/+server.ts @@ -3,8 +3,8 @@ import { loadOktaOAuthService } from '$lib/server/oauth/okta' import { handleSsoCallback } from '$lib/server/oauth/sso-callback' import type { RequestHandler } from './$types' -export const GET: RequestHandler = async ({ url, cookies }) => { - await handleSsoCallback(url, cookies, { +export const GET: RequestHandler = async ({ url, cookies, locals }) => { + return handleSsoCallback(url, cookies, { provider: 'okta', loadConfig: getOktaAuthConfig, loadService: loadOktaOAuthService, @@ -18,6 +18,7 @@ export const GET: RequestHandler = async ({ url, cookies }) => { callbackUrl, ), callbackPath: '/auth/okta/callback', + logger: locals.logger, }) }