fix: share global schedules across org admins/owners#4334
fix: share global schedules across org admins/owners#4334ne1i wants to merge 1 commit intoDokploy:canaryfrom
Conversation
dosubot
Bot
added
size:L
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| await findMemberByUserId( | ||
| existingSchedule.userId, | ||
| ctx.session.activeOrganizationId, | ||
| ); |
There was a problem hiding this comment.
findMemberByUserId throws UNAUTHORIZED when the target user is not found in the org. If a user who created a dokploy-server schedule leaves the organisation, this call throws and blocks admins from updating, deleting, viewing (one), or running that schedule — the schedule becomes a ghost that keeps executing but can never be managed. The intent is to verify cross-org access, but the current implementation conflates "schedule owner is in-org" with "the operation is permitted". An admin should still be able to delete a schedule whose creator has since departed. Consider replacing the throw with a simple org-membership check and allowing the operation to proceed (or explicitly choosing to block it on departure), rather than silently making the schedule unmanageable.
| const currentMember = await findMemberByUserId( | ||
| ctx.user.id, | ||
| ctx.session.activeOrganizationId, | ||
| ); | ||
| if ( | ||
| existingSchedule.scheduleType === "dokploy-server" && | ||
| existingSchedule.userId && | ||
| existingSchedule.userId !== ctx.user.id | ||
| currentMember.role !== "owner" && | ||
| currentMember.role !== "admin" | ||
| ) { | ||
| throw new TRPCError({ | ||
| code: "UNAUTHORIZED", | ||
| message: "You can only manage your own host-level schedules.", | ||
| message: | ||
| "You can only manage your own host-level schedules.", | ||
| }); | ||
| } |
There was a problem hiding this comment.
The pre-existing owner/admin guard at line 139 already calls findMemberByUserId(ctx.user.id, ...) and throws if the user is not an owner or admin. By the time execution reaches this new inner block (which only triggers when userId !== ctx.user.id), only owners/admins can be here — so the call on line 172 and the role check on lines 176–185 always pass and add an unnecessary extra DB round-trip. The same pattern repeats in delete (lines 280–293) and runManually (lines 505–518).
| listWhere = and( | ||
| eq(schedules.scheduleType, "dokploy-server"), | ||
| inArray(schedules.userId, userIds), | ||
| ); |
There was a problem hiding this comment.
inArray with a potentially empty array
If orgMembers is somehow empty, inArray(schedules.userId, []) generates IN () which is invalid SQL and will throw a database error. In practice orgMembers always contains at least the calling admin, but adding an early-return guard (if (userIds.length === 0) return []) would make this robust and self-documenting.
Global (dokploy-server) schedules were scoped to the creator's user ID, making them invisible to other admins and owners in the same organization. This was inconsistent with server and application schedules which are org-scoped. - schedule.list: admins/owners now see all dokploy-server schedules from every member in the org, not just their own - schedule.update/delete/one/runManually: admins/owners can now manage dokploy-server schedules created by other org members - Use direct DB query instead of findMemberByUserId for the schedule owner check so schedules remain manageable after the creator leaves the org
ne1i
force-pushed
the
fix/global-schedules-org-sharing
branch
from
f43aa6e to
057200b
Compare
| if ( | ||
| existingSchedule.scheduleType === "dokploy-server" && | ||
| existingSchedule.userId && | ||
| existingSchedule.userId !== ctx.user.id | ||
| ) { | ||
| // Admin/owner role already verified above. | ||
| // Don't use findMemberByUserId here — it would throw if the | ||
| // schedule owner left the org, making the schedule unmanageable. | ||
| const scheduleOwnerInOrg = await db.query.member.findFirst({ | ||
| where: and( | ||
| eq(member.userId, existingSchedule.userId), | ||
| eq(member.organizationId, ctx.session.activeOrganizationId), | ||
| ), | ||
| columns: { id: true }, | ||
| }); | ||
| if (scheduleOwnerInOrg) { | ||
| // Owner is still in org — allowed. | ||
| } | ||
| // If owner left the org, still allow the admin/owner to manage it. | ||
| } | ||
| } |
There was a problem hiding this comment.
The scheduleOwnerInOrg lookup is performed but its result is never used to restrict access: the if (scheduleOwnerInOrg) { /* allowed */ } body is empty and there is no else { throw } branch. Execution always falls through to updateSchedule(input) regardless of whether the schedule owner is in the org or not. This means the only guard remaining is the admin/owner role check — org-scoping for dokploy-server schedules has been fully removed.
In a multi-org self-hosted deployment an admin from org A can update, delete, view (one), and run any dokploy-server schedule created by a user in org B by knowing its scheduleId. The same dead-code pattern repeats in delete (lines 271-288), one (lines 436-445), and runManually (lines 500-517).
If cross-org access is intentionally accepted, remove the dead lookup entirely (it wastes a DB round-trip). If within-org + orphaned-schedule access is the intended behaviour, add an explicit throw when the owner is found to belong to a different org's member record. As written, the DB query is misleading: it implies a check is being made while actually allowing everything.
1 participant
Closes #4300
dokploy-serverschedules were scoped to the creating user's ID in both the list query and the update/delete/one/runManually access checks. This meant an admin couldn't see or manage global schedules created by the owner (and vice versa), even though both are in the same org.Server schedules and application schedules don't have this restriction — they're org-scoped. This change brings global schedules in line.
schedule.list: admins and owners now get all
dokploy-serverschedules for every member in the active org. Members still only see their own.schedule.update / delete / one / runManually: instead of blocking when
schedule.userId !== ctx.user.id, admins and owners can now operate on anydokploy-serverschedule whose owner belongs to the org (verified viafindMemberByUserId). Members still only get their own.Greptile Summary
This PR replaces the per-user ownership gate on
dokploy-serverschedules with a role-based org-scoped check, correctly fixing the listing path for admins/owners. However, inupdate,delete,one, andrunManuallythe newly introducedscheduleOwnerInOrgquery result is never acted upon — theif (scheduleOwnerInOrg) { /* allowed */ }body is empty and there is noelse { throw }branch, so org-boundary enforcement has been silently removed for those four operations.Confidence Score: 3/5
Not safe to merge as-is — the authorization check introduced for cross-org scoping is dead code, leaving multi-org self-hosted deployments without an org boundary on dokploy-server schedule mutations.
One P1 finding: the scheduleOwnerInOrg guard is a no-op across four procedures, meaning the org boundary was removed rather than relaxed. The listing fix is correct. Score sits at 3 because the same dead-code pattern is repeated in four separate procedures, indicating a systemic issue rather than a one-off oversight.
apps/dokploy/server/api/routers/schedule.ts — specifically the authorization blocks in update (lines 167-187), delete (271-288), one (436-445), and runManually (500-517).
Security Review
update,delete,one,runManually): ThescheduleOwnerInOrgmembership check is computed but its result is unused. Any admin/owner in org A can modify or execute adokploy-serverschedule created by a user in org B by knowing itsscheduleId. Mitigated in practice by theIS_CLOUDguard that blocksdokploy-serverschedules in the hosted environment, but the flaw exists in all self-hosted multi-org deployments.Reviews (2): Last reviewed commit: "fix: share global schedules across org a..." | Re-trigger Greptile