Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

vole
Clone

Read only, no credentials:

https://rodent.software/git/vole.git

Write over SSH with a registered key:

git clone ssh://git@0.0.0.0:2222/vole.git

A free and open-source forge that lives in your Git repo.

Phase 4: only allowed runners green-light a branch

852c067d5fb8226a46574c82077c2fc10bba2eb4 · Keenan Nemetz · 17d ago · parent 5d6db024

Phase 4: only allowed runners green-light a branch

Signing made a result attributable; this makes the attribution matter. A required
check is a claim about a commit, and until now any admin of the repository could
satisfy it — which is exactly the party the check exists to constrain.

`acl.runners` is an allowlist of the accounts whose CI results count for a
repository, and the gate's rule is: a green run counts only when its attestation
names an account on that list. Admins are implicit, because they can rewrite the list
and a protected branch already lets them land anything with no checks at all — so the
list is about everybody else. A writer running CI on their laptop gets their result
recorded and not counted, and the run page says which, so the answer to "why is my
green run not gating?" is where somebody looks for it rather than only in a refusal.

It is deliberately not a role. Read/write answers "may this account touch the
repository"; this answers "may this account's build be believed about it", and
conflating them would make every writer trusted about builds.

The same list decides who may lease a job, since running one executes the
repository's code: the answer arrives before the build rather than after it. That
also loosens the old lease rule, which required admin — a named runner needs no
write access at all.

`vole acl runners <repo>` shows the list, bare names replace it, `+name` adds, and
`none` clears it. There is no `-name`: revoking trust is the operation where being
made to state the whole list is a feature, and the list is printed after every change
so a dropped name is visible at once.

Fixed on the way: the new "skip non-successful runs" test would have dereferenced a
nil run for the no-run case, which TestMissingRunBlocksWithAHint caught as a panic in
the hook rather than a message.

Verified: TestOnlyAllowedRunnersGreenLightABranch walks the whole rule — an unlisted
writer is given no work, and the run page and the refusal both explain it; naming them
a runner lets them lease, run and report (attested to them); revoking the runner
refuses the merge naming the account and the command that would allow it, with the run
page saying "does not count as a check"; allowing them again lets the same green
result land, with nothing else changed.
TestACLRunnersEditsTheList covers show, +add, replacement and the unknown-account
refusal. internal/acl tests pin the rule itself: admins implicit, listed accounts
count, read-only accounts do not, and an empty account never does; plus list
normalisation and the round trip through the ref it is stored on.
scripts/ci.sh green (gofmt, vet, build, markdown links, go test -race ./...).

internal/server/integration_test.go +157−4

