fix(security): block FK repoint and add access check on git provider update

apps/dokploy/__test__/git-provider/git-provider-update-fk-repoint.test.ts

@@ -0,0 +1,134 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+// Service-level guard: the update* services must never write the identity/FK
+// columns (giteaId/githubId/gitlabId and gitProviderId). Repointing gitProviderId
+// at another org's git_provider is the credential-leak mechanism. We capture the
+// argument passed to drizzle's `.set()` and assert the FK/PK keys are absent.
+
+const capture = vi.hoisted(() => ({ setArgs: [] as unknown[] }));
+const setMock = vi.hoisted(() =>
+	vi.fn((values: unknown) => {
+		capture.setArgs.push(values);
+		return { where: () => ({ returning: () => Promise.resolve([{}]) }) };
+	}),
+);
+const updateMock = vi.hoisted(() => vi.fn(() => ({ set: setMock })));
+
+vi.mock("@dokploy/server/db", () => ({
+	db: {
+		update: updateMock,
+	},
+}));
+
+import { updateGitea } from "@dokploy/server/services/gitea";
+import { updateGithub } from "@dokploy/server/services/github";
+import { updateGitlab } from "@dokploy/server/services/gitlab";
+
+beforeEach(() => {
+	vi.clearAllMocks();
+	capture.setArgs.length = 0;
+});
+
+const hijackPayload = {
+	giteaId: "gitea-victim",
+	gitProviderId: "gp-attacker",
+	giteaUrl: "https://evil.gitea.com",
+	clientSecret: "stolen-secret",
+	accessToken: "stolen-at",
+	refreshToken: "stolen-rt",
+};
+
+describe("updateGitea — FK/PK repoint guard", () => {
+	it("strips giteaId and gitProviderId from the SET clause", async () => {
+		await updateGitea("gitea-victim", hijackPayload);
+
+		expect(setMock).toHaveBeenCalledTimes(1);
+		const setArg = capture.setArgs[0] as Record<string, unknown>;
+		expect(setArg).not.toHaveProperty("giteaId");
+		expect(setArg).not.toHaveProperty("gitProviderId");
+		expect(setArg.giteaUrl).toBe("https://evil.gitea.com");
+		expect(setArg.clientSecret).toBe("stolen-secret");
+	});
+
+	it("preserves OAuth secret columns when they are provided", async () => {
+		await updateGitea("gitea-victim", {
+			accessToken: "at",
+			refreshToken: "rt",
+			expiresAt: 123,
+		});
+
+		const setArg = capture.setArgs[0] as Record<string, unknown>;
+		expect(setArg.accessToken).toBe("at");
+		expect(setArg.refreshToken).toBe("rt");
+		expect(setArg.expiresAt).toBe(123);
+		expect(setArg).not.toHaveProperty("gitProviderId");
+		expect(setArg).not.toHaveProperty("giteaId");
+	});
+
+	it("only sends provided editable columns (omitted fields are not sent)", async () => {
+		await updateGitea("gitea-victim", {
+			giteaUrl: "https://gitea.com",
+		});
+
+		const setArg = capture.setArgs[0] as Record<string, unknown>;
+		expect(Object.keys(setArg)).toEqual(["giteaUrl"]);
+	});
+});
+
+describe("updateGithub — FK/PK repoint guard", () => {
+	it("strips githubId and gitProviderId from the SET clause", async () => {
+		await updateGithub("gh-victim", {
+			githubId: "gh-victim",
+			gitProviderId: "gp-attacker",
+			githubAppName: "stolen-app",
+			githubUrl: "https://github.corp.evil.com",
+			githubClientSecret: "stolen-secret",
+		});
+
+		expect(setMock).toHaveBeenCalledTimes(1);
+		const setArg = capture.setArgs[0] as Record<string, unknown>;
+		expect(setArg).not.toHaveProperty("githubId");
+		expect(setArg).not.toHaveProperty("gitProviderId");
+		expect(setArg.githubAppName).toBe("stolen-app");
+		expect(setArg.githubUrl).toBe("https://github.corp.evil.com");
+		expect(setArg.githubClientSecret).toBe("stolen-secret");
+	});
+
+	it("only sends provided editable columns", async () => {
+		await updateGithub("gh-victim", {
+			githubAppName: "renamed-app",
+		});
+
+		const setArg = capture.setArgs[0] as Record<string, unknown>;
+		expect(Object.keys(setArg)).toEqual(["githubAppName"]);
+	});
+});
+
+describe("updateGitlab — FK/PK repoint guard", () => {
+	it("strips gitlabId and gitProviderId from the SET clause", async () => {
+		await updateGitlab("gl-victim", {
+			gitlabId: "gl-victim",
+			gitProviderId: "gp-attacker",
+			gitlabUrl: "https://gitlab.corp.evil.com",
+			secret: "stolen-secret",
+			accessToken: "stolen-at",
+		});
+
+		expect(setMock).toHaveBeenCalledTimes(1);
+		const setArg = capture.setArgs[0] as Record<string, unknown>;
+		expect(setArg).not.toHaveProperty("gitlabId");
+		expect(setArg).not.toHaveProperty("gitProviderId");
+		expect(setArg.gitlabUrl).toBe("https://gitlab.corp.evil.com");
+		expect(setArg.secret).toBe("stolen-secret");
+		expect(setArg.accessToken).toBe("stolen-at");
+	});
+
+	it("only sends provided editable columns", async () => {
+		await updateGitlab("gl-victim", {
+			gitlabUrl: "https://gitlab.com",
+		});
+
+		const setArg = capture.setArgs[0] as Record<string, unknown>;
+		expect(Object.keys(setArg)).toEqual(["gitlabUrl"]);
+	});
+});

