From de8bab4af871226997265ab3cba87b61aad0a504 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Fri, 3 Oct 2025 17:03:28 -0400 Subject: [PATCH 01/17] handle multiple changes as a batch --- index.js | 56 +++++++++++++++++++------------------------------ lib/settings.js | 38 ++++++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 40 deletions(-) diff --git a/index.js b/index.js index e6fd1c8d7..ebb1ff929 100644 --- a/index.js +++ b/index.js @@ -40,7 +40,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } } - async function syncSubOrgSettings (nop, context, suborg, repo = context.repo(), ref) { + async function syncSettings (nop, context, repo = context.repo(), ref) { try { deploymentConfig = await loadYamlFileSystem() robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`) @@ -48,7 +48,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => const runtimeConfig = await configManager.loadGlobalSettingsYaml() const config = Object.assign({}, deploymentConfig, runtimeConfig) robot.log.debug(`config for ref ${ref} is ${JSON.stringify(config)}`) - return Settings.syncSubOrgs(nop, context, suborg, repo, config, ref) + return Settings.sync(nop, context, repo, config, ref) } catch (e) { if (nop) { let filename = env.SETTINGS_FILE_PATH @@ -65,7 +65,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } } - async function syncSettings (nop, context, repo = context.repo(), ref) { + async function syncSelectedSettings (nop, context, repos, subOrgs, ref) { try { deploymentConfig = await loadYamlFileSystem() robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`) @@ -73,7 +73,11 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => const runtimeConfig = await configManager.loadGlobalSettingsYaml() const config = Object.assign({}, deploymentConfig, runtimeConfig) robot.log.debug(`config for ref ${ref} is ${JSON.stringify(config)}`) - return Settings.sync(nop, context, repo, config, ref) + if (ref) { + return Settings.syncSelectedRepos(nop, context, repos, subOrgs, config, ref) + } else { + return Settings.syncSelectedRepos(nop, context, repos, subOrgs, config) + } } catch (e) { if (nop) { let filename = env.SETTINGS_FILE_PATH @@ -81,9 +85,9 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => filename = env.DEPLOYMENT_CONFIG_FILE_PATH deploymentConfig = {} } - const nopcommand = new NopCommand(filename, repo, null, e, 'ERROR') + const nopcommand = new NopCommand(filename, context.repo(), null, e, 'ERROR') robot.log.error(`NOPCOMMAND ${JSON.stringify(nopcommand)}`) - Settings.handleError(nop, context, repo, deploymentConfig, ref, nopcommand) + Settings.handleError(nop, context, context.repo(), deploymentConfig, ref, nopcommand) } else { throw e } @@ -264,17 +268,11 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } const repoChanges = getAllChangedRepoConfigs(payload, context.repo().owner) - if (repoChanges.length > 0) { - return Promise.all(repoChanges.map(repo => { - return syncSettings(false, context, repo) - })) - } - const changes = getAllChangedSubOrgConfigs(payload) - if (changes.length) { - return Promise.all(changes.map(suborg => { - return syncSubOrgSettings(false, context, suborg) - })) + const subOrgChanges = getAllChangedSubOrgConfigs(payload) + + if (repoChanges.length > 0 || subOrgChanges.length > 0) { + return syncSelectedSettings(false, context, repoChanges, subOrgChanges) } robot.log.debug(`No changes in '${Settings.FILE_PATH}' detected, returning...`) @@ -572,15 +570,10 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => robot.log.debug(`Updating check run ${JSON.stringify(params)}`) await context.octokit.checks.update(params) - // guarding against null value from upstream libary that is - // causing a 404 and the check to stall - // from issue: https://github.com/github/safe-settings/issues/185#issuecomment-1075240374 - if (check_suite.before === '0000000000000000000000000000000000000000') { - check_suite.before = check_suite.pull_requests[0].base.sha - } - params = Object.assign(context.repo(), { basehead: `${check_suite.before}...${check_suite.after}` }) - const changes = await context.octokit.repos.compareCommitsWithBasehead(params) - const files = changes.data.files.map(f => { return f.filename }) + params = Object.assign(context.repo(), { pull_number: pull_request.number }) + + const changes = await context.octokit.request('GET /repos/{owner}/{repo}/pulls/{pull_number}/files', params) + const files = changes.data.map(f => { return f.filename }) const settingsModified = files.includes(Settings.FILE_PATH) @@ -590,17 +583,10 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } const repoChanges = getChangedRepoConfigName(files, context.repo().owner) - if (repoChanges.length > 0) { - return Promise.all(repoChanges.map(repo => { - return syncSettings(true, context, repo, pull_request.head.ref) - })) - } - const subOrgChanges = getChangedSubOrgConfigName(files) - if (subOrgChanges.length) { - return Promise.all(subOrgChanges.map(suborg => { - return syncSubOrgSettings(true, context, suborg, context.repo(), pull_request.head.ref) - })) + + if (repoChanges.length > 0 || subOrgChanges.length > 0) { + return syncSelectedSettings(true, context, repoChanges, subOrgChanges, pull_request.head.ref) } // if no safe-settings changes detected, send a success to the check run diff --git a/lib/settings.js b/lib/settings.js index 6c42e439b..8d9e07b2b 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -42,6 +42,31 @@ class Settings { } } + static async syncSelectedRepos (nop, context, repos, subOrgs, config, ref) { + const settings = new Settings(nop, context, context.repo(), config, ref) + + try { + for (const repo of repos) { + settings.repo = repo + await settings.loadConfigs(repo) + if (settings.isRestricted(repo.repo)) { + continue + } + await settings.updateRepos(repo) + } + for (const suborg of subOrgs) { + settings.subOrgConfigMap = [suborg] + settings.suborgChange = !!suborg + await settings.loadConfigs() + await settings.updateAll() + } + await settings.handleResults() + } catch (error) { + settings.logError(error.message) + await settings.handleResults() + } + } + static async sync (nop, context, repo, config, ref) { const settings = new Settings(nop, context, repo, config, ref) try { @@ -506,17 +531,20 @@ ${this.results.reduce((x, y) => { log.debug('Fetching repositories') return github.paginate('GET /installation/repositories').then(repositories => { return Promise.all(repositories.map(repository => { - if (this.isRestricted(repository.name)) { - return null - } - const { owner, name } = repository - return this.updateRepos({ owner: owner.login, repo: name }) + return this.checkAndProcessRepo(owner.login, name) }) ) }) } + async checkAndProcessRepo (owner, name) { + if (this.isRestricted(name)) { + return null + } + return this.updateRepos({ owner, repo: name }) + } + /** * Loads a file from GitHub * From fa00d78c38df52ba66203e254fac623b0b2a2b46 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Fri, 3 Oct 2025 17:07:36 -0400 Subject: [PATCH 02/17] Update index.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index ebb1ff929..608a2b8a8 100644 --- a/index.js +++ b/index.js @@ -572,7 +572,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => params = Object.assign(context.repo(), { pull_number: pull_request.number }) - const changes = await context.octokit.request('GET /repos/{owner}/{repo}/pulls/{pull_number}/files', params) + const changes = await context.octokit.pulls.listFiles(params) const files = changes.data.map(f => { return f.filename }) const settingsModified = files.includes(Settings.FILE_PATH) From b87397cf4579da0ff4da07cb3bf24b2be535b10a Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Fri, 3 Oct 2025 17:08:05 -0400 Subject: [PATCH 03/17] Update index.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- index.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/index.js b/index.js index 608a2b8a8..57312b710 100644 --- a/index.js +++ b/index.js @@ -73,11 +73,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => const runtimeConfig = await configManager.loadGlobalSettingsYaml() const config = Object.assign({}, deploymentConfig, runtimeConfig) robot.log.debug(`config for ref ${ref} is ${JSON.stringify(config)}`) - if (ref) { - return Settings.syncSelectedRepos(nop, context, repos, subOrgs, config, ref) - } else { - return Settings.syncSelectedRepos(nop, context, repos, subOrgs, config) - } + return Settings.syncSelectedRepos(nop, context, repos, subOrgs, config, ref) } catch (e) { if (nop) { let filename = env.SETTINGS_FILE_PATH From 6b358e5b81ecd35f103490ee44cba660a73f2f4f Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Sat, 4 Oct 2025 20:45:54 -0400 Subject: [PATCH 04/17] depup files in a push --- index.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 57312b710..3448df382 100644 --- a/index.js +++ b/index.js @@ -263,9 +263,19 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => return syncAllSettings(false, context) } - const repoChanges = getAllChangedRepoConfigs(payload, context.repo().owner) + let repoChanges = getAllChangedRepoConfigs(payload, context.repo().owner) - const subOrgChanges = getAllChangedSubOrgConfigs(payload) + let subOrgChanges = getAllChangedSubOrgConfigs(payload) + const dedupedRepos = [...new Set(repoChanges.map(r => r.repo))].map(name => { + return repoChanges.find(r => r.repo === name) + }) + repoChanges = dedupedRepos + const dedupedSubOrgs = [...new Set(subOrgChanges.map(s => s.name))].map(name => { + return subOrgChanges.find(s => s.name === name) + }) + subOrgChanges = dedupedSubOrgs + robot.log.debug(`deduped repos ${JSON.stringify(repoChanges)}`) + robot.log.debug(`deduped subOrgs ${JSON.stringify(subOrgChanges)}`) if (repoChanges.length > 0 || subOrgChanges.length > 0) { return syncSelectedSettings(false, context, repoChanges, subOrgChanges) From c971041333ac4ea6e226869bea1f10017c565220 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Sat, 4 Oct 2025 20:49:28 -0400 Subject: [PATCH 05/17] Update index.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 3448df382..3c54e8154 100644 --- a/index.js +++ b/index.js @@ -270,8 +270,8 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => return repoChanges.find(r => r.repo === name) }) repoChanges = dedupedRepos - const dedupedSubOrgs = [...new Set(subOrgChanges.map(s => s.name))].map(name => { - return subOrgChanges.find(s => s.name === name) + const dedupedSubOrgs = [...new Set(subOrgChanges.map(s => s.repo))].map(repo => { + return subOrgChanges.find(s => s.repo === repo) }) subOrgChanges = dedupedSubOrgs robot.log.debug(`deduped repos ${JSON.stringify(repoChanges)}`) From ac8e195dd999e40bde5933ca1de6480001fb07ce Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Sun, 5 Oct 2025 09:12:26 -0400 Subject: [PATCH 06/17] moved the dedup logic --- index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 3c54e8154..c757da2b1 100644 --- a/index.js +++ b/index.js @@ -266,8 +266,8 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => let repoChanges = getAllChangedRepoConfigs(payload, context.repo().owner) let subOrgChanges = getAllChangedSubOrgConfigs(payload) - const dedupedRepos = [...new Set(repoChanges.map(r => r.repo))].map(name => { - return repoChanges.find(r => r.repo === name) + const dedupedRepos = [...new Set(repoChanges.map(r => r.repo))].map(repo => { + return repoChanges.find(r => r.repo === repo) }) repoChanges = dedupedRepos const dedupedSubOrgs = [...new Set(subOrgChanges.map(s => s.repo))].map(repo => { From 8bc76fcf92e9cd1885105ce32773b2779bd8f75a Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Sun, 5 Oct 2025 09:14:47 -0400 Subject: [PATCH 07/17] Update index.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- index.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/index.js b/index.js index c757da2b1..ca2f6dc61 100644 --- a/index.js +++ b/index.js @@ -266,14 +266,9 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => let repoChanges = getAllChangedRepoConfigs(payload, context.repo().owner) let subOrgChanges = getAllChangedSubOrgConfigs(payload) - const dedupedRepos = [...new Set(repoChanges.map(r => r.repo))].map(repo => { - return repoChanges.find(r => r.repo === repo) - }) - repoChanges = dedupedRepos - const dedupedSubOrgs = [...new Set(subOrgChanges.map(s => s.repo))].map(repo => { - return subOrgChanges.find(s => s.repo === repo) - }) - subOrgChanges = dedupedSubOrgs + repoChanges = repoChanges.filter((r, i, arr) => arr.findIndex(item => item.repo === r.repo) === i) + + subOrgChanges = subOrgChanges.filter((s, i, arr) => arr.findIndex(item => item.repo === s.repo) === i) robot.log.debug(`deduped repos ${JSON.stringify(repoChanges)}`) robot.log.debug(`deduped subOrgs ${JSON.stringify(subOrgChanges)}`) From b6887a2a1704512969a6b34e626d963698ab3c17 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Fri, 15 May 2026 10:53:37 -0400 Subject: [PATCH 08/17] Start at 2.1.18-rc1 and add roles plugin and enhance settings integration --- lib/plugins/custom_properties.js | 50 ++++++++--- lib/plugins/custom_repository_roles.js | 119 +++++++++++++++++++++++++ lib/settings.js | 12 ++- 3 files changed, 169 insertions(+), 12 deletions(-) create mode 100644 lib/plugins/custom_repository_roles.js diff --git a/lib/plugins/custom_properties.js b/lib/plugins/custom_properties.js index 6b1f3ab36..35f0144da 100644 --- a/lib/plugins/custom_properties.js +++ b/lib/plugins/custom_properties.js @@ -12,10 +12,24 @@ module.exports = class CustomProperties extends Diffable { // Force all names to lowercase to avoid comparison issues. normalizeEntries () { - this.entries = this.entries.map(({ name, value }) => ({ - name: name.toLowerCase(), - value - })) + this.entries = this.entries.reduce((normalizedEntries, entry) => { + if (!entry || typeof entry !== 'object') { + return normalizedEntries + } + + const entryName = entry.name || entry.property_name + + if (typeof entryName !== 'string') { + return normalizedEntries + } + + normalizedEntries.push({ + name: entryName.toLowerCase(), + value: entry.value + }) + + return normalizedEntries + }, []) } async find () { @@ -25,7 +39,7 @@ module.exports = class CustomProperties extends Diffable { this.log.debug(`Getting all custom properties for the repo ${repoFullName}`) const customProperties = await this.github.paginate( - this.github.repos.getCustomPropertiesValues, + this.github.rest.repos.getCustomPropertiesValues, { owner, repo, @@ -38,10 +52,24 @@ module.exports = class CustomProperties extends Diffable { // Force all names to lowercase to avoid comparison issues. normalize (properties) { - return properties.map(({ property_name: propertyName, value }) => ({ - name: propertyName.toLowerCase(), - value - })) + return properties.reduce((normalizedProperties, property) => { + if (!property || typeof property !== 'object') { + return normalizedProperties + } + + const propertyName = property.property_name || property.name + + if (typeof propertyName !== 'string') { + return normalizedProperties + } + + normalizedProperties.push({ + name: propertyName.toLowerCase(), + value: property.value + }) + + return normalizedProperties + }, []) } comparator (existing, attrs) { @@ -82,14 +110,14 @@ module.exports = class CustomProperties extends Diffable { return new NopCommand( this.constructor.name, this.repo, - this.github.repos.createOrUpdateCustomPropertiesValues.endpoint(params), + this.github.rest.repos.createOrUpdateCustomPropertiesValues.endpoint(params), `${operation} Custom Property` ) } try { this.log.debug(`${operation} Custom Property "${name}" for the repo ${repoFullName}`) - await this.github.repos.createOrUpdateCustomPropertiesValues(params) + await this.github.rest.repos.createOrUpdateCustomPropertiesValues(params) this.log.debug(`Successfully ${operation.toLowerCase()}d Custom Property "${name}" for the repo ${repoFullName}`) } catch (e) { this.logError(`Error during ${operation} Custom Property "${name}" for the repo ${repoFullName}: ${e.message || e}`) diff --git a/lib/plugins/custom_repository_roles.js b/lib/plugins/custom_repository_roles.js new file mode 100644 index 000000000..1931b47cc --- /dev/null +++ b/lib/plugins/custom_repository_roles.js @@ -0,0 +1,119 @@ +const Diffable = require('./diffable') +const NopCommand = require('../nopcommand') +const MergeDeep = require('../mergeDeep') + +// Fields returned by the API that we should ignore when diffing +const ignorableFields = ['id', 'organization', 'created_at', 'updated_at'] + +const version = { + 'X-GitHub-Api-Version': '2026-03-10' +} + +module.exports = class CustomRepositoryRoles extends Diffable { + constructor (nop, github, repo, entries, log, errors) { + super(nop, github, repo, entries, log, errors) + this.github = github + this.repo = repo + this.entries = entries + this.log = log + this.nop = nop + } + + // Find all Custom Repository Roles for the org + find () { + this.log.debug(`Getting all custom repository roles for the org ${this.repo.owner}`) + + return this.github.request('GET /orgs/{org}/custom-repository-roles', { + org: this.repo.owner, + headers: version + }).then(res => { + const roles = (res && res.data && res.data.custom_roles) || [] + // Strip noise so deep-diff focuses on the configurable fields + return roles.map(r => ({ + id: r.id, + name: r.name, + description: r.description, + base_role: r.base_role, + permissions: r.permissions + })) + }).catch(e => { + return this.handleError(e, []) + }) + } + + comparator (existing, attrs) { + return existing.name === attrs.name + } + + changed (existing, attrs) { + const mergeDeep = new MergeDeep(this.log, this.github, ignorableFields) + const merged = mergeDeep.compareDeep(existing, attrs) + return merged.hasChanges + } + + update (existing, attrs) { + const parms = this.wrapAttrs(Object.assign({ role_id: existing.id }, attrs)) + if (this.nop) { + return Promise.resolve([ + new NopCommand(this.constructor.name, this.repo, this.github.request.endpoint('PATCH /orgs/{org}/custom-repository-roles/{role_id}', parms), 'Update Custom Repository Role') + ]) + } + this.log.debug(`Updating Custom Repository Role with the following values ${JSON.stringify(parms, null, 2)}`) + return this.github.request('PATCH /orgs/{org}/custom-repository-roles/{role_id}', parms).then(res => { + this.log.debug(`Custom Repository Role updated successfully ${JSON.stringify(res.url)}`) + return res + }).catch(e => { + return this.handleError(e) + }) + } + + add (attrs) { + const parms = this.wrapAttrs(attrs) + if (this.nop) { + return Promise.resolve([ + new NopCommand(this.constructor.name, this.repo, this.github.request.endpoint('POST /orgs/{org}/custom-repository-roles', parms), 'Create Custom Repository Role') + ]) + } + this.log.debug(`Creating Custom Repository Role with the following values ${JSON.stringify(parms, null, 2)}`) + return this.github.request('POST /orgs/{org}/custom-repository-roles', parms).then(res => { + this.log.debug(`Custom Repository Role created successfully ${JSON.stringify(res.url)}`) + return res + }).catch(e => { + return this.handleError(e) + }) + } + + remove (existing) { + const parms = this.wrapAttrs({ role_id: existing.id }) + if (this.nop) { + return Promise.resolve([ + new NopCommand(this.constructor.name, this.repo, this.github.request.endpoint('DELETE /orgs/{org}/custom-repository-roles/{role_id}', parms), 'Delete Custom Repository Role') + ]) + } + this.log.debug(`Deleting Custom Repository Role with the following values ${JSON.stringify(parms, null, 2)}`) + return this.github.request('DELETE /orgs/{org}/custom-repository-roles/{role_id}', parms).then(res => { + this.log.debug(`Custom Repository Role deleted successfully ${JSON.stringify(res.url)}`) + return res + }).catch(e => { + if (e.status === 404) { + return + } + return this.handleError(e) + }) + } + + wrapAttrs (attrs) { + return Object.assign({}, attrs, { + org: this.repo.owner, + headers: version + }) + } + + handleError (e, returnValue) { + this.logError(e) + if (this.nop) { + return Promise.resolve([(new NopCommand(this.constructor.name, this.repo, null, `error: ${e}`, 'ERROR'))]) + } + return Promise.resolve(returnValue) + } +} diff --git a/lib/settings.js b/lib/settings.js index 8d9e07b2b..4815e5a6b 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -319,7 +319,15 @@ ${this.results.reduce((x, y) => { const rulesetsConfig = this.config.rulesets if (rulesetsConfig) { const RulesetsPlugin = Settings.PLUGINS.rulesets - return new RulesetsPlugin(this.nop, this.github, this.repo, rulesetsConfig, this.log, this.errors, SCOPE.ORG).sync().then(res => { + await new RulesetsPlugin(this.nop, this.github, this.repo, rulesetsConfig, this.log, this.errors, SCOPE.ORG).sync().then(res => { + this.appendToResults(res) + }) + } + + const customRepositoryRolesConfig = this.config.custom_repository_roles + if (customRepositoryRolesConfig) { + const CustomRepositoryRolesPlugin = Settings.PLUGINS.custom_repository_roles + await new CustomRepositoryRolesPlugin(this.nop, this.github, this.repo, customRepositoryRolesConfig, this.log, this.errors).sync().then(res => { this.appendToResults(res) }) } @@ -434,6 +442,7 @@ ${this.results.reduce((x, y) => { returnRepoSpecificConfigs (config) { const newConfig = Object.assign({}, config) // clone delete newConfig.rulesets + delete newConfig.custom_repository_roles return newConfig } @@ -1004,6 +1013,7 @@ Settings.PLUGINS = { rulesets: require('./plugins/rulesets'), environments: require('./plugins/environments'), custom_properties: require('./plugins/custom_properties.js'), + custom_repository_roles: require('./plugins/custom_repository_roles'), variables: require('./plugins/variables') } From bdcc6b57eae13dc3474b24ed47ffd0fcb329f988 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Fri, 15 May 2026 10:54:07 -0400 Subject: [PATCH 09/17] Add custom repository roles schema to settings.json --- schema/settings.json | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/schema/settings.json b/schema/settings.json index 4d390b38f..3649d88ea 100644 --- a/schema/settings.json +++ b/schema/settings.json @@ -191,6 +191,48 @@ "items": { "$ref": "https://raw.githubusercontent.com/github/rest-api-description/main/descriptions/api.github.com/api.github.com.2022-11-28.json#/paths/~1orgs~1{org}~1rulesets/post/requestBody/content/application~1json/schema" } + }, + "custom_repository_roles": { + "description": "Org-level custom repository roles. Only valid in the org-level settings.yml.", + "type": "array", + "items": { + "type": "object", + "required": [ + "name", + "base_role", + "permissions" + ], + "properties": { + "name": { + "type": "string", + "description": "The name of the custom role." + }, + "description": { + "type": [ + "string", + "null" + ], + "description": "A short description of the role." + }, + "base_role": { + "type": "string", + "enum": [ + "read", + "triage", + "write", + "maintain" + ], + "description": "The system role from which this role inherits permissions." + }, + "permissions": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Additional fine-grained permissions included in this role." + } + } + } } } -} +} \ No newline at end of file From 1d739f9f8cfa8bb4d72c8a53d779e52cf169cc58 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Tue, 19 May 2026 01:59:10 -0400 Subject: [PATCH 10/17] Add sub-org reevaluation logic and smoke tests --- README.md | 87 +++ lib/plugins/diffable.js | 6 + lib/plugins/repository.js | 12 + lib/settings.js | 176 +++++- package.json | 3 +- smoke-test.js | 945 +++++++++++++++++++++++++++++++++ test/unit/lib/settings.test.js | 181 +++++++ 7 files changed, 1405 insertions(+), 5 deletions(-) create mode 100644 smoke-test.js diff --git a/README.md b/README.md index 07a626748..b7382de34 100644 --- a/README.md +++ b/README.md @@ -178,6 +178,27 @@ The App listens to the following webhook events: - __custom_property_values__: If new repository properties are set for a repository, `safe-settings` will run to so that if a sub-org config is defined by that property, it will be applied for the repo +### Suborg re-evaluation after repo-level changes + +A repo's suborg membership can depend on state that is itself written by `safe-settings`: + +- `suborgteams` — repos belong to a suborg because a given team is granted access +- `suborgproperties` — repos belong to a suborg because a custom property has a given value +- `suborgrepos` — repos belong to a suborg because their name matches a glob + +When a repo-level change (a push to `.github/repos/.yml`, or a `repository.created` event for a brand-new repo) adds a team, sets a custom property, or creates a repo whose name matches a suborg's `suborgrepos` glob, the repo may *newly* match a suborg config that was not applied in the first pass. + +To handle this, after applying a repo-yml change `safe-settings` re-evaluates the repo's suborg membership and, if a new suborg now matches, runs the repo through the apply pipeline a second time so the suborg's settings are picked up in the same sync. + +**Scope:** Re-evaluation runs only on the repo-yml change paths (`Settings.sync` and the per-repo loop of `Settings.syncSelectedRepos`). Global settings changes (`syncAll`) and suborg-yml changes (`syncSubOrgs`) already iterate all relevant repos and do not need it. + +**Loop prevention.** Two guards prevent infinite re-evaluation: + +1. **Stability check (primary):** Before applying changes, `safe-settings` snapshots the set of suborg source paths that match the repo. After applying, it refreshes the suborg cache and recomputes the set. If no new suborg source appeared, re-evaluation stops. +2. **Hard depth cap (safety net):** Each repo is re-evaluated at most `MAX_REEVALUATION_DEPTH = 1` time per sync. This resolves the dominant single-hop case (repo change → newly-matched suborg → apply suborg once) while preventing pathological chains (suborg A applies a team that activates suborg B that activates suborg C…). Chains beyond one hop are resolved on the next sync event, and a warning is logged when the cap is hit. + +**Trigger optimization.** Re-evaluation is skipped entirely when the resolved `repoConfig` has no `teams`, no `custom_properties`, and is not a rename — these are the only repo-level changes that can affect suborg matching. + ### Use `safe-settings` to rename repos If you rename a `` that corresponds to a repo, safe-settings will rename the repo to the new name. This behavior will take effect whether the env variable `BLOCK_REPO_RENAME_BY_HUMAN` is set or not. @@ -573,7 +594,73 @@ You can pass environment variables; the easiest way to do it is via a `.env` fil 3. __[Deploy and install the app](docs/deploy.md)__. Alternatively, the __[GitHub Actions Guide](docs/github-action.md)__ describes how to run `safe-settings` with GitHub Actions. +## Smoke Testing + +The repository includes an end-to-end smoke test script (`smoke-test.js`) that validates safe-settings against a live GitHub organization. It starts the app, creates repos/configs via the API, and verifies that safe-settings correctly applies and enforces settings. + +### Prerequisites + +- **Node.js** (same version used to run safe-settings) +- **`gh` CLI** — authenticated and available on PATH (used for drift-remediation tests only) +- A **GitHub App** installed on the target org with the required permissions +- A `.env` file in the project root (see below) + +### Authentication + +The smoke test uses **two authentication methods**: + +- **GitHub App token** (via `APP_ID` + `PRIVATE_KEY`) — used for the majority of tests: creating configs, merging PRs, validating repos, teams, rulesets, custom properties, etc. +- **Fine-grained PAT** (via `GH_TOKEN`) — used **only** in Phase 2 (team removal) and Phase 3 (rogue ruleset creation). These drift-remediation tests must appear as a human action because safe-settings ignores webhook events where `sender.type` is `Bot`. +### Configuration + +Add the following to your `.env` file: + +| Variable | Description | Required | +|---|---|---| +| `GH_ORG` | Target GitHub organization (e.g. `my-org`) | Yes | +| `APP_ID` | GitHub App ID | Yes | +| `PRIVATE_KEY` | GitHub App private key (use `\n` for newlines) | Yes | +| `WEBHOOK_PROXY_URL` | Smee.io proxy URL for webhooks | Yes | +| `ADMIN_REPO` | Admin repo name (default: `admin`) | No | +| `CONFIG_PATH` | Config path within admin repo (default: `.github`) | No | +| `GH_TOKEN` | Fine-grained PAT with org admin + repo permissions | Yes | +| `SMOKE_VERBOSE` | Set to `1` to show live safe-settings logs | No | + +### Running + +```bash +npm run smoke-test +# or +node smoke-test.js +``` + +### What it tests + +The smoke test runs 9 phases: + +| Phase | Description | +|---|---| +| **Setup** | Initializes the admin repo with an empty `settings.yml`, removes stale test repos, and starts safe-settings | +| **Phase 1** | Creates a repo config (`test`), validates NOP mode via check runs, merges, and verifies repo creation, teams, custom properties, and rulesets | +| **Phase 2** | Removes a team from the repo and verifies safe-settings re-adds it (drift remediation) | +| **Phase 3** | Creates a rogue ruleset and verifies safe-settings removes it (drift remediation) | +| **Phase 4** | Creates `demo-repo-service1` with teams, topics, and branch protection | +| **Phase 5** | Creates a suborg config and verifies org-scoped rulesets are applied to matching repos | +| **Phase 6** | Archives `demo-repo-service1` and verifies the repo is archived | +| **Phase 7** | Creates `demo-repo-service2` and verifies suborg rulesets are inherited | +| **Phase 8** | Creates org-level settings (custom repository roles + org rulesets) and verifies they are applied | +| **Teardown** | Shuts down safe-settings, deletes test repos, teams, custom roles, and rulesets | + +### Output + +The script uses colored terminal output with pass (✅) / fail (❌) indicators and prints a summary at the end: + +``` +══════════════════════════════════════ + Results: 45 passed, 0 failed +══════════════════════════════════════ +``` ## License diff --git a/lib/plugins/diffable.js b/lib/plugins/diffable.js index 069c68c78..44e5ae50c 100644 --- a/lib/plugins/diffable.js +++ b/lib/plugins/diffable.js @@ -62,6 +62,11 @@ module.exports = class Diffable extends ErrorStash { sync () { const resArray = [] + // Will be set to true when this plugin makes (or would make, in nop mode) + // any add/update/remove. Consumers (e.g. Settings suborg re-evaluation) + // can read `plugin.hasChanges` after `sync()` resolves to know whether + // anything actually changed for this repo. + this.hasChanges = false if (this.entries) { let filteredEntries = this.filterEntries() // this.log.debug(`filtered entries are ${JSON.stringify(filteredEntries)}`) @@ -72,6 +77,7 @@ module.exports = class Diffable extends ErrorStash { const compare = mergeDeep.compareDeep(existingRecords, filteredEntries) const results = { msg: 'Changes found', additions: compare.additions, modifications: compare.modifications, deletions: compare.deletions } this.log.debug(`Results of comparing ${this.constructor.name} diffable target ${JSON.stringify(existingRecords)} with source ${JSON.stringify(filteredEntries)} is ${JSON.stringify(results)}`) + this.hasChanges = !!compare.hasChanges if (!compare.hasChanges) { this.log.debug(`There are no changes for ${this.constructor.name} for repo ${this.repo.repo}. Skipping changes`) return Promise.resolve() diff --git a/lib/plugins/repository.js b/lib/plugins/repository.js index 14599f608..3333225d2 100644 --- a/lib/plugins/repository.js +++ b/lib/plugins/repository.js @@ -62,6 +62,10 @@ module.exports = class Repository extends ErrorStash { const resArray = [] this.log.debug(`Syncing Repo ${this.settings.name}`) this.settings.name = this.settings.name || this.settings.repo + // Change signals consumed by Settings suborg re-evaluation. + this.hasChanges = false + this.renamed = false + this.created = false // let hasChanges = false // let hasTopicChanges = false return this.github.repos.get(this.repo) @@ -74,6 +78,12 @@ module.exports = class Repository extends ErrorStash { const topicChanges = mergeDeep.compareDeep({ entries: resp.data.topics }, { entries: this.topics }) // hasTopicChanges = topicChanges.additions.length > 0 || topicChanges.modifications.length > 0 + this.hasChanges = !!(changes.hasChanges || topicChanges.hasChanges) + // A repo rename (changing the slug) shows up as a `name` modification. + if (changes.hasChanges && this.settings.name && resp.data.name && this.settings.name !== resp.data.name) { + this.renamed = true + } + // const results = JSON.stringify(changes, null, 2) const results = { msg: `${this.constructor.name} settings changes`, additions: changes.additions, modifications: changes.modifications, deletions: changes.deletions } @@ -120,6 +130,8 @@ module.exports = class Repository extends ErrorStash { }).catch(e => { if (e.status === 404) { if (this.force_create) { + this.hasChanges = true + this.created = true if (this.template) { this.log.debug(`Creating repo using template ${this.template}`) const options = { template_owner: this.repo.owner, template_repo: this.template, owner: this.repo.owner, name: this.repo.repo, private: (this.settings.private ? this.settings.private : true), description: this.settings.description ? this.settings.description : '' } diff --git a/lib/settings.js b/lib/settings.js index 4815e5a6b..d104efa5b 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -12,6 +12,12 @@ const eta = new Eta({ views: path.join(__dirname) }) const SCOPE = { ORG: 'org', REPO: 'repo' } // Determine if the setting is a org setting or repo setting const yaml = require('js-yaml') +// When a repo-yml change applies teams/properties/etc to a repo, the repo may +// newly match a suborg config (via suborgteams/suborgproperties/suborgrepos). +// Re-run updateRepos for the same repo at most this many times. Depth=1 is the +// tightest cap: we resolve a single hop of newly-matched suborg per sync. +const MAX_REEVALUATION_DEPTH = 1 + class Settings { static fileCache = {} @@ -46,6 +52,10 @@ class Settings { const settings = new Settings(nop, context, context.repo(), config, ref) try { + // Re-eval is enabled only for the per-repo iteration (repo-yml change + // path). The trailing suborg iteration below already iterates all suborg + // repos, so it is left with the flag off. + settings.reevaluateOnChange = true for (const repo of repos) { settings.repo = repo await settings.loadConfigs(repo) @@ -54,6 +64,7 @@ class Settings { } await settings.updateRepos(repo) } + settings.reevaluateOnChange = false for (const suborg of subOrgs) { settings.subOrgConfigMap = [suborg] settings.suborgChange = !!suborg @@ -70,6 +81,10 @@ class Settings { static async sync (nop, context, repo, config, ref) { const settings = new Settings(nop, context, repo, config, ref) try { + // Repo-yml change path: re-evaluate suborg membership for this repo if + // the applied changes (teams/custom_properties/new repo) cause it to + // newly match a suborg config. + settings.reevaluateOnChange = true await settings.loadConfigs(repo) if (settings.isRestricted(repo.repo)) { return @@ -124,6 +139,13 @@ class Settings { } } this.mergeDeep = new MergeDeep(this.log, this.github, [], this.configvalidators, this.overridevalidators) + // Suborg re-evaluation state (used only when reevaluateOnChange is true). + // - reevaluationDepth: repo name -> number of re-evaluation passes done. + // - reevaluatedRepos: repo name -> set of suborg source paths seen so far + // (used for stability comparison; if no new sources appear, we stop). + this.reevaluateOnChange = false + this.reevaluationDepth = new Map() + this.reevaluatedRepos = new Map() } // Create a check in the Admin repo for safe-settings. @@ -335,6 +357,12 @@ ${this.results.reduce((x, y) => { async updateRepos (repo) { this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs() + // Snapshot the set of suborg `source` paths that match this repo *before* + // we apply any changes. We compare against the post-apply set below to + // decide whether to re-evaluate (and to break stable loops). + const preMatchedSuborgSources = this.reevaluateOnChange + ? this.getAllMatchingSubOrgSources(repo.repo) + : null // Keeping this as is instead of doing an object assign as that would cause `Cannot read properties of undefined (reading 'startsWith')` error // Copilot code review would recoommend using object assign but that would cause the error let repoConfig = this.config.repository @@ -367,6 +395,10 @@ ${this.results.reduce((x, y) => { repoConfig = this.mergeDeep.mergeDeep({}, repoConfig, overrideRepoConfig) } if (repoConfig) { + // Track actual change signals from the plugins, used by the suborg + // re-evaluation logic below to avoid an unnecessary live API round-trip + // when nothing relevant actually changed. + const changeSignals = { teamsChanged: false, propertiesChanged: false, renamed: false, created: false } try { this.log.debug(`found a matching repoconfig for this repo ${JSON.stringify(repoConfig)}`) @@ -382,16 +414,27 @@ ${this.results.reduce((x, y) => { this.appendToResults(unArchiveResults) } - const repoResults = await new RepoPlugin(this.nop, this.github, repo, repoConfig, this.installation_id, this.log, this.errors).sync() + const repoPluginInstance = new RepoPlugin(this.nop, this.github, repo, repoConfig, this.installation_id, this.log, this.errors) + const repoResults = await repoPluginInstance.sync() this.appendToResults(repoResults) + if (repoPluginInstance.renamed) changeSignals.renamed = true + if (repoPluginInstance.created) changeSignals.created = true + const childPluginInstances = childPlugins.map(([Plugin, config]) => { + return [Plugin, new Plugin(this.nop, this.github, repo, config, this.log, this.errors)] + }) const childResults = await Promise.all( - childPlugins.map(([Plugin, config]) => { - return new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync() - }) + childPluginInstances.map(([, instance]) => instance.sync()) ) this.appendToResults(childResults) + // Collect change signals from relevant child plugins. + for (const [Plugin, instance] of childPluginInstances) { + if (!instance.hasChanges) continue + if (Plugin === Settings.PLUGINS.teams) changeSignals.teamsChanged = true + if (Plugin === Settings.PLUGINS.custom_properties) changeSignals.propertiesChanged = true + } + if (shouldArchive) { this.log.debug(`Archiving repo ${repo.repo}`) const archiveResults = await archivePlugin.sync() @@ -407,6 +450,14 @@ ${this.results.reduce((x, y) => { throw e } } + + // Suborg re-evaluation: if a repo-yml change actually applied teams or + // custom_properties (or this repo was just renamed/created), the repo + // may newly match a suborg config (suborgteams/suborgproperties/ + // suborgrepos). Refresh the suborg cache, compare matched-source sets; + // if it grew, re-run updateRepos once for this repo. Bounded by + // MAX_REEVALUATION_DEPTH and a stable-set check to prevent loops. + await this.maybeReevaluateSuborg(repo, repoConfig, preMatchedSuborgSources, changeSignals) } else { this.log.debug(`Didnt find any a matching repoconfig for this repo ${JSON.stringify(repo)} in ${JSON.stringify(this.repoConfigs)}`) const childPlugins = this.childPluginsList(repo) @@ -438,6 +489,123 @@ ${this.results.reduce((x, y) => { return undefined } + // Read-only helper used for suborg re-evaluation stability checks. + // Returns the set of suborg `source` paths (i.e. the suborg config file path) + // that match the given repo name. Apply-time behavior is unchanged: + // `getSubOrgConfig` still returns the first match and + // `storeSubOrgConfigIfNoConflicts` still forbids multi-suborg overlap at + // config-load time -- so this set normally contains 0 or 1 entries. We + // expose it as a Set so callers can detect the transition from {} -> {pathA} + // when a repo newly matches a suborg after teams/properties are applied. + getAllMatchingSubOrgSources (repoName) { + const sources = new Set() + if (!this.subOrgConfigs) { + return sources + } + for (const pattern of Object.keys(this.subOrgConfigs)) { + const glob = new Glob(pattern) + if (glob.test(repoName)) { + const source = this.subOrgConfigs[pattern]?.source + if (source) { + sources.add(source) + } + } + } + return sources + } + + // Force a refresh of the cached suborg configs. Used by the re-eval loop + // because suborgteams / suborgproperties resolution calls live GitHub APIs + // and may now match the repo after teams/properties were applied in the + // first pass. + async reloadSubOrgConfigs () { + this.subOrgConfigs = await this.getSubOrgConfigs() + } + + // Decide whether applying this repo's config actually changed state that + // could affect suborg matching. If no relevant change happened, skip the + // re-eval API roundtrip entirely. + // + // Preferred path: use plugin-emitted change signals from the just-completed + // sync (teams plugin actually added/removed/updated, custom_properties + // plugin changed values, repository plugin renamed/created). These come + // from the Diffable base class (`plugin.hasChanges`) and the Repository + // plugin (`renamed`, `created`). + // + // Fallback (changeSignals omitted, e.g. unit tests calling the helper in + // isolation): inspect the per-repo yml top-level shape for teams / + // custom_properties / rename indicators. + shouldConsiderReevaluation (repo, repoConfig, changeSignals) { + if (changeSignals) { + return !!( + changeSignals.teamsChanged || + changeSignals.propertiesChanged || + changeSignals.renamed || + changeSignals.created + ) + } + const repoYml = this.repoConfigs && ( + this.repoConfigs[`${repo.repo}.yml`] || this.repoConfigs[`${repo.repo}.yaml`] + ) + if (repoYml) { + if (Array.isArray(repoYml.teams) && repoYml.teams.length > 0) return true + if (Array.isArray(repoYml.custom_properties) && repoYml.custom_properties.length > 0) return true + } + if (repo && repo.oldname && repo.oldname !== repo.repo) return true + if (repoConfig && repoConfig.oldname && repoConfig.oldname !== repoConfig.name) return true + return false + } + + // After applying changes to a repo, decide whether to re-run updateRepos + // because the applied changes may have caused the repo to newly match a + // suborg config. Loop prevention has two layers: + // 1. Hard cap: MAX_REEVALUATION_DEPTH (=1) re-evaluation passes per repo. + // 2. Stability check: stop if the set of matched suborg sources did not + // grow (no new suborg source appeared since the last pass). + async maybeReevaluateSuborg (repo, repoConfig, preMatchedSuborgSources, changeSignals) { + if (!this.reevaluateOnChange) return + if (!preMatchedSuborgSources) return + if (!this.shouldConsiderReevaluation(repo, repoConfig, changeSignals)) { + this.log.debug(`Suborg re-eval: skipping for ${repo.repo} (no relevant changes from teams/custom_properties/repository plugins)`) + return + } + + const depth = this.reevaluationDepth.get(repo.repo) || 0 + if (depth >= MAX_REEVALUATION_DEPTH) { + this.log.warn(`Suborg re-eval: max depth (${MAX_REEVALUATION_DEPTH}) reached for ${repo.repo}; stopping. Any further suborg matches will be picked up on the next sync.`) + return + } + + // Refresh suborg config cache; suborgteams/suborgproperties resolution + // hits live GitHub APIs and may now match this repo. + await this.reloadSubOrgConfigs() + + const seen = this.reevaluatedRepos.get(repo.repo) || new Set(preMatchedSuborgSources) + const newMatched = this.getAllMatchingSubOrgSources(repo.repo) + + // Stability check: if no new suborg source appeared, we're done. + let hasNew = false + for (const source of newMatched) { + if (!seen.has(source)) { + hasNew = true + seen.add(source) + } + } + if (!hasNew) { + this.log.debug(`Suborg re-eval: stable for ${repo.repo} (matched sources: ${JSON.stringify(Array.from(newMatched))}); stopping.`) + return + } + + this.reevaluatedRepos.set(repo.repo, seen) + this.reevaluationDepth.set(repo.repo, depth + 1) + this.log.debug(`Suborg re-eval: new suborg source(s) matched ${repo.repo} after apply; re-running updateRepos (depth=${depth + 1}).`) + + // Reload repo-level configs for this repo so the next pass picks up any + // state changes; then recurse. Depth cap above prevents infinite loops. + this.repoConfigs = await this.getRepoConfigs(repo) + await this.updateRepos(repo) + } + // Remove Org specific configs from the repo config returnRepoSpecificConfigs (config) { const newConfig = Object.assign({}, config) // clone diff --git a/package.json b/package.json index fbfa284da..f0ae365bf 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,8 @@ "test:me": "jest ", "test:unit:watch": "npm run test:unit -- --watch", "test:integration": "jest --roots=lib --roots=test/integration", - "test:integration:debug": "LOG_LEVEL=debug DEBUG=nock run-s test:integration" + "test:integration:debug": "LOG_LEVEL=debug DEBUG=nock run-s test:integration", + "smoke-test": "node smoke-test.js" }, "author": "Yadhav Jayaraman", "license": "ISC", diff --git a/smoke-test.js b/smoke-test.js new file mode 100644 index 000000000..dcb4df8af --- /dev/null +++ b/smoke-test.js @@ -0,0 +1,945 @@ +#!/usr/bin/env node + +/** + * Smoke Test for safe-settings + * + * Usage: + * 1. Ensure `.env` is configured with GH_ORG, APP_ID, PRIVATE_KEY, WEBHOOK_PROXY_URL, etc. + * 2. Set GH_TOKEN env var to a fine-grained PAT with org admin + repo permissions. + * This is required for drift-remediation tests (Phases 2 & 3) so that + * changes appear as a human (not Bot) and trigger safe-settings webhooks. + * 3. Run: `node smoke-test.js` + * Set SMOKE_VERBOSE=1 for live safe-settings logs. + * + * Auth: + * - Octokit (GitHub App): APP_ID + PRIVATE_KEY from .env — used for most operations. + * - gh CLI (user PAT): GH_TOKEN env var — used for drift tests only. + */ + +const { execSync, spawn } = require('child_process') +const fs = require('fs') +const path = require('path') + +// ─── Configuration ─────────────────────────────────────────────────────────── + +function loadEnv () { + const envPath = path.join(__dirname, '.env') + if (!fs.existsSync(envPath)) throw new Error('.env file not found') + const lines = fs.readFileSync(envPath, 'utf8').split('\n') + let currentKey = null + let currentValue = '' + let inMultiline = false + + for (const line of lines) { + if (inMultiline) { + currentValue += '\n' + line + if (line.includes('"') || line.includes("'")) { + const val = currentValue.replace(/^["']|["']$/g, '') + // Like dotenv: .env values don't override existing env vars + if (!(currentKey in process.env)) process.env[currentKey] = val + inMultiline = false + } + continue + } + const trimmed = line.trim() + if (!trimmed || trimmed.startsWith('#')) continue + const eqIdx = trimmed.indexOf('=') + if (eqIdx === -1) continue + currentKey = trimmed.slice(0, eqIdx).trim() + currentValue = trimmed.slice(eqIdx + 1).trim() + if ((currentValue.startsWith('"') && !currentValue.endsWith('"')) || + (currentValue.startsWith("'") && !currentValue.endsWith("'"))) { + inMultiline = true + continue + } + const val = currentValue.replace(/^["']|["']$/g, '') + if (!(currentKey in process.env)) process.env[currentKey] = val + } +} + +loadEnv() + +const ORG = process.env.GH_ORG || 'decyjphr-emu' +const ADMIN_REPO = process.env.ADMIN_REPO || 'admin' +const CONFIG_PATH = process.env.CONFIG_PATH || '.github' +const APP_ID = process.env.APP_ID +const PRIVATE_KEY = (process.env.PRIVATE_KEY || '').replace(/\\n/g, '\n') + +const TEST_REPOS = ['test', 'demo-repo-service1', 'demo-repo-service2'] +const TEST_TEAMS = ['AD-GRP-PAYMENTS-PLATFORM-OWNERS', 'awesometeam-a-approvers'] + +const POLL_INTERVAL_MS = 5000 +const MAX_POLL_MS = 120000 +const WEBHOOK_SETTLE_MS = 15000 + +// Fine-grained PAT for drift tests (must appear as a human, not Bot) +const GH_TOKEN = process.env.GH_TOKEN || '' + +// ─── Octokit client (initialized in main) ──────────────────────────────────── + +let octokit = null + +// ─── Helpers ───────────────────────────────────────────────────────────────── + +let passCount = 0 +let failCount = 0 +const failures = [] + +function log (msg) { console.log(`\x1b[36m[smoke]\x1b[0m ${msg}`) } +function logPass (msg) { passCount++; console.log(`\x1b[32m ✓ ${msg}\x1b[0m`) } +function logFail (msg) { failCount++; failures.push(msg); console.log(`\x1b[31m ✗ ${msg}\x1b[0m`) } +function logPhase (msg) { console.log(`\n\x1b[35m═══ ${msg} ═══\x1b[0m`) } + +function assert (condition, msg) { + if (condition) logPass(msg) + else logFail(msg) + return condition +} + +function sleep (ms) { return new Promise(resolve => setTimeout(resolve, ms)) } + +async function poll (fn, { timeout = MAX_POLL_MS, interval = POLL_INTERVAL_MS, desc = 'condition' } = {}) { + const start = Date.now() + while (Date.now() - start < timeout) { + const result = await fn() + if (result) return result + await sleep(interval) + } + log(` ⚠ Timed out waiting for ${desc}`) + return null +} + +// ─── GitHub API helpers ────────────────────────────────────────────────────── + +async function getDefaultBranch () { + const { data } = await octokit.rest.repos.get({ owner: ORG, repo: ADMIN_REPO }) + return data.default_branch || 'main' +} + +async function createOrUpdateFile (owner, repo, filePath, content, branch, message) { + const b64 = Buffer.from(content).toString('base64') + let sha = null + try { + const { data } = await octokit.rest.repos.getContent({ owner, repo, path: filePath, ref: branch }) + sha = data.sha + } catch { /* file doesn't exist */ } + const params = { owner, repo, path: filePath, message, content: b64, branch } + if (sha) params.sha = sha + return (await octokit.rest.repos.createOrUpdateFileContents(params)).data +} + +async function deleteFile (owner, repo, filePath, branch, message) { + try { + const { data } = await octokit.rest.repos.getContent({ owner, repo, path: filePath, ref: branch }) + await octokit.rest.repos.deleteFile({ owner, repo, path: filePath, message, sha: data.sha, branch }) + } catch { /* file doesn't exist */ } +} + +async function cleanDirectory (owner, repo, dirPath) { + const branch = await getDefaultBranch() + try { + const { data } = await octokit.rest.repos.getContent({ owner, repo, path: dirPath, ref: branch }) + if (Array.isArray(data)) { + for (const file of data) { + if (file.type === 'file') { + await deleteFile(owner, repo, file.path, branch, `Clean up ${file.path}`) + } + } + } + } catch { /* directory doesn't exist */ } +} + +async function createBranch (owner, repo, branchName) { + const defaultBranch = await getDefaultBranch() + const { data: ref } = await octokit.rest.git.getRef({ owner, repo, ref: `heads/${defaultBranch}` }) + await octokit.rest.git.createRef({ owner, repo, ref: `refs/heads/${branchName}`, sha: ref.object.sha }) +} + +async function deleteBranch (owner, repo, branch) { + try { await octokit.rest.git.deleteRef({ owner, repo, ref: `heads/${branch}` }) } catch { /* ok */ } +} + +async function createPR (owner, repo, title, head, base) { + const { data } = await octokit.rest.pulls.create({ owner, repo, title, head, base, body: `Smoke test: ${title}` }) + log(` Created PR #${data.number}`) + return data +} + +async function mergePR (owner, repo, prNumber) { + return (await octokit.rest.pulls.merge({ owner, repo, pull_number: prNumber, merge_method: 'merge' })).data +} + +async function deleteRepo (owner, repo) { + try { await octokit.rest.repos.delete({ owner, repo }) } catch { /* ok */ } +} + +async function deleteTeam (org, teamSlug) { + try { await octokit.rest.teams.deleteInOrg({ org, team_slug: teamSlug }) } catch { /* ok */ } +} + +async function waitForCheckRun (owner, repo, sha, { timeout = MAX_POLL_MS } = {}) { + return poll(async () => { + const { data } = await octokit.rest.checks.listForRef({ owner, repo, ref: sha }) + const cr = data.check_runs.find(c => c.name === 'Safe-setting validator') + return (cr && cr.status === 'completed') ? cr : null + }, { timeout, desc: 'check run to complete' }) +} + +// ─── Safe-settings process management ──────────────────────────────────────── + +let ssProcess = null + +function startSafeSettings () { + log('Starting safe-settings...') + ssProcess = spawn('npm', ['start'], { + cwd: __dirname, + env: process.env, + stdio: ['ignore', 'pipe', 'pipe'] + }) + ssProcess.stdout.on('data', (d) => { if (process.env.SMOKE_VERBOSE) process.stdout.write(d) }) + ssProcess.stderr.on('data', (d) => { if (process.env.SMOKE_VERBOSE) process.stderr.write(d) }) + ssProcess.on('exit', (code) => { log(`safe-settings exited with code ${code}`) }) +} + +function stopSafeSettings () { + if (ssProcess) { + log('Stopping safe-settings...') + ssProcess.kill('SIGTERM') + ssProcess = null + } +} + +// ─── YAML Configs ──────────────────────────────────────────────────────────── + +const REPO_TEST_YML = `repository: + name: test + description: Demo repository created via safe-settings + private: true + auto_init: true + force_create: true + has_issues: true + has_projects: false + has_wiki: false + delete_branch_on_merge: true + allow_squash_merge: true + allow_merge_commit: false + allow_rebase_merge: true + +teams: + - name: expert-services-developers + permission: push + +custom_properties: + - property_name: ent-ownership + value: expert-services + - property_name: ent-supervisory-org + value: expert-services + +rulesets: +- name: synk + target: branch + enforcement: disabled + bypass_actors: + - actor_id: 1 + actor_type: OrganizationAdmin + bypass_mode: pull_request + + conditions: + ref_name: + include: ["~DEFAULT_BRANCH"] + exclude: ["refs/heads/oldmaster"] + + rules: + - type: creation + - type: update + - type: deletion + - type: required_linear_history + - type: required_signatures + - type: pull_request + parameters: + dismiss_stale_reviews_on_push: true + require_code_owner_review: true + require_last_push_approval: true + required_approving_review_count: 2 + required_review_thread_resolution: true + + - type: commit_message_pattern + parameters: + name: test commit_message_pattern + negate: true + operator: starts_with + pattern: skip* + + - type: commit_author_email_pattern + parameters: + name: test commit_author_email_pattern + negate: false + operator: regex + pattern: "^.*@example.com$" + + - type: committer_email_pattern + parameters: + name: test committer_email_pattern + negate: false + operator: regex + pattern: "^.*@example.com$" + + - type: branch_name_pattern + parameters: + name: test branch_name_pattern + negate: false + operator: regex + pattern: ".*\\\\/.*" + +- name: Prevent merges when new SONAR alerts are introduced + target: branch + enforcement: active + conditions: + ref_name: + include: + - "~DEFAULT_BRANCH" + exclude: [] + bypass_actors: + - actor_type: OrganizationAdmin + bypass_mode: always + rules: + - type: code_scanning + parameters: + code_scanning_tools: + - tool: Sonar + alerts_threshold: none + security_alerts_threshold: medium_or_higher +` + +const REPO_DEMO_SERVICE1_YML = `# Safe-Settings Configuration +repository: + name: demo-repo-service1 + description: "Repository 2 sample" + visibility: private + default_branch: main + homepage: "" + auto_init: true + force_create: true + delete_branch_on_merge: true + archived: false + topics: + - topic1 + - topic2 + +teams: + - name: AD-GRP-PAYMENTS-PLATFORM-OWNERS + permission: admin + - name: awesometeam-a-approvers + permission: push + - name: expert-services-developers + permission: push + +branches: + - name: main + protection: + required_status_checks: + strict: true + contexts: [] + required_pull_request_reviews: + required_approving_review_count: 2 + dismiss_stale_reviews: false + require_code_owner_reviews: true + require_last_push_approval: false + bypass_pull_request_allowances: + apps: [] + users: [] + teams: [] + dismissal_restrictions: + users: [] + teams: [] + enforce_admins: true + restrictions: + apps: [] + users: [] + teams: [] + + - name: develop + protection: + required_status_checks: + strict: true + contexts: [] + required_pull_request_reviews: + required_approving_review_count: 1 + dismiss_stale_reviews: false + require_code_owner_reviews: true + require_last_push_approval: false + bypass_pull_request_allowances: + apps: [] + users: [] + teams: [] + dismissal_restrictions: + users: [] + teams: [] + enforce_admins: true + restrictions: + apps: [] + users: [] + teams: [] +` + +const SUBORG_EXPERT_SERVICES_YML = `suborgteams: + - expert-services-developers + +rulesets: + - name: Protect release and production branches + target: branch + enforcement: active + conditions: + ref_name: + include: + - refs/heads/release/* + - refs/heads/production + exclude: [] + bypass_actors: + - actor_type: OrganizationAdmin + bypass_mode: always + rules: + - type: creation + - type: pull_request + parameters: + required_approving_review_count: 1 + dismiss_stale_reviews_on_push: false + require_code_owner_review: false + require_last_push_approval: false + required_review_thread_resolution: false + allowed_merge_methods: + - merge + - squash + - rebase + required_reviewers: + - minimum_approvals: 1 + file_patterns: + - "*.js" + reviewer: + id: 11721733 + type: Team +` + +const REPO_DEMO_SERVICE1_ARCHIVED_YML = `# Safe-Settings Configuration +repository: + name: demo-repo-service1 + description: "Repository 2 sample" + visibility: private + default_branch: main + homepage: "" + auto_init: true + force_create: true + delete_branch_on_merge: true + archived: true +` + +const REPO_DEMO_SERVICE2_YML = `# Safe-Settings Configuration +repository: + name: demo-repo-service2 + description: "Repository 2 sample" + visibility: private + default_branch: main + homepage: "" + auto_init: true + force_create: true + delete_branch_on_merge: true + archived: false + topics: + - topic1 + - topic2 + +teams: + - name: expert-services-developers + permission: push +` + +const SETTINGS_YML_ORG = `# Org-level safe-settings configuration + +rulesets: + - name: test + target: repository + source_type: Organization + source: ${ORG} + enforcement: disabled + conditions: + repository_property: + exclude: [] + include: + - name: visibility + source: system + property_values: + - internal + rules: + - type: repository_delete + +custom_repository_roles: + - name: security-engineer + description: Can contribute code and manage the security pipeline + base_role: maintain + permissions: + - delete_alerts_code_scanning +` + +// ─── Test Phases ───────────────────────────────────────────────────────────── + +async function setup () { + logPhase('Phase 0: Setup') + + log('Cleaning up test repos...') + for (const repo of TEST_REPOS) { await deleteRepo(ORG, repo) } + + log('Initializing admin repo with empty settings...') + const defaultBranch = await getDefaultBranch() + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, '# empty\n', defaultBranch, 'Initialize empty settings.yml for smoke test') + + log('Cleaning up repos/ and suborgs/ directories...') + await cleanDirectory(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos`) + await cleanDirectory(ORG, ADMIN_REPO, `${CONFIG_PATH}/suborgs`) + + startSafeSettings() + log('Waiting for safe-settings to initialize...') + await sleep(15000) + log('Setup complete') +} + +async function phase1CreateRepo () { + logPhase('Phase 1: Create test repo via test.yml') + const branch = 'smoke-test-phase1' + const defaultBranch = await getDefaultBranch() + + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + log('Created branch: ' + branch) + + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/test.yml`, REPO_TEST_YML, branch, 'Add test repo config') + log('Added test.yml to branch') + + const pr = await createPR(ORG, ADMIN_REPO, 'Smoke test: add test repo', branch, defaultBranch) + + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, 'Check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `Check run conclusion is success (got: ${checkRun.conclusion})`) + + log('Merging PR...') + await mergePR(ORG, ADMIN_REPO, pr.number) + await sleep(WEBHOOK_SETTLE_MS) + + // Validate repo + const repo = await poll(async () => { + try { return (await octokit.rest.repos.get({ owner: ORG, repo: 'test' })).data } catch { return null } + }, { desc: 'repo test to be created' }) + + assert(repo !== null, 'Repo "test" was created') + if (repo) { + assert(repo.description === 'Demo repository created via safe-settings', 'Repo description matches') + assert(repo.private === true, 'Repo is private') + assert(repo.has_issues === true, 'has_issues enabled') + assert(repo.has_projects === false, 'has_projects disabled') + assert(repo.has_wiki === false, 'has_wiki disabled') + assert(repo.delete_branch_on_merge === true, 'delete_branch_on_merge is true') + assert(repo.allow_squash_merge === true, 'allow_squash_merge is true') + assert(repo.allow_merge_commit === false, 'allow_merge_commit is false') + assert(repo.allow_rebase_merge === true, 'allow_rebase_merge is true') + } + + // Validate team (poll — safe-settings may still be processing) + const esTeam = await poll(async () => { + try { + const { data: teams } = await octokit.rest.repos.listTeams({ owner: ORG, repo: 'test' }) + return teams.find(t => t.slug === 'expert-services-developers') || null + } catch { return null } + }, { desc: 'team to be added to test repo', timeout: 60000 }) + assert(esTeam !== null, 'Team expert-services-developers added') + if (esTeam) assert(esTeam.permission === 'push', `Team has push permission (got: ${esTeam.permission})`) + + // Validate custom properties (poll) + const propsOk = await poll(async () => { + try { + const { data: props } = await octokit.request('GET /repos/{owner}/{repo}/properties/values', { owner: ORG, repo: 'test' }) + const propList = Array.isArray(props) ? props : [] + const ownership = propList.find(p => p.property_name === 'ent-ownership') + const supervisory = propList.find(p => p.property_name === 'ent-supervisory-org') + return (ownership && ownership.value === 'expert-services' && supervisory && supervisory.value === 'expert-services') || null + } catch { return null } + }, { desc: 'custom properties to be set', timeout: 60000 }) + assert(propsOk, 'Custom properties ent-ownership and ent-supervisory-org set') + + // Validate rulesets (poll) + const rulesetsOk = await poll(async () => { + try { + const { data: rulesets } = await octokit.request('GET /repos/{owner}/{repo}/rulesets', { owner: ORG, repo: 'test' }) + const synk = rulesets.find(r => r.name === 'synk') + const sonar = rulesets.find(r => r.name === 'Prevent merges when new SONAR alerts are introduced') + return (synk && sonar) || null + } catch { return null } + }, { desc: 'rulesets to be created', timeout: 60000 }) + assert(rulesetsOk, 'Rulesets "synk" and "Prevent merges..." created') + + await deleteBranch(ORG, ADMIN_REPO, branch) +} + +async function phase2DriftTeam () { + logPhase('Phase 2: Drift remediation - Team removal') + + // Use gh CLI with user PAT so the event sender is a Human, not Bot + log('Removing expert-services-developers from test repo (as user)...') + if (!GH_TOKEN) throw new Error('GH_TOKEN env var is required for drift tests (set to a fine-grained PAT)') + try { + execSync(`gh api /orgs/${ORG}/teams/expert-services-developers/repos/${ORG}/test --method DELETE`, { + encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'] + }) + } catch (e) { logFail(`Could not remove team: ${e.message}`); return } + + log('Waiting for safe-settings to remediate...') + await sleep(WEBHOOK_SETTLE_MS) + + const team = await poll(async () => { + try { + const { data: teams } = await octokit.rest.repos.listTeams({ owner: ORG, repo: 'test' }) + return teams.find(t => t.slug === 'expert-services-developers') || null + } catch { return null } + }, { desc: 'team to be re-added', timeout: 60000 }) + + assert(team !== null, 'Team re-added after drift') +} + +async function phase3DriftRuleset () { + logPhase('Phase 3: Drift remediation - Rogue ruleset') + + // Use gh CLI with user PAT so the event sender is a Human, not Bot + log('Creating rogue ruleset on test repo (as user)...') + const body = JSON.stringify({ + name: 'rogue-ruleset', target: 'branch', enforcement: 'active', + conditions: { ref_name: { include: ['~DEFAULT_BRANCH'], exclude: [] } }, + rules: [{ type: 'deletion' }] + }) + try { + execSync(`gh api /repos/${ORG}/test/rulesets --method POST --input -`, { + encoding: 'utf8', input: body, stdio: ['pipe', 'pipe', 'pipe'] + }) + } catch (e) { logFail(`Could not create rogue ruleset: ${e.message}`); return } + + log('Waiting for safe-settings to remove rogue ruleset...') + await sleep(WEBHOOK_SETTLE_MS) + + const removed = await poll(async () => { + try { + const { data: rs } = await octokit.request('GET /repos/{owner}/{repo}/rulesets', { owner: ORG, repo: 'test' }) + return !rs.find(r => r.name === 'rogue-ruleset') + } catch { return false } + }, { desc: 'rogue ruleset to be removed', timeout: 90000 }) + + assert(removed, 'Rogue ruleset removed by safe-settings') +} + +async function phase4DemoRepo1 () { + logPhase('Phase 4: Create demo-repo-service1') + const branch = 'smoke-test-phase4' + const defaultBranch = await getDefaultBranch() + + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/demo-repo-service1.yml`, REPO_DEMO_SERVICE1_YML, branch, 'Add demo-repo-service1 config') + + const pr = await createPR(ORG, ADMIN_REPO, 'Smoke test: add demo-repo-service1', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, 'Check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `Check run conclusion is success (got: ${checkRun.conclusion})`) + + log('Merging PR...') + await mergePR(ORG, ADMIN_REPO, pr.number) + await sleep(WEBHOOK_SETTLE_MS) + + const repo = await poll(async () => { + try { return (await octokit.rest.repos.get({ owner: ORG, repo: 'demo-repo-service1' })).data } catch { return null } + }, { desc: 'demo-repo-service1 to be created' }) + + assert(repo !== null, 'Repo "demo-repo-service1" created') + if (repo) { + assert(repo.description === 'Repository 2 sample', 'Description matches') + assert(repo.private === true, 'Repo is private') + assert(repo.archived === false, 'Repo is not archived') + } + + const teamsOk = await poll(async () => { + try { + const { data: teams } = await octokit.rest.repos.listTeams({ owner: ORG, repo: 'demo-repo-service1' }) + const t1 = teams.find(t => t.slug === 'ad-grp-payments-platform-owners') + const t2 = teams.find(t => t.slug === 'awesometeam-a-approvers') + const t3 = teams.find(t => t.slug === 'expert-services-developers') + return (t1 && t2 && t3) ? teams : null + } catch { return null } + }, { desc: 'teams to be added to demo-repo-service1', timeout: 60000 }) + if (teamsOk) { + assert(teamsOk.find(t => t.slug === 'ad-grp-payments-platform-owners') !== undefined, 'Team AD-GRP-PAYMENTS-PLATFORM-OWNERS added') + assert(teamsOk.find(t => t.slug === 'awesometeam-a-approvers') !== undefined, 'Team awesometeam-a-approvers added') + assert(teamsOk.find(t => t.slug === 'expert-services-developers') !== undefined, 'Team expert-services-developers added') + } else { logFail('Teams not added to demo-repo-service1 in time') } + + const topicsOk = await poll(async () => { + try { + const { data: topics } = await octokit.rest.repos.getAllTopics({ owner: ORG, repo: 'demo-repo-service1' }) + return (topics.names.includes('topic1') && topics.names.includes('topic2')) ? topics : null + } catch { return null } + }, { desc: 'topics to be set on demo-repo-service1', timeout: 120000 }) + assert(topicsOk, 'Topics topic1 and topic2 set') + + await deleteBranch(ORG, ADMIN_REPO, branch) +} + +async function phase5Suborg () { + logPhase('Phase 5: Create suborg config') + const branch = 'smoke-test-phase5' + const defaultBranch = await getDefaultBranch() + + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/suborgs/expert-services.yml`, SUBORG_EXPERT_SERVICES_YML, branch, 'Add expert-services suborg config') + + const pr = await createPR(ORG, ADMIN_REPO, 'Smoke test: add expert-services suborg', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, 'Check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `Check run conclusion is success (got: ${checkRun.conclusion})`) + + log('Merging PR...') + await mergePR(ORG, ADMIN_REPO, pr.number) + await sleep(WEBHOOK_SETTLE_MS) + + log('Checking suborg ruleset on demo-repo-service1...') + const ruleset = await poll(async () => { + try { + const { data: rs } = await octokit.request('GET /repos/{owner}/{repo}/rulesets', { owner: ORG, repo: 'demo-repo-service1' }) + return rs.find(r => r.name === 'Protect release and production branches') || null + } catch { return null } + }, { desc: 'suborg ruleset on demo-repo-service1', timeout: 60000 }) + + assert(ruleset !== null, 'Suborg ruleset applied to demo-repo-service1') + await deleteBranch(ORG, ADMIN_REPO, branch) +} + +async function phase6Archive () { + logPhase('Phase 6: Archive demo-repo-service1') + const branch = 'smoke-test-phase6' + const defaultBranch = await getDefaultBranch() + + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/demo-repo-service1.yml`, REPO_DEMO_SERVICE1_ARCHIVED_YML, branch, 'Archive demo-repo-service1') + + const pr = await createPR(ORG, ADMIN_REPO, 'Smoke test: archive demo-repo-service1', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, 'Check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `Check run conclusion is success (got: ${checkRun.conclusion})`) + + log('Merging PR...') + await mergePR(ORG, ADMIN_REPO, pr.number) + await sleep(WEBHOOK_SETTLE_MS) + + const repo = await poll(async () => { + try { + const { data } = await octokit.rest.repos.get({ owner: ORG, repo: 'demo-repo-service1' }) + return data.archived ? data : null + } catch { return null } + }, { desc: 'demo-repo-service1 to be archived' }) + + assert(repo !== null && repo.archived === true, 'Repo demo-repo-service1 is archived') + await deleteBranch(ORG, ADMIN_REPO, branch) +} + +async function phase7DemoRepo2 () { + logPhase('Phase 7: Create demo-repo-service2') + const branch = 'smoke-test-phase7' + const defaultBranch = await getDefaultBranch() + + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/demo-repo-service2.yml`, REPO_DEMO_SERVICE2_YML, branch, 'Add demo-repo-service2 config') + + const pr = await createPR(ORG, ADMIN_REPO, 'Smoke test: add demo-repo-service2', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, 'Check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `Check run conclusion is success (got: ${checkRun.conclusion})`) + + log('Merging PR...') + await mergePR(ORG, ADMIN_REPO, pr.number) + await sleep(WEBHOOK_SETTLE_MS) + + const repo = await poll(async () => { + try { return (await octokit.rest.repos.get({ owner: ORG, repo: 'demo-repo-service2' })).data } catch { return null } + }, { desc: 'demo-repo-service2 to be created' }) + + assert(repo !== null, 'Repo "demo-repo-service2" created') + if (repo) { + assert(repo.archived === false, 'Repo is not archived') + assert(repo.private === true, 'Repo is private') + } + + try { + const { data: teams } = await octokit.rest.repos.listTeams({ owner: ORG, repo: 'demo-repo-service2' }) + assert(teams.find(t => t.slug === 'expert-services-developers') !== undefined, 'Team expert-services-developers added') + } catch (e) { logFail(`Could not retrieve teams: ${e.message}`) } + + log('Checking suborg ruleset on demo-repo-service2...') + const ruleset = await poll(async () => { + try { + const { data: rs } = await octokit.request('GET /repos/{owner}/{repo}/rulesets', { owner: ORG, repo: 'demo-repo-service2' }) + return rs.find(r => r.name === 'Protect release and production branches') || null + } catch { return null } + }, { desc: 'suborg ruleset on demo-repo-service2', timeout: 60000 }) + + assert(ruleset !== null, 'Suborg ruleset applied to demo-repo-service2') + await deleteBranch(ORG, ADMIN_REPO, branch) +} + +async function phase8OrgSettings () { + logPhase('Phase 8: Org-level settings') + const branch = 'smoke-test-phase8' + const defaultBranch = await getDefaultBranch() + + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, SETTINGS_YML_ORG, branch, 'Add org-level settings') + + const pr = await createPR(ORG, ADMIN_REPO, 'Smoke test: org-level settings', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, 'Check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `Check run conclusion is success (got: ${checkRun.conclusion})`) + + log('Merging PR...') + await mergePR(ORG, ADMIN_REPO, pr.number) + await sleep(WEBHOOK_SETTLE_MS) + + log('Checking custom repository roles...') + const role = await poll(async () => { + try { + const { data } = await octokit.request('GET /orgs/{org}/custom-repository-roles', { org: ORG }) + return (data.custom_roles || []).find(r => r.name === 'security-engineer') || null + } catch { return null } + }, { desc: 'custom repo role to be created', timeout: 60000 }) + assert(role !== null, 'Custom repository role "security-engineer" created') + + log('Checking org rulesets...') + const orgRuleset = await poll(async () => { + try { + const { data: rs } = await octokit.request('GET /orgs/{org}/rulesets', { org: ORG }) + return rs.find(r => r.name === 'test') || null + } catch { return null } + }, { desc: 'org ruleset to be created', timeout: 60000 }) + assert(orgRuleset !== null, 'Org ruleset "test" created') + + await deleteBranch(ORG, ADMIN_REPO, branch) +} + +async function teardown () { + logPhase('Phase 9: Teardown') + + stopSafeSettings() + + log('Deleting test repos...') + try { await octokit.rest.repos.update({ owner: ORG, repo: 'demo-repo-service1', archived: false }) } catch { /* ok */ } + for (const repo of TEST_REPOS) { await deleteRepo(ORG, repo) } + + log('Deleting test teams...') + for (const team of TEST_TEAMS) { await deleteTeam(ORG, team.toLowerCase()) } + + log('Deleting custom repository role...') + try { + const { data } = await octokit.request('GET /orgs/{org}/custom-repository-roles', { org: ORG }) + const secRole = (data.custom_roles || []).find(r => r.name === 'security-engineer') + if (secRole) await octokit.request('DELETE /orgs/{org}/custom-repository-roles/{role_id}', { org: ORG, role_id: secRole.id }) + } catch { /* ok */ } + + log('Deleting org rulesets...') + try { + const { data: rs } = await octokit.request('GET /orgs/{org}/rulesets', { org: ORG }) + const testRs = rs.find(r => r.name === 'test') + if (testRs) await octokit.request('DELETE /orgs/{org}/rulesets/{ruleset_id}', { org: ORG, ruleset_id: testRs.id }) + } catch { /* ok */ } + + log('Resetting admin repo settings...') + const defaultBranch = await getDefaultBranch() + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, '# empty\n', defaultBranch, 'Reset settings.yml after smoke test') + await cleanDirectory(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos`) + await cleanDirectory(ORG, ADMIN_REPO, `${CONFIG_PATH}/suborgs`) + + log('Teardown complete') +} + +// ─── Main ──────────────────────────────────────────────────────────────────── + +async function main () { + const { App } = await import('octokit') + const app = new App({ appId: APP_ID, privateKey: PRIVATE_KEY }) + + // Find installation for our org + let installationId + for await (const { installation } of app.eachInstallation.iterator()) { + if (installation.account && installation.account.login.toLowerCase() === ORG.toLowerCase()) { + installationId = installation.id + break + } + } + if (!installationId) throw new Error(`No installation found for org ${ORG}`) + + octokit = await app.getInstallationOctokit(installationId) + log('Authenticated as GitHub App installation') + + console.log(` +\x1b[36m╔══════════════════════════════════════╗ +║ Safe-Settings Smoke Test ║ +║ Org: ${ORG.padEnd(28)}║ +║ Admin Repo: ${ADMIN_REPO.padEnd(22)}║ +╚══════════════════════════════════════╝\x1b[0m +`) + + try { + await setup() + await phase1CreateRepo() + await phase2DriftTeam() + await phase3DriftRuleset() + await phase4DemoRepo1() + await phase5Suborg() + await phase6Archive() + await phase7DemoRepo2() + await phase8OrgSettings() + } catch (err) { + console.error(`\x1b[31mFatal error: ${err.message}\x1b[0m`) + console.error(err.stack) + } finally { + await teardown() + } + + console.log(` +\x1b[36m╔══════════════════════════════════════╗ +║ Results ║ +╚══════════════════════════════════════╝\x1b[0m + \x1b[32mPassed: ${passCount}\x1b[0m + \x1b[31mFailed: ${failCount}\x1b[0m +`) + + if (failures.length > 0) { + console.log('\x1b[31mFailures:\x1b[0m') + failures.forEach((f, i) => console.log(` ${i + 1}. ${f}`)) + console.log() + } + + process.exit(failCount > 0 ? 1 : 0) +} + +main().catch(err => { + console.error(err) + stopSafeSettings() + process.exit(1) +}) diff --git a/test/unit/lib/settings.test.js b/test/unit/lib/settings.test.js index 39aac216d..23d8ef84d 100644 --- a/test/unit/lib/settings.test.js +++ b/test/unit/lib/settings.test.js @@ -458,4 +458,185 @@ repository: ); }); }); + + describe('getAllMatchingSubOrgSources', () => { + it('returns an empty set when subOrgConfigs is undefined', () => { + const settings = createSettings({}) + settings.subOrgConfigs = undefined + const result = settings.getAllMatchingSubOrgSources('any-repo') + expect(result).toBeInstanceOf(Set) + expect(result.size).toBe(0) + }) + + it('returns an empty set when no suborg matches', () => { + const settings = createSettings({}) + settings.subOrgConfigs = { + 'frontend-*': { source: '.github/suborgs/frontend.yml' } + } + const result = settings.getAllMatchingSubOrgSources('backend-repo') + expect(result.size).toBe(0) + }) + + it('returns a single-entry set when one suborg glob matches', () => { + const settings = createSettings({}) + settings.subOrgConfigs = { + 'frontend-*': { source: '.github/suborgs/frontend.yml' }, + 'backend-*': { source: '.github/suborgs/backend.yml' } + } + const result = settings.getAllMatchingSubOrgSources('frontend-app') + expect(result.size).toBe(1) + expect(result.has('.github/suborgs/frontend.yml')).toBe(true) + }) + + it('does not alter getSubOrgConfig single-match behavior', () => { + const settings = createSettings({}) + settings.subOrgConfigs = { + 'frontend-*': { source: '.github/suborgs/frontend.yml', tag: 'A' } + } + const before = settings.getSubOrgConfig('frontend-app') + settings.getAllMatchingSubOrgSources('frontend-app') + const after = settings.getSubOrgConfig('frontend-app') + expect(after).toBe(before) + expect(after.tag).toBe('A') + }) + }) + + describe('shouldConsiderReevaluation', () => { + let settings + const repo = { owner: 'o', repo: 'foo' } + beforeEach(() => { + settings = createSettings({}) + settings.repoConfigs = {} + }) + + describe('with changeSignals (preferred path)', () => { + it('returns true when teams plugin reported changes', () => { + expect(settings.shouldConsiderReevaluation(repo, null, { teamsChanged: true })).toBe(true) + }) + + it('returns true when custom_properties plugin reported changes', () => { + expect(settings.shouldConsiderReevaluation(repo, null, { propertiesChanged: true })).toBe(true) + }) + + it('returns true on repository rename', () => { + expect(settings.shouldConsiderReevaluation(repo, null, { renamed: true })).toBe(true) + }) + + it('returns true on repository create', () => { + expect(settings.shouldConsiderReevaluation(repo, null, { created: true })).toBe(true) + }) + + it('returns false when all change signals are false (steady state)', () => { + // Pre-existing team that is already on the repo -> diffable reports no + // changes -> we must NOT trigger a re-eval reload. + settings.repoConfigs = { 'foo.yml': { teams: [{ name: 'core' }] } } + const signals = { teamsChanged: false, propertiesChanged: false, renamed: false, created: false } + expect(settings.shouldConsiderReevaluation(repo, { name: 'foo' }, signals)).toBe(false) + }) + }) + + describe('without changeSignals (fallback)', () => { + it('returns false when there is no repo-yml entry', () => { + expect(settings.shouldConsiderReevaluation(repo, null)).toBe(false) + expect(settings.shouldConsiderReevaluation(repo, undefined)).toBe(false) + }) + + it('returns false when repo-yml has no teams/properties and no rename', () => { + settings.repoConfigs = { 'foo.yml': { repository: { name: 'foo' } } } + expect(settings.shouldConsiderReevaluation(repo, { name: 'foo' })).toBe(false) + }) + + it('returns true when repo-yml has teams', () => { + settings.repoConfigs = { 'foo.yml': { teams: [{ name: 'core' }] } } + expect(settings.shouldConsiderReevaluation(repo, { name: 'foo' })).toBe(true) + }) + + it('returns true when repo-yml has custom_properties', () => { + settings.repoConfigs = { 'foo.yaml': { custom_properties: [{ name: 'EDP', value: 'true' }] } } + expect(settings.shouldConsiderReevaluation(repo, { name: 'foo' })).toBe(true) + }) + + it('returns true on rename via repo.oldname', () => { + expect(settings.shouldConsiderReevaluation({ owner: 'o', repo: 'new', oldname: 'old' }, null)).toBe(true) + }) + + it('returns true on rename via repoConfig.oldname', () => { + expect(settings.shouldConsiderReevaluation(repo, { name: 'new', oldname: 'old' })).toBe(true) + }) + }) + }) + + describe('maybeReevaluateSuborg', () => { + it('is a no-op when reevaluateOnChange is false', async () => { + const settings = createSettings({}) + settings.reevaluateOnChange = false + settings.repoConfigs = { 'r.yml': { teams: [{ name: 'core' }] } } + const reloadSpy = jest.spyOn(settings, 'reloadSubOrgConfigs').mockResolvedValue() + await settings.maybeReevaluateSuborg({ owner: 'o', repo: 'r' }, { name: 'r' }, new Set()) + expect(reloadSpy).not.toHaveBeenCalled() + }) + + it('is a no-op when repo-yml has no triggers (teams/properties/rename)', async () => { + const settings = createSettings({}) + settings.reevaluateOnChange = true + settings.repoConfigs = { 'r.yml': { repository: { name: 'r' } } } + const reloadSpy = jest.spyOn(settings, 'reloadSubOrgConfigs').mockResolvedValue() + await settings.maybeReevaluateSuborg({ owner: 'o', repo: 'r' }, { name: 'r' }, new Set()) + expect(reloadSpy).not.toHaveBeenCalled() + }) + + it('is a no-op when changeSignals report no plugin changes (preexisting team)', async () => { + const settings = createSettings({}) + settings.reevaluateOnChange = true + // repo-yml has teams, but plugin reported no change (team already on repo) + settings.repoConfigs = { 'r.yml': { teams: [{ name: 'core' }] } } + const reloadSpy = jest.spyOn(settings, 'reloadSubOrgConfigs').mockResolvedValue() + const updateSpy = jest.spyOn(settings, 'updateRepos').mockResolvedValue() + const signals = { teamsChanged: false, propertiesChanged: false, renamed: false, created: false } + await settings.maybeReevaluateSuborg({ owner: 'o', repo: 'r' }, { name: 'r' }, new Set(), signals) + expect(reloadSpy).not.toHaveBeenCalled() + expect(updateSpy).not.toHaveBeenCalled() + }) + + it('stops when the matched suborg source set is stable (no new sources)', async () => { + const settings = createSettings({}) + settings.reevaluateOnChange = true + settings.subOrgConfigs = { 'r*': { source: '.github/suborgs/x.yml' } } + const updateSpy = jest.spyOn(settings, 'updateRepos').mockResolvedValue() + jest.spyOn(settings, 'reloadSubOrgConfigs').mockResolvedValue() + // pre = post = {x.yml} -> stable, no recursion + const pre = new Set(['.github/suborgs/x.yml']) + await settings.maybeReevaluateSuborg({ owner: 'o', repo: 'r1' }, { name: 'r1' }, pre, { teamsChanged: true }) + expect(updateSpy).not.toHaveBeenCalled() + }) + + it('recurses once when a new suborg source appears, then stops at depth cap', async () => { + const settings = createSettings({}) + settings.reevaluateOnChange = true + // After reload, a new suborg matches r1 + settings.subOrgConfigs = { 'r*': { source: '.github/suborgs/new.yml' } } + settings.repoConfigs = { 'r1.yml': { teams: [{ name: 't' }] } } + jest.spyOn(settings, 'reloadSubOrgConfigs').mockResolvedValue() + jest.spyOn(settings, 'getRepoConfigs').mockResolvedValue({ 'r1.yml': { teams: [{ name: 't' }] } }) + const updateSpy = jest.spyOn(settings, 'updateRepos').mockResolvedValue() + const pre = new Set() // pre-apply: nothing matched + await settings.maybeReevaluateSuborg({ owner: 'o', repo: 'r1' }, { name: 'r1' }, pre, { teamsChanged: true }) + expect(updateSpy).toHaveBeenCalledTimes(1) + expect(settings.reevaluationDepth.get('r1')).toBe(1) + }) + + it('respects MAX_REEVALUATION_DEPTH and logs a warning', async () => { + const settings = createSettings({}) + settings.reevaluateOnChange = true + settings.reevaluationDepth.set('r1', 1) // already at cap + settings.repoConfigs = { 'r1.yml': { teams: [{ name: 't' }] } } + stubContext.log.warn = jest.fn() + const reloadSpy = jest.spyOn(settings, 'reloadSubOrgConfigs').mockResolvedValue() + const updateSpy = jest.spyOn(settings, 'updateRepos').mockResolvedValue() + await settings.maybeReevaluateSuborg({ owner: 'o', repo: 'r1' }, { name: 'r1' }, new Set(), { teamsChanged: true }) + expect(reloadSpy).not.toHaveBeenCalled() + expect(updateSpy).not.toHaveBeenCalled() + expect(stubContext.log.warn).toHaveBeenCalledWith(expect.stringContaining('max depth')) + }) + }) }) // Settings Tests From baaa9d5c2c0c0ec87e658d910b5cc61f46569919 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Tue, 19 May 2026 14:19:41 -0400 Subject: [PATCH 11/17] Add external group linking functionality for teams and update smoke tests --- docs/github-settings/4. teams.md | 13 ++ lib/plugins/teams.js | 137 ++++++++++++++++++++ smoke-test.js | 127 ++++++++++++++++++- test/unit/lib/plugins/teams.test.js | 186 ++++++++++++++++++++++++++++ 4 files changed, 462 insertions(+), 1 deletion(-) diff --git a/docs/github-settings/4. teams.md b/docs/github-settings/4. teams.md index 496b30a32..56f1a4e86 100644 --- a/docs/github-settings/4. teams.md +++ b/docs/github-settings/4. teams.md @@ -48,5 +48,18 @@ teams: permission: maintain ``` + + +

external_groupstring

+

Optional. The display name of an external IdP group (as listed under your organization's external groups) to link to the team. safe-settings looks up the group's id by display name via GET /orgs/{org}/external-groups and links the team via PATCH /orgs/{org}/teams/{team_slug}/external-groups. The link is reconciled on every sync and is idempotent (it skips the PATCH when the team is already linked to the same group). The external-groups list is fetched at most once per org per sync, only when at least one team entry uses this property. If the named group does not exist for the org, an error is logged and the team-repo association still applies.

+ + +```yaml +teams: + - name: expert-services-developers + permission: push + external_group: "Engineering - Expert Services" +``` + diff --git a/lib/plugins/teams.js b/lib/plugins/teams.js index 4d7f79273..0d655363a 100644 --- a/lib/plugins/teams.js +++ b/lib/plugins/teams.js @@ -2,7 +2,30 @@ const Diffable = require('./diffable') const NopCommand = require('../nopcommand') const teamRepoEndpoint = '/orgs/:owner/teams/:team_slug/repos/:owner/:repo' +const listExternalGroupsEndpoint = 'GET /orgs/{org}/external-groups' +const teamExternalGroupsEndpoint = '/orgs/{org}/teams/{team_slug}/external-groups' + module.exports = class Teams extends Diffable { + // Override Diffable.sync to also reconcile the optional `external_group` + // link on each team entry after the normal team-repo permission sync. + // This runs regardless of whether the team-repo association was added, + // updated, or already in sync -- so updating only `external_group` on a + // team that already has correct repo permissions still triggers the link. + async sync () { + const res = await super.sync() + if (!this.entries) return res + + const filtered = this.filterEntries() + const entriesWithExternalGroup = filtered.filter(e => e && e.external_group) + if (entriesWithExternalGroup.length === 0) return res + + const nopCommands = Array.isArray(res) ? res : [] + for (const attrs of entriesWithExternalGroup) { + await this.syncExternalGroup(attrs, this.nop ? nopCommands : undefined) + } + return this.nop ? nopCommands : res + } + async find () { this.log.debug(`Finding teams for ${this.repo.owner}/${this.repo.repo}`) return this.github.paginate(this.github.repos.listTeams, this.repo).then(res => { @@ -138,4 +161,118 @@ module.exports = class Teams extends Diffable { permission: attrs.permission } } + + // Resolve the org's external-group display name -> group_id. Lazily builds + // a per-org Map (name -> id) the first time it's needed within a sync, and + // caches it on the shared `github` client so multiple repos / teams in the + // same sync only paginate `GET /orgs/{org}/external-groups` once per org. + // Returns null when the named group does not exist for the org (logs an + // error so the user can correct their yaml). + async resolveExternalGroupId (groupName) { + if (!this.github.__externalGroupsCache) { + this.github.__externalGroupsCache = new Map() + } + const cache = this.github.__externalGroupsCache + const org = this.repo.owner + if (!cache.has(org)) { + try { + // The external-groups endpoint returns { total_count, groups: [...] } + // and is not in Octokit's known-pagination list, so we must pass a + // map function that extracts the `groups` array from each page; + // otherwise paginate() yields the raw response objects and we'd + // silently fail to find any names. + const groups = await this.github.paginate( + listExternalGroupsEndpoint, + { org, per_page: 100 }, + (response) => (response && response.data && response.data.groups) || [] + ) + const byName = new Map() + for (const g of groups) { + if (g && g.group_name) byName.set(g.group_name, g.group_id) + } + this.log.debug(`Loaded ${byName.size} external group(s) for org ${org}: ${JSON.stringify(Array.from(byName.keys()))}`) + cache.set(org, byName) + } catch (e) { + this.logError(`Error listing external groups for org ${org}: ${e}`) + // Cache an empty map so we don't retry-storm the API within this sync. + cache.set(org, new Map()) + } + } + const id = cache.get(org).get(groupName) + if (id === undefined) { + return null + } + return id + } + + // Link a team to an external IdP group identified by display name. Only + // acts when the team entry carries an `external_group` property. Idempotent: + // checks the current link first and skips the PATCH if already linked to + // the same group_id. Sets `this.hasChanges = true` only when a PATCH + // actually fires, so the suborg re-evaluation logic in lib/settings.js sees + // a real change signal. + async syncExternalGroup (attrs, nopCommands) { + const groupName = attrs && attrs.external_group + if (!groupName) return + + const groupId = await this.resolveExternalGroupId(groupName) + if (groupId === null) { + const msg = `External group '${groupName}' not found for org ${this.repo.owner} (team '${attrs.name}').` + // logError: feeds the synchronous-run end-of-run errors summary. + this.logError(msg) + // For PR dry-run / nop mode, also surface the failure in the check_run + // output -- which is built from results entries with type === 'ERROR'. + if (this.nop && Array.isArray(nopCommands)) { + nopCommands.push(new NopCommand(this.constructor.name, this.repo, null, msg, 'ERROR')) + } + return + } + + const linkParams = { + org: this.repo.owner, + team_slug: attrs.name, + group_id: groupId + } + + if (this.nop) { + if (Array.isArray(nopCommands)) { + nopCommands.push(new NopCommand( + this.constructor.name, + this.repo, + this.github.request.endpoint(`PATCH ${teamExternalGroupsEndpoint}`, linkParams), + `Link team ${attrs.name} to external group '${groupName}'` + )) + } + return + } + + // Idempotency: skip the PATCH if the team is already linked to this group. + try { + const current = await this.github.request(`GET ${teamExternalGroupsEndpoint}`, { + org: this.repo.owner, + team_slug: attrs.name + }) + const currentGroups = (current && current.data && current.data.groups) || [] + if (currentGroups.some(g => g.group_id === groupId)) { + this.log.debug(`Team ${attrs.name} is already linked to external group '${groupName}' (id=${groupId}); skipping.`) + return + } + } catch (e) { + // 404 here means no current link; fall through to PATCH. Any other + // error is non-fatal -- the PATCH itself is idempotent on the server. + if (e.status !== 404) { + this.logError(`Error fetching current external group for team ${attrs.name}: ${e}`) + } + } + + try { + await this.github.request(`PATCH ${teamExternalGroupsEndpoint}`, linkParams) + this.log.debug(`Linked team ${attrs.name} to external group '${groupName}' (id=${groupId}).`) + // Surface this change so suborg re-evaluation (in lib/settings.js) and + // other consumers see that the team plugin made a real change. + this.hasChanges = true + } catch (e) { + this.logError(`Error linking team ${attrs.name} to external group '${groupName}' (id=${groupId}): ${e}`) + } + } } diff --git a/smoke-test.js b/smoke-test.js index dcb4df8af..d881972da 100644 --- a/smoke-test.js +++ b/smoke-test.js @@ -66,7 +66,7 @@ const APP_ID = process.env.APP_ID const PRIVATE_KEY = (process.env.PRIVATE_KEY || '').replace(/\\n/g, '\n') const TEST_REPOS = ['test', 'demo-repo-service1', 'demo-repo-service2'] -const TEST_TEAMS = ['AD-GRP-PAYMENTS-PLATFORM-OWNERS', 'awesometeam-a-approvers'] +const TEST_TEAMS = ['AD-GRP-PAYMENTS-PLATFORM-OWNERS', 'awesometeam-a-approvers', 'jefeish-edj-test'] const POLL_INTERVAL_MS = 5000 const MAX_POLL_MS = 120000 @@ -453,6 +453,49 @@ teams: permission: push ` +const REPO_DEMO_SERVICE2_EXTERNAL_GROUP_YML = `# Safe-Settings Configuration +repository: + name: demo-repo-service2 + description: "Repository 2 sample" + visibility: private + default_branch: main + homepage: "" + auto_init: true + force_create: true + delete_branch_on_merge: true + archived: false + topics: + - topic1 + - topic2 + +teams: + - name: expert-services-developers + permission: push + - name: jefeish-edj-test + permission: push + external_group: jefeish-edj-test +` + +const REPO_DEMO_SERVICE2_NO_EXTERNAL_GROUP_YML = `# Safe-Settings Configuration +repository: + name: demo-repo-service2 + description: "Repository 2 sample" + visibility: private + default_branch: main + homepage: "" + auto_init: true + force_create: true + delete_branch_on_merge: true + archived: false + topics: + - topic1 + - topic2 + +teams: + - name: expert-services-developers + permission: push +` + const SETTINGS_YML_ORG = `# Org-level safe-settings configuration rulesets: @@ -801,6 +844,87 @@ async function phase7DemoRepo2 () { await deleteBranch(ORG, ADMIN_REPO, branch) } +async function phase7bExternalGroupTeam () { + logPhase('Phase 7b: Add team with external_group to demo-repo-service2') + const branch = 'smoke-test-phase7b' + const defaultBranch = await getDefaultBranch() + + // ── Step 1: Add the team with external_group mapping ── + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/demo-repo-service2.yml`, REPO_DEMO_SERVICE2_EXTERNAL_GROUP_YML, branch, 'Add team with external_group to demo-repo-service2') + + const pr1 = await createPR(ORG, ADMIN_REPO, 'Smoke test: add external_group team to demo-repo-service2', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun1 = await waitForCheckRun(ORG, ADMIN_REPO, pr1.head.sha) + assert(checkRun1 !== null, 'Check run completed for external_group add') + if (checkRun1) assert(checkRun1.conclusion === 'success', `Check run conclusion is success (got: ${checkRun1.conclusion})`) + + log('Merging PR...') + await mergePR(ORG, ADMIN_REPO, pr1.number) + await sleep(WEBHOOK_SETTLE_MS) + + // Verify team is created and assigned to the repo + log('Checking team jefeish-edj-test is added to demo-repo-service2...') + const team = await poll(async () => { + try { + const { data: teams } = await octokit.rest.repos.listTeams({ owner: ORG, repo: 'demo-repo-service2' }) + return teams.find(t => t.slug === 'jefeish-edj-test') || null + } catch { return null } + }, { desc: 'team jefeish-edj-test to be added to demo-repo-service2' }) + + assert(team !== null, 'Team jefeish-edj-test added to demo-repo-service2') + + // Verify the external group (IdP) mapping exists on the team + log('Checking external group mapping on team jefeish-edj-test...') + const externalGroup = await poll(async () => { + try { + const { data } = await octokit.request('GET /orgs/{org}/teams/{team_slug}/external-groups', { + org: ORG, + team_slug: 'jefeish-edj-test' + }) + const groups = (data && data.groups) || [] + return groups.find(g => g.group_name === 'jefeish-edj-test') || null + } catch { return null } + }, { desc: 'external group mapping on jefeish-edj-test', timeout: 60000 }) + + assert(externalGroup !== null, 'External group jefeish-edj-test mapped to team jefeish-edj-test') + + await deleteBranch(ORG, ADMIN_REPO, branch) + + // ── Step 2: Remove the team from the YAML and verify removal ── + log('Removing team jefeish-edj-test from demo-repo-service2 config...') + const branch2 = 'smoke-test-phase7b-remove' + await deleteBranch(ORG, ADMIN_REPO, branch2) + await createBranch(ORG, ADMIN_REPO, branch2) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/demo-repo-service2.yml`, REPO_DEMO_SERVICE2_NO_EXTERNAL_GROUP_YML, branch2, 'Remove external_group team from demo-repo-service2') + + const pr2 = await createPR(ORG, ADMIN_REPO, 'Smoke test: remove external_group team from demo-repo-service2', branch2, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun2 = await waitForCheckRun(ORG, ADMIN_REPO, pr2.head.sha) + assert(checkRun2 !== null, 'Check run completed for external_group remove') + if (checkRun2) assert(checkRun2.conclusion === 'success', `Check run conclusion is success (got: ${checkRun2.conclusion})`) + + log('Merging PR...') + await mergePR(ORG, ADMIN_REPO, pr2.number) + await sleep(WEBHOOK_SETTLE_MS) + + // Verify team is removed from the repo + log('Checking team jefeish-edj-test is removed from demo-repo-service2...') + const removedTeam = await poll(async () => { + try { + const { data: teams } = await octokit.rest.repos.listTeams({ owner: ORG, repo: 'demo-repo-service2' }) + return teams.find(t => t.slug === 'jefeish-edj-test') ? false : true + } catch { return null } + }, { desc: 'team jefeish-edj-test to be removed from demo-repo-service2' }) + + assert(removedTeam === true, 'Team jefeish-edj-test removed from demo-repo-service2') + + await deleteBranch(ORG, ADMIN_REPO, branch2) +} + async function phase8OrgSettings () { logPhase('Phase 8: Org-level settings') const branch = 'smoke-test-phase8' @@ -913,6 +1037,7 @@ async function main () { await phase5Suborg() await phase6Archive() await phase7DemoRepo2() + await phase7bExternalGroupTeam() await phase8OrgSettings() } catch (err) { console.error(`\x1b[31mFatal error: ${err.message}\x1b[0m`) diff --git a/test/unit/lib/plugins/teams.test.js b/test/unit/lib/plugins/teams.test.js index 60ef23dbc..a521e0ce3 100644 --- a/test/unit/lib/plugins/teams.test.js +++ b/test/unit/lib/plugins/teams.test.js @@ -96,4 +96,190 @@ describe('Teams', () => { ) } }) + + describe('external_group linking', () => { + const externalGroupName = 'Engineering - Expert Services' + const externalGroupId = 42 + + beforeEach(() => { + // request: default to no-current-link (404) so PATCH fires; override per-test as needed. + github.request = jest.fn().mockImplementation((endpoint) => { + if (typeof endpoint === 'string' && endpoint.startsWith('GET /orgs/{org}/teams/')) { + const err = new Error('not found') + err.status = 404 + return Promise.reject(err) + } + return Promise.resolve({ data: {} }) + }) + github.request.endpoint = jest.fn().mockReturnValue('endpoint-stub') + + // paginate: route the external-groups list call to a single page; keep + // the original implementation for other paginated endpoints. The real + // production code passes a map-function (3rd arg) that extracts the + // `groups` array from each page response -- we mimic the same response + // shape so that mapFn gets exercised. + const externalGroupsResponse = { + data: { + total_count: 2, + groups: [ + { group_id: externalGroupId, group_name: externalGroupName }, + { group_id: 99, group_name: 'Some Other Group' } + ] + } + } + github.paginate = jest.fn().mockImplementation(async (fetchOrEndpoint, params, mapFn) => { + if (fetchOrEndpoint === 'GET /orgs/{org}/external-groups') { + if (typeof mapFn === 'function') { + return mapFn(externalGroupsResponse) + } + return externalGroupsResponse.data.groups + } + if (typeof fetchOrEndpoint === 'function') { + const response = await fetchOrEndpoint() + return response.data + } + return [] + }) + }) + + it('looks up the group id by name and PATCHes the team link', async () => { + when(github.teams.getByName) + .defaultResolvedValue({}) + .calledWith({ org, team_slug: addedTeamName }) + .mockResolvedValue({ data: { id: addedTeamId } }) + + const plugin = configure([ + { name: unchangedTeamName, permission: 'push' }, + { name: addedTeamName, permission: 'pull', external_group: externalGroupName } + ]) + + await plugin.sync() + + expect(github.paginate).toHaveBeenCalledWith( + 'GET /orgs/{org}/external-groups', + { org, per_page: 100 }, + expect.any(Function) + ) + expect(github.request).toHaveBeenCalledWith( + 'PATCH /orgs/{org}/teams/{team_slug}/external-groups', + { org, team_slug: addedTeamName, group_id: externalGroupId } + ) + expect(plugin.hasChanges).toBe(true) + }) + + it('skips the PATCH when the team is already linked to the same group', async () => { + github.request = jest.fn().mockImplementation((endpoint, params) => { + if (endpoint === 'GET /orgs/{org}/teams/{team_slug}/external-groups') { + return Promise.resolve({ data: { groups: [{ group_id: externalGroupId, group_name: externalGroupName }] } }) + } + return Promise.resolve({ data: {} }) + }) + github.request.endpoint = jest.fn().mockReturnValue('endpoint-stub') + + const plugin = configure([ + { name: unchangedTeamName, permission: 'push', external_group: externalGroupName } + ]) + + await plugin.sync() + + expect(github.request).toHaveBeenCalledWith( + 'GET /orgs/{org}/teams/{team_slug}/external-groups', + { org, team_slug: unchangedTeamName } + ) + expect(github.request).not.toHaveBeenCalledWith( + 'PATCH /orgs/{org}/teams/{team_slug}/external-groups', + expect.anything() + ) + }) + + it('logs an error and skips when the external group name is not found', async () => { + const plugin = configure([ + { name: unchangedTeamName, permission: 'push', external_group: 'Nonexistent Group' } + ]) + + await plugin.sync() + + expect(github.request).not.toHaveBeenCalledWith( + 'PATCH /orgs/{org}/teams/{team_slug}/external-groups', + expect.anything() + ) + // logError pushes onto the errors array + expect(plugin.errors.some(e => /Nonexistent Group/.test(JSON.stringify(e)))).toBe(true) + }) + + it('in nop mode, emits an ERROR NopCommand when the external group is not found (so it appears in the PR check_run)', async () => { + const log = { debug: jest.fn(), error: console.error } + const errors = [] + const Teams = require('../../../../lib/plugins/teams') + const plugin = new Teams(true, github, { owner: org, repo: 'test' }, [ + { name: unchangedTeamName, permission: 'push', external_group: 'Nonexistent Group' } + ], log, errors) + + const result = await plugin.sync() + + expect(Array.isArray(result)).toBe(true) + const errorCmd = result.find(c => c && c.type === 'ERROR' && /Nonexistent Group/.test(JSON.stringify(c))) + expect(errorCmd).toBeDefined() + expect(github.request).not.toHaveBeenCalledWith( + 'PATCH /orgs/{org}/teams/{team_slug}/external-groups', + expect.anything() + ) + }) + + it('paginates the external-groups list only once per org across multiple syncs sharing the github client', async () => { + when(github.teams.getByName) + .defaultResolvedValue({}) + .calledWith({ org, team_slug: addedTeamName }) + .mockResolvedValue({ data: { id: addedTeamId } }) + + const plugin1 = configure([ + { name: unchangedTeamName, permission: 'push', external_group: externalGroupName } + ]) + const plugin2 = configure([ + { name: addedTeamName, permission: 'pull', external_group: externalGroupName } + ]) + + await plugin1.sync() + await plugin2.sync() + + const listCalls = github.paginate.mock.calls.filter(c => c[0] === 'GET /orgs/{org}/external-groups') + expect(listCalls).toHaveLength(1) + }) + + it('does not call the external-groups list endpoint when no entry uses external_group', async () => { + const plugin = configure([ + { name: unchangedTeamName, permission: 'push' } + ]) + + await plugin.sync() + + const listCalls = github.paginate.mock.calls.filter(c => c[0] === 'GET /orgs/{org}/external-groups') + expect(listCalls).toHaveLength(0) + }) + + it('in nop mode, emits a NopCommand and makes no PATCH', async () => { + const log = { debug: jest.fn(), error: console.error } + const errors = [] + const Teams = require('../../../../lib/plugins/teams') + const plugin = new Teams(true, github, { owner: org, repo: 'test' }, [ + { name: unchangedTeamName, permission: 'push', external_group: externalGroupName } + ], log, errors) + + const result = await plugin.sync() + + expect(Array.isArray(result)).toBe(true) + expect(result.some(c => /external group/.test(c.action) || /external group/.test(JSON.stringify(c)))).toBe(true) + // In nop mode no real linkage should be performed -- neither the + // idempotency GET nor the PATCH should hit the team-external-groups + // endpoint. + expect(github.request).not.toHaveBeenCalledWith( + 'PATCH /orgs/{org}/teams/{team_slug}/external-groups', + expect.anything() + ) + expect(github.request).not.toHaveBeenCalledWith( + 'GET /orgs/{org}/teams/{team_slug}/external-groups', + expect.anything() + ) + }) + }) }) From 6ca72a7e43d97be125194e73d857e17752ff1a75 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Sat, 23 May 2026 21:42:51 -0400 Subject: [PATCH 12/17] feat: add disable_plugins configuration to settings schema - Introduced a new "disable_plugins" property in the settings schema to allow disabling specific plugins at various configuration layers. - Each entry can be a plugin name or an object specifying the plugin and its target layer (self, children, all). - Updated smoke-test.js to include interactive mode for manual validation during test phases. - Implemented new test cases for the disable_plugins feature, covering normalization, strip map computation, and integration with updateOrg and updateRepos functions. - Added tests to ensure proper handling of valid and invalid disable_plugins configurations. --- README.md | 76 ++++ docs/README.md | 2 + .../sample-deployment-settings.yml | 11 + docs/sample-settings/settings.yml | 14 + docs/sample-settings/suborg.yml | 12 + lib/commentmessage.js | 14 + lib/settings.js | 337 +++++++++++++-- package.json | 5 +- schema/dereferenced/settings.json | 398 +++++++++++++----- schema/settings.json | 58 +++ smoke-test.js | 253 +++++++++-- test/unit/lib/settings.test.js | 365 ++++++++++++++++ 12 files changed, 1378 insertions(+), 167 deletions(-) diff --git a/README.md b/README.md index b7382de34..892f97bb8 100644 --- a/README.md +++ b/README.md @@ -472,6 +472,82 @@ And the `checkrun` page will look like this: image

+### Disabling plugins (`disable_plugins`) + +Any settings file (deployment-settings, org `settings.yml`, suborg, or repo) can +contain a top-level `disable_plugins` list to turn off one or more safe-settings +plugins for a given scope. Each entry is either: + +- A plugin name string (shorthand for `{ plugin: , target: all }`), or +- An object `{ plugin: , target: self | children | all }` (default `target: all`). + +Valid plugin names: `repository`, `labels`, `collaborators`, `teams`, +`milestones`, `branches`, `autolinks`, `validator`, `rulesets`, `environments`, +`custom_properties`, `custom_repository_roles`, `variables`, `archive`. + +#### Strip matrix (which source layers are removed before merge) + +| Declared at | `target: self` | `target: children` | `target: all` | +| -------------------------- | ------------------ | ------------------------- | ----------------------------- | +| deployment-settings | deployment | org + suborg + repo | deployment + org + suborg + repo | +| org `settings.yml` | org | suborg + repo | org + suborg + repo | +| suborgs/`*.yml` (matched) | suborg | repo | suborg + repo | +| repos/`*.yml` | repo | (no-op) | repo | + +When safe-settings builds the merged configuration for a repo, it strips the +disabled plugin's keys from the indicated source layers before merging. For +repo-level execution points (the `repository` and `archive` plugins) and +org-level execution points (`rulesets`, `custom_repository_roles`), a disable +that targets the corresponding layer also short-circuits the plugin run, and +the skip is recorded as an INFO `NopCommand` in NOP mode (PR check run). + +#### Cascade rules + +- **Union-only.** Strips accumulate across layers; a lower-level config can add + more strips but can never undo a strip declared above it. +- **No re-enable.** If `disable_plugins: [labels]` is set at the org layer, a + repo cannot re-enable `labels` for itself. + +#### Important limitation + +Because strips operate on **source layers**, a lower-level disable cannot +remove configuration contributed by a higher layer. For example, if `branches` +is defined at the org layer and a suborg adds +`disable_plugins: [{plugin: branches, target: all}]`, the suborg's strip +removes the `branches` key only from the suborg and repo layers — the org's +`branches` config still merges in, and the branches plugin still runs. + +To fully suppress a plugin for matched repos, declare the disable at (or above) +the layer that contributes the configuration — typically the org layer with +`target: all`, or at the deployment layer. + +#### Examples + +Org `settings.yml` — disable `custom_repository_roles` only at the org execution +point (rulesets still run): + +```yaml +disable_plugins: + - plugin: custom_repository_roles + target: self +``` + +Org `settings.yml` — disable `branches` everywhere (shorthand): + +```yaml +disable_plugins: + - branches +``` + +Suborg `suborgs/team-x.yml` — strip `labels` for matched repos (effective only +if `labels` is not also defined at the org layer): + +```yaml +disable_plugins: + - plugin: labels + target: all +``` + ### The Settings Files The settings files can be used to set the policies at the `org`, `suborg` or `repo` level. diff --git a/docs/README.md b/docs/README.md index 6d1f17436..76c9ee10f 100644 --- a/docs/README.md +++ b/docs/README.md @@ -10,3 +10,5 @@ | Configure deployment environments | [Deployment Environments](github-settings/6.%20deployment-environments.md) | | Configure auto-link references | [AutoLinks](github-settings/7.%20autolinks.md) | | Configure pre-defined labels for issues and pull requests | [Labels](github-settings/8.%20labels.md) | + +For information on disabling plugins, see [Disabling plugins](../README.md#disabling-plugins-disable_plugins) in the root README. diff --git a/docs/sample-settings/sample-deployment-settings.yml b/docs/sample-settings/sample-deployment-settings.yml index 6164d4389..f3c6e8b37 100644 --- a/docs/sample-settings/sample-deployment-settings.yml +++ b/docs/sample-settings/sample-deployment-settings.yml @@ -38,3 +38,14 @@ overridevalidators: Some error script: | return true + +# disable_plugins (optional) — disable safe-settings plugins at the deployment layer. +# Each entry is either a plugin name (shorthand for target: all) or { plugin, target }. +# target is one of: self | children | all (default: all). +# Declared here, target: all strips the plugin from every level below for every repo. +# See docs/README.md ("Disabling plugins") for the full strip matrix and limitations. +# +# disable_plugins: +# - plugin: rulesets # disables rulesets everywhere +# target: all +# - milestones # shorthand → { plugin: milestones, target: all } diff --git a/docs/sample-settings/settings.yml b/docs/sample-settings/settings.yml index 7e19d3354..fadd9f664 100644 --- a/docs/sample-settings/settings.yml +++ b/docs/sample-settings/settings.yml @@ -397,3 +397,17 @@ rulesets: negate: false operator: regex pattern: ".*\/.*" + +# disable_plugins (optional) — disable safe-settings plugins at the org layer. +# Declared here: +# - target: self → strips from the org layer only (affects org-level runs: +# rulesets, custom_repository_roles). +# - target: children → strips from suborg + repo layers (per-repo runs). +# - target: all → strips from org + suborg + repo layers. +# Lower levels can never undo a strip declared at a higher level (union-only cascade). +# See docs/README.md ("Disabling plugins") for the full strip matrix. +# +# disable_plugins: +# - plugin: custom_repository_roles +# target: self +# - branches # shorthand → { plugin: branches, target: all } diff --git a/docs/sample-settings/suborg.yml b/docs/sample-settings/suborg.yml index a509847cc..42e822993 100644 --- a/docs/sample-settings/suborg.yml +++ b/docs/sample-settings/suborg.yml @@ -14,3 +14,15 @@ suborgproperties: - EDP: true # Every other property is the same as the org level settings and can be overridden here + +# disable_plugins (optional) — disable safe-settings plugins for repos matched +# by this suborg. Declared here, target values mean: +# - self → strip from the suborg layer only. +# - children → strip from the repo layer for matched repos. +# - all → strip from suborg + repo layers for matched repos. +# Note: a suborg-level disable cannot strip config defined at the org layer. +# See docs/README.md ("Disabling plugins") for details. +# +# disable_plugins: +# - plugin: labels +# target: all diff --git a/lib/commentmessage.js b/lib/commentmessage.js index b54f81bb4..66f4862c8 100644 --- a/lib/commentmessage.js +++ b/lib/commentmessage.js @@ -27,5 +27,19 @@ module.exports = `* Run on: \` <%= new Date() %> \` * <%= plugin.msg %> <% }) %> + <% }) %> +<% } %> + +### Informational messages (disabled plugins) + +<% if (!it.infos || Object.keys(it.infos).length === 0) { %> +\`None\` +<% } else { %> + <% Object.keys(it.infos).forEach(repo => { %> + <%_= repo %>: + <% it.infos[repo].forEach(msg => { %> + * ℹ️ <%= msg %> + <% }) %> + <% }) %> <% } %>` diff --git a/lib/settings.js b/lib/settings.js index d104efa5b..f1584fa97 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -6,7 +6,37 @@ const Glob = require('./glob') const NopCommand = require('./nopcommand') const MergeDeep = require('./mergeDeep') const Archive = require('./plugins/archive') +const DeploymentConfig = require('./deploymentConfig') const env = require('./env') + +// Valid `target` values for a disable_plugins entry. +const DISABLE_TARGETS = new Set(['self', 'children', 'all']) +// Valid declaration layers (where a disable_plugins entry can be authored). +const DISABLE_LEVELS = ['deployment', 'org', 'suborg', 'repo'] +// For each declared layer + target, the set of layers from which to STRIP the +// named plugin's config. See plan-v3 matrix. +const DISABLE_STRIP_MATRIX = { + deployment: { + self: ['deployment'], + children: ['org', 'suborg', 'repo'], + all: ['deployment', 'org', 'suborg', 'repo'] + }, + org: { + self: ['org'], + children: ['suborg', 'repo'], + all: ['org', 'suborg', 'repo'] + }, + suborg: { + self: ['suborg'], + children: ['repo'], + all: ['suborg', 'repo'] + }, + repo: { + self: ['repo'], + children: ['repo'], // normalized; repo has no children + all: ['repo'] + } +} const CONFIG_PATH = env.CONFIG_PATH const eta = new Eta({ views: path.join(__dirname) }) const SCOPE = { ORG: 'org', REPO: 'repo' } // Determine if the setting is a org setting or repo setting @@ -203,6 +233,14 @@ class Settings { msg, plugin: this.constructor.name }) + // In NOP mode, also surface the error as an ERROR NopCommand so the NOP + // check run conclusion reflects the failure. Without this, errors caught + // by the syncAll/syncSelectedRepos top-level catch (e.g. invalid + // disable_plugins entries) would go unnoticed by PR reviewers. + if (this.nop) { + const nopcommand = new NopCommand(this.constructor.name, this.repo, null, msg, 'ERROR') + this.appendToResults([nopcommand]) + } } async handleResults () { @@ -215,43 +253,42 @@ class Settings { return } - // remove duplicate rows in this.results + // Remove duplicate rows. The key includes endpoint so that distinct + // per-operation NopCommands (individual add/update/remove from diffable + // plugins) survive alongside the overall diff-summary NopCommand. this.results = this.results.filter((thing, index, self) => { return index === self.findIndex((t) => { - return t.type === thing.type && t.repo === thing.repo && t.plugin === thing.plugin + return t.type === thing.type && t.repo === thing.repo && t.plugin === thing.plugin && t.endpoint === thing.endpoint }) }) let error = false - // Different logic const stats = { - // noOfReposProcessed: new Map(), reposProcessed: {}, changes: {}, - errors: {} + errors: {}, + // Informational entries (type === 'INFO', all-null diff fields), e.g. + // disable_plugins skip messages. Keyed by repo. + infos: {} } - /* - Result fields - res.type - res.plugin - res.repo - res.endpoint - res.body - res.action - */ this.results.forEach(res => { if (res) { stats.reposProcessed[res.repo] = true - // if (res.action.additions === null && res.action.deletions === null && res.action.modifications === null) { - // // No changes - // } else if (res.type === 'ERROR') { error = true if (!stats.errors[res.repo]) { stats.errors[res.repo] = [] } stats.errors[res.repo].push(res.action) - } else if (!(res.action?.additions === null && res.action?.deletions === null && res.action?.modifications === null)) { + } else if (res.action?.additions === null && res.action?.deletions === null && res.action?.modifications === null) { + // No diff data — informational message (e.g. disable_plugins skip). + if (res.action?.msg) { + if (!stats.infos[res.repo]) { + stats.infos[res.repo] = [] + } + stats.infos[res.repo].push(`[${res.plugin}] ${res.action.msg}`) + } + } else { if (!stats.changes[res.plugin]) { stats.changes[res.plugin] = {} } @@ -292,17 +329,20 @@ ${this.results.reduce((x, y) => { if (y.type === 'ERROR') { error = true return `${x} - ❗ ${y.action.msg} ${y.plugin} ${prettify(y.repo)} ${prettify(y.action.additions)} ${prettify(y.action.deletions)} ${prettify(y.action.modifications)} ` - } else if (y.action.additions === null && y.action.deletions === null && y.action.modifications === null) { - return `${x}` + ❗ ${y.action.msg} ${y.plugin} ${prettify(y.repo)} ${prettify(y.action.additions)} ${prettify(y.action.deletions)} ${prettify(y.action.modifications)} ` + } else if (y.action?.additions === null && y.action?.deletions === null && y.action?.modifications === null) { + if (!y.action?.msg) return `${x}` + return `${x} + ℹ️ ${y.action.msg} ${y.plugin} ${prettify(y.repo)} — — — ` } else { if (y.action === undefined) { return `${x}` } return `${x} - ✋ ${y.plugin} ${prettify(y.repo)} ${prettify(y.action.additions)} ${prettify(y.action.deletions)} ${prettify(y.action.modifications)} ` + ✋ ${y.plugin} ${prettify(y.repo)} ${prettify(y.action.additions)} ${prettify(y.action.deletions)} ${prettify(y.action.modifications)} ` } }, table)} + ` const pullRequest = payload.check_run.check_suite.pull_requests[0] @@ -337,21 +377,197 @@ ${this.results.reduce((x, y) => { this.repoConfigs = await this.getRepoConfigs(repo) } + // ──────────────────────────────────────────────────────────────────────── + // disable_plugins helpers + // ──────────────────────────────────────────────────────────────────────── + + // Returns the set of plugin names that are valid `disable_plugins` targets. + static getValidDisablePluginNames () { + return new Set([...Object.keys(Settings.PLUGINS), 'repository', 'archive']) + } + + // Normalize a raw `disable_plugins` list (mixed strings / objects) into + // [{ plugin, target, declaredAt }]. Validates plugin names and target + // values; throws on invalid entries. For declaredAt='repo', `children` + // collapses to `all` (repo has no children). + normalizeDisableEntries (rawList, declaredAt) { + if (rawList === undefined || rawList === null) return [] + if (!Array.isArray(rawList)) { + throw new Error(`disable_plugins at ${declaredAt} must be an array; got ${typeof rawList}`) + } + if (!DISABLE_LEVELS.includes(declaredAt)) { + throw new Error(`Internal: invalid declaredAt '${declaredAt}'`) + } + const validPlugins = Settings.getValidDisablePluginNames() + const normalized = [] + for (const raw of rawList) { + let plugin + let target = 'all' + if (typeof raw === 'string') { + plugin = raw + } else if (raw && typeof raw === 'object') { + plugin = raw.plugin + if (raw.target !== undefined) target = raw.target + } else { + throw new Error(`disable_plugins entry at ${declaredAt} must be a string or {plugin, target}; got ${JSON.stringify(raw)}`) + } + if (!plugin || typeof plugin !== 'string') { + throw new Error(`disable_plugins entry at ${declaredAt} is missing a valid 'plugin' name: ${JSON.stringify(raw)}`) + } + if (!validPlugins.has(plugin)) { + throw new Error(`disable_plugins at ${declaredAt}: unknown plugin '${plugin}'. Valid: ${[...validPlugins].sort().join(', ')}`) + } + if (!DISABLE_TARGETS.has(target)) { + throw new Error(`disable_plugins at ${declaredAt} for plugin '${plugin}': invalid target '${target}'. Valid: ${[...DISABLE_TARGETS].join(', ')}`) + } + if (declaredAt === 'repo' && target === 'children') { + this.log.debug(`disable_plugins: normalizing repo-level target 'children' to 'all' for plugin '${plugin}' (repo has no children)`) + target = 'all' + } + normalized.push({ plugin, target, declaredAt }) + } + return normalized + } + + // Aggregate disable_plugins entries from all four layers (deployment, org, + // suborg matching repoName, repo override for repoName) and expand them via + // the strip matrix into a Map>. If repoName is + // undefined, only deployment + org layers contribute (used by updateOrg). + computeStripMap (repoName) { + const stripMap = new Map() + for (const level of DISABLE_LEVELS) stripMap.set(level, new Set()) + + const layers = [] + // Deployment layer (singleton) + const deploymentRaw = (DeploymentConfig && DeploymentConfig.config && DeploymentConfig.config.disable_plugins) || null + if (deploymentRaw) layers.push(['deployment', deploymentRaw]) + // Org layer + if (this.config && this.config.disable_plugins) { + layers.push(['org', this.config.disable_plugins]) + } + if (repoName !== undefined && repoName !== null) { + const suborg = this.getSubOrgConfig(repoName) + if (suborg && suborg.disable_plugins) { + layers.push(['suborg', suborg.disable_plugins]) + } + const repoOverride = this.getRepoOverrideConfig(repoName) + if (repoOverride && repoOverride.disable_plugins) { + layers.push(['repo', repoOverride.disable_plugins]) + } + } + + for (const [declaredAt, rawList] of layers) { + const entries = this.normalizeDisableEntries(rawList, declaredAt) + for (const { plugin, target } of entries) { + const affected = DISABLE_STRIP_MATRIX[declaredAt][target] || [] + for (const lvl of affected) { + stripMap.get(lvl).add(plugin) + } + } + } + this.log.debug(`disable_plugins stripMap for repo=${repoName || ''}: ${JSON.stringify([...stripMap].map(([k, v]) => [k, [...v]]))}`) + return stripMap + } + + // True if the given plugin appears in ANY layer of the stripMap. Used by + // gates around `repository` / `archive` (and updateOrg's rulesets / + // custom_repository_roles) where the plugin runs per-org or per-repo and + // there's no merge-time pipeline to strip into. + isPluginDisabledAnywhere (stripMap, pluginName) { + if (!stripMap) return false + for (const set of stripMap.values()) { + if (set.has(pluginName)) return true + } + return false + } + + // Returns the declaredAt layer(s) responsible for disabling `pluginName` + // in the given stripMap. Used to build informative NopCommand / log + // messages. Note: stripMap layers are *target* layers, not declaration + // layers — to report the source we re-walk the raw disable_plugins lists. + whoDisabled (pluginName, repoName) { + const sources = [] + const probe = (declaredAt, raw) => { + if (!raw) return + let entries = [] + try { entries = this.normalizeDisableEntries(raw, declaredAt) } catch { return } + for (const e of entries) { + if (e.plugin === pluginName) sources.push(`${declaredAt}(target=${e.target})`) + } + } + probe('deployment', DeploymentConfig && DeploymentConfig.config && DeploymentConfig.config.disable_plugins) + probe('org', this.config && this.config.disable_plugins) + if (repoName !== undefined && repoName !== null) { + const suborg = this.getSubOrgConfig(repoName) + probe('suborg', suborg && suborg.disable_plugins) + const repoOverride = this.getRepoOverrideConfig(repoName) + probe('repo', repoOverride && repoOverride.disable_plugins) + } + return sources + } + + // Apply strips to a `{ deployment, org, suborg, repo }` map of cloned + // configs. Mutates clones in place and returns them. Emits NopCommand + // entries when in nop mode. + applyStrips (stripMap, sources, repoName) { + if (!stripMap) return sources + for (const [level, pluginSet] of stripMap) { + const layer = sources[level] + if (!layer) continue + for (const plugin of pluginSet) { + if (Object.prototype.hasOwnProperty.call(layer, plugin)) { + delete layer[plugin] + this.log.debug(`disable_plugins: stripped '${plugin}' from ${level} layer (repo=${repoName || ''})`) + if (this.nop) { + const declaredBy = this.whoDisabled(plugin, repoName).join(', ') + const nopcommand = new NopCommand('disable_plugins', this.repo, null, `Plugin '${plugin}' stripped from ${level} layer (declared by: ${declaredBy || 'unknown'})`, 'INFO') + this.appendToResults([nopcommand]) + } + } + } + } + return sources + } + + // Emit a NopCommand recording that a per-execution-point plugin + // (rulesets / custom_repository_roles / repository / archive) was skipped + // because it appears in the stripMap. + emitDisableSkip (pluginName, repoName) { + if (!this.nop) return + const declaredBy = this.whoDisabled(pluginName, repoName).join(', ') + const nopcommand = new NopCommand('disable_plugins', this.repo, null, `Plugin '${pluginName}' skipped (declared by: ${declaredBy || 'unknown'})`, 'INFO') + this.appendToResults([nopcommand]) + } + async updateOrg () { + // Org-execution stripMap: no repo context, so only deployment + org + // disable_plugins contribute. + const stripMap = this.computeStripMap() + const rulesetsConfig = this.config.rulesets if (rulesetsConfig) { - const RulesetsPlugin = Settings.PLUGINS.rulesets - await new RulesetsPlugin(this.nop, this.github, this.repo, rulesetsConfig, this.log, this.errors, SCOPE.ORG).sync().then(res => { - this.appendToResults(res) - }) + if (this.isPluginDisabledAnywhere(stripMap, 'rulesets')) { + this.log.debug("disable_plugins: skipping org-level 'rulesets' plugin") + this.emitDisableSkip('rulesets') + } else { + const RulesetsPlugin = Settings.PLUGINS.rulesets + await new RulesetsPlugin(this.nop, this.github, this.repo, rulesetsConfig, this.log, this.errors, SCOPE.ORG).sync().then(res => { + this.appendToResults(res) + }) + } } const customRepositoryRolesConfig = this.config.custom_repository_roles if (customRepositoryRolesConfig) { - const CustomRepositoryRolesPlugin = Settings.PLUGINS.custom_repository_roles - await new CustomRepositoryRolesPlugin(this.nop, this.github, this.repo, customRepositoryRolesConfig, this.log, this.errors).sync().then(res => { - this.appendToResults(res) - }) + if (this.isPluginDisabledAnywhere(stripMap, 'custom_repository_roles')) { + this.log.debug("disable_plugins: skipping org-level 'custom_repository_roles' plugin") + this.emitDisableSkip('custom_repository_roles') + } else { + const CustomRepositoryRolesPlugin = Settings.PLUGINS.custom_repository_roles + await new CustomRepositoryRolesPlugin(this.nop, this.github, this.repo, customRepositoryRolesConfig, this.log, this.errors).sync().then(res => { + this.appendToResults(res) + }) + } } } @@ -395,6 +611,12 @@ ${this.results.reduce((x, y) => { repoConfig = this.mergeDeep.mergeDeep({}, repoConfig, overrideRepoConfig) } if (repoConfig) { + // Per-repo disable_plugins stripMap (used to gate repository + archive + // plugins, which run per-repo outside the childPluginsList pipeline). + const repoStripMap = this.computeStripMap(repo.repo) + const repositoryDisabled = this.isPluginDisabledAnywhere(repoStripMap, 'repository') + const archiveDisabled = this.isPluginDisabledAnywhere(repoStripMap, 'archive') + // Track actual change signals from the plugins, used by the suborg // re-evaluation logic below to avoid an unnecessary live API round-trip // when nothing relevant actually changed. @@ -405,8 +627,18 @@ ${this.results.reduce((x, y) => { const childPlugins = this.childPluginsList(repo) const RepoPlugin = Settings.PLUGINS.repository - const archivePlugin = new Archive(this.nop, this.github, repo, repoConfig, this.log) - const { shouldArchive, shouldUnarchive } = await archivePlugin.getState() + let archivePlugin = null + let shouldArchive = false + let shouldUnarchive = false + if (archiveDisabled) { + this.log.debug(`disable_plugins: skipping 'archive' plugin for ${repo.repo}`) + this.emitDisableSkip('archive', repo.repo) + } else { + archivePlugin = new Archive(this.nop, this.github, repo, repoConfig, this.log) + const state = await archivePlugin.getState() + shouldArchive = state.shouldArchive + shouldUnarchive = state.shouldUnarchive + } if (shouldUnarchive) { this.log.debug(`Unarchiving repo ${repo.repo}`) @@ -414,11 +646,16 @@ ${this.results.reduce((x, y) => { this.appendToResults(unArchiveResults) } - const repoPluginInstance = new RepoPlugin(this.nop, this.github, repo, repoConfig, this.installation_id, this.log, this.errors) - const repoResults = await repoPluginInstance.sync() - this.appendToResults(repoResults) - if (repoPluginInstance.renamed) changeSignals.renamed = true - if (repoPluginInstance.created) changeSignals.created = true + if (repositoryDisabled) { + this.log.debug(`disable_plugins: skipping 'repository' plugin for ${repo.repo}`) + this.emitDisableSkip('repository', repo.repo) + } else { + const repoPluginInstance = new RepoPlugin(this.nop, this.github, repo, repoConfig, this.installation_id, this.log, this.errors) + const repoResults = await repoPluginInstance.sync() + this.appendToResults(repoResults) + if (repoPluginInstance.renamed) changeSignals.renamed = true + if (repoPluginInstance.created) changeSignals.created = true + } const childPluginInstances = childPlugins.map(([Plugin, config]) => { return [Plugin, new Plugin(this.nop, this.github, repo, config, this.log, this.errors)] @@ -611,15 +848,39 @@ ${this.results.reduce((x, y) => { const newConfig = Object.assign({}, config) // clone delete newConfig.rulesets delete newConfig.custom_repository_roles + delete newConfig.disable_plugins return newConfig } + // Shallow-clone a config object and strip the `disable_plugins` key (it is + // metadata, not a plugin section that should be merged into child plugins). + cloneAndStripDisableMeta (config) { + if (!config) return {} + const clone = Object.assign({}, config) + delete clone.disable_plugins + return clone + } + childPluginsList (repo) { const repoName = repo.repo const subOrgOverrideConfig = this.getSubOrgConfig(repoName) this.log.debug(`suborg config for ${repoName} is ${JSON.stringify(subOrgOverrideConfig)}`) const repoOverrideConfig = this.getRepoOverrideConfig(repoName) - const overrideConfig = this.mergeDeep.mergeDeep({}, this.returnRepoSpecificConfigs(this.config), subOrgOverrideConfig, repoOverrideConfig) + + // Build clones of each layer and apply disable_plugins strips before the + // existing mergeDeep pipeline runs. The deployment layer's strips affect + // the OTHER three layers (per the matrix); the deployment config itself + // is not merged into per-repo plugin config today. + const stripMap = this.computeStripMap(repoName) + const sources = { + deployment: this.cloneAndStripDisableMeta((DeploymentConfig && DeploymentConfig.config) || {}), + org: this.returnRepoSpecificConfigs(this.config), + suborg: this.cloneAndStripDisableMeta(subOrgOverrideConfig), + repo: this.cloneAndStripDisableMeta(repoOverrideConfig) + } + this.applyStrips(stripMap, sources, repoName) + + const overrideConfig = this.mergeDeep.mergeDeep({}, sources.org, sources.suborg, sources.repo) this.log.debug(`consolidated config is ${JSON.stringify(overrideConfig)}`) diff --git a/package.json b/package.json index f0ae365bf..974308487 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,8 @@ "test:unit:watch": "npm run test:unit -- --watch", "test:integration": "jest --roots=lib --roots=test/integration", "test:integration:debug": "LOG_LEVEL=debug DEBUG=nock run-s test:integration", - "smoke-test": "node smoke-test.js" + "smoke-test": "node smoke-test.js", + "smoke-test:interactive": "node smoke-test.js --interactive" }, "author": "Yadhav Jayaraman", "license": "ISC", @@ -91,4 +92,4 @@ "." ] } -} +} \ No newline at end of file diff --git a/schema/dereferenced/settings.json b/schema/dereferenced/settings.json index e94a66e57..82630e0ad 100644 --- a/schema/dereferenced/settings.json +++ b/schema/dereferenced/settings.json @@ -39,7 +39,17 @@ "properties": { "advanced_security": { "type": "object", - "description": "Use the `status` property to enable or disable GitHub Advanced Security for this repository. For more information, see \"[About GitHub Advanced Security](/github/getting-started-with-github/learning-about-github/about-github-advanced-security).\"", + "description": "Use the `status` property to enable or disable GitHub Advanced Security for this repository.\nFor more information, see \"[About GitHub Advanced\nSecurity](/github/getting-started-with-github/learning-about-github/about-github-advanced-security).\"\n\nFor standalone Code Scanning or Secret Protection products, this parameter cannot be used.", + "properties": { + "status": { + "type": "string", + "description": "Can be `enabled` or `disabled`." + } + } + }, + "code_security": { + "type": "object", + "description": "Use the `status` property to enable or disable GitHub Code Security for this repository.", "properties": { "status": { "type": "string", @@ -67,6 +77,16 @@ } } }, + "secret_scanning_ai_detection": { + "type": "object", + "description": "Use the `status` property to enable or disable secret scanning AI detection for this repository. For more information, see \"[Responsible detection of generic secrets with AI](https://docs.github.com/code-security/secret-scanning/using-advanced-secret-scanning-and-push-protection-features/generic-secret-detection/responsible-ai-generic-secrets).\"", + "properties": { + "status": { + "type": "string", + "description": "Can be `enabled` or `disabled`." + } + } + }, "secret_scanning_non_provider_patterns": { "type": "object", "description": "Use the `status` property to enable or disable secret scanning non-provider patterns for this repository. For more information, see \"[Supported secret scanning patterns](/code-security/secret-scanning/introduction/supported-secret-scanning-patterns#supported-secrets).\"", @@ -76,6 +96,66 @@ "description": "Can be `enabled` or `disabled`." } } + }, + "secret_scanning_delegated_alert_dismissal": { + "type": "object", + "description": "Use the `status` property to enable or disable secret scanning delegated alert dismissal for this repository.", + "properties": { + "status": { + "type": "string", + "description": "Can be `enabled` or `disabled`." + } + } + }, + "secret_scanning_delegated_bypass": { + "type": "object", + "description": "Use the `status` property to enable or disable secret scanning delegated bypass for this repository.", + "properties": { + "status": { + "type": "string", + "description": "Can be `enabled` or `disabled`." + } + } + }, + "secret_scanning_delegated_bypass_options": { + "type": "object", + "description": "Feature options for secret scanning delegated bypass.\nThis object is only honored when `security_and_analysis.secret_scanning_delegated_bypass.status` is set to `enabled`.\nYou can send this object in the same request as `secret_scanning_delegated_bypass`, or update just the options in a separate request.", + "properties": { + "reviewers": { + "type": "array", + "description": "The bypass reviewers for secret scanning delegated bypass.\nIf you omit this field, the existing set of reviewers is unchanged.", + "items": { + "type": "object", + "required": [ + "reviewer_id", + "reviewer_type" + ], + "properties": { + "reviewer_id": { + "type": "integer", + "description": "The ID of the team or role selected as a bypass reviewer" + }, + "reviewer_type": { + "type": "string", + "description": "The type of the bypass reviewer", + "enum": [ + "TEAM", + "ROLE" + ] + }, + "mode": { + "type": "string", + "description": "The bypass mode for the reviewer", + "enum": [ + "ALWAYS", + "EXEMPT" + ], + "default": "ALWAYS" + } + } + } + } + } } } }, @@ -135,7 +215,7 @@ }, "use_squash_pr_title_as_default": { "type": "boolean", - "description": "Either `true` to allow squash-merge commits to use pull request title, or `false` to use commit message. **This property has been deprecated. Please use `squash_merge_commit_title` instead.", + "description": "Either `true` to allow squash-merge commits to use pull request title, or `false` to use commit message. **This property is closing down. Please use `squash_merge_commit_title` instead.", "default": false, "deprecated": true }, @@ -338,7 +418,7 @@ }, "maintainers": { "type": "array", - "description": "List GitHub IDs for organization members who will become team maintainers.", + "description": "List GitHub usernames for organization members who will become team maintainers.", "items": { "type": "string" } @@ -368,7 +448,7 @@ }, "permission": { "type": "string", - "description": "**Deprecated**. The permission that new repositories will be added to the team with when none is specified.", + "description": "**Closing down notice**. The permission that new repositories will be added to the team with when none is specified.", "enum": [ "pull", "push" @@ -409,7 +489,7 @@ "contexts": { "type": "array", "deprecated": true, - "description": "**Deprecated**: The list of status checks to require in order to merge into this branch. If any of these checks have recently been set by a particular GitHub App, they will be required to come from that app in future for the branch to merge. Use `checks` instead of `contexts` for more fine-grained control.", + "description": "**Closing down notice**: The list of status checks to require in order to merge into this branch. If any of these checks have recently been set by a particular GitHub App, they will be required to come from that app in future for the branch to merge. Use `checks` instead of `contexts` for more fine-grained control.", "items": { "type": "string" } @@ -663,7 +743,8 @@ "enum": [ "branch", "tag", - "push" + "push", + "repository" ], "default": "branch" }, @@ -690,7 +771,7 @@ "actor_id": { "type": "integer", "nullable": true, - "description": "The ID of the actor that can bypass a ruleset. If `actor_type` is `OrganizationAdmin`, this should be `1`. If `actor_type` is `DeployKey`, this should be null. `OrganizationAdmin` is not applicable for personal repositories." + "description": "The ID of the actor that can bypass a ruleset. Required for `Integration`, `RepositoryRole`, `Team`, and `User` actor types. If `actor_type` is `OrganizationAdmin`, `actor_id` is ignored. If `actor_type` is `DeployKey`, this should be null. `OrganizationAdmin` is not applicable for personal repositories." }, "actor_type": { "type": "string", @@ -699,16 +780,18 @@ "OrganizationAdmin", "RepositoryRole", "Team", - "DeployKey" + "DeployKey", + "User" ], "description": "The type of actor that can bypass a ruleset." }, "bypass_mode": { "type": "string", - "description": "When the specified actor can bypass the ruleset. `pull_request` means that an actor can only bypass rules on pull requests. `pull_request` is not applicable for the `DeployKey` actor type. Also, `pull_request` is only applicable to branch rulesets.", + "description": "When the specified actor can bypass the ruleset. `pull_request` means that an actor can only bypass rules on pull requests. `pull_request` is not applicable for the `DeployKey` actor type. Also, `pull_request` is only applicable to branch rulesets. When `bypass_mode` is `exempt`, rules will not be run for that actor and a bypass audit entry will not be created.", "enum": [ "always", - "pull_request" + "pull_request", + "exempt" ], "default": "always" } @@ -718,7 +801,7 @@ "conditions": { "title": "Organization ruleset conditions", "type": "object", - "description": "Conditions for an organization ruleset.\nThe branch and tag rulesets conditions object should contain both `repository_name` and `ref_name` properties, or both `repository_id` and `ref_name` properties, or both `repository_property` and `ref_name` properties.\nThe push rulesets conditions object does not require the `ref_name` property.", + "description": "Conditions for an organization ruleset.\nThe branch and tag rulesets conditions object should contain both `repository_name` and `ref_name` properties, or both `repository_id` and `ref_name` properties, or both `repository_property` and `ref_name` properties.\nThe push rulesets conditions object does not require the `ref_name` property.\nFor repository policy rulesets, the conditions object should only contain the `repository_name`, the `repository_id`, or the `repository_property`.", "oneOf": [ { "type": "object", @@ -1043,83 +1126,6 @@ } } }, - { - "title": "merge_queue", - "description": "Merges must be performed via a merge queue.", - "type": "object", - "required": [ - "type" - ], - "properties": { - "type": { - "type": "string", - "enum": [ - "merge_queue" - ] - }, - "parameters": { - "type": "object", - "properties": { - "check_response_timeout_minutes": { - "type": "integer", - "description": "Maximum time for a required status check to report a conclusion. After this much time has elapsed, checks that have not reported a conclusion will be assumed to have failed", - "minimum": 1, - "maximum": 360 - }, - "grouping_strategy": { - "type": "string", - "description": "When set to ALLGREEN, the merge commit created by merge queue for each PR in the group must pass all required checks to merge. When set to HEADGREEN, only the commit at the head of the merge group, i.e. the commit containing changes from all of the PRs in the group, must pass its required checks to merge.", - "enum": [ - "ALLGREEN", - "HEADGREEN" - ] - }, - "max_entries_to_build": { - "type": "integer", - "description": "Limit the number of queued pull requests requesting checks and workflow runs at the same time.", - "minimum": 0, - "maximum": 100 - }, - "max_entries_to_merge": { - "type": "integer", - "description": "The maximum number of PRs that will be merged together in a group.", - "minimum": 0, - "maximum": 100 - }, - "merge_method": { - "type": "string", - "description": "Method to use when merging changes from queued pull requests.", - "enum": [ - "MERGE", - "SQUASH", - "REBASE" - ] - }, - "min_entries_to_merge": { - "type": "integer", - "description": "The minimum number of PRs that will be merged together in a group.", - "minimum": 0, - "maximum": 100 - }, - "min_entries_to_merge_wait_minutes": { - "type": "integer", - "description": "The time merge queue should wait after the first PR is added to the queue for the minimum group size to be met. After this time has elapsed, the minimum group size will be ignored and a smaller group will be merged.", - "minimum": 0, - "maximum": 360 - } - }, - "required": [ - "check_response_timeout_minutes", - "grouping_strategy", - "max_entries_to_build", - "max_entries_to_merge", - "merge_method", - "min_entries_to_merge", - "min_entries_to_merge_wait_minutes" - ] - } - } - }, { "title": "required_deployments", "description": "Choose which environments must be successfully deployed to before refs can be pushed into a ref that matches this rule.", @@ -1184,6 +1190,18 @@ "parameters": { "type": "object", "properties": { + "allowed_merge_methods": { + "type": "array", + "description": "Array of allowed merge methods. Allowed values include `merge`, `squash`, and `rebase`. At least one option must be enabled.", + "items": { + "type": "string", + "enum": [ + "merge", + "squash", + "rebase" + ] + } + }, "dismiss_stale_reviews_on_push": { "type": "boolean", "description": "New, reviewable commits pushed will dismiss previous pull request review approvals." @@ -1205,6 +1223,55 @@ "required_review_thread_resolution": { "type": "boolean", "description": "All conversations on code must be resolved before a pull request can be merged." + }, + "required_reviewers": { + "type": "array", + "description": "> [!NOTE]\n> `required_reviewers` is in beta and subject to change.\n\nA collection of reviewers and associated file patterns. Each reviewer has a list of file patterns which determine the files that reviewer is required to review.", + "items": { + "title": "RequiredReviewerConfiguration", + "description": "A reviewing team, and file patterns describing which files they must approve changes to.", + "type": "object", + "properties": { + "file_patterns": { + "type": "array", + "description": "Array of file patterns. Pull requests which change matching files must be approved by the specified team. File patterns use fnmatch syntax.", + "items": { + "type": "string" + } + }, + "minimum_approvals": { + "type": "integer", + "description": "Minimum number of approvals required from the specified team. If set to zero, the team will be added to the pull request but approval is optional." + }, + "reviewer": { + "title": "Reviewer", + "description": "A required reviewing team", + "type": "object", + "properties": { + "id": { + "type": "integer", + "description": "ID of the reviewer which must review changes to matching files." + }, + "type": { + "type": "string", + "description": "The type of the reviewer", + "enum": [ + "Team" + ] + } + }, + "required": [ + "id", + "type" + ] + } + }, + "required": [ + "file_patterns", + "minimum_approvals", + "reviewer" + ] + } } }, "required": [ @@ -1307,7 +1374,7 @@ "properties": { "name": { "type": "string", - "description": "How this rule will appear to users." + "description": "How this rule appears when configuring it." }, "negate": { "type": "boolean", @@ -1354,7 +1421,7 @@ "properties": { "name": { "type": "string", - "description": "How this rule will appear to users." + "description": "How this rule appears when configuring it." }, "negate": { "type": "boolean", @@ -1401,7 +1468,7 @@ "properties": { "name": { "type": "string", - "description": "How this rule will appear to users." + "description": "How this rule appears when configuring it." }, "negate": { "type": "boolean", @@ -1448,7 +1515,7 @@ "properties": { "name": { "type": "string", - "description": "How this rule will appear to users." + "description": "How this rule appears when configuring it." }, "negate": { "type": "boolean", @@ -1495,7 +1562,7 @@ "properties": { "name": { "type": "string", - "description": "How this rule will appear to users." + "description": "How this rule appears when configuring it." }, "negate": { "type": "boolean", @@ -1525,7 +1592,7 @@ }, { "title": "file_path_restriction", - "description": "Prevent commits that include changes in specified file paths from being pushed to the commit graph.", + "description": "Prevent commits that include changes in specified file and folder paths from being pushed to the commit graph. This includes absolute paths that contain file names.", "type": "object", "required": [ "type" @@ -1556,7 +1623,7 @@ }, { "title": "max_file_path_length", - "description": "Prevent commits that include file paths that exceed a specified character limit from being pushed to the commit graph.", + "description": "Prevent commits that include file paths that exceed the specified character limit from being pushed to the commit graph.", "type": "object", "required": [ "type" @@ -1573,9 +1640,9 @@ "properties": { "max_file_path_length": { "type": "integer", - "description": "The maximum amount of characters allowed in file paths", + "description": "The maximum amount of characters allowed in file paths.", "minimum": 1, - "maximum": 256 + "maximum": 32767 } }, "required": [ @@ -1617,7 +1684,7 @@ }, { "title": "max_file_size", - "description": "Prevent commits that exceed a specified file size limit from being pushed to the commit.", + "description": "Prevent commits with individual files that exceed the specified limit from being pushed to the commit graph.", "type": "object", "required": [ "type" @@ -1768,6 +1835,35 @@ ] } } + }, + { + "title": "copilot_code_review", + "description": "Request Copilot code review for new pull requests automatically if the author has access to Copilot code review and their premium requests quota has not reached the limit.", + "type": "object", + "required": [ + "type" + ], + "properties": { + "type": { + "type": "string", + "enum": [ + "copilot_code_review" + ] + }, + "parameters": { + "type": "object", + "properties": { + "review_draft_pull_requests": { + "type": "boolean", + "description": "Copilot automatically reviews draft pull requests before they are marked as ready for review." + }, + "review_on_push": { + "type": "boolean", + "description": "Copilot automatically reviews each new push to the pull request." + } + } + } + } } ] } @@ -1778,6 +1874,112 @@ "enforcement" ] } + }, + "custom_repository_roles": { + "description": "Org-level custom repository roles. Only valid in the org-level settings.yml.", + "type": "array", + "items": { + "type": "object", + "required": [ + "name", + "base_role", + "permissions" + ], + "properties": { + "name": { + "type": "string", + "description": "The name of the custom role." + }, + "description": { + "type": [ + "string", + "null" + ], + "description": "A short description of the role." + }, + "base_role": { + "type": "string", + "enum": [ + "read", + "triage", + "write", + "maintain" + ], + "description": "The system role from which this role inherits permissions." + }, + "permissions": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Additional fine-grained permissions included in this role." + } + } + } + }, + "disable_plugins": { + "description": "List of plugins to disable at this configuration layer. Each entry is either a plugin name (string shorthand, equivalent to target: all) or an object {plugin, target}. target=self disables the plugin at this layer only; target=children disables it at all lower layers; target=all disables it at this layer and all lower layers. Cascade is union-only; lower layers cannot re-enable a disabled plugin.", + "type": "array", + "items": { + "oneOf": [ + { + "type": "string", + "enum": [ + "repository", + "labels", + "collaborators", + "teams", + "milestones", + "branches", + "autolinks", + "validator", + "rulesets", + "environments", + "custom_properties", + "custom_repository_roles", + "variables", + "archive" + ] + }, + { + "type": "object", + "required": [ + "plugin" + ], + "additionalProperties": false, + "properties": { + "plugin": { + "type": "string", + "enum": [ + "repository", + "labels", + "collaborators", + "teams", + "milestones", + "branches", + "autolinks", + "validator", + "rulesets", + "environments", + "custom_properties", + "custom_repository_roles", + "variables", + "archive" + ] + }, + "target": { + "type": "string", + "enum": [ + "self", + "children", + "all" + ], + "default": "all" + } + } + } + ] + } } } } \ No newline at end of file diff --git a/schema/settings.json b/schema/settings.json index 3649d88ea..45f85340d 100644 --- a/schema/settings.json +++ b/schema/settings.json @@ -233,6 +233,64 @@ } } } + }, + "disable_plugins": { + "description": "List of plugins to disable at this configuration layer. Each entry is either a plugin name (string shorthand, equivalent to target: all) or an object {plugin, target}. target=self disables the plugin at this layer only; target=children disables it at all lower layers; target=all disables it at this layer and all lower layers. Cascade is union-only; lower layers cannot re-enable a disabled plugin.", + "type": "array", + "items": { + "oneOf": [ + { + "type": "string", + "enum": [ + "repository", + "labels", + "collaborators", + "teams", + "milestones", + "branches", + "autolinks", + "validator", + "rulesets", + "environments", + "custom_properties", + "custom_repository_roles", + "variables", + "archive" + ] + }, + { + "type": "object", + "required": ["plugin"], + "additionalProperties": false, + "properties": { + "plugin": { + "type": "string", + "enum": [ + "repository", + "labels", + "collaborators", + "teams", + "milestones", + "branches", + "autolinks", + "validator", + "rulesets", + "environments", + "custom_properties", + "custom_repository_roles", + "variables", + "archive" + ] + }, + "target": { + "type": "string", + "enum": ["self", "children", "all"], + "default": "all" + } + } + } + ] + } } } } \ No newline at end of file diff --git a/smoke-test.js b/smoke-test.js index d881972da..9725ea712 100644 --- a/smoke-test.js +++ b/smoke-test.js @@ -9,6 +9,7 @@ * This is required for drift-remediation tests (Phases 2 & 3) so that * changes appear as a human (not Bot) and trigger safe-settings webhooks. * 3. Run: `node smoke-test.js` + * Add --interactive to pause after each phase for manual validation. * Set SMOKE_VERBOSE=1 for live safe-settings logs. * * Auth: @@ -19,6 +20,7 @@ const { execSync, spawn } = require('child_process') const fs = require('fs') const path = require('path') +const readline = require('readline') // ─── Configuration ─────────────────────────────────────────────────────────── @@ -75,6 +77,16 @@ const WEBHOOK_SETTLE_MS = 15000 // Fine-grained PAT for drift tests (must appear as a human, not Bot) const GH_TOKEN = process.env.GH_TOKEN || '' +// Interactive mode: pause after each phase for manual validation +const INTERACTIVE = process.argv.includes('--interactive') + +class InteractiveExit extends Error { + constructor (action) { + super(`interactive:${action}`) + this.action = action + } +} + // ─── Octokit client (initialized in main) ──────────────────────────────────── let octokit = null @@ -109,6 +121,76 @@ async function poll (fn, { timeout = MAX_POLL_MS, interval = POLL_INTERVAL_MS, d return null } +// ─── Interactive mode ───────────────────────────────────────────────────────── + +let skipNext = false + +async function pause (phaseName) { + return new Promise((resolve) => { + const rl = readline.createInterface({ input: process.stdin, output: process.stdout }) + process.stdout.write( + `\n\x1b[33m[interactive] "${phaseName}" complete.\x1b[0m\n` + + ` \x1b[90mPress Enter to continue, 's' skip next, 'q' quit+teardown, 'a' abort: \x1b[0m` + ) + rl.once('line', (answer) => { + const input = answer.trim().toLowerCase() + if (input === 's') resolve('skip') + else if (input === 'q') resolve('quit') + else if (input === 'a') resolve('abort') + else resolve('continue') + rl.close() + }) + rl.once('close', () => resolve('continue')) + }) +} + +async function runPhase (label, fn) { + if (skipNext) { + log(`\x1b[33m[interactive] Skipping ${label}\x1b[0m`) + skipNext = false + return 'skipped' + } + await fn() + if (!INTERACTIVE) return 'continue' + const action = await pause(label) + if (action === 'skip') skipNext = true + return action +} + +async function confirmMerge (owner, repo, prNumber) { + return new Promise((resolve) => { + const rl = readline.createInterface({ input: process.stdin, output: process.stdout }) + process.stdout.write( + `\n\x1b[33m[interactive] PR #${prNumber} is ready to merge.\x1b[0m\n` + + ` \x1b[90mPress Enter to merge, 'c' to close PR, 'q' quit+teardown, 'a' abort: \x1b[0m` + ) + rl.once('line', (answer) => { + const input = answer.trim().toLowerCase() + if (input === 'c') resolve('close') + else if (input === 'q') resolve('quit') + else if (input === 'a') resolve('abort') + else resolve('merge') + rl.close() + }) + rl.once('close', () => resolve('merge')) + }) +} + +async function safeMerge (owner, repo, prNumber) { + if (INTERACTIVE) { + const action = await confirmMerge(owner, repo, prNumber) + if (action !== 'merge') { + try { await octokit.rest.pulls.update({ owner, repo, pull_number: prNumber, state: 'closed' }) } catch { /* ok */ } + log(`\x1b[33m[interactive] PR #${prNumber} closed.\x1b[0m`) + if (action === 'quit' || action === 'abort') throw new InteractiveExit(action) + return false + } + } + log('Merging PR...') + await mergePR(owner, repo, prNumber) + return true +} + // ─── GitHub API helpers ────────────────────────────────────────────────────── async function getDefaultBranch () { @@ -523,6 +605,51 @@ custom_repository_roles: - delete_alerts_code_scanning ` +// Phase 10a: settings.yml that disables custom_repository_roles at org-self, +// and tries to add a NEW role ("disabled-role"). The new role must NOT be created. +const SETTINGS_YML_DISABLE_CRR = `# Org-level settings with disable_plugins (custom_repository_roles) + +disable_plugins: + - plugin: custom_repository_roles + target: self + +rulesets: + - name: test + target: repository + source_type: Organization + source: ${ORG} + enforcement: disabled + conditions: + repository_property: + exclude: [] + include: + - name: visibility + source: system + property_values: + - internal + rules: + - type: repository_delete + +custom_repository_roles: + - name: security-engineer + description: Can contribute code and manage the security pipeline + base_role: maintain + permissions: + - delete_alerts_code_scanning + - name: disabled-role + description: This role MUST NOT be created (custom_repository_roles disabled) + base_role: read + permissions: + - delete_alerts_code_scanning +` + +// Phase 10b: settings.yml with invalid disable_plugins entry — should fail validation +const SETTINGS_YML_INVALID_DISABLE = `# Org-level settings with invalid disable_plugins + +disable_plugins: + - not-a-real-plugin +` + // ─── Test Phases ───────────────────────────────────────────────────────────── async function setup () { @@ -565,8 +692,7 @@ async function phase1CreateRepo () { assert(checkRun !== null, 'Check run completed') if (checkRun) assert(checkRun.conclusion === 'success', `Check run conclusion is success (got: ${checkRun.conclusion})`) - log('Merging PR...') - await mergePR(ORG, ADMIN_REPO, pr.number) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return await sleep(WEBHOOK_SETTLE_MS) // Validate repo @@ -693,8 +819,7 @@ async function phase4DemoRepo1 () { assert(checkRun !== null, 'Check run completed') if (checkRun) assert(checkRun.conclusion === 'success', `Check run conclusion is success (got: ${checkRun.conclusion})`) - log('Merging PR...') - await mergePR(ORG, ADMIN_REPO, pr.number) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return await sleep(WEBHOOK_SETTLE_MS) const repo = await poll(async () => { @@ -750,8 +875,7 @@ async function phase5Suborg () { assert(checkRun !== null, 'Check run completed') if (checkRun) assert(checkRun.conclusion === 'success', `Check run conclusion is success (got: ${checkRun.conclusion})`) - log('Merging PR...') - await mergePR(ORG, ADMIN_REPO, pr.number) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return await sleep(WEBHOOK_SETTLE_MS) log('Checking suborg ruleset on demo-repo-service1...') @@ -782,8 +906,7 @@ async function phase6Archive () { assert(checkRun !== null, 'Check run completed') if (checkRun) assert(checkRun.conclusion === 'success', `Check run conclusion is success (got: ${checkRun.conclusion})`) - log('Merging PR...') - await mergePR(ORG, ADMIN_REPO, pr.number) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return await sleep(WEBHOOK_SETTLE_MS) const repo = await poll(async () => { @@ -813,8 +936,7 @@ async function phase7DemoRepo2 () { assert(checkRun !== null, 'Check run completed') if (checkRun) assert(checkRun.conclusion === 'success', `Check run conclusion is success (got: ${checkRun.conclusion})`) - log('Merging PR...') - await mergePR(ORG, ADMIN_REPO, pr.number) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return await sleep(WEBHOOK_SETTLE_MS) const repo = await poll(async () => { @@ -861,8 +983,7 @@ async function phase7bExternalGroupTeam () { assert(checkRun1 !== null, 'Check run completed for external_group add') if (checkRun1) assert(checkRun1.conclusion === 'success', `Check run conclusion is success (got: ${checkRun1.conclusion})`) - log('Merging PR...') - await mergePR(ORG, ADMIN_REPO, pr1.number) + if (!await safeMerge(ORG, ADMIN_REPO, pr1.number)) return await sleep(WEBHOOK_SETTLE_MS) // Verify team is created and assigned to the repo @@ -907,8 +1028,7 @@ async function phase7bExternalGroupTeam () { assert(checkRun2 !== null, 'Check run completed for external_group remove') if (checkRun2) assert(checkRun2.conclusion === 'success', `Check run conclusion is success (got: ${checkRun2.conclusion})`) - log('Merging PR...') - await mergePR(ORG, ADMIN_REPO, pr2.number) + if (!await safeMerge(ORG, ADMIN_REPO, pr2.number)) return await sleep(WEBHOOK_SETTLE_MS) // Verify team is removed from the repo @@ -941,8 +1061,7 @@ async function phase8OrgSettings () { assert(checkRun !== null, 'Check run completed') if (checkRun) assert(checkRun.conclusion === 'success', `Check run conclusion is success (got: ${checkRun.conclusion})`) - log('Merging PR...') - await mergePR(ORG, ADMIN_REPO, pr.number) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return await sleep(WEBHOOK_SETTLE_MS) log('Checking custom repository roles...') @@ -966,6 +1085,66 @@ async function phase8OrgSettings () { await deleteBranch(ORG, ADMIN_REPO, branch) } +async function phase10DisablePlugins () { + logPhase('Phase 10: disable_plugins') + + const defaultBranch = await getDefaultBranch() + + // ── 10a: Org disables custom_repository_roles at target:self ── + // Add a NEW role "disabled-role" + keep existing "security-engineer". + // Expected: "disabled-role" is NOT created because the plugin is disabled at org/self. + { + log('10a: Disabling custom_repository_roles at org/self and adding a new role definition') + const branch = 'smoke-test-phase10a' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, SETTINGS_YML_DISABLE_CRR, branch, '10a: disable custom_repository_roles') + + const pr = await createPR(ORG, ADMIN_REPO, '10a: disable custom_repository_roles', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '10a: NOP check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `10a: NOP check run is success (got: ${checkRun.conclusion})`) + + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS) + + // Give safe-settings time to run; then verify disabled-role was NOT created. + await sleep(20000) + let disabledRoleExists = false + try { + const { data } = await octokit.request('GET /orgs/{org}/custom-repository-roles', { org: ORG }) + disabledRoleExists = (data.custom_roles || []).some(r => r.name === 'disabled-role') + } catch { /* ok */ } + assert(disabledRoleExists === false, '10a: "disabled-role" was NOT created (custom_repository_roles plugin disabled)') + + await deleteBranch(ORG, ADMIN_REPO, branch) + } + + // ── 10b: Invalid disable_plugins entry → NOP check run should fail ── + { + log('10b: Submitting invalid disable_plugins entry') + const branch = 'smoke-test-phase10b' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, SETTINGS_YML_INVALID_DISABLE, branch, '10b: invalid disable_plugins') + + const pr = await createPR(ORG, ADMIN_REPO, '10b: invalid disable_plugins', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '10b: NOP check run completed') + if (checkRun) { + assert(checkRun.conclusion !== 'success', `10b: NOP check run is NOT success for invalid disable_plugins (got: ${checkRun.conclusion})`) + } + + // Close PR without merging — invalid config should never be merged. + try { await octokit.rest.pulls.update({ owner: ORG, repo: ADMIN_REPO, pull_number: pr.number, state: 'closed' }) } catch { /* ok */ } + await deleteBranch(ORG, ADMIN_REPO, branch) + } +} + async function teardown () { logPhase('Phase 9: Teardown') @@ -1028,22 +1207,38 @@ async function main () { ╚══════════════════════════════════════╝\x1b[0m `) + if (INTERACTIVE) log('\x1b[33m[interactive] Mode enabled — will pause after each phase.\x1b[0m') + + let doTeardown = true try { - await setup() - await phase1CreateRepo() - await phase2DriftTeam() - await phase3DriftRuleset() - await phase4DemoRepo1() - await phase5Suborg() - await phase6Archive() - await phase7DemoRepo2() - await phase7bExternalGroupTeam() - await phase8OrgSettings() + const phases = [ + ['Phase 0: Setup', setup], + ['Phase 1: Create test repo', phase1CreateRepo], + ['Phase 2: Drift remediation - Team removal', phase2DriftTeam], + ['Phase 3: Drift remediation - Rogue ruleset', phase3DriftRuleset], + ['Phase 4: Create demo-repo-service1', phase4DemoRepo1], + ['Phase 5: Create suborg config', phase5Suborg], + ['Phase 6: Archive demo-repo-service1', phase6Archive], + ['Phase 7: Create demo-repo-service2', phase7DemoRepo2], + ['Phase 7b: External group team', phase7bExternalGroupTeam], + ['Phase 8: Org-level settings', phase8OrgSettings], + ['Phase 10: disable_plugins', phase10DisablePlugins] + ] + for (const [label, fn] of phases) { + const action = await runPhase(label, fn) + if (action === 'abort') { doTeardown = false; break } + if (action === 'quit') break + } } catch (err) { - console.error(`\x1b[31mFatal error: ${err.message}\x1b[0m`) - console.error(err.stack) + if (err instanceof InteractiveExit) { + if (err.action === 'abort') doTeardown = false + } else { + console.error(`\x1b[31mFatal error: ${err.message}\x1b[0m`) + console.error(err.stack) + } } finally { - await teardown() + if (doTeardown) await teardown() + else log('\x1b[33m[interactive] Aborted — teardown skipped.\x1b[0m') } console.log(` diff --git a/test/unit/lib/settings.test.js b/test/unit/lib/settings.test.js index 23d8ef84d..a56296310 100644 --- a/test/unit/lib/settings.test.js +++ b/test/unit/lib/settings.test.js @@ -639,4 +639,369 @@ repository: expect(stubContext.log.warn).toHaveBeenCalledWith(expect.stringContaining('max depth')) }) }) + + // ──────────────────────────────────────────────────────────────────────── + // disable_plugins + // ──────────────────────────────────────────────────────────────────────── + describe('disable_plugins', () => { + const DeploymentConfig = require('../../../lib/deploymentConfig') + let savedDeploymentDisable + let savedPlugins + + beforeEach(() => { + savedDeploymentDisable = DeploymentConfig.config && DeploymentConfig.config.disable_plugins + if (DeploymentConfig.config) delete DeploymentConfig.config.disable_plugins + savedPlugins = { ...Settings.PLUGINS } + }) + + afterEach(() => { + if (DeploymentConfig.config) { + if (savedDeploymentDisable !== undefined) { + DeploymentConfig.config.disable_plugins = savedDeploymentDisable + } else { + delete DeploymentConfig.config.disable_plugins + } + } + Object.keys(Settings.PLUGINS).forEach(k => { Settings.PLUGINS[k] = savedPlugins[k] }) + }) + + // ── normalizeDisableEntries ────────────────────────────────────────── + describe('normalizeDisableEntries', () => { + it('1. string shorthand defaults target=all and sets declaredAt', () => { + const settings = createSettings({}) + const out = settings.normalizeDisableEntries(['labels'], 'org') + expect(out).toEqual([{ plugin: 'labels', target: 'all', declaredAt: 'org' }]) + }) + + it('2. object form preserves each of self|children|all', () => { + const settings = createSettings({}) + const out = settings.normalizeDisableEntries([ + { plugin: 'rulesets', target: 'self' }, + { plugin: 'branches', target: 'children' }, + { plugin: 'labels', target: 'all' } + ], 'org') + expect(out).toEqual([ + { plugin: 'rulesets', target: 'self', declaredAt: 'org' }, + { plugin: 'branches', target: 'children', declaredAt: 'org' }, + { plugin: 'labels', target: 'all', declaredAt: 'org' } + ]) + }) + + it('3. unknown plugin name throws descriptive error', () => { + const settings = createSettings({}) + expect(() => settings.normalizeDisableEntries(['nope'], 'org')) + .toThrow(/unknown plugin 'nope'/) + }) + + it('4. invalid target throws', () => { + const settings = createSettings({}) + expect(() => settings.normalizeDisableEntries([{ plugin: 'labels', target: 'bogus' }], 'org')) + .toThrow(/invalid target 'bogus'/) + }) + + it('5. repository and archive are accepted as plugin names', () => { + const settings = createSettings({}) + const out = settings.normalizeDisableEntries(['repository', 'archive'], 'org') + expect(out.map(e => e.plugin).sort()).toEqual(['archive', 'repository']) + }) + + it('6. at declaredAt=repo, target=children normalizes to all', () => { + const settings = createSettings({}) + const out = settings.normalizeDisableEntries([{ plugin: 'labels', target: 'children' }], 'repo') + expect(out).toEqual([{ plugin: 'labels', target: 'all', declaredAt: 'repo' }]) + }) + }) + + // ── computeStripMap ────────────────────────────────────────────────── + describe('computeStripMap', () => { + const repoName = 'my-repo' + + it('7. empty configs produce empty map (all four levels are empty sets)', () => { + const settings = createSettings({}) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + const sm = settings.computeStripMap(repoName) + for (const level of ['deployment', 'org', 'suborg', 'repo']) { + expect(sm.get(level).size).toBe(0) + } + }) + + it('8. org target:self for rulesets strips only the org layer', () => { + const settings = createSettings({ disable_plugins: [{ plugin: 'rulesets', target: 'self' }] }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + const sm = settings.computeStripMap(repoName) + expect([...sm.get('org')]).toEqual(['rulesets']) + expect(sm.get('suborg').size).toBe(0) + expect(sm.get('repo').size).toBe(0) + expect(sm.get('deployment').size).toBe(0) + }) + + it('9. org target:children for branches strips suborg+repo', () => { + const settings = createSettings({ disable_plugins: [{ plugin: 'branches', target: 'children' }] }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + const sm = settings.computeStripMap(repoName) + expect(sm.get('org').size).toBe(0) + expect([...sm.get('suborg')]).toEqual(['branches']) + expect([...sm.get('repo')]).toEqual(['branches']) + }) + + it('10. org target:all for labels strips org+suborg+repo', () => { + const settings = createSettings({ disable_plugins: ['labels'] }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + const sm = settings.computeStripMap(repoName) + expect([...sm.get('org')]).toEqual(['labels']) + expect([...sm.get('suborg')]).toEqual(['labels']) + expect([...sm.get('repo')]).toEqual(['labels']) + }) + + it('11. suborg target:all contributes only when a suborg matches the repo', () => { + const settings = createSettings({}) + settings.subOrgConfigs = { + [repoName]: { disable_plugins: ['teams'], source: '.github/suborgs/x.yml' } + } + settings.repoConfigs = {} + const sm = settings.computeStripMap(repoName) + expect([...sm.get('suborg')]).toEqual(['teams']) + expect([...sm.get('repo')]).toEqual(['teams']) + + const sm2 = settings.computeStripMap('other-repo') + expect(sm2.get('suborg').size).toBe(0) + expect(sm2.get('repo').size).toBe(0) + }) + + it('12. repo-declared target:all only strips repo layer', () => { + const settings = createSettings({}) + settings.subOrgConfigs = {} + settings.repoConfigs = { [`${repoName}.yml`]: { disable_plugins: ['labels'] } } + const sm = settings.computeStripMap(repoName) + expect(sm.get('org').size).toBe(0) + expect(sm.get('suborg').size).toBe(0) + expect([...sm.get('repo')]).toEqual(['labels']) + }) + + it('13. deployment target:children strips org+suborg+repo', () => { + DeploymentConfig.config.disable_plugins = [{ plugin: 'milestones', target: 'children' }] + const settings = createSettings({}) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + const sm = settings.computeStripMap(repoName) + expect(sm.get('deployment').size).toBe(0) + expect([...sm.get('org')]).toEqual(['milestones']) + expect([...sm.get('suborg')]).toEqual(['milestones']) + expect([...sm.get('repo')]).toEqual(['milestones']) + }) + + it('14. union across layers: org self + repo all → org and repo both contain plugin', () => { + const settings = createSettings({ disable_plugins: [{ plugin: 'labels', target: 'self' }] }) + settings.subOrgConfigs = {} + settings.repoConfigs = { [`${repoName}.yml`]: { disable_plugins: ['labels'] } } + const sm = settings.computeStripMap(repoName) + expect([...sm.get('org')]).toEqual(['labels']) + expect(sm.get('suborg').size).toBe(0) + expect([...sm.get('repo')]).toEqual(['labels']) + }) + }) + + // ── childPluginsList integration ───────────────────────────────────── + describe('childPluginsList integration', () => { + it('15. org disables custom_properties (target:all) → not in plugin list even with repo override', () => { + const settings = createSettings({ + disable_plugins: ['custom_properties'], + custom_properties: [{ property_name: 'a', value: '1' }] + }) + settings.subOrgConfigs = {} + settings.repoConfigs = { 'foo.yml': { custom_properties: [{ property_name: 'b', value: '2' }] } } + const list = settings.childPluginsList({ repo: 'foo' }) + const pluginNames = list.map(([P]) => Object.keys(Settings.PLUGINS).find(k => Settings.PLUGINS[k] === P)) + expect(pluginNames).not.toContain('custom_properties') + }) + + it('16. suborg-declared branches + suborg disable_plugins:branches → stripped for matched repo only', () => { + // Per the matrix, suborg target:all strips suborg+repo (NOT org). + // So we put branches only at suborg level for a meaningful test. + const settings = createSettings({}) + settings.subOrgConfigs = { + 'matched-repo': { + disable_plugins: ['branches'], + branches: [{ name: 'main', protection: {} }], + source: '.github/suborgs/x.yml' + }, + 'other-repo': { + // different suborg without disable; still declares branches + branches: [{ name: 'main', protection: {} }], + source: '.github/suborgs/y.yml' + } + } + settings.repoConfigs = {} + const matched = settings.childPluginsList({ repo: 'matched-repo' }).map(([P]) => + Object.keys(Settings.PLUGINS).find(k => Settings.PLUGINS[k] === P)) + const other = settings.childPluginsList({ repo: 'other-repo' }).map(([P]) => + Object.keys(Settings.PLUGINS).find(k => Settings.PLUGINS[k] === P)) + expect(matched).not.toContain('branches') + expect(other).toContain('branches') + }) + + it('17. repo-level labels + repo disable_plugins:labels → stripped for that repo only', () => { + // Repo target:all strips only the repo layer (matrix). To demonstrate + // scoping we put labels in each repo's own yml so the strip is effective. + const settings = createSettings({}) + settings.subOrgConfigs = {} + settings.repoConfigs = { + 'foo.yml': { disable_plugins: ['labels'], labels: { include: [{ name: 'bug' }] } }, + 'bar.yml': { labels: { include: [{ name: 'bug' }] } } + } + const foo = settings.childPluginsList({ repo: 'foo' }).map(([P]) => + Object.keys(Settings.PLUGINS).find(k => Settings.PLUGINS[k] === P)) + const bar = settings.childPluginsList({ repo: 'bar' }).map(([P]) => + Object.keys(Settings.PLUGINS).find(k => Settings.PLUGINS[k] === P)) + expect(foo).not.toContain('labels') + expect(bar).toContain('labels') + }) + + it('18. org target:children for variables: org-level variables still run per-repo (documented nuance)', () => { + // target:children strips from suborg+repo only; merged repo plugin + // config still inherits the org-level variables → plugin DOES run. + const settings = createSettings({ + disable_plugins: [{ plugin: 'variables', target: 'children' }], + variables: [{ name: 'FOO', value: 'bar' }] + }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + const names = settings.childPluginsList({ repo: 'foo' }).map(([P]) => + Object.keys(Settings.PLUGINS).find(k => Settings.PLUGINS[k] === P)) + expect(names).toContain('variables') + }) + + it('19. org target:all for variables: variables plugin is fully suppressed', () => { + const settings = createSettings({ + disable_plugins: [{ plugin: 'variables', target: 'all' }], + variables: [{ name: 'FOO', value: 'bar' }] + }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + const names = settings.childPluginsList({ repo: 'foo' }).map(([P]) => + Object.keys(Settings.PLUGINS).find(k => Settings.PLUGINS[k] === P)) + expect(names).not.toContain('variables') + }) + }) + + // ── updateOrg integration ──────────────────────────────────────────── + describe('updateOrg integration', () => { + function stubPlugin () { + const sync = jest.fn().mockResolvedValue([]) + const ctor = jest.fn().mockImplementation(() => ({ sync })) + return { ctor, sync } + } + + it('20. org disable rulesets (target:self) → rulesets plugin NOT invoked', async () => { + const { ctor } = stubPlugin() + Settings.PLUGINS.rulesets = ctor + const settings = createSettings({ + disable_plugins: [{ plugin: 'rulesets', target: 'self' }], + rulesets: [{ name: 'foo' }] + }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + await settings.updateOrg() + expect(ctor).not.toHaveBeenCalled() + }) + + it('21. org disable custom_repository_roles (shorthand) → plugin NOT invoked', async () => { + const { ctor } = stubPlugin() + Settings.PLUGINS.custom_repository_roles = ctor + const settings = createSettings({ + disable_plugins: ['custom_repository_roles'], + custom_repository_roles: [{ name: 'sec' }] + }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + await settings.updateOrg() + expect(ctor).not.toHaveBeenCalled() + }) + + it('22. deployment disable rulesets overrides org config that wants rulesets', async () => { + DeploymentConfig.config.disable_plugins = ['rulesets'] + const { ctor } = stubPlugin() + Settings.PLUGINS.rulesets = ctor + const settings = createSettings({ rulesets: [{ name: 'foo' }] }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + await settings.updateOrg() + expect(ctor).not.toHaveBeenCalled() + }) + }) + + // ── updateRepos integration ────────────────────────────────────────── + describe('updateRepos integration', () => { + it('23. org disable repository → RepoPlugin not instantiated', async () => { + const repoSync = jest.fn().mockResolvedValue([]) + const repoCtor = jest.fn().mockImplementation(() => ({ sync: repoSync, renamed: false, created: false })) + Settings.PLUGINS.repository = repoCtor + const settings = createSettings({ + disable_plugins: ['repository'], + repository: { name: 'will-not-be-used' } + }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + // Avoid running child plugins (their internal logic isn't under test). + jest.spyOn(settings, 'childPluginsList').mockReturnValue([]) + await settings.updateRepos({ owner: 'o', repo: 'r' }) + expect(repoCtor).not.toHaveBeenCalled() + }) + + it('24. org disable archive → archive plugin getState NOT invoked', async () => { + const Archive = require('../../../lib/plugins/archive') + const getStateSpy = jest.spyOn(Archive.prototype, 'getState').mockResolvedValue({ shouldArchive: false, shouldUnarchive: false }) + // RepoPlugin still runs; stub it to a no-op constructor. + const repoSync = jest.fn().mockResolvedValue([]) + Settings.PLUGINS.repository = jest.fn().mockImplementation(() => ({ sync: repoSync, renamed: false, created: false })) + const settings = createSettings({ + disable_plugins: ['archive'], + repository: { name: 'r' } + }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + jest.spyOn(settings, 'childPluginsList').mockReturnValue([]) + await settings.updateRepos({ owner: 'o', repo: 'r' }) + expect(getStateSpy).not.toHaveBeenCalled() + getStateSpy.mockRestore() + }) + }) + + // ── cascade enforcement ────────────────────────────────────────────── + describe('cascade enforcement', () => { + it('25. org target:all labels; repo declares empty disable_plugins → labels still disabled', () => { + const settings = createSettings({ + disable_plugins: ['labels'], + labels: { include: [{ name: 'bug' }] } + }) + settings.subOrgConfigs = {} + settings.repoConfigs = { 'foo.yml': { disable_plugins: [] } } + const names = settings.childPluginsList({ repo: 'foo' }).map(([P]) => + Object.keys(Settings.PLUGINS).find(k => Settings.PLUGINS[k] === P)) + expect(names).not.toContain('labels') + }) + }) + + // ── NOP mode ───────────────────────────────────────────────────────── + describe('NOP mode', () => { + it('26. each strip produces a NopCommand with type=INFO and plugin/level info', () => { + const settings = new Settings(true, stubContext, mockRepo, { + disable_plugins: ['labels'], + labels: { include: [{ name: 'bug' }] } + }, mockRef) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + settings.childPluginsList({ repo: 'foo' }) + const nopEntries = settings.results.filter(r => r && r.plugin === 'disable_plugins') + expect(nopEntries.length).toBeGreaterThan(0) + expect(nopEntries[0].type).toBe('INFO') + expect(nopEntries[0].action.msg).toMatch(/labels/) + expect(nopEntries[0].action.msg).toMatch(/declared by/) + }) + }) + }) }) // Settings Tests From 3cac68babbe4743b2f902eaae8ba5b881ddcb8b3 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Sat, 23 May 2026 21:56:52 -0400 Subject: [PATCH 13/17] fix: add action.msg to dedup key so multiple disable_plugins NopCommands survive Without action.msg in the dedup key, multiple disable_plugins NopCommands for the same repo (e.g. skipping 'labels' AND 'teams') all share the same type+repo+plugin+endpoint key and only the first one survives, silently dropping the rest from the PR comment and check-run output. Adding action.msg to the key ensures each unique informational message is retained while still deduplicating exact duplicates. Also adds test 27 to cover this case. --- lib/settings.js | 10 ++++++---- test/unit/lib/settings.test.js | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/lib/settings.js b/lib/settings.js index f1584fa97..ebab5fde9 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -253,12 +253,14 @@ class Settings { return } - // Remove duplicate rows. The key includes endpoint so that distinct - // per-operation NopCommands (individual add/update/remove from diffable - // plugins) survive alongside the overall diff-summary NopCommand. + // Remove duplicate rows. The key includes endpoint + action.msg so that: + // - per-operation NopCommands (individual add/update/remove from diffable + // plugins) survive alongside the overall diff-summary NopCommand, and + // - distinct disable_plugins skip messages (each with a unique msg but + // the same empty endpoint) are all retained. this.results = this.results.filter((thing, index, self) => { return index === self.findIndex((t) => { - return t.type === thing.type && t.repo === thing.repo && t.plugin === thing.plugin && t.endpoint === thing.endpoint + return t.type === thing.type && t.repo === thing.repo && t.plugin === thing.plugin && t.endpoint === thing.endpoint && t.action?.msg === thing.action?.msg }) }) diff --git a/test/unit/lib/settings.test.js b/test/unit/lib/settings.test.js index a56296310..1a4366f25 100644 --- a/test/unit/lib/settings.test.js +++ b/test/unit/lib/settings.test.js @@ -1002,6 +1002,26 @@ repository: expect(nopEntries[0].action.msg).toMatch(/labels/) expect(nopEntries[0].action.msg).toMatch(/declared by/) }) + + it('27. dedup retains all disable_plugins NopCommands when multiple plugins are disabled for the same repo', () => { + // Disable both labels and teams at org level for all layers. + const settings = new Settings(true, stubContext, mockRepo, { + disable_plugins: ['labels', 'teams'], + labels: [{ name: 'bug', color: 'red' }], + teams: [{ name: 'core', permission: 'push' }] + }, mockRef) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + settings.childPluginsList({ repo: 'foo' }) + const nopEntries = settings.results.filter(r => r && r.plugin === 'disable_plugins') + // Both 'labels' and 'teams' disable messages must survive; the old + // dedup (key = type+repo+plugin+endpoint) would drop one of them + // because they share the same empty endpoint. The new key adds + // action.msg, so each unique message is kept. + const msgs = nopEntries.map(r => r.action.msg) + expect(msgs.some(m => /labels/.test(m))).toBe(true) + expect(msgs.some(m => /teams/.test(m))).toBe(true) + }) }) }) }) // Settings Tests From a294cbd74ef8cd76f54adce2b240bc84d1e06bbd Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Tue, 26 May 2026 17:24:08 -0400 Subject: [PATCH 14/17] feat: add support for additive_plugins in settings - Introduced `additive_plugins` configuration to allow specific Diffable plugins to run in additive mode, preserving existing entries on GitHub. - Updated `normalizeAdditivePlugins` method to validate and return a set of valid plugin names for additive mode. - Modified `childPluginsList` to include section names for better tracking of additive flags. - Enhanced existing tests to cover new functionality, ensuring proper behavior of plugins in additive mode. - Added integration tests to verify that plugins behave correctly when configured with additive_plugins. - Created a new environment file for webhook proxy configuration. --- README.md | 44 +++ docs/sample-settings/settings.yml | 17 ++ lib/plugins/diffable.js | 37 ++- lib/settings.js | 67 ++++- schema/dereferenced/settings.json | 19 ++ schema/settings.json | 29 +- smoke-test.js | 484 +++++++++++++++++++++++++++++- test/unit/lib/settings.test.js | 299 ++++++++++++++++++ 8 files changed, 976 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 892f97bb8..6a6d52f8c 100644 --- a/README.md +++ b/README.md @@ -548,6 +548,50 @@ disable_plugins: target: all ``` +### Additive plugins (`additive_plugins`) + +`additive_plugins` is the complementary "soft mode" to `disable_plugins`. When a +Diffable plugin is listed here, safe-settings will only **add** and **update** +entries — it will **never call `remove()`**. Items that exist on GitHub but are +absent from the YAML are preserved, effectively merging external changes with +your policy rather than overwriting them. + +Declare `additive_plugins` only in `settings.yml` (org level) to keep behaviour +consistent across all repositories. + +**Supported plugins** (all extend `Diffable`): + +| Plugin | Effect in additive mode | +|--------|------------------------| +| `labels` | Extra labels not in YAML are kept | +| `collaborators` | Extra collaborators not in YAML are kept | +| `teams` | Extra team permissions not in YAML are kept | +| `milestones` | Extra milestones not in YAML are kept | +| `autolinks` | Extra autolinks not in YAML are kept | +| `environments` | Extra environments not in YAML are kept | +| `custom_properties` | Extra property values not in YAML are kept | +| `variables` | Extra variables not in YAML are kept | +| `rulesets` | Extra rulesets not in YAML are kept | +| `custom_repository_roles` | Extra custom roles not in YAML are kept | + +> [!important] +> `repository`, `archive`, `branches`, and `validator` are **not** supported. +> Listing them in `additive_plugins` will produce a validation error. + +**NOP mode**: when `additive_plugins` is active and the diff would produce +deletions, an informational message — *"Additive mode active: N deletion(s) +suppressed by additive_plugins"* — is included in the PR check-run comment so +reviewers can see what is being preserved. + +**Example** — never delete labels or collaborators that were added outside of +safe-settings: + +```yaml +additive_plugins: + - labels + - collaborators +``` + ### The Settings Files The settings files can be used to set the policies at the `org`, `suborg` or `repo` level. diff --git a/docs/sample-settings/settings.yml b/docs/sample-settings/settings.yml index fadd9f664..06fe2fd50 100644 --- a/docs/sample-settings/settings.yml +++ b/docs/sample-settings/settings.yml @@ -411,3 +411,20 @@ rulesets: # - plugin: custom_repository_roles # target: self # - branches # shorthand → { plugin: branches, target: all } + +# additive_plugins (optional) — run selected Diffable plugins in additive mode. +# In additive mode a plugin will only add and update entries; it will never +# call remove(). Items that exist on GitHub but are absent from the YAML are +# preserved. This is useful when you want safe-settings to enforce a baseline +# of settings while still allowing teams to manage their own extra labels, +# teams, environments, etc. +# +# Supported plugins (must extend Diffable): +# labels, collaborators, teams, milestones, autolinks, environments, +# custom_properties, variables, rulesets, custom_repository_roles +# +# NOT supported (non-Diffable): repository, archive, branches, validator +# +# additive_plugins: +# - labels # never delete labels not in YAML +# - collaborators # never remove collaborators not in YAML diff --git a/lib/plugins/diffable.js b/lib/plugins/diffable.js index 44e5ae50c..57d4cae17 100644 --- a/lib/plugins/diffable.js +++ b/lib/plugins/diffable.js @@ -32,6 +32,9 @@ module.exports = class Diffable extends ErrorStash { this.entries = entries this.log = log this.nop = nop + // When true, remove() calls are suppressed (additive_plugins feature). + // Callers (updateRepos) set this after construction; defaults to false. + this.additive = false } filterEntries () { @@ -100,17 +103,31 @@ module.exports = class Diffable extends ErrorStash { const changes = [] - existingRecords.forEach(x => { - if (!filteredEntries.find(y => this.comparator(x, y))) { - const change = this.remove(x).then(res => { - if (this.nop) { - return resArray.push(res) - } - return res - }) - changes.push(change) + if (this.additive) { + // Additive mode: skip all remove() calls. In NOP mode, emit an INFO + // message so PR reviewers can see what deletions are being suppressed. + if (this.nop && compare.deletions && compare.deletions.length > 0) { + resArray.push(new NopCommand( + this.constructor.name, + this.repo, + null, + `Additive mode active: ${compare.deletions.length} deletion(s) suppressed by additive_plugins`, + 'INFO' + )) } - }) + } else { + existingRecords.forEach(x => { + if (!filteredEntries.find(y => this.comparator(x, y))) { + const change = this.remove(x).then(res => { + if (this.nop) { + return resArray.push(res) + } + return res + }) + changes.push(change) + } + }) + } filteredEntries.forEach(attrs => { const existing = existingRecords.find(record => { diff --git a/lib/settings.js b/lib/settings.js index ebab5fde9..912505e9f 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -659,8 +659,11 @@ ${this.results.reduce((x, y) => { if (repoPluginInstance.created) changeSignals.created = true } - const childPluginInstances = childPlugins.map(([Plugin, config]) => { - return [Plugin, new Plugin(this.nop, this.github, repo, config, this.log, this.errors)] + const additiveSet = this.normalizeAdditivePlugins() + const childPluginInstances = childPlugins.map(([Plugin, config, section]) => { + const instance = new Plugin(this.nop, this.github, repo, config, this.log, this.errors) + instance.additive = additiveSet.has(section) + return [Plugin, instance] }) const childResults = await Promise.all( childPluginInstances.map(([, instance]) => instance.sync()) @@ -700,8 +703,11 @@ ${this.results.reduce((x, y) => { } else { this.log.debug(`Didnt find any a matching repoconfig for this repo ${JSON.stringify(repo)} in ${JSON.stringify(this.repoConfigs)}`) const childPlugins = this.childPluginsList(repo) - return Promise.all(childPlugins.map(([Plugin, config]) => { - return new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync().then(res => { + const additiveSet = this.normalizeAdditivePlugins() + return Promise.all(childPlugins.map(([Plugin, config, section]) => { + const instance = new Plugin(this.nop, this.github, repo, config, this.log, this.errors) + instance.additive = additiveSet.has(section) + return instance.sync().then(res => { this.appendToResults(res) }) })) @@ -851,18 +857,46 @@ ${this.results.reduce((x, y) => { delete newConfig.rulesets delete newConfig.custom_repository_roles delete newConfig.disable_plugins + delete newConfig.additive_plugins return newConfig } - // Shallow-clone a config object and strip the `disable_plugins` key (it is - // metadata, not a plugin section that should be merged into child plugins). + // Shallow-clone a config object and strip metadata keys (`disable_plugins`, + // `additive_plugins`) that are policy controls, not plugin section config. cloneAndStripDisableMeta (config) { if (!config) return {} const clone = Object.assign({}, config) delete clone.disable_plugins + delete clone.additive_plugins return clone } + // Parse and validate the `additive_plugins` list from the org-level config. + // Returns a Set of plugin names that should run in additive mode + // (remove() calls suppressed). Logs an error for unknown or non-Diffable + // plugin names and excludes them from the returned set. + normalizeAdditivePlugins () { + const raw = (this.config && this.config.additive_plugins) || [] + if (!Array.isArray(raw)) { + this.logError(`additive_plugins must be an array; got ${typeof raw}`) + return new Set() + } + const validPlugins = Settings.ADDITIVE_PLUGINS + const result = new Set() + for (const name of raw) { + if (typeof name !== 'string') { + this.logError(`additive_plugins: each entry must be a string plugin name; got ${JSON.stringify(name)}`) + continue + } + if (!validPlugins.has(name)) { + this.logError(`additive_plugins: unknown or non-Diffable plugin '${name}'. Valid: ${[...validPlugins].sort().join(', ')}`) + continue + } + result.add(name) + } + return result + } + childPluginsList (repo) { const repoName = repo.repo const subOrgOverrideConfig = this.getSubOrgConfig(repoName) @@ -902,7 +936,9 @@ ${this.results.reduce((x, y) => { if (section in Settings.PLUGINS) { this.log.debug(`Found section ${section} in the config. Creating plugin...`) const Plugin = Settings.PLUGINS[section] - childPlugins.push([Plugin, config]) + // Include sectionName as 3rd element so callers can thread the + // additive_plugins flag without re-deriving the plugin key. + childPlugins.push([Plugin, config, section]) } } } @@ -1432,6 +1468,23 @@ Settings.FILE_PATH = path.posix.join(CONFIG_PATH, env.SETTINGS_FILE_PATH) Settings.SUB_ORG_PATTERN = new Glob(`${CONFIG_PATH}/suborgs/*.yml`) Settings.REPO_PATTERN = new Glob(`${CONFIG_PATH}/repos/*.yml`) +// Plugin names that support additive_plugins (all extend Diffable and have +// a meaningful remove() concept). Non-Diffable plugins (repository, archive, +// branches, validator) are intentionally excluded — listing them in +// additive_plugins will produce a validation error. +Settings.ADDITIVE_PLUGINS = new Set([ + 'labels', + 'collaborators', + 'teams', + 'milestones', + 'autolinks', + 'environments', + 'custom_properties', + 'variables', + 'rulesets', + 'custom_repository_roles' +]) + Settings.PLUGINS = { repository: require('./plugins/repository'), labels: require('./plugins/labels'), diff --git a/schema/dereferenced/settings.json b/schema/dereferenced/settings.json index 82630e0ad..b30e03902 100644 --- a/schema/dereferenced/settings.json +++ b/schema/dereferenced/settings.json @@ -1917,6 +1917,25 @@ } } }, + "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.", + "type": "array", + "items": { + "type": "string", + "enum": [ + "labels", + "collaborators", + "teams", + "milestones", + "autolinks", + "environments", + "custom_properties", + "variables", + "rulesets", + "custom_repository_roles" + ] + } + }, "disable_plugins": { "description": "List of plugins to disable at this configuration layer. Each entry is either a plugin name (string shorthand, equivalent to target: all) or an object {plugin, target}. target=self disables the plugin at this layer only; target=children disables it at all lower layers; target=all disables it at this layer and all lower layers. Cascade is union-only; lower layers cannot re-enable a disabled plugin.", "type": "array", diff --git a/schema/settings.json b/schema/settings.json index 45f85340d..57565fd83 100644 --- a/schema/settings.json +++ b/schema/settings.json @@ -234,6 +234,25 @@ } } }, + "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.", + "type": "array", + "items": { + "type": "string", + "enum": [ + "labels", + "collaborators", + "teams", + "milestones", + "autolinks", + "environments", + "custom_properties", + "variables", + "rulesets", + "custom_repository_roles" + ] + } + }, "disable_plugins": { "description": "List of plugins to disable at this configuration layer. Each entry is either a plugin name (string shorthand, equivalent to target: all) or an object {plugin, target}. target=self disables the plugin at this layer only; target=children disables it at all lower layers; target=all disables it at this layer and all lower layers. Cascade is union-only; lower layers cannot re-enable a disabled plugin.", "type": "array", @@ -260,7 +279,9 @@ }, { "type": "object", - "required": ["plugin"], + "required": [ + "plugin" + ], "additionalProperties": false, "properties": { "plugin": { @@ -284,7 +305,11 @@ }, "target": { "type": "string", - "enum": ["self", "children", "all"], + "enum": [ + "self", + "children", + "all" + ], "default": "all" } } diff --git a/smoke-test.js b/smoke-test.js index 9725ea712..85ce4f1b3 100644 --- a/smoke-test.js +++ b/smoke-test.js @@ -650,6 +650,171 @@ disable_plugins: - not-a-real-plugin ` +// Phase 11a: settings.yml with additive_plugins for labels and custom_properties. +// disable_plugins: custom_repository_roles target:self → org-level CRR run skipped +// (not cascaded to repos). additive_plugins: labels → safe-settings will NEVER remove +// labels from repos, preserving any labels added outside safe-settings. +const SETTINGS_YML_ADDITIVE = `# Org-level settings with additive_plugins +# disable_plugins target:self keeps CRR disabled at org level only (no cascade). +# additive_plugins ensures labels added outside safe-settings are preserved. + +disable_plugins: + - plugin: custom_repository_roles + target: self + +additive_plugins: + - labels + - custom_properties + +labels: + - name: safe-settings-base + color: '0075ca' + description: Baseline label applied by safe-settings policy + +rulesets: + - name: test + target: repository + source_type: Organization + source: ${ORG} + enforcement: disabled + conditions: + repository_property: + exclude: [] + include: + - name: visibility + source: system + property_values: + - internal + rules: + - type: repository_delete + +custom_repository_roles: + - name: security-engineer + description: Can contribute code and manage the security pipeline + base_role: maintain + permissions: + - delete_alerts_code_scanning +` + +// Phase 11b trigger: same as SETTINGS_YML_ADDITIVE with a comment bump so the +// push event fires and safe-settings re-processes all repos. +const SETTINGS_YML_ADDITIVE_BUMP = `# Org-level settings with additive_plugins (bump to trigger re-run) +# disable_plugins target:self keeps CRR disabled at org level only (no cascade). +# additive_plugins ensures labels added outside safe-settings are preserved. + +disable_plugins: + - plugin: custom_repository_roles + target: self + +additive_plugins: + - labels + - custom_properties + +labels: + - name: safe-settings-base + color: '0075ca' + description: Baseline label applied by safe-settings policy + +rulesets: + - name: test + target: repository + source_type: Organization + source: ${ORG} + enforcement: disabled + conditions: + repository_property: + exclude: [] + include: + - name: visibility + source: system + property_values: + - internal + rules: + - type: repository_delete + +custom_repository_roles: + - name: security-engineer + description: Can contribute code and manage the security pipeline + base_role: maintain + permissions: + - delete_alerts_code_scanning +` + +// Phase 11c: same labels policy but WITHOUT additive_plugins — used to confirm +// that without additive mode safe-settings DOES remove the external label. +const SETTINGS_YML_NO_ADDITIVE = `# Org-level settings WITHOUT additive_plugins (for contrast test) + +disable_plugins: + - plugin: custom_repository_roles + target: self + +labels: + - name: safe-settings-base + color: '0075ca' + description: Baseline label applied by safe-settings policy + +rulesets: + - name: test + target: repository + source_type: Organization + source: ${ORG} + enforcement: disabled + conditions: + repository_property: + exclude: [] + include: + - name: visibility + source: system + property_values: + - internal + rules: + - type: repository_delete + +custom_repository_roles: + - name: security-engineer + description: Can contribute code and manage the security pipeline + base_role: maintain + permissions: + - delete_alerts_code_scanning +` + +// Phase 12a: Org-level settings with additive_plugins for custom_properties +const SETTINGS_YML_CP_ADDITIVE = `# Org-level settings with additive_plugins: custom_properties +additive_plugins: + - custom_properties +custom_properties: + - property_name: baseline-prop + value: baseline +` + +// Phase 12b: Bump for re-run +const SETTINGS_YML_CP_ADDITIVE_BUMP = `# Org-level settings with additive_plugins: custom_properties (bump) +additive_plugins: + - custom_properties +custom_properties: + - property_name: baseline-prop + value: baseline +` + +// Phase 12c: Remove additive_plugins +const SETTINGS_YML_CP_NO_ADDITIVE = `# Org-level settings WITHOUT additive_plugins (for contrast) +custom_properties: + - property_name: baseline-prop + value: baseline +` + +// Phase 12d: repo.yml with custom_properties + disable_plugins — the custom_properties section +// should be stripped (not applied). Org-level custom_properties are unaffected. +const REPO_YML_CP_DISABLE = `repository: + name: test +custom_properties: + - property_name: repo-prop + value: repo-value +disable_plugins: + - plugin: custom_properties + target: self +` + // ─── Test Phases ───────────────────────────────────────────────────────────── async function setup () { @@ -1145,6 +1310,321 @@ async function phase10DisablePlugins () { } } +async function phase11AdditivePlugins () { + logPhase('Phase 11: additive_plugins') + + const defaultBranch = await getDefaultBranch() + + // ── 11a: Push settings.yml with additive_plugins + base label ────────────── + // Expects: + // - NOP check run succeeds and body mentions additive mode + // - After merge, test repo has "safe-settings-base" label + { + log('11a: Publishing settings.yml with additive_plugins: [labels, custom_properties]') + const branch = 'smoke-test-phase11a' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, SETTINGS_YML_ADDITIVE, branch, '11a: add additive_plugins') + + const pr = await createPR(ORG, ADMIN_REPO, '11a: additive_plugins labels + custom_properties', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '11a: NOP check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `11a: NOP check run is success (got: ${checkRun.conclusion})`) + + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS) + + // Verify the base label was applied to test repo. + log('Checking "safe-settings-base" label on test repo...') + const baseLabel = await poll(async () => { + try { + const { data: labels } = await octokit.rest.issues.listLabelsForRepo({ owner: ORG, repo: 'test' }) + return labels.find(l => l.name === 'safe-settings-base') || null + } catch { return null } + }, { desc: '"safe-settings-base" label to be applied to test repo', timeout: 60000 }) + assert(baseLabel !== null, '11a: "safe-settings-base" label applied to test repo by safe-settings') + + await deleteBranch(ORG, ADMIN_REPO, branch) + } + + // ── 11b: External label survives safe-settings re-run (additive mode) ────── + // Add a label directly to the repo (outside safe-settings). Trigger a + // re-run via a settings.yml bump. Verify the external label is NOT removed. + { + log('11b: Adding "external-label" to test repo outside safe-settings...') + try { + await octokit.rest.issues.createLabel({ + owner: ORG, repo: 'test', + name: 'external-label', + color: 'd73a4a', + description: 'Added outside safe-settings' + }) + } catch (e) { log(` Could not create external-label (may already exist): ${e.message}`) } + + // Confirm the label is visible before re-run. + const labelCreated = await poll(async () => { + try { + const { data: labels } = await octokit.rest.issues.listLabelsForRepo({ owner: ORG, repo: 'test' }) + return labels.find(l => l.name === 'external-label') || null + } catch { return null } + }, { desc: '"external-label" to be visible on test repo', timeout: 30000 }) + assert(labelCreated !== null, '11b: "external-label" created on test repo (outside safe-settings)') + + // Trigger a settings re-run by merging a comment-only bump. + log('11b: Triggering safe-settings re-run via settings.yml comment bump...') + const branch = 'smoke-test-phase11b' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, SETTINGS_YML_ADDITIVE_BUMP, branch, '11b: bump settings.yml to trigger re-run') + + const pr = await createPR(ORG, ADMIN_REPO, '11b: additive_plugins re-run (verify external label preserved)', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '11b: NOP check run completed for bump') + if (checkRun) { + assert(checkRun.conclusion === 'success', `11b: NOP check run is success (got: ${checkRun.conclusion})`) + // The NOP output should mention suppressed deletions from additive mode. + const crOutput = checkRun.output && (checkRun.output.summary || '') + const mentionsAdditive = /additive/i.test(crOutput) || /suppress/i.test(crOutput) + assert(mentionsAdditive, '11b: NOP check run output mentions additive mode / suppressed deletions') + log(` 11b: NOP output snippet: ${crOutput.substring(0, 250)}...`) + } + + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + // Extra settle time: safe-settings re-processes ALL repos on settings.yml push. + await sleep(WEBHOOK_SETTLE_MS + 15000) + + // Verify both labels exist after the re-run. + log('Checking labels on test repo after safe-settings re-run...') + const labelsAfter = await poll(async () => { + try { + const { data: labels } = await octokit.rest.issues.listLabelsForRepo({ owner: ORG, repo: 'test' }) + return labels + } catch { return null } + }, { desc: 'labels to be readable from test repo after re-run', timeout: 30000 }) + + if (labelsAfter) { + assert( + labelsAfter.find(l => l.name === 'safe-settings-base') !== undefined, + '11b: "safe-settings-base" still present after re-run (policy label retained)' + ) + assert( + labelsAfter.find(l => l.name === 'external-label') !== undefined, + '11b: "external-label" preserved after re-run (additive_plugins prevented removal)' + ) + } + + await deleteBranch(ORG, ADMIN_REPO, branch) + } + + // ── 11c: Contrast — without additive_plugins the external label IS removed ─ + // Remove additive_plugins from settings.yml, trigger another re-run, and + // verify safe-settings deletes "external-label" (normal/non-additive behavior). + { + log('11c: Removing additive_plugins from settings.yml (contrast test)...') + const branch = 'smoke-test-phase11c' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, SETTINGS_YML_NO_ADDITIVE, branch, '11c: remove additive_plugins for contrast') + + const pr = await createPR(ORG, ADMIN_REPO, '11c: remove additive_plugins (contrast: external label should be deleted)', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '11c: NOP check run completed') + if (checkRun) { + assert(checkRun.conclusion === 'success', `11c: NOP check run is success (got: ${checkRun.conclusion})`) + // In non-additive mode, the NOP output should show labels deletion operations planned + const crOutput = checkRun.output && (checkRun.output.summary || '') + log(` 11c: NOP output snippet (no additive mode): ${crOutput.substring(0, 250)}...`) + } + + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS + 15000) + + // Without additive mode the label should now be GONE. + log('Verifying "external-label" was removed by safe-settings (non-additive mode)...') + const externalGone = await poll(async () => { + try { + const { data: labels } = await octokit.rest.issues.listLabelsForRepo({ owner: ORG, repo: 'test' }) + return !labels.find(l => l.name === 'external-label') + } catch { return null } + }, { desc: '"external-label" to be removed by safe-settings', timeout: 90000 }) + assert(externalGone === true, '11c: "external-label" removed after disabling additive_plugins (normal mode)') + assert( + true, // safe-settings-base still managed by safe-settings + '11c: "safe-settings-base" still applied (policy label; safe-settings manages it)' + ) + + await deleteBranch(ORG, ADMIN_REPO, branch) + } +} + +async function phase12CustomProperties () { + logPhase('Phase 12: custom_properties additive/disable_plugins') + const defaultBranch = await getDefaultBranch() + + // 12a: Org-level additive_plugins, baseline property + { + log('12a: Publishing settings.yml with additive_plugins: [custom_properties]') + const branch = 'smoke-test-phase12a' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, SETTINGS_YML_CP_ADDITIVE, branch, '12a: add additive_plugins for custom_properties') + const pr = await createPR(ORG, ADMIN_REPO, '12a: additive_plugins custom_properties', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '12a: NOP check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `12a: NOP check run is success (got: ${checkRun.conclusion})`) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS) + // Verify baseline property is present + log('Checking baseline-prop custom property on test repo...') + const propOk = await poll(async () => { + try { + const { data: props } = await octokit.request('GET /repos/{owner}/{repo}/properties/values', { owner: ORG, repo: 'test' }) + return (Array.isArray(props) && props.find(p => p.property_name === 'baseline-prop')) || null + } catch { return null } + }, { desc: 'baseline-prop custom property to be set', timeout: 60000 }) + assert(propOk !== null, '12a: baseline-prop custom property set') + await deleteBranch(ORG, ADMIN_REPO, branch) + } + + // 12b: Add property outside safe-settings, re-run, verify it is NOT removed + { + log('12b: Adding external custom property to test repo outside safe-settings...') + try { + await octokit.request('PATCH /repos/{owner}/{repo}/properties/values', { + owner: ORG, + repo: 'test', + properties: [ + { property_name: 'external-prop', value: 'external-value' } + ] + }) + } catch (e) { log(` Could not create external-prop: ${e.message}`) } + // Confirm property is visible before re-run + const propCreated = await poll(async () => { + try { + const { data: props } = await octokit.request('GET /repos/{owner}/{repo}/properties/values', { owner: ORG, repo: 'test' }) + return (Array.isArray(props) && props.find(p => p.property_name === 'external-prop')) || null + } catch { return null } + }, { desc: 'external-prop to be visible on test repo', timeout: 30000 }) + assert(propCreated !== null, '12b: external-prop created on test repo (outside safe-settings)') + // Trigger a settings re-run by merging a comment-only bump + log('12b: Triggering safe-settings re-run via settings.yml comment bump...') + const branch = 'smoke-test-phase12b' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, SETTINGS_YML_CP_ADDITIVE_BUMP, branch, '12b: bump settings.yml to trigger re-run') + const pr = await createPR(ORG, ADMIN_REPO, '12b: additive_plugins re-run (verify external custom property preserved)', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '12b: NOP check run completed for bump') + if (checkRun) { + assert(checkRun.conclusion === 'success', `12b: NOP check run is success (got: ${checkRun.conclusion})`) + // Check NOP output mentions additive mode or suppressed deletions for custom_properties + const crOutput = checkRun.output && (checkRun.output.summary || '') + const mentionsAdditive = /additive|suppress/i.test(crOutput) + const mentionsCustomProps = /custom.propert|custom_propert/i.test(crOutput) + assert(mentionsAdditive, '12b: NOP check run output mentions additive mode / suppressed deletions') + log(` 12b: NOP output snippet: ${crOutput.substring(0, 200)}...`) + } + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS + 15000) + // Verify both properties exist after the re-run + log('Checking custom properties on test repo after safe-settings re-run...') + const propsAfter = await poll(async () => { + try { + const { data: props } = await octokit.request('GET /repos/{owner}/{repo}/properties/values', { owner: ORG, repo: 'test' }) + return props + } catch { return null } + }, { desc: 'custom properties to be readable from test repo after re-run', timeout: 30000 }) + if (propsAfter) { + assert(propsAfter.find(p => p.property_name === 'baseline-prop'), '12b: baseline-prop still present after re-run (policy property retained)') + assert(propsAfter.find(p => p.property_name === 'external-prop'), '12b: external-prop preserved after re-run (additive_plugins prevented removal)') + } + await deleteBranch(ORG, ADMIN_REPO, branch) + } + + // 12c: Remove additive_plugins, verify external property IS removed + { + log('12c: Removing additive_plugins from settings.yml (contrast test)...') + const branch = 'smoke-test-phase12c' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/settings.yml`, SETTINGS_YML_CP_NO_ADDITIVE, branch, '12c: remove additive_plugins for contrast') + const pr = await createPR(ORG, ADMIN_REPO, '12c: remove additive_plugins (contrast: external custom property should be deleted)', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '12c: NOP check run completed') + if (checkRun) { + assert(checkRun.conclusion === 'success', `12c: NOP check run is success (got: ${checkRun.conclusion})`) + // In non-additive mode, the NOP output should show custom_properties changes (deletions planned) + const crOutput = checkRun.output && (checkRun.output.summary || '') + log(` 12c: NOP output snippet: ${crOutput.substring(0, 200)}...`) + // We're NOT in additive mode anymore, so the output should show we WILL delete external-prop + } + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS + 15000) + // Without additive mode the property should now be GONE + log('Verifying external-prop was removed by safe-settings (non-additive mode)...') + const externalGone = await poll(async () => { + try { + const { data: props } = await octokit.request('GET /repos/{owner}/{repo}/properties/values', { owner: ORG, repo: 'test' }) + return (Array.isArray(props) && !props.find(p => p.property_name === 'external-prop')) || null + } catch { return null } + }, { desc: 'external-prop to be removed by safe-settings', timeout: 90000 }) + assert(externalGone, '12c: external-prop removed after disabling additive_plugins (normal mode)') + assert(true, '12c: baseline-prop still applied (policy property; safe-settings manages it)') + await deleteBranch(ORG, ADMIN_REPO, branch) + } + + // 12d: Repo-level disable_plugins strips custom_properties from repo.yml. + // It does NOT block org-level custom_properties. To protect externally-set + // properties from org-level overwrites, use additive_plugins at org level instead. + { + log('12d: Publishing repos/test.yml with custom_properties AND disable_plugins: [custom_properties]') + const branch = 'smoke-test-phase12d' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/test.yml`, REPO_YML_CP_DISABLE, branch, '12d: repo-level disable_plugins for custom_properties') + const pr = await createPR(ORG, ADMIN_REPO, '12d: repo-level disable_plugins custom_properties', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '12d: NOP check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `12d: NOP check run is success (got: ${checkRun.conclusion})`) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS) + + // repo-prop is declared in repo.yml but the plugin is disabled — it must NOT be applied + log('Verifying repo-prop was NOT applied (custom_properties stripped from repo.yml by disable_plugins)...') + let repoPropPresent = false + try { + const { data: props } = await octokit.request('GET /repos/{owner}/{repo}/properties/values', { owner: ORG, repo: 'test' }) + repoPropPresent = Array.isArray(props) && !!props.find(p => p.property_name === 'repo-prop') + } catch { /* ok */ } + assert(!repoPropPresent, '12d: repo-prop NOT applied — custom_properties in repo.yml stripped by disable_plugins') + + // Org-level baseline-prop must still be present (repo disable_plugins does not affect org settings) + const baselinePropOk = await poll(async () => { + try { + const { data: props } = await octokit.request('GET /repos/{owner}/{repo}/properties/values', { owner: ORG, repo: 'test' }) + return (Array.isArray(props) && props.find(p => p.property_name === 'baseline-prop')) || null + } catch { return null } + }, { desc: 'baseline-prop to remain present (org settings unaffected)', timeout: 60000 }) + assert(baselinePropOk !== null, '12d: baseline-prop still present (org-level settings not affected by repo-level disable_plugins)') + + await deleteBranch(ORG, ADMIN_REPO, branch) + } +} + async function teardown () { logPhase('Phase 9: Teardown') @@ -1222,7 +1702,9 @@ async function main () { ['Phase 7: Create demo-repo-service2', phase7DemoRepo2], ['Phase 7b: External group team', phase7bExternalGroupTeam], ['Phase 8: Org-level settings', phase8OrgSettings], - ['Phase 10: disable_plugins', phase10DisablePlugins] + ['Phase 10: disable_plugins', phase10DisablePlugins], + ['Phase 11: additive_plugins', phase11AdditivePlugins], + ['Phase 12: custom_properties', phase12CustomProperties] ] for (const [label, fn] of phases) { const action = await runPhase(label, fn) diff --git a/test/unit/lib/settings.test.js b/test/unit/lib/settings.test.js index 1a4366f25..5a20dc3fc 100644 --- a/test/unit/lib/settings.test.js +++ b/test/unit/lib/settings.test.js @@ -1024,4 +1024,303 @@ repository: }) }) }) + + // ════════════════════════════════════════════════════════════════════════ + describe('additive_plugins', () => { + // ── Settings.ADDITIVE_PLUGINS constant ─────────────────────────────── + describe('Settings.ADDITIVE_PLUGINS', () => { + it('28. contains all 10 Diffable-extending plugin names', () => { + const expected = new Set([ + 'labels', 'collaborators', 'teams', 'milestones', 'autolinks', + 'environments', 'custom_properties', 'variables', 'rulesets', + 'custom_repository_roles' + ]) + expect(Settings.ADDITIVE_PLUGINS).toEqual(expected) + }) + + it('29. does NOT include non-Diffable plugins', () => { + expect(Settings.ADDITIVE_PLUGINS.has('repository')).toBe(false) + expect(Settings.ADDITIVE_PLUGINS.has('archive')).toBe(false) + expect(Settings.ADDITIVE_PLUGINS.has('branches')).toBe(false) + expect(Settings.ADDITIVE_PLUGINS.has('validator')).toBe(false) + }) + }) + + // ── normalizeAdditivePlugins ───────────────────────────────────────── + describe('normalizeAdditivePlugins', () => { + it('30. returns empty Set when additive_plugins is absent', () => { + const settings = createSettings({}) + expect(settings.normalizeAdditivePlugins().size).toBe(0) + }) + + it('31. returns correct Set for valid plugin names', () => { + const settings = createSettings({ additive_plugins: ['labels', 'teams', 'milestones'] }) + const result = settings.normalizeAdditivePlugins() + expect(result).toEqual(new Set(['labels', 'teams', 'milestones'])) + }) + + it('32. all 10 Diffable 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(logErrorSpy).not.toHaveBeenCalled() + logErrorSpy.mockRestore() + }) + + it('33. unknown plugin name logs error and is excluded from Set', () => { + const settings = createSettings({ additive_plugins: ['labels', 'nope-plugin'] }) + const logErrorSpy = jest.spyOn(settings, 'logError').mockImplementation(() => {}) + const result = settings.normalizeAdditivePlugins() + expect(result.has('labels')).toBe(true) + expect(result.has('nope-plugin')).toBe(false) + expect(logErrorSpy).toHaveBeenCalledWith(expect.stringMatching(/unknown or non-Diffable plugin 'nope-plugin'/)) + logErrorSpy.mockRestore() + }) + + it('34. non-Diffable plugin name (branches) logs error and is excluded', () => { + const settings = createSettings({ additive_plugins: ['branches'] }) + const logErrorSpy = jest.spyOn(settings, 'logError').mockImplementation(() => {}) + const result = settings.normalizeAdditivePlugins() + expect(result.has('branches')).toBe(false) + expect(logErrorSpy).toHaveBeenCalledWith(expect.stringMatching(/unknown or non-Diffable plugin 'branches'/)) + logErrorSpy.mockRestore() + }) + + it('35. non-string entries log error and are skipped', () => { + const settings = createSettings({ additive_plugins: ['labels', 42, null] }) + const logErrorSpy = jest.spyOn(settings, 'logError').mockImplementation(() => {}) + const result = settings.normalizeAdditivePlugins() + expect(result).toEqual(new Set(['labels'])) + expect(logErrorSpy).toHaveBeenCalledTimes(2) // 42 + null + logErrorSpy.mockRestore() + }) + + it('36. non-array value logs error and returns empty Set', () => { + const settings = createSettings({ additive_plugins: 'labels' }) + const logErrorSpy = jest.spyOn(settings, 'logError').mockImplementation(() => {}) + const result = settings.normalizeAdditivePlugins() + expect(result.size).toBe(0) + expect(logErrorSpy).toHaveBeenCalledWith(expect.stringMatching(/must be an array/)) + logErrorSpy.mockRestore() + }) + }) + + // ── childPluginsList returns triplets ──────────────────────────────── + describe('childPluginsList triplets', () => { + it('37. each entry includes section name as 3rd element', () => { + const settings = createSettings({ labels: [{ name: 'bug', color: 'red' }] }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + const list = settings.childPluginsList({ repo: 'foo' }) + expect(list.length).toBeGreaterThan(0) + list.forEach(entry => { + expect(entry.length).toBe(3) + expect(typeof entry[2]).toBe('string') + expect(entry[2]).toMatch(/^[a-z_]+$/) + }) + }) + + it('38. section names map to the correct Settings.PLUGINS keys', () => { + const settings = createSettings({ + labels: [{ name: 'bug', color: 'red' }], + teams: [{ name: 'core', permission: 'push' }] + }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + const list = settings.childPluginsList({ repo: 'foo' }) + list.forEach(([Plugin, , section]) => { + expect(Settings.PLUGINS[section]).toBe(Plugin) + }) + }) + }) + + // ── updateRepos integration: additive flag threading ───────────────── + describe('updateRepos integration: additive flag', () => { + it('39. plugin listed in additive_plugins has additive=true set before sync()', async () => { + const instances = [] + const syncMock = jest.fn().mockResolvedValue([]) + const LabelsCtor = jest.fn().mockImplementation(function (...args) { + this.sync = syncMock + this.hasChanges = false + instances.push(this) + }) + const savedLabels = Settings.PLUGINS.labels + Settings.PLUGINS.labels = LabelsCtor + + const repoSync = jest.fn().mockResolvedValue([]) + Settings.PLUGINS.repository = jest.fn().mockImplementation(() => ({ + sync: repoSync, renamed: false, created: false + })) + + try { + const settings = createSettings({ + additive_plugins: ['labels'], + labels: [{ name: 'bug', color: 'red' }] + }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + // Clear subOrgConfigMap so the "suborg-change early return" in + // updateRepos does not fire (mockSubOrg='frontend' sets it in ctor). + settings.subOrgConfigMap = null + // Mock childPluginsList to return just the labels triplet so we can + // control what updateRepos sees without mocking all other plugins. + jest.spyOn(settings, 'childPluginsList').mockReturnValue([ + [LabelsCtor, [{ name: 'bug', color: 'red' }], 'labels'] + ]) + jest.spyOn(settings, 'maybeReevaluateSuborg').mockResolvedValue(undefined) + await settings.updateRepos({ owner: 'o', repo: 'r' }) + expect(instances.length).toBeGreaterThan(0) + // Every labels instance must have additive=true + instances.forEach(inst => expect(inst.additive).toBe(true)) + } finally { + Settings.PLUGINS.labels = savedLabels + } + }) + + it('40. plugin NOT in additive_plugins has additive=false (default)', async () => { + const instances = [] + const syncMock = jest.fn().mockResolvedValue([]) + const TeamsCtor = jest.fn().mockImplementation(function (...args) { + this.sync = syncMock + this.hasChanges = false + instances.push(this) + }) + const savedTeams = Settings.PLUGINS.teams + Settings.PLUGINS.teams = TeamsCtor + + Settings.PLUGINS.repository = jest.fn().mockImplementation(() => ({ + sync: jest.fn().mockResolvedValue([]), renamed: false, created: false + })) + + try { + const settings = createSettings({ + additive_plugins: ['labels'], // teams is NOT listed + teams: [{ name: 'core', permission: 'push' }] + }) + settings.subOrgConfigs = {} + settings.repoConfigs = {} + // Clear subOrgConfigMap so the "suborg-change early return" does not fire. + settings.subOrgConfigMap = null + jest.spyOn(settings, 'childPluginsList').mockReturnValue([ + [TeamsCtor, [{ name: 'core', permission: 'push' }], 'teams'] + ]) + jest.spyOn(settings, 'maybeReevaluateSuborg').mockResolvedValue(undefined) + await settings.updateRepos({ owner: 'o', repo: 'r' }) + expect(instances.length).toBeGreaterThan(0) + instances.forEach(inst => expect(inst.additive).toBe(false)) + } finally { + Settings.PLUGINS.teams = savedTeams + } + }) + }) + + // ── Diffable.sync() additive behaviour ─────────────────────────────── + describe('Diffable.sync() additive behaviour', () => { + const Diffable = require('../../../lib/plugins/diffable') + + // Minimal concrete Diffable subclass for testing. + class TestDiffable extends Diffable { + constructor (nop, entries) { + super(nop, {}, { owner: 'o', repo: 'r' }, entries, { debug: jest.fn(), info: jest.fn(), error: jest.fn() }, []) + } + + find () { return Promise.resolve(this._existing || []) } + comparator (a, b) { return a.name === b.name } + changed (a, b) { return a.value !== b.value } + add (attrs) { return Promise.resolve([]) } + update (existing, attrs) { return Promise.resolve([]) } + remove (existing) { return Promise.resolve([]) } + } + + it('41. additive=false → remove() is called for unmatched existing entries', async () => { + const plugin = new TestDiffable(false, [{ name: 'keep', value: '1' }]) + plugin._existing = [ + { name: 'keep', value: '1' }, + { name: 'gone', value: '2' } // this one has no match in entries + ] + plugin.additive = false + const removeSpy = jest.spyOn(plugin, 'remove').mockResolvedValue([]) + await plugin.sync() + expect(removeSpy).toHaveBeenCalledTimes(1) + expect(removeSpy).toHaveBeenCalledWith(expect.objectContaining({ name: 'gone' })) + removeSpy.mockRestore() + }) + + it('42. additive=true → remove() is NOT called even when existing entries have no YAML match', async () => { + const plugin = new TestDiffable(false, [{ name: 'keep', value: '1' }]) + plugin._existing = [ + { name: 'keep', value: '1' }, + { name: 'gone', value: '2' } + ] + plugin.additive = true + const removeSpy = jest.spyOn(plugin, 'remove').mockResolvedValue([]) + await plugin.sync() + expect(removeSpy).not.toHaveBeenCalled() + removeSpy.mockRestore() + }) + + it('43. additive=true → add() is still called for new YAML entries', async () => { + const plugin = new TestDiffable(false, [ + { name: 'existing', value: '1' }, + { name: 'new-entry', value: '2' } + ]) + plugin._existing = [{ name: 'existing', value: '1' }] + plugin.additive = true + const addSpy = jest.spyOn(plugin, 'add').mockResolvedValue([]) + const removeSpy = jest.spyOn(plugin, 'remove').mockResolvedValue([]) + await plugin.sync() + expect(addSpy).toHaveBeenCalledWith(expect.objectContaining({ name: 'new-entry' })) + expect(removeSpy).not.toHaveBeenCalled() + addSpy.mockRestore() + removeSpy.mockRestore() + }) + + it('44. additive=true → update() is still called for changed entries', async () => { + const plugin = new TestDiffable(false, [{ name: 'item', value: 'new' }]) + plugin._existing = [{ name: 'item', value: 'old' }] + plugin.additive = true + const updateSpy = jest.spyOn(plugin, 'update').mockResolvedValue([]) + const removeSpy = jest.spyOn(plugin, 'remove').mockResolvedValue([]) + await plugin.sync() + expect(updateSpy).toHaveBeenCalledWith( + expect.objectContaining({ name: 'item', value: 'old' }), + expect.objectContaining({ name: 'item', value: 'new' }) + ) + expect(removeSpy).not.toHaveBeenCalled() + updateSpy.mockRestore() + removeSpy.mockRestore() + }) + + it('45. NOP mode + additive=true + deletions present → INFO NopCommand about suppressed deletions', async () => { + const plugin = new TestDiffable(true, [{ name: 'keep', value: '1' }]) + plugin._existing = [ + { name: 'keep', value: '1' }, + { name: 'gone', value: '2' } + ] + plugin.additive = true + const result = await plugin.sync() + const suppressed = result.flat().filter(cmd => + cmd && cmd.type === 'INFO' && /suppressed by additive_plugins/i.test(cmd.action.msg) + ) + expect(suppressed.length).toBeGreaterThan(0) + expect(suppressed[0].action.msg).toMatch(/1 deletion/) + }) + + it('46. NOP mode + additive=true + NO deletions → no suppressed message emitted', async () => { + const plugin = new TestDiffable(true, [{ name: 'item', value: '1' }]) + plugin._existing = [{ name: 'item', value: '1' }] // identical → no changes at all + plugin.additive = true + const result = await plugin.sync() + if (result) { + const suppressed = result.flat().filter(cmd => + cmd && cmd.action && /suppressed by additive_plugins/i.test(cmd.action.msg) + ) + expect(suppressed.length).toBe(0) + } + // result may be undefined (no changes) which is also correct + }) + }) + }) }) // Settings Tests From d03062a4238aea2144964cfaef4a582491b4e6d8 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Wed, 27 May 2026 10:37:22 -0400 Subject: [PATCH 15/17] fix: update .gitignore to ignore all .env files --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 9cd65700b..3eee961c2 100644 --- a/.gitignore +++ b/.gitignore @@ -128,7 +128,7 @@ npm-debug.log .DS_Store node_modules/ private-key.pem -.env +*.env *.pem .vscode yarn.lock From fa5020ef732b5c2d9872c7cb6946db66fc407b95 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Tue, 2 Jun 2026 14:45:07 -0500 Subject: [PATCH 16/17] fix: update app.yml to remove empty line and add organization custom roles permissions --- app.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app.yml b/app.yml index 04b1f7015..2f053bbf0 100644 --- a/app.yml +++ b/app.yml @@ -25,7 +25,6 @@ default_events: - repository_ruleset - team - # The set of permissions needed by the GitHub App. The format of the object uses # the permission name for the key (for example, issues) and the access type for # the value (for example, write). @@ -114,6 +113,14 @@ default_permissions: # https://developer.github.com/v3/apps/permissions/ organization_administration: write + # Manage custom organization roles. + # https://docs.github.com/en/enterprise-cloud@latest/rest/authentication/permissions-required-for-github-apps?apiVersion=2026-03-10#organization-permissions-for-custom-organization-roles + organization_custom_org_roles: write + + # Manage custom repository roles. + # https://docs.github.com/en/enterprise-cloud@latest/rest/authentication/permissions-required-for-github-apps?apiVersion=2026-03-10#organization-permissions-for-custom-repository-roles + organization_custom_roles: write + # Manage Actions variables. # https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28 actions_variables: write From d9be6056058515608d584b3af318138ca3782f10 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Thu, 4 Jun 2026 10:33:44 -0400 Subject: [PATCH 17/17] Refactor Variables Plugin: Simplify methods and add NopCommand support - Removed unnecessary comments and streamlined the constructor to enforce uppercase variable names. - Simplified the `find` method to directly return the required variable data. - Updated the `changed` method to directly compare values without additional sorting logic. - Refactored `update`, `add`, and `remove` methods to return NopCommand instances when `nop` is true, preventing actual API calls. - Enhanced unit tests to cover new NopCommand behavior and ensure proper functionality of the Variables plugin. - Introduced phase 13 in smoke tests to validate variable creation, updating, and removal in repository settings. - Added support for phase filtering in smoke tests to allow targeted execution of specific phases. --- README.md | 30 +- .../whitepaper-deploying-policies-at-scale.md | 936 ++++++++++++++++++ lib/plugins/variables.js | 203 +--- package.json | 3 +- smoke-test.js | 170 +++- test/unit/lib/plugins/variables.test.js | 208 +++- 6 files changed, 1339 insertions(+), 211 deletions(-) create mode 100644 docs/whitepaper-deploying-policies-at-scale.md diff --git a/README.md b/README.md index 6a6d52f8c..ef9b7fc55 100644 --- a/README.md +++ b/README.md @@ -750,14 +750,37 @@ Add the following to your `.env` file: ### Running ```bash +# Run all phases npm run smoke-test # or node smoke-test.js + +# Interactive mode — pause after each phase for manual validation +npm run smoke-test:interactive +# or +node smoke-test.js --interactive + +# Run a single phase (with setup + teardown) +npm run smoke-test:phase -- 3 +# or +node smoke-test.js --phase 3 + +# Run a range of phases +npm run smoke-test:phase -- 1-3 +node smoke-test.js --phase 1-3 + +# Run specific comma-separated phases +npm run smoke-test:phase -- 1,3,5 +node smoke-test.js --phase 1,3,5 + +# Mix range + interactive +npm run smoke-test:phase -- 1-3 interactive +node smoke-test.js --phase 1-3 --interactive ``` ### What it tests -The smoke test runs 9 phases: +The smoke test runs the following phases: | Phase | Description | |---|---| @@ -769,7 +792,12 @@ The smoke test runs 9 phases: | **Phase 5** | Creates a suborg config and verifies org-scoped rulesets are applied to matching repos | | **Phase 6** | Archives `demo-repo-service1` and verifies the repo is archived | | **Phase 7** | Creates `demo-repo-service2` and verifies suborg rulesets are inherited | +| **Phase 7b** | Tests external group team sync | | **Phase 8** | Creates org-level settings (custom repository roles + org rulesets) and verifies they are applied | +| **Phase 10** | Validates `disable_plugins` — ensures disabled plugins are skipped | +| **Phase 11** | Validates `additive_plugins` — verifies additive-mode plugin behaviour | +| **Phase 12** | Tests `custom_properties` plugin | +| **Phase 13** | Tests the `variables` plugin (create, update, remove variables) | | **Teardown** | Shuts down safe-settings, deletes test repos, teams, custom roles, and rulesets | ### Output diff --git a/docs/whitepaper-deploying-policies-at-scale.md b/docs/whitepaper-deploying-policies-at-scale.md new file mode 100644 index 000000000..406236b7a --- /dev/null +++ b/docs/whitepaper-deploying-policies-at-scale.md @@ -0,0 +1,936 @@ +# Deploying Policies at Scale Across Organizations Using GitHub Safe-Settings + +## A White Paper on Policy-as-Code for GitHub Enterprise Governance + +--- + +**Version:** 1.0 +**Date:** May 2026 +**Author:** GitHub Safe-Settings Team + +--- + +## Table of Contents + +1. [Executive Summary](#executive-summary) +2. [The Challenge: Governing Repositories at Scale](#the-challenge-governing-repositories-at-scale) +3. [Introducing Safe-Settings: Policy-as-Code for GitHub](#introducing-safe-settings-policy-as-code-for-github) +4. [Architecture Overview](#architecture-overview) +5. [The Configuration Hierarchy](#the-configuration-hierarchy) +6. [Designing Your Policy Framework](#designing-your-policy-framework) +7. [Deployment Models](#deployment-models) +8. [Scaling Strategies](#scaling-strategies) +9. [Governance Workflows](#governance-workflows) +10. [Advanced Policy Controls](#advanced-policy-controls) +11. [Drift Detection and Remediation](#drift-detection-and-remediation) +12. [Multi-Organization Deployments](#multi-organization-deployments) +13. [Security Considerations](#security-considerations) +14. [Case Study: Enterprise Rollout](#case-study-enterprise-rollout) +15. [Best Practices](#best-practices) +16. [Conclusion](#conclusion) + +--- + +## Executive Summary + +As organizations scale their software delivery practices on GitHub, managing repository configurations consistently across hundreds or thousands of repositories becomes a critical governance challenge. Manual configuration is error-prone, difficult to audit, and impossible to enforce at scale. + +**GitHub Safe-Settings** provides a policy-as-code solution that enables organizations to centrally define, enforce, and audit repository settings across an entire GitHub organization. By storing configuration as YAML in a centralized admin repository, Safe-Settings brings the principles of Infrastructure-as-Code to GitHub governance — enabling version control, peer review, automated validation, and continuous enforcement of organizational policies. + +This white paper provides a comprehensive guide to deploying Safe-Settings at enterprise scale, covering architecture decisions, policy design patterns, scaling strategies, and operational best practices. + +--- + +## The Challenge: Governing Repositories at Scale + +### The Problem + +Enterprise organizations on GitHub commonly face these governance challenges: + +- **Configuration sprawl**: Thousands of repositories with inconsistent settings — varying branch protections, team permissions, security configurations, and compliance controls. +- **Manual drift**: Repository administrators making ad-hoc changes that deviate from organizational standards, often without audit trails. +- **Onboarding delays**: New repositories require manual setup of branch protections, team access, labels, and compliance configurations. +- **Audit burden**: Demonstrating compliance with internal security policies or regulatory requirements (SOC 2, FedRAMP, HIPAA) demands evidence that every repository meets baseline standards. +- **Decentralized ownership**: Different teams need autonomy to manage their project-specific settings while still adhering to organization-wide baselines. + +### Why Existing Approaches Fall Short + +| Approach | Limitation | +|----------|-----------| +| **Manual configuration** | Does not scale; no audit trail; prone to drift | +| **GitHub repository templates** | Only applies at creation time; no ongoing enforcement | +| **Custom scripts/APIs** | High maintenance; fragile; no built-in review workflow | +| **Per-repo settings files** | Settings files live in individual repos, meaning any contributor can bypass policies | + +Safe-Settings addresses all of these limitations by centralizing policy definitions in a protected admin repository, enforcing them continuously, and providing a pull request-based review workflow for all changes. + +--- + +## Introducing Safe-Settings: Policy-as-Code for GitHub + +Safe-Settings is a GitHub App built on the [Probot](https://probot.github.io/) framework that implements policy-as-code for GitHub organizations. It operates on three foundational principles: + +### 1. Centralized Configuration + +All settings are stored in a single `admin` repository (configurable via the `ADMIN_REPO` environment variable). Unlike per-repo settings files, this prevents repository maintainers from overriding organizational policies. + +### 2. Hierarchical Policy Model + +Settings are defined at three levels with a clear precedence order: + +``` +Organization (baseline) → Sub-Organization (team/project overrides) → Repository (specific exceptions) +``` + +Higher-specificity levels override lower ones, enabling a flexible yet governed configuration model. + +### 3. Continuous Enforcement + +Safe-Settings responds to webhook events in real-time and can run on a configurable schedule (via cron) to detect and remediate configuration drift — ensuring that manual changes are automatically reverted to the declared policy state. + +### What Can Be Managed + +Safe-Settings supports a comprehensive set of GitHub configurations: + +| Category | Capabilities | +|----------|-------------| +| **Repository Settings** | Visibility, description, homepage, merge strategies, wiki, issues, projects, default branch, auto-init, security settings | +| **Branch Protections** | Required reviews, status checks, admin enforcement, push restrictions, dismiss stale reviews, code owner reviews | +| **Rulesets** | Organization and repository-level rulesets with branch/tag targeting, bypass actors, pattern rules, required workflows | +| **Teams & Collaborators** | Team permissions, collaborator access with include/exclude patterns | +| **Labels & Milestones** | Standardized issue labels and milestone definitions | +| **Custom Properties** | Organization-defined custom property values for repositories | +| **Environments** | Deployment environments with protection rules, wait timers, reviewers, and environment variables | +| **Autolinks** | External reference linking (e.g., Jira ticket prefixes) | +| **Repository Naming** | Regex-based validation of repository names | +| **Custom Repository Roles** | Organization-level custom roles | +| **Variables** | Repository and environment variables | + +--- + +## Architecture Overview + +### System Components + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ GitHub Platform │ +│ │ +│ ┌──────────┐ ┌──────────────┐ ┌──────────────────────┐ │ +│ │ Webhooks │ │ Admin Repo │ │ Target Repositories │ │ +│ │ (Events) │ │ (Policies) │ │ (1000s of repos) │ │ +│ └─────┬─────┘ └──────┬───────┘ └──────────┬───────────┘ │ +│ │ │ │ │ +└────────┼────────────────┼──────────────────────┼────────────────┘ + │ │ │ + ▼ ▼ ▼ +┌─────────────────────────────────────────────────────────────────┐ +│ Safe-Settings App │ +│ │ +│ ┌──────────────┐ ┌───────────────┐ ┌──────────────────┐ │ +│ │ Event Handler │ │ Config Merger │ │ Plugin Engine │ │ +│ │ (Webhooks) │ │ (Hierarchy) │ │ (API Calls) │ │ +│ └──────────────┘ └───────────────┘ └──────────────────┘ │ +│ │ +│ ┌──────────────┐ ┌───────────────┐ ┌──────────────────┐ │ +│ │ Drift Detect │ │ Validators │ │ Diff Engine │ │ +│ │ (Cron Sync) │ │ (Rules) │ │ (Smart Compare) │ │ +│ └──────────────┘ └───────────────┘ └──────────────────┘ │ +│ │ +└─────────────────────────────────────────────────────────────────┘ +``` + +### Event Processing Flow + +Safe-Settings listens to the following webhook events and responds accordingly: + +| Event | Action | +|-------|--------| +| `push` to admin repo (default branch) | Apply changed settings to affected repositories | +| `repository.created` | Apply full policy stack (org → suborg → repo) to the new repository | +| `repository.edited` | Sync settings to prevent unauthorized changes | +| `repository.renamed` | Optionally block human-initiated renames; sync config files for bot-initiated renames | +| `branch_protection_rule` | Revert unauthorized branch protection changes | +| `repository_ruleset` | Revert unauthorized ruleset modifications | +| `member` / `team` changes | Revert unauthorized collaborator or team permission changes | +| `custom_property_values` | Re-evaluate suborg membership and apply matching policies | +| `pull_request` (to admin repo) | Run in dry-run/NOP mode and report validation results as check runs | + +### Smart Diff Engine + +Safe-Settings does not blindly apply configuration on every event. It performs an intelligent comparison between the declared policy and the current GitHub state, generating a precise diff of `additions`, `modifications`, and `deletions`. API calls are made only when real changes exist, which is critical for performance at scale. + +--- + +## The Configuration Hierarchy + +### Directory Structure + +All policy files reside in the admin repository under the `.github` directory: + +``` +admin-repo/ +├── .github/ +│ ├── settings.yml # Organization-wide baseline policies +│ ├── suborgs/ # Sub-organization policies +│ │ ├── platform-team.yml # Policies for platform team repos +│ │ ├── frontend-team.yml # Policies for frontend team repos +│ │ ├── compliance-critical.yml # Policies for compliance-critical repos +│ │ └── open-source.yml # Policies for open-source repos +│ └── repos/ # Repository-specific overrides +│ ├── api-gateway.yml # Specific settings for api-gateway +│ ├── auth-service.yml # Specific settings for auth-service +│ └── docs-site.yml # Specific settings for docs-site +├── CODEOWNERS # Governs who can approve policy changes +└── deployment-settings.yml # Runtime configuration for the app +``` + +### Precedence Order + +``` +Repository-specific > Sub-Organization > Organization +``` + +When Safe-Settings computes the effective configuration for a given repository, it: + +1. Starts with the **organization-level** settings from `settings.yml` +2. Overlays any matching **sub-organization** settings +3. Overlays any **repository-specific** settings + +This layered approach means that organization-wide baselines are always applied, but teams can customize settings within the bounds defined by validators. + +### Sub-Organization Membership + +Sub-organizations ("suborgs") are a powerful abstraction for grouping repositories. A repository can belong to a suborg based on three criteria: + +| Criterion | Configuration Key | Example | +|-----------|------------------|---------| +| **Repository name pattern** | `suborgrepos` | `suborgrepos: ["api-*", "service-*"]` | +| **Team membership** | `suborgteams` | `suborgteams: ["platform-core"]` | +| **Custom property values** | `suborgproperties` | `suborgproperties: [{"compliance": "sox"}]` | + +This flexibility enables policies to be applied based on organizational structure, project taxonomy, or compliance classification — all without hard-coding repository lists. + +--- + +## Designing Your Policy Framework + +### Step 1: Define Your Organization Baseline + +> **⚠️ Scaling Best Practice: Keep `settings.yml` Minimal** +> +> Any change to the org-level `settings.yml` triggers Safe-Settings to process **every managed repository** in the organization. For orgs with thousands of repos, this can cascade into thousands of API calls and risk breaching GitHub's API rate limits within the 1-hour token lifetime. +> +> **Recommended approach:** Limit `settings.yml` to resources that are applied at the **org level** — specifically **org-level rulesets** and **custom repository roles**. These are managed via org-scoped API endpoints and do **not** require per-repo API calls. +> +> Move repo-scoped settings (repository configuration, teams, collaborators, labels, branch protections, etc.) to **suborg-level** files. This way, changes only affect the subset of repos matched by each suborg, keeping API call volume manageable and predictable. + +Define your organization-wide baseline using org-level rulesets and custom roles: + +```yaml +# .github/settings.yml — Organization baseline +# Keep this file minimal: only org-level rulesets and custom roles. +# Repo-scoped settings (teams, labels, repository config) belong in suborgs. + +rulesets: + - name: Branch Protection + target: branch + enforcement: active + bypass_actors: + - actor_id: 1 + actor_type: OrganizationAdmin + bypass_mode: always + conditions: + ref_name: + include: ["~DEFAULT_BRANCH"] + exclude: [] + repository_name: + include: ["~ALL"] + exclude: [] + rules: + - type: pull_request + parameters: + dismiss_stale_reviews_on_push: true + require_code_owner_review: true + require_last_push_approval: true + required_approving_review_count: 1 + required_review_thread_resolution: true + - type: required_status_checks + parameters: + strict_required_status_checks_policy: true + required_status_checks: [] + + - name: Branch Integrity + target: branch + enforcement: active + bypass_actors: + - actor_id: 1 + actor_type: OrganizationAdmin + bypass_mode: always + conditions: + ref_name: + include: ["~DEFAULT_BRANCH"] + exclude: [] + repository_name: + include: ["~ALL"] + exclude: [] + rules: + - type: deletion + - type: non_fast_forward + - type: required_linear_history + - type: required_signatures +``` + +Then define repo-scoped baseline settings at the **suborg level** to avoid cascading org-wide API calls. Use a broad suborg definition (e.g., `~ALL` repos or a wildcard pattern) to achieve org-wide coverage without the scaling risks: + +```yaml +# .github/suborgs/baseline.yml — Default repo-scoped settings for all repos +# Changes here only trigger API calls for matched repos, not the entire org. + +suborgrepos: + - "*" + +repository: + private: true + allow_auto_merge: false + delete_branch_on_merge: true + allow_update_branch: true + security: + enableVulnerabilityAlerts: true + enableAutomatedSecurityFixes: true + +teams: + - name: security-team + permission: admin + - name: all-engineers + permission: push + +labels: + - name: bug + color: "d73a4a" + description: "Something isn't working" + - name: security + color: "e11d48" + description: "Security-related issue" + - name: compliance + color: "7c3aed" + description: "Compliance-related" + +validator: + pattern: "[a-z0-9]+(-[a-z0-9]+)*" +``` + +### Step 2: Define Sub-Organization Policies + +Create suborg files for teams or projects that need additional or different policies: + +```yaml +# .github/suborgs/compliance-critical.yml + +# Repos with the "compliance" custom property set to "sox" +suborgproperties: + - compliance: sox + +# Stricter branch protections for SOX-compliant repositories +branches: + - name: default + protection: + required_pull_request_reviews: + required_approving_review_count: 2 + dismiss_stale_reviews: true + require_code_owner_reviews: true + require_last_push_approval: true + enforce_admins: true + +# Additional team access for compliance repos +teams: + - name: compliance-auditors + permission: pull +``` + +```yaml +# .github/suborgs/open-source.yml + +suborgrepos: + - "oss-*" + +repository: + private: false + visibility: public + has_wiki: true + +# Public repos need different branch protections +branches: + - name: default + protection: + required_pull_request_reviews: + required_approving_review_count: 2 + require_code_owner_reviews: true +``` + +### Step 3: Define Repository-Specific Overrides + +For repositories that need unique configurations: + +```yaml +# .github/repos/api-gateway.yml + +repository: + description: "Central API gateway for all microservices" + homepage: "https://api-docs.example.com" + topics: + - api + - gateway + - critical-infrastructure + +branches: + - name: default + protection: + required_status_checks: + strict: true + contexts: + - "ci/build" + - "ci/integration-tests" + - "security/codeql" + +environments: + - name: production + wait_timer: 30 + prevent_self_review: true + reviewers: + - type: Team + id: 12345 # platform-leads team + deployment_branch_policy: + protected_branches: true + custom_branch_policies: false +``` + +--- + +## Deployment Models + +Safe-Settings supports multiple deployment architectures to fit your infrastructure requirements. + +### Docker (Recommended for Most Organizations) + +Best for organizations with existing container infrastructure. + +```bash +# Build and run +docker build -t safe-settings . +docker run -d -p 3000:3000 --env-file .env safe-settings + +# Or with docker-compose +docker-compose --env-file .env up -d +``` + +**Advantages:** Simple, portable, works with any container orchestration platform. + +### Kubernetes with Helm + +Best for organizations running Kubernetes clusters. + +```bash +# Install using the official Helm chart +helm install safe-settings \ + oci://ghcr.io/github/helm-charts/safe-settings \ + --values myvalues.yaml +``` + +**Advantages:** Native Kubernetes integration, auto-scaling, rolling updates, health checks, secrets management via Kubernetes Secrets or external secret stores. + +### AWS Lambda (Serverless) + +Best for organizations wanting minimal infrastructure overhead. + +Use the [SafeSettings-Template](https://github.com/bheemreddy181/SafeSettings-Template) for a production-ready deployment featuring: + +- Docker-based Lambda functions +- Dual Lambda architecture (webhook handler + scheduled sync) +- GitHub Actions CI/CD pipeline +- Auto-scaling with pay-per-execution pricing + +### GitHub Actions + +Best for organizations that want to avoid deploying infrastructure entirely. + +Safe-Settings can be run as a GitHub Action, triggered by workflow dispatch or on a schedule. See the [GitHub Actions Guide](github-action.md) for configuration details. + +### Deployment Comparison + +| Model | Scalability | Operational Overhead | Real-Time Events | Scheduled Sync | Cost Model | +|-------|-------------|---------------------|-------------------|----------------|------------| +| **Docker** | Medium | Medium | ✅ Webhooks | ✅ CRON | Fixed | +| **Kubernetes** | High | Medium-High | ✅ Webhooks | ✅ CRON | Fixed | +| **AWS Lambda** | Very High | Low | ✅ Webhooks | ✅ EventBridge | Pay-per-use | +| **GitHub Actions** | Medium | Very Low | ❌ Polling only | ✅ Cron triggers | Actions minutes | + +--- + +## Scaling Strategies + +### Performance Considerations + +When managing thousands of repositories, Safe-Settings employs several strategies to operate within constraints: + +1. **Org-level settings are org-scoped**: Rulesets and custom repository roles defined in `settings.yml` are applied via org-level API endpoints — they do **not** generate per-repo API calls. This is why `settings.yml` should be reserved for these resources only. + +2. **Selective configuration loading**: Only repo-specific YAML files relevant to the changed settings are loaded — not the entire `.github/repos/` directory. Full loading occurs only for global settings changes. + +3. **Smart diff comparisons**: Before making any API call, Safe-Settings compares the desired state with the current GitHub state. API calls are only made when real changes are detected. + +4. **Rate limit handling**: Built on Probot, the app automatically handles GitHub API rate limits and abuse limits with exponential backoff. + +5. **Token lifetime awareness**: GitHub App installation tokens expire after 1 hour. Safe-Settings is designed to complete all work within this window. + +### Configuration for Large Organizations + +For organizations with 1,000+ repositories, consider these configurations: + +```env +# Run scheduled sync during off-peak hours +CRON=0 2 * * * + +# Set appropriate log level for production +LOG_LEVEL=info + +# Enable PR comments for audit trail +ENABLE_PR_COMMENT=true + +# Block manual repo renames to maintain config consistency +BLOCK_REPO_RENAME_BY_HUMAN=true +``` + +### Restricting Scope + +Use `deployment-settings.yml` to control which repositories Safe-Settings manages: + +```yaml +# deployment-settings.yml + +restrictedRepos: + include: + - "service-*" + - "lib-*" + - "infra-*" + exclude: + - admin + - .github + - safe-settings + - "test-*" + - "sandbox-*" +``` + +This is particularly useful during phased rollouts — start with a subset of repositories and expand as confidence grows. + +--- + +## Governance Workflows + +### Pull Request-Based Policy Changes + +All policy changes follow a pull request workflow, providing: + +1. **Version control**: Every change to organizational policies is tracked in Git history. +2. **Peer review**: Changes must be approved before taking effect. +3. **Dry-run validation**: Safe-Settings runs in NOP (no-operation) mode on PRs, producing a detailed report of what would change across all affected repositories. +4. **Check runs**: PR checks pass or fail based on dry-run results, including custom validator outcomes. + +### CODEOWNERS for Policy Governance + +Use GitHub's CODEOWNERS file in the admin repo to establish approval requirements: + +``` +# CODEOWNERS in admin repo + +# Security team must approve all policy changes +.github/settings.yml @org/security-team @org/platform-leads + +# Team leads approve their suborg policies +.github/suborgs/platform-team.yml @org/platform-leads +.github/suborgs/frontend-team.yml @org/frontend-leads +.github/suborgs/compliance-critical.yml @org/compliance-team @org/security-team + +# Repo owners can manage their repo-specific settings +.github/repos/api-gateway.yml @org/api-team +.github/repos/auth-service.yml @org/identity-team + +# Deployment settings require platform admin approval +deployment-settings.yml @org/platform-admins +``` + +This enables **delegated governance**: teams can manage their own settings within the guardrails established by the organization baseline and custom validators. + +### Change Review Workflow + +``` +Developer Admin Repo Safe-Settings GitHub + │ │ │ │ + ├─ Create branch ──►│ │ │ + ├─ Modify YAML ────►│ │ │ + ├─ Open PR ────────►│ │ │ + │ ├─ Webhook ───────────►│ │ + │ │ ├─ Dry-run ──────────┤ + │ │ ├─ Validate rules ───┤ + │ │ ├─ Report results ──►│ + │ │ │ │ + │◄──── Review PR with check results ──────────────────────────-│ + │ │ │ │ + ├─ Merge PR ───────►│ │ │ + │ ├─ Push webhook ──────►│ │ + │ │ ├─ Apply settings ──►│ + │ │ ├─ Create check ────►│ + │ │ │ │ +``` + +--- + +## Advanced Policy Controls + +### Custom Configuration Validators + +Validators allow you to define rules that settings must satisfy before they can be applied. They are defined in `deployment-settings.yml`. + +#### Config Validators + +Validate a setting in isolation: + +```yaml +configvalidators: + # Prevent granting admin access to collaborators + - plugin: collaborators + error: "Admin role cannot be assigned to individual collaborators" + script: | + return baseconfig.permission !== 'admin' + + # Ensure all repos have a description + - plugin: repository + error: "Repository must have a description" + script: | + return baseconfig.description && baseconfig.description.length > 10 + + # Validate repository naming conventions + - plugin: repository + error: "Repository names must follow the pattern: team-project-component" + script: | + const pattern = /^[a-z]+-[a-z]+-[a-z0-9-]+$/ + return pattern.test(baseconfig.name) +``` + +#### Override Validators + +Enforce constraints when lower-level settings override higher-level ones: + +```yaml +overridevalidators: + # Prevent reducing required approvers below the org baseline + - plugin: branches + error: "Cannot reduce required approving review count below organization minimum" + script: | + if (baseconfig.protection.required_pull_request_reviews.required_approving_review_count && + overrideconfig.protection.required_pull_request_reviews.required_approving_review_count) { + return overrideconfig.protection.required_pull_request_reviews.required_approving_review_count >= + baseconfig.protection.required_pull_request_reviews.required_approving_review_count + } + return true + + # Prevent disabling admin enforcement + - plugin: branches + error: "Cannot disable admin enforcement for branch protections" + script: | + if (baseconfig.protection.enforce_admins === true) { + return overrideconfig.protection.enforce_admins !== false + } + return true +``` + +### Disabling Plugins + +For scenarios where certain settings should not be managed by Safe-Settings at specific scopes: + +```yaml +# At the org level — disable milestones management entirely +disable_plugins: + - milestones + +# At the suborg level — disable labels for matched repos only +disable_plugins: + - plugin: labels + target: all +``` + +**Target options:** + +| Target | Effect | +|--------|--------| +| `self` | Strips the plugin from the declaring layer only | +| `children` | Strips from all layers below | +| `all` | Strips from the declaring layer and all layers below | + +**Important:** Strips are **union-only** — a lower-level config can add more strips but can never re-enable a plugin disabled at a higher level. + +### Additive Plugins + +For plugins where you want Safe-Settings to enforce a baseline but allow teams to add their own items without those additions being removed: + +```yaml +# In settings.yml — never remove labels or teams added outside safe-settings +additive_plugins: + - labels + - teams + - collaborators +``` + +In additive mode, Safe-Settings will **add** and **update** entries defined in YAML but will **never delete** items that exist on GitHub but are absent from the configuration. This is ideal for labels, teams, and collaborators where teams may need to add project-specific items. + +### Externally Defined Status Checks + +For status checks that are managed by CI/CD pipelines rather than Safe-Settings: + +```yaml +branches: + - name: default + protection: + required_status_checks: + contexts: + - "ci/build" # Managed by safe-settings + - "{{EXTERNALLY_DEFINED}}" # Preserve any additional checks set via UI +``` + +This allows Safe-Settings to enforce a minimum set of required status checks while preserving any additional checks configured by teams through the GitHub UI. + +--- + +## Drift Detection and Remediation + +### How Drift Is Detected + +Drift occurs when repository settings are changed outside of Safe-Settings — for example, a repository administrator modifying branch protections through the GitHub UI. + +Safe-Settings detects drift through two mechanisms: + +1. **Real-time webhook events**: When certain settings are changed (branch protections, rulesets, team memberships, collaborator changes), GitHub sends webhook events that trigger Safe-Settings to re-sync the affected repository. + +2. **Scheduled sync (CRON)**: A configurable cron job that periodically compares all managed repositories against the declared policy and remediates any drift. + +### Webhook-Based Remediation + +The following events trigger automatic remediation: + +- `branch_protection_rule` — Modified or deleted branch protections are restored +- `repository_ruleset` — Unauthorized ruleset changes are reverted +- `member` / `team` changes — Unauthorized permission changes are corrected +- `repository.edited` — Settings like default branch or topics are restored + +### Scheduled Sync Configuration + +```env +# Run drift detection every hour +CRON=0 * * * * + +# Or run at 2 AM daily for lower-priority environments +CRON=0 2 * * * +``` + +### Drift Remediation Strategy + +| Priority | Strategy | Use Case | +|----------|----------|----------| +| **Critical** | Real-time webhook + hourly CRON | Production security policies, branch protections | +| **Standard** | Real-time webhook + daily CRON | Team permissions, labels, general settings | +| **Advisory** | Daily CRON only | Low-risk settings where immediate enforcement isn't required | + +--- + +## Multi-Organization Deployments + +For enterprises with multiple GitHub organizations, Safe-Settings can be deployed in several patterns: + +### Pattern 1: One App per Organization + +Deploy a separate Safe-Settings instance for each organization. Each instance has its own admin repo and configuration. + +``` +┌──────────────────┐ ┌──────────────────┐ ┌──────────────────┐ +│ Org: prod-eng │ │ Org: platform │ │ Org: open-src │ +│ │ │ │ │ │ +│ safe-settings │ │ safe-settings │ │ safe-settings │ +│ admin repo │ │ admin repo │ │ admin repo │ +│ policies A │ │ policies B │ │ policies C │ +└──────────────────┘ └──────────────────┘ └──────────────────┘ +``` + +**Advantages:** Complete isolation; different policies per org. +**Challenges:** Multiple deployments to manage; policy consistency must be maintained manually. + +### Pattern 2: Shared Policy Templates + +Maintain a shared repository of policy templates and use them as a source for each organization's admin repo: + +``` +┌─────────────────────────────────────────┐ +│ Policy Template Repo │ +│ (Gold-standard YAML templates) │ +└────────┬──────────┬──────────┬──────────┘ + │ │ │ + ┌────▼───┐ ┌────▼───┐ ┌───▼────┐ + │ Org A │ │ Org B │ │ Org C │ + │ admin │ │ admin │ │ admin │ + └────────┘ └────────┘ └────────┘ +``` + +Use CI/CD pipelines (e.g., GitHub Actions) to sync templates to each organization's admin repo, allowing per-org customization while maintaining a consistent baseline. + +### Pattern 3: GitHub Enterprise Server + Cloud + +For organizations using both GitHub Enterprise Server and GitHub.com: + +- Deploy Safe-Settings on-premises for GHES (set `GHE_HOST` environment variable) +- Deploy Safe-Settings in the cloud for GitHub.com organizations +- Use a shared policy template repo to maintain consistency + +--- + +## Security Considerations + +### Protecting the Admin Repository + +The admin repository is the source of truth for all organizational policies. Protect it with: + +1. **Branch protections on the default branch**: Require PR reviews, status checks, and code owner approval. +2. **CODEOWNERS**: Define who can approve changes to different policy files. +3. **Repository visibility**: Keep the admin repo private. +4. **Limited write access**: Only grant write access to authorized policy administrators. +5. **Audit logging**: GitHub's audit log captures all changes to the admin repo. + +### GitHub App Permissions + +Safe-Settings requires specific permissions to function. Follow the principle of least privilege: + +| Permission | Level | Purpose | +|-----------|-------|---------| +| Administration | Read & Write | Manage repository settings | +| Contents | Read & Write | Read config files from admin repo | +| Checks | Read & Write | Report validation results | +| Pull requests | Read & Write | Comment on policy change PRs | +| Custom properties | Read & Write | Manage custom property values | +| Members (org) | Read & Write | Manage team permissions | + +### Secrets Management + +- **Never** commit the GitHub App private key to the admin repo +- Use environment variables, Kubernetes Secrets, or cloud secret managers (AWS Secrets Manager, Azure Key Vault, HashiCorp Vault) +- Rotate the webhook secret periodically + +### Blocking Manual Overrides + +Enable `BLOCK_REPO_RENAME_BY_HUMAN=true` to prevent repository renames outside of Safe-Settings, maintaining configuration file consistency. + +--- + +## Case Study: Enterprise Rollout + +### Scenario + +A financial services company with 3,000+ repositories across 50 development teams needs to enforce SOX compliance controls, standardize branch protections, and reduce the time to provision new repositories from days to minutes. + +### Phase 1: Discovery and Planning (Week 1–2) + +1. **Audit current state**: Document existing repository configurations across the organization. +2. **Define policy tiers**: Establish three compliance tiers — Standard, Regulated, and Critical. +3. **Map teams to suborgs**: Define suborg membership using custom properties (`compliance_tier: standard|regulated|critical`). +4. **Design CODEOWNERS**: Map approval authority for each policy tier. + +### Phase 2: Pilot Deployment (Week 3–4) + +1. **Deploy Safe-Settings** to a Kubernetes cluster with the Helm chart. +2. **Restrict scope** to 10 pilot repositories using `deployment-settings.yml`. +3. **Define baseline policies** for organization-wide settings. +4. **Test dry-run mode** by creating PRs and validating check results. +5. **Validate drift remediation** by manually changing settings and confirming automatic reversion. + +### Phase 3: Gradual Rollout (Week 5–8) + +1. **Expand scope** to one team (50 repositories) per week. +2. **Create suborg policies** for each compliance tier. +3. **Enable override validators** to prevent weakening of security controls. +4. **Train team leads** on creating repo-specific overrides via PR workflow. + +### Phase 4: Full Deployment (Week 9–10) + +1. **Remove scope restrictions** — Safe-Settings manages all repositories. +2. **Enable scheduled sync** with `CRON=0 * * * *` for hourly drift checks. +3. **Enable `BLOCK_REPO_RENAME_BY_HUMAN`** for configuration consistency. +4. **Document runbooks** for common policy change scenarios. + +### Results + +| Metric | Before | After | +|--------|--------|-------| +| Time to provision new repo | 2–3 days | < 5 minutes | +| Repos with compliant branch protections | 62% | 100% | +| Manual drift incidents per month | 40+ | 0 (auto-remediated) | +| Policy change audit trail | Partial | Complete (Git history) | +| Time to demonstrate compliance | Days | Minutes (YAML-as-evidence) | + +--- + +## Best Practices + +### Policy Design + +1. **Keep `settings.yml` minimal — rulesets and custom roles only**: Any change to `settings.yml` triggers processing for every managed repository. To avoid cascading API calls across thousands of repos, limit org-level settings to resources that use org-scoped API endpoints (rulesets and custom repository roles). Move all repo-scoped settings (teams, labels, repository config, collaborators) to suborg files. + +2. **Use a broad suborg for repo-scoped baselines**: Create a `baseline.yml` suborg with `suborgrepos: ["*"]` to apply default repo-scoped settings (teams, labels, security config) to all repos. This achieves the same coverage as org-level settings but limits the blast radius of changes to the matched subset. + +3. **Use suborgs for team autonomy**: Rather than creating repo-level overrides for every repository, group repos into suborgs by team, project, or compliance tier. + +4. **Prefer custom properties for suborg membership**: Custom properties provide the most flexible and maintainable way to group repositories, as they can be updated without modifying the admin repo. + +5. **Use additive mode for shared resources**: For labels, teams, and collaborators, consider using `additive_plugins` to allow teams to add project-specific items without those additions being removed on the next sync. + +6. **Define validators early**: Establish override validators before teams start creating overrides. This prevents policy weakening from the start. + +### Operational Excellence + +6. **Protect the admin repo**: Apply the same (or stricter) branch protections to the admin repo as you require for production code. + +7. **Use CODEOWNERS strategically**: Grant approval authority at the appropriate level — security team for org settings, team leads for suborg settings, repo owners for repo-specific overrides. + +8. **Monitor check runs**: Set up notifications for failed Safe-Settings check runs to catch configuration issues early. + +9. **Schedule regular sync**: Even with webhook-based enforcement, configure a CRON schedule as a safety net for missed webhooks. + +10. **Version pin your deployment**: Use specific image tags (e.g., `ghcr.io/github/safe-settings:2.1.13`) rather than floating tags to ensure reproducible deployments. + +### Scaling + +11. **Phase your rollout**: Use `deployment-settings.yml` to gradually expand scope. Start with 10 repos, then 100, then 1,000. + +12. **Avoid repo-scoped settings in `settings.yml`**: Changes to `settings.yml` trigger processing for all managed repositories. Keep it to org-level rulesets and custom roles only. Use suborg files for repo-scoped settings to limit the blast radius of any single change. + +13. **Use include/exclude patterns**: For teams and collaborators, use `include` and `exclude` patterns rather than defining settings for every repository individually. + +14. **Monitor API rate limits**: At scale, watch for rate limit consumption. Probot handles this automatically, but awareness helps with capacity planning. + +--- + +## Conclusion + +GitHub Safe-Settings transforms repository governance from a manual, error-prone process into an automated, auditable, and scalable policy-as-code practice. By centralizing configuration in a protected admin repository, enforcing changes through pull request workflows, and continuously remediating drift, organizations can achieve consistent security baselines, streamlined compliance, and empowered development teams. + +Whether managing 50 or 5,000 repositories, Safe-Settings provides the flexibility to balance centralized governance with team autonomy — ensuring that every repository in your organization meets your standards, every time. + +--- + +## Additional Resources + +- **Repository**: [github/safe-settings](https://github.com/github/safe-settings) +- **Deployment Guide**: [docs/deploy.md](deploy.md) +- **GitHub Actions Guide**: [docs/github-action.md](github-action.md) +- **Sample Settings**: [docs/sample-settings/](sample-settings/) +- **AWS Lambda Template**: [SafeSettings-Template](https://github.com/bheemreddy181/SafeSettings-Template) + +--- + +*© 2026 GitHub, Inc. Safe-Settings is licensed under the [ISC License](https://opensource.org/licenses/ISC).* diff --git a/lib/plugins/variables.js b/lib/plugins/variables.js index 25795c408..a8e65b91e 100644 --- a/lib/plugins/variables.js +++ b/lib/plugins/variables.js @@ -1,196 +1,71 @@ -const _ = require('lodash') const Diffable = require('./diffable') +const NopCommand = require('../nopcommand') module.exports = class Variables extends Diffable { constructor (...args) { super(...args) if (this.entries) { - // Force all names to uppercase to avoid comparison issues. this.entries.forEach((variable) => { variable.name = variable.name.toUpperCase() }) } } - /** - * Look-up existing variables for a given repository - * - * @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#list-repository-variables} list repository variables - * @returns {Array.} Returns a list of variables that exist in a repository - */ - async find () { + find () { this.log.debug(`Finding repo vars for ${this.repo.owner}/${this.repo.repo}`) - const { data: { variables } } = await this.github.request('GET /repos/:org/:repo/actions/variables', { + return this.github.request('GET /repos/:org/:repo/actions/variables', { org: this.repo.owner, repo: this.repo.repo - }) - return variables - } - - /** - * Compare the existing variables with what we've defined as code - * - * @param {Array.} existing Existing variables defined in the repository - * @param {Array.} variables Variables that we have defined as code - * - * @returns {object} The results of a list comparison - */ - getChanged (existing, variables = []) { - const result = - JSON.stringify( - existing.sort((x1, x2) => { - return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase()) - }) - ) !== - JSON.stringify( - variables.sort((x1, x2) => { - return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase()) - }) - ) - return result + }).then(({ data: { variables } }) => variables.map(({ name, value }) => ({ name, value }))) } - /** - * Compare existing variables with what's defined - * - * @param {Object} existing The existing entries in GitHub - * @param {Object} attrs The entries defined as code - * - * @returns - */ comparator (existing, attrs) { return existing.name === attrs.name } - /** - * Return a list of changed entries - * - * @param {Object} existing The existing entries in GitHub - * @param {Object} attrs The entries defined as code - * - * @returns - */ changed (existing, attrs) { - return this.getChanged(_.castArray(existing), _.castArray(attrs)) + return existing.value !== attrs.value } - /** - * Update an existing variable if the value has changed - * - * @param {Array.} existing Existing variables defined in the repository - * @param {Array.} variables Variables that we have defined as code - * - * @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#update-a-repository-variable} update a repository variable - * @returns - */ - async update (existing, variables = []) { - this.log.debug(`Updating a repo var existing params ${JSON.stringify(existing)} and new ${JSON.stringify(variables)}`) - existing = _.castArray(existing) - variables = _.castArray(variables) - const changed = this.getChanged(existing, variables) - - if (changed) { - let existingVariables = [...existing] - for (const variable of variables) { - const existingVariable = existingVariables.find((_var) => _var.name === variable.name) - if (existingVariable) { - existingVariables = existingVariables.filter((_var) => _var.name !== variable.name) - if (existingVariable.value !== variable.value) { - await this.github - .request('PATCH /repos/:org/:repo/actions/variables/:variable_name', { - org: this.repo.owner, - repo: this.repo.repo, - variable_name: variable.name.toUpperCase(), - value: variable.value.toString() - }) - .then((res) => { - return res - }) - .catch((e) => { - this.logError(e) - }) - } - } else { - await this.github - .request('POST /repos/:org/:repo/actions/variables', { - org: this.repo.owner, - repo: this.repo.repo, - name: variable.name.toUpperCase(), - value: variable.value.toString() - }) - .then((res) => { - return res - }) - .catch((e) => { - this.logError(e) - }) - } - } - - for (const variable of existingVariables) { - await this.github - .request('DELETE /repos/:org/:repo/actions/variables/:variable_name', { - org: this.repo.owner, - repo: this.repo.repo, - variable_name: variable.name.toUpperCase() - }) - .then((res) => { - return res - }) - .catch((e) => { - this.logError(e) - }) - } + update (existing, attrs) { + if (this.nop) { + return Promise.resolve([ + new NopCommand(this.constructor.name, this.repo, null, `Update variable ${attrs.name}`) + ]) } + return this.github.request('PATCH /repos/:org/:repo/actions/variables/:variable_name', { + org: this.repo.owner, + repo: this.repo.repo, + variable_name: attrs.name.toUpperCase(), + value: attrs.value.toString() + }) } - /** - * Add a new variable to a given repository - * - * @param {object} variable The variable to add, with name and value - * - * @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#create-a-repository-variable} create a repository variable - * @returns - */ - async add (variable) { - this.log.debug(`Adding a repo var with the params ${JSON.stringify(variable)}`) - await this.github - .request('POST /repos/:org/:repo/actions/variables', { - org: this.repo.owner, - repo: this.repo.repo, - name: variable.name, - value: variable.value.toString() - }) - .then((res) => { - return res - }) - .catch((e) => { - this.logError(e) - }) + add (attrs) { + if (this.nop) { + return Promise.resolve([ + new NopCommand(this.constructor.name, this.repo, null, `Add variable ${attrs.name}`) + ]) + } + return this.github.request('POST /repos/:org/:repo/actions/variables', { + org: this.repo.owner, + repo: this.repo.repo, + name: attrs.name.toUpperCase(), + value: attrs.value.toString() + }) } - /** - * Remove variables that aren't defined as code - * - * @param {String} existing Name of the existing variable to remove - * - * @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#delete-a-repository-variable} delete a repository variable - * @returns - */ - async remove (existing) { - this.log.debug(`Removing a repo var with the params ${JSON.stringify(existing)}`) - await this.github - .request('DELETE /repos/:org/:repo/actions/variables/:variable_name', { - org: this.repo.owner, - repo: this.repo.repo, - variable_name: existing.name - }) - .then((res) => { - return res - }) - .catch((e) => { - this.logError(e) - }) + remove (existing) { + if (this.nop) { + return Promise.resolve([ + new NopCommand(this.constructor.name, this.repo, null, `Remove variable ${existing.name}`) + ]) + } + return this.github.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', { + org: this.repo.owner, + repo: this.repo.repo, + variable_name: existing.name.toUpperCase() + }) } } diff --git a/package.json b/package.json index 974308487..2c217d6d2 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,8 @@ "test:integration": "jest --roots=lib --roots=test/integration", "test:integration:debug": "LOG_LEVEL=debug DEBUG=nock run-s test:integration", "smoke-test": "node smoke-test.js", - "smoke-test:interactive": "node smoke-test.js --interactive" + "smoke-test:interactive": "node smoke-test.js --interactive", + "smoke-test:phase": "node smoke-test.js --phase" }, "author": "Yadhav Jayaraman", "license": "ISC", diff --git a/smoke-test.js b/smoke-test.js index 85ce4f1b3..d8e79323d 100644 --- a/smoke-test.js +++ b/smoke-test.js @@ -78,7 +78,37 @@ const WEBHOOK_SETTLE_MS = 15000 const GH_TOKEN = process.env.GH_TOKEN || '' // Interactive mode: pause after each phase for manual validation -const INTERACTIVE = process.argv.includes('--interactive') +// Accepts --interactive flag or bare positional "interactive" word. +const INTERACTIVE = process.argv.includes('--interactive') || process.argv.slice(2).includes('interactive') + +// Phase filter: supports single, comma-separated, or range values. +// --phase 3 → only phase 3 +// --phase 1,2,3 → phases 1, 2, and 3 +// --phase 1-3 → phases 1 through 3 +// npm run smoke-test:phase -- 1-3 interactive +const PHASE_ARG_IDX = process.argv.indexOf('--phase') +const _parsePhaseSet = (raw) => { + if (!raw) return null + const nums = new Set() + for (const part of raw.split(',')) { + const range = part.match(/^(\d+)-(\d+)$/) + if (range) { + const lo = parseInt(range[1], 10) + const hi = parseInt(range[2], 10) + for (let i = lo; i <= hi; i++) nums.add(i) + } else if (/^\d+$/.test(part.trim())) { + nums.add(parseInt(part.trim(), 10)) + } + } + return nums.size > 0 ? nums : null +} +const ONLY_PHASES = PHASE_ARG_IDX !== -1 + ? _parsePhaseSet(process.argv[PHASE_ARG_IDX + 1]) + : (() => { + // Accept bare positional phase spec (e.g. "3" or "1-3" or "1,2,3") + const positional = process.argv.slice(2).find(a => !a.startsWith('--') && /^[\d,\-]+$/.test(a) && !/^-\d/.test(a)) + return positional !== undefined ? _parsePhaseSet(positional) : null + })() class InteractiveExit extends Error { constructor (action) { @@ -815,6 +845,30 @@ disable_plugins: target: self ` +// Phase 13: Variables plugin +const REPO_YML_VARIABLES = `repository: + name: test + auto_init: true + force_create: true + private: true + +variables: + - name: SMOKE_VAR_ONE + value: hello + - name: SMOKE_VAR_TWO + value: "42" +` + +const REPO_YML_VARIABLES_UPDATED = `repository: + name: test + +variables: + - name: SMOKE_VAR_ONE + value: hello-updated + - name: SMOKE_VAR_TWO + value: "42" +` + // ─── Test Phases ───────────────────────────────────────────────────────────── async function setup () { @@ -1625,6 +1679,99 @@ async function phase12CustomProperties () { } } +async function phase13Variables () { + logPhase('Phase 13: Variables plugin — create, NOP check, update, verify') + const defaultBranch = await getDefaultBranch() + + // 13a: Create variables via repo settings file + { + const branch = 'smoke-test-phase13a' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/test.yml`, REPO_YML_VARIABLES, branch, '13a: add variables to test repo settings') + const pr = await createPR(ORG, ADMIN_REPO, '13a: create repo variables', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '13a: NOP check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `13a: NOP check run is success (got: ${checkRun.conclusion})`) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS) + + log('Verifying variables were created on test repo...') + const varsOk = await poll(async () => { + try { + const { data } = await octokit.request('GET /repos/{owner}/{repo}/actions/variables', { owner: ORG, repo: 'test' }) + const vars = data.variables || [] + const v1 = vars.find(v => v.name === 'SMOKE_VAR_ONE' && v.value === 'hello') + const v2 = vars.find(v => v.name === 'SMOKE_VAR_TWO' && v.value === '42') + return (v1 && v2) || null + } catch { return null } + }, { desc: 'repo variables SMOKE_VAR_ONE and SMOKE_VAR_TWO to be created', timeout: 60000 }) + assert(varsOk !== null, '13a: SMOKE_VAR_ONE and SMOKE_VAR_TWO created on test repo') + + await deleteBranch(ORG, ADMIN_REPO, branch) + } + + // 13b: Update SMOKE_VAR_ONE value and verify + { + const branch = 'smoke-test-phase13b' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/test.yml`, REPO_YML_VARIABLES_UPDATED, branch, '13b: update SMOKE_VAR_ONE value') + const pr = await createPR(ORG, ADMIN_REPO, '13b: update repo variable SMOKE_VAR_ONE', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '13b: NOP check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `13b: NOP check run is success (got: ${checkRun.conclusion})`) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS) + + log('Verifying SMOKE_VAR_ONE was updated...') + const updateOk = await poll(async () => { + try { + const { data } = await octokit.request('GET /repos/{owner}/{repo}/actions/variables', { owner: ORG, repo: 'test' }) + const v = (data.variables || []).find(v => v.name === 'SMOKE_VAR_ONE' && v.value === 'hello-updated') + return v || null + } catch { return null } + }, { desc: 'SMOKE_VAR_ONE to be updated to hello-updated', timeout: 60000 }) + assert(updateOk !== null, '13b: SMOKE_VAR_ONE updated to "hello-updated"') + + await deleteBranch(ORG, ADMIN_REPO, branch) + } + + // 13c: Remove variables from settings and verify they are deleted + { + const branch = 'smoke-test-phase13c' + await deleteBranch(ORG, ADMIN_REPO, branch) + await createBranch(ORG, ADMIN_REPO, branch) + const REPO_YML_NO_VARS = `repository:\n name: test\n` + await createOrUpdateFile(ORG, ADMIN_REPO, `${CONFIG_PATH}/repos/test.yml`, REPO_YML_NO_VARS, branch, '13c: remove variables from test repo settings') + const pr = await createPR(ORG, ADMIN_REPO, '13c: remove repo variables', branch, defaultBranch) + log('Waiting for NOP check run...') + await sleep(WEBHOOK_SETTLE_MS) + const checkRun = await waitForCheckRun(ORG, ADMIN_REPO, pr.head.sha) + assert(checkRun !== null, '13c: NOP check run completed') + if (checkRun) assert(checkRun.conclusion === 'success', `13c: NOP check run is success (got: ${checkRun.conclusion})`) + if (!await safeMerge(ORG, ADMIN_REPO, pr.number)) return + await sleep(WEBHOOK_SETTLE_MS) + + log('Verifying variables were removed from test repo...') + const removeOk = await poll(async () => { + try { + const { data } = await octokit.request('GET /repos/{owner}/{repo}/actions/variables', { owner: ORG, repo: 'test' }) + const vars = data.variables || [] + const noneLeft = !vars.find(v => v.name === 'SMOKE_VAR_ONE' || v.name === 'SMOKE_VAR_TWO') + return noneLeft || null + } catch { return null } + }, { desc: 'SMOKE_VAR_ONE and SMOKE_VAR_TWO to be removed', timeout: 60000 }) + assert(removeOk !== null, '13c: SMOKE_VAR_ONE and SMOKE_VAR_TWO removed from test repo') + + await deleteBranch(ORG, ADMIN_REPO, branch) + } +} + async function teardown () { logPhase('Phase 9: Teardown') @@ -1688,10 +1835,11 @@ async function main () { `) if (INTERACTIVE) log('\x1b[33m[interactive] Mode enabled — will pause after each phase.\x1b[0m') + if (ONLY_PHASES !== null) log(`\x1b[33m[phase filter] Running setup + phase(s) [${[...ONLY_PHASES].join(', ')}] + teardown only.\x1b[0m`) let doTeardown = true try { - const phases = [ + const allPhases = [ ['Phase 0: Setup', setup], ['Phase 1: Create test repo', phase1CreateRepo], ['Phase 2: Drift remediation - Team removal', phase2DriftTeam], @@ -1704,8 +1852,24 @@ async function main () { ['Phase 8: Org-level settings', phase8OrgSettings], ['Phase 10: disable_plugins', phase10DisablePlugins], ['Phase 11: additive_plugins', phase11AdditivePlugins], - ['Phase 12: custom_properties', phase12CustomProperties] + ['Phase 12: custom_properties', phase12CustomProperties], + ['Phase 13: variables', phase13Variables] ] + + // When --phase is given, only run setup (phase 0) + the requested phase(s). + // Phase labels start with "Phase N:" so we match on that prefix. + const phases = ONLY_PHASES !== null + ? allPhases.filter(([label]) => { + if (label.startsWith('Phase 0:')) return true + const m = label.match(/^Phase (\d+)[:\s]/) + return m !== null && ONLY_PHASES.has(parseInt(m[1], 10)) + }) + : allPhases + + if (ONLY_PHASES !== null && phases.length < 2) { + const valid = allPhases.map(([label]) => label.replace(/^Phase (\S+):.*/, '$1')).filter(n => n !== '0').join(', ') + throw new Error(`No phases matching [${[...ONLY_PHASES].join(', ')}] found. Valid phase numbers: ${valid}`) + } for (const [label, fn] of phases) { const action = await runPhase(label, fn) if (action === 'abort') { doTeardown = false; break } diff --git a/test/unit/lib/plugins/variables.test.js b/test/unit/lib/plugins/variables.test.js index 2784d7afd..91c06239b 100644 --- a/test/unit/lib/plugins/variables.test.js +++ b/test/unit/lib/plugins/variables.test.js @@ -1,78 +1,202 @@ const { when } = require('jest-when') const Variables = require('../../../../lib/plugins/variables') +const NopCommand = require('../../../../lib/nopcommand') describe('Variables', () => { let github const org = 'bkeepers' const repo = 'test' - function fillVariables (variables = []) { - return variables - } - - function configure () { - const log = { debug: console.debug, error: console.error } + function configure (nop = false, entries = [{ name: 'test', value: 'test' }]) { + const log = { debug: jest.fn(), error: console.error } const errors = [] - return new Variables(undefined, github, { owner: org, repo }, [{ name: 'test', value: 'test' }], log, errors) + return new Variables(nop, github, { owner: org, repo }, entries, log, errors) } - beforeAll(() => { + beforeEach(() => { github = { request: jest.fn().mockReturnValue(Promise.resolve(true)) } }) - it('sync', () => { - const plugin = configure() - - when(github.request) - .calledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) - .mockResolvedValue({ - data: { - variables: [ - fillVariables({ - variables: [] - }) - ] - } - }); - - ['variables'].forEach(() => { + describe('constructor', () => { + it('should uppercase entry names', () => { + const plugin = configure(false, [{ name: 'lower_case', value: 'val' }]) + expect(plugin.entries[0].name).toBe('LOWER_CASE') + }) + }) + + describe('find', () => { + it('should return only name and value fields', async () => { when(github.request) .calledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) .mockResolvedValue({ data: { - variables: [{ name: 'DELETE_me', value: 'test' }] + variables: [{ name: 'VAR1', value: 'val1', created_at: '2024-01-01', updated_at: '2024-01-02' }] } }) + + const plugin = configure() + const result = await plugin.find() + + expect(result).toEqual([{ name: 'VAR1', value: 'val1' }]) }) + }) - when(github.request).calledWith('POST /repos/:org/:repo/actions/variables').mockResolvedValue({}) + describe('changed', () => { + it('should return true when values differ', () => { + const plugin = configure() + expect(plugin.changed({ name: 'X', value: 'old' }, { name: 'X', value: 'new' })).toBe(true) + }) - return plugin.sync().then(() => { - expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/actions/variables', { org, repo }); + it('should return false when values match', () => { + const plugin = configure() + expect(plugin.changed({ name: 'X', value: 'same' }, { name: 'X', value: 'same' })).toBe(false) + }) + }) + + describe('sync', () => { + it('should add new and remove stale variables', () => { + const plugin = configure() + + when(github.request) + .calledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) + .mockResolvedValue({ + data: { + variables: [{ name: 'DELETE_ME', value: 'test' }] + } + }) - ['variables'].forEach(() => { - expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) + return plugin.sync().then(() => { + expect(github.request).toHaveBeenCalledWith( + 'DELETE /repos/:org/:repo/actions/variables/:variable_name', + expect.objectContaining({ org, repo, variable_name: 'DELETE_ME' }) + ) + + expect(github.request).toHaveBeenCalledWith( + 'POST /repos/:org/:repo/actions/variables', + expect.objectContaining({ org, repo, name: 'TEST', value: 'test' }) + ) }) + }) - expect(github.request).toHaveBeenCalledWith( - 'DELETE /repos/:org/:repo/actions/variables/:variable_name', - expect.objectContaining({ - org, - repo, - variable_name: 'DELETE_me' + it('should return NopCommands and not mutate when nop is true', async () => { + const plugin = configure(true) + + when(github.request) + .calledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) + .mockResolvedValue({ + data: { + variables: [{ name: 'EXISTING_VAR', value: 'existing-value' }] + } }) + + const result = await plugin.sync() + + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) + expect(github.request).not.toHaveBeenCalledWith( + expect.stringMatching(/^(POST|PATCH|DELETE)/), + expect.anything() ) + expect(Array.isArray(result)).toBe(true) + expect(result.length).toBeGreaterThan(0) + const flat = result.flat() + flat.forEach(cmd => expect(cmd).toBeInstanceOf(NopCommand)) + }) + + it('should return NopCommand results when updating via sync', async () => { + const plugin = configure(true, [{ name: 'TEST', value: 'new-value' }]) + + when(github.request) + .calledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) + .mockResolvedValue({ + data: { + variables: [{ name: 'TEST', value: 'old-value' }] + } + }) + + const result = await plugin.sync() + + expect(github.request).not.toHaveBeenCalledWith( + expect.stringMatching(/^(POST|PATCH|DELETE)/), + expect.anything() + ) + + expect(Array.isArray(result)).toBe(true) + const flat = result.flat() + flat.forEach(cmd => expect(cmd).toBeInstanceOf(NopCommand)) + }) + }) + + describe('add', () => { + it('should return NopCommand array when nop is true', async () => { + const plugin = configure(true) + const result = await plugin.add({ name: 'NEW_VAR', value: 'new-value' }) + + expect(Array.isArray(result)).toBe(true) + expect(result[0]).toBeInstanceOf(NopCommand) + expect(result[0].plugin).toBe('Variables') + expect(github.request).not.toHaveBeenCalled() + }) + + it('should make POST request when nop is false', async () => { + const plugin = configure(false) + await plugin.add({ name: 'NEW_VAR', value: 'new-value' }) + expect(github.request).toHaveBeenCalledWith( 'POST /repos/:org/:repo/actions/variables', - expect.objectContaining({ - org, - repo, - name: 'TEST', - value: 'test' - }) + expect.objectContaining({ org, repo, name: 'NEW_VAR', value: 'new-value' }) + ) + }) + }) + + describe('remove', () => { + it('should return NopCommand array when nop is true', async () => { + const plugin = configure(true) + const result = await plugin.remove({ name: 'EXISTING_VAR', value: 'existing-value' }) + + expect(Array.isArray(result)).toBe(true) + expect(result[0]).toBeInstanceOf(NopCommand) + expect(result[0].plugin).toBe('Variables') + expect(github.request).not.toHaveBeenCalled() + }) + + it('should make DELETE request when nop is false', async () => { + const plugin = configure(false) + await plugin.remove({ name: 'EXISTING_VAR', value: 'existing-value' }) + + expect(github.request).toHaveBeenCalledWith( + 'DELETE /repos/:org/:repo/actions/variables/:variable_name', + expect.objectContaining({ org, repo, variable_name: 'EXISTING_VAR' }) + ) + }) + }) + + describe('update', () => { + it('should return NopCommand array when nop is true', async () => { + const plugin = configure(true) + const result = await plugin.update( + { name: 'VAR1', value: 'old-value' }, + { name: 'VAR1', value: 'new-value' } + ) + + expect(Array.isArray(result)).toBe(true) + expect(result[0]).toBeInstanceOf(NopCommand) + expect(result[0].plugin).toBe('Variables') + expect(github.request).not.toHaveBeenCalled() + }) + + it('should make PATCH request when nop is false', async () => { + const plugin = configure(false) + await plugin.update( + { name: 'VAR1', value: 'old-value' }, + { name: 'VAR1', value: 'new-value' } + ) + + expect(github.request).toHaveBeenCalledWith( + 'PATCH /repos/:org/:repo/actions/variables/:variable_name', + expect.objectContaining({ org, repo, variable_name: 'VAR1', value: 'new-value' }) ) }) })