mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 00:03:20 +00:00
fix(codex): preserve verified trust grants across launch prep
This commit is contained in:
@@ -21,6 +21,7 @@
|
||||
"../src/main/codex/codex-hook-identity.ts",
|
||||
"../src/main/codex/codex-hook-trust-grant.ts",
|
||||
"../src/main/codex/codex-managed-trust-reconciliation.ts",
|
||||
"../src/main/codex/codex-process-exit-deadline.ts",
|
||||
"../src/main/codex/codex-trust-config-rollback.ts",
|
||||
"../src/main/codex/codex-trust-grant-host.ts",
|
||||
"../src/main/codex/codex-trust-grant-ledger.ts",
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync, existsSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
@@ -90,6 +90,7 @@ function writeFileSyncSafe(file, contents) { require('node:fs').writeFileSync(fi
|
||||
let tempRoots: string[] = []
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks()
|
||||
for (const root of tempRoots) {
|
||||
rmSync(root, { recursive: true, force: true })
|
||||
}
|
||||
@@ -149,6 +150,8 @@ function managedHook(key: string, trustStatus = 'untrusted'): StubHook {
|
||||
|
||||
describe('runCodexHookTrustGrantSession', () => {
|
||||
it('grants and verifies exactly the expected managed entries', async () => {
|
||||
const setTimeoutSpy = vi.spyOn(globalThis, 'setTimeout')
|
||||
const clearTimeoutSpy = vi.spyOn(globalThis, 'clearTimeout')
|
||||
const keys = [
|
||||
'/home/a/.codex/hooks.json:session_start:0:0',
|
||||
'/home/a/.codex/hooks.json:stop:0:0'
|
||||
@@ -191,6 +194,13 @@ describe('runCodexHookTrustGrantSession', () => {
|
||||
expect(written.edits[0].mergeStrategy).toBe('upsert')
|
||||
expect(Object.keys(written.edits[0].value).sort()).toEqual([...keys].sort())
|
||||
expect(written.reloadUserConfig).toBe(true)
|
||||
// Why: the entry process waits on these handles after setting exitCode, so
|
||||
// an uncleared grace timer adds its full delay to synchronous launch prep.
|
||||
const timerHandles = setTimeoutSpy.mock.results.map(({ value }) => value)
|
||||
expect(timerHandles).toHaveLength(2)
|
||||
expect(clearTimeoutSpy.mock.calls.map(([handle]) => handle)).toEqual(
|
||||
expect.arrayContaining(timerHandles)
|
||||
)
|
||||
})
|
||||
|
||||
it('skips config/batchWrite when every expected entry is already trusted', async () => {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'
|
||||
import { normalizeHookTrustKeyForLookup } from './config-toml-trust'
|
||||
import { waitForProcessExitUntil } from './codex-process-exit-deadline'
|
||||
|
||||
// Why: Codex gates hooks on a `trusted_hash` it computes from a private
|
||||
// canonical-JSON identity. Orca used to replicate that algorithm
|
||||
@@ -363,11 +364,10 @@ export async function runCodexHookTrustGrantSession(
|
||||
if (!exited) {
|
||||
// Why: the server exits promptly on stdin EOF; the grace period only
|
||||
// bounds a wedged child before the guaranteed SIGKILL reap.
|
||||
const grace = new Promise<void>((resolve) => setTimeout(resolve, 1500))
|
||||
await Promise.race([exitPromise, grace])
|
||||
await waitForProcessExitUntil(exitPromise, 1500)
|
||||
if (!exited) {
|
||||
child.kill('SIGKILL')
|
||||
await Promise.race([exitPromise, new Promise<void>((resolve) => setTimeout(resolve, 1000))])
|
||||
await waitForProcessExitUntil(exitPromise, 1000)
|
||||
}
|
||||
}
|
||||
clearTimeout(deadline)
|
||||
|
||||
@@ -268,6 +268,9 @@ describe('syncSystemConfigIntoManagedCodexHome', () => {
|
||||
[
|
||||
'model = "runtime-model"',
|
||||
'',
|
||||
'[hooks.state]',
|
||||
'# runtime-owned parent',
|
||||
'',
|
||||
'[hooks.state."runtime-hooks:stop:0:0"]',
|
||||
'enabled = false',
|
||||
'trusted_hash = "sha256:runtime"',
|
||||
@@ -286,6 +289,9 @@ describe('syncSystemConfigIntoManagedCodexHome', () => {
|
||||
[
|
||||
'model = "system-model"',
|
||||
'',
|
||||
'[hooks.state]',
|
||||
'# system-owned parent',
|
||||
'',
|
||||
'[projects."/repo"] # explicit revocation',
|
||||
'trust_level = "untrusted"',
|
||||
'',
|
||||
@@ -310,6 +316,8 @@ describe('syncSystemConfigIntoManagedCodexHome', () => {
|
||||
expect(runtimeConfig).toContain('[projects."/system-only"]')
|
||||
expect(runtimeConfig).toContain('[hooks.state."runtime-hooks:stop:0:0"]')
|
||||
expect(runtimeConfig).not.toContain('[hooks.state."system-hooks:stop:0:0"]')
|
||||
expect(runtimeConfig).toContain('# runtime-owned parent')
|
||||
expect(runtimeConfig).not.toContain('# system-owned parent')
|
||||
expect(runtimeConfig).toContain('trust_level = "untrusted"')
|
||||
expect(runtimeConfig.match(/\[projects\."\/repo"\]/g)?.length).toBe(1)
|
||||
})
|
||||
|
||||
@@ -275,7 +275,10 @@ function isRuntimePreservedTomlSection(header: string): boolean {
|
||||
}
|
||||
|
||||
function isRuntimeHookTrustTomlSection(header: string): boolean {
|
||||
return header.trimStart().startsWith('[hooks.state.')
|
||||
const trimmed = header.trim()
|
||||
// Why: Codex's config writer materializes the parent table on Windows. It is
|
||||
// part of runtime-owned trust and must survive the next config mirror too.
|
||||
return trimmed === '[hooks.state]' || trimmed.startsWith('[hooks.state.')
|
||||
}
|
||||
|
||||
function isRuntimeProjectTomlSection(header: string): boolean {
|
||||
|
||||
@@ -0,0 +1,18 @@
|
||||
export async function waitForProcessExitUntil(
|
||||
exitPromise: Promise<void>,
|
||||
timeoutMs: number
|
||||
): Promise<void> {
|
||||
let timer: ReturnType<typeof setTimeout> | undefined
|
||||
const timeout = new Promise<void>((resolve) => {
|
||||
timer = setTimeout(resolve, timeoutMs)
|
||||
})
|
||||
try {
|
||||
await Promise.race([exitPromise, timeout])
|
||||
} finally {
|
||||
// Why: this runs in a short-lived entry; a live grace timer delays the
|
||||
// parent spawnSync even after the app-server process has already exited.
|
||||
if (timer !== undefined) {
|
||||
clearTimeout(timer)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import { wrapPosixHookCommand } from '../agent-hooks/installer-utils'
|
||||
import {
|
||||
computeTrustedHash,
|
||||
parseTrustKey,
|
||||
readHookTrustEntries,
|
||||
upsertHookTrustEntries,
|
||||
type CodexTrustEntry
|
||||
} from './config-toml-trust'
|
||||
@@ -160,7 +161,7 @@ describe('CodexHookService app-server trust grant lane', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('upgrades self-computed trust in place without duplicate tables', () => {
|
||||
it('upgrades self-computed trust in place without duplicate logical entries', () => {
|
||||
prepareSystemHome()
|
||||
const service = new CodexHookService()
|
||||
process.env.ORCA_DISABLE_CODEX_TRUST_RPC = '1'
|
||||
@@ -171,6 +172,9 @@ describe('CodexHookService app-server trust grant lane', () => {
|
||||
|
||||
expect(service.install().state).toBe('installed')
|
||||
const upgraded = readFileSync(join(managedHome, 'config.toml'), 'utf-8')
|
||||
// Why: the legacy Windows fallback intentionally writes slash variants;
|
||||
// duplicate detection is about the normalized trust identity.
|
||||
const upgradedEntries = readHookTrustEntries(join(managedHome, 'config.toml'))
|
||||
for (const eventLabel of [
|
||||
'session_start',
|
||||
'user_prompt_submit',
|
||||
@@ -179,13 +183,12 @@ describe('CodexHookService app-server trust grant lane', () => {
|
||||
'post_tool_use',
|
||||
'stop'
|
||||
]) {
|
||||
const count = upgraded
|
||||
.split('\n')
|
||||
.filter(
|
||||
(line) => line.startsWith('[hooks.state.') && line.includes(`:${eventLabel}:0:0`)
|
||||
).length
|
||||
expect(count, `duplicate trust tables for ${eventLabel}`).toBe(1)
|
||||
const count = [...upgradedEntries.keys()].filter((key) =>
|
||||
key.endsWith(`:${eventLabel}:0:0`)
|
||||
).length
|
||||
expect(count, `duplicate trust entries for ${eventLabel}`).toBe(1)
|
||||
}
|
||||
expect(upgraded).toContain('sha256:codex-session_start')
|
||||
})
|
||||
|
||||
it('leaves user trust byte-untouched while granting managed entries', () => {
|
||||
|
||||
@@ -522,7 +522,11 @@ describe('Codex WSL runtime hook install app-server grant lane', () => {
|
||||
const basePlan = createTestPlan()
|
||||
writeFileSync(basePlan.configPath, '{"hooks":{}}\n', 'utf-8')
|
||||
writeFileSync(basePlan.tomlPath, '', 'utf-8')
|
||||
let staleKeyExpectedRemoved: string | null = null
|
||||
const runner = vi.fn((request: CodexHookTrustGrantRequest) => {
|
||||
if (staleKeyExpectedRemoved) {
|
||||
expect(readHookTrustEntries(basePlan.tomlPath).has(staleKeyExpectedRemoved)).toBe(false)
|
||||
}
|
||||
const entries = request.expectedTrustKeys.map((key) => {
|
||||
const parsed = parseTrustKey(key)!
|
||||
return {
|
||||
@@ -557,6 +561,7 @@ describe('Codex WSL runtime hook install app-server grant lane', () => {
|
||||
const oldKey = computeTrustKey(
|
||||
getManagedTrustEntry(oldPlan, expectedManagedCommand(oldPlan.commandScriptPath))
|
||||
)
|
||||
staleKeyExpectedRemoved = oldKey
|
||||
|
||||
const newPlan = {
|
||||
...basePlan,
|
||||
|
||||
@@ -842,6 +842,13 @@ function installManagedHooksIntoWslRuntime(
|
||||
// Why: a successful re-grant replaces the ledger. Keep the previous
|
||||
// records long enough to prove ownership of stale canonical-path keys.
|
||||
const previousLedgerHome = readCodexTrustGrantLedgerHomeForReconciliation(runtimeHomePath)
|
||||
// Why: Codex's verified RPC write must be the final config mutation. A
|
||||
// host-side rewrite after verification can race or invalidate that grant.
|
||||
removeStaleWslRuntimeManagedHookTrustEntries(
|
||||
plan.tomlPath,
|
||||
trustEntries,
|
||||
previousLedgerHome ? [previousLedgerHome] : []
|
||||
)
|
||||
const grant = grantManagedCodexHookTrust({
|
||||
runtimeHomePath,
|
||||
tomlPath: plan.tomlPath,
|
||||
@@ -849,17 +856,10 @@ function installManagedHooksIntoWslRuntime(
|
||||
managedEntries: trustEntries,
|
||||
host: { kind: 'wsl', distro: plan.wslDistro, linuxRuntimeHome: plan.linuxRuntimeHome }
|
||||
})
|
||||
if (grant.lane === 'rpc') {
|
||||
removeStaleWslRuntimeManagedHookTrustEntries(
|
||||
plan.tomlPath,
|
||||
grant.entries,
|
||||
previousLedgerHome ? [previousLedgerHome] : []
|
||||
)
|
||||
} else {
|
||||
if (grant.lane === 'fallback') {
|
||||
// Why: WSL runtime homes may carry user hook approvals we did not rebuild
|
||||
// here; only upsert Orca's entries instead of sweeping the whole source.
|
||||
upsertHookTrustEntries(plan.tomlPath, trustEntries)
|
||||
removeStaleWslRuntimeManagedHookTrustEntries(plan.tomlPath, trustEntries)
|
||||
}
|
||||
} catch (error) {
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user