fix: derive safe child key for spend sharing

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

@@ -5,6 +5,7 @@ import { BitGoBase } from '../bitgoBase';
 import { decryptKeychainPrivateKey, IKeychains, Keychain, KeychainWithEncryptedPrv } from '../keychain';
 import { deriveSafeChildHardenedFromXprv, parseDerivedFromParentWithHardenedPath } from '../safe/safeDerivation';
 import { IncorrectPasswordError } from '../errors';
+import type { DecryptedKeychainData } from './iWallet';
 
 export class InvalidRootKeychainSourceError extends Error {
   constructor(id: string, source: string | undefined) {
@@ -35,6 +36,14 @@ export class SafeOwnerSigningNotImplementedError extends Error {
   }
 }
 
+/** Thrown when wallet sharing is not implemented for this safe slot (TSS, ed25519 multisig, …). */
+export class SafeShareNotImplementedError extends Error {
+  constructor(walletId: string, detail: string) {
+    super(`Safe wallet ${walletId}: ${detail}`);
+    this.name = 'SafeShareNotImplementedError';
+  }
+}
+
 /** ed25519 onchain multisig (slot ④). Needs SLIP-0010, not secp256k1 BIP32. */
 const ED25519_ONCHAIN_FAMILIES = new Set(['algo', 'xlm', 'hbar']);
 
@@ -70,7 +79,11 @@ export async function fetchRootKeychainForSafeChild(
   return root as KeychainWithEncryptedPrv;
 }
 
-export interface ResolveSafeOwnerSigningPrvParams {
+/**
+ * Shared params for resolving safe-owner key material (owner signing and wallet sharing).
+ * Both resolvers extend this so the precondition/derivation path cannot diverge.
+ */
+export interface SafeKeyMaterialBaseParams {
   bitgo: BitGoBase;
   keychains: IKeychains;
   walletId: string;
@@ -83,31 +96,41 @@ export interface ResolveSafeOwnerSigningPrvParams {
   rootKeychain?: KeychainWithEncryptedPrv;
 }
 
+export type ResolveSafeOwnerSigningPrvParams = SafeKeyMaterialBaseParams;
+
+type SafeKeyMaterialSlot = 'tss' | 'ed25519';
+
+type ResolveSafeKeyMaterialParams = SafeKeyMaterialBaseParams & {
+  /** Constructs the not-implemented error for the current resolver (signing vs sharing). */
+  makeNotImplementedError: (slot: SafeKeyMaterialSlot, walletId: string) => Error;
+};
+
 /**
- * Resolve signing material for a safe owner (child key has no encryptedPrv).
+ * Shared core that resolves safe key material for a pub-only safe child. Returns the CHILD
+ * `{prv, pub}` only — never the root — so the root can never leak into a share document.
  *
  * Onchain secp256k1: decrypt root, hardened-derive at `derivedFromParentWithHardenedPath`
- * (`m/<n>'`), and verify the registered pub.
- * TSS and ed25519 onchain: throw — do not return root material or BIP32-derive the wrong curve.
- *
- * Do not use for wallet sharing — that must not receive root key material.
- * Call only when `isSafeChildPublicOnlyKeychain` is true.
+ * (`m/<n>'`), and verify the registered pub. TSS and ed25519 onchain throw via
+ * `makeNotImplementedError` — the caller constructs its own error class + message, so the
+ * guard set stays shared while signing/sharing report their own errors.
  */
-export async function resolveSafeOwnerSigningPrv(params: ResolveSafeOwnerSigningPrvParams): Promise<string> {
-  const { bitgo, keychains, walletId, multisigType, coinFamily, childKeychain, walletPassphrase } = params;
+async function resolveSafeKeyMaterial(params: ResolveSafeKeyMaterialParams): Promise<{ prv: string; pub: string }> {
+  const {
+    bitgo,
+    keychains,
+    walletId,
+    multisigType,
+    coinFamily,
+    childKeychain,
+    walletPassphrase,
+    makeNotImplementedError,
+  } = params;
 
   if (multisigType !== 'onchain') {
-    throw new SafeOwnerSigningNotImplementedError(
-      walletId,
-      'TSS owner signing from the root keyshare is not implemented. ' +
-        'Returning the root private key would expose material that can derive every child in this slot.'
-    );
+    throw makeNotImplementedError('tss', walletId);
   }
   if (ED25519_ONCHAIN_FAMILIES.has(coinFamily)) {
-    throw new SafeOwnerSigningNotImplementedError(
-      walletId,
-      `ed25519 multisig owner derivation (${coinFamily}) is not implemented; BIP32 would produce the wrong child key.`
-    );
+    throw makeNotImplementedError('ed25519', walletId);
   }
 
   const rootKeychain = params.rootKeychain ?? (await fetchRootKeychainForSafeChild(keychains, childKeychain));
@@ -130,5 +153,70 @@ export async function resolveSafeOwnerSigningPrv(params: ResolveSafeOwnerSigning
   if (derived.pub !== childKeychain.pub) {
     throw new SafeDerivedPublicKeyMismatchError(walletId, childKeychain.pub, derived.pub);
   }
-  return derived.prv;
+
+  if (derived.pub === rootKeychain.pub) {
+    throw new Error(`Safe wallet ${walletId}: derived child pub unexpectedly equals the root pub`);
+  }
+  return { prv: derived.prv, pub: derived.pub };
+}
+
+/**
+ * Resolve signing material for a safe owner (child key has no encryptedPrv).
+ *
+ * Onchain secp256k1: decrypt root, hardened-derive at `derivedFromParentWithHardenedPath`
+ * (`m/<n>'`), and verify the registered pub.
+ * TSS and ed25519 onchain: throw — do not return root material or BIP32-derive the wrong curve.
+ *
+ * Do not use for wallet sharing — that must not receive root key material.
+ * Call only when `isSafeChildPublicOnlyKeychain` is true.
+ */
+export async function resolveSafeOwnerSigningPrv(params: ResolveSafeOwnerSigningPrvParams): Promise<string> {
+  const { prv } = await resolveSafeKeyMaterial({
+    ...params,
+    makeNotImplementedError: (slot, walletId) =>
+      slot === 'ed25519'
+        ? new SafeOwnerSigningNotImplementedError(
+            walletId,
+            `ed25519 multisig owner derivation (${params.coinFamily}) is not implemented; BIP32 would produce the wrong child key.`
+          )
+        : new SafeOwnerSigningNotImplementedError(
+            walletId,
+            'TSS owner signing from the root keyshare is not implemented. ' +
+              'Returning the root private key would expose material that can derive every child in this slot.'
+          ),
+  });
+  return prv;
+}
+
+export interface ResolveSafeChildPrvForSharingParams extends SafeKeyMaterialBaseParams {
+  mpcAlgorithm?: 'ecdsa' | 'eddsa';
+}
+
+/** Slot-named not-implemented detail for wallet sharing. */
+function safeShareSlotDetail(slot: SafeKeyMaterialSlot, params: ResolveSafeChildPrvForSharingParams): string {
+  if (slot === 'ed25519') {
+    return `ed25519 multisig safe sharing (${params.coinFamily}) is not implemented; BIP32 would derive the wrong child.`;
+  }
+  return params.mpcAlgorithm === 'eddsa'
+    ? 'eddsaMpc safe sharing is not implemented (needs the EdDSA derive ceremony).'
+    : 'ecdsaMpc safe sharing is not implemented (needs the DKLS derive ceremony).';
+}
+
+/**
+ * Resolve sharing material for a safe owner (child key has no encryptedPrv).
+ *
+ * Onchain secp256k1: decrypt root, hardened-derive at `derivedFromParentWithHardenedPath`
+ * (`m/<n>'`), verify the registered pub, and return the CHILD `{prv, pub}` — never the root.
+ * TSS and ed25519 onchain: throw `SafeShareNotImplementedError` naming the slot + blocker.
+ *
+ * Call only when `isSafeChildPublicOnlyKeychain` is true.
+ */
+export async function resolveSafeChildPrvForSharing(
+  params: ResolveSafeChildPrvForSharingParams
+): Promise<DecryptedKeychainData> {
+  return resolveSafeKeyMaterial({
+    ...params,
+    makeNotImplementedError: (slot, walletId) =>
+      new SafeShareNotImplementedError(walletId, safeShareSlotDetail(slot, params)),
+  });
 }

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

@@ -61,6 +61,7 @@ import { buildParamKeys, BuildParams } from './BuildParams';
 import {
   fetchRootKeychainForSafeChild,
   isSafeChildPublicOnlyKeychain,
+  resolveSafeChildPrvForSharing,
   resolveSafeOwnerSigningPrv,
 } from './safeKeychain';
 import {
@@ -1760,6 +1761,18 @@ export class Wallet implements IWallet {
     return tryKeyChain(0);
   }
 
+  private async getSafeOwnerChildKeychain(): Promise<(Keychain & { parent: string }) | undefined> {
+    if (!this.safeId()) {
+      return undefined;
+    }
+    const userKeyId = this._wallet.keys?.[KeyIndices.USER];
+    if (!userKeyId) {
+      return undefined;
+    }
+    const keychain = await this.baseCoin.keychains().get({ id: userKeyId });
+    return isSafeChildPublicOnlyKeychain(this.safeId(), keychain) ? keychain : undefined;
+  }
+
   /**
    * Gets the unencrypted private key for this wallet (be careful!)
    * Requires wallet passphrase
@@ -1866,11 +1879,8 @@ export class Wallet implements IWallet {
       try {
         decryptedKeychain = await this.getDecryptedKeychainForSharing(params.walletPassphrase);
       } catch (e) {
-        if (e instanceof MissingEncryptedKeychainError) {
-          decryptedKeychain = undefined;
-        } else {
-          throw e;
-        }
+        this.rethrowUnlessColdWalletShare(e);
+        decryptedKeychain = undefined;
       }
     }
 
@@ -1961,6 +1971,28 @@ export class Wallet implements IWallet {
   async getDecryptedKeychainForSharing(
     walletPassphrase: string | undefined
   ): Promise<DecryptedKeychainData | undefined> {
+    /**
+     * For Safe owners: detect child safes first and derive the child private key from the root keychain if present.
+     * Skip `lnbtc` as it uses the user auth key instead
+     */
+    if (this.baseCoin.getFamily() !== 'lnbtc') {
+      const safeChildKeychain = await this.getSafeOwnerChildKeychain();
+      if (safeChildKeychain) {
+        if (!walletPassphrase) {
+          throw new Error('Missing walletPassphrase argument');
+        }
+        return resolveSafeChildPrvForSharing({
+          bitgo: this.bitgo,
+          keychains: this.baseCoin.keychains(),
+          walletId: this._wallet.id,
+          multisigType: this._wallet.multisigType,
+          coinFamily: this.baseCoin.getFamily(),
+          childKeychain: safeChildKeychain,
+          walletPassphrase,
+        });
+      }
+    }
+
     const keychain = await this.getEncryptedWalletKeychainForWalletSharing();
 
     if (!keychain.encryptedPrv) {
@@ -2031,6 +2063,18 @@ export class Wallet implements IWallet {
     return keychain;
   }
 
+  private rethrowUnlessColdWalletShare(e: unknown): void {
+    if (!(e instanceof MissingEncryptedKeychainError)) {
+      throw e;
+    }
+    if (this.safeId()) {
+      throw new MissingEncryptedKeychainError(
+        `Safe wallet ${this._wallet.id}: no keychain with an encryptedPrv and the safe child ` +
+          `could not be resolved; refusing to create a spend share without key material.`
+      );
+    }
+  }
+
   /**
    * Prepares a keychain for sharing with another user.
    * Fetches the wallet keychain, decrypts it, and encrypts it for the recipient.
@@ -2056,11 +2100,9 @@ export class Wallet implements IWallet {
       }
       return await this.encryptPrvForUser(keychain.prv, keychain.pub, pubkey, path, encryptionVersion);
     } catch (e) {
-      if (e instanceof MissingEncryptedKeychainError) {
-        // ignore this error because this looks like a cold wallet
-        return {};
-      }
-      throw e;
+      this.rethrowUnlessColdWalletShare(e);
+      // ignore this error because this looks like a cold wallet
+      return {};
     }
   }