From 1c46f899ca03edf62053f4f14d65b4eabff4255d Mon Sep 17 00:00:00 2001
From: hugocasa <hugo@casademont.ch>
Date: Wed, 1 Jul 2026 10:42:42 +0200
Subject: [PATCH] fix(mcp): stop double-escaping string query params in
 build_query_string (#9855)

MCP tool arguments were converted to URL query values via `value.to_string()`
+ `trim_matches('"')`. For string values containing JSON (e.g. the `args`/`result`
filters on job listing, `args` on schedule listing), `to_string()` JSON-encodes the
string and escapes inner quotes with backslashes; stripping the outer quotes leaves
`{\"k\":\"v\"}`, which the backend's `serde_json::from_str` then fails to parse,
falling back to `FALSE` and returning zero results.

Use `value.as_str()` to emit the raw string content for `Value::String`, falling
back to `value.to_string()` for non-string types (numbers, booleans). Adds
regression tests covering JSON-string, non-string, and plain-string params.

Fixes WIN-2114

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 backend/windmill-api/src/mcp/utils.rs | 63 +++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 3 deletions(-)

diff --git a/backend/windmill-api/src/mcp/utils.rs b/backend/windmill-api/src/mcp/utils.rs
index 2a0c34ae8d..2184c1548f 100644
--- a/backend/windmill-api/src/mcp/utils.rs
+++ b/backend/windmill-api/src/mcp/utils.rs
@@ -382,12 +382,18 @@ pub fn build_query_string(
                 .map(|value| {
                     // Use the original name for the query parameter key
                     let original_name = get_original_name(param_name, query_field_renames);
-                    let value_str = value.to_string();
-                    let str_val = value_str.trim_matches('"');
+                    // For string values, use the raw content: to_string() would JSON-encode
+                    // it, and stripping the outer quotes leaves inner quotes backslash-escaped
+                    // (e.g. `{\"k\":\"v\"}`), which breaks downstream JSON parsing of params
+                    // like `args`/`result`. Non-string values keep their JSON serialization.
+                    let str_val = value
+                        .as_str()
+                        .map(|s| s.to_string())
+                        .unwrap_or_else(|| value.to_string());
                     format!(
                         "{}={}",
                         urlencoding::encode(&original_name),
-                        urlencoding::encode(str_val)
+                        urlencoding::encode(&str_val)
                     )
                 })
         })
@@ -624,4 +630,55 @@ mod tests {
         .expect("legitimate path should substitute");
         assert_eq!(result, "/w/dev/scripts/get/p/u/alice/my_script");
     }
+
+    fn single_query_schema(param: &str) -> Option<Value> {
+        Some(json!({
+            "type": "object",
+            "properties": { param: { "type": "string" } }
+        }))
+    }
+
+    #[test]
+    fn build_query_string_preserves_json_string_content() {
+        // A string param carrying JSON (e.g. the `args` filter on listJobs) must be
+        // emitted as its raw content so the backend can `serde_json::from_str` it.
+        let mut args = serde_json::Map::new();
+        args.insert("args".to_string(), json!("{\"key\":\"val\"}"));
+
+        let qs = build_query_string(&args, &single_query_schema("args"), &None);
+
+        // No backslash escaping: %5C must not appear; the encoded braces/quotes are exact.
+        assert_eq!(qs, "?args=%7B%22key%22%3A%22val%22%7D");
+        assert!(
+            !qs.contains("%5C"),
+            "must not contain backslash escapes: {qs}"
+        );
+    }
+
+    #[test]
+    fn build_query_string_keeps_non_string_serialization() {
+        let mut args = serde_json::Map::new();
+        args.insert("per_page".to_string(), json!(42));
+        assert_eq!(
+            build_query_string(&args, &single_query_schema("per_page"), &None),
+            "?per_page=42"
+        );
+
+        let mut args = serde_json::Map::new();
+        args.insert("running".to_string(), json!(true));
+        assert_eq!(
+            build_query_string(&args, &single_query_schema("running"), &None),
+            "?running=true"
+        );
+    }
+
+    #[test]
+    fn build_query_string_encodes_plain_string() {
+        let mut args = serde_json::Map::new();
+        args.insert("path".to_string(), json!("u/alice/my script"));
+        assert_eq!(
+            build_query_string(&args, &single_query_schema("path"), &None),
+            "?path=u%2Falice%2Fmy%20script"
+        );
+    }
 }