From b63440db7a0f5a34ba4655dd9a814a02736f9b1d Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Wed, 10 Apr 2024 09:59:01 -0400 Subject: [PATCH 1/8] Move update channel into launcher options --- ee/agent/flags/flag_controller.go | 3 +- pkg/autoupdate/autoupdate.go | 24 --------------- pkg/autoupdate/autoupdate_test.go | 50 ------------------------------- pkg/launcher/options.go | 36 ++++++++++++++++++---- pkg/launcher/options_test.go | 44 +++++++++++++++++++++++++++ 5 files changed, 75 insertions(+), 82 deletions(-) delete mode 100644 pkg/autoupdate/autoupdate_test.go diff --git a/ee/agent/flags/flag_controller.go b/ee/agent/flags/flag_controller.go index 96776209f..151e2d2f3 100644 --- a/ee/agent/flags/flag_controller.go +++ b/ee/agent/flags/flag_controller.go @@ -11,7 +11,6 @@ import ( "github.com/kolide/launcher/ee/agent/flags/keys" "github.com/kolide/launcher/ee/agent/types" "github.com/kolide/launcher/ee/tuf" - "github.com/kolide/launcher/pkg/autoupdate" "github.com/kolide/launcher/pkg/launcher" "golang.org/x/exp/maps" ) @@ -462,7 +461,7 @@ func (fc *FlagController) SetUpdateChannel(channel string) error { } func (fc *FlagController) UpdateChannel() string { return NewStringFlagValue( - WithSanitizer(autoupdate.SanitizeUpdateChannel), + WithSanitizer(launcher.SanitizeUpdateChannel), WithDefaultString(string(fc.cmdLineOpts.UpdateChannel)), ).get(fc.getControlServerValue(keys.UpdateChannel)) } diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go index d7e17b846..e4699de22 100644 --- a/pkg/autoupdate/autoupdate.go +++ b/pkg/autoupdate/autoupdate.go @@ -1,30 +1,6 @@ // Package autoupdate has largely been superseded by ee/tuf. package autoupdate -// UpdateChannel determines the TUF target for a Updater. -// The Default UpdateChannel is Stable. -type UpdateChannel string - -const ( - Stable UpdateChannel = "stable" - Alpha UpdateChannel = "alpha" - Beta UpdateChannel = "beta" - Nightly UpdateChannel = "nightly" -) - -func (c UpdateChannel) String() string { - return string(c) -} - -func SanitizeUpdateChannel(value string) string { - switch UpdateChannel(value) { - case Stable, Alpha, Beta, Nightly: - return value - } - // Fallback to stable if invalid channel - return Stable.String() -} - const ( DefaultMirror = "https://dl.kolide.co" ) diff --git a/pkg/autoupdate/autoupdate_test.go b/pkg/autoupdate/autoupdate_test.go deleted file mode 100644 index 8ce45ebbf..000000000 --- a/pkg/autoupdate/autoupdate_test.go +++ /dev/null @@ -1,50 +0,0 @@ -package autoupdate - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestSanitizeUpdateChannel(t *testing.T) { - t.Parallel() - var tests = []struct { - name string - channel string - expectedChannel string - }{ - { - name: "default", - expectedChannel: Stable.String(), - }, - { - name: "alpha", - channel: Alpha.String(), - expectedChannel: Alpha.String(), - }, - { - name: "beta", - channel: Beta.String(), - expectedChannel: Beta.String(), - }, - { - name: "nightly", - channel: Nightly.String(), - expectedChannel: Nightly.String(), - }, - { - name: "invalid", - channel: "not-a-real-channel", - expectedChannel: Stable.String(), - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - assert.Equal(t, tt.expectedChannel, SanitizeUpdateChannel(tt.channel)) - }) - } -} diff --git a/pkg/launcher/options.go b/pkg/launcher/options.go index bf218d3ec..659f78e3f 100644 --- a/pkg/launcher/options.go +++ b/pkg/launcher/options.go @@ -82,7 +82,7 @@ type Options struct { // updates. AutoupdateInterval time.Duration // UpdateChannel is the channel to pull options from (stable, beta, nightly). - UpdateChannel autoupdate.UpdateChannel + UpdateChannel UpdateChannel // AutoupdateInitialDelay set an initial startup delay on the autoupdater process. AutoupdateInitialDelay time.Duration // UpdateDirectory is the location of the update libraries for osqueryd and launcher @@ -319,16 +319,16 @@ func ParseOptions(subcommandName string, args []string) (*Options, error) { return nil, errors.New("both enroll_secret and enroll_secret_path were defined") } - var updateChannel autoupdate.UpdateChannel + var updateChannel UpdateChannel switch *flUpdateChannel { case "", "stable": - updateChannel = autoupdate.Stable + updateChannel = Stable case "beta": - updateChannel = autoupdate.Beta + updateChannel = Beta case "alpha": - updateChannel = autoupdate.Alpha + updateChannel = Alpha case "nightly": - updateChannel = autoupdate.Nightly + updateChannel = Nightly default: return nil, fmt.Errorf("unknown update channel %s", *flUpdateChannel) } @@ -582,6 +582,30 @@ func FindOsquery() string { return "" } +// UpdateChannel determines the TUF target for a Updater. +// The Default UpdateChannel is Stable. +type UpdateChannel string + +const ( + Stable UpdateChannel = "stable" + Alpha UpdateChannel = "alpha" + Beta UpdateChannel = "beta" + Nightly UpdateChannel = "nightly" +) + +func (c UpdateChannel) String() string { + return string(c) +} + +func SanitizeUpdateChannel(value string) string { + switch UpdateChannel(value) { + case Stable, Alpha, Beta, Nightly: + return value + } + // Fallback to stable if invalid channel + return Stable.String() +} + func commandUsage(fs *flag.FlagSet, short string) func() { return func() { fmt.Fprintf(os.Stderr, " Usage:\n") diff --git a/pkg/launcher/options_test.go b/pkg/launcher/options_test.go index 88d0641c7..25fbc4222 100644 --- a/pkg/launcher/options_test.go +++ b/pkg/launcher/options_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/kolide/kit/stringutil" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -269,6 +270,49 @@ func getArgsAndResponse() (map[string]string, *Options) { return args, opts } +func TestSanitizeUpdateChannel(t *testing.T) { + t.Parallel() + var tests = []struct { + name string + channel string + expectedChannel string + }{ + { + name: "default", + expectedChannel: Stable.String(), + }, + { + name: "alpha", + channel: Alpha.String(), + expectedChannel: Alpha.String(), + }, + { + name: "beta", + channel: Beta.String(), + expectedChannel: Beta.String(), + }, + { + name: "nightly", + channel: Nightly.String(), + expectedChannel: Nightly.String(), + }, + { + name: "invalid", + channel: "not-a-real-channel", + expectedChannel: Stable.String(), + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tt.expectedChannel, SanitizeUpdateChannel(tt.channel)) + }) + } +} + // windowsAddExe appends ".exe" to the input string when running on Windows func windowsAddExe(in string) string { if runtime.GOOS == "windows" { From 63baa0e1930ae64c20474198a01ed38c219e0029 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Wed, 10 Apr 2024 10:00:08 -0400 Subject: [PATCH 2/8] Move DefaultMirror into options --- pkg/autoupdate/autoupdate.go | 6 ------ pkg/launcher/options.go | 5 ++--- 2 files changed, 2 insertions(+), 9 deletions(-) delete mode 100644 pkg/autoupdate/autoupdate.go diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go deleted file mode 100644 index e4699de22..000000000 --- a/pkg/autoupdate/autoupdate.go +++ /dev/null @@ -1,6 +0,0 @@ -// Package autoupdate has largely been superseded by ee/tuf. -package autoupdate - -const ( - DefaultMirror = "https://dl.kolide.co" -) diff --git a/pkg/launcher/options.go b/pkg/launcher/options.go index 659f78e3f..c929c573f 100644 --- a/pkg/launcher/options.go +++ b/pkg/launcher/options.go @@ -13,8 +13,6 @@ import ( "text/tabwriter" "time" - "github.com/kolide/launcher/pkg/autoupdate" - "github.com/kolide/kit/version" "github.com/peterbourgon/ff/v3" ) @@ -170,6 +168,7 @@ const ( defaultRootDirectory = "launcher-root" skipEnvParse = runtime.GOOS == "windows" // skip environmental variable parsing on windows DefaultTufServer = "https://tuf.kolide.com" + DefaultMirror = "https://dl.kolide.co" ) // Adapted from @@ -240,7 +239,7 @@ func ParseOptions(subcommandName string, args []string) (*Options, error) { // Autoupdate options flAutoupdate = flagset.Bool("autoupdate", DefaultAutoupdate, "Whether or not the osquery autoupdater is enabled (default: false)") flTufServerURL = flagset.String("tuf_url", DefaultTufServer, "TUF update server (default: https://tuf.kolide.com)") - flMirrorURL = flagset.String("mirror_url", autoupdate.DefaultMirror, "The mirror server for autoupdates (default: https://dl.kolide.co)") + flMirrorURL = flagset.String("mirror_url", DefaultMirror, "The mirror server for autoupdates (default: https://dl.kolide.co)") flAutoupdateInterval = flagset.Duration("autoupdate_interval", 1*time.Hour, "The interval to check for updates (default: once every hour)") flUpdateChannel = flagset.String("update_channel", "stable", "The channel to pull updates from (options: stable, beta, nightly)") flAutoupdateInitialDelay = flagset.Duration("autoupdater_initial_delay", 1*time.Hour, "Initial autoupdater subprocess delay") From fd94ab0779dacc14e1a3c0db67bdc80d1a3f3fea Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Wed, 10 Apr 2024 10:12:12 -0400 Subject: [PATCH 3/8] Move CheckExecutable into ee/tuf --- ee/tuf/library_lookup.go | 3 +- ee/tuf/library_manager.go | 7 +- ee/tuf/library_manager_test.go | 7 +- ee/tuf/util.go | 81 +++++++++ ee/tuf/util_darwin.go | 24 +++ ee/tuf/util_linux.go | 25 +++ ee/tuf/util_test.go | 275 ++++++++++++++++++++++++++++++ ee/tuf/util_windows.go | 28 +++ pkg/autoupdate/findnew.go | 76 +-------- pkg/autoupdate/findnew_test.go | 68 -------- pkg/autoupdate/helpers.go | 32 ---- pkg/autoupdate/helpers_test.go | 54 ------ pkg/autoupdate/helpers_windows.go | 36 ---- 13 files changed, 443 insertions(+), 273 deletions(-) create mode 100644 ee/tuf/util.go delete mode 100644 pkg/autoupdate/helpers.go delete mode 100644 pkg/autoupdate/helpers_test.go delete mode 100644 pkg/autoupdate/helpers_windows.go diff --git a/ee/tuf/library_lookup.go b/ee/tuf/library_lookup.go index 14e87d617..098d8ac2a 100644 --- a/ee/tuf/library_lookup.go +++ b/ee/tuf/library_lookup.go @@ -11,7 +11,6 @@ import ( "github.com/kolide/launcher/ee/agent/flags/keys" "github.com/kolide/launcher/ee/agent/startupsettings" - "github.com/kolide/launcher/pkg/autoupdate" "github.com/kolide/launcher/pkg/launcher" "github.com/kolide/launcher/pkg/traces" "github.com/peterbourgon/ff/v3" @@ -238,7 +237,7 @@ func findExecutable(ctx context.Context, binary autoupdatableBinary, tufReposito } targetPath, targetVersion := pathToTargetVersionExecutable(binary, targetName, baseUpdateDirectory) - if err := autoupdate.CheckExecutable(ctx, targetPath, "--version"); err != nil { + if err := CheckExecutable(ctx, targetPath, "--version"); err != nil { traces.SetError(span, err) return nil, fmt.Errorf("version %s from target %s at %s is either originally installed version, not yet downloaded, or corrupted: %w", targetVersion, targetName, targetPath, err) } diff --git a/ee/tuf/library_manager.go b/ee/tuf/library_manager.go index 3c5b94ff3..985b08f03 100644 --- a/ee/tuf/library_manager.go +++ b/ee/tuf/library_manager.go @@ -17,7 +17,6 @@ import ( "github.com/Masterminds/semver" "github.com/kolide/kit/fsutil" - "github.com/kolide/launcher/pkg/autoupdate" "github.com/kolide/launcher/pkg/backoff" "github.com/kolide/launcher/pkg/traces" "github.com/theupdateframework/go-tuf/data" @@ -68,7 +67,7 @@ func updatesDirectory(binary autoupdatableBinary, baseUpdateDirectory string) st // Available determines if the given target is already available in the update library. func (ulm *updateLibraryManager) Available(binary autoupdatableBinary, targetFilename string) bool { executablePath, _ := pathToTargetVersionExecutable(binary, targetFilename, ulm.baseDir) - return autoupdate.CheckExecutable(context.TODO(), executablePath, "--version") == nil + return CheckExecutable(context.TODO(), executablePath, "--version") == nil } // pathToTargetVersionExecutable returns the path to the executable for the desired target, @@ -223,7 +222,7 @@ func (ulm *updateLibraryManager) moveVerifiedUpdate(binary autoupdatableBinary, // Validate the executable -- the executable check will occasionally time out, especially on Windows, // and we aren't in a rush here, so we retry a couple times. if err := backoff.WaitFor(func() error { - return autoupdate.CheckExecutable(context.TODO(), executableLocation(stagedVersionedDirectory, binary), "--version") + return CheckExecutable(context.TODO(), executableLocation(stagedVersionedDirectory, binary), "--version") }, 45*time.Second, 15*time.Second); err != nil { return fmt.Errorf("could not verify executable after retries: %w", err) } @@ -345,7 +344,7 @@ func sortedVersionsInLibrary(ctx context.Context, binary autoupdatableBinary, ba } versionDir := filepath.Join(updatesDirectory(binary, baseUpdateDirectory), rawVersion) - if err := autoupdate.CheckExecutable(ctx, executableLocation(versionDir, binary), "--version"); err != nil { + if err := CheckExecutable(ctx, executableLocation(versionDir, binary), "--version"); err != nil { traces.SetError(span, err) invalidVersions = append(invalidVersions, rawVersion) continue diff --git a/ee/tuf/library_manager_test.go b/ee/tuf/library_manager_test.go index 6ae45633c..7143b7319 100644 --- a/ee/tuf/library_manager_test.go +++ b/ee/tuf/library_manager_test.go @@ -13,7 +13,6 @@ import ( "testing" tufci "github.com/kolide/launcher/ee/tuf/ci" - "github.com/kolide/launcher/pkg/autoupdate" "github.com/kolide/launcher/pkg/log/multislogger" "github.com/stretchr/testify/require" "github.com/theupdateframework/go-tuf/data" @@ -208,7 +207,7 @@ func TestAddToLibrary_alreadyAdded(t *testing.T) { require.NoError(t, os.Chmod(executablePath, 0755)) _, err := os.Stat(executablePath) require.NoError(t, err, "did not create binary for test") - require.NoError(t, autoupdate.CheckExecutable(context.TODO(), executablePath, "--version"), "binary created for test is corrupt") + require.NoError(t, CheckExecutable(context.TODO(), executablePath, "--version"), "binary created for test is corrupt") // Ask the library manager to perform the download targetFilename := fmt.Sprintf("%s-%s.tar.gz", binary, testVersion) @@ -538,7 +537,7 @@ func Test_sortedVersionsInLibrary(t *testing.T) { require.NoError(t, os.Chmod(executablePath, 0755)) _, err := os.Stat(executablePath) require.NoError(t, err, "did not create binary for test") - require.NoError(t, autoupdate.CheckExecutable(context.TODO(), executablePath, "--version"), "binary created for test is corrupt") + require.NoError(t, CheckExecutable(context.TODO(), executablePath, "--version"), "binary created for test is corrupt") } // Get sorted versions @@ -578,7 +577,7 @@ func Test_sortedVersionsInLibrary_devBuilds(t *testing.T) { require.NoError(t, os.Chmod(executablePath, 0755)) _, err := os.Stat(executablePath) require.NoError(t, err, "did not create binary for test") - require.NoError(t, autoupdate.CheckExecutable(context.TODO(), executablePath, "--version"), "binary created for test is corrupt") + require.NoError(t, CheckExecutable(context.TODO(), executablePath, "--version"), "binary created for test is corrupt") } // Get sorted versions diff --git a/ee/tuf/util.go b/ee/tuf/util.go new file mode 100644 index 000000000..9c51a4ee0 --- /dev/null +++ b/ee/tuf/util.go @@ -0,0 +1,81 @@ +package tuf + +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "syscall" + "time" + + "github.com/kolide/launcher/pkg/traces" +) + +// CheckExecutable tests whether something is an executable. It +// examines permissions, mode, and tries to exec it directly. +func CheckExecutable(ctx context.Context, potentialBinary string, args ...string) error { + ctx, span := traces.StartSpan(ctx) + defer span.End() + + if err := checkExecutablePermissions(potentialBinary); err != nil { + return err + } + + // If we can determine that the requested executable is + // ourself, don't try to exec. It's needless, and a potential + // fork bomb. Ignore errors, either we get an answer or we don't. + selfPath, _ := os.Executable() + if filepath.Clean(selfPath) == filepath.Clean(potentialBinary) { + return nil + } + + // If we get ETXTBSY error when execing, this could be because this + // binary is freshly downloaded. Retry a small number of times only + // in that circumstance. + // See: https://github.com/golang/go/issues/22315 + for i := 0; i < 3; i += 1 { + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, potentialBinary, args...) //nolint:forbidigo // We trust the autoupdate library to find the correct location so we don't need allowedcmd + + // Set env, this should prevent launcher for fork-bombing + cmd.Env = append(cmd.Env, "LAUNCHER_SKIP_UPDATES=TRUE") + + out, execErr := cmd.CombinedOutput() + if execErr != nil && errors.Is(execErr, syscall.ETXTBSY) { + continue + } + + if ctx.Err() != nil { + return fmt.Errorf("timeout when checking executable: %w", ctx.Err()) + } + + return supressRoutineErrors(execErr, out) + } + + return fmt.Errorf("could not exec %s despite retries due to text file busy", potentialBinary) +} + +// supressRoutineErrors attempts to tell whether the error was a +// program that has executed, and then exited, vs one that's execution +// was entirely unsuccessful. This differentiation allows us to +// detect, and recover, from corrupt updates vs something in-app. +func supressRoutineErrors(err error, combinedOutput []byte) error { + if err == nil { + return nil + } + + // Suppress exit codes of 1 or 2. These are generally indicative of + // an unknown command line flag, _not_ a corrupt download. (exit + // code 0 will be nil, and never trigger this block) + if exitError, ok := err.(*exec.ExitError); ok { + if exitError.ExitCode() == 1 || exitError.ExitCode() == 2 { + // suppress these + return nil + } + } + return fmt.Errorf("exec error: output: `%s`, err: %w", string(combinedOutput), err) +} diff --git a/ee/tuf/util_darwin.go b/ee/tuf/util_darwin.go index 5bd34c49c..3cd2b52e1 100644 --- a/ee/tuf/util_darwin.go +++ b/ee/tuf/util_darwin.go @@ -4,6 +4,8 @@ package tuf import ( + "errors" + "fmt" "os" "path/filepath" ) @@ -27,3 +29,25 @@ func executableLocation(updateDirectory string, binary autoupdatableBinary) stri return "" } } + +// checkExecutablePermissions checks whether a specific file looks +// like it's executable. This is used in evaluating whether something +// is an updated version. +func checkExecutablePermissions(potentialBinary string) error { + if potentialBinary == "" { + return errors.New("empty string isn't executable") + } + stat, err := os.Stat(potentialBinary) + switch { + case os.IsNotExist(err): + return errors.New("no such file") + case err != nil: + return fmt.Errorf("statting file: %w", err) + case stat.IsDir(): + return errors.New("is a directory") + case stat.Mode()&0111 == 0: + return errors.New("not executable") + } + + return nil +} diff --git a/ee/tuf/util_linux.go b/ee/tuf/util_linux.go index 7f4a3f112..9d7520266 100644 --- a/ee/tuf/util_linux.go +++ b/ee/tuf/util_linux.go @@ -4,6 +4,9 @@ package tuf import ( + "errors" + "fmt" + "os" "path/filepath" ) @@ -11,3 +14,25 @@ import ( func executableLocation(updateDirectory string, binary autoupdatableBinary) string { return filepath.Join(updateDirectory, string(binary)) } + +// checkExecutablePermissions checks whether a specific file looks +// like it's executable. This is used in evaluating whether something +// is an updated version. +func checkExecutablePermissions(potentialBinary string) error { + if potentialBinary == "" { + return errors.New("empty string isn't executable") + } + stat, err := os.Stat(potentialBinary) + switch { + case os.IsNotExist(err): + return errors.New("no such file") + case err != nil: + return fmt.Errorf("statting file: %w", err) + case stat.IsDir(): + return errors.New("is a directory") + case stat.Mode()&0111 == 0: + return errors.New("not executable") + } + + return nil +} diff --git a/ee/tuf/util_test.go b/ee/tuf/util_test.go index 05cfa0d9e..98527aa19 100644 --- a/ee/tuf/util_test.go +++ b/ee/tuf/util_test.go @@ -1,10 +1,14 @@ package tuf import ( + "context" + "fmt" + "io" "os" "path/filepath" "runtime" "testing" + "time" "github.com/stretchr/testify/require" ) @@ -57,3 +61,274 @@ func Test_executableLocation_nonAppBundle(t *testing.T) { osquerydLocation := executableLocation(updateDir, "osqueryd") require.Equal(t, expectedOsquerydLocation, osquerydLocation) } + +func TestCheckExecutable(t *testing.T) { + t.Parallel() + + // We need to run this from a temp dir, else the early return + // from matching os.Executable bypasses the point of this. + tmpDir, binaryName := setupTestDir(t, executableUpdates) + targetExe := filepath.Join(tmpDir, binaryName) + + var tests = []struct { + testName string + expectedErr bool + }{ + { + testName: "exit0", + expectedErr: false, + }, + { + testName: "exit1", + expectedErr: false, + }, + { + testName: "exit2", + expectedErr: false, + }, + { + testName: "sleep", + expectedErr: true, + }, + } + + for _, tt := range tests { // nolint:paralleltest + tt := tt + t.Run(tt.testName, func(t *testing.T) { + err := CheckExecutable(context.TODO(), targetExe, "-test.run=TestHelperProcess", "--", tt.testName) + if tt.expectedErr { + require.Error(t, err, tt.testName) + + // As a bonus, we can test that if we call os.Args[0], + // we still don't get an error. This is because we + // trigger the match against os.Executable and don't + // invoked. This is here, and not a dedicated test, + // because we ensure the same test arguments. + require.NoError(t, CheckExecutable(context.TODO(), os.Args[0], "-test.run=TestHelperProcess", "--", tt.testName), "calling self with %s", tt.testName) + } else { + require.NoError(t, err, tt.testName) + } + }) + + } +} + +func TestCheckExecutableTruncated(t *testing.T) { + t.Parallel() + + // First make a broken truncated binary. Lots of setup for this. + truncatedBinary, err := os.CreateTemp("", "test-autoupdate-check-executable-truncation") + require.NoError(t, err, "make temp file") + defer os.Remove(truncatedBinary.Name()) + truncatedBinary.Close() + + require.NoError(t, copyFile(truncatedBinary.Name(), os.Args[0], true), "copy executable") + require.NoError(t, os.Chmod(truncatedBinary.Name(), 0755)) + + require.Error(t, + CheckExecutable(context.TODO(), truncatedBinary.Name(), "-test.run=TestHelperProcess", "--", "exit0"), + "truncated binary") +} + +// copyFile copies a file from srcPath to dstPath. If truncate is set, +// only half the file is copied. (This is a trivial wrapper to +// simplify setting up test cases) +func copyFile(dstPath, srcPath string, truncate bool) error { + src, err := os.Open(srcPath) + if err != nil { + return err + } + defer src.Close() + + dst, err := os.Create(dstPath) + if err != nil { + return err + } + defer dst.Close() + + if !truncate { + if _, err := io.Copy(dst, src); err != nil { + return err + } + } else { + stat, err := src.Stat() + if err != nil { + return fmt.Errorf("statting srcFile: %w", err) + } + + if _, err = io.CopyN(dst, src, stat.Size()/2); err != nil { + return fmt.Errorf("copying srcFile: %w", err) + } + } + + return nil +} + +type setupState int + +const ( + emptySetup setupState = iota + emptyUpdateDirs + nonExecutableUpdates + executableUpdates + truncatedUpdates +) + +// This suffix is added to the binary path to find the updates +const updateDirSuffix = "-updates" + +// setupTestDir function to setup the test dirs. This work is broken +// up in stages, allowing test functions to tap into various +// points. This is setup this way to allow simpler isolation on test +// failures. +func setupTestDir(t *testing.T, stage setupState) (string, string) { + tmpDir := t.TempDir() + + // Create a test binary + binaryName := windowsAddExe("binary") + binaryPath := filepath.Join(tmpDir, binaryName) + updatesDir := fmt.Sprintf("%s%s", binaryPath, updateDirSuffix) + + require.NoError(t, copyFile(binaryPath, os.Args[0], false), "copy executable") + require.NoError(t, os.Chmod(binaryPath, 0755), "chmod") + + if stage <= emptySetup { + return tmpDir, binaryName + } + + // make some update directories + // (these are out of order, to jumble up the create times) + for _, n := range []string{"2", "5", "3", "1"} { + require.NoError(t, os.MkdirAll(filepath.Join(updatesDir, n), 0755)) + if runtime.GOOS == "darwin" { + require.NoError(t, os.MkdirAll(filepath.Join(updatesDir, n, "Test.app", "Contents", "MacOS"), 0755)) + } + } + + if stage <= emptyUpdateDirs { + return tmpDir, binaryName + } + + // Copy executable to update directories + for _, n := range []string{"2", "5", "3", "1"} { + updatedBinaryPath := filepath.Join(updatesDir, n, binaryName) + require.NoError(t, copyFile(updatedBinaryPath, binaryPath, false), "copy executable") + if runtime.GOOS == "darwin" { + updatedAppBundleBinaryPath := filepath.Join(updatesDir, n, "Test.app", "Contents", "MacOS", filepath.Base(binaryPath)) + require.NoError(t, copyFile(updatedAppBundleBinaryPath, binaryPath, false), "copy executable") + } + } + + if stage <= nonExecutableUpdates { + return tmpDir, binaryName + } + + // Make our top-level binaries executable + for _, n := range []string{"2", "5", "3", "1"} { + require.NoError(t, os.Chmod(filepath.Join(updatesDir, n, binaryName), 0755)) + if runtime.GOOS == "darwin" { + require.NoError(t, os.Chmod(filepath.Join(updatesDir, n, "Test.app", "Contents", "MacOS", binaryName), 0755)) + } + } + + if stage <= executableUpdates { + return tmpDir, binaryName + } + + for _, n := range []string{"5", "1"} { + updatedBinaryPath := filepath.Join(updatesDir, n, binaryName) + require.NoError(t, copyFile(updatedBinaryPath, binaryPath, true), "copy & truncate executable") + if runtime.GOOS == "darwin" { + require.NoError(t, copyFile(filepath.Join(updatesDir, n, "Test.app", "Contents", "MacOS", binaryName), binaryPath, true), "copy & truncate executable") + } + } + + return tmpDir, binaryName +} + +// TestHelperProcess isn't a real test. It's used as a helper process +// to make a portableish binary. See +// https://github.com/golang/go/blob/master/src/os/exec/exec_test.go#L724 +// and https://npf.io/2015/06/testing-exec-command/ +func TestHelperProcess(t *testing.T) { + t.Parallel() + + // find out magic arguments + args := os.Args + for len(args) > 0 { + if args[0] == "--" { + args = args[1:] + break + } + args = args[1:] + } + if len(args) == 0 { + // Indicates an error, or just being run in the test suite. + return + } + + switch args[0] { + case "sleep": + time.Sleep(10 * time.Second) + case "exit0": + os.Exit(0) + case "exit1": + os.Exit(1) + case "exit2": + os.Exit(2) + } + + // default behavior nothing +} + +func windowsAddExe(in string) string { + if runtime.GOOS == "windows" { + return in + ".exe" + } + + return in +} + +func Test_checkExecutablePermissions(t *testing.T) { + t.Parallel() + + require.Error(t, checkExecutablePermissions(""), "passing empty string") + require.Error(t, checkExecutablePermissions("/random/path/should/not/exist"), "passing non-existent file path") + + // Setup the tests + tmpDir := t.TempDir() + + require.Error(t, checkExecutablePermissions(tmpDir), "directory should not be executable") + + dotExe := "" + if runtime.GOOS == "windows" { + dotExe = ".exe" + } + + fileName := filepath.Join(tmpDir, "file") + dotExe + tmpFile, err := os.Create(fileName) + require.NoError(t, err, "os create") + tmpFile.Close() + + hardLink := filepath.Join(tmpDir, "hardlink") + dotExe + require.NoError(t, os.Link(fileName, hardLink), "making link") + + symLink := filepath.Join(tmpDir, "symlink") + dotExe + require.NoError(t, os.Symlink(fileName, symLink), "making symlink") + + // windows doesn't have an executable bit + if runtime.GOOS == "windows" { + require.NoError(t, checkExecutablePermissions(fileName), "plain file") + require.NoError(t, checkExecutablePermissions(hardLink), "hard link") + require.NoError(t, checkExecutablePermissions(symLink), "symlink") + } else { + require.Error(t, checkExecutablePermissions(fileName), "plain file") + require.Error(t, checkExecutablePermissions(hardLink), "hard link") + require.Error(t, checkExecutablePermissions(symLink), "symlink") + + require.NoError(t, os.Chmod(fileName, 0755)) + require.NoError(t, checkExecutablePermissions(fileName), "plain file") + require.NoError(t, checkExecutablePermissions(hardLink), "hard link") + require.NoError(t, checkExecutablePermissions(symLink), "symlink") + } +} diff --git a/ee/tuf/util_windows.go b/ee/tuf/util_windows.go index 7eea5f089..015cb9e4d 100644 --- a/ee/tuf/util_windows.go +++ b/ee/tuf/util_windows.go @@ -4,11 +4,39 @@ package tuf import ( + "errors" "fmt" + "os" "path/filepath" + "strings" ) // executableLocation returns the path to the executable in `updateDirectory`. func executableLocation(updateDirectory string, binary autoupdatableBinary) string { return filepath.Join(updateDirectory, fmt.Sprintf("%s.exe", binary)) } + +// checkExecutablePermissions checks whether a specific file looks +// like it's executable. This is used in evaluating whether something +// is an updated version. +// +// Windows does not have executable bits, so we omit those. And +// instead check the file extension. +func checkExecutablePermissions(potentialBinary string) error { + if potentialBinary == "" { + return errors.New("empty string isn't executable") + } + stat, err := os.Stat(potentialBinary) + switch { + case os.IsNotExist(err): + return errors.New("no such file") + case err != nil: + return fmt.Errorf("statting file: %w", err) + case stat.IsDir(): + return errors.New("is a directory") + case !strings.HasSuffix(potentialBinary, ".exe"): + return errors.New("not executable") + } + + return nil +} diff --git a/pkg/autoupdate/findnew.go b/pkg/autoupdate/findnew.go index 896f16f4a..412f4a538 100644 --- a/pkg/autoupdate/findnew.go +++ b/pkg/autoupdate/findnew.go @@ -5,18 +5,15 @@ import ( "errors" "fmt" "os" - "os/exec" "path/filepath" "runtime" "sort" "strings" - "syscall" - "time" "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" + "github.com/kolide/launcher/ee/tuf" "github.com/kolide/launcher/pkg/contexts/ctxlog" - "github.com/kolide/launcher/pkg/traces" ) // defaultBuildTimestamp is used to set the _oldest_ allowed update. Eg, if @@ -198,7 +195,7 @@ func FindNewest(ctx context.Context, fullBinaryPath string, opts ...newestOption // check that executions work. If the exec fails, // there's clearly an issue and we should remove it. if newestSettings.runningExecutable != foundExecutable { - if err := CheckExecutable(ctx, foundExecutable, "--version"); err != nil { + if err := tuf.CheckExecutable(ctx, foundExecutable, "--version"); err != nil { if newestSettings.deleteCorrupt { level.Error(logger).Log("msg", "not executable. Removing", "binary", foundExecutable, "reason", err) if err := os.RemoveAll(basedir); err != nil { @@ -234,7 +231,7 @@ func FindNewest(ctx context.Context, fullBinaryPath string, opts ...newestOption return fullBinaryPath } - if err := CheckExecutable(ctx, fullBinaryPath, "--version"); err != nil { + if err := tuf.CheckExecutable(ctx, fullBinaryPath, "--version"); err != nil { level.Debug(logger).Log("msg", "fullBinaryPath not executable. Returning nil", "err", err) return "" } @@ -350,70 +347,3 @@ func findBaseDir(path string) string { components := strings.SplitN(path, updateDirSuffix, 2) return filepath.Dir(components[0]) } - -// CheckExecutable tests whether something is an executable. It -// examines permissions, mode, and tries to exec it directly. -func CheckExecutable(ctx context.Context, potentialBinary string, args ...string) error { - ctx, span := traces.StartSpan(ctx) - defer span.End() - - if err := checkExecutablePermissions(potentialBinary); err != nil { - return err - } - - // If we can determine that the requested executable is - // ourself, don't try to exec. It's needless, and a potential - // fork bomb. Ignore errors, either we get an answer or we don't. - selfPath, _ := os.Executable() - if filepath.Clean(selfPath) == filepath.Clean(potentialBinary) { - return nil - } - - // If we get ETXTBSY error when execing, this could be because this - // binary is freshly downloaded. Retry a small number of times only - // in that circumstance. - // See: https://github.com/golang/go/issues/22315 - for i := 0; i < 3; i += 1 { - ctx, cancel := context.WithTimeout(ctx, 5*time.Second) - defer cancel() - - cmd := exec.CommandContext(ctx, potentialBinary, args...) //nolint:forbidigo // We trust the autoupdate library to find the correct location so we don't need allowedcmd - - // Set env, this should prevent launcher for fork-bombing - cmd.Env = append(cmd.Env, "LAUNCHER_SKIP_UPDATES=TRUE") - - out, execErr := cmd.CombinedOutput() - if execErr != nil && errors.Is(execErr, syscall.ETXTBSY) { - continue - } - - if ctx.Err() != nil { - return fmt.Errorf("timeout when checking executable: %w", ctx.Err()) - } - - return supressRoutineErrors(execErr, out) - } - - return fmt.Errorf("could not exec %s despite retries due to text file busy", potentialBinary) -} - -// supressRoutineErrors attempts to tell whether the error was a -// program that has executed, and then exited, vs one that's execution -// was entirely unsuccessful. This differentiation allows us to -// detect, and recover, from corrupt updates vs something in-app. -func supressRoutineErrors(err error, combinedOutput []byte) error { - if err == nil { - return nil - } - - // Suppress exit codes of 1 or 2. These are generally indicative of - // an unknown command line flag, _not_ a corrupt download. (exit - // code 0 will be nil, and never trigger this block) - if exitError, ok := err.(*exec.ExitError); ok { - if exitError.ExitCode() == 1 || exitError.ExitCode() == 2 { - // suppress these - return nil - } - } - return fmt.Errorf("exec error: output: `%s`, err: %w", string(combinedOutput), err) -} diff --git a/pkg/autoupdate/findnew_test.go b/pkg/autoupdate/findnew_test.go index e9797ff19..f59081a0d 100644 --- a/pkg/autoupdate/findnew_test.go +++ b/pkg/autoupdate/findnew_test.go @@ -365,74 +365,6 @@ func copyFile(dstPath, srcPath string, truncate bool) error { return nil } -func TestCheckExecutable(t *testing.T) { - t.Parallel() - - // We need to run this from a temp dir, else the early return - // from matching os.Executable bypasses the point of this. - tmpDir, binaryName := setupTestDir(t, executableUpdates) - targetExe := filepath.Join(tmpDir, binaryName) - - var tests = []struct { - testName string - expectedErr bool - }{ - { - testName: "exit0", - expectedErr: false, - }, - { - testName: "exit1", - expectedErr: false, - }, - { - testName: "exit2", - expectedErr: false, - }, - { - testName: "sleep", - expectedErr: true, - }, - } - - for _, tt := range tests { // nolint:paralleltest - tt := tt - t.Run(tt.testName, func(t *testing.T) { - err := CheckExecutable(context.TODO(), targetExe, "-test.run=TestHelperProcess", "--", tt.testName) - if tt.expectedErr { - require.Error(t, err, tt.testName) - - // As a bonus, we can test that if we call os.Args[0], - // we still don't get an error. This is because we - // trigger the match against os.Executable and don't - // invoked. This is here, and not a dedicated test, - // because we ensure the same test arguments. - require.NoError(t, CheckExecutable(context.TODO(), os.Args[0], "-test.run=TestHelperProcess", "--", tt.testName), "calling self with %s", tt.testName) - } else { - require.NoError(t, err, tt.testName) - } - }) - - } -} - -func TestCheckExecutableTruncated(t *testing.T) { - t.Parallel() - - // First make a broken truncated binary. Lots of setup for this. - truncatedBinary, err := os.CreateTemp("", "test-autoupdate-check-executable-truncation") - require.NoError(t, err, "make temp file") - defer os.Remove(truncatedBinary.Name()) - truncatedBinary.Close() - - require.NoError(t, copyFile(truncatedBinary.Name(), os.Args[0], true), "copy executable") - require.NoError(t, os.Chmod(truncatedBinary.Name(), 0755)) - - require.Error(t, - CheckExecutable(context.TODO(), truncatedBinary.Name(), "-test.run=TestHelperProcess", "--", "exit0"), - "truncated binary") -} - func TestBuildTimestamp(t *testing.T) { t.Parallel() diff --git a/pkg/autoupdate/helpers.go b/pkg/autoupdate/helpers.go deleted file mode 100644 index dbddd10cb..000000000 --- a/pkg/autoupdate/helpers.go +++ /dev/null @@ -1,32 +0,0 @@ -//go:build !windows -// +build !windows - -package autoupdate - -import ( - "errors" - "fmt" - "os" -) - -// checkExecutablePermissions checks wehether a specific file looks -// like it's executable. This is used in evaluating whether something -// is an updated version. -func checkExecutablePermissions(potentialBinary string) error { - if potentialBinary == "" { - return errors.New("empty string isn't executable") - } - stat, err := os.Stat(potentialBinary) - switch { - case os.IsNotExist(err): - return errors.New("no such file") - case err != nil: - return fmt.Errorf("statting file: %w", err) - case stat.IsDir(): - return errors.New("is a directory") - case stat.Mode()&0111 == 0: - return errors.New("not executable") - } - - return nil -} diff --git a/pkg/autoupdate/helpers_test.go b/pkg/autoupdate/helpers_test.go deleted file mode 100644 index 27a405e63..000000000 --- a/pkg/autoupdate/helpers_test.go +++ /dev/null @@ -1,54 +0,0 @@ -package autoupdate - -import ( - "os" - "path/filepath" - "runtime" - "testing" - - "github.com/stretchr/testify/require" -) - -func TestCheckExecutablePermissions(t *testing.T) { - t.Parallel() - - require.Error(t, checkExecutablePermissions(""), "passing empty string") - require.Error(t, checkExecutablePermissions("/random/path/should/not/exist"), "passing non-existent file path") - - // Setup the tests - tmpDir := t.TempDir() - - require.Error(t, checkExecutablePermissions(tmpDir), "directory should not be executable") - - dotExe := "" - if runtime.GOOS == "windows" { - dotExe = ".exe" - } - - fileName := filepath.Join(tmpDir, "file") + dotExe - tmpFile, err := os.Create(fileName) - require.NoError(t, err, "os create") - tmpFile.Close() - - hardLink := filepath.Join(tmpDir, "hardlink") + dotExe - require.NoError(t, os.Link(fileName, hardLink), "making link") - - symLink := filepath.Join(tmpDir, "symlink") + dotExe - require.NoError(t, os.Symlink(fileName, symLink), "making symlink") - - // windows doesn't have an executable bit - if runtime.GOOS == "windows" { - require.NoError(t, checkExecutablePermissions(fileName), "plain file") - require.NoError(t, checkExecutablePermissions(hardLink), "hard link") - require.NoError(t, checkExecutablePermissions(symLink), "symlink") - } else { - require.Error(t, checkExecutablePermissions(fileName), "plain file") - require.Error(t, checkExecutablePermissions(hardLink), "hard link") - require.Error(t, checkExecutablePermissions(symLink), "symlink") - - require.NoError(t, os.Chmod(fileName, 0755)) - require.NoError(t, checkExecutablePermissions(fileName), "plain file") - require.NoError(t, checkExecutablePermissions(hardLink), "hard link") - require.NoError(t, checkExecutablePermissions(symLink), "symlink") - } -} diff --git a/pkg/autoupdate/helpers_windows.go b/pkg/autoupdate/helpers_windows.go deleted file mode 100644 index 052dbc555..000000000 --- a/pkg/autoupdate/helpers_windows.go +++ /dev/null @@ -1,36 +0,0 @@ -//go:build windows -// +build windows - -package autoupdate - -import ( - "errors" - "fmt" - "os" - "strings" -) - -// checkExecutablePermissions checks wehether a specific file looks -// like it's executable. This is used in evaluating whether something -// is an updated version. -// -// Windows does not have executable bits, so we omit those. And -// instead check the file extension. -func checkExecutablePermissions(potentialBinary string) error { - if potentialBinary == "" { - return errors.New("empty string isn't executable") - } - stat, err := os.Stat(potentialBinary) - switch { - case os.IsNotExist(err): - return errors.New("no such file") - case err != nil: - return fmt.Errorf("statting file: %w", err) - case stat.IsDir(): - return errors.New("is a directory") - case !strings.HasSuffix(potentialBinary, ".exe"): - return errors.New("not executable") - } - - return nil -} From da84fd59c5bbdaab72d4158b943f8a0b822f4bdc Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Wed, 10 Apr 2024 10:12:27 -0400 Subject: [PATCH 4/8] Lint --- ee/tuf/finalize_other.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/tuf/finalize_other.go b/ee/tuf/finalize_other.go index b645ffbfe..b328ee675 100644 --- a/ee/tuf/finalize_other.go +++ b/ee/tuf/finalize_other.go @@ -3,6 +3,6 @@ package tuf -func patchExecutable(executableLocation string) error { +func patchExecutable(_ string) error { return nil } From 9a695ca6b22d74d0e037a20c91312b0067729ba5 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Wed, 10 Apr 2024 10:24:47 -0400 Subject: [PATCH 5/8] Remove and simplify test setup --- ee/tuf/util_test.go | 124 +++++--------------------------------------- 1 file changed, 14 insertions(+), 110 deletions(-) diff --git a/ee/tuf/util_test.go b/ee/tuf/util_test.go index 98527aa19..6251a43c4 100644 --- a/ee/tuf/util_test.go +++ b/ee/tuf/util_test.go @@ -2,7 +2,6 @@ package tuf import ( "context" - "fmt" "io" "os" "path/filepath" @@ -10,6 +9,7 @@ import ( "testing" "time" + tufci "github.com/kolide/launcher/ee/tuf/ci" "github.com/stretchr/testify/require" ) @@ -67,8 +67,11 @@ func TestCheckExecutable(t *testing.T) { // We need to run this from a temp dir, else the early return // from matching os.Executable bypasses the point of this. - tmpDir, binaryName := setupTestDir(t, executableUpdates) + tmpDir := t.TempDir() + binaryName := windowsAddExe("testbinary") targetExe := filepath.Join(tmpDir, binaryName) + tufci.CopyBinary(t, targetExe) + require.NoError(t, os.Chmod(targetExe, 0755)) var tests = []struct { testName string @@ -122,7 +125,7 @@ func TestCheckExecutableTruncated(t *testing.T) { defer os.Remove(truncatedBinary.Name()) truncatedBinary.Close() - require.NoError(t, copyFile(truncatedBinary.Name(), os.Args[0], true), "copy executable") + copyFileTruncated(t, truncatedBinary.Name(), os.Args[0]) require.NoError(t, os.Chmod(truncatedBinary.Name(), 0755)) require.Error(t, @@ -130,120 +133,21 @@ func TestCheckExecutableTruncated(t *testing.T) { "truncated binary") } -// copyFile copies a file from srcPath to dstPath. If truncate is set, -// only half the file is copied. (This is a trivial wrapper to -// simplify setting up test cases) -func copyFile(dstPath, srcPath string, truncate bool) error { +// copyFile copies half of the file from srcPath to dstPath. +func copyFileTruncated(t *testing.T, dstPath, srcPath string) { src, err := os.Open(srcPath) - if err != nil { - return err - } + require.NoError(t, err, "opening src") defer src.Close() dst, err := os.Create(dstPath) - if err != nil { - return err - } + require.NoError(t, err, "opening dest") defer dst.Close() - if !truncate { - if _, err := io.Copy(dst, src); err != nil { - return err - } - } else { - stat, err := src.Stat() - if err != nil { - return fmt.Errorf("statting srcFile: %w", err) - } - - if _, err = io.CopyN(dst, src, stat.Size()/2); err != nil { - return fmt.Errorf("copying srcFile: %w", err) - } - } - - return nil -} - -type setupState int - -const ( - emptySetup setupState = iota - emptyUpdateDirs - nonExecutableUpdates - executableUpdates - truncatedUpdates -) - -// This suffix is added to the binary path to find the updates -const updateDirSuffix = "-updates" - -// setupTestDir function to setup the test dirs. This work is broken -// up in stages, allowing test functions to tap into various -// points. This is setup this way to allow simpler isolation on test -// failures. -func setupTestDir(t *testing.T, stage setupState) (string, string) { - tmpDir := t.TempDir() - - // Create a test binary - binaryName := windowsAddExe("binary") - binaryPath := filepath.Join(tmpDir, binaryName) - updatesDir := fmt.Sprintf("%s%s", binaryPath, updateDirSuffix) - - require.NoError(t, copyFile(binaryPath, os.Args[0], false), "copy executable") - require.NoError(t, os.Chmod(binaryPath, 0755), "chmod") - - if stage <= emptySetup { - return tmpDir, binaryName - } - - // make some update directories - // (these are out of order, to jumble up the create times) - for _, n := range []string{"2", "5", "3", "1"} { - require.NoError(t, os.MkdirAll(filepath.Join(updatesDir, n), 0755)) - if runtime.GOOS == "darwin" { - require.NoError(t, os.MkdirAll(filepath.Join(updatesDir, n, "Test.app", "Contents", "MacOS"), 0755)) - } - } - - if stage <= emptyUpdateDirs { - return tmpDir, binaryName - } - - // Copy executable to update directories - for _, n := range []string{"2", "5", "3", "1"} { - updatedBinaryPath := filepath.Join(updatesDir, n, binaryName) - require.NoError(t, copyFile(updatedBinaryPath, binaryPath, false), "copy executable") - if runtime.GOOS == "darwin" { - updatedAppBundleBinaryPath := filepath.Join(updatesDir, n, "Test.app", "Contents", "MacOS", filepath.Base(binaryPath)) - require.NoError(t, copyFile(updatedAppBundleBinaryPath, binaryPath, false), "copy executable") - } - } - - if stage <= nonExecutableUpdates { - return tmpDir, binaryName - } - - // Make our top-level binaries executable - for _, n := range []string{"2", "5", "3", "1"} { - require.NoError(t, os.Chmod(filepath.Join(updatesDir, n, binaryName), 0755)) - if runtime.GOOS == "darwin" { - require.NoError(t, os.Chmod(filepath.Join(updatesDir, n, "Test.app", "Contents", "MacOS", binaryName), 0755)) - } - } - - if stage <= executableUpdates { - return tmpDir, binaryName - } - - for _, n := range []string{"5", "1"} { - updatedBinaryPath := filepath.Join(updatesDir, n, binaryName) - require.NoError(t, copyFile(updatedBinaryPath, binaryPath, true), "copy & truncate executable") - if runtime.GOOS == "darwin" { - require.NoError(t, copyFile(filepath.Join(updatesDir, n, "Test.app", "Contents", "MacOS", binaryName), binaryPath, true), "copy & truncate executable") - } - } + stat, err := src.Stat() + require.NoError(t, err, "statting src") - return tmpDir, binaryName + _, err = io.CopyN(dst, src, stat.Size()/2) + require.NoError(t, err, "copying src to dest") } // TestHelperProcess isn't a real test. It's used as a helper process From a692fced458f0e45b8e7746de2d574e8e1858925 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Wed, 10 Apr 2024 12:30:42 -0400 Subject: [PATCH 6/8] Give test more time --- ee/tuf/autoupdate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/tuf/autoupdate_test.go b/ee/tuf/autoupdate_test.go index c93ca0ba8..5850144fd 100644 --- a/ee/tuf/autoupdate_test.go +++ b/ee/tuf/autoupdate_test.go @@ -217,7 +217,7 @@ func TestExecute_osquerydUpdate(t *testing.T) { // Let the autoupdater run for a bit go autoupdater.Execute() - time.Sleep(500 * time.Millisecond) + time.Sleep(1000 * time.Millisecond) // Assert expectation that we added the expected `testReleaseVersion` to the updates library mockLibraryManager.AssertExpectations(t) From af28a91f63f662aac5c6921aa51f62c7f1996626 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Wed, 10 Apr 2024 14:57:05 -0400 Subject: [PATCH 7/8] Remove FindNewest --- cmd/launcher/interactive.go | 9 +- cmd/launcher/main.go | 10 +- cmd/launcher/svc_windows.go | 15 -- ee/agent/knapsack/knapsack.go | 3 +- pkg/autoupdate/findnew.go | 349 ------------------------ pkg/autoupdate/findnew_test.go | 479 --------------------------------- pkg/make/builder.go | 4 - pkg/osquery/runtime/runner.go | 12 +- 8 files changed, 6 insertions(+), 875 deletions(-) delete mode 100644 pkg/autoupdate/findnew.go delete mode 100644 pkg/autoupdate/findnew_test.go diff --git a/cmd/launcher/interactive.go b/cmd/launcher/interactive.go index 337dd2a08..c0d1f9794 100644 --- a/cmd/launcher/interactive.go +++ b/cmd/launcher/interactive.go @@ -2,7 +2,6 @@ package main import ( "context" - "errors" "flag" "fmt" "log/slog" @@ -12,7 +11,6 @@ import ( "github.com/kolide/launcher/cmd/launcher/internal" "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/tuf" - "github.com/kolide/launcher/pkg/autoupdate" "github.com/kolide/launcher/pkg/launcher" "github.com/kolide/launcher/pkg/log/multislogger" "github.com/kolide/launcher/pkg/osquery/interactive" @@ -48,12 +46,7 @@ func runInteractive(systemMultiSlogger *multislogger.MultiSlogger, args []string if osquerydPath == "" { latestOsquerydBinary, err := tuf.CheckOutLatestWithoutConfig("osqueryd", systemMultiSlogger.Logger) if err != nil { - osquerydPath = launcher.FindOsquery() - if osquerydPath == "" { - return errors.New("could not find osqueryd binary") - } - // Fall back to old autoupdate library - osquerydPath = autoupdate.FindNewest(context.Background(), osquerydPath) + return fmt.Errorf("finding osqueryd binary: %w", err) } else { osquerydPath = latestOsquerydBinary.Path } diff --git a/cmd/launcher/main.go b/cmd/launcher/main.go index 179c8e822..6216bb2c4 100644 --- a/cmd/launcher/main.go +++ b/cmd/launcher/main.go @@ -16,7 +16,6 @@ import ( "github.com/kolide/kit/logutil" "github.com/kolide/kit/version" "github.com/kolide/launcher/ee/tuf" - "github.com/kolide/launcher/pkg/autoupdate" "github.com/kolide/launcher/pkg/contexts/ctxlog" "github.com/kolide/launcher/pkg/execwrapper" "github.com/kolide/launcher/pkg/launcher" @@ -178,15 +177,10 @@ func runNewerLauncherIfAvailable(ctx context.Context, slogger *slog.Logger) { newerBinary, err := latestLauncherPath(ctx, slogger) if err != nil { slogger.Log(ctx, slog.LevelError, - "could not check out latest launcher, will fall back to old autoupdate library", + "could not check out latest launcher", "err", err, ) - - // Fall back to legacy autoupdate library - newerBinary, err = autoupdate.FindNewestSelf(ctx) - if err != nil { - return - } + return } if newerBinary == "" { diff --git a/cmd/launcher/svc_windows.go b/cmd/launcher/svc_windows.go index 85a2ad292..9d3a23a29 100644 --- a/cmd/launcher/svc_windows.go +++ b/cmd/launcher/svc_windows.go @@ -15,7 +15,6 @@ import ( "github.com/go-kit/kit/log/level" "github.com/kolide/kit/logutil" "github.com/kolide/kit/version" - "github.com/kolide/launcher/pkg/autoupdate" "github.com/kolide/launcher/pkg/contexts/ctxlog" "github.com/kolide/launcher/pkg/launcher" "github.com/kolide/launcher/pkg/log/locallogger" @@ -65,20 +64,6 @@ func runWindowsSvc(systemSlogger *multislogger.MultiSlogger, args []string) erro systemSlogger.AddHandler(localSloggerHandler) } - // Use the FindNewest mechanism to delete old - // updates. We do this here, as windows will pick up - // 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. - go func() { - time.Sleep(15 * time.Second) - _ = autoupdate.FindNewest( - context.TODO(), - os.Args[0], - autoupdate.DeleteOldUpdates(), - ) - }() - // Confirm that service configuration is up-to-date checkServiceConfiguration(systemSlogger.Logger, opts) diff --git a/ee/agent/knapsack/knapsack.go b/ee/agent/knapsack/knapsack.go index 9d5896df4..20ee82468 100644 --- a/ee/agent/knapsack/knapsack.go +++ b/ee/agent/knapsack/knapsack.go @@ -12,7 +12,6 @@ import ( "github.com/kolide/launcher/ee/agent/storage" "github.com/kolide/launcher/ee/agent/types" "github.com/kolide/launcher/ee/tuf" - "github.com/kolide/launcher/pkg/autoupdate" "github.com/kolide/launcher/pkg/log/multislogger" "go.etcd.io/bbolt" ) @@ -149,7 +148,7 @@ func (k *knapsack) getKVStore(storeType storage.Store) types.KVStore { func (k *knapsack) LatestOsquerydPath(ctx context.Context) string { latestBin, err := tuf.CheckOutLatest(ctx, "osqueryd", k.RootDirectory(), k.UpdateDirectory(), k.PinnedOsquerydVersion(), k.UpdateChannel(), k.Slogger()) if err != nil { - return autoupdate.FindNewest(ctx, k.OsquerydPath()) + return k.OsquerydPath() } return latestBin.Path diff --git a/pkg/autoupdate/findnew.go b/pkg/autoupdate/findnew.go deleted file mode 100644 index 412f4a538..000000000 --- a/pkg/autoupdate/findnew.go +++ /dev/null @@ -1,349 +0,0 @@ -package autoupdate - -import ( - "context" - "errors" - "fmt" - "os" - "path/filepath" - "runtime" - "sort" - "strings" - - "github.com/go-kit/kit/log" - "github.com/go-kit/kit/log/level" - "github.com/kolide/launcher/ee/tuf" - "github.com/kolide/launcher/pkg/contexts/ctxlog" -) - -// defaultBuildTimestamp is used to set the _oldest_ allowed update. Eg, if -// there's an update with a download timestamp older than build, just -// ignore it. It's probably indicative of a machine that's been re-installed. -// -// This is a private variable. It should be set via build time -// LDFLAGS. -const defaultBuildTimestamp = "0" - -// This suffix is added to the binary path to find the updates -const updateDirSuffix = "-updates" - -type newestSettings struct { - deleteOld bool - deleteCorrupt bool - skipFullBinaryPathCheck bool - buildTimestamp string - runningExecutable string -} - -type newestOption func(*newestSettings) - -func DeleteOldUpdates() newestOption { - return func(no *newestSettings) { - no.deleteOld = true - } -} - -func DeleteCorruptUpdates() newestOption { - return func(no *newestSettings) { - no.deleteCorrupt = true - } -} - -// overrideBuildTimestamp overrides the buildTimestamp constant. This -// is to allow tests to mock that behavior. It is not exported, as it -// is expected to only be used for testing. -func overrideBuildTimestamp(ts string) newestOption { - return func(no *newestSettings) { - no.buildTimestamp = ts - } -} - -// 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 - } -} - -// withRunningExectuable sets the current executable. This is because -// we never need to run an executable check against ourselves. (And -// doing so will trigger a fork bomb) -func withRunningExectuable(exe string) newestOption { - return func(no *newestSettings) { - no.runningExecutable = exe - } -} - -// 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. -func FindNewestSelf(ctx context.Context, opts ...newestOption) (string, error) { - logger := log.With(ctxlog.FromContext(ctx), "caller", log.DefaultCaller) - - exPath, err := os.Executable() - if err != nil { - return "", fmt.Errorf("determine running executable path: %w", err) - } - - if exPath == "" { - return "", errors.New("can't find newest empty string") - } - - opts = append(opts, SkipFullBinaryPathCheck(), withRunningExectuable(exPath)) - - newest := FindNewest(ctx, exPath, opts...) - - if newest == "" { - return "", nil - } - - if exPath == newest { - return "", nil - } - - level.Debug(logger).Log( - "msg", "found an update", - "newest", newest, - "exPath", exPath, - ) - - return newest, nil -} - -// FindNewest takes the full path to a binary, and returns the newest -// update on disk. If there are no updates on disk, it returns the -// original path. It will return the same fullBinaryPath if that is -// the newest version. -func FindNewest(ctx context.Context, fullBinaryPath string, opts ...newestOption) string { - logger := log.With(ctxlog.FromContext(ctx), "caller", log.DefaultCaller) - - if fullBinaryPath == "" { - level.Debug(logger).Log("msg", "called with empty string") - return "" - } - - newestSettings := &newestSettings{ - buildTimestamp: defaultBuildTimestamp, - } - for _, opt := range opts { - opt(newestSettings) - } - - updateDir := getUpdateDir(fullBinaryPath) - binaryName := filepath.Base(fullBinaryPath) - - logger = log.With(logger, - "fullBinaryPath", fullBinaryPath, - "updateDir", updateDir, - "binaryName", binaryName, - ) - - // If no updates are found, the forloop is skipped, and we return either the seed fullBinaryPath or "" - possibleUpdates, err := getPossibleUpdates(ctx, updateDir, binaryName) - if err != nil { - level.Error(logger).Log("msg", "could not find possible updates", "err", err) - return fullBinaryPath - } - - // iterate backwards over files, looking for a suitable binary - foundCount := 0 - foundFile := "" - for i := len(possibleUpdates) - 1; i >= 0; i-- { - file := possibleUpdates[i] - basedir := filepath.Dir(file) - updateDownloadTime := filepath.Base(basedir) - foundExecutable := file - if strings.HasSuffix(file, ".app") { - // Add back the rest of the path to the binary that we'd stripped off to make - // timestamp comparison and old/broken updates cleanup easier. - foundExecutable = filepath.Join(file, "Contents", "MacOS", binaryName) - } - - // We only want to consider updates with a download time _newer_ - // than our build timestamp. Note that we're not comparing against - // the update's build time, only the download time. This is an - // important distinction to allow for downgrades. - if strings.Compare(newestSettings.buildTimestamp, updateDownloadTime) >= 0 { - level.Debug(logger).Log( - "msg", "update download is older than buildtime", - "dir", basedir, - "buildtime", newestSettings.buildTimestamp, - ) - - if newestSettings.deleteOld { - if err := os.RemoveAll(basedir); err != nil { - level.Error(logger).Log("msg", "error deleting old update dir", "dir", basedir, "err", err) - } - } - - continue - } - - // If we've already found at least 2 files, (newest, and presumed - // current), trigger delete routine - if newestSettings.deleteOld && foundCount >= 2 { - level.Debug(logger).Log("msg", "deleting old updates", "dir", basedir) - if err := os.RemoveAll(basedir); err != nil { - level.Error(logger).Log("msg", "error deleting old update dir", "dir", basedir, "err", err) - } - } - - // If the file is _not_ the running executable, sanity - // check that executions work. If the exec fails, - // there's clearly an issue and we should remove it. - if newestSettings.runningExecutable != foundExecutable { - if err := tuf.CheckExecutable(ctx, foundExecutable, "--version"); err != nil { - if newestSettings.deleteCorrupt { - level.Error(logger).Log("msg", "not executable. Removing", "binary", foundExecutable, "reason", err) - if err := os.RemoveAll(basedir); err != nil { - level.Error(logger).Log("msg", "error deleting broken update dir", "dir", basedir, "err", err) - } - } else { - level.Error(logger).Log("msg", "not executable. Skipping", "binary", foundExecutable, "reason", err) - } - - continue - } - } else { - // This logging is mostly here to make test coverage of the conditional clear - level.Debug(logger).Log("msg", "Skipping checkExecutable against self", "file", file) - } - - // We always want to increment the foundCount, since it's what triggers deletion. - foundCount = foundCount + 1 - - // Only set what we've found, if it's unset. - if foundFile == "" { - foundFile = foundExecutable - } - } - - if foundFile != "" { - return foundFile - } - - level.Debug(logger).Log("msg", "no updates found") - - if newestSettings.skipFullBinaryPathCheck { - return fullBinaryPath - } - - if err := tuf.CheckExecutable(ctx, fullBinaryPath, "--version"); err != nil { - level.Debug(logger).Log("msg", "fullBinaryPath not executable. Returning nil", "err", err) - return "" - } - - return fullBinaryPath -} - -// getUpdateDir returns the expected update path for a given -// binary. It should work when called with either a base executable -// `/usr/local/bin/launcher` or with an existing updated -// `/usr/local/bin/launcher-updates/1234/launcher`. -// -// It makes some string assumptions about how things are named. -func getUpdateDir(fullBinaryPath string) string { - if strings.Contains(fullBinaryPath, ".app") { - binary := filepath.Base(fullBinaryPath) - return filepath.Join(findBaseDir(fullBinaryPath), binary+updateDirSuffix) - } - - // These are cases that shouldn't really happen. But, this is - // a bare string function. So return "" when they do. - fullBinaryPath = strings.TrimSuffix(fullBinaryPath, "/") - - if fullBinaryPath == "" { - return "" - } - - // If we SplitN on updateDirSuffix, we will either get an - // array, or the full string back. This means we can forgo a - // strings.Contains, and just use the returned element - components := strings.SplitN(fullBinaryPath, updateDirSuffix, 2) - - return fmt.Sprintf("%s%s", components[0], updateDirSuffix) -} - -// Find the possible updates. filepath.Glob returns a list of things -// that match the requested pattern. We sort the list to ensure that -// we can tell which ones are earlier or later (remember, these are -// timestamps). -func getPossibleUpdates(ctx context.Context, updateDir, binaryName string) ([]string, error) { - logger := log.With(ctxlog.FromContext(ctx), "caller", log.DefaultCaller) - - // If this is launcher running on macOS, then we should have app bundles available instead -- - // check for those first. - if runtime.GOOS == "darwin" { - binarySuffix := filepath.Join("Contents", "MacOS", binaryName) - fileGlob := filepath.Join(updateDir, "*", "*.app", binarySuffix) - possibleUpdates, err := filepath.Glob(fileGlob) - - if err == nil && len(possibleUpdates) > 0 { - appBundleNames := make([]string, len(possibleUpdates)) - for i, binaryPath := range possibleUpdates { - // We trim the suffix here for compatibility with prior logic for timestamp - // comparison in the directory and cleanup for old/broken updates. The suffix - // is added back later by the caller. - appBundleNames[i] = strings.TrimSuffix(binaryPath, "/"+binarySuffix) - } - sort.Strings(appBundleNames) - return appBundleNames, nil - } - - // If the error is non-nil, something has gone very wrong -- log and then ignore the - // error so that we can fall back to previous behavior below, so that launcher is - // still able to auto-update. - if err != nil { - level.Error(logger).Log("msg", "could not glob for app bundle binaries", "err", err) - } - } - - // Either not macOS/launcher or no app bundles found. Fall back to searching for binaries. - fileGlob := filepath.Join(updateDir, "*", binaryName) - - possibleUpdates, err := filepath.Glob(fileGlob) - if err != nil { - return nil, err - } - - sort.Strings(possibleUpdates) - - return possibleUpdates, nil -} - -// findBaseDir takes a binary path, that may or may not include the -// update directory, and returns the base directory. It's used by the -// launcher runtime in finding the various binaries. -func findBaseDir(path string) string { - if path == "" { - return "" - } - - // If this is an app bundle installation, we need to adjust the directory -- otherwise we end up with a library - // of updates at /usr/local//Kolide.app/Contents/MacOS/launcher-updates. - if strings.Contains(path, ".app") { - components := strings.SplitN(path, ".app", 2) - baseDir := filepath.Dir(components[0]) // gets rid of app bundle name and trailing slash - - // If baseDir still contains an update directory (i.e. the original path was something like - // /usr/local//launcher-updates//Kolide.app/Contents/MacOS/launcher), - // then strip the update directory out. - if strings.Contains(baseDir, updateDirSuffix) { - baseDirComponents := strings.SplitN(baseDir, updateDirSuffix, 2) - baseDir = filepath.Dir(baseDirComponents[0]) - } - - // We moved the Kolide.app installation out of the bin directory, but we want the bin directory - // here -- so put the "bin" suffix back on if needed. - if !strings.HasSuffix(baseDir, "bin") { - baseDir = filepath.Join(baseDir, "bin") - } - return baseDir - } - - components := strings.SplitN(path, updateDirSuffix, 2) - return filepath.Dir(components[0]) -} diff --git a/pkg/autoupdate/findnew_test.go b/pkg/autoupdate/findnew_test.go deleted file mode 100644 index f59081a0d..000000000 --- a/pkg/autoupdate/findnew_test.go +++ /dev/null @@ -1,479 +0,0 @@ -package autoupdate - -import ( - "context" - "fmt" - "io" - "os" - "path/filepath" - "runtime" - "strings" - "testing" - "time" - - "github.com/stretchr/testify/require" -) - -// TestFindNewestSelf tests the FindNewestSelf. Hard to test this, as -// it's a light wrapper around os.Executable -func TestFindNewestSelf(t *testing.T) { - t.Parallel() - - ctx := context.TODO() - - { - newest, err := FindNewestSelf(ctx) - require.NoError(t, err) - require.Empty(t, newest, "No updates, should be empty") - } - - // Let's try making a set of update directories - binaryPath := os.Args[0] - updatesDir := getUpdateDir(binaryPath) - require.NotEmpty(t, updatesDir) - defer os.RemoveAll(updatesDir) - for _, n := range []string{"2", "5", "3", "1"} { - require.NoError(t, os.MkdirAll(filepath.Join(updatesDir, n), 0755)) - f, err := os.Create(filepath.Join(updatesDir, n, "wrong-binary")) - require.NoError(t, err) - f.Close() - require.NoError(t, os.Chmod(f.Name(), 0755)) - } - - { - newest, err := FindNewestSelf(ctx) - require.NoError(t, err) - require.Empty(t, newest, "No correct binaries, should be empty") - } - - for _, n := range []string{"2", "3"} { - updatedBinaryPath := filepath.Join(updatesDir, n, filepath.Base(binaryPath)) - require.NoError(t, copyFile(updatedBinaryPath, binaryPath, false), "copy executable") - require.NoError(t, os.Chmod(updatedBinaryPath, 0755), "chmod") - } - - { - expectedNewest := filepath.Join(updatesDir, "3", filepath.Base(binaryPath)) - newest, err := FindNewestSelf(ctx) - require.NoError(t, err) - require.Equal(t, expectedNewest, newest, "Should find newer binary") - } - -} - -func TestGetUpdateDir(t *testing.T) { - t.Parallel() - - var tests = []struct { - in string - out string - }{ - {in: "/a/bin/path", out: "/a/bin/path-updates"}, - {in: "/a/bin/path-updates", out: "/a/bin/path-updates"}, - {in: "/a/bin/path-updates/1234/binary", out: "/a/bin/path-updates"}, - {in: "/a/bin/path/foo/bar-updates/1234/binary", out: "/a/bin/path/foo/bar-updates"}, - {in: "/a/bin/b-updates/123/b-updates/456/b", out: "/a/bin/b-updates"}, - {in: "/a/bin/path/", out: "/a/bin/path-updates"}, - {in: "/a/Test.app/Contents/MacOS/path", out: filepath.Clean("/a/bin/path-updates")}, - {in: "/a/bin/path-updates/1234/Test.app/Contents/MacOS/path", out: filepath.Clean("/a/bin/path-updates")}, - {in: "/a/bin/Test.app/Contents/MacOS/launcher-updates/1569339163/Test.app/Contents/MacOS/path", out: filepath.Clean("/a/bin/path-updates")}, - {in: "/a/bin/Test.app/Contents/MacOS/launcher", out: filepath.Clean("/a/bin/launcher-updates")}, - {in: "/a/bin/Test.app/Contents/MacOS/launcher-updates/1569339163/Test.app/Contents/MacOS/launcher", out: filepath.Clean("/a/bin/launcher-updates")}, - {in: "", out: ""}, - {in: "/", out: ""}, - } - - for _, tt := range tests { - require.Equal(t, tt.out, getUpdateDir(tt.in), "input: %s", tt.in) - } -} - -func TestFindBaseDir(t *testing.T) { - t.Parallel() - - var tests = []struct { - in string - out string - }{ - {in: "", out: ""}, - {in: "/a/path/bin/launcher", out: filepath.Clean("/a/path/bin")}, - {in: "/a/path/bin/launcher-updates/1569339163/launcher", out: filepath.Clean("/a/path/bin")}, - {in: "/a/path/bin/launcher-updates/1569339163/Test.app/Contents/MacOS/launcher", out: filepath.Clean("/a/path/bin")}, - {in: "/a/path/Test.app/Contents/MacOS/launcher", out: filepath.Clean("/a/path/bin")}, - {in: "/a/path/Test.app/Contents/MacOS/launcher-updates/1569339163/Test.app/Contents/MacOS/launcher", out: filepath.Clean("/a/path/bin")}, - {in: "/a/path/bin/Test.app/Contents/MacOS/launcher", out: filepath.Clean("/a/path/bin")}, - {in: "/a/path/bin/Test.app/Contents/MacOS/launcher-updates/1569339163/Test.app/Contents/MacOS/launcher", out: filepath.Clean("/a/path/bin")}, - } - - for _, tt := range tests { - require.Equal(t, tt.out, findBaseDir(tt.in), "input: %s", tt.in) - } -} - -func TestFindNewestEmpty(t *testing.T) { - t.Parallel() - - tmpDir, binaryName := setupTestDir(t, emptySetup) - ctx := context.TODO() - binaryPath := filepath.Join(tmpDir, binaryName) - - // Basic tests, test with binary and no updates - require.Empty(t, FindNewest(ctx, ""), "passing empty string") - require.Empty(t, FindNewest(ctx, tmpDir), "passing directory as arg") - require.Equal(t, binaryPath, FindNewest(ctx, binaryPath), "no update directory") -} - -func TestFindNewestEmptyUpdateDirs(t *testing.T) { - t.Parallel() - - tmpDir, binaryName := setupTestDir(t, emptyUpdateDirs) - ctx := context.TODO() - binaryPath := filepath.Join(tmpDir, binaryName) - - require.Equal(t, binaryPath, FindNewest(ctx, binaryPath), "update dir, but no updates") -} - -func TestFindNewestNonExecutable(t *testing.T) { - t.Parallel() - - if runtime.GOOS == "windows" { - t.Skip("Windows doesn't use executable bit") - } - - tmpDir, binaryName := setupTestDir(t, nonExecutableUpdates) - ctx := context.TODO() - binaryPath := filepath.Join(tmpDir, binaryName) - updatesDir := fmt.Sprintf("%s%s", binaryPath, updateDirSuffix) - - require.Equal(t, binaryPath, FindNewest(ctx, binaryPath), "update dir, but only plain files") - - expectedNewest := filepath.Join(updatesDir, "1", "binary") - require.NoError(t, os.Chmod(expectedNewest, 0755)) - if runtime.GOOS == "darwin" { - expectedNewest = filepath.Join(updatesDir, "1", "Test.app", "Contents", "MacOS", "binary") - require.NoError(t, os.Chmod(expectedNewest, 0755)) - } - - require.Equal(t, - expectedNewest, - FindNewest(ctx, binaryPath), - "Should find number 1", - ) -} - -func TestFindNewestExecutableUpdates(t *testing.T) { - t.Parallel() - - tmpDir, binaryName := setupTestDir(t, executableUpdates) - ctx := context.TODO() - binaryPath := filepath.Join(tmpDir, binaryName) - updatesDir := fmt.Sprintf("%s%s", binaryPath, updateDirSuffix) - - expectedNewest := filepath.Join(updatesDir, "5", "binary") - if runtime.GOOS == "windows" { - expectedNewest = expectedNewest + ".exe" - } else if runtime.GOOS == "darwin" { - expectedNewest = filepath.Join(updatesDir, "5", "Test.app", "Contents", "MacOS", "binary") - } - - require.Equal(t, expectedNewest, FindNewest(ctx, binaryPath), "Should find number 5") - require.Equal(t, expectedNewest, FindNewest(ctx, expectedNewest), "already running the newest") - -} - -func TestFindNewestCleanup(t *testing.T) { - t.Parallel() - - // delete doesn't seem to work on windows. It gets a - // "Access is denied" error". This may be a test setup - // issue, or something with an open file handle. - if runtime.GOOS == "windows" { - t.Skip("TODO: Windows deletion test is broken") - } - - tmpDir, binaryName := setupTestDir(t, executableUpdates) - ctx := context.TODO() - binaryPath := filepath.Join(tmpDir, binaryName) - updatesDir := fmt.Sprintf("%s%s", binaryPath, updateDirSuffix) - - expectedNewest := filepath.Join(updatesDir, "5", "binary") - if runtime.GOOS == "windows" { - expectedNewest = expectedNewest + ".exe" - } else if runtime.GOOS == "darwin" { - expectedNewest = filepath.Join(updatesDir, "5", "Test.app", "Contents", "MacOS", "binary") - } - - { - updatesOnDisk, err := os.ReadDir(updatesDir) - require.NoError(t, err) - require.Equal(t, 4, len(updatesOnDisk)) - require.Equal(t, expectedNewest, FindNewest(ctx, binaryPath), "Should find number 5") - } - - { - _ = FindNewest(ctx, binaryPath, DeleteOldUpdates()) - updatesOnDisk, err := os.ReadDir(updatesDir) - require.NoError(t, err) - require.Equal(t, expectedNewest, FindNewest(ctx, binaryPath), "Should find number 5") - require.Equal(t, 2, len(updatesOnDisk), "after delete") - - } -} - -func TestCheckExecutableCorruptCleanup(t *testing.T) { - t.Parallel() - - tmpDir, binaryName := setupTestDir(t, truncatedUpdates) - ctx := context.TODO() - binaryPath := filepath.Join(tmpDir, binaryName) - updatesDir := fmt.Sprintf("%s%s", binaryPath, updateDirSuffix) - - expectedNewest := filepath.Join(updatesDir, "3", "binary") - if runtime.GOOS == "windows" { - expectedNewest = expectedNewest + ".exe" - } else if runtime.GOOS == "darwin" { - expectedNewest = filepath.Join(updatesDir, "3", "Test.app", "Contents", "MacOS", "binary") - } - - { - updatesOnDisk, err := os.ReadDir(updatesDir) - require.NoError(t, err) - require.Equal(t, 4, len(updatesOnDisk)) - require.Equal(t, expectedNewest, FindNewest(ctx, binaryPath), "Should find number 3") - } - - { - _ = FindNewest(ctx, binaryPath, DeleteCorruptUpdates()) - updatesOnDisk, err := os.ReadDir(updatesDir) - require.NoError(t, err) - require.Equal(t, 2, len(updatesOnDisk), "after cleaning corruption") - require.Equal(t, expectedNewest, FindNewest(ctx, binaryPath), "Should find number 3") - } -} - -type setupState int - -const ( - emptySetup setupState = iota - emptyUpdateDirs - nonExecutableUpdates - executableUpdates - truncatedUpdates -) - -// setupTestDir function to setup the test dirs. This work is broken -// up in stages, allowing test functions to tap into various -// points. This is setup this way to allow simpler isolation on test -// failures. -func setupTestDir(t *testing.T, stage setupState) (string, string) { - tmpDir := t.TempDir() - - // Create a test binary - binaryName := windowsAddExe("binary") - binaryPath := filepath.Join(tmpDir, binaryName) - updatesDir := fmt.Sprintf("%s%s", binaryPath, updateDirSuffix) - - require.NoError(t, copyFile(binaryPath, os.Args[0], false), "copy executable") - require.NoError(t, os.Chmod(binaryPath, 0755), "chmod") - - if stage <= emptySetup { - return tmpDir, binaryName - } - - // make some update directories - // (these are out of order, to jumble up the create times) - for _, n := range []string{"2", "5", "3", "1"} { - require.NoError(t, os.MkdirAll(filepath.Join(updatesDir, n), 0755)) - if runtime.GOOS == "darwin" { - require.NoError(t, os.MkdirAll(filepath.Join(updatesDir, n, "Test.app", "Contents", "MacOS"), 0755)) - } - } - - if stage <= emptyUpdateDirs { - return tmpDir, binaryName - } - - // Copy executable to update directories - for _, n := range []string{"2", "5", "3", "1"} { - updatedBinaryPath := filepath.Join(updatesDir, n, binaryName) - require.NoError(t, copyFile(updatedBinaryPath, binaryPath, false), "copy executable") - if runtime.GOOS == "darwin" { - updatedAppBundleBinaryPath := filepath.Join(updatesDir, n, "Test.app", "Contents", "MacOS", filepath.Base(binaryPath)) - require.NoError(t, copyFile(updatedAppBundleBinaryPath, binaryPath, false), "copy executable") - } - } - - if stage <= nonExecutableUpdates { - return tmpDir, binaryName - } - - // Make our top-level binaries executable - for _, n := range []string{"2", "5", "3", "1"} { - require.NoError(t, os.Chmod(filepath.Join(updatesDir, n, binaryName), 0755)) - if runtime.GOOS == "darwin" { - require.NoError(t, os.Chmod(filepath.Join(updatesDir, n, "Test.app", "Contents", "MacOS", binaryName), 0755)) - } - } - - if stage <= executableUpdates { - return tmpDir, binaryName - } - - for _, n := range []string{"5", "1"} { - updatedBinaryPath := filepath.Join(updatesDir, n, binaryName) - require.NoError(t, copyFile(updatedBinaryPath, binaryPath, true), "copy & truncate executable") - if runtime.GOOS == "darwin" { - require.NoError(t, copyFile(filepath.Join(updatesDir, n, "Test.app", "Contents", "MacOS", binaryName), binaryPath, true), "copy & truncate executable") - } - } - - return tmpDir, binaryName - -} - -// copyFile copies a file from srcPath to dstPath. If truncate is set, -// only half the file is copied. (This is a trivial wrapper to -// simplify setting up test cases) -func copyFile(dstPath, srcPath string, truncate bool) error { - src, err := os.Open(srcPath) - if err != nil { - return err - } - defer src.Close() - - dst, err := os.Create(dstPath) - if err != nil { - return err - } - defer dst.Close() - - if !truncate { - if _, err := io.Copy(dst, src); err != nil { - return err - } - } else { - stat, err := src.Stat() - if err != nil { - return fmt.Errorf("statting srcFile: %w", err) - } - - if _, err = io.CopyN(dst, src, stat.Size()/2); err != nil { - return fmt.Errorf("copying srcFile: %w", err) - } - } - - return nil -} - -func TestBuildTimestamp(t *testing.T) { - t.Parallel() - - if runtime.GOOS == "windows" { - t.Skip("FIXME: Windows") - } - - var tests = []struct { - buildTimestamp string - expectedNewest string - expectedOnDisk int - }{ - { - buildTimestamp: "0", - expectedNewest: "5", - expectedOnDisk: 2, - }, - { - buildTimestamp: "3", - expectedNewest: "5", - expectedOnDisk: 1, // remember, 4 is broken, so there should only be update 5 on disk - }, - { - buildTimestamp: "5", - expectedOnDisk: 0, - }, - { - buildTimestamp: "6", - expectedOnDisk: 0, - }, - } - - for _, tt := range tests { - tt := tt - t.Run("buildTimestamp="+tt.buildTimestamp, func(t *testing.T) { - t.Parallel() - - tmpDir, binaryName := setupTestDir(t, executableUpdates) - ctx := context.TODO() - - binaryPath := filepath.Join(tmpDir, binaryName) - updatesDir := binaryPath + updateDirSuffix - - returnedNewest := FindNewest( - ctx, - binaryPath, - overrideBuildTimestamp(tt.buildTimestamp), - DeleteOldUpdates(), - ) - - updatesOnDisk, err := os.ReadDir(updatesDir) - require.NoError(t, err) - require.Equal(t, tt.expectedOnDisk, len(updatesOnDisk), "remaining updates on disk") - - if tt.expectedNewest == "" { - require.Equal(t, binaryPath, returnedNewest, "Expected to get original binary path") - } else { - updateFragment := strings.TrimPrefix(strings.TrimPrefix(returnedNewest, updatesDir), "/") - expectedNewest := filepath.Join(tt.expectedNewest, "binary") - if runtime.GOOS == "darwin" { - expectedNewest = filepath.Join(tt.expectedNewest, "Test.app", "Contents", "MacOS", "binary") - } - require.Equal(t, expectedNewest, updateFragment) - } - - }) - } - -} - -// TestHelperProcess isn't a real test. It's used as a helper process -// to make a portableish binary. See -// https://github.com/golang/go/blob/master/src/os/exec/exec_test.go#L724 -// and https://npf.io/2015/06/testing-exec-command/ -func TestHelperProcess(t *testing.T) { - t.Parallel() - - // find out magic arguments - args := os.Args - for len(args) > 0 { - if args[0] == "--" { - args = args[1:] - break - } - args = args[1:] - } - if len(args) == 0 { - // Indicates an error, or just being run in the test suite. - return - } - - switch args[0] { - case "sleep": - time.Sleep(10 * time.Second) - case "exit0": - os.Exit(0) - case "exit1": - os.Exit(1) - case "exit2": - os.Exit(2) - } - - // default behavior nothing -} - -func windowsAddExe(in string) string { - if runtime.GOOS == "windows" { - return in + ".exe" - } - - return in -} diff --git a/pkg/make/builder.go b/pkg/make/builder.go index cb728e5d7..2762788be 100644 --- a/pkg/make/builder.go +++ b/pkg/make/builder.go @@ -20,7 +20,6 @@ import ( "path/filepath" "regexp" "runtime" - "strconv" "strings" "time" @@ -342,9 +341,6 @@ func (b *Builder) BuildCmd(src, appName string) func(context.Context) error { ldFlags = append(ldFlags, fmt.Sprintf(`-X "github.com/kolide/kit/version.goVersion=%s"`, runtime.Version())) } - // Set the build time for autoupdate.FindNewest - ldFlags = append(ldFlags, fmt.Sprintf(`-X "github.com/kolide/launcher/pkg/autoupdate.defaultBuildTimestamp=%s"`, strconv.FormatInt(time.Now().Unix(), 10))) - if len(ldFlags) != 0 { baseArgs = append(baseArgs, fmt.Sprintf("--ldflags=%s", strings.Join(ldFlags, " "))) } diff --git a/pkg/osquery/runtime/runner.go b/pkg/osquery/runtime/runner.go index f3979764d..a904693fd 100644 --- a/pkg/osquery/runtime/runner.go +++ b/pkg/osquery/runtime/runner.go @@ -16,7 +16,6 @@ import ( "github.com/kolide/launcher/ee/agent/types" "github.com/kolide/launcher/ee/tuf" - "github.com/kolide/launcher/pkg/autoupdate" "github.com/kolide/launcher/pkg/backoff" "github.com/kolide/launcher/pkg/osquery/runtime/history" "github.com/kolide/launcher/pkg/osquery/table" @@ -311,21 +310,14 @@ func (r *Runner) launchOsqueryInstance() error { // see if we have the newest version. Do this every time. If // this proves undesirable, we can expose a function to set // o.opts.binaryPath in the finalizer to call. - // - // FindNewest uses context as a way to get a logger, so we need to - // create and pass a ctxlog in. var currentOsquerydBinaryPath string currentOsquerydBinary, err := tuf.CheckOutLatest(ctx, "osqueryd", o.opts.rootDirectory, o.opts.updateDirectory, o.knapsack.PinnedOsquerydVersion(), o.opts.updateChannel, r.slogger) if err != nil { r.slogger.Log(ctx, slog.LevelDebug, - "could not get latest version of osqueryd from new autoupdate library, falling back", + "could not get latest version of osqueryd from autoupdate library", "err", err, ) - currentOsquerydBinaryPath = autoupdate.FindNewest( - ctx, - o.opts.binaryPath, - autoupdate.DeleteOldUpdates(), - ) + return fmt.Errorf("getting osqueryd path: %w", err) } else { currentOsquerydBinaryPath = currentOsquerydBinary.Path } From 8b366bada238162283d725f2808ccd2ecfb51df7 Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Thu, 11 Apr 2024 10:13:20 -0400 Subject: [PATCH 8/8] Remove WithOsquerydBinary option since we now retrieve it from the knapsack --- cmd/launcher/launcher.go | 1 - pkg/osquery/runtime/osqueryinstance.go | 12 ------- pkg/osquery/runtime/runner.go | 43 ++--------------------- pkg/osquery/runtime/runtime_posix_test.go | 6 ++-- pkg/osquery/runtime/runtime_test.go | 21 ++++------- 5 files changed, 12 insertions(+), 71 deletions(-) diff --git a/cmd/launcher/launcher.go b/cmd/launcher/launcher.go index 93d46309d..5ccfe5e57 100644 --- a/cmd/launcher/launcher.go +++ b/cmd/launcher/launcher.go @@ -325,7 +325,6 @@ func runLauncher(ctx context.Context, cancel func(), multiSlogger, systemMultiSl osqueryRunner := osqueryruntime.New( k, osqueryruntime.WithKnapsack(k), - osqueryruntime.WithOsquerydBinary(k.OsquerydPath()), osqueryruntime.WithRootDirectory(k.RootDirectory()), osqueryruntime.WithOsqueryExtensionPlugins(table.LauncherTables(k)...), osqueryruntime.WithSlogger(k.Slogger().With("component", "osquery_instance")), diff --git a/pkg/osquery/runtime/osqueryinstance.go b/pkg/osquery/runtime/osqueryinstance.go index 632a40a0b..8fa121722 100644 --- a/pkg/osquery/runtime/osqueryinstance.go +++ b/pkg/osquery/runtime/osqueryinstance.go @@ -43,17 +43,6 @@ func WithOsqueryExtensionPlugins(plugins ...osquery.OsqueryPlugin) OsqueryInstan } } -// WithOsquerydBinary is a functional option which allows the user to define the -// path of the osqueryd binary which will be launched. This should only be called -// once as only one binary will be executed. Defining the path to the osqueryd -// binary is optional. If it is not explicitly defined by the caller, an osqueryd -// binary will be looked for in the current $PATH. -func WithOsquerydBinary(path string) OsqueryInstanceOption { - return func(i *OsqueryInstance) { - i.opts.binaryPath = path - } -} - // WithRootDirectory is a functional option which allows the user to define the // path where filesystem artifacts will be stored. This may include pidfiles, // RocksDB database files, etc. If this is not defined, a temporary directory @@ -305,7 +294,6 @@ type osqueryOptions struct { // the following are options which may or may not be set by the functional // options included by the caller of LaunchOsqueryInstance augeasLensFunc func(dir string) error - binaryPath string configPluginFlag string distributedPluginFlag string extensionPlugins []osquery.OsqueryPlugin diff --git a/pkg/osquery/runtime/runner.go b/pkg/osquery/runtime/runner.go index a904693fd..4ac24eb14 100644 --- a/pkg/osquery/runtime/runner.go +++ b/pkg/osquery/runtime/runner.go @@ -6,8 +6,6 @@ import ( "fmt" "log/slog" "os" - "os/exec" - "runtime" "strings" "sync" "time" @@ -15,7 +13,6 @@ import ( "github.com/kolide/launcher/ee/agent/flags/keys" "github.com/kolide/launcher/ee/agent/types" - "github.com/kolide/launcher/ee/tuf" "github.com/kolide/launcher/pkg/backoff" "github.com/kolide/launcher/pkg/osquery/runtime/history" "github.com/kolide/launcher/pkg/osquery/table" @@ -216,23 +213,6 @@ func (r *Runner) launchOsqueryInstance() error { o := r.instance - // What binary name to look for - lookFor := "osqueryd" - if runtime.GOOS == "windows" { - lookFor = lookFor + ".exe" - } - - // If the path of the osqueryd binary wasn't explicitly defined by the caller, - // try to find it in the path. - if o.opts.binaryPath == "" { - path, err := exec.LookPath(lookFor) - if err != nil { - traces.SetError(span, fmt.Errorf("osqueryd not supplied and not found: %w", err)) - return fmt.Errorf("osqueryd not supplied and not found: %w", err) - } - o.opts.binaryPath = path - } - // If the caller did not define the directory which all of the osquery file // artifacts should be stored in, use a temporary directory. if o.opts.rootDirectory == "" { @@ -301,26 +281,9 @@ func (r *Runner) launchOsqueryInstance() error { o.opts.distributedPluginFlag = "internal_noop" } - // If we're on windows, ensure that we're looking for the .exe - if runtime.GOOS == "windows" && !strings.HasSuffix(o.opts.binaryPath, ".exe") { - o.opts.binaryPath = o.opts.binaryPath + ".exe" - } - - // before we start osqueryd, check with the update system to - // see if we have the newest version. Do this every time. If - // this proves undesirable, we can expose a function to set - // o.opts.binaryPath in the finalizer to call. - var currentOsquerydBinaryPath string - currentOsquerydBinary, err := tuf.CheckOutLatest(ctx, "osqueryd", o.opts.rootDirectory, o.opts.updateDirectory, o.knapsack.PinnedOsquerydVersion(), o.opts.updateChannel, r.slogger) - if err != nil { - r.slogger.Log(ctx, slog.LevelDebug, - "could not get latest version of osqueryd from autoupdate library", - "err", err, - ) - return fmt.Errorf("getting osqueryd path: %w", err) - } else { - currentOsquerydBinaryPath = currentOsquerydBinary.Path - } + // The knapsack will retrieve the correct version of osqueryd from the download library if available. + // If not available, it will fall back to the configured installed version of osqueryd. + currentOsquerydBinaryPath := o.knapsack.LatestOsquerydPath(ctx) span.AddEvent("got_osqueryd_binary_path", trace.WithAttributes(attribute.String("path", currentOsquerydBinaryPath))) // Now that we have accepted options from the caller and/or determined what diff --git a/pkg/osquery/runtime/runtime_posix_test.go b/pkg/osquery/runtime/runtime_posix_test.go index 615829dc3..f4d09d8c1 100644 --- a/pkg/osquery/runtime/runtime_posix_test.go +++ b/pkg/osquery/runtime/runtime_posix_test.go @@ -50,13 +50,12 @@ func TestOsquerySlowStart(t *testing.T) { k.On("RegisterChangeObserver", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) slogger := multislogger.New(slog.NewJSONHandler(&logBytes, &slog.HandlerOptions{Level: slog.LevelDebug})) k.On("Slogger").Return(slogger.Logger) - k.On("PinnedOsquerydVersion").Return("") + k.On("LatestOsquerydPath", mock.Anything).Return(testOsqueryBinaryDirectory) runner := New( k, WithKnapsack(k), WithRootDirectory(rootDirectory), - WithOsquerydBinary(testOsqueryBinaryDirectory), WithSlogger(slogger.Logger), WithStartFunc(func(cmd *exec.Cmd) error { err := cmd.Start() @@ -95,7 +94,7 @@ func TestExtensionSocketPath(t *testing.T) { k.On("WatchdogEnabled").Return(false) k.On("RegisterChangeObserver", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) k.On("Slogger").Return(multislogger.NewNopLogger()) - k.On("PinnedOsquerydVersion").Return("") + k.On("LatestOsquerydPath", mock.Anything).Return(testOsqueryBinaryDirectory) extensionSocketPath := filepath.Join(rootDirectory, "sock") runner := New( @@ -103,7 +102,6 @@ func TestExtensionSocketPath(t *testing.T) { WithKnapsack(k), WithRootDirectory(rootDirectory), WithExtensionSocketPath(extensionSocketPath), - WithOsquerydBinary(testOsqueryBinaryDirectory), ) go runner.Run() diff --git a/pkg/osquery/runtime/runtime_test.go b/pkg/osquery/runtime/runtime_test.go index 74680d2e4..18bab0514 100644 --- a/pkg/osquery/runtime/runtime_test.go +++ b/pkg/osquery/runtime/runtime_test.go @@ -312,13 +312,12 @@ func TestBadBinaryPath(t *testing.T) { k.On("WatchdogEnabled").Return(false) k.On("Slogger").Return(multislogger.NewNopLogger()) k.On("RegisterChangeObserver", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) - k.On("PinnedOsquerydVersion").Return("") + k.On("LatestOsquerydPath", mock.Anything).Return("") runner := New( k, WithKnapsack(k), WithRootDirectory(rootDirectory), - WithOsquerydBinary("/foobar"), ) assert.Error(t, runner.Run()) @@ -336,13 +335,12 @@ func TestWithOsqueryFlags(t *testing.T) { k.On("WatchdogEnabled").Return(false) k.On("RegisterChangeObserver", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) k.On("Slogger").Return(multislogger.NewNopLogger()) - k.On("PinnedOsquerydVersion").Return("") + k.On("LatestOsquerydPath", mock.Anything).Return(testOsqueryBinaryDirectory) runner := New( k, WithKnapsack(k), WithRootDirectory(rootDirectory), - WithOsquerydBinary(testOsqueryBinaryDirectory), WithOsqueryFlags([]string{"verbose=false"}), ) go runner.Run() @@ -369,14 +367,13 @@ func TestFlagsChanged(t *testing.T) { k.On("WatchdogDelaySec").Return(120) k.On("RegisterChangeObserver", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) k.On("Slogger").Return(multislogger.NewNopLogger()) - k.On("PinnedOsquerydVersion").Return("") + k.On("LatestOsquerydPath", mock.Anything).Return(testOsqueryBinaryDirectory) // Start the runner runner := New( k, WithKnapsack(k), WithRootDirectory(rootDirectory), - WithOsquerydBinary(testOsqueryBinaryDirectory), WithOsqueryFlags([]string{"verbose=false"}), ) go runner.Run() @@ -463,13 +460,12 @@ func TestSimplePath(t *testing.T) { k.On("WatchdogEnabled").Return(false) k.On("RegisterChangeObserver", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) k.On("Slogger").Return(multislogger.NewNopLogger()) - k.On("PinnedOsquerydVersion").Return("") + k.On("LatestOsquerydPath", mock.Anything).Return(testOsqueryBinaryDirectory) runner := New( k, WithKnapsack(k), WithRootDirectory(rootDirectory), - WithOsquerydBinary(testOsqueryBinaryDirectory), ) go runner.Run() @@ -492,13 +488,12 @@ func TestMultipleShutdowns(t *testing.T) { k.On("WatchdogEnabled").Return(false) k.On("RegisterChangeObserver", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) k.On("Slogger").Return(multislogger.NewNopLogger()) - k.On("PinnedOsquerydVersion").Return("") + k.On("LatestOsquerydPath", mock.Anything).Return(testOsqueryBinaryDirectory) runner := New( k, WithKnapsack(k), WithRootDirectory(rootDirectory), - WithOsquerydBinary(testOsqueryBinaryDirectory), ) go runner.Run() @@ -520,13 +515,12 @@ func TestOsqueryDies(t *testing.T) { k.On("WatchdogEnabled").Return(false) k.On("RegisterChangeObserver", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) k.On("Slogger").Return(multislogger.NewNopLogger()) - k.On("PinnedOsquerydVersion").Return("") + k.On("LatestOsquerydPath", mock.Anything).Return(testOsqueryBinaryDirectory) runner := New( k, WithKnapsack(k), WithRootDirectory(rootDirectory), - WithOsquerydBinary(testOsqueryBinaryDirectory), ) go runner.Run() require.NoError(t, err) @@ -615,13 +609,12 @@ func setupOsqueryInstanceForTests(t *testing.T) (runner *Runner, teardown func() k.On("WatchdogDelaySec").Return(120) k.On("RegisterChangeObserver", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Maybe() k.On("Slogger").Return(multislogger.NewNopLogger()) - k.On("PinnedOsquerydVersion").Return("") + k.On("LatestOsquerydPath", mock.Anything).Return(testOsqueryBinaryDirectory) runner = New( k, WithKnapsack(k), WithRootDirectory(rootDirectory), - WithOsquerydBinary(testOsqueryBinaryDirectory), ) go runner.Run() waitHealthy(t, runner)