Conversation
…s-20260827 fix: resolve safe issues from open-issue triage
Eager MCP name blocker resolvedThe final Cursor protobuf encoder now defensively sanitizes both name-bearing fields at the last protocol boundary, including callers that provide an alias directly. Names are ASCII-only, bounded to Cursor's maximum, and remain independent from the original registry dispatch key. Added regression coverage for 20 eager MCP descriptors containing spaces, slashes, dots, and other unsafe characters. Focused validation now passes with 44 Cursor runtime tests, and bounded live acceptance with |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let result = run_native_text_command( | ||
| client, | ||
| tx.clone(), | ||
| &prompt, | ||
| &model, | ||
| None, | ||
| resume_session_id.as_deref(), | ||
| &stream_uuid, | ||
| &tools, | ||
| &system, | ||
| tool_result_rx, |
There was a problem hiding this comment.
build_cli_prompt(system, messages) already includes the system text, but this call passes both that assembled prompt and the original system to the transport. routed_prompt then prepends the system text again. Every non-empty system instruction is sent twice with different formatting, which changes instruction precedence. The extra copy and tool-routing directive are also added after the 120,000-character limit, so the final request is no longer bounded by MAX_PROMPT_CHARS.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-provider-cursor-runtime/src/lib.rs
Line: 563-573
Comment:
**Avoid Duplicate System Prompts**
`build_cli_prompt(system, messages)` already includes the system text, but this call passes both that assembled prompt and the original `system` to the transport. `routed_prompt` then prepends the system text again. Every non-empty system instruction is sent twice with different formatting, which changes instruction precedence. The extra copy and tool-routing directive are also added after the 120,000-character limit, so the final request is no longer bounded by `MAX_PROMPT_CHARS`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if !tools.is_empty() { | ||
| if let Ok(mcp_tools_bytes) = crate::wire::encode_mcp_tools(tools) { | ||
| req.extend(field_ld(4, &mcp_tools_bytes)); | ||
| } |
There was a problem hiding this comment.
If any advertised tool has a schema the bounded protobuf encoder cannot represent, such as more than 32 nested object or array levels, encode_mcp_tools returns an error that this branch silently discards. The request then proceeds without the entire tool payload. Because this provider reports that it handles tools internally, jcode has no fallback execution path, so the requested tools cannot run. Propagate the encoding error instead of sending a request without its tools.
Artifacts
- Copies the target crate outside the repository, appends a narrow runtime test, and runs both the valid and unencodable schema cases; it provides the exact executed source.
- Runs the valid-tool control against the copied target crate with exit code 0 and reports `baseline valid_tool_field4_present=true`; valid schemas are advertised.
- Runs the deeply nested tool schema case against the copied target crate with exit code 0 and reports the depth-limit error, one emitted request frame, and missing field 4; the error is silently converted into a tools-free request.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-provider-cursor-runtime/src/agent_transport.rs
Line: 328-331
Comment:
**Propagate Tool Encoding Failures**
If any advertised tool has a schema the bounded protobuf encoder cannot represent, such as more than 32 nested object or array levels, `encode_mcp_tools` returns an error that this branch silently discards. The request then proceeds without the entire tool payload. Because this provider reports that it handles tools internally, jcode has no fallback execution path, so the requested tools cannot run. Propagate the encoding error instead of sending a request without its tools.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| let len = u32::from_be_bytes([buf[1], buf[2], buf[3], buf[4]]) as usize; | ||
| let end = 5 + len; | ||
| if buf.len() < end { | ||
| return None; | ||
| return Ok(None); | ||
| } | ||
| let mut payload = buf[5..end].to_vec(); | ||
| if flag & 0x01 != 0 { | ||
| // gzip-compressed payload | ||
| let mut decoded = Vec::new(); | ||
| if GzDecoder::new(&payload[..]) | ||
| GzDecoder::new(&payload[..]) | ||
| .read_to_end(&mut decoded) | ||
| .is_ok() | ||
| { | ||
| payload = decoded; | ||
| } | ||
| .context("Invalid gzip payload in Cursor agent stream")?; |
There was a problem hiding this comment.
The response path allocates from peer-controlled data without a byte limit. An incomplete frame can grow pending toward its declared 32-bit length, gzip decoding uses unbounded read_to_end, and decoded KV blobs are retained in an unbounded map. A configured or compromised AgentService peer can therefore send a large frame, compression bomb, or repeated unique blobs and exhaust the jcode process before the time deadline. Enforce limits for compressed payloads, decompressed payloads, the pending buffer, and aggregate blob storage.
How this was verified: A response reproduction retained 8,388,613 bytes for an incomplete 4 GiB frame, expanded 16,328 compressed bytes to 16,777,216 bytes, and retained 12 unique peer blobs totaling 12,582,960 bytes.
Artifacts
- This read-only script extracts and executes the exact pre-change and candidate `next_frame` implementations with incomplete-frame and gzip inputs, showing that both allocate without a bound.
- The executed pre-change decoder test retained 8,388,613 pending bytes for a 4 GiB declared frame and expanded 16,328 compressed bytes to 16,777,216 bytes, showing the framing and gzip issue already existed.
- The executed candidate decoder test produced the same unbounded pending-buffer and gzip-expansion result, showing the candidate retains those response allocation paths.
- This read-only script compares pre-change source and runs a standalone protobuf harness matching the candidate KV set-blob response handling, showing unique peer blobs accumulate.
- The executed pre-change comparison found zero `blob_store.insert` and zero `set_blob` occurrences, showing this blob-retention behavior was not present before the candidate commit.
- The executed candidate KV handler reproduction stored 12 unique 1 MiB peer blobs as 12 entries and 12,582,960 retained bytes, showing unbounded aggregate retention.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-provider-cursor-runtime/src/agent_transport.rs
Line: 378-389
Comment:
**Bound Response Memory Usage**
The response path allocates from peer-controlled data without a byte limit. An incomplete frame can grow `pending` toward its declared 32-bit length, gzip decoding uses unbounded `read_to_end`, and decoded KV blobs are retained in an unbounded map. A configured or compromised AgentService peer can therefore send a large frame, compression bomb, or repeated unique blobs and exhaust the jcode process before the time deadline. Enforce limits for compressed payloads, decompressed payloads, the pending buffer, and aggregate blob storage.
**How this was verified:** A response reproduction retained 8,388,613 bytes for an incomplete 4 GiB frame, expanded 16,328 compressed bytes to 16,777,216 bytes, and retained 12 unique peer blobs totaling 12,582,960 bytes.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| fn stream_debug(message: impl std::fmt::Display) { | ||
| if std::env::var_os("CURSOR_STREAM_DEBUG").is_some() { | ||
| eprintln!("cursor-stream: {message}"); | ||
| } | ||
| } |
There was a problem hiding this comment.
stream_debug commits an eprintln!-based transport diagnostic. The repository directive permits eprintln! only for throwaway diagnostics that are deleted before committing. Remove this path or route durable diagnostics through the supported logging mechanism. This repository requirement must be satisfied before merging.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-provider-cursor-runtime/src/agent_transport.rs
Line: 49-53
Comment:
**Remove Debug Diagnostic**
`stream_debug` commits an `eprintln!`-based transport diagnostic. The repository directive permits `eprintln!` only for throwaway diagnostics that are deleted before committing. Remove this path or route durable diagnostics through the supported logging mechanism. This repository requirement must be satisfied before merging.
**Context Used:** AGENTS.md ([source](https://github.com/1jehuang/jcode/blob/master/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
b5cb0e5 to
48cdbd8
Compare
1jehuang
added
area: providers
github-actions
Bot
added
area: providers
3 participants
Summary
Implement the Cursor native AgentService HTTP/2 transport needed for reliable streaming and MCP tool execution through jcode.
Changes
This is complementary to #1226 and #575. The branch is intentionally focused on native AgentService transport and tool bridging.
Validation
upstream/mastercompleted successfully.jcode-provider-cursor-runtimelibrary tests passed.jcode-provider-doctorlibrary tests passed.Closes #1258
Refs #1226
Refs #575