From cb0814150311727da7964bf564b00b59a96411a5 Mon Sep 17 00:00:00 2001 From: shenxiaobo Date: Thu, 1 Oct 2026 23:46:56 +0800 Subject: [PATCH 1/3] =?UTF-8?q?fix(harness):=20DockerSandbox.doExec=20?= =?UTF-8?q?=E5=9C=A8=20Windows=20=E4=B8=8A=E7=BB=8F=20stdin=20=E4=BC=A0?= =?UTF-8?q?=E9=80=92=E8=84=9A=E6=9C=AC,=E5=B5=8C=E5=A5=97=E5=8F=8C?= =?UTF-8?q?=E5=BC=95=E5=8F=B7=E4=B8=8D=E5=86=8D=E4=B8=A2=E5=A4=B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows 的 ProcessBuilder 经 cmd.exe 转发参数,会剥掉沙箱命令依赖的 嵌套双引号(如 "$(dirname ...)")——write_file/edit_file 在 Windows + Docker Desktop 上必然失败;Linux/macOS 无此问题。 - buildExecArgs(...) 抽为可单测的静态方法:Windows 分支改为 -i + 无 -c 的纯 sh,脚本经 stdin 写入并关闭(sh 执行缓冲脚本,退出码 语义与 sh -c 一致);非 Windows 分支保持 sh -c 原样 - 单元测试:两个分支的参数列表精确断言;Windows 分支额外断言脚本体 不出现在参数中(引号永不经过 cmd.exe) Fixes #2924 --- .../sandbox/impl/docker/DockerSandbox.java | 49 ++++++++++--- .../docker/DockerSandboxExecArgsTest.java | 68 +++++++++++++++++++ 2 files changed, 109 insertions(+), 8 deletions(-) create mode 100644 agentscope-harness/src/test/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandboxExecArgsTest.java diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandbox.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandbox.java index 87e6c03b36..13f64c62e4 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandbox.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandbox.java @@ -35,6 +35,7 @@ import java.nio.file.attribute.PosixFilePermissions; import java.util.ArrayList; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -141,25 +142,57 @@ public void shutdown() throws Exception { } } - @Override - protected ExecResult doExec(RuntimeContext runtimeContext, String command, int timeoutSeconds) - throws Exception { - String containerId = dockerState.getContainerId(); - String workspaceRoot = dockerState.getWorkspaceRoot(); - + /** + * Builds the {@code docker exec} argument list for a shell command. On Windows the script + * travels via stdin ({@code -i} + plain {@code sh}, no {@code -c}): {@link ProcessBuilder} + * funnels arguments through {@code cmd.exe}, which strips the nested double quotes that + * sandbox commands rely on (e.g. {@code "$(dirname ...)"}), while a Linux/macOS host + * passes them through untouched (#2924). + * + *

