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] 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)