From c264a7cebece74e2872cd28a7349f5bd5b591bb9 Mon Sep 17 00:00:00 2001 From: merge-check <140916359+agentHits@users.noreply.github.com> Date: Tue, 27 Oct 2026 19:57:48 -0400 Subject: [PATCH] fix(monitoring): show stats for compose and stack containers The Monitoring tab of a compose or stack service stayed empty: CPU, memory, block and network I/O were all zero and no error was shown. The tab asks for stats of one container, named after the container ("--1" for compose, "_.1." for a stack), not after the Dokploy service. The fork's metrics access check looked that name up as a service appName, found nothing and refused, so the stats socket closed at once without a reason and readAppMonitoring returned UNAUTHORIZED. Applications and databases were not affected, because their container is named after the service. When the name matches no service and no metrics server is involved, the check now reads the local container's owner labels (compose project, stack namespace, swarm service), takes the service they point to and applies the same organization and monitoring-read checks to it. Only an exact container name counts (docker inspect also resolves ID prefixes), and only services on this host, since only local containers are seen. The stats socket now closes with 4003 "Not authorized" when access is refused, so the page shows why instead of staying blank. Co-Authored-By: Claude Opus 5.5 --- .../container-metrics-access.test.ts | 220 ++++++++++++++++++ .../dokploy/__test__/wss/docker-stats.test.ts | 25 ++ .../server/api/utils/monitoring-access.ts | 63 ++++- apps/dokploy/server/wss/docker-stats.ts | 2 +- 4 files changed, 308 insertions(+), 2 deletions(-) create mode 100644 apps/dokploy/__test__/monitoring/container-metrics-access.test.ts diff --git a/apps/dokploy/__test__/monitoring/container-metrics-access.test.ts b/apps/dokploy/__test__/monitoring/container-metrics-access.test.ts new file mode 100644 index 00000000000..640ff59ca1b --- /dev/null +++ b/apps/dokploy/__test__/monitoring/container-metrics-access.test.ts @@ -0,0 +1,220 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +type Service = { + appName: string; + serverId: string | null; + organizationId: string; + id: string; +}; + +const mocks = vi.hoisted(() => { + const services = new Map>(); + const idColumns: Record = { + applications: "applicationId", + compose: "composeId", + postgres: "postgresId", + mysql: "mysqlId", + mariadb: "mariadbId", + mongo: "mongoId", + redis: "redisId", + libsql: "libsqlId", + previewDeployments: "previewDeploymentId", + }; + const findFirst = + (table: string) => + async ({ where }: { where: { value: string } }) => { + const service = services.get(table)?.get(where.value); + if (!service) return undefined; + return { + [idColumns[table] as string]: service.id, + appName: service.appName, + serverId: service.serverId, + environment: { + project: { organizationId: service.organizationId }, + }, + }; + }; + return { + services, + query: Object.fromEntries( + Object.keys(idColumns).map((table) => [ + table, + { findFirst: findFirst(table) }, + ]), + ), + execAsync: vi.fn(), + checkServicePermissionAndAccess: vi.fn(), + }; +}); + +vi.mock("drizzle-orm", async (importOriginal) => ({ + ...(await importOriginal()), + eq: (_column: unknown, value: unknown) => ({ value }), +})); +vi.mock("@dokploy/server/db", () => ({ + db: { query: mocks.query }, +})); +vi.mock("@dokploy/server/services/permission", () => ({ + checkServicePermissionAndAccess: mocks.checkServicePermissionAndAccess, +})); +vi.mock("@dokploy/server/utils/process/execAsync", () => ({ + execAsync: mocks.execAsync, +})); + +import { assertContainerMetricsServiceAccess } from "@/server/api/utils/monitoring-access"; + +const ctx = { + user: { id: "user-1" }, + session: { activeOrganizationId: "org-1" }, +}; + +const addService = (table: string, service: Service) => { + const rows = mocks.services.get(table) ?? new Map(); + rows.set(service.appName, service); + mocks.services.set(table, rows); +}; + +const inspectResult = (name: string, labels: Record | null) => + mocks.execAsync.mockResolvedValue({ + stdout: `${JSON.stringify({ name, labels })}\n`, + stderr: "", + }); + +beforeEach(() => { + vi.clearAllMocks(); + mocks.services.clear(); + mocks.checkServicePermissionAndAccess.mockResolvedValue(undefined); + addService("compose", { + id: "compose-1", + appName: "shop-abc123", + serverId: null, + organizationId: "org-1", + }); +}); + +describe("assertContainerMetricsServiceAccess", () => { + it("checks access to the compose service that owns a container", async () => { + inspectResult("/shop-abc123-worker-1", { + "com.docker.compose.project": "shop-abc123", + "com.docker.compose.service": "worker", + }); + + await assertContainerMetricsServiceAccess(ctx, "shop-abc123-worker-1"); + + expect(mocks.execAsync).toHaveBeenCalledWith( + expect.stringContaining("shop-abc123-worker-1"), + ); + expect(mocks.checkServicePermissionAndAccess).toHaveBeenCalledWith( + ctx, + "compose-1", + { monitoring: ["read"] }, + ); + }); + + it("checks access to the stack that owns a task container", async () => { + addService("compose", { + id: "stack-1", + appName: "blog-def456", + serverId: null, + organizationId: "org-1", + }); + inspectResult("/blog-def456_web.1.k2j3h4", { + "com.docker.stack.namespace": "blog-def456", + "com.docker.swarm.service.name": "blog-def456_web", + }); + + await assertContainerMetricsServiceAccess(ctx, "blog-def456_web.1.k2j3h4"); + + expect(mocks.checkServicePermissionAndAccess).toHaveBeenCalledWith( + ctx, + "stack-1", + { monitoring: ["read"] }, + ); + }); + + it("does not inspect containers when the name is a service", async () => { + await assertContainerMetricsServiceAccess(ctx, "shop-abc123"); + + expect(mocks.execAsync).not.toHaveBeenCalled(); + expect(mocks.checkServicePermissionAndAccess).toHaveBeenCalledWith( + ctx, + "compose-1", + { monitoring: ["read"] }, + ); + }); + + it("denies a container resolved by an ID prefix instead of its name", async () => { + inspectResult("/shop-abc123-worker-1", { + "com.docker.compose.project": "shop-abc123", + }); + + await expect( + assertContainerMetricsServiceAccess(ctx, "3f9a1c"), + ).rejects.toMatchObject({ code: "UNAUTHORIZED" }); + expect(mocks.checkServicePermissionAndAccess).not.toHaveBeenCalled(); + }); + + it("denies a container whose service is in another organization", async () => { + addService("compose", { + id: "compose-2", + appName: "other-xyz789", + serverId: null, + organizationId: "org-2", + }); + inspectResult("/other-xyz789-db-1", { + "com.docker.compose.project": "other-xyz789", + }); + + await expect( + assertContainerMetricsServiceAccess(ctx, "other-xyz789-db-1"), + ).rejects.toMatchObject({ code: "UNAUTHORIZED" }); + expect(mocks.checkServicePermissionAndAccess).not.toHaveBeenCalled(); + }); + + it("ignores labels that point at a service on a remote server", async () => { + addService("compose", { + id: "compose-3", + appName: "remote-qwe321", + serverId: "server-1", + organizationId: "org-1", + }); + inspectResult("/remote-qwe321-api-1", { + "com.docker.compose.project": "remote-qwe321", + }); + + await expect( + assertContainerMetricsServiceAccess(ctx, "remote-qwe321-api-1"), + ).rejects.toMatchObject({ code: "UNAUTHORIZED" }); + }); + + it("denies containers without labels or when docker fails", async () => { + inspectResult("/loose-container", null); + await expect( + assertContainerMetricsServiceAccess(ctx, "loose-container"), + ).rejects.toMatchObject({ code: "UNAUTHORIZED" }); + + mocks.execAsync.mockRejectedValue(new Error("No such container")); + await expect( + assertContainerMetricsServiceAccess(ctx, "missing-container"), + ).rejects.toMatchObject({ code: "UNAUTHORIZED" }); + expect(mocks.checkServicePermissionAndAccess).not.toHaveBeenCalled(); + }); + + it("does not resolve containers for a metrics server request", async () => { + await expect( + assertContainerMetricsServiceAccess( + ctx, + "shop-abc123-worker-1", + "server-1", + ), + ).rejects.toMatchObject({ code: "UNAUTHORIZED" }); + expect(mocks.execAsync).not.toHaveBeenCalled(); + }); + + it("rejects names that are not docker identifiers without running docker", async () => { + await expect( + assertContainerMetricsServiceAccess(ctx, "x; rm -rf /"), + ).rejects.toMatchObject({ code: "UNAUTHORIZED" }); + expect(mocks.execAsync).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/dokploy/__test__/wss/docker-stats.test.ts b/apps/dokploy/__test__/wss/docker-stats.test.ts index 65617e07d92..794677e7ed5 100644 --- a/apps/dokploy/__test__/wss/docker-stats.test.ts +++ b/apps/dokploy/__test__/wss/docker-stats.test.ts @@ -27,6 +27,10 @@ vi.mock("@/server/wss/authorize", () => ({ vi.mock("@/server/wss/server-permission", () => ({ canAccessMonitoringWebSocket: vi.fn(async () => true), })); +const mockAssertMetricsAccess = vi.hoisted(() => vi.fn()); +vi.mock("@/server/api/utils/monitoring-access", () => ({ + assertContainerMetricsServiceAccess: mockAssertMetricsAccess, +})); import { setupDockerStatsMonitoringSocketServer } from "@/server/wss/docker-stats"; @@ -111,4 +115,25 @@ describe("docker stats websocket (#5504)", () => { expect(mockListContainers).not.toHaveBeenCalled(); }); + + it("tells the client why it was refused instead of closing silently", async () => { + mockValidateRequest.mockResolvedValue(AUTH); + mockAssertMetricsAccess.mockRejectedValue(new Error("UNAUTHORIZED")); + const ws = new WebSocket( + url.replace( + "appName=dokploy", + "appName=shop-abc123-worker-1&appType=docker-compose", + ), + ); + + const [code, reason] = await new Promise<[number, string]>((r) => + ws.once("close", (closeCode, closeReason) => + r([closeCode, closeReason.toString()]), + ), + ); + + expect(code).toBe(4003); + expect(reason).toBe("Not authorized"); + expect(mockListContainers).not.toHaveBeenCalled(); + }); }); diff --git a/apps/dokploy/server/api/utils/monitoring-access.ts b/apps/dokploy/server/api/utils/monitoring-access.ts index a25b87c5df7..e5c9167a374 100644 --- a/apps/dokploy/server/api/utils/monitoring-access.ts +++ b/apps/dokploy/server/api/utils/monitoring-access.ts @@ -11,8 +11,10 @@ import { redis, } from "@dokploy/server/db/schema"; import { checkServicePermissionAndAccess } from "@dokploy/server/services/permission"; +import { execAsync } from "@dokploy/server/utils/process/execAsync"; import { TRPCError } from "@trpc/server"; import { eq } from "drizzle-orm"; +import { quote } from "shell-quote"; type MonitoringAccessCtx = { session: { @@ -384,6 +386,61 @@ export const findAccessibleContainerResourceStat = async < ); }; +const CONTAINER_OWNER_LABELS = [ + "com.docker.compose.project", + "com.docker.stack.namespace", + "com.docker.swarm.service.name", +]; + +// Compose and stack containers are named "--1" or +// "_.1.", not after the Dokploy service, so the monitoring +// tab asks for a container name that matches no service. The owning service is +// then taken from the local container's labels; only services on this host +// count, because the lookup only sees local containers. +const findLocalContainerMonitoringService = async (containerName: string) => { + const name = normalizeDockerLabelValue(containerName); + if (!name) { + return null; + } + + let container: { name?: string; labels?: Record | null }; + try { + const { stdout } = await execAsync( + quote([ + "docker", + "inspect", + "--type", + "container", + "--format", + '{"name":{{json .Name}},"labels":{{json .Config.Labels}}}', + name, + ]), + ); + container = JSON.parse(stdout.trim()); + } catch { + return null; + } + + // `docker inspect` also resolves ID prefixes; only an exact name counts. + if (container.name !== `/${name}`) { + return null; + } + + const labels = parseDockerLabels(container.labels); + for (const label of CONTAINER_OWNER_LABELS) { + const ownerAppName = normalizeDockerLabelValue(labels[label]); + if (!ownerAppName) { + continue; + } + const service = await findMonitoringServiceByAppName(ownerAppName); + if (service && !service.serverId) { + return service; + } + } + + return null; +}; + export const assertContainerMetricsServiceAccess = async ( ctx: MonitoringAccessCtx, appName: string, @@ -395,7 +452,11 @@ export const assertContainerMetricsServiceAccess = async ( message: "You are not authorized to access this monitored service", }); } - const service = await findMonitoringServiceByAppName(appName); + const service = + (await findMonitoringServiceByAppName(appName)) ?? + (serverId === undefined + ? await findLocalContainerMonitoringService(appName) + : null); if ( !service || service.environment?.project?.organizationId !== diff --git a/apps/dokploy/server/wss/docker-stats.ts b/apps/dokploy/server/wss/docker-stats.ts index abf3f59bfcb..f5a8e2c9346 100644 --- a/apps/dokploy/server/wss/docker-stats.ts +++ b/apps/dokploy/server/wss/docker-stats.ts @@ -136,7 +136,7 @@ export const setupDockerStatsMonitoringSocketServer = ( ); } } catch { - ws.close(); + ws.close(4003, "Not authorized"); return; }