From bfea35fbb5b82da508e47e35a6d1cedaa6c78ba5 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 17 Jul 2026 02:22:14 -0700 Subject: [PATCH] fix(codex): sanitize managed account config trust --- src/main/codex-accounts/service.test.ts | 183 +++++++++++++++++- src/main/codex-accounts/service.ts | 27 ++- .../codex-managed-trust-reconciliation.ts | 35 +++- src/main/codex/config-toml-trust.ts | 21 +- 4 files changed, 251 insertions(+), 15 deletions(-) diff --git a/src/main/codex-accounts/service.test.ts b/src/main/codex-accounts/service.test.ts index 9b185e4c357..032a79668af 100644 --- a/src/main/codex-accounts/service.test.ts +++ b/src/main/codex-accounts/service.test.ts @@ -16,6 +16,7 @@ import { join } from 'node:path' import { PassThrough } from 'node:stream' import type { GlobalSettings } from '../../shared/types' import { buildWslCodexAvailabilityArgs, buildWslCodexLoginArgs } from './wsl-codex-command' +import type { readHookTrustEntries as ReadHookTrustEntries } from '../codex/config-toml-trust' const testState = { userDataDir: '', @@ -220,6 +221,65 @@ function createCodexAuthJson(email: string, accountId: string, refreshToken: str )}\n` } +async function createCanonicalHookTrustFixture(): Promise<{ + config: string + orcaKeys: string[] + userKey: string +}> { + const { MANAGED_HOOK_TIMEOUT_SECONDS } = await import('../agent-hooks/installer-utils') + const { getCodexManagedHookInstallMaterial } = await import('../codex/hook-service') + const { computeTrustKey, computeTrustedHash, escapeTomlString, normalizeHookTrustKeyForLookup } = + await import('../codex/config-toml-trust') + const { getCodexHookTrustSignature } = await import('../codex/codex-hook-identity') + const { writeCodexTrustGrantLedgerHome } = await import('../codex/codex-trust-grant-ledger') + const sourceHomePath = join(testState.fakeHomeDir, '.codex') + const sourcePath = join(sourceHomePath, 'hooks.json') + const material = getCodexManagedHookInstallMaterial() + const expectedHashEntry = { + sourcePath, + eventLabel: 'stop' as const, + groupIndex: 0, + handlerIndex: 0, + command: material.command, + timeoutSec: MANAGED_HOOK_TIMEOUT_SECONDS + } + const ledgerHashEntry = { + ...expectedHashEntry, + eventLabel: 'session_start' as const, + groupIndex: 1 + } + const userEntry = { + ...expectedHashEntry, + groupIndex: 2, + command: 'user-authored-hook' + } + const expectedHashKey = computeTrustKey(expectedHashEntry) + const ledgerHashKey = computeTrustKey(ledgerHashEntry) + const userKey = computeTrustKey(userEntry) + const ledgerTrustedHash = 'sha256:codex-granted-orca-hook' + writeCodexTrustGrantLedgerHome(sourceHomePath, { + binary: null, + entries: { + [normalizeHookTrustKeyForLookup(ledgerHashKey)]: { + signature: getCodexHookTrustSignature(ledgerHashEntry), + trustedHash: ledgerTrustedHash + } + } + }) + const block = (key: string, trustedHash: string): string => + `[hooks.state."${escapeTomlString(key)}"]\ntrusted_hash = "${trustedHash}"\nenabled = true` + return { + config: [ + 'approval_policy = "never"', + block(expectedHashKey, computeTrustedHash(expectedHashEntry)), + block(ledgerHashKey, ledgerTrustedHash), + block(userKey, computeTrustedHash(userEntry)) + ].join('\n\n'), + orcaKeys: [expectedHashKey, ledgerHashKey], + userKey + } +} + describe('CodexAccountService config sync', () => { beforeEach(() => { vi.resetModules() @@ -280,6 +340,60 @@ describe('CodexAccountService config sync', () => { ) }) + it('strips only Orca-owned source-home hook trust on startup and account selection', async () => { + const fixture = await createCanonicalHookTrustFixture() + const canonicalConfigPath = join(testState.fakeHomeDir, '.codex', 'config.toml') + writeFileSync(canonicalConfigPath, fixture.config, 'utf-8') + const managedHomePath = createManagedHome( + testState.userDataDir, + 'account-1', + 'approval_policy = "on-request"\n' + ) + const settings = createSettings({ + codexManagedAccounts: [ + { + id: 'account-1', + email: 'user@example.com', + managedHomePath, + providerAccountId: null, + workspaceLabel: null, + workspaceAccountId: null, + createdAt: 1, + updatedAt: 1, + lastAuthenticatedAt: 1 + } + ] + }) + const store = createStore(settings) + const rateLimits = createRateLimits() + const runtimeHome = createRuntimeHome() + const { readHookTrustEntries } = await import('../codex/config-toml-trust') + const { readCodexTrustGrantLedgerHome } = await import('../codex/codex-trust-grant-ledger') + + const { CodexAccountService } = await import('./service') + const service = new CodexAccountService( + store as never, + rateLimits as never, + runtimeHome as never + ) + const expectSanitizedManagedConfig = (): void => { + const entries = readHookTrustEntries(join(managedHomePath, 'config.toml')) + for (const key of fixture.orcaKeys) { + expect(entries.has(key)).toBe(false) + } + expect(entries.has(fixture.userKey)).toBe(true) + } + + expectSanitizedManagedConfig() + expect(readFileSync(canonicalConfigPath, 'utf-8')).toBe(fixture.config) + expect(readCodexTrustGrantLedgerHome(join(testState.fakeHomeDir, '.codex'))).not.toBeNull() + + writeFileSync(join(managedHomePath, 'config.toml'), 'approval_policy = "untrusted"\n', 'utf-8') + await service.selectAccount('account-1') + + expectSanitizedManagedConfig() + }) + it('rewrites relative path config values when syncing into managed homes', async () => { const canonicalConfigPath = join(testState.fakeHomeDir, '.codex', 'config.toml') writeFileSync( @@ -325,7 +439,15 @@ describe('CodexAccountService config sync', () => { it('does not rewrite managed configs that already match canonical config', async () => { const canonicalConfigPath = join(testState.fakeHomeDir, '.codex', 'config.toml') - const canonicalConfig = 'approval_policy = "never"\nsandbox_mode = "danger-full-access"\n' + const { escapeTomlString } = await import('../codex/config-toml-trust') + const userHookKey = `${join(testState.fakeHomeDir, '.codex', 'user-hooks.json')}:stop:0:0` + const canonicalConfig = [ + 'approval_policy = "never"', + 'sandbox_mode = "danger-full-access"', + `[hooks.state."${escapeTomlString(userHookKey)}"]`, + 'trusted_hash = "sha256:user-owned"', + '' + ].join('\n') writeFileSync(canonicalConfigPath, canonicalConfig, 'utf-8') const managedHomePath = createManagedHome( testState.userDataDir, @@ -564,6 +686,65 @@ describe('CodexAccountService config sync', () => { expect(runtimeHome.syncForCurrentSelection).toHaveBeenCalledTimes(1) }) + it('does not seed Orca-owned source-home trust when adding an account', async () => { + vi.resetModules() + let fixture: Awaited> + let readHookTrustEntries: typeof ReadHookTrustEntries + const spawnMock = vi.fn( + (_command: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => { + const child = new EventEmitter() as EventEmitter & { + stdout: PassThrough + stderr: PassThrough + kill: () => void + } + child.stdout = new PassThrough() + child.stderr = new PassThrough() + child.kill = vi.fn() + + const loginHome = options.env.CODEX_HOME + expect(loginHome).toBeTruthy() + const entries = readHookTrustEntries(join(loginHome!, 'config.toml')) + for (const key of fixture.orcaKeys) { + expect(entries.has(key)).toBe(false) + } + expect(entries.has(fixture.userKey)).toBe(true) + writeFileSync( + join(loginHome!, 'auth.json'), + createCodexAuthJson('user@example.com', 'provider-account-1', 'refresh-token'), + 'utf-8' + ) + + queueMicrotask(() => child.emit('close', 0)) + return child + } + ) + + vi.doMock('node:child_process', () => ({ + execFileSync: vi.fn(), + spawn: spawnMock + })) + vi.doMock('../codex-cli/command', () => ({ + resolveCodexCommand: () => 'codex' + })) + fixture = await createCanonicalHookTrustFixture() + readHookTrustEntries = (await import('../codex/config-toml-trust')).readHookTrustEntries + writeFileSync(join(testState.fakeHomeDir, '.codex', 'config.toml'), fixture.config, 'utf-8') + + const store = createStore(createSettings()) + const rateLimits = createRateLimits() + const runtimeHome = createRuntimeHome() + const { CodexAccountService } = await import('./service') + const service = new CodexAccountService( + store as never, + rateLimits as never, + runtimeHome as never + ) + + await service.addAccount() + + expect(spawnMock).toHaveBeenCalledTimes(1) + }) + it('recreates the expected missing managed home before reauthenticating', async () => { vi.resetModules() diff --git a/src/main/codex-accounts/service.ts b/src/main/codex-accounts/service.ts index 2a0fb54bdc4..ff56fb4d036 100644 --- a/src/main/codex-accounts/service.ts +++ b/src/main/codex-accounts/service.ts @@ -16,6 +16,9 @@ import type { import type { CodexRuntimeHomeService } from './runtime-home-service' import { writeFileAtomically } from './fs-utils' import { rewriteRelativePathConfigValues } from '../codex/codex-config-path-reference-rewrite' +import { stripCodexManagedHookTrustEntriesFromConfig } from '../codex/codex-managed-trust-reconciliation' +import { getCodexManagedHookInstallMaterial } from '../codex/hook-service' +import { MANAGED_HOOK_TIMEOUT_SECONDS } from '../agent-hooks/installer-utils' import { resolveCodexCommand } from '../codex-cli/command' import type { Store } from '../persistence' import type { RateLimitService } from '../rate-limits/service' @@ -58,6 +61,7 @@ type CanonicalCodexConfig = { /** Home the config was read from, in the path style Codex sees at runtime * (Linux-side for WSL); relative path-valued settings resolve against it. */ sourceHomePath: string + sourceHooksPath: string } export type CodexAccountAddTarget = { @@ -468,9 +472,18 @@ export class CodexAccountService { // account while preserving consistent Codex behavior. Managed homes are // real CODEX_HOMEs for `codex login`, so relative path-valued settings // must keep resolving against the home the config was read from. + const material = getCodexManagedHookInstallMaterial() + // Why: source-home Orca trust is foreign to each managed home's hooks.json. + const sanitizedConfig = stripCodexManagedHookTrustEntriesFromConfig(canonicalConfig.contents, { + runtimeHomePath: canonicalConfig.sourceHomePath, + sourcePath: canonicalConfig.sourceHooksPath, + command: material.command, + managedEventLabels: new Set(Object.values(material.eventLabel)), + timeoutSec: MANAGED_HOOK_TIMEOUT_SECONDS + }) this.writeManagedConfig( trustedManagedHomePath, - rewriteRelativePathConfigValues(canonicalConfig.contents, canonicalConfig.sourceHomePath) + rewriteRelativePathConfigValues(sanitizedConfig, canonicalConfig.sourceHomePath) ) } @@ -482,7 +495,11 @@ export class CodexAccountService { } try { - return { contents: readFileSync(primaryConfigPath, 'utf-8'), sourceHomePath } + return { + contents: readFileSync(primaryConfigPath, 'utf-8'), + sourceHomePath, + sourceHooksPath: join(sourceHomePath, 'hooks.json') + } } catch (error) { console.warn('[codex-accounts] Failed to read canonical config:', error) return null @@ -509,7 +526,11 @@ export class CodexAccountService { try { // Why: the config is read over UNC but consumed by Codex inside WSL, so // path rewrites must anchor to the Linux-side ~/.codex, not the UNC path. - return { contents: readFileSync(configPath, 'utf-8'), sourceHomePath: `${wslHome}/.codex` } + return { + contents: readFileSync(configPath, 'utf-8'), + sourceHomePath: `${wslHome}/.codex`, + sourceHooksPath: `${wslHome}/.codex/hooks.json` + } } catch (error) { console.warn('[codex-accounts] Failed to read WSL canonical config:', error) return null diff --git a/src/main/codex/codex-managed-trust-reconciliation.ts b/src/main/codex/codex-managed-trust-reconciliation.ts index b6c9e9d5d3a..78567134f98 100644 --- a/src/main/codex/codex-managed-trust-reconciliation.ts +++ b/src/main/codex/codex-managed-trust-reconciliation.ts @@ -6,8 +6,11 @@ import { normalizeCodexHookSourcePath, parseTrustKey, readHookTrustEntries, + readHookTrustEntriesFromContent, removeHookTrustEntries, + removeHookTrustEntriesFromContent, type CodexEventLabel, + type CodexHookTrustState, type CodexTrustEntry } from './config-toml-trust' import { getCodexHookTrustSignature } from './codex-hook-identity' @@ -52,8 +55,7 @@ function addLedgerRecognizedHashes( } } -export function removeCodexManagedHookTrustEntries(options: { - tomlPath: string +type CodexManagedHookTrustOwnershipOptions = { runtimeHomePath: string sourcePath: string command: string @@ -61,8 +63,12 @@ export function removeCodexManagedHookTrustEntries(options: { timeoutSec: number /** Explicit native homes resolve their parent before hook discovery. */ sourceUsesExplicitCodexHome?: boolean -}): void { - const existingEntries = readHookTrustEntries(options.tomlPath) +} + +function getCodexManagedHookTrustEntryKeys( + existingEntries: ReadonlyMap, + options: CodexManagedHookTrustOwnershipOptions +): string[] { const ledgerHome = readCodexTrustGrantLedgerHomeForReconciliation(options.runtimeHomePath) const expectedSourcePath = options.sourceUsesExplicitCodexHome ? getCodexExplicitHomeHookSourcePath(options.sourcePath) @@ -94,6 +100,27 @@ export function removeCodexManagedHookTrustEntries(options: { ownedKeys.push(key) } } + return ownedKeys +} + +export function stripCodexManagedHookTrustEntriesFromConfig( + contents: string, + options: CodexManagedHookTrustOwnershipOptions +): string { + const ownedKeys = getCodexManagedHookTrustEntryKeys( + readHookTrustEntriesFromContent(contents), + options + ) + return removeHookTrustEntriesFromContent(contents, ownedKeys) +} + +export function removeCodexManagedHookTrustEntries( + options: CodexManagedHookTrustOwnershipOptions & { tomlPath: string } +): void { + const ownedKeys = getCodexManagedHookTrustEntryKeys( + readHookTrustEntries(options.tomlPath), + options + ) if (ownedKeys.length > 0) { removeHookTrustEntries(options.tomlPath, ownedKeys) } diff --git a/src/main/codex/config-toml-trust.ts b/src/main/codex/config-toml-trust.ts index 63a7f7555e8..28445d40490 100644 --- a/src/main/codex/config-toml-trust.ts +++ b/src/main/codex/config-toml-trust.ts @@ -876,16 +876,20 @@ export function removeHookTrustEntries(configPath: string, keys: readonly string return } const existing = readTomlFile(configPath) - let updated = existing - for (const key of keys) { - updated = removeTrustBlock(updated, key) - } + const updated = removeHookTrustEntriesFromContent(existing, keys) if (updated === existing) { return } writeConfigAtomically(configPath, updated) } +export function removeHookTrustEntriesFromContent( + content: string, + keys: readonly string[] +): string { + return keys.reduce((updated, key) => removeTrustBlock(updated, key), content) +} + function removeTrustBlock(content: string, key: string): string { const ranges = findTrustBlockRanges(content, key) if (ranges.length === 0) { @@ -902,11 +906,14 @@ function removeTrustBlock(content: string, key: string): string { } export function readHookTrustEntries(configPath: string): Map { - const result = new HookTrustEntryMap() if (!existsSync(configPath)) { - return result + return new HookTrustEntryMap() } - const content = readTomlFile(configPath) + return readHookTrustEntriesFromContent(readTomlFile(configPath)) +} + +export function readHookTrustEntriesFromContent(content: string): Map { + const result = new HookTrustEntryMap() // Why: walk line-by-line so `[hooks.state."..."]` inside a `"""..."""` or // `'''...'''` multi-line string isn't mistaken for a real header. let cursor = 0