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 0a0265ab4..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 && len(b.Repo.PR) == 0), + Privileged: (b.Privileged && b.Repo.IsTrusted()), } if host.Privileged { diff --git a/shared/build/build_test.go b/shared/build/build_test.go index 636fff442..cc9bcf152 100644 --- a/shared/build/build_test.go +++ b/shared/build/build_test.go @@ -428,6 +428,24 @@ func TestRunPrivileged(t *testing.T) { 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") + } } func TestRunErrorCreate(t *testing.T) { 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 a61967922..329d5ab60 100644 --- a/shared/build/script/docker.go +++ b/shared/build/script/docker.go @@ -30,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.