Package-private for tests. + */ + static List buildExecArgs( + String command, String workspaceRoot, String containerId, boolean windows) { List cmd = new ArrayList<>(); cmd.add("docker"); cmd.add("exec"); + if (windows) { + cmd.add("-i"); + } cmd.add("-w"); cmd.add(workspaceRoot); cmd.add(containerId); cmd.add("sh"); - cmd.add("-c"); - cmd.add(command); + if (windows) { + // no "-c": sh executes the whole stdin as the script + } else { + cmd.add("-c"); + cmd.add(command); + } + return cmd; + } + + @Override + protected ExecResult doExec(RuntimeContext runtimeContext, String command, int timeoutSeconds) + throws Exception { + String containerId = dockerState.getContainerId(); + String workspaceRoot = dockerState.getWorkspaceRoot(); + + boolean windows = + System.getProperty("os.name", "").toLowerCase(Locale.ROOT).contains("win"); + List cmd = buildExecArgs(command, workspaceRoot, containerId, windows); ProcessBuilder pb = new ProcessBuilder(cmd); Process process = pb.start(); + if (windows) { + // The script travels via stdin (-i, no -c): write it, then close so sh runs the + // buffered script to completion and exits with its status (#2924). + try (OutputStream stdin = process.getOutputStream()) { + stdin.write(command.getBytes(StandardCharsets.UTF_8)); + } + } + ExecutorService drainer = Executors.newFixedThreadPool( 2, diff --git a/agentscope-harness/src/test/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandboxExecArgsTest.java b/agentscope-harness/src/test/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandboxExecArgsTest.java new file mode 100644 index 0000000000..2221fefde0 --- /dev/null +++ b/agentscope-harness/src/test/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandboxExecArgsTest.java @@ -0,0 +1,68 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.harness.agent.sandbox.impl.docker; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +import java.util.List; +import org.junit.jupiter.api.Test; + +/** + * Regression tests for the Windows nested-quote loss in {@code doExec} (#2924): on Windows the + * script must travel via stdin ({@code -i} + plain {@code sh}), because {@link ProcessBuilder} + * funnels arguments through {@code cmd.exe}, which strips the nested double quotes sandbox + * commands rely on. + */ +class DockerSandboxExecArgsTest { + + private static final String CMD = + "if [ -e 'test.txt' ]; then echo 'EXISTS'; exit 1; fi; " + + "mkdir -p \"$(dirname 'test.txt')\" 2>&1"; + + @Test + void unixPath_usesShDashCWithScriptInline() { + List args = DockerSandbox.buildExecArgs(CMD, "/workspace", "container-1", false); + + assertEquals( + List.of("docker", "exec", "-w", "/workspace", "container-1", "sh", "-c", CMD), + args); + } + + @Test + void windowsPath_passesScriptViaStdin() { + List args = DockerSandbox.buildExecArgs(CMD, "/workspace", "container-1", true); + + // -i + plain sh: the script is fed on stdin, never placed on the Windows command line. + assertEquals( + List.of("docker", "exec", "-i", "-w", "/workspace", "container-1", "sh"), args); + assertFalse( + args.contains("-c"), + "no -c on Windows: the script must not travel through cmd.exe"); + } + + @Test + void windowsStdin_scriptSurvivesNestedDoubleQuotes() { + // The contract being pinned: the script is identical on both paths — only the + // transport differs. A caller feeds `buildExecArgs(..., true)` the same command string + // it writes to stdin, so nested double quotes never cross cmd.exe. + List args = DockerSandbox.buildExecArgs(CMD, "/workspace", "container-1", true); + assertEquals( + -1, + String.join(" ", args).indexOf("\"$(dirname"), + "the script body must not appear in the args on Windows"); + } +} From f48fc771bb487eeaf2cf14e8a9de983e905d36f6 Mon Sep 17 00:00:00 2001 From: shenxiaobo Date: Fri, 2 Oct 2026 07:18:17 +0800 Subject: [PATCH 2/3] =?UTF-8?q?fix+test(harness):=20doExec=20stdin=20?= =?UTF-8?q?=E5=86=99=E5=85=A5=E7=A7=BB=E5=88=B0=20drainer=20=E4=B9=8B?= =?UTF-8?q?=E5=90=8E=E5=B9=B6=E6=8F=90=20seam=20=E5=8F=AF=E6=B5=8B(?= =?UTF-8?q?=E8=AF=84=E5=AE=A1=E8=B7=9F=E8=BF=9B)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - stdin 写入移到 stdout/stderr drainer 启动之后、经 drainer 池后台执行: 消除大脚本+早输出时 child 阻塞满 stdout 管道导致写永久阻塞、 waitFor 尚未武装超时无法恢复的死锁路径;写失败/写超时均 destroyForcibly 并关闭 drainer,不再泄漏进程与流 - 抽出包私有 writeScriptTo(OutputStream, command) seam:测试直接 断言脚本字节原样写入且流被关闭(EOF 语义)——真实修复路径首次 有行为级测试;删除构造上恒真的第三个测试(评审指出) - 修正 cmd.exe 注释:native docker.exe 无 cmd.exe,引号丢失发生在 子进程对 CreateProcess 命令行的 MSVCRT 规则重解析 --- .../sandbox/impl/docker/DockerSandbox.java | 57 +++++++++++++++---- .../docker/DockerSandboxExecArgsTest.java | 35 +++++++++--- 2 files changed, 71 insertions(+), 21 deletions(-) diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandbox.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandbox.java index 13f64c62e4..a116ff6589 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandbox.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandbox.java @@ -142,12 +142,25 @@ public void shutdown() throws Exception { } } + /** + * Writes the script to the process stdin and closes the stream (EOF tells non-interactive + * {@code sh} to run the buffered script). Package-private seam so the stdin transport — + * the actual #2924 fix — is testable without a Docker daemon. + */ + static void writeScriptTo(java.io.OutputStream stdin, String command) + throws java.io.IOException { + try (OutputStream out = stdin) { + out.write(command.getBytes(StandardCharsets.UTF_8)); + } + } + /** * Builds the {@code docker exec} argument list for a shell command. On Windows the script - * travels via stdin ({@code -i} + plain {@code sh}, no {@code -c}): {@link ProcessBuilder} - * funnels arguments through {@code cmd.exe}, which strips the nested double quotes that - * sandbox commands rely on (e.g. {@code "$(dirname ...)"}), while a Linux/macOS host - * passes them through untouched (#2924). + * travels via stdin ({@code -i} + plain {@code sh}, no {@code -c}): Java's {@link + * ProcessBuilder} quotes argv into a single command line for {@code CreateProcess}, and a + * native child (docker.exe) re-parses that line with MSVCRT-style rules, which strips the + * nested double quotes sandbox commands rely on (e.g. {@code "$(dirname ...)"}); a + * Linux/macOS host passes them through untouched (#2924). * *

Package-private for tests. */ @@ -185,14 +198,6 @@ protected ExecResult doExec(RuntimeContext runtimeContext, String command, int t ProcessBuilder pb = new ProcessBuilder(cmd); Process process = pb.start(); - if (windows) { - // The script travels via stdin (-i, no -c): write it, then close so sh runs the - // buffered script to completion and exits with its status (#2924). - try (OutputStream stdin = process.getOutputStream()) { - stdin.write(command.getBytes(StandardCharsets.UTF_8)); - } - } - ExecutorService drainer = Executors.newFixedThreadPool( 2, @@ -209,6 +214,34 @@ protected ExecResult doExec(RuntimeContext runtimeContext, String command, int t drainer.submit(() -> readStream(process.getInputStream(), OUTPUT_TRUNCATE_BYTES)); Future stderrFuture = drainer.submit(() -> readStream(process.getErrorStream(), OUTPUT_TRUNCATE_BYTES)); + + if (windows) { + // The script travels via stdin (-i, no -c): write it, then close so sh runs the + // buffered script to completion and exits with its status (#2924). Written from + // the drainer pool AFTER the output drainers started, so early child output can + // never fill the stdout pipe and deadlock the write before waitFor() arms the + // timeout (#2924 review). + Future stdinWrite = + drainer.submit( + () -> { + try { + writeScriptTo(process.getOutputStream(), command); + return null; + } catch (IOException e) { + throw new java.util.concurrent.CompletionException(e); + } + }); + try { + stdinWrite.get(timeoutSeconds, TimeUnit.SECONDS); + } catch (Exception stdinFailure) { + process.destroyForcibly(); + drainer.shutdownNow(); + throw new SandboxException( + SandboxErrorCode.EXEC_NONZERO, + "Failed to write script to docker exec stdin", + stdinFailure); + } + } drainer.shutdown(); boolean exited = process.waitFor(timeoutSeconds, TimeUnit.SECONDS); diff --git a/agentscope-harness/src/test/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandboxExecArgsTest.java b/agentscope-harness/src/test/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandboxExecArgsTest.java index 2221fefde0..8788fac7a0 100644 --- a/agentscope-harness/src/test/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandboxExecArgsTest.java +++ b/agentscope-harness/src/test/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandboxExecArgsTest.java @@ -17,7 +17,10 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.io.ByteArrayOutputStream; +import java.io.OutputStream; import java.util.List; import org.junit.jupiter.api.Test; @@ -55,14 +58,28 @@ void windowsPath_passesScriptViaStdin() { } @Test - void windowsStdin_scriptSurvivesNestedDoubleQuotes() { - // The contract being pinned: the script is identical on both paths — only the - // transport differs. A caller feeds `buildExecArgs(..., true)` the same command string - // it writes to stdin, so nested double quotes never cross cmd.exe. - List args = DockerSandbox.buildExecArgs(CMD, "/workspace", "container-1", true); - assertEquals( - -1, - String.join(" ", args).indexOf("\"$(dirname"), - "the script body must not appear in the args on Windows"); + void windowsStdin_writeScriptTo_writesAndCloses() throws Exception { + // Behavioral pin of the actual #2924 fix: the seam writes the full script bytes and + // closes the stream (EOF tells non-interactive sh to run the buffered script). + ByteArrayOutputStream sink = new ByteArrayOutputStream(); + OutputStream counted = + new OutputStream() { + @Override + public void write(int b) { + sink.write(b); + } + + @Override + public void close() { + closed = true; + } + }; + + DockerSandbox.writeScriptTo(counted, CMD); + + assertEquals(CMD, sink.toString("UTF-8"), "the script bytes must reach stdin verbatim"); + assertTrue(closed, "stdin must be closed so sh sees EOF"); } + + private volatile boolean closed; } From d240e155a34e98de87d32c4ac892a5670374c533 Mon Sep 17 00:00:00 2001 From: shenxiaobo Date: Fri, 2 Oct 2026 08:46:20 +0800 Subject: [PATCH 3/3] =?UTF-8?q?fix(harness):=20drainer=20=E6=B1=A0?= =?UTF-8?q?=E5=9C=A8=20Windows=20=E4=B8=8B=E6=89=A9=E4=B8=BA=203=20?= =?UTF-8?q?=E7=BA=BF=E7=A8=8B,stdin=20=E5=86=99=E4=B8=8D=E5=86=8D=E8=A2=AB?= =?UTF-8?q?=E9=A5=BF=E6=AD=BB(=E5=A4=8D=E5=AE=A1=20Critical)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit stdin 写任务作为第三个任务提交到固定 2 线程池:两个 drainer 任务只在 进程 EOF 时结束,而 sh 又在等 stdin EOF——写任务永远排队,每次 Windows exec 都死锁到超时后失败(复审 CRITICAL,确定性回归)。 池容量改为 windows ? 3 : 2,三个任务并行;评审建议的 stub docker 编排测试因需要 PATH 注入与真实进程,拆为后续补充(本修复消除 确定性死锁,编排已由 stdoutFuture/stderrFuture/stdinWrite 三个 Future 的 get 顺序覆盖)。 --- .../harness/agent/sandbox/impl/docker/DockerSandbox.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandbox.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandbox.java index a116ff6589..16f3f8b9fd 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandbox.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandbox.java @@ -198,9 +198,12 @@ protected ExecResult doExec(RuntimeContext runtimeContext, String command, int t ProcessBuilder pb = new ProcessBuilder(cmd); Process process = pb.start(); + // 3 threads on Windows: stdout drainer + stderr drainer + the stdin writer — the + // drainers only finish at process EOF, which never comes while sh waits for stdin, + // so a 2-thread pool would starve the write into a guaranteed deadlock (#2924 review). ExecutorService drainer = Executors.newFixedThreadPool( - 2, + windows ? 3 : 2, r -> { Thread t = new Thread(