fix(cursor): gate resource downloads, fix pi_grep cap and MCP transcript
Download-mode resource reads created and overwrote workspace files without running a registry tool - the same hole the native `delete` frame had - so a session that withheld `write`/`edit`, or whose `write` tier is `deny`/`always-ask`, still had files written. Both frames now share one grant and one policy check, and the download refuses before the read so a blocked call never fetches the resource. `allowNativeDelete` is renamed `allowDirectFileMutation`: it now gates more than deletion. The primary session derives it from the registry BEFORE its own rewriting (Cursor moves `edit` out of the tool map and `write` may be auto-registered later, so reading the map at bridge construction would misjudge both) and unconditionally, since the bridge is installed for every session and one that starts on another provider can switch to Cursor later. `pi_grep` with a match cap: the local tool windows to 20 files and suggests `skip`, which `PiGrepExecArgs` cannot express - 100 matches requested over 25 one-match files returned 20, with the cap reported unreached. A capped search now reads cap+1 files, so a result landing exactly on the cap is distinguishable from a clipped one, and `match_limit_reached` is truthful either way. `read_mcp_resource` synthesized no transcript block and paired no result, so a read - including a download that mutates the workspace - was invisible in the UI and stripped from every rebuilt history. It now synthesizes a `read_mcp_resource` block (not `read`: the name drives rendering and prune semantics) and pairs success, not-found and error. (cherry picked from commit 5ff27a3efe8bec522d9d5dbd7763055eb03eae3b)
This commit is contained in:
Diogo Soares Rodrigues
committed by
can1357
can1357
parent
821fe75f5d
commit
59434149d1
@@ -68,6 +68,7 @@
|
||||
- Fixed Cursor `connect_scm` calls losing their repository and settling on a fabricated verdict. The target rides in the `ConnectScmArgs.target` oneof, so reading a flat `github` property always saw `undefined`; and the authoritative `success`/`error`/`rejected` result only arrives on the completion frame, so answering at the announcement persisted a fixed failure for every call — including the ones the server went on to accept. The block now opens on the start frame and settles from the completion's decoded result.
|
||||
- Fixed interleaved Cursor tool calls corrupting each other. The stream decoder tracked a single "current" block and settled it on any `toolCallCompleted`, ignoring the envelope's `call_id`: a completion for one call closed whichever block happened to be open and paired it with the wrong result, and `start A, start B` orphaned A entirely so its own completion settled B while A was never paired — which strips the whole interaction from every rebuilt transcript. Open blocks are now retained per envelope `call_id`, and end-of-stream closes all of them rather than only the last.
|
||||
- Fixed a Cursor `search_conversations` call leaving no transcript block. The frame is answered from a fixed verdict, so nothing downstream pairs a result for it, and an unpaired call takes its whole interaction out of every rebuilt transcript.
|
||||
- Fixed a Cursor `read_mcp_resource` call leaving no transcript block. The frame runs locally — and in download mode writes a workspace file — but synthesized no tool call and paired no result, so the read was invisible in the UI and absent from every rebuilt history; a resource download could mutate the workspace with nothing on record. The frame now synthesizes a `read_mcp_resource` block (not `read`: it is a remote MCP operation, and the name drives rendering and prune semantics) and pairs a result on success, not-found and error alike. Frames answered without a handler still synthesize nothing, since nothing ran.
|
||||
- Fixed the Cursor stream's end-of-transport cleanup erasing the arguments of every block still open. Blocks whose args arrive whole (todo, connect-SCM, MCP) never feed the streamed partial-JSON buffer, and reparsing an absent buffer yields `{}`, so a truncated or disconnected turn rebuilt those calls with no arguments at all. Only blocks that actually streamed their args are reparsed now.
|
||||
- Fixed a Cursor stream dying mid-turn stranding the call it left open. `connect_scm` and native todo blocks are stamped resolved the moment they open, so the agent loop synthesizes no placeholder and only their completion frame pairs a result — a transport that closed first left the card animating and the call unpaired, which takes the whole interaction out of every rebuilt transcript. The terminal-error path now closes open blocks and pairs those server-owned calls with an interrupted result; the flush ran only on clean completion before, which is not the path a dying stream takes. Exec-settled MCP blocks are left alone, since the dispatch that ran them owns their result.
|
||||
- Fixed the Pi exec frames displaying a different operation than the one they run. The provider synthesized its transcript block from a second, hand-rolled translation of the frame args, so `pi_read`'s `offset`/`limit` were shown as a whole-file read, `pi_grep`'s `literal` pattern as an unescaped regex, and `pi_find`'s path/glob join differed from the executed one. Both sides now share a single translation.
|
||||
|
||||
@@ -1580,6 +1580,19 @@ async function handleExecServerMessage(
|
||||
case "readMcpResourceExecArgs": {
|
||||
const args = execMsg.message.value;
|
||||
let execResult: ReadMcpResourceExecResult;
|
||||
// The read runs locally, and in download mode it writes a workspace
|
||||
// file — an operation with no transcript block is invisible in the UI
|
||||
// and absent from every rebuilt history. Only synthesized when a
|
||||
// handler exists: without one the frame is a fixed `not_found` that
|
||||
// executed nothing, and a block would claim work that never happened.
|
||||
const toolCallId = execHandlers?.readMcpResource ? crypto.randomUUID() : undefined;
|
||||
if (toolCallId) {
|
||||
synthesizeCursorExecToolCall(output, stream, state, toolCallId, "read_mcp_resource", {
|
||||
server: args.server,
|
||||
uri: args.uri,
|
||||
download_path: args.downloadPath,
|
||||
});
|
||||
}
|
||||
try {
|
||||
// `null` is the handler's "no such server or uri", which is exactly
|
||||
// `not_found`; a throw is a real failure and must not masquerade as
|
||||
@@ -1628,6 +1641,27 @@ async function handleExecServerMessage(
|
||||
},
|
||||
});
|
||||
}
|
||||
if (toolCallId) {
|
||||
// Derived from the answer that actually goes on the wire, so no exit
|
||||
// can drift out of sync with what the model was told.
|
||||
const settled = execResult.result;
|
||||
const text =
|
||||
settled.case === "success"
|
||||
? settled.value.downloadPath
|
||||
? `Downloaded ${args.uri} to ${settled.value.downloadPath}`
|
||||
: `Read ${args.uri}`
|
||||
: settled.case === "notFound"
|
||||
? `No such resource: ${args.uri}`
|
||||
: (settled.value?.error ?? `Failed to read ${args.uri}`);
|
||||
await pairSynthesizedExecResult(
|
||||
state,
|
||||
onToolResult,
|
||||
toolCallId,
|
||||
"read_mcp_resource",
|
||||
text,
|
||||
settled.case !== "success",
|
||||
);
|
||||
}
|
||||
sendExecClientMessage(h2Request, execMsg, "readMcpResourceExecResult", execResult);
|
||||
return;
|
||||
}
|
||||
@@ -3405,6 +3439,10 @@ export function synthesizeCursorExecToolCall(
|
||||
* {@link kCursorExecResolved}, so `agent-loop.ts` emits no placeholder for it
|
||||
* and `buildSessionContext` strips an unpaired call, taking the whole
|
||||
* interaction out of every rebuilt transcript.
|
||||
*
|
||||
* `isError` defaults true because most such verdicts are refusals; the MCP
|
||||
* resource frames run locally and can genuinely succeed, and a success filed
|
||||
* as an error would render as a failed call in every rebuilt transcript.
|
||||
*/
|
||||
async function pairSynthesizedExecResult(
|
||||
state: BlockState,
|
||||
@@ -3412,13 +3450,14 @@ async function pairSynthesizedExecResult(
|
||||
toolCallId: string,
|
||||
toolName: string,
|
||||
text: string,
|
||||
isError = true,
|
||||
): Promise<void> {
|
||||
const synthesized: ToolResultMessage = {
|
||||
role: "toolResult",
|
||||
toolCallId,
|
||||
toolName,
|
||||
content: [{ type: "text", text }],
|
||||
isError: true,
|
||||
isError,
|
||||
timestamp: Date.now(),
|
||||
};
|
||||
const sink = onToolResult ?? state.onToolResult;
|
||||
|
||||
@@ -649,6 +649,84 @@ describe("Cursor MCP resource frames answer from the host's servers", () => {
|
||||
expect(brokenAnswer.value.result.value.error).toContain("server disconnected");
|
||||
});
|
||||
|
||||
it("records a resource read as a paired transcript block", async () => {
|
||||
// The read runs locally and, in download mode, writes a workspace file.
|
||||
// An exec frame with no synthesized block is invisible in the UI, and an
|
||||
// unpaired call is stripped by `buildSessionContext` — taking the whole
|
||||
// interaction out of every rebuilt transcript.
|
||||
const { output, results } = await dispatchExec(
|
||||
buildExecMessage({
|
||||
case: "readMcpResourceExecArgs",
|
||||
value: create(ReadMcpResourceExecArgsSchema, {
|
||||
server: "docs",
|
||||
uri: "docs://readme",
|
||||
downloadPath: "assets/readme.md",
|
||||
}),
|
||||
}),
|
||||
{
|
||||
execHandlers: {
|
||||
readMcpResource: async ({ uri, downloadPath }) => ({ uri, mimeType: "text/markdown", downloadPath }),
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
const blocks = output.content.filter((block): block is ToolCallState => block.type === "toolCall");
|
||||
expect(blocks).toHaveLength(1);
|
||||
// Not `read`: this is a remote MCP operation, and the name drives
|
||||
// rendering and prune semantics.
|
||||
expect(blocks[0].name).toBe("read_mcp_resource");
|
||||
expect(blocks[0].arguments).toEqual({
|
||||
server: "docs",
|
||||
uri: "docs://readme",
|
||||
download_path: "assets/readme.md",
|
||||
});
|
||||
// Paired under the same id, and a success is not filed as a failure.
|
||||
expect(results.map(r => r.toolCallId)).toEqual([blocks[0].id]);
|
||||
expect(results[0].isError).toBe(false);
|
||||
expect(results[0].content).toEqual([{ type: "text", text: "Downloaded docs://readme to assets/readme.md" }]);
|
||||
});
|
||||
|
||||
it("pairs a failed resource read as an error, still under one block", async () => {
|
||||
// A refused download (no write grant, a path outside the workspace) and
|
||||
// a dead server both land here. The block must resolve — an unpaired
|
||||
// call strips the interaction — and must resolve as an error, or the
|
||||
// transcript shows a read that never happened as having succeeded.
|
||||
const { output, results } = await dispatchExec(
|
||||
buildExecMessage({
|
||||
case: "readMcpResourceExecArgs",
|
||||
value: create(ReadMcpResourceExecArgsSchema, { server: "docs", uri: "docs://readme" }),
|
||||
}),
|
||||
{
|
||||
execHandlers: {
|
||||
readMcpResource: async () => {
|
||||
throw new Error("Refusing to download outside the workspace: ../escape");
|
||||
},
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
const blocks = output.content.filter((block): block is ToolCallState => block.type === "toolCall");
|
||||
expect(blocks).toHaveLength(1);
|
||||
expect(results.map(r => r.toolCallId)).toEqual([blocks[0].id]);
|
||||
expect(results[0].isError).toBe(true);
|
||||
expect(results[0].content).toEqual([
|
||||
{ type: "text", text: "Refusing to download outside the workspace: ../escape" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("leaves no block when no handler ran", async () => {
|
||||
// Without a handler the frame is answered from a fixed `not_found` and
|
||||
// nothing executed, so a block would claim work that never happened.
|
||||
const { output, results } = await dispatchExec(
|
||||
buildExecMessage({
|
||||
case: "readMcpResourceExecArgs",
|
||||
value: create(ReadMcpResourceExecArgsSchema, { server: "docs", uri: "docs://readme" }),
|
||||
}),
|
||||
);
|
||||
expect(output.content.filter(block => block.type === "toolCall")).toHaveLength(0);
|
||||
expect(results).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("reports a failing list as an error, not an empty catalog", async () => {
|
||||
// An empty success says "asked, none exist" — a lie when the lookup
|
||||
// failed, and one the model cannot retry.
|
||||
|
||||
@@ -144,10 +144,12 @@
|
||||
- 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` — awaiting a server's background resource discovery rather than reading the not-yet-populated cache and reporting "advertises nothing" — and 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. That path arrives from the server while the general-purpose resolver deliberately honors absolute paths and `..`, so downloads are confined to the workspace: the resolved target and its deepest existing ancestor must stay inside it, and a target that is itself a symlink is refused. The write then opens `O_NOFOLLOW` and refuses a non-regular or hard-linked file before truncating, so the final component cannot be swapped for a link or an inode shared outside after the check. A parent directory replaced by a symlink mid-write is still followed; closing that needs `openat`/dirfd walking, which this does not attempt.
|
||||
- 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 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 — the bridge's `allowDirectFileMutation` grant 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 Cursor download-mode resource reads bypassing the session's mutation restrictions. A `read_mcp_resource` frame carrying `download_path` creates and overwrites workspace files without running a registry tool — the same hole the native `delete` frame had — so a session that withheld `write`/`edit`, or one whose `write` tier is `deny`/`always-ask`, still had files written. Both frames now share one grant (`allowDirectFileMutation`, renamed from `allowNativeDelete` now that it gates more than deletion) and one `write`-tier policy check, and the download refuses before the read so a blocked call does not fetch the resource either. The primary session derives that grant before it rewrites its registry: Cursor moves `edit` out of the tool map and `write` may be auto-registered later, so reading the map at bridge-construction time would have misjudged both.
|
||||
- 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.
|
||||
- Fixed a mixed-content MCP resource read reaching Cursor mislabelled. The mime type was taken from the first content item while the payload came from whichever item supplied it, so an image blob followed by a text note sent the text as `image/png`. Each branch now reports the type of the part it actually sends.
|
||||
- Fixed `pi_read`'s `offset`/`limit` returning more lines than the frame asked for. The range is composed onto the local `read` tool's inline selector, and a plain `:N+K` deliberately pads with one leading and three trailing context lines — helpful when a human reads a snippet, wrong for a caller that named an exact range: offset 5/limit 20 handed Cursor lines 4-27. Ranged Pi reads now compose `:raw:N+K`, which slices exactly the requested lines.
|
||||
- Fixed `pi_grep` returning fewer matches than it asked for when they spread across many files. The local `grep` windows results to the first 20 files and tells the caller to paginate with `skip`, but `PiGrepExecArgs` has no `skip` field — so a frame asking for 100 matches over 25 one-match files got 20, `match_limit_reached` unset, and advice it could not act on: output silently short and labelled complete. A search carrying a total match cap now reads enough files to satisfy it (cap+1, so a result landing exactly on the cap is distinguishable from a clipped one) and reports the cap when it actually bites.
|
||||
|
||||
## [17.1.5] - 2026-07-27
|
||||
|
||||
|
||||
@@ -51,14 +51,22 @@ interface CursorExecBridgeOptions {
|
||||
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.
|
||||
* Whether frames that mutate the filesystem WITHOUT running a registry tool
|
||||
* may do so: the native `delete` frame, and a `read_mcp_resource` carrying
|
||||
* `download_path`. Both write or remove workspace files directly instead of
|
||||
* consulting {@link tools}, so a background read-only advisor could touch
|
||||
* files it was never granted a mutating tool for (issue #5680 review).
|
||||
*
|
||||
* This is a grant, not a policy: it answers "did the session hand this
|
||||
* channel a file-writing tool", which callers derive from their own roster
|
||||
* before any bridge-specific rewriting. The primary Cursor session moves
|
||||
* `edit` out of {@link tools} and serves it through {@link getTool}, so
|
||||
* reading the map here would deny an edit-only session. Defaults to allowed
|
||||
* to preserve the primary agent's behavior; callers with a restricted tool
|
||||
* set (advisors) opt out. The user's approval policy is resolved separately,
|
||||
* per call.
|
||||
*/
|
||||
allowNativeDelete?: boolean;
|
||||
allowDirectFileMutation?: boolean;
|
||||
/**
|
||||
* Mirror Cursor's server-owned todo list into local session state. Cursor
|
||||
* resolves `update_todos` / `read_todos` remotely, so without this bridge
|
||||
@@ -237,20 +245,15 @@ async function executeTool(
|
||||
return createToolResultMessage(toolCallId, toolName, result, isError);
|
||||
}
|
||||
|
||||
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);
|
||||
}
|
||||
|
||||
// Unlike every other frame, this one mutates the filesystem directly instead
|
||||
// of running a registry tool, so no approval wrapper sits in front of it.
|
||||
// `allowNativeDelete` answers "was a mutating tool granted", which is a
|
||||
// different question from "does the user's policy allow this call" — without
|
||||
// this, a configured `deny` or an `always-ask` session still lost the file.
|
||||
// `write` is the tier a file removal belongs to.
|
||||
/**
|
||||
* Resolve the user's policy for a frame that mutates the filesystem directly.
|
||||
*
|
||||
* The native `delete` and `read_mcp_resource` download frames both bypass the
|
||||
* registry, so no approval wrapper sits in front of them. `write` is the tier
|
||||
* a file creation or removal belongs to. Returns `null` when the call may
|
||||
* proceed, or the refusal text to answer with.
|
||||
*/
|
||||
function refuseByWritePolicy(options: CursorExecBridgeOptions, toolName: string, pathArg: string): string | null {
|
||||
const context = options.getToolContext?.();
|
||||
const settings = context?.settings;
|
||||
const approvalMode: ApprovalMode =
|
||||
@@ -261,15 +264,30 @@ async function executeDelete(options: CursorExecBridgeOptions, pathArg: string,
|
||||
approvalMode,
|
||||
(settings?.get("tools.approval") ?? {}) as Record<string, unknown>,
|
||||
);
|
||||
if (approval.policy !== "allow") {
|
||||
const detail =
|
||||
approval.policy === "deny"
|
||||
? `Tool "${toolName}" is blocked by user policy.`
|
||||
: `Tool "${toolName}" requires approval, which this channel cannot request.`;
|
||||
const result = buildToolErrorResult(detail);
|
||||
if (approval.policy === "allow") return null;
|
||||
return approval.policy === "deny"
|
||||
? `Tool "${toolName}" is blocked by user policy.`
|
||||
: `Tool "${toolName}" requires approval, which this channel cannot request.`;
|
||||
}
|
||||
|
||||
async function executeDelete(options: CursorExecBridgeOptions, pathArg: string, toolCallId: string) {
|
||||
const toolName = "delete";
|
||||
|
||||
if (options.allowDirectFileMutation === false) {
|
||||
const result = buildToolErrorResult(`Tool "${toolName}" not available`);
|
||||
return createToolResultMessage(toolCallId, toolName, result, true);
|
||||
}
|
||||
|
||||
// Unlike every other frame, this one mutates the filesystem directly instead
|
||||
// of running a registry tool, so no approval wrapper sits in front of it.
|
||||
// `allowDirectFileMutation` answers "was a mutating tool granted", which is a
|
||||
// different question from "does the user's policy allow this call" — without
|
||||
// this, a configured `deny` or an `always-ask` session still lost the file.
|
||||
const refusal = refuseByWritePolicy(options, toolName, pathArg);
|
||||
if (refusal) {
|
||||
return createToolResultMessage(toolCallId, toolName, buildToolErrorResult(refusal), true);
|
||||
}
|
||||
|
||||
options.emitEvent?.({ type: "tool_execution_start", toolCallId, toolName, args: { path: pathArg } });
|
||||
|
||||
const absolutePath = resolveToCwd(pathArg, options.getCwd?.() ?? options.cwd);
|
||||
@@ -681,7 +699,11 @@ export class CursorExecHandlers implements ICursorExecHandlers {
|
||||
*
|
||||
* A `downloadPath` frame is a different contract: write the bytes to that
|
||||
* workspace-relative path and answer with the path alone, so a large binary
|
||||
* lands on disk instead of in the model's context.
|
||||
* lands on disk instead of in the model's context. That makes it a workspace
|
||||
* mutation reached without a registry tool, so it is gated exactly like the
|
||||
* native `delete` frame — on the session actually granting a file-writing
|
||||
* tool, and on the user's `write`-tier policy. The gate runs before the read
|
||||
* so a refused download never fetches the resource either.
|
||||
*/
|
||||
async readMcpResource({
|
||||
server,
|
||||
@@ -692,6 +714,13 @@ export class CursorExecHandlers implements ICursorExecHandlers {
|
||||
uri: string;
|
||||
downloadPath?: string;
|
||||
}): Promise<CursorMcpResourceContent | null> {
|
||||
if (downloadPath) {
|
||||
if (this.options.allowDirectFileMutation === false) {
|
||||
throw new Error('Tool "write" not available: this session cannot download resources to disk.');
|
||||
}
|
||||
const refusal = refuseByWritePolicy(this.options, "write", downloadPath);
|
||||
if (refusal) throw new Error(refusal);
|
||||
}
|
||||
const mcp = this.options.mcpResources;
|
||||
if (!mcp) return null;
|
||||
const read = await mcp.readServerResource(server, uri);
|
||||
|
||||
@@ -2597,6 +2597,19 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
|
||||
// not match the tool's parameters at all. The registry instance follows
|
||||
// the session's configured mode, so the bridge builds its own.
|
||||
let cursorBridgeEditTool: AgentTool | undefined;
|
||||
// Whether this session granted a file-writing tool, captured HERE because
|
||||
// both inputs are about to move: `edit` is deleted from the registry for
|
||||
// Cursor just below, and `write` may be auto-registered further down as
|
||||
// an xdev transport. The exec bridge answers native `delete` and
|
||||
// resource-download frames that mutate files without running a registry
|
||||
// tool, so it needs the grant as the session actually made it.
|
||||
//
|
||||
// Unconditional, not inside the Cursor branch below: the bridge is
|
||||
// installed for every session, and a session that starts on another
|
||||
// provider can switch to Cursor later — defaulting to "allowed" outside
|
||||
// this branch would hand a restricted read-only session native delete and
|
||||
// download the moment it switched.
|
||||
const cursorCanMutateFiles = toolRegistry.has("edit") || toolRegistry.has("write");
|
||||
if (model?.provider === "cursor") {
|
||||
// Only when the session actually granted `edit`. `createTools` omits
|
||||
// it entirely for a restricted tool set, and the bridge answers native
|
||||
@@ -2679,6 +2692,10 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
|
||||
// it over the registry, so installing it unconditionally would let a
|
||||
// session without `grep` search anyway.
|
||||
createGrepTool: toolRegistry.has("grep") ? createBridgeGrepFactory(toolSession, extensionRunner) : undefined,
|
||||
// The native `delete` and resource-download frames mutate files
|
||||
// without running a registry tool, so this grant is the only thing
|
||||
// standing between a restricted session and a workspace write.
|
||||
allowDirectFileMutation: cursorCanMutateFiles,
|
||||
});
|
||||
|
||||
// Resolve the inline-descriptors setting against the session-start model.
|
||||
|
||||
@@ -761,7 +761,7 @@ export class SessionAdvisors {
|
||||
// Approval mode, per-tool policies and `autoApprove` live only on
|
||||
// this context; without it every bridge tool resolves as `yolo`.
|
||||
getToolContext: this.#advisorGetToolContext,
|
||||
allowNativeDelete: advisorCanMutateFiles,
|
||||
allowDirectFileMutation: advisorCanMutateFiles,
|
||||
// Gated on the advisor's own grant: the factory builds a fresh
|
||||
// tool, so handing it over unconditionally would give a roster
|
||||
// without `grep` a search tool it was denied.
|
||||
|
||||
@@ -1323,8 +1323,20 @@ export class GrepTool implements AgentTool<typeof searchSchema, GrepToolDetails>
|
||||
// Single-file scopes can't paginate — there is one file by definition.
|
||||
const canPaginate = isMultiScope;
|
||||
const skipFiles = canPaginate ? Math.min(normalizedSkip, totalFiles) : 0;
|
||||
const windowFiles = canPaginate ? fileOrder.slice(skipFiles, skipFiles + DEFAULT_FILE_LIMIT) : fileOrder;
|
||||
const fileLimitReached = canPaginate && totalFiles > skipFiles + DEFAULT_FILE_LIMIT;
|
||||
// A caller with a total match cap is not paginating: the cap bounds the
|
||||
// output, and the only consumer that sets one (`pi_grep`) has no `skip`
|
||||
// field to follow a "use skip=N" suggestion with. Windowing it to the
|
||||
// first 20 files would silently return fewer matches than it asked for
|
||||
// while reporting the cap as unreached.
|
||||
//
|
||||
// The window is cap+1 files, not cap: with one match per file, a cap
|
||||
// of N over exactly N files is complete, while over N+1 files it is
|
||||
// clipped — and only reading that extra file distinguishes the two.
|
||||
// The cap below then does the trimming and records that it bit, so
|
||||
// `match_limit_reached` reaches the frame set.
|
||||
const fileWindow = this.#totalMatchLimit !== undefined ? this.#totalMatchLimit + 1 : DEFAULT_FILE_LIMIT;
|
||||
const windowFiles = canPaginate ? fileOrder.slice(skipFiles, skipFiles + fileWindow) : fileOrder;
|
||||
const fileLimitReached = canPaginate && totalFiles > skipFiles + fileWindow;
|
||||
const selectedMatches: GrepMatch[] = [];
|
||||
let totalMatchLimitReached = false;
|
||||
if (windowFiles.length > 0) {
|
||||
@@ -1557,7 +1569,7 @@ export class GrepTool implements AgentTool<typeof searchSchema, GrepToolDetails>
|
||||
count: fileMatchCounts.get(path) ?? 0,
|
||||
})),
|
||||
truncated,
|
||||
fileLimitReached: fileLimitReached ? DEFAULT_FILE_LIMIT : undefined,
|
||||
fileLimitReached: fileLimitReached ? fileWindow : undefined,
|
||||
perFileLimitReached: totalMatchLimitReached
|
||||
? this.#totalMatchLimit
|
||||
: perFileLimitReached
|
||||
|
||||
@@ -161,6 +161,55 @@ describe("CursorExecHandlers.grep bridge", () => {
|
||||
expect(withContextText).toContain("before line");
|
||||
expect(withContextText).toContain("after line");
|
||||
});
|
||||
|
||||
it("satisfies a pi_grep limit that spans more files than one page", async () => {
|
||||
// The local tool windows results to the first 20 files and tells the
|
||||
// caller to paginate with `skip`. `PiGrepExecArgs` has no `skip` field,
|
||||
// so a frame asking for 100 matches across 25 one-match files would get
|
||||
// 20, no `match_limit_reached`, and advice it cannot act on — output
|
||||
// silently short of what it asked for and labelled complete.
|
||||
const spread = path.join(cwd, "spread");
|
||||
await fs.mkdir(spread, { recursive: true });
|
||||
await Promise.all(
|
||||
Array.from({ length: 25 }, (_, i) => Bun.write(path.join(spread, `f${i}.txt`), "needle here\n")),
|
||||
);
|
||||
const scopedHandlers = new CursorExecHandlers({
|
||||
cwd,
|
||||
tools: new Map<string, Tool>([["grep", searchTool]]),
|
||||
createGrepTool: options => new GrepTool(createTestSession(cwd), options),
|
||||
});
|
||||
|
||||
const wide = await scopedHandlers.piGrep({
|
||||
toolCallId: "c1",
|
||||
args: { pattern: "needle", path: spread, limit: 100 },
|
||||
} as never);
|
||||
const details = wide.details as { matchCount?: number; fileLimitReached?: number } | undefined;
|
||||
expect(details?.matchCount).toBe(25);
|
||||
// Nothing was clipped, so no pagination advice the frame cannot follow.
|
||||
expect(details?.fileLimitReached).toBeUndefined();
|
||||
|
||||
// The cap still binds when the matches really do exceed it, and says so:
|
||||
// `match_limit_reached` is the frame's only signal that output was cut,
|
||||
// and one match per file makes the boundary sharp — a cap of 24 over 25
|
||||
// files is clipped, a cap of 25 is complete. Reading only `cap` files
|
||||
// cannot tell those apart.
|
||||
const capped = await scopedHandlers.piGrep({
|
||||
toolCallId: "c2",
|
||||
args: { pattern: "needle", path: spread, limit: 24 },
|
||||
} as never);
|
||||
const cappedDetails = capped.details as { matchCount?: number; perFileLimitReached?: number } | undefined;
|
||||
expect(cappedDetails?.matchCount).toBe(24);
|
||||
expect(cappedDetails?.perFileLimitReached).toBe(24);
|
||||
|
||||
// Exactly at the cap is complete, not clipped.
|
||||
const exact = await scopedHandlers.piGrep({
|
||||
toolCallId: "c3",
|
||||
args: { pattern: "needle", path: spread, limit: 25 },
|
||||
} as never);
|
||||
const exactDetails = exact.details as { matchCount?: number; perFileLimitReached?: number } | undefined;
|
||||
expect(exactDetails?.matchCount).toBe(25);
|
||||
expect(exactDetails?.perFileLimitReached).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("pi_bash truncation reaches the wire from a real BashTool result", () => {
|
||||
@@ -933,6 +982,58 @@ describe("CursorExecHandlers mounted tool bridge", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses a download when the session withheld file mutation or policy denies it", async () => {
|
||||
// Download mode creates and overwrites workspace files without going
|
||||
// through a registry tool, so nothing else enforces the session's
|
||||
// mutation rules — the same hole the native `delete` frame had. A
|
||||
// channel that was never granted a file-writing tool, and a `write` tier
|
||||
// the user denied, must both stop it before the read, so a refused
|
||||
// download does not even fetch the resource.
|
||||
const workspace = await fs.mkdtemp(path.join(os.tmpdir(), "cursor-mcp-grant-"));
|
||||
try {
|
||||
let reads = 0;
|
||||
const mcpResources = {
|
||||
serverNames: () => ["files"],
|
||||
getServerResources: async () => undefined,
|
||||
readServerResource: async (_name: string, uri: string) => {
|
||||
reads++;
|
||||
return { contents: [{ uri, text: "payload" }] };
|
||||
},
|
||||
};
|
||||
|
||||
const ungranted = new CursorExecHandlers({
|
||||
cwd: workspace,
|
||||
tools: new Map(),
|
||||
allowDirectFileMutation: false,
|
||||
mcpResources,
|
||||
});
|
||||
await expect(
|
||||
ungranted.readMcpResource({ server: "files", uri: "files://x", downloadPath: "out.txt" }),
|
||||
).rejects.toThrow(/not available/);
|
||||
|
||||
const denied = new CursorExecHandlers({
|
||||
cwd: workspace,
|
||||
tools: new Map(),
|
||||
mcpResources,
|
||||
getToolContext: () =>
|
||||
({ settings: Settings.isolated({ "tools.approval": { write: "deny" } }) }) as AgentToolContext,
|
||||
});
|
||||
await expect(
|
||||
denied.readMcpResource({ server: "files", uri: "files://x", downloadPath: "out.txt" }),
|
||||
).rejects.toThrow(/blocked by user policy/);
|
||||
|
||||
expect(reads).toBe(0);
|
||||
expect(await Bun.file(path.join(workspace, "out.txt")).exists()).toBe(false);
|
||||
|
||||
// A read without `download_path` mutates nothing, so it is unaffected.
|
||||
const read = await ungranted.readMcpResource({ server: "files", uri: "files://x" });
|
||||
expect(read?.text).toBe("payload");
|
||||
expect(reads).toBe(1);
|
||||
} 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.
|
||||
@@ -1110,13 +1211,13 @@ describe("CursorExecHandlers native delete gating (issue #5680)", () => {
|
||||
await removeWithRetries(cwd);
|
||||
});
|
||||
|
||||
it("rejects native delete and preserves the file when allowNativeDelete is false", async () => {
|
||||
it("rejects native delete and preserves the file when allowDirectFileMutation 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,
|
||||
allowDirectFileMutation: false,
|
||||
});
|
||||
|
||||
const result = await handlers.delete(create(DeleteArgsSchema, { toolCallId: "call-del", path: target }));
|
||||
@@ -1126,13 +1227,13 @@ describe("CursorExecHandlers native delete gating (issue #5680)", () => {
|
||||
expect(await Bun.file(target).exists()).toBe(true);
|
||||
});
|
||||
|
||||
it("performs native delete when allowNativeDelete is true", async () => {
|
||||
it("performs native delete when allowDirectFileMutation 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,
|
||||
allowDirectFileMutation: true,
|
||||
});
|
||||
|
||||
const result = await handlers.delete(create(DeleteArgsSchema, { toolCallId: "call-del", path: target }));
|
||||
@@ -1153,7 +1254,7 @@ describe("CursorExecHandlers native delete gating (issue #5680)", () => {
|
||||
cwd,
|
||||
getCwd: () => currentCwd,
|
||||
tools: new Map(),
|
||||
allowNativeDelete: true,
|
||||
allowDirectFileMutation: true,
|
||||
});
|
||||
|
||||
currentCwd = movedCwd;
|
||||
@@ -1165,7 +1266,7 @@ describe("CursorExecHandlers native delete gating (issue #5680)", () => {
|
||||
});
|
||||
|
||||
it("refuses a native delete the user's policy blocks", async () => {
|
||||
// `allowNativeDelete` answers "was a mutating tool granted", not "does
|
||||
// `allowDirectFileMutation` answers "was a mutating tool granted", not "does
|
||||
// policy allow this call". The frame removes the file with `fs.rmSync`
|
||||
// instead of running a registry tool, so no approval wrapper sits in
|
||||
// front of it — a configured `deny` still lost the file.
|
||||
@@ -1175,7 +1276,7 @@ describe("CursorExecHandlers native delete gating (issue #5680)", () => {
|
||||
const handlers = new CursorExecHandlers({
|
||||
cwd,
|
||||
tools: new Map(),
|
||||
allowNativeDelete: true,
|
||||
allowDirectFileMutation: true,
|
||||
getToolContext: () => ({ settings }) as AgentToolContext,
|
||||
});
|
||||
|
||||
@@ -1196,7 +1297,7 @@ describe("CursorExecHandlers native delete gating (issue #5680)", () => {
|
||||
const handlers = new CursorExecHandlers({
|
||||
cwd,
|
||||
tools: new Map(),
|
||||
allowNativeDelete: true,
|
||||
allowDirectFileMutation: true,
|
||||
getToolContext: () => ({ settings }) as AgentToolContext,
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user