Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/bashkit-python/tests/test_bashkit.py
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ def test_scripted_tool_callback_error():
)
r = tool.execute_sync("fail_cmd")
assert r.exit_code != 0
assert "service down" in r.stderr
assert "callback failed" in r.stderr


def test_scripted_tool_error_fallback():
Expand Down Expand Up @@ -863,7 +863,7 @@ def test_scripted_tool_callback_runtime_error():
)
r = tool.execute_sync("fail")
assert r.exit_code != 0
assert "runtime fail" in r.stderr
assert "callback failed" in r.stderr


def test_scripted_tool_callback_type_error():
Expand Down
69 changes: 68 additions & 1 deletion crates/bashkit/src/scripted_tool/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ struct ToolBuiltinAdapter {
callback: ToolCallback,
schema: serde_json::Value,
log: InvocationLog,
sanitize_errors: bool,
}

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

match (self.callback)(&tool_args) {
Ok(stdout) => ExecResult::ok(stdout),
Err(msg) => ExecResult::err(msg, 1),
Err(msg) => {
// THREAT[TM-INF-030]: Sanitize callback errors to prevent
// leaking internal details (connection strings, file paths,
// stack traces) in tool output visible to LLM agents.
if self.sanitize_errors {
#[cfg(feature = "tracing")]
tracing::debug!(
tool = %self.name,
error = %msg,
"tool callback error (sanitized)"
);
ExecResult::err(format!("{}: callback failed\n", self.name), 1)
} else {
ExecResult::err(msg, 1)
}
}
}
}
Err(msg) => ExecResult::err(msg, 2),
Expand Down Expand Up @@ -439,6 +455,7 @@ impl ScriptedTool {
callback: Arc::clone(&tool.callback),
schema: tool.def.input_schema.clone(),
log: Arc::clone(&log),
sanitize_errors: self.sanitize_errors,
});
builder = builder.builtin(name, builtin);
}
Expand Down Expand Up @@ -1135,4 +1152,54 @@ mod tests {
assert_eq!(def.tags, vec!["admin", "billing"]);
assert_eq!(def.category.as_deref(), Some("payments"));
}

// THREAT[TM-INF-030]: Callback error sanitization tests

#[tokio::test]
async fn test_callback_error_sanitized_by_default() {
let tool = ScriptedTool::builder("api")
.tool(
ToolDef::new("fail", "Always fails"),
|_args: &super::ToolArgs| {
Err("connection failed: postgres://admin:secret@internal-db:5432/prod".into())
},
)
.build();
let resp = tool
.execute(ToolRequest {
commands: "fail".to_string(),
timeout_ms: None,
})
.await;
assert_ne!(resp.exit_code, 0);
// Internal details must NOT appear in output
assert!(
!resp.stderr.contains("postgres://"),
"internal details leaked: {}",
resp.stderr
);
assert!(resp.stderr.contains("callback failed"));
}

#[tokio::test]
async fn test_callback_error_unsanitized_when_disabled() {
let tool = ScriptedTool::builder("api")
.sanitize_errors(false)
.tool(
ToolDef::new("fail", "Always fails"),
|_args: &super::ToolArgs| {
Err("connection failed: postgres://admin:secret@internal-db:5432/prod".into())
},
)
.build();
let resp = tool
.execute(ToolRequest {
commands: "fail".to_string(),
timeout_ms: None,
})
.await;
assert_ne!(resp.exit_code, 0);
// With sanitization disabled, full error should appear
assert!(resp.stderr.contains("postgres://"));
}
}
18 changes: 17 additions & 1 deletion crates/bashkit/src/scripted_tool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,9 @@ pub struct ScriptedToolBuilder {
limits: Option<ExecutionLimits>,
env_vars: Vec<(String, String)>,
compact_prompt: bool,
/// When true, callback errors are replaced with a generic message to prevent
/// leaking internal details (file paths, connection strings, stack traces).
sanitize_errors: bool,
}

impl ScriptedToolBuilder {
Expand All @@ -320,6 +323,7 @@ impl ScriptedToolBuilder {
limits: None,
env_vars: Vec::new(),
compact_prompt: false,
sanitize_errors: true,
}
}

Expand Down Expand Up @@ -363,6 +367,16 @@ impl ScriptedToolBuilder {
self
}

/// Control whether callback error messages are sanitized before appearing in
/// tool output. When `true` (the default), internal error details are replaced
/// with a generic "callback failed" message to prevent leaking file paths,
/// connection strings, or stack traces to LLM agents.
// THREAT[TM-INF-030]: Prevent information disclosure through callback errors.
pub fn sanitize_errors(mut self, sanitize: bool) -> Self {
self.sanitize_errors = sanitize;
self
}

/// Emit compact `system_prompt()` that omits full schemas and adds help tip.
///
/// When enabled, `system_prompt()` lists only tool names + one-liners and
Expand Down Expand Up @@ -404,6 +418,7 @@ impl ScriptedToolBuilder {
limits: self.limits.clone(),
env_vars: self.env_vars.clone(),
compact_prompt: self.compact_prompt,
sanitize_errors: self.sanitize_errors,
last_execution_trace: Arc::new(Mutex::new(None)),
}
}
Expand Down Expand Up @@ -475,6 +490,7 @@ pub struct ScriptedTool {
pub(crate) limits: Option<ExecutionLimits>,
pub(crate) env_vars: Vec<(String, String)>,
pub(crate) compact_prompt: bool,
pub(crate) sanitize_errors: bool,
pub(crate) last_execution_trace: Arc<Mutex<Option<ScriptedExecutionTrace>>>,
}

Expand Down Expand Up @@ -772,7 +788,7 @@ mod tests {
})
.await;
assert_ne!(resp.exit_code, 0);
assert!(resp.stderr.contains("service unavailable"));
assert!(resp.stderr.contains("callback failed"));
}

#[tokio::test]
Expand Down
Loading