harden secure storage against stale mirrors and post-clear writes

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Samuel Newman
2026-08-15 20:04:12 +03:00
parent 30607c26a4
commit af92c6de21
7 changed files with 431 additions and 18 deletions
+1
View File
@@ -64,6 +64,7 @@ export type Events = {
divergence: 'none' | 'legacy-lost' | 'secure-behind'
secureAccountsWithCreds: number
legacyAccountsWithCreds: number
adoptTokensFromLegacy: number
}
'notifications:openApp': {
reason: NotificationReason
@@ -76,6 +76,28 @@ const expiredAlice: SessionAccount = {
refreshJwt: undefined,
accessJwt: undefined,
}
const bob: SessionAccount = {
service: 'https://bsky.social',
did: 'did:plc:bob',
handle: 'bob.test',
refreshJwt: 'bob-refresh',
accessJwt: 'bob-access',
}
/**
* A minimal unsigned JWT carrying an `iat`. `jwt-decode` reads the payload
* without verifying anything, so the signature can be junk. `jti` is there to
* make two tokens of the same age differ as strings.
*/
function jwtWithIat(iat: number, jti = 'a'): string {
const encode = (value: object) =>
Buffer.from(JSON.stringify(value)).toString('base64url')
return [
encode({alg: 'HS256', typ: 'JWT'}),
encode({iat, jti}),
'not-a-signature',
].join('.')
}
/** An account with usable credentials. */
const withCreds: SessionSnapshot = {accounts: [alice], currentDid: alice.did}
@@ -88,6 +110,12 @@ const withoutCreds: SessionSnapshot = {
type ExpectedDecision = Omit<BootDecision, 'adopt' | 'report'> & {
adopts: boolean
divergence: SessionStorageBootReport['divergence']
/**
* The adopted snapshot, when the per-account merge makes it something other
* than the secure snapshot as read.
*/
adopt?: SessionSnapshot
adoptTokensFromLegacy?: number
}
const rows: {
@@ -248,6 +276,8 @@ const rows: {
expected: {
source: 'secure',
adopts: true,
// The merge keeps the credential-less account the blob still lists.
adopt: withoutCreds,
backfill: 'none',
forceIndex: false,
divergence: 'none',
@@ -296,6 +326,11 @@ const rows: {
describe('decideBootSource', () => {
it.each(rows)('$name', ({gateEnabled, secure: read, legacy, expected}) => {
const {
adopt: expectedAdopt,
adoptTokensFromLegacy = 0,
...expectedCore
} = expected
const decision = decideBootSource({gateEnabled, secure: read, legacy})
expect({
@@ -304,10 +339,13 @@ describe('decideBootSource', () => {
backfill: decision.backfill,
forceIndex: decision.forceIndex,
divergence: decision.report.divergence,
}).toEqual(expected)
}).toEqual(expectedCore)
expect(decision.adopt).toEqual(
expected.adopts && read.status === 'ok' ? read.snapshot : undefined,
expected.adopts
? (expectedAdopt ?? (read.status === 'ok' ? read.snapshot : undefined))
: undefined,
)
expect(decision.report.adoptTokensFromLegacy).toBe(adoptTokensFromLegacy)
expect(decision.report.gateEnabled).toBe(gateEnabled)
expect(decision.report.secureStatus).toBe(read.status)
expect(decision.report.source).toBe(expected.source)
@@ -331,6 +369,80 @@ describe('decideBootSource', () => {
})
})
/**
* Adopting the secure store wholesale would lose whatever only the blob holds,
* which is what a single failed mirror write looks like: the store as a whole
* still has credentials, so the whole-store guard sees nothing wrong.
*/
describe('decideBootSource credential merge', () => {
function adoptedBy(secureSnapshot: SessionSnapshot, legacy: SessionSnapshot) {
return decideBootSource({
gateEnabled: true,
secure: {status: 'ok', snapshot: secureSnapshot},
legacy,
})
}
it('keeps an account the secure store never received', () => {
const legacy: SessionSnapshot = {
accounts: [alice, bob],
currentDid: alice.did,
}
const decision = adoptedBy(withCreds, legacy)
expect(decision.source).toBe('secure')
expect(decision.adopt).toEqual(legacy)
expect(decision.report.adoptTokensFromLegacy).toBe(1)
})
it('keeps an account only the secure store still holds', () => {
const secureSnapshot: SessionSnapshot = {
accounts: [alice, bob],
currentDid: bob.did,
}
const decision = adoptedBy(secureSnapshot, withCreds)
expect(decision.adopt).toEqual(secureSnapshot)
expect(decision.report.adoptTokensFromLegacy).toBe(0)
})
it('takes whichever refresh token was issued last', () => {
const older = {...alice, refreshJwt: jwtWithIat(1_000), accessJwt: 'older'}
const newer = {...alice, refreshJwt: jwtWithIat(2_000), accessJwt: 'newer'}
const snapshotOf = (account: SessionAccount): SessionSnapshot => ({
accounts: [account],
currentDid: alice.did,
})
const legacyIsNewer = adoptedBy(snapshotOf(older), snapshotOf(newer))
expect(legacyIsNewer.adopt).toEqual(snapshotOf(newer))
expect(legacyIsNewer.report.adoptTokensFromLegacy).toBe(1)
const secureIsNewer = adoptedBy(snapshotOf(newer), snapshotOf(older))
expect(secureIsNewer.adopt).toEqual(snapshotOf(newer))
expect(secureIsNewer.report.adoptTokensFromLegacy).toBe(0)
})
it('falls back to the blob when the comparison cannot be made', () => {
const snapshotOf = (account: SessionAccount): SessionSnapshot => ({
accounts: [account],
currentDid: alice.did,
})
const undecodable = {...alice, refreshJwt: 'not-a-jwt'}
const decodable = {...alice, refreshJwt: jwtWithIat(1_000)}
const sameAge = {...alice, refreshJwt: jwtWithIat(1_000, 'b')}
const unreadable = adoptedBy(snapshotOf(undecodable), snapshotOf(decodable))
expect(unreadable.adopt).toEqual(snapshotOf(decodable))
expect(unreadable.report.adoptTokensFromLegacy).toBe(1)
const tied = adoptedBy(snapshotOf(decodable), snapshotOf(sameAge))
expect(tied.adopt).toEqual(snapshotOf(sameAge))
})
})
describe('initSessionStorage', () => {
beforeEach(() => {
secure.values.clear()
@@ -361,6 +473,47 @@ describe('initSessionStorage', () => {
divergence: 'legacy-lost',
secureAccountsWithCreds: 1,
legacyAccountsWithCreds: 0,
adoptTokensFromLegacy: 0,
})
})
it('adopts the newer blob credential and converges the store on it', () => {
device.set(['sessionSecureStorageReadEnabled'], true)
const stored = {
...alice,
refreshJwt: jwtWithIat(1_000),
accessJwt: 'stored-access',
}
const refreshed = {
...alice,
refreshJwt: jwtWithIat(2_000),
accessJwt: 'refreshed-access',
}
writeSessions(
EMPTY_SNAPSHOT,
{accounts: [stored], currentDid: alice.did},
{forceIndex: true},
)
persistedState.session = {
accounts: [refreshed],
currentAccount: refreshed,
}
initSessionStorage()
expect(persistedState.session).toEqual({
accounts: [refreshed],
currentAccount: refreshed,
})
// The mirror write lands the adopted credential back in the keychain.
expect(readSessions()).toEqual({
accounts: [refreshed],
currentDid: alice.did,
})
expect(consumeSessionStorageBootReport()).toMatchObject({
source: 'secure',
divergence: 'none',
adoptTokensFromLegacy: 1,
})
})
@@ -3,12 +3,23 @@ import {afterEach, beforeEach, describe, expect, it, jest} from '@jest/globals'
jest.mock('expo-secure-store', () => {
const values = new Map<string, string>()
const writes: [key: string, value: string][] = []
const control: {failKey?: string} = {}
/*
* `failAfter` lets a key succeed that many times before it starts failing,
* which is how a write is made to fail on its second index write - the one
* that cleans up a journal it already published.
*/
const control: {failKey?: string; failAfter?: number} = {}
return {
AFTER_FIRST_UNLOCK: 0,
getItem: (key: string) => values.get(key) ?? null,
setItem: (key: string, value: string) => {
if (key === control.failKey) throw new Error('disk full')
if (key === control.failKey) {
if (control.failAfter) {
control.failAfter -= 1
} else {
throw new Error('disk full')
}
}
writes.push([key, value])
values.set(key, value)
},
@@ -49,13 +60,14 @@ jest.mock('#/logger', () => {
import {type SessionAccount} from '#/state/session/types'
import {accountKeys, SESSION_INDEX_KEY} from '../keys'
import {EMPTY_SNAPSHOT, type SessionSnapshot} from '../schema'
import {readSessions} from '../secureStore'
import {createSessionStorageStore} from '../store'
const {__mock: secure} = jest.requireMock('expo-secure-store') as {
__mock: {
values: Map<string, string>
writes: [key: string, value: string][]
control: {failKey?: string}
control: {failKey?: string; failAfter?: number}
}
}
const {__listeners: appStateListeners} = jest.requireMock('#/lib/appState') as {
@@ -82,6 +94,7 @@ beforeEach(() => {
secure.values.clear()
secure.writes.length = 0
secure.control.failKey = undefined
secure.control.failAfter = undefined
appStateListeners.length = 0
loggedErrors.length = 0
})
@@ -192,6 +205,101 @@ describe('createSessionStorageStore', () => {
expect(storedIndex()).toEqual({version: 1, dids: []})
})
it('drops writes for the rest of the process once a clear succeeds', () => {
const store = createSessionStorageStore()
store.write(active)
store.clear()
const writesAfterClear = secure.writes.length
/*
* Clearing storage does not end the session, so a token refresh already in
* flight will still ask for a mirror write.
*/
store.write(active)
expect(secure.writes.length).toBe(writesAfterClear)
expect(secure.values.get(aliceKeys.refresh)).toBe('')
expect(secure.values.get(aliceKeys.access)).toBe('')
expect(readSessions()).toEqual(EMPTY_SNAPSHOT)
})
it('force-publishes a clean index over a journal left by a failed write', () => {
const store = createSessionStorageStore()
store.write(active)
/*
* Removing alice fails on the final index write: the journaled index that
* names her as retired is already published and her keys are tombstoned.
*/
secure.control.failKey = SESSION_INDEX_KEY
secure.control.failAfter = 1
store.write(EMPTY_SNAPSHOT)
expect(storedIndex()).toMatchObject({dids: [], retiredDids: [alice.did]})
expect(secure.values.get(aliceKeys.refresh)).toBe('')
// alice is back, byte-identical to the snapshot still believed durable.
secure.control.failKey = undefined
secure.control.failAfter = undefined
store.write(active)
expect(secure.values.get(aliceKeys.refresh)).toBe(alice.refreshJwt)
expect(secure.values.get(aliceKeys.access)).toBe(alice.accessJwt)
expect(storedIndex()).toEqual({
version: 1,
currentDid: alice.did,
dids: [alice.did],
})
expect(readSessions()).toEqual(active)
expect(store.getDurable()).toEqual(active)
})
it('rewrites credentials after a failure even when the snapshot is unchanged', () => {
const store = createSessionStorageStore()
store.write(active)
secure.control.failKey = SESSION_INDEX_KEY
store.write({
accounts: [{...alice, refreshJwt: 'rotated-refresh'}],
currentDid: alice.did,
})
secure.control.failKey = undefined
/*
* A throw can land anywhere in the write ordering, so the keychain may hold
* something the baseline does not describe.
*/
secure.values.set(aliceKeys.refresh, 'partially-applied')
store.write(active)
expect(secure.values.get(aliceKeys.refresh)).toBe(alice.refreshJwt)
expect(secure.values.get(aliceKeys.access)).toBe(alice.accessJwt)
expect(readSessions()).toEqual(active)
})
it('tombstones absent credentials when rewriting from an unknown baseline', () => {
const store = createSessionStorageStore()
store.write(active)
secure.control.failKey = SESSION_INDEX_KEY
store.write({
accounts: [{...alice, accessJwt: 'rotated-access'}],
currentDid: alice.did,
})
secure.control.failKey = undefined
// The user logs out: the account is kept, its credentials are not.
const loggedOut: SessionSnapshot = {
accounts: [{...alice, refreshJwt: undefined, accessJwt: undefined}],
currentDid: undefined,
}
store.write(loggedOut)
expect(secure.values.get(aliceKeys.refresh)).toBe('')
expect(secure.values.get(aliceKeys.access)).toBe('')
expect(readSessions()).toEqual(loggedOut)
})
it('retries when the app returns to the foreground', () => {
const store = createSessionStorageStore()
secure.control.failKey = aliceKeys.access
+112 -2
View File
@@ -1,3 +1,6 @@
import {jwtDecode} from 'jwt-decode'
import {type SessionAccount} from '#/state/session/types'
import {type SessionSnapshot} from './schema'
/** Which store the app boots its session state from. */
@@ -22,11 +25,22 @@ export type SessionStorageBootReport = {
divergence: 'none' | 'legacy-lost' | 'secure-behind'
secureAccountsWithCreds: number
legacyAccountsWithCreds: number
/**
* How many accounts in the adopted snapshot took their credentials from the
* legacy blob, i.e. how often the secure store held a stale or missing token
* that would have been adopted as-is. Always 0 when not booting from the
* secure store.
*/
adoptTokensFromLegacy: number
}
export type BootDecision = {
source: BootSource
/** The snapshot to adopt, set only when booting from the secure store. */
/**
* The snapshot to adopt, set only when booting from the secure store. It is
* the secure snapshot merged with the legacy blob, not the secure snapshot
* itself - see {@link mergeFreshestCredentials}.
*/
adopt: SessionSnapshot | undefined
backfill: 'none' | 'secure-from-legacy'
/** Publish an index even if the backfill changes nothing. */
@@ -44,6 +58,11 @@ export type BootDecision = {
* mirror writes look like - booting from the secure store there would log the
* user out. A real logout clears the credentials in both stores, so it does
* not trip the guard.
*
* That guard is whole-store, so winning the boot does not mean the secure
* snapshot is adopted verbatim: it is merged per account against the blob by
* {@link mergeFreshestCredentials}, which is what keeps a single failed mirror
* write from dropping an account or reinstating a spent refresh token.
*/
export function decideBootSource({
gateEnabled,
@@ -62,6 +81,10 @@ export function decideBootSource({
gateEnabled &&
secureSnapshot !== undefined &&
(secureHasCreds || !legacyHasCreds)
const merged =
bootSecure && secureSnapshot
? mergeFreshestCredentials(secureSnapshot, legacy)
: undefined
let backfill: BootDecision['backfill'] = 'none'
let forceIndex = false
@@ -90,7 +113,7 @@ export function decideBootSource({
return {
source: bootSecure ? 'secure' : 'legacy',
adopt: bootSecure ? secureSnapshot : undefined,
adopt: merged?.snapshot,
backfill,
forceIndex,
report: {
@@ -107,10 +130,97 @@ export function decideBootSource({
: 'none',
secureAccountsWithCreds: secureSnapshot ? countCreds(secureSnapshot) : 0,
legacyAccountsWithCreds: countCreds(legacy),
adoptTokensFromLegacy: merged?.tokensFromLegacy ?? 0,
},
}
}
/**
* Merge the two stores account by account, taking the freshest credentials for
* each did, so that adopting the secure store cannot lose what only the blob
* holds. A mirror write can fail for one account - a login whose write failed
* and whose in-memory retry died with the process, or a token rotation that
* never landed - while the rest of the store looks perfectly healthy, which is
* exactly what the whole-store guard cannot see.
*
* Ties and unprovable comparisons go to the blob: it is the historical write of
* record, and the secure store is the mirror on trial.
*
* Deliberate asymmetry: this can resurrect a credential the other store
* recorded as revoked, e.g. a logout only one of the two writes captured. That
* is accepted. A token the server has already revoked self-corrects on first
* use with a re-login prompt, while a valid token dropped here logs the user
* out of an account with no way back - the failure this module exists to
* prevent.
*/
function mergeFreshestCredentials(
secure: SessionSnapshot,
legacy: SessionSnapshot,
): {snapshot: SessionSnapshot; tokensFromLegacy: number} {
const legacyByDid = new Map(legacy.accounts.map(a => [a.did, a]))
const secureDids = new Set(secure.accounts.map(a => a.did))
let tokensFromLegacy = 0
const accounts = [
...secure.accounts.map(account => {
const other = legacyByDid.get(account.did)
if (!other) return account
if (freshest(account, other) === 'secure') return account
if (other.refreshJwt) tokensFromLegacy += 1
return other
}),
/*
* Accounts the secure store never received. The blob is the only copy, so
* dropping them here is the multi-account logout this merge exists to stop.
*/
...legacy.accounts.filter(account => {
if (secureDids.has(account.did)) return false
if (account.refreshJwt) tokensFromLegacy += 1
return true
}),
]
const dids = new Set<string>(accounts.map(account => account.did))
const currentDid = secure.currentDid ?? legacy.currentDid
return {
snapshot: {
accounts,
currentDid: currentDid && dids.has(currentDid) ? currentDid : undefined,
},
tokensFromLegacy,
}
}
/**
* Which store holds the newer refresh token for one account. The caller takes
* the winner's account object whole, so its tokens and descriptor fields stay
* coherent with each other.
*/
function freshest(
secure: SessionAccount,
legacy: SessionAccount,
): 'secure' | 'legacy' {
if (secure.refreshJwt === legacy.refreshJwt) return 'secure'
if (!legacy.refreshJwt) return 'secure'
if (!secure.refreshJwt) return 'legacy'
const secureIssuedAt = issuedAt(secure.refreshJwt)
const legacyIssuedAt = issuedAt(legacy.refreshJwt)
if (secureIssuedAt === undefined || legacyIssuedAt === undefined) {
return 'legacy'
}
return secureIssuedAt > legacyIssuedAt ? 'secure' : 'legacy'
}
/** A token's `iat` claim, or undefined if it cannot be read. */
function issuedAt(token: string): number | undefined {
try {
const {iat} = jwtDecode(token)
return typeof iat === 'number' ? iat : undefined
} catch {
return undefined
}
}
function hasCreds(snapshot: SessionSnapshot): boolean {
return snapshot.accounts.some(account => Boolean(account.refreshJwt))
}
+11 -3
View File
@@ -46,8 +46,9 @@ let bootReport: SessionStorageBootReport | undefined
* Call once during bootstrap, after `persisted.init()` has resolved and before
* anything reads the session.
*
* When the secure store wins, its snapshot is adopted by writing it back into
* the persisted state. `persisted.write` updates its in-memory copy
* When the secure store wins, its snapshot - merged per account with the blob,
* see `decideBootSource` - is adopted by writing it back into the persisted
* state. `persisted.write` updates its in-memory copy
* synchronously before awaiting storage, so every existing reader - the
* session store constructor, the last-active-account lookup, the expiry rescue
* path - sees the adopted data with no seam of its own. It also means the
@@ -90,10 +91,17 @@ export function initSessionStorage(): void {
legacy,
})
store.setDurable(secure.status === 'ok' ? secure.snapshot : EMPTY_SNAPSHOT)
if (decision.adopt) {
adopt(decision.adopt)
/*
* The adopted snapshot is a merge, so it can hold credentials the
* keychain does not. The write diffs against the durable snapshot set
* just above - the secure store as it was read - so it no-ops when the
* merge took nothing from the blob, and retries if it fails.
*/
store.write(decision.adopt)
}
store.setDurable(secure.status === 'ok' ? secure.snapshot : EMPTY_SNAPSHOT)
if (decision.backfill === 'secure-from-legacy') {
backfill(legacy, decision, installId)
}
+9 -2
View File
@@ -207,10 +207,17 @@ export function writeSessions(
for (const account of next.accounts) {
const prior = previousByDid.get(account.did)
const keys = accountKeys(account.did)
if (prior?.refreshJwt !== account.refreshJwt) {
/*
* The absent-prior case must write, not skip. A caller that distrusts its
* baseline passes an empty `previous` to force a full rewrite, and there
* `undefined !== undefined` would skip an account whose token is absent -
* leaving whatever the keychain still holds under that key to be read back
* as a live credential under an index that names the did.
*/
if (!prior || prior.refreshJwt !== account.refreshJwt) {
SecureStore.setItem(keys.refresh, account.refreshJwt ?? '', WRITE_OPTIONS)
}
if (prior?.accessJwt !== account.accessJwt) {
if (!prior || prior.accessJwt !== account.accessJwt) {
SecureStore.setItem(keys.access, account.accessJwt ?? '', WRITE_OPTIONS)
}
const descriptor = canonicalJson(toDescriptor(account))
+32 -6
View File
@@ -10,9 +10,8 @@ import {eraseSessions, writeSessions} from './secureStore'
const RETRY_DELAY = 5_000
/**
* Pending work is a union so the sticky-clear rule reads as one guard: while a
* clear is pending, a write is dropped rather than risk resurrecting a session
* after a requested wipe.
* Pending work is a union: a failed write and a failed clear retry the same
* way, and only the newest of them is ever outstanding.
*/
type PendingWork =
{type: 'write'; snapshot: SessionSnapshot} | {type: 'clear'; dids: string[]}
@@ -45,6 +44,14 @@ export type SessionStorageStore = {
export function createSessionStorageStore(): SessionStorageStore {
let durableSnapshot: SessionSnapshot = EMPTY_SNAPSHOT
let pending: PendingWork | undefined
/**
* Sticky: once a clear is requested, every later write is dropped for the
* rest of the process, including after the erase succeeds. Clearing storage
* does not stop the session, so an in-flight token refresh would otherwise
* mirror the credentials straight back into the keychain. The user is told to
* restart the app, and the next boot lifts the latch via `setDurable`.
*/
let clearRequested = false
let retryTimer: ReturnType<typeof setTimeout> | undefined
let retryTriggersAttached = false
const maybeOrphanedDids = new Set<string>()
@@ -57,15 +64,17 @@ export function createSessionStorageStore(): SessionStorageStore {
durableSnapshot = snapshot
maybeOrphanedDids.clear()
pending = undefined
clearRequested = false
cancelRetry()
}
function write(next: SessionSnapshot) {
if (pending?.type === 'clear') return
if (clearRequested) return
persist(next, 'write')
}
function clear() {
clearRequested = true
const dids = [
...new Set([
...durableSnapshot.accounts.map(account => account.did),
@@ -93,9 +102,26 @@ export function createSessionStorageStore(): SessionStorageStore {
operation: SessionStorageErrorOperation,
) {
trackMaybeOrphaned(next)
/*
* A pending write means the last attempt threw, and a throw can land
* anywhere in the write ordering - including after a journaled index was
* published. The baseline is then a guess, so it is thrown away: diffing
* from empty rewrites every account in `next` in full, retires everything
* else the store might still name, and force-publishes a clean index over
* any journal the failure left behind.
*/
const rebaseline = pending !== undefined
const previous = rebaseline ? EMPTY_SNAPSHOT : durableSnapshot
const alsoRetire = rebaseline
? [
...durableSnapshot.accounts.map(account => account.did),
...maybeOrphanedDids,
]
: [...maybeOrphanedDids]
try {
writeSessions(durableSnapshot, next, {
alsoRetire: [...maybeOrphanedDids],
writeSessions(previous, next, {
alsoRetire,
forceIndex: rebaseline,
})
durableSnapshot = next
maybeOrphanedDids.clear()