From 012ad7e69ebcf2d46a4a6ee17f6bac8cc492f624 Mon Sep 17 00:00:00 2001
From: Michael Bolin <mbolin@openai.com>
Date: Tue, 22 Apr 2025 12:56:07 -0700
Subject: [PATCH] when a shell tool call invokes apply_patch, resolve relative
 paths against workdir, if specified

---
 codex-cli/src/approvals.ts                    | 36 ++++++++++++++++---
 .../src/utils/agent/handle-exec-command.ts    |  4 +--
 codex-cli/tests/approvals.test.ts             |  8 ++++-
 3 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/codex-cli/src/approvals.ts b/codex-cli/src/approvals.ts
index ff37a8903f..e962d548df 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<string>,
+  workdir: string | undefined,
   policy: ApprovalPolicy,
   writableRoots: ReadonlyArray<string>,
   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<string>,
   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<string>,
 ): 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,19 +249,29 @@ function isWritePatchConstrainedToWritablePaths(
 
 function allPathsConstrainedTowritablePaths(
   candidatePaths: ReadonlyArray<string>,
+  workdir: string | undefined,
   writableRoots: ReadonlyArray<string>,
 ): 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<string>,
 ): boolean {
-  const candidateAbsolutePath = path.resolve(candidatePath);
+  let candidateAbsolutePath: string;
+  if (path.isAbsolute(candidatePath)) {
+    candidateAbsolutePath = candidatePath;
+  } else if (workdir != null) {
+    candidateAbsolutePath = path.resolve(workdir, candidatePath);
+  } else {
+    candidateAbsolutePath = path.resolve(candidatePath);
+  }
+
   return writableRoots.some((writablePath) =>
     pathContains(writablePath, candidateAbsolutePath),
   );
diff --git a/codex-cli/src/utils/agent/handle-exec-command.ts b/codex-cli/src/utils/agent/handle-exec-command.ts
index aea2c3a707..1932ab9840 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<CommandConfirmation>,
   abortSignal?: AbortSignal,
 ): Promise<HandleExecCommandResult> {
-  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) {
diff --git a/codex-cli/tests/approvals.test.ts b/codex-cli/tests/approvals.test.ts
index a90abad6eb..94daacce00 100644
--- a/codex-cli/tests/approvals.test.ts
+++ b/codex-cli/tests/approvals.test.ts
@@ -11,7 +11,13 @@ describe("canAutoApprove()", () => {
 
   const writeablePaths: Array<string> = [];
   const check = (command: ReadonlyArray<string>): SafetyAssessment =>
-    canAutoApprove(command, "suggest", writeablePaths, env);
+    canAutoApprove(
+      command,
+      /* workdir */ undefined,
+      "suggest",
+      writeablePaths,
+      env,
+    );
 
   test("simple safe commands", () => {
     expect(check(["ls"])).toEqual({