Skip to content

[Improve] Create public GitHub Apps so the install step picks the org#416

Merged
mrubens merged 4 commits into
developfrom
improve/github-app-public-install
Jul 16, 2026
Merged

[Improve] Create public GitHub Apps so the install step picks the org#416
mrubens merged 4 commits into
developfrom
improve/github-app-public-install

Conversation

@mrubens

@mrubens mrubens commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

The GitHub setup flow currently asks admins to type their organization name before creating the GitHub App. That was forced by creating the app as private: a private GitHub App can only be installed on the account that owns it, and GitHub's manifest-creation endpoint is owner-specific with no account picker.

This PR flips manifest-created apps to public: true, so the app can be installed on any account the admin belongs to. GitHub's own install screen (github.com/apps/{slug}/installations/new) then shows an account picker, and typing the org name is no longer needed for the common case.

Changes

  • buildGitHubAppManifest now sets public: true
  • The setup wizard and the settings GitHub setup no longer show the organization field by default; the intro copy explains that you pick the account or organization during the GitHub install step
  • The organization field moves behind the standard Show advanced config toggle (same pattern and field styling as the ADO/GitLab/Gitea config forms), for deployments that want the organization to own the app
  • The webhook route now verifies the delivery signature, then drops events from installations this deployment does not know before they are persisted or handled (isFromKnownInstallation); installation.created is accepted only when the installing account matches a pending-installation record, mirroring the check its handler enforces
  • Docs: the fast path describes the pick-at-install-time flow and the advanced option; the manual setup steps gained a step covering GitHub's "Where can this GitHub App be installed?" choice

Notes on public apps

Anyone who finds a deployment's app slug could install the public app on their own account. Those installations stay inert:

  • Signed webhook deliveries from unknown installations are rejected before recordWebhook, so outsider installs cannot fill the webhooks table or trigger handler work
  • installation.created is only accepted when the installing account matches a pending-installation record, both in the guard (before anything is persisted) and again in its handler
  • syncGitHubInstallations only iterates installations already in the database
  • The callback sync path requires a signed-in deployment admin

Existing deployments are unaffected; their apps stay private unless flipped to public in GitHub's app settings, which also enables the install-time picker for them.

Test plan

  • Updated the manifest unit test to assert public: true
  • Updated the setup wizard client tests: the org field is hidden until Show advanced config is clicked, and the entered org still flows through to app creation
  • New unit tests for isFromKnownInstallation covering the pending-install bypass, known/unknown installations, installation-less events, and malformed payloads
  • vitest, check-types, and lint pass

Manifest-created GitHub Apps are now public, so they can be installed on
any account via GitHub's own install-time account picker instead of
requiring the app to be created inside the target organization. The
organization name field is no longer needed for the common case and
moves behind the standard "Show advanced config" toggle, for deployments
that want the organization to own the app.

Unsolicited installations of a public app remain inert: the
installation.created webhook only completes installations with a
matching pending record, bulk sync only iterates installations already
in the database, and callback sync requires a deployment admin.
@roomote-roomote

roomote-roomote Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

No new code issues found. See task

  • High: apps/api/src/handlers/github/isFromKnownInstallation.ts:40-41 - The prior unknown-installation issue remains for installation.created: this unconditional exception lets every outsider installation reach recordWebhook before the handler checks for a pending account, so those deliveries are still persisted and processed. Check installation.account.id against github_pending_installations before allowing the event.
  • Medium: apps/api/src/handlers/github/index.ts:493 - The new webhooks.verify(...) call breaks the existing GitHub webhook router suite because its Octokit mock has no verify method; the targeted run now fails five tests with TypeError: webhooks.verify is not a function. Update the router mock/filter boundary and add route-level coverage for accepted and rejected installations.

Reviewed 47890b1

mrubens added 3 commits July 16, 2026 01:36
With a public app, any account can install it and produce validly
signed webhook deliveries. Verify the signature, then require the
payload's installation to already exist in github_installations before
the event is persisted or handled. installation.created still passes
through so pending installations can complete; that handler is guarded
by its own pending-record match.
The router test's Webhooks mock had no verify method, so every routed
delivery hit the catch path in CI. Mock verify and the new installation
guard, and add coverage for the invalid-signature and
unknown-installation early returns.
The unconditional installation.created bypass still let outsider
installs of the public app reach recordWebhook before the handler's
pending check rejected them. Match the installing account against
github_pending_installations in the guard itself - the same check the
handler enforces - so unrequested installs are dropped before anything
is persisted.
@mrubens
mrubens merged commit e2ff49c into develop Jul 16, 2026
18 checks passed
@mrubens
mrubens deleted the improve/github-app-public-install branch July 16, 2026 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant