Merge pull request #1098 from shawnzhu/privileged

trusted builds from private repo or non pull requests can run under privileged mode

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 {
@@ -126,10 +117,20 @@ func (d *Docker) Do(c context.Context, r *worker.Work) {
 		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)
 
 	// send all "started" notifications
@@ -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)
 	}
 

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 {

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) {

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
+}

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)
+		}
+	}
+}

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.