-
Notifications
You must be signed in to change notification settings - Fork 74
feat: include Tempo MCP server in set of exposed Tempo APIs #851
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: include Tempo MCP server in set of exposed Tempo APIs #851
Conversation
Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com>
| prometheus.Labels{"group": "tracesv1api", "handler": "traces"}, | ||
| tempoProxyRead)) | ||
| // The MCP endpoint at /api/mcp uses JSON-RPC | ||
| r.Post("/tempo/api/mcp", c.instrument.NewHandler( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will these APIs be tenanted? (or even need to be tenanted?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the MCP endpoint is handled like a regular Tempo API endpoint.
It requires the X-Scope-OrgID header (specifying the tenant) to be set (if the Tempo instance has multi-tenancy enabled).
squat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm concerned that this MCP endpoint has no authorization on it. This mean that a user/account who lacks the permissions to read/write traces for a tenant will be able to side step that permissions issue by issuing commands to the MCP server and having the agent do it for them directly on Tempo, where Observatorium API can't check permissions.
The MCP endpoint has the same authorization configured which we use for accessing the other Tempo API methods: Lines 877 to 878 in 9e931b5
The only issue I see is that the MCP server may gain tools which should require write permissions in the future. Currently it only exposes read-only tools, i.e. tools which query trace data. I can't think of any MCP tool requiring write access to Tempo, I don't think the MCP server ever will have a tool to ingest traces for example. |
This is exactly what I am driving at: users could escalate permissions via the MCP server. If you have read permissions on the Tempo API via Observation RBAC and the MCP server allows other functionality, you could escalate your permissions. I'm not sure how we want to deal with this, since we aren't in control of the Tempo project and what they choose to put into the MCP server. I would say that RBAC is simply insufficient for MCP and we need fine grained authorization for every It would be a different story if the MCP server was a client of the Observatorium API, rather than the upstream. |
I think RBAC with the current
I can proxy all requests to the MCP server in observatorium and reject tool calls based on their "read-only hint" annotation. Would that work? If so, I'll update this PR.
I did propose to extract the MCP server in upstream, i.e. to change the request flow to For reference, the Tempo MCP server currently exposes the following: Resources
Tools
|
I think this should be good. This means that any tool without the read-only hint annotation won't be callable, which works for us. It moves the burden onto Tempo to correctly annotate tools if they want them to work in truly read-only contexts. |
Include Tempo MCP server in set of exposed Tempo APIs