From 5563699f71b2058b49eebdfd66c6c3dbd92ede9c Mon Sep 17 00:00:00 2001 From: Mauricio Siu Date: Sun, 19 Jul 2026 21:20:47 -0600 Subject: [PATCH] fix(security): enforce org-scope on swarm reads and escape nodeId swarm.getNodes/getNodeInfo/getNodeApps/getAppInfos accepted a caller-supplied serverId and ran remote Docker Swarm reads against it with no organization check, exposing another tenant's swarm topology cross-org (getContainerStats already had the check; the others did not). getNodeInfo additionally interpolated nodeId unescaped into 'docker node inspect ${nodeId}'. Adds an org-ownership assertion to the four unscoped handlers and passes nodeId through shell-quote in getNodeInfo. Closes GHSA-jj6h-388v-9rwm --- .../server/swarm-nodeid-injection.test.ts | 41 +++++++++++++++++++ apps/dokploy/server/api/routers/swarm.ts | 41 +++++++++++++++++-- packages/server/src/services/docker.ts | 3 +- 3 files changed, 80 insertions(+), 5 deletions(-) create mode 100644 apps/dokploy/__test__/server/swarm-nodeid-injection.test.ts diff --git a/apps/dokploy/__test__/server/swarm-nodeid-injection.test.ts b/apps/dokploy/__test__/server/swarm-nodeid-injection.test.ts new file mode 100644 index 000000000..24a7a97f6 --- /dev/null +++ b/apps/dokploy/__test__/server/swarm-nodeid-injection.test.ts @@ -0,0 +1,41 @@ +import { execSync } from "node:child_process"; +import { existsSync, rmSync } from "node:fs"; +import { quote } from "shell-quote"; +import { describe, expect, it } from "vitest"; + +// Mirrors how getNodeInfo builds its command in services/docker.ts: +// `docker node inspect ${quote([nodeId])} --format '{{json .}}'` +// We swap `docker node inspect` for `:` (a no-op) so the test only exercises +// whether the nodeId payload can break out of the command, not real docker. +const buildCommand = (nodeId: string) => + `: node inspect ${quote([nodeId])} --format '{{json .}}'`; + +const INJECTION_NODE_IDS = [ + "$(touch %MARK%)", + "`touch %MARK%`", + "; touch %MARK%", + "abc | touch %MARK%", + "&& touch %MARK%", +]; + +describe("getNodeInfo nodeId command injection", () => { + it("does not execute injected commands from the nodeId", () => { + const mark = `/tmp/dokploy_swarm_pwned_${process.pid}`; + for (const template of INJECTION_NODE_IDS) { + if (existsSync(mark)) rmSync(mark); + const nodeId = template.replace("%MARK%", mark); + try { + execSync(buildCommand(nodeId), { shell: "/bin/sh", stdio: "ignore" }); + } catch { + // A non-zero exit from the no-op is fine; we only care about the marker. + } + expect(existsSync(mark)).toBe(false); + } + if (existsSync(mark)) rmSync(mark); + }); + + it("keeps a legitimate node id intact as a single literal token", () => { + const nodeId = "abc123def456"; + expect(quote([nodeId])).toBe(nodeId); + }); +}); diff --git a/apps/dokploy/server/api/routers/swarm.ts b/apps/dokploy/server/api/routers/swarm.ts index f5c8f02c6..2921bffc5 100644 --- a/apps/dokploy/server/api/routers/swarm.ts +++ b/apps/dokploy/server/api/routers/swarm.ts @@ -11,6 +11,23 @@ import { z } from "zod"; import { createTRPCRouter, withPermission } from "../trpc"; import { containerIdRegex } from "./docker"; +// Ensures a caller-supplied serverId belongs to the caller's active +// organization before any Swarm read runs against it. Without this, the +// server-scoped read procedures were reachable cross-organization (IDOR). +const assertServerInActiveOrg = async ( + serverId: string | undefined, + activeOrganizationId: string | undefined, +) => { + if (!serverId) return; + const server = await findServerById(serverId); + if (server.organizationId !== activeOrganizationId) { + throw new TRPCError({ + code: "UNAUTHORIZED", + message: "You are not authorized to access this server", + }); + } +}; + export const swarmRouter = createTRPCRouter({ getNodes: withPermission("server", "read") .input( @@ -18,12 +35,20 @@ export const swarmRouter = createTRPCRouter({ serverId: z.string().optional(), }), ) - .query(async ({ input }) => { + .query(async ({ input, ctx }) => { + await assertServerInActiveOrg( + input.serverId, + ctx.session?.activeOrganizationId, + ); return await getSwarmNodes(input.serverId); }), getNodeInfo: withPermission("server", "read") .input(z.object({ nodeId: z.string(), serverId: z.string().optional() })) - .query(async ({ input }) => { + .query(async ({ input, ctx }) => { + await assertServerInActiveOrg( + input.serverId, + ctx.session?.activeOrganizationId, + ); return await getNodeInfo(input.nodeId, input.serverId); }), getNodeApps: withPermission("server", "read") @@ -32,7 +57,11 @@ export const swarmRouter = createTRPCRouter({ serverId: z.string().optional(), }), ) - .query(async ({ input }) => { + .query(async ({ input, ctx }) => { + await assertServerInActiveOrg( + input.serverId, + ctx.session?.activeOrganizationId, + ); return getNodeApplications(input.serverId); }), getAppInfos: withPermission("server", "read") @@ -54,7 +83,11 @@ export const swarmRouter = createTRPCRouter({ serverId: z.string().optional(), }), ) - .query(async ({ input }) => { + .query(async ({ input, ctx }) => { + await assertServerInActiveOrg( + input.serverId, + ctx.session?.activeOrganizationId, + ); return await getApplicationInfo(input.appName, input.serverId); }), getContainerStats: withPermission("server", "read") diff --git a/packages/server/src/services/docker.ts b/packages/server/src/services/docker.ts index 226230fbc..ea200e0c0 100644 --- a/packages/server/src/services/docker.ts +++ b/packages/server/src/services/docker.ts @@ -2,6 +2,7 @@ import { execAsync, execAsyncRemote, } from "@dokploy/server/utils/process/execAsync"; +import { quote } from "shell-quote"; export const getContainers = async (serverId?: string | null) => { try { @@ -519,7 +520,7 @@ export const getSwarmNodes = async (serverId?: string) => { export const getNodeInfo = async (nodeId: string, serverId?: string) => { try { - const command = `docker node inspect ${nodeId} --format '{{json .}}'`; + const command = `docker node inspect ${quote([nodeId])} --format '{{json .}}'`; let stdout = ""; let stderr = ""; if (serverId) {