fix(agent): guard apply_patch with secret-guard and realpath bound

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

@@ -1,12 +1,15 @@
-import { mkdtemp, readFile, writeFile, stat } from "node:fs/promises";
+import { mkdtemp, readFile, stat, symlink, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { expect, test } from "bun:test";
 
 import { createApplyPatchTool } from "./apply-patch-tool.js";
 
 async function run(cwd: string, input: string): Promise<string> {
-  const tool = createApplyPatchTool(cwd);
+  const tool = createApplyPatchTool(cwd, {
+    allowOutside: () => false,
+    rootsProvider: () => [],
+  });
   if (tool.kind !== "string") throw new Error("expected a string tool");
   return tool.handler({ input }, new AbortController().signal);
 }
@@ -65,3 +68,25 @@ test("rejects paths that escape the workspace", async () => {
   );
   expect(out).toContain("escapes the workspace");
 });
+
+test("refuses to write a secret file", async () => {
+  const cwd = await mkdtemp(join(tmpdir(), "apply-patch-"));
+  const out = await run(
+    cwd,
+    "*** Begin Patch\n*** Add File: .env\n+TOKEN=x\n*** End Patch",
+  );
+  expect(out).toContain("sensitive file blocked");
+  await expect(stat(join(cwd, ".env"))).rejects.toThrow();
+});
+
+test("refuses to follow a workspace symlink out of the workspace", async () => {
+  const cwd = await mkdtemp(join(tmpdir(), "apply-patch-"));
+  const outside = await mkdtemp(join(tmpdir(), "apply-patch-outside-"));
+  await symlink(outside, join(cwd, "link"));
+  const out = await run(
+    cwd,
+    "*** Begin Patch\n*** Add File: link/pwned.txt\n+x\n*** End Patch",
+  );
+  expect(out).toContain("escapes the workspace");
+  await expect(stat(join(outside, "pwned.txt"))).rejects.toThrow();
+});

src/agent/apply-patch-tool.ts

@@ -1,10 +1,16 @@
 import { mkdir, readFile, rm, writeFile } from "node:fs/promises";
-import { dirname, isAbsolute, relative, resolve } from "node:path";
+import { dirname, resolve } from "node:path";
 import { stringTool } from "@intx/agent";
 import type { AgentTool } from "@intx/agent";
 import type { ToolDefinition } from "@intx/types/runtime";
 import { type } from "arktype";
 
