diff --git a/apps/dokploy/__test__/deploy/docker-build-injection.test.ts b/apps/dokploy/__test__/deploy/docker-build-injection.test.ts index 2e4cc9789..7723641e7 100644 --- a/apps/dokploy/__test__/deploy/docker-build-injection.test.ts +++ b/apps/dokploy/__test__/deploy/docker-build-injection.test.ts @@ -1,7 +1,9 @@ import { execSync } from "node:child_process"; -import { existsSync, rmSync } from "node:fs"; +import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import path from "node:path"; +import { getDockerCommand } from "@dokploy/server/utils/builders/docker-file"; import { parse, quote } from "shell-quote"; -import { describe, expect, it } from "vitest"; +import { afterEach, 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 @@ -64,3 +66,139 @@ describe("docker build/pull command injection", () => { expect(parse(quote(["dist/static"]))).toEqual(["dist/static"]); }); }); + +describe("getDockerCommand full-string injection (real builder)", () => { + // Generate the real `getDockerCommand(`) output, neutralise the + // `docker build` invocation to a shell no-op (`:`) so real docker never + // runs, then execute the whole script through /bin/sh. Every other shell + // construct — the `cd` guard, `echo`, `|| { … }` error handling, + // metacharacter parsing — runs verbatim. The test passes only if no + // injected payload can create a marker file on disk, i.e. no + // user-controlled field breaks out of the `docker build` argument list. + const APP_NAME = `injapp_${process.pid}`; + const codeDir = path.join( + process.cwd(), + ".docker", + "applications", + APP_NAME, + "code", + ); + // Slash-free marker name so payloads can stay slash-free, mirroring the + // real attack constraint that a literal `/` would trip the `cd` guard. + const MARK_NAME = `pwned_df_${process.pid}`; + + const baseApp = (overrides: Record = {}) => + ({ + appName: APP_NAME, + env: null, + publishDirectory: null, + buildArgs: null, + buildSecrets: null, + dockerBuildStage: null, + cleanCache: false, + createEnvFile: false, + sourceType: "github", + buildType: "dockerfile", + buildPath: "", + dockerfile: "Dockerfile", + dockerContextPath: null, + serverId: null, + buildServerId: null, + gitlabBuildPath: null, + bitbucketBuildPath: null, + giteaBuildPath: null, + dropBuildPath: null, + customGitBuildPath: null, + environment: { project: { env: null }, env: null }, + ...overrides, + }) as any; + + const ensureCodeDir = () => { + mkdirSync(codeDir, { recursive: true }); + writeFileSync(path.join(codeDir, "Dockerfile"), "FROM alpine\n"); + }; + + // Replace only the `docker build -t` token with `:` (a shell no-op) so the + // args are parsed by the shell but docker is never invoked. `:` ignores + // its args and returns 0, so the `|| { …; exit 1; }` handler does not fire + // — any marker that appears must have come from an injected + // `;`/`|`/`$()`/backtick breaking out of the `docker build` args. + const neutraliseDocker = (cmd: string) => + cmd.replace("docker build -t", ": build -t"); + + const runAndCheckSafe = (app: any) => { + ensureCodeDir(); + const mark = path.join(codeDir, MARK_NAME); + if (existsSync(mark)) rmSync(mark); + try { + execSync(neutraliseDocker(getDockerCommand(app)), { + shell: "/bin/sh", + stdio: "ignore", + }); + } catch { + // The no-op stand-in and the `cd` guard may abort the script; only the + // marker matters. + } + const fired = existsSync(mark); + if (existsSync(mark)) rmSync(mark); + return !fired; + }; + + afterEach(() => { + rmSync(codeDir, { recursive: true, force: true }); + }); + + it("dockerBuildStage (--target) single-field payload does not fire", () => { + const app = baseApp({ dockerBuildStage: `x; touch ${MARK_NAME}; #` }); + expect(runAndCheckSafe(app)).toBe(true); + }); + + it("dockerfile (-f) slash-free payload does not fire", () => { + const app = baseApp({ dockerfile: `Dockerfile; touch ${MARK_NAME}; #` }); + expect(runAndCheckSafe(app)).toBe(true); + }); + + it("dockerContextPath + dockerfile two-field payload does not fire", () => { + const app = baseApp({ + dockerContextPath: ".", + dockerfile: `Dockerfile; touch ${MARK_NAME}; #`, + }); + expect(runAndCheckSafe(app)).toBe(true); + }); + + it("customGitBuildPath (*BuildPath) two-field payload does not fire", () => { + const app = baseApp({ + sourceType: "git", + customGitBuildPath: `seg; touch ${MARK_NAME}; #`, + dockerContextPath: ".", + }); + expect(runAndCheckSafe(app)).toBe(true); + }); + + it("preserves legitimate build values as single tokens (no over-escaping)", () => { + ensureCodeDir(); + const app = baseApp({ + sourceType: "git", + customGitBuildPath: "repo/sub", + dockerfile: "Dockerfile.prod", + dockerBuildStage: "builder-node", + dockerContextPath: "apps/web", + }); + const cmd = getDockerCommand(app); + expect(cmd).toContain("docker build -t"); + // Legitimate values contain no shell metacharacters, so shell-quote + // leaves them untouched and the build still receives the original args. + expect(cmd).toContain("--target builder-node"); + expect(cmd).toContain("Dockerfile.prod"); + expect(cmd).toContain("repo/sub"); + expect(cmd).toContain("apps/web"); + // The build stage must round-trip through the shell as a single literal + // token, never as shell operators. + const buildLine = + cmd + .split("\n") + .find((l) => l.includes("docker build")) + ?.trim() ?? ""; + expect(parse(buildLine)).toContain("builder-node"); + }); +}); diff --git a/packages/server/src/utils/builders/docker-file.ts b/packages/server/src/utils/builders/docker-file.ts index 13bc05fc6..24ad0d268 100644 --- a/packages/server/src/utils/builders/docker-file.ts +++ b/packages/server/src/utils/builders/docker-file.ts @@ -35,14 +35,14 @@ export const getDockerCommand = (application: ApplicationNested) => { const commandArgs = [ "build", "-t", - image, + quote([image]), "-f", - dockerFilePath, - dockerContextPath, + quote([dockerFilePath]), + quote([dockerContextPath]), ]; if (dockerBuildStage) { - commandArgs.push("--target", dockerBuildStage); + commandArgs.push("--target", quote([dockerBuildStage])); } if (cleanCache) {