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..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 @@ -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,28 +142,68 @@ 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(); + /** + * 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}): 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. + */ + 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(); + // 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( @@ -176,6 +217,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 new file mode 100644 index 0000000000..8788fac7a0 --- /dev/null +++ b/agentscope-harness/src/test/java/io/agentscope/harness/agent/sandbox/impl/docker/DockerSandboxExecArgsTest.java @@ -0,0 +1,85 @@ +/* + * 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 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; + +/** + * 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_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; +}