From f593f4f0fed2f951b30cb543223689839e3e80f1 Mon Sep 17 00:00:00 2001 From: "detail-app[bot]" <180357370+detail-app[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 02:44:17 +0000 Subject: [PATCH] fix(libsql): validate appName and quote shell sinks to prevent command injection --- .../server/libsql-appname-injection.test.ts | 229 ++++++++++++++++++ packages/server/src/db/schema/libsql.ts | 9 +- packages/server/src/services/docker.ts | 4 +- packages/server/src/utils/docker/utils.ts | 16 +- 4 files changed, 250 insertions(+), 8 deletions(-) create mode 100644 apps/dokploy/__test__/server/libsql-appname-injection.test.ts diff --git a/apps/dokploy/__test__/server/libsql-appname-injection.test.ts b/apps/dokploy/__test__/server/libsql-appname-injection.test.ts new file mode 100644 index 000000000..db09e529d --- /dev/null +++ b/apps/dokploy/__test__/server/libsql-appname-injection.test.ts @@ -0,0 +1,229 @@ +import { execSync } from "node:child_process"; +import { existsSync, rmSync } from "node:fs"; +import { apiCreateLibsql } from "@dokploy/server/db/schema"; +import { getContainerLogs } from "@dokploy/server/services/docker"; +import { + removeService, + startService, + startServiceRemote, + stopService, + stopServiceRemote, +} from "@dokploy/server/utils/docker/utils"; +import { quote } from "shell-quote"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// The libsql `appName` is later interpolated raw into shell commands in +// packages/server/src/utils/docker/utils.ts (stopService/startService/ +// removeService and their *Remote variants) and packages/server/src/ +// services/docker.ts (the two `--filter` find commands in getContainerLogs). +// We mock execAsync/execAsyncRemote to *capture* the exact command string the +// server would run, then replay it through /bin/sh with `docker` swapped for +// `:` (a shell no-op) so the test only exercises shell execution semantics — +// mirroring the convention in __test__/server/swarm-nodeid-injection.test.ts +// but against the REAL sink functions rather than a reimplementation. + +const { execAsync, execAsyncRemote } = vi.hoisted(() => ({ + execAsync: vi.fn(), + execAsyncRemote: vi.fn(), +})); + +vi.mock("@dokploy/server/utils/process/execAsync", async (importOriginal) => { + const actual = + await importOriginal< + typeof import("@dokploy/server/utils/process/execAsync") + >(); + return { + ...actual, + execAsync, + execAsyncRemote, + }; +}); + +let captured: { command: string }[] = []; + +const resetSinks = () => { + captured = []; + execAsync.mockImplementation(async (command: string) => { + captured.push({ command }); + return { stdout: "", stderr: "" }; + }); + execAsyncRemote.mockImplementation( + async (_serverId: string, command: string) => { + captured.push({ command }); + return { stdout: "", stderr: "" }; + }, + ); +}; + +// `docker` -> `:` so the replay runs through the shell without touching docker. +const toNoOp = (command: string) => command.replace(/\bdocker\b/g, ":"); + +// Returns true if running `command` (no-op'd) creates no marker file. +const shellIsSafe = (command: string, mark: string) => { + if (existsSync(mark)) rmSync(mark); + try { + execSync(toNoOp(command), { shell: "/bin/sh", stdio: "ignore" }); + } catch { + // A non-zero exit is fine; we only care whether the marker was created. + } + const created = existsSync(mark); + if (created) rmSync(mark); + return !created; +}; + +const VALID_BASE = { + name: "my-libsql", + appName: "my-libsql", + dockerImage: "ghcr.io/tursodatabase/libsql-server:v0.24.32", + environmentId: "env-1", + description: "", + databaseUser: "root", + databasePassword: "password123", + sqldNode: "primary" as const, + sqldPrimaryUrl: null, + enableNamespaces: false, + serverId: "", +}; + +// Shell-injection payloads that the pre-fix libsql schema persisted verbatim +// because cleanAppName only lowercases and swaps spaces for `-` (it leaves +// $, backticks, ;, |, &, >, " etc. untouched). %MARK% is replaced with a +// unique file path; if the shell evaluates the payload the marker appears. +const INJECTION_TEMPLATES = [ + "$(touch %MARK%)", + "`touch %MARK%`", + "$(>%MARK%)", + "$(>%MARK%);echo", + ";touch %MARK%;", + "|touch %MARK%|", + "&& touch %MARK%", + '$(id)";touch %MARK%;echo "', +]; + +const runSink = async (sink: string, appName: string) => { + resetSinks(); + switch (sink) { + case "stopService": + await stopService(appName); + break; + case "stopServiceRemote": + await stopServiceRemote("srv-1", appName); + break; + case "startService": + await startService(appName); + break; + case "startServiceRemote": + await startServiceRemote("srv-1", appName); + break; + case "removeService": + await removeService(appName); + break; + case "getContainerLogs": { + // The mock returns empty stdout, so both `--filter` finds are emitted + // before getContainerLogs throws "No container or service found". + try { + await getContainerLogs(appName, 100, "all", undefined, undefined); + } catch { + /* expected */ + } + break; + } + } + const cmds = [...captured]; + captured = []; + return cmds; +}; + +const SINKS = [ + "stopService", + "stopServiceRemote", + "startService", + "startServiceRemote", + "removeService", + "getContainerLogs", +] as const; + +describe("libsql.create appName schema validation (root-cause fix)", () => { + it("rejects shell-injection payloads that cleanAppName would have let through", () => { + const mark = `/tmp/dokploy_libsql_schema_${process.pid}`; + for (const template of INJECTION_TEMPLATES) { + const appName = template.replaceAll("%MARK%", mark); + const result = apiCreateLibsql.safeParse({ ...VALID_BASE, appName }); + expect(result.success, `appName=${appName}`).toBe(false); + } + if (existsSync(mark)) rmSync(mark); + }); + + it("rejects appName longer than 63 characters", () => { + const result = apiCreateLibsql.safeParse({ + ...VALID_BASE, + appName: "a".repeat(64), + }); + expect(result.success).toBe(false); + }); + + it("rejects an empty appName", () => { + const result = apiCreateLibsql.safeParse({ ...VALID_BASE, appName: "" }); + expect(result.success).toBe(false); + }); + + it("accepts legitimate appNames and preserves them unchanged", () => { + for (const appName of [ + "my-db", + "my.db_1", + "My-DB.2", + "libsql-prod-01", + "a", + "a.b.c-d_e", + ]) { + const result = apiCreateLibsql.safeParse({ ...VALID_BASE, appName }); + expect(result.success, `appName=${appName}`).toBe(true); + if (result.success) { + expect(result.data.appName).toBe(appName); + } + } + }); + + it("still accepts the whole valid payload when appName is valid", () => { + const result = apiCreateLibsql.safeParse({ + ...VALID_BASE, + appName: "my-libsql-prod", + }); + expect(result.success).toBe(true); + }); +}); + +describe("libsql shell sinks neutralize stored appName command injection", () => { + beforeEach(() => { + resetSinks(); + }); + + it("passes a legitimate appName through unchanged in every sink", async () => { + const legit = "libsql-my-db-a1b2c3"; + for (const sink of SINKS) { + const cmds = await runSink(sink, legit); + expect(cmds.length, sink).toBeGreaterThan(0); + for (const { command } of cmds) { + expect(command, sink).toContain(legit); + } + } + expect(quote([legit])).toBe(legit); + }); + + for (const sink of SINKS) { + it(`${sink}: no injected command executes from a malicious appName`, async () => { + for (const template of INJECTION_TEMPLATES) { + const stamp = Math.random().toString(36).slice(2); + const mark = `/tmp/dokploy_libsql_${sink}_${process.pid}_${stamp}`; + const appName = template.replaceAll("%MARK%", mark); + const cmds = await runSink(sink, appName); + expect(cmds.length, `payload=${appName}`).toBeGreaterThan(0); + for (const { command } of cmds) { + expect(shellIsSafe(command, mark), `${sink} command=${command}`).toBe( + true, + ); + } + } + }); + } +}); diff --git a/packages/server/src/db/schema/libsql.ts b/packages/server/src/db/schema/libsql.ts index 0307991ae..fbbda82fc 100644 --- a/packages/server/src/db/schema/libsql.ts +++ b/packages/server/src/db/schema/libsql.ts @@ -35,6 +35,8 @@ import { UpdateConfigSwarmSchema, } from "./shared"; import { + APP_NAME_MESSAGE, + APP_NAME_REGEX, DATABASE_PASSWORD_MESSAGE, DATABASE_PASSWORD_REGEX, encryptedText, @@ -115,7 +117,12 @@ export const libsqlRelations = relations(libsql, ({ one, many }) => ({ const createSchema = createInsertSchema(libsql, { libsqlId: z.string(), name: z.string().min(1), - appName: z.string().min(1), + appName: z + .string() + .min(1) + .max(63) + .regex(APP_NAME_REGEX, APP_NAME_MESSAGE) + .optional(), createdAt: z.string(), databaseUser: z.string().min(1), databasePassword: z.string().regex(DATABASE_PASSWORD_REGEX, { diff --git a/packages/server/src/services/docker.ts b/packages/server/src/services/docker.ts index 65eb29041..0e9b86f47 100644 --- a/packages/server/src/services/docker.ts +++ b/packages/server/src/services/docker.ts @@ -435,14 +435,14 @@ export const getContainerLogs = async ( if (!useContainerIdDirectly) { // Find the real container ID by appName filter const findResult = await exec( - `docker ps -q --filter "name=^${appNameOrId}" | head -1`, + `docker ps -q --filter ${quote([`name=^${appNameOrId}`])} | head -1`, ); const containerId = findResult.stdout.trim(); if (!containerId) { // Fallback: try as a swarm service const svcResult = await exec( - `docker service ls -q --filter "name=${appNameOrId}" | head -1`, + `docker service ls -q --filter ${quote([`name=${appNameOrId}`])} | head -1`, ); const serviceId = svcResult.stdout.trim(); if (!serviceId) { diff --git a/packages/server/src/utils/docker/utils.ts b/packages/server/src/utils/docker/utils.ts index e342eb450..b2258787e 100644 --- a/packages/server/src/utils/docker/utils.ts +++ b/packages/server/src/utils/docker/utils.ts @@ -110,7 +110,7 @@ export const containerExists = async (containerName: string) => { export const stopService = async (appName: string) => { try { - await execAsync(`docker service scale ${appName}=0 `); + await execAsync(`docker service scale ${quote([appName])}=0 `); } catch (error) { console.error(error); return error; @@ -119,7 +119,10 @@ export const stopService = async (appName: string) => { export const stopServiceRemote = async (serverId: string, appName: string) => { try { - await execAsyncRemote(serverId, `docker service scale ${appName}=0 `); + await execAsyncRemote( + serverId, + `docker service scale ${quote([appName])}=0 `, + ); } catch (error) { console.error(error); return error; @@ -422,7 +425,7 @@ export const cleanupAllBackground = async (serverId?: string) => { export const startService = async (appName: string) => { try { - await execAsync(`docker service scale ${appName}=1 `); + await execAsync(`docker service scale ${quote([appName])}=1 `); } catch (error) { console.error(error); throw error; @@ -431,7 +434,10 @@ export const startService = async (appName: string) => { export const startServiceRemote = async (serverId: string, appName: string) => { try { - await execAsyncRemote(serverId, `docker service scale ${appName}=1 `); + await execAsyncRemote( + serverId, + `docker service scale ${quote([appName])}=1 `, + ); } catch (error) { console.error(error); throw error; @@ -444,7 +450,7 @@ export const removeService = async ( _deleteVolumes = false, ) => { try { - const command = `docker service rm ${appName}`; + const command = `docker service rm ${quote([appName])}`; if (serverId) { await execAsyncRemote(serverId, command);