lint: enable errcheck; add allowlist and explicit checks (crowdsecuri…

…ty#3403)

* lint: enable errcheck with explicit allow list
* add explicit error checks
* windows tests
* windows nolint

.golangci.yml

@@ -5,6 +5,37 @@ run:
     - expr_debug
 
 linters-settings:
+  errcheck:
+    # Report about not checking of errors in type assertions: `a := b.(MyStruct)`.
+    # Such cases aren't reported by default.
+    # Default: false
+    check-type-assertions: false
+    # List of functions to exclude from checking, where each entry is a single function to exclude.
+    # See https://github.com/kisielk/errcheck#excluding-functions for details.
+    exclude-functions:
+     - (*bytes.Buffer).ReadFrom   # TODO:
+     - io.Copy                    # TODO:
+     - (net/http.ResponseWriter).Write    # TODO:
+     - (*os/exec.Cmd).Start
+     - (*os/exec.Cmd).Wait
+     - (*os.Process).Kill
+     - (*text/template.Template).ExecuteTemplate
+     - syscall.FreeLibrary
+     - golang.org/x/sys/windows.CloseHandle
+     - golang.org/x/sys/windows.ResetEvent
+     - (*golang.org/x/sys/windows/svc/eventlog.Log).Info
+     - (*golang.org/x/sys/windows/svc/mgr.Mgr).Disconnect
+
+     - (github.com/bluele/gcache.Cache).Set
+     - (github.com/gin-gonic/gin.ResponseWriter).WriteString
+     - (*github.com/segmentio/kafka-go.Reader).SetOffsetAt
+     - (*gopkg.in/tomb.v2.Tomb).Wait
+
+     - (*github.com/crowdsecurity/crowdsec/pkg/appsec.ReqDumpFilter).FilterArgs
+     - (*github.com/crowdsecurity/crowdsec/pkg/appsec.ReqDumpFilter).FilterBody
+     - (*github.com/crowdsecurity/crowdsec/pkg/appsec.ReqDumpFilter).FilterHeaders
+     - (*github.com/crowdsecurity/crowdsec/pkg/longpollclient.LongPollClient).Stop
+
   gci:
     sections:
      - standard
@@ -318,10 +349,6 @@ issues:
         - govet
       text: "shadow: declaration of \"(err|ctx)\" shadows declaration"
 
-    - linters:
-        - errcheck
-      text: "Error return value of `.*` is not checked"
-
     # Will fix, trivial - just beware of merge conflicts
 
     - linters:

cmd/crowdsec-cli/climachine/add.go

@@ -73,7 +73,9 @@ func (cli *cliMachines) add(ctx context.Context, args []string, machinePassword
 		qs := &survey.Password{
 			Message: "Please provide a password for the machine:",
 		}
-		survey.AskOne(qs, &machinePassword)
+		if err := survey.AskOne(qs, &machinePassword); err != nil {
+			return err
+		}
 	}
 
 	password := strfmt.Password(machinePassword)
@@ -147,9 +149,9 @@ cscli machines add -f- --auto > /tmp/mycreds.yaml`,
 	flags.VarP(&password, "password", "p", "machine password to login to the API")
 	flags.StringVarP(&dumpFile, "file", "f", "", "output file destination (defaults to "+csconfig.DefaultConfigPath("local_api_credentials.yaml")+")")
 	flags.StringVarP(&apiURL, "url", "u", "", "URL of the local API")
-	flags.BoolVarP(&interactive, "interactive", "i", false, "interfactive mode to enter the password")
+	flags.BoolVarP(&interactive, "interactive", "i", false, "interactive mode to enter the password")
 	flags.BoolVarP(&autoAdd, "auto", "a", false, "automatically generate password (and username if not provided)")
-	flags.BoolVar(&force, "force", false, "will force add the machine if it already exist")
+	flags.BoolVar(&force, "force", false, "will force add the machine if it already exists")
 
 	return cmd
 }

cmd/crowdsec-cli/completion.go

@@ -71,13 +71,13 @@ func NewCompletionCmd() *cobra.Command {
 		Run: func(cmd *cobra.Command, args []string) {
 			switch args[0] {
 			case "bash":
-				cmd.Root().GenBashCompletion(os.Stdout)
+				_ = cmd.Root().GenBashCompletion(os.Stdout)
 			case "zsh":
-				cmd.Root().GenZshCompletion(os.Stdout)
+				_ = cmd.Root().GenZshCompletion(os.Stdout)
 			case "powershell":
-				cmd.Root().GenPowerShellCompletion(os.Stdout)
+				_ = cmd.Root().GenPowerShellCompletion(os.Stdout)
 			case "fish":
-				cmd.Root().GenFishCompletion(os.Stdout, true)
+				_ = cmd.Root().GenFishCompletion(os.Stdout, true)
 			}
 		},
 	}

cmd/crowdsec/serve.go

@@ -419,7 +419,7 @@ func Serve(cConfig *csconfig.Config, agentReady chan bool) error {
 	}
 
 	if cConfig.Common != nil && cConfig.Common.Daemonize {
-		csdaemon.Notify(csdaemon.Ready, log.StandardLogger())
+		_ = csdaemon.Notify(csdaemon.Ready, log.StandardLogger())
 		// wait for signals
 		return HandleSignals(cConfig)
 	}

pkg/acquisition/modules/appsec/appsec.go

@@ -330,7 +330,10 @@ func (w *AppsecSource) StreamingAcquisition(ctx context.Context, out chan types.
 		w.logger.Info("Shutting down Appsec server")
 		// xx let's clean up the appsec runners :)
 		appsec.AppsecRulesDetails = make(map[int]appsec.RulesDetails)
-		w.server.Shutdown(ctx)
+
+		if err := w.server.Shutdown(ctx); err != nil {
+			w.logger.Errorf("Error shutting down Appsec server: %s", err.Error())
+		}
 
 		return nil
 	})

pkg/acquisition/modules/kubernetesaudit/k8s_audit.go

@@ -154,7 +154,9 @@ func (ka *KubernetesAuditSource) StreamingAcquisition(ctx context.Context, out c
 		})
 		<-t.Dying()
 		ka.logger.Infof("Stopping k8s-audit server on %s:%d%s", ka.config.ListenAddr, ka.config.ListenPort, ka.config.WebhookPath)
-		ka.server.Shutdown(ctx)
+		if err := ka.server.Shutdown(ctx); err != nil {
+			ka.logger.Errorf("Error shutting down k8s-audit server: %s", err.Error())
+		}
 
 		return nil
 	})

pkg/acquisition/modules/wineventlog/wineventlog_windows_test.go

@@ -7,18 +7,22 @@ import (
 	"testing"
 	"time"
 
-	"github.com/crowdsecurity/crowdsec/pkg/acquisition/configuration"
-	"github.com/crowdsecurity/crowdsec/pkg/exprhelpers"
-	"github.com/crowdsecurity/crowdsec/pkg/types"
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sys/windows/svc/eventlog"
 	"gopkg.in/tomb.v2"
+
+	"github.com/crowdsecurity/go-cs-lib/cstest"
+
+	"github.com/crowdsecurity/crowdsec/pkg/acquisition/configuration"
+	"github.com/crowdsecurity/crowdsec/pkg/exprhelpers"
+	"github.com/crowdsecurity/crowdsec/pkg/types"
 )
 
 func TestBadConfiguration(t *testing.T) {
-	exprhelpers.Init(nil)
+	err := exprhelpers.Init(nil)
+	require.NoError(t, err)
 
 	tests := []struct {
 		config      string
@@ -62,7 +66,8 @@ xpath_query: test`,
 }
 
 func TestQueryBuilder(t *testing.T) {
-	exprhelpers.Init(nil)
+	err := exprhelpers.Init(nil)
+	require.NoError(t, err)
 
 	tests := []struct {
 		config        string
@@ -111,23 +116,26 @@ event_level: bla`,
 	}
 	subLogger := log.WithField("type", "windowseventlog")
 	for _, test := range tests {
-		f := WinEventLogSource{}
-		f.Configure([]byte(test.config), subLogger, configuration.METRICS_NONE)
-		q, err := f.buildXpathQuery()
-		if test.expectedErr != "" {
-			if err == nil {
-				t.Fatalf("expected error '%s' but got none", test.expectedErr)
+		t.Run(test.config, func(t *testing.T) {
+			f := WinEventLogSource{}
+
+			err := f.Configure([]byte(test.config), subLogger, configuration.METRICS_NONE)
+			cstest.AssertErrorContains(t, err, test.expectedErr)
+			if test.expectedErr != "" {
+				return
 			}
-			assert.Contains(t, err.Error(), test.expectedErr)
-		} else {
+
+			q, err := f.buildXpathQuery()
 			require.NoError(t, err)
 			assert.Equal(t, test.expectedQuery, q)
-		}
+		})
 	}
 }
 
 func TestLiveAcquisition(t *testing.T) {
-	exprhelpers.Init(nil)
+	err := exprhelpers.Init(nil)
+	require.NoError(t, err)
+
 	ctx := context.Background()
 
 	tests := []struct {
@@ -185,8 +193,13 @@ event_ids:
 		to := &tomb.Tomb{}
 		c := make(chan types.Event)
 		f := WinEventLogSource{}
-		f.Configure([]byte(test.config), subLogger, configuration.METRICS_NONE)
-		f.StreamingAcquisition(ctx, c, to)
+
+		err := f.Configure([]byte(test.config), subLogger, configuration.METRICS_NONE)
+		require.NoError(t, err)
+
+		err = f.StreamingAcquisition(ctx, c, to)
+		require.NoError(t, err)
+
 		time.Sleep(time.Second)
 		lines := test.expectedLines
 		go func() {
@@ -261,23 +274,22 @@ func TestOneShotAcquisition(t *testing.T) {
 		},
 	}
 
-	exprhelpers.Init(nil)
+	err := exprhelpers.Init(nil)
+	require.NoError(t, err)
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			lineCount := 0
 			to := &tomb.Tomb{}
 			c := make(chan types.Event)
 			f := WinEventLogSource{}
-			err := f.ConfigureByDSN(test.dsn, map[string]string{"type": "wineventlog"}, log.WithField("type", "windowseventlog"), "")
 
+			err := f.ConfigureByDSN(test.dsn, map[string]string{"type": "wineventlog"}, log.WithField("type", "windowseventlog"), "")
+			cstest.AssertErrorContains(t, err, test.expectedConfigureErr)
 			if test.expectedConfigureErr != "" {
-				assert.Contains(t, err.Error(), test.expectedConfigureErr)
 				return
 			}
 
-			require.NoError(t, err)
-
 			go func() {
 				for {
 					select {

pkg/cticlient/example/fire.go

@@ -57,6 +57,12 @@ func main() {
 			})
 		}
 	}
-	csvWriter.Write(csvHeader)
-	csvWriter.WriteAll(allItems)
+
+	if err = csvWriter.Write(csvHeader); err != nil {
+		panic(err)
+	}
+
+	if err = csvWriter.WriteAll(allItems); err != nil {
+		panic(err)
+	}
 }

pkg/exprhelpers/helpers.go

@@ -29,8 +29,6 @@ import (
 	"github.com/umahmood/haversine"
 	"github.com/wasilibs/go-re2"
 
-	"github.com/crowdsecurity/go-cs-lib/ptr"
-
 	"github.com/crowdsecurity/crowdsec/pkg/cache"
 	"github.com/crowdsecurity/crowdsec/pkg/database"
 	"github.com/crowdsecurity/crowdsec/pkg/fflag"
@@ -146,25 +144,27 @@ func RegexpCacheInit(filename string, cacheCfg types.DataSource) error {
 	}
 	// cache is enabled
 
-	if cacheCfg.Size == nil {
-		cacheCfg.Size = ptr.Of(50)
+	size := 50
+	if cacheCfg.Size != nil {
+		size = *cacheCfg.Size
 	}
 
-	gc := gcache.New(*cacheCfg.Size)
+	gc := gcache.New(size)
 
-	if cacheCfg.Strategy == nil {
-		cacheCfg.Strategy = ptr.Of("LRU")
+	strategy := "LRU"
+	if cacheCfg.Strategy != nil {
+		strategy = *cacheCfg.Strategy
 	}
 
-	switch *cacheCfg.Strategy {
+	switch strategy {
 	case "LRU":
 		gc = gc.LRU()
 	case "LFU":
 		gc = gc.LFU()
 	case "ARC":
 		gc = gc.ARC()
 	default:
-		return fmt.Errorf("unknown cache strategy '%s'", *cacheCfg.Strategy)
+		return fmt.Errorf("unknown cache strategy '%s'", strategy)
 	}
 
 	if cacheCfg.TTL != nil {

pkg/leakybucket/manager_load.go

@@ -458,7 +458,9 @@ func LoadBucket(bucketFactory *BucketFactory, tomb *tomb.Tomb) error {
 		}
 
 		if data.Type == "regexp" { // cache only makes sense for regexp
-			exprhelpers.RegexpCacheInit(data.DestPath, *data)
+			if err := exprhelpers.RegexpCacheInit(data.DestPath, *data); err != nil {
+				bucketFactory.logger.Error(err.Error())
+			}
 		}
 	}
 

pkg/parser/node.go

@@ -353,7 +353,9 @@ func (n *Node) process(p *types.Event, ctx UnixParserCtx, expressionEnv map[stri
 				clog.Warningf("unexpected type %t (%v) while running '%s'", output, output, stash.Key)
 				continue
 			}
-			cache.SetKey(stash.Name, key, value, &stash.TTLVal)
+			if err = cache.SetKey(stash.Name, key, value, &stash.TTLVal); err != nil {
+				clog.Warningf("failed to store data in cache: %s", err.Error())
+			}
 		}
 	}
 

pkg/parser/stage.go

@@ -114,10 +114,12 @@ func LoadStages(stageFiles []Stagefile, pctx *UnixParserCtx, ectx EnricherCtx) (
 			for _, data := range node.Data {
 				err = exprhelpers.FileInit(pctx.DataFolder, data.DestPath, data.Type)
 				if err != nil {
-					log.Error(err)
+					log.Error(err.Error())
 				}
 				if data.Type == "regexp" { //cache only makes sense for regexp
-					exprhelpers.RegexpCacheInit(data.DestPath, *data)
+					if err = exprhelpers.RegexpCacheInit(data.DestPath, *data); err != nil {
+						log.Error(err.Error())
+					}
 				}
 			}
 

pkg/setup/install.go

@@ -192,7 +192,9 @@ func marshalAcquisDocuments(ads []AcquisDocument, toDir string) (string, error)
 				return "", fmt.Errorf("while writing to %s: %w", ad.AcquisFilename, err)
 			}
 
-			f.Sync()
+			if err = f.Sync(); err != nil {
+				return "", fmt.Errorf("while syncing %s: %w", ad.AcquisFilename, err)
+			}
 
 			continue
 		}