Skip to content

fix(mcp): recommend direct mcp__* tool calls instead of mcptool CLI#77

Open
cantalupo555 wants to merge 1 commit into
Nomadcxx:mainfrom
cantalupo555:fix/mcp-tool-permission-bypass-hardening
Open

fix(mcp): recommend direct mcp__* tool calls instead of mcptool CLI#77
cantalupo555 wants to merge 1 commit into
Nomadcxx:mainfrom
cantalupo555:fix/mcp-tool-permission-bypass-hardening

Conversation

@cantalupo555
Copy link
Copy Markdown

@cantalupo555 cantalupo555 commented May 22, 2026

Summary

Minimal change to address the MCP permission bypass vulnerability (#74) by updating the system message to instruct the model to use direct tool calls (mcp__<server>__<tool>) instead of the mcptool CLI via shell.

Why this scope?

After review feedback, this PR was reduced to the smallest possible change that moves in the right direction. The original broader PR included schema fallback, tool hook changes, and edit→write rerouting — all deferred to follow-up PRs per maintainer request.

Changes

  • Export MCP_TOOL_PREFIX constant from src/mcp/tool-bridge.ts
  • Update system message in src/plugin.ts to recommend direct mcp__* calls instead of mcptool via shell
  • Update corresponding test expectations

Test Results

Local testing confirmed:

  • ls and grep (MCP tools) work reliably via direct mcp__* calls
  • File read/write operations still bypass permissions in some cases (the model appears to fall back to shell-based access)

This validates that first-class MCP tool calls are viable for several tools, but a follow-up PR with a hybrid approach (prompt + backend interception layer) is needed to fully close the permission bypass for file operations.

Related

Addresses #74

@Nomadcxx
Copy link
Copy Markdown
Owner

Thanks for putting this together, I appreciate you taking a proper crack at it.

I had a good look at this today. I agree with the direction, but I don’t think this is quite as low-risk as it looks from the outside.

The tricky bit is that this touches a few sensitive paths at once: mcp routing, opencode permissions, cursor-agent tool behaviour, plugin tool hooks, and schema fallback. When I rebased it locally, a few existing tests also started failing, which is usually a sign that we need to slow down and split the change up a bit.

So I’m not comfortable merging this as-is yet. I think we need some deeper testing around the permission behaviour first, and probably one or two smaller follow-up PRs so we can separate the actual mcp routing fix from the broader tool/schema changes.

Appreciate the PR though, there’s good work in here and I do want to keep moving on this carefully.

@cantalupo555 cantalupo555 force-pushed the fix/mcp-tool-permission-bypass-hardening branch from 2c1eb8e to 766617c Compare May 23, 2026 09:13
@cantalupo555
Copy link
Copy Markdown
Author

cantalupo555 commented May 23, 2026

Thanks for the review and for the clear guidance on keeping the scope minimal.

Following your suggestion, I reduced the PR to the smallest possible change:

  • Updated the system message to instruct the model to use direct MCP tool calls (mcp__<server>__<tool>) instead of mcptool via shell.
  • Exported MCP_TOOL_PREFIX from the MCP tool bridge so the prefix is consistent.

I tested the change locally with real usage:

  • ls and grep (MCP tools) are now working reliably through direct tool calls (mcp__*). This confirms that the direction of exposing MCP tools as first-class calls is viable for several tools.
  • However, the permission bypass issue described in Permissions are broken when installing OpenCode with this plugin. #74 still occurs during read and write operations. The model is able to successfully read and write files, but it does so without respecting the configured permissions (it appears to still fall back to shell-based access in some cases). This is exactly the behavior we need to prevent.

Because the pure prompt change is not sufficient to fully close the permission bypass on file operations, I'm considering a hybrid approach for a follow-up PR:

  • Keep recommending direct mcp__* calls in the system message (preserving the security improvement for tools that already work well).
  • Add a lightweight fallback/interception layer in the tool loop to catch cases where the model attempts to use shell commands for sensitive file operations, and either reroute them to the proper MCP tool or block the insecure path.

This would be a separate, focused PR.

Would you be open to exploring a hybrid approach in a follow-up, or do you have a different preference on how to handle the remaining read/write permission bypass?

Happy to adjust based on your feedback.

@cantalupo555 cantalupo555 force-pushed the fix/mcp-tool-permission-bypass-hardening branch from 766617c to 27208dc Compare May 23, 2026 10:38
@cantalupo555 cantalupo555 changed the title fix(mcp): route MCP tools as direct tool calls to prevent permission bypass fix(mcp): recommend direct mcp__* tool calls instead of mcptool CLI May 23, 2026
@cantalupo555
Copy link
Copy Markdown
Author

cantalupo555 commented May 23, 2026

@Nomadcxx
As requested in the review, the PR has been reduced to the minimal change: commit 31c795e.

- Export MCP_TOOL_PREFIX constant
- Update system message to instruct models to use direct MCP tool calls
- Adjust corresponding test
@cantalupo555 cantalupo555 force-pushed the fix/mcp-tool-permission-bypass-hardening branch from 27208dc to 31c795e Compare May 23, 2026 11:40
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.

2 participants