Skip to content

fix(background): treat a wiped hardware wallet session as locked - #3090

Merged
JakeUrban merged 4 commits into
masterfrom
claude/laughing-lamport-ssez1a
Nov 12, 2026
Merged

JakeUrban merged 4 commits into
masterfrom
claude/laughing-lamport-ssez1a

Conversation

@JakeUrban

@JakeUrban JakeUrban commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

What

A hardware wallet session is now "unlocked" only when the session has a public key.

The change is in buildHasPrivateKeySelector (extension/src/background/ducks/session.ts). The old rule was:

  • A hardware wallet account is active, and
  • isHardwareWalletLocked is false.

The new rule adds one condition:

  • The session has a publicKey.

The rule for hot wallet (mnemonic) sessions does not change.

userActivity now uses buildHasPrivateKeySelector, as saveSettings does. Before, it had its own copy of the unlock check.

Why

Users with an active Ledger account can be stuck on the unlock screen. They type the correct password and click Unlock. A loading animation shows. Then nothing happens. No error shows. A wrong password shows the error correctly.

The sequence is:

  1. The browser clears storage.session. This occurs on a browser restart and on an extension reload or update.
  2. The background rebuilds the session from defaults. publicKey is "". isHardwareWalletLocked is false.
  3. The selector sees an active hardware wallet that is not locked. It returns hasPrivateKey: true.
  4. The popup saves hasPrivateKey: true in Redux. It sees no public key, so it opens the unlock screen.
  5. The unlock screen navigates only when hasPrivateKey changes from false to true. The value is already true when the screen mounts.
  6. The user unlocks. confirmPassword succeeds. hasPrivateKey stays true. The screen does not navigate.

This occurs each time the popup opens. The user cannot get past the unlock screen.

With this fix, step 3 returns false. The unlock screen mounts with hasPrivateKey: false. A successful unlock sets the public key (loginToAllAccounts dispatches logIn with the hardware wallet public key). hasPrivateKey changes to true, and the screen navigates.

Other effects:

  • loadAccount, confirmPassword, importHardwareWallet, saveSettings and userActivity use this selector. importHardwareWallet sets the public key before it reads the selector, so import is not affected.
  • userActivity rearms the idle alarm only when the selector says the wallet is unlocked. This gives two behavior changes:
    • A wiped hardware wallet session no longer rearms the alarm.
    • A hot wallet session with a hash key but an empty temporary store no longer rearms the alarm. loadAccount already reports this session as locked.

Tests

Selector tests (extension/src/background/helpers/__tests__/session.test.ts):

  • Updated: "returns true for unlocked hardware-wallet sessions" now sets a publicKey.
  • Added: "returns false for idle-locked hardware-wallet sessions".
  • Added: "returns false for a wiped hardware-wallet session with no public key". This test fails without the fix.

Flow test (extension/src/background/messageListener/__tests__/hardwareWalletWipedSessionUnlock.test.ts):

This test uses the real session Redux store, the real loadAccount and confirmPassword handlers, and the real loginToAllAccounts. It mocks only storage, the key manager, store persistence and the session broadcast. The wallet has one hot account and one Ledger account. The Ledger account is active. The session store starts empty, the same as after a wipe.

  • Correct password:
    1. loadAccount returns hasPrivateKey: false and an empty publicKey.
    2. confirmPassword succeeds. It returns the Ledger public key and hasPrivateKey: true.
    3. The password is checked against the hot keystore. The Ledger key ID is never loaded.
    4. loadAccount returns the Ledger public key and hasPrivateKey: true.
  • Wrong password: confirmPassword returns Incorrect password. loadAccount still returns hasPrivateKey: false.

Both flow cases fail without the fix. loadAccount returns hasPrivateKey: true for the wiped session.

userActivity tests (extension/src/background/messageListener/__tests__/userActivity.test.ts):

  • Updated: the hot wallet case now has a populated temporary store. The hardware wallet cases now set a publicKey.
  • Added: "does NOT reset the timer for a wiped hardware-wallet session with no public key".
  • Added: "does NOT reset the timer when the hash key exists but the temporary store is empty".

yarn test:ci passes, except testNodeCompat.js. That suite needs a built @stellar/varial-sdk package and is not related to this change.

Not done yet: a manual test with a real Ledger. Steps to reproduce:

  1. Import a Ledger account and make it the active account.
  2. Restart Chrome, or reload the extension from chrome://extensions.
  3. Open the popup and unlock with the correct password.

