mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(mcp): let MCP tokens call preview run tools (jobs:run scope) — Fixes GIT-920 (#10107)
* fix(mcp): let MCP tokens call preview run tools (jobs:run scope) The MCP proxy mints an internal JWT scoped to exactly `scope_for_route` for the endpoint it forwards to. For preview run routes (`run/preview`, `run/preview_bundle`, `run/preview_flow`, `run_wait_result/preview`, `run_wait_result/preview_flow`), `determine_kind_from_route` matched the `SCRIPT_JOBS` prefix `jobs/run_wait_result/p` (because "preview" starts with "p") and derived `jobs:run:scripts`. But the preview handlers run arbitrary request-supplied code with no deployed path and require the broad `jobs:run` scope, so `jobs:run:scripts` was rejected with 403 "Required scope: jobs:run". Preview/bundle routes now carry no runnable kind, so the derived scope is the broad `jobs:run` the handlers expect. This also aligns the route-level access check with the handler check for these routes. Fixes GIT-920 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): anchor preview-route match to endpoint segment Address CI review: `route_path.contains("preview")` also matched by-path runs of a deployed runnable whose path contains "preview" (e.g. `jobs/run_wait_result/p/f/team/preview_report`). Since determine_kind_from_route also feeds check_route_access, such a route would derive the broad `jobs:run` and reject a legitimately kind-scoped `jobs:run:scripts:*`/`jobs:run:flows:*` token with 403. Anchor the exception to the actual preview endpoints (`jobs/run/preview*`, `jobs/run_wait_result/preview*`) so by-path runs keep their kind. Add regression tests for preview-named by-path paths, and trim the comments per AGENTS.md. 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
ba7f9c065f
commit
4917f79935
@@ -596,6 +596,17 @@ fn map_http_method_to_action(method: &str, route_path: &str) -> ScopeAction {
|
||||
/// Returns `"flows"` or `"scripts"` based on the match, or `None` if no match is found.
|
||||
fn determine_kind_from_route(route_path: &str) -> Option<String> {
|
||||
if route_path.starts_with("jobs") {
|
||||
// Preview/bundle runs execute arbitrary code with no deployed path, so
|
||||
// their handlers require the broad `jobs:run` scope: they must carry no
|
||||
// kind, else the derived scope is narrower than the handler demands.
|
||||
// Anchor to the endpoint segment so by-path runs of a deployed runnable
|
||||
// whose path contains "preview" (e.g. `run/p/f/team/preview_report`) are
|
||||
// still classified by their kind.
|
||||
if route_path.starts_with("jobs/run/preview")
|
||||
|| route_path.starts_with("jobs/run_wait_result/preview")
|
||||
{
|
||||
return None;
|
||||
}
|
||||
if FLOW_JOBS.iter().any(|path| route_path.starts_with(path)) {
|
||||
return Some("flows".to_string());
|
||||
} else if SCRIPT_JOBS.iter().any(|path| route_path.starts_with(path)) {
|
||||
@@ -1296,6 +1307,42 @@ mod tests {
|
||||
Some("jobs:run:flows")
|
||||
);
|
||||
|
||||
// Preview/bundle runs have no deployed path and their handlers require the
|
||||
// broad `jobs:run` scope, so the derived scope must not carry a kind.
|
||||
for path in [
|
||||
"/api/w/ws/jobs/run/preview",
|
||||
"/api/w/ws/jobs/run/preview_bundle",
|
||||
"/api/w/ws/jobs/run/preview_flow",
|
||||
"/api/w/ws/jobs/run_wait_result/preview",
|
||||
"/api/w/ws/jobs/run_wait_result/preview_flow",
|
||||
] {
|
||||
assert_eq!(
|
||||
scope_for_route("POST", path).as_deref(),
|
||||
Some("jobs:run"),
|
||||
"preview route {path} must derive the broad jobs:run scope"
|
||||
);
|
||||
}
|
||||
|
||||
// By-path runs of a deployed runnable whose path contains "preview" must
|
||||
// still derive their kind (not be swept into the broad jobs:run above),
|
||||
// otherwise a `jobs:run:scripts:*`/`jobs:run:flows:*` token is denied.
|
||||
assert_eq!(
|
||||
scope_for_route("POST", "/api/w/ws/jobs/run/p/u/alice/preview_report").as_deref(),
|
||||
Some("jobs:run:scripts")
|
||||
);
|
||||
assert_eq!(
|
||||
scope_for_route(
|
||||
"POST",
|
||||
"/api/w/ws/jobs/run_wait_result/p/f/team/preview_report"
|
||||
)
|
||||
.as_deref(),
|
||||
Some("jobs:run:scripts")
|
||||
);
|
||||
assert_eq!(
|
||||
scope_for_route("POST", "/api/w/ws/jobs/run/f/f/team/preview_report").as_deref(),
|
||||
Some("jobs:run:flows")
|
||||
);
|
||||
|
||||
// The minted scope actually satisfies the route check it targets.
|
||||
let s = scope_for_route("POST", "/api/w/ws/variables/create").unwrap();
|
||||
assert!(check_route_access(&[s], "/api/w/ws/variables/create", "POST").is_ok());
|
||||
|
||||
Reference in New Issue
Block a user