apps/dokploy/__test__/git-provider/git-provider-update-router.test.ts

@@ -0,0 +1,312 @@
+import { TRPCError } from "@trpc/server";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+// Router-level IDOR guard for the *.update handlers.
+//
+// Models the exploit from the bug report: an attacker who has
+// `gitProviders:create` in their own org (so withPermission passes) and knows a
+// victim's <provider>Id calls <provider>.update with the victim id plus their own
+// gitProviderId. The handler must resolve the row's owning git_provider via
+// find*ById and run assertGitProviderAccess BEFORE mutating, so:
+//   - cross-org updates are rejected (NOT_FOUND);
+//   - updateGitProvider is no longer fed organizationId (cannot move a provider
+//     across orgs);
+//   - the service layer never repoints gitProviderId (FK-repoint mechanism closed).
+
+const mockFindGiteaById = vi.hoisted(() => vi.fn());
+const mockFindGithubById = vi.hoisted(() => vi.fn());
+const mockFindGitlabById = vi.hoisted(() => vi.fn());
+const mockUpdateGitProvider = vi.hoisted(() =>
+	vi.fn().mockResolvedValue(undefined),
+);
+
+vi.mock("@dokploy/server", async (importOriginal) => {
+	const actual = await importOriginal<typeof import("@dokploy/server")>();
+	return {
+		...actual,
+		findGiteaById: mockFindGiteaById,
+		findGithubById: mockFindGithubById,
+		findGitlabById: mockFindGitlabById,
+		updateGitProvider: mockUpdateGitProvider,
+	};
+});
+
+vi.mock("@dokploy/server/services/permission", async (importOriginal) => {
+	const actual =
+		await importOriginal<
+			typeof import("@dokploy/server/services/permission")
+		>();
+	return { ...actual, checkPermission: vi.fn().mockResolvedValue(undefined) };
+});
+
+const mockHasValidLicense = vi.hoisted(() => vi.fn().mockResolvedValue(false));
+vi.mock("@dokploy/server/services/proprietary/license-key", () => ({
+	hasValidLicense: mockHasValidLicense,
+}));
+
+// Real `updateGitProvider` (kept real) and `assertGitProviderAccess`/`update*`
+// (kept real via the barrel) run against this db mock. `update` captures the
+// `.set()` arg so we can prove the service never repoints the FK, while `query`
+// feeds `getAccessibleGitProviderIds` for the same-org success path.
+const mocks = vi.hoisted(() => {
+	const capture = { setArgs: [] as unknown[] };
+	const dbSetMock = vi.fn((values: unknown) => {
+		capture.setArgs.push(values);
+		return { where: () => ({ returning: () => Promise.resolve([{}]) }) };
+	});
+	const gitProviderFindMany = vi.fn(() =>
+		Promise.resolve([
+			{
+				gitProviderId: "gp-1",
+				userId: "user-1",
+				sharedWithOrganization: false,
+			},
+		]),
+	);
+	const memberFindFirst = vi.fn(() =>
+		Promise.resolve({ role: "owner", accessedGitProviders: [] }),
+	);
+	const db = {
+		update: vi.fn(() => ({ set: dbSetMock })),
+		query: {
+			gitProvider: {
+				findMany: gitProviderFindMany,
+				findFirst: vi.fn(() => Promise.resolve(undefined)),
+			},
+			member: {
+				findFirst: memberFindFirst,
+				findMany: vi.fn(() => Promise.resolve([])),
+			},
+		},
+	};
+	return { capture, dbSetMock, db };
+});
+
+vi.mock("@dokploy/server/db", () => ({ db: mocks.db }));
+
+vi.mock("@dokploy/server/lib/auth", () => ({
+	validateRequest: vi.fn().mockResolvedValue({ session: null, user: null }),
+}));
+
+const mockAudit = vi.hoisted(() => vi.fn().mockResolvedValue(undefined));
+vi.mock("@/server/api/utils/audit", () => ({ audit: mockAudit }));
+
+import { giteaRouter } from "@/server/api/routers/gitea";
+import { githubRouter } from "@/server/api/routers/github";
+import { gitlabRouter } from "@/server/api/routers/gitlab";
+
+const ORG = "org-1";
+const USER = "user-1";
+
+const ownProvider = {
+	gitProviderId: "gp-1",
+	organizationId: ORG,
+	userId: USER,
+	name: "mine",
+	providerType: "gitea",
+	createdAt: "",
+	sharedWithOrganization: false,
+};
+
+const foreignProvider = {
+	gitProviderId: "gp-victim",
+	organizationId: "org-2",
+	userId: "user-2",
+	name: "victim",
+	providerType: "gitea",
+	createdAt: "",
+	sharedWithOrganization: false,
+};
+
+const buildCtx = () =>
+	({
+		session: { userId: USER, activeOrganizationId: ORG },
+		user: { id: USER, email: "owner@test.com", role: "owner" },
+		db: mocks.db,
+		req: {},
+		res: {},
+	}) as unknown as Parameters<(typeof giteaRouter)["createCaller"]>[0];
+
+beforeEach(() => {
+	vi.clearAllMocks();
+	mocks.capture.setArgs.length = 0;
+	mockFindGiteaById.mockResolvedValue({
+		giteaId: "gitea-1",
+		gitProvider: ownProvider,
+	});
+	mockFindGithubById.mockResolvedValue({
+		githubId: "gh-1",
+		gitProvider: ownProvider,
+	});
+	mockFindGitlabById.mockResolvedValue({
+		gitlabId: "gl-1",
+		gitProvider: ownProvider,
+	});
+});
+
+describe("gitea.update — cross-org IDOR guard", () => {
+	const caller = () => giteaRouter.createCaller(buildCtx());
+
+	it("rejects a cross-org update (victim giteaId + attacker gitProviderId)", async () => {
+		mockFindGiteaById.mockResolvedValue({
+			giteaId: "gitea-victim",
+			gitProvider: foreignProvider,
+		});
+
+		const err = await caller()
+			.update({
+				giteaId: "gitea-victim",
+				gitProviderId: "gp-attacker",
+				name: "stolen",
+				giteaUrl: "https://gitea.com",
+			})
+			.catch((e) => e);
+
+		expect(err).toBeInstanceOf(TRPCError);
+		expect(err).toMatchObject({ code: "NOT_FOUND" });
+		expect(mockUpdateGitProvider).not.toHaveBeenCalled();
+		expect(mocks.dbSetMock).not.toHaveBeenCalled();
+		expect(mockAudit).not.toHaveBeenCalled();
+	});
+
+	it("does not pass organizationId to updateGitProvider", async () => {
+		await caller().update({
+			giteaId: "gitea-1",
+			gitProviderId: "gp-1",
+			name: "renamed",
+			giteaUrl: "https://gitea.com",
+		});
+
+		expect(mockUpdateGitProvider).toHaveBeenCalledWith("gp-1", {
+			name: "renamed",
+		});
+	});
+
+	it("does not repoint the gitea row's gitProviderId via the service", async () => {
+		await caller().update({
+			giteaId: "gitea-1",
+			gitProviderId: "gp-attacker",
+			name: "stolen",
+			giteaUrl: "https://gitea.com",
+		});
+
+		expect(mocks.dbSetMock).toHaveBeenCalledTimes(1);
+		const setArg = mocks.capture.setArgs[0] as Record<string, unknown>;
+		expect(setArg).not.toHaveProperty("giteaId");
+		expect(setArg).not.toHaveProperty("gitProviderId");
+		expect(setArg.giteaUrl).toBe("https://gitea.com");
+	});
+
+	it("succeeds for a legitimate same-org own-provider update", async () => {
+		const result = await caller().update({
+			giteaId: "gitea-1",
+			gitProviderId: "gp-1",
+			name: "renamed",
+			giteaUrl: "https://gitea.com",
+		});
+		expect(result).toEqual({ success: true });
+		expect(mockAudit).toHaveBeenCalledTimes(1);
+	});
+});
+
+describe("github.update — cross-org IDOR guard", () => {
+	const caller = () => githubRouter.createCaller(buildCtx());
+
+	it("rejects a cross-org update", async () => {
+		mockFindGithubById.mockResolvedValue({
+			githubId: "gh-victim",
+			gitProvider: foreignProvider,
+		});
+
+		await expect(
+			caller().update({
+				githubId: "gh-victim",
+				gitProviderId: "gp-attacker",
+				name: "stolen",
+				githubAppName: "stolen-app",
+			}),
+		).rejects.toMatchObject({ code: "NOT_FOUND" });
+
+		expect(mockUpdateGitProvider).not.toHaveBeenCalled();
+		expect(mocks.dbSetMock).not.toHaveBeenCalled();
+	});
+
+	it("does not pass organizationId to updateGitProvider", async () => {
+		await caller().update({
+			githubId: "gh-1",
+			gitProviderId: "gp-1",
+			name: "renamed",
+			githubAppName: "my-app",
+		});
+
+		expect(mockUpdateGitProvider).toHaveBeenCalledWith("gp-1", {
+			name: "renamed",
+		});
+	});
+
+	it("does not repoint the github row's gitProviderId via the service", async () => {
+		await caller().update({
+			githubId: "gh-1",
+			gitProviderId: "gp-attacker",
+			name: "stolen",
+			githubAppName: "stolen-app",
+		});
+
+		expect(mocks.dbSetMock).toHaveBeenCalledTimes(1);
+		const setArg = mocks.capture.setArgs[0] as Record<string, unknown>;
+		expect(setArg).not.toHaveProperty("githubId");
+		expect(setArg).not.toHaveProperty("gitProviderId");
+		expect(setArg.githubAppName).toBe("stolen-app");
+	});
+});
+
+describe("gitlab.update — cross-org IDOR guard", () => {
+	const caller = () => gitlabRouter.createCaller(buildCtx());
+
+	it("rejects a cross-org update", async () => {
+		mockFindGitlabById.mockResolvedValue({
+			gitlabId: "gl-victim",
+			gitProvider: foreignProvider,
+		});
+
+		await expect(
+			caller().update({
+				gitlabId: "gl-victim",
+				gitProviderId: "gp-attacker",
+				name: "stolen",
+				gitlabUrl: "https://gitlab.com",
+			}),
+		).rejects.toMatchObject({ code: "NOT_FOUND" });
+
+		expect(mockUpdateGitProvider).not.toHaveBeenCalled();
+		expect(mocks.dbSetMock).not.toHaveBeenCalled();
+	});
+
+	it("does not pass organizationId to updateGitProvider", async () => {
+		await caller().update({
+			gitlabId: "gl-1",
+			gitProviderId: "gp-1",
+			name: "renamed",
+			gitlabUrl: "https://gitlab.com",
+		});
+
+		expect(mockUpdateGitProvider).toHaveBeenCalledWith("gp-1", {
+			name: "renamed",
+		});
+	});
+
+	it("does not repoint the gitlab row's gitProviderId via the service", async () => {
+		await caller().update({
+			gitlabId: "gl-1",
+			gitProviderId: "gp-attacker",
+			name: "stolen",
+			gitlabUrl: "https://gitlab.com",
+		});
+
+		expect(mocks.dbSetMock).toHaveBeenCalledTimes(1);
+		const setArg = mocks.capture.setArgs[0] as Record<string, unknown>;
+		expect(setArg).not.toHaveProperty("gitlabId");
+		expect(setArg).not.toHaveProperty("gitProviderId");
+		expect(setArg.gitlabUrl).toBe("https://gitlab.com");
+	});
+});

