diff --git a/wallet-gateway/remote/src/init.ts b/wallet-gateway/remote/src/init.ts index ff57ead87..7cc50f421 100644 --- a/wallet-gateway/remote/src/init.ts +++ b/wallet-gateway/remote/src/init.ts @@ -50,6 +50,7 @@ import { Env } from './env.js' import { SigningWorker } from './signing/signing-worker.js' import { apiKeyAuth } from './middleware/apiKeyAuth.js' import { securityHeaders } from './middleware/securityHeaders.js' +import { errorHandler } from './middleware/errorHandler.js' let isReady = false let signingWorker: SigningWorker | undefined @@ -375,9 +376,7 @@ export async function initialize(opts: CliOptions, logger: Logger) { ], } - app.use( - '/api/*splat', - express.json(), + const apiMiddleware = [ preAuthRateLimit, apiKeyAuth( store, @@ -390,8 +389,11 @@ export async function initialize(opts: CliOptions, logger: Logger) { store, allowedPaths, logger.child({ component: 'SessionHandler' }) - ) - ) + ), + ] + + app.use(config.server.userPath, ...apiMiddleware) + app.use(config.server.dappPath, ...apiMiddleware) logger.info({ ...config.server, port }, 'Server configuration') @@ -440,8 +442,20 @@ export async function initialize(opts: CliOptions, logger: Logger) { config.server.admin ) + const { userPath, dappPath } = config.server + const isApiPath = (path: string) => + path === userPath || + path === dappPath || + path.startsWith(`${userPath}/`) || + path.startsWith(`${dappPath}/`) + // register web handler - web(app, server, userApiUrl, dappApiUrl) + web(app, server, userApiUrl, dappApiUrl, isApiPath) + + app.use( + errorHandler(logger.child({ component: 'ErrorHandler' }), isApiPath) + ) + isReady = true logger.info( diff --git a/wallet-gateway/remote/src/middleware/errorHandler.test.ts b/wallet-gateway/remote/src/middleware/errorHandler.test.ts new file mode 100644 index 000000000..dc3dadfae --- /dev/null +++ b/wallet-gateway/remote/src/middleware/errorHandler.test.ts @@ -0,0 +1,138 @@ +// Copyright (c) 2025-2026 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { NextFunction, Request, Response } from 'express' +import { pino } from 'pino' +import { sink } from 'pino-test' +import { providerErrors, rpcErrors } from '@canton-network/core-rpc-errors' +import { errorHandler } from './errorHandler.js' + +describe('errorHandler', () => { + const logger = pino({ level: 'silent' }, sink()) + const isApiPath = (path: string) => path.startsWith('/api/') + + let next: NextFunction + let status: ReturnType + let json: ReturnType + + beforeEach(() => { + next = vi.fn() as NextFunction + status = vi.fn().mockReturnThis() + json = vi.fn() + }) + + function makeReq(partial: Partial = {}): Request { + return { + path: '/api/v0/user', + body: { id: 1 }, + ...partial, + } as Request + } + + function makeRes(headersSent = false): Response { + return { status, json, headersSent } as unknown as Response + } + + it('maps a JsonRpcError to its HTTP status and keeps the message', () => { + const err = providerErrors.unauthorized({ + message: 'User is not connected', + }) + + errorHandler(logger, isApiPath)(err, makeReq(), makeRes(), next) + + expect(status).toHaveBeenCalledWith(401) + expect(json).toHaveBeenCalledWith({ + jsonrpc: '2.0', + id: 1, + error: { + code: providerErrors.unauthorized().code, + message: 'User is not connected', + }, + }) + }) + + it('replaces an unexpected error with a generic JSON-RPC 500 on API paths', () => { + const err = new Error('connect ECONNREFUSED 127.0.0.1:5432') + + errorHandler(logger, isApiPath)(err, makeReq(), makeRes(), next) + + expect(status).toHaveBeenCalledWith(500) + expect(json).toHaveBeenCalledWith({ + jsonrpc: '2.0', + id: 1, + error: { + code: rpcErrors.internal().code, + message: 'Something went wrong', + }, + }) + }) + + it('never sends the stack trace to the client', () => { + const err = new Error('internal detail') + + errorHandler(logger, isApiPath)(err, makeReq(), makeRes(), next) + + const body = JSON.stringify(json.mock.calls[0][0]) + expect(body).not.toContain('internal detail') + expect(body).not.toContain('at ') // stack trace + }) + + it('keeps the 413 from express.json() for err.status', () => { + const err = Object.assign(new Error('request too large'), { + status: 413, + }) + + errorHandler(logger, isApiPath)(err, makeReq(), makeRes(), next) + + expect(status).toHaveBeenCalledWith(413) + expect(json).toHaveBeenCalledWith({ error: 'Payload Too Large' }) + }) + + it('keeps the 413 from express.json() for err.statusCode', () => { + const err = Object.assign(new Error('request too large'), { + statusCode: 413, + }) + + errorHandler(logger, isApiPath)(err, makeReq(), makeRes(), next) + + expect(status).toHaveBeenCalledWith(413) + expect(json).toHaveBeenCalledWith({ error: 'Payload Too Large' }) + }) + + it('leaves an error without a 413 status as a generic 500', () => { + const err = Object.assign(new Error('not a 413 error'), { + status: 404, + }) + + errorHandler(logger, isApiPath)(err, makeReq(), makeRes(), next) + + expect(status).toHaveBeenCalledWith(500) + }) + + it('returns a generic error body for non-API paths', () => { + const req = makeReq({ path: '/login' }) + + errorHandler(logger, isApiPath)( + new Error('internal detail'), + req, + makeRes(), + next + ) + + expect(status).toHaveBeenCalledWith(500) + expect(json).toHaveBeenCalledWith({ error: 'Internal Server Error' }) + }) + + it('delegates to express when the response has already started', () => { + const err = new Error( + 'error that some middleware started res on, but still passed error down' + ) + + errorHandler(logger, isApiPath)(err, makeReq(), makeRes(true), next) + + expect(next).toHaveBeenCalledWith(err) + expect(status).not.toHaveBeenCalled() + expect(json).not.toHaveBeenCalled() + }) +}) diff --git a/wallet-gateway/remote/src/middleware/errorHandler.ts b/wallet-gateway/remote/src/middleware/errorHandler.ts new file mode 100644 index 000000000..0738c458e --- /dev/null +++ b/wallet-gateway/remote/src/middleware/errorHandler.ts @@ -0,0 +1,78 @@ +// Copyright (c) 2025-2026 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import type { NextFunction, Request, Response } from 'express' +import { Logger } from 'pino' +import { + JsonRpcError, + rpcErrors, + toHttpErrorCode, +} from '@canton-network/core-rpc-errors' +import { jsonRpcResponse } from '@canton-network/core-rpc-transport' + +const isPayloadTooLargeError = (err: unknown): boolean => { + if (typeof err !== 'object' || err === null) { + return false + } + + const { status, statusCode } = err as { + status?: unknown + statusCode?: unknown + } + + return status === 413 || statusCode === 413 +} + +// Catches unhandled errors and prevents internal details like stack trace from reaching end user +export function errorHandler( + logger: Logger, + isApiPath: (path: string) => boolean +) { + return ( + err: unknown, + req: Request, + res: Response, + next: NextFunction + ): void => { + // Full error with stack goes to logs only. + logger.error({ err }, 'Unhandled request error') + + // If the response has already started, we can't safely send an error response. + if (res.headersSent) { + next(err) + return + } + + if (isPayloadTooLargeError(err)) { + res.status(413).json({ error: 'Payload Too Large' }) + return + } + + // jsonRpcHandler already maps controllers errors via handleRpcError. + // This only runs for errors that escape earlier middlewares (e.g. auth/session checks). + if (isApiPath(req.path)) { + const id = req.body?.id ?? null + + if (err instanceof JsonRpcError) { + res.status(toHttpErrorCode(err.code)).json( + jsonRpcResponse(id, { + error: { code: err.code, message: err.message }, + }) + ) + return + } + + res.status(500).json( + jsonRpcResponse(id, { + error: { + code: rpcErrors.internal().code, + message: 'Something went wrong', + }, + }) + ) + return + } + + res.status(500).json({ error: 'Internal Server Error' }) + } +} diff --git a/wallet-gateway/remote/src/middleware/jsonRpcHandler.test.ts b/wallet-gateway/remote/src/middleware/jsonRpcHandler.test.ts index 4eb8d0cf4..c82000a57 100644 --- a/wallet-gateway/remote/src/middleware/jsonRpcHandler.test.ts +++ b/wallet-gateway/remote/src/middleware/jsonRpcHandler.test.ts @@ -228,12 +228,31 @@ describe('handleRpcError', () => { expect(body).toEqual({ jsonrpc: '2.0', id: 99, - error: err, + error: { code: err.code, message: 'bad' }, }) expect(errorLog).not.toHaveBeenCalled() }) - it('uses generic method-specific message for non-JsonRpcError then replaces with Error.message', () => { + it('keeps the JsonRpcError message once the response is serialised', () => { + const err = rpcErrors.invalidParams({ message: 'error description' }) + const [, body] = handleRpcError(err, 99) + + expect(JSON.parse(JSON.stringify(body))).toMatchObject({ + error: { code: err.code, message: 'error description' }, + }) + }) + + it('forwards data that was deliberately attached to a JsonRpcError', () => { + const err = rpcErrors.invalidParams({ + message: 'bad', + data: { field: 'value' }, + }) + const [, body] = handleRpcError(err, 1) + + expect(errorPayload(body)).toMatchObject({ data: { field: 'value' } }) + }) + + it('forwards the Error message but not the error object itself', () => { const [status, body] = handleRpcError( new Error('some error'), 'id', @@ -244,12 +263,24 @@ describe('handleRpcError', () => { expect(body).toEqual({ jsonrpc: '2.0', id: 'id', - error: expect.objectContaining({ + error: { code: rpcErrors.internal().code, message: 'some error', - data: expect.any(Error), - }), + }, + }) + }) + + it('does not leak enumerable properties of runtime errors', () => { + const dbError = Object.assign(new Error('connect ECONNREFUSED'), { + code: 'ECONNREFUSED', + address: '127.0.0.1', + port: 5432, }) + + const [, body] = handleRpcError(dbError, 1, 'listWallets') + + expect(errorPayload(body)).not.toHaveProperty('data') + expect(JSON.stringify(body)).not.toContain('5432') }) it('uses generic message when method name is omitted', () => { @@ -307,10 +338,9 @@ describe('handleRpcError', () => { const [status, body] = handleRpcError({ foo: 'bar' }, 2, 'wrongMethod') expect(status).toBe(500) - expect(errorPayload(body)).toMatchObject({ + expect(errorPayload(body)).toEqual({ code: rpcErrors.internal().code, message: 'Something went wrong while calling wrongMethod', - data: { foo: 'bar' }, }) }) }) diff --git a/wallet-gateway/remote/src/middleware/jsonRpcHandler.ts b/wallet-gateway/remote/src/middleware/jsonRpcHandler.ts index c9acc145a..1e84bf315 100644 --- a/wallet-gateway/remote/src/middleware/jsonRpcHandler.ts +++ b/wallet-gateway/remote/src/middleware/jsonRpcHandler.ts @@ -25,7 +25,6 @@ interface JsonRpcHttpOptions { * Handles JSON-RPC errors and maps them to HTTP responses. * @param error The error that occurred. * @param id The JSON-RPC request ID. - * @param logger The logger instance. * @param method The name of the JSON-RPC method being called. * @returns A tuple containing the HTTP status code and the JSON-RPC response. */ @@ -42,12 +41,15 @@ export const handleRpcError = ( error: { ...rpcErrors.internal(), message: genericMessage, - data: error, }, } if (error instanceof JsonRpcError) { - response.error = error + response.error = { + code: error.code, + message: error.message, + data: error.data, + } const httpCode = toHttpErrorCode(error.code) return [httpCode, jsonRpcResponse(id, response)] } @@ -164,8 +166,9 @@ export const jsonRpcHandler = method ) + // Full error with callstack in logs, sanitized version in response logger.error( - { response }, + { err: error, response }, 'RPC response: error with response' ) res.status(status).json(response) diff --git a/wallet-gateway/remote/src/middleware/jwtAuth.test.ts b/wallet-gateway/remote/src/middleware/jwtAuth.test.ts index d7835e81f..a791e269f 100644 --- a/wallet-gateway/remote/src/middleware/jwtAuth.test.ts +++ b/wallet-gateway/remote/src/middleware/jwtAuth.test.ts @@ -4,6 +4,7 @@ import { describe, expect, it, vi, beforeEach } from 'vitest' import type { Request, Response, NextFunction } from 'express' import { jwtAuth } from './jwtAuth.js' +import { providerErrors } from '@canton-network/core-rpc-errors' import { pino } from 'pino' import { sink } from 'pino-test' @@ -27,6 +28,7 @@ describe('jwtAuth', () => { partial: Partial & { headers?: { authorization?: string } query?: Record + body?: { id?: number } } ): Request { return { @@ -89,11 +91,12 @@ describe('jwtAuth', () => { expect(next).toHaveBeenCalledOnce() }) - it('returns 401 JSON when verification throws', async () => { + it('returns a JSON-RPC 401 when verification throws', async () => { verifyToken.mockRejectedValue(new Error('bad sig')) const req = makeReq({ headers: { authorization: 'Bearer x' }, + body: { id: 7 }, }) const res = makeRes() const middleware = jwtAuth(authService, logger) @@ -103,12 +106,21 @@ describe('jwtAuth', () => { expect(next).not.toHaveBeenCalled() expect(status).toHaveBeenCalledWith(401) expect(json).toHaveBeenCalledWith({ - error: 'Invalid or expired token: bad sig', + jsonrpc: '2.0', + id: 7, + error: { + code: providerErrors.unauthorized().code, + message: 'Invalid or expired token', + }, }) }) - it('stringifies non-Error rejection values in the response', async () => { - verifyToken.mockRejectedValue('rejected') + it('does not leak the underlying verification failure to the client', async () => { + verifyToken.mockRejectedValue( + new Error( + 'signature doesnt match the secret which is leaked-secret-123' + ) + ) const req = makeReq({ headers: { authorization: 'Bearer x' }, @@ -118,8 +130,8 @@ describe('jwtAuth', () => { await middleware(req, res, next) - expect(json).toHaveBeenCalledWith({ - error: 'Invalid or expired token: rejected', - }) + const body = JSON.stringify(json.mock.calls[0][0]) + expect(body).not.toContain('leaked-secret-123') + expect(body).toContain('Invalid or expired token') }) }) diff --git a/wallet-gateway/remote/src/middleware/jwtAuth.ts b/wallet-gateway/remote/src/middleware/jwtAuth.ts index 3492f331f..0b0e853e3 100644 --- a/wallet-gateway/remote/src/middleware/jwtAuth.ts +++ b/wallet-gateway/remote/src/middleware/jwtAuth.ts @@ -3,6 +3,8 @@ import type { Request, Response, NextFunction } from 'express' import { AuthService } from '@canton-network/core-wallet-auth' +import { providerErrors } from '@canton-network/core-rpc-errors' +import { jsonRpcResponse } from '@canton-network/core-rpc-transport' import { Logger } from 'pino' export function jwtAuth(authService: AuthService, logger: Logger) { @@ -22,11 +24,16 @@ export function jwtAuth(authService: AuthService, logger: Logger) { req.authContext = context next() } catch (err) { + // Reason stays in the logs, the client sees the token was rejected. logger.warn({ err }, 'JWT verification failed') - const message = err instanceof Error ? err.message : String(err) - res.status(401).json({ - error: 'Invalid or expired token: ' + message, - }) + res.status(401).json( + jsonRpcResponse(req.body?.id ?? null, { + error: { + code: providerErrors.unauthorized().code, + message: 'Invalid or expired token', + }, + }) + ) } } } diff --git a/wallet-gateway/remote/src/middleware/sessionHandler.test.ts b/wallet-gateway/remote/src/middleware/sessionHandler.test.ts index b85db0b3b..6942bf38e 100644 --- a/wallet-gateway/remote/src/middleware/sessionHandler.test.ts +++ b/wallet-gateway/remote/src/middleware/sessionHandler.test.ts @@ -7,6 +7,7 @@ import type { AuthAware, AuthContext } from '@canton-network/core-wallet-auth' import { pino } from 'pino' import { sink } from 'pino-test' import { sessionHandler } from './sessionHandler.js' +import { providerErrors } from '@canton-network/core-rpc-errors' import { Store } from '@canton-network/core-wallet-store' describe('sessionHandler', () => { @@ -42,7 +43,7 @@ describe('sessionHandler', () => { partial: Partial & { method?: string baseUrl?: string - body?: { method?: string } + body?: { method?: string; id?: number } authContext?: AuthContext } ): Request { @@ -127,10 +128,49 @@ describe('sessionHandler', () => { expect(next).not.toHaveBeenCalled() expect(status).toHaveBeenCalledWith(401) expect(json).toHaveBeenCalledWith({ - error: 'No active session found', + jsonrpc: '2.0', + id: null, + error: { + code: providerErrors.unauthorized().code, + message: 'No active session found', + }, }) }) + it('returns 401 without calling getSession when no access token is present', async () => { + const req = makeReq({ + body: { method: 'listWallets' }, + authContext: undefined, + }) + const res = makeRes() + const middleware = sessionHandler(store, allowedPaths, logger) + + await middleware(req, res, next) + + expect(getSession).not.toHaveBeenCalled() + expect(next).not.toHaveBeenCalled() + expect(status).toHaveBeenCalledWith(401) + expect(json).toHaveBeenCalledWith({ + jsonrpc: '2.0', + id: null, + error: { + code: providerErrors.unauthorized().code, + message: 'No active session found', + }, + }) + }) + + it('keeps the JSON-RPC request id in the 401 response', async () => { + getSession.mockResolvedValue(undefined) + const req = makeReq({ body: { method: 'listWallets', id: 42 } }) + const res = makeRes() + const middleware = sessionHandler(store, allowedPaths, logger) + + await middleware(req, res, next) + + expect(json).toHaveBeenCalledWith(expect.objectContaining({ id: 42 })) + }) + it('requires a session when the path is not in the allow list config', async () => { getSession.mockResolvedValue(undefined) const req = makeReq({ diff --git a/wallet-gateway/remote/src/middleware/sessionHandler.ts b/wallet-gateway/remote/src/middleware/sessionHandler.ts index 0a6e40b05..9052d895e 100644 --- a/wallet-gateway/remote/src/middleware/sessionHandler.ts +++ b/wallet-gateway/remote/src/middleware/sessionHandler.ts @@ -3,6 +3,8 @@ import type { Request, Response, NextFunction } from 'express' import { AuthAware } from '@canton-network/core-wallet-auth' +import { providerErrors } from '@canton-network/core-rpc-errors' +import { jsonRpcResponse } from '@canton-network/core-rpc-transport' import { Logger } from 'pino' import { Store } from '@canton-network/core-wallet-store' @@ -26,8 +28,10 @@ export function sessionHandler( logger.debug( `Skipping authentication for ${req.method} request to ${req.baseUrl}` ) - next() - } else if ( + return next() + } + + if ( allowedMethods && (allowedMethods.includes(req.body.method) || allowedMethods.includes('*')) @@ -35,18 +39,39 @@ export function sessionHandler( logger.debug( `Allowing unauthenticated access to ${req.baseUrl} for method ${req.body.method}` ) - next() - } else { - logger.debug('Checking for active session for ' + context?.userId) - const session = await store - .withAuthContext(context) - .getSession(context?.accessToken || '') - if (!session) { - logger.debug('No active session found for ' + context?.userId) - res.status(401).json({ error: 'No active session found' }) - } else { - next() - } + return next() + } + + const reqId = req.body?.id ?? null + + if (!context?.accessToken) { + logger.debug('No access token provided for protected method') + return res.status(401).json( + jsonRpcResponse(reqId, { + error: { + code: providerErrors.unauthorized().code, + message: 'No active session found', + }, + }) + ) } + + logger.debug('Checking for active session for ' + context.userId) + const session = await store + .withAuthContext(context) + .getSession(context.accessToken) + if (!session) { + logger.debug('No active session found for ' + context.userId) + return res.status(401).json( + jsonRpcResponse(reqId, { + error: { + code: providerErrors.unauthorized().code, + message: 'No active session found', + }, + }) + ) + } + + next() } } diff --git a/wallet-gateway/remote/src/web/frontend/rpc-client.test.ts b/wallet-gateway/remote/src/web/frontend/rpc-client.test.ts index 806af1caa..6a9a4a29b 100644 --- a/wallet-gateway/remote/src/web/frontend/rpc-client.test.ts +++ b/wallet-gateway/remote/src/web/frontend/rpc-client.test.ts @@ -128,7 +128,7 @@ describe('rpc-client', () => { it('createUserClient uses userPath from gateway config', async () => { const customUserPath = `${window.location.origin}/api/v0/custom-user` fetchMock.mockResolvedValue( - new Response(JSON.stringify({ userPath: customUserPath }), { + new Response(JSON.stringify({ userApiUrl: customUserPath }), { status: 200, }) ) diff --git a/wallet-gateway/remote/src/web/frontend/rpc-client.ts b/wallet-gateway/remote/src/web/frontend/rpc-client.ts index 6a2ff117b..b049b4b04 100644 --- a/wallet-gateway/remote/src/web/frontend/rpc-client.ts +++ b/wallet-gateway/remote/src/web/frontend/rpc-client.ts @@ -23,7 +23,7 @@ export function resetRpcClientCachesForTests(): void { } const getUserApiPath = async (): Promise => { - const defaultUserPath = new URL( + const defaultUserUrl = new URL( toRelPath('/api/v0/user'), window.location.origin ) @@ -34,14 +34,14 @@ const getUserApiPath = async (): Promise => { ) .then((response) => response.json()) .then((config) => - config?.userPath ? new URL(config.userPath) : defaultUserPath + config?.userApiUrl ? new URL(config.userApiUrl) : defaultUserUrl ) .catch((error) => { console.warn( - 'Failed to fetch userPath from config, using default', + 'Failed to fetch userApiUrl from config, using default', error ) - return defaultUserPath + return defaultUserUrl }) } return userApiPathPromise diff --git a/wallet-gateway/remote/src/web/server.ts b/wallet-gateway/remote/src/web/server.ts index 314a4462f..a02b41473 100644 --- a/wallet-gateway/remote/src/web/server.ts +++ b/wallet-gateway/remote/src/web/server.ts @@ -11,12 +11,13 @@ import { GATEWAY_VERSION } from '../version.js' export const web = ( app: express.Express, server: Server, - userPath: string, - dappApiUrl: string + userApiUrl: string, + dappApiUrl: string, + isApiPath: (path: string) => boolean ) => { // Expose API URLs via well-known configuration endpoint app.get('/.well-known/wallet-gateway-config', (_req, res) => { - res.json({ userPath, dappApiUrl }) + res.json({ userApiUrl, dappApiUrl }) }) if (process.env.NODE_ENV === 'development') { // Enable live reloading and Vite dev server for frontend in development @@ -44,7 +45,7 @@ export const web = ( if ( req.method !== 'GET' || req.path.length <= 1 || // Skip root path - req.path.startsWith('/api') || // Ignore API routes + isApiPath(req.path) || // Ignore API routes req.path.endsWith('/') || // Path already ends with a slash req.path.includes('.') || // Ignore paths with file extensions req.path.includes('@vite') // Ignore Vite dev server paths