AurimasNav opened a new issue, #43358:
URL: https://github.com/apache/superset/issues/43358
> _Drafted with AI assistance._
### Bug description
Every failed MCP tool call returns a **successful** result with the error as
plain text. `isError` is never set, so a client following the MCP spec reads a
failure — including a permission denial — as a success.
Observed on 6.1.0 against a read-only service user whose role forbids the
tool:
```json
{"jsonrpc":"2.0","id":7,"result":{"content":[{"type":"text","text":
"Error: Error calling tool 'save_sql_query': Permission denied: can_write
on SavedQuery for user mcp-svc (tool: save_sql_query)"}],
"isError":false}}
```
The message itself is good — it names the permission, view, user and tool.
The problem is purely the flag: an agent that checks `isError` concludes the
write succeeded, and may report it as done.
### Root cause, and why this isn't a simple fix
`StructuredContentStripperMiddleware.on_call_tool` catches every exception
and returns an ordinary `ToolResult`:
https://github.com/apache/superset/blob/master/superset/mcp_service/middleware.py#L767-L821
The catch is deliberate, and the comment explains why:
> When exceptions propagate past the middleware chain to the MCP SDK layer,
they become `CallToolResult(isError=True)`. Some transports (Claude.ai's MCP
bridge) cannot encode these error responses, producing "encoding without a
string argument". Catch ALL exceptions (not just specific types) because any
unhandled exception … will cause encoding failures on the wire.
So this is a workaround for a client-side encoding bug, traded against the
protocol contract for every other client. That trade is invisible to callers
and currently silent.
A fix has to keep the workaround working for that transport while restoring
the contract for conforming clients. Possible directions, in rough order of
preference:
1. Set the error flag while keeping the same encodable text payload — if the
bridge's failure is specifically about the SDK's error *object* rather than a
result carrying `isError`, this may satisfy both.
2. Gate the behaviour on transport or on a config flag, defaulting to
spec-conformant.
3. Leave as-is but document it prominently, so integrators know `isError` is
not usable against Superset.
**A question I can't answer from outside:** does the Claude.ai bridge
encoding bug still reproduce on current `fastmcp` (3.4.7 here)? If it has since
been fixed upstream, the workaround can simply be removed and this becomes a
one-line change. Maintainers with access to that transport are better placed to
check than I am.
### How to reproduce
1. Give the MCP identity a role lacking `can_write on SavedQuery`.
2. Call `save_sql_query` with valid arguments (`database_id`, `label`,
`sql`) so it reaches the RBAC check rather than failing validation.
3. Inspect the JSON-RPC result: the text describes a permission denial, and
`isError` is `false`.
Any failing tool call shows it; a denial is just the clearest case, since
the operation definitively did not happen.
### Why it matters to us
We proxy an internal agent to a deliberately read-only Superset MCP. The
proxy forwards caller tokens and does not pattern-match Superset's error
strings to synthesise the flag, since that would couple us to another service's
message wording. So a forbidden write currently returns a success-shaped
response to an agent.
### Not part of this report
Two things I checked that are already handled on `master`, noted so nobody
re-investigates:
- **Error IDs are no longer collision-prone.** 6.1.0 builds them as
`f"err_{int(time.time())}"`, so two failures in the same second share an ID;
`master` uses `mcp_call_id or f"err_{secrets.token_hex(8)}"`.
- **Denials are not mislabelled.** They render with a specific permission
message, as above. The generic `Internal error … Error ID: …` text appears for
*validation* errors (e.g. a missing required field), which is a separate and
much less serious cosmetic issue.
### Superset version
master / latest-dev
### Python version
3.11
### Node version
Not applicable
### Browser
Not applicable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
