PMM-15129 Fix gosec Go linter issues (p2)

managed/cmd/pmm-encryption-rotation/main.go

@@ -75,7 +75,10 @@ func main() {
 	}
 
 	statusCode, err := encryptionService.RotateEncryptionKey(sqlDB, "pmm-managed")
-	sqlDB.Close() //nolint:errcheck
+	closeErr := sqlDB.Close()
+	if closeErr != nil {
+		logrus.Error(closeErr)
+	}
 	if err != nil {
 		logrus.Error(err)
 		os.Exit(statusCode)

managed/cmd/pmm-managed/main.go

@@ -130,8 +130,13 @@ const (
 	cleanOlderThan = 30 * time.Minute
 
 	defaultContextTimeout = 10 * time.Second
-	pProfProfileDuration  = 30 * time.Second
-	pProfTraceDuration    = 10 * time.Second
+	// When ReadHeaderTimeout is not configured, a malicious client can open
+	// a connection and send headers extremely slowly (or not at all),
+	// keeping the connection open indefinitely. This "Slowloris" attack can eventually
+	// exhaust the server's connection pool, leading to a Denial of Service (DoS).
+	readHeaderTimeout    = 10 * time.Second
+	pProfProfileDuration = 30 * time.Second
+	pProfTraceDuration   = 10 * time.Second
 
 	clickhouseMaxIdleConns = 5
 	clickhouseMaxOpenConns = 10
@@ -439,10 +444,11 @@ func runHTTP1Server(ctx context.Context, deps *http1ServerDeps) {
 	mux.Handle("/v1/users/current", deps.currentUserHandler)
 	mux.Handle("/", proxyMux)
 
-	server := &http.Server{ //nolint:gosec
-		Addr:     http1Addr,
-		ErrorLog: log.New(os.Stderr, "runJSONServer: ", 0),
-		Handler:  mux,
+	server := &http.Server{
+		Addr:              http1Addr,
+		ErrorLog:          log.New(os.Stderr, "runJSONServer: ", 0),
+		Handler:           mux,
+		ReadHeaderTimeout: readHeaderTimeout,
 	}
 	go func() {
 		err := server.ListenAndServe()
@@ -499,13 +505,17 @@ func runDebugServer(ctx context.Context) {
 		l.Fatal(err)
 	}
 	http.HandleFunc("/debug", func(rw http.ResponseWriter, _ *http.Request) {
-		rw.Write(buf.Bytes()) //nolint:errcheck
+		_, wErr := rw.Write(buf.Bytes())
+		if wErr != nil {
+			l.Errorf("Failed to write debug page: %v", wErr)
+		}
 	})
 	l.Infof("Starting server on http://%s/debug\nRegistered handlers:\n\t%s", debugAddr, strings.Join(handlers, "\n\t"))
 
-	server := &http.Server{ //nolint:gosec
-		Addr:     debugAddr,
-		ErrorLog: log.New(os.Stderr, "runDebugServer: ", 0),
+	server := &http.Server{
+		Addr:              debugAddr,
+		ErrorLog:          log.New(os.Stderr, "runDebugServer: ", 0),
+		ReadHeaderTimeout: readHeaderTimeout,
 	}
 	go func() {
 		err := server.ListenAndServe()

managed/models/artifact_helpers.go

@@ -330,10 +330,8 @@ func DeleteArtifact(q *reform.Querier, id string) error {
 
 // MetadataRemoveFirstN removes first N records from artifact metadata list.
 func (s *Artifact) MetadataRemoveFirstN(q *reform.Querier, n uint32) error {
-	if n > uint32(len(s.MetadataList)) {
-		n = uint32(len(s.MetadataList))
-	}
-	s.MetadataList = s.MetadataList[n:]
+	idx := min(int(n), len(s.MetadataList))
+	s.MetadataList = s.MetadataList[idx:]
 	err := q.Update(s)
 	if err != nil {
 		return fmt.Errorf("failed to remove artifact metadata records: %w", err)

managed/models/artifact_helpers_test.go

@@ -118,6 +118,7 @@ func TestArtifacts(t *testing.T) {
 		assert.Equal(t, createParams.DataModel, a.DataModel)
 		assert.Equal(t, createParams.Status, a.Status)
 		assert.Equal(t, createParams.Folder, a.Folder)
+		assert.Equal(t, models.OnDemandArtifactType, a.Type)
 		assert.Less(t, time.Now().UTC().Unix()-a.CreatedAt.Unix(), int64(5))
 
 		updateParams := models.UpdateArtifactParams{
@@ -135,9 +136,108 @@ func TestArtifacts(t *testing.T) {
 		assert.Equal(t, *updateParams.ServiceID, a.ServiceID)
 		assert.Equal(t, updateParams.IsShardedCluster, a.IsShardedCluster)
 		assert.Less(t, time.Now().UTC().Unix()-a.UpdatedAt.Unix(), int64(5))
+
+		t.Run("scheduled type", func(t *testing.T) {
+			createParams.Name = "scheduled_backup"
+			createParams.ScheduleID = "some_schedule"
+			sa, err := models.CreateArtifact(q, createParams)
+			require.NoError(t, err)
+			assert.Equal(t, models.ScheduledArtifactType, sa.Type)
+			assert.Equal(t, "some_schedule", sa.ScheduleID)
+		})
+	})
+
+	t.Run("unique name", func(t *testing.T) {
+		tx, err := db.Begin()
+		require.NoError(t, err)
+		t.Cleanup(func() {
+			require.NoError(t, tx.Rollback())
+		})
+
+		q := tx.Querier
+		prepareLocationsAndService(q)
+
+		params := models.CreateArtifactParams{
+			Name:       "unique_name",
+			Vendor:     "MySQL",
+			LocationID: locationID1,
+			ServiceID:  serviceID1,
+			DataModel:  models.PhysicalDataModel,
+			Status:     models.PendingBackupStatus,
+			Mode:       models.Snapshot,
+		}
+
+		_, err = models.CreateArtifact(q, params)
+		require.NoError(t, err)
+
+		_, err = models.CreateArtifact(q, params)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), `Artifact with name "unique_name" already exists.`)
+	})
+
+	t.Run("find by ID and name", func(t *testing.T) {
+		tx, err := db.Begin()
+		require.NoError(t, err)
+		t.Cleanup(func() {
+			require.NoError(t, tx.Rollback())
+		})
+
+		q := tx.Querier
+		prepareLocationsAndService(q)
+
+		a, err := models.CreateArtifact(q, models.CreateArtifactParams{
+			Name:       "find_me",
+			Vendor:     "MySQL",
+			LocationID: locationID1,
+			ServiceID:  serviceID1,
+			DataModel:  models.PhysicalDataModel,
+			Status:     models.PendingBackupStatus,
+			Mode:       models.Snapshot,
+		})
+		require.NoError(t, err)
+
+		// Find by ID
+		found, err := models.FindArtifactByID(q, a.ID)
+		require.NoError(t, err)
+		assert.Equal(t, a.Name, found.Name)
+
+		_, err = models.FindArtifactByID(q, "non-existent")
+		require.ErrorIs(t, err, models.ErrNotFound)
+
+		// Find by Name
+		found, err = models.FindArtifactByName(q, "find_me")
+		require.NoError(t, err)
+		assert.Equal(t, a.ID, found.ID)
+
+		_, err = models.FindArtifactByName(q, "unknown")
+		assert.ErrorIs(t, err, models.ErrNotFound)
 	})
 
-	t.Run("list", func(t *testing.T) {
+	t.Run("find by IDs", func(t *testing.T) {
+		tx, err := db.Begin()
+		require.NoError(t, err)
+		t.Cleanup(func() {
+			require.NoError(t, tx.Rollback())
+		})
+
+		q := tx.Querier
+		prepareLocationsAndService(q)
+
+		a1, _ := models.CreateArtifact(q, models.CreateArtifactParams{Name: "a1", Vendor: "V", LocationID: locationID1, ServiceID: serviceID1, DataModel: models.PhysicalDataModel, Status: models.PendingBackupStatus, Mode: models.Snapshot})
+		a2, _ := models.CreateArtifact(q, models.CreateArtifactParams{Name: "a2", Vendor: "V", LocationID: locationID1, ServiceID: serviceID1, DataModel: models.PhysicalDataModel, Status: models.PendingBackupStatus, Mode: models.Snapshot})
+
+		res, err := models.FindArtifactsByIDs(q, []string{a1.ID, a2.ID, "unknown"})
+		require.NoError(t, err)
+		assert.Len(t, res, 2)
+		assert.Contains(t, res, a1.ID)
+		assert.Contains(t, res, a2.ID)
+
+		emptyRes, err := models.FindArtifactsByIDs(q, []string{})
+		require.NoError(t, err)
+		assert.Empty(t, emptyRes)
+	})
+
+	t.Run("list and filters", func(t *testing.T) {
 		tx, err := db.Begin()
 		require.NoError(t, err)
 		t.Cleanup(func() {
@@ -206,13 +306,23 @@ func TestArtifacts(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, []*models.Artifact{a3}, actual2)
 
-		actual3, err := models.FindArtifacts(q, models.ArtifactFilters{})
+		// Check filters by status and service.
+		actual3, err := models.FindArtifacts(q, models.ArtifactFilters{Status: models.PausedBackupStatus, ServiceID: serviceID2})
 		require.NoError(t, err)
-		require.Len(t, actual3, 3)
+		assert.Len(t, actual3, 1)
+		assert.Equal(t, a2.ID, actual3[0].ID)
 
-		for _, a := range actual3 {
-			assert.Contains(t, []models.Artifact{*a1, *a2, *a3}, *a)
-		}
+		// Check non-existent location error.
+		_, err = models.FindArtifacts(q, models.ArtifactFilters{LocationID: "invalid_loc"})
+		require.Error(t, err)
+
+		// Check sorting (latest first).
+		all, err := models.FindArtifacts(q, models.ArtifactFilters{})
+		require.NoError(t, err)
+		require.Len(t, all, 3)
+		assert.Equal(t, a3.ID, all[0].ID)
+		assert.Equal(t, a2.ID, all[1].ID)
+		assert.Equal(t, a1.ID, all[2].ID)
 	})
 
 	t.Run("remove", func(t *testing.T) {
@@ -446,3 +556,19 @@ func TestArtifactValidation(t *testing.T) {
 		})
 	}
 }
+
+func TestIsArtifactFinalStatus(t *testing.T) {
+	testCases := []struct {
+		status   models.BackupStatus
+		expected bool
+	}{
+		{models.SuccessBackupStatus, true},
+		{models.ErrorBackupStatus, true},
+		{models.FailedToDeleteBackupStatus, true},
+		{models.PendingBackupStatus, false},
+		{models.PausedBackupStatus, false},
+	}
+	for _, tc := range testCases {
+		assert.Equal(t, tc.expected, models.IsArtifactFinalStatus(tc.status), "Status: %s", tc.status)
+	}
+}

managed/models/encryption_helpers.go

@@ -158,6 +158,9 @@ func awsOptionsHandler(val any, handler func(string) (string, error)) (any, erro
 		return nil, err
 	}
 
+	// The gosec G117 warning is triggered when sensitive fields (like ClientSecret, AWSSecretKey, or TLSKey)
+	// are part of a structure being marshaled to JSON.
+	// In the context of this file, these operations are intentional.
 	res, err := json.Marshal(o)
 	if err != nil {
 		return nil, err
@@ -205,7 +208,10 @@ func azureOptionsHandler(val any, handler func(string) (string, error)) (any, er
 		return nil, err
 	}
 
-	res, err := json.Marshal(o)
+	// The gosec G117 warning is triggered when sensitive fields (like ClientSecret, AWSSecretKey, or TLSKey)
+	// are part of a structure being marshaled to JSON.
+	// In the context of this file, these operations are intentional.
+	res, err := json.Marshal(o) //nolint:gosec
 	if err != nil {
 		return nil, err
 	}
@@ -244,6 +250,9 @@ func mongoDBOptionsHandler(val any, handler func(string) (string, error)) (any,
 		return nil, err
 	}
 
+	// The gosec G117 warning is triggered when sensitive fields (like ClientSecret, AWSSecretKey, or TLSKey)
+	// are part of a structure being marshaled to JSON.
+	// In the context of this file, these operations are intentional.
 	res, err := json.Marshal(o)
 	if err != nil {
 		return nil, err
@@ -283,6 +292,9 @@ func mySQLOptionsHandler(val any, handler func(string) (string, error)) (any, er
 		return nil, err
 	}
 
+	// The gosec G117 warning is triggered when sensitive fields (like ClientSecret, AWSSecretKey, or TLSKey)
+	// are part of a structure being marshaled to JSON.
+	// In the context of this file, these operations are intentional.
 	res, err := json.Marshal(o)
 	if err != nil {
 		return nil, err
@@ -322,6 +334,9 @@ func postgreSQLOptionsHandler(val any, handler func(string) (string, error)) (an
 		return nil, err
 	}
 
+	// The gosec G117 warning is triggered when sensitive fields (like ClientSecret, AWSSecretKey, or TLSKey)
+	// are part of a structure being marshaled to JSON.
+	// In the context of this file, these operations are intentional.
 	res, err := json.Marshal(o)
 	if err != nil {
 		return nil, err

managed/models/role_helpers.go

@@ -18,6 +18,7 @@ package models
 import (
 	"errors"
 	"fmt"
+	"math"
 	"strings"
 
 	"github.com/sirupsen/logrus"
@@ -51,6 +52,9 @@ func AssignRoles(tx *reform.TX, userID int, roleIDs []int) error {
 			return err
 		}
 
+		if roleID < 0 || roleID > math.MaxUint32 {
+			return fmt.Errorf("role ID %d is out of range for uint32", roleID)
+		}
 		var userRole UserRoles
 		userRole.UserID = userID
 		userRole.RoleID = uint32(roleID)

managed/services/agents/actions.go

@@ -80,7 +80,7 @@ func (s *ActionsService) StartMySQLExplainAction(
 		return err
 	}
 
-	if res.PlaceholdersCount != uint32(len(placeholders)) {
+	if res.PlaceholdersCount != uint32(len(placeholders)) { //nolint:gosec // placeholders count is not expected to exceed uint32
 		return status.Error(codes.FailedPrecondition, "placeholders count is not correct")
 	}
 	q = res.ExplainFingerprint

managed/services/agents/service_info_broker.go

@@ -18,6 +18,7 @@ package agents
 import (
 	"context"
 	"fmt"
+	"math"
 	"time"
 
 	"github.com/AlekSi/pointer"
@@ -213,7 +214,9 @@ func (c *ServiceInfoBroker) GetInfoFromService(ctx context.Context, q *reform.Qu
 			}
 		}
 		agent.PostgreSQLOptions.PGSMVersion = sInfo.PgsmVersion
-		agent.PostgreSQLOptions.DatabaseCount = int32(databaseCount - excludedDatabaseCount)
+
+		dbCount := min(max(databaseCount-excludedDatabaseCount, 0), math.MaxInt32)
+		agent.PostgreSQLOptions.DatabaseCount = int32(dbCount)
 
 		l.Debugf("Updating PostgreSQL options, database count: %d.", agent.PostgreSQLOptions.DatabaseCount)
 		err = q.Update(new(models.EncryptAgent(*agent)))