fix processmanager executable path

internal/processmanager/README.md

@@ -45,6 +45,41 @@ unreachable container runtime — is reported as undetermined rather than as a
 stopped server. The loop leaves such a server alone: treating an unreadable probe
 as "down" would restart a healthy server.
 
+## How the start command's program is found
+
+A start command names its program relative to the directory the game server
+process runs in — `./srcds_run` on Linux, `srcds.exe` on Windows. That is the
+server directory joined with the configured `work_dir` (see the root README),
+and the server directory itself when no `work_dir` is set. The supervisor that
+launches the command does not run from there. systemd expands `ExecStart=` before it
+applies `WorkingDirectory=`, and Windows resolves a program that contains a path
+separator against the directory of the process that asked for the start, never
+against the one the service is given. A command written as `.\server.exe` or
+`bin\srcds.exe` therefore names a path that does not exist, and the launch fails
+with nothing but the supervisor's own "file not found" to explain it.
+
+`systemd`, `shawl` and `winsw` all resolve the program before the command is
+written into a unit or registered as a service. That process working directory is
+searched first, PATH second, so `powershell` or `java` stays reachable while a
+binary shipped with the server always wins over a same-named one elsewhere on the
+host. What is registered is the absolute path whenever the program was found,
+which means the same file for every supervisor.
+
+The daemon's own working directory is not searched at all, and only absolute PATH
+entries are searched. Windows looks in the calling process's directory before
+PATH, which would let a file dropped next to the daemon binary stand in for the
+interpreter a game server asked for.
+
+The two Windows managers keep an unresolved command as it stands instead of
+refusing to register the service: shawl searches its own `--cwd` for a name that
+carries no path separator, and a game server whose files are downloaded on the
+first start would otherwise never get to run. `systemd` fails instead, because a
+unit is written once and a wrong `ExecStart=` would keep failing silently.
+
+Because the registered command line changes from a relative path to an absolute
+one, Windows services registered by an older daemon differ from the ones the
+config now describes and are registered again on the next start.
+
 ## Metrics support
 
 The `Metrics(ctx, server)` method returns Prometheus-style samples (see
@@ -167,6 +202,12 @@ service control manager accepts the request, and tails the shawl log when a serv
 immediately. Because shawl restarts the game process itself, a running service proves the
 supervisor came up, not that the game stayed up.
 
+All shawl writes about a program it could not launch is `program not found`, which names
+neither the file it wanted nor the directories it searched. When a service stops immediately
+and the start command's program is in neither the working directory nor PATH, the daemon says
+so before the log tail, naming both — the difference between an archive that unpacked into a
+subdirectory and a game that crashed on startup, which are fixed in entirely different places.
+
 Metrics are liveness-only; see the table above.
 
 ### Changing the restart policy
@@ -239,6 +280,13 @@ with that relative path, e.g. `/server/GroundBranch/Binaries/Linux`. In the star
 command `{dir}` expands to `docker_workdir` and `{work_dir}` to that container
 working directory, not to the host paths.
 
+A configured home directory (`home_dir`, `home_dir_linux`, `home_dir_windows`,
+`home_dir_macos`; see the root README) is resolved the same way: `HOME` is set to
+`docker_workdir` joined with that relative path, so a `home_dir` of `.` gives
+`HOME=/server`. During installation the server directory is mounted at
+`/mnt/server` instead, and `HOME` follows it there. Without a configured
+`home_dir` the container keeps whatever `HOME` its image sets.
+
 #### Installation Configuration
 
 | Key                              | Description                       | Example                 | Default       |
@@ -442,7 +490,9 @@ Podman uses the same metadata keys as Docker for compatibility:
 As with Docker, the server directory is mounted at `docker_workdir`, which is the
 container working directory by default; a configured process work directory
 (`work_dir*` keys, see the root README) is joined onto it. `{dir}` and `{work_dir}`
-in the start command expand to those container paths.
+in the start command expand to those container paths. A configured `home_dir*`
+is joined onto `docker_workdir` as well and exported as `HOME`, and installation
+uses the `/mnt/server` mount for both.
 
 ### Socket Configuration
 

internal/processmanager/executable.go

@@ -0,0 +1,97 @@
+package processmanager
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+
+	"github.com/pkg/errors"
+)
+
+// resolveCommandExecutable returns the absolute path of the program a start command begins
+// with, looking for it in the directory the game server process runs in and then in PATH.
+//
+// Supervisors do not resolve a relative program against the working directory they are told to
+// use. systemd expands ExecStart= before it applies WorkingDirectory=, and Windows resolves a
+// program that carries a path separator against the directory of the process that requested the
+// start, never against the one the service is given. A start command spelled `.\server.exe` or
+// `bin\srcds.exe` — the spelling every Linux entry in the games catalogue uses — therefore names
+// a path that does not exist, and the game server fails to launch with nothing but the
+// supervisor's own "file not found" to go on. An absolute path means the same file for every
+// supervisor, whatever directory it happens to start from.
+//
+// A bare name that the server directory does not hold falls back to PATH, so an interpreter
+// such as powershell or java stays reachable.
+func resolveCommandExecutable(cmd, processWorkDir string) (string, error) {
+	if filepath.IsAbs(cmd) {
+		path, err := lookPathAbs(cmd)
+		if err != nil {
+			return "", errors.WithMessagef(err, "failed to find command %q", cmd)
+		}
+
+		return path, nil
+	}
+
+	path, workDirErr := lookPathAbs(filepath.Join(processWorkDir, cmd))
+	if workDirErr == nil {
+		return path, nil
+	}
+
+	path, pathErr := lookPathInPATH(cmd)
+	if pathErr == nil {
+		return path, nil
+	}
+
+	return "", errors.WithMessagef(
+		workDirErr, "failed to find command %q in %q and in PATH", cmd, processWorkDir,
+	)
+}
+
+// lookPathAbs is exec.LookPath with a result that is always absolute.
+//
+// A hit reported as exec.ErrDot is refused rather than used. LookPath returns that sentinel
+// when the name resolved inside the calling process's own working directory, which on Windows
+// is searched implicitly and before PATH. That directory holds the daemon binary and has
+// nothing to do with the game server, so a file dropped next to the daemon must never become
+// the program a service is registered with.
+func lookPathAbs(name string) (string, error) {
+	path, err := exec.LookPath(name)
+	if err != nil {
+		return "", errors.Wrapf(err, "failed to look up %q", name)
+	}
+
+	abs, err := filepath.Abs(path)
+	if err != nil {
+		return "", errors.Wrapf(err, "failed to make path %q absolute", path)
+	}
+
+	return abs, nil
+}
+
+// lookPathInPATH searches PATH alone for a command name.
+//
+// exec.LookPath cannot do this on Windows: it looks in the calling process's working directory
+// first and returns that hit instead of going on to PATH, so refusing the hit afterwards would
+// also lose the interpreter that PATH really does provide. Each PATH entry is therefore joined
+// with the name and checked on its own, which keeps LookPath's PATHEXT handling while leaving
+// the implicit search of the daemon's own directory out of it.
+//
+// Entries that are not absolute are skipped. The resolved path is written into a unit or a
+// service that a supervisor starts from some other directory, where a path that only means
+// something relative to the daemon's working directory would point somewhere else or nowhere.
+func lookPathInPATH(cmd string) (string, error) {
+	for _, dir := range filepath.SplitList(os.Getenv("PATH")) {
+		if !filepath.IsAbs(dir) {
+			continue
+		}
+
+		path, err := exec.LookPath(filepath.Join(dir, cmd))
+		if err != nil {
+			continue
+		}
+
+		return path, nil
+	}
+
+	return "", errors.Errorf("failed to find %q in PATH", cmd)
+}

