Quote args and plan path that may contain spaces.

Bitbucket repo owner names can contain spaces. Need to quote everywhere these might be used because we're executing with sh -c.
cshabi · Oct 2, 2018 · e98caf3 · e98caf3
1 parent 5f26c81
commit e98caf3
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 32 deletions.
diff --git a/server/events/runtime/apply_step_runner.go b/server/events/runtime/apply_step_runner.go
@@ -21,7 +21,9 @@ func (a *ApplyStepRunner) Run(ctx models.ProjectCommandContext, extraArgs []stri
 		return "", fmt.Errorf("no plan found at path %q and workspace %q–did you run plan?", ctx.RepoRelDir, ctx.Workspace)
 	}
 
-	tfApplyCmd := append(append(append([]string{"apply", "-input=false", "-no-color"}, extraArgs...), ctx.CommentArgs...), planPath)
+	// NOTE: we need to quote the plan path because Bitbucket Server can
+	// have spaces in its repo owner names which is part of the path.
+	tfApplyCmd := append(append(append([]string{"apply", "-input=false", "-no-color"}, extraArgs...), ctx.CommentArgs...), fmt.Sprintf("%q", planPath))
 	var tfVersion *version.Version
 	if ctx.ProjectConfig != nil && ctx.ProjectConfig.TerraformVersion != nil {
 		tfVersion = ctx.ProjectConfig.TerraformVersion

diff --git a/server/events/runtime/apply_step_runner_test.go b/server/events/runtime/apply_step_runner_test.go
@@ -1,6 +1,7 @@
 package runtime_test
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -63,7 +64,7 @@ func TestRun_Success(t *testing.T) {
 	}, []string{"extra", "args"}, tmpDir)
 	Ok(t, err)
 	Equals(t, "output", output)
-	terraform.VerifyWasCalledOnce().RunCommandWithVersion(nil, tmpDir, []string{"apply", "-input=false", "-no-color", "extra", "args", "comment", "args", planPath}, nil, "workspace")
+	terraform.VerifyWasCalledOnce().RunCommandWithVersion(nil, tmpDir, []string{"apply", "-input=false", "-no-color", "extra", "args", "comment", "args", fmt.Sprintf("%q", planPath)}, nil, "workspace")
 	_, err = os.Stat(planPath)
 	Assert(t, os.IsNotExist(err), "planfile should be deleted")
 }
@@ -95,7 +96,7 @@ func TestRun_AppliesCorrectProjectPlan(t *testing.T) {
 	}, []string{"extra", "args"}, tmpDir)
 	Ok(t, err)
 	Equals(t, "output", output)
-	terraform.VerifyWasCalledOnce().RunCommandWithVersion(nil, tmpDir, []string{"apply", "-input=false", "-no-color", "extra", "args", "comment", "args", planPath}, nil, "default")
+	terraform.VerifyWasCalledOnce().RunCommandWithVersion(nil, tmpDir, []string{"apply", "-input=false", "-no-color", "extra", "args", "comment", "args", fmt.Sprintf("%q", planPath)}, nil, "default")
 	_, err = os.Stat(planPath)
 	Assert(t, os.IsNotExist(err), "planfile should be deleted")
 }
@@ -126,7 +127,7 @@ func TestRun_UsesConfiguredTFVersion(t *testing.T) {
 	}, []string{"extra", "args"}, tmpDir)
 	Ok(t, err)
 	Equals(t, "output", output)
-	terraform.VerifyWasCalledOnce().RunCommandWithVersion(nil, tmpDir, []string{"apply", "-input=false", "-no-color", "extra", "args", "comment", "args", planPath}, tfVersion, "workspace")
+	terraform.VerifyWasCalledOnce().RunCommandWithVersion(nil, tmpDir, []string{"apply", "-input=false", "-no-color", "extra", "args", "comment", "args", fmt.Sprintf("%q", planPath)}, tfVersion, "workspace")
 	_, err = os.Stat(planPath)
 	Assert(t, os.IsNotExist(err), "planfile should be deleted")
 }
