Merge pull request #1477 from hashicorp/issues-1456-1463-command-quote

Issues 1456 1463, fixes command quoting bug
hashicorp · Jun 8, 2021 · d43dff9 · d43dff9
2 parents 1fa6270 + a245455
commit d43dff9
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 6 deletions.
diff --git a/go.mod b/go.mod
@@ -27,7 +27,7 @@ require (
 	github.com/hashicorp/serf v0.9.4 // indirect
 	github.com/hashicorp/vault/api v1.0.5-0.20190730042357-746c0b111519
 	github.com/mattn/go-colorable v0.1.7 // indirect
-	github.com/mattn/go-shellwords v1.0.10
+	github.com/mattn/go-shellwords v1.0.12
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/mitchellh/hashstructure v1.0.0
 	github.com/mitchellh/mapstructure v1.3.3

diff --git a/go.sum b/go.sum
@@ -23,6 +23,8 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/eikenb/go-shellwords v1.0.12-0.20210603231637-cbc54604f068 h1:y0DBnMgUmMeM+y1h7R2Hv0IZNiAcK5FAjNbRbbJNe3A=
+github.com/eikenb/go-shellwords v1.0.12-0.20210603231637-cbc54604f068/go.mod h1:EZzvwXDESEeg03EKmM+RmDnNOPKG4lLtQsUlTZDWQ8Y=
 github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
 github.com/fatih/color v1.9.0 h1:8xPHl4/q1VyqGIPif1F+1V3Y3lSmrq01EabUW3CoW5s=
 github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU=
@@ -146,6 +148,8 @@ github.com/mattn/go-isatty v0.0.12 h1:wuysRhFDzyxgEmMf5xjvJ2M9dZoWAXNNr5LSBS7uHX
 github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU=
 github.com/mattn/go-shellwords v1.0.10 h1:Y7Xqm8piKOO3v10Thp7Z36h4FYFjt5xB//6XvOrs2Gw=
 github.com/mattn/go-shellwords v1.0.10/go.mod h1:EZzvwXDESEeg03EKmM+RmDnNOPKG4lLtQsUlTZDWQ8Y=
+github.com/mattn/go-shellwords v1.0.12 h1:M2zGm7EW6UQJvDeQxo4T51eKPurbeFbe8WtebGE2xrk=
+github.com/mattn/go-shellwords v1.0.12/go.mod h1:EZzvwXDESEeg03EKmM+RmDnNOPKG4lLtQsUlTZDWQ8Y=
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
 github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg=
 github.com/miekg/dns v1.1.26 h1:gPxPSwALAeHJSjarOs00QjVdV9QoBvc1D2ujQUr5BzU=

diff --git a/manager/runner.go b/manager/runner.go
@@ -1152,14 +1152,10 @@ type spawnChildInput struct {
 // spawnChild spawns a child process with the given inputs and returns the
 // resulting child.
 func spawnChild(i *spawnChildInput) (*child.Child, error) {
-	p := shellwords.NewParser()
-	p.ParseEnv = true
-	p.ParseBacktick = true
-	args, err := p.Parse(i.Command)
+	args, err := parseCommand(i.Command)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed parsing command")
 	}
-
 	child, err := child.New(&child.NewInput{
 		Stdin:        i.Stdin,
 		Stdout:       i.Stdout,
@@ -1183,6 +1179,14 @@ func spawnChild(i *spawnChildInput) (*child.Child, error) {
 	return child, nil
 }
 
+// parseCommand parses the shell command line into usable format
+func parseCommand(command string) ([]string, error) {
+	p := shellwords.NewParser()
+	p.ParseEnv = true
+	p.ParseBacktick = true
+	return p.Parse(command)
+}
+
 // quiescence is an internal representation of a single template's quiescence
 // state.
 type quiescence struct {

diff --git a/manager/runner_test.go b/manager/runner_test.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"reflect"
 	"strings"
 	"testing"
 	"time"
@@ -1075,3 +1076,49 @@ func TestRunner_quiescence(t *testing.T) {
 		}
 	})
 }
+
+func TestRunner_parseCommand(t *testing.T) {
+	type testCase struct {
+		name, input string
+		expect      []string
+	}
+	runTest := func(tc testCase) {
+		out, err := parseCommand(tc.input)
+		mismatchErr := "bad command parse\ngot: '%#v'\nwanted: '%#v'"
+		switch {
+		case err != nil:
+			t.Fatal("unexpected error:", err)
+		case len(out) != len(tc.expect):
+			t.Fatalf(mismatchErr, out, tc.expect)
+		case !reflect.DeepEqual(out, tc.expect):
+			t.Fatalf(mismatchErr, out, tc.expect)
+		}
+	}
+	for i, tc := range []testCase{
+		{
+			name:   "null",
+			input:  "",
+			expect: []string{},
+		},
+		{
+			name:   "simple",
+			input:  "echo hi",
+			expect: []string{"echo", "hi"},
+		},
+		{
+			name:   "subshell-single-quoting", // GH-1456 & GH-1463
+			input:  "sh -c 'echo hi'",
+			expect: []string{"sh", "-c", "echo hi"},
+		},
+		{
+			name:   "subshell-double-quoting",
+			input:  `sh -c "echo hi"`,
+			expect: []string{"sh", "-c", "echo hi"},
+		},
+	} {
+		t.Run(fmt.Sprintf("%d_%s", i, tc.name),
+			func(t *testing.T) {
+				runTest(tc)
+			})
+	}
+}