Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate CODEOWNERS #4

Merged
merged 6 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -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
60 changes: 56 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -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 }}"
16 changes: 14 additions & 2 deletions .github/workflows/review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
4 changes: 1 addition & 3 deletions doc/org-repo.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
25 changes: 22 additions & 3 deletions review-body.sh → scripts/review-body.sh
Original file line number Diff line number Diff line change
@@ -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.

Expand All @@ -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
50 changes: 50 additions & 0 deletions scripts/unprivileged-owners.sh
Original file line number Diff line number Diff line change
@@ -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