fix(FLEETMDM-002-2): CU-86akj32d7 62 review findings across 40 files

ee/orbit/pkg/httpsigproxy/httpsigproxy.go

@@ -110,6 +110,7 @@ func NewProxy(
 
 	handler, err := newProxyHandler(targetURL, rootCA, insecure, signer)
 	if err != nil {
+		listener.Close()
 		return nil, fmt.Errorf("make proxy handler: %w", err)
 	}
 
@@ -226,7 +227,7 @@ type signingRoundTripper struct {
 func (s *signingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
 	// Sign the request before sending
 	if err := s.signer.Sign(req); err != nil {
-		return nil, fmt.Errorf("signing request: %#v", err)
+		return nil, fmt.Errorf("signing request: %w", err)
 	}
 
 	// Remove X-Forwarded-For because we are forwarding from 127.0.0.1,

ee/server/calendar/google_calendar_load.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"log/slog"
@@ -126,7 +125,7 @@ func (lowLevelAPI *GoogleCalendarLoadAPI) CreateEvent(event *calendar.Event) (*c
 }
 
 func (lowLevelAPI *GoogleCalendarLoadAPI) UpdateEvent(event *calendar.Event) (*calendar.Event, error) {
-	return nil, errors.New("GoogleCalendarLoadAPI.UpdateEvent is not implemented")
+	return nil, fmt.Errorf("update event: %w", errNotImplemented)
 }
 
 func (lowLevelAPI *GoogleCalendarLoadAPI) GetEvent(id, _ string) (*calendar.Event, error) {
@@ -249,3 +248,5 @@ func (lowLevelAPI *GoogleCalendarLoadAPI) Watch(eventUUID string, channelID stri
 func (lowLevelAPI *GoogleCalendarLoadAPI) Stop(channelID string, resourceID string) error {
 	return nil
 }
+
+var errNotImplemented = errors.New("GoogleCalendarLoadAPI.UpdateEvent is not implemented")

orbit/cmd/desktop/desktop_linux.go

@@ -54,7 +54,8 @@ func blockWaitForStopEvent(_ string) error {
 func trayIconExists() bool {
 	conn, err := dbus.SessionBus()
 	if err != nil {
-		log.Error().Err(err)
+		log.Error().Err(err).Msg("trayIconExists: connect to session bus")
+		return false
 	}
 
 	// Get the name we would expect systray to reserve for our tray icon.

orbit/pkg/dataflatten/json.go

@@ -10,7 +10,7 @@ import (
 func JsonFile(file string, opts ...FlattenOpts) ([]Row, error) {
 	rawdata, err := os.ReadFile(file)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("reading json file %s: %w", file, err)
 	}
 	return Json(rawdata, opts...)
 }

orbit/pkg/dataflatten/jsonl.go

@@ -11,7 +11,7 @@ import (
 func JsonlFile(file string, opts ...FlattenOpts) ([]Row, error) {
 	f, err := os.Open(file)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("opening jsonl file %s: %w", file, err)
 	}
 	defer f.Close()
 	return Jsonl(f, opts...)

orbit/pkg/dataflatten/plist.go

@@ -11,7 +11,7 @@ import (
 func PlistFile(file string, opts ...FlattenOpts) ([]Row, error) {
 	rawdata, err := os.ReadFile(file)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("reading plist file %s: %w", file, err)
 	}
 	return Plist(rawdata, opts...)
 }

orbit/pkg/go-paniclog/paniclog_windows.go

@@ -10,7 +10,7 @@
 package paniclog
 
 import (
-	"errors"
+	"fmt"
 	"os"
 	"syscall"
 )
@@ -57,28 +57,28 @@ func setStdHandle(stdhandle int32, handle syscall.Handle) error {
 func redirectStderr(f *os.File) (UndoFunction, error) {
 	stderrFd, err := getStdHandle(syscall.STD_ERROR_HANDLE)
 	if err != nil {
-		return nil, errors.New("Failed to redirect stderr to file: " + err.Error())
+		return nil, fmt.Errorf("failed to redirect stderr to file: %w", err)
 	}
 
 	// duplicate the handle to match unix behavior
 	fHandle, err := dupFD(f.Fd())
 	if err != nil {
-		return nil, errors.New("Failed to duplicate file: " + err.Error())
+		return nil, fmt.Errorf("failed to duplicate file: %w", err)
 	}
 
 	err = setStdHandle(syscall.STD_ERROR_HANDLE, fHandle)
 	if err != nil {
-		return nil, errors.New("Failed to redirect stderr to file: " + err.Error())
+		return nil, fmt.Errorf("failed to redirect stderr to file: %w", err)
 	}
 
 	undo := func() error {
 		err := setStdHandle(syscall.STD_ERROR_HANDLE, stderrFd)
 		if err != nil {
-			return errors.New("Failed to redirect stderr to file: " + err.Error())
+			return fmt.Errorf("failed to redirect stderr to file: %w", err)
 		}
 		err = syscall.CloseHandle(fHandle)
 		if err != nil {
-			return errors.New("Failed to close STD_ERROR handle: " + err.Error())
+			return fmt.Errorf("failed to close STD_ERROR handle: %w", err)
 		}
 		return nil
 	}

orbit/pkg/osquery/osquery.go

@@ -182,6 +182,9 @@ func (r *Runner) Execute() error {
 
 // Runner interrupts the running osquery process.
 func (r *Runner) Interrupt(err error) {
+	if err != nil {
+		log.Info().Err(fmt.Errorf("interrupt osqueryd runner: %w", err)).Msg("osqueryd runner interrupted")
+	}
 	if _, cancel := r.getContextAndCancel(); cancel != nil {
 		cancel()
 	}

orbit/pkg/packaging/linux_shared.go

@@ -57,7 +57,7 @@ func buildNFPM(opt Options, pkger nfpm.Packager) (string, error) {
 	}
 
 	if opt.Architecture != ArchAmd64 && opt.Architecture != ArchArm64 {
-		return "", fmt.Errorf("Invalid architecture: %s", opt.Architecture)
+		return "", fmt.Errorf("invalid architecture: %s", opt.Architecture)
 	}
 
 	// Initialize autoupdate metadata
@@ -543,3 +543,4 @@ pkill fleet-desktop || true
 
 	return nil
 }
+

orbit/pkg/packaging/macos_rcodesign.go

@@ -12,12 +12,23 @@ import (
 )
 
 func rSign(pkgPath, cert string) error {
-	pemPath := filepath.Join(os.TempDir(), "cert.pem")
-	defer os.Remove(pemPath)
-	err := os.WriteFile(pemPath, []byte(cert), 0o600)
+	pemFile, err := os.CreateTemp("", "cert-*.pem")
 	if err != nil {
+		return fmt.Errorf("creating cert temp file: %s", err)
+	}
+	pemPath := pemFile.Name()
+	defer os.Remove(pemPath)
+	if err := pemFile.Chmod(0o600); err != nil {
+		pemFile.Close()
+		return fmt.Errorf("setting cert temp file permissions: %s", err)
+	}
+	if _, err := pemFile.WriteString(cert); err != nil {
+		pemFile.Close()
 		return fmt.Errorf("writing cert data: %s", err)
 	}
+	if err := pemFile.Close(); err != nil {
+		return fmt.Errorf("closing cert temp file: %s", err)
+	}
 
 	return retry.Do(func() error {
 		var outBuf bytes.Buffer
@@ -30,8 +41,7 @@ func rSign(pkgPath, cert string) error {
 		cmd.Stdout = &outBuf
 		cmd.Stderr = &outBuf
 		if err := cmd.Run(); err != nil {
-			fmt.Println(outBuf.String())
-			return fmt.Errorf("rcodesign: %w", err)
+			return fmt.Errorf("rcodesign: %w: %s", err, outBuf.String())
 		}
 		return nil
 	}, retry.WithMaxAttempts(3))
@@ -56,8 +66,7 @@ func rNotarizeStaple(pkg, apiKeyID, apiKeyIssuer, apiKeyContent string) error {
 		cmd.Stdout = &outBuf
 		cmd.Stderr = &outBuf
 		if err := cmd.Run(); err != nil {
-			fmt.Println(outBuf.String())
-			return fmt.Errorf("rcodesign notarize: %w", err)
+			return fmt.Errorf("rcodesign notarize: %w: %s", err, outBuf.String())
 		}
 		return nil
 	}, retry.WithMaxAttempts(3))
@@ -72,8 +81,8 @@ func writeAPIKeys(issuer, id, content string) (string, error) {
 	// The underliying tools (rcodesign and Transporter) expect to find a
 	// certificate key in this path.
 	path := filepath.Join(homedir, ".appstoreconnect", "private_keys")
-	if err = secure.MkdirAll(path, 0o600); err != nil {
-		return "", fmt.Errorf("finding home dir: %s", err)
+	if err = secure.MkdirAll(path, 0o700); err != nil {
+		return "", fmt.Errorf("creating private keys dir: %s", err)
 	}
 
 	keyPath := filepath.Join(path, fmt.Sprintf("AuthKey_%s.p8", id))

orbit/pkg/packaging/wix/transform.go

@@ -5,6 +5,7 @@ import (
 	"encoding/xml"
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 )
 
@@ -64,13 +65,35 @@ func TransformHeat(path string) error {
 		return fmt.Errorf("marshal xml: %w", err)
 	}
 
+	// Write to a temp file in the same directory first, then rename over the
+	// original. This avoids destroying the original file if the write fails,
+	// and the rename is atomic on the same filesystem.
+	dir := filepath.Dir(path)
+	tmp, err := os.CreateTemp(dir, filepath.Base(path)+".tmp-*")
+	if err != nil {
+		return fmt.Errorf("create temp file: %w", err)
+	}
+	tmpPath := tmp.Name()
+	defer os.Remove(tmpPath)
+
+	if _, err := tmp.Write(contents); err != nil {
+		tmp.Close()
+		return fmt.Errorf("write temp file: %w", err)
+	}
+	if err := tmp.Close(); err != nil {
+		return fmt.Errorf("close temp file: %w", err)
+	}
+	if err := os.Chmod(tmpPath, 0o600); err != nil {
+		return fmt.Errorf("chmod temp file: %w", err)
+	}
+
 	// Remove first as we encounter permission errors on some Linux configurations.
 	if err := os.Remove(path); err != nil {
 		return fmt.Errorf("remove old file: %w", err)
 	}
 
-	if err := os.WriteFile(path, contents, 0o600); err != nil {
-		return fmt.Errorf("write file: %w", err)
+	if err := os.Rename(tmpPath, path); err != nil {
+		return fmt.Errorf("rename temp file: %w", err)
 	}
 
 	return nil

orbit/pkg/table/extension.go

@@ -129,7 +129,7 @@ func (r *Runner) Execute() error {
 	r.srv.RegisterPlugin(plugins...)
 
 	if err := r.srv.Run(); err != nil {
-		return err
+		return fmt.Errorf("running extension manager server: %w", err)
 	}
 
 	return nil