diff --git a/src/main/agent-hooks/execution-host-agent-homes.ts b/src/main/agent-hooks/execution-host-agent-homes.ts new file mode 100644 index 00000000000..fbfb707f030 --- /dev/null +++ b/src/main/agent-hooks/execution-host-agent-homes.ts @@ -0,0 +1,129 @@ +import { execFile } from 'node:child_process' +import { basename } from 'node:path' +import { userInfo } from 'node:os' +import { promisify } from 'node:util' +import { initializeCodexAppServerConnection } from '../codex/codex-app-server-handshake' +import { runCodexAppServerSession } from '../codex/codex-app-server-session' + +// Why: this module runs on the execution host, so every home it returns is a +// path on that host. Agent config homes are resolved here rather than by each +// installer, which otherwise assumes `~/.` and writes where the agent +// will never read. + +const execFileAsync = promisify(execFile) +const AGENT_HOME_MAX_LENGTH = 4096 +const HOME_PROBE_TIMEOUT_MS = 8_000 + +export function hasControlCharacter(value: string): boolean { + return Array.from(value).some((character) => { + const code = character.charCodeAt(0) + return code <= 0x1f || code === 0x7f + }) +} + +/** Rejects anything that is not a plain absolute POSIX directory path. The + * execution host for these installers is POSIX; a Windows-shaped or + * control-character path is a probe that went wrong, not a home. */ +export function normalizePosixAgentHome(candidate: string): string | null { + if ( + candidate.length === 0 || + candidate.length > AGENT_HOME_MAX_LENGTH || + candidate !== candidate.trim() || + !candidate.startsWith('/') || + candidate.includes('\\') || + hasControlCharacter(candidate) + ) { + return null + } + return candidate.replace(/\/+$/, '') || '/' +} + +export function resolveLoginShell(): string { + const candidate = process.env.SHELL || userInfo().shell || '/bin/sh' + if (!candidate.startsWith('/') || candidate.includes('\\') || hasControlCharacter(candidate)) { + return '/bin/sh' + } + return candidate +} + +function loginShellFlag(shell: string): string { + const shellName = basename(shell) + return shellName === 'sh' || shellName === 'dash' ? '-c' : '-lc' +} + +function defaultAgentHome(home: string, directoryName: string): string { + return `${home.replace(/\/+$/, '') || home}/${directoryName}` +} + +export async function resolveExecutionHostGrokHome( + home: string, + signal?: AbortSignal +): Promise { + const fallback = defaultAgentHome(home, '.grok') + try { + const shell = resolveLoginShell() + // Why: agent PTYs start login shells, so read the same profile-derived + // GROK_HOME without opening two additional SSH exec channels. + const { stdout } = await execFileAsync( + shell, + [loginShellFlag(shell), `printenv GROK_HOME | head -c ${AGENT_HOME_MAX_LENGTH + 1}`], + { encoding: 'utf8', timeout: HOME_PROBE_TIMEOUT_MS, signal } + ) + return normalizePosixAgentHome(stdout.split(/\r?\n/, 1)[0] ?? '') ?? fallback + } catch { + signal?.throwIfAborted() + return fallback + } +} + +/** + * Asks the host's own Codex where its CODEX_HOME is. + * + * Reading the environment the way the Grok probe does is not enough here: a + * `codex` launcher commonly exports CODEX_HOME inside the wrapper script, so it + * exists only for the Codex process and a login shell reports nothing. The + * app-server handshake returns the home the server actually resolved, which + * costs no wrapper parsing and stays correct for any launcher shape. + */ +export async function resolveExecutionHostCodexHome( + home: string, + signal?: AbortSignal +): Promise { + const fallback = defaultAgentHome(home, '.codex') + try { + const shell = resolveLoginShell() + const initializeResult = await runCodexAppServerSession( + { + // Why: launch through the login shell so the probe resolves `codex` the + // same way the PTY does, wrapper included. That leaves no host CLI path + // to pair a node runtime against, which is what `cliPath: null` means. + command: shell, + args: [loginShellFlag(shell), 'exec codex app-server'], + cliPath: null, + timeoutMs: HOME_PROBE_TIMEOUT_MS + }, + async (rpc) => await initializeCodexAppServerConnection(rpc) + ) + const reported = (initializeResult as { codexHome?: unknown } | null)?.codexHome + return (typeof reported === 'string' ? normalizePosixAgentHome(reported) : null) ?? fallback + } catch { + // Why: an absent, old, or hung Codex must not fail hook installation. The + // default home is still right for every launcher that does not redirect. + signal?.throwIfAborted() + return fallback + } +} + +/** + * The Codex home only when the host redirects it away from `~/.codex`. + * + * `undefined` means "the default home", which every installer already assumes, + * so a host with no redirection stays on exactly the path it uses today. + */ +export async function resolveRedirectedExecutionHostCodexHome( + home: string, + signal?: AbortSignal +): Promise { + const resolved = await resolveExecutionHostCodexHome(home, signal) + return resolved === defaultAgentHome(home, '.codex') ? undefined : resolved +} diff --git a/src/main/agent-hooks/execution-host-codex-home.test.ts b/src/main/agent-hooks/execution-host-codex-home.test.ts new file mode 100644 index 00000000000..9d048349c8f --- /dev/null +++ b/src/main/agent-hooks/execution-host-codex-home.test.ts @@ -0,0 +1,172 @@ +import { chmod, mkdir, mkdtemp, readFile, readdir, rm, writeFile } from 'node:fs/promises' +import { existsSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { + resolveExecutionHostCodexHome, + resolveRedirectedExecutionHostCodexHome +} from './execution-host-agent-homes' +import { installManagedHooks } from './managed-hook-runtime' + +const tempHomes: string[] = [] +const tempRoot = process.platform === 'win32' ? tmpdir() : '/tmp' + +async function createTempHome(): Promise { + const home = await mkdtemp(join(tempRoot, 'orca-execution-host-codex-')) + tempHomes.push(home) + return home +} + +/** + * A login shell whose `codex` resolves to a stub app-server, standing in for the + * host wrapper in the report: CODEX_HOME exists only for the Codex process, so + * the handshake is the only place the real home is observable. + */ +async function stubLoginShellWithCodex( + home: string, + { reportedHome }: { reportedHome: string | null } +): Promise { + const server = [ + "const readline = require('node:readline')", + "readline.createInterface({ input: process.stdin }).on('line', (line) => {", + ' const message = JSON.parse(line)', + " if (typeof message.id !== 'number') return", + ` const result = ${reportedHome === null ? '{}' : `{ codexHome: ${JSON.stringify(reportedHome)} }`}`, + " process.stdout.write(JSON.stringify({ id: message.id, result }) + '\\n')", + '})', + '' + ].join('\n') + const serverPath = join(home, 'codex-app-server.js') + await writeFile(serverPath, server, 'utf8') + const shell = join(home, 'login-shell') + await writeFile( + shell, + [ + '#!/bin/sh', + 'case "$2" in', + ` *"codex app-server"*) exec ${JSON.stringify(process.execPath)} ${JSON.stringify(serverPath)} ;;`, + ' *) exit 0 ;;', + 'esac', + '' + ].join('\n'), + 'utf8' + ) + await chmod(shell, 0o755) + vi.stubEnv('HOME', home) + vi.stubEnv('SHELL', shell) +} + +/** A login shell with no `codex` at all: the probe must degrade, not throw. */ +async function stubLoginShellWithoutCodex(home: string): Promise { + const shell = join(home, 'login-shell') + await writeFile(shell, '#!/bin/sh\nexit 127\n', 'utf8') + await chmod(shell, 0o755) + vi.stubEnv('HOME', home) + vi.stubEnv('SHELL', shell) +} + +afterEach(async () => { + vi.unstubAllEnvs() + await Promise.all(tempHomes.splice(0).map((home) => rm(home, { recursive: true, force: true }))) +}) + +describe.runIf(process.platform !== 'win32')('resolveExecutionHostCodexHome', () => { + it('reports the home the host Codex resolved, not the default', async () => { + const home = await createTempHome() + await stubLoginShellWithCodex(home, { reportedHome: `${home}/.codex-openai` }) + + await expect(resolveExecutionHostCodexHome(home)).resolves.toBe(`${home}/.codex-openai`) + }) + + it('normalizes a trailing separator on the reported home', async () => { + const home = await createTempHome() + await stubLoginShellWithCodex(home, { reportedHome: `${home}/.codex-openai///` }) + + await expect(resolveExecutionHostCodexHome(home)).resolves.toBe(`${home}/.codex-openai`) + }) + + it('falls back when the reported home is not an absolute POSIX path', async () => { + const home = await createTempHome() + await stubLoginShellWithCodex(home, { reportedHome: '../relative' }) + + await expect(resolveExecutionHostCodexHome(home)).resolves.toBe(`${home}/.codex`) + }) + + it('falls back when the handshake reports no home at all', async () => { + const home = await createTempHome() + await stubLoginShellWithCodex(home, { reportedHome: null }) + + await expect(resolveExecutionHostCodexHome(home)).resolves.toBe(`${home}/.codex`) + }) + + it('falls back when the host has no usable codex', async () => { + const home = await createTempHome() + await stubLoginShellWithoutCodex(home) + + await expect(resolveExecutionHostCodexHome(home)).resolves.toBe(`${home}/.codex`) + }) +}) + +describe.runIf(process.platform !== 'win32')('resolveRedirectedExecutionHostCodexHome', () => { + it('returns nothing when the host uses the default home', async () => { + const home = await createTempHome() + await stubLoginShellWithCodex(home, { reportedHome: `${home}/.codex` }) + + await expect(resolveRedirectedExecutionHostCodexHome(home)).resolves.toBeUndefined() + }) + + it('returns the home only when it is genuinely redirected', async () => { + const home = await createTempHome() + await stubLoginShellWithCodex(home, { reportedHome: `${home}/.codex-openai` }) + + await expect(resolveRedirectedExecutionHostCodexHome(home)).resolves.toBe( + `${home}/.codex-openai` + ) + }) +}) + +describe.runIf(process.platform !== 'win32')('installManagedHooks on a redirecting host', () => { + it('installs Codex hooks into the home Codex actually reads', async () => { + const home = await createTempHome() + const redirected = join(home, '.codex-openai') + await mkdir(redirected, { recursive: true }) + await stubLoginShellWithCodex(home, { reportedHome: redirected }) + + await expect(installManagedHooks({ agents: ['codex'] })).resolves.toEqual({ + installers: 1, + errors: 0 + }) + + const installed = JSON.parse(await readFile(join(redirected, 'hooks.json'), 'utf8')) as { + hooks: Record + } + expect(Object.keys(installed.hooks).length).toBeGreaterThan(0) + expect(JSON.stringify(installed)).toContain('codex-hook.sh') + // Why: the whole defect is hooks landing in a home Codex never reads. + expect(existsSync(join(home, '.codex', 'hooks.json'))).toBe(false) + // Why: only the config location moves. A redirected home on an SSH host is + // the user's own, with no Orca runtime installer writing it, so it keeps the + // guest-home script contract rather than the runtime-installer one. + expect(existsSync(join(home, '.orca', 'agent-hooks', 'codex-hook.sh'))).toBe(true) + expect(existsSync(join(redirected, '.orca', 'agent-hooks', 'codex-hook.sh'))).toBe(false) + }) + + it('leaves a host that does not redirect on its existing install contract', async () => { + const home = await createTempHome() + await stubLoginShellWithCodex(home, { reportedHome: `${home}/.codex` }) + + await expect(installManagedHooks({ agents: ['codex'] })).resolves.toEqual({ + installers: 1, + errors: 0 + }) + + expect(existsSync(join(home, '.codex', 'hooks.json'))).toBe(true) + // Why: the guest-home script location is the unchanged plain-SSH contract. + // Passing the default home through as a redirect would move it and silently + // reorder every user's hooks, so pin where it lands. + expect(existsSync(join(home, '.orca', 'agent-hooks', 'codex-hook.sh'))).toBe(true) + expect(await readdir(join(home, '.codex'))).not.toContain('.orca') + }) +}) diff --git a/src/main/agent-hooks/managed-hook-runtime.ts b/src/main/agent-hooks/managed-hook-runtime.ts index e995467ee7d..93d644dbb1c 100644 --- a/src/main/agent-hooks/managed-hook-runtime.ts +++ b/src/main/agent-hooks/managed-hook-runtime.ts @@ -1,7 +1,4 @@ -import { execFile } from 'node:child_process' -import { basename } from 'node:path' -import { homedir, userInfo } from 'node:os' -import { promisify } from 'node:util' +import { homedir } from 'node:os' import { installRemoteManagedAgentHooks } from './remote-managed-hook-installers' import type { AgentHookTarget } from '../../shared/agent-hook-types' import { createManagedHookLocalFilesystem } from './managed-hook-local-filesystem' @@ -10,68 +7,19 @@ import { readManagedHookHostIdentity, scopeManagedHookHostIdentity } from './managed-hook-owner-identity' - -const execFileAsync = promisify(execFile) -const GROK_HOME_MAX_LENGTH = 4096 -const GROK_HOME_PROBE_TIMEOUT_MS = 8_000 +import { + resolveExecutionHostGrokHome, + resolveRedirectedExecutionHostCodexHome +} from './execution-host-agent-homes' export type ManagedHookInstallSummary = { installers: number errors: number } -function defaultGrokHome(home: string): string { - return `${home.replace(/\/+$/, '') || home}/.grok` -} - -function hasControlCharacter(value: string): boolean { - return Array.from(value).some((character) => { - const code = character.charCodeAt(0) - return code <= 0x1f || code === 0x7f - }) -} - -function normalizeGrokHome(candidate: string): string | null { - if ( - candidate.length === 0 || - candidate.length > GROK_HOME_MAX_LENGTH || - candidate !== candidate.trim() || - !candidate.startsWith('/') || - candidate.includes('\\') || - hasControlCharacter(candidate) - ) { - return null - } - return candidate.replace(/\/+$/, '') || '/' -} - -function resolveLoginShell(): string { - const candidate = process.env.SHELL || userInfo().shell || '/bin/sh' - if (!candidate.startsWith('/') || candidate.includes('\\') || hasControlCharacter(candidate)) { - return '/bin/sh' - } - return candidate -} - -export async function resolveRelayGrokHome(home: string, signal?: AbortSignal): Promise { - const fallback = defaultGrokHome(home) - try { - const shell = resolveLoginShell() - const shellName = basename(shell) - const mode = shellName === 'sh' || shellName === 'dash' ? '-c' : '-lc' - // Why: agent PTYs start login shells, so read the same profile-derived - // GROK_HOME without opening two additional SSH exec channels. - const { stdout } = await execFileAsync( - shell, - [mode, `printenv GROK_HOME | head -c ${GROK_HOME_MAX_LENGTH + 1}`], - { encoding: 'utf8', timeout: GROK_HOME_PROBE_TIMEOUT_MS, signal } - ) - return normalizeGrokHome(stdout.split(/\r?\n/, 1)[0] ?? '') ?? fallback - } catch { - signal?.throwIfAborted() - return fallback - } -} +/** Kept as the relay-facing name for the Grok probe; the resolution itself now + * lives with every other execution-host home. */ +export const resolveRelayGrokHome = resolveExecutionHostGrokHome export async function installManagedHooks(options?: { signal?: AbortSignal @@ -85,7 +33,16 @@ export async function installManagedHooks(options?: { return { installers: 0, errors: 0 } } const home = homedir() - const grokHomeDir = await resolveRelayGrokHome(home, options?.signal) + const grokHomeDir = await resolveExecutionHostGrokHome(home, options?.signal) + options?.signal?.throwIfAborted() + // Why: gated on Codex being present so a host without it never pays for an + // app-server handshake, and only a home that is actually redirected is passed + // on — handing the installer the default `~/.codex` would change nothing about + // where hooks land while flipping every ordinary SSH host onto the + // redirected-runtime contract. + const codexHomeDir = agents.includes('codex') + ? await resolveRedirectedExecutionHostCodexHome(home, options?.signal) + : undefined options?.signal?.throwIfAborted() const hostIdentity = scopeManagedHookHostIdentity( await readManagedHookHostIdentity(), @@ -99,6 +56,7 @@ export async function installManagedHooks(options?: { createManagedHookLocalFilesystem(), home, { + ...(codexHomeDir ? { codexHomeDir, useRuntimeInstallerHookContract: false } : {}), grokHomeDir, signal: options?.signal, agents diff --git a/src/main/agent-hooks/remote-managed-hook-installers.ts b/src/main/agent-hooks/remote-managed-hook-installers.ts index bf97436f8f8..07c6988dcce 100644 --- a/src/main/agent-hooks/remote-managed-hook-installers.ts +++ b/src/main/agent-hooks/remote-managed-hook-installers.ts @@ -20,6 +20,8 @@ export type RemoteManagedHookInstallOptions = { codexHomeDir?: string /** Skip the trust write when a redirected runtime config is seeded by the launch path. */ deferTrustUntilConfigToml?: boolean + /** Whether `codexHomeDir` is a home Orca's own runtime installer also writes. */ + useRuntimeInstallerHookContract?: boolean /** Explicit GROK_HOME for remote runtimes that redirect Grok's config. */ grokHomeDir?: string /** Stops before starting the next installer when the owning relay request @@ -47,7 +49,8 @@ const REMOTE_MANAGED_HOOK_INSTALLERS: readonly RemoteManagedHookInstaller[] = [ (sftp, remoteHome, options) => codexHookService.installRemote(sftp, remoteHome, { codexHomeDir: options?.codexHomeDir, - deferTrustUntilConfigToml: options?.deferTrustUntilConfigToml + deferTrustUntilConfigToml: options?.deferTrustUntilConfigToml, + useRuntimeInstallerHookContract: options?.useRuntimeInstallerHookContract }) ], ['gemini', (sftp, remoteHome) => geminiHookService.installRemote(sftp, remoteHome)], diff --git a/src/main/codex/codex-app-server-handshake.ts b/src/main/codex/codex-app-server-handshake.ts index 9c89d653baa..0effab0ef43 100644 --- a/src/main/codex/codex-app-server-handshake.ts +++ b/src/main/codex/codex-app-server-handshake.ts @@ -2,10 +2,12 @@ import type { CodexAppServerConnection } from './codex-app-server-connection-typ const HANDSHAKE_TIMEOUT_MS = 15_000 +/** Returns the `initialize` result, which carries the CODEX_HOME the server + * itself resolved — the only wrapper-aware answer available to a caller. */ export async function initializeCodexAppServerConnection( - connection: CodexAppServerConnection -): Promise { - await connection.request( + connection: Pick +): Promise { + const result = await connection.request( 'initialize', { clientInfo: { name: 'orca_desktop', title: 'Orca', version: '0.0.0' }, @@ -19,4 +21,5 @@ export async function initializeCodexAppServerConnection( { timeoutMs: HANDSHAKE_TIMEOUT_MS } ) connection.notify('initialized') + return result } diff --git a/src/main/codex/codex-config-mirror-runtime-added-sections.test.ts b/src/main/codex/codex-config-mirror-runtime-added-sections.test.ts index 493ac056877..40951693b3b 100644 --- a/src/main/codex/codex-config-mirror-runtime-added-sections.test.ts +++ b/src/main/codex/codex-config-mirror-runtime-added-sections.test.ts @@ -1,5 +1,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { appendFileSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { + appendFileSync, + mkdtempSync, + mkdirSync, + readFileSync, + rmSync, + writeFileSync +} from 'node:fs' import { tmpdir } from 'node:os' import type * as NodeOs from 'node:os' import { join } from 'node:path' @@ -218,12 +225,7 @@ describe('ownership between the source config and the managed home', () => { }) it('keeps an added server through the same pass that honours a deletion', () => { - writeSystemConfig( - 'model = "system-model"', - '', - '[mcp_servers.retired]', - 'command = "retired"' - ) + writeSystemConfig('model = "system-model"', '', '[mcp_servers.retired]', 'command = "retired"') syncSystemConfigIntoManagedCodexHome() addServerInsideManagedHome('[mcp_servers.added-in-orca]', 'command = "added"') diff --git a/src/main/codex/codex-hook-remote-install.ts b/src/main/codex/codex-hook-remote-install.ts index 2b6105cdc3d..15067b534fa 100644 --- a/src/main/codex/codex-hook-remote-install.ts +++ b/src/main/codex/codex-hook-remote-install.ts @@ -26,7 +26,14 @@ import { getManagedScript } from './codex-hook-script' export async function installCodexHooksRemote( sftp: SFTPWrapper, remoteHome: string, - options?: { codexHomeDir?: string; deferTrustUntilConfigToml?: boolean } + options?: { + codexHomeDir?: string + deferTrustUntilConfigToml?: boolean + /** Whether this home is also written by Orca's own runtime installer. + * Defaults to the historical coupling with `codexHomeDir`; an SSH host + * whose Codex merely points elsewhere keeps the guest-home contract. */ + useRuntimeInstallerHookContract?: boolean + } ): Promise { const codexHomeBase = options?.codexHomeDir?.replace(/\/$/, '') ?? `${remoteHome.replace(/\/$/, '')}/.codex` @@ -35,7 +42,10 @@ export async function installCodexHooksRemote( // Redirected WSL homes must use the same script location and command shape // as the runtime installer; two representations of one hooks.json race // into stale trust keys. Plain SSH keeps its guest-home script contract. - const redirectedCodexHome = options?.codexHomeDir?.replace(/\/$/, '') + const redirectedCodexHome = + (options?.useRuntimeInstallerHookContract ?? options?.codexHomeDir !== undefined) + ? options?.codexHomeDir?.replace(/\/$/, '') + : undefined const remoteScriptPath = redirectedCodexHome ? `${redirectedCodexHome}/.orca/agent-hooks/codex-hook.sh` : `${remoteHome.replace(/\/$/, '')}/.orca/agent-hooks/codex-hook.sh` diff --git a/src/main/codex/codex-hook-service-implementation.ts b/src/main/codex/codex-hook-service-implementation.ts index fad1cfca459..e44227cade7 100644 --- a/src/main/codex/codex-hook-service-implementation.ts +++ b/src/main/codex/codex-hook-service-implementation.ts @@ -240,7 +240,11 @@ export class CodexHookService { installRemote( sftp: SFTPWrapper, remoteHome: string, - options?: { codexHomeDir?: string; deferTrustUntilConfigToml?: boolean } + options?: { + codexHomeDir?: string + deferTrustUntilConfigToml?: boolean + useRuntimeInstallerHookContract?: boolean + } ): Promise { return installCodexHooksRemote(sftp, remoteHome, options) }