Merge pull request #603 from MarekSpodBiedry:issue-560

crates/mcp/src/tests.rs

@@ -517,3 +517,29 @@ fn text_to_columns_destination_follows_its_sheet() {
     cmd_fails(&mut s, "data.textToColumns", json!({"range": "Other!A1", "delimiters": [","], "destination": "D0"}));
     cmd_fails(&mut s, "data.textToColumns", json!({"range": "Other!A1", "delimiters": [","], "destination": "Sheet1!D1"}));
 }
+
+#[test]
+fn invalid_borders_are_invalid_params() {
+    let mut s = server();
+    let bad = |s: &mut Server, id: u64, borders: Value| {
+        let v = rpc(s, id, "tools/call", json!({"name": "format_range", "arguments": {"range": "A1", "borders": borders}}));
+        assert_eq!(err_code(&v), -32602, "{v}");
+    };
+    bad(&mut s, 1, json!(123));
+    bad(&mut s, 2, json!("bogus"));
+    bad(&mut s, 3, json!({"preset": "bogus"}));
+    bad(&mut s, 4, json!({"style": "thick"}));
+    ok(&mut s, "format_range", json!({"range": "A1", "borders": "all"}));
+    ok(&mut s, "format_range", json!({"range": "B1", "borders": {"preset": "all", "style": "thick", "color": "#123456"}}));
+}
+
+#[test]
+fn invalid_sort_keys_are_invalid_params() {
+    let mut s = server();
+    ok(&mut s, "write_range", json!({"range": "A1", "values": [["b"], ["c"], ["a"]]}));
+    for (id, keys) in [(1, json!([123])), (2, json!([{"order": "desc"}])), (3, json!([{"column": "A", "order": "sideways"}]))] {
+        let v = rpc(&mut s, id, "tools/call", json!({"name": "sort_range", "arguments": {"range": "A1:A3", "keys": keys}}));
+        assert_eq!(err_code(&v), -32602, "{v}");
+    }
+    ok(&mut s, "sort_range", json!({"range": "A1:A3", "keys": ["A", {"column": "A", "order": "desc"}]}));
+}

crates/mcp/src/tools.rs

@@ -391,8 +391,41 @@ fn type_matches(t: &str, v: &Value) -> bool {
     }
 }
 
-/// Validates `args` against the tool's schema: unknown tool, missing required properties and
-/// top-level type mismatches are JSON-RPC "invalid params" errors.
+/// Checks a value against a (sub)schema: `type`, `enum`, `oneOf`, array `items`, and for objects the required and known properties.
+fn schema_matches(schema: &Value, v: &Value) -> bool {
+    if let Some(t) = schema["type"].as_str()
+        && !type_matches(t, v)
+    {
+        return false;
+    }
+    if let Some(allowed) = schema["enum"].as_array()
+        && !allowed.contains(v)
+    {
+        return false;
+    }
+    if let Some(alts) = schema["oneOf"].as_array()
+        && !alts.iter().any(|alt| schema_matches(alt, v))
+    {
+        return false;
+    }
+    if let Some(xs) = v.as_array()
+        && !xs.iter().all(|x| schema_matches(&schema["items"], x))
+    {
+        return false;
+    }
+    if let Some(o) = v.as_object() {
+        if schema["required"].as_array().into_iter().flatten().filter_map(Value::as_str).any(|r| o.get(r).is_none_or(Value::is_null)) {
+            return false;
+        }
+        if let Some(props) = schema["properties"].as_object() {
+            return o.iter().all(|(k, x)| props.get(k).is_some_and(|ps| x.is_null() || schema_matches(ps, x)));
+        }
+    }
+    true
+}
+
+/// Validates `args` against the tool's schema: unknown tool, missing required properties,
+/// top-level type mismatches and values matching none of a `oneOf` are JSON-RPC "invalid params" errors.
 pub(crate) fn check_arguments(name: &str, args: &Value) -> Result<(), String> {
     let defs = tool_definitions();
     let def = defs.iter().find(|d| d["name"] == name).ok_or_else(|| format!("unknown tool `{name}`"))?;
@@ -421,6 +454,9 @@ pub(crate) fn check_arguments(name: &str, args: &Value) -> Result<(), String> {
         {
             return Err(format!("{name}: `{k}` must be one of {}", Value::Array(allowed.clone())));
         }
+        if (p["oneOf"].is_array() || p["items"]["oneOf"].is_array()) && !schema_matches(p, v) {
+            return Err(format!("{name}: argument `{k}` does not match the schema: {}", p["description"]));
+        }
     }
     Ok(())
 }