diff --git a/README.md b/README.md index ff1f0237..4419722c 100644 --- a/README.md +++ b/README.md @@ -552,7 +552,8 @@ plugins for a given scope. Each entry is either: Valid plugin names: `repository`, `labels`, `collaborators`, `teams`, `milestones`, `branches`, `autolinks`, `validator`, `rulesets`, `environments`, -`custom_properties`, `custom_repository_roles`, `variables`, `archive`. +`custom_properties`, `custom_repository_roles`, `variables`, `archive`, +`app_installations`. #### Strip matrix (which source layers are removed before merge) @@ -661,6 +662,104 @@ additive_plugins: - collaborators ``` +### App installation management (`app_installations`) + +Most safe-settings plugins target a **repository**. The `app_installations` +plugin is different: its target is a **GitHub App installation**. It lets you +declaratively manage *which repositories a GitHub App can access* (the app's +`repository_selection`), using the same `org` → `suborg` → `repo` config +hierarchy you already use for repository settings. + +This is useful for controlling, as code, which repos apps such as Copilot, +Dependabot, or your own internal apps are installed on across the org. + +#### Prerequisites + +- Safe-settings must be installed on the **enterprise** with the **Enterprise + organization installations** permission (see the + [Enterprise organization installations API](https://docs.github.com/en/enterprise-cloud@latest/rest/enterprise-admin/organization-installations)). + Managing app installations requires an enterprise-level token; the regular + org installation token is not sufficient. If safe-settings is not installed + on the enterprise with this permission, app installation sync is reported as + an error and skipped. +- The enterprise slug is read from the webhook event payload + (`payload.enterprise.slug`); no extra environment variable is required. + +#### How repository selection is resolved + +The config layer where `app_installations` is declared determines which repos +are selected for the app: + +| Layer | File | Repos selected for the app | +| --- | --- | --- | +| Org | `settings.yml` | All repos in the org | +| Suborg | `suborgs/*.yml` | Repos matching the suborg's targeting (`suborgrepos`, `suborgteams`, `suborgproperties`) | +| Repo | `repos/.yml` | That specific repo | + +> [!important] +> An app listed at the **org** level (which implies all repos) takes +> precedence. Suborg/repo-level selections for that same app are ignored, and +> repos are never removed from it by incremental (suborg/repo) changes — it is +> reconciled only by the full (scheduled) sync. + +#### Examples + +Org-level `settings.yml` — give an app access to **all** repos in the org: + +```yaml +app_installations: + - app_slug: my-internal-app +``` + +Suborg-level `suborgs/backend.yml` — give an app access to the repos targeted +by this suborg (here, all repos with the `Team=backend` custom property): + +```yaml +suborgproperties: + - Team: backend +app_installations: + - app_slug: my-internal-app +``` + +Repo-level `repos/my-repo.yml` — add this specific repo to the app: + +```yaml +app_installations: + - app_slug: my-internal-app +``` + +Removing an app from a suborg/repo config (or changing the suborg's targeting) +removes the affected repos from that app on the next sync, unless another layer +still selects them. + +#### Sync behavior + +- **Incremental (delta) sync** runs when a `suborgs/*.yml` or `repos/*.yml` + file changes. Only the apps affected by the changed file are reconciled: the + previous version of the file is compared with the new one to compute repos to + add (`repository_selection`) and repos to remove (`repository_unselection`). + Additions are applied before removals (422-safe swap), so a repo removed by one config and + added by another ends up present. +- **Full sync** runs on the schedule (cron), on manual sync, and when + `settings.yml` changes. It recomputes the full desired state for every managed + app across all layers and reconciles it against the live installation state. + This is the mechanism that corrects any configuration drift. +- Add/remove operations are automatically batched in chunks of 50 repos (the + API limit). + +> [!note] +> Drift on managed apps is reconciled by the **full (cron) sync**, not by +> webhooks. A GitHub App only receives `installation` repository events for its +> *own* installation, so safe-settings cannot detect — via webhooks — when a +> human changes another app's repository access. Keep the scheduled sync enabled +> for timely drift correction. + +#### Disabling and additive mode + +`app_installations` honors both [`disable_plugins`](#disabling-plugins-disable_plugins) +and [`additive_plugins`](#additive-plugins-additive_plugins). In additive mode +the plugin only **adds** repos to installations and never removes them. + ### The Settings Files The settings files can be used to set the policies at the `org`, `suborg` or `repo` level. @@ -680,6 +779,7 @@ The following can be configured: - `Repository name validation` using regex pattern - `Rulesets` - `Environments` - wait timer, required reviewers, prevent self review, protected branches deployment branch policy, custom deployment branch policy, variables, deployment protection rules +- `App installations` - which repositories a GitHub App installation can access (see [App installation management](#app-installation-management-app_installations)) See [`docs/sample-settings/settings.yml`](docs/sample-settings/settings.yml) for a sample settings file. diff --git a/app.yml b/app.yml index e61ffdf3..9402d1df 100644 --- a/app.yml +++ b/app.yml @@ -31,13 +31,14 @@ default_events: # the value (for example, write). # Valid values are `read`, `write`, and `none` default_permissions: - repository_custom_properties: write - organization_custom_properties: admin - # Workflows, workflow runs and artifacts. (needed to read environments when repo is private or internal) # https://developer.github.com/v3/apps/permissions/#repository-permissions-for-actions actions: read + # Manage Actions variables. + # https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28 + actions_variables: write + # Repository creation, deletion, settings, teams, and collaborators. # https://developer.github.com/v3/apps/permissions/#permission-on-administration administration: write @@ -54,6 +55,10 @@ default_permissions: # https://developer.github.com/v3/apps/permissions/#permission-on-deployments # deployments: read + enterprise_organization_installations: write + + enterprise_organization_installation_repositories: write + # Manage repository environments. # https://developer.github.com/v3/apps/permissions/#repository-permissions-for-environments environments: write @@ -62,38 +67,14 @@ default_permissions: # https://developer.github.com/v3/apps/permissions/#permission-on-issues issues: write - # Search repositories, list collaborators, and access repository metadata. - # https://developer.github.com/v3/apps/permissions/#metadata-permissions - metadata: read - - # Retrieve Pages statuses, configuration, and builds, as well as create new builds. - # https://developer.github.com/v3/apps/permissions/#permission-on-pages - # pages: read - - # Pull requests and related comments, assignees, labels, milestones, and merges. - # https://developer.github.com/v3/apps/permissions/#permission-on-pull-requests - pull_requests: write - - # Manage the post-receive hooks for a repository. - # https://developer.github.com/v3/apps/permissions/#permission-on-repository-hooks - # repository_hooks: read - - # Manage repository projects, columns, and cards. - # https://developer.github.com/v3/apps/permissions/#permission-on-repository-projects - # repository_projects: read - - # Retrieve security vulnerability alerts. - # https://developer.github.com/v4/object/repositoryvulnerabilityalert/ - # vulnerability_alerts: read - - # Commit statuses. - # https://developer.github.com/v3/apps/permissions/#permission-on-statuses - statuses: write - # Organization members and teams. # https://developer.github.com/v3/apps/permissions/#permission-on-members members: write + # Search repositories, list collaborators, and access repository metadata. + # https://developer.github.com/v3/apps/permissions/#metadata-permissions + metadata: read + # View and manage users blocked by the organization. # https://developer.github.com/v3/apps/permissions/#permission-on-organization-user-blocking # organization_user_blocking: read @@ -122,9 +103,33 @@ default_permissions: # https://docs.github.com/en/enterprise-cloud@latest/rest/authentication/permissions-required-for-github-apps?apiVersion=2026-03-10#organization-permissions-for-custom-repository-roles organization_custom_roles: write - # Manage Actions variables. - # https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28 - actions_variables: write + organization_custom_properties: admin + + # Retrieve Pages statuses, configuration, and builds, as well as create new builds. + # https://developer.github.com/v3/apps/permissions/#permission-on-pages + # pages: read + + # Pull requests and related comments, assignees, labels, milestones, and merges. + # https://developer.github.com/v3/apps/permissions/#permission-on-pull-requests + pull_requests: write + + repository_custom_properties: write + + # Manage the post-receive hooks for a repository. + # https://developer.github.com/v3/apps/permissions/#permission-on-repository-hooks + # repository_hooks: read + + # Manage repository projects, columns, and cards. + # https://developer.github.com/v3/apps/permissions/#permission-on-repository-projects + # repository_projects: read + + # Retrieve security vulnerability alerts. + # https://developer.github.com/v4/object/repositoryvulnerabilityalert/ + # vulnerability_alerts: read + + # Commit statuses. + # https://developer.github.com/v3/apps/permissions/#permission-on-statuses + statuses: write # The name of the GitHub App. Defaults to the name specified in package.json diff --git a/docs/adr/0001-app-installation-plugin.md b/docs/adr/0001-app-installation-plugin.md new file mode 100644 index 00000000..5a4ec05a --- /dev/null +++ b/docs/adr/0001-app-installation-plugin.md @@ -0,0 +1,313 @@ +# 1. App installation management plugin + +- Status: Accepted +- Date: 2026-07-06 +- Last updated: 2026-07-08 — documented the repo-centric → app-centric shift and + the delta/full-sync model, and made reconciliation 422-safe (additions before + removals in both paths, with an explicit error when a `selected` installation + would be reduced to zero repositories). Earlier (2026-07-07): reporting subject + model, per-repo pipeline exclusion, startup verification, non-managed-app + safety guarantee, smoke tests, and removal of the redundant + `repository_selection` config attribute (org level is implicitly "all"). +- Deciders: safe-settings maintainers +- Related PR: `decyjphr-app-installation-plugin` + +## Context + +Safe-settings manages configuration whose **target is a repository** (branch +protection, labels, collaborators, …), with a couple of exceptions +(`rulesets`, `custom_repository_roles`) that target the organization. All of +these are driven through the org → suborg → repo configuration hierarchy and +applied by `syncAll` / `syncSelectedRepos` / `sync`. + +We need a new capability where the **target of the operation is a GitHub App +installation** rather than a repository. Concretely, safe-settings should +declaratively control **which repositories each installed GitHub App can +access** (the installation's repository access), driven by the same config +hierarchy: + +- **Org-level `settings.yml`** → the app should have access to **all** repos in + the org. +- **Suborg-level `suborgs/*.yml`** → repos selected by the suborg's targeting + criteria (custom properties, teams, names). +- **Repo-level `repos/*.yml`** → the specific repo, by name. + +Two hard constraints shaped the design: + +1. **A different credential is required.** Reading org/suborg/repo config and + resolving repos can use the normal per-installation Octokit client. But + **mutating an app's installation repository access** requires an Octokit + client authenticated as the App at the **enterprise** level, using the + [Enterprise Organization Installations API][ent-api] (permission: + *Enterprise organization installations*). +2. **Drift.** Humans can change an app's repo access outside safe-settings, so + we want to detect and revert that drift. + +We also want the design to accommodate **future non-repo targets** (e.g., +Copilot policies) without another ground-up rewrite. + +## Decision + +Add an `app_installations` plugin plus supporting infrastructure, wired into +the existing sync pipeline as a **separate phase**. + +This capability shifts safe-settings from a purely **repo-centric** model toward +a more general one. Until now, a trigger — a change to `settings.yml`, a +`suborgs/*.yml`, or a `repos/*.yml` — caused `syncAll` or `syncSelectedRepos` to +resolve a **collection of repositories** and invoke each plugin against them. +App installation management inverts part of that flow: a change still resolves a +collection of repositories, but it **also** resolves a **collection of +applications**, which are then processed iteratively. For each application the +plugin computes the set of repositories that should constitute its +`repository_selection`. + +The subtlety is that a single `repos/*.yml` change surfaces only **one** +repository in the changed set, whereas an app's desired access is the **union of +every** repository that currently targets it — conceptually `existing + new`. +Removal is harder still: to know which repositories an app should *lose*, we must +compare against the repositories derived from the **previous commit** on the +default branch. We call the process of computing these per-app additions and +deletions from a config change **delta sync**; a complementary **full sync** +recomputes each app's complete desired state from scratch to reconcile +configuration drift. Both are detailed under *Sync model* below. + +### Configuration shape + +```yaml +# settings.yml (org level) — implies "all repos" +app_installations: + - app_slug: ghas-compliance-decyjphr-emu + - app_slug: migrator-destination-dotcom + +# suborgs/team-a.yml — repos selected by this suborg's criteria +app_installations: + - app_slug: migrator-destination-dotcom + +# repos/my-repo.yml — this specific repo +app_installations: + - app_slug: migrator-destination-dotcom +``` + +### Components + +| Component | Responsibility | +| --- | --- | +| `lib/plugins/appInstallations.js` | Reconcile desired vs. live repo access per app (`syncDelta` / `syncFull`). Not `Diffable` — app installations are an org-scoped target, not a per-repo list. | +| `lib/appOctokitClient.js` | Enterprise-level App client for the Enterprise Organization Installations API. | +| `lib/repoSelector.js` | Resolve a repo set from **fixed** criteria: name, team, custom properties, or "all". | +| `lib/settings.js` | `syncAppInstallations` phase; delta computation from changed configs; full desired-state computation; app-aware NOP reporting. | +| `lib/nopcommand.js` | Carries an optional `subject` / `subjectType` so reporting can render the **app** (not the org placeholder repo) as the subject of a change. | +| `index.js` | Enterprise-client enrichment on the context (`getEnterpriseAppClient`); `installation_target` webhook handler; startup `verifyAppInstallationsPlugin` diagnostic. | + +### Sync model: delta vs. full + +- **Delta (`syncSelectedRepos`, push events)**: only apps that appear in + **changed** config files are "marked for change". For each changed + suborg/repo file we compute, per app: + - `repository_selection` — repos to **add**, + - `repository_unselection` — repos to **remove** (by diffing the previous + `baseRef` version of *that one file* — one extra fetch, reusing the existing + "removed from suborg targeting" pattern). + + Apps configured with org-level "all" are **not** handled in delta mode; they + are managed only by full sync. + +- **Full (`syncAll`, cron/manual)**: recompute the complete desired state for + every managed app across all config layers and reconcile against live API + state. This is the only place the expensive full computation runs, and the + only path that reconciles drift. + +### Enterprise API usage + +All mutations go through the org-scoped Enterprise Organization Installations +API (version `2026-03-10`) and operate on repository **names**: + +- List installations: `GET /enterprises/{ent}/apps/organizations/{org}/installations` +- List repos: `GET …/installations/{id}/repositories` +- Toggle all/selected: `PATCH …/installations/{id}/repositories` +- Add: `PATCH …/installations/{id}/repositories/add` +- Remove: `PATCH …/installations/{id}/repositories/remove` + +Add/remove are capped at **50 repos per call** and are auto-batched. + +### Reporting (NOP / PR check-run) + +The rest of safe-settings reports changes **per repository**. App installation +changes have no meaningful repository subject — the NopCommands are emitted +against the ` (org)` placeholder repo, which rendered confusingly (e.g. a +`**admin**` heading with a nested `value: (all repositories)` row). + +To fix this without a disruptive rename of the repo-centric reporting pipeline, +`NopCommand` gained an **optional, additive `subject` / `subjectType`**: + +- `subject` defaults to the repo, so every existing plugin is unaffected. +- `app_installations` sets `subject = `, `subjectType = 'app'`. +- Reporting groups changes by `subject` (identical to the repo for all other + plugins), so each **app** becomes its own heading. +- For `app_installations`, the impact summary pluralizes **apps** (not repos), + rows render as a flat `+`/`-` list of repositories, and the section is + excluded from the "repos affected" count (app subjects must not inflate it). +- The `NopCommand` constructor accepts the `subject` object in the `type` + position (defaulting `type` to `INFO`) so callers can pass a subject without + restating the default type. + +## Decisions and rationale + +1. **Enterprise auth is a prerequisite, not a config knob.** + safe-settings must already be installed on the enterprise with the + *Enterprise organization installations* permission. If it is not, the plugin + surfaces a clear error rather than accepting a separate private-key env var. + The enterprise slug is read from the webhook payload + (`payload.enterprise.slug`); the enterprise installation id is discovered via + `apps.listInstallations` (matching `target_type === 'Enterprise'` && + `account.slug === enterprise.slug`) and cached for reuse. + +2. **App installation sync is a separate phase**, not folded into `updateOrg()`. + This keeps repo iteration and app reconciliation independent and easier to + reason about and disable. + +3. **Fixed repo-selection criteria only** (name, team, custom properties, plus + "all"). No arbitrary Search API queries, to keep behavior predictable and + reuse existing `getReposForTeam` / `getRepositoriesByProperty` patterns. + +4. **Org-level "all" takes precedence** over any suborg/repo-level selection or + exclusion. An org-level `app_installations` entry lists only the `app_slug` + — it always implies **all** repos (there is no `repository_selection` config + attribute; it was removed as redundant). When an app is named at the org + level, the installation is toggled to `all` and deltas for that app are + skipped. + +5. **Repository NAMES, not IDs.** The Enterprise Org Installations API accepts + names, so the plugin no longer resolves names → IDs or enumerates all repos + for the "all" case (it uses the native toggle instead). + +6. **Additions before removals (both delta and full sync).** The add/remove + sets are always disjoint before they are applied: in **delta** mode + `_buildAppChangesFromDelta` drops any repo appearing in both selection and + unselection (selection wins), and in **full sync** they are disjoint by + construction (`toAdd = desired − live`, `toRemove = live − desired`). Because + the sets are disjoint, ordering does not change the final repo set — so + additions are applied **first** in both paths. Adding first prevents a + "swap" (e.g. live `{A}` → desired `{B}`) from momentarily dropping a + `selected` installation to zero repositories, which the Enterprise API + rejects with `422`. + + A `selected` installation must retain at least one repository, so a change + that would drive one to **zero** is surfaced as an error rather than + attempted — but *only* when it would actually strip access while + safe-settings is managing it. Concretely, an empty desired/required repo set + is **not** universally an error: + - **Full sync, non-additive, installation would lose all repos** (currently + `all` and asked to narrow to none, or currently `selected` with live repos + and desired resolves to none) → **error**; the installation is left + unchanged. Full sync knows the complete desired set, so it detects this + up-front. + - **Additive mode** → never an error: additive never narrows or removes, so + an empty desired set simply adds nothing. + - **Nothing to reconcile** (installation already has no relevant repos) → + no-op, no error. + - **Org-level `all`** → not applicable; the app is toggled to `all` and the + empty case never arises. + - **Delta mode** → an empty incremental change is a no-op. Delta does not + fetch live state, so it cannot know up-front that a removal would empty the + installation; it instead catches the `422` on removal, emits a descriptive + error, and defers the correct end state to the next full sync. + + +7. **Churn skip.** In delta mode, if an app's targeting is unchanged between the + previous and current versions of a file, it is skipped entirely to avoid + redundant add/remove writes. + +8. **Full-sync `current_selection` awareness.** Full sync reads each + installation's live `repository_selection` and chooses the minimal action: + skip when already correct; toggle `all` ↔ `selected`; or diff names and + add-then-remove when already `selected` (see decision #6). In `additive` mode + it never narrows an `all` installation. + +9. **`disable_plugins` / `additive_plugins` support.** `app_installations` + participates in the same gating: it can be disabled at any layer, and in + additive mode it only adds, never removes. + +10. **Future target abstraction.** The plugin is structured around a target that + is *not* a repository, paving the way for future targets (e.g., Copilot + policies) to reuse the same phase/plumbing without being repo-bound. + +11. **`app_installations` is excluded from the per-repo plugin pipeline.** + Although it is registered in `Settings.PLUGINS` (so `disable_plugins` / + `additive_plugins` name-validation and suborg-cleanup detection recognize + it), `childPluginsList` explicitly skips it. The per-repo pipeline calls + `instance.sync()`, which `AppInstallations` does not implement (it exposes + `syncDelta` / `syncFull` and has a different constructor signature). It is + reconciled **only** through the dedicated `syncAppInstallations` phase. + +12. **App is the reporting subject.** See *Reporting* above — an additive + `NopCommand.subject` avoids a global rename of the repo-centric pipeline + while presenting app installation changes with the app as the subject and + keeping repo counts accurate. + +13. **Startup verification.** On boot, `index.js` runs + `verifyAppInstallationsPlugin`: when `GH_ENTERPRISE` is set it mints an + enterprise installation token and confirms it can list app installations in + the target org (`GH_ORG`), logging a clear success/failure. When + `GH_ENTERPRISE` is unset the check is skipped, so non-enterprise + deployments are unaffected. This surfaces a mis-scoped enterprise install + early rather than at first sync. + +14. **Only explicitly-named apps are ever touched.** Both full and delta sync + operate solely on apps that appear in an `app_installations` entry in some + config layer. `listOrgInstallations` is used only to resolve + `app_slug → installation_id`; it never seeds the desired state, and there is + no "remove apps not in config" sweep. Consequently the safe-settings app's + own installation (and every other unlisted app) is left untouched on every + sync unless a config explicitly names it. + +## Consequences + +### Positive + +- App access is now declarative and flows through the existing config hierarchy. +- Delta processing keeps incremental (push-triggered) runs cheap. +- Names-based API + native "all" toggle removes an entire class of ID-resolution + and enumeration work. +- Batching respects the 50-repo API limit transparently. +- App installation changes read clearly in PR comments (app as subject) without + reworking the repo-centric reporting pipeline or distorting repo counts. +- Unlisted apps — including safe-settings itself — are provably never modified, + so enabling the plugin cannot accidentally lock the app out of repositories. +- A startup self-check catches missing/mis-scoped enterprise permissions before + the first sync. +- Smoke coverage (`smoke-test.js` Phase 17) exercises org-level `all`, + repo-level selection, add/remove, drift remediation via full sync, and + sub-org (delta) targeting, restoring each app's original state on teardown. + +### Negative / limitations + +- **Managed-app drift relies on the scheduled full sync.** An app only receives + `installation` repository events for its *own* installation, so there is no + webhook that reports drift on *other* managed apps. The + `installation.repositories_added/removed` handler was intentionally **removed** + because it could not detect managed-app drift; only `installation_target` is + retained. Drift on managed apps is reconciled on the next cron full sync. +- **Multi-suborg overlap** in delta mode is handled by de-duplicating the + selection/unselection sets (selection wins) and applying additions before + removals, so the net end state is correct and a `selected` installation is + never momentarily emptied. A removal that would still drop the installation to + zero repos (which delta cannot detect without a live-state fetch) is caught as + a `422`, reported, and reconciled by the next full sync. +- Requires an enterprise-level installation with the specific permission; orgs + not on enterprise cannot use the plugin. + +## Alternatives considered + +- **Enumerate all repos and add them individually for the "all" case** — + rejected in favor of the API's native `repository_selection: all` toggle + (fewer calls, no drift from newly created repos). +- **Arbitrary Search API queries for repo selection** — rejected for now in + favor of a fixed, predictable criteria set. +- **Suborg exclusions overriding org "all"** — rejected; org "all" takes + precedence to keep the mental model simple. +- **A dedicated private-key env var for enterprise auth** — rejected in favor of + reusing the existing app credentials and treating enterprise installation as a + prerequisite. + +[ent-api]: https://docs.github.com/en/enterprise-cloud@latest/rest/enterprise-admin/organization-installations?apiVersion=2026-03-10 diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 00000000..03a952f8 --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,10 @@ +# Architecture Decision Records + +This directory captures significant architectural decisions for safe-settings +using lightweight [ADRs](https://cognitect.com/blog/2011/11/15/documenting-architecture-decisions). + +Each record is numbered and immutable once accepted; supersede rather than edit. + +| ADR | Title | Status | +| --- | --- | --- | +| [0001](0001-app-installation-plugin.md) | App installation management plugin | Accepted | diff --git a/index.js b/index.js index d3b801c5..d3a0908b 100644 --- a/index.js +++ b/index.js @@ -6,12 +6,17 @@ const Glob = require('./lib/glob') const ConfigManager = require('./lib/configManager') const NopCommand = require('./lib/nopcommand') const SettingsGenerator = require('./lib/settingsGenerator') +const AppOctokitClient = require('./lib/appOctokitClient') const env = require('./lib/env') let deploymentConfig module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => { let appSlug = 'safe-settings' + // Cache of enterprise slug → enterprise installation id. Keyed by slug so a + // cached id is never reused for a different enterprise (e.g. when the app + // handles events from multiple enterprises). + const cachedEnterpriseInstallationIds = new Map() async function syncAllSettings (nop, context, repo = context.repo(), ref, baseRef, changedFiles = {}) { try { deploymentConfig = await loadYamlFileSystem() @@ -21,6 +26,9 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => const config = Object.assign({}, deploymentConfig, runtimeConfig) robot.log.debug(`config for ref ${ref} is ${JSON.stringify(config)}`) + // Enrich context with enterprise info for app installation management + await enrichContextWithEnterprise(context) + // Load base branch config for NOP filtering (only show PR-introduced changes) let baseConfig = null if (nop && baseRef) { @@ -88,6 +96,9 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => const config = Object.assign({}, deploymentConfig, runtimeConfig) robot.log.debug(`config for ref ${ref} is ${JSON.stringify(config)}`) + // Enrich context with enterprise info for app installation management + await enrichContextWithEnterprise(context) + // Load base branch config for NOP filtering (only show PR-introduced changes) let baseConfig = null if (nop && baseRef) { @@ -142,6 +153,102 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } } } + /** + * Lists all installations of the app using a JWT-authenticated client. + * + * @returns {Promise} All app installations + */ + async function listAllInstallations () { + const github = await robot.auth() + return github.paginate( + github.apps.listInstallations.endpoint.merge({ per_page: 100 }) + ) + } + + /** + * Finds the enterprise installation matching the given slug from the app's + * installation list. Returns null if none matches. + * + * @param {string} enterpriseSlug - Enterprise slug + * @returns {Promise} The matching enterprise installation + */ + async function findEnterpriseInstallation (enterpriseSlug) { + const installations = await listAllInstallations() + return installations.find( + i => i.target_type === 'Enterprise' && i.account && `${i.account.slug}`.toLowerCase() === enterpriseSlug.toLowerCase() + ) || null + } + + /** + * Finds the enterprise installation for a given slug and returns an Octokit + * client authenticated with the enterprise installation token, along with + * the installation ID. + * + * Uses the cached enterprise installation ID for the given slug when + * available to avoid re-listing installations. The cache is keyed by + * enterprise slug so an id is never reused across enterprises. Returns null + * if no matching enterprise installation is found. + * + * @param {string} enterpriseSlug - Enterprise slug + * @returns {Promise<{ appGithub: object, installationId: number } | null>} + */ + async function getEnterpriseAppClient (enterpriseSlug) { + if (!enterpriseSlug) return null + // Normalize the slug to lowercase for consistent cache keying + enterpriseSlug = enterpriseSlug.toLowerCase() + + // Use the cached enterprise installation id for THIS slug if available. + // Keying by slug ensures a cached id is never reused for a different + // enterprise. + const cachedId = cachedEnterpriseInstallationIds.get(enterpriseSlug) + if (cachedId) { + try { + const appGithub = await robot.auth(cachedId) + return { appGithub, installationId: cachedId } + } catch (e) { + cachedEnterpriseInstallationIds.delete(enterpriseSlug) + } + } + + // Find the installation targeting this enterprise + const enterpriseInstallation = await findEnterpriseInstallation(enterpriseSlug) + if (!enterpriseInstallation) { + return null + } + cachedEnterpriseInstallationIds.set(enterpriseSlug, enterpriseInstallation.id) + const enterpriseGithub = await robot.auth(enterpriseInstallation.id) + return { appGithub: enterpriseGithub, installationId: enterpriseInstallation.id } + } + + /** + * Enriches the context with enterprise info for app installation management. + * Extracts enterprise slug from the webhook payload, finds the enterprise + * installation from the app's installation list, and creates an Octokit + * client authenticated with the enterprise installation token. + * + * @param {object} context - Probot context + */ + async function enrichContextWithEnterprise (context) { + const { payload } = context + const slugFromPayload = (payload.enterprise && payload.enterprise.slug) || + (payload.installation && payload.installation.enterprise && payload.installation.enterprise.slug) + const enterpriseSlug = slugFromPayload || process.env.GH_ENTERPRISE + + if (!enterpriseSlug) return + + context.enterpriseSlug = enterpriseSlug + try { + const result = await getEnterpriseAppClient(enterpriseSlug) + if (result) { + context.appGithub = result.appGithub + } else { + robot.log.debug(`No enterprise installation found for slug '${enterpriseSlug}'. App installation management will not be available.`) + } + } catch (e) { + robot.log.debug(`Could not create enterprise-authenticated client: ${e.message}`) + } + } + /** * Loads the deployment config file from file system * Do this once when the app starts and then return the cached value @@ -228,27 +335,68 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } async function info () { - const github = await robot.auth() - const installations = await github.paginate( - github.apps.listInstallations.endpoint.merge({ per_page: 100 }) - ) + const installations = await listAllInstallations() robot.log.debug(`installations: ${JSON.stringify(installations)}`) if (installations.length > 0) { const installation = installations[0] const github = await robot.auth(installation.id) const app = await github.apps.getAuthenticated() appSlug = app.data.slug - robot.log.debug(`Validated the app is configured properly = \n${JSON.stringify(app.data, null, 2)}`) + robot.log.info(`Validated the app is configured properly = \n${JSON.stringify(app.data, null, 2)}`) + } + + await verifyAppInstallationsPlugin() + } + + /** + * Verifies that the app-installations plugin can function properly. + * + * When the `GH_ENTERPRISE` env variable is set, this: + * 1. Finds the enterprise installation matching the slug. + * 2. Mints an installation token for that enterprise installation. + * 3. Confirms the token has permission to manage app installations in the + * target org (`GH_ORG`) by listing the org's app installations via the + * Enterprise organization installations API. + * + * If `GH_ENTERPRISE` is not set, this verification is skipped entirely. + */ + async function verifyAppInstallationsPlugin () { + const enterpriseSlug = process.env.GH_ENTERPRISE + if (!enterpriseSlug) { + robot.log.info('GH_ENTERPRISE is not set — skipping app-installations plugin verification') + return + } + + const org = process.env.GH_ORG + if (!org) { + robot.log.warn('GH_ENTERPRISE is set but GH_ORG is not — cannot verify app-installations plugin without a target org') + return + } + + try { + const result = await getEnterpriseAppClient(enterpriseSlug) + if (!result) { + robot.log.warn(`No enterprise installation found for slug '${enterpriseSlug}'. App-installations plugin will not be able to manage app access. Ensure safe-settings is installed on the enterprise.`) + return + } + + const client = new AppOctokitClient({ + github: result.appGithub, + enterpriseSlug, + log: robot.log + }) + + // Confirm the token can list org app installations (validates permission) + const orgInstallations = await client.listOrgInstallations(org) + robot.log.info(`App-installations plugin verified: enterprise '${enterpriseSlug}' installation (id: ${result.installationId}) can manage apps in org '${org}' (${orgInstallations.length} installation(s) visible)`) + } catch (e) { + robot.log.error(`App-installations plugin verification failed for enterprise '${enterpriseSlug}' / org '${org}': ${e.message}`) } } async function syncInstallation (nop = false) { robot.log.trace('Fetching installations') - const github = await robot.auth() - - const installations = await github.paginate( - github.apps.listInstallations.endpoint.merge({ per_page: 100 }) - ) + const installations = await listAllInstallations() if (installations.length > 0) { const installation = installations[0] @@ -482,6 +630,39 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } }) + // ──────────────────────────────────────────────────────────────────────── + // App installation target handler + // + // Note: We intentionally do NOT handle `installation.repositories_added` / + // `installation.repositories_removed`. A GitHub App only receives those + // events for its OWN installation, not for the managed apps (e.g. Copilot, + // Dependabot) whose repository access safe-settings controls. They cannot + // detect drift on managed apps, so drift is reconciled by the scheduled + // (cron) full sync instead. + // ──────────────────────────────────────────────────────────────────────── + + robot.on('installation_target', async context => { + const { payload } = context + const { sender } = payload + robot.log.debug('Installation target changed by ', JSON.stringify(sender)) + if (sender.type === 'Bot') { + robot.log.debug('Installation target changed by Bot') + return + } + robot.log.debug('Installation target changed by a Human — triggering sync to revert drift') + + const orgLogin = (payload.organization && payload.organization.login) || + (payload.installation && payload.installation.account && payload.installation.account.login) + if (!orgLogin) { + robot.log.debug('Could not determine org login from installation_target event, skipping') + return + } + const updatedContext = Object.assign({}, context, { + repo: () => { return { repo: env.ADMIN_REPO, owner: orgLogin } } + }) + return syncAllSettings(false, updatedContext) + }) + robot.on('check_suite.requested', async context => { const { payload } = context const { repository } = payload diff --git a/lib/appOctokitClient.js b/lib/appOctokitClient.js new file mode 100644 index 00000000..c888f97e --- /dev/null +++ b/lib/appOctokitClient.js @@ -0,0 +1,189 @@ +const BATCH_SIZE = 50 +const API_VERSION = '2026-03-10' + +/** + * AppOctokitClient wraps an Octokit client authenticated as the GitHub App at + * the enterprise level and provides methods for managing GitHub App + * installation repository access via the Enterprise Organization Installations + * API. + * + * All endpoints are org-scoped under + * `/enterprises/{enterprise}/apps/organizations/{org}/...` and operate on + * repository **names** (not IDs). Add/remove are capped at 50 repos per call + * and are auto-batched here. + * + * Prerequisites: + * - safe-settings must be installed on the enterprise with the + * "Enterprise organization installations" permission. + * - The enterprise slug is obtained from the webhook event payload + * (payload.enterprise.slug). + * + * @see https://docs.github.com/en/enterprise-cloud@latest/rest/enterprise-admin/organization-installations + * + * @param {object} options + * @param {object} options.github - Octokit client authenticated as the app at the enterprise installation + * @param {string} options.enterpriseSlug - Enterprise slug from webhook payload + * @param {object} options.log - Logger instance + */ +class AppOctokitClient { + constructor ({ github, enterpriseSlug, log }) { + this.github = github + this.enterpriseSlug = enterpriseSlug + this.log = log + } + + /** + * List the GitHub App installations on an enterprise-owned organization. + * Returns array of installation objects with + * { id, app_slug, client_id, repository_selection, ... } + * + * @param {string} org - Organization login name + * @returns {Promise} List of installations + */ + async listOrgInstallations (org) { + try { + const options = this.github.request.endpoint.merge( + 'GET /enterprises/{enterprise}/apps/organizations/{org}/installations', + { + enterprise: this.enterpriseSlug, + org, + headers: { 'X-GitHub-Api-Version': API_VERSION } + } + ) + return await this.github.paginate(options) + } catch (e) { + if (e.status === 403 || e.status === 404) { + throw new Error( + `Cannot access enterprise installations API. Ensure safe-settings is installed on the enterprise '${this.enterpriseSlug}' with 'Enterprise organization installations' permission. Error: ${e.message}` + ) + } + throw e + } + } + + /** + * List repositories accessible to an app installation on an org. + * Returns array of { id, name, full_name }. + * + * @param {string} org - Organization login name + * @param {number} installationId - The installation ID + * @returns {Promise} List of repository objects + */ + async listInstallationRepos (org, installationId) { + try { + const options = this.github.request.endpoint.merge( + 'GET /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories', + { + enterprise: this.enterpriseSlug, + org, + installation_id: installationId, + headers: { 'X-GitHub-Api-Version': API_VERSION } + } + ) + return await this.github.paginate(options) + } catch (e) { + this.log.error(`Error listing repos for installation ${installationId}: ${e.message}`) + throw e + } + } + + /** + * Toggle an installation's repository access between 'all' and 'selected'. + * When setting 'selected', `repositories` (names) must contain at least one + * repo. When setting 'all', `repositories` must be omitted. + * + * @param {string} org - Organization login name + * @param {number} installationId - The installation ID + * @param {('all'|'selected')} selection - Desired repository selection + * @param {string[]} [repositories] - Repo names (required for 'selected') + * @returns {Promise} + */ + async setRepositorySelection (org, installationId, selection, repositories) { + const params = { + enterprise: this.enterpriseSlug, + org, + installation_id: installationId, + repository_selection: selection, + headers: { 'X-GitHub-Api-Version': API_VERSION } + } + if (selection === 'selected') { + params.repositories = repositories || [] + } + this.log.debug(`Setting repository_selection='${selection}' for installation ${installationId}`) + await this.github.request( + 'PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories', + params + ) + } + + /** + * Grant repository access to an org installation. + * Automatically batches into chunks of 50 (API limit). + * + * @param {string} org - Organization login name + * @param {number} installationId - The installation ID + * @param {string[]} repositoryNames - Repo names to add + * @returns {Promise} + */ + async addReposToInstallation (org, installationId, repositoryNames) { + if (!repositoryNames || repositoryNames.length === 0) return + + for (const batch of this._chunk(repositoryNames, BATCH_SIZE)) { + this.log.debug(`Adding ${batch.length} repos to installation ${installationId}`) + await this.github.request( + 'PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories/add', + { + enterprise: this.enterpriseSlug, + org, + installation_id: installationId, + repositories: batch, + headers: { 'X-GitHub-Api-Version': API_VERSION } + } + ) + } + } + + /** + * Remove repository access from an org installation. + * Automatically batches into chunks of 50 (API limit). + * + * Note: the API returns 422 if you attempt to remove repos from an + * installation set to 'all', or remove the last remaining repository. + * + * @param {string} org - Organization login name + * @param {number} installationId - The installation ID + * @param {string[]} repositoryNames - Repo names to remove + * @returns {Promise} + */ + async removeReposFromInstallation (org, installationId, repositoryNames) { + if (!repositoryNames || repositoryNames.length === 0) return + + for (const batch of this._chunk(repositoryNames, BATCH_SIZE)) { + this.log.debug(`Removing ${batch.length} repos from installation ${installationId}`) + await this.github.request( + 'PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories/remove', + { + enterprise: this.enterpriseSlug, + org, + installation_id: installationId, + repositories: batch, + headers: { 'X-GitHub-Api-Version': API_VERSION } + } + ) + } + } + + /** + * Split an array into chunks of the given size. + * @private + */ + _chunk (array, size) { + const chunks = [] + for (let i = 0; i < array.length; i += size) { + chunks.push(array.slice(i, i + size)) + } + return chunks + } +} + +module.exports = AppOctokitClient diff --git a/lib/nopcommand.js b/lib/nopcommand.js index 75965a8b..133eae48 100644 --- a/lib/nopcommand.js +++ b/lib/nopcommand.js @@ -1,8 +1,22 @@ class NopCommand { - constructor (pluginName, repo, endpoint, action, type = 'INFO') { + constructor (pluginName, repo, endpoint, action, type = 'INFO', subject = null) { + // Ergonomic overload: allow passing the subject in the `type` position so + // callers can supply a subject while omitting the (default 'INFO') type, + // e.g. new NopCommand(plugin, repo, endpoint, action, { name, type }). + if (type !== null && typeof type === 'object') { + subject = type + type = 'INFO' + } this.type = type this.plugin = pluginName this.repo = repo.repo + // Optional presentation subject. Some plugins (e.g. app_installations) + // operate on a non-repo entity — the "subject" of the change is a GitHub + // App, not a repository. `subject` overrides the repo as the heading in the + // PR-comment/check-run report, while `repo` is still used for grouping and + // repo counts. Defaults to the repo so existing plugins are unaffected. + this.subject = (subject && subject.name) || repo.repo + this.subjectType = (subject && subject.type) || 'repo' this.endpoint = endpoint ? endpoint.url : '' this.body = endpoint ? endpoint.body : '' // check if action is a string diff --git a/lib/plugins/appInstallations.js b/lib/plugins/appInstallations.js new file mode 100644 index 00000000..7eea8432 --- /dev/null +++ b/lib/plugins/appInstallations.js @@ -0,0 +1,389 @@ +/* eslint-disable camelcase */ +const NopCommand = require('../nopcommand') +const AppOctokitClient = require('../appOctokitClient') + +/** + * AppInstallations plugin manages which repositories are accessible to + * GitHub App installations in the organization. + * + * Unlike repo-targeting plugins (which extend Diffable), this plugin + * operates at the org level — the "target" is an app installation, + * not a repository. + * + * Supports: + * - Delta-based sync (incremental changes from config file diffs) + * - Full sync (compare desired state against live API state) + * - disable_plugins (skipped when disabled) + * - additive_plugins (only adds repos, never removes) + */ +class AppInstallations { + /** + * @param {boolean} nop - Dry-run mode + * @param {object} github - Octokit client (installation-authenticated) + * @param {object} appGithub - Octokit client (app-authenticated, for enterprise API) + * @param {object} repo - { owner, repo } context + * @param {string} enterpriseSlug - Enterprise slug from webhook payload + * @param {object} log - Logger + * @param {Array} errors - Shared errors array + */ + constructor (nop, github, appGithub, repo, enterpriseSlug, log, errors) { + this.nop = nop + this.github = github + this.repo = repo + this.org = repo.owner + this.log = log + this.errors = errors || [] + this.additive = false + + if (appGithub && enterpriseSlug) { + this.enterpriseClient = new AppOctokitClient({ + github: appGithub, + enterpriseSlug, + log + }) + } + } + + /** + * Delta-based sync: process pre-computed per-app changes. + * + * @param {Array} appChanges - Array of per-app change objects: + * { + * app_slug: string, + * installation_id: number, + * repository_selection: Set | Array | 'all', // repos to add + * repository_unselection: Set | Array, // repos to remove + * } + * @returns {Promise} NopCommand results (in nop mode) or empty + */ + async syncDelta (appChanges) { + const results = [] + + if (!appChanges || appChanges.length === 0) return results + + for (const change of appChanges) { + try { + const appResults = await this._processAppChange(change) + results.push(...appResults) + } catch (e) { + this.log.error(`Error processing app installation '${change.app_slug}': ${e.message}`) + this.errors.push({ + owner: this.repo.owner, + repo: this.repo.repo, + msg: e.message, + plugin: 'app_installations' + }) + if (this.nop) { + results.push(new NopCommand( + 'app_installations', + this.repo, + null, + `Error: ${e.message}`, + 'ERROR' + )) + } + } + } + + return results + } + + /** + * Full sync: compute full desired state for all managed apps, + * compare against live API state, and reconcile. + * + * @param {object} desiredState - Map of app_slug → { + * installation_id, repos: Set | 'all', current_selection: 'all' | 'selected' + * } + * @returns {Promise} NopCommand results (in nop mode) or empty + */ + async syncFull (desiredState) { + const results = [] + + if (!desiredState || Object.keys(desiredState).length === 0) return results + + if (!this.enterpriseClient) { + const msg = 'Cannot sync app installations: enterprise client not configured. Ensure safe-settings is installed on the enterprise.' + this.log.error(msg) + if (this.nop) { + results.push(new NopCommand('app_installations', this.repo, null, msg, 'ERROR')) + } + return results + } + + for (const [appSlug, desired] of Object.entries(desiredState)) { + try { + const appResults = await this._reconcileApp(appSlug, desired) + results.push(...appResults) + } catch (e) { + this.log.error(`Error in full sync for app '${appSlug}': ${e.message}`) + this.errors.push({ + owner: this.repo.owner, + repo: this.repo.repo, + msg: e.message, + plugin: 'app_installations' + }) + if (this.nop) { + results.push(new NopCommand('app_installations', this.repo, null, `Error: ${e.message}`, 'ERROR')) + } + } + } + + return results + } + + /** + * Reconcile a single app's desired state against its live installation state. + * @private + */ + async _reconcileApp (appSlug, desired) { + const results = [] + const { installation_id, repos, current_selection } = desired + + // Desired = all repos in the org → toggle the installation to 'all'. + if (repos === 'all') { + if (current_selection === 'all') { + this.log.debug(`App '${appSlug}': already set to all repositories, no change`) + return results + } + if (this.nop) { + results.push(new NopCommand('app_installations', this.repo, null, { + msg: `App '${appSlug}': set repository_selection to 'all'`, + additions: ['(all repositories)'], + modifications: null, + deletions: null + }, { name: appSlug, type: 'app' })) + return results + } + await this.enterpriseClient.setRepositorySelection(this.org, installation_id, 'all') + this.log.debug(`App '${appSlug}': set repository_selection to 'all'`) + return results + } + + const desiredNames = repos instanceof Set ? repos : new Set(repos) + + // Installation is currently 'all' but desired is a specific set → switch + // the installation to 'selected' with the desired repos. In additive mode + // we must not narrow access, so leave 'all' untouched. + if (current_selection === 'all') { + if (this.additive) { + this.log.debug(`App '${appSlug}': additive mode, leaving 'all' selection untouched`) + return results + } + if (desiredNames.size === 0) { + // The desired set is empty but the installation is 'all'. We cannot + // narrow to zero repositories (the API rejects selecting none), so the + // over-broad 'all' access would otherwise be silently preserved. + // Surface this as an error so operators notice the (likely) + // misconfiguration; the installation is left unchanged. + const msg = `App '${appSlug}': desired repo set is empty, so safe-settings cannot narrow repository_selection from 'all' to 'selected' (the API rejects selecting zero repositories). Add at least one repository to the app's configuration, set the app to all repos at the org level, or remove the app_installations entry.` + this.log.error(msg) + this.errors.push({ owner: this.repo.owner, repo: this.repo.repo, msg, plugin: 'app_installations' }) + if (this.nop) { + results.push(new NopCommand('app_installations', this.repo, null, msg, 'ERROR', { name: appSlug, type: 'app' })) + } + return results + } + if (this.nop) { + results.push(new NopCommand('app_installations', this.repo, null, { + msg: `App '${appSlug}': narrow repository_selection from 'all' to selected`, + additions: [...desiredNames], + modifications: null, + deletions: ['(all repositories)'] + }, { name: appSlug, type: 'app' })) + return results + } + await this.enterpriseClient.setRepositorySelection(this.org, installation_id, 'selected', [...desiredNames]) + this.log.debug(`App '${appSlug}': set repository_selection to 'selected' with ${desiredNames.size} repos`) + return results + } + + // Installation is 'selected' → diff against live repos and add/remove. + const liveRepos = await this.enterpriseClient.listInstallationRepos(this.org, installation_id) + const liveRepoNames = new Set(liveRepos.map(r => r.name)) + + const toAdd = [...desiredNames].filter(r => !liveRepoNames.has(r)) + const toRemove = this.additive + ? [] // Additive mode: never remove + : [...liveRepoNames].filter(r => !desiredNames.has(r)) + + if (toAdd.length === 0 && toRemove.length === 0) { + this.log.debug(`App '${appSlug}': no changes needed`) + return results + } + + // A 'selected' installation must retain at least one repository. When the + // resolved desired set is empty we would have to remove every repo, which + // the Enterprise API rejects (422). There is no valid reconciliation to + // zero repos — surface a descriptive error instead of attempting it. + if (desiredNames.size === 0) { + const msg = `App '${appSlug}': cannot reconcile a 'selected' installation to zero repositories (the resolved repo set is empty). Add at least one repository to the app's configuration, set the app to all repos at the org level, or remove the app_installations entry.` + this.log.error(msg) + this.errors.push({ owner: this.repo.owner, repo: this.repo.repo, msg, plugin: 'app_installations' }) + if (this.nop) { + results.push(new NopCommand('app_installations', this.repo, null, msg, 'ERROR', { name: appSlug, type: 'app' })) + } + return results + } + + if (this.nop) { + results.push(new NopCommand('app_installations', this.repo, null, { + msg: `App '${appSlug}' installation repos`, + additions: toAdd.length > 0 ? toAdd : null, + modifications: null, + deletions: toRemove.length > 0 ? toRemove : null + }, { name: appSlug, type: 'app' })) + return results + } + + // Apply additions BEFORE removals. In a swap (e.g. live={A}, desired={B}) + // removing first would momentarily drop the installation to zero repos, + // which the Enterprise API rejects with 422 ('selected' installations must + // keep at least one repo). Adding first guarantees the installation never + // passes through an empty state. In full sync toAdd/toRemove are disjoint, + // so ordering does not change the final repo set. + if (toAdd.length > 0) { + await this.enterpriseClient.addReposToInstallation(this.org, installation_id, toAdd) + this.log.debug(`App '${appSlug}': added ${toAdd.length} repos`) + } + if (toRemove.length > 0) { + try { + await this.enterpriseClient.removeReposFromInstallation(this.org, installation_id, toRemove) + this.log.debug(`App '${appSlug}': removed ${toRemove.length} repos`) + } catch (e) { + if (e && e.status === 422) { + const msg = `App '${appSlug}': removing ${toRemove.length} repo(s) was rejected by the Enterprise API (422); a 'selected' installation must retain at least one repository.` + this.log.error(msg) + this.errors.push({ owner: this.repo.owner, repo: this.repo.repo, msg, plugin: 'app_installations' }) + return results + } + throw e + } + } + + return results + } + + /** + * Process a single app's delta change. + * @private + */ + async _processAppChange (change) { + const results = [] + const { app_slug, installation_id } = change + + // Normalise selection/unselection to Sets. Delta changes computed by + // Settings._buildAppChangesFromDelta arrive as arrays, while direct callers + // and unit tests pass Sets — accept both. 'all' is a sentinel for the + // whole-org toggle and is handled separately below. + const repository_selection = change.repository_selection === 'all' + ? 'all' + : (change.repository_selection instanceof Set + ? change.repository_selection + : new Set(change.repository_selection || [])) + const repository_unselection = change.repository_unselection instanceof Set + ? change.repository_unselection + : new Set(change.repository_unselection || []) + + if (!this.enterpriseClient) { + const msg = 'Cannot sync app installations: enterprise client not configured. Ensure safe-settings is installed on the enterprise.' + this.log.error(msg) + this.errors.push({ + owner: this.repo.owner, + repo: this.repo.repo, + msg, + plugin: 'app_installations' + }) + if (this.nop) { + results.push(new NopCommand('app_installations', this.repo, null, msg, 'ERROR')) + } + return results + } + + const hasSelections = repository_selection === 'all' || + (repository_selection instanceof Set && repository_selection.size > 0) + const hasUnselections = !this.additive && + (repository_unselection instanceof Set && repository_unselection.size > 0) + + if (!hasSelections && !hasUnselections) { + return results + } + + // Handle "all" selection — toggle the installation to 'all' via the API + if (repository_selection === 'all') { + if (this.nop) { + results.push(new NopCommand( + 'app_installations', + this.repo, + null, + { + msg: `App '${app_slug}': set repository_selection to 'all'`, + additions: ['(all repositories)'], + modifications: null, + deletions: null + }, + { name: app_slug, type: 'app' } + )) + return results + } + + await this.enterpriseClient.setRepositorySelection(this.org, installation_id, 'all') + this.log.debug(`App '${app_slug}': set repository_selection to 'all'`) + return results + } + + // Handle specific repos + if (this.nop) { + const additions = hasSelections ? [...repository_selection] : null + const deletions = hasUnselections ? [...repository_unselection] : null + results.push(new NopCommand( + 'app_installations', + this.repo, + null, + { + msg: `App '${app_slug}' installation repos`, + additions, + modifications: null, + deletions + }, + { name: app_slug, type: 'app' } + )) + return results + } + + // Apply additions BEFORE removals. Delta selection/unselection sets are + // disjoint (Settings._buildAppChangesFromDelta drops any repo appearing in + // both, selection winning), so ordering does not change the final set. + // Adding first avoids a transient empty selection during a "swap" (e.g. a + // suborg retargeted from repo-A to repo-B where the installation currently + // holds only repo-A), which the Enterprise API rejects with 422 ('selected' + // installations must keep at least one repo). + if (hasSelections) { + await this.enterpriseClient.addReposToInstallation(this.org, installation_id, [...repository_selection]) + this.log.debug(`App '${app_slug}': added ${repository_selection.size} repos`) + } + + if (hasUnselections) { + try { + await this.enterpriseClient.removeReposFromInstallation(this.org, installation_id, [...repository_unselection]) + this.log.debug(`App '${app_slug}': removed ${repository_unselection.size} repos`) + } catch (e) { + if (e && e.status === 422) { + // Delta does not fetch live installation state, so it cannot detect + // up-front that a removal would drop the installation to zero repos. + // Surface a descriptive error; the scheduled full sync reconciles the + // correct end state. + const msg = `App '${app_slug}': removing ${repository_unselection.size} repo(s) was rejected by the Enterprise API (422); a 'selected' installation must retain at least one repository. This will be reconciled on the next full sync.` + this.log.error(msg) + this.errors.push({ owner: this.repo.owner, repo: this.repo.repo, msg, plugin: 'app_installations' }) + return results + } + throw e + } + } + + return results + } +} + +module.exports = AppInstallations diff --git a/lib/repoSelector.js b/lib/repoSelector.js new file mode 100644 index 00000000..47b40163 --- /dev/null +++ b/lib/repoSelector.js @@ -0,0 +1,159 @@ +const Glob = require('./glob') + +/** + * RepoSelector resolves a set of repository names from fixed criteria. + * + * Supported criteria: + * - name: explicit repo names (or glob patterns) + * - team: repos belonging to a GitHub team + * - custom_properties: repos matching custom property values + * - all: all repos visible to the installation + * + * @param {object} github - Authenticated Octokit client + * @param {string} org - Organization name + * @param {object} log - Logger instance + */ +class RepoSelector { + constructor (github, org, log) { + this.github = github + this.org = org + this.log = log + } + + /** + * Resolve repos from a list of criteria. Returns a Set of repo names. + * + * @param {object} criteria - Selection criteria + * @param {boolean} [criteria.all] - Select all repos in the org + * @param {string[]} [criteria.names] - Explicit repo names or glob patterns + * @param {string[]} [criteria.teams] - Team slugs + * @param {object[]} [criteria.custom_properties] - Array of { name: value } property filters + * @returns {Promise>} Set of resolved repo names + */ + async resolve (criteria) { + if (!criteria) return new Set() + + // "all" takes precedence — return all repos without filtering + if (criteria.all) { + return this.getAllRepos() + } + + const results = new Set() + const promises = [] + + if (criteria.names && Array.isArray(criteria.names)) { + promises.push(this.resolveByName(criteria.names)) + } + + if (criteria.teams && Array.isArray(criteria.teams)) { + promises.push(this.resolveByTeam(criteria.teams)) + } + + if (criteria.custom_properties && Array.isArray(criteria.custom_properties)) { + promises.push(this.resolveByCustomProperties(criteria.custom_properties)) + } + + const resolved = await Promise.all(promises) + for (const repoSet of resolved) { + for (const name of repoSet) { + results.add(name) + } + } + + return results + } + + /** + * Get all repos visible to the installation. + */ + async getAllRepos () { + const repos = new Set() + const repositories = await this.github.paginate('GET /installation/repositories') + for (const repo of repositories) { + repos.add(repo.name) + } + return repos + } + + /** + * Resolve repos by explicit name or glob pattern. + */ + async resolveByName (names) { + const repos = new Set() + const hasGlobs = names.some(n => n.includes('*') || n.includes('?')) + + if (hasGlobs) { + // Need to fetch all repos and match against globs + const allRepos = await this.github.paginate('GET /installation/repositories') + for (const name of names) { + const glob = new Glob(name) + for (const repo of allRepos) { + if (glob.test(repo.name)) { + repos.add(repo.name) + } + } + } + } else { + // Plain names — add directly + for (const name of names) { + repos.add(name) + } + } + + return repos + } + + /** + * Resolve repos by team membership. + */ + async resolveByTeam (teams) { + const repos = new Set() + const teamPromises = teams.map(teamSlug => { + const options = this.github.rest.teams.listReposInOrg.endpoint.merge({ + org: this.org, + team_slug: teamSlug, + per_page: 100 + }) + return this.github.paginate(options) + }) + + const results = await Promise.all(teamPromises) + for (const teamRepos of results) { + for (const repo of teamRepos) { + repos.add(repo.name) + } + } + + return repos + } + + /** + * Resolve repos by custom property values. + * Each entry in the array is an object { propertyName: propertyValue }. + */ + async resolveByCustomProperties (properties) { + const repos = new Set() + const propPromises = properties.map(async (propertyFilter) => { + const [name] = Object.keys(propertyFilter) + const value = propertyFilter[name] + + const query = `props.${name}:${value}` + const encodedQuery = encodeURIComponent(query) + const options = this.github.request.endpoint( + `/orgs/${this.org}/properties/values?repository_query=${encodedQuery}` + ) + return this.github.paginate(options) + }) + + const results = await Promise.all(propPromises) + for (const propRepos of results) { + for (const repo of propRepos) { + repos.add(repo.repository_name) + } + } + + return repos + } +} + +module.exports = RepoSelector diff --git a/lib/settings.js b/lib/settings.js index bc89e016..42d4c0f5 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -6,6 +6,8 @@ const Glob = require('./glob') const NopCommand = require('./nopcommand') const MergeDeep = require('./mergeDeep') const Archive = require('./plugins/archive') +const AppInstallations = require('./plugins/appInstallations') +const RepoSelector = require('./repoSelector') const DeploymentConfig = require('./deploymentConfig') const env = require('./env') @@ -168,14 +170,19 @@ function filterActionByChangedNames (action, changedNames) { function buildChangeSections (changes, baseConfig, config) { return Object.keys(changes).map(plugin => { + const isAppInstallations = plugin === 'app_installations' const repoSections = [] Object.keys(changes[plugin]).forEach(repo => { const targetMap = new Map() changes[plugin][repo].forEach(action => { - targetsForAction(plugin, repo, action, baseConfig, config).forEach(target => { + const actionTargets = isAppInstallations + ? appInstallationTargets(action) + : targetsForAction(plugin, repo, action, baseConfig, config) + actionTargets.forEach(target => { if (!targetMap.has(target.target)) { targetMap.set(target.target, { target: target.target, + flat: target.flat === true, rows: [] }) } @@ -192,11 +199,17 @@ function buildChangeSections (changes, baseConfig, config) { const changeCount = filteredRepoSections.reduce((count, repoSection) => { return count + repoSection.targets.reduce((targetCount, target) => targetCount + target.rows.length, 0) }, 0) - const targetCount = filteredRepoSections.reduce((count, repoSection) => count + repoSection.targets.length, 0) + // For flat targets (app_installations) each row is a distinct change; for + // regular targets each target counts as one changed setting. + const targetCount = filteredRepoSections.reduce((count, repoSection) => { + return count + repoSection.targets.reduce((tc, target) => tc + (target.flat ? target.rows.length : 1), 0) + }, 0) const repoCount = filteredRepoSections.length + const subjectSingular = isAppInstallations ? 'app' : 'repo' + const subjectPlural = isAppInstallations ? 'apps' : 'repos' const targetSingular = plugin.toLowerCase() === 'rulesets' ? 'policy' : 'setting' const targetPlural = plugin.toLowerCase() === 'rulesets' ? 'policies' : 'settings' - const impactSummary = `${repoCount} ${pluralize(repoCount, 'repo', 'repos')}, ${targetCount} ${pluralize(targetCount, targetSingular, targetPlural)} changed` + const impactSummary = `${repoCount} ${pluralize(repoCount, subjectSingular, subjectPlural)}, ${targetCount} ${pluralize(targetCount, targetSingular, targetPlural)} changed` return { plugin, repoSections: filteredRepoSections, @@ -209,10 +222,40 @@ function buildChangeSections (changes, baseConfig, config) { }).filter(section => section.repoSections.length > 0) } +// app_installations changes are presented with the GitHub App as the subject +// (heading) and a flat list of repositories added/removed (or a toggle to +// "all"). Returns a single flat target whose rows carry a `label` per change. +function appInstallationTargets (action) { + if (!action || typeof action === 'string') { + return [{ target: '', flat: true, rows: action ? [{ change: 'Info', label: action }] : [] }] + } + const toList = value => { + if (value === null || value === undefined) return [] + const arr = Array.isArray(value) ? value : [value] + return arr + .filter(entry => !isDeepEmpty(entry)) + .map(entry => (typeof entry === 'string' ? entry : (getEntryIdentityValue(entry) || JSON.stringify(entry)))) + } + const rows = [] + toList(action.additions).forEach(label => rows.push({ change: 'Added', label })) + toList(action.modifications).forEach(label => rows.push({ change: 'Modified', label })) + toList(action.deletions).forEach(label => rows.push({ change: 'Deleted', label })) + if (rows.length === 0 && action.msg) rows.push({ change: 'Info', label: action.msg }) + return [{ target: '', flat: true, rows }] +} + function renderChangeSections (changeSections) { return changeSections.map(section => { const repoBlocks = section.repoSections.map(repoSection => { const targetBlocks = repoSection.targets.map(target => { + if (target.flat) { + return target.rows.map(row => { + const marker = changeMarker(row.change) + return row.change === 'Info' + ? `- ${marker} ${markdownText(row.label)}` + : `- ${marker} ${markdownInlineCode(row.label)}` + }).join('\n') + } return `- ${markdownInlineCode(target.target)}\n${renderFieldChangeList(target.rows, ' ')}` }) return `**${markdownText(displayRepoName(repoSection.repo))}**\n${targetBlocks.join('\n')}` @@ -223,9 +266,13 @@ function renderChangeSections (changeSections) { } function affectedRepoCount (changeSections) { - return new Set(changeSections.flatMap(section => { - return section.repoSections.map(repoSection => displayRepoName(repoSection.repo)) - })).size + return new Set(changeSections + // app_installations sections are keyed by app subject, not repositories, + // so they must not inflate the "repos affected" count. + .filter(section => section.plugin !== 'app_installations') + .flatMap(section => { + return section.repoSections.map(repoSection => displayRepoName(repoSection.repo)) + })).size } function displayRepoName (repo) { @@ -563,6 +610,10 @@ class Settings { settings.trackChangedReposFromSubOrgConfigs() // settings.repoConfigs = await settings.getRepoConfigs() await settings.updateOrg() + await settings.syncAppInstallations({ + appGithub: context.appGithub, + enterpriseSlug: context.enterpriseSlug + }) await settings.updateAll() await settings.updateChangedRepoConfigs(changedFiles.repos) await settings.handleResults() @@ -649,6 +700,16 @@ class Settings { await settings.loadConfigs() await settings.updateAll() } + + // Sync app installations for affected apps (delta mode) + await settings.syncAppInstallations({ + appGithub: context.appGithub, + enterpriseSlug: context.enterpriseSlug, + changedSubOrgs: subOrgs, + changedRepos: repos, + baseRef + }) + await settings.handleResults() } catch (error) { settings.logError(error.message) @@ -1087,10 +1148,14 @@ class Settings { if (!stats.changes[res.plugin]) { stats.changes[res.plugin] = {} } - if (!stats.changes[res.plugin][res.repo]) { - stats.changes[res.plugin][res.repo] = [] + // Group by the result's subject (defaults to the repo). Plugins that + // act on a non-repo entity — e.g. app_installations, whose subject is + // a GitHub App — group under that subject instead of the (org) repo. + const subject = res.subject || res.repo + if (!stats.changes[res.plugin][subject]) { + stats.changes[res.plugin][subject] = [] } - stats.changes[res.plugin][res.repo].push(res.action) + stats.changes[res.plugin][subject].push(res.action) } } }) @@ -1404,6 +1469,471 @@ class Settings { } } + /** + * Sync app installations as a separate phase. + * In full sync mode, computes desired state for all managed apps across all + * config layers and reconciles against live API state. + * In delta mode, processes only the apps affected by changed config files. + * + * @param {object} [options] + * @param {object} [options.appGithub] - App-authenticated Octokit (for enterprise API) + * @param {string} [options.enterpriseSlug] - Enterprise slug from payload + * @param {Array} [options.appChanges] - Pre-computed per-app changes (delta mode); takes precedence over changedSubOrgs/changedRepos + * @param {Array} [options.changedSubOrgs] - Changed suborg config descriptors ({ repo|name, path }) to diff (delta mode) + * @param {Array} [options.changedRepos] - Changed repo config descriptors ({ owner, repo }) to diff (delta mode) + * @param {string} [options.baseRef] - Base git ref used to load the previous config versions when diffing (delta mode) + */ + async syncAppInstallations (options = {}) { + const { appGithub, enterpriseSlug, appChanges, changedSubOrgs, changedRepos, baseRef } = options + + const appInstallationsConfig = this.config.app_installations + // Check if any layer has app_installations config (org, suborg, or repo) + const hasOrgConfig = appInstallationsConfig && Array.isArray(appInstallationsConfig) && appInstallationsConfig.length > 0 + const hasChangedConfigs = (changedSubOrgs && changedSubOrgs.length > 0) || (changedRepos && changedRepos.length > 0) + const hasPrecomputedChanges = appChanges && appChanges.length > 0 + + // In full-sync mode (no delta inputs) app_installations may be defined only + // at the repo or suborg layer, with nothing at the org level. Detect those + // so the plugin still runs when org settings.yml has no app_installations. + const hasLayeredConfig = !hasChangedConfigs && !hasPrecomputedChanges && this._hasLayeredAppInstallations() + + if (!hasOrgConfig && !hasChangedConfigs && !hasPrecomputedChanges && !hasLayeredConfig) { + this.log.debug('No app_installations config found, skipping') + return + } + + // Check disable_plugins + const stripMap = this.computeStripMap() + if (this.isPluginDisabledAnywhere(stripMap, 'app_installations')) { + this.log.debug("disable_plugins: skipping 'app_installations' plugin") + this.emitDisableSkip('app_installations') + return + } + + if (!enterpriseSlug) { + const msg = 'Cannot sync app installations: enterprise slug not available in context (webhook payload missing enterprise info and no fallback configured).' + this.errors.push({ owner: this.repo.owner, repo: this.repo.repo, msg, plugin: 'app_installations' }) + this.log.error(msg) + if (this.nop) { + this.appendToResults([new NopCommand('app_installations', this.repo, null, msg, 'ERROR')]) + } + return + } + + if (!appGithub) { + const msg = `Cannot sync app installations: enterprise-authenticated client not available for '${enterpriseSlug}'. Ensure safe-settings is installed on the enterprise with 'Enterprise organization installations' permission.` + this.errors.push({ owner: this.repo.owner, repo: this.repo.repo, msg, plugin: 'app_installations' }) + this.log.error(msg) + if (this.nop) this.appendToResults([new NopCommand('app_installations', this.repo, null, msg, 'ERROR')]) + return + } + const additiveSet = this.normalizeAdditivePlugins() + const plugin = new AppInstallations( + this.nop, + this.github, + appGithub, + this.repo, + enterpriseSlug, + this.log, + this.errors + ) + plugin.additive = additiveSet.has('app_installations') + + let results + if (appChanges && appChanges.length > 0) { + // Pre-computed delta mode + results = await plugin.syncDelta(appChanges) + } else if (hasChangedConfigs) { + // Delta mode: build app changes from changed suborg/repo configs + const deltaChanges = await this._buildAppChangesFromDelta(appGithub, enterpriseSlug, changedSubOrgs, changedRepos, baseRef) + if (deltaChanges.length > 0) { + results = await plugin.syncDelta(deltaChanges) + } else { + results = [] + } + } else { + // Full sync mode: compute desired state from all config layers + const desiredState = await this._computeFullAppDesiredState(appInstallationsConfig, appGithub, enterpriseSlug) + results = await plugin.syncFull(desiredState) + } + + if (this.nop && Array.isArray(results)) { + results.forEach(r => { if (r) r.repo = `${this.repo.owner} (org)` }) + } + this.appendToResults(results) + } + + /** + * Detect app_installations defined at the repo or suborg layer (used in + * full-sync mode where org settings.yml may have no app_installations of its + * own but repo/suborg configs still declare apps to manage). + * @private + */ + _hasLayeredAppInstallations () { + const hasInMap = (map) => { + if (!map) return false + for (const cfg of Object.values(map)) { + if (cfg && Array.isArray(cfg.app_installations) && cfg.app_installations.length > 0) return true + } + return false + } + return hasInMap(this.repoConfigs) || hasInMap(this.subOrgConfigs) + } + + /** + * Report a configured `app_installations` app_slug that is not installed on + * the org (typically a typo, or an app that has not been installed yet). + * Surfaced as an ERROR so the PR check run / sync fails visibly instead of + * silently skipping the entry. + * @private + */ + _reportUnknownApp (slug, layer) { + const where = layer ? ` (${layer})` : '' + const msg = `app_installations: app '${slug}'${where} is not installed on org '${this.repo.owner}'. Check the app_slug for typos and ensure the GitHub App is installed. Skipping this app.` + this.log.error(msg) + this.errors.push({ owner: this.repo.owner, repo: this.repo.repo, msg, plugin: 'app_installations' }) + if (this.nop) { + this.appendToResults([new NopCommand('app_installations', this.repo, null, msg, 'ERROR', { name: slug, type: 'app' })]) + } + } + + /** + * Build delta-based app changes from changed suborg/repo config files. + * Loads both current and previous (baseRef) versions of each changed config, + * diffs the app_installations sections, and computes repository_selection + * (repos to add) and repository_unselection (repos to remove) per app. + * @private + */ + async _buildAppChangesFromDelta (appGithub, enterpriseSlug, changedSubOrgs = [], changedRepos = [], baseRef) { + const AppOctokitClient = require('./appOctokitClient') + const repoSelector = new RepoSelector(this.github, this.repo.owner, this.log) + const appChangeMap = new Map() // app_slug → { installation_id, repository_selection, repository_unselection } + + // Get installation map (app_slug → installation_id) + const installationMap = new Map() + if (appGithub && enterpriseSlug) { + try { + const enterpriseClient = new AppOctokitClient({ github: appGithub, enterpriseSlug, log: this.log }) + const orgInstallations = await enterpriseClient.listOrgInstallations(this.repo.owner) + for (const inst of orgInstallations) { + installationMap.set(inst.app_slug, inst.id) + } + } catch (e) { + const msg = `Failed to list org installations for delta: ${e.message}` + this.log.error(msg) + this.errors.push({ owner: this.repo.owner, repo: this.repo.repo, msg, plugin: 'app_installations' }) + if (this.nop) this.appendToResults([new NopCommand('app_installations', this.repo, null, msg, 'ERROR')]) + return [] + } + } + + // Apps configured as "all" at the org level take precedence — they must + // never have repos unselected by suborg/repo deltas, and adding repos is + // redundant since the app already targets all repos. + const orgAllApps = new Set() + const orgAppInstallations = this.config && this.config.app_installations + if (Array.isArray(orgAppInstallations)) { + for (const appConfig of orgAppInstallations) { + // Any org-level app_installations entry always implies 'all'. + if (appConfig && appConfig.app_slug) { + orgAllApps.add(appConfig.app_slug) + } + } + } + + // Helper to ensure an entry exists in the change map + const ensureEntry = (slug) => { + // Org-level "all" apps are fully managed by full sync; deltas must not + // add or remove repos for them (org "all" takes precedence). + if (orgAllApps.has(slug)) return null + if (!appChangeMap.has(slug)) { + const installationId = installationMap.get(slug) + if (!installationId) { + this._reportUnknownApp(slug, 'suborg/repo') + return null + } + appChangeMap.set(slug, { + app_slug: slug, + installation_id: installationId, + repository_selection: new Set(), + repository_unselection: new Set() + }) + } + return appChangeMap.get(slug) + } + + // Helper to resolve repos for a suborg config's targeting criteria + const resolveSuborgRepos = async (config) => { + if (!config) return new Set() + const criteria = {} + if (config.suborgrepos) criteria.names = config.suborgrepos + if (config.suborgteams) criteria.teams = config.suborgteams + if (config.suborgproperties) criteria.custom_properties = config.suborgproperties + try { + return await repoSelector.resolve(criteria) + } catch (e) { + this.log.debug(`Error resolving suborg repos: ${e.message}`) + return new Set() + } + } + + // Process changed suborg configs + for (const suborg of changedSubOrgs) { + // Resolve the CURRENT suborg config. this.subOrgConfigs is unreliable in + // the delta path: getSubOrgConfigs keys it by the suborg file name (with + // extension) and by each targeted repo — never by the bare suborg name + // (suborg.repo) — and syncSelectedRepos filters it down to a single + // suborg's targeted repos. When the lookup misses, load the config + // authoritatively from this.ref (mirroring the previous-version load) so + // apps are not misclassified as removed and emit incorrect unselections. + let currentConfig = this.subOrgConfigs && this.subOrgConfigs[suborg.repo] + if (!currentConfig && suborg.path) { + try { + currentConfig = await this.loadYamlFromRef(suborg.path, this.ref) + } catch (e) { + this.log.debug(`Could not load current suborg config for '${suborg.repo || suborg.name}': ${e.message}`) + } + } + const currentApps = (currentConfig && currentConfig.app_installations) || [] + const currentAppSlugs = new Set(currentApps.map(a => a.app_slug).filter(Boolean)) + + // Load previous version of this suborg config + let previousConfig = null + let previousApps = [] + if (baseRef && suborg.path) { + try { + previousConfig = await this.loadYamlFromRef(suborg.path, baseRef) + previousApps = (previousConfig && previousConfig.app_installations) || [] + } catch (e) { + this.log.debug(`Could not load previous suborg config for '${suborg.repo || suborg.name}': ${e.message}`) + } + } + const previousAppSlugs = new Set(previousApps.map(a => a.app_slug).filter(Boolean)) + + // Resolve repos for current and previous targeting criteria + const currentRepos = await resolveSuborgRepos(currentConfig) + const previousRepos = await resolveSuborgRepos(previousConfig) + + // App newly added to this suborg: select all currently targeted repos + for (const slug of currentAppSlugs) { + if (previousAppSlugs.has(slug)) continue + const entry = ensureEntry(slug) + if (!entry) continue + for (const repo of currentRepos) { + entry.repository_selection.add(repo) + } + } + + // App removed from this suborg: unselect all previously targeted repos + for (const slug of previousAppSlugs) { + if (currentAppSlugs.has(slug)) continue + const entry = ensureEntry(slug) + if (!entry) continue + for (const repo of previousRepos) { + entry.repository_unselection.add(repo) + } + } + + // App present in both: only act on the targeting diff. If the targeting + // is unchanged, skip entirely to avoid redundant churn. + const addedRepos = [...currentRepos].filter(r => !previousRepos.has(r)) + const removedRepos = [...previousRepos].filter(r => !currentRepos.has(r)) + if (addedRepos.length > 0 || removedRepos.length > 0) { + for (const slug of currentAppSlugs) { + if (!previousAppSlugs.has(slug)) continue // handled as "newly added" above + const entry = ensureEntry(slug) + if (!entry) continue + for (const repo of addedRepos) entry.repository_selection.add(repo) + for (const repo of removedRepos) entry.repository_unselection.add(repo) + } + } + } + + // Process changed repo configs + for (const repo of changedRepos) { + const repoFilePath = path.posix.join(CONFIG_PATH, 'repos', `${repo.repo}.yml`) + + // Resolve the CURRENT repo config. During syncSelectedRepos this.repoConfigs + // is loaded one repo at a time and typically retains only the last + // processed repo, so it cannot be relied on for every changed repo. When + // the entry is missing, load it authoritatively from this.ref — otherwise + // other changed repos would look empty and be treated as "app removed", + // emitting incorrect unselections. + let repoConfig = this.repoConfigs && + (this.repoConfigs[`${repo.repo}.yml`] || this.repoConfigs[`${repo.repo}.yaml`]) + if (!repoConfig) { + try { + repoConfig = await this.loadYamlFromRef(repoFilePath, this.ref) + } catch (e) { + this.log.debug(`Could not load current repo config for '${repo.repo}': ${e.message}`) + } + } + const currentApps = (repoConfig && repoConfig.app_installations) || [] + const currentAppSlugs = new Set(currentApps.map(a => a.app_slug).filter(Boolean)) + + // Load previous version of this repo config + let previousApps = [] + if (baseRef) { + try { + const previousData = await this.loadYamlFromRef(repoFilePath, baseRef) + previousApps = (previousData && previousData.app_installations) || [] + } catch (e) { + this.log.debug(`Could not load previous repo config for '${repo.repo}': ${e.message}`) + } + } + const previousAppSlugs = new Set(previousApps.map(a => a.app_slug).filter(Boolean)) + + // App newly added to this repo config: select this repo. If the app was + // already present in the previous version, its selection is unchanged — + // skip to avoid redundant churn. + for (const slug of currentAppSlugs) { + if (previousAppSlugs.has(slug)) continue + const entry = ensureEntry(slug) + if (!entry) continue + entry.repository_selection.add(repo.repo) + } + + // App removed from this repo config: unselect this repo + for (const slug of previousAppSlugs) { + if (currentAppSlugs.has(slug)) continue + const entry = ensureEntry(slug) + if (!entry) continue + entry.repository_unselection.add(repo.repo) + } + } + + // Convert Sets to arrays and remove repos that appear in both selection and unselection + // (selection wins — if a repo is being added by one config and removed by another, keep it) + const results = [] + for (const change of appChangeMap.values()) { + for (const repo of change.repository_selection) { + change.repository_unselection.delete(repo) + } + results.push({ + ...change, + repository_selection: [...change.repository_selection], + repository_unselection: [...change.repository_unselection] + }) + } + return results + } + + /** + * Compute the full desired state for all managed apps by merging + * org + suborg + repo level app_installations configs. + * Used only in full sync mode (cron/manual). + * @private + */ + async _computeFullAppDesiredState (orgAppInstallations, appGithub, enterpriseSlug) { + const AppOctokitClient = require('./appOctokitClient') + const desiredState = {} + const repoSelector = new RepoSelector(this.github, this.repo.owner, this.log) + // Org-level app_installations may be absent entirely (apps declared only at + // the repo/suborg layer); normalise so the org loop below is safe. + if (!Array.isArray(orgAppInstallations)) orgAppInstallations = [] + + // Get all org installations to map app_slug → installation_id + let orgInstallations = [] + if (appGithub && enterpriseSlug) { + const enterpriseClient = new AppOctokitClient({ github: appGithub, enterpriseSlug, log: this.log }) + try { + orgInstallations = await enterpriseClient.listOrgInstallations(this.repo.owner) + } catch (e) { + const msg = `Failed to list org installations: ${e.message}` + this.log.error(msg) + this.errors.push({ owner: this.repo.owner, repo: this.repo.repo, msg, plugin: 'app_installations' }) + if (this.nop) this.appendToResults([new NopCommand('app_installations', this.repo, null, msg, 'ERROR')]) + return desiredState + } + } + + const installationMap = new Map() + const selectionMap = new Map() + for (const inst of orgInstallations) { + installationMap.set(inst.app_slug, inst.id) + selectionMap.set(inst.app_slug, inst.repository_selection) + } + + // Process org-level config. An org-level app_installations entry always + // implies access to ALL repos in the org (there is no per-repo selection + // at this layer). + for (const appConfig of orgAppInstallations) { + const slug = appConfig.app_slug + if (!slug) continue + + const installationId = installationMap.get(slug) + if (!installationId) { + this._reportUnknownApp(slug, 'org settings.yml') + continue + } + + desiredState[slug] = { installation_id: installationId, repos: 'all' } + } + + // Overlay suborg-level configs + if (this.subOrgConfigs) { + for (const [pattern, subOrgConfig] of Object.entries(this.subOrgConfigs)) { + if (!subOrgConfig || !subOrgConfig.app_installations) continue + + // Resolve repos for this suborg + const criteria = {} + if (subOrgConfig.suborgrepos) criteria.names = subOrgConfig.suborgrepos + if (subOrgConfig.suborgteams) criteria.teams = subOrgConfig.suborgteams + if (subOrgConfig.suborgproperties) criteria.custom_properties = subOrgConfig.suborgproperties + + let suborgRepos = new Set() + try { + suborgRepos = await repoSelector.resolve(criteria) + } catch (e) { + this.log.debug(`Error resolving suborg repos for pattern '${pattern}': ${e.message}`) + } + + for (const appConfig of subOrgConfig.app_installations) { + const slug = appConfig.app_slug + if (!slug) continue + if (!desiredState[slug]) { + const installationId = installationMap.get(slug) + if (!installationId) { this._reportUnknownApp(slug, 'suborg'); continue } + desiredState[slug] = { installation_id: installationId, repos: new Set() } + } + // Org "all" takes precedence — don't add specific repos + if (desiredState[slug].repos === 'all') continue + for (const repo of suborgRepos) { + desiredState[slug].repos.add(repo) + } + } + } + } + + // Overlay repo-level configs + if (this.repoConfigs) { + for (const [repoFileName, repoConfig] of Object.entries(this.repoConfigs)) { + if (!repoConfig || !repoConfig.app_installations) continue + const repoName = repoFileName.replace(/\.ya?ml$/, '') + + for (const appConfig of repoConfig.app_installations) { + const slug = appConfig.app_slug + if (!slug) continue + if (!desiredState[slug]) { + const installationId = installationMap.get(slug) + if (!installationId) { this._reportUnknownApp(slug, 'repo'); continue } + desiredState[slug] = { installation_id: installationId, repos: new Set() } + } + if (desiredState[slug].repos === 'all') continue + desiredState[slug].repos.add(repoName) + } + } + } + + // Attach each app's current (live) repository_selection so the plugin can + // decide whether to toggle 'all' ↔ 'selected' or add/remove individually. + for (const [slug, entry] of Object.entries(desiredState)) { + entry.current_selection = selectionMap.get(slug) + } + + return desiredState + } + async updateRepos (repo) { this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs() // Snapshot the set of suborg `source` paths that match this repo *before* @@ -1793,6 +2323,11 @@ class Settings { if (section !== 'repositories' && section !== 'repository') { // Ignore any config that is not a plugin if (section in Settings.PLUGINS) { + // app_installations is not a per-repo Diffable plugin; it operates at + // the org level on app installations and is reconciled separately by + // syncAppInstallations(). Skip it here so the per-repo pipeline does + // not try to call the (non-existent) sync() on it. + if (section === 'app_installations') continue this.log.debug(`Found section ${section} in the config. Creating plugin...`) const Plugin = Settings.PLUGINS[section] // Include sectionName as 3rd element so callers can thread the @@ -2335,7 +2870,8 @@ Settings.ADDITIVE_PLUGINS = new Set([ 'custom_properties', 'variables', 'rulesets', - 'custom_repository_roles' + 'custom_repository_roles', + 'app_installations' ]) Settings.PLUGINS = { @@ -2351,7 +2887,8 @@ Settings.PLUGINS = { environments: require('./plugins/environments'), custom_properties: require('./plugins/custom_properties.js'), custom_repository_roles: require('./plugins/custom_repository_roles'), - variables: require('./plugins/variables') + variables: require('./plugins/variables'), + app_installations: require('./plugins/appInstallations') } module.exports = Settings diff --git a/schema/settings.json b/schema/settings.json index 57565fd8..02c8b36f 100644 --- a/schema/settings.json +++ b/schema/settings.json @@ -234,8 +234,25 @@ } } }, + "app_installations": { + "description": "Manage which repositories a GitHub App installation can access. The target is a GitHub App installation rather than a repository. Repo selection follows the config hierarchy: org-level settings.yml selects all repos in the org; suborgs/*.yml selects repos by the suborg's targeting criteria; repos/*.yml adds the specific repo. Requires safe-settings to be installed on the enterprise with 'Enterprise organization installations' permission.", + "type": "array", + "items": { + "type": "object", + "required": [ + "app_slug" + ], + "additionalProperties": false, + "properties": { + "app_slug": { + "type": "string", + "description": "The slug of the GitHub App installation to manage." + } + } + } + }, "additive_plugins": { - "description": "List of Diffable plugins to run in additive mode. In additive mode the plugin will only add and update entries; it will never call remove(), so items that exist on GitHub but are absent from the YAML are preserved. Only Diffable-extending plugins are supported (labels, collaborators, teams, milestones, autolinks, environments, custom_properties, variables, rulesets, custom_repository_roles). Declare only in settings.yml (org level) to keep behavior consistent across all repos.", + "description": "List of plugins to run in additive mode. In additive mode the plugin will only add and update entries; it will never call remove(), so items that exist on GitHub but are absent from the YAML are preserved. Supported plugins: labels, collaborators, teams, milestones, autolinks, environments, custom_properties, variables, rulesets, custom_repository_roles, app_installations. Declare only in settings.yml (org level) to keep behavior consistent across all repos.", "type": "array", "items": { "type": "string", @@ -249,7 +266,8 @@ "custom_properties", "variables", "rulesets", - "custom_repository_roles" + "custom_repository_roles", + "app_installations" ] } }, @@ -274,7 +292,8 @@ "custom_properties", "custom_repository_roles", "variables", - "archive" + "archive", + "app_installations" ] }, { @@ -300,7 +319,8 @@ "custom_properties", "custom_repository_roles", "variables", - "archive" + "archive", + "app_installations" ] }, "target": { diff --git a/smoke-test.js b/smoke-test.js index 40b705dc..2b4d9dba 100644 --- a/smoke-test.js +++ b/smoke-test.js @@ -84,6 +84,24 @@ const WEBHOOK_SETTLE_MS = 15000 // Fine-grained PAT for drift tests (must appear as a human, not Bot) const GH_TOKEN = process.env.GH_TOKEN || '' +// ─── App installation plugin config (Phase 17) ─────────────────────────────── +// Enterprise slug — required for the app_installations plugin, which manages +// GitHub App installation repository access via the Enterprise organization +// installations API. Read from GH_ENTERPRISE (same var safe-settings uses). +const GH_ENTERPRISE = process.env.GH_ENTERPRISE || '' +// One or more enterprise-level GitHub App slugs to exercise the plugin. +// ONE app is sufficient for full lifecycle coverage; providing a second app +// (comma-separated) additionally verifies that changes to one app's +// installation do not affect another's. Pre-create these as enterprise GitHub +// Apps and install them on the org (creation requires a human). +// SMOKE_APP_SLUGS=app-one,app-two (or singular SMOKE_APP_SLUG=app-one) +const APP_SLUGS = (process.env.SMOKE_APP_SLUGS || process.env.SMOKE_APP_SLUG || '') + .split(',').map(s => s.trim()).filter(Boolean) +// Dedicated repos created/managed by the app_installations phase. +const APP_TEST_REPOS = ['smoke-app-repo-1', 'smoke-app-repo-2', 'smoke-app-repo-3'] +// Enterprise org installations API version (matches lib/appOctokitClient.js). +const APPS_API_VERSION = '2026-03-10' + // Interactive mode: pause after each phase for manual validation // Accepts --interactive flag or bare positional "interactive" word. const INTERACTIVE = process.argv.includes('--interactive') || process.argv.slice(2).includes('interactive') @@ -127,6 +145,12 @@ class InteractiveExit extends Error { // ─── Octokit client (initialized in main) ──────────────────────────────────── let octokit = null +// Enterprise-installation-authenticated client (Phase 17). Null when the app +// is not installed on the enterprise or GH_ENTERPRISE is unset. +let entOctokit = null +// Snapshot of each managed app's installation state, captured at the start of +// Phase 17 and restored during teardown so shared apps are left untouched. +const appInstallSnapshot = {} // ─── Helpers ───────────────────────────────────────────────────────────────── @@ -399,6 +423,66 @@ async function waitForCheckRun (owner, repo, sha, { timeout = MAX_POLL_MS } = {} }, { timeout, desc: 'check run to complete' }) } +// Ensure a repository exists in the org (create it directly if missing). +async function ensureRepo (name) { + try { + await octokit.rest.repos.get({ owner: ORG, repo: name }) + return + } catch { /* needs creation */ } + try { + await octokit.rest.repos.createInOrg({ org: ORG, name, private: true, auto_init: true }) + await poll(async () => { + try { await octokit.rest.repos.get({ owner: ORG, repo: name }); return true } catch { return null } + }, { desc: `repo ${name} to be created`, timeout: 30000 }) + } catch (e) { log(` Could not create repo ${name}: ${e.message}`) } +} + +// ─── Enterprise organization installations API helpers (Phase 17) ──────────── + +async function listOrgAppInstallations () { + const options = entOctokit.request.endpoint.merge( + 'GET /enterprises/{enterprise}/apps/organizations/{org}/installations', + { enterprise: GH_ENTERPRISE, org: ORG, headers: { 'X-GitHub-Api-Version': APPS_API_VERSION } } + ) + return entOctokit.paginate(options) +} + +async function getAppInstallation (appSlug) { + const installations = await listOrgAppInstallations() + return installations.find(i => i.app_slug === appSlug) || null +} + +async function listInstallationRepoNames (installationId) { + const options = entOctokit.request.endpoint.merge( + 'GET /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories', + { enterprise: GH_ENTERPRISE, org: ORG, installation_id: installationId, headers: { 'X-GitHub-Api-Version': APPS_API_VERSION } } + ) + const repos = await entOctokit.paginate(options) + return repos.map(r => r.name) +} + +async function setInstallationSelection (installationId, selection, repositories) { + const params = { + enterprise: GH_ENTERPRISE, + org: ORG, + installation_id: installationId, + repository_selection: selection, + headers: { 'X-GitHub-Api-Version': APPS_API_VERSION } + } + if (selection === 'selected') params.repositories = repositories || [] + await entOctokit.request('PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories', params) +} + +async function addInstallationRepos (installationId, repositoryNames) { + await entOctokit.request('PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories/add', { + enterprise: GH_ENTERPRISE, + org: ORG, + installation_id: installationId, + repositories: repositoryNames, + headers: { 'X-GitHub-Api-Version': APPS_API_VERSION } + }) +} + // ─── Safe-settings process management ──────────────────────────────────────── let ssProcess = null @@ -2366,6 +2450,24 @@ async function teardown () { try { await octokit.rest.repos.update({ owner: ORG, repo: 'demo-repo-service1', archived: false }) } catch { /* ok */ } for (const repo of TEST_REPOS) { await deleteRepo(ORG, repo) } + // Restore app installations to their original state so shared enterprise + // apps are left untouched by the smoke test. + if (entOctokit && Object.keys(appInstallSnapshot).length > 0) { + log('Restoring app installation repository selections...') + for (const [slug, snap] of Object.entries(appInstallSnapshot)) { + try { + if (snap.selection === 'all') { + await setInstallationSelection(snap.installationId, 'all') + } else if (snap.selection === 'selected' && snap.repos.length > 0) { + await setInstallationSelection(snap.installationId, 'selected', snap.repos) + } + } catch (e) { log(` Could not restore app '${slug}' installation: ${e.message}`) } + } + } + + log('Deleting app-install smoke repos...') + for (const repo of APP_TEST_REPOS) { await deleteRepo(ORG, repo) } + log('Deleting test teams...') for (const team of TEST_TEAMS) { await deleteTeam(ORG, team.toLowerCase()) } try { await deleteTeam(ORG, SMOKE_NR_TEAM) } catch { /* ok */ } @@ -2701,6 +2803,296 @@ async function phase16RulesetNameResolution () { } } +// ─── App installation config builders (Phase 17) ───────────────────────────── + +// Org-level settings.yml giving an app access to ALL repos. +const settingsAppInstallAll = (slug) => `# App installations: org-level (implies all repos) +app_installations: + - app_slug: ${slug} +` + +// Empty org settings (no app_installations) with an optional comment bump to +// force a full sync while app selection is driven entirely by repo configs. +const settingsAppInstallEmpty = (bump = '') => `# App installations: no org-level app config${bump ? ` (${bump})` : ''} +` + +// Repo-level config that force-creates the repo and adds it to the app. +const repoAppInstallConfig = (name, slug) => `repository: + name: ${name} +app_installations: + - app_slug: ${slug} +` + +// Suborg-level config that targets an explicit list of repos (suborgrepos) and +// adds the app for those repos. Suborg config changes drive the delta sync. +const suborgAppInstallConfig = (repos, slug) => `suborgrepos: +${repos.map(r => ` - ${r}`).join('\n')} +app_installations: + - app_slug: ${slug} +` + +async function phase17AppInstallations () { + logPhase('Phase 17: App installation management (app_installations plugin)') + + if (!GH_ENTERPRISE) { + log('17: GH_ENTERPRISE not set — skipping app_installations tests') + return + } + if (APP_SLUGS.length === 0) { + log('17: SMOKE_APP_SLUGS not set — skipping app_installations tests') + return + } + if (!entOctokit) { + logFail('17: enterprise installation not available (app not installed on enterprise or GH_ENTERPRISE mismatch) — cannot run app_installations tests') + return + } + + const defaultBranch = await getDefaultBranch() + const app = APP_SLUGS[0] + + // Snapshot every managed app's live installation state for restore in teardown. + for (const slug of APP_SLUGS) { + const inst = await getAppInstallation(slug) + if (!inst) { + logFail(`17: app '${slug}' is not installed on org ${ORG} via enterprise '${GH_ENTERPRISE}' — pre-create and install it`) + continue + } + appInstallSnapshot[slug] = { + installationId: inst.id, + selection: inst.repository_selection, + repos: inst.repository_selection === 'selected' ? await listInstallationRepoNames(inst.id) : [] + } + } + + const primary = appInstallSnapshot[app] + if (!primary) { + logFail(`17: primary app '${app}' installation not found — aborting app_installations tests`) + return + } + const installationId = primary.installationId + log(`17: primary app '${app}' installation id=${installationId}, initial selection='${primary.selection}'`) + + log('17: Ensuring dedicated app-install smoke repos exist...') + await ensureRepo(APP_TEST_REPOS[0]) + await ensureRepo(APP_TEST_REPOS[1]) + + // ── 17a: Org-level repository_selection: all ─────────────────────────────── + { + log('17a: Setting app to repository_selection: all via org settings.yml') + const branch = 'smoke-test-phase17a' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, settingsAppInstallAll(app), branch, '17a: app_installations repository_selection all') + const pr = await createPR(ORG, ADMIN_REPO, '17a: app_installations org-level all', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '17a: NOP check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `17a: NOP check run is success (got: ${checkRun.conclusion})`) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS + 15000) + + const selAll = await poll(async () => { + const i = await getAppInstallation(app) + return (i && i.repository_selection === 'all') ? i : null + }, { desc: "app installation to be set to 'all'", timeout: 90000 }) + assert(selAll !== null, "17a: app repository_selection set to 'all' by org-level config") + + // Isolation: any other configured app must be unchanged by app[0]'s config. + for (const slug of APP_SLUGS.slice(1)) { + const snap = appInstallSnapshot[slug] + if (!snap) continue + const other = await getAppInstallation(slug) + assert(other !== null && other.repository_selection === snap.selection, + `17a: other app '${slug}' repository_selection unchanged (isolation)`) + } + + await deleteBranch(ORG, ADMIN_REPO, branch) + } + + // ── 17b: Repo-level selection for two repos (full sync via settings.yml) ──── + // Remove the org-level 'all' and drive selection entirely from repo configs. + // A settings.yml change triggers a full sync, which recomputes desired state + // from all layers and narrows the installation from 'all' → 'selected'. + { + log('17b: Narrowing app to two specific repos via repo-level configs') + const branch = 'smoke-test-phase17b' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, settingsAppInstallEmpty(), branch, '17b: clear org app_installations') + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/${APP_TEST_REPOS[0]}.yml`, repoAppInstallConfig(APP_TEST_REPOS[0], app), branch, `17b: add ${APP_TEST_REPOS[0]} to app`) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/${APP_TEST_REPOS[1]}.yml`, repoAppInstallConfig(APP_TEST_REPOS[1], app), branch, `17b: add ${APP_TEST_REPOS[1]} to app`) + const pr = await createPR(ORG, ADMIN_REPO, '17b: app_installations repo-level selection', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '17b: NOP check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `17b: NOP check run is success (got: ${checkRun.conclusion})`) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS + 15000) + + const selected = await poll(async () => { + const i = await getAppInstallation(app) + if (!i || i.repository_selection !== 'selected') return null + const repos = await listInstallationRepoNames(i.id) + return (repos.includes(APP_TEST_REPOS[0]) && repos.includes(APP_TEST_REPOS[1])) ? repos : null + }, { desc: "app installation to be 'selected' with the two repos", timeout: 90000 }) + assert(selected !== null, `17b: app narrowed to 'selected' with ${APP_TEST_REPOS[0]} and ${APP_TEST_REPOS[1]}`) + } + + // ── 17c: Add a third repo via a new repo config ──────────────────────────── + { + log('17c: Adding a third repo to the app via a new repo config') + await ensureRepo(APP_TEST_REPOS[2]) + const branch = 'smoke-test-phase17c' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + // Bump settings.yml to force a full sync that includes the new repo config. + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, settingsAppInstallEmpty('bump 17c'), branch, '17c: bump settings to trigger full sync') + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/${APP_TEST_REPOS[2]}.yml`, repoAppInstallConfig(APP_TEST_REPOS[2], app), branch, `17c: add ${APP_TEST_REPOS[2]} to app`) + const pr = await createPR(ORG, ADMIN_REPO, '17c: app_installations add repo', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '17c: NOP check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `17c: NOP check run is success (got: ${checkRun.conclusion})`) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS + 15000) + + const added = await poll(async () => { + const i = await getAppInstallation(app) + if (!i || i.repository_selection !== 'selected') return null + const repos = await listInstallationRepoNames(i.id) + return repos.includes(APP_TEST_REPOS[2]) ? repos : null + }, { desc: `${APP_TEST_REPOS[2]} to be added to the app installation`, timeout: 90000 }) + assert(added !== null, `17c: ${APP_TEST_REPOS[2]} added to app installation`) + } + + // ── 17d: Remove the third repo by deleting its config ────────────────────── + { + log('17d: Removing the third repo from the app by deleting its repo config') + const branch = 'smoke-test-phase17d' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await deleteFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/${APP_TEST_REPOS[2]}.yml`, branch, `17d: remove ${APP_TEST_REPOS[2]} config`) + // Bump settings.yml to force a full sync (removals are reconciled by full sync). + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, settingsAppInstallEmpty('bump 17d'), branch, '17d: bump settings to trigger full sync') + const pr = await createPR(ORG, ADMIN_REPO, '17d: app_installations remove repo', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '17d: NOP check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `17d: NOP check run is success (got: ${checkRun.conclusion})`) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS + 15000) + + const removed = await poll(async () => { + const i = await getAppInstallation(app) + if (!i || i.repository_selection !== 'selected') return null + const repos = await listInstallationRepoNames(i.id) + return !repos.includes(APP_TEST_REPOS[2]) ? repos : null + }, { desc: `${APP_TEST_REPOS[2]} to be removed from the app installation`, timeout: 90000 }) + assert(removed !== null, `17d: ${APP_TEST_REPOS[2]} removed from app installation`) + } + + // ── 17e: Drift remediation via full sync ─────────────────────────────────── + // Manually add an unmanaged repo to the installation, then trigger a full + // sync (settings.yml bump). Full sync must remove the drifted repo since it + // is not in any config layer's desired state. + { + log(`17e: Injecting drift — adding unmanaged ${APP_TEST_REPOS[2]} to the installation directly`) + try { + await addInstallationRepos(installationId, [APP_TEST_REPOS[2]]) + } catch (e) { logFail(`17e: could not inject drift: ${e.message}`) } + + const driftPresent = await poll(async () => { + const repos = await listInstallationRepoNames(installationId) + return repos.includes(APP_TEST_REPOS[2]) ? repos : null + }, { desc: `drifted ${APP_TEST_REPOS[2]} to be present before remediation`, timeout: 30000 }) + assert(driftPresent !== null, `17e: drift injected (${APP_TEST_REPOS[2]} present on installation)`) + + const branch = 'smoke-test-phase17e' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, settingsAppInstallEmpty('bump 17e'), branch, '17e: bump settings to trigger full sync drift remediation') + const pr = await createPR(ORG, ADMIN_REPO, '17e: app_installations drift remediation', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '17e: NOP check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `17e: NOP check run is success (got: ${checkRun.conclusion})`) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS + 15000) + + const driftRemoved = await poll(async () => { + const repos = await listInstallationRepoNames(installationId) + return !repos.includes(APP_TEST_REPOS[2]) ? repos : null + }, { desc: `drifted ${APP_TEST_REPOS[2]} to be removed by full sync`, timeout: 90000 }) + assert(driftRemoved !== null, `17e: drift remediated — ${APP_TEST_REPOS[2]} removed by full sync`) + + await deleteBranch(ORG, ADMIN_REPO, branch) + } + + // ── 17f: Sub-org targeting (delta sync via suborgs/*.yml) ────────────────── + // A suborg config change triggers the delta sync path (not full sync). The + // suborg's targeting (here suborgrepos) resolves the repos to add/remove for + // the app. Entering this block the installation is 'selected' with + // APP_TEST_REPOS[0] and [1]; [2] is not selected. + { + // Step A: add a suborg that targets repo-3 and adds the app → repo-3 added. + log('17f: Adding a repo to the app via suborg targeting (suborgrepos, delta sync)') + const branchA = 'smoke-test-phase17f-add' + await deleteBranch(ORG, ADMIN_REPO, branchA) + await createBranch(ORG, ADMIN_REPO, branchA) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/suborgs/smoke-app-suborg.yml`, suborgAppInstallConfig([APP_TEST_REPOS[2]], app), branchA, '17f: suborg targeting adds smoke-app-repo-3 to app') + const prA = await createPR(ORG, ADMIN_REPO, '17f: app_installations suborg add', branchA, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRunA = await waitForCheckRun(ORG, ADMIN_REPO, prA.head.sha) + assert(checkRunA !== null, '17f: NOP check run completed (suborg add)') + if (checkRunA) assert(checkRunA.conclusion === 'success', `17f: NOP check run is success (got: ${checkRunA.conclusion})`) + if (!await safeMerge(ORG, ADMIN_REPO, prA.number)) return + await sleep(WEBHOOK_SETTLE_MS + 15000) + + const suborgAdded = await poll(async () => { + const i = await getAppInstallation(app) + if (!i || i.repository_selection !== 'selected') return null + const repos = await listInstallationRepoNames(i.id) + return repos.includes(APP_TEST_REPOS[2]) ? repos : null + }, { desc: `${APP_TEST_REPOS[2]} to be added via suborg targeting`, timeout: 90000 }) + assert(suborgAdded !== null, `17f: ${APP_TEST_REPOS[2]} added to app via suborg targeting (delta sync)`) + + await deleteBranch(ORG, ADMIN_REPO, branchA) + + // Step B: narrow the suborg targeting so repo-3 drops out → repo-3 removed. + // Retarget to repo-1 (already selected by its repo config, so a no-op add); + // the delta unselection must remove repo-3. + log('17f: Narrowing suborg targeting so smoke-app-repo-3 drops out (delta unselection)') + const branchB = 'smoke-test-phase17f-narrow' + await deleteBranch(ORG, ADMIN_REPO, branchB) + await createBranch(ORG, ADMIN_REPO, branchB) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/suborgs/smoke-app-suborg.yml`, suborgAppInstallConfig([APP_TEST_REPOS[0]], app), branchB, '17f: narrow suborg targeting to smoke-app-repo-1') + const prB = await createPR(ORG, ADMIN_REPO, '17f: app_installations suborg narrow', branchB, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRunB = await waitForCheckRun(ORG, ADMIN_REPO, prB.head.sha) + assert(checkRunB !== null, '17f: NOP check run completed (suborg narrow)') + if (checkRunB) assert(checkRunB.conclusion === 'success', `17f: NOP check run is success (got: ${checkRunB.conclusion})`) + if (!await safeMerge(ORG, ADMIN_REPO, prB.number)) return + await sleep(WEBHOOK_SETTLE_MS + 15000) + + const suborgRemoved = await poll(async () => { + const i = await getAppInstallation(app) + if (!i || i.repository_selection !== 'selected') return null + const repos = await listInstallationRepoNames(i.id) + return !repos.includes(APP_TEST_REPOS[2]) ? repos : null + }, { desc: `${APP_TEST_REPOS[2]} to be removed when dropped from suborg targeting`, timeout: 90000 }) + assert(suborgRemoved !== null, `17f: ${APP_TEST_REPOS[2]} removed when dropped from suborg targeting (delta unselection)`) + + await deleteBranch(ORG, ADMIN_REPO, branchB) + } +} + // ─── Main ──────────────────────────────────────────────────────────────────── async function main () { @@ -2710,7 +3102,8 @@ async function main () { // Find installation for our org let installationId for await (const { installation } of app.eachInstallation.iterator()) { - if (installation.account && installation.account.login.toLowerCase() === ORG.toLowerCase()) { + // Org/user installations key off account.login (only enterprise accounts have a slug). + if (installation.account && installation.account.login && installation.account.login.toLowerCase() === ORG.toLowerCase()) { installationId = installation.id break } @@ -2720,6 +3113,22 @@ async function main () { octokit = await app.getInstallationOctokit(installationId) log('Authenticated as GitHub App installation') + // Optionally resolve the enterprise installation for the app_installations + // plugin phase. The same App must be installed on the enterprise with the + // "Enterprise organization installations" permission. + if (GH_ENTERPRISE && APP_SLUGS.length > 0) { + for await (const { installation } of app.eachInstallation.iterator()) { + if (installation.target_type === 'Enterprise' && + installation.account && installation.account.slug && + installation.account.slug.toLowerCase() === GH_ENTERPRISE.toLowerCase()) { + entOctokit = await app.getInstallationOctokit(installation.id) + log(`Authenticated as enterprise installation for '${GH_ENTERPRISE}' (app_installations phase enabled)`) + break + } + } + if (!entOctokit) log(`\x1b[33m⚠ No enterprise installation found for '${GH_ENTERPRISE}' — Phase 17 will report a failure.\x1b[0m`) + } + console.log(` \x1b[36m╔══════════════════════════════════════╗ ║ Safe-Settings Smoke Test ║ @@ -2752,7 +3161,8 @@ async function main () { ['Phase 13: variables', phase13Variables], ['Phase 14: regressions', phase14RegressionCoverage], ['Phase 15: Ruleset array drift', phase15RulesetArrayDrift], - ['Phase 16: Ruleset name/slug resolution', phase16RulesetNameResolution] + ['Phase 16: Ruleset name/slug resolution', phase16RulesetNameResolution], + ['Phase 17: App installation management', phase17AppInstallations] ] // When --phase is given, only run setup (phase 0) + the requested phase(s). diff --git a/test/unit/lib/appOctokitClient.test.js b/test/unit/lib/appOctokitClient.test.js new file mode 100644 index 00000000..dd7206f6 --- /dev/null +++ b/test/unit/lib/appOctokitClient.test.js @@ -0,0 +1,148 @@ +const AppOctokitClient = require('../../../lib/appOctokitClient') + +describe('AppOctokitClient', () => { + let github + let log + let client + + beforeEach(() => { + log = { + debug: jest.fn(), + error: jest.fn() + } + + github = { + paginate: jest.fn(), + request: jest.fn().mockResolvedValue({ data: {} }) + } + github.request.endpoint = { + merge: jest.fn().mockReturnValue({}) + } + + client = new AppOctokitClient({ + github, + enterpriseSlug: 'my-enterprise', + log + }) + }) + + describe('listOrgInstallations', () => { + it('returns org installations', async () => { + github.paginate.mockResolvedValue([ + { id: 1, app_slug: 'app-a', repository_selection: 'all' }, + { id: 3, app_slug: 'app-c', repository_selection: 'selected' } + ]) + + const result = await client.listOrgInstallations('my-org') + expect(result).toHaveLength(2) + expect(result[0].app_slug).toBe('app-a') + expect(github.request.endpoint.merge).toHaveBeenCalledWith( + 'GET /enterprises/{enterprise}/apps/organizations/{org}/installations', + expect.objectContaining({ enterprise: 'my-enterprise', org: 'my-org' }) + ) + }) + + it('throws descriptive error on 403', async () => { + github.paginate.mockRejectedValue({ status: 403, message: 'Forbidden' }) + + await expect(client.listOrgInstallations('my-org')) + .rejects.toThrow(/enterprise/) + }) + + it('throws descriptive error on 404', async () => { + github.paginate.mockRejectedValue({ status: 404, message: 'Not Found' }) + + await expect(client.listOrgInstallations('my-org')) + .rejects.toThrow(/enterprise/) + }) + }) + + describe('setRepositorySelection', () => { + it("toggles to 'all' without repositories", async () => { + await client.setRepositorySelection('my-org', 123, 'all') + expect(github.request).toHaveBeenCalledWith( + 'PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories', + expect.objectContaining({ + org: 'my-org', + installation_id: 123, + repository_selection: 'all' + }) + ) + const callArgs = github.request.mock.calls[0][1] + expect(callArgs.repositories).toBeUndefined() + }) + + it("toggles to 'selected' with repository names", async () => { + await client.setRepositorySelection('my-org', 123, 'selected', ['repo-a', 'repo-b']) + expect(github.request).toHaveBeenCalledWith( + 'PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories', + expect.objectContaining({ + repository_selection: 'selected', + repositories: ['repo-a', 'repo-b'] + }) + ) + }) + }) + + describe('addReposToInstallation', () => { + it('does nothing for empty array', async () => { + await client.addReposToInstallation('my-org', 123, []) + expect(github.request).not.toHaveBeenCalled() + }) + + it('sends single batch for <= 50 repos using names', async () => { + const names = Array.from({ length: 10 }, (_, i) => `repo-${i}`) + await client.addReposToInstallation('my-org', 123, names) + expect(github.request).toHaveBeenCalledTimes(1) + expect(github.request).toHaveBeenCalledWith( + 'PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories/add', + expect.objectContaining({ + repositories: names, + installation_id: 123, + org: 'my-org' + }) + ) + }) + + it('batches into chunks of 50', async () => { + const names = Array.from({ length: 120 }, (_, i) => `repo-${i}`) + await client.addReposToInstallation('my-org', 123, names) + expect(github.request).toHaveBeenCalledTimes(3) // 50 + 50 + 20 + }) + }) + + describe('removeReposFromInstallation', () => { + it('does nothing for empty array', async () => { + await client.removeReposFromInstallation('my-org', 123, []) + expect(github.request).not.toHaveBeenCalled() + }) + + it('uses the remove endpoint with names', async () => { + await client.removeReposFromInstallation('my-org', 123, ['repo-a']) + expect(github.request).toHaveBeenCalledWith( + 'PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories/remove', + expect.objectContaining({ repositories: ['repo-a'] }) + ) + }) + + it('batches into chunks of 50', async () => { + const names = Array.from({ length: 75 }, (_, i) => `repo-${i}`) + await client.removeReposFromInstallation('my-org', 123, names) + expect(github.request).toHaveBeenCalledTimes(2) // 50 + 25 + }) + }) + + describe('_chunk', () => { + it('splits array into correct chunks', () => { + expect(client._chunk([1, 2, 3, 4, 5], 2)).toEqual([[1, 2], [3, 4], [5]]) + }) + + it('returns single chunk for small array', () => { + expect(client._chunk([1, 2], 50)).toEqual([[1, 2]]) + }) + + it('returns empty array for empty input', () => { + expect(client._chunk([], 50)).toEqual([]) + }) + }) +}) diff --git a/test/unit/lib/plugins/appInstallations.test.js b/test/unit/lib/plugins/appInstallations.test.js new file mode 100644 index 00000000..26332ae2 --- /dev/null +++ b/test/unit/lib/plugins/appInstallations.test.js @@ -0,0 +1,393 @@ +const AppInstallations = require('../../../../lib/plugins/appInstallations') + +describe('AppInstallations', () => { + let github + let appGithub + let log + let errors + + beforeEach(() => { + log = { + debug: jest.fn(), + error: jest.fn() + } + errors = [] + + github = { + paginate: jest.fn(), + repos: { + get: jest.fn() + }, + request: jest.fn().mockResolvedValue({ data: {} }) + } + github.request.endpoint = { + merge: jest.fn().mockReturnValue({}) + } + + appGithub = { + paginate: jest.fn(), + request: jest.fn().mockResolvedValue({ data: {} }) + } + appGithub.request.endpoint = { + merge: jest.fn().mockReturnValue({}) + } + }) + + describe('syncDelta', () => { + it('returns empty array for no changes', async () => { + const plugin = new AppInstallations(false, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + const result = await plugin.syncDelta([]) + expect(result).toEqual([]) + }) + + it('returns empty array for null changes', async () => { + const plugin = new AppInstallations(false, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + const result = await plugin.syncDelta(null) + expect(result).toEqual([]) + }) + + it('reports error when enterprise client is not configured', async () => { + const plugin = new AppInstallations(true, github, null, { owner: 'org', repo: 'admin' }, null, log, errors) + const result = await plugin.syncDelta([{ + app_slug: 'test-app', + installation_id: 1, + repository_selection: new Set(['repo-a']), + repository_unselection: new Set() + }]) + + expect(result).toHaveLength(1) + expect(result[0].type).toBe('ERROR') + }) + + it('generates NopCommand in nop mode for specific repos', async () => { + // Mock enterprise client listing repos + appGithub.paginate.mockResolvedValue([]) + + const plugin = new AppInstallations(true, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + const result = await plugin.syncDelta([{ + app_slug: 'copilot', + installation_id: 1, + repository_selection: new Set(['repo-a', 'repo-b']), + repository_unselection: new Set(['repo-c']) + }]) + + expect(result).toHaveLength(1) + expect(result[0].plugin).toBe('app_installations') + expect(result[0].action.additions).toEqual(['repo-a', 'repo-b']) + expect(result[0].action.deletions).toEqual(['repo-c']) + }) + + it('generates NopCommand in nop mode for "all" selection', async () => { + const plugin = new AppInstallations(true, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + const result = await plugin.syncDelta([{ + app_slug: 'copilot', + installation_id: 1, + repository_selection: 'all', + repository_unselection: new Set() + }]) + + expect(result).toHaveLength(1) + expect(result[0].action.additions).toEqual(['(all repositories)']) + }) + + it('suppresses unselections in additive mode', async () => { + const plugin = new AppInstallations(true, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + plugin.additive = true + + const result = await plugin.syncDelta([{ + app_slug: 'copilot', + installation_id: 1, + repository_selection: new Set(['repo-a']), + repository_unselection: new Set(['repo-b']) + }]) + + expect(result).toHaveLength(1) + // Should only have additions, no deletions + expect(result[0].action.additions).toEqual(['repo-a']) + expect(result[0].action.deletions).toBeNull() + }) + + it('processes additions before unselections in non-nop mode (422-safe swap)', async () => { + const callOrder = [] + appGithub.request.mockImplementation((route) => { + if (route.includes('/repositories/remove')) callOrder.push('remove') + if (route.includes('/repositories/add')) callOrder.push('add') + return Promise.resolve({ data: {} }) + }) + + const plugin = new AppInstallations(false, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + await plugin.syncDelta([{ + app_slug: 'copilot', + installation_id: 1, + repository_selection: new Set(['repo-a']), + repository_unselection: new Set(['repo-b']) + }]) + + // Additions are applied before removals so a "swap" never drops the + // installation to zero repos (which the Enterprise API rejects with 422). + // Selection/unselection are already disjoint (dedup), so ordering does not + // affect the final set. + expect(callOrder).toEqual(['add', 'remove']) + }) + + it('records a descriptive error when a delta removal is rejected with 422', async () => { + appGithub.request.mockImplementation((route) => { + if (route.includes('/repositories/remove')) { + return Promise.reject(Object.assign(new Error('Unprocessable'), { status: 422 })) + } + return Promise.resolve({ data: {} }) + }) + + const plugin = new AppInstallations(false, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + const result = await plugin.syncDelta([{ + app_slug: 'copilot', + installation_id: 1, + repository_selection: new Set(), + repository_unselection: new Set(['repo-a']) + }]) + + // The 422 is caught and recorded, not thrown out of syncDelta. + expect(Array.isArray(result)).toBe(true) + expect(errors.some(e => /422/.test(e.msg))).toBe(true) + }) + + it('accepts array-based selection/unselection (Settings._buildAppChangesFromDelta output shape)', async () => { + // Settings._buildAppChangesFromDelta returns arrays (not Sets). The + // plugin must treat them the same as Sets, otherwise real add/remove + // operations are silently skipped. + const routes = [] + appGithub.request.mockImplementation((route) => { + routes.push(route) + return Promise.resolve({ data: {} }) + }) + + const plugin = new AppInstallations(false, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + await plugin.syncDelta([{ + app_slug: 'copilot', + installation_id: 1, + repository_selection: ['repo-a'], + repository_unselection: ['repo-b'] + }]) + + expect(routes.some(r => r.includes('/repositories/add'))).toBe(true) + expect(routes.some(r => r.includes('/repositories/remove'))).toBe(true) + }) + + it('generates NopCommand for array-based selection/unselection in nop mode', async () => { + const plugin = new AppInstallations(true, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + const result = await plugin.syncDelta([{ + app_slug: 'copilot', + installation_id: 1, + repository_selection: ['repo-a', 'repo-b'], + repository_unselection: ['repo-c'] + }]) + + expect(result).toHaveLength(1) + expect(result[0].action.additions).toEqual(['repo-a', 'repo-b']) + expect(result[0].action.deletions).toEqual(['repo-c']) + }) + }) + + describe('syncFull', () => { + it('returns empty array for no desired state', async () => { + const plugin = new AppInstallations(false, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + const result = await plugin.syncFull({}) + expect(result).toEqual([]) + }) + + it('reports error when enterprise client is missing', async () => { + const plugin = new AppInstallations(true, github, null, { owner: 'org', repo: 'admin' }, null, log, errors) + const result = await plugin.syncFull({ + copilot: { installation_id: 1, repos: new Set(['repo-a']) } + }) + expect(result).toHaveLength(1) + expect(result[0].type).toBe('ERROR') + }) + + it('generates NopCommand with additions and deletions in nop mode', async () => { + // Mock listInstallationRepos (live state) + appGithub.paginate.mockResolvedValue([ + { name: 'existing-repo', id: 10 }, + { name: 'stale-repo', id: 20 } + ]) + + const plugin = new AppInstallations(true, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + const result = await plugin.syncFull({ + copilot: { + installation_id: 1, + repos: new Set(['existing-repo', 'new-repo']) + } + }) + + expect(result).toHaveLength(1) + expect(result[0].action.additions).toEqual(['new-repo']) + expect(result[0].action.deletions).toEqual(['stale-repo']) + }) + + it('suppresses deletions in additive mode during full sync', async () => { + appGithub.paginate.mockResolvedValue([ + { name: 'existing-repo', id: 10 }, + { name: 'stale-repo', id: 20 } + ]) + + const plugin = new AppInstallations(true, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + plugin.additive = true + + const result = await plugin.syncFull({ + copilot: { + installation_id: 1, + repos: new Set(['existing-repo', 'new-repo']) + } + }) + + expect(result).toHaveLength(1) + expect(result[0].action.additions).toEqual(['new-repo']) + expect(result[0].action.deletions).toBeNull() + }) + + it('skips app when no changes needed', async () => { + appGithub.paginate.mockResolvedValue([ + { name: 'repo-a', id: 10 } + ]) + + const plugin = new AppInstallations(true, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + const result = await plugin.syncFull({ + copilot: { + installation_id: 1, + repos: new Set(['repo-a']) + } + }) + + expect(result).toEqual([]) + }) + + it("toggles to 'all' when desired is all and current is selected", async () => { + const plugin = new AppInstallations(false, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + await plugin.syncFull({ + copilot: { installation_id: 1, repos: 'all', current_selection: 'selected' } + }) + + expect(appGithub.request).toHaveBeenCalledWith( + expect.stringContaining('/repositories'), + expect.objectContaining({ repository_selection: 'all', installation_id: 1 }) + ) + }) + + it('skips when desired is all and current is already all', async () => { + const plugin = new AppInstallations(false, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + const result = await plugin.syncFull({ + copilot: { installation_id: 1, repos: 'all', current_selection: 'all' } + }) + + expect(result).toEqual([]) + expect(appGithub.request).not.toHaveBeenCalled() + }) + + it("narrows from 'all' to 'selected' when desired is a set", async () => { + const plugin = new AppInstallations(false, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + await plugin.syncFull({ + copilot: { installation_id: 1, repos: new Set(['repo-a']), current_selection: 'all' } + }) + + expect(appGithub.request).toHaveBeenCalledWith( + expect.stringContaining('/repositories'), + expect.objectContaining({ repository_selection: 'selected', repositories: ['repo-a'] }) + ) + }) + + it("leaves 'all' untouched in additive mode", async () => { + const plugin = new AppInstallations(false, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + plugin.additive = true + const result = await plugin.syncFull({ + copilot: { installation_id: 1, repos: new Set(['repo-a']), current_selection: 'all' } + }) + + expect(result).toEqual([]) + expect(appGithub.request).not.toHaveBeenCalled() + }) + + it('applies additions before removals to avoid a transient empty selection (422-safe swap)', async () => { + // live = {repo-a}; desired = {repo-b} → swap. Removing first could drop the + // installation to zero repos (422); additions must be applied first. + appGithub.paginate.mockResolvedValue([{ name: 'repo-a', id: 10 }]) + const routes = [] + appGithub.request.mockImplementation((route) => { + routes.push(route) + return Promise.resolve({ data: {} }) + }) + + const plugin = new AppInstallations(false, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + await plugin.syncFull({ + copilot: { installation_id: 1, repos: new Set(['repo-b']), current_selection: 'selected' } + }) + + const addIdx = routes.findIndex(r => r.includes('/repositories/add')) + const removeIdx = routes.findIndex(r => r.includes('/repositories/remove')) + expect(addIdx).toBeGreaterThanOrEqual(0) + expect(removeIdx).toBeGreaterThanOrEqual(0) + expect(addIdx).toBeLessThan(removeIdx) + }) + + it("errors (without mutating) when the desired repo set is empty for a 'selected' installation", async () => { + appGithub.paginate.mockResolvedValue([{ name: 'repo-a', id: 10 }]) + + const plugin = new AppInstallations(true, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + const result = await plugin.syncFull({ + copilot: { installation_id: 1, repos: new Set(), current_selection: 'selected' } + }) + + expect(result).toHaveLength(1) + expect(result[0].type).toBe('ERROR') + expect(errors.some(e => /zero repositories/i.test(e.msg))).toBe(true) + expect(appGithub.request).not.toHaveBeenCalled() + }) + + it('records a descriptive error when removal is rejected with 422', async () => { + appGithub.paginate.mockResolvedValue([ + { name: 'repo-a', id: 10 }, + { name: 'repo-b', id: 20 } + ]) + appGithub.request.mockImplementation((route) => { + if (route.includes('/repositories/remove')) { + return Promise.reject(Object.assign(new Error('Unprocessable'), { status: 422 })) + } + return Promise.resolve({ data: {} }) + }) + + const plugin = new AppInstallations(false, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + // desired = {repo-a}; live = {repo-a, repo-b} → toRemove = [repo-b] + const result = await plugin.syncFull({ + copilot: { installation_id: 1, repos: new Set(['repo-a']), current_selection: 'selected' } + }) + + // The 422 is caught and recorded, not thrown out of syncFull. + expect(Array.isArray(result)).toBe(true) + expect(errors.some(e => /422/.test(e.msg))).toBe(true) + }) + + it("errors (without mutating) when narrowing 'all' to an empty desired set", async () => { + const plugin = new AppInstallations(true, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + const result = await plugin.syncFull({ + copilot: { installation_id: 1, repos: new Set(), current_selection: 'all' } + }) + + expect(result).toHaveLength(1) + expect(result[0].type).toBe('ERROR') + expect(errors.some(e => /narrow repository_selection from 'all'/.test(e.msg))).toBe(true) + // The over-broad 'all' installation must be left unchanged. + expect(appGithub.request).not.toHaveBeenCalled() + }) + + it("leaves 'all' untouched (no error) for an empty desired set in additive mode", async () => { + const plugin = new AppInstallations(true, github, appGithub, { owner: 'org', repo: 'admin' }, 'ent', log, errors) + plugin.additive = true + const result = await plugin.syncFull({ + copilot: { installation_id: 1, repos: new Set(), current_selection: 'all' } + }) + + expect(result).toEqual([]) + expect(errors).toEqual([]) + expect(appGithub.request).not.toHaveBeenCalled() + }) + }) +}) diff --git a/test/unit/lib/repoSelector.test.js b/test/unit/lib/repoSelector.test.js new file mode 100644 index 00000000..1285100b --- /dev/null +++ b/test/unit/lib/repoSelector.test.js @@ -0,0 +1,152 @@ +const RepoSelector = require('../../../lib/repoSelector') + +describe('RepoSelector', () => { + let github + let log + + beforeEach(() => { + log = { + debug: jest.fn(), + error: jest.fn() + } + + github = { + paginate: jest.fn(), + rest: { + teams: { + listReposInOrg: { + endpoint: { + merge: jest.fn().mockReturnValue({}) + } + } + } + }, + request: { + endpoint: jest.fn().mockReturnValue({}) + } + } + }) + + describe('resolve', () => { + it('returns empty set for null criteria', async () => { + const selector = new RepoSelector(github, 'my-org', log) + const result = await selector.resolve(null) + expect(result).toEqual(new Set()) + }) + + it('returns empty set for empty criteria', async () => { + const selector = new RepoSelector(github, 'my-org', log) + const result = await selector.resolve({}) + expect(result).toEqual(new Set()) + }) + }) + + describe('getAllRepos', () => { + it('returns all repo names from installation', async () => { + github.paginate.mockResolvedValue([ + { name: 'repo-a' }, + { name: 'repo-b' }, + { name: 'repo-c' } + ]) + + const selector = new RepoSelector(github, 'my-org', log) + const result = await selector.resolve({ all: true }) + expect(result).toEqual(new Set(['repo-a', 'repo-b', 'repo-c'])) + }) + }) + + describe('resolveByName', () => { + it('returns explicit repo names directly', async () => { + const selector = new RepoSelector(github, 'my-org', log) + const result = await selector.resolve({ names: ['repo-a', 'repo-b'] }) + expect(result).toEqual(new Set(['repo-a', 'repo-b'])) + expect(github.paginate).not.toHaveBeenCalled() + }) + + it('resolves glob patterns against all repos', async () => { + github.paginate.mockResolvedValue([ + { name: 'api-service' }, + { name: 'api-gateway' }, + { name: 'web-frontend' } + ]) + + const selector = new RepoSelector(github, 'my-org', log) + const result = await selector.resolve({ names: ['api-*'] }) + expect(result).toEqual(new Set(['api-service', 'api-gateway'])) + }) + }) + + describe('resolveByTeam', () => { + it('returns repos from team membership', async () => { + github.paginate.mockResolvedValue([ + { name: 'team-repo-1' }, + { name: 'team-repo-2' } + ]) + + const selector = new RepoSelector(github, 'my-org', log) + const result = await selector.resolve({ teams: ['my-team'] }) + expect(result).toEqual(new Set(['team-repo-1', 'team-repo-2'])) + }) + + it('unions repos from multiple teams', async () => { + github.paginate + .mockResolvedValueOnce([{ name: 'repo-a' }, { name: 'repo-b' }]) + .mockResolvedValueOnce([{ name: 'repo-b' }, { name: 'repo-c' }]) + + const selector = new RepoSelector(github, 'my-org', log) + const result = await selector.resolve({ teams: ['team-1', 'team-2'] }) + expect(result).toEqual(new Set(['repo-a', 'repo-b', 'repo-c'])) + }) + }) + + describe('resolveByCustomProperties', () => { + it('returns repos matching property values', async () => { + github.paginate.mockResolvedValue([ + { repository_name: 'prop-repo-1' }, + { repository_name: 'prop-repo-2' } + ]) + + const selector = new RepoSelector(github, 'my-org', log) + const result = await selector.resolve({ + custom_properties: [{ environment: 'production' }] + }) + expect(result).toEqual(new Set(['prop-repo-1', 'prop-repo-2'])) + }) + }) + + describe('combined criteria', () => { + it('unions results from multiple criteria types', async () => { + // First call: teams resolution + github.paginate + .mockResolvedValueOnce([{ name: 'team-repo' }]) + // Second call: custom properties + .mockResolvedValueOnce([{ repository_name: 'prop-repo' }]) + + const selector = new RepoSelector(github, 'my-org', log) + const result = await selector.resolve({ + names: ['explicit-repo'], + teams: ['my-team'], + custom_properties: [{ tier: 'critical' }] + }) + expect(result).toEqual(new Set(['explicit-repo', 'team-repo', 'prop-repo'])) + }) + + it('all=true takes precedence over other criteria', async () => { + github.paginate.mockResolvedValue([ + { name: 'repo-1' }, + { name: 'repo-2' } + ]) + + const selector = new RepoSelector(github, 'my-org', log) + const result = await selector.resolve({ + all: true, + names: ['specific-repo'], + teams: ['my-team'] + }) + // Should return all repos, not filter by names/teams + expect(result).toEqual(new Set(['repo-1', 'repo-2'])) + // paginate called once for getAllRepos, not for teams + expect(github.paginate).toHaveBeenCalledTimes(1) + }) + }) +}) diff --git a/test/unit/lib/settings.test.js b/test/unit/lib/settings.test.js index 79d4a8a1..93de139f 100644 --- a/test/unit/lib/settings.test.js +++ b/test/unit/lib/settings.test.js @@ -1096,11 +1096,11 @@ repository: describe('additive_plugins', () => { // ── Settings.ADDITIVE_PLUGINS constant ─────────────────────────────── describe('Settings.ADDITIVE_PLUGINS', () => { - it('28. contains all 10 Diffable-extending plugin names', () => { + it('28. contains all 11 additive plugin names', () => { const expected = new Set([ 'labels', 'collaborators', 'teams', 'milestones', 'autolinks', 'environments', 'custom_properties', 'variables', 'rulesets', - 'custom_repository_roles' + 'custom_repository_roles', 'app_installations' ]) expect(Settings.ADDITIVE_PLUGINS).toEqual(expected) }) @@ -1126,12 +1126,12 @@ repository: expect(result).toEqual(new Set(['labels', 'teams', 'milestones'])) }) - it('32. all 10 Diffable plugins are accepted without error', () => { + it('32. all 11 additive plugins are accepted without error', () => { const all = [...Settings.ADDITIVE_PLUGINS] const settings = createSettings({ additive_plugins: all }) const logErrorSpy = jest.spyOn(settings, 'logError').mockImplementation(() => {}) const result = settings.normalizeAdditivePlugins() - expect(result.size).toBe(10) + expect(result.size).toBe(11) expect(logErrorSpy).not.toHaveBeenCalled() logErrorSpy.mockRestore() }) @@ -1615,4 +1615,106 @@ repository: expect(result).toEqual({ repos: [], previousPluginSections: [] }) }) }) + + describe('_buildAppChangesFromDelta', () => { + let settings + const AppOctokitClient = require('../../../lib/appOctokitClient') + const RepoSelector = require('../../../lib/repoSelector') + + beforeEach(() => { + stubConfig = { restrictedRepos: {} } + settings = createSettings(stubConfig) + // Map app slug -> installation id + jest.spyOn(AppOctokitClient.prototype, 'listOrgInstallations').mockResolvedValue([ + { app_slug: 'my-app', id: 42 } + ]) + }) + + afterEach(() => { + jest.restoreAllMocks() + }) + + it('skips an app when suborg targeting and app_installations are unchanged', async () => { + // Same targeting resolves to the same repos in both versions + jest.spyOn(RepoSelector.prototype, 'resolve').mockResolvedValue(new Set(['repo-a', 'repo-b'])) + + // Current suborg config: app present + settings.subOrgConfigs = { + frontend: { + suborgrepos: ['repo-a', 'repo-b'], + app_installations: [{ app_slug: 'my-app' }] + } + } + // Previous version (baseRef): identical app_installations + settings.loadYamlFromRef = jest.fn().mockResolvedValue({ + suborgrepos: ['repo-a', 'repo-b'], + app_installations: [{ app_slug: 'my-app' }] + }) + + const result = await settings._buildAppChangesFromDelta( + settings.github, + 'my-enterprise', + [{ repo: 'frontend', path: '.github/suborgs/frontend.yml' }], + [], + 'prev-sha' + ) + + // No churn: nothing to add or remove + expect(result).toEqual([]) + }) + + it('emits only the targeting diff when suborg repos change', async () => { + // previous: repo-a, repo-b ; current: repo-b, repo-c + jest.spyOn(RepoSelector.prototype, 'resolve') + .mockResolvedValueOnce(new Set(['repo-b', 'repo-c'])) // current + .mockResolvedValueOnce(new Set(['repo-a', 'repo-b'])) // previous + + settings.subOrgConfigs = { + frontend: { + suborgrepos: ['repo-b', 'repo-c'], + app_installations: [{ app_slug: 'my-app' }] + } + } + settings.loadYamlFromRef = jest.fn().mockResolvedValue({ + suborgrepos: ['repo-a', 'repo-b'], + app_installations: [{ app_slug: 'my-app' }] + }) + + const result = await settings._buildAppChangesFromDelta( + settings.github, + 'my-enterprise', + [{ repo: 'frontend', path: '.github/suborgs/frontend.yml' }], + [], + 'prev-sha' + ) + + expect(result).toHaveLength(1) + expect(result[0].app_slug).toBe('my-app') + expect(result[0].repository_selection.sort()).toEqual(['repo-c']) + expect(result[0].repository_unselection.sort()).toEqual(['repo-a']) + }) + + it('skips apps configured at the org level (org entry implies all repos)', async () => { + jest.spyOn(RepoSelector.prototype, 'resolve').mockResolvedValue(new Set(['repo-a'])) + + settings.config = { + ...settings.config, + app_installations: [{ app_slug: 'my-app' }] + } + settings.subOrgConfigs = { + frontend: { suborgrepos: ['repo-a'], app_installations: [{ app_slug: 'my-app' }] } + } + settings.loadYamlFromRef = jest.fn().mockResolvedValue({}) + + const result = await settings._buildAppChangesFromDelta( + settings.github, + 'my-enterprise', + [{ repo: 'frontend', path: '.github/suborgs/frontend.yml' }], + [], + 'prev-sha' + ) + + expect(result).toEqual([]) + }) + }) }) // Settings Tests