From 4788d5191e66cc27f893e8708e6ac983fad7fce4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:45:39 -0700 Subject: [PATCH 01/10] fix(codex): preserve user trust during real-home cleanup --- config/tsconfig.cli.json | 4 + .../agent-hooks/hook-config-write-path.ts | 19 + src/main/agent-hooks/installer-utils.test.ts | 23 ++ src/main/agent-hooks/installer-utils.ts | 16 +- src/main/codex/codex-app-server-client.ts | 340 ++++-------------- .../codex/codex-app-server-grant-bridge.ts | 24 +- .../codex/codex-app-server-grant-entry.ts | 21 +- .../codex/codex-app-server-grant-envelope.ts | 17 +- src/main/codex/codex-app-server-session.ts | 221 ++++++++++++ .../codex-real-home-hook-install.test.ts | 40 +++ .../codex/codex-real-home-hook-install.ts | 137 +++---- ...odex-user-hook-trust-rebase-client.test.ts | 112 ++++++ .../codex-user-hook-trust-rebase-client.ts | 163 +++++++++ .../codex-user-hook-trust-rebase.test.ts | 185 ++++++++++ .../codex/codex-user-hook-trust-rebase.ts | 163 +++++++++ src/main/codex/config-toml-trust.test.ts | 40 +++ src/main/codex/config-toml-trust.ts | 23 +- .../codex/hook-service-trust-grant.test.ts | 62 +++- src/main/codex/hook-service.ts | 28 +- 19 files changed, 1274 insertions(+), 364 deletions(-) create mode 100644 src/main/agent-hooks/hook-config-write-path.ts create mode 100644 src/main/codex/codex-app-server-session.ts create mode 100644 src/main/codex/codex-user-hook-trust-rebase-client.test.ts create mode 100644 src/main/codex/codex-user-hook-trust-rebase-client.ts create mode 100644 src/main/codex/codex-user-hook-trust-rebase.test.ts create mode 100644 src/main/codex/codex-user-hook-trust-rebase.ts diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index a4df2dab4a9..c97f1f4d805 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -4,6 +4,7 @@ "../src/cli/**/*", "../src/shared/**/*", "../src/main/agent-hooks/hook-stdin-contract.ts", + "../src/main/agent-hooks/hook-config-write-path.ts", "../src/main/agent-hooks/installer-utils.ts", "../src/main/agent-hooks/installer-utils-remote.ts", "../src/main/agent-hooks/managed-agent-hook-controls.ts", @@ -15,6 +16,7 @@ "../src/main/codex/codex-app-server-client.ts", "../src/main/codex/codex-app-server-grant-bridge.ts", "../src/main/codex/codex-app-server-grant-envelope.ts", + "../src/main/codex/codex-app-server-session.ts", "../src/main/codex/codex-config-mirror.ts", "../src/main/codex/codex-config-path-reference-rewrite.ts", "../src/main/codex/codex-home-paths.ts", @@ -25,6 +27,8 @@ "../src/main/codex/codex-trust-config-rollback.ts", "../src/main/codex/codex-trust-grant-host.ts", "../src/main/codex/codex-trust-grant-ledger.ts", + "../src/main/codex/codex-user-hook-trust-rebase-client.ts", + "../src/main/codex/codex-user-hook-trust-rebase.ts", "../src/main/codex/codex-wsl-hook-install-plan.ts", "../src/main/codex/config-settings-promotion.ts", "../src/main/codex/config-toml-line-scan.ts", diff --git a/src/main/agent-hooks/hook-config-write-path.ts b/src/main/agent-hooks/hook-config-write-path.ts new file mode 100644 index 00000000000..9feb348c8b1 --- /dev/null +++ b/src/main/agent-hooks/hook-config-write-path.ts @@ -0,0 +1,19 @@ +import { lstatSync, realpathSync } from 'node:fs' + +export function resolveHooksJsonWritePath(configPath: string): string { + let isSymlink = false + try { + isSymlink = lstatSync(configPath).isSymbolicLink() + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error + } + return configPath + } + if (isSymlink) { + // Why: atomic rename on the link path disconnects dotfiles-managed hook + // configs. A dangling link must fail closed rather than be replaced. + return realpathSync.native(configPath) + } + return configPath +} diff --git a/src/main/agent-hooks/installer-utils.test.ts b/src/main/agent-hooks/installer-utils.test.ts index 6883308be6f..915e7ad6bdb 100644 --- a/src/main/agent-hooks/installer-utils.test.ts +++ b/src/main/agent-hooks/installer-utils.test.ts @@ -1,12 +1,14 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { existsSync, + lstatSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, statSync, + symlinkSync, writeFileSync, chmodSync } from 'node:fs' @@ -42,6 +44,27 @@ afterEach(() => { }) describe('writeHooksJson', () => { + it('updates a symlink target without replacing the hook config link', () => { + const targetPath = join(tmpDir, 'dotfiles-hooks.json') + writeFileSync(targetPath, '{"hooks":{}}\n') + symlinkSync(targetPath, configPath) + + writeHooksJson(configPath, { hooks: { Stop: [] } }) + + expect(lstatSync(configPath).isSymbolicLink()).toBe(true) + expect(JSON.parse(readFileSync(targetPath, 'utf-8'))).toEqual({ hooks: { Stop: [] } }) + }) + + it('does not replace a dangling hook config symlink', () => { + const targetPath = join(tmpDir, 'missing-dotfiles-hooks.json') + symlinkSync(targetPath, configPath) + + expect(() => writeHooksJson(configPath, { hooks: { Stop: [] } })).toThrow() + + expect(lstatSync(configPath).isSymbolicLink()).toBe(true) + expect(existsSync(targetPath)).toBe(false) + }) + it('writes the config as formatted JSON', () => { const config: HooksConfig = { hooks: { Stop: [{ hooks: [{ type: 'command', command: 'foo' }] }] } diff --git a/src/main/agent-hooks/installer-utils.ts b/src/main/agent-hooks/installer-utils.ts index d9e36edcbfd..8a99affaddb 100644 --- a/src/main/agent-hooks/installer-utils.ts +++ b/src/main/agent-hooks/installer-utils.ts @@ -15,6 +15,7 @@ import { randomUUID } from 'node:crypto' import type { AgentHookSource } from '../../shared/agent-hook-relay' import { grantDirAcl, isPermissionError } from '../win32-utils' import { POSIX_HOOK_STDIN_DRAIN_COMMAND } from './hook-stdin-contract' +import { resolveHooksJsonWritePath } from './hook-config-write-path' export type HookCommandConfig = { type: 'command' @@ -359,7 +360,8 @@ export function writeHooksJson( config: HooksConfig, options?: { preserveMode?: boolean } ): void { - const dir = dirname(configPath) + const writePath = resolveHooksJsonWritePath(configPath) + const dir = dirname(writePath) mkdirSync(dir, { recursive: true }) // Why: write to a temp file then rename so a crash or disk-full mid-write @@ -374,15 +376,15 @@ export function writeHooksJson( const tmpPath = join(dir, `.${Date.now()}-${randomUUID()}.tmp`) const serialized = `${JSON.stringify(config, null, 2)}\n` const existingMode = - options?.preserveMode === true && existsSync(configPath) ? statSync(configPath).mode : undefined + options?.preserveMode === true && existsSync(writePath) ? statSync(writePath).mode : undefined // Why: skip the write (and therefore the .bak rotation) when the on-disk // content is already identical. Without this, every install() rewrites the // file and rolls the backup forward, which can silently destroy the last // recoverable copy if install() is called repeatedly (e.g. on app start). - if (existsSync(configPath)) { + if (existsSync(writePath)) { try { - if (readFileSync(configPath, 'utf-8') === serialized) { + if (readFileSync(writePath, 'utf-8') === serialized) { return } } catch { @@ -397,10 +399,10 @@ export function writeHooksJson( // Why: single rolling backup — one file, no accumulation in ~/.claude. // Protects against a merge-logic bug producing bad JSON; the original is // always recoverable from .bak until the next write. - if (existsSync(configPath)) { - copyFileSync(configPath, `${configPath}.bak`) + if (existsSync(writePath)) { + copyFileSync(writePath, `${writePath}.bak`) } - renameSync(tmpPath, configPath) + renameSync(tmpPath, writePath) } finally { // Clean up temp file if rename failed. if (existsSync(tmpPath)) { diff --git a/src/main/codex/codex-app-server-client.ts b/src/main/codex/codex-app-server-client.ts index ab5dced425c..3fd8e538a9f 100644 --- a/src/main/codex/codex-app-server-client.ts +++ b/src/main/codex/codex-app-server-client.ts @@ -1,6 +1,13 @@ -import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' +import { spawn } from 'node:child_process' import { normalizeHookTrustKeyForLookup } from './config-toml-trust' -import { waitForProcessExitUntil } from './codex-process-exit-deadline' +import { runCodexAppServerSession, type CodexAppServerInvocation } from './codex-app-server-session' + +export { + CodexAppServerTimeoutError, + CodexAppServerUnsupportedError, + isCodexAppServerUnsupportedError +} from './codex-app-server-session' +export type { CodexAppServerInvocation } from './codex-app-server-session' // Why: Codex gates hooks on a `trusted_hash` it computes from a private // canonical-JSON identity. Orca used to replicate that algorithm @@ -12,15 +19,6 @@ import { waitForProcessExitUntil } from './codex-process-exit-deadline' // Codex as the only hash authority. See upstream codex-rs/tui/src/hooks_rpc.rs // and codex-rs/tui/src/startup_hooks_review.rs. -export type CodexAppServerInvocation = { - command: string - args: string[] - /** Overlay applied on top of the inherited environment (e.g. CODEX_HOME). */ - env?: Record - /** Whole-session deadline. The codex child is SIGKILLed when it lapses. */ - timeoutMs: number -} - export type CodexHookTrustGrantRequest = { invocation: CodexAppServerInvocation /** cwd passed to hooks/list. Discovery of the managed CODEX_HOME's @@ -52,55 +50,15 @@ export type CodexHookTrustGrantSessionResult = } | { outcome: 'verify-failed'; reason: string } -/** Codex-side absence of the trust-grant RPC surface (old CLI without the - * app-server subcommand, or a server without hooks/list / config/batchWrite). - * This is the ONLY error class the capability cache marks unsupported. */ -export class CodexAppServerUnsupportedError extends Error { - constructor(message: string) { - super(message) - this.name = 'CodexAppServerUnsupportedError' - } -} - -export class CodexAppServerTimeoutError extends Error { - constructor(message: string) { - super(message) - this.name = 'CodexAppServerTimeoutError' - } -} - -export function isCodexAppServerUnsupportedError(error: unknown): boolean { - return error instanceof Error && error.name === 'CodexAppServerUnsupportedError' -} - -type JsonRpcResponse = { - id?: number - result?: unknown - error?: { code?: number; message?: string } -} - -type CodexHookListing = { +export type CodexHookListing = { key: string command: string | null currentHash: string trustStatus: string + enabled: boolean } -const JSON_RPC_METHOD_NOT_FOUND = -32601 -const STDERR_TAIL_MAX_BYTES = 8192 -const STDOUT_LINE_MAX_BYTES = 1024 * 1024 - -function isMethodNotFoundError(error: { code?: number; message?: string }): boolean { - return error.code === JSON_RPC_METHOD_NOT_FOUND || /method not found/i.test(error.message ?? '') -} - -// Why: a CLI predating the app-server subcommand fails argv parsing before -// speaking any JSON-RPC; that shape is a capability signal, not a transient. -function stderrIndicatesMissingAppServer(stderrTail: string): boolean { - return /unrecognized subcommand|unexpected argument|invalid subcommand/i.test(stderrTail) -} - -function collectHookListings(result: unknown): CodexHookListing[] { +export function collectCodexHookListings(result: unknown): CodexHookListing[] { const data = result && typeof result === 'object' && Array.isArray((result as { data?: unknown }).data) ? ((result as { data: unknown[] }).data as { hooks?: unknown }[]) @@ -127,7 +85,8 @@ function collectHookListings(result: unknown): CodexHookListing[] { key: hook.key, command: typeof hook.command === 'string' ? hook.command : null, currentHash: hook.currentHash, - trustStatus: hook.trustStatus + trustStatus: hook.trustStatus, + enabled: typeof hook.enabled === 'boolean' ? hook.enabled : true }) } } @@ -144,232 +103,59 @@ export async function runCodexHookTrustGrantSession( request: CodexHookTrustGrantRequest, spawnImpl: typeof spawn = spawn ): Promise { - const { invocation } = request - const child = spawnImpl(invocation.command, invocation.args, { - env: { ...process.env, ...invocation.env }, - stdio: ['pipe', 'pipe', 'pipe'], - windowsHide: true - }) as ChildProcessWithoutNullStreams + return runCodexAppServerSession( + request.invocation, + async (requestRpc) => { + const expectedKeys = new Set(request.expectedTrustKeys) + const matchManaged = (listing: CodexHookListing): boolean => + listing.command === request.managedCommand && + expectedKeys.has(normalizeHookTrustKeyForLookup(listing.key)) - let stderrTail = '' - let exited = false - let nextRequestId = 1 - let timedOut = false - const pending = new Map< - number, - { resolve: (r: JsonRpcResponse) => void; reject: (e: Error) => void } - >() - - const exitPromise = new Promise((resolve) => { - child.on('exit', () => { - exited = true - resolve() - }) - }) - // Why: 'error' fires instead of 'exit' when the spawn itself fails - // (ENOENT); surface it to every in-flight request or they wait forever. - let spawnError: Error | null = null - child.on('error', (error) => { - spawnError = error - exited = true - failPending(error) - }) - // Why: 'close' (not 'exit') guarantees the stderr tail is complete, so an - // early death classifies correctly as missing-subcommand vs transient. - child.on('close', () => { - failPending(buildEarlyExitError()) - }) - // Why: JSONL can contain non-ASCII hook paths. Stream decoding must retain a - // multibyte character split across pipe chunks or the response becomes invalid JSON. - child.stderr.setEncoding('utf8').on('data', (chunk: string) => { - stderrTail = (stderrTail + chunk).slice(-STDERR_TAIL_MAX_BYTES) - }) - // Why: a child can exit between the liveness check and stdin.write(); an - // EPIPE must reject the RPC instead of becoming an unhandled stream error. - child.stdin.on('error', (error) => { - failPending(error) - }) - - let stdoutBuffer = '' - child.stdout.setEncoding('utf8').on('data', (chunk: string) => { - stdoutBuffer += chunk - if (Buffer.byteLength(stdoutBuffer) > STDOUT_LINE_MAX_BYTES) { - child.kill('SIGKILL') - failPending(new Error('codex app-server emitted an oversized JSONL response')) - return - } - let newlineIndex - while ((newlineIndex = stdoutBuffer.indexOf('\n')) !== -1) { - const line = stdoutBuffer.slice(0, newlineIndex).trim() - stdoutBuffer = stdoutBuffer.slice(newlineIndex + 1) - if (!line) { - continue + const listResult = await requestRpc('hooks/list', { cwds: [request.hooksListCwd] }) + const managedListings = collectCodexHookListings(listResult).filter(matchManaged) + if (managedListings.length !== expectedKeys.size) { + return { + outcome: 'verify-failed', + reason: `hooks/list reported ${managedListings.length} of ${expectedKeys.size} expected managed entries` + } } - let message: JsonRpcResponse - try { - message = JSON.parse(line) as JsonRpcResponse - } catch { - continue + + const needingTrust = managedListings.filter((listing) => listing.trustStatus !== 'trusted') + if (needingTrust.length > 0) { + // Why: same wire shape as the Codex TUI "Trust all" flow — one upsert + // edit under hooks.state with each key's Codex-computed current hash. + const value: Record = {} + for (const listing of needingTrust) { + value[listing.key] = { trusted_hash: listing.currentHash } + } + await requestRpc('config/batchWrite', { + edits: [{ keyPath: 'hooks.state', value, mergeStrategy: 'upsert' }], + reloadUserConfig: true + }) } - if (typeof message.id === 'number' && pending.has(message.id)) { - const waiter = pending.get(message.id)! - pending.delete(message.id) - waiter.resolve(message) + + const verifyResult = await requestRpc('hooks/list', { cwds: [request.hooksListCwd] }) + const verifiedListings = collectCodexHookListings(verifyResult).filter(matchManaged) + const untrusted = verifiedListings.filter((listing) => listing.trustStatus !== 'trusted') + if (verifiedListings.length !== expectedKeys.size || untrusted.length > 0) { + return { + outcome: 'verify-failed', + reason: + untrusted.length > 0 + ? `post-grant verify left ${untrusted.length} entries ${untrusted[0].trustStatus}` + : `post-grant verify reported ${verifiedListings.length} of ${expectedKeys.size} entries` + } } - } - }) - - function failPending(error: Error): void { - for (const waiter of pending.values()) { - waiter.reject(error) - } - pending.clear() - } - - const deadline = setTimeout(() => { - timedOut = true - child.kill('SIGKILL') - failPending( - new CodexAppServerTimeoutError( - `codex app-server session exceeded ${invocation.timeoutMs}ms (${invocation.command})` - ) - ) - }, invocation.timeoutMs) - - function sendLine(payload: Record): void { - child.stdin.write(`${JSON.stringify(payload)}\n`) - } - - async function requestRpc(method: string, params?: Record): Promise { - if (spawnError) { - throw spawnError - } - if (timedOut) { - throw new CodexAppServerTimeoutError('codex app-server session already timed out') - } - if (exited) { - throw buildEarlyExitError() - } - const id = nextRequestId++ - const response = await new Promise((resolve, reject) => { - pending.set(id, { resolve, reject }) - const payload: Record = { method, id } - if (params !== undefined) { - payload.params = params - } - try { - sendLine(payload) - } catch (error) { - pending.delete(id) - reject(error instanceof Error ? error : new Error(String(error))) - } - }) - if (response.error) { - if (isMethodNotFoundError(response.error)) { - throw new CodexAppServerUnsupportedError( - `codex app-server does not support ${method}: ${response.error.message ?? 'method not found'}` - ) - } - throw new Error( - `codex app-server ${method} failed: ${response.error.message ?? 'unknown error'}` - ) - } - return response.result - } - - function buildEarlyExitError(): Error { - if (stderrIndicatesMissingAppServer(stderrTail)) { - return new CodexAppServerUnsupportedError( - `codex CLI does not support the app-server subcommand: ${stderrTail.trim().slice(0, 400)}` - ) - } - return new Error( - `codex app-server exited before completing the session${stderrTail ? `: ${stderrTail.trim().slice(0, 400)}` : ''}` - ) - } - - try { - await requestRpc('initialize', { - clientInfo: { name: 'orca_desktop', title: 'Orca', version: '0.0.0' } - }) - sendLine({ method: 'initialized' }) - - const expectedKeys = new Set(request.expectedTrustKeys) - const matchManaged = (listing: CodexHookListing): boolean => - listing.command === request.managedCommand && - expectedKeys.has(normalizeHookTrustKeyForLookup(listing.key)) - - const listResult = await requestRpc('hooks/list', { cwds: [request.hooksListCwd] }) - const managedListings = collectHookListings(listResult).filter(matchManaged) - if (managedListings.length !== expectedKeys.size) { return { - outcome: 'verify-failed', - reason: `hooks/list reported ${managedListings.length} of ${expectedKeys.size} expected managed entries` + outcome: 'granted', + wroteTrust: needingTrust.length > 0, + entries: verifiedListings.map((listing) => ({ + key: listing.key, + normalizedKey: normalizeHookTrustKeyForLookup(listing.key), + trustedHash: listing.currentHash + })) } - } - - const needingTrust = managedListings.filter((listing) => listing.trustStatus !== 'trusted') - if (needingTrust.length > 0) { - // Why: same wire shape as the Codex TUI "Trust all" flow — one upsert - // edit under hooks.state with each key's Codex-computed current hash. - const value: Record = {} - for (const listing of needingTrust) { - value[listing.key] = { trusted_hash: listing.currentHash } - } - await requestRpc('config/batchWrite', { - edits: [{ keyPath: 'hooks.state', value, mergeStrategy: 'upsert' }], - reloadUserConfig: true - }) - } - - const verifyResult = await requestRpc('hooks/list', { cwds: [request.hooksListCwd] }) - const verifiedListings = collectHookListings(verifyResult).filter(matchManaged) - const untrusted = verifiedListings.filter((listing) => listing.trustStatus !== 'trusted') - if (verifiedListings.length !== expectedKeys.size || untrusted.length > 0) { - return { - outcome: 'verify-failed', - reason: - untrusted.length > 0 - ? `post-grant verify left ${untrusted.length} entries ${untrusted[0].trustStatus}` - : `post-grant verify reported ${verifiedListings.length} of ${expectedKeys.size} entries` - } - } - return { - outcome: 'granted', - wroteTrust: needingTrust.length > 0, - entries: verifiedListings.map((listing) => ({ - key: listing.key, - normalizedKey: normalizeHookTrustKeyForLookup(listing.key), - trustedHash: listing.currentHash - })) - } - } catch (error) { - if ( - error instanceof Error && - !(error instanceof CodexAppServerUnsupportedError) && - !(error instanceof CodexAppServerTimeoutError) && - stderrIndicatesMissingAppServer(stderrTail) - ) { - throw new CodexAppServerUnsupportedError( - `codex CLI does not support the app-server subcommand: ${stderrTail.trim().slice(0, 400)}` - ) - } - throw error - } finally { - try { - child.stdin.end() - } catch { - // stdin may already be destroyed after a kill; reaping below still runs. - } - if (!exited) { - // Why: the server exits promptly on stdin EOF; the grace period only - // bounds a wedged child before the guaranteed SIGKILL reap. - await waitForProcessExitUntil(exitPromise, 1500) - if (!exited) { - child.kill('SIGKILL') - await waitForProcessExitUntil(exitPromise, 1000) - } - } - clearTimeout(deadline) - } + }, + spawnImpl + ) } diff --git a/src/main/codex/codex-app-server-grant-bridge.ts b/src/main/codex/codex-app-server-grant-bridge.ts index 88f8d6ef80a..c85c4a1c479 100644 --- a/src/main/codex/codex-app-server-grant-bridge.ts +++ b/src/main/codex/codex-app-server-grant-bridge.ts @@ -7,7 +7,15 @@ import { type CodexHookTrustGrantRequest, type CodexHookTrustGrantSessionResult } from './codex-app-server-client' -import type { GrantEntryEnvelope } from './codex-app-server-grant-envelope' +import type { + CodexAppServerEntryRequest, + CodexAppServerEntryResult, + GrantEntryEnvelope +} from './codex-app-server-grant-envelope' +import type { + CodexUserHookTrustRebaseRequest, + CodexUserHookTrustRebaseResult +} from './codex-user-hook-trust-rebase-client' // Why: hook install/refresh is synchronous launch prep — a Codex pane must // not start before its trust is settled — but a stdio JSON-RPC session needs @@ -63,6 +71,20 @@ export function runCodexHookTrustGrantSessionSync( request: CodexHookTrustGrantRequest, options: RunGrantSessionSyncOptions = {} ): CodexHookTrustGrantSessionResult { + return runCodexAppServerEntrySync(request, options) as CodexHookTrustGrantSessionResult +} + +export function runCodexUserHookTrustRebaseSessionSync( + request: CodexUserHookTrustRebaseRequest, + options: RunGrantSessionSyncOptions = {} +): CodexUserHookTrustRebaseResult { + return runCodexAppServerEntrySync(request, options) as CodexUserHookTrustRebaseResult +} + +function runCodexAppServerEntrySync( + request: CodexAppServerEntryRequest, + options: RunGrantSessionSyncOptions +): CodexAppServerEntryResult { const entryPath = options.entryPath ?? resolveCodexGrantEntryPath() if (!entryPath) { throw new Error('codex trust-grant entry bundle not found') diff --git a/src/main/codex/codex-app-server-grant-entry.ts b/src/main/codex/codex-app-server-grant-entry.ts index 6fe201b30c3..8e16892d279 100644 --- a/src/main/codex/codex-app-server-grant-entry.ts +++ b/src/main/codex/codex-app-server-grant-entry.ts @@ -4,12 +4,13 @@ // JSONL RPC session itself needs a live event loop. Reads the request JSON // from stdin, writes a single result-envelope JSON line to stdout, and never // imports electron (see PLAIN_NODE_ENTRY_NAMES in the build guard). -import { buildGrantEntryEnvelope } from './codex-app-server-grant-envelope' -import { writeSync } from 'node:fs' import { - runCodexHookTrustGrantSession, - type CodexHookTrustGrantRequest -} from './codex-app-server-client' + buildGrantEntryEnvelope, + type CodexAppServerEntryRequest +} from './codex-app-server-grant-envelope' +import { writeSync } from 'node:fs' +import { runCodexHookTrustGrantSession } from './codex-app-server-client' +import { runCodexUserHookTrustRebaseSession } from './codex-user-hook-trust-rebase-client' const HARD_EXIT_MARGIN_MS = 2_000 @@ -23,9 +24,9 @@ async function readStdin(): Promise { async function main(): Promise { const raw = await readStdin() - let request: CodexHookTrustGrantRequest + let request: CodexAppServerEntryRequest try { - request = JSON.parse(raw) as CodexHookTrustGrantRequest + request = JSON.parse(raw) as CodexAppServerEntryRequest } catch (error) { process.stdout.write( `${JSON.stringify({ @@ -52,7 +53,11 @@ async function main(): Promise { ) process.exit(3) }, request.invocation.timeoutMs + HARD_EXIT_MARGIN_MS) - const envelope = await buildGrantEntryEnvelope(runCodexHookTrustGrantSession(request)) + const run = + 'operation' in request + ? runCodexUserHookTrustRebaseSession(request) + : runCodexHookTrustGrantSession(request) + const envelope = await buildGrantEntryEnvelope(run) clearTimeout(hardExit) process.stdout.write(`${JSON.stringify(envelope)}\n`) } diff --git a/src/main/codex/codex-app-server-grant-envelope.ts b/src/main/codex/codex-app-server-grant-envelope.ts index a9665b5c2ce..d5b133c2975 100644 --- a/src/main/codex/codex-app-server-grant-envelope.ts +++ b/src/main/codex/codex-app-server-grant-envelope.ts @@ -1,14 +1,27 @@ import { isCodexAppServerUnsupportedError, + type CodexHookTrustGrantRequest, type CodexHookTrustGrantSessionResult } from './codex-app-server-client' +import type { + CodexUserHookTrustRebaseRequest, + CodexUserHookTrustRebaseResult +} from './codex-user-hook-trust-rebase-client' + +export type CodexAppServerEntryRequest = + | CodexHookTrustGrantRequest + | CodexUserHookTrustRebaseRequest + +export type CodexAppServerEntryResult = + | CodexHookTrustGrantSessionResult + | CodexUserHookTrustRebaseResult export type GrantEntryEnvelope = - | { ok: true; result: CodexHookTrustGrantSessionResult } + | { ok: true; result: CodexAppServerEntryResult } | { ok: false; errorName: string; message: string; unsupported?: boolean } export function buildGrantEntryEnvelope( - run: Promise + run: Promise ): Promise { return run.then( (result) => ({ ok: true as const, result }), diff --git a/src/main/codex/codex-app-server-session.ts b/src/main/codex/codex-app-server-session.ts new file mode 100644 index 00000000000..6798792de70 --- /dev/null +++ b/src/main/codex/codex-app-server-session.ts @@ -0,0 +1,221 @@ +import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' +import { waitForProcessExitUntil } from './codex-process-exit-deadline' + +export type CodexAppServerInvocation = { + command: string + args: string[] + /** Overlay applied on top of the inherited environment (for example, CODEX_HOME). */ + env?: Record + /** Whole-session deadline; the Codex child is killed when it lapses. */ + timeoutMs: number +} + +export class CodexAppServerUnsupportedError extends Error { + constructor(message: string) { + super(message) + this.name = 'CodexAppServerUnsupportedError' + } +} + +export class CodexAppServerTimeoutError extends Error { + constructor(message: string) { + super(message) + this.name = 'CodexAppServerTimeoutError' + } +} + +export function isCodexAppServerUnsupportedError(error: unknown): boolean { + return error instanceof Error && error.name === 'CodexAppServerUnsupportedError' +} + +type JsonRpcResponse = { + id?: number + result?: unknown + error?: { code?: number; message?: string } +} + +export type CodexAppServerRequestRpc = ( + method: string, + params?: Record +) => Promise + +const JSON_RPC_METHOD_NOT_FOUND = -32601 +const STDERR_TAIL_MAX_BYTES = 8192 +const STDOUT_LINE_MAX_BYTES = 1024 * 1024 + +function stderrIndicatesMissingAppServer(stderrTail: string): boolean { + return /unrecognized subcommand|unexpected argument|invalid subcommand/i.test(stderrTail) +} + +export async function runCodexAppServerSession( + invocation: CodexAppServerInvocation, + run: (requestRpc: CodexAppServerRequestRpc) => Promise, + spawnImpl: typeof spawn = spawn +): Promise { + const child = spawnImpl(invocation.command, invocation.args, { + env: { ...process.env, ...invocation.env }, + stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true + }) as ChildProcessWithoutNullStreams + + let stderrTail = '' + let stdoutBuffer = '' + let exited = false + let nextRequestId = 1 + let timedOut = false + let spawnError: Error | null = null + const pending = new Map< + number, + { resolve: (response: JsonRpcResponse) => void; reject: (error: Error) => void } + >() + + function buildEarlyExitError(): Error { + if (stderrIndicatesMissingAppServer(stderrTail)) { + return new CodexAppServerUnsupportedError( + `codex CLI does not support the app-server subcommand: ${stderrTail.trim().slice(0, 400)}` + ) + } + return new Error( + `codex app-server exited before completing the session${stderrTail ? `: ${stderrTail.trim().slice(0, 400)}` : ''}` + ) + } + + function failPending(error: Error): void { + for (const waiter of pending.values()) { + waiter.reject(error) + } + pending.clear() + } + + const exitPromise = new Promise((resolve) => { + child.on('exit', () => { + exited = true + resolve() + }) + }) + // Why: a spawn failure emits `error` instead of `exit`; reject active RPCs + // immediately so a missing executable cannot wait for the deadline. + child.on('error', (error) => { + spawnError = error + exited = true + failPending(error) + }) + // Why: close guarantees the stderr tail is complete before classifying an + // old CLI as unsupported rather than as a transient early exit. + child.on('close', () => failPending(buildEarlyExitError())) + // Why: JSONL may contain non-ASCII paths; stream decoding must retain a + // multibyte character split across pipe chunks. + child.stderr.setEncoding('utf8').on('data', (chunk: string) => { + stderrTail = (stderrTail + chunk).slice(-STDERR_TAIL_MAX_BYTES) + }) + // Why: the child can exit between the liveness check and stdin.write; an + // EPIPE must reject the RPC instead of becoming an unhandled stream error. + child.stdin.on('error', (error) => failPending(error)) + child.stdout.setEncoding('utf8').on('data', (chunk: string) => { + stdoutBuffer += chunk + if (Buffer.byteLength(stdoutBuffer) > STDOUT_LINE_MAX_BYTES) { + child.kill('SIGKILL') + failPending(new Error('codex app-server emitted an oversized JSONL response')) + return + } + let newlineIndex + while ((newlineIndex = stdoutBuffer.indexOf('\n')) !== -1) { + const line = stdoutBuffer.slice(0, newlineIndex).trim() + stdoutBuffer = stdoutBuffer.slice(newlineIndex + 1) + if (!line) { + continue + } + let message: JsonRpcResponse + try { + message = JSON.parse(line) as JsonRpcResponse + } catch { + continue + } + if (typeof message.id === 'number' && pending.has(message.id)) { + const waiter = pending.get(message.id)! + pending.delete(message.id) + waiter.resolve(message) + } + } + }) + + const deadline = setTimeout(() => { + timedOut = true + child.kill('SIGKILL') + failPending( + new CodexAppServerTimeoutError( + `codex app-server session exceeded ${invocation.timeoutMs}ms (${invocation.command})` + ) + ) + }, invocation.timeoutMs) + + const requestRpc: CodexAppServerRequestRpc = async (method, params) => { + if (spawnError) { + throw spawnError + } + if (timedOut) { + throw new CodexAppServerTimeoutError('codex app-server session already timed out') + } + if (exited) { + throw buildEarlyExitError() + } + const id = nextRequestId++ + const response = await new Promise((resolve, reject) => { + pending.set(id, { resolve, reject }) + try { + child.stdin.write(`${JSON.stringify({ method, id, ...(params ? { params } : {}) })}\n`) + } catch (error) { + pending.delete(id) + reject(error instanceof Error ? error : new Error(String(error))) + } + }) + if (response.error) { + if ( + response.error.code === JSON_RPC_METHOD_NOT_FOUND || + /method not found/i.test(response.error.message ?? '') + ) { + throw new CodexAppServerUnsupportedError( + `codex app-server does not support ${method}: ${response.error.message ?? 'method not found'}` + ) + } + throw new Error( + `codex app-server ${method} failed: ${response.error.message ?? 'unknown error'}` + ) + } + return response.result + } + + try { + await requestRpc('initialize', { + clientInfo: { name: 'orca_desktop', title: 'Orca', version: '0.0.0' } + }) + child.stdin.write(`${JSON.stringify({ method: 'initialized' })}\n`) + return await run(requestRpc) + } catch (error) { + if ( + error instanceof Error && + !(error instanceof CodexAppServerUnsupportedError) && + !(error instanceof CodexAppServerTimeoutError) && + stderrIndicatesMissingAppServer(stderrTail) + ) { + throw new CodexAppServerUnsupportedError( + `codex CLI does not support the app-server subcommand: ${stderrTail.trim().slice(0, 400)}` + ) + } + throw error + } finally { + try { + child.stdin.end() + } catch { + // stdin may already be destroyed after a kill; reaping below still runs. + } + if (!exited) { + await waitForProcessExitUntil(exitPromise, 1500) + if (!exited) { + child.kill('SIGKILL') + await waitForProcessExitUntil(exitPromise, 1000) + } + } + clearTimeout(deadline) + } +} diff --git a/src/main/codex/codex-real-home-hook-install.test.ts b/src/main/codex/codex-real-home-hook-install.test.ts index 6dfa04f4749..0a33ae69016 100644 --- a/src/main/codex/codex-real-home-hook-install.test.ts +++ b/src/main/codex/codex-real-home-hook-install.test.ts @@ -43,6 +43,7 @@ import { _internals } from './codex-real-home-hook-install' import { getCodexManagedHookInstallMaterial } from './hook-service' +import { _internals as rebaseInternals } from './codex-user-hook-trust-rebase' let fakeHomeDir: string let userDataDir: string @@ -86,6 +87,7 @@ beforeEach(() => { }) afterEach(() => { + rebaseInternals.setSessionRunnerSync(null) rmSync(fakeHomeDir, { recursive: true, force: true }) rmSync(userDataDir, { recursive: true, force: true }) if (previousUserDataPath === undefined) { @@ -312,6 +314,44 @@ describe('ensureRealHomeCodexHookState (install)', () => { }) describe('ensureRealHomeCodexHookState (opt-out sweep)', () => { + it('rebases trust when a user appended hooks after Orca installed', () => { + grantSucceeds() + const before = { type: 'command', command: 'before.sh' } + writeFileSync( + getRealHooksJsonPath(), + `${JSON.stringify({ hooks: { Stop: [{ hooks: [before] }] } }, null, 2)}\n` + ) + ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) + const installed = readRealHooksJson() + const after = { type: 'command', command: 'after.sh' } + installed.hooks!.Stop!.push({ hooks: [after] }) + writeFileSync(getRealHooksJsonPath(), `${JSON.stringify(installed, null, 2)}\n`) + const operations: string[] = [] + rebaseInternals.setSessionRunnerSync((request) => { + operations.push(request.operation) + if (request.operation === 'inspect-user-hook-trust') { + expect(readRealHooksJson().hooks?.Stop?.[2]?.hooks?.[0]?.command).toBe('after.sh') + return { + outcome: 'inspected', + moves: request.moves.map((move) => ({ + ...move, + reportedOldKey: move.oldKey, + wasTrusted: true, + enabled: true + })) + } + } + expect(readRealHooksJson().hooks?.Stop?.[1]?.hooks?.[0]?.command).toBe('after.sh') + return { outcome: 'repaired', repaired: 1 } + }) + + expect(ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir })).toBe( + 'removed' + ) + expect(operations).toEqual(['inspect-user-hook-trust', 'repair-user-hook-trust']) + expect(readRealHooksJson().hooks?.Stop).toEqual([{ hooks: [before] }, { hooks: [after] }]) + }) + it('removes only Orca entries and reports the removed lane', () => { grantSucceeds() const userStop = { diff --git a/src/main/codex/codex-real-home-hook-install.ts b/src/main/codex/codex-real-home-hook-install.ts index 47eb718d8a9..d71e78a1ac9 100644 --- a/src/main/codex/codex-real-home-hook-install.ts +++ b/src/main/codex/codex-real-home-hook-install.ts @@ -1,12 +1,4 @@ -import { - existsSync, - lstatSync, - mkdirSync, - readFileSync, - realpathSync, - statSync, - unlinkSync -} from 'node:fs' +import { existsSync, mkdirSync, readFileSync, statSync, unlinkSync } from 'node:fs' import { join } from 'node:path' import { writeFileAtomically } from '../codex-accounts/fs-utils' import { @@ -20,6 +12,7 @@ import { type HookDefinition, type HooksConfig } from '../agent-hooks/installer-utils' +import { resolveHooksJsonWritePath } from '../agent-hooks/hook-config-write-path' import { getCodexManagedScriptFileName } from './codex-hook-identity' import { CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS, @@ -30,6 +23,8 @@ import { removeCodexManagedHookTrustEntries } from './codex-managed-trust-reconc import { getCodexManagedHookInstallMaterial } from './hook-service' import { getSystemCodexHomePath } from './codex-home-paths' import type { CodexTrustEntry } from './config-toml-trust' +import { restoreCodexTrustConfig } from './codex-trust-config-rollback' +import { mutateRealHomeHooksPreservingUserTrust } from './codex-user-hook-trust-rebase' /** * Real-home Codex hook lane for the system-default selection (flag ON). @@ -53,12 +48,12 @@ export function getRealHomeCodexHookLane(): RealHomeCodexHookLane { } /** - * Routing gate consumed by CodexRuntimeHomeService: the real home is usable - * whenever hooks are opted out (nothing to install) or the grant lane has not - * proven incapable on this host. + * Routing gate consumed by CodexRuntimeHomeService. Both a failed install and + * a failed opt-out cleanup use the managed lane so no half-mutated hook state + * can diverge from PTY, rate-limit, or commit-message routing. */ -export function isRealHomeCodexHookLaneUsable(hooksEnabled: boolean): boolean { - return !hooksEnabled || currentLane !== 'unavailable' +export function isRealHomeCodexHookLaneUsable(_hooksEnabled: boolean): boolean { + return currentLane !== 'unavailable' } function getRealHomeHooksJsonPath(): string { @@ -69,35 +64,24 @@ function getRealHomeConfigTomlPath(): string { return join(getSystemCodexHomePath(), 'config.toml') } -function resolveRealHomeHooksWritePath(hooksJsonPath: string): string { - let isSymlink = false - try { - isSymlink = lstatSync(hooksJsonPath).isSymbolicLink() - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - return hooksJsonPath - } - throw error - } - if (!isSymlink) { - return hooksJsonPath - } - try { - // Why: replacing the link itself would silently disconnect a user's - // dotfiles-managed hooks.json. Atomic writes belong at its real target. - return realpathSync.native(hooksJsonPath) - } catch (error) { - throw new Error(`Could not resolve symlinked Codex hooks file ${hooksJsonPath}`, { - cause: error - }) - } -} - /** Orca-side state dir; nothing extra is ever written into the user's ~/.codex. */ function getRealHomeHookStateDir(userDataPath: string): string { return join(userDataPath, 'codex-real-home-hooks') } +function assertHooksJsonGeneration( + hooksJsonPath: string, + hooksWritePath: string, + expectedRaw: string | null +): void { + const currentRaw = existsSync(hooksJsonPath) ? readFileSync(hooksJsonPath, 'utf-8') : null + if (currentRaw !== expectedRaw || resolveHooksJsonWritePath(hooksJsonPath) !== hooksWritePath) { + // Why: the pre-mutation RPC can overlap a user's editor save. Abort rather + // than atomically replacing a newer file with the stale parsed snapshot. + throw new Error('Codex hooks.json changed while Orca prepared its trust repair') + } +} + /** * Ensures the real-home hook state matches the settings: installs and trusts * the Orca status hook when enabled, sweeps it when opted out. Idempotent and @@ -123,7 +107,7 @@ export function ensureRealHomeCodexHookState(args: { } } catch (error) { console.warn('[codex-real-home-hooks] ensure failed; staying on managed lane:', error) - currentLane = args.hooksEnabled ? 'unavailable' : currentLane + currentLane = 'unavailable' if (args.hooksEnabled) { installRetryAfterMs = Date.now() + CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS } @@ -134,7 +118,7 @@ export function ensureRealHomeCodexHookState(args: { function installRealHomeCodexHook(userDataPath: string): RealHomeCodexHookLane { const material = getCodexManagedHookInstallMaterial() const hooksJsonPath = getRealHomeHooksJsonPath() - const hooksWritePath = resolveRealHomeHooksWritePath(hooksJsonPath) + const hooksWritePath = resolveHooksJsonWritePath(hooksJsonPath) const config = readHooksJson(hooksJsonPath) if (!config) { // Why: an unparseable user file must never be clobbered; without a hook @@ -183,8 +167,19 @@ function installRealHomeCodexHook(userDataPath: string): RealHomeCodexHookLane { backupRealHomeHooksJsonOnce(userDataPath, previousRaw) // Why: unknown top-level fields belong to the user (other managers' // metadata); unlike the managed-home writer, preserve them verbatim. - writeHooksJson(hooksWritePath, { ...config, hooks: nextHooks } as HooksConfig, { - preserveMode: true + const trustConfigSnapshot = mutateRealHomeHooksPreservingUserTrust({ + sourcePath: hooksJsonPath, + runtimeHomePath: getSystemCodexHomePath(), + tomlPath: getRealHomeConfigTomlPath(), + beforeHooks: config.hooks ?? {}, + afterHooks: nextHooks, + writeHooks: () => { + assertHooksJsonGeneration(hooksJsonPath, hooksWritePath, previousRaw) + writeHooksJson(hooksWritePath, { ...config, hooks: nextHooks } as HooksConfig, { + preserveMode: true + }) + }, + restoreHooks: () => restoreRealHomeHooksJson(hooksWritePath, previousRaw, previousMode) }) const grant = grantManagedCodexHookTrust({ @@ -202,7 +197,15 @@ function installRealHomeCodexHook(userDataPath: string): RealHomeCodexHookLane { // would surface as "Hooks need review". Roll the file back to its prior // bytes and keep this host on the managed-home lane; the grant client // already logged the fallback reason. - restoreRealHomeHooksJson(hooksWritePath, previousRaw, previousMode) + try { + restoreRealHomeHooksJson(hooksWritePath, previousRaw, previousMode) + } finally { + // Why: a user-trust rebase may have succeeded before the managed grant + // failed. Roll both files back to the same pre-mutation generation. + if (trustConfigSnapshot) { + restoreCodexTrustConfig(getRealHomeConfigTomlPath(), trustConfigSnapshot) + } + } installRetryAfterMs = getInstallRetryAfterMs(grant.reason) console.warn( `[codex-real-home-hooks] trust grant unavailable (${grant.reason}); entry rolled back, managed lane kept` @@ -284,14 +287,28 @@ function sweepRealHomeCodexHook(): RealHomeCodexHookLane { } } if (removedAny) { - writeHooksJson( - resolveRealHomeHooksWritePath(hooksJsonPath), - { - ...config, - hooks: nextHooks - } as HooksConfig, - { preserveMode: true } - ) + const hooksWritePath = resolveHooksJsonWritePath(hooksJsonPath) + const previousRaw = readFileSync(hooksJsonPath, 'utf-8') + const previousMode = statSync(hooksWritePath).mode + mutateRealHomeHooksPreservingUserTrust({ + sourcePath: hooksJsonPath, + runtimeHomePath: getSystemCodexHomePath(), + tomlPath: getRealHomeConfigTomlPath(), + beforeHooks: config.hooks, + afterHooks: nextHooks, + writeHooks: () => { + assertHooksJsonGeneration(hooksJsonPath, hooksWritePath, previousRaw) + writeHooksJson( + hooksWritePath, + { + ...config, + hooks: nextHooks + } as HooksConfig, + { preserveMode: true } + ) + }, + restoreHooks: () => restoreRealHomeHooksJson(hooksWritePath, previousRaw, previousMode) + }) // Why: dead [hooks.state] blocks for a removed hook are Orca-owned records; // dropping them keeps the user's config.toml from accumulating orphans. // Verify ownership by the expected hash or grant ledger: stale/mixed hook @@ -333,19 +350,15 @@ function restoreRealHomeHooksJson( previousRaw: string | null, previousMode?: number ): void { - try { - if (previousRaw === null) { - if (existsSync(hooksJsonPath)) { - unlinkSync(hooksJsonPath) - } - return + if (previousRaw === null) { + if (existsSync(hooksJsonPath)) { + unlinkSync(hooksJsonPath) } - // Why: rollback is part of the safety boundary. Use the shared atomic - // writer so Windows file-lock retries and failed-temp cleanup are covered. - writeFileAtomically(hooksJsonPath, previousRaw, { mode: previousMode }) - } catch (error) { - console.warn('[codex-real-home-hooks] failed to roll back hooks.json:', error) + return } + // Why: rollback is part of the safety boundary. Use the shared atomic + // writer so Windows file-lock retries and failed-temp cleanup are covered. + writeFileAtomically(hooksJsonPath, previousRaw, { mode: previousMode }) } export const _internals = { diff --git a/src/main/codex/codex-user-hook-trust-rebase-client.test.ts b/src/main/codex/codex-user-hook-trust-rebase-client.test.ts new file mode 100644 index 00000000000..cfdc74bc236 --- /dev/null +++ b/src/main/codex/codex-user-hook-trust-rebase-client.test.ts @@ -0,0 +1,112 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type * as CodexAppServerSession from './codex-app-server-session' +import type { CodexAppServerRequestRpc } from './codex-app-server-session' + +const requestRpcMock = vi.hoisted(() => vi.fn()) +vi.mock('./codex-app-server-session', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + runCodexAppServerSession: vi.fn((_invocation, run) => run(requestRpcMock)) + } +}) + +import { runCodexUserHookTrustRebaseSession } from './codex-user-hook-trust-rebase-client' + +const invocation = { command: 'codex', args: ['app-server'], timeoutMs: 1000 } +const oldTrusted = '/home/a/.codex/hooks.json:stop:1:0' +const oldUntrusted = '/home/a/.codex/hooks.json:stop:2:0' +const newTrusted = '/home/a/.codex/hooks.json:stop:0:0' +const newUntrusted = '/home/a/.codex/hooks.json:stop:1:0' + +function listing(key: string, command: string, trustStatus: string, enabled = true) { + return { key, command, currentHash: `sha256:${key}`, trustStatus, enabled } +} + +function listResult(hooks: ReturnType[]) { + return { data: [{ cwd: '/tmp', hooks }] } +} + +beforeEach(() => requestRpcMock.mockReset()) + +describe('Codex user hook trust rebase RPCs', () => { + it('captures trusted, untrusted, and disabled states without writing config', async () => { + requestRpcMock.mockResolvedValueOnce( + listResult([ + listing(oldTrusted, 'trusted-user', 'trusted'), + listing(oldUntrusted, 'untrusted-user', 'untrusted', false) + ]) + ) + const result = await runCodexUserHookTrustRebaseSession({ + operation: 'inspect-user-hook-trust', + invocation, + hooksListCwd: '/tmp', + moves: [ + { oldKey: oldTrusted, newKey: newTrusted, command: 'trusted-user' }, + { oldKey: oldUntrusted, newKey: newUntrusted, command: 'untrusted-user' } + ] + }) + + expect(result).toEqual({ + outcome: 'inspected', + moves: [ + expect.objectContaining({ command: 'trusted-user', wasTrusted: true, enabled: true }), + expect.objectContaining({ command: 'untrusted-user', wasTrusted: false, enabled: false }) + ] + }) + expect(requestRpcMock).toHaveBeenCalledTimes(1) + }) + + it('clears shifted states, re-grants only prior trust, and verifies by re-list', async () => { + const postMutation = listResult([ + listing(newTrusted, 'trusted-user', 'untrusted'), + listing(newUntrusted, 'untrusted-user', 'trusted') + ]) + const verified = listResult([ + listing(newTrusted, 'trusted-user', 'trusted'), + listing(newUntrusted, 'untrusted-user', 'untrusted', false) + ]) + requestRpcMock + .mockResolvedValueOnce(postMutation) + .mockResolvedValueOnce({ status: 'ok' }) + .mockResolvedValueOnce(verified) + + await expect( + runCodexUserHookTrustRebaseSession({ + operation: 'repair-user-hook-trust', + invocation, + hooksListCwd: '/tmp', + moves: [ + { + oldKey: oldTrusted, + newKey: newTrusted, + command: 'trusted-user', + reportedOldKey: oldTrusted, + wasTrusted: true, + enabled: true + }, + { + oldKey: oldUntrusted, + newKey: newUntrusted, + command: 'untrusted-user', + reportedOldKey: oldUntrusted, + wasTrusted: false, + enabled: false + } + ] + }) + ).resolves.toEqual({ outcome: 'repaired', repaired: 1 }) + + const batch = requestRpcMock.mock.calls[1] + expect(batch?.[0]).toBe('config/batchWrite') + expect(batch).toBeDefined() + const edits = (batch![1] as { edits: { value: unknown }[] }).edits + expect(edits.some((edit) => edit.value === null)).toBe(true) + expect(edits).toContainEqual( + expect.objectContaining({ + value: expect.objectContaining({ trusted_hash: `sha256:${newTrusted}` }) + }) + ) + expect(edits).toContainEqual(expect.objectContaining({ value: { enabled: false } })) + }) +}) diff --git a/src/main/codex/codex-user-hook-trust-rebase-client.ts b/src/main/codex/codex-user-hook-trust-rebase-client.ts new file mode 100644 index 00000000000..c749946064f --- /dev/null +++ b/src/main/codex/codex-user-hook-trust-rebase-client.ts @@ -0,0 +1,163 @@ +import { collectCodexHookListings, type CodexHookListing } from './codex-app-server-client' +import { runCodexAppServerSession, type CodexAppServerInvocation } from './codex-app-server-session' +import { normalizeHookTrustKeyForLookup } from './config-toml-trust' + +export type CodexUserHookTrustMove = { + oldKey: string + newKey: string + command: string +} + +export type CodexCapturedUserHookTrustMove = CodexUserHookTrustMove & { + reportedOldKey: string + wasTrusted: boolean + enabled: boolean +} + +type CodexUserHookTrustRebaseRequestBase = { + invocation: CodexAppServerInvocation + hooksListCwd: string +} + +export type CodexUserHookTrustInspectRequest = CodexUserHookTrustRebaseRequestBase & { + operation: 'inspect-user-hook-trust' + moves: CodexUserHookTrustMove[] +} + +export type CodexUserHookTrustRepairRequest = CodexUserHookTrustRebaseRequestBase & { + operation: 'repair-user-hook-trust' + moves: CodexCapturedUserHookTrustMove[] +} + +export type CodexUserHookTrustRebaseRequest = + | CodexUserHookTrustInspectRequest + | CodexUserHookTrustRepairRequest + +export type CodexUserHookTrustRebaseResult = + | { outcome: 'inspected'; moves: CodexCapturedUserHookTrustMove[] } + | { outcome: 'repaired'; repaired: number } + +function matchingListings( + listings: readonly CodexHookListing[], + moves: readonly CodexUserHookTrustMove[], + key: 'oldKey' | 'newKey' +): Map { + const expected = new Map( + moves.map((move) => [normalizeHookTrustKeyForLookup(move[key]), move.command]) + ) + return new Map( + listings + .filter( + (listing) => expected.get(normalizeHookTrustKeyForLookup(listing.key)) === listing.command + ) + .map((listing) => [normalizeHookTrustKeyForLookup(listing.key), listing]) + ) +} + +function quotedKeyPath(key: string): string { + const escaped = key.replaceAll('\\', '\\\\').replaceAll('"', '\\"') + return `hooks.state."${escaped}"` +} + +async function inspectUserHookTrust( + request: CodexUserHookTrustInspectRequest +): Promise { + return runCodexAppServerSession(request.invocation, async (requestRpc) => { + const result = await requestRpc('hooks/list', { cwds: [request.hooksListCwd] }) + const byOldKey = matchingListings(collectCodexHookListings(result), request.moves, 'oldKey') + if (byOldKey.size !== request.moves.length) { + throw new Error( + `pre-mutation hooks/list reported ${byOldKey.size} of ${request.moves.length} moved user hooks` + ) + } + return { + outcome: 'inspected', + moves: request.moves.map((move) => { + const listing = byOldKey.get(normalizeHookTrustKeyForLookup(move.oldKey))! + return { + ...move, + reportedOldKey: listing.key, + wasTrusted: listing.trustStatus === 'trusted', + enabled: listing.enabled + } + }) + } + }) +} + +async function repairUserHookTrust( + request: CodexUserHookTrustRepairRequest +): Promise { + return runCodexAppServerSession(request.invocation, async (requestRpc) => { + const result = await requestRpc('hooks/list', { cwds: [request.hooksListCwd] }) + const byNewKey = matchingListings(collectCodexHookListings(result), request.moves, 'newKey') + if (byNewKey.size !== request.moves.length) { + throw new Error( + `post-mutation hooks/list reported ${byNewKey.size} of ${request.moves.length} moved user hooks` + ) + } + + const keysToClear = new Set([ + ...request.moves.map((move) => move.reportedOldKey), + ...Array.from(byNewKey.values(), (listing) => listing.key) + ]) + const edits: { keyPath: string; value: unknown; mergeStrategy: 'replace' }[] = Array.from( + keysToClear, + (key) => ({ + keyPath: quotedKeyPath(key), + value: null, + mergeStrategy: 'replace' as const + }) + ) + for (const move of request.moves) { + const listing = byNewKey.get(normalizeHookTrustKeyForLookup(move.newKey))! + if (move.wasTrusted) { + edits.push({ + keyPath: quotedKeyPath(listing.key), + value: { + trusted_hash: listing.currentHash, + ...(move.enabled ? {} : { enabled: false }) + }, + mergeStrategy: 'replace' + }) + } else if (!move.enabled) { + edits.push({ + keyPath: quotedKeyPath(listing.key), + value: { enabled: false }, + mergeStrategy: 'replace' + }) + } + } + await requestRpc('config/batchWrite', { edits, reloadUserConfig: true }) + + const verified = await requestRpc('hooks/list', { cwds: [request.hooksListCwd] }) + const verifiedByKey = matchingListings( + collectCodexHookListings(verified), + request.moves, + 'newKey' + ) + const invalid = request.moves.find((move) => { + const listing = verifiedByKey.get(normalizeHookTrustKeyForLookup(move.newKey)) + return ( + !listing || + (listing.trustStatus === 'trusted') !== move.wasTrusted || + listing.enabled !== move.enabled + ) + }) + if (invalid) { + throw new Error(`post-rebase verify failed for moved user hook ${invalid.newKey}`) + } + return { + outcome: 'repaired', + repaired: request.moves.filter((move) => move.wasTrusted).length + } + }) +} + +export function runCodexUserHookTrustRebaseSession( + request: CodexUserHookTrustRebaseRequest +): Promise { + return request.operation === 'inspect-user-hook-trust' + ? inspectUserHookTrust(request) + : repairUserHookTrust(request) +} diff --git a/src/main/codex/codex-user-hook-trust-rebase.test.ts b/src/main/codex/codex-user-hook-trust-rebase.test.ts new file mode 100644 index 00000000000..411af224969 --- /dev/null +++ b/src/main/codex/codex-user-hook-trust-rebase.test.ts @@ -0,0 +1,185 @@ +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { HookCommandConfig, HookDefinition } from '../agent-hooks/installer-utils' +import type { CodexUserHookTrustRebaseRequest } from './codex-user-hook-trust-rebase-client' + +const resolveCodexCommandMock = vi.hoisted(() => vi.fn(() => process.execPath)) +vi.mock('../codex-cli/command', () => ({ resolveCodexCommand: resolveCodexCommandMock })) + +import { + _internals, + getMovedCodexUserHookTrust, + mutateRealHomeHooksPreservingUserTrust +} from './codex-user-hook-trust-rebase' + +let root: string +let hooksPath: string +let configPath: string + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'orca-user-hook-rebase-')) + hooksPath = join(root, 'hooks.json') + configPath = join(root, 'config.toml') +}) + +afterEach(() => { + _internals.setSessionRunnerSync(null) + rmSync(root, { recursive: true, force: true }) +}) + +function command(command: string): HookCommandConfig { + return { type: 'command', command } +} + +describe('real-home user hook trust rebasing', () => { + it('writes directly without reading config or spawning Codex when user positions stay stable', () => { + const user = command('user-hook') + const orca = command('orca-hook') + const before = { Stop: [{ hooks: [user] }] } + const after = { Stop: [{ hooks: [user] }, { hooks: [orca] }] } + let wroteHooks = false + _internals.setSessionRunnerSync(() => { + throw new Error('stable positions must not open an app-server session') + }) + + expect( + mutateRealHomeHooksPreservingUserTrust({ + sourcePath: hooksPath, + runtimeHomePath: root, + tomlPath: configPath, + beforeHooks: before, + afterHooks: after, + writeHooks: () => { + wroteHooks = true + }, + restoreHooks: () => { + throw new Error('restore must not run') + } + }) + ).toBeNull() + expect(wroteHooks).toBe(true) + expect(existsSync(configPath)).toBe(false) + }) + + it('finds multiple shifted user hooks, including a handler from a mixed group', () => { + const orca = command('orca-hook') + const first = command('first-user') + const second = command('second-user') + const mixed = command('mixed-user') + const before: Record = { + Stop: [{ hooks: [orca] }, { hooks: [first] }, { hooks: [second] }, { hooks: [orca, mixed] }] + } + const after: Record = { + Stop: [{ hooks: [first] }, { hooks: [second] }, { hooks: [mixed] }] + } + + expect(getMovedCodexUserHookTrust(hooksPath, before, after)).toEqual([ + expect.objectContaining({ + command: 'first-user', + oldKey: expect.stringContaining(':1:0'), + newKey: expect.stringContaining(':0:0') + }), + expect.objectContaining({ + command: 'second-user', + oldKey: expect.stringContaining(':2:0'), + newKey: expect.stringContaining(':1:0') + }), + expect.objectContaining({ + command: 'mixed-user', + oldKey: expect.stringContaining(':3:1'), + newKey: expect.stringContaining(':2:0') + }) + ]) + }) + + it('carries only previously trusted states into the repair request', () => { + const orca = command('orca-hook') + const trusted = command('trusted-user') + const untrusted = command('untrusted-user') + const before = { Stop: [{ hooks: [orca] }, { hooks: [trusted] }, { hooks: [untrusted] }] } + const after = { Stop: [{ hooks: [trusted] }, { hooks: [untrusted] }] } + writeFileSync(hooksPath, `${JSON.stringify({ hooks: before }, null, 2)}\n`) + writeFileSync(configPath, '# original config\n') + const requests: CodexUserHookTrustRebaseRequest[] = [] + _internals.setSessionRunnerSync((request) => { + requests.push(request) + if (request.operation === 'inspect-user-hook-trust') { + return { + outcome: 'inspected', + moves: request.moves.map((move) => ({ + ...move, + reportedOldKey: move.oldKey, + wasTrusted: move.command === 'trusted-user', + enabled: move.command !== 'untrusted-user' + })) + } + } + return { outcome: 'repaired', repaired: 1 } + }) + + mutateRealHomeHooksPreservingUserTrust({ + sourcePath: hooksPath, + runtimeHomePath: root, + tomlPath: configPath, + beforeHooks: before, + afterHooks: after, + writeHooks: () => writeFileSync(hooksPath, `${JSON.stringify({ hooks: after }, null, 2)}\n`), + restoreHooks: () => { + throw new Error('restore must not run') + } + }) + + expect(requests).toHaveLength(2) + const repair = requests[1] + expect(repair?.operation).toBe('repair-user-hook-trust') + if (repair?.operation === 'repair-user-hook-trust') { + expect(repair.moves).toEqual([ + expect.objectContaining({ command: 'trusted-user', wasTrusted: true, enabled: true }), + expect.objectContaining({ command: 'untrusted-user', wasTrusted: false, enabled: false }) + ]) + } + }) + + it('restores both files byte-exactly when post-mutation repair fails', () => { + const orca = command('orca-hook') + const user = command('user-hook') + const before = { Stop: [{ hooks: [orca] }, { hooks: [user] }] } + const after = { Stop: [{ hooks: [user] }] } + const originalHooks = + '{ "hooks": { "Stop": [{"hooks":[{"type":"command","command":"orca-hook"}]},{"hooks":[{"type":"command","command":"user-hook"}]}] } }\r\n' + const originalConfig = '# user formatting\r\nmodel = "x"\r\n' + writeFileSync(hooksPath, originalHooks) + writeFileSync(configPath, originalConfig) + _internals.setSessionRunnerSync((request) => { + if (request.operation === 'inspect-user-hook-trust') { + return { + outcome: 'inspected', + moves: request.moves.map((move) => ({ + ...move, + reportedOldKey: move.oldKey, + wasTrusted: true, + enabled: true + })) + } + } + writeFileSync(configPath, '[hooks.state."partial"]\ntrusted_hash = "bad"\n') + throw new Error('repair transport failed') + }) + + expect(() => + mutateRealHomeHooksPreservingUserTrust({ + sourcePath: hooksPath, + runtimeHomePath: root, + tomlPath: configPath, + beforeHooks: before, + afterHooks: after, + writeHooks: () => writeFileSync(hooksPath, `${JSON.stringify({ hooks: after })}\n`), + restoreHooks: () => writeFileSync(hooksPath, originalHooks) + }) + ).toThrow('repair transport failed') + expect(readFileSync(hooksPath, 'utf-8')).toBe(originalHooks) + expect(readFileSync(configPath, 'utf-8')).toBe(originalConfig) + }) +}) diff --git a/src/main/codex/codex-user-hook-trust-rebase.ts b/src/main/codex/codex-user-hook-trust-rebase.ts new file mode 100644 index 00000000000..5da7fa1d148 --- /dev/null +++ b/src/main/codex/codex-user-hook-trust-rebase.ts @@ -0,0 +1,163 @@ +import type { HookCommandConfig, HookDefinition } from '../agent-hooks/installer-utils' +import { runCodexUserHookTrustRebaseSessionSync } from './codex-app-server-grant-bridge' +import { createCodexHookTrustEntry } from './codex-hook-identity' +import { resolveCodexTrustGrantHost } from './codex-trust-grant-host' +import { + captureCodexTrustConfig, + restoreCodexTrustConfig, + type CodexTrustConfigSnapshot +} from './codex-trust-config-rollback' +import { computeTrustKey, type CodexTrustEntry } from './config-toml-trust' +import type { + CodexUserHookTrustRebaseRequest, + CodexUserHookTrustRebaseResult, + CodexUserHookTrustMove +} from './codex-user-hook-trust-rebase-client' + +type HooksByEvent = Record + +type RebaseSessionRunnerSync = ( + request: CodexUserHookTrustRebaseRequest +) => CodexUserHookTrustRebaseResult + +let runSessionSync: RebaseSessionRunnerSync = runCodexUserHookTrustRebaseSessionSync + +function entriesByHookObject( + sourcePath: string, + hooksByEvent: HooksByEvent +): Map { + const result = new Map() + for (const [eventName, definitions] of Object.entries(hooksByEvent)) { + if (!Array.isArray(definitions)) { + continue + } + definitions.forEach((definition, groupIndex) => { + if (!Array.isArray(definition.hooks)) { + return + } + definition.hooks.forEach((hook, handlerIndex) => { + const entry = createCodexHookTrustEntry( + sourcePath, + eventName, + groupIndex, + handlerIndex, + definition, + hook + ) + if (entry) { + result.set(hook, entry) + } + }) + }) + } + return result +} + +export function getMovedCodexUserHookTrust( + sourcePath: string, + beforeHooks: HooksByEvent, + afterHooks: HooksByEvent +): CodexUserHookTrustMove[] { + const before = entriesByHookObject(sourcePath, beforeHooks) + const after = entriesByHookObject(sourcePath, afterHooks) + const moves: CodexUserHookTrustMove[] = [] + for (const [hook, oldEntry] of before) { + const newEntry = after.get(hook) + if (!newEntry) { + continue + } + const oldKey = computeTrustKey(oldEntry) + const newKey = computeTrustKey(newEntry) + if (oldKey !== newKey) { + moves.push({ oldKey, newKey, command: oldEntry.command }) + } + } + return moves +} + +function rollbackMutation( + restoreHooks: () => void, + tomlPath: string, + snapshot: CodexTrustConfigSnapshot, + originalError: unknown +): never { + const rollbackErrors: unknown[] = [] + try { + restoreHooks() + } catch (error) { + rollbackErrors.push(error) + } + try { + restoreCodexTrustConfig(tomlPath, snapshot) + } catch (error) { + rollbackErrors.push(error) + } + if (rollbackErrors.length > 0) { + throw new AggregateError( + [originalError, ...rollbackErrors], + 'Failed to rebase moved user hook trust and restore the original files' + ) + } + throw originalError +} + +export function mutateRealHomeHooksPreservingUserTrust(args: { + sourcePath: string + runtimeHomePath: string + tomlPath: string + beforeHooks: HooksByEvent + afterHooks: HooksByEvent + writeHooks: () => void + restoreHooks: () => void +}): CodexTrustConfigSnapshot | null { + const moves = getMovedCodexUserHookTrust(args.sourcePath, args.beforeHooks, args.afterHooks) + if (moves.length === 0) { + args.writeHooks() + return null + } + const snapshot = captureCodexTrustConfig(args.tomlPath) + + const baseRequest = resolveCodexTrustGrantHost({ kind: 'native' }).buildRequest({ + runtimeHomePath: args.runtimeHomePath, + managedCommand: '', + expectedTrustKeys: [] + }) + // Why: inspection happens before the write, so an unavailable RPC aborts + // without shifting a user's positional trust key. + const inspected: CodexUserHookTrustRebaseResult = runSessionSync({ + operation: 'inspect-user-hook-trust', + invocation: baseRequest.invocation, + hooksListCwd: baseRequest.hooksListCwd, + moves + }) + if (inspected.outcome !== 'inspected') { + throw new Error('Unexpected Codex user hook trust inspection result') + } + + let hooksWritten = false + try { + args.writeHooks() + hooksWritten = true + const repaired = runSessionSync({ + operation: 'repair-user-hook-trust', + invocation: baseRequest.invocation, + hooksListCwd: baseRequest.hooksListCwd, + moves: inspected.moves + }) + if (repaired.outcome !== 'repaired') { + throw new Error('Unexpected Codex user hook trust repair result') + } + return snapshot + } catch (error) { + if (hooksWritten) { + return rollbackMutation(args.restoreHooks, args.tomlPath, snapshot, error) + } + throw error + } +} + +export const _internals = { + setSessionRunnerSync(runner: RebaseSessionRunnerSync | null): void { + runSessionSync = runner ?? runCodexUserHookTrustRebaseSessionSync + } +} diff --git a/src/main/codex/config-toml-trust.test.ts b/src/main/codex/config-toml-trust.test.ts index 9f8efbea403..9dd98e5bd3f 100644 --- a/src/main/codex/config-toml-trust.test.ts +++ b/src/main/codex/config-toml-trust.test.ts @@ -2,11 +2,13 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { existsSync, + lstatSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, + symlinkSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' @@ -1300,6 +1302,44 @@ describe('normalizeCodexProjectPathForRevocationLookup', () => { }) describe('removeHookTrustEntries', () => { + it('updates a symlink target without replacing config.toml', () => { + const targetPath = join(tmpDir, 'dotfiles-config.toml') + const entry: CodexTrustEntry = { + sourcePath: '/x/hooks.json', + eventLabel: 'stop', + groupIndex: 0, + handlerIndex: 0, + command: 'echo trusted' + } + upsertHookTrustEntries(targetPath, [entry]) + symlinkSync(targetPath, configPath) + + removeHookTrustEntries(configPath, [computeTrustKey(entry)]) + + expect(lstatSync(configPath).isSymbolicLink()).toBe(true) + expect(readHookTrustEntries(targetPath).has(computeTrustKey(entry))).toBe(false) + }) + + it('does not replace a dangling config.toml symlink', () => { + const targetPath = join(tmpDir, 'missing-dotfiles-config.toml') + symlinkSync(targetPath, configPath) + + expect(() => + upsertHookTrustEntries(configPath, [ + { + sourcePath: '/x/hooks.json', + eventLabel: 'stop', + groupIndex: 0, + handlerIndex: 0, + command: 'echo trusted' + } + ]) + ).toThrow() + + expect(lstatSync(configPath).isSymbolicLink()).toBe(true) + expect(existsSync(targetPath)).toBe(false) + }) + it('is a no-op (creates no file) when the config does not exist', () => { removeHookTrustEntries(configPath, ['/x/hooks.json:pre_tool_use:0:0']) expect(existsSync(configPath)).toBe(false) diff --git a/src/main/codex/config-toml-trust.ts b/src/main/codex/config-toml-trust.ts index eede0efea32..316f089e570 100644 --- a/src/main/codex/config-toml-trust.ts +++ b/src/main/codex/config-toml-trust.ts @@ -1,6 +1,7 @@ /* eslint-disable max-lines -- Why: Codex hook trust parsing, hashing, and byte-preserving TOML edits share one fragile file-format contract; splitting would make the compatibility shim harder to audit. */ import { existsSync, + lstatSync, mkdirSync, readFileSync, realpathSync, @@ -780,16 +781,30 @@ function isCompleteTableHeader(line: string): boolean { // tmp and rename. Random-suffix tmp name avoids cross-process races on // rapid reinstalls. export function writeConfigAtomically(configPath: string, contents: string): void { - const dir = dirname(configPath) + let writePath = configPath + let isSymlink = false + try { + isSymlink = lstatSync(configPath).isSymbolicLink() + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error + } + } + if (isSymlink) { + // Why: atomic rename at the lexical path replaces the user's dotfiles + // link. A dangling link must fail closed rather than be replaced. + writePath = realpathSync.native(configPath) + } + const dir = dirname(writePath) mkdirSync(dir, { recursive: true }) const tmpPath = join(dir, `.${Date.now()}-${randomUUID()}.tmp`) let renamed = false try { writeFileSync(tmpPath, contents, 'utf-8') - if (existsSync(configPath)) { - copyFileWithWindowsRetry(configPath, `${configPath}.bak`) + if (existsSync(writePath)) { + copyFileWithWindowsRetry(writePath, `${writePath}.bak`) } - renameFileWithWindowsRetry(tmpPath, configPath) + renameFileWithWindowsRetry(tmpPath, writePath) renamed = true } finally { if (!renamed && existsSync(tmpPath)) { diff --git a/src/main/codex/hook-service-trust-grant.test.ts b/src/main/codex/hook-service-trust-grant.test.ts index cb3a6dc0ee1..ac5278aebed 100644 --- a/src/main/codex/hook-service-trust-grant.test.ts +++ b/src/main/codex/hook-service-trust-grant.test.ts @@ -1,5 +1,13 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { + lstatSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync +} from 'node:fs' import { tmpdir } from 'node:os' import type * as Os from 'node:os' import { join } from 'node:path' @@ -22,6 +30,7 @@ import { } from './codex-trust-grant-ledger' import type { CodexHookTrustGrantRequest } from './codex-app-server-client' import { getCodexHookTrustSignature } from './codex-hook-identity' +import { _internals as rebaseInternals } from './codex-user-hook-trust-rebase' const { getPathMock, homedirMock, resolveCodexCommandMock } = vi.hoisted(() => ({ getPathMock: vi.fn<(name: string) => string>(), @@ -60,6 +69,7 @@ beforeEach(() => { }) afterEach(() => { + rebaseInternals.setSessionRunnerSync(null) trustGrantInternals.setGrantSessionRunnerSync(null) trustGrantInternals.resetDiagnostics() codexAppServerCapabilityCache.clear() @@ -209,6 +219,56 @@ describe('CodexHookService app-server trust grant lane', () => { expect(readCodexTrustGrantLedgerHome(systemHome)).toBeNull() }) + it('keeps a real-home symlink and rebases later user trust during flag-off cleanup', () => { + prepareSystemHome() + const systemHome = join(tmpHome, '.codex') + const hooksPath = join(systemHome, 'hooks.json') + const targetPath = join(tmpHome, 'dotfiles-hooks.json') + const material = getCodexManagedHookInstallMaterial() + const userHook = { type: 'command' as const, command: 'after-orca.sh' } + writeFileSync( + targetPath, + `${JSON.stringify( + { + hooks: { + Stop: [ + { hooks: [{ type: 'command', command: material.command }] }, + { hooks: [userHook] } + ] + } + }, + null, + 2 + )}\n` + ) + symlinkSync(targetPath, hooksPath) + const operations: string[] = [] + rebaseInternals.setSessionRunnerSync((request) => { + operations.push(request.operation) + if (request.operation === 'inspect-user-hook-trust') { + return { + outcome: 'inspected', + moves: request.moves.map((move) => ({ + ...move, + reportedOldKey: move.oldKey, + wasTrusted: true, + enabled: true + })) + } + } + return { outcome: 'repaired', repaired: 1 } + }) + installCodexLikeGrantRunner() + + expect(new CodexHookService().install().state).toBe('installed') + + expect(lstatSync(hooksPath).isSymbolicLink()).toBe(true) + expect(JSON.parse(readFileSync(targetPath, 'utf-8')).hooks.Stop).toEqual([ + { hooks: [userHook] } + ]) + expect(operations).toEqual(['inspect-user-hook-trust', 'repair-user-hook-trust']) + }) + it('does not accept a ledger hash after the recorded Codex binary stamp changes', () => { prepareSystemHome() installCodexLikeGrantRunner() diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index fd72b9b3bcd..6da68c331ee 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -1,5 +1,5 @@ /* eslint-disable max-lines -- Why: getStatus + install + remove all share the managed-command and trust-key derivation. Splitting would hide that the three operations must agree on group index, event label, and command bytes. */ -import { existsSync, readFileSync, unlinkSync } from 'node:fs' +import { existsSync, readFileSync, statSync, unlinkSync } from 'node:fs' import { join, win32 as pathWin32 } from 'node:path' import type { SFTPWrapper } from 'ssh2' import type { AgentHookInstallState, AgentHookInstallStatus } from '../../shared/agent-hook-types' @@ -18,6 +18,8 @@ import { writeManagedScript, type HookDefinition } from '../agent-hooks/installer-utils' +import { resolveHooksJsonWritePath } from '../agent-hooks/hook-config-write-path' +import { writeFileAtomically } from '../codex-accounts/fs-utils' import { readHooksJsonRemote, readTextFileRemote, @@ -75,6 +77,7 @@ import { removeStaleWslCodexManagedHookTrustEntries } from './codex-managed-trust-reconciliation' import type { CodexTrustGrantLedgerHome } from './codex-trust-grant-ledger' +import { mutateRealHomeHooksPreservingUserTrust } from './codex-user-hook-trust-rebase' // Why: PreToolUse/PostToolUse give the dashboard a live readout of the // in-flight tool (name + input preview) between UserPromptSubmit and Stop. @@ -642,7 +645,28 @@ function cleanupLegacySystemManagedHooks(): void { if (removedManagedHook) { // Why: this is the user's system hooks file, not Orca's runtime copy. // Remove only stale Orca hook entries and preserve other managers' metadata. - writeHooksJson(legacyConfigPath, { ...config, hooks: nextHooks }) + const hooksWritePath = resolveHooksJsonWritePath(legacyConfigPath) + const previousRaw = readFileSync(legacyConfigPath, 'utf-8') + const previousMode = statSync(hooksWritePath).mode + mutateRealHomeHooksPreservingUserTrust({ + sourcePath: legacyConfigPath, + runtimeHomePath: systemHomePath, + tomlPath: getSystemCodexConfigTomlPath(), + beforeHooks: config.hooks, + afterHooks: nextHooks, + writeHooks: () => { + if ( + readFileSync(legacyConfigPath, 'utf-8') !== previousRaw || + resolveHooksJsonWritePath(legacyConfigPath) !== hooksWritePath + ) { + // Why: the pre-mutation RPC may overlap a user save; downgrade must + // never replace that newer dotfiles generation with our stale parse. + throw new Error('System Codex hooks changed during trust repair') + } + writeHooksJson(hooksWritePath, { ...config, hooks: nextHooks }) + }, + restoreHooks: () => writeFileAtomically(hooksWritePath, previousRaw, { mode: previousMode }) + }) // Why: stale dev/version entries can reference an older managed script // path that is not represented by the current grant ledger. removeSelfComputedMatchingTrustEntries(getSystemCodexConfigTomlPath(), trustEntries) From f5cf6ea53d8503c2a41d05e0e0aa07b430ef528a Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 16 Jul 2026 01:16:23 -0700 Subject: [PATCH 02/10] fix: harden real-home hook cleanup --- .../codex-real-home-hook-install.test.ts | 40 +++++++++++++------ .../codex/codex-real-home-hook-install.ts | 6 +++ src/main/codex/config-toml-trust.test.ts | 18 +++++++++ src/main/codex/config-toml-trust.ts | 6 ++- .../codex/hook-service-trust-grant.test.ts | 21 ++++++++++ src/main/codex/hook-service.ts | 2 +- src/main/index.ts | 6 +-- 7 files changed, 82 insertions(+), 17 deletions(-) diff --git a/src/main/codex/codex-real-home-hook-install.test.ts b/src/main/codex/codex-real-home-hook-install.test.ts index 0a33ae69016..b49dcf691ea 100644 --- a/src/main/codex/codex-real-home-hook-install.test.ts +++ b/src/main/codex/codex-real-home-hook-install.test.ts @@ -119,7 +119,7 @@ describe('ensureRealHomeCodexHookState (install)', () => { expect(plan.managedEntries.every((entry) => entry.groupIndex === 0)).toBe(true) }) - it('appends LAST and preserves user entries, unknown fields, and trust positions', () => { + it('keeps the managed lane for unknown top-level fields Codex cannot load', () => { grantSucceeds() const userConfig = { hooks: { @@ -128,27 +128,43 @@ describe('ensureRealHomeCodexHookState (install)', () => { }, _pluginManagerMetadata: { owner: 'someone-else' } } - writeFileSync(getRealHooksJsonPath(), `${JSON.stringify(userConfig, null, 2)}\n`, 'utf-8') + const original = `${JSON.stringify(userConfig, null, 2)}\n` + writeFileSync(getRealHooksJsonPath(), original, 'utf-8') const lane = ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir }) - expect(lane).toBe('installed') + expect(lane).toBe('unavailable') + expect(readFileSync(getRealHooksJsonPath(), 'utf-8')).toBe(original) + expect(grantMock).not.toHaveBeenCalled() + expect(existsSync(join(userDataDir, 'codex-real-home-hooks', 'hooks.json.pre-orca'))).toBe( + false + ) + }) + + it('appends LAST and preserves user entries and trust positions', () => { + grantSucceeds() + const userConfig = { + hooks: { + Stop: [{ matcher: 'deploy-*', hooks: [{ type: 'command', command: 'my-stop-hook.sh' }] }], + PreCompact: [{ hooks: [{ type: 'command', command: 'my-compact-hook.sh' }] }] + } + } + const original = `${JSON.stringify(userConfig, null, 2)}\n` + writeFileSync(getRealHooksJsonPath(), original, 'utf-8') + + expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( + 'installed' + ) + const config = readRealHooksJson() - // User's Stop entry keeps position 0; Orca's entry is appended after it. expect(config.hooks?.Stop).toHaveLength(2) expect(config.hooks?.Stop?.[0]).toEqual(userConfig.hooks.Stop[0]) - // Non-managed events Orca does not subscribe to stay untouched. expect(config.hooks?.PreCompact).toEqual(userConfig.hooks.PreCompact) - // Unknown top-level metadata survives, unlike the managed-home writer. - expect(config._pluginManagerMetadata).toEqual(userConfig._pluginManagerMetadata) - // The appended entry's trust key uses its appended position. const plan = grantMock.mock.calls[0]![0] as CodexManagedTrustGrantPlan - const stopEntry = plan.managedEntries.find((entry) => entry.eventLabel === 'stop') - expect(stopEntry?.groupIndex).toBe(1) - // Pristine pre-Orca backup lands under userData, not in ~/.codex. + expect(plan.managedEntries.find((entry) => entry.eventLabel === 'stop')?.groupIndex).toBe(1) expect( readFileSync(join(userDataDir, 'codex-real-home-hooks', 'hooks.json.pre-orca'), 'utf-8') - ).toBe(`${JSON.stringify(userConfig, null, 2)}\n`) + ).toBe(original) }) it('updates a symlinked hooks.json target without replacing the symlink', () => { diff --git a/src/main/codex/codex-real-home-hook-install.ts b/src/main/codex/codex-real-home-hook-install.ts index d71e78a1ac9..8e122f69812 100644 --- a/src/main/codex/codex-real-home-hook-install.ts +++ b/src/main/codex/codex-real-home-hook-install.ts @@ -127,6 +127,12 @@ function installRealHomeCodexHook(userDataPath: string): RealHomeCodexHookLane { installRetryAfterMs = Date.now() + CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS return 'unavailable' } + if (Object.keys(config).some((key) => key !== 'hooks')) { + // Why: Codex rejects unknown root keys instead of ignoring them. Avoid a + // transient rewrite of a user-owned file that the trust RPC cannot load. + installRetryAfterMs = Date.now() + CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS + return 'unavailable' + } // Why: the same script the managed lane maintains; deploying here too keeps // host-connect ordering independent of the managed installer loop. diff --git a/src/main/codex/config-toml-trust.test.ts b/src/main/codex/config-toml-trust.test.ts index 9dd98e5bd3f..26ccd96cd33 100644 --- a/src/main/codex/config-toml-trust.test.ts +++ b/src/main/codex/config-toml-trust.test.ts @@ -1,6 +1,7 @@ /* eslint-disable max-lines -- Why: this suite keeps the hash fixture, TOML edit edge cases, and trust-state parser regressions together so Codex compatibility failures are easy to audit. */ import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { + chmodSync, existsSync, lstatSync, mkdirSync, @@ -9,6 +10,7 @@ import { realpathSync, rmSync, symlinkSync, + statSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' @@ -1302,6 +1304,22 @@ describe('normalizeCodexProjectPathForRevocationLookup', () => { }) describe('removeHookTrustEntries', () => { + it.skipIf(process.platform === 'win32')('preserves restrictive config permissions', () => { + const entry: CodexTrustEntry = { + sourcePath: '/x/hooks.json', + eventLabel: 'stop', + groupIndex: 0, + handlerIndex: 0, + command: 'echo trusted' + } + upsertHookTrustEntries(configPath, [entry]) + chmodSync(configPath, 0o600) + + removeHookTrustEntries(configPath, [computeTrustKey(entry)]) + + expect(statSync(configPath).mode & 0o777).toBe(0o600) + }) + it('updates a symlink target without replacing config.toml', () => { const targetPath = join(tmpDir, 'dotfiles-config.toml') const entry: CodexTrustEntry = { diff --git a/src/main/codex/config-toml-trust.ts b/src/main/codex/config-toml-trust.ts index 316f089e570..4eb06612f33 100644 --- a/src/main/codex/config-toml-trust.ts +++ b/src/main/codex/config-toml-trust.ts @@ -5,6 +5,7 @@ import { mkdirSync, readFileSync, realpathSync, + statSync, unlinkSync, writeFileSync } from 'node:fs' @@ -798,9 +799,12 @@ export function writeConfigAtomically(configPath: string, contents: string): voi const dir = dirname(writePath) mkdirSync(dir, { recursive: true }) const tmpPath = join(dir, `.${Date.now()}-${randomUUID()}.tmp`) + const existingMode = existsSync(writePath) ? statSync(writePath).mode : undefined let renamed = false try { - writeFileSync(tmpPath, contents, 'utf-8') + // Why: real-home trust cleanup must not widen a dotfiles-managed config's + // restrictive permissions when the atomic rename installs new bytes. + writeFileSync(tmpPath, contents, { encoding: 'utf-8', mode: existingMode }) if (existsSync(writePath)) { copyFileWithWindowsRetry(writePath, `${writePath}.bak`) } diff --git a/src/main/codex/hook-service-trust-grant.test.ts b/src/main/codex/hook-service-trust-grant.test.ts index ac5278aebed..77f8e494ea1 100644 --- a/src/main/codex/hook-service-trust-grant.test.ts +++ b/src/main/codex/hook-service-trust-grant.test.ts @@ -1,11 +1,13 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { + chmodSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, + statSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' @@ -269,6 +271,25 @@ describe('CodexHookService app-server trust grant lane', () => { expect(operations).toEqual(['inspect-user-hook-trust', 'repair-user-hook-trust']) }) + it.skipIf(process.platform === 'win32')( + 'preserves restrictive real-home hooks permissions during flag-off cleanup', + () => { + prepareSystemHome() + const hooksPath = join(tmpHome, '.codex', 'hooks.json') + const material = getCodexManagedHookInstallMaterial() + writeFileSync( + hooksPath, + `${JSON.stringify({ hooks: { Stop: [{ hooks: [{ type: 'command', command: material.command }] }] } }, null, 2)}\n` + ) + chmodSync(hooksPath, 0o600) + installCodexLikeGrantRunner() + + expect(new CodexHookService().install().state).toBe('installed') + + expect(statSync(hooksPath).mode & 0o777).toBe(0o600) + } + ) + it('does not accept a ledger hash after the recorded Codex binary stamp changes', () => { prepareSystemHome() installCodexLikeGrantRunner() diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index 6da68c331ee..fbc6ded0fad 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -663,7 +663,7 @@ function cleanupLegacySystemManagedHooks(): void { // never replace that newer dotfiles generation with our stale parse. throw new Error('System Codex hooks changed during trust repair') } - writeHooksJson(hooksWritePath, { ...config, hooks: nextHooks }) + writeHooksJson(hooksWritePath, { ...config, hooks: nextHooks }, { preserveMode: true }) }, restoreHooks: () => writeFileAtomically(hooksWritePath, previousRaw, { mode: previousMode }) }) diff --git a/src/main/index.ts b/src/main/index.ts index 0a3bcab2848..43970bf11a8 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -1791,12 +1791,12 @@ app.whenReady().then(async () => { ) // Why: while the real-home lane owns ~/.codex/hooks.json, the legacy // system-home sweep inside managed installs would delete the entry the - // real-home installer just appended. Flag OFF or hooks off re-arms the sweep, - // which is also what removes the entry again on downgrade or opt-out. + // real-home installer just appended. Flag OFF, hooks off, or an incapable + // trust lane re-arms the sweep so downgrade, opt-out, and rollback converge. setSystemCodexHomeHookSweepSuppressed( () => codexRuntimeHome !== null && - codexRuntimeHome.isHostSystemDefaultRealHomeSelected() && + codexRuntimeHome.isHostSystemDefaultRealHome() && isAgentStatusHooksEnabled(store?.getSettings()) ) codexAccounts = new CodexAccountService(store, rateLimits, codexRuntimeHome) From f64fb139619e68fb55f5bd62d27b54ff6fa5970a Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 16 Jul 2026 01:18:29 -0700 Subject: [PATCH 03/10] fix(codex): narrow app-server unsupported detection --- config/tsconfig.cli.json | 1 + .../codex-app-server-capability-signal.ts | 7 +++++ .../codex/codex-app-server-client.test.ts | 31 +++++++++++++++++++ src/main/codex/codex-app-server-client.ts | 7 +---- 4 files changed, 40 insertions(+), 6 deletions(-) create mode 100644 src/main/codex/codex-app-server-capability-signal.ts diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index a4df2dab4a9..c2dff7a1c0d 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -12,6 +12,7 @@ "../src/main/claude/hook-settings.ts", "../src/main/claude/hook-service.ts", "../src/main/codex/codex-app-server-capability-cache.ts", + "../src/main/codex/codex-app-server-capability-signal.ts", "../src/main/codex/codex-app-server-client.ts", "../src/main/codex/codex-app-server-grant-bridge.ts", "../src/main/codex/codex-app-server-grant-envelope.ts", diff --git a/src/main/codex/codex-app-server-capability-signal.ts b/src/main/codex/codex-app-server-capability-signal.ts new file mode 100644 index 00000000000..a35518f0ded --- /dev/null +++ b/src/main/codex/codex-app-server-capability-signal.ts @@ -0,0 +1,7 @@ +// Why: only an argv parse failure that names app-server proves the RPC surface +// is absent; unrelated stderr must remain transient rather than poison the cache. +export function stderrIndicatesMissingAppServer(stderrTail: string): boolean { + return /^(?=.*\bapp-server\b).*(?:unrecognized subcommand|unexpected argument|invalid subcommand)/im.test( + stderrTail + ) +} diff --git a/src/main/codex/codex-app-server-client.test.ts b/src/main/codex/codex-app-server-client.test.ts index 5bb01ae5614..804cd50ae82 100644 --- a/src/main/codex/codex-app-server-client.test.ts +++ b/src/main/codex/codex-app-server-client.test.ts @@ -49,6 +49,11 @@ if (config.scenario === 'no-subcommand') { process.stderr.write("error: unrecognized subcommand 'app-server'\\n") process.exit(2) } +if (config.scenario === 'transient-stderr') { + process.stderr.write('config loader: unexpected argument in value\\n') + process.exit(1) +} +if (config.scenario === 'silent-exit-0') process.exit(0) process.stdin.setEncoding('utf8') process.stdin.on('data', (chunk) => { buffer += chunk @@ -278,6 +283,32 @@ describe('runCodexHookTrustGrantSession', () => { expect(isCodexAppServerUnsupportedError(error)).toBe(true) }) + it('does not classify unrelated stderr as a missing app-server capability', async () => { + const { request } = createStubRequest({ + scenario: 'transient-stderr', + hooks: [], + expectedTrustKeys: ['k'], + managedCommand: MANAGED_COMMAND + }) + + const error = await runCodexHookTrustGrantSession(request).catch((caught: unknown) => caught) + expect(error).toBeInstanceOf(Error) + expect(isCodexAppServerUnsupportedError(error)).toBe(false) + }) + + it('treats a silent exit zero before initialize as transient', async () => { + const { request } = createStubRequest({ + scenario: 'silent-exit-0', + hooks: [], + expectedTrustKeys: ['k'], + managedCommand: MANAGED_COMMAND + }) + + const error = await runCodexHookTrustGrantSession(request).catch((caught: unknown) => caught) + expect(error).toBeInstanceOf(Error) + expect(isCodexAppServerUnsupportedError(error)).toBe(false) + }) + it('kills a hung server at the session deadline', async () => { const keys = ['/home/a/.codex/hooks.json:session_start:0:0'] const { request, pidFile } = createStubRequest({ diff --git a/src/main/codex/codex-app-server-client.ts b/src/main/codex/codex-app-server-client.ts index ab5dced425c..4e58c8348cc 100644 --- a/src/main/codex/codex-app-server-client.ts +++ b/src/main/codex/codex-app-server-client.ts @@ -1,6 +1,7 @@ import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' import { normalizeHookTrustKeyForLookup } from './config-toml-trust' import { waitForProcessExitUntil } from './codex-process-exit-deadline' +import { stderrIndicatesMissingAppServer } from './codex-app-server-capability-signal' // Why: Codex gates hooks on a `trusted_hash` it computes from a private // canonical-JSON identity. Orca used to replicate that algorithm @@ -94,12 +95,6 @@ function isMethodNotFoundError(error: { code?: number; message?: string }): bool return error.code === JSON_RPC_METHOD_NOT_FOUND || /method not found/i.test(error.message ?? '') } -// Why: a CLI predating the app-server subcommand fails argv parsing before -// speaking any JSON-RPC; that shape is a capability signal, not a transient. -function stderrIndicatesMissingAppServer(stderrTail: string): boolean { - return /unrecognized subcommand|unexpected argument|invalid subcommand/i.test(stderrTail) -} - function collectHookListings(result: unknown): CodexHookListing[] { const data = result && typeof result === 'object' && Array.isArray((result as { data?: unknown }).data) From a04fa8259b0113fd6d5a306f63ea70f8b55748c3 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 16 Jul 2026 01:34:40 -0700 Subject: [PATCH 04/10] fix(codex): bound user hook trust rebase retries per host The rebase lane ran a codex app-server session on every launch prep while a host was stuck (CLI without app-server support, or keys hooks/list cannot match). Gate the transaction on the shared capability cache and add the same 5-minute transient cooldown the grant lane uses, so sweep and legacy-cleanup retries cost plain fs reads instead of a codex session per pane spawn. --- .../codex-user-hook-trust-rebase.test.ts | 67 +++++++++++++++++++ .../codex/codex-user-hook-trust-rebase.ts | 54 +++++++++++++-- 2 files changed, 115 insertions(+), 6 deletions(-) diff --git a/src/main/codex/codex-user-hook-trust-rebase.test.ts b/src/main/codex/codex-user-hook-trust-rebase.test.ts index 411af224969..6e409487d6d 100644 --- a/src/main/codex/codex-user-hook-trust-rebase.test.ts +++ b/src/main/codex/codex-user-hook-trust-rebase.test.ts @@ -8,6 +8,8 @@ import type { CodexUserHookTrustRebaseRequest } from './codex-user-hook-trust-re const resolveCodexCommandMock = vi.hoisted(() => vi.fn(() => process.execPath)) vi.mock('../codex-cli/command', () => ({ resolveCodexCommand: resolveCodexCommandMock })) +import { codexAppServerCapabilityCache } from './codex-app-server-capability-cache' +import { CodexAppServerUnsupportedError } from './codex-app-server-session' import { _internals, getMovedCodexUserHookTrust, @@ -26,6 +28,8 @@ beforeEach(() => { afterEach(() => { _internals.setSessionRunnerSync(null) + _internals.resetRetryState() + codexAppServerCapabilityCache.clear() rmSync(root, { recursive: true, force: true }) }) @@ -142,6 +146,69 @@ describe('real-home user hook trust rebasing', () => { } }) + it('marks the host unsupported and skips further codex sessions', () => { + const orca = command('orca-hook') + const user = command('user-hook') + const before = { Stop: [{ hooks: [orca] }, { hooks: [user] }] } + const after = { Stop: [{ hooks: [user] }] } + writeFileSync(configPath, '# config\n') + let sessions = 0 + _internals.setSessionRunnerSync(() => { + sessions += 1 + throw new CodexAppServerUnsupportedError('unrecognized subcommand app-server') + }) + const args = { + sourcePath: hooksPath, + runtimeHomePath: root, + tomlPath: configPath, + beforeHooks: before, + afterHooks: after, + writeHooks: () => { + throw new Error('write must not run') + }, + restoreHooks: () => { + throw new Error('restore must not run') + } + } + + expect(() => mutateRealHomeHooksPreservingUserTrust(args)).toThrow('unrecognized subcommand') + expect(() => mutateRealHomeHooksPreservingUserTrust(args)).toThrow('marked unsupported') + expect(sessions).toBe(1) + expect(codexAppServerCapabilityCache.shouldTry('native')).toBe(false) + }) + + it('cools down after a transient session failure instead of retrying every launch prep', () => { + const orca = command('orca-hook') + const user = command('user-hook') + const before = { Stop: [{ hooks: [orca] }, { hooks: [user] }] } + const after = { Stop: [{ hooks: [user] }] } + writeFileSync(configPath, '# config\n') + let sessions = 0 + _internals.setSessionRunnerSync(() => { + sessions += 1 + throw new Error('pre-mutation hooks/list reported 0 of 1 moved user hooks') + }) + const args = { + sourcePath: hooksPath, + runtimeHomePath: root, + tomlPath: configPath, + beforeHooks: before, + afterHooks: after, + writeHooks: () => { + throw new Error('write must not run') + }, + restoreHooks: () => { + throw new Error('restore must not run') + } + } + + expect(() => mutateRealHomeHooksPreservingUserTrust(args)).toThrow('0 of 1 moved user hooks') + expect(() => mutateRealHomeHooksPreservingUserTrust(args)).toThrow('cooling down') + expect(sessions).toBe(1) + // Why: a transient failure must not poison the shared capability signal. + expect(codexAppServerCapabilityCache.shouldTry('native')).toBe(true) + }) + it('restores both files byte-exactly when post-mutation repair fails', () => { const orca = command('orca-hook') const user = command('user-hook') diff --git a/src/main/codex/codex-user-hook-trust-rebase.ts b/src/main/codex/codex-user-hook-trust-rebase.ts index 5da7fa1d148..17ea73ab52a 100644 --- a/src/main/codex/codex-user-hook-trust-rebase.ts +++ b/src/main/codex/codex-user-hook-trust-rebase.ts @@ -1,5 +1,12 @@ import type { HookCommandConfig, HookDefinition } from '../agent-hooks/installer-utils' +import { + codexAppServerCapabilityCache, + getCodexAppServerHostKey, + type CodexAppServerHostKey +} from './codex-app-server-capability-cache' import { runCodexUserHookTrustRebaseSessionSync } from './codex-app-server-grant-bridge' +import { isCodexAppServerUnsupportedError } from './codex-app-server-session' +import { CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS } from './codex-hook-trust-grant' import { createCodexHookTrustEntry } from './codex-hook-identity' import { resolveCodexTrustGrantHost } from './codex-trust-grant-host' import { @@ -22,6 +29,19 @@ type RebaseSessionRunnerSync = ( let runSessionSync: RebaseSessionRunnerSync = runCodexUserHookTrustRebaseSessionSync +// Why: launch prep re-runs the callers on every pane spawn. A host stuck +// without a usable rebase lane (old CLI, unmatched keys) must not pay a codex +// session each time — bound retries like the grant lane does. +const rebaseRetryAfterByHost = new Map() + +function rememberRebaseSessionFailure(hostKey: CodexAppServerHostKey, error: unknown): void { + if (isCodexAppServerUnsupportedError(error)) { + codexAppServerCapabilityCache.rememberUnsupported(hostKey) + return + } + rebaseRetryAfterByHost.set(hostKey, Date.now() + CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS) +} + function entriesByHookObject( sourcePath: string, hooksByEvent: HooksByEvent @@ -115,6 +135,17 @@ export function mutateRealHomeHooksPreservingUserTrust(args: { args.writeHooks() return null } + const hostKey = getCodexAppServerHostKey({ kind: 'native' }) + if (!codexAppServerCapabilityCache.shouldTry(hostKey)) { + throw new Error('codex app-server is marked unsupported on this host; trust rebase skipped') + } + const retryAfterMs = rebaseRetryAfterByHost.get(hostKey) + if (retryAfterMs !== undefined) { + if (Date.now() < retryAfterMs) { + throw new Error('Codex user hook trust rebase is cooling down after a recent failure') + } + rebaseRetryAfterByHost.delete(hostKey) + } const snapshot = captureCodexTrustConfig(args.tomlPath) const baseRequest = resolveCodexTrustGrantHost({ kind: 'native' }).buildRequest({ @@ -124,12 +155,19 @@ export function mutateRealHomeHooksPreservingUserTrust(args: { }) // Why: inspection happens before the write, so an unavailable RPC aborts // without shifting a user's positional trust key. - const inspected: CodexUserHookTrustRebaseResult = runSessionSync({ - operation: 'inspect-user-hook-trust', - invocation: baseRequest.invocation, - hooksListCwd: baseRequest.hooksListCwd, - moves - }) + let inspected: CodexUserHookTrustRebaseResult + try { + inspected = runSessionSync({ + operation: 'inspect-user-hook-trust', + invocation: baseRequest.invocation, + hooksListCwd: baseRequest.hooksListCwd, + moves + }) + } catch (error) { + rememberRebaseSessionFailure(hostKey, error) + throw error + } + codexAppServerCapabilityCache.rememberSupported(hostKey) if (inspected.outcome !== 'inspected') { throw new Error('Unexpected Codex user hook trust inspection result') } @@ -149,6 +187,7 @@ export function mutateRealHomeHooksPreservingUserTrust(args: { } return snapshot } catch (error) { + rememberRebaseSessionFailure(hostKey, error) if (hooksWritten) { return rollbackMutation(args.restoreHooks, args.tomlPath, snapshot, error) } @@ -159,5 +198,8 @@ export function mutateRealHomeHooksPreservingUserTrust(args: { export const _internals = { setSessionRunnerSync(runner: RebaseSessionRunnerSync | null): void { runSessionSync = runner ?? runCodexUserHookTrustRebaseSessionSync + }, + resetRetryState(): void { + rebaseRetryAfterByHost.clear() } } From 69a85f56d2057f7dbcb25ddc2a7b4726e3034488 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 16 Jul 2026 01:45:35 -0700 Subject: [PATCH 05/10] fix(codex): establish real-home lane before cleanup --- src/main/index.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/index.ts b/src/main/index.ts index 43970bf11a8..d56d1c98609 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -2016,6 +2016,14 @@ app.whenReady().then(async () => { runtimeService.notifyEmulatorAutoAttachFromWatcher(worktreeId, info) }) nativeTheme.themeSource = store.getSettings().theme ?? 'system' + if (codexRuntimeHome.isHostSystemDefaultRealHomeSelected()) { + // Why: establish capability before managed-hook reconciliation so an + // incapable host re-arms and completes the legacy real-home sweep now. + ensureRealHomeCodexHookState({ + hooksEnabled: isAgentStatusHooksEnabled(store.getSettings()), + userDataPath: app.getPath('userData') + }) + } if (shouldInstallManagedHooks(is.dev)) { // Why: the persisted off switch must run before any auto-install path so // users who removed Orca-managed hooks do not see them silently reappear on launch. @@ -2025,14 +2033,6 @@ app.whenReady().then(async () => { removeManagedAgentHooks() } } - if (codexRuntimeHome.isHostSystemDefaultRealHomeSelected()) { - // Why: establish the lane before background rate-limit polling starts, so - // an incapable grant host never polls a home its PTYs will not use. - ensureRealHomeCodexHookState({ - hooksEnabled: isAgentStatusHooksEnabled(store.getSettings()), - userDataPath: app.getPath('userData') - }) - } app.on('child-process-gone', (_event, details) => { recordProcessGoneCrash('child', details.type, details.reason, details.exitCode ?? null, { name: details.name, From 0b772fe90d697ed6c94f273643cc12c6bdfaaf9b Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 16 Jul 2026 01:58:11 -0700 Subject: [PATCH 06/10] fix(codex): protect symlinked rolling backups --- config/tsconfig.cli.json | 1 + src/main/agent-hooks/installer-utils.test.ts | 16 ++++++++++++ src/main/agent-hooks/installer-utils.ts | 4 +-- src/main/codex/config-toml-trust.test.ts | 24 +++++++++++++++++ src/main/codex/config-toml-trust.ts | 5 ++-- src/main/rolling-file-backup.ts | 27 ++++++++++++++++++++ 6 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 src/main/rolling-file-backup.ts diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index c97f1f4d805..49c8d52b4a9 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -52,6 +52,7 @@ "../src/main/kimi/hook-service.ts", "../src/main/kimi/kimi-hook-config-toml.ts", "../src/main/openclaude/hook-service.ts", + "../src/main/rolling-file-backup.ts", "../src/main/runtime/runtime-metadata.ts", "../src/main/win32-utils.ts" ], diff --git a/src/main/agent-hooks/installer-utils.test.ts b/src/main/agent-hooks/installer-utils.test.ts index 915e7ad6bdb..7fac297ae86 100644 --- a/src/main/agent-hooks/installer-utils.test.ts +++ b/src/main/agent-hooks/installer-utils.test.ts @@ -95,6 +95,22 @@ describe('writeHooksJson', () => { expect(bak).toEqual(original) }) + it('does not follow an existing .bak symlink', () => { + const original = '{"hooks":{}}\n' + const backupTarget = join(tmpDir, 'dotfiles-backup.json') + writeFileSync(configPath, original, 'utf-8') + writeFileSync(backupTarget, 'pristine backup target\n', 'utf-8') + symlinkSync(backupTarget, `${configPath}.bak`) + + expect(() => writeHooksJson(configPath, { hooks: { Stop: [] } })).toThrow( + 'Refusing to overwrite symlinked backup' + ) + + expect(readFileSync(configPath, 'utf-8')).toBe(original) + expect(lstatSync(`${configPath}.bak`).isSymbolicLink()).toBe(true) + expect(readFileSync(backupTarget, 'utf-8')).toBe('pristine backup target\n') + }) + it('does not create a .bak file when the config does not yet exist', () => { writeHooksJson(configPath, {}) expect(existsSync(`${configPath}.bak`)).toBe(false) diff --git a/src/main/agent-hooks/installer-utils.ts b/src/main/agent-hooks/installer-utils.ts index 8a99affaddb..63cadabe534 100644 --- a/src/main/agent-hooks/installer-utils.ts +++ b/src/main/agent-hooks/installer-utils.ts @@ -5,7 +5,6 @@ import { statSync, writeFileSync, chmodSync, - copyFileSync, renameSync, unlinkSync } from 'node:fs' @@ -16,6 +15,7 @@ import type { AgentHookSource } from '../../shared/agent-hook-relay' import { grantDirAcl, isPermissionError } from '../win32-utils' import { POSIX_HOOK_STDIN_DRAIN_COMMAND } from './hook-stdin-contract' import { resolveHooksJsonWritePath } from './hook-config-write-path' +import { writeRollingFileBackup } from '../rolling-file-backup' export type HookCommandConfig = { type: 'command' @@ -400,7 +400,7 @@ export function writeHooksJson( // Protects against a merge-logic bug producing bad JSON; the original is // always recoverable from .bak until the next write. if (existsSync(writePath)) { - copyFileSync(writePath, `${writePath}.bak`) + writeRollingFileBackup(writePath, `${writePath}.bak`) } renameSync(tmpPath, writePath) } finally { diff --git a/src/main/codex/config-toml-trust.test.ts b/src/main/codex/config-toml-trust.test.ts index 26ccd96cd33..91a548d6388 100644 --- a/src/main/codex/config-toml-trust.test.ts +++ b/src/main/codex/config-toml-trust.test.ts @@ -501,6 +501,30 @@ describe('upsertHookTrustEntries', () => { expect(readFileSync(`${configPath}.bak`, 'utf-8')).toBe('model = "old"\n') }) + it('does not follow an existing .bak symlink', () => { + const original = 'model = "old"\n' + const backupTarget = join(tmpDir, 'dotfiles-config-backup.toml') + writeFileSync(configPath, original, 'utf-8') + writeFileSync(backupTarget, 'pristine backup target\n', 'utf-8') + symlinkSync(backupTarget, `${configPath}.bak`) + + expect(() => + upsertHookTrustEntries(configPath, [ + { + sourcePath: '/x/hooks.json', + eventLabel: 'pre_tool_use', + groupIndex: 0, + handlerIndex: 0, + command: 'echo' + } + ]) + ).toThrow('Refusing to overwrite symlinked backup') + + expect(readFileSync(configPath, 'utf-8')).toBe(original) + expect(lstatSync(`${configPath}.bak`).isSymbolicLink()).toBe(true) + expect(readFileSync(backupTarget, 'utf-8')).toBe('pristine backup target\n') + }) + it('does not write at all when the file already has the right hash', () => { const entry: CodexTrustEntry = { sourcePath: '/x/hooks.json', diff --git a/src/main/codex/config-toml-trust.ts b/src/main/codex/config-toml-trust.ts index 4eb06612f33..f2f7496aada 100644 --- a/src/main/codex/config-toml-trust.ts +++ b/src/main/codex/config-toml-trust.ts @@ -11,8 +11,9 @@ import { } from 'node:fs' import { dirname, join } from 'node:path' import { createHash, randomUUID } from 'node:crypto' -import { copyFileWithWindowsRetry, renameFileWithWindowsRetry } from '../codex-accounts/fs-utils' +import { renameFileWithWindowsRetry } from '../codex-accounts/fs-utils' import { foldWslUncPathCaseInsensitiveParts } from '../../shared/wsl-paths' +import { writeRollingFileBackup } from '../rolling-file-backup' import { createTomlLineScanState, isTomlStructuralLine, @@ -806,7 +807,7 @@ export function writeConfigAtomically(configPath: string, contents: string): voi // restrictive permissions when the atomic rename installs new bytes. writeFileSync(tmpPath, contents, { encoding: 'utf-8', mode: existingMode }) if (existsSync(writePath)) { - copyFileWithWindowsRetry(writePath, `${writePath}.bak`) + writeRollingFileBackup(writePath, `${writePath}.bak`) } renameFileWithWindowsRetry(tmpPath, writePath) renamed = true diff --git a/src/main/rolling-file-backup.ts b/src/main/rolling-file-backup.ts new file mode 100644 index 00000000000..45f2366b8c6 --- /dev/null +++ b/src/main/rolling-file-backup.ts @@ -0,0 +1,27 @@ +import { randomUUID } from 'node:crypto' +import { lstatSync, rmSync } from 'node:fs' +import { copyFileWithWindowsRetry, renameFileWithWindowsRetry } from './codex-accounts/fs-utils' + +export function writeRollingFileBackup(sourcePath: string, backupPath: string): void { + try { + if (lstatSync(backupPath).isSymbolicLink()) { + // Why: copyFile follows a destination symlink and can corrupt an + // unrelated dotfiles target before the primary atomic write begins. + throw new Error(`Refusing to overwrite symlinked backup: ${backupPath}`) + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error + } + } + + const tempPath = `${backupPath}.${process.pid}.${randomUUID()}.tmp` + try { + // Why: replacing the backup from a fresh inode cannot mutate another file + // through a hard link, and a failed rename leaves the prior backup intact. + copyFileWithWindowsRetry(sourcePath, tempPath) + renameFileWithWindowsRetry(tempPath, backupPath) + } finally { + rmSync(tempPath, { force: true }) + } +} From 1374d311bd86a8883e7060ce7b65f5247cc8ace4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 16 Jul 2026 12:22:11 -0700 Subject: [PATCH 07/10] fix(codex): preserve hook symlink trust path --- src/main/codex/config-toml-trust.test.ts | 19 ++++++++++++++++++- src/main/codex/config-toml-trust.ts | 13 +++++++------ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/main/codex/config-toml-trust.test.ts b/src/main/codex/config-toml-trust.test.ts index 91a548d6388..bc5f847b629 100644 --- a/src/main/codex/config-toml-trust.test.ts +++ b/src/main/codex/config-toml-trust.test.ts @@ -14,7 +14,7 @@ import { writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { dirname, join } from 'node:path' import { escapeRegex } from '../../shared/string-utils' import { computeTrustKey, @@ -294,6 +294,23 @@ describe('computeTrustKey', () => { ).toBe(`${realpathSync.native(hooksPath)}:user_prompt_submit:0:0`) }) + it('preserves a hooks.json leaf symlink in the trust key', () => { + const hooksPath = join(tmpDir, 'hooks.json') + const targetPath = join(tmpDir, 'dotfiles-hooks.json') + writeFileSync(targetPath, '{"hooks":{}}\n', 'utf-8') + symlinkSync(targetPath, hooksPath) + + expect( + computeTrustKey({ + sourcePath: hooksPath, + eventLabel: 'stop', + groupIndex: 0, + handlerIndex: 0, + command: 'irrelevant' + }) + ).toBe(`${join(realpathSync.native(dirname(hooksPath)), 'hooks.json')}:stop:0:0`) + }) + it('uses native Windows backslashes in the trust key Codex looks up', () => { // Why: Codex 0.140 writes approved Windows hook trust keys as raw native // paths under [hooks.state]. diff --git a/src/main/codex/config-toml-trust.ts b/src/main/codex/config-toml-trust.ts index f2f7496aada..87396638300 100644 --- a/src/main/codex/config-toml-trust.ts +++ b/src/main/codex/config-toml-trust.ts @@ -9,7 +9,7 @@ import { unlinkSync, writeFileSync } from 'node:fs' -import { dirname, join } from 'node:path' +import { basename, dirname, join } from 'node:path' import { createHash, randomUUID } from 'node:crypto' import { renameFileWithWindowsRetry } from '../codex-accounts/fs-utils' import { foldWslUncPathCaseInsensitiveParts } from '../../shared/wsl-paths' @@ -169,12 +169,13 @@ export function computeTrustKey(entry: CodexTrustEntry): string { } export function getCodexCanonicalTrustPath(sourcePath: string): string { + if (process.platform !== 'win32' && usesWindowsPathSeparators(sourcePath)) { + return normalizeWindowsPathForCodexLookup(sourcePath) + } try { - // Why: Codex canonicalizes trust paths before building config keys. On - // macOS, /var is a symlink to /private/var; trusting the raw path still - // leaves the TUI in review/trust prompts. On Windows, Codex 0.140 writes - // hook state keys with native raw backslashes under an explicit parent table. - return realpathSync.native(sourcePath) + // Why: Codex canonicalizes parent directories but preserves a hooks.json + // leaf symlink in trust keys, so resolving the whole path misses its RPC listing. + return join(realpathSync.native(dirname(sourcePath)), basename(sourcePath)) } catch { return normalizeWindowsPathForCodexLookup(sourcePath) } From e33551d215237ed839211dd49b7bb533f69a992c Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 16 Jul 2026 12:33:32 -0700 Subject: [PATCH 08/10] fix(codex): disambiguate POSIX trust paths --- src/main/codex/codex-hook-trust-grant.test.ts | 4 ++++ src/main/codex/config-toml-trust.test.ts | 10 ++++++++++ src/main/codex/config-toml-trust.ts | 14 ++++++++------ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/main/codex/codex-hook-trust-grant.test.ts b/src/main/codex/codex-hook-trust-grant.test.ts index 5f184355b58..cebe6ad20b6 100644 --- a/src/main/codex/codex-hook-trust-grant.test.ts +++ b/src/main/codex/codex-hook-trust-grant.test.ts @@ -31,6 +31,10 @@ let previousUserDataPath: string | undefined beforeEach(() => { userDataDir = mkdtempSync(join(tmpdir(), 'orca-trust-grant-userdata-')) runtimeHomeDir = join(userDataDir, 'codex-runtime-home', 'home') + // Why: production writes hooks.json before granting trust; keeping that + // ordering prevents test-only canonical-path drift during ledger setup. + mkdirSync(runtimeHomeDir, { recursive: true }) + writeFileSync(join(runtimeHomeDir, 'hooks.json'), '{"hooks":{}}\n', 'utf-8') previousUserDataPath = process.env.ORCA_USER_DATA_PATH process.env.ORCA_USER_DATA_PATH = userDataDir codexAppServerCapabilityCache.clear() diff --git a/src/main/codex/config-toml-trust.test.ts b/src/main/codex/config-toml-trust.test.ts index bc5f847b629..9e8ae78cdc0 100644 --- a/src/main/codex/config-toml-trust.test.ts +++ b/src/main/codex/config-toml-trust.test.ts @@ -311,6 +311,16 @@ describe('computeTrustKey', () => { ).toBe(`${join(realpathSync.native(dirname(hooksPath)), 'hooks.json')}:stop:0:0`) }) + it.skipIf(process.platform === 'win32')( + 'canonicalizes an existing POSIX path with two leading slashes', + () => { + const hooksPath = `/${join(tmpDir, 'hooks.json')}` + writeFileSync(hooksPath, '{"hooks":{}}\n', 'utf-8') + + expect(getCodexCanonicalTrustPath(hooksPath)).toBe(realpathSync.native(hooksPath)) + } + ) + it('uses native Windows backslashes in the trust key Codex looks up', () => { // Why: Codex 0.140 writes approved Windows hook trust keys as raw native // paths under [hooks.state]. diff --git a/src/main/codex/config-toml-trust.ts b/src/main/codex/config-toml-trust.ts index 87396638300..2e40f49895d 100644 --- a/src/main/codex/config-toml-trust.ts +++ b/src/main/codex/config-toml-trust.ts @@ -169,7 +169,9 @@ export function computeTrustKey(entry: CodexTrustEntry): string { } export function getCodexCanonicalTrustPath(sourcePath: string): string { - if (process.platform !== 'win32' && usesWindowsPathSeparators(sourcePath)) { + if (process.platform !== 'win32' && isWindowsDriveOrBackslashUncPath(sourcePath)) { + // Why: an existing `//` path is valid on POSIX, so let realpath distinguish + // it from forward-slash UNC before falling back to Windows normalization. return normalizeWindowsPathForCodexLookup(sourcePath) } try { @@ -206,11 +208,11 @@ function normalizeWindowsPathSeparators(sourcePath: string): string { } function usesWindowsPathSeparators(sourcePath: string): boolean { - return ( - /^[A-Za-z]:[\\/]/.test(sourcePath) || - sourcePath.startsWith('\\\\') || - sourcePath.startsWith('//') - ) + return isWindowsDriveOrBackslashUncPath(sourcePath) || sourcePath.startsWith('//') +} + +function isWindowsDriveOrBackslashUncPath(sourcePath: string): boolean { + return /^[A-Za-z]:[\\/]/.test(sourcePath) || sourcePath.startsWith('\\\\') } // Why: Codex and Orca can disagree on quote style, separators, and casing for From 8630eb286ad2ea1896f60d135df8bdf5985edfca Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 16 Jul 2026 12:56:24 -0700 Subject: [PATCH 09/10] fix(codex): align hook trust source paths --- .../codex/codex-app-server-session.test.ts | 41 +++++++++ src/main/codex/codex-app-server-session.ts | 10 ++- src/main/codex/codex-hook-trust-grant.test.ts | 13 +++ src/main/codex/codex-hook-trust-grant.ts | 5 +- .../codex-managed-trust-reconciliation.ts | 13 ++- .../codex-real-home-hook-install.test.ts | 19 ++++ .../codex/codex-real-home-hook-install.ts | 3 +- src/main/codex/codex-trust-grant-host.ts | 6 +- .../codex-user-hook-trust-rebase.test.ts | 1 + .../codex/codex-user-hook-trust-rebase.ts | 3 +- .../codex/config-toml-trust-paths.test.ts | 80 +++++++++++++++++ src/main/codex/config-toml-trust.test.ts | 29 ++++-- src/main/codex/config-toml-trust.ts | 90 ++++++++++++++----- src/main/codex/hook-service.test.ts | 45 +++++----- src/main/codex/hook-service.ts | 23 +++-- src/main/codex/hook-trust-promotion.test.ts | 32 ++++++- src/main/codex/hook-trust-promotion.ts | 18 ++-- 17 files changed, 352 insertions(+), 79 deletions(-) create mode 100644 src/main/codex/codex-app-server-session.test.ts create mode 100644 src/main/codex/config-toml-trust-paths.test.ts diff --git a/src/main/codex/codex-app-server-session.test.ts b/src/main/codex/codex-app-server-session.test.ts new file mode 100644 index 00000000000..c953237033e --- /dev/null +++ b/src/main/codex/codex-app-server-session.test.ts @@ -0,0 +1,41 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { runCodexAppServerSession } from './codex-app-server-session' + +const originalCodexHome = process.env.CODEX_HOME + +afterEach(() => { + if (originalCodexHome === undefined) { + delete process.env.CODEX_HOME + } else { + process.env.CODEX_HOME = originalCodexHome + } +}) + +describe('runCodexAppServerSession environment', () => { + it('removes inherited variables requested by a default-home invocation', async () => { + process.env.CODEX_HOME = '/tmp/inherited-managed-home' + const server = String.raw` + 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 = message.method === 'env/get' + ? { codexHome: process.env.CODEX_HOME ?? null } + : {} + process.stdout.write(JSON.stringify({ id: message.id, result }) + '\n') + }) + ` + + const result = await runCodexAppServerSession( + { + command: process.execPath, + args: ['-e', server], + envToDelete: ['CODEX_HOME'], + timeoutMs: 5_000 + }, + (requestRpc) => requestRpc('env/get') + ) + + expect(result).toEqual({ codexHome: null }) + }) +}) diff --git a/src/main/codex/codex-app-server-session.ts b/src/main/codex/codex-app-server-session.ts index 6798792de70..b73f2f46537 100644 --- a/src/main/codex/codex-app-server-session.ts +++ b/src/main/codex/codex-app-server-session.ts @@ -6,6 +6,8 @@ export type CodexAppServerInvocation = { args: string[] /** Overlay applied on top of the inherited environment (for example, CODEX_HOME). */ env?: Record + /** Variables that would make this child select a different runtime than its caller. */ + envToDelete?: readonly string[] /** Whole-session deadline; the Codex child is killed when it lapses. */ timeoutMs: number } @@ -52,8 +54,14 @@ export async function runCodexAppServerSession( run: (requestRpc: CodexAppServerRequestRpc) => Promise, spawnImpl: typeof spawn = spawn ): Promise { + const childEnv = { ...process.env, ...invocation.env } + // Why: default-home RPCs must observe the same missing CODEX_HOME as the + // pane whose hook keys they inspect; omission alone would inherit Orca's value. + for (const key of invocation.envToDelete ?? []) { + delete childEnv[key] + } const child = spawnImpl(invocation.command, invocation.args, { - env: { ...process.env, ...invocation.env }, + env: childEnv, stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true }) as ChildProcessWithoutNullStreams diff --git a/src/main/codex/codex-hook-trust-grant.test.ts b/src/main/codex/codex-hook-trust-grant.test.ts index cebe6ad20b6..59f2807605d 100644 --- a/src/main/codex/codex-hook-trust-grant.test.ts +++ b/src/main/codex/codex-hook-trust-grant.test.ts @@ -120,6 +120,19 @@ describe('grantManagedCodexHookTrust', () => { expect(getCodexTrustGrantDiagnostics()).toMatchObject({ granted: 1, fellBack: 0 }) }) + it('builds a default-home grant invocation without an inherited CODEX_HOME', () => { + const entries = [managedEntry('stop')] + const runner = vi.fn((_request: CodexHookTrustGrantRequest) => grantedSessionResult(entries)) + _internals.setGrantSessionRunnerSync(runner) + + expect( + grantManagedCodexHookTrust({ ...buildPlan(entries), useDefaultCodexHome: true }) + ).toMatchObject({ lane: 'rpc' }) + const invocation = runner.mock.calls[0]![0]!.invocation + expect(invocation.env?.CODEX_HOME).toBeUndefined() + expect(invocation.envToDelete).toContain('CODEX_HOME') + }) + it('skips the RPC session while the ledger grant still holds, and re-grants on config drift', () => { const entries = [managedEntry('session_start')] const runner = vi.fn(() => grantedSessionResult(entries)) diff --git a/src/main/codex/codex-hook-trust-grant.ts b/src/main/codex/codex-hook-trust-grant.ts index b66c63b65cb..322d4b39116 100644 --- a/src/main/codex/codex-hook-trust-grant.ts +++ b/src/main/codex/codex-hook-trust-grant.ts @@ -44,6 +44,8 @@ export type CodexManagedTrustGrantPlan = { /** Managed trust identities Orca just wrote (no trustedHash). */ managedEntries: readonly CodexTrustEntry[] host: CodexTrustGrantHost + /** Match a pane where CODEX_HOME is absent instead of an explicit managed home. */ + useDefaultCodexHome?: boolean } export type CodexTrustGrantFallbackReason = @@ -226,7 +228,8 @@ export function grantManagedCodexHookTrust( resolvedHost.buildRequest({ runtimeHomePath: plan.runtimeHomePath, managedCommand: plan.managedCommand, - expectedTrustKeys: expected.map(({ normalizedKey }) => normalizedKey) + expectedTrustKeys: expected.map(({ normalizedKey }) => normalizedKey), + useDefaultCodexHome: plan.useDefaultCodexHome }) ) } catch (error) { diff --git a/src/main/codex/codex-managed-trust-reconciliation.ts b/src/main/codex/codex-managed-trust-reconciliation.ts index e422f2a4c32..b6c9e9d5d3a 100644 --- a/src/main/codex/codex-managed-trust-reconciliation.ts +++ b/src/main/codex/codex-managed-trust-reconciliation.ts @@ -1,8 +1,9 @@ import { computeTrustKey, computeTrustedHash, - getCodexCanonicalTrustPath, + getCodexExplicitHomeHookSourcePath, normalizeHookTrustKeyForLookup, + normalizeCodexHookSourcePath, parseTrustKey, readHookTrustEntries, removeHookTrustEntries, @@ -58,22 +59,26 @@ export function removeCodexManagedHookTrustEntries(options: { command: string managedEventLabels: ReadonlySet timeoutSec: number + /** Explicit native homes resolve their parent before hook discovery. */ + sourceUsesExplicitCodexHome?: boolean }): void { const existingEntries = readHookTrustEntries(options.tomlPath) const ledgerHome = readCodexTrustGrantLedgerHomeForReconciliation(options.runtimeHomePath) - const canonicalSourcePath = getCodexCanonicalTrustPath(options.sourcePath) + const expectedSourcePath = options.sourceUsesExplicitCodexHome + ? getCodexExplicitHomeHookSourcePath(options.sourcePath) + : normalizeCodexHookSourcePath(options.sourcePath) const ownedKeys: string[] = [] for (const [key, state] of existingEntries) { const parts = parseTrustKey(key) if ( !parts || - getCodexCanonicalTrustPath(parts.sourcePath) !== canonicalSourcePath || + normalizeCodexHookSourcePath(parts.sourcePath) !== expectedSourcePath || !options.managedEventLabels.has(parts.eventLabel) ) { continue } const expectedEntry: CodexTrustEntry = { - sourcePath: options.sourcePath, + sourcePath: expectedSourcePath, eventLabel: parts.eventLabel, groupIndex: parts.groupIndex, handlerIndex: parts.handlerIndex, diff --git a/src/main/codex/codex-real-home-hook-install.test.ts b/src/main/codex/codex-real-home-hook-install.test.ts index b49dcf691ea..efc91cd5e2a 100644 --- a/src/main/codex/codex-real-home-hook-install.test.ts +++ b/src/main/codex/codex-real-home-hook-install.test.ts @@ -116,9 +116,28 @@ describe('ensureRealHomeCodexHookState (install)', () => { const plan = grantMock.mock.calls[0]![0] as CodexManagedTrustGrantPlan expect(plan.runtimeHomePath).toBe(join(fakeHomeDir, '.codex')) expect(plan.host).toEqual({ kind: 'native' }) + expect(plan.useDefaultCodexHome).toBe(true) expect(plan.managedEntries.every((entry) => entry.groupIndex === 0)).toBe(true) }) + it('keeps a symlinked default home logical in the keys sent to Codex', () => { + grantSucceeds() + const logicalHome = join(fakeHomeDir, '.codex') + const targetHome = join(fakeHomeDir, 'dotfiles-codex') + rmSync(logicalHome, { recursive: true }) + mkdirSync(targetHome) + symlinkSync(targetHome, logicalHome) + + expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( + 'installed' + ) + + const plan = grantMock.mock.calls[0]![0] as CodexManagedTrustGrantPlan + expect( + plan.managedEntries.map(computeTrustKey).every((key) => key.startsWith(logicalHome)) + ).toBe(true) + }) + it('keeps the managed lane for unknown top-level fields Codex cannot load', () => { grantSucceeds() const userConfig = { diff --git a/src/main/codex/codex-real-home-hook-install.ts b/src/main/codex/codex-real-home-hook-install.ts index 8e122f69812..30c0704734a 100644 --- a/src/main/codex/codex-real-home-hook-install.ts +++ b/src/main/codex/codex-real-home-hook-install.ts @@ -193,7 +193,8 @@ function installRealHomeCodexHook(userDataPath: string): RealHomeCodexHookLane { tomlPath: getRealHomeConfigTomlPath(), managedCommand: material.command, managedEntries, - host: { kind: 'native' } + host: { kind: 'native' }, + useDefaultCodexHome: true }) if (grant.lane === 'rpc') { return 'installed' diff --git a/src/main/codex/codex-trust-grant-host.ts b/src/main/codex/codex-trust-grant-host.ts index 63cd81eb425..0c0a97381df 100644 --- a/src/main/codex/codex-trust-grant-host.ts +++ b/src/main/codex/codex-trust-grant-host.ts @@ -23,6 +23,7 @@ type CodexTrustGrantRequestInput = { runtimeHomePath: string managedCommand: string expectedTrustKeys: string[] + useDefaultCodexHome?: boolean } export type ResolvedCodexTrustGrantHost = { @@ -54,11 +55,14 @@ export function resolveCodexTrustGrantHost(host: CodexTrustGrantHost): ResolvedC binaryStamp: command === 'codex' ? null : buildNativeCodexBinaryStamp(command), buildRequest: (input) => { const { spawnCmd, spawnArgs } = getSpawnArgsForWindows(command, ['app-server']) + const useDefaultCodexHome = input.useDefaultCodexHome === true return { invocation: { command: spawnCmd, args: spawnArgs, - env: { CODEX_HOME: input.runtimeHomePath }, + ...(useDefaultCodexHome + ? { envToDelete: ['CODEX_HOME'] } + : { env: { CODEX_HOME: input.runtimeHomePath } }), timeoutMs: NATIVE_GRANT_TIMEOUT_MS }, hooksListCwd: input.runtimeHomePath, diff --git a/src/main/codex/codex-user-hook-trust-rebase.test.ts b/src/main/codex/codex-user-hook-trust-rebase.test.ts index 6e409487d6d..89f25176d50 100644 --- a/src/main/codex/codex-user-hook-trust-rebase.test.ts +++ b/src/main/codex/codex-user-hook-trust-rebase.test.ts @@ -136,6 +136,7 @@ describe('real-home user hook trust rebasing', () => { }) expect(requests).toHaveLength(2) + expect(requests[0]?.invocation.envToDelete).toContain('CODEX_HOME') const repair = requests[1] expect(repair?.operation).toBe('repair-user-hook-trust') if (repair?.operation === 'repair-user-hook-trust') { diff --git a/src/main/codex/codex-user-hook-trust-rebase.ts b/src/main/codex/codex-user-hook-trust-rebase.ts index 17ea73ab52a..0020e106c2f 100644 --- a/src/main/codex/codex-user-hook-trust-rebase.ts +++ b/src/main/codex/codex-user-hook-trust-rebase.ts @@ -151,7 +151,8 @@ export function mutateRealHomeHooksPreservingUserTrust(args: { const baseRequest = resolveCodexTrustGrantHost({ kind: 'native' }).buildRequest({ runtimeHomePath: args.runtimeHomePath, managedCommand: '', - expectedTrustKeys: [] + expectedTrustKeys: [], + useDefaultCodexHome: true }) // Why: inspection happens before the write, so an unavailable RPC aborts // without shifting a user's positional trust key. diff --git a/src/main/codex/config-toml-trust-paths.test.ts b/src/main/codex/config-toml-trust-paths.test.ts new file mode 100644 index 00000000000..6aa1ced1b81 --- /dev/null +++ b/src/main/codex/config-toml-trust-paths.test.ts @@ -0,0 +1,80 @@ +import { resolve } from 'node:path' +import { describe, expect, it } from 'vitest' +import { + computeTrustKey, + normalizeCodexHookSourcePath, + normalizeHookTrustKeyForLookup, + type CodexTrustEntry +} from './config-toml-trust' + +function entry(sourcePath: string): CodexTrustEntry { + return { + sourcePath, + eventLabel: 'stop', + groupIndex: 2, + handlerIndex: 3, + command: 'irrelevant' + } +} + +describe('Codex hook trust source path normalization', () => { + it.each([ + ['C:/Users/Rod/.codex/./hooks.json', 'C:\\Users\\Rod\\.codex\\hooks.json'], + ['C:\\Users\\Rod\\tmp\\..\\.codex\\hooks.json', 'C:\\Users\\Rod\\.codex\\hooks.json'], + ['C:\\Users\\Rod\\.codex\\hooks.json\\', 'C:\\Users\\Rod\\.codex\\hooks.json'], + ['\\\\server\\share\\dir\\.\\hooks.json', '\\\\server\\share\\dir\\hooks.json'], + ['\\\\?\\C:\\Users\\Rod\\.codex\\hooks.json', 'C:\\Users\\Rod\\.codex\\hooks.json'], + ['\\\\?\\UNC\\server\\share\\dir\\hooks.json', '\\\\server\\share\\dir\\hooks.json'], + ['\\\\.\\C:\\Users\\Rod\\.codex\\hooks.json', 'C:\\Users\\Rod\\.codex\\hooks.json'], + ['\\\\.\\UNC\\server\\share\\dir\\hooks.json', '\\\\server\\share\\dir\\hooks.json'] + ])('matches Windows PathBuf display semantics for %s', (sourcePath, expected) => { + expect(normalizeCodexHookSourcePath(sourcePath)).toBe(expected) + }) + + it('uses native semantics for the otherwise ambiguous forward-slash UNC form', () => { + expect(normalizeCodexHookSourcePath('//server/share/dir/hooks.json')).toBe( + process.platform === 'win32' + ? '\\\\server\\share\\dir\\hooks.json' + : '/server/share/dir/hooks.json' + ) + }) + + it('normalizes POSIX doubles, dot segments, repeated separators, and trailing slashes', () => { + expect(normalizeCodexHookSourcePath('//tmp/a//b/../hooks.json/')).toBe('/tmp/a/hooks.json') + }) + + it('resolves relative native paths to the absolute discovery shape', () => { + expect(normalizeCodexHookSourcePath('relative/../hooks.json')).toBe(resolve('hooks.json')) + }) + + it('preserves POSIX case, literal backslashes, spaces, and Unicode bytes', () => { + const sourcePath = '/tmp/Üser Name/e\u0301\\literal/Hooks.JSON' + expect(normalizeCodexHookSourcePath(sourcePath)).toBe(sourcePath) + }) + + it('preserves Windows drive-letter and component casing in the lexical path', () => { + expect(normalizeCodexHookSourcePath('d:/Üser Name/Codex/Hooks.JSON')).toBe( + 'd:\\Üser Name\\Codex\\Hooks.JSON' + ) + }) + + it('builds the key byte-for-byte from the normalized source and positional suffix', () => { + expect(computeTrustKey(entry('C:/Users/Rod/.codex/../.codex/hooks.json'))).toBe( + 'C:\\Users\\Rod\\.codex\\hooks.json:stop:2:3' + ) + }) +}) + +describe('Codex hook trust key lookup normalization', () => { + it('folds Windows separator and case variants without changing the persisted key', () => { + const native = 'C:\\Users\\Rod\\.codex\\hooks.json:stop:2:3' + const slash = 'c:/users/rod/.codex/hooks.json:stop:2:3' + expect(normalizeHookTrustKeyForLookup(native)).toBe(normalizeHookTrustKeyForLookup(slash)) + }) + + it('keeps POSIX paths case-sensitive', () => { + const upper = '/home/Üser/.codex/hooks.json:stop:2:3' + const lower = '/home/üser/.codex/hooks.json:stop:2:3' + expect(normalizeHookTrustKeyForLookup(upper)).not.toBe(normalizeHookTrustKeyForLookup(lower)) + }) +}) diff --git a/src/main/codex/config-toml-trust.test.ts b/src/main/codex/config-toml-trust.test.ts index 9e8ae78cdc0..e36e7a1a8d7 100644 --- a/src/main/codex/config-toml-trust.test.ts +++ b/src/main/codex/config-toml-trust.test.ts @@ -14,13 +14,14 @@ import { writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' -import { dirname, join } from 'node:path' +import { join } from 'node:path' import { escapeRegex } from '../../shared/string-utils' import { computeTrustKey, computeTrustedHash, escapeTomlString, - getCodexCanonicalTrustPath, + getCodexExplicitHomeHookSourcePath, + normalizeCodexHookSourcePath, normalizeCodexProjectPathForLookup, normalizeCodexProjectPathForRevocationLookup, parseTrustKey, @@ -277,7 +278,7 @@ describe('computeTrustKey', () => { ).toBe('/Users/thebr/.codex/hooks.json:pre_tool_use:0:0') }) - it('uses Codex canonicalized source paths when hooks.json exists', () => { + it('lexically normalizes source paths without resolving default-home aliases', () => { const nestedDir = join(tmpDir, 'nested') mkdirSync(nestedDir) const hooksPath = join(nestedDir, '..', 'hooks.json') @@ -291,7 +292,7 @@ describe('computeTrustKey', () => { handlerIndex: 0, command: 'irrelevant' }) - ).toBe(`${realpathSync.native(hooksPath)}:user_prompt_submit:0:0`) + ).toBe(`${join(tmpDir, 'hooks.json')}:user_prompt_submit:0:0`) }) it('preserves a hooks.json leaf symlink in the trust key', () => { @@ -308,7 +309,7 @@ describe('computeTrustKey', () => { handlerIndex: 0, command: 'irrelevant' }) - ).toBe(`${join(realpathSync.native(dirname(hooksPath)), 'hooks.json')}:stop:0:0`) + ).toBe(`${hooksPath}:stop:0:0`) }) it.skipIf(process.platform === 'win32')( @@ -317,7 +318,7 @@ describe('computeTrustKey', () => { const hooksPath = `/${join(tmpDir, 'hooks.json')}` writeFileSync(hooksPath, '{"hooks":{}}\n', 'utf-8') - expect(getCodexCanonicalTrustPath(hooksPath)).toBe(realpathSync.native(hooksPath)) + expect(normalizeCodexHookSourcePath(hooksPath)).toBe(join(tmpDir, 'hooks.json')) } ) @@ -339,10 +340,24 @@ describe('computeTrustKey', () => { it('preserves literal backslashes in non-Windows-style fallback paths', () => { // Why: SSH/POSIX paths can legally contain `\` as a filename character; // only Windows-style separators should be normalized. - expect(getCodexCanonicalTrustPath('/tmp/with\\literal/hooks.json')).toBe( + expect(normalizeCodexHookSourcePath('/tmp/with\\literal/hooks.json')).toBe( '/tmp/with\\literal/hooks.json' ) }) + + it('resolves an explicit home parent while preserving its hooks.json leaf symlink', () => { + const logicalHome = join(tmpDir, 'logical-home') + const targetHome = join(tmpDir, 'target-home') + const targetHooks = join(tmpDir, 'target-hooks.json') + mkdirSync(targetHome) + writeFileSync(targetHooks, '{"hooks":{}}\n', 'utf-8') + symlinkSync(targetHome, logicalHome) + symlinkSync(targetHooks, join(targetHome, 'hooks.json')) + + expect(getCodexExplicitHomeHookSourcePath(join(logicalHome, 'hooks.json'))).toBe( + join(realpathSync.native(targetHome), 'hooks.json') + ) + }) }) describe('upsertHookTrustEntries', () => { diff --git a/src/main/codex/config-toml-trust.ts b/src/main/codex/config-toml-trust.ts index 2e40f49895d..63a7f7555e8 100644 --- a/src/main/codex/config-toml-trust.ts +++ b/src/main/codex/config-toml-trust.ts @@ -9,7 +9,7 @@ import { unlinkSync, writeFileSync } from 'node:fs' -import { basename, dirname, join } from 'node:path' +import { basename, dirname, join, posix as pathPosix, win32 as pathWin32 } from 'node:path' import { createHash, randomUUID } from 'node:crypto' import { renameFileWithWindowsRetry } from '../codex-accounts/fs-utils' import { foldWslUncPathCaseInsensitiveParts } from '../../shared/wsl-paths' @@ -164,25 +164,64 @@ export function computeTrustedHash(entry: CodexTrustEntry): string { return `sha256:${createHash('sha256').update(serialized).digest('hex')}` } +// Why: key_source is already home-derived by discovery. Realpath here would +// change default-home keys; explicit-home callers canonicalize the home first. export function computeTrustKey(entry: CodexTrustEntry): string { - return `${getCodexCanonicalTrustPath(entry.sourcePath)}:${entry.eventLabel}:${entry.groupIndex}:${entry.handlerIndex}` + return `${normalizeCodexHookSourcePath(entry.sourcePath)}:${entry.eventLabel}:${entry.groupIndex}:${entry.handlerIndex}` } -export function getCodexCanonicalTrustPath(sourcePath: string): string { - if (process.platform !== 'win32' && isWindowsDriveOrBackslashUncPath(sourcePath)) { - // Why: an existing `//` path is valid on POSIX, so let realpath distinguish - // it from forward-slash UNC before falling back to Windows normalization. - return normalizeWindowsPathForCodexLookup(sourcePath) +/** + * Returns the hook source path Codex derives after an explicit CODEX_HOME is + * canonicalized. The source leaf remains logical because hook discovery only + * normalizes the joined path lexically. + */ +export function getCodexExplicitHomeHookSourcePath(sourcePath: string): string { + if (process.platform !== 'win32' && isUnambiguousWindowsPath(sourcePath)) { + return normalizeCodexHookSourcePath(sourcePath) } try { - // Why: Codex canonicalizes parent directories but preserves a hooks.json - // leaf symlink in trust keys, so resolving the whole path misses its RPC listing. - return join(realpathSync.native(dirname(sourcePath)), basename(sourcePath)) + // Why: explicit CODEX_HOME resolves the home directory before hooks.json + // is appended, so a symlinked leaf must remain logical in the reported key. + return normalizeCodexHookSourcePath( + join(realpathSync.native(dirname(sourcePath)), basename(sourcePath)) + ) } catch { - return normalizeWindowsPathForCodexLookup(sourcePath) + return normalizeCodexHookSourcePath(sourcePath) } } +/** Matches the platform-native lexical path displayed by hook discovery. */ +export function normalizeCodexHookSourcePath(sourcePath: string): string { + if (isWindowsPathForTrustSource(sourcePath)) { + const withoutDevicePrefix = stripWindowsDevicePrefix(sourcePath) + const normalized = pathWin32.isAbsolute(withoutDevicePrefix) + ? pathWin32.normalize(withoutDevicePrefix) + : pathWin32.resolve(withoutDevicePrefix) + return trimNonRootTrailingSeparators(normalized, pathWin32.parse(normalized).root, /[\\/]/) + } + const normalized = pathPosix.isAbsolute(sourcePath) + ? pathPosix.normalize(sourcePath) + : pathPosix.resolve(sourcePath) + return trimNonRootTrailingSeparators(normalized, pathPosix.parse(normalized).root, /\//) +} + +function trimNonRootTrailingSeparators(path: string, root: string, separators: RegExp): string { + let end = path.length + while (end > root.length && separators.test(path[end - 1]!)) { + end -= 1 + } + return path.slice(0, end) +} + +function stripWindowsDevicePrefix(sourcePath: string): string { + const unc = /^(?:\\\\\?|\\\\\.)\\UNC\\/i.exec(sourcePath) + if (unc) { + return `\\\\${sourcePath.slice(unc[0].length)}` + } + const drive = /^(?:\\\\\?|\\\\\.)\\(?=[A-Za-z]:[\\/])/i.exec(sourcePath) + return drive ? sourcePath.slice(drive[0].length) : sourcePath +} + function getCodexCanonicalProjectPath(projectPath: string): string { try { // Why: local project trust still needs Codex's realpath shape, but remote @@ -193,13 +232,6 @@ function getCodexCanonicalProjectPath(projectPath: string): string { } } -function normalizeWindowsPathForCodexLookup(sourcePath: string): string { - if (!usesWindowsPathSeparators(sourcePath)) { - return sourcePath - } - return sourcePath.replace(/\//g, '\\') -} - function normalizeWindowsPathSeparators(sourcePath: string): string { if (!usesWindowsPathSeparators(sourcePath)) { return sourcePath @@ -208,13 +240,21 @@ function normalizeWindowsPathSeparators(sourcePath: string): string { } function usesWindowsPathSeparators(sourcePath: string): boolean { - return isWindowsDriveOrBackslashUncPath(sourcePath) || sourcePath.startsWith('//') + return isUnambiguousWindowsPath(sourcePath) || sourcePath.startsWith('//') } -function isWindowsDriveOrBackslashUncPath(sourcePath: string): boolean { +function isUnambiguousWindowsPath(sourcePath: string): boolean { return /^[A-Za-z]:[\\/]/.test(sourcePath) || sourcePath.startsWith('\\\\') } +function isWindowsPathForTrustSource(sourcePath: string): boolean { + return ( + isUnambiguousWindowsPath(sourcePath) || + (process.platform === 'win32' && + (sourcePath.startsWith('//') || !pathPosix.isAbsolute(sourcePath))) + ) +} + // Why: Codex and Orca can disagree on quote style, separators, and casing for // the same Windows project, including when the caller targets a remote host. export function normalizeCodexProjectPathForLookup(projectPath: string): string { @@ -336,7 +376,7 @@ export function upsertHookTrustEntriesInContent( const existing = existingContent.charCodeAt(0) === 0xfeff ? existingContent.slice(1) : existingContent let updated = entries.some((entry) => - usesWindowsPathSeparators(getCodexCanonicalTrustPath(entry.sourcePath)) + usesWindowsPathSeparators(normalizeCodexHookSourcePath(entry.sourcePath)) ) ? ensureHooksStateParentTable(existing) : existing @@ -546,7 +586,13 @@ export function normalizeHookTrustKeyForLookup(key: string): string { const parsed = parseTrustKey(key) // Why: fold by path shape, not host platform — hook sources on WSL and SSH // Windows remotes need the same folding when Orca runs on macOS or Linux. - const foldedPath = normalizeCodexProjectPathForLookup(parsed ? parsed.sourcePath : key) + const foldedPath = normalizeCodexProjectPathForLookup( + parsed + ? parsed.sourcePath.startsWith('//') + ? parsed.sourcePath + : normalizeCodexHookSourcePath(parsed.sourcePath) + : key + ) return parsed ? `${foldedPath}:${parsed.eventLabel}:${parsed.groupIndex}:${parsed.handlerIndex}` : foldedPath diff --git a/src/main/codex/hook-service.test.ts b/src/main/codex/hook-service.test.ts index 526a64d99ab..348a62a11f8 100644 --- a/src/main/codex/hook-service.test.ts +++ b/src/main/codex/hook-service.test.ts @@ -5,7 +5,6 @@ import { mkdirSync, mkdtempSync, readFileSync, - realpathSync, rmSync, symlinkSync, writeFileSync @@ -17,7 +16,12 @@ import { spawn } from 'node:child_process' import { createServer } from 'node:http' import type { AddressInfo } from 'node:net' import { createManagedCommandMatcher, wrapPosixHookCommand } from '../agent-hooks/installer-utils' -import { computeTrustedHash, upsertHookTrustEntriesInContent } from './config-toml-trust' +import { + computeTrustedHash, + getCodexExplicitHomeHookSourcePath, + normalizeCodexHookSourcePath, + upsertHookTrustEntriesInContent +} from './config-toml-trust' const { getPathMock, homedirMock } = vi.hoisted(() => ({ getPathMock: vi.fn<(name: string) => string>(), @@ -81,14 +85,14 @@ function escapeTomlBasicString(value: string): string { return value.replaceAll('\\', '\\\\').replaceAll('"', '\\"') } -function hookTrustHeader(key: string): string { - const canonicalKey = canonicalizeHookTrustKeyForTest(key) +function hookTrustHeader(key: string, useDefaultCodexHome = false): string { + const canonicalKey = canonicalizeHookTrustKeyForTest(key, useDefaultCodexHome) return /^[A-Za-z]:[\\/]|^\\\\/.test(canonicalKey) && !canonicalKey.includes("'") ? `[hooks.state.'${canonicalKey}']` : `[hooks.state."${escapeTomlBasicString(canonicalKey)}"]` } -function canonicalizeHookTrustKeyForTest(key: string): string { +function canonicalizeHookTrustKeyForTest(key: string, useDefaultCodexHome: boolean): string { const lastColon = key.lastIndexOf(':') const secondLast = lastColon === -1 ? -1 : key.lastIndexOf(':', lastColon - 1) const thirdLast = secondLast === -1 ? -1 : key.lastIndexOf(':', secondLast - 1) @@ -96,15 +100,10 @@ function canonicalizeHookTrustKeyForTest(key: string): string { return key } const sourcePath = key.slice(0, thirdLast) - try { - // Why: mirrors getCodexCanonicalTrustPath so test expectations match the - // native raw-backslash keys Codex writes after hook approval on Windows. - return `${realpathSync.native(sourcePath)}${key.slice(thirdLast)}` - } catch { - return /^[A-Za-z]:[\\/]|^\\\\/.test(sourcePath) - ? `${sourcePath.replace(/\//g, '\\')}${key.slice(thirdLast)}` - : key - } + const trustSourcePath = useDefaultCodexHome + ? normalizeCodexHookSourcePath(sourcePath) + : getCodexExplicitHomeHookSourcePath(sourcePath) + return `${trustSourcePath}${key.slice(thirdLast)}` } function markHookTrustDisabled(toml: string, header: string): string { @@ -473,7 +472,7 @@ describe('CodexHookService', () => { const runtimeToml = readFileSync(join(managedCodexHome, 'config.toml'), 'utf-8') expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:stop:1:0`)) expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:stop:0:0`)) - expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:0:0`)) + expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:0:0`, true)) }) it('runs managed PostToolUse status before mirrored user hooks', () => { @@ -521,7 +520,7 @@ describe('CodexHookService', () => { const runtimeToml = readFileSync(join(managedCodexHome, 'config.toml'), 'utf-8') expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:post_tool_use:0:0`)) expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:post_tool_use:1:0`)) - expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:post_tool_use:0:0`)) + expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:post_tool_use:0:0`, true)) }) it('mirrors system user hook approvals when the system trust indices are stale', () => { @@ -573,8 +572,8 @@ describe('CodexHookService', () => { expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:stop:0:0`)) expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:stop:1:0`)) expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:stop:2:0`)) - expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:0:0`)) - expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:1:0`)) + expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:0:0`, true)) + expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:1:0`, true)) }) it('skips plugin-placeholder system hooks when mirroring into runtime CODEX_HOME', () => { @@ -679,7 +678,7 @@ describe('CodexHookService', () => { )}\n`, 'utf-8' ) - const disabledPostCompactHeader = hookTrustHeader(`${systemHooksPath}:post_compact:0:0`) + const disabledPostCompactHeader = hookTrustHeader(`${systemHooksPath}:post_compact:0:0`, true) const systemToml = upsertHookTrustEntriesInContent('model = "system-model"\n', [ { sourcePath: systemHooksPath, @@ -719,8 +718,8 @@ describe('CodexHookService', () => { expect(runtimeToml).toContain( `${hookTrustHeader(`${managedHooksPath}:post_compact:0:0`)}\nenabled = false` ) - expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:pre_compact:0:0`)) - expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:post_compact:0:0`)) + expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:pre_compact:0:0`, true)) + expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:post_compact:0:0`, true)) }) it('removes runtime user hook trust after system approval is revoked', () => { @@ -813,7 +812,7 @@ describe('CodexHookService', () => { })}\n`, 'utf-8' ) - const disabledStopHeader = hookTrustHeader(`${systemHooksPath}:stop:0:0`) + const disabledStopHeader = hookTrustHeader(`${systemHooksPath}:stop:0:0`, true) const systemToml = upsertHookTrustEntriesInContent('model = "system-model"\n', [ { sourcePath: systemHooksPath, @@ -1336,7 +1335,7 @@ describe('CodexHookService', () => { `${managedHooksPath}:permission_request:0:0` ) const legacyPermissionHeader = `[hooks.state."${escapeTomlBasicString( - `${realpathSync.native(managedHooksPath).replace(/\\/g, '/')}:permission_request:0:0` + `${getCodexExplicitHomeHookSourcePath(managedHooksPath).replace(/\\/g, '/')}:permission_request:0:0` )}"]` const installedToml = readFileSync(runtimeTomlPath, 'utf-8') expect(installedToml).toContain(canonicalPermissionHeader) diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index fbc6ded0fad..957bfd669f1 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -37,7 +37,8 @@ import { computeTrustKey, computeTrustedHash, escapeTomlString, - getCodexCanonicalTrustPath, + getCodexExplicitHomeHookSourcePath, + normalizeCodexHookSourcePath, normalizeCodexProjectPathForLookup, normalizeHookTrustKeyForLookup, parseTrustKey, @@ -262,11 +263,11 @@ function removeStaleRuntimeHookTrustEntries( entry.trustedHash ?? computeTrustedHash(entry) ]) ) - const canonicalRuntimeHooksPath = getCodexCanonicalTrustPath(runtimeHooksPath) + const canonicalRuntimeHooksPath = getCodexExplicitHomeHookSourcePath(runtimeHooksPath) const staleKeys: string[] = [] for (const [key, state] of readHookTrustEntries(tomlPath)) { const parsed = parseTrustKey(key) - if (!parsed || getCodexCanonicalTrustPath(parsed.sourcePath) !== canonicalRuntimeHooksPath) { + if (!parsed || normalizeCodexHookSourcePath(parsed.sourcePath) !== canonicalRuntimeHooksPath) { continue } if (expectedHashes.get(normalizeHookTrustKeyForLookup(key)) === state.trustedHash) { @@ -434,13 +435,13 @@ function getTrustedSystemHookHashesByEvent( trustEntries: ReadonlyMap ): Map> { const trustedHashesByEvent = new Map>() - const canonicalSystemConfigPath = getCodexCanonicalTrustPath(systemConfigPath) + const canonicalSystemConfigPath = normalizeCodexHookSourcePath(systemConfigPath) for (const [key, state] of trustEntries) { const parsed = parseTrustKey(key) if (!parsed || !state.trustedHash) { continue } - if (getCodexCanonicalTrustPath(parsed.sourcePath) !== canonicalSystemConfigPath) { + if (normalizeCodexHookSourcePath(parsed.sourcePath) !== canonicalSystemConfigPath) { continue } let hashes = trustedHashesByEvent.get(parsed.eventLabel) @@ -469,6 +470,7 @@ function collectMirroredRuntimeUserHookTrustEntries( } const entries: MirroredRuntimeUserHookTrustEntry[] = [] + const trustSourcePath = getCodexExplicitHomeHookSourcePath(runtimeConfigPath) for (const [eventName, definitions] of Object.entries(runtimeHooks)) { if (!Array.isArray(definitions)) { continue @@ -480,7 +482,7 @@ function collectMirroredRuntimeUserHookTrustEntries( return } const entry = createCodexHookTrustEntry( - runtimeConfigPath, + trustSourcePath, eventName, groupIndex, handlerIndex, @@ -733,7 +735,8 @@ function removeRuntimeManagedHookTrustEntries(configPath: string): void { sourcePath: configPath, command: getManagedCommand(getManagedScriptPath()), managedEventLabels: CODEX_MANAGED_EVENT_LABELS, - timeoutSec: MANAGED_HOOK_TIMEOUT_SECONDS + timeoutSec: MANAGED_HOOK_TIMEOUT_SECONDS, + sourceUsesExplicitCodexHome: true }) } catch (error) { // Best effort — stale trust entries are harmless once hooks.json no @@ -1194,6 +1197,7 @@ export class CodexHookService { const missing: string[] = [] const trustMissing: string[] = [] const disabled: string[] = [] + const trustSourcePath = getCodexExplicitHomeHookSourcePath(configPath) let presentCount = 0 for (const eventName of CODEX_EVENTS) { const definitions = Array.isArray(config.hooks?.[eventName]) ? config.hooks![eventName]! : [] @@ -1227,7 +1231,7 @@ export class CodexHookService { // Codex folds the handler timeout into its trust hash. Hash the same // timeout here or status would report every managed hook as stale-trust. const trustInput: CodexTrustEntry = { - sourcePath: configPath, + sourcePath: trustSourcePath, eventLabel: CODEX_EVENT_LABEL[eventName], groupIndex: foundGroupIndex, handlerIndex: foundHandlerIndex, @@ -1352,6 +1356,7 @@ export class CodexHookService { ({ entry }) => entry ) const managedTrustEntries: CodexTrustEntry[] = [] + const trustSourcePath = getCodexExplicitHomeHookSourcePath(configPath) for (const eventName of CODEX_EVENTS) { const current = Array.isArray(nextHooks[eventName]) ? nextHooks[eventName] : [] const cleaned = removeManagedCommands(current, isManagedCommand) @@ -1365,7 +1370,7 @@ export class CodexHookService { // timeoutSec mirrors the hook's `timeout` so the trust hash matches the // entry actually written to hooks.json. managedTrustEntries.push({ - sourcePath: configPath, + sourcePath: trustSourcePath, eventLabel: CODEX_EVENT_LABEL[eventName], groupIndex: 0, handlerIndex: 0, diff --git a/src/main/codex/hook-trust-promotion.test.ts b/src/main/codex/hook-trust-promotion.test.ts index b497591db63..0f65b73b515 100644 --- a/src/main/codex/hook-trust-promotion.test.ts +++ b/src/main/codex/hook-trust-promotion.test.ts @@ -1,11 +1,20 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync +} from 'node:fs' import { tmpdir } from 'node:os' import type * as Os from 'node:os' import { join } from 'node:path' import { computeTrustKey, computeTrustedHash, + getCodexExplicitHomeHookSourcePath, readHookTrustEntries, upsertHookTrustEntriesInContent, type CodexTrustEntry @@ -97,7 +106,7 @@ function runtimeUserStopEntry(): CodexTrustEntry { // install() prepends the managed status hook on Stop, so the mirrored user // hook lands at groupIndex 1. return { - sourcePath: join(runtimeHomeDir(), 'hooks.json'), + sourcePath: getCodexExplicitHomeHookSourcePath(join(runtimeHomeDir(), 'hooks.json')), eventLabel: 'stop', groupIndex: 1, handlerIndex: 0, @@ -121,6 +130,25 @@ function readSystemToml(): string { } describe('codex hook trust write-back promotion', () => { + it('mirrors default-home trust when the .codex directory is a symlink', () => { + const targetHome = join(tmpHome, 'dotfiles-codex') + mkdirSync(targetHome) + symlinkSync(targetHome, systemCodexDir(), process.platform === 'win32' ? 'junction' : 'dir') + writeSystemUserHook() + const systemEntry = systemUserStopEntry() + writeFileSync( + join(systemCodexDir(), 'config.toml'), + upsertHookTrustEntriesInContent('', [systemEntry]) + ) + + expect(new CodexHookService().install().state).toBe('installed') + + const runtimeTrust = readHookTrustEntries(join(runtimeHomeDir(), 'config.toml')) + expect(runtimeTrust.get(computeTrustKey(runtimeUserStopEntry()))?.trustedHash).toBe( + computeTrustedHash(systemEntry) + ) + }) + it('keeps an in-Orca approval of a user hook across launches and promotes it to ~/.codex', () => { writeSystemUserHook() const service = new CodexHookService() diff --git a/src/main/codex/hook-trust-promotion.ts b/src/main/codex/hook-trust-promotion.ts index 36a570a662f..8e4a4a58668 100644 --- a/src/main/codex/hook-trust-promotion.ts +++ b/src/main/codex/hook-trust-promotion.ts @@ -7,7 +7,8 @@ import { } from '../agent-hooks/installer-utils' import { getOrcaManagedCodexHomePath, getSystemCodexHomePath } from './codex-home-paths' import { - getCodexCanonicalTrustPath, + getCodexExplicitHomeHookSourcePath, + normalizeCodexHookSourcePath, normalizeHookTrustKeyForLookup, parseTrustKey, readHookTrustEntries, @@ -81,11 +82,14 @@ export function snapshotCodexRuntimeHookTrustProvenance(): void { try { const runtimeHomePath = getOrcaManagedCodexHomePath() const runtimeHooksPath = join(runtimeHomePath, 'hooks.json') - const canonicalRuntimeHooksPath = getCodexCanonicalTrustPath(runtimeHooksPath) + const canonicalRuntimeHooksPath = getCodexExplicitHomeHookSourcePath(runtimeHooksPath) const entries: Record = {} for (const [key, state] of readHookTrustEntries(join(runtimeHomePath, 'config.toml'))) { const parsed = parseTrustKey(key) - if (!parsed || getCodexCanonicalTrustPath(parsed.sourcePath) !== canonicalRuntimeHooksPath) { + if ( + !parsed || + normalizeCodexHookSourcePath(parsed.sourcePath) !== canonicalRuntimeHooksPath + ) { continue } entries[normalizeHookTrustKeyForLookup(key)] = { @@ -126,8 +130,8 @@ function promoteCodexRuntimeHookApprovalsToSystemUnsafe(): void { const systemHomePath = getSystemCodexHomePath() const runtimeHooksPath = join(runtimeHomePath, 'hooks.json') const systemHooksPath = join(systemHomePath, 'hooks.json') - const canonicalRuntimeHooksPath = getCodexCanonicalTrustPath(runtimeHooksPath) - if (canonicalRuntimeHooksPath === getCodexCanonicalTrustPath(systemHooksPath)) { + const canonicalRuntimeHooksPath = getCodexExplicitHomeHookSourcePath(runtimeHooksPath) + if (canonicalRuntimeHooksPath === normalizeCodexHookSourcePath(systemHooksPath)) { return } const runtimeTomlPath = join(runtimeHomePath, 'config.toml') @@ -164,7 +168,7 @@ function promoteCodexRuntimeHookApprovalsToSystemUnsafe(): void { continue } const parsed = parseTrustKey(key) - if (!parsed || getCodexCanonicalTrustPath(parsed.sourcePath) !== canonicalRuntimeHooksPath) { + if (!parsed || normalizeCodexHookSourcePath(parsed.sourcePath) !== canonicalRuntimeHooksPath) { continue } const previous = provenance.get(normalizeHookTrustKeyForLookup(key)) @@ -191,7 +195,7 @@ function promoteCodexRuntimeHookApprovalsToSystemUnsafe(): void { continue } const runtimeEntry = createCodexHookTrustEntry( - runtimeHooksPath, + canonicalRuntimeHooksPath, eventName, parsed.groupIndex, parsed.handlerIndex, From 6ca71ebeae0c4d748ab7c4c47211b3e48af7d939 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:34:03 -0700 Subject: [PATCH 10/10] fix(codex): restore envToDelete on client invocation type after base reconcile --- src/main/codex/codex-app-server-client.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/codex/codex-app-server-client.ts b/src/main/codex/codex-app-server-client.ts index 4e58c8348cc..4ae3a157bda 100644 --- a/src/main/codex/codex-app-server-client.ts +++ b/src/main/codex/codex-app-server-client.ts @@ -18,6 +18,9 @@ export type CodexAppServerInvocation = { args: string[] /** Overlay applied on top of the inherited environment (e.g. CODEX_HOME). */ env?: Record + /** Env keys stripped from the inherited environment before spawn (e.g. an + * inherited CODEX_HOME, so a default-home grant runs against the real ~/.codex). */ + envToDelete?: readonly string[] /** Whole-session deadline. The codex child is SIGKILLed when it lapses. */ timeoutMs: number }