fix: update argument handling based on ansible execution tracing

Add a bunch of test cases created by sniffing the execution of ssh
commands on an openssh client and server.

go.mod

@@ -3,11 +3,11 @@ module github.com/uselagoon/ssh-portal
 go 1.22.2
 
 require (
-	al.essio.dev/pkg/shellescape v1.5.1
 	github.com/DATA-DOG/go-sqlmock v1.5.2
 	github.com/MicahParks/keyfunc/v2 v2.1.0
 	github.com/alecthomas/assert/v2 v2.11.0
 	github.com/alecthomas/kong v1.2.1
+	github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
 	github.com/gliderlabs/ssh v0.3.7
 	github.com/go-sql-driver/mysql v1.8.1
@@ -34,7 +34,6 @@ require (
 require (
 	filippo.io/edwards25519 v1.1.0 // indirect
 	github.com/alecthomas/repr v0.4.0 // indirect
-	github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/emicklei/go-restful/v3 v3.11.0 // indirect

go.sum

@@ -1,5 +1,3 @@
-al.essio.dev/pkg/shellescape v1.5.1 h1:86HrALUujYS/h+GtqoB26SBEdkWfmMI6FubjXlsXyho=
-al.essio.dev/pkg/shellescape v1.5.1/go.mod h1:6sIqp7X2P6mThCQ7twERpZTuigpr6KbZWtls1U8I890=
 filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA=
 filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
 github.com/DATA-DOG/go-sqlmock v1.5.2 h1:OcvFkGmslmlZibjAjaHm3L//6LiuBgolP7OputlJIzU=
@@ -69,8 +67,6 @@ github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0=
 github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af h1:kmjWCqn2qkEml422C2Rrd27c3VGxi6a/6HNq8QmHRKM=
 github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af/go.mod h1:K1liHPHnj73Fdn/EKuT8nrFqBihUSKXoLYU0BuatOYo=
-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.1-0.20240806143717-0e97ed3b5379 h1:9pvPp/2VCtCB2xdSUCaKe1VKCzVHMR+GGgIAVLfQxIs=
 github.com/google/uuid v1.6.1-0.20240806143717-0e97ed3b5379/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/gorilla/securecookie v1.1.2 h1:YCIWL56dvtr73r6715mJs5ZvhtnY73hBvEF8kXD8ePA=

internal/sshserver/connectionparams.go

@@ -8,9 +8,9 @@ import (
 )
 
 var (
-	serviceRegex   = regexp.MustCompile(`^service=(.+)`)
-	containerRegex = regexp.MustCompile(`^container=(.+)`)
-	logsRegex      = regexp.MustCompile(`^logs=(.+)`)
+	serviceRegex   = regexp.MustCompile(`^service=(\S+)`)
+	containerRegex = regexp.MustCompile(`^container=(\S+)`)
+	logsRegex      = regexp.MustCompile(`^logs=(\S+)`)
 	tailLinesRegex = regexp.MustCompile(`^tailLines=(\d+)$`)
 )
 
@@ -26,16 +26,16 @@ var (
 	ErrNoServiceForLogs = errors.New("missing service argument for logs argument")
 )
 
-// parseConnectionParams takes the raw SSH command, and parses out any
+// parseConnectionParams takes the split and raw SSH command, and parses out any
 // leading service=..., container=..., and logs=... arguments. It returns:
 //   - If a service=... argument is given, the value of that argument.
 //     If no such argument is given, it falls back to a default of "cli".
 //   - If a container=... argument is given, the value of that argument.
 //     If no such argument is given, it returns an empty string.
 //   - If a logs=... argument is given, the value of that argument.
 //     If no such argument is given, it returns an empty string.
-//   - The remaining arguments as a slice of strings, with any leading
-//     service=, container=, or logs= arguments removed.
+//   - The remaining raw SSH command, with any leading service=, container=, or
+//     logs= arguments removed.
 //
 // Notes about the logic implemented here:
 //   - service=... must be given as the first argument to be recognised.
@@ -48,48 +48,54 @@ var (
 //
 //	[service=... [container=...]] CMD...
 //	service=... [container=...] logs=...
-func parseConnectionParams(args []string) (string, string, string, []string) {
+func parseConnectionParams(
+	cmd []string,
+	rawCmd string,
+) (string, string, string, string) {
 	// exit early if we have no args
-	if len(args) == 0 {
-		return "cli", "", "", nil
+	if len(cmd) == 0 {
+		return "cli", "", "", rawCmd
 	}
 	// check for service argument
-	serviceMatches := serviceRegex.FindStringSubmatch(args[0])
+	serviceMatches := serviceRegex.FindStringSubmatch(cmd[0])
 	if len(serviceMatches) == 0 {
 		// no service= match, so assume cli and return all args
-		return "cli", "", "", args
+		return "cli", "", "", rawCmd
 	}
 	service := serviceMatches[1]
+	rawCmd = strings.TrimSpace(serviceRegex.ReplaceAllString(rawCmd, ""))
 	// exit early if we are out of arguments
-	if len(args) == 1 {
-		return service, "", "", nil
+	if len(cmd) == 1 {
+		return service, "", "", rawCmd
 	}
 	// check for container and/or logs argument
-	containerMatches := containerRegex.FindStringSubmatch(args[1])
+	containerMatches := containerRegex.FindStringSubmatch(cmd[1])
 	if len(containerMatches) == 0 {
 		// no container= match, so check for logs=
-		logsMatches := logsRegex.FindStringSubmatch(args[1])
+		logsMatches := logsRegex.FindStringSubmatch(cmd[1])
 		if len(logsMatches) == 0 {
 			// no container= or logs= match, so just return the args
-			return service, "", "", args[1:]
+			return service, "", "", rawCmd
 		}
-		// found logs=, so return it along with the remaining args
-		// (which should be empty)
-		return service, "", logsMatches[1], args[2:]
+		rawCmd = strings.TrimSpace(logsRegex.ReplaceAllString(rawCmd, ""))
+		// found logs=, so return it along with the remaining rawCmd
+		return service, "", logsMatches[1], rawCmd
 	}
 	container := containerMatches[1]
+	rawCmd = strings.TrimSpace(containerRegex.ReplaceAllString(rawCmd, ""))
 	// exit early if we are out of arguments
-	if len(args) == 2 {
-		return service, container, "", nil
+	if len(cmd) == 2 {
+		return service, container, "", rawCmd
 	}
 	// container= matched, so check for logs=
-	logsMatches := logsRegex.FindStringSubmatch(args[2])
+	logsMatches := logsRegex.FindStringSubmatch(cmd[2])
 	if len(logsMatches) == 0 {
 		// no logs= match, so just return the remaining args
-		return service, container, "", args[2:]
+		return service, container, "", rawCmd
 	}
+	rawCmd = strings.TrimSpace(logsRegex.ReplaceAllString(rawCmd, ""))
 	// container= and logs= matched, so return both
-	return service, container, logsMatches[1], args[3:]
+	return service, container, logsMatches[1], rawCmd
 }
 
 // parseLogsArg checks that:
@@ -104,8 +110,8 @@ func parseConnectionParams(args []string) (string, string, string, []string) {
 //
 // Note that if multiple tailLines= values are specified, the last one will be
 // the value used.
-func parseLogsArg(service, logs string, cmd []string) (bool, int64, error) {
-	if len(cmd) != 0 {
+func parseLogsArg(service, logs string, rawCmd string) (bool, int64, error) {
+	if len(rawCmd) != 0 {
 		return false, 0, ErrCmdArgsAfterLogs
 	}
 	if service == "" {