From 559aef8157a1bb1577bb616f8fbeec075e85da8d Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Thu, 10 Aug 2023 09:22:45 +0200 Subject: [PATCH 1/6] Enable gosec scanning --- .golangci.yml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..5384d968 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,3 @@ +linters: + enable: + - gosec From a345ef4d44dbace841c66f1d05d53c5546a1dbca Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Thu, 10 Aug 2023 09:23:06 +0200 Subject: [PATCH 2/6] Address "G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec)" --- cmd/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/root.go b/cmd/root.go index 81921bbf..277f38ca 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -152,7 +152,7 @@ func ensureToken(ctx context.Context, requiredScopes []string, signals chan os.S // Start the webserver log.WithContext(ctx).Trace("Starting webserver to listen for callback, press Ctrl+C to cancel") - srv := &http.Server{Addr: ":7837"} + srv := &http.Server{Addr: ":7837", ReadHeaderTimeout: 30 * time.Second} http.HandleFunc("/oauth/callback", handler) go func() { From c697dda137631a2996ca8b9bf24a7ff7436c6f1a Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Thu, 10 Aug 2023 09:25:05 +0200 Subject: [PATCH 3/6] Enable all `bugs` linters --- .golangci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5384d968..fa040781 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,3 +1,3 @@ linters: - enable: - - gosec + presets: + - bugs From 5ade62938752344ee50f64c9a96717a0141fb931 Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Thu, 10 Aug 2023 09:29:05 +0200 Subject: [PATCH 4/6] go vet is part of golangci-lint --- .github/workflows/tests.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 3bc62349..60dc95a0 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -19,9 +19,6 @@ jobs: run: | go get -v -t -d ./... - - name: Go Vet - run: go vet ./... - - name: Go Test run: | go run main.go --version From 4c365d2b26b552ceec3bd39b7d8c3aa66c29d850 Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Thu, 10 Aug 2023 09:41:39 +0200 Subject: [PATCH 5/6] Address all `bugs`-level lints --- cmd/createbookmark.go | 8 ++++++-- cmd/getbookmark.go | 8 ++++++-- cmd/getchange.go | 9 +++++++-- cmd/request.go | 4 ++++ cmd/root.go | 6 +++--- cmd/submitplan.go | 4 ++++ tracing/main.go | 1 + 7 files changed, 31 insertions(+), 9 deletions(-) diff --git a/cmd/createbookmark.go b/cmd/createbookmark.go index 9d7550f5..f97e820b 100644 --- a/cmd/createbookmark.go +++ b/cmd/createbookmark.go @@ -118,8 +118,12 @@ func CreateBookmark(signals chan os.Signal, ready chan bool) int { }).Info("created bookmark excluded item") } - b, _ := json.MarshalIndent(response.Msg.Bookmark.Properties, "", " ") - log.Info(string(b)) + b, err := json.MarshalIndent(response.Msg.Bookmark.Properties, "", " ") + if err != nil { + log.Infof("Error rendering bookmark: %v", err) + } else { + log.Info(string(b)) + } return 0 } diff --git a/cmd/getbookmark.go b/cmd/getbookmark.go index 6fc64ed6..d94bdba0 100644 --- a/cmd/getbookmark.go +++ b/cmd/getbookmark.go @@ -101,8 +101,12 @@ func GetBookmark(signals chan os.Signal, ready chan bool) int { }).Info("found bookmark excluded item") } - b, _ := json.MarshalIndent(response.Msg.Bookmark.Properties, "", " ") - log.Info(string(b)) + b, err := json.MarshalIndent(response.Msg.Bookmark.Properties, "", " ") + if err != nil { + log.Infof("Error rendering bookmark: %v", err) + } else { + log.Info(string(b)) + } return 0 } diff --git a/cmd/getchange.go b/cmd/getchange.go index a2616539..f954d1b3 100644 --- a/cmd/getchange.go +++ b/cmd/getchange.go @@ -97,8 +97,13 @@ func GetChange(signals chan os.Signal, ready chan bool) int { switch viper.GetString("format") { case "json": - b, _ := json.MarshalIndent(response.Msg.Change, "", " ") - fmt.Println(string(b)) + b, err := json.MarshalIndent(response.Msg.Change, "", " ") + if err != nil { + log.Errorf("Error rendering bookmark: %v", err) + return 1 + } else { + fmt.Println(string(b)) + } case "markdown": changeUrl := fmt.Sprintf("%v/changes/%v", viper.GetString("frontend"), changeUuid.String()) if response.Msg.Change.Metadata.NumAffectedApps != 0 || response.Msg.Change.Metadata.NumAffectedItems != 0 { diff --git a/cmd/request.go b/cmd/request.go index 563f3f11..3f8aa019 100644 --- a/cmd/request.go +++ b/cmd/request.go @@ -88,6 +88,8 @@ func Request(signals chan os.Signal, ready chan bool) int { HTTPClient: NewAuthenticatedClient(ctx, otelhttp.DefaultClient), } + // nhooyr.io/websocket reads the body internally + // nolint: bodyclose c, _, err := websocket.Dial(ctx, gatewayUrl, options) if err != nil { log.WithContext(ctx).WithFields(lf).WithError(err).Error("Failed to connect to overmind API") @@ -197,6 +199,8 @@ responses: statusFields["query"] = queryUuid switch status.Status { + case sdp.QueryStatus_UNSPECIFIED: + statusFields["unexpected_status"] = true case sdp.QueryStatus_STARTED: activeQueries[*queryUuid] = true case sdp.QueryStatus_FINISHED: diff --git a/cmd/root.go b/cmd/root.go index 277f38ca..2ed9895f 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -192,18 +192,18 @@ func getChangeUuid(ctx context.Context, expectedStatus sdp.ChangeStatus, errNotF if viper.GetString("uuid") != "" { changeUuid, err = uuid.Parse(viper.GetString("uuid")) if err != nil { - return uuid.Nil, fmt.Errorf("invalid --uuid value '%v', error: %v", viper.GetString("uuid"), err) + return uuid.Nil, fmt.Errorf("invalid --uuid value '%v', error: %w", viper.GetString("uuid"), err) } } if viper.GetString("change") != "" { changeUrl, err := url.ParseRequestURI(viper.GetString("change")) if err != nil { - return uuid.Nil, fmt.Errorf("invalid --change value '%v', error: %v", viper.GetString("change"), err) + return uuid.Nil, fmt.Errorf("invalid --change value '%v', error: %w", viper.GetString("change"), err) } changeUuid, err = uuid.Parse(path.Base(changeUrl.Path)) if err != nil { - return uuid.Nil, fmt.Errorf("invalid --change value '%v', couldn't parse: %v", viper.GetString("change"), err) + return uuid.Nil, fmt.Errorf("invalid --change value '%v', couldn't parse: %w", viper.GetString("change"), err) } } diff --git a/cmd/submitplan.go b/cmd/submitplan.go index e2bda661..4b711731 100644 --- a/cmd/submitplan.go +++ b/cmd/submitplan.go @@ -276,6 +276,8 @@ func SubmitPlan(signals chan os.Signal, ready chan bool) int { } log.WithContext(ctx).WithFields(lf).WithField("item_count", len(queries)).Info("identifying items") + // nhooyr.io/websocket reads the body internally + // nolint: bodyclose c, _, err := websocket.Dial(ctx, viper.GetString("gateway-url"), options) if err != nil { log.WithContext(ctx).WithFields(lf).WithError(err).Error("Failed to connect to overmind API") @@ -399,6 +401,8 @@ func SubmitPlan(signals chan os.Signal, ready chan bool) int { statusFields["query"] = queryUuid switch status.Status { + case sdp.QueryStatus_UNSPECIFIED: + statusFields["unexpected_status"] = true case sdp.QueryStatus_STARTED: activeQueries[*queryUuid] = true case sdp.QueryStatus_FINISHED: diff --git a/tracing/main.go b/tracing/main.go index 346a375d..61c2ae5a 100644 --- a/tracing/main.go +++ b/tracing/main.go @@ -147,6 +147,7 @@ func InitTracer(opts ...otlptracehttp.Option) error { return nil } +// nolint: contextcheck func ShutdownTracer() { // Flush buffered events before the program terminates. defer sentry.Flush(5 * time.Second) From bfe1dc71007cf24b1bb1ccdc87a71191b269ba32 Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Thu, 10 Aug 2023 11:32:19 +0200 Subject: [PATCH 6/6] Improve nolint comments --- cmd/request.go | 3 +-- cmd/submitplan.go | 3 +-- tracing/main.go | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/cmd/request.go b/cmd/request.go index 3f8aa019..ef30f64f 100644 --- a/cmd/request.go +++ b/cmd/request.go @@ -88,8 +88,7 @@ func Request(signals chan os.Signal, ready chan bool) int { HTTPClient: NewAuthenticatedClient(ctx, otelhttp.DefaultClient), } - // nhooyr.io/websocket reads the body internally - // nolint: bodyclose + // nolint: bodyclose // nhooyr.io/websocket reads the body internally c, _, err := websocket.Dial(ctx, gatewayUrl, options) if err != nil { log.WithContext(ctx).WithFields(lf).WithError(err).Error("Failed to connect to overmind API") diff --git a/cmd/submitplan.go b/cmd/submitplan.go index 4b711731..c9223cb8 100644 --- a/cmd/submitplan.go +++ b/cmd/submitplan.go @@ -276,8 +276,7 @@ func SubmitPlan(signals chan os.Signal, ready chan bool) int { } log.WithContext(ctx).WithFields(lf).WithField("item_count", len(queries)).Info("identifying items") - // nhooyr.io/websocket reads the body internally - // nolint: bodyclose + // nolint: bodyclose // nhooyr.io/websocket reads the body internally c, _, err := websocket.Dial(ctx, viper.GetString("gateway-url"), options) if err != nil { log.WithContext(ctx).WithFields(lf).WithError(err).Error("Failed to connect to overmind API") diff --git a/tracing/main.go b/tracing/main.go index 61c2ae5a..4e158675 100644 --- a/tracing/main.go +++ b/tracing/main.go @@ -147,7 +147,7 @@ func InitTracer(opts ...otlptracehttp.Option) error { return nil } -// nolint: contextcheck +// nolint: contextcheck // deliberate use of local context to avoid getting tangled up in any existing timeouts or cancels func ShutdownTracer() { // Flush buffered events before the program terminates. defer sentry.Flush(5 * time.Second)