upgrade@31305715701

.github/workflows/access-report.yml

@@ -0,0 +1,63 @@
+name: Access Report
+
+on:
+  workflow_dispatch:
+    inputs:
+      organization:
+        description: Organization config to report on
+        required: true
+
+defaults:
+  run:
+    shell: bash
+
+jobs:
+  report:
+    permissions:
+      contents: read
+    name: Access report
+    runs-on: ubuntu-latest
+    environment: read
+    env:
+      TF_IN_AUTOMATION: 1
+      TF_INPUT: 0
+      TF_WORKSPACE: ${{ github.event.inputs.organization }}
+      AWS_ACCESS_KEY_ID: ${{ secrets.RO_AWS_ACCESS_KEY_ID }}
+      AWS_SECRET_ACCESS_KEY: ${{ secrets.RO_AWS_SECRET_ACCESS_KEY }}
+    steps:
+      - name: Checkout
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+      - name: Setup terraform
+        uses: hashicorp/setup-terraform@5e8dbf3c6d9deaf4193ca7a8fb23f2ac83bb6c85 # v4.0.0
+        with:
+          terraform_version: 1.12.0
+          terraform_wrapper: false
+      - name: Initialize terraform
+        run: terraform init
+        working-directory: terraform
+      - 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: Generate access report
+        run: node lib/actions/access-report.js
+        working-directory: scripts
+        env:
+          ACCESS_REPORT_PATH: ../ACCESS_REPORT.md
+      - name: Publish access report summary
+        run: cat ACCESS_REPORT.md >> "$GITHUB_STEP_SUMMARY"
+      - name: Upload access report
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+        with:
+          name: access-report-${{ env.TF_WORKSPACE }}
+          path: ACCESS_REPORT.md
+          if-no-files-found: error
+          retention-days: 14

.github/workflows/apply.yml

@@ -132,7 +132,14 @@ jobs:
         run: terraform init
       - name: Allow destroy in guarded environment
         if: matrix.environment == 'write-allow-destroy'
-        run: cp allow_destroy_override.tf.disabled allow_destroy_override.tf
+        env:
+          ALLOW_DESTROY: ${{ vars.ALLOW_DESTROY }}
+        run: |
+          if [[ "${ALLOW_DESTROY}" != "true" ]]; then
+            echo "The write-allow-destroy environment must define ALLOW_DESTROY=true."
+            exit 1
+          fi
+          cp allow_destroy_override.tf.disabled allow_destroy_override.tf
       - name: Download reviewed terraform plan
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/fix.yml

@@ -117,6 +117,19 @@ jobs:
         id: fix
         run: node lib/actions/fix-yaml-config.js
         working-directory: scripts
+        env:
+          ACCESS_REPORT_PATH: ../ACCESS_REPORT.md
+      - name: Publish access report summary
+        if: always() && hashFiles('ACCESS_REPORT.md') != ''
+        run: cat ACCESS_REPORT.md >> "$GITHUB_STEP_SUMMARY"
+      - name: Upload access report
+        if: always() && hashFiles('ACCESS_REPORT.md') != ''
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+        with:
+          name: access-report-${{ env.TF_WORKSPACE }}
+          path: ACCESS_REPORT.md
+          if-no-files-found: error
+          retention-days: 14
       - name: Upload YAML config
         uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
         with:

.github/workflows/plan.yml

@@ -141,7 +141,14 @@ jobs:
         working-directory: terraform
       - name: Allow destroy in guarded environment
         if: matrix.environment == 'read-allow-destroy'
-        run: cp allow_destroy_override.tf.disabled allow_destroy_override.tf
+        env:
+          ALLOW_DESTROY: ${{ vars.ALLOW_DESTROY }}
+        run: |
+          if [[ "${ALLOW_DESTROY}" != "true" ]]; then
+            echo "The read-allow-destroy environment must define ALLOW_DESTROY=true."
+            exit 1
+          fi
+          cp allow_destroy_override.tf.disabled allow_destroy_override.tf
         working-directory: terraform
       - name: Plan terraform
         run: |

CHANGELOG.md

