diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/HarnessAgent.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/HarnessAgent.java index 2a3b8f7b0..278dee1cc 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/HarnessAgent.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/HarnessAgent.java @@ -2047,6 +2047,9 @@ public HarnessAgent build() { SandboxLifecycleMiddleware sandboxLifecycleMw = null; SandboxContext defaultSandboxContext = null; SandboxBackedFilesystem capturedSandboxFs = null; + // bus/registry are process-scoped infrastructure; keep them on the pre-sandbox + // filesystem so they are never routed through SandboxBackedFilesystem + AbstractFilesystem busFilesystem = filesystem; if (sandboxFilesystemSpec != null) { capturedSandboxFs = new SandboxBackedFilesystem(); filesystem = capturedSandboxFs; @@ -2098,16 +2101,17 @@ public HarnessAgent build() { // ---- MessageBus / AsyncToolRegistry: workspace defaults ---- // If not set explicitly or via DistributedStore, fall back to workspace-backed - // implementations that use the same AbstractFilesystem as the rest of the agent. - if (messageBus == null && filesystem != null) { + // implementations. Use busFilesystem (pre-sandbox) so these process-scoped components + // are never accidentally routed through SandboxBackedFilesystem. + if (messageBus == null && busFilesystem != null) { messageBus = new io.agentscope.harness.agent.bus.WorkspaceMessageBus( - filesystem, ".agentscope/bus"); + busFilesystem, ".agentscope/bus"); } - if (asyncToolRegistry == null && filesystem != null) { + if (asyncToolRegistry == null && busFilesystem != null) { asyncToolRegistry = new io.agentscope.harness.agent.bus.WorkspaceAsyncToolRegistry( - filesystem, ".agentscope/bus/async-tools"); + busFilesystem, ".agentscope/bus/async-tools"); } // ---- Middlewares ---- diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/sandbox/SandboxBackedFilesystem.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/sandbox/SandboxBackedFilesystem.java index 36903ae79..2e7a947c9 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/sandbox/SandboxBackedFilesystem.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/sandbox/SandboxBackedFilesystem.java @@ -28,35 +28,42 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * A {@link BaseSandboxFilesystem} that delegates execution to a live {@link Sandbox}. * - *

Stable proxy created at agent build time; a fresh {@link Sandbox} is injected on each call - * via the volatile {@code sandbox} field by {@link - * io.agentscope.harness.agent.middleware.SandboxLifecycleMiddleware}. + *

Stable proxy created at agent build time. A sandbox is bound per session key on each call via + * {@link io.agentscope.harness.agent.middleware.SandboxLifecycleMiddleware}, allowing a single + * proxy instance to serve concurrent sessions without cross-user contamination. */ public class SandboxBackedFilesystem extends BaseSandboxFilesystem implements SandboxAware { private static final Logger log = LoggerFactory.getLogger(SandboxBackedFilesystem.class); private final String fsId; - private volatile Sandbox sandbox; + // per-session sandbox bindings; key is sessionId (or userId for USER isolation scope) + private final ConcurrentHashMap activeSandboxes = new ConcurrentHashMap<>(); public SandboxBackedFilesystem() { this.fsId = "sandbox-" + UUID.randomUUID().toString().substring(0, 8); } @Override - public void setSandbox(Sandbox sandbox) { - this.sandbox = sandbox; + public void bindSandbox(String sessionKey, Sandbox sandbox) { + activeSandboxes.put(sessionKey, sandbox); } @Override - public Sandbox getSandbox() { - return sandbox; + public void unbindSandbox(String sessionKey) { + activeSandboxes.remove(sessionKey); + } + + @Override + public Sandbox getSandbox(String sessionKey) { + return activeSandboxes.get(sessionKey); } @Override @@ -67,7 +74,7 @@ public String id() { @Override public ExecuteResponse execute( RuntimeContext runtimeContext, String command, Integer timeoutSeconds) { - Sandbox active = requireSandbox(); + Sandbox active = requireSandbox(runtimeContext); try { ExecResult result = active.exec(runtimeContext, command, timeoutSeconds); return new ExecuteResponse( @@ -90,7 +97,7 @@ public ExecuteResponse execute( @Override public List uploadFiles( RuntimeContext runtimeContext, List> files) { - Sandbox active = requireSandbox(); + Sandbox active = requireSandbox(runtimeContext); List results = new ArrayList<>(files.size()); for (Map.Entry file : files) { @@ -134,7 +141,7 @@ public List uploadFiles( @Override public List downloadFiles( RuntimeContext runtimeContext, List paths) { - Sandbox active = requireSandbox(); + Sandbox active = requireSandbox(runtimeContext); List results = new ArrayList<>(paths.size()); for (String path : paths) { @@ -168,11 +175,14 @@ public List downloadFiles( return results; } - private Sandbox requireSandbox() { - Sandbox s = sandbox; + private Sandbox requireSandbox(RuntimeContext rc) { + String key = rc != null ? rc.getSessionId() : null; + Sandbox s = key != null ? activeSandboxes.get(key) : null; if (s == null) { throw new SandboxException.SandboxConfigurationException( - "No active sandbox — sandbox filesystem used outside of a call context"); + "No active sandbox for session '" + + key + + "' — sandbox filesystem used outside of a call context"); } return s; } diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/middleware/SandboxLifecycleMiddleware.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/middleware/SandboxLifecycleMiddleware.java index d754345c7..236d58a0b 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/middleware/SandboxLifecycleMiddleware.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/middleware/SandboxLifecycleMiddleware.java @@ -21,7 +21,7 @@ import io.agentscope.harness.agent.sandbox.SandboxAcquireResult; import io.agentscope.harness.agent.sandbox.SandboxContext; import io.agentscope.harness.agent.sandbox.SandboxManager; -import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.ConcurrentHashMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,8 +53,9 @@ public class SandboxLifecycleMiddleware implements HarnessRuntimeMiddleware { private final SandboxManager sandboxManager; private final SandboxBackedFilesystem filesystemProxy; - private final AtomicReference currentAcquireResult = - new AtomicReference<>(); + // per-session acquire results; keyed by sessionId so concurrent sessions don't interfere + private final ConcurrentHashMap acquireResults = + new ConcurrentHashMap<>(); public SandboxLifecycleMiddleware( SandboxManager sandboxManager, SandboxBackedFilesystem filesystemProxy) { @@ -77,18 +78,20 @@ public void acquireForCall(RuntimeContext ctx) { if (sandboxContext == null) { return; } + String sessionKey = ctx.getSessionId(); try { SandboxAcquireResult result = sandboxManager.acquire(sandboxContext, ctx); Sandbox sandbox = result.getSandbox(); try { sandbox.start(); - filesystemProxy.setSandbox(sandbox); - currentAcquireResult.set(result); + filesystemProxy.bindSandbox(sessionKey, sandbox); + acquireResults.put(sessionKey, result); log.debug( - "[sandbox-mw] Acquired sandbox {}", - sandbox.getState() != null ? sandbox.getState().getSessionId() : "?"); + "[sandbox-mw] Acquired sandbox {} for session {}", + sandbox.getState() != null ? sandbox.getState().getSessionId() : "?", + sessionKey); } catch (Exception e) { - filesystemProxy.setSandbox(null); + filesystemProxy.unbindSandbox(sessionKey); try { sandboxManager.release(result); } catch (Exception releaseErr) { @@ -113,11 +116,12 @@ public void acquireForCall(RuntimeContext ctx) { * @param ctx the per-call RuntimeContext (captured at acquire time) */ public void releaseForCall(RuntimeContext ctx) { - SandboxAcquireResult result = currentAcquireResult.getAndSet(null); + String sessionKey = ctx != null ? ctx.getSessionId() : null; + SandboxAcquireResult result = sessionKey != null ? acquireResults.remove(sessionKey) : null; if (result == null) { return; } - SandboxContext sandboxContext = ctx != null ? ctx.get(SandboxContext.class) : null; + SandboxContext sandboxContext = ctx.get(SandboxContext.class); try { sandboxManager.persistState(result, sandboxContext, ctx); } catch (Exception e) { @@ -129,6 +133,6 @@ public void releaseForCall(RuntimeContext ctx) { log.warn("[sandbox-mw] Failed to release sandbox session: {}", e.getMessage(), e); } result.getLease().close(); - filesystemProxy.setSandbox(null); + filesystemProxy.unbindSandbox(sessionKey); } } diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/SandboxAware.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/SandboxAware.java index d6904e851..40532b133 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/SandboxAware.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/sandbox/SandboxAware.java @@ -21,12 +21,16 @@ * Marks a filesystem that can have its backing {@link Sandbox} injected at runtime. * *

Implemented by {@link SandboxBackedFilesystem} so {@link - * io.agentscope.harness.agent.middleware.SandboxLifecycleMiddleware} can set the active sandbox for each - * call and clear it afterward. + * io.agentscope.harness.agent.middleware.SandboxLifecycleMiddleware} can bind and unbind a sandbox + * per session key, allowing a single filesystem proxy to serve concurrent sessions safely. */ public interface SandboxAware { - void setSandbox(Sandbox sandbox); + // Binds a live sandbox to the given session key for the duration of one call. + void bindSandbox(String sessionKey, Sandbox sandbox); - Sandbox getSandbox(); + // Removes the sandbox binding for the given session key after a call completes. + void unbindSandbox(String sessionKey); + + Sandbox getSandbox(String sessionKey); } diff --git a/agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/sandbox/SandboxBackedFilesystemTest.java b/agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/sandbox/SandboxBackedFilesystemTest.java index 6e2c740ad..6f2d79a5a 100644 --- a/agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/sandbox/SandboxBackedFilesystemTest.java +++ b/agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/sandbox/SandboxBackedFilesystemTest.java @@ -17,30 +17,41 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import io.agentscope.core.agent.RuntimeContext; import io.agentscope.harness.agent.filesystem.model.FileDownloadResponse; import io.agentscope.harness.agent.sandbox.ExecResult; import io.agentscope.harness.agent.sandbox.Sandbox; +import io.agentscope.harness.agent.sandbox.SandboxException; import io.agentscope.harness.agent.sandbox.SandboxState; import java.io.InputStream; import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.Test; class SandboxBackedFilesystemTest { - private static final RuntimeContext RT = RuntimeContext.empty(); + private static RuntimeContext rc(String sessionId) { + return RuntimeContext.builder().sessionId(sessionId).build(); + } @Test void downloadFiles_decodesWrappedBase64Output() { byte[] expected = new byte[] {1, 2, 3, 4, 5, 6}; SandboxBackedFilesystem filesystem = new SandboxBackedFilesystem(); FakeSandbox sandbox = new FakeSandbox(new ExecResult(0, "AQID\nBAUG", "", false)); - filesystem.setSandbox(sandbox); + RuntimeContext ctx = rc("session-a"); + filesystem.bindSandbox(ctx.getSessionId(), sandbox); List responses = - filesystem.downloadFiles(RT, List.of("/tmp/data.bin")); + filesystem.downloadFiles(ctx, List.of("/tmp/data.bin")); assertEquals("base64 '/tmp/data.bin'", sandbox.lastCommand); assertEquals(1, responses.size()); @@ -53,10 +64,11 @@ void downloadFiles_decodesWrappedBase64Output() { void downloadFiles_decodesEmptyPayloadWhenStdoutIsNull() { SandboxBackedFilesystem filesystem = new SandboxBackedFilesystem(); FakeSandbox sandbox = new FakeSandbox(new ExecResult(0, null, "", false)); - filesystem.setSandbox(sandbox); + RuntimeContext ctx = rc("session-a"); + filesystem.bindSandbox(ctx.getSessionId(), sandbox); List responses = - filesystem.downloadFiles(RT, List.of("/tmp/empty.bin")); + filesystem.downloadFiles(ctx, List.of("/tmp/empty.bin")); assertEquals("base64 '/tmp/empty.bin'", sandbox.lastCommand); assertEquals(1, responses.size()); @@ -69,10 +81,11 @@ void downloadFiles_decodesEmptyPayloadWhenStdoutIsNull() { void downloadFiles_returnsFailureWhenCommandFails() { SandboxBackedFilesystem filesystem = new SandboxBackedFilesystem(); FakeSandbox sandbox = new FakeSandbox(new ExecResult(1, "", "boom", false)); - filesystem.setSandbox(sandbox); + RuntimeContext ctx = rc("session-a"); + filesystem.bindSandbox(ctx.getSessionId(), sandbox); List responses = - filesystem.downloadFiles(RT, List.of("/tmp/fail.bin")); + filesystem.downloadFiles(ctx, List.of("/tmp/fail.bin")); assertEquals("base64 '/tmp/fail.bin'", sandbox.lastCommand); assertEquals(1, responses.size()); @@ -81,10 +94,96 @@ void downloadFiles_returnsFailureWhenCommandFails() { assertEquals("[stderr] boom", responses.get(0).error()); } + @Test + void requireSandbox_throwsWhenNoBindingForSession() { + SandboxBackedFilesystem filesystem = new SandboxBackedFilesystem(); + RuntimeContext ctx = rc("unknown-session"); + + assertThrows( + SandboxException.SandboxConfigurationException.class, + () -> filesystem.execute(ctx, "echo hi", null)); + } + + @Test + void bindAndUnbind_isolatesSessionsFromEachOther() { + SandboxBackedFilesystem filesystem = new SandboxBackedFilesystem(); + FakeSandbox sandboxA = new FakeSandbox(new ExecResult(0, "AAAA", "", false)); + FakeSandbox sandboxB = new FakeSandbox(new ExecResult(0, "BBBB", "", false)); + + filesystem.bindSandbox("session-a", sandboxA); + filesystem.bindSandbox("session-b", sandboxB); + + // session-a routes to sandboxA + filesystem.downloadFiles(rc("session-a"), List.of("/f")); + assertNotNull(sandboxA.lastCommand); + assertNull(sandboxB.lastCommand); + + // session-b routes to sandboxB + filesystem.downloadFiles(rc("session-b"), List.of("/g")); + assertNotNull(sandboxB.lastCommand); + + // unbind session-a; session-b still works + filesystem.unbindSandbox("session-a"); + assertThrows( + SandboxException.SandboxConfigurationException.class, + () -> filesystem.execute(rc("session-a"), "echo", null)); + filesystem.downloadFiles(rc("session-b"), List.of("/h")); // must not throw + } + + @Test + void concurrentSessionsDontCrossContaminate() throws InterruptedException { + int sessions = 8; + SandboxBackedFilesystem filesystem = new SandboxBackedFilesystem(); + FakeSandbox[] sandboxes = new FakeSandbox[sessions]; + for (int i = 0; i < sessions; i++) { + sandboxes[i] = new FakeSandbox(new ExecResult(0, "AA==", "", false)); // 1 byte + filesystem.bindSandbox("session-" + i, sandboxes[i]); + } + + CountDownLatch start = new CountDownLatch(1); + CountDownLatch done = new CountDownLatch(sessions); + ExecutorService pool = Executors.newFixedThreadPool(sessions); + String[] errors = new String[sessions]; + + for (int i = 0; i < sessions; i++) { + final int idx = i; + pool.submit( + () -> { + try { + start.await(); + RuntimeContext ctx = rc("session-" + idx); + // each session executes a uniquely named command + filesystem.execute(ctx, "cmd-" + idx, null); + // verify command reached the correct sandbox + String expected = "cmd-" + idx; + if (!expected.equals(sandboxes[idx].lastCommand)) { + errors[idx] = + "session-" + + idx + + " routed to wrong sandbox: " + + sandboxes[idx].lastCommand; + } + } catch (Exception e) { + errors[idx] = e.getMessage(); + } finally { + done.countDown(); + } + }); + } + + start.countDown(); + assertTrue(done.await(10, TimeUnit.SECONDS)); + pool.shutdown(); + + for (int i = 0; i < sessions; i++) { + assertNull(errors[i], "Error in session-" + i + ": " + errors[i]); + } + } + private static final class FakeSandbox implements Sandbox { private final ExecResult execResult; - private String lastCommand; + volatile String lastCommand; private FakeSandbox(ExecResult execResult) { this.execResult = execResult;