diff --git a/server/events/runtime/plan_step_runner.go b/server/events/runtime/plan_step_runner.go
@@ -117,15 +117,17 @@ func (p *PlanStepRunner) buildPlanCmd(ctx models.ProjectCommandContext, extraArg
 func (p *PlanStepRunner) tfVars(ctx models.ProjectCommandContext) []string {
 	// NOTE: not using maps and looping here because we need to keep the
 	// ordering for testing purposes.
+	// NOTE: quoting the values because in Bitbucket the owner can have
+	// spaces, ex -var atlantis_repo_owner="bitbucket owner".
 	return []string{
 		"-var",
-		fmt.Sprintf("%s=%s", "atlantis_user", ctx.User.Username),
+		fmt.Sprintf("%s=%q", "atlantis_user", ctx.User.Username),
 		"-var",
-		fmt.Sprintf("%s=%s", "atlantis_repo", ctx.BaseRepo.FullName),
+		fmt.Sprintf("%s=%q", "atlantis_repo", ctx.BaseRepo.FullName),
 		"-var",
-		fmt.Sprintf("%s=%s", "atlantis_repo_name", ctx.BaseRepo.Name),
+		fmt.Sprintf("%s=%q", "atlantis_repo_name", ctx.BaseRepo.Name),
 		"-var",
-		fmt.Sprintf("%s=%s", "atlantis_repo_owner", ctx.BaseRepo.Owner),
+		fmt.Sprintf("%s=%q", "atlantis_repo_owner", ctx.BaseRepo.Owner),
 		"-var",
 		fmt.Sprintf("%s=%d", "atlantis_pull_num", ctx.Pull.Num),
 	}

diff --git a/server/events/runtime/plan_step_runner_test.go b/server/events/runtime/plan_step_runner_test.go
@@ -63,13 +63,13 @@ func TestRun_NoWorkspaceIn08(t *testing.T) {
 			"-out",
 			"\"/path/default.tfplan\"",
 			"-var",
-			"atlantis_user=username",
+			"atlantis_user=\"username\"",
 			"-var",
-			"atlantis_repo=owner/repo",
+			"atlantis_repo=\"owner/repo\"",
 			"-var",
-			"atlantis_repo_name=repo",
+			"atlantis_repo_name=\"repo\"",
 			"-var",
-			"atlantis_repo_owner=owner",
+			"atlantis_repo_owner=\"owner\"",
 			"-var",
 			"atlantis_pull_num=2",
 			"extra",
@@ -198,13 +198,13 @@ func TestRun_SwitchesWorkspace(t *testing.T) {
 					"-out",
 					"\"/path/workspace.tfplan\"",
 					"-var",
-					"atlantis_user=username",
+					"atlantis_user=\"username\"",
 					"-var",
-					"atlantis_repo=owner/repo",
+					"atlantis_repo=\"owner/repo\"",
 					"-var",
-					"atlantis_repo_name=repo",
+					"atlantis_repo_name=\"repo\"",
 					"-var",
-					"atlantis_repo_owner=owner",
+					"atlantis_repo_owner=\"owner\"",
 					"-var",
 					"atlantis_pull_num=2",
 					"extra",
@@ -267,13 +267,13 @@ func TestRun_CreatesWorkspace(t *testing.T) {
 				"-out",
 				"\"/path/workspace.tfplan\"",
 				"-var",
-				"atlantis_user=username",
+				"atlantis_user=\"username\"",
 				"-var",
-				"atlantis_repo=owner/repo",
+				"atlantis_repo=\"owner/repo\"",
 				"-var",
-				"atlantis_repo_name=repo",
+				"atlantis_repo_name=\"repo\"",
 				"-var",
-				"atlantis_repo_owner=owner",
+				"atlantis_repo_owner=\"owner\"",
 				"-var",
 				"atlantis_pull_num=2",
 				"extra",
@@ -327,13 +327,13 @@ func TestRun_NoWorkspaceSwitchIfNotNecessary(t *testing.T) {
 		"-out",
 		"\"/path/workspace.tfplan\"",
 		"-var",
-		"atlantis_user=username",
+		"atlantis_user=\"username\"",
 		"-var",
-		"atlantis_repo=owner/repo",
+		"atlantis_repo=\"owner/repo\"",
 		"-var",
-		"atlantis_repo_name=repo",
+		"atlantis_repo_name=\"repo\"",
 		"-var",
-		"atlantis_repo_owner=owner",
+		"atlantis_repo_owner=\"owner\"",
 		"-var",
 		"atlantis_pull_num=2",
 		"extra",
@@ -395,13 +395,13 @@ func TestRun_AddsEnvVarFile(t *testing.T) {
 		"-out",
 		fmt.Sprintf("%q", filepath.Join(tmpDir, "workspace.tfplan")),
 		"-var",
-		"atlantis_user=username",
+		"atlantis_user=\"username\"",
 		"-var",
-		"atlantis_repo=owner/repo",
+		"atlantis_repo=\"owner/repo\"",
 		"-var",
-		"atlantis_repo_name=repo",
+		"atlantis_repo_name=\"repo\"",
 		"-var",
-		"atlantis_repo_owner=owner",
+		"atlantis_repo_owner=\"owner\"",
 		"-var",
 		"atlantis_pull_num=2",
 		"extra",
@@ -456,13 +456,13 @@ func TestRun_UsesDiffPathForProject(t *testing.T) {
 		"-out",
 		"\"/path/projectname-default.tfplan\"",
 		"-var",
-		"atlantis_user=username",
+		"atlantis_user=\"username\"",
 		"-var",
-		"atlantis_repo=owner/repo",
+		"atlantis_repo=\"owner/repo\"",
 		"-var",
-		"atlantis_repo_name=repo",
+		"atlantis_repo_name=\"repo\"",
 		"-var",
-		"atlantis_repo_owner=owner",
+		"atlantis_repo_owner=\"owner\"",
 		"-var",
 		"atlantis_pull_num=2",
 		"extra",