@@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 - workflows: added separate GitHub Actions environments for reading organization state, writing organization state, and pushing repository changes
+- **BREAKING**: access changes action now emits only the access change comment by default; update custom usage to avoid nesting the full access breakdown in PR comments
 - workflows: pin third-party actions to latest release SHAs and replan from the merged commit before applying
 - docs: update template repository references from `github-mgmt-template` to `github-as-code`
 - scripts: update dependencies with security advisories

docs/SETUP.md

@@ -117,7 +117,7 @@
 ## GitHub Actions Environments and Secrets
 
 - [ ] Create GitHub Actions environments named `read`, `read-allow-destroy`, `write`, `write-allow-destroy`, 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`.
-- [ ] Configure `read-allow-destroy` and `write-allow-destroy` with stricter protection rules for repository and membership deletion plans/applies:
+- [ ] Configure `read-allow-destroy` and `write-allow-destroy` with stricter protection rules for repository and membership deletion plans/applies, and set an environment variable named `ALLOW_DESTROY` to `true` in each environment:
     - [ ] Require reviewers
     - [ ] Prevent self-review
     - [ ] Restrict deployment branches to `master`

scripts/__tests__/actions/access-summary.test.ts

@@ -10,6 +10,7 @@ import {
 } from '../../src/actions/shared/access-summary.js'
 import {
   describeAccessChanges,
+  describeAccessChangesComment,
   describeAccessReport
 } from '../../src/actions/shared/describe-access-changes.js'
 import {StateSchema} from '../../src/terraform/schema.js'
@@ -22,6 +23,7 @@ members:
     - alice
     - carol
     - dave
+    - frank
     - kept # KEEP: manual exception
 repositories:
   private-repo:
@@ -45,6 +47,10 @@ repositories:
         - maintainers
     visibility: public
 teams:
+  empty:
+    members:
+      member:
+        - frank
   guests:
     members:
       member:
@@ -64,11 +70,11 @@ teams:
     assert.equal(summary['team-only-non-member'].isOutsideCollaborator, false)
     assert.deepEqual(categories.outsideCollaborators, ['outside'])
     assert.deepEqual(categories.potentialOutsideCollaborators, ['alice'])
-    assert.deepEqual(categories.potentialNoMembers, ['carol'])
+    assert.deepEqual(categories.potentialNoMembers, ['carol', 'frank'])
     assert.deepEqual(categories.anyOtherMembers, ['dave', 'kept'])
   })
 
-  it('annotates repository visibility in access changes and summaries', () => {
+  it('annotates repository visibility and access path in access changes and summaries', () => {
     const state = new State(
       JSON.stringify({
         values: {
@@ -128,11 +134,103 @@ repositories:
 
     assert.match(
       changes,
-      /will have the permission to public-repo \(public\) change from pull to push/
+      /will change from having direct pull permission to public-repo \(public\) to having direct push permission to public-repo \(public\)/
     )
     assert.match(report, /<summary>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\)/)
+    assert.match(report, /has direct push permission to public-repo \(public\)/)
+  })
+
+  it('describes team and mixed repository access paths', () => {
+    const state = new State(
+      JSON.stringify({values: {root_module: {resources: []}}})
+    )
+    const config = new Config(`
+members:
+  member:
+    - alice
+    - bob
+repositories:
+  private-repo:
+    collaborators:
+      pull:
+        - bob
+    teams:
+      admin:
+        - owners
+      push:
+        - maintainers
+    visibility: private
+teams:
+  maintainers:
+    members:
+      member:
+        - alice
+  owners:
+    members:
+      member:
+        - bob
+`)
+
+    const changes = describeAccessChanges(state, config)
+    const report = describeAccessReport(state, config)
+
+    assert.match(
+      changes,
+      /will gain push permission to private-repo \(private\) through team maintainers/
+    )
+    assert.match(
+      changes,
+      /will gain effective admin permission to private-repo \(private\) through direct pull permission and team owners/
+    )
+    assert.match(
+      report,
+      /has push permission to private-repo \(private\) through team maintainers/
+    )
+    assert.match(
+      report,
+      /has effective admin permission to private-repo \(private\) through direct pull permission and team owners/
+    )
+  })
+
+  it('keeps routine comments to access changes only', () => {
+    const state = new State(
+      JSON.stringify({values: {root_module: {resources: []}}})
+    )
+    const config = new Config(`
+members:
+  member:
+    - alice
+`)
+
+    const comment = describeAccessChangesComment(state, config)
+
+    assert.match(comment, /<summary>Access Changes<\/summary>/)
+    assert.doesNotMatch(comment, /Potential no members/)
+    assert.doesNotMatch(comment, /Any other members/)
+  })
+
+  it('falls back to workflow output when access change comments are too long', () => {
+    const state = new State(
+      JSON.stringify({values: {root_module: {resources: []}}})
+    )
+    const config = new Config(`
+members:
+  member:
+    - alice
+`)
+
+    const comment = describeAccessChangesComment(
+      state,
+      config,
+      10,
+      'https://github.example/runs/1'
+    )
+
+    assert.equal(
+      comment,
+      'Access changes are too long to post as a comment. Please inspect [the Fix workflow summary or access report artifact](https://github.example/runs/1) instead.'
+    )
   })
 })

scripts/__tests__/actions/classify-allow-destroy.test.ts

@@ -4,7 +4,8 @@ import {describe, it} from 'node:test'
 import assert from 'node:assert'
 import {
   getEnvironment,
-  hasAllowDestroyChange
+  hasAllowDestroyChange,
+  validateRemovedMembersHaveNoDanglingAccess
 } from '../../src/actions/classify-allow-destroy.js'
 import {Config} from '../../src/yaml/config.js'
 import {State} from '../../src/terraform/state.js'
@@ -178,4 +179,107 @@ repositories:
 
     assert.equal(allowDestroy, false)
   })
+
+  it('fails when removing a member who remains in a team', async () => {
+    setManagedResourceTypes(['github_membership'])
+
+    await assert.rejects(
+      validateRemovedMembersHaveNoDanglingAccess(
+        new Config(`
+teams:
+  maintainers:
+    members:
+      member:
+        - removed
+`),
+        state({
+          values: {
+            root_module: {
+              resources: [
+                {
+                  mode: 'managed',
+                  type: 'github_membership',
+                  values: {username: 'removed', role: 'member'}
+                }
+              ]
+            }
+          }
+        })
+      ),
+      /removed is still a member of team maintainers/
+    )
+  })
+
+  it('fails when removing a member who keeps direct private repository access', async () => {
+    setManagedResourceTypes(['github_membership'])
+
+    await assert.rejects(
+      validateRemovedMembersHaveNoDanglingAccess(
+        new Config(`
+repositories:
+  private-repo:
+    collaborators:
+      pull:
+        - removed
+    visibility: private
+`),
+        state({
+          values: {
+            root_module: {
+              resources: [
+                {
+                  mode: 'managed',
+                  type: 'github_membership',
+                  values: {username: 'removed', role: 'member'}
+                }
+              ]
+            }
+          }
+        })
+      ),
+      /removed still has direct access to private repository private-repo/
+    )
+  })
+
+  it('allows member removal when team and private direct access are removed too', async () => {
+    setManagedResourceTypes(['github_membership'])
+
+    await validateRemovedMembersHaveNoDanglingAccess(
+      new Config(`
+repositories:
+  public-repo:
+    collaborators:
+      pull:
+        - removed
+    visibility: public
+`),
+      state({
+        values: {
+          root_module: {
+            resources: [
+              {
+                mode: 'managed',
+                type: 'github_membership',
+                values: {username: 'removed', role: 'member'}
+              },
+              {
+                mode: 'managed',
+                type: 'github_repository',
+                values: {name: 'private-repo', visibility: 'private'}
+              },
+              {
+                mode: 'managed',
+                type: 'github_repository_collaborator',
+                values: {
+                  username: 'removed',
+                  repository: 'private-repo',
+                  permission: 'pull'
+                }
+              }
+            ]
+          }
+        }
+      })
+    )
+  })
 })