From e5129b53ee258cd042904aa9d9a6e641091eb632 Mon Sep 17 00:00:00 2001 From: rajangarg047 Date: Wed, 24 Jun 2026 17:07:39 -0400 Subject: [PATCH] feat(express): verify created addresses against client-supplied trusted keychains MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an opt-in "bring your own trusted keys" check to address creation. When the createAddress request includes trustedKeychains (public key material the caller holds independently), Express locally re-derives each newly created address via coin.deriveAddress and fails the request if it does not match what the service returned — an independent verification folded into the create call, with no client-side round-trip. Unlike the SDK's built-in check (which verifies using keychains fetched from the same service), this uses caller-supplied keys, so a match is an independent trust guarantee. It is purely per-request opt-in: requests without trustedKeychains are unchanged, so other clients/coins are unaffected, and no server config is needed. The caller opted in, so an unsupported coin/wallet surfaces a clear 400 rather than silently skipping. The createAddress result is modeled as a CreateAddressResult type (reusing DeriveAddressOptions types) instead of any, and derivation failures and mismatches alike map to 400. Token (e.g. SPL) deposit addresses derive differently and are not verified here yet (follows WCN-1054). WCN-1055 Co-Authored-By: Claude Opus 4.8 (1M context) --- modules/express/src/clientRoutes.ts | 82 ++++++++++++++++- .../src/typedRoutes/api/v2/createAddress.ts | 10 ++ .../test/unit/typedRoutes/createAddress.ts | 92 +++++++++++++++++++ 3 files changed, 183 insertions(+), 1 deletion(-) diff --git a/modules/express/src/clientRoutes.ts b/modules/express/src/clientRoutes.ts index 7761fcca75..9e10658849 100755 --- a/modules/express/src/clientRoutes.ts +++ b/modules/express/src/clientRoutes.ts @@ -18,6 +18,7 @@ import { CustomPaillierModulusGetterFunction, CustomRShareGeneratingFunction, CustomSShareGeneratingFunction, + DeriveAddressOptions, EcdsaMPCv2Utils, EcdsaUtils, EddsaMPCv2Utils, @@ -697,6 +698,75 @@ export async function handleV2GenerateWallet(req: ExpressApiRouteRequest<'expres return { ...result, wallet: result.wallet.toJSON() }; } +/** + * Independently verify newly created address(es) against client-supplied trusted keychains. + * + * "Bring your own trusted keys": the caller passes keychains it holds independently, and we + * locally re-derive each created address and compare it to what the service returned. Because the + * keys do not come from the service, a match is an independent correctness guarantee (unlike the + * SDK's built-in check, which verifies using service-fetched keys). Throws on any mismatch. + * + * The caller opted in by supplying the keychains, so if the coin/wallet does not support local + * derivation we surface that as an error rather than silently skipping the requested verification. + */ +/** + * The fields of a created address we read to re-derive it. `wallet.createAddress` is typed `any` + * in the SDK and returns either a single address or `{ addresses: [...] }`, so we model just the + * subset we consume and reuse {@link DeriveAddressOptions} types for the derivation inputs. + */ +type CreatedAddress = { + address?: unknown; + index?: unknown; + chain?: number; + format?: DeriveAddressOptions['format']; + baseAddress?: string; + coinSpecific?: DeriveAddressOptions['coinSpecific']; +}; +type CreateAddressResult = CreatedAddress & { addresses?: CreatedAddress[] }; + +async function verifyCreatedAddressesWithTrustedKeys( + coin: ReturnType, + wallet: Wallet, + result: CreateAddressResult, + trustedKeychains: ({ pub: string } | { commonKeychain: string })[] +): Promise { + const addresses = Array.isArray(result?.addresses) ? result.addresses : [result]; + const walletCoinSpecific = (wallet.coinSpecific?.() ?? {}) as { walletVersion?: number; baseAddress?: string }; + + for (const addr of addresses) { + if (!addr || typeof addr.address !== 'string' || typeof addr.index !== 'number') { + continue; + } + // The caller opted in, so any derivation failure (e.g. an unsupported coin) must surface as a + // 400 rather than the default 500 — hence we map derivation errors and mismatches alike to 400. + try { + const derived = await coin.deriveAddress({ + keychains: trustedKeychains, + index: addr.index, + chain: addr.chain, + format: addr.format, + walletVersion: walletCoinSpecific.walletVersion, + baseAddress: addr.baseAddress ?? walletCoinSpecific.baseAddress, + coinSpecific: addr.coinSpecific, + }); + if (derived.address !== addr.address) { + throw new ApiResponseError( + `address verification failed: service returned ${addr.address} but trusted keychains derive ${derived.address}`, + 400 + ); + } + } catch (e) { + if (e instanceof ApiResponseError) { + throw e; + } + throw new ApiResponseError( + `address verification with trusted keychains failed for ${addr.address}: ${(e as Error).message}`, + 400 + ); + } + } +} + /** * handle new address creation * @param req @@ -705,7 +775,17 @@ export async function handleV2CreateAddress(req: ExpressApiRouteRequest<'express const bitgo = req.bitgo; const coin = bitgo.coin(req.decoded.coin); const wallet = await coin.wallets().get({ id: req.decoded.id }); - return wallet.createAddress(req.decoded); + const result = await wallet.createAddress(req.decoded); + + // Opt-in, per-request: if the caller supplied trusted keychains, independently verify the + // created address(es) before returning. No effect on requests that omit trustedKeychains. + // Token (e.g. SPL) deposit addresses derive differently (associated token account) and are not + // verified here yet — that lands once token derivation (WCN-1054) is available on this path. + const trustedKeychains = req.decoded.trustedKeychains; + if (trustedKeychains && trustedKeychains.length > 0 && !req.decoded.onToken) { + await verifyCreatedAddressesWithTrustedKeys(coin, wallet, result, trustedKeychains); + } + return result; } /** diff --git a/modules/express/src/typedRoutes/api/v2/createAddress.ts b/modules/express/src/typedRoutes/api/v2/createAddress.ts index 7a366fb999..b44251c61d 100644 --- a/modules/express/src/typedRoutes/api/v2/createAddress.ts +++ b/modules/express/src/typedRoutes/api/v2/createAddress.ts @@ -2,6 +2,7 @@ import * as t from 'io-ts'; import { httpRoute, httpRequest, optional } from '@api-ts/io-ts-http'; import { BitgoExpressError } from '../../schemas/error'; import { EIP1559, ForwarderVersion, CreateAddressFormat } from '../../schemas/address'; +import { DeriveAddressKeychainCodec } from './deriveAddress'; /** * Path parameters for creating a wallet address @@ -51,6 +52,15 @@ export const CreateAddressBody = { baseAddress: optional(t.string), /** When false, throws error if address verification is skipped (e.g., during pending chain initialization). Default: true */ allowSkipVerifyAddress: optional(t.boolean), + /** + * Optional independently-held keychains (public key material only). When provided, Express + * locally re-derives each newly created address from these keys and fails the request if it + * does not match the address returned by the service — an independent ("bring your own trusted + * keys") verification folded into the create call. Omit to keep the default behavior. + * Supported for coins with local derivation (BTC/UTXO, ETH MPC + forwarder, SOL incl. SPL tokens); + * requesting it for an unsupported coin returns an error rather than silently skipping. + */ + trustedKeychains: optional(t.array(DeriveAddressKeychainCodec)), } as const; /** Response for creating wallet address(es) */ diff --git a/modules/express/test/unit/typedRoutes/createAddress.ts b/modules/express/test/unit/typedRoutes/createAddress.ts index 5e34dd1669..dad41a13cd 100644 --- a/modules/express/test/unit/typedRoutes/createAddress.ts +++ b/modules/express/test/unit/typedRoutes/createAddress.ts @@ -157,6 +157,98 @@ describe('CreateAddress codec tests', function () { true ); }); + + describe('trustedKeychains verification', function () { + const trustedKeychains = [{ pub: 'xpub-user' }, { pub: 'xpub-backup' }, { pub: 'xpub-bitgo' }]; + + function stubCoin(deriveAddressStub: sinon.SinonStub) { + const mockWallet = { + createAddress: sinon.stub().resolves(mockResponse), + coinSpecific: sinon.stub().returns({}), + }; + const mockCoin = { + wallets: sinon.stub().returns({ get: sinon.stub().resolves(mockWallet) }), + deriveAddress: deriveAddressStub, + }; + sinon.stub(BitGo.prototype, 'coin').returns(mockCoin as any); + return { mockCoin }; + } + + it('returns the address when the derived address matches the trusted keychains', async function () { + const deriveAddressStub = sinon.stub().resolves({ address: mockResponse.address, index: 1 }); + const { mockCoin } = stubCoin(deriveAddressStub); + + const result = await agent + .post(`/api/v2/${coin}/wallet/${walletId}/address`) + .set('Authorization', 'Bearer test_access_token_12345') + .set('Content-Type', 'application/json') + .send({ chain: 0, trustedKeychains }); + + assert.strictEqual(result.status, 200); + assert.strictEqual(result.body.address, mockResponse.address); + // deriveAddress was called with the client's trusted keychains (not service keys) + sinon.assert.calledOnce(mockCoin.deriveAddress); + const callArgs = mockCoin.deriveAddress.firstCall.args[0]; + assert.deepStrictEqual(callArgs.keychains, trustedKeychains); + assert.strictEqual(callArgs.index, 1); + }); + + it('returns 400 when the derived address does not match (service returned a different address)', async function () { + const deriveAddressStub = sinon.stub().resolves({ address: '2NDifferentAddressXXXXXXXXXXXXXXXXXXX', index: 1 }); + stubCoin(deriveAddressStub); + + const result = await agent + .post(`/api/v2/${coin}/wallet/${walletId}/address`) + .set('Authorization', 'Bearer test_access_token_12345') + .set('Content-Type', 'application/json') + .send({ chain: 0, trustedKeychains }); + + assert.strictEqual(result.status, 400); + result.body.should.have.property('error'); + }); + + it('returns 400 when derivation is not supported for the coin/wallet (caller opted in)', async function () { + const deriveAddressStub = sinon.stub().rejects(new Error('deriveAddress is not supported for this coin')); + stubCoin(deriveAddressStub); + + const result = await agent + .post(`/api/v2/${coin}/wallet/${walletId}/address`) + .set('Authorization', 'Bearer test_access_token_12345') + .set('Content-Type', 'application/json') + .send({ chain: 0, trustedKeychains }); + + assert.strictEqual(result.status, 400); + result.body.should.have.property('error'); + }); + + it('does not verify (no deriveAddress call) when trustedKeychains is omitted', async function () { + const deriveAddressStub = sinon.stub().resolves({ address: mockResponse.address, index: 1 }); + const { mockCoin } = stubCoin(deriveAddressStub); + + const result = await agent + .post(`/api/v2/${coin}/wallet/${walletId}/address`) + .set('Authorization', 'Bearer test_access_token_12345') + .set('Content-Type', 'application/json') + .send({ chain: 0 }); + + assert.strictEqual(result.status, 200); + sinon.assert.notCalled(mockCoin.deriveAddress); + }); + + it('skips trusted verification for token (onToken) addresses', async function () { + const deriveAddressStub = sinon.stub().resolves({ address: mockResponse.address, index: 1 }); + const { mockCoin } = stubCoin(deriveAddressStub); + + const result = await agent + .post(`/api/v2/${coin}/wallet/${walletId}/address`) + .set('Authorization', 'Bearer test_access_token_12345') + .set('Content-Type', 'application/json') + .send({ chain: 0, trustedKeychains, onToken: 'tsol:usdc' }); + + assert.strictEqual(result.status, 200); + sinon.assert.notCalled(mockCoin.deriveAddress); + }); + }); }); describe('CreateAddressParams', function () {