diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index b2ecce3..bb4f960 100644 --- a/.github/CODEOWNERS +++ b/.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 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 97f9648..e22906a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,8 +1,11 @@ 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: @@ -10,6 +13,55 @@ jobs: 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/codeowners-validator@v0.7.4 + 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 }}" diff --git a/.github/workflows/review.yml b/.github/workflows/review.yml index b63c140..bc70633 100644 --- a/.github/workflows/review.yml +++ b/.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 }} diff --git a/doc/org-repo.md b/doc/org-repo.md index 7314250..d213cb8 100644 --- a/doc/org-repo.md +++ b/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. diff --git a/review-body.sh b/scripts/review-body.sh similarity index 66% rename from review-body.sh rename to scripts/review-body.sh index c9e8758..2b68d59 100755 --- a/review-body.sh +++ b/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 diff --git a/scripts/unprivileged-owners.sh b/scripts/unprivileged-owners.sh new file mode 100755 index 0000000..36e16d3 --- /dev/null +++ b/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