Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,18 @@ jobs:
pnpm exec vitest run --config config/vitest.config.ts \
src/main/pty/codex-no-daemon-binary-contract.test.ts

# Why the no-daemon install: it is the newest pinned Codex, and its project
# lookup decides whether a worktree launch stops at the trust prompt.
- name: Verify Codex project-trust contract
env:
ORCA_CODEX_TRUST_CONTRACT_REQUIRED: '1'
ORCA_CODEX_TRUST_CONTRACT_VERSION: ${{ env.CODEX_NO_DAEMON_CLI_VERSION }}
run: |
set -euo pipefail
ORCA_CODEX_TRUST_CONTRACT_BINARY="$RUNNER_TEMP/codex-cli-no-daemon/node_modules/.bin/codex" \
pnpm exec vitest run --config config/vitest.config.ts \
src/main/agent-trust-presets.test.ts

xterm_patch_sync:
name: xterm patch sync
needs: [code_paths]
Expand Down
13 changes: 13 additions & 0 deletions config/scripts/codex-index-heal-contract-workflow.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,17 @@ describe('Codex index-heal contract PR gate', () => {
)
expect(verify.run).toContain('src/main/pty/codex-no-daemon-binary-contract.test.ts')
})

it('pins the project-trust contract to the no-daemon Codex and fails when it is missing', () => {
const verify = job.steps.find((step) => step.name === 'Verify Codex project-trust contract')

expect(verify.env.ORCA_CODEX_TRUST_CONTRACT_VERSION).toBe(
'${{ env.CODEX_NO_DAEMON_CLI_VERSION }}'
)
expect(verify.env.ORCA_CODEX_TRUST_CONTRACT_REQUIRED).toBe('1')
expect(verify.run).toContain(
'ORCA_CODEX_TRUST_CONTRACT_BINARY="$RUNNER_TEMP/codex-cli-no-daemon/node_modules/.bin/codex"'
)
expect(verify.run).toContain('src/main/agent-trust-presets.test.ts')
})
})
4 changes: 3 additions & 1 deletion config/scripts/pr-code-change-scope.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,10 @@ const GIT_COMPAT_PREFIXES = [

// Why narrow: the contract pins Codex's read-repair, so it runs when the heal that
// depends on it, its app-server transport, or the contract itself changes. The same
// job pins --no-daemon for Orca's codex shell wrapper.
// job pins --no-daemon for Orca's codex shell wrapper and the project-trust key.
const CODEX_INDEX_HEAL_CONTRACT_PREFIXES = [
'src/main/agent-trust-presets',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Trust contract has missing triggers

The new trigger covers agent-trust-presets.ts and its test, but not helpers that locate or write Codex trust entries, such as codex-home-paths.ts and config-toml-trust.ts. A change to either helper can skip the pinned-binary trust test, so that test may miss a regression in the path or entry Codex reads. This is a non-blocking CI coverage gap; include those dependencies in the job’s path triggers.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@Jinwoo-H Jinwoo-H Sep 29, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added src/main/codex/config-toml-trust to the contract triggers in 3315c33 (with a classification test). Leaving out codex-home-paths: it decides which config files Orca writes, and the ordinary (no-binary) run of the same test already asserts the key lands in both the system and runtime config, so a regression there fails in every unit run without the pinned binary.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That distinction is reasonable. With config-toml-trust now covered, the remaining pinned-binary-specific dependency is exercised; agent-trust-presets.test.ts already verifies that getLocalCodexTrustConfigFiles() causes the workspace key to be written to both the system and runtime config paths, so a codex-home-paths regression should fail the ordinary test run. No further trigger change is needed.

'src/main/codex/config-toml-trust',
'src/main/pty/codex-no-daemon-binary-contract',
'src/main/pty/codex-shell-launch-preflight',
'src/main/codex/codex-index-heal-binary-contract',
Expand Down
7 changes: 7 additions & 0 deletions config/scripts/pr-code-change-scope.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,13 @@ describe('per-job path classification', () => {
expectClassification(['src/main/codex/codex-index-heal-binary-contract.test.ts'], {
codex_index_heal_contract: true
})
for (const file of ['src/main/agent-trust-presets.ts', 'src/main/codex/config-toml-trust.ts']) {
expectClassification([file], {
codex_index_heal_contract: true,
package: true,
package_windows: true
})
}
// Keep the real-binary gate live when a transport or launch dependency changes.
for (const file of [
'src/main/codex/codex-app-server-capability-signal.ts',
Expand Down
184 changes: 129 additions & 55 deletions src/main/agent-trust-presets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import {
} from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
import { runProcessSync } from '../shared/child-process/run-process'
import { runCodexAppServerSession } from './codex/codex-app-server-session'

const testState = {
fakeHomeDir: '',
Expand Down Expand Up @@ -48,6 +50,16 @@ const { runExclusivelyForCodexTrustConfig } =
await import('./codex/codex-trust-config-mutation-queue')
const { getLocalCodexTrustConfigFiles } = await import('./codex/codex-home-paths')

// Why: fixture tests pin Orca's half; only the real binary proves Codex reads the key
// Orca writes. CI sets REQUIRED so a missing binary fails instead of skipping.
const codexTrustContract = {
binary: process.env.ORCA_CODEX_TRUST_CONTRACT_BINARY,
version: process.env.ORCA_CODEX_TRUST_CONTRACT_VERSION
}
if (process.env.ORCA_CODEX_TRUST_CONTRACT_REQUIRED === '1' && !codexTrustContract.binary) {
throw new Error('ORCA_CODEX_TRUST_CONTRACT_REQUIRED=1 but no Codex binary was given')
}

beforeEach(() => {
testState.fakeHomeDir = mkdtempSync(join(tmpdir(), 'orca-trust-presets-'))
testState.userDataDir = mkdtempSync(join(tmpdir(), 'orca-trust-presets-user-data-'))
Expand Down Expand Up @@ -238,69 +250,127 @@ describe('markCodexProjectTrusted', () => {
}
})

it('trusts the main repository root for a linked worktree without reading commondir', async () => {
const fixtureRoot = mkdtempSync(join(tmpdir(), 'orca-codex-linked-ws-'))
const repository = join(fixtureRoot, 'repo')
const workspace = join(fixtureRoot, 'worktrees', 'feature')
const worktreeGitDir = join(repository, '.git', 'worktrees', 'feature')
try {
mkdirSync(worktreeGitDir, { recursive: true })
mkdirSync(workspace, { recursive: true })
writeFileSync(join(workspace, '.git'), `gitdir: ${worktreeGitDir}\n`, 'utf-8')
writeFileSync(join(worktreeGitDir, 'gitdir'), join(workspace, '.git'), 'utf-8')

await markCodexProjectTrusted(workspace, getLocalCodexTrustConfigFiles())

const repositoryRoot = realpathSync.native(repository)
const workspaceRoot = realpathSync.native(workspace)
const configPath = join(testState.fakeHomeDir, '.codex', 'config.toml')
const runtimeConfigPath = join(
testState.userDataDir,
'codex-runtime-home',
'home',
'config.toml'
)
for (const written of [
readFileSync(configPath, 'utf-8'),
readFileSync(runtimeConfigPath, 'utf-8')
]) {
expect(written).toContain(`[projects."${escapeTomlBasicString(repositoryRoot)}"]`)
expect(written).not.toContain(`[projects."${escapeTomlBasicString(workspaceRoot)}"]`)
// Why: Codex checks the cwd's own entry before any repo root, so the workspace key
// must satisfy it whatever the git layout.
describe('linked worktree trust key', { timeout: 120_000 }, () => {
let fixtureRoot = ''
beforeAll(() => {
if (codexTrustContract.binary) {
const version = runProcessSync({ program: codexTrustContract.binary, args: ['--version'] })
expect(version.stdout.trim()).toBe(`codex-cli ${codexTrustContract.version}`)
}
} finally {
})
beforeEach(() => {
fixtureRoot = realpathSync.native(mkdtempSync(join(tmpdir(), 'orca-codex-layout-')))
})
afterEach(() => {
rmSync(fixtureRoot, { recursive: true, force: true })
})

function git(cwd: string, ...args: string[]): void {
const result = runProcessSync({
program: 'git',
args: ['-c', 'user.name=Orca', '-c', 'user.email=orca@example.com', ...args],
cwd
})
expect(result.code, result.stderr).toBe(0)
}
})

it('does not broaden trust through arbitrary or adversarial Git metadata', async () => {
const fixtureRoot = mkdtempSync(join(tmpdir(), 'orca-codex-untrusted-gitdir-'))
const workspace = join(fixtureRoot, 'workspace')
const arbitraryGitDir = join(fixtureRoot, 'metadata', 'feature')
const unrelatedRoot = join(fixtureRoot, 'unrelated')
try {
mkdirSync(arbitraryGitDir, { recursive: true })
mkdirSync(workspace, { recursive: true })
mkdirSync(unrelatedRoot, { recursive: true })
writeFileSync(join(workspace, '.git'), `gitdir: ${arbitraryGitDir}\n`, 'utf-8')
writeFileSync(join(arbitraryGitDir, 'commondir'), join(unrelatedRoot, '.git'), 'utf-8')
function initRepoWithCommit(repo: string, ...initArgs: string[]): void {
mkdirSync(repo, { recursive: true })
git(repo, 'init', '-q', ...initArgs)
git(repo, '-c', 'commit.gpgsign=false', 'commit', '-q', '--allow-empty', '-m', 'init')
}

async function expectWorkspaceTrusted(workspace: string): Promise<void> {
await markCodexProjectTrusted(workspace, getLocalCodexTrustConfigFiles())
const structuredGitDir = join(unrelatedRoot, '.git', 'worktrees', 'feature')
mkdirSync(structuredGitDir, { recursive: true })
writeFileSync(join(workspace, '.git'), `gitdir: ${structuredGitDir}\n`, 'utf-8')
writeFileSync(join(structuredGitDir, 'gitdir'), join(unrelatedRoot, '.git'), 'utf-8')
await markCodexProjectTrusted(workspace, getLocalCodexTrustConfigFiles())

const written = readFileSync(join(testState.fakeHomeDir, '.codex', 'config.toml'), 'utf-8')
expect(written).toContain(
`[projects."${escapeTomlBasicString(realpathSync.native(workspace))}"]`
const runtimeHome = join(testState.userDataDir, 'codex-runtime-home', 'home')
const [system, runtime] = [
join(testState.fakeHomeDir, '.codex', 'config.toml'),
join(runtimeHome, 'config.toml')
].map((path) =>
[...readFileSync(path, 'utf-8').matchAll(/^\[projects\."(.*)"\]$/gm)].map((m) =>
m[1].replaceAll('\\\\', '\\')
)
)
expect(written).not.toContain(
`[projects."${escapeTomlBasicString(realpathSync.native(unrelatedRoot))}"]`
expect(system).toEqual([workspace])
expect(runtime).toEqual([workspace])
if (codexTrustContract.binary) {
expect(await codexSandboxFor(workspace, runtimeHome)).toBe('workspaceWrite')
const emptyHome = join(fixtureRoot, 'empty-codex-home')
mkdirSync(emptyHome, { recursive: true })
expect(await codexSandboxFor(workspace, emptyHome)).toBe('readOnly')
}
}

// Why thread/start: its default sandbox and the TUI trust prompt read the same
// project lookup, so workspaceWrite (not readOnly) means no prompt.
async function codexSandboxFor(cwd: string, codexHome: string): Promise<unknown> {
const binary = codexTrustContract.binary!
return runCodexAppServerSession(
{
command: binary,
args: ['-c', 'features.plugins=false', 'app-server'],
cliPath: binary,
env: { CODEX_HOME: codexHome, HOME: testState.fakeHomeDir },
timeoutMs: 60_000
},
async (rpc) => {
const started = await rpc.request('thread/start', { cwd })
const sandbox = isRecord(started) ? started.sandbox : undefined
return isRecord(sandbox) ? sandbox.type : undefined
}
)
} finally {
rmSync(fixtureRoot, { recursive: true, force: true })
}

it('trusts a linked worktree of a standard repository', async () => {
const repo = join(fixtureRoot, 'repo')
initRepoWithCommit(repo)
git(repo, 'worktree', 'add', '-q', join(fixtureRoot, 'wt'))
await expectWorkspaceTrusted(join(fixtureRoot, 'wt'))
})

it('trusts a worktree whose main checkout .git points at a sibling .bare dir', async () => {
const source = join(fixtureRoot, 'source')
const project = join(fixtureRoot, 'project')
initRepoWithCommit(source)
git(fixtureRoot, 'clone', '-q', '--bare', source, join(project, '.bare'))
writeFileSync(join(project, '.git'), 'gitdir: ./.bare\n', 'utf-8')
git(project, 'worktree', 'add', '-q', join(project, 'wt'))
await expectWorkspaceTrusted(join(project, 'wt'))
})

it('trusts a worktree of a bare repository', async () => {
const source = join(fixtureRoot, 'source')
const bare = join(fixtureRoot, 'proj.git')
initRepoWithCommit(source)
git(fixtureRoot, 'clone', '-q', '--bare', source, bare)
git(bare, 'worktree', 'add', '-q', join(fixtureRoot, 'wt'))
await expectWorkspaceTrusted(join(fixtureRoot, 'wt'))
})

it('trusts a worktree of a --separate-git-dir repository', async () => {
const repo = join(fixtureRoot, 'repo')
initRepoWithCommit(repo, `--separate-git-dir=${join(fixtureRoot, 'repo.git')}`)
git(repo, 'worktree', 'add', '-q', join(fixtureRoot, 'wt'))
await expectWorkspaceTrusted(join(fixtureRoot, 'wt'))
})

it('trusts a worktree whose parent .git points at another repository', async () => {
const source = join(fixtureRoot, 'source')
initRepoWithCommit(source)
git(fixtureRoot, 'clone', '-q', '--bare', source, join(fixtureRoot, 'proj.git'))
git(fixtureRoot, 'clone', '-q', '--bare', source, join(fixtureRoot, 'other.git'))
writeFileSync(join(fixtureRoot, '.git'), 'gitdir: ./other.git\n', 'utf-8')
git(join(fixtureRoot, 'proj.git'), 'worktree', 'add', '-q', join(fixtureRoot, 'wt'))
await expectWorkspaceTrusted(join(fixtureRoot, 'wt'))
})

it('trusts a plain folder workspace', async () => {
const folder = join(fixtureRoot, 'folder')
mkdirSync(folder)
await expectWorkspaceTrusted(folder)
})
})

it('writes ~/.codex/config.toml with the project marked trusted', async () => {
Expand Down Expand Up @@ -382,3 +452,7 @@ describe('markCodexProjectTrusted', () => {
function escapeTomlBasicString(value: string): string {
return value.replaceAll('\\', '\\\\').replaceAll('"', '\\"')
}

function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null
}
42 changes: 3 additions & 39 deletions src/main/agent-trust-presets.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { existsSync, mkdirSync, readFileSync, realpathSync } from 'node:fs'
import { homedir } from 'node:os'
import { basename, dirname, join, resolve } from 'node:path'
import { join } from 'node:path'
import { writeFileAtomically } from './codex-accounts/fs-utils'
import { upsertProjectTrustLevel } from './codex/config-toml-trust'
import { runExclusivelyForCodexTrustConfig } from './codex/codex-trust-config-mutation-queue'
Expand Down Expand Up @@ -169,7 +169,8 @@ export function markCodexProjectTrusted(
workspacePath: string,
configFiles: readonly string[]
): Promise<void> {
const absPath = resolveCodexProjectTrustRoot(workspacePath)
// Why: Codex checks the cwd's own entry before the repo root, so no git-layout logic is needed.
const absPath = canonicalize(workspacePath)
// Why (#16441): hook installs now await a codex app-server grant, so an
// unqueued write here can land inside their capture->restore window and be
// reverted. Same runtime-before-system lock order the installer takes.
Expand All @@ -184,43 +185,6 @@ export function markCodexProjectTrusted(
return write()
}

/** The folder Codex looks trust up under: a linked worktree's main checkout, else the realpath. */
export function resolveCodexProjectTrustRoot(workspacePath: string): string {
const absPath = canonicalize(workspacePath)
try {
const gitDirReference = readFileSync(join(absPath, '.git'), 'utf-8').trim()
if (!gitDirReference.startsWith('gitdir:')) {
return absPath
}
const gitDirPath = gitDirReference.slice('gitdir:'.length).trim()
if (!gitDirPath) {
return absPath
}
const gitDir = resolve(absPath, gitDirPath)
const worktreesDir = dirname(gitDir)
if (basename(worktreesDir) !== 'worktrees') {
return absPath
}
// Why: workspace-controlled .git metadata must not broaden trust without Git's reciprocal link.
const gitDirBacklink = readFileSync(join(gitDir, 'gitdir'), 'utf-8').trim()
if (!gitDirBacklink) {
return absPath
}
const resolvedBacklink = resolve(gitDir, gitDirBacklink)
const workspaceGitFile = join(absPath, '.git')
if (
resolvedBacklink !== workspaceGitFile &&
canonicalize(resolvedBacklink) !== canonicalize(workspaceGitFile)
) {
return absPath
}
// Why: mirror Codex's validated .git/worktrees/<name> traversal instead of trusting arbitrary commondir contents.
return canonicalize(dirname(dirname(worktreesDir)))
} catch {
return absPath
}
}

function canonicalize(p: string): string {
// Why: macOS reports `/tmp/x` and `/private/tmp/x` as the same inode, but
// both Cursor and Copilot's trust comparators run realpath() before the
Expand Down
1 change: 0 additions & 1 deletion src/main/agent-workspace-trust.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ const mocks = vi.hoisted(() => ({
}))

vi.mock('./agent-trust-presets', () => ({
resolveCodexProjectTrustRoot: (path: string) => path,
markCodexProjectTrusted: mocks.codex,
markCursorWorkspaceTrusted: mocks.cursor,
markCopilotFolderTrusted: mocks.copilot,
Expand Down
Loading
Loading