Merge pull request #5733 from Dokploy/fix/exec-error-secret-leak

apps/dokploy/__test__/process/exec-error-secrets.test.ts

@@ -0,0 +1,17 @@
+import { inspect } from "node:util";
+import { execAsync, ExecError } from "@dokploy/server/utils/process/execAsync";
+import { describe, expect, it } from "vitest";
+
+describe("execAsync failure", () => {
+	it("does not leak secrets embedded in the command", async () => {
+		const secret = "s3cr3t-value";
+		const error = await execAsync(`printf %s '${secret}' >/dev/null; exit 3`)
+			.then(() => null)
+			.catch((e) => e);
+
+		expect(error).toBeInstanceOf(ExecError);
+		expect(error.exitCode).toBe(3);
+		expect(error.message).not.toContain(secret);
+		expect(inspect(error)).not.toContain(secret);
+	});
+});

apps/dokploy/__test__/wss/container-ownership.test.ts

@@ -0,0 +1,70 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const mockRows = vi.hoisted(() => new Map<unknown, { appName: string }>());
+const mockExec = vi.hoisted(() => vi.fn());
+vi.mock("@dokploy/server/db", () => ({
+	db: {
+		select: () => ({
+			from: (table: unknown) => ({
+				where: () => ({
+					limit: async () => {
+						const row = mockRows.get(table);
+						return row ? [row] : [];
+					},
+				}),
+			}),
+		}),
+	},
+}));
+vi.mock("@dokploy/server/utils/process/execAsync", () => ({
+	execAsync: mockExec,
+	execAsyncRemote: mockExec,
+}));
+
+import { applications, libsql } from "@dokploy/server/db/schema";
+import { containerBelongsToService } from "@/server/wss/container-ownership";
+
+const labels = (l: Record<string, string>) => ({ stdout: JSON.stringify(l) });
+
+beforeEach(() => {
+	vi.clearAllMocks();
+	mockRows.clear();
+	mockRows.set(applications, { appName: "app-mine-abc123" });
+});
+
+describe("containerBelongsToService", () => {
+	it("allows a container of the service", async () => {
+		mockExec.mockResolvedValue(
+			labels({ "com.docker.swarm.service.name": "app-mine-abc123" }),
+		);
+		expect(await containerBelongsToService("svc", "c1")).toBe(true);
+	});
+
+	it("denies a container of another service", async () => {
+		mockExec.mockResolvedValue(
+			labels({ "com.docker.swarm.service.name": "dokploy" }),
+		);
+		expect(await containerBelongsToService("svc", "c1")).toBe(false);
+	});
+
+	it("denies containers without labels or when docker fails", async () => {
+		mockExec.mockResolvedValue({ stdout: "null" });
+		expect(await containerBelongsToService("svc", "c1")).toBe(false);
+		mockExec.mockRejectedValue(new Error("No such object"));
+		expect(await containerBelongsToService("svc", "c1")).toBe(false);
+	});
+
+	it("resolves libsql services", async () => {
+		mockRows.clear();
+		mockRows.set(libsql, { appName: "libsql-abc" });
+		mockExec.mockResolvedValue(
+			labels({ "com.docker.swarm.service.name": "libsql-abc" }),
+		);
+		expect(await containerBelongsToService("svc", "c1")).toBe(true);
+	});
+
+	it("denies an unknown service", async () => {
+		mockRows.clear();
+		expect(await containerBelongsToService("nope", "c1")).toBe(false);
+	});
+});

apps/dokploy/server/wss/authorize.ts

