From 4ae84dea27a7ac68247574ae19e970292a2ab323 Mon Sep 17 00:00:00 2001 From: Pascal Date: Mon, 10 Aug 2026 13:31:09 +0200 Subject: [PATCH] server: add more tool isolation support (ssh remote + podman rootless) (#26774) * server: add an ssh transport to the tools runtime --tools-runtime ssh: runs the built-in tools on a remote host, where target is whatever ssh already resolves, a user@host or a config alias, so no credentials live in llama.cpp. Only build_argv and upload differ from the docker transport: the remote shell re-parses the command line, so the argv travels through shell_quote_join, and files go over scp with the same quoting on the remote path. Authentication is key-based and the host key must already be trusted, since the tools run without a console and any prompt would hang them. The target is validated before use. The spec can reach us from the x-tool-runtime header, and a leading dash would turn it into an ssh option, which is enough to run a command back on the host. Nothing is created and nothing is reclaimed, so an ssh spec goes straight to the tool call instead of through the container runtime. Note that this is remoting rather than isolation: the tools can do whatever the target account can do, and the isolation is whatever runs them on the far side. * server: support podman in the tools runtime docker and podman expose the same run, exec, cp and inspect verbs with the same argument order, so a single implementation drives both and the engine is carried by the spec prefix: podman: and podman-container: sit next to the docker forms. tools_io_docker becomes tools_io_container and the runtime spawner becomes server_tools_container_runtime, both holding the client binary chosen at parse time. A single parse_container_runtime() resolves every spec, so adding another engine is one string in the table. make_tools_io() now rejects the spawning forms. The spec also reaches it from the x-tool-runtime header, which is client controlled, and only the runtime that owns a container is allowed to create one: a tool call can attach to a running container, nothing more. * ./build/bin/llama-gen-docs * server: simplify the tools runtime and drop the file copy step A server_tools_runtime base with one virtual spec() replaces the container runtime and the bare spec string that ssh needed next to it, so server_tools is back to a single pointer and neither setup nor the handler tests which of the two is set. write_file used to spill its content into a temporary file on the host and copy it in, because run_subprocess had no way to feed a child. It now takes an optional stdin payload and creates the parent directory and the file in a single round trip through a shell in the isolate. That removes the upload virtual and both implementations: no more container cp or scp, no second binary on the host, no sftp subsystem on the target, no predictable temporary in a shared tmp, and none of the content reaching an argv the remote shell re-parses. It also fixes write_file over ssh, which never worked: scp speaks sftp and takes the remote path literally, so quoting it kept the quotes in the file name. Writing the payload before reading the output relies on the child draining stdin as it goes, which holds for cat, its only user today. * ./build/bin/llama-gen-docs * server: harden the tools runtime against argv injection and a stdin stall Validate the container id from x-tool-runtime and --tools-runtime the same way the ssh target already is, so an id shaped like an option (docker-container:--privileged) is rejected before it reaches the engine's exec command line instead of running against a hardened container. Feed the child's stdin after the watchdog is armed, so a transport that stalls mid-write is terminated at the deadline rather than blocking the request forever. Cover both guards and fix the unknown-scheme test, which used ssh: as its example and now names a real runtime. * tests: exercise the tools runtime tests on podman as well as docker Follow-up #26507. The container runtime drives docker and podman through one implementation, so parametrize the availability helper, the container fixture and the attach test on the engine, and cover both engine prefixes in the container id injection test. Each engine skips on its own when it is not installed. The spawn cleanup test stays docker only: it recovers the spawned id from the container hostname, which docker sets to the short id and podman rootless does not guarantee. Podman keeps its coverage through the attach path. * server: release the container handle before respawning Follow-up #26507. create() writes over the handle it is given, so a respawn after the container died on its own leaked the pipes and the process handle of the previous one. * server: trim the tools runtime comments * server: read tool output as raw bytes and harden the runtime on Windows The stdout pipe is read with read() instead of fgets(), so a chunk can hold any byte, including NUL, and still streams as soon as data is available. Past the size cap the pipe keeps draining so the child never blocks on a full pipe. Both pipe fds are forced to binary mode on Windows, where the CRT defaults them to text mode and translates line endings in both directions. Stdin is now always closed after the feed: the child reads a deterministic EOF, and the Windows docker and ssh clients stop outliving their command on a stdin pipe that never closes. The attach form of --tools-runtime has no lifecycle to own, so it becomes a static target validated once at startup. This removes the subprocess that ran on every tool call and serialized calls behind a mutex; a stopped container now surfaces the engine's own error at exec time. The cidfile path is passed as UTF-8, matching the encoding the subprocess layer expects for the CreateProcessW command line, so the spawn form works from a non-ASCII Windows profile. The SIGPIPE note in server.cpp now names the tools runtime children as well as the MCP ones. * clean up comments * less pollute global scope * nits * tests: name the container image after both engines --------- Co-authored-by: Xuan Son Nguyen --- common/arg.cpp | 5 +- tools/cli/README.md | 3 +- tools/completion/README.md | 3 +- tools/server/README-dev.md | 2 +- tools/server/README.md | 8 +- tools/server/server-tools.cpp | 362 ++++++++++++------ tools/server/server-tools.h | 6 +- tools/server/server.cpp | 2 +- tools/server/tests/unit/test_tools_builtin.py | 85 ++-- 9 files changed, 308 insertions(+), 168 deletions(-) diff --git a/common/arg.cpp b/common/arg.cpp index 4cb853c7a4..c37d5cd0aa 100644 --- a/common/arg.cpp +++ b/common/arg.cpp @@ -3312,8 +3312,9 @@ common_params_context common_params_parser_init(common_params & params, llama_ex {"--tools-runtime"}, "OPTION", "experimental: run tools in a separate runtime environment (default: none, use host environment)\n" "available options:\n" - " 'docker:': spin up a new Docker container and reuse it for all invocations, clean up on server exit\n" - " 'docker-container:': use an existing Docker container by ID, won't stop on server exit\n", + " 'docker:', 'podman:': spin up a new container and reuse it for all invocations, clean up on server exit\n" + " 'docker-container:', 'podman-container:': use an existing container by ID, won't stop on server exit\n" + " 'ssh:': run tools on a remote POSIX host over SSH, key-based auth and a trusted host key are required\n", [](common_params & params, const std::string & value) { params.server_tools_runtime = value; } diff --git a/tools/cli/README.md b/tools/cli/README.md index 640d4fee80..b42b2e5343 100644 --- a/tools/cli/README.md +++ b/tools/cli/README.md @@ -54,6 +54,7 @@ | `-ctv, --cache-type-v TYPE` | KV cache data type for V
allowed values: f32, f16, bf16, q8_0, q4_0, q4_1, iq4_nl, q5_0, q5_1
(default: f16)
(env: LLAMA_ARG_CACHE_TYPE_V) | | `-dt, --defrag-thold N` | KV cache defragmentation threshold (DEPRECATED)
(env: LLAMA_ARG_DEFRAG_THOLD) | | `-np, --parallel N` | number of parallel sequences to decode (default: 1)
(env: LLAMA_ARG_N_PARALLEL) | +| `--rpc SERVERS` | comma-separated list of RPC servers (host:port)
(env: LLAMA_ARG_RPC) | | `--mlock` | DEPRECATED in favor of `--load-mode`: force system to keep model in RAM rather than swapping or compressing
(env: LLAMA_ARG_MLOCK) | | `--mmap, --no-mmap` | DEPRECATED in favor of `--load-mode`: whether to memory-map model. (if mmap disabled, slower load but may reduce pageouts if not using mlock)
(env: LLAMA_ARG_MMAP) | | `-dio, --direct-io, -ndio, --no-direct-io` | DEPRECATED in favor of `--load-mode`: use DirectIO if available
(env: LLAMA_ARG_DIO) | @@ -84,8 +85,6 @@ | `-dr, --docker-repo [/][:quant]` | Docker Hub model repository. repo is optional, default to ai/. quant is optional, default to :latest.
example: gemma3
(default: unused)
(env: LLAMA_ARG_DOCKER_REPO) | | `-hf, -hfr, --hf-repo /[:quant]` | Hugging Face model repository; quant is optional, case-insensitive, default to Q4_K_M, or falls back to the first file in the repo if Q4_K_M doesn't exist.
mmproj is also downloaded automatically if available. to disable, add --no-mmproj
example: ggml-org/GLM-4.7-Flash-GGUF:Q4_K_M
(default: unused)
(env: LLAMA_ARG_HF_REPO) | | `-hff, --hf-file FILE` | Hugging Face model file. If specified, it will override the quant in --hf-repo (default: unused)
(env: LLAMA_ARG_HF_FILE) | -| `-hfv, -hfrv, --hf-repo-v /[:quant]` | Hugging Face model repository for the vocoder model (default: unused)
(env: LLAMA_ARG_HF_REPO_V) | -| `-hffv, --hf-file-v FILE` | Hugging Face model file for the vocoder model (default: unused)
(env: LLAMA_ARG_HF_FILE_V) | | `-hft, --hf-token TOKEN` | Hugging Face access token (default: value from HF_TOKEN environment variable)
(env: HF_TOKEN) | | `--log-disable` | Log disable | | `--log-file FNAME` | Log to file
(env: LLAMA_ARG_LOG_FILE) | diff --git a/tools/completion/README.md b/tools/completion/README.md index e0923ea300..552a0c6abf 100644 --- a/tools/completion/README.md +++ b/tools/completion/README.md @@ -137,6 +137,7 @@ llama-completion.exe -m models\gemma-1.1-7b-it.Q4_K_M.gguf --ignore-eos -n -1 | `-ctv, --cache-type-v TYPE` | KV cache data type for V
allowed values: f32, f16, bf16, q8_0, q4_0, q4_1, iq4_nl, q5_0, q5_1
(default: f16)
(env: LLAMA_ARG_CACHE_TYPE_V) | | `-dt, --defrag-thold N` | KV cache defragmentation threshold (DEPRECATED)
(env: LLAMA_ARG_DEFRAG_THOLD) | | `-np, --parallel N` | number of parallel sequences to decode (default: 1)
(env: LLAMA_ARG_N_PARALLEL) | +| `--rpc SERVERS` | comma-separated list of RPC servers (host:port)
(env: LLAMA_ARG_RPC) | | `--mlock` | DEPRECATED in favor of `--load-mode`: force system to keep model in RAM rather than swapping or compressing
(env: LLAMA_ARG_MLOCK) | | `--mmap, --no-mmap` | DEPRECATED in favor of `--load-mode`: whether to memory-map model. (if mmap disabled, slower load but may reduce pageouts if not using mlock)
(env: LLAMA_ARG_MMAP) | | `-dio, --direct-io, -ndio, --no-direct-io` | DEPRECATED in favor of `--load-mode`: use DirectIO if available
(env: LLAMA_ARG_DIO) | @@ -167,8 +168,6 @@ llama-completion.exe -m models\gemma-1.1-7b-it.Q4_K_M.gguf --ignore-eos -n -1 | `-dr, --docker-repo [/][:quant]` | Docker Hub model repository. repo is optional, default to ai/. quant is optional, default to :latest.
example: gemma3
(default: unused)
(env: LLAMA_ARG_DOCKER_REPO) | | `-hf, -hfr, --hf-repo /[:quant]` | Hugging Face model repository; quant is optional, case-insensitive, default to Q4_K_M, or falls back to the first file in the repo if Q4_K_M doesn't exist.
mmproj is also downloaded automatically if available. to disable, add --no-mmproj
example: ggml-org/GLM-4.7-Flash-GGUF:Q4_K_M
(default: unused)
(env: LLAMA_ARG_HF_REPO) | | `-hff, --hf-file FILE` | Hugging Face model file. If specified, it will override the quant in --hf-repo (default: unused)
(env: LLAMA_ARG_HF_FILE) | -| `-hfv, -hfrv, --hf-repo-v /[:quant]` | Hugging Face model repository for the vocoder model (default: unused)
(env: LLAMA_ARG_HF_REPO_V) | -| `-hffv, --hf-file-v FILE` | Hugging Face model file for the vocoder model (default: unused)
(env: LLAMA_ARG_HF_FILE_V) | | `-hft, --hf-token TOKEN` | Hugging Face access token (default: value from HF_TOKEN environment variable)
(env: HF_TOKEN) | | `--log-disable` | Log disable | | `--log-file FNAME` | Log to file
(env: LLAMA_ARG_LOG_FILE) | diff --git a/tools/server/README-dev.md b/tools/server/README-dev.md index 31408f4267..613017acff 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-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 +- `x-tool-runtime`: optional; if set, run the tool inside this isolate instead of on the host. Either `docker-container:` or `podman-container:`, using an already-running container, or `ssh:`, running the tool on a remote host 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/README.md b/tools/server/README.md index 64f0b03269..6927caddbb 100644 --- a/tools/server/README.md +++ b/tools/server/README.md @@ -71,6 +71,7 @@ For the full list of features, please refer to [server's changelog](https://gith | `-ctk, --cache-type-k TYPE` | KV cache data type for K
allowed values: f32, f16, bf16, q8_0, q4_0, q4_1, iq4_nl, q5_0, q5_1
(default: f16)
(env: LLAMA_ARG_CACHE_TYPE_K) | | `-ctv, --cache-type-v TYPE` | KV cache data type for V
allowed values: f32, f16, bf16, q8_0, q4_0, q4_1, iq4_nl, q5_0, q5_1
(default: f16)
(env: LLAMA_ARG_CACHE_TYPE_V) | | `-dt, --defrag-thold N` | KV cache defragmentation threshold (DEPRECATED)
(env: LLAMA_ARG_DEFRAG_THOLD) | +| `--rpc SERVERS` | comma-separated list of RPC servers (host:port)
(env: LLAMA_ARG_RPC) | | `--mlock` | DEPRECATED in favor of `--load-mode`: force system to keep model in RAM rather than swapping or compressing
(env: LLAMA_ARG_MLOCK) | | `--mmap, --no-mmap` | DEPRECATED in favor of `--load-mode`: whether to memory-map model. (if mmap disabled, slower load but may reduce pageouts if not using mlock)
(env: LLAMA_ARG_MMAP) | | `-dio, --direct-io, -ndio, --no-direct-io` | DEPRECATED in favor of `--load-mode`: use DirectIO if available
(env: LLAMA_ARG_DIO) | @@ -101,8 +102,6 @@ For the full list of features, please refer to [server's changelog](https://gith | `-dr, --docker-repo [/][:quant]` | Docker Hub model repository. repo is optional, default to ai/. quant is optional, default to :latest.
example: gemma3
(default: unused)
(env: LLAMA_ARG_DOCKER_REPO) | | `-hf, -hfr, --hf-repo /[:quant]` | Hugging Face model repository; quant is optional, case-insensitive, default to Q4_K_M, or falls back to the first file in the repo if Q4_K_M doesn't exist.
mmproj is also downloaded automatically if available. to disable, add --no-mmproj
example: ggml-org/GLM-4.7-Flash-GGUF:Q4_K_M
(default: unused)
(env: LLAMA_ARG_HF_REPO) | | `-hff, --hf-file FILE` | Hugging Face model file. If specified, it will override the quant in --hf-repo (default: unused)
(env: LLAMA_ARG_HF_FILE) | -| `-hfv, -hfrv, --hf-repo-v /[:quant]` | Hugging Face model repository for the vocoder model (default: unused)
(env: LLAMA_ARG_HF_REPO_V) | -| `-hffv, --hf-file-v FILE` | Hugging Face model file for the vocoder model (default: unused)
(env: LLAMA_ARG_HF_FILE_V) | | `-hft, --hf-token TOKEN` | Hugging Face access token (default: value from HF_TOKEN environment variable)
(env: HF_TOKEN) | | `--log-disable` | Log disable | | `--log-file FNAME` | Log to file
(env: LLAMA_ARG_LOG_FILE) | @@ -197,9 +196,8 @@ For the full list of features, please refer to [server's changelog](https://gith | `--ui-config, --webui-config JSON` | JSON that provides default UI settings (overrides UI defaults)
(env: LLAMA_ARG_UI_CONFIG) | | `--ui-config-file, --webui-config-file PATH` | JSON file that provides default UI settings (overrides UI defaults)
(env: LLAMA_ARG_UI_CONFIG_FILE) | | `--ui-mcp-proxy, --webui-mcp-proxy, --no-ui-mcp-proxy, --no-webui-mcp-proxy` | experimental: whether to enable MCP CORS proxy - do not enable in untrusted environments (default: disabled)
(env: LLAMA_ARG_UI_MCP_PROXY) | -| `--tools TOOL1,TOOL2,...` | experimental: whether to enable built-in tools for AI agents - do not enable in untrusted environments (default: no tools)
specify "all" to enable all tools
available tools: read_file, file_glob_search, grep_search, exec_shell_command, write_file, edit_file, get_datetime
note: for security reasons, this will limit --cors-origins to localhost by default
(env: LLAMA_ARG_TOOLS) | -| `--tools-runtime OPTION` | experimental: run tools in a separate runtime environment (default: none, use host environment)
available options:
'docker:': spin up a new Docker container and reuse it for all invocations, clean up on server exit
'docker-container:': use an existing Docker container by ID, won't stop on server exit

(env: LLAMA_ARG_TOOLS_RUNTIME) | | `--tools TOOL1,TOOL2,...` | experimental: whether to enable built-in tools for AI agents - do not enable in untrusted environments (default: no tools)
specify "all" to enable all tools
available tools: read_file, file_glob_search, grep_search, exec_shell_command, write_file, edit_file, get_datetime, get_info
note: for security reasons, this will limit --cors-origins to localhost by default
(env: LLAMA_ARG_TOOLS) | +| `--tools-runtime OPTION` | experimental: run tools in a separate runtime environment (default: none, use host environment)
available options:
'docker:', 'podman:': spin up a new container and reuse it for all invocations, clean up on server exit
'docker-container:', 'podman-container:': use an existing container by ID, won't stop on server exit
'ssh:': run tools on a remote POSIX host over SSH, key-based auth and a trusted host key are required

(env: LLAMA_ARG_TOOLS_RUNTIME) | | `--mcp-servers-config PATH` | experimental: path to JSON file with MCP server definitions (Cursor-compatible format) - do not enable in untrusted environments (default: none)
note: for security reasons, this will limit --cors-origins to localhost by default
(env: LLAMA_ARG_MCP_SERVERS_CONFIG) | | `--mcp-servers-json JSON` | experimental: inline JSON with MCP server definitions (Cursor-compatible format) - do not enable in untrusted environments (default: none)
note: for security reasons, this will limit --cors-origins to localhost by default
(env: LLAMA_ARG_MCP_SERVERS_JSON) | | `-ag, --agent, -no-ag, --no-agent` | whether to enable CORS proxy and all built-in tools - do not enable in untrusted environments (default: disabled)
note: for security reasons, this will limit --cors-origins to localhost by default
(env: LLAMA_ARG_AGENT) | @@ -280,8 +278,6 @@ For the full list of features, please refer to [server's changelog](https://gith | `--spec-ngram-size-n N` | the argument has been removed. use the respective --spec-ngram-*-size-n or --spec-ngram-mod-n-match | | `--spec-ngram-size-m N` | the argument has been removed. use the respective --spec-ngram-*-size-m | | `--spec-ngram-min-hits N` | the argument has been removed. use the respective --spec-ngram-*-min-hits | -| `-mv, --model-vocoder FNAME` | vocoder model for audio generation (default: unused) | -| `--tts-use-guide-tokens` | Use guide tokens to improve TTS word recall | | `--embd-gemma-default` | use default EmbeddingGemma model (note: can download weights from the internet) | | `--fim-qwen-1.5b-default` | use default Qwen 2.5 Coder 1.5B (note: can download weights from the internet) | | `--fim-qwen-3b-default` | use default Qwen 2.5 Coder 3B (note: can download weights from the internet) | diff --git a/tools/server/server-tools.cpp b/tools/server/server-tools.cpp index 27c663fdd2..a4c1059fff 100644 --- a/tools/server/server-tools.cpp +++ b/tools/server/server-tools.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -25,6 +26,11 @@ # define NOMINMAX # endif # include +# include +# include +#else +# include +# include #endif namespace fs = std::filesystem; @@ -176,7 +182,7 @@ public: const std::function & on_chunk = nullptr) const = 0; }; -// shared subprocess execution helper, used by both the local and the docker-backed tools_io implementations. +// shared subprocess execution helper, used by both the local and the isolate-backed tools_io implementations. // combine_stderr=false when the raw stdout bytes must not be tainted by stderr, e.g. reading file contents. static tools_io::exec_result run_subprocess( const std::vector & args, @@ -184,7 +190,8 @@ static tools_io::exec_result run_subprocess( int timeout_secs, const std::function & on_chunk, bool combine_stderr, - const std::string & cwd = "") { + const std::string & cwd = "", + const std::string * stdin_data = nullptr) { tools_io::exec_result res; common_subproc proc; @@ -216,26 +223,59 @@ static tools_io::exec_result run_subprocess( } }); + // write stdin before reading stdout, the child drains stdin as it goes + // always close stdin, a transport client waits forever if its stdin pipe stays open + if (FILE * in = proc.stdin_file()) { + if (stdin_data != nullptr && !stdin_data->empty()) { +#if defined(_WIN32) + // pipe fds default to CRT text mode: binary keeps the bytes untranslated + _setmode(_fileno(in), _O_BINARY); +#endif + // a short write is not an error by itself, the exit code below decides + fwrite(stdin_data->data(), 1, stdin_data->size(), in); + } + fflush(in); + } + proc.close_stdin(); + FILE * f = proc.stdout_file(); std::string output; bool truncated = false; if (f) { +#if defined(_WIN32) + // pipe fds default to CRT text mode: binary keeps the bytes untranslated + _setmode(_fileno(f), _O_BINARY); +#endif + // read raw bytes, not lines: the output can hold NUL and must arrive as soon as it is ready + // keep draining past the size cap, else the child blocks on a full pipe char buf[4096]; - while (fgets(buf, sizeof(buf), f) != nullptr) { - if (!truncated) { - size_t len = strlen(buf); - if (output.size() + len <= max_output) { - output.append(buf, len); - if (on_chunk && !on_chunk(console_output_to_utf8(std::string(buf, len)))) { - proc.terminate(); - break; - } - } else { - size_t remaining = max_output - output.size(); - output.append(buf, remaining); - if (on_chunk && remaining > 0) on_chunk(console_output_to_utf8(std::string(buf, remaining))); - truncated = true; + for (;;) { +#if defined(_WIN32) + const int n = _read(_fileno(f), buf, (unsigned) sizeof(buf)); +#else + ssize_t n = read(fileno(f), buf, sizeof(buf)); + while (n < 0 && errno == EINTR) { + n = read(fileno(f), buf, sizeof(buf)); + } +#endif + if (n <= 0) { + break; + } + if (truncated) { + continue; + } + const size_t len = (size_t) n; + if (output.size() + len <= max_output) { + output.append(buf, len); + if (on_chunk && !on_chunk(console_output_to_utf8(std::string(buf, len)))) { + proc.terminate(); + break; } + } else { + size_t remaining = max_output - output.size(); + output.append(buf, remaining); + if (on_chunk && remaining > 0) on_chunk(console_output_to_utf8(std::string(buf, remaining))); + truncated = true; } } } @@ -473,7 +513,7 @@ private: } }; -// timeout for auxiliary isolate calls (stat/mkdir/ls/cp helpers); exec_shell_command uses its own +// timeout for auxiliary isolate calls (stat/mkdir/ls helpers); exec_shell_command uses its own // caller-controlled timeout instead, enforced separately in run() static constexpr int SERVER_TOOL_ISOLATE_EXEC_TIMEOUT = 15; // seconds static constexpr size_t SERVER_TOOL_ISOLATE_READ_FILE_MAX_SIZE = 64 * 1024 * 1024; // 64 MB @@ -524,33 +564,12 @@ public: } bool write_file(const std::string & path, const std::string & content) const override { - std::string abs_path = resolve(path); - - std::error_code ec; - fs::path tmp_dir = fs::temp_directory_path(ec); - if (ec) return false; - - static std::atomic tmp_counter{0}; - fs::path tmp = tmp_dir / string_format( - "llama-tools-io-isolate-%zu-%llu.tmp", - std::hash{}(std::this_thread::get_id()), - (unsigned long long) tmp_counter.fetch_add(1)); - - { - std::ofstream f(tmp, std::ios::binary); - if (!f) return false; - f << content; - if (!f) return false; - } - - bool ok = shell_run({"sh", "-c", "mkdir -p \"$(dirname \"$1\")\"", "_", abs_path}); - if (ok) { - ok = upload(tmp.string(), abs_path); - } - - std::error_code rm_ec; - fs::remove(tmp, rm_ec); - return ok; + // the content travels on stdin: no argv for the far side to re-parse, no temp file on the host + auto res = run_subprocess( + build_argv({"sh", "-c", "mkdir -p \"$(dirname \"$1\")\" && cat > \"$1\"", "_", resolve(path)}, + /*needs_stdin=*/true), + 4096, SERVER_TOOL_ISOLATE_EXEC_TIMEOUT, nullptr, true, "", &content); + return res.exit_code == 0 && !res.timed_out; } list_result list_entries(const std::string & base, int max_depth, list_kind kind) const override { @@ -612,9 +631,6 @@ protected: // a transport that re-parses its args in a remote shell (ssh) must join `inner` with shell_quote_join() virtual std::vector build_argv(const std::vector & inner, bool needs_stdin) const = 0; - // copy a host file into the isolate, `isolate_path` is absolute and its parent already exists - virtual bool upload(const std::string & host_path, const std::string & isolate_path) const = 0; - // quote `argv` into a single string that a POSIX shell re-parses into exactly `argv` static std::string shell_quote_join(const std::vector & argv) { std::string out; @@ -634,7 +650,7 @@ protected: private: std::string cwd; - // set the working directory in the command itself, docker's `-w` has no equivalent on every transport + // set the working directory in the command itself, no `-w` equivalent exists on every transport // auxiliary calls do not need this, they use the absolute paths from resolve() std::vector with_cwd(const std::vector & inner) const { if (cwd.empty()) { @@ -697,15 +713,16 @@ private: } }; -// an already-running docker container, driven through `docker exec` and `docker cp` -class tools_io_docker : public tools_io_isolate { +// an already-running container, driven through ` exec` +// docker and podman take the same verbs and the same argument order, so one class drives both +class tools_io_container : public tools_io_isolate { public: - tools_io_docker(std::string container_id, std::string cwd = "") - : tools_io_isolate(std::move(cwd)), container_id(std::move(container_id)) {} + tools_io_container(std::string bin, std::string container_id, std::string cwd = "") + : tools_io_isolate(std::move(cwd)), bin(std::move(bin)), container_id(std::move(container_id)) {} protected: std::vector build_argv(const std::vector & inner, bool needs_stdin) const override { - std::vector argv = {"docker", "exec"}; + std::vector argv = {bin, "exec"}; if (needs_stdin) { argv.push_back("-i"); } @@ -714,30 +731,118 @@ protected: return argv; } - bool upload(const std::string & host_path, const std::string & isolate_path) const override { - auto res = run_subprocess( - {"docker", "cp", host_path, container_id + ":" + isolate_path}, - 4096, SERVER_TOOL_ISOLATE_EXEC_TIMEOUT, nullptr, true); - return res.exit_code == 0 && !res.timed_out; - } - private: + std::string bin; 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:"; +// a remote host reached over ssh +// this is remoting, not isolation: the tools can do anything the target account can do +class tools_io_ssh : public tools_io_isolate { +public: + tools_io_ssh(std::string target, std::string cwd = "") + : tools_io_isolate(std::move(cwd)), target(std::move(target)) {} + + // the target can come from a client header, and ssh reads options from its argv + // a target starting with '-' would become one, e.g. -oProxyCommand= runs on the host + static bool is_valid_target(const std::string & target) { + if (target.empty() || target[0] == '-') { + return false; + } + return std::all_of(target.begin(), target.end(), [](unsigned char c) { + return std::isalnum(c) || c == '.' || c == '-' || c == '_' || c == '@'; + }); + } + +protected: + std::vector build_argv(const std::vector & inner, bool needs_stdin) const override { + // the remote shell re-parses the command line, so `inner` travels as one quoted word + std::vector argv = ssh_argv(); + if (!needs_stdin) { + argv.push_back("-n"); + } + argv.push_back(target); + argv.push_back(shell_quote_join(inner)); + return argv; + } + +private: + std::string target; + + // there is no console here, so a prompt would hang the tool call + // key-based auth only, and the admin must trust the host key beforehand + static std::vector ssh_argv() { + return { + "ssh", + "-o", "BatchMode=yes", + "-o", "PasswordAuthentication=no", + "-o", "KbdInteractiveAuthentication=no", + "-o", "StrictHostKeyChecking=yes", + }; + } +}; + +// ":" spawns a container and owns it, "-container:" attaches to one +struct container_runtime_spec { + std::string bin; + std::string arg; // image name when spawning, container id when attaching + bool attach = false; + + static bool parse(const std::string & spec, container_runtime_spec & out) { + // docker and podman take the same verbs, hence a single implementation + static const char * engines[] = {"docker", "podman"}; + for (const char * bin : engines) { + const std::string attach_prefix = std::string(bin) + "-container:"; + if (spec.rfind(attach_prefix, 0) == 0) { + out = {bin, spec.substr(attach_prefix.size()), true}; + return true; + } + const std::string spawn_prefix = std::string(bin) + ":"; + if (spec.rfind(spawn_prefix, 0) == 0) { + out = {bin, spec.substr(spawn_prefix.size()), false}; + return true; + } + } + return false; + } + + // same risk as the ssh target: an id starting with '-' would become an engine option, + // e.g. --privileged + static bool is_valid_id(const std::string & id) { + if (id.empty() || !std::isalnum((unsigned char) id[0])) { + return false; + } + return std::all_of(id.begin(), id.end(), [](unsigned char c) { + return std::isalnum(c) || c == '.' || c == '-' || c == '_'; + }); + } +}; -// 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()); std::string runtime = json_value(params, "runtime", std::string()); if (runtime.empty()) { + // an empty runtime runs the tools on the host 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); + container_runtime_spec container; + if (container_runtime_spec::parse(runtime, container)) { + // spawning belongs to the runtime that owns the container, a tool call only attaches + if (!container.attach) { + throw std::runtime_error("tool runtime must name a running container: " + runtime); + } + if (!container_runtime_spec::is_valid_id(container.arg)) { + throw std::runtime_error("invalid container id: " + container.arg); + } + return std::make_unique(container.bin, container.arg, cwd); + } + const std::string ssh_prefix = "ssh:"; + if (runtime.rfind(ssh_prefix, 0) == 0) { + std::string target = runtime.substr(ssh_prefix.size()); + if (!tools_io_ssh::is_valid_target(target)) { + throw std::runtime_error("invalid ssh target: " + target); + } + return std::make_unique(target, cwd); } // do not fall back to the host, the caller asked for an isolate throw std::runtime_error("unknown tool runtime: " + runtime); @@ -1769,81 +1874,82 @@ struct server_mcp_tool : server_tool { } }; -// owns the docker container used as the sandboxed runtime for tool invocations, as configured by -// --tools-runtime. "spawned" mode starts and stops the container itself; "existing" mode just reuses -// a container id the user already has running and never stops it. -struct server_tools_docker_runtime { - server_tools_docker_runtime(const server_tools_docker_runtime &) = delete; +// resolves --tools-runtime into the isolate that every tool call runs through +// spec() returns the runtime string make_tools_io() takes, and runs once per tool call +struct server_tools_runtime { + virtual ~server_tools_runtime() = default; + virtual std::string spec() = 0; +}; - explicit server_tools_docker_runtime(const std::string & spec) { - static const std::string docker_prefix = "docker:"; - if (spec.rfind(docker_prefix, 0) == 0) { - spawned = true; - image = spec.substr(docker_prefix.size()); - if (image.empty()) { - throw std::runtime_error("--tools-runtime docker: requires an image name"); - } - spawn(); - } else if (spec.rfind(SERVER_TOOL_RUNTIME_DOCKER_CONTAINER, 0) == 0) { - spawned = false; - 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"); - } - } else { +// a target that already exists and needs no lifecycle +// the spec is validated once at startup, then passed straight through +struct server_tools_static_runtime : server_tools_runtime { + explicit server_tools_static_runtime(std::string spec) : runtime_spec(std::move(spec)) {} + std::string spec() override { return runtime_spec; } + +private: + std::string runtime_spec; +}; + +// owns the container the tools run in, as set by --tools-runtime ":" +// it is spawned here and stopped when the server exits +struct server_tools_container_runtime : server_tools_runtime { + server_tools_container_runtime(const server_tools_container_runtime &) = delete; + + explicit server_tools_container_runtime(const std::string & spec) { + container_runtime_spec parsed; + if (!container_runtime_spec::parse(spec, parsed)) { throw std::runtime_error("unknown --tools-runtime option: " + spec); } - } - ~server_tools_docker_runtime() { - if (spawned && !container_id.empty()) { - // closing stdin signals the container's shell (its pid 1) to exit; --rm then removes it - proc.close_stdin(); - proc.join(); + bin = parsed.bin; + image = parsed.arg; + if (image.empty()) { + throw std::runtime_error("--tools-runtime " + bin + ": requires an image name"); } + spawn(); } - // container id to use for the next tool call; respawns a spawned container that died on its own, - // or throws if an externally-managed one is no longer reachable - std::string get_container_id() { + ~server_tools_container_runtime() override { + // closing stdin signals the container's shell (its pid 1) to exit; --rm then removes it + proc.close_stdin(); + proc.join(); + } + + // respawns a container that died on its own, so the returned spec always names a running one + std::string spec() override { std::lock_guard lock(mutex); - if (!spawned) { - if (!is_running(container_id)) { - throw std::runtime_error(string_format( - "docker container \"%s\" is no longer running, restart it to keep using tools", - container_id.c_str())); - } - return container_id; - } - if (!proc.alive()) { - SRV_WRN("docker tools runtime container \"%s\" died, respawning\n", container_id.c_str()); + SRV_WRN("%s tools runtime container \"%s\" died, respawning\n", bin.c_str(), container_id.c_str()); spawn(); } - return container_id; + return bin + "-container:" + container_id; } private: - bool spawned = false; - std::string image; // spawned mode only + std::string bin; + std::string image; std::string container_id; - common_subproc proc; // spawned mode only: `docker run` client that keeps the container alive + common_subproc proc; // ` run` client that keeps the container alive std::mutex mutex; - // spawns "docker run --rm -i sh" and keeps its stdin open; the shell blocks reading stdin, + // spawns " run --rm -i sh" and keeps its stdin open; the shell blocks reading stdin, // so the container stays alive until we close it (see destructor) or it is killed from the outside void spawn() { + // create() writes over the handle it is given, so the previous one is released first + proc.join(); + std::error_code ec; fs::path cidfile = fs::temp_directory_path(ec) / string_format( "llama-tools-runtime-cid-%zu.tmp", std::hash{}(std::this_thread::get_id())); fs::remove(cidfile, ec); - std::vector args = {"docker", "run", "--rm", "-i", "--cidfile", cidfile.string(), image, "sh"}; + std::vector args = {bin, "run", "--rm", "-i", "--cidfile", path_to_utf8(cidfile), image, "sh"}; int options = subprocess_option_no_window | subprocess_option_inherit_environment | subprocess_option_search_user_path; if (!proc.create(args, options)) { - throw std::runtime_error("failed to spawn docker container for tools runtime (image: " + image + ")"); + throw std::runtime_error("failed to spawn " + bin + " container for tools runtime (image: " + image + ")"); } std::string cid; @@ -1855,15 +1961,10 @@ private: fs::remove(cidfile, ec); if (cid.empty()) { proc.terminate(); - throw std::runtime_error("timed out waiting for docker container to start (image: " + image + ")"); + throw std::runtime_error("timed out waiting for " + bin + " container to start (image: " + image + ")"); } container_id = cid; } - - static bool is_running(const std::string & id) { - auto res = run_subprocess({"docker", "inspect", "-f", "{{.State.Running}}", id}, 16, 5, nullptr, true); - return res.exit_code == 0 && !res.timed_out && res.output.rfind("true", 0) == 0; - } }; static server_tool & find_tool(std::vector> & tools, const std::string & name, bool require_stream) { @@ -1914,11 +2015,22 @@ static std::string get_header(const std::map & headers server_tools::server_tools() = default; server_tools::~server_tools() = default; +// the ":" form owns a container lifecycle +// anything else names an existing target, so only its spec is validated here at startup +static std::unique_ptr make_tools_runtime(const std::string & spec) { + container_runtime_spec parsed; + if (container_runtime_spec::parse(spec, parsed) && !parsed.attach) { + return std::make_unique(spec); + } + make_tools_io({{"runtime", spec}}); // nothing to own, just reject a bad spec now + return std::make_unique(spec); +} + void server_tools::setup(const std::vector & enabled_tools, server_mcp & mcp_mgr, const std::string & tools_runtime) { if (!tools_runtime.empty()) { - docker_runtime = std::make_unique(tools_runtime); + runtime = make_tools_runtime(tools_runtime); } if (!enabled_tools.empty()) { @@ -2016,11 +2128,11 @@ void server_tools::setup(const std::vector & enabled_tools, if (params.contains("runtime")) { params.erase("runtime"); } - auto runtime = get_header(req.headers, "x-tool-runtime"); - if (!runtime.empty()) { - params["runtime"] = runtime; - } else if (docker_runtime) { - params["runtime"] = SERVER_TOOL_RUNTIME_DOCKER_CONTAINER + docker_runtime->get_container_id(); + auto runtime_header = get_header(req.headers, "x-tool-runtime"); + if (!runtime_header.empty()) { + params["runtime"] = runtime_header; + } else if (runtime) { + params["runtime"] = runtime->spec(); } server_tool & tool = find_tool(tools, tool_name, stream); diff --git a/tools/server/server-tools.h b/tools/server/server-tools.h index ede303181b..c4509ca80f 100644 --- a/tools/server/server-tools.h +++ b/tools/server/server-tools.h @@ -31,7 +31,7 @@ struct server_tool { json to_json() const; }; -struct server_tools_docker_runtime; // impl detail, defined in server-tools.cpp +struct server_tools_runtime; // impl detail, defined in server-tools.cpp struct server_tools { std::vector> tools; @@ -40,8 +40,8 @@ struct server_tools { server_response queue_res; std::atomic res_id{0}; - // set when --tools-runtime is configured; owns the docker container used to run tools, if any - std::unique_ptr docker_runtime; + // set when --tools-runtime is configured; routes every tool call through an isolate + std::unique_ptr runtime; void setup(const std::vector & enabled_tools, server_mcp & mcp_mgr, diff --git a/tools/server/server.cpp b/tools/server/server.cpp index 1b2e6edb4e..6d1aa43516 100644 --- a/tools/server/server.cpp +++ b/tools/server/server.cpp @@ -89,7 +89,7 @@ int llama_server(int argc, char ** argv) { std::setlocale(LC_NUMERIC, "C"); #ifndef _WIN32 - // Ignore SIGPIPE so the server does not crash if an MCP child exits while we are writing to its stdin + // Ignore SIGPIPE so the server does not crash if a child (MCP server, tools runtime) exits while we are writing to its stdin signal(SIGPIPE, SIG_IGN); #endif diff --git a/tools/server/tests/unit/test_tools_builtin.py b/tools/server/tests/unit/test_tools_builtin.py index 7da569d99f..a69052c6d7 100755 --- a/tools/server/tests/unit/test_tools_builtin.py +++ b/tools/server/tests/unit/test_tools_builtin.py @@ -14,7 +14,7 @@ PROJECT_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".. GREP_MARKER = "llama_cpp_test_tools_builtin_marker_grep_search" # image the container runtime tests run their shell in -DOCKER_IMAGE = "busybox" +CONTAINER_IMAGE = "busybox" @pytest.fixture(autouse=True) @@ -151,54 +151,59 @@ def test_tools_builtin_cwd_header(): os.remove(marker_path) -def _docker_unavailable_reason() -> str | None: - """None if docker can run the image these tests use, otherwise the reason it can't.""" - docker_bin = shutil.which("docker") - if docker_bin is None: - return "docker is not installed" +def _container_engine_unavailable_reason(engine: str) -> str | None: + """None if `engine` can run the image these tests use, otherwise the reason it can't.""" + engine_bin = shutil.which(engine) + if engine_bin is None: + return f"{engine} is not installed" try: - # a daemon that answers `docker info` still cannot run a linux image when it serves - # windows containers, so probe the image itself, which also pulls it before the tests - subprocess.run([docker_bin, "run", "--rm", DOCKER_IMAGE, "true"], capture_output=True, timeout=60, check=True) + # a daemon that answers `info` still cannot run a linux image when it serves windows + # containers, so probe the image itself, which also pulls it before the tests + subprocess.run([engine_bin, "run", "--rm", CONTAINER_IMAGE, "true"], capture_output=True, timeout=60, check=True) except Exception as e: - return f"docker cannot run {DOCKER_IMAGE}: {e}" + return f"{engine} cannot run {CONTAINER_IMAGE}: {e}" return None -@pytest.fixture -def docker_container(): - reason = _docker_unavailable_reason() +@pytest.fixture(params=["docker", "podman"]) +def container_engine(request): + engine = request.param + reason = _container_engine_unavailable_reason(engine) if reason is not None: pytest.skip(reason) # ty: ignore[too-many-positional-arguments, invalid-argument-type] + return engine + +@pytest.fixture +def container_id(container_engine: str): proc = subprocess.run( - ["docker", "run", "-d", "--rm", DOCKER_IMAGE, "sleep", "300"], + [container_engine, "run", "-d", "--rm", CONTAINER_IMAGE, "sleep", "300"], capture_output=True, text=True, ) if proc.returncode != 0: - pytest.skip(f"failed to start docker container: {proc.stderr.strip()}") # ty: ignore[too-many-positional-arguments, invalid-argument-type] + pytest.skip(f"failed to start {container_engine} container: {proc.stderr.strip()}") # ty: ignore[too-many-positional-arguments, invalid-argument-type] - container_id = proc.stdout.strip() + cid = proc.stdout.strip() try: - yield container_id + yield cid finally: - subprocess.run(["docker", "rm", "-f", container_id], capture_output=True) + subprocess.run([container_engine, "rm", "-f", cid], capture_output=True) -def test_tools_builtin_runtime_header(docker_container: str): +def test_tools_builtin_runtime_header(container_engine: str, container_id: str): global server server.start() - headers = {"x-tool-runtime": f"docker-container:{docker_container}", "x-tool-cwd": "/tmp"} + headers = {"x-tool-runtime": f"{container_engine}-container:{container_id}", "x-tool-cwd": "/tmp"} - write_res = call_tool("write_file", {"path": "test.log", "content": "hello docker\n"}, headers=headers) + write_res = call_tool("write_file", {"path": "test.log", "content": "hello container\n"}, headers=headers) assert write_res["result"] == "file written successfully" read_res = call_tool("read_file", {"path": "test.log"}, headers=headers) - assert read_res["plain_text_response"] == "hello docker\n" + assert read_res["plain_text_response"] == "hello container\n" exec_res = call_tool("exec_shell_command", {"command": "cat test.log"}, headers=headers) - assert "hello docker" in exec_res["plain_text_response"] + assert "hello container" in exec_res["plain_text_response"] def test_tools_builtin_runtime_header_unknown_scheme(): @@ -208,18 +213,46 @@ def test_tools_builtin_runtime_header_unknown_scheme(): # 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"}) + headers={"x-tool-runtime": "fake:does-not-exist"}) assert res.status_code == 500, res.body assert "unknown tool runtime" in str(res.body) +def test_tools_builtin_runtime_header_rejects_ssh_option_injection(): + global server + server.start() + + # ssh reads options from its argv, so a target starting with '-' must be rejected + res = server.make_request("POST", "/tools", + data={"tool": "exec_shell_command", "params": {"command": "echo hi"}}, + headers={"x-tool-runtime": "ssh:-oProxyCommand=touch /tmp/pwned"}) + assert res.status_code == 500, res.body + assert "invalid ssh target" in str(res.body) + + +@pytest.mark.parametrize("engine", ["docker", "podman"]) +def test_tools_builtin_runtime_header_rejects_container_option_injection(engine: str): + global server + server.start() + + # the container id lands on the ` exec` command line, so an id that looks + # like an option must be rejected + res = server.make_request("POST", "/tools", + data={"tool": "exec_shell_command", "params": {"command": "echo hi"}}, + headers={"x-tool-runtime": f"{engine}-container:--privileged"}) + assert res.status_code == 500, res.body + assert "invalid container id" in str(res.body) + + def test_tools_builtin_docker_runtime_cleans_up_spawned_container(): - reason = _docker_unavailable_reason() + # docker-only: this reads the container hostname to get the spawned id, which only docker + # sets to the short id. podman is covered by the attach path above + reason = _container_engine_unavailable_reason("docker") if reason is not None: pytest.skip(reason) # ty: ignore[too-many-positional-arguments, invalid-argument-type] global server - server.server_tools_runtime = f"docker:{DOCKER_IMAGE}" + server.server_tools_runtime = f"docker:{CONTAINER_IMAGE}" server.start() # exec_shell_command runs inside the container spawned for --tools-runtime; docker sets