From 0ce3f431a462fb59c32bdac2f74f82211577e4eb Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 25 Jun 2026 22:38:13 -0700 Subject: [PATCH] feat(selfhost): record installation app_id and filter foreign-app webhooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-host migration prereq (blocker 2a). When the cloud App and a self-host App are installed on the same account during the parallel-run phase, a backend should only act on ITS OWN App's installations. - Add a nullable installations.app_id column (Drizzle + migration 0071), captured in upsertInstallation from installation events / the App-installation API refresh; a payload without it never clears the stored value. - upsertInstallation returns the resolved app_id so the webhook entry can filter without a second read. - New pure isForeignAppInstallation(ownAppId, installationAppId): true ONLY on a positive numeric mismatch with GITHUB_APP_ID; fail-open on any unknown. - Wire it at the webhook entry: a foreign-app delivery is acked (webhook_events 'foreign_app') without processing. Defense-in-depth: the per-App webhook secret (GITHUB_WEBHOOK_SECRET) is the PRIMARY isolation; this is the belt-and-suspenders for a shared-endpoint/secret misconfig. FAIL-OPEN — an unknown/own-matching app_id always processes, so the live single-app path is byte-identical until the column is populated. --- migrations/0071_installations_app_id.sql | 6 +++ src/db/repositories.ts | 12 ++++- src/db/schema.ts | 4 ++ src/github/app.ts | 16 ++++++ src/queue/processors.ts | 21 +++++++- src/types.ts | 4 ++ test/unit/github-app.test.ts | 22 ++++++++ test/unit/queue.test.ts | 67 ++++++++++++++++++++++++ 8 files changed, 148 insertions(+), 4 deletions(-) create mode 100644 migrations/0071_installations_app_id.sql diff --git a/migrations/0071_installations_app_id.sql b/migrations/0071_installations_app_id.sql new file mode 100644 index 0000000000..027d748d5c --- /dev/null +++ b/migrations/0071_installations_app_id.sql @@ -0,0 +1,6 @@ +-- Dual-app identity (#selfhost-app-id): record which GitHub App an installation belongs to, so a backend can +-- tell its OWN installations from a SECOND gittensory App installed on the same account (cloud + self-host +-- running side by side during the migration). Nullable: only `installation` events and the App-installation API +-- refresh carry app_id, so existing rows backfill lazily on their next event. The webhook entry fails OPEN — an +-- unknown app_id always processes — so this column is byte-identical until it is populated. +ALTER TABLE installations ADD COLUMN app_id INTEGER; diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 30eab0bbc3..73921b5acc 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -181,8 +181,8 @@ const FRESHNESS_SIGNAL_TYPES = [ "queue-health", ]; -export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload): Promise { - if (!payload.installation?.id) return; +export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload): Promise { + if (!payload.installation?.id) return null; const account = payload.installation.account; const existing = await getInstallation(env, payload.installation.id); const permissions = @@ -195,6 +195,10 @@ export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload const targetType = payload.installation.target_type ?? account?.type ?? existing?.targetType ?? "unknown"; const repositorySelection = payload.installation.repository_selection ?? existing?.repositorySelection; const suspendedAt = payload.installation.suspended_at !== undefined ? payload.installation.suspended_at : (existing?.suspendedAt ?? undefined); + // Capture app_id when the payload carries it (installation events + the App-installation API refresh); keep the + // stored value otherwise so a payload without it (e.g. a pull_request event) never clears it. Returned so the + // caller can filter a dual-app webhook without a second read (#selfhost-app-id). + const appId = payload.installation.app_id ?? existing?.appId ?? null; const db = getDb(env.DB); await db .insert(installations) @@ -202,6 +206,7 @@ export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload id: payload.installation.id, accountLogin, accountId, + appId, targetType, repositorySelection, permissionsJson: jsonString(permissions), @@ -214,6 +219,7 @@ export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload set: { accountLogin, accountId, + appId, targetType, repositorySelection, permissionsJson: jsonString(permissions), @@ -222,6 +228,7 @@ export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload updatedAt: nowIso(), }, }); + return appId; } export async function markInstallationDeleted(env: Env, installationId: number): Promise { @@ -3805,6 +3812,7 @@ function toInstallationRecord(row: typeof installations.$inferSelect): Installat id: row.id, accountLogin: row.accountLogin, accountId: row.accountId, + appId: row.appId, targetType: row.targetType, repositorySelection: row.repositorySelection, permissions: parseJson>(row.permissionsJson, {}), diff --git a/src/db/schema.ts b/src/db/schema.ts index 04cce59339..4ed30b32aa 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -9,6 +9,10 @@ export const installations = sqliteTable("installations", { id: integer("id").primaryKey(), accountLogin: text("account_login").notNull(), accountId: integer("account_id").notNull(), + // The GitHub App this installation belongs to (#selfhost-app-id). Nullable: only `installation` events (and + // the App-installation API refresh) carry it, so existing rows backfill lazily. Lets a backend tell its OWN + // installations from a SECOND gittensory App installed on the same account (cloud + self-host side by side). + appId: integer("app_id"), targetType: text("target_type").notNull(), repositorySelection: text("repository_selection"), permissionsJson: text("permissions_json").notNull().default("{}"), diff --git a/src/github/app.ts b/src/github/app.ts index f512d7bcdd..322db2e703 100644 --- a/src/github/app.ts +++ b/src/github/app.ts @@ -77,6 +77,22 @@ export async function createInstallationToken(env: Env, installationId: number): return payload.token; } +/** + * Dual-app webhook safety (#selfhost-app-id): TRUE when a delivery's installation belongs to a DIFFERENT + * gittensory App than this backend's own (`GITHUB_APP_ID`), e.g. the cloud App and a self-host App installed on + * the same account during the migration. FAIL-OPEN by construction — returns FALSE (process the webhook) whenever + * we cannot be certain it is foreign: no configured own id, an unparseable own id, or an unknown installation + * app_id (existing rows backfill lazily). It returns TRUE only on a POSITIVE numeric mismatch, so it can never + * drop a legitimate delivery whose app_id is null/unknown. Signature verification (per-App webhook secret) is the + * PRIMARY isolation; this is defense-in-depth for a shared-endpoint/secret misconfiguration. PURE. + */ +export function isForeignAppInstallation(ownAppId: string | undefined, installationAppId: number | null | undefined): boolean { + if (!ownAppId || installationAppId === null || installationAppId === undefined) return false; + const own = Number.parseInt(ownAppId, 10); + if (!Number.isFinite(own)) return false; + return own !== installationAppId; +} + /** Test-only: clear the in-isolate installation-token cache so each test starts fresh (the module-level Map * otherwise leaks a cached token across test cases that share an installation id). */ export function clearInstallationTokenCacheForTest(): void { diff --git a/src/queue/processors.ts b/src/queue/processors.ts index d9dc61c07d..7e5809719d 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -83,7 +83,7 @@ import { refreshPullRequestDetails, } from "../github/backfill"; import { contributorRepoStatsFromGittensor, fetchGittensorContributorSnapshot, fetchOfficialGittensorMiner, type GittensorContributorSnapshot, type OfficialGittensorMinerDetection } from "../gittensor/api"; -import { createInstallationToken, createOrUpdateCheckRun, createOrUpdateErroredGateCheckRun, createOrUpdateGateCheckRun, createOrUpdateOverriddenGateCheckRun, createOrUpdatePendingGateCheckRun, createOrUpdateSkippedGateCheckRun, getInstallationId, getRepositoryCollaboratorPermission } from "../github/app"; +import { createInstallationToken, createOrUpdateCheckRun, createOrUpdateErroredGateCheckRun, createOrUpdateGateCheckRun, createOrUpdateOverriddenGateCheckRun, createOrUpdatePendingGateCheckRun, createOrUpdateSkippedGateCheckRun, getInstallationId, getRepositoryCollaboratorPermission, isForeignAppInstallation } from "../github/app"; import { AGENT_COMMAND_COMMENT_MARKER, createOrUpdateAgentCommandComment, createOrUpdatePrIntelligenceComment, PR_PANEL_COMMENT_MARKER } from "../github/comments"; import { gittensoryFooter, gittensorRepoEarnUrl, maintainerControlPanelUrl } from "../github/footer"; import { @@ -1531,7 +1531,24 @@ async function processGitHubWebhook(env: Env, deliveryId: string, eventName: str return; } - await upsertInstallation(env, payload); + const installationAppId = await upsertInstallation(env, payload); + // Dual-app safety (#selfhost-app-id): if this delivery's installation belongs to a DIFFERENT gittensory App + // (cloud + self-host installed on the same account), ack it without processing so neither backend acts on the + // other's installation. FAIL-OPEN — an unknown/own-matching app_id always processes, so the LIVE single-app + // path is byte-identical. Signature verification (per-App secret) is the primary isolation; this is the + // belt-and-suspenders for a shared-endpoint/secret misconfig. + if (isForeignAppInstallation(env.GITHUB_APP_ID, installationAppId)) { + await recordWebhookEvent(env, { + deliveryId, + eventName, + action: payload.action, + installationId: payload.installation?.id, + repositoryFullName: payload.repository?.full_name, + payloadHash: "foreign_app", + status: "processed", + }); + return; + } const installationActor = payload.installation?.account?.login ?? (payload.installation?.id ? (await getInstallation(env, payload.installation.id))?.accountLogin : undefined); diff --git a/src/types.ts b/src/types.ts index 1ef856fbbe..039f9e11ec 100644 --- a/src/types.ts +++ b/src/types.ts @@ -205,6 +205,7 @@ export type GitHubWebhookPayload = { action?: string; installation?: { id: number; + app_id?: number; account?: { login?: string; id?: number; @@ -1069,6 +1070,9 @@ export type InstallationRecord = { id: number; accountLogin: string; accountId: number; + /** The GitHub App this installation belongs to (#selfhost-app-id); null until an `installation` event or the + * App-installation API refresh populates it. */ + appId?: number | null | undefined; targetType: string; repositorySelection?: string | null | undefined; permissions: Record; diff --git a/test/unit/github-app.test.ts b/test/unit/github-app.test.ts index 95d3fa7dd8..c1103ccd8b 100644 --- a/test/unit/github-app.test.ts +++ b/test/unit/github-app.test.ts @@ -10,6 +10,7 @@ import { getAppInstallation, getInstallationId, getRepositoryCollaboratorPermission, + isForeignAppInstallation, } from "../../src/github/app"; import type { Advisory } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; @@ -823,3 +824,24 @@ function gateAdvisory(headSha: string): Advisory { generatedAt: "2026-05-22T00:00:00.000Z", }; } + +describe("isForeignAppInstallation (#selfhost-app-id)", () => { + it("returns true only on a positive numeric app_id mismatch", () => { + expect(isForeignAppInstallation("12345", 99999)).toBe(true); + }); + + it("returns false when this backend's own app id and the installation's match", () => { + expect(isForeignAppInstallation("12345", 12345)).toBe(false); + }); + + it("FAILS OPEN (false) when the installation app_id is unknown — null or undefined", () => { + expect(isForeignAppInstallation("12345", null)).toBe(false); + expect(isForeignAppInstallation("12345", undefined)).toBe(false); + }); + + it("FAILS OPEN (false) when this backend has no / an unparseable own app id", () => { + expect(isForeignAppInstallation(undefined, 99999)).toBe(false); + expect(isForeignAppInstallation("", 99999)).toBe(false); + expect(isForeignAppInstallation("not-a-number", 99999)).toBe(false); + }); +}); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 8f2e103d06..454f7ca21e 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -7506,3 +7506,70 @@ function reopenedPayload(sender: string): any { }, }; } + +describe("installation app_id capture + dual-app webhook filter (#selfhost-app-id)", () => { + it("captures app_id from an installation payload, returns it, and preserves it when a later payload omits it", async () => { + const env = createTestEnv(); + const stored = await upsertInstallation(env, { + action: "created", + installation: { id: 4242, app_id: 555, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] }, + }); + expect(stored).toBe(555); + expect((await getInstallation(env, 4242))?.appId).toBe(555); + // A subsequent payload WITHOUT app_id (e.g. a pull_request event) must not clear the stored value. + const preserved = await upsertInstallation(env, { action: "synchronize", installation: { id: 4242, account: { login: "owner", id: 1, type: "Organization" } } }); + expect(preserved).toBe(555); + expect((await getInstallation(env, 4242))?.appId).toBe(555); + }); + + it("acks a webhook whose installation belongs to a DIFFERENT app without processing it", async () => { + const env = createTestEnv(); // own GITHUB_APP_ID defaults to "3824093" + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 7777); + // The installation is recorded as belonging to a FOREIGN app (99999 ≠ 3824093). + await upsertInstallation(env, { action: "created", installation: { id: 7777, app_id: 99999, account: { login: "JSONbored", id: 1, type: "User" }, repository_selection: "selected", permissions: {}, events: [] } }); + vi.stubGlobal("fetch", async () => Response.json({})); + + await processJob(env, { + type: "github-webhook", + deliveryId: "foreign-app-pr", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 7777 }, // a PR event carries no app_id; the stored 99999 is used + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 88, title: "Foreign", state: "open", user: { login: "contributor" }, head: { sha: "f88" }, labels: [], body: "x" }, + }, + }); + + // The delivery was acked as foreign, and the PR was never upserted (the handler returned before the PR block). + const evt = await env.DB.prepare("select payload_hash from webhook_events where delivery_id = ?").bind("foreign-app-pr").first<{ payload_hash: string }>(); + expect(evt?.payload_hash).toBe("foreign_app"); + const pr = await env.DB.prepare("select count(*) as n from pull_requests where repo_full_name = ? and number = ?").bind("JSONbored/gittensory", 88).first<{ n: number }>(); + expect(pr?.n).toBe(0); + }); + + it("processes a webhook whose installation app_id matches this backend (no false filtering)", async () => { + const env = createTestEnv(); // own GITHUB_APP_ID "3824093" + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 3824093001); + await upsertInstallation(env, { action: "created", installation: { id: 3824093001, app_id: 3824093, account: { login: "JSONbored", id: 1, type: "User" }, repository_selection: "selected", permissions: {}, events: [] } }); + vi.stubGlobal("fetch", async () => Response.json({})); + + await processJob(env, { + type: "github-webhook", + deliveryId: "own-app-pr", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 3824093001 }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 89, title: "Own", state: "open", user: { login: "contributor" }, head: { sha: "o89" }, labels: [], body: "x" }, + }, + }); + + // The matching-app webhook was processed normally — the PR row exists and it was NOT acked as foreign. + const pr = await env.DB.prepare("select count(*) as n from pull_requests where repo_full_name = ? and number = ?").bind("JSONbored/gittensory", 89).first<{ n: number }>(); + expect(pr?.n).toBe(1); + const evt = await env.DB.prepare("select payload_hash from webhook_events where delivery_id = ?").bind("own-app-pr").first<{ payload_hash: string }>(); + expect(evt?.payload_hash).not.toBe("foreign_app"); + }); +});