From f6054be32b3040731dca71cb54eada7f8abdf586 Mon Sep 17 00:00:00 2001 From: Aaron Loe Date: Tue, 3 Nov 2026 12:38:10 -0800 Subject: [PATCH] fix: serialization failure in changePassword fixes edge case where change password would fail during JSON serialization on large private key counts. Byte size of overall payload is calculated per id and aggregated, instead of using JSON.stringify over the whole payload. Ticket: WAL-2426 --- modules/sdk-api/src/bitgoAPI.ts | 37 ++++++++++++++++++++++-- modules/sdk-api/test/unit/bitgoAPI.ts | 41 +++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/modules/sdk-api/src/bitgoAPI.ts b/modules/sdk-api/src/bitgoAPI.ts index 504868e629..11cfb313fe 100644 --- a/modules/sdk-api/src/bitgoAPI.ts +++ b/modules/sdk-api/src/bitgoAPI.ts @@ -234,6 +234,38 @@ function deriveTokenIssuanceEcdhSecret(ecdhXprv: string, derivationPath: string, ).toString('hex'); } +type PasswordUpdatePayload = { + keychains: Record; + v2_keychains: Record; + version: number; + oldPassword: string; + password: string; +}; + +function serializedMapLength(keychains: Record): number { + return Object.entries(keychains).reduce( + (length, [id, value], index) => + length + (index > 0 ? 1 : 0) + JSON.stringify(id).length + 1 + JSON.stringify(value).length, + 2 + ); +} + +function passwordUpdatePayloadSizeKB(updatePasswordParams: PasswordUpdatePayload): number { + const emptyPayloadLength = JSON.stringify({ + ...updatePasswordParams, + keychains: {}, + v2_keychains: {}, + }).length; + // Replace the two "{}" map placeholders with their serialized entries. + const payloadLength = + emptyPayloadLength - + 4 + + serializedMapLength(updatePasswordParams.keychains) + + serializedMapLength(updatePasswordParams.v2_keychains); + + return Math.ceil(payloadLength / 1024); +} + export class BitGoAPI implements BitGoBase { // v1 types protected _keychains: any; @@ -2156,9 +2188,8 @@ export class BitGoAPI implements BitGoBase { password: hmacNewPassword, }; - // Calculate payload size in KB - const payloadSizeBytes = JSON.stringify(updatePasswordParams).length; - const payloadSizeKB = Math.ceil(payloadSizeBytes / 1024); + // Calculate payload size in KB without serializing both keychain maps together. + const payloadSizeKB = passwordUpdatePayloadSizeKB(updatePasswordParams); // Check if batching flow is enabled let useBatchingFlow = false; diff --git a/modules/sdk-api/test/unit/bitgoAPI.ts b/modules/sdk-api/test/unit/bitgoAPI.ts index b6da41d55b..06d0b49548 100644 --- a/modules/sdk-api/test/unit/bitgoAPI.ts +++ b/modules/sdk-api/test/unit/bitgoAPI.ts @@ -1104,6 +1104,47 @@ describe('Constructor', function () { changePassScope.isDone().should.be.true(); }); + it('checks batching without serializing the complete keychain payload', async function () { + const largeKeychains = Object.fromEntries( + Array.from({ length: 200 }, (_, index) => [`keychain-${index}`, 'x'.repeat(50)]) + ); + const updatePasswordParams = { + keychains: largeKeychains, + v2_keychains: { v2k1: 'v2enc' }, + version: 25, + oldPassword: 'hashed-pw', + password: 'hashed-pw', + }; + const expectedPayloadSizeKB = Math.ceil(JSON.stringify(updatePasswordParams).length / 1024); + v1UpdatePasswordStub.resolves({ keychains: largeKeychains, version: 25 }); + + nock(ROOT) + .get('/api/v2/user/checkBatchingPasswordFlow') + .query({ payloadSize: expectedPayloadSizeKB.toString() }) + .reply(200, { isBatchingFlowEnabled: true, maxBatchSizeKB: 1 }); + const batchPutScope = nock(ROOT).persist().put('/api/v2/user/keychains').reply(200, {}); + const changePassScope = nock(ROOT) + .post('/api/v1/user/changepassword', (body: any) => !body.keychains && !body.v2_keychains) + .reply(200, {}); + + const originalStringify = JSON.stringify; + const stringifyStub = sandbox.stub(JSON, 'stringify').callsFake((value: any) => { + if (value?.keychains && Object.keys(value.keychains).length > 100) { + throw new Error('complete keychain payload was serialized'); + } + return originalStringify(value); + }); + + try { + await bitgo.changePassword({ oldPassword: 'oldpw', newPassword: 'newpw' }); + } finally { + stringifyStub.restore(); + } + + batchPutScope.isDone().should.be.true(); + changePassScope.isDone().should.be.true(); + }); + it('stages an empty batch and changes password when no keychains are re-encryptable', async function () { v1UpdatePasswordStub.resolves({ keychains: {}, version: 25 }); v2UpdatePasswordStub.resolves({});