mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 08:03:13 +00:00
fix(codex): harden real-home heal boundaries
This commit is contained in:
@@ -1003,6 +1003,9 @@ describe('CodexRuntimeHomeService', () => {
|
||||
getRuntimeCodexHomePath(),
|
||||
getSystemCodexHomePath()
|
||||
])
|
||||
service.setRealHomeLaneGate(() => false)
|
||||
expect(service.getHostCodexHomePathsForSessionDiscovery()).toEqual([getRuntimeCodexHomePath()])
|
||||
service.setRealHomeLaneGate(() => true)
|
||||
const perSpawnCustomHome = join(testState.fakeHomeDir, 'per-spawn-custom-codex-home')
|
||||
expect(service.isHostSystemDefaultRealHome({ CODEX_HOME: perSpawnCustomHome })).toBe(false)
|
||||
expect(service.prepareForCodexLaunch(undefined, { CODEX_HOME: perSpawnCustomHome })).toBe(
|
||||
|
||||
@@ -217,7 +217,7 @@ export class CodexRuntimeHomeService {
|
||||
|
||||
getHostCodexHomePathsForSessionDiscovery(): string[] {
|
||||
const homes = [this.getRuntimeHomePath()]
|
||||
if (this.isHostSystemDefaultRealHomeSelected()) {
|
||||
if (this.isHostSystemDefaultRealHome()) {
|
||||
// Why: nested Orca processes can retain an ambient managed CODEX_HOME;
|
||||
// explicitly include the real lane so its sessions remain discoverable.
|
||||
homes.push(getSystemCodexHomePath())
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
runCodexHookTrustGrantSession,
|
||||
type CodexHookTrustGrantRequest
|
||||
} from './codex-app-server-client'
|
||||
import { runCodexAppServerSession } from './codex-app-server-session'
|
||||
import {
|
||||
resolveCodexGrantEntryPath,
|
||||
runCodexHookTrustGrantSessionSync
|
||||
@@ -234,6 +235,24 @@ describe('runCodexHookTrustGrantSession', () => {
|
||||
expect(result.outcome).toBe('verify-failed')
|
||||
})
|
||||
|
||||
it('rejects duplicate normalized aliases that conceal a missing expected key', async () => {
|
||||
const aliasedKey = 'C:\\Users\\Ada\\.codex\\hooks.json:session_start:0:0'
|
||||
const { request, recordFile } = createStubRequest({
|
||||
scenario: 'happy',
|
||||
hooks: [
|
||||
managedHook(aliasedKey),
|
||||
managedHook('c:/users/ada/.codex/hooks.json:session_start:0:0')
|
||||
],
|
||||
expectedTrustKeys: [aliasedKey, 'C:\\Users\\Ada\\.codex\\hooks.json:stop:0:0'],
|
||||
managedCommand: MANAGED_COMMAND
|
||||
})
|
||||
|
||||
await expect(runCodexHookTrustGrantSession(request)).resolves.toMatchObject({
|
||||
outcome: 'verify-failed'
|
||||
})
|
||||
expect(existsSync(recordFile)).toBe(false)
|
||||
})
|
||||
|
||||
it('decodes JSONL when a non-ASCII hook path is split across stdout chunks', async () => {
|
||||
const command = "/bin/sh '/tmp/rené/codex-hook.sh'"
|
||||
const key = '/home/rené/.codex/hooks.json:session_start:0:0'
|
||||
@@ -299,6 +318,24 @@ describe('runCodexHookTrustGrantSession', () => {
|
||||
expect(() => process.kill(childPid, 0)).toThrow()
|
||||
})
|
||||
|
||||
it('bounds a callback that stalls between RPC requests', async () => {
|
||||
const { request, pidFile } = createStubRequest({
|
||||
scenario: 'happy',
|
||||
hooks: [],
|
||||
expectedTrustKeys: [],
|
||||
managedCommand: MANAGED_COMMAND,
|
||||
timeoutMs: 500
|
||||
})
|
||||
|
||||
const startedAt = Date.now()
|
||||
await expect(
|
||||
runCodexAppServerSession(request.invocation, async () => new Promise<never>(() => {}))
|
||||
).rejects.toBeInstanceOf(CodexAppServerTimeoutError)
|
||||
expect(Date.now() - startedAt).toBeLessThan(5_000)
|
||||
const childPid = Number(readFileSync(pidFile, 'utf8'))
|
||||
expect(() => process.kill(childPid, 0)).toThrow()
|
||||
})
|
||||
|
||||
it('surfaces spawn failures as regular errors, not capability signals', async () => {
|
||||
const request: CodexHookTrustGrantRequest = {
|
||||
invocation: {
|
||||
|
||||
@@ -110,10 +110,14 @@ export async function runCodexHookTrustGrantSession(
|
||||
|
||||
const listResult = await rpc.request('hooks/list', { cwds: [request.hooksListCwd] })
|
||||
const managedListings = collectHookListings(listResult).filter(matchManaged)
|
||||
if (managedListings.length !== expectedKeys.size) {
|
||||
const managedKeyCoverage = normalizedKeyCoverage(managedListings)
|
||||
if (
|
||||
managedListings.length !== expectedKeys.size ||
|
||||
!setContainsEvery(managedKeyCoverage, expectedKeys)
|
||||
) {
|
||||
return {
|
||||
outcome: 'verify-failed',
|
||||
reason: `hooks/list reported ${managedListings.length} of ${expectedKeys.size} expected managed entries`
|
||||
reason: `hooks/list reported ${managedListings.length} entries covering ${managedKeyCoverage.size} of ${expectedKeys.size} expected managed entries`
|
||||
}
|
||||
}
|
||||
|
||||
@@ -133,14 +137,19 @@ export async function runCodexHookTrustGrantSession(
|
||||
|
||||
const verifyResult = await rpc.request('hooks/list', { cwds: [request.hooksListCwd] })
|
||||
const verifiedListings = collectHookListings(verifyResult).filter(matchManaged)
|
||||
const verifiedKeyCoverage = normalizedKeyCoverage(verifiedListings)
|
||||
const untrusted = verifiedListings.filter((listing) => listing.trustStatus !== 'trusted')
|
||||
if (verifiedListings.length !== expectedKeys.size || untrusted.length > 0) {
|
||||
if (
|
||||
verifiedListings.length !== expectedKeys.size ||
|
||||
!setContainsEvery(verifiedKeyCoverage, expectedKeys) ||
|
||||
untrusted.length > 0
|
||||
) {
|
||||
return {
|
||||
outcome: 'verify-failed',
|
||||
reason:
|
||||
untrusted.length > 0
|
||||
? `post-grant verify left ${untrusted.length} entries ${untrusted[0].trustStatus}`
|
||||
: `post-grant verify reported ${verifiedListings.length} of ${expectedKeys.size} entries`
|
||||
: `post-grant verify reported ${verifiedListings.length} entries covering ${verifiedKeyCoverage.size} of ${expectedKeys.size} expected entries`
|
||||
}
|
||||
}
|
||||
return {
|
||||
@@ -156,3 +165,16 @@ export async function runCodexHookTrustGrantSession(
|
||||
spawnImpl
|
||||
)
|
||||
}
|
||||
|
||||
function normalizedKeyCoverage(listings: readonly CodexHookListing[]): Set<string> {
|
||||
return new Set(listings.map((listing) => normalizeHookTrustKeyForLookup(listing.key)))
|
||||
}
|
||||
|
||||
function setContainsEvery(values: ReadonlySet<string>, expected: ReadonlySet<string>): boolean {
|
||||
for (const value of expected) {
|
||||
if (!values.has(value)) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -152,14 +152,18 @@ export async function runCodexAppServerSession<T>(
|
||||
pending.clear()
|
||||
}
|
||||
|
||||
let rejectDeadline: (error: Error) => void = () => {}
|
||||
const deadlinePromise = new Promise<never>((_resolve, reject) => {
|
||||
rejectDeadline = reject
|
||||
})
|
||||
const deadline = setTimeout(() => {
|
||||
timedOut = true
|
||||
child.kill('SIGKILL')
|
||||
failPending(
|
||||
new CodexAppServerTimeoutError(
|
||||
`codex app-server session exceeded ${invocation.timeoutMs}ms (${invocation.command})`
|
||||
)
|
||||
const error = new CodexAppServerTimeoutError(
|
||||
`codex app-server session exceeded ${invocation.timeoutMs}ms (${invocation.command})`
|
||||
)
|
||||
child.kill('SIGKILL')
|
||||
failPending(error)
|
||||
rejectDeadline(error)
|
||||
}, invocation.timeoutMs)
|
||||
|
||||
function sendLine(payload: Record<string, unknown>): void {
|
||||
@@ -227,11 +231,16 @@ export async function runCodexAppServerSession<T>(
|
||||
}
|
||||
|
||||
try {
|
||||
await requestRpc('initialize', {
|
||||
clientInfo: { name: 'orca_desktop', title: 'Orca', version: '0.0.0' }
|
||||
})
|
||||
notify('initialized')
|
||||
return await body({ request: requestRpc, notify })
|
||||
const session = async (): Promise<T> => {
|
||||
await requestRpc('initialize', {
|
||||
clientInfo: { name: 'orca_desktop', title: 'Orca', version: '0.0.0' }
|
||||
})
|
||||
notify('initialized')
|
||||
return body({ request: requestRpc, notify })
|
||||
}
|
||||
// Why: the timeout owns the whole callback, including time between RPCs;
|
||||
// killing the child alone cannot settle a callback awaiting unrelated work.
|
||||
return await Promise.race([session(), deadlinePromise])
|
||||
} catch (error) {
|
||||
if (
|
||||
error instanceof Error &&
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
import { sep } from 'node:path'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { hasCustomCodexHomeOverride } from './codex-real-home-path'
|
||||
|
||||
describe('hasCustomCodexHomeOverride', () => {
|
||||
it('recognizes normalized aliases of Orca-owned CODEX_HOME', () => {
|
||||
const managedHome = `${process.cwd()}${sep}codex-runtime-home${sep}home`
|
||||
|
||||
expect(
|
||||
hasCustomCodexHomeOverride({
|
||||
CODEX_HOME: `${managedHome}${sep}.`,
|
||||
ORCA_CODEX_HOME: managedHome
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('preserves a genuinely custom CODEX_HOME', () => {
|
||||
expect(
|
||||
hasCustomCodexHomeOverride({
|
||||
CODEX_HOME: `${process.cwd()}${sep}custom-codex-home`,
|
||||
ORCA_CODEX_HOME: `${process.cwd()}${sep}codex-runtime-home${sep}home`
|
||||
})
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -5,12 +5,16 @@ import { getSystemCodexHomePath } from './codex-home-paths'
|
||||
export function hasCustomCodexHomeOverride(env: NodeJS.ProcessEnv = process.env): boolean {
|
||||
const codexHome = env.CODEX_HOME?.trim()
|
||||
const orcaCodexHome = env.ORCA_CODEX_HOME?.trim()
|
||||
const normalizedCodexHome = codexHome ? normalizePathForComparison(codexHome) : undefined
|
||||
const normalizedOrcaCodexHome = orcaCodexHome
|
||||
? normalizePathForComparison(orcaCodexHome)
|
||||
: undefined
|
||||
// Why: phase 1 owns only ~/.codex and can clean that path on downgrade. A
|
||||
// custom home needs cross-home ownership tracking before Orca may mutate it.
|
||||
return Boolean(
|
||||
codexHome &&
|
||||
codexHome !== orcaCodexHome &&
|
||||
normalizePathForComparison(codexHome) !== normalizePathForComparison(getSystemCodexHomePath())
|
||||
normalizedCodexHome &&
|
||||
normalizedCodexHome !== normalizedOrcaCodexHome &&
|
||||
normalizedCodexHome !== normalizePathForComparison(getSystemCodexHomePath())
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -7,18 +7,30 @@ export function createCodexSessionBackfillAuditWriter(
|
||||
auditLogPath: string
|
||||
): CodexSessionBackfillAuditWriter {
|
||||
let auditDirectoryReady: Promise<string | undefined> | undefined
|
||||
const appendRecord = async (serializedRecord: string): Promise<void> => {
|
||||
auditDirectoryReady ??= mkdir(dirname(auditLogPath), { recursive: true }).catch(
|
||||
(error: unknown) => {
|
||||
auditDirectoryReady = undefined
|
||||
throw error
|
||||
}
|
||||
)
|
||||
await auditDirectoryReady
|
||||
await appendFile(auditLogPath, serializedRecord, { encoding: 'utf-8' })
|
||||
}
|
||||
return async (record): Promise<void> => {
|
||||
const serializedRecord = `${JSON.stringify({ at: new Date().toISOString(), ...record })}\n`
|
||||
try {
|
||||
auditDirectoryReady ??= mkdir(dirname(auditLogPath), { recursive: true })
|
||||
await auditDirectoryReady
|
||||
await appendFile(
|
||||
auditLogPath,
|
||||
`${JSON.stringify({ at: new Date().toISOString(), ...record })}\n`,
|
||||
{ encoding: 'utf-8' }
|
||||
)
|
||||
await appendRecord(serializedRecord)
|
||||
return
|
||||
} catch {
|
||||
// Why: the heal consumes this ledger as its work queue. Retry the same
|
||||
// record once so a transient mkdir/write failure cannot omit a session.
|
||||
}
|
||||
try {
|
||||
await appendRecord(serializedRecord)
|
||||
} catch (error) {
|
||||
// Why: the audit trail is diagnostics; losing a line must not fail the
|
||||
// backfill or leave a half-linked tree unrecorded in the summary counts.
|
||||
// Why: a published hardlink/copy may already be in use, so persistent
|
||||
// ledger failure is reported but cannot safely roll back the backfill.
|
||||
console.warn('[codex-session-backfill] Failed to append audit record:', error)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -26,6 +26,7 @@ const { fsMockState } = vi.hoisted(() => ({
|
||||
failInstallLink: false,
|
||||
failInstallRename: false,
|
||||
failCopy: false,
|
||||
failAuditMkdirOnce: false,
|
||||
failDirectoryPath: null as string | null,
|
||||
failLstatPath: null as string | null
|
||||
}
|
||||
@@ -48,6 +49,17 @@ vi.mock('node:fs/promises', async () => {
|
||||
const actual = await vi.importActual<typeof NodeFsPromises>('node:fs/promises')
|
||||
return {
|
||||
...actual,
|
||||
mkdir: (...args: Parameters<typeof actual.mkdir>) => {
|
||||
if (fsMockState.failAuditMkdirOnce && String(args[0]).includes('codex-session-backfill')) {
|
||||
fsMockState.failAuditMkdirOnce = false
|
||||
const error = new Error(
|
||||
'EACCES: transient audit directory failure'
|
||||
) as NodeJS.ErrnoException
|
||||
error.code = 'EACCES'
|
||||
throw error
|
||||
}
|
||||
return actual.mkdir(...args)
|
||||
},
|
||||
lstat: (...args: Parameters<typeof actual.lstat>) => {
|
||||
if (args[0] === fsMockState.failLstatPath) {
|
||||
const error = new Error('EACCES: path inaccessible') as NodeJS.ErrnoException
|
||||
@@ -156,6 +168,7 @@ beforeEach(() => {
|
||||
fsMockState.failInstallLink = false
|
||||
fsMockState.failInstallRename = false
|
||||
fsMockState.failCopy = false
|
||||
fsMockState.failAuditMkdirOnce = false
|
||||
fsMockState.failDirectoryPath = null
|
||||
fsMockState.failLstatPath = null
|
||||
fakeHomeDir = mkdtempSync(join(tmpdir(), 'orca-codex-backfill-home-'))
|
||||
@@ -292,6 +305,18 @@ describe('backfillManagedCodexSessionsIntoSystemHome', () => {
|
||||
expect(second).toMatchObject({ linkedFiles: 0, copiedFiles: 0, skippedExistingFiles: 1 })
|
||||
})
|
||||
|
||||
it('retries the same audit record after a transient directory failure', async () => {
|
||||
fsMockState.failAuditMkdirOnce = true
|
||||
writeManagedSession(join('2026', '05', '26', 'rollout-a.jsonl'), '{"id":"a"}\n')
|
||||
|
||||
const summary = await backfillManagedCodexSessionsIntoSystemHome(
|
||||
resolveCodexSessionBackfillPaths()
|
||||
)
|
||||
|
||||
expect(summary).toMatchObject({ linkedFiles: 1, failedFiles: 0 })
|
||||
expect(readAuditActions()).toEqual(['hardlink', 'run-summary'])
|
||||
})
|
||||
|
||||
it('falls back to copy when hardlinking fails across volumes', async () => {
|
||||
fsMockState.failLink = true
|
||||
const managedPath = writeManagedSession(
|
||||
|
||||
@@ -50,7 +50,7 @@ export type HealMarkerSummary = {
|
||||
export function collectPendingHealThreads(paths: CodexSessionIndexHealPaths): PendingHealThread[] {
|
||||
const processedThreadIds = readProcessedHealThreadIds(paths)
|
||||
const pendingByThreadId = new Map<string, PendingHealThread>()
|
||||
for (const line of readJsonlLines(paths.auditLogPath)) {
|
||||
for (const line of readJsonlLines(paths.auditLogPath, true)) {
|
||||
if ((line.action !== 'hardlink' && line.action !== 'copy') || typeof line.target !== 'string') {
|
||||
continue
|
||||
}
|
||||
@@ -118,11 +118,16 @@ export function appendHealLedgerRecord(
|
||||
}
|
||||
}
|
||||
|
||||
function readJsonlLines(filePath: string): Record<string, unknown>[] {
|
||||
function readJsonlLines(filePath: string, throwOnReadFailure = false): Record<string, unknown>[] {
|
||||
let contents: string
|
||||
try {
|
||||
contents = readFileSync(filePath, 'utf-8')
|
||||
} catch {
|
||||
} catch (error) {
|
||||
if (throwOnReadFailure && !isNotFoundError(error)) {
|
||||
// Why: the audit is the heal work queue. Treating EACCES/EIO as empty
|
||||
// would write a completion marker that permanently skips every session.
|
||||
throw error
|
||||
}
|
||||
return []
|
||||
}
|
||||
const lines: Record<string, unknown>[] = []
|
||||
@@ -145,11 +150,18 @@ function readJsonlLines(filePath: string): Record<string, unknown>[] {
|
||||
export function readAuditLogSize(auditLogPath: string): number {
|
||||
try {
|
||||
return statSync(auditLogPath).size
|
||||
} catch {
|
||||
} catch (error) {
|
||||
if (!isNotFoundError(error)) {
|
||||
throw error
|
||||
}
|
||||
return 0
|
||||
}
|
||||
}
|
||||
|
||||
function isNotFoundError(error: unknown): boolean {
|
||||
return (error as NodeJS.ErrnoException | null)?.code === 'ENOENT'
|
||||
}
|
||||
|
||||
export function isHealMarkerCurrent(
|
||||
paths: CodexSessionIndexHealPaths,
|
||||
auditBytes: number
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import {
|
||||
appendFileSync,
|
||||
existsSync,
|
||||
mkdtempSync,
|
||||
mkdirSync,
|
||||
readFileSync,
|
||||
@@ -8,7 +9,7 @@ import {
|
||||
writeFileSync
|
||||
} from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { dirname, join } from 'node:path'
|
||||
import type { CodexAppServerInvocation } from './codex-app-server-session'
|
||||
import { CODEX_SESSION_INDEX_HEAL_VERSION } from './codex-session-index-heal-state'
|
||||
import {
|
||||
@@ -540,4 +541,19 @@ describe('runCodexSessionIndexHeal', () => {
|
||||
expect(summary).toMatchObject({ outcome: 'completed', healedThreads: 1 })
|
||||
expect(rig.readLog().threadIds).toEqual([currentId])
|
||||
})
|
||||
|
||||
it('does not mark the heal complete when the audit cannot be read', async () => {
|
||||
const rig = createHealRig({})
|
||||
rig.paths.auditLogPath = dirname(rig.paths.auditLogPath)
|
||||
|
||||
await expect(
|
||||
runCodexSessionIndexHeal(rig.paths, {
|
||||
buildInvocation: rig.buildInvocation,
|
||||
interBatchDelayMs: 0
|
||||
})
|
||||
).rejects.toBeInstanceOf(Error)
|
||||
|
||||
expect(rig.readLog().serverStarts).toBe(0)
|
||||
expect(existsSync(rig.paths.healMarkerPath)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user