diff --git a/modules/bitgo/test/v2/unit/pendingApproval.ts b/modules/bitgo/test/v2/unit/pendingApproval.ts index 3a71b58d1d..880faf96ad 100644 --- a/modules/bitgo/test/v2/unit/pendingApproval.ts +++ b/modules/bitgo/test/v2/unit/pendingApproval.ts @@ -706,4 +706,58 @@ describe('Pending Approvals:', () => { }); }); }); + + describe('message-sign approvals (transactionRequestFull)', () => { + const buildMessageSignPendingApprovalData = (intentType: string): PendingApprovalData => ({ + id: 'pa-msg', + info: { + type: Type.TRANSACTION_REQUEST_FULL, + transactionRequestFull: { + txRequestId: 'txreq-msg-1', + intent: { intentType }, + }, + } as PendingApprovalInfo, + state: State.PENDING, + creator: 'test', + txRequestId: 'txreq-msg-1', + }); + + ['signMessage', 'signTypedStructuredData'].forEach((intentType) => { + it(`should approve without the post-approve auto-sign for a ${intentType} approval`, async () => { + // EVM coin: canRecreateTransaction is true with a walletPassphrase, so + // without the message-sign guard the auto-sign below would run. + const ethBaseCoin = bitgo.coin('teth'); + const paData = buildMessageSignPendingApprovalData(intentType); + const pendingApproval = new PendingApproval(bitgo, ethBaseCoin, paData, wallet); + + const paScope = nock(bgUrl) + .put(`/api/v2/teth/pendingapprovals/pa-msg`, { state: 'approved', otp: undefined }) + .reply(200, { ...paData, state: 'approved' }); + const recreateTransactionTssStub = sandbox.stub(PendingApproval.prototype, 'recreateAndSignTSSTransaction'); + + await pendingApproval.approve({ walletPassphrase: 'pass' }); + + // Message requests have no unsigned transaction; the post-approve + // auto-sign must never run for them (it reads transactions[0].unsignedTx). + recreateTransactionTssStub.notCalled.should.be.true(); + paScope.isDone().should.be.true(); + }); + }); + + it('should keep the post-approve auto-sign for transaction approvals', async () => { + const ethBaseCoin = bitgo.coin('teth'); + const paData = buildMessageSignPendingApprovalData('payment'); + const pendingApproval = new PendingApproval(bitgo, ethBaseCoin, paData, wallet); + + const paScope = nock(bgUrl) + .put(`/api/v2/teth/pendingapprovals/pa-msg`, { state: 'approved', otp: undefined }) + .reply(200, { ...paData, state: 'approved' }); + const recreateTransactionTssStub = sandbox.stub(PendingApproval.prototype, 'recreateAndSignTSSTransaction'); + + await pendingApproval.approve({ walletPassphrase: 'pass' }); + + recreateTransactionTssStub.calledOnce.should.be.true(); + paScope.isDone().should.be.true(); + }); + }); }); diff --git a/modules/sdk-core/src/bitgo/pendingApproval/iPendingApproval.ts b/modules/sdk-core/src/bitgo/pendingApproval/iPendingApproval.ts index 5b33a47e22..6b12a9d96e 100644 --- a/modules/sdk-core/src/bitgo/pendingApproval/iPendingApproval.ts +++ b/modules/sdk-core/src/bitgo/pendingApproval/iPendingApproval.ts @@ -48,6 +48,21 @@ export interface PendingApprovalInfo { }; sourceWallet?: string; }; + /** + * Present on transactionRequestFull pending approvals. `intent` is echoed + * by wallet-platform (with finalizeUnwrap secrets stripped) and is the only + * client-visible signal for message-sign approvals: message requests carry + * intentType 'signMessage' or 'signTypedStructuredData' and have NO + * unsigned transaction, so post-approval auto-sign must not run for them. + */ + transactionRequestFull?: { + txRequestId?: string; + intent?: { + intentType?: string; + [index: string]: unknown; + }; + [index: string]: unknown; + }; } export interface PendingApprovalData { diff --git a/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts b/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts index 4f18b59d69..7fdd8f28e3 100644 --- a/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts +++ b/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts @@ -489,12 +489,31 @@ export class PendingApproval implements IPendingApproval { reqId ); } else if (this.canRecreateTransaction(params) && this.baseCoin.supportsTss()) { + if (this.isMessageSigningRequest()) { + // Message-sign approvals (WalletConnect personal_sign / EIP-712) + // have no unsigned transaction: the auto recreate-and-sign below + // reads transactions[0].unsignedTx and crashes on them. The + // message is signed separately via signAndSendMessageTxRequest + // once the approval resolves, so approving ends here. + return; + } await this.recreateAndSignTSSTransaction(params, reqId); } } } } + /** + * True when this approval guards a message-sign txRequest (WalletConnect + * personal_sign / EIP-712 typed data). Detected from the PA's echoed + * intent — message requests carry no unsigned transaction, so callers that + * assume a transaction (recreate-and-sign, txHex reads) must skip them. + */ + private isMessageSigningRequest(): boolean { + const intentType = this.info()?.transactionRequestFull?.intent?.intentType; + return intentType === 'signMessage' || intentType === 'signTypedStructuredData'; + } + /** * Helper function to ensure that self.wallet is set */ diff --git a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts index 3ef50fe19e..2017e96a83 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts @@ -590,11 +590,13 @@ export class EcdsaUtils extends BaseEcdsaUtils { if (requestType === RequestType.tx) { assert( - txRequest.transactions || (txRequest as TxRequest).unsignedTxs, + (txRequest.transactions && txRequest.transactions.length > 0) || + ((txRequest as TxRequest).unsignedTxs && txRequest.unsignedTxs.length > 0), 'Unable to find transactions in txRequest' ); const unsignedTx = txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0]; + assert(unsignedTx, 'Missing unsignedTx in txRequest'); derivationPath = unsignedTx.derivationPath; } else if (requestType === RequestType.message) { // TODO BG-67299 Message signing with derivation path @@ -641,9 +643,14 @@ export class EcdsaUtils extends BaseEcdsaUtils { const txRequest: TxRequest = tssParams.txRequest; let signablePayload; if (requestType === RequestType.tx) { - assert(txRequest.transactions || txRequest.unsignedTxs, 'Unable to find transactions in txRequest'); + assert( + (txRequest.transactions && txRequest.transactions.length > 0) || + (txRequest.unsignedTxs && txRequest.unsignedTxs.length > 0), + 'Unable to find transactions in txRequest' + ); const unsignedTx = txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0]; + assert(unsignedTx, 'Missing unsignedTx in txRequest'); signablePayload = Buffer.from(unsignedTx.signableHex, 'hex'); } else if (requestType === RequestType.message) { signablePayload = (params.tssParams as TSSParamsForMessage).bufferToSign; @@ -799,9 +806,14 @@ export class EcdsaUtils extends BaseEcdsaUtils { let derivationPath = ''; if (requestType === RequestType.tx) { - assert(txRequest.transactions || txRequest.unsignedTxs, 'Unable to find transactions in txRequest'); + assert( + (txRequest.transactions && txRequest.transactions.length > 0) || + (txRequest.unsignedTxs && txRequest.unsignedTxs.length > 0), + 'Unable to find transactions in txRequest' + ); const unsignedTx = txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0]; + assert(unsignedTx, 'Missing unsignedTx in txRequest'); if ( 'recipientSource' in params &&