From 6637f1e319844cd2ec06bba5ef918d33a463901b Mon Sep 17 00:00:00 2001 From: Anant Date: Thu, 8 Oct 2026 12:30:15 +0530 Subject: [PATCH 1/3] Redact stored verifier from legacy signer migration response --- .../assistedKeys.ts/ss-signer/signerV3.ts | 7 ++- tests/unit/signerV3.test.ts | 61 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/services/assistedKeys.ts/ss-signer/signerV3.ts b/src/services/assistedKeys.ts/ss-signer/signerV3.ts index 66a98cc..cbe0be0 100644 --- a/src/services/assistedKeys.ts/ss-signer/signerV3.ts +++ b/src/services/assistedKeys.ts/ss-signer/signerV3.ts @@ -1059,7 +1059,12 @@ export class SignerV3 { bhXpub: xpub, masterFingerprint, derivationPath, - verification: policy.verification, + // This migration reuses an existing verifier. Older Keeper clients only + // consume migrationSuccessful, so never send the stored TOTP secret back. + verification: { + method: policy.verification.method, + verifier: null, + }, }, }; }; diff --git a/tests/unit/signerV3.test.ts b/tests/unit/signerV3.test.ts index 210d26a..ba9be4f 100644 --- a/tests/unit/signerV3.test.ts +++ b/tests/unit/signerV3.test.ts @@ -2,6 +2,7 @@ import SignerV3 from "../../src/services/assistedKeys.ts/ss-signer/signerV3"; import dbV2 from "../../src/databases/dbV2"; import bitHyveWallet from "../../src/wallet/bithyve"; import { + CosignersMapUpdateAction, VerificationType, PermittedAction, } from "../../src/interfaces/signer"; @@ -1654,4 +1655,64 @@ describe("SignerV3", () => { ).rejects.toThrow("Policy mismatch: spending restrictions do not match"); }); }); + + describe("migrateSignersV2ToV3", () => { + it("migrates a legacy signer without returning its stored verifier", async () => { + const storedVerifier = "UNIT_TEST_STORED_TOTP_SECRET"; + const policy = { + verification: { + method: VerificationType.TWO_FA, + verifier: storedVerifier, + }, + restrictions: { none: false, maxTransactionAmount: 10000 }, + }; + const save = jest.fn((callback) => callback && callback()); + const signerV3Model: any = jest.fn().mockImplementation((data) => ({ + ...data, + save, + })); + signerV3Model.find = jest.fn().mockResolvedValue([]); + (dbV2.getSignerV2Model as jest.Mock).mockReturnValue({ + find: jest.fn().mockResolvedValue([{ xIndex: mockSigner.xIndex, policy }]), + }); + (dbV2.getSignerV3Model as jest.Mock).mockReturnValue(signerV3Model); + const insertMany = jest.fn().mockResolvedValue([]); + const cosignersMapModel: any = jest.fn().mockImplementation((data) => data); + cosignersMapModel.find = jest.fn().mockResolvedValue([]); + cosignersMapModel.insertMany = insertMany; + (dbV2.getCoSignersToSignerMapV3Model as jest.Mock).mockReturnValue(cosignersMapModel); + const derive = jest.spyOn(bitHyveWallet, "getRandomXpub").mockReturnValue({ + xpub: mockSigner.xpub, + xfp: mockSigner.xfp, + masterFingerprint: mockSigner.masterFingerprint, + derivationPath: mockSigner.derivationPath, + xIndex: mockSigner.xIndex, + }); + const mapUpdates = [{ + cosignersId: "unit-test-cosigners", + signerId: mockSigner.xfp, + action: CosignersMapUpdateAction.ADD, + }]; + + try { + const result = await SignerV3.migrateSignersV2ToV3( + "unit-test-vault", + "unit-test-app", + mapUpdates + ); + + expect(result.migrationSuccessful).toBe(true); + expect(result.setupData.verification).toEqual({ + method: VerificationType.TWO_FA, + verifier: null, + }); + expect(JSON.stringify(result)).not.toContain(storedVerifier); + expect(signerV3Model).toHaveBeenCalledWith(expect.objectContaining({ policy })); + expect(save).toHaveBeenCalled(); + expect(insertMany).toHaveBeenCalled(); + } finally { + derive.mockRestore(); + } + }); + }); }); From bfde030311aba552925c9ae52df738947d782f1d Mon Sep 17 00:00:00 2001 From: Anant Date: Thu, 8 Oct 2026 17:50:27 +0530 Subject: [PATCH 2/3] Cover legacy migration retry redaction with V2 verifier shape --- tests/unit/signerV3.test.ts | 49 ++++++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/tests/unit/signerV3.test.ts b/tests/unit/signerV3.test.ts index ba9be4f..cce05f6 100644 --- a/tests/unit/signerV3.test.ts +++ b/tests/unit/signerV3.test.ts @@ -1658,11 +1658,11 @@ describe("SignerV3", () => { describe("migrateSignersV2ToV3", () => { it("migrates a legacy signer without returning its stored verifier", async () => { - const storedVerifier = "UNIT_TEST_STORED_TOTP_SECRET"; + const storedVerifierKey = "UNIT_TEST_STORED_TOTP_SECRET"; const policy = { verification: { method: VerificationType.TWO_FA, - verifier: storedVerifier, + verifier: { twoFAKey: storedVerifierKey }, }, restrictions: { none: false, maxTransactionAmount: 10000 }, }; @@ -1706,7 +1706,7 @@ describe("SignerV3", () => { method: VerificationType.TWO_FA, verifier: null, }); - expect(JSON.stringify(result)).not.toContain(storedVerifier); + expect(JSON.stringify(result)).not.toContain(storedVerifierKey); expect(signerV3Model).toHaveBeenCalledWith(expect.objectContaining({ policy })); expect(save).toHaveBeenCalled(); expect(insertMany).toHaveBeenCalled(); @@ -1714,5 +1714,48 @@ describe("SignerV3", () => { derive.mockRestore(); } }); + + it("keeps the verifier redacted when retrying after a V3 signer was saved", async () => { + const storedVerifierKey = "UNIT_TEST_RETRY_TOTP_SECRET"; + const policy = { + verification: { + method: VerificationType.TWO_FA, + verifier: { twoFAKey: storedVerifierKey }, + }, + }; + const signerV3Model: any = jest.fn(); + signerV3Model.find = jest.fn().mockResolvedValue([{ id: mockSigner.xfp, policy }]); + (dbV2.getSignerV2Model as jest.Mock).mockReturnValue({ + find: jest.fn().mockResolvedValue([{ xIndex: mockSigner.xIndex, policy }]), + }); + (dbV2.getSignerV3Model as jest.Mock).mockReturnValue(signerV3Model); + const cosignersMapModel: any = jest.fn().mockImplementation((data) => data); + cosignersMapModel.find = jest.fn().mockResolvedValue([]); + cosignersMapModel.insertMany = jest.fn().mockResolvedValue([]); + (dbV2.getCoSignersToSignerMapV3Model as jest.Mock).mockReturnValue(cosignersMapModel); + const derive = jest.spyOn(bitHyveWallet, "getRandomXpub").mockReturnValue({ + xpub: mockSigner.xpub, + xfp: mockSigner.xfp, + masterFingerprint: mockSigner.masterFingerprint, + derivationPath: mockSigner.derivationPath, + xIndex: mockSigner.xIndex, + }); + + try { + const result = await SignerV3.migrateSignersV2ToV3( + "unit-test-vault", + "unit-test-app", + [{ cosignersId: "unit-test-cosigners", signerId: mockSigner.xfp, action: CosignersMapUpdateAction.ADD }] + ); + + expect(result.migrationSuccessful).toBe(true); + expect(result.setupData.verification).toEqual({ method: VerificationType.TWO_FA, verifier: null }); + expect(JSON.stringify(result)).not.toContain(storedVerifierKey); + expect(signerV3Model).not.toHaveBeenCalled(); + expect(cosignersMapModel.insertMany).toHaveBeenCalled(); + } finally { + derive.mockRestore(); + } + }); }); }); From 7c14b6e67325eab6be95fa1a5e03e855d96e985b Mon Sep 17 00:00:00 2001 From: Anant Date: Thu, 8 Oct 2026 22:10:26 +0530 Subject: [PATCH 3/3] Normalize legacy verifier before V3 migration save --- src/services/assistedKeys.ts/ss-signer/signerV3.ts | 11 ++++++++++- tests/unit/signerV3.test.ts | 14 ++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/services/assistedKeys.ts/ss-signer/signerV3.ts b/src/services/assistedKeys.ts/ss-signer/signerV3.ts index cbe0be0..1307761 100644 --- a/src/services/assistedKeys.ts/ss-signer/signerV3.ts +++ b/src/services/assistedKeys.ts/ss-signer/signerV3.ts @@ -1035,11 +1035,20 @@ export class SignerV3 { let [docV3] = await signerV3Model.find({ id: xfp }); // won't be available typically, unless previous migration // request failed at updateCosignersToSignerMap step if (!docV3) { + // V2 stores the TOTP key inside verifier.twoFAKey, while V3 stores the + // key as a string. Passing the V2 subdocument to Mongoose stringifies it. + const storedVerifier = policy?.verification?.verifier; + const verifier = typeof storedVerifier === "string" ? storedVerifier : storedVerifier?.twoFAKey; + if (typeof verifier !== "string" || !verifier) throw new Error("Invalid legacy signer verifier"); + const policyV3 = { + ...(typeof policy.toObject === "function" ? policy.toObject() : policy), + verification: { method: policy.verification.method, verifier }, + }; const signerInstance = new signerV3Model({ id: xfp, isBIP85, xIndex, - policy, + policy: policyV3, }); await signerInstance.save((err) => { diff --git a/tests/unit/signerV3.test.ts b/tests/unit/signerV3.test.ts index cce05f6..3e52cb0 100644 --- a/tests/unit/signerV3.test.ts +++ b/tests/unit/signerV3.test.ts @@ -1658,13 +1658,18 @@ describe("SignerV3", () => { describe("migrateSignersV2ToV3", () => { it("migrates a legacy signer without returning its stored verifier", async () => { - const storedVerifierKey = "UNIT_TEST_STORED_TOTP_SECRET"; + const storedVerifierKey = "JBSWY3DPEHPK3PXP"; + const verificationToken = authenticator.generate(storedVerifierKey); const policy = { verification: { method: VerificationType.TWO_FA, verifier: { twoFAKey: storedVerifierKey }, }, restrictions: { none: false, maxTransactionAmount: 10000 }, + toObject: jest.fn(() => ({ + verification: { method: VerificationType.TWO_FA, verifier: { twoFAKey: storedVerifierKey } }, + restrictions: { none: false, maxTransactionAmount: 10000 }, + })), }; const save = jest.fn((callback) => callback && callback()); const signerV3Model: any = jest.fn().mockImplementation((data) => ({ @@ -1707,7 +1712,12 @@ describe("SignerV3", () => { verifier: null, }); expect(JSON.stringify(result)).not.toContain(storedVerifierKey); - expect(signerV3Model).toHaveBeenCalledWith(expect.objectContaining({ policy })); + const savedPolicy = signerV3Model.mock.calls[0][0].policy; + expect(policy.toObject).toHaveBeenCalled(); + expect(savedPolicy.verification).toEqual({ method: VerificationType.TWO_FA, verifier: storedVerifierKey }); + expect(savedPolicy.restrictions).toEqual(policy.restrictions); + expect(authenticator.verify({ secret: savedPolicy.verification.verifier, token: verificationToken })).toBe(true); + expect(policy.verification.verifier).toEqual({ twoFAKey: storedVerifierKey }); expect(save).toHaveBeenCalled(); expect(insertMany).toHaveBeenCalled(); } finally {