fix(permissions): decouple env editing and domain service list from "Create Services" permission#4055
Conversation
…Create Services" permission The "Create Services" (`canCreateServices`) permission was incorrectly required for two unrelated operations: 1. Loading compose service names in the domain form dropdown (`compose.loadServices` checked `service: ["create"]` for a read-only operation). Changed to `service: ["read"]`. 2. Editing environment variables on database and compose services. The `ShowEnvironment` component used the generic `update` mutation (which checks `service: ["create"]`) instead of the dedicated `saveEnvironment` endpoints (which correctly check `envVars: ["write"]`). Changes: - compose.loadServices: service: ["create"] → service: ["read"] - Omit `env` from update schemas for postgres, mysql, mariadb, redis, mongo, and compose to prevent env updates through the update endpoint - Add `saveEnvironment` endpoint to compose router (matching the pattern already used by all 5 database routers) - Update ShowEnvironment component to use `saveEnvironment` mutations with correctly typed per-service payloads Made-with: Cursor
|
Related Documentation 1 document(s) may need updating based on files changed in this PR: Dokploy's Space README
|
|
LGTM 👍 This is a necessary fix - ran into this exact permission issue ourselves. Would love to see this land in stable. @nizepart appreciate you! |
mahdirajaee
left a comment
There was a problem hiding this comment.
This is a well-scoped permission fix. Changing compose.loadServices from service: ["create"] to service: ["read"] is clearly correct — listing service names from parsed YAML is a read-only operation and should never have required create permission. The new dedicated saveEnvironment endpoint for compose follows the exact pattern already established by postgres, redis, mysql, mariadb, and mongo routers, which makes this consistent and easy to audit.
The approach of omitting env from all apiUpdate* schemas is a good defensive measure — it ensures env modifications can only go through the saveEnvironment path with envVars: ["write"] permission, eliminating any bypass via the generic update mutation. I verified that no other frontend components pass env through those update mutations, so this is a safe change with no regression risk.
One small thing worth noting: the ShowEnvironment component's as const assertion on the payloadMap and the cast on mutateAsync could be slightly cleaner with a generic helper, but that's purely stylistic and not a blocker. The permission model is correct and backward compatible — users who previously had both permissions will see no change, while users who only had env write access will now correctly be able to edit environment variables.
Summary
Fixes #4052
The "Create Services" (
canCreateServices) permission was incorrectly required for two unrelated operations:compose.loadServicescheckedservice: ["create"]for a read-only operation (parsing YAML to list service names). Changed toservice: ["read"].ShowEnvironmentcomponent used the genericupdatemutation (gated byservice: ["create"]) instead of the dedicatedsaveEnvironmentendpoints (correctly gated byenvVars: ["write"]).Changes
apps/dokploy/server/api/routers/compose.tsloadServices:service: ["create"]→service: ["read"]; addedsaveEnvironmentendpoint withenvVars: ["write"]packages/server/src/db/schema/compose.tsenvfromapiUpdateCompose; addapiSaveEnvironmentVariablesComposeschemapackages/server/src/db/schema/{postgres,mysql,mariadb,redis,mongo}.tsenvfrom update schemas to force usage ofsaveEnvironmentapps/dokploy/components/.../show-enviroment.tsxsaveEnvironmentmutations with per-service typed payloadsGreptile Summary
This PR correctly decouples two read/write operations from the
canCreateServices(service: ["create"]) permission by routing them through their appropriate permission gates.What changed:
compose.loadServicesnow requiresservice: ["read"]instead ofservice: ["create"]— appropriate since parsing YAML to list service names is a read-only operationsaveEnvironmenttRPC procedure is added to the compose router, gated byenvVars: ["write"], exactly matching the pre-existing pattern on postgres, redis, mysql, mariadb, and mongo routersenvis omitted from all sixapiUpdate*schemas (compose, postgres, redis, mysql, mariadb, mongo), enforcing that env changes go exclusively through the dedicatedsaveEnvironmentendpointShowEnvironmentnow dispatches per-servicesaveEnvironmentmutations with typed payloads instead of the catch-allupdatemutationReview notes:
importendpoint incompose.tsstill callsupdateCompose()directly with anenvfield. This is intentional and safe — the internalupdateCompose(id, data: Partial<Compose>)function is not constrained byapiUpdateComposeand is protected byservice: ["create"]permission, which is correct for template imports.apiSaveEnvironmentVariablesComposeschema mirrors the exact.pick().required()pattern used by all sibling service schemas..updateon these services passenvin their payloads, so there is no regression risk from omittingenvfrom the update schemas.Confidence Score: 5/5
saveEnvironmentendpoint for compose follows the exact same structure as the already-shipped endpoints for postgres/redis/mysql/mariadb/mongo. The schema omissions forenvin the update validators are safe because no other frontend components passenvthrough those update mutations. The fix forloadServicesis a straightforward and clearly correct permission downgrade.Reviews (1): Last reviewed commit: "fix(permissions): decouple env editing a..." | Re-trigger Greptile
(2/5) Greptile learns from your feedback when you react with thumbs up/down!