mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-03 17:51:58 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - GitHub review bots require signed webhook delivery before activation. > - Cloud can deduplicate an old ping after a connection repair. > - A fresh event could verify active connections, but an attention-state connection discarded it before authentication. > - This pull request allows signed delivery verification while the connection needs attention. > - It discards ordinary work until the manager completes the existing reconnect and activation checks. ## Linked Issues or Issue Description Refs #16259. Related: #16138 addresses the separate classification of local authorization failures. **What happened?** Live verification of #16259 found that a GitHub App already marked as needing attention remained stuck waiting for a signed ping. The Cloud relay had already delivered that ping. New signed events were discarded by the attention-state ingress gate before they could verify delivery. **Expected behavior** A fresh valid event from the bound installation verifies delivery. It does not activate the connection, change repository access, or start work. The manager can then complete the normal reconnect flow. **Steps to reproduce** 1. Put an existing App connection into attention state with webhook verification cleared after repair. 2. Deliver a fresh signed comment event for its bound installation. 3. Continue setup. Before this fix, webhook verification remains missing and reconnect cannot proceed. ## What Changed - Use a content-free verification path for signed ordinary GitHub events while the connection needs attention. - Allow a fresh signed event from the same installation to verify webhook delivery. - Verify delivery even from disabled or unavailable repositories. Discard the event before content storage, resource reconciliation, or runtime creation. - Keep newly verified connections in the provider-setup step. Later installation events cannot activate them before reconnect. Preserve normal installation recovery for existing qualified connections. - Extend recovery regressions to cover attention state, foreign installations, invalid signatures, missing installation IDs, unchanged resources, no wakeups, and successful reconnect after verification. - Document the recovery behavior. ## Verification - The targeted regression reproduced the attention-state failure before the change. - All ten recovery cases pass after the change. The tests cover disabled and unavailable repositories, content-free verification, and installation events after verification. - Server typecheck passed. - All 260 GitHub integration tests passed after the review fixes. Final server typecheck also passed. - `pnpm -r typecheck` and `pnpm build` passed. - The full local suite is running. One unrelated workspace-runtime conflict-reference test failed and then passed on its isolated rerun. Final totals and CI/review outcomes will be updated before merge. - Live verification will use the existing advisory review bot after deployment. ## Risks - Attention-state ordinary events authenticate under the current credential lease without creating a durable content record. They return before resource reconciliation or runtime creation. - Verification is still bound to the current credential lease, runtime generation, valid signature, and installation ID. It does not activate the bot or replace normal setup checks. - Paused, revoked, archived, and unconfigured connections retain their existing ingress rules. No schema changes or permission expansion. ## Model Used OpenAI Codex (GPT-6), with code inspection, editing, test execution, and browser verification. The exact deployment model ID and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
286 lines
13 KiB
TypeScript
286 lines
13 KiB
TypeScript
import { type GitHubIngressStageResult } from "./webhook-ingress.js";
|
|
|
|
import { createHmac, timingSafeEqual } from "node:crypto";
|
|
import { eq } from "drizzle-orm";
|
|
import { recordChatWebhookReceipt } from "../../chat-webhook-diagnostics.js";
|
|
import type { Db } from "@paperclipai/db";
|
|
import type { CredentialMutationLeaseGuard } from "../../chat-credential-mutation-lease.js";
|
|
import { chatEndpoints } from "@paperclipai/db";
|
|
import { logger } from "../../../middleware/logger.js";
|
|
import { logActivity } from "../../activity-log.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";
|
|
import type { createChatResourceReconciliation } from "../resource-reconciliation.js";
|
|
type AuthenticationOptions = Pick<Parameters<typeof createGitHubWebhookIngress>[1], "endpointRecord" | "runtimeContextForRecord" | "withCredentialMutationLease" | "redactError"> & Pick<ReturnType<typeof createGitHubWebhookIngress>, "stageGitHubWebhookIngress" | "processGitHubWebhookIngress" | "githubIngressAcceptedResponse"> & {
|
|
resolveCredentials: ReturnType<typeof createChatCredentials>["resolveCredentials"];
|
|
reconcileGitHubWebhookRepository: ReturnType<typeof createChatResourceReconciliation>["reconcileGitHubWebhookRepository"];
|
|
scheduleMessageProcessing: (task: () => Promise<void>) => void;
|
|
backgroundMessageTasks: Set<Promise<void>>;
|
|
readonly githubWebhookAuthenticationBarrier?: () => Promise<void>;
|
|
readonly deferWebhookProcessing?: boolean;
|
|
};
|
|
export function retryableGitHubWebhookResponse(): Response {
|
|
return new Response(
|
|
"Paperclip could not durably accept the event in time",
|
|
{
|
|
status: 503,
|
|
headers: {
|
|
"content-type": "text/plain; charset=utf-8",
|
|
"retry-after": "1",
|
|
},
|
|
},
|
|
);
|
|
}
|
|
export function createGitHubWebhookAuthentication(db: Db, options: AuthenticationOptions) {
|
|
const { endpointRecord, runtimeContextForRecord, withCredentialMutationLease, resolveCredentials, reconcileGitHubWebhookRepository, stageGitHubWebhookIngress, processGitHubWebhookIngress, githubIngressAcceptedResponse, scheduleMessageProcessing, backgroundMessageTasks, redactError } = options;
|
|
async function recordVerifiedWebhook(endpoint: EndpointRow, credentialLease: CredentialMutationLeaseGuard, providerDeliveryId: string | null) {
|
|
const verifiedAt = new Date();
|
|
await db.transaction(async tx => {
|
|
// Verification belongs to the same credential lease and ingress epoch
|
|
// that authenticated the original provider bytes.
|
|
await credentialLease.assertOwned(tx);
|
|
await tx.update(chatEndpoints).set({
|
|
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, {
|
|
companyId: endpoint.companyId, actorType: "system", actorId: "github-webhook",
|
|
action: "chat_endpoint.webhook_verified", entityType: "tool_connection", entityId: endpoint.connectionId,
|
|
details: { endpointId: endpoint.id, provider: "github", providerDeliveryId },
|
|
});
|
|
await credentialLease.assertOwned(tx);
|
|
});
|
|
}
|
|
async function authenticateGitHubWebhook(endpoint: EndpointRow, request: Request, scope: GitHubWebhookScope): Promise<Response | null> {
|
|
const { replayingDurableGitHubIngress, matchesGitHubIngressFence, completeGitHubPreflightBeforeDeadline } = scope;
|
|
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 {
|
|
staged = await stageGitHubWebhookIngress(endpoint, request.clone());
|
|
} catch (error) {
|
|
logger.warn(
|
|
{ endpointId: endpoint.id, error: redactError(error) },
|
|
"GitHub webhook could not be durably authenticated",
|
|
);
|
|
return retryableGitHubWebhookResponse();
|
|
}
|
|
if (staged.kind === "invalid_signature")
|
|
return new Response("Invalid signature", { status: 401 });
|
|
if (staged.kind === "ignored")
|
|
return new Response("ignored", { status: 200 });
|
|
recordChatWebhookReceipt(endpoint.id, staged.actionId, "github_ingress");
|
|
|
|
if (options.deferWebhookProcessing === true) {
|
|
scheduleMessageProcessing(async () => {
|
|
await processGitHubWebhookIngress(staged.actionId);
|
|
});
|
|
return githubIngressAcceptedResponse();
|
|
}
|
|
|
|
const processing = processGitHubWebhookIngress(staged.actionId, true);
|
|
const tracked = processing.then(
|
|
() => undefined,
|
|
() => undefined,
|
|
);
|
|
backgroundMessageTasks.add(tracked);
|
|
void tracked.finally(() => backgroundMessageTasks.delete(tracked));
|
|
const outcome = await completeGitHubPreflightBeforeDeadline(processing);
|
|
return outcome.completed
|
|
? (outcome.value ?? githubIngressAcceptedResponse())
|
|
: githubIngressAcceptedResponse();
|
|
}
|
|
const inspection = request.clone();
|
|
const body = await inspection.text();
|
|
const signature = inspection.headers.get("x-hub-signature-256");
|
|
if (!replayingDurableGitHubIngress)
|
|
await options.githubWebhookAuthenticationBarrier?.();
|
|
if (eventType === "ping") {
|
|
let verifiedAttempt:
|
|
| { completed: true; value: boolean }
|
|
| { completed: false; value?: never };
|
|
const verification = withCredentialMutationLease(
|
|
endpoint,
|
|
async (credentialLease): Promise<boolean> => {
|
|
const current = await endpointRecord(endpoint.id);
|
|
if (
|
|
!current ||
|
|
current.endpoint.provider !== "github" ||
|
|
!matchesGitHubIngressFence(runtimeContextForRecord(current)) ||
|
|
["archived", "paused", "revoked"].includes(
|
|
current.endpoint.status,
|
|
)
|
|
)
|
|
return false;
|
|
const credentials = await resolveCredentials(current.endpoint);
|
|
const expected = `sha256=${createHmac("sha256", credentials.webhookSecret).update(body).digest("hex")}`;
|
|
let valid = false;
|
|
try {
|
|
valid =
|
|
typeof signature === "string" &&
|
|
timingSafeEqual(Buffer.from(signature), Buffer.from(expected));
|
|
} catch {
|
|
valid = false;
|
|
}
|
|
if (!valid) return false;
|
|
await recordVerifiedWebhook(current.endpoint, credentialLease, inspection.headers.get("x-github-delivery"));
|
|
return true;
|
|
},
|
|
);
|
|
try {
|
|
verifiedAttempt =
|
|
await completeGitHubPreflightBeforeDeadline(verification);
|
|
} catch (error) {
|
|
logger.warn(
|
|
{ endpointId: endpoint.id, error: redactError(error) },
|
|
"GitHub ping authentication failed before durable verification",
|
|
);
|
|
return retryableGitHubWebhookResponse();
|
|
}
|
|
if (!verifiedAttempt.completed) {
|
|
// This path is reachable only for the private durable worker. The
|
|
// public request has already committed its authenticated receipt.
|
|
void verification.catch((error) => {
|
|
logger.warn(
|
|
{ endpointId: endpoint.id, error: redactError(error) },
|
|
"timed-out GitHub ping authentication later failed",
|
|
);
|
|
});
|
|
return retryableGitHubWebhookResponse();
|
|
}
|
|
const verified = verifiedAttempt.value;
|
|
return verified
|
|
? new Response("pong", { status: 200 })
|
|
: new Response("Invalid signature", { status: 401 });
|
|
}
|
|
const preflightTask = withCredentialMutationLease(
|
|
endpoint,
|
|
async (
|
|
credentialLease,
|
|
): Promise<"accepted" | "ignored" | "invalid_signature"> => {
|
|
// The repository upsert below is a Paperclip mutation derived from
|
|
// the webhook credential. Re-read and authenticate while holding the
|
|
// same lease as rotation/reconnect/removal so a callback signed with
|
|
// an obsolete secret cannot reopen a quarantined repository in the
|
|
// resolve-secret -> persist-resource race window.
|
|
const current = await endpointRecord(endpoint.id);
|
|
if (
|
|
!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" &&
|
|
eventType !== "installation")
|
|
) {
|
|
return "ignored";
|
|
}
|
|
const credentials = await resolveCredentials(current.endpoint);
|
|
const expected = `sha256=${createHmac("sha256", credentials.webhookSecret).update(body).digest("hex")}`;
|
|
let signatureValid = false;
|
|
try {
|
|
signatureValid =
|
|
typeof signature === "string" &&
|
|
timingSafeEqual(Buffer.from(signature), Buffer.from(expected));
|
|
} catch {
|
|
signatureValid = false;
|
|
}
|
|
if (!signatureValid) return "invalid_signature";
|
|
|
|
let payload: {
|
|
installation?: { id?: unknown };
|
|
repository?: unknown;
|
|
} | null = null;
|
|
try {
|
|
payload = JSON.parse(body) as {
|
|
installation?: { id?: unknown };
|
|
repository?: unknown;
|
|
};
|
|
} 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 (
|
|
incomingInstallationId !== undefined &&
|
|
String(incomingInstallationId) !== credentials.installationId &&
|
|
eventType !== "installation"
|
|
) {
|
|
// A dedicated endpoint represents exactly one GitHub App
|
|
// installation. GitHub sends every installation's events to the
|
|
// App webhook, so acknowledge foreign signed traffic without
|
|
// admitting it to Paperclip or prompting endless redelivery.
|
|
// Installation lifecycle events are the one exception: even when
|
|
// their id differs, canonical App inventory under the lifecycle
|
|
// lease must decide whether this is a valid sole-installation
|
|
// replacement or a second active installation that quarantines
|
|
// the endpoint.
|
|
return "ignored";
|
|
}
|
|
// A fresh signed event for the bound installation proves delivery
|
|
// 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", "attention"].includes(current.endpoint.status) &&
|
|
incomingInstallationId !== undefined &&
|
|
String(incomingInstallationId) === credentials.installationId &&
|
|
!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,
|
|
credentialLease,
|
|
);
|
|
}
|
|
return "accepted";
|
|
},
|
|
);
|
|
let preflightAttempt:
|
|
| {
|
|
completed: true;
|
|
value: "accepted" | "ignored" | "invalid_signature";
|
|
}
|
|
| { completed: false; value?: never };
|
|
try {
|
|
preflightAttempt =
|
|
await completeGitHubPreflightBeforeDeadline(preflightTask);
|
|
} catch (error) {
|
|
logger.warn(
|
|
{ endpointId: endpoint.id, error: redactError(error) },
|
|
"GitHub webhook authentication failed before durable acceptance",
|
|
);
|
|
return retryableGitHubWebhookResponse();
|
|
}
|
|
if (!preflightAttempt.completed) {
|
|
void preflightTask.catch((error) => {
|
|
logger.warn(
|
|
{ endpointId: endpoint.id, error: redactError(error) },
|
|
"timed-out GitHub webhook authentication later failed",
|
|
);
|
|
});
|
|
return retryableGitHubWebhookResponse();
|
|
}
|
|
const preflight = preflightAttempt.value;
|
|
if (preflight === "ignored") {
|
|
return new Response("ignored", { status: 200 });
|
|
}
|
|
if (preflight === "invalid_signature") {
|
|
return new Response("Invalid signature", { status: 401 });
|
|
}
|
|
return null;
|
|
}
|
|
return { authenticateGitHubWebhook };
|
|
}
|