Small cleanup for initial TUF rollout (#1305)

kolide · Aug 30, 2023 · 5d943ae · 5d943ae
1 parent 04d9aae
commit 5d943ae
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 12 deletions.
diff --git a/cmd/launcher/launcher.go b/cmd/launcher/launcher.go
@@ -383,7 +383,7 @@ func runLauncher(ctx context.Context, cancel func(), opts *launcher.Options) err
 	if k.Autoupdate() {
 		// Create a new TUF autoupdater
 		metadataClient := http.DefaultClient
-		metadataClient.Timeout = 1 * time.Minute
+		metadataClient.Timeout = 30 * time.Second
 		mirrorClient := http.DefaultClient
 		mirrorClient.Timeout = 8 * time.Minute // gives us extra time to avoid a timeout on download
 		tufAutoupdater, err := tuf.NewTufAutoupdater(

diff --git a/pkg/autoupdate/findnew.go b/pkg/autoupdate/findnew.go
@@ -251,8 +251,7 @@ func FindNewest(ctx context.Context, fullBinaryPath string, opts ...newestOption
 //
 // It makes some string assumptions about how things are named.
 func getUpdateDir(fullBinaryPath string) string {
-	installedPath := os.Getenv(LegacyAutoupdatePathEnvVar)
-	if installedPath != "" {
+	if installedPath := os.Getenv(LegacyAutoupdatePathEnvVar); installedPath != "" {
 		fullBinaryPath = installedPath
 	}
 
@@ -263,9 +262,7 @@ func getUpdateDir(fullBinaryPath string) string {
 
 	// These are cases that shouldn't really happen. But, this is
 	// a bare string function. So return "" when they do.
-	if strings.HasSuffix(fullBinaryPath, "/") {
-		fullBinaryPath = strings.TrimSuffix(fullBinaryPath, "/")
-	}
+	fullBinaryPath = strings.TrimSuffix(fullBinaryPath, "/")
 
 	if fullBinaryPath == "" {
 		return ""
@@ -334,8 +331,7 @@ func FindBaseDir(path string) string {
 		return ""
 	}
 
-	installedPath := os.Getenv(LegacyAutoupdatePathEnvVar)
-	if installedPath != "" {
+	if installedPath := os.Getenv(LegacyAutoupdatePathEnvVar); installedPath != "" {
 		path = installedPath
 	}
 

diff --git a/pkg/autoupdate/findnew_test.go b/pkg/autoupdate/findnew_test.go
@@ -92,6 +92,10 @@ func TestGetUpdateDir_WithEnvVar(t *testing.T) { //nolint:paralleltest
 	// Since we're setting an environment variable, we don't want to run this test at the
 	// same time as other tests.
 
+	t.Cleanup(func() {
+		os.Setenv(LegacyAutoupdatePathEnvVar, "")
+	})
+
 	var tests = []struct {
 		installPath string
 		currentPath string
@@ -123,8 +127,6 @@ func TestGetUpdateDir_WithEnvVar(t *testing.T) { //nolint:paralleltest
 		os.Setenv(LegacyAutoupdatePathEnvVar, tt.installPath)
 		require.Equal(t, tt.out, getUpdateDir(tt.currentPath), "input: install path %s, current path %s", tt.installPath, tt.currentPath)
 	}
-
-	os.Setenv(LegacyAutoupdatePathEnvVar, "")
 }
 
 func TestFindBaseDir(t *testing.T) {
@@ -153,6 +155,10 @@ func TestFindBaseDir_WithEnvVar(t *testing.T) { //nolint:paralleltest
 	// Since we're setting an environment variable, we don't want to run this test at the
 	// same time as other tests.
 
+	t.Cleanup(func() {
+		os.Setenv(LegacyAutoupdatePathEnvVar, "")
+	})
+
 	var tests = []struct {
 		installPath string
 		currentPath string
@@ -184,8 +190,6 @@ func TestFindBaseDir_WithEnvVar(t *testing.T) { //nolint:paralleltest
 		os.Setenv(LegacyAutoupdatePathEnvVar, tt.installPath)
 		require.Equal(t, tt.out, FindBaseDir(tt.currentPath), "input: install path %s, current path %s", tt.installPath, tt.currentPath)
 	}
-
-	os.Setenv(LegacyAutoupdatePathEnvVar, "")
 }
 
 func TestFindNewestEmpty(t *testing.T) {

diff --git a/pkg/autoupdate/tuf/library_manager.go b/pkg/autoupdate/tuf/library_manager.go
@@ -197,6 +197,10 @@ func (ulm *updateLibraryManager) moveVerifiedUpdate(binary autoupdatableBinary,
 	if err := os.Rename(stagedVersionedDirectory, newUpdateDirectory); err != nil {
 		return fmt.Errorf("could not move staged target %s from %s to %s: %w", targetFilename, stagedVersionedDirectory, newUpdateDirectory, err)
 	}
+	// Need rwxr-xr-x so that the desktop (running as user) can execute the downloaded binary too
+	if err := os.Chmod(newUpdateDirectory, 0755); err != nil {
+		return fmt.Errorf("could not chmod %s: %w", newUpdateDirectory, err)
+	}
 
 	return nil
 }

diff --git a/pkg/autoupdate/tuf/library_manager_test.go b/pkg/autoupdate/tuf/library_manager_test.go
@@ -145,6 +145,9 @@ func TestAddToLibrary(t *testing.T) {
 			dirInfo, err := os.Stat(filepath.Join(updatesDirectory(tt.binary, testBaseDir), testReleaseVersion))
 			require.NoError(t, err, "checking that update was downloaded")
 			require.True(t, dirInfo.IsDir())
+			if runtime.GOOS == "darwin" || runtime.GOOS == "linux" {
+				require.Equal(t, "drwxr-xr-x", dirInfo.Mode().String())
+			}
 			executableInfo, err := os.Stat(executableLocation(filepath.Join(updatesDirectory(tt.binary, testBaseDir), testReleaseVersion), tt.binary))
 			require.NoError(t, err, "checking that downloaded update includes executable")
 			require.False(t, executableInfo.IsDir())