fix(cursor): confine MCP resource downloads to the workspace

`download_path` is workspace-relative by contract, but it arrives from
the server and `resolveToCwd` deliberately honors absolute paths, `~`,
and `..` - correct for a path a user typed, a write-anywhere primitive
for one a remote peer supplied. `/etc/cron.d/x` or `../../escape` would
have been written wherever the process can reach.

`confineToWorkspace` accepts only a non-empty relative path resolving
under the live cwd, and the download refuses anything else. The refusal
throws inside the dispatch's existing try, so it reaches the model as a
`ReadMcpResourceError` rather than a silent success or a crash.

(cherry picked from commit 963cfee21a56576033ec115db745bb18ab0a8d06)

packages/coding-agent/CHANGELOG.md

@@ -143,7 +143,7 @@
 - Fixed `pi_bash` killing commands that explicitly asked for no deadline. `timeout` is `optional int32` and `bash` documents `0` as "disables the command deadline", but a truthiness check folded a supplied `0` into unset, applying the 300s default instead. A present `0` now passes through; negatives, which have no local meaning and would otherwise clamp to the 1s floor, still fall back to the default.
 - Fixed the Cursor exec bridge granting `edit` and `grep` to sessions that withheld them. Both bridge-only tools are constructed rather than looked up, and `executeTool` prefers a constructed override over the registry, so a restricted tool set (`toolNames` without them, or `restrictToolNames`) still got a working `pi_edit`/`pi_grep` — native frames arrive regardless of the advertised catalog. Both are now gated on the session having actually granted the tool, matching the `delete` frame's existing check (issue #5680).
 - Fixed Cursor advisor bridge tools bypassing approval settings. The advisor's `pi_edit`/`pi_grep` instances are approval-wrapped, but the wrapper reads `tools.approvalMode`, per-tool `tools.approval.<tool>` policies and `autoApprove` only from the execute-time tool context — which the advisor bridge never supplied, so every native advisor frame resolved as `yolo` with empty policies and ran past a configured `ask` or `deny`. Advisors now receive the same context store as the primary bridge.
-- Fixed Cursor's `list_mcp_resources`/`read_mcp_resource` frames answering as though the client hosted no MCP servers. The bridge hardcoded an empty catalog and `not_found`, so resources from servers the session held live connections to were invisible to the model even while the same session read them through `mcp://`. Both frames now answer from the session's `MCPManager`; a lookup failure surfaces as an error rather than an empty catalog, which would read as "asked, none exist". A read carrying `download_path` writes the resource to that workspace-relative path and answers with the path alone, per the wire contract, instead of putting the payload back in the model's context.
+- Fixed Cursor's `list_mcp_resources`/`read_mcp_resource` frames answering as though the client hosted no MCP servers. The bridge hardcoded an empty catalog and `not_found`, so resources from servers the session held live connections to were invisible to the model even while the same session read them through `mcp://`. Both frames now answer from the session's `MCPManager`; a lookup failure surfaces as an error rather than an empty catalog, which would read as "asked, none exist". A read carrying `download_path` writes the resource to that path and answers with the path alone, per the wire contract, instead of putting the payload back in the model's context — confined to the workspace, since that path arrives from the server and the general-purpose resolver deliberately honors absolute paths and `..`.
 - Fixed the Cursor native `delete` frame bypassing approval settings. Unlike every other frame it removes the file directly instead of running a registry tool, so no approval wrapper sat in front of it — `allowNativeDelete` answers whether a mutating tool was granted, which is a different question from whether the user's policy allows the call. A configured `tools.approval.delete: deny`, or an `always-ask` session that this channel cannot prompt in, now refuses the frame and keeps the file.
 - Fixed `pi_ls` never reporting that a listing was clipped. The bridge read the entry cap from a flat `details.resultLimitReached`, which `glob` sets but `read` — the tool serving `pi_ls` — does not: it records the cap through `OutputMeta` at `details.meta.limits.resultLimit.reached`. Every capped listing therefore reached Cursor with `entry_limit_reached` unset, reading as complete. Both shapes are now checked, the same way the truncation translation already handles its two producers.
 

packages/coding-agent/src/cursor.ts

@@ -28,7 +28,7 @@ import { sanitizeText } from "@oh-my-pi/pi-utils";
 import type { MCPResourceReadResult } from "./mcp/types";
 import type { ApprovalMode } from "./tools/approval";
 import { resolveApproval } from "./tools/approval";
-import { resolveToCwd } from "./tools/path-utils";
+import { confineToWorkspace, resolveToCwd } from "./tools/path-utils";
 import type { TodoPhase, TodoStatus } from "./tools/todo";
 
 /** Phase used for Cursor-owned tasks with no local phase grouping. */
@@ -647,10 +647,17 @@ export class CursorExecHandlers implements ICursorExecHandlers {
 			const payload =
 				texts.length > 0 ? texts.join("\n") : blob !== undefined ? Buffer.from(blob, "base64") : undefined;
 			if (payload === undefined) return null;
-			const absolutePath = resolveToCwd(downloadPath, this.options.getCwd?.() ?? this.options.cwd);
+			// The path is workspace-relative BY CONTRACT, but it arrives from the
+			// server, and `resolveToCwd` deliberately honors absolute paths and
+			// `..` for user-authored tool input. Taking it at its word would let a
+			// frame write anywhere this process can reach, so confine it here
+			// rather than trusting the declaration.
+			const cwd = this.options.getCwd?.() ?? this.options.cwd;
+			const absolutePath = confineToWorkspace(downloadPath, cwd);
+			if (!absolutePath) throw new Error(`Refusing to download outside the workspace: ${downloadPath}`);
 			await Bun.write(absolutePath, payload);
-			// The path echoed back is the one the frame asked for: it is
-			// workspace-relative by contract, and the model addresses it that way.
+			// The path echoed back is the one the frame asked for; the model
+			// addresses it the same relative way.
 			return { uri, mimeType, downloadPath };
 		}
 

packages/coding-agent/src/tools/path-utils.ts

@@ -520,6 +520,29 @@ export function resolveToCwd(filePath: string, cwd: string): string {
 	return path.resolve(cwd, expanded);
 }
 
+/**
+ * Resolve a path that MUST stay inside `cwd`, or `null` when it would escape.
+ *
+ * {@link resolveToCwd} deliberately honors absolute paths, `~`, and `..` —
+ * correct for a path a user typed, wrong for one a remote peer supplied.
+ * Callers handling untrusted input (Cursor's `download_path`) use this instead:
+ * only a non-empty relative path resolving under the live cwd is accepted, so
+ * neither `/etc/passwd` nor `../../escape` can be written through.
+ *
+ * The cwd itself is rejected: a download names a file, never the directory.
+ */
+export function confineToWorkspace(filePath: string, cwd: string): string | null {
+	if (!filePath || path.isAbsolute(filePath)) return null;
+	// `~` expands to an absolute path, and an internal URL is not a filesystem
+	// target at all; neither is a relative workspace path.
+	if (filePath.startsWith("~") || isInternalUrlPath(filePath)) return null;
+	const root = path.resolve(cwd);
+	const resolved = path.resolve(root, filePath);
+	const relative = path.relative(root, resolved);
+	if (!relative || relative.startsWith("..") || path.isAbsolute(relative)) return null;
+	return resolved;
+}
+
 export function formatPathRelativeToCwd(
 	filePath: string,
 	cwd: string,

packages/coding-agent/test/cursor-exec.test.ts

@@ -730,6 +730,38 @@ describe("CursorExecHandlers mounted tool bridge", () => {
 		}
 	});
 
+	it("refuses a download path that escapes the workspace", async () => {
+		// `download_path` is workspace-relative by contract, but it comes from
+		// the server and the generic resolver honors absolute paths and `..` —
+		// correct for a path a user typed, a write-anywhere primitive for one a
+		// remote peer supplied.
+		const workspace = await fs.mkdtemp(path.join(os.tmpdir(), "cursor-mcp-escape-"));
+		const outside = path.join(workspace, "outside.txt");
+		try {
+			const inner = path.join(workspace, "ws");
+			await fs.mkdir(inner);
+			const handlers = new CursorExecHandlers({
+				cwd: inner,
+				tools: new Map(),
+				mcpResources: {
+					serverNames: () => ["files"],
+					getServerResources: () => undefined,
+					readServerResource: async (_name, uri) => ({ contents: [{ uri, text: "payload" }] }),
+				},
+			});
+
+			for (const escape of ["../outside.txt", outside, "nested/../../outside.txt"]) {
+				await expect(
+					handlers.readMcpResource({ server: "files", uri: "files://x", downloadPath: escape }),
+				).rejects.toThrow(/outside the workspace/);
+			}
+			// Nothing was written on any of those attempts.
+			expect(await Bun.file(outside).exists()).toBe(false);
+		} finally {
+			await removeWithRetries(workspace);
+		}
+	});
+
 	it("answers nothing when the session has no MCP manager", async () => {
 		// A host without MCP must still answer truthfully rather than throwing:
 		// an empty catalog and `not_found` are the honest responses.