Skip to content

Commit

Permalink
Introduce make targets for sast and address security issues. (#130)
Browse files Browse the repository at this point in the history
* Introduce sast using gosec

* Add license headers

* Update comment

* Adapt pipeline_definitions to include SAST linting logs in OCM descriptor

* Address review comments
  • Loading branch information
thiyyakat authored Dec 23, 2024
1 parent 33a3222 commit b243713
Show file tree
Hide file tree
Showing 22 changed files with 162 additions and 10 deletions.
4 changes: 3 additions & 1 deletion .ci/check
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,6 @@ fi

make check

echo "All checks are passing."
make sast-report

echo "All checks are passing."
17 changes: 17 additions & 0 deletions .ci/pipeline_definitions
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@
# SPDX-License-Identifier: Apache-2.0
dependency-watchdog:
base_definition:
repo:
source_labels:
- name: cloud.gardener.cnudie/dso/scanning-hints/source_analysis/v1
value:
policy: skip
comment: |
we use gosec for sast scanning. See attached log.
traits:
version:
preprocess:
Expand Down Expand Up @@ -51,6 +58,16 @@ dependency-watchdog:
image: europe-docker.pkg.dev/gardener-project/releases/gardener/dependency-watchdog
release:
nextversion: 'bump_minor'
assets:
- type: build-step-log
step_name: check
purposes:
- lint
- sast
- gosec
comment: |
we use gosec (linter) for SAST scans
see: https://github.com/securego/gosec
slack:
default_channel: 'internal_scp_workspace'
channel_cfgs:
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,6 @@ dev
hack/test
.DS_Store
.idea/

# gosec
gosec-report.sarif
8 changes: 8 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,11 @@ stress_args = test-package test-func tool-params
.PHONY: stress
stress: $(GO_STRESS)
@./hack/stress-test.sh $@ $(call args,$@)

.PHONY: sast
sast: $(GOSEC)
@./hack/sast.sh

.PHONY: sast-report
sast-report:$(GOSEC)
@./hack/sast.sh --gosec-report true
41 changes: 41 additions & 0 deletions hack/sast.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#!/usr/bin/env bash
#
# SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors
#
# SPDX-License-Identifier: Apache-2.0

set -e

root_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )/.." &> /dev/null && pwd )"

gosec_report="false"
gosec_report_parse_flags=""

parse_flags() {
while test $# -gt 1; do
case "$1" in
--gosec-report)
shift; gosec_report="$1"
;;
*)
echo "Unknown argument: $1"
exit 1
;;
esac
shift
done
}

parse_flags "$@"

echo "> Running gosec"
gosec --version
if [[ "$gosec_report" != "false" ]]; then
echo "Exporting report to $root_dir/gosec-report.sarif"
gosec_report_parse_flags="-track-suppressions -fmt=sarif -out=gosec-report.sarif -stdout"
fi

# Generated code is excluded from gosec scan.
# Nested go modules are not supported by gosec (see https://github.com/securego/gosec/issues/501), so the ./hack folder
# is excluded too. It does not contain productive code anyway.
gosec -exclude-generated -exclude-dir=hack -exclude-dir=tmp $gosec_report_parse_flags ./...
9 changes: 7 additions & 2 deletions hack/tools.mk
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ GO_IMPORT_BOSS := $(TOOLS_BIN_DIR)/import-boss
GO_STRESS := $(TOOLS_BIN_DIR)/stress
SETUP_ENVTEST := $(TOOLS_BIN_DIR)/setup-envtest
GOTESTFMT := $(TOOLS_BIN_DIR)/gotestfmt
GOSEC := $(TOOLS_BIN_DIR)/gosec

# Use this function to get the version of a go module from go.mod
version_gomod = $(shell go list -mod=mod -f '{{ .Version }}' -m $(1))

#default tool versions
GOLANGCI_LINT_VERSION ?= v1.60.1
GOLANGCI_LINT_VERSION ?= v1.60.3
GO_VULN_CHECK_VERSION ?= latest
GOIMPORTS_VERSION ?= latest
LOGCHECK_VERSION ?= 282c229e4dc4b4088523b97a2a696a48e8506975 # this commit hash corresponds to v1.108.1 which is the gardener/gardener version in go.mod - we could use regular tags when https://github.com/gardener/gardener/issues/8811 is resolved
Expand All @@ -26,6 +27,7 @@ K8S_VERSION ?= $(subst v0,v1,$(call version_gomod,k8s.io/api))
GO_STRESS_VERSION ?= latest
SETUP_ENVTEST_VERSION ?= latest
GOTESTFMT_VERSION ?= v2.5.0
GOSEC_VERSION ?= v2.21.4

# add ./hack/tools/bin to the PATH
export TOOLS_BIN_DIR := $(TOOLS_BIN_DIR)
Expand Down Expand Up @@ -60,4 +62,7 @@ $(SETUP_ENVTEST):
GOBIN=$(abspath $(TOOLS_BIN_DIR)) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@$(SETUP_ENVTEST_VERSION)

$(GOTESTFMT):
GOBIN=$(abspath $(TOOLS_BIN_DIR)) go install github.com/gotesttools/gotestfmt/v2/cmd/gotestfmt@$(GOTESTFMT_VERSION)
GOBIN=$(abspath $(TOOLS_BIN_DIR)) go install github.com/gotesttools/gotestfmt/v2/cmd/gotestfmt@$(GOTESTFMT_VERSION)

$(GOSEC):
GOSEC_VERSION=$(GOSEC_VERSION) bash $(TOOLS_DIR)/install-gosec.sh
Empty file added hack/tools/bin/.gitkeep
Empty file.
40 changes: 40 additions & 0 deletions hack/tools/install-gosec.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#!/usr/bin/env bash
#
# SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors
#
# SPDX-License-Identifier: Apache-2.0

set -e

echo "> Installing gosec"

TOOLS_BIN_DIR=${TOOLS_BIN_DIR:-$(dirname $0)/bin}

platform=$(uname -s | tr '[:upper:]' '[:lower:]')
version=$GOSEC_VERSION
case $(uname -m) in
aarch64 | arm64)
arch="arm64"
;;
x86_64)
arch="amd64"
;;
*)
echo "Unknown architecture"
exit -1
;;
esac

archive_name="gosec_${version#v}_${platform}_${arch}"
file_name="${archive_name}.tar.gz"

temp_dir="$(mktemp -d)"
function cleanup {
rm -rf "${temp_dir}"
}
trap cleanup EXIT ERR INT TERM
curl -L -o ${temp_dir}/${file_name} "https://github.com/securego/gosec/releases/download/${version}/${file_name}"

tar -xzm -C "${temp_dir}" -f "${temp_dir}/${file_name}"
mv "${temp_dir}/gosec" $TOOLS_BIN_DIR
chmod +x $TOOLS_BIN_DIR/gosec
4 changes: 4 additions & 0 deletions internal/prober/errors/errors.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and Gardener contributors
//
// SPDX-License-Identifier: Apache-2.0

package errors

import "fmt"
Expand Down
4 changes: 4 additions & 0 deletions internal/prober/fakes/k8s/discoveryclient.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and Gardener contributors
//
// SPDX-License-Identifier: Apache-2.0

package k8s

import (
Expand Down
4 changes: 4 additions & 0 deletions internal/prober/fakes/scale/scale.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and Gardener contributors
//
// SPDX-License-Identifier: Apache-2.0

package scale

import (
Expand Down
4 changes: 4 additions & 0 deletions internal/prober/fakes/shoot/clientcreator.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and Gardener contributors
//
// SPDX-License-Identifier: Apache-2.0

package shoot

import (
Expand Down
4 changes: 2 additions & 2 deletions internal/prober/scaler/scale.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,11 @@ func (r *resScaler) determineTargetReplicas(annotations map[string]string) (int3
return defaultScaleDownReplicas, nil
}
if replicasStr, ok := annotations[replicasAnnotationKey]; ok {
replicas, err := strconv.Atoi(replicasStr)
replicas, err := strconv.Atoi(replicasStr) // #nosec G109 -- replicas will not exceed MaxInt32
if err != nil {
return 0, fmt.Errorf("unexpected and invalid replicasStr set as value for annotation: %s for resource, Err: %w", replicasAnnotationKey, err)
}
return int32(replicas), nil
return int32(replicas), nil // #nosec G109 G115 -- number of replicas will not exceed MaxInt32
}
r.logger.Info("Replicas annotation not found, falling back to default scale-up replicas", "operation", r.resourceInfo.operation, "annotationKey", replicasAnnotationKey, "default-replicas", defaultScaleUpReplicas)
return defaultScaleUpReplicas, nil
Expand Down
4 changes: 4 additions & 0 deletions internal/prober/types/types.go
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and Gardener contributors
//
// SPDX-License-Identifier: Apache-2.0

package types
2 changes: 1 addition & 1 deletion internal/test/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func createWorkers(workerCount int, workerNodeConditions [][]string) []gardencor
w := gardencorev1beta1.Worker{
Name: rand.String(4),
Machine: gardencorev1beta1.Machine{},
Maximum: int32(mx),
Maximum: int32(mx), // #nosec G115 -- lies between 0 and 5.
Minimum: 1,
}
if i < len(workerNodeConditions) {
Expand Down
4 changes: 4 additions & 0 deletions internal/test/constants.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and Gardener contributors
//
// SPDX-License-Identifier: Apache-2.0

package test

// DefaultNamespace is the default namespace used in tests
Expand Down
4 changes: 4 additions & 0 deletions internal/test/k8sresources.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and Gardener contributors
//
// SPDX-License-Identifier: Apache-2.0

package test

import (
Expand Down
2 changes: 1 addition & 1 deletion internal/test/kind.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func CreateKindCluster(config KindConfig) (KindCluster, error) {
return nil, err
}
// store the kubeconfig file at kubeConfigPath, this will be later used to delete the cluster or perform operations on the cluster
err = os.WriteFile(kubeConfigPath, kubeConfigBytes, 0644)
err = os.WriteFile(kubeConfigPath, kubeConfigBytes, 0644) // #nosec G306 -- Test only
if err != nil {
return nil, fmt.Errorf("failed to store the kubeconfig file at %s :%w", kubeConfigPath, err)
}
Expand Down
6 changes: 5 additions & 1 deletion internal/test/miscellaneous.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and Gardener contributors
//
// SPDX-License-Identifier: Apache-2.0

package test

import (
Expand All @@ -12,7 +16,7 @@ import (

// ReadFile reads the file present at the given filePath and returns a byte Buffer containing its contents.
func ReadFile(filePath string) (*bytes.Buffer, error) {
f, err := os.Open(filePath)
f, err := os.Open(filePath) // #nosec G304 -- Test only
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/util/k8shelper.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func GetResourceReadyReplicas(ctx context.Context, cli client.Client, namespace
return 0, err
}

return int32(readyReplicas), nil
return int32(readyReplicas), nil // #nosec G115 -- number of replicas will not exceed MaxInt32
}

// CreateClientSetFromRestConfig creates a kubernetes.Clientset from rest.Config.
Expand Down
4 changes: 4 additions & 0 deletions internal/util/node.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company and Gardener contributors
//
// SPDX-License-Identifier: Apache-2.0

package util

import (
Expand Down
2 changes: 1 addition & 1 deletion internal/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func SleepWithContext(ctx context.Context, sleepFor time.Duration) error {

// ReadAndUnmarshall reads file and Unmarshall the contents in a generic type
func ReadAndUnmarshall[T any](filename string) (*T, error) {
configBytes, err := os.ReadFile(filename)
configBytes, err := os.ReadFile(filename) // #nosec G304 -- Loaded from ConfigMap
if err != nil {
return nil, err
}
Expand Down

0 comments on commit b243713

Please sign in to comment.