From 3b9fbe2cd75a778e8d4d5a14e555cb52947094bf Mon Sep 17 00:00:00 2001 From: Sascha Schwarze Date: Wed, 4 Oct 2023 21:36:59 +0200 Subject: [PATCH] Address gosec findings --- .golangci.yml | 5 ----- cmd/shp/main.go | 9 ++++++--- pkg/shp/cmd/build/list.go | 4 +--- pkg/shp/cmd/build/run_test.go | 4 ++-- pkg/shp/cmd/buildrun/list.go | 4 +--- pkg/shp/cmd/buildrun/logs_test.go | 4 ++-- pkg/shp/cmd/follower/follow.go | 2 +- pkg/shp/flags/flags.go | 4 ++-- pkg/shp/params/params.go | 3 +-- pkg/shp/streamer/tar.go | 8 +++++--- pkg/shp/streamer/util.go | 1 + pkg/shp/tail/tail.go | 10 ++++++++-- test/mock/fake_clientset.go | 5 ++++- test/stub/client.go | 6 +++++- 14 files changed, 39 insertions(+), 30 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 50d5bbd2f..ac7aa1f25 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -5,8 +5,3 @@ linters: - misspell disable: - errcheck - -linters-settings: - gosec: - excludes: - - G101 # Potential hardcoded credentials diff --git a/cmd/shp/main.go b/cmd/shp/main.go index 0b9293d3e..fa8e96f9d 100644 --- a/cmd/shp/main.go +++ b/cmd/shp/main.go @@ -35,7 +35,10 @@ var hiddenLogFlags = []string{ } func main() { - initGoFlags() + if err := initGoFlags(); err != nil { + fmt.Fprintf(os.Stderr, "ERROR: %v\n", err) + os.Exit(1) + } initPFlags() streams := genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr} @@ -48,7 +51,7 @@ func main() { // initGoFlags initializes the flag sets for klog. // Any flags for "-h" or "--help" are ignored because pflag will show the usage later with all subcommands. -func initGoFlags() { +func initGoFlags() error { flagset := goflag.NewFlagSet(ApplicationName, goflag.ContinueOnError) goflag.CommandLine = flagset klog.InitFlags(flagset) @@ -59,7 +62,7 @@ func initGoFlags() { args = append(args, arg) } } - flagset.Parse(args) + return flagset.Parse(args) } // initPFlags initializes the pflags used by Cobra subcommands. diff --git a/pkg/shp/cmd/build/list.go b/pkg/shp/cmd/build/list.go index fa42ea68c..2dd5c647a 100644 --- a/pkg/shp/cmd/build/list.go +++ b/pkg/shp/cmd/build/list.go @@ -79,7 +79,5 @@ func (c *ListCommand) Run(params *params.Params, io *genericclioptions.IOStreams fmt.Fprintf(writer, columnTemplate, b.Name, b.Spec.Output.Image, message) } - writer.Flush() - - return nil + return writer.Flush() } diff --git a/pkg/shp/cmd/build/run_test.go b/pkg/shp/cmd/build/run_test.go index 55a410f0c..0f1316743 100644 --- a/pkg/shp/cmd/build/run_test.go +++ b/pkg/shp/cmd/build/run_test.go @@ -90,7 +90,7 @@ func TestStartBuildRunFollowLog(t *testing.T) { }, } - for _, test := range tests { + for i, test := range tests { name := "testpod" containerName := "container" pod := &corev1.Pod{ @@ -144,7 +144,7 @@ func TestStartBuildRunFollowLog(t *testing.T) { cmd.Cmd().ExecuteC() pm := genericclioptions.NewConfigFlags(true) if len(test.to) > 0 { - pm.Timeout = &test.to + pm.Timeout = &tests[i].to } failureDuration := 1 * time.Millisecond param := params.NewParamsForTest(kclientset, shpclientset, pm, metav1.NamespaceDefault, &failureDuration, &failureDuration) diff --git a/pkg/shp/cmd/buildrun/list.go b/pkg/shp/cmd/buildrun/list.go index fbea919f8..92fb65b72 100644 --- a/pkg/shp/cmd/buildrun/list.go +++ b/pkg/shp/cmd/buildrun/list.go @@ -90,7 +90,5 @@ func (c *ListCommand) Run(params *params.Params, _ *genericclioptions.IOStreams) fmt.Fprintf(writer, columnTemplate, name, status, age) } - writer.Flush() - - return nil + return writer.Flush() } diff --git a/pkg/shp/cmd/buildrun/logs_test.go b/pkg/shp/cmd/buildrun/logs_test.go index a5ea39e41..8acdb37fa 100644 --- a/pkg/shp/cmd/buildrun/logs_test.go +++ b/pkg/shp/cmd/buildrun/logs_test.go @@ -124,7 +124,7 @@ func TestStreamBuildRunFollowLogs(t *testing.T) { }, } - for _, test := range tests { + for i, test := range tests { name := "testpod" containerName := "container" pod := &corev1.Pod{ @@ -178,7 +178,7 @@ func TestStreamBuildRunFollowLogs(t *testing.T) { cmd.Cmd().ExecuteC() pm := genericclioptions.NewConfigFlags(true) if len(test.to) > 0 { - pm.Timeout = &test.to + pm.Timeout = &tests[i].to } param := params.NewParamsForTest(kclientset, shpclientset, pm, metav1.NamespaceDefault, nil, nil) diff --git a/pkg/shp/cmd/follower/follow.go b/pkg/shp/cmd/follower/follow.go index 3133a90c2..ea6c5f921 100644 --- a/pkg/shp/cmd/follower/follow.go +++ b/pkg/shp/cmd/follower/follow.go @@ -220,7 +220,7 @@ func (f *Follower) OnNoPodEventsYet(podList *corev1.PodList) { f.Log(fmt.Sprintf("BuildRun %q log following has not observed any pod events yet.\n", f.buildRun.Name)) if podList != nil && len(podList.Items) > 0 { f.Log(fmt.Sprintf("BuildRun %q's Pod completed before the log following's watch was established.\n", f.buildRun.Name)) - f.OnEvent(&podList.Items[0]) + f.OnEvent(&podList.Items[0]) // #nosec G104 there is nothing we must handle here, the error is logged in the function already return } brClient := f.buildClientset.ShipwrightV1alpha1().BuildRuns(f.buildRun.Namespace) diff --git a/pkg/shp/flags/flags.go b/pkg/shp/flags/flags.go index 709b7befb..7b2eefac0 100644 --- a/pkg/shp/flags/flags.go +++ b/pkg/shp/flags/flags.go @@ -29,7 +29,7 @@ const ( // SourceContextDirFlag command-line flag. SourceContextDirFlag = "source-context-dir" // SourceCredentialsSecretFlag command-line flag. - SourceCredentialsSecretFlag = "source-credentials-secret" + SourceCredentialsSecretFlag = "source-credentials-secret" // #nosec G101 // SourceBundleImageFlag command-line flag SourceBundleImageFlag = "source-bundle-image" // SourceBundlePruneFlag command-line flag @@ -45,7 +45,7 @@ const ( // OutputInsecure command-line flag. OutputInsecureFlag = "output-insecure" // OutputCredentialsSecretFlag command-line flag. - OutputCredentialsSecretFlag = "output-credentials-secret" + OutputCredentialsSecretFlag = "output-credentials-secret" // #nosec G101 // ServiceAccountNameFlag command-line flag. ServiceAccountNameFlag = "sa-name" // ServiceAccountGenerateFlag command-line flag. diff --git a/pkg/shp/params/params.go b/pkg/shp/params/params.go index 7d42add8d..5974b2fce 100644 --- a/pkg/shp/params/params.go +++ b/pkg/shp/params/params.go @@ -136,12 +136,11 @@ func (p *Params) ShipwrightClientSet() (buildclientset.Interface, error) { return p.buildClientset, nil } -// Namespace returns kubernetes namespace with alle the overrides +// Namespace returns kubernetes namespace with all the overrides // from command line and kubernetes config func (p *Params) Namespace() string { if len(p.namespace) == 0 { clientConfig := p.configFlags.ToRawKubeConfigLoader() - clientConfig.ClientConfig() p.namespace, _, _ = clientConfig.Namespace() } diff --git a/pkg/shp/streamer/tar.go b/pkg/shp/streamer/tar.go index e57b9559a..29d36e1c6 100644 --- a/pkg/shp/streamer/tar.go +++ b/pkg/shp/streamer/tar.go @@ -37,7 +37,7 @@ func (t *Tar) skipPath(fpath string, stat fs.FileInfo) bool { // Create the actual tar by inspecting all files in source path, skipping some. func (t *Tar) Create(w io.Writer) error { tw := tar.NewWriter(w) - filepath.Walk(t.src, func(fpath string, stat fs.FileInfo, err error) error { + if err := filepath.Walk(t.src, func(fpath string, stat fs.FileInfo, err error) error { if err != nil { return err } @@ -45,7 +45,9 @@ func (t *Tar) Create(w io.Writer) error { return nil } return writeFileToTar(tw, t.src, fpath, stat) - }) + }); err != nil { + return err + } return tw.Close() } @@ -99,6 +101,6 @@ func (t *Tar) tarSize() error { return nil }) - t.Size = size+size*1/100 + t.Size = size + size*1/100 return err } diff --git a/pkg/shp/streamer/util.go b/pkg/shp/streamer/util.go index 439d9dc72..53eda9d8b 100644 --- a/pkg/shp/streamer/util.go +++ b/pkg/shp/streamer/util.go @@ -24,6 +24,7 @@ func writeFileToTar(tw *tar.Writer, src, fpath string, stat fs.FileInfo) error { return err } + // #nosec G304 intentionally opening file from variable f, err := os.Open(fpath) if err != nil { return err diff --git a/pkg/shp/tail/tail.go b/pkg/shp/tail/tail.go index 672ca73b0..56b73eba6 100644 --- a/pkg/shp/tail/tail.go +++ b/pkg/shp/tail/tail.go @@ -48,11 +48,17 @@ func (t *Tail) Start(ns, podName, container string) { fmt.Fprintln(t.stderr, err) return } - defer stream.Close() + defer func() { + if err := stream.Close(); err != nil { + fmt.Fprintf(t.stderr, "Failed to close stream: %v", err) + } + }() go func() { <-t.stopCh - stream.Close() + if err := stream.Close(); err != nil { + fmt.Fprintf(t.stderr, "Failed to close stream: %v", err) + } }() containerName := strings.TrimPrefix(container, "step-") diff --git a/test/mock/fake_clientset.go b/test/mock/fake_clientset.go index 0036bab4f..aa1ae538f 100644 --- a/test/mock/fake_clientset.go +++ b/test/mock/fake_clientset.go @@ -3,6 +3,7 @@ package mock import ( "bytes" "io" + "log" "net/http" corev1 "k8s.io/api/core/v1" @@ -58,7 +59,9 @@ func (f *FakeClientset) roundTripperFn(req *http.Request) (*http.Response, error // bootstrap instantiate the basic elements of the clientset. func (f *FakeClientset) bootstrap() { f.scheme = runtime.NewScheme() - f.scheme.AddIgnoredConversionType(&metav1.TypeMeta{}, &metav1.TypeMeta{}) + if err := f.scheme.AddIgnoredConversionType(&metav1.TypeMeta{}, &metav1.TypeMeta{}); err != nil { + log.Fatal(err) + } f.scheme.AddKnownTypes(corev1.SchemeGroupVersion, &corev1.Pod{}, &metav1.Status{}) f.codecs = serializer.NewCodecFactory(f.scheme) diff --git a/test/stub/client.go b/test/stub/client.go index ecfc4b50d..f8b7046c9 100644 --- a/test/stub/client.go +++ b/test/stub/client.go @@ -1,6 +1,8 @@ package stub import ( + "log" + buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" "k8s.io/apimachinery/pkg/runtime" @@ -11,6 +13,8 @@ import ( // NewFakeClient creates a fake client with Shipwright's Build scheme. func NewFakeClient() dynamic.Interface { scheme := runtime.NewScheme() - buildv1alpha1.SchemeBuilder.AddToScheme(scheme) + if err := buildv1alpha1.SchemeBuilder.AddToScheme(scheme); err != nil { + log.Fatal(err) + } return fake.NewSimpleDynamicClient(scheme) }