systemd: escape and quote command

Add single-quoting and escaping of single quotes in the specified program in
a systemd daemon's "ExecStart". This allows the created daemon to properly
run a command with spaces in the path, as well as being generally safer.
Update unit tests to more thoroughly test service unit file contents.

Signed-off-by: Victoria Dye <[email protected]>

Author: vdye

internal/daemon/systemd.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"path/filepath"
+	"strings"
 	"text/template"
 
 	"github.com/github/git-bundle-server/internal/common"
@@ -14,7 +15,7 @@ Description={{.Description}}
 
 [Service]
 Type=simple
-ExecStart={{.Program}}
+ExecStart={{sq_escape .Program}}
 `
 
 type systemd struct {
@@ -43,7 +44,11 @@ func (s *systemd) Create(config *DaemonConfig, force bool) error {
 
 	// Generate the configuration
 	var newServiceUnit bytes.Buffer
-	t, err := template.New(config.Label).Parse(serviceTemplate)
+	t, err := template.New(config.Label).Funcs(template.FuncMap{
+		"sq_escape": func(str string) string {
+			return fmt.Sprintf("'%s'", strings.ReplaceAll(str, "'", "\\'"))
+		},
+	}).Parse(serviceTemplate)
 	if err != nil {
 		return fmt.Errorf("unable to generate systemd configuration: %w", err)
 	}

internal/daemon/systemd_test.go

@@ -4,14 +4,16 @@ import (
 	"fmt"
 	"os/user"
 	"path/filepath"
+	"regexp"
+	"strings"
 	"testing"
 
 	"github.com/github/git-bundle-server/internal/daemon"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 )
 
-var systemdCreateTests = []struct {
+var systemdCreateBehaviorTests = []struct {
 	title string
 
 	// Inputs
@@ -64,6 +66,43 @@ var systemdCreateTests = []struct {
 	},
 }
 
+var systemdCreateServiceUnitTests = []struct {
+	title string
+
+	// Inputs
+	config *daemon.DaemonConfig
+
+	// Expected values
+	expectedServiceUnitLines []string
+}{
+	{
+		title:  "Created service unit contents are correct",
+		config: &basicDaemonConfig,
+		expectedServiceUnitLines: []string{
+			"[Unit]",
+			fmt.Sprintf("Description=%s", basicDaemonConfig.Description),
+			"[Service]",
+			"Type=simple",
+			fmt.Sprintf("ExecStart='%s'", basicDaemonConfig.Program),
+		},
+	},
+	{
+		title: "Service unit ExecStart with space is quoted",
+		config: &daemon.DaemonConfig{
+			Label:       "test-quoting",
+			Description: "My program's description (double quotes \" are ok too)",
+			Program:     "/path/to/the/program with a space",
+		},
+		expectedServiceUnitLines: []string{
+			"[Unit]",
+			"Description=My program's description (double quotes \" are ok too)",
+			"[Service]",
+			"Type=simple",
+			"ExecStart='/path/to/the/program with a space'",
+		},
+	},
+}
+
 func TestSystemd_Create(t *testing.T) {
 	// Set up mocks
 	testUser := &user.User{
@@ -80,7 +119,7 @@ func TestSystemd_Create(t *testing.T) {
 
 	systemd := daemon.NewSystemdProvider(testUserProvider, testCommandExecutor, testFileSystem)
 
-	for _, tt := range systemdCreateTests {
+	for _, tt := range systemdCreateBehaviorTests {
 		forceArg := tt.force.toBoolList()
 		for _, force := range forceArg {
 			t.Run(fmt.Sprintf("%s (force='%t')", tt.title, force), func(t *testing.T) {
@@ -122,45 +161,54 @@ func TestSystemd_Create(t *testing.T) {
 	}
 
 	// Verify content of created file
-	t.Run("Created file content and path are correct", func(t *testing.T) {
-		var actualFilename string
-		var actualFileBytes []byte
-
-		// Mock responses for successful fresh write
-		testCommandExecutor.On("Run",
-			"systemctl",
-			[]string{"--user", "daemon-reload"},
-		).Return(0, nil).Once()
-		testFileSystem.On("FileExists",
-			mock.AnythingOfType("string"),
-		).Return(false, nil).Once()
-
-		// Use mock to save off input args
-		testFileSystem.On("WriteFile",
-			mock.MatchedBy(func(filename string) bool {
-				actualFilename = filename
-				return true
-			}),
-			mock.MatchedBy(func(fileBytes any) bool {
-				// Save off value and always match
-				actualFileBytes = fileBytes.([]byte)
-				return true
-			}),
-		).Return(nil).Once()
-
-		err := systemd.Create(&basicDaemonConfig, false)
-		assert.Nil(t, err)
-		mock.AssertExpectationsForObjects(t, testCommandExecutor, testFileSystem)
-
-		// Check filename
-		expectedFilename := filepath.Clean(fmt.Sprintf("/my/test/dir/.config/systemd/user/%s.service", basicDaemonConfig.Label))
-		assert.Equal(t, expectedFilename, actualFilename)
-
-		// Check file contents
-		fileContents := string(actualFileBytes)
-		assert.Contains(t, fileContents, fmt.Sprintf("ExecStart=%s", basicDaemonConfig.Program))
-		assert.Contains(t, fileContents, fmt.Sprintf("Description=%s", basicDaemonConfig.Description))
-	})
+	for _, tt := range systemdCreateServiceUnitTests {
+		t.Run(tt.title, func(t *testing.T) {
+			var actualFilename string
+			var actualFileBytes []byte
+
+			// Mock responses for successful fresh write
+			testCommandExecutor.On("Run",
+				"systemctl",
+				[]string{"--user", "daemon-reload"},
+			).Return(0, nil).Once()
+			testFileSystem.On("FileExists",
+				mock.AnythingOfType("string"),
+			).Return(false, nil).Once()
+
+			// Use mock to save off input args
+			testFileSystem.On("WriteFile",
+				mock.MatchedBy(func(filename string) bool {
+					actualFilename = filename
+					return true
+				}),
+				mock.MatchedBy(func(fileBytes any) bool {
+					// Save off value and always match
+					actualFileBytes = fileBytes.([]byte)
+					return true
+				}),
+			).Return(nil).Once()
+
+			err := systemd.Create(tt.config, false)
+			assert.Nil(t, err)
+			mock.AssertExpectationsForObjects(t, testCommandExecutor, testFileSystem)
+
+			// Check filename
+			expectedFilename := filepath.Clean(fmt.Sprintf("/my/test/dir/.config/systemd/user/%s.service", tt.config.Label))
+			assert.Equal(t, expectedFilename, actualFilename)
+
+			// Check file contents
+			// Ensure there's no more than one newline between each line
+			// before splitting the file.
+			fileContents := strings.TrimSpace(string(actualFileBytes))
+			serviceUnitLines := strings.Split(
+				regexp.MustCompile(`\n+`).ReplaceAllString(fileContents, "\n"), "\n")
+			assert.ElementsMatch(t, tt.expectedServiceUnitLines, serviceUnitLines)
+
+			// Reset mocks
+			testCommandExecutor.Mock = mock.Mock{}
+			testFileSystem.Mock = mock.Mock{}
+		})
+	}
 }
 
 func TestSystemd_Start(t *testing.T) {