diff --git a/apps/dokploy/__test__/docker/docker-volume-injection.test.ts b/apps/dokploy/__test__/docker/docker-volume-injection.test.ts new file mode 100644 index 000000000..5f1076fef --- /dev/null +++ b/apps/dokploy/__test__/docker/docker-volume-injection.test.ts @@ -0,0 +1,129 @@ +import { + deleteVolumeFile, + listVolumeFiles, + readVolumeFile, + writeVolumeFile, +} from "@dokploy/server"; +import { parse } from "shell-quote"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { volumeNameRegex } from "@/server/api/routers/docker-volume"; + +const mocks = vi.hoisted(() => ({ + execAsync: vi.fn(), + execAsyncRemote: vi.fn(), +})); + +vi.mock("@dokploy/server/utils/process/execAsync", () => ({ + execAsync: mocks.execAsync, + execAsyncRemote: mocks.execAsyncRemote, +})); + +// shell-quote's quote() backslash-escapes "=" and "," in the raw command +// string; parse() the command to assert the literal token docker receives. +const hasToken = (command: string, token: string) => + parse(command).some((t) => t === token); + +describe("volumeNameRegex", () => { + // Docker's CLI treats a `-v`/`--mount` source starting with "." or "-" as a + // client-relative bind-mount path, not a named volume — the exact escape + // this fix closes. These must not pass validation. + const REJECTED = [ + ".", + "..", + ".foo", + ".ssh", + "-foo", + "-", + "/", + "foo/bar", + "", + " leading-space", + ]; + + it.each(REJECTED)("rejects %p", (value) => { + expect(volumeNameRegex.test(value)).toBe(false); + }); + + const ACCEPTED = [ + "a", + "0", + "myvol", + "my-vol", + "my_vol", + "my.vol", + "abc123", + "Volume1.2-3_4", + "dbdata", + ]; + + it.each(ACCEPTED)("accepts %p", (value) => { + expect(volumeNameRegex.test(value)).toBe(true); + }); +}); + +describe("docker-volume service commands", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.execAsync.mockResolvedValue({ stdout: "", stderr: "" }); + mocks.execAsyncRemote.mockResolvedValue({ stdout: "", stderr: "" }); + }); + + const READONLY = "type=volume,source=myvol,target=/__volume,readonly"; + const READWRITE = "type=volume,source=myvol,target=/__volume"; + + it("listVolumeFiles mounts a read-only named volume, not a bind mount", async () => { + await listVolumeFiles("myvol", "/dir"); + expect(mocks.execAsync).toHaveBeenCalledOnce(); + const command = mocks.execAsync.mock.calls[0]?.[0] as string; + expect(hasToken(command, READONLY)).toBe(true); + expect(command).not.toContain(" -v "); + expect(command).toContain("busybox ls -1Ap"); + }); + + it("readVolumeFile mounts the volume read-only", async () => { + await readVolumeFile("myvol", "/file.txt"); + const command = mocks.execAsync.mock.calls[0]?.[0] as string; + expect(hasToken(command, READONLY)).toBe(true); + expect(command).not.toContain(" -v "); + expect(command).toContain("busybox cat"); + }); + + it("writeVolumeFile mounts the volume read-write", async () => { + await writeVolumeFile("myvol", "/file.txt", "hello"); + const command = mocks.execAsync.mock.calls[0]?.[0] as string; + expect(hasToken(command, READWRITE)).toBe(true); + expect(hasToken(command, READONLY)).toBe(false); + expect(command).not.toContain("readonly"); + expect(command).not.toContain(" -v "); + expect(command).toContain("busybox sh -c"); + }); + + it("deleteVolumeFile mounts the volume read-write", async () => { + await deleteVolumeFile("myvol", "/dir"); + const command = mocks.execAsync.mock.calls[0]?.[0] as string; + expect(hasToken(command, READWRITE)).toBe(true); + expect(command).not.toContain(" -v "); + expect(command).toContain("busybox rm -rf"); + }); + + it("uses execAsyncRemote with the serverId on remote calls", async () => { + await listVolumeFiles("myvol", "/dir", "srv-1"); + await deleteVolumeFile("myvol", "/dir", "srv-2"); + expect(mocks.execAsync).not.toHaveBeenCalled(); + expect(mocks.execAsyncRemote).toHaveBeenCalledTimes(2); + const [listServerId, listCommand] = mocks.execAsyncRemote.mock.calls[0] as [ + string, + string, + ]; + const [delServerId, delCommand] = mocks.execAsyncRemote.mock.calls[1] as [ + string, + string, + ]; + expect(listServerId).toBe("srv-1"); + expect(hasToken(listCommand, READONLY)).toBe(true); + expect(listCommand).not.toContain(" -v "); + expect(delServerId).toBe("srv-2"); + expect(hasToken(delCommand, READWRITE)).toBe(true); + expect(delCommand).not.toContain(" -v "); + }); +}); diff --git a/apps/dokploy/server/api/routers/docker-volume.ts b/apps/dokploy/server/api/routers/docker-volume.ts index 643dff5d4..189b546ea 100644 --- a/apps/dokploy/server/api/routers/docker-volume.ts +++ b/apps/dokploy/server/api/routers/docker-volume.ts @@ -14,7 +14,11 @@ import { z } from "zod"; import { audit } from "@/server/api/utils/audit"; import { createTRPCRouter, withPermission } from "../trpc"; -export const volumeNameRegex = /^[a-zA-Z0-9.\-_]+$/; +// Docker only treats a `-v`/`--mount` source as a *named volume* when it starts +// with an alphanumeric character; a leading "." or "-" makes the CLI resolve it +// as a (client-relative) bind-mount path instead. Forbid those leading +// characters so a crafted volumeName can never escape into bind-mount mode. +export const volumeNameRegex = /^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/; const volumePathSchema = z .string() diff --git a/packages/server/src/services/docker-volume.ts b/packages/server/src/services/docker-volume.ts index 1fbe12376..65857b596 100644 --- a/packages/server/src/services/docker-volume.ts +++ b/packages/server/src/services/docker-volume.ts @@ -84,7 +84,7 @@ export const listVolumeFiles = async ( path: string, serverId?: string, ) => { - const command = `docker run --rm -v ${quote([`${volumeName}:${VOLUME_MOUNT}:ro`])} busybox ls -1Ap ${quote([`${VOLUME_MOUNT}${path}`])}`; + const command = `docker run --rm --mount ${quote([`type=volume,source=${volumeName},target=${VOLUME_MOUNT},readonly`])} busybox ls -1Ap ${quote([`${VOLUME_MOUNT}${path}`])}`; const { stdout } = serverId ? await execAsyncRemote(serverId, command) : await execAsync(command); @@ -109,7 +109,7 @@ export const readVolumeFile = async ( filePath: string, serverId?: string, ) => { - const command = `docker run --rm -v ${quote([`${volumeName}:${VOLUME_MOUNT}:ro`])} busybox cat ${quote([`${VOLUME_MOUNT}${filePath}`])} | head -c ${CONTAINER_FILE_SIZE_LIMIT + 1} | base64 | tr -d '\\n'`; + const command = `docker run --rm --mount ${quote([`type=volume,source=${volumeName},target=${VOLUME_MOUNT},readonly`])} busybox cat ${quote([`${VOLUME_MOUNT}${filePath}`])} | head -c ${CONTAINER_FILE_SIZE_LIMIT + 1} | base64 | tr -d '\\n'`; const { stdout, stderr } = serverId ? await execAsyncRemote(serverId, command) : await execAsync(command); @@ -137,7 +137,7 @@ export const writeVolumeFile = async ( } const innerCommand = `printf '%s' ${base64Content} | base64 -d > ${quote([`${VOLUME_MOUNT}${filePath}`])}`; - const command = `docker run --rm -v ${quote([`${volumeName}:${VOLUME_MOUNT}`])} busybox sh -c ${quote([innerCommand])}`; + const command = `docker run --rm --mount ${quote([`type=volume,source=${volumeName},target=${VOLUME_MOUNT}`])} busybox sh -c ${quote([innerCommand])}`; if (serverId) { await execAsyncRemote(serverId, command); @@ -151,7 +151,7 @@ export const deleteVolumeFile = async ( path: string, serverId?: string, ) => { - const command = `docker run --rm -v ${quote([`${volumeName}:${VOLUME_MOUNT}`])} busybox rm -rf ${quote([`${VOLUME_MOUNT}${path}`])}`; + const command = `docker run --rm --mount ${quote([`type=volume,source=${volumeName},target=${VOLUME_MOUNT}`])} busybox rm -rf ${quote([`${VOLUME_MOUNT}${path}`])}`; if (serverId) { await execAsyncRemote(serverId, command);