Files
windmill/backend/tests/mcp_resource_authz.rs
Ruben Fiszel 317a8629d1 fix(ai): enforce resource authz when loading MCP tools in agent worker (#9571)
* fix(ai): enforce resource authz when loading MCP tools in agent worker

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

* test: describe MCP authz test invariants instead of drafting history

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

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-14 23:50:18 +02:00

103 lines
4.0 KiB
Rust

//! Regression test for the MCP resource-authorization bypass
//! (WIN-2041, GHSA-7qg3-pr4g-cq5x).
//!
//! Invariant: the AI Agent worker (`load_mcp_tools`) must load an MCP resource
//! only when the job identity is allowed to read it — the same
//! `resources:read:{path}` + RLS gate the regular MCP tools API (`get_mcp_tools`)
//! enforces. It loads the resource through the job's permissioned client
//! (`AuthedClient::get_resource_value`, the `resources/get_value` path), not the
//! raw DB pool. Otherwise a low-privileged user who can edit a flow could make
//! the agent load and use an MCP resource (URL + inline headers + token) they are
//! not allowed to read: a confused-deputy bypass.
//!
//! This test pins, against the `mcp_resource_authz` fixture (an MCP resource in
//! a folder only the admin can read):
//! - a plain developer (test-user-3) is DENIED loading the private resource,
//! before any MCP connection is attempted, and the resource never leaks;
//! - an admin (test-user) clears the gate, the resource resolves, and loading
//! only fails later at the connect step — proving no over-blocking.
#![cfg(feature = "mcp")]
use sqlx::{Pool, Postgres};
use windmill_common::client::AuthedClient;
use windmill_test_utils::*;
use windmill_worker::{load_mcp_tools, McpResourceConfig};
fn config() -> Vec<McpResourceConfig> {
vec![McpResourceConfig {
resource_path: "$res:f/private/admin_mcp".to_string(),
include_tools: None,
exclude_tools: None,
}]
}
// The Ok variant ((HashMap<_, Arc<McpClient>>, Vec<Tool>)) does not implement
// Debug, so `expect_err` is unavailable — extract the error explicitly.
async fn load_err(db: &Pool<Postgres>, client: &AuthedClient, ctx: &str) -> String {
match load_mcp_tools(db, "test-workspace", config(), client).await {
Ok(_) => panic!("{ctx}"),
Err(e) => e.to_string(),
}
}
#[sqlx::test(fixtures("base", "mcp_resource_authz"))]
async fn test_mcp_resource_not_loaded_without_authorization(
db: Pool<Postgres>,
) -> anyhow::Result<()> {
initialize_tracing().await;
let server = ApiServer::start(db.clone()).await?;
let base_internal_url = format!("http://localhost:{}", server.addr.port());
// ---- CORE REGRESSION: the developer cannot read the private MCP resource,
// so the worker must refuse to load it — before any MCP connection is
// attempted, and without the resource ever leaking.
let dev_client = AuthedClient::new(
base_internal_url.clone(),
"test-workspace".to_string(),
"SECRET_TOKEN_3".to_string(),
None,
);
let msg = load_err(
&db,
&dev_client,
"developer must be denied loading a resource they can't read",
)
.await;
assert!(
msg.contains("don't have access"),
"denial should come from the resource-RLS gate, not a connection error: {msg}"
);
// An unauthorized caller must be blocked before MCP client creation, so no
// resource material (URL, headers, token) is ever loaded for them.
assert!(
!msg.contains("Failed to create MCP client"),
"developer must be blocked before the connection step (would mean the resource was loaded): {msg}"
);
// ---- NO OVER-BLOCKING: an admin clears the resource-RLS gate, so the
// resource resolves and loading only fails later at the connect step.
let admin_client = AuthedClient::new(
base_internal_url,
"test-workspace".to_string(),
"SECRET_TOKEN".to_string(),
None,
);
let msg = load_err(
&db,
&admin_client,
"the fixture URL is non-resolvable, so the connect step must fail",
)
.await;
assert!(
!msg.contains("don't have access"),
"admin must clear the resource-RLS gate, not be denied: {msg}"
);
assert!(
msg.contains("Failed to create MCP client"),
"admin should resolve the resource and only fail at the connect step: {msg}"
);
Ok(())
}