fix(security): escape swarm nodeId and registry tag in cluster commands

- 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
This commit is contained in:
Mauricio Siu
2026-07-19 23:34:18 -06:00
parent 95a3556baa
commit df2779eaeb
4 changed files with 80 additions and 7 deletions

View File

@@ -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]);
});
});

View File

@@ -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);

View File

@@ -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;
}

View File

@@ -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",