Skip to content

Commit

Permalink
Fix G402 reports with latest gosec version
Browse files Browse the repository at this point in the history
Ref: securego/gosec#1044 (comment)

Break up transport setup so that the `InsecureSkipVerify` is a single line, so
that their latest changes can find the ignore statement at the exact statement
level to be properly ignored.

Refactor test code to only have the test client setup once.
  • Loading branch information
HeavyWombat committed Oct 26, 2023
1 parent 31e7555 commit 4596605
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 87 deletions.
16 changes: 7 additions & 9 deletions pkg/image/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,15 @@ func GetOptions(ctx context.Context, imageName name.Reference, insecure bool, do
options = append(options, remote.WithContext(ctx))

transport := http.DefaultTransport.(*http.Transport).Clone()
transport.TLSClientConfig = &tls.Config{
MinVersion: tls.VersionTLS12,
InsecureSkipVerify: false,
}

if insecure {
// #nosec:G402 explicitly requested by user to use insecure registry
transport.TLSClientConfig = &tls.Config{
InsecureSkipVerify: true,
}
} else {
transport.TLSClientConfig = &tls.Config{
InsecureSkipVerify: false,
MinVersion: tls.VersionTLS12,
}
// #nosec:G402 insecure is explicitly requested by user, make sure to skip verification and reset empty defaults
transport.TLSClientConfig.InsecureSkipVerify = insecure
transport.TLSClientConfig.MinVersion = 0
}

// find a Docker config.json
Expand Down
29 changes: 3 additions & 26 deletions test/utils/v1alpha1/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

"github.com/shipwright-io/build/pkg/webhook/conversion"
"github.com/shipwright-io/build/test/utils"

"github.com/onsi/ginkgo/v2"
"github.com/onsi/gomega"
Expand Down Expand Up @@ -51,20 +52,8 @@ func StartBuildWebhook() *http.Server {
}
}()

client := &http.Client{
Transport: &http.Transport{
IdleConnTimeout: 5 * time.Second,
ResponseHeaderTimeout: 5 * time.Second,
// #nosec:G402 test code
TLSClientConfig: &tls.Config{
InsecureSkipVerify: true,
},
TLSHandshakeTimeout: 5 * time.Second,
},
}

gomega.Eventually(func() int {
r, err := client.Get("https://localhost:30443/health")
r, err := utils.TestClient().Get("https://localhost:30443/health")
if err != nil {
return 0
}
Expand All @@ -81,20 +70,8 @@ func StopBuildWebhook(webhookServer *http.Server) {
err := webhookServer.Close()
gomega.Expect(err).ToNot(gomega.HaveOccurred())

client := &http.Client{
Transport: &http.Transport{
IdleConnTimeout: 5 * time.Second,
ResponseHeaderTimeout: 5 * time.Second,
// #nosec:G402 test code
TLSClientConfig: &tls.Config{
InsecureSkipVerify: true,
},
TLSHandshakeTimeout: 5 * time.Second,
},
}

gomega.Eventually(func() int {
r, err := client.Get("https://localhost:30443/health")
r, err := utils.TestClient().Get("https://localhost:30443/health")
if err != nil {
return 0
}
Expand Down
29 changes: 3 additions & 26 deletions test/utils/v1beta1/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

"github.com/shipwright-io/build/pkg/webhook/conversion"
"github.com/shipwright-io/build/test/utils"

"github.com/onsi/ginkgo/v2"
"github.com/onsi/gomega"
Expand Down Expand Up @@ -51,20 +52,8 @@ func StartBuildWebhook() *http.Server {
}
}()

client := &http.Client{
Transport: &http.Transport{
IdleConnTimeout: 5 * time.Second,
ResponseHeaderTimeout: 5 * time.Second,
// #nosec:G402 test code
TLSClientConfig: &tls.Config{
InsecureSkipVerify: true,
},
TLSHandshakeTimeout: 5 * time.Second,
},
}

gomega.Eventually(func() int {
r, err := client.Get("https://localhost:30443/health")
r, err := utils.TestClient().Get("https://localhost:30443/health")
if err != nil {
return 0
}
Expand All @@ -81,20 +70,8 @@ func StopBuildWebhook(webhookServer *http.Server) {
err := webhookServer.Close()
gomega.Expect(err).ToNot(gomega.HaveOccurred())

client := &http.Client{
Transport: &http.Transport{
IdleConnTimeout: 5 * time.Second,
ResponseHeaderTimeout: 5 * time.Second,
// #nosec:G402 test code
TLSClientConfig: &tls.Config{
InsecureSkipVerify: true,
},
TLSHandshakeTimeout: 5 * time.Second,
},
}

gomega.Eventually(func() int {
r, err := client.Get("https://localhost:30443/health")
r, err := utils.TestClient().Get("https://localhost:30443/health")
if err != nil {
return 0
}
Expand Down
46 changes: 20 additions & 26 deletions test/utils/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,24 @@ import (
"github.com/onsi/gomega"
)

func TestClient() *http.Client {
transport := &http.Transport{
IdleConnTimeout: 5 * time.Second,
ResponseHeaderTimeout: 5 * time.Second,
TLSHandshakeTimeout: 5 * time.Second,
TLSClientConfig: &tls.Config{
MinVersion: tls.VersionTLS12,
},
}

// #nosec:G402 test code
transport.TLSClientConfig.InsecureSkipVerify = true

return &http.Client{
Transport: transport,
}
}

func StartBuildWebhook() *http.Server {
mux := http.NewServeMux()
mux.HandleFunc("/convert", conversion.CRDConvertHandler(context.Background()))
Expand Down Expand Up @@ -51,20 +69,8 @@ func StartBuildWebhook() *http.Server {
}
}()

client := &http.Client{
Transport: &http.Transport{
IdleConnTimeout: 5 * time.Second,
ResponseHeaderTimeout: 5 * time.Second,
// #nosec:G402 test code
TLSClientConfig: &tls.Config{
InsecureSkipVerify: true,
},
TLSHandshakeTimeout: 5 * time.Second,
},
}

gomega.Eventually(func() int {
r, err := client.Get("https://localhost:30443/health")
r, err := TestClient().Get("https://localhost:30443/health")
if err != nil {
return 0
}
Expand All @@ -81,20 +87,8 @@ func StopBuildWebhook(webhookServer *http.Server) {
err := webhookServer.Close()
gomega.Expect(err).ToNot(gomega.HaveOccurred())

client := &http.Client{
Transport: &http.Transport{
IdleConnTimeout: 5 * time.Second,
ResponseHeaderTimeout: 5 * time.Second,
// #nosec:G402 test code
TLSClientConfig: &tls.Config{
InsecureSkipVerify: true,
},
TLSHandshakeTimeout: 5 * time.Second,
},
}

gomega.Eventually(func() int {
r, err := client.Get("https://localhost:30443/health")
r, err := TestClient().Get("https://localhost:30443/health")
if err != nil {
return 0
}
Expand Down

0 comments on commit 4596605

Please sign in to comment.