Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions migrations/0071_installations_app_id.sql
Original file line number Diff line number Diff line change
@@ -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;
12 changes: 10 additions & 2 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ const FRESHNESS_SIGNAL_TYPES = [
"queue-health",
];

export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload): Promise<void> {
if (!payload.installation?.id) return;
export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload): Promise<number | null> {
if (!payload.installation?.id) return null;
const account = payload.installation.account;
const existing = await getInstallation(env, payload.installation.id);
const permissions =
Expand All @@ -195,13 +195,18 @@ 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)
.values({
id: payload.installation.id,
accountLogin,
accountId,
appId,
targetType,
repositorySelection,
permissionsJson: jsonString(permissions),
Expand All @@ -214,6 +219,7 @@ export async function upsertInstallation(env: Env, payload: GitHubWebhookPayload
set: {
accountLogin,
accountId,
appId,
targetType,
repositorySelection,
permissionsJson: jsonString(permissions),
Expand All @@ -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<void> {
Expand Down Expand Up @@ -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<Record<string, string>>(row.permissionsJson, {}),
Expand Down
4 changes: 4 additions & 0 deletions src/db/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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("{}"),
Expand Down
16 changes: 16 additions & 0 deletions src/github/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 19 additions & 2 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ export type GitHubWebhookPayload = {
action?: string;
installation?: {
id: number;
app_id?: number;
account?: {
login?: string;
id?: number;
Expand Down Expand Up @@ -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<string, string>;
Expand Down
22 changes: 22 additions & 0 deletions test/unit/github-app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
getAppInstallation,
getInstallationId,
getRepositoryCollaboratorPermission,
isForeignAppInstallation,
} from "../../src/github/app";
import type { Advisory } from "../../src/types";
import { createTestEnv } from "../helpers/d1";
Expand Down Expand Up @@ -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);
});
});
67 changes: 67 additions & 0 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});
Loading