feat(contrib): data retention guidance and enforcement for cached session state - #369
Merged
davedumto merged 1 commit intoAug 31, 2026
Conversation
…sion state Cached session state persisted through a SessionStorageAdapter had no documented bound on how long it may live in consumer local storage, and no mechanism to enforce one. Add guidance plus a reference implementation, both inside contrib/: - contrib/session-retention.md documents the recommended window and the reasoning behind it. Cached session state is not a credential (no key material; every signature still needs a live WebAuthn ceremony), but it is a durable link between a browser profile and an on-chain account, so the risk is privacy and lingering account linkage rather than key compromise. That is why the recommendation is a bounded window rather than an aggressive minutes-long expiry. Includes a per-deployment table for choosing a stricter value, the enforcement semantics, an edge-case table, a step-by-step recipe for landing it in src/, and the proposed root README section. - contrib/session-retention.ts implements the window. 30 days of inactivity is the documented default (DEFAULT_SESSION_MAX_AGE_MS). withSessionRetention() wraps any SessionStorageAdapter and enforces the max age on read: state past the window yields null and is cleared from the underlying storage, so it is evicted rather than merely ignored. Wrapping the adapter instead of the store applies the window to every read path and composes with any adapter without the core store needing to know retention exists. Age is measured from lastActiveAt, which touch() refreshes, so the window is an idle timeout rather than a hard cap on session lifetime. isSessionExpired() exposes the same rule as a pure helper. Unparseable lastActiveAt counts as expired (an age that cannot be bounded is what the window exists to prevent); a future lastActiveAt (clock skew) is clamped at zero; a failed eviction still reports expiry so a read-only storage backend cannot resurrect a session; a non-positive or NaN maxAgeMs throws a RangeError at wiring time. - contrib/session-retention.test.ts covers all of the above, including expiry past the max age, eviction from the underlying storage, the exact boundary, the 30-day default, and end-to-end restore() behavior through both the memory and web storage adapters.
|
@Adeolu01 is attempting to deploy a commit to the david's projects Team on Vercel. A member of the Team first needs to authorize it. |
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 #292
closes #297
closes #287
closes #299
Summary
Documents how long cached session state should persist in consumer local storage, and enforces that window on read.
This PR is scoped entirely to
contrib/per CONTRIBUTING.md rule 3, so it is not caught by the contrib-only bot. Four files, all undercontrib/:contrib/session-retention.mdcontrib/session-retention.tscontrib/session-retention.test.tscontrib/README.mdRequirements
Document a recommended retention window. 30 days of inactivity, as
DEFAULT_SESSION_MAX_AGE_MS.The reasoning matters more than the number, so the guidance leads with what this data actually is. Cached session state holds no key material and cannot authorize anything on its own — every signature still requires a live WebAuthn ceremony against the passkey. It is the public smart-account address, the public passkey credential id, and two timestamps. What it does carry is a durable link between a browser profile and an on-chain account.
So the risk is privacy and lingering account linkage, not key compromise. That is precisely why the recommendation is a bounded window rather than an aggressive minutes-long expiry: a shorter default would push users through avoidable passkey prompts and train them to click through auth ceremonies, which is itself a security cost. 30 days keeps "open the app, still signed in" true for ordinary use while bounding how long an abandoned profile keeps pointing at an account. The doc includes a table of suggested values per deployment (personal device, shared device, kiosk, custodial dashboard, ephemeral storage).
A configurable max age enforced by the SDK on read.
withSessionRetention(adapter, { maxAgeMs })wraps anySessionStorageAdapter. Onload()it measures the entry's age fromlastActiveAtand, past the window, returnsnulland clears the entry from the underlying storage.A test verifying expired cached state past the max age is discarded. Plus eviction, boundary, default, and end-to-end
restore()cases — see below.Document the retention behavior in the README.
contrib/README.mdgets the summary section. Since the rootREADME.mdis outside contributor scope, the guidance doc carries the exact proposed root-README section, ready to paste, along with theHelperstable row — so a maintainer can land it verbatim without rewriting anything.Design notes
Why wrap the adapter rather than the store. The window then applies to every read path —
restore()today, plus anything that later loads through the same adapter — and composes with any adapter (web storage, extension storage, custom) without the core store needing to know retention exists. It is also what makes a faithful implementation possible fromcontrib/without touchingsrc/.Expired state is evicted, not merely ignored. Returning
nullwhile leaving the record inlocalStoragewould defeat the purpose — the very entry the window exists to bound would sit there forever. The wrapper clears it.The window is an idle timeout, not a hard cap on session lifetime. Age comes from
lastActiveAt, which the store'stouch()already refreshes on user activity, so an actively used session keeps renewing while an untouched one ages out. A hard cap would force re-authentication on active users for no benefit, given the cached data is not a credential.Edge cases resolved deliberately, each with a test:
lastActiveAtlastActiveAt(clock skew)clear()throwsnullload()throwsrestore()already maps this todisconnected; the wrapper must not swallow itmaxAgeMs: InfinitymaxAgeMszero, negative,NaNRangeErrorat wiringnullclear()Verification
Full-suite comparison against
dev, to be explicit that this PR does not regress anything:dev(baseline)The same 18 pre-existing failures, all in
contrib/examples/andcontrib/rpc-server.test.ts, identical before and after and untouched here — plus 22 new passing tests.One thing a maintainer should know
src/session.test.tsdoes not parse ondev. Thedispose() clears the timer so the store is garbage-collectablecase is missing its closing braces and a newdescribe(opens immediately after it:tscreportssession.test.ts(428,1): error TS1005: '}' expected.and vitest fails the file withTransform failed. That is why this PR's tests live in their owncontrib/file rather than extending the existing suite, and it will need fixing before these tests can be ported intosrc/. Flagged in the integration recipe.Related: that file's shared
sessionfixture pinslastActiveAtto a hardcoded2026-07-16. Once any retention window exists, a fixed past timestamp silently ages out as the calendar moves and breaks therestore()tests — worth making relative to now during integration. Also noted in the recipe.