fix: derive safe child key for spend sharing

[pranishnepal]

What

Safe-minted wallets have pub-only children (owner material stays on the root), so the share path's scan for an encrypted user key wrongly treated them as cold and sent skipKeychain=true

This PR fixes the issue to derive the key correctly and account for safe shared wallets.

Ticket: WCN-2429

[linear-code]

WCN-2429

[github-actions]

⚠️ Unit tests are failing on Node 26.x (Current release line, non-blocking). This is not an LTS version yet, so it does not block merge, but it signals an incompatibility to fix before Node 26.x becomes LTS.

View run

[davidkaplanbitgo]

@claude

[github-actions]

Claude finished @davidkaplanbitgo's task —— View job

---

Code Review for PR #9657 ✅

Summary

This PR fixes an important security issue where safe-minted wallets were being incorrectly treated as cold wallets during spend sharing, causing them to bypass key derivation and send skipKeychain=true. The fix properly detects safe child keychains and derives the child private key from the root keychain for sharing purposes.

Todo List:

• Read and understand the repository setup (CLAUDE.md)
• Analyze the modified safeKeychain.ts file
• Analyze the modified wallet.ts file
• Review the new test file safeShareWallet.ts
• Check for code quality, security, and best practices
• Provide comprehensive review feedback

---

Code Review Findings

✅ Strengths

1. Security-First Design: The fix properly ensures that root key material never leaks into share documents. The resolveSafeKeyMaterial function explicitly returns only child {prv, pub} and validates that the derived child pub doesn't equal the root pub (safeKeychain.ts:157-159).

2. Excellent Code Organization: The refactoring in safeKeychain.ts creates a shared resolveSafeKeyMaterial core function that both signing and sharing can use, preventing code duplication and ensuring consistent derivation logic.

3. Comprehensive Error Handling:

   • New SafeShareNotImplementedError for unsupported wallet types
   • Proper validation that derived public keys match registered keys (SafeDerivedPublicKeyMismatchError)
   • Clear error messages distinguishing between TSS and ed25519 limitations

4. Robust Testing: The test suite (safeShareWallet.ts) is thorough and covers:

   • Normal safe wallet sharing flow
   • Edge cases (wrong passphrase, missing passphrase, pub key mismatch)
   • Unsupported wallet types (TSS, ed25519)
   • Regression cases (cold wallets, sharee re-sharing, non-safe wallets)

5. Type Safety: Proper TypeScript types throughout, avoiding any types as per repository guidelines.

✅ Architecture & Logic

1. Correct Problem Identification: The fix properly identifies that safe-minted wallets have pub-only children (no encryptedPrv at child level) and need special handling in wallet.ts:1978-1994.

2. Fail-Safe Design: The code fails closed on mismatched keys rather than silently proceeding, which is excellent for security.

3. Proper Abstraction: The SafeKeyMaterialBaseParams interface ensures both signing and sharing use identical parameters, preventing divergence in derivation paths.

🔍 Minor Considerations

1. Performance: The code fetches keychains sequentially in some paths. This appears intentional for security validation and is appropriate for the use case.

2. Special Case Handling: The lnbtc exclusion in wallet.ts:1978 is properly documented and tested, showing awareness of edge cases.

3. Error Message Clarity: Error messages are descriptive and include wallet IDs, which will help with debugging in production.

🔒 Security Assessment

• ✅ Root Key Protection: Root private keys are never exposed in share documents
• ✅ Key Derivation Validation: Derived public keys are verified against registered keys
• ✅ Unsupported Path Blocking: TSS and ed25519 paths correctly throw errors rather than attempting unsafe operations
• ✅ Passphrase Handling: Proper validation that passphrase is required for spend shares

📋 Code Quality

• ✅ Follows Repository Guidelines: Uses proper TypeScript patterns, conventional commits
• ✅ Documentation: Code is well-commented explaining the security implications
• ✅ Testing: Comprehensive test coverage including edge cases and regressions
• ✅ Error Handling: Proper custom error classes with descriptive messages

Overall Assessment

This is a well-implemented security fix that:

1. ✅ Correctly identifies and solves the core problem
2. ✅ Maintains strong security guarantees
3. ✅ Uses proper abstractions to prevent code duplication
4. ✅ Includes comprehensive testing
5. ✅ Follows repository coding standards

The implementation demonstrates good understanding of the cryptographic requirements and security implications of multi-signature wallet sharing. The code is production-ready.

Recommendation: ✅ APPROVE - This is a solid fix that addresses an important security issue while maintaining code quality and test coverage.

---

View run | Branch: WCN-2429