mirror of
https://github.com/Dokploy/dokploy.git
synced 2026-10-19 07:01:35 +00:00
fix(security): prevent bind-mount escape in Docker volume file explorer
This commit is contained in:
@@ -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 ");
|
||||
});
|
||||
});
|
||||
@@ -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()
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user