Skip to content

Commit 0ce3f43

Browse files
committed
feat(selfhost): record installation app_id and filter foreign-app webhooks
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.
1 parent 9b53afa commit 0ce3f43

8 files changed

Lines changed: 148 additions & 4 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
-- Dual-app identity (#selfhost-app-id): record which GitHub App an installation belongs to, so a backend can
2+
-- tell its OWN installations from a SECOND gittensory App installed on the same account (cloud + self-host
3+
-- running side by side during the migration). Nullable: only `installation` events and the App-installation API
4+
-- refresh carry app_id, so existing rows backfill lazily on their next event. The webhook entry fails OPEN — an
5+
-- unknown app_id always processes — so this column is byte-identical until it is populated.
6+
ALTER TABLE installations ADD COLUMN app_id INTEGER;

src/db/repositories.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,8 @@ const FRESHNESS_SIGNAL_TYPES = [
181181
"queue-health",
182182
];
183183

184-
export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload): Promise<void> {
185-
if (!payload.installation?.id) return;
184+
export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload): Promise<number | null> {
185+
if (!payload.installation?.id) return null;
186186
const account = payload.installation.account;
187187
const existing = await getInstallation(env, payload.installation.id);
188188
const permissions =
@@ -195,13 +195,18 @@ export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload
195195
const targetType = payload.installation.target_type ?? account?.type ?? existing?.targetType ?? "unknown";
196196
const repositorySelection = payload.installation.repository_selection ?? existing?.repositorySelection;
197197
const suspendedAt = payload.installation.suspended_at !== undefined ? payload.installation.suspended_at : (existing?.suspendedAt ?? undefined);
198+
// Capture app_id when the payload carries it (installation events + the App-installation API refresh); keep the
199+
// stored value otherwise so a payload without it (e.g. a pull_request event) never clears it. Returned so the
200+
// caller can filter a dual-app webhook without a second read (#selfhost-app-id).
201+
const appId = payload.installation.app_id ?? existing?.appId ?? null;
198202
const db = getDb(env.DB);
199203
await db
200204
.insert(installations)
201205
.values({
202206
id: payload.installation.id,
203207
accountLogin,
204208
accountId,
209+
appId,
205210
targetType,
206211
repositorySelection,
207212
permissionsJson: jsonString(permissions),
@@ -214,6 +219,7 @@ export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload
214219
set: {
215220
accountLogin,
216221
accountId,
222+
appId,
217223
targetType,
218224
repositorySelection,
219225
permissionsJson: jsonString(permissions),
@@ -222,6 +228,7 @@ export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload
222228
updatedAt: nowIso(),
223229
},
224230
});
231+
return appId;
225232
}
226233