@@ -25,7 +25,6 @@ export const canAccessDockerOverWss = async (
 	serverId?: string | null,
 	serviceId?: string | null,
 ): Promise<boolean> => {
-	// return false;
 	if (!user || !session?.activeOrganizationId) return false;
 
 	const ctx = buildCtx(user, session.activeOrganizationId);

apps/dokploy/server/wss/container-ownership.ts

@@ -0,0 +1,71 @@
+import { db } from "@dokploy/server/db";
+import {
+	applications,
+	compose,
+	libsql,
+	mariadb,
+	mongo,
+	mysql,
+	postgres,
+	redis,
+} from "@dokploy/server/db/schema";
+import {
+	execAsync,
+	execAsyncRemote,
+} from "@dokploy/server/utils/process/execAsync";
+import { eq } from "drizzle-orm";
+import { quote } from "shell-quote";
+
+const OWNERSHIP_LABELS = [
+	"com.docker.swarm.service.name",
+	"com.docker.compose.project",
+	"com.docker.stack.namespace",
+];
+
+const SERVICE_TABLES = [
+	{ table: applications, id: applications.applicationId },
+	{ table: compose, id: compose.composeId },
+	{ table: postgres, id: postgres.postgresId },
+	{ table: mysql, id: mysql.mysqlId },
+	{ table: mariadb, id: mariadb.mariadbId },
+	{ table: mongo, id: mongo.mongoId },
+	{ table: redis, id: redis.redisId },
+	{ table: libsql, id: libsql.libsqlId },
+];
+
+const findServiceAppName = async (serviceId: string) => {
+	for (const { table, id } of SERVICE_TABLES) {
+		const [row] = await db
+			.select({ appName: table.appName })
+			.from(table)
+			.where(eq(id, serviceId))
+			.limit(1);
+		if (row) return row.appName;
+	}
+};
+
+// Service access alone must not allow `docker exec` into other tenants' containers.
+export const containerBelongsToService = async (
+	serviceId: string,
+	containerId: string,
+	serverId?: string | null,
+): Promise<boolean> => {
+	try {
+		const appName = await findServiceAppName(serviceId);
+		if (!appName) return false;
+		const command = quote([
+			"docker",
+			"inspect",
+			"--format",
+			"{{json .Config.Labels}}",
+			containerId,
+		]);
+		const { stdout } = serverId
+			? await execAsyncRemote(serverId, command)
+			: await execAsync(command);
+		const labels = JSON.parse(stdout.trim()) ?? {};
+		return OWNERSHIP_LABELS.some((label) => labels[label] === appName);
+	} catch {
+		return false;
+	}
+};

apps/dokploy/server/wss/docker-container-terminal.ts

@@ -4,6 +4,7 @@ import { spawn } from "node-pty";
 import { Client } from "ssh2";
 import { WebSocketServer } from "ws";
 import { canAccessDockerOverWss } from "./authorize";
+import { containerBelongsToService } from "./container-ownership";
 import {
 	isValidContainerId,
 	isValidShell,
@@ -74,6 +75,13 @@ export const setupDockerContainerTerminalWebSocketServer = (
 			ws.close(4003, "Not authorized");
 			return;
 		}
+		if (
+			serviceId &&
+			!(await containerBelongsToService(serviceId, containerId, serverId))
+		) {
+			ws.close(4003, "Not authorized");
+			return;
+		}
 		try {
 			if (serverId) {
 				const server = await findServerById(serverId);

packages/server/src/utils/process/ExecError.ts

@@ -25,6 +25,11 @@ export class ExecError extends Error {
 		this.originalError = details.originalError;
 		this.serverId = details.serverId;
 
+		// Keep the command (may embed secrets) out of console/inspect output.
+		for (const key of ["command", "originalError"] as const) {
+			Object.defineProperty(this, key, { enumerable: false });
+		}
+
 		// Maintains proper stack trace for where our error was thrown (only available on V8)
 		if (Error.captureStackTrace) {
 			Error.captureStackTrace(this, ExecError);

packages/server/src/utils/process/execAsync.ts

@@ -23,6 +23,10 @@ export { ExecError } from "./ExecError";
 
 const execAsyncBase = util.promisify(exec);
 
+// Node embeds the full command in error.message; it may carry secrets.
+const withoutCommand = (message: string, command: string) =>
+	message.split(command).join("[command omitted]");
+
 export const execAsync = async (
 	command: string,
 	options?: { cwd?: string; env?: NodeJS.ProcessEnv; shell?: string },
@@ -42,13 +46,16 @@ export const execAsync = async (
 			// @ts-expect-error
 			const stderr = error.stderr?.toString() || "";
 
-			throw new ExecError(`Command execution failed: ${error.message}`, {
-				command,
-				stdout,
-				stderr,
-				exitCode,
-				originalError: error,
-			});
+			throw new ExecError(
+				`Command execution failed: ${withoutCommand(error.message, command)}`,
+				{
+					command,
+					stdout,
+					stderr,
+					exitCode,
+					originalError: error,
+				},
+			);
 		}
 		throw error;
 	}
@@ -71,13 +78,16 @@ export const execAsyncStream = (
 		const childProcess = exec(command, options, (error) => {
 			if (error) {
 				reject(
-					new ExecError(`Command execution failed: ${error.message}`, {
-						command,
-						stdout: stdoutComplete,
-						stderr: stderrComplete,
-						exitCode: error.code,
-						originalError: error,
-					}),
+					new ExecError(
+						`Command execution failed: ${withoutCommand(error.message, command)}`,
+						{
+							command,
+							stdout: stdoutComplete,
+							stderr: stderrComplete,
+							exitCode: error.code,
+							originalError: error,
+						},
+					),
 				);
 				return;
 			}