From 9e7a946b65e50dbf39fcbb5e2736f43cbda52107 Mon Sep 17 00:00:00 2001 From: Piotr Galar Date: Thu, 16 Jul 2026 14:11:12 +0100 Subject: [PATCH 1/3] Add inactive member workflows --- .../workflows/finalize-membership-changes.yml | 99 ++++++ .github/workflows/update-inactive-members.yml | 119 +++++++ docs/SETUP.md | 12 +- .../__tests__/actions/access-summary.test.ts | 123 +++++++ .../finalize-membership-changes.test.ts | 43 +++ .../actions/update-inactive-members.test.ts | 157 +++++++++ .../actions/finalize-membership-changes.ts | 131 ++++++++ scripts/src/actions/fix-yaml-config.ts | 13 +- scripts/src/actions/shared/access-summary.ts | 299 +++++++++++++++++ .../actions/shared/describe-access-changes.ts | 227 ++++++------- .../src/actions/update-inactive-members.ts | 302 ++++++++++++++++++ scripts/src/github.ts | 26 ++ scripts/src/terraform/schema.ts | 2 + 13 files changed, 1416 insertions(+), 137 deletions(-) create mode 100644 .github/workflows/finalize-membership-changes.yml create mode 100644 .github/workflows/update-inactive-members.yml create mode 100644 scripts/__tests__/actions/access-summary.test.ts create mode 100644 scripts/__tests__/actions/finalize-membership-changes.test.ts create mode 100644 scripts/__tests__/actions/update-inactive-members.test.ts create mode 100644 scripts/src/actions/finalize-membership-changes.ts create mode 100644 scripts/src/actions/shared/access-summary.ts create mode 100644 scripts/src/actions/update-inactive-members.ts diff --git a/.github/workflows/finalize-membership-changes.yml b/.github/workflows/finalize-membership-changes.yml new file mode 100644 index 0000000..1aa3589 --- /dev/null +++ b/.github/workflows/finalize-membership-changes.yml @@ -0,0 +1,99 @@ +name: Finalize Membership Changes + +on: + workflow_dispatch: + inputs: + organization: + description: Organization whose membership changes should be finalized + required: true + mode: + description: Which potential member changes to apply + required: true + default: both + type: choice + options: + - both + - convert-potential-outside-collaborators + - remove-potential-no-members + ignore: + description: Comma, space, or newline separated usernames to ignore + required: false + only: + description: Comma, space, or newline separated usernames to target + required: false + +defaults: + run: + shell: bash + +jobs: + preview: + permissions: + contents: read + name: Preview membership changes + runs-on: ubuntu-latest + outputs: + affected-users-json: ${{ steps.preview.outputs.affected-users-json }} + env: + TF_WORKSPACE: ${{ github.event.inputs.organization }} + steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - name: Install pnpm + uses: pnpm/action-setup@91ab88e2619ed1f46221f0ba42d1492c02baf788 # v6.0.6 + with: + version: 10 + - name: Use Node.js lts/* + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: lts/* + cache: '' + - name: Initialize scripts + run: pnpm install --frozen-lockfile && pnpm run build + working-directory: scripts + - name: Preview membership changes + id: preview + run: node lib/actions/finalize-membership-changes.js + working-directory: scripts + env: + MODE: ${{ github.event.inputs.mode }} + IGNORE: ${{ github.event.inputs.ignore }} + ONLY: ${{ github.event.inputs.only }} + APPLY: 'false' + + apply: + needs: [preview] + if: needs.preview.outputs.affected-users-json != '[]' + permissions: + contents: read + name: Apply membership changes + runs-on: ubuntu-latest + environment: membership-write + env: + GITHUB_APP_ID: ${{ secrets.RW_GITHUB_APP_ID }} + GITHUB_APP_INSTALLATION_ID: ${{ secrets[format('RW_GITHUB_APP_INSTALLATION_ID_{0}', github.event.inputs.organization)] || secrets.RW_GITHUB_APP_INSTALLATION_ID }} + GITHUB_APP_PEM_FILE: ${{ secrets.RW_GITHUB_APP_PEM_FILE }} + TF_WORKSPACE: ${{ github.event.inputs.organization }} + steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - name: Install pnpm + uses: pnpm/action-setup@91ab88e2619ed1f46221f0ba42d1492c02baf788 # v6.0.6 + with: + version: 10 + - name: Use Node.js lts/* + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: lts/* + cache: '' + - name: Initialize scripts + run: pnpm install --frozen-lockfile && pnpm run build + working-directory: scripts + - name: Apply membership changes + run: node lib/actions/finalize-membership-changes.js + working-directory: scripts + env: + MODE: ${{ github.event.inputs.mode }} + IGNORE: ${{ github.event.inputs.ignore }} + ONLY: ${{ github.event.inputs.only }} + APPLY: 'true' diff --git a/.github/workflows/update-inactive-members.yml b/.github/workflows/update-inactive-members.yml new file mode 100644 index 0000000..678e402 --- /dev/null +++ b/.github/workflows/update-inactive-members.yml @@ -0,0 +1,119 @@ +name: Update Inactive Members + +on: + workflow_dispatch: + inputs: + organization: + description: Organization config to update + required: true + cutoff-date: + description: Inactive before this date (YYYY-MM-DD) + required: false + limit: + description: Remove up to this many longest-inactive members + required: false + ignore: + description: Comma, space, or newline separated usernames to ignore + required: false + only: + description: Comma, space, or newline separated usernames to target + required: false + public-repo-access: + description: Whether to retain effective access to public repositories + required: true + default: retain + type: choice + options: + - retain + - remove + +defaults: + run: + shell: bash + +jobs: + update: + permissions: + contents: write + pull-requests: write + name: Update inactive members + runs-on: ubuntu-latest + environment: push + env: + GITHUB_APP_ID: ${{ secrets.RO_GITHUB_APP_ID }} + GITHUB_APP_INSTALLATION_ID: ${{ secrets[format('RO_GITHUB_APP_INSTALLATION_ID_{0}', github.event.inputs.organization)] || secrets.RO_GITHUB_APP_INSTALLATION_ID }} + GITHUB_APP_PEM_FILE: ${{ secrets.RO_GITHUB_APP_PEM_FILE }} + TF_WORKSPACE: ${{ github.event.inputs.organization }} + steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - name: Install pnpm + uses: pnpm/action-setup@91ab88e2619ed1f46221f0ba42d1492c02baf788 # v6.0.6 + with: + version: 10 + - name: Use Node.js lts/* + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: lts/* + cache: '' + - name: Initialize scripts + run: pnpm install --frozen-lockfile && pnpm run build + working-directory: scripts + - name: Update inactive members + id: update + run: node lib/actions/update-inactive-members.js + working-directory: scripts + env: + CUTOFF_DATE: ${{ github.event.inputs['cutoff-date'] }} + LIMIT: ${{ github.event.inputs.limit }} + IGNORE: ${{ github.event.inputs.ignore }} + ONLY: ${{ github.event.inputs.only }} + PUBLIC_REPO_ACCESS: ${{ github.event.inputs['public-repo-access'] }} + - name: Check if organization config was modified + id: config-modified + env: + ORGANIZATION: ${{ github.event.inputs.organization }} + run: | + if [ -z "$(git status --porcelain -- "github/${ORGANIZATION}.yml")" ]; then + echo "this=false" >> $GITHUB_OUTPUT + else + echo "this=true" >> $GITHUB_OUTPUT + fi + - uses: ./.github/actions/git-config-user + if: steps.config-modified.outputs.this == 'true' + - name: Create draft pull request + if: steps.config-modified.outputs.this == 'true' + env: + ORGANIZATION: ${{ github.event.inputs.organization }} + CUTOFF_DATE: ${{ github.event.inputs['cutoff-date'] }} + LIMIT: ${{ github.event.inputs.limit }} + IGNORE: ${{ github.event.inputs.ignore }} + ONLY: ${{ github.event.inputs.only }} + PUBLIC_REPO_ACCESS: ${{ github.event.inputs['public-repo-access'] }} + AFFECTED_USERS: ${{ steps.update.outputs.affected-users }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + branch="inactive-members-${ORGANIZATION}-${GITHUB_RUN_ID}" + body="$(mktemp)" + { + echo 'The changes in this PR were made by a bot. Please review carefully.' + echo + echo "Organization: ${ORGANIZATION}" + echo "Cutoff date: ${CUTOFF_DATE:-not set}" + echo "Limit: ${LIMIT:-not set}" + echo "Ignore: ${IGNORE:-not set}" + echo "Only: ${ONLY:-not set}" + echo "Public repo access: ${PUBLIC_REPO_ACCESS}" + echo "Affected users: ${AFFECTED_USERS:-none}" + } > "${body}" + + git checkout -B "${branch}" + git add "github/${ORGANIZATION}.yml" + git commit -m "update-inactive-members@${GITHUB_RUN_ID} ${ORGANIZATION}" + git push origin "${branch}" --force + gh pr create \ + --draft \ + --title "Update inactive members for ${ORGANIZATION}" \ + --body-file "${body}" \ + --head "${branch}" \ + --base "${GITHUB_REF_NAME}" diff --git a/docs/SETUP.md b/docs/SETUP.md index 85cc586..03bedc6 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -110,13 +110,14 @@ - `Pull requests`: `Read & Write` - `Workflows`: `Read & Write` - `Organization permissions` - - `Members`: `Read & Write` + - `Members`: `Read & Write` (required for Terraform membership management and the `Finalize Membership Changes` workflow) - [ ] [Install the GitHub Apps](https://docs.github.com/en/developers/apps/managing-github-apps/installing-github-apps) in the GitHub organization for `All repositories` ## GitHub Actions Environments and Secrets -- [ ] Create GitHub Actions environments named `read`, `write`, and `push`, and configure protection rules such as required reviewers. Workflows that read organization state reference `read`; workflows that write organization state reference `write`; workflows that push generated changes to the GitHub Management repository reference `push`. +- [ ] Create GitHub Actions environments named `read`, `write`, `push`, and `membership-write`, and configure protection rules such as required reviewers. Workflows that read organization state reference `read`; workflows that write organization state reference `write`; workflows that push generated changes to the GitHub Management repository reference `push`; workflows that directly convert or remove organization members through the GitHub API reference `membership-write`. +- [ ] Configure the `membership-write` environment with required reviewers. Treat approval of this environment as approval to call the GitHub API for every user listed by the `Finalize Membership Changes` workflow preview job. - [ ] [Create encrypted secrets](https://docs.github.com/en/actions/security-guides/encrypted-secrets#creating-encrypted-secrets-for-an-organization) for the GitHub organization and allow the repository to access them (\*replace `$GITHUB_ORGANIZATION_NAME` with the GitHub organization name) - *these secrets are read by the GitHub Action workflows* - [ ] Go to `https://github.com/organizations/$GITHUB_ORGANIZATION_NAME/settings/apps/$GITHUB_APP_NAME` and copy the `App ID` - [ ] `RO_GITHUB_APP_ID` @@ -150,6 +151,13 @@ - [ ] Follow [How to synchronize GitHub Management with GitHub?](HOWTOS.md#synchronize-github-management-with-github) to commit the terraform lock and initialize terraform state +## Inactive Member Workflows + +- [ ] Use `Update Inactive Members` to create a draft PR that removes selected members from teams and repository collaborators in `github/$ORGANIZATION_NAME.yml`. The workflow requires either `cutoff-date` or `only`, supports `ignore` and `limit`, and can retain effective public repository access by converting that access to direct public repository collaborators in the YAML config. +- [ ] Review and merge the draft PR through the normal GitHub Management PR flow. +- [ ] Use `Finalize Membership Changes` only after the YAML PR has landed and the preview job lists the expected users. The `membership-write` environment approval gates API calls that convert potential outside collaborators or remove potential no members. +- [ ] After `Finalize Membership Changes` completes, run `Sync` for the same organization so Terraform state and YAML config reflect the membership changes made through the GitHub API. + ## GitHub Management Repository Protections *NOTE*: Advanced users might have to skip/adjust this step if they are not managing some of the arguments/attributes mentioned here with GitHub Management. diff --git a/scripts/__tests__/actions/access-summary.test.ts b/scripts/__tests__/actions/access-summary.test.ts new file mode 100644 index 0000000..a59aa6e --- /dev/null +++ b/scripts/__tests__/actions/access-summary.test.ts @@ -0,0 +1,123 @@ +import 'reflect-metadata' + +import assert from 'node:assert' +import {describe, it} from 'node:test' +import {Config} from '../../src/yaml/config.js' +import {State} from '../../src/terraform/state.js' +import { + categorizeAccessSummary, + getAccessSummaryFrom +} from '../../src/actions/shared/access-summary.js' +import { + describeAccessChanges, + describeAccessReport +} from '../../src/actions/shared/describe-access-changes.js' +import {StateSchema} from '../../src/terraform/schema.js' + +describe('access summaries', () => { + it('categorizes post-change users', () => { + const config = new Config(` +members: + member: + - alice + - carol + - dave + - kept # KEEP: manual exception +repositories: + private-repo: + collaborators: + pull: + - outside + visibility: private + public-repo: + collaborators: + pull: + - alice + visibility: public + team-repo: + teams: + push: + - maintainers + visibility: public +teams: + maintainers: + members: + member: + - dave +`) + + const categories = categorizeAccessSummary(getAccessSummaryFrom(config)) + + assert.deepEqual(categories.outsideCollaborators, ['outside']) + assert.deepEqual(categories.potentialOutsideCollaborators, ['alice']) + assert.deepEqual(categories.potentialNoMembers, ['carol']) + assert.deepEqual(categories.anyOtherMembers, ['dave', 'kept']) + }) + + it('annotates repository visibility in access changes and summaries', () => { + const state = new State( + JSON.stringify({ + values: { + root_module: { + resources: [ + { + mode: 'managed', + index: 'alice', + address: 'github_membership.this["alice"]', + type: 'github_membership', + values: { + username: 'alice', + role: 'member' + } + }, + { + mode: 'managed', + index: 'public-repo', + address: 'github_repository.this["public-repo"]', + type: 'github_repository', + values: { + name: 'public-repo', + visibility: 'public' + } + }, + { + mode: 'managed', + index: 'public-repo:alice', + address: + 'github_repository_collaborator.this["public-repo:alice"]', + type: 'github_repository_collaborator', + values: { + repository: 'public-repo', + username: 'alice', + permission: 'pull' + } + } + ] + } + } + } satisfies StateSchema) + ) + const config = new Config(` +members: + member: + - alice +repositories: + public-repo: + collaborators: + push: + - alice + visibility: public +`) + + const changes = describeAccessChanges(state, config) + const report = describeAccessReport(state, config) + + assert.match( + changes, + /will have the permission to public-repo \(public\) change from pull to push/ + ) + assert.match(report, /Potential outside collaborators<\/summary>/) + assert.match(report, /Affected users: alice/) + assert.match(report, /has push permission to public-repo \(public\)/) + }) +}) diff --git a/scripts/__tests__/actions/finalize-membership-changes.test.ts b/scripts/__tests__/actions/finalize-membership-changes.test.ts new file mode 100644 index 0000000..5ac2819 --- /dev/null +++ b/scripts/__tests__/actions/finalize-membership-changes.test.ts @@ -0,0 +1,43 @@ +import 'reflect-metadata' + +import assert from 'node:assert' +import {describe, it} from 'node:test' +import {Config} from '../../src/yaml/config.js' +import { + parseFinalizeMembershipMode, + planFinalizeMembershipChanges +} from '../../src/actions/finalize-membership-changes.js' + +describe('finalize membership changes', () => { + it('validates mode input', () => { + assert.equal(parseFinalizeMembershipMode('both'), 'both') + assert.throws(() => parseFinalizeMembershipMode('invalid')) + }) + + it('plans filtered conversion and removal targets', () => { + const config = new Config(` +members: + member: + - outside-candidate + - no-member-candidate + - ignored +repositories: + public-repo: + collaborators: + pull: + - outside-candidate + - ignored + visibility: public +`) + + const plan = planFinalizeMembershipChanges( + config, + 'both', + ['ignored'], + ['outside-candidate', 'no-member-candidate', 'ignored'] + ) + + assert.deepEqual(plan.potentialOutsideCollaborators, ['outside-candidate']) + assert.deepEqual(plan.potentialNoMembers, ['no-member-candidate']) + }) +}) diff --git a/scripts/__tests__/actions/update-inactive-members.test.ts b/scripts/__tests__/actions/update-inactive-members.test.ts new file mode 100644 index 0000000..a954997 --- /dev/null +++ b/scripts/__tests__/actions/update-inactive-members.test.ts @@ -0,0 +1,157 @@ +import 'reflect-metadata' + +import assert from 'node:assert' +import {describe, it} from 'node:test' +import {Config} from '../../src/yaml/config.js' +import { + parseCutoffDate, + parseLimit, + selectInactiveMembers, + updateInactiveMembersConfig +} from '../../src/actions/update-inactive-members.js' +import {TeamMember} from '../../src/resources/team-member.js' +import {RepositoryCollaborator} from '../../src/resources/repository-collaborator.js' + +describe('update inactive members', () => { + it('requires cutoff date or only list', () => { + const config = new Config('members:\n member:\n - alice\n') + + assert.throws(() => + selectInactiveMembers(config, [], { + ignore: [], + only: [], + publicRepoAccess: 'retain' + }) + ) + }) + + it('validates workflow inputs', () => { + assert.equal( + parseCutoffDate('2025-01-02')?.toISOString(), + '2025-01-02T00:00:00.000Z' + ) + assert.equal(parseLimit('2'), 2) + assert.throws(() => parseCutoffDate('01-02-2025')) + assert.throws(() => parseLimit('0')) + }) + + it('selects inactive members with only, ignore, limit, and KEEP handling', () => { + const config = new Config(` +members: + member: + - active + - ignored + - kept # KEEP: manual exception + - manual + - never-active + - old +`) + + const selected = selectInactiveMembers( + config, + [ + {username: 'active', latestActivity: new Date('2025-01-01T00:00:00Z')}, + {username: 'old', latestActivity: new Date('2023-01-01T00:00:00Z')} + ], + { + cutoffDate: new Date('2024-01-01T00:00:00Z'), + limit: 2, + ignore: ['ignored'], + only: ['active', 'ignored', 'kept', 'manual', 'never-active', 'old'], + publicRepoAccess: 'retain' + } + ) + + assert.deepEqual(selected, ['manual', 'never-active']) + }) + + it('retains effective public repository access when configured', () => { + const config = new Config(` +members: + member: + - alice +repositories: + private-repo: + collaborators: + pull: + - alice + teams: + admin: + - maintainers + visibility: private + public-repo: + collaborators: + pull: + - alice + teams: + push: + - maintainers + visibility: public +teams: + maintainers: + members: + member: + - alice +`) + + updateInactiveMembersConfig(config, ['alice'], 'retain') + + assert.equal( + config + .getResources(TeamMember) + .some(teamMember => teamMember.username === 'alice'), + false + ) + assert.equal( + config + .getResources(RepositoryCollaborator) + .some( + collaborator => + collaborator.username === 'alice' && + collaborator.repository === 'private-repo' + ), + false + ) + + const publicCollaborator = config + .getResources(RepositoryCollaborator) + .find( + collaborator => + collaborator.username === 'alice' && + collaborator.repository === 'public-repo' + ) + + assert.equal(publicCollaborator?.permission, 'push') + }) + + it('removes public repository access when configured', () => { + const config = new Config(` +members: + member: + - alice +repositories: + public-repo: + collaborators: + pull: + - alice + teams: + push: + - maintainers + visibility: public +teams: + maintainers: + members: + member: + - alice +`) + + updateInactiveMembersConfig(config, ['alice'], 'remove') + + assert.equal( + config + .getResources(RepositoryCollaborator) + .some(collaborator => collaborator.username === 'alice'), + false + ) + }) +}) diff --git a/scripts/src/actions/finalize-membership-changes.ts b/scripts/src/actions/finalize-membership-changes.ts new file mode 100644 index 0000000..b86bcd6 --- /dev/null +++ b/scripts/src/actions/finalize-membership-changes.ts @@ -0,0 +1,131 @@ +import 'reflect-metadata' +import * as core from '@actions/core' +import {pathToFileURL} from 'url' +import {Config} from '../yaml/config.js' +import {GitHub} from '../github.js' +import { + categorizeAccessSummary, + getAccessSummaryFrom, + parseUserList +} from './shared/access-summary.js' + +export type FinalizeMembershipMode = + | 'convert-potential-outside-collaborators' + | 'remove-potential-no-members' + | 'both' + +export type FinalizeMembershipPlan = { + potentialOutsideCollaborators: string[] + potentialNoMembers: string[] +} + +export function parseFinalizeMembershipMode( + source?: string +): FinalizeMembershipMode { + if ( + source === 'convert-potential-outside-collaborators' || + source === 'remove-potential-no-members' || + source === 'both' + ) { + return source + } + throw new Error( + 'mode must be convert-potential-outside-collaborators, remove-potential-no-members, or both' + ) +} + +export function planFinalizeMembershipChanges( + config: Config, + mode: FinalizeMembershipMode, + ignore: string[], + only: string[] +): FinalizeMembershipPlan { + const ignoredUsers = new Set(ignore) + const onlyUsers = new Set(only) + const categories = categorizeAccessSummary(getAccessSummaryFrom(config)) + + const filter = (users: string[]): string[] => + users + .filter(username => !ignoredUsers.has(username)) + .filter(username => onlyUsers.size === 0 || onlyUsers.has(username)) + .sort() + + return { + potentialOutsideCollaborators: + mode === 'remove-potential-no-members' + ? [] + : filter(categories.potentialOutsideCollaborators), + potentialNoMembers: + mode === 'convert-potential-outside-collaborators' + ? [] + : filter(categories.potentialNoMembers) + } +} + +export function formatFinalizeMembershipPlan( + plan: FinalizeMembershipPlan +): string { + return [ + 'Potential outside collaborators to convert:', + plan.potentialOutsideCollaborators.length > 0 + ? plan.potentialOutsideCollaborators + .map(username => `- ${username}`) + .join('\n') + : '- none', + '', + 'Potential no members to remove:', + plan.potentialNoMembers.length > 0 + ? plan.potentialNoMembers.map(username => `- ${username}`).join('\n') + : '- none' + ].join('\n') +} + +async function run(): Promise { + const mode = parseFinalizeMembershipMode(process.env.MODE || 'both') + const ignore = parseUserList(process.env.IGNORE) + const only = parseUserList(process.env.ONLY) + const shouldApply = process.env.APPLY === 'true' + const config = Config.FromPath() + + const plan = planFinalizeMembershipChanges(config, mode, ignore, only) + const affectedUsers = Array.from( + new Set([...plan.potentialOutsideCollaborators, ...plan.potentialNoMembers]) + ).sort() + + core.info(formatFinalizeMembershipPlan(plan)) + core.setOutput('affected-users', affectedUsers.join(', ')) + core.setOutput('affected-users-json', JSON.stringify(affectedUsers)) + core.setOutput( + 'potential-outside-collaborators-json', + JSON.stringify(plan.potentialOutsideCollaborators) + ) + core.setOutput( + 'potential-no-members-json', + JSON.stringify(plan.potentialNoMembers) + ) + + if (!shouldApply) { + return + } + + const github = await GitHub.getGitHub() + + for (const username of plan.potentialOutsideCollaborators) { + await github.convertMemberToOutsideCollaborator(username) + } + + for (const username of plan.potentialNoMembers) { + await github.removeOrganizationMembership(username) + } + + core.notice( + 'Membership changes are complete. Run the Sync workflow for this organization so Terraform state and YAML config reflect GitHub.' + ) +} + +if ( + process.argv[1] && + import.meta.url === pathToFileURL(process.argv[1]).href +) { + run() +} diff --git a/scripts/src/actions/fix-yaml-config.ts b/scripts/src/actions/fix-yaml-config.ts index a88b191..733d370 100644 --- a/scripts/src/actions/fix-yaml-config.ts +++ b/scripts/src/actions/fix-yaml-config.ts @@ -10,18 +10,7 @@ async function run(): Promise { const accessChangesDescription = await runDescribeAccessChanges() - core.setOutput( - 'comment', - `The following access changes will be introduced as a result of applying the plan: - -
Access Changes - -\`\`\` -${accessChangesDescription} -\`\`\` - -
` - ) + core.setOutput('comment', accessChangesDescription) } run() diff --git a/scripts/src/actions/shared/access-summary.ts b/scripts/src/actions/shared/access-summary.ts new file mode 100644 index 0000000..5c5c112 --- /dev/null +++ b/scripts/src/actions/shared/access-summary.ts @@ -0,0 +1,299 @@ +import {Config} from '../../yaml/config.js' +import {State} from '../../terraform/state.js' +import {RepositoryCollaborator} from '../../resources/repository-collaborator.js' +import {Member} from '../../resources/member.js' +import {TeamMember} from '../../resources/team-member.js' +import {RepositoryTeam} from '../../resources/repository-team.js' +import {Repository, Visibility} from '../../resources/repository.js' + +export type RepositoryAccess = { + permission: string + visibility: Visibility +} + +export type UserAccess = { + role?: string + repositories: Record + directRepositories: Record + teams: string[] + hasKeepComment: boolean +} + +export type AccessSummary = Record + +export type AccessCategory = + | 'outsideCollaborators' + | 'potentialOutsideCollaborators' + | 'potentialNoMembers' + | 'anyOtherMembers' + +export type AccessCategories = Record + +export const permissions = ['admin', 'maintain', 'push', 'triage', 'pull'] + +export function betterPermission(current: string, next: string): string { + return permissions.indexOf(next) < permissions.indexOf(current) + ? next + : current +} + +export function parseUserList(source?: string): string[] { + return Array.from( + new Set( + (source || '') + .split(/[\s,]+/) + .map(username => username.trim().toLowerCase()) + .filter(username => username !== '') + ) + ).sort() +} + +export function hasKeepComment(config: Config, member: Member): boolean { + const node = config.document.getIn( + member.getSchemaPath(config.get()), + true + ) as {comment?: string} | undefined + return node?.comment?.includes('KEEP:') ?? false +} + +export function getAccessSummaryFrom(source: State | Config): AccessSummary { + const members = source.getResources(Member) + const teamMembers = source.getResources(TeamMember) + const teamRepositories = source.getResources(RepositoryTeam) + const repositoryCollaborators = source.getResources(RepositoryCollaborator) + const repositories = source.getResources(Repository) + + const repositoryVisibility = new Map( + repositories.map(repository => [ + repository.name.toLowerCase(), + repository.visibility ?? Visibility.Private + ]) + ) + const archivedRepositories = repositories + .filter(repository => repository.archived) + .map(repository => repository.name.toLowerCase()) + + const usernames = new Set([ + ...members.map(member => member.username.toLowerCase()), + ...teamMembers.map(teamMember => teamMember.username.toLowerCase()), + ...repositoryCollaborators.map(collaborator => + collaborator.username.toLowerCase() + ) + ]) + + const accessSummary: AccessSummary = {} + + for (const username of Array.from(usernames).sort()) { + const member = members.find( + candidate => candidate.username.toLowerCase() === username + ) + const role = member?.role + const teams = teamMembers + .filter(teamMember => teamMember.username.toLowerCase() === username) + .map(teamMember => teamMember.team.toLowerCase()) + .sort() + const repositoryCollaborator = repositoryCollaborators + .filter(collaborator => collaborator.username.toLowerCase() === username) + .filter( + collaborator => + !archivedRepositories.includes(collaborator.repository.toLowerCase()) + ) + const teamRepository = teamRepositories + .filter(repository => teams.includes(repository.team.toLowerCase())) + .filter( + repository => + !archivedRepositories.includes(repository.repository.toLowerCase()) + ) + + const repositories: Record = {} + const directRepositories: Record = {} + + for (const rc of repositoryCollaborator) { + const repository = rc.repository.toLowerCase() + const access = { + permission: rc.permission, + visibility: repositoryVisibility.get(repository) ?? Visibility.Private + } + directRepositories[repository] = directRepositories[repository] + ? { + ...access, + permission: betterPermission( + directRepositories[repository].permission, + access.permission + ) + } + : access + repositories[repository] = repositories[repository] + ? { + ...access, + permission: betterPermission( + repositories[repository].permission, + access.permission + ) + } + : access + } + + for (const tr of teamRepository) { + const repository = tr.repository.toLowerCase() + const access = { + permission: tr.permission, + visibility: repositoryVisibility.get(repository) ?? Visibility.Private + } + repositories[repository] = repositories[repository] + ? { + ...access, + permission: betterPermission( + repositories[repository].permission, + access.permission + ) + } + : access + } + + const hasKeep = + source instanceof Config && member !== undefined + ? hasKeepComment(source, member) + : false + + if ( + role !== undefined || + teams.length > 0 || + Object.keys(repositories).length > 0 + ) { + accessSummary[username] = { + role, + repositories, + directRepositories, + teams, + hasKeepComment: hasKeep + } + } + } + + return deepSort(accessSummary) +} + +export function getComparableAccessSummary(source: State | Config): Record< + string, + { + role?: string + repositories: Record + } +> { + return Object.fromEntries( + Object.entries(getAccessSummaryFrom(source)).map(([username, access]) => [ + username, + { + role: access.role, + repositories: Object.fromEntries( + Object.entries(access.repositories).map(([repository, value]) => [ + repository, + {permission: value.permission} + ]) + ) + } + ]) + ) +} + +export function categorizeAccessSummary( + summary: AccessSummary +): AccessCategories { + const categories: AccessCategories = { + outsideCollaborators: [], + potentialOutsideCollaborators: [], + potentialNoMembers: [], + anyOtherMembers: [] + } + + for (const [username, access] of Object.entries(summary)) { + const repositories = Object.values(access.repositories) + if (access.role === undefined) { + if (repositories.length > 0) { + categories.outsideCollaborators.push(username) + } + } else if ( + !access.hasKeepComment && + access.teams.length === 0 && + repositories.length > 0 && + repositories.every( + repository => repository.visibility === Visibility.Public + ) + ) { + categories.potentialOutsideCollaborators.push(username) + } else if (!access.hasKeepComment && repositories.length === 0) { + categories.potentialNoMembers.push(username) + } else { + categories.anyOtherMembers.push(username) + } + } + + for (const users of Object.values(categories)) { + users.sort() + } + + return categories +} + +export function formatRepositoryAccess( + repository: string, + access: RepositoryAccess +): string { + return `${repository} (${access.visibility})` +} + +export function formatAccessSummarySection( + title: string, + users: string[], + summary: AccessSummary +): string { + const lines = [ + `
${title}`, + '', + `Affected users: ${users.length > 0 ? users.join(', ') : 'none'}`, + '', + '```' + ] + + if (users.length === 0) { + lines.push('No users in this section') + } + + for (const username of users) { + const access = summary[username] + lines.push(`User ${username}:`) + const repositories = Object.entries(access.repositories) + if (repositories.length === 0) { + lines.push(' - has no repository access') + } else { + for (const [repository, repositoryAccess] of repositories) { + lines.push( + ` - has ${repositoryAccess.permission} permission to ${formatRepositoryAccess( + repository, + repositoryAccess + )}` + ) + } + } + } + + lines.push('```', '', '
') + return lines.join('\n') +} + +// deep sort object +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function deepSort(obj: any): any { + if (Array.isArray(obj)) { + return obj.map(deepSort) + } else if (obj !== null && typeof obj === 'object') { + const sorted: Record = {} + for (const key of Object.keys(obj).sort()) { + sorted[key] = deepSort(obj[key]) + } + return sorted + } else { + return obj + } +} diff --git a/scripts/src/actions/shared/describe-access-changes.ts b/scripts/src/actions/shared/describe-access-changes.ts index df76407..93f602d 100644 --- a/scripts/src/actions/shared/describe-access-changes.ts +++ b/scripts/src/actions/shared/describe-access-changes.ts @@ -1,128 +1,87 @@ import {Config} from '../../yaml/config.js' import {State} from '../../terraform/state.js' -import {RepositoryCollaborator} from '../../resources/repository-collaborator.js' -import {Member} from '../../resources/member.js' -import {TeamMember} from '../../resources/team-member.js' -import {RepositoryTeam} from '../../resources/repository-team.js' import diff from 'deep-diff' import * as core from '@actions/core' -import {Repository} from '../../resources/repository.js' - -type AccessSummary = Record< - string, - { - role?: string - repositories: Record - } -> - -function getAccessSummaryFrom(source: State | Config): AccessSummary { - const members = source.getResources(Member) - const teamMembers = source.getResources(TeamMember) - const teamRepositories = source.getResources(RepositoryTeam) - const repositoryCollaborators = source.getResources(RepositoryCollaborator) - - const archivedRepositories = source - .getResources(Repository) - .filter(repository => repository.archived) - .map(repository => repository.name.toLowerCase()) - - const usernames = new Set([ - ...members.map(member => member.username.toLowerCase()), - ...repositoryCollaborators.map(collaborator => - collaborator.username.toLowerCase() - ) - ]) - - const accessSummary: AccessSummary = {} - const permissions = ['admin', 'maintain', 'push', 'triage', 'pull'] - - for (const username of usernames) { - const role = members.find( - member => member.username.toLowerCase() === username - )?.role - const teams = teamMembers - .filter(teamMember => teamMember.username.toLowerCase() === username) - .map(teamMember => teamMember.team.toLowerCase()) - const repositoryCollaborator = repositoryCollaborators - .filter(collaborator => collaborator.username.toLowerCase() === username) - .filter( - collaborator => - !archivedRepositories.includes(collaborator.repository.toLowerCase()) - ) - const teamRepository = teamRepositories - .filter(repository => teams.includes(repository.team.toLowerCase())) - .filter( - repository => - !archivedRepositories.includes(repository.repository.toLowerCase()) - ) - - const repositories: Record = {} - - for (const rc of repositoryCollaborator) { - const repository = rc.repository.toLowerCase() - repositories[repository] = repositories[repository] ?? {} - if ( - !repositories[repository].permission || - permissions.indexOf(rc.permission) < - permissions.indexOf(repositories[repository].permission) - ) { - repositories[repository].permission = rc.permission - } - } - - for (const tr of teamRepository) { - const repository = tr.repository.toLowerCase() - repositories[repository] = repositories[repository] ?? {} - if ( - !repositories[repository].permission || - permissions.indexOf(tr.permission) < - permissions.indexOf(repositories[repository].permission) - ) { - repositories[repository].permission = tr.permission - } - } - - if (role !== undefined || Object.keys(repositories).length > 0) { - accessSummary[username] = { - role, - repositories - } - } - } - - return deepSort(accessSummary) -} - -// deep sort object -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function deepSort(obj: any): any { - if (Array.isArray(obj)) { - return obj.map(deepSort) - } else if (typeof obj === 'object') { - const sorted: Record = {} - for (const key of Object.keys(obj).sort()) { - sorted[key] = deepSort(obj[key]) - } - return sorted - } else { - return obj - } +import { + categorizeAccessSummary, + formatAccessSummarySection, + formatRepositoryAccess, + getAccessSummaryFrom, + getComparableAccessSummary, + RepositoryAccess +} from './access-summary.js' + +function repositoryLabel( + repository: string, + afterSummary: ReturnType, + beforeSummary: ReturnType +): string { + const access = + Object.values(afterSummary) + .map(user => user.repositories[repository]) + .find(Boolean) ?? + Object.values(beforeSummary) + .map(user => user.repositories[repository]) + .find(Boolean) ?? + ({permission: 'pull', visibility: 'private'} as RepositoryAccess) + + return formatRepositoryAccess(repository, access) } export async function runDescribeAccessChanges(): Promise { const state = await State.New() const config = Config.FromPath() - return await describeAccessChanges(state, config) + return describeAccessReport(state, config) } -export async function describeAccessChanges( - state: State, - config: Config -): Promise { - const before = getAccessSummaryFrom(state) +export function describeAccessReport(state: State, config: Config): string { + const accessChangesDescription = describeAccessChanges(state, config) const after = getAccessSummaryFrom(config) + const categories = categorizeAccessSummary(after) + + return [ + 'The following access changes will be introduced as a result of applying the plan:', + '', + '
Access Changes', + '', + '```', + accessChangesDescription, + '```', + '', + '
', + '', + formatAccessSummarySection( + 'Outside collaborators', + categories.outsideCollaborators, + after + ), + '', + formatAccessSummarySection( + 'Potential outside collaborators', + categories.potentialOutsideCollaborators, + after + ), + '', + formatAccessSummarySection( + 'Potential no members', + categories.potentialNoMembers, + after + ), + '', + formatAccessSummarySection( + 'Any other members', + categories.anyOtherMembers, + after + ) + ].join('\n') +} + +export function describeAccessChanges(state: State, config: Config): string { + const before = getComparableAccessSummary(state) + const after = getComparableAccessSummary(config) + const beforeWithVisibility = getAccessSummaryFrom(state) + const afterWithVisibility = getAccessSummaryFrom(config) core.info(JSON.stringify({before, after}, null, 2)) @@ -136,11 +95,10 @@ export async function describeAccessChanges( throw new Error(`Change ${change.kind} has no path`) } const path = change.path - changesByUser[path[0]] = changesByUser[path[0]] || [] - changesByUser[path[0]].push(change) + changesByUser[String(path[0])] = changesByUser[String(path[0])] || [] + changesByUser[String(path[0])].push(change) } - // iterate over changesByUser and build a description const lines = [] for (const [username, userChanges] of Object.entries(changesByUser)) { lines.push(`User ${username}:`) @@ -157,15 +115,20 @@ export async function describeAccessChanges( ` - will join the organization as a ${change.rhs} (remind them to accept the email invitation)` ) } else if (change.rhs === undefined) { - lines.push(` - will leave the organization`) + lines.push(' - will leave the organization') } else { lines.push( ` - will have the role in the organization change from ${change.lhs} to ${change.rhs}` ) } } else { + const repository = String(path[2]) lines.push( - ` - will have the permission to ${path[2]} change from ${change.lhs} to ${change.rhs}` + ` - will have the permission to ${repositoryLabel( + repository, + afterWithVisibility, + beforeWithVisibility + )} change from ${change.lhs} to ${change.rhs}` ) } break @@ -173,7 +136,7 @@ export async function describeAccessChanges( if (path.length === 1) { if (change.rhs.role) { lines.push( - ` - will join the organization as a ${change.rhs} (remind them to accept the email invitation)` + ` - will join the organization as a ${change.rhs.role} (remind them to accept the email invitation)` ) } if (change.rhs.repositories) { @@ -185,20 +148,29 @@ export async function describeAccessChanges( repositories )) { lines.push( - ` - will gain ${permission} permission to ${repository}` + ` - will gain ${permission} permission to ${repositoryLabel( + repository, + afterWithVisibility, + beforeWithVisibility + )}` ) } } } else { + const repository = String(path[2]) lines.push( - ` - will gain ${change.rhs.permission} permission to ${path[2]}` + ` - will gain ${change.rhs.permission} permission to ${repositoryLabel( + repository, + afterWithVisibility, + beforeWithVisibility + )}` ) } break case 'D': if (path.length === 1) { if (change.lhs.role) { - lines.push(` - will leave the organization`) + lines.push(' - will leave the organization') } if (change.lhs.repositories) { const repositories = change.lhs.repositories as unknown as Record< @@ -209,13 +181,22 @@ export async function describeAccessChanges( repositories )) { lines.push( - ` - will lose ${permission} permission to ${repository}` + ` - will lose ${permission} permission to ${repositoryLabel( + repository, + afterWithVisibility, + beforeWithVisibility + )}` ) } } } else { + const repository = String(path[2]) lines.push( - ` - will lose ${change.lhs.permission} permission to ${path[2]}` + ` - will lose ${change.lhs.permission} permission to ${repositoryLabel( + repository, + afterWithVisibility, + beforeWithVisibility + )}` ) } break diff --git a/scripts/src/actions/update-inactive-members.ts b/scripts/src/actions/update-inactive-members.ts new file mode 100644 index 0000000..f8bfdc2 --- /dev/null +++ b/scripts/src/actions/update-inactive-members.ts @@ -0,0 +1,302 @@ +import 'reflect-metadata' +import * as core from '@actions/core' +import {pathToFileURL} from 'url' +import {Config} from '../yaml/config.js' +import {Member} from '../resources/member.js' +import {Repository, Visibility} from '../resources/repository.js' +import { + Permission as RepositoryCollaboratorPermission, + RepositoryCollaborator +} from '../resources/repository-collaborator.js' +import {TeamMember} from '../resources/team-member.js' +import {GitHub} from '../github.js' +import { + betterPermission, + getAccessSummaryFrom, + hasKeepComment, + parseUserList +} from './shared/access-summary.js' + +export type PublicRepoAccess = 'retain' | 'remove' + +export type MemberActivity = { + username: string + latestActivity?: Date +} + +export type UpdateInactiveMembersOptions = { + cutoffDate?: Date + limit?: number + ignore: string[] + only: string[] + publicRepoAccess: PublicRepoAccess +} + +type ActivityRecord = { + username: string + createdAt: Date +} + +export function parseCutoffDate(source?: string): Date | undefined { + if (source === undefined || source.trim() === '') { + return undefined + } + if (!/^\d{4}-\d{2}-\d{2}$/.test(source)) { + throw new Error('cutoff-date must use YYYY-MM-DD format') + } + const date = new Date(`${source}T00:00:00.000Z`) + if (Number.isNaN(date.valueOf())) { + throw new Error('cutoff-date must be a valid date') + } + return date +} + +export function parseLimit(source?: string): number | undefined { + if (source === undefined || source.trim() === '') { + return undefined + } + const limit = Number(source) + if (!Number.isInteger(limit) || limit <= 0) { + throw new Error('limit must be a positive integer') + } + return limit +} + +export function parsePublicRepoAccess(source?: string): PublicRepoAccess { + if (source === 'retain' || source === 'remove') { + return source + } + throw new Error('public-repo-access must be retain or remove') +} + +export function selectInactiveMembers( + config: Config, + activities: MemberActivity[], + options: UpdateInactiveMembersOptions +): string[] { + if (options.cutoffDate === undefined && options.only.length === 0) { + throw new Error('Either cutoff-date or only must be provided') + } + + const activityByUsername = new Map( + activities.map(activity => [activity.username.toLowerCase(), activity]) + ) + const ignore = new Set(options.ignore) + const only = new Set(options.only) + + const candidates = config + .getResources(Member) + .filter(member => !hasKeepComment(config, member)) + .map(member => member.username.toLowerCase()) + .filter(username => !ignore.has(username)) + .filter(username => only.size === 0 || only.has(username)) + .filter(username => { + if (options.cutoffDate === undefined) { + return true + } + const latestActivity = activityByUsername.get(username)?.latestActivity + return latestActivity === undefined || latestActivity < options.cutoffDate + }) + .sort((a, b) => { + const aActivity = activityByUsername.get(a)?.latestActivity?.valueOf() + const bActivity = activityByUsername.get(b)?.latestActivity?.valueOf() + if (aActivity === undefined && bActivity === undefined) { + return a.localeCompare(b) + } + if (aActivity === undefined) { + return -1 + } + if (bActivity === undefined) { + return 1 + } + return aActivity - bActivity || a.localeCompare(b) + }) + + return options.limit === undefined + ? candidates + : candidates.slice(0, options.limit) +} + +export function updateInactiveMembersConfig( + config: Config, + usernames: string[], + publicRepoAccess: PublicRepoAccess +): string[] { + const targets = new Set(usernames.map(username => username.toLowerCase())) + const repositories = new Map( + config + .getResources(Repository) + .map(repository => [repository.name.toLowerCase(), repository]) + ) + const accessBefore = getAccessSummaryFrom(config) + const retainedPublicAccess = new Map< + string, + Map + >() + + if (publicRepoAccess === 'retain') { + for (const username of targets) { + const userAccess = accessBefore[username] + if (userAccess === undefined) { + continue + } + for (const [repository, access] of Object.entries( + userAccess.repositories + )) { + const configRepository = repositories.get(repository) + if ( + configRepository?.archived || + access.visibility !== Visibility.Public + ) { + continue + } + const retainedRepositories = + retainedPublicAccess.get(username) ?? new Map() + const current = retainedRepositories.get(repository) + retainedRepositories.set( + repository, + (current === undefined + ? access.permission + : betterPermission( + current, + access.permission + )) as RepositoryCollaboratorPermission + ) + retainedPublicAccess.set(username, retainedRepositories) + } + } + } + + for (const teamMember of config.getResources(TeamMember)) { + if (targets.has(teamMember.username.toLowerCase())) { + core.info(`Removing ${teamMember.username} from ${teamMember.team} team`) + config.removeResource(teamMember) + } + } + + for (const collaborator of config.getResources(RepositoryCollaborator)) { + if (targets.has(collaborator.username.toLowerCase())) { + core.info( + `Removing ${collaborator.username} from ${collaborator.repository} repository` + ) + config.removeResource(collaborator) + } + } + + for (const [username, retainedRepositories] of retainedPublicAccess) { + for (const [repository, permission] of retainedRepositories) { + core.info( + `Retaining ${username} ${permission} access to public repository ${repository}` + ) + config.addResource( + new RepositoryCollaborator(repository, username, permission) + ) + } + } + + return Array.from(targets).sort() +} + +function latestActivityByUser(activities: ActivityRecord[]): MemberActivity[] { + const latest = new Map() + for (const activity of activities) { + const username = activity.username.toLowerCase() + const previous = latest.get(username) + if (previous === undefined || activity.createdAt > previous) { + latest.set(username, activity.createdAt) + } + } + return Array.from(latest.entries()).map(([username, latestActivity]) => ({ + username, + latestActivity + })) +} + +async function collectActivities(since: Date): Promise { + const github = await GitHub.getGitHub() + const [ + githubRepositoryActivities, + githubRepositoryIssues, + githubRepositoryPullRequestReviewComments, + githubRepositoryIssueComments, + githubRepositoryCommitComments + ] = await Promise.all([ + github.listRepositoryActivities(since), + github.listRepositoryIssues(since), + github.listRepositoryPullRequestReviewComments(since), + github.listRepositoryIssueComments(since), + github.listRepositoryCommitComments(since) + ]) + + return latestActivityByUser( + [ + ...githubRepositoryActivities.map(({activity}) => ({ + username: activity.actor?.login, + createdAt: new Date(activity.timestamp) + })), + ...githubRepositoryIssues.map(({issue}) => ({ + username: issue.user?.login, + createdAt: new Date(issue.created_at) + })), + ...githubRepositoryPullRequestReviewComments.map(({comment}) => ({ + username: comment.user?.login, + createdAt: new Date(comment.created_at) + })), + ...githubRepositoryIssueComments.map(({comment}) => ({ + username: comment.user?.login, + createdAt: new Date(comment.created_at) + })), + ...githubRepositoryCommitComments.map(({comment}) => ({ + username: comment.user?.login, + createdAt: new Date(comment.created_at) + })) + ] + .filter( + ( + activity + ): activity is { + username: string + createdAt: Date + } => activity.username !== undefined + ) + .filter(activity => !Number.isNaN(activity.createdAt.valueOf())) + ) +} + +async function run(): Promise { + const cutoffDate = parseCutoffDate(process.env.CUTOFF_DATE) + const limit = parseLimit(process.env.LIMIT) + const ignore = parseUserList(process.env.IGNORE) + const only = parseUserList(process.env.ONLY) + const publicRepoAccess = parsePublicRepoAccess(process.env.PUBLIC_REPO_ACCESS) + + const config = Config.FromPath() + const activities = + cutoffDate === undefined + ? [] + : await collectActivities(limit === undefined ? cutoffDate : new Date(0)) + const selectedMembers = selectInactiveMembers(config, activities, { + cutoffDate, + limit, + ignore, + only, + publicRepoAccess + }) + const affectedUsers = updateInactiveMembersConfig( + config, + selectedMembers, + publicRepoAccess + ) + + config.save() + + core.setOutput('affected-users', affectedUsers.join(', ')) + core.setOutput('affected-users-json', JSON.stringify(affectedUsers)) +} + +if ( + process.argv[1] && + import.meta.url === pathToFileURL(process.argv[1]).href +) { + run() +} diff --git a/scripts/src/github.ts b/scripts/src/github.ts index 882e477..5fac482 100644 --- a/scripts/src/github.ts +++ b/scripts/src/github.ts @@ -587,4 +587,30 @@ export class GitHub { } return commitComments } + + async convertMemberToOutsideCollaborator(username: string): Promise { + core.info(`Converting ${username} to outside collaborator...`) + await this.client.request( + 'PUT /orgs/{org}/outside_collaborators/{username}', + { + org: env.GITHUB_ORG, + username, + async: false, + headers: { + 'X-GitHub-Api-Version': '2026-03-10' + } + } + ) + } + + async removeOrganizationMembership(username: string): Promise { + core.info(`Removing organization membership for ${username}...`) + await this.client.request('DELETE /orgs/{org}/memberships/{username}', { + org: env.GITHUB_ORG, + username, + headers: { + 'X-GitHub-Api-Version': '2026-03-10' + } + }) + } } diff --git a/scripts/src/terraform/schema.ts b/scripts/src/terraform/schema.ts index fded2c2..0f7b027 100644 --- a/scripts/src/terraform/schema.ts +++ b/scripts/src/terraform/schema.ts @@ -53,6 +53,8 @@ type ResourceSchema = { type: 'github_repository' values: { name: string + archived?: boolean + visibility?: 'private' | 'public' pages?: { source?: object[] }[] From e32fa783297cc0b68ef25c83e6f526d058499e76 Mon Sep 17 00:00:00 2001 From: Piotr Galar Date: Thu, 16 Jul 2026 18:04:00 +0100 Subject: [PATCH 2/3] Refine member update workflow --- .../workflows/finalize-membership-changes.yml | 99 ------------- ...nactive-members.yml => update-members.yml} | 16 +-- docs/SETUP.md | 11 +- .../__tests__/actions/access-summary.test.ts | 17 ++- .../finalize-membership-changes.test.ts | 43 ------ ...members.test.ts => update-members.test.ts} | 16 +-- .../actions/finalize-membership-changes.ts | 131 ------------------ scripts/src/actions/shared/access-summary.ts | 36 +++-- ...-inactive-members.ts => update-members.ts} | 12 +- scripts/src/github.ts | 26 ---- 10 files changed, 66 insertions(+), 341 deletions(-) delete mode 100644 .github/workflows/finalize-membership-changes.yml rename .github/workflows/{update-inactive-members.yml => update-members.yml} (91%) delete mode 100644 scripts/__tests__/actions/finalize-membership-changes.test.ts rename scripts/__tests__/actions/{update-inactive-members.test.ts => update-members.test.ts} (90%) delete mode 100644 scripts/src/actions/finalize-membership-changes.ts rename scripts/src/actions/{update-inactive-members.ts => update-members.ts} (96%) diff --git a/.github/workflows/finalize-membership-changes.yml b/.github/workflows/finalize-membership-changes.yml deleted file mode 100644 index 1aa3589..0000000 --- a/.github/workflows/finalize-membership-changes.yml +++ /dev/null @@ -1,99 +0,0 @@ -name: Finalize Membership Changes - -on: - workflow_dispatch: - inputs: - organization: - description: Organization whose membership changes should be finalized - required: true - mode: - description: Which potential member changes to apply - required: true - default: both - type: choice - options: - - both - - convert-potential-outside-collaborators - - remove-potential-no-members - ignore: - description: Comma, space, or newline separated usernames to ignore - required: false - only: - description: Comma, space, or newline separated usernames to target - required: false - -defaults: - run: - shell: bash - -jobs: - preview: - permissions: - contents: read - name: Preview membership changes - runs-on: ubuntu-latest - outputs: - affected-users-json: ${{ steps.preview.outputs.affected-users-json }} - env: - TF_WORKSPACE: ${{ github.event.inputs.organization }} - steps: - - name: Checkout - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - name: Install pnpm - uses: pnpm/action-setup@91ab88e2619ed1f46221f0ba42d1492c02baf788 # v6.0.6 - with: - version: 10 - - name: Use Node.js lts/* - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 - with: - node-version: lts/* - cache: '' - - name: Initialize scripts - run: pnpm install --frozen-lockfile && pnpm run build - working-directory: scripts - - name: Preview membership changes - id: preview - run: node lib/actions/finalize-membership-changes.js - working-directory: scripts - env: - MODE: ${{ github.event.inputs.mode }} - IGNORE: ${{ github.event.inputs.ignore }} - ONLY: ${{ github.event.inputs.only }} - APPLY: 'false' - - apply: - needs: [preview] - if: needs.preview.outputs.affected-users-json != '[]' - permissions: - contents: read - name: Apply membership changes - runs-on: ubuntu-latest - environment: membership-write - env: - GITHUB_APP_ID: ${{ secrets.RW_GITHUB_APP_ID }} - GITHUB_APP_INSTALLATION_ID: ${{ secrets[format('RW_GITHUB_APP_INSTALLATION_ID_{0}', github.event.inputs.organization)] || secrets.RW_GITHUB_APP_INSTALLATION_ID }} - GITHUB_APP_PEM_FILE: ${{ secrets.RW_GITHUB_APP_PEM_FILE }} - TF_WORKSPACE: ${{ github.event.inputs.organization }} - steps: - - name: Checkout - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - name: Install pnpm - uses: pnpm/action-setup@91ab88e2619ed1f46221f0ba42d1492c02baf788 # v6.0.6 - with: - version: 10 - - name: Use Node.js lts/* - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 - with: - node-version: lts/* - cache: '' - - name: Initialize scripts - run: pnpm install --frozen-lockfile && pnpm run build - working-directory: scripts - - name: Apply membership changes - run: node lib/actions/finalize-membership-changes.js - working-directory: scripts - env: - MODE: ${{ github.event.inputs.mode }} - IGNORE: ${{ github.event.inputs.ignore }} - ONLY: ${{ github.event.inputs.only }} - APPLY: 'true' diff --git a/.github/workflows/update-inactive-members.yml b/.github/workflows/update-members.yml similarity index 91% rename from .github/workflows/update-inactive-members.yml rename to .github/workflows/update-members.yml index 678e402..5eb34ca 100644 --- a/.github/workflows/update-inactive-members.yml +++ b/.github/workflows/update-members.yml @@ -1,4 +1,4 @@ -name: Update Inactive Members +name: Update Members on: workflow_dispatch: @@ -36,7 +36,7 @@ jobs: permissions: contents: write pull-requests: write - name: Update inactive members + name: Update members runs-on: ubuntu-latest environment: push env: @@ -55,13 +55,13 @@ jobs: uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: lts/* - cache: '' + cache: "" - name: Initialize scripts run: pnpm install --frozen-lockfile && pnpm run build working-directory: scripts - - name: Update inactive members + - name: Update members id: update - run: node lib/actions/update-inactive-members.js + run: node lib/actions/update-members.js working-directory: scripts env: CUTOFF_DATE: ${{ github.event.inputs['cutoff-date'] }} @@ -93,7 +93,7 @@ jobs: AFFECTED_USERS: ${{ steps.update.outputs.affected-users }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - branch="inactive-members-${ORGANIZATION}-${GITHUB_RUN_ID}" + branch="update-members-${ORGANIZATION}-${GITHUB_RUN_ID}" body="$(mktemp)" { echo 'The changes in this PR were made by a bot. Please review carefully.' @@ -109,11 +109,11 @@ jobs: git checkout -B "${branch}" git add "github/${ORGANIZATION}.yml" - git commit -m "update-inactive-members@${GITHUB_RUN_ID} ${ORGANIZATION}" + git commit -m "update-members@${GITHUB_RUN_ID} ${ORGANIZATION}" git push origin "${branch}" --force gh pr create \ --draft \ - --title "Update inactive members for ${ORGANIZATION}" \ + --title "Update members for ${ORGANIZATION}" \ --body-file "${body}" \ --head "${branch}" \ --base "${GITHUB_REF_NAME}" diff --git a/docs/SETUP.md b/docs/SETUP.md index 03bedc6..fdca08a 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -110,14 +110,13 @@ - `Pull requests`: `Read & Write` - `Workflows`: `Read & Write` - `Organization permissions` - - `Members`: `Read & Write` (required for Terraform membership management and the `Finalize Membership Changes` workflow) + - `Members`: `Read & Write` - [ ] [Install the GitHub Apps](https://docs.github.com/en/developers/apps/managing-github-apps/installing-github-apps) in the GitHub organization for `All repositories` ## GitHub Actions Environments and Secrets -- [ ] Create GitHub Actions environments named `read`, `write`, `push`, and `membership-write`, and configure protection rules such as required reviewers. Workflows that read organization state reference `read`; workflows that write organization state reference `write`; workflows that push generated changes to the GitHub Management repository reference `push`; workflows that directly convert or remove organization members through the GitHub API reference `membership-write`. -- [ ] Configure the `membership-write` environment with required reviewers. Treat approval of this environment as approval to call the GitHub API for every user listed by the `Finalize Membership Changes` workflow preview job. +- [ ] Create GitHub Actions environments named `read`, `write`, and `push`, and configure protection rules such as required reviewers. Workflows that read organization state reference `read`; workflows that write organization state reference `write`; workflows that push generated changes to the GitHub Management repository reference `push`. - [ ] [Create encrypted secrets](https://docs.github.com/en/actions/security-guides/encrypted-secrets#creating-encrypted-secrets-for-an-organization) for the GitHub organization and allow the repository to access them (\*replace `$GITHUB_ORGANIZATION_NAME` with the GitHub organization name) - *these secrets are read by the GitHub Action workflows* - [ ] Go to `https://github.com/organizations/$GITHUB_ORGANIZATION_NAME/settings/apps/$GITHUB_APP_NAME` and copy the `App ID` - [ ] `RO_GITHUB_APP_ID` @@ -151,12 +150,10 @@ - [ ] Follow [How to synchronize GitHub Management with GitHub?](HOWTOS.md#synchronize-github-management-with-github) to commit the terraform lock and initialize terraform state -## Inactive Member Workflows +## Member Update Workflows -- [ ] Use `Update Inactive Members` to create a draft PR that removes selected members from teams and repository collaborators in `github/$ORGANIZATION_NAME.yml`. The workflow requires either `cutoff-date` or `only`, supports `ignore` and `limit`, and can retain effective public repository access by converting that access to direct public repository collaborators in the YAML config. +- [ ] Use `Update Members` to create a draft PR that removes selected members from teams and repository collaborators in `github/$ORGANIZATION_NAME.yml`. The workflow requires either `cutoff-date` or `only`, supports `ignore` and `limit`, and can retain effective public repository access by converting that access to direct public repository collaborators in the YAML config. - [ ] Review and merge the draft PR through the normal GitHub Management PR flow. -- [ ] Use `Finalize Membership Changes` only after the YAML PR has landed and the preview job lists the expected users. The `membership-write` environment approval gates API calls that convert potential outside collaborators or remove potential no members. -- [ ] After `Finalize Membership Changes` completes, run `Sync` for the same organization so Terraform state and YAML config reflect the membership changes made through the GitHub API. ## GitHub Management Repository Protections diff --git a/scripts/__tests__/actions/access-summary.test.ts b/scripts/__tests__/actions/access-summary.test.ts index a59aa6e..14e5854 100644 --- a/scripts/__tests__/actions/access-summary.test.ts +++ b/scripts/__tests__/actions/access-summary.test.ts @@ -34,20 +34,34 @@ repositories: pull: - alice visibility: public + team-only-repo: + teams: + push: + - guests + visibility: public team-repo: teams: push: - maintainers visibility: public teams: + guests: + members: + member: + - team-only-non-member maintainers: members: member: - dave `) - const categories = categorizeAccessSummary(getAccessSummaryFrom(config)) + const summary = getAccessSummaryFrom(config) + const categories = categorizeAccessSummary(summary) + assert.equal(summary.outside.isMember, false) + assert.equal(summary.outside.isOutsideCollaborator, true) + assert.equal(summary['team-only-non-member'].isMember, false) + assert.equal(summary['team-only-non-member'].isOutsideCollaborator, false) assert.deepEqual(categories.outsideCollaborators, ['outside']) assert.deepEqual(categories.potentialOutsideCollaborators, ['alice']) assert.deepEqual(categories.potentialNoMembers, ['carol']) @@ -118,6 +132,7 @@ repositories: ) assert.match(report, /Potential outside collaborators<\/summary>/) assert.match(report, /Affected users: alice/) + assert.match(report, /User alice \(member\):/) assert.match(report, /has push permission to public-repo \(public\)/) }) }) diff --git a/scripts/__tests__/actions/finalize-membership-changes.test.ts b/scripts/__tests__/actions/finalize-membership-changes.test.ts deleted file mode 100644 index 5ac2819..0000000 --- a/scripts/__tests__/actions/finalize-membership-changes.test.ts +++ /dev/null @@ -1,43 +0,0 @@ -import 'reflect-metadata' - -import assert from 'node:assert' -import {describe, it} from 'node:test' -import {Config} from '../../src/yaml/config.js' -import { - parseFinalizeMembershipMode, - planFinalizeMembershipChanges -} from '../../src/actions/finalize-membership-changes.js' - -describe('finalize membership changes', () => { - it('validates mode input', () => { - assert.equal(parseFinalizeMembershipMode('both'), 'both') - assert.throws(() => parseFinalizeMembershipMode('invalid')) - }) - - it('plans filtered conversion and removal targets', () => { - const config = new Config(` -members: - member: - - outside-candidate - - no-member-candidate - - ignored -repositories: - public-repo: - collaborators: - pull: - - outside-candidate - - ignored - visibility: public -`) - - const plan = planFinalizeMembershipChanges( - config, - 'both', - ['ignored'], - ['outside-candidate', 'no-member-candidate', 'ignored'] - ) - - assert.deepEqual(plan.potentialOutsideCollaborators, ['outside-candidate']) - assert.deepEqual(plan.potentialNoMembers, ['no-member-candidate']) - }) -}) diff --git a/scripts/__tests__/actions/update-inactive-members.test.ts b/scripts/__tests__/actions/update-members.test.ts similarity index 90% rename from scripts/__tests__/actions/update-inactive-members.test.ts rename to scripts/__tests__/actions/update-members.test.ts index a954997..e2aeef3 100644 --- a/scripts/__tests__/actions/update-inactive-members.test.ts +++ b/scripts/__tests__/actions/update-members.test.ts @@ -6,18 +6,18 @@ import {Config} from '../../src/yaml/config.js' import { parseCutoffDate, parseLimit, - selectInactiveMembers, - updateInactiveMembersConfig -} from '../../src/actions/update-inactive-members.js' + selectMembersForUpdate, + updateMembersConfig +} from '../../src/actions/update-members.js' import {TeamMember} from '../../src/resources/team-member.js' import {RepositoryCollaborator} from '../../src/resources/repository-collaborator.js' -describe('update inactive members', () => { +describe('update members', () => { it('requires cutoff date or only list', () => { const config = new Config('members:\n member:\n - alice\n') assert.throws(() => - selectInactiveMembers(config, [], { + selectMembersForUpdate(config, [], { ignore: [], only: [], publicRepoAccess: 'retain' @@ -47,7 +47,7 @@ members: - old `) - const selected = selectInactiveMembers( + const selected = selectMembersForUpdate( config, [ {username: 'active', latestActivity: new Date('2025-01-01T00:00:00Z')}, @@ -94,7 +94,7 @@ teams: - alice `) - updateInactiveMembersConfig(config, ['alice'], 'retain') + updateMembersConfig(config, ['alice'], 'retain') assert.equal( config @@ -145,7 +145,7 @@ teams: - alice `) - updateInactiveMembersConfig(config, ['alice'], 'remove') + updateMembersConfig(config, ['alice'], 'remove') assert.equal( config diff --git a/scripts/src/actions/finalize-membership-changes.ts b/scripts/src/actions/finalize-membership-changes.ts deleted file mode 100644 index b86bcd6..0000000 --- a/scripts/src/actions/finalize-membership-changes.ts +++ /dev/null @@ -1,131 +0,0 @@ -import 'reflect-metadata' -import * as core from '@actions/core' -import {pathToFileURL} from 'url' -import {Config} from '../yaml/config.js' -import {GitHub} from '../github.js' -import { - categorizeAccessSummary, - getAccessSummaryFrom, - parseUserList -} from './shared/access-summary.js' - -export type FinalizeMembershipMode = - | 'convert-potential-outside-collaborators' - | 'remove-potential-no-members' - | 'both' - -export type FinalizeMembershipPlan = { - potentialOutsideCollaborators: string[] - potentialNoMembers: string[] -} - -export function parseFinalizeMembershipMode( - source?: string -): FinalizeMembershipMode { - if ( - source === 'convert-potential-outside-collaborators' || - source === 'remove-potential-no-members' || - source === 'both' - ) { - return source - } - throw new Error( - 'mode must be convert-potential-outside-collaborators, remove-potential-no-members, or both' - ) -} - -export function planFinalizeMembershipChanges( - config: Config, - mode: FinalizeMembershipMode, - ignore: string[], - only: string[] -): FinalizeMembershipPlan { - const ignoredUsers = new Set(ignore) - const onlyUsers = new Set(only) - const categories = categorizeAccessSummary(getAccessSummaryFrom(config)) - - const filter = (users: string[]): string[] => - users - .filter(username => !ignoredUsers.has(username)) - .filter(username => onlyUsers.size === 0 || onlyUsers.has(username)) - .sort() - - return { - potentialOutsideCollaborators: - mode === 'remove-potential-no-members' - ? [] - : filter(categories.potentialOutsideCollaborators), - potentialNoMembers: - mode === 'convert-potential-outside-collaborators' - ? [] - : filter(categories.potentialNoMembers) - } -} - -export function formatFinalizeMembershipPlan( - plan: FinalizeMembershipPlan -): string { - return [ - 'Potential outside collaborators to convert:', - plan.potentialOutsideCollaborators.length > 0 - ? plan.potentialOutsideCollaborators - .map(username => `- ${username}`) - .join('\n') - : '- none', - '', - 'Potential no members to remove:', - plan.potentialNoMembers.length > 0 - ? plan.potentialNoMembers.map(username => `- ${username}`).join('\n') - : '- none' - ].join('\n') -} - -async function run(): Promise { - const mode = parseFinalizeMembershipMode(process.env.MODE || 'both') - const ignore = parseUserList(process.env.IGNORE) - const only = parseUserList(process.env.ONLY) - const shouldApply = process.env.APPLY === 'true' - const config = Config.FromPath() - - const plan = planFinalizeMembershipChanges(config, mode, ignore, only) - const affectedUsers = Array.from( - new Set([...plan.potentialOutsideCollaborators, ...plan.potentialNoMembers]) - ).sort() - - core.info(formatFinalizeMembershipPlan(plan)) - core.setOutput('affected-users', affectedUsers.join(', ')) - core.setOutput('affected-users-json', JSON.stringify(affectedUsers)) - core.setOutput( - 'potential-outside-collaborators-json', - JSON.stringify(plan.potentialOutsideCollaborators) - ) - core.setOutput( - 'potential-no-members-json', - JSON.stringify(plan.potentialNoMembers) - ) - - if (!shouldApply) { - return - } - - const github = await GitHub.getGitHub() - - for (const username of plan.potentialOutsideCollaborators) { - await github.convertMemberToOutsideCollaborator(username) - } - - for (const username of plan.potentialNoMembers) { - await github.removeOrganizationMembership(username) - } - - core.notice( - 'Membership changes are complete. Run the Sync workflow for this organization so Terraform state and YAML config reflect GitHub.' - ) -} - -if ( - process.argv[1] && - import.meta.url === pathToFileURL(process.argv[1]).href -) { - run() -} diff --git a/scripts/src/actions/shared/access-summary.ts b/scripts/src/actions/shared/access-summary.ts index 5c5c112..9561a38 100644 --- a/scripts/src/actions/shared/access-summary.ts +++ b/scripts/src/actions/shared/access-summary.ts @@ -13,6 +13,8 @@ export type RepositoryAccess = { export type UserAccess = { role?: string + isMember: boolean + isOutsideCollaborator: boolean repositories: Record directRepositories: Record teams: string[] @@ -156,13 +158,15 @@ export function getAccessSummaryFrom(source: State | Config): AccessSummary { ? hasKeepComment(source, member) : false - if ( - role !== undefined || - teams.length > 0 || - Object.keys(repositories).length > 0 - ) { + const isMember = role !== undefined + const isOutsideCollaborator = + !isMember && Object.keys(directRepositories).length > 0 + + if (isMember || isOutsideCollaborator || teams.length > 0) { accessSummary[username] = { role, + isMember, + isOutsideCollaborator, repositories, directRepositories, teams, @@ -209,11 +213,11 @@ export function categorizeAccessSummary( for (const [username, access] of Object.entries(summary)) { const repositories = Object.values(access.repositories) - if (access.role === undefined) { - if (repositories.length > 0) { - categories.outsideCollaborators.push(username) - } + const directRepositories = Object.values(access.directRepositories) + if (access.isOutsideCollaborator) { + categories.outsideCollaborators.push(username) } else if ( + access.isMember && !access.hasKeepComment && access.teams.length === 0 && repositories.length > 0 && @@ -222,9 +226,14 @@ export function categorizeAccessSummary( ) ) { categories.potentialOutsideCollaborators.push(username) - } else if (!access.hasKeepComment && repositories.length === 0) { + } else if ( + access.isMember && + !access.hasKeepComment && + directRepositories.length === 0 && + access.teams.length === 0 + ) { categories.potentialNoMembers.push(username) - } else { + } else if (access.isMember) { categories.anyOtherMembers.push(username) } } @@ -262,7 +271,10 @@ export function formatAccessSummarySection( for (const username of users) { const access = summary[username] - lines.push(`User ${username}:`) + const kind = access.isOutsideCollaborator + ? 'outside collaborator' + : 'member' + lines.push(`User ${username} (${kind}):`) const repositories = Object.entries(access.repositories) if (repositories.length === 0) { lines.push(' - has no repository access') diff --git a/scripts/src/actions/update-inactive-members.ts b/scripts/src/actions/update-members.ts similarity index 96% rename from scripts/src/actions/update-inactive-members.ts rename to scripts/src/actions/update-members.ts index f8bfdc2..25d649c 100644 --- a/scripts/src/actions/update-inactive-members.ts +++ b/scripts/src/actions/update-members.ts @@ -24,7 +24,7 @@ export type MemberActivity = { latestActivity?: Date } -export type UpdateInactiveMembersOptions = { +export type UpdateMembersOptions = { cutoffDate?: Date limit?: number ignore: string[] @@ -69,10 +69,10 @@ export function parsePublicRepoAccess(source?: string): PublicRepoAccess { throw new Error('public-repo-access must be retain or remove') } -export function selectInactiveMembers( +export function selectMembersForUpdate( config: Config, activities: MemberActivity[], - options: UpdateInactiveMembersOptions + options: UpdateMembersOptions ): string[] { if (options.cutoffDate === undefined && options.only.length === 0) { throw new Error('Either cutoff-date or only must be provided') @@ -117,7 +117,7 @@ export function selectInactiveMembers( : candidates.slice(0, options.limit) } -export function updateInactiveMembersConfig( +export function updateMembersConfig( config: Config, usernames: string[], publicRepoAccess: PublicRepoAccess @@ -275,14 +275,14 @@ async function run(): Promise { cutoffDate === undefined ? [] : await collectActivities(limit === undefined ? cutoffDate : new Date(0)) - const selectedMembers = selectInactiveMembers(config, activities, { + const selectedMembers = selectMembersForUpdate(config, activities, { cutoffDate, limit, ignore, only, publicRepoAccess }) - const affectedUsers = updateInactiveMembersConfig( + const affectedUsers = updateMembersConfig( config, selectedMembers, publicRepoAccess diff --git a/scripts/src/github.ts b/scripts/src/github.ts index 5fac482..882e477 100644 --- a/scripts/src/github.ts +++ b/scripts/src/github.ts @@ -587,30 +587,4 @@ export class GitHub { } return commitComments } - - async convertMemberToOutsideCollaborator(username: string): Promise { - core.info(`Converting ${username} to outside collaborator...`) - await this.client.request( - 'PUT /orgs/{org}/outside_collaborators/{username}', - { - org: env.GITHUB_ORG, - username, - async: false, - headers: { - 'X-GitHub-Api-Version': '2026-03-10' - } - } - ) - } - - async removeOrganizationMembership(username: string): Promise { - core.info(`Removing organization membership for ${username}...`) - await this.client.request('DELETE /orgs/{org}/memberships/{username}', { - org: env.GITHUB_ORG, - username, - headers: { - 'X-GitHub-Api-Version': '2026-03-10' - } - }) - } } From 34f200fbea77d191b001d5911949a9455f64e539 Mon Sep 17 00:00:00 2001 From: Piotr Galar Date: Thu, 16 Jul 2026 19:34:53 +0100 Subject: [PATCH 3/3] Make org membership updates configurable --- .github/workflows/update-members.yml | 11 ++++++ docs/SETUP.md | 2 +- .../__tests__/actions/update-members.test.ts | 34 ++++++++++++++++--- scripts/src/actions/update-members.ts | 32 +++++++++++++++-- 4 files changed, 71 insertions(+), 8 deletions(-) diff --git a/.github/workflows/update-members.yml b/.github/workflows/update-members.yml index 5eb34ca..6d9165a 100644 --- a/.github/workflows/update-members.yml +++ b/.github/workflows/update-members.yml @@ -26,6 +26,14 @@ on: options: - retain - remove + organization-membership: + description: Whether selected users remain organization members + required: true + default: keep + type: choice + options: + - keep + - remove defaults: run: @@ -69,6 +77,7 @@ jobs: IGNORE: ${{ github.event.inputs.ignore }} ONLY: ${{ github.event.inputs.only }} PUBLIC_REPO_ACCESS: ${{ github.event.inputs['public-repo-access'] }} + ORGANIZATION_MEMBERSHIP: ${{ github.event.inputs['organization-membership'] }} - name: Check if organization config was modified id: config-modified env: @@ -90,6 +99,7 @@ jobs: IGNORE: ${{ github.event.inputs.ignore }} ONLY: ${{ github.event.inputs.only }} PUBLIC_REPO_ACCESS: ${{ github.event.inputs['public-repo-access'] }} + ORGANIZATION_MEMBERSHIP: ${{ github.event.inputs['organization-membership'] }} AFFECTED_USERS: ${{ steps.update.outputs.affected-users }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | @@ -104,6 +114,7 @@ jobs: echo "Ignore: ${IGNORE:-not set}" echo "Only: ${ONLY:-not set}" echo "Public repo access: ${PUBLIC_REPO_ACCESS}" + echo "Organization membership: ${ORGANIZATION_MEMBERSHIP}" echo "Affected users: ${AFFECTED_USERS:-none}" } > "${body}" diff --git a/docs/SETUP.md b/docs/SETUP.md index fdca08a..0fdeb8f 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -152,7 +152,7 @@ ## Member Update Workflows -- [ ] Use `Update Members` to create a draft PR that removes selected members from teams and repository collaborators in `github/$ORGANIZATION_NAME.yml`. The workflow requires either `cutoff-date` or `only`, supports `ignore` and `limit`, and can retain effective public repository access by converting that access to direct public repository collaborators in the YAML config. +- [ ] Use `Update Members` to create a draft PR that removes selected members from teams and repository collaborators in `github/$ORGANIZATION_NAME.yml`. The workflow requires either `cutoff-date` or `only`, supports `ignore` and `limit`, can retain effective public repository access by converting that access to direct public repository collaborators, and can optionally remove selected users from organization membership in the YAML config. - [ ] Review and merge the draft PR through the normal GitHub Management PR flow. ## GitHub Management Repository Protections diff --git a/scripts/__tests__/actions/update-members.test.ts b/scripts/__tests__/actions/update-members.test.ts index e2aeef3..b115ef7 100644 --- a/scripts/__tests__/actions/update-members.test.ts +++ b/scripts/__tests__/actions/update-members.test.ts @@ -6,9 +6,11 @@ import {Config} from '../../src/yaml/config.js' import { parseCutoffDate, parseLimit, + parseOrganizationMembership, selectMembersForUpdate, updateMembersConfig } from '../../src/actions/update-members.js' +import {Member} from '../../src/resources/member.js' import {TeamMember} from '../../src/resources/team-member.js' import {RepositoryCollaborator} from '../../src/resources/repository-collaborator.js' @@ -20,7 +22,8 @@ describe('update members', () => { selectMembersForUpdate(config, [], { ignore: [], only: [], - publicRepoAccess: 'retain' + publicRepoAccess: 'retain', + organizationMembership: 'keep' }) ) }) @@ -31,8 +34,11 @@ describe('update members', () => { '2025-01-02T00:00:00.000Z' ) assert.equal(parseLimit('2'), 2) + assert.equal(parseOrganizationMembership('keep'), 'keep') + assert.equal(parseOrganizationMembership('remove'), 'remove') assert.throws(() => parseCutoffDate('01-02-2025')) assert.throws(() => parseLimit('0')) + assert.throws(() => parseOrganizationMembership('invalid')) }) it('selects inactive members with only, ignore, limit, and KEEP handling', () => { @@ -58,7 +64,8 @@ members: limit: 2, ignore: ['ignored'], only: ['active', 'ignored', 'kept', 'manual', 'never-active', 'old'], - publicRepoAccess: 'retain' + publicRepoAccess: 'retain', + organizationMembership: 'keep' } ) @@ -94,7 +101,7 @@ teams: - alice `) - updateMembersConfig(config, ['alice'], 'retain') + updateMembersConfig(config, ['alice'], 'retain', 'keep') assert.equal( config @@ -145,7 +152,7 @@ teams: - alice `) - updateMembersConfig(config, ['alice'], 'remove') + updateMembersConfig(config, ['alice'], 'remove', 'keep') assert.equal( config @@ -154,4 +161,23 @@ teams: false ) }) + + it('removes organization membership when configured', () => { + const config = new Config(` +members: + member: + - alice + - bob +repositories: + public-repo: + visibility: public +`) + + updateMembersConfig(config, ['alice'], 'remove', 'remove') + + assert.deepEqual( + config.getResources(Member).map(member => member.username), + ['bob'] + ) + }) }) diff --git a/scripts/src/actions/update-members.ts b/scripts/src/actions/update-members.ts index 25d649c..77613a4 100644 --- a/scripts/src/actions/update-members.ts +++ b/scripts/src/actions/update-members.ts @@ -18,6 +18,7 @@ import { } from './shared/access-summary.js' export type PublicRepoAccess = 'retain' | 'remove' +export type OrganizationMembership = 'keep' | 'remove' export type MemberActivity = { username: string @@ -30,6 +31,7 @@ export type UpdateMembersOptions = { ignore: string[] only: string[] publicRepoAccess: PublicRepoAccess + organizationMembership: OrganizationMembership } type ActivityRecord = { @@ -69,6 +71,15 @@ export function parsePublicRepoAccess(source?: string): PublicRepoAccess { throw new Error('public-repo-access must be retain or remove') } +export function parseOrganizationMembership( + source?: string +): OrganizationMembership { + if (source === 'keep' || source === 'remove') { + return source + } + throw new Error('organization-membership must be keep or remove') +} + export function selectMembersForUpdate( config: Config, activities: MemberActivity[], @@ -120,7 +131,8 @@ export function selectMembersForUpdate( export function updateMembersConfig( config: Config, usernames: string[], - publicRepoAccess: PublicRepoAccess + publicRepoAccess: PublicRepoAccess, + organizationMembership: OrganizationMembership ): string[] { const targets = new Set(usernames.map(username => username.toLowerCase())) const repositories = new Map( @@ -194,6 +206,15 @@ export function updateMembersConfig( } } + if (organizationMembership === 'remove') { + for (const member of config.getResources(Member)) { + if (targets.has(member.username.toLowerCase())) { + core.info(`Removing ${member.username} from the organization`) + config.removeResource(member) + } + } + } + return Array.from(targets).sort() } @@ -269,6 +290,9 @@ async function run(): Promise { const ignore = parseUserList(process.env.IGNORE) const only = parseUserList(process.env.ONLY) const publicRepoAccess = parsePublicRepoAccess(process.env.PUBLIC_REPO_ACCESS) + const organizationMembership = parseOrganizationMembership( + process.env.ORGANIZATION_MEMBERSHIP || 'keep' + ) const config = Config.FromPath() const activities = @@ -280,12 +304,14 @@ async function run(): Promise { limit, ignore, only, - publicRepoAccess + publicRepoAccess, + organizationMembership }) const affectedUsers = updateMembersConfig( config, selectedMembers, - publicRepoAccess + publicRepoAccess, + organizationMembership ) config.save()