diff --git a/tools/server/README-dev.md b/tools/server/README-dev.md index 3c5428248a..31408f4267 100644 --- a/tools/server/README-dev.md +++ b/tools/server/README-dev.md @@ -201,7 +201,7 @@ Invoke a tool call, request body is a JSON object with: Headers: - `x-tool-cwd`: optional; if set, use as the CWD for tool; this is not part of tool's params because it's meant to be set by the runtime, not the LLM itself -- `x-tool-docker`: optional; if set, use this docker container ID as tool runtime environment +- `x-tool-runtime`: optional; if set, run the tool inside this isolate instead of on the host. Only `docker-container:` is supported for now, using an already-running container Returns JSON object. There are two response formats (MCP tools use the same two formats: their result content is concatenated into `plain_text_response`, and RPC or tool errors are surfaced as the `error` string): diff --git a/tools/server/server-tools.cpp b/tools/server/server-tools.cpp index 4326e85972..eacfbf0f74 100644 --- a/tools/server/server-tools.cpp +++ b/tools/server/server-tools.cpp @@ -724,12 +724,22 @@ private: std::string container_id; }; +// runtime spec used by --tools-runtime and the x-tool-runtime header +// this is the only scheme for now, ssh: and podman: can be added next to it +static const std::string SERVER_TOOL_RUNTIME_DOCKER_CONTAINER = "docker-container:"; + +// an empty runtime runs the tools on the host static std::unique_ptr make_tools_io(const json & params) { - std::string cwd = json_value(params, "cwd", std::string()); - if (params.contains("docker_container_id")) { - return std::make_unique(params.at("docker_container_id").get(), cwd); + std::string cwd = json_value(params, "cwd", std::string()); + std::string runtime = json_value(params, "runtime", std::string()); + if (runtime.empty()) { + return std::make_unique(cwd); } - return std::make_unique(cwd); + if (runtime.rfind(SERVER_TOOL_RUNTIME_DOCKER_CONTAINER, 0) == 0) { + return std::make_unique(runtime.substr(SERVER_TOOL_RUNTIME_DOCKER_CONTAINER.size()), cwd); + } + // do not fall back to the host, the caller asked for an isolate + throw std::runtime_error("unknown tool runtime: " + runtime); } // no '/' in pattern -> match basename at any depth; else match full relative path @@ -1138,9 +1148,9 @@ struct server_tool_exec_shell_command : server_tool { timeout = std::min(timeout, SERVER_TOOL_EXEC_SHELL_COMMAND_MAX_TIMEOUT); max_output = std::min(max_output, SERVER_TOOL_EXEC_SHELL_COMMAND_MAX_OUTPUT_SIZE); - // docker containers are Linux-based regardless of host OS, so a docker target always gets `sh -c` + // an isolate is always POSIX regardless of host OS, so it always gets `sh -c` #ifdef _WIN32 - std::vector args = params.contains("docker_container_id") + std::vector args = !json_value(params, "runtime", std::string()).empty() ? std::vector{"sh", "-c", command} : std::vector{"cmd", "/c", command}; #else @@ -1635,9 +1645,9 @@ struct server_tool_get_info : server_tool { json invoke(json params, server_tool::stream *) const override { auto io = make_tools_io(params); - // inside docker, we always use the linux command + // inside an isolate, we always use the linux command #ifdef _WIN32 - std::vector args = params.contains("docker_container_id") + std::vector args = !json_value(params, "runtime", std::string()).empty() ? std::vector{"uname", "-a"} : std::vector{"cmd", "/c", "ver"}; #else @@ -1753,8 +1763,7 @@ struct server_tools_docker_runtime { server_tools_docker_runtime(const server_tools_docker_runtime &) = delete; explicit server_tools_docker_runtime(const std::string & spec) { - static const std::string docker_prefix = "docker:"; - static const std::string docker_container_prefix = "docker-container:"; + static const std::string docker_prefix = "docker:"; if (spec.rfind(docker_prefix, 0) == 0) { spawned = true; image = spec.substr(docker_prefix.size()); @@ -1762,9 +1771,9 @@ struct server_tools_docker_runtime { throw std::runtime_error("--tools-runtime docker: requires an image name"); } spawn(); - } else if (spec.rfind(docker_container_prefix, 0) == 0) { + } else if (spec.rfind(SERVER_TOOL_RUNTIME_DOCKER_CONTAINER, 0) == 0) { spawned = false; - container_id = spec.substr(docker_container_prefix.size()); + container_id = spec.substr(SERVER_TOOL_RUNTIME_DOCKER_CONTAINER.size()); if (container_id.empty()) { throw std::runtime_error("--tools-runtime docker-container: requires a container id"); } @@ -1989,16 +1998,16 @@ void server_tools::setup(const std::vector & enabled_tools, params["cwd"] = cwd; } - // accept x-tool-docker header to route tool I/O through a running docker container; - // falls back to the --tools-runtime container, if configured - if (params.contains("docker_container_id")) { - params.erase("docker_container_id"); + // accept x-tool-runtime header to route tool I/O through an isolate, e.g. "docker-container:"; + // falls back to the --tools-runtime isolate, if configured + if (params.contains("runtime")) { + params.erase("runtime"); } - auto docker_container_id = get_header(req.headers, "x-tool-docker"); - if (!docker_container_id.empty()) { - params["docker_container_id"] = docker_container_id; + auto runtime = get_header(req.headers, "x-tool-runtime"); + if (!runtime.empty()) { + params["runtime"] = runtime; } else if (docker_runtime) { - params["docker_container_id"] = docker_runtime->get_container_id(); + params["runtime"] = SERVER_TOOL_RUNTIME_DOCKER_CONTAINER + docker_runtime->get_container_id(); } server_tool & tool = find_tool(tools, tool_name, stream); diff --git a/tools/server/tests/unit/test_tools_builtin.py b/tools/server/tests/unit/test_tools_builtin.py index 4aee00cd3d..c651e8e72d 100755 --- a/tools/server/tests/unit/test_tools_builtin.py +++ b/tools/server/tests/unit/test_tools_builtin.py @@ -180,11 +180,11 @@ def docker_container(): subprocess.run(["docker", "rm", "-f", container_id], capture_output=True) -def test_tools_builtin_docker_header(docker_container: str): +def test_tools_builtin_runtime_header(docker_container: str): global server server.start() - headers = {"x-tool-docker": docker_container, "x-tool-cwd": "/tmp"} + headers = {"x-tool-runtime": f"docker-container:{docker_container}", "x-tool-cwd": "/tmp"} write_res = call_tool("write_file", {"path": "test.log", "content": "hello docker\n"}, headers=headers) assert write_res["result"] == "file written successfully" @@ -196,6 +196,18 @@ def test_tools_builtin_docker_header(docker_container: str): assert "hello docker" in exec_res["plain_text_response"] +def test_tools_builtin_runtime_header_unknown_scheme(): + global server + server.start() + + # an unknown runtime must fail, never silently fall back to running on the host + res = server.make_request("POST", "/tools", + data={"tool": "exec_shell_command", "params": {"command": "echo hi"}}, + headers={"x-tool-runtime": "ssh:example.com"}) + assert res.status_code == 500, res.body + assert "unknown tool runtime" in str(res.body) + + def test_tools_builtin_docker_runtime_cleans_up_spawned_container(): reason = _docker_unavailable_reason() if reason is not None: