Merge pull request #4 from nix-open-org/validate-codeowners

Validate CODEOWNERS

Author: zimbatm

.github/CODEOWNERS

@@ -1,5 +1,19 @@
-# See ./org-repo.md
+# See doc/org-repo.md
+
+/README.md @infinisil @zimbatm
+/CONTRIBUTING.md @infinisil @zimbatm
+/LICENSE @infinisil @zimbatm
 
 /.github/CODEOWNERS @infinisil @zimbatm
+/.github/workflows @infinisil @zimbatm
+/scripts @infinisil @zimbatm
 
 /doc/org-repo.md @infinisil @zimbatm
+/doc/discourse.md @infinisil @zimbatm
+/doc/github.md @infinisil @zimbatm
+/doc/matrix.md @infinisil @zimbatm
+/doc/moderation.md @infinisil @zimbatm
+/doc/nixos-releases.md @infinisil @zimbatm
+/doc/resources.md @infinisil @zimbatm
+/doc/rfcs.md @infinisil @zimbatm
+/doc/teams.md @infinisil @zimbatm

.github/workflows/ci.yml

@@ -1,15 +1,67 @@
 name: CI
 on:
-  pull_request:
-    branches:
-      - main
+  # We use pull_request_target such that the code owner validation works for PRs from forks,
+  # because we need repository secrets for that, which pull_request wouldn't allow from forks.
+  # However, it's very important that we don't run code from forks without sandboxing it,
+  # because that way anybody could potentially extract repository secrets!
+  # Furthermore, using pull_request_target doesn't require manually approving first-time contributors
+  pull_request_target:
 
 jobs:
   xrefcheck:
     name: Check references
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v4
+      with:
+        ref: refs/pull/${{ github.event.pull_request.number }}/merge
+        path: untrusted-pr
+
     - uses: serokell/xrefcheck-action@v1
+      with:
+        xrefcheck-args: "--root untrusted-pr"
+
+  codeowners:
+    name: Validate codeowners
+    runs-on: ubuntu-latest
+    steps:
+    - uses: cachix/install-nix-action@v26
+
+    - uses: actions/checkout@v4
+      with:
+        path: trusted-base
+
+    - uses: actions/checkout@v4
+      with:
+        ref: refs/pull/${{ github.event.pull_request.number }}/merge
+        path: untrusted-pr
+
+    - uses: mszostok/[email protected]
+      with:
+        # GitHub access token is required only if the `owners` check is enabled
+        # See https://github.com/mszostok/codeowners-validator/blob/main/docs/gh-auth.md#public-repositories
+        github_access_token: "${{ secrets.OWNERS_VALIDATOR_GITHUB_SECRET }}"
+
+        # The repository path in which CODEOWNERS file should be validated."
+        repository_path: untrusted-pr
+
+        # The owner and repository name. For example, gh-codeowners/codeowners-samples. Used to check if GitHub team is in the given organization and has permission to the given repository."
+        owner_checker_repository: "${{ github.repository }}"
+
+        # "The comma-separated list of experimental checks that should be executed. By default, all experimental checks are turned off. Possible values: notowned,avoid-shadowing"
+        experimental_checks: "notowned,avoid-shadowing"
+
+        # Specifies whether CODEOWNERS may have unowned files. For example, `/infra/oncall-rotator/oncall-config.yml` doesn't have owner and this is not reported.
+        owner_checker_allow_unowned_patterns: "false"
+
+        # Specifies whether only teams are allowed as owners of files.
+        owner_checker_owners_must_be_teams: "false"
 
-# TODO: Use https://github.com/marketplace/actions/github-codeowners-validator
+    # The above validator doesn't currently ensure that people have write access: https://github.com/mszostok/codeowners-validator/issues/157
+    # So we're doing it manually instead
+    - name: Check that codeowners have write access
+      # Important that we run the script from the base branch,
+      # because otherwise a PR from a fork could change it to extract the secret
+      run: trusted-base/scripts/unprivileged-owners.sh untrusted-pr ${{ github.repository }}
+      env:
+        GH_TOKEN: "${{ secrets.OWNERS_VALIDATOR_GITHUB_SECRET }}"

.github/workflows/review.yml

@@ -11,14 +11,26 @@ jobs:
   update:
     runs-on: ubuntu-latest
     steps:
+      - uses: cachix/install-nix-action@v26
+
       - uses: actions/checkout@v4
