From af47dbcc9c5f7fdead2a7dcc693e355681754939 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 26 Apr 2024 16:16:17 +0200 Subject: [PATCH] Use GitHub App for codeowner validation and remove hacky script We shouldn't use personal access tokens, instead we created a GitHub App with read-only access to just this repository. While codeowners-validator supports GitHub App authentication, the same cannot be said for the hacky script I wrote because there was no support for checking write access: https://github.com/mszostok/codeowners-validator/issues/157 Instead of trying to hack the script more to make it work with GitHub App authentication, I decided to implement it into codeowners-validator itself: https://github.com/mszostok/codeowners-validator/pull/222 Because it's not merged/released yet, we need to build it ourselves, so I added some Nix to do that reproducibly. --- .github/CODEOWNERS | 4 +++ .github/workflows/ci.yml | 44 +++++++++++------------------- .gitignore | 1 + default.nix | 34 +++++++++++++++++++++++ npins/default.nix | 47 ++++++++++++++++++++++++++++++++ npins/sources.json | 23 ++++++++++++++++ scripts/unprivileged-owners.sh | 50 ---------------------------------- shell.nix | 1 + 8 files changed, 126 insertions(+), 78 deletions(-) create mode 100644 .gitignore create mode 100644 default.nix create mode 100644 npins/default.nix create mode 100644 npins/sources.json delete mode 100755 scripts/unprivileged-owners.sh create mode 100644 shell.nix diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index bb4f960..2469090 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -4,9 +4,13 @@ /CONTRIBUTING.md @infinisil @zimbatm /LICENSE @infinisil @zimbatm +/.gitignore @infinisil @zimbatm /.github/CODEOWNERS @infinisil @zimbatm /.github/workflows @infinisil @zimbatm /scripts @infinisil @zimbatm +/npins @infinisil @zimbatm +/default.nix @infinisil @zimbatm +/shell.nix @infinisil @zimbatm /doc/org-repo.md @infinisil @zimbatm /doc/discourse.md @infinisil @zimbatm diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e22906a..c1ed1a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,37 +31,25 @@ jobs: with: path: trusted-base + - name: Build codeowners validator + # We run the result of this with access to secrets later, so it's important to trust this! + run: nix-build trusted-base -A packages.codeowners-validator + - 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" - - # 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 }} + - name: Validate codeowners + run: result/bin/codeowners-validator env: - GH_TOKEN: "${{ secrets.OWNERS_VALIDATOR_GITHUB_SECRET }}" + # See https://github.com/mszostok/codeowners-validator/blob/main/docs/gh-auth.md#public-repositories + # And https://github.com/mszostok/codeowners-validator/pull/222#issuecomment-2079521185 + GITHUB_APP_ID: ${{ secrets.APP_ID }} + GITHUB_APP_INSTALLATION_ID: ${{ secrets.APP_INSTALLATION_ID }} + GITHUB_APP_PRIVATE_KEY: ${{ secrets.APP_PRIVATE_KEY }} + REPOSITORY_PATH: untrusted-pr + OWNER_CHECKER_REPOSITORY: ${{ github.repository }} + EXPERIMENTAL_CHECKS: "notowned,avoid-shadowing" + OWNER_CHECKER_ALLOW_UNOWNED_PATTERNS: "false" + OWNER_CHECKER_OWNERS_MUST_BE_TEAMS: "false" diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..fcfc4a1 --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +result* diff --git a/default.nix b/default.nix new file mode 100644 index 0000000..bebac2e --- /dev/null +++ b/default.nix @@ -0,0 +1,34 @@ +let + sources = import ./npins; +in +{ + system ? builtins.currentSystem, + nixpkgs ? sources.nixpkgs, +}: +let + pkgs = import nixpkgs { + inherit system; + config = { }; + overlays = [ ]; + }; + inherit (pkgs) lib; + + packages = { + codeowners-validator = pkgs.buildGoModule { + name = "codeowners-validator"; + src = sources.codeowners-validator; + vendorHash = "sha256-R+pW3xcfpkTRqfS2ETVOwG8PZr0iH5ewroiF7u8hcYI="; + postPatch = "rm -r docs/investigation"; + }; + }; + +in +{ + inherit packages; + + shell = pkgs.mkShell { + packages = [ + pkgs.npins + ]; + }; +} diff --git a/npins/default.nix b/npins/default.nix new file mode 100644 index 0000000..4a7c372 --- /dev/null +++ b/npins/default.nix @@ -0,0 +1,47 @@ +# Generated by npins. Do not modify; will be overwritten regularly +let + data = builtins.fromJSON (builtins.readFile ./sources.json); + version = data.version; + + mkSource = spec: + assert spec ? type; let + path = + if spec.type == "Git" then mkGitSource spec + else if spec.type == "GitRelease" then mkGitSource spec + else if spec.type == "PyPi" then mkPyPiSource spec + else if spec.type == "Channel" then mkChannelSource spec + else builtins.throw "Unknown source type ${spec.type}"; + in + spec // { outPath = path; }; + + mkGitSource = { repository, revision, url ? null, hash, ... }: + assert repository ? type; + # At the moment, either it is a plain git repository (which has an url), or it is a GitHub/GitLab repository + # In the latter case, there we will always be an url to the tarball + if url != null then + (builtins.fetchTarball { + inherit url; + sha256 = hash; # FIXME: check nix version & use SRI hashes + }) + else assert repository.type == "Git"; builtins.fetchGit { + url = repository.url; + rev = revision; + # hash = hash; + }; + + mkPyPiSource = { url, hash, ... }: + builtins.fetchurl { + inherit url; + sha256 = hash; + }; + + mkChannelSource = { url, hash, ... }: + builtins.fetchTarball { + inherit url; + sha256 = hash; + }; +in +if version == 3 then + builtins.mapAttrs (_: mkSource) data.pins +else + throw "Unsupported format version ${toString version} in sources.json. Try running `npins upgrade`" diff --git a/npins/sources.json b/npins/sources.json new file mode 100644 index 0000000..6510ca9 --- /dev/null +++ b/npins/sources.json @@ -0,0 +1,23 @@ +{ + "pins": { + "codeowners-validator": { + "type": "Git", + "repository": { + "type": "GitHub", + "owner": "tweag", + "repo": "codeowners-validator" + }, + "branch": "simpler-faster-permission-check", + "revision": "a69f70c0bd8ec168ff695f412afa83c7b7a65413", + "url": "https://github.com/tweag/codeowners-validator/archive/a69f70c0bd8ec168ff695f412afa83c7b7a65413.tar.gz", + "hash": "1rybdypjgn4i065r6msfwyx1rvv73x19p28lps3si79bwbkg2xg0" + }, + "nixpkgs": { + "type": "Channel", + "name": "nixpkgs-unstable", + "url": "https://releases.nixos.org/nixpkgs/nixpkgs-24.05pre616757.4c86138ce486/nixexprs.tar.xz", + "hash": "0lbvdj9jc7g3pqs0yvahpb8y453gn65jvkvbnnkbi6m4afp92p04" + } + }, + "version": 3 +} \ No newline at end of file diff --git a/scripts/unprivileged-owners.sh b/scripts/unprivileged-owners.sh deleted file mode 100755 index 36e16d3..0000000 --- a/scripts/unprivileged-owners.sh +++ /dev/null @@ -1,50 +0,0 @@ -#!/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 diff --git a/shell.nix b/shell.nix new file mode 100644 index 0000000..a6bdf20 --- /dev/null +++ b/shell.nix @@ -0,0 +1 @@ +(import ./. { }).shell