From 5f9fec36ee364872ab2676ff737b4d2e94320bad Mon Sep 17 00:00:00 2001 From: merge-check <140916359+agentHits@users.noreply.github.com> Date: Tue, 6 Oct 2026 12:42:45 -0400 Subject: [PATCH] fix: redact compose service response secrets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Что: - Перенесен фикс 9b80df3a0 с удаленной ветки agentHits-prod (локальная копия backup/agentHits-Prod-before-delete-20261006): при синхронизации 7da5612c0 он выпал и в AgentHits-Dev не попал. - В compose router добавлен helper redactComposeSecrets для ответов create, one, update, delete, deployTemplate и move: env и секреты git provider скрываются как раньше, креды в customGitUrl теперь маскируются. - composeFile в ответах мутаций скрывается всегда, а в compose.one только для пользователей без права envVars.read: редактор compose-файла в UI берет файл из compose.one, поэтому дословный перенос сломал бы его. - Тест service-environment-reveal проверяет compose.one с правом envVars.read и без него, а также ответ update. Зачем: - compose.one, create и update отдавали customGitUrl с токеном и composeFile с inline-секретами всем, у кого есть доступ к сервису на чтение, хотя роутер environment эти поля уже скрывал. Риски: - customGitUrl с кредами в UI и API теперь показывается с placeholder вместо токена. Сохранение без правки оставляет прежний URL; чтобы сменить URL, его нужно вставить целиком, иначе сервер оставит старое значение. - Роли без envVars.read (бывают только среди custom roles) видят placeholder вместо compose-файла; getConvertedCompose (требует service.create) по-прежнему возвращает файл, это вне scope. Проверки: - Команды и результаты: `bun install --frozen-lockfile` в отдельном worktree - passed; в apps/dokploy `bun run test -- --run service-environment-reveal` - passed (1 файл / 6 тестов), контрольный прогон того же теста на compose.ts из origin/AgentHits-Dev - 3 compose-теста упали, как и ожидалось; `bun run test -- --run __test__/security/ __test__/compose/provider-status.test.ts __test__/deployment/cloud-deploy-idempotency.test.ts __test__/git-provider/deploy-source-credential-access.test.ts __test__/placement/deploy-target-boundary.test.ts __test__/placement/service-create-environment-boundary.test.ts __test__/environment/environment-secret-boundary.test.ts` - passed (18 файлов / 175 тестов); полный набор `bun run test -- --run` - 232 файла passed, 1 skipped, упал только __test__/deploy/application.real.test.ts (4 теста реальной сборки, connect ECONNREFUSED /Users/agent/.docker/run/docker.sock); `bun run typecheck` - passed; `./node_modules/.bin/biome check` по двум измененным файлам - passed; `git diff --check` - passed. - Ограничения: application.real.test.ts требует локальный Docker socket и роутер compose не импортирует; UI в браузере не проверялся; push и деплой не выполнялись. What: - Ported fix 9b80df3a0 from the deleted agentHits-prod branch (local copy backup/agentHits-Prod-before-delete-20261006): the 7da5612c0 sync dropped it, so it never reached AgentHits-Dev. - Added a redactComposeSecrets helper to the compose router for the create, one, update, delete, deployTemplate, and move responses: env and git provider secrets stay redacted as before, and customGitUrl credentials are now masked. - composeFile is always redacted in mutation responses, while compose.one redacts it only for users without the envVars.read permission: the UI compose file editor reads the file from compose.one, so a verbatim port would break it. - The service-environment-reveal test covers compose.one with and without envVars.read, plus the update response. Why: - compose.one, create, and update returned customGitUrl with its token and composeFile with inline secrets to anyone with read access to the service, even though the environment router already redacted these fields. Risks: - A credentialed customGitUrl now shows a placeholder instead of the token in the UI and API. Saving without edits keeps the stored URL; to change the URL it must be pasted in full, otherwise the server keeps the old value. - Roles without envVars.read (only possible with custom roles) see a placeholder instead of the compose file; getConvertedCompose (requires service.create) still returns the file, which is out of scope. Checks: - Commands and results: `bun install --frozen-lockfile` in a separate worktree - passed; in apps/dokploy `bun run test -- --run service-environment-reveal` - passed (1 file / 6 tests), a control run of the same test against compose.ts from origin/AgentHits-Dev - 3 compose tests failed as expected; `bun run test -- --run __test__/security/ __test__/compose/provider-status.test.ts __test__/deployment/cloud-deploy-idempotency.test.ts __test__/git-provider/deploy-source-credential-access.test.ts __test__/placement/deploy-target-boundary.test.ts __test__/placement/service-create-environment-boundary.test.ts __test__/environment/environment-secret-boundary.test.ts` - passed (18 files / 175 tests); the full `bun run test -- --run` suite - 232 files passed, 1 skipped, only __test__/deploy/application.real.test.ts failed (4 real build tests, connect ECONNREFUSED /Users/agent/.docker/run/docker.sock); `bun run typecheck` - passed; `./node_modules/.bin/biome check` on both changed files - passed; `git diff --check` - passed. - Limitations: application.real.test.ts needs a local Docker socket and does not import the compose router; the UI was not checked in a browser; nothing was pushed or deployed. Co-Authored-By: Claude Opus 5.5 --- .../service-environment-reveal.test.ts | 50 +++++++++++++- apps/dokploy/server/api/routers/compose.ts | 67 ++++++++++++++++--- 2 files changed, 108 insertions(+), 9 deletions(-) diff --git a/apps/dokploy/__test__/security/service-environment-reveal.test.ts b/apps/dokploy/__test__/security/service-environment-reveal.test.ts index 1a1bb432783..edfaf567de0 100644 --- a/apps/dokploy/__test__/security/service-environment-reveal.test.ts +++ b/apps/dokploy/__test__/security/service-environment-reveal.test.ts @@ -73,6 +73,7 @@ const permissionMocks = vi.hoisted(() => ({ checkServiceAccess: vi.fn(), checkServicePermissionAndAccess: vi.fn(), findMemberByUserId: vi.fn(), + hasPermission: vi.fn(), })); const auditMocks = vi.hoisted(() => ({ @@ -161,6 +162,7 @@ vi.mock("@dokploy/server/services/permission", () => ({ checkServicePermissionAndAccess: permissionMocks.checkServicePermissionAndAccess, findMemberByUserId: permissionMocks.findMemberByUserId, + hasPermission: permissionMocks.hasPermission, })); vi.mock("@dokploy/server/templates/github", () => ({ @@ -236,6 +238,9 @@ const compose = (organizationId = "org-1") => ({ appName: "compose-one", sourceType: "docker", env: "COMPOSE_SECRET=secret", + composeFile: + "services:\n api:\n environment:\n TOKEN: compose-secret", + customGitUrl: "https://compose-token@example.com/org/private.git", environment: { project: { organizationId, @@ -243,6 +248,8 @@ const compose = (organizationId = "org-1") => ({ }, }); +const redactedCustomGitUrl = `https://${REDACTED_SECRET_VALUE}@example.com/org/private.git`; + describe("service environment reveal boundary", () => { beforeEach(() => { vi.clearAllMocks(); @@ -250,6 +257,7 @@ describe("service environment reveal boundary", () => { permissionMocks.checkServicePermissionAndAccess.mockResolvedValue( undefined, ); + permissionMocks.hasPermission.mockResolvedValue(false); serverMocks.canEditDeployGitSource.mockResolvedValue(true); serverMocks.findApplicationById.mockResolvedValue(application()); serverMocks.findComposeById.mockResolvedValue(compose()); @@ -283,7 +291,15 @@ describe("service environment reveal boundary", () => { const caller = composeRouter.createCaller(createContext()); const normalRead = await caller.one({ composeId: "compose-1" }); - expect(normalRead.env).toBe(REDACTED_SECRET_VALUE); + expect(normalRead).toMatchObject({ + env: REDACTED_SECRET_VALUE, + composeFile: REDACTED_SECRET_VALUE, + customGitUrl: redactedCustomGitUrl, + }); + expect(permissionMocks.hasPermission).toHaveBeenCalledWith( + expect.anything(), + { envVars: ["read"] }, + ); const revealed = await caller.revealEnvironment({ composeId: "compose-1", @@ -299,6 +315,38 @@ describe("service environment reveal boundary", () => { }); }); + it("keeps the compose file for callers that can read env vars", async () => { + permissionMocks.hasPermission.mockResolvedValueOnce(true); + + const normalRead = await composeRouter + .createCaller(createContext()) + .one({ composeId: "compose-1" }); + + expect(normalRead).toMatchObject({ + env: REDACTED_SECRET_VALUE, + composeFile: compose().composeFile, + customGitUrl: redactedCustomGitUrl, + }); + }); + + it("redacts compose secrets in update responses", async () => { + serverMocks.updateCompose.mockResolvedValueOnce(compose()); + + const updated = await composeRouter + .createCaller(createContext()) + .update({ composeId: "compose-1", composeFile: REDACTED_SECRET_VALUE }); + + expect(serverMocks.updateCompose).toHaveBeenCalledWith( + "compose-1", + expect.objectContaining({ composeFile: compose().composeFile }), + ); + expect(updated).toMatchObject({ + env: REDACTED_SECRET_VALUE, + composeFile: REDACTED_SECRET_VALUE, + customGitUrl: redactedCustomGitUrl, + }); + }); + it("denies reveal when the service belongs to another organization", async () => { serverMocks.findApplicationById.mockResolvedValueOnce(application("org-2")); diff --git a/apps/dokploy/server/api/routers/compose.ts b/apps/dokploy/server/api/routers/compose.ts index 1e8f4a6a514..bf4f82983c3 100644 --- a/apps/dokploy/server/api/routers/compose.ts +++ b/apps/dokploy/server/api/routers/compose.ts @@ -48,6 +48,7 @@ import { checkServiceAccess, checkServicePermissionAndAccess, findMemberByUserId, + hasPermission, } from "@dokploy/server/services/permission"; import { type CompleteTemplate, @@ -60,6 +61,8 @@ import { assertCustomGitUrlAllowed } from "@dokploy/server/utils/providers/git"; import { preserveSecretPlaceholderFields, redactDeployableServiceSecrets, + redactSecretFields, + redactSensitiveText, } from "@dokploy/server/utils/security/redaction"; import { TRPCError } from "@trpc/server"; import { and, desc, eq, ilike, or, sql } from "drizzle-orm"; @@ -101,6 +104,51 @@ import { assertDeploySourceCredentialAccess } from "../utils/deploy-source-acces import { assertTargetEnvironmentAccess } from "../utils/placement-access"; import { assertServiceEnvironmentReadAccess } from "../utils/service-environment"; +type SecretRecord = Record; + +const redactCustomGitUrl = ( + record: T, +) => { + if (!record) { + return record; + } + + const redacted = { ...record }; + if ("customGitUrl" in redacted) { + redacted.customGitUrl = redactSensitiveText( + redacted.customGitUrl as string | null | undefined, + ); + } + + return redacted as T; +}; + +const redactComposeSecrets = ( + record: T, + { redactComposeFile = true }: { redactComposeFile?: boolean } = {}, +) => { + if (!record) { + return record; + } + + const redacted = redactCustomGitUrl( + redactDeployableServiceSecrets( + redactGitProviderSecrets( + record as T & { + bitbucket?: object | null; + gitea?: object | null; + github?: object | null; + gitlab?: object | null; + }, + ), + ), + ); + + return redactComposeFile + ? redactSecretFields(redacted, ["composeFile"]) + : redacted; +}; + const composeSourceUpdateFields = [ "bitbucketBranch", "bitbucketId", @@ -258,7 +306,7 @@ export const composeRouter = createTRPCRouter({ resourceId: newService.composeId, resourceName: newService.appName, }); - return redactDeployableServiceSecrets(newService); + return redactComposeSecrets(newService); } catch (error) { throw error; } @@ -311,8 +359,13 @@ export const composeRouter = createTRPCRouter({ } } + const canReadEnvVars = await hasPermission(ctx, { envVars: ["read"] }); + return { - ...redactDeployableServiceSecrets(redactGitProviderSecrets(compose)), + // The compose file editor loads composeFile from this response. + ...redactComposeSecrets(compose, { + redactComposeFile: !canReadEnvVars, + }), hasGitProviderAccess, unauthorizedProvider, }; @@ -360,7 +413,7 @@ export const composeRouter = createTRPCRouter({ resourceId: input.composeId, resourceName: updated?.name, }); - return redactDeployableServiceSecrets(updated); + return redactComposeSecrets(updated); }), saveEnvironment: protectedProcedure .input(apiSaveEnvironmentVariablesCompose) @@ -439,9 +492,7 @@ export const composeRouter = createTRPCRouter({ resourceId: composeResult.composeId, resourceName: composeResult.appName, }); - return redactDeployableServiceSecrets( - redactGitProviderSecrets(composeResult), - ); + return redactComposeSecrets(composeResult); }), cleanQueues: protectedProcedure .input(apiFindCompose) @@ -932,7 +983,7 @@ export const composeRouter = createTRPCRouter({ resourceId: compose.composeId, resourceName: compose.name, }); - return redactDeployableServiceSecrets(compose); + return redactComposeSecrets(compose); }), templates: protectedProcedure @@ -1055,7 +1106,7 @@ export const composeRouter = createTRPCRouter({ resourceId: input.composeId, resourceName: updatedCompose.name, }); - return redactDeployableServiceSecrets(updatedCompose); + return redactComposeSecrets(updatedCompose); }), processTemplate: protectedProcedure