From d21382999f03cd5a02bc3839491d7cb033540c33 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 7 Aug 2026 14:58:41 -0700 Subject: [PATCH] fix(agent-session): fail closed on recovered identity --- ...ent-session-process-identity-probe.test.ts | 14 ++++ .../agent-session-process-identity-probe.ts | 15 ++-- .../agent-session-record-store-file.ts | 9 ++- .../agent-session-record-store.test.ts | 5 +- .../runtime/agent-session-record-store.ts | 1 + .../agent-session-store-transaction-queue.ts | 10 +++ .../agent-session-operation-ledger.test.ts | 1 + src/shared/agent-session-operation-ledger.ts | 3 +- .../agent-session-provider-handle.test.ts | 69 ++++++++++++++++++- src/shared/agent-session-provider-handle.ts | 51 ++++++++++---- 10 files changed, 152 insertions(+), 26 deletions(-) diff --git a/src/main/runtime/agent-session-process-identity-probe.test.ts b/src/main/runtime/agent-session-process-identity-probe.test.ts index 9ff4697c8c2..57e0361baa7 100644 --- a/src/main/runtime/agent-session-process-identity-probe.test.ts +++ b/src/main/runtime/agent-session-process-identity-probe.test.ts @@ -88,6 +88,20 @@ describe('owner identity probe', () => { ).resolves.toEqual({ outcome: 'identity-mismatch', field: 'process-start-time' }) }) + it('fails closed when an exact token and the reconstructed start time disagree', async () => { + await expect( + probeAgentSessionProcessIdentity({ + identity: IDENTITY, + deps: deps({ + readProcessStartTimeMs: async () => START_TIME + PROCESS_START_TIME_TOLERANCE_MS + 1 + }) + }) + ).resolves.toEqual({ + outcome: 'indeterminate', + reason: 'process identity evidence contradicted' + }) + }) + it('tolerates start-time jitter inside the tolerance', async () => { await expect( probeAgentSessionProcessIdentity({ diff --git a/src/main/runtime/agent-session-process-identity-probe.ts b/src/main/runtime/agent-session-process-identity-probe.ts index e1a05ae9221..8d63fd485ac 100644 --- a/src/main/runtime/agent-session-process-identity-probe.ts +++ b/src/main/runtime/agent-session-process-identity-probe.ts @@ -45,19 +45,18 @@ function defaultIsPidPresent(pid: number): boolean { async function readLinuxProcessStartTimeMs(pid: number): Promise { try { - const [stat, uptime] = await Promise.all([ + const [stat, systemStat] = await Promise.all([ readFile(`/proc/${pid}/stat`, 'utf-8'), - readFile('/proc/uptime', 'utf-8') + readFile('/proc/stat', 'utf-8') ]) // Field 22 is starttime in clock ticks; the comm field can contain spaces, so cut past ") ". const fields = stat.slice(stat.lastIndexOf(') ') + 2).split(' ') const ticks = Number(fields[19]) - const uptimeSeconds = Number(uptime.split(' ')[0]) - if (!Number.isFinite(ticks) || !Number.isFinite(uptimeSeconds)) { + const bootTimeSeconds = Number(/^btime\s+(\d+)$/m.exec(systemStat)?.[1]) + if (!Number.isFinite(ticks) || !Number.isFinite(bootTimeSeconds)) { return null } - const bootTimeMs = Date.now() - uptimeSeconds * 1000 - return Math.round(bootTimeMs + (ticks / 100) * 1000) + return Math.round(bootTimeSeconds * 1000 + (ticks / 100) * 1000) } catch { return null } @@ -125,6 +124,10 @@ export async function probeAgentSessionProcessIdentity(args: { ) if (observed !== null) { if (Math.abs(observed - identity.processStartTimeMs) > PROCESS_START_TIME_TOLERANCE_MS) { + if (matchedOn.includes('spawn-token')) { + // Why: contradictory evidence cannot prove that a token-authenticated child is dead. + return { outcome: 'indeterminate', reason: 'process identity evidence contradicted' } + } return { outcome: 'identity-mismatch', field: 'process-start-time' } } matchedOn.push('process-start-time') diff --git a/src/main/runtime/agent-session-record-store-file.ts b/src/main/runtime/agent-session-record-store-file.ts index f490c4d3a36..57fe0001920 100644 --- a/src/main/runtime/agent-session-record-store-file.ts +++ b/src/main/runtime/agent-session-record-store-file.ts @@ -36,6 +36,7 @@ export type AgentSessionStoreState = { export type LoadedAgentSessionStore = { state: AgentSessionStoreState + storeFound: boolean /** True when the file was written by a newer schema; this host reads but never writes it. */ readOnly: boolean /** True when the primary file was unusable and the previous committed copy was used. */ @@ -184,6 +185,7 @@ export async function loadAgentSessionStore( } return { state: parsed.state, + storeFound: true, readOnly: parsed.state.schemaVersion > AGENT_SESSION_STORE_SCHEMA_VERSION, recoveredFromBackup } @@ -191,7 +193,12 @@ export async function loadAgentSessionStore( if (unusableStoreFound) { throw new Error('agent_session_store_corrupt') } - return { state: emptyState(hostId), readOnly: false, recoveredFromBackup: false } + return { + state: emptyState(hostId), + storeFound: false, + readOnly: false, + recoveredFromBackup: false + } } function serializeState(state: AgentSessionStoreState): string { diff --git a/src/main/runtime/agent-session-record-store.test.ts b/src/main/runtime/agent-session-record-store.test.ts index 4074dc76cc9..e7bf796704c 100644 --- a/src/main/runtime/agent-session-record-store.test.ts +++ b/src/main/runtime/agent-session-record-store.test.ts @@ -356,9 +356,8 @@ describe('concurrent claims', () => { const store = await open() await establishOwner(store) const before = store.getRecord('session-alpha') - // The next commit cannot land: the store directory is no longer a directory. + // Losing both committed copies must not reset the live store to empty authority. await rm(directory, { recursive: true, force: true }) - await writeFile(directory, 'not a directory') await expect( store.setJournalCheckpoint({ sessionId: 'session-alpha', @@ -829,7 +828,7 @@ describe('orphans, claim keys, checkpoints, and unreadable rows', () => { expect(reopened.recoveredFromBackup).toBe(true) expect(reopened.getRecord('session-alpha')?.lease.runtimeFence).toBe(1) - await reopened.retireClaimKey('key-2', NOW) + await expect(reopened.retireClaimKey('key-2', NOW)).rejects.toThrow() const backup = JSON.parse(await readFile(`${agentSessionStorePath(directory)}.bak`, 'utf-8')) expect(backup.records['session-alpha'].lease.runtimeFence).toBe(1) }) diff --git a/src/main/runtime/agent-session-record-store.ts b/src/main/runtime/agent-session-record-store.ts index 569d3f5402a..3f70fa44a73 100644 --- a/src/main/runtime/agent-session-record-store.ts +++ b/src/main/runtime/agent-session-record-store.ts @@ -80,6 +80,7 @@ export class AgentSessionRecordStore { args.hostId, loaded.readOnly, loaded.recoveredFromBackup, + loaded.storeFound, loaded.state, diskRevision ) diff --git a/src/main/runtime/agent-session-store-transaction-queue.ts b/src/main/runtime/agent-session-store-transaction-queue.ts index f85ca92c13f..230ac6093a4 100644 --- a/src/main/runtime/agent-session-store-transaction-queue.ts +++ b/src/main/runtime/agent-session-store-transaction-queue.ts @@ -53,6 +53,7 @@ export class AgentSessionStoreTransactionQueue { readonly hostId: string, readonly readOnly: boolean, readonly recoveredFromBackup: boolean, + private diskStoreFound: boolean, public state: AgentSessionStoreState, private diskRevision: string ) { @@ -66,6 +67,10 @@ export class AgentSessionStoreTransactionQueue { throw new Error('agent_session_legacy_required') } await this.refreshExternallyChangedState() + if (this.diskRecoveredFromBackup) { + // Why: the missing commit may hold a newer fence, so rollback cannot mint another writer. + throw new Error('execution_owner_reconciling') + } const records = new Map(this.state.records) const operations = new Map(this.state.operations) const retiredClaimKeys = [...this.state.retiredClaimKeys] @@ -80,6 +85,7 @@ export class AgentSessionStoreTransactionQueue { this.state.schemaVersion = AGENT_SESSION_STORE_SCHEMA_VERSION this.diskRevision = agentSessionStoreRevision(this.state) this.diskRecoveredFromBackup = false + this.diskStoreFound = true return result } catch (error) { this.state.records = records @@ -95,6 +101,10 @@ export class AgentSessionStoreTransactionQueue { private async refreshExternallyChangedState(): Promise { const loaded = await loadAgentSessionStore(this.filePath, this.hostId) + if (this.diskStoreFound && !loaded.storeFound) { + throw new Error('agent_session_store_corrupt') + } + this.diskStoreFound ||= loaded.storeFound const diskRevision = agentSessionStoreRevision(loaded.state) this.diskRecoveredFromBackup = loaded.recoveredFromBackup if (diskRevision === this.diskRevision) { diff --git a/src/shared/agent-session-operation-ledger.test.ts b/src/shared/agent-session-operation-ledger.test.ts index e725236e4f7..0710eade461 100644 --- a/src/shared/agent-session-operation-ledger.test.ts +++ b/src/shared/agent-session-operation-ledger.test.ts @@ -168,6 +168,7 @@ describe('persisted row validation', () => { expect(isAgentSessionOperationRow({ ...row, callerKey: '' })).toBe(false) expect(isAgentSessionOperationRow({ ...row, expiresAt: 1.5 })).toBe(false) expect(isAgentSessionOperationRow({ ...row, outcome: { status: 'succeeded' } })).toBe(false) + expect(isAgentSessionOperationRow({ ...row, outcome: null })).toBe(false) expect(isAgentSessionOperationRow(null)).toBe(false) }) }) diff --git a/src/shared/agent-session-operation-ledger.ts b/src/shared/agent-session-operation-ledger.ts index 90f5ba3afcc..864c2c66485 100644 --- a/src/shared/agent-session-operation-ledger.ts +++ b/src/shared/agent-session-operation-ledger.ts @@ -151,7 +151,8 @@ export function isAgentSessionOperationRow(value: unknown): value is AgentSessio const row = value as Partial const outcome = row.outcome as AgentSessionOperationOutcome | undefined const outcomeValid = - outcome !== undefined && + typeof outcome === 'object' && + outcome !== null && ((outcome.status === 'pending' && true) || (outcome.status === 'succeeded' && typeof outcome.sessionId === 'string') || (outcome.status === 'failed' && typeof outcome.code === 'string') || diff --git a/src/shared/agent-session-provider-handle.test.ts b/src/shared/agent-session-provider-handle.test.ts index 98e325a9829..4605a2c1129 100644 --- a/src/shared/agent-session-provider-handle.test.ts +++ b/src/shared/agent-session-provider-handle.test.ts @@ -43,8 +43,8 @@ describe('handle identity', () => { it('keys a Codex handle by thread id alone', () => { const codex: AgentSessionProviderHandle = { provider: 'codex', threadId: 'thread-1' } - expect(agentSessionProviderHandleKey(codex)).toBe('codex:thread-1') - expect(agentSessionProviderHandleRoot(codex)).toBe('codex:thread-1') + expect(agentSessionProviderHandleKey(codex)).toBe('codex:"thread-1"') + expect(agentSessionProviderHandleRoot(codex)).toBe('codex:"thread-1"') expect( agentSessionProviderHandlesEqual(codex, { provider: 'codex', threadId: 'thread-2' }) ).toBe(false) @@ -57,6 +57,21 @@ describe('handle identity', () => { expect(isAgentSessionProviderHandle({ provider: 'gemini', sessionId: 'x' })).toBe(false) expect(isAgentSessionProviderHandle({ ...CLAUDE, sessionId: ' sess-1 ' })).toBe(false) }) + + it('uses collision-free keys when Claude ids contain delimiters', () => { + const left: AgentSessionProviderHandle = { + provider: 'claude', + sessionId: 'a#b', + leafUuid: 'c' + } + const right: AgentSessionProviderHandle = { + provider: 'claude', + sessionId: 'a', + leafUuid: 'b#c' + } + expect(agentSessionProviderHandleKey(left)).not.toBe(agentSessionProviderHandleKey(right)) + expect(agentSessionProviderHandlesEqual(left, right)).toBe(false) + }) }) describe('chain append', () => { @@ -153,6 +168,19 @@ describe('chain append', () => { ).toHaveLength(2) }) + it('rejects reuse of a stable link id for a different proof', () => { + expect(() => + appendAgentSessionProviderHandleLink( + [link()], + link({ + origin: 'resumed', + handle: { ...CLAUDE, leafUuid: 'leaf-2' }, + mintedAtFence: 2 + }) + ) + ).toThrow('agent_session_provider_handle_invalid') + }) + it('refuses to grow past the cap rather than dropping fork provenance', () => { const chain: AgentSessionProviderHandleLink[] = [link()] for (let index = 1; index < MAX_AGENT_SESSION_PROVIDER_HANDLE_LINKS; index += 1) { @@ -218,6 +246,9 @@ describe('chain lookup and validation', () => { it('rejects a persisted chain that is over the cap or holds a malformed link', () => { expect(isAgentSessionProviderHandleChain([{ ...link(), mintedAtFence: -1 }])).toBe(false) expect(isAgentSessionProviderHandleChain([{ ...link(), linkId: 'not a link id!' }])).toBe(false) + expect(isAgentSessionProviderHandleChain([{ ...link(), forkedFromKey: 'claude:seed' }])).toBe( + false + ) expect( isAgentSessionProviderHandleChain( Array.from({ length: MAX_AGENT_SESSION_PROVIDER_HANDLE_LINKS + 1 }, (_value, index) => @@ -226,4 +257,38 @@ describe('chain lookup and validation', () => { ) ).toBe(false) }) + + it('rejects persisted chains that bypass append invariants', () => { + expect( + isAgentSessionProviderHandleChain([ + link(), + link({ + linkId: 'link-2', + origin: 'created', + mintedAtFence: 2 + }) + ]) + ).toBe(false) + expect( + isAgentSessionProviderHandleChain([ + link(), + link({ + origin: 'resumed', + handle: { ...CLAUDE, leafUuid: 'leaf-2' }, + mintedAtFence: 2 + }) + ]) + ).toBe(false) + expect( + isAgentSessionProviderHandleChain([ + link(), + link({ + linkId: 'link-2', + origin: 'resumed', + handle: { provider: 'claude', sessionId: 'sess-2', leafUuid: 'leaf-2' }, + mintedAtFence: 2 + }) + ]) + ).toBe(false) + }) }) diff --git a/src/shared/agent-session-provider-handle.ts b/src/shared/agent-session-provider-handle.ts index 34e76252dfa..f36be80672c 100644 --- a/src/shared/agent-session-provider-handle.ts +++ b/src/shared/agent-session-provider-handle.ts @@ -64,8 +64,8 @@ export function isAgentSessionProviderHandle(value: unknown): value is AgentSess /** Stable string identity for one handle. Two handles with the same key name the same writer target. */ export function agentSessionProviderHandleKey(handle: AgentSessionProviderHandle): string { return handle.provider === 'claude' - ? `claude:${handle.sessionId}#${handle.leafUuid ?? ''}` - : `codex:${handle.threadId}` + ? `claude:${JSON.stringify([handle.sessionId, handle.leafUuid])}` + : `codex:${JSON.stringify(handle.threadId)}` } /** @@ -73,7 +73,9 @@ export function agentSessionProviderHandleKey(handle: AgentSessionProviderHandle * whatever the provider called it. */ export function agentSessionProviderHandleRoot(handle: AgentSessionProviderHandle): string { - return handle.provider === 'claude' ? `claude:${handle.sessionId}` : `codex:${handle.threadId}` + return handle.provider === 'claude' + ? `claude:${JSON.stringify(handle.sessionId)}` + : `codex:${JSON.stringify(handle.threadId)}` } export function agentSessionProviderHandlesEqual( @@ -103,29 +105,48 @@ export function isAgentSessionProviderHandleLink( return false } const link = value as Partial + const originValid = + link.origin === 'created' || + link.origin === 'adopted' || + link.origin === 'resumed' || + link.origin === 'forked' return ( typeof link.linkId === 'string' && LINK_ID_PATTERN.test(link.linkId) && isAgentSessionProviderHandle(link.handle) && - (link.origin === 'created' || - link.origin === 'adopted' || - link.origin === 'resumed' || - link.origin === 'forked') && + originValid && Number.isSafeInteger(link.mintedAtFence) && (link.mintedAtFence as number) >= 0 && Number.isSafeInteger(link.observedAt) && - (link.forkedFromKey === undefined || isHandleField(link.forkedFromKey)) + (link.origin === 'forked' + ? isHandleField(link.forkedFromKey) + : link.forkedFromKey === undefined) ) } export function isAgentSessionProviderHandleChain( value: unknown ): value is AgentSessionProviderHandleLink[] { - return ( - Array.isArray(value) && - value.length <= MAX_AGENT_SESSION_PROVIDER_HANDLE_LINKS && - value.every((link) => isAgentSessionProviderHandleLink(link)) - ) + if (!Array.isArray(value) || value.length > MAX_AGENT_SESSION_PROVIDER_HANDLE_LINKS) { + return false + } + let validated: AgentSessionProviderHandleLink[] = [] + try { + for (const link of value) { + if (!isAgentSessionProviderHandleLink(link)) { + return false + } + const next = appendAgentSessionProviderHandleLink(validated, link) + // A persisted chain must name every link exactly once; retry elision belongs at append time. + if (next.length !== validated.length + 1) { + return false + } + validated = next + } + return true + } catch { + return false + } } /** @@ -178,6 +199,10 @@ export function appendAgentSessionProviderHandleLink( // Why: re-proving the same handle at the same fence is a retry, not a new identity. return [...chain] } + if (findAgentSessionProviderHandleLink(chain, link.linkId)) { + // Why: the lease names its exact proof by link id; reuse would make that reference ambiguous. + throw new Error('agent_session_provider_handle_invalid') + } if (chain.length >= MAX_AGENT_SESSION_PROVIDER_HANDLE_LINKS) { // Why: dropping older links would erase fork provenance, so refuse and let the caller roll // the journal epoch instead of silently losing where this conversation came from.