feat(selfhost): record installation app_id and filter foreign-app webhooks - #1419
Merged
Conversation
…hooks 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.
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1419 +/- ##
=======================================
Coverage 95.37% 95.37%
=======================================
Files 192 192
Lines 20857 20866 +9
Branches 7542 7546 +4
=======================================
+ Hits 19892 19901 +9
Misses 383 383
Partials 582 582
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
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, each backend should only act on its own App's installations.
installations.app_idcolumn (Drizzle schema + migration0071), captured inupsertInstallationfrominstallationevents and the App-installation API refresh. A payload that omitsapp_id(e.g. apull_requestevent) never clears the stored value.upsertInstallationnow returns the resolvedapp_id, so the webhook entry can filter without a second DB read.isForeignAppInstallation(ownAppId, installationAppId): returnstrueonly on a positive numeric mismatch withGITHUB_APP_ID; fail-open on any unknown (no own id, unparseable id, or null/unknown installation app_id).webhook_eventspayload_hash = "foreign_app") without processing, so neither backend acts on the other's installation.Why it's safe for the live cloud: the per-App webhook secret (
GITHUB_WEBHOOK_SECRET) is the primary isolation — each backend already only accepts deliveries signed with its own App's secret — so this is defense-in-depth for a shared-endpoint/secret misconfig. It is fail-open: an unknown or own-matchingapp_idalways processes, so the live single-app path is byte-identical until the column is populated (existing rows backfill lazily on their nextinstallationevent).Part of [[gittensory-selfhost-migration-plan]] Phase 2. No GitHub issue — internal migration prereq.
Scope
src/) + a DB migration —schema.ts,repositories.ts,app.ts,processors.ts,types.ts,migrations/0071_installations_app_id.sqlwrangler.jsoncbinding/var change (GITHUB_APP_IDalready exists)0071,db:migrations:checkgreen)Validation
npm run test:ci— green (4513 passed | 4 skipped);db:migrations:checkcontiguousnpm run test:coverage— every changed line and branch covered (verified againstcoverage/lcov.info)npm audit --audit-level=moderate— 0 vulnerabilities;typecheck/ui:typecheckclean;git diff --checkcleanisForeignAppInstallation(mismatch / match / null-or-unknown fail-open / unparseable-own fail-open);app_idcapture + return + preserve-on-omit round trip; a foreign-app webhook is acked without upserting the PR; a matching-app webhook processes normally (no false filtering).Safety
app_idis unknown; live single-app path byte-identical