From b11d467ba6b6abed79d231b2e4cefc5a8103f874 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Wed, 19 Aug 2026 11:53:17 +1000 Subject: [PATCH] fix: quote automation commands for the host shell (#72) Generated automation commands always used single quotes, which cmd.exe passes through verbatim, so the command fails to find the entity. Values are now quoted using the rules of the shell the CLI is running under, and values needing no quoting are emitted bare. The shell can be forced with `octopus config set Shell cmd`, OCTOPUS_SHELL, or --shell. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/config/list/list.go | 3 + pkg/cmd/config/set/set.go | 12 +- pkg/cmd/release/deploy/deploy_test.go | 6 +- pkg/cmd/root/root.go | 7 + pkg/config/config.go | 4 + pkg/constants/constants.go | 3 + pkg/util/flag/flag.go | 21 ++- pkg/util/flag/flag_test.go | 62 +++++++ pkg/util/shell/parent_other.go | 9 + pkg/util/shell/parent_windows.go | 29 ++++ pkg/util/shell/quote.go | 100 +++++++++++ pkg/util/shell/quote_test.go | 234 ++++++++++++++++++++++++++ pkg/util/shell/shell.go | 89 ++++++++++ pkg/util/shell/shell_test.go | 76 +++++++++ 14 files changed, 646 insertions(+), 9 deletions(-) create mode 100644 pkg/util/flag/flag_test.go create mode 100644 pkg/util/shell/parent_other.go create mode 100644 pkg/util/shell/parent_windows.go create mode 100644 pkg/util/shell/quote.go create mode 100644 pkg/util/shell/quote_test.go create mode 100644 pkg/util/shell/shell.go create mode 100644 pkg/util/shell/shell_test.go diff --git a/pkg/cmd/config/list/list.go b/pkg/cmd/config/list/list.go index 51a1d630..a58a395c 100644 --- a/pkg/cmd/config/list/list.go +++ b/pkg/cmd/config/list/list.go @@ -49,6 +49,7 @@ func listRun(cmd *cobra.Command) error { Host string `json:"host"` NoPrompt string `json:"noprompt"` OutputFormat string `json:"outputformat"` + Shell string `json:"shell"` Space string `json:"space"` } @@ -74,6 +75,8 @@ func listRun(cmd *cobra.Command) error { configData.Space = configFile.GetString(key) case strings.ToLower(constants.ConfigOutputFormat): configData.OutputFormat = configFile.GetString(key) + case strings.ToLower(constants.ConfigShell): + configData.Shell = configFile.GetString(key) default: return fmt.Errorf("the key '%s' is not a supported config option", key) } diff --git a/pkg/cmd/config/set/set.go b/pkg/cmd/config/set/set.go index e27381af..f01a12ea 100644 --- a/pkg/cmd/config/set/set.go +++ b/pkg/cmd/config/set/set.go @@ -10,6 +10,7 @@ import ( "github.com/OctopusDeploy/cli/pkg/constants" "github.com/OctopusDeploy/cli/pkg/factory" "github.com/OctopusDeploy/cli/pkg/question" + "github.com/OctopusDeploy/cli/pkg/util/shell" "github.com/spf13/cobra" "github.com/spf13/viper" ) @@ -67,13 +68,19 @@ func setRun(isPromptEnabled bool, ask question.Asker, key string, value string) key = k } key = strings.ToLower(key) - if key == strings.ToLower(constants.ConfigNoPrompt) { + switch key { + case strings.ToLower(constants.ConfigNoPrompt): boolValue, err := strconv.ParseBool(value) if err != nil { return fmt.Errorf("the provided value %s is not valid for NoPrompt, please use true of false", value) } localViper.Set(key, boolValue) - } else { + case strings.ToLower(constants.ConfigShell): + if err := shell.Validate(value); err != nil { + return err + } + localViper.Set(key, value) + default: localViper.Set(key, value) } if err := localViper.WriteConfig(); err != nil { @@ -91,6 +98,7 @@ func promptMissing(ask question.Asker, key string) (string, string, error) { constants.ConfigOutputFormat, constants.ConfigShowOctopus, constants.ConfigEditor, + constants.ConfigShell, // constants.ConfigProxyUrl, } diff --git a/pkg/cmd/release/deploy/deploy_test.go b/pkg/cmd/release/deploy/deploy_test.go index fde01017..91e6b6e8 100644 --- a/pkg/cmd/release/deploy/deploy_test.go +++ b/pkg/cmd/release/deploy/deploy_test.go @@ -18,6 +18,7 @@ import ( "github.com/OctopusDeploy/cli/pkg/executor" "github.com/OctopusDeploy/cli/pkg/question" "github.com/OctopusDeploy/cli/pkg/surveyext" + "github.com/OctopusDeploy/cli/pkg/util/shell" "github.com/OctopusDeploy/cli/test/fixtures" "github.com/OctopusDeploy/cli/test/testutil" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/channels" @@ -2025,6 +2026,9 @@ func TestDeployCreate_AutomationMode(t *testing.T) { // this happens outside the scope of the normal AskQuestions flow so warrants its own integration-style test func TestDeployCreate_GenerationOfAutomationCommand_MasksSensitiveVariables(t *testing.T) { + // pin the shell so the expected command doesn't depend on where the tests are run + t.Setenv(constants.EnvOctopusShell, string(shell.Bash)) + const spaceID = "Spaces-1" const fireProjectID = "Projects-22" @@ -2202,7 +2206,7 @@ func TestDeployCreate_GenerationOfAutomationCommand_MasksSensitiveVariables(t *t Package Download: Use cached packages (if available) Deployment Targets: All included - Automation Command: octopus release deploy --space 'Default Space' --project 'Fire Project' --version '2.0' --environment 'dev' --variable 'Boring Variable:BORING' --variable 'Nuclear Launch Codes:*****' --variable 'Secret Password:*****' --no-prompt + Automation Command: octopus release deploy --space 'Default Space' --project 'Fire Project' --version 2.0 --environment dev --variable 'Boring Variable:BORING' --variable 'Nuclear Launch Codes:*****' --variable 'Secret Password:*****' --no-prompt Warning: Command includes some sensitive variable values which have been replaced with placeholders. Successfully started 2 deployment(s) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 05106062..d7a44792 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -1,6 +1,9 @@ package root import ( + "fmt" + "strings" + "github.com/OctopusDeploy/cli/pkg/apiclient" accountCmd "github.com/OctopusDeploy/cli/pkg/cmd/account" apiCmd "github.com/OctopusDeploy/cli/pkg/cmd/api" @@ -27,6 +30,7 @@ import ( "github.com/OctopusDeploy/cli/pkg/constants" "github.com/OctopusDeploy/cli/pkg/factory" "github.com/OctopusDeploy/cli/pkg/question" + "github.com/OctopusDeploy/cli/pkg/util/shell" "github.com/spf13/cobra" "github.com/spf13/viper" ) @@ -96,6 +100,8 @@ func NewCmdRoot(f factory.Factory, clientFactory apiclient.ClientFactory, askPro cmdPFlags.BoolP(constants.FlagNoPrompt, "", false, "Disable prompting in interactive mode") + cmdPFlags.String(constants.FlagShell, "", fmt.Sprintf(`Specify the shell that generated automation commands are quoted for (%s); defaults to the shell the CLI is running under`, strings.Join(shell.Names, ", "))) + // Enable service messages flag is hidden as it's intended for internal CI/CD use only cmdPFlags.BoolP(constants.FlagEnableServiceMessages, "", false, "Enable service messages for integration with Octopus CI/CD") cmdPFlags.MarkHidden(constants.FlagEnableServiceMessages) @@ -112,6 +118,7 @@ func NewCmdRoot(f factory.Factory, clientFactory apiclient.ClientFactory, askPro _ = viper.BindPFlag(constants.ConfigNoPrompt, cmdPFlags.Lookup(constants.FlagNoPrompt)) _ = viper.BindPFlag(constants.ConfigSpace, cmdPFlags.Lookup(constants.FlagSpace)) + _ = viper.BindPFlag(constants.ConfigShell, cmdPFlags.Lookup(constants.FlagShell)) _ = viper.BindPFlag(constants.FlagEnableServiceMessages, cmdPFlags.Lookup(constants.FlagEnableServiceMessages)) // if we attempt to check the flags before Execute is called, cobra hasn't parsed anything yet, // so we'll get bad values. PersistentPreRun is a convenient callback for setting up our diff --git a/pkg/config/config.go b/pkg/config/config.go index 7b2b7999..8fcd5338 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -30,6 +30,7 @@ func setDefaults(v *viper.Viper) { // v.SetDefault(constants.ConfigProxyUrl, "") v.SetDefault(constants.ConfigShowOctopus, true) v.SetDefault(constants.ConfigOutputFormat, "table") + v.SetDefault(constants.ConfigShell, "") if runtime.GOOS == "windows" { v.SetDefault(constants.ConfigEditor, "notepad") @@ -58,6 +59,9 @@ func bindEnvironment(v *viper.Viper) error { if err := v.BindEnv(constants.ConfigNoPrompt, constants.EnvCI); err != nil { return err } + if err := v.BindEnv(constants.ConfigShell, constants.EnvOctopusShell); err != nil { + return err + } return nil } diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 39b2ccf0..39f95e95 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -12,6 +12,7 @@ const ( FlagOutputFormatLegacy = "outputFormat" FlagNoPrompt = "no-prompt" FlagEnableServiceMessages = "enable-service-messages" + FlagShell = "shell" ) // flags for storing things in the go context @@ -38,6 +39,7 @@ const ( ConfigEditor = "Editor" ConfigShowOctopus = "ShowOctopus" ConfigOutputFormat = "OutputFormat" + ConfigShell = "Shell" ) const ( @@ -45,6 +47,7 @@ const ( EnvOctopusApiKey = "OCTOPUS_API_KEY" EnvOctopusAccessToken = "OCTOPUS_ACCESS_TOKEN" EnvOctopusSpace = "OCTOPUS_SPACE" + EnvOctopusShell = "OCTOPUS_SHELL" EnvEditor = "EDITOR" EnvVisual = "VISUAL" EnvCI = "CI" diff --git a/pkg/util/flag/flag.go b/pkg/util/flag/flag.go index 35da9467..483eb0c2 100644 --- a/pkg/util/flag/flag.go +++ b/pkg/util/flag/flag.go @@ -2,7 +2,8 @@ package flag import ( "fmt" - "strings" + + "github.com/OctopusDeploy/cli/pkg/util/shell" ) type Flag[T any] struct { @@ -60,27 +61,35 @@ func New[T any](name string, secure bool) *Flag[T] { // GenerateAutomationCmd generates the command that can be used to achive // the same results with the CLI in automation mode. func GenerateAutomationCmd(cmdPath string, space string, flags ...Generatable) string { + return GenerateAutomationCmdForShell(shell.Current(), cmdPath, space, flags...) +} + +// GenerateAutomationCmdForShell generates the automation command, quoting values using +// the rules of the given shell. +func GenerateAutomationCmdForShell(sh shell.Shell, cmdPath string, space string, flags ...Generatable) string { + quote := func(value string) string { return shell.Quote(sh, value) } + autoCmd := cmdPath if space != "" { - autoCmd += fmt.Sprintf(" --space '%s'", strings.ReplaceAll(space, "'", "'\\''")) + autoCmd += fmt.Sprintf(" --space %s", quote(space)) } for _, flag := range flags { switch value := flag.GetValue().(type) { case string: if value != "" { if flag.IsSecure() { - autoCmd += fmt.Sprintf(" --%s '***'", flag.GetName()) + autoCmd += fmt.Sprintf(" --%s %s", flag.GetName(), quote("***")) continue } - autoCmd += fmt.Sprintf(" --%s '%s'", flag.GetName(), strings.ReplaceAll(value, "'", "'\\''")) + autoCmd += fmt.Sprintf(" --%s %s", flag.GetName(), quote(value)) } case []string: for _, val := range value { if flag.IsSecure() { - autoCmd += fmt.Sprintf(" --%s '***'", flag.GetName()) + autoCmd += fmt.Sprintf(" --%s %s", flag.GetName(), quote("***")) continue } - autoCmd += fmt.Sprintf(" --%s '%s'", flag.GetName(), strings.ReplaceAll(val, "'", "'\\''")) + autoCmd += fmt.Sprintf(" --%s %s", flag.GetName(), quote(val)) } case bool: if value { diff --git a/pkg/util/flag/flag_test.go b/pkg/util/flag/flag_test.go new file mode 100644 index 00000000..05fa5aff --- /dev/null +++ b/pkg/util/flag/flag_test.go @@ -0,0 +1,62 @@ +package flag_test + +import ( + "testing" + + "github.com/OctopusDeploy/cli/pkg/util/flag" + "github.com/OctopusDeploy/cli/pkg/util/shell" + "github.com/stretchr/testify/assert" +) + +func TestGenerateAutomationCmdForShell(t *testing.T) { + project := flag.New[string]("project", false) + project.Value = "Soft Drinks" + version := flag.New[string]("version", false) + version.Value = "0.0.3" + environments := flag.New[[]string]("environment", false) + environments.Value = []string{"Dev", "Test Environment"} + tenantTag := flag.New[string]("tenant-tag", false) + tenantTag.Value = "Regions/us-east" + force := flag.New[bool]("force-package-download", false) + force.Value = true + timeout := flag.New[int]("timeout", false) + timeout.Value = 30 + password := flag.New[string]("password", true) + password.Value = "hunter2" + empty := flag.New[string]("description", false) + + flags := []flag.Generatable{project, version, environments, tenantTag, force, timeout, password, empty} + + tests := []struct { + shell shell.Shell + expected string + }{ + { + shell.Bash, + `octopus release deploy --space 'Default Space' --project 'Soft Drinks' --version 0.0.3 --environment Dev --environment 'Test Environment' --tenant-tag Regions/us-east --force-package-download --timeout 30 --password '***' --no-prompt`, + }, + { + shell.PowerShell, + `octopus release deploy --space 'Default Space' --project 'Soft Drinks' --version 0.0.3 --environment Dev --environment 'Test Environment' --tenant-tag Regions/us-east --force-package-download --timeout 30 --password '***' --no-prompt`, + }, + { + shell.Cmd, + `octopus release deploy --space "Default Space" --project "Soft Drinks" --version 0.0.3 --environment Dev --environment "Test Environment" --tenant-tag Regions/us-east --force-package-download --timeout 30 --password "***" --no-prompt`, + }, + } + + for _, test := range tests { + t.Run(string(test.shell), func(t *testing.T) { + actual := flag.GenerateAutomationCmdForShell(test.shell, "octopus release deploy", "Default Space", flags...) + assert.Equal(t, test.expected, actual) + }) + } +} + +func TestGenerateAutomationCmdForShell_NoSpace(t *testing.T) { + name := flag.New[string]("name", false) + name.Value = "Dev" + + actual := flag.GenerateAutomationCmdForShell(shell.Bash, "octopus environment create", "", name) + assert.Equal(t, "octopus environment create --name Dev --no-prompt", actual) +} diff --git a/pkg/util/shell/parent_other.go b/pkg/util/shell/parent_other.go new file mode 100644 index 00000000..01387ba0 --- /dev/null +++ b/pkg/util/shell/parent_other.go @@ -0,0 +1,9 @@ +//go:build !windows + +package shell + +// parentProcessName is only used to tell cmd and PowerShell apart, so there is +// nothing to look up on unix. +func parentProcessName() string { + return "" +} diff --git a/pkg/util/shell/parent_windows.go b/pkg/util/shell/parent_windows.go new file mode 100644 index 00000000..56fbf9ca --- /dev/null +++ b/pkg/util/shell/parent_windows.go @@ -0,0 +1,29 @@ +//go:build windows + +package shell + +import ( + "os" + "syscall" + "unsafe" +) + +// parentProcessName returns the file name of the executable that launched us. +func parentProcessName() string { + snapshot, err := syscall.CreateToolhelp32Snapshot(syscall.TH32CS_SNAPPROCESS, 0) + if err != nil { + return "" + } + defer syscall.CloseHandle(snapshot) + + entry := syscall.ProcessEntry32{} + entry.Size = uint32(unsafe.Sizeof(entry)) + ppid := uint32(os.Getppid()) + + for err = syscall.Process32First(snapshot, &entry); err == nil; err = syscall.Process32Next(snapshot, &entry) { + if entry.ProcessID == ppid { + return syscall.UTF16ToString(entry.ExeFile[:]) + } + } + return "" +} diff --git a/pkg/util/shell/quote.go b/pkg/util/shell/quote.go new file mode 100644 index 00000000..f41678be --- /dev/null +++ b/pkg/util/shell/quote.go @@ -0,0 +1,100 @@ +package shell + +import "strings" + +// Characters which carry no special meaning to the shell and so never need quoting. +// Letters and digits are always safe and aren't repeated here. +const ( + posixSafeChars = `@%+=:,./-_` + powerShellSafeChars = `+=:./\-_` + cmdSafeChars = `+=:./\-_` +) + +// Quote renders value so that sh passes it to the CLI as a single argument, unchanged. +// Values which don't need quoting are returned as they are. +func Quote(sh Shell, value string) string { + switch sh { + case PowerShell: + return quotePowerShell(value) + case Cmd: + return quoteCmd(value) + default: + return quotePosix(value) + } +} + +func isBare(value string, safeChars string) bool { + if value == "" { + return false + } + for _, r := range value { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9': + case strings.ContainsRune(safeChars, r): + default: + return false + } + } + return true +} + +// quotePosix quotes for sh, bash, zsh and friends. Everything inside single quotes is +// literal, so only the single quote itself needs handling; it is closed, escaped with a +// backslash, and reopened. +func quotePosix(value string) string { + if isBare(value, posixSafeChars) { + return value + } + return "'" + strings.ReplaceAll(value, "'", `'\''`) + "'" +} + +// quotePowerShell quotes for PowerShell. Single quoted strings are literal, and a single +// quote is escaped by doubling it. +func quotePowerShell(value string) string { + if isBare(value, powerShellSafeChars) { + return value + } + return "'" + strings.ReplaceAll(value, "'", "''") + "'" +} + +// quoteCmd quotes for cmd.exe, which has to survive two passes: cmd's own parsing, and +// then the argv parsing the CLI does as it starts up. +// - a literal double quote is `\"` for argv; the surrounding quotes are closed around it +// and the quote is caret escaped so cmd's own quoting stays balanced +// - backslashes only matter to argv where they run into a double quote, in which case +// the whole run has to be doubled +// - % is expanded even inside double quotes and can't be escaped there, so it is emitted +// outside them as ^% +// +// Two things can't be fixed here: a newline can't be represented in cmd at all, and ! is +// expanded when delayed expansion has been switched on. +func quoteCmd(value string) string { + if isBare(value, cmdSafeChars) { + return value + } + + var sb strings.Builder + sb.WriteByte('"') + backslashes := 0 + for i := 0; i < len(value); i++ { + switch c := value[i]; c { + case '\\': + backslashes++ + case '"': + sb.WriteString(strings.Repeat(`\`, backslashes*2)) + backslashes = 0 + sb.WriteString(`"\^""`) + case '%': + sb.WriteString(strings.Repeat(`\`, backslashes*2)) + backslashes = 0 + sb.WriteString(`"^%"`) + default: + sb.WriteString(strings.Repeat(`\`, backslashes)) + backslashes = 0 + sb.WriteByte(c) + } + } + sb.WriteString(strings.Repeat(`\`, backslashes*2)) + sb.WriteByte('"') + return sb.String() +} diff --git a/pkg/util/shell/quote_test.go b/pkg/util/shell/quote_test.go new file mode 100644 index 00000000..c8004754 --- /dev/null +++ b/pkg/util/shell/quote_test.go @@ -0,0 +1,234 @@ +package shell_test + +import ( + "fmt" + "os/exec" + "strings" + "testing" + + "github.com/OctopusDeploy/cli/pkg/util/shell" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestQuote(t *testing.T) { + tests := []struct { + name string + value string + posix string + powerShell string + cmd string + }{ + {"plain word", "Dev", "Dev", "Dev", "Dev"}, + {"version number", "0.0.3", "0.0.3", "0.0.3", "0.0.3"}, + {"tenant tag", "Regions/us-east", "Regions/us-east", "Regions/us-east", "Regions/us-east"}, + {"variable assignment", "Name:Value", "Name:Value", "Name:Value", "Name:Value"}, + {"empty", "", `''`, `''`, `""`}, + {"space", "Soft Drinks", `'Soft Drinks'`, `'Soft Drinks'`, `"Soft Drinks"`}, + {"single quote", "it's", `'it'\''s'`, `'it''s'`, `"it's"`}, + {"single quote and space", "it's here", `'it'\''s here'`, `'it''s here'`, `"it's here"`}, + {"double quote", `say "hi"`, `'say "hi"'`, `'say "hi"'`, `"say "\^""hi"\^"""`}, + {"backtick", "a`b", "'a`b'", "'a`b'", "\"a`b\""}, + {"dollar variable", "$HOME", `'$HOME'`, `'$HOME'`, `"$HOME"`}, + {"percent variable", "%PATH%", `%PATH%`, `'%PATH%'`, `""^%"PATH"^%""`}, + {"newline", "line1\nline2", "'line1\nline2'", "'line1\nline2'", "\"line1\nline2\""}, + {"comma", "a,b", "a,b", `'a,b'`, `"a,b"`}, + {"tilde", "~/tmp", `'~/tmp'`, `'~/tmp'`, `"~/tmp"`}, + {"masked secret", "*****", `'*****'`, `'*****'`, `"*****"`}, + {"ampersand", "A & B", `'A & B'`, `'A & B'`, `"A & B"`}, + {"windows path", `C:\Program Files\Octopus\`, `'C:\Program Files\Octopus\'`, `'C:\Program Files\Octopus\'`, `"C:\Program Files\Octopus\\"`}, + {"backslash then quote", `a\"b`, `'a\"b'`, `'a\"b'`, `"a\\"\^""b"`}, + {"non ascii", "Café", `'Café'`, `'Café'`, `"Café"`}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.posix, shell.Quote(shell.Bash, test.value), "bash") + assert.Equal(t, test.powerShell, shell.Quote(shell.PowerShell, test.value), "powershell") + assert.Equal(t, test.cmd, shell.Quote(shell.Cmd, test.value), "cmd") + }) + } +} + +// roundTripValues are fed through the real quoting and then back out again by the +// round trip tests below. +var roundTripValues = []string{ + "Dev", + "0.0.3", + "Regions/us-east", + "Soft Drinks", + "", + "it's", + "it's here", + `say "hi"`, + "a`b", + "$HOME", + "%PATH%", + "a,b", + "~/tmp", + "*****", + "A & B", + "a|b", + "a>b= len(line) { + return "", fmt.Errorf("trailing caret in %q", line) + } + sb.WriteByte(line[i]) + case c == '%': + return "", fmt.Errorf("unescaped %% in %q; cmd expands it even inside quotes", line) + case !inQuote && strings.ContainsRune(`&|<>()^`, rune(c)): + return "", fmt.Errorf("unquoted metacharacter %q in %q", c, line) + default: + sb.WriteByte(c) + } + } + if inQuote { + return "", fmt.Errorf("unbalanced quotes in %q", line) + } + return sb.String(), nil +} + +// parseArgv splits a windows command line into arguments the same way the go runtime +// does when it populates os.Args. The rules are documented at +// http://daviddeley.com/autohotkey/parameters/parameters.htm#WINARGV, including the +// "prior to 2008" handling of a doubled quote inside a quoted run. +func parseArgv(line string) []string { + var args []string + var current []byte + var backslashes int + started := false + inQuote := false + + appendBackslashes := func(n int) { + for ; n > 0; n-- { + current = append(current, '\\') + } + } + + for i := 0; i < len(line); i++ { + c := line[i] + switch c { + case '\\': + backslashes++ + continue + case '"': + appendBackslashes(backslashes / 2) + if backslashes%2 == 0 { + if inQuote && i+1 < len(line) && line[i+1] == '"' { + current = append(current, '"') + i++ + } + inQuote = !inQuote + } else { + current = append(current, '"') + } + backslashes = 0 + started = true + continue + case ' ', '\t': + if !inQuote { + appendBackslashes(backslashes) + backslashes = 0 + if started { + args = append(args, string(current)) + current = nil + started = false + } + continue + } + } + appendBackslashes(backslashes) + backslashes = 0 + current = append(current, c) + started = true + } + + appendBackslashes(backslashes) + if started { + args = append(args, string(current)) + } + return args +} diff --git a/pkg/util/shell/shell.go b/pkg/util/shell/shell.go new file mode 100644 index 00000000..d9f342c7 --- /dev/null +++ b/pkg/util/shell/shell.go @@ -0,0 +1,89 @@ +package shell + +import ( + "fmt" + "os" + "path/filepath" + "runtime" + "strings" + + "github.com/OctopusDeploy/cli/pkg/constants" + "github.com/spf13/viper" +) + +// Shell identifies the command line interpreter that generated automation commands +// are quoted for. Each shell has its own quoting and escaping rules. +type Shell string + +const ( + // Bash covers the POSIX shell family; sh, bash, zsh and friends all quote the same way. + Bash Shell = "bash" + PowerShell Shell = "powershell" + Cmd Shell = "cmd" +) + +// Names lists the values accepted by Parse, for help text and error messages. +var Names = []string{string(Bash), string(PowerShell), string(Cmd)} + +// Parse converts a shell name, such as the value of a config setting or the name of +// an executable, into a Shell. +func Parse(name string) (Shell, bool) { + name = strings.ToLower(strings.TrimSpace(name)) + name = strings.TrimSuffix(name, ".exe") + switch name { + case "sh", "bash", "zsh", "ksh", "dash", "ash": + return Bash, true + case "powershell", "pwsh": + return PowerShell, true + case "cmd", "command": + return Cmd, true + } + return "", false +} + +// Validate returns an error if name isn't a shell we can generate commands for. +func Validate(name string) error { + if _, ok := Parse(name); !ok { + return fmt.Errorf("the provided value %s is not a valid shell, please use one of %s", name, strings.Join(Names, ", ")) + } + return nil +} + +// Current returns the shell to generate automation commands for; the explicitly +// configured shell if there is one, otherwise the detected host shell. +func Current() Shell { + if s, ok := Parse(viper.GetString(constants.ConfigShell)); ok { + return s + } + return Detect(runtime.GOOS, os.Getenv) +} + +// Detect works out which shell the CLI is being run from. goos is a runtime.GOOS value +// and getenv looks up environment variables; both are parameters so this can be tested. +func Detect(goos string, getenv func(string) string) Shell { + // checked here as well as through viper so the override still works if the + // config system hasn't been set up, such as in tests + if s, ok := Parse(getenv(constants.EnvOctopusShell)); ok { + return s + } + + if goos == "windows" { + // the parent process is the only reliable signal on windows; PSModulePath is + // a machine wide variable that cmd.exe inherits too, so it tells us nothing. + // note this always misses when cross-compiled elsewhere, which is why it can't + // be the only check. + if s, ok := Parse(parentProcessName()); ok { + return s + } + // cmd is the safer guess: double quoted output also works in PowerShell, + // whereas single quoted output doesn't work in cmd at all. + return Cmd + } + + if sh := getenv("SHELL"); sh != "" { + if s, ok := Parse(filepath.Base(sh)); ok { + return s + } + } + return Bash +} diff --git a/pkg/util/shell/shell_test.go b/pkg/util/shell/shell_test.go new file mode 100644 index 00000000..395af9c5 --- /dev/null +++ b/pkg/util/shell/shell_test.go @@ -0,0 +1,76 @@ +package shell_test + +import ( + "runtime" + "testing" + + "github.com/OctopusDeploy/cli/pkg/util/shell" + "github.com/stretchr/testify/assert" +) + +func TestParse(t *testing.T) { + tests := []struct { + name string + expected shell.Shell + ok bool + }{ + {"bash", shell.Bash, true}, + {"BASH", shell.Bash, true}, + {" zsh ", shell.Bash, true}, + {"sh", shell.Bash, true}, + {"ksh", shell.Bash, true}, + {"dash", shell.Bash, true}, + {"powershell", shell.PowerShell, true}, + {"powershell.exe", shell.PowerShell, true}, + {"pwsh", shell.PowerShell, true}, + {"pwsh.exe", shell.PowerShell, true}, + {"cmd", shell.Cmd, true}, + {"cmd.exe", shell.Cmd, true}, + {"", "", false}, + {"fish", "", false}, + {"nushell", "", false}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual, ok := shell.Parse(test.name) + assert.Equal(t, test.ok, ok) + assert.Equal(t, test.expected, actual) + }) + } +} + +func TestValidate(t *testing.T) { + assert.NoError(t, shell.Validate("cmd")) + assert.EqualError(t, shell.Validate("fish"), "the provided value fish is not a valid shell, please use one of bash, powershell, cmd") +} + +func TestDetect(t *testing.T) { + tests := []struct { + name string + goos string + env map[string]string + expected shell.Shell + usesParentProcess bool + }{ + {"unix with SHELL", "linux", map[string]string{"SHELL": "/bin/zsh"}, shell.Bash, false}, + {"unix with unknown SHELL", "linux", map[string]string{"SHELL": "/usr/bin/fish"}, shell.Bash, false}, + {"unix without SHELL", "darwin", nil, shell.Bash, false}, + {"unix with pwsh as SHELL", "linux", map[string]string{"SHELL": "/usr/local/bin/pwsh"}, shell.PowerShell, false}, + {"windows falls back to cmd", "windows", nil, shell.Cmd, true}, + {"windows ignores PSModulePath", "windows", map[string]string{"PSModulePath": `C:\Program Files\WindowsPowerShell\Modules`}, shell.Cmd, true}, + {"override wins on unix", "linux", map[string]string{"SHELL": "/bin/bash", "OCTOPUS_SHELL": "cmd"}, shell.Cmd, false}, + {"override wins on windows", "windows", map[string]string{"OCTOPUS_SHELL": "pwsh"}, shell.PowerShell, false}, + {"invalid override is ignored", "linux", map[string]string{"SHELL": "/bin/bash", "OCTOPUS_SHELL": "fish"}, shell.Bash, false}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if test.usesParentProcess && runtime.GOOS == "windows" { + t.Skip("the real parent process is inspected when actually running on windows") + } + getenv := func(key string) string { return test.env[key] } + assert.Equal(t, test.expected, shell.Detect(test.goos, getenv)) + }) + } +}