Commit 94f55ffbcd

94f55ffbcd8c6d53c5255e49217e06bb2692724e

parent: ca8187d6b5

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-28 06:31 UTC

runner: disposable home for untrusted builds

A trusted build keeps its repository's home, now under
<workdir>/trusted-home; an untrusted build gets a new home removed with
the build, and no secrets whatever the claim carries.

Ref #255

Layout: unified · split

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}