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

fix: goagent gosec issues #215

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
5 changes: 4 additions & 1 deletion examples/gin-server/main.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"log"
"net/http"

"github.com/gin-gonic/gin"
Expand Down Expand Up @@ -36,5 +37,7 @@ func main() {

r := setupRouter()
// Listen and Server in 0.0.0.0:8080
r.Run(":8080")
if err := r.Run(":8080"); err != nil {
log.Fatalf("error running server: %v", err)
}
}
18 changes: 13 additions & 5 deletions examples/mux-server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@ import (
"net/http"
"time"

"github.com/hypertrace/goagent/config"
sdkhttp "github.com/hypertrace/goagent/sdk/instrumentation/net/http"

"github.com/gorilla/mux"
"github.com/hypertrace/goagent/config"
"github.com/hypertrace/goagent/instrumentation/hypertrace"
"github.com/hypertrace/goagent/instrumentation/opentelemetry/github.com/gorilla/hypermux"
sdkhttp "github.com/hypertrace/goagent/sdk/instrumentation/net/http"
)

func main() {
Expand All @@ -26,7 +25,14 @@ func main() {
r := mux.NewRouter()
r.Use(hypermux.NewMiddleware(&sdkhttp.Options{})) // here we use the mux middleware
r.HandleFunc("/foo", http.HandlerFunc(fooHandler))
log.Fatal(http.ListenAndServe(":8081", r))
srv := http.Server{
Addr: ":8081",
Handler: r,
ReadTimeout: 60 * time.Second,
WriteTimeout: 60 * time.Second,
ReadHeaderTimeout: 60 * time.Second,
}
log.Fatal(srv.ListenAndServe())
}

type person struct {
Expand All @@ -52,5 +58,7 @@ func fooHandler(w http.ResponseWriter, r *http.Request) {

w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
w.Write([]byte(fmt.Sprintf("{\"message\": \"Hello %s\"}", p.Name)))
if _, err := w.Write([]byte(fmt.Sprintf("{\"message\": \"Hello %s\"}", p.Name))); err != nil {
log.Printf("error while writing response body: %v", err)
}
}
5 changes: 4 additions & 1 deletion examples/postgres-query/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import (
"fmt"
"os"

"github.com/hypertrace/goagent/instrumentation/hypertrace/github.com/jackc/hyperpgx"
// gosec complains about this pkg not following golang repo standards
// "could not import github.com/hypertrace/goagent/instrumentation/hypertrace/github.com/jackc/hyperpgx (invalid package name: "")"
// It is caused the pkg having its own go.mod
"github.com/hypertrace/goagent/instrumentation/hypertrace/github.com/jackc/hyperpgx" // #nosec
)

func main() {
Expand Down
19 changes: 16 additions & 3 deletions examples/sql-query/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import (
"net/http"
"time"

"github.com/go-sql-driver/mysql"
// gosec complains about github.com/go-sql-driver/mysql not following golang repo standards
// "could not import github.com/go-sql-driver/mysql (invalid package name: "")"
"github.com/go-sql-driver/mysql" // #nosec
"github.com/gorilla/mux"
"github.com/hypertrace/goagent/config"
"github.com/hypertrace/goagent/instrumentation/hypertrace"
Expand Down Expand Up @@ -40,7 +42,16 @@ func main() {
http.HandlerFunc(fooHandlerFunc),
"/foo",
))
log.Fatal(http.ListenAndServe(":8081", r))
// Using log.Fatal(http.ListenAndServe(":8081", r)) causes a gosec timeout error.
// G114 (CWE-676): Use of net/http serve function that has no support for setting timeouts (Confidence: HIGH, Severity: MEDIUM)
srv := http.Server{
Addr: ":8081",
Handler: r,
ReadTimeout: 60 * time.Second,
WriteTimeout: 60 * time.Second,
ReadHeaderTimeout: 60 * time.Second,
}
log.Fatal(srv.ListenAndServe())
}

type person struct {
Expand Down Expand Up @@ -68,7 +79,9 @@ func fooHandler(db *sql.DB, w http.ResponseWriter, r *http.Request) {

w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
w.Write([]byte(fmt.Sprintf("{\"message\": \"Hello %s\"}", p.Name)))
if _, err := w.Write([]byte(fmt.Sprintf("{\"message\": \"Hello %s\"}", p.Name))); err != nil {
log.Printf("errow while writing response body: %v", err)
}
}

func dbConn() (db *sql.DB) {
Expand Down
4 changes: 3 additions & 1 deletion instrumentation/opencensus/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@

client := &http.Client{Transport: &http.Transport{
TLSClientConfig: &tls.Config{
MinVersion: tls.VersionTLS12,
MinVersion: tls.VersionTLS12,
// Ignore gosec: G402 (CWE-295): TLS InsecureSkipVerify may be true.
// #nosec G402

Check warning on line 26 in instrumentation/opencensus/init.go

View check run for this annotation

Codecov / codecov/patch

instrumentation/opencensus/init.go#L24-L26

Added lines #L24 - L26 were not covered by tests
InsecureSkipVerify: !cfg.GetReporting().GetSecure().GetValue(),
},
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/hypertrace/goagent/sdk"
"github.com/jackc/pgconn"
"github.com/jackc/pgtype/pgxtype"
"github.com/jackc/pgx/v4"
pgx "github.com/jackc/pgx/v4"
)

var _ PGXConn = (*pgx.Conn)(nil)
Expand Down
7 changes: 6 additions & 1 deletion sdk/instrumentation/net/http/attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

import (
"fmt"
"log"
"strings"

"github.com/hypertrace/goagent/sdk"
)

// SetAttributesFromHeaders set attributes into span from a HeaderAccessor
func SetAttributesFromHeaders(_type string, headers HeaderAccessor, span sdk.Span) {
headers.ForEachHeader(func(key string, values []string) error {
err := headers.ForEachHeader(func(key string, values []string) error {
if len(values) == 1 {
span.SetAttribute(
fmt.Sprintf("http.%s.header.%s", _type, strings.ToLower(key)),
Expand All @@ -26,4 +27,8 @@
}
return nil
})

if err != nil {
log.Printf("error occurred while setting attributes from headers: %v\n", err)
}

Check warning on line 33 in sdk/instrumentation/net/http/attributes.go

View check run for this annotation

Codecov / codecov/patch

sdk/instrumentation/net/http/attributes.go#L32-L33

Added lines #L32 - L33 were not covered by tests
}
Loading