apps/dokploy/server/api/routers/gitea.ts

@@ -187,10 +187,12 @@ export const giteaRouter = createTRPCRouter({
 	update: withPermission("gitProviders", "create")
 		.input(apiUpdateGitea)
 		.mutation(async ({ input, ctx }) => {
+			const gitea = await findGiteaById(input.giteaId);
+			await assertGitProviderAccess(ctx.session, gitea.gitProvider);
+
 			if (input.name) {
 				await updateGitProvider(input.gitProviderId, {
 					name: input.name,
-					organizationId: ctx.session.activeOrganizationId,
 				});
 
 				await updateGitea(input.giteaId, {

apps/dokploy/server/api/routers/github.ts

@@ -106,9 +106,11 @@ export const githubRouter = createTRPCRouter({
 	update: withPermission("gitProviders", "create")
 		.input(apiUpdateGithub)
 		.mutation(async ({ input, ctx }) => {
+			const github = await findGithubById(input.githubId);
+			await assertGitProviderAccess(ctx.session, github.gitProvider);
+
 			await updateGitProvider(input.gitProviderId, {
 				name: input.name,
-				organizationId: ctx.session.activeOrganizationId,
 			});
 
 			await updateGithub(input.githubId, {

apps/dokploy/server/api/routers/gitlab.ts

@@ -136,10 +136,12 @@ export const gitlabRouter = createTRPCRouter({
 	update: withPermission("gitProviders", "create")
 		.input(apiUpdateGitlab)
 		.mutation(async ({ input, ctx }) => {
+			const gitlab = await findGitlabById(input.gitlabId);
+			await assertGitProviderAccess(ctx.session, gitlab.gitProvider);
+
 			if (input.name) {
 				await updateGitProvider(input.gitProviderId, {
 					name: input.name,
-					organizationId: ctx.session.activeOrganizationId,
 				});
 
 				await updateGitlab(input.gitlabId, {

packages/server/src/services/gitea.ts

@@ -82,9 +82,14 @@ export const findGiteaById = async (giteaId: string) => {
 
 export const updateGitea = async (giteaId: string, input: Partial<Gitea>) => {
 	try {
+		const {
+			giteaId: _giteaId,
+			gitProviderId: _gitProviderId,
+			...editable
+		} = input;
 		const updateResult = await db
 			.update(gitea)
-			.set(input)
+			.set(editable)
 			.where(eq(gitea.giteaId, giteaId))
 			.returning();
 

packages/server/src/services/github.ts

@@ -68,11 +68,14 @@ export const updateGithub = async (
 	githubId: string,
 	input: Partial<Github>,
 ) => {
+	const {
+		githubId: _githubId,
+		gitProviderId: _gitProviderId,
+		...editable
+	} = input;
 	return await db
 		.update(github)
-		.set({
-			...input,
-		})
+		.set(editable)
 		.where(eq(github.githubId, githubId))
 		.returning()
 		.then((response) => response[0]);

packages/server/src/services/gitlab.ts

@@ -67,11 +67,14 @@ export const updateGitlab = async (
 	gitlabId: string,
 	input: Partial<Gitlab>,
 ) => {
+	const {
+		gitlabId: _gitlabId,
+		gitProviderId: _gitProviderId,
+		...editable
+	} = input;
 	return await db
 		.update(gitlab)
-		.set({
-			...input,
-		})
+		.set(editable)
 		.where(eq(gitlab.gitlabId, gitlabId))
 		.returning()
 		.then((response) => response[0]);