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
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -21,6 +21,15 @@ interface CursorExecBridgeOptions {
|
||||
tools: Map<string, AgentTool>;
|
||||
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);
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user