From df2779eaeb4a58f0c85d4caa713c776c790fa708 Mon Sep 17 00:00:00 2001 From: Mauricio Siu Date: Sun, 19 Jul 2026 23:34:18 -0600 Subject: [PATCH] fix(security): escape swarm nodeId and registry tag in cluster commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - cluster.removeWorker: input.nodeId (z.string(), no regex) was interpolated raw into 'docker node update/rm ${nodeId}' — now shell-quoted. - swarm image upload (getRegistryCommands): registryTag / imageName were interpolated raw into 'docker tag'/'docker push' and an echo. registryTag is built from username and imagePrefix, which have no schema regex, so it was injectable — now shell-quoted. (The docker login already used safeDockerLoginCommand, so credentials were already safe.) - gpu-setup: nodeId (derived from 'docker info', not user input) escaped as defense-in-depth. Closes GHSA-4mfc-grxw-6858, GHSA-hfwh-69ch-gv47, GHSA-prwq-2mcm-mvhr --- .../cluster/registry-node-injection.test.ts | 70 +++++++++++++++++++ apps/dokploy/server/api/routers/cluster.ts | 5 +- packages/server/src/utils/cluster/upload.ts | 7 +- packages/server/src/utils/gpu-setup.ts | 5 +- 4 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 apps/dokploy/__test__/cluster/registry-node-injection.test.ts diff --git a/apps/dokploy/__test__/cluster/registry-node-injection.test.ts b/apps/dokploy/__test__/cluster/registry-node-injection.test.ts new file mode 100644 index 000000000..f4d54d237 --- /dev/null +++ b/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]); + }); +}); diff --git a/apps/dokploy/server/api/routers/cluster.ts b/apps/dokploy/server/api/routers/cluster.ts index 441410182..ff0bc4de6 100644 --- a/apps/dokploy/server/api/routers/cluster.ts +++ b/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); diff --git a/packages/server/src/utils/cluster/upload.ts b/packages/server/src/utils/cluster/upload.ts index 0968afc7b..d6d8e4df9 100644 --- a/packages/server/src/utils/cluster/upload.ts +++ b/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; } diff --git a/packages/server/src/utils/gpu-setup.ts b/packages/server/src/utils/gpu-setup.ts index f658b7727..59aa98047 100644 --- a/packages/server/src/utils/gpu-setup.ts +++ b/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",