fix(railpack): use ShellQuote to preserve env vars in command (#1428)

Author: lionello

pkgs/defang/cli.nix

@@ -7,7 +7,7 @@ buildGoModule {
   pname = "defang-cli";
   version = "git";
   src = ../../src;
-  vendorHash = "sha256-8caEfevVUKXsYA5YyVDuPZTqMnYXIZnC4kTTGfPmuqA="; # TODO: use fetchFromGitHub
+  vendorHash = "sha256-2BQght2PFjXOwV7K7C74hc3yFvIS46N15qoUmkxZAe8="; # TODO: use fetchFromGitHub
 
   subPackages = [ "cmd/cli" ];
 

src/go.mod

@@ -125,7 +125,6 @@ require (
 )
 
 require (
-	al.essio.dev/pkg/shellescape v1.6.0
 	github.com/Microsoft/go-winio v0.6.2 // indirect
 	github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.2 // indirect
 	github.com/aws/aws-sdk-go-v2/credentials v1.16.16

src/go.sum

@@ -1,5 +1,3 @@
-al.essio.dev/pkg/shellescape v1.6.0 h1:NxFcEqzFSEVCGN2yq7Huv/9hyCEGVa/TncnOOBBeXHA=
-al.essio.dev/pkg/shellescape v1.6.0/go.mod h1:6sIqp7X2P6mThCQ7twERpZTuigpr6KbZWtls1U8I890=
 cel.dev/expr v0.20.0 h1:OunBvVCfvpWlt4dN7zg3FM6TDkzOePe1+foGJ9AXeeI=
 cel.dev/expr v0.20.0/go.mod h1:MrpN08Q+lEBs+bGYdLxxHkZoUSsCp0nSKTs0nTymJgw=
 cloud.google.com/go v0.120.0 h1:wc6bgG9DHyKqF5/vQvX1CiZrtHnxJjBlKUyF9nP6meA=
@@ -187,8 +185,6 @@ github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 h1:yAJXTCF9TqKcTiHJAE
 github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
 github.com/google/s2a-go v0.1.9 h1:LGD7gtMgezd8a/Xak7mEWL0PjoTQFvpRudN895yqKW0=
 github.com/google/s2a-go v0.1.9/go.mod h1:YA0Ei2ZQL3acow2O62kdp9UlnvMmU7kA6Eutn0dXayM=
-github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
-github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
 github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/googleapis/enterprise-certificate-proxy v0.3.6 h1:GW/XbdyBFQ8Qe+YAmFU9uHLo7OnF5tL52HFAgMmyrf4=
@@ -223,9 +219,9 @@ github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9Y
 github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo=
 github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8=
 github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U=
-github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
 github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0=
 github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4=
+github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
 github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=

src/pkg/cli/compose/fixup.go

@@ -9,7 +9,7 @@ import (
 	"strconv"
 	"strings"
 
-	"al.essio.dev/pkg/shellescape"
+	"github.com/DefangLabs/defang/src/pkg"
 	"github.com/DefangLabs/defang/src/pkg/cli/client"
 	"github.com/DefangLabs/defang/src/pkg/term"
 	defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1"
@@ -108,7 +108,7 @@ func FixupServices(ctx context.Context, provider client.Provider, project *compo
 					// command is run as intended by replacing `command: [ "npm", "start" ]`
 					// with `command: [ "npm start" ]`.
 					if len(svccfg.Command) > 1 {
-						svccfg.Command = []string{shellescape.QuoteCommand(svccfg.Command)}
+						svccfg.Command = []string{pkg.ShellQuote(svccfg.Command...)}
 					}
 				}
 			}

src/pkg/clouds/do/appPlatform/setup.go

@@ -7,7 +7,6 @@ import (
 	"os"
 	"path"
 	"regexp"
-	"strconv"
 	"strings"
 	"time"
 
@@ -89,14 +88,6 @@ func (d *DoApp) SetUpBucket(ctx context.Context) error {
 	return err
 }
 
-func shellQuote(args ...string) string {
-	quoted := make([]string, len(args))
-	for i, arg := range args {
-		quoted[i] = strconv.Quote(arg)
-	}
-	return strings.Join(quoted, " ")
-}
-
 func getImageSourceSpec(cdImagePath string) (*godo.ImageSourceSpec, error) {
 	term.Debugf("Using CD image: %q", cdImagePath)
 	image, err := ParseImage(cdImagePath)
@@ -143,7 +134,7 @@ func (d DoApp) Run(ctx context.Context, env []*godo.AppVariableDefinition, cdIma
 			Image:            image,
 			InstanceCount:    1,
 			InstanceSizeSlug: "basic-xs", // TODO: this is legacy and we should use new slugs
-			RunCommand:       shellQuote(cmd...),
+			RunCommand:       pkg.ShellQuote(cmd...),
 			Termination: &godo.AppJobSpecTermination{
 				GracePeriodSeconds: 600, // max 10mins to avoid killing the job while it's still running
 			},

src/pkg/clouds/do/appPlatform/setup_test.go

@@ -6,38 +6,6 @@ import (
 	"github.com/digitalocean/godo"
 )
 
-func TestShellQuote(t *testing.T) {
-	// Given
-	tests := []struct {
-		input    []string
-		expected string
-	}{
-		{
-			input:    []string{"true"},
-			expected: `"true"`,
-		},
-		{
-			input:    []string{"echo", "hello world"},
-			expected: `"echo" "hello world"`,
-		},
-		{
-			input:    []string{"echo", "hello", "world"},
-			expected: `"echo" "hello" "world"`,
-		},
-		{
-			input:    []string{"echo", `hello"world`},
-			expected: `"echo" "hello\"world"`,
-		},
-	}
-
-	for _, test := range tests {
-		actual := shellQuote(test.input...)
-		if actual != test.expected {
-			t.Errorf("Expected `%s` but got: `%s`", test.expected, actual)
-		}
-	}
-}
-
 func Test_getImageSourceSpec(t *testing.T) {
 	// Given
 	tests := []struct {

src/pkg/utils.go

@@ -175,3 +175,18 @@ func Diff(actualRaw, goldenRaw string) error {
 	diff := fmt.Sprint(gotextdiff.ToUnified("expected", "actual", goldenRaw, edits))
 	return fmt.Errorf("mismatch:\n%s", diff)
 }
+
+var shellSpecialChars = regexp.MustCompile(`[^\w@%+=:,./-]`) // copied from al.essio.dev/pkg/shellescape
+
+// ShellQuote returns a shell-quoted string of the given arguments.
+// When needed, arguments are quoted with double quotes, so that spaces and env vars are preserved.
+func ShellQuote(args ...string) string {
+	quoted := make([]string, len(args))
+	for i, arg := range args {
+		if shellSpecialChars.MatchString(arg) {
+			arg = strconv.Quote(arg)
+		}
+		quoted[i] = arg
+	}
+	return strings.Join(quoted, " ")
+}

src/pkg/utils_test.go

@@ -158,3 +158,38 @@ func TestGetCurrentUser(t *testing.T) {
 		t.Errorf("GetCurrentUser() returned an empty string")
 	}
 }
+
+func TestShellQuote(t *testing.T) {
+	tests := []struct {
+		input    []string
+		expected string
+	}{
+		{
+			input:    []string{"true"},
+			expected: `true`,
+		},
+		{
+			input:    []string{"echo", "hello world"},
+			expected: `echo "hello world"`,
+		},
+		{
+			input:    []string{"echo", "hello", "world"},
+			expected: `echo hello world`,
+		},
+		{
+			input:    []string{"echo", `hello"world`},
+			expected: `echo "hello\"world"`,
+		},
+		{
+			input:    []string{"bash", "-c", "start.sh $PORT"},
+			expected: `bash -c "start.sh $PORT"`,
+		},
+	}
+
+	for _, test := range tests {
+		actual := ShellQuote(test.input...)
+		if actual != test.expected {
+			t.Errorf("Expected `%s` but got: `%s`", test.expected, actual)
+		}
+	}
+}

src/testdata/railpack/compose.yaml

@@ -10,4 +10,4 @@ services:
     build:
       context: .
       dockerfile: Dockerfile
-    command: something "with space"
+    command: something "with space" $PORT

src/testdata/railpack/compose.yaml.fixup

@@ -31,7 +31,7 @@
       "dockerfile": "*Railpack"
     },
     "command": [
-      "something 'with space'"
+      "something \"with space\" \"${PORT}\""
     ],
     "entrypoint": null,
     "networks": {