Skip to content

Commit ed4245e

Browse files
thephezclaude
andcommitted
fix(dashnote): tighten secret retention and session-recovery semantics
- Drop the module-scoped previewCache in useWifPreview: cached raw WIFs survived modal close/reopen for the page lifetime, extending secret retention beyond the form. Resolution state is now strictly component-local. - Move the loadLoginModule() await inside the try in useWifPreview so a chunk-fetch failure collapses to idle instead of stranding the preview on "checking". - Restore the active session in SessionContext's login catch path when priorStatus is "authenticated": typing a bad secret while signed in no longer demotes the user to browsing, even when a remembered identity is on disk. Switch flows already logout() before login() and are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 2b2c5a0 commit ed4245e

4 files changed

Lines changed: 56 additions & 96 deletions

File tree

example-apps/dashnote/src/hooks/useWifPreview.ts

Lines changed: 28 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -39,22 +39,6 @@ const IDLE: WifPreviewState = { status: "idle" };
3939
const CHECKING: WifPreviewState = { status: "checking" };
4040
const DEBOUNCE_MS = 400;
4141

42-
interface Resolved {
43-
wif: string;
44-
state: WifPreviewState;
45-
}
46-
47-
// Module-scoped cache: a WIF that resolved to a given identity (or to an
48-
// actionable error) yields the same answer on every future paste, so survive
49-
// modal close/reopen. Idle outcomes (UnknownIdentity / network blips) are
50-
// not cached — see the resolver below.
51-
const previewCache = new Map<string, WifPreviewState>();
52-
53-
/** Test-only: reset the module-scoped preview cache between tests. */
54-
export function _resetWifPreviewCacheForTests(): void {
55-
previewCache.clear();
56-
}
57-
5842
/**
5943
* Eagerly resolves a pasted WIF to its owning identity (and DPNS name) so
6044
* the user gets pre-submit confirmation and "wrong key type" feedback before
@@ -66,8 +50,11 @@ export function _resetWifPreviewCacheForTests(): void {
6650
* committed to login yet, so we don't surface UnknownIdentityError or network
6751
* failures as UI errors. The actual login path will surface them on submit.
6852
*
69-
* Results are cached per WIF for the lifetime of the hook so re-typing the
70-
* same key doesn't re-query.
53+
* Resolution state is component-local: changing the WIF cancels any in-flight
54+
* resolver and clears the prior result. We do NOT cache resolved outcomes
55+
* across renders — keeping a Map of pasted secrets keyed by raw WIF would
56+
* extend secret retention beyond the form lifecycle for no real UX win
57+
* (re-pasting the exact same WIF is rare).
7158
*/
7259
export function useWifPreview(
7360
sdk: DashSdk | null,
@@ -76,24 +63,27 @@ export function useWifPreview(
7663
): WifPreviewState {
7764
const trimmed = secret.trim();
7865
const gateOk = enabled && Boolean(sdk) && looksLikeWif(trimmed);
79-
const cached = gateOk ? previewCache.get(trimmed) : undefined;
8066

81-
// The resolver tags its result with the WIF that produced it; the render
82-
// path ignores stale results from a previous WIF. This avoids any setState
83-
// in the effect body or cleanup.
84-
const [resolved, setResolved] = useState<Resolved | null>(null);
67+
// The resolver tags its result with the WIF that produced it; if `trimmed`
68+
// changes between scheduling and rendering, we ignore the stale result at
69+
// the bottom of this hook. This avoids any setState in the effect body.
70+
// Note: `wif` here is component-local state — it lives only as long as the
71+
// input does, matching the lifetime of `secret` in the parent component.
72+
const [resolved, setResolved] = useState<{
73+
wif: string;
74+
state: WifPreviewState;
75+
} | null>(null);
8576

8677
useEffect(() => {
87-
if (!gateOk || cached) {
88-
return;
89-
}
78+
if (!gateOk) return;
9079
let cancelled = false;
9180
const timer = window.setTimeout(async () => {
9281
if (cancelled) return;
93-
const mod = await loadLoginModule();
94-
if (cancelled) return;
95-
let next: WifPreviewState;
82+
let next: WifPreviewState = IDLE;
83+
let mod: LoginModule | null = null;
9684
try {
85+
mod = await loadLoginModule();
86+
if (cancelled) return;
9787
const result = await mod.resolveIdentityFromWif(sdk!, trimmed);
9888
let dpns: string | null = null;
9989
try {
@@ -107,9 +97,13 @@ export function useWifPreview(
10797
dpnsName: dpns,
10898
};
10999
} catch (err) {
100+
// mod is null only if loadLoginModule itself rejected (chunk fetch
101+
// failure / offline). Treat that as an idle outcome — same silent
102+
// policy we apply to UnknownIdentity and network blips.
110103
if (
111-
err instanceof mod.WrongKeyPurposeError ||
112-
err instanceof mod.KeyDisabledError
104+
mod &&
105+
(err instanceof mod.WrongKeyPurposeError ||
106+
err instanceof mod.KeyDisabledError)
113107
) {
114108
// We have an identity ID — resolve its DPNS name so the warning
115109
// can show the user-friendly handle instead of a truncated ID.
@@ -134,29 +128,23 @@ export function useWifPreview(
134128
dpnsName: dpns,
135129
};
136130
}
137-
} else if (err instanceof mod.UnknownIdentityError) {
138-
next = IDLE;
139131
} else {
132+
// UnknownIdentityError, network failure, import failure, or any
133+
// other unexpected error — stay silent until the user submits.
140134
next = IDLE;
141135
}
142136
}
143137
if (cancelled) return;
144-
// Only cache stable outcomes — silent "idle" results from transient
145-
// network errors should be retryable on the next keystroke.
146-
if (next.status !== "idle") {
147-
previewCache.set(trimmed, next);
148-
}
149138
setResolved({ wif: trimmed, state: next });
150139
}, DEBOUNCE_MS);
151140

152141
return () => {
153142
cancelled = true;
154143
window.clearTimeout(timer);
155144
};
156-
}, [sdk, trimmed, gateOk, cached]);
145+
}, [sdk, trimmed, gateOk]);
157146

