Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lint: enable errcheck; add allowlist and explicit checks #3403

Merged
merged 5 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
8 changes: 5 additions & 3 deletions cmd/crowdsec-cli/climachine/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@
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
}

Check warning on line 78 in cmd/crowdsec-cli/climachine/add.go

View check run for this annotation

Codecov / codecov/patch

cmd/crowdsec-cli/climachine/add.go#L76-L78

Added lines #L76 - L78 were not covered by tests
}

password := strfmt.Password(machinePassword)
Expand Down Expand Up @@ -147,9 +149,9 @@
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
}
8 changes: 4 additions & 4 deletions cmd/crowdsec-cli/completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
},
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/crowdsec/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/acquisition/modules/appsec/appsec.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,10 @@
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())
}

Check warning on line 336 in pkg/acquisition/modules/appsec/appsec.go

View check run for this annotation

Codecov / codecov/patch

pkg/acquisition/modules/appsec/appsec.go#L333-L336

Added lines #L333 - L336 were not covered by tests

return nil
})
Expand Down
4 changes: 3 additions & 1 deletion pkg/acquisition/modules/kubernetesaudit/k8s_audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@
})
<-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())
}

Check warning on line 159 in pkg/acquisition/modules/kubernetesaudit/k8s_audit.go

View check run for this annotation

Codecov / codecov/patch

pkg/acquisition/modules/kubernetesaudit/k8s_audit.go#L158-L159

Added lines #L158 - L159 were not covered by tests

return nil
})
Expand Down
56 changes: 34 additions & 22 deletions pkg/acquisition/modules/wineventlog/wineventlog_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 8 additions & 2 deletions pkg/cticlient/example/fire.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@
})
}
}
csvWriter.Write(csvHeader)
csvWriter.WriteAll(allItems)

if err = csvWriter.Write(csvHeader); err != nil {
panic(err)

Check warning on line 62 in pkg/cticlient/example/fire.go

View check run for this annotation

Codecov / codecov/patch

pkg/cticlient/example/fire.go#L61-L62

Added lines #L61 - L62 were not covered by tests
}

if err = csvWriter.WriteAll(allItems); err != nil {
panic(err)

Check warning on line 66 in pkg/cticlient/example/fire.go

View check run for this annotation

Codecov / codecov/patch

pkg/cticlient/example/fire.go#L65-L66

Added lines #L65 - L66 were not covered by tests
}
}
18 changes: 9 additions & 9 deletions pkg/exprhelpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@
"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"
Expand Down Expand Up @@ -146,25 +144,27 @@
}
// 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

Check warning on line 156 in pkg/exprhelpers/helpers.go

View check run for this annotation

Codecov / codecov/patch

pkg/exprhelpers/helpers.go#L156

Added line #L156 was not covered by tests
}

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)

Check warning on line 167 in pkg/exprhelpers/helpers.go

View check run for this annotation

Codecov / codecov/patch

pkg/exprhelpers/helpers.go#L167

Added line #L167 was not covered by tests
}

if cacheCfg.TTL != nil {
Expand Down
4 changes: 3 additions & 1 deletion pkg/leakybucket/manager_load.go
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,9 @@
}

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())
}

Check warning on line 463 in pkg/leakybucket/manager_load.go

View check run for this annotation

Codecov / codecov/patch

pkg/leakybucket/manager_load.go#L461-L463

Added lines #L461 - L463 were not covered by tests
}
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/parser/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,9 @@
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())
}

Check warning on line 358 in pkg/parser/node.go

View check run for this annotation

Codecov / codecov/patch

pkg/parser/node.go#L357-L358

Added lines #L357 - L358 were not covered by tests
}
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/parser/stage.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,12 @@
for _, data := range node.Data {
err = exprhelpers.FileInit(pctx.DataFolder, data.DestPath, data.Type)
if err != nil {
log.Error(err)
log.Error(err.Error())

Check warning on line 117 in pkg/parser/stage.go

View check run for this annotation

Codecov / codecov/patch

pkg/parser/stage.go#L117

Added line #L117 was not covered by tests
}
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())
}

Check warning on line 122 in pkg/parser/stage.go

View check run for this annotation

Codecov / codecov/patch

pkg/parser/stage.go#L120-L122

Added lines #L120 - L122 were not covered by tests
}
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/setup/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,9 @@
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)
}

Check warning on line 197 in pkg/setup/install.go

View check run for this annotation

Codecov / codecov/patch

pkg/setup/install.go#L196-L197

Added lines #L196 - L197 were not covered by tests

continue
}
Expand Down
Loading