mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-13 00:03:00 +00:00
fix: scrub the request target from mcp error bodies
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
31f230af21
commit
816cb273e8
@@ -9,7 +9,7 @@ use rmcp::{
|
||||
},
|
||||
};
|
||||
use sse_stream::Sse;
|
||||
use tracing::{instrument::WithSubscriber, subscriber::NoSubscriber};
|
||||
use tracing::{instrument::WithSubscriber, Dispatch};
|
||||
|
||||
type Error = StreamableHttpError<reqwest::Error>;
|
||||
|
||||
@@ -20,13 +20,35 @@ type Error = StreamableHttpError<reqwest::Error>;
|
||||
#[derive(Clone)]
|
||||
pub(super) struct UrlRedactingClient(pub(super) reqwest::Client);
|
||||
|
||||
async fn redacted<T>(request: impl Future<Output = Result<T, Error>>) -> Result<T, Error> {
|
||||
match request.with_subscriber(NoSubscriber::default()).await {
|
||||
async fn redacted<T>(
|
||||
uri: Arc<str>,
|
||||
request: impl Future<Output = Result<T, Error>>,
|
||||
) -> Result<T, Error> {
|
||||
match request.with_subscriber(Dispatch::none()).await {
|
||||
Err(StreamableHttpError::Client(e)) => Err(StreamableHttpError::Client(e.without_url())),
|
||||
Err(StreamableHttpError::UnexpectedServerResponse(body)) => {
|
||||
Err(StreamableHttpError::UnexpectedServerResponse(
|
||||
without_request_target(&uri, &body).into(),
|
||||
))
|
||||
}
|
||||
r => r,
|
||||
}
|
||||
}
|
||||
|
||||
/// The body of an error response is the server's to write, and a common one
|
||||
/// echoes the request target (`Cannot POST /mcp/<key>`).
|
||||
fn without_request_target(uri: &str, body: &str) -> String {
|
||||
let mut body = body.replace(uri, "<url>");
|
||||
if let Ok(url) = reqwest::Url::parse(uri) {
|
||||
for part in [Some(url.path()).filter(|p| p.len() > 1), url.query()] {
|
||||
if let Some(part) = part.filter(|p| !p.is_empty()) {
|
||||
body = body.replace(part, "<redacted>");
|
||||
}
|
||||
}
|
||||
}
|
||||
body
|
||||
}
|
||||
|
||||
impl StreamableHttpClient for UrlRedactingClient {
|
||||
type Error = reqwest::Error;
|
||||
|
||||
@@ -39,6 +61,7 @@ impl StreamableHttpClient for UrlRedactingClient {
|
||||
custom_headers: HashMap<HeaderName, HeaderValue>,
|
||||
) -> Result<StreamableHttpPostResponse, Error> {
|
||||
redacted(
|
||||
uri.clone(),
|
||||
self.0
|
||||
.post_message(uri, message, session_id, auth_header, custom_headers),
|
||||
)
|
||||
@@ -54,14 +77,17 @@ impl StreamableHttpClient for UrlRedactingClient {
|
||||
custom_headers: HashMap<HeaderName, HeaderValue>,
|
||||
max_sse_event_size: usize,
|
||||
) -> Result<StreamableHttpPostResponse, Error> {
|
||||
redacted(self.0.post_message_with_max_sse_event_size(
|
||||
uri,
|
||||
message,
|
||||
session_id,
|
||||
auth_header,
|
||||
custom_headers,
|
||||
max_sse_event_size,
|
||||
))
|
||||
redacted(
|
||||
uri.clone(),
|
||||
self.0.post_message_with_max_sse_event_size(
|
||||
uri,
|
||||
message,
|
||||
session_id,
|
||||
auth_header,
|
||||
custom_headers,
|
||||
max_sse_event_size,
|
||||
),
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
@@ -73,6 +99,7 @@ impl StreamableHttpClient for UrlRedactingClient {
|
||||
custom_headers: HashMap<HeaderName, HeaderValue>,
|
||||
) -> Result<(), Error> {
|
||||
redacted(
|
||||
uri.clone(),
|
||||
self.0
|
||||
.delete_session(uri, session_id, auth_header, custom_headers),
|
||||
)
|
||||
@@ -88,6 +115,7 @@ impl StreamableHttpClient for UrlRedactingClient {
|
||||
custom_headers: HashMap<HeaderName, HeaderValue>,
|
||||
) -> Result<BoxStream<'static, Result<Sse, SseError>>, Error> {
|
||||
redacted(
|
||||
uri.clone(),
|
||||
self.0
|
||||
.get_stream(uri, session_id, last_event_id, auth_header, custom_headers),
|
||||
)
|
||||
@@ -103,14 +131,42 @@ impl StreamableHttpClient for UrlRedactingClient {
|
||||
custom_headers: HashMap<HeaderName, HeaderValue>,
|
||||
max_sse_event_size: usize,
|
||||
) -> Result<BoxStream<'static, Result<Sse, SseError>>, Error> {
|
||||
redacted(self.0.get_stream_with_max_sse_event_size(
|
||||
uri,
|
||||
session_id,
|
||||
last_event_id,
|
||||
auth_header,
|
||||
custom_headers,
|
||||
max_sse_event_size,
|
||||
))
|
||||
redacted(
|
||||
uri.clone(),
|
||||
self.0.get_stream_with_max_sse_event_size(
|
||||
uri,
|
||||
session_id,
|
||||
last_event_id,
|
||||
auth_header,
|
||||
custom_headers,
|
||||
max_sse_event_size,
|
||||
),
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::without_request_target;
|
||||
|
||||
#[test]
|
||||
fn error_body_loses_the_request_target() {
|
||||
let uri = "https://mcp.example.com/mcp/KEY?token=T0K";
|
||||
for body in [
|
||||
"Cannot POST /mcp/KEY",
|
||||
"no route for /mcp/KEY?token=T0K",
|
||||
uri,
|
||||
] {
|
||||
let scrubbed = without_request_target(uri, &format!("HTTP 404: {body}"));
|
||||
assert!(
|
||||
!scrubbed.contains("KEY") && !scrubbed.contains("T0K"),
|
||||
"{scrubbed}"
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
without_request_target("https://mcp.example.com/", "a/b"),
|
||||
"a/b"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user