launcher: log on Windows options parsing, better handle path overrides

cmd/launcher/desktop.go

@@ -29,6 +29,10 @@ import (
 	"github.com/peterbourgon/ff/v3"
 )
 
+// runDesktop executes the userland, unprivileged launcher in the background
+// which surfaces notifications and provides a tray icon. It bypasses launcher
+// proper's option parsing and its arguments are set explicitly by the parent
+// process.
 func runDesktop(_ *multislogger.MultiSlogger, args []string) error {
 	var (
 		flagset = flag.NewFlagSet("kolide desktop", flag.ExitOnError)

cmd/launcher/doctor.go

@@ -21,23 +21,24 @@ func runDoctor(systemMultiSlogger *multislogger.MultiSlogger, args []string) err
 	launcher.DefaultAutoupdate = true
 	launcher.SetDefaultPaths()
 
-	opts, err := launcher.ParseOptions("doctor", os.Args[2:])
+	slogLevel := new(slog.LevelVar)
+	slogLevel.Set(slog.LevelInfo)
+	// Add handler to write to stdout
+	systemMultiSlogger.AddHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{
+		Level:     slogLevel,
+		AddSource: true,
+	}))
+
+	opts, err := launcher.ParseOptions(systemMultiSlogger.Logger, "doctor", os.Args[2:])
 	if err != nil {
 		return err
 	}
 
-	fcOpts := []flags.Option{flags.WithCmdLineOpts(opts)}
-
-	slogLevel := slog.LevelInfo
 	if opts.Debug {
-		slogLevel = slog.LevelDebug
+		slogLevel.Set(slog.LevelDebug)
 	}
 
-	// Add handler to write to stdout
-	systemMultiSlogger.AddHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{
-		Level:     slogLevel,
-		AddSource: true,
-	}))
+	fcOpts := []flags.Option{flags.WithCmdLineOpts(opts)}
 
 	flagController := flags.NewFlagController(systemMultiSlogger.Logger, nil, fcOpts...)
 	k := knapsack.New(nil, flagController, nil, nil, nil)

cmd/launcher/enroll.go

@@ -34,7 +34,7 @@ func runEnroll(systemMultiSlogger *multislogger.MultiSlogger, args []string) err
 	if err := ff.Parse(flagset, args); err != nil {
 		return fmt.Errorf("parsing flags: %w", err)
 	}
-	opts, err := launcher.ParseOptions("enroll", []string{"-config", *flConfigFilePath})
+	opts, err := launcher.ParseOptions(systemMultiSlogger.Logger, "enroll", []string{"-config", *flConfigFilePath})
 	if err != nil {
 		return fmt.Errorf("parsing options for subcommand enroll: %w", err)
 	}

cmd/launcher/flare.go

@@ -38,28 +38,29 @@ func runFlare(systemMultiSlogger *multislogger.MultiSlogger, args []string) erro
 		flOutputDir        = flagset.String("output_dir", ".", "path to directory to save flare output")
 		flUploadRequestURL = flagset.String("upload_request_url", "https://api.kolide.com/api/agent/flare", "URL to request a signed upload URL")
 		flConfigFilePath   = flagset.String("config", launcher.DefaultConfigFilePath, "config file to parse options from (optional)")
+		slogLevel          = new(slog.LevelVar)
 	)
+	slogLevel.Set(slog.LevelInfo)
+
+	// Add handler to write to stdout
+	systemMultiSlogger.AddHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{
+		Level:     slogLevel,
+		AddSource: true,
+	}))
 
 	if err := ff.Parse(flagset, args); err != nil {
 		return fmt.Errorf("parsing flags: %w", err)
 	}
 
-	opts, err := launcher.ParseOptions("flare", []string{"-config", *flConfigFilePath})
+	opts, err := launcher.ParseOptions(systemMultiSlogger.Logger, "flare", []string{"-config", *flConfigFilePath})
 	if err != nil {
 		return err
 	}
 