internal/processmanager/executable_test.go

@@ -0,0 +1,176 @@
+package processmanager
+
+import (
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// executableName is the file name a start command would name on the running OS, together with
+// the token a games catalogue entry spells it with.
+func executableName() (fileName, bareToken string) {
+	if runtime.GOOS == "windows" {
+		return "server.exe", "server.exe"
+	}
+
+	return "server", "server"
+}
+
+func writeExecutable(t *testing.T, dir, name string) string {
+	t.Helper()
+
+	path := filepath.Join(dir, name)
+	require.NoError(t, os.WriteFile(path, []byte("#!/bin/sh\n"), 0o755))
+
+	return path
+}
+
+// A command with a path separator is the case supervisors get wrong: they resolve it against
+// their own working directory rather than the one the game server is given.
+func TestResolveCommandExecutable_RelativePathWithSeparator(t *testing.T) {
+	workDir := t.TempDir()
+	subDir := filepath.Join(workDir, "bin")
+	require.NoError(t, os.Mkdir(subDir, 0o755))
+
+	fileName, _ := executableName()
+	want := writeExecutable(t, subDir, fileName)
+
+	// "." is spelled out rather than joined: filepath.Join cleans it away, and the leading
+	// separator is the whole point of the case.
+	tests := map[string]struct {
+		cmd string
+		dir string
+	}{
+		"dot_prefixed":  {cmd: "." + string(filepath.Separator) + fileName, dir: subDir},
+		"subdirectory":  {cmd: filepath.Join("bin", fileName), dir: workDir},
+		"parent_walked": {cmd: filepath.Join("..", "bin", fileName), dir: subDir},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			got, err := resolveCommandExecutable(tt.cmd, tt.dir)
+
+			require.NoError(t, err)
+			assert.Equal(t, want, got)
+		})
+	}
+}
+
+func TestResolveCommandExecutable_BareNameInWorkDir(t *testing.T) {
+	workDir := t.TempDir()
+	fileName, bareToken := executableName()
+	want := writeExecutable(t, workDir, fileName)
+
+	got, err := resolveCommandExecutable(bareToken, workDir)
+
+	require.NoError(t, err)
+	assert.Equal(t, want, got)
+}
+
+func TestResolveCommandExecutable_AbsolutePath(t *testing.T) {
+	workDir := t.TempDir()
+	fileName, _ := executableName()
+	want := writeExecutable(t, workDir, fileName)
+
+	got, err := resolveCommandExecutable(want, t.TempDir())
+
+	require.NoError(t, err)
+	assert.Equal(t, want, got)
+}
+
+// An interpreter the server directory does not hold has to stay reachable, or a start command
+// such as `powershell -File run.ps1` would stop working.
+func TestResolveCommandExecutable_FallsBackToPath(t *testing.T) {
+	binDir := t.TempDir()
+	fileName, bareToken := executableName()
+	want := writeExecutable(t, binDir, fileName)
+
+	t.Setenv("PATH", binDir)
+	if runtime.GOOS == "windows" {
+		t.Setenv("PATHEXT", ".EXE")
+	}
+
+	got, err := resolveCommandExecutable(bareToken, t.TempDir())
+
+	require.NoError(t, err)
+	assert.Equal(t, want, got)
+}
+
+// The server directory wins over PATH, so a server never runs a same-named binary from
+// somewhere else on the host.
+func TestResolveCommandExecutable_WorkDirWinsOverPath(t *testing.T) {
+	workDir := t.TempDir()
+	binDir := t.TempDir()
+	fileName, bareToken := executableName()
+
+	want := writeExecutable(t, workDir, fileName)
+	writeExecutable(t, binDir, fileName)
+
+	t.Setenv("PATH", binDir)
+	if runtime.GOOS == "windows" {
+		t.Setenv("PATHEXT", ".EXE")
+	}
+
+	got, err := resolveCommandExecutable(bareToken, workDir)
+
+	require.NoError(t, err)
+	assert.Equal(t, want, got)
+}
+
+// Windows searches the calling process's own working directory before PATH, so without an
+// explicit PATH-only lookup a file sitting next to the daemon binary would be registered as the
+// game server's interpreter instead of the real one.
+func TestResolveCommandExecutable_DaemonDirDoesNotShadowPath(t *testing.T) {
+	daemonDir := t.TempDir()
+	binDir := t.TempDir()
+	fileName, bareToken := executableName()
+
+	writeExecutable(t, daemonDir, fileName)
+	want := writeExecutable(t, binDir, fileName)
+
+	t.Chdir(daemonDir)
+	t.Setenv("PATH", binDir)
+	if runtime.GOOS == "windows" {
+		t.Setenv("PATHEXT", ".EXE")
+	}
+
+	got, err := resolveCommandExecutable(bareToken, t.TempDir())
+
+	require.NoError(t, err)
+	assert.Equal(t, want, got)
+}
+
+// A command that exists only next to the daemon is not found at all: the daemon's working
+// directory is not a place a game server's program may come from.
+func TestResolveCommandExecutable_DaemonDirIsNotSearched(t *testing.T) {
+	daemonDir := t.TempDir()
+	fileName, bareToken := executableName()
+
+	writeExecutable(t, daemonDir, fileName)
+
+	t.Chdir(daemonDir)
+	t.Setenv("PATH", t.TempDir())
+	if runtime.GOOS == "windows" {
+		t.Setenv("PATHEXT", ".EXE")
+	}
+
+	_, err := resolveCommandExecutable(bareToken, t.TempDir())
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), bareToken)
+}
+
+func TestResolveCommandExecutable_NotFound(t *testing.T) {
+	workDir := t.TempDir()
+	t.Setenv("PATH", t.TempDir())
+
+	_, err := resolveCommandExecutable("definitely-not-here", workDir)
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "definitely-not-here")
+	assert.Contains(t, err.Error(), workDir)
+}