diff --git a/doc/connections/GITHUB.md b/doc/connections/GITHUB.md index 352ac9b2b45..1d2abe95ec5 100644 --- a/doc/connections/GITHUB.md +++ b/doc/connections/GITHUB.md @@ -220,6 +220,12 @@ callback recovery scanner and its callback-mismatch warning; the Cloud inbox own relay delivery retries. Only that local check can verify a connection or admit work. After a repair, a fresh authenticated event for the bound installation can verify webhook delivery even when the relay deduplicates a redelivered old ping. +This also applies while the connection needs attention: the signed event verifies +delivery without retaining its content, admitting work, or changing repository +access, even when its repository is disabled or unavailable. Verification returns +the connection to the provider-setup step; later installation events cannot +activate it. The manager must still complete reconnect and activation before new +review requests can run. Shared-App account connectors keep the behavior described above. Unverified gateway traffic has request, byte, pending-queue, retention and wake diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index 1b9d41d72ed..6ff16ab4fb0 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -4108,21 +4108,52 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { expect((await context.service.listResources(context.endpoint.id)).every(resource => !resource.enabled)).toBe(true); expect(await management.configuration(context.endpoint.id, "owner-user")).toMatchObject({ revision: 1, configuration: { toolsEnabled: false } }); }); - it.each(["current", "foreign", "invalid_signature"] as const)("verifies a repaired GitHub webhook only from a %s installation delivery", async kind => { + it.each([ + ["active", "current", "enabled"], ["active", "foreign", "enabled"], ["active", "invalid_signature", "enabled"], ["active", "missing_installation", "enabled"], + ["attention", "current", "enabled"], ["attention", "foreign", "enabled"], ["attention", "invalid_signature", "enabled"], ["attention", "missing_installation", "enabled"], + ["attention", "current", "disabled"], ["attention", "current", "unavailable"], + ] as const)("verifies a repaired GitHub webhook in %s only from a %s installation delivery with %s repository", async (status, kind, resourceState) => { const f = await reviewBotFixture(); const [endpoint] = await readChatEndpointsById(f.endpoint.id); - await db.update(chatEndpoints).set({ setup: { ...endpoint.setup, webhookVerifiedAt: null } }).where(eq(chatEndpoints.id, endpoint.id)); + if (resourceState !== "enabled") { + await db.update(chatEndpointResources).set(resourceState === "disabled" ? { enabled: false } : { availability: "unavailable" }).where(eq(chatEndpointResources.endpointId, endpoint.id)); + } + if (status === "attention") await db.update(toolConnections).set({ status: "disabled", enabled: false }).where(eq(toolConnections.id, endpoint.connectionId)); + const resourcesBefore = await f.service.listResources(endpoint.id); + const conversationsBefore = await readChatConversationsByEndpointId(endpoint.id); + const actionsBefore = await db.select().from(chatActions).where(eq(chatActions.endpointId, endpoint.id)); + f.wakeup.mockClear(); + await db.update(chatEndpoints).set({ status, setup: { ...endpoint.setup, webhookVerifiedAt: null } }).where(eq(chatEndpoints.id, endpoint.id)); const response = await f.service.handleWebhook(endpoint.publicId, "github", signedGitHubWebhookRequest({ event: "issue_comment", delivery: randomUUID(), webhookSecret: kind === "invalid_signature" ? "wrong-secret" : f.webhookSecret, - payload: { action: "created", installation: { id: kind === "foreign" ? 9999 : 2468 }, + payload: { action: "created", ...(kind === "missing_installation" ? {} : { installation: { id: kind === "foreign" ? 9999 : 2468 } }), repository: { id: 97531, full_name: "paperclipai/paperclip", owner: { id: 1357, login: "paperclipai" } }, - issue: { number: 418 }, comment: { id: 41801, body: "Ordinary comment", user: { id: 42, login: "octocat" } }, sender: { id: 42, login: "octocat" } }, + issue: { number: 418 }, comment: { id: 41801, body: status === "attention" ? `@${endpoint.botUsername} please review` : "Ordinary comment", user: { id: 42, login: "octocat" } }, sender: { id: 42, login: "octocat" } }, })); if (kind === "invalid_signature") expect(response.status).toBe(401); else expect(response.status).toBeLessThan(300); const current = await f.service.get(endpoint.id); if (kind === "current") expect(current.setup.webhookVerifiedAt).toEqual(expect.any(String)); else expect(current.setup.webhookVerifiedAt).toBeNull(); + if (status === "attention") { + expect(current.status).toBe("attention"); + expect(await f.service.listResources(endpoint.id)).toEqual(resourcesBefore); + expect(await readChatConversationsByEndpointId(endpoint.id)).toEqual(conversationsBefore); + expect(await db.select().from(chatActions).where(eq(chatActions.endpointId, endpoint.id))).toEqual(actionsBefore); + expect(f.wakeup).not.toHaveBeenCalled(); + if (kind === "current") { + expect(current.setup.step).toBe("provider_setup"); + await f.service.handleWebhook(endpoint.publicId, "github", signedGitHubWebhookRequest({ + event: "installation_repositories", delivery: randomUUID(), webhookSecret: f.webhookSecret, + payload: { action: "added", installation: { id: 2468 }, repositories_added: [], repositories_removed: [] }, + })); + expect((await f.service.get(endpoint.id)).status).toBe("attention"); + expect((await readToolConnectionsById(endpoint.connectionId))[0]).toMatchObject({ status: "disabled", enabled: false }); + expect(f.wakeup).not.toHaveBeenCalled(); + const configured = await f.service.configure(endpoint.id, { action: "reconnect" }, "owner-user"); + expect(configured.status).toBe("verifying"); + } + } }); it("stack rejects forged raw gateway deliveries without changing verification or waking an agent", async () => { const f = await reviewBotFixture(); diff --git a/server/src/services/chat-channels/github/installation-lifecycle.ts b/server/src/services/chat-channels/github/installation-lifecycle.ts index e0b2d9efda9..15023223a91 100644 --- a/server/src/services/chat-channels/github/installation-lifecycle.ts +++ b/server/src/services/chat-channels/github/installation-lifecycle.ts @@ -290,9 +290,11 @@ export function createGitHubInstallationLifecycle(db: Db, options: GitHubInstall const intentionallyUnavailable = canonical.endpoint.status === "paused" || canonical.endpoint.status === "archived" || + (canonical.endpoint.status === "attention" && canonical.endpoint.setup.step === "provider_setup") || !canonical.endpoint.setup.webhookVerifiedAt; const advanceRuntimeGeneration = refreshRuntimeAfterLifecycle || + (canonical.endpoint.status === "attention" && canonical.endpoint.setup.step === "provider_setup") || !canonical.endpoint.setup.webhookVerifiedAt; await db.transaction(async (tx) => { await credentialLease.assertOwned(tx); @@ -374,7 +376,7 @@ export function createGitHubInstallationLifecycle(db: Db, options: GitHubInstall ); await credentialLease.assertOwned(tx); }); - if (!canonical.endpoint.setup.webhookVerifiedAt) { + if (intentionallyUnavailable) { stopRuntimeAfterLifecycle = true; } } catch (error) { @@ -383,6 +385,7 @@ export function createGitHubInstallationLifecycle(db: Db, options: GitHubInstall !latest || latest.endpoint.status === "paused" || latest.endpoint.status === "archived" || + (latest.endpoint.status === "attention" && latest.endpoint.setup.step === "provider_setup") || !latest.endpoint.setup.webhookVerifiedAt; if (!preserveIntentionalState) { const failedAt = new Date(); diff --git a/server/src/services/chat-channels/github/webhook-authentication.ts b/server/src/services/chat-channels/github/webhook-authentication.ts index 31bcf6f820e..d393306d084 100644 --- a/server/src/services/chat-channels/github/webhook-authentication.ts +++ b/server/src/services/chat-channels/github/webhook-authentication.ts @@ -8,7 +8,7 @@ import type { CredentialMutationLeaseGuard } from "../../chat-credential-mutatio import { chatEndpoints } from "@paperclipai/db"; import { logger } from "../../../middleware/logger.js"; import { logActivity } from "../../activity-log.js"; -import type { EndpointRow } from "../runtime-types.js"; +import type { EndpointRow, InternalSetupState } from "../runtime-types.js"; import type { GitHubWebhookScope } from "./webhook-scope.js"; import type { createGitHubWebhookIngress } from "./webhook-ingress.js"; import type { createChatCredentials } from "../credentials.js"; @@ -42,7 +42,7 @@ export function createGitHubWebhookAuthentication(db: Db, options: Authenticatio // that authenticated the original provider bytes. await credentialLease.assertOwned(tx); await tx.update(chatEndpoints).set({ - setup: { ...endpoint.setup, webhookVerifiedAt: verifiedAt.toISOString() }, + setup: { ...endpoint.setup, ...(endpoint.status === "attention" ? { step: "provider_setup" as const } : {}), webhookVerifiedAt: verifiedAt.toISOString() }, healthMessage: "GitHub webhook verified", updatedAt: verifiedAt, }).where(eq(chatEndpoints.id, endpoint.id)); await logActivity(tx as unknown as Db, { @@ -55,7 +55,10 @@ export function createGitHubWebhookAuthentication(db: Db, options: Authenticatio } async function authenticateGitHubWebhook(endpoint: EndpointRow, request: Request, scope: GitHubWebhookScope): Promise { const { replayingDurableGitHubIngress, matchesGitHubIngressFence, completeGitHubPreflightBeforeDeadline } = scope; - if (!replayingDurableGitHubIngress) { + const eventType = request.headers.get("x-github-event")?.trim() ?? ""; + const verificationOnly = !replayingDurableGitHubIngress && endpoint.status === "attention" && + ["issue_comment", "issues", "pull_request", "pull_request_review", "pull_request_review_comment"].includes(eventType); + if (!replayingDurableGitHubIngress && !verificationOnly) { await options.githubWebhookAuthenticationBarrier?.(); let staged: GitHubIngressStageResult; try { @@ -95,7 +98,6 @@ export function createGitHubWebhookAuthentication(db: Db, options: Authenticatio const inspection = request.clone(); const body = await inspection.text(); const signature = inspection.headers.get("x-hub-signature-256"); - const eventType = inspection.headers.get("x-github-event"); if (!replayingDurableGitHubIngress) await options.githubWebhookAuthenticationBarrier?.(); if (eventType === "ping") { @@ -171,6 +173,8 @@ export function createGitHubWebhookAuthentication(db: Db, options: Authenticatio !current || current.endpoint.provider !== "github" || !matchesGitHubIngressFence(runtimeContextForRecord(current)) || + (verificationOnly && (current.endpoint.status !== "attention" || + runtimeContextForRecord(current).generation !== ((endpoint.setup as InternalSetupState).runtimeGeneration ?? 0))) || current.endpoint.status === "archived" || current.endpoint.status === "paused" || (current.endpoint.status === "revoked" && @@ -202,6 +206,7 @@ export function createGitHubWebhookAuthentication(db: Db, options: Authenticatio } catch { // Let the native adapter return its normal invalid-JSON response. } + const lifecycleEvent = ["github_app_authorization", "installation", "installation_repositories"].includes(eventType ?? ""); if (payload) { const incomingInstallationId = payload.installation?.id; if ( @@ -224,12 +229,15 @@ export function createGitHubWebhookAuthentication(db: Db, options: Authenticatio // after repair even when Cloud has already deduplicated the old ping. // Foreign/lifecycle traffic cannot qualify a changed installation. if (!current.endpoint.setup.webhookVerifiedAt && - ["active", "verifying"].includes(current.endpoint.status) && + ["active", "verifying", "attention"].includes(current.endpoint.status) && incomingInstallationId !== undefined && String(incomingInstallationId) === credentials.installationId && - eventType !== "installation") { + !lifecycleEvent) { await recordVerifiedWebhook(current.endpoint, credentialLease, inspection.headers.get("x-github-delivery")); } + } + if (current.endpoint.status === "attention" && !lifecycleEvent) return "ignored"; + if (payload) { await reconcileGitHubWebhookRepository( current.endpoint, payload,