+        with:
+          path: repo
+
+      - name: Generate issue body
+        run: repo/scripts/review-body.sh repo ${{ github.repository }} > body
+        env:
+          # This token has read-only admin access to see who has write access to this repo
+          GH_TOKEN: "${{ secrets.OWNERS_VALIDATOR_GITHUB_SECRET }}"
+
       - run: |
           gh api \
             --method POST \
             -H "Accept: application/vnd.github+json" \
             -H "X-GitHub-Api-Version: 2022-11-28" \
-            /repos/"$GITHUB_REPOSITORY"/issues \
+            /repos/${{ github.repository }}/issues \
             -f title="[$(date +'%Y %B')] Regular manual review " \
-            -f body="$(./review-body.sh)"
+            -F body=@body
         env:
+          # This token has write access to only issues to create one
           GH_TOKEN: ${{ github.token }}

doc/org-repo.md

@@ -4,9 +4,7 @@ This repository itself is the entry point for documentation on official resource
 
 Everybody in the [CODEOWNERS](../.github/CODEOWNERS) file has write permission to this repository.
 This allows people to get automatic review requests and merge PRs for the files that concern them.
-
-TODO: Enable branch protection to require reviews by code owners.
-TODO: Ensure that all files have a code owner
+PRs can only be merged if a codeowner for the respective files approves it, and all files need to have a codeowner entry.
 
 Furthermore, the code owners for the CODEOWNERS file should have permission to give more people write access to this repository.
 These people get requested for reviews when new people add themselves to CODEOWNERS, allowing them to give write access when merged.

review-body.sh renamed to scripts/review-body.sh

@@ -1,9 +1,21 @@
-#!/usr/bin/env bash
+#!/usr/bin/env nix-shell
+#!nix-shell -i bash --pure --keep GH_TOKEN -I nixpkgs=channel:nixpkgs-unstable -p codeowners github-cli gitMinimal
+
 set -euo pipefail
 
 # This script outputs the contents of the regular review issue, see ./github/workflows/review.yml
 
-rev=$(git rev-parse HEAD)
+SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
+
+if (( $# != 2 )); then
+  echo "Usage: $0 PATH OWNER/REPO"
+  exit 1
+fi
+
+root=$1
+repo=$2
+
+rev=$(git -C "$root" rev-parse HEAD)
 
 echo "Because the documentation in this repository may slowly deviate from reality, this monthly issue is created to regularly review the files.
 
@@ -30,4 +42,11 @@ while read -r file users; do
     continue
   fi
   echo "- [ ] \`$file\`: $users"
-done < .github/CODEOWNERS
+done < "$root"/.github/CODEOWNERS
+
+echo ""
+
+# Check that all code owners have write permissions
+# `|| true` because this script fails when there are code owners without permissions,
+# which is useful to fail PRs, but not here
+bash "$SCRIPT_DIR"/unprivileged-owners.sh "$root" "$repo" || true

scripts/unprivileged-owners.sh

@@ -0,0 +1,50 @@
+#!/usr/bin/env nix-shell
+#!nix-shell -i bash --pure --keep GH_TOKEN -I nixpkgs=channel:nixpkgs-unstable -p codeowners github-cli
+
+set -euo pipefail
+
+tmp=$(mktemp -d)
+trap 'rm -rf "$tmp"' exit
+
+if (( $# != 2 )); then
+  echo "Usage: $0 PATH OWNER/REPO"
+  exit 1
+fi
+
+root=$1
+repo=$2
+
+# Writes all code owners into $tmp/codeowners, one user per line (without @)
+while read -r -a fields; do
+  # The first field is the filename
+  unset 'fields[0]'
+  if [[ "${fields[1]}" != "(unowned)" ]]; then
+    (IFS=$'\n'; echo "${fields[*]##@}")
+  fi
+done < <(cd "$root"; codeowners) |
+  sort -u > "$tmp/codeowners"
+
+# Get all users with push access
+gh api \
+  -H "Accept: application/vnd.github+json" \
+  -H "X-GitHub-Api-Version: 2022-11-28" \
+  --method GET \
+  -f permission=push \
+  /repos/"$repo"/collaborators \
+  -F per_page=100 \
+  --paginate \
+  --jq '.[].login' |
+  sort > "$tmp/collaborators"
+
+# Figure out all the owners that aren't collaborators
+readarray -t unprivilegedOwners < <(comm -23 "$tmp/codeowners" "$tmp/collaborators")
+
+if (( "${#unprivilegedOwners[@]}" == 0 )); then
+  echo "All code owners have write permission"
+else
+  echo "There are code owners that don't have write permission. Either remove them as code owners or give them write permission:"
+  for handle in "${unprivilegedOwners[@]}"; do
+    echo "- [ ] @$handle"
+  done
+  exit 1
+fi