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/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

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

Phase 4: a runner can narrow what it takes, and cannot widen it

b238ac2448290fb706d0503c2e65798fc618f994 · Keenan Nemetz · 17d ago · parent 8d336083

Phase 4: a runner can narrow what it takes, and cannot widen it

`vole runner --repos name` (repeatable) restricts a session to the
repositories it names. The restriction is applied by the server as an
intersection with what the account's runner allowlist grants, which is the
whole design: the flag is a filter, not a grant, so the worst a client can do
is run less than it was allowed to. Honouring the client's list directly
would have made an SSH flag an access-control decision.

An empty filter means "no narrowing" rather than "nothing", which keeps an
unfiltered runner following allowlist edits instead of freezing its list at
connect time.

The filter has a failure mode with no symptom: a runner whose --repos matches
nothing connects, waits, and never sees work, which looks exactly like a quiet
instance. So "ping" now reports the account's grant and the runner refuses at
startup, naming what it asked for and what it was allowed — the same class of
check as the signing key, cheaper before a build than after one that never
happened. An account with an empty grant and no filter is only a warning,
because an allowlist nobody has written yet takes effect without a restart.

runs.Store.Lease now takes a LeaseRequest struct. It gained a second list of
names, and two adjacent []string arguments are exactly the shape that gets
transposed in silence.

Measured: TestARequestCannotLeaseBeyondTheAllowlist — a repository the account
may *write* to but is not a runner for is refused even when asked for by name,
and an admin can still lease it. TestARepoFilterTakesOnlyWhatItNames,
TestNarrowCanOnlySubtract, TestRepoFilterThatCanNeverMatchIsRefused and
TestARepoFilterOutsideTheGrantIsRefusedAtStartup cover the rest.

internal/sshserver/repos_filter_test.go +168

