From f5ded8b2731e68b66adf17b1cbfdaa53c2f593c0 Mon Sep 17 00:00:00 2001 From: agentHits <140916359+agentHits@users.noreply.github.com> Date: Wed, 1 Jul 2026 22:23:24 +0300 Subject: [PATCH] fix: use github owner login for webhook deploy matching (#4674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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. --- .../deploy/github-webhook-handler.test.ts | 323 ++++++++++++++++++ apps/dokploy/pages/api/deploy/github.ts | 9 +- 2 files changed, 329 insertions(+), 3 deletions(-) create mode 100644 apps/dokploy/__test__/deploy/github-webhook-handler.test.ts diff --git a/apps/dokploy/__test__/deploy/github-webhook-handler.test.ts b/apps/dokploy/__test__/deploy/github-webhook-handler.test.ts new file mode 100644 index 000000000..e25bd425e --- /dev/null +++ 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; + json: ReturnType; + }; + + 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" }); + }); +}); diff --git a/apps/dokploy/pages/api/deploy/github.ts b/apps/dokploy/pages/api/deploy/github.ts index 7f9b93e69..f03bf786e 100644 --- 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