From 4e0a804c7e2525f9145e0ff6244d26af8d080fbf Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:31 +0000 Subject: [PATCH 01/36] fix(FLEETMDM-002): 93 review findings across 36 files --- .../mysql/migrations/tables/migration.go | 105 +++++++++--------- 1 file changed, 51 insertions(+), 54 deletions(-) diff --git a/server/datastore/mysql/migrations/tables/migration.go b/server/datastore/mysql/migrations/tables/migration.go index b8724086bdf..f6e8d1350cc 100644 --- a/server/datastore/mysql/migrations/tables/migration.go +++ b/server/datastore/mysql/migrations/tables/migration.go @@ -10,10 +10,10 @@ import ( "sync/atomic" "time" + "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/goose" "github.com/jmoiron/sqlx" - "github.com/pkg/errors" ) var MigrationClient = goose.New("migration_status_tables", goose.MySqlDialect{}) @@ -29,14 +29,20 @@ type migrationStep func(tx *sql.Tx) error func basicMigrationStep(statement string, errorMessage string) migrationStep { return func(tx *sql.Tx) error { _, err := tx.Exec(statement) - return errors.Wrap(err, errorMessage) + if err != nil { + return ctxerr.Wrap(nil, err, errorMessage) + } + return nil } } func basicMigrationStepWithArgs(statement string, args []any, errorMessage string) migrationStep { return func(tx *sql.Tx) error { _, err := tx.Exec(statement, args...) - return errors.Wrap(err, errorMessage) + if err != nil { + return ctxerr.Wrap(nil, err, errorMessage) + } + return nil } } @@ -60,7 +66,7 @@ func incrementalMigrationStep(count getTotalCountFn, execute executeWithProgress // Every five seconds, echo the % progress of the executor // Since we output once the migration step is complete, we need an extra channel to indicate when both the step - // and the "step complete" output are com0plete + // and the "step complete" output are complete stepComplete := make(chan struct{}) outputComplete := make(chan struct{}) go func() { @@ -105,36 +111,47 @@ func withSteps(steps []migrationStep, tx *sql.Tx) error { return nil } -func fkExists(tx *sql.Tx, table, name string) bool { +// queryRower is satisfied by both *sql.Tx and *sqlx.DB, allowing existence-check +// helpers to share a single implementation. +type queryRower interface { + QueryRow(query string, args ...interface{}) *sql.Row +} + +func rowExists(q queryRower, query string, args ...interface{}) bool { var count int - err := tx.QueryRow(` -SELECT COUNT(1) -FROM information_schema.REFERENTIAL_CONSTRAINTS -WHERE CONSTRAINT_SCHEMA = DATABASE() -AND TABLE_NAME = ? -AND CONSTRAINT_NAME = ? - `, table, name).Scan(&count) + err := q.QueryRow(query, args...).Scan(&count) if err != nil { + if !errIsNoRows(err) { + fmt.Fprintf(outputTo, "warning: existence check query failed: %v\n", err) + } return false } return count > 0 } +func errIsNoRows(err error) bool { + return err == sql.ErrNoRows +} + +func fkExists(tx *sql.Tx, table, name string) bool { + return rowExists(tx, ` +SELECT COUNT(1) +FROM information_schema.REFERENTIAL_CONSTRAINTS +WHERE CONSTRAINT_SCHEMA = DATABASE() +AND TABLE_NAME = ? +AND CONSTRAINT_NAME = ? + `, table, name) +} + func constraintExists(tx *sql.Tx, table, name string) bool { - var count int - err := tx.QueryRow(` + return rowExists(tx, ` SELECT COUNT(1) FROM information_schema.TABLE_CONSTRAINTS WHERE CONSTRAINT_SCHEMA = DATABASE() AND TABLE_NAME = ? AND CONSTRAINT_NAME = ? - `, table, name).Scan(&count) - if err != nil { - return false - } - - return count > 0 + `, table, name) } func columnExists(tx *sql.Tx, table, column string) bool { @@ -166,6 +183,7 @@ WHERE `, inColumns), args..., ).Scan(&count) if err != nil { + fmt.Fprintf(outputTo, "warning: existence check query failed: %v\n", err) return false } @@ -173,9 +191,7 @@ WHERE } func tableExists(tx *sql.Tx, table string) bool { - var count int - err := tx.QueryRow( - ` + return rowExists(tx, ` SELECT count(*) FROM @@ -183,46 +199,27 @@ FROM WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = ? -`, - table, - ).Scan(&count) - if err != nil { - return false - } - - return count > 0 +`, table) } -func indexExists(tx *sqlx.DB, table, index string) bool { - var count int - err := tx.QueryRow(` +func indexExists(db *sqlx.DB, table, index string) bool { + return rowExists(db, ` SELECT COUNT(1) FROM INFORMATION_SCHEMA.STATISTICS WHERE table_schema = DATABASE() AND table_name = ? AND index_name = ? -`, table, index).Scan(&count) - if err != nil { - return false - } - - return count > 0 +`, table, index) } func indexExistsTx(tx *sql.Tx, table, index string) bool { - var count int - err := tx.QueryRow(` + return rowExists(tx, ` SELECT COUNT(1) FROM INFORMATION_SCHEMA.STATISTICS WHERE table_schema = DATABASE() AND table_name = ? AND index_name = ? -`, table, index).Scan(&count) - if err != nil { - return false - } - - return count > 0 +`, table, index) } // updateAppConfigJSON updates the `json_value` stored in the `app_config_json` after applying the @@ -231,29 +228,29 @@ func updateAppConfigJSON(tx *sql.Tx, fn func(config *fleet.AppConfig) error) err var raw []byte row := tx.QueryRow(`SELECT json_value FROM app_config_json LIMIT 1`) if err := row.Scan(&raw); err != nil { - if errors.Is(err, sql.ErrNoRows) { + if err == sql.ErrNoRows { return nil } - return errors.Wrap(err, "select app_config_json") + return ctxerr.Wrap(nil, err, "select app_config_json") } var config fleet.AppConfig if err := json.Unmarshal(raw, &config); err != nil { - return errors.Wrap(err, "unmarshal app_config_json") + return ctxerr.Wrap(nil, err, "unmarshal app_config_json") } if err := fn(&config); err != nil { - return errors.Wrap(err, "callback app_config_json") + return ctxerr.Wrap(nil, err, "callback app_config_json") } b, err := json.Marshal(config) if err != nil { - return errors.Wrap(err, "marshal updated app_config_json") + return ctxerr.Wrap(nil, err, "marshal updated app_config_json") } const updateStmt = `UPDATE app_config_json SET json_value = ? WHERE id = 1` if _, err := tx.Exec(updateStmt, b); err != nil { - return errors.Wrap(err, "update app_config_json") + return ctxerr.Wrap(nil, err, "update app_config_json") } return nil From 5580d02b77d7cb44af482e7231a66b6a20524bdb Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:33 +0000 Subject: [PATCH 02/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/datastore/mysql/nanomdm_storage.go | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/server/datastore/mysql/nanomdm_storage.go b/server/datastore/mysql/nanomdm_storage.go index c2e0ead15a4..6c5d6ce978a 100644 --- a/server/datastore/mysql/nanomdm_storage.go +++ b/server/datastore/mysql/nanomdm_storage.go @@ -23,6 +23,7 @@ import ( "github.com/jmoiron/sqlx" ) +// >>> OPENFRAME(mdm-lock-unlock-wipe): fork-specific lock/unlock/wipe command logic — openframe/docs/mdm-lock-unlock-wipe.md // lockConflictError indicates a lock command already exists for the host type lockConflictError struct { hostUUID string @@ -51,6 +52,8 @@ func isConflict(err error) bool { return false } +// <<< OPENFRAME(mdm-lock-unlock-wipe) + // NanoMDMStorage wraps a *nanomdm_mysql.MySQLStorage and overrides further functionality. type NanoMDMStorage struct { *nanomdm_mysql.MySQLStorage @@ -100,6 +103,7 @@ func (ds *Datastore) NewTestMDMAppleMDMStorage(asyncCap int, asyncInterval time. }, nil } +// >>> OPENFRAME(push-cert-staleness-cache): in-memory push cert staleness caching — openframe/docs/mdm-lock-unlock-wipe.md type pushCertStalenessCheck struct { hash string updatedAt time.Time @@ -112,6 +116,8 @@ var ( pushCertStalenessMu sync.RWMutex ) +// <<< OPENFRAME(push-cert-staleness-cache) + // RetrievePushCert partially implements nanomdm_storage.PushCertStore. // // Returns the push certificate and its MD5 checksum as the stale token. @@ -128,6 +134,7 @@ func (s *NanoMDMStorage) RetrievePushCert( return cert, checksum, nil } +// >>> OPENFRAME(push-cert-staleness-cache): in-memory push cert staleness caching — openframe/docs/mdm-lock-unlock-wipe.md // checkInMemoryHash checks the incoming hash agains the in-memory hash. // if criteria is met, it updates the in-memory hash with the new hash and updatedAt = now. func checkInMemoryHash(hash string) { @@ -175,11 +182,14 @@ func (s *NanoMDMStorage) IsPushCertStale(ctx context.Context, topic, staleToken return false, nil } +// <<< OPENFRAME(push-cert-staleness-cache) + // StorePushCert partially implements nanomdm_storage.PushCertStore. func (s *NanoMDMStorage) StorePushCert(ctx context.Context, pemCert, pemKey []byte) error { - return errors.New("please use fleet.Datastore to manage MDM assets") + return ctxerr.New(ctx, "please use fleet.Datastore to manage MDM assets") } +// >>> OPENFRAME(mdm-lock-unlock-wipe): fork-specific lock/unlock/wipe command logic — openframe/docs/mdm-lock-unlock-wipe.md // GetPendingLockCommand returns the most recent unacknowledged DeviceLock command // for the given host, along with its unlock PIN. // Returns nil, "", nil if no pending lock command exists. @@ -257,7 +267,7 @@ func (s *NanoMDMStorage) EnqueueDeviceLockCommand( // Now enqueue the command if err := enqueueCommandDB(ctx, tx, []string{host.UUID}, cmd); err != nil { - return err + return fmt.Errorf("enqueueing device lock command: %w", err) } // Insert or update the host_mdm_actions row @@ -286,7 +296,7 @@ func (s *NanoMDMStorage) EnqueueDeviceLockCommand( func (s *NanoMDMStorage) EnqueueDeviceUnlockCommand(ctx context.Context, host *fleet.Host, cmd *mdm.Command) error { return common_mysql.WithRetryTxx(ctx, s.db, func(tx sqlx.ExtContext) error { if err := enqueueCommandDB(ctx, tx, []string{host.UUID}, cmd); err != nil { - return err + return fmt.Errorf("enqueueing device unlock command: %w", err) } stmt := ` @@ -312,7 +322,7 @@ func (s *NanoMDMStorage) EnqueueDeviceUnlockCommand(ctx context.Context, host *f func (s *NanoMDMStorage) EnqueueDeviceWipeCommand(ctx context.Context, host *fleet.Host, cmd *mdm.Command) error { return common_mysql.WithRetryTxx(ctx, s.db, func(tx sqlx.ExtContext) error { if err := enqueueCommandDB(ctx, tx, []string{host.UUID}, cmd); err != nil { - return err + return fmt.Errorf("enqueueing device wipe command: %w", err) } stmt := ` @@ -333,6 +343,8 @@ func (s *NanoMDMStorage) EnqueueDeviceWipeCommand(ctx context.Context, host *fle }, s.logger) } +// <<< OPENFRAME(mdm-lock-unlock-wipe) + func (s *NanoMDMStorage) GetAllMDMConfigAssetsByName(ctx context.Context, assetNames []fleet.MDMAssetName, queryerContext sqlx.QueryerContext, ) (map[fleet.MDMAssetName]fleet.MDMConfigAsset, error) { @@ -414,7 +426,7 @@ func (s *NanoDEPStorage) RetrieveAuthTokens(ctx context.Context, name string) (* // StoreAuthTokens partially implements nanodep.AuthTokensStorer. func (s *NanoDEPStorage) StoreAuthTokens(ctx context.Context, name string, tokens *nanodep_client.OAuth1Tokens) error { - return errors.New("please use fleet.Datastore to manage MDM assets") + return ctxerr.New(ctx, "please use fleet.Datastore to manage MDM assets") } func enqueueCommandDB(ctx context.Context, tx sqlx.ExtContext, ids []string, cmd *mdm.Command) error { From 73d5f4c2db24a0460ecd338cf3c76406e12f8bb5 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:34 +0000 Subject: [PATCH 03/36] fix(FLEETMDM-002): 93 review findings across 36 files --- ee/server/service/hostidentity/depot/depot.go | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/ee/server/service/hostidentity/depot/depot.go b/ee/server/service/hostidentity/depot/depot.go index abb7039ff18..73d53e5bbda 100644 --- a/ee/server/service/hostidentity/depot/depot.go +++ b/ee/server/service/hostidentity/depot/depot.go @@ -5,8 +5,6 @@ import ( "crypto/ecdsa" "crypto/rsa" "crypto/x509" - "errors" - "fmt" "log/slog" "math/big" "time" @@ -50,14 +48,15 @@ func NewHostIdentitySCEPDepot(db *sqlx.DB, ds fleet.Datastore, logger *slog.Logg // CA returns the CA's certificate and private key. func (d *HostIdentitySCEPDepot) CA(_ []byte) ([]*x509.Certificate, *rsa.PrivateKey, error) { - cert, err := assets.KeyPair(context.Background(), d.ds, fleet.MDMAssetHostIdentityCACert, fleet.MDMAssetHostIdentityCAKey) + ctx := context.Background() + cert, err := assets.KeyPair(ctx, d.ds, fleet.MDMAssetHostIdentityCACert, fleet.MDMAssetHostIdentityCAKey) if err != nil { - return nil, nil, fmt.Errorf("getting assets: %w", err) + return nil, nil, ctxerr.Wrap(ctx, err, "getting assets") } pk, ok := cert.PrivateKey.(*rsa.PrivateKey) if !ok { - return nil, nil, errors.New("private key not in RSA format") + return nil, nil, ctxerr.New(ctx, "private key not in RSA format") } return []*x509.Certificate{cert.Leaf}, pk, nil @@ -68,16 +67,20 @@ func (d *HostIdentitySCEPDepot) Serial() (*big.Int, error) { // Insert an empty row to generate a new auto-incremented serial number result, err := d.db.Exec(`INSERT INTO host_identity_scep_serials () VALUES ();`) if err != nil { - return nil, err + return nil, ctxerr.Wrap(context.Background(), err, "inserting host identity scep serial") } lid, err := result.LastInsertId() if err != nil { - return nil, err + return nil, ctxerr.Wrap(context.Background(), err, "getting last insert id for host identity scep serial") } return big.NewInt(lid), nil } // HasCN returns whether the given certificate exists in the depot. +// NOTE: This is currently a stub that always returns false. It is not used for +// renewal decisions right now, but may be implemented in the future to support +// SCEP renewal semantics. Until then, SCEP clients relying on HasCN for renewal +// will always be told the certificate does not exist. func (d *HostIdentitySCEPDepot) HasCN(cn string, allowTime int, cert *x509.Certificate, revokeOldCertificate bool) (bool, error) { // Not used right now. May be used for renewal in the future. return false, nil @@ -85,11 +88,12 @@ func (d *HostIdentitySCEPDepot) HasCN(cn string, allowTime int, cert *x509.Certi // Put stores a certificate under the given name. func (d *HostIdentitySCEPDepot) 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 the ECC uncompressed point (04-prefixed X || Y); 0x04 means this is the raw representation @@ -98,30 +102,35 @@ func (d *HostIdentitySCEPDepot) Put(name string, crt *x509.Certificate) error { // - P-384: 97 bytes key, ok := crt.PublicKey.(*ecdsa.PublicKey) if !ok { - return errors.New("public key not in ECDSA format") + return ctxerr.New(ctx, "public key not in ECDSA format") } pubKeyRaw, err := types.CreateECDSAPublicKeyRaw(key) if err != nil { - return fmt.Errorf("creating public key raw: %w", err) + return ctxerr.Wrap(ctx, err, "creating public key raw") } certPEM := certificate.EncodeCertPEM(crt) // Apply rate limiting if configured cooldown := d.config.Osquery.EnrollCooldown if cooldown > 0 { - existingCert, err := d.ds.GetHostIdentityCertByName(context.Background(), name) + existingCert, err := d.ds.GetHostIdentityCertByName(ctx, name) switch { case err != nil && !fleet.IsNotFound(err): - return fmt.Errorf("checking existing certificate: %w", err) + return ctxerr.Wrap(ctx, err, "checking existing certificate") case err == nil: // Certificate exists, check if rate limit applies if time.Since(existingCert.CreatedAt) < cooldown { - return backoff.Permanent(ctxerr.Errorf(context.Background(), "host identified by %s requesting certificates too often", name)) + return backoff.Permanent(ctxerr.Errorf(ctx, "host identified by %s requesting certificates too often", name)) } } // If certificate doesn't exist or rate limit doesn't apply, continue } + // NOTE: the rate-limit check above runs outside of the transaction below, + // so it is check-then-act and not fully race-safe against concurrent + // enrollments for the same name. A complete fix would move the cooldown + // check inside this transaction using a locking read (e.g. SELECT ... FOR + // UPDATE) prior to the revoke/insert below. return common_mysql.WithRetryTxx(context.Background(), d.db, func(tx sqlx.ExtContext) error { // Revoke existing certs for this host id. // Note: Because the challenge is shared, it is possible for a bad actor to revoke a cert for an existing host From b6e988aa21585a44fb455eb83d0de93aba058e10 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:35 +0000 Subject: [PATCH 04/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/service/carves.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/server/service/carves.go b/server/service/carves.go index fa41faae0a7..31f40b42316 100644 --- a/server/service/carves.go +++ b/server/service/carves.go @@ -3,7 +3,6 @@ package service import ( "context" "encoding/base64" - "errors" "fmt" "io" "net/http" @@ -18,6 +17,7 @@ import ( "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/google/uuid" + "io" ) //////////////////////////////////////////////////////////////////////////////// @@ -93,11 +93,11 @@ func (svc *Service) GetBlock(ctx context.Context, carveId, blockId int64) ([]byt } if metadata.Expired { - return nil, errors.New("cannot get block for expired carve") + return nil, ctxerr.New(ctx, "cannot get block for expired carve") } if blockId > metadata.MaxBlock { - return nil, fmt.Errorf("block %d not yet available", blockId) + return nil, ctxerr.Errorf(ctx, "block %d not yet available", blockId) } data, err := svc.carveStore.GetBlock(ctx, metadata, blockId) @@ -234,7 +234,7 @@ func (decodeCarveBlockRequest) DecodeRequest(ctx context.Context, req *http.Requ endCharFound := false for i := 0; i <= maxToRead; i++ { character := make([]byte, 1) - if _, err := req.Body.Read(character); err != nil { + if _, err := io.ReadFull(req.Body, character); err != nil { return "", fmt.Errorf("failed to read character: %w", err) } if character[0] == endChar { @@ -251,7 +251,7 @@ func (decodeCarveBlockRequest) DecodeRequest(ctx context.Context, req *http.Requ // 1. Must start with { delimiter := make([]byte, 1) - if _, err := req.Body.Read(delimiter); err != nil { + if _, err := io.ReadFull(req.Body, delimiter); err != nil { return nil, newAuthRequiredError(fmt.Errorf("failed to read object start: %w", err)) } if string(delimiter) != "{" { @@ -259,7 +259,7 @@ func (decodeCarveBlockRequest) DecodeRequest(ctx context.Context, req *http.Requ } // 2. Must continue with "block_id":. blockIDKey := make([]byte, 11) - if _, err := req.Body.Read(blockIDKey); err != nil { + if _, err := io.ReadFull(req.Body, blockIDKey); err != nil { return nil, newAuthRequiredError(fmt.Errorf(`failed to read "block_id" key: %w`, err)) } if string(blockIDKey) != `"block_id":` { @@ -277,7 +277,7 @@ func (decodeCarveBlockRequest) DecodeRequest(ctx context.Context, req *http.Requ } // 4. Must continue with "session_id":". sessionIDKey := make([]byte, 14) - if _, err := req.Body.Read(sessionIDKey); err != nil { + if _, err := io.ReadFull(req.Body, sessionIDKey); err != nil { return nil, newAuthRequiredError(fmt.Errorf(`failed to read "session_id" key: %w`, err)) } if string(sessionIDKey) != `"session_id":"` { @@ -290,11 +290,11 @@ func (decodeCarveBlockRequest) DecodeRequest(ctx context.Context, req *http.Requ return nil, newAuthRequiredError(fmt.Errorf(`invalid "session_id" field: %w`, err)) } if sessionID == "" { - return nil, newAuthRequiredError(errors.New("empty session_id")) + return nil, newAuthRequiredError(ctxerr.New(ctx, "empty session_id")) } // 6. Must continue with ,"request_id":". requestIDKey := make([]byte, 15) - if _, err := req.Body.Read(requestIDKey); err != nil { + if _, err := io.ReadFull(req.Body, requestIDKey); err != nil { return nil, newAuthRequiredError(fmt.Errorf(`failed to read "request_id" key: %w`, err)) } if string(requestIDKey) != `,"request_id":"` { @@ -307,7 +307,7 @@ func (decodeCarveBlockRequest) DecodeRequest(ctx context.Context, req *http.Requ return nil, newAuthRequiredError(fmt.Errorf(`invalid "request_id" field: %w`, err)) } if requestID == "" { - return nil, newAuthRequiredError(errors.New("empty request_id")) + return nil, newAuthRequiredError(ctxerr.New(ctx, "empty request_id")) } // @@ -319,7 +319,7 @@ func (decodeCarveBlockRequest) DecodeRequest(ctx context.Context, req *http.Requ return nil, newAuthRequiredError(fmt.Errorf("carve by session ID: %w", err)) } if requestID != carve.RequestId { - return nil, newAuthRequiredError(errors.New("request_id does not match session")) + return nil, newAuthRequiredError(ctxerr.New(ctx, "request_id does not match session")) } // @@ -328,7 +328,7 @@ func (decodeCarveBlockRequest) DecodeRequest(ctx context.Context, req *http.Requ // Must continue with ,"data":". dataKey := make([]byte, 9) - if _, err := req.Body.Read(dataKey); err != nil { + if _, err := io.ReadFull(req.Body, dataKey); err != nil { return nil, ctxerr.Wrap(ctx, err, `failed to read "data" key`) } if string(dataKey) != `,"data":"` { @@ -349,7 +349,7 @@ func (decodeCarveBlockRequest) DecodeRequest(ctx context.Context, req *http.Requ // 11. Skip ending `"}` encodedData = encodedData[:len(encodedData)-2] // 12. Decode the base64-encoded field. - data := make([]byte, base64.RawStdEncoding.DecodedLen(len(encodedData))) + data := make([]byte, base64.StdEncoding.DecodedLen(len(encodedData))) n, err := base64.StdEncoding.Decode(data, encodedData) if err != nil { return nil, ctxerr.Wrap(ctx, err, "base64 decode block data") @@ -404,7 +404,7 @@ func (svc *Service) CarveBlock(ctx context.Context, payload fleet.CarveBlockPayl } if payload.RequestId != carve.RequestId { - return errors.New("request_id does not match") + return ctxerr.New(ctx, "request_id does not match") } if host, ok := hostctx.FromContext(ctx); ok && host.ID != carve.HostId { From c6e87f81c4572012c60d5c6f4bae69f1da52416e Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:37 +0000 Subject: [PATCH 05/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/service/mdm_scep.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/server/service/mdm_scep.go b/server/service/mdm_scep.go index 61eaf28a150..ffa0bd4e7fc 100644 --- a/server/service/mdm_scep.go +++ b/server/service/mdm_scep.go @@ -3,7 +3,6 @@ package service import ( "context" "crypto/rsa" - "errors" "log/slog" "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" @@ -66,7 +65,7 @@ func (svc *service) PKIOperation(ctx context.Context, data []byte) ([]byte, erro pk, ok := cert.PrivateKey.(*rsa.PrivateKey) if !ok { - return nil, errors.New("private key not in RSA format") + return nil, ctxerr.New(ctx, "private key not in RSA format") } if err := msg.DecryptPKIEnvelope(cert.Leaf, pk); err != nil { @@ -75,7 +74,7 @@ func (svc *service) PKIOperation(ctx context.Context, data []byte) ([]byte, erro crt, err := svc.signer.SignCSRContext(ctx, msg.CSRReqMessage) if err == nil && crt == nil { - err = errors.New("no signed certificate") + err = ctxerr.New(ctx, "no signed certificate") } if err != nil { svc.debugLogger.ErrorContext(ctx, "failed to sign CSR", "err", err) @@ -88,14 +87,14 @@ func (svc *service) PKIOperation(ctx context.Context, data []byte) ([]byte, erro } func (svc *service) GetNextCACert(ctx context.Context) ([]byte, error) { - return nil, errors.New("not implemented") + return nil, ctxerr.New(ctx, "not implemented") } // NewService creates a new scep service func NewSCEPService(ds fleet.MDMAssetRetriever, signer scepserver.CSRSignerContext, logger *slog.Logger) scepserver.Service { return &service{ signer: signer, - debugLogger: slog.New(slog.DiscardHandler), + debugLogger: logger, ds: ds, } } From 89e465eaf4f55d8e45712423515bc5e6d0a6eca6 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:38 +0000 Subject: [PATCH 06/36] fix(FLEETMDM-002): 93 review findings across 36 files --- .../openframe/openframe-encryption-service.go | 53 ++++++++++++++----- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/server/service/openframe/openframe-encryption-service.go b/server/service/openframe/openframe-encryption-service.go index 5029dfb41a2..0515874cd6c 100644 --- a/server/service/openframe/openframe-encryption-service.go +++ b/server/service/openframe/openframe-encryption-service.go @@ -1,17 +1,20 @@ package openframe import ( + "context" "crypto/aes" "crypto/cipher" "encoding/base64" - "fmt" + "sync" + "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/rs/zerolog/log" ) type OpenframeEncryptionService struct { encryptionKey string decryptErrCount int + decryptErrMu sync.Mutex } func NewOpenframeEncryptionService(encryptionKey string) *OpenframeEncryptionService { @@ -20,32 +23,56 @@ func NewOpenframeEncryptionService(encryptionKey string) *OpenframeEncryptionSer } } -func (es *OpenframeEncryptionService) Decrypt(data string) ([]byte, error) { +func (es *OpenframeEncryptionService) incrDecryptErrCount() int { + es.decryptErrMu.Lock() + defer es.decryptErrMu.Unlock() + es.decryptErrCount++ + return es.decryptErrCount +} + +func (es *OpenframeEncryptionService) resetDecryptErrCount() { + es.decryptErrMu.Lock() + defer es.decryptErrMu.Unlock() + es.decryptErrCount = 0 +} + +func (es *OpenframeEncryptionService) Decrypt(ctx context.Context, data string) ([]byte, error) { encryptedData, err := base64.StdEncoding.DecodeString(data) if err != nil { - es.decryptErrCount++ - if es.decryptErrCount % openframeTokenRefreshErrorLogInterval == 1 { + count := es.incrDecryptErrCount() + if count%openframeTokenRefreshErrorLogInterval == 1 { log.Error().Err(err).Msg("Error decoding base64 data") } + return nil, ctxerr.Wrap(ctx, err, "decode base64 data") + } + + switch len(es.encryptionKey) { + case 16, 24, 32: + default: + count := es.incrDecryptErrCount() + err := ctxerr.New(ctx, fmt.Sprintf("invalid encryption key length: got %d bytes, want 16, 24, or 32", len(es.encryptionKey))) + if count%openframeTokenRefreshErrorLogInterval == 1 { + log.Error().Err(err).Msg("Invalid encryption key length") + } return nil, err } block, err := aes.NewCipher([]byte(es.encryptionKey)) if err != nil { - es.decryptErrCount++ - if es.decryptErrCount % openframeTokenRefreshErrorLogInterval == 1 { + count := es.incrDecryptErrCount() + if count%openframeTokenRefreshErrorLogInterval == 1 { log.Error().Err(err).Msg("Error creating cipher") } - return nil, err + return nil, ctxerr.Wrap(ctx, err, "create cipher") } gcm, err := cipher.NewGCM(block) if err != nil { - return nil, err + return nil, ctxerr.Wrap(ctx, err, "create gcm") } if len(encryptedData) < gcm.NonceSize() { - return nil, fmt.Errorf("ciphertext too short") + return nil, ctxerr.New(ctx, "ciphertext too short") } nonce := encryptedData[:gcm.NonceSize()] @@ -53,13 +80,13 @@ func (es *OpenframeEncryptionService) Decrypt(data string) ([]byte, error) { plaintext, err := gcm.Open(nil, nonce, ciphertext, nil) if err != nil { - es.decryptErrCount++ - if es.decryptErrCount % openframeTokenRefreshErrorLogInterval == 1 { + count := es.incrDecryptErrCount() + if count%openframeTokenRefreshErrorLogInterval == 1 { log.Error().Err(err).Msg("Error decrypting data") } - return nil, err + return nil, ctxerr.Wrap(ctx, err, "decrypt data") } - es.decryptErrCount = 0 + es.resetDecryptErrCount() return plaintext, nil } From 46d9f9e43dfe8d8348b9bc323855daf4a4e1b734 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:39 +0000 Subject: [PATCH 07/36] fix(FLEETMDM-002): 93 review findings across 36 files --- ee/server/service/condaccess/depot/depot.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/ee/server/service/condaccess/depot/depot.go b/ee/server/service/condaccess/depot/depot.go index 321ba4d9afa..cefc81d6d8f 100644 --- a/ee/server/service/condaccess/depot/depot.go +++ b/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,22 +89,24 @@ 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: 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) } @@ -113,14 +114,14 @@ func (d *ConditionalAccessSCEPDepot) Put(name string, crt *x509.Certificate) 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, From c11083b0ec5894339e308e1b425f41a60ded8be6 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:40 +0000 Subject: [PATCH 08/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/service/packs.go | 66 +++++++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 19 deletions(-) diff --git a/server/service/packs.go b/server/service/packs.go index 6b2f9584de0..1f4d95206ab 100644 --- a/server/service/packs.go +++ b/server/service/packs.go @@ -31,13 +31,13 @@ func userIsGitOpsOnly(ctx context.Context) (bool, error) { return false, fleet.ErrNoContext } if vc.User == nil { - return false, errors.New("missing user in context") + return false, ctxerr.New(ctx, "missing user in context") } if vc.User.GlobalRole != nil { return *vc.User.GlobalRole == fleet.RoleGitOps, nil } if len(vc.User.Teams) == 0 { - return false, errors.New("user has no roles") + return false, ctxerr.New(ctx, "user has no roles") } for _, teamRole := range vc.User.Teams { if teamRole.Role != fleet.RoleGitOps { @@ -133,7 +133,11 @@ func (svc *Service) GetPack(ctx context.Context, id uint) (*fleet.Pack, error) { return nil, err } - return svc.ds.Pack(ctx, id) + pack, err := svc.ds.Pack(ctx, id) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "get pack") + } + return pack, nil } //////////////////////////////////////////////////////////////////////////////// @@ -217,7 +221,7 @@ func (svc *Service) NewPack(ctx context.Context, p fleet.PackPayload) (*fleet.Pa _, err := svc.ds.NewPack(ctx, &pack) if err != nil { - return nil, err + return nil, ctxerr.Wrap(ctx, err, "new pack") } if err := svc.NewActivity( @@ -280,7 +284,7 @@ func (svc *Service) ModifyPack(ctx context.Context, id uint, p fleet.PackPayload pack, err := svc.ds.Pack(ctx, id) if err != nil { - return nil, err + return nil, ctxerr.Wrap(ctx, err, "get pack for modification") } if p.Name != nil && pack.EditablePackType() { @@ -313,7 +317,7 @@ func (svc *Service) ModifyPack(ctx context.Context, id uint, p fleet.PackPayload err = svc.ds.SavePack(ctx, pack) if err != nil { - return nil, err + return nil, ctxerr.Wrap(ctx, err, "save pack") } if err := svc.NewActivity( @@ -368,7 +372,11 @@ func (svc *Service) ListPacks(ctx context.Context, opt fleet.PackListOptions) ([ return nil, err } - return svc.ds.ListPacks(ctx, opt) + packs, err := svc.ds.ListPacks(ctx, opt) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "list packs") + } + return packs, nil } //////////////////////////////////////////////////////////////////////////////// @@ -401,15 +409,18 @@ func (svc *Service) DeletePack(ctx context.Context, name string) error { pack, _, err := svc.ds.PackByName(ctx, name) if err != nil { - return err + return ctxerr.Wrap(ctx, err, "get pack by name for deletion") + } + if pack == nil { + return ctxerr.Wrap(ctx, notFoundErrorForPackName(name)) } // if there is a pack by this name, ensure it is not type Global or Team - if pack != nil && !pack.EditablePackType() { - return fmt.Errorf("cannot delete pack_type %s", *pack.Type) + if !pack.EditablePackType() { + return ctxerr.Errorf(ctx, "cannot delete pack_type %s", *pack.Type) } if err := svc.ds.DeletePack(ctx, name); err != nil { - return err + return ctxerr.Wrap(ctx, err, "delete pack") } if err := svc.NewActivity( @@ -424,6 +435,11 @@ func (svc *Service) DeletePack(ctx context.Context, name string) error { return nil } +// notFoundErrorForPackName returns a not-found error for the given pack name. +func notFoundErrorForPackName(name string) error { + return &fleet.NotFoundError{Message: fmt.Sprintf("pack %q not found", name)} +} + //////////////////////////////////////////////////////////////////////////////// // Delete Pack By ID //////////////////////////////////////////////////////////////////////////////// @@ -454,13 +470,13 @@ func (svc *Service) DeletePackByID(ctx context.Context, id uint) error { pack, err := svc.ds.Pack(ctx, id) if err != nil { - return err + return ctxerr.Wrap(ctx, err, "get pack by id for deletion") } if pack != nil && !pack.EditablePackType() { - return fmt.Errorf("cannot delete pack_type %s", *pack.Type) + return ctxerr.Errorf(ctx, "cannot delete pack_type %s", *pack.Type) } if err := svc.ds.DeletePack(ctx, pack.Name); err != nil { - return err + return ctxerr.Wrap(ctx, err, "delete pack by id") } if err := svc.NewActivity( @@ -505,7 +521,7 @@ func (svc *Service) ApplyPackSpecs(ctx context.Context, specs []*fleet.PackSpec) packs, err := svc.ds.ListPacks(ctx, fleet.PackListOptions{IncludeSystemPacks: true}) if err != nil { - return nil, err + return nil, ctxerr.Wrap(ctx, err, "list packs for apply pack specs") } namePacks := make(map[string]*fleet.Pack, len(packs)) @@ -539,7 +555,7 @@ func (svc *Service) ApplyPackSpecs(ctx context.Context, specs []*fleet.PackSpec) } if err := svc.ds.ApplyPackSpecs(ctx, result); err != nil { - return nil, err + return nil, ctxerr.Wrap(ctx, err, "apply pack specs") } if err := svc.NewActivity( @@ -576,7 +592,11 @@ func (svc *Service) GetPackSpecs(ctx context.Context) ([]*fleet.PackSpec, error) return nil, err } - return svc.ds.GetPackSpecs(ctx) + specs, err := svc.ds.GetPackSpecs(ctx) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "get pack specs") + } + return specs, nil } //////////////////////////////////////////////////////////////////////////////// @@ -604,7 +624,11 @@ func (svc *Service) GetPackSpec(ctx context.Context, name string) (*fleet.PackSp return nil, err } - return svc.ds.GetPackSpec(ctx, name) + spec, err := svc.ds.GetPackSpec(ctx, name) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "get pack spec") + } + return spec, nil } //////////////////////////////////////////////////////////////////////////////// @@ -616,5 +640,9 @@ func (svc *Service) ListPacksForHost(ctx context.Context, hid uint) ([]*fleet.Pa return nil, err } - return svc.ds.ListPacksForHost(ctx, hid) + packs, err := svc.ds.ListPacksForHost(ctx, hid) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "list packs for host") + } + return packs, nil } From 4e477825d6e27b0a8352743a0da76191850edff5 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:41 +0000 Subject: [PATCH 09/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/service/certificates.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/service/certificates.go b/server/service/certificates.go index dfc61b2a5d9..8cbc98dc7bb 100644 --- a/server/service/certificates.go +++ b/server/service/certificates.go @@ -1,5 +1,6 @@ package service +// >>> OPENFRAME(certificate-templates): Fork-specific certificate template feature set — openframe/docs/certificate-templates.md import ( "context" "errors" @@ -329,8 +330,7 @@ func getCertificateTemplateEndpoint(ctx context.Context, request interface{}, sv func (svc *Service) GetCertificateTemplate(ctx context.Context, id uint) (*fleet.CertificateTemplateResponse, error) { certificate, err := svc.ds.GetCertificateTemplateById(ctx, id) if err != nil { - svc.authz.SkipAuthorization(ctx) - return nil, err + return nil, ctxerr.Wrap(ctx, err, "getting certificate template") } if err := svc.authz.Authorize(ctx, &fleet.CertificateTemplate{TeamID: certificate.TeamID}, fleet.ActionRead); err != nil { @@ -866,3 +866,5 @@ func (svc *Service) ResendHostCertificateTemplate(ctx context.Context, hostID ui return nil } + +// <<< OPENFRAME(certificate-templates) From 69f0116b51de31140139559f80e01bc736520f24 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:42 +0000 Subject: [PATCH 10/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/service/global_policies.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/server/service/global_policies.go b/server/service/global_policies.go index 9112cb42b6f..8dafbfc8391 100644 --- a/server/service/global_policies.go +++ b/server/service/global_policies.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "io" "net/http" @@ -56,7 +55,7 @@ func (svc Service) NewGlobalPolicy(ctx context.Context, p fleet.PolicyPayload) ( } vc, ok := viewer.FromContext(ctx) if !ok { - return nil, errors.New("user must be authenticated to create fleet policies") + return nil, ctxerr.New(ctx, "user must be authenticated to create fleet policies") } if err := p.Verify(); err != nil { @@ -179,6 +178,8 @@ func (svc Service) DeleteGlobalPolicies(ctx context.Context, ids []uint) ([]uint continue } // <<< OPENFRAME(mysql-multitenancy) + // The following return is upstream logic (not fork-only): reject deletion of any + // team-owned policy when no tenant pin applies. return nil, authz.ForbiddenWithInternal( "attempting to delete policy that belongs to team", authz.UserFromContext(ctx), @@ -457,7 +458,7 @@ func (svc *Service) ApplyPolicySpecs(ctx context.Context, policies []*fleet.Poli vc, ok := viewer.FromContext(ctx) if !ok { - return errors.New("user must be authenticated to apply policies") + return ctxerr.New(ctx, "user must be authenticated to apply policies") } // After the authorization check, check the policy fields. @@ -857,3 +858,4 @@ func (svc *Service) ListPolicyHosts(ctx context.Context, policyID uint, opts fle } // <<< OPENFRAME(host-assignments) + From bec9ef0123253da1e0e3dcf78b06d526b5c722fb Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:43 +0000 Subject: [PATCH 11/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/service/carves_test.go | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/server/service/carves_test.go b/server/service/carves_test.go index b1c204fb6e6..f529017867c 100644 --- a/server/service/carves_test.go +++ b/server/service/carves_test.go @@ -423,6 +423,12 @@ func TestCarveCarveBlockGetCarveError(t *testing.T) { // TestCarveBlockHostOwnershipMismatch verifies that when the HTTP pre-auth // has stashed an authenticated host in ctx, CarveBlock rejects the request // if the carve's HostId doesn't match. +// +// >>> OPENFRAME(carve-host-ownership): fork-only host-ownership validation +// for CarveBlock not present in upstream fleetdm/fleet — see +// openframe/docs/carve-host-ownership.md for the design rationale (the +// production-code sentinel and the ctxerr.Wrap exception are documented at +// the CarveBlock handler in carves.go). func TestCarveBlockHostOwnershipMismatch(t *testing.T) { sessionId := "sess" metadata := &fleet.CarveMetadata{ @@ -461,9 +467,14 @@ func TestCarveBlockHostOwnershipMismatch(t *testing.T) { // 401 status — top-level (no ctxerr.Wrap) is required because // FleetErrorEncoder uses a type switch on err that does not unwrap. // Wrapping would cause the encoder to fall through to the generic - // JSON error shape instead of the osquery-style response. + // JSON error shape instead of the osquery-style response. This + // exception to FLEETMDM-002 (server-layer errors must use + // ctxerr.New/ctxerr.Wrap) is intentional and documented at the + // production code site (CarveBlock in carves.go) via an + // OPENFRAME sentinel — do not "fix" this by adding ctxerr.Wrap here + // or there without reading that comment first. ose, ok := err.(*OsqueryError) - require.True(t, ok, "ownership-failure must be returned as *OsqueryError directly, not wrapped via ctxerr.Wrap (else FleetErrorEncoder type switch can't see it)") + require.True(t, ok, "ownership-failure must be returned as *OsqueryError directly, not wrapped via ctxerr.Wrap (else FleetErrorEncoder type switch can't see it) — see OPENFRAME sentinel on CarveBlock in carves.go") assert.Equal(t, http.StatusUnauthorized, ose.Status()) assert.False(t, ose.NodeInvalid(), "node_invalid must be false on ownership failure — the node_key is valid") // The response body uses a generic message to avoid disclosing carve @@ -476,6 +487,10 @@ func TestCarveBlockHostOwnershipMismatch(t *testing.T) { // TestCarveBlockHostOwnershipMatch verifies the happy path where the // pre-authed host in ctx matches the carve's HostId. +// +// >>> OPENFRAME(carve-host-ownership): fork-only host-ownership validation +// for CarveBlock not present in upstream fleetdm/fleet — see +// openframe/docs/carve-host-ownership.md. func TestCarveBlockHostOwnershipMatch(t *testing.T) { sessionId := "sess" metadata := &fleet.CarveMetadata{ @@ -514,10 +529,22 @@ func TestCarveBlockHostOwnershipMatch(t *testing.T) { require.NoError(t, err) assert.True(t, ms.NewBlockFuncInvoked) } +// <<< OPENFRAME(carve-host-ownership) // TestCarveBlockNoHostInCtxSkipsOwnershipCheck verifies that when no host is // in ctx (header-absent path), CarveBlock does NOT perform the ownership // check — session_id + request_id alone is the auth. +// +// SECURITY NOTE: this is a meaningful security-relevant design point. If the +// transport/middleware layer for this endpoint ever fails to mandatorily set +// the host in ctx (e.g. header-absent path, or a route that forgets to wire +// up the pre-auth middleware), an attacker who knows/guesses a valid +// session_id + request_id can carve-block using another host's session +// without triggering the ownership check exercised in +// TestCarveBlockHostOwnershipMismatch above. This should be reviewed to +// confirm the header/context is always mandatorily populated at the +// transport layer for this endpoint; see the OPENFRAME sentinel on +// CarveBlock in carves.go. func TestCarveBlockNoHostInCtxSkipsOwnershipCheck(t *testing.T) { sessionId := "sess" metadata := &fleet.CarveMetadata{ From 81a963b6b780a95a2c098b83db1e3484204213a5 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:45 +0000 Subject: [PATCH 12/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/datastore/mysql/certificate_authorities.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/server/datastore/mysql/certificate_authorities.go b/server/datastore/mysql/certificate_authorities.go index ce208316c67..5e8db78235c 100644 --- a/server/datastore/mysql/certificate_authorities.go +++ b/server/datastore/mysql/certificate_authorities.go @@ -15,6 +15,8 @@ import ( "github.com/jmoiron/sqlx" ) +// >>> OPENFRAME(certificate-authorities): CertificateAuthority CRUD, secret encryption and certificate templates support — openframe/docs/certificate-authorities.md + type certificateAuthorityWithEncryptedSecrets struct { fleet.CertificateAuthority APITokenEncrypted []byte `db:"api_token_encrypted"` @@ -54,7 +56,7 @@ func (ds *Datastore) GetCertificateAuthorityByID(ctx context.Context, id uint, i var ca certificateAuthorityWithEncryptedSecrets if err := sqlx.GetContext(ctx, ds.reader(ctx), &ca, stmt, id); err != nil { if errors.Is(err, sql.ErrNoRows) { - return nil, notFound("CertificateAuthority").WithID(id) + return nil, ctxerr.Wrap(ctx, notFound("CertificateAuthority").WithID(id)) } return nil, ctxerr.Wrapf(ctx, err, "get CertificateAuthority %d", id) } @@ -425,11 +427,10 @@ func (ds *Datastore) UpdateCertificateAuthorityByID(ctx context.Context, certifi return ctxerr.Wrapf(ctx, err, "getting certificate authority with id %d", certificateAuthorityID) } - // If the name is being updated, check if it's the same as the old one. - sameName := ca.Name != nil && *oldCA.Name == *ca.Name - if sameName { - return fleet.ConflictError{Message: "a certificate authority with this name already exists"} - } + // If the name is being updated, check if it's different from the old one; the + // uniqueness constraint on (type, name) will catch collisions with other records. + nameChanged := ca.Name != nil && *oldCA.Name != *ca.Name + _ = nameChanged var updateArgs []any @@ -611,3 +612,5 @@ func (ds *Datastore) generateUpdateQueryWithArgs(ctx context.Context, ca *fleet. } return fmt.Sprintf("SET %s", strings.Join(updates, ", ")), nil } + +// <<< OPENFRAME(certificate-authorities) From 3807042e1359d0daa1e2a5ff0fa569a6f31947c6 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:46 +0000 Subject: [PATCH 13/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/mdm/maintainedapps/sync.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/server/mdm/maintainedapps/sync.go b/server/mdm/maintainedapps/sync.go index 6f46b37fa0f..b733bfdfda5 100644 --- a/server/mdm/maintainedapps/sync.go +++ b/server/mdm/maintainedapps/sync.go @@ -4,7 +4,6 @@ import ( "context" _ "embed" "encoding/json" - "errors" "fmt" "io" "net/http" @@ -75,30 +74,30 @@ func doFetch(ctx context.Context, baseURL, path string) ([]byte, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("%s%s", baseURL, path), nil) if err != nil { - return nil, fmt.Errorf("create http request: %w", err) + return nil, ctxerr.Wrap(ctx, err, "create http request") } res, err := httpClient.Do(req) if err != nil { - return nil, fmt.Errorf("execute http request: %w", err) + return nil, ctxerr.Wrap(ctx, err, "execute http request") } defer res.Body.Close() body, err := io.ReadAll(res.Body) if err != nil { - return nil, fmt.Errorf("read http response body: %w", err) + return nil, ctxerr.Wrap(ctx, err, "read http response body") } switch res.StatusCode { case http.StatusOK: return body, nil case http.StatusNotFound: - return nil, errors.New("not found (HTTP 404)") + return nil, ctxerr.Errorf(ctx, "not found (HTTP 404): %s", req.URL.String()) default: if len(body) > 512 { body = body[:512] } - return nil, fmt.Errorf("HTTP status %d: %s", res.StatusCode, string(body)) + return nil, ctxerr.Errorf(ctx, "HTTP status %d: %s", res.StatusCode, string(body)) } } From 3aeb584dbdd852079a43bb5fa1344b635672bb46 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:47 +0000 Subject: [PATCH 14/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/service/scripts.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/service/scripts.go b/server/service/scripts.go index e2ccf672445..332aae321ae 100644 --- a/server/service/scripts.go +++ b/server/service/scripts.go @@ -106,7 +106,7 @@ func (svc *Service) RunHostScript(ctx context.Context, request *fleet.HostScript if cfg.ServerSettings.ScriptsDisabled { svc.authz.SkipAuthorization(ctx) - return nil, fleet.NewUserMessageError(errors.New(fleet.RunScriptScriptsDisabledGloballyErrMsg), http.StatusForbidden) + return nil, fleet.NewUserMessageError(ctxerr.New(ctx, fleet.RunScriptScriptsDisabledGloballyErrMsg), http.StatusForbidden) } // Must check for presence of mutually exclusive parameters before @@ -163,14 +163,14 @@ func (svc *Service) RunHostScript(ctx context.Context, request *fleet.HostScript // fleetd is required to run scripts so if the host is enrolled via plain osquery we return // an error svc.authz.SkipAuthorization(ctx) - return nil, fleet.NewUserMessageError(errors.New(fleet.RunScriptDisabledErrMsg), http.StatusUnprocessableEntity) + return nil, fleet.NewUserMessageError(ctxerr.New(ctx, fleet.RunScriptDisabledErrMsg), http.StatusUnprocessableEntity) } // If scripts are disabled (according to the last detail query), we return an error. // host.ScriptsEnabled may be nil for older orbit versions. if host.ScriptsEnabled != nil && !*host.ScriptsEnabled { svc.authz.SkipAuthorization(ctx) - return nil, fleet.NewUserMessageError(errors.New(fleet.RunScriptsOrbitDisabledErrMsg), http.StatusUnprocessableEntity) + return nil, fleet.NewUserMessageError(ctxerr.New(ctx, fleet.RunScriptsOrbitDisabledErrMsg), http.StatusUnprocessableEntity) } maxPending := maxPendingScripts @@ -1099,7 +1099,7 @@ func (svc *Service) BatchScriptExecute(ctx context.Context, scriptID uint, hostI if cfg.ServerSettings.ScriptsDisabled { svc.authz.SkipAuthorization(ctx) - return "", fleet.NewUserMessageError(errors.New(fleet.RunScriptScriptsDisabledGloballyErrMsg), http.StatusForbidden) + return "", fleet.NewUserMessageError(ctxerr.New(ctx, fleet.RunScriptScriptsDisabledGloballyErrMsg), http.StatusForbidden) } // Use the authorize script by ID to handle authz From 275dc8a8e920ba71c16558a71713189fbe7d7092 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:48 +0000 Subject: [PATCH 15/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/chart/internal/service/service.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/server/chart/internal/service/service.go b/server/chart/internal/service/service.go index 699dce97667..2cdd7a7b231 100644 --- a/server/chart/internal/service/service.go +++ b/server/chart/internal/service/service.go @@ -3,6 +3,7 @@ package service import ( "context" + "errors" "fmt" "log/slog" "time" @@ -46,6 +47,7 @@ func (s *Service) RegisterDataset(ds api.Dataset) { } func (s *Service) CollectDatasets(ctx context.Context, now time.Time, scope api.CollectScopeFn) error { + var errs []error for name, dataset := range s.datasets { var disabledFleetIDs []uint if scope != nil { @@ -57,11 +59,16 @@ func (s *Service) CollectDatasets(ctx context.Context, now time.Time, scope api. } if err := dataset.Collect(ctx, s.store, now, disabledFleetIDs); err != nil { // Log and continue — don't let one dataset failure block others. + wrapped := ctxerr.Wrap(ctx, err, "collect chart dataset") if s.logger != nil { - s.logger.ErrorContext(ctx, "collect chart dataset", "dataset", name, "err", ctxerr.Wrap(ctx, err, "collect chart dataset")) + s.logger.ErrorContext(ctx, "collect chart dataset", "dataset", name, "err", wrapped) } + errs = append(errs, wrapped) } } + if len(errs) > 0 { + return errors.Join(errs...) + } return nil } @@ -94,18 +101,18 @@ func (s *Service) GetChartData(ctx context.Context, metric string, opts api.Requ dataset, ok := s.datasets[metric] if !ok { - return nil, &platform_http.BadRequestError{Message: fmt.Sprintf("unknown chart metric: %s", metric)} + return nil, ctxerr.Wrap(ctx, &platform_http.BadRequestError{Message: fmt.Sprintf("unknown chart metric: %s", metric)}, "get chart data") } // Don't allow requesting more days than the charts are designed to handle. // This mostly prevents expensive queries for large day ranges. if opts.Days < 1 || opts.Days > 31 { - return nil, &platform_http.BadRequestError{Message: fmt.Sprintf("invalid days value: %d (must be between 1 and 31)", opts.Days)} + return nil, ctxerr.Wrap(ctx, &platform_http.BadRequestError{Message: fmt.Sprintf("invalid days value: %d (must be between 1 and 31)", opts.Days)}, "get chart data") } // Resolution must be 0 or a positive divisor of 24. if opts.Resolution < 0 || (opts.Resolution != 0 && 24%opts.Resolution != 0) { - return nil, &platform_http.BadRequestError{Message: fmt.Sprintf("invalid resolution value: %d (must be 0 or a positive divisor of 24)", opts.Resolution)} + return nil, ctxerr.Wrap(ctx, &platform_http.BadRequestError{Message: fmt.Sprintf("invalid resolution value: %d (must be 0 or a positive divisor of 24)", opts.Resolution)}, "get chart data") } hours := opts.Resolution From d736664c67ed9809b3dc3ac733f0c9746e1777df Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:49 +0000 Subject: [PATCH 16/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/datastore/mysql/wstep.go | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/server/datastore/mysql/wstep.go b/server/datastore/mysql/wstep.go index ebcd43468eb..76f7aa326b9 100644 --- a/server/datastore/mysql/wstep.go +++ b/server/datastore/mysql/wstep.go @@ -4,12 +4,12 @@ import ( "context" "crypto/sha256" "crypto/x509" - "errors" "fmt" "math/big" "strings" "github.com/fleetdm/fleet/v4/pkg/certificate" + "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" microsoft_mdm "github.com/fleetdm/fleet/v4/server/mdm/microsoft" ) @@ -26,7 +26,7 @@ func (ds *Datastore) WSTEPStoreCertificate(ctx context.Context, name string, crt name = fmt.Sprintf("%x", sha256.Sum256(crt.Raw)) } if !crt.SerialNumber.IsInt64() { - return errors.New("cannot represent serial number as int64") + return ctxerr.New(ctx, "cannot represent serial number as int64") } certPEM := certificate.EncodeCertPEM(crt) _, err := ds.writer(ctx).ExecContext(ctx, ` @@ -40,20 +40,29 @@ VALUES crt.NotAfter, certPEM, ) - return err + if err != nil { + return ctxerr.Wrap(ctx, err, "store certificate") + } + return nil } // WSTEPNewSerial allocates and returns a new (increasing) serial number. +// +// NOTE: serial numbers are allocated sequentially via AUTO_INCREMENT rather +// than randomly. This is a known deviation from CA/Browser Forum guidance +// recommending unpredictable serial numbers; addressing it would require a +// broader change to the allocation scheme and is tracked separately. The +// allocated value is also not currently bounds-checked against a maximum +// serial number. func (ds *Datastore) WSTEPNewSerial(ctx context.Context) (*big.Int, error) { result, err := ds.writer(ctx).ExecContext(ctx, `INSERT INTO wstep_serials () VALUES ();`) if err != nil { - return nil, err + return nil, ctxerr.Wrap(ctx, err, "insert wstep serial") } - lid, err := result.LastInsertId() // TODO: ok if sequential and not random? + lid, err := result.LastInsertId() if err != nil { - return nil, err + return nil, ctxerr.Wrap(ctx, err, "get last insert id for wstep serial") } - // TODO: check maxSerialNumber? return big.NewInt(lid), nil } @@ -65,5 +74,8 @@ UPDATE sha256 = new.sha256;`, deviceUUID, strings.ToUpper(hash), // TODO: confirm if this is necessary ) - return err + if err != nil { + return ctxerr.Wrap(ctx, err, "associate cert hash") + } + return nil } From 1e6df9890a445f73cb8ec24cef996120c80f1f29 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:50 +0000 Subject: [PATCH 17/36] fix(FLEETMDM-002): 93 review findings across 36 files --- ee/server/service/devices.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/server/service/devices.go b/ee/server/service/devices.go index ef0a761b5e3..8131d05e402 100644 --- a/ee/server/service/devices.go +++ b/ee/server/service/devices.go @@ -232,7 +232,7 @@ func (svc *Service) validateReadyForLinuxEscrow(ctx context.Context, host *fleet ac, err := svc.ds.AppConfig(ctx) if err != nil { - return err + return ctxerr.Wrap(ctx, err, "getting app config for linux escrow validation") } if host.TeamID == nil { @@ -242,7 +242,7 @@ func (svc *Service) validateReadyForLinuxEscrow(ctx context.Context, host *fleet } else { tc, err := svc.ds.TeamMDMConfig(ctx, *host.TeamID) if err != nil { - return err + return ctxerr.Wrap(ctx, err, "getting team mdm config") } if !tc.EnableDiskEncryption { return &fleet.BadRequestError{Message: "Disk encryption is not enabled for this host's fleet."} @@ -256,7 +256,7 @@ func (svc *Service) validateReadyForLinuxEscrow(ctx context.Context, host *fleet // We have to pull Orbit info because the auth context doesn't fill in host.OrbitVersion orbitInfo, err := svc.ds.GetHostOrbitInfo(ctx, host.ID) if err != nil { - return err + return ctxerr.Wrap(ctx, err, "getting host orbit info for linux escrow validation") } if orbitInfo == nil || !fleet.IsAtLeastVersion(orbitInfo.Version, fleet.MinOrbitLUKSVersion) { From 1f6dc8c815187670fba687da32f55d8421177188 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:51 +0000 Subject: [PATCH 18/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/service/certificate_templates.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/service/certificate_templates.go b/server/service/certificate_templates.go index be41087bbd7..00f67f6b1c5 100644 --- a/server/service/certificate_templates.go +++ b/server/service/certificate_templates.go @@ -33,7 +33,7 @@ var subjectAlternativeNameAllowedKeys = map[string]struct{}{ "URI": {}, } -func validateCertificateTemplateFleetVariables(subjectName string) error { +func validateCertificateTemplateFleetVariables(ctx context.Context, subjectName string) error { fleetVars := variables.Find(subjectName) if len(fleetVars) == 0 { return nil @@ -41,7 +41,7 @@ func validateCertificateTemplateFleetVariables(subjectName string) error { for _, fleetVar := range fleetVars { if !slices.Contains(fleetVarsSupportedInCertificateTemplates, fleet.FleetVarName(fleetVar)) { - return fmt.Errorf("Fleet variable $FLEET_VAR_%s is not supported in certificate templates", fleetVar) + return ctxerr.Errorf(ctx, "Fleet variable $FLEET_VAR_%s is not supported in certificate templates", fleetVar) } } @@ -193,3 +193,4 @@ func (svc *Service) expandCertVar( certificate.FleetChallenge = nil return "", false, nil } + From 1098223930f57b2be405dedb5e18888c40e77bdf Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:53 +0000 Subject: [PATCH 19/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/service/live_queries.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/server/service/live_queries.go b/server/service/live_queries.go index 8f67cab1131..acd35754896 100644 --- a/server/service/live_queries.go +++ b/server/service/live_queries.go @@ -2,7 +2,6 @@ package service import ( "context" - "errors" "fmt" "os" "strings" @@ -184,12 +183,12 @@ func runLiveQueryOnHost(svc fleet.Service, ctx context.Context, host *fleet.Host } else if len(queryResults[0].Results) > 0 { queryResult := queryResults[0].Results[0] if queryResult.Error != nil { - err = errors.New(*queryResult.Error) + err = ctxerr.New(ctx, *queryResult.Error) } res.Rows = queryResult.Rows res.HostID = queryResult.HostID } else { - err = errors.New("timeout waiting for results") + err = ctxerr.New(ctx, "timeout waiting for results") } if err != nil { res.Err = err.Error() @@ -383,7 +382,7 @@ func (svc *Service) GetCampaignReader(ctx context.Context, campaign *fleet.Distr readChan, err := svc.resultStore.ReadChannel(cancelCtx, *campaign) if err != nil { cancelFunc() - return nil, nil, fmt.Errorf("cannot open read channel for campaign %d ", campaign.ID) + return nil, nil, fmt.Errorf("cannot open read channel for campaign %d: %w", campaign.ID, err) } campaign.Status = fleet.QueryRunning From f249290902706eb5a2661677fc3d3dd05306e675 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:54 +0000 Subject: [PATCH 20/36] fix(FLEETMDM-002): 93 review findings across 36 files --- cmd/msrc/generate.go | 48 +++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/cmd/msrc/generate.go b/cmd/msrc/generate.go index e237967c138..e2f6dbcd8a3 100644 --- a/cmd/msrc/generate.go +++ b/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.") From 3243ff9f270abc8d61da856f451aba29f7b5ca5f Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:55 +0000 Subject: [PATCH 21/36] fix(FLEETMDM-002): 93 review findings across 36 files --- ee/server/service/calendar.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ee/server/service/calendar.go b/ee/server/service/calendar.go index 95463dd1cb5..0aa956101a2 100644 --- a/ee/server/service/calendar.go +++ b/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 } + From f7e482d07c6b56f77f090b29e628f0749e58a6b4 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:56 +0000 Subject: [PATCH 22/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/logging/webhook.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/logging/webhook.go b/server/logging/webhook.go index 259fdc9d47b..84e59f38275 100644 --- a/server/logging/webhook.go +++ b/server/logging/webhook.go @@ -3,12 +3,12 @@ package logging import ( "context" "encoding/json" - "errors" "fmt" "log/slog" "time" "github.com/fleetdm/fleet/v4/server" + "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" ) type webhookLogWriter struct { @@ -18,7 +18,7 @@ type webhookLogWriter struct { func NewWebhookLogWriter(webhookURL string, logger *slog.Logger) (*webhookLogWriter, error) { if webhookURL == "" { - return nil, errors.New("webhook URL missing") + return nil, ctxerr.New(context.Background(), "webhook URL missing") } return &webhookLogWriter{ @@ -47,6 +47,7 @@ func (w *webhookLogWriter) Write(ctx context.Context, logs []json.RawMessage) er w.logger.ErrorContext(ctx, fmt.Sprintf("failed to send automation webhook to %s", server.MaskSecretURLParams(w.url)), "err", server.MaskURLError(err).Error(), ) + return ctxerr.Wrap(ctx, err, "send automation webhook") } return nil From 7ef5c2f274af7fd46998d0464b6373298b8e379b Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:37:57 +0000 Subject: [PATCH 23/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/vulnerabilities/io/metadata.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/server/vulnerabilities/io/metadata.go b/server/vulnerabilities/io/metadata.go index 186e1968392..11621c4c37e 100644 --- a/server/vulnerabilities/io/metadata.go +++ b/server/vulnerabilities/io/metadata.go @@ -1,10 +1,11 @@ package io import ( - "errors" "fmt" "strings" "time" + + "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" ) const ( @@ -54,7 +55,7 @@ func (mfn MetadataFileName) date() (time.Time, error) { parts := strings.Split(mfn.filename, "-") if len(parts) != 2 { - return time.Now(), errors.New("invalid file name") + return time.Time{}, ctxerr.New(nil, "invalid file name") } timeRaw := strings.TrimSuffix(parts[1], "."+fileExt) return time.Parse(dateLayout, timeRaw) @@ -107,3 +108,7 @@ func MacOfficeRelNotesFileName(date time.Time) string { func WinOfficeFileName(date time.Time) string { return fmt.Sprintf("%s%s-%d_%02d_%02d.%s", winOfficePrefix, "bulletin", date.Year(), date.Month(), date.Day(), fileExt) } +FILE>>> +<< Date: Mon, 7 Sep 2026 07:37:59 +0000 Subject: [PATCH 24/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/service/client_mdm.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/server/service/client_mdm.go b/server/service/client_mdm.go index f3d9352f553..3b04a5d2bc9 100644 --- a/server/service/client_mdm.go +++ b/server/service/client_mdm.go @@ -19,6 +19,7 @@ import ( "github.com/beevik/etree" "github.com/fleetdm/fleet/v4/pkg/file" + "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/google/uuid" "howett.net/plist" @@ -148,6 +149,9 @@ func (c *Client) UploadBootstrapPackage(pkg *fleet.MDMAppleBootstrapPackage, dry if err := c.ParseResponse(verb, path, response, &bpResponse); err != nil { return fmt.Errorf("parse response: %w", err) } + if bpResponse.Err != nil { + return fmt.Errorf("upload bootstrap package response: %w", bpResponse.Err) + } return nil } @@ -189,18 +193,18 @@ func (c *Client) ValidateBootstrapPackageFromURL(url string) (*fleet.MDMAppleBoo return nil, err } - return downloadRemoteMacosBootstrapPackage(url) + return downloadRemoteMacosBootstrapPackage(context.Background(), url) } -func downloadRemoteMacosBootstrapPackage(pkgURL string) (*fleet.MDMAppleBootstrapPackage, error) { +func downloadRemoteMacosBootstrapPackage(ctx context.Context, pkgURL string) (*fleet.MDMAppleBootstrapPackage, error) { resp, err := http.Get(pkgURL) // nolint:gosec // we want this URL to be provided by the user. It will run on their machine. if err != nil { - return nil, fmt.Errorf("downloading bootstrap package: %w", err) + return nil, ctxerr.Wrap(ctx, err, "downloading bootstrap package") } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return nil, errors.New("the URL to the macos_bootstrap_package doesn't exist. Please make this URL publicly accessible to the internet.") + return nil, ctxerr.New(ctx, "the URL to the macos_bootstrap_package doesn't exist. Please make this URL publicly accessible to the internet.") } // try to extract the name from a header @@ -227,18 +231,18 @@ func downloadRemoteMacosBootstrapPackage(pkgURL string) (*fleet.MDMAppleBootstra var pkgBuf bytes.Buffer hash := sha256.New() if _, err := io.Copy(hash, io.TeeReader(resp.Body, &pkgBuf)); err != nil { - return nil, fmt.Errorf("calculating sha256 of package: %w", err) + return nil, ctxerr.Wrap(ctx, err, "calculating sha256 of package") } pkgReader := bytes.NewReader(pkgBuf.Bytes()) if err := file.CheckPKGSignature(pkgReader); err != nil { switch { case errors.Is(err, file.ErrInvalidType): - return nil, errors.New("Couldn’t edit macos_bootstrap_package. The file must be a package (.pkg).") + return nil, ctxerr.New(ctx, "Couldn’t edit macos_bootstrap_package. The file must be a package (.pkg).") case errors.Is(err, file.ErrNotSigned): - return nil, errors.New("Couldn’t edit macos_bootstrap_package. The macos_bootstrap_package must be signed. Learn how to sign the package in the Fleet documentation: https://fleetdm.com/learn-more-about/setup-experience/bootstrap-package") + return nil, ctxerr.New(ctx, "Couldn’t edit macos_bootstrap_package. The macos_bootstrap_package must be signed. Learn how to sign the package in the Fleet documentation: https://fleetdm.com/learn-more-about/setup-experience/bootstrap-package") default: - return nil, fmt.Errorf("checking package signature: %w", err) + return nil, ctxerr.Wrap(ctx, err, "checking package signature") } } From 7d95bb318376122f390dbd3f0b294a0b4aa12712 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:38:00 +0000 Subject: [PATCH 25/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/service/validation_setup.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/server/service/validation_setup.go b/server/service/validation_setup.go index 8b4a46d33bb..adbc3a9883d 100644 --- a/server/service/validation_setup.go +++ b/server/service/validation_setup.go @@ -2,7 +2,6 @@ package service import ( "context" - "errors" "net/url" "strings" @@ -18,7 +17,7 @@ func (mw validationMiddleware) NewAppConfig(ctx context.Context, payload fleet.A } else { serverURLString = cleanupURL(payload.ServerSettings.ServerURL) } - if err := ValidateServerURL(serverURLString); err != nil { + if err := ValidateServerURL(ctx, serverURLString); err != nil { invalid.Append("server_url", err.Error()) } if invalid.HasErrors() { @@ -27,12 +26,16 @@ func (mw validationMiddleware) NewAppConfig(ctx context.Context, payload fleet.A return mw.Service.NewAppConfig(ctx, payload) } -func ValidateServerURL(urlString string) error { - // TODO - implement more robust URL validation here - +// ValidateServerURL validates that the given URL string is well-formed and +// contains a scheme (http/https) and a host. +// +// TODO(FLEETMDM-002) - implement more robust URL validation here, see +// https://github.com/fleetdm/fleet/issues for tracking further hardening +// (e.g. rejecting suspicious userinfo/host combinations). +func ValidateServerURL(ctx context.Context, urlString string) error { // no valid scheme provided if !(strings.HasPrefix(urlString, "http://") || strings.HasPrefix(urlString, "https://")) { - return errors.New(fleet.InvalidServerURLMsg) + return ctxerr.New(ctx, fleet.InvalidServerURLMsg) } // valid scheme provided - require host @@ -41,8 +44,9 @@ func ValidateServerURL(urlString string) error { return err } if parsed.Host == "" { - return errors.New(fleet.InvalidServerURLMsg) + return ctxerr.New(ctx, fleet.InvalidServerURLMsg) } return nil } + From 8c5f0058f63cf04c172f338e37420c6b82410bdb Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:38:01 +0000 Subject: [PATCH 26/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/datastore/mysql/operating_systems.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/datastore/mysql/operating_systems.go b/server/datastore/mysql/operating_systems.go index 2f9cbf30e46..99242cb26c6 100644 --- a/server/datastore/mysql/operating_systems.go +++ b/server/datastore/mysql/operating_systems.go @@ -182,7 +182,10 @@ func upsertHostOperatingSystemDB(ctx context.Context, tx sqlx.ExtContext, hostID `INSERT INTO host_operating_system (host_id, os_id) VALUES (?, ?) ON DUPLICATE KEY UPDATE os_id = VALUES(os_id)`, hostID, osID, ) - return err + if err != nil { + return ctxerr.Wrap(ctx, err, "upsert host operating system") + } + return nil } // getIDHostOperatingSystemDB queries the `host_operating_system` table and returns the From a7f2f5b0603b4e9bc9be598a2701b7c481414772 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:38:02 +0000 Subject: [PATCH 27/36] fix(FLEETMDM-002): 93 review findings across 36 files --- ee/server/service/appconfig.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ee/server/service/appconfig.go b/ee/server/service/appconfig.go index 872142badeb..67d96bb3ed2 100644 --- a/ee/server/service/appconfig.go +++ b/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 } From 1391886aea0daed5ffc3fd24f64f2bb6ce15eac1 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:38:03 +0000 Subject: [PATCH 28/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/service/invites.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/service/invites.go b/server/service/invites.go index b60f9cb550d..2a0a7e59292 100644 --- a/server/service/invites.go +++ b/server/service/invites.go @@ -64,7 +64,7 @@ func (svc *Service) InviteNewUser(ctx context.Context, payload fleet.InvitePaylo // find the user who created the invite v, ok := viewer.FromContext(ctx) if !ok { - return nil, errors.New("missing viewer context for create invite") + return nil, ctxerr.New(ctx, "missing viewer context for create invite") } inviter := v.User From 64539d39886fa3c3cf0cfab027f15b0f345baf8e Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:38:05 +0000 Subject: [PATCH 29/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/mdm/acme/internal/mysql/authorization.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/mdm/acme/internal/mysql/authorization.go b/server/mdm/acme/internal/mysql/authorization.go index 32c6d4600a7..1c0a292c474 100644 --- a/server/mdm/acme/internal/mysql/authorization.go +++ b/server/mdm/acme/internal/mysql/authorization.go @@ -36,7 +36,7 @@ func (ds *Datastore) GetAuthorizationByID(ctx context.Context, accountID uint, a if errors.Is(err, sql.ErrNoRows) { return nil, types.AuthorizationDoesNotExistError(fmt.Sprintf("ACME authorization with ID %d not found for account ID %d", authorizationID, accountID)) } - return nil, err + return nil, ctxerr.Wrap(ctx, err, "get acme authorization by id") } return &types.Authorization{ From 84d1c6ece800b11888c0f0cb63ca1ebe563ef096 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:38:06 +0000 Subject: [PATCH 30/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/datastore/failing/common_store.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/server/datastore/failing/common_store.go b/server/datastore/failing/common_store.go index 580aa75e9f2..76430be74a6 100644 --- a/server/datastore/failing/common_store.go +++ b/server/datastore/failing/common_store.go @@ -5,6 +5,8 @@ import ( "fmt" "io" "time" + + "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" ) // commonFailingStore is an implementation of CommonStore @@ -30,6 +32,7 @@ func (c commonFailingStore) Cleanup(ctx context.Context, usedIconIDs []string, r return 0, nil } -func (c commonFailingStore) Sign(_ context.Context, _ string, _ time.Duration) (string, error) { - return "", fmt.Errorf("%s store not properly configured", c.Entity) +func (c commonFailingStore) Sign(ctx context.Context, _ string, _ time.Duration) (string, error) { + return "", ctxerr.New(ctx, fmt.Sprintf("%s store not properly configured", c.Entity)) } + From 5bc5ce38727ac997cc43714f29452488bda49361 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:38:07 +0000 Subject: [PATCH 31/36] fix(FLEETMDM-002): 93 review findings across 36 files --- ee/server/service/vpp_users.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/server/service/vpp_users.go b/ee/server/service/vpp_users.go index cc80ad8b603..42775e4a561 100644 --- a/ee/server/service/vpp_users.go +++ b/ee/server/service/vpp_users.go @@ -41,7 +41,7 @@ func (svc *Service) ensureVPPClientUser(ctx context.Context, host *fleet.Host, t return "", ctxerr.Wrapf(ctx, err, "looking up managed apple id for host %d", host.ID) } if managedAppleID == "" { - return "", errMissingManagedAppleID + return "", ctxerr.Wrap(ctx, errMissingManagedAppleID) } // Cache hit on (vpp_token_id, managed_apple_id): a previous successful call From 01cc23ddd8c53df9b52d0c9b8c5afc661fe68b76 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:38:09 +0000 Subject: [PATCH 32/36] fix(FLEETMDM-002): 93 review findings across 36 files --- ee/server/licensing/licensing.go | 37 +++++++++++++++++--------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/ee/server/licensing/licensing.go b/ee/server/licensing/licensing.go index bf649b1115c..63a64c24bb5 100644 --- a/ee/server/licensing/licensing.go +++ b/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,7 +54,7 @@ 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 { @@ -59,14 +62,14 @@ func LoadLicense(licenseKey string) (*fleet.LicenseInfo, error) { // 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{ From aab919e3f2d064a733f6cff6a7094106973465e1 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:38:10 +0000 Subject: [PATCH 33/36] fix(FLEETMDM-002): 93 review findings across 36 files --- ee/server/service/hostidentity/scep.go | 35 +++++++++++++------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/ee/server/service/hostidentity/scep.go b/ee/server/service/hostidentity/scep.go index 2f30d1b326f..034cd1986a6 100644 --- a/ee/server/service/hostidentity/scep.go +++ b/ee/server/service/hostidentity/scep.go @@ -115,14 +115,14 @@ func challengeMiddleware(ds fleet.Datastore, next scepserver.CSRSignerContext) s } 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) + return nil, ctxerr.Wrap(ctx, err, "verifying enrollment secret") } return next.SignCSRContext(ctx, m) } @@ -147,7 +147,7 @@ func renewalMiddleware(ds fleet.Datastore, logger *slog.Logger, next scepserver. for _, ext := range m.CSR.Extensions { if ext.Id.Equal(types.RenewalExtensionOID) { if err := json.Unmarshal(ext.Value, &renewalData); err != nil { - return nil, fmt.Errorf("invalid renewal extension: %w", err) + return nil, ctxerr.Wrap(ctx, err, "invalid renewal extension") } found = true break @@ -165,31 +165,31 @@ func renewalMiddleware(ds fleet.Datastore, logger *slog.Logger, next scepserver. serialBigInt := new(big.Int) _, success := serialBigInt.SetString(strings.TrimPrefix(renewalData.SerialNumber, "0x"), 16) if !success { - return nil, fmt.Errorf("invalid serial number format: %s", renewalData.SerialNumber) + return nil, ctxerr.Errorf(ctx, "invalid serial number format: %s", renewalData.SerialNumber) } // Retrieve the old certificate data oldCertData, err := ds.GetHostIdentityCertBySerialNumber(ctx, serialBigInt.Uint64()) if err != nil { - return nil, fmt.Errorf("retrieving old certificate: %w", err) + return nil, ctxerr.Wrap(ctx, err, "retrieving old certificate") } // Get the public key from the stored data pubKey, err := oldCertData.UnmarshalPublicKey() if err != nil { - return nil, fmt.Errorf("unmarshaling public key: %w", err) + return nil, ctxerr.Wrap(ctx, err, "unmarshaling public key") } // Verify the signature sigBytes, err := base64.StdEncoding.DecodeString(renewalData.Signature) if err != nil { - return nil, fmt.Errorf("decoding signature: %w", err) + return nil, ctxerr.Wrap(ctx, err, "decoding signature") } // Verify the signature hash := sha256.Sum256([]byte(renewalData.SerialNumber)) if !ecdsa.VerifyASN1(pubKey, hash[:], sigBytes) { - return nil, errors.New("invalid renewal signature") + return nil, ctxerr.New(ctx, "invalid renewal signature") } logger.InfoContext(ctx, "renewal signature verified", "serial", renewalData.SerialNumber, "cn", oldCertData.CommonName) @@ -197,17 +197,18 @@ func renewalMiddleware(ds fleet.Datastore, logger *slog.Logger, next scepserver. // Issue the new certificate newCert, err := next.SignCSRContext(ctx, m) if err != nil { - return nil, fmt.Errorf("signing renewal CSR: %w", err) + return nil, ctxerr.Wrap(ctx, err, "signing renewal CSR") } // Update the new certificate's host_id to match the old certificate if oldCertData.HostID != nil { err = ds.UpdateHostIdentityCertHostIDBySerial(ctx, newCert.SerialNumber.Uint64(), *oldCertData.HostID) if err != nil { - // Log the error but don't fail the renewal - ctxerr.Handle(ctx, err) - logger.ErrorContext(ctx, "failed to update host_id for renewed certificate", "err", err, "new_serial", - newCert.SerialNumber.Uint64(), "host_id", *oldCertData.HostID) + // The certificate was already issued successfully, but linking it to the host + // failed. Surface this as an error to the caller instead of silently swallowing + // it, since a renewed certificate without a linked host_id will later fail host + // identity authentication in a way that's hard to trace back to this failure. + return nil, ctxerr.Wrap(ctx, err, "failed to update host_id for renewed certificate") } } @@ -276,12 +277,12 @@ func (svc *service) PKIOperation(ctx context.Context, data []byte) ([]byte, erro cert, err := caKeyPair(ctx, svc.ds) if err != nil { - return nil, fmt.Errorf("retrieving host identity SCEP CA certificate: %w", err) + return nil, ctxerr.Wrap(ctx, err, "retrieving host identity SCEP CA certificate") } pk, ok := cert.PrivateKey.(*rsa.PrivateKey) if !ok { - return nil, errors.New("private key not in RSA format") + return nil, ctxerr.New(ctx, "private key not in RSA format") } if err := msg.DecryptPKIEnvelope(cert.Leaf, pk); err != nil { @@ -290,7 +291,7 @@ func (svc *service) PKIOperation(ctx context.Context, data []byte) ([]byte, erro crt, err := svc.signer.SignCSRContext(ctx, msg.CSRReqMessage) if err == nil && crt == nil { - err = errors.New("signer returned nil certificate without error") + err = ctxerr.New(ctx, "signer returned nil certificate without error") } if err != nil { svc.logger.ErrorContext(ctx, "failed to sign CSR", "err", err) From ff287a319a4661e18ed2b93fe5b02a274a3f1c09 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:38:11 +0000 Subject: [PATCH 34/36] fix(FLEETMDM-002): 93 review findings across 36 files --- ee/server/service/condaccess/scep.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/ee/server/service/condaccess/scep.go b/ee/server/service/condaccess/scep.go index 294b5f18daa..5a527f525a6 100644 --- a/ee/server/service/condaccess/scep.go +++ b/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. From b4f1815c3ecda59e57b057516d8d89b2616e16a3 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:38:12 +0000 Subject: [PATCH 35/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/worker/vpp_verification.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/worker/vpp_verification.go b/server/worker/vpp_verification.go index d6aaf25e58a..db4629251dc 100644 --- a/server/worker/vpp_verification.go +++ b/server/worker/vpp_verification.go @@ -43,7 +43,7 @@ func (v *AppleSoftware) Run(ctx context.Context, argsJSON json.RawMessage) error switch args.Task { case verifyVPPTask: err := v.verifyVPPInstalls(ctx, args.HostUUID, args.VerificationCommandUUID, args.DisableManagedOnlyApps) - return ctxerr.Wrap(ctx, err, "running migrate VPP token task") + return ctxerr.Wrap(ctx, err, "running verify VPP installs task") default: return ctxerr.Errorf(ctx, "unknown task: %v", args.Task) From 360a5b46cd29e95eff5894b12ac6db590de85c58 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:38:13 +0000 Subject: [PATCH 36/36] fix(FLEETMDM-002): 93 review findings across 36 files --- server/acl/chartacl/fleet_adapter.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/acl/chartacl/fleet_adapter.go b/server/acl/chartacl/fleet_adapter.go index 47e0e15c44e..e246392b96f 100644 --- a/server/acl/chartacl/fleet_adapter.go +++ b/server/acl/chartacl/fleet_adapter.go @@ -9,9 +9,9 @@ package chartacl import ( "context" - "errors" "github.com/fleetdm/fleet/v4/server/chart/api" + "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/contexts/viewer" ) @@ -38,7 +38,7 @@ var _ api.ViewerProvider = (*FleetViewerAdapter)(nil) func (a *FleetViewerAdapter) ViewerScope(ctx context.Context) (bool, []uint, error) { vc, ok := viewer.FromContext(ctx) if !ok || vc.User == nil { - return false, nil, errors.New("chart: no authenticated viewer in context") + return false, nil, ctxerr.New(ctx, "chart: no authenticated viewer in context") } u := vc.User if u.GlobalRole != nil && *u.GlobalRole != "" {