fix: use github owner login for webhook deploy matching (#4674)

* fix: use github owner login for webhook deploy matching * fix: prefer github owner name for webhook matching Что: - Инвертирован порядок fallback для GitHub webhook owner: сначала repository.owner.name, затем repository.owner.login. - Обновлен focused regression test для приоритета owner.name и fallback на owner.login. Зачем: - Выполнить maintainer review request в PR #4674 и сохранить совместимость deploy matching для payload без owner.name. Риски: - Не выявлены для push/tag matching; preview pull_request путь использует тот же helper, но отдельным PR-event тестом не покрыт. Проверки: - Команды и результаты: git diff --check -- apps/dokploy/pages/api/deploy/github.ts apps/dokploy/__test__/deploy/github-webhook-handler.test.ts - passed; CI=true corepack pnpm --dir apps/dokploy exec vitest --config __test__/vitest.config.ts run __test__/deploy/github-webhook-handler.test.ts --reporter=verbose - passed, 1 file / 4 tests; CI=true corepack pnpm exec biome check apps/dokploy/pages/api/deploy/github.ts apps/dokploy/__test__/deploy/github-webhook-handler.test.ts - exit 0, reported existing Number.parseInt radix info at github.ts:464; mandatory QA subagent Boyle - pass. - Ограничения: repo-wide format-and-lint, typecheck, build, and test не запускались для этого точечного review fix. What: - Inverted the GitHub webhook owner fallback order to prefer repository.owner.name before repository.owner.login. - Updated the focused regression test for owner.name precedence and owner.login fallback. Why: - Address the maintainer review request in PR #4674 while preserving deploy matching for payloads without owner.name. Risks: - None identified for push/tag matching; the preview pull_request path uses the same helper but is not covered by a dedicated PR-event test. Checks: - Commands and results: git diff --check -- apps/dokploy/pages/api/deploy/github.ts apps/dokploy/__test__/deploy/github-webhook-handler.test.ts - passed; CI=true corepack pnpm --dir apps/dokploy exec vitest --config __test__/vitest.config.ts run __test__/deploy/github-webhook-handler.test.ts --reporter=verbose - passed, 1 file / 4 tests; CI=true corepack pnpm exec biome check apps/dokploy/pages/api/deploy/github.ts apps/dokploy/__test__/deploy/github-webhook-handler.test.ts - exit 0, reported existing Number.parseInt radix info at github.ts:464; mandatory QA subagent Boyle - pass. - Limitations: repo-wide format-and-lint, typecheck, build, and test were not run for this targeted review fix.
2026-07-02 04:25:21 +02:00 · 2026-07-01 22:23:24 +03:00
parent d87229ccd3
commit f5ded8b273
2 changed files with 329 additions and 3 deletions
--- a/apps/dokploy/test/deploy/github-webhook-handler.test.ts
+++ b/apps/dokploy/test/deploy/github-webhook-handler.test.ts
@@ -0,0 +1,323 @@
+import type { NextApiRequest, NextApiResponse } from "next";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const mocks = vi.hoisted(() => ({
+	eq: vi.fn((field: string, value: unknown) => ({ field, value })),
+	and: vi.fn((...conditions: Array<{ field: string; value: unknown }>) => ({
+		conditions,
+	})),
+	githubFindFirst: vi.fn(),
+	applicationsFindMany: vi.fn(),
+	composeFindMany: vi.fn(),
+	queueAdd: vi.fn(),
+	verify: vi.fn(),
+	shouldDeploy: vi.fn(),
+}));
+
+vi.mock("drizzle-orm", () => ({
+	eq: mocks.eq,
+	and: mocks.and,
+}));
+
+vi.mock("@/server/db/schema", () => ({
+	applications: {
+		sourceType: "application.sourceType",
+		autoDeploy: "application.autoDeploy",
+		triggerType: "application.triggerType",
+		branch: "application.branch",
+		repository: "application.repository",
+		owner: "application.owner",
+		githubId: "application.githubId",
+		isPreviewDeploymentsActive: "application.isPreviewDeploymentsActive",
+	},
+	compose: {
+		sourceType: "compose.sourceType",
+		autoDeploy: "compose.autoDeploy",
+		triggerType: "compose.triggerType",
+		branch: "compose.branch",
+		repository: "compose.repository",
+		owner: "compose.owner",
+		githubId: "compose.githubId",
+	},
+	github: {
+		githubInstallationId: "github.githubInstallationId",
+	},
+}));
+
+vi.mock("@dokploy/server/db", () => ({
+	db: {
+		query: {
+			github: {
+				findFirst: mocks.githubFindFirst,
+			},
+			applications: {
+				findMany: mocks.applicationsFindMany,
+			},
+			compose: {
+				findMany: mocks.composeFindMany,
+			},
+		},
+	},
+}));
+
+vi.mock("@dokploy/server", () => ({
+	IS_CLOUD: false,
+	shouldDeploy: mocks.shouldDeploy,
+	checkUserRepositoryPermissions: vi.fn(),
+	createPreviewDeployment: vi.fn(),
+	createSecurityBlockedComment: vi.fn(),
+	findGithubById: vi.fn(),
+	findPreviewDeploymentByApplicationId: vi.fn(),
+	findPreviewDeploymentsByPullRequestId: vi.fn(),
+	getBitbucketHeaders: vi.fn(() => ({})),
+	removePreviewDeployment: vi.fn(),
+}));
+
+vi.mock("@octokit/webhooks", () => ({
+	Webhooks: vi.fn().mockImplementation(function Webhooks() {
+		return {
+			verify: mocks.verify,
+		};
+	}),
+}));
+
+vi.mock("@/server/queues/queueSetup", () => ({
+	myQueue: {
+		add: mocks.queueAdd,
+	},
+}));
+
+vi.mock("@/server/utils/deploy", () => ({
+	deploy: vi.fn(),
+}));
+
+import handler from "@/pages/api/deploy/github";
+
+const getConditionValue = (
+	where: { conditions?: Array<{ field: string; value: unknown }> } | undefined,
+	field: string,
+) => where?.conditions?.find((condition) => condition.field === field)?.value;
+
+const createResponse = () => {
+	const res = {
+		status: vi.fn(),
+		json: vi.fn(),
+	} as unknown as NextApiResponse & {
+		status: ReturnType<typeof vi.fn>;
+		json: ReturnType<typeof vi.fn>;
+	};
+
+	res.status.mockImplementation(() => res);
+	res.json.mockImplementation(() => res);
+
+	return res;
+};
+
+const createPushRequest = (
+	branch: string,
+	owner: { login?: string; name?: string } = { login: "agentHits" },
+) =>
+	({
+		headers: {
+			"x-hub-signature-256": "sha256=test-signature",
+			"x-github-event": "push",
+		},
+		body: {
+			installation: {
+				id: 12345,
+			},
+			ref: `refs/heads/${branch}`,
+			after: "abc123",
+			head_commit: {
+				message: "fix: trigger deployment",
+			},
+			commits: [
+				{
+					modified: ["src/index.ts"],
+				},
+			],
+			repository: {
+				name: "dokploy",
+				full_name: "agentHits/dokploy",
+				clone_url: "https://github.com/agentHits/dokploy.git",
+				html_url: "https://github.com/agentHits/dokploy",
+				owner,
+			},
+		},
+	}) as unknown as NextApiRequest;
+
+const createTagRequest = (tagName: string) => {
+	const req = createPushRequest("main") as unknown as {
+		body: { ref: string; head_commit: { message: string } };
+	};
+
+	req.body.ref = `refs/tags/${tagName}`;
+	req.body.head_commit.message = `release: ${tagName}`;
+
+	return req as unknown as NextApiRequest;
+};
+
+describe("GitHub app webhook auto-deploy", () => {
+	beforeEach(() => {
+		vi.clearAllMocks();
+		mocks.githubFindFirst.mockResolvedValue({
+			githubId: "github-provider-id",
+			githubInstallationId: 12345,
+			githubWebhookSecret: "webhook-secret",
+		});
+		mocks.verify.mockResolvedValue(true);
+		mocks.shouldDeploy.mockReturnValue(true);
+		mocks.composeFindMany.mockResolvedValue([]);
+		mocks.queueAdd.mockResolvedValue({ id: "job-id" });
+
+		mocks.applicationsFindMany.mockImplementation(({ where }) => {
+			const matches =
+				getConditionValue(where, "application.sourceType") === "github" &&
+				getConditionValue(where, "application.autoDeploy") === true &&
+				getConditionValue(where, "application.triggerType") === "push" &&
+				getConditionValue(where, "application.branch") === "main" &&
+				getConditionValue(where, "application.repository") === "dokploy" &&
+				getConditionValue(where, "application.owner") === "agentHits" &&
+				getConditionValue(where, "application.githubId") ===
+					"github-provider-id";
+
+			return Promise.resolve(
+				matches
+					? [
+							{
+								applicationId: "application-id",
+								serverId: null,
+								watchPaths: null,
+							},
+						]
+					: [],
+			);
+		});
+	});
+
+	it("matches push events using repository owner name when available", async () => {
+		const res = createResponse();
+
+		await handler(
+			createPushRequest("main", {
+				login: "agentHits-login",
+				name: "agentHits",
+			}),
+			res,
+		);
+
+		expect(mocks.queueAdd).toHaveBeenCalledWith(
+			"deployments",
+			expect.objectContaining({
+				applicationId: "application-id",
+				applicationType: "application",
+				type: "deploy",
+			}),
+			expect.objectContaining({
+				removeOnComplete: true,
+				removeOnFail: true,
+			}),
+		);
+		expect(res.status).toHaveBeenCalledWith(200);
+		expect(res.json).toHaveBeenCalledWith({ message: "Deployed 1 apps" });
+	});
+
+	it("matches compose push events using repository owner login fallback", async () => {
+		mocks.applicationsFindMany.mockResolvedValue([]);
+		mocks.composeFindMany.mockImplementation(({ where }) => {
+			const matches =
+				getConditionValue(where, "compose.sourceType") === "github" &&
+				getConditionValue(where, "compose.autoDeploy") === true &&
+				getConditionValue(where, "compose.triggerType") === "push" &&
+				getConditionValue(where, "compose.branch") === "main" &&
+				getConditionValue(where, "compose.repository") === "dokploy" &&
+				getConditionValue(where, "compose.owner") === "agentHits" &&
+				getConditionValue(where, "compose.githubId") === "github-provider-id";
+
+			return Promise.resolve(
+				matches
+					? [
+							{
+								composeId: "compose-id",
+								serverId: null,
+								watchPaths: null,
+							},
+						]
+					: [],
+			);
+		});
+		const res = createResponse();
+
+		await handler(createPushRequest("main"), res);
+
+		expect(mocks.queueAdd).toHaveBeenCalledWith(
+			"deployments",
+			expect.objectContaining({
+				applicationType: "compose",
+				composeId: "compose-id",
+				type: "deploy",
+			}),
+			expect.objectContaining({
+				removeOnComplete: true,
+				removeOnFail: true,
+			}),
+		);
+		expect(res.status).toHaveBeenCalledWith(200);
+		expect(res.json).toHaveBeenCalledWith({ message: "Deployed 1 apps" });
+	});
+
+	it("matches tag events using repository owner login fallback", async () => {
+		mocks.applicationsFindMany.mockImplementation(({ where }) => {
+			const matches =
+				getConditionValue(where, "application.sourceType") === "github" &&
+				getConditionValue(where, "application.autoDeploy") === true &&
+				getConditionValue(where, "application.triggerType") === "tag" &&
+				getConditionValue(where, "application.repository") === "dokploy" &&
+				getConditionValue(where, "application.owner") === "agentHits" &&
+				getConditionValue(where, "application.githubId") ===
+					"github-provider-id";
+
+			return Promise.resolve(
+				matches
+					? [
+							{
+								applicationId: "application-id",
+								serverId: null,
+							},
+						]
+					: [],
+			);
+		});
+		const res = createResponse();
+
+		await handler(createTagRequest("v1.0.0"), res);
+
+		expect(mocks.queueAdd).toHaveBeenCalledWith(
+			"deployments",
+			expect.objectContaining({
+				applicationId: "application-id",
+				applicationType: "application",
+				titleLog: "Tag created: v1.0.0",
+				type: "deploy",
+			}),
+			expect.objectContaining({
+				removeOnComplete: true,
+				removeOnFail: true,
+			}),
+		);
+		expect(res.status).toHaveBeenCalledWith(200);
+		expect(res.json).toHaveBeenCalledWith({
+			message: "Deployed 1 apps based on tag v1.0.0",
+		});
+	});
+
+	it("does not deploy when the pushed branch does not match", async () => {
+		const res = createResponse();
+
+		await handler(createPushRequest("feature"), res);
+
+		expect(mocks.queueAdd).not.toHaveBeenCalled();
+		expect(res.status).toHaveBeenCalledWith(200);
+		expect(res.json).toHaveBeenCalledWith({ message: "No apps to deploy" });
+	});
+});
--- a/apps/dokploy/pages/api/deploy/github.ts
+++ b/apps/dokploy/pages/api/deploy/github.ts
@@ -23,6 +23,9 @@ import {
 	logWebhookError,
 } from "./[refreshToken]";

+const getGithubRepositoryOwner = (githubBody: any) =>
+	githubBody?.repository?.owner?.name ?? githubBody?.repository?.owner?.login;
+
 export default async function handler(
 	req: NextApiRequest,
 	res: NextApiResponse,
@@ -109,7 +112,7 @@ export default async function handler(
 		try {
 			const tagName = githubBody?.ref.replace("refs/tags/", "");
 			const repository = githubBody?.repository?.name;
-			const owner = githubBody?.repository?.owner?.name;
+			const owner = getGithubRepositoryOwner(githubBody);
 			const deploymentTitle = `Tag created: ${tagName}`;
 			const deploymentHash = extractHash(req.headers, githubBody);

@@ -219,7 +222,7 @@ export default async function handler(

 			const deploymentTitle = extractCommitMessage(req.headers, req.body);
 			const deploymentHash = extractHash(req.headers, req.body);
-			const owner = githubBody?.repository?.owner?.name;
+			const owner = getGithubRepositoryOwner(githubBody);
 			const normalizedCommits = githubBody?.commits?.flatMap(
 				(commit: any) => commit.modified,
 			);
@@ -372,7 +375,7 @@ export default async function handler(
 			const repository = githubBody?.repository?.name;
 			const deploymentHash = githubBody?.pull_request?.head?.sha;
 			const branch = githubBody?.pull_request?.base?.ref;
-			const owner = githubBody?.repository?.owner?.login;
+			const owner = getGithubRepositoryOwner(githubBody);
 			const prAuthor = githubBody?.pull_request?.user?.login;

 			// Validate PR author information is present