diff --git a/codex-cli/src/approvals.ts b/codex-cli/src/approvals.ts index ff37a890..5ea73eab 100644 --- a/codex-cli/src/approvals.ts +++ b/codex-cli/src/approvals.ts @@ -71,13 +71,14 @@ export type ApprovalPolicy = */ export function canAutoApprove( command: ReadonlyArray, + workdir: string | undefined, policy: ApprovalPolicy, writableRoots: ReadonlyArray, env: NodeJS.ProcessEnv = process.env, ): SafetyAssessment { if (command[0] === "apply_patch") { return command.length === 2 && typeof command[1] === "string" - ? canAutoApproveApplyPatch(command[1], writableRoots, policy) + ? canAutoApproveApplyPatch(command[1], workdir, writableRoots, policy) : { type: "reject", reason: "Invalid apply_patch command", @@ -103,7 +104,12 @@ export function canAutoApprove( ) { const applyPatchArg = tryParseApplyPatch(command[2]); if (applyPatchArg != null) { - return canAutoApproveApplyPatch(applyPatchArg, writableRoots, policy); + return canAutoApproveApplyPatch( + applyPatchArg, + workdir, + writableRoots, + policy, + ); } let bashCmd; @@ -162,6 +168,7 @@ export function canAutoApprove( function canAutoApproveApplyPatch( applyPatchArg: string, + workdir: string | undefined, writableRoots: ReadonlyArray, policy: ApprovalPolicy, ): SafetyAssessment { @@ -179,7 +186,13 @@ function canAutoApproveApplyPatch( break; } - if (isWritePatchConstrainedToWritablePaths(applyPatchArg, writableRoots)) { + if ( + isWritePatchConstrainedToWritablePaths( + applyPatchArg, + workdir, + writableRoots, + ) + ) { return { type: "auto-approve", reason: "apply_patch command is constrained to writable paths", @@ -208,6 +221,7 @@ function canAutoApproveApplyPatch( */ function isWritePatchConstrainedToWritablePaths( applyPatchArg: string, + workdir: string | undefined, writableRoots: ReadonlyArray, ): boolean { // `identify_files_needed()` returns a list of files that will be modified or @@ -222,10 +236,12 @@ function isWritePatchConstrainedToWritablePaths( return ( allPathsConstrainedTowritablePaths( identify_files_needed(applyPatchArg), + workdir, writableRoots, ) && allPathsConstrainedTowritablePaths( identify_files_added(applyPatchArg), + workdir, writableRoots, ) ); @@ -233,24 +249,47 @@ function isWritePatchConstrainedToWritablePaths( function allPathsConstrainedTowritablePaths( candidatePaths: ReadonlyArray, + workdir: string | undefined, writableRoots: ReadonlyArray, ): boolean { return candidatePaths.every((candidatePath) => - isPathConstrainedTowritablePaths(candidatePath, writableRoots), + isPathConstrainedTowritablePaths(candidatePath, workdir, writableRoots), ); } /** If candidatePath is relative, it will be resolved against cwd. */ function isPathConstrainedTowritablePaths( candidatePath: string, + workdir: string | undefined, writableRoots: ReadonlyArray, ): boolean { - const candidateAbsolutePath = path.resolve(candidatePath); + const candidateAbsolutePath = resolvePathAgainstWorkdir( + candidatePath, + workdir, + ); + return writableRoots.some((writablePath) => pathContains(writablePath, candidateAbsolutePath), ); } +/** + * If not already an absolute path, resolves `candidatePath` against `workdir` + * if specified; otherwise, against `process.cwd()`. + */ +export function resolvePathAgainstWorkdir( + candidatePath: string, + workdir: string | undefined, +): string { + if (path.isAbsolute(candidatePath)) { + return candidatePath; + } else if (workdir != null) { + return path.resolve(workdir, candidatePath); + } else { + return path.resolve(candidatePath); + } +} + /** Both `parent` and `child` must be absolute paths. */ function pathContains(parent: string, child: string): boolean { const relative = path.relative(parent, child); diff --git a/codex-cli/src/utils/agent/exec.ts b/codex-cli/src/utils/agent/exec.ts index 25c6f86a..f0177979 100644 --- a/codex-cli/src/utils/agent/exec.ts +++ b/codex-cli/src/utils/agent/exec.ts @@ -10,6 +10,7 @@ import { formatCommandForDisplay } from "../../format-command.js"; import fs from "fs"; import os from "os"; import { parse } from "shell-quote"; +import { resolvePathAgainstWorkdir } from "src/approvals.js"; const DEFAULT_TIMEOUT_MS = 10_000; // 10 seconds @@ -60,16 +61,20 @@ export function exec( return execForSandbox(cmd, opts, writableRoots, abortSignal); } -export function execApplyPatch(patchText: string): ExecResult { +export function execApplyPatch( + patchText: string, + workdir: string | undefined, +): ExecResult { // This is a temporary measure to understand what are the common base commands // until we start persisting and uploading rollouts try { const result = process_patch( patchText, - (p) => fs.readFileSync(p, "utf8"), - (p, c) => fs.writeFileSync(p, c, "utf8"), - (p) => fs.unlinkSync(p), + (p) => fs.readFileSync(resolvePathAgainstWorkdir(p, workdir), "utf8"), + (p, c) => + fs.writeFileSync(resolvePathAgainstWorkdir(p, workdir), c, "utf8"), + (p) => fs.unlinkSync(resolvePathAgainstWorkdir(p, workdir)), ); return { stdout: result, diff --git a/codex-cli/src/utils/agent/handle-exec-command.ts b/codex-cli/src/utils/agent/handle-exec-command.ts index aea2c3a7..85d68691 100644 --- a/codex-cli/src/utils/agent/handle-exec-command.ts +++ b/codex-cli/src/utils/agent/handle-exec-command.ts @@ -81,7 +81,7 @@ export async function handleExecCommand( ) => Promise, abortSignal?: AbortSignal, ): Promise { - const { cmd: command } = args; + const { cmd: command, workdir } = args; const key = deriveCommandKey(command); @@ -103,7 +103,7 @@ export async function handleExecCommand( // working directory so that edits are constrained to the project root. If // the caller wishes to broaden or restrict the set it can be made // configurable in the future. - const safety = canAutoApprove(command, policy, [process.cwd()]); + const safety = canAutoApprove(command, workdir, policy, [process.cwd()]); let runInSandbox: boolean; switch (safety.type) { @@ -247,7 +247,7 @@ async function execCommand( const start = Date.now(); const execResult = applyPatchCommand != null - ? execApplyPatch(applyPatchCommand.patch) + ? execApplyPatch(applyPatchCommand.patch, workdir) : await exec( { ...execInput, additionalWritableRoots }, await getSandbox(runInSandbox), diff --git a/codex-cli/tests/approvals.test.ts b/codex-cli/tests/approvals.test.ts index a90abad6..94daacce 100644 --- a/codex-cli/tests/approvals.test.ts +++ b/codex-cli/tests/approvals.test.ts @@ -11,7 +11,13 @@ describe("canAutoApprove()", () => { const writeablePaths: Array = []; const check = (command: ReadonlyArray): SafetyAssessment => - canAutoApprove(command, "suggest", writeablePaths, env); + canAutoApprove( + command, + /* workdir */ undefined, + "suggest", + writeablePaths, + env, + ); test("simple safe commands", () => { expect(check(["ls"])).toEqual({