Escape branch name in release scripts; honour --debug in validate

cmd/ciReleaseDeploy.go

@@ -250,7 +250,7 @@ var ciReleaseDeployCmd = &cobra.Command{
 				--timeout "${DEPLOYMENT_TIMEOUT}" \
 				--wait`,
 				releaseName, chartName, chartRepository, chartVersionOverride,
-				siltaEnvironmentName, branchname, nginxImageUrl,
+				common.EscapeSingleQuoted(siltaEnvironmentName), common.EscapeSingleQuoted(branchname), nginxImageUrl,
 				clusterDomain, extraNoAuthIPs, vpcNativeOverride, extraClusterType,
 				namespace, siltaConfig, helmFlags, deploymentTimeout)
 			pipedExec(command, "", "ERROR: ", debug)
@@ -367,7 +367,7 @@ var ciReleaseDeployCmd = &cobra.Command{
 				rm -f helm-output.log || true
 				`,
 				releaseName, chartName, chartRepository, chartVersionOverride,
-				siltaEnvironmentName, branchname,
+				common.EscapeSingleQuoted(siltaEnvironmentName), common.EscapeSingleQuoted(branchname),
 				repositoryUrl, gitAuthUsername, gitAuthPassword,
 				clusterDomain, namespace,
 				extraNoAuthIPs, vpcNativeOverride, extraClusterType,
@@ -565,7 +565,7 @@ var ciReleaseDeployCmd = &cobra.Command{
 				rm -f helm-output.log || true
 				`,
 				releaseName, chartName, chartRepository, chartVersionOverride,
-				siltaEnvironmentName, branchname,
+				common.EscapeSingleQuoted(siltaEnvironmentName), common.EscapeSingleQuoted(branchname),
 				phpImageUrl, nginxImageUrl, shellImageUrl,
 				repositoryUrl, gitAuthUsername, gitAuthPassword,
 				clusterDomain, extraNoAuthIPs, vpcNativeOverride, extraClusterType,

cmd/ciReleaseDiff.go

@@ -194,7 +194,7 @@ var ciReleaseDiffCmd = &cobra.Command{
 				--values "${SILTA_CONFIG}" \
 				${EXTRA_HELM_FLAGS}`,
 				releaseName, chartName, chartRepository, chartVersionOverride,
-				siltaEnvironmentName, branchname, nginxImageUrl,
+				common.EscapeSingleQuoted(siltaEnvironmentName), common.EscapeSingleQuoted(branchname), nginxImageUrl,
 				clusterDomain, extraNoAuthIPs, vpcNativeOverride, extraClusterType,
 				namespace, siltaConfig, helmFlags)
 			pipedExec(command, "", "ERROR: ", debug)
@@ -249,7 +249,7 @@ var ciReleaseDiffCmd = &cobra.Command{
 				--values "${SILTA_CONFIG}" \
 				${EXTRA_HELM_FLAGS}`,
 				releaseName, chartName, chartRepository, chartVersionOverride,
-				siltaEnvironmentName, branchname,
+				common.EscapeSingleQuoted(siltaEnvironmentName), common.EscapeSingleQuoted(branchname),
 				repositoryUrl, gitAuthUsername, gitAuthPassword,
 				clusterDomain, namespace,
 				extraNoAuthIPs, vpcNativeOverride, extraClusterType,
@@ -352,7 +352,7 @@ var ciReleaseDiffCmd = &cobra.Command{
 				--values "${SILTA_CONFIG}" \
 				${EXTRA_HELM_FLAGS}`,
 				releaseName, chartName, chartRepository, chartVersionOverride,
-				siltaEnvironmentName, branchname,
+				common.EscapeSingleQuoted(siltaEnvironmentName), common.EscapeSingleQuoted(branchname),
 				phpImageUrl, nginxImageUrl, shellImageUrl,
 				repositoryUrl, gitAuthUsername, gitAuthPassword,
 				clusterDomain, extraNoAuthIPs, vpcNativeOverride, extraClusterType,

cmd/ciReleaseValidate.go

@@ -188,29 +188,33 @@ var ciReleaseValidateCmd = &cobra.Command{
 
 				`,
 				releaseName, chartName, chartRepository, chartVersionOverride,
-				siltaEnvironmentName, branchname,
+				common.EscapeSingleQuoted(siltaEnvironmentName), common.EscapeSingleQuoted(branchname),
 				namespace, siltaConfig, extraNoAuthIPs, vpcNativeOverride, extraClusterType)
 
-			cmd := exec.Command("bash", "-c", command)
-			// StdOutPipe omitted to avoid exposing secrets
-			cmdErrReader, err := cmd.StderrPipe()
-			if err != nil {
-				log.Fatal("Error (stderr pipe): ", err)
-				return
-			}
-			errScanner := bufio.NewScanner(cmdErrReader)
-			go func() {
-				for errScanner.Scan() {
-					fmt.Printf("ERROR: %s\n", errScanner.Text())
+			if debug {
+				fmt.Printf("Command (not executed): %s\n", command)
+			} else {
+				cmd := exec.Command("bash", "-c", command)
+				// StdOutPipe omitted to avoid exposing secrets
+				cmdErrReader, err := cmd.StderrPipe()
+				if err != nil {
+					log.Fatal("Error (stderr pipe): ", err)
+					return
+				}
+				errScanner := bufio.NewScanner(cmdErrReader)
+				go func() {
+					for errScanner.Scan() {
+						fmt.Printf("ERROR: %s\n", errScanner.Text())
+					}
+				}()
+				err = cmd.Start()
+				if err != nil {
+					log.Fatal("Error (Start): ", err)
+				}
+				err = cmd.Wait()
+				if err != nil {
+					log.Fatal("Error (Wait): ", err)
 				}
-			}()
-			err = cmd.Start()
-			if err != nil {
-				log.Fatal("Error (Start): ", err)
-			}
-			err = cmd.Wait()
-			if err != nil {
-				log.Fatal("Error (Wait): ", err)
 			}
 
 		} else {

cmd/root.go

@@ -69,7 +69,7 @@ func init() {
 
 func bufferedExec(command string, debug bool) {
 	if debug {
-		fmt.Sprintf("Command (not executed): %s\n", command)
+		fmt.Printf("Command (not executed): %s\n", command)
 	} else {
 		out, err := exec.Command("bash", "-c", command).CombinedOutput()
 		fmt.Printf("%s\n", out)

internal/common/shell.go

@@ -0,0 +1,12 @@
+package common
+
+import "strings"
+
+// EscapeSingleQuoted makes a value safe to embed inside a single-quoted bash
+// string literal. A single quote cannot be escaped inside single quotes, so
+// each one is rewritten as: close quote, escaped quote, reopen quote ('\”).
+// Every other character is literal inside single quotes, so nothing else
+// needs handling. The value itself is preserved exactly.
+func EscapeSingleQuoted(value string) string {
+	return strings.ReplaceAll(value, "'", `'\''`)
+}

tests/release_test.go

@@ -445,3 +445,36 @@ func TestReleaseDeployCmd(t *testing.T) {
 	// Change dir back to previous
 	os.Chdir(wd)
 }
+
+// A branch name containing shell metacharacters must stay inside its
+// single-quoted assignment in the generated release scripts. Before escaping,
+// the quote closed the literal and the rest ran as shell code.
+func TestReleaseBranchnameEscaping(t *testing.T) {
+
+	// Go to main directory
+	wd, _ := os.Getwd()
+	os.Chdir("..")
+
+	branchname := `--branchname "feature/x'; echo pwned; #"`
+
+	// All three subcommands print the generated script in debug mode, so the
+	// escaped assignments can be checked directly. Environment name is derived
+	// from the branch name and escaped the same way.
+	testString := `SILTA_ENVIRONMENT_NAME='feature/x'\''; echo pwned; #'
+			BRANCHNAME='feature/x'\''; echo pwned; #'`
+	for _, subcommand := range []string{"deploy", "diff", "validate"} {
+		imageFlags := ""
+		if subcommand != "validate" {
+			imageFlags = `--php-image-url php-image --nginx-image-url nginx-image --shell-image-url shell-image`
+		}
+		command := `ci release ` + subcommand + ` \
+			--namespace default \
+			--release-name 'test' \
+			--chart-name drupal ` + imageFlags + ` \
+			` + branchname + ` \
+			--debug`
+		CliExecTest(t, command, []string{}, testString, false)
+	}
+
+	os.Chdir(wd)
+}