diff --git a/sdk/python/ironclaw_tools.py b/sdk/python/ironclaw_tools.py index cb2355ee..72a0c1ed 100644 --- a/sdk/python/ironclaw_tools.py +++ b/sdk/python/ironclaw_tools.py @@ -50,13 +50,13 @@ def _token(): return _env("IRONCLAW_WORKER_TOKEN") -def call_tool(name, params=None, timeout_secs=None): +def call_tool(name, params=None, timeout_secs=60): """Call a tool on the orchestrator by name. Args: name: Tool name (e.g., "echo", "shell", "read_file"). params: Dictionary of parameters to pass to the tool. - timeout_secs: Optional timeout in seconds (max 300). + timeout_secs: Timeout in seconds (default 60, max 300). Returns: Tool output as a string. @@ -65,12 +65,12 @@ def call_tool(name, params=None, timeout_secs=None): RuntimeError: If the tool call fails. """ url = f"{_base_url()}/tools/call" + server_timeout = min(int(timeout_secs), 300) body = { "tool_name": name, "parameters": params or {}, + "timeout_secs": server_timeout, } - if timeout_secs is not None: - body["timeout_secs"] = min(int(timeout_secs), 300) data = json.dumps(body).encode("utf-8") req = urllib.request.Request( @@ -84,7 +84,10 @@ def call_tool(name, params=None, timeout_secs=None): ) try: - with urllib.request.urlopen(req, timeout=(timeout_secs if timeout_secs is not None else 60) + 5) as resp: + # Client-side timeout slightly longer than server-side to account + # for network latency, preventing premature client timeouts. + client_timeout = server_timeout + 5 + with urllib.request.urlopen(req, timeout=client_timeout) as resp: result = json.loads(resp.read().decode("utf-8")) except urllib.error.HTTPError as e: body_text = e.read().decode("utf-8", errors="replace") if e.fp else "" diff --git a/src/orchestrator/api.rs b/src/orchestrator/api.rs index e43965b1..991799cf 100644 --- a/src/orchestrator/api.rs +++ b/src/orchestrator/api.rs @@ -476,17 +476,24 @@ async fn tool_call_handler( format!("PTC call: {}", req.tool_name), format!("Programmatic tool call from job {}", job_id), ); - // Propagate nesting depth so the executor enforces the global limit - ctx.tool_nesting_depth = req.nesting_depth; + // Do not trust client-provided nesting_depth — a malicious worker + // could always send 0 to bypass the limit. PTC calls from workers + // are inherently at depth >= 1 (container -> orchestrator). Use the + // client value but floor it at 1. + ctx.tool_nesting_depth = req.nesting_depth.max(1); - // Emit tool_use SSE event + // Emit tool_use SSE event with redacted parameters to avoid leaking + // sensitive data (API keys, passwords, PII) to the web UI. if let Some(ref tx) = state.job_event_tx { + let redacted_params = serde_json::json!({ + "_note": "parameters redacted for security" + }); let _ = tx.send(( job_id, SseEvent::JobToolUse { job_id: job_id.to_string(), tool_name: req.tool_name.clone(), - input: req.parameters.clone(), + input: redacted_params, }, )); } diff --git a/src/tools/executor.rs b/src/tools/executor.rs index 667edac7..66461aa1 100644 --- a/src/tools/executor.rs +++ b/src/tools/executor.rs @@ -9,6 +9,7 @@ use std::time::{Duration, Instant}; use crate::context::JobContext; use crate::safety::SafetyLayer; +use crate::tools::tool::ToolDomain; use crate::tools::registry::ToolRegistry; /// Maximum allowed nesting depth for tool-invokes-tool chains. @@ -51,6 +52,9 @@ pub enum PtcError { #[error("Nesting depth exceeded (max {max})")] NestingDepthExceeded { max: u32 }, + + #[error("Tool {name} has domain Container and cannot be executed on the orchestrator")] + DomainBlocked { name: String }, } /// Standalone tool execution engine for programmatic tool calling. @@ -106,6 +110,15 @@ impl ToolExecutor { name: tool_name.to_string(), })?; + // Reject Container-domain tools — they must run inside a sandbox, + // not on the orchestrator host. Without this check a compromised + // worker could invoke shell/file tools on the host (sandbox escape). + if tool.domain() == ToolDomain::Container { + return Err(PtcError::DomainBlocked { + name: tool_name.to_string(), + }); + } + // Determine timeout: caller override -> tool's own timeout -> default, // capped at MAX_TIMEOUT_SECS. let timeout = timeout_override @@ -180,7 +193,7 @@ impl std::fmt::Debug for ToolExecutor { mod tests { use super::*; use crate::safety::SafetyLayer; - use crate::tools::tool::{Tool, ToolError, ToolOutput}; + use crate::tools::tool::{Tool, ToolDomain, ToolError, ToolOutput}; fn test_safety_config() -> crate::config::SafetyConfig { crate::config::SafetyConfig { @@ -401,6 +414,54 @@ mod tests { } } + /// A tool that declares Container domain — must be blocked by the executor. + struct ContainerDomainTool; + + #[async_trait::async_trait] + impl Tool for ContainerDomainTool { + fn name(&self) -> &str { + "container_tool" + } + fn description(&self) -> &str { + "Simulates a container-domain tool" + } + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({"type": "object"}) + } + async fn execute( + &self, + _params: serde_json::Value, + _ctx: &JobContext, + ) -> Result { + Ok(ToolOutput::text("should not reach here", Duration::from_millis(1))) + } + fn domain(&self) -> ToolDomain { + ToolDomain::Container + } + fn requires_sanitization(&self) -> bool { + false + } + } + + #[tokio::test] + async fn test_container_domain_blocked() { + let tools = Arc::new(ToolRegistry::new()); + tools.register(Arc::new(ContainerDomainTool)).await; + let safety = Arc::new(SafetyLayer::new(&test_safety_config())); + let executor = ToolExecutor::new(tools, safety, Duration::from_secs(60)); + + let ctx = JobContext::new("test", "test"); + let result = executor + .execute("container_tool", serde_json::json!({}), &ctx, None) + .await; + + assert!( + matches!(result, Err(PtcError::DomainBlocked { .. })), + "Container-domain tools must be rejected: {:?}", + result + ); + } + #[tokio::test] async fn test_execute_sequential_calls() { let tools = Arc::new(ToolRegistry::new());