fix(apply_patch): enforce workspace containment and secret-guard on raw reads

readRawFile bypassed every ToolPlugin (pathEscapePlugin, secretGuardPlugin,
authzPlugin, permissionPlugin) since apply_patch's applyOp calls it outside
the posixTools middleware chain entirely. requireRelativePath only rejects
absolute paths, not `../` traversal, so an Update File op naming a path like
../../.env with an insertion-only hunk (no context match required) could
read a secret and hand it to write_file via Move to — exfiltration, not just
an unauthorized read.

readRawFile now reuses the same containment and secret-file authorities the
plugin chain already uses instead of reimplementing them: resolveWorkspacePath
(symlink-aware realpath containment, from path-restriction.ts) and
isSensitivePath (the secret-guard denylist). Containment honors the same
skipPermissions/yolo escape hatch pathEscapePlugin does; the secret check
never bypasses, matching secretGuardPlugin's own unconditional behavior. Both
are hard failures with a clear error, not a silent fallthrough.

Added regression tests for the attack shapes: ../ traversal out of the
workspace, a symlinked directory leading outside the workspace, a
secret-guard path (.env) under skipPermissions, and ../ traversal to a
secret file — all via the insertion-only hunk shape that made the
unauthorized read exploitable.

Author: TheGreatAxios

src/agent/apply-patch-diff.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from "bun:test";
-import { mkdtemp, rm, writeFile } from "node:fs/promises";
+import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { createPosixTools } from "@intx/tools-posix";
@@ -24,11 +24,14 @@ async function invokeApplyPatch(tools: AgentTool[], input: string) {
   );
 }
 
-async function makeApplyPatch(cwd: string): Promise<AgentTool[]> {
+async function makeApplyPatch(
+  cwd: string,
+  options: { skipPermissions?: boolean } = {},
+): Promise<AgentTool[]> {
   const gate = createPermissionGate({
     approvals: [],
     interactive: false,
-    skipPermissions: true,
+    skipPermissions: options.skipPermissions ?? true,
     auto: false,
     cwd,
   });
@@ -51,7 +54,7 @@ async function makeApplyPatch(cwd: string): Promise<AgentTool[]> {
   return createCodexToolProxies({
     isCodex: true,
     runTool,
-    readRawFile: createCodexReadRawFile(cwd),
+    readRawFile: createCodexReadRawFile(cwd, gate),
     runManageTasks: async () => ({ content: "ok" }),
   });
 }
@@ -148,3 +151,98 @@ describe("apply_patch Update File matches raw content, not read_file's numbered
     }
   });
 });
+
+describe("apply_patch Update File refuses reads outside the sanctioned workspace (CL-6966 follow-up)", () => {
+  // Insertion-only hunk (no context to match): the shape that makes an
+  // unauthorized raw read exploitable rather than merely wrong, since it
+  // requires no content match to "succeed" and hands the read content
+  // straight to write_file via Move to.
+  const insertionOnlyMoveInput = (path: string, moveTo: string) =>
+    [
+      "*** Begin Patch",
+      `*** Update File: ${path}`,
+      `*** Move to: ${moveTo}`,
+      "@@",
+      "+",
+      "*** End Patch",
+    ].join("\n");
+
+  test("../ traversal out of the workspace is refused", async () => {
+    const parent = await mkdtemp(join(tmpdir(), "apply-patch-cl6966-parent-"));
+    const cwd = join(parent, "workspace");
+    await mkdir(cwd);
+    try {
+      await writeFile(join(parent, "victim.txt"), "outside secret\n");
+      const tools = await makeApplyPatch(cwd, { skipPermissions: false });
+
+      const result = await invokeApplyPatch(
+        tools,
+        insertionOnlyMoveInput("../victim.txt", "leaked.txt"),
+      );
+
+      expect(result.isError).toBe(true);
+      expect(String(result.content)).toMatch(/escapes working directory/i);
+    } finally {
+      await rm(parent, { recursive: true, force: true });
+    }
+  });
+
+  test("a symlinked directory leading outside the workspace is refused", async () => {
+    const parent = await mkdtemp(join(tmpdir(), "apply-patch-cl6966-symlink-"));
+    const cwd = join(parent, "workspace");
+    const outside = join(parent, "outside");
+    await mkdir(cwd);
+    await mkdir(outside);
+    try {
+      await writeFile(join(outside, "victim.txt"), "outside secret\n");
+      await symlink(outside, join(cwd, "escape-link"));
+      const tools = await makeApplyPatch(cwd, { skipPermissions: false });
+
+      const result = await invokeApplyPatch(
+        tools,
+        insertionOnlyMoveInput("escape-link/victim.txt", "leaked.txt"),
+      );
+
+      expect(result.isError).toBe(true);
+      expect(String(result.content)).toMatch(/escapes working directory/i);
+    } finally {
+      await rm(parent, { recursive: true, force: true });
+    }
+  });
+
+  test("a secret-guard path (.env) is refused even with skipPermissions", async () => {
+    const cwd = await mkdtemp(join(tmpdir(), "apply-patch-cl6966-secret-"));
+    try {
+      await writeFile(join(cwd, ".env"), "API_KEY=super-secret\n");
+      // skipPermissions: true (yolo) — secret-guard has no bypass, unlike containment.
+      const tools = await makeApplyPatch(cwd, { skipPermissions: true });
+
+      const result = await invokeApplyPatch(tools, insertionOnlyMoveInput(".env", "leaked.txt"));
+
+      expect(result.isError).toBe(true);
+      expect(String(result.content)).toMatch(/sensitive file/i);
+      // The secret must never have reached the workspace under a new name.
+      expect(await Bun.file(join(cwd, "leaked.txt")).exists()).toBe(false);
+    } finally {
+      await rm(cwd, { recursive: true, force: true });
+    }
+  });
+
+  test("../ traversal to a secret file is refused (secret-guard applies to relative paths too)", async () => {
+    const parent = await mkdtemp(join(tmpdir(), "apply-patch-cl6966-secret-parent-"));
+    const cwd = join(parent, "workspace");
+    await mkdir(cwd);
+    try {
+      await writeFile(join(parent, ".env"), "API_KEY=super-secret\n");
+      const tools = await makeApplyPatch(cwd, { skipPermissions: false });
+
+      const result = await invokeApplyPatch(tools, insertionOnlyMoveInput("../.env", "leaked.txt"));
+
+      expect(result.isError).toBe(true);
+      expect(String(result.content)).toMatch(/sensitive file|escapes working directory/i);
+      expect(await Bun.file(join(cwd, "leaked.txt")).exists()).toBe(false);
+    } finally {
+      await rm(parent, { recursive: true, force: true });
+    }
+  });
+});