@@ -0,0 +1,168 @@
1 package sshserver
2
3 // --repos lets one runner narrow what it will take, and the narrowing is done on the
4 // server against what the account's allowlist grants. These tests are about the
5 // direction of that: an operator's filter may shrink a grant, and a client's request
6 // may never grow it. The pure half (narrow) is pinned directly; the wire half is pinned
7 // through a real session, because the thing that could regress is a request field that
8 // nobody remembers to apply.
9
10 import (
11 "strings"
12 "testing"
13 "time"
14
15 "github.com/rodent-software/vole/internal/id"
16 "github.com/rodent-software/vole/internal/runs"
17 )
18
19 func TestNarrowCanOnlySubtract(t *testing.T) {
20 granted := []string{"a", "b", "c"}
21 cases := []struct {
22 name string
23 asked []string
24 want []string
25 }{
26 {"no filter keeps the whole grant", nil, []string{"a", "b", "c"}},
27 {"a filter keeps only what it names", []string{"b"}, []string{"b"}},
28 {"order follows the grant, not the filter", []string{"c", "a"}, []string{"a", "c"}},
29 {"duplicates are harmless", []string{"a", "a"}, []string{"a"}},
30 {"a name that was never granted is dropped", []string{"a", "z"}, []string{"a"}},
31 {"names outside the grant yield nothing", []string{"z"}, nil},
32 {"an empty grant yields nothing however it is asked for", []string{"a"}, nil},
33 }
34 for _, c := range cases {
35 t.Run(c.name, func(t *testing.T) {
36 g := granted
37 if strings.Contains(c.name, "empty grant") {
38 g = nil
39 }
40 got := narrow(g, c.asked)
41 if strings.Join(got, ",") != strings.Join(c.want, ",") {
42 t.Fatalf("narrow(%v, %v) = %v, want %v", g, c.asked, got, c.want)
43 }
44 })
45 }
46 }
47
48 // A lease request that names a repository the account may not run for gets nothing,
49 // rather than that repository. The allowlist is what decides; the flag only filters
50 // what it produced.
51 func TestARequestCannotLeaseBeyondTheAllowlist(t *testing.T) {
52 h := newHarness(t)
53 c := h.runner(t, h.keyPath)
54 if rep := c.call(runnerRequest{Op: "ping"}); rep.Error != "" {
55 t.Fatal(rep.Error)
56 }
57
58 // A second repository the writer's account is not a runner for. Its ACL gives the
59 // writer write access, which is exactly the distinction that matters: writing to a
60 // repository is not permission to execute its code.
61 other, err := h.repos.Create("other", h.writerID, "")
62 if err != nil {
63 t.Fatal(err)
64 }
65 a, err := other.ACL()
66 if err != nil {
67 t.Fatal(err)
68 }
69 if err := a.SetEntry(h.writerID, "write"); err != nil {
70 t.Fatal(err)
71 }
72 if err := other.SetACL(a, "test"); err != nil {
73 t.Fatal(err)
74 }
75 runID, err := id.New(time.Now().UTC())
76 if err != nil {
77 t.Fatal(err)
78 }
79 if _, err := h.runs.Enqueue(runs.Run{
80 ID: runID, Repo: "other", Ref: "refs/heads/main", Branch: "main",
81 SHA: strings.Repeat("ab", 20), Workflow: ".vole/workflows/ci.yaml",
82 WorkflowSHA: strings.Repeat("cd", 20), Job: "forbidden",
83 Steps: []runs.Step{{Name: "test", Run: "true"}}, Created: time.Now().UTC(),
84 }); err != nil {
85 t.Fatal(err)
86 }
87
88 // Asking for it explicitly must not help: the grant for demo does not include
89 // other, so the intersection is empty and nothing is leased.
90 rep := c.call(runnerRequest{Op: "lease", Repos: []string{"other"}})
91 if rep.Error != "" {
92 t.Fatal(rep.Error)
93 }
94 if rep.Run != nil {
95 t.Fatalf("leased %q from %q, which the account may not run for", rep.Run.Job, rep.Run.Repo)
96 }
97
98 // The work is still there for somebody who is allowed: an admin is implicit.
99 admin, err := h.runs.Lease(runs.LeaseRequest{
100 Owner: h.writerID, Repos: []string{"other"}, TTL: time.Minute,
101 }, time.Now().UTC())
102 if err != nil {
103 t.Fatal(err)
104 }
105 if admin == nil {
106 t.Fatal("nobody could lease the job; it should still be queued")
107 }
108 }
109
110 // A filter naming one repository takes that repository's jobs and leaves the rest.
111 func TestARepoFilterTakesOnlyWhatItNames(t *testing.T) {
112 h := newHarness(t)
113 c := h.runner(t, h.keyPath)
114 if rep := c.call(runnerRequest{Op: "ping"}); rep.Error != "" {
115 t.Fatal(rep.Error)
116 }
117
118 // Two repositories the account may run for, one job in each.
119 for _, name := range []string{"demo", "second"} {
120 m, err := h.repos.Open(name)
121 if err != nil {
122 m, err = h.repos.Create(name, h.writerID, "")
123 if err != nil {
124 t.Fatal(err)
125 }
126 }
127 a, err := m.ACL()
128 if err != nil {
129 t.Fatal(err)
130 }
131 a.SetRunners([]string{h.writerID})
132 if err := m.SetACL(a, "test"); err != nil {
133 t.Fatal(err)
134 }
135 runID, err := id.New(time.Now().UTC())
136 if err != nil {
137 t.Fatal(err)
138 }
139 if _, err := h.runs.Enqueue(runs.Run{
140 ID: runID, Repo: name, Ref: "refs/heads/main", Branch: "main",
141 SHA: strings.Repeat("ab", 20), Workflow: ".vole/workflows/ci.yaml",
142 WorkflowSHA: strings.Repeat("cd", 20), Job: "job-" + name,
143 Steps: []runs.Step{{Name: "test", Run: "true"}}, Created: time.Now().UTC(),
144 }); err != nil {
145 t.Fatal(err)
146 }
147 }
148
149 // The ping reports the grant so the client can check its own filter against it.
150 if rep := c.call(runnerRequest{Op: "ping"}); len(rep.Repos) != 2 {
151 t.Fatalf("ping reported %v, want both repositories the account may run for", rep.Repos)
152 }
153
154 rep := c.call(runnerRequest{Op: "lease", Repos: []string{"second"}})
155 if rep.Error != "" {
156 t.Fatal(rep.Error)
157 }
158 if rep.Run == nil {
159 t.Fatal("the filtered repository's job was not leased")
160 }
161 if rep.Run.Repo != "second" {
162 t.Fatalf("leased %q from %q; the filter named only second", rep.Run.Job, rep.Run.Repo)
163 }
164 // demo's job is untouched by that filter: a second, unfiltered runner takes it.
165 if again := c.call(runnerRequest{Op: "lease"}); again.Run == nil || again.Run.Repo != "demo" {
166 t.Fatalf("unfiltered lease = %+v, want demo's job", again.Run)
167 }
168 }