feat(wallet): expire idle sessions and warn before disconnecting - #1214
Open
bilhokista wants to merge 3 commits into
Open
feat(wallet): expire idle sessions and warn before disconnecting#1214bilhokista wants to merge 3 commits into
bilhokista wants to merge 3 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1084.
The hole that matters most is not the timer
A 30-minute in-memory timer does nothing once the tab is closed.
WalletProviderrestored the address fromlocalStorageunconditionally:So on a shared machine, closing the browser and reopening the page tomorrow silently reconnects the previous person's wallet. That is the public-terminal risk the issue describes, and a timer alone would not have touched it.
The restore path now checks a persisted
inheritx_wallet_last_activitytimestamp and clears all three keys if the session is stale.A missing or unparseable timestamp counts as expired. That is deliberate: a session written before this change has no recorded activity, and treating "unknown" as "still valid" would leave in place exactly the sessions this is meant to clear. The cost is that everyone is asked to reconnect once after deploy, which seems like the right trade for a security fix.
The one exception is a timestamp in the future — a clock change or a tampered value — which restarts the window rather than locking the user out.
The timer
sessionPhasesplits idle time intoactive/warning/expired, with the warning covering the final 60 seconds. A 1-second interval reads it and either exposessessionSecondsLeftfor a countdown modal or disconnects.Two implementation details worth review:
disconnectis reached through a ref. Listing it as an effect dependency would tear down and rebuild the interval every timekitchanges, resetting the countdown.secondsUntilExpiryrounds up, so the countdown never displays "0 seconds" while time remains.What this PR does not include
Requirement 2 asks for a modal. I exposed
sessionSecondsLeftandextendSessionon the context but did not add the modal component, because the app has no shared modal primitive and dropping a new one intoWalletProviderwould put UI inside a context provider that currently renders none. A consumer can render it in three lines from the values now available.Say the word and I will add the modal here — I would just rather you choose where it lives than have me pick for you.
Tests
frontend/tests/lib/walletSession.test.ts— 18 cases: the phase boundaries asserted to the millisecond (including that expiry happens at the timeout, and that one millisecond earlier is stillwarning), the countdown never going negative or showing 0 while time remains, and every rejection path for a stored timestamp — missing, empty, unparseable, zero, negative, stale — plus the future-timestamp tolerance.Honest note on verification: I could not run Vitest locally (no full workspace install), but this frontend has Vitest configured so the file runs in CI normally. I transpiled
walletSession.tsand executed it standalone against 25 assertions covering the same ground — all pass.The
WalletContextwiring — the listeners, the interval, the ref indirection — has no test here and needs CI or a manual check. The behaviour I would most like verified is that the interval is genuinely torn down on unmount and on disconnect, since a leaked 1-second timer would be an unpleasant thing to ship.🤖 Generated with Claude Code
https://claude.ai/code/session_01CrfEY1tvXrbeMDAUzxfuk7