Fix an infinite exec bug

cmd/launcher/main.go

@@ -38,6 +38,10 @@ func main() {
 	// This does not call DeleteOldUpdates, on the theory that
 	// it's better left to the service to handle cleanup. This is
 	// a straight forward exec.
+	//
+	// launcher is _also_ called when we're checking update
+	// validity (with autoupdate.checkExecutable). This is
+	// somewhat awkward as we end up with extra call layers.
 	newerBinary, err := autoupdate.FindNewestSelf(ctx)
 	if err != nil {
 		logutil.Fatal(logger, err, "checking for updated version")

cmd/launcher/updater-finalizer.go

@@ -25,13 +25,19 @@ func updateFinalizer(logger log.Logger, shutdownOsquery func() error) func() err
 			level.Debug(logger).Log("method", "updateFinalizer", "err", err, "stack", fmt.Sprintf("%+v", err))
 		}
 		// find the newest version of launcher on disk.
-		// FindNewest uses context as a way to get a logger, so we need to create and pass one.
-		binaryPath := autoupdate.FindNewest(
+		// FindNewestSelf uses context as a way to get a
+		// logger, so we need to create and pass one.
+		binaryPath, err := autoupdate.FindNewestSelf(
 			ctxlog.NewContext(context.TODO(), logger),
-			os.Args[0],
+			autoupdate.DeleteCorruptUpdates(),
 			autoupdate.DeleteOldUpdates(),
 		)
 
+		if err != nil {
+			level.Info(logger).Log("method", "updateFinalizer", "err", err)
+			return errors.Wrap(err, "finding newest")
+		}
+
 		// replace launcher
 		level.Info(logger).Log(
 			"msg", "Exec updated launcher",

cmd/launcher/updater-finalizer_windows.go

@@ -5,7 +5,6 @@ package main
 import (
 	"context"
 	"fmt"
-	"os"
 
 	"github.com/go-kit/kit/log"
 	"github.com/go-kit/kit/log/level"
@@ -30,9 +29,9 @@ func updateFinalizer(logger log.Logger, shutdownOsquery func() error) func() err
 		// the update in main, which does not delete.  Note
 		// that this will likely produce non-fatal errors when
 		// it tries to delete the running one.
-		_ = autoupdate.FindNewest(
+		autoupdate.FindNewestSelf(
 			ctxlog.NewContext(context.TODO(), logger),
-			os.Args[0],
+			autoupdate.DeleteCorruptUpdates(),
 			autoupdate.DeleteOldUpdates(),
 		)
 

pkg/autoupdate/findnew.go

@@ -20,8 +20,9 @@ import (
 const updateDirSuffix = "-updates"
 
 type newestSettings struct {
-	deleteOld     bool
-	deleteCorrupt bool
+	deleteOld               bool
+	deleteCorrupt           bool
+	skipFullBinaryPathCheck bool
 }
 
 type newestOption func(*newestSettings)
@@ -38,6 +39,16 @@ func DeleteCorruptUpdates() newestOption {
 	}
 }
 
+// SkipFullBinaryPathCheck skips the final check on FindNewest. This
+// is desirable when being called by FindNewestSelf, otherewise we end
+// up in a infineite recursion. (The recursion is saved by the exec
+// check, but it's better not to trigger it.
+func SkipFullBinaryPathCheck() newestOption {
+	return func(no *newestSettings) {
+		no.skipFullBinaryPathCheck = true
+	}
+}
+
 // FindNewestSelf invokes `FindNewest` with the running binary path,
 // as determined by os.Executable. However, if the current running
 // version is the same as the newest on disk, it will return empty string.
@@ -53,6 +64,8 @@ func FindNewestSelf(ctx context.Context, opts ...newestOption) (string, error) {
 		return "", errors.New("can't find newest empty string")
 	}
 
+	opts = append(opts, SkipFullBinaryPathCheck())
+
 	newest := FindNewest(ctx, exPath, opts...)
 
 	if newest == "" {
@@ -159,6 +172,10 @@ func FindNewest(ctx context.Context, fullBinaryPath string, opts ...newestOption
 
 	level.Debug(logger).Log("msg", "no updates found")
 
+	if newestSettings.skipFullBinaryPathCheck {
+		return fullBinaryPath
+	}
+
 	if err := checkExecutable(ctx, fullBinaryPath, "--version"); err == nil {
 		return fullBinaryPath
 	}