Files
windmill/backend/tests/mcp_header_var_refs.rs
T
Ruben FiszelandClaude Opus 5.5 c09038b106 fix: hold mcp token references to resource reference scopes (#11803)
* fix: require variables:read for the token variable of an mcp resource

A token scoped to resources:read/write on an MCP resource could have the API
decrypt the variable the resource's `token` points at and send it as a bearer
to the resource's URL. The url and header `$var:` references already required
variables:read on the variable; the token reference now does too, checked before
the OAuth refresh touches that variable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: let an mcp resource's scope cover the variables it references

A scoped token reading or calling an MCP resource resolved its token variable
with the resource scope alone, but its url and header variables also needed
variables:read on each. The resource scope now covers all three; RLS still
bounds them to secrets the caller can read. Reverts the token-side check from
the previous commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: scope resource references when they are written, not when they are read

A token scoped to a resource now reads everything that resource references
(`$var:`, `$jsonvar:`, `$res:`), through get_value_interpolated and the MCP
tool routes alike. What makes that safe moves to the write: create, update,
update_value and version restore refuse a scoped token adding a reference it
holds no read scope on. References the stored value already carries are kept,
since whoever put them there was allowed to.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: hold every resource reference writer to its own user's read rights

A resource-scoped reader resolves stored references as itself, so an unscoped
session could plant a reference to a secret its own user cannot read and have
a reader's scoped token resolve it. Adding a reference now also requires the
writer's user to see its target under RLS. A reference to nothing yet stays
allowed, since a promotion pushes resources before the secrets they use.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: vet only variable references on write and keep nested resources scoped

A nested `$res:` brought along the variables of a resource the writer could see
but whose references it could not read, and what it references can change after
it is written. It needs its own read scope again, and the write check covers
only `$var:`/`$jsonvar:`.

An MCP token stored as a bare path resolved as a variable without being vetted
on write. It now needs its own variables:read scope, and the `$var:` prefix is
parsed with strip_prefix, as the write check does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: keep variable scopes on result-cache resources

The worker writes job output to g/results/<hash> straight to the table, so its
references never pass the write check. A token scoped to such a resource still
needs read scope on the variables it references.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: require read rights on kept references and pin result-cache resources to g/results/

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: check resource variable references at read time, with wildcard resource scopes covering matching variables

Returns to the read-time rule: a variable a resource references needs
variables:read on it, unless it is the resource's linked secret or a wildcard
pattern in the token's resources: scopes matches it. The MCP token reference is
now held to that rule like the url and header references.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-16 17:28:29 +02:00

147 lines
5.4 KiB
Rust

//! An MCP resource header whose value is a `$var:` reference reaches the MCP
//! server as the variable's value, and only for a caller allowed to read it.
#![cfg(feature = "mcp")]
use sqlx::{Pool, Postgres};
use tokio::io::AsyncReadExt;
use windmill_common::client::AuthedClient;
use windmill_test_utils::*;
use windmill_worker::{load_mcp_tools, McpResourceConfig};
const SECRET_VALUE: &str = "S3CRET-MCP-HEADER-VALUE";
/// A secret variable and an MCP resource at `url` sending it as a header.
async fn seed(db: &Pool<Postgres>, url: &str) -> anyhow::Result<()> {
let mc = windmill_common::variables::build_crypt(db, "test-workspace").await?;
sqlx::query(
"INSERT INTO variable (workspace_id, path, value, is_secret, description, extra_perms)
VALUES ('test-workspace', 'u/test-user/header_secret', $1, true, '', '{}')",
)
.bind(windmill_common::variables::encrypt(&mc, SECRET_VALUE))
.execute(db)
.await?;
sqlx::query(
"INSERT INTO resource (workspace_id, path, value, resource_type, extra_perms, created_by)
VALUES ('test-workspace', 'u/test-user/mcp', $1, 'mcp', '{}', 'test-user')",
)
.bind(serde_json::json!({
"name": "mcp",
"url": url,
"headers": { "X-Api-Key": "$var:u/test-user/header_secret", "X-Plain": "plain" }
}))
.execute(db)
.await?;
Ok(())
}
#[sqlx::test(fixtures("base"))]
async fn test_mcp_header_var_ref_is_resolved(db: Pool<Postgres>) -> anyhow::Result<()> {
initialize_tracing().await;
std::env::set_var("ALLOW_PRIVATE_MCP_SERVER_URLS", "true");
// Not an MCP server: it captures the first request and hangs up, so loading
// fails at the connect step after the headers were sent.
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await?;
let mcp_port = listener.local_addr()?.port();
let first_request = tokio::spawn(async move {
let (mut socket, _) = listener.accept().await?;
let mut buf = vec![0u8; 16 * 1024];
let n = socket.read(&mut buf).await?;
anyhow::Ok(String::from_utf8_lossy(&buf[..n]).to_lowercase())
});
seed(&db, &format!("http://127.0.0.1:{mcp_port}/mcp")).await?;
let server = ApiServer::start(db.clone()).await?;
let client = AuthedClient::new(
format!("http://localhost:{}", server.addr.port()),
"test-workspace".to_string(),
"SECRET_TOKEN".to_string(),
None,
);
let config = vec![McpResourceConfig {
resource_path: "$res:u/test-user/mcp".to_string(),
include_tools: None,
exclude_tools: None,
}];
let _ = load_mcp_tools(&db, "test-workspace", config, &client).await;
let request = first_request.await??;
assert!(
request.contains(&format!("x-api-key: {}", SECRET_VALUE.to_lowercase())),
"the header must carry the variable's value, not the reference: {request}"
);
assert!(request.contains("x-plain: plain"), "{request}");
Ok(())
}
/// A token scoped to the resource alone must not have the API read the
/// variable a header points at.
#[sqlx::test(fixtures("base"))]
async fn test_mcp_header_var_ref_requires_variable_scope(db: Pool<Postgres>) -> anyhow::Result<()> {
initialize_tracing().await;
seed(&db, "https://mcp.invalid.windmill.test").await?;
sqlx::query(
"INSERT INTO token (token_hash, token_prefix, token, email, label, super_admin, scopes)
VALUES (encode(sha256('RES_ONLY_TOKEN'::bytea), 'hex'), 'RES_ONLY_T', 'RES_ONLY_TOKEN',
'test@windmill.dev', 'resources only', false, '{resources:read}')",
)
.execute(&db)
.await?;
let server = ApiServer::start(db.clone()).await?;
let resp = reqwest::Client::new()
.get(format!(
"http://localhost:{}/api/w/test-workspace/resources/mcp_tools/u/test-user/mcp",
server.addr.port()
))
.header("Authorization", "Bearer RES_ONLY_TOKEN")
.send()
.await?;
let status = resp.status();
let body = resp.text().await?;
assert_eq!(status, reqwest::StatusCode::FORBIDDEN, "{body}");
assert!(
body.contains("variables:read:u/test-user/header_secret"),
"{body}"
);
// The token is held to the same scope, and a wildcard resource scope covers both.
sqlx::query(
"UPDATE resource SET value = jsonb_build_object('name', 'mcp', 'url', value->>'url',
'token', '$var:u/test-user/header_secret') WHERE path = 'u/test-user/mcp'",
)
.execute(&db)
.await?;
for (token, scope, denied) in [
("EXACT_TOKEN", "resources:read:u/test-user/mcp", true),
("WILDCARD_TOKEN", "resources:read:u/test-user/*", false),
] {
sqlx::query(
"INSERT INTO token (token_hash, token_prefix, token, email, label, super_admin, scopes)
VALUES (encode(sha256($1::bytea), 'hex'), left($1, 10), $1,
'test@windmill.dev', 'scoped', false, ARRAY[$2])",
)
.bind(token)
.bind(scope)
.execute(&db)
.await?;
let resp = reqwest::Client::new()
.get(format!(
"http://localhost:{}/api/w/test-workspace/resources/mcp_tools/u/test-user/mcp",
server.addr.port()
))
.bearer_auth(token)
.send()
.await?;
let body = resp.text().await?;
assert_eq!(
body.contains("variables:read:u/test-user/header_secret"),
denied,
"{scope}: {body}"
);
}
Ok(())
}