Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 113 additions & 0 deletions src/__tests__/api/master/generateWallet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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';

Expand Down
49 changes: 49 additions & 0 deletions src/__tests__/errorFormatters.test.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown>;

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<string, unknown>;

assert.equal(body.error, 'Internal Server Error');
assert.equal(body.details, 'not an Error');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -512,6 +515,7 @@ export function createKeyGenRouter(
): WrappedRouter<typeof AdvancedWalletManagerApiSpec> {
const router = createRouter(AdvancedWalletManagerApiSpec, {
decodeErrorFormatter: customDecodeErrorFormatter,
encodeErrorFormatter: customEncodeErrorFormatter,
});
// Add middleware
router.use(express.json());
Expand Down
6 changes: 5 additions & 1 deletion src/advancedWalletManager/routers/healthCheck.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -60,6 +63,7 @@ export const HealthCheckApiSpec = apiSpec({
export function createHealthCheckRouter(): WrappedRouter<typeof HealthCheckApiSpec> {
const router = createRouter(HealthCheckApiSpec, {
decodeErrorFormatter: customDecodeErrorFormatter,
encodeErrorFormatter: customEncodeErrorFormatter,
});
// Ping endpoint handler
router.post('advancedwallet.awm.ping', [
Expand Down
6 changes: 5 additions & 1 deletion src/masterBitgoExpress/routers/awmExpressHealth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -49,6 +52,7 @@ export function createAdvancedWalletManagerHealthRouter(
): WrappedRouter<typeof AdvancedWalletManagerHealthSpec> {
const router = createRouter(AdvancedWalletManagerHealthSpec, {
decodeErrorFormatter: customDecodeErrorFormatter,
encodeErrorFormatter: customEncodeErrorFormatter,
});

// Create an instance of awmClient
Expand Down
6 changes: 5 additions & 1 deletion src/masterBitgoExpress/routers/healthCheck.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -63,6 +66,7 @@ export function createHealthCheckRouter(
): WrappedRouter<typeof HealthCheckApiSpec> {
const router = createRouter(HealthCheckApiSpec, {
decodeErrorFormatter: customDecodeErrorFormatter,
encodeErrorFormatter: customEncodeErrorFormatter,
});

// Ping endpoint handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -104,6 +107,7 @@ export function createMasterApiRouter(
): WrappedRouter<typeof MasterBitGoExpressApiSpec> {
const router = createRouter(MasterBitGoExpressApiSpec, {
decodeErrorFormatter: customDecodeErrorFormatter,
encodeErrorFormatter: customEncodeErrorFormatter,
});

// Add middleware to all routes
Expand Down
28 changes: 27 additions & 1 deletion src/shared/errorFormatters.ts
Original file line number Diff line number Diff line change
@@ -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,
};
};
Loading