227234
export async function markInstallationDeleted(env: Env, installationId: number): Promise<void> {
@@ -3805,6 +3812,7 @@ function toInstallationRecord(row: typeof installations.$inferSelect): Installat
38053812
id: row.id,
38063813
accountLogin: row.accountLogin,
38073814
accountId: row.accountId,
3815+
appId: row.appId,
38083816
targetType: row.targetType,
38093817
repositorySelection: row.repositorySelection,
38103818
permissions: parseJson<Record<string, string>>(row.permissionsJson, {}),

src/db/schema.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ export const installations = sqliteTable("installations", {
99
id: integer("id").primaryKey(),
1010
accountLogin: text("account_login").notNull(),
1111
accountId: integer("account_id").notNull(),
12+
// The GitHub App this installation belongs to (#selfhost-app-id). Nullable: only `installation` events (and
13+
// the App-installation API refresh) carry it, so existing rows backfill lazily. Lets a backend tell its OWN
14+
// installations from a SECOND gittensory App installed on the same account (cloud + self-host side by side).
15+
appId: integer("app_id"),
1216
targetType: text("target_type").notNull(),
1317
repositorySelection: text("repository_selection"),
1418
permissionsJson: text("permissions_json").notNull().default("{}"),

src/github/app.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,22 @@ export async function createInstallationToken(env: Env, installationId: number):
7777
return payload.token;
7878
}
7979

80+
/**
81+
* Dual-app webhook safety (#selfhost-app-id): TRUE when a delivery's installation belongs to a DIFFERENT
82+
* gittensory App than this backend's own (`GITHUB_APP_ID`), e.g. the cloud App and a self-host App installed on
83+
* the same account during the migration. FAIL-OPEN by construction — returns FALSE (process the webhook) whenever
84+
* we cannot be certain it is foreign: no configured own id, an unparseable own id, or an unknown installation
85+
* app_id (existing rows backfill lazily). It returns TRUE only on a POSITIVE numeric mismatch, so it can never
86+
* drop a legitimate delivery whose app_id is null/unknown. Signature verification (per-App webhook secret) is the
87+
* PRIMARY isolation; this is defense-in-depth for a shared-endpoint/secret misconfiguration. PURE.
88+
*/
89+
export function isForeignAppInstallation(ownAppId: string | undefined, installationAppId: number | null | undefined): boolean {
90+
if (!ownAppId || installationAppId === null || installationAppId === undefined) return false;
91+
const own = Number.parseInt(ownAppId, 10);
92+
if (!Number.isFinite(own)) return false;
93+
return own !== installationAppId;
94+
}
95+
8096
/** Test-only: clear the in-isolate installation-token cache so each test starts fresh (the module-level Map
8197
* otherwise leaks a cached token across test cases that share an installation id). */
8298
export function clearInstallationTokenCacheForTest(): void {

src/queue/processors.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ import {
8383
refreshPullRequestDetails,
8484
} from "../github/backfill";
8585
import { contributorRepoStatsFromGittensor, fetchGittensorContributorSnapshot, fetchOfficialGittensorMiner, type GittensorContributorSnapshot, type OfficialGittensorMinerDetection } from "../gittensor/api";
86-
import { createInstallationToken, createOrUpdateCheckRun, createOrUpdateErroredGateCheckRun, createOrUpdateGateCheckRun, createOrUpdateOverriddenGateCheckRun, createOrUpdatePendingGateCheckRun, createOrUpdateSkippedGateCheckRun, getInstallationId, getRepositoryCollaboratorPermission } from "../github/app";
86+
import { createInstallationToken, createOrUpdateCheckRun, createOrUpdateErroredGateCheckRun, createOrUpdateGateCheckRun, createOrUpdateOverriddenGateCheckRun, createOrUpdatePendingGateCheckRun, createOrUpdateSkippedGateCheckRun, getInstallationId, getRepositoryCollaboratorPermission, isForeignAppInstallation } from "../github/app";
8787
import { AGENT_COMMAND_COMMENT_MARKER, createOrUpdateAgentCommandComment, createOrUpdatePrIntelligenceComment, PR_PANEL_COMMENT_MARKER } from "../github/comments";
8888
import { gittensoryFooter, gittensorRepoEarnUrl, maintainerControlPanelUrl } from "../github/footer";
8989
import {
@@ -1531,7 +1531,24 @@ async function processGitHubWebhook(env: Env, deliveryId: string, eventName: str
15311531
return;
15321532
}
15331533

1534-
await upsertInstallation(env, payload);
1534+
const installationAppId = await upsertInstallation(env, payload);
1535+
// Dual-app safety (#selfhost-app-id): if this delivery's installation belongs to a DIFFERENT gittensory App
1536+
// (cloud + self-host installed on the same account), ack it without processing so neither backend acts on the
1537+
// other's installation. FAIL-OPEN — an unknown/own-matching app_id always processes, so the LIVE single-app
1538+
// path is byte-identical. Signature verification (per-App secret) is the primary isolation; this is the
1539+
// belt-and-suspenders for a shared-endpoint/secret misconfig.
1540+
if (isForeignAppInstallation(env.GITHUB_APP_ID, installationAppId)) {
1541+
await recordWebhookEvent(env, {
1542+
deliveryId,
1543+
eventName,
1544+
action: payload.action,
1545+
installationId: payload.installation?.id,
1546+
repositoryFullName: payload.repository?.full_name,
1547+
payloadHash: "foreign_app",
1548+
status: "processed",
1549+
});
1550+
return;
1551+
}
15351552
const installationActor =
15361553
payload.installation?.account?.login ??
15371554
(payload.installation?.id ? (await getInstallation(env, payload.installation.id))?.accountLogin : undefined);

src/types.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ export type GitHubWebhookPayload = {
205205
action?: string;
206206
installation?: {
207207
id: number;
208+
app_id?: number;
208209
account?: {
209210
login?: string;
210211
id?: number;
@@ -1069,6 +1070,9 @@ export type InstallationRecord = {
10691070
id: number;
10701071
accountLogin: string;
10711072
accountId: number;
1073+
/** The GitHub App this installation belongs to (#selfhost-app-id); null until an `installation` event or the
1074+
* App-installation API refresh populates it. */
1075+
appId?: number | null | undefined;
10721076
targetType: string;
10731077
repositorySelection?: string | null | undefined;
10741078
permissions: Record<string, string>;

test/unit/github-app.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
getAppInstallation,
1111
getInstallationId,
1212
getRepositoryCollaboratorPermission,
13+
isForeignAppInstallation,
1314
} from "../../src/github/app";
1415
import type { Advisory } from "../../src/types";
1516
import { createTestEnv } from "../helpers/d1";
@@ -823,3 +824,24 @@ function gateAdvisory(headSha: string): Advisory {
823824
generatedAt: "2026-05-22T00:00:00.000Z",
824825
};
825826
}
827+
828+
describe("isForeignAppInstallation (#selfhost-app-id)", () => {
829+
it("returns true only on a positive numeric app_id mismatch", () => {
830+
expect(isForeignAppInstallation("12345", 99999)).toBe(true);
831+
});
832+
833+
it("returns false when this backend's own app id and the installation's match", () => {
834+
expect(isForeignAppInstallation("12345", 12345)).toBe(false);
835+
});
836+
837+
it("FAILS OPEN (false) when the installation app_id is unknown — null or undefined", () => {
838+
expect(isForeignAppInstallation("12345", null)).toBe(false);
839+
expect(isForeignAppInstallation("12345", undefined)).toBe(false);
840+
});
841+
842+
it("FAILS OPEN (false) when this backend has no / an unparseable own app id", () => {
843+
expect(isForeignAppInstallation(undefined, 99999)).toBe(false);
844+
expect(isForeignAppInstallation("", 99999)).toBe(false);
845+
expect(isForeignAppInstallation("not-a-number", 99999)).toBe(false);
846+
});
847+
});

test/unit/queue.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7506,3 +7506,70 @@ function reopenedPayload(sender: string): any {
75067506
},
75077507
};
75087508
}
7509+
7510+
describe("installation app_id capture + dual-app webhook filter (#selfhost-app-id)", () => {
7511+
it("captures app_id from an installation payload, returns it, and preserves it when a later payload omits it", async () => {
7512+
const env = createTestEnv();
7513+
const stored = await upsertInstallation(env, {
7514+
action: "created",
7515+
installation: { id: 4242, app_id: 555, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] },
7516+
});
7517+
expect(stored).toBe(555);
7518+
expect((await getInstallation(env, 4242))?.appId).toBe(555);
7519+
// A subsequent payload WITHOUT app_id (e.g. a pull_request event) must not clear the stored value.
7520+
const preserved = await upsertInstallation(env, { action: "synchronize", installation: { id: 4242, account: { login: "owner", id: 1, type: "Organization" } } });
7521+
expect(preserved).toBe(555);
7522+
expect((await getInstallation(env, 4242))?.appId).toBe(555);
7523+
});
7524+
7525+
it("acks a webhook whose installation belongs to a DIFFERENT app without processing it", async () => {
7526+
const env = createTestEnv(); // own GITHUB_APP_ID defaults to "3824093"
7527+
await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 7777);
7528+
// The installation is recorded as belonging to a FOREIGN app (99999 ≠ 3824093).
7529+
await upsertInstallation(env, { action: "created", installation: { id: 7777, app_id: 99999, account: { login: "JSONbored", id: 1, type: "User" }, repository_selection: "selected", permissions: {}, events: [] } });
7530+
vi.stubGlobal("fetch", async () => Response.json({}));
7531+
7532+
await processJob(env, {
7533+
type: "github-webhook",
7534+
deliveryId: "foreign-app-pr",
7535+
eventName: "pull_request",
7536+
payload: {
7537+
action: "opened",
7538+
installation: { id: 7777 }, // a PR event carries no app_id; the stored 99999 is used
7539+
repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } },
7540+
pull_request: { number: 88, title: "Foreign", state: "open", user: { login: "contributor" }, head: { sha: "f88" }, labels: [], body: "x" },
7541+
},
7542+
});
7543+
7544+
// The delivery was acked as foreign, and the PR was never upserted (the handler returned before the PR block).
7545+
const evt = await env.DB.prepare("select payload_hash from webhook_events where delivery_id = ?").bind("foreign-app-pr").first<{ payload_hash: string }>();
7546+
expect(evt?.payload_hash).toBe("foreign_app");
7547+
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 }>();
7548+
expect(pr?.n).toBe(0);
7549+
});
7550+
7551+
it("processes a webhook whose installation app_id matches this backend (no false filtering)", async () => {
7552+
const env = createTestEnv(); // own GITHUB_APP_ID "3824093"
7553+
await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 3824093001);
7554+
await upsertInstallation(env, { action: "created", installation: { id: 3824093001, app_id: 3824093, account: { login: "JSONbored", id: 1, type: "User" }, repository_selection: "selected", permissions: {}, events: [] } });
7555+
vi.stubGlobal("fetch", async () => Response.json({}));
7556+
7557+
await processJob(env, {
7558+
type: "github-webhook",
7559+
deliveryId: "own-app-pr",
7560+
eventName: "pull_request",
7561+
payload: {
7562+
action: "opened",
7563+
installation: { id: 3824093001 },
7564+
repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } },
7565+
pull_request: { number: 89, title: "Own", state: "open", user: { login: "contributor" }, head: { sha: "o89" }, labels: [], body: "x" },
7566+
},
7567+
});
7568+
7569+
// The matching-app webhook was processed normally — the PR row exists and it was NOT acked as foreign.
7570+
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 }>();
7571+
expect(pr?.n).toBe(1);
7572+
const evt = await env.DB.prepare("select payload_hash from webhook_events where delivery_id = ?").bind("own-app-pr").first<{ payload_hash: string }>();
7573+
expect(evt?.payload_hash).not.toBe("foreign_app");
7574+
});
7575+
});

0 commit comments

Comments
 (0)