starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-09-11 08:07:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(mcp): add proper check for mcp routes (#6282)

Browse Source 
* add proper check for mcp routes

* cleaner

* apply to flow

* fix add checks scopes

---------

Co-authored-by: dieriba <dieriba.pro@gmail.com>
This commit is contained in:
centdix
2025-07-28 10:11:36 +00:00
committed by GitHub
co-authored by dieriba
parent 6e8f5a5b00
commit f892f7c9af
5 changed files with 56 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/windmill-api/src/jobs.rs
+13 -8
View File
@@ -4464,6 +4464,9 @@ pub async fn run_wait_result_flow_by_path_get(
#[cfg(feature = "enterprise")]
check_license_key_valid().await?;
let path = flow_path.to_path();
check_scopes(&authed, || format!("jobs:run:flows:{path}"))?;
if method == http::Method::HEAD {
return Ok(Json(serde_json::json!("")).into_response());
}
@@ -4486,7 +4489,7 @@ pub async fn run_wait_result_flow_by_path_get(
.to_args_from_runnable(
&db,
&w_id,
RunnableId::from_flow_path(flow_path.to_path()),
RunnableId::from_flow_path(path),
run_query.skip_preprocessor,
)
.await?;
@@ -4506,12 +4509,15 @@ pub async fn run_wait_result_script_by_path(
#[cfg(feature = "enterprise")]
check_license_key_valid().await?;
let path = script_path.to_path();
check_scopes(&authed, || format!("jobs:run:scripts:{path}"))?;
let args = args
.to_args_from_runnable(
&authed,
&db,
&w_id,
RunnableId::from_script_path(script_path.to_path()),
RunnableId::from_script_path(path),
run_query.skip_preprocessor,
)
.await?;
@@ -4529,14 +4535,11 @@ pub async fn run_wait_result_script_by_path_internal(
w_id: String,
args: PushArgsOwned,
) -> error::Result<Response> {
let script_path = script_path.to_path();
check_scopes(&authed, || format!("jobs:run:scripts:{script_path}"))?;
check_queue_too_long(&db, QUEUE_LIMIT_WAIT_RESULT.or(run_query.queue_limit)).await?;
let mut tx = user_db.clone().begin(&authed).await?;
let (job_payload, tag, delete_after_use, timeout, on_behalf_of) =
script_path_to_payload(script_path, &mut *tx, &w_id, run_query.skip_preprocessor).await?;
script_path_to_payload(script_path.to_path(), &mut *tx, &w_id, run_query.skip_preprocessor).await?;
drop(tx);
let tag = run_query.tag.clone().or(tag);
@@ -4719,12 +4722,15 @@ pub async fn run_wait_result_flow_by_path(
#[cfg(feature = "enterprise")]
check_license_key_valid().await?;
let path = flow_path.to_path();
check_scopes(&authed, || format!("jobs:run:flows:{path}"))?;
let args = args
.to_args_from_runnable(
&authed,
&db,
&w_id,
RunnableId::from_flow_path(flow_path.to_path()),
RunnableId::from_flow_path(path),
run_query.skip_preprocessor,
)
.await?;
@@ -4745,7 +4751,6 @@ pub async fn run_wait_result_flow_by_path_internal(
check_queue_too_long(&db, run_query.queue_limit).await?;
let flow_path = flow_path.to_path();
check_scopes(&authed, || format!("jobs:run:flows:{flow_path}"))?;
let scheduled_for = run_query.get_scheduled_for(&db).await?;
backend/windmill-api/src/mcp.rs
+32 -19
View File
@@ -230,6 +230,20 @@ impl Runner {
Self {}
}
fn check_scopes(authed: &ApiAuthed) -> Result<(), Error> {
let scopes = authed.scopes.as_ref();
if scopes.is_none()
|| scopes
.unwrap()
.iter()
.all(|scope| scope != "mcp:all" && scope != "mcp:favorites" && !scope.starts_with("mcp:hub:"))
{
tracing::error!("Unauthorized: missing mcp scope");
return Err(Error::internal_error("Unauthorized: missing mcp scope".to_string(), None));
}
Ok(())
}
async fn get_item_schema(
path: &str,
user_db: &UserDB,
@@ -854,14 +868,6 @@ impl ServerHandler for Runner {
request: CallToolRequestParam,
context: RequestContext<RoleServer>,
) -> Result<CallToolResult, Error> {
let parse_args = |args_opt: Option<JsonObject>| -> Result<Value, Error> {
args_opt.map(Value::Object).ok_or_else(|| {
Error::invalid_params(
"Missing arguments for tool",
Some(request.name.clone().into()),
)
})
};
let http_parts = context
.extensions
@@ -875,15 +881,22 @@ impl ServerHandler for Runner {
tracing::error!("ApiAuthed Axum extension not found");
Error::internal_error("ApiAuthed Axum extension not found", None)
})?;
Runner::check_scopes(authed)?;
let db = http_parts.extensions.get::<DB>().ok_or_else(|| {
tracing::error!("DB Axum extension not found");
Error::internal_error("DB Axum extension not found", None)
})?;
let user_db = http_parts.extensions.get::<UserDB>().ok_or_else(|| {
tracing::error!("UserDB Axum extension not found");
Error::internal_error("UserDB Axum extension not found", None)
})?;
let args = parse_args(request.arguments)?;
let args = request.arguments.map(Value::Object).ok_or_else(|| {
Error::invalid_params("Missing arguments for tool", Some(request.name.clone().into()))
})?;
let workspace_id = http_parts
.extensions
@@ -1007,6 +1020,13 @@ impl ServerHandler for Runner {
Error::internal_error("http::request::Parts not found", None)
})?;
let authed = http_parts.extensions.get::<ApiAuthed>().ok_or_else(|| {
tracing::error!("ApiAuthed Axum extension not found");
Error::internal_error("ApiAuthed Axum extension not found", None)
})?;
Runner::check_scopes(authed)?;
let db = http_parts.extensions.get::<DB>().ok_or_else(|| {
tracing::error!("DB Axum extension not found");
Error::internal_error("DB Axum extension not found", None)
@@ -1017,11 +1037,6 @@ impl ServerHandler for Runner {
Error::internal_error("UserDB Axum extension not found", None)
})?;
let authed = http_parts.extensions.get::<ApiAuthed>().ok_or_else(|| {
tracing::error!("ApiAuthed Axum extension not found");
Error::internal_error("ApiAuthed Axum extension not found", None)
})?;
let workspace_id = http_parts
.extensions
.get::<WorkspaceId>()
@@ -1031,15 +1046,13 @@ impl ServerHandler for Runner {
})
.map(|w_id| w_id.0.clone())?;
let owned_scope = authed.scopes.as_ref().and_then(|scopes| {
let scopes = authed.scopes.as_ref();
let owned_scope = scopes.and_then(|scopes| {
scopes
.iter()
.find(|scope| scope.starts_with("mcp:") && !scope.contains("hub"))
});
let hub_scope = authed
.scopes
.as_ref()
.and_then(|scopes| scopes.iter().find(|scope| scope.starts_with("mcp:hub")));
let hub_scope = scopes.and_then(|scopes| scopes.iter().find(|scope| scope.starts_with("mcp:hub")));
let scope_type = owned_scope.map_or("all", |scope| {
let parts = scope.split(":").collect::<Vec<&str>>();
parts[1]
backend/windmill-api/src/scopes.rs
+5 -1
View File
@@ -418,7 +418,8 @@ impl ScopeAction {
}
}
pub fn check_route_access(
pub fn
check_route_access(
token_scopes: &[String],
route_path: &str,
http_method: &str,
@@ -428,9 +429,12 @@ pub fn check_route_access(
// Find the domain and kind for this route
let (required_domain, required_kind, route_suffix) = extract_domain_from_route(route_path)?;
// Backward compatibility: MCP handlers expect unusual scope actions: all, favorites, hub.
if required_domain == ScopeDomain::Mcp {
return Ok(());
}
let mut is_scoped_token = false;
// Check if any token scope grants the required access
for scope_str in token_scopes {
backend/windmill-api/src/trigger_helpers.rs
+6 -3
View File
@@ -31,6 +31,7 @@ use crate::{
check_tag_available_for_workspace, delete_job_metadata_after_use, result_to_response,
run_flow_by_path_inner, run_script_by_path_inner, run_wait_result_internal, RunJobQuery,
},
utils::check_scopes,
HTTP_CLIENT,
};
@@ -703,13 +704,15 @@ async fn trigger_script_with_retry_and_error_handler(
error_handler_args: Option<&sqlx::types::Json<HashMap<String, Box<RawValue>>>>,
trigger_path: String,
) -> Result<(Uuid, Option<bool>)> {
#[cfg(feature = "enterprise")]
check_license_key_valid().await?;
check_scopes(&authed, || format!("jobs:run:scripts:{script_path}"))?;
let retry = retry.map(|r| r.0.clone());
let error_handler_path = error_handler_path.map(|p| p.to_string());
let error_handler_args = error_handler_args.map(|args| args.0.clone());
#[cfg(feature = "enterprise")]
check_license_key_valid().await?;
let (job_payload, tag, delete_after_use, timeout, on_behalf_of) = {
let mut tx = user_db.clone().begin(&authed).await?;
script_path_to_payload(script_path, &mut *tx, &workspace_id, Some(false)).await?
backend/windmill-api/src/utils.rs
-4
View File
@@ -56,10 +56,6 @@ where
is_scoped_token = true;
}
if scope.starts_with("mcp:") {
return Ok(());
}
match ScopeDefinition::from_scope_string(scope) {
Ok(scope) if scope.includes(&required_scope) => return Ok(()),
_ => {}