-	slogLevel := slog.LevelInfo
 	if opts.Debug {
-		slogLevel = slog.LevelDebug
+		slogLevel.Set(slog.LevelDebug)
 	}
 
-	// Add handler to write to stdout
-	systemMultiSlogger.AddHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{
-		Level:     slogLevel,
-		AddSource: true,
-	}))
-
 	fcOpts := []flags.Option{flags.WithCmdLineOpts(opts)}
 	flagController := flags.NewFlagController(systemMultiSlogger.Logger, inmemory.NewStore(), fcOpts...)
 

cmd/launcher/interactive.go

@@ -20,7 +20,7 @@ import (
 )
 
 func runInteractive(systemMultiSlogger *multislogger.MultiSlogger, args []string) error {
-	opts, err := launcher.ParseOptions("interactive", args)
+	opts, err := launcher.ParseOptions(systemMultiSlogger.Logger, "interactive", args)
 	if err != nil {
 		return err
 	}

cmd/launcher/launcher_test.go

@@ -60,7 +60,7 @@ func Test_runLauncher(t *testing.T) {
 			})
 			downloadOnceFunc() // get an osquery binary
 			require.NoError(t, osqueryBinaryDownloadErr, "could not download osquery, cannot proceed with tests")
-			defaultOpts, err := launcher.ParseOptions("launcher", []string{
+			defaultOpts, err := launcher.ParseOptions(multislogger.NewNopLogger(), "launcher", []string{
 				"--root_directory", testRootDir,
 				"--osqueryd_path", testOsqueryBinary,
 			})

cmd/launcher/main.go

@@ -83,7 +83,7 @@ func runMain() int {
 
 	// create initial logger. As this is prior to options parsing,
 	// use the environment to determine verbosity.  It will be
-	// re-leveled during options parsing.
+	// re-leveled after options parsing.
 	logger := logutil.NewServerLogger(env.Bool("LAUNCHER_DEBUG", false))
 	ctx = ctxlog.NewContext(ctx, logger)
 
@@ -134,8 +134,9 @@ func runMain() int {
 		}
 	}
 
-	// if the launcher is being ran with a positional argument,
-	// handle that argument.
+	// if the launcher is being run with a positional argument, we
+	// bypass launcher proper's option parsing and let the subcommand
+	// figure it out.
 	if len(os.Args) > 1 && !strings.HasPrefix(os.Args[1], `-`) {
 		if err := runSubcommands(systemSlogger); err != nil {
 			systemSlogger.Log(ctx, slog.LevelError,
@@ -147,8 +148,8 @@ func runMain() int {
 		return 0
 	}
 
-	// Fall back to running launcher
-	opts, err := launcher.ParseOptions("", os.Args[1:])
+	// Default to running launcher proper with its extensive set of options.
+	opts, err := launcher.ParseOptions(systemSlogger.Logger, "", os.Args[1:])
 	if err != nil {
 		if launcher.IsInfoCmd(err) {
 			return 0
@@ -160,7 +161,7 @@ func runMain() int {
 		return 0
 	}
 
-	// recreate the logger with  the appropriate level.
+	// recreate the logger with the appropriate level.
 	logger = logutil.NewServerLogger(opts.Debug)
 
 	// set up slogger for internal launcher logging

cmd/launcher/run_compactdb.go

@@ -39,20 +39,20 @@ func runCompactDb(systemMultiSlogger *multislogger.MultiSlogger, args []string)
 		launcherOptions = append(launcherOptions, "-config", *flConfigFilePath)
 	}
 
-	opts, err := launcher.ParseOptions("compactdb", launcherOptions)
+	// Add handler to write to stdout
+	systemMultiSlogger.AddHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{
+		Level:     slog.LevelDebug,
+		AddSource: true,
+	}))
+
+	opts, err := launcher.ParseOptions(systemMultiSlogger.Logger, "compactdb", launcherOptions)
 	if err != nil {
 		return fmt.Errorf("parsing launcher options: %w", err)
 	}
 	if opts.RootDirectory == "" {
 		return errors.New("no root directory specified")
 	}
 
-	// Add handler to write to stdout
-	systemMultiSlogger.AddHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{
-		Level:     slog.LevelDebug,
-		AddSource: true,
-	}))
-
 	boltPath := filepath.Join(opts.RootDirectory, "launcher.db")
 	if *flDbFileName != "" {
 		// Check to make sure this is a launcher.db or launcher.db.bak file

cmd/launcher/svc_windows.go

@@ -46,7 +46,7 @@ func runWindowsSvc(systemSlogger *multislogger.MultiSlogger, args []string) erro
 		"version", version.Version().Version,
 	)
 
-	opts, err := launcher.ParseOptions("", os.Args[2:])
+	opts, err := launcher.ParseOptions(systemSlogger.Logger, "", os.Args[2:])
 	if err != nil {
 		systemSlogger.Log(context.TODO(), slog.LevelInfo,
 			"error parsing options",
@@ -141,7 +141,7 @@ func runWindowsSvcForeground(systemSlogger *multislogger.MultiSlogger, args []st
 	localSlogger.AddHandler(handler)
 	systemSlogger.AddHandler(handler)
 
-	opts, err := launcher.ParseOptions("", os.Args[2:])
+	opts, err := launcher.ParseOptions(systemSlogger.Logger, "", os.Args[2:])
 	if err != nil {
 		level.Info(logger).Log("err", err)
 		return fmt.Errorf("parsing options: %w", err)

cmd/launcher/uninstall.go

@@ -37,7 +37,7 @@ func runUninstall(_ *multislogger.MultiSlogger, args []string) error {
 		launcherOptions = append(launcherOptions, "-config", *flConfigFilePath)
 	}
 
-	opts, err := launcher.ParseOptions("uninstall", launcherOptions)
+	opts, err := launcher.ParseOptions(multislogger.NewNopLogger(), "uninstall", launcherOptions)
 	if err != nil {
 		return fmt.Errorf("parsing launcher options: %w", err)
 	}

ee/debug/checkups/launcher_flags.go

@@ -50,7 +50,7 @@ func (lf *launcherFlags) Run(_ context.Context, extraFh io.Writer) error {
 		return nil
 	}
 
-	if _, err := launcher.ParseOptions("", []string{fmt.Sprintf("--config=%s", configFilePath)}); err != nil {
+	if _, err := launcher.ParseOptions(lf.k.Slogger(), "", []string{fmt.Sprintf("--config=%s", configFilePath)}); err != nil {
 		lf.summary = fmt.Sprintf("failed to parse flags: %s", err)
 		return nil
 	}

ee/desktop/runner/runner.go

@@ -1,4 +1,5 @@
-// runner handles multiuser process management for launcher desktop
+// runner spawns child processes to run the unprivileged launcher for all users
+// logged in.
 package runner
 
 import (
@@ -1159,8 +1160,10 @@ func (r *DesktopUsersProcessesRunner) writeLocalizationFile() error {
 	return nil
 }
 
-// desktopCommand invokes the launcher desktop executable with the appropriate env vars
+// desktopCommand invokes the launcher desktop executable with configuration
+// to interface with this process over a socket and URL
 func (r *DesktopUsersProcessesRunner) desktopCommand(uid, socketPath, menuPath string) (*allowedcmd.TracedCmd, error) {
+	// desktop subcommand runs the more limited desktop launcher
 	cmd, err := allowedcmd.Launcher.Cmd(context.TODO(), "desktop")
 	if err != nil {
 		return nil, fmt.Errorf("creating launcher desktop command: %w", err)
@@ -1199,7 +1202,7 @@ func (r *DesktopUsersProcessesRunner) desktopCommand(uid, socketPath, menuPath s
 		// Set GOMAXPROCS for the desktop subprocess (Go respects this natively).
 		// This overrides the GOMAXPROCS value set by allowedcmd.
 		fmt.Sprintf("GOMAXPROCS=%d", r.knapsack.DesktopGoMaxProcs()),
-		"LAUNCHER_SKIP_UPDATES=true", // We already know that we want to run the version of launcher in `executablePath`, so there's no need to perform lookups
+		"LAUNCHER_SKIP_UPDATES=true", // run the _exact_ binary that we run and dodge cross-version incompatibility
 	}
 
 	stdErr, err := cmd.StderrPipe()

ee/tuf/library_lookup.go

@@ -49,7 +49,7 @@ func CheckOutLatestWithoutConfig(binary autoupdatableBinary, slogger *slog.Logge
 	}
 
 	// check for old root directories before returning final config in case we've stomped over with windows MSI install
-	updatedRootDirectory := launcher.DetermineRootDirectoryOverride(cfg.rootDirectory, cfg.hostname, cfg.identifier)
+	updatedRootDirectory := launcher.DetermineRootDirectoryOverride(slogger, cfg.rootDirectory, cfg.hostname, cfg.identifier)
 	if updatedRootDirectory != cfg.rootDirectory {
 		slogger.Log(context.TODO(), slog.LevelInfo,
 			"old root directory contents detected, overriding for autoupdate config",

ee/watchdog/watchdog_task_windows.go

@@ -35,7 +35,7 @@ func RunWatchdogTask(systemSlogger *multislogger.MultiSlogger, args []string) er
 	ff.Parse(flagset, args)
 
 	// pass the config file through our standard options parsing to get all default options
-	opts, err := launcher.ParseOptions("watchdog", []string{"-config", *flConfigFilePath})
+	opts, err := launcher.ParseOptions(systemSlogger.Logger, "watchdog", []string{"-config", *flConfigFilePath})
 	if err != nil {
 		return fmt.Errorf("parsing watchdog options: %w", err)
 	}

pkg/launcher/options.go

@@ -1,10 +1,12 @@
 package launcher
 
 import (
+	"context"
 	"encoding/hex"
 	"errors"
 	"flag"
 	"fmt"
+	"log/slog"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -14,6 +16,7 @@ import (
 	"time"
 
 	"github.com/kolide/kit/version"
+	"github.com/kolide/launcher/v2/pkg/log/multislogger"
 	"github.com/peterbourgon/ff/v3"
 )
 
@@ -182,10 +185,14 @@ func (i *ArrayFlags) Set(value string) error {
 	return nil
 }
 
-// ParseOptions parses the options that may be configured via command-line flags
-// and/or environment variables, determines order of precedence and returns a
-// typed struct of options for further application use
-func ParseOptions(subcommandName string, args []string) (*Options, error) {
+// ParseOptions handles flags, environment variable overrides, and
+// their order of precedence for launcher. These generally apply
+// to the proper launcher runtime that communicates with a
+// controlserver and runs osquery. A parsed set of options is
+// returned.
+//
+// Launcher subcommands may opt-in to these flags, but most do not.
+func ParseOptions(logger *slog.Logger, subcommandName string, args []string) (*Options, error) {
 	flagsetName := "launcher"
 	if subcommandName != "" {
 		flagsetName = fmt.Sprintf("launcher %s", subcommandName)
@@ -196,6 +203,9 @@ func ParseOptions(subcommandName string, args []string) (*Options, error) {
 	} else {
 		flagset.Usage = commandUsage(flagset, flagsetName)
 	}
+	if logger == nil {
+		logger = multislogger.NewNopLogger()
+	}
 
 	var (
 		// Primary options
@@ -312,6 +322,10 @@ func ParseOptions(subcommandName string, args []string) (*Options, error) {
 
 	// On windows, we should make sure osquerydPath ends in .exe
 	if runtime.GOOS == "windows" && !strings.HasSuffix(osquerydPath, ".exe") {
+		logger.Log(context.TODO(), slog.LevelInfo,
+			"appending missing file extension to osqueryd path",
+			"original_path", osquerydPath,
+		)
 		osquerydPath = osquerydPath + ".exe"
 	}
 
@@ -378,10 +392,7 @@ func ParseOptions(subcommandName string, args []string) (*Options, error) {
 
 	if runtime.GOOS == "windows" {
 		// check for old root directories before returning the configured option in case we've stomped over with windows MSI install
-		updatedRootDirectory := DetermineRootDirectoryOverride(*flRootDirectory, *flKolideServerURL, *flPackageIdentifier)
-		if updatedRootDirectory != *flRootDirectory {
-			*flRootDirectory = updatedRootDirectory
-		}
+		*flRootDirectory = DetermineRootDirectoryOverride(logger, *flRootDirectory, *flKolideServerURL, *flPackageIdentifier)
 	}
 
 	opts := &Options{

pkg/launcher/options_test.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/kolide/kit/stringutil"
+	"github.com/kolide/launcher/v2/pkg/log/multislogger"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/goleak"
@@ -90,7 +91,7 @@ func TestOptionsFromFlags(t *testing.T) { //nolint:paralleltest
 		}
 	}
 
-	opts, err := ParseOptions("", testFlags)
+	opts, err := ParseOptions(multislogger.NewNopLogger(), "", testFlags)
 	require.NoError(t, err)
 	require.Equal(t, expectedOpts, opts)
 }
@@ -112,7 +113,7 @@ func TestOptionsFromEnv(t *testing.T) { //nolint:paralleltest
 		name := fmt.Sprintf("KOLIDE_LAUNCHER_%s", strings.ToUpper(strings.TrimLeft(k, "-")))
 		t.Setenv(name, val)
 	}
-	opts, err := ParseOptions("", []string{})
+	opts, err := ParseOptions(multislogger.NewNopLogger(), "", []string{})
 	require.NoError(t, err)
 	require.Equal(t, expectedOpts, opts)
 }
@@ -143,7 +144,7 @@ func TestOptionsFromFile(t *testing.T) { // nolint:paralleltest
 
 	require.NoError(t, flagFile.Close())
 
-	opts, err := ParseOptions("", []string{"-config", flagFile.Name()})
+	opts, err := ParseOptions(multislogger.NewNopLogger(), "", []string{"-config", flagFile.Name()})
 	require.NoError(t, err)
 	require.Equal(t, expectedOpts, opts)
 }
@@ -178,7 +179,7 @@ func TestAutoupdateDownloadSPlayCanBeDisabledFromFlagsFile(t *testing.T) { //nol
 
 	require.NoError(t, flagFile.Close())
 
-	opts, err := ParseOptions("", []string{"-config", flagFile.Name()})
+	opts, err := ParseOptions(multislogger.NewNopLogger(), "", []string{"-config", flagFile.Name()})
 	require.NoError(t, err)
 	require.Equal(t, expectedOpts, opts)
 }
@@ -215,7 +216,7 @@ func TestOsqueryLogPublishFlags(t *testing.T) { //nolint:paralleltest
 
 	require.NoError(t, flagFile.Close())
 
-	opts, err := ParseOptions("", []string{"-config", flagFile.Name()})
+	opts, err := ParseOptions(multislogger.NewNopLogger(), "", []string{"-config", flagFile.Name()})
 	require.NoError(t, err)
 	require.Equal(t, expectedOpts, opts)
 }
@@ -313,7 +314,7 @@ func TestOptionsSetControlServerHost(t *testing.T) { // nolint:paralleltest
 	for _, tt := range testCases { // nolint:paralleltest
 		os.Clearenv()
 		t.Run(tt.testName, func(t *testing.T) {
-			opts, err := ParseOptions("", tt.testFlags)
+			opts, err := ParseOptions(multislogger.NewNopLogger(), "", tt.testFlags)
 			require.NoError(t, err, "could not parse options")
 			require.Equal(t, tt.expectedControlServer, opts.ControlServerURL, "incorrect control server")
 			require.Equal(t, tt.expectedInsecureControlTLS, opts.InsecureControlTLS, "incorrect insecure TLS")