+import { resolveWorkspacePath } from "../permission/path-restriction.js";
+import type { RootsProvider } from "../permission/worktree-roots.js";
+import {
+  createExtraDeniedPathMatcher,
+  isSensitivePathResolved,
+} from "../plugins/secret-guard-plugin.js";
 import {
   applyUpdateHunks,
   CodexApplyPatchError,
@@ -31,28 +37,52 @@ type Planned =
   | { kind: "write"; path: string; content: string; label: string }
   | { kind: "remove"; path: string; label: string };
 
-function contained(cwd: string, path: string): string {
+export interface ApplyPatchGuard {
+  allowOutside: () => boolean;
+  rootsProvider: RootsProvider;
+  extraDeniedPaths?: readonly string[];
+}
+
+// apply_patch is mounted outside the posix plugin stack, so it re-enforces the
+// secret-guard denylist and the realpath workspace bound itself. A lexical
+// check alone would let a symlink inside the workspace lead out of it.
+function guardedPath(
+  cwd: string,
+  path: string,
+  guard: ApplyPatchGuard,
+  isExtraDenied: (value: string) => boolean,
+): string {
   const abs = resolve(cwd, path);
-  const rel = relative(cwd, abs);
-  if (rel === "" || rel.startsWith("..") || isAbsolute(rel)) {
+  if (isSensitivePathResolved(abs) || isExtraDenied(abs)) {
+    throw new CodexApplyPatchError(
+      `Access to sensitive file blocked by policy: ${path}`,
+    );
+  }
+  if (
+    !guard.allowOutside() &&
+    resolveWorkspacePath(cwd, abs, guard.rootsProvider) === undefined
+  ) {
     throw new CodexApplyPatchError(`path escapes the workspace: ${path}`);
   }
   return abs;
 }
 
-async function plan(cwd: string, op: PatchOp): Promise<Planned[]> {
+async function plan(
+  op: PatchOp,
+  contained: (path: string) => string,
+): Promise<Planned[]> {
   if (op.type === "add") {
-    const path = contained(cwd, op.path);
+    const path = contained(op.path);
     return [
       { kind: "write", path, content: op.content, label: `A ${op.path}` },
     ];
   }
   if (op.type === "delete") {
     return [
-      { kind: "remove", path: contained(cwd, op.path), label: `D ${op.path}` },
+      { kind: "remove", path: contained(op.path), label: `D ${op.path}` },
     ];
   }
-  const source = contained(cwd, op.path);
+  const source = contained(op.path);
   let original: string;
   try {
     original = await readFile(source, "utf8");
@@ -66,7 +96,7 @@ async function plan(cwd: string, op: PatchOp): Promise<Planned[]> {
   return [
     {
       kind: "write",
-      path: contained(cwd, op.moveTo),
+      path: contained(op.moveTo),
       content,
       label: `M ${op.moveTo}`,
     },
@@ -78,7 +108,15 @@ async function plan(cwd: string, op: PatchOp): Promise<Planned[]> {
  * Plans every op before touching disk so a bad hunk in the last file cannot
  * leave the earlier files half-patched.
  */
-export function createApplyPatchTool(cwd: string): AgentTool {
+export function createApplyPatchTool(
+  cwd: string,
+  guard: ApplyPatchGuard,
+): AgentTool {
+  const isExtraDenied = createExtraDeniedPathMatcher(
+    guard.extraDeniedPaths ?? [],
+  );
+  const contained = (path: string): string =>
+    guardedPath(cwd, path, guard, isExtraDenied);
   return stringTool({
     definition: applyPatchDefinition,
     handler: async (rawArgs: Record<string, unknown>): Promise<string> => {
@@ -89,7 +127,7 @@ export function createApplyPatchTool(cwd: string): AgentTool {
       try {
         const patch = parseCodexApplyPatch(parsedArgs.input);
         const steps: Planned[] = [];
-        for (const op of patch.ops) steps.push(...(await plan(cwd, op)));
+        for (const op of patch.ops) steps.push(...(await plan(op, contained)));
         for (const step of steps) {
           if (step.kind === "write") {
             await mkdir(dirname(step.path), { recursive: true });

src/agent/tools.ts

@@ -666,7 +666,13 @@ export async function createAgentToolset(
       rootsProvider: createWorktreeRootsProvider(cwd),
     }),
     createUseSkillTool(cwd, skillDirs, args.telemetry),
-    createApplyPatchTool(cwd),
+    createApplyPatchTool(cwd, {
+      allowOutside: () => permissionGate.getSkipPermissions(),
+      rootsProvider: createWorktreeRootsProvider(cwd),
+      ...(args.secretGuardExtraDeniedPaths !== undefined
+        ? { extraDeniedPaths: args.secretGuardExtraDeniedPaths }
+        : {}),
+    }),
     builtinExaEnabled
       ? createExaMCPWebFetchTool({ connect: waitForBuiltinExaConnection })
       : createWebFetchTool(),

src/subagent/run.ts

@@ -68,6 +68,7 @@ import {
 import { normalizeToolDefinitionsForProvider } from "../agent/tool-schema-normalize.js";
 import { canonicalToolName } from "../agent/canonical-tool-name.js";
 import { createApplyPatchTool } from "../agent/apply-patch-tool.js";
+import { createWorktreeRootsProvider } from "../permission/worktree-roots.js";
 import {
   checkMountedRequiresTools,
   formatCapabilityUnavailable,
@@ -811,7 +812,16 @@ async function runSubAgentInner(
         ),
       )
     ) {
-      tools = [...tools, createApplyPatchTool(params.cwd)];
+      tools = [
+        ...tools,
+        createApplyPatchTool(params.cwd, {
+          allowOutside: () => permissionGate.getSkipPermissions(),
+          rootsProvider: createWorktreeRootsProvider(params.cwd),
+          ...(params.secretGuardExtraDeniedPaths !== undefined
+            ? { extraDeniedPaths: params.secretGuardExtraDeniedPaths }
+            : {}),
+        }),
+      ];
     }
     hostCommandsMounted = tools.some(
       (tool) => canonicalToolName(tool.definition.name) === "run_shell",