diff --git a/server/worker/docker/docker.go b/server/worker/docker/docker.go index 4f497fcee..b2eda0c74 100644 --- a/server/worker/docker/docker.go +++ b/server/worker/docker/docker.go @@ -102,15 +102,6 @@ func (d *Docker) Do(c context.Context, r *worker.Work) { log.Printf("Error parsing YAML for %s/%s, Err: %s", r.Repo.Owner, r.Repo.Name, err.Error()) } - // append private parameters to the environment - // variable section of the .drone.yml file, iff - // this is not a pull request (for security purposes) - if params != nil && (r.Repo.Private || len(r.Commit.PullRequest) == 0) { - for k, v := range params { - script.Env = append(script.Env, k+"="+v) - } - } - // TODO: handle error better? buildNumber, err := datastore.GetBuildNumber(c, r.Commit) if err != nil { @@ -121,13 +112,23 @@ func (d *Docker) Do(c context.Context, r *worker.Work) { path := r.Repo.Host + "/" + r.Repo.Owner + "/" + r.Repo.Name repo := &repo.Repo{ - Name: path, - Path: r.Repo.CloneURL, - Branch: r.Commit.Branch, - Commit: r.Commit.Sha, - PR: r.Commit.PullRequest, - Dir: filepath.Join("/var/cache/drone/src", git.GitPath(script.Git, path)), - Depth: git.GitDepth(script.Git), + Name: path, + Path: r.Repo.CloneURL, + Branch: r.Commit.Branch, + Commit: r.Commit.Sha, + PR: r.Commit.PullRequest, + Private: r.Repo.Private, + Dir: filepath.Join("/var/cache/drone/src", git.GitPath(script.Git, path)), + Depth: git.GitDepth(script.Git), + } + + // append private parameters to the environment + // variable section of the .drone.yml file, if + // this is trusted + if params != nil && repo.IsTrusted() { + for k, v := range params { + script.Env = append(script.Env, k+"="+v) + } } priorCommit, _ := datastore.GetCommitPrior(c, r.Commit) @@ -152,7 +153,7 @@ func (d *Docker) Do(c context.Context, r *worker.Work) { builder.Timeout = time.Duration(r.Repo.Timeout) * time.Second builder.Privileged = r.Repo.Privileged - if r.Repo.Private || len(r.Commit.PullRequest) == 0 { + if repo.IsTrusted() { builder.Key = []byte(r.Repo.PrivateKey) } diff --git a/shared/build/build.go b/shared/build/build.go index b37f032dc..37433c6af 100644 --- a/shared/build/build.go +++ b/shared/build/build.go @@ -327,7 +327,7 @@ func (b *Builder) run() error { // configure if Docker should run in privileged mode host := docker.HostConfig{ - Privileged: (b.Privileged && script.DockerPrivileged(b.Build.Docker)), + Privileged: (b.Privileged && b.Repo.IsTrusted()), } if host.Privileged { diff --git a/shared/build/build_test.go b/shared/build/build_test.go index b00d464e6..cc9bcf152 100644 --- a/shared/build/build_test.go +++ b/shared/build/build_test.go @@ -412,21 +412,39 @@ func TestRunPrivileged(t *testing.T) { t.Errorf("Expected container NOT started in Privileged mode") } - // now lets try to enable priviliged mode + // now lets set priviliged mode b.Privileged = true b.run() - if conf.Privileged != false { - t.Errorf("Expected container NOT started in Privileged mode when no setup in build script") - } - - // now lets set priviliged mode in build script - b.Privileged = true - b.Build = &script.Build{Docker: &script.Docker{Privileged: true}} - b.run() - if conf.Privileged != true { - t.Errorf("Expected container IS started in Privileged mode when setup privileged mode in build script") + t.Errorf("Expected container IS started in Privileged mode") + } + + // now lets set priviliged mode but for a pull request + b.Privileged = true + b.Repo.PR = "55" + b.run() + + if conf.Privileged != false { + t.Errorf("Expected container NOT started in Privileged mode when PR") + } + + // now lets set priviliged mode for a pull request from public repo + b.Privileged = true + b.Repo.Private = false + b.run() + + if conf.Privileged != false { + t.Errorf("Expected container NOT started in Privileged mode when PR from public repo") + } + + // now lets set priviliged mode for a pull request from private repo + b.Privileged = true + b.Repo.Private = true + b.run() + + if conf.Privileged != true { + t.Errorf("Expected container started in Privileged mode when PR from private repo") } } diff --git a/shared/build/repo/repo.go b/shared/build/repo/repo.go index 82973efa1..01daa073c 100644 --- a/shared/build/repo/repo.go +++ b/shared/build/repo/repo.go @@ -33,6 +33,9 @@ type Repo struct { // checkout when the Repository is cloned. PR string + // Private specifies if a git repo is private or not + Private bool + // (optional) The filesystem path that the repository // will be cloned into (or copied to) inside the // host system (Docker Container). @@ -125,3 +128,8 @@ func (r *Repo) Commands() []string { return cmds } + +// IsTrusted returns if a repo is trusted to run under privileged mode +func (r *Repo) IsTrusted() bool { + return r.Private || len(r.PR) == 0 +} diff --git a/shared/build/repo/repo_test.go b/shared/build/repo/repo_test.go index b8f4d54ef..972cd30af 100644 --- a/shared/build/repo/repo_test.go +++ b/shared/build/repo/repo_test.go @@ -52,3 +52,23 @@ func TestIsGit(t *testing.T) { } } } + +func TestIsTrusted(t *testing.T) { + repos := []struct { + private bool + PR string + trusted bool + }{ + {true, "1", true}, + {false, "1", false}, + {true, "", true}, + {false, "", true}, + } + + for _, r := range repos { + repo := Repo{Private: r.private, PR: r.PR} + if trusted := repo.IsTrusted(); trusted != r.trusted { + t.Errorf("IsTrusted was %v, expected %v", trusted, r.trusted) + } + } +} diff --git a/shared/build/script/docker.go b/shared/build/script/docker.go index d2a9ad665..329d5ab60 100644 --- a/shared/build/script/docker.go +++ b/shared/build/script/docker.go @@ -17,10 +17,6 @@ type Docker struct { // Allocate a pseudo-TTY (also known as `--tty` option) TTY bool `yaml:"tty,omitempty"` - - // Privileged means enabling all devices on the host in container - // (also known as `--privileged=true` option) - Privileged bool `yaml:"privileged,omitempty"` } // DockerNetworkMode returns DefaultNetworkMode @@ -34,7 +30,7 @@ func DockerNetworkMode(d *Docker) string { return *d.NetworkMode } -// DockerNetworkMode returns empty string +// DockerHostname returns empty string // when Docker.NetworkMode is empty. // DockerNetworkMode returns Docker.NetworkMode // when it is not empty. @@ -53,12 +49,3 @@ func DockerTty(d *Docker) bool { } return d.TTY } - -// DockerPrivileged returns true if the build -// should have privileged mode -func DockerPrivileged(d *Docker) bool { - if d == nil { - return false - } - return d.Privileged -}