@@ -2555,8 +2555,10 @@ func TestInvalidWorkflowRejectedAtPush(t *testing.T) {
2555 2555 // ---------- CI ----------
2556 2556
2557 2557 // ciEnv is the environment a runner needs: the same key git would push with.
2558 func (f *fixture) ciEnv() []string {
2559 sshCmd := "ssh -i " + f.keyPath + " -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o LogLevel=ERROR"
2558 func (f *fixture) ciEnv() []string { return f.ciEnvFor(f.keyPath) }
2559
2560 func (f *fixture) ciEnvFor(keyPath string) []string {
2561 sshCmd := "ssh -i " + keyPath + " -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o LogLevel=ERROR"
2560 2562 return append(os.Environ(), "VOLE_SSH_COMMAND="+sshCmd, "GIT_SSH_COMMAND="+sshCmd,
2561 2563 "GIT_TERMINAL_PROMPT=0", "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null")
2562 2564 }
@@ -2795,15 +2797,30 @@ jobs:
2795 2797 }
2796 2798
2797 2799 // runOnce executes one queued job with a real runner.
2798 func (f *fixture) runOnce(t *testing.T) {
2800 func (f *fixture) runOnce(t *testing.T) { f.runOnceAs(t, f.keyPath) }
2801
2802 // runOnceAs runs one CI job as the account the key belongs to, which is how a test
2803 // gets a result reported by somebody other than the instance's admin.
2804 func (f *fixture) runOnceAs(t *testing.T, keyPath string) {
2799 2805 t.Helper()
2800 2806 cmd := exec.Command(binPath, "runner", "--remote", f.sshBase(), "--once", "--dir", t.TempDir())
2801 cmd.Env = f.ciEnv()
2807 cmd.Env = f.ciEnvFor(keyPath)
2802 2808 if out, err := cmd.CombinedOutput(); err != nil {
2803 2809 t.Fatalf("runner: %v\n%s", err, out)
2804 2810 }
2805 2811 }
2806 2812
2813 // cfgFile writes a config that points at this fixture's data directory, so a test can
2814 // run the CLI the way an operator would next to the running server.
2815 func (f *fixture) cfgFile(t *testing.T) string {
2816 t.Helper()
2817 path := filepath.Join(t.TempDir(), "vole.toml")
2818 if err := os.WriteFile(path, []byte(fmt.Sprintf("data_dir = %q\n", f.cfg.DataDir)), 0o644); err != nil {
2819 t.Fatal(err)
2820 }
2821 return path
2822 }
2823
2807 2824 // openAndApprove files a request for head and gets it approved.
2808 2825 func (f *fixture) openAndApprove(t *testing.T, head, author string) {
2809 2826 t.Helper()
@@ -2854,6 +2871,142 @@ func TestProtectedBranchRequiresGreenChecks(t *testing.T) {
2854 2871 }
2855 2872 }
2856 2873
2874 // TestOnlyAllowedRunnersGreenLightABranch: a green run counts only if the host let
2875 // the account that reported it. A required check is a claim about a commit, and
2876 // without this rule any writer could satisfy it by running CI on their own machine.
2877 func TestOnlyAllowedRunnersGreenLightABranch(t *testing.T) {
2878 f := newFixtureWith(t, func(c *config.Config) { c.LeaseSeconds = 2 })
2879 writerKey := f.addWriter(t)
2880 wilbur := f.accountID("wilbur")
2881 f.publish(f.repo)
2882 if _, errs, err := f.git(workRepo(t, map[string]string{"a.md": "a\n"}),
2883 "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
2884 t.Fatalf("initial main: %s", errs)
2885 }
2886 m := mustRepo(t, f)
2887 a, _ := m.ACL()
2888 a.Protected = []string{"refs/heads/main"}
2889 if err := m.SetACL(a, "test"); err != nil {
2890 t.Fatal(err)
2891 }
2892
2893 feature, head := f.cloneWithChange(t, workflowFile("version: 1\njobs:\n test:\n steps:\n - run: echo ok\n"), "ci")
2894 if _, errs, err := f.gitWithKey(writerKey, feature, "push", f.sshURL(f.repo), "HEAD:refs/heads/feature"); err != nil {
2895 t.Fatalf("feature: %s", errs)
2896 }
2897 f.waitForRun(t, 10*time.Second, func(r *runs.Run) bool { return r.SHA == head })
2898
2899 // Not a runner yet: the job is not even leased, because running a job executes
2900 // the repository's code. The push stays queued rather than silently green.
2901 f.runOnceAs(t, writerKey)
2902 if got := f.waitForRun(t, 10*time.Second, func(r *runs.Run) bool { return r.SHA == head }); got.Status != runs.StatusQueued {
2903 t.Fatalf("a non-runner was given work: %+v", got)
2904 }
2905
2906 // Allow the account, through the command a host admin would use.
2907 out, errs, err := f.cli("", "acl", "runners", f.repo, "+wilbur", "--config", f.cfgFile(t))
2908 if err != nil {
2909 t.Fatalf("acl runners: %v\n%s", err, errs)
2910 }
2911 if !strings.Contains(out, "wilbur") {
2912 t.Fatalf("acl runners did not name the account:\n%s", out)
2913 }
2914 f.runOnceAs(t, writerKey)
2915 done := f.waitForRun(t, 20*time.Second, func(r *runs.Run) bool {
2916 return r.SHA == head && r.Status == runs.StatusSuccess
2917 })
2918 att, err := f.srv.Runs.Attestation(done.ID)
2919 if err != nil || att == nil || att.Signer != wilbur {
2920 t.Fatalf("the run is not attributed to wilbur: %+v, %v", att, err)
2921 }
2922
2923 // Revoking trust is what the rule is for: the same green result stops counting,
2924 // and the refusal says who reported it and what to do about it.
2925 a, _ = m.ACL()
2926 a.SetRunners(nil)
2927 if err := m.SetACL(a, "test"); err != nil {
2928 t.Fatal(err)
2929 }
2930 f.openAndApprove(t, head, "wilbur")
2931 _, errs, err = f.gitWithKey(writerKey, feature, "push", f.sshURL(f.repo), "HEAD:refs/heads/main")
2932 if err == nil {
2933 t.Fatal("a result from a revoked runner landed on a protected branch")
2934 }
2935 if !strings.Contains(errs, "is not a runner for this repository") || !strings.Contains(errs, "acl runners") {
2936 t.Fatalf("the refusal does not say why:\n%s", errs)
2937 }
2938 // The page explains the same thing where somebody would look for it.
2939 if _, page := f.get("/r/" + f.repo + "/runs/" + done.ID); !strings.Contains(page, "does not count as a check") {
2940 t.Fatalf("the run page does not say the result does not count:\n%s", page)
2941 }
2942
2943 // Allowing it again is enough: nothing else changed, and nothing was lost.
2944 if _, errs, err := f.cli("", "acl", "runners", f.repo, "+wilbur", "--config", f.cfgFile(t)); err != nil {
2945 t.Fatalf("acl runners: %v\n%s", err, errs)
2946 }
2947 if _, errs, err := f.gitWithKey(writerKey, feature, "push", f.sshURL(f.repo), "HEAD:refs/heads/main"); err != nil {
2948 t.Fatalf("an allowed runner's result was refused: %s", errs)
2949 }
2950 }
2951
2952 // TestACLRunnersEditsTheList: three shapes, because replacing a list to add a sixth
2953 // name is how a list gets shortened by mistake.
2954 func TestACLRunnersEditsTheList(t *testing.T) {
2955 f := newFixture(t)
2956 f.addWriter(t)
2957 f.publish(f.repo)
2958 cfg := f.cfgFile(t)
2959 runners := func() []string {
2960 t.Helper()
2961 a, err := mustRepo(t, f).ACL()
2962 if err != nil {
2963 t.Fatal(err)
2964 }
2965 return a.Runners
2966 }
2967
2968 if out, errs, err := f.cli("", "acl", "runners", f.repo, "+wilbur", "--config", cfg); err != nil {
2969 t.Fatalf("add: %v\n%s", err, errs)
2970 } else if !strings.Contains(out, "wilbur") {
2971 t.Fatalf("add did not report the account:\n%s", out)
2972 }
2973 if got := runners(); len(got) != 1 || got[0] != f.accountID("wilbur") {
2974 t.Fatalf("runners after +wilbur = %#v", got)
2975 }
2976
2977 // The owner is an admin, so they already count; naming them is legal but adds
2978 // nothing, and an empty list is rendered as what it means rather than as "none".
2979 if out, _, err := f.cli("", "acl", "runners", f.repo, "--config", cfg); err != nil {
2980 t.Fatalf("show: %v", err)
2981 } else if !strings.Contains(out, "wilbur") {
2982 t.Fatalf("show did not list the runner:\n%s", out)
2983 }
2984
2985 // Bare names replace the list: a repo with one runner stays a repo with one.
2986 if _, errs, err := f.cli("", "acl", "runners", f.repo, "keenan", "--config", cfg); err != nil {
2987 t.Fatalf("replace: %v\n%s", err, errs)
2988 }
2989 if got := runners(); len(got) != 1 || got[0] != f.ownerID {
2990 t.Fatalf("runners after replacement = %#v", got)
2991 }
2992
2993 // Revoking trust is a replacement, not an inverse operation, and `none` is how
2994 // "back to admins only" is said — the same word `acl set` uses for no role.
2995 if _, errs, err := f.cli("", "acl", "runners", f.repo, "none", "--config", cfg); err != nil {
2996 t.Fatalf("clear: %v\n%s", err, errs)
2997 }
2998 if got := runners(); len(got) != 0 {
2999 t.Fatalf("runners after none = %#v", got)
3000 }
3001
3002 // An unknown account is refused rather than written as a typo that silently
3003 // permits nobody.
3004 _, errs, err := f.cli("", "acl", "runners", f.repo, "+nobody", "--config", cfg)
3005 if err == nil {
3006 t.Fatalf("an unknown account was accepted:\n%s", errs)
3007 }
3008 }
3009
2857 3010 // TestFailingCheckBlocksTheMerge: a green approval does not outvote a red build.
2858 3011 func TestFailingCheckBlocksTheMerge(t *testing.T) {
2859 3012 f := newFixtureWith(t, func(c *config.Config) { c.LeaseSeconds = 2 })