🔒 Better parameter escaping

cmd/dump/dump.go

@@ -5,6 +5,7 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
+	"github.com/clevyr/kubedb/internal/command"
 	"github.com/clevyr/kubedb/internal/config"
 	"github.com/clevyr/kubedb/internal/config/flags"
 	"github.com/clevyr/kubedb/internal/database/sqlformat"
@@ -18,7 +19,6 @@ import (
 	"os"
 	"path/filepath"
 	"strconv"
-	"strings"
 )
 
 var Command = &cobra.Command{
@@ -194,9 +194,9 @@ func run(cmd *cobra.Command, args []string) (err error) {
 func buildCommand(db config.Databaser, conf config.Dump) []string {
 	cmd := db.DumpCommand(conf)
 	if conf.OutputFormat != sqlformat.Custom {
-		cmd = append(cmd, "|", "gzip", "--force")
+		cmd.Push(command.Raw("|"), "gzip", "--force")
 	}
 	// base64 is required since TTYs use CRLF
-	cmd = append(cmd, "|", "base64", "--wrap=0")
-	return []string{"sh", "-c", strings.Join(cmd, " ")}
+	cmd.Push(command.Raw("|"), "base64", "--wrap=0")
+	return []string{"sh", "-c", cmd.Join()}
 }

cmd/exec/exec.go

@@ -1,14 +1,14 @@
 package exec
 
 import (
+	"github.com/clevyr/kubedb/internal/command"
 	"github.com/clevyr/kubedb/internal/config"
 	"github.com/clevyr/kubedb/internal/util"
 	"github.com/docker/cli/cli/streams"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 	"os"
-	"strings"
 )
 
 var Command = &cobra.Command{
@@ -38,11 +38,14 @@ func run(cmd *cobra.Command, args []string) (err error) {
 }
 
 func buildCommand(db config.Databaser, conf config.Exec, args []string) []string {
-	var cmd []string
+	var cmd *command.Builder
 	if len(args) == 0 {
 		cmd = db.ExecCommand(conf)
 	} else {
-		cmd = append([]string{"exec"}, args...)
+		cmd = command.NewBuilder("exec")
+		for _, arg := range args {
+			cmd.Push(arg)
+		}
 	}
-	return []string{"sh", "-c", strings.Join(cmd, " ")}
+	return []string{"sh", "-c", cmd.Join()}
 }

cmd/restore/restore.go

@@ -4,6 +4,7 @@ import (
 	"compress/gzip"
 	"fmt"
 	"github.com/AlecAivazis/survey/v2"
+	"github.com/clevyr/kubedb/internal/command"
 	"github.com/clevyr/kubedb/internal/config"
 	"github.com/clevyr/kubedb/internal/config/flags"
 	"github.com/clevyr/kubedb/internal/database/sqlformat"
@@ -170,9 +171,9 @@ func run(cmd *cobra.Command, args []string) (err error) {
 }
 
 func buildCommand(conf config.Restore, inputFormat sqlformat.Format) []string {
-	cmd := []string{"gunzip", "--force", "|"}
-	cmd = append(cmd, conf.Grammar.RestoreCommand(conf, inputFormat)...)
-	return []string{"sh", "-c", strings.Join(cmd, " ")}
+	cmd := conf.Grammar.RestoreCommand(conf, inputFormat)
+	cmd.Unshift("gunzip", "--force", command.Raw("|"))
+	return []string{"sh", "-c", cmd.Join()}
 }
 
 func gzipCopy(w io.Writer, r io.Reader) (err error) {

go.mod

@@ -26,6 +26,7 @@ require (
 	github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
 	github.com/Azure/go-autorest/logger v0.2.1 // indirect
 	github.com/Azure/go-autorest/tracing v0.6.0 // indirect
+	github.com/alessio/shellescape v1.4.1 // indirect
 	github.com/cpuguy83/go-md2man/v2 v2.0.0 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/fsnotify/fsnotify v1.4.9 // indirect
@@ -68,6 +69,7 @@ require (
 	golang.org/x/time v0.0.0-20211116232009-f0f3c7e86c11 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
 	google.golang.org/protobuf v1.27.1 // indirect
+	gopkg.in/alessio/shellescape.v1 v1.0.0-20170105083845-52074bc9df61 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/ini.v1 v1.62.0 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect

go.sum

@@ -75,6 +75,8 @@ github.com/Netflix/go-expect v0.0.0-20180615182759-c93bf25de8e8/go.mod h1:oX5x61
 github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
 github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0=
 github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
+github.com/alessio/shellescape v1.4.1 h1:V7yhSDDn8LP4lc4jS8pFkt0zCnzVJlG5JXy9BVKJUX0=
+github.com/alessio/shellescape v1.4.1/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPpyFjDCtHLS30=
 github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
 github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o=
 github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY=
@@ -813,6 +815,8 @@ google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp0
 google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
 google.golang.org/protobuf v1.27.1 h1:SnqbnDw1V7RiZcXPx5MEeqPv2s79L9i7BJUlG/+RurQ=
 google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
+gopkg.in/alessio/shellescape.v1 v1.0.0-20170105083845-52074bc9df61 h1:8ajkpB4hXVftY5ko905id+dOnmorcS2CHNxxHLLDcFM=
+gopkg.in/alessio/shellescape.v1 v1.0.0-20170105083845-52074bc9df61/go.mod h1:IfMagxm39Ys4ybJrDb7W3Ob8RwxftP0Yy+or/NVz1O8=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

internal/command/builder.go

@@ -0,0 +1,47 @@
+package command
+
+import (
+	"fmt"
+	"gopkg.in/alessio/shellescape.v1"
+	"strings"
+)
+
+func NewBuilder(p ...any) *Builder {
+	return &Builder{
+		cmd: p,
+	}
+}
+
+type Builder struct {
+	cmd []any
+}
+
+func (j *Builder) Push(p ...any) *Builder {
+	j.cmd = append(j.cmd, p...)
+	return j
+}
+
+func (j *Builder) Unshift(p ...any) *Builder {
+	j.cmd = append(p, j.cmd...)
+	return j
+}
+
+func (j Builder) Join() string {
+	var buf strings.Builder
+	for k, v := range j.cmd {
+		switch v.(type) {
+		case string:
+			buf.WriteString(shellescape.Quote(v.(string)))
+		case Env:
+			buf.WriteString(v.(Env).String())
+		case Raw:
+			buf.WriteString(string(v.(Raw)))
+		default:
+			panic(fmt.Errorf("unknown value in command: %#v", v))
+		}
+		if k < len(j.cmd)-1 {
+			buf.WriteString(" ")
+		}
+	}
+	return buf.String()
+}

internal/command/env.go

@@ -0,0 +1,19 @@
+package command
+
+import "gopkg.in/alessio/shellescape.v1"
+
+func NewEnv(k, v string) Env {
+	return Env{
+		Key:   k,
+		Value: v,
+	}
+}
+
+type Env struct {
+	Key   string
+	Value string
+}
+
+func (e Env) String() string {
+	return e.Key + "=" + shellescape.Quote(e.Value)
+}

internal/command/raw.go

@@ -0,0 +1,3 @@
+package command
+
+type Raw string

internal/config/database.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"github.com/clevyr/kubedb/internal/command"
 	"github.com/clevyr/kubedb/internal/database/sqlformat"
 	"github.com/clevyr/kubedb/internal/kubernetes"
 	v1 "k8s.io/api/core/v1"
@@ -22,7 +23,7 @@ type Databaser interface {
 	FilterPods(client kubernetes.KubeClient, pods []v1.Pod) ([]v1.Pod, error)
 	PasswordEnvNames() []string
 
-	ExecCommand(conf Exec) []string
-	DumpCommand(conf Dump) []string
-	RestoreCommand(conf Restore, inputFormat sqlformat.Format) []string
+	ExecCommand(conf Exec) *command.Builder
+	DumpCommand(conf Dump) *command.Builder
+	RestoreCommand(conf Restore, inputFormat sqlformat.Format) *command.Builder
 }

internal/database/grammar/mariadb.go

@@ -1,6 +1,7 @@
 package grammar
 
 import (
+	"github.com/clevyr/kubedb/internal/command"
 	"github.com/clevyr/kubedb/internal/config"
 	"github.com/clevyr/kubedb/internal/database/sqlformat"
 	"github.com/clevyr/kubedb/internal/kubernetes"
@@ -59,22 +60,33 @@ func (MariaDB) PasswordEnvNames() []string {
 	return []string{"MARIADB_PASSWORD"}
 }
 
-func (MariaDB) ExecCommand(conf config.Exec) []string {
-	return []string{"MYSQL_PWD=" + conf.Password, "mysql", "--host=127.0.0.1", "--user=" + conf.Username, "--database=" + conf.Database}
+func (MariaDB) ExecCommand(conf config.Exec) *command.Builder {
+	return command.NewBuilder(
+		command.NewEnv("MYSQL_PWD", conf.Password),
+		"mysql", "--host=127.0.0.1", "--user="+conf.Username, "--database="+conf.Database,
+	)
 }
 
-func (MariaDB) DumpCommand(conf config.Dump) []string {
-	cmd := []string{"MYSQL_PWD=" + conf.Password, "mysqldump", "--host=127.0.0.1", "--user=" + conf.Username, conf.Database}
+func (MariaDB) DumpCommand(conf config.Dump) *command.Builder {
+	cmd := command.NewBuilder(
+		command.NewEnv("MYSQL_PWD", conf.Password),
+		"mysqldump", "--host=127.0.0.1", "--user="+conf.Username, conf.Database,
+	)
 	if conf.Clean {
-		cmd = append(cmd, "--add-drop-table")
+		cmd.Push("--add-drop-table")
+	}
+	for _, table := range conf.Tables {
+		cmd.Push(table)
 	}
-	cmd = append(cmd, conf.Tables...)
 	for _, table := range conf.ExcludeTable {
-		cmd = append(cmd, "--ignore-table='"+table+"'")
+		cmd.Push("--ignore-table=" + table)
 	}
 	return cmd
 }
 
-func (MariaDB) RestoreCommand(conf config.Restore, inputFormat sqlformat.Format) []string {
-	return []string{"MYSQL_PWD=" + conf.Password, "mysql", "--host=127.0.0.1", "--user=" + conf.Username, conf.Database}
+func (MariaDB) RestoreCommand(conf config.Restore, inputFormat sqlformat.Format) *command.Builder {
+	return command.NewBuilder(
+		command.NewEnv("MYSQL_PWD", conf.Password),
+		"mysql", "--host=127.0.0.1", "--user="+conf.Username, conf.Database,
+	)
 }

internal/database/grammar/postgres.go

@@ -2,6 +2,7 @@ package grammar
 
 import (
 	"encoding/csv"
+	"github.com/clevyr/kubedb/internal/command"
 	"github.com/clevyr/kubedb/internal/config"
 	"github.com/clevyr/kubedb/internal/database/sqlformat"
 	"github.com/clevyr/kubedb/internal/kubernetes"
@@ -101,8 +102,11 @@ func (Postgres) PasswordEnvNames() []string {
 	return []string{"POSTGRES_PASSWORD", "PGPOOL_POSTGRES_PASSWORD"}
 }
 
-func (Postgres) ExecCommand(conf config.Exec) []string {
-	return []string{"PGPASSWORD=" + conf.Password, "psql", "--host=127.0.0.1", "--username=" + conf.Username, "--dbname=" + conf.Database}
+func (Postgres) ExecCommand(conf config.Exec) *command.Builder {
+	return command.NewBuilder(
+		command.NewEnv("PGPASSWORD", conf.Password),
+		"psql", "--host=127.0.0.1", "--username="+conf.Username, "--dbname="+conf.Database,
+	)
 }
 
 func quoteParam(param string) string {
@@ -111,46 +115,52 @@ func quoteParam(param string) string {
 	return param
 }
 
-func (Postgres) DumpCommand(conf config.Dump) []string {
-	cmd := []string{"PGPASSWORD=" + conf.Password, "pg_dump", "--host=127.0.0.1", "--username=" + conf.Username, "--dbname=" + conf.Database}
+func (Postgres) DumpCommand(conf config.Dump) *command.Builder {
+	cmd := command.NewBuilder(
+		command.NewEnv("PGPASSWORD", conf.Password),
+		"pg_dump", "--host=127.0.0.1", "--username="+conf.Username, "--dbname="+conf.Database,
+	)
 	if conf.Clean {
-		cmd = append(cmd, "--clean")
+		cmd.Push("--clean")
 	}
 	if conf.NoOwner {
-		cmd = append(cmd, "--no-owner")
+		cmd.Push("--no-owner")
 	}
 	if conf.IfExists {
-		cmd = append(cmd, "--if-exists")
+		cmd.Push("--if-exists")
 	}
 	for _, table := range conf.Tables {
-		cmd = append(cmd, "--table='"+quoteParam(table)+"'")
+		cmd.Push("--table=" + quoteParam(table))
 	}
 	for _, table := range conf.ExcludeTable {
-		cmd = append(cmd, "--exclude-table='"+quoteParam(table)+"'")
+		cmd.Push("--exclude-table=" + quoteParam(table))
 	}
 	for _, table := range conf.ExcludeTableData {
-		cmd = append(cmd, "--exclude-table-data='"+quoteParam(table)+"'")
+		cmd.Push("--exclude-table-data=" + quoteParam(table))
 	}
 	if conf.OutputFormat == sqlformat.Custom {
-		cmd = append(cmd, "--format=c")
+		cmd.Push("--format=c")
 	}
 	return cmd
 }
 
-func (Postgres) RestoreCommand(conf config.Restore, inputFormat sqlformat.Format) []string {
-	cmd := []string{"PGPASSWORD=" + conf.Password, "PGOPTIONS='-c client_min_messages=WARNING'"}
+func (Postgres) RestoreCommand(conf config.Restore, inputFormat sqlformat.Format) *command.Builder {
+	cmd := command.NewBuilder(
+		command.NewEnv("PGPASSWORD", conf.Password),
+		command.NewEnv("PGOPTIONS", "-c client_min_messages=WARNING"),
+	)
 	switch inputFormat {
 	case sqlformat.Gzip, sqlformat.Plain:
-		cmd = append(cmd, "psql", "--quiet", "--output=/dev/null")
+		cmd.Push("psql", "--quiet", "--output=/dev/null")
 	case sqlformat.Custom:
-		cmd = append(cmd, "pg_restore", "--format=custom", "--verbose", "--clean", "--exit-on-error")
+		cmd.Push("pg_restore", "--format=custom", "--verbose", "--clean", "--exit-on-error")
 		if conf.NoOwner {
-			cmd = append(cmd, "--no-owner")
+			cmd.Push("--no-owner")
 		}
 	}
-	cmd = append(cmd, "--host=127.0.0.1", "--username="+conf.Username, "--dbname="+conf.Database)
+	cmd.Push("--host=127.0.0.1", "--username="+conf.Username, "--dbname="+conf.Database)
 	if conf.SingleTransaction {
-		cmd = append(cmd, "--single-transaction")
+		cmd.Push("--single-transaction")
 	}
 	return cmd
 }