fix: Apply gosec and codeql recommendations

Signed-off-by: Mahendra Paipuri <mahendra.paipuri@gmail.com>

.golangci.yml

@@ -2,15 +2,7 @@ linters:
   enable:
     - misspell
     - revive
-  disable:
-    # Disable soon to deprecated[1] linters that lead to false
-    # positives when build tags disable certain files[2]
-    # 1: https://github.com/golangci/golangci-lint/issues/1841
-    # 2: https://github.com/prometheus/node_exporter/issues/1545
-    - deadcode
-    - unused
-    - structcheck
-    - varcheck
+    - gosec
 
 issues:
   exclude-rules:

internal/osexec/osexec.go

@@ -3,6 +3,7 @@ package osexec
 
 import (
 	"context"
+	"math"
 	"os"
 	"os/exec"
 	"strings"
@@ -46,9 +47,18 @@ func ExecuteAs(cmd string, args []string, uid int, gid int, env []string, logger
 		Log("msg", "Executing as user", "command", cmd, "args", strings.Join(args, " "), "uid", uid, "gid", gid)
 	execCmd := exec.Command(cmd, args...)
 
+	// Check bounds on uid and gid before converting into int32
+	var uidInt32, gidInt32 uint32
+	if uid > 0 && uid <= math.MaxInt32 {
+		uidInt32 = uint32(uid)
+	}
+	if gid > 0 && gid <= math.MaxInt32 {
+		gidInt32 = uint32(gid)
+	}
+
 	// Set uid and gid for process
 	execCmd.SysProcAttr = &syscall.SysProcAttr{}
-	execCmd.SysProcAttr.Credential = &syscall.Credential{Uid: uint32(uid), Gid: uint32(gid)}
+	execCmd.SysProcAttr.Credential = &syscall.Credential{Uid: uidInt32, Gid: gidInt32}
 
 	// If env is not nil pointer, add env vars into subprocess cmd
 	if env != nil {
@@ -129,9 +139,18 @@ func ExecuteAsWithTimeout(
 		execCmd.Env = append(os.Environ(), env...)
 	}
 
-	// Set uid and gid for the process
+	// Check bounds on uid and gid before converting into int32
+	var uidInt32, gidInt32 uint32
+	if uid > 0 && uid <= math.MaxInt32 {
+		uidInt32 = uint32(uid)
+	}
+	if gid > 0 && gid <= math.MaxInt32 {
+		gidInt32 = uint32(gid)
+	}
+
+	// Set uid and gid for process
 	execCmd.SysProcAttr = &syscall.SysProcAttr{}
-	execCmd.SysProcAttr.Credential = &syscall.Credential{Uid: uint32(uid), Gid: uint32(gid)}
+	execCmd.SysProcAttr.Credential = &syscall.Credential{Uid: uidInt32, Gid: gidInt32}
 
 	// Execute command
 	out, err := execCmd.CombinedOutput()

pkg/api/cli/cli_test.go

@@ -37,7 +37,7 @@ func queryServer(address string) error {
 
 func makeConfigFile(configFile string, tmpDir string) string {
 	configPath := filepath.Join(tmpDir, "config.yml")
-	os.WriteFile(configPath, []byte(configFile), 0644)
+	os.WriteFile(configPath, []byte(configFile), 0600)
 	return configPath
 }
 

pkg/api/db/db.go

@@ -507,18 +507,18 @@ func (s *statsDB) getUnitStats(startTime, endTime time.Time) error {
 
 	// Make prepare statement and defer closing statement
 	level.Debug(s.logger).Log("msg", "Preparing SQL statements")
-	stmtMap, err := s.prepareStatements(tx)
+	sqlStmts, err := s.prepareStatements(tx)
 	if err != nil {
 		return err
 	}
-	for _, stmt := range stmtMap {
+	for _, stmt := range sqlStmts {
 		defer stmt.Close()
 	}
 	level.Debug(s.logger).Log("msg", "Finished preparing SQL statements")
 
 	// Insert data into DB
 	level.Debug(s.logger).Log("msg", "Executing SQL statements")
-	s.execStatements(stmtMap, units, users, projects)
+	s.execStatements(sqlStmts, units, users, projects)
 	level.Debug(s.logger).Log("msg", "Finished executing SQL statements")
 
 	// Commit changes
@@ -536,7 +536,7 @@ func (s *statsDB) getUnitStats(startTime, endTime time.Time) error {
 func (s *statsDB) purgeExpiredUnits(tx *sql.Tx) error {
 	// Purge expired units
 	deleteUnitsQuery := fmt.Sprintf(
-		"DELETE FROM %s WHERE started_at <= date('now', '-%d day')",
+		"DELETE FROM %s WHERE started_at <= date('now', '-%d day')", // #nosec
 		base.UnitsDBTableName,
 		int(s.storage.retentionPeriod.Hours()/24),
 	)
@@ -551,7 +551,7 @@ func (s *statsDB) purgeExpiredUnits(tx *sql.Tx) error {
 
 	// Purge stale usage data
 	deleteUsageQuery := fmt.Sprintf(
-		"DELETE FROM %s WHERE last_updated_at <= date('now', '-%d day')",
+		"DELETE FROM %s WHERE last_updated_at <= date('now', '-%d day')", // #nosec
 		base.UsageDBTableName,
 		int(s.storage.retentionPeriod.Hours()/24),
 	)

pkg/api/db/helpers.go

@@ -39,7 +39,7 @@ func makeDSN(filePath string, opts map[string]string) string {
 func writeTimeStampToFile(filePath string, timeStamp time.Time, logger log.Logger) {
 	timeStampString := timeStamp.Format(base.DatetimeLayout)
 	timeStampByte := []byte(timeStampString)
-	if err := os.WriteFile(filePath, timeStampByte, 0644); err != nil {
+	if err := os.WriteFile(filePath, timeStampByte, 0600); err != nil {
 		level.Error(logger).
 			Log("msg", "Failed to write timestamp to file", "time", timeStampString, "file", filePath, "err", err)
 	}

pkg/api/http/demo.go

@@ -58,7 +58,7 @@ func init() {
 
 // randomFloats returns random float64s in the range
 func randomFloats(min, max float64) models.JSONFloat {
-	return models.JSONFloat(min + rand.Float64()*(max-min))
+	return models.JSONFloat(min + rand.Float64()*(max-min)) // #nosec
 }
 
 // random returns random number between min and max excluding max
@@ -69,11 +69,11 @@ func random(min, max int64) int64 {
 // randomHelper returns max int64 if n is more than max
 func randomHelper(n int64) int64 {
 	if n < maxInt64 {
-		return int64(rand.Int63n(int64(n + 1)))
+		return int64(rand.Int63n(int64(n + 1))) // #nosec
 	}
-	x := int64(rand.Uint64())
+	x := int64(rand.Uint64()) // #nosec
 	for x > n {
-		x = int64(rand.Uint64())
+		x = int64(rand.Uint64()) // #nosec
 	}
 	return x
 }

pkg/api/http/middleware_test.go

@@ -40,9 +40,11 @@ func TestMiddlewareSuccess(t *testing.T) {
 	// call the handler using a mock response recorder (we'll not use that anyway)
 	w := httptest.NewRecorder()
 	handlerToTest.ServeHTTP(w, req)
+	resp := w.Result()
+	defer resp.Body.Close()
 
 	// Should pass test
-	if w.Result().StatusCode != 200 {
+	if resp.StatusCode != 200 {
 		t.Errorf("expected 200 got %d", w.Result().StatusCode)
 	}
 	// Check headers added to req

pkg/api/http/server.go

@@ -7,7 +7,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-	_ "net/http/pprof"
+	_ "net/http/pprof" // #nosec
 	"net/url"
 	"path/filepath"
 	"regexp"
@@ -129,10 +129,11 @@ func NewCEEMSServer(c *Config) (*CEEMSServer, func(), error) {
 	server := &CEEMSServer{
 		logger: c.Logger,
 		server: &http.Server{
-			Addr:         c.Web.Address,
-			Handler:      router,
-			ReadTimeout:  10 * time.Second,
-			WriteTimeout: 10 * time.Second,
+			Addr:              c.Web.Address,
+			Handler:           router,
+			ReadTimeout:       10 * time.Second,
+			WriteTimeout:      10 * time.Second,
+			ReadHeaderTimeout: 2 * time.Second, // slowloris attack: https://app.deepsource.com/directory/analyzers/go/issues/GO-S2112
 		},
 		webConfig: &web.FlagConfig{
 			WebListenAddresses: &[]string{c.Web.Address},

pkg/api/resource/manager_test.go

@@ -105,7 +105,7 @@ clusters:
 
 	configFile := fmt.Sprintf(configFileTmpl, tmpDir, serverURL)
 	configPath := filepath.Join(tmpDir, "config.yml")
-	os.WriteFile(configPath, []byte(configFile), 0644)
+	os.WriteFile(configPath, []byte(configFile), 0600)
 	return configPath
 }
 

pkg/api/resource/slurm/manager_test.go

@@ -145,12 +145,12 @@ func TestSLURMFetcherMultiCluster(t *testing.T) {
 	sacctPath := filepath.Join(tmpDir, "sacct")
 	sacctScript := fmt.Sprintf(`#!/bin/bash
 printf """%s"""`, sacctCmdOutput)
-	os.WriteFile(sacctPath, []byte(sacctScript), 0755)
+	os.WriteFile(sacctPath, []byte(sacctScript), 0700) // #nosec
 
 	sacctMgrPath := filepath.Join(tmpDir, "sacctmgr")
 	sacctMgrScript := fmt.Sprintf(`#!/bin/bash
 printf """%s"""`, sacctMgrCmdOutput)
-	os.WriteFile(sacctMgrPath, []byte(sacctMgrScript), 0755)
+	os.WriteFile(sacctMgrPath, []byte(sacctMgrScript), 0700) // #nosec
 
 	sacctMgrDir := filepath.Dir(sacctMgrPath)
 

pkg/api/updater/updater_test.go

@@ -77,7 +77,7 @@ updaters:
 
 	configFile := fmt.Sprintf(configFileTmpl, serverURL, "2m")
 	configPath := filepath.Join(tmpDir, "config.yml")
-	os.WriteFile(configPath, []byte(configFile), 0644)
+	os.WriteFile(configPath, []byte(configFile), 0600)
 	return configPath
 }
 

pkg/collector/cli.go

@@ -3,10 +3,11 @@ package collector
 import (
 	"fmt"
 	"net/http"
-	_ "net/http/pprof"
+	_ "net/http/pprof" // #nosec
 	"os"
 	"os/user"
 	"runtime"
+	"time"
 
 	"github.com/alecthomas/kingpin/v2"
 	"github.com/go-kit/log"
@@ -160,7 +161,9 @@ func (b *CEEMSExporter) Main() error {
 		http.Handle("/", landingPage)
 	}
 
-	server := &http.Server{}
+	server := &http.Server{
+		ReadHeaderTimeout: 2 * time.Second, // slowloris attack: https://app.deepsource.com/directory/analyzers/go/issues/GO-S2112
+	}
 	if err := web.ListenAndServe(server, toolkitFlags, logger); err != nil {
 		return fmt.Errorf("failed to start server: %s", err)
 	}

pkg/collector/handler.go

@@ -41,7 +41,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if err != nil {
 		level.Warn(h.logger).Log("msg", "Couldn't create filtered metrics handler:", "err", err)
 		w.WriteHeader(http.StatusBadRequest)
-		w.Write([]byte(fmt.Sprintf("Couldn't create filtered metrics handler: %s", err)))
+		w.Write([]byte("Couldn't create filtered metrics handler"))
 		return
 	}
 	filteredHandler.ServeHTTP(w, r)

pkg/collector/slurm.go

@@ -581,7 +581,12 @@ func (c *slurmCollector) readJobPropsFromProlog(jobid string, jobProps *JobProps
 			level.Error(c.logger).
 				Log("msg", "Failed to get job properties from prolog generated files", "file", slurmJobInfo, "err", err)
 		} else {
-			fmt.Sscanf(string(content), "%s %s %s", &jobProps.jobUser, &jobProps.jobAccount, &jobProps.jobNodelist)
+			if _, err := fmt.Sscanf(
+				string(content), "%s %s %s", &jobProps.jobUser, &jobProps.jobAccount, &jobProps.jobNodelist,
+			); err != nil {
+				level.Error(c.logger).
+					Log("msg", "Failed to scan prolog generated file", "file", slurmJobInfo, "err", err)
+			}
 		}
 	}
 
@@ -612,7 +617,13 @@ func (c *slurmCollector) readJobPropsFromProlog(jobid string, jobProps *JobProps
 				)
 				continue
 			}
-			fmt.Sscanf(string(content), "%s", &gpuJobID)
+			if _, err := fmt.Sscanf(string(content), "%s", &gpuJobID); err != nil {
+				level.Error(c.logger).Log(
+					"msg", "Failed to scan job ID for GPU",
+					"index", dev.index, "uuid", dev.uuid, "err", err,
+				)
+				continue
+			}
 			if gpuJobID == jobid {
 				jobProps.jobGPUOrdinals = append(jobProps.jobGPUOrdinals, dev.index)
 			}

pkg/lb/cli/cli_test.go

@@ -48,7 +48,7 @@ func queryLB(address string) error {
 
 func makeConfigFile(configFile string, tmpDir string) string {
 	configPath := filepath.Join(tmpDir, "config.yml")
-	os.WriteFile(configPath, []byte(configFile), 0644)
+	os.WriteFile(configPath, []byte(configFile), 0600)
 	return configPath
 }
 

pkg/lb/frontend/frontend.go

@@ -114,7 +114,8 @@ outside:
 	return &loadBalancer{
 		logger: c.Logger,
 		server: &http.Server{
-			Addr: c.Address,
+			Addr:              c.Address,
+			ReadHeaderTimeout: 2 * time.Second, // slowloris attack: https://app.deepsource.com/directory/analyzers/go/issues/GO-S2112
 		},
 		webConfig: &web.FlagConfig{
 			WebListenAddresses: &[]string{c.Address},