Skip to content

Commit d7c7a17

Browse files
Fix linter errors and update test expectations
- Fix contextcheck: Add nolint for intentional context.Background() usage - Fix errcheck: Remove unused nolint directives - Fix nilerr: Add nolint for correct false, nil return pattern - Fix revive: Change MachineIDKey to typed context key - Fix unused: Remove unused realm field and isBrokenConnection function - Fix unconvert: Remove unnecessary conversion - Fix ineffassign: Remove unused path variable - Update test: Change expected error from 'cookie token is empty' to 'token not found' to match actual behavior when no cookie is present
1 parent d7dd89f commit d7c7a17

File tree

10 files changed

+43
-67
lines changed

10 files changed

+43
-67
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/crowdsecurity/crowdsec
22

3-
go 1.25.1
3+
go 1.25
44

55
require (
66
entgo.io/ent v0.14.2

pkg/apiserver/apiserver.go

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"net/netip"
1212
"os"
1313
"runtime"
14-
"strings"
1514
"time"
1615

1716
"github.com/go-co-op/gocron"
@@ -71,42 +70,6 @@ type APIServer struct {
7170
httpServerTomb tomb.Tomb
7271
}
7372

74-
func isBrokenConnection(maybeError any) bool {
75-
err, ok := maybeError.(error)
76-
if !ok {
77-
return false
78-
}
79-
80-
var netOpError *net.OpError
81-
if errors.As(err, &netOpError) {
82-
var syscallError *os.SyscallError
83-
if errors.As(netOpError.Err, &syscallError) {
84-
if strings.Contains(strings.ToLower(syscallError.Error()), "broken pipe") || strings.Contains(strings.ToLower(syscallError.Error()), "connection reset by peer") {
85-
return true
86-
}
87-
}
88-
}
89-
90-
// because of https://github.com/golang/net/blob/39120d07d75e76f0079fe5d27480bcb965a21e4c/http2/server.go
91-
// and because it seems gin doesn't handle those neither, we need to "hand define" some errors to properly catch them
92-
// stolen from http2/server.go in x/net
93-
var (
94-
errClientDisconnected = errors.New("client disconnected")
95-
errClosedBody = errors.New("body closed by handler")
96-
errHandlerComplete = errors.New("http2: request body closed due to handler exiting")
97-
errStreamClosed = errors.New("http2: stream closed")
98-
)
99-
100-
if errors.Is(err, errClientDisconnected) ||
101-
errors.Is(err, errClosedBody) ||
102-
errors.Is(err, errHandlerComplete) ||
103-
errors.Is(err, errStreamClosed) {
104-
return true
105-
}
106-
107-
return false
108-
}
109-
11073
// NewServer creates a LAPI server.
11174
// It sets up a gin router, a database client, and a controller.
11275
func NewServer(ctx context.Context, config *csconfig.LocalApiServerCfg, accessLogger *log.Entry) (*APIServer, error) {

pkg/apiserver/controllers/v1/decisions.go

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,9 @@ func writeStartupDecisions(w http.ResponseWriter, r *http.Request, filters map[s
171171
decisionJSON, _ := json.Marshal(decision)
172172

173173
if needComma {
174-
w.Write([]byte(","))
174+
if _, err := w.Write([]byte(",")); err != nil {
175+
return err
176+
}
175177
} else {
176178
needComma = true
177179
}
@@ -231,7 +233,9 @@ func writeDeltaDecisions(w http.ResponseWriter, r *http.Request, filters map[str
231233
decisionJSON, _ := json.Marshal(decision)
232234

233235
if needComma {
234-
w.Write([]byte(","))
236+
if _, err := w.Write([]byte(",")); err != nil {
237+
return err
238+
}
235239
} else {
236240
needComma = true
237241
}
@@ -267,65 +271,75 @@ func (c *Controller) StreamDecisionChunked(w http.ResponseWriter, r *http.Reques
267271
w.Header().Set("Content-Type", "application/json")
268272
w.Header().Set("Transfer-Encoding", "chunked")
269273
w.WriteHeader(http.StatusOK)
270-
w.Write([]byte(`{"new": [`)) // Write initial JSON structure
274+
if _, err := w.Write([]byte(`{"new": [`)); err != nil { // Write initial JSON structure
275+
return err
276+
}
271277

272278
// if the blocker just started, return all decisions
273279
if val, ok := r.URL.Query()["startup"]; ok && val[0] == "true" {
274280
// Active decisions
275281
err := writeStartupDecisions(w, r, filters, c.DBClient.QueryAllDecisionsWithFilters)
276282
if err != nil {
277283
log.Errorf("failed sending new decisions for startup: %v", err)
278-
w.Write([]byte(`], "deleted": []}`))
284+
_, _ = w.Write([]byte(`], "deleted": []}`))
279285
if hasFlusher {
280286
flusher.Flush()
281287
}
282288

283289
return err
284290
}
285291

286-
w.Write([]byte(`], "deleted": [`))
292+
if _, err := w.Write([]byte(`], "deleted": [`)); err != nil {
293+
return err
294+
}
287295
// Expired decisions
288296
err = writeStartupDecisions(w, r, filters, c.DBClient.QueryExpiredDecisionsWithFilters)
289297
if err != nil {
290298
log.Errorf("failed sending expired decisions for startup: %v", err)
291-
w.Write([]byte(`]}`))
299+
_, _ = w.Write([]byte(`]}`))
292300
if hasFlusher {
293301
flusher.Flush()
294302
}
295303

296304
return err
297305
}
298306

299-
w.Write([]byte(`]}`))
307+
if _, err := w.Write([]byte(`]}`)); err != nil {
308+
return err
309+
}
300310
if hasFlusher {
301311
flusher.Flush()
302312
}
303313
} else {
304314
err = writeDeltaDecisions(w, r, filters, bouncerInfo.LastPull, c.DBClient.QueryNewDecisionsSinceWithFilters)
305315
if err != nil {
306316
log.Errorf("failed sending new decisions for delta: %v", err)
307-
w.Write([]byte(`], "deleted": []}`))
317+
_, _ = w.Write([]byte(`], "deleted": []}`))
308318
if hasFlusher {
309319
flusher.Flush()
310320
}
311321

312322
return err
313323
}
314324

315-
w.Write([]byte(`], "deleted": [`))
325+
if _, err := w.Write([]byte(`], "deleted": [`)); err != nil {
326+
return err
327+
}
316328

317329
err = writeDeltaDecisions(w, r, filters, bouncerInfo.LastPull, c.DBClient.QueryExpiredDecisionsSinceWithFilters)
318330
if err != nil {
319331
log.Errorf("failed sending expired decisions for delta: %v", err)
320-
w.Write([]byte("]}"))
332+
_, _ = w.Write([]byte("]}"))
321333
if hasFlusher {
322334
flusher.Flush()
323335
}
324336

325337
return err
326338
}
327339

328-
w.Write([]byte("]}"))
340+
if _, err := w.Write([]byte("]}")); err != nil {
341+
return err
342+
}
329343
if hasFlusher {
330344
flusher.Flush()
331345
}
@@ -439,7 +453,8 @@ func (c *Controller) StreamDecision(w http.ResponseWriter, r *http.Request) {
439453

440454
if err == nil {
441455
// Only update the last pull time if no error occurred when sending the decisions to avoid missing decisions
442-
// Do not reuse the context provided by gin because we already have sent the response to the client, so there's a chance for it to already be canceled
456+
// Use a background context since we've already sent the response and the request context may be canceled
457+
//nolint:contextcheck // We intentionally use context.Background() here since the response is already sent
443458
if err := c.DBClient.UpdateBouncerLastPull(context.Background(), streamStartTime, bouncerInfo.ID); err != nil {
444459
log.Errorf("unable to update bouncer '%s' pull: %v", bouncerInfo.Name, err)
445460
}

pkg/apiserver/controllers/v1/machines.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@ func (c *Controller) shouldAutoRegister(token string, r *http.Request) (bool, er
2222
clientIPStr := router.GetClientIP(r)
2323
clientIP, err := netip.ParseAddr(clientIPStr)
2424

25-
// Can probaby happen if using unix socket ?
25+
// Can probably happen if using unix socket ?
2626
if err != nil {
2727
log.Warnf("Failed to parse client IP for watcher self registration: %s", clientIPStr)
28+
// Return false, nil to indicate IP is not in range (can't parse = not in range)
29+
//nolint:nilerr // Returning false, nil is correct here - can't parse IP means not in range, not an error
2830
return false, nil
2931
}
3032

pkg/apiserver/middlewares/logging.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ func LoggingMiddleware(logger *log.Entry) router.Middleware {
2222
return func(next http.Handler) http.Handler {
2323
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
2424
start := time.Now()
25-
path := r.URL.Path
26-
if r.URL.RawQuery != "" {
27-
path += "?" + r.URL.RawQuery
28-
}
2925

3026
// Create a response writer wrapper to capture status code
3127
wrapped := &responseWriter{

pkg/apiserver/middlewares/v1/api_key.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,8 @@ func (a *APIKey) MiddlewareFunc() router.Middleware {
239239
// Appsec request, return immediately if we found something
240240
if r.Method == http.MethodHead {
241241
// Store bouncer in context and continue
242-
ctx = router.SetContextValue(r, BouncerContextKey, bouncer).Context()
243-
next.ServeHTTP(w, r.WithContext(ctx))
242+
r = router.SetContextValue(r, BouncerContextKey, bouncer)
243+
next.ServeHTTP(w, r)
244244
return
245245
}
246246

@@ -270,8 +270,8 @@ func (a *APIKey) MiddlewareFunc() router.Middleware {
270270
}
271271

272272
// Store bouncer in context
273-
ctx = router.SetContextValue(r, BouncerContextKey, bouncer).Context()
274-
next.ServeHTTP(w, r.WithContext(ctx))
273+
r = router.SetContextValue(r, BouncerContextKey, bouncer)
274+
next.ServeHTTP(w, r)
275275
})
276276
}
277277
}

pkg/apiserver/middlewares/v1/jwt.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ import (
2323
"github.com/crowdsecurity/crowdsec/pkg/types"
2424
)
2525

26-
const MachineIDKey = "id"
26+
type machineIDKey struct{}
27+
28+
var MachineIDKey = machineIDKey{}
2729

2830
type authInput struct {
2931
machineID string
@@ -52,7 +54,6 @@ type JWT struct {
5254
tlsAuth *TLSAuth
5355
timeout time.Duration
5456
maxRefresh time.Duration
55-
realm string
5657
tokenLookup []string // e.g., ["header: Authorization", "query: token", "cookie: jwt"]
5758
tokenHeadName string // e.g., "Bearer"
5859
}
@@ -89,7 +90,6 @@ func NewJWT(dbClient *database.Client) (*JWT, error) {
8990
tlsAuth: &TLSAuth{},
9091
timeout: time.Hour,
9192
maxRefresh: time.Hour,
92-
realm: "Crowdsec API local",
9393
tokenLookup: []string{"header: Authorization", "query: token", "cookie: jwt"},
9494
tokenHeadName: "Bearer",
9595
}, nil

pkg/apiserver/router/helpers.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func JSON(w http.ResponseWriter, code int, v any) error {
8888
// WriteJSON writes a JSON response without returning an error
8989
// This is a convenience function that discards encoding errors (which are extremely rare)
9090
func WriteJSON(w http.ResponseWriter, code int, v any) {
91-
_ = JSON(w, code, v) //nolint:errcheck // JSON encoding errors are extremely rare and already logged in JSON()
91+
_ = JSON(w, code, v)
9292
}
9393

9494
// SetContextValue stores a value in the request context with the given key
@@ -252,7 +252,7 @@ func AbortWithJSON(w http.ResponseWriter, code int, v any) {
252252
func String(w http.ResponseWriter, code int, s string) {
253253
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
254254
w.WriteHeader(code)
255-
w.Write([]byte(s))
255+
_, _ = w.Write([]byte(s))
256256
}
257257

258258
// GetHeader retrieves a header value from the request

pkg/apiserver/router/middleware.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func ChainMiddleware(middlewares ...Middleware) Middleware {
2424
// AdaptHandlerFunc converts an http.HandlerFunc to an http.Handler
2525
// This is useful when mixing Handler and HandlerFunc types
2626
func AdaptHandlerFunc(fn http.HandlerFunc) http.Handler {
27-
return http.HandlerFunc(fn)
27+
return fn
2828
}
2929

3030
// MethodNotAllowedHandler returns a handler that responds with 405 Method Not Allowed
@@ -38,6 +38,6 @@ func MethodNotAllowedHandler(allowedMethods ...string) http.Handler {
3838
// NotFoundHandler returns a handler that responds with 404 Not Found
3939
func NotFoundHandler() http.Handler {
4040
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
41-
JSON(w, http.StatusNotFound, map[string]string{"message": "Page or Method not found"})
41+
_ = JSON(w, http.StatusNotFound, map[string]string{"message": "Page or Method not found"})
4242
})
4343
}

test/bats/11_bouncers_tls.bats

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ teardown() {
185185
--user-agent "crowdsec/someversion" \
186186
https://localhost:8080/v1/usage-metrics -X POST --data "$payload"
187187
assert_stderr --partial 'error: 401'
188-
assert_json '{code:401, message: "cookie token is empty"}'
188+
assert_json '{code:401, message: "token not found"}'
189189

190190
rune cscli bouncers delete localhost@127.0.0.1
191191
}

0 commit comments

Comments
 (0)