Skip to content

Commit 108fd50

Browse files
fix(sdk-core): return clear 401 for wrong wallet passphrase
Preserve signing error details instead of wrapping them into generic Internal Server Error responses. Ticket: COINS-1257
1 parent 90d0c0f commit 108fd50

10 files changed

Lines changed: 79 additions & 50 deletions

File tree

modules/abstract-utxo/test/unit/webauthn.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ describe('webauthn passphrase decryption', function () {
4545

4646
it('should throw when all passphrases are wrong', async function () {
4747
await assert.rejects(() => wallet.getUserPrv({ keychain, walletPassphrase: 'wrong' }), {
48-
message: 'failed to decrypt user keychain',
48+
message: 'unable to decrypt keychain with the given wallet passphrase',
4949
});
5050
});
5151
});

modules/express/src/clientRoutes.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
encryptRsaWithAesGcm,
2828
GetNetworkPartnersResponse,
2929
GShare,
30+
IncorrectPasswordError,
3031
MPCType,
3132
multisigTypes,
3233
ShareType,
@@ -433,7 +434,11 @@ async function decryptPrivKey(bg: BitGo, encryptedPrivKey: string, walletPw: str
433434
try {
434435
return await bg.decrypt({ password: walletPw, input: encryptedPrivKey });
435436
} catch (e) {
436-
throw new Error(`Error when trying to decrypt private key: ${e}`);
437+
const detail = e instanceof Error ? e.message : String(e);
438+
if (/not valid JSON|unknown envelope version|salt must be|iv must be/i.test(detail)) {
439+
throw new Error(`Error when trying to decrypt private key: ${detail}`);
440+
}
441+
throw new IncorrectPasswordError();
437442
}
438443
}
439444

@@ -1702,6 +1707,7 @@ function handleRequestHandlerError(res: express.Response, error: unknown) {
17021707
name: err.name || 'BitGoExpressError',
17031708
bitgoJsVersion: version,
17041709
bitgoExpressVersion: pjson.version,
1710+
...(err.code ? { code: err.code } : {}),
17051711
});
17061712
const status = err.status || 500;
17071713
if (!(status >= 200 && status < 300)) {

modules/express/src/retryPromise.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export async function retryPromise<T>(
3333
if (err.code === 'ECONNREFUSED') {
3434
onError(err, tryCount);
3535
} else {
36-
throw new Error(err);
36+
throw err;
3737
}
3838
}
3939

modules/express/test/unit/clientRoutes/signPayload.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ describe('With the handler to sign an arbitrary payload in external signing mode
358358
} as unknown as ExpressApiRouteRequest<'express.v2.ofc.extSignPayload', 'post'>;
359359

360360
await handleV2OFCSignPayloadInExtSigningMode(req).should.be.rejectedWith(
361-
'Error when trying to decrypt private key: Error: decrypt: ciphertext is not valid JSON'
361+
'Error when trying to decrypt private key: decrypt: ciphertext is not valid JSON'
362362
);
363363

364364
readFileStub.restore();
@@ -387,7 +387,7 @@ describe('With the handler to sign an arbitrary payload in external signing mode
387387
} as unknown as ExpressApiRouteRequest<'express.v2.ofc.extSignPayload', 'post'>;
388388

389389
await handleV2OFCSignPayloadInExtSigningMode(req).should.be.rejectedWith(
390-
'Error when trying to decrypt private key: Error: incorrect password'
390+
'unable to decrypt keychain with the given wallet passphrase'
391391
);
392392

393393
readFileStub.restore();
@@ -415,7 +415,7 @@ describe('With the handler to sign an arbitrary payload in external signing mode
415415
} as unknown as ExpressApiRouteRequest<'express.v2.ofc.extSignPayload', 'post'>;
416416

417417
await handleV2OFCSignPayloadInExtSigningMode(req).should.be.rejectedWith(
418-
'Error when trying to decrypt private key: Error: incorrect password'
418+
'unable to decrypt keychain with the given wallet passphrase'
419419
);
420420

421421
readFileStub.restore();
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* @prettier
3+
*/
4+
import * as assert from 'assert';
5+
import { retryPromise } from '../../src/retryPromise';
6+
7+
describe('retryPromise', function () {
8+
it('rethrows the original non-ECONNREFUSED error without wrapping', async function () {
9+
const original = Object.assign(new Error('Internal Server Error'), { status: 500 });
10+
await assert.rejects(
11+
() =>
12+
retryPromise(
13+
async () => {
14+
throw original;
15+
},
16+
() => undefined,
17+
{ retryLimit: 1 }
18+
),
19+
(err: Error) => err === original
20+
);
21+
});
22+
});

modules/express/test/unit/typedRoutes/coinSign.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -581,8 +581,8 @@ describe('CoinSign codec tests (External Signer Mode)', function () {
581581
.set('Content-Type', 'application/json')
582582
.send(requestBody);
583583

584-
// Verify error response - runtime errors return 500
585-
assert.strictEqual(result.status, 500);
584+
// Verify error response - wrong passphrase returns 401
585+
assert.strictEqual(result.status, 401);
586586
assert.ok(result.body);
587587
});
588588

modules/express/test/unit/typedRoutes/generateShareTSS.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -935,8 +935,8 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () {
935935
.set('Content-Type', 'application/json')
936936
.send(requestBody);
937937

938-
// Verify error response - runtime errors return 500
939-
assert.strictEqual(result.status, 500);
938+
// Verify error response - wrong passphrase returns 401
939+
assert.strictEqual(result.status, 401);
940940
assert.ok(result.body);
941941
});
942942

