diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 4b10e558fd0..3ebd71aa41f 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", @@ -12,6 +13,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", @@ -26,6 +28,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", @@ -49,6 +53,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/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..7fac297ae86 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' }] }] } @@ -72,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 d9e36edcbfd..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' @@ -15,6 +14,8 @@ 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' +import { writeRollingFileBackup } from '../rolling-file-backup' 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)) { + writeRollingFileBackup(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-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 7030f7a9979..b99902ed224 100644 --- a/src/main/codex/codex-app-server-client.test.ts +++ b/src/main/codex/codex-app-server-client.test.ts @@ -53,6 +53,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 @@ -381,6 +386,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-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.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 f9699ddbfaf..0705924e905 100644 --- a/src/main/codex/codex-app-server-session.ts +++ b/src/main/codex/codex-app-server-session.ts @@ -11,6 +11,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 } diff --git a/src/main/codex/codex-hook-trust-grant.test.ts b/src/main/codex/codex-hook-trust-grant.test.ts index 5f184355b58..59f2807605d 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() @@ -116,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 02945104606..6fdf852bf6b 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) { @@ -114,10 +116,29 @@ 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('appends LAST and preserves user entries, unknown fields, and trust positions', () => { + 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 = { hooks: { @@ -126,27 +147,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', () => { @@ -330,6 +367,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 ae13635cf5f..30c0704734a 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 @@ -143,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. @@ -183,8 +173,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({ @@ -192,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' @@ -202,7 +204,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 +294,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 diff --git a/src/main/codex/codex-trust-grant-host.ts b/src/main/codex/codex-trust-grant-host.ts index e79b2becb1c..d16bd393a19 100644 --- a/src/main/codex/codex-trust-grant-host.ts +++ b/src/main/codex/codex-trust-grant-host.ts @@ -28,6 +28,7 @@ type CodexTrustGrantRequestInput = { runtimeHomePath: string managedCommand: string expectedTrustKeys: string[] + useDefaultCodexHome?: boolean } export type ResolvedCodexTrustGrantHost = { @@ -59,11 +60,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-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..ea27671e9fb --- /dev/null +++ b/src/main/codex/codex-user-hook-trust-rebase-client.ts @@ -0,0 +1,205 @@ +import { runCodexAppServerSession, type CodexAppServerInvocation } from './codex-app-server-session' +import { normalizeHookTrustKeyForLookup } from './config-toml-trust' + +type CodexHookListing = { + key: string + command: string | null + currentHash: string + trustStatus: string + enabled: boolean +} + +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 }[]) + : [] + const listings: CodexHookListing[] = [] + const seenKeys = new Set() + for (const entry of data) { + const hooks = Array.isArray(entry?.hooks) ? entry.hooks : [] + for (const hook of hooks as Record[]) { + if ( + typeof hook?.key !== 'string' || + typeof hook.currentHash !== 'string' || + typeof hook.trustStatus !== 'string' + ) { + continue + } + // Why: hooks/list repeats user-scope hooks per requested cwd; trust + // rebasing must consider each key once. + if (seenKeys.has(hook.key)) { + continue + } + seenKeys.add(hook.key) + listings.push({ + key: hook.key, + command: typeof hook.command === 'string' ? hook.command : null, + currentHash: hook.currentHash, + trustStatus: hook.trustStatus, + enabled: typeof hook.enabled === 'boolean' ? hook.enabled : true + }) + } + } + return listings +} + +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..89f25176d50 --- /dev/null +++ b/src/main/codex/codex-user-hook-trust-rebase.test.ts @@ -0,0 +1,253 @@ +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 { codexAppServerCapabilityCache } from './codex-app-server-capability-cache' +import { CodexAppServerUnsupportedError } from './codex-app-server-session' +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) + _internals.resetRetryState() + codexAppServerCapabilityCache.clear() + 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) + 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') { + expect(repair.moves).toEqual([ + expect.objectContaining({ command: 'trusted-user', wasTrusted: true, enabled: true }), + expect.objectContaining({ command: 'untrusted-user', wasTrusted: false, enabled: false }) + ]) + } + }) + + 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') + 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..0020e106c2f --- /dev/null +++ b/src/main/codex/codex-user-hook-trust-rebase.ts @@ -0,0 +1,206 @@ +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 { + 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 + +// 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 +): 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 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({ + runtimeHomePath: args.runtimeHomePath, + managedCommand: '', + expectedTrustKeys: [], + useDefaultCodexHome: true + }) + // Why: inspection happens before the write, so an unavailable RPC aborts + // without shifting a user's positional trust key. + 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') + } + + 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) { + rememberRebaseSessionFailure(hostKey, error) + if (hooksWritten) { + return rollbackMutation(args.restoreHooks, args.tomlPath, snapshot, error) + } + throw error + } +} + +export const _internals = { + setSessionRunnerSync(runner: RebaseSessionRunnerSync | null): void { + runSessionSync = runner ?? runCodexUserHookTrustRebaseSessionSync + }, + resetRetryState(): void { + rebaseRetryAfterByHost.clear() + } +} 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 9f8efbea403..e36e7a1a8d7 100644 --- a/src/main/codex/config-toml-trust.test.ts +++ b/src/main/codex/config-toml-trust.test.ts @@ -1,12 +1,16 @@ /* 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, mkdtempSync, readFileSync, realpathSync, rmSync, + symlinkSync, + statSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' @@ -16,7 +20,8 @@ import { computeTrustKey, computeTrustedHash, escapeTomlString, - getCodexCanonicalTrustPath, + getCodexExplicitHomeHookSourcePath, + normalizeCodexHookSourcePath, normalizeCodexProjectPathForLookup, normalizeCodexProjectPathForRevocationLookup, parseTrustKey, @@ -273,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') @@ -287,9 +292,36 @@ 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', () => { + 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(`${hooksPath}: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(normalizeCodexHookSourcePath(hooksPath)).toBe(join(tmpDir, 'hooks.json')) + } + ) + 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]. @@ -308,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', () => { @@ -497,6 +543,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', @@ -1300,6 +1370,60 @@ 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 = { + 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..63a7f7555e8 100644 --- a/src/main/codex/config-toml-trust.ts +++ b/src/main/codex/config-toml-trust.ts @@ -1,16 +1,19 @@ /* 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, + statSync, unlinkSync, writeFileSync } from 'node:fs' -import { 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 { 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, @@ -161,20 +164,62 @@ 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 { - 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) - } catch { - 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: 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 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 { @@ -187,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 @@ -202,10 +240,18 @@ function normalizeWindowsPathSeparators(sourcePath: string): string { } function usesWindowsPathSeparators(sourcePath: string): boolean { + return isUnambiguousWindowsPath(sourcePath) || sourcePath.startsWith('//') +} + +function isUnambiguousWindowsPath(sourcePath: string): boolean { + return /^[A-Za-z]:[\\/]/.test(sourcePath) || sourcePath.startsWith('\\\\') +} + +function isWindowsPathForTrustSource(sourcePath: string): boolean { return ( - /^[A-Za-z]:[\\/]/.test(sourcePath) || - sourcePath.startsWith('\\\\') || - sourcePath.startsWith('//') + isUnambiguousWindowsPath(sourcePath) || + (process.platform === 'win32' && + (sourcePath.startsWith('//') || !pathPosix.isAbsolute(sourcePath))) ) } @@ -330,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 @@ -540,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 @@ -780,16 +832,33 @@ 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`) + const existingMode = existsSync(writePath) ? statSync(writePath).mode : undefined let renamed = false try { - writeFileSync(tmpPath, contents, 'utf-8') - if (existsSync(configPath)) { - copyFileWithWindowsRetry(configPath, `${configPath}.bak`) + // 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)) { + writeRollingFileBackup(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 583b8fe11f0..f7080308e0a 100644 --- a/src/main/codex/hook-service-trust-grant.test.ts +++ b/src/main/codex/hook-service-trust-grant.test.ts @@ -1,5 +1,15 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { + chmodSync, + lstatSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + statSync, + writeFileSync +} from 'node:fs' import { tmpdir } from 'node:os' import type * as Os from 'node:os' import { join } from 'node:path' @@ -22,6 +32,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>(), @@ -63,6 +74,7 @@ beforeEach(() => { }) afterEach(() => { + rebaseInternals.setSessionRunnerSync(null) trustGrantInternals.setGrantSessionRunnerSync(null) trustGrantInternals.resetDiagnostics() codexAppServerCapabilityCache.clear() @@ -216,6 +228,75 @@ 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.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.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 fd72b9b3bcd..957bfd669f1 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, @@ -35,7 +37,8 @@ import { computeTrustKey, computeTrustedHash, escapeTomlString, - getCodexCanonicalTrustPath, + getCodexExplicitHomeHookSourcePath, + normalizeCodexHookSourcePath, normalizeCodexProjectPathForLookup, normalizeHookTrustKeyForLookup, parseTrustKey, @@ -75,6 +78,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. @@ -259,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) { @@ -431,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) @@ -466,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 @@ -477,7 +482,7 @@ function collectMirroredRuntimeUserHookTrustEntries( return } const entry = createCodexHookTrustEntry( - runtimeConfigPath, + trustSourcePath, eventName, groupIndex, handlerIndex, @@ -642,7 +647,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 }, { preserveMode: true }) + }, + 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) @@ -709,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 @@ -1170,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]! : [] @@ -1203,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, @@ -1328,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) @@ -1341,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, diff --git a/src/main/index.ts b/src/main/index.ts index b91cfba9067..11278e73a37 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -1868,12 +1868,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) @@ -2124,6 +2124,14 @@ app.whenReady().then(async () => { const emulatorBridge = new EmulatorBridge() runtimeService.setEmulatorBridge(emulatorBridge) 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. @@ -2133,14 +2141,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, 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 }) + } +}