diff --git a/src/__tests__/api/master/generateWallet.test.ts b/src/__tests__/api/master/generateWallet.test.ts index 206c2a6b..c049f0e9 100644 --- a/src/__tests__/api/master/generateWallet.test.ts +++ b/src/__tests__/api/master/generateWallet.test.ts @@ -9,6 +9,7 @@ import { AppMode, MasterExpressConfig, TlsMode } from '../../../shared/types'; import { BitgoMpcGpgPubKeys, Environments } from '@bitgo-beta/sdk-core'; import { BitGoAPI } from '@bitgo-beta/sdk-api'; import * as middleware from '../../../shared/middleware'; +import logger from '../../../shared/logger'; import { BitGoRequest } from '../../../types/request'; import { BitGoAPITestHarness, DEFAULT_ASYNC_MODE_CONFIG } from './testUtils'; @@ -347,6 +348,118 @@ describe('POST /api/v1/:coin/advancedwallet/generate', () => { bitgoAddWalletNock.done(); }); + it('returns a descriptive 500 error body when the wallet/add response violates the response schema', async () => { + // WCN-2395 leaves the wallet creator at view-only, and the wallet/add response can + // come back without fields MBE declares as required. The wallet is still created on + // the backend, but the response payload fails its io-ts codec check in sendEncoded(). + // typed-express-router's default encodeErrorFormatter would swallow the validation + // error and reply with a bare HTTP 500 and an empty JSON body. + const loggerErrorSpy = sinon.spy(logger, 'error'); + + const userKeychainNock = nock(advancedWalletManagerUrl) + .post(`/api/${coin}/key/independent`, { + source: 'user', + }) + .reply(200, { + pub: validUserPub, + source: 'user', + type: 'independent', + }); + + const backupKeychainNock = nock(advancedWalletManagerUrl) + .post(`/api/${coin}/key/independent`, { + source: 'backup', + }) + .reply(200, { + pub: validBackupPub, + source: 'backup', + type: 'independent', + }); + + const bitgoAddUserKeyNock = nock(bitgoApiUrl) + .post(`/api/v2/${coin}/key`, { + pub: validUserPub, + keyType: 'independent', + source: 'user', + }) + .matchHeader('any', () => true) + .reply(200, { id: 'user-key-id', pub: validUserPub, source: 'user', type: 'independent' }); + + const bitgoAddBackupKeyNock = nock(bitgoApiUrl) + .post(`/api/v2/${coin}/key`, { + pub: validBackupPub, + keyType: 'independent', + source: 'backup', + }) + .matchHeader('any', () => true) + .reply(200, { + id: 'backup-key-id', + pub: validBackupPub, + source: 'backup', + type: 'independent', + }); + + const bitgoAddBitGoKeyNock = nock(bitgoApiUrl) + .post(`/api/v2/${coin}/key`, { + source: 'bitgo', + enterprise: 'test_enterprise', + }) + .reply(200, { + id: 'bitgo-key-id', + pub: 'xpub_bitgo', + source: 'bitgo', + type: 'independent', + isBitGo: true, + isTrust: false, + hsmType: 'institutional', + }); + + // JSON.stringify drops undefined properties, so the wallet/add response omits + // the required `users` field, violating MBE's GenerateWalletResponse schema + const schemaViolatingWallet = mockWalletResponse('new-wallet-id', coin, { + users: undefined, + }); + + const bitgoAddWalletNock = nock(bitgoApiUrl) + .post(`/api/v2/${coin}/wallet/add`, { + label: 'test_wallet', + enterprise: 'test_enterprise', + m: 2, + n: 3, + keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], + type: 'advanced', + }) + .matchHeader('any', () => true) + .reply(200, schemaViolatingWallet); + + const response = await agent + .post(`/api/v1/${coin}/advancedwallet/generate`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + label: 'test_wallet', + enterprise: 'test_enterprise', + multisigType: 'onchain', + }); + + // The response is rejected (500) because the payload does not match the declared + // schema, but the error must be descriptive instead of an empty body. + response.status.should.equal(500); + response.body.error.should.equal('Internal Server Error'); + response.body.details.should.containEql('response does not match expected type'); + + // The real validation error is logged server-side for debugging. + assert.equal(loggerErrorSpy.callCount, 1); + loggerErrorSpy.firstCall.args[0].should.containEql('Failed to encode API response'); + loggerErrorSpy.firstCall.args[0].should.containEql(`/api/v1/${coin}/advancedwallet/generate`); + + userKeychainNock.done(); + backupKeychainNock.done(); + bitgoAddUserKeyNock.done(); + bitgoAddBackupKeyNock.done(); + bitgoAddBitGoKeyNock.done(); + bitgoAddWalletNock.done(); + }); + it('should generate a TSS MPC v1 (EdDSA) wallet with separate backup AWM', async () => { const backupAwmUrl = 'http://backup-awm.invalid'; diff --git a/src/__tests__/errorFormatters.test.ts b/src/__tests__/errorFormatters.test.ts new file mode 100644 index 00000000..f2cca6fe --- /dev/null +++ b/src/__tests__/errorFormatters.test.ts @@ -0,0 +1,49 @@ +import 'should'; +import assert from 'assert'; +import sinon from 'sinon'; +import { WrappedRequest } from '@api-ts/typed-express-router'; +import logger from '../shared/logger'; +import { customEncodeErrorFormatter } from '../shared/errorFormatters'; + +function makeFakeRequest(): WrappedRequest { + return { + method: 'POST', + originalUrl: '/api/v1/tbtc/advancedwallet/generate', + apiName: 'v1.wallet.generate', + } as unknown as WrappedRequest; +} + +describe('customEncodeErrorFormatter', () => { + afterEach(() => { + sinon.restore(); + }); + + it('logs the real validation error with request context and stack, and returns it in the response body', () => { + const loggerErrorSpy = sinon.spy(logger, 'error'); + const req = makeFakeRequest(); + const err = new Error('response does not match expected type SomeResponseCodec'); + + const body = customEncodeErrorFormatter(err, req) as Record; + + assert.equal(body.error, 'Internal Server Error'); + assert.equal(body.details, 'response does not match expected type SomeResponseCodec'); + + assert.equal(loggerErrorSpy.callCount, 1); + const logged = loggerErrorSpy.firstCall.args[0] as unknown as string; + logged.should.containEql( + 'Failed to encode API response for POST /api/v1/tbtc/advancedwallet/generate (api: v1.wallet.generate)', + ); + logged.should.containEql('response does not match expected type SomeResponseCodec'); + // The stack trace points at the codec check that failed + logged.should.containEql('Error: response does not match expected type SomeResponseCodec'); + }); + + it('falls back to String() for non-Error throwables', () => { + const req = makeFakeRequest(); + + const body = customEncodeErrorFormatter('not an Error', req) as Record; + + assert.equal(body.error, 'Internal Server Error'); + assert.equal(body.details, 'not an Error'); + }); +}); diff --git a/src/advancedWalletManager/routers/advancedWalletManagerApiSpec.ts b/src/advancedWalletManager/routers/advancedWalletManagerApiSpec.ts index 958ce036..f7659fc5 100644 --- a/src/advancedWalletManager/routers/advancedWalletManagerApiSpec.ts +++ b/src/advancedWalletManager/routers/advancedWalletManagerApiSpec.ts @@ -35,7 +35,10 @@ import { ecdsaMPCv2Finalize } from '../handlers/ecdsaMPCV2WalletGenerationFinali import { ecdsaMPCv2Recovery } from '../handlers/ecdsaMPCV2Recovery'; import { signEddsaRecoveryTransaction } from '../handlers/eddsaMPCRecovery'; import { isEddsaCoin } from '../../shared/coinUtils'; -import { customDecodeErrorFormatter } from '../../shared/errorFormatters'; +import { + customDecodeErrorFormatter, + customEncodeErrorFormatter, +} from '../../shared/errorFormatters'; // Request type for /key/independent endpoint const IndependentKeyRequest = { @@ -512,6 +515,7 @@ export function createKeyGenRouter( ): WrappedRouter { const router = createRouter(AdvancedWalletManagerApiSpec, { decodeErrorFormatter: customDecodeErrorFormatter, + encodeErrorFormatter: customEncodeErrorFormatter, }); // Add middleware router.use(express.json()); diff --git a/src/advancedWalletManager/routers/healthCheck.ts b/src/advancedWalletManager/routers/healthCheck.ts index d0462bcd..4e1666fe 100644 --- a/src/advancedWalletManager/routers/healthCheck.ts +++ b/src/advancedWalletManager/routers/healthCheck.ts @@ -4,7 +4,10 @@ import { Response } from '@api-ts/response'; import pjson from '../../../package.json'; import { responseHandler } from '../../shared/middleware'; import { PingResponseType, VersionResponseType } from '../../types/health'; -import { customDecodeErrorFormatter } from '../../shared/errorFormatters'; +import { + customDecodeErrorFormatter, + customEncodeErrorFormatter, +} from '../../shared/errorFormatters'; // API Response types const PingResponse: HttpResponse = { @@ -60,6 +63,7 @@ export const HealthCheckApiSpec = apiSpec({ export function createHealthCheckRouter(): WrappedRouter { const router = createRouter(HealthCheckApiSpec, { decodeErrorFormatter: customDecodeErrorFormatter, + encodeErrorFormatter: customEncodeErrorFormatter, }); // Ping endpoint handler router.post('advancedwallet.awm.ping', [ diff --git a/src/masterBitgoExpress/routers/awmExpressHealth.ts b/src/masterBitgoExpress/routers/awmExpressHealth.ts index 04d83e56..97ddd81f 100644 --- a/src/masterBitgoExpress/routers/awmExpressHealth.ts +++ b/src/masterBitgoExpress/routers/awmExpressHealth.ts @@ -7,7 +7,10 @@ import logger from '../../shared/logger'; import { responseHandler } from '../../shared/middleware'; import { AdvancedWalletManagerClient } from '../clients/advancedWalletManagerClient'; import { PingResponseType } from '../../types/health'; -import { customDecodeErrorFormatter } from '../../shared/errorFormatters'; +import { + customDecodeErrorFormatter, + customEncodeErrorFormatter, +} from '../../shared/errorFormatters'; // Response type for /ping/advancedWalletManager endpoint const PingAwmResponse: HttpResponse = { @@ -49,6 +52,7 @@ export function createAdvancedWalletManagerHealthRouter( ): WrappedRouter { const router = createRouter(AdvancedWalletManagerHealthSpec, { decodeErrorFormatter: customDecodeErrorFormatter, + encodeErrorFormatter: customEncodeErrorFormatter, }); // Create an instance of awmClient diff --git a/src/masterBitgoExpress/routers/healthCheck.ts b/src/masterBitgoExpress/routers/healthCheck.ts index 0dc78e2e..055624fb 100644 --- a/src/masterBitgoExpress/routers/healthCheck.ts +++ b/src/masterBitgoExpress/routers/healthCheck.ts @@ -4,7 +4,10 @@ import { Response } from '@api-ts/response'; import pjson from '../../../package.json'; import { responseHandler } from '../../shared/middleware'; import { PingResponseType, VersionResponseType } from '../../types/health'; -import { customDecodeErrorFormatter } from '../../shared/errorFormatters'; +import { + customDecodeErrorFormatter, + customEncodeErrorFormatter, +} from '../../shared/errorFormatters'; // API Response types const PingResponse: HttpResponse = { @@ -63,6 +66,7 @@ export function createHealthCheckRouter( ): WrappedRouter { const router = createRouter(HealthCheckApiSpec, { decodeErrorFormatter: customDecodeErrorFormatter, + encodeErrorFormatter: customEncodeErrorFormatter, }); // Ping endpoint handler diff --git a/src/masterBitgoExpress/routers/masterBitGoExpressApiSpec.ts b/src/masterBitgoExpress/routers/masterBitGoExpressApiSpec.ts index 2ffe05d3..0636ee26 100644 --- a/src/masterBitgoExpress/routers/masterBitGoExpressApiSpec.ts +++ b/src/masterBitgoExpress/routers/masterBitGoExpressApiSpec.ts @@ -6,7 +6,10 @@ import { type WrappedRouter, } from '@api-ts/typed-express-router'; import express from 'express'; -import { customDecodeErrorFormatter } from '../../shared/errorFormatters'; +import { + customDecodeErrorFormatter, + customEncodeErrorFormatter, +} from '../../shared/errorFormatters'; import { MasterExpressConfig } from '../../shared/types'; import * as utxolib from '@bitgo-beta/utxo-lib'; import { prepareBitGo, responseHandler } from '../../shared/middleware'; @@ -104,6 +107,7 @@ export function createMasterApiRouter( ): WrappedRouter { const router = createRouter(MasterBitGoExpressApiSpec, { decodeErrorFormatter: customDecodeErrorFormatter, + encodeErrorFormatter: customEncodeErrorFormatter, }); // Add middleware to all routes diff --git a/src/shared/errorFormatters.ts b/src/shared/errorFormatters.ts index 290cf27a..a2791542 100644 --- a/src/shared/errorFormatters.ts +++ b/src/shared/errorFormatters.ts @@ -1,8 +1,34 @@ import * as PathReporter from 'io-ts/lib/PathReporter'; -import { DecodeErrorFormatterFn } from '@api-ts/typed-express-router'; +import { DecodeErrorFormatterFn, EncodeErrorFormatterFn } from '@api-ts/typed-express-router'; +import logger from './logger'; export const customDecodeErrorFormatter: DecodeErrorFormatterFn = (errs, _req) => { const validationErrors = PathReporter.failure(errs); const validationErrorMessage = validationErrors.join('\n'); return { error: validationErrorMessage }; }; + +/** + * typed-express-router validates every outbound response against its io-ts codec in + * res.sendEncoded(). When the payload fails that check the library falls back to + * encodeErrorFormatter, whose default swallows the validation error and responds with + * a bare HTTP 500 and an empty JSON body. Log the real error (message and stack) and + * surface it in the response body so schema violations are debuggable instead of silent. + */ +export const customEncodeErrorFormatter: EncodeErrorFormatterFn = (err, req) => { + const errMessage = err instanceof Error ? err.message : String(err); + const errStack = err instanceof Error ? err.stack : undefined; + + const logLines = [ + `Failed to encode API response for ${req.method} ${req.originalUrl} (api: ${req.apiName}): ${errMessage}`, + ]; + if (errStack) { + logLines.push(errStack); + } + logger.error(logLines.join('\n')); + + return { + error: 'Internal Server Error', + details: errMessage, + }; +};