Check children path during wrapper boot

pidfile content may be a reassigned to a new process and gitaly-wrapper will
not start a new gitaly instance.

This commit checks if the process path includes gitaly before adopting a child
process.

5 files changed, 150 insertions, 0 deletions

diff --git a/changelogs/unreleased/ac-wrapper-check-pid.yml b/changelogs/unreleased/ac-wrapper-check-pid.yml
new file mode 100644
index 000000000..59a00bbaa
--- /dev/null
+++ b/changelogs/unreleased/ac-wrapper-check-pid.yml
@@ -0,0 +1,5 @@
+---
+title: "Check if PID belongs to Gitaly before adopting an existing process"
+merge_request: 1249
+author:
+type: fixed
diff --git a/cmd/gitaly-wrapper/main.go b/cmd/gitaly-wrapper/main.go
index 646661b47..4a8ea6ae2 100644
--- a/cmd/gitaly-wrapper/main.go
+++ b/cmd/gitaly-wrapper/main.go
@@ -6,6 +6,7 @@ import (
 	"os"
 	"os/exec"
 	"os/signal"
+	"path"
 	"strconv"
 	"syscall"
 	"time"
@@ -81,6 +82,10 @@ func findGitaly() (*os.Process, error) {
 	}
 
 	if isAlive(gitaly) {
+		if ok, err := isGitaly(pid); !ok {
+			return nil, err
+		}
+
 		return gitaly, nil
 	}
 
@@ -138,6 +143,19 @@ func isAlive(p *os.Process) bool {
 	return p.Signal(syscall.Signal(0)) == nil
 }
 
+func isGitaly(pid int) (bool, error) {
+	command, err := procPath(pid)
+	if err != nil {
+		return false, err
+	}
+
+	if path.Base(command) == "gitaly" {
+		return true, nil
+	}
+
+	return false, nil
+}
+
 func pidFile() string {
 	return os.Getenv(config.EnvPidFile)
 }
diff --git a/cmd/gitaly-wrapper/main_test.go b/cmd/gitaly-wrapper/main_test.go
new file mode 100644
index 000000000..cde68e840
--- /dev/null
+++ b/cmd/gitaly-wrapper/main_test.go
@@ -0,0 +1,92 @@
+package main
+
+import (
+	"io"
+	"io/ioutil"
+	"os"
+	"os/exec"
+	"path"
+	"strconv"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/config"
+)
+
+// TestStolenPid tests for regressions in https://gitlab.com/gitlab-org/gitaly/issues/1661
+func TestStolenPid(t *testing.T) {
+	defer func(oldValue string) {
+		os.Setenv(config.EnvPidFile, oldValue)
+	}(os.Getenv(config.EnvPidFile))
+
+	pidFile, err := ioutil.TempFile("", "pidfile")
+	require.NoError(t, err)
+	defer os.Remove(pidFile.Name())
+
+	os.Setenv(config.EnvPidFile, pidFile.Name())
+
+	cmd := exec.Command("tail", "-f")
+	require.NoError(t, cmd.Start())
+	defer cmd.Process.Kill()
+
+	_, err = pidFile.WriteString(strconv.Itoa(cmd.Process.Pid))
+	require.NoError(t, err)
+	require.NoError(t, pidFile.Close())
+
+	gitaly, err := findGitaly()
+	require.NoError(t, err)
+	require.Nil(t, gitaly)
+}
+
+func TestExistingGitaly(t *testing.T) {
+	defer func(oldValue string) {
+		os.Setenv(config.EnvPidFile, oldValue)
+	}(os.Getenv(config.EnvPidFile))
+
+	tmpDir, err := ioutil.TempDir("", "gitaly-pid")
+	require.NoError(t, err)
+	defer os.RemoveAll(tmpDir)
+
+	pidFile := path.Join(tmpDir, "gitaly.pid")
+	fakeGitaly := path.Join(tmpDir, "gitaly")
+
+	require.NoError(t, buildFakeGitaly(t, fakeGitaly), "Can't build a fake gitaly binary")
+
+	os.Setenv(config.EnvPidFile, pidFile)
+
+	cmd := exec.Command(fakeGitaly, "-f")
+	require.NoError(t, cmd.Start())
+	defer cmd.Process.Kill()
+
+	require.NoError(t, ioutil.WriteFile(pidFile, []byte(strconv.Itoa(cmd.Process.Pid)), 0644))
+
+	gitaly, err := findGitaly()
+	require.NoError(t, err)
+	require.NotNil(t, gitaly)
+	require.Equal(t, cmd.Process.Pid, gitaly.Pid)
+	gitaly.Kill()
+}
+
+func buildFakeGitaly(t *testing.T, path string) error {
+	tail := exec.Command("tail", "-f")
+	require.NoError(t, tail.Start())
+	defer tail.Process.Kill()
+
+	tailPath, err := procPath(tail.Process.Pid)
+	require.NoError(t, err)
+	tail.Process.Kill()
+
+	src, err := os.Open(tailPath)
+	require.NoError(t, err)
+	defer src.Close()
+
+	out, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, 0744)
+	require.NoError(t, err)
+	defer out.Close()
+
+	if _, err := io.Copy(out, src); err != nil {
+		return err
+	}
+
+	return out.Sync()
+}
diff --git a/cmd/gitaly-wrapper/proc_path_darwin.go b/cmd/gitaly-wrapper/proc_path_darwin.go
new file mode 100644
index 000000000..4979cc203
--- /dev/null
+++ b/cmd/gitaly-wrapper/proc_path_darwin.go
@@ -0,0 +1,25 @@
+package main
+
+// #include <libproc.h>
+// #include <stdlib.h>
+import "C"
+
+import (
+	"fmt"
+	"unsafe"
+)
+
+func procPath(pid int) (string, error) {
+	// MacOS does not implement procfs, this simple function calls proc_pidpath from MacOS libproc
+	// https://opensource.apple.com/source/xnu/xnu-2422.1.72/libsyscall/wrappers/libproc/libproc.h.auto.html
+	// this is just for testing purpose as we do not support MacOS as a production environment
+
+	buf := C.CString(string(make([]byte, C.PROC_PIDPATHINFO_MAXSIZE)))
+	defer C.free(unsafe.Pointer(buf))
+
+	if ret, err := C.proc_pidpath(C.int(pid), unsafe.Pointer(buf), C.PROC_PIDPATHINFO_MAXSIZE); ret <= 0 {
+		return "", fmt.Errorf("failed process path retrieval: %v", err)
+	}
+
+	return C.GoString(buf), nil
+}
diff --git a/cmd/gitaly-wrapper/proc_path_linux.go b/cmd/gitaly-wrapper/proc_path_linux.go
new file mode 100644
index 000000000..d48828a93
--- /dev/null
+++ b/cmd/gitaly-wrapper/proc_path_linux.go
@@ -0,0 +1,10 @@
+package main
+
+import (
+	"os"
+	"strconv"
+)
+
+func procPath(pid int) (path string, err error) {
+	return os.Readlink("/proc/" + strconv.Itoa(pid) + "/exe")
+}