fix(FLEETMDM-002): CU-86akbhhtv 93 review findings across 36 files

cmd/msrc/generate.go

@@ -10,36 +10,40 @@ import (
 	"time"
 
 	"github.com/fleetdm/fleet/v4/pkg/fleethttp"
+	"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
 	"github.com/fleetdm/fleet/v4/server/vulnerabilities/io"
 	"github.com/fleetdm/fleet/v4/server/vulnerabilities/msrc"
 	"github.com/fleetdm/fleet/v4/server/vulnerabilities/msrc/parsed"
 	"github.com/google/go-github/v37/github"
 )
 
-func panicif(err error) {
-	if err != nil {
-		panic(err)
-	}
-}
-
 const cleanEnvVar = "MSRC_CLEAN"
 
 func main() {
+	ctx := context.Background()
+
 	wd, err := os.Getwd()
-	panicif(err)
+	if err != nil {
+		ctxerr.Handle(ctx, ctxerr.Wrap(ctx, err, "get working directory"))
+		os.Exit(1)
+	}
 
 	inPath := filepath.Join(wd, "msrc_in")
 	err = os.MkdirAll(inPath, 0o755)
-	panicif(err)
+	if err != nil {
+		ctxerr.Handle(ctx, ctxerr.Wrap(ctx, err, "create msrc_in directory"))
+		os.Exit(1)
+	}
 
 	outPath := filepath.Join(wd, "msrc_out")
 	err = os.MkdirAll(outPath, 0o755)
-	panicif(err)
+	if err != nil {
+		ctxerr.Handle(ctx, ctxerr.Wrap(ctx, err, "create msrc_out directory"))
+		os.Exit(1)
+	}
 
 	now := time.Now()
 
-	ctx := context.Background()
-
 	githubHttp := fleethttp.NewGithubClient()
 	ghAPI := io.NewGitHubClient(githubHttp, github.NewClient(githubHttp).Repositories, wd)
 
@@ -48,22 +52,34 @@ func main() {
 
 	fmt.Println("Downloading existing MSRC bulletins...")
 	eBulletins, err := ghAPI.MSRCBulletins(ctx)
-	panicif(err)
+	if err != nil {
+		ctxerr.Handle(ctx, ctxerr.Wrap(ctx, err, "download existing MSRC bulletins"))
+		os.Exit(1)
+	}
 	var bulletins []*parsed.SecurityBulletin
-	if len(eBulletins) == 0 || os.Getenv(cleanEnvVar) != "false" {
+	if len(eBulletins) == 0 || os.Getenv(cleanEnvVar) == "true" {
 		fmt.Println("None found, backfilling...")
 		bulletins, err = backfill(now.Month(), now.Year(), msrcAPI)
-		panicif(err)
+		if err != nil {
+			ctxerr.Handle(ctx, ctxerr.Wrap(ctx, err, "backfill bulletins"))
+			os.Exit(1)
+		}
 	} else {
 		fmt.Println("Updating existing bulletins")
 		bulletins, err = update(now.Month(), now.Year(), eBulletins, msrcAPI, ghAPI)
-		panicif(err)
+		if err != nil {
+			ctxerr.Handle(ctx, ctxerr.Wrap(ctx, err, "update bulletins"))
+			os.Exit(1)
+		}
 	}
 
 	fmt.Println("Saving bulletins...")
 	for _, b := range bulletins {
 		err := serialize(b, now, outPath)
-		panicif(err)
+		if err != nil {
+			ctxerr.Handle(ctx, ctxerr.Wrap(ctx, err, "serialize bulletin"))
+			os.Exit(1)
+		}
 	}
 
 	fmt.Println("Done processing MSRC feed.")

ee/server/licensing/licensing.go

@@ -5,12 +5,13 @@ import (
 	"crypto/x509"
 	_ "embed"
 	"encoding/pem"
-	"errors"
 	"fmt"
 	"time"
 
+	"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
 	"github.com/fleetdm/fleet/v4/server/fleet"
 	"github.com/golang-jwt/jwt/v4"
+	"golang.org/x/net/context"
 )
 
 const (
@@ -22,25 +23,27 @@ const (
 var pubKeyPEM []byte
 
 // loadPublicKey loads the public key from pubkey.pem.
-func loadPublicKey() (*ecdsa.PublicKey, error) {
+func loadPublicKey(ctx context.Context) (*ecdsa.PublicKey, error) {
 	block, _ := pem.Decode(pubKeyPEM)
 	if block == nil {
-		return nil, errors.New("no key block found in pem")
+		return nil, ctxerr.New(ctx, "no key block found in pem")
 	}
 
 	pub, err := x509.ParsePKIXPublicKey(block.Bytes)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse ecdsa key: %w", err)
+		return nil, ctxerr.Wrap(ctx, err, "failed to parse ecdsa key")
 	}
 
 	if pub, ok := pub.(*ecdsa.PublicKey); ok {
 		return pub, nil
 	}
-	return nil, fmt.Errorf("%T is not *ecdsa.PublicKey", pub)
+	return nil, ctxerr.Errorf(ctx, "%T is not *ecdsa.PublicKey", pub)
 }
 
 // LoadLicense loads and validates the license key.
 func LoadLicense(licenseKey string) (*fleet.LicenseInfo, error) {
+	ctx := context.Background()
+
 	// No license key
 	if licenseKey == "" {
 		return &fleet.LicenseInfo{Tier: fleet.TierFree}, nil
@@ -51,22 +54,22 @@ func LoadLicense(licenseKey string) (*fleet.LicenseInfo, error) {
 		&licenseClaims{},
 		// Always use the same public key
 		func(*jwt.Token) (interface{}, error) {
-			return loadPublicKey()
+			return loadPublicKey(ctx)
 		},
 	)
 	if err != nil {
 		v, _ := err.(*jwt.ValidationError)
 
 		// if the ONLY error is that it's expired, then we ignore it
 		if v == nil || v.Errors != jwt.ValidationErrorExpired {
-			return nil, fmt.Errorf("parse license: %w", err)
+			return nil, ctxerr.Wrap(ctx, err, "parse license")
 		}
 		parsedToken.Valid = true
 	}
 
-	license, err := validate(parsedToken)
+	license, err := validate(ctx, parsedToken)
 	if err != nil {
-		return nil, fmt.Errorf("validate license: %w", err)
+		return nil, ctxerr.Wrap(ctx, err, "validate license")
 	}
 
 	// for backwards compatibility we'll convert basic tier to premium
@@ -84,38 +87,38 @@ type licenseClaims struct {
 	AllowDisableTelemetry bool   `json:"notel"`
 }
 
-func validate(token *jwt.Token) (*fleet.LicenseInfo, error) {
+func validate(ctx context.Context, token *jwt.Token) (*fleet.LicenseInfo, error) {
 	// token.IssuedAt, token.ExpiresAt, token.NotBefore already validated by JWT
 	// library.
 	if !token.Valid {
 		// ParseWithClaims should have errored already, but double-check here
-		return nil, errors.New("token invalid")
+		return nil, ctxerr.New(ctx, "token invalid")
 	}
 
 	if token.Method.Alg() != expectedAlgorithm {
-		return nil, fmt.Errorf("unexpected algorithm %s", token.Method.Alg())
+		return nil, ctxerr.Errorf(ctx, "unexpected algorithm %s", token.Method.Alg())
 	}
 
 	var claims *licenseClaims
 	claims, ok := token.Claims.(*licenseClaims)
 	if !ok || claims == nil {
-		return nil, fmt.Errorf("unexpected claims type %T", token.Claims)
+		return nil, ctxerr.Errorf(ctx, "unexpected claims type %T", token.Claims)
 	}
 
 	if claims.Devices == 0 {
-		return nil, errors.New("missing devices")
+		return nil, ctxerr.New(ctx, "missing devices")
 	}
 
 	if claims.Tier == "" {
-		return nil, errors.New("missing tier")
+		return nil, ctxerr.New(ctx, "missing tier")
 	}
 
 	if claims.ExpiresAt == 0 {
-		return nil, errors.New("missing exp")
+		return nil, ctxerr.New(ctx, "missing exp")
 	}
 
 	if claims.Issuer != expectedIssuer {
-		return nil, fmt.Errorf("unexpected issuer %s", claims.Issuer)
+		return nil, ctxerr.Errorf(ctx, "unexpected issuer %s", claims.Issuer)
 	}
 
 	return &fleet.LicenseInfo{

ee/server/service/appconfig.go

@@ -3,6 +3,7 @@ package service
 import (
 	"context"
 
+	"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
 	"github.com/fleetdm/fleet/v4/server/fleet"
 )
 
@@ -16,14 +17,14 @@ func (svc *Service) HostFeatures(ctx context.Context, host *fleet.Host) (*fleet.
 	if host.TeamID != nil {
 		features, err := svc.ds.TeamFeatures(ctx, *host.TeamID)
 		if err != nil {
-			return nil, err
+			return nil, ctxerr.Wrap(ctx, err, "get team features")
 		}
 		return features, nil
 	}
 
 	appConfig, err := svc.ds.AppConfig(ctx)
 	if err != nil {
-		return nil, err
+		return nil, ctxerr.Wrap(ctx, err, "get app config")
 	}
 	return &appConfig.Features, nil
 }

ee/server/service/calendar.go

@@ -24,7 +24,7 @@ func (svc *Service) CalendarWebhook(ctx context.Context, eventUUID string, chann
 	appConfig, err := svc.ds.AppConfig(ctx)
 	if err != nil {
 		svc.authz.SkipAuthorization(ctx)
-		return fmt.Errorf("load app config: %w", err)
+		return ctxerr.New(ctx, fmt.Sprintf("load app config: %s", err))
 	}
 
 	if len(appConfig.Integrations.GoogleCalendar) == 0 {
@@ -341,6 +341,9 @@ func (svc *Service) getCalendarLock(ctx context.Context, eventUUID string, addTo
 
 func (svc *Service) processCalendarAsync(ctx context.Context, eventIDs []string) {
 	defer func() {
+		if r := recover(); r != nil {
+			svc.logger.ErrorContext(ctx, "Recovered from panic in async calendar processing", "err", r)
+		}
 		asyncMutex.Lock()
 		asyncCalendarProcessing = false
 		asyncMutex.Unlock()
@@ -452,3 +455,4 @@ func (svc *Service) processCalendarEventAsync(ctx context.Context, eventUUID str
 	}
 	return true
 }
+

ee/server/service/condaccess/depot/depot.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"crypto/rsa"
 	"crypto/x509"
-	"errors"
 	"fmt"
 	"log/slog"
 	"math/big"
@@ -57,7 +56,7 @@ func (d *ConditionalAccessSCEPDepot) CA(_ []byte) ([]*x509.Certificate, *rsa.Pri
 
 	pk, ok := cert.PrivateKey.(*rsa.PrivateKey)
 	if !ok {
-		return nil, nil, errors.New("private key not in RSA format")
+		return nil, nil, ctxerr.New(context.Background(), "private key not in RSA format")
 	}
 
 	return []*x509.Certificate{cert.Leaf}, pk, nil
@@ -90,37 +89,39 @@ func (d *ConditionalAccessSCEPDepot) HasCN(cn string, allowTime int, cert *x509.
 // The UUID is used to look up the host in Fleet, and the certificate is only issued
 // if the host exists. Old certificates for the same host are automatically revoked.
 func (d *ConditionalAccessSCEPDepot) Put(name string, crt *x509.Certificate) error {
+	ctx := context.Background()
+
 	if crt.Subject.CommonName == "" || len(crt.Subject.CommonName) > maxCommonNameLength {
-		return errors.New("common name empty or too long")
+		return ctxerr.New(ctx, "common name empty or too long")
 	}
 	if !crt.SerialNumber.IsInt64() {
-		return errors.New("cannot represent serial number as int64")
+		return ctxerr.New(ctx, "cannot represent serial number as int64")
 	}
 
 	// Extract UUID from SAN URI
 	// Expected format: urn:device:apple:uuid:<device-uuid>
 	uuid := extractUUIDFromCert(crt)
 	if uuid == "" {
-		return errors.New("no device UUID found in certificate SAN URI")
+		return ctxerr.New(ctx, "no device UUID found in certificate SAN URI")
 	}
 
 	// Look up host BEFORE storing certificate
-	host, err := d.ds.HostByIdentifier(context.Background(), uuid)
+	host, err := d.ds.HostByIdentifier(ctx, uuid)
 	if err != nil {
 		return fmt.Errorf("host not found for UUID %s: %w", uuid, err)
 	}
 
 	// Apply rate limiting if configured
 	cooldown := d.config.Osquery.EnrollCooldown
 	if cooldown > 0 {
-		existingCertCreatedAt, err := d.ds.GetConditionalAccessCertCreatedAtByHostID(context.Background(), host.ID)
+		existingCertCreatedAt, err := d.ds.GetConditionalAccessCertCreatedAtByHostID(ctx, host.ID)
 		switch {
 		case err != nil && !fleet.IsNotFound(err):
 			return fmt.Errorf("checking existing certificate: %w", err)
 		case err == nil:
 			// Certificate exists, check if rate limit applies
 			if time.Since(*existingCertCreatedAt) < cooldown {
-				return backoff.Permanent(ctxerr.Errorf(context.Background(), "host %s requesting certificates too often", uuid))
+				return backoff.Permanent(ctxerr.Errorf(ctx, "host %s requesting certificates too often", uuid))
 			}
 		}
 		// If certificate doesn't exist or rate limit doesn't apply, continue
@@ -135,7 +136,7 @@ func (d *ConditionalAccessSCEPDepot) Put(name string, crt *x509.Certificate) err
 	// This prevents authentication failures when:
 	// - Network delays in delivering the new certificate to the client
 	// - Client is offline during certificate rotation (client will request new cert when it comes back online)
-	_, err = d.db.ExecContext(context.Background(), `
+	_, err = d.db.ExecContext(ctx, `
 		INSERT INTO conditional_access_scep_certificates
 			(serial, host_id, name, not_valid_before, not_valid_after, certificate_pem)
 		VALUES
@@ -151,7 +152,7 @@ func (d *ConditionalAccessSCEPDepot) Put(name string, crt *x509.Certificate) err
 		return err
 	}
 
-	d.logger.InfoContext(context.TODO(), "stored conditional access certificate",
+	d.logger.InfoContext(ctx, "stored conditional access certificate",
 		"cn", name,
 		"serial", crt.SerialNumber.Int64(),
 		"host_id", host.ID,

ee/server/service/condaccess/scep.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/cenkalti/backoff/v4"
 	"github.com/fleetdm/fleet/v4/server/config"
+	"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
 	"github.com/fleetdm/fleet/v4/server/fleet"
 	"github.com/fleetdm/fleet/v4/server/mdm/assets"
 	scepdepot "github.com/fleetdm/fleet/v4/server/mdm/scep/depot"
@@ -94,12 +95,12 @@ func challengeMiddleware(ds fleet.Datastore, next scepserver.CSRSignerContext) s
 		// Always require a valid challenge password
 
 		if m.ChallengePassword == "" {
-			return nil, errors.New("missing challenge")
+			return nil, ctxerr.New(ctx, "missing challenge")
 		}
 		_, err := ds.VerifyEnrollSecret(ctx, m.ChallengePassword)
 		switch {
 		case fleet.IsNotFound(err):
-			return nil, errors.New("invalid challenge")
+			return nil, ctxerr.New(ctx, "invalid challenge")
 		case err != nil:
 			return nil, fmt.Errorf("verifying enrollment secret: %w", err)
 		}
@@ -192,18 +193,24 @@ func (svc *service) PKIOperation(ctx context.Context, data []byte) ([]byte, erro
 			return nil, &RateLimitError{Message: err.Error()}
 		}
 
-		certRep, err := msg.Fail(cert.Leaf, pk, scep.BadRequest)
+		certRep, failErr := msg.Fail(cert.Leaf, pk, scep.BadRequest)
+		if failErr != nil {
+			return nil, failErr
+		}
 		if certRep == nil {
-			return nil, err
+			return nil, failErr
 		}
-		return certRep.Raw, err
+		return certRep.Raw, nil
 	}
 
 	certRep, err := msg.Success(cert.Leaf, pk, crt)
+	if err != nil {
+		return nil, err
+	}
 	if certRep == nil {
 		return nil, err
 	}
-	return certRep.Raw, err
+	return certRep.Raw, nil
 }
 
 // GetNextCACert is not implemented for conditional access SCEP.