diff --git a/.github/workflows/adhoc-mac-build.yml b/.github/workflows/adhoc-mac-build.yml index d7dd6d5ffb6..761a9585b73 100644 --- a/.github/workflows/adhoc-mac-build.yml +++ b/.github/workflows/adhoc-mac-build.yml @@ -127,9 +127,12 @@ jobs: esac # Bare: a work-tree repo refuses to fetch over its own checked-out # branch. tree:0 keeps the fetch to the commit graph — no trees, no - # blobs — so this stays cheap next to the build it fronts. + # blobs — so this stays cheap next to the build it fronts. reftable + # because this repo has branches that differ only in casing, and the + # files backend cannot store both on a case-insensitive runner disk — + # it fails the entire fetch, not just the one ref. scratch="$RUNNER_TEMP/vet-requested-ref" - git init -q --bare "$scratch" + git init -q --bare --ref-format=reftable "$scratch" git -C "$scratch" fetch -q --filter=tree:0 "$REPO_URL" '+refs/heads/*:refs/heads/*' '+refs/tags/*:refs/tags/*' # Branch first to keep actions/checkout's old tie-break: bare # rev-parse would prefer the tag when a branch shares its name. diff --git a/.github/workflows/dev-channel-win-build.yml b/.github/workflows/dev-channel-win-build.yml index e16a50f1c3c..89fda2ebef9 100644 --- a/.github/workflows/dev-channel-win-build.yml +++ b/.github/workflows/dev-channel-win-build.yml @@ -149,9 +149,12 @@ jobs: fi # Reachability is the trust test: GitHub serves PR-only commits by SHA, # so resolving the object is not proof a branch or tag of this repo - # reaches it. Bare + tree:0 keeps this to the commit graph. + # reaches it. Bare + tree:0 keeps this to the commit graph; reftable + # because branches that differ only in casing cannot both be stored by + # the files backend on a case-insensitive runner disk, which fails the + # entire fetch rather than the one ref. scratch="$RUNNER_TEMP/vet-requested-ref" - git init -q --bare "$scratch" + git init -q --bare --ref-format=reftable "$scratch" git -C "$scratch" fetch -q --filter=tree:0 "$REPO_URL" '+refs/heads/*:refs/heads/*' '+refs/tags/*:refs/tags/*' if ! git -C "$scratch" rev-parse --verify --quiet "$REQUESTED_SHA^{commit}" >/dev/null; then echo "::error::Commit $REQUESTED_SHA is not in stablyai/orca." diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index ba2eaf83192..93bc4c0afc8 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -844,6 +844,7 @@ jobs: src/main/providers/pty-repaint-wide-char-buffer.node-pty.test.ts src/shared/child-process/windows-command-line.win32.test.ts src/main/agent-hooks/windows-hook-payload-delivery.test.ts + src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts src/main/windows/windows-pty-job.win32.test.ts src/main/windows/windows-host-job.win32.test.ts src/main/windows-live-tree-kill.win32.test.ts diff --git a/config/scripts/pr-code-change-scope.mjs b/config/scripts/pr-code-change-scope.mjs index 15ded915c67..fd36a803bb9 100644 --- a/config/scripts/pr-code-change-scope.mjs +++ b/config/scripts/pr-code-change-scope.mjs @@ -217,6 +217,7 @@ const WINDOWS_PACKAGE_TESTS = [ 'src/main/providers/pty-repaint-wide-char-buffer.node-pty.test.ts', 'src/shared/child-process/windows-command-line.win32.test.ts', 'src/main/agent-hooks/windows-hook-payload-delivery.test.ts', + 'src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts', 'src/main/windows/windows-pty-job.win32.test.ts', 'src/main/windows/windows-host-job.win32.test.ts', 'src/main/windows-live-tree-kill.win32.test.ts', diff --git a/config/scripts/workflow-ref-mirror-case-safety.test.mjs b/config/scripts/workflow-ref-mirror-case-safety.test.mjs new file mode 100644 index 00000000000..6008c9d8d5f --- /dev/null +++ b/config/scripts/workflow-ref-mirror-case-safety.test.mjs @@ -0,0 +1,34 @@ +import { readFileSync } from 'node:fs' +import { join, resolve } from 'node:path' +import { describe, expect, it } from 'vitest' +import { parse } from 'yaml' + +const projectDir = resolve(import.meta.dirname, '../..') + +const readWorkflow = (relativePath) => parse(readFileSync(join(projectDir, relativePath), 'utf8')) + +// Every step that mirrors this repo's whole ref namespace onto a runner disk to +// prove a commit is reachable from a branch or tag before signing it. +const REF_MIRRORS = [ + ['.github/workflows/adhoc-mac-build.yml', 'build-adhoc-mac', 'Vet the requested ref'], + ['.github/workflows/dev-channel-win-build.yml', 'build-win', 'Vet the requested inputs'] +] + +describe('ref-mirroring vet steps', () => { + // Why: macOS and Windows runner disks are case-insensitive, and this repo has + // branches that differ only in casing. The files backend cannot store both, and + // it fails the whole fetch rather than the one ref — so the vet step dies before + // any build runs. reftable keys refs in a table instead of file paths. + it.each(REF_MIRRORS)( + '%s creates its scratch repo with the reftable backend', + (path, job, step) => { + const run = readWorkflow(path).jobs[job].steps.find( + (candidate) => candidate.name === step + ).run + + expect(run).toContain('+refs/heads/*:refs/heads/*') + expect(run).toMatch(/git init\b[^\n]*--ref-format=reftable/) + expect(run).not.toMatch(/git init -q --bare "\$scratch"/) + } + ) +}) diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 1b9600188f2..2423647577b 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -16,6 +16,7 @@ "../src/main/agent-hooks/managed-hook-script-refresh.ts", "../src/main/agent-hooks/posix-hook-command.ts", "../src/main/agent-hooks/runtime-home-hook-command.ts", + "../src/main/agent-hooks/windows-direct-cmd-hook-command.ts", "../src/main/agent-hooks/windows-powershell-hook-launcher.ts", "../src/main/amp/agent-status-plugin-source.ts", "../src/main/amp/hook-service.ts", @@ -117,6 +118,7 @@ "../src/main/hermes/hermes-home-filesystem.ts", "../src/main/hermes/hermes-managed-plugin-source.ts", "../src/main/hermes/hook-service.ts", + "../src/main/git-bash.ts", "../src/main/in-flight-run-dedupe.ts", "../src/main/kimi/hook-service.ts", "../src/main/kimi/kimi-hook-config-toml.ts", diff --git a/docs/assets/readme-downloads.svg b/docs/assets/readme-downloads.svg index fe660c42295..33ad276aa2d 100644 --- a/docs/assets/readme-downloads.svg +++ b/docs/assets/readme-downloads.svg @@ -1,5 +1,5 @@ - - downloads: 40m + + downloads: 41m @@ -15,7 +15,7 @@ downloads downloads - 40m - 40m + 41m + 41m diff --git a/docs/reference/windows-edr-posture.md b/docs/reference/windows-edr-posture.md index 9a8c5d51524..1f6ac777f17 100644 --- a/docs/reference/windows-edr-posture.md +++ b/docs/reference/windows-edr-posture.md @@ -179,7 +179,8 @@ What remains is `-EncodedCommand` without the bypass: the PTY bootstraps `src/main/providers/windows-shell-args.ts`), the hook wrappers (`src/main/agent-hooks/windows-powershell-hook-launcher.ts` and its callers `src/main/agent-hooks/runtime-home-hook-command.ts`, -`src/main/agent-hooks/installer-utils.ts`, `src/main/claude/hook-settings.ts`), +`src/main/agent-hooks/installer-utils.ts`, and `src/main/claude/hook-settings.ts` +— that last one only as a *fallback* since #18875, see below), `src/main/runtime/windows-default-route-interfaces.ts`, `src/main/runtime/orchestration/setup-completion-signal.ts`, `src/shared/hermes-startup-query.ts`, and the four ex-bypass sites above. @@ -255,6 +256,41 @@ breadth: every interpreter hop between Orca and the thing the user asked for add a scored edge, which is why the shipped doctrine of #15520 and #15595 is to *shorten the interpreter chain* rather than to hide a window. +#18875 is a worked example of that doctrine. The Claude Code lifecycle hook was +registered as `powershell.exe -NoProfile -EncodedCommand <...>` whose entire +decoded payload was a `Test-Path` and a call to `~/.orca/agent-hooks/claude-hook.cmd`. +It now registers the script path itself (` || echo {}`), so `bash -> +powershell -> cmd -> curl` became `bash -> cmd -> curl` and one +`powershell.exe -EncodedCommand` per hook event — a first-class Defender alert +title — leaves the tree. The reporting box fired ~6 900 of them in five days, +70% from Claude sessions that were not running under Orca at all and whose hook +exits at its first `ORCA_PANE_KEY` guard. + +What is measured is latency and the hop count, nothing else: median 471 ms -> +213 ms per event idle, and 656 ms -> 296 ms (p95 696 ms -> 337 ms) under 10-way +concurrency, invoked as Claude Code invokes it. **No EDR verdict on either tree +was measured**, so claim the removed `-EncodedCommand` spelling and the shorter +chain, not a score. `cmd.exe` remains in the tree, spelled by MSYS's own `.cmd` +spawn rather than by us — the doc's one "unavoidable for `.cmd`/`.bat`" case, +carrying an absolute path and two literal tokens, with no caret escaping, no +encoding and no free text. The encoded launcher is still the shape for profile +paths the shells cannot carry bare (a space, `%`, `^`, `&`, non-ASCII, a UNC +profile) and for hosts where Git Bash is not resolvable, because PowerShell 5.1 +rejects `||` (measured: parse error, exit 1). + +That last clause is the standing assumption of this change, and it is worth +stating plainly because it is **not** measured. `||` parses in Git Bash, cmd.exe +and pwsh, but not in Windows PowerShell 5.1, so the direct shape is correct for +any host that is one of the first three. Claude Code itself is a Git Bash host on +native Windows. What no one here has verified is which host a *compat consumer* +uses: cursor-agent and Devin import `~/.claude/settings.json` and run `command` +through their own launcher (the managed `.cmd` carries a `DEVIN_PROJECT_DIR` skip +for exactly that). If one of them spawns hook strings through Windows PowerShell +5.1, its imported Claude events become a parse error with empty stdout, which is +the fail-closed case #14818 exists to prevent. The encoded launcher had no such +assumption — it was a `powershell.exe` invocation and therefore parsed anywhere. +Before widening the direct shape to another agent, measure that consumer's host. + ### Computer use: screen capture, synthetic input, runtime-compiled MSIL `native/computer-use-windows/runtime.ps1` is a large PowerShell script. diff --git a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts index 40237141d24..af70f6f54f0 100644 --- a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts +++ b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts @@ -4,7 +4,7 @@ // missing-Orca-env path, so their writer may break there. import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { spawn } from 'node:child_process' -import { mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import type { SFTPWrapper } from 'ssh2' @@ -60,6 +60,7 @@ import { DroidHookService } from '../droid/hook-service' import { GeminiHookService } from '../gemini/hook-service' import { GrokHookService } from '../grok/hook-service' import { KimiHookService } from '../kimi/hook-service' + import { openClaudeHookService } from '../openclaude/hook-service' import { wrapPosixHookCommand, wrapWindowsHookCommand } from './installer-utils' import { POSIX_HOOK_STDIN_READER } from './hook-stdin-contract' @@ -69,6 +70,16 @@ import { findGitBash } from './windows-git-bash-path.test-fixture' const REMOTE_HOME = '/home/dev' const LARGE_PAYLOAD = Buffer.alloc(1_000_000, 'x') + +// Why: a developer box may set HKCU\...\Command Processor\AutoRun, which cmd.exe runs before any +// .cmd — and MSYS spawns a .cmd without `/d`, so it fires on the Git Bash legs. Redirecting the +// profile makes the usual `%USERPROFILE%\.cmd_aliases.cmd` target vanish, putting cmd's "not +// recognized" on the hook's stderr. Seed an empty target so these suites measure the launcher +// rather than the host's shell configuration. +function seedCmdAutoRunTarget(profileDir: string): void { + mkdirSync(profileDir, { recursive: true }) + writeFileSync(join(profileDir, '.cmd_aliases.cmd'), '@echo off\r\n', 'utf8') +} const REMOTE_INSTALLERS = [ { agent: 'antigravity', @@ -228,6 +239,7 @@ describe('Windows managed hook stdin structure', () => { it('exits immediately when Orca env is missing and keeps drain for other failures', async () => { const home = mkdtempSync(join(tmpdir(), 'orca-hook-stdin-windows-')) homedirMock.mockReturnValue(home) + seedCmdAutoRunTarget(home) const previousGrokHome = process.env.GROK_HOME const previousKimiHome = process.env.KIMI_CODE_HOME delete process.env.GROK_HOME @@ -317,6 +329,7 @@ describe('Windows managed hook stdin structure', () => { async () => { const home = mkdtempSync(join(tmpdir(), 'orca-hook-stdin-windows-live-')) homedirMock.mockReturnValue(home) + seedCmdAutoRunTarget(home) try { const gitBash = findGitBash() for (const entry of LOCAL_INSTALLERS) { @@ -399,6 +412,9 @@ describe('Windows managed hook stdin structure', () => { async () => { const home = mkdtempSync(join(tmpdir(), 'orca-hook-stdout-json-')) homedirMock.mockReturnValue(home) + const absentProfile = join(home, 'absent') + seedCmdAutoRunTarget(home) + seedCmdAutoRunTarget(absentProfile) try { expect(new ClaudeHookService().install().state).toBe('installed') const settings = JSON.parse( @@ -426,8 +442,12 @@ describe('Windows managed hook stdin structure', () => { }) }, { + // Why: the encoded launcher resolves %USERPROFILE% at run time, so redirecting it is + // what makes the script vanish for that shape. The direct launcher (#18875) carries + // an absolute path, so here it asserts only that a bogus profile changes nothing; its + // missing-script fallback is covered live in windows-direct-cmd-hook-command.test.ts. name: 'missing managed script', - env: hookEnvironment({ USERPROFILE: join(home, 'absent') }) + env: hookEnvironment({ USERPROFILE: absentProfile }) } ] for (const shell of shells) { diff --git a/src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts b/src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts new file mode 100644 index 00000000000..acb4bf2d46d --- /dev/null +++ b/src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts @@ -0,0 +1,143 @@ +// Why (#18875): the registered Windows Claude hook is now the script path itself, so this file +// pins the two things that make that safe — the shape carries nothing MSYS or cmd.exe rewrites, +// and it still answers with neutral JSON when the script is gone. The live legs run the string +// through BOTH hosts Claude Code can pick, because the shape has to parse in either. +import { describe, expect, it } from 'vitest' +import { execFileSync } from 'node:child_process' +import { existsSync, mkdtempSync, readdirSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { removeTreeSync } from '../../shared/windows-transient-lock-removal' +import { WINDOWS_CMD_SAFE_PATH } from './installer-utils' +import { wrapWindowsDirectCmdHookCommand } from './windows-direct-cmd-hook-command' +import { findGitBash } from './windows-git-bash-path.test-fixture' + +const SAFE_PATH = 'C:\\Users\\alice\\.orca\\agent-hooks\\claude-hook.cmd' + +describe('wrapWindowsDirectCmdHookCommand', () => { + it('emits the script path with forward slashes and a neutral-JSON fallback', () => { + expect(wrapWindowsDirectCmdHookCommand(SAFE_PATH)).toBe( + 'C:/Users/alice/.orca/agent-hooks/claude-hook.cmd || echo {}' + ) + }) + + it('spells nothing either shell would rewrite or reinterpret', () => { + const command = wrapWindowsDirectCmdHookCommand(SAFE_PATH)! + + // Why: MSYS rewrites `/c`-shaped tokens into drive paths — a literal `cmd.exe /d /c ` + // does not survive Git Bash (measured), which is why no interpreter is spelled at all. + expect(command).not.toMatch(/ \/[a-zA-Z]+( |$)/) + expect(command).not.toMatch(/\\/) + expect(command).not.toMatch(/["']/) + expect(command).not.toMatch(/powershell|cmd\.exe|conhost/i) + // Why: `2>nul` writes a literal file named `nul` into the cwd under MSYS (measured), and no + // stderr sink parses in both hosts. The missing-script line is left on stderr deliberately. + expect(command).not.toContain('2>') + }) + + it('declines any path the shells cannot carry bare', () => { + for (const path of [ + 'C:\\Users\\Bob Smith\\.orca\\agent-hooks\\claude-hook.cmd', + 'C:\\Users\\%name%\\.orca\\agent-hooks\\claude-hook.cmd', + 'C:\\Users\\a^b\\.orca\\agent-hooks\\claude-hook.cmd', + 'C:\\Users\\a&b\\.orca\\agent-hooks\\claude-hook.cmd', + 'C:\\Users\\a(b)\\.orca\\agent-hooks\\claude-hook.cmd', + 'C:\\Users\\rené\\.orca\\agent-hooks\\claude-hook.cmd', + '/home/alice/.orca/agent-hooks/claude-hook.sh', + // Why: WINDOWS_CMD_SAFE_PATH admits a UNC profile, but `//server/share/...` is not a + // command cmd.exe reliably starts — keep those on the encoded launcher. + '\\\\server\\share\\alice\\.orca\\agent-hooks\\claude-hook.cmd' + ]) { + expect(wrapWindowsDirectCmdHookCommand(path), path).toBeNull() + } + }) +}) + +describe.skipIf(process.platform !== 'win32')('direct hook command, run by both hook hosts', () => { + // Why: the fixture throws when Git Bash is absent, and that is a skip here, not a failure — + // a box without it never gets this command shape in the first place. + const gitBash = ((): string | null => { + try { + return findGitBash() + } catch { + return null + } + })() + + function runInCmd(command: string, cwd: string): { stdout: string; status: number } { + return runCapture('cmd.exe', ['/d', '/c', command], cwd) + } + + function runInBash(command: string, cwd: string): { stdout: string; status: number } { + return runCapture(gitBash!, ['-c', command], cwd) + } + + function runCapture(file: string, args: string[], cwd: string) { + try { + const stdout = execFileSync(file, args, { + cwd, + input: '{"hook_event_name":"PreToolUse"}', + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'] + }) + return { stdout, status: 0 } + } catch (error) { + const failure = error as { stdout?: string; status?: number } + return { stdout: failure.stdout ?? '', status: failure.status ?? 1 } + } + } + + // Why: a runner whose TEMP sits under a profile with a space is the encoded-launcher case, + // so these legs skip rather than assert a contract that shape never claimed. + const tempIsCmdSafe = WINDOWS_CMD_SAFE_PATH.test(join(tmpdir(), 'orca-direct-hook-x', 'x.cmd')) + const canRunLive = Boolean(gitBash) && tempIsCmdSafe + + function withTempDir(run: (dir: string, scriptPath: string, command: string) => void): void { + const dir = mkdtempSync(join(tmpdir(), 'orca-direct-hook-')) + try { + const scriptPath = join(dir, 'claude-hook.cmd') + const command = wrapWindowsDirectCmdHookCommand(scriptPath) + expect(command, 'precondition: temp path must be cmd-safe').not.toBeNull() + run(dir, scriptPath, command!) + } finally { + // Why: cmd.exe/bash have just exited in this tree; a raw recursive rm throws EPERM on + // Windows while their handles drain. + removeTreeSync(dir) + } + } + + it.skipIf(!canRunLive)('answers {} and exit 0 in both hosts when the script exists', () => { + withTempDir((dir, scriptPath, command) => { + writeFileSync(scriptPath, '@echo off\r\necho {}\r\nexit /b 0\r\n', 'utf8') + for (const result of [runInCmd(command, dir), runInBash(command, dir)]) { + expect(result.stdout.trim()).toBe('{}') + expect(result.status).toBe(0) + } + }) + }) + + it.skipIf(!canRunLive)( + 'still answers {} and exit 0 in both hosts when the script is gone', + () => { + // Why: compat consumers require neutral JSON even with no managed script (#14818). The + // encoded launcher did this with a Test-Path; `|| echo {}` does it with no interpreter. + withTempDir((dir, scriptPath, command) => { + expect(existsSync(scriptPath)).toBe(false) + for (const result of [runInCmd(command, dir), runInBash(command, dir)]) { + expect(result.stdout.trim()).toBe('{}') + expect(result.status).toBe(0) + } + }) + } + ) + + it.skipIf(!canRunLive)('leaves no stray `nul` file behind in the working directory', () => { + // Why this is worth a test: adding `2>nul` to silence the missing-script line looks like + // tidy-up, but under MSYS it creates a real file named `nul` in the cwd — which is the + // user's repo. Measured on Windows 11. Keep stderr unredirected. + withTempDir((dir, _scriptPath, command) => { + runInBash(command, dir) + expect(readdirSync(dir)).not.toContain('nul') + }) + }) +}) diff --git a/src/main/agent-hooks/windows-direct-cmd-hook-command.ts b/src/main/agent-hooks/windows-direct-cmd-hook-command.ts new file mode 100644 index 00000000000..f7646f19578 --- /dev/null +++ b/src/main/agent-hooks/windows-direct-cmd-hook-command.ts @@ -0,0 +1,30 @@ +import { WINDOWS_CMD_SAFE_PATH } from './installer-utils' + +// Why: a drive-letter path only. WINDOWS_CMD_SAFE_PATH also admits a UNC profile, and +// `//server/share/...` is not a command cmd.exe reliably starts. +const WINDOWS_DRIVE_LETTER_PATH = /^[A-Za-z]:\\/ + +/** + * Shortest launcher for a managed Windows `.cmd` hook: the script path itself (#18875). + * + * The encoded PowerShell launcher spent a full interpreter start-up per hook event to reach a + * script that exits at its first `ORCA_PANE_KEY` guard, and left a stdout-holding orphan behind + * when the hook's timeout kill landed. Measurements and the EDR trade are in + * `docs/reference/windows-edr-posture.md`. + * + * Returns null when the caller must keep the encoded launcher: a path either shell would mangle. + */ +export function wrapWindowsDirectCmdHookCommand(scriptPath: string): string | null { + if (!WINDOWS_CMD_SAFE_PATH.test(scriptPath) || !WINDOWS_DRIVE_LETTER_PATH.test(scriptPath)) { + return null + } + // Why: forward slashes are the one separator both hosts read, and no token here is a switch + // MSYS can rewrite — a literal `cmd.exe /d /c ` does not survive Git Bash (measured). + const invocation = scriptPath.replaceAll('\\', '/') + // Why: neutral JSON when the script is missing (#14818), with no interpreter to Test-Path with. + // Valid in bash and cmd.exe; PowerShell 5.1 rejects `||`, which is what gates this on Git Bash. + // It also fires when cmd.exe itself exits non-zero (a failing AutoRun), printing `{}` twice — + // on that same box the encoded launcher exited 1 instead, so neither shape is clean there. + // Stderr stays unredirected: `2>nul` writes a literal `nul` file into the cwd under MSYS. + return `${invocation} || echo {}` +} diff --git a/src/main/agent-hooks/windows-hook-payload-delivery.test.ts b/src/main/agent-hooks/windows-hook-payload-delivery.test.ts index 22103b178ff..79d9f4f60ae 100644 --- a/src/main/agent-hooks/windows-hook-payload-delivery.test.ts +++ b/src/main/agent-hooks/windows-hook-payload-delivery.test.ts @@ -7,7 +7,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { spawn } from 'node:child_process' import { createServer, type Server } from 'node:http' -import { mkdtempSync, readFileSync } from 'node:fs' +import { mkdtempSync, readFileSync, writeFileSync } from 'node:fs' import { removeTreeSync } from '../../shared/windows-transient-lock-removal' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -32,6 +32,7 @@ vi.mock('os', async (importOriginal) => { }) import { ClaudeHookService } from '../claude/hook-service' +import { WINDOWS_CMD_SAFE_PATH } from './installer-utils' import { getConfigPath, getWindowsManagedLifecycleHook } from '../claude/hook-settings' import { findGitBash } from './windows-git-bash-path.test-fixture' @@ -122,6 +123,15 @@ function runHookCommand( }) } +// Why: a developer box may set HKCU\...\Command Processor\AutoRun, which cmd.exe runs before +// any .cmd — and MSYS spawns a .cmd without `/d`, so it fires on the Git Bash leg. Redirecting +// USERPROFILE to a temp home makes the usual `%USERPROFILE%\.cmd_aliases.cmd` target vanish, and +// cmd's "not recognized" lands on the hook's stderr. Seed an empty target so this suite measures +// the launcher rather than the host's shell configuration. +function seedCmdAutoRunTarget(home: string): void { + writeFileSync(join(home, '.cmd_aliases.cmd'), '@echo off\r\n', 'utf8') +} + function hookEnvironment(extra: NodeJS.ProcessEnv): NodeJS.ProcessEnv { const base = Object.fromEntries( Object.entries(process.env).filter(([key]) => !key.startsWith('ORCA_')) @@ -158,6 +168,7 @@ describe.skipIf(process.platform !== 'win32')('Windows managed hook payload deli it('delivers the piped payload to the hook listener through cmd.exe and Git Bash', async () => { home = mkdtempSync(join(tmpdir(), 'orca-hook-payload-')) homedirMock.mockReturnValue(home) + seedCmdAutoRunTarget(home) expect(new ClaudeHookService().install().state).toBe('installed') const settings = JSON.parse(readFileSync(getConfigPath(), 'utf8')) as { @@ -166,6 +177,11 @@ describe.skipIf(process.platform !== 'win32')('Windows managed hook payload deli // Why: assert nothing about the launcher's shape here — this test's whole value is // that it fails for any launcher that loses the payload, named conhost or not. const registeredCommand = settings.hooks.PreToolUse[0].hooks[0].command + // ...with one exception: a cmd-safe profile must reach the script with no interpreter in + // front of it, or #18875's per-event PowerShell start-up has quietly come back. + if (WINDOWS_CMD_SAFE_PATH.test(join(home, '.orca', 'agent-hooks', 'claude-hook.cmd'))) { + expect(registeredCommand).not.toMatch(/powershell|-EncodedCommand/i) + } const listener = await startHookListener() server = listener.server diff --git a/src/main/agent-hooks/windows-powershell-hook-launcher.ts b/src/main/agent-hooks/windows-powershell-hook-launcher.ts index b9a9f6dd208..2cdb8c0f3fa 100644 --- a/src/main/agent-hooks/windows-powershell-hook-launcher.ts +++ b/src/main/agent-hooks/windows-powershell-hook-launcher.ts @@ -39,6 +39,11 @@ export function getWindowsPowerShellExecutablePath(): string { * Do not restore the flag to fix a console report. That trades every hook on an * AV host for a flicker. The answer is to shorten the interpreter chain — the * shipped doctrine of #15520 and #15595 — or a launcher that owns no console. + * + * #18875 took that answer for the Claude lifecycle hook, which now registers the + * managed `.cmd` path directly (`windows-direct-cmd-hook-command.ts`) and reaches + * this launcher only when the profile path is not cmd-safe or Git Bash is not + * resolvable. Every other caller still comes through here on every event. */ export const WINDOWS_POWERSHELL_HOOK_SWITCHES = '-NoProfile' diff --git a/src/main/claude/hook-service.test.ts b/src/main/claude/hook-service.test.ts index e2937015bea..a4e48c98120 100644 --- a/src/main/claude/hook-service.test.ts +++ b/src/main/claude/hook-service.test.ts @@ -7,6 +7,7 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync import { tmpdir } from 'node:os' import { join } from 'node:path' import { vi, describe, expect, it } from 'vitest' +import type * as GitBashModule from '../git-bash' vi.mock('electron', () => ({ app: { @@ -14,11 +15,23 @@ vi.mock('electron', () => ({ } })) +// Why: the installed hook shape depends on whether Git Bash is resolvable on the host, so the +// install assertions below have to state which host they describe rather than inherit the box's. +const { gitBashAvailableMock } = vi.hoisted(() => ({ gitBashAvailableMock: { value: true } })) +vi.mock('../git-bash', async (importOriginal) => ({ + ...(await importOriginal()), + isGitBashAvailable: () => gitBashAvailableMock.value +})) + import type { SFTPWrapper } from 'ssh2' -import { createManagedCommandMatcher } from '../agent-hooks/installer-utils' +import { createManagedCommandMatcher, WINDOWS_CMD_SAFE_PATH } from '../agent-hooks/installer-utils' import { WINDOWS_HOOK_STDIN_DRAIN_LABEL } from '../agent-hooks/hook-stdin-contract' import { ClaudeHookService } from './hook-service' -import { getWindowsManagedLifecycleHook, OPENCLAUDE_HOOK_SETTINGS } from './hook-settings' +import { + CLAUDE_EVENTS, + getWindowsManagedLifecycleHook, + OPENCLAUDE_HOOK_SETTINGS +} from './hook-settings' const CLAUDE_SCRIPT_FILE_NAME = process.platform === 'win32' ? 'claude-hook.cmd' : 'claude-hook.sh' const STATUSLINE_SCRIPT_FILE_NAME = @@ -35,14 +48,29 @@ function hasManagedCommand(hook: TestHook, matcher: (command: string | undefined } describe('getWindowsManagedLifecycleHook', () => { - it('resolves the managed script from the runtime Windows profile, as a single command string', () => { - const scriptPath = 'C:\\Users\\%name%\\a^b&c\\.orca\\agent-hooks\\claude-hook.cmd' - const hook = getWindowsManagedLifecycleHook(scriptPath) + const SAFE_SCRIPT_PATH = 'C:\\Users\\alice\\.orca\\agent-hooks\\claude-hook.cmd' + const UNSAFE_SCRIPT_PATH = 'C:\\Users\\%name%\\a^b&c\\.orca\\agent-hooks\\claude-hook.cmd' + + it('registers the script itself, with no interpreter in front of it (#18875)', () => { + // Why this is the whole point: the encoded launcher spent a PowerShell start-up per hook + // event (471ms vs 201ms measured) before the .cmd could reach its ORCA_PANE_KEY guard, and + // its orphan outlived the hook's timeout kill still holding the stdout the agent reads. + const hook = getWindowsManagedLifecycleHook(SAFE_SCRIPT_PATH, { gitBashAvailable: true }) + + expect(hook.args).toBeUndefined() + expect(hook.command).toBe('C:/Users/alice/.orca/agent-hooks/claude-hook.cmd || echo {}') + expect(hook.command).not.toMatch(/powershell|-EncodedCommand|conhost/i) + // Why: Git Bash/MSYS mangles backslash paths and rewrites slash-prefixed switches. + expect(hook.command).not.toMatch(/\\/) + expect(hook.command).not.toMatch(/ \/[a-zA-Z]+( |$)/) + }) + + it('falls back to the encoded launcher when the profile path is not cmd-safe', () => { + const hook = getWindowsManagedLifecycleHook(UNSAFE_SCRIPT_PATH, { gitBashAvailable: true }) expect(hook.args).toBeUndefined() expect(hook.command).toMatch(/\/powershell\.exe -NoProfile -EncodedCommand /) - expect(hook.command).not.toContain(scriptPath) - // Why: Git Bash/MSYS mangles backslash paths and slash-prefixed switches. + expect(hook.command).not.toContain(UNSAFE_SCRIPT_PATH) expect(hook.command.replace(/-EncodedCommand \S+$/, '')).not.toMatch(/\\| \/[a-zA-Z]+( |$)/) const encoded = hook.command.match(/-EncodedCommand (\S+)$/)?.[1] @@ -51,10 +79,21 @@ describe('getWindowsManagedLifecycleHook', () => { expect(decoded).toContain('.orca\\agent-hooks\\claude-hook.cmd') }) + it('falls back to the encoded launcher when Git Bash is not resolvable', () => { + // Why: without Git Bash, Claude Code hosts the hook in PowerShell, and PowerShell 5.1 + // rejects `||` as a statement separator (measured) — every event would be a parse error. + const hook = getWindowsManagedLifecycleHook(SAFE_SCRIPT_PATH, { gitBashAvailable: false }) + + expect(hook.command).toMatch(/\/powershell\.exe -NoProfile -EncodedCommand /) + }) + it('is still recognized as managed by createManagedCommandMatcher (#14825)', () => { - const scriptPath = 'C:\\Users\\alice\\.orca\\agent-hooks\\claude-hook.cmd' - const hook = getWindowsManagedLifecycleHook(scriptPath) - expect(isClaudeManagedCommand(hook.command)).toBe(true) + for (const hook of [ + getWindowsManagedLifecycleHook(SAFE_SCRIPT_PATH, { gitBashAvailable: true }), + getWindowsManagedLifecycleHook(SAFE_SCRIPT_PATH, { gitBashAvailable: false }) + ]) { + expect(isClaudeManagedCommand(hook.command)).toBe(true) + } }) }) @@ -200,7 +239,13 @@ describe('ClaudeHookService.install', () => { const managedHook = legacyHooks.find((hook: TestHook) => hasManagedCommand(hook, isClaudeManagedCommand) ) - expect(JSON.stringify(managedHook)).not.toContain(tmpHome.replaceAll('\\', '/')) + // Why: POSIX resolves the profile at runtime (`${HOME-}`, STA-3348). Windows cannot — + // no single token expands in both Git Bash and cmd.exe — so it registers the absolute + // path, as Codex/Grok/Devin/Antigravity already do (#18875). A moved profile is caught + // by getStatus's exact match and rewritten, and `|| echo {}` keeps a stale entry neutral. + if (process.platform !== 'win32') { + expect(JSON.stringify(managedHook)).not.toContain(tmpHome.replaceAll('\\', '/')) + } expect( legacyHooks.some((hook: TestHook) => hasManagedCommand(hook, isClaudeManagedCommand)) ).toBe(true) @@ -365,7 +410,7 @@ describe('ClaudeHookService.install', () => { }) it.skipIf(process.platform !== 'win32')( - 'runs portable managed hooks through a single headless command string', + 'pins the encoded-launcher fallback for a profile path the shells cannot carry bare', () => { const tmpHome = mkdtempSync(join(tmpdir(), 'orca claude home with spaces ')) vi.stubEnv('HOME', tmpHome) @@ -397,6 +442,137 @@ describe('ClaudeHookService.install', () => { } ) + it.skipIf(process.platform !== 'win32')( + 'installs the bare script path on every event when the profile path is cmd-safe (#18875)', + () => { + const tmpHome = mkdtempSync(join(tmpdir(), 'orca-claude-direct-')) + vi.stubEnv('HOME', tmpHome) + vi.stubEnv('USERPROFILE', tmpHome) + const scriptPath = join(tmpHome, '.orca', 'agent-hooks', CLAUDE_SCRIPT_FILE_NAME) + // Why: a runner whose tmpdir carries a space (a profile-scoped TEMP) belongs to the + // fallback case above, not this one; skip rather than assert the wrong contract. + if (!WINDOWS_CMD_SAFE_PATH.test(scriptPath)) { + vi.unstubAllEnvs() + rmSync(tmpHome, { recursive: true, force: true }) + return + } + try { + expect(new ClaudeHookService().install().state).toBe('installed') + + const settings = JSON.parse( + readFileSync(join(tmpHome, '.claude', 'settings.json'), 'utf-8') + ) as { hooks: Record } + + const expected = `${scriptPath.replaceAll('\\', '/')} || echo {}` + for (const { eventName } of CLAUDE_EVENTS) { + const hook = settings.hooks[eventName]?.[0]?.hooks?.[0] + expect(hook?.args, eventName).toBeUndefined() + expect(hook?.command, eventName).toBe(expected) + } + // Why: the whole point of #18875 — no interpreter is started to reach the script. + expect(JSON.stringify(settings.hooks)).not.toMatch(/powershell|EncodedCommand/i) + expect(new ClaudeHookService().getStatus().state).toBe('installed') + } finally { + vi.unstubAllEnvs() + rmSync(tmpHome, { recursive: true, force: true }) + } + } + ) + + it.skipIf(process.platform !== 'win32')( + 'sweeps a previously installed encoded launcher on reinstall, keeping user hooks', + () => { + const tmpHome = mkdtempSync(join(tmpdir(), 'orca-claude-migrate-')) + vi.stubEnv('HOME', tmpHome) + vi.stubEnv('USERPROFILE', tmpHome) + const scriptPath = join(tmpHome, '.orca', 'agent-hooks', CLAUDE_SCRIPT_FILE_NAME) + if (!WINDOWS_CMD_SAFE_PATH.test(scriptPath)) { + vi.unstubAllEnvs() + rmSync(tmpHome, { recursive: true, force: true }) + return + } + try { + const settingsPath = join(tmpHome, '.claude', 'settings.json') + mkdirSync(join(tmpHome, '.claude'), { recursive: true }) + const stale = getWindowsManagedLifecycleHook(scriptPath, { gitBashAvailable: false }) + writeFileSync( + settingsPath, + JSON.stringify({ + hooks: { + Stop: [{ hooks: [stale] }], + PreToolUse: [{ matcher: '*', hooks: [stale] }], + UserPromptSubmit: [{ hooks: [{ type: 'command', command: 'echo mine' }] }] + } + }), + 'utf-8' + ) + + expect(new ClaudeHookService().install().state).toBe('installed') + + const settings = JSON.parse(readFileSync(settingsPath, 'utf-8')) as { + hooks: Record + } + expect(JSON.stringify(settings.hooks)).not.toContain('-EncodedCommand') + expect( + settings.hooks.UserPromptSubmit.some((definition) => + definition.hooks.some((hook) => hook.command === 'echo mine') + ) + ).toBe(true) + } finally { + vi.unstubAllEnvs() + rmSync(tmpHome, { recursive: true, force: true }) + } + } + ) + + it.skipIf(process.platform !== 'win32')( + 'reports a stale absolute path as not_installed and rewrites it on install (#18875)', + () => { + // Why: the direct shape bakes the profile path in, where the encoded launcher resolved + // %USERPROFILE% at run time (STA-3348). That is only safe because a moved profile is + // caught here and rewritten, so this is the test that carries the replaced contract. + const tmpHome = mkdtempSync(join(tmpdir(), 'orca-claude-moved-')) + vi.stubEnv('HOME', tmpHome) + vi.stubEnv('USERPROFILE', tmpHome) + const scriptPath = join(tmpHome, '.orca', 'agent-hooks', CLAUDE_SCRIPT_FILE_NAME) + if (!WINDOWS_CMD_SAFE_PATH.test(scriptPath)) { + vi.unstubAllEnvs() + rmSync(tmpHome, { recursive: true, force: true }) + return + } + try { + const settingsPath = join(tmpHome, '.claude', 'settings.json') + mkdirSync(join(tmpHome, '.claude'), { recursive: true }) + const staleCommand = 'C:/Users/someone-else/.orca/agent-hooks/claude-hook.cmd || echo {}' + const stale = { type: 'command', command: staleCommand, timeout: 10 } + writeFileSync( + settingsPath, + JSON.stringify({ + hooks: Object.fromEntries( + CLAUDE_EVENTS.map(({ eventName }) => [eventName, [{ hooks: [stale] }]]) + ) + }), + 'utf-8' + ) + + expect(new ClaudeHookService().getStatus().state).toBe('not_installed') + expect(new ClaudeHookService().install().state).toBe('installed') + + const settings = JSON.parse(readFileSync(settingsPath, 'utf-8')) as { + hooks: Record + } + expect(JSON.stringify(settings.hooks)).not.toContain('someone-else') + expect(settings.hooks.PreToolUse[0].hooks[0].command).toBe( + `${scriptPath.replaceAll('\\', '/')} || echo {}` + ) + expect(new ClaudeHookService().getStatus().state).toBe('installed') + } finally { + vi.unstubAllEnvs() + rmSync(tmpHome, { recursive: true, force: true }) + } + } + ) + it.skipIf(process.platform !== 'win32')( 'posts from the managed .cmd via curl.exe, not a second PowerShell', () => { diff --git a/src/main/claude/hook-settings.ts b/src/main/claude/hook-settings.ts index c6cf3a9b53c..047fcbb26b6 100644 --- a/src/main/claude/hook-settings.ts +++ b/src/main/claude/hook-settings.ts @@ -14,23 +14,25 @@ import { type HooksConfig } from '../agent-hooks/installer-utils' import { wrapRuntimeHomeHookCommand } from '../agent-hooks/runtime-home-hook-command' +import { wrapWindowsDirectCmdHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' +import { isGitBashAvailable } from '../git-bash' export type ClaudeCompatibleHookSettings = { configDirName: '.claude' | '.openclaude' scriptBaseName: 'claude-hook' | 'openclaude-hook' - usesWindowsPowerShellLauncher: boolean + usesWindowsCompatLauncher: boolean } export const CLAUDE_HOOK_SETTINGS: ClaudeCompatibleHookSettings = { configDirName: '.claude', scriptBaseName: 'claude-hook', - usesWindowsPowerShellLauncher: true + usesWindowsCompatLauncher: true } export const OPENCLAUDE_HOOK_SETTINGS: ClaudeCompatibleHookSettings = { configDirName: '.openclaude', scriptBaseName: 'openclaude-hook', - usesWindowsPowerShellLauncher: false + usesWindowsCompatLauncher: false } export const CLAUDE_EVENTS = [ @@ -153,16 +155,31 @@ export function getManagedCommand( export function getManagedLifecycleHook( scriptPath: string, - settings = CLAUDE_HOOK_SETTINGS + settings = CLAUDE_HOOK_SETTINGS, + options: WindowsManagedLifecycleHookOptions = {} ): HookCommandConfig { - if (process.platform !== 'win32' || !settings.usesWindowsPowerShellLauncher) { + if (process.platform !== 'win32' || !settings.usesWindowsCompatLauncher) { return buildManagedCommandHook(getManagedCommand(scriptPath, { neutralJsonWhenMissing: true })) } - return getWindowsManagedLifecycleHook(scriptPath) + return getWindowsManagedLifecycleHook(scriptPath, options) } +export type WindowsManagedLifecycleHookOptions = { gitBashAvailable?: boolean } + // Why: some Claude-compatible consumers ignore `args`, so the invocation must be self-contained. -export function getWindowsManagedLifecycleHook(scriptPath: string): HookCommandConfig { +export function getWindowsManagedLifecycleHook( + scriptPath: string, + options: WindowsManagedLifecycleHookOptions = {} +): HookCommandConfig { + // Why (#18875): the encoded launcher cost a PowerShell start-up per hook event. Take the direct + // path only where the host can parse `||` — Git Bash can, Windows PowerShell 5.1 cannot. + const directCommand = + (options.gitBashAvailable ?? isGitBashAvailable()) + ? wrapWindowsDirectCmdHookCommand(scriptPath) + : null + if (directCommand) { + return { type: 'command', command: directCommand, timeout: MANAGED_HOOK_TIMEOUT_SECONDS } + } const scriptFileName = win32.basename(scriptPath) // Why: runtime profile resolution keeps the managed entry portable across users (STA-3348). const quotedRelativePath = quotePowerShellString(`.orca\\agent-hooks\\${scriptFileName}`) diff --git a/src/renderer/src/components/dashboard-popout/AgentMapWorkspaceContextMenu.test.tsx b/src/renderer/src/components/dashboard-popout/AgentMapWorkspaceContextMenu.test.tsx index c5477a997e9..bc4958408a2 100644 --- a/src/renderer/src/components/dashboard-popout/AgentMapWorkspaceContextMenu.test.tsx +++ b/src/renderer/src/components/dashboard-popout/AgentMapWorkspaceContextMenu.test.tsx @@ -314,7 +314,19 @@ describe('Agent Map workspace context menu', () => { clientX: 100, clientY: 110 }) - fireEvent.click(await screen.findByText('Create new worktree for Orca', {}, { timeout: 5_000 })) + const createWorktree = await screen.findByText( + 'Create new worktree for Orca', + {}, + { timeout: 5_000 } + ) + // Radix restores focus after unmount; drain it before the next test opens a menu. + const focusRestored = new Promise((resolve) => { + screen + .getByRole('menu') + .addEventListener('focusScope.autoFocusOnUnmount', () => resolve(), { once: true }) + }) + fireEvent.click(createWorktree) + await act(async () => focusRestored) expect(useAppStore.getState().activeModal).toBe('new-workspace-composer') expect(useAppStore.getState().modalData).toEqual({