Before the fix: the unlock screen stays. After the fix: the account view opens.

Screenshots / video

N/A. There is no UI change. The fix changes when the unlock screen navigates.

Before After
Correct password: unlock screen stays, no error Correct password: account view opens

Checklist

  • I have read CONTRIBUTING.md and this PR meets its requirements.
  • This PR has narrow scope and does not mix refactoring with feature changes.
  • This PR adds tests for the new functionality or fixes.
  • These changes have been tested in both Chrome and Firefox.
  • This PR includes relevant before and after screenshots for UI changes.
  • I took the time to review my own PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01R8cp428MzVhQRRFYoqTVpA

When a hardware wallet account is active, buildHasPrivateKeySelector
returned true whenever isHardwareWalletLocked was false. The browser
wipes storage.session on restart and on extension reload/update. The
rehydrated session then has no public key, and isHardwareWalletLocked
falls back to false, so the empty session read as unlocked.

The popup saved hasPrivateKey: true, saw no public key, and routed to
the unlock screen. The unlock screen only navigates on a false -> true
change of hasPrivateKey, so a correct password did nothing and showed
no error.

Require a session public key before a hardware wallet session counts
as unlocked. Add selector tests for the wiped and idle-locked cases.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R8cp428MzVhQRRFYoqTVpA

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Because the change alters Ledger background unlock and session hydration, automated selector tests are insufficient while real-device Chrome and Firefox reload/restart validation remains outstanding.

No open issues 🎉

What changed in this PR

This PR fixes a hardware wallet unlock bug. After the browser wipes storage.session (on restart, reload, or update), the background rebuilds an empty session. The session selector then reports the wallet as unlocked, so the popup never leaves the unlock screen after a correct password. The fix requires a publicKey in the session before a hardware wallet counts as unlocked. Hot wallet (mnemonic) behavior is unchanged.

Changes:

  • buildHasPrivateKeySelector now returns true for a hardware wallet only when the session is not idle-locked and has a publicKey.
  • Tests now set a publicKey on the unlocked case, and add cases for an idle-locked session and a wiped session with no public key.
File Description
extension/​src/​background/​helpers/​__tests__/​session.test.ts Adds a public key to the unlocked test and adds tests for idle-locked and wiped hardware-wallet sessions.
extension/​src/​background/​ducks/​session.ts Adds a publicKey check to the hardware wallet unlock rule in buildHasPrivateKeySelector, with a comment that explains the wiped-session case.

Review effort: Lite

@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-58bfbe213c96efbf5ad8
Backend: V1 prod + V2 beta (no sandbox configured for @JakeUrban). SDF collaborators only — install instructions in the release description.

@JakeUrban
JakeUrban marked this pull request as ready for review November 12, 2026 18:52

@aristidesstaffieri aristidesstaffieri left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this lgtm, a couple of things we could add to harden these flows:

  • a test that covers this flow: wiped session → loadAccount returns false → confirmPassword succeeds with the hardware wallet key
  • userActivity could call the buildHasPrivateKeySelector selector the same way saveSettings does in order to avoid drift between the two.

Run the real loadAccount and confirmPassword handlers against a fresh
session store with a hardware wallet active. Check that loadAccount
reports the wiped session as locked, that the correct password unlocks
it with the hardware wallet public key, and that a wrong password
leaves it locked. Both cases fail without the selector fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R8cp428MzVhQRRFYoqTVpA
userActivity had its own copy of the unlock check. Use the shared
selector, as saveSettings does, so the idle alarm and loadAccount
cannot drift apart.

Behavior changes:
- A wiped hardware wallet session (no public key) no longer rearms the
  idle alarm.
- A hot wallet session with a hash key but an empty temporary store no
  longer rearms the idle alarm. loadAccount already reports it locked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R8cp428MzVhQRRFYoqTVpA
@JakeUrban
JakeUrban merged commit 595abe5 into master Nov 12, 2026
11 checks passed
@JakeUrban
JakeUrban deleted the claude/laughing-lamport-ssez1a branch November 12, 2026 21:59
CassioMG added a commit that referenced this pull request Nov 13, 2026
Brings in #3085 (Ledger option visible in light mode) and #3090 (a wiped
hardware wallet session is treated as locked).

Conflict:
- en/pt translation.json: master added "{{walletType}} logo" next to the
  Earn keys. Merged key by key; both sides' keys are kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants