Merge pull request #4864 from Dokploy/fix/cmdi-registry-swarm-node

fix(security): OS command injection via swarm nodeId and registry tag

apps/dokploy/__test__/cluster/registry-node-injection.test.ts

@@ -0,0 +1,70 @@
+import { execSync } from "node:child_process";
+import { existsSync, rmSync } from "node:fs";
+import { getRegistryTag } from "@dokploy/server/utils/cluster/upload";
+import { parse, quote } from "shell-quote";
+import { describe, expect, it } from "vitest";
+
+const MARK = `/tmp/dokploy_regnode_pwned_${process.pid}`;
+
+const runsSafely = (command: string) => {
+	if (existsSync(MARK)) rmSync(MARK);
+	try {
+		execSync(command, { shell: "/bin/sh", stdio: "ignore" });
+	} catch {}
+	const fired = existsSync(MARK);
+	if (existsSync(MARK)) rmSync(MARK);
+	return !fired;
+};
+
+const PAYLOADS = (m: string) => [
+	`$(touch ${m})`,
+	"`touch " + m + "`",
+	`x; touch ${m}`,
+	`x | touch ${m}`,
+];
+
+describe("cluster removeWorker nodeId injection", () => {
+	// docker node update/rm ${quote([nodeId])} — replace `docker node` with `:`.
+	it("escapes nodeId in drain/remove commands", () => {
+		for (const nodeId of PAYLOADS(MARK)) {
+			const drain = `: node update --availability drain ${quote([nodeId])}`;
+			const remove = `: node rm ${quote([nodeId])} --force`;
+			expect(runsSafely(drain)).toBe(true);
+			expect(runsSafely(remove)).toBe(true);
+		}
+	});
+});
+
+describe("swarm upload registry tag/push injection", () => {
+	// registryTag is built from registryUrl/username/imagePrefix (username and
+	// imagePrefix have no schema regex). Assert docker tag/push stay safe.
+	it("escapes a malicious imagePrefix flowing into the registry tag", () => {
+		for (const payload of PAYLOADS(MARK)) {
+			const registryTag = getRegistryTag(
+				{
+					registryUrl: "registry.example.com",
+					imagePrefix: payload,
+					username: "user",
+				} as any,
+				"app:latest",
+			);
+			const tagCmd = `: tag ${quote(["app:latest"])} ${quote([registryTag])}`;
+			const pushCmd = `: push ${quote([registryTag])}`;
+			expect(runsSafely(tagCmd)).toBe(true);
+			expect(runsSafely(pushCmd)).toBe(true);
+		}
+	});
+
+	it("keeps a legitimate registry tag intact", () => {
+		const tag = getRegistryTag(
+			{
+				registryUrl: "registry.example.com",
+				imagePrefix: "team",
+				username: "user",
+			} as any,
+			"myapp:1.2.3",
+		);
+		expect(tag).toBe("registry.example.com/team/myapp:1.2.3");
+		expect(parse(quote([tag]))).toEqual([tag]);
+	});
+});

apps/dokploy/server/api/routers/cluster.ts

@@ -6,6 +6,7 @@ import {
 	getRemoteDocker,
 } from "@dokploy/server";
 import { TRPCError } from "@trpc/server";
+import { quote } from "shell-quote";
 import { z } from "zod";
 import { audit } from "@/server/api/utils/audit";
 import { getLocalServerIp } from "@/server/wss/terminal";
@@ -51,8 +52,8 @@ export const clusterRouter = createTRPCRouter({
 				}
 			}
 			try {
-				const drainCommand = `docker node update --availability drain ${input.nodeId}`;
-				const removeCommand = `docker node rm ${input.nodeId} --force`;
+				const drainCommand = `docker node update --availability drain ${quote([input.nodeId])}`;
+				const removeCommand = `docker node rm ${quote([input.nodeId])} --force`;
 
 				if (input.serverId) {
 					await execAsyncRemote(input.serverId, drainCommand);

packages/server/src/utils/cluster/upload.ts

@@ -5,6 +5,7 @@ import {
 	safeDockerLoginCommand,
 } from "@dokploy/server/services/registry";
 import { createRollback } from "@dokploy/server/services/rollbacks";
+import { quote } from "shell-quote";
 import type { ApplicationNested } from "../builders";
 
 export const uploadImageRemoteCommand = async (
@@ -124,18 +125,18 @@ const getRegistryCommands = (
 		registry.password,
 	);
 	return `
-echo "📦 [Enabled Registry] Uploading image to '${registry.registryType}' | '${registryTag}'" ;
+echo ${quote([`📦 [Enabled Registry] Uploading image to '${registry.registryType}' | '${registryTag}'`])} ;
 ${loginCmd} || {
 	echo "❌ DockerHub Failed" ;
 	exit 1;
 }
 echo "✅ Registry Login Success" ;
-docker tag ${imageName} ${registryTag} || {
+docker tag ${quote([imageName])} ${quote([registryTag])} || {
 	echo "❌ Error tagging image" ;
 	exit 1;
 }
 echo "✅ Image Tagged" ;
-docker push ${registryTag} || {
+docker push ${quote([registryTag])} || {
 	echo "❌ Error pushing image" ;
 	exit 1;
 }

packages/server/src/utils/gpu-setup.ts

@@ -1,4 +1,5 @@
 import * as fs from "node:fs/promises";
+import { quote } from "shell-quote";
 import { execAsync, execAsyncRemote, sleep } from "../utils/process/execAsync";
 
 interface GPUInfo {
@@ -322,7 +323,7 @@ const setupLocalServer = async (daemonConfig: any) => {
 };
 
 const addGpuLabel = async (nodeId: string, serverId?: string) => {
-	const labelCommand = `docker node update --label-add gpu=true ${nodeId}`;
+	const labelCommand = `docker node update --label-add gpu=true ${quote([nodeId])}`;
 	if (serverId) {
 		await execAsyncRemote(serverId, labelCommand);
 	} else {
@@ -335,7 +336,7 @@ const verifySetup = async (nodeId: string, serverId?: string) => {
 
 	if (!finalStatus.swarmEnabled) {
 		const diagnosticCommands = [
-			`docker node inspect ${nodeId}`,
+			`docker node inspect ${quote([nodeId])}`,
 			'nvidia-smi -a | grep "GPU UUID"',
 			"cat /etc/docker/daemon.json",
 			"cat /etc/nvidia-container-runtime/config.toml",