src/agent/codex-read-raw-file.ts

@@ -1,18 +1,75 @@
 /**
  * Raw (non-`cat -n`) file reads for apply_patch's Update File leg (CL-6966).
- * Mirrors the error mapping in @intx/tools-posix's read-file.js so failures
- * read the same way `read_file` would report them.
+ *
+ * `applyOp` calls this directly from outside the posixTools middleware chain
+ * — no ToolPlugin ever sees `op.path` here, unlike the write leg which still
+ * goes through the full pathEscapePlugin / secretGuardPlugin / authzPlugin /
+ * permissionPlugin stack (see buildCorePosixToolPlugins in
+ * posix-tool-plugins.ts). `requireRelativePath` in codex-apply-patch.ts only
+ * rejects absolute paths — it does nothing about `../` traversal — so this
+ * reader must apply the same containment and secret-file checks itself, or
+ * an Update File op naming e.g. `../../.env` (with a no-op insertion hunk,
+ * which requires no context match) can read a secret and hand it straight to
+ * write_file as an exfiltration primitive.
+ *
+ * Reuses the existing containment and secret-file authorities rather than
+ * reimplementing them: `resolveWorkspacePath` (symlink-aware realpath
+ * containment, the same function pathEscapePlugin calls) and `isSensitivePath`
+ * (the secretGuardPlugin denylist). Both are hard denials — the latter has no
+ * yolo/allowOutside bypass, matching secretGuardPlugin's own unconditional
+ * behavior.
  */
 
 import { readFile } from "node:fs/promises";
 import { resolve } from "node:path";
 import { hasCode } from "@intx/types";
+import { resolveWorkspacePath } from "../permission/path-restriction.js";
+import { createWorktreeRootsProvider } from "../permission/worktree-roots.js";
+import { isSensitivePath } from "../plugins/secret-guard-plugin.js";
+import type { PermissionGate } from "../permission/gate.js";
 import type { CodexReadRawFile } from "./codex-tool-proxies.js";
 
-/** `path` is a workspace-relative path (apply_patch rejects absolute paths). */
-export function createCodexReadRawFile(cwd: string): CodexReadRawFile {
+/** `path` is workspace-relative (apply_patch's parser rejects absolute paths). */
+export function createCodexReadRawFile(
+  cwd: string,
+  permissionGate: PermissionGate,
+): CodexReadRawFile {
+  const rootsProvider = createWorktreeRootsProvider(cwd);
+  const allowOutside = (): boolean => permissionGate.getSkipPermissions();
+
   return async (path) => {
-    const absolutePath = resolve(cwd, path);
+    // Secret-file denylist first, on the raw path — this must hold regardless
+    // of containment or yolo, exactly like secretGuardPlugin's hard deny.
+    if (isSensitivePath(path)) {
+      return {
+        content: `Access to sensitive file blocked by policy: ${path}`,
+        isError: true,
+      };
+    }
+
+    const resolved = resolveWorkspacePath(cwd, path, rootsProvider);
+    let absolutePath: string;
+    if (resolved !== undefined) {
+      absolutePath = resolved;
+    } else if (allowOutside()) {
+      // Mirrors pathEscapePlugin's own allowOutside fallback (yolo mode).
+      absolutePath = resolve(cwd, path);
+    } else {
+      return {
+        content: `Path escapes working directory: ${path}`,
+        isError: true,
+      };
+    }
+
+    // Re-check the resolved, symlink-realpath'd form too: a symlink can name
+    // something innocuous while pointing at a sensitive real path.
+    if (isSensitivePath(absolutePath)) {
+      return {
+        content: `Access to sensitive file blocked by policy: ${path}`,
+        isError: true,
+      };
+    }
+
     try {
       const buf = await readFile(absolutePath);
       if (buf.includes(0)) {

src/agent/tools.ts

@@ -388,7 +388,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
     ...createCodexToolProxies({
       isCodex: args.isCodex === true,
       runTool,
-      readRawFile: createCodexReadRawFile(cwd),
+      readRawFile: createCodexReadRawFile(cwd, permissionGate),
       runManageTasks,
     }),
   );

src/subagent/run.ts

@@ -377,7 +377,7 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<string> {
       ...createCodexToolProxies({
         isCodex: isCodexProviderName(params.provider.providerName),
         runTool,
-        readRawFile: createCodexReadRawFile(params.cwd),
+        readRawFile: createCodexReadRawFile(params.cwd, permissionGate),
         runManageTasks,
         allowDelete: allowDeleteFromCapabilities(params.capabilities),
         allowShell: allowShellFromCapabilities(params.capabilities),