fix: Correct installer log problems and its wrong error handling

- Installer is guarded to only run once.
- Installer log should now be visible when installer crashes.

githooks/apps/cli/cli.go

@@ -10,7 +10,25 @@ import (
 	"github.com/gabyx/githooks/githooks/hooks"
 )
 
-func mainRun() (exitCode int) {
+func installSignalHandling() *cm.InterruptContext {
+	var cleanUpX cm.InterruptContext
+
+	c := make(chan os.Signal, 1)
+	signal.Notify(c, os.Interrupt)
+	defer func() {
+		signal.Stop(c)
+	}()
+
+	go func() {
+		<-c
+		cleanUpX.RunHandlers()
+		os.Exit(1) // Return 1 := canceled always...
+	}()
+
+	return &cleanUpX
+}
+
+func mainRun(cleanUpX *cm.InterruptContext) (exitCode int) {
 
 	log, err := cm.CreateLogContext(false)
 	cm.AssertOrPanic(err == nil, "Could not create log")
@@ -29,21 +47,7 @@ func mainRun() (exitCode int) {
 		}
 	}()
 
-	// Install signal handling
-	var cleanUpX cm.InterruptContext
-	c := make(chan os.Signal, 1)
-	signal.Notify(c, os.Interrupt)
-	defer func() {
-		signal.Stop(c)
-	}()
-
-	go func() {
-		<-c
-		cleanUpX.RunHandlers()
-		os.Exit(1) // Return 1 := canceled always...
-	}()
-
-	err = cmd.Run(log, log, wrapPanicExitCode, &cleanUpX)
+	err = cmd.Run(log, log, wrapPanicExitCode, cleanUpX)
 
 	// Overwrite the exit code if its a command exit error.
 	if v, ok := err.(ccm.CmdExit); ok {
@@ -56,5 +60,8 @@ func mainRun() (exitCode int) {
 }
 
 func main() {
-	os.Exit(mainRun())
+	cleanUpX := installSignalHandling()
+	exitCode := mainRun(cleanUpX)
+	cleanUpX.RunHandlers()
+	os.Exit(exitCode)
 }

githooks/apps/cli/cli_test.go

@@ -4,6 +4,7 @@ package main
 
 import (
 	"github.com/gabyx/githooks/githooks/apps/coverage"
+	cm "github.com/gabyx/githooks/githooks/common"
 
 	"testing"
 )
@@ -14,7 +15,11 @@ func TestCoverage(t *testing.T) {
 		// fmt.Printf("Forward args: %q\n", os.Args)
 
 		// Run the main binary...
-		if mainRun() != 0 {
+		var cleanUpX cm.InterruptContext
+		exitCode := mainRun(&cleanUpX)
+		cleanUpX.RunHandlers()
+
+		if exitCode != 0 {
 			t.Fatal()
 		}
 	}

githooks/cmd/installer/installer.go

@@ -36,8 +36,8 @@ func NewCmd(ctx *ccm.CmdContext) *cobra.Command {
 		Long: "Githooks installer application\n" +
 			"See further information at https://github.com/gabyx/githooks/blob/main/README.md",
 		PreRun: ccm.PanicIfAnyArgs(ctx.Log),
-		Run: func(cmd *cobra.Command, _ []string) {
-			runInstall(cmd, ctx, vi)
+		RunE: func(cmd *cobra.Command, _ []string) error {
+			return runInstall(cmd, ctx, vi)
 		}}
 
 	defineArguments(cmd, vi)
@@ -375,16 +375,18 @@ func getDeploySettings(
 	return deploySettings
 }
 
-func prepareDispatch(
+func runInstallDispatched(
 	log cm.ILogContext,
 	gitx *git.Context,
 	settings *Settings,
-	args *Arguments,
+	args Arguments,
 	cleanUpX *cm.InterruptContext) (bool, error) {
 
 	var status updates.ReleaseStatus
 	var err error
 
+	log.Info("Running dispatched installer.")
+
 	if args.InternalAutoUpdate {
 		log.Info("Executing auto update...")
 
@@ -411,9 +413,7 @@ func prepareDispatch(
 		log.DebugF("Status: %v", status)
 	}
 
-	updateAvailable := status.LocalCommitSHA != status.RemoteCommitSHA
-
-	cm.PanicIfF(args.InternalAutoUpdate && !updateAvailable,
+	cm.PanicIfF(args.InternalAutoUpdate && !status.IsUpdateAvailable,
 		"An autoupdate should only be triggered when and update is found.")
 
 	installer := hooks.GetInstallerExecutable(settings.InstallDir)
@@ -423,9 +423,12 @@ func prepareDispatch(
 	// or the installer is missing.
 	binaries := updates.Binaries{}
 
-	if updateAvailable || !haveInstaller {
+	log.InfoF("Githooks update available: '%v'", status.IsUpdateAvailable)
+	log.InfoF("Githooks installer existing: '%v'", haveInstaller)
+
+	if status.IsUpdateAvailable || !haveInstaller {
 
-		log.Info("Getting Githooks binaries...")
+		log.Info("Getting Githooks binaries ...")
 
 		tempDir, err := os.MkdirTemp(os.TempDir(), "githooks-update-*")
 		log.AssertNoErrorPanic(err, "Can not create temporary update dir in '%s'", os.TempDir())
@@ -459,17 +462,18 @@ func prepareDispatch(
 
 			log.InfoF("Download '%s' from deploy source...", tag)
 
-			deploySettings := getDeploySettings(log, settings.InstallDir, status.RemoteURL, args)
+			deploySettings := getDeploySettings(log, settings.InstallDir, status.RemoteURL, &args)
 			binaries = downloadBinaries(log, deploySettings, tempDir, tag)
 		}
 
 		installer.Cmd = binaries.Cli
 	}
 
 	// Set variables for further update procedure...
+	// Note: `args` is passed by value.
 	args.InternalPostDispatch = true
 	args.InternalBinaries = binaries.All
-	if updateAvailable {
+	if status.IsUpdateAvailable {
 		args.InternalUpdateFromVersion = build.BuildVersion
 		args.InternalUpdateTo = status.UpdateCommitSHA
 	}
@@ -480,7 +484,7 @@ func prepareDispatch(
 
 	log.PanicIfF(!cm.IsFile(installer.Cmd), "Githooks executable '%s' is not existing.", installer)
 
-	return true, runInstaller(log, &installer, args)
+	return true, runInstaller(log, &installer, &args)
 }
 
 func runInstaller(log cm.ILogContext, installer cm.IExecutable, args *Arguments) error {
@@ -1287,28 +1291,50 @@ func runUpdate(
 	}
 }
 
-func addInstallerLog(path string, log cm.ILogContext) string {
+func addInstallerLog(path string, log cm.ILogContext) (isDefault bool, resPath string) {
 	var err error
 	var file *os.File
 
 	if strs.IsEmpty(path) {
 		file, err = os.CreateTemp("", "githooks-installer-*.log")
 		log.AssertNoErrorF(err, "Failed to create installer log at '%v'", path)
+		isDefault = true
 	} else {
 		file, err = os.OpenFile(path, os.O_CREATE|os.O_RDWR|os.O_APPEND, cm.DefaultFileModeFile)
 		log.AssertNoErrorF(err, "Failed to append to installer log at '%v'", path)
 	}
 
 	if err != nil {
-		return ""
+		return
 	}
 
 	log.AddFileWriter(file)
+	resPath = file.Name()
 
-	return file.Name()
+	return
+}
+
+func assertOneInstallerRunning(log cm.ILogContext, interruptCtx *cm.InterruptContext) {
+	lockFile := path.Join(os.TempDir(), "githooks-installer-lock")
+	if exists, _ := cm.IsPathExisting(lockFile); exists {
+		log.PanicF("Only one Githooks installer can run at the same time. "+
+			"Maybe delete the lock file '%v", lockFile)
+	}
+
+	log.DebugF("Created lockfile '%s'.", lockFile)
+	err := cm.TouchFile(lockFile, true)
+	log.AssertNoErrorPanic(err, "Could not create lockfile '%v'.", lockFile)
+
+	// Remove the lock on any exit.
+	deleteLock := func() {
+		log.DebugF("Remove lockfile '%s'.", lockFile)
+		err := os.Remove(lockFile)
+		log.AssertNoError(err, "Lockfile not removed?")
+	}
+	interruptCtx.AddHandler(deleteLock)
 }
 
-func runInstall(cmd *cobra.Command, ctx *ccm.CmdContext, vi *viper.Viper) {
+func runInstall(cmd *cobra.Command, ctx *ccm.CmdContext, vi *viper.Viper) error {
 
 	args := Arguments{}
 	log := ctx.Log
@@ -1317,7 +1343,9 @@ func runInstall(cmd *cobra.Command, ctx *ccm.CmdContext, vi *viper.Viper) {
 	initArgs(log, &args, vi)
 	validateArgs(log, cmd, &args)
 
-	args.Log = addInstallerLog(args.Log, log)
+	isDefaultLog := false
+	isDefaultLog, args.Log = addInstallerLog(args.Log, log)
+
 	log.InfoF("Githooks Installer [version: %s]", build.BuildVersion)
 	dt := time.Now()
 	log.InfoF("Started at: %s", dt.String())
@@ -1329,16 +1357,16 @@ func runInstall(cmd *cobra.Command, ctx *ccm.CmdContext, vi *viper.Viper) {
 			if r := recover(); r != nil {
 				panic(r)
 			}
-			log.RemoveFileWriter()
-			if RemoveInstallerLogOnSuccess && !args.InternalPostDispatch &&
-				logStats.ErrorCount() == 0 {
+
+			if RemoveInstallerLogOnSuccess && logStats.ErrorCount() == 0 &&
+				isDefaultLog && !args.InternalPostDispatch {
+				log.RemoveFileWriter()
 				_ = os.Remove(args.Log)
 			}
 		}()
 	}
 
-	log.InfoF("Logfile: '%s'", args.Log)
-
+	log.InfoF("Log file: '%s'", args.Log)
 	settings, uiSettings := setupSettings(log, ctx.GitX, &args)
 
 	log.DebugF("Arguments: %+v", args)
@@ -1349,11 +1377,16 @@ func runInstall(cmd *cobra.Command, ctx *ccm.CmdContext, vi *viper.Viper) {
 	}
 
 	if !args.InternalPostDispatch {
-		isDispatched, err := prepareDispatch(log, ctx.GitX, &settings, &args, ctx.CleanupX)
+		assertOneInstallerRunning(log, ctx.CleanupX)
+
+		isDispatched, err := runInstallDispatched(log, ctx.GitX, &settings, args, ctx.CleanupX)
 		log.MoveFileWriterToEnd() // We are logging to the same file. Move it to the end.
-		log.AssertNoErrorPanic(err, "Running installer failed.")
+		if err != nil {
+			return ctx.NewCmdExit(1, "%v", err)
+		}
+
 		if isDispatched {
-			return
+			return nil
 		}
 		// intended fallthrough ... (only debug)
 	}
@@ -1367,7 +1400,11 @@ func runInstall(cmd *cobra.Command, ctx *ccm.CmdContext, vi *viper.Viper) {
 			" • %v errors\n"+
 			" • %v warnings\n"+
 			"occurred!", logStats.ErrorCount(), logStats.WarningCount())
+
+		return ctx.NewCmdExit(1, "Install failed.")
 	}
+
+	return nil
 }
 
 func transformLegacyGitConfigSettings(log cm.ILogContext, gitx *git.Context) {

githooks/cmd/uninstaller/uninstaller.go

@@ -113,7 +113,7 @@ func setupSettings(log cm.ILogContext, gitx *git.Context, args *Arguments) (Sett
 		UISettings{PromptCtx: promptx}
 }
 
-func prepareDispatch(log cm.ILogContext, settings *Settings, args *Arguments) bool {
+func runDispatchedInstall(log cm.ILogContext, settings *Settings, args *Arguments) bool {
 
 	uninstaller := hooks.GetUninstallerExecutable(settings.InstallDir)
 	if !cm.IsFile(uninstaller.Cmd) {
@@ -384,7 +384,7 @@ func runUninstall(ctx *ccm.CmdContext, vi *viper.Viper) {
 	settings, uiSettings := setupSettings(log, ctx.GitX, &args)
 
 	if !args.InternalPostDispatch {
-		if isDispatched := prepareDispatch(log, &settings, &args); isDispatched {
+		if isDispatched := runDispatchedInstall(log, &settings, &args); isDispatched {
 			return
 		}
 	}

githooks/common/error-handler.go

@@ -9,7 +9,8 @@ import (
 func HandleCLIErrors(
 	err interface{},
 	log ILogContext,
-	getBugReportingInfo func() string) bool {
+	getBugReportingInfo func() string,
+) bool {
 
 	if err == nil {
 		return false

githooks/common/signals.go

@@ -1,17 +1,26 @@
 package common
 
-// Cleanup context with registered handlers which can be executed in reverse order.
-// Mainly for signal intertupts.
+// Cleanup context with registered handlers which can be executed in reverse order
+// only once.
+// Mainly for signal interrupts.
 type InterruptContext struct {
 	handlers []func()
+
+	isRun bool
 }
 
 func (c *InterruptContext) AddHandler(handler func()) {
 	c.handlers = append(c.handlers, handler)
 }
 
 func (c *InterruptContext) RunHandlers() {
+	if c.isRun {
+		return
+	}
+
 	for i := range c.handlers {
 		c.handlers[len(c.handlers)-i-1]()
 	}
+
+	c.isRun = true
 }

tests/exec-rules.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-if ! grep '/docker/' </proc/self/cgroup >/dev/null 2>&1; then
+if [ "$DOCKER_RUNNING" != "true" ]; then
     echo "! This script is only meant to be run in a Docker container"
     exit 1
 fi

tests/exec-steps.sh

@@ -3,8 +3,8 @@
 if [ "$1" = "--skip-docker-check" ]; then
     shift
 else
-    if ! grep '/docker/' </proc/self/cgroup >/dev/null 2>&1; then
-        echo "! This script is only meant to be run in a Docker container" >&2
+    if [ "$DOCKER_RUNNING" != "true" ]; then
+        echo "! This script is only meant to be run in a Docker container"
         exit 1
     fi
 fi

tests/exec-tests.sh

@@ -13,6 +13,7 @@ fi
 cat <<EOF | docker build --force-rm -t "githooks:$IMAGE_TYPE" -f - "$ROOT_DIR" || exit 1
 FROM githooks:$IMAGE_TYPE-base
 
+ENV DOCKER_RUNNING=true
 ENV GH_TESTS="/var/lib/githooks-tests"
 ENV GH_TEST_TMP="/tmp"
 ENV GH_TEST_REPO="/var/lib/githooks"

tests/exec-testsuite.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-if ! grep '/docker/' </proc/self/cgroup >/dev/null 2>&1; then
+if [ "$DOCKER_RUNNING" != "true" ]; then
     echo "! This script is only meant to be run in a Docker container"
     exit 1
 fi

tests/test-alpine-nolfs.sh

@@ -6,6 +6,9 @@ cat <<EOF | docker build --force-rm -t githooks:alpine-nolfs-base -
 FROM golang:1.17-alpine
 RUN apk add git --update-cache --repository http://dl-3.alpinelinux.org/alpine/edge/main --allow-untrusted
 RUN apk add bash jq curl
+
+# CVE https://github.blog/2022-10-18-git-security-vulnerabilities-announced/#cve-2022-39253
+RUN git config --system protocol.file.allow always
 EOF
 
 exec "$TEST_DIR/exec-tests.sh" 'alpine-nolfs' "$@"