krz/gitbay

runner: disposable home for untrusted builds !479

merged merged by cmc on 2026-09-28 22:33 UTC · krz/gitbay:ci-untrusted-home into main

11 files changed, +206 −101

Layout: unified · split

.gitbay/wiki/Admin.org +8 −3
@@ -681,9 +681,14 @@ allocates without bound, and it sits above the e2e suite's 5GB peak
681rather than at a fair share. =OOMPolicy=continue= keeps systemd from 681rather than at a fair share. =OOMPolicy=continue= keeps systemd from
682stopping the runner when a build is OOM-killed. 682stopping the runner when a build is OOM-killed.
683 683
684Each repository gets its own build home under the runner's workdir, 684A trusted build's home is its repository's, under
685mounted into its containers as =HOME=. Caches persist between builds of 685=<workdir>/trusted-home/<owner>/<name>=, mounted into its containers as
686one repository and are never read by another's. 686=HOME=: caches persist between trusted builds of one repository and are
687never read by another's. An untrusted build — a merge request head from
688a fork — gets =<workdir>/build-<id>-home=, new and empty, removed when
689the build ends. Homes under =<workdir>/home= are from runners before
690krz/gitbay#255, which shared them with untrusted builds; nothing reads
691them any more, and they can be deleted.
687 692
688*Images are provisioned, never pulled by a build.* The runner passes 693*Images are provisioned, never pulled by a build.* The runner passes
689=--pull=never=. Two reasons, and the second is the better one: the 694=--pull=never=. Two reasons, and the second is the better one: the
.gitbay/wiki/Architecture/04-Trust-Boundaries.org +1 −1
@@ -24,7 +24,7 @@
24| TB4 | Z1 → Z3 git | argv, repository path, stdin packs | argv built by code, never a shell; repository path from the database, not the request (=internal/gitutil=) | 24| TB4 | Z1 → Z3 git | argv, repository path, stdin packs | argv built by code, never a shell; repository path from the database, not the request (=internal/gitutil=) |
25| TB5 | Z3 → Z1 hook socket | ref updates, repository id, user id, key scope, push token, commit objects | the socket is mode 0600 and, on Linux, refuses a peer whose uid is not the daemon's; a request must carry the token sshd minted for its receive-pack (stored hashed in =push_tokens=) and name the same repository, account and scope. The daemon then decides with =policy.CheckPush= and =sig.VerifyCommit= (=internal/hookd/hookd.go=) | 25| TB5 | Z3 → Z1 hook socket | ref updates, repository id, user id, key scope, push token, commit objects | the socket is mode 0600 and, on Linux, refuses a peer whose uid is not the daemon's; a request must carry the token sshd minted for its receive-pack (stored hashed in =push_tokens=) and name the same repository, account and scope. The daemon then decides with =policy.CheckPush= and =sig.VerifyCommit= (=internal/hookd/hookd.go=) |
26| TB6 | Z4 ↔ Z1 runner channel | build claims (with secrets for trusted builds), logs, results | runner-scoped SSH key; claims limited to attached repositories; secrets only when the build is trusted (=internal/control/build.go=) | 26| TB6 | Z4 ↔ Z1 runner channel | build claims (with secrets for trusted builds), logs, results | runner-scoped SSH key; claims limited to attached repositories; secrets only when the build is trusted (=internal/control/build.go=) |
27| TB7 | Z5 → Z4 container | build steps, workspace, build home | rootless podman, operator-provisioned image, cgroup limits; the build home is shared per repository and the network is open (#255, #260) | 27| TB7 | Z5 → Z4 container | build steps, workspace, build home | rootless podman, operator-provisioned image, cgroup limits; a trusted build's home is its repository's, an untrusted build's is discarded with it; the network is open (#260) |
28| TB8 | Z1 → Z0 outbound | webhooks, mirrors, mail, push | address checks on user-supplied URLs; HMAC on webhooks; no redirects ([[file:03-Deployment.org][3]]) | 28| TB8 | Z1 → Z0 outbound | webhooks, mirrors, mail, push | address checks on user-supplied URLs; HMAC on webhooks; no redirects ([[file:03-Deployment.org][3]]) |
29| TB9 | user content → browser | Markdown and Org bodies, READMEs, filenames | HTML sanitised (=ugcHTML=, =internal/httpd/web.go=, bluemonday); CSP =script-src 'none'= | 29| TB9 | user content → browser | Markdown and Org bodies, READMEs, filenames | HTML sanitised (=ugcHTML=, =internal/httpd/web.go=, bluemonday); CSP =script-src 'none'= |
30| TB10| Z6 → everything | host shell | operator SSH on 2222, keys only, fail2ban; append-only offsite backup credentials | 30| TB10| Z6 → everything | host shell | operator SSH on 2222, keys only, fail2ban; append-only offsite backup credentials |
.gitbay/wiki/Architecture/07-CI-and-Supply-Chain.org +4 −3
@@ -31,8 +31,9 @@ commit instead of failing silently.
31 runner key claims only for repositories it is attached to with 31 runner key claims only for repositories it is attached to with
32 =repo runner add=. Untrusted builds are claimable only by a runner 32 =repo runner add=. Untrusted builds are claimable only by a runner
33 started with =-untrusted= (=internal/store/builds.go=). The 33 started with =-untrusted= (=internal/store/builds.go=). The
34 claim returns id, repository, job, commit, ref, steps, image and — 34 claim returns id, repository, job, commit, ref, steps, image, the
35 for trusted builds only — the repository's secrets (=build.go=). 35 build's trust, and — for trusted builds only — the repository's secrets
36 (=build.go=).
363. *Run.* The runner clones over SSH into =build-<id>=, starts a 373. *Run.* The runner clones over SSH into =build-<id>=, starts a
37 container and runs each step with =podman exec … sh -c <step>= 38 container and runs each step with =podman exec … sh -c <step>=
38 (=cmd/gitbay-runner/isolate.go=). 39 (=cmd/gitbay-runner/isolate.go=).
@@ -64,7 +65,7 @@ Who may do what:
64| Container runtime | rootless podman under the =ci-runner= user and its subordinate uid range | 65| Container runtime | rootless podman under the =ci-runner= user and its subordinate uid range |
65| Image | =--pull=never=; images are built by the operator (=deploy/Containerfile.ci=) and referenced by tag | 66| Image | =--pull=never=; images are built by the operator (=deploy/Containerfile.ci=) and referenced by tag |
66| Workspace | =<workdir>/build-<id>=, removed after the build; workdir must be 0700 and owned by the runner (=main.go=) | 67| Workspace | =<workdir>/build-<id>=, removed after the build; workdir must be 0700 and owned by the runner (=main.go=) |
67| Build home | =<workdir>/home/<owner>/<name>=, one per repository, mounted read-write, shared by trusted and untrusted builds of that repository (#255) | 68| Build home | trusted: =<workdir>/trusted-home/<owner>/<name>=, one per repository, persistent; untrusted: =<workdir>/build-<id>-home=, removed with the build (=main.go=) |
68| Secrets | env file 0600 outside the workspace, or =--env NAME= for multi-line values | 69| Secrets | env file 0600 outside the workspace, or =--env NAME= for multi-line values |
69| Resources | per-build cgroup with =memory.max= and =cpu.max= written by the runner; unit-level =MemoryMax=6G=, =CPUQuota=300%= | 70| Resources | per-build cgroup with =memory.max= and =cpu.max= written by the runner; unit-level =MemoryMax=6G=, =CPUQuota=300%= |
70| Network | podman default (pasta); outbound unrestricted (#260) | 71| Network | podman default (pasta); outbound unrestricted (#260) |
.gitbay/wiki/Architecture/09-Controls.org +1 −1
@@ -84,7 +84,7 @@ chapter names of OWASP ASVS 4.0 where one fits.
84 84
85| Control | Status | Evidence | 85| Control | Status | Evidence |
86|---------------------------------------------+----------+------------------------------------------------------------------| 86|---------------------------------------------+----------+------------------------------------------------------------------|
87| Untrusted code runs isolated | partial | rootless podman, cgroup limits; shared build home per repository (#255) | 87| Untrusted code runs isolated | in place | rootless podman, cgroup limits; untrusted builds get a disposable home (=cmd/gitbay-runner/main.go=) |
88| No secrets for untrusted builds | in place | =internal/control/build.go= | 88| No secrets for untrusted builds | in place | =internal/control/build.go= |
89| Runner limited to attached repositories | in place | =runnerMayBuild= (=build.go=) | 89| Runner limited to attached repositories | in place | =runnerMayBuild= (=build.go=) |
90| Build images fixed by the operator | in place | =--pull=never= | 90| Build images fixed by the operator | in place | =--pull=never= |
.gitbay/wiki/Architecture/10-Known-Gaps.org −1
@@ -10,7 +10,6 @@ what the 2026-09-27 review found; remove a row when its issue closes.
10 10
11| Issue | Area | Gap | Severity | 11| Issue | Area | Gap | Severity |
12|-------+------------------+-----------------------------------------------------------------------+----------| 12|-------+------------------+-----------------------------------------------------------------------+----------|
13| #255 | CI isolation | Untrusted and trusted builds of a repository share a writable build home | high |
14| #258 | CI integrity | Any writer can post a =ci/*= status; tree reuse ignores trust and image | high | 13| #258 | CI integrity | Any writer can post a =ci/*= status; tree reuse ignores trust and image | high |
15| #259 | Recovery | No restore has been exercised; verification does not check git connectivity | high | 14| #259 | Recovery | No restore has been exercised; verification does not check git connectivity | high |
16| #260 | CI network | Builds share the runner's source address; no egress policy | medium | 15| #260 | CI network | Builds share the runner's source address; no egress policy | medium |
.gitbay/wiki/Threat-Model.org +13 −11
@@ -161,10 +161,12 @@ runner, polling over SSH, clones the commit and runs its steps.
161 secrets, and nothing the operator set on the service. =HOME= is a build 161 secrets, and nothing the operator set on the service. =HOME= is a build
162 home under the runner's =-workdir=, not the runner's own home, so a 162 home under the runner's =-workdir=, not the runner's own home, so a
163 build cannot read the =.netrc=, =.npmrc= or =.gitconfig= where tools 163 build cannot read the =.netrc=, =.npmrc= or =.gitconfig= where tools
164 keep credentials. That home is shared by every build on the runner — 164 keep credentials. A trusted build's home belongs to its repository
165 one build can poison a cache another reads, which is no more than 165 and persists, so caches survive; an untrusted build's home is new,
166 anything a step can already do as this user, and is what isolation 166 empty and removed when the build ends, so nothing a fork's build
167 (krz/gitbay#144) is for. 167 writes is read by a later build (krz/gitbay#255). The claim names a
168 build's trust explicitly, and a runner that finds no trust flag treats
169 the build as untrusted.
168- *Where it runs.* Steps run in a rootless podman container, one per 170- *Where it runs.* Steps run in a rootless podman container, one per
169 job, with the workspace bind mounted and nothing else. The clone 171 job, with the workspace bind mounted and nothing else. The clone
170 happens outside it with the runner's key, so the container never sees 172 happens outside it with the runner's key, so the container never sees
@@ -194,13 +196,13 @@ runner, polling over SSH, clones the commit and runs its steps.
194 196
195Under =-isolation none=, anything a step can do as the runner's user a 197Under =-isolation none=, anything a step can do as the runner's user a
196pushed =ci.yml= can do. Under podman a step is confined to its 198pushed =ci.yml= can do. Under podman a step is confined to its
197container, the bind-mounted workspace and the repository's own build 199container, the bind-mounted workspace and its build home: a trusted
198home, so what a build leaves in a cache is read only by later builds of 200build's cache is read only by later trusted builds of the same
199the same repository. Treat the runner host as executing untrusted code 201repository, and an untrusted build's home is discarded with it. Treat
200all the same: keep it off the daemon's host where the database lives, 202the runner host as executing untrusted code all the same: keep it off
201or scope it to repositories whose writers you trust. gitbay.org does 203the daemon's host where the database lives, or scope it to repositories
202the latter — its runner builds only the repositories the operator 204whose writers you trust. gitbay.org does the latter — its runner builds
203names. 205only the repositories the operator names.
204 206
205* What has not been audited 207* What has not been audited
206 208
cmd/gitbay-runner/env_test.go +10 −7
@@ -44,17 +44,20 @@ func TestStepEnvDoesNotInherit(t *testing.T) {
44 } 44 }
45} 45}
46 46
47// Secrets are passed through when the server sent them, which it does 47// Secrets reach a trusted build's steps and never an untrusted one's,
48// only for a trusted build. 48// whatever the claim carried: the trust flag decides, not whether any
49// secrets arrived (#255).
49func TestStepEnvCarriesSecrets(t *testing.T) { 50func TestStepEnvCarriesSecrets(t *testing.T) {
50 env := stepEnv(job{Secrets: map[string]string{"TOKEN": "s3cret"}}, "/tmp/buildhome", "git@x.test") 51 secrets := map[string]string{"TOKEN": "s3cret"}
52 env := stepEnv(job{Trusted: true, Secrets: secrets}, "/tmp/buildhome", "git@x.test")
51 if !containsEnv(env, "TOKEN=s3cret") { 53 if !containsEnv(env, "TOKEN=s3cret") {
52 t.Error("a trusted build's secret did not reach the step") 54 t.Error("a trusted build's secret did not reach the step")
53 } 55 }
54 env = stepEnv(job{}, "/tmp/buildhome", "git@x.test") 56 for _, j := range []job{{}, {Secrets: secrets}} {
55 for _, e := range env { 57 for _, e := range stepEnv(j, "/tmp/buildhome", "git@x.test") {
56 if strings.HasPrefix(e, "TOKEN=") { 58 if strings.HasPrefix(e, "TOKEN=") {
57 t.Errorf("a secret appeared with none sent: %q", e) 59 t.Errorf("a secret reached an untrusted build: %q", e)
60 }
58 } 61 }
59 } 62 }
60} 63}
cmd/gitbay-runner/home_test.go +56 −19
@@ -3,51 +3,88 @@ package main
3import ( 3import (
4 "os" 4 "os"
5 "path/filepath" 5 "path/filepath"
6 "strings"
6 "testing" 7 "testing"
7) 8)
8 9
9// The build home is per repository: one shared home let a step poison 10// A trusted build's home is its repository's, kept between builds so
10// the module cache or plant a .gitconfig that another repository's build 11// tool caches survive: the same repository gets the same directory back,
11// would honour (#184). 12// another repository a different one (#184).
12func TestBuildHomeIsPerRepository(t *testing.T) { 13func TestTrustedHomeIsPerRepositoryAndKept(t *testing.T) {
13 work := t.TempDir() 14 work := t.TempDir()
14 a, err := buildHomeFor(work, "alice/app") 15 a, done, err := buildHome(work, job{ID: 1, Repo: "alice/app", Trusted: true})
15 if err != nil { 16 if err != nil {
16 t.Fatal(err) 17 t.Fatal(err)
17 } 18 }
18 b, err := buildHomeFor(work, "bob/app") 19 done()
20 if _, err := os.Stat(a); err != nil {
21 t.Fatalf("trusted home removed after its build: %v", err)
22 }
23 b, done, err := buildHome(work, job{ID: 2, Repo: "bob/app", Trusted: true})
19 if err != nil { 24 if err != nil {
20 t.Fatal(err) 25 t.Fatal(err)
21 } 26 }
27 done()
22 if a == b { 28 if a == b {
23 t.Fatalf("two repositories share a build home: %s", a) 29 t.Fatalf("two repositories share a build home: %s", a)
24 } 30 }
31 again, done, _ := buildHome(work, job{ID: 3, Repo: "alice/app", Trusted: true})
32 done()
33 if again != a {
34 t.Fatalf("build home moved between builds: %s then %s", a, again)
35 }
25 for _, dir := range []string{a, b} { 36 for _, dir := range []string{a, b} {
26 rel, err := filepath.Rel(filepath.Join(work, "home"), dir) 37 rel, err := filepath.Rel(filepath.Join(work, "trusted-home"), dir)
27 if err != nil || rel == "." || filepath.IsAbs(rel) || rel[0] == '.' { 38 if err != nil || rel == "." || strings.HasPrefix(rel, "..") {
28 t.Fatalf("build home %s is not under %s/home", dir, work) 39 t.Fatalf("build home %s is not under %s/trusted-home", dir, work)
29 } 40 }
30 st, err := os.Stat(dir) 41 st, err := os.Stat(dir)
31 if err != nil { 42 if err != nil {
32 t.Fatalf("build home not created: %v", err) 43 t.Fatal(err)
33 } 44 }
34 if st.Mode().Perm() != 0o700 { 45 if st.Mode().Perm() != 0o700 {
35 t.Fatalf("build home mode %o, want 0700", st.Mode().Perm()) 46 t.Fatalf("build home mode %o, want 0700", st.Mode().Perm())
36 } 47 }
37 } 48 }
38 // The same repository gets the same home back: that is what makes it 49}
39 // a cache. 50
40 again, _ := buildHomeFor(work, "alice/app") 51// An untrusted build gets a home of its own, outside the trusted root,
41 if again != a { 52// removed when the build ends: nothing a fork's build writes reaches a
42 t.Fatalf("build home moved between builds: %s then %s", a, again) 53// later build of the repository (#255).
54func TestUntrustedHomeIsDisposable(t *testing.T) {
55 work := t.TempDir()
56 trusted, done, err := buildHome(work, job{ID: 1, Repo: "alice/app", Trusted: true})
57 if err != nil {
58 t.Fatal(err)
59 }
60 done()
61 home, done, err := buildHome(work, job{ID: 2, Repo: "alice/app"})
62 if err != nil {
63 t.Fatal(err)
64 }
65 if home == trusted || strings.HasPrefix(home, filepath.Join(work, "trusted-home")) {
66 t.Fatalf("untrusted build got a trusted home: %s", home)
67 }
68 // What the Go module cache leaves behind: read-only directories.
69 cache := filepath.Join(home, "go", "pkg", "mod", "example.com", "m@v1")
70 if err := os.MkdirAll(cache, 0o755); err != nil {
71 t.Fatal(err)
72 }
73 if err := os.WriteFile(filepath.Join(cache, "go.mod"), []byte("module m\n"), 0o444); err != nil {
74 t.Fatal(err)
75 }
76 os.Chmod(cache, 0o555)
77 os.Chmod(filepath.Dir(cache), 0o555)
78 done()
79 if _, err := os.Stat(home); !os.IsNotExist(err) {
80 t.Fatalf("untrusted home left behind: %v", err)
43 } 81 }
44} 82}
45 83
46// A repository path is server-validated, but the home must still never 84// A repository path is server-validated, but a trusted home must still
47// resolve outside the runner's home root. 85// never resolve outside the runner's home root.
48func TestBuildHomeRefusesTraversal(t *testing.T) { 86func TestBuildHomeRefusesTraversal(t *testing.T) {
49 work := t.TempDir() 87 if _, _, err := buildHome(t.TempDir(), job{Repo: "../../etc", Trusted: true}); err == nil {
50 if _, err := buildHomeFor(work, "../../etc"); err == nil {
51 t.Fatal("a traversing repository path produced a build home") 88 t.Fatal("a traversing repository path produced a build home")
52 } 89 }
53} 90}
cmd/gitbay-runner/main.go +79 −46
@@ -14,6 +14,7 @@ import (
14 "flag" 14 "flag"
15 "fmt" 15 "fmt"
16 "io" 16 "io"
17 "io/fs"
17 "log" 18 "log"
18 "os" 19 "os"
19 "os/exec" 20 "os/exec"
@@ -30,14 +31,18 @@ import (
30) 31)
31 32
32type job struct { 33type job struct {
33 ID int64 `json:"id"` 34 ID int64 `json:"id"`
34 Repo string `json:"repo"` 35 Repo string `json:"repo"`
35 Number int64 `json:"number"` 36 Number int64 `json:"number"`
36 Job string `json:"job"` 37 Job string `json:"job"`
37 SHA string `json:"sha"` 38 SHA string `json:"sha"`
38 Ref string `json:"ref"` 39 Ref string `json:"ref"`
39 Steps []string `json:"steps"` 40 Steps []string `json:"steps"`
40 Image string `json:"image"` 41 Image string `json:"image"`
42 // Trusted is false for a merge request head from a fork, and when the
43 // server did not say: such a build gets no secrets and a home of its
44 // own (#255).
45 Trusted bool `json:"trusted"`
41 Secrets map[string]string `json:"secrets"` 46 Secrets map[string]string `json:"secrets"`
42} 47}
43 48
@@ -312,22 +317,12 @@ func (r *runner) run(j job) bool {
312 dir := filepath.Join(r.workdir, fmt.Sprintf("build-%d", j.ID)) 317 dir := filepath.Join(r.workdir, fmt.Sprintf("build-%d", j.ID))
313 defer os.RemoveAll(dir) 318 defer os.RemoveAll(dir)
314 319
315 // A build's HOME. Not the workspace, which is removed after every 320 home, doneHome, err := buildHome(r.workdir, j)
316 // build: the Go module cache, the sonar scanner and every other tool
317 // cache live under HOME, so a per-build one re-downloads the world
318 // each time. Not the runner's own home either, where its SSH key and
319 // credential dotfiles are. A directory beside the workspaces is
320 // neither.
321 //
322 // One per repository: shared across repositories, a step could poison
323 // the module cache or plant a .gitconfig that another repository's
324 // build would honour, and the container mounts the home read-write
325 // (#184).
326 buildHome, err := buildHomeFor(r.workdir, j.Repo)
327 if err != nil { 321 if err != nil {
328 log.Printf("build %d: build home: %v", j.ID, err) 322 log.Printf("build %d: build home: %v", j.ID, err)
329 return false 323 return false
330 } 324 }
325 defer doneHome()
331 326
332 // One long-lived `runner log` session receives the whole stream. 327 // One long-lived `runner log` session receives the whole stream.
333 logCmd := exec.Command(toolpath.Look("ssh"), append(r.sshOpts, r.remote, "runner", "log", fmt.Sprint(j.ID))...) 328 logCmd := exec.Command(toolpath.Look("ssh"), append(r.sshOpts, r.remote, "runner", "log", fmt.Sprint(j.ID))...)
@@ -430,36 +425,62 @@ func (r *runner) run(j job) bool {
430 } 425 }
431 } 426 }
432 427
433 env := stepEnv(j, buildHome, r.buildSSH()) 428 env := stepEnv(j, home, r.buildSSH())
434 return r.runSteps(j, dir, env, sink, deadline, runStep) 429 return r.runSteps(j, dir, env, sink, deadline, runStep)
435} 430}
436 431
437// stepEnv builds the environment a build step runs with. It is 432// buildHome is a build's HOME and what to do with it when the build ends.
438// constructed, not inherited: os.Environ() would hand repository content
439// the runner's entire environment, including anything an operator set on
440// the service (#144).
441// 433//
442// HOME is the repository's build home, not the runner's own: tools read 434// Not the workspace, which is removed after every build: the Go module
443// credentials out of dotfiles — .netrc, .npmrc, .gitconfig — and a build 435// cache and every other tool cache live under HOME. Not the runner's own
444// has no business finding the runner's. It is not the workspace either, 436// home either, where its SSH key and credential dotfiles are.
445// because the workspace is deleted after every build and every tool
446// cache lives under HOME.
447// 437//
448// PATH is the one thing carried over: without it a step cannot find the 438// A trusted build gets its repository's home,
449// tools the host was provisioned with. 439// <workdir>/trusted-home/<owner>/<name>, kept between builds so the
450// buildHomeFor is the build home for one repository: <workdir>/home/<owner>/<name>, 440// caches survive. One per repository: shared across repositories, a step
451// created on first use. The repository path comes from the server, but a 441// could poison a cache or plant a .gitconfig that another repository's
452// home must still never resolve outside the home root. 442// build would honour (#184). The root is not <workdir>/home, where homes
453func buildHomeFor(workdir, repo string) (string, error) { 443// that untrusted builds could write were kept before #255, so none of
454 root := filepath.Join(workdir, "home") 444// those is read again.
455 dir := filepath.Join(root, filepath.FromSlash(repo)) 445//
446// An untrusted build gets <workdir>/build-<id>-home, new and empty,
447// removed when the build ends. The container mounts HOME read-write, so
448// a home a fork's build could write is a cache a stranger controls
449// (#255).
450func buildHome(workdir string, j job) (string, func(), error) {
451 if !j.Trusted {
452 dir := filepath.Join(workdir, fmt.Sprintf("build-%d-home", j.ID))
453 if err := os.Mkdir(dir, 0o700); err != nil {
454 return "", nil, err
455 }
456 return dir, func() {
457 if err := removeTree(dir); err != nil {
458 log.Printf("build %d: removing its home: %v", j.ID, err)
459 }
460 }, nil
461 }
462 root := filepath.Join(workdir, "trusted-home")
463 dir := filepath.Join(root, filepath.FromSlash(j.Repo))
456 if rel, err := filepath.Rel(root, dir); err != nil || rel == "." || strings.HasPrefix(rel, "..") { 464 if rel, err := filepath.Rel(root, dir); err != nil || rel == "." || strings.HasPrefix(rel, "..") {
457 return "", fmt.Errorf("repository path %q escapes the build home root", repo) 465 return "", nil, fmt.Errorf("repository path %q escapes the build home root", j.Repo)
458 } 466 }
459 if err := os.MkdirAll(dir, 0o700); err != nil { 467 if err := os.MkdirAll(dir, 0o700); err != nil {
460 return "", err 468 return "", nil, err
461 } 469 }
462 return dir, nil 470 return dir, func() {}, nil
471}
472
473// removeTree deletes dir and everything under it. os.RemoveAll alone
474// fails on a directory without write permission, and the Go module cache
475// makes every directory it fills read-only.
476func removeTree(dir string) error {
477 filepath.WalkDir(dir, func(p string, d fs.DirEntry, err error) error {
478 if err == nil && d.IsDir() {
479 os.Chmod(p, 0o700)
480 }
481 return nil
482 })
483 return os.RemoveAll(dir)
463} 484}
464 485
465// buildSSH is the instance's ssh destination as a build reaches it. Under 486// buildSSH is the instance's ssh destination as a build reaches it. Under
@@ -485,6 +506,17 @@ func (r *runner) buildSSH() string {
485 return "169.254.1.2" 506 return "169.254.1.2"
486} 507}
487 508
509// stepEnv builds the environment a build step runs with. It is
510// constructed, not inherited: os.Environ() would hand repository content
511// the runner's entire environment, including anything an operator set on
512// the service (#144).
513//
514// HOME is the build's home (buildHome), not the runner's own: tools read
515// credentials out of dotfiles — .netrc, .npmrc, .gitconfig — and a build
516// has no business finding the runner's.
517//
518// PATH is the one thing carried over: without it a step cannot find the
519// tools the host was provisioned with.
488func stepEnv(j job, home, sshDest string) []string { 520func stepEnv(j job, home, sshDest string) []string {
489 path := os.Getenv("PATH") 521 path := os.Getenv("PATH")
490 if path == "" { 522 if path == "" {
@@ -501,11 +533,12 @@ func stepEnv(j job, home, sshDest string) []string {
501 "GITBAY_JOB=" + j.Job, 533 "GITBAY_JOB=" + j.Job,
502 "GITBAY_SSH=" + sshDest, 534 "GITBAY_SSH=" + sshDest,
503 } 535 }
504 // The server sends secrets only for a trusted build — a merge request 536 // The server sends secrets only for a trusted build. The claim's
505 // head from a fork arrives with none — so this loop is empty exactly 537 // trust flag decides here as well, not whether any arrived (#255).
506 // when it should be. 538 if j.Trusted {
507 for name, value := range j.Secrets { 539 for name, value := range j.Secrets {
508 env = append(env, name+"="+value) 540 env = append(env, name+"="+value)
541 }
509 } 542 }
510 return env 543 return env
511} 544}
internal/control/build.go +13 −9
@@ -552,16 +552,20 @@ func runRunnerNext(c *Ctx, args []string) int {
552 } 552 }
553 } 553 }
554 d := struct { 554 d := struct {
555 ID int64 `json:"id"` 555 ID int64 `json:"id"`
556 Repo string `json:"repo"` 556 Repo string `json:"repo"`
557 Number int64 `json:"number"` 557 Number int64 `json:"number"`
558 Job string `json:"job"` 558 Job string `json:"job"`
559 SHA string `json:"sha"` 559 SHA string `json:"sha"`
560 Ref string `json:"ref"` 560 Ref string `json:"ref"`
561 Steps []string `json:"steps"` 561 Steps []string `json:"steps"`
562 Image string `json:"image,omitempty"` 562 Image string `json:"image,omitempty"`
563 // Trusted is always sent: a runner decides a build's home and
564 // secrets from it, and reads a missing field as untrusted (#255).
565 Trusted bool `json:"trusted"`
563 Secrets map[string]string `json:"secrets,omitempty"` 566 Secrets map[string]string `json:"secrets,omitempty"`
564 }{b.ID, repo.Path(), b.Number, b.Job, b.SHA, b.Ref, steps, b.Image, secrets} 567 }{ID: b.ID, Repo: repo.Path(), Number: b.Number, Job: b.Job, SHA: b.SHA, Ref: b.Ref,
568 Steps: steps, Image: b.Image, Trusted: b.Trusted, Secrets: secrets}
565 return c.emit(d, func(w io.Writer) { 569 return c.emit(d, func(w io.Writer) {
566 fmt.Fprintf(w, "build %d: %s %s @ %.10s\n", d.ID, d.Repo, d.Job, d.SHA) 570 fmt.Fprintf(w, "build %d: %s %s @ %.10s\n", d.ID, d.Repo, d.Job, d.SHA)
567 }) 571 })
internal/control/runnernext_test.go +21
@@ -245,3 +245,24 @@ func TestRunnerLogMarksStreamClosed(t *testing.T) {
245 t.Errorf("status %s, want still running until the runner reports", got.Status) 245 t.Errorf("status %s, want still running until the runner reports", got.Status)
246 } 246 }
247} 247}
248
249// The claim says whether a build is trusted in so many words. A runner
250// must not infer it from secrets being absent: a trusted repository with
251// no secrets looks the same (#255).
252func TestRunnerNextSaysWhetherTrusted(t *testing.T) {
253 st, repo, uid, root, baseSHA, _ := setupOrphanRepo(t)
254 for _, trusted := range []bool{true, false} {
255 if _, err := st.CreateBuild(repo.ID, "unit", baseSHA, "main", "[]", "", "", trusted); err != nil {
256 t.Fatal(err)
257 }
258 c, out := runnerCtx(st, uid, root)
259 c.JSON = true
260 if code := runRunnerNext(c, []string{"--untrusted"}); code != protocol.ExitOK {
261 t.Fatalf("runner next: exit %d, output:\n%s", code, out.String())
262 }
263 want := fmt.Sprintf(`"trusted":%v`, trusted)
264 if !strings.Contains(out.String(), want) {
265 t.Fatalf("claim of a trusted=%v build lacks %s:\n%s", trusted, want, out.String())
266 }
267 }
268}