Bind Express wallet signing to route identity

[mmcshinsky-bitgo]

Summary

• Reject wallet transaction signing requests whose txPrebuild.walletId differs from the route wallet ID.
• Apply the check to regular wallet and TSS wallet signing routes.
• Add regression coverage proving mismatches are rejected before wallet lookup or signing.

Test plan

• git diff --check
• IDE linter diagnostics are clean
• Run targeted Express tests in CI; local Yarn/Mocha execution is unavailable because the checkout lacks Yarn and a workspace Mocha binary.

PT-000000

[rishikeshdadam136]

@claude

[github-actions]

Claude finished @rishikeshdadam136's task —— View job

---

PR Review Analysis

Analyzing the Express wallet binding security changes...

Todo List:

• Read and analyze the main implementation in clientRoutes.ts
• Review test coverage in walletSignTx.ts
• Review test coverage in walletTxSignTSS.ts
• Provide comprehensive code review feedback
• Check for security considerations and best practices

Code Review

This PR implements a crucial security feature that prevents wallet transaction signing requests from being misdirected to different wallets. The implementation is solid with good test coverage.

✅ Strengths

1. Security Enhancement: The core security check in clientRoutes.ts:874-878 properly validates that txPrebuild.walletId matches the route wallet ID before any wallet lookup or signing operations.

2. Consistent Application: The validation is correctly applied to both:

   • Regular wallet signing: handleV2SignTxWallet (line 884)
   • TSS wallet signing: handleV2SignTSSWalletTx (line 552)

3. Early Validation: The check happens before wallet retrieval, preventing unnecessary operations and potential information leakage.

4. Comprehensive Test Coverage: Both test files include regression tests that verify the mismatch rejection:

   • walletSignTx.ts:37-64 - Tests wallet ID mismatch rejection
   • walletTxSignTSS.ts:37-65 - Tests TSS wallet ID mismatch rejection

5. Proper Error Handling: Returns a clear 400 status with descriptive error message: "Wallet ID in txPrebuild does not match the route wallet ID"

🔧 Implementation Details

The assertWalletIdMatchesRoute function is well-designed:

• Handles the case where prebuildWalletId is undefined (allows requests without wallet ID)
• Only rejects when there's an explicit mismatch
• Uses appropriate HTTP 400 status code for client errors

📋 Test Quality

The tests are thorough and properly structured:

• Mock the wallet methods to ensure the validation happens before wallet operations
• Verify that neither wallet lookup nor signing methods are called when validation fails
• Test both successful and failure scenarios
• Use realistic wallet IDs and transaction data

🎯 Security Impact

This change addresses a significant security vulnerability where a malicious request could potentially:

• Attempt to sign transactions for different wallets
• Bypass wallet-specific authorization checks
• Create confusion about which wallet is being used

Minor Suggestions

1. Consider logging: While not critical, adding a debug log for rejected requests might help with troubleshooting:

   function assertWalletIdMatchesRoute(routeWalletId: string, prebuildWalletId: string | undefined): void {
     if (prebuildWalletId !== undefined && prebuildWalletId !== routeWalletId) {
       logger.debug(`Rejected wallet signing request: route=${routeWalletId}, prebuild=${prebuildWalletId}`);
       throw new ApiResponseError('Wallet ID in txPrebuild does not match the route wallet ID', 400);
     }
   }

2. Input sanitization: The current implementation is safe, but consider if wallet IDs should be validated for format/length to prevent potential issues.

✅ Verdict

This is an excellent security improvement that:

• ✅ Addresses a real security concern
• ✅ Has minimal performance impact (early validation)
• ✅ Maintains backward compatibility
• ✅ Includes comprehensive test coverage
• ✅ Uses clear, maintainable code

Recommendation: Approve - This PR significantly improves security with well-tested, clean implementation.

---

Job Run | Branch: pt-000000-express-wallet-binding

[rishikeshdadam136]

Looks Good