Skip to content

Add Cursor MCP support for global mode#1273

Merged
dyoshikawa merged 4 commits intodyoshikawa:mainfrom
tmak:cursor-mcp-support-for-global-mode
Mar 9, 2026
Merged

Add Cursor MCP support for global mode#1273
dyoshikawa merged 4 commits intodyoshikawa:mainfrom
tmak:cursor-mcp-support-for-global-mode

Conversation

@tmak
Copy link
Contributor

@tmak tmak commented Mar 6, 2026

This PR adds Cursor MCP support for global mode.

Cursor supports global MCP server configuration via ~/.cursor/mcp.json.

Changes

  • Updated CursorMcp.getSettablePaths() to accept global flag
  • Updated fromFile() to gracefully initialize an empty config when the file doesn't exist (instead of throwing), which is needed for first-time global setup and is consistent with what ClaudecodeMcp.fromFile() does
  • Updated fromRulesyncMcp() to be async and merge MCP servers into existing config (preserving non-mcpServers properties like user settings)
  • Added isDeletable() override to prevent deleting the shared global mcp.json
  • Set supportsGlobal: true in mcp-processor.ts
  • Added comprehensive tests for global mode: path resolution, config merging, property preservation, and round-trip conversion
  • Added cursor to global mode E2E MCP test

Documentation:

  • Updated supported tools tables in README.md, docs/reference/supported-tools.md, and skills/rulesync/supported-tools.md to show 🌏 for Cursor MCP and rules
  • Updated docs/guide/global-mode.md and skills/rulesync/global-mode.md to list Cursor among supported tools

@dyoshikawa-claw

This comment has been minimized.

@github-actions

This comment has been minimized.

@dyoshikawa-claw

This comment has been minimized.

Co-authored-by: dyoshikawa-claw <dyoshikawa-claw@users.noreply.github.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2026

Overall Mergeability Verdict for PR #1273: ✅ MERGEABLE

The PR can be merged as it only contains medium and low severity issues (no high/critical blockers).


Integrated Review Results

Code Review Summary

Status: No critical or high severity issues found

Key Findings:

  1. Code Quality - Excellent

    • Clean implementation following existing patterns (ClaudecodeMcp)
    • Proper isDeletable() override preventing global config deletion
    • Comprehensive error handling with descriptive messages
    • Cross-platform path handling with join()
  2. Test Coverage - Comprehensive (298 lines added)

    • ✅ Global vs local mode path resolution
    • isDeletable() in both modes
    • ✅ Property preservation during config updates
    • ✅ Environment variable format conversion
    • ✅ Round-trip conversion tests
    • ✅ E2E integration test
  3. Pattern Consistency - Strong

    • Consistent with ClaudecodeMcp implementation
    • Proper async handling in fromRulesyncMcp()
    • Defensive null/undefined handling with ??
  4. Minor Observations (Non-blocking)

    • getSettablePaths() accepts but doesn't use global parameter (intentional - same paths for both modes)
    • Double-defaulting of mcpServers in fromFile() (harmless, defensive)

Security Review Summary

Status: Medium and low severity issues identified

Key Findings:

  1. Medium Severity - TOCTOU Race Condition (src/features/utils/file.ts:20-30)

    • Time-of-check to time-of-use window in readOrInitializeFileContent()
    • Could result in file content overwrites in concurrent scenarios
    • Mitigating factors: Local CLI tool, user-specific config, small timing window
    • Recommendation: Consider atomic file operations with exclusive flags
  2. Medium Severity - JSON Prototype Pollution (cursor-mcp.ts:79-81, 117-118)

    • No protection against __proto__ pollution in JSON parsing
    • Mitigating factors: CLI tool processing local config files only, attacker needs file write access
    • Recommendation: Consider using reviver function or validation
  3. Low-Medium Severity - Symlink Security

    • No symlink detection for global config files
    • Potential symlink attack could cause indirect information disclosure
    • Mitigating factors: Requires local write access to ~/.cursor/
    • Recommendation: Add symlink checks for global configs
  4. Low Severity - Error Message Verbosity

    • Verbose error messages include file paths and Zod issues
    • Could leak directory structure in automation contexts
    • Acceptable for CLI tool: Helps debugging

Security Strengths:

  • ✅ Path traversal protection in base AiFile class
  • ✅ Global mode isolation with isDeletable() protection
  • ✅ Proper type safety through TypeScript
  • ✅ No dynamic code execution
  • ✅ Safe environment variable transformations
  • ✅ Input validation via Zod schemas

Recommendations

Priority 1 - Consider Before Merge:

  1. Add Symlink Detection (Security Finding Can generate/import to gemini-cli #7)
    • Prevent symlink attacks on global config files
    • Example implementation:
      if (await isSymlink(filePath)) {
        throw new Error(`Refusing to follow symlink: ${filePath}`);
      }

Priority 2 - Future Improvements:

  1. Address TOCTOU Race Condition (Security Finding Add support for opencode-ai/opencode #5)

    • Use exclusive file creation flags for atomic operations
    • Low priority given CLI nature of tool
  2. Add Prototype Pollution Protection (Security Finding Add support for gemini-cli #6)

    • Add validation for __proto__, constructor, prototype keys
    • Low priority given local file access requirement

Final Assessment

Category Rating Notes
Code Quality ⭐⭐⭐⭐⭐ Clean, well-structured, follows patterns
Security ⭐⭐⭐⭐ Good posture, medium-risk issues mitigated by use case
Test Coverage ⭐⭐⭐⭐⭐ Comprehensive, edge cases covered
Documentation ⭐⭐⭐⭐⭐ All relevant docs updated

PR #1273 is ready for merge. The identified security issues are medium-to-low severity and are mitigated by the CLI nature of the tool. Consider addressing symlink detection as a quick win before merge, but it's not blocking.

github run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants