From ee93bae9d1ff77591430963961f0acfcb5c4a263 Mon Sep 17 00:00:00 2001 From: Zahin Mohammad Date: Wed, 21 Oct 2026 15:41:47 -0400 Subject: [PATCH 1/3] fix(sdk-core): skip ECDH keychain lookup in bulkAcceptShare when unneeded (Refs: WCN-3735) bulkAcceptShare always fetched and decrypted the user's ECDH sharing keychain, even for batches made only of userMultiKeyRotationRequired shares (Go Account / OFC). Those shares carry no keychain and generate a fresh keypair encrypted with the login password, so they never use the ECDH key. A user without an ECDH keychain could therefore not accept them and saw "encryptedXprv was not found on sharing keychain". Fetch the keychain only when at least one selected share has a keychain, as bulkUpdateWalletShare already does, and assert the decrypted key exists before deriving the shared secret, with a client-readable message. Co-Authored-By: Claude Sonnet 5.5 --- modules/sdk-core/src/bitgo/wallet/wallets.ts | 25 +++++++---- .../bitgo/wallet/walletsEncryptionVersion.ts | 44 +++++++++++++++++++ 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/modules/sdk-core/src/bitgo/wallet/wallets.ts b/modules/sdk-core/src/bitgo/wallet/wallets.ts index 329d1405fb2..618c3f1daaf 100644 --- a/modules/sdk-core/src/bitgo/wallet/wallets.ts +++ b/modules/sdk-core/src/bitgo/wallet/wallets.ts @@ -1358,15 +1358,18 @@ export class Wallets implements IWallets { throw new Error('No valid wallet shares found to accept'); } - const sharingKeychain = await this.bitgo.getECDHKeychain(); - if (_.isUndefined(sharingKeychain.encryptedXprv)) { - throw new Error('encryptedXprv was not found on sharing keychain'); - } + let sharingKeychainPrv: string | undefined; + if (walletShares.some((walletShare) => !walletShare.userMultiKeyRotationRequired && walletShare.keychain)) { + const sharingKeychain = await this.bitgo.getECDHKeychain(); + if (_.isUndefined(sharingKeychain.encryptedXprv)) { + throw new Error('encryptedXprv was not found on sharing keychain'); + } - sharingKeychain.prv = await this.bitgo.decrypt({ - password: params.userLoginPassword, - input: sharingKeychain.encryptedXprv, - }); + sharingKeychainPrv = await this.bitgo.decrypt({ + password: params.userLoginPassword, + input: sharingKeychain.encryptedXprv, + }); + } const newWalletPassphrase = params.newWalletPassphrase || params.userLoginPassword; const webauthnInfo = params.webauthnInfo; @@ -1409,8 +1412,12 @@ export class Wallets implements IWallets { if (!walletShare.keychain) { return []; } + assert( + sharingKeychainPrv, + 'Unable to accept this wallet share because your sharing key could not be loaded. Please try again, or contact support@bitgo.com if the problem continues.' + ); const secret = getSharedSecret( - bip32.fromBase58(sharingKeychain.prv).derivePath(sanitizeLegacyPath(walletShare.keychain.path)), + bip32.fromBase58(sharingKeychainPrv).derivePath(sanitizeLegacyPath(walletShare.keychain.path)), Buffer.from(walletShare.keychain.fromPubKey, 'hex') ).toString('hex'); diff --git a/modules/sdk-core/test/unit/bitgo/wallet/walletsEncryptionVersion.ts b/modules/sdk-core/test/unit/bitgo/wallet/walletsEncryptionVersion.ts index cf08425c7b8..7519cc2f70f 100644 --- a/modules/sdk-core/test/unit/bitgo/wallet/walletsEncryptionVersion.ts +++ b/modules/sdk-core/test/unit/bitgo/wallet/walletsEncryptionVersion.ts @@ -145,6 +145,50 @@ describe('Wallets - encryptionVersion threading', function () { assert.strictEqual(call.args[1], undefined); }); + it('accepts a UMKR-only batch without fetching the ECDH keychain', async function () { + mockBitGo.getECDHKeychain.resolves(new Error('missing')); + + await wallets.bulkAcceptShare({ + walletShareIds: ['share-id'], + userLoginPassword: 'login-password', + }); + + assert.strictEqual(mockBitGo.getECDHKeychain.called, false); + assert.strictEqual(mockBitGo.put.called, true); + }); + + it('still requires the ECDH keychain when the batch also holds a share with a keychain', async function () { + mockBitGo.get.returns({ + result: sinon.stub().resolves({ + incoming: [ + ...walletSharesList.incoming, + { + id: 'keychain-share-id', + keychain: { + pub: 'pub', + toPubKey: 'toPubKey', + fromPubKey: 'fromPubKey', + encryptedPrv: 'prv', + path: 'm/0', + }, + permissions: ['spend'], + }, + ], + outgoing: [], + }), + }); + mockBitGo.getECDHKeychain.resolves({}); + + await assert.rejects( + wallets.bulkAcceptShare({ + walletShareIds: ['share-id', 'keychain-share-id'], + userLoginPassword: 'login-password', + }), + { message: 'encryptedXprv was not found on sharing keychain' } + ); + assert.strictEqual(mockBitGo.getECDHKeychain.calledOnce, true); + }); + it('processes shares in batches of 16 to avoid WASM memory exhaustion', async function () { // 20 shares → 2 batches (16 + 4), verifying the batch boundary is crossed const manyShares = Array.from({ length: 20 }, (_, i) => ({ From 41375d2c2ecbb7349463a22d0f73c4fcc644500d Mon Sep 17 00:00:00 2001 From: Zahin Mohammad Date: Wed, 21 Oct 2026 15:42:27 -0400 Subject: [PATCH 2/3] fix(sdk-api): throw from getECDHKeychain when the user has none (Refs: WCN-3735) getECDHKeychain returned an Error object instead of throwing it when the user's settings had no ecdhKeychain. Callers read encryptedXprv off the result, so they reported "encryptedXprv was not found on sharing keychain" instead of the real cause. The error now tells the client the account is missing its sharing key and to contact support@bitgo.com so it can be set up. Co-Authored-By: Claude Sonnet 5.5 --- modules/sdk-api/src/bitgoAPI.ts | 4 +++- modules/sdk-api/test/unit/bitgoAPI.ts | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/modules/sdk-api/src/bitgoAPI.ts b/modules/sdk-api/src/bitgoAPI.ts index e3d3d750d94..9bb9be45ea6 100644 --- a/modules/sdk-api/src/bitgoAPI.ts +++ b/modules/sdk-api/src/bitgoAPI.ts @@ -1882,7 +1882,9 @@ export class BitGoAPI implements BitGoBase { if (!ecdhKeychainPub) { const result = await this.get(this.url('/user/settings')).result(); if (!result.settings.ecdhKeychain) { - return new Error('ecdh keychain not found for user'); + throw new Error( + 'Your account is missing the sharing key needed for this action. Please contact support@bitgo.com so it can be set up.' + ); } ecdhKeychainPub = result.settings.ecdhKeychain; } diff --git a/modules/sdk-api/test/unit/bitgoAPI.ts b/modules/sdk-api/test/unit/bitgoAPI.ts index 4f9dbed15f6..6caa60494f2 100644 --- a/modules/sdk-api/test/unit/bitgoAPI.ts +++ b/modules/sdk-api/test/unit/bitgoAPI.ts @@ -1562,3 +1562,24 @@ describe('wallets() v1 facade', function () { wallets.resendShareInvite.should.be.a.Function(); }); }); + +describe('getECDHKeychain', function () { + const ROOT = 'https://app.example.local'; + + afterEach(function () { + nock.cleanAll(); + }); + + it('rejects when the user has no ecdhKeychain in settings', async function () { + const bitgo = new BitGoAPI({ env: 'custom', customRootURI: ROOT }); + nock(ROOT) + .get('/api/v1/user/settings') + .reply(200, { settings: { ecdhKeychain: null } }); + + await bitgo + .getECDHKeychain() + .should.be.rejectedWith( + 'Your account is missing the sharing key needed for this action. Please contact support@bitgo.com so it can be set up.' + ); + }); +}); From 12675b80e45dc52dada05ad9c9e6489e10d4c430 Mon Sep 17 00:00:00 2001 From: Zahin Mohammad Date: Fri, 23 Oct 2026 15:14:44 -0400 Subject: [PATCH 3/3] fix(sdk-core): validate password for UMKR share acceptance TICKET: WCN-3735 --- modules/sdk-api/src/bitgoAPI.ts | 5 +++- modules/sdk-api/test/unit/bitgoAPI.ts | 11 ++++++++ modules/sdk-core/src/bitgo/bitgoBase.ts | 2 +- modules/sdk-core/src/bitgo/wallet/wallets.ts | 9 +++++-- .../bitgo/wallet/walletsEncryptionVersion.ts | 26 ++++++++++++++++--- 5 files changed, 46 insertions(+), 7 deletions(-) diff --git a/modules/sdk-api/src/bitgoAPI.ts b/modules/sdk-api/src/bitgoAPI.ts index 9bb9be45ea6..504868e629a 100644 --- a/modules/sdk-api/src/bitgoAPI.ts +++ b/modules/sdk-api/src/bitgoAPI.ts @@ -1878,10 +1878,13 @@ export class BitGoAPI implements BitGoBase { /** * Gets the user's private ECDH keychain */ - async getECDHKeychain(ecdhKeychainPub?: string): Promise { + async getECDHKeychain(ecdhKeychainPub?: string, options: { allowMissing?: boolean } = {}): Promise { if (!ecdhKeychainPub) { const result = await this.get(this.url('/user/settings')).result(); if (!result.settings.ecdhKeychain) { + if (options.allowMissing) { + return undefined; + } throw new Error( 'Your account is missing the sharing key needed for this action. Please contact support@bitgo.com so it can be set up.' ); diff --git a/modules/sdk-api/test/unit/bitgoAPI.ts b/modules/sdk-api/test/unit/bitgoAPI.ts index 6caa60494f2..b6da41d55ba 100644 --- a/modules/sdk-api/test/unit/bitgoAPI.ts +++ b/modules/sdk-api/test/unit/bitgoAPI.ts @@ -1582,4 +1582,15 @@ describe('getECDHKeychain', function () { 'Your account is missing the sharing key needed for this action. Please contact support@bitgo.com so it can be set up.' ); }); + + it('returns undefined when the user has no ecdhKeychain and allowMissing is true', async function () { + const bitgo = new BitGoAPI({ env: 'custom', customRootURI: ROOT }); + nock(ROOT) + .get('/api/v1/user/settings') + .reply(200, { settings: { ecdhKeychain: null } }); + + const keychain = await bitgo.getECDHKeychain(undefined, { allowMissing: true }); + + assert.strictEqual(keychain, undefined); + }); }); diff --git a/modules/sdk-core/src/bitgo/bitgoBase.ts b/modules/sdk-core/src/bitgo/bitgoBase.ts index d77728d07ff..698e4846fd4 100644 --- a/modules/sdk-core/src/bitgo/bitgoBase.ts +++ b/modules/sdk-core/src/bitgo/bitgoBase.ts @@ -48,7 +48,7 @@ export interface BitGoBase { readonly env: EnvironmentName; fetchConstants(): Promise; get(url: string): BitGoRequest; - getECDHKeychain(ecdhKeychainPub?: string): Promise; + getECDHKeychain(ecdhKeychainPub?: string, options?: { allowMissing?: boolean }): Promise; getEcdhKeypairPrivate(password: string, entId?: string): Promise; getEnv(): EnvironmentName; getSharingKey({ email }: GetSharingKeyOptions): Promise; diff --git a/modules/sdk-core/src/bitgo/wallet/wallets.ts b/modules/sdk-core/src/bitgo/wallet/wallets.ts index 618c3f1daaf..772482dbf3f 100644 --- a/modules/sdk-core/src/bitgo/wallet/wallets.ts +++ b/modules/sdk-core/src/bitgo/wallet/wallets.ts @@ -1358,9 +1358,14 @@ export class Wallets implements IWallets { throw new Error('No valid wallet shares found to accept'); } + const hasSharesRequiringDecryption = walletShares.some( + (walletShare) => !walletShare.userMultiKeyRotationRequired && walletShare.keychain + ); + const sharingKeychain = await this.bitgo.getECDHKeychain(undefined, { + allowMissing: !hasSharesRequiringDecryption, + }); let sharingKeychainPrv: string | undefined; - if (walletShares.some((walletShare) => !walletShare.userMultiKeyRotationRequired && walletShare.keychain)) { - const sharingKeychain = await this.bitgo.getECDHKeychain(); + if (sharingKeychain) { if (_.isUndefined(sharingKeychain.encryptedXprv)) { throw new Error('encryptedXprv was not found on sharing keychain'); } diff --git a/modules/sdk-core/test/unit/bitgo/wallet/walletsEncryptionVersion.ts b/modules/sdk-core/test/unit/bitgo/wallet/walletsEncryptionVersion.ts index 7519cc2f70f..d9b81fb247e 100644 --- a/modules/sdk-core/test/unit/bitgo/wallet/walletsEncryptionVersion.ts +++ b/modules/sdk-core/test/unit/bitgo/wallet/walletsEncryptionVersion.ts @@ -145,18 +145,37 @@ describe('Wallets - encryptionVersion threading', function () { assert.strictEqual(call.args[1], undefined); }); - it('accepts a UMKR-only batch without fetching the ECDH keychain', async function () { - mockBitGo.getECDHKeychain.resolves(new Error('missing')); + it('accepts a UMKR-only batch when the user has no ECDH keychain', async function () { + mockBitGo.getECDHKeychain.resolves(undefined); await wallets.bulkAcceptShare({ walletShareIds: ['share-id'], userLoginPassword: 'login-password', }); - assert.strictEqual(mockBitGo.getECDHKeychain.called, false); + assert.deepStrictEqual(mockBitGo.getECDHKeychain.firstCall.args, [undefined, { allowMissing: true }]); + assert.strictEqual(mockBitGo.decrypt.called, false); assert.strictEqual(mockBitGo.put.called, true); }); + it('validates the login password against an existing ECDH keychain for a UMKR-only batch', async function () { + mockBitGo.decrypt.rejects(new Error('password error')); + + await assert.rejects( + wallets.bulkAcceptShare({ + walletShareIds: ['share-id'], + userLoginPassword: 'incorrect-password', + }), + { message: 'password error' } + ); + + assert.deepStrictEqual(mockBitGo.getECDHKeychain.firstCall.args, [undefined, { allowMissing: true }]); + assert.deepStrictEqual(mockBitGo.decrypt.firstCall.args, [{ password: 'incorrect-password', input: 'encXprv' }]); + assert.strictEqual(mockBitGo.createEncryptionSession.called, false); + assert.strictEqual(mockKeychains.create.called, false); + assert.strictEqual(mockBitGo.put.called, false); + }); + it('still requires the ECDH keychain when the batch also holds a share with a keychain', async function () { mockBitGo.get.returns({ result: sinon.stub().resolves({ @@ -187,6 +206,7 @@ describe('Wallets - encryptionVersion threading', function () { { message: 'encryptedXprv was not found on sharing keychain' } ); assert.strictEqual(mockBitGo.getECDHKeychain.calledOnce, true); + assert.deepStrictEqual(mockBitGo.getECDHKeychain.firstCall.args, [undefined, { allowMissing: false }]); }); it('processes shares in batches of 16 to avoid WASM memory exhaustion', async function () {