fix: exit non-zero when CLI flag/env parsing fails

cmd/cloudflared/generic_service.go

@@ -4,7 +4,6 @@ package main
 
 import (
 	"fmt"
-	"os"
 
 	cli "github.com/urfave/cli/v2"
 
@@ -28,7 +27,7 @@ func runApp(app *cli.App, graceShutdownC chan struct{}) {
 			},
 		},
 	})
-	app.Run(os.Args)
+	runAppAndExit(app)
 }
 
 func installGenericService(c *cli.Context) error {

cmd/cloudflared/linux_service.go

@@ -51,7 +51,7 @@ out if no configuration file with credentials was found).`,
 			},
 		},
 	})
-	_ = app.Run(os.Args)
+	runAppAndExit(app)
 }
 
 // The directory and files that are used by the service.

cmd/cloudflared/macos_service.go

@@ -50,7 +50,7 @@ causing it to look for credentials in a configuration file upon startup.`,
 			},
 		},
 	})
-	_ = app.Run(os.Args)
+	runAppAndExit(app)
 }
 
 func newLaunchdTemplate(installPath, stdoutPath, stderrPath string) *ServiceTemplate {

cmd/cloudflared/main_test.go

@@ -0,0 +1,40 @@
+package main
+
+import (
+	"os"
+	"os/exec"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestMain doubles as the helper-process entry point: when
+// GO_WANT_HELPER_PROCESS is set, the test binary re-executes main() so tests
+// can assert on the process exit code, which plain unit tests cannot.
+func TestMain(m *testing.M) {
+	if os.Getenv("GO_WANT_HELPER_PROCESS") == "1" {
+		main()
+		os.Exit(0)
+	}
+	os.Exit(m.Run())
+}
+
+// Regression test for https://github.com/cloudflare/cloudflared/issues/1606:
+// an invalid TUNNEL_GRACE_PERIOD value used to make cloudflared exit 0 with no
+// output, because runApp discarded the error from app.Run. It must now print
+// the parse error and exit non-zero.
+func TestRunAppInvalidGracePeriodEnvExitsNonZero(t *testing.T) {
+	//nolint:gosec // G204: re-executes this test binary (os.Args[0]) with a fixed command line to assert the exit code
+	cmd := exec.Command(os.Args[0], "tunnel", "run")
+	cmd.Env = append(os.Environ(),
+		"GO_WANT_HELPER_PROCESS=1",
+		"TUNNEL_GRACE_PERIOD=10",
+	)
+	out, err := cmd.CombinedOutput()
+
+	var exitErr *exec.ExitError
+	require.ErrorAs(t, err, &exitErr, "expected a non-zero exit code, got: %s", out)
+	assert.NotZero(t, exitErr.ExitCode())
+	assert.Contains(t, string(out), "grace-period")
+}

cmd/cloudflared/run_app.go

@@ -0,0 +1,20 @@
+package main
+
+import (
+	"fmt"
+	"os"
+
+	"github.com/urfave/cli/v2"
+)
+
+// runAppAndExit runs the CLI app and exits with a non-zero status when it
+// fails. Flag and env-var parse errors (e.g. an invalid TUNNEL_GRACE_PERIOD)
+// are returned by app.Run without being printed, so ignoring the error used
+// to make cloudflared exit 0 with no output (see #1606). Print the error to
+// stderr and exit 1 so scripts and service managers can detect the failure.
+func runAppAndExit(app *cli.App) {
+	if err := app.Run(os.Args); err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
+	}
+}

cmd/cloudflared/windows_service.go

@@ -94,7 +94,7 @@ causing it to look for credentials in a configuration file upon startup.`,
 		log.Fatal().Err(err).Msg("failed to determine if we are running in an interactive session")
 	}
 	if isIntSess {
-		app.Run(os.Args)
+		runAppAndExit(app)
 		return
 	}
 
@@ -106,7 +106,7 @@ causing it to look for credentials in a configuration file upon startup.`,
 		if errno, ok := err.(syscall.Errno); ok && int(errno) == serviceControllerConnectionFailure {
 			// Hack: assume this is a false negative from the IsAnInteractiveSession() check above.
 			// Run the app in "interactive" mode anyway.
-			app.Run(os.Args)
+			runAppAndExit(app)
 			return
 		}
 		log.Fatal().Err(err).Msgf("%s service failed", windowsServiceName)