From a08ba6a66d209968520af4ea3b15434a2fbe2832 Mon Sep 17 00:00:00 2001 From: clawbot Date: Wed, 12 Aug 2026 12:07:48 +0200 Subject: [PATCH] fix: filter the restored view stack against RESTORABLE_VIEWS (closes #224) The persisted view stack was restored verbatim. RESTORABLE_VIEWS stopped the popup opening ONTO a view it will not re-render, but nothing kept such a view out of the stack, so Back could land on a screen whose content was deliberately never restored. No secret leaks -- those views are blank precisely because nothing is restored into them; this is a navigation defect. loadState() now truncates the stored stack at the first entry outside RESTORABLE_VIEWS, dropping it and everything above it. Truncating rather than splicing keeps the result a prefix of what was stored, so every surviving entry keeps the Back target it had; splicing would silently re-point the entry above the hole at a different screen. Filtering on load rather than on save is what makes it retroactive for stacks already in storage, and leaves the live in-session stack whole, which it should be. The general case where Back lands on a blank screen even for restorable views, because goBack() re-renders nothing, is separate and tracked at #268. --- TODO.md | 8 ++++ src/shared/state.js | 37 ++++++++++++++- tests/state.test.js | 110 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 1 deletion(-) diff --git a/TODO.md b/TODO.md index e398fd3..f8792a3 100644 --- a/TODO.md +++ b/TODO.md @@ -44,6 +44,14 @@ undefined identifiers, which is how # Completed Steps +- 2026-08-12: The restored navigation stack is filtered against + `RESTORABLE_VIEWS` on load, truncated at the first entry the popup would not + render so that every surviving entry keeps the Back target it had. Back after + reopening can no longer land on a view the popup declined to restore, such as + `export-privkey` or `show-phrase` + ([#224](https://git.eeqj.de/sneak/AutistMask/issues/224)). Restorable views in + the stack are still unhidden without being re-rendered; that is tracked + separately in ([#268](https://git.eeqj.de/sneak/AutistMask/issues/268)). - 2026-08-12: One wording for a rejected password on every screen that asks for one — the send confirmation and the delete-wallet confirmation no longer say "Wrong password." (a fragment, which `RULES.md` Language & Labeling forbids) diff --git a/src/shared/state.js b/src/shared/state.js index b7e627f..82456fe 100644 --- a/src/shared/state.js +++ b/src/shared/state.js @@ -2,6 +2,8 @@ const { DEFAULT_RPC_URL, DEFAULT_BLOCKSCOUT_URL } = require("./constants"); const { networkById } = require("./networks"); +// Dependency-free constant module; safe to pull into a background bundle. +const { RESTORABLE_VIEWS } = require("../popup/restorableViews"); const storageApi = typeof browser !== "undefined" @@ -43,6 +45,39 @@ const state = { viewStack: [], }; +// Keep only the leading run of stored views the popup is willing to render. +// +// restoreView() refuses to reopen ONTO a non-restorable view, but the stack +// behind it used to be restored verbatim, so Back could walk onto a screen +// whose content is deliberately never re-rendered — and "show-phrase" has no +// Back control to leave by. Truncating at the first such entry instead of +// splicing it out keeps the result a prefix of the stored stack, so every +// surviving entry's Back target is exactly the one it had; splicing would +// silently re-point the entry above the hole at a different screen. +// +// Filtering happens here on load rather than in saveState(): the live +// in-session stack is legitimate (the screen really is rendered while the +// popup is open), and only a load-side filter also repairs the stacks +// already in storage, including ones written before a view left the set. +function restorableStack(stored, currentView) { + // A stored stack that is missing or not an array keeps nothing, but it + // still goes through the never-empty rule below rather than returning + // early: otherwise a corrupt stack would depend on exactly the goBack() + // fallback that the explicit ["main"] exists in order not to depend on. + const source = Array.isArray(stored) ? stored : []; + const cut = source.findIndex((view) => !RESTORABLE_VIEWS.has(view)); + const kept = cut === -1 ? source.slice() : source.slice(0, cut); + // A view restored below the root still needs somewhere for Back to go. + if ( + kept.length === 0 && + currentView !== "main" && + RESTORABLE_VIEWS.has(currentView) + ) { + return ["main"]; + } + return kept; +} + // Return the network configuration for the currently selected network. function currentNetwork() { return networkById(state.networkId); @@ -150,7 +185,7 @@ async function loadState() { saved.selectedAddress !== undefined ? saved.selectedAddress : null; state.selectedToken = saved.selectedToken || null; state.viewData = saved.viewData || {}; - state.viewStack = Array.isArray(saved.viewStack) ? saved.viewStack : []; + state.viewStack = restorableStack(saved.viewStack, state.currentView); } } diff --git a/tests/state.test.js b/tests/state.test.js index 70a1ef0..af67fc8 100644 --- a/tests/state.test.js +++ b/tests/state.test.js @@ -159,3 +159,113 @@ describe("hideSpoofedSymbols persistence", () => { expect(second.mod.state.hideSpoofedSymbols).toBe(true); }); }); + +// restoreView() refuses to reopen ONTO a non-restorable view, but the stack +// behind it was restored verbatim, so Back could still walk onto a screen +// whose content is deliberately never re-rendered — and "show-phrase" has no +// Back control of its own to leave by. The stack is filtered on load, at the +// first entry the popup would not render, and everything above it goes too: +// those entries were reached THROUGH the dropped one. +describe("restored viewStack is filtered against RESTORABLE_VIEWS", () => { + const NON_RESTORABLE = ["export-privkey", "show-phrase"]; + + function restoredStack(viewStack, currentView = "settings") { + return loadModuleWith({ + wallets: oneWallet(), + currentView, + viewStack, + }); + } + + test("a non-restorable view at the top of the stack is dropped", async () => { + const { mod } = restoredStack(["main", "address", "export-privkey"]); + await mod.loadState(); + expect(mod.state.viewStack).toEqual(["main", "address"]); + }); + + test("a non-restorable view in the middle truncates the stack there", async () => { + const { mod } = restoredStack(["main", "show-phrase", "address"]); + await mod.loadState(); + expect(mod.state.viewStack).toEqual(["main"]); + }); + + // Truncating a stack rooted at a non-restorable view leaves nothing, and + // the restored view still needs somewhere for Back to go. + test("a non-restorable view at the bottom leaves main to go back to", async () => { + const { mod } = restoredStack(["export-privkey", "address", "receive"]); + await mod.loadState(); + expect(mod.state.viewStack).toEqual(["main"]); + }); + + test("no restored stack retains a secret-bearing view", async () => { + for (const view of NON_RESTORABLE) { + const { mod } = restoredStack(["main", "address", view, "receive"]); + await mod.loadState(); + expect(mod.state.viewStack).not.toContain(view); + } + }); + + // The rule is "views the popup will render", not a blocklist of the two + // secret screens: a name no longer in the set (or never a view at all) + // has to go the same way. + test("a name that is not a restorable view at all is dropped", async () => { + const { mod } = restoredStack(["main", "welcome", "address"]); + await mod.loadState(); + expect(mod.state.viewStack).toEqual(["main"]); + }); + + // Restorable entries are kept verbatim. That they are then unhidden + // without being re-rendered is a separate defect, tracked in #268; this + // filter is only about views the popup declined to restore. + test("an ordinary restorable stack is restored unchanged", async () => { + const stack = ["main", "address", "address-token"]; + const { mod } = restoredStack(stack); + await mod.loadState(); + expect(mod.state.viewStack).toEqual(stack); + }); + + test("restoring onto main keeps the stack empty", async () => { + const { mod } = restoredStack(["show-phrase"], "main"); + await mod.loadState(); + expect(mod.state.viewStack).toEqual([]); + }); + + // main is not the only view that gets no ["main"] beneath it: restoreView() + // will not reopen onto a non-restorable view either, so nothing is left for + // Back to sit under and the stack stays empty. + test("restoring onto a view the popup will not reopen keeps the stack empty", async () => { + const { mod } = restoredStack(["export-privkey"], "show-phrase"); + await mod.loadState(); + expect(mod.state.viewStack).toEqual([]); + }); + + // Not an array means nothing survives, but the never-empty rule still + // applies: a corrupt stack must not leave a restored view with no Back + // target of its own. + test("a stack that is not an array still gets main beneath a restored view", async () => { + const { mod } = restoredStack("main"); + await mod.loadState(); + expect(mod.state.viewStack).toEqual(["main"]); + }); + + test("a stack that is not an array loads as empty under main", async () => { + const { mod } = restoredStack({ 0: "main" }, "main"); + await mod.loadState(); + expect(mod.state.viewStack).toEqual([]); + }); + + // Filtering belongs on load, not on save: the live in-session stack is + // legitimate — the user really is one Back away from a screen that is + // rendered right now — and only a load-side filter also cleans the + // stacks already sitting in storage. + test("saveState persists the live stack verbatim", async () => { + const { mod, set } = loadModuleWith(null); + mod.state.viewStack = ["main", "address", "export-privkey"]; + await mod.saveState(); + expect(set).toHaveBeenCalledWith({ + autistmask: expect.objectContaining({ + viewStack: ["main", "address", "export-privkey"], + }), + }); + }); +});