diff --git a/cmd/ciReleaseDeploy.go b/cmd/ciReleaseDeploy.go index 605a06f..1309dec 100644 --- a/cmd/ciReleaseDeploy.go +++ b/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, diff --git a/cmd/ciReleaseDiff.go b/cmd/ciReleaseDiff.go index 480a5ea..afbfa4e 100644 --- a/cmd/ciReleaseDiff.go +++ b/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, diff --git a/cmd/ciReleaseValidate.go b/cmd/ciReleaseValidate.go index 5983110..fa8d981 100644 --- a/cmd/ciReleaseValidate.go +++ b/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 { diff --git a/cmd/root.go b/cmd/root.go index fefe43f..1edd558 100644 --- a/cmd/root.go +++ b/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) diff --git a/internal/common/shell.go b/internal/common/shell.go new file mode 100644 index 0000000..b80edcb --- /dev/null +++ b/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, "'", `'\''`) +} diff --git a/tests/release_test.go b/tests/release_test.go index d7d90ea..312c78d 100644 --- a/tests/release_test.go +++ b/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) +}