From 062d26195511ed876c3e5edb9d8e76fceaa71a4e Mon Sep 17 00:00:00 2001 From: theone139344 Date: Thu, 23 Jul 2026 07:57:22 +0800 Subject: [PATCH] fix(desktop): prevent cross-session leak in background queue drain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A background queue drain (fromQueue: true) whose runtime binding was reaped by the gateway fires with sessionId=null. The expression options?.sessionId ?? activeSessionIdRef.current falls back to whichever runtime id the foreground happens to hold, landing the queued prompt in the session the user is currently viewing instead of the session that owns the queue entry — a cross-session message leak. Guard the fallback: only inherit the foreground runtime when the drain targets the current view (no storedSessionId, or it matches the foreground). A background drain (storedSessionId differs) is left with sessionId=null so the existing session.resume path rebinds the correct runtime before prompt.submit fires. Includes a regression test: "a fromQueue drain with null runtime id does NOT land in the foreground session (cross-session leak guard)". All 53 existing tests pass. --- .../hooks/use-prompt-actions/index.test.tsx | 56 +++++++++++++++++++ .../hooks/use-prompt-actions/submit.ts | 14 ++++- .../theone139344@users.noreply.github.com | 1 + 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 contributors/emails/theone139344@users.noreply.github.com diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx index 9e8a9532d4fc..b9f18979ec3a 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx @@ -1162,6 +1162,62 @@ describe('usePromptActions submit / queue drain semantics', () => { expect($busy.get()).toBe(false) }) + it('a fromQueue drain with null runtime id does NOT land in the foreground session (cross-session leak guard)', async () => { + // The cross-session leak: a background drain fires with sessionId=null + // (the stored session's runtime was reaped by the gateway). Without the + // guard, `null ?? activeSessionIdRef.current` falls back to whichever + // runtime id the foreground happens to hold — landing the queued prompt + // in the chat the user is currently viewing, NOT the session that owns + // the queue entry. The drain must instead go through session.resume to + // rebind the correct runtime before submitting. + const requestGateway = vi.fn( + async (method: string) => + (method === 'session.resume' ? { session_id: 'rt-session-a-rebound' } : {}) as never + ) + + let handle: HarnessHandle | null = null + render( + null} + onReady={h => (handle = h)} + refreshSessions={async () => undefined} + requestGateway={requestGateway} + /> + ) + + // Background drain: sessionId=null (binding reaped), storedSessionId + // points to a DIFFERENT session than the foreground. + const accepted = await handle!.submitText('queued for background session', { + fromQueue: true, + sessionId: null, + storedSessionId: 'stored-session-a' + }) + + expect(accepted).toBe(true) + // Must resume the correct stored session to get the right runtime id. + expect(requestGateway).toHaveBeenCalledWith('session.resume', { + session_id: 'stored-session-a', + source: 'desktop' + }) + // The prompt must land in the resumed session, NOT the foreground. + expect(requestGateway).toHaveBeenCalledWith( + 'prompt.submit', + { + session_id: 'rt-session-a-rebound', + text: 'queued for background session' + }, + 1_800_000 + ) + // The invariant: the foreground runtime never receives the prompt. + expect( + requestGateway.mock.calls.every( + ([method, params]) => method !== 'prompt.submit' || params?.session_id !== 'rt-foreground' + ) + ).toBe(true) + }) + it('a rejected fromQueue drain returns false (entry stays queued) and a later retry sends it', async () => { // A stale-session 404 must not strand the queued entry: submitPrompt returns // false on failure so the composer keeps it, and the edge-independent diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts index dc7abd08bb73..2f5fd6a653ed 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts @@ -167,7 +167,19 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { const targetStartedInCurrentView = !targetStoredSessionId || targetStoredSessionId === selectedStoredSessionIdRef.current - let sessionId: null | string = options?.sessionId ?? activeSessionIdRef.current + // A queued/background drain whose runtime binding was reaped must NOT + // inherit the foreground runtime id when its storedSessionId targets a + // different session — that would land the queued prompt in whichever + // session the user happens to be viewing (cross-session leak). When the + // drain is for the current view (no storedSessionId, or it matches the + // foreground), the foreground runtime is correct and must be kept. + const isBackgroundQueueDrain = Boolean( + options?.fromQueue && + options?.storedSessionId && + options.storedSessionId !== selectedStoredSessionIdRef.current + ) + + let sessionId: null | string = options?.sessionId ?? (isBackgroundQueueDrain ? null : activeSessionIdRef.current) // Pin the foreground session context for the whole async submit pipeline. // Without this, a fast session switch during session.resume / file.attach diff --git a/contributors/emails/theone139344@users.noreply.github.com b/contributors/emails/theone139344@users.noreply.github.com new file mode 100644 index 000000000000..43a4b1473720 --- /dev/null +++ b/contributors/emails/theone139344@users.noreply.github.com @@ -0,0 +1 @@ +theone139344