From 8630eb286ad2ea1896f60d135df8bdf5985edfca Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 16 Jul 2026 12:56:24 -0700 Subject: [PATCH] fix(codex): align hook trust source paths --- .../codex/codex-app-server-session.test.ts | 41 +++++++++ src/main/codex/codex-app-server-session.ts | 10 ++- src/main/codex/codex-hook-trust-grant.test.ts | 13 +++ src/main/codex/codex-hook-trust-grant.ts | 5 +- .../codex-managed-trust-reconciliation.ts | 13 ++- .../codex-real-home-hook-install.test.ts | 19 ++++ .../codex/codex-real-home-hook-install.ts | 3 +- src/main/codex/codex-trust-grant-host.ts | 6 +- .../codex-user-hook-trust-rebase.test.ts | 1 + .../codex/codex-user-hook-trust-rebase.ts | 3 +- .../codex/config-toml-trust-paths.test.ts | 80 +++++++++++++++++ src/main/codex/config-toml-trust.test.ts | 29 ++++-- src/main/codex/config-toml-trust.ts | 90 ++++++++++++++----- src/main/codex/hook-service.test.ts | 45 +++++----- src/main/codex/hook-service.ts | 23 +++-- src/main/codex/hook-trust-promotion.test.ts | 32 ++++++- src/main/codex/hook-trust-promotion.ts | 18 ++-- 17 files changed, 352 insertions(+), 79 deletions(-) create mode 100644 src/main/codex/codex-app-server-session.test.ts create mode 100644 src/main/codex/config-toml-trust-paths.test.ts diff --git a/src/main/codex/codex-app-server-session.test.ts b/src/main/codex/codex-app-server-session.test.ts new file mode 100644 index 00000000000..c953237033e --- /dev/null +++ b/src/main/codex/codex-app-server-session.test.ts @@ -0,0 +1,41 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { runCodexAppServerSession } from './codex-app-server-session' + +const originalCodexHome = process.env.CODEX_HOME + +afterEach(() => { + if (originalCodexHome === undefined) { + delete process.env.CODEX_HOME + } else { + process.env.CODEX_HOME = originalCodexHome + } +}) + +describe('runCodexAppServerSession environment', () => { + it('removes inherited variables requested by a default-home invocation', async () => { + process.env.CODEX_HOME = '/tmp/inherited-managed-home' + const server = String.raw` + const readline = require('node:readline') + readline.createInterface({ input: process.stdin }).on('line', (line) => { + const message = JSON.parse(line) + if (typeof message.id !== 'number') return + const result = message.method === 'env/get' + ? { codexHome: process.env.CODEX_HOME ?? null } + : {} + process.stdout.write(JSON.stringify({ id: message.id, result }) + '\n') + }) + ` + + const result = await runCodexAppServerSession( + { + command: process.execPath, + args: ['-e', server], + envToDelete: ['CODEX_HOME'], + timeoutMs: 5_000 + }, + (requestRpc) => requestRpc('env/get') + ) + + expect(result).toEqual({ codexHome: null }) + }) +}) diff --git a/src/main/codex/codex-app-server-session.ts b/src/main/codex/codex-app-server-session.ts index 6798792de70..b73f2f46537 100644 --- a/src/main/codex/codex-app-server-session.ts +++ b/src/main/codex/codex-app-server-session.ts @@ -6,6 +6,8 @@ export type CodexAppServerInvocation = { args: string[] /** Overlay applied on top of the inherited environment (for example, CODEX_HOME). */ env?: Record + /** Variables that would make this child select a different runtime than its caller. */ + envToDelete?: readonly string[] /** Whole-session deadline; the Codex child is killed when it lapses. */ timeoutMs: number } @@ -52,8 +54,14 @@ export async function runCodexAppServerSession( run: (requestRpc: CodexAppServerRequestRpc) => Promise, spawnImpl: typeof spawn = spawn ): Promise { + const childEnv = { ...process.env, ...invocation.env } + // Why: default-home RPCs must observe the same missing CODEX_HOME as the + // pane whose hook keys they inspect; omission alone would inherit Orca's value. + for (const key of invocation.envToDelete ?? []) { + delete childEnv[key] + } const child = spawnImpl(invocation.command, invocation.args, { - env: { ...process.env, ...invocation.env }, + env: childEnv, stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true }) as ChildProcessWithoutNullStreams diff --git a/src/main/codex/codex-hook-trust-grant.test.ts b/src/main/codex/codex-hook-trust-grant.test.ts index cebe6ad20b6..59f2807605d 100644 --- a/src/main/codex/codex-hook-trust-grant.test.ts +++ b/src/main/codex/codex-hook-trust-grant.test.ts @@ -120,6 +120,19 @@ describe('grantManagedCodexHookTrust', () => { expect(getCodexTrustGrantDiagnostics()).toMatchObject({ granted: 1, fellBack: 0 }) }) + it('builds a default-home grant invocation without an inherited CODEX_HOME', () => { + const entries = [managedEntry('stop')] + const runner = vi.fn((_request: CodexHookTrustGrantRequest) => grantedSessionResult(entries)) + _internals.setGrantSessionRunnerSync(runner) + + expect( + grantManagedCodexHookTrust({ ...buildPlan(entries), useDefaultCodexHome: true }) + ).toMatchObject({ lane: 'rpc' }) + const invocation = runner.mock.calls[0]![0]!.invocation + expect(invocation.env?.CODEX_HOME).toBeUndefined() + expect(invocation.envToDelete).toContain('CODEX_HOME') + }) + it('skips the RPC session while the ledger grant still holds, and re-grants on config drift', () => { const entries = [managedEntry('session_start')] const runner = vi.fn(() => grantedSessionResult(entries)) diff --git a/src/main/codex/codex-hook-trust-grant.ts b/src/main/codex/codex-hook-trust-grant.ts index b66c63b65cb..322d4b39116 100644 --- a/src/main/codex/codex-hook-trust-grant.ts +++ b/src/main/codex/codex-hook-trust-grant.ts @@ -44,6 +44,8 @@ export type CodexManagedTrustGrantPlan = { /** Managed trust identities Orca just wrote (no trustedHash). */ managedEntries: readonly CodexTrustEntry[] host: CodexTrustGrantHost + /** Match a pane where CODEX_HOME is absent instead of an explicit managed home. */ + useDefaultCodexHome?: boolean } export type CodexTrustGrantFallbackReason = @@ -226,7 +228,8 @@ export function grantManagedCodexHookTrust( resolvedHost.buildRequest({ runtimeHomePath: plan.runtimeHomePath, managedCommand: plan.managedCommand, - expectedTrustKeys: expected.map(({ normalizedKey }) => normalizedKey) + expectedTrustKeys: expected.map(({ normalizedKey }) => normalizedKey), + useDefaultCodexHome: plan.useDefaultCodexHome }) ) } catch (error) { diff --git a/src/main/codex/codex-managed-trust-reconciliation.ts b/src/main/codex/codex-managed-trust-reconciliation.ts index e422f2a4c32..b6c9e9d5d3a 100644 --- a/src/main/codex/codex-managed-trust-reconciliation.ts +++ b/src/main/codex/codex-managed-trust-reconciliation.ts @@ -1,8 +1,9 @@ import { computeTrustKey, computeTrustedHash, - getCodexCanonicalTrustPath, + getCodexExplicitHomeHookSourcePath, normalizeHookTrustKeyForLookup, + normalizeCodexHookSourcePath, parseTrustKey, readHookTrustEntries, removeHookTrustEntries, @@ -58,22 +59,26 @@ export function removeCodexManagedHookTrustEntries(options: { command: string managedEventLabels: ReadonlySet timeoutSec: number + /** Explicit native homes resolve their parent before hook discovery. */ + sourceUsesExplicitCodexHome?: boolean }): void { const existingEntries = readHookTrustEntries(options.tomlPath) const ledgerHome = readCodexTrustGrantLedgerHomeForReconciliation(options.runtimeHomePath) - const canonicalSourcePath = getCodexCanonicalTrustPath(options.sourcePath) + const expectedSourcePath = options.sourceUsesExplicitCodexHome + ? getCodexExplicitHomeHookSourcePath(options.sourcePath) + : normalizeCodexHookSourcePath(options.sourcePath) const ownedKeys: string[] = [] for (const [key, state] of existingEntries) { const parts = parseTrustKey(key) if ( !parts || - getCodexCanonicalTrustPath(parts.sourcePath) !== canonicalSourcePath || + normalizeCodexHookSourcePath(parts.sourcePath) !== expectedSourcePath || !options.managedEventLabels.has(parts.eventLabel) ) { continue } const expectedEntry: CodexTrustEntry = { - sourcePath: options.sourcePath, + sourcePath: expectedSourcePath, eventLabel: parts.eventLabel, groupIndex: parts.groupIndex, handlerIndex: parts.handlerIndex, diff --git a/src/main/codex/codex-real-home-hook-install.test.ts b/src/main/codex/codex-real-home-hook-install.test.ts index b49dcf691ea..efc91cd5e2a 100644 --- a/src/main/codex/codex-real-home-hook-install.test.ts +++ b/src/main/codex/codex-real-home-hook-install.test.ts @@ -116,9 +116,28 @@ describe('ensureRealHomeCodexHookState (install)', () => { const plan = grantMock.mock.calls[0]![0] as CodexManagedTrustGrantPlan expect(plan.runtimeHomePath).toBe(join(fakeHomeDir, '.codex')) expect(plan.host).toEqual({ kind: 'native' }) + expect(plan.useDefaultCodexHome).toBe(true) expect(plan.managedEntries.every((entry) => entry.groupIndex === 0)).toBe(true) }) + it('keeps a symlinked default home logical in the keys sent to Codex', () => { + grantSucceeds() + const logicalHome = join(fakeHomeDir, '.codex') + const targetHome = join(fakeHomeDir, 'dotfiles-codex') + rmSync(logicalHome, { recursive: true }) + mkdirSync(targetHome) + symlinkSync(targetHome, logicalHome) + + expect(ensureRealHomeCodexHookState({ hooksEnabled: true, userDataPath: userDataDir })).toBe( + 'installed' + ) + + const plan = grantMock.mock.calls[0]![0] as CodexManagedTrustGrantPlan + expect( + plan.managedEntries.map(computeTrustKey).every((key) => key.startsWith(logicalHome)) + ).toBe(true) + }) + it('keeps the managed lane for unknown top-level fields Codex cannot load', () => { grantSucceeds() const userConfig = { diff --git a/src/main/codex/codex-real-home-hook-install.ts b/src/main/codex/codex-real-home-hook-install.ts index 8e122f69812..30c0704734a 100644 --- a/src/main/codex/codex-real-home-hook-install.ts +++ b/src/main/codex/codex-real-home-hook-install.ts @@ -193,7 +193,8 @@ function installRealHomeCodexHook(userDataPath: string): RealHomeCodexHookLane { tomlPath: getRealHomeConfigTomlPath(), managedCommand: material.command, managedEntries, - host: { kind: 'native' } + host: { kind: 'native' }, + useDefaultCodexHome: true }) if (grant.lane === 'rpc') { return 'installed' diff --git a/src/main/codex/codex-trust-grant-host.ts b/src/main/codex/codex-trust-grant-host.ts index 63cd81eb425..0c0a97381df 100644 --- a/src/main/codex/codex-trust-grant-host.ts +++ b/src/main/codex/codex-trust-grant-host.ts @@ -23,6 +23,7 @@ type CodexTrustGrantRequestInput = { runtimeHomePath: string managedCommand: string expectedTrustKeys: string[] + useDefaultCodexHome?: boolean } export type ResolvedCodexTrustGrantHost = { @@ -54,11 +55,14 @@ export function resolveCodexTrustGrantHost(host: CodexTrustGrantHost): ResolvedC binaryStamp: command === 'codex' ? null : buildNativeCodexBinaryStamp(command), buildRequest: (input) => { const { spawnCmd, spawnArgs } = getSpawnArgsForWindows(command, ['app-server']) + const useDefaultCodexHome = input.useDefaultCodexHome === true return { invocation: { command: spawnCmd, args: spawnArgs, - env: { CODEX_HOME: input.runtimeHomePath }, + ...(useDefaultCodexHome + ? { envToDelete: ['CODEX_HOME'] } + : { env: { CODEX_HOME: input.runtimeHomePath } }), timeoutMs: NATIVE_GRANT_TIMEOUT_MS }, hooksListCwd: input.runtimeHomePath, diff --git a/src/main/codex/codex-user-hook-trust-rebase.test.ts b/src/main/codex/codex-user-hook-trust-rebase.test.ts index 6e409487d6d..89f25176d50 100644 --- a/src/main/codex/codex-user-hook-trust-rebase.test.ts +++ b/src/main/codex/codex-user-hook-trust-rebase.test.ts @@ -136,6 +136,7 @@ describe('real-home user hook trust rebasing', () => { }) expect(requests).toHaveLength(2) + expect(requests[0]?.invocation.envToDelete).toContain('CODEX_HOME') const repair = requests[1] expect(repair?.operation).toBe('repair-user-hook-trust') if (repair?.operation === 'repair-user-hook-trust') { diff --git a/src/main/codex/codex-user-hook-trust-rebase.ts b/src/main/codex/codex-user-hook-trust-rebase.ts index 17ea73ab52a..0020e106c2f 100644 --- a/src/main/codex/codex-user-hook-trust-rebase.ts +++ b/src/main/codex/codex-user-hook-trust-rebase.ts @@ -151,7 +151,8 @@ export function mutateRealHomeHooksPreservingUserTrust(args: { const baseRequest = resolveCodexTrustGrantHost({ kind: 'native' }).buildRequest({ runtimeHomePath: args.runtimeHomePath, managedCommand: '', - expectedTrustKeys: [] + expectedTrustKeys: [], + useDefaultCodexHome: true }) // Why: inspection happens before the write, so an unavailable RPC aborts // without shifting a user's positional trust key. diff --git a/src/main/codex/config-toml-trust-paths.test.ts b/src/main/codex/config-toml-trust-paths.test.ts new file mode 100644 index 00000000000..6aa1ced1b81 --- /dev/null +++ b/src/main/codex/config-toml-trust-paths.test.ts @@ -0,0 +1,80 @@ +import { resolve } from 'node:path' +import { describe, expect, it } from 'vitest' +import { + computeTrustKey, + normalizeCodexHookSourcePath, + normalizeHookTrustKeyForLookup, + type CodexTrustEntry +} from './config-toml-trust' + +function entry(sourcePath: string): CodexTrustEntry { + return { + sourcePath, + eventLabel: 'stop', + groupIndex: 2, + handlerIndex: 3, + command: 'irrelevant' + } +} + +describe('Codex hook trust source path normalization', () => { + it.each([ + ['C:/Users/Rod/.codex/./hooks.json', 'C:\\Users\\Rod\\.codex\\hooks.json'], + ['C:\\Users\\Rod\\tmp\\..\\.codex\\hooks.json', 'C:\\Users\\Rod\\.codex\\hooks.json'], + ['C:\\Users\\Rod\\.codex\\hooks.json\\', 'C:\\Users\\Rod\\.codex\\hooks.json'], + ['\\\\server\\share\\dir\\.\\hooks.json', '\\\\server\\share\\dir\\hooks.json'], + ['\\\\?\\C:\\Users\\Rod\\.codex\\hooks.json', 'C:\\Users\\Rod\\.codex\\hooks.json'], + ['\\\\?\\UNC\\server\\share\\dir\\hooks.json', '\\\\server\\share\\dir\\hooks.json'], + ['\\\\.\\C:\\Users\\Rod\\.codex\\hooks.json', 'C:\\Users\\Rod\\.codex\\hooks.json'], + ['\\\\.\\UNC\\server\\share\\dir\\hooks.json', '\\\\server\\share\\dir\\hooks.json'] + ])('matches Windows PathBuf display semantics for %s', (sourcePath, expected) => { + expect(normalizeCodexHookSourcePath(sourcePath)).toBe(expected) + }) + + it('uses native semantics for the otherwise ambiguous forward-slash UNC form', () => { + expect(normalizeCodexHookSourcePath('//server/share/dir/hooks.json')).toBe( + process.platform === 'win32' + ? '\\\\server\\share\\dir\\hooks.json' + : '/server/share/dir/hooks.json' + ) + }) + + it('normalizes POSIX doubles, dot segments, repeated separators, and trailing slashes', () => { + expect(normalizeCodexHookSourcePath('//tmp/a//b/../hooks.json/')).toBe('/tmp/a/hooks.json') + }) + + it('resolves relative native paths to the absolute discovery shape', () => { + expect(normalizeCodexHookSourcePath('relative/../hooks.json')).toBe(resolve('hooks.json')) + }) + + it('preserves POSIX case, literal backslashes, spaces, and Unicode bytes', () => { + const sourcePath = '/tmp/Üser Name/e\u0301\\literal/Hooks.JSON' + expect(normalizeCodexHookSourcePath(sourcePath)).toBe(sourcePath) + }) + + it('preserves Windows drive-letter and component casing in the lexical path', () => { + expect(normalizeCodexHookSourcePath('d:/Üser Name/Codex/Hooks.JSON')).toBe( + 'd:\\Üser Name\\Codex\\Hooks.JSON' + ) + }) + + it('builds the key byte-for-byte from the normalized source and positional suffix', () => { + expect(computeTrustKey(entry('C:/Users/Rod/.codex/../.codex/hooks.json'))).toBe( + 'C:\\Users\\Rod\\.codex\\hooks.json:stop:2:3' + ) + }) +}) + +describe('Codex hook trust key lookup normalization', () => { + it('folds Windows separator and case variants without changing the persisted key', () => { + const native = 'C:\\Users\\Rod\\.codex\\hooks.json:stop:2:3' + const slash = 'c:/users/rod/.codex/hooks.json:stop:2:3' + expect(normalizeHookTrustKeyForLookup(native)).toBe(normalizeHookTrustKeyForLookup(slash)) + }) + + it('keeps POSIX paths case-sensitive', () => { + const upper = '/home/Üser/.codex/hooks.json:stop:2:3' + const lower = '/home/üser/.codex/hooks.json:stop:2:3' + expect(normalizeHookTrustKeyForLookup(upper)).not.toBe(normalizeHookTrustKeyForLookup(lower)) + }) +}) diff --git a/src/main/codex/config-toml-trust.test.ts b/src/main/codex/config-toml-trust.test.ts index 9e8ae78cdc0..e36e7a1a8d7 100644 --- a/src/main/codex/config-toml-trust.test.ts +++ b/src/main/codex/config-toml-trust.test.ts @@ -14,13 +14,14 @@ import { writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' -import { dirname, join } from 'node:path' +import { join } from 'node:path' import { escapeRegex } from '../../shared/string-utils' import { computeTrustKey, computeTrustedHash, escapeTomlString, - getCodexCanonicalTrustPath, + getCodexExplicitHomeHookSourcePath, + normalizeCodexHookSourcePath, normalizeCodexProjectPathForLookup, normalizeCodexProjectPathForRevocationLookup, parseTrustKey, @@ -277,7 +278,7 @@ describe('computeTrustKey', () => { ).toBe('/Users/thebr/.codex/hooks.json:pre_tool_use:0:0') }) - it('uses Codex canonicalized source paths when hooks.json exists', () => { + it('lexically normalizes source paths without resolving default-home aliases', () => { const nestedDir = join(tmpDir, 'nested') mkdirSync(nestedDir) const hooksPath = join(nestedDir, '..', 'hooks.json') @@ -291,7 +292,7 @@ describe('computeTrustKey', () => { handlerIndex: 0, command: 'irrelevant' }) - ).toBe(`${realpathSync.native(hooksPath)}:user_prompt_submit:0:0`) + ).toBe(`${join(tmpDir, 'hooks.json')}:user_prompt_submit:0:0`) }) it('preserves a hooks.json leaf symlink in the trust key', () => { @@ -308,7 +309,7 @@ describe('computeTrustKey', () => { handlerIndex: 0, command: 'irrelevant' }) - ).toBe(`${join(realpathSync.native(dirname(hooksPath)), 'hooks.json')}:stop:0:0`) + ).toBe(`${hooksPath}:stop:0:0`) }) it.skipIf(process.platform === 'win32')( @@ -317,7 +318,7 @@ describe('computeTrustKey', () => { const hooksPath = `/${join(tmpDir, 'hooks.json')}` writeFileSync(hooksPath, '{"hooks":{}}\n', 'utf-8') - expect(getCodexCanonicalTrustPath(hooksPath)).toBe(realpathSync.native(hooksPath)) + expect(normalizeCodexHookSourcePath(hooksPath)).toBe(join(tmpDir, 'hooks.json')) } ) @@ -339,10 +340,24 @@ describe('computeTrustKey', () => { it('preserves literal backslashes in non-Windows-style fallback paths', () => { // Why: SSH/POSIX paths can legally contain `\` as a filename character; // only Windows-style separators should be normalized. - expect(getCodexCanonicalTrustPath('/tmp/with\\literal/hooks.json')).toBe( + expect(normalizeCodexHookSourcePath('/tmp/with\\literal/hooks.json')).toBe( '/tmp/with\\literal/hooks.json' ) }) + + it('resolves an explicit home parent while preserving its hooks.json leaf symlink', () => { + const logicalHome = join(tmpDir, 'logical-home') + const targetHome = join(tmpDir, 'target-home') + const targetHooks = join(tmpDir, 'target-hooks.json') + mkdirSync(targetHome) + writeFileSync(targetHooks, '{"hooks":{}}\n', 'utf-8') + symlinkSync(targetHome, logicalHome) + symlinkSync(targetHooks, join(targetHome, 'hooks.json')) + + expect(getCodexExplicitHomeHookSourcePath(join(logicalHome, 'hooks.json'))).toBe( + join(realpathSync.native(targetHome), 'hooks.json') + ) + }) }) describe('upsertHookTrustEntries', () => { diff --git a/src/main/codex/config-toml-trust.ts b/src/main/codex/config-toml-trust.ts index 2e40f49895d..63a7f7555e8 100644 --- a/src/main/codex/config-toml-trust.ts +++ b/src/main/codex/config-toml-trust.ts @@ -9,7 +9,7 @@ import { unlinkSync, writeFileSync } from 'node:fs' -import { basename, dirname, join } from 'node:path' +import { basename, dirname, join, posix as pathPosix, win32 as pathWin32 } from 'node:path' import { createHash, randomUUID } from 'node:crypto' import { renameFileWithWindowsRetry } from '../codex-accounts/fs-utils' import { foldWslUncPathCaseInsensitiveParts } from '../../shared/wsl-paths' @@ -164,25 +164,64 @@ export function computeTrustedHash(entry: CodexTrustEntry): string { return `sha256:${createHash('sha256').update(serialized).digest('hex')}` } +// Why: key_source is already home-derived by discovery. Realpath here would +// change default-home keys; explicit-home callers canonicalize the home first. export function computeTrustKey(entry: CodexTrustEntry): string { - return `${getCodexCanonicalTrustPath(entry.sourcePath)}:${entry.eventLabel}:${entry.groupIndex}:${entry.handlerIndex}` + return `${normalizeCodexHookSourcePath(entry.sourcePath)}:${entry.eventLabel}:${entry.groupIndex}:${entry.handlerIndex}` } -export function getCodexCanonicalTrustPath(sourcePath: string): string { - if (process.platform !== 'win32' && isWindowsDriveOrBackslashUncPath(sourcePath)) { - // Why: an existing `//` path is valid on POSIX, so let realpath distinguish - // it from forward-slash UNC before falling back to Windows normalization. - return normalizeWindowsPathForCodexLookup(sourcePath) +/** + * Returns the hook source path Codex derives after an explicit CODEX_HOME is + * canonicalized. The source leaf remains logical because hook discovery only + * normalizes the joined path lexically. + */ +export function getCodexExplicitHomeHookSourcePath(sourcePath: string): string { + if (process.platform !== 'win32' && isUnambiguousWindowsPath(sourcePath)) { + return normalizeCodexHookSourcePath(sourcePath) } try { - // Why: Codex canonicalizes parent directories but preserves a hooks.json - // leaf symlink in trust keys, so resolving the whole path misses its RPC listing. - return join(realpathSync.native(dirname(sourcePath)), basename(sourcePath)) + // Why: explicit CODEX_HOME resolves the home directory before hooks.json + // is appended, so a symlinked leaf must remain logical in the reported key. + return normalizeCodexHookSourcePath( + join(realpathSync.native(dirname(sourcePath)), basename(sourcePath)) + ) } catch { - return normalizeWindowsPathForCodexLookup(sourcePath) + return normalizeCodexHookSourcePath(sourcePath) } } +/** Matches the platform-native lexical path displayed by hook discovery. */ +export function normalizeCodexHookSourcePath(sourcePath: string): string { + if (isWindowsPathForTrustSource(sourcePath)) { + const withoutDevicePrefix = stripWindowsDevicePrefix(sourcePath) + const normalized = pathWin32.isAbsolute(withoutDevicePrefix) + ? pathWin32.normalize(withoutDevicePrefix) + : pathWin32.resolve(withoutDevicePrefix) + return trimNonRootTrailingSeparators(normalized, pathWin32.parse(normalized).root, /[\\/]/) + } + const normalized = pathPosix.isAbsolute(sourcePath) + ? pathPosix.normalize(sourcePath) + : pathPosix.resolve(sourcePath) + return trimNonRootTrailingSeparators(normalized, pathPosix.parse(normalized).root, /\//) +} + +function trimNonRootTrailingSeparators(path: string, root: string, separators: RegExp): string { + let end = path.length + while (end > root.length && separators.test(path[end - 1]!)) { + end -= 1 + } + return path.slice(0, end) +} + +function stripWindowsDevicePrefix(sourcePath: string): string { + const unc = /^(?:\\\\\?|\\\\\.)\\UNC\\/i.exec(sourcePath) + if (unc) { + return `\\\\${sourcePath.slice(unc[0].length)}` + } + const drive = /^(?:\\\\\?|\\\\\.)\\(?=[A-Za-z]:[\\/])/i.exec(sourcePath) + return drive ? sourcePath.slice(drive[0].length) : sourcePath +} + function getCodexCanonicalProjectPath(projectPath: string): string { try { // Why: local project trust still needs Codex's realpath shape, but remote @@ -193,13 +232,6 @@ function getCodexCanonicalProjectPath(projectPath: string): string { } } -function normalizeWindowsPathForCodexLookup(sourcePath: string): string { - if (!usesWindowsPathSeparators(sourcePath)) { - return sourcePath - } - return sourcePath.replace(/\//g, '\\') -} - function normalizeWindowsPathSeparators(sourcePath: string): string { if (!usesWindowsPathSeparators(sourcePath)) { return sourcePath @@ -208,13 +240,21 @@ function normalizeWindowsPathSeparators(sourcePath: string): string { } function usesWindowsPathSeparators(sourcePath: string): boolean { - return isWindowsDriveOrBackslashUncPath(sourcePath) || sourcePath.startsWith('//') + return isUnambiguousWindowsPath(sourcePath) || sourcePath.startsWith('//') } -function isWindowsDriveOrBackslashUncPath(sourcePath: string): boolean { +function isUnambiguousWindowsPath(sourcePath: string): boolean { return /^[A-Za-z]:[\\/]/.test(sourcePath) || sourcePath.startsWith('\\\\') } +function isWindowsPathForTrustSource(sourcePath: string): boolean { + return ( + isUnambiguousWindowsPath(sourcePath) || + (process.platform === 'win32' && + (sourcePath.startsWith('//') || !pathPosix.isAbsolute(sourcePath))) + ) +} + // Why: Codex and Orca can disagree on quote style, separators, and casing for // the same Windows project, including when the caller targets a remote host. export function normalizeCodexProjectPathForLookup(projectPath: string): string { @@ -336,7 +376,7 @@ export function upsertHookTrustEntriesInContent( const existing = existingContent.charCodeAt(0) === 0xfeff ? existingContent.slice(1) : existingContent let updated = entries.some((entry) => - usesWindowsPathSeparators(getCodexCanonicalTrustPath(entry.sourcePath)) + usesWindowsPathSeparators(normalizeCodexHookSourcePath(entry.sourcePath)) ) ? ensureHooksStateParentTable(existing) : existing @@ -546,7 +586,13 @@ export function normalizeHookTrustKeyForLookup(key: string): string { const parsed = parseTrustKey(key) // Why: fold by path shape, not host platform — hook sources on WSL and SSH // Windows remotes need the same folding when Orca runs on macOS or Linux. - const foldedPath = normalizeCodexProjectPathForLookup(parsed ? parsed.sourcePath : key) + const foldedPath = normalizeCodexProjectPathForLookup( + parsed + ? parsed.sourcePath.startsWith('//') + ? parsed.sourcePath + : normalizeCodexHookSourcePath(parsed.sourcePath) + : key + ) return parsed ? `${foldedPath}:${parsed.eventLabel}:${parsed.groupIndex}:${parsed.handlerIndex}` : foldedPath diff --git a/src/main/codex/hook-service.test.ts b/src/main/codex/hook-service.test.ts index 526a64d99ab..348a62a11f8 100644 --- a/src/main/codex/hook-service.test.ts +++ b/src/main/codex/hook-service.test.ts @@ -5,7 +5,6 @@ import { mkdirSync, mkdtempSync, readFileSync, - realpathSync, rmSync, symlinkSync, writeFileSync @@ -17,7 +16,12 @@ import { spawn } from 'node:child_process' import { createServer } from 'node:http' import type { AddressInfo } from 'node:net' import { createManagedCommandMatcher, wrapPosixHookCommand } from '../agent-hooks/installer-utils' -import { computeTrustedHash, upsertHookTrustEntriesInContent } from './config-toml-trust' +import { + computeTrustedHash, + getCodexExplicitHomeHookSourcePath, + normalizeCodexHookSourcePath, + upsertHookTrustEntriesInContent +} from './config-toml-trust' const { getPathMock, homedirMock } = vi.hoisted(() => ({ getPathMock: vi.fn<(name: string) => string>(), @@ -81,14 +85,14 @@ function escapeTomlBasicString(value: string): string { return value.replaceAll('\\', '\\\\').replaceAll('"', '\\"') } -function hookTrustHeader(key: string): string { - const canonicalKey = canonicalizeHookTrustKeyForTest(key) +function hookTrustHeader(key: string, useDefaultCodexHome = false): string { + const canonicalKey = canonicalizeHookTrustKeyForTest(key, useDefaultCodexHome) return /^[A-Za-z]:[\\/]|^\\\\/.test(canonicalKey) && !canonicalKey.includes("'") ? `[hooks.state.'${canonicalKey}']` : `[hooks.state."${escapeTomlBasicString(canonicalKey)}"]` } -function canonicalizeHookTrustKeyForTest(key: string): string { +function canonicalizeHookTrustKeyForTest(key: string, useDefaultCodexHome: boolean): string { const lastColon = key.lastIndexOf(':') const secondLast = lastColon === -1 ? -1 : key.lastIndexOf(':', lastColon - 1) const thirdLast = secondLast === -1 ? -1 : key.lastIndexOf(':', secondLast - 1) @@ -96,15 +100,10 @@ function canonicalizeHookTrustKeyForTest(key: string): string { return key } const sourcePath = key.slice(0, thirdLast) - try { - // Why: mirrors getCodexCanonicalTrustPath so test expectations match the - // native raw-backslash keys Codex writes after hook approval on Windows. - return `${realpathSync.native(sourcePath)}${key.slice(thirdLast)}` - } catch { - return /^[A-Za-z]:[\\/]|^\\\\/.test(sourcePath) - ? `${sourcePath.replace(/\//g, '\\')}${key.slice(thirdLast)}` - : key - } + const trustSourcePath = useDefaultCodexHome + ? normalizeCodexHookSourcePath(sourcePath) + : getCodexExplicitHomeHookSourcePath(sourcePath) + return `${trustSourcePath}${key.slice(thirdLast)}` } function markHookTrustDisabled(toml: string, header: string): string { @@ -473,7 +472,7 @@ describe('CodexHookService', () => { const runtimeToml = readFileSync(join(managedCodexHome, 'config.toml'), 'utf-8') expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:stop:1:0`)) expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:stop:0:0`)) - expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:0:0`)) + expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:0:0`, true)) }) it('runs managed PostToolUse status before mirrored user hooks', () => { @@ -521,7 +520,7 @@ describe('CodexHookService', () => { const runtimeToml = readFileSync(join(managedCodexHome, 'config.toml'), 'utf-8') expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:post_tool_use:0:0`)) expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:post_tool_use:1:0`)) - expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:post_tool_use:0:0`)) + expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:post_tool_use:0:0`, true)) }) it('mirrors system user hook approvals when the system trust indices are stale', () => { @@ -573,8 +572,8 @@ describe('CodexHookService', () => { expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:stop:0:0`)) expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:stop:1:0`)) expect(runtimeToml).toContain(hookTrustHeader(`${managedHooksPath}:stop:2:0`)) - expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:0:0`)) - expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:1:0`)) + expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:0:0`, true)) + expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:stop:1:0`, true)) }) it('skips plugin-placeholder system hooks when mirroring into runtime CODEX_HOME', () => { @@ -679,7 +678,7 @@ describe('CodexHookService', () => { )}\n`, 'utf-8' ) - const disabledPostCompactHeader = hookTrustHeader(`${systemHooksPath}:post_compact:0:0`) + const disabledPostCompactHeader = hookTrustHeader(`${systemHooksPath}:post_compact:0:0`, true) const systemToml = upsertHookTrustEntriesInContent('model = "system-model"\n', [ { sourcePath: systemHooksPath, @@ -719,8 +718,8 @@ describe('CodexHookService', () => { expect(runtimeToml).toContain( `${hookTrustHeader(`${managedHooksPath}:post_compact:0:0`)}\nenabled = false` ) - expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:pre_compact:0:0`)) - expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:post_compact:0:0`)) + expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:pre_compact:0:0`, true)) + expect(runtimeToml).not.toContain(hookTrustHeader(`${systemHooksPath}:post_compact:0:0`, true)) }) it('removes runtime user hook trust after system approval is revoked', () => { @@ -813,7 +812,7 @@ describe('CodexHookService', () => { })}\n`, 'utf-8' ) - const disabledStopHeader = hookTrustHeader(`${systemHooksPath}:stop:0:0`) + const disabledStopHeader = hookTrustHeader(`${systemHooksPath}:stop:0:0`, true) const systemToml = upsertHookTrustEntriesInContent('model = "system-model"\n', [ { sourcePath: systemHooksPath, @@ -1336,7 +1335,7 @@ describe('CodexHookService', () => { `${managedHooksPath}:permission_request:0:0` ) const legacyPermissionHeader = `[hooks.state."${escapeTomlBasicString( - `${realpathSync.native(managedHooksPath).replace(/\\/g, '/')}:permission_request:0:0` + `${getCodexExplicitHomeHookSourcePath(managedHooksPath).replace(/\\/g, '/')}:permission_request:0:0` )}"]` const installedToml = readFileSync(runtimeTomlPath, 'utf-8') expect(installedToml).toContain(canonicalPermissionHeader) diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index fbc6ded0fad..957bfd669f1 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -37,7 +37,8 @@ import { computeTrustKey, computeTrustedHash, escapeTomlString, - getCodexCanonicalTrustPath, + getCodexExplicitHomeHookSourcePath, + normalizeCodexHookSourcePath, normalizeCodexProjectPathForLookup, normalizeHookTrustKeyForLookup, parseTrustKey, @@ -262,11 +263,11 @@ function removeStaleRuntimeHookTrustEntries( entry.trustedHash ?? computeTrustedHash(entry) ]) ) - const canonicalRuntimeHooksPath = getCodexCanonicalTrustPath(runtimeHooksPath) + const canonicalRuntimeHooksPath = getCodexExplicitHomeHookSourcePath(runtimeHooksPath) const staleKeys: string[] = [] for (const [key, state] of readHookTrustEntries(tomlPath)) { const parsed = parseTrustKey(key) - if (!parsed || getCodexCanonicalTrustPath(parsed.sourcePath) !== canonicalRuntimeHooksPath) { + if (!parsed || normalizeCodexHookSourcePath(parsed.sourcePath) !== canonicalRuntimeHooksPath) { continue } if (expectedHashes.get(normalizeHookTrustKeyForLookup(key)) === state.trustedHash) { @@ -434,13 +435,13 @@ function getTrustedSystemHookHashesByEvent( trustEntries: ReadonlyMap ): Map> { const trustedHashesByEvent = new Map>() - const canonicalSystemConfigPath = getCodexCanonicalTrustPath(systemConfigPath) + const canonicalSystemConfigPath = normalizeCodexHookSourcePath(systemConfigPath) for (const [key, state] of trustEntries) { const parsed = parseTrustKey(key) if (!parsed || !state.trustedHash) { continue } - if (getCodexCanonicalTrustPath(parsed.sourcePath) !== canonicalSystemConfigPath) { + if (normalizeCodexHookSourcePath(parsed.sourcePath) !== canonicalSystemConfigPath) { continue } let hashes = trustedHashesByEvent.get(parsed.eventLabel) @@ -469,6 +470,7 @@ function collectMirroredRuntimeUserHookTrustEntries( } const entries: MirroredRuntimeUserHookTrustEntry[] = [] + const trustSourcePath = getCodexExplicitHomeHookSourcePath(runtimeConfigPath) for (const [eventName, definitions] of Object.entries(runtimeHooks)) { if (!Array.isArray(definitions)) { continue @@ -480,7 +482,7 @@ function collectMirroredRuntimeUserHookTrustEntries( return } const entry = createCodexHookTrustEntry( - runtimeConfigPath, + trustSourcePath, eventName, groupIndex, handlerIndex, @@ -733,7 +735,8 @@ function removeRuntimeManagedHookTrustEntries(configPath: string): void { sourcePath: configPath, command: getManagedCommand(getManagedScriptPath()), managedEventLabels: CODEX_MANAGED_EVENT_LABELS, - timeoutSec: MANAGED_HOOK_TIMEOUT_SECONDS + timeoutSec: MANAGED_HOOK_TIMEOUT_SECONDS, + sourceUsesExplicitCodexHome: true }) } catch (error) { // Best effort — stale trust entries are harmless once hooks.json no @@ -1194,6 +1197,7 @@ export class CodexHookService { const missing: string[] = [] const trustMissing: string[] = [] const disabled: string[] = [] + const trustSourcePath = getCodexExplicitHomeHookSourcePath(configPath) let presentCount = 0 for (const eventName of CODEX_EVENTS) { const definitions = Array.isArray(config.hooks?.[eventName]) ? config.hooks![eventName]! : [] @@ -1227,7 +1231,7 @@ export class CodexHookService { // Codex folds the handler timeout into its trust hash. Hash the same // timeout here or status would report every managed hook as stale-trust. const trustInput: CodexTrustEntry = { - sourcePath: configPath, + sourcePath: trustSourcePath, eventLabel: CODEX_EVENT_LABEL[eventName], groupIndex: foundGroupIndex, handlerIndex: foundHandlerIndex, @@ -1352,6 +1356,7 @@ export class CodexHookService { ({ entry }) => entry ) const managedTrustEntries: CodexTrustEntry[] = [] + const trustSourcePath = getCodexExplicitHomeHookSourcePath(configPath) for (const eventName of CODEX_EVENTS) { const current = Array.isArray(nextHooks[eventName]) ? nextHooks[eventName] : [] const cleaned = removeManagedCommands(current, isManagedCommand) @@ -1365,7 +1370,7 @@ export class CodexHookService { // timeoutSec mirrors the hook's `timeout` so the trust hash matches the // entry actually written to hooks.json. managedTrustEntries.push({ - sourcePath: configPath, + sourcePath: trustSourcePath, eventLabel: CODEX_EVENT_LABEL[eventName], groupIndex: 0, handlerIndex: 0, diff --git a/src/main/codex/hook-trust-promotion.test.ts b/src/main/codex/hook-trust-promotion.test.ts index b497591db63..0f65b73b515 100644 --- a/src/main/codex/hook-trust-promotion.test.ts +++ b/src/main/codex/hook-trust-promotion.test.ts @@ -1,11 +1,20 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync +} from 'node:fs' import { tmpdir } from 'node:os' import type * as Os from 'node:os' import { join } from 'node:path' import { computeTrustKey, computeTrustedHash, + getCodexExplicitHomeHookSourcePath, readHookTrustEntries, upsertHookTrustEntriesInContent, type CodexTrustEntry @@ -97,7 +106,7 @@ function runtimeUserStopEntry(): CodexTrustEntry { // install() prepends the managed status hook on Stop, so the mirrored user // hook lands at groupIndex 1. return { - sourcePath: join(runtimeHomeDir(), 'hooks.json'), + sourcePath: getCodexExplicitHomeHookSourcePath(join(runtimeHomeDir(), 'hooks.json')), eventLabel: 'stop', groupIndex: 1, handlerIndex: 0, @@ -121,6 +130,25 @@ function readSystemToml(): string { } describe('codex hook trust write-back promotion', () => { + it('mirrors default-home trust when the .codex directory is a symlink', () => { + const targetHome = join(tmpHome, 'dotfiles-codex') + mkdirSync(targetHome) + symlinkSync(targetHome, systemCodexDir(), process.platform === 'win32' ? 'junction' : 'dir') + writeSystemUserHook() + const systemEntry = systemUserStopEntry() + writeFileSync( + join(systemCodexDir(), 'config.toml'), + upsertHookTrustEntriesInContent('', [systemEntry]) + ) + + expect(new CodexHookService().install().state).toBe('installed') + + const runtimeTrust = readHookTrustEntries(join(runtimeHomeDir(), 'config.toml')) + expect(runtimeTrust.get(computeTrustKey(runtimeUserStopEntry()))?.trustedHash).toBe( + computeTrustedHash(systemEntry) + ) + }) + it('keeps an in-Orca approval of a user hook across launches and promotes it to ~/.codex', () => { writeSystemUserHook() const service = new CodexHookService() diff --git a/src/main/codex/hook-trust-promotion.ts b/src/main/codex/hook-trust-promotion.ts index 36a570a662f..8e4a4a58668 100644 --- a/src/main/codex/hook-trust-promotion.ts +++ b/src/main/codex/hook-trust-promotion.ts @@ -7,7 +7,8 @@ import { } from '../agent-hooks/installer-utils' import { getOrcaManagedCodexHomePath, getSystemCodexHomePath } from './codex-home-paths' import { - getCodexCanonicalTrustPath, + getCodexExplicitHomeHookSourcePath, + normalizeCodexHookSourcePath, normalizeHookTrustKeyForLookup, parseTrustKey, readHookTrustEntries, @@ -81,11 +82,14 @@ export function snapshotCodexRuntimeHookTrustProvenance(): void { try { const runtimeHomePath = getOrcaManagedCodexHomePath() const runtimeHooksPath = join(runtimeHomePath, 'hooks.json') - const canonicalRuntimeHooksPath = getCodexCanonicalTrustPath(runtimeHooksPath) + const canonicalRuntimeHooksPath = getCodexExplicitHomeHookSourcePath(runtimeHooksPath) const entries: Record = {} for (const [key, state] of readHookTrustEntries(join(runtimeHomePath, 'config.toml'))) { const parsed = parseTrustKey(key) - if (!parsed || getCodexCanonicalTrustPath(parsed.sourcePath) !== canonicalRuntimeHooksPath) { + if ( + !parsed || + normalizeCodexHookSourcePath(parsed.sourcePath) !== canonicalRuntimeHooksPath + ) { continue } entries[normalizeHookTrustKeyForLookup(key)] = { @@ -126,8 +130,8 @@ function promoteCodexRuntimeHookApprovalsToSystemUnsafe(): void { const systemHomePath = getSystemCodexHomePath() const runtimeHooksPath = join(runtimeHomePath, 'hooks.json') const systemHooksPath = join(systemHomePath, 'hooks.json') - const canonicalRuntimeHooksPath = getCodexCanonicalTrustPath(runtimeHooksPath) - if (canonicalRuntimeHooksPath === getCodexCanonicalTrustPath(systemHooksPath)) { + const canonicalRuntimeHooksPath = getCodexExplicitHomeHookSourcePath(runtimeHooksPath) + if (canonicalRuntimeHooksPath === normalizeCodexHookSourcePath(systemHooksPath)) { return } const runtimeTomlPath = join(runtimeHomePath, 'config.toml') @@ -164,7 +168,7 @@ function promoteCodexRuntimeHookApprovalsToSystemUnsafe(): void { continue } const parsed = parseTrustKey(key) - if (!parsed || getCodexCanonicalTrustPath(parsed.sourcePath) !== canonicalRuntimeHooksPath) { + if (!parsed || normalizeCodexHookSourcePath(parsed.sourcePath) !== canonicalRuntimeHooksPath) { continue } const previous = provenance.get(normalizeHookTrustKeyForLookup(key)) @@ -191,7 +195,7 @@ function promoteCodexRuntimeHookApprovalsToSystemUnsafe(): void { continue } const runtimeEntry = createCodexHookTrustEntry( - runtimeHooksPath, + canonicalRuntimeHooksPath, eventName, parsed.groupIndex, parsed.handlerIndex,