Skip to content

Commit a276c78

Browse files
committed
fix(tool): sanitize ScriptedTool callback errors to prevent info leaks
Callback errors are now replaced with generic "callback failed" when sanitize_errors is true (the default). Full error is logged at DEBUG level. Opt out via `.sanitize_errors(false)`. Closes #1172
1 parent 1c02434 commit a276c78

2 files changed

Lines changed: 84 additions & 1 deletion

File tree

crates/bashkit/src/scripted_tool/execute.rs

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ struct ToolBuiltinAdapter {
149149
callback: ToolCallback,
150150
schema: serde_json::Value,
151151
log: InvocationLog,
152+
sanitize_errors: bool,
152153
}
153154

154155
#[async_trait]
@@ -163,7 +164,22 @@ impl Builtin for ToolBuiltinAdapter {
163164

164165
match (self.callback)(&tool_args) {
165166
Ok(stdout) => ExecResult::ok(stdout),
166-
Err(msg) => ExecResult::err(msg, 1),
167+
Err(msg) => {
168+
// THREAT[TM-INF-030]: Sanitize callback errors to prevent
169+
// leaking internal details (connection strings, file paths,
170+
// stack traces) in tool output visible to LLM agents.
171+
if self.sanitize_errors {
172+
#[cfg(feature = "tracing")]
173+
tracing::debug!(
174+
tool = %self.name,
175+
error = %msg,
176+
"tool callback error (sanitized)"
177+
);
178+
ExecResult::err(format!("{}: callback failed\n", self.name), 1)
179+
} else {
180+
ExecResult::err(msg, 1)
181+
}
182+
}
167183
}
168184
}
169185
Err(msg) => ExecResult::err(msg, 2),
@@ -439,6 +455,7 @@ impl ScriptedTool {
439455
callback: Arc::clone(&tool.callback),
440456
schema: tool.def.input_schema.clone(),
441457
log: Arc::clone(&log),
458+
sanitize_errors: self.sanitize_errors,
442459
});
443460
builder = builder.builtin(name, builtin);
444461
}
@@ -1135,4 +1152,54 @@ mod tests {
11351152
assert_eq!(def.tags, vec!["admin", "billing"]);
11361153
assert_eq!(def.category.as_deref(), Some("payments"));
11371154
}
1155+
1156+
// THREAT[TM-INF-030]: Callback error sanitization tests
1157+
1158+
#[tokio::test]
1159+
async fn test_callback_error_sanitized_by_default() {
1160+
let tool = ScriptedTool::builder("api")
1161+
.tool(
1162+
ToolDef::new("fail", "Always fails"),
1163+
|_args: &super::ToolArgs| {
1164+
Err("connection failed: postgres://admin:secret@internal-db:5432/prod".into())
1165+
},
1166+
)
1167+
.build();
1168+
let resp = tool
1169+
.execute(ToolRequest {
1170+
commands: "fail".to_string(),
1171+
timeout_ms: None,
1172+
})
1173+
.await;
1174+
assert_ne!(resp.exit_code, 0);
1175+
// Internal details must NOT appear in output
1176+
assert!(
1177+
!resp.stderr.contains("postgres://"),
1178+
"internal details leaked: {}",
1179+
resp.stderr
1180+
);
1181+
assert!(resp.stderr.contains("callback failed"));
1182+
}
1183+
1184+
#[tokio::test]
1185+
async fn test_callback_error_unsanitized_when_disabled() {
1186+
let tool = ScriptedTool::builder("api")
1187+
.sanitize_errors(false)
1188+
.tool(
1189+
ToolDef::new("fail", "Always fails"),
1190+
|_args: &super::ToolArgs| {
1191+
Err("connection failed: postgres://admin:secret@internal-db:5432/prod".into())
1192+
},
1193+
)
1194+
.build();
1195+
let resp = tool
1196+
.execute(ToolRequest {
1197+
commands: "fail".to_string(),
1198+
timeout_ms: None,
1199+
})
1200+
.await;
1201+
assert_ne!(resp.exit_code, 0);
1202+
// With sanitization disabled, full error should appear
1203+
assert!(resp.stderr.contains("postgres://"));
1204+
}
11381205
}

crates/bashkit/src/scripted_tool/mod.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,9 @@ pub struct ScriptedToolBuilder {
308308
limits: Option<ExecutionLimits>,
309309
env_vars: Vec<(String, String)>,
310310
compact_prompt: bool,
311+
/// When true, callback errors are replaced with a generic message to prevent
312+
/// leaking internal details (file paths, connection strings, stack traces).
313+
sanitize_errors: bool,
311314
}
312315

313316
impl ScriptedToolBuilder {
@@ -320,6 +323,7 @@ impl ScriptedToolBuilder {
320323
limits: None,
321324
env_vars: Vec::new(),
322325
compact_prompt: false,
326+
sanitize_errors: true,
323327
}
324328
}
325329

@@ -363,6 +367,16 @@ impl ScriptedToolBuilder {
363367
self
364368
}
365369

370+
/// Control whether callback error messages are sanitized before appearing in
371+
/// tool output. When `true` (the default), internal error details are replaced
372+
/// with a generic "callback failed" message to prevent leaking file paths,
373+
/// connection strings, or stack traces to LLM agents.
374+
// THREAT[TM-INF-030]: Prevent information disclosure through callback errors.
375+
pub fn sanitize_errors(mut self, sanitize: bool) -> Self {
376+
self.sanitize_errors = sanitize;
377+
self
378+
}
379+
366380
/// Emit compact `system_prompt()` that omits full schemas and adds help tip.
367381
///
368382
/// When enabled, `system_prompt()` lists only tool names + one-liners and
@@ -404,6 +418,7 @@ impl ScriptedToolBuilder {
404418
limits: self.limits.clone(),
405419
env_vars: self.env_vars.clone(),
406420
compact_prompt: self.compact_prompt,
421+
sanitize_errors: self.sanitize_errors,
407422
last_execution_trace: Arc::new(Mutex::new(None)),
408423
}
409424
}
@@ -475,6 +490,7 @@ pub struct ScriptedTool {
475490
pub(crate) limits: Option<ExecutionLimits>,
476491
pub(crate) env_vars: Vec<(String, String)>,
477492
pub(crate) compact_prompt: bool,
493+
pub(crate) sanitize_errors: bool,
478494
pub(crate) last_execution_trace: Arc<Mutex<Option<ScriptedExecutionTrace>>>,
479495
}
480496

0 commit comments

Comments
 (0)