158147
if (!gateOk) return IDLE;
159-
if (cached) return cached;
160148
if (resolved && resolved.wif === trimmed) return resolved.state;
161149
return CHECKING;
162150
}

example-apps/dashnote/src/session/SessionContext.tsx

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,11 +213,19 @@ export function SessionProvider({ children }: { children: ReactNode }) {
213213
} catch (err) {
214214
const message = errorMessage(err);
215215
setError(message);
216-
// Restore prior session state. If a remembered identity exists,
217-
// prefer landing in browsing mode so a failed key-swap still shows
218-
// the remembered identity panel instead of the logged-out form.
216+
// Restore prior session state. If the user was already authenticated
217+
// (e.g. they opened Settings and pasted a bad secret), keep them
218+
// logged in — a typo shouldn't demote an active session. Otherwise,
219+
// prefer landing in browsing mode if a remembered identity exists,
220+
// so a failed key-swap still shows the remembered identity panel
221+
// instead of the logged-out form.
219222
const remembered = loadRememberedIdentity();
220-
if (remembered) {
223+
if (priorStatus === "authenticated" && priorKeyManager) {
224+
setKeyManager(priorKeyManager);
225+
setIdentityId(priorIdentityId);
226+
setDpnsName(priorDpnsName);
227+
setStatus(priorStatus);
228+
} else if (remembered) {
221229
setKeyManager(null);
222230
setIdentityId(remembered.id);
223231
setDpnsName(remembered.name ?? null);

example-apps/dashnote/test/SessionContext.test.tsx

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ describe("SessionProvider", () => {
596596
);
597597
});
598598

599-
it("failed switch-identity from authenticated falls back to browsing the remembered identity", async () => {
599+
it("failed switch-identity from authenticated keeps the active session", async () => {
600600
// First, authenticate so a real keyManager + remembered identity exist.
601601
mockKeyManagerCreate.mockResolvedValueOnce({
602602
identityId: "logged-in-id",
@@ -606,20 +606,22 @@ describe("SessionProvider", () => {
606606
await ref.current.login("test mnemonic", { rememberMe: true });
607607
});
608608
expect(ref.current.status).toBe("authenticated");
609-
expect(ref.current.keyManager).not.toBeNull();
609+
const priorKeyManager = ref.current.keyManager;
610+
expect(priorKeyManager).not.toBeNull();
610611

611612
// Now attempt to switch with a wrong-purpose WIF. The active session
612-
// should drop, but a remembered identity exists, so we land in browsing.
613+
// must NOT drop — typing a bad secret while signed in should never log
614+
// the user out, even if a remembered identity is present in storage.
613615
mockLoginWithPrivateKey.mockRejectedValueOnce(
614616
new Error("Found identity Y, but this is a transfer key."),
615617
);
616618
await act(async () => {
617619
await ref.current.login("cVHcfvcWNc7DvqaPCwM6Z3").catch(() => undefined);
618620
});
619621

620-
expect(ref.current.status).toBe("browsing");
622+
expect(ref.current.status).toBe("authenticated");
621623
expect(ref.current.identityId).toBe("logged-in-id");
622-
expect(ref.current.keyManager).toBeNull();
624+
expect(ref.current.keyManager).toBe(priorKeyManager);
623625
expect(ref.current.error).toMatch(/transfer key/);
624626
expect(mockToastError).toHaveBeenCalledWith(
625627
expect.stringContaining("transfer key"),

example-apps/dashnote/test/useWifPreview.test.tsx

Lines changed: 9 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ vi.mock("../src/dash/resolveDpnsName", () => ({
5858
}));
5959

6060
import {
61-
_resetWifPreviewCacheForTests,
6261
useWifPreview,
6362
type WifPreviewState,
6463
} from "../src/hooks/useWifPreview";
@@ -94,7 +93,6 @@ beforeEach(() => {
9493
vi.useFakeTimers();
9594
mockResolveIdentityFromWif.mockReset();
9695
mockResolveDpnsName.mockReset();
97-
_resetWifPreviewCacheForTests();
9896
});
9997

10098
afterEach(() => {
@@ -263,62 +261,31 @@ describe("useWifPreview", () => {
263261
expect(states.at(-1)?.status).toBe("resolved");
264262
});
265263

266-
it("caches stable outcomes — re-rendering the same WIF skips the network", async () => {
264+
it("does not retain results across remount — secret state is component-local", async () => {
265+
// The hook intentionally does not cache resolved outcomes across mounts;
266+
// every fresh mount that re-passes the WIF performs the network call
267+
// again. This bounds raw-WIF retention to the form's lifetime.
267268
mockResolveIdentityFromWif.mockResolvedValue({
268-
identityId: "id-cache",
269+
identityId: "id-fresh",
269270
identity: {},
270271
matched: { id: 1, purpose: 0, securityLevel: 2 },
271272
identityKey: {},
272273
});
273274
mockResolveDpnsName.mockResolvedValue(null);
274275

275-
const states: WifPreviewState[] = [];
276276
const { unmount } = render(
277-
<Harness secret={VALID_WIF} onState={(s) => states.push(s)} />,
277+
<Harness secret={VALID_WIF} onState={() => {}} />,
278278
);
279279
await flushDebounce();
280280
expect(mockResolveIdentityFromWif).toHaveBeenCalledTimes(1);
281281
unmount();
282282

283-
// Second mount with the same WIF must not re-query — the cache returns
284-
// the prior outcome synchronously.
285-
const states2: WifPreviewState[] = [];
286-
render(<Harness secret={VALID_WIF} onState={(s) => states2.push(s)} />);
287-
expect(mockResolveIdentityFromWif).toHaveBeenCalledTimes(1);
288-
expect(states2.at(-1)?.status).toBe("resolved");
289-
});
290-
291-
it("does NOT cache idle outcomes — silent failures retry on next render", async () => {
292-
mockResolveIdentityFromWif.mockRejectedValueOnce(
293-
new UnknownIdentityError(),
294-
);
295-
296-
const states: WifPreviewState[] = [];
297-
const { unmount } = render(
298-
<Harness secret={VALID_WIF} onState={(s) => states.push(s)} />,
299-
);
300-
await flushDebounce();
301-
expect(states.at(-1)?.status).toBe("idle");
302-
unmount();
303-
304-
// Now the WIF resolves successfully — the previous idle result must not
305-
// have been cached, so this run should hit the network again.
306-
mockResolveIdentityFromWif.mockResolvedValueOnce({
307-
identityId: "id-retry",
308-
identity: {},
309-
matched: { id: 1, purpose: 0, securityLevel: 2 },
310-
identityKey: {},
311-
});
312-
mockResolveDpnsName.mockResolvedValue(null);
313-
314-
const states2: WifPreviewState[] = [];
315-
render(<Harness secret={VALID_WIF} onState={(s) => states2.push(s)} />);
283+
render(<Harness secret={VALID_WIF} onState={() => {}} />);
316284
await flushDebounce();
317285
expect(mockResolveIdentityFromWif).toHaveBeenCalledTimes(2);
318-
expect(states2.at(-1)?.status).toBe("resolved");
319286
});
320287

321-
it("trims whitespace before gating + caching (paste with surrounding spaces)", async () => {
288+
it("trims whitespace before gating; passes the trimmed WIF to the resolver", async () => {
322289
mockResolveIdentityFromWif.mockResolvedValue({
323290
identityId: "id-trim",
324291
identity: {},
@@ -328,18 +295,13 @@ describe("useWifPreview", () => {
328295
mockResolveDpnsName.mockResolvedValue(null);
329296

330297
const states: WifPreviewState[] = [];
331-
const { unmount } = render(
298+
render(
332299
<Harness secret={` ${VALID_WIF}\n`} onState={(s) => states.push(s)} />,
333300
);
334301
await flushDebounce();
335302
expect(mockResolveIdentityFromWif).toHaveBeenCalledTimes(1);
336303
// Resolver was called with the trimmed WIF, not the padded input.
337304
expect(mockResolveIdentityFromWif).toHaveBeenCalledWith(sdk, VALID_WIF);
338-
unmount();
339-
340-
// Re-rendering with the bare WIF must hit the cache (same key after trim).
341-
render(<Harness secret={VALID_WIF} onState={(s) => states.push(s)} />);
342-
expect(mockResolveIdentityFromWif).toHaveBeenCalledTimes(1);
343305
expect(states.at(-1)?.status).toBe("resolved");
344306
});
345307

0 commit comments

Comments
 (0)