diff --git a/apps/dokploy/__test__/ai/ai-settings-boundary.test.ts b/apps/dokploy/__test__/ai/ai-settings-boundary.test.ts index 1f9b909f47..db35e36e40 100644 --- a/apps/dokploy/__test__/ai/ai-settings-boundary.test.ts +++ b/apps/dokploy/__test__/ai/ai-settings-boundary.test.ts @@ -111,6 +111,49 @@ describe("AI settings organization boundary", () => { ); }); + it("rejects a new API URL while keeping the stored API key", async () => { + mocks.aiFindFirst.mockResolvedValue({ + ...aiSettings, + organizationId: "org-1", + }); + + for (const apiKey of ["__DOKPLOY_REDACTED_SECRET__", "", undefined]) { + await expect( + saveAiSettings("org-1", { + aiId: "ai-1", + apiKey, + apiUrl: "https://collector.example.net/v1", + model: "gpt-4o-mini", + }), + ).rejects.toMatchObject({ + code: "BAD_REQUEST", + message: "Re-enter the API key to change apiUrl", + }); + } + expect(mocks.insert).not.toHaveBeenCalled(); + + await saveAiSettings("org-1", { + aiId: "ai-1", + apiKey: "__DOKPLOY_REDACTED_SECRET__", + apiUrl: "https://api.openai.com/v1/", + model: "gpt-4o", + }); + await saveAiSettings("org-1", { + aiId: "ai-1", + apiKey: "new-secret", + apiUrl: "https://api.mistral.ai/v1", + }); + expect(mocks.onConflictDoUpdate).toHaveBeenCalledTimes(2); + expect(mocks.onConflictDoUpdate).toHaveBeenLastCalledWith( + expect.objectContaining({ + set: expect.objectContaining({ + apiKey: "new-secret", + apiUrl: "https://api.mistral.ai/v1", + }), + }), + ); + }); + it("rejects deleting another organization's AI settings by aiId", async () => { await expect(deleteAiSettings("ai-1", "org-1")).rejects.toMatchObject({ code: "NOT_FOUND", diff --git a/apps/dokploy/__test__/dns/dns-provider-service.test.ts b/apps/dokploy/__test__/dns/dns-provider-service.test.ts index 995fc7fc60..918a4c570c 100644 --- a/apps/dokploy/__test__/dns/dns-provider-service.test.ts +++ b/apps/dokploy/__test__/dns/dns-provider-service.test.ts @@ -28,20 +28,34 @@ describe("maskDnsProviderConfig", () => { }); }); - it("masks only the secretAccessKey for a route53 config, keeping accessKeyId visible", () => { + it("masks both the accessKeyId and the secretAccessKey for a route53 config", () => { const masked = maskDnsProviderConfig({ providerType: "route53", - accessKeyId: "AKIA_VISIBLE", + accessKeyId: "AKIA_STORED", secretAccessKey: "shh", }); expect(masked).toEqual({ providerType: "route53", - accessKeyId: "AKIA_VISIBLE", + accessKeyId: DNS_SECRET_MASK, secretAccessKey: DNS_SECRET_MASK, }); }); + it("masks both porkbun keys", () => { + const masked = maskDnsProviderConfig({ + providerType: "porkbun", + apiKey: "pk1_stored", + secretApiKey: "sk1_stored", + }); + + expect(masked).toEqual({ + providerType: "porkbun", + apiKey: DNS_SECRET_MASK, + secretApiKey: DNS_SECRET_MASK, + }); + }); + it("leaves an empty sensitive field untouched instead of masking a blank value", () => { const masked = maskDnsProviderConfig({ providerType: "cloudflare", @@ -95,7 +109,46 @@ describe("mergeDnsProviderConfig", () => { ); }); - it("does not require re-entry for fields that are not sensitive", () => { + it("restores masked route53 and porkbun key ids from the stored config", () => { + expect( + mergeDnsProviderConfig( + { + providerType: "route53", + accessKeyId: DNS_SECRET_MASK, + secretAccessKey: DNS_SECRET_MASK, + }, + { + providerType: "route53", + accessKeyId: "AKIA_STORED", + secretAccessKey: "stored-secret", + }, + ), + ).toEqual({ + providerType: "route53", + accessKeyId: "AKIA_STORED", + secretAccessKey: "stored-secret", + }); + expect( + mergeDnsProviderConfig( + { + providerType: "porkbun", + apiKey: DNS_SECRET_MASK, + secretApiKey: DNS_SECRET_MASK, + }, + { + providerType: "porkbun", + apiKey: "pk1_stored", + secretApiKey: "sk1_stored", + }, + ), + ).toEqual({ + providerType: "porkbun", + apiKey: "pk1_stored", + secretApiKey: "sk1_stored", + }); + }); + + it("keeps a newly entered key id next to a masked secret", () => { const existing = { providerType: "route53" as const, accessKeyId: "AKIA_OLD", diff --git a/apps/dokploy/__test__/env/vault-provider-secret-binding.test.ts b/apps/dokploy/__test__/env/vault-provider-secret-binding.test.ts new file mode 100644 index 0000000000..d2273624b1 --- /dev/null +++ b/apps/dokploy/__test__/env/vault-provider-secret-binding.test.ts @@ -0,0 +1,218 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + findFirst: vi.fn(), + findMany: vi.fn(), + update: vi.fn(), +})); + +vi.mock("@dokploy/server/db", () => ({ + db: { + query: { + vaultProvider: { findFirst: mocks.findFirst }, + projects: { findMany: mocks.findMany }, + }, + update: mocks.update, + }, +})); + +const { + VAULT_SECRET_MASK, + maskVaultProviderConfig, + mergeVaultProviderConfig, + updateVaultProvider, +} = await import("@dokploy/server/services/vault-provider"); + +const storedHashicorp = { + providerType: "hashicorp" as const, + url: "https://vault.example.com", + token: "stored-vault-token", + mount: "secret", +}; + +describe("vault provider stored secret binding", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.findMany.mockResolvedValue([]); + }); + + it("masks the AWS access key id together with the secret key", () => { + for (const providerType of ["aws", "aws-parameter-store"] as const) { + expect( + maskVaultProviderConfig({ + providerType, + region: "eu-west-1", + accessKeyId: "AKIA_STORED", + secretAccessKey: "stored-secret", + }), + ).toMatchObject({ + accessKeyId: VAULT_SECRET_MASK, + secretAccessKey: VAULT_SECRET_MASK, + }); + } + }); + + it("restores masked AWS credentials when the endpoint is unchanged", () => { + const stored = { + providerType: "aws" as const, + region: "eu-west-1", + accessKeyId: "AKIA_STORED", + secretAccessKey: "stored-secret", + endpoint: "https://secrets.example.com", + }; + + expect( + mergeVaultProviderConfig( + { + ...stored, + accessKeyId: VAULT_SECRET_MASK, + secretAccessKey: VAULT_SECRET_MASK, + }, + stored, + ), + ).toEqual(stored); + }); + + it("rejects sending a kept HashiCorp token to a new URL", () => { + expect(() => + mergeVaultProviderConfig( + { + ...storedHashicorp, + url: "https://collector.example.net", + token: VAULT_SECRET_MASK, + }, + storedHashicorp, + ), + ).toThrow( + expect.objectContaining({ + code: "BAD_REQUEST", + message: "Re-enter the vault provider credentials to change url", + }), + ); + + expect( + mergeVaultProviderConfig( + { ...storedHashicorp, token: VAULT_SECRET_MASK, mount: "kv" }, + storedHashicorp, + ), + ).toEqual({ ...storedHashicorp, mount: "kv" }); + + const reentered = { + ...storedHashicorp, + url: "https://vault-new.example.com", + token: "new-token", + }; + expect(mergeVaultProviderConfig(reentered, storedHashicorp)).toEqual( + reentered, + ); + }); + + it("rejects kept credentials with a new endpoint or identity for every URL-bearing provider", () => { + const cases = [ + [ + { + providerType: "infisical" as const, + siteUrl: "https://app.infisical.com", + clientId: "client", + clientSecret: "stored", + projectId: "p", + environmentSlug: "prod", + secretPath: "/", + }, + { siteUrl: "https://collector.example.net" }, + "clientSecret", + ], + [ + { + providerType: "aws" as const, + region: "eu-west-1", + accessKeyId: "AKIA_STORED", + secretAccessKey: "stored", + }, + { endpoint: "https://collector.example.net" }, + "secretAccessKey", + ], + [ + { + providerType: "aws-parameter-store" as const, + region: "eu-west-1", + accessKeyId: "AKIA_STORED", + secretAccessKey: "stored", + }, + { accessKeyId: "AKIA_OTHER" }, + "secretAccessKey", + ], + [ + { + providerType: "azure" as const, + vaultUri: "https://vault.vault.azure.net", + tenantId: "tenant", + clientId: "client", + clientSecret: "stored", + }, + { vaultUri: "https://collector.example.net" }, + "clientSecret", + ], + [ + { + providerType: "scaleway" as const, + region: "fr-par", + projectId: "p", + secretKey: "stored", + apiUrl: "https://api.scaleway.com", + }, + { apiUrl: "https://collector.example.net" }, + "secretKey", + ], + [ + { + providerType: "phase" as const, + token: "stored", + appId: "app", + env: "prod", + path: "/", + apiUrl: "https://api.phase.dev", + }, + { apiUrl: "https://collector.example.net" }, + "token", + ], + ] as const; + + for (const [stored, change, secretField] of cases) { + const incoming = { + ...stored, + ...change, + [secretField]: VAULT_SECRET_MASK, + }; + expect(() => mergeVaultProviderConfig(incoming as never, stored)).toThrow( + expect.objectContaining({ code: "BAD_REQUEST" }), + ); + } + }); + + it("does not persist a provider update that redirects the stored token", async () => { + mocks.findFirst.mockResolvedValue({ + vaultProviderId: "vault-1", + organizationId: "org-1", + name: "vault", + providerType: "hashicorp", + config: storedHashicorp, + assignments: [], + }); + + await expect( + updateVaultProvider( + "vault-1", + "vault", + { + ...storedHashicorp, + url: "https://collector.example.net", + token: VAULT_SECRET_MASK, + }, + [], + ), + ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + + expect(mocks.update).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/dokploy/__test__/git-provider/provider-stored-secret-binding.test.ts b/apps/dokploy/__test__/git-provider/provider-stored-secret-binding.test.ts new file mode 100644 index 0000000000..c3d7a45dc0 --- /dev/null +++ b/apps/dokploy/__test__/git-provider/provider-stored-secret-binding.test.ts @@ -0,0 +1,163 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + giteaFindFirst: vi.fn(), + gitlabFindFirst: vi.fn(), + set: vi.fn(), + update: vi.fn(), +})); + +vi.mock("@dokploy/server/db", () => ({ + db: { + query: { + gitea: { findFirst: mocks.giteaFindFirst }, + gitlab: { findFirst: mocks.gitlabFindFirst }, + }, + update: mocks.update, + }, +})); + +const { updateGitlab } = await import("@dokploy/server/services/gitlab"); +const { updateGitea } = await import("@dokploy/server/services/gitea"); + +const storedGitlab = { + gitlabId: "gitlab-1", + gitlabUrl: "https://gitlab.example.com", + gitlabInternalUrl: null, + applicationId: "app-id", + redirectUri: "https://dokploy.example.com/api/providers/gitlab/callback", + secret: "stored-app-secret", + webhookSecret: null, + accessToken: "stored-access-token", + refreshToken: "stored-refresh-token", + groupName: null, + expiresAt: 1_900_000_000, + gitProviderId: "provider-1", +}; + +const storedGitea = { + giteaId: "gitea-1", + giteaUrl: "https://gitea.example.com", + giteaInternalUrl: null, + redirectUri: "https://dokploy.example.com/api/providers/gitea/callback", + clientId: "client-id", + clientSecret: "stored-client-secret", + organizationName: null, + gitProviderId: "provider-2", + accessToken: "stored-access-token", + refreshToken: "stored-refresh-token", + expiresAt: 1_900_000_000, + scopes: "repo", + lastAuthenticatedAt: null, +}; + +describe("git provider stored secret binding", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.gitlabFindFirst.mockResolvedValue(storedGitlab); + mocks.giteaFindFirst.mockResolvedValue(storedGitea); + mocks.update.mockReturnValue({ set: mocks.set }); + mocks.set.mockReturnValue({ + where: () => ({ returning: async () => [{ giteaId: "gitea-1" }] }), + }); + }); + + it.each([ + ["gitlabUrl", { gitlabUrl: "https://collector.example.net" }], + ["gitlabInternalUrl", { gitlabInternalUrl: "http://collector:80" }], + ["applicationId", { applicationId: "other-app" }], + ])( + "rejects a new GitLab %s while keeping the stored secret", + async (field, change) => { + await expect( + updateGitlab("gitlab-1", { + gitlabUrl: storedGitlab.gitlabUrl, + gitlabInternalUrl: "", + ...change, + }), + ).rejects.toMatchObject({ + code: "BAD_REQUEST", + message: `Re-enter the GitLab application secret to change ${field}`, + }); + expect(mocks.update).not.toHaveBeenCalled(); + }, + ); + + it("drops GitLab OAuth tokens when the URL changes with a new secret", async () => { + await updateGitlab("gitlab-1", { + gitlabUrl: "https://gitlab-new.example.com", + secret: "new-app-secret", + }); + + expect(mocks.set).toHaveBeenCalledWith( + expect.objectContaining({ + gitlabUrl: "https://gitlab-new.example.com", + secret: "new-app-secret", + accessToken: null, + refreshToken: null, + expiresAt: null, + }), + ); + + await updateGitlab("gitlab-1", { + gitlabUrl: storedGitlab.gitlabUrl, + gitlabInternalUrl: "", + groupName: "platform", + }); + expect(mocks.set).toHaveBeenLastCalledWith({ + gitlabUrl: storedGitlab.gitlabUrl, + gitlabInternalUrl: "", + groupName: "platform", + }); + }); + + it.each([ + ["giteaUrl", { giteaUrl: "https://collector.example.net" }], + ["giteaInternalUrl", { giteaInternalUrl: "http://collector:3000" }], + ["clientId", { clientId: "other-client" }], + ])( + "rejects a new Gitea %s while keeping the stored secret", + async (field, change) => { + await expect( + updateGitea("gitea-1", { + giteaUrl: storedGitea.giteaUrl, + ...change, + }), + ).rejects.toMatchObject({ + code: "BAD_REQUEST", + message: `Re-enter the Gitea client secret to change ${field}`, + }); + expect(mocks.update).not.toHaveBeenCalled(); + }, + ); + + it("drops Gitea OAuth tokens when the URL changes with a new secret", async () => { + await updateGitea("gitea-1", { + giteaUrl: "https://gitea-new.example.com", + clientId: "client-id", + clientSecret: "new-client-secret", + }); + + expect(mocks.set).toHaveBeenCalledWith( + expect.objectContaining({ + giteaUrl: "https://gitea-new.example.com", + clientSecret: "new-client-secret", + accessToken: null, + refreshToken: null, + expiresAt: null, + }), + ); + + await updateGitea("gitea-1", { + accessToken: "refreshed-token", + refreshToken: "refreshed-refresh-token", + expiresAt: 1_900_000_100, + }); + expect(mocks.giteaFindFirst).toHaveBeenCalledTimes(1); + expect(mocks.set).toHaveBeenLastCalledWith({ + accessToken: "refreshed-token", + refreshToken: "refreshed-refresh-token", + expiresAt: 1_900_000_100, + }); + }); +}); diff --git a/apps/dokploy/__test__/notification/notification-router-boundary.test.ts b/apps/dokploy/__test__/notification/notification-router-boundary.test.ts index c6c4eb8816..921607dd38 100644 --- a/apps/dokploy/__test__/notification/notification-router-boundary.test.ts +++ b/apps/dokploy/__test__/notification/notification-router-boundary.test.ts @@ -1,4 +1,5 @@ import { REDACTED_NOTIFICATION_SECRET } from "@dokploy/server/utils/notifications/security"; +import { TRPCError } from "@trpc/server"; import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ @@ -261,6 +262,33 @@ describe("notification router secret and organization boundaries", () => { expect(mocks.audit).not.toHaveBeenCalled(); }); + it("returns the re-enter message when an email update would reuse the stored password", async () => { + mocks.findNotificationById.mockResolvedValue({ + ...notificationWithSecrets, + notificationType: "email", + emailId: "email-1", + }); + mocks.updateEmailNotification.mockRejectedValue( + new TRPCError({ + code: "BAD_REQUEST", + message: "Re-enter the SMTP password to change smtpServer", + }), + ); + + await expect( + createCaller().updateEmail({ + notificationId: "notification-1", + emailId: "email-1", + smtpServer: "smtp.collector.example.net", + password: REDACTED_NOTIFICATION_SECRET, + }), + ).rejects.toMatchObject({ + code: "BAD_REQUEST", + message: "Re-enter the SMTP password to change smtpServer", + }); + expect(mocks.audit).not.toHaveBeenCalled(); + }); + it("accepts blank required secret update values so existing secrets are preserved", async () => { await expect( createCaller().updateSlack({ diff --git a/apps/dokploy/__test__/notification/notification-stored-secret-binding.test.ts b/apps/dokploy/__test__/notification/notification-stored-secret-binding.test.ts new file mode 100644 index 0000000000..ad2653bc98 --- /dev/null +++ b/apps/dokploy/__test__/notification/notification-stored-secret-binding.test.ts @@ -0,0 +1,227 @@ +import { REDACTED_NOTIFICATION_SECRET } from "@dokploy/server/utils/notifications/security"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + customFindFirst: vi.fn(), + emailFindFirst: vi.fn(), + gotifyFindFirst: vi.fn(), + ntfyFindFirst: vi.fn(), + set: vi.fn(), + transaction: vi.fn(), +})); + +vi.mock("@dokploy/server/db", () => ({ + db: { + query: { + custom: { findFirst: mocks.customFindFirst }, + email: { findFirst: mocks.emailFindFirst }, + gotify: { findFirst: mocks.gotifyFindFirst }, + ntfy: { findFirst: mocks.ntfyFindFirst }, + }, + transaction: mocks.transaction, + }, +})); + +const { + updateCustomNotification, + updateEmailNotification, + updateGotifyNotification, + updateNtfyNotification, +} = await import("@dokploy/server/services/notification"); + +const notificationFields = { + notificationId: "notification-1", + name: "alerts", + organizationId: "org-1", +}; + +const providerSets = () => + mocks.set.mock.calls + .map(([values]) => values as Record) + .filter((values) => !("name" in values)); + +describe("notification update stored secret binding", () => { + beforeEach(() => { + vi.clearAllMocks(); + const chain = { + returning: vi.fn(async () => [{ notificationId: "notification-1" }]), + }; + const tx = { + update: vi.fn(() => ({ + set: (values: unknown) => { + mocks.set(values); + return { where: vi.fn(() => chain) }; + }, + })), + }; + mocks.transaction.mockImplementation(async (callback) => callback(tx)); + mocks.emailFindFirst.mockResolvedValue({ + emailId: "email-1", + smtpServer: "smtp.example.com", + smtpPort: 587, + username: "alerts@example.com", + password: "stored-smtp-password", + }); + mocks.gotifyFindFirst.mockResolvedValue({ + gotifyId: "gotify-1", + serverUrl: "https://gotify.example.com", + appToken: "stored-gotify-token", + }); + mocks.ntfyFindFirst.mockResolvedValue({ + ntfyId: "ntfy-1", + serverUrl: "https://ntfy.example.com", + topic: "alerts", + accessToken: "stored-ntfy-token", + }); + mocks.customFindFirst.mockResolvedValue({ + customId: "custom-1", + endpoint: "https://hooks.example.com/notify", + headers: { Authorization: "Bearer stored-header-token" }, + }); + }); + + it.each([ + ["smtpServer", { smtpServer: "smtp.collector.example.net" }], + ["smtpPort", { smtpPort: 2525 }], + ["username", { username: "other@example.net" }], + ])( + "rejects a new SMTP %s with the stored password", + async (field, change) => { + for (const password of [REDACTED_NOTIFICATION_SECRET, "", undefined]) { + await expect( + updateEmailNotification({ + ...notificationFields, + emailId: "email-1", + smtpServer: "smtp.example.com", + smtpPort: 587, + username: "alerts@example.com", + password, + ...change, + }), + ).rejects.toMatchObject({ + code: "BAD_REQUEST", + message: `Re-enter the SMTP password to change ${field}`, + }); + } + expect(mocks.transaction).not.toHaveBeenCalled(); + + await updateEmailNotification({ + ...notificationFields, + emailId: "email-1", + smtpServer: "SMTP.example.com", + smtpPort: 587, + username: "alerts@example.com", + password: REDACTED_NOTIFICATION_SECRET, + }); + await updateEmailNotification({ + ...notificationFields, + emailId: "email-1", + ...change, + password: "new-smtp-password", + }); + expect(providerSets()).toEqual([ + expect.objectContaining({ + smtpServer: "smtp.example.com", + password: undefined, + }), + expect.objectContaining({ ...change, password: "new-smtp-password" }), + ]); + }, + ); + + it("rejects a new Gotify server URL with the stored app token", async () => { + await expect( + updateGotifyNotification({ + ...notificationFields, + gotifyId: "gotify-1", + serverUrl: "https://collector.example.net", + appToken: REDACTED_NOTIFICATION_SECRET, + }), + ).rejects.toMatchObject({ + code: "BAD_REQUEST", + message: "Re-enter the Gotify app token to change serverUrl", + }); + expect(mocks.transaction).not.toHaveBeenCalled(); + + await updateGotifyNotification({ + ...notificationFields, + gotifyId: "gotify-1", + serverUrl: "https://gotify.example.com/", + appToken: REDACTED_NOTIFICATION_SECRET, + priority: 8, + }); + expect(providerSets()).toEqual([ + expect.objectContaining({ appToken: undefined, priority: 8 }), + ]); + }); + + it("rejects a new ntfy server URL with the stored access token", async () => { + await expect( + updateNtfyNotification({ + ...notificationFields, + ntfyId: "ntfy-1", + serverUrl: "https://collector.example.net", + topic: "alerts", + accessToken: REDACTED_NOTIFICATION_SECRET, + }), + ).rejects.toMatchObject({ + code: "BAD_REQUEST", + message: "Re-enter the ntfy access token to change serverUrl", + }); + expect(mocks.transaction).not.toHaveBeenCalled(); + + await updateNtfyNotification({ + ...notificationFields, + ntfyId: "ntfy-1", + serverUrl: "https://ntfy-new.example.com", + topic: "alerts", + accessToken: "", + }); + expect(providerSets()).toEqual([ + expect.objectContaining({ + serverUrl: "https://ntfy-new.example.com", + accessToken: null, + }), + ]); + }); + + it("rejects a new custom endpoint with the stored header values", async () => { + for (const headers of [ + { Authorization: REDACTED_NOTIFICATION_SECRET }, + undefined, + ]) { + await expect( + updateCustomNotification({ + ...notificationFields, + customId: "custom-1", + endpoint: "https://collector.example.net/hook", + headers, + }), + ).rejects.toMatchObject({ + code: "BAD_REQUEST", + message: "Re-enter the custom notification headers to change endpoint", + }); + } + expect(mocks.transaction).not.toHaveBeenCalled(); + + await updateCustomNotification({ + ...notificationFields, + customId: "custom-1", + endpoint: REDACTED_NOTIFICATION_SECRET, + headers: { Authorization: REDACTED_NOTIFICATION_SECRET }, + }); + await updateCustomNotification({ + ...notificationFields, + customId: "custom-1", + endpoint: "https://hooks-new.example.com/notify", + headers: { Authorization: "Bearer new-token" }, + }); + expect(providerSets()).toEqual([ + { endpoint: undefined, headers: undefined }, + { + endpoint: "https://hooks-new.example.com/notify", + headers: { Authorization: "Bearer new-token" }, + }, + ]); + }); +}); diff --git a/apps/dokploy/__test__/registry/registry-stored-password-binding.test.ts b/apps/dokploy/__test__/registry/registry-stored-password-binding.test.ts new file mode 100644 index 0000000000..f508b98e33 --- /dev/null +++ b/apps/dokploy/__test__/registry/registry-stored-password-binding.test.ts @@ -0,0 +1,119 @@ +import { REDACTED_SECRET_VALUE } from "@dokploy/server/utils/security/redaction"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + execAsync: vi.fn(), + execAsyncRemote: vi.fn(), + findFirst: vi.fn(), + returning: vi.fn(), + set: vi.fn(), + update: vi.fn(), + where: vi.fn(), +})); + +vi.mock("@dokploy/server/constants", () => ({ + IS_CLOUD: false, +})); + +vi.mock("@dokploy/server/db", () => ({ + db: { + query: { registry: { findFirst: mocks.findFirst } }, + update: mocks.update, + }, +})); + +vi.mock("@dokploy/server/utils/process/execAsync", () => ({ + execAsync: mocks.execAsync, + execAsyncRemote: mocks.execAsyncRemote, +})); + +const { updateRegistry } = await import("@dokploy/server/services/registry"); +const { apiUpdateRegistry } = await import("@dokploy/server/db/schema"); + +const storedRegistry = { + registryId: "registry-1", + registryName: "registry", + registryUrl: "registry.example.com", + registryType: "cloud" as const, + username: "deploy", + imagePrefix: null, + organizationId: "org-1", + createdAt: "2026-01-01T00:00:00.000Z", +}; + +describe("registry update stored password binding", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.findFirst.mockResolvedValue(storedRegistry); + mocks.update.mockReturnValue({ set: mocks.set }); + mocks.set.mockReturnValue({ where: mocks.where }); + mocks.where.mockReturnValue({ returning: mocks.returning }); + mocks.returning.mockResolvedValue([ + { ...storedRegistry, password: "stored-password" }, + ]); + mocks.execAsync.mockResolvedValue({ stdout: "", stderr: "" }); + }); + + it.each([ + ["registryUrl", { registryUrl: "collector.example.net" }], + ["username", { username: "someone-else" }], + ])( + "rejects a new %s while keeping the stored password", + async (_field, change) => { + await expect( + updateRegistry("registry-1", { + registryName: "registry", + registryUrl: storedRegistry.registryUrl, + username: storedRegistry.username, + ...change, + }), + ).rejects.toMatchObject({ + code: "BAD_REQUEST", + message: expect.stringContaining("Re-enter the registry password"), + }); + + expect(mocks.update).not.toHaveBeenCalled(); + expect(mocks.execAsync).not.toHaveBeenCalled(); + expect(mocks.execAsyncRemote).not.toHaveBeenCalled(); + + await updateRegistry("registry-1", { + ...change, + password: "new-password", + }); + expect(mocks.set).toHaveBeenCalledWith( + expect.objectContaining({ ...change, password: "new-password" }), + ); + }, + ); + + it("treats the redacted placeholder as keeping the stored password", async () => { + await expect( + updateRegistry("registry-1", { + registryUrl: "collector.example.net", + password: REDACTED_SECRET_VALUE, + }), + ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + expect(mocks.update).not.toHaveBeenCalled(); + + await updateRegistry("registry-1", { + registryName: "renamed", + registryUrl: storedRegistry.registryUrl, + password: REDACTED_SECRET_VALUE, + }); + expect(mocks.set).toHaveBeenCalledWith( + expect.not.objectContaining({ password: REDACTED_SECRET_VALUE }), + ); + expect(mocks.set).toHaveBeenCalledWith( + expect.objectContaining({ registryName: "renamed" }), + ); + }); + + it("does not accept organizationId in the update input", () => { + const parsed = apiUpdateRegistry.parse({ + registryId: "registry-1", + organizationId: "org-2", + }); + + expect(parsed).not.toHaveProperty("organizationId"); + }); +}); diff --git a/apps/dokploy/components/dashboard/settings/git/gitlab/edit-gitlab-provider.tsx b/apps/dokploy/components/dashboard/settings/git/gitlab/edit-gitlab-provider.tsx index 4bcbbf15be..44d9ca9a16 100644 --- a/apps/dokploy/components/dashboard/settings/git/gitlab/edit-gitlab-provider.tsx +++ b/apps/dokploy/components/dashboard/settings/git/gitlab/edit-gitlab-provider.tsx @@ -38,6 +38,7 @@ const Schema = z.object({ .union([z.string().url(), z.literal("")]) .optional() .transform((v) => (v === "" ? undefined : v)), + secret: z.string().optional(), webhookSecret: z.string().optional(), groupName: z.string().optional(), }); @@ -68,6 +69,7 @@ export const EditGitlabProvider = ({ gitlabId }: Props) => { name: "", gitlabUrl: "https://gitlab.com", gitlabInternalUrl: "", + secret: "", webhookSecret: "", }, resolver: zodResolver(Schema), @@ -81,6 +83,7 @@ export const EditGitlabProvider = ({ gitlabId }: Props) => { name: gitlab?.gitProvider.name || "", gitlabUrl: gitlab?.gitlabUrl || "", gitlabInternalUrl: gitlab?.gitlabInternalUrl || "", + secret: "", webhookSecret: "", }); }, [form, isOpen]); @@ -93,6 +96,7 @@ export const EditGitlabProvider = ({ gitlabId }: Props) => { name: data.name || "", gitlabUrl: data.gitlabUrl || "", gitlabInternalUrl: data.gitlabInternalUrl ?? null, + secret: data.secret || undefined, webhookSecret: data.webhookSecret || undefined, }) .then(async () => { @@ -186,6 +190,29 @@ export const EditGitlabProvider = ({ gitlabId }: Props) => { )} /> + ( + + Application Secret (Optional) + + + + + Required when changing the GitLab URL. Authorize the + provider again after the change. + + + + )} + /> + url.trim().replace(/\/+$/, ""); export const saveAiSettings = async (organizationId: string, settings: any) => { const aiId = settings.aiId; - if (aiId) { - const existingAiSetting = await db.query.ai.findFirst({ - where: eq(ai.aiId, aiId), - }); - if ( - existingAiSetting && - existingAiSetting.organizationId !== organizationId - ) { - throwAiSettingsNotFound(); - } + const existingAiSetting = aiId + ? await db.query.ai.findFirst({ + where: eq(ai.aiId, aiId), + }) + : undefined; + if ( + existingAiSetting && + existingAiSetting.organizationId !== organizationId + ) { + throwAiSettingsNotFound(); } const normalizedSettings = { ...settings }; @@ -142,6 +145,23 @@ export const saveAiSettings = async (organizationId: string, settings: any) => { normalizedSettings.apiUrl, ); } + if ( + existingAiSetting?.apiKey && + normalizedSettings.apiKey === undefined && + normalizedSettings.apiUrl + ) { + const storedApiUrl = existingAiSetting.apiUrl; + const sameApiUrl = + settings.apiUrl === storedApiUrl || + normalizeApiUrl(normalizedSettings.apiUrl) === + normalizeApiUrl(storedApiUrl); + assertStoredSecretTargetUnchanged("API key", { + apiUrl: [ + sameApiUrl ? storedApiUrl : normalizedSettings.apiUrl, + storedApiUrl, + ], + }); + } if (normalizedSettings.apiUrl) { const customProviders = await getCustomAiProviders(organizationId); if (customProviders.length > 0) { diff --git a/packages/server/src/services/dns-provider.ts b/packages/server/src/services/dns-provider.ts index 0ea22696b1..320f8d0b66 100644 --- a/packages/server/src/services/dns-provider.ts +++ b/packages/server/src/services/dns-provider.ts @@ -16,8 +16,8 @@ export const DNS_SECRET_MASK = "********"; const SENSITIVE_FIELDS: Record = { cloudflare: ["apiToken"], - route53: ["secretAccessKey"], - porkbun: ["secretApiKey"], + route53: ["accessKeyId", "secretAccessKey"], + porkbun: ["apiKey", "secretApiKey"], infomaniak: ["apiToken"], ovh: ["applicationSecret", "consumerKey"], }; diff --git a/packages/server/src/services/gitea.ts b/packages/server/src/services/gitea.ts index a1dae3855e..cee970c799 100644 --- a/packages/server/src/services/gitea.ts +++ b/packages/server/src/services/gitea.ts @@ -4,6 +4,11 @@ import { gitea, gitProvider, } from "@dokploy/server/db/schema"; +import { + assertStoredSecretTargetUnchanged, + changedSecretTargetFields, + secretUpdateValue, +} from "@dokploy/server/utils/security/redaction"; import { TRPCError } from "@trpc/server"; import { eq } from "drizzle-orm"; import type { z } from "zod"; @@ -98,11 +103,67 @@ export const findGiteaGitProviderId = async (giteaId: string) => { return giteaProviderResult.gitProviderId; }; +const giteaTargetFields = ["giteaUrl", "giteaInternalUrl", "clientId"] as const; + +// OAuth tokens and the client secret are sent to the Gitea URL, so a caller +// may only move the provider to another URL by entering the secret again, and +// tokens issued by the old instance are dropped. +const bindStoredGiteaSecrets = async ( + giteaId: string, + input: Partial, +) => { + if (giteaTargetFields.every((field) => input[field] === undefined)) { + return {}; + } + const current = await db.query.gitea.findFirst({ + where: eq(gitea.giteaId, giteaId), + }); + if (!current) { + return {}; + } + + const targets = Object.fromEntries( + giteaTargetFields.map((field) => [ + field, + [ + input[field] === undefined ? current[field] : input[field], + current[field], + ] as const, + ]), + ); + if ( + current.clientSecret && + secretUpdateValue(input.clientSecret) === undefined + ) { + assertStoredSecretTargetUnchanged("Gitea client secret", targets); + } + + const urlChanged = changedSecretTargetFields(targets).some( + (field) => field !== "clientId", + ); + return urlChanged + ? { + accessToken: input.accessToken ?? null, + refreshToken: input.refreshToken ?? null, + expiresAt: input.expiresAt ?? null, + } + : {}; +}; + export const updateGitea = async (giteaId: string, input: Partial) => { try { + const { clientSecret, ...giteaInput } = input; + const nextClientSecret = secretUpdateValue(clientSecret); + const tokenReset = await bindStoredGiteaSecrets(giteaId, input); const updateResult = await db .update(gitea) - .set(input) + .set({ + ...giteaInput, + ...tokenReset, + ...(nextClientSecret !== undefined && { + clientSecret: nextClientSecret, + }), + }) .where(eq(gitea.giteaId, giteaId)) .returning(); diff --git a/packages/server/src/services/gitlab.ts b/packages/server/src/services/gitlab.ts index b5640e08b1..c90a4a78ef 100644 --- a/packages/server/src/services/gitlab.ts +++ b/packages/server/src/services/gitlab.ts @@ -4,7 +4,11 @@ import { gitlab, gitProvider, } from "@dokploy/server/db/schema"; -import { secretUpdateValue } from "@dokploy/server/utils/security/redaction"; +import { + assertStoredSecretTargetUnchanged, + changedSecretTargetFields, + secretUpdateValue, +} from "@dokploy/server/utils/security/redaction"; import { TRPCError } from "@trpc/server"; import { eq } from "drizzle-orm"; import type { z } from "zod"; @@ -84,16 +88,68 @@ export const findGitlabGitProviderId = async (gitlabId: string) => { return gitlabProviderResult.gitProviderId; }; +const gitlabTargetFields = [ + "gitlabUrl", + "gitlabInternalUrl", + "applicationId", +] as const; + +// OAuth tokens and the application secret are sent to the GitLab URL, so a +// caller may only move the provider to another URL by entering the secret +// again, and tokens issued by the old instance are dropped. +const bindStoredGitlabSecrets = async ( + gitlabId: string, + input: Partial, +) => { + if (gitlabTargetFields.every((field) => input[field] === undefined)) { + return {}; + } + const current = await db.query.gitlab.findFirst({ + where: eq(gitlab.gitlabId, gitlabId), + }); + if (!current) { + return {}; + } + + const targets = Object.fromEntries( + gitlabTargetFields.map((field) => [ + field, + [ + input[field] === undefined ? current[field] : input[field], + current[field], + ] as const, + ]), + ); + if (current.secret && secretUpdateValue(input.secret) === undefined) { + assertStoredSecretTargetUnchanged("GitLab application secret", targets); + } + + const urlChanged = changedSecretTargetFields(targets).some( + (field) => field !== "applicationId", + ); + return urlChanged + ? { + accessToken: input.accessToken ?? null, + refreshToken: input.refreshToken ?? null, + expiresAt: input.expiresAt ?? null, + } + : {}; +}; + export const updateGitlab = async ( gitlabId: string, input: Partial, ) => { - const { webhookSecret, ...gitlabInput } = input; + const { webhookSecret, secret, ...gitlabInput } = input; const nextWebhookSecret = secretUpdateValue(webhookSecret); + const nextSecret = secretUpdateValue(secret); + const tokenReset = await bindStoredGitlabSecrets(gitlabId, input); return await db .update(gitlab) .set({ ...gitlabInput, + ...tokenReset, + ...(nextSecret !== undefined && { secret: nextSecret }), ...(nextWebhookSecret !== undefined && { webhookSecret: nextWebhookSecret, }), diff --git a/packages/server/src/services/notification.ts b/packages/server/src/services/notification.ts index b8128d12c7..f95cf26c6b 100644 --- a/packages/server/src/services/notification.ts +++ b/packages/server/src/services/notification.ts @@ -38,6 +38,7 @@ import { teams, telegram, } from "@dokploy/server/db/schema"; +import { assertStoredSecretTargetUnchanged } from "@dokploy/server/utils/security/redaction"; import { TRPCError } from "@trpc/server"; import { eq } from "drizzle-orm"; import type { z } from "zod"; @@ -73,6 +74,14 @@ const normalizeBaseUrlUpdateValue = ( : undefined; }; +// Rows saved before targets were normalized must still match when the form +// sends their stored value back unchanged. +const nextTargetValue = ( + raw: T | undefined, + normalized: T | undefined, + stored: T, +) => (normalized === undefined || raw === stored ? stored : normalized); + export const createSlackNotification = async ( input: z.infer, organizationId: string, @@ -425,6 +434,26 @@ export const createEmailNotification = async ( export const updateEmailNotification = async ( input: z.infer, ) => { + const smtpServer = input.smtpServer + ? normalizeNotificationSmtpHost(input.smtpServer) + : undefined; + const password = notificationSecretUpdateValue(input.password); + if (password === undefined) { + const current = await db.query.email.findFirst({ + where: eq(email.emailId, input.emailId), + }); + if (current?.password) { + assertStoredSecretTargetUnchanged("SMTP password", { + smtpServer: [ + nextTargetValue(input.smtpServer, smtpServer, current.smtpServer), + current.smtpServer, + ], + smtpPort: [input.smtpPort ?? current.smtpPort, current.smtpPort], + username: [input.username ?? current.username, current.username], + }); + } + } + await db.transaction(async (tx) => { const newDestination = await tx .update(notifications) @@ -454,12 +483,10 @@ export const updateEmailNotification = async ( await tx .update(email) .set({ - smtpServer: input.smtpServer - ? normalizeNotificationSmtpHost(input.smtpServer) - : undefined, + smtpServer, smtpPort: input.smtpPort, username: input.username, - password: notificationSecretUpdateValue(input.password), + password, fromAddress: input.fromAddress, toAddresses: input.toAddresses, }) @@ -625,6 +652,25 @@ export const createGotifyNotification = async ( export const updateGotifyNotification = async ( input: z.infer, ) => { + const serverUrl = normalizeBaseUrlUpdateValue( + input.serverUrl, + "Gotify server URL", + ); + const appToken = notificationSecretUpdateValue(input.appToken); + if (appToken === undefined) { + const current = await db.query.gotify.findFirst({ + where: eq(gotify.gotifyId, input.gotifyId), + }); + if (current?.appToken) { + assertStoredSecretTargetUnchanged("Gotify app token", { + serverUrl: [ + nextTargetValue(input.serverUrl, serverUrl, current.serverUrl), + current.serverUrl, + ], + }); + } + } + await db.transaction(async (tx) => { const newDestination = await tx .update(notifications) @@ -654,11 +700,8 @@ export const updateGotifyNotification = async ( await tx .update(gotify) .set({ - serverUrl: normalizeBaseUrlUpdateValue( - input.serverUrl, - "Gotify server URL", - ), - appToken: notificationSecretUpdateValue(input.appToken), + serverUrl, + appToken, priority: input.priority, decoration: input.decoration, }) @@ -726,6 +769,25 @@ export const createNtfyNotification = async ( export const updateNtfyNotification = async ( input: z.infer, ) => { + const serverUrl = normalizeBaseUrlUpdateValue( + input.serverUrl, + "ntfy server URL", + ); + const accessToken = notificationOptionalSecretUpdateValue(input.accessToken); + if (accessToken === undefined) { + const current = await db.query.ntfy.findFirst({ + where: eq(ntfy.ntfyId, input.ntfyId), + }); + if (current?.accessToken) { + assertStoredSecretTargetUnchanged("ntfy access token", { + serverUrl: [ + nextTargetValue(input.serverUrl, serverUrl, current.serverUrl), + current.serverUrl, + ], + }); + } + } + await db.transaction(async (tx) => { const newDestination = await tx .update(notifications) @@ -755,12 +817,9 @@ export const updateNtfyNotification = async ( await tx .update(ntfy) .set({ - serverUrl: normalizeBaseUrlUpdateValue( - input.serverUrl, - "ntfy server URL", - ), + serverUrl, topic: input.topic, - accessToken: notificationOptionalSecretUpdateValue(input.accessToken), + accessToken, priority: input.priority, }) .where(eq(ntfy.ntfyId, input.ntfyId)); @@ -826,6 +885,26 @@ export const createCustomNotification = async ( export const updateCustomNotification = async ( input: z.infer, ) => { + const endpoint = normalizeWebhookUpdateValue( + input.endpoint, + "Custom notification endpoint", + { allowPrivateNetwork: false }, + ); + const headers = notificationHeadersUpdateValue(input.headers); + if (headers === undefined) { + const current = await db.query.custom.findFirst({ + where: eq(custom.customId, input.customId), + }); + if (current?.headers && Object.keys(current.headers).length > 0) { + assertStoredSecretTargetUnchanged("custom notification headers", { + endpoint: [ + nextTargetValue(input.endpoint, endpoint, current.endpoint), + current.endpoint, + ], + }); + } + } + await db.transaction(async (tx) => { const newDestination = await tx .update(notifications) @@ -855,12 +934,8 @@ export const updateCustomNotification = async ( await tx .update(custom) .set({ - endpoint: normalizeWebhookUpdateValue( - input.endpoint, - "Custom notification endpoint", - { allowPrivateNetwork: false }, - ), - headers: notificationHeadersUpdateValue(input.headers), + endpoint, + headers, }) .where(eq(custom.customId, input.customId)); diff --git a/packages/server/src/services/registry.ts b/packages/server/src/services/registry.ts index 9292f98e9f..3b6e16eb24 100644 --- a/packages/server/src/services/registry.ts +++ b/packages/server/src/services/registry.ts @@ -4,7 +4,11 @@ import { execAsync, execAsyncRemote, } from "@dokploy/server/utils/process/execAsync"; -import { redactSecretFields } from "@dokploy/server/utils/security/redaction"; +import { + assertStoredSecretTargetUnchanged, + redactSecretFields, + secretUpdateValue, +} from "@dokploy/server/utils/security/redaction"; import { TRPCError } from "@trpc/server"; import { eq } from "drizzle-orm"; import type { z } from "zod"; @@ -129,10 +133,22 @@ export const updateRegistry = async ( ) => { try { assertCloudRegistryServer(registryData.serverId); + const password = secretUpdateValue(registryData.password); + if (password === undefined) { + const current = await findRegistryById(registryId); + assertStoredSecretTargetUnchanged("registry password", { + registryUrl: [ + registryData.registryUrl ?? current.registryUrl, + current.registryUrl, + ], + username: [registryData.username ?? current.username, current.username], + }); + } const rows = await db .update(registry) .set({ ...registryData, + password, }) .where(eq(registry.registryId, registryId)) .returning(); diff --git a/packages/server/src/services/vault-provider.ts b/packages/server/src/services/vault-provider.ts index 6bd7dfa120..0285291d6d 100644 --- a/packages/server/src/services/vault-provider.ts +++ b/packages/server/src/services/vault-provider.ts @@ -6,6 +6,7 @@ import { type VaultProviderConfig, vaultProvider, } from "@dokploy/server/db/schema"; +import { assertStoredSecretTargetUnchanged } from "@dokploy/server/utils/security/redaction"; import { getVaultClient } from "@dokploy/server/utils/vault"; import { TRPCError } from "@trpc/server"; import { eq } from "drizzle-orm"; @@ -19,14 +20,28 @@ const SENSITIVE_FIELDS: Record = { hashicorp: ["token"], infisical: ["clientSecret"], - aws: ["secretAccessKey"], - "aws-parameter-store": ["secretAccessKey"], + aws: ["accessKeyId", "secretAccessKey"], + "aws-parameter-store": ["accessKeyId", "secretAccessKey"], doppler: ["serviceToken"], azure: ["clientSecret"], scaleway: ["secretKey"], phase: ["token"], }; +const SECRET_TARGET_FIELDS: Record< + VaultProviderConfig["providerType"], + string[] +> = { + hashicorp: ["url"], + infisical: ["siteUrl", "clientId"], + aws: ["region", "endpoint", "accessKeyId"], + "aws-parameter-store": ["region", "endpoint", "accessKeyId"], + doppler: [], + azure: ["vaultUri", "tenantId", "clientId"], + scaleway: ["apiUrl"], + phase: ["apiUrl"], +}; + export const maskVaultProviderConfig = ( config: VaultProviderConfig, ): VaultProviderConfig => { @@ -44,6 +59,8 @@ export const mergeVaultProviderConfig = ( existing: VaultProviderConfig, ): VaultProviderConfig => { const merged: Record = { ...incoming }; + const stored = existing as Record; + let keepsStoredSecret = false; for (const field of SENSITIVE_FIELDS[incoming.providerType]) { if (merged[field] === VAULT_SECRET_MASK) { if (incoming.providerType !== existing.providerType) { @@ -53,9 +70,21 @@ export const mergeVaultProviderConfig = ( "Credentials must be re-entered when changing the provider type", }); } - merged[field] = (existing as Record)[field]; + merged[field] = stored[field]; + keepsStoredSecret = true; } } + if (keepsStoredSecret) { + assertStoredSecretTargetUnchanged( + "vault provider credentials", + Object.fromEntries( + SECRET_TARGET_FIELDS[incoming.providerType].map((field) => [ + field, + [merged[field], stored[field]] as const, + ]), + ), + ); + } return merged as VaultProviderConfig; }; diff --git a/packages/server/src/utils/security/redaction.ts b/packages/server/src/utils/security/redaction.ts index 11b16e277c..454c16db20 100644 --- a/packages/server/src/utils/security/redaction.ts +++ b/packages/server/src/utils/security/redaction.ts @@ -1,3 +1,5 @@ +import { TRPCError } from "@trpc/server"; + export const REDACTED_SECRET_VALUE = "__DOKPLOY_REDACTED_SECRET__"; const MCP_REDACTED_SECRET_VALUE = "[REDACTED]"; @@ -453,6 +455,39 @@ export const secretUpdateValue = (value: unknown) => { return value; }; +type SecretTargetFields = Record< + string, + readonly [next: unknown, stored: unknown] +>; + +const isUnsetSecretTargetValue = (value: unknown) => + value === undefined || value === null || value === ""; + +export const changedSecretTargetFields = (fields: SecretTargetFields) => + Object.entries(fields) + .filter(([, [next, stored]]) => + isUnsetSecretTargetValue(next) && isUnsetSecretTargetValue(stored) + ? false + : next !== stored, + ) + .map(([field]) => field); + +// A kept stored secret may only travel to the endpoint and identity it was +// saved with. Otherwise a caller who can edit the record but not read the +// secret could point it at a host they control and capture it. +export const assertStoredSecretTargetUnchanged = ( + secretLabel: string, + fields: SecretTargetFields, +) => { + const changed = changedSecretTargetFields(fields); + if (changed.length > 0) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: `Re-enter the ${secretLabel} to change ${changed.join(", ")}`, + }); + } +}; + export const preserveSecretPlaceholderFields = < TUpdate extends object, TCurrent extends object,