MCP tool calls bypass ctx.approval — no gate on side-effectful bridged tools

[ezzaldeeen]

what I observed

@deepseek-ai/dsh-mcp-client appears to have no reference to the approval seam.
Grepping the published lib/index.js for ctx.approval / approval.request
returns nothing, while dsh-sandbox, dsh-tool-bash, dsh-tool-fs and
dsh-tools all consume it.

The effect is that a bridged MCP tool call is never gated. The permission model
covers file and shell access, but a tool call from an MCP server runs
unconditionally.

why it matters for us

We embed the harness in a desktop audio app and mount several MCP servers,
including ones that call paid generation APIs. Those calls spend real credits.
Under the harness they run with no prompt; other ACP/agent hosts we've used ask
before the first call and let the user grant it thereafter.

There is no way to close this from the host side: the harness spawns the MCP
servers as its own children and calls them directly, so an embedding
application never sees the call and cannot interpose.

what I'm unsure about

Gating every MCP call would clearly be wrong — it would prompt on read-only
tools and make the seam useless through noise. Some options, none obviously
right:

• Key off annotations.readOnlyHint: gate only tools that don't declare it
  (related to a separate discussion I'm opening about that annotation).
• A per-server config field, e.g. requireApproval: true, so the deployment
  decides.
• A ctx.tools classification the bridge sets, letting the existing pipeline
  route the ask decision rather than the bridge calling the seam itself.

The last seems most in keeping with the architecture — the README notes the
tools pipeline already routes ask decisions and fails closed without an
answerer — but I don't know the intended boundary well enough to say.

Is the current behaviour deliberate (MCP servers treated as trusted because
mounting one is itself the act of trust), or a gap?

Version

@deepseek-ai/dsh-mcp-client@0.1.0-rc.7. Read from compiled output, not source
— apologies if I've misread the structure.

[weijiafu14]

There is an immediately usable alternative path for this through pi2dsh + the unmodified pi-mcp-adapter package. The adapter has its own per-server/per-tool approval policy (approveTools: true or a tool-pattern list) and applies the same gate to proxy calls, dynamically registered direct tools, resources, and MCP App-originated calls.

Through pi2dsh, the approval request crosses DSH userQuestions; the real-host E2E verifies that a human decision is collected and reused for the session. Headless mode fails closed when approval is required and no answerer exists.

dsh plugin --profile web add pi2dsh
dsh plugin --profile web add pi-mcp-adapter

This is package-owned approval rather than a claim that the official dsh-mcp-client now calls ctx.approval. The native client still needs the architectural decision discussed here, but users with paid or side-effectful MCP tools can enforce a gate today.

https://github.com/weijiafu14/pi2dsh/blob/main/docs/mcp-compatibility.md

[huangyiyang89]

I can reproduce this gap with @deepseek-ai/dsh@0.1.1-rc.2 using the official MCP client and Playwright MCP in a Windows desktop host.

This is particularly important for browser MCP tools: a "read-only" call can still expose content from an authenticated browser session, while navigation, clicks, form input, uploads, and script execution can cause external side effects.

On the current bridge path, syncTools forwards the tool name, description, input/output schemas, and task support to createDefinition, but drops MCP annotations and does not expose stable MCP provenance to ToolRuntime policy producers. That means a host cannot implement robust per-server/per-tool allow / ask / deny rules through tools/pre-execute without depending on the mcp__<server>__<tool> naming convention or patching the official package.

I don't think dsh-mcp-client should own dialogs or persistent grants. A minimal architecture-compatible direction could be:

1. Preserve immutable, non-model-facing MCP provenance (server identity + raw tool name) and ToolAnnotations when registering tools, or expose an equivalent public catalog API.
2. Let a separate Host/policy plugin map explicit per-server/per-tool rules to tools/pre-execute.
3. Treat ToolAnnotations only as untrusted hints for defaults/UI, not as an authorization boundary.
4. Route ask through the existing dsh-user-approval seam so interactive hosts can answer and headless deployments fail closed.
5. Keep existing behavior unchanged by default initially.

This would also provide the policy metadata needed beyond the concurrency concern reported in #3905.

Would the maintainers prefer a generic non-model-facing metadata/provenance field on ToolDefinition, an MCP-specific catalog/service, or an optional policy hook owned by dsh-mcp-client?

[dodogeny]

The option you're leaning toward, gating only tools that don't declare readOnlyHint, works but it inherits whatever the upstream server decided to annotate. Plenty of servers omit the hint or set it wrong, so a paid generation tool that should be gated slips through because someone marked it read-only by accident, or didn't annotate at all and your bridge treats missing as safe. The per-server requireApproval field sidesteps that since the deployment decides, not the server.

I work on Apitella (https://apitella.com/?utm_source=github-discussion&utm_medium=comment&utm_campaign=outreach-12). We snapshot MCP servers and diff their tool annotations over time, which catches exactly the case where a hint you're relying on for gating changes or was never trustworthy to start with. It doesn't wire the seam into your client, that part's yours, but it'd tell you which mounted servers you can actually trust the hints from. Happy to compare notes.

[bhooshanvarma]

For a paid generation endpoint, I would first test approval against a fake credit ledger: approved, denied, and no approval response. The debit count should be checked outside the tool result. Is there one client call path all audio tools use, or can some bypass that wrapper?