Merge pull request #9178 from Luap99/fix-9176

Fix podman generate systemd --new special char handling

5 files changed, 93 insertions, 9 deletions

diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go
index de6751a17..e9902319c 100644
--- a/pkg/systemd/generate/common.go
+++ b/pkg/systemd/generate/common.go
@@ -60,13 +60,21 @@ func filterPodFlags(command []string) []string {
 	return processed
 }
 
-// quoteArguments makes sure that all arguments with at least one whitespace
+// escapeSystemdArguments makes sure that all arguments with at least one whitespace
 // are quoted to make sure those are interpreted as one argument instead of
-// multiple ones.
-func quoteArguments(command []string) []string {
+// multiple ones. Also make sure to escape all characters which have a special
+// meaning to systemd -> $,% and \
+// see: https://www.freedesktop.org/software/systemd/man/systemd.service.html#Command%20lines
+func escapeSystemdArguments(command []string) []string {
 	for i := range command {
+		command[i] = strings.ReplaceAll(command[i], "$", "$$")
+		command[i] = strings.ReplaceAll(command[i], "%", "%%")
 		if strings.ContainsAny(command[i], " \t") {
 			command[i] = strconv.Quote(command[i])
+		} else if strings.Contains(command[i], `\`) {
+			// strconv.Quote also escapes backslashes so
+			// we should replace only if strconv.Quote was not used
+			command[i] = strings.ReplaceAll(command[i], `\`, `\\`)
 		}
 	}
 	return command
diff --git a/pkg/systemd/generate/common_test.go b/pkg/systemd/generate/common_test.go
index d0ec5637c..a0691d1ad 100644
--- a/pkg/systemd/generate/common_test.go
+++ b/pkg/systemd/generate/common_test.go
@@ -29,7 +29,7 @@ func TestFilterPodFlags(t *testing.T) {
 	}
 }
 
-func TestQuoteArguments(t *testing.T) {
+func TestEscapeSystemdArguments(t *testing.T) {
 	tests := []struct {
 		input  []string
 		output []string
@@ -46,10 +46,46 @@ func TestQuoteArguments(t *testing.T) {
 			[]string{"foo", "bar=\"arg with\ttab\""},
 			[]string{"foo", "\"bar=\\\"arg with\\ttab\\\"\""},
 		},
+		{
+			[]string{"$"},
+			[]string{"$$"},
+		},
+		{
+			[]string{"foo", "command with dollar sign $"},
+			[]string{"foo", "\"command with dollar sign $$\""},
+		},
+		{
+			[]string{"foo", "command with two dollar signs $$"},
+			[]string{"foo", "\"command with two dollar signs $$$$\""},
+		},
+		{
+			[]string{"%"},
+			[]string{"%%"},
+		},
+		{
+			[]string{"foo", "command with percent sign %"},
+			[]string{"foo", "\"command with percent sign %%\""},
+		},
+		{
+			[]string{"foo", "command with two percent signs %%"},
+			[]string{"foo", "\"command with two percent signs %%%%\""},
+		},
+		{
+			[]string{`\`},
+			[]string{`\\`},
+		},
+		{
+			[]string{"foo", `command with backslash \`},
+			[]string{"foo", `"command with backslash \\"`},
+		},
+		{
+			[]string{"foo", `command with two backslashs \\`},
+			[]string{"foo", `"command with two backslashs \\\\"`},
+		},
 	}
 
 	for _, test := range tests {
-		quoted := quoteArguments(test.input)
+		quoted := escapeSystemdArguments(test.input)
 		assert.Equal(t, test.output, quoted)
 	}
 }
diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go
index 5f52b0a77..abe159812 100644
--- a/pkg/systemd/generate/containers.go
+++ b/pkg/systemd/generate/containers.go
@@ -204,7 +204,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 		startCommand := []string{info.Executable}
 		if index > 2 {
 			// include root flags
-			info.RootFlags = strings.Join(quoteArguments(info.CreateCommand[1:index-1]), " ")
+			info.RootFlags = strings.Join(escapeSystemdArguments(info.CreateCommand[1:index-1]), " ")
 			startCommand = append(startCommand, info.CreateCommand[1:index-1]...)
 		}
 		startCommand = append(startCommand,
@@ -279,7 +279,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 			}
 		}
 		startCommand = append(startCommand, remainingCmd...)
-		startCommand = quoteArguments(startCommand)
+		startCommand = escapeSystemdArguments(startCommand)
 
 		info.ExecStartPre = "/bin/rm -f {{{{.PIDFile}}}} {{{{.ContainerIDFile}}}}"
 		info.ExecStart = strings.Join(startCommand, " ")
diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go
index 96d95644b..be14e4c28 100644
--- a/pkg/systemd/generate/containers_test.go
+++ b/pkg/systemd/generate/containers_test.go
@@ -352,6 +352,30 @@ Type=forking
 [Install]
 WantedBy=multi-user.target default.target
 `
+
+	goodNewWithSpecialChars := `# jadda-jadda.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman jadda-jadda.service
+Documentation=man:podman-generate-systemd(1)
+Wants=network.target
+After=network-online.target
+
+[Service]
+Environment=PODMAN_SYSTEMD_UNIT=%n
+Restart=always
+TimeoutStopSec=70
+ExecStartPre=/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id
+ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon -d --replace --name test awesome-image:latest sh -c "kill $$$$ && echo %%\\"
+ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 10
+ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
+PIDFile=%t/jadda-jadda.pid
+Type=forking
+
+[Install]
+WantedBy=multi-user.target default.target
+`
 	tests := []struct {
 		name    string
 		info    containerInfo
@@ -647,6 +671,22 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 		},
+		{"good with special chars",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				RestartPolicy:     "always",
+				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       10,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "create", "--name", "test", "awesome-image:latest", "sh", "-c", "kill $$ && echo %\\"},
+				EnvVariable:       EnvVariable,
+			},
+			goodNewWithSpecialChars,
+			true,
+			false,
+		},
 	}
 	for _, tt := range tests {
 		test := tt
diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go
index c7e3aa955..d6ede19af 100644
--- a/pkg/systemd/generate/pods.go
+++ b/pkg/systemd/generate/pods.go
@@ -269,7 +269,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 				return "", errors.Errorf("pod does not appear to be created via `podman pod create`: %v", info.CreateCommand)
 			}
 			podRootArgs = info.CreateCommand[1 : podCreateIndex-1]
-			info.RootFlags = strings.Join(quoteArguments(podRootArgs), " ")
+			info.RootFlags = strings.Join(escapeSystemdArguments(podRootArgs), " ")
 			podCreateArgs = filterPodFlags(info.CreateCommand[podCreateIndex+1:])
 		}
 		// We're hard-coding the first five arguments and append the
@@ -306,7 +306,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 		}
 
 		startCommand = append(startCommand, podCreateArgs...)
-		startCommand = quoteArguments(startCommand)
+		startCommand = escapeSystemdArguments(startCommand)
 
 		info.ExecStartPre1 = "/bin/rm -f {{{{.PIDFile}}}} {{{{.PodIDFile}}}}"
 		info.ExecStartPre2 = strings.Join(startCommand, " ")