diff --git a/src/main/agent-hooks/local-agent-cli-presence.test.ts b/src/main/agent-hooks/local-agent-cli-presence.test.ts index ff88aba84f3..15f474a9cc7 100644 --- a/src/main/agent-hooks/local-agent-cli-presence.test.ts +++ b/src/main/agent-hooks/local-agent-cli-presence.test.ts @@ -84,6 +84,24 @@ describe('detectLocalManagedAgentCliPresence', () => { expect(probe).toHaveBeenCalledWith(overridePath) }) + it('expands Windows home-relative override paths with Windows separators', async () => { + const overridePath = 'C:\\Users\\orca\\bin\\claude.cmd' + const probe = vi.fn(async (filePath: string) => filePath === overridePath) + const result = await detectLocalManagedAgentCliPresence( + [claudeTarget], + { agentCmdOverrides: { claude: '~\\bin\\claude.cmd --flag' } }, + { + pathEnv: '', + fileProbe: { isExecutableFile: probe }, + platform: 'win32', + homeDir: 'C:\\Users\\orca' + } + ) + + expect(result.claude?.state).toBe('found') + expect(probe).toHaveBeenCalledWith(overridePath) + }) + it('expands home-relative override paths', async () => { const probe = vi.fn(async (filePath: string) => filePath === '/home/orca/bin/codex') const result = await detectLocalManagedAgentCliPresence( @@ -120,13 +138,12 @@ describe('detectLocalManagedAgentCliPresence', () => { }) it('honors PATHEXT for Windows PATH candidates', async () => { - const probe = vi.fn(async (filePath: string) => filePath === 'C:\\Tools/codex.CMD') + const probe = vi.fn(async (filePath: string) => filePath === 'C:\\Tools\\codex.CMD') const result = await detectLocalManagedAgentCliPresence( [codexTarget], { agentCmdOverrides: {} }, { - pathEnv: 'C:\\Tools', - pathDelimiter: ';', + pathEnv: 'C:\\Other;C:\\Tools', pathExt: '.EXE;.CMD', fileProbe: { isExecutableFile: probe }, platform: 'win32' @@ -134,6 +151,12 @@ describe('detectLocalManagedAgentCliPresence', () => { ) expect(result.codex?.state).toBe('found') + expect(probe.mock.calls.map(([filePath]) => filePath)).toEqual([ + 'C:\\Other\\codex.EXE', + 'C:\\Other\\codex.CMD', + 'C:\\Tools\\codex.EXE', + 'C:\\Tools\\codex.CMD' + ]) }) it('warns and uses the inherited PATH when shell hydration throws', async () => { diff --git a/src/main/agent-hooks/local-agent-cli-presence.ts b/src/main/agent-hooks/local-agent-cli-presence.ts index 1c1be7fab0c..256e6520456 100644 --- a/src/main/agent-hooks/local-agent-cli-presence.ts +++ b/src/main/agent-hooks/local-agent-cli-presence.ts @@ -40,6 +40,10 @@ type CommandOverrideSettings = Partial const DEFAULT_WINDOWS_EXTENSIONS = ['.COM', '.EXE', '.BAT', '.CMD'] +function pathApiForPlatform(platform: NodeJS.Platform) { + return platform === 'win32' ? path.win32 : path.posix +} + async function isExecutableFile(filePath: string, platform: NodeJS.Platform): Promise { try { const fileStat = await stat(filePath) @@ -79,7 +83,7 @@ function candidateFileNames( platform: NodeJS.Platform, pathExt?: string ): string[] { - if (platform !== 'win32' || path.extname(candidate)) { + if (platform !== 'win32' || pathApiForPlatform(platform).extname(candidate)) { return [candidate] } return windowsPathExts(pathExt).map((suffix) => `${candidate}${suffix}`) @@ -98,7 +102,7 @@ function expandHomePathToken(token: string, platform: NodeJS.Platform, homeDir: return homeDir } if (token.startsWith('~/') || (platform === 'win32' && token.startsWith('~\\'))) { - return path.join(homeDir, token.slice(2)) + return pathApiForPlatform(platform).join(homeDir, token.slice(2)) } return token } @@ -115,7 +119,7 @@ async function probePathCandidate( } for (const dir of dirs) { for (const fileName of candidateFileNames(candidate, platform, pathExt)) { - if (await fileProbe.isExecutableFile(path.join(dir, fileName))) { + if (await fileProbe.isExecutableFile(pathApiForPlatform(platform).join(dir, fileName))) { return true } } @@ -149,7 +153,7 @@ export async function detectLocalManagedAgentCliPresence( ): Promise { await maybeHydrateShellPath(options) const platform = options.platform ?? process.platform - const delimiter = options.pathDelimiter ?? path.delimiter + const delimiter = options.pathDelimiter ?? pathApiForPlatform(platform).delimiter const dirs = pathEntries(options.pathEnv ?? process.env.PATH ?? '', delimiter) const homeDir = options.homeDir ?? homedir() const fileProbe = options.fileProbe ?? { diff --git a/src/main/agent-hooks/managed-agent-hook-controls.test.ts b/src/main/agent-hooks/managed-agent-hook-controls.test.ts index 95cb40326a9..f87fd8a06dd 100644 --- a/src/main/agent-hooks/managed-agent-hook-controls.test.ts +++ b/src/main/agent-hooks/managed-agent-hook-controls.test.ts @@ -73,6 +73,27 @@ describe('managed agent hook controls', () => { ]) }) + it('fails closed when CLI detection rejects', async () => { + mocks.detect.mockRejectedValue(new Error('detection unavailable')) + + const results = await installManagedAgentHooks({ agentCmdOverrides: {} }) + + expect(mocks.installClaude).not.toHaveBeenCalled() + expect(mocks.installCodex).not.toHaveBeenCalled() + expect(results).toEqual([ + expect.objectContaining({ + agent: 'claude', + state: 'skipped', + skipReason: 'cli_presence_unknown' + }), + expect.objectContaining({ + agent: 'codex', + state: 'skipped', + skipReason: 'cli_presence_unknown' + }) + ]) + }) + it('removes disabled agents without probing or reinstalling them', async () => { mocks.detect.mockResolvedValue({ codex: { state: 'found' } }) diff --git a/src/main/index.ts b/src/main/index.ts index 821f05896de..6ab9eb48d47 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -2565,6 +2565,8 @@ void app.whenReady().then(async () => { const settings = managedHookStore.getSettings() return isAgentStatusHooksEnabled(settings) && !settings.disabledTuiAgents.includes(agent) } + }).catch((error) => { + console.warn('[agent-hooks] failed to reconcile managed hooks on startup:', error) }) } else { removeManagedAgentHooks() diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index d99cf812749..f39b284bfc4 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -1746,6 +1746,45 @@ describe('OrcaRuntimeService', () => { ) }) + it('serializes paired-client hook reconciliation and reads current settings', async () => { + let settings = { + ...store.getSettings(), + agentStatusHooksEnabled: true, + disabledTuiAgents: ['codex', 'claude'] + } + const updateSettings = vi.fn((updates: Partial) => { + settings = { ...settings, ...updates } + return settings + }) + const firstReconciliation = deferred<[]>() + applyAgentStatusHooksEnabledMock + .mockImplementationOnce(() => firstReconciliation.promise) + .mockResolvedValueOnce([]) + const runtime = new OrcaRuntimeService({ + ...store, + getSettings: () => settings, + updateSettings + } as never) + + const first = runtime.updateClientSettings({ disabledTuiAgents: ['claude'] }) + await vi.waitFor(() => expect(applyAgentStatusHooksEnabledMock).toHaveBeenCalledOnce()) + const second = runtime.updateClientSettings({ disabledTuiAgents: [] }) + + expect(applyAgentStatusHooksEnabledMock).toHaveBeenCalledOnce() + const firstOptions = applyAgentStatusHooksEnabledMock.mock.calls[0]?.[2] + expect(firstOptions?.shouldContinue?.('claude')).toBe(true) + + firstReconciliation.resolve([]) + await Promise.all([first, second]) + + expect(applyAgentStatusHooksEnabledMock).toHaveBeenCalledTimes(2) + expect(applyAgentStatusHooksEnabledMock).toHaveBeenLastCalledWith( + true, + expect.objectContaining({ disabledTuiAgents: [] }), + expect.objectContaining({ shouldContinue: expect.any(Function) }) + ) + }) + it('rejects relative paths for runtime nested repo scan/import', async () => { const runtime = new OrcaRuntimeService({ ...store, diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index de95d86043f..a7c6063ed18 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -2578,6 +2578,8 @@ export class OrcaRuntimeService { private readonly runtimeId = randomUUID() private readonly startedAt = Date.now() private readonly store: RuntimeStore | null + private managedHookReconciliationGeneration = 0 + private managedHookReconciliationTail: Promise = Promise.resolve() private readonly orchestrationEnvironmentTransport: OrchestrationEnvironmentTransport | null private readonly orchestrationFederationTimers = new Map>() private readonly orchestrationFederationSyncs = new Map>() @@ -3322,6 +3324,32 @@ export class OrcaRuntimeService { } } + private reconcileManagedAgentHooks(): Promise { + const generation = ++this.managedHookReconciliationGeneration + const reconciliation = this.managedHookReconciliationTail.then(async () => { + if (generation !== this.managedHookReconciliationGeneration) { + return + } + const settings = this.store?.getSettings() + if (!settings) { + return + } + await applyAgentStatusHooksEnabled(settings.agentStatusHooksEnabled !== false, settings, { + onInstallError: recordManagedHookInstallFailure, + shouldContinue: (agent) => { + const current = this.store?.getSettings() + return ( + current !== undefined && + current.agentStatusHooksEnabled !== false && + !current.disabledTuiAgents?.includes(agent) + ) + } + }) + }) + this.managedHookReconciliationTail = reconciliation.catch(() => {}) + return reconciliation + } + async updateClientSettings( updates: Pick< Partial, @@ -3377,17 +3405,7 @@ export class OrcaRuntimeService { (updates.disabledTuiAgents !== undefined && !haveSameDisabledTuiAgents(beforeSettings.disabledTuiAgents, settings.disabledTuiAgents)) ) { - await applyAgentStatusHooksEnabled(settings.agentStatusHooksEnabled !== false, settings, { - onInstallError: recordManagedHookInstallFailure, - shouldContinue: (agent) => { - const current = this.store?.getSettings() - return ( - current !== undefined && - current.agentStatusHooksEnabled !== false && - !current.disabledTuiAgents?.includes(agent) - ) - } - }) + await this.reconcileManagedAgentHooks() } return this.getClientSettings() } diff --git a/src/relay/managed-hook-installer.test.ts b/src/relay/managed-hook-installer.test.ts index ffbac414e96..f33086e871f 100644 --- a/src/relay/managed-hook-installer.test.ts +++ b/src/relay/managed-hook-installer.test.ts @@ -27,11 +27,14 @@ describe('registerManagedHookInstaller', () => { const installManagedHooks = vi.fn().mockResolvedValue({ installers: 14, errors: 0 }) const handler = captureHandler(() => ({ installManagedHooks })) - await expect(handler({}, context(controller.signal))).resolves.toEqual({ + await expect(handler({ agents: ['codex'] }, context(controller.signal))).resolves.toEqual({ installers: 14, errors: 0 }) - expect(installManagedHooks).toHaveBeenCalledWith({ signal: controller.signal }) + expect(installManagedHooks).toHaveBeenCalledWith({ + signal: controller.signal, + agents: ['codex'] + }) }) it('does not load or start the runtime for an already-cancelled request', async () => { @@ -51,14 +54,30 @@ describe('registerManagedHookInstaller', () => { const handler = captureHandler(() => ({ installManagedHooks })) const fingerprint = 'SHA256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA' - await handler({ hostKeyFingerprint: fingerprint }, context()) - await handler({ hostKeyFingerprint: 'ssh://untrusted-host' }, context()) + await handler({ hostKeyFingerprint: fingerprint, agents: ['codex'] }, context()) + await handler({ hostKeyFingerprint: 'ssh://untrusted-host', agents: ['codex'] }, context()) expect(installManagedHooks).toHaveBeenNthCalledWith(1, { signal: undefined, - hostKeyFingerprint: fingerprint + hostKeyFingerprint: fingerprint, + agents: ['codex'] + }) + expect(installManagedHooks).toHaveBeenNthCalledWith(2, { + signal: undefined, + agents: ['codex'] + }) + }) + + it('fails closed when the detected agent allowlist is omitted', async () => { + const installManagedHooks = vi.fn().mockResolvedValue({ installers: 0, errors: 0 }) + const handler = captureHandler(() => ({ installManagedHooks })) + + await handler({}, context()) + + expect(installManagedHooks).toHaveBeenCalledWith({ + signal: undefined, + agents: [] }) - expect(installManagedHooks).toHaveBeenNthCalledWith(2, { signal: undefined }) }) it('validates, deduplicates, and forwards the detected agent allowlist', async () => { diff --git a/src/relay/managed-hook-installer.ts b/src/relay/managed-hook-installer.ts index 849866d5e27..4bb67da2460 100644 --- a/src/relay/managed-hook-installer.ts +++ b/src/relay/managed-hook-installer.ts @@ -30,10 +30,10 @@ function readHostKeyFingerprint(params: unknown): string | undefined { : undefined } -function readAgents(params: unknown): AgentHookTarget[] | undefined { +function readAgents(params: unknown): AgentHookTarget[] { const raw = (params as Partial | null)?.agents if (raw === undefined) { - return undefined + return [] } if (!Array.isArray(raw) || !raw.every(isManagedAgentHookTarget)) { throw new Error('invalid_managed_hook_agents') @@ -65,7 +65,7 @@ export function registerManagedHookInstaller( return await loadRuntime().installManagedHooks({ signal: context.signal, ...(hostKeyFingerprint ? { hostKeyFingerprint } : {}), - ...(agents ? { agents } : {}) + agents }) } ) diff --git a/src/shared/agent-hook-relay.ts b/src/shared/agent-hook-relay.ts index 7443e9199a5..71e7953170a 100644 --- a/src/shared/agent-hook-relay.ts +++ b/src/shared/agent-hook-relay.ts @@ -117,7 +117,7 @@ export type AgentHookInstallManagedHooksParams = { /** SHA-256 fingerprint of the server key negotiated by Orca's SSH transport. */ hostKeyFingerprint?: string /** Positively detected and enabled agents allowed to mutate remote config. */ - agents?: readonly AgentHookTarget[] + agents: readonly AgentHookTarget[] } /** Feature-flag env var. Read once at process start by Orca and the relay. diff --git a/src/shared/managed-agent-command-token.test.ts b/src/shared/managed-agent-command-token.test.ts new file mode 100644 index 00000000000..b58e9c7ac4e --- /dev/null +++ b/src/shared/managed-agent-command-token.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest' +import { + extractExecutableToken, + hasPathSeparatorToken, + isSafeExecutableBasename, + isSafeOverrideExecutableToken +} from './managed-agent-command-token' + +describe('managed agent command tokens', () => { + it('extracts quoted and escaped POSIX executable paths', () => { + expect(extractExecutableToken('"/opt/Agent Tools/codex" --flag', { platform: 'linux' })).toBe( + '/opt/Agent Tools/codex' + ) + expect(extractExecutableToken('/opt/Agent\\ Tools/codex --flag', { platform: 'linux' })).toBe( + '/opt/Agent Tools/codex' + ) + }) + + it('preserves Windows path separators', () => { + expect( + extractExecutableToken('"C:\\Program Files\\Claude\\claude.exe" --flag', { + platform: 'win32' + }) + ).toBe('C:\\Program Files\\Claude\\claude.exe') + }) + + it('distinguishes safe basenames from path tokens', () => { + expect(isSafeExecutableBasename('claude-code_1.2+')).toBe(true) + expect(isSafeExecutableBasename('../claude')).toBe(false) + expect(isSafeExecutableBasename('claude;echo')).toBe(false) + expect(hasPathSeparatorToken('C:\\Tools\\claude.exe')).toBe(true) + expect(hasPathSeparatorToken('/opt/codex')).toBe(true) + expect(hasPathSeparatorToken('codex')).toBe(false) + }) + + it('rejects traversal, control characters, and shell syntax in override paths', () => { + expect(isSafeOverrideExecutableToken('~/bin/codex')).toBe(true) + expect(isSafeOverrideExecutableToken('C:\\Program Files\\Claude\\claude.exe')).toBe(true) + expect(isSafeOverrideExecutableToken('../bin/codex')).toBe(false) + expect(isSafeOverrideExecutableToken('/opt/codex;echo')).toBe(false) + expect(isSafeOverrideExecutableToken('/opt/codex\0')).toBe(false) + }) +})