From cba0b253c7de4157dd932de7f16a2ad247c7cee9 Mon Sep 17 00:00:00 2001 From: Mauricio Siu Date: Sun, 19 Jul 2026 21:53:55 -0600 Subject: [PATCH] fix(security): escape user input in docker build/pull commands - dockerImage (buildRemoteDocker) -> docker pull / echo - dockerContextPath (docker-file builder) -> cd - publishDirectory (nixpacks builder) -> docker cp source/dest paths These fields were interpolated unescaped into shell commands run via execAsync / execAsyncRemote during deployment. All are now passed through shell-quote. Registry credentials already went through safeDockerLoginCommand (unchanged). Closes GHSA-g9cg-4mmj-mh7p, GHSA-jxxj-gmpx-h5rj, GHSA-qjrc-g63x-qhp9, GHSA-98j8-6vjr-c3xw --- .../deploy/docker-build-injection.test.ts | 66 +++++++++++++++++++ .../server/src/utils/builders/docker-file.ts | 6 +- .../server/src/utils/builders/nixpacks.ts | 7 +- packages/server/src/utils/providers/docker.ts | 5 +- 4 files changed, 76 insertions(+), 8 deletions(-) create mode 100644 apps/dokploy/__test__/deploy/docker-build-injection.test.ts diff --git a/apps/dokploy/__test__/deploy/docker-build-injection.test.ts b/apps/dokploy/__test__/deploy/docker-build-injection.test.ts new file mode 100644 index 000000000..2e4cc9789 --- /dev/null +++ b/apps/dokploy/__test__/deploy/docker-build-injection.test.ts @@ -0,0 +1,66 @@ +import { execSync } from "node:child_process"; +import { existsSync, rmSync } from "node:fs"; +import { parse, quote } from "shell-quote"; +import { describe, expect, it } from "vitest"; + +// Reproduces the escaping applied at the docker build/pull sinks and asserts no +// payload can break out of the command. `docker`/`cd` are replaced by `:` so the +// test exercises only the injection surface, not real docker. +const MARK = `/tmp/dokploy_docker_pwned_${process.pid}`; + +const runAndCheckSafe = (command: string) => { + if (existsSync(MARK)) rmSync(MARK); + try { + execSync(command, { shell: "/bin/sh", stdio: "ignore" }); + } catch { + // no-op stand-ins may exit non-zero; only the marker matters. + } + const fired = existsSync(MARK); + if (existsSync(MARK)) rmSync(MARK); + return !fired; +}; + +const PAYLOADS = [ + "$(touch %MARK%)", + "`touch %MARK%`", + "x; touch %MARK%", + "x && touch %MARK%", + "x | touch %MARK%", +]; + +describe("docker build/pull command injection", () => { + it("dockerImage (buildRemoteDocker: docker pull / echo) is escaped", () => { + for (const p of PAYLOADS) { + const dockerImage = p.replace("%MARK%", MARK); + const command = `: pull ${quote([dockerImage])}; : echo ${quote([`Pulling ${dockerImage}`])}`; + expect(runAndCheckSafe(command)).toBe(true); + } + }); + + it("dockerContextPath (docker-file: cd) is escaped", () => { + for (const p of PAYLOADS) { + const dockerContextPath = p.replace("%MARK%", MARK); + const command = `: cd ${quote([dockerContextPath])}`; + expect(runAndCheckSafe(command)).toBe(true); + } + }); + + it("publishDirectory (nixpacks: docker cp source path) is escaped", () => { + for (const p of PAYLOADS) { + const publishDirectory = p.replace("%MARK%", MARK); + const containerId = "buildabc"; + const command = `: cp ${quote([`${containerId}:/app/${publishDirectory}/.`])} /dest`; + expect(runAndCheckSafe(command)).toBe(true); + } + }); + + it("keeps a legitimate image / path intact as a single token", () => { + // Escaping may add backslashes (e.g. before ':'), but the shell must parse + // the result back to exactly the original single token. + expect(parse(quote(["nginx:1.27-alpine"]))).toEqual(["nginx:1.27-alpine"]); + expect(parse(quote(["registry.io/team/app:tag"]))).toEqual([ + "registry.io/team/app:tag", + ]); + expect(parse(quote(["dist/static"]))).toEqual(["dist/static"]); + }); +}); diff --git a/packages/server/src/utils/builders/docker-file.ts b/packages/server/src/utils/builders/docker-file.ts index b20e3f170..5f315cd58 100644 --- a/packages/server/src/utils/builders/docker-file.ts +++ b/packages/server/src/utils/builders/docker-file.ts @@ -85,9 +85,9 @@ export const getDockerCommand = (application: ApplicationNested) => { } command += ` -echo "Building ${appName}" ; -cd ${dockerContextPath} || { - echo "❌ The path ${dockerContextPath} does not exist" ; +echo ${quote([`Building ${appName}`])} ; +cd ${quote([dockerContextPath])} || { + echo ${quote([`❌ The path ${dockerContextPath} does not exist`])} ; exit 1; } diff --git a/packages/server/src/utils/builders/nixpacks.ts b/packages/server/src/utils/builders/nixpacks.ts index b7134ea65..e02be0fc3 100644 --- a/packages/server/src/utils/builders/nixpacks.ts +++ b/packages/server/src/utils/builders/nixpacks.ts @@ -1,6 +1,7 @@ import path from "node:path"; import { getStaticCommand } from "@dokploy/server/utils/builders/static"; import { nanoid } from "nanoid"; +import { quote } from "shell-quote"; import { prepareEnvironmentVariablesForShell } from "../docker/utils"; import { getBuildAppDirectory } from "../filesystem/directory"; import type { ApplicationNested } from "."; @@ -52,10 +53,10 @@ export const getNixpacksCommand = (application: ApplicationNested) => { bashCommand += ` docker create --name ${buildContainerId} ${appName} - mkdir -p ${localPath} - docker cp ${buildContainerId}:/app/${publishDirectory}${isDirectory ? "/." : ""} ${path.join(buildAppDirectory, publishDirectory)} || { + mkdir -p ${quote([localPath])} + docker cp ${quote([`${buildContainerId}:/app/${publishDirectory}${isDirectory ? "/." : ""}`])} ${quote([localPath])} || { docker rm ${buildContainerId} - echo "❌ Copying ${publishDirectory} to ${path.join(buildAppDirectory, publishDirectory)} failed" ; + echo ${quote([`❌ Copying ${publishDirectory} to ${localPath} failed`])} ; exit 1; } docker rm ${buildContainerId} diff --git a/packages/server/src/utils/providers/docker.ts b/packages/server/src/utils/providers/docker.ts index f3a4c39f3..25a1ffa82 100644 --- a/packages/server/src/utils/providers/docker.ts +++ b/packages/server/src/utils/providers/docker.ts @@ -1,4 +1,5 @@ import { safeDockerLoginCommand } from "@dokploy/server/services/registry"; +import { quote } from "shell-quote"; import type { ApplicationNested } from "../builders"; export const buildRemoteDocker = async (application: ApplicationNested) => { @@ -9,7 +10,7 @@ export const buildRemoteDocker = async (application: ApplicationNested) => { throw new Error("Docker image not found"); } let command = ` -echo "Pulling ${dockerImage}"; +echo ${quote([`Pulling ${dockerImage}`])}; `; if (username && password) { @@ -22,7 +23,7 @@ fi } command += ` -docker pull ${dockerImage} 2>&1 || { +docker pull ${quote([dockerImage])} 2>&1 || { echo "❌ Pulling image failed"; exit 1; }