From 6aa3ee39b32c388748c49cf8c559db40adf887c6 Mon Sep 17 00:00:00 2001 From: merge-check <140916359+agentHits@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:47:19 -0400 Subject: [PATCH 1/3] fix: scope docker container and volume file access to admins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Что: - docker.listContainerFiles, readContainerFile, writeContainerFile и deleteContainerFile теперь проходят assertDockerServerAccess (только owner/admin и доступный сервер) и resolveAuthorizedContainerId (локальный контейнер должен принадлежать сервису, к которому есть доступ). - writeContainerFile требует docker.write, deleteContainerFile требует docker.delete вместо docker.read. - docker.getEvents использует ту же проверку owner/admin и доступа к серверу. - dockerVolume.listVolumeFiles, readVolumeFile, writeVolumeFile, deleteVolumeFile и removeVolume используют assertDockerServerAccess; запись требует docker.write, удаление docker.delete. Зачем: - Раньше эти процедуры проверяли только docker.read и принадлежность сервера организации, поэтому пользователь с docker.read мог читать, менять и удалять файлы в любом контейнере или volume и удалять volumes. Риски: - Owner/admin больше не могут открыть файлы локальных контейнеров, которые не привязаны к сервису Dokploy (так же, как уже работают restart/upload). - Пользователи с кастомными ролями, у которых есть docker.read, больше не видят файлы контейнеров и volumes и Docker events. Проверки: - Команды и результаты: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/docker/docker-router-security.test.ts __test__/docker/docker-volume-router-security.test.ts - все тесты прошли; новые тесты падают на старом коде (исходники временно возвращены к origin/AgentHits-Dev); bun run typecheck - exit 0; bunx biome check по измененным файлам - без ошибок. - Ограничения: UI проверен только чтением кода (файловый менеджер есть только на странице Docker, где список контейнеров уже требует owner/admin); на реальном Docker не запускалось. What: - docker.listContainerFiles, readContainerFile, writeContainerFile and deleteContainerFile now go through assertDockerServerAccess (owner/admin plus an accessible server) and resolveAuthorizedContainerId (a local container must belong to a service the caller may access). - writeContainerFile requires docker.write and deleteContainerFile requires docker.delete instead of docker.read. - docker.getEvents uses the same owner/admin and server access check. - dockerVolume.listVolumeFiles, readVolumeFile, writeVolumeFile, deleteVolumeFile and removeVolume use assertDockerServerAccess; writes require docker.write, deletes docker.delete. Why: - These procedures only checked docker.read and that the server belongs to the organization, so a docker.read holder could read, change and delete files in any container or volume and remove volumes. Risks: - Owners/admins can no longer open files of local containers that are not bound to a Dokploy service (the same rule restart/upload already follow). - Custom roles with docker.read no longer see container/volume files or Docker events. Checks: - Commands and results: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/docker/docker-router-security.test.ts __test__/docker/docker-volume-router-security.test.ts - all passed; the new tests fail on the old code (sources temporarily reset to origin/AgentHits-Dev); bun run typecheck - exit 0; bunx biome check on changed files - no errors. - Limitations: UI checked by reading code only (the file explorer lives on the Docker page, whose container list already requires owner/admin); not run against a real Docker host. Co-Authored-By: Claude Opus 5.5 --- .../docker/docker-router-security.test.ts | 222 ++++++++++++++++++ .../docker-volume-router-security.test.ts | 201 ++++++++++++++++ .../server/api/routers/docker-volume.ts | 42 +--- apps/dokploy/server/api/routers/docker.ts | 77 +++--- 4 files changed, 466 insertions(+), 76 deletions(-) create mode 100644 apps/dokploy/__test__/docker/docker-volume-router-security.test.ts diff --git a/apps/dokploy/__test__/docker/docker-router-security.test.ts b/apps/dokploy/__test__/docker/docker-router-security.test.ts index 5ad535b0960..2a06fb73cca 100644 --- a/apps/dokploy/__test__/docker/docker-router-security.test.ts +++ b/apps/dokploy/__test__/docker/docker-router-security.test.ts @@ -10,6 +10,7 @@ const mocks = vi.hoisted(() => ({ containerRestart: vi.fn(), containerStart: vi.fn(), containerStop: vi.fn(), + deleteContainerFile: vi.fn(), findMemberByUserId: vi.fn(), findServerById: vi.fn(), getAccessibleServerIds: vi.fn(), @@ -17,9 +18,13 @@ const mocks = vi.hoisted(() => ({ getContainers: vi.fn(), getContainersByAppLabel: vi.fn(), getContainersByAppNameMatch: vi.fn(), + getDockerEvents: vi.fn(), getServiceContainersByAppName: vi.fn(), getStackContainersByAppName: vi.fn(), + listContainerFiles: vi.fn(), + readContainerFile: vi.fn(), uploadFileToContainer: vi.fn(), + writeContainerFile: vi.fn(), })); vi.mock("@dokploy/server", () => ({ @@ -28,15 +33,20 @@ vi.mock("@dokploy/server", () => ({ containerRestart: mocks.containerRestart, containerStart: mocks.containerStart, containerStop: mocks.containerStop, + deleteContainerFile: mocks.deleteContainerFile, findServerById: mocks.findServerById, getAccessibleServerIds: mocks.getAccessibleServerIds, getConfig: mocks.getConfig, getContainers: mocks.getContainers, getContainersByAppLabel: mocks.getContainersByAppLabel, getContainersByAppNameMatch: mocks.getContainersByAppNameMatch, + getDockerEvents: mocks.getDockerEvents, getServiceContainersByAppName: mocks.getServiceContainersByAppName, getStackContainersByAppName: mocks.getStackContainersByAppName, + listContainerFiles: mocks.listContainerFiles, + readContainerFile: mocks.readContainerFile, uploadFileToContainer: mocks.uploadFileToContainer, + writeContainerFile: mocks.writeContainerFile, })); vi.mock("@dokploy/server/services/permission", () => ({ @@ -241,3 +251,215 @@ describe("docker router assigned-server boundary", () => { ); }); }); + +describe("docker router container file and event boundary", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.checkPermission.mockResolvedValue(undefined); + mocks.findMemberByUserId.mockResolvedValue({ role: "admin" }); + mocks.findServerById.mockResolvedValue({ + serverId: "server-1", + organizationId: "org-1", + }); + mocks.assertLocalDockerContainerAccess.mockResolvedValue({ + Id: "resolved-container-1", + Config: { + Labels: { + "com.docker.swarm.service.name": "app-1", + }, + }, + }); + mocks.getAccessibleServerIds.mockResolvedValue(new Set(["server-1"])); + mocks.listContainerFiles.mockResolvedValue([]); + mocks.readContainerFile.mockResolvedValue({ content: "" }); + mocks.getDockerEvents.mockResolvedValue([]); + }); + + const fileCalls = ( + caller: ReturnType, + serverId?: string, + ) => [ + { + name: "listContainerFiles", + run: () => + caller.listContainerFiles({ + containerId: "container-1", + path: "/", + serverId, + }), + sideEffect: mocks.listContainerFiles, + }, + { + name: "readContainerFile", + run: () => + caller.readContainerFile({ + containerId: "container-1", + path: "/etc/app.env", + serverId, + }), + sideEffect: mocks.readContainerFile, + }, + { + name: "writeContainerFile", + run: () => + caller.writeContainerFile({ + containerId: "container-1", + path: "/etc/app.env", + content: "KEY=value", + serverId, + }), + sideEffect: mocks.writeContainerFile, + }, + { + name: "deleteContainerFile", + run: () => + caller.deleteContainerFile({ + containerId: "container-1", + path: "/etc/app.env", + serverId, + }), + sideEffect: mocks.deleteContainerFile, + }, + ]; + + it("denies non-admin container file access before Docker side effects", async () => { + mocks.findMemberByUserId.mockResolvedValue({ role: "member" }); + + for (const call of fileCalls(createCaller())) { + await expect(call.run(), call.name).rejects.toMatchObject({ + code: "UNAUTHORIZED", + }); + expect(call.sideEffect, call.name).not.toHaveBeenCalled(); + } + expect(mocks.audit).not.toHaveBeenCalled(); + }); + + it("denies container file access on inaccessible servers", async () => { + mocks.getAccessibleServerIds.mockResolvedValue(new Set(["server-2"])); + + for (const call of fileCalls(createCaller(), "server-1")) { + await expect(call.run(), call.name).rejects.toMatchObject({ + code: "UNAUTHORIZED", + }); + expect(call.sideEffect, call.name).not.toHaveBeenCalled(); + } + expect(mocks.audit).not.toHaveBeenCalled(); + }); + + it("denies local container file access for containers outside the caller's services", async () => { + mocks.assertLocalDockerContainerAccess.mockRejectedValue( + new TRPCError({ code: "UNAUTHORIZED" }), + ); + + for (const call of fileCalls(createCaller())) { + await expect(call.run(), call.name).rejects.toMatchObject({ + code: "UNAUTHORIZED", + }); + expect(call.sideEffect, call.name).not.toHaveBeenCalled(); + } + expect(mocks.audit).not.toHaveBeenCalled(); + }); + + it("binds local container file reads to the authorized container", async () => { + const caller = createCaller(); + + await caller.listContainerFiles({ containerId: "container-1", path: "/" }); + await caller.readContainerFile({ + containerId: "container-1", + path: "/etc/app.env", + }); + + expect(mocks.assertLocalDockerContainerAccess).toHaveBeenCalledWith( + expect.anything(), + "container-1", + "read", + ); + expect(mocks.listContainerFiles).toHaveBeenCalledWith( + "resolved-container-1", + "/", + undefined, + ); + expect(mocks.readContainerFile).toHaveBeenCalledWith( + "resolved-container-1", + "/etc/app.env", + undefined, + ); + }); + + it("requires docker.write for container file writes", async () => { + await createCaller().writeContainerFile({ + containerId: "container-1", + path: "/etc/app.env", + content: "KEY=value", + }); + + expect(mocks.checkPermission).toHaveBeenCalledWith(expect.anything(), { + docker: ["write"], + }); + expect(mocks.assertLocalDockerContainerAccess).toHaveBeenCalledWith( + expect.anything(), + "container-1", + "write", + ); + expect(mocks.writeContainerFile).toHaveBeenCalledWith( + "resolved-container-1", + "/etc/app.env", + "KEY=value", + undefined, + ); + }); + + it("requires docker.delete for container file deletes", async () => { + await createCaller().deleteContainerFile({ + containerId: "container-1", + path: "/etc/app.env", + }); + + expect(mocks.checkPermission).toHaveBeenCalledWith(expect.anything(), { + docker: ["delete"], + }); + expect(mocks.assertLocalDockerContainerAccess).toHaveBeenCalledWith( + expect.anything(), + "container-1", + "delete", + ); + expect(mocks.deleteContainerFile).toHaveBeenCalledWith( + "resolved-container-1", + "/etc/app.env", + undefined, + ); + }); + + it("denies non-admin Docker event reads", async () => { + mocks.findMemberByUserId.mockResolvedValue({ role: "member" }); + + await expect(createCaller().getEvents({})).rejects.toMatchObject({ + code: "UNAUTHORIZED", + }); + + expect(mocks.getDockerEvents).not.toHaveBeenCalled(); + }); + + it("denies Docker event reads on inaccessible servers", async () => { + mocks.getAccessibleServerIds.mockResolvedValue(new Set(["server-2"])); + + await expect( + createCaller().getEvents({ serverId: "server-1" }), + ).rejects.toMatchObject({ code: "UNAUTHORIZED" }); + + expect(mocks.getDockerEvents).not.toHaveBeenCalled(); + }); + + it("checks admin role and server access before reading Docker events", async () => { + await expect( + createCaller().getEvents({ serverId: "server-1", minutes: 5 }), + ).resolves.toEqual([]); + + expect(mocks.findMemberByUserId).toHaveBeenCalledWith("user-1", "org-1"); + expect(mocks.getAccessibleServerIds).toHaveBeenCalledWith({ + userId: "user-1", + activeOrganizationId: "org-1", + }); + expect(mocks.getDockerEvents).toHaveBeenCalledWith("server-1", 5); + }); +}); diff --git a/apps/dokploy/__test__/docker/docker-volume-router-security.test.ts b/apps/dokploy/__test__/docker/docker-volume-router-security.test.ts new file mode 100644 index 00000000000..77eee8d13b2 --- /dev/null +++ b/apps/dokploy/__test__/docker/docker-volume-router-security.test.ts @@ -0,0 +1,201 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + audit: vi.fn(), + assertLocalDockerContainerAccess: vi.fn(), + checkPermission: vi.fn(), + deleteVolumeFile: vi.fn(), + findMemberByUserId: vi.fn(), + findServerById: vi.fn(), + getAccessibleServerIds: vi.fn(), + getVolumeConfig: vi.fn(), + getVolumes: vi.fn(), + getVolumesSize: vi.fn(), + listVolumeFiles: vi.fn(), + readVolumeFile: vi.fn(), + removeVolume: vi.fn(), + writeVolumeFile: vi.fn(), +})); + +vi.mock("@dokploy/server", () => ({ + deleteVolumeFile: mocks.deleteVolumeFile, + findServerById: mocks.findServerById, + getAccessibleServerIds: mocks.getAccessibleServerIds, + getVolumeConfig: mocks.getVolumeConfig, + getVolumes: mocks.getVolumes, + getVolumesSize: mocks.getVolumesSize, + listVolumeFiles: mocks.listVolumeFiles, + readVolumeFile: mocks.readVolumeFile, + removeVolume: mocks.removeVolume, + writeVolumeFile: mocks.writeVolumeFile, +})); + +vi.mock("@dokploy/server/services/permission", () => ({ + checkPermission: mocks.checkPermission, + findMemberByUserId: mocks.findMemberByUserId, +})); + +vi.mock("@/server/api/utils/audit", () => ({ + audit: mocks.audit, +})); + +vi.mock("@/server/api/utils/local-docker-access", () => ({ + assertLocalDockerContainerAccess: mocks.assertLocalDockerContainerAccess, +})); + +const { dockerVolumeRouter } = await import( + "../../server/api/routers/docker-volume" +); + +const createCaller = () => + dockerVolumeRouter.createCaller({ + db: {}, + req: {}, + res: {}, + session: { + userId: "user-1", + activeOrganizationId: "org-1", + }, + user: { + id: "user-1", + role: "member", + }, + } as never); + +const volumeCalls = (serverId?: string) => { + const caller = createCaller(); + return [ + { + name: "listVolumeFiles", + run: () => + caller.listVolumeFiles({ volumeName: "data", path: "/", serverId }), + sideEffect: mocks.listVolumeFiles, + }, + { + name: "readVolumeFile", + run: () => + caller.readVolumeFile({ + volumeName: "data", + path: "/app.env", + serverId, + }), + sideEffect: mocks.readVolumeFile, + }, + { + name: "writeVolumeFile", + run: () => + caller.writeVolumeFile({ + volumeName: "data", + path: "/app.env", + content: "KEY=value", + serverId, + }), + sideEffect: mocks.writeVolumeFile, + }, + { + name: "deleteVolumeFile", + run: () => + caller.deleteVolumeFile({ + volumeName: "data", + path: "/app.env", + serverId, + }), + sideEffect: mocks.deleteVolumeFile, + }, + { + name: "removeVolume", + run: () => caller.removeVolume({ volumeName: "data", serverId }), + sideEffect: mocks.removeVolume, + }, + ]; +}; + +describe("docker volume router access boundary", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.checkPermission.mockResolvedValue(undefined); + mocks.findMemberByUserId.mockResolvedValue({ role: "admin" }); + mocks.findServerById.mockResolvedValue({ + serverId: "server-1", + organizationId: "org-1", + }); + mocks.getAccessibleServerIds.mockResolvedValue(new Set(["server-1"])); + mocks.listVolumeFiles.mockResolvedValue([]); + mocks.readVolumeFile.mockResolvedValue({ content: "" }); + }); + + it("denies non-admin volume file access and removal on the local host", async () => { + mocks.findMemberByUserId.mockResolvedValue({ role: "member" }); + + for (const call of volumeCalls()) { + await expect(call.run(), call.name).rejects.toMatchObject({ + code: "UNAUTHORIZED", + }); + expect(call.sideEffect, call.name).not.toHaveBeenCalled(); + } + expect(mocks.audit).not.toHaveBeenCalled(); + }); + + it("denies non-admin volume file access on remote servers", async () => { + mocks.findMemberByUserId.mockResolvedValue({ role: "member" }); + + for (const call of volumeCalls("server-1")) { + await expect(call.run(), call.name).rejects.toMatchObject({ + code: "UNAUTHORIZED", + }); + expect(call.sideEffect, call.name).not.toHaveBeenCalled(); + } + }); + + it("denies volume file access on inaccessible servers", async () => { + mocks.getAccessibleServerIds.mockResolvedValue(new Set(["server-2"])); + + for (const call of volumeCalls("server-1")) { + await expect(call.run(), call.name).rejects.toMatchObject({ + code: "UNAUTHORIZED", + }); + expect(call.sideEffect, call.name).not.toHaveBeenCalled(); + } + expect(mocks.audit).not.toHaveBeenCalled(); + }); + + it("requires docker.write for writes and docker.delete for deletes", async () => { + const caller = createCaller(); + + await caller.writeVolumeFile({ + volumeName: "data", + path: "/app.env", + content: "KEY=value", + }); + expect(mocks.checkPermission).toHaveBeenLastCalledWith(expect.anything(), { + docker: ["write"], + }); + + await caller.deleteVolumeFile({ volumeName: "data", path: "/app.env" }); + expect(mocks.checkPermission).toHaveBeenLastCalledWith(expect.anything(), { + docker: ["delete"], + }); + + await caller.removeVolume({ volumeName: "data" }); + expect(mocks.checkPermission).toHaveBeenLastCalledWith(expect.anything(), { + docker: ["delete"], + }); + }); + + it("checks admin role and server access before browsing volumes", async () => { + await expect( + createCaller().listVolumeFiles({ + volumeName: "data", + path: "/", + serverId: "server-1", + }), + ).resolves.toEqual([]); + + expect(mocks.findMemberByUserId).toHaveBeenCalledWith("user-1", "org-1"); + expect(mocks.getAccessibleServerIds).toHaveBeenCalledWith({ + userId: "user-1", + activeOrganizationId: "org-1", + }); + expect(mocks.listVolumeFiles).toHaveBeenCalledWith("data", "/", "server-1"); + }); +}); diff --git a/apps/dokploy/server/api/routers/docker-volume.ts b/apps/dokploy/server/api/routers/docker-volume.ts index 643dff5d411..9fc30bd917a 100644 --- a/apps/dokploy/server/api/routers/docker-volume.ts +++ b/apps/dokploy/server/api/routers/docker-volume.ts @@ -13,6 +13,7 @@ import { TRPCError } from "@trpc/server"; import { z } from "zod"; import { audit } from "@/server/api/utils/audit"; import { createTRPCRouter, withPermission } from "../trpc"; +import { assertDockerServerAccess } from "./docker"; export const volumeNameRegex = /^[a-zA-Z0-9.\-_]+$/; @@ -70,12 +71,7 @@ export const dockerVolumeRouter = createTRPCRouter({ }), ) .query(async ({ input, ctx }) => { - if (input.serverId) { - const server = await findServerById(input.serverId); - if (server.organizationId !== ctx.session?.activeOrganizationId) { - throw new TRPCError({ code: "UNAUTHORIZED" }); - } - } + await assertDockerServerAccess(ctx, input.serverId); return await listVolumeFiles( input.volumeName, input.path, @@ -95,16 +91,11 @@ export const dockerVolumeRouter = createTRPCRouter({ }), ) .query(async ({ input, ctx }) => { - if (input.serverId) { - const server = await findServerById(input.serverId); - if (server.organizationId !== ctx.session?.activeOrganizationId) { - throw new TRPCError({ code: "UNAUTHORIZED" }); - } - } + await assertDockerServerAccess(ctx, input.serverId); return await readVolumeFile(input.volumeName, input.path, input.serverId); }), - writeVolumeFile: withPermission("docker", "read") + writeVolumeFile: withPermission("docker", "write") .input( z.object({ volumeName: z @@ -117,12 +108,7 @@ export const dockerVolumeRouter = createTRPCRouter({ }), ) .mutation(async ({ input, ctx }) => { - if (input.serverId) { - const server = await findServerById(input.serverId); - if (server.organizationId !== ctx.session?.activeOrganizationId) { - throw new TRPCError({ code: "UNAUTHORIZED" }); - } - } + await assertDockerServerAccess(ctx, input.serverId); await writeVolumeFile( input.volumeName, input.path, @@ -137,7 +123,7 @@ export const dockerVolumeRouter = createTRPCRouter({ }); }), - deleteVolumeFile: withPermission("docker", "read") + deleteVolumeFile: withPermission("docker", "delete") .input( z.object({ volumeName: z @@ -152,12 +138,7 @@ export const dockerVolumeRouter = createTRPCRouter({ }), ) .mutation(async ({ input, ctx }) => { - if (input.serverId) { - const server = await findServerById(input.serverId); - if (server.organizationId !== ctx.session?.activeOrganizationId) { - throw new TRPCError({ code: "UNAUTHORIZED" }); - } - } + await assertDockerServerAccess(ctx, input.serverId); await deleteVolumeFile(input.volumeName, input.path, input.serverId); await audit(ctx, { action: "delete", @@ -187,7 +168,7 @@ export const dockerVolumeRouter = createTRPCRouter({ return await getVolumeConfig(input.volumeName, input.serverId); }), - removeVolume: withPermission("docker", "read") + removeVolume: withPermission("docker", "delete") .input( z.object({ volumeName: z @@ -198,12 +179,7 @@ export const dockerVolumeRouter = createTRPCRouter({ }), ) .mutation(async ({ input, ctx }) => { - if (input.serverId) { - const server = await findServerById(input.serverId); - if (server.organizationId !== ctx.session?.activeOrganizationId) { - throw new TRPCError({ code: "UNAUTHORIZED" }); - } - } + await assertDockerServerAccess(ctx, input.serverId); await removeVolume(input.volumeName, input.serverId); await audit(ctx, { action: "delete", diff --git a/apps/dokploy/server/api/routers/docker.ts b/apps/dokploy/server/api/routers/docker.ts index 870a819085f..cd3147c54dd 100644 --- a/apps/dokploy/server/api/routers/docker.ts +++ b/apps/dokploy/server/api/routers/docker.ts @@ -36,7 +36,7 @@ import { createTRPCRouter, protectedProcedure, withPermission } from "../trpc"; export const containerIdRegex = /^[a-zA-Z0-9.\-_]+$/; -const assertDockerServerAccess = async ( +export const assertDockerServerAccess = async ( ctx: { session: { userId: string; @@ -400,17 +400,14 @@ export const dockerRouter = createTRPCRouter({ }), ) .query(async ({ input, ctx }) => { - if (input.serverId) { - const server = await findServerById(input.serverId); - if (server.organizationId !== ctx.session?.activeOrganizationId) { - throw new TRPCError({ code: "UNAUTHORIZED" }); - } - } - return await listContainerFiles( + await assertDockerServerAccess(ctx, input.serverId); + const containerId = await resolveAuthorizedContainerId( + ctx, input.containerId, - input.path, input.serverId, + "read", ); + return await listContainerFiles(containerId, input.path, input.serverId); }), readContainerFile: withPermission("docker", "read") @@ -425,20 +422,17 @@ export const dockerRouter = createTRPCRouter({ }), ) .query(async ({ input, ctx }) => { - if (input.serverId) { - const server = await findServerById(input.serverId); - if (server.organizationId !== ctx.session?.activeOrganizationId) { - throw new TRPCError({ code: "UNAUTHORIZED" }); - } - } - return await readContainerFile( + await assertDockerServerAccess(ctx, input.serverId); + const containerId = await resolveAuthorizedContainerId( + ctx, input.containerId, - input.path, input.serverId, + "read", ); + return await readContainerFile(containerId, input.path, input.serverId); }), - writeContainerFile: withPermission("docker", "read") + writeContainerFile: withPermission("docker", "write") .input( z.object({ containerId: z @@ -451,14 +445,15 @@ export const dockerRouter = createTRPCRouter({ }), ) .mutation(async ({ input, ctx }) => { - if (input.serverId) { - const server = await findServerById(input.serverId); - if (server.organizationId !== ctx.session?.activeOrganizationId) { - throw new TRPCError({ code: "UNAUTHORIZED" }); - } - } - await writeContainerFile( + await assertDockerServerAccess(ctx, input.serverId); + const containerId = await resolveAuthorizedContainerId( + ctx, input.containerId, + input.serverId, + "write", + ); + await writeContainerFile( + containerId, input.path, input.content, input.serverId, @@ -466,12 +461,12 @@ export const dockerRouter = createTRPCRouter({ await audit(ctx, { action: "update", resourceType: "docker", - resourceId: input.containerId, - resourceName: `${input.containerId}:${input.path}`, + resourceId: containerId, + resourceName: `${containerId}:${input.path}`, }); }), - deleteContainerFile: withPermission("docker", "read") + deleteContainerFile: withPermission("docker", "delete") .input( z.object({ containerId: z @@ -486,18 +481,19 @@ export const dockerRouter = createTRPCRouter({ }), ) .mutation(async ({ input, ctx }) => { - if (input.serverId) { - const server = await findServerById(input.serverId); - if (server.organizationId !== ctx.session?.activeOrganizationId) { - throw new TRPCError({ code: "UNAUTHORIZED" }); - } - } - await deleteContainerFile(input.containerId, input.path, input.serverId); + await assertDockerServerAccess(ctx, input.serverId); + const containerId = await resolveAuthorizedContainerId( + ctx, + input.containerId, + input.serverId, + "delete", + ); + await deleteContainerFile(containerId, input.path, input.serverId); await audit(ctx, { action: "delete", resourceType: "docker", - resourceId: input.containerId, - resourceName: `${input.containerId}:${input.path}`, + resourceId: containerId, + resourceName: `${containerId}:${input.path}`, }); }), @@ -509,12 +505,7 @@ export const dockerRouter = createTRPCRouter({ }), ) .query(async ({ input, ctx }) => { - if (input.serverId) { - const server = await findServerById(input.serverId); - if (server.organizationId !== ctx.session?.activeOrganizationId) { - throw new TRPCError({ code: "UNAUTHORIZED" }); - } - } + await assertDockerServerAccess(ctx, input.serverId); return await getDockerEvents(input.serverId, input.minutes); }), }); From 9a0aa3d5067159e474529205fd70b72682977ebe Mon Sep 17 00:00:00 2001 From: merge-check <140916359+agentHits@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:47:28 -0400 Subject: [PATCH 2/3] fix: require logs.read for service log reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Что: - readLogs в роутерах application, compose, postgres, mysql, mariadb, mongo, redis и libsql теперь вызывает checkServicePermissionAndAccess с { service: ["read"], logs: ["read"] } вместо checkServiceAccess(..., "read"). Зачем: - Право logs.read есть в модели прав, но эти процедуры его не проверяли: кастомная роль без logs.read все равно могла читать логи контейнеров сервиса. Риски: - Кастомные роли без logs.read теряют доступ к логам сервисов. Статические роли owner, admin и member имеют logs.read, их поведение не меняется. Проверки: - Команды и результаты: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/logs/read-logs-permission.test.ts - 8 тестов прошли; на старом коде все 8 падают (исходники временно возвращены к origin/AgentHits-Dev); bun run typecheck - exit 0; bunx biome check по измененным файлам - без ошибок. - Ограничения: логи через WebSocket (server/wss) не менялись. What: - readLogs in the application, compose, postgres, mysql, mariadb, mongo, redis and libsql routers now calls checkServicePermissionAndAccess with { service: ["read"], logs: ["read"] } instead of checkServiceAccess(..., "read"). Why: - logs.read exists in the permission model but these procedures never checked it, so a custom role without logs.read could still read a service's container logs. Risks: - Custom roles without logs.read lose access to service logs. The static owner, admin and member roles all grant logs.read, so their behavior is unchanged. Checks: - Commands and results: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/logs/read-logs-permission.test.ts - 8 tests passed; all 8 fail on the old code (sources temporarily reset to origin/AgentHits-Dev); bun run typecheck - exit 0; bunx biome check on changed files - no errors. - Limitations: WebSocket log streaming (server/wss) is unchanged. Co-Authored-By: Claude Opus 5.5 --- .../logs/read-logs-permission.test.ts | 343 ++++++++++++++++++ .../dokploy/server/api/routers/application.ts | 5 +- apps/dokploy/server/api/routers/compose.ts | 5 +- apps/dokploy/server/api/routers/libsql.ts | 5 +- apps/dokploy/server/api/routers/mariadb.ts | 5 +- apps/dokploy/server/api/routers/mongo.ts | 5 +- apps/dokploy/server/api/routers/mysql.ts | 5 +- apps/dokploy/server/api/routers/postgres.ts | 5 +- apps/dokploy/server/api/routers/redis.ts | 5 +- 9 files changed, 375 insertions(+), 8 deletions(-) create mode 100644 apps/dokploy/__test__/logs/read-logs-permission.test.ts diff --git a/apps/dokploy/__test__/logs/read-logs-permission.test.ts b/apps/dokploy/__test__/logs/read-logs-permission.test.ts new file mode 100644 index 00000000000..66499018b14 --- /dev/null +++ b/apps/dokploy/__test__/logs/read-logs-permission.test.ts @@ -0,0 +1,343 @@ +import { ac, memberRole } from "@dokploy/server/lib/access-control"; +import { TRPCError } from "@trpc/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const serverMocks = vi.hoisted(() => ({ + findApplicationById: vi.fn(), + findComposeById: vi.fn(), + findLibsqlById: vi.fn(), + findMariadbById: vi.fn(), + findMongoById: vi.fn(), + findMySqlById: vi.fn(), + findPostgresById: vi.fn(), + findRedisById: vi.fn(), + getContainerLogs: vi.fn(), + getContainersByAppNameMatch: vi.fn(), + noop: vi.fn(), +})); + +const permissionMocks = vi.hoisted(() => ({ + activeRole: { current: null as null | { authorize: (p: never) => unknown } }, + checkServiceAccess: vi.fn(), + checkServicePermissionAndAccess: vi.fn(), +})); + +vi.mock("@dokploy/server", () => ({ + IS_CLOUD: false, + addDomainToCompose: serverMocks.noop, + checkPortInUse: serverMocks.noop, + clearOldDeployments: serverMocks.noop, + cloneCompose: serverMocks.noop, + createApplication: serverMocks.noop, + createCommand: serverMocks.noop, + createCompose: serverMocks.noop, + createComposeByTemplate: serverMocks.noop, + createDomain: serverMocks.noop, + createLibsql: serverMocks.noop, + createMariadb: serverMocks.noop, + createMongo: serverMocks.noop, + createMount: serverMocks.noop, + createMysql: serverMocks.noop, + createPostgres: serverMocks.noop, + createRedis: serverMocks.noop, + deleteAllMiddlewares: serverMocks.noop, + deleteMount: serverMocks.noop, + deployLibsql: serverMocks.noop, + deployMariadb: serverMocks.noop, + deployMongo: serverMocks.noop, + deployMySql: serverMocks.noop, + deployPostgres: serverMocks.noop, + deployRedis: serverMocks.noop, + execAsync: serverMocks.noop, + execAsyncRemote: serverMocks.noop, + findApplicationById: serverMocks.findApplicationById, + findBackupsByDbId: serverMocks.noop, + findComposeById: serverMocks.findComposeById, + findDomainsByComposeId: serverMocks.noop, + findEnvironmentById: serverMocks.noop, + findLibsqlById: serverMocks.findLibsqlById, + findMariadbById: serverMocks.findMariadbById, + findMongoById: serverMocks.findMongoById, + findMySqlById: serverMocks.findMySqlById, + findPostgresById: serverMocks.findPostgresById, + findProjectById: serverMocks.noop, + findRedisById: serverMocks.findRedisById, + findRegistryById: serverMocks.noop, + findServerById: serverMocks.noop, + getAccessibleServerIds: serverMocks.noop, + getApplicationStats: serverMocks.noop, + getComposeContainer: serverMocks.noop, + getContainerLogs: serverMocks.getContainerLogs, + getContainersByAppNameMatch: serverMocks.getContainersByAppNameMatch, + getMountPath: serverMocks.noop, + getServiceContainerCommand: serverMocks.noop, + getWebServerSettings: serverMocks.noop, + loadServices: serverMocks.noop, + mechanizeDockerContainer: serverMocks.noop, + randomizeComposeFile: serverMocks.noop, + randomizeIsolatedDeploymentComposeFile: serverMocks.noop, + readConfig: serverMocks.noop, + readRemoteConfig: serverMocks.noop, + rebuildDatabase: serverMocks.noop, + removeCompose: serverMocks.noop, + removeComposeDirectory: serverMocks.noop, + removeDeployments: serverMocks.noop, + removeDeploymentsByComposeId: serverMocks.noop, + removeDirectoryCode: serverMocks.noop, + removeDomainById: serverMocks.noop, + removeLibsqlById: serverMocks.noop, + removeMariadbById: serverMocks.noop, + removeMongoById: serverMocks.noop, + removeMonitoringDirectory: serverMocks.noop, + removeMySqlById: serverMocks.noop, + removePostgresById: serverMocks.noop, + removeRedisById: serverMocks.noop, + removeService: serverMocks.noop, + removeTraefikConfig: serverMocks.noop, + startCompose: serverMocks.noop, + startService: serverMocks.noop, + startServiceRemote: serverMocks.noop, + stopCompose: serverMocks.noop, + stopService: serverMocks.noop, + stopServiceRemote: serverMocks.noop, + unzipDrop: serverMocks.noop, + updateApplication: serverMocks.noop, + updateApplicationStatus: serverMocks.noop, + updateCompose: serverMocks.noop, + updateDeploymentStatus: serverMocks.noop, + updateLibsqlById: serverMocks.noop, + updateMariadbById: serverMocks.noop, + updateMongoById: serverMocks.noop, + updateMySqlById: serverMocks.noop, + updatePostgresById: serverMocks.noop, + updateRedisById: serverMocks.noop, + upsertApplicationEnvironment: serverMocks.noop, + writeConfig: serverMocks.noop, + writeConfigRemote: serverMocks.noop, +})); + +vi.mock("@dokploy/server/db", () => ({ + db: {}, +})); + +vi.mock("@dokploy/server/services/git-provider", () => ({ + canEditDeployGitSource: vi.fn(), + redactGitProviderSecrets: vi.fn((value) => value), +})); + +vi.mock("@dokploy/server/services/permission", () => ({ + addNewService: vi.fn(), + checkPermission: vi.fn(), + checkServiceAccess: permissionMocks.checkServiceAccess, + checkServicePermissionAndAccess: + permissionMocks.checkServicePermissionAndAccess, + findMemberByUserId: vi.fn(), + hasPermission: vi.fn(), +})); + +vi.mock("@dokploy/server/templates/github", () => ({ + fetchTemplateFiles: vi.fn(), + fetchTemplatesList: vi.fn(), +})); + +vi.mock("@dokploy/server/templates/processors", () => ({ + processTemplate: vi.fn(), +})); + +vi.mock("@/server/api/utils/audit", () => ({ + audit: vi.fn(), +})); + +vi.mock("@/server/db", () => ({ + db: {}, +})); + +vi.mock("@/server/queues/queueSetup", () => ({ + cleanQueuesByApplication: vi.fn(), + cleanQueuesByCompose: vi.fn(), + killDockerBuild: vi.fn(), + myQueue: { + add: vi.fn(), + getJob: vi.fn(), + remove: vi.fn(), + }, +})); + +vi.mock("@/server/utils/deploy", () => ({ + cancelDeployment: vi.fn(), + deploy: vi.fn(), +})); + +const { applicationRouter } = await import( + "../../server/api/routers/application" +); +const { composeRouter } = await import("../../server/api/routers/compose"); +const { libsqlRouter } = await import("../../server/api/routers/libsql"); +const { mariadbRouter } = await import("../../server/api/routers/mariadb"); +const { mongoRouter } = await import("../../server/api/routers/mongo"); +const { mysqlRouter } = await import("../../server/api/routers/mysql"); +const { postgresRouter } = await import("../../server/api/routers/postgres"); +const { redisRouter } = await import("../../server/api/routers/redis"); + +const createContext = () => + ({ + db: {}, + req: {}, + res: {}, + session: { + userId: "user-1", + activeOrganizationId: "org-1", + }, + user: { + id: "user-1", + role: "member", + }, + }) as never; + +const serviceRecord = (idField: string, id: string) => ({ + [idField]: id, + appName: `${id}-app`, + composeType: "docker-compose", + serverId: null, + environment: { + project: { + organizationId: "org-1", + }, + }, +}); + +type ReadLogsCaller = { + readLogs: (input: Record) => Promise; +}; + +const cases = [ + { + name: "application", + id: "app-1", + idField: "applicationId", + find: serverMocks.findApplicationById, + caller: () => + applicationRouter.createCaller( + createContext(), + ) as unknown as ReadLogsCaller, + }, + { + name: "compose", + id: "compose-1", + idField: "composeId", + find: serverMocks.findComposeById, + caller: () => + composeRouter.createCaller(createContext()) as unknown as ReadLogsCaller, + extraInput: { containerId: "compose-container-1" }, + }, + { + name: "postgres", + id: "postgres-1", + idField: "postgresId", + find: serverMocks.findPostgresById, + caller: () => + postgresRouter.createCaller(createContext()) as unknown as ReadLogsCaller, + }, + { + name: "mysql", + id: "mysql-1", + idField: "mysqlId", + find: serverMocks.findMySqlById, + caller: () => + mysqlRouter.createCaller(createContext()) as unknown as ReadLogsCaller, + }, + { + name: "mariadb", + id: "mariadb-1", + idField: "mariadbId", + find: serverMocks.findMariadbById, + caller: () => + mariadbRouter.createCaller(createContext()) as unknown as ReadLogsCaller, + }, + { + name: "mongo", + id: "mongo-1", + idField: "mongoId", + find: serverMocks.findMongoById, + caller: () => + mongoRouter.createCaller(createContext()) as unknown as ReadLogsCaller, + }, + { + name: "redis", + id: "redis-1", + idField: "redisId", + find: serverMocks.findRedisById, + caller: () => + redisRouter.createCaller(createContext()) as unknown as ReadLogsCaller, + }, + { + name: "libsql", + id: "libsql-1", + idField: "libsqlId", + find: serverMocks.findLibsqlById, + caller: () => + libsqlRouter.createCaller(createContext()) as unknown as ReadLogsCaller, + }, +]; + +const roleWithoutLogs = ac.newRole({ service: ["read"], logs: [] }); + +describe("service readLogs permission boundary", () => { + beforeEach(() => { + vi.clearAllMocks(); + permissionMocks.activeRole.current = null; + const authorizeWithActiveRole = async ( + _ctx: unknown, + _serviceId: string, + permissions: Record, + ) => { + const result = permissionMocks.activeRole.current?.authorize( + permissions as never, + ) as { success: boolean } | undefined; + if (!result?.success) { + throw new TRPCError({ code: "UNAUTHORIZED" }); + } + }; + permissionMocks.checkServiceAccess.mockImplementation( + (ctx: unknown, serviceId: string, action = "read") => + authorizeWithActiveRole(ctx, serviceId, { service: [action] }), + ); + permissionMocks.checkServicePermissionAndAccess.mockImplementation( + authorizeWithActiveRole, + ); + serverMocks.getContainerLogs.mockResolvedValue("log line"); + serverMocks.getContainersByAppNameMatch.mockResolvedValue([ + { containerId: "compose-container-1" }, + ]); + for (const testCase of cases) { + testCase.find.mockResolvedValue( + serviceRecord(testCase.idField, testCase.id), + ); + } + }); + + it.each(cases)( + "$name readLogs requires logs.read and keeps member access", + async (testCase) => { + const input = { + [testCase.idField]: testCase.id, + ...(testCase.extraInput ?? {}), + }; + + permissionMocks.activeRole.current = roleWithoutLogs; + await expect(testCase.caller().readLogs(input)).rejects.toMatchObject({ + code: "UNAUTHORIZED", + }); + expect(serverMocks.getContainerLogs).not.toHaveBeenCalled(); + + permissionMocks.activeRole.current = memberRole; + await expect(testCase.caller().readLogs(input)).resolves.toBe("log line"); + expect( + permissionMocks.checkServicePermissionAndAccess, + ).toHaveBeenLastCalledWith(expect.anything(), testCase.id, { + service: ["read"], + logs: ["read"], + }); + }, + ); +}); diff --git a/apps/dokploy/server/api/routers/application.ts b/apps/dokploy/server/api/routers/application.ts index 4db0c8e8f0a..cc5c03b90eb 100644 --- a/apps/dokploy/server/api/routers/application.ts +++ b/apps/dokploy/server/api/routers/application.ts @@ -1579,7 +1579,10 @@ export const applicationRouter = createTRPCRouter({ }), ) .query(async ({ input, ctx }) => { - await checkServiceAccess(ctx, input.applicationId, "read"); + await checkServicePermissionAndAccess(ctx, input.applicationId, { + service: ["read"], + logs: ["read"], + }); const application = await findApplicationById(input.applicationId); if ( application.environment.project.organizationId !== diff --git a/apps/dokploy/server/api/routers/compose.ts b/apps/dokploy/server/api/routers/compose.ts index 1669b4f3652..bebebbf7e79 100644 --- a/apps/dokploy/server/api/routers/compose.ts +++ b/apps/dokploy/server/api/routers/compose.ts @@ -1552,7 +1552,10 @@ export const composeRouter = createTRPCRouter({ }), ) .query(async ({ input, ctx }) => { - await checkServiceAccess(ctx, input.composeId, "read"); + await checkServicePermissionAndAccess(ctx, input.composeId, { + service: ["read"], + logs: ["read"], + }); const compose = await findComposeById(input.composeId); if ( compose.environment.project.organizationId !== diff --git a/apps/dokploy/server/api/routers/libsql.ts b/apps/dokploy/server/api/routers/libsql.ts index 1b41b017bd3..1c54d717a96 100644 --- a/apps/dokploy/server/api/routers/libsql.ts +++ b/apps/dokploy/server/api/routers/libsql.ts @@ -531,7 +531,10 @@ export const libsqlRouter = createTRPCRouter({ }), ) .query(async ({ input, ctx }) => { - await checkServiceAccess(ctx, input.libsqlId, "read"); + await checkServicePermissionAndAccess(ctx, input.libsqlId, { + service: ["read"], + logs: ["read"], + }); const libsql = await findLibsqlById(input.libsqlId); if ( libsql.environment.project.organizationId !== diff --git a/apps/dokploy/server/api/routers/mariadb.ts b/apps/dokploy/server/api/routers/mariadb.ts index ada5a8008d5..907946d25d3 100644 --- a/apps/dokploy/server/api/routers/mariadb.ts +++ b/apps/dokploy/server/api/routers/mariadb.ts @@ -673,7 +673,10 @@ export const mariadbRouter = createTRPCRouter({ }), ) .query(async ({ input, ctx }) => { - await checkServiceAccess(ctx, input.mariadbId, "read"); + await checkServicePermissionAndAccess(ctx, input.mariadbId, { + service: ["read"], + logs: ["read"], + }); const mariadb = await findMariadbById(input.mariadbId); if ( mariadb.environment.project.organizationId !== diff --git a/apps/dokploy/server/api/routers/mongo.ts b/apps/dokploy/server/api/routers/mongo.ts index 87f821127ef..4d516cb4ae9 100644 --- a/apps/dokploy/server/api/routers/mongo.ts +++ b/apps/dokploy/server/api/routers/mongo.ts @@ -673,7 +673,10 @@ export const mongoRouter = createTRPCRouter({ }), ) .query(async ({ input, ctx }) => { - await checkServiceAccess(ctx, input.mongoId, "read"); + await checkServicePermissionAndAccess(ctx, input.mongoId, { + service: ["read"], + logs: ["read"], + }); const mongo = await findMongoById(input.mongoId); if ( mongo.environment.project.organizationId !== diff --git a/apps/dokploy/server/api/routers/mysql.ts b/apps/dokploy/server/api/routers/mysql.ts index df2c93d6d7d..5af046aab90 100644 --- a/apps/dokploy/server/api/routers/mysql.ts +++ b/apps/dokploy/server/api/routers/mysql.ts @@ -687,7 +687,10 @@ export const mysqlRouter = createTRPCRouter({ }), ) .query(async ({ input, ctx }) => { - await checkServiceAccess(ctx, input.mysqlId, "read"); + await checkServicePermissionAndAccess(ctx, input.mysqlId, { + service: ["read"], + logs: ["read"], + }); const mysql = await findMySqlById(input.mysqlId); if ( mysql.environment.project.organizationId !== diff --git a/apps/dokploy/server/api/routers/postgres.ts b/apps/dokploy/server/api/routers/postgres.ts index 31bc6549245..0e67575114c 100644 --- a/apps/dokploy/server/api/routers/postgres.ts +++ b/apps/dokploy/server/api/routers/postgres.ts @@ -685,7 +685,10 @@ export const postgresRouter = createTRPCRouter({ }), ) .query(async ({ input, ctx }) => { - await checkServiceAccess(ctx, input.postgresId, "read"); + await checkServicePermissionAndAccess(ctx, input.postgresId, { + service: ["read"], + logs: ["read"], + }); const postgres = await findPostgresById(input.postgresId); if ( postgres.environment.project.organizationId !== diff --git a/apps/dokploy/server/api/routers/redis.ts b/apps/dokploy/server/api/routers/redis.ts index 99059bbf5cf..a1823cadc7c 100644 --- a/apps/dokploy/server/api/routers/redis.ts +++ b/apps/dokploy/server/api/routers/redis.ts @@ -658,7 +658,10 @@ export const redisRouter = createTRPCRouter({ }), ) .query(async ({ input, ctx }) => { - await checkServiceAccess(ctx, input.redisId, "read"); + await checkServicePermissionAndAccess(ctx, input.redisId, { + service: ["read"], + logs: ["read"], + }); const redis = await findRedisById(input.redisId); if ( redis.environment.project.organizationId !== From b7913b8de0f38c85a28417ee4c8d789b09c3b553 Mon Sep 17 00:00:00 2001 From: merge-check <140916359+agentHits@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:47:28 -0400 Subject: [PATCH 3/3] fix: keep traefik tls secrets out of the traefik file editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Что: - resolveTraefikConfigPath отклоняет acme.json, файлы *.key и все внутри dynamic/certificates, поэтому settings.readTraefikFile и settings.updateTraefikFile не могут читать или перезаписывать их ни локально, ни на удаленном сервере. - settings.readDirectories скрывает эти файлы и папку certificates из дерева. - readDirectories, readTraefikFile и updateTraefikFile без serverId требуют доступ к локальному хосту (owner/admin, assertLocalHostAccess), как другие процедуры локального хоста. Зачем: - Пользователь с traefikFiles.read мог прочитать acme.json и приватные ключи сертификатов, а без serverId не было проверки роли для локального хоста. Риски: - Эти файлы больше нельзя открыть или изменить в редакторе Traefik даже owner/admin; сертификаты по-прежнему управляются через раздел сертификатов. - Кастомные роли с traefikFiles.* теряют доступ к файлам Traefik локального хоста; удаленные серверы работают как раньше. Проверки: - Команды и результаты: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/traefik/traefik-file-boundary.test.ts __test__/settings/docker-server-boundary.test.ts - все тесты прошли; новые тесты падают на старом коде (исходники временно возвращены к origin/AgentHits-Dev); bun run typecheck - exit 0; bunx biome check по измененным файлам - без ошибок. - Ограничения: симлинки внутри папки Traefik не разрешаются (проверка идет по пути); на реальном сервере не проверялось. What: - resolveTraefikConfigPath rejects acme.json, *.key files and anything under dynamic/certificates, so settings.readTraefikFile and settings.updateTraefikFile can no longer read or overwrite them, locally or on a remote server. - settings.readDirectories hides these files and the certificates directory from the tree. - readDirectories, readTraefikFile and updateTraefikFile without a serverId require local host access (owner/admin, assertLocalHostAccess), like the other local host procedures. Why: - A traefikFiles.read holder could read acme.json and certificate private keys, and without a serverId there was no role check for the local host. Risks: - These files can no longer be opened or edited in the Traefik editor, even by owners/admins; certificates are still managed through the certificates section. - Custom roles with traefikFiles.* lose access to local host Traefik files; remote servers work as before. Checks: - Commands and results: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/traefik/traefik-file-boundary.test.ts __test__/settings/docker-server-boundary.test.ts - all passed; the new tests fail on the old code (sources temporarily reset to origin/AgentHits-Dev); bun run typecheck - exit 0; bunx biome check on changed files - no errors. - Limitations: symlinks inside the Traefik directory are not resolved (the check is path based); not checked on a real server. Co-Authored-By: Claude Opus 5.5 --- .../settings/docker-server-boundary.test.ts | 73 ++++++++++++++++ .../traefik/traefik-file-boundary.test.ts | 87 ++++++++++++++++++- apps/dokploy/server/api/routers/settings.ts | 28 +++++- .../server/src/utils/traefik/application.ts | 40 +++++++++ 4 files changed, 222 insertions(+), 6 deletions(-) diff --git a/apps/dokploy/__test__/settings/docker-server-boundary.test.ts b/apps/dokploy/__test__/settings/docker-server-boundary.test.ts index f83350b3e22..f990a92be4e 100644 --- a/apps/dokploy/__test__/settings/docker-server-boundary.test.ts +++ b/apps/dokploy/__test__/settings/docker-server-boundary.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ + assertLocalHostAccess: vi.fn(), audit: vi.fn(), checkGPUStatus: vi.fn(), checkPermission: vi.fn(), @@ -17,6 +18,7 @@ const mocks = vi.hoisted(() => ({ cleanupSystem: vi.fn(), cleanupVolumes: vi.fn(), execAsync: vi.fn(), + filterProtectedTraefikEntries: vi.fn(), findServerById: vi.fn(), generateOpenApiDocument: vi.fn(), getAccessibleServerIds: vi.fn(), @@ -78,6 +80,7 @@ vi.mock("@dokploy/server", () => ({ cleanupSystem: mocks.cleanupSystem, cleanupVolumes: mocks.cleanupVolumes, execAsync: mocks.execAsync, + filterProtectedTraefikEntries: mocks.filterProtectedTraefikEntries, findServerById: mocks.findServerById, getAccessibleServerIds: mocks.getAccessibleServerIds, getAgentHitsUpdateCommand: mocks.getAgentHitsUpdateCommand, @@ -141,6 +144,10 @@ vi.mock("@/server/api/utils/audit", () => ({ audit: mocks.audit, })); +vi.mock("@/server/api/utils/local-host-access", () => ({ + assertLocalHostAccess: mocks.assertLocalHostAccess, +})); + vi.mock("@/server/queues/concurrency", () => ({ assertBuildsConcurrencyAllowed: vi.fn(), })); @@ -493,3 +500,69 @@ describe("settings Docker server boundary", () => { }); }); }); + +describe("settings Traefik file access", () => { + const localTraefikPath = "/etc/dokploy/traefik"; + + beforeEach(() => { + vi.clearAllMocks(); + mocks.checkPermission.mockResolvedValue(undefined); + mocks.assertLocalHostAccess.mockResolvedValue(undefined); + mocks.getAccessibleServerIds.mockResolvedValue(new Set(["server-1"])); + mocks.paths.mockReturnValue({ + MAIN_TRAEFIK_PATH: localTraefikPath, + }); + mocks.readConfigInPath.mockResolvedValue("http: {}"); + mocks.readDirectory.mockResolvedValue([]); + mocks.filterProtectedTraefikEntries.mockImplementation( + (entries: unknown[]) => entries, + ); + }); + + it("requires local host access for local Traefik files", async () => { + mocks.assertLocalHostAccess.mockRejectedValue( + Object.assign(new Error("Local host operations require owner or admin"), { + code: "UNAUTHORIZED", + }), + ); + const caller = createCaller("member"); + + await expect(caller.readDirectories({})).rejects.toThrow( + "Local host operations require owner or admin", + ); + await expect( + caller.readTraefikFile({ + path: `${process.cwd()}/.docker/traefik/dynamic/app.yml`, + }), + ).rejects.toThrow("Local host operations require owner or admin"); + await expect( + caller.updateTraefikFile({ + path: `${localTraefikPath}/dynamic/app.yml`, + traefikConfig: "http: {}", + }), + ).rejects.toThrow("Local host operations require owner or admin"); + + expect(mocks.assertLocalHostAccess).toHaveBeenCalledTimes(3); + expect(mocks.readDirectory).not.toHaveBeenCalled(); + expect(mocks.readConfigInPath).not.toHaveBeenCalled(); + expect(mocks.writeTraefikConfigInPath).not.toHaveBeenCalled(); + }); + + it("hides protected entries from remote Traefik directory listings", async () => { + const listing = [ + { id: `${localTraefikPath}/dynamic/acme.json`, name: "acme.json" }, + ]; + mocks.readDirectory.mockResolvedValue(listing); + mocks.filterProtectedTraefikEntries.mockReturnValue([]); + + await expect( + createCaller().readDirectories({ serverId: "server-1" }), + ).resolves.toEqual([]); + + expect(mocks.filterProtectedTraefikEntries).toHaveBeenCalledWith( + listing, + "server-1", + ); + expect(mocks.assertLocalHostAccess).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/dokploy/__test__/traefik/traefik-file-boundary.test.ts b/apps/dokploy/__test__/traefik/traefik-file-boundary.test.ts index 4abc537497a..1be8c13879b 100644 --- a/apps/dokploy/__test__/traefik/traefik-file-boundary.test.ts +++ b/apps/dokploy/__test__/traefik/traefik-file-boundary.test.ts @@ -17,13 +17,20 @@ vi.mock("@dokploy/server/utils/process/execAsync", () => ({ writeFileRemote: mocks.writeFileRemote, })); -const { readConfigInPath, writeTraefikConfigInPath, writeTraefikConfigRemote } = - await import("@dokploy/server/utils/traefik/application"); +const { + filterProtectedTraefikEntries, + readConfigInPath, + writeTraefikConfigInPath, + writeTraefikConfigRemote, +} = await import("@dokploy/server/utils/traefik/application"); describe("Traefik file path boundary", () => { beforeEach(() => { vi.clearAllMocks(); mocks.paths.mockImplementation((remote?: boolean) => ({ + CERTIFICATES_PATH: remote + ? "/etc/dokploy/traefik/dynamic/certificates" + : "/var/lib/dokploy/traefik/dynamic/certificates", DYNAMIC_TRAEFIK_PATH: remote ? "/etc/dokploy/traefik/dynamic" : "/var/lib/dokploy/traefik/dynamic", @@ -69,6 +76,82 @@ describe("Traefik file path boundary", () => { expect(mocks.execAsyncRemote).not.toHaveBeenCalled(); }); + it.each([ + ["/var/lib/dokploy/traefik/dynamic/acme.json", undefined], + ["/var/lib/dokploy/traefik/dynamic/certificates", undefined], + [ + "/var/lib/dokploy/traefik/dynamic/certificates/cert-1/chain.crt", + undefined, + ], + ["/var/lib/dokploy/traefik/dynamic/custom/privkey.key", undefined], + [ + "/var/lib/dokploy/traefik/dynamic/./certificates/../certificates/cert-1/privkey.key", + undefined, + ], + ["/etc/dokploy/traefik/dynamic/acme.json", "server-1"], + ["/etc/dokploy/traefik/dynamic/certificates/cert-1/chain.crt", "server-1"], + ["/etc/dokploy/traefik/tls/site.KEY", "server-1"], + ])( + "rejects reads and writes of TLS secret file %s", + async (filePath, serverId) => { + await expect(readConfigInPath(filePath, serverId)).rejects.toThrow( + "Access to this Traefik file is not allowed", + ); + await expect( + writeTraefikConfigInPath(filePath, "{}", serverId), + ).rejects.toThrow("Access to this Traefik file is not allowed"); + + expect(mocks.execAsyncRemote).not.toHaveBeenCalled(); + expect(mocks.writeFileRemote).not.toHaveBeenCalled(); + }, + ); + + it("hides TLS secret files from Traefik directory listings", () => { + const root = "/etc/dokploy/traefik"; + const listing = [ + { id: `${root}/traefik.yml`, name: "traefik.yml", type: "file" }, + { + id: `${root}/dynamic`, + name: "dynamic", + type: "directory", + children: [ + { id: `${root}/dynamic/acme.json`, name: "acme.json", type: "file" }, + { id: `${root}/dynamic/app.yml`, name: "app.yml", type: "file" }, + { + id: `${root}/dynamic/certificates`, + name: "certificates", + type: "directory", + children: [ + { + id: `${root}/dynamic/certificates/cert-1`, + name: "cert-1", + type: "directory", + children: [], + }, + ], + }, + { + id: `${root}/dynamic/custom.key`, + name: "custom.key", + type: "file", + }, + ], + }, + ]; + + expect(filterProtectedTraefikEntries(listing, "server-1")).toEqual([ + { id: `${root}/traefik.yml`, name: "traefik.yml", type: "file" }, + { + id: `${root}/dynamic`, + name: "dynamic", + type: "directory", + children: [ + { id: `${root}/dynamic/app.yml`, name: "app.yml", type: "file" }, + ], + }, + ]); + }); + it("quotes remote Traefik file paths before shell execution", async () => { const remotePath = "/etc/dokploy/traefik/dynamic/app'$(id).yml"; diff --git a/apps/dokploy/server/api/routers/settings.ts b/apps/dokploy/server/api/routers/settings.ts index 9ecdb933a48..2e14e12ecef 100644 --- a/apps/dokploy/server/api/routers/settings.ts +++ b/apps/dokploy/server/api/routers/settings.ts @@ -12,6 +12,7 @@ import { cleanupSystem, cleanupVolumes, DEFAULT_UPDATE_DATA, + filterProtectedTraefikEntries, findServerById, getAccessibleServerIds, getAgentHitsUpdateCommand, @@ -60,6 +61,7 @@ import { scheduledJobs, scheduleJob } from "node-schedule"; import { parse, stringify } from "yaml"; import { z } from "zod"; import { audit } from "@/server/api/utils/audit"; +import { assertLocalHostAccess } from "@/server/api/utils/local-host-access"; import { apiAssignDomain, apiEnableDashboard, @@ -108,6 +110,24 @@ const assertSettingsServerAccess = async ( } }; +const assertTraefikFilesHostAccess = async ( + ctx: { + user: { id: string }; + session: { + userId: string; + activeOrganizationId: string; + }; + }, + serverId?: string, +) => { + if (!serverId) { + await assertLocalHostAccess(ctx); + return; + } + + await assertSettingsServerAccess(ctx, serverId); +}; + export const settingsRouter = createTRPCRouter({ getWebServerSettings: protectedProcedure.query(async () => { if (IS_CLOUD) { @@ -657,10 +677,10 @@ export const settingsRouter = createTRPCRouter({ .query(async ({ ctx, input }) => { try { await checkPermission(ctx, { traefikFiles: ["read"] }); - await assertSettingsServerAccess(ctx, input?.serverId); + await assertTraefikFilesHostAccess(ctx, input?.serverId); const { MAIN_TRAEFIK_PATH } = paths(!!input?.serverId); const result = await readDirectory(MAIN_TRAEFIK_PATH, input?.serverId); - return result || []; + return filterProtectedTraefikEntries(result || [], input?.serverId); } catch (error) { throw error; } @@ -670,7 +690,7 @@ export const settingsRouter = createTRPCRouter({ .input(apiModifyTraefikConfig) .mutation(async ({ input, ctx }) => { await checkPermission(ctx, { traefikFiles: ["write"] }); - await assertSettingsServerAccess(ctx, input?.serverId); + await assertTraefikFilesHostAccess(ctx, input?.serverId); await writeTraefikConfigInPath( input.path, input.traefikConfig, @@ -688,7 +708,7 @@ export const settingsRouter = createTRPCRouter({ .input(apiReadTraefikConfig) .query(async ({ input, ctx }) => { await checkPermission(ctx, { traefikFiles: ["read"] }); - await assertSettingsServerAccess(ctx, input.serverId); + await assertTraefikFilesHostAccess(ctx, input.serverId); return readConfigInPath(input.path, input.serverId); }), diff --git a/packages/server/src/utils/traefik/application.ts b/packages/server/src/utils/traefik/application.ts index 5f870dc1d26..ec05c0bb41c 100644 --- a/packages/server/src/utils/traefik/application.ts +++ b/packages/server/src/utils/traefik/application.ts @@ -196,6 +196,42 @@ export const readMonitoringConfig = async (readAll = false) => { return null; }; +// ACME storage and uploaded certificates hold TLS private keys, so they stay +// out of the generic Traefik file editor. +export const isProtectedTraefikPath = ( + configPath: string, + serverId?: string, +) => { + const { CERTIFICATES_PATH } = paths(!!serverId); + const certificatesPath = path.resolve(CERTIFICATES_PATH); + const resolvedPath = path.resolve(configPath); + const fileName = path.basename(resolvedPath).toLowerCase(); + + return ( + fileName === "acme.json" || + fileName.endsWith(".key") || + resolvedPath === certificatesPath || + resolvedPath.startsWith(`${certificatesPath}${path.sep}`) + ); +}; + +export const filterProtectedTraefikEntries = < + T extends { id: string; children?: T[] }, +>( + entries: T[], + serverId?: string, +): T[] => + entries + .filter((entry) => !isProtectedTraefikPath(entry.id, serverId)) + .map((entry) => + entry.children + ? { + ...entry, + children: filterProtectedTraefikEntries(entry.children, serverId), + } + : entry, + ); + export const resolveTraefikConfigPath = ( pathFile: string, serverId?: string, @@ -219,6 +255,10 @@ export const resolveTraefikConfigPath = ( throw new Error("Invalid Traefik config path"); } + if (isProtectedTraefikPath(configPath, serverId)) { + throw new Error("Access to this Traefik file is not allowed"); + } + return configPath; };