mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 16:03:26 +00:00
fix(agent-hooks): harden reconciliation and detection
This commit is contained in:
@@ -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 () => {
|
||||
|
||||
@@ -40,6 +40,10 @@ type CommandOverrideSettings = Partial<Pick<GlobalSettings, 'agentCmdOverrides'>
|
||||
|
||||
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<boolean> {
|
||||
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<LocalCliPresenceByAgent> {
|
||||
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 ?? {
|
||||
|
||||
@@ -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' } })
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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<typeof settings>) => {
|
||||
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,
|
||||
|
||||
@@ -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<void> = Promise.resolve()
|
||||
private readonly orchestrationEnvironmentTransport: OrchestrationEnvironmentTransport | null
|
||||
private readonly orchestrationFederationTimers = new Map<string, ReturnType<typeof setInterval>>()
|
||||
private readonly orchestrationFederationSyncs = new Map<string, Promise<void>>()
|
||||
@@ -3322,6 +3324,32 @@ export class OrcaRuntimeService {
|
||||
}
|
||||
}
|
||||
|
||||
private reconcileManagedAgentHooks(): Promise<void> {
|
||||
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<GlobalSettings>,
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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<AgentHookInstallManagedHooksParams> | 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
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user