modules/express/test/unit/typedRoutes/ofcExtSignPayload.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ describe('OfcExtSignPayload External Signer Mode Tests', function () {
266266
.set('Content-Type', 'application/json')
267267
.send(requestBody);
268268

269-
assert.strictEqual(result.status, 500);
269+
assert.strictEqual(result.status, 401);
270270
result.body.should.have.property('error');
271271
});
272272

modules/sdk-core/src/bitgo/errors.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,12 @@ export class MissingEncryptedKeychainError extends Error {
168168
}
169169

170170
export class IncorrectPasswordError extends Error {
171+
public code = 'wallet_passphrase_incorrect';
172+
public status = 401;
173+
171174
public constructor(message?: string) {
172-
super(message || 'Incorrect password');
175+
super(message || 'unable to decrypt keychain with the given wallet passphrase');
176+
this.name = 'IncorrectPasswordError';
173177
}
174178
}
175179

modules/sdk-core/src/bitgo/wallet/wallet.ts

Lines changed: 35 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2562,7 +2562,7 @@ export class Wallet implements IWallet {
25622562
}
25632563
userPrv = await decryptKeychainPrivateKey(this.bitgo, userKeychain, params.walletPassphrase);
25642564
if (!userPrv) {
2565-
throw new Error('failed to decrypt user keychain');
2565+
throw new IncorrectPasswordError();
25662566
}
25672567
}
25682568
return userPrv;
@@ -4813,16 +4813,16 @@ export class Wallet implements IWallet {
48134813
await this.tssUtils.deleteSignatureShares(txRequestId, reqId);
48144814

48154815
try {
4816-
const signedTxRequest = await this.tssUtils.signEddsaTssUsingExternalSigner(
4816+
return await this.tssUtils.signEddsaTssUsingExternalSigner(
48174817
txRequestId,
48184818
params.customCommitmentGeneratingFunction,
48194819
params.customRShareGeneratingFunction,
48204820
params.customGShareGeneratingFunction,
48214821
reqId
48224822
);
4823-
return signedTxRequest;
48244823
} catch (e) {
4825-
throw new Error('failed to sign transaction ' + e);
4824+
debug('failed to sign transaction %O', e);
4825+
throw e;
48264826
}
48274827
}
48284828

@@ -4919,9 +4919,9 @@ export class Wallet implements IWallet {
49194919
throw new Error('Generator function for S share required to sign transactions with External Signer.');
49204920
}
49214921

4922+
assert(this.tssUtils, 'tssUtils must be defined');
49224923
try {
4923-
assert(this.tssUtils, 'tssUtils must be defined');
4924-
const signedTxRequest = await this.tssUtils.signEcdsaTssUsingExternalSigner(
4924+
return await this.tssUtils.signEcdsaTssUsingExternalSigner(
49254925
{
49264926
txRequest: txRequestId,
49274927
reqId: params.reqId || new RequestTracer(),
@@ -4932,9 +4932,9 @@ export class Wallet implements IWallet {
49324932
params.customMuDeltaShareGeneratingFunction,
49334933
params.customSShareGeneratingFunction
49344934
);
4935-
return signedTxRequest;
49364935
} catch (e) {
4937-
throw new Error('failed to sign transaction ' + e);
4936+
debug('failed to sign transaction %O', e);
4937+
throw e;
49384938
}
49394939
}
49404940

@@ -4974,9 +4974,9 @@ export class Wallet implements IWallet {
49744974
);
49754975
}
49764976

4977+
assert(this.tssUtils, 'tssUtils must be defined');
49774978
try {
4978-
assert(this.tssUtils, 'tssUtils must be defined');
4979-
const signedTxRequest = await this.tssUtils.signEddsaMPCv2TssUsingExternalSigner(
4979+
return await this.tssUtils.signEddsaMPCv2TssUsingExternalSigner(
49804980
{
49814981
txRequest: txRequestId,
49824982
reqId: params.reqId || new RequestTracer(),
@@ -4985,9 +4985,9 @@ export class Wallet implements IWallet {
49854985
params.customEddsaMPCv2SigningRound2GenerationFunction,
49864986
params.customEddsaMPCv2SigningRound3GenerationFunction
49874987
);
4988-
return signedTxRequest;
49894988
} catch (e) {
4990-
throw new Error('failed to sign transaction ' + e);
4989+
debug('failed to sign transaction %O', e);
4990+
throw e;
49914991
}
49924992
}
49934993

@@ -5021,9 +5021,9 @@ export class Wallet implements IWallet {
50215021
throw new Error('Generator function for MPCv2 Round 3 share required to sign transactions with External Signer.');
50225022
}
50235023

5024+
assert(this.tssUtils, 'tssUtils must be defined');
50245025
try {
5025-
assert(this.tssUtils, 'tssUtils must be defined');
5026-
const signedTxRequest = await this.tssUtils.signEcdsaMPCv2TssUsingExternalSigner(
5026+
return await this.tssUtils.signEcdsaMPCv2TssUsingExternalSigner(
50275027
{
50285028
txRequest: txRequestId,
50295029
reqId: params.reqId || new RequestTracer(),
@@ -5032,9 +5032,9 @@ export class Wallet implements IWallet {
50325032
params.customMPCv2SigningRound2GenerationFunction,
50335033
params.customMPCv2SigningRound3GenerationFunction
50345034
);
5035-
return signedTxRequest;
50365035
} catch (e) {
5037-
throw new Error('failed to sign transaction ' + e);
5036+
debug('failed to sign transaction %O', e);
5037+
throw e;
50385038
}
50395039
}
50405040

@@ -5056,23 +5056,23 @@ export class Wallet implements IWallet {
50565056
throw new Error('prv required to sign transactions with TSS');
50575057
}
50585058

5059-
try {
5060-
let txRequest: string | TxRequest = params.txPrebuild.txRequestId;
5061-
let txParams: TransactionParams | undefined = params.txPrebuild.buildParams;
5062-
5063-
// EdDSA MPCv2 re-sign path: buildParams is absent when the UI calls signAndSendTxRequest with
5064-
// only txRequestId. Derive txParams from the persisted intent so verifyTransaction receives
5065-
// the correct recipients before DSG starts. Other TSS variants are unaffected by the guard.
5066-
if (!txParams && this.multisigTypeVersion() === 'MPCv2' && this.baseCoin.getMPCAlgorithm() === 'eddsa') {
5067-
txRequest = await getTxRequest(
5068-
this.bitgo,
5069-
this.id(),
5070-
params.txPrebuild.txRequestId,
5071-
params.reqId || new RequestTracer()
5072-
);
5073-
txParams = txParamsFromIntent(txRequest.intent, this.baseCoin.getChain());
5074-
}
5059+
let txRequest: string | TxRequest = params.txPrebuild.txRequestId;
5060+
let txParams: TransactionParams | undefined = params.txPrebuild.buildParams;
5061+
5062+
// EdDSA MPCv2 re-sign path: buildParams is absent when the UI calls signAndSendTxRequest with
5063+
// only txRequestId. Derive txParams from the persisted intent so verifyTransaction receives
5064+
// the correct recipients before DSG starts. Other TSS variants are unaffected by the guard.
5065+
if (!txParams && this.multisigTypeVersion() === 'MPCv2' && this.baseCoin.getMPCAlgorithm() === 'eddsa') {
5066+
txRequest = await getTxRequest(
5067+
this.bitgo,
5068+
this.id(),
5069+
params.txPrebuild.txRequestId,
5070+
params.reqId || new RequestTracer()
5071+
);
5072+
txParams = txParamsFromIntent(txRequest.intent, this.baseCoin.getChain());
5073+
}
50755074

5075+
try {
50765076
return await this.tssUtils!.signTxRequest({
50775077
txRequest,
50785078
txParams,
@@ -5081,7 +5081,8 @@ export class Wallet implements IWallet {
50815081
apiVersion: params.apiVersion,
50825082
});
50835083
} catch (e) {
5084-
throw new Error('failed to sign transaction ' + e);
5084+
debug('failed to sign transaction %O', e);
5085+
throw e;
50855086
}
50865087
}
50875088

@@ -5383,11 +5384,7 @@ export class Wallet implements IWallet {
53835384
// which means that the user is handling the signing in external signing mode
53845385
if (!customSigningFunction && keychains?.[0]?.encryptedPrv && walletPassphrase) {
53855386
if (!(await decryptKeychainPrivateKey(this.bitgo, keychains[0], walletPassphrase))) {
5386-
const error: Error & { code?: string } = new Error(
5387-
`unable to decrypt keychain with the given wallet passphrase`
5388-
);
5389-
error.code = 'wallet_passphrase_incorrect';
5390-
throw error;
5387+
throw new IncorrectPasswordError();
53915388
}
53925389
}
53935390
return keychains;

0 commit comments

Comments
 (0)