From 3dac5ca445c1d197fc94dcaa3fe5975063578319 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 26 Jul 2026 17:57:34 -0700 Subject: [PATCH] fix(codex): strip the sequenced resume argv on local spawns too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `pty:spawn` handler bound `env` to `baseEnv` before the sequenced-env strip ran. `stripSequencedStartupResumeArgv` returns a new object rather than mutating, so rebinding `baseEnv` left `env` pointing at the pre-strip object. Only the `isDaemonHostSpawn` branch re-derived `env` from `baseEnv`, so every local-provider spawn (LocalPtyProvider, or a provider that routes fresh spawns there) reached the PTY with ORCA_SEQUENCED_STARTUP_COMMAND still carrying `resume `. buildPtyHostEnv prefers that env var over the launch command and the sequenced wrapper evals it, so the pane resumed under whichever account was selected — the exact invariant this change exists to protect — while the renderer showed "started fresh". Declaring `env` after the strip removes the two-variable asymmetry: there is no longer a point in the handler where the pre-strip value is reachable. The runtime-controller path was already correct (it rebinds `env` itself). The gap survived review because every sequenced assertion ran against the daemon provider, which re-derives its env. Adds local-provider coverage for both the stripped and the verified-provenance cases. --- src/main/ipc/pty.test.ts | 36 ++++++++++++++++++++++++++++++++++++ src/main/ipc/pty.ts | 4 +++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 39752745d4a..8de2e6bd40d 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -2097,6 +2097,42 @@ describe('registerPtyHandlers', () => { expect(daemonSpawn.mock.calls.at(-1)![0].env.ORCA_SEQUENCED_STARTUP_COMMAND).toBe(sequenced) }) + posixOnlyIt( + 'strips the sequenced startup command on the local-provider spawn path too', + async () => { + // Why: the daemon branch is the only one that re-derives the spawn env from + // baseEnv, so a local spawn is where a strip that lands on the wrong variable + // silently survives — and the wrapper would `eval` the resume anyway. + const mockProc = createMockProc() + spawnMock.mockReturnValue(mockProc.proc) + registerWithTrustedHomes([OTHER_HOME], OTHER_HOME) + + await spawnCodexResume(ORIGIN_ROLLOUT, { + env: { ORCA_SEQUENCED_STARTUP_COMMAND: `codex 'resume' '${RESUME_SESSION_ID}'` } + }) + + const env = spawnMock.mock.calls.at(-1)![2].env as Record + expect(env.ORCA_SEQUENCED_STARTUP_COMMAND).toBe('codex') + } + ) + + posixOnlyIt( + 'leaves the local-provider sequenced startup command alone when provenance is verified', + async () => { + const mockProc = createMockProc() + spawnMock.mockReturnValue(mockProc.proc) + registerWithTrustedHomes([ORIGIN_HOME], OTHER_HOME) + const sequenced = `codex 'resume' '${RESUME_SESSION_ID}'` + + await spawnCodexResume(ORIGIN_ROLLOUT, { + env: { ORCA_SEQUENCED_STARTUP_COMMAND: sequenced } + }) + + const env = spawnMock.mock.calls.at(-1)![2].env as Record + expect(env.ORCA_SEQUENCED_STARTUP_COMMAND).toBe(sequenced) + } + ) + it('omits the notice on a reattach that never ran this launch command', async () => { setupResumeDaemonProvider({ isReattach: true }) registerWithTrustedHomes([OTHER_HOME], OTHER_HOME) diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index ffbe1b528cd..1bf38567061 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -4472,7 +4472,6 @@ export function registerPtyHandlers( // Why: SSH can strip ORCA_PANE_KEY when remote hooks are off; IPC tab/leaf metadata still names the pane. const reservationPaneKey = metadataPaneKey ?? validatedPaneKey const validatedLeafId = verifiedLeafId ?? metadataLeafId - let env: Record | undefined = baseEnv const effectiveShellOverride = terminalRuntimeOptions.shellOverride const nativeWindowsConptySpawn = isNativeWindowsLocalPtySpawn({ connectionId: args.connectionId, @@ -4498,6 +4497,9 @@ export function registerPtyHandlers( const codexResumeHome = codexResumeLaunch.codexResumeHome const launchCommand = codexResumeLaunch.command baseEnv = stripSequencedStartupResumeArgv(baseEnv, codexResumeLaunch) + // Why: declared after the strip so a local-provider spawn cannot capture the + // pre-strip env — only the daemon branch below re-derives this from baseEnv. + let env: Record | undefined = baseEnv const selectedCodexHomePath = isDaemonHostSpawn ? getCompatibleSelectedCodexHomePath( codexSelectionTarget,