From 7378088bd48cd605b3b6043aab76c9ad5841bcc6 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 19 Jul 2026 16:33:32 -0700 Subject: [PATCH] test(codex): canonicalize harness containment guards and retry cleanup Resolve symlinks before the disposable-root containment checks so a symlinked temp parent cannot smuggle the throwaway home inside the primary home, and give the final cleanup rm Windows retry/force so a briefly lingering codex handle cannot strand the credential-bearing root. --- .../run-codex-real-account-validation.mjs | 29 +++++++++++++++-- .../run-codex-real-account-validation.test.ts | 31 ++++++++++++++++++- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/config/scripts/run-codex-real-account-validation.mjs b/config/scripts/run-codex-real-account-validation.mjs index 0146712539b..87f862c269c 100644 --- a/config/scripts/run-codex-real-account-validation.mjs +++ b/config/scripts/run-codex-real-account-validation.mjs @@ -57,6 +57,15 @@ function isWithin(candidate, parent) { return relative === '' || (!relative.startsWith('..') && !path.isAbsolute(relative)) } +async function resolveRealPath(candidate) { + try { + return await realpath(candidate) + } catch { + // Why: containment guards must still run for paths that do not exist yet. + return candidate + } +} + export function createValidationEnv(inheritedEnv, layout) { const env = { ...inheritedEnv } for (const key of RESTRICTED_ENV_KEYS) { @@ -78,10 +87,16 @@ export async function createValidationLayout(options = {}) { const primaryHome = path.resolve(options.primaryHome ?? os.homedir()) const envTempParent = process.env.ORCA_CODEX_VALIDATION_TEMP_PARENT?.trim() const tempParent = path.resolve(options.tempParent ?? (envTempParent || os.tmpdir())) + // Why: guards must compare canonical paths — a symlinked temp parent must + // not smuggle the disposable root inside the primary home. + const [primaryHomeReal, tempParentReal] = await Promise.all([ + resolveRealPath(primaryHome), + resolveRealPath(tempParent) + ]) // Why: on Windows the default %TEMP% lives inside %USERPROFILE%, which the // disposable-home guard below rightly refuses. Fail before creating anything // and point at the overrides instead of aborting with an opaque guard error. - if (samePath(tempParent, primaryHome) || isWithin(tempParent, primaryHome)) { + if (samePath(tempParentReal, primaryHomeReal) || isWithin(tempParentReal, primaryHomeReal)) { throw new Error( `Refusing to place the disposable validation root inside the primary home (${primaryHome}). ` + 'Pass --temp-parent or set ORCA_CODEX_VALIDATION_TEMP_PARENT to a directory outside it.' @@ -94,7 +109,8 @@ export async function createValidationLayout(options = {}) { mkdir(homeDir, { recursive: true, mode: 0o700 }), mkdir(userDataDir, { recursive: true, mode: 0o700 }) ]) - if (samePath(primaryHome, homeDir) || isWithin(homeDir, primaryHome)) { + const homeDirReal = await resolveRealPath(homeDir) + if (samePath(primaryHomeReal, homeDirReal) || isWithin(homeDirReal, primaryHomeReal)) { throw new Error('Refusing to place the disposable validation home inside the primary home') } return { primaryHome, tempRoot, homeDir, userDataDir } @@ -528,7 +544,14 @@ async function main() { if (options.keep) { console.warn(`Credential-bearing disposable root kept at ${layout.tempRoot}`) } else { - await rm(layout.tempRoot, { recursive: true }) + // Why: a detached codex descendant can briefly hold a Windows handle + // in the disposable home; retry so cleanup never strands credentials. + await rm(layout.tempRoot, { + recursive: true, + force: true, + maxRetries: 10, + retryDelay: 250 + }) console.log('Removed the credential-bearing disposable root.') } } finally { diff --git a/config/scripts/run-codex-real-account-validation.test.ts b/config/scripts/run-codex-real-account-validation.test.ts index 9689aac5ac7..025a961ade4 100644 --- a/config/scripts/run-codex-real-account-validation.test.ts +++ b/config/scripts/run-codex-real-account-validation.test.ts @@ -1,5 +1,5 @@ import { execFileSync } from 'node:child_process' -import { mkdtemp, rm } from 'node:fs/promises' +import { mkdir, mkdtemp, rm, symlink } from 'node:fs/promises' import os from 'node:os' import path from 'node:path' import { pathToFileURL } from 'node:url' @@ -99,6 +99,35 @@ describe('Codex real-account validation harness', () => { expect(path.dirname(layout.tempRoot)).toBe(tempParent) }) + it.skipIf(process.platform === 'win32')( + 'refuses a symlinked temp parent that resolves inside the primary home', + async () => { + const root = await mkdtemp(path.join(os.tmpdir(), 'orca-symlink-guard-')) + cleanupPaths.push(root) + const primaryHome = path.join(root, 'primary-home') + const insidePrimary = path.join(primaryHome, 'nested-temp') + const link = path.join(root, 'temp-link') + await mkdir(insidePrimary, { recursive: true }) + await symlink(insidePrimary, link) + + const { error } = runValidationModule<{ error: string | null }>( + ` + const { createValidationLayout } = await import(process.argv[1]) + try { + const layout = await createValidationLayout({ primaryHome: process.argv[2] }) + console.log(JSON.stringify({ error: null, layout })) + } catch (caught) { + console.log(JSON.stringify({ error: caught.message })) + } + `, + [primaryHome], + { ORCA_CODEX_VALIDATION_TEMP_PARENT: link } + ) + + expect(error).toContain('Refusing to place the disposable validation root') + } + ) + it('refuses a temp parent inside the primary home and points at the overrides', () => { // Why: matches Windows, where the default %TEMP% lives inside %USERPROFILE%. const { error } = runValidationModule<{ error: string | null }>(