From b8acfd882e2a970de380bcd20bbb0c35a3e384e9 Mon Sep 17 00:00:00 2001 From: Paul Abel Date: Tue, 27 Feb 2024 12:18:18 +0000 Subject: [PATCH 1/2] address gosec G101 - hardcoded credentials issues --- .golangci.yml | 5 +++++ internal/configs/configurator.go | 2 +- internal/k8s/secrets/validation.go | 6 +++--- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 86ad2ab504..0c14a26c50 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -61,5 +61,10 @@ issues: max-issues-per-linter: 0 max-same-issues: 0 exclude-use-default: false + exclude-rules: + - path: _test\.go + text: "Potential hardcoded credentials" + linters: + - gosec run: timeout: 5m diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index f060eb9039..bbf48e212a 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -41,7 +41,7 @@ const ( ) // DefaultServerSecretPath is the full path to the Secret with a TLS cert and a key for the default server. #nosec G101 -const DefaultServerSecretPath = "/etc/nginx/secrets/default" +const DefaultServerSecretPath = "/etc/nginx/secrets/default" //nolint:gosec // G101: Potential hardcoded credentials - false positive // DefaultSecretPath is the full default path to where secrets are stored and accessed. const DefaultSecretPath = "/etc/nginx/secrets" // #nosec G101 diff --git a/internal/k8s/secrets/validation.go b/internal/k8s/secrets/validation.go index 7599d92169..80ffdfe22b 100644 --- a/internal/k8s/secrets/validation.go +++ b/internal/k8s/secrets/validation.go @@ -23,13 +23,13 @@ const ClientSecretKey = "client-secret" const HtpasswdFileKey = "htpasswd" // SecretTypeCA contains a certificate authority for TLS certificate verification. #nosec G101 -const SecretTypeCA api_v1.SecretType = "nginx.org/ca" +const SecretTypeCA api_v1.SecretType = "nginx.org/ca" //nolint:gosec // G101: Potential hardcoded credentials - false positive // SecretTypeJWK contains a JWK (JSON Web Key) for validating JWTs (JSON Web Tokens). #nosec G101 -const SecretTypeJWK api_v1.SecretType = "nginx.org/jwk" +const SecretTypeJWK api_v1.SecretType = "nginx.org/jwk" //nolint:gosec // G101: Potential hardcoded credentials - false positive // SecretTypeOIDC contains an OIDC client secret for use in oauth flows. #nosec G101 -const SecretTypeOIDC api_v1.SecretType = "nginx.org/oidc" +const SecretTypeOIDC api_v1.SecretType = "nginx.org/oidc" //nolint:gosec // G101: Potential hardcoded credentials - false positive // SecretTypeHtpasswd contains an htpasswd file for use in HTTP Basic authorization.. #nosec G101 const SecretTypeHtpasswd api_v1.SecretType = "nginx.org/htpasswd" // #nosec G101 From e3efe8a5f7136a3909fc71e912de6444650c875f Mon Sep 17 00:00:00 2001 From: Paul Abel Date: Tue, 27 Feb 2024 12:41:44 +0000 Subject: [PATCH 2/2] address G601: Implicit memory aliasing in for loop. (gosec) --- internal/configs/configurator.go | 1 + internal/k8s/controller.go | 2 ++ internal/k8s/controller_test.go | 7 +++++++ internal/k8s/status_test.go | 3 +++ internal/k8s/validation.go | 1 + pkg/apis/configuration/validation/virtualserver.go | 1 + pkg/apis/configuration/validation/virtualserver_test.go | 7 ++++--- pkg/apis/externaldns/validation/externaldns_test.go | 2 ++ 8 files changed, 21 insertions(+), 3 deletions(-) diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index bbf48e212a..8f057e1346 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -1166,6 +1166,7 @@ func (cnf *Configurator) updatePlusEndpoints(ingEx *IngressEx) error { } for _, path := range rule.HTTP.Paths { + path := path // address gosec G601 endps, exists := ingEx.Endpoints[path.Backend.Service.Name+GetBackendPortAsString(path.Backend.Service.Port)] if exists { if _, isExternalName := ingEx.ExternalNameSvcs[path.Backend.Service.Name]; isExternalName { diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 12748a8982..b54cc855c5 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -1171,6 +1171,7 @@ func (lbc *LoadBalancerController) cleanupUnwatchedNamespacedResources(nsi *name glog.Warningf("unable to list Ingress resources for recently unwatched namespace %s", nsi.namespace) } else { for _, ing := range il.Items { + ing := ing // address gosec G601 key := getResourceKey(&ing.ObjectMeta) delIngressList = append(delIngressList, key) lbc.configuration.DeleteIngress(key) @@ -2908,6 +2909,7 @@ func (lbc *LoadBalancerController) createIngressEx(ing *networking.Ingress, vali } for _, path := range rule.HTTP.Paths { + path := path // address gosec G601 podEndps := []podEndpoint{} if validMinionPaths != nil && !validMinionPaths[path.Path] { glog.V(3).Infof("Skipping path %s for minion Ingress %s", path.Path, ing.Name) diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index 489700f708..cb8b30f274 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -579,6 +579,7 @@ func TestGetEndpointsFromEndpointSlices_DuplicateEndpointsInOneEndpointSlice(t * } for _, test := range tests { + test := test // address gosec G601 t.Run(test.desc, func(t *testing.T) { gotEndpoints, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc) if err != nil { @@ -670,6 +671,7 @@ func TestGetEndpointsFromEndpointSlices_TwoDifferentEndpointsInOnEndpointSlice(t } for _, test := range tests { + test := test // address gosec G601 t.Run(test.desc, func(t *testing.T) { gotEndpoints, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc) if err != nil { @@ -790,6 +792,7 @@ func TestGetEndpointsFromEndpointSlices_DuplicateEndpointsAcrossTwoEndpointSlice } for _, test := range tests { + test := test // address gosec G601 t.Run(test.desc, func(t *testing.T) { gotEndpoints, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc) if err != nil { @@ -879,6 +882,7 @@ func TestGetEndpointsFromEndpointSlices_TwoDifferentEndpointsInOnEndpointSliceOn } for _, test := range tests { + test := test // address gosec G601 t.Run(test.desc, func(t *testing.T) { gotEndpoints, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc) if err != nil { @@ -978,6 +982,7 @@ func TestGetEndpointsFromEndpointSlices_TwoDifferentEndpointsAcrossTwoEndpointSl } for _, test := range tests { + test := test // address gosec G601 t.Run(test.desc, func(t *testing.T) { gotEndpoints, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc) if err != nil { @@ -1053,6 +1058,7 @@ func TestGetEndpointsFromEndpointSlices_ErrorsOnInvalidTargetPort(t *testing.T) } for _, test := range tests { + test := test // address gosec G601 t.Run(test.desc, func(t *testing.T) { _, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc) if err == nil { @@ -1103,6 +1109,7 @@ func TestGetEndpointsFromEndpointSlices_ErrorsOnNoEndpointSlicesFound(t *testing } for _, test := range tests { + test := test // address gosec G601 t.Run(test.desc, func(t *testing.T) { _, err := lbc.getEndpointsForPortFromEndpointSlices(test.svcEndpointSlices, backendServicePort, &test.svc) if err == nil { diff --git a/internal/k8s/status_test.go b/internal/k8s/status_test.go index 07af62480c..ea9f7c6270 100644 --- a/internal/k8s/status_test.go +++ b/internal/k8s/status_test.go @@ -500,6 +500,7 @@ func TestHasVsStatusChanged(t *testing.T) { } for _, test := range tests { + test := test // address gosec G601 changed := hasVsStatusChanged(&test.vs, state, reason, msg) if changed != test.expected { @@ -577,6 +578,7 @@ func TestHasVsrStatusChanged(t *testing.T) { } for _, test := range tests { + test := test // address gosec G601 changed := hasVsrStatusChanged(&test.vsr, state, reason, msg, referencedBy) if changed != test.expected { @@ -728,6 +730,7 @@ func TestHasPolicyStatusChanged(t *testing.T) { } for _, test := range tests { + test := test // address gosec G601 changed := hasPolicyStatusChanged(&test.pol, state, reason, msg) if changed != test.expected { diff --git a/internal/k8s/validation.go b/internal/k8s/validation.go index 9ae802bf02..9db7d8117f 100644 --- a/internal/k8s/validation.go +++ b/internal/k8s/validation.go @@ -829,6 +829,7 @@ func validateIngressSpec(spec *networking.IngressSpec, fieldPath *field.Path) fi } for _, path := range r.HTTP.Paths { + path := path // address gosec G601 idxPath := idxRule.Child("http").Child("path").Index(i) allErrs = append(allErrs, validatePath(path.Path, path.PathType, idxPath.Child("path"))...) diff --git a/pkg/apis/configuration/validation/virtualserver.go b/pkg/apis/configuration/validation/virtualserver.go index 3c81ff420f..77d0a89aa1 100644 --- a/pkg/apis/configuration/validation/virtualserver.go +++ b/pkg/apis/configuration/validation/virtualserver.go @@ -572,6 +572,7 @@ func (vsv *VirtualServerValidator) validateUpstreams(upstreams []v1.Upstream, fi upstreamNames = sets.Set[string]{} for i, u := range upstreams { + u := u // address gosec G601 idxPath := fieldPath.Index(i) upstreamErrors := validateUpstreamName(u.Name, idxPath.Child("name")) diff --git a/pkg/apis/configuration/validation/virtualserver_test.go b/pkg/apis/configuration/validation/virtualserver_test.go index 6a27af5b0d..783cd00fee 100644 --- a/pkg/apis/configuration/validation/virtualserver_test.go +++ b/pkg/apis/configuration/validation/virtualserver_test.go @@ -4359,7 +4359,7 @@ func TestValidateErrorPageReturn(t *testing.T) { vsv := &VirtualServerValidator{isPlus: false} for _, epr := range tests { - // FIXME #nosec G601 + epr := epr // address gosec G601 allErrs := vsv.validateErrorPageReturn(&epr, field.NewPath("return")) if len(allErrs) != 0 { t.Errorf("validateErrorPageReturn(%v) returned errors for valid input: %v", epr, allErrs) @@ -4424,6 +4424,7 @@ func TestValidateErrorPageReturnFails(t *testing.T) { vsv := &VirtualServerValidator{isPlus: false} for _, test := range tests { + test := test // address gosec G601 allErrs := vsv.validateErrorPageReturn(&test.epr, field.NewPath("return")) if len(allErrs) == 0 { t.Errorf("validateErrorPageReturn(%v) returned no errors for invalid input for the case of %v", test.epr, test.msg) @@ -4451,7 +4452,7 @@ func TestValidateErrorPageRedirect(t *testing.T) { vsv := &VirtualServerValidator{isPlus: false} for _, epr := range tests { - // FIXME #nosec G601 + epr := epr // address gosec G601 allErrs := vsv.validateErrorPageRedirect(&epr, field.NewPath("redirect")) if len(allErrs) != 0 { t.Errorf("validateErrorPageRedirect(%v) returned errors for valid input: %v", epr, allErrs) @@ -4497,7 +4498,7 @@ func TestValidateErrorPageRedirectFails(t *testing.T) { vsv := &VirtualServerValidator{isPlus: false} for _, epr := range tests { - // FIXME #nosec G601 + epr := epr // address gosec G601 allErrs := vsv.validateErrorPageRedirect(&epr, field.NewPath("redirect")) if len(allErrs) == 0 { t.Errorf("validateErrorPageRedirect(%v) returned no errors for invalid input", epr) diff --git a/pkg/apis/externaldns/validation/externaldns_test.go b/pkg/apis/externaldns/validation/externaldns_test.go index 5f85d792f8..85db15c6b6 100644 --- a/pkg/apis/externaldns/validation/externaldns_test.go +++ b/pkg/apis/externaldns/validation/externaldns_test.go @@ -95,6 +95,7 @@ func TestValidateDNSEndpoint(t *testing.T) { } for _, tc := range tt { + tc := tc // address gosec G601 t.Run(tc.name, func(t *testing.T) { if err := validation.ValidateDNSEndpoint(&tc.endpoint); err != nil { t.Errorf("want no error on %v, got %v", tc.endpoint, err) @@ -282,6 +283,7 @@ func TestValidateDNSEndpoint_ReturnsErrorOn(t *testing.T) { } for _, tc := range tt { + tc := tc // address gosec G601 t.Run(tc.name, func(t *testing.T) { err := validation.ValidateDNSEndpoint(&tc.endpoint) if !errors.Is(err, tc.want) {