Skip to content

Commit 56fa487

Browse files
committed
fix(scripted_tool): use Display format instead of Debug in error messages
Debug format ({:?}) in error responses could leak internal details like variant names and struct fields. Switched to Display format for consistent, user-safe error messages. Closes #428
1 parent afc3373 commit 56fa487

File tree

1 file changed

+29
-2
lines changed

1 file changed

+29
-2
lines changed

crates/bashkit/src/scripted_tool/execute.rs

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ impl Tool for ScriptedTool {
320320
stdout: String::new(),
321321
stderr: e.to_string(),
322322
exit_code: 1,
323-
error: Some(format!("{:?}", e)),
323+
error: Some(e.to_string()),
324324
},
325325
}
326326
}
@@ -352,7 +352,7 @@ impl Tool for ScriptedTool {
352352
stdout: String::new(),
353353
stderr: e.to_string(),
354354
exit_code: 1,
355-
error: Some(format!("{:?}", e)),
355+
error: Some(e.to_string()),
356356
},
357357
};
358358

@@ -452,4 +452,31 @@ mod tests {
452452
usage_from_schema(&serde_json::json!({"type": "object", "properties": {}})).is_none()
453453
);
454454
}
455+
456+
#[tokio::test]
457+
async fn test_error_uses_display_not_debug() {
458+
use super::ScriptedTool;
459+
use crate::ToolDef;
460+
use crate::tool::Tool;
461+
462+
let mut tool = ScriptedTool::builder("test")
463+
.short_description("test")
464+
.tool(
465+
ToolDef::new("fail", "Always fails"),
466+
|_args: &super::ToolArgs| Err("service error".to_string()),
467+
)
468+
.build();
469+
let req = ToolRequest {
470+
commands: "fail".into(),
471+
timeout_ms: None,
472+
};
473+
let resp = tool.execute(req).await;
474+
// Error messages use Display format, not Debug, to avoid leaking internals
475+
if let Some(ref err) = resp.error {
476+
assert!(
477+
!err.contains("Execution("),
478+
"error should use Display not Debug: {err}",
479+
);
480+
}
481+
}
455482
}

0 commit comments

Comments
 (0)