fix(codex): never write a Codex config.toml that Codex can't load, and approve both symlink spellings (#25741)

* fix(codex): never write a hook approval Codex cannot load, and approve both symlink spellings

- Refuse any hooks.state write into a Codex config.toml (upsert, move,
  remove, mirrored enabled state, SSH installer) that would turn a loadable
  file into one Codex cannot load; write nothing and surface the reason.
- Read approvals written as dotted keys or inline tables.
- Approve and remove Orca's ~/.codex hook under both the spelled and the
  resolved key when ~/.codex or HOME is a symlink; move user approvals
  under both keys.
- Stale runtime trust cleanup no longer keeps an unexpected key whose
  conflicting duplicate tables read as no hash.

* refactor(codex): pass every hooks.json spelling as one sourcePaths list

* fix(codex): sweep retired-hook approvals under every key spelling; read literal-string trusted_hash
This commit is contained in:
Jinwoo Hong
2026-10-06 02:57:25 -04:00
committed by GitHub
parent 1de3aa405f
commit 29e669680d
19 changed files with 808 additions and 52 deletions
+15 -10
View File
@@ -21,6 +21,7 @@ import {
removeSelfComputedMatchingTrustEntries
} from './codex-hook-trust-cleanup'
import { runExclusivelyForCodexTrustConfig } from './codex-trust-config-mutation-queue'
import { getRealHomeHookKeySourcePaths } from './codex-real-home-hooks-json'
import { mutateRealHomeHooksPreservingUserTrust } from './codex-user-hook-trust-moves'
const LEGACY_ORCA_PROFILE_NAME = 'orca-agent-status'
@@ -61,6 +62,9 @@ async function sweepLegacySystemManagedHooks(): Promise<void> {
return
}
// Why every spelling: with a symlinked home, Codex may have approved the
// retired hook under its resolved key too.
const sourcePaths = getRealHomeHookKeySourcePaths()
const nextHooks = { ...config.hooks }
const trustEntries: CodexTrustEntry[] = []
let removedManagedHook = false
@@ -68,15 +72,16 @@ async function sweepLegacySystemManagedHooks(): Promise<void> {
if (!Array.isArray(definitions)) {
continue
}
const eventTrustEntries = collectManagedTrustEntries(
legacyConfigPath,
eventName,
definitions,
isRetiredCodexHookCommand
)
// Why: user hook configs can be large; avoid the argument limit from push(...entries).
for (const entry of eventTrustEntries) {
trustEntries.push(entry)
for (const sourcePath of sourcePaths) {
// Why: user hook configs can be large; avoid the argument limit from push(...entries).
for (const entry of collectManagedTrustEntries(
sourcePath,
eventName,
definitions,
isRetiredCodexHookCommand
)) {
trustEntries.push(entry)
}
}
const cleaned = removeManagedCommands(definitions, isRetiredCodexHookCommand)
removedManagedHook ||= definitions.some((definition) =>
@@ -95,7 +100,7 @@ async function sweepLegacySystemManagedHooks(): Promise<void> {
// Remove only retired Orca hook entries and preserve other managers' metadata.
const hooksWritePath = resolveHooksJsonWritePath(legacyConfigPath)
mutateRealHomeHooksPreservingUserTrust({
sourcePath: legacyConfigPath,
sourcePaths,
tomlPath: getSystemCodexConfigTomlPath(),
beforeHooks: config.hooks,
afterHooks: nextHooks,
+6 -1
View File
@@ -13,7 +13,11 @@ import {
writeManagedScriptRemote,
writeTextFileRemoteAtomic
} from '../agent-hooks/installer-utils-remote'
import { upsertHookTrustEntriesInContent, type CodexTrustEntry } from './config-toml-trust'
import {
assertLoadableHookTrustConfig,
upsertHookTrustEntriesInContent,
type CodexTrustEntry
} from './config-toml-trust'
import {
CODEX_EVENTS,
CODEX_EVENT_LABEL,
@@ -108,6 +112,7 @@ export async function installCodexHooksRemote(
const existingToml = existingTomlRaw ?? ''
const updatedToml = upsertHookTrustEntriesInContent(existingToml, trustEntries)
if (updatedToml !== existingToml) {
assertLoadableHookTrustConfig(remoteTomlPath, existingToml, updatedToml)
await writeTextFileRemoteAtomic(sftp, remoteTomlPath, updatedToml)
}
} catch (error) {
@@ -0,0 +1,50 @@
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import { mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import {
computeTrustKey,
escapeTomlString,
readHookTrustEntries,
type CodexTrustEntry
} from './config-toml-trust'
import { removeStaleRuntimeHookTrustEntries } from './codex-hook-trust-cleanup'
let dir: string
let tomlPath: string
let hooksPath: string
function entry(command: string, groupIndex: number): CodexTrustEntry {
return { sourcePath: hooksPath, eventLabel: 'stop', groupIndex, handlerIndex: 0, command }
}
beforeEach(() => {
// Why realpath: runtime keys are resolved, and the temp dir may sit under a symlink.
dir = realpathSync.native(mkdtempSync(join(tmpdir(), 'orca-codex-trust-cleanup-')))
tomlPath = join(dir, 'config.toml')
hooksPath = join(dir, 'hooks.json')
})
afterEach(() => {
rmSync(dir, { recursive: true, force: true })
})
describe('removeStaleRuntimeHookTrustEntries', () => {
it('removes an unexpected key whose duplicate tables disagree, so read as no hash', () => {
const expected = { ...entry('orca.sh', 0), trustedHash: 'sha256:orca' }
const staleKey = escapeTomlString(computeTrustKey(entry('gone.sh', 1)))
writeFileSync(
tomlPath,
`[hooks.state."${escapeTomlString(computeTrustKey(expected))}"]\ntrusted_hash = "sha256:orca"\n\n` +
`[hooks.state."${staleKey}"]\ntrusted_hash = "sha256:a"\n\n` +
`[hooks.state."${staleKey}"]\ntrusted_hash = "sha256:b"\n`
)
removeStaleRuntimeHookTrustEntries(tomlPath, hooksPath, [expected])
expect(readFileSync(tomlPath, 'utf-8')).not.toContain(staleKey)
expect(readHookTrustEntries(tomlPath).get(computeTrustKey(expected))?.trustedHash).toBe(
'sha256:orca'
)
})
})
+8 -5
View File
@@ -95,7 +95,10 @@ export function removeStaleRuntimeHookTrustEntries(
if (!parsed || !codexHookSourcePathsEqual(parsed.sourcePath, canonicalRuntimeHooksPath)) {
continue
}
if (expectedHashes.get(normalizeHookTrustKeyForLookup(key)) === state.trustedHash) {
const expectedHash = expectedHashes.get(normalizeHookTrustKeyForLookup(key))
// Why a defined hash: conflicting duplicate tables read as no hash, and an
// unexpected key must not match that and survive.
if (expectedHash !== undefined && expectedHash === state.trustedHash) {
continue
}
staleKeys.push(key)
@@ -107,12 +110,12 @@ export function removeStaleRuntimeHookTrustEntries(
export function removeSystemManagedHookTrustEntries(
systemHomePath: string,
hooksJsonPath: string
sourcePaths: readonly [string, ...string[]]
): void {
removeCodexManagedHookTrustEntries({
tomlPath: getSystemCodexConfigTomlPath(),
runtimeHomePath: systemHomePath,
sourcePath: hooksJsonPath,
sourcePaths,
command: getManagedCommand(getManagedScriptPath()),
managedEventLabels: CODEX_MANAGED_EVENT_LABELS,
timeoutSec: MANAGED_HOOK_TIMEOUT_SECONDS
@@ -124,7 +127,7 @@ export function removeRuntimeManagedHookTrustEntries(configPath: string): void {
removeCodexManagedHookTrustEntries({
tomlPath: getCodexConfigTomlPath(),
runtimeHomePath: getOrcaManagedCodexHomePath(),
sourcePath: configPath,
sourcePaths: [configPath],
command: getManagedCommand(getManagedScriptPath()),
managedEventLabels: CODEX_MANAGED_EVENT_LABELS,
timeoutSec: MANAGED_HOOK_TIMEOUT_SECONDS,
@@ -143,7 +146,7 @@ export function removeWslRuntimeManagedHookTrustEntries(
removeCodexManagedHookTrustEntries({
tomlPath: plan.tomlPath,
runtimeHomePath: pathWin32.dirname(plan.tomlPath),
sourcePath: plan.trustConfigPath,
sourcePaths: [plan.trustConfigPath],
command: wrapReadablePosixHookCommand(plan.commandScriptPath),
managedEventLabels: CODEX_MANAGED_EVENT_LABELS,
timeoutSec: MANAGED_HOOK_TIMEOUT_SECONDS
+2 -2
View File
@@ -9,7 +9,7 @@ import {
escapeTomlString,
getCodexExplicitHomeHookSourcePath,
parseTrustKey,
writeConfigAtomically,
writeLoadableHookTrustConfig,
type CodexTrustEntry
} from './config-toml-trust'
import { createCodexHookTrustEntry, getCodexHookTrustSignature } from './codex-hook-identity'
@@ -202,7 +202,7 @@ export function applyMirroredRuntimeUserHookTrustStates(
updated = updated.replace(pattern, `$1${enabled}`)
}
if (updated !== existing) {
writeConfigAtomically(tomlPath, updated)
writeLoadableHookTrustConfig(tomlPath, existing, updated)
}
}
@@ -58,7 +58,8 @@ function addLedgerRecognizedHashes(
type CodexManagedHookTrustOwnershipOptions = {
runtimeHomePath: string
sourcePath: string
/** hooks.json first, then other spellings Codex may key it by (same hash). */
sourcePaths: readonly [string, ...string[]]
command: string
managedEventLabels: ReadonlySet<CodexEventLabel>
timeoutSec: number
@@ -71,17 +72,21 @@ function getCodexManagedHookTrustEntryKeys(
options: CodexManagedHookTrustOwnershipOptions
): string[] {
const ledgerHome = readCodexTrustGrantLedgerHomeForReconciliation(options.runtimeHomePath)
const [sourcePath, ...aliases] = options.sourcePaths
const expectedSourcePath = options.sourceUsesExplicitCodexHome
? getCodexExplicitHomeHookSourcePath(options.sourcePath)
: normalizeCodexHookSourcePath(options.sourcePath)
? getCodexExplicitHomeHookSourcePath(sourcePath)
: normalizeCodexHookSourcePath(sourcePath)
const aliasSourcePaths = aliases.map(normalizeCodexHookSourcePath)
const ownedKeys: string[] = []
for (const [key, state] of existingEntries) {
const parts = parseTrustKey(key)
if (
!parts ||
!codexHookSourcePathsEqual(parts.sourcePath, expectedSourcePath) ||
!options.managedEventLabels.has(parts.eventLabel)
) {
if (!parts || !options.managedEventLabels.has(parts.eventLabel)) {
continue
}
const isAlias = aliasSourcePaths.some((alias) =>
codexHookSourcePathsEqual(parts.sourcePath, alias)
)
if (!isAlias && !codexHookSourcePathsEqual(parts.sourcePath, expectedSourcePath)) {
continue
}
const expectedEntry: CodexTrustEntry = {
@@ -97,6 +102,15 @@ function getCodexManagedHookTrustEntryKeys(
computeTrustedHash({ ...expectedEntry, timeoutSec: undefined })
])
addLedgerRecognizedHashes(recognizedHashes, [ledgerHome], key, expectedEntry)
if (isAlias) {
// Why: the ledger records Codex's grant under the primary spelling's key only.
addLedgerRecognizedHashes(
recognizedHashes,
[ledgerHome],
computeTrustKey(expectedEntry),
expectedEntry
)
}
if (state.trustedHash && recognizedHashes.has(state.trustedHash)) {
ownedKeys.push(key)
}
+30 -3
View File
@@ -9,8 +9,10 @@ import {
assertHooksJsonGeneration,
backupRealHomeHooksJsonOnce,
getRealHomeConfigTomlPath,
getRealHomeHookKeySourcePaths,
getRealHomeHooksJsonPath
} from './codex-real-home-hooks-json'
import { upsertHookTrustEntries, type CodexTrustEntry } from './config-toml-trust'
import { getCodexManagedScriptFileName } from './codex-hook-identity'
import {
CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS,
@@ -212,6 +214,9 @@ async function settleApproval(
console.warn('[codex-real-home-hooks] background trust grant failed:', error)
}
}
if (outcome?.lane === 'rpc' && readCodexHooksEnabled()) {
approveOtherRealHomeKeySpellings(outcome.entries)
}
installRetryAfterMs = recordRealHomeApprovalOutcome(outcome)
approval = null
// Why from the settings: hooks turned off during the session must not read as
@@ -268,7 +273,7 @@ async function installRealHomeCodexHook(
if (plan.changed) {
backupRealHomeHooksJsonOnce(userDataPath, previousRaw)
mutateRealHomeHooksPreservingUserTrust({
sourcePath: hooksJsonPath,
sourcePaths: getRealHomeHookKeySourcePaths(),
tomlPath: getRealHomeConfigTomlPath(),
beforeHooks: config.hooks ?? {},
afterHooks: plan.hooks,
@@ -295,7 +300,9 @@ async function installRealHomeCodexHook(
useDefaultCodexHome: true,
background: true
}
if (await findCurrentManagedCodexHookTrust(grantPlan)) {
const current = await findCurrentManagedCodexHookTrust(grantPlan)
if (current) {
approveOtherRealHomeKeySpellings(current)
return { verdict: 'installed' }
}
return {
@@ -304,6 +311,26 @@ async function installRealHomeCodexHook(
}
}
/**
* Codex's grant keys ~/.codex as spelled; a pane whose CODEX_HOME names a
* symlinked home keys it resolved. Codex's hash ignores the path, so it carries.
*/
function approveOtherRealHomeKeySpellings(granted: readonly CodexTrustEntry[]): void {
const [, ...otherSpellings] = getRealHomeHookKeySourcePaths()
if (otherSpellings.length === 0 || granted.length === 0) {
return
}
try {
upsertHookTrustEntries(
getRealHomeConfigTomlPath(),
otherSpellings.flatMap((sourcePath) => granted.map((entry) => ({ ...entry, sourcePath })))
)
} catch (error) {
// Why not a failure: the spelled key Codex wrote still approves default-home panes.
console.warn('[codex-real-home-hooks] could not approve the resolved ~/.codex key:', error)
}
}
/**
* The user's explicit opt-out: strips Orca's entry and its trust from the real
* ~/.codex. Joins the system lane an opt-out caller already holds.
@@ -320,7 +347,7 @@ export async function removeRealHomeCodexHookForOptOut(): Promise<RealHomeCodexH
readCodexTrustGrantLedgerHomeForReconciliation(systemHomePath) !== null
) {
// Why: the ledger outlives a sweep that removed the entry but not its trust.
removeSystemManagedHookTrustEntries(systemHomePath, getRealHomeHooksJsonPath())
removeSystemManagedHookTrustEntries(systemHomePath, getRealHomeHookKeySourcePaths())
}
return lane
})
@@ -0,0 +1,269 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import {
mkdirSync,
mkdtempSync,
readFileSync,
realpathSync,
rmSync,
symlinkSync,
writeFileSync
} from 'node:fs'
import type * as NodeOs from 'node:os'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import type { CodexManagedTrustGrantPlan } from './codex-hook-trust-grant'
import {
computeTrustKey,
getCodexExplicitHomeHookSourcePath,
normalizeCodexHookSourcePath,
normalizeHookTrustKeyForLookup,
readHookTrustEntries,
upsertHookTrustEntries,
type CodexTrustEntry
} from './config-toml-trust'
const { homedirMock, grantMock, findCurrentMock } = vi.hoisted(() => ({
homedirMock: vi.fn<() => string>(),
grantMock: vi.fn(),
findCurrentMock: vi.fn()
}))
vi.mock('node:os', async () => {
const actual = await vi.importActual<typeof NodeOs>('node:os')
return { ...actual, homedir: homedirMock }
})
vi.mock('./codex-hook-trust-grant', () => ({
CODEX_TRUST_GRANT_TRANSIENT_RETRY_INTERVAL_MS: 300_000,
findCurrentManagedCodexHookTrust: findCurrentMock,
grantManagedCodexHookTrust: grantMock
}))
import {
ensureRealHomeCodexHookState,
removeRealHomeCodexHookForOptOut,
_internals
} from './codex-real-home-hook-install'
import { getRealHomeHookKeySourcePaths } from './codex-real-home-hooks-json'
import { cleanupLegacyManagedHookRepresentations } from './codex-hook-legacy-cleanup'
import { getCodexHookTrustSignature } from './codex-hook-identity'
import { getCodexManagedHookInstallMaterial } from './hook-service'
import { writeCodexTrustGrantLedgerHome } from './codex-trust-grant-ledger'
// Why this file: Codex keys ~/.codex/hooks.json as spelled on its default home
// and resolved when CODEX_HOME names it, so a symlinked home has two keys.
let root: string
let home: string
let userDataDir: string
let previousUserDataPath: string | undefined
const codexHome = (): string => join(home, '.codex')
const hooksPath = (): string => join(codexHome(), 'hooks.json')
const tomlPath = (): string => join(codexHome(), 'config.toml')
const codexHash = (entry: CodexTrustEntry): string => `sha256:codex-${entry.eventLabel}`
function stopEntry(sourcePath: string, groupIndex = 0): CodexTrustEntry {
return {
sourcePath,
eventLabel: 'stop',
groupIndex,
handlerIndex: 0,
command: getCodexManagedHookInstallMaterial().command,
timeoutSec: 10
}
}
/** Stands in for Codex: approves the spelled keys it was asked about, and records the ledger. */
function grantLikeCodex(): void {
grantMock.mockImplementation((plan: CodexManagedTrustGrantPlan) => {
const entries = plan.managedEntries.map((entry) => ({
...entry,
trustedHash: codexHash(entry)
}))
upsertHookTrustEntries(plan.tomlPath, entries)
writeCodexTrustGrantLedgerHome(plan.runtimeHomePath, {
binary: null,
entries: Object.fromEntries(
entries.map((entry) => [
normalizeHookTrustKeyForLookup(computeTrustKey(entry)),
{ signature: getCodexHookTrustSignature(entry), trustedHash: entry.trustedHash }
])
)
})
return { lane: 'rpc', entries }
})
}
async function ensureSettled(): Promise<string> {
await ensureRealHomeCodexHookState({
hooksEnabled: true,
userDataPath: userDataDir,
writePolicy: 'add-missing-only'
})
return _internals.settledVerdictForTesting()
}
function linkCodexHomeToDotfiles(): string {
const target = join(home, 'dotfiles-codex')
mkdirSync(target)
symlinkSync(target, codexHome(), process.platform === 'win32' ? 'junction' : 'dir')
return join(realpathSync.native(target), 'hooks.json')
}
beforeEach(() => {
grantMock.mockReset()
findCurrentMock.mockReset()
findCurrentMock.mockResolvedValue(null)
// Why realpath: the temp dir itself may sit under a symlink (macOS /var), which
// would give every home in this file a second spelling.
root = realpathSync.native(mkdtempSync(join(tmpdir(), 'orca-real-home-spellings-')))
home = join(root, 'home')
mkdirSync(home)
userDataDir = join(root, 'user-data')
mkdirSync(userDataDir)
previousUserDataPath = process.env.ORCA_USER_DATA_PATH
process.env.ORCA_USER_DATA_PATH = userDataDir
homedirMock.mockReturnValue(home)
_internals.resetForTesting('pending')
})
afterEach(() => {
rmSync(root, { recursive: true, force: true })
if (previousUserDataPath === undefined) {
delete process.env.ORCA_USER_DATA_PATH
} else {
process.env.ORCA_USER_DATA_PATH = previousUserDataPath
}
vi.clearAllMocks()
})
describe('both spellings of a symlinked ~/.codex', () => {
it('has one key when nothing on the path is a symlink', () => {
mkdirSync(codexHome())
expect(getRealHomeHookKeySourcePaths()).toEqual([normalizeCodexHookSourcePath(hooksPath())])
})
it('resolves the key through a symlinked HOME before ~/.codex exists', () => {
const linkedHome = join(root, 'linked-home')
symlinkSync(home, linkedHome, process.platform === 'win32' ? 'junction' : 'dir')
homedirMock.mockReturnValue(linkedHome)
expect(getRealHomeHookKeySourcePaths()).toEqual([
normalizeCodexHookSourcePath(join(linkedHome, '.codex', 'hooks.json')),
normalizeCodexHookSourcePath(join(home, '.codex', 'hooks.json'))
])
expect(getCodexExplicitHomeHookSourcePath(join(linkedHome, '.codex', 'hooks.json'))).toBe(
normalizeCodexHookSourcePath(join(home, '.codex', 'hooks.json'))
)
})
it("copies Codex's approval to the resolved key, and the opt-out removes both", async () => {
const resolvedHooks = linkCodexHomeToDotfiles()
grantLikeCodex()
expect(await ensureSettled()).toBe('installed')
const spelled = stopEntry(hooksPath())
const resolved = stopEntry(resolvedHooks)
const trust = readHookTrustEntries(tomlPath())
expect(trust.get(computeTrustKey(spelled))?.trustedHash).toBe(codexHash(spelled))
expect(trust.get(computeTrustKey(resolved))?.trustedHash).toBe(codexHash(spelled))
expect(await removeRealHomeCodexHookForOptOut()).toBe('removed')
const after = readHookTrustEntries(tomlPath())
expect(after.get(computeTrustKey(spelled))).toBeUndefined()
expect(after.get(computeTrustKey(resolved))).toBeUndefined()
})
it('copies the approval when an earlier grant is still current, with no session', async () => {
const resolvedHooks = linkCodexHomeToDotfiles()
findCurrentMock.mockImplementation(async (plan: CodexManagedTrustGrantPlan) =>
plan.managedEntries.map((entry) => ({ ...entry, trustedHash: codexHash(entry) }))
)
expect(await ensureSettled()).toBe('installed')
expect(grantMock).not.toHaveBeenCalled()
expect(
readHookTrustEntries(tomlPath()).get(computeTrustKey(stopEntry(resolvedHooks)))?.trustedHash
).toBe(codexHash(stopEntry(resolvedHooks)))
})
it('moves a user approval under both keys when the opt-out shifts the hook', async () => {
const resolvedHooks = linkCodexHomeToDotfiles()
grantLikeCodex()
await ensureSettled()
const installed = JSON.parse(readFileSync(hooksPath(), 'utf-8'))
installed.hooks.Stop.push({ hooks: [{ type: 'command', command: 'after.sh' }] })
writeFileSync(hooksPath(), `${JSON.stringify(installed, null, 2)}\n`)
const afterAt = (sourcePath: string, groupIndex: number): CodexTrustEntry => ({
sourcePath,
eventLabel: 'stop',
groupIndex,
handlerIndex: 0,
command: 'after.sh'
})
upsertHookTrustEntries(tomlPath(), [
{ ...afterAt(hooksPath(), 1), trustedHash: 'sha256:user-spelled' },
{ ...afterAt(resolvedHooks, 1), trustedHash: 'sha256:user-resolved' }
])
expect(await removeRealHomeCodexHookForOptOut()).toBe('removed')
const trust = readHookTrustEntries(tomlPath())
expect(trust.get(computeTrustKey(afterAt(hooksPath(), 0)))?.trustedHash).toBe(
'sha256:user-spelled'
)
expect(trust.get(computeTrustKey(afterAt(resolvedHooks, 0)))?.trustedHash).toBe(
'sha256:user-resolved'
)
expect(trust.get(computeTrustKey(afterAt(resolvedHooks, 1)))).toBeUndefined()
})
it("sweeps a retired hook's approval under both keys", async () => {
const resolvedHooks = linkCodexHomeToDotfiles()
const retired = `/bin/sh "${join(home, 'old-user-data', 'agent-hooks', 'codex-hook.sh')}"`
writeFileSync(
hooksPath(),
`${JSON.stringify({ hooks: { Stop: [{ hooks: [{ type: 'command', command: retired }] }] } })}\n`
)
const retiredAt = (sourcePath: string): CodexTrustEntry => ({
sourcePath,
eventLabel: 'stop',
groupIndex: 0,
handlerIndex: 0,
command: retired
})
upsertHookTrustEntries(tomlPath(), [retiredAt(hooksPath()), retiredAt(resolvedHooks)])
await cleanupLegacyManagedHookRepresentations()
const trust = readHookTrustEntries(tomlPath())
expect(trust.get(computeTrustKey(retiredAt(hooksPath())))).toBeUndefined()
expect(trust.get(computeTrustKey(retiredAt(resolvedHooks)))).toBeUndefined()
})
it('keeps the lane and the file when the copy would break config.toml', async () => {
linkCodexHomeToDotfiles()
// Why no write: Codex keeps its own approval in this inline form, which an
// appended [hooks.state."k"] table would turn into a file Codex cannot load.
const original = 'model = "m"\nhooks = { state = {} }\n'
writeFileSync(tomlPath(), original)
grantMock.mockImplementation((plan: CodexManagedTrustGrantPlan) => ({
lane: 'rpc',
entries: plan.managedEntries.map((entry) => ({ ...entry, trustedHash: codexHash(entry) }))
}))
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
expect(await ensureSettled()).toBe('installed')
expect(readFileSync(tomlPath(), 'utf-8')).toBe(original)
expect(warn).toHaveBeenCalledWith(
'[codex-real-home-hooks] could not approve the resolved ~/.codex key:',
expect.objectContaining({ name: 'CodexConfigTomlRefusedError' })
)
})
})
+4 -2
View File
@@ -10,6 +10,7 @@ import { resolveHooksJsonWritePath } from '../agent-hooks/hook-config-write-path
import {
assertHooksJsonGeneration,
getRealHomeConfigTomlPath,
getRealHomeHookKeySourcePaths,
getRealHomeHooksJsonPath
} from './codex-real-home-hooks-json'
import { getCodexManagedScriptFileName } from './codex-hook-identity'
@@ -55,8 +56,9 @@ export async function sweepRealHomeCodexHook(): Promise<'removed' | 'unavailable
}
if (removedAny) {
const hooksWritePath = resolveHooksJsonWritePath(hooksJsonPath)
const sourcePaths = getRealHomeHookKeySourcePaths()
mutateRealHomeHooksPreservingUserTrust({
sourcePath: hooksJsonPath,
sourcePaths,
tomlPath: getRealHomeConfigTomlPath(),
beforeHooks: config.hooks,
afterHooks: nextHooks,
@@ -73,7 +75,7 @@ export async function sweepRealHomeCodexHook(): Promise<'removed' | 'unavailable
removeCodexManagedHookTrustEntries({
tomlPath: getRealHomeConfigTomlPath(),
runtimeHomePath: getSystemCodexHomePath(),
sourcePath: hooksJsonPath,
sourcePaths,
command: material.command,
managedEventLabels: new Set(Object.values(material.eventLabel)),
timeoutSec: MANAGED_HOOK_TIMEOUT_SECONDS
@@ -10,6 +10,7 @@ import type { RealHomeCodexHookSlotWrite } from './codex-real-home-hook-entry-pl
import {
assertHooksJsonGeneration,
getRealHomeConfigTomlPath,
getRealHomeHookKeySourcePaths,
getRealHomeHooksJsonPath
} from './codex-real-home-hooks-json'
import { readHookTrustEntries } from './config-toml-trust'
@@ -91,7 +92,7 @@ export function withdrawUntrustedRealHomeWrites(
if (withdrew > 0) {
// Why: a hook appended after this entry meanwhile moves up a slot; its trust moves with it.
mutateRealHomeHooksPreservingUserTrust({
sourcePath: hooksJsonPath,
sourcePaths: getRealHomeHookKeySourcePaths(),
tomlPath: getRealHomeConfigTomlPath(),
beforeHooks: config.hooks,
afterHooks: nextHooks,
@@ -3,6 +3,10 @@ import { join } from 'node:path'
import { writeFileAtomically } from '../codex-accounts/fs-utils'
import { resolveHooksJsonWritePath } from '../agent-hooks/hook-config-write-path'
import { getSystemCodexHomePath } from './codex-home-paths'
import {
getCodexExplicitHomeHookSourcePath,
normalizeCodexHookSourcePath
} from './config-toml-trust'
/** The user's real `~/.codex` hook files, plus the guard and pristine backup
* the real-home lane needs before it is allowed to mutate them. */
@@ -10,6 +14,18 @@ export function getRealHomeHooksJsonPath(): string {
return join(getSystemCodexHomePath(), 'hooks.json')
}
/**
* Every key Codex may give an entry in ~/.codex/hooks.json: as spelled when it
* runs on its default home, resolved when a pane's CODEX_HOME names it. They
* differ when ~/.codex or HOME is a symlink, and Orca approves under both.
*/
export function getRealHomeHookKeySourcePaths(): [string, ...string[]] {
const hooksJsonPath = getRealHomeHooksJsonPath()
const spelled = normalizeCodexHookSourcePath(hooksJsonPath)
const resolved = getCodexExplicitHomeHookSourcePath(hooksJsonPath)
return resolved === spelled ? [spelled] : [spelled, resolved]
}
export function getRealHomeConfigTomlPath(): string {
return join(getSystemCodexHomePath(), 'config.toml')
}
+21 -7
View File
@@ -80,13 +80,27 @@ export function getExplicitHomeCodexHookSourcePath(sourcePath: string): string {
if (process.platform !== 'win32' && isUnambiguousWindowsPath(sourcePath)) {
return normalizeCodexTrustSourcePath(sourcePath)
}
try {
// Why: hook discovery resolves the explicit home but keeps the hooks.json leaf logical.
return normalizeCodexTrustSourcePath(
join(realpathSync.native(dirname(sourcePath)), basename(sourcePath))
)
} catch {
return normalizeCodexTrustSourcePath(sourcePath)
// Why: hook discovery resolves the explicit home but keeps the hooks.json leaf logical.
return normalizeCodexTrustSourcePath(
join(resolveThroughExistingAncestor(dirname(sourcePath)), basename(sourcePath))
)
}
// Why the nearest existing ancestor: a home not created yet still sits under a resolved HOME.
function resolveThroughExistingAncestor(path: string): string {
const missing: string[] = []
let current = path
for (;;) {
try {
return join(realpathSync.native(current), ...missing.toReversed())
} catch {
const parent = dirname(current)
if (parent === current) {
return path
}
missing.push(basename(current))
current = parent
}
}
}
@@ -48,7 +48,7 @@ function mutate(
after: Record<string, HookDefinition[]>
): void {
mutateRealHomeHooksPreservingUserTrust({
sourcePath: hooksPath,
sourcePaths: [hooksPath],
tomlPath: configPath,
beforeHooks: before,
afterHooks: after,
@@ -68,13 +68,16 @@ export function getMovedCodexUserHookTrust(
* its new key, verbatim. Needs no Codex session, so no removal waits on one.
*/
export function mutateRealHomeHooksPreservingUserTrust(args: {
sourcePath: string
/** Every spelling Codex may key this file by (as spelled, and resolved). */
sourcePaths: readonly string[]
tomlPath: string
beforeHooks: HooksByEvent
afterHooks: HooksByEvent
writeHooks: () => void
}): void {
const moves = getMovedCodexUserHookTrust(args.sourcePath, args.beforeHooks, args.afterHooks)
const moves = args.sourcePaths.flatMap((sourcePath) =>
getMovedCodexUserHookTrust(sourcePath, args.beforeHooks, args.afterHooks)
)
args.writeHooks()
try {
moveHookTrustEntries(args.tomlPath, moves)
+48 -4
View File
@@ -1,3 +1,5 @@
import { parse as parseToml } from 'smol-toml'
import { isPlainObject } from '../agent-hooks/hooks-json-read'
import type { CodexHookTrustState } from './config-toml-trust'
import { normalizeCodexHookTrustLookupKey } from './codex-trust-identity'
import { findAllHookTrustBlocks } from './config-toml-hook-trust-blocks'
@@ -27,6 +29,44 @@ export class CodexHookTrustEntryMap extends Map<string, CodexHookTrustState> {
}
export function readHookTrustContent(content: string): Map<string, CodexHookTrustState> {
const result = readHookTrustTables(content)
// Why: approvals written as dotted keys or inline tables have no [hooks.state."k"] header.
for (const [key, state] of readParsedHookTrust(content)) {
if (!result.has(key)) {
result.set(key, state)
}
}
return result
}
function readParsedHookTrust(content: string): [string, CodexHookTrustState][] {
let parsed: unknown
try {
parsed = parseToml(content)
} catch {
return []
}
const hooks = isPlainObject(parsed) ? parsed.hooks : undefined
const state = isPlainObject(hooks) ? hooks.state : undefined
if (!isPlainObject(state)) {
return []
}
return Object.entries(state).flatMap(([key, value]): [string, CodexHookTrustState][] =>
isPlainObject(value)
? [
[
key,
{
trustedHash: typeof value.trusted_hash === 'string' ? value.trusted_hash : undefined,
enabled: typeof value.enabled === 'boolean' ? value.enabled : undefined
}
]
]
: []
)
}
function readHookTrustTables(content: string): CodexHookTrustEntryMap {
const result = new CodexHookTrustEntryMap()
const conflictingTrustedHashKeys = new Set<string>()
for (const block of findAllHookTrustBlocks(content)) {
@@ -69,11 +109,15 @@ function readHookTrustBlockState(block: string): {
const lineEnd = newlineIndex === -1 ? block.length : newlineIndex
const line = block.slice(cursor, lineEnd).replace(/\r$/, '')
if (isTomlStructuralLine(scanState)) {
const hashMatch = /^[ \t]*trusted_hash[ \t]*=[ \t]*"((?:[^"\\]|\\.)*)"[ \t]*(?:#.*)?$/.exec(
line
)
// Why both forms: a literal-string hash must not read as "no approval".
const hashMatch =
/^[ \t]*trusted_hash[ \t]*=[ \t]*(?:"((?:[^"\\]|\\.)*)"|'([^'\r\n]*)')[ \t]*(?:#.*)?$/.exec(
line
)
if (hashMatch) {
trustedHashes.add(unescapeTomlBasicString(hashMatch[1]!))
trustedHashes.add(
hashMatch[1] !== undefined ? unescapeTomlBasicString(hashMatch[1]) : hashMatch[2]!
)
}
const enabledMatch = /^[ \t]*enabled[ \t]*=[ \t]*(true|false)[ \t]*(?:#.*)?$/.exec(line)
if (enabledMatch) {
@@ -70,6 +70,31 @@ describe('readHookTrustEntries', () => {
expect(readHookTrustEntries(configPath).get(key)?.trustedHash).toBeUndefined()
})
it('reads a literal-string trusted_hash', () => {
const key = '/x/hooks.json:stop:0:0'
writeFileSync(configPath, `[hooks.state."${key}"]\ntrusted_hash = 'sha256:LITERAL'\n`, 'utf-8')
expect(readHookTrustEntries(configPath).get(key)?.trustedHash).toBe('sha256:LITERAL')
})
it('fails closed when a literal and a basic hash conflict', () => {
const key = '/x/hooks.json:stop:0:0'
writeFileSync(
configPath,
[
`[hooks.state."${key}"]`,
"trusted_hash = 'sha256:USER'",
'',
`[hooks.state.'${key}']`,
'trusted_hash = "sha256:ORCA"',
''
].join('\n'),
'utf-8'
)
expect(readHookTrustEntries(configPath).get(key)?.trustedHash).toBeUndefined()
})
it('ignores trust-looking fields inside multiline strings', () => {
const key = '/x/hooks.json:stop:0:0'
writeFileSync(
@@ -0,0 +1,203 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import type { SFTPWrapper } from 'ssh2'
import type * as SmolToml from 'smol-toml'
import type * as InstallerUtilsRemote from '../agent-hooks/installer-utils-remote'
const mocks = vi.hoisted(() => {
const state: {
// Why: removal and enabled-flag edits cannot break real TOML, so these cases
// make the parser reject the edited bytes to prove the write is still checked.
rejectParse: ((content: string) => boolean) | null
remoteFiles: Map<string, string>
} = { rejectParse: null, remoteFiles: new Map() }
return state
})
vi.mock('smol-toml', async (importOriginal) => {
const actual = await importOriginal<typeof SmolToml>()
return {
...actual,
parse: (content: string) => {
if (mocks.rejectParse?.(content)) {
throw new Error('simulated unloadable TOML')
}
return actual.parse(content)
}
}
})
vi.mock('../agent-hooks/installer-utils-remote', async (importOriginal) => {
const actual = await importOriginal<typeof InstallerUtilsRemote>()
return {
...actual,
readHooksJsonRemote: async (_sftp: SFTPWrapper, path: string) =>
JSON.parse(mocks.remoteFiles.get(path) ?? '{}'),
readTextFileRemote: async (_sftp: SFTPWrapper, path: string) =>
mocks.remoteFiles.get(path) ?? null,
writeHooksJsonRemote: async (_sftp: SFTPWrapper, path: string, config: unknown) => {
mocks.remoteFiles.set(path, JSON.stringify(config))
},
writeManagedScriptRemote: async () => {},
writeTextFileRemoteAtomic: async (_sftp: SFTPWrapper, path: string, content: string) => {
mocks.remoteFiles.set(path, content)
}
}
})
import {
computeTrustKey,
escapeTomlString,
isCodexConfigTomlRefusedError,
moveHookTrustEntries,
readHookTrustEntries,
removeHookTrustEntries,
upsertHookTrustEntries,
type CodexTrustEntry
} from './config-toml-trust'
import { applyMirroredRuntimeUserHookTrustStates } from './codex-hook-user-mirroring'
import { installCodexHooksRemote } from './codex-hook-remote-install'
let dir: string
let tomlPath: string
let hooksPath: string
function stopEntry(groupIndex = 0): CodexTrustEntry {
return {
sourcePath: hooksPath,
eventLabel: 'stop',
groupIndex,
handlerIndex: 0,
command: 'orca-hook.sh'
}
}
function tomlKey(entry: CodexTrustEntry): string {
return escapeTomlString(computeTrustKey(entry))
}
function expectRefusal(write: () => void): void {
let thrown: unknown
try {
write()
} catch (error) {
thrown = error
}
expect(isCodexConfigTomlRefusedError(thrown)).toBe(true)
}
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), 'orca-codex-toml-loadability-'))
tomlPath = join(dir, 'config.toml')
hooksPath = join(dir, 'hooks.json')
mocks.rejectParse = null
mocks.remoteFiles.clear()
})
afterEach(() => {
rmSync(dir, { recursive: true, force: true })
})
describe('hook approval writes never break a config.toml Codex can load', () => {
it.each([
['an inline hooks.state table', '[hooks]\nstate = { "x:stop:0:0" = { trusted_hash = "u" } }\n'],
['an inline hooks table', 'hooks = { state = {} }\n'],
[
"a dotted approval for Orca's own key",
(): string => `hooks.state."${tomlKey(stopEntry())}".trusted_hash = "sha256:user"\n`
]
])('upsert refuses and leaves the file byte-identical: %s', (_case, content) => {
const original = `model = "m"\n${typeof content === 'string' ? content : content()}`
writeFileSync(tomlPath, original)
expectRefusal(() => upsertHookTrustEntries(tomlPath, [stopEntry()]))
expect(readFileSync(tomlPath, 'utf-8')).toBe(original)
})
it('upsert may still repair a file Codex already cannot load', () => {
writeFileSync(tomlPath, 'model = \n')
upsertHookTrustEntries(tomlPath, [stopEntry()])
expect(readFileSync(tomlPath, 'utf-8')).toContain('trusted_hash')
})
it('a move refuses to land on a key the user approved as dotted keys', () => {
const original =
`hooks.state."${tomlKey(stopEntry(0))}".trusted_hash = "sha256:user"\n\n` +
`[hooks.state."${tomlKey(stopEntry(1))}"]\ntrusted_hash = "sha256:moved"\n`
writeFileSync(tomlPath, original)
expectRefusal(() =>
moveHookTrustEntries(tomlPath, [
{ oldKey: computeTrustKey(stopEntry(1)), newKey: computeTrustKey(stopEntry(0)) }
])
)
expect(readFileSync(tomlPath, 'utf-8')).toBe(original)
})
it('a removal is checked before it is written', () => {
upsertHookTrustEntries(tomlPath, [stopEntry(0), { ...stopEntry(1), command: 'user.sh' }])
const original = readFileSync(tomlPath, 'utf-8')
mocks.rejectParse = (content) => !content.includes(computeTrustKey(stopEntry(0)))
expectRefusal(() => removeHookTrustEntries(tomlPath, [computeTrustKey(stopEntry(0))]))
expect(readFileSync(tomlPath, 'utf-8')).toBe(original)
})
it("mirroring a user hook's enabled state is checked before it is written", () => {
upsertHookTrustEntries(tomlPath, [{ ...stopEntry(), enabled: true }])
const original = readFileSync(tomlPath, 'utf-8')
mocks.rejectParse = (content) => content.includes('enabled = false')
expectRefusal(() =>
applyMirroredRuntimeUserHookTrustStates(tomlPath, [{ entry: stopEntry(), enabled: false }])
)
expect(readFileSync(tomlPath, 'utf-8')).toBe(original)
})
it('the SSH installer reports the refusal and leaves the remote config.toml untouched', async () => {
const remoteToml = '/home/u/.codex/config.toml'
const original = 'model = "m"\nhooks = { state = {} }\n'
mocks.remoteFiles.set(remoteToml, original)
mocks.remoteFiles.set('/home/u/.codex/hooks.json', '{"hooks":{}}')
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the mocked remote helpers never touch the SFTP handle.
const status = await installCodexHooksRemote({} as SFTPWrapper, '/home/u')
expect(status).toMatchObject({
state: 'error',
detail: expect.stringContaining('defines hook approvals in a form Orca cannot add to')
})
expect(mocks.remoteFiles.get(remoteToml)).toBe(original)
})
})
describe('reading approvals Codex or the user wrote without a table header', () => {
it('reads an approval written as dotted keys', () => {
writeFileSync(tomlPath, `hooks.state."${tomlKey(stopEntry())}".trusted_hash = "sha256:user"\n`)
expect(readHookTrustEntries(tomlPath).get(computeTrustKey(stopEntry()))).toEqual({
trustedHash: 'sha256:user',
enabled: undefined
})
})
it('reads an approval written as an inline table', () => {
writeFileSync(
tomlPath,
`[hooks]\nstate = { "${tomlKey(stopEntry())}" = { trusted_hash = "sha256:user", enabled = false } }\n`
)
expect(readHookTrustEntries(tomlPath).get(computeTrustKey(stopEntry()))).toEqual({
trustedHash: 'sha256:user',
enabled: false
})
})
})
+66 -6
View File
@@ -1,4 +1,5 @@
import { existsSync, readFileSync } from 'node:fs'
import { parse as parseToml } from 'smol-toml'
import {
codexTrustSourcePathsEqual,
computeCodexTrustedHash,
@@ -103,7 +104,11 @@ export function parseTrustKey(key: string): {
return parseCodexTrustKey(key)
}
// Why: trust edits preserve unrelated bytes instead of reserializing the user's config.
/**
* Upserts hook approvals, preserving unrelated bytes instead of reserializing
* the user's config. Throws CodexConfigTomlRefusedError, writing nothing, when
* Codex could not load the result; see writeLoadableHookTrustConfig.
*/
export function upsertHookTrustEntries(
configPath: string,
entries: readonly CodexTrustEntry[]
@@ -111,7 +116,59 @@ export function upsertHookTrustEntries(
const existing = readTomlForMutation(configPath)
const updated = upsertHookTrustEntriesInContent(existing, entries)
if (updated !== existing) {
writeConfigAtomically(configPath, updated)
writeLoadableHookTrustConfig(configPath, existing, updated)
}
}
/** Thrown instead of writing a config.toml that Codex could no longer load. */
export class CodexConfigTomlRefusedError extends Error {
constructor(message: string) {
super(message)
this.name = 'CodexConfigTomlRefusedError'
}
}
export function isCodexConfigTomlRefusedError(
error: unknown
): error is CodexConfigTomlRefusedError {
return error instanceof Error && error.name === 'CodexConfigTomlRefusedError'
}
/**
* Every hooks.state write goes through here. A user's inline
* `hooks.state = {...}` or dotted `hooks.state."k".trusted_hash` key cannot take
* an appended `[hooks.state."k"]` table, and Codex refuses to start with a
* config.toml it cannot load, so a write that would break a loadable file is
* refused instead. One that is already broken may still be repaired.
*/
export function writeLoadableHookTrustConfig(
configPath: string,
previous: string,
contents: string
): void {
assertLoadableHookTrustConfig(configPath, previous, contents)
writeConfigAtomically(configPath, contents)
}
/** Throws CodexConfigTomlRefusedError when `contents` would break a `previous` Codex could load. */
export function assertLoadableHookTrustConfig(
configPath: string,
previous: string,
contents: string
): void {
if (isLoadableToml(previous) && !isLoadableToml(contents)) {
throw new CodexConfigTomlRefusedError(
`${configPath} defines hook approvals in a form Orca cannot add to without breaking it`
)
}
}
function isLoadableToml(content: string): boolean {
try {
parseToml(stripLeadingBom(content))
return true
} catch {
return false
}
}
@@ -126,7 +183,7 @@ export function moveHookTrustEntries(
const existing = readTomlForMutation(configPath)
const updated = moveHookTrustContent(existing, moves)
if (updated !== existing) {
writeConfigAtomically(configPath, updated)
writeLoadableHookTrustConfig(configPath, existing, updated)
}
}
@@ -181,7 +238,7 @@ export function removeHookTrustEntries(configPath: string, keys: readonly string
const existing = readTomlFile(configPath)
const updated = removeHookTrustEntriesFromContent(existing, keys)
if (updated !== existing) {
writeConfigAtomically(configPath, updated)
writeLoadableHookTrustConfig(configPath, existing, updated)
}
}
@@ -212,6 +269,9 @@ function readTomlForMutation(configPath: string): string {
}
function readTomlFile(configPath: string): string {
const raw = readFileSync(configPath, 'utf-8')
return raw.charCodeAt(0) === 0xfeff ? raw.slice(1) : raw
return stripLeadingBom(readFileSync(configPath, 'utf-8'))
}
function stripLeadingBom(content: string): string {
return content.charCodeAt(0) === 0xfeff ? content.slice(1) : content
}
@@ -129,6 +129,21 @@ describe('CodexHookService', () => {
expect(trustConfig).toContain(':permission_request:0:0')
})
it('reports, instead of writing, approvals a mirrored inline hooks.state cannot take', async () => {
const systemCodexHome = join(homes.tmpHome, '.codex')
mkdirSync(systemCodexHome, { recursive: true })
writeFileSync(join(systemCodexHome, 'config.toml'), 'hooks = { state = {} }\n', 'utf-8')
const status = await new CodexHookService().install()
expect(status).toMatchObject({
state: 'error',
detail: expect.stringContaining('defines hook approvals in a form Orca cannot add to')
})
const managedToml = join(homes.userDataDir, 'codex-runtime-home', 'home', 'config.toml')
expect(readFileSync(managedToml, 'utf-8')).not.toContain('[hooks.state.')
})
it('installs managed hooks + trust into a per-account self-contained home, not the shared mirror', async () => {
const systemCodexHome = join(homes.tmpHome, '.codex')
mkdirSync(systemCodexHome, { recursive: true })