Skip to content

Commit

Permalink
Use GitHub App for codeowner validation and remove hacky script
Browse files Browse the repository at this point in the history
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: mszostok/codeowners-validator#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: mszostok/codeowners-validator#222

Because it's not merged/released yet, we need to build it ourselves,
so I added some Nix to do that reproducibly.
  • Loading branch information
infinisil committed Apr 26, 2024
1 parent 9c1ee55 commit b9d3520
Show file tree
Hide file tree
Showing 8 changed files with 125 additions and 78 deletions.
4 changes: 4 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 15 additions & 28 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,37 +31,24 @@ 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
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"
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
result*
34 changes: 34 additions & 0 deletions default.nix
Original file line number Diff line number Diff line change
@@ -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
];
};
}
47 changes: 47 additions & 0 deletions npins/default.nix
Original file line number Diff line number Diff line change
@@ -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`"
23 changes: 23 additions & 0 deletions npins/sources.json
Original file line number Diff line number Diff line change
@@ -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
}
50 changes: 0 additions & 50 deletions scripts/unprivileged-owners.sh

This file was deleted.

1 change: 1 addition & 0 deletions shell.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(import ./. { }).shell

0 comments on commit b9d3520

Please sign in to comment.