Files
windmill/backend/tests/mcp_token_exfil.rs
hugocasaandClaude Opus 5 3f07a1a803 feat: let the global AI chat call connected MCP servers as the user (#10656)
* feat: let the global AI chat call connected MCP servers as the user

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: address review findings on the chat MCP tools

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat: connect MCP servers from a predefined list in chat and agent steps

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat: show the OAuth redirect URL in the instance connect settings

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: clarify the OAuth redirect URL copy in instance settings

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: match the instance settings warning style and drop the redirect tooltip

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: use the standard warning alert for the redirect url mismatch

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: correct the GitHub token guidance in the MCP registry

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat: warn when an OAuth connect lacks the scopes an MCP server needs

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: request the connect's scopes when the oauth popup is opened directly

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat: connect an oauth-app MCP server without leaving the panel

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: seed connect scopes from the instance config only

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat: make the chat use only the MCP servers you turn on

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* refactor: align the MCP connect UI with the design system

* feat: make a pasted url the default way to connect an mcp server

* feat: show provider icons on the suggested mcp servers

* fix: make both mcp sign-in paths behave the same and stop reloading on toggle

* fix: clarify the mcp tool step's server field and drop its info alert

* fix: name the mcp resource in the tool step and move the transport note into the connect box

* fix: drop the redundant description on the mcp resource field

* fix: make the mcp connections trigger icon-only

* fix: scope enabled mcp servers to the account and address review nits

* fix: wait for connect scopes and create session connections in the operating workspace

* feat: move mcp connections into the chat's plus menu and fix review findings

* fix: show mcp servers as checkboxes so off reads as a state

* feat: give menu rows an on/off switch and use it for mcp servers

* fix: lead the mcp menu rows with the switch

* feat: keep the menu open while toggling and simplify the connect card

* fix: ask for the server before the credential in the connect card

* fix: show one credential path at a time in the connect card

* fix: label the path field and move token guidance into its tooltip

* fix: open straight into connect and keep the server menu scannable

* feat: warn when an mcp connection lands outside your own space

* refactor: require the workspace on the mcp connect components and rename the oauth child

* fix: replace the oauth variable on reconnect and bound every mcp result

* feat: show a connected server's provider icon in the connections list

* feat: resolve mcp provider icons from the url and clarify the path field

* style: align the mcp connect card with the design system surfaces

* style: drop the redundant oauth support line and name the scopes oauth scopes

* feat: keep the mcp connect card open in the connections drawer

* feat: preopen the mcp connect card under the agent step resource picker

* feat: resolve a typed mcp url to its registry entry and describe the token field

* style: name both mcp connect actions connect

* style: name the mcp oauth actions connect with the provider

* style: say in the path description what the connect action will save

* style: name the resource type in the mcp connect path description

* feat: cache mcp provider icons and confirm disconnect in a modal

* fix: keep the mcp menu switches live and the disconnect modal above the drawer

* style: fall back to the plug icon in the mcp menu rows

* fix: never destroy a foreign variable or resource when connecting an mcp server

* fix: prove a token variable is ours before writing it and bound mcp search failures

* fix: pin an mcp oauth popup to the target it was opened for

* fix: bind an mcp credential to the server and popup it was requested for

* fix: bound mcp tool calls with a deadline and drop stale server listings

* fix: keep the disconnect confirmation handler returning void

* fix: tie the mcp tool cache to the resource revision and the grant to its scopes

* fix: verify mcp read-only server-side, keep oauth connector mounted

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-14 18:51:19 +02:00

185 lines
7.3 KiB
Rust

//! Regression test for the MCP token-exfiltration vulnerability.
//!
//! `GET /api/w/{w}/resources/mcp_tools/{path}` builds an MCP client from a
//! resource whose `token` field is a `$var:` reference. Before the fix the token
//! was resolved with `get_secret_value_as_admin` on the bare DB pool — no RLS,
//! no audit — so any workspace member who could read an MCP *resource* could
//! point its token at *any* secret variable in the workspace (e.g. one in an
//! admin-only folder) and have it decrypted and shipped as a bearer token.
//!
//! The fix resolves the token through the caller's permissioned path
//! (`get_value_internal` over the authed `user_db`), so the variable RLS — the
//! same gate as `variables/get_value` — applies and the secret read is audited.
//!
//! This test pins, against the `mcp_token_exfil` fixture:
//! - a plain developer (test-user-3) who can read the MCP resource but has no
//! access to the locked secret is DENIED (401) at token resolution, before
//! any connection is attempted, and the secret never leaks;
//! - an admin (test-user) clears the variable-RLS gate, the token resolves,
//! and the request only fails later at the connect/SSRF step — proving the
//! legitimate path still resolves the token (no over-blocking).
//!
//! `POST .../resources/mcp_call_tool/{path}` reaches the same MCP server through
//! the same resource, so it is pinned to the same property here — both handlers
//! share `connect_mcp_client`, and a future split of that helper must not let
//! one of them regress.
//!
//! SSRF rejection of an author-controlled URL is covered by the unit test in
//! `windmill-mcp` (`from_resource_rejects_ssrf_url`).
#![cfg(feature = "mcp")]
use sqlx::{Pool, Postgres};
use windmill_test_utils::*;
const SECRET_VALUE: &str = "S3CRET-MCP-TOKEN-VALUE";
const RESOURCE_PATH: &str = "u/test-user-3/evil_mcp";
fn client() -> reqwest::Client {
reqwest::Client::new()
}
async fn get(base: &str, path: &str, token: &str) -> (reqwest::StatusCode, String) {
let resp = client()
.get(format!("{base}/{path}"))
.header("Authorization", format!("Bearer {token}"))
.send()
.await
.expect("request");
let status = resp.status();
let body = resp.text().await.expect("body");
(status, body)
}
async fn post(
base: &str,
path: &str,
token: &str,
body: serde_json::Value,
) -> (reqwest::StatusCode, String) {
let resp = client()
.post(format!("{base}/{path}"))
.header("Authorization", format!("Bearer {token}"))
.json(&body)
.send()
.await
.expect("request");
let status = resp.status();
let body = resp.text().await.expect("body");
(status, body)
}
/// Insert the locked secret variable with a real, workspace-key-encrypted value
/// so an authorized read genuinely decrypts it.
async fn insert_locked_secret(db: &Pool<Postgres>) -> anyhow::Result<()> {
let mc = windmill_common::variables::build_crypt(db, "test-workspace").await?;
let encrypted = windmill_common::variables::encrypt(&mc, SECRET_VALUE);
// Runtime-checked query (not the `query!` macro) so no offline `.sqlx` cache
// entry is needed for this test-only insert.
sqlx::query(
"INSERT INTO variable (workspace_id, path, value, is_secret, description, extra_perms)
VALUES ('test-workspace', 'f/locked/secret_token', $1, true, 'Locked secret', '{}')",
)
.bind(&encrypted)
.execute(db)
.await?;
Ok(())
}
#[sqlx::test(fixtures("base", "mcp_token_exfil"))]
async fn test_mcp_token_not_exfiltrated(db: Pool<Postgres>) -> anyhow::Result<()> {
initialize_tracing().await;
insert_locked_secret(&db).await?;
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
let base = format!("http://localhost:{port}/api/w/test-workspace/resources/mcp_tools");
let path = RESOURCE_PATH;
// ---- CORE REGRESSION: the developer can read the resource but must NOT be
// able to resolve the locked secret. They are denied (401) at the
// variable-RLS gate, before any MCP connection is attempted, and the
// secret never appears in the response.
let (status, body) = get(&base, path, "SECRET_TOKEN_3").await;
assert_eq!(
status,
reqwest::StatusCode::UNAUTHORIZED,
"developer must be denied resolving a secret they can't read (got {status}): {body}"
);
assert!(
!body.contains(SECRET_VALUE),
"the locked secret must never leak to the developer: {body}"
);
assert!(
body.contains("don't have access"),
"denial should come from the variable-RLS gate, not a connection error: {body}"
);
// Pre-fix, the token was decrypted as admin and the handler proceeded to the
// connection step; that path must no longer be reached for the developer.
assert!(
!body.contains("Failed to connect to MCP server"),
"developer must be blocked before the connection step (would mean the token was resolved): {body}"
);
// ---- NO OVER-BLOCKING: an admin clears the variable-RLS gate, so the token
// resolves and the request only fails later at the connect/SSRF step.
// A different failure mode (not 401, reaches the connection) proves the
// legitimate read still works.
let (status, body) = get(&base, path, "SECRET_TOKEN").await;
assert_ne!(
status,
reqwest::StatusCode::UNAUTHORIZED,
"admin must clear the variable-RLS gate (got {status}): {body}"
);
assert!(
body.contains("Failed to connect to MCP server"),
"admin should resolve the token and only fail at the connect/SSRF step: {body}"
);
Ok(())
}
#[sqlx::test(fixtures("base", "mcp_token_exfil"))]
async fn test_mcp_call_tool_token_not_exfiltrated(db: Pool<Postgres>) -> anyhow::Result<()> {
initialize_tracing().await;
insert_locked_secret(&db).await?;
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
let base = format!("http://localhost:{port}/api/w/test-workspace/resources/mcp_call_tool");
let body = serde_json::json!({ "tool": "whoami", "arguments": {} });
let (status, resp) = post(&base, RESOURCE_PATH, "SECRET_TOKEN_3", body.clone()).await;
assert_eq!(
status,
reqwest::StatusCode::UNAUTHORIZED,
"developer must be denied resolving a secret they can't read (got {status}): {resp}"
);
assert!(
!resp.contains(SECRET_VALUE),
"the locked secret must never leak to the developer: {resp}"
);
assert!(
resp.contains("don't have access"),
"denial should come from the variable-RLS gate, not a connection error: {resp}"
);
assert!(
!resp.contains("Failed to connect to MCP server"),
"developer must be blocked before the connection step (would mean the token was resolved): {resp}"
);
let (status, resp) = post(&base, RESOURCE_PATH, "SECRET_TOKEN", body).await;
assert_ne!(
status,
reqwest::StatusCode::UNAUTHORIZED,
"admin must clear the variable-RLS gate (got {status}): {resp}"
);
assert!(
resp.contains("Failed to connect to MCP server"),
"admin should resolve the token and only fail at the connect/SSRF step: {resp}"
);
Ok(())
}