fix(safety): allow empty string tool params (#848)

* fix(safety): allow empty string tool params

* fix(safety): preserve heuristic checks and add path context to tool validation

This follow-up refactor addresses PR review feedback by restoring
heuristic checks (whitespace ratio, character repetition) for tool
parameter validation and improving error reporting.

Changes:
- Restored heuristic warnings in validate_non_empty_input so they apply
  to both user input and tool parameters (when non-empty).
- Refactored check_strings to recursively build and pass JSON paths
  (e.g., "metadata.tags[1]").
- Updated validation errors to use the specific JSON path as the field
  name instead of the generic "input".
- Added regression tests for whitespace/repetition warnings and JSON
  path reporting in tool parameters.

This ensures the safety layer remains semantically neutral about empty
strings (fixing the memory_tree path: "" issue) while maintaining
rigorous protection and providing better developer ergonomics.

* style: run cargo fmt

src/safety/validator.rs

@@ -117,8 +117,6 @@ impl Validator {
 
     /// Validate input text.
     pub fn validate(&self, input: &str) -> ValidationResult {
-        let mut result = ValidationResult::ok();
-
         // Check empty
         if input.is_empty() {
             return ValidationResult::error(ValidationError {
@@ -128,10 +126,16 @@ impl Validator {
             });
         }
 
+        self.validate_non_empty_input(input, "input")
+    }
+
+    fn validate_non_empty_input(&self, input: &str, field: &str) -> ValidationResult {
+        let mut result = ValidationResult::ok();
+
         // Check length
         if input.len() > self.max_length {
             result = result.merge(ValidationResult::error(ValidationError {
-                field: "input".to_string(),
+                field: field.to_string(),
                 message: format!(
                     "Input too long: {} bytes (max {})",
                     input.len(),
@@ -143,7 +147,7 @@ impl Validator {
 
         if input.len() < self.min_length {
             result = result.merge(ValidationResult::error(ValidationError {
-                field: "input".to_string(),
+                field: field.to_string(),
                 message: format!(
                     "Input too short: {} bytes (min {})",
                     input.len(),
@@ -156,7 +160,7 @@ impl Validator {
         // Check for valid UTF-8 (should always pass since we have a &str, but check for weird chars)
         if input.chars().any(|c| c == '\x00') {
             result = result.merge(ValidationResult::error(ValidationError {
-                field: "input".to_string(),
+                field: field.to_string(),
                 message: "Input contains null bytes".to_string(),
                 code: ValidationErrorCode::InvalidEncoding,
             }));
@@ -167,7 +171,7 @@ impl Validator {
         for pattern in &self.forbidden_patterns {
             if lower_input.contains(pattern) {
                 result = result.merge(ValidationResult::error(ValidationError {
-                    field: "input".to_string(),
+                    field: field.to_string(),
                     message: format!("Input contains forbidden pattern: {}", pattern),
                     code: ValidationErrorCode::ForbiddenContent,
                 }));
@@ -196,29 +200,40 @@ impl Validator {
         // Recursively check all string values in the JSON
         fn check_strings(
             value: &serde_json::Value,
+            path: &str,
             validator: &Validator,
             result: &mut ValidationResult,
         ) {
             match value {
                 serde_json::Value::String(s) => {
-                    let string_result = validator.validate(s);
+                    let string_result = if s.is_empty() {
+                        ValidationResult::ok()
+                    } else {
+                        validator.validate_non_empty_input(s, path)
+                    };
                     *result = std::mem::take(result).merge(string_result);
                 }
                 serde_json::Value::Array(arr) => {
-                    for item in arr {
-                        check_strings(item, validator, result);
+                    for (i, item) in arr.iter().enumerate() {
+                        let child_path = format!("{path}[{i}]");
+                        check_strings(item, &child_path, validator, result);
                     }
                 }
                 serde_json::Value::Object(obj) => {
-                    for (_, v) in obj {
-                        check_strings(v, validator, result);
+                    for (k, v) in obj {
+                        let child_path = if path.is_empty() {
+                            k.clone()
+                        } else {
+                            format!("{path}.{k}")
+                        };
+                        check_strings(v, &child_path, validator, result);
                     }
                 }
                 _ => {}
             }
         }
 
-        check_strings(params, self, &mut result);
+        check_strings(params, "", self, &mut result);
         result
     }
 }
@@ -312,4 +327,100 @@ mod tests {
         assert!(result.is_valid); // Still valid, just a warning
         assert!(!result.warnings.is_empty());
     }
+
+    #[test]
+    fn test_tool_params_allow_empty_strings() {
+        let validator = Validator::new();
+        let result = validator.validate_tool_params(&serde_json::json!({
+            "path": "",
+            "nested": {
+                "label": ""
+            },
+            "items": [""]
+        }));
+
+        assert!(result.is_valid);
+        assert!(result.errors.is_empty());
+    }
+
+    #[test]
+    fn test_tool_params_still_block_null_bytes() {
+        let validator = Validator::new();
+        let result = validator.validate_tool_params(&serde_json::json!({
+            "path": "bad\u{0000}path"
+        }));
+
+        assert!(!result.is_valid);
+        assert!(
+            result
+                .errors
+                .iter()
+                .any(|e| e.code == ValidationErrorCode::InvalidEncoding)
+        );
+    }
+
+    #[test]
+    fn test_tool_params_still_block_forbidden_patterns() {
+        let validator = Validator::new().forbid_pattern("forbidden");
+        let result = validator.validate_tool_params(&serde_json::json!({
+            "path": "contains forbidden content"
+        }));
+
+        assert!(!result.is_valid);
+        assert!(
+            result
+                .errors
+                .iter()
+                .any(|e| e.code == ValidationErrorCode::ForbiddenContent)
+        );
+    }
+
+    #[test]
+    fn test_tool_params_still_warn_on_repetition() {
+        let validator = Validator::new();
+        let result = validator.validate_tool_params(&serde_json::json!({
+            "content": format!("prefix{}suffix", "x".repeat(50))
+        }));
+
+        assert!(result.is_valid);
+        assert!(
+            result.warnings.iter().any(|w| w.contains("repetition")),
+            "expected repetition warning for tool params, got: {:?}",
+            result.warnings
+        );
+    }
+
+    #[test]
+    fn test_tool_params_still_warn_on_whitespace_ratio() {
+        let validator = Validator::new();
+        // >100 chars, >90% whitespace
+        let result = validator.validate_tool_params(&serde_json::json!({
+            "content": format!("a{}b", " ".repeat(200))
+        }));
+
+        assert!(result.is_valid);
+        assert!(
+            result.warnings.iter().any(|w| w.contains("whitespace")),
+            "expected whitespace warning for tool params, got: {:?}",
+            result.warnings
+        );
+    }
+
+    #[test]
+    fn test_tool_params_error_field_contains_json_path() {
+        let validator = Validator::new().forbid_pattern("evil");
+        let result = validator.validate_tool_params(&serde_json::json!({
+            "metadata": {
+                "tags": ["good", "evil"]
+            }
+        }));
+
+        assert!(!result.is_valid);
+        let error = result
+            .errors
+            .iter()
+            .find(|e| e.code == ValidationErrorCode::ForbiddenContent)
+            .expect("expected forbidden content error");
+        assert_eq!(error.field, "metadata.tags[1]");
+    }
 }