From 341f4837365cb1822e753ea7766ec0c16c6036b1 Mon Sep 17 00:00:00 2001 From: Aristides Staffieri Date: Fri, 13 Nov 2026 15:44:46 -0700 Subject: [PATCH] fix(auth): never sign as a conflicting source when the pin is missing --- .../__tests__/signing-identity.test.ts | 62 +++++++++++++++++++ .../background/helpers/signing-identity.ts | 22 +++++-- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/extension/src/background/helpers/__tests__/signing-identity.test.ts b/extension/src/background/helpers/__tests__/signing-identity.test.ts index ee56cdc744..b5a8dc8426 100644 --- a/extension/src/background/helpers/__tests__/signing-identity.test.ts +++ b/extension/src/background/helpers/__tests__/signing-identity.test.ts @@ -14,6 +14,7 @@ import { sessionSlice } from "background/ducks/session"; import { KEY_ID, KEY_ID_LIST, + LINK_STATUS_ID, PINNED_SOURCE_ID, } from "constants/localStorageTypes"; @@ -262,6 +263,67 @@ describe("getSigningIdentity", () => { expect((await getSigningIdentity(stores))?.sourceId).toBe(phraseAId); }); + describe("when the pinned source is not held", () => { + // A phrase and an imported key, neither of them the pinned source. + const unlockWithoutPin = async (status: Record) => { + const stores = await unlock({ + keystores: { "key-imported": importedSecret }, + mnemonics: { [phraseAId]: phraseA.mnemonic }, + order: [], + }); + await setPinnedSourceId({ + localStore: stores.localStore, + sourceId: "f".repeat(64), + }); + await stores.localStore.setItem(LINK_STATUS_ID, status); + return stores; + }; + + it("signs with a linked source over a phrase another user owns", async () => { + const stores = await unlockWithoutPin({ + [phraseAId]: "conflict", + [importedId]: "linked", + }); + + expect((await getSigningIdentity(stores))?.sourceId).toBe(importedId); + expect(await getPinnedSourceId(stores)).toBe("f".repeat(64)); + }); + + it("signs with an unconfirmed source over one another user owns", async () => { + const stores = await unlockWithoutPin({ [phraseAId]: "conflict" }); + + expect((await getSigningIdentity(stores))?.sourceId).toBe(importedId); + }); + + it("prefers a linked source over an unconfirmed phrase", async () => { + const stores = await unlockWithoutPin({ [importedId]: "linked" }); + + expect((await getSigningIdentity(stores))?.sourceId).toBe(importedId); + }); + + it("returns null when every held source belongs to another user", async () => { + const stores = await unlockWithoutPin({ + [phraseAId]: "conflict", + [importedId]: "conflict", + }); + + expect(await getSigningIdentity(stores)).toBeNull(); + }); + }); + + it("signs with the held pinned source whatever the link status says", async () => { + const stores = await unlock({ + keystores: { "key-imported": importedSecret }, + mnemonics: { [phraseAId]: phraseA.mnemonic }, + order: [phrase, key], + }); + await stores.localStore.setItem(LINK_STATUS_ID, { + [importedId]: "linked", + }); + + expect((await getSigningIdentity(stores))?.sourceId).toBe(phraseAId); + }); + it("signs as the earliest registered source, without pinning it, when the wallet has no pin", async () => { // Held sources list phrases first, so the key is not the first held one. const stores = await unlock({ diff --git a/extension/src/background/helpers/signing-identity.ts b/extension/src/background/helpers/signing-identity.ts index 29e9c3bfc3..646f052640 100644 --- a/extension/src/background/helpers/signing-identity.ts +++ b/extension/src/background/helpers/signing-identity.ts @@ -8,6 +8,7 @@ import { getFirstSourceId, getPinnedSourceId, } from "background/helpers/secret-sources"; +import { getLinkStatus } from "background/helpers/link-status"; import { getWalletSources } from "background/helpers/wallet-sources"; /* @@ -42,10 +43,14 @@ export interface SigningIdentity { * - The pinned source is held: sign with it. With no pin, as after a failed * pin write, the earliest registered source stands in for it (the source * the next registration or unlock will pin). - * - That source is not held, as after a restore that recovered only some - * sources: sign with the first held source (phrases first) and leave the - * pin alone. Nothing re-pins from the server yet, so the pin stays put, - * and a source that failed to derive this unlock cannot move it either. + * - That source is not held, as when it failed to derive this unlock: sign + * with a held source the server listed in this wallet's cluster + * ("linked"), else one it has not answered for yet, phrases first in each + * case. Never sign with a source the server reported as another user's + * ("conflict"): that would speak as that user. With only those held, + * return null. The pin stays put, so a source that failed to derive this + * unlock cannot move it; moving it waits for account removal, the first + * way a pinned source can leave the wallet for good. * - Nothing pinned and nothing registered, as when a request overlaps a * wallet wipe: sign with the first held phrase, or return null when no * phrase is held. Nothing is persisted, so a wrong guess lasts one request. @@ -85,8 +90,13 @@ export const getSigningIdentity = async ({ } captureException("getSigningIdentity: the pinned source is not held"); - const [{ id: firstHeldId }] = sources; - return { sourceId: firstHeldId, keypair: signers.get(firstHeldId)! }; + const status = await getLinkStatus({ localStore }); + const fallback = + sources.find(({ id }) => status[id] === "linked") ?? + sources.find(({ id }) => status[id] === undefined); + return fallback + ? { sourceId: fallback.id, keypair: signers.get(fallback.id)! } + : null; } catch (e) { // A locked or empty wallet is handled above, so anything reaching here // is unexpected (a WebCrypto/PBKDF2 failure, a storage read error).