From 5894eae43194d6236a995a17e3902dafdfb499ca Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Thu, 25 Jun 2026 11:02:05 -0400 Subject: [PATCH 1/9] feat: add app installation plugin for managing GitHub App repo access MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new plugin system where the target is a GitHub App installation rather than a repository. This enables managing which repos each app has access to (repository_selection) through the same config hierarchy (org → suborg → repo) that safe-settings uses for repo-level settings. New files: - lib/plugins/appInstallations.js: Plugin with delta + full sync modes - lib/appOctokitClient.js: Enterprise Octokit client with auto-batching at 50 repos per API call - lib/repoSelector.js: Repo resolution utility (name, team, properties) Integration: - settings.js: syncAppInstallations as separate phase after updateOrg - index.js: installation/installation_target webhook handlers for drift detection, enrichContextWithEnterprise helper Supports disable_plugins and additive_plugins. Enterprise slug is extracted from webhook payload (no env var needed). Closes #1005 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- index.js | 72 +++++ lib/appOctokitClient.js | 145 +++++++++ lib/plugins/appInstallations.js | 302 ++++++++++++++++++ lib/repoSelector.js | 159 +++++++++ lib/settings.js | 182 ++++++++++- test/unit/lib/appOctokitClient.test.js | 110 +++++++ .../unit/lib/plugins/appInstallations.test.js | 206 ++++++++++++ test/unit/lib/repoSelector.test.js | 152 +++++++++ test/unit/lib/settings.test.js | 8 +- 9 files changed, 1330 insertions(+), 6 deletions(-) create mode 100644 lib/appOctokitClient.js create mode 100644 lib/plugins/appInstallations.js create mode 100644 lib/repoSelector.js create mode 100644 test/unit/lib/appOctokitClient.test.js create mode 100644 test/unit/lib/plugins/appInstallations.test.js create mode 100644 test/unit/lib/repoSelector.test.js diff --git a/index.js b/index.js index d3b801c5..f1d161f0 100644 --- a/index.js +++ b/index.js @@ -21,6 +21,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 +145,26 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } } } + /** + * Enriches the context with enterprise info for app installation management. + * Extracts enterprise slug from the webhook payload and creates an + * app-authenticated Octokit client for enterprise API calls. + * + * @param {object} context - Probot context + */ + async function enrichContextWithEnterprise (context) { + const { payload } = context + const enterprise = payload.enterprise || (payload.installation && payload.installation.enterprise) + if (enterprise && enterprise.slug) { + context.enterpriseSlug = enterprise.slug + try { + context.appGithub = await robot.auth() + } catch (e) { + robot.log.debug(`Could not create app-authenticated client for enterprise: ${e.message}`) + } + } + } + /** * Loads the deployment config file from file system * Do this once when the app starts and then return the cached value @@ -482,6 +505,55 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } }) + // ──────────────────────────────────────────────────────────────────────── + // App installation drift detection handlers + // ──────────────────────────────────────────────────────────────────────── + + const installation_change_events = [ + 'installation.repositories_added', + 'installation.repositories_removed' + ] + + robot.on(installation_change_events, async context => { + const { payload } = context + const { sender } = payload + robot.log.debug('App installation repos changed by ', JSON.stringify(sender)) + if (sender.type === 'Bot') { + robot.log.debug('App installation repos changed by Bot') + return + } + robot.log.debug('App installation repos changed by a Human — triggering sync to revert drift') + + // Build a context that targets the admin repo for this org + const orgLogin = payload.installation.account.login + const updatedContext = Object.assign({}, context, { + repo: () => { return { repo: env.ADMIN_REPO, owner: orgLogin } } + }) + return syncAllSettings(false, updatedContext) + }) + + 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..dda22141 --- /dev/null +++ b/lib/appOctokitClient.js @@ -0,0 +1,145 @@ +const BATCH_SIZE = 50 + +/** + * AppOctokitClient wraps an Octokit client authenticated as the GitHub App + * (JWT) and provides methods for managing app installation repository access + * via the Enterprise Organization Installations API. + * + * Prerequisites: + * - safe-settings must be installed on the enterprise with + * "Enterprise organization installations" permission. + * - The enterprise slug is obtained from the webhook event payload + * (payload.enterprise.slug). + * + * @param {object} options + * @param {object} options.github - Octokit client authenticated as the app (via robot.auth()) + * @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 all app installations in the enterprise for a given org. + * Returns array of installation objects with { id, app_slug, app_id, ... } + * + * @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/installations', + { + enterprise: this.enterpriseSlug, + headers: { 'X-GitHub-Api-Version': '2026-03-10' } + } + ) + const installations = await this.github.paginate(options) + // Filter to installations for the specified org + return installations.filter(i => + i.account && i.account.login === org + ) + } 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. + * + * @param {number} installationId - The installation ID + * @returns {Promise} List of repository objects + */ + async listInstallationRepos (installationId) { + try { + const options = this.github.request.endpoint.merge( + 'GET /enterprises/{enterprise}/apps/installations/{installation_id}/repositories', + { + enterprise: this.enterpriseSlug, + installation_id: installationId, + headers: { 'X-GitHub-Api-Version': '2026-03-10' } + } + ) + return this.github.paginate(options) + } catch (e) { + this.log.error(`Error listing repos for installation ${installationId}: ${e.message}`) + throw e + } + } + + /** + * Grant repository access to an app installation. + * Automatically batches into chunks of 50 (API limit). + * + * @param {number} installationId - The installation ID + * @param {number[]} repositoryIds - Array of repository IDs to add + * @returns {Promise} + */ + async addReposToInstallation (installationId, repositoryIds) { + if (!repositoryIds || repositoryIds.length === 0) return + + const batches = this._chunk(repositoryIds, BATCH_SIZE) + for (const batch of batches) { + this.log.debug(`Adding ${batch.length} repos to installation ${installationId}`) + await this.github.request( + 'POST /enterprises/{enterprise}/apps/installations/{installation_id}/repositories', + { + enterprise: this.enterpriseSlug, + installation_id: installationId, + repository_ids: batch, + headers: { 'X-GitHub-Api-Version': '2026-03-10' } + } + ) + } + } + + /** + * Remove repository access from an app installation. + * Automatically batches into chunks of 50 (API limit). + * + * @param {number} installationId - The installation ID + * @param {number[]} repositoryIds - Array of repository IDs to remove + * @returns {Promise} + */ + async removeReposFromInstallation (installationId, repositoryIds) { + if (!repositoryIds || repositoryIds.length === 0) return + + const batches = this._chunk(repositoryIds, BATCH_SIZE) + for (const batch of batches) { + this.log.debug(`Removing ${batch.length} repos from installation ${installationId}`) + await this.github.request( + 'DELETE /enterprises/{enterprise}/apps/installations/{installation_id}/repositories', + { + enterprise: this.enterpriseSlug, + installation_id: installationId, + repository_ids: batch, + headers: { 'X-GitHub-Api-Version': '2026-03-10' } + } + ) + } + } + + /** + * 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/plugins/appInstallations.js b/lib/plugins/appInstallations.js new file mode 100644 index 00000000..bdd03a19 --- /dev/null +++ b/lib/plugins/appInstallations.js @@ -0,0 +1,302 @@ +/* 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.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 | 'all', // repos to add + * repository_unselection: Set, // 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' } + * @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 { + // Get live state + const liveRepos = await this.enterpriseClient.listInstallationRepos(desired.installation_id) + const liveRepoNames = new Set(liveRepos.map(r => r.name)) + const liveRepoMap = new Map(liveRepos.map(r => [r.name, r.id])) + + let desiredRepoNames + if (desired.repos === 'all') { + // Desired = all repos in org + desiredRepoNames = await this._getAllRepoNames() + } else { + desiredRepoNames = desired.repos + } + + // Compute diff + const toAdd = new Set([...desiredRepoNames].filter(r => !liveRepoNames.has(r))) + const toRemove = this.additive + ? new Set() // Additive mode: never remove + : new Set([...liveRepoNames].filter(r => !desiredRepoNames.has(r))) + + if (toAdd.size === 0 && toRemove.size === 0) { + this.log.debug(`App '${appSlug}': no changes needed`) + continue + } + + if (this.nop) { + results.push(new NopCommand( + 'app_installations', + this.repo, + null, + { + msg: `App '${appSlug}' installation repos`, + additions: toAdd.size > 0 ? [...toAdd] : null, + modifications: null, + deletions: toRemove.size > 0 ? [...toRemove] : null + } + )) + continue + } + + // Resolve names to IDs for additions (need to look up IDs) + if (toAdd.size > 0) { + const addIds = await this._resolveRepoIds([...toAdd]) + await this.enterpriseClient.addReposToInstallation(desired.installation_id, addIds) + this.log.debug(`App '${appSlug}': added ${addIds.length} repos`) + } + + if (toRemove.size > 0) { + const removeIds = [...toRemove].map(name => liveRepoMap.get(name)).filter(Boolean) + await this.enterpriseClient.removeReposFromInstallation(desired.installation_id, removeIds) + this.log.debug(`App '${appSlug}': removed ${removeIds.length} repos`) + } + } 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 + } + + /** + * Process a single app's delta change. + * @private + */ + async _processAppChange (change) { + const results = [] + const { app_slug, installation_id, repository_selection, repository_unselection } = change + + 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 + } + + 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 — set repository_selection 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 + } + )) + return results + } + + // For "all" repos, get the full list and add all + const allRepoNames = await this._getAllRepoNames() + const liveRepos = await this.enterpriseClient.listInstallationRepos(installation_id) + const liveRepoNames = new Set(liveRepos.map(r => r.name)) + const toAdd = [...allRepoNames].filter(r => !liveRepoNames.has(r)) + + if (toAdd.length > 0) { + const addIds = await this._resolveRepoIds(toAdd) + await this.enterpriseClient.addReposToInstallation(installation_id, addIds) + this.log.debug(`App '${app_slug}': added all repos (${addIds.length} new)`) + } + + 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 + } + )) + return results + } + + if (hasSelections) { + const addIds = await this._resolveRepoIds([...repository_selection]) + await this.enterpriseClient.addReposToInstallation(installation_id, addIds) + this.log.debug(`App '${app_slug}': added ${addIds.length} repos`) + } + + if (hasUnselections) { + const removeIds = await this._resolveRepoIds([...repository_unselection]) + await this.enterpriseClient.removeReposFromInstallation(installation_id, removeIds) + this.log.debug(`App '${app_slug}': removed ${removeIds.length} repos`) + } + + return results + } + + /** + * Get all repo names visible to the installation. + * @private + */ + async _getAllRepoNames () { + const repos = await this.github.paginate('GET /installation/repositories') + return new Set(repos.map(r => r.name)) + } + + /** + * Resolve repo names to IDs. + * @private + */ + async _resolveRepoIds (repoNames) { + const ids = [] + for (const name of repoNames) { + try { + const { data } = await this.github.repos.get({ + owner: this.repo.owner, + repo: name + }) + ids.push(data.id) + } catch (e) { + this.log.debug(`Could not resolve repo ID for '${name}': ${e.message}`) + } + } + return ids + } +} + +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..8f7dcb59 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') @@ -563,6 +565,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() @@ -1404,6 +1410,176 @@ 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.changedAppSlugs] - App slugs affected by config changes (delta mode) + * @param {Array} [options.appChanges] - Pre-computed per-app changes (delta mode) + */ + async syncAppInstallations (options = {}) { + const { appGithub, enterpriseSlug, appChanges } = options + + const appInstallationsConfig = this.config.app_installations + if (!appInstallationsConfig || !Array.isArray(appInstallationsConfig) || appInstallationsConfig.length === 0) { + 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 webhook payload. Ensure the webhook is from an enterprise-managed org.' + 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) { + // Delta mode: process pre-computed changes + results = await plugin.syncDelta(appChanges) + } 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) + } + + /** + * 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) + + // 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) { + this.log.error(`Failed to list org installations: ${e.message}`) + return desiredState + } + } + + const installationMap = new Map() + for (const inst of orgInstallations) { + installationMap.set(inst.app_slug, inst.id) + } + + // Process org-level config + for (const appConfig of orgAppInstallations) { + const slug = appConfig.app_slug + if (!slug) continue + + const installationId = installationMap.get(slug) + if (!installationId) { + this.log.debug(`App '${slug}' not found in org installations, skipping`) + continue + } + + if (appConfig.repository_selection === 'all') { + desiredState[slug] = { installation_id: installationId, repos: 'all' } + } else { + desiredState[slug] = { installation_id: installationId, repos: new Set() } + } + } + + // 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) 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) continue + desiredState[slug] = { installation_id: installationId, repos: new Set() } + } + if (desiredState[slug].repos === 'all') continue + desiredState[slug].repos.add(repoName) + } + } + } + + return desiredState + } + async updateRepos (repo) { this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs() // Snapshot the set of suborg `source` paths that match this repo *before* @@ -2335,7 +2511,8 @@ Settings.ADDITIVE_PLUGINS = new Set([ 'custom_properties', 'variables', 'rulesets', - 'custom_repository_roles' + 'custom_repository_roles', + 'app_installations' ]) Settings.PLUGINS = { @@ -2351,7 +2528,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/test/unit/lib/appOctokitClient.test.js b/test/unit/lib/appOctokitClient.test.js new file mode 100644 index 00000000..fdd37517 --- /dev/null +++ b/test/unit/lib/appOctokitClient.test.js @@ -0,0 +1,110 @@ +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 installations filtered by org', async () => { + github.paginate.mockResolvedValue([ + { id: 1, app_slug: 'app-a', account: { login: 'my-org' } }, + { id: 2, app_slug: 'app-b', account: { login: 'other-org' } }, + { id: 3, app_slug: 'app-c', account: { login: 'my-org' } } + ]) + + const result = await client.listOrgInstallations('my-org') + expect(result).toHaveLength(2) + expect(result[0].app_slug).toBe('app-a') + expect(result[1].app_slug).toBe('app-c') + }) + + 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('addReposToInstallation', () => { + it('does nothing for empty array', async () => { + await client.addReposToInstallation(123, []) + expect(github.request).not.toHaveBeenCalled() + }) + + it('sends single batch for <= 50 repos', async () => { + const ids = Array.from({ length: 10 }, (_, i) => i + 1) + await client.addReposToInstallation(123, ids) + expect(github.request).toHaveBeenCalledTimes(1) + expect(github.request).toHaveBeenCalledWith( + expect.stringContaining('POST'), + expect.objectContaining({ + repository_ids: ids, + installation_id: 123 + }) + ) + }) + + it('batches into chunks of 50', async () => { + const ids = Array.from({ length: 120 }, (_, i) => i + 1) + await client.addReposToInstallation(123, ids) + expect(github.request).toHaveBeenCalledTimes(3) // 50 + 50 + 20 + }) + }) + + describe('removeReposFromInstallation', () => { + it('does nothing for empty array', async () => { + await client.removeReposFromInstallation(123, []) + expect(github.request).not.toHaveBeenCalled() + }) + + it('batches into chunks of 50', async () => { + const ids = Array.from({ length: 75 }, (_, i) => i + 1) + await client.removeReposFromInstallation(123, ids) + 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..739ca6f9 --- /dev/null +++ b/test/unit/lib/plugins/appInstallations.test.js @@ -0,0 +1,206 @@ +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('adds repos via enterprise client in non-nop mode', async () => { + github.repos.get + .mockResolvedValueOnce({ data: { id: 100 } }) + .mockResolvedValueOnce({ data: { id: 200 } }) + + 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', 'repo-b']), + repository_unselection: new Set() + }]) + + // Should have called request to add repos + expect(appGithub.request).toHaveBeenCalledWith( + expect.stringContaining('POST'), + expect.objectContaining({ + repository_ids: [100, 200] + }) + ) + }) + }) + + 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([]) + }) + }) +}) 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..85c66181 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() }) From a51ab87782374cd35158a659b038c3ad0ebeebbe Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Thu, 25 Jun 2026 12:29:24 -0400 Subject: [PATCH 2/9] fix: use enterprise installation token for app installation API calls robot.auth() without args returns a JWT app client, not an enterprise installation-authenticated client. Fix enrichContextWithEnterprise to: 1. Use JWT client to list all installations 2. Find the enterprise installation matching payload.enterprise.slug 3. Call robot.auth(enterpriseInstallation.id) to get the properly authenticated client for enterprise API calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- index.js | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index f1d161f0..a1ad3fdd 100644 --- a/index.js +++ b/index.js @@ -147,8 +147,9 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } /** * Enriches the context with enterprise info for app installation management. - * Extracts enterprise slug from the webhook payload and creates an - * app-authenticated Octokit client for enterprise API calls. + * 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 */ @@ -158,9 +159,22 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => if (enterprise && enterprise.slug) { context.enterpriseSlug = enterprise.slug try { - context.appGithub = await robot.auth() + // Get a JWT-authenticated client to list all installations + const appGithub = await robot.auth() + const installations = await appGithub.paginate( + appGithub.apps.listInstallations.endpoint.merge({ per_page: 100 }) + ) + // Find the installation targeting this enterprise + const enterpriseInstallation = installations.find( + i => i.target_type === 'Enterprise' && i.account && i.account.slug === enterprise.slug + ) + if (enterpriseInstallation) { + context.appGithub = await robot.auth(enterpriseInstallation.id) + } else { + robot.log.debug(`No enterprise installation found for slug '${enterprise.slug}'. App installation management will not be available.`) + } } catch (e) { - robot.log.debug(`Could not create app-authenticated client for enterprise: ${e.message}`) + robot.log.debug(`Could not create enterprise-authenticated client: ${e.message}`) } } } From 8fee0dddfbffdeffc6894f0270822998e55bb19f Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Thu, 25 Jun 2026 12:34:30 -0400 Subject: [PATCH 3/9] perf: cache enterprise installation ID to avoid repeated lookups Store the enterprise installation ID after the first successful lookup so subsequent calls to enrichContextWithEnterprise can skip the listInstallations API call and directly call robot.auth(cachedId). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- index.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index a1ad3fdd..a91199aa 100644 --- a/index.js +++ b/index.js @@ -12,6 +12,7 @@ let deploymentConfig module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => { let appSlug = 'safe-settings' + let cachedEnterpriseInstallationId = null async function syncAllSettings (nop, context, repo = context.repo(), ref, baseRef, changedFiles = {}) { try { deploymentConfig = await loadYamlFileSystem() @@ -159,6 +160,12 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => if (enterprise && enterprise.slug) { context.enterpriseSlug = enterprise.slug try { + // Use cached enterprise installation ID if available + if (cachedEnterpriseInstallationId) { + context.appGithub = await robot.auth(cachedEnterpriseInstallationId) + return + } + // Get a JWT-authenticated client to list all installations const appGithub = await robot.auth() const installations = await appGithub.paginate( @@ -169,7 +176,8 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => i => i.target_type === 'Enterprise' && i.account && i.account.slug === enterprise.slug ) if (enterpriseInstallation) { - context.appGithub = await robot.auth(enterpriseInstallation.id) + cachedEnterpriseInstallationId = enterpriseInstallation.id + context.appGithub = await robot.auth(cachedEnterpriseInstallationId) } else { robot.log.debug(`No enterprise installation found for slug '${enterprise.slug}'. App installation management will not be available.`) } From d0be177ae64406a3f7d0e112c31121a7619e087a Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Thu, 25 Jun 2026 22:23:53 -0400 Subject: [PATCH 4/9] Wire syncAppInstallations into syncSelectedRepos for delta processing - Call enrichContextWithEnterprise in syncSelectedSettings (index.js) - Call syncAppInstallations with changedSubOrgs/changedRepos in syncSelectedRepos - Add _buildAppChangesFromDelta helper to extract affected apps from changed suborg/repo configs and compute repository_selection per app - syncAppInstallations now handles three modes: pre-computed delta, config-based delta (from changed files), and full sync Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- index.js | 3 ++ lib/settings.js | 119 ++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 119 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index a91199aa..f8d2ffd5 100644 --- a/index.js +++ b/index.js @@ -92,6 +92,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) { diff --git a/lib/settings.js b/lib/settings.js index 8f7dcb59..2ea2d731 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -655,6 +655,15 @@ 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 + }) + await settings.handleResults() } catch (error) { settings.logError(error.message) @@ -1423,10 +1432,14 @@ class Settings { * @param {Array} [options.appChanges] - Pre-computed per-app changes (delta mode) */ async syncAppInstallations (options = {}) { - const { appGithub, enterpriseSlug, appChanges } = options + const { appGithub, enterpriseSlug, appChanges, changedSubOrgs, changedRepos } = options const appInstallationsConfig = this.config.app_installations - if (!appInstallationsConfig || !Array.isArray(appInstallationsConfig) || appInstallationsConfig.length === 0) { + // 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) + + if (!hasOrgConfig && !hasChangedConfigs && (!appChanges || appChanges.length === 0)) { this.log.debug('No app_installations config found, skipping') return } @@ -1462,8 +1475,16 @@ class Settings { let results if (appChanges && appChanges.length > 0) { - // Delta mode: process pre-computed changes + // 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) + 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) @@ -1476,6 +1497,98 @@ class Settings { this.appendToResults(results) } + /** + * Build delta-based app changes from changed suborg/repo config files. + * Extracts app_installations from each changed config and computes + * repository_selection / repository_unselection per app. + * @private + */ + async _buildAppChangesFromDelta (appGithub, enterpriseSlug, changedSubOrgs = [], changedRepos = []) { + 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) + let 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) { + this.log.error(`Failed to list org installations for delta: ${e.message}`) + return [] + } + } + + // Process changed suborg configs + for (const suborg of changedSubOrgs) { + const suborgConfig = this.subOrgConfigs && this.subOrgConfigs[suborg.repo] + if (!suborgConfig || !suborgConfig.app_installations) continue + + // Resolve repos for this suborg's current targeting + 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 '${suborg.repo}': ${e.message}`) + } + + for (const appConfig of suborgConfig.app_installations) { + const slug = appConfig.app_slug + if (!slug) continue + const installationId = installationMap.get(slug) + if (!installationId) continue + + if (!appChangeMap.has(slug)) { + appChangeMap.set(slug, { + app_slug: slug, + installation_id: installationId, + repository_selection: new Set(), + repository_unselection: new Set() + }) + } + const change = appChangeMap.get(slug) + for (const repo of suborgRepos) { + change.repository_selection.add(repo) + } + } + } + + // Process changed repo configs + for (const repo of changedRepos) { + const repoConfig = this.repoConfigs && + (this.repoConfigs[`${repo.repo}.yml`] || this.repoConfigs[`${repo.repo}.yaml`]) + if (!repoConfig || !repoConfig.app_installations) continue + + for (const appConfig of repoConfig.app_installations) { + const slug = appConfig.app_slug + if (!slug) continue + const installationId = installationMap.get(slug) + if (!installationId) continue + + if (!appChangeMap.has(slug)) { + appChangeMap.set(slug, { + app_slug: slug, + installation_id: installationId, + repository_selection: new Set(), + repository_unselection: new Set() + }) + } + appChangeMap.get(slug).repository_selection.add(repo.repo) + } + } + + return [...appChangeMap.values()] + } + /** * Compute the full desired state for all managed apps by merging * org + suborg + repo level app_installations configs. From 24fb684cb45bccb683c558abb529b4d465dcf2a8 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Thu, 25 Jun 2026 23:24:30 -0400 Subject: [PATCH 5/9] Compute repository_unselection by diffing previous vs current configs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Load previous version of changed suborg/repo configs using loadYamlFromRef - Compare old vs new app_installations sections to detect: - Apps removed from config → unselect all previously targeted repos - Targeting criteria changed → unselect repos no longer in scope - Selection takes precedence over unselection when both apply - Pass baseRef through syncAppInstallations → _buildAppChangesFromDelta Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/settings.js | 175 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 124 insertions(+), 51 deletions(-) diff --git a/lib/settings.js b/lib/settings.js index 2ea2d731..575ebb3d 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -661,7 +661,8 @@ class Settings { appGithub: context.appGithub, enterpriseSlug: context.enterpriseSlug, changedSubOrgs: subOrgs, - changedRepos: repos + changedRepos: repos, + baseRef }) await settings.handleResults() @@ -1432,7 +1433,7 @@ class Settings { * @param {Array} [options.appChanges] - Pre-computed per-app changes (delta mode) */ async syncAppInstallations (options = {}) { - const { appGithub, enterpriseSlug, appChanges, changedSubOrgs, changedRepos } = 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) @@ -1479,7 +1480,7 @@ class Settings { 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) + const deltaChanges = await this._buildAppChangesFromDelta(appGithub, enterpriseSlug, changedSubOrgs, changedRepos, baseRef) if (deltaChanges.length > 0) { results = await plugin.syncDelta(deltaChanges) } else { @@ -1499,17 +1500,18 @@ class Settings { /** * Build delta-based app changes from changed suborg/repo config files. - * Extracts app_installations from each changed config and computes - * repository_selection / repository_unselection per app. + * 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 = []) { + 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) - let installationMap = new Map() + const installationMap = new Map() if (appGithub && enterpriseSlug) { try { const enterpriseClient = new AppOctokitClient({ github: appGithub, enterpriseSlug, log: this.log }) @@ -1523,41 +1525,87 @@ class Settings { } } - // Process changed suborg configs - for (const suborg of changedSubOrgs) { - const suborgConfig = this.subOrgConfigs && this.subOrgConfigs[suborg.repo] - if (!suborgConfig || !suborgConfig.app_installations) continue + // Helper to ensure an entry exists in the change map + const ensureEntry = (slug) => { + if (!appChangeMap.has(slug)) { + const installationId = installationMap.get(slug) + if (!installationId) return null + appChangeMap.set(slug, { + app_slug: slug, + installation_id: installationId, + repository_selection: new Set(), + repository_unselection: new Set() + }) + } + return appChangeMap.get(slug) + } - // Resolve repos for this suborg's current targeting + // Helper to resolve repos for a suborg config's targeting criteria + const resolveSuborgRepos = async (config) => { + if (!config) return new Set() 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() + if (config.suborgrepos) criteria.names = config.suborgrepos + if (config.suborgteams) criteria.teams = config.suborgteams + if (config.suborgproperties) criteria.custom_properties = config.suborgproperties try { - suborgRepos = await repoSelector.resolve(criteria) + return await repoSelector.resolve(criteria) } catch (e) { - this.log.debug(`Error resolving suborg repos for '${suborg.repo}': ${e.message}`) + this.log.debug(`Error resolving suborg repos: ${e.message}`) + return new Set() + } + } + + // Process changed suborg configs + for (const suborg of changedSubOrgs) { + const currentConfig = this.subOrgConfigs && this.subOrgConfigs[suborg.repo] + 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}': ${e.message}`) + } } + const previousAppSlugs = new Set(previousApps.map(a => a.app_slug).filter(Boolean)) - for (const appConfig of suborgConfig.app_installations) { - const slug = appConfig.app_slug - if (!slug) continue - const installationId = installationMap.get(slug) - if (!installationId) continue - - if (!appChangeMap.has(slug)) { - appChangeMap.set(slug, { - app_slug: slug, - installation_id: installationId, - repository_selection: new Set(), - repository_unselection: new Set() - }) + // Resolve repos for current and previous targeting criteria + const currentRepos = await resolveSuborgRepos(currentConfig) + const previousRepos = await resolveSuborgRepos(previousConfig) + + // Apps present in current config: repos to add = currentRepos + for (const slug of currentAppSlugs) { + const entry = ensureEntry(slug) + if (!entry) continue + for (const repo of currentRepos) { + entry.repository_selection.add(repo) } - const change = appChangeMap.get(slug) - for (const repo of suborgRepos) { - change.repository_selection.add(repo) + } + + // Apps removed from config: repos to unselect = previousRepos + for (const slug of previousAppSlugs) { + if (currentAppSlugs.has(slug)) continue // still present + const entry = ensureEntry(slug) + if (!entry) continue + for (const repo of previousRepos) { + entry.repository_unselection.add(repo) + } + } + + // Apps still present but targeting changed: unselect repos no longer targeted + for (const slug of currentAppSlugs) { + if (!previousAppSlugs.has(slug)) continue // newly added, no unselection needed + const entry = ensureEntry(slug) + if (!entry) continue + for (const repo of previousRepos) { + if (!currentRepos.has(repo)) { + entry.repository_unselection.add(repo) + } } } } @@ -1566,27 +1614,52 @@ class Settings { for (const repo of changedRepos) { const repoConfig = this.repoConfigs && (this.repoConfigs[`${repo.repo}.yml`] || this.repoConfigs[`${repo.repo}.yaml`]) - if (!repoConfig || !repoConfig.app_installations) continue + const currentApps = (repoConfig && repoConfig.app_installations) || [] + const currentAppSlugs = new Set(currentApps.map(a => a.app_slug).filter(Boolean)) - for (const appConfig of repoConfig.app_installations) { - const slug = appConfig.app_slug - if (!slug) continue - const installationId = installationMap.get(slug) - if (!installationId) continue - - if (!appChangeMap.has(slug)) { - appChangeMap.set(slug, { - app_slug: slug, - installation_id: installationId, - repository_selection: new Set(), - repository_unselection: new Set() - }) + // Load previous version of this repo config + let previousApps = [] + if (baseRef) { + const repoFilePath = `repos/${repo.repo}.yml` + 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}`) } - appChangeMap.get(slug).repository_selection.add(repo.repo) + } + const previousAppSlugs = new Set(previousApps.map(a => a.app_slug).filter(Boolean)) + + // Apps present in current config: add this repo + for (const slug of currentAppSlugs) { + const entry = ensureEntry(slug) + if (!entry) continue + entry.repository_selection.add(repo.repo) + } + + // Apps removed from 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) } } - return [...appChangeMap.values()] + // 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 } /** From 4b2cec7a58b4e9b1081ffad1fceac670a5412151 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Thu, 25 Jun 2026 23:30:16 -0400 Subject: [PATCH 6/9] Fix delta bugs: org-all precedence + repo config path; update schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Delta mode now skips apps configured as repository_selection: all at org level (org 'all' takes precedence — never add/remove repos via delta) - Fix repo config path used to load previous version from baseRef: use CONFIG_PATH/repos/.yml instead of bare repos/.yml (the bare path 404'd, so repo-level repository_unselection was never computed) - schema/settings.json: add app_installations top-level property and include it in additive_plugins and disable_plugins enums for editor validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/settings.js | 18 +++++++++++++++++- schema/settings.json | 35 +++++++++++++++++++++++++++++++---- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/lib/settings.js b/lib/settings.js index 575ebb3d..fd944fa4 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -1525,8 +1525,24 @@ class Settings { } } + // 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) { + if (appConfig && appConfig.app_slug && appConfig.repository_selection === 'all') { + 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) return null @@ -1620,7 +1636,7 @@ class Settings { // Load previous version of this repo config let previousApps = [] if (baseRef) { - const repoFilePath = `repos/${repo.repo}.yml` + const repoFilePath = path.posix.join(CONFIG_PATH, 'repos', `${repo.repo}.yml`) try { const previousData = await this.loadYamlFromRef(repoFilePath, baseRef) previousApps = (previousData && previousData.app_installations) || [] diff --git a/schema/settings.json b/schema/settings.json index 57565fd8..0c349a5d 100644 --- a/schema/settings.json +++ b/schema/settings.json @@ -234,8 +234,32 @@ } } }, + "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 (repository_selection: all); 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." + }, + "repository_selection": { + "type": "string", + "enum": [ + "all" + ], + "description": "Only valid at the org level (settings.yml). 'all' selects every repository in the org and takes precedence over any suborg/repo-level selections." + } + } + } + }, "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 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). The app_installations plugin also honors additive mode (only adds repos to installations, never removes). Declare only in settings.yml (org level) to keep behavior consistent across all repos.", "type": "array", "items": { "type": "string", @@ -249,7 +273,8 @@ "custom_properties", "variables", "rulesets", - "custom_repository_roles" + "custom_repository_roles", + "app_installations" ] } }, @@ -274,7 +299,8 @@ "custom_properties", "custom_repository_roles", "variables", - "archive" + "archive", + "app_installations" ] }, { @@ -300,7 +326,8 @@ "custom_properties", "custom_repository_roles", "variables", - "archive" + "archive", + "app_installations" ] }, "target": { From b731d54aa54f9ac639c7a46a447dc90d103783a8 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Thu, 25 Jun 2026 23:41:17 -0400 Subject: [PATCH 7/9] Address app installation review: remove bad drift handler, fix ordering, docs - Remove installation.repositories_added/removed handler: an app only receives those events for its own installation, so they cannot detect drift on managed apps. Drift is reconciled by the scheduled full sync. - Process repository_unselection before repository_selection (delta and full sync) so a repo removed by one config and added by another ends up present - Add test asserting removal-before-addition ordering - Document app_installations in README (prerequisites, hierarchy, examples, sync behavior, drift note, disable/additive support) and add it to the configurable-items and disable_plugins lists Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 103 +++++++++++++++++- index.js | 32 ++---- lib/plugins/appInstallations.js | 28 ++--- .../unit/lib/plugins/appInstallations.test.js | 32 +++--- 4 files changed, 144 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index ff1f0237..86314547 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,105 @@ 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 (`repository_selection: all`) | +| Suborg | `suborgs/*.yml` | Repos matching the suborg's targeting (`suborgrepos`, `suborgteams`, `suborgproperties`) | +| Repo | `repos/.yml` | That specific repo | + +> [!important] +> An app configured with `repository_selection: all` at the **org** level 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 + repository_selection: all +``` + +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`). + Removals are applied before additions, 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 +780,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/index.js b/index.js index f8d2ffd5..3b512282 100644 --- a/index.js +++ b/index.js @@ -531,32 +531,16 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => }) // ──────────────────────────────────────────────────────────────────────── - // App installation drift detection handlers + // 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. // ──────────────────────────────────────────────────────────────────────── - const installation_change_events = [ - 'installation.repositories_added', - 'installation.repositories_removed' - ] - - robot.on(installation_change_events, async context => { - const { payload } = context - const { sender } = payload - robot.log.debug('App installation repos changed by ', JSON.stringify(sender)) - if (sender.type === 'Bot') { - robot.log.debug('App installation repos changed by Bot') - return - } - robot.log.debug('App installation repos changed by a Human — triggering sync to revert drift') - - // Build a context that targets the admin repo for this org - const orgLogin = payload.installation.account.login - const updatedContext = Object.assign({}, context, { - repo: () => { return { repo: env.ADMIN_REPO, owner: orgLogin } } - }) - return syncAllSettings(false, updatedContext) - }) - robot.on('installation_target', async context => { const { payload } = context const { sender } = payload diff --git a/lib/plugins/appInstallations.js b/lib/plugins/appInstallations.js index bdd03a19..8af81674 100644 --- a/lib/plugins/appInstallations.js +++ b/lib/plugins/appInstallations.js @@ -149,18 +149,20 @@ class AppInstallations { continue } - // Resolve names to IDs for additions (need to look up IDs) - if (toAdd.size > 0) { - const addIds = await this._resolveRepoIds([...toAdd]) - await this.enterpriseClient.addReposToInstallation(desired.installation_id, addIds) - this.log.debug(`App '${appSlug}': added ${addIds.length} repos`) - } - + // Resolve names to IDs. Process removals first, then additions, so a + // repo that should be both removed (by one config) and added (by + // another) ends up present. if (toRemove.size > 0) { const removeIds = [...toRemove].map(name => liveRepoMap.get(name)).filter(Boolean) await this.enterpriseClient.removeReposFromInstallation(desired.installation_id, removeIds) this.log.debug(`App '${appSlug}': removed ${removeIds.length} repos`) } + + if (toAdd.size > 0) { + const addIds = await this._resolveRepoIds([...toAdd]) + await this.enterpriseClient.addReposToInstallation(desired.installation_id, addIds) + this.log.debug(`App '${appSlug}': added ${addIds.length} repos`) + } } catch (e) { this.log.error(`Error in full sync for app '${appSlug}': ${e.message}`) this.errors.push({ @@ -254,18 +256,18 @@ class AppInstallations { return results } - if (hasSelections) { - const addIds = await this._resolveRepoIds([...repository_selection]) - await this.enterpriseClient.addReposToInstallation(installation_id, addIds) - this.log.debug(`App '${app_slug}': added ${addIds.length} repos`) - } - if (hasUnselections) { const removeIds = await this._resolveRepoIds([...repository_unselection]) await this.enterpriseClient.removeReposFromInstallation(installation_id, removeIds) this.log.debug(`App '${app_slug}': removed ${removeIds.length} repos`) } + if (hasSelections) { + const addIds = await this._resolveRepoIds([...repository_selection]) + await this.enterpriseClient.addReposToInstallation(installation_id, addIds) + this.log.debug(`App '${app_slug}': added ${addIds.length} repos`) + } + return results } diff --git a/test/unit/lib/plugins/appInstallations.test.js b/test/unit/lib/plugins/appInstallations.test.js index 739ca6f9..6e7333f1 100644 --- a/test/unit/lib/plugins/appInstallations.test.js +++ b/test/unit/lib/plugins/appInstallations.test.js @@ -107,26 +107,32 @@ describe('AppInstallations', () => { expect(result[0].action.deletions).toBeNull() }) - it('adds repos via enterprise client in non-nop mode', async () => { - github.repos.get - .mockResolvedValueOnce({ data: { id: 100 } }) - .mockResolvedValueOnce({ data: { id: 200 } }) + it('processes unselections before selections in non-nop mode', async () => { + // repo-a (add) resolves to id 100; repo-b (remove) resolves to id 200 + github.repos.get.mockImplementation(({ repo }) => { + if (repo === 'repo-a') return Promise.resolve({ data: { id: 100 } }) + if (repo === 'repo-b') return Promise.resolve({ data: { id: 200 } }) + return Promise.resolve({ data: { id: 0 } }) + }) + + const callOrder = [] + appGithub.request.mockImplementation((route) => { + if (route.startsWith('DELETE')) callOrder.push('remove') + if (route.startsWith('POST')) 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', 'repo-b']), - repository_unselection: new Set() + repository_selection: new Set(['repo-a']), + repository_unselection: new Set(['repo-b']) }]) - // Should have called request to add repos - expect(appGithub.request).toHaveBeenCalledWith( - expect.stringContaining('POST'), - expect.objectContaining({ - repository_ids: [100, 200] - }) - ) + // Removal must be applied before addition so a repo removed by one + // config and added by another ends up present. + expect(callOrder).toEqual(['remove', 'add']) }) }) From 9a25bdcebbf8d81cef8db70cc1a7d2d3c3e223ce Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Fri, 26 Jun 2026 00:01:45 -0400 Subject: [PATCH 8/9] Skip redundant app installation churn for unchanged delta MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In delta mode, when an app is present in both the previous and current suborg/repo config: - If suborg targeting is unchanged, skip the app entirely (no selection or unselection) — avoids re-adding all suborg repos on unrelated config edits - If targeting changed, emit only the diff (newly targeted repos to add, no-longer targeted repos to remove) instead of re-adding the full set - Repo-level: only select when the app is newly added to the repo config Add unit tests covering the skip, targeting-diff, and org-'all' precedence cases for _buildAppChangesFromDelta. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/settings.js | 34 ++++++----- test/unit/lib/settings.test.js | 102 +++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 14 deletions(-) diff --git a/lib/settings.js b/lib/settings.js index fd944fa4..59007161 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -1594,8 +1594,9 @@ class Settings { const currentRepos = await resolveSuborgRepos(currentConfig) const previousRepos = await resolveSuborgRepos(previousConfig) - // Apps present in current config: repos to add = currentRepos + // 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) { @@ -1603,9 +1604,9 @@ class Settings { } } - // Apps removed from config: repos to unselect = previousRepos + // App removed from this suborg: unselect all previously targeted repos for (const slug of previousAppSlugs) { - if (currentAppSlugs.has(slug)) continue // still present + if (currentAppSlugs.has(slug)) continue const entry = ensureEntry(slug) if (!entry) continue for (const repo of previousRepos) { @@ -1613,15 +1614,17 @@ class Settings { } } - // Apps still present but targeting changed: unselect repos no longer targeted - for (const slug of currentAppSlugs) { - if (!previousAppSlugs.has(slug)) continue // newly added, no unselection needed - const entry = ensureEntry(slug) - if (!entry) continue - for (const repo of previousRepos) { - if (!currentRepos.has(repo)) { - 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) } } } @@ -1646,14 +1649,17 @@ class Settings { } const previousAppSlugs = new Set(previousApps.map(a => a.app_slug).filter(Boolean)) - // Apps present in current config: add this repo + // 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) } - // Apps removed from config: unselect this repo + // App removed from this repo config: unselect this repo for (const slug of previousAppSlugs) { if (currentAppSlugs.has(slug)) continue const entry = ensureEntry(slug) diff --git a/test/unit/lib/settings.test.js b/test/unit/lib/settings.test.js index 85c66181..32fe1247 100644 --- a/test/unit/lib/settings.test.js +++ b/test/unit/lib/settings.test.js @@ -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 as repository_selection: all at org level', async () => { + jest.spyOn(RepoSelector.prototype, 'resolve').mockResolvedValue(new Set(['repo-a'])) + + settings.config = { + ...settings.config, + app_installations: [{ app_slug: 'my-app', repository_selection: 'all' }] + } + 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 From eacef7f5a723380204d062cafaf2dee5dbb9768a Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Fri, 26 Jun 2026 00:18:49 -0400 Subject: [PATCH 9/9] Use correct Enterprise Org Installations API (names, toggle, PATCH add/remove) Rewrite appOctokitClient to the documented 2026-03-10 endpoints: - org-scoped paths under /enterprises/{ent}/apps/organizations/{org}/... - repository NAMES instead of IDs - setRepositorySelection toggle for 'all'/'selected' - PATCH /add and PATCH /remove, batched at 50 Update appInstallations plugin to pass org, drop ID resolution and all-repo enumeration, use the toggle for 'all', and handle live current_selection (all<->selected transitions) in full sync. Thread current_selection through _computeFullAppDesiredState. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/appOctokitClient.js | 118 +++++++--- lib/plugins/appInstallations.js | 210 +++++++++--------- lib/settings.js | 8 + test/unit/lib/appOctokitClient.test.js | 72 ++++-- .../unit/lib/plugins/appInstallations.test.js | 56 ++++- 5 files changed, 299 insertions(+), 165 deletions(-) diff --git a/lib/appOctokitClient.js b/lib/appOctokitClient.js index dda22141..c888f97e 100644 --- a/lib/appOctokitClient.js +++ b/lib/appOctokitClient.js @@ -1,18 +1,27 @@ const BATCH_SIZE = 50 +const API_VERSION = '2026-03-10' /** - * AppOctokitClient wraps an Octokit client authenticated as the GitHub App - * (JWT) and provides methods for managing app installation repository access - * via the Enterprise Organization Installations API. + * 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 + * - 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 (via robot.auth()) + * @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 */ @@ -24,8 +33,9 @@ class AppOctokitClient { } /** - * List all app installations in the enterprise for a given org. - * Returns array of installation objects with { id, app_slug, app_id, ... } + * 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 @@ -33,17 +43,14 @@ class AppOctokitClient { async listOrgInstallations (org) { try { const options = this.github.request.endpoint.merge( - 'GET /enterprises/{enterprise}/apps/installations', + 'GET /enterprises/{enterprise}/apps/organizations/{org}/installations', { enterprise: this.enterpriseSlug, - headers: { 'X-GitHub-Api-Version': '2026-03-10' } + org, + headers: { 'X-GitHub-Api-Version': API_VERSION } } ) - const installations = await this.github.paginate(options) - // Filter to installations for the specified org - return installations.filter(i => - i.account && i.account.login === org - ) + return await this.github.paginate(options) } catch (e) { if (e.status === 403 || e.status === 404) { throw new Error( @@ -55,22 +62,25 @@ class AppOctokitClient { } /** - * List repositories accessible to an app installation. + * 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 (installationId) { + async listInstallationRepos (org, installationId) { try { const options = this.github.request.endpoint.merge( - 'GET /enterprises/{enterprise}/apps/installations/{installation_id}/repositories', + 'GET /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories', { enterprise: this.enterpriseSlug, + org, installation_id: installationId, - headers: { 'X-GitHub-Api-Version': '2026-03-10' } + headers: { 'X-GitHub-Api-Version': API_VERSION } } ) - return this.github.paginate(options) + return await this.github.paginate(options) } catch (e) { this.log.error(`Error listing repos for installation ${installationId}: ${e.message}`) throw e @@ -78,52 +88,86 @@ class AppOctokitClient { } /** - * Grant repository access to an app installation. + * 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 {number[]} repositoryIds - Array of repository IDs to add + * @param {string[]} repositoryNames - Repo names to add * @returns {Promise} */ - async addReposToInstallation (installationId, repositoryIds) { - if (!repositoryIds || repositoryIds.length === 0) return + async addReposToInstallation (org, installationId, repositoryNames) { + if (!repositoryNames || repositoryNames.length === 0) return - const batches = this._chunk(repositoryIds, BATCH_SIZE) - for (const batch of batches) { + for (const batch of this._chunk(repositoryNames, BATCH_SIZE)) { this.log.debug(`Adding ${batch.length} repos to installation ${installationId}`) await this.github.request( - 'POST /enterprises/{enterprise}/apps/installations/{installation_id}/repositories', + 'PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories/add', { enterprise: this.enterpriseSlug, + org, installation_id: installationId, - repository_ids: batch, - headers: { 'X-GitHub-Api-Version': '2026-03-10' } + repositories: batch, + headers: { 'X-GitHub-Api-Version': API_VERSION } } ) } } /** - * Remove repository access from an app installation. + * 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 {number[]} repositoryIds - Array of repository IDs to remove + * @param {string[]} repositoryNames - Repo names to remove * @returns {Promise} */ - async removeReposFromInstallation (installationId, repositoryIds) { - if (!repositoryIds || repositoryIds.length === 0) return + async removeReposFromInstallation (org, installationId, repositoryNames) { + if (!repositoryNames || repositoryNames.length === 0) return - const batches = this._chunk(repositoryIds, BATCH_SIZE) - for (const batch of batches) { + for (const batch of this._chunk(repositoryNames, BATCH_SIZE)) { this.log.debug(`Removing ${batch.length} repos from installation ${installationId}`) await this.github.request( - 'DELETE /enterprises/{enterprise}/apps/installations/{installation_id}/repositories', + 'PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories/remove', { enterprise: this.enterpriseSlug, + org, installation_id: installationId, - repository_ids: batch, - headers: { 'X-GitHub-Api-Version': '2026-03-10' } + repositories: batch, + headers: { 'X-GitHub-Api-Version': API_VERSION } } ) } diff --git a/lib/plugins/appInstallations.js b/lib/plugins/appInstallations.js index 8af81674..4e008a86 100644 --- a/lib/plugins/appInstallations.js +++ b/lib/plugins/appInstallations.js @@ -30,6 +30,7 @@ class AppInstallations { this.nop = nop this.github = github this.repo = repo + this.org = repo.owner this.log = log this.errors = errors || [] this.additive = false @@ -91,7 +92,9 @@ class AppInstallations { * 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' } + * @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) { @@ -110,59 +113,8 @@ class AppInstallations { for (const [appSlug, desired] of Object.entries(desiredState)) { try { - // Get live state - const liveRepos = await this.enterpriseClient.listInstallationRepos(desired.installation_id) - const liveRepoNames = new Set(liveRepos.map(r => r.name)) - const liveRepoMap = new Map(liveRepos.map(r => [r.name, r.id])) - - let desiredRepoNames - if (desired.repos === 'all') { - // Desired = all repos in org - desiredRepoNames = await this._getAllRepoNames() - } else { - desiredRepoNames = desired.repos - } - - // Compute diff - const toAdd = new Set([...desiredRepoNames].filter(r => !liveRepoNames.has(r))) - const toRemove = this.additive - ? new Set() // Additive mode: never remove - : new Set([...liveRepoNames].filter(r => !desiredRepoNames.has(r))) - - if (toAdd.size === 0 && toRemove.size === 0) { - this.log.debug(`App '${appSlug}': no changes needed`) - continue - } - - if (this.nop) { - results.push(new NopCommand( - 'app_installations', - this.repo, - null, - { - msg: `App '${appSlug}' installation repos`, - additions: toAdd.size > 0 ? [...toAdd] : null, - modifications: null, - deletions: toRemove.size > 0 ? [...toRemove] : null - } - )) - continue - } - - // Resolve names to IDs. Process removals first, then additions, so a - // repo that should be both removed (by one config) and added (by - // another) ends up present. - if (toRemove.size > 0) { - const removeIds = [...toRemove].map(name => liveRepoMap.get(name)).filter(Boolean) - await this.enterpriseClient.removeReposFromInstallation(desired.installation_id, removeIds) - this.log.debug(`App '${appSlug}': removed ${removeIds.length} repos`) - } - - if (toAdd.size > 0) { - const addIds = await this._resolveRepoIds([...toAdd]) - await this.enterpriseClient.addReposToInstallation(desired.installation_id, addIds) - this.log.debug(`App '${appSlug}': added ${addIds.length} repos`) - } + 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({ @@ -180,6 +132,101 @@ class AppInstallations { 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 + })) + 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) { + // Cannot set 'selected' with an empty list; nothing safe to do here. + this.log.debug(`App '${appSlug}': desired set is empty, leaving 'all' selection untouched`) + 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)'] + })) + 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 + } + + 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 + })) + return results + } + + // Process removals first, then additions, so a repo that should be both + // removed (by one config) and added (by another) ends up present. + if (toRemove.length > 0) { + await this.enterpriseClient.removeReposFromInstallation(this.org, installation_id, toRemove) + this.log.debug(`App '${appSlug}': removed ${toRemove.length} repos`) + } + if (toAdd.length > 0) { + await this.enterpriseClient.addReposToInstallation(this.org, installation_id, toAdd) + this.log.debug(`App '${appSlug}': added ${toAdd.length} repos`) + } + + return results + } + /** * Process a single app's delta change. * @private @@ -206,7 +253,7 @@ class AppInstallations { return results } - // Handle "all" selection — set repository_selection to all via the API + // Handle "all" selection — toggle the installation to 'all' via the API if (repository_selection === 'all') { if (this.nop) { results.push(new NopCommand( @@ -223,18 +270,8 @@ class AppInstallations { return results } - // For "all" repos, get the full list and add all - const allRepoNames = await this._getAllRepoNames() - const liveRepos = await this.enterpriseClient.listInstallationRepos(installation_id) - const liveRepoNames = new Set(liveRepos.map(r => r.name)) - const toAdd = [...allRepoNames].filter(r => !liveRepoNames.has(r)) - - if (toAdd.length > 0) { - const addIds = await this._resolveRepoIds(toAdd) - await this.enterpriseClient.addReposToInstallation(installation_id, addIds) - this.log.debug(`App '${app_slug}': added all repos (${addIds.length} new)`) - } - + await this.enterpriseClient.setRepositorySelection(this.org, installation_id, 'all') + this.log.debug(`App '${app_slug}': set repository_selection to 'all'`) return results } @@ -257,48 +294,17 @@ class AppInstallations { } if (hasUnselections) { - const removeIds = await this._resolveRepoIds([...repository_unselection]) - await this.enterpriseClient.removeReposFromInstallation(installation_id, removeIds) - this.log.debug(`App '${app_slug}': removed ${removeIds.length} repos`) + await this.enterpriseClient.removeReposFromInstallation(this.org, installation_id, [...repository_unselection]) + this.log.debug(`App '${app_slug}': removed ${repository_unselection.size} repos`) } if (hasSelections) { - const addIds = await this._resolveRepoIds([...repository_selection]) - await this.enterpriseClient.addReposToInstallation(installation_id, addIds) - this.log.debug(`App '${app_slug}': added ${addIds.length} repos`) + await this.enterpriseClient.addReposToInstallation(this.org, installation_id, [...repository_selection]) + this.log.debug(`App '${app_slug}': added ${repository_selection.size} repos`) } return results } - - /** - * Get all repo names visible to the installation. - * @private - */ - async _getAllRepoNames () { - const repos = await this.github.paginate('GET /installation/repositories') - return new Set(repos.map(r => r.name)) - } - - /** - * Resolve repo names to IDs. - * @private - */ - async _resolveRepoIds (repoNames) { - const ids = [] - for (const name of repoNames) { - try { - const { data } = await this.github.repos.get({ - owner: this.repo.owner, - repo: name - }) - ids.push(data.id) - } catch (e) { - this.log.debug(`Could not resolve repo ID for '${name}': ${e.message}`) - } - } - return ids - } } module.exports = AppInstallations diff --git a/lib/settings.js b/lib/settings.js index 59007161..1e3fb80d 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -1708,8 +1708,10 @@ class Settings { } 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 @@ -1785,6 +1787,12 @@ class Settings { } } + // 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 } diff --git a/test/unit/lib/appOctokitClient.test.js b/test/unit/lib/appOctokitClient.test.js index fdd37517..dd7206f6 100644 --- a/test/unit/lib/appOctokitClient.test.js +++ b/test/unit/lib/appOctokitClient.test.js @@ -27,17 +27,19 @@ describe('AppOctokitClient', () => { }) describe('listOrgInstallations', () => { - it('returns installations filtered by org', async () => { + it('returns org installations', async () => { github.paginate.mockResolvedValue([ - { id: 1, app_slug: 'app-a', account: { login: 'my-org' } }, - { id: 2, app_slug: 'app-b', account: { login: 'other-org' } }, - { id: 3, app_slug: 'app-c', account: { login: 'my-org' } } + { 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(result[1].app_slug).toBe('app-c') + 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 () => { @@ -55,41 +57,77 @@ describe('AppOctokitClient', () => { }) }) + 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(123, []) + await client.addReposToInstallation('my-org', 123, []) expect(github.request).not.toHaveBeenCalled() }) - it('sends single batch for <= 50 repos', async () => { - const ids = Array.from({ length: 10 }, (_, i) => i + 1) - await client.addReposToInstallation(123, ids) + 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( - expect.stringContaining('POST'), + 'PATCH /enterprises/{enterprise}/apps/organizations/{org}/installations/{installation_id}/repositories/add', expect.objectContaining({ - repository_ids: ids, - installation_id: 123 + repositories: names, + installation_id: 123, + org: 'my-org' }) ) }) it('batches into chunks of 50', async () => { - const ids = Array.from({ length: 120 }, (_, i) => i + 1) - await client.addReposToInstallation(123, ids) + 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(123, []) + 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 ids = Array.from({ length: 75 }, (_, i) => i + 1) - await client.removeReposFromInstallation(123, ids) + const names = Array.from({ length: 75 }, (_, i) => `repo-${i}`) + await client.removeReposFromInstallation('my-org', 123, names) expect(github.request).toHaveBeenCalledTimes(2) // 50 + 25 }) }) diff --git a/test/unit/lib/plugins/appInstallations.test.js b/test/unit/lib/plugins/appInstallations.test.js index 6e7333f1..a69c23eb 100644 --- a/test/unit/lib/plugins/appInstallations.test.js +++ b/test/unit/lib/plugins/appInstallations.test.js @@ -108,17 +108,10 @@ describe('AppInstallations', () => { }) it('processes unselections before selections in non-nop mode', async () => { - // repo-a (add) resolves to id 100; repo-b (remove) resolves to id 200 - github.repos.get.mockImplementation(({ repo }) => { - if (repo === 'repo-a') return Promise.resolve({ data: { id: 100 } }) - if (repo === 'repo-b') return Promise.resolve({ data: { id: 200 } }) - return Promise.resolve({ data: { id: 0 } }) - }) - const callOrder = [] appGithub.request.mockImplementation((route) => { - if (route.startsWith('DELETE')) callOrder.push('remove') - if (route.startsWith('POST')) callOrder.push('add') + if (route.includes('/repositories/remove')) callOrder.push('remove') + if (route.includes('/repositories/add')) callOrder.push('add') return Promise.resolve({ data: {} }) }) @@ -208,5 +201,50 @@ describe('AppInstallations', () => { 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() + }) }) })