diff --git a/packages/plugins/sandbox-providers/daytona/DIRECTORY-CONSTRAINT-FINDINGS.md b/packages/plugins/sandbox-providers/daytona/DIRECTORY-CONSTRAINT-FINDINGS.md new file mode 100644 index 0000000000..9ccabdb89f --- /dev/null +++ b/packages/plugins/sandbox-providers/daytona/DIRECTORY-CONSTRAINT-FINDINGS.md @@ -0,0 +1,68 @@ +# Directory-constraint (`bwrap`) findings for the Daytona session path + +## Summary + +The Daytona provider ran an advisory `bubblewrap` (`bwrap`) wrapper around each +user command. The wrapper gave an agent real-time feedback when the agent wrote +to a directory that the ephemeral sandbox does not keep. The wrapper was +advisory only. It added no security boundary; the ephemeral sandbox is the only +boundary. + +The provider now runs each user command plain, both on the persistent-session +path and on the one-shot fallback path. This note records why the provider drops +the directory-constraint goal for now, and what a future re-introduction needs. + +## Why the goal is dropped for now + +The session model runs one persistent shell per lease and feeds every command +into that shell. The plan aimed to enter the `bwrap` sandbox one time per session +and amortize its cost across every command in the session. A live validation +proved this aim is not reachable with the current Daytona session daemon. + +- An `exec`-replace of the session shell with `bwrap` stops the session from + running any later command. A side-channel file proved the inner shell never + ran the next command. A repeat without `--new-session` gave the same result, + so `--new-session` is not the cause. The `exec`-replace itself breaks the + session daemon's per-command delivery. +- A per-command `bwrap` wrap works inside a session (separated `stdout` and + `stderr`, correct exit codes, the session stays alive), but it costs about + 1.75 s per command. An un-wrapped session command takes about 0.29 s; a + `bwrap`-wrapped session command takes about 2.05 s. The wrap adds the full + cost back on every command, which removes the session model's speed win. + +The wrapper was advisory, not a security boundary. The provider already ran a +command plain when the `bwrap` capability probe failed, so the plain path is an +accepted, shipped behavior. Dropping the wrapper drops only the advisory +feedback; it does not change the command's privilege or isolation. The command +runs as the unprivileged sandbox user (`daytona`) in both cases. + +## Removal scope + +- The session dispatch runs the plain login-shell script, wrapped in a subshell + so a top-level `exit` cannot end the persistent session shell. +- The one-shot fallback (used when the session model is off) also runs the plain + login-shell script. +- The provider removed the per-command `bwrap` apply, the `bwrap` capability + probe, the `bwrap` command builder, and the `bwrapAvailable` / + `sandboxUsername` lease metadata that only the removed path used. +- The provider keeps the advisory writable-directory set, which a sync operation + records from each `access: "rw"` mapping. Nothing reads the set today. It stays + in place so a future isolation wrapper can consume it without a new sync + change. + +## What a future re-introduction needs + +- A persistent-namespace approach needs its own spike. Two candidates are: + - Enter one namespace per session and route each command into it with + `nsenter`. + - Feed each command to the namespace shell through a FIFO instead of an + `exec`-replace. +- Both candidates need a live test against the Daytona session daemon, because + the `exec`-replace model is proven unreachable and the per-command wrap is too + slow. +- The re-introduction must keep the session speed win. It must not pay the full + `bwrap` setup cost on every command. +- The writable-directory set is already tracked per lease, so a new wrapper can + bind those directories read-write without a sync change. +- The wrapper stays advisory. It must not become a security boundary; the + ephemeral sandbox stays the only boundary. diff --git a/packages/plugins/sandbox-providers/daytona/src/file-sync.ts b/packages/plugins/sandbox-providers/daytona/src/file-sync.ts index 3b91438138..be9e51547d 100644 --- a/packages/plugins/sandbox-providers/daytona/src/file-sync.ts +++ b/packages/plugins/sandbox-providers/daytona/src/file-sync.ts @@ -43,7 +43,7 @@ const SPAN_STATUS_CODE_ERROR = 2; * their existing `*.wall_ms` attribute. A per-round-trip span omits it, so it * carries no `*.wall_ms` attribute and relies on the native span width. */ -async function withProviderSpan(input: { +export async function withProviderSpan(input: { name: string; wallMsAttr?: string; attributes?: Record; diff --git a/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts b/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts index 4dd11030a8..7e3d04d9c9 100644 --- a/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts +++ b/packages/plugins/sandbox-providers/daytona/src/plugin.test.ts @@ -34,7 +34,6 @@ import plugin, { __resetDaytonaSandboxHandleCacheForTest, __getDaytonaWritableDirsForTest, __setDaytonaPluginContextForTest, - buildBwrapCommand, } from "./plugin.js"; import type { PluginContext } from "@paperclipai/plugin-sdk"; import manifest from "./manifest.js"; @@ -87,6 +86,15 @@ function createMockSandbox(overrides: { result: "bash", artifacts: { stdout: "bash" }, }), + // Session API (Daytona SDK 0.203.0). The exec hook opens one session per + // lease and dispatches every command into it. `executeSessionCommand` + // returns a `cmdId`; `getSessionCommand` reports the exit code; and + // `getSessionCommandLogs` returns separated `stdout` and `stderr`. + createSession: vi.fn().mockResolvedValue(undefined), + executeSessionCommand: vi.fn().mockResolvedValue({ cmdId: "cmd-1" }), + getSessionCommand: vi.fn().mockResolvedValue({ id: "cmd-1", command: "", exitCode: 0 }), + getSessionCommandLogs: vi.fn().mockResolvedValue({ stdout: "", stderr: "" }), + deleteSession: vi.fn().mockResolvedValue(undefined), }, }; } @@ -160,6 +168,7 @@ describe("Daytona sandbox provider plugin", () => { autoDeleteInterval: -1, reuseLease: true, archiveOnRelease: false, + useSessions: false, }, }); }); @@ -1075,6 +1084,457 @@ describe("Daytona sandbox provider plugin", () => { expect(warnSpy).toHaveBeenCalled(); }); + describe("session model lifecycle (per-lease session store)", () => { + // A recording plugin tracer that captures every provider span the session + // hooks open. It satisfies the structural plugin tracer contract. + const makeRecordingTracer = () => { + const spans: Array<{ + name: string; + attributes: Record; + status: { code: number; message?: string } | null; + ended: boolean; + }> = []; + const tracer = { + startSpan(name: string, options?: { attributes?: Record }) { + const span = { + name, + attributes: { ...(options?.attributes ?? {}) } as Record, + status: null as { code: number; message?: string } | null, + ended: false, + setAttribute(key: string, value: unknown) { + span.attributes[key] = value; + }, + setStatus(status: { code: number; message?: string }) { + span.status = status; + }, + end() { + span.ended = true; + }, + }; + spans.push(span); + return span; + }, + }; + return { tracer, spans }; + }; + + const sessionExecParams = (overrides: Record = {}) => ({ + driverKey: "daytona" as const, + companyId: "company-1", + environmentId: "env-1", + config: { timeoutMs: 300000, reuseLease: false, useSessions: true }, + lease: { providerLeaseId: "sandbox-123", metadata: {} }, + command: "printf", + args: ["hello"], + cwd: "/workspace", + timeoutMs: 1000, + ...overrides, + }); + + it("creates one session on the first execute and reuses it on the next", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + + expect(sandbox.process.createSession).toHaveBeenCalledTimes(1); + const sessionId = sandbox.process.createSession.mock.calls[0]![0] as string; + expect(sessionId).toMatch(/^paperclip-/); + }); + + it("opens one session when two first commands overlap", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + mockGet.mockResolvedValue(sandbox); + + // Hold the first session create open, so the second first command reaches + // the session store before the first create resolves. Without a single + // flight guard, both commands would open a session for one lease and the + // first session id would leak. + let releaseCreate: () => void = () => {}; + const createGate = new Promise((resolve) => { + releaseCreate = resolve; + }); + sandbox.process.createSession.mockImplementationOnce(async () => { + await createGate; + }); + + const first = plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + const second = plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + // Flush the pending microtasks, so both commands park on the held create. + await new Promise((resolve) => setTimeout(resolve, 0)); + releaseCreate(); + await Promise.all([first, second]); + + expect(sandbox.process.createSession).toHaveBeenCalledTimes(1); + const sessionId = sandbox.process.createSession.mock.calls[0]![0] as string; + // Teardown deletes the same single session id, so no session leaks. + await plugin.definition.onEnvironmentReleaseLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "sandbox-123", + config: { timeoutMs: 300000, reuseLease: false, useSessions: true }, + }); + expect(sandbox.process.deleteSession).toHaveBeenCalledTimes(1); + expect(sandbox.process.deleteSession).toHaveBeenCalledWith(sessionId); + }); + + it("opens no session when the session model is off", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.( + sessionExecParams({ config: { timeoutMs: 300000, reuseLease: false } }), + ); + + expect(sandbox.process.createSession).not.toHaveBeenCalled(); + expect(sandbox.process.executeCommand).toHaveBeenCalledTimes(1); + }); + + it("runs a bypassSession command one-shot and leaves the session closed", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + mockGet.mockResolvedValue(sandbox); + + // The provision command runs before the run opens its trace root, so the + // host marks it `bypassSession`. The provider must not open the session for + // it, or the `session.setup` span loses its run parent. + await plugin.definition.onEnvironmentExecute?.( + sessionExecParams({ bypassSession: true }), + ); + + expect(sandbox.process.createSession).not.toHaveBeenCalled(); + expect(sandbox.process.executeCommand).toHaveBeenCalledTimes(1); + }); + + it("opens the session on the first in-run command after a bypassSession command", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + mockGet.mockResolvedValue(sandbox); + + // A bypassSession command runs first (no session), then an in-run command + // opens the one session. The session-setup span then parents to the run + // trace, and every later in-run command reuses the same session. + await plugin.definition.onEnvironmentExecute?.( + sessionExecParams({ bypassSession: true }), + ); + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + + expect(sandbox.process.createSession).toHaveBeenCalledTimes(1); + const sessionId = sandbox.process.createSession.mock.calls[0]![0] as string; + expect(sessionId).toMatch(/^paperclip-/); + }); + + it("deletes the session and clears the store on release", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + const sessionId = sandbox.process.createSession.mock.calls[0]![0] as string; + + await plugin.definition.onEnvironmentReleaseLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "sandbox-123", + config: { timeoutMs: 300000, reuseLease: false, useSessions: true }, + }); + + expect(sandbox.process.deleteSession).toHaveBeenCalledWith(sessionId); + }); + + it("deletes the session at destroy even when the sandbox delete throws", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + sandbox.delete.mockRejectedValueOnce(new Error("delete failed")); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + const sessionId = sandbox.process.createSession.mock.calls[0]![0] as string; + + await expect( + plugin.definition.onEnvironmentDestroyLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "sandbox-123", + config: { timeoutMs: 300000, reuseLease: false, useSessions: true }, + }), + ).rejects.toThrow(/delete failed/); + + expect(sandbox.process.deleteSession).toHaveBeenCalledWith(sessionId); + }); + + it("deletes the session on interactive-setup cancel", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + const sessionId = sandbox.process.createSession.mock.calls[0]![0] as string; + + await plugin.definition.onEnvironmentCancelInteractiveSetup?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "sandbox-123", + config: { timeoutMs: 300000, reuseLease: false, useSessions: true }, + }); + + expect(sandbox.process.deleteSession).toHaveBeenCalledWith(sessionId); + }); + + it("logs loudly and does not throw when the session delete fails", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + sandbox.process.deleteSession.mockRejectedValueOnce(new Error("session gone")); + mockGet.mockResolvedValue(sandbox); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + const sessionId = sandbox.process.createSession.mock.calls[0]![0] as string; + + await plugin.definition.onEnvironmentReleaseLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "sandbox-123", + config: { timeoutMs: 300000, reuseLease: false, useSessions: true }, + }); + + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining(sessionId)); + // The rest of teardown still ran: the ephemeral sandbox was deleted. + expect(sandbox.delete).toHaveBeenCalledWith(300); + }); + + it("clears the session store after delete so no orphan id survives a second teardown", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + mockGet.mockResolvedValue(sandbox); + const releaseParams = { + driverKey: "daytona" as const, + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "sandbox-123", + config: { timeoutMs: 300000, reuseLease: true, useSessions: true }, + }; + + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + await plugin.definition.onEnvironmentReleaseLease?.(releaseParams); + // The store is now clear. A second teardown finds no id and does not delete + // a session again, which proves no orphan id survived the first delete. + await plugin.definition.onEnvironmentReleaseLease?.(releaseParams); + + expect(sandbox.process.deleteSession).toHaveBeenCalledTimes(1); + }); + + it("clears the session store when resume restarts a stopped sandbox", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + // A stopped sandbox lost its session shell, so resume must clear the stale + // id and the next execute must open a fresh session. + const sandbox = createMockSandbox({ state: "stopped" }); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + expect(sandbox.process.createSession).toHaveBeenCalledTimes(1); + + await plugin.definition.onEnvironmentResumeLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "sandbox-123", + config: { timeoutMs: 300000, reuseLease: false, useSessions: true }, + leaseMetadata: { + remoteCwd: "/home/daytona/paperclip-workspace", + workspaceSentinel: { + path: "/home/daytona/paperclip-workspace/.paperclip-runtime/reusable-sandbox-lease.json", + token: "token-1", + }, + }, + }); + + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + expect(sandbox.process.createSession).toHaveBeenCalledTimes(2); + }); + + it("keeps the session when resume runs on a still-running sandbox", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + // A running sandbox keeps its live session shell. Resume must not clear the + // stored id, or a later command opens a second session and teardown deletes + // only one, so the first session leaks until sandbox reaping. + const sandbox = createMockSandbox({ state: "started" }); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + expect(sandbox.process.createSession).toHaveBeenCalledTimes(1); + const sessionId = sandbox.process.createSession.mock.calls[0]![0] as string; + + await plugin.definition.onEnvironmentResumeLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "sandbox-123", + config: { timeoutMs: 300000, reuseLease: false, useSessions: true }, + leaseMetadata: { + remoteCwd: "/home/daytona/paperclip-workspace", + workspaceSentinel: { + path: "/home/daytona/paperclip-workspace/.paperclip-runtime/reusable-sandbox-lease.json", + token: "token-1", + }, + }, + }); + + // The next command reuses the one live session, so no second session opens. + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + expect(sandbox.process.createSession).toHaveBeenCalledTimes(1); + + // Teardown deletes the one session id, so no shell leaks. + await plugin.definition.onEnvironmentReleaseLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "sandbox-123", + config: { timeoutMs: 300000, reuseLease: false, useSessions: true }, + }); + expect(sandbox.process.deleteSession).toHaveBeenCalledTimes(1); + expect(sandbox.process.deleteSession).toHaveBeenCalledWith(sessionId); + }); + + it("emits a session.setup span on create and a session.teardown span on delete", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + mockGet.mockResolvedValue(sandbox); + const { tracer, spans } = makeRecordingTracer(); + const restore = __setDaytonaPluginContextForTest({ tracer } as unknown as PluginContext); + try { + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + const setup = spans.find((span) => span.name === "session.setup"); + expect(setup).toBeDefined(); + expect(setup!.ended).toBe(true); + expect(setup!.attributes["paperclip.sandbox.startup.provider"]).toBe("daytona"); + + await plugin.definition.onEnvironmentReleaseLease?.({ + driverKey: "daytona", + companyId: "company-1", + environmentId: "env-1", + providerLeaseId: "sandbox-123", + config: { timeoutMs: 300000, reuseLease: false, useSessions: true }, + }); + const teardown = spans.find((span) => span.name === "session.teardown"); + expect(teardown).toBeDefined(); + expect(teardown!.ended).toBe(true); + expect(teardown!.attributes["paperclip.sandbox.startup.provider"]).toBe("daytona"); + } finally { + restore(); + } + }); + + it("marks the session.setup span failed when the session create throws", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + sandbox.process.createSession.mockRejectedValueOnce(new Error("create boom")); + mockGet.mockResolvedValue(sandbox); + const { tracer, spans } = makeRecordingTracer(); + const restore = __setDaytonaPluginContextForTest({ tracer } as unknown as PluginContext); + try { + await expect( + plugin.definition.onEnvironmentExecute?.(sessionExecParams()), + ).rejects.toThrow(/create boom/); + const setup = spans.find((span) => span.name === "session.setup"); + expect(setup).toBeDefined(); + expect(setup!.ended).toBe(true); + expect(setup!.status?.code).toBe(2); + } finally { + restore(); + } + }); + + it("dispatches commands into the session and returns separate stdout and stderr", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + sandbox.process.getSessionCommand.mockResolvedValue({ id: "cmd-1", command: "", exitCode: 7 }); + sandbox.process.getSessionCommandLogs.mockResolvedValue({ stdout: "out-here", stderr: "err-here" }); + mockGet.mockResolvedValue(sandbox); + + const result = await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + + // The command runs through the session, not the one-shot path. + expect(sandbox.process.executeSessionCommand).toHaveBeenCalledTimes(1); + expect(sandbox.process.executeCommand).not.toHaveBeenCalled(); + const [sid, req, timeoutArg] = sandbox.process.executeSessionCommand.mock.calls[0] as [ + string, + { command: string; runAsync?: boolean }, + number, + ]; + expect(sid).toMatch(/^paperclip-/); + expect(req.runAsync).toBe(true); + expect(timeoutArg).toBe(1); + // The built command carries the login-shell script and the user command. + expect(req.command).toMatch(/&& env .*'printf' 'hello'/); + // The session command runs plain: no bwrap wrapper and no su privilege drop. + expect(req.command).not.toContain("sudo -n bwrap"); + expect(req.command).not.toContain("su -s /bin/sh"); + // True separated streams come from the logs endpoint. + expect(result).toMatchObject({ exitCode: 7, timedOut: false, stdout: "out-here", stderr: "err-here" }); + expect(typeof (result!.metadata as Record)?.durationMs).toBe("number"); + }); + + it("wraps each user command in a subshell so a top-level exit cannot kill the session shell", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.( + sessionExecParams({ command: "exit", args: ["3"] }), + ); + + const [, req] = sandbox.process.executeSessionCommand.mock.calls[0] as [ + string, + { command: string }, + ]; + // The whole login-shell script (with the user `exit`) runs inside a + // subshell, so a top-level exit ends the subshell, not the session shell. + expect(req.command.trimStart()).toMatch(/^\(/); + expect(req.command.trimEnd()).toMatch(/\)$/); + expect(req.command).toMatch(/'exit' '3'/); + }); + + it("reuses the same session (one persistent shell) across commands", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + mockGet.mockResolvedValue(sandbox); + + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + await plugin.definition.onEnvironmentExecute?.(sessionExecParams()); + + expect(sandbox.process.createSession).toHaveBeenCalledTimes(1); + const firstSid = sandbox.process.executeSessionCommand.mock.calls[0]![0] as string; + const secondSid = sandbox.process.executeSessionCommand.mock.calls[1]![0] as string; + expect(firstSid).toBe(secondSid); + }); + + it("returns a session timeout when the command never reports an exit code", async () => { + process.env.DAYTONA_API_KEY = "host-key"; + const sandbox = createMockSandbox(); + // The command stays running: the exit code never arrives. + sandbox.process.getSessionCommand.mockResolvedValue({ id: "cmd-1", command: "", exitCode: undefined }); + mockGet.mockResolvedValue(sandbox); + + const result = await plugin.definition.onEnvironmentExecute?.( + sessionExecParams({ timeoutMs: 1 }), + ); + + expect(result).toMatchObject({ exitCode: null, timedOut: true }); + expect(result!.stderr).toMatch(/timed out/); + }); + }); + it("executes commands one-shot and returns combined output via stdout", async () => { process.env.DAYTONA_API_KEY = "host-key"; const sandbox = createMockSandbox(); @@ -1233,7 +1693,7 @@ describe("Daytona sandbox provider plugin", () => { }); }); - it("wraps the command when the lease reports bwrap available and username known", async () => { + it("runs the one-shot command plain with no bwrap or su wrapper", async () => { process.env.DAYTONA_API_KEY = "host-key"; const sandbox = createMockSandbox(); mockGet.mockResolvedValue(sandbox); @@ -1243,226 +1703,17 @@ describe("Daytona sandbox provider plugin", () => { companyId: "company-1", environmentId: "env-1", config: { timeoutMs: 300000, reuseLease: false }, - lease: { - providerLeaseId: "sandbox-123", - metadata: { - remoteCwd: "/home/daytona/paperclip-workspace", - bwrapAvailable: true, - sandboxUsername: "daytona", - }, - }, + lease: { providerLeaseId: "sandbox-123", metadata: { remoteCwd: "/home/daytona/paperclip-workspace" } }, command: "printf", args: ["hello"], + cwd: "/workspace", timeoutMs: 1000, }); const [command] = sandbox.process.executeCommand.mock.calls[0] as [string]; - // The wrapper runs `sudo -n bwrap`, drops to the sandbox user via su, and - // binds the workspace directory read-write. - expect(command.startsWith("sudo -n bwrap")).toBe(true); - expect(command).toContain("su -s /bin/sh 'daytona'"); - expect(command).toContain( - "--bind-try '/home/daytona/paperclip-workspace' '/home/daytona/paperclip-workspace'", - ); - // The login-shell script still rides inside the wrapper through `su -c`. - expect(command).toContain("/etc/profile"); - }); - - it("binds collected rw sync directories", async () => { - process.env.DAYTONA_API_KEY = "host-key"; - const remoteCwd = "/home/daytona/paperclip-workspace"; - const collectedDir = `${remoteCwd}/data`; - const hostDir = await fs.mkdtemp(path.join(os.tmpdir(), "daytona-bwrap-test-")); - const source = path.join(hostDir, "in-place.txt"); - await fs.writeFile(source, "bytes"); - const sandbox = createMockSandbox(); - mockGet.mockResolvedValue(sandbox); - - const scopeParams = { - driverKey: "daytona", - companyId: "company-1", - environmentId: "env-1", - config: { timeoutMs: 300000, reuseLease: false }, - }; - - // Record a read-write sync destination under the shared scope. The execute - // hook reads the same scope, so the wrapper must bind this directory too. - await plugin.definition.onEnvironmentSyncIn?.({ - ...scopeParams, - lease: { providerLeaseId: "sandbox-123", metadata: { remoteCwd } }, - operations: [ - { - operationId: "sync-op-rw", - files: [ - { - sourcePath: source, - targetPath: `${collectedDir}/in-place.txt`, - kind: "file" as const, - access: "rw" as const, - }, - ], - }, - ], - }); - sandbox.process.executeCommand.mockClear(); - - await plugin.definition.onEnvironmentExecute?.({ - ...scopeParams, - lease: { - providerLeaseId: "sandbox-123", - metadata: { remoteCwd, bwrapAvailable: true, sandboxUsername: "daytona" }, - }, - command: "printf", - args: ["hello"], - timeoutMs: 1000, - }); - - const [command] = sandbox.process.executeCommand.mock.calls[0] as [string]; - expect(command).toContain(`--bind-try '${collectedDir}' '${collectedDir}'`); - await fs.rm(hostDir, { recursive: true, force: true }); - }); - - it("keeps binding a collected rw directory after a later sync deletes it, so a stale path does not abort a later command", async () => { - // A sync records a read-write destination, then a later operation removes - // that path in the sandbox. The store still holds the path, so the wrapper - // still adds it to the command. `--bind-try` skips a missing source, so the - // command still runs. This test proves the wrapper uses `--bind-try` (not - // `--bind`) for the collected path, which stops a stale path from failing - // every later command for the scope. - process.env.DAYTONA_API_KEY = "host-key"; - const remoteCwd = "/home/daytona/paperclip-workspace"; - const collectedDir = `${remoteCwd}/scratch`; - const hostDir = await fs.mkdtemp(path.join(os.tmpdir(), "daytona-bwrap-stale-")); - const source = path.join(hostDir, "note.txt"); - await fs.writeFile(source, "bytes"); - const sandbox = createMockSandbox(); - mockGet.mockResolvedValue(sandbox); - - const scopeParams = { - driverKey: "daytona", - companyId: "company-1", - environmentId: "env-1", - config: { timeoutMs: 300000, reuseLease: false }, - }; - - await plugin.definition.onEnvironmentSyncIn?.({ - ...scopeParams, - lease: { providerLeaseId: "sandbox-123", metadata: { remoteCwd } }, - operations: [ - { - operationId: "sync-op-rw", - files: [ - { - sourcePath: source, - targetPath: `${collectedDir}/note.txt`, - kind: "file" as const, - access: "rw" as const, - }, - ], - }, - ], - }); - sandbox.process.executeCommand.mockClear(); - - await plugin.definition.onEnvironmentExecute?.({ - ...scopeParams, - lease: { - providerLeaseId: "sandbox-123", - metadata: { remoteCwd, bwrapAvailable: true, sandboxUsername: "daytona" }, - }, - command: "printf", - args: ["hello"], - timeoutMs: 1000, - }); - - const [command] = sandbox.process.executeCommand.mock.calls[0] as [string]; - // The stale collected path binds with `--bind-try`, so a missing source is - // skipped and the command still runs. - expect(command).toContain(`--bind-try '${collectedDir}' '${collectedDir}'`); - // No writable directory uses a plain `--bind`, so no stale source can abort. - expect(command).not.toMatch(/(^| )--bind '/); - await fs.rm(hostDir, { recursive: true, force: true }); - }); - - it("re-binds the stdin path when stdin is present and bwrap is available", async () => { - process.env.DAYTONA_API_KEY = "host-key"; - const sandbox = createMockSandbox(); - mockGet.mockResolvedValue(sandbox); - - await plugin.definition.onEnvironmentExecute?.({ - driverKey: "daytona", - companyId: "company-1", - environmentId: "env-1", - config: { timeoutMs: 300000, reuseLease: false }, - lease: { - providerLeaseId: "sandbox-123", - metadata: { - remoteCwd: "/home/daytona/paperclip-workspace", - bwrapAvailable: true, - sandboxUsername: "daytona", - }, - }, - command: "cat", - args: [], - stdin: "input payload", - timeoutMs: 1000, - }); - - const stdinPath = sandbox.fs.uploadFile.mock.calls[0][1] as string; - expect(stdinPath).toMatch(/^\/tmp\/paperclip-stdin-/); - const [command] = sandbox.process.executeCommand.mock.calls[0] as [string]; - // The `--tmpfs /tmp` flag hides the uploaded stdin file, so the wrapper must - // re-bind it read-only after the tmpfs. - expect(command).toContain(`--ro-bind '${stdinPath}' '${stdinPath}'`); - }); - - it("runs the plain command when bwrap is unavailable or username is unknown", async () => { - process.env.DAYTONA_API_KEY = "host-key"; - const sandbox = createMockSandbox(); - mockGet.mockResolvedValue(sandbox); - - // Case 1: the probe reported bwrap unavailable. - await plugin.definition.onEnvironmentExecute?.({ - driverKey: "daytona", - companyId: "company-1", - environmentId: "env-1", - config: { timeoutMs: 300000, reuseLease: false }, - lease: { - providerLeaseId: "sandbox-123", - metadata: { - remoteCwd: "/home/daytona/paperclip-workspace", - bwrapAvailable: false, - sandboxUsername: "daytona", - }, - }, - command: "printf", - args: ["hello"], - timeoutMs: 1000, - }); - expect((sandbox.process.executeCommand.mock.calls[0] as [string])[0]).not.toContain("bwrap"); - - // Case 2: bwrap is available but the username is unknown. A wrap without a - // username would run as root inside bwrap and give the agent's files root - // ownership, so the seam keeps the plain command. - sandbox.process.executeCommand.mockClear(); - await plugin.definition.onEnvironmentExecute?.({ - driverKey: "daytona", - companyId: "company-1", - environmentId: "env-1", - config: { timeoutMs: 300000, reuseLease: false }, - lease: { - providerLeaseId: "sandbox-123", - metadata: { - remoteCwd: "/home/daytona/paperclip-workspace", - bwrapAvailable: true, - sandboxUsername: null, - }, - }, - command: "printf", - args: ["hello"], - timeoutMs: 1000, - }); - expect((sandbox.process.executeCommand.mock.calls[0] as [string])[0]).not.toContain("bwrap"); + expect(command).not.toContain("sudo -n bwrap"); + expect(command).not.toContain("su -s /bin/sh"); + expect(command).toMatch(/'printf' 'hello'$/); }); it("rejects invalid shell env keys before execution", async () => { @@ -3937,238 +4188,3 @@ describe("daytona manifest form defaults", () => { } }); }); - -describe("buildBwrapCommand advisory wrapper builder", () => { - it("uses su to drop to sandbox user without --unshare-user", () => { - const command = buildBwrapCommand( - "echo hi", - ["/home/daytona/paperclip-workspace"], - null, - "daytona", - ); - - expect(command).toBe( - "sudo -n bwrap " - + "--ro-bind / / --dev /dev --proc /proc --tmpfs /tmp " - + "--bind-try '/home/daytona/paperclip-workspace' '/home/daytona/paperclip-workspace' " - + "--new-session -- su -s /bin/sh 'daytona' -c 'echo hi'", - ); - expect(command).not.toContain("--unshare-user"); - expect(command).not.toContain("--uid"); - expect(command).not.toContain("--gid"); - }); - - it("re-binds the stdin path after tmpfs /tmp", () => { - const command = buildBwrapCommand( - "run-cmd", - ["/work"], - "/tmp/stdin.bin", - "daytona", - ); - - expect(command).toBe( - "sudo -n bwrap " - + "--ro-bind / / --dev /dev --proc /proc --tmpfs /tmp " - + "--bind-try '/work' '/work' " - + "--ro-bind '/tmp/stdin.bin' '/tmp/stdin.bin' " - + "--new-session -- su -s /bin/sh 'daytona' -c 'run-cmd'", - ); - // The stdin re-bind must come after the tmpfs, so the tmpfs does not hide it. - expect(command.indexOf("--ro-bind '/tmp/stdin.bin'")).toBeGreaterThan(command.indexOf("--tmpfs /tmp")); - }); - - it("quotes writable paths and inner script with single quotes", () => { - const command = buildBwrapCommand( - "echo 'hello'", - ["/data/o'brien"], - null, - "daytona", - ); - - // shellQuote rewrites each embedded single quote as the `'"'"'` token. - expect(command).toContain(`'"'"'`); - expect(command).toContain(`--bind-try '/data/o'"'"'brien' '/data/o'"'"'brien'`); - expect(command).toContain(`-c 'echo '"'"'hello'"'"''`); - }); - - it("falls back to sh -c when no username is given", () => { - const command = buildBwrapCommand("plain", ["/w"], null, null); - - expect(command).toBe( - "sudo -n bwrap " - + "--ro-bind / / --dev /dev --proc /proc --tmpfs /tmp " - + "--bind-try '/w' '/w' " - + "--new-session -- sh -c 'plain'", - ); - expect(command).not.toContain("--unshare-user"); - expect(command).not.toContain("su -s"); - }); - - it("binds each writable directory with --bind-try so a stale or deleted path does not abort bwrap", () => { - // The writable set holds advisory sandbox paths. A later sync can delete or - // replace one. `--bind` aborts when the source is absent; `--bind-try` skips - // a missing source and runs the command. Assert the builder emits - // `--bind-try` for every writable directory and never a plain `--bind` for - // them, so one stale path never poisons a later command. - const command = buildBwrapCommand( - "echo hi", - ["/home/daytona/paperclip-workspace", "/home/daytona/data"], - null, - "daytona", - ); - - expect(command).toContain( - "--bind-try '/home/daytona/paperclip-workspace' '/home/daytona/paperclip-workspace'", - ); - expect(command).toContain("--bind-try '/home/daytona/data' '/home/daytona/data'"); - // The only plain `--bind` family in the command is the read-only bind - // (`--ro-bind`). No writable directory uses a plain `--bind`. - expect(command).not.toMatch(/(^| )--bind '/); - }); -}); - -describe("advisory bwrap capability probe at lease time", () => { - // Route each probed command to a deterministic result so the hook exercises - // the real end-to-end path: shell detect, bwrap capability, and username read. - function bwrapExecMock(opts: { - bwrapExit?: number; - username?: string; - usernameExit?: number; - sentinelToken?: string; - } = {}) { - return async (command: string) => { - if (command.startsWith("cat ")) { - const token = opts.sentinelToken ?? "sentinel-token"; - return { exitCode: 0, result: JSON.stringify({ token }), artifacts: { stdout: JSON.stringify({ token }) } }; - } - if (command.includes("command -v bash")) { - return { exitCode: 0, result: "bash", artifacts: { stdout: "bash" } }; - } - if (command.startsWith("sudo -n bwrap")) { - return { exitCode: opts.bwrapExit ?? 0, result: "", artifacts: { stdout: "" } }; - } - if (command === "id -un") { - const username = opts.username ?? "daytona"; - return { exitCode: opts.usernameExit ?? 0, result: username, artifacts: { stdout: username } }; - } - return { exitCode: 0, result: "", artifacts: { stdout: "" } }; - }; - } - - const acquireParams = { - driverKey: "daytona", - companyId: "company-1", - environmentId: "env-1", - runId: "run-1", - agentId: "agent-1", - executionWorkspaceId: "workspace-1", - adapterType: "codex_local", - config: { image: "node:20", timeoutMs: 300000, reuseLease: true }, - }; - - it("records bwrap available and reads username when the probe exits zero", async () => { - process.env.DAYTONA_API_KEY = "host-key"; - const sandbox = createMockSandbox(); - sandbox.process.executeCommand.mockImplementation(bwrapExecMock({ bwrapExit: 0, username: "daytona" })); - mockCreate.mockResolvedValue(sandbox); - - const lease = await plugin.definition.onEnvironmentAcquireLease?.(acquireParams); - - expect(lease).toMatchObject({ - metadata: { - bwrapAvailable: true, - sandboxUsername: "daytona", - }, - }); - }); - - it("bounds the probe timeout well under the hook deadline so the hook returns fallback metadata", async () => { - process.env.DAYTONA_API_KEY = "host-key"; - const sandbox = createMockSandbox(); - sandbox.process.executeCommand.mockImplementation(bwrapExecMock({ bwrapExit: 0, username: "daytona" })); - mockCreate.mockResolvedValue(sandbox); - - // The hook deadline is 300 s; the probe must cap far below it. - await plugin.definition.onEnvironmentAcquireLease?.(acquireParams); - - const probeCalls = sandbox.process.executeCommand.mock.calls.filter( - ([command]: [string]) => command === "id -un" || command.startsWith("sudo -n bwrap"), - ); - expect(probeCalls.length).toBeGreaterThan(0); - for (const call of probeCalls) { - const timeoutArg = call[3] as number; - expect(timeoutArg).toBe(10); - } - }); - - it("records bwrap unavailable when the capability probe exits non-zero", async () => { - process.env.DAYTONA_API_KEY = "host-key"; - const sandbox = createMockSandbox(); - sandbox.process.executeCommand.mockImplementation(bwrapExecMock({ bwrapExit: 1, username: "daytona" })); - mockCreate.mockResolvedValue(sandbox); - - const lease = await plugin.definition.onEnvironmentAcquireLease?.(acquireParams); - - expect(lease?.metadata).toMatchObject({ bwrapAvailable: false }); - }); - - it("records bwrap unavailable and does not throw when the probe throws", async () => { - process.env.DAYTONA_API_KEY = "host-key"; - const sandbox = createMockSandbox(); - sandbox.process.executeCommand.mockRejectedValue(new Error("sandbox exec failed")); - mockCreate.mockResolvedValue(sandbox); - - const lease = await plugin.definition.onEnvironmentAcquireLease?.(acquireParams); - - expect(lease?.metadata).toMatchObject({ - bwrapAvailable: false, - sandboxUsername: null, - }); - }); - - it("runs the probe on the environment probe hook", async () => { - process.env.DAYTONA_API_KEY = "host-key"; - const sandbox = createMockSandbox(); - sandbox.process.executeCommand.mockImplementation(bwrapExecMock({ bwrapExit: 0, username: "daytona" })); - mockCreate.mockResolvedValue(sandbox); - - const result = await plugin.definition.onEnvironmentProbe?.({ - driverKey: "daytona", - companyId: "company-1", - environmentId: "env-1", - config: { snapshot: "base-snapshot", timeoutMs: 300000, reuseLease: false }, - }); - - expect(result).toMatchObject({ - ok: true, - metadata: { bwrapAvailable: true, sandboxUsername: "daytona" }, - }); - }); - - it("runs the probe on the resume-lease hook", async () => { - process.env.DAYTONA_API_KEY = "host-key"; - const sandbox = createMockSandbox({ id: "sandbox-reuse", state: "stopped" }); - sandbox.process.executeCommand.mockImplementation(bwrapExecMock({ bwrapExit: 0, username: "daytona" })); - mockGet.mockResolvedValue(sandbox); - - const lease = await plugin.definition.onEnvironmentResumeLease?.({ - driverKey: "daytona", - companyId: "company-1", - environmentId: "env-1", - providerLeaseId: "sandbox-reuse", - config: { timeoutMs: 300000, reuseLease: true }, - leaseMetadata: { - workspaceSentinel: { - path: "/home/daytona/paperclip-workspace/.paperclip-runtime/reusable-sandbox-lease.json", - token: "sentinel-token", - result: "written", - }, - }, - }); - - expect(lease).toMatchObject({ - providerLeaseId: "sandbox-reuse", - metadata: { bwrapAvailable: true, sandboxUsername: "daytona" }, - }); - }); -}); diff --git a/packages/plugins/sandbox-providers/daytona/src/plugin.ts b/packages/plugins/sandbox-providers/daytona/src/plugin.ts index 6efa06ed58..e00c0a2ca0 100644 --- a/packages/plugins/sandbox-providers/daytona/src/plugin.ts +++ b/packages/plugins/sandbox-providers/daytona/src/plugin.ts @@ -40,7 +40,7 @@ import type { PluginEnvironmentValidationResult, PluginSyncOperation, } from "@paperclipai/plugin-sdk"; -import { performSyncIn, performSyncOut } from "./file-sync.js"; +import { performSyncIn, performSyncOut, withProviderSpan } from "./file-sync.js"; // Injectable monotonic clock for provider-boundary timing (Open Q1). Defaults // to the real wall clock; `plugin.test.ts` overrides it via @@ -122,6 +122,7 @@ interface DaytonaDriverConfig { autoDeleteInterval: number | null; reuseLease: boolean; archiveOnRelease: boolean; + useSessions: boolean; } type WorkspaceSentinelResult = { @@ -175,14 +176,6 @@ const ARCHIVE_ON_RELEASE_AUTO_DELETE_MINUTES = 60; // RPC ceiling; callers always see an actionable error within this window. const GIT_NETWORK_TIMEOUT_MS = 120_000; -// Fail-fast cap for the advisory bwrap capability probe. The probe is -// best-effort, so it must return fallback metadata inside the lease hook. It -// must never consume the full hook deadline. A stalled probe command would -// otherwise expire the outer lease RPC before the probe records its unavailable -// result. This short cap keeps the probe well under the hook deadline, so the -// probe fails fast, records `bwrapAvailable: false`, and the hook still returns. -const BWRAP_PROBE_TIMEOUT_MS = 10_000; - // Noninteractive git credential defaults injected into every Daytona one-shot // command so that git operations never stall waiting for a terminal prompt. // Callers can override any of these via the env parameter. @@ -231,6 +224,11 @@ function parseDriverConfig(raw: Record): DaytonaDriverConfig { autoDeleteInterval: parseOptionalInteger(raw.autoDeleteInterval) ?? DEFAULT_AUTO_DELETE_INTERVAL_MINUTES, reuseLease: raw.reuseLease === true, archiveOnRelease: raw.archiveOnRelease === true, + // Session model opt-in. Default OFF. When off, the provider keeps the + // one-shot command path. When on, the exec hook opens one persistent + // Daytona session per lease and dispatches every command into it. The flag + // stays default off until a live leak soak passes. + useSessions: raw.useSessions === true, }; } @@ -330,14 +328,6 @@ function toTimeoutSeconds(timeoutMs: number): number { return Math.max(1, Math.ceil(timeoutMs / 1000)); } -// Bounded timeout for the advisory bwrap capability probe. The probe never uses -// the full hook deadline. It uses the smaller of the short probe cap and the -// hook timeout, so a stalled probe command returns fallback metadata inside the -// hook instead of expiring the outer lease RPC. -function toBwrapProbeTimeoutSeconds(config: DaytonaDriverConfig): number { - return toTimeoutSeconds(Math.min(BWRAP_PROBE_TIMEOUT_MS, config.timeoutMs)); -} - function resolveTimeoutMs(paramsTimeoutMs: number | undefined, config: DaytonaDriverConfig): number { return paramsTimeoutMs != null && Number.isFinite(paramsTimeoutMs) && paramsTimeoutMs > 0 ? Math.trunc(paramsTimeoutMs) @@ -415,75 +405,6 @@ function parseProbeInteger(value: string | undefined | null): number | null { return Number.isInteger(parsed) ? parsed : null; } -// Best-effort probe for the sandbox user's username. It runs `id -un` as the -// normal sandbox user (no `sudo`). The username is an image fact, not a code -// fact, so the probe is the only source of truth; the wrapper never assumes a -// hardcoded username. A non-zero exit code, an empty output, or a thrown error -// records no username. The probe never throws. -async function detectSandboxUsername( - sandbox: Sandbox, - timeoutSeconds: number, -): Promise { - try { - const result = await sandbox.process.executeCommand("id -un", undefined, undefined, timeoutSeconds); - if (result.exitCode !== 0) return null; - const username = result.result?.trim() ?? ""; - return username.length > 0 ? username : null; - } catch { - return null; - } -} - -// Best-effort probe for the advisory bwrap capability. The probe tests the real -// end-to-end capability using the su-based approach, not the old user-namespace -// approach. One command exercises the binary, the passwordless `sudo -n` rule, -// and the su user-switch together. The probe optionally binds the workspace -// directory (`--bind-try` suppresses ENOENT but not EACCES; on some Daytona -// images the home dir is `drwx------`, so including the workspace bind here -// catches that). A zero exit code means the capability is present. A non-zero -// exit code or a thrown error means the capability is absent. The probe never -// throws. It records the result, and the caller runs the command unwrapped when -// the capability is absent. -async function detectBwrapAvailable( - sandbox: Sandbox, - timeoutSeconds: number, - username: string, - remoteCwd?: string, -): Promise { - try { - const workspaceBind = remoteCwd - ? ` --bind-try ${shellQuote(remoteCwd)} ${shellQuote(remoteCwd)}` - : ""; - const result = await sandbox.process.executeCommand( - `sudo -n bwrap --ro-bind / /${workspaceBind} -- su -s /bin/sh ${shellQuote(username)} -c true`, - undefined, - undefined, - timeoutSeconds, - ); - return result.exitCode === 0; - } catch { - return false; - } -} - -// Run advisory bwrap probes sequentially (username first, then the bwrap probe -// which needs the username to construct the su command). The username is the -// ground-truth read; the wrapper relies on the probed name and never a -// hardcoded default. A missing username short-circuits and returns unavailable, -// so the caller runs the command unwrapped. Neither probe fails the lease. -async function detectBwrapCapability( - sandbox: Sandbox, - timeoutSeconds: number, - remoteCwd?: string, -): Promise<{ bwrapAvailable: boolean; sandboxUsername: string | null }> { - const username = await detectSandboxUsername(sandbox, timeoutSeconds); - if (username === null) { - return { bwrapAvailable: false, sandboxUsername: null }; - } - const bwrapAvailable = await detectBwrapAvailable(sandbox, timeoutSeconds, username, remoteCwd); - return { bwrapAvailable, sandboxUsername: username }; -} - function workspaceSentinelToken(input: { params: Pick; config: DaytonaDriverConfig; @@ -591,8 +512,6 @@ function leaseMetadata(input: { config: DaytonaDriverConfig; sandbox: Sandbox; shellCommand: "bash" | "sh"; - bwrapAvailable: boolean; - sandboxUsername: string | null; remoteCwd: string; resumedLease: boolean; workspaceSentinel?: WorkspaceSentinelResult; @@ -600,10 +519,6 @@ function leaseMetadata(input: { return { provider: "daytona", shellCommand: input.shellCommand, - // Advisory bwrap capability probed at lease time. `bwrapAvailable` false - // runs the command unwrapped; it never fails the lease. - bwrapAvailable: input.bwrapAvailable, - sandboxUsername: input.sandboxUsername, sandboxId: input.sandbox.id, sandboxName: input.sandbox.name, sandboxState: input.sandbox.state ?? null, @@ -631,64 +546,6 @@ function shellQuote(value: string): string { return `'${value.replace(/'/g, `'"'"'`)}'`; } -// Advisory bubblewrap (`bwrap`) wrapper. -// -// The wrapper gives an agent real-time feedback when the agent tries to change a -// file that the ephemeral sandbox will not keep. It adds NO security. The -// ephemeral sandbox stays the only security posture. The read-only root -// (`--ro-bind / /`) is a feedback signal, not a control: a write to a path -// outside the writable set fails at once, so the agent learns the change is not -// durable. -// -// `buildBwrapCommand` is pure. It builds one command string and runs no process. -// It needs no live sandbox. The flag order is load-bearing, because a later -// filesystem operation over the same path wins. So the writable `--bind` flags -// and the stdin re-bind must come after the read-only root and the fresh -// pseudo-filesystems. The function emits the flags in this fixed order: -// 1. `--ro-bind / /` (read-only root — the static system allowance base). -// 2. `--dev /dev --proc /proc --tmpfs /tmp` (fresh pseudo-filesystems). -// 3. one `--bind-try ` per writable directory, in the caller's order. -// 4. `--ro-bind ` when a stdin path is supplied. -// 5. `--new-session`. -// 6. `-- su -s /bin/sh '' -c ''` when a -// username is supplied, or `-- sh -c ''` otherwise. -// -// `sudo -n bwrap` runs as real root (for bind-mount capability). `su` then -// drops into the sandbox user, so inside uid= maps to outside uid= -// and workspace files owned by that uid are writable. The old `--unshare-user -// --uid`/`--gid` approach created a uid_map that made workspace files appear as -// overflow uid 65534 (nobody) from inside the namespace, causing EACCES. -// -// The writable binds use `--bind-try`, not `--bind`. The writable set is an -// advisory in-memory collection of sandbox paths. The host cannot check whether -// a sandbox path still exists. A path can be deleted or replaced after the store -// records it. `--bind` aborts bwrap when the source is absent, so one stale path -// would fail every later command for the scope. `--bind-try` skips a missing -// source and runs the command, which keeps the wrapper advisory and best-effort. -export function buildBwrapCommand( - innerScript: string, - writableDirs: string[], - stdinPath: string | null, - username: string | null, -): string { - const rootBinds = ["--ro-bind", "/", "/", "--dev", "/dev", "--proc", "/proc", "--tmpfs", "/tmp"]; - const writableBinds = writableDirs.flatMap((dir) => ["--bind-try", shellQuote(dir), shellQuote(dir)]); - // Re-bind the stdin file after `--tmpfs /tmp`, so the tmpfs does not hide it. - const stdinReBind = stdinPath ? ["--ro-bind", shellQuote(stdinPath), shellQuote(stdinPath)] : []; - const tail = username - ? ["--new-session", "--", "su", "-s", "/bin/sh", shellQuote(username), "-c", shellQuote(innerScript)] - : ["--new-session", "--", "sh", "-c", shellQuote(innerScript)]; - return [ - "sudo", - "-n", - "bwrap", - ...rootBinds, - ...writableBinds, - ...stdinReBind, - ...tail, - ].join(" "); -} - function resolveConnectionExpiresInMinutes(value: number | null | undefined): number { if (typeof value !== "number" || !Number.isFinite(value)) return DEFAULT_SSH_ACCESS_MINUTES; return Math.min(24 * 60, Math.max(1, Math.trunc(value))); @@ -1296,19 +1153,16 @@ const sandboxHandleCache = (() => { })(); // Advisory writable-set store. It holds, per lease scope, the sandbox -// directories that a sync operation declared read-write (`access: "rw"`). An -// optional sandbox feedback wrapper reads this set later to bind those -// directories read-write, so an agent gets real-time feedback when a write to a -// non-persistent path fails. The store is advisory and best-effort in-memory -// state: it adds no security (the ephemeral sandbox stays the only boundary), -// and a cold store (for example after a worker restart) degrades to the -// workspace baseline, never to a crash. The store is keyed the same way as -// `sandboxHandleCache`, by `sandboxHandleCacheKey(scope)`. +// directories that a sync operation declared read-write (`access: "rw"`). The +// store is advisory and best-effort in-memory state: it adds no security (the +// ephemeral sandbox stays the only boundary). The store is keyed the same way +// as `sandboxHandleCache`, by `sandboxHandleCacheKey(scope)`. // -// The store keeps a path after a sync records it, so a path that a later -// operation deletes or replaces can stay in the set. The wrapper binds each -// path with `bwrap --bind-try`, which skips a missing source, so a stale path -// never fails a later command. See `buildBwrapCommand`. +// The command path no longer reads this set. The provider dropped the advisory +// `bwrap` wrapper that once bound these directories read-write for real-time +// feedback (see `DIRECTORY-CONSTRAINT-FINDINGS.md`). The store still records the +// read-write set, so a future isolation wrapper for the session can consume it +// without a new sync change. const sandboxHandleWritableDirs = (() => { const dirsByKey = new Map>(); @@ -1349,6 +1203,61 @@ const sandboxHandleWritableDirs = (() => { return { recordWritableTargets, get, reset }; })(); +// Per-lease Daytona session-id store. It holds, per lease scope, the id of the +// one persistent session the exec hook opened for that lease. The store is +// keyed the same way as `sandboxHandleCache`, by `sandboxHandleCacheKey(scope)`. +// The exec hook creates one session on a cache miss and records its id here. The +// teardown hooks delete the session and clear the id. A resume clears the id, +// because a restarted sandbox loses its session shell, so the next exec must +// open a fresh session. The store is process-memory only; it holds an id string, +// never a handle, a credential, or a command. +const sandboxHandleSessionStore = (() => { + const idByKey = new Map(); + // In-flight session creates, keyed the same way as `idByKey`. A create records + // its promise here for the time it runs, then removes it. The map lets two + // overlapping first commands for one lease share one create. See `runSingle`. + const pendingByKey = new Map>(); + + function get(scope: SandboxScope): string | undefined { + return idByKey.get(sandboxHandleCacheKey(scope)); + } + + function set(scope: SandboxScope, sessionId: string): void { + idByKey.set(sandboxHandleCacheKey(scope), sessionId); + } + + function clear(scope: SandboxScope): void { + idByKey.delete(sandboxHandleCacheKey(scope)); + } + + // Single-flight guard for the first-command session create. Two overlapping + // first commands for one lease must open at most one live session. The first + // caller runs `create` and records its in-flight promise; every concurrent + // caller awaits the same promise instead of a second `create`. The store keeps + // the promise only while `create` runs, then removes it, so a later command + // (for example, after a resume clears the id) can open a fresh session. A + // failed `create` removes the promise too, so the next command retries. + function runSingle(scope: SandboxScope, create: () => Promise): Promise { + const key = sandboxHandleCacheKey(scope); + const inFlight = pendingByKey.get(key); + if (inFlight) return inFlight; + const promise = create(); + pendingByKey.set(key, promise); + const settle = (): void => { + pendingByKey.delete(key); + }; + promise.then(settle, settle); + return promise; + } + + function reset(): void { + idByKey.clear(); + pendingByKey.clear(); + } + + return { get, set, clear, runSingle, reset }; +})(); + /** * Test seam: clear the process-scoped handle cache between tests so a handle * memoized under a reused composite key in one test never leaks into the next. @@ -1360,6 +1269,7 @@ export function __resetDaytonaSandboxHandleCacheForTest(): void { sandboxHandleActivityGates.reset(); sandboxHandleLeaseAdmissionStates.reset(); sandboxHandleWritableDirs.reset(); + sandboxHandleSessionStore.reset(); } /** @@ -1403,47 +1313,72 @@ function evictSandboxHandle(scope: SandboxScope): void { sandboxHandleCache.clear(scope); } -// Advisory bwrap execution plan. When present, `executeOneShot` wraps the -// login-shell string with `buildBwrapCommand`. When null, it runs the plain -// string, which keeps today's behavior. `writableDirs` holds the workspace -// directory (baseline, always read-write) plus the collected read-write sync -// destinations. `username` is the sandbox user to su into inside bwrap. -type BwrapExecPlan = { - writableDirs: string[]; - username: string; -}; - -// Decide whether the advisory bwrap wrapper runs for one exec. The wrapper runs -// only when the lease reports bwrap available, a username is known, and the -// workspace directory is known. A wrap without a username would run as root -// inside bwrap and give the agent's files root ownership, so this returns null -// (run the plain command) in that case. The writable set is the workspace -// directory (baseline, always read-write) plus the per-scope read-write sync -// destinations. The baseline guarantees a safe result even when the collected -// store is cold. -function resolveBwrapExecPlan( - metadata: Record | null | undefined, - scope: SandboxScope, -): BwrapExecPlan | null { - if (metadata?.bwrapAvailable !== true) return null; - const username = typeof metadata.sandboxUsername === "string" ? metadata.sandboxUsername.trim() : ""; - if (username.length === 0) return null; - const remoteCwd = typeof metadata.remoteCwd === "string" ? metadata.remoteCwd.trim() : ""; - if (remoteCwd.length === 0) return null; - const writableDirs = new Set([remoteCwd]); - for (const dir of sandboxHandleWritableDirs.get(scope)) { - writableDirs.add(dir); - } - return { writableDirs: [...writableDirs], username }; +// Return the persistent session id for a lease, and open one session on a cache +// miss. The exec hook calls this once per command. The first call opens the +// session through `createSession` and records its id; every later call returns +// the stored id, so one lease runs every command in one persistent shell. The +// provider never falls back to a one-shot command to open a session. +// +// Leak bound: the Daytona SDK exposes NO per-session TTL. `createSession` takes +// only a session id, and there is no session update or expiry field. Two +// backstops bound the session against a leak. First, `teardownSession` runs a +// guaranteed `deleteSession` in every teardown hook's `try/finally`. Second, the +// sandbox-level `autoStopInterval` (15 minutes idle by default) stops the +// sandbox and, with it, every session; the `autoArchiveInterval` and +// `autoDeleteInterval` intervals then reap the sandbox. A session is a shell +// inside its sandbox and cannot outlive it. +async function getOrCreateSession(sandbox: Sandbox, scope: SandboxScope): Promise { + const existing = sandboxHandleSessionStore.get(scope); + if (existing) return existing; + // Single-flight the first-command create. Two overlapping first commands for + // one lease share one create promise, so the lease opens at most one live + // session. The guard checks and starts the create in one synchronous step, so + // no second command can slip in between the store read and the create start. + return sandboxHandleSessionStore.runSingle(scope, async () => { + const sessionId = `paperclip-${randomUUID()}`; + // Wrap the session create in a short `session.setup` provider span. The span + // carries no session id and no command text, only the provider family. The + // host maps the name to `sandbox.provider.session.setup`. + await withProviderSpan({ + name: "session.setup", + run: () => sandbox.process.createSession(sessionId), + }); + sandboxHandleSessionStore.set(scope, sessionId); + return sessionId; + }); } -// One-shot command execution via Daytona's `process.executeCommand`. The -// session-based API (`createSession` + `executeSessionCommand` with -// `runAsync: false`) hangs indefinitely when the supplied command ends with -// `exec `, which `buildLoginShellScript` always produces. Reproduced -// directly against the Daytona SDK: identical login-shell wrapper returns in -// ~600 ms via `executeCommand` but times out via `executeSessionCommand`. So we -// use the one-shot path, mirroring e2b's `sandbox.commands.run` model. +// Delete the persistent session for a lease and clear its stored id. Each +// teardown hook calls this inside its `try/finally`, so a failed delete never +// skips the rest of teardown. A failed delete logs the session id and the error +// loudly and does not throw past teardown; the sandbox stop or delete that +// follows removes the session shell anyway, and the sandbox-level +// `autoStopInterval` / `autoDeleteInterval` / `autoArchiveInterval` backstops +// bound any residual state (the session API exposes no per-session TTL). The +// store id is always cleared, so no orphan id survives. +async function teardownSession(sandbox: Sandbox, scope: SandboxScope): Promise { + const sessionId = sandboxHandleSessionStore.get(scope); + if (!sessionId) return; + try { + // Wrap the session delete in a short `session.teardown` provider span. The + // host maps the name to `sandbox.provider.session.teardown`. + await withProviderSpan({ + name: "session.teardown", + run: () => sandbox.process.deleteSession(sessionId), + }); + } catch (error) { + console.error( + `Failed to delete Daytona session ${sessionId} during teardown: ${formatErrorMessage(error)}`, + ); + } finally { + sandboxHandleSessionStore.clear(scope); + } +} + +// One-shot command execution via Daytona's `process.executeCommand`. This is the +// fallback path the exec hook uses when the session model is off. The command +// runs plain as the unprivileged sandbox user; the provider no longer wraps a +// user command with the advisory `bwrap` wrapper on any path. // // `executeCommand` returns combined stdout+stderr in `result`. We surface that // as `stdout` and leave `stderr` empty; callers that grep for error messages @@ -1452,7 +1387,6 @@ async function executeOneShot( sandbox: Sandbox, params: PluginEnvironmentExecuteParams, config: DaytonaDriverConfig, - bwrap: BwrapExecPlan | null, ): Promise { const gitNet = isGitNetworkCommand(params.command, params.args ?? []); const timeoutMs = resolveTimeoutMs(params.timeoutMs, config); @@ -1472,7 +1406,9 @@ async function executeOneShot( await sandbox.fs.uploadFile(Buffer.from(params.stdin ?? "", "utf8"), stdinPath, timeoutSeconds); } - const loginScript = buildLoginShellScript({ + // Run the plain login-shell script as the unprivileged sandbox user. The + // provider no longer wraps a user command with the advisory `bwrap` wrapper. + const command = buildLoginShellScript({ command: params.command, args: params.args ?? [], cwd: params.cwd, @@ -1480,16 +1416,6 @@ async function executeOneShot( stdinPath: stdinPath ?? undefined, }); - // Advisory bwrap wrapper (best-effort, automatic, no security boundary). When - // the lease reports bwrap available and a username is known, wrap the - // login-shell string so a write to a non-persistent path fails and the agent - // gets real-time feedback. The writable set binds the workspace and the - // read-write sync destinations; the stdin re-bind survives the `--tmpfs /tmp`. - // When the plan is null, run the plain string, which keeps today's behavior. - const command = bwrap - ? buildBwrapCommand(loginScript, bwrap.writableDirs, stdinPath, bwrap.username) - : loginScript; - // Pass cwd undefined: `buildLoginShellScript` already injects the `cd` after // it sources the login profiles, when params.cwd is set. The Daytona // executor's own cwd argument runs before that profile sourcing, which is @@ -1534,6 +1460,135 @@ async function executeOneShot( } } +// Poll interval for a session command's exit code. The live spike measured a +// session command resolving in about 260-300 ms, so a short interval keeps the +// poll responsive without a busy loop. +const SESSION_POLL_INTERVAL_MS = 50; + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +// Dispatch one user command into the persistent session and return its true +// stdout and stderr. +// +// The Daytona session is one persistent shell. A top-level `exit N` inside a +// session command ends that shell, so the next command then fails with "session +// process has exited". To stop a user `exit` from reaching the session shell, +// the dispatch wraps the whole login-shell script in a subshell `( ... )`. A +// user `exit` then ends only the subshell and reports its exit code, and the +// session shell stays alive. The provider passes the caller cwd and env inside +// the login-shell script on every command, so it never relies on implicit state +// that leaks between commands. +// +// The SDK exposes no built-in wait for a session command, so the dispatch runs +// the command with `runAsync: true` and polls `getSessionCommand` until the exit +// code is set. It then reads true `stdout` and `stderr` from +// `getSessionCommandLogs`, because the synchronous response fields are optional. +// The `runAsync: true` path also avoids the known `runAsync: false` login-shell +// hang. +async function executeInSession( + sandbox: Sandbox, + sessionId: string, + params: PluginEnvironmentExecuteParams, + config: DaytonaDriverConfig, +): Promise { + const gitNet = isGitNetworkCommand(params.command, params.args ?? []); + const timeoutMs = resolveTimeoutMs(params.timeoutMs, config); + const effectiveTimeoutMs = gitNet ? Math.min(timeoutMs, GIT_NETWORK_TIMEOUT_MS) : timeoutMs; + const timeoutSeconds = toTimeoutSeconds(effectiveTimeoutMs); + const stdinPath = params.stdin != null ? `/tmp/paperclip-stdin-${randomUUID()}` : null; + + // Marks the start of the session dispatch and poll. The timeout paths report + // the exec wall-time spent before the abort, so a slow command is still + // attributed to the provider boundary. + let execStart: number | null = null; + + try { + if (stdinPath) { + await sandbox.fs.uploadFile(Buffer.from(params.stdin ?? "", "utf8"), stdinPath, timeoutSeconds); + } + + const loginScript = buildLoginShellScript({ + command: params.command, + args: params.args ?? [], + cwd: params.cwd, + env: params.env, + stdinPath: stdinPath ?? undefined, + }); + // Subshell wrap: a top-level `exit` in the user command exits only the + // subshell, not the persistent session shell. + const command = `( ${loginScript} )`; + + execStart = timingNow(); + const dispatched = await sandbox.process.executeSessionCommand( + sessionId, + { command, runAsync: true }, + timeoutSeconds, + ); + const commandId = dispatched.cmdId; + + // Poll for the exit code; the SDK has no wait method. The poll deadline uses + // the wall clock, separate from the injected timing clock that measures the + // reported `durationMs`. + const deadlineMs = Date.now() + effectiveTimeoutMs; + let exitCode: number | null = null; + while (true) { + const status = await sandbox.process.getSessionCommand(sessionId, commandId); + if (typeof status.exitCode === "number") { + exitCode = status.exitCode; + break; + } + if (Date.now() >= deadlineMs) { + const durationMs = timingNow() - execStart; + const timeoutMessage = gitNet + ? `Git network operation timed out after ${Math.round(effectiveTimeoutMs / 1000)} s — the remote may be unreachable or noninteractive credentials are not configured.` + : `Command timed out after ${Math.round(effectiveTimeoutMs / 1000)} s.`; + return { + exitCode: null, + timedOut: true, + stdout: "", + stderr: `${timeoutMessage}\n`, + metadata: { durationMs }, + }; + } + await sleep(SESSION_POLL_INTERVAL_MS); + } + + // Read true, separated stdout and stderr from the logs endpoint. The + // synchronous dispatch response fields are optional, so the logs endpoint is + // the source of truth. + const logs = await sandbox.process.getSessionCommandLogs(sessionId, commandId); + const durationMs = timingNow() - execStart; + return { + exitCode, + timedOut: false, + stdout: logs.stdout ?? "", + stderr: logs.stderr ?? "", + metadata: { durationMs }, + }; + } catch (error) { + if (error instanceof DaytonaTimeoutError) { + const timeoutMessage = gitNet + ? `Git network operation timed out after ${Math.round(effectiveTimeoutMs / 1000)} s — the remote may be unreachable or noninteractive credentials are not configured.` + : error.message.trim(); + const durationMs = execStart != null ? timingNow() - execStart : undefined; + return { + exitCode: null, + timedOut: true, + stdout: "", + stderr: `${timeoutMessage}\n`, + ...(durationMs != null ? { metadata: { durationMs } } : {}), + }; + } + throw error; + } finally { + if (stdinPath) { + await sandbox.fs.deleteFile(stdinPath).catch(() => undefined); + } + } +} + const plugin = definePlugin({ async setup(ctx) { // Hoist the context to a module variable so the lifecycle hooks and the @@ -1613,15 +1668,12 @@ const plugin = definePlugin({ try { const remoteCwd = await resolveSandboxWorkingDirectory(sandbox); const shellCommand = await detectSandboxShellCommand(sandbox, toTimeoutSeconds(config.timeoutMs)); - const bwrapCapability = await detectBwrapCapability(sandbox, toBwrapProbeTimeoutSeconds(config), remoteCwd); return { ok: true, summary: `Connected to Daytona sandbox ${sandbox.name}.`, metadata: { provider: "daytona", shellCommand, - bwrapAvailable: bwrapCapability.bwrapAvailable, - sandboxUsername: bwrapCapability.sandboxUsername, sandboxId: sandbox.id, sandboxName: sandbox.name, target: sandbox.target, @@ -1659,7 +1711,6 @@ const plugin = definePlugin({ try { const remoteCwd = await resolveSandboxWorkingDirectory(sandbox); const shellCommand = await detectSandboxShellCommand(sandbox, toTimeoutSeconds(config.timeoutMs)); - const bwrapCapability = await detectBwrapCapability(sandbox, toBwrapProbeTimeoutSeconds(config), remoteCwd); const workspaceSentinel = await writeWorkspaceSentinel({ sandbox, remoteCwd, @@ -1693,8 +1744,6 @@ const plugin = definePlugin({ config, sandbox, shellCommand, - bwrapAvailable: bwrapCapability.bwrapAvailable, - sandboxUsername: bwrapCapability.sandboxUsername, remoteCwd, resumedLease: false, workspaceSentinel, @@ -1723,6 +1772,17 @@ const plugin = definePlugin({ return { providerLeaseId: null, metadata: { expired: true } }; } + // A stopped sandbox loses its session shell, so the stored session id is + // stale after a real restart. Clear the id only when the sandbox is not + // already running, and clear it before the restart. A stopped sandbox has + // no live session, so the clear drops a dead id and a later command opens + // a fresh session. A running sandbox keeps its live session, so the resume + // leaves the id in place; a concurrent command still finds it and teardown + // deletes one session. An unconditional clear would drop the id of a live + // session and leak its shell until sandbox reaping. + if (sandbox.state !== "started") { + sandboxHandleSessionStore.clear(scope); + } await ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs)); try { const remoteCwd = await resolveSandboxWorkingDirectory(sandbox); @@ -1741,7 +1801,6 @@ const plugin = definePlugin({ return { providerLeaseId: null, metadata: { expired: true, workspaceSentinel } }; } const shellCommand = await detectSandboxShellCommand(sandbox, toTimeoutSeconds(config.timeoutMs)); - const bwrapCapability = await detectBwrapCapability(sandbox, toBwrapProbeTimeoutSeconds(config), remoteCwd); sandboxHandleCache.markFresh(scope); sandboxHandleLeaseAdmissionStates.open(scope); return { @@ -1750,8 +1809,6 @@ const plugin = definePlugin({ config, sandbox, shellCommand, - bwrapAvailable: bwrapCapability.bwrapAvailable, - sandboxUsername: bwrapCapability.sandboxUsername, remoteCwd, resumedLease: true, workspaceSentinel, @@ -1788,6 +1845,7 @@ const plugin = definePlugin({ evictSandboxHandle(scope); await sandboxHandleActivityGates.waitForIdle(scope); + await teardownSession(sandbox, scope); if (config.reuseLease) { if (sandbox.state !== "stopped") { @@ -1851,6 +1909,7 @@ const plugin = definePlugin({ evictSandboxHandle(scope); await sandboxHandleActivityGates.waitForIdle(scope); + await teardownSession(sandbox, scope); await sandbox.delete(toTimeoutSeconds(config.timeoutMs)); } finally { sandboxHandleTeardownGates.end(scope, teardownGate); @@ -2088,6 +2147,7 @@ const plugin = definePlugin({ } evictSandboxHandle(scope); await sandboxHandleActivityGates.waitForIdle(scope); + await teardownSession(sandbox, scope); await sandbox.delete(toTimeoutSeconds(config.timeoutMs)); return { status: params.reason === "timed_out" ? "timed_out" : "cancelled", @@ -2179,24 +2239,34 @@ const plugin = definePlugin({ }); const getDurationMs = timingNow() - getStart; await ensureSandboxStarted(sandbox, toTimeoutSeconds(resolveTimeoutMs(params.timeoutMs, config))); - // Read the advisory bwrap flags from the lease metadata and read the - // collected writable directories from the same scope the sync-in hook uses. - const bwrapPlan = resolveBwrapExecPlan(params.lease.metadata, { + const scope: SandboxScope = { driverKey: params.driverKey, companyId: params.companyId, environmentId: params.environmentId, providerLeaseId, config, - }); - const result = await executeOneShot(sandbox, params, config, bwrapPlan); + }; + // Dispatch the command. When the session model is on, open the persistent + // session on a cache miss and run the command in it. The provider never + // falls back to a one-shot command to open a session; a cache miss creates + // one. When the session model is off, run the command on the one-shot path. + // + // A `bypassSession` command runs one-shot even when the session model is + // on, and it does NOT open the session. The host sets this flag on a + // pre-run command (the workspace provision command) that runs before the + // run opens its trace root. Opening the session there would emit a + // `session.setup` span with no run parent, and the span backend would drop + // it. With the bypass the session opens on the first in-run command, whose + // setup span parents to the run trace. + let result: PluginEnvironmentExecuteResult; + if (config.useSessions && !params.bypassSession) { + const sessionId = await getOrCreateSession(sandbox, scope); + result = await executeInSession(sandbox, sessionId, params, config); + } else { + result = await executeOneShot(sandbox, params, config); + } if (!result.timedOut) { - sandboxHandleCache.markFresh({ - driverKey: params.driverKey, - companyId: params.companyId, - environmentId: params.environmentId, - providerLeaseId, - config, - }); + sandboxHandleCache.markFresh(scope); } return { ...result, diff --git a/packages/plugins/sdk/src/protocol.ts b/packages/plugins/sdk/src/protocol.ts index 7e299764e1..e14ba5fa26 100644 --- a/packages/plugins/sdk/src/protocol.ts +++ b/packages/plugins/sdk/src/protocol.ts @@ -671,6 +671,20 @@ export interface PluginEnvironmentExecuteParams extends PluginEnvironmentDriverB env?: Record; stdin?: string; timeoutMs?: number; + /** + * Run this command outside the lease's persistent session. + * + * The host sets this flag on a command that runs before the run's agent work, + * for example the workspace provision command. A provider that opens a + * persistent session on the first command must NOT open the session for such a + * command; it runs the command one-shot and leaves the session closed. The + * session then opens on the first in-run command instead. A provider that does + * not use a persistent session ignores this flag. + * + * The default (absent or `false`) keeps the session path, so a normal in-run + * command opens and reuses the session as before. + */ + bypassSession?: boolean; } export interface PluginEnvironmentExecuteResult { diff --git a/server/src/__tests__/environment-runtime.test.ts b/server/src/__tests__/environment-runtime.test.ts index 3c836883ab..5c13b76f52 100644 --- a/server/src/__tests__/environment-runtime.test.ts +++ b/server/src/__tests__/environment-runtime.test.ts @@ -32,6 +32,13 @@ import { environmentRuntimeService, findReusableSandboxLeaseId } from "../servic import { environmentService } from "../services/environments.ts"; import { secretService } from "../services/secrets.ts"; import type { PluginWorkerManager } from "../services/plugin-worker-manager.ts"; +import { + getActiveStepContext, + runWithRuntimeParent, + type StartupSpanContext, +} from "@paperclipai/adapter-utils/acpx-engine/startup-timing"; +import { traceparentFromContextToken } from "../instrumentation.ts"; +import { ROOT_CONTEXT, trace } from "@opentelemetry/api"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; @@ -699,6 +706,321 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { expect(workerManager.call).toHaveBeenCalledWith(pluginId, "environmentReleaseLease", expect.anything(), 31234); }); + // Build the fake plugin fixture for the run-parent release tests below. A + // plugin sandbox provider can open a persistent session on the first command + // and delete it on lease release; the delete emits a provider + // `session.teardown` span. The host mints that span's parent from the active + // step context at the release RPC. So the release must run under the run + // parent, or the span loses its traceparent and the backend drops it. + async function seedFakePluginSandbox() { + const pluginId = randomUUID(); + const { companyId, environment: baseEnvironment, runId } = await seedEnvironment(); + const fakePluginConfig = { + provider: "fake-plugin", + image: "fake:test", + timeoutMs: 1234, + reuseLease: false, + }; + const environment = { + ...baseEnvironment, + name: "Fake Plugin Sandbox", + driver: "sandbox", + config: fakePluginConfig, + }; + await environmentService(db).update(environment.id, { + driver: "sandbox", + name: environment.name, + config: fakePluginConfig, + }); + await db.insert(plugins).values({ + id: pluginId, + pluginKey: "paperclip.fake-plugin-sandbox-provider", + packageName: "@paperclipai/plugin-fake-sandbox", + version: "1.0.0", + apiVersion: 1, + categories: ["automation"], + manifestJson: { + id: "paperclip.fake-plugin-sandbox-provider", + apiVersion: 1, + version: "1.0.0", + displayName: "Fake Plugin Sandbox Provider", + description: "Test fake plugin provider", + author: "Paperclip", + categories: ["automation"], + capabilities: ["environment.drivers.register"], + entrypoints: { worker: "dist/worker.js" }, + environmentDrivers: [ + { + driverKey: "fake-plugin", + kind: "sandbox_provider", + displayName: "Fake Plugin", + configSchema: { type: "object" }, + }, + ], + }, + status: "ready", + installOrder: 1, + updatedAt: new Date(), + } as any); + return { companyId, environment, runId, pluginId }; + } + + it("runs a plugin-backed lease release under the run-time exec parent, so the teardown span keeps a valid traceparent", async () => { + const { companyId, environment, runId, pluginId } = await seedFakePluginSandbox(); + + // The active step context observed at the moment the host issues the release + // RPC. The real worker manager mints the provider-span traceparent from + // exactly this value, so a non-null context with a valid parent proves the + // teardown span keeps a host-minted parent. + let releaseStepContext: ReturnType = null; + const workerManager = { + isRunning: vi.fn((id: string) => id === pluginId), + call: vi.fn(async (_pluginId: string, method: string) => { + if (method === "environmentAcquireLease") { + return { + providerLeaseId: "sandbox-1", + metadata: { + provider: "fake-plugin", + image: "fake:test", + timeoutMs: 1234, + reuseLease: false, + remoteCwd: "/workspace", + }, + }; + } + if (method === "environmentExecute") { + return { exitCode: 0, signal: null, timedOut: false, stdout: "ok\n", stderr: "" }; + } + if (method === "environmentReleaseLease") { + releaseStepContext = getActiveStepContext(); + return undefined; + } + throw new Error(`Unexpected plugin method: ${method}`); + }), + } as unknown as PluginWorkerManager; + const runtimeWithPlugin = environmentRuntimeService(db, { pluginWorkerManager: workerManager }); + + const acquired = await runtimeWithPlugin.acquireRunLease({ + companyId, + environment, + issueId: null, + heartbeatRunId: runId, + persistedExecutionWorkspace: null, + }); + + // Build a real host run-parent context and run the first command under it, + // exactly as the run drives an exec. The driver records this context for the + // lease and replays it around the later release RPC. + const runParent: StartupSpanContext = trace.setSpanContext(ROOT_CONTEXT, { + traceId: "0af7651916cd43dd8448eb211c80319c", + spanId: "b7ad6b7169203331", + traceFlags: 1, + isRemote: false, + }); + await runWithRuntimeParent(runParent, () => + runtimeWithPlugin.execute({ + environment, + lease: acquired.lease, + command: "printf", + args: ["ok"], + cwd: "/workspace", + env: {}, + timeoutMs: 1000, + }), + ); + + await environmentService(db).update(environment.id, { driver: "local", config: {} }); + const released = await runtimeWithPlugin.releaseRunLeases(runId); + + expect(released).toHaveLength(1); + // The release RPC ran under the recorded run parent, so the host mints a + // valid W3C traceparent from it. A dropped span would show a null context. + expect(releaseStepContext).not.toBeNull(); + expect(releaseStepContext?.parentContext).toBe(runParent); + expect(traceparentFromContextToken(releaseStepContext?.parentContext)).toBe( + "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01", + ); + }); + + it("releases a plugin-backed lease with no active step context when the run drove no exec", async () => { + const { companyId, environment, runId, pluginId } = await seedFakePluginSandbox(); + + let releaseStepContext: ReturnType = null; + let releaseCalled = false; + const workerManager = { + isRunning: vi.fn((id: string) => id === pluginId), + call: vi.fn(async (_pluginId: string, method: string) => { + if (method === "environmentAcquireLease") { + return { + providerLeaseId: "sandbox-1", + metadata: { + provider: "fake-plugin", + image: "fake:test", + timeoutMs: 1234, + reuseLease: false, + remoteCwd: "/workspace", + }, + }; + } + if (method === "environmentReleaseLease") { + releaseCalled = true; + releaseStepContext = getActiveStepContext(); + return undefined; + } + throw new Error(`Unexpected plugin method: ${method}`); + }), + } as unknown as PluginWorkerManager; + const runtimeWithPlugin = environmentRuntimeService(db, { pluginWorkerManager: workerManager }); + + await runtimeWithPlugin.acquireRunLease({ + companyId, + environment, + issueId: null, + heartbeatRunId: runId, + persistedExecutionWorkspace: null, + }); + await environmentService(db).update(environment.id, { driver: "local", config: {} }); + const released = await runtimeWithPlugin.releaseRunLeases(runId); + + expect(released).toHaveLength(1); + // No command ran, so no session opened and no teardown span is emitted. The + // release must not invent a parent from an unrelated ambient context; it + // runs unwrapped, exactly as before the fix. + expect(releaseCalled).toBe(true); + expect(releaseStepContext).toBeNull(); + }); + + it("forwards the bypassSession flag to the plugin execute RPC so a pre-run command skips the session", async () => { + const { companyId, environment, runId, pluginId } = await seedFakePluginSandbox(); + + // Capture the params of each environmentExecute RPC, so the test can assert + // the host forwards `bypassSession` to the provider unchanged. + const executeParams: Array> = []; + const workerManager = { + isRunning: vi.fn((id: string) => id === pluginId), + call: vi.fn(async (_pluginId: string, method: string, params: Record) => { + if (method === "environmentAcquireLease") { + return { + providerLeaseId: "sandbox-1", + metadata: { + provider: "fake-plugin", + image: "fake:test", + timeoutMs: 1234, + reuseLease: false, + remoteCwd: "/workspace", + }, + }; + } + if (method === "environmentExecute") { + executeParams.push(params); + return { exitCode: 0, signal: null, timedOut: false, stdout: "ok\n", stderr: "" }; + } + throw new Error(`Unexpected plugin method: ${method}`); + }), + } as unknown as PluginWorkerManager; + const runtimeWithPlugin = environmentRuntimeService(db, { pluginWorkerManager: workerManager }); + + const acquired = await runtimeWithPlugin.acquireRunLease({ + companyId, + environment, + issueId: null, + heartbeatRunId: runId, + persistedExecutionWorkspace: null, + }); + + // A pre-run command (the provision command) sets bypassSession explicitly. + await runtimeWithPlugin.execute({ + environment, + lease: acquired.lease, + command: "bash", + args: ["-lc", "true"], + cwd: "/workspace", + env: {}, + timeoutMs: 1000, + bypassSession: true, + }); + // An in-run command runs under the run-parent context, so the host does not + // bypass the session and the provider opens/uses the session. + const runParent: StartupSpanContext = trace.setSpanContext(ROOT_CONTEXT, { + traceId: "0af7651916cd43dd8448eb211c80319c", + spanId: "b7ad6b7169203331", + traceFlags: 1, + isRemote: false, + }); + await runWithRuntimeParent(runParent, () => + runtimeWithPlugin.execute({ + environment, + lease: acquired.lease, + command: "printf", + args: ["ok"], + cwd: "/workspace", + env: {}, + timeoutMs: 1000, + }), + ); + + expect(executeParams).toHaveLength(2); + expect(executeParams[0]?.bypassSession).toBe(true); + // An in-run command carries a run parent, so it does not bypass the session. + expect(executeParams[1]?.bypassSession).toBe(false); + }); + + it("bypasses the session for a context-less command so the setup span keeps a run parent", async () => { + const { companyId, environment, runId, pluginId } = await seedFakePluginSandbox(); + + // Capture the params of each environmentExecute RPC, so the test can assert + // the host derives `bypassSession` from the active run-parent context. + const executeParams: Array> = []; + const workerManager = { + isRunning: vi.fn((id: string) => id === pluginId), + call: vi.fn(async (_pluginId: string, method: string, params: Record) => { + if (method === "environmentAcquireLease") { + return { + providerLeaseId: "sandbox-1", + metadata: { + provider: "fake-plugin", + image: "fake:test", + timeoutMs: 1234, + reuseLease: false, + remoteCwd: "/workspace", + }, + }; + } + if (method === "environmentExecute") { + executeParams.push(params); + return { exitCode: 0, signal: null, timedOut: false, stdout: "ok\n", stderr: "" }; + } + throw new Error(`Unexpected plugin method: ${method}`); + }), + } as unknown as PluginWorkerManager; + const runtimeWithPlugin = environmentRuntimeService(db, { pluginWorkerManager: workerManager }); + + const acquired = await runtimeWithPlugin.acquireRunLease({ + companyId, + environment, + issueId: null, + heartbeatRunId: runId, + persistedExecutionWorkspace: null, + }); + + // A command with no active run-parent context (a pre-run install/probe or + // the agent process launch) runs before the run trace is active. The host + // mints no plugin RPC traceparent for it, so a session opened here would drop + // its setup span. The host bypasses the session for such a command. + await runtimeWithPlugin.execute({ + environment, + lease: acquired.lease, + command: "sh", + args: ["-c", "command -v claude"], + cwd: "/workspace", + env: {}, + timeoutMs: 1000, + }); + + expect(executeParams).toHaveLength(1); + expect(executeParams[0]?.bypassSession).toBe(true); + }); + it("builds the workspace-realization record with referenced sources for a plugin-backed sandbox realize", async () => { // A provider plugin realize handler returns only its realized cwd and provider metadata; it does // not build the workspace-realization record. The server must build that record from the run diff --git a/server/src/__tests__/plugin-host-services-span.test.ts b/server/src/__tests__/plugin-host-services-span.test.ts index abc4ea058f..110d06a0b6 100644 --- a/server/src/__tests__/plugin-host-services-span.test.ts +++ b/server/src/__tests__/plugin-host-services-span.test.ts @@ -143,6 +143,21 @@ describe("plugin provider span host handler", () => { ]); }); + it("admits the session setup and teardown span names", async () => { + const services = servicesFor(); + for (const name of ["session.setup", "session.teardown"]) { + await services.tracer.record( + { name }, + { traceparent: VALID_TRACEPARENT } as WorkerHostCallContext, + ); + } + const recorded = mockRecordSpan.mock.calls.map((c) => (c[0] as { name: string }).name); + expect(recorded).toEqual([ + "sandbox.provider.session.setup", + "sandbox.provider.session.teardown", + ]); + }); + it("forwards a valid start-time and end-time pair to the recorder", async () => { const services = servicesFor(); const startTimeMs = Date.now() - 4500; diff --git a/server/src/services/environment-run-orchestrator.ts b/server/src/services/environment-run-orchestrator.ts index 7ebad63739..8d60e77d8e 100644 --- a/server/src/services/environment-run-orchestrator.ts +++ b/server/src/services/environment-run-orchestrator.ts @@ -432,6 +432,13 @@ export function environmentRunOrchestrator( SHELL: "/bin/bash", }, timeoutMs: 300_000, + // The provision command runs before the run opens its trace root, so it + // carries no run parent. A sandbox provider that opens a persistent + // session on the first command must not open the session here, or the + // session-setup span loses its parent and the span backend drops it. + // Bypass the session for this command; the session opens on the first + // in-run command instead, whose setup span parents to the run trace. + bypassSession: true, }); if (provisionResult.exitCode !== 0 || provisionResult.timedOut) { throw new Error(formatProvisionFailureDetail(provisionResult)); diff --git a/server/src/services/environment-runtime.ts b/server/src/services/environment-runtime.ts index 7457718a50..0e381340a9 100644 --- a/server/src/services/environment-runtime.ts +++ b/server/src/services/environment-runtime.ts @@ -20,6 +20,11 @@ import type { PluginSyncOperation, } from "@paperclipai/plugin-sdk"; import { ensureSshWorkspaceReady } from "@paperclipai/adapter-utils/ssh"; +import { + getActiveStepContext, + runWithRuntimeParent, + type StartupSpanContext, +} from "@paperclipai/adapter-utils/acpx-engine/startup-timing"; import { environmentService } from "./environments.js"; import { collectEnvironmentSecretRefs, @@ -188,6 +193,15 @@ export interface EnvironmentDriverExecuteInput extends EnvironmentDriverLeaseInp env?: Record; stdin?: string; timeoutMs?: number; + /** + * Run this command outside the lease's persistent session. The run + * orchestrator sets this on the workspace provision command, which runs before + * the run opens its trace root. A sandbox provider that opens a persistent + * session on the first command must run this command one-shot and keep the + * session closed, so the session first opens on an in-run command whose setup + * span parents to the run trace. The default keeps the session path. + */ + bypassSession?: boolean; } export interface EnvironmentDriverSyncInput extends EnvironmentDriverLeaseInput { @@ -639,6 +653,31 @@ function createSandboxEnvironmentDriver( const pluginWorkerReadyPollMs = options.pluginWorkerReadyPollMs ?? DEFAULT_PLUGIN_SANDBOX_WORKER_READY_POLL_MS; const environmentsSvc = environmentService(db); + // The run-time exec parent context, held per lease id. A plugin sandbox + // provider can open a persistent session on the first command and delete it + // on lease release. The session open runs inside `execute`, under the run + // parent (the same context the `sandbox.exec` span reads). The session delete + // runs inside the lease-release RPC, which the run orchestrator calls after + // the run, outside that scope. Without a parent the host mints no + // `traceparent` and drops the provider `session.teardown` span. So `execute` + // records the exec parent here, and the release paths replay it around the + // release RPC. The host still mints and validates the `traceparent` itself; + // the value only widens which host calls carry a run parent. An entry is + // removed on release, so the map holds at most one context per live lease. + const runExecParentByLeaseId = new Map(); + + // Run a lease-release RPC under the lease's recorded exec parent context, and + // then drop the entry — the lease is gone. Under the parent the host mints a + // `traceparent`, so a provider `session.teardown` span reaches the span + // backend in the run trace. With no recorded context (no command ran, or a + // local target with no trace context) the call runs unwrapped, exactly as + // before, so the change never fails a release. + function runLeaseReleaseWithRunParent(leaseId: string, call: () => Promise): Promise { + const runParent = runExecParentByLeaseId.get(leaseId); + runExecParentByLeaseId.delete(leaseId); + return runParent !== undefined ? runWithRuntimeParent(runParent, call) : call(); + } + async function resolveSandboxProviderPlugin(input: { provider: string }) { const running = await resolvePluginSandboxProviderDriverByKey({ db, @@ -1274,6 +1313,31 @@ function createSandboxEnvironmentDriver( async execute(input) { // Plugin-backed sandbox providers: delegate command execution. if (input.lease.metadata?.sandboxProviderPlugin && pluginWorkerManager) { + // Read the active run-parent context once. The host mints the plugin + // RPC `traceparent` from this same context, so a provider `session.setup` + // span parents to the run trace only when this context is present. + const activeStep = getActiveStepContext(); + // Record the run-time exec parent context for this lease, so a later + // lease-release RPC that emits the provider `session.teardown` span can + // parent to the same run trace. This is the same context the + // `sandbox.exec` span reads. Keep only a defined context; a local or SSH + // target with no host trace context yields undefined and stores nothing. + const execParentContext = activeStep?.parentContext; + if (execParentContext !== undefined) { + runExecParentByLeaseId.set(input.lease.id, execParentContext); + } + // Bypass the persistent session for any command that runs with no active + // run-parent context. Such a command runs before the run trace is active: + // the workspace provision command, the CLI install command, the + // resolvability probe, and the agent process launch all run at the top of + // the adapter execute, outside a measured step and outside the run + // trace. A session opened on such a command emits a `session.setup` span + // with no host-minted parent, and the span backend drops it. So run the + // command one-shot and keep the session closed; the session then opens on + // the first in-run command that carries a run parent (an agent tool + // command runs under the run trace), whose setup span parents to the run + // trace. A command that sets `bypassSession` explicitly always bypasses. + const bypassSession = input.bypassSession === true || activeStep === null; const pluginId = readString(input.lease.metadata?.pluginId); const providerKey = readString(input.lease.metadata?.provider); if (pluginId && providerKey) { @@ -1300,6 +1364,13 @@ function createSandboxEnvironmentDriver( env: input.env, stdin: input.stdin, timeoutMs: input.timeoutMs, + // Forward the effective session-bypass flag so a provider that opens + // a persistent session skips it for a pre-run or context-less command + // (the workspace provision command, the CLI install command, the + // resolvability probe, the agent process launch). The session then + // opens on the first in-run command that carries a run parent, whose + // setup span parents to the run trace. + bypassSession, }, resolvePluginExecuteRpcTimeoutMs({ requestedTimeoutMs: input.timeoutMs, config: sanitizedConfig, @@ -1368,15 +1439,17 @@ function createSandboxEnvironmentDriver( lease: input.lease, provider: providerKey, }); - await pluginWorkerManager.call(pluginId, "environmentReleaseLease", { - driverKey: providerKey, - companyId: input.lease.companyId, - environmentId: input.environment.id, - issueId: input.lease.issueId, - config: stripSandboxProviderEnvelope(config as SandboxEnvironmentConfig), - providerLeaseId: input.lease.providerLeaseId, - leaseMetadata: metadata, - }, resolvePluginSandboxRpcTimeoutMs(stripSandboxProviderEnvelope(config as SandboxEnvironmentConfig))); + await runLeaseReleaseWithRunParent(input.lease.id, () => + pluginWorkerManager.call(pluginId, "environmentReleaseLease", { + driverKey: providerKey, + companyId: input.lease.companyId, + environmentId: input.environment.id, + issueId: input.lease.issueId, + config: stripSandboxProviderEnvelope(config as SandboxEnvironmentConfig), + providerLeaseId: input.lease.providerLeaseId, + leaseMetadata: metadata, + }, resolvePluginSandboxRpcTimeoutMs(stripSandboxProviderEnvelope(config as SandboxEnvironmentConfig))), + ); } catch { cleanupStatus = "failed"; } @@ -1414,15 +1487,17 @@ function createSandboxEnvironmentDriver( lease: input.lease, provider: providerKey, }); - await pluginWorkerManager.call(pluginId, "environmentDestroyLease", { - driverKey: providerKey, - companyId: input.lease.companyId, - environmentId: input.environment.id, - issueId: input.lease.issueId, - config: stripSandboxProviderEnvelope(config as SandboxEnvironmentConfig), - providerLeaseId: input.lease.providerLeaseId, - leaseMetadata: metadata, - }, resolvePluginSandboxRpcTimeoutMs(stripSandboxProviderEnvelope(config as SandboxEnvironmentConfig))); + await runLeaseReleaseWithRunParent(input.lease.id, () => + pluginWorkerManager.call(pluginId, "environmentDestroyLease", { + driverKey: providerKey, + companyId: input.lease.companyId, + environmentId: input.environment.id, + issueId: input.lease.issueId, + config: stripSandboxProviderEnvelope(config as SandboxEnvironmentConfig), + providerLeaseId: input.lease.providerLeaseId, + leaseMetadata: metadata, + }, resolvePluginSandboxRpcTimeoutMs(stripSandboxProviderEnvelope(config as SandboxEnvironmentConfig))), + ); } } else { const metadataConfig = sandboxConfigFromLeaseMetadata(input.lease); diff --git a/server/src/services/plugin-host-services.ts b/server/src/services/plugin-host-services.ts index bf490c69a7..1eb3b8a01a 100644 --- a/server/src/services/plugin-host-services.ts +++ b/server/src/services/plugin-host-services.ts @@ -510,7 +510,8 @@ const SPAN_ATTRS = SANDBOX_STARTUP_SPAN_ATTRS; /** The closed set of provider span names a plugin may emit. `pack` and * `transfer` are the host-local build and the byte upload. `mkdir`, `guard`, * `rename`, `extract`, and `provision` are the per-round-trip command spans in - * the inbound sync path. */ + * the inbound sync path. `session.setup` and `session.teardown` are the short + * spans that wrap a persistent-session create and delete. */ const KNOWN_PROVIDER_SPAN_NAMES: ReadonlySet = new Set([ "pack", "transfer", @@ -519,6 +520,8 @@ const KNOWN_PROVIDER_SPAN_NAMES: ReadonlySet = new Set([ "rename", "extract", "provision", + "session.setup", + "session.teardown", ]); /** Clamp the span name to a closed, namespaced set. A known name maps to