fix(codex): harden managed trust sanitization

This commit is contained in:
Brennan Benson
2026-07-17 02:39:16 -07:00
parent bfea35fbb5
commit efaa61d596
4 changed files with 193 additions and 26 deletions
+37
View File
@@ -394,6 +394,43 @@ describe('CodexAccountService config sync', () => {
expectSanitizedManagedConfig()
})
it('keeps flag-off config mirroring byte-identical', 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({
codexSystemDefaultRealHomeEnabled: false,
codexManagedAccounts: [
{
id: 'account-1',
email: 'user@example.com',
managedHomePath,
providerAccountId: null,
workspaceLabel: null,
workspaceAccountId: null,
createdAt: 1,
updatedAt: 1,
lastAuthenticatedAt: 1
}
]
})
const { CodexAccountService } = await import('./service')
new CodexAccountService(
createStore(settings) as never,
createRateLimits() as never,
createRuntimeHome() as never
)
expect(readFileSync(join(managedHomePath, 'config.toml'), 'utf-8')).toBe(fixture.config)
expect(readFileSync(canonicalConfigPath, 'utf-8')).toBe(fixture.config)
})
it('rewrites relative path config values when syncing into managed homes', async () => {
const canonicalConfigPath = join(testState.fakeHomeDir, '.codex', 'config.toml')
writeFileSync(
+13 -9
View File
@@ -17,6 +17,7 @@ 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 { isCodexSystemDefaultRealHomeEnabled } from '../codex/codex-real-home-flag'
import { getCodexManagedHookInstallMaterial } from '../codex/hook-service'
import { MANAGED_HOOK_TIMEOUT_SECONDS } from '../agent-hooks/installer-utils'
import { resolveCodexCommand } from '../codex-cli/command'
@@ -472,15 +473,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
})
let sanitizedConfig = canonicalConfig.contents
if (isCodexSystemDefaultRealHomeEnabled(this.store.getSettings())) {
const material = getCodexManagedHookInstallMaterial()
// Why: source-home Orca trust is foreign to each managed home's hooks.json.
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(sanitizedConfig, canonicalConfig.sourceHomePath)
+66
View File
@@ -26,7 +26,9 @@ import {
normalizeCodexProjectPathForRevocationLookup,
parseTrustKey,
readHookTrustEntries,
readHookTrustEntriesFromContent,
removeHookTrustEntries,
removeHookTrustEntriesFromContent,
upsertHookTrustEntries,
upsertProjectTrustLevel,
upsertProjectTrustLevelInContent,
@@ -1664,6 +1666,70 @@ describe('readHookTrustEntries', () => {
expect(result.get(keyB)?.enabled).toBe(true)
})
it('fails closed when normalized duplicate blocks have conflicting hashes', () => {
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(
configPath,
[
`[hooks.state."${key}"]`,
'note = """',
'trusted_hash = "sha256:NOT-A-FIELD"',
'enabled = false',
'"""',
''
].join('\n'),
'utf-8'
)
expect(readHookTrustEntries(configPath).get(key)).toEqual({
trustedHash: undefined,
enabled: undefined
})
})
it('does not accept an unterminated trusted_hash string', () => {
const key = '/x/hooks.json:stop:0:0'
writeFileSync(
configPath,
`[hooks.state."${key}"]\ntrusted_hash = "sha256:UNTERMINATED\n`,
'utf-8'
)
expect(readHookTrustEntries(configPath).get(key)?.trustedHash).toBeUndefined()
})
it('recognizes and removes a first trust block after a leading BOM', () => {
const key = '/x/hooks.json:stop:0:0'
const content = [
`\uFEFF[hooks.state."${key}"]`,
'trusted_hash = "sha256:ORCA"',
'[other]',
'value = true',
''
].join('\n')
expect(readHookTrustEntriesFromContent(content).get(key)?.trustedHash).toBe('sha256:ORCA')
expect(removeHookTrustEntriesFromContent(content, [key])).toBe('[other]\nvalue = true\n')
})
it('does not let triple quotes in comments hide later trust entries', () => {
const key = '/x/hooks.json:pre_tool_use:0:0'
const original = [
+77 -17
View File
@@ -599,18 +599,34 @@ export function normalizeHookTrustKeyForLookup(key: string): string {
}
function findTrustBlockRanges(content: string, key: string): TrustBlockRange[] {
return findTrustBlockRangesForNormalizedKeys(
content,
new Set([normalizeHookTrustKeyForLookup(key)])
)
}
function findTrustBlockRangesForNormalizedKeys(
content: string,
normalizedKeys: ReadonlySet<string>
): TrustBlockRange[] {
const ranges: TrustBlockRange[] = []
const normalizedKey = normalizeHookTrustKeyForLookup(key)
if (normalizedKeys.size === 0) {
return ranges
}
let cursor = 0
let scanState = createTomlLineScanState()
while (cursor < content.length) {
const newlineIdx = content.indexOf('\n', cursor)
const lineEnd = newlineIdx === -1 ? content.length : newlineIdx
const rawLine = content.slice(cursor, lineEnd)
const line = rawLine.replace(/\r$/, '')
const lineWithoutCr = rawLine.replace(/\r$/, '')
const line =
cursor === 0 && lineWithoutCr.charCodeAt(0) === 0xfeff
? lineWithoutCr.slice(1)
: lineWithoutCr
const nextCursor = newlineIdx === -1 ? content.length : newlineIdx + 1
const headerKey = isTomlStructuralLine(scanState) ? parseHookStateHeaderKey(line) : null
if (headerKey !== null && normalizeHookTrustKeyForLookup(headerKey) === normalizedKey) {
if (headerKey !== null && normalizedKeys.has(normalizeHookTrustKeyForLookup(headerKey))) {
const headerLineEnd = rawLine.endsWith('\r') ? lineEnd - 1 : lineEnd
const after = content.slice(headerLineEnd)
const nextHeaderRel = findNextTableHeader(after)
@@ -887,11 +903,8 @@ 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)
const normalizedKeys = new Set(keys.map(normalizeHookTrustKeyForLookup))
const ranges = findTrustBlockRangesForNormalizedKeys(content, normalizedKeys)
if (ranges.length === 0) {
return content
}
@@ -912,8 +925,39 @@ export function readHookTrustEntries(configPath: string): Map<string, CodexHookT
return readHookTrustEntriesFromContent(readTomlFile(configPath))
}
function readHookTrustBlockState(block: string): {
trustedHashes: Set<string>
enabled?: boolean
} {
const trustedHashes = new Set<string>()
let enabled: boolean | undefined
let cursor = 0
let scanState = createTomlLineScanState()
while (cursor < block.length) {
const newlineIdx = block.indexOf('\n', cursor)
const lineEnd = newlineIdx === -1 ? block.length : newlineIdx
const line = block.slice(cursor, lineEnd).replace(/\r$/, '')
if (isTomlStructuralLine(scanState)) {
const hashMatch = /^[ \t]*trusted_hash[ \t]*=[ \t]*"((?:[^"\\]|\\.)*)"[ \t]*(?:#.*)?$/.exec(
line
)
if (hashMatch) {
trustedHashes.add(unescapeTomlString(hashMatch[1]))
}
const enabledMatch = /^[ \t]*enabled[ \t]*=[ \t]*(true|false)[ \t]*(?:#.*)?$/.exec(line)
if (enabledMatch) {
enabled = enabled !== false && enabledMatch[1] === 'true'
}
}
scanState = updateTomlLineScanState(scanState, line)
cursor = newlineIdx === -1 ? block.length : newlineIdx + 1
}
return { trustedHashes, enabled }
}
export function readHookTrustEntriesFromContent(content: string): Map<string, CodexHookTrustState> {
const result = new HookTrustEntryMap()
const conflictingTrustedHashKeys = new Set<string>()
// Why: walk line-by-line so `[hooks.state."..."]` inside a `"""..."""` or
// `'''...'''` multi-line string isn't mistaken for a real header.
let cursor = 0
@@ -922,7 +966,11 @@ export function readHookTrustEntriesFromContent(content: string): Map<string, Co
const newlineIdx = content.indexOf('\n', cursor)
const lineEnd = newlineIdx === -1 ? content.length : newlineIdx
const rawLine = content.slice(cursor, lineEnd)
const line = rawLine.replace(/\r$/, '')
const lineWithoutCr = rawLine.replace(/\r$/, '')
const line =
cursor === 0 && lineWithoutCr.charCodeAt(0) === 0xfeff
? lineWithoutCr.slice(1)
: lineWithoutCr
const nextCursor = newlineIdx === -1 ? content.length : newlineIdx + 1
const key = isTomlStructuralLine(scanState) ? parseHookStateHeaderKey(line) : null
if (key !== null) {
@@ -931,21 +979,33 @@ export function readHookTrustEntriesFromContent(content: string): Map<string, Co
const nextHeaderRel = findNextTableHeader(after)
const blockEnd = nextHeaderRel === -1 ? content.length : nextCursor + nextHeaderRel
const block = content.slice(nextCursor, blockEnd)
// Why: we own this block's shape (only `enabled` + `trusted_hash`), so
// a line scan beats pulling in a full TOML value parser.
const hashMatch = /^[ \t]*trusted_hash[ \t]*=[ \t]*"((?:[^"\\]|\\.)*)"/m.exec(block)
const enabledMatch = /^[ \t]*enabled[ \t]*=[ \t]*(true|false)[ \t\r]*(?:#.*)?$/m.exec(block)
const blockState = readHookTrustBlockState(block)
const normalizedKey = normalizeHookTrustKeyForLookup(key)
const existingState = result.get(normalizedKey)
const enabled = enabledMatch ? enabledMatch[1] === 'true' : undefined
const trustedHash =
blockState.trustedHashes.size === 1
? blockState.trustedHashes.values().next().value
: undefined
if (
blockState.trustedHashes.size > 1 ||
(trustedHash !== undefined &&
existingState?.trustedHash !== undefined &&
existingState.trustedHash !== trustedHash)
) {
// Why: conflicting normalized duplicates are malformed and cannot prove
// which block is owned, so trust cleanup must preserve them all.
conflictingTrustedHashKeys.add(normalizedKey)
}
result.set(normalizedKey, {
trustedHash: hashMatch ? unescapeTomlString(hashMatch[1]) : existingState?.trustedHash,
trustedHash: conflictingTrustedHashKeys.has(normalizedKey)
? undefined
: (trustedHash ?? existingState?.trustedHash),
// Why: Windows writes both slash variants for one hook; a disabled copy
// must remain authoritative regardless of which variant appears last.
enabled:
existingState?.enabled === false || enabled === false
existingState?.enabled === false || blockState.enabled === false
? false
: (enabled ?? existingState?.enabled)
: (blockState.enabled ?? existingState?.enabled)
})
cursor = nextCursor
continue