mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-07 16:03:21 +00:00
feat(mcp): add multi-workspace MCP tokens via the gateway endpoint (#10043)
* feat(mcp): add multi-workspace MCP tokens via the gateway endpoint A single MCP token with no bound workspace (workspace_id NULL + mcp scope) now works across every workspace the token owner can access, served through the existing /api/mcp/gateway endpoint. This avoids having to register one MCP server entry per workspace in clients like Claude/Cursor. In multi-workspace mode the runner exposes a synthetic `list_workspaces` tool plus the generic API endpoint tools, each workspace-scoped one gaining a required `workspace_id` argument (mirroring the proxy pattern users built externally). Per-workspace scripts/flows are not enumerated to avoid flooding the tool list — they are run via runScriptByPath/runFlowByPath with an explicit workspace_id. Auth is resolved per tool call: the gateway middleware detects a workspace-less mcp token and marks the request MultiWorkspaceMcp, and the runner resolves a per-workspace ApiAuthed from the raw token via the AuthCache (validating membership; superadmins may act in any workspace). Single-workspace tokens are unchanged. Frontend: the MCP token creation flow gains an "All workspaces" option that produces a workspace-less token and the gateway URL. Fixes WIN-2153 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): cover multi-workspace endpoint tool transformation Unit tests for endpoint_tool_to_mcp_tool_multi and list_workspaces_tool: workspace-scoped tools gain a required workspace_id arg, global tools are left unchanged, workspace_id is not duplicated, and list_workspaces takes no arguments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): forward script/flow args for runScriptByPath/runFlowByPath These endpoints have an additionalProperties body (no declared properties), so build_request_body previously returned an empty body and dropped every script/flow argument. This was latent for the per-path run endpoints and became load-bearing in multi-workspace mode, where scripts/flows can only be run via runScriptByPath/runFlowByPath — parameterized runs silently lost their arguments. build_request_body now forwards all arguments not consumed by a path/query parameter for pass-through (additionalProperties) bodies, keeping the strict declared-only behavior for endpoints with explicit properties. The runner strips the synthetic workspace_id argument before dispatch so it can't leak into the forwarded body. Reported by Codex review on #10043. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(mcp): note workspace_id requirement in multi-workspace tool descriptions Workspace-scoped tools already gain a required workspace_id parameter (with its own schema description) in multi-workspace mode, but the tool's prose description was unchanged. Append a note so models/clients that read the description text know to pass workspace_id (and to call list_workspaces first). Global tool descriptions are left untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): trim multi-workspace tool/arg descriptions The workspace_id note repeats across every workspace-scoped tool in each tools/list, so keep it terse: description suffix "Requires `workspace_id`." and arg description "Target workspace id (from list_workspaces)." to avoid spending tokens on repeated boilerplate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): enforce script/flow path scopes for multi-workspace run-by-path In multi-workspace mode runScriptByPath/runFlowByPath are the only way to run scripts/flows, but they were authorized against the endpoint scope only — never the caller's mcp:scripts:/mcp:flows: path scopes. A granular token could run items outside its allowed paths (e.g. mcp:scripts:f/team/* + mcp:endpoints:* running f/other/secret), and a mcp:endpoints:* token could run arbitrary scripts. Now these two endpoints are authorized by the script/flow scope of the requested path (matching single-workspace mode's per-item tools): exposed in list_tools only when the token grants some script/flow (McpScopeConfig::has_any), and at call time the path is checked via is_allowed("script"/"flow", path). Verified e2e: mcp:scripts:f/team/* runs f/team/* but is denied f/other/*; mcp:endpoints:* alone no longer exposes or runs run-by-path. Reported by Codex + Pi review on #10043. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): deny run-by-path for mcp:favorites multi-workspace tokens mcp:favorites sets granular=false, so the previous run-by-path scope check (gated on `granular`) was skipped entirely — a default "Favorites only" all-workspaces token could run any script/flow by naming its path, bypassing the favorites restriction. Favorites are an enumerated set reachable only through per-item tools, not by arbitrary path, so they grant nothing for run-by-path. has_any() now returns true only for mcp:all (not favorites), and the call-time check drops the `granular` gate and relies on is_allowed() directly (already false for favorites, true for mcp:all, pattern-matched for granular). Verified e2e: mcp:favorites no longer exposes or runs run-by-path; mcp:all still runs; granular script scopes still path-enforced. Reported by Codex review on #10043. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
3b0781761b
commit
8343203ec2
@@ -483,8 +483,13 @@ pub async fn run_server(
|
||||
add_www_authenticate_header, add_www_authenticate_header_gateway,
|
||||
extract_workspace_from_token,
|
||||
};
|
||||
let (mcp_router, mcp_cancellation_token) =
|
||||
setup_mcp_server(db.clone(), user_db, _base_internal_url.clone()).await?;
|
||||
let (mcp_router, mcp_cancellation_token) = setup_mcp_server(
|
||||
db.clone(),
|
||||
user_db,
|
||||
_base_internal_url.clone(),
|
||||
auth_cache.clone(),
|
||||
)
|
||||
.await?;
|
||||
// Workspace-scoped MCP router
|
||||
let workspaced_mcp_router = mcp_router
|
||||
.clone()
|
||||
|
||||
@@ -10,10 +10,11 @@ use windmill_common::{db::UserDB, utils::StripPath, DB};
|
||||
use windmill_mcp::common::schema::enrich_resource_schemas;
|
||||
use windmill_mcp::common::transform::apply_key_transformation;
|
||||
use windmill_mcp::common::types::{
|
||||
FlowInfo, HubScriptInfo, ResourceInfo, ResourceType, SchemaType, ScriptInfo,
|
||||
FlowInfo, HubScriptInfo, ResourceInfo, ResourceType, SchemaType, ScriptInfo, WorkspaceInfo,
|
||||
};
|
||||
use windmill_mcp::server::{BackendResult, EndpointTool, ErrorData, McpBackend};
|
||||
|
||||
use crate::auth::AuthCache;
|
||||
use crate::db::ApiAuthed;
|
||||
use crate::jobs::{
|
||||
run_wait_result_flow_by_path_internal, run_wait_result_script_by_path_internal, RunJobQuery,
|
||||
@@ -31,7 +32,8 @@ use std::time::Duration;
|
||||
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use windmill_mcp::server::{
|
||||
LocalSessionManager, Runner, StreamableHttpServerConfig, StreamableHttpService,
|
||||
LocalSessionManager, McpToken, MultiWorkspaceMcp, Runner, StreamableHttpServerConfig,
|
||||
StreamableHttpService,
|
||||
};
|
||||
use windmill_mcp::WorkspaceId;
|
||||
|
||||
@@ -53,11 +55,17 @@ pub struct WindmillBackend {
|
||||
pub db: DB,
|
||||
pub user_db: UserDB,
|
||||
pub base_internal_url: String,
|
||||
pub auth_cache: Arc<AuthCache>,
|
||||
}
|
||||
|
||||
impl WindmillBackend {
|
||||
pub fn new(db: DB, user_db: UserDB, base_internal_url: String) -> Self {
|
||||
Self { db, user_db, base_internal_url }
|
||||
pub fn new(
|
||||
db: DB,
|
||||
user_db: UserDB,
|
||||
base_internal_url: String,
|
||||
auth_cache: Arc<AuthCache>,
|
||||
) -> Self {
|
||||
Self { db, user_db, base_internal_url, auth_cache }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -305,6 +313,8 @@ impl McpBackend for WindmillBackend {
|
||||
args_map,
|
||||
&endpoint_tool.body_schema,
|
||||
&endpoint_tool.body_field_renames,
|
||||
&endpoint_tool.path_params_schema,
|
||||
&endpoint_tool.query_params_schema,
|
||||
);
|
||||
|
||||
// Create and execute request
|
||||
@@ -338,6 +348,57 @@ impl McpBackend for WindmillBackend {
|
||||
}
|
||||
}
|
||||
|
||||
async fn list_accessible_workspaces(
|
||||
&self,
|
||||
auth: &ApiAuthed,
|
||||
) -> BackendResult<Vec<WorkspaceInfo>> {
|
||||
// A superadmin can act in every workspace and often has no explicit `usr`
|
||||
// membership row (matching resolve_workspace_auth, which authorizes any
|
||||
// workspace for a superadmin), so list them all. Everyone else is limited
|
||||
// to the workspaces they are a member of.
|
||||
let workspaces = if auth.is_admin {
|
||||
sqlx::query_as!(
|
||||
WorkspaceInfo,
|
||||
"SELECT id, name FROM workspace WHERE deleted = false ORDER BY name",
|
||||
)
|
||||
.fetch_all(&self.db)
|
||||
.await
|
||||
} else {
|
||||
sqlx::query_as!(
|
||||
WorkspaceInfo,
|
||||
"SELECT workspace.id, workspace.name
|
||||
FROM workspace
|
||||
JOIN usr ON usr.workspace_id = workspace.id
|
||||
WHERE usr.email = $1 AND usr.disabled = false AND workspace.deleted = false
|
||||
ORDER BY workspace.name",
|
||||
auth.email,
|
||||
)
|
||||
.fetch_all(&self.db)
|
||||
.await
|
||||
};
|
||||
|
||||
workspaces.map_err(|e| ErrorData::internal_error(e.to_string(), None))
|
||||
}
|
||||
|
||||
async fn resolve_workspace_auth(
|
||||
&self,
|
||||
token: &str,
|
||||
workspace_id: &str,
|
||||
) -> BackendResult<ApiAuthed> {
|
||||
self.auth_cache
|
||||
.get_authed(Some(workspace_id.to_string()), token)
|
||||
.await
|
||||
.ok_or_else(|| {
|
||||
ErrorData::invalid_params(
|
||||
format!(
|
||||
"Access denied: token owner is not a member of workspace '{}'",
|
||||
workspace_id
|
||||
),
|
||||
None,
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
fn all_endpoint_tools(&self) -> Vec<EndpointTool> {
|
||||
all_tools()
|
||||
}
|
||||
@@ -401,37 +462,67 @@ pub async fn add_www_authenticate_header(
|
||||
}
|
||||
}
|
||||
|
||||
/// Middleware for gateway: extract workspace_id from the Bearer token in the DB
|
||||
/// and inject it as WorkspaceId extension so the MCP runner can use it.
|
||||
/// Extract the bearer token from either the `Authorization` header or the
|
||||
/// `?token=` query parameter (MCP clients commonly pass it in the URL).
|
||||
fn extract_gateway_token(request: &Request<axum::body::Body>) -> Option<String> {
|
||||
if let Some(token) = request
|
||||
.headers()
|
||||
.get(axum::http::header::AUTHORIZATION)
|
||||
.and_then(|v| v.to_str().ok())
|
||||
.and_then(|h| h.strip_prefix("Bearer "))
|
||||
{
|
||||
return Some(token.to_string());
|
||||
}
|
||||
request.uri().query().and_then(|q| {
|
||||
url::form_urlencoded::parse(q.as_bytes())
|
||||
.find(|(k, _)| k == "token")
|
||||
.map(|(_, v)| v.into_owned())
|
||||
})
|
||||
}
|
||||
|
||||
/// Middleware for gateway: resolve the MCP session mode from the Bearer token in
|
||||
/// the DB. A token bound to a workspace injects `WorkspaceId` (single-workspace
|
||||
/// mode). A workspace-less MCP token (`workspace_id IS NULL` with an `mcp:` scope)
|
||||
/// injects `MultiWorkspaceMcp` + `McpToken`, putting the runner in
|
||||
/// multi-workspace mode where tools take an explicit `workspace_id` argument.
|
||||
pub async fn extract_workspace_from_token(
|
||||
Extension(db): Extension<DB>,
|
||||
mut request: Request<axum::body::Body>,
|
||||
next: Next,
|
||||
) -> Response {
|
||||
if let Some(auth_header) = request
|
||||
.headers()
|
||||
.get(axum::http::header::AUTHORIZATION)
|
||||
.and_then(|v| v.to_str().ok())
|
||||
{
|
||||
if let Some(token) = auth_header.strip_prefix("Bearer ") {
|
||||
let t_hash = hash_token(token);
|
||||
match sqlx::query_scalar!(
|
||||
"SELECT workspace_id FROM token WHERE token_hash = $1 AND workspace_id IS NOT NULL AND (expiration > NOW() OR expiration IS NULL)",
|
||||
t_hash
|
||||
)
|
||||
.fetch_optional(&db)
|
||||
.await
|
||||
{
|
||||
Ok(Some(Some(workspace_id))) => {
|
||||
if let Some(token) = extract_gateway_token(&request) {
|
||||
let t_hash = hash_token(&token);
|
||||
match sqlx::query!(
|
||||
"SELECT workspace_id, scopes FROM token WHERE token_hash = $1 AND (expiration > NOW() OR expiration IS NULL)",
|
||||
t_hash
|
||||
)
|
||||
.fetch_optional(&db)
|
||||
.await
|
||||
{
|
||||
Ok(Some(row)) => match row.workspace_id {
|
||||
Some(workspace_id) => {
|
||||
request
|
||||
.extensions_mut()
|
||||
.insert(GatewayWorkspaceId(workspace_id.clone()));
|
||||
request.extensions_mut().insert(WorkspaceId(workspace_id));
|
||||
}
|
||||
Ok(_) => {}
|
||||
Err(e) => {
|
||||
tracing::error!("Gateway token workspace lookup failed: {}", e);
|
||||
None => {
|
||||
// Only enter multi-workspace mode for genuine MCP tokens; a
|
||||
// full-privilege global token without mcp scope is rejected
|
||||
// by the runner's mcp-scope check anyway.
|
||||
let is_mcp = row
|
||||
.scopes
|
||||
.as_deref()
|
||||
.is_some_and(|s| s.iter().any(|scope| scope.starts_with("mcp:")));
|
||||
if is_mcp {
|
||||
request.extensions_mut().insert(MultiWorkspaceMcp);
|
||||
request.extensions_mut().insert(McpToken(token));
|
||||
}
|
||||
}
|
||||
},
|
||||
Ok(None) => {}
|
||||
Err(e) => {
|
||||
tracing::error!("Gateway token workspace lookup failed: {}", e);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -472,11 +563,12 @@ pub async fn setup_mcp_server(
|
||||
db: DB,
|
||||
user_db: UserDB,
|
||||
base_internal_url: String,
|
||||
auth_cache: Arc<AuthCache>,
|
||||
) -> anyhow::Result<(Router, CancellationToken)> {
|
||||
let cancellation_token = CancellationToken::new();
|
||||
let session_manager = Arc::new(LocalSessionManager::default());
|
||||
|
||||
let backend = WindmillBackend::new(db, user_db, base_internal_url);
|
||||
let backend = WindmillBackend::new(db, user_db, base_internal_url, auth_cache);
|
||||
let runner = Runner::new(backend);
|
||||
|
||||
let service_config = StreamableHttpServerConfig {
|
||||
|
||||
@@ -412,12 +412,50 @@ pub fn build_request_body(
|
||||
args_map: &serde_json::Map<String, Value>,
|
||||
body_schema: &Option<Value>,
|
||||
body_field_renames: &Option<Value>,
|
||||
path_params_schema: &Option<Value>,
|
||||
query_params_schema: &Option<Value>,
|
||||
) -> Option<Value> {
|
||||
if method == "GET" {
|
||||
return None;
|
||||
}
|
||||
|
||||
let schema = body_schema.as_ref()?;
|
||||
|
||||
let has_declared_props = schema
|
||||
.get("properties")
|
||||
.and_then(|p| p.as_object())
|
||||
.map(|o| !o.is_empty())
|
||||
.unwrap_or(false);
|
||||
|
||||
// Pass-through body: the schema declares no explicit properties (e.g.
|
||||
// runScriptByPath / runFlowByPath, whose body is `additionalProperties: true`
|
||||
// and carries the script/flow arguments verbatim). Forward every argument
|
||||
// that isn't already consumed by a path or query parameter — without this the
|
||||
// request body would be empty and parameterized runs would lose their args.
|
||||
if !has_declared_props {
|
||||
if schema.get("type").and_then(|t| t.as_str()) != Some("object") {
|
||||
return None;
|
||||
}
|
||||
let consumed: std::collections::HashSet<&str> = [path_params_schema, query_params_schema]
|
||||
.into_iter()
|
||||
.filter_map(|s| s.as_ref())
|
||||
.filter_map(|s| s.get("properties").and_then(|p| p.as_object()))
|
||||
.flat_map(|props| props.keys().map(|k| k.as_str()))
|
||||
.collect();
|
||||
|
||||
let body_map: serde_json::Map<String, Value> = args_map
|
||||
.iter()
|
||||
.filter(|(k, v)| !consumed.contains(k.as_str()) && !v.is_null())
|
||||
.map(|(k, v)| (k.clone(), v.clone()))
|
||||
.collect();
|
||||
|
||||
return if body_map.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(Value::Object(body_map))
|
||||
};
|
||||
}
|
||||
|
||||
let props = schema.get("properties")?.as_object()?;
|
||||
|
||||
let body_map: serde_json::Map<String, Value> = props
|
||||
@@ -540,6 +578,65 @@ mod tests {
|
||||
use super::*;
|
||||
use serde_json::json;
|
||||
|
||||
#[test]
|
||||
fn build_request_body_passthrough_forwards_script_args_minus_path() {
|
||||
// runScriptByPath-shaped body: additionalProperties, no declared props.
|
||||
// `path` is a path param and must be excluded; the rest are the script's
|
||||
// arguments and must be forwarded verbatim.
|
||||
let body_schema = Some(json!({ "type": "object", "additionalProperties": true }));
|
||||
let path_schema = Some(json!({
|
||||
"type": "object",
|
||||
"properties": { "path": { "type": "string" } },
|
||||
"required": ["path"]
|
||||
}));
|
||||
let args: serde_json::Map<String, Value> = json!({
|
||||
"path": "u/admin/my_script",
|
||||
"name": "alice",
|
||||
"count": 3
|
||||
})
|
||||
.as_object()
|
||||
.unwrap()
|
||||
.clone();
|
||||
|
||||
let body = build_request_body("POST", &args, &body_schema, &None, &path_schema, &None)
|
||||
.expect("passthrough body should be built");
|
||||
let obj = body.as_object().unwrap();
|
||||
assert_eq!(obj.get("name"), Some(&json!("alice")));
|
||||
assert_eq!(obj.get("count"), Some(&json!(3)));
|
||||
assert!(
|
||||
!obj.contains_key("path"),
|
||||
"path param must be excluded from body"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_request_body_declared_props_only_forwards_declared() {
|
||||
// Endpoints with explicit properties keep the strict declared-only behavior.
|
||||
let body_schema = Some(json!({
|
||||
"type": "object",
|
||||
"properties": { "value": { "type": "string" } },
|
||||
"required": ["value"]
|
||||
}));
|
||||
let args: serde_json::Map<String, Value> = json!({ "value": "x", "sneaky": "y" })
|
||||
.as_object()
|
||||
.unwrap()
|
||||
.clone();
|
||||
let body = build_request_body("POST", &args, &body_schema, &None, &None, &None).unwrap();
|
||||
let obj = body.as_object().unwrap();
|
||||
assert_eq!(obj.get("value"), Some(&json!("x")));
|
||||
assert!(
|
||||
!obj.contains_key("sneaky"),
|
||||
"undeclared args must be dropped"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_request_body_get_has_no_body() {
|
||||
let body_schema = Some(json!({ "type": "object", "additionalProperties": true }));
|
||||
let args: serde_json::Map<String, Value> = json!({ "a": 1 }).as_object().unwrap().clone();
|
||||
assert!(build_request_body("GET", &args, &body_schema, &None, &None, &None).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_path_param_value_accepts_legitimate_windmill_paths() {
|
||||
for ok in [
|
||||
|
||||
Reference in New Issue
Block a user