Skip to content

Conversation

@ColeMurray
Copy link
Contributor

Summary

The open_deeplink function in the Cursor CLI integration accepted arbitrary URLs without validation and used cmd.exe on Windows to open them. This fix adds URL scheme validation and replaces the Windows subprocess call with a safer API.

Changes

  • URL validation: The function now validates that deeplinks use the cursor:// scheme before opening them. URLs with other schemes (http, https, file, etc.) are rejected.
  • Windows improvement: Replaced subprocess.run(["cmd", "/c", "start", ...]) with os.startfile(), which is the standard Python API for opening files and URLs on Windows and doesn't involve shell interpretation.
  • Test coverage: Added tests for scheme validation, empty URLs, and error handling on Windows.

Why This Matters

The original implementation on Windows passed user-controlled input directly to cmd.exe, which could potentially be exploited through command injection. While the actual attack surface depends on how deeplinks are constructed in practice, this defense-in-depth fix ensures that:

  1. Only valid cursor:// URLs are processed
  2. Windows URL opening doesn't involve shell commands
  3. The code follows Python best practices for cross-platform URL handling

The fix maintains backward compatibility while improving security posture.

# Before: No validation, shell command on Windows
subprocess.run(["cmd", "/c", "start", deeplink], ...)

# After: Validated scheme, native API
parsed = urlparse(deeplink)
if parsed.scheme != "cursor":
    return False
os.startfile(deeplink)  # Windows only

- Add URL scheme validation to reject non-cursor:// URLs
- Replace subprocess cmd.exe call with os.startfile() on Windows
- Add tests for scheme validation and error handling
@marvin-context-protocol marvin-context-protocol bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. cli Related to FastMCP CLI commands (run, dev, install) or CLI functionality. labels Nov 2, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 2, 2025

Walkthrough

Modified deeplink handler in cursor module to validate cursor scheme, replace subprocess with os.startfile for Windows, and expand error handling to OSError. Changes include new imports for os and urlparse.

Changes

Cohort / File(s) Summary
Deeplink Handler Improvements
src/fastmcp/cli/install/cursor.py
Added os and urlparse imports; added deeplink scheme validation in open_deeplink function; replaced subprocess call with os.startfile for Windows path handling; expanded exception handling to include OSError alongside subprocess.CalledProcessError and FileNotFoundError.

Poem

🐰 Through cursor depths, the deeplinks dart,
Schemes validated from the start!
From subprocess hops to startfile's grace,
Windows paths find their rightful place! 🌟
Error handlers hop with care,
Input validation's everywhere!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description provides thorough and well-structured content (Summary, Changes, Why This Matters, and code examples) that effectively explains the modifications and their rationale. However, it does not follow the required template structure specified in the repository. The description is missing both required sections: the Contributors Checklist (with items for issue closure, workflow compliance, testing, and documentation) and the Review Checklist (with self-review and readiness confirmation items). While the descriptive content itself is complete and high-quality, the omission of these mandatory template sections represents a significant structural deviation from the repository's requirements. Update the pull request description to follow the required template structure by adding the Contributors Checklist (confirming the issue number, workflow compliance, test coverage, and documentation updates) and the Review Checklist (confirming self-review and readiness for review). The existing descriptive content can remain and complement the template structure.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "Security: Validate Cursor deeplink URLs and use safer Windows API" is concise, specific, and directly captures the two main changes in the changeset: URL scheme validation and replacement of subprocess calls with a safer Windows API. The title avoids vague terms and clearly communicates the primary security-focused improvements, allowing a teammate scanning the repository history to immediately understand the change's purpose.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c8ddbff and 21027cb.

⛔ Files ignored due to path filters (1)
  • tests/cli/test_cursor.py is excluded by none and included by none
📒 Files selected for processing (1)
  • src/fastmcp/cli/install/cursor.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use Python ≥ 3.10 and provide full type annotations for library code

Files:

  • src/fastmcp/cli/install/cursor.py
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Never use bare except; always catch specific exception types

Files:

  • src/fastmcp/cli/install/cursor.py
🔇 Additional comments (4)
src/fastmcp/cli/install/cursor.py (4)

4-4: LGTM! Standard library imports for enhanced security.

The added imports support URL validation and the safer Windows API approach.

Also applies to: 9-9


56-59: LGTM! Effective scheme validation prevents URL injection.

The validation correctly restricts deeplinks to the cursor:// scheme, preventing potential injection via alternative schemes like file://, http://, etc. The use of urlparse handles case-insensitive scheme matching automatically.


69-69: LGTM! Proper exception handling for os.startfile.

Adding OSError to the exception tuple is necessary since os.startfile raises OSError when it cannot open the URL (e.g., if the cursor:// protocol handler is not registered). The code continues to catch specific exception types as required by the coding guidelines.


65-65: Good security improvement using os.startfile for Windows URI scheme handling.

The implementation correctly uses os.startfile to invoke the cursor:// protocol handler on Windows, which is the standard and safe approach. The exception handling at line 70 properly catches OSError (along with subprocess exceptions for other platforms), which covers scenarios where the URI scheme is not registered or system calls fail. Comprehensive test coverage in tests/cli/test_cursor.py verifies both successful invocation and error handling.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Owner

@jlowin jlowin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ColeMurray thank you for this PR!

@jlowin jlowin merged commit 0e97a26 into jlowin:main Nov 3, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. cli Related to FastMCP CLI commands (run, dev, install) or CLI functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants