security: address review findings -- domain check, nesting depth, SSE redaction

Three security fixes from code review:

1. CRITICAL: Block Container-domain tools from executing on the
   orchestrator host. The ToolExecutor now checks tool.domain() and
   rejects Container tools with PtcError::DomainBlocked, preventing
   sandbox escape / RCE.

2. MEDIUM: Floor client-provided nesting_depth at 1 instead of trusting
   the worker's value. A malicious worker can no longer send
   nesting_depth=0 to bypass MAX_NESTING_DEPTH.

3. MEDIUM: Redact tool parameters in SSE JobToolUse events to prevent
   leaking sensitive data (API keys, passwords) to web UI observers.

4. Python SDK: Always send timeout_secs to server and use server_timeout+5
   for client-side HTTP timeout to prevent premature client timeouts.

Regression test: test_container_domain_blocked verifies Container-domain
tools are rejected.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

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 ""

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,
             },
         ));
     }

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<ToolOutput, ToolError> {
+            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());