diff --git a/AGENTS.md b/AGENTS.md index 835c8a4edb..ef87f1c4dc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,7 +66,7 @@ Open-source platform for internal tools, workflows, API integrations, background - **Domain vocabulary**: `CONTEXT.md` — the words this codebase uses for its own concepts (step, step setting, trigger step, …). Name things the way it does. - **CLI commands**: when adding/modifying/removing a command, subcommand, option, or description in `cli/src/commands/`, run `python system_prompts/generate.py` to refresh `system_prompts/auto-generated/` and `cli/src/guidance/skills.gen.ts`. The CLI docs the agents use to operate `wmill` are derived from the source — stale generated files give agents the wrong flags. - **AI guidance** (chat prompts and CLI skills): what the AI knows about Windmill itself — how flows, scripts, apps, pipelines and resources work — is written once, in `system_prompts/base/` and `system_prompts/languages/`, and `system_prompts/generate.py` builds both the chat's `$system_prompts` exports and the CLI's skills from it. Add or change such guidance there, never inline in one consumer: a sentence only one side should see goes in a `` / `` block, and a new file reaches both through the `TOPICS` table. The chat prompt builders under `frontend/src/lib/components/copilot/chat/` add only tool plumbing and runtime values, and shared markdown names no chat tool, since it reaches every chat mode. Rerun `generate.py` after editing; `system_prompts/README.md` has the details. -- **Windmill MCP server**: `/api/mcp` gives a token with `mcp:all` the AI session's GLOBAL toolset (`backend/windmill-api/src/mcp/session/`), and a token scoped to a selection of scripts and flows the tools server's per-runnable tools, without endpoint tools. The tools server's own URLs, `/api/mcp/w/{workspace}/mcp` and `/api/mcp/gateway`, keep serving existing tokens, endpoint scopes included. Every tool takes a `workspace_id`; the token bounds which (its bound workspace, the `mcp:workspaces:` list, or all of its owner's). What a model reads there (tool names, descriptions, input schemas, instructions, `get_instructions` texts) is generated from the session's TypeScript into `session/gen/`: after changing a GLOBAL tool definition, the global system prompt or `system_prompts/`, run `npm run gen:mcp-session` from `frontend/` (`mcpSessionGen.test.ts` fails otherwise). What a tool *does* is written twice, in `chat/global/core.ts` and in a Rust handler: a behaviour change on one side must be mirrored on the other, and a new session tool is either implemented in Rust or added to `EDITOR_ONLY` (a `cargo test` in `session/mod.rs` fails otherwise). +- **Windmill MCP server**: `/api/mcp` gives a token with `mcp:all` the AI session's GLOBAL toolset (`backend/windmill-api/src/mcp/session/`), and a token scoped to a selection of scripts and flows the tools server's per-runnable tools, without endpoint tools. The tools server's own URLs, `/api/mcp/w/{workspace}/mcp` and `/api/mcp/gateway`, keep serving existing tokens, endpoint scopes included, except that they give the session toolset to an `mcp:all` token carrying `mcp:session` (added by the OAuth approve, kept across scope edits) or `mcp:read_only`/`mcp:read_run`. Every tool takes a `workspace_id`; the token bounds which (its bound workspace, the `mcp:workspaces:` list, or all of its owner's). What a model reads there (tool names, descriptions, input schemas, instructions, `get_instructions` texts) is generated from the session's TypeScript into `session/gen/`: after changing a GLOBAL tool definition, the global system prompt or `system_prompts/`, run `npm run gen:mcp-session` from `frontend/` (`mcpSessionGen.test.ts` fails otherwise). What a tool *does* is written twice, in `chat/global/core.ts` and in a Rust handler: a behaviour change on one side must be mirrored on the other, and a new session tool is either implemented in Rust or added to `EDITOR_ONLY` (a `cargo test` in `session/mod.rs` fails otherwise). - **Session recorder**: `frontend/src/lib/components/recording/` is also the recorder `wmill app dev --recording` serves, vendored into the CLI as `cli/src/commands/app/devRecorderBundle.gen.ts`. After changing `rawAppSnapshot.ts` or `rawAppRecording.svelte.ts`, run `bun run gen:dev-recorder` from `cli/` (`cli/test/dev_recorder_bundle_unit.test.ts` fails otherwise). - **Raw-app policy**: `frontend/src/lib/components/raw_apps/rawAppPolicy.ts` also derives the policy the server's raw-app deploy stores, vendored into the bundle job as `backend/windmill-api/src/apps_raw_policy.gen.js`. After changing it or anything it imports, run `bun run gen:app-policy` from `cli/` (`cli/test/app_policy_bundle_unit.test.ts` fails otherwise). It rides in the job rather than being read from the CLI the job runs because the images install `windmill-cli` unpinned, so an image can carry one older than its server. diff --git a/backend/windmill-api-auth/src/lib.rs b/backend/windmill-api-auth/src/lib.rs index 82a40377a0..6a09710b5e 100644 --- a/backend/windmill-api-auth/src/lib.rs +++ b/backend/windmill-api-auth/src/lib.rs @@ -591,7 +591,9 @@ pub fn ensure_scopes_within_caller( for requested in requested_restrictions { if requested.starts_with("mcp:") { - if !caller_mcp.contains(requested.as_str()) { + // `mcp:session` (windmill_mcp's SESSION_SCOPE) grants nothing: it only picks + // the toolset an `mcp:all` token gets, and `mcp:all` is matched here itself. + if requested != "mcp:session" && !caller_mcp.contains(requested.as_str()) { return Err(Error::PermissionDenied(format!( "A scope-restricted token cannot grant MCP scope '{requested}' unless the \ caller holds the same scope verbatim" @@ -1964,6 +1966,11 @@ mod tests { opt_scopes(Some(vec!["mcp:scripts:*"])).as_deref() ) .is_err()); + assert!(ensure_scopes_within_caller( + &mcp_all, + opt_scopes(Some(vec!["mcp:all", "mcp:session"])).as_deref() + ) + .is_ok()); } #[test] diff --git a/backend/windmill-api/src/mcp/core.rs b/backend/windmill-api/src/mcp/core.rs index d31234b585..52a6a32014 100644 --- a/backend/windmill-api/src/mcp/core.rs +++ b/backend/windmill-api/src/mcp/core.rs @@ -35,6 +35,7 @@ use std::sync::Arc; use std::time::Duration; use tokio_util::sync::CancellationToken; +use windmill_mcp::common::SESSION_SCOPE; use windmill_mcp::server::{ LocalSessionManager, McpToken, MultiWorkspaceMcp, NoEndpointTools, Runner, SessionRunner, StreamableHttpServerConfig, StreamableHttpService, @@ -503,6 +504,17 @@ fn scoped_to_selection(scopes: &[String]) -> bool { mcp.clone().next().is_some() && !mcp.any(|scope| scope == "mcp:all") } +/// Whether the tools server's URLs (`/api/mcp/gateway`, `/api/mcp/w/{workspace}/mcp`) serve +/// a token the session's toolset: an `mcp:all` grant marked `mcp:session`, or narrowed by +/// `mcp:read_only` / `mcp:read_run`, which the consent page describes as session access. +/// Every other token there keeps the tools server it was issued for. +fn serves_session_toolset(scopes: &[String]) -> bool { + scopes.iter().any(|scope| scope == "mcp:all") + && scopes + .iter() + .any(|scope| [SESSION_SCOPE, "mcp:read_only", "mcp:read_run"].contains(&scope.as_str())) +} + /// The path of a workspace-less MCP URL, when it is not the tools server's /// `/api/mcp/gateway`: each URL is its own OAuth protected resource. #[derive(Clone, Copy)] @@ -519,6 +531,16 @@ pub async fn extract_and_store_workspace_id( next.run(request).await } +/// RFC 9728 challenge pointing at the endpoint's protected resource metadata. `scope` is what +/// a client requests: without it, Claude requests every scope the metadata lists, read-only +/// included, and the consent page would then default to read-only access. +fn www_authenticate_value(base_url: &str, endpoint: &str) -> String { + format!( + "Bearer resource_metadata=\"{base_url}/.well-known/oauth-protected-resource{endpoint}\", \ + scope=\"mcp:all\"" + ) +} + /// Middleware that adds WWW-Authenticate header to 401 responses /// This helps MCP clients discover the OAuth authorization server (RFC 9728) pub async fn add_www_authenticate_header( @@ -545,11 +567,8 @@ pub async fn add_www_authenticate_header( if response.status() == StatusCode::UNAUTHORIZED { let base_url = BASE_URL.load(); - // RFC 9728: The resource parameter contains the protected resource URL. - // Clients derive the metadata URL by inserting /.well-known/oauth-protected-resource - // after the host, e.g., http://host/.well-known/oauth-protected-resource/api/mcp/w/test/mcp - let resource_url = format!("{}/api/mcp/w/{}/mcp", base_url, workspace_id); - let www_authenticate = format!("Bearer resource=\"{}\"", resource_url); + let www_authenticate = + www_authenticate_value(&base_url, &format!("/api/mcp/w/{}/mcp", workspace_id)); // Reconstruct response with the new header let (mut parts, body) = response.into_parts(); @@ -678,8 +697,7 @@ pub async fn add_www_authenticate_header_gateway( if response.status() == StatusCode::UNAUTHORIZED { let base_url = BASE_URL.load(); - let resource_url = format!("{}{}", base_url, endpoint); - let www_authenticate = format!("Bearer resource=\"{}\"", resource_url); + let www_authenticate = www_authenticate_value(&base_url, endpoint); let (mut parts, body) = response.into_parts(); parts.headers.insert( @@ -735,7 +753,30 @@ pub async fn setup_mcp_server( let service = StreamableHttpService::new(move || Ok(runner.clone()), session_manager, service_config); - let router = Router::new().route_service("/", service.clone()); + let router = Router::new().route( + "/", + axum::routing::any({ + let (service, session_service) = (service.clone(), session_service.clone()); + move |request: Request| { + let (service, session_service) = (service.clone(), session_service.clone()); + async move { + use tower::ServiceExt; + let session = request.extensions().get::().is_some_and(|a| { + serves_session_toolset(a.scopes.as_deref().unwrap_or_default()) + }); + let response = if session { + session_service.oneshot(request).await + } else { + service.oneshot(request).await + }; + match response { + Ok(response) => response.into_response(), + Err(never) => match never {}, + } + } + } + }), + ); // `/api/mcp` serves both servers: a token scoped to a selection of scripts, flows and // endpoints gets the tools server, any other the session's toolset. At `/mcp` itself, // beside the `/mcp/...` routes, so it is merged rather than nested. @@ -810,7 +851,7 @@ pub fn toolset_service() -> Router { #[cfg(test)] mod tests { - use super::scoped_to_selection; + use super::{scoped_to_selection, serves_session_toolset}; #[test] fn only_a_token_without_mcp_all_is_a_selection() { @@ -829,4 +870,17 @@ mod tests { assert!(!selection(&[])); assert!(!selection(&["variables:read"])); } + + #[test] + fn tools_server_urls_keep_unmarked_tokens() { + let session = |scopes: &[&str]| { + serves_session_toolset(&scopes.iter().map(|s| s.to_string()).collect::>()) + }; + assert!(session(&["mcp:all", "mcp:session"])); + assert!(session(&["mcp:all", "mcp:read_only"])); + assert!(session(&["mcp:all", "mcp:read_run", "mcp:workspaces:a,b"])); + assert!(!session(&["mcp:all"])); + assert!(!session(&["mcp:favorites", "mcp:session"])); + assert!(!session(&["mcp:all:f/x/*", "mcp:read_only"])); + } } diff --git a/backend/windmill-api/src/mcp/oauth_server.rs b/backend/windmill-api/src/mcp/oauth_server.rs index de07647305..677875da96 100644 --- a/backend/windmill-api/src/mcp/oauth_server.rs +++ b/backend/windmill-api/src/mcp/oauth_server.rs @@ -19,7 +19,7 @@ use windmill_common::{ use crate::db::ApiAuthed; use windmill_api_auth::forbid_elevated_job_token; -use windmill_mcp::parse_mcp_scopes; +use windmill_mcp::{common::SESSION_SCOPE, parse_mcp_scopes}; /// Token expiration for MCP OAuth tokens (1 week in seconds) const MCP_OAUTH_TOKEN_EXPIRATION_SECS: u64 = 7 * 24 * 60 * 60; @@ -225,17 +225,25 @@ struct RefreshTokenRow { revoked: bool, } -fn supported_scopes() -> Vec { - vec![ - "mcp:all".to_string(), - "mcp:favorites".to_string(), - "mcp:scripts:*".to_string(), - "mcp:flows:*".to_string(), - "mcp:endpoints:*".to_string(), - "mcp:read_only".to_string(), - "mcp:read_run".to_string(), - "mcp:runnables_as_tools".to_string(), +/// The scopes that grant access. A protected resource advertises only these: MCP clients +/// that request every listed scope must not pick up a narrowing, which would make the +/// consent page start from a narrower access than they meant to ask for. +fn granting_scopes() -> Vec { + [ + "mcp:all", + "mcp:favorites", + "mcp:scripts:*", + "mcp:flows:*", + "mcp:endpoints:*", ] + .map(String::from) + .to_vec() +} + +fn supported_scopes() -> Vec { + let mut scopes = granting_scopes(); + scopes.extend(["mcp:read_only", "mcp:read_run", "mcp:runnables_as_tools"].map(String::from)); + scopes } /// GET /.well-known/oauth-authorization-server/api/w/:workspace_id/mcp/oauth/server @@ -263,7 +271,7 @@ fn build_protected_resource_metadata( ProtectedResourceMetadata { resource: format!("{}{}", base_url, resource_path), authorization_servers: vec![format!("{}{}", base_url, oauth_prefix)], - scopes_supported: Some(supported_scopes()), + scopes_supported: Some(granting_scopes()), bearer_methods_supported: Some(vec!["header".to_string()]), } } @@ -873,11 +881,14 @@ async fn oauth_approve_inner( let code = format!("mcp-code-{}", rd_string(32)); - let scopes: Vec = form + let mut scopes: Vec = form .scope .split_whitespace() .map(|s| s.to_string()) .collect(); + if scopes.iter().any(|s| s == "mcp:all") && !scopes.iter().any(|s| s == SESSION_SCOPE) { + scopes.push(SESSION_SCOPE.to_string()); + } // The approver's own token bounds what it may grant: a scope-restricted MCP // token must not approve a broader one (e.g. mcp:scripts:f/x -> mcp:all). An diff --git a/backend/windmill-mcp/src/common/mod.rs b/backend/windmill-mcp/src/common/mod.rs index 2632a28962..6707943482 100644 --- a/backend/windmill-mcp/src/common/mod.rs +++ b/backend/windmill-mcp/src/common/mod.rs @@ -9,7 +9,7 @@ pub mod transform; pub mod types; pub use schema::convert_schema_to_schema_type; -pub use scope::{is_resource_allowed, parse_mcp_scopes, McpScopeConfig}; +pub use scope::{is_resource_allowed, parse_mcp_scopes, McpScopeConfig, SESSION_SCOPE}; pub use transform::{ apply_key_transformation, extract_hub_version_id_from_hashed, extract_path_prefix_from_hashed, parse_tool_prefix, reverse_transform, reverse_transform_key, transform_hub_path, diff --git a/backend/windmill-mcp/src/common/scope.rs b/backend/windmill-mcp/src/common/scope.rs index 42327215ae..642c02a2bc 100644 --- a/backend/windmill-mcp/src/common/scope.rs +++ b/backend/windmill-mcp/src/common/scope.rs @@ -161,6 +161,11 @@ fn pattern_covers(caller: &str, requested: &str) -> bool { /// URL, which an older selection, carrying none of the newer scopes, still does. pub const RUNNABLES_AS_TOOLS_SCOPE: &str = "mcp:runnables_as_tools"; +/// Added server-side to an `mcp:all` OAuth grant so the tools server's URLs serve it the AI +/// session's toolset. Grants nothing: an un-narrowed `mcp:all` token without it predates the +/// session toolset there, and must keep the tools server's endpoint tools. +pub const SESSION_SCOPE: &str = "mcp:session"; + /// Parse MCP scopes from token scope strings pub fn parse_mcp_scopes(scopes: &[String]) -> Result { let mut config = McpScopeConfig::default(); @@ -233,6 +238,10 @@ pub fn parse_mcp_scopes(scopes: &[String]) -> Result { continue; } + if scope == SESSION_SCOPE { + continue; + } + if let Some(ids) = scope.strip_prefix("mcp:workspaces:") { config .workspaces diff --git a/frontend/src/lib/components/mcp/McpAccessPicker.svelte b/frontend/src/lib/components/mcp/McpAccessPicker.svelte index c80ab00ed2..6466bb8761 100644 --- a/frontend/src/lib/components/mcp/McpAccessPicker.svelte +++ b/frontend/src/lib/components/mcp/McpAccessPicker.svelte @@ -4,6 +4,10 @@ /** Marks a selection, so the per-workspace URLs add no API endpoint tools to it. */ export const RUNNABLES_AS_TOOLS = 'mcp:runnables_as_tools' + /** Added by the server to an `mcp:all` sign-in so the tools server's URLs serve it the AI + * session's tools. Grants nothing, so it is never shown. */ + export const MCP_SESSION = 'mcp:session' + const ACCESS_SCOPES: Record, string[]> = { read: ['mcp:all', 'mcp:read_only'], read_run: ['mcp:all', 'mcp:read_run'], @@ -13,7 +17,7 @@ /** The access a token's MCP scopes give: `mcp:all` and how it is narrowed, or else a * selection of scripts, flows and endpoints. */ export function mcpAccessOf(scopes: string[]): McpAccess { - scopes = scopes.filter((s) => s !== RUNNABLES_AS_TOOLS) + scopes = scopes.filter((s) => s !== RUNNABLES_AS_TOOLS && s !== MCP_SESSION) if (!scopes.includes('mcp:all')) return scopes.length > 0 ? 'selected' : 'full' if (scopes.includes('mcp:read_only')) return 'read' if (scopes.includes('mcp:read_run')) return 'read_run' @@ -50,7 +54,7 @@ access = initialAccess const initialSelection = initialAccess === 'selected' - ? initialScopes.filter((s) => s !== RUNNABLES_AS_TOOLS).join(' ') + ? initialScopes.filter((s) => s !== RUNNABLES_AS_TOOLS && s !== MCP_SESSION).join(' ') : undefined let selection = $state(initialSelection ?? 'mcp:favorites') diff --git a/frontend/src/lib/components/settings/EditTokenScopesModal.svelte b/frontend/src/lib/components/settings/EditTokenScopesModal.svelte index 7125093749..46ad5b4720 100644 --- a/frontend/src/lib/components/settings/EditTokenScopesModal.svelte +++ b/frontend/src/lib/components/settings/EditTokenScopesModal.svelte @@ -6,7 +6,7 @@ import Modal from '../common/modal/Modal.svelte' import TextInput from '../text_input/TextInput.svelte' import ScopesPicker from './ScopesPicker.svelte' - import McpAccessPicker, { mcpAccessOf } from '../mcp/McpAccessPicker.svelte' + import McpAccessPicker, { MCP_SESSION, mcpAccessOf } from '../mcp/McpAccessPicker.svelte' import { useOperatingWorkspace } from '$lib/components/operatingWorkspace.svelte' const operatingWorkspace = useOperatingWorkspace() @@ -50,10 +50,13 @@ // user has used the picker; until then the token keeps exactly the scopes it had. let mcpTouched = $state(false) // Carried over as is, the picker knowing nothing of them: dropping the workspace list - // would widen the token to every workspace. + // would widen the token to every workspace, dropping `mcp:session` would move a sign-in + // back to the endpoint tools once it is given `mcp:all` again. const isWorkspaceList = (s: string) => s.startsWith('mcp:workspaces:') const keptAside = (s: string) => - isWorkspaceList(s) || (mcpAccessOf(mcpScopes) !== 'selected' && s.startsWith('mcp:hub:')) + isWorkspaceList(s) || + s === MCP_SESSION || + (mcpAccessOf(mcpScopes) !== 'selected' && s.startsWith('mcp:hub:')) const scopesToSave = $derived( isMcp ? mcpTouched diff --git a/frontend/src/lib/components/settings/TokensTable.svelte b/frontend/src/lib/components/settings/TokensTable.svelte index a13c06312b..3cd83e7d89 100644 --- a/frontend/src/lib/components/settings/TokensTable.svelte +++ b/frontend/src/lib/components/settings/TokensTable.svelte @@ -10,6 +10,7 @@ import Badge from '../common/badge/Badge.svelte' import Alert from '../common/alert/Alert.svelte' import { Pen, Trash } from 'lucide-svelte' + import { MCP_SESSION } from '../mcp/McpAccessPicker.svelte' // --- Props --- interface Props { @@ -114,6 +115,10 @@ listTokens() } + function shownScopes(scopes: string[] | null | undefined): string { + return (scopes ?? []).filter((s) => s !== MCP_SESSION).join(', ') + } + function handleEditClick( tokenPrefix: string, tokenLabel: string | undefined, @@ -199,13 +204,13 @@
{#if read_only} Read-only {/if} - {scopes?.join(', ') ?? ''} + {shownScopes(scopes)}
diff --git a/frontend/src/routes/(root)/(logged)/oauth/mcp_authorize/+page@(root).svelte b/frontend/src/routes/(root)/(logged)/oauth/mcp_authorize/+page@(root).svelte index a917de4d4a..3daaeba05c 100644 --- a/frontend/src/routes/(root)/(logged)/oauth/mcp_authorize/+page@(root).svelte +++ b/frontend/src/routes/(root)/(logged)/oauth/mcp_authorize/+page@(root).svelte @@ -20,9 +20,16 @@ let clientName = page.url.searchParams.get('client_name') || 'Unknown Client' let redirectUri = page.url.searchParams.get('redirect_uri') || '' // What the client asked for seeds the access picker; what is approved is the picker's. - const requestedScopes = (page.url.searchParams.get('scope') || 'mcp:all') + const askedScopes = (page.url.searchParams.get('scope') || 'mcp:all') .split(' ') .filter((s) => s.startsWith('mcp:')) + // Both narrowings at once is a client requesting every scope it was offered, not a choice. + const requestedScopes = + askedScopes.includes('mcp:all') && + askedScopes.includes('mcp:read_only') && + askedScopes.includes('mcp:read_run') + ? askedScopes.filter((s) => s !== 'mcp:read_only' && s !== 'mcp:read_run') + : askedScopes let scopes = $state([]) let access = $state() let oauthState = page.url.searchParams.get('state') || ''