fix(stores): preserve exemption expiresAt across persistence - #59
Open
goeric wants to merge 1 commit into
Open
Conversation
parseExemptionRecord dropped the optional expiresAt field, so a session self-exemption reloaded from a persisting store had no expiry. pruneExpiredState only prunes when expiresAt is set, making the exemption permanent and restoring the unbounded behaviour that sessionExemptionTtlMs was added in 0.3.3 to bound. Parse it back the same way parseSessionRecord already does. Read-side only: cloneState already preserves the field, which is why MemoryStateStore-based tests never caught 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.
What
parseExemptionRecorddoes not readexpiresAtback off persisted state, so a session self-exemption loses its expiry whenever state round-trips through a store that serialises.Why
0.3.3 added
sessionExemptionTtlMsto bound how long a self-exemption lives — "Session exemptions were never pruned before, so a single self-click grant lasted the whole session; this bounds that window."That bound is lost on reload:
session.ts:486setsrecord.expiresAt = grantedAt + ttlMson grant.stores.ts:382parseExemptionRecorddestructuresadvertiserHost,policyIds,networkIds,grantedAt—expiresAtis dropped.parseSessionRecord, directly below, handles the same optional field correctly.session.ts:684pruneExpiredStateprunes an exemption only whenexpiresAt !== undefined, so the field-less record is never pruned.The result is a permanent self-exemption for that host — the pre-0.3.3 behaviour, reintroduced by persistence. It affects
SessionStorageStateStoreand everything behind the persisted-envelope parser.MemoryStateStoreis unaffected because it round-trips throughcloneState, which preserves the field via spread — which is why the existing suite doesn't catch it.Fix
Parse
expiresAtback the same wayparseSessionRecordalready does: same optional-numeric validation clause, same conditional assignment. Read-side only —cloneStatealready preserved it.Tests
Both fail on
mainand pass with the fix:tests/stores.test.ts—expiresAtsurvives aChromeLocalStateStoresave/load.tests/session.test.ts— the persistence-backed twin of the existingbounds a session self-exemption by the default TTLtest. Grant, reload through a real store, advance past the TTL, and the lingering cookie stands down again. Onmainit returnsself-exempted-sessioninstead.169 tests pass (167 + 2). The
auditharness passes 30/30.Checklist
npm run typecheck,npm run lint,npm test, andnpm run buildpasspolicies-cite-checkis untouchedSignals(I2)Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.