fix(sandbox): block inter-sandbox traffic, restart container when the timeout kill cannot fork, skip sandboxes mid-creation in reconcile

apps/dokploy/__test__/sandbox/exec.test.ts

@@ -51,6 +51,7 @@ const createDocker = (opts: {
 	const kill = vi.fn(async () => {
 		stream.end();
 	});
+	const start = vi.fn(async () => undefined);
 	const killExecStart = vi.fn(async () => {
 		if (opts.dieOnKill) stream.end();
 	});
@@ -58,7 +59,7 @@ const createDocker = (opts: {
 
 	const exec = vi.fn(async (options: { Cmd: string[] }) => {
 		execCreate(options);
-		if (options.Cmd[2]?.startsWith("for p in /proc/")) {
+		if (options.Cmd[2]?.startsWith("if grep -qzF x /dev/null")) {
 			return { start: killExecStart, inspect };
 		}
 		return {
@@ -77,10 +78,10 @@ const createDocker = (opts: {
 	});
 
 	const docker = {
-		getContainer: vi.fn(() => ({ exec, kill })),
+		getContainer: vi.fn(() => ({ exec, kill, start })),
 		modem: { demuxStream },
 	};
-	return { docker, execCreate, execStart, kill, killExecStart, inspect };
+	return { docker, execCreate, execStart, kill, start, killExecStart, inspect };
 };
 
 describe("runSandboxExec", () => {
@@ -168,10 +169,11 @@ describe("runSandboxExec", () => {
 		expect(killExecStart).toHaveBeenCalledWith({ Detach: true });
 		expect(kill).not.toHaveBeenCalled();
 		expect(result.containerKilled).toBe(false);
+		expect(result.containerRestarted).toBe(false);
 	});
 
-	it("kills the container when the process survives the marker kill", async () => {
-		const { docker, kill } = createDocker({
+	it("kills and restarts the container when the process survives the marker kill", async () => {
+		const { docker, kill, start } = createDocker({
 			frames: [],
 			exitCode: 0,
 			endAfterMs: 0,
@@ -184,14 +186,35 @@ describe("runSandboxExec", () => {
 		expect(result.timedOut).toBe(true);
 		expect(result.exitCode).toBe(SANDBOX_EXEC_TIMEOUT_EXIT_CODE);
 		expect(kill).toHaveBeenCalledWith({ signal: "SIGKILL" });
+		expect(start).toHaveBeenCalledTimes(1);
 		expect(result.containerKilled).toBe(true);
+		expect(result.containerRestarted).toBe(true);
+	}, 10_000);
+
+	it("reports a failed restart so the sandbox can be marked as error", async () => {
+		const { docker, start } = createDocker({
+			frames: [],
+			exitCode: 0,
+			endAfterMs: 0,
+			dieOnKill: false,
+		});
+		start.mockRejectedValueOnce(new Error("no such container"));
+		const result = await runSandboxExec(docker as never, "ctr", {
+			cmd: "sleep 100",
+			timeoutMs: 50,
+		});
+		expect(result.containerKilled).toBe(true);
+		expect(result.containerRestarted).toBe(false);
 	}, 10_000);
 });
 
 describe("buildSandboxKillCommand", () => {
 	it("matches the marker as a fixed string and never fails the shell", () => {
 		const cmd = buildSandboxKillCommand("abc-123");
-		expect(cmd).toContain(`grep -qxF ${quote(["DOKPLOY_EXEC_ID=abc-123"])}`);
+		expect(cmd).toContain(`grep -qzxF ${quote(["DOKPLOY_EXEC_ID=abc-123"])}`);
+		expect(cmd).toContain(
+			`tr '\\0' '\\n' < "$f" 2>/dev/null | grep -qxF ${quote(["DOKPLOY_EXEC_ID=abc-123"])}`,
+		);
 		expect(parse(cmd)).toContain("DOKPLOY_EXEC_ID=abc-123");
 		expect(cmd).toContain("kill -9");
 		expect(cmd.endsWith("; true")).toBe(true);

apps/dokploy/__test__/sandbox/network.test.ts

@@ -0,0 +1,107 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+vi.mock("@dokploy/server/utils/servers/remote-docker", () => ({
+	getRemoteDocker: vi.fn(),
+}));
+
+import {
+	ensureSandboxNetwork,
+	SANDBOX_NETWORK_OPTIONS,
+} from "@dokploy/server/services/sandbox";
+
+const notFound = () =>
+	Object.assign(new Error("no such network"), { statusCode: 404 });
+
+const createDocker = (inspect: () => Promise<unknown>) => {
+	const remove = vi.fn(async () => undefined);
+	const createNetwork = vi.fn(async () => undefined);
+	const getNetwork = vi.fn(() => ({ inspect, remove }));
+	return {
+		docker: { getNetwork, createNetwork },
+		remove,
+		createNetwork,
+		getNetwork,
+	};
+};
+
+beforeEach(() => {
+	vi.spyOn(console, "warn").mockImplementation(() => {});
+});
+
+describe("ensureSandboxNetwork", () => {
+	it("creates an internal bridge without inter-container traffic for isolated mode", async () => {
+		const { docker, createNetwork } = createDocker(async () => {
+			throw notFound();
+		});
+		const name = await ensureSandboxNetwork(docker as never, "isolated");
+		expect(name).toBe("dokploy-sandboxes-isolated");
+		expect(createNetwork).toHaveBeenCalledWith(
+			expect.objectContaining({
+				Name: "dokploy-sandboxes-isolated",
+				Driver: "bridge",
+				Internal: true,
+				Options: SANDBOX_NETWORK_OPTIONS,
+			}),
+		);
+		expect(
+			SANDBOX_NETWORK_OPTIONS["com.docker.network.bridge.enable_icc"],
+		).toBe("false");
+	});
+
+	it("creates a non-internal bridge for internet mode", async () => {
+		const { docker, createNetwork } = createDocker(async () => {
+			throw notFound();
+		});
+		await ensureSandboxNetwork(docker as never, "internet");
+		expect(createNetwork).toHaveBeenCalledWith(
+			expect.objectContaining({
+				Name: "dokploy-sandboxes",
+				Internal: false,
+				Options: SANDBOX_NETWORK_OPTIONS,
+			}),
+		);
+	});
+
+	it("reuses a network that already blocks inter-container traffic", async () => {
+		const { docker, createNetwork, remove } = createDocker(async () => ({
+			Options: SANDBOX_NETWORK_OPTIONS,
+			Containers: {},
+		}));
+		await ensureSandboxNetwork(docker as never, "isolated");
+		expect(createNetwork).not.toHaveBeenCalled();
+		expect(remove).not.toHaveBeenCalled();
+	});
+
+	it("recreates an unused legacy network that allows inter-container traffic", async () => {
+		const { docker, createNetwork, remove } = createDocker(async () => ({
+			Options: {},
+			Containers: {},
+		}));
+		await ensureSandboxNetwork(docker as never, "isolated");
+		expect(remove).toHaveBeenCalledTimes(1);
+		expect(createNetwork).toHaveBeenCalledTimes(1);
+	});
+
+	it("keeps a legacy network that still has containers attached and warns", async () => {
+		const { docker, createNetwork, remove } = createDocker(async () => ({
+			Options: {},
+			Containers: { abc: { Name: "dokploy-sandbox-x" } },
+		}));
+		await ensureSandboxNetwork(docker as never, "isolated");
+		expect(remove).not.toHaveBeenCalled();
+		expect(createNetwork).not.toHaveBeenCalled();
+		expect(console.warn).toHaveBeenCalled();
+	});
+
+	it("tolerates a concurrent create of the same network", async () => {
+		const { docker, createNetwork } = createDocker(async () => {
+			throw notFound();
+		});
+		createNetwork.mockRejectedValueOnce(
+			Object.assign(new Error("exists"), { statusCode: 409 }),
+		);
+		await expect(
+			ensureSandboxNetwork(docker as never, "isolated"),
+		).resolves.toBe("dokploy-sandboxes-isolated");
+	});
+});

apps/dokploy/__test__/sandbox/reaper.test.ts

@@ -76,9 +76,9 @@ describe("planSandboxReconcile", () => {
 	it("removes containers without a running row and errors rows without a container", () => {
 		const plan = planSandboxReconcile(
 			[
-				{ sandboxId: "ok", containerId: "c-ok" },
-				{ sandboxId: "gone", containerId: "c-gone" },
-				{ sandboxId: "never-started", containerId: null },
+				{ sandboxId: "ok", containerId: "c-ok", status: "running" },
+				{ sandboxId: "gone", containerId: "c-gone", status: "running" },
+				{ sandboxId: "never-started", containerId: null, status: "running" },
 			],
 			[
 				container("c-ok", "ok"),
@@ -92,16 +92,24 @@ describe("planSandboxReconcile", () => {
 
 	it("removes a stopped container even when its row is running, and errors the row", () => {
 		const plan = planSandboxReconcile(
-			[{ sandboxId: "sb", containerId: "c1" }],
+			[{ sandboxId: "sb", containerId: "c1", status: "running" }],
 			[container("c1", "sb", "exited")],
 		);
 		expect(plan.removeContainers).toEqual(["c1"]);
 		expect(plan.markError).toEqual(["sb"]);
 	});
 
+	it("leaves containers of sandboxes that are still being created alone", () => {
+		const plan = planSandboxReconcile(
+			[{ sandboxId: "new", containerId: null, status: "creating" }],
+			[container("c-new", "new")],
+		);
+		expect(plan).toEqual({ removeContainers: [], markError: [] });
+	});
+
 	it("does nothing when everything matches", () => {
 		const plan = planSandboxReconcile(
-			[{ sandboxId: "sb", containerId: "c1" }],
+			[{ sandboxId: "sb", containerId: "c1", status: "running" }],
 			[container("c1", "sb")],
 		);
 		expect(plan).toEqual({ removeContainers: [], markError: [] });

docs/sandboxes.md

@@ -183,15 +183,20 @@ Each sandbox container is created with:
   capability is needed inside the container.
 - A dedicated bridge network per server: `dokploy-sandboxes-isolated`
   (`internal: true`, no egress) for `networkMode: "isolated"` (default) or
-  `dokploy-sandboxes` for `networkMode: "internet"`. Sandboxes are never
-  attached to `dokploy-network` or to any application network, and nothing is
-  published through Traefik.
+  `dokploy-sandboxes` for `networkMode: "internet"`. Both are created with
+  `com.docker.network.bridge.enable_icc=false`, so sandboxes cannot reach
+  each other even on the same bridge (a network created by an earlier build
+  without that option is recreated automatically once no sandbox is attached
+  to it). Sandboxes are never attached to `dokploy-network` or to any
+  application network, and nothing is published through Traefik.
 - Labels `dokploy.sandbox=true`, `dokploy.sandboxId`, `dokploy.projectId` and
   `dokploy.environmentId` so the reaper/reconcile can find orphans.
 - Commands run through `docker exec` with a per-command timeout; on timeout the
   whole process tree started by that exec is killed (matched by a marker env
-  var, no `procps` needed), falling back to killing the container, in which
-  case the sandbox is marked `error`.
+  var, no `procps` needed). If that kill cannot run (for example the command
+  exhausted `PidsLimit`, so nothing can fork), the container is killed and
+  started again: every process is gone, files are kept and the sandbox stays
+  `running`. Only when the restart itself fails is the sandbox marked `error`.
 
 Environment variables passed at creation are stored encrypted at rest and are
 never returned by the API.

packages/server/src/services/sandbox.ts

@@ -123,23 +123,47 @@ export const ensureSandboxImage = async (docker: Dockerode, image: string) => {
 	});
 };
 
+// Sandboxes share a bridge per server, so inter-container traffic must be
+// disabled or two isolated sandboxes could talk to each other.
+export const SANDBOX_NETWORK_OPTIONS = {
+	"com.docker.network.bridge.enable_icc": "false",
+};
+
 export const ensureSandboxNetwork = async (
 	docker: Dockerode,
 	mode: SandboxNetworkMode,
 ) => {
 	const name = getSandboxNetworkName(mode);
+	let existing: {
+		Options?: Record<string, string>;
+		Containers?: Record<string, unknown>;
+	} | null = null;
 	try {
-		await docker.getNetwork(name).inspect();
-		return name;
+		existing = await docker.getNetwork(name).inspect();
 	} catch (error) {
 		if (!isNotFound(error)) throw error;
 	}
+	if (existing) {
+		if (
+			existing.Options?.["com.docker.network.bridge.enable_icc"] === "false"
+		) {
+			return name;
+		}
+		if (Object.keys(existing.Containers ?? {}).length > 0) {
+			console.warn(
+				`[Sandbox] Network ${name} allows inter-container traffic but is in use; it will be recreated once no sandbox is attached`,
+			);
+			return name;
+		}
+		await docker.getNetwork(name).remove();
+	}
 	try {
 		await docker.createNetwork({
 			Name: name,
 			Driver: "bridge",
 			Internal: mode === "isolated",
 			CheckDuplicate: true,
+			Options: { ...SANDBOX_NETWORK_OPTIONS },
 			Labels: { [SANDBOX_LABEL]: "true" },
 		});
 	} catch (error) {
@@ -321,7 +345,7 @@ export const execInSandbox = async (
 		onStdout: options.onStdout,
 		onStderr: options.onStderr,
 	});
-	if (result.containerKilled) {
+	if (result.containerKilled && !result.containerRestarted) {
 		await updateSandboxById(sandbox.sandboxId, { status: "error" });
 	}
 	return result;

packages/server/src/utils/sandbox/exec.ts

@@ -27,6 +27,7 @@ export interface SandboxExecResult {
 	timedOut: boolean;
 	truncated: boolean;
 	containerKilled: boolean;
+	containerRestarted: boolean;
 }
 
 export const toSandboxEnvArray = (env?: Record<string, string> | string[]) =>
@@ -35,9 +36,17 @@ export const toSandboxEnvArray = (env?: Record<string, string> | string[]) =>
 		: Object.entries(env ?? {}).map(([key, value]) => `${key}=${value}`);
 
 // Finds every process started by the exec (children included) through the
-// marker env var and kills it, without relying on pkill/procps being installed.
-export const buildSandboxKillCommand = (marker: string) =>
-	`for p in /proc/[0-9]*; do if tr '\\0' '\\n' < "$p/environ" 2>/dev/null | grep -qxF ${quote([`${SANDBOX_EXEC_MARKER_ENV}=${marker}`])}; then kill -9 "\${p#/proc/}" 2>/dev/null; fi; done; true`;
+// marker env var and kills it with the shell builtin `kill`, so each
+// iteration needs a single extra pid, which matters when the command
+// exhausted PidsLimit. GNU grep reads the NUL-separated environ with -z;
+// busybox grep has no -z, so it falls back to tr.
+export const buildSandboxKillCommand = (marker: string) => {
+	const pattern = quote([`${SANDBOX_EXEC_MARKER_ENV}=${marker}`]);
+	const kill = 'p="${f#/proc/}"; kill -9 "${p%/environ}" 2>/dev/null';
+	const withZ = `for f in /proc/[0-9]*/environ; do if grep -qzxF ${pattern} "$f" 2>/dev/null; then ${kill}; fi; done`;
+	const withTr = `for f in /proc/[0-9]*/environ; do if tr '\\0' '\\n' < "$f" 2>/dev/null | grep -qxF ${pattern}; then ${kill}; fi; done`;
+	return `if grep -qzF x /dev/null 2>/dev/null; [ $? -ne 2 ]; then ${withZ}; else ${withTr}; fi; true`;
+};
 
 const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
 
@@ -114,6 +123,7 @@ export const runSandboxExec = async (
 
 	let timedOut = false;
 	let containerKilled = false;
+	let containerRestarted = false;
 	let timeoutTask: Promise<void> | null = null;
 	const timer = setTimeout(() => {
 		timedOut = true;
@@ -127,6 +137,9 @@ export const runSandboxExec = async (
 				sleep(KILL_GRACE_MS).then(() => false),
 			]);
 			if (!ended) {
+				// The marker kill could not run (e.g. PidsLimit exhausted, no
+				// grep). Killing and starting the container again drops every
+				// process while keeping the filesystem, so the sandbox stays usable.
 				await container
 					.kill({ signal: "SIGKILL" })
 					.then(() => {
@@ -134,6 +147,14 @@ export const runSandboxExec = async (
 					})
 					.catch(() => {});
 				stream.destroy();
+				if (containerKilled) {
+					await container
+						.start()
+						.then(() => {
+							containerRestarted = true;
+						})
+						.catch(() => {});
+				}
 			}
 		})();
 	}, options.timeoutMs);
@@ -158,5 +179,6 @@ export const runSandboxExec = async (
 		timedOut,
 		truncated: state.truncated,
 		containerKilled,
+		containerRestarted,
 	};
 };

packages/server/src/utils/sandbox/reaper.ts

@@ -55,7 +55,7 @@ export interface SandboxReconcilePlan {
 }
 
 export const planSandboxReconcile = (
-	rows: { sandboxId: string; containerId: string | null }[],
+	rows: { sandboxId: string; containerId: string | null; status: string }[],
 	containers: { Id: string; State: string; Labels: Record<string, string> }[],
 ): SandboxReconcilePlan => {
 	const rowsById = new Map(rows.map((row) => [row.sandboxId, row]));
@@ -65,6 +65,8 @@ export const planSandboxReconcile = (
 	for (const container of containers) {
 		const sandboxId = container.Labels?.[SANDBOX_ID_LABEL];
 		const row = sandboxId ? rowsById.get(sandboxId) : undefined;
+		// A sandbox mid-creation has no containerId yet; leave it to createSandbox.
+		if (row?.status === "creating") continue;
 		if (
 			!row ||
 			row.containerId !== container.Id ||
@@ -77,7 +79,11 @@ export const planSandboxReconcile = (
 	}
 
 	const markError = rows
-		.filter((row) => !row.containerId || !liveContainerIds.has(row.containerId))
+		.filter(
+			(row) =>
+				row.status === "running" &&
+				(!row.containerId || !liveContainerIds.has(row.containerId)),
+		)
 		.map((row) => row.sandboxId);
 
 	return { removeContainers, markError };
@@ -95,12 +101,12 @@ const reconcileServer = async (serverId: string | null) => {
 		listSandboxContainers(docker),
 		db.query.sandboxes.findMany({
 			where: and(
-				eq(sandboxes.status, "running"),
+				inArray(sandboxes.status, ["running", "creating"]),
 				serverId
 					? eq(sandboxes.serverId, serverId)
 					: isNull(sandboxes.serverId),
 			),
-			columns: { sandboxId: true, containerId: true },
+			columns: { sandboxId: true, containerId: true, status: true },
 		}),
 	]);
 	const plan = planSandboxReconcile(rows, containers);