From 05af550d815ac51acf0b52c3852d2a99f29613d2 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 16 Jul 2026 10:37:25 +0000 Subject: [PATCH] fix(cursor): gated native delete for read-only advisors CursorExecHandlers.executeDelete removes files directly via fs.rmSync, bypassing the tool map that every other exec handler consults. A background advisor with the default read-only set (advise/read/grep/glob) could delete workspace files from a Cursor deleteArgs frame despite holding no mutating tool. Add an allowNativeDelete option (default allowed, preserving the primary agent's behavior) and set it for the advisor only when it was granted a file-mutating tool (write/edit). Fixes #5680 --- packages/coding-agent/CHANGELOG.md | 2 +- packages/coding-agent/src/cursor.ts | 15 ++++++ .../coding-agent/src/session/agent-session.ts | 6 +++ .../coding-agent/test/cursor-exec.test.ts | 47 +++++++++++++++++++ 4 files changed, 69 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index fe743109b..56368dd06 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed the built-in advisor silently doing nothing when its model routes through the `cursor` provider: the advisor runs in its own `Agent` that was constructed without `cursorExecHandlers`, so on Cursor — where every tool executes server-side and is dispatched back through the client's exec handlers — each advisor tool call (including the MCP `advise` tool) came back `toolNotFound`/"tool not available" and no advice was ever routed. The advisor `Agent` now gets a Cursor exec bridge scoped to its own granted tool set, mirroring the primary agent ([#5680](https://github.com/can1357/oh-my-pi/issues/5680)). +- Fixed the built-in advisor silently doing nothing when its model routes through the `cursor` provider: the advisor runs in its own `Agent` that was constructed without `cursorExecHandlers`, so on Cursor — where every tool executes server-side and is dispatched back through the client's exec handlers — each advisor tool call (including the MCP `advise` tool) came back `toolNotFound`/"tool not available" and no advice was ever routed. The advisor `Agent` now gets a Cursor exec bridge scoped to its own granted tool set, mirroring the primary agent. The bridge's native `delete` frame is gated so a read-only advisor cannot delete workspace files it was never granted a mutating tool for ([#5680](https://github.com/can1357/oh-my-pi/issues/5680)). ## [17.0.1] - 2026-07-16 diff --git a/packages/coding-agent/src/cursor.ts b/packages/coding-agent/src/cursor.ts index 4bc29fb0c..4890cd3f4 100644 --- a/packages/coding-agent/src/cursor.ts +++ b/packages/coding-agent/src/cursor.ts @@ -21,6 +21,15 @@ interface CursorExecBridgeOptions { tools: Map; getToolContext?: () => AgentToolContext | undefined; emitEvent?: (event: AgentEvent) => void; + /** + * Whether the Cursor native `delete` frame may remove files. Unlike every + * other exec handler, `executeDelete` mutates the filesystem directly instead + * of consulting {@link tools}, so a background read-only advisor could delete + * workspace files it was never granted a mutating tool for (issue #5680 + * review). Defaults to allowed to preserve the primary agent's behavior; + * callers with a restricted tool set (advisors) opt out. + */ + allowNativeDelete?: boolean; } function createToolResultMessage( @@ -106,6 +115,12 @@ async function executeTool( async function executeDelete(options: CursorExecBridgeOptions, pathArg: string, toolCallId: string) { const toolName = "delete"; + + if (options.allowNativeDelete === false) { + const result = buildToolErrorResult(`Tool "${toolName}" not available`); + return createToolResultMessage(toolCallId, toolName, result, true); + } + options.emitEvent?.({ type: "tool_execution_start", toolCallId, toolName, args: { path: pathArg } }); const absolutePath = resolveToCwd(pathArg, options.cwd); diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 047c0f802..833b95cc6 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -2976,9 +2976,15 @@ export class AgentSession { // own tools (including the MCP `advise` tool) return `toolNotFound` and // no advice is ever routed (issue #5680). Mirrors the primary agent's // bridge (`sdk.ts`), scoped to this advisor's granted tool set. + // Cursor's native `delete` frame removes files directly, bypassing the + // tool map, so gate it on the advisor actually holding a file-mutating + // tool. A default read-only advisor (advise/read/grep/glob) never gets + // to delete workspace files it was never granted (issue #5680 review). + const advisorCanMutateFiles = advisorToolMap.has("write") || advisorToolMap.has("edit"); const advisorCursorExecHandlers = new CursorExecHandlers({ cwd: this.sessionManager.getCwd(), tools: advisorToolMap, + allowNativeDelete: advisorCanMutateFiles, }); const advisorAgent = new Agent({ initialState: { diff --git a/packages/coding-agent/test/cursor-exec.test.ts b/packages/coding-agent/test/cursor-exec.test.ts index 345bb77bb..791d8bedf 100644 --- a/packages/coding-agent/test/cursor-exec.test.ts +++ b/packages/coding-agent/test/cursor-exec.test.ts @@ -10,6 +10,7 @@ import { AssistantMessageEventStream } from "@oh-my-pi/pi-ai/utils/event-stream" import { AgentClientMessageSchema, AgentServerMessageSchema, + DeleteArgsSchema, ExecServerMessageSchema, McpArgsSchema, ReadArgsSchema, @@ -284,3 +285,49 @@ describe("CursorExecHandlers advise routing (issue #5680)", () => { expect(decodeMcpResultCase(written[0])).toBe("toolNotFound"); }); }); + +// Regression for the #5686 review: Cursor's native `delete` frame removes files +// directly (bypassing the tool map), so a read-only advisor that was granted no +// mutating tool must not be able to delete workspace files. +describe("CursorExecHandlers native delete gating (issue #5680)", () => { + let cwd: string; + + beforeEach(async () => { + cwd = await fs.mkdtemp(path.join(os.tmpdir(), "cursor-delete-test-")); + }); + + afterEach(async () => { + await removeWithRetries(cwd); + }); + + it("rejects native delete and preserves the file when allowNativeDelete is false", async () => { + const target = path.join(cwd, "victim.txt"); + await Bun.write(target, "keep me"); + const handlers = new CursorExecHandlers({ + cwd, + tools: new Map(), + allowNativeDelete: false, + }); + + const result = await handlers.delete(create(DeleteArgsSchema, { toolCallId: "call-del", path: target })); + + expect(result.isError).toBe(true); + expect(result.content).toEqual([{ type: "text", text: 'Tool "delete" not available' }]); + expect(await Bun.file(target).exists()).toBe(true); + }); + + it("performs native delete when allowNativeDelete is true", async () => { + const target = path.join(cwd, "victim.txt"); + await Bun.write(target, "remove me"); + const handlers = new CursorExecHandlers({ + cwd, + tools: new Map(), + allowNativeDelete: true, + }); + + const result = await handlers.delete(create(DeleteArgsSchema, { toolCallId: "call-del", path: target })); + + expect(result.isError).toBe(false); + expect(await Bun.file(target).exists()).toBe(false); + }); +});