From c14b642fdbefbdc5e50a28ad454c752052c7ecbb Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Sun, 14 Aug 2022 11:58:07 +0200 Subject: [PATCH 1/6] fix panic when os.Stat returns an error besides ErrNotExists Signed-off-by: Samsondeen Dare --- cmd/cosign/cli/generate/generate_key_pair.go | 7 +++- .../cli/importkeypair/import_key_pair.go | 7 +++- pkg/cosign/common.go | 9 +++-- pkg/cosign/common_test.go | 36 +++++++++++++++++++ 4 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 pkg/cosign/common_test.go diff --git a/cmd/cosign/cli/generate/generate_key_pair.go b/cmd/cosign/cli/generate/generate_key_pair.go index 95f373e8b7f..037e5a5e901 100644 --- a/cmd/cosign/cli/generate/generate_key_pair.go +++ b/cmd/cosign/cli/generate/generate_key_pair.go @@ -85,7 +85,12 @@ func GenerateKeyPairCmd(ctx context.Context, kmsVal string, args []string) error return err } - if cosign.FileExists("cosign.key") { + fileExists, err := cosign.FileExists("cosign.key") + if err != nil { + return err + } + + if fileExists { var overwrite string fmt.Fprint(os.Stderr, "File cosign.key already exists. Overwrite (y/n)? ") fmt.Scanf("%s", &overwrite) diff --git a/cmd/cosign/cli/importkeypair/import_key_pair.go b/cmd/cosign/cli/importkeypair/import_key_pair.go index 4cfee452625..3a29898627d 100644 --- a/cmd/cosign/cli/importkeypair/import_key_pair.go +++ b/cmd/cosign/cli/importkeypair/import_key_pair.go @@ -36,7 +36,12 @@ func ImportKeyPairCmd(ctx context.Context, keyVal string, args []string) error { return err } - if cosign.FileExists("import-cosign.key") { + fileExists, err := cosign.FileExists("import-cosign.key\000x") + if err != nil { + return err + } + + if fileExists { var overwrite string fmt.Fprint(os.Stderr, "File import-cosign.key already exists. Overwrite (y/n)? ") fmt.Scanf("%s", &overwrite) diff --git a/pkg/cosign/common.go b/pkg/cosign/common.go index e723f306d78..1915f81f645 100644 --- a/pkg/cosign/common.go +++ b/pkg/cosign/common.go @@ -27,12 +27,15 @@ import ( ) // TODO(jason): Move this to an internal package. -func FileExists(filename string) bool { +func FileExists(filename string) (bool, error) { info, err := os.Stat(filename) if os.IsNotExist(err) { - return false + return false, nil } - return !info.IsDir() + if err != nil { + return false, err + } + return !info.IsDir(), nil } // ConfirmPrompt prompts the user for confirmation for an action. Supports skipping diff --git a/pkg/cosign/common_test.go b/pkg/cosign/common_test.go new file mode 100644 index 00000000000..97fd13ef8ca --- /dev/null +++ b/pkg/cosign/common_test.go @@ -0,0 +1,36 @@ +package cosign + +import ( + "os" + "testing" +) + +func Test_FileExists(t *testing.T) { + tmpFile, err := os.CreateTemp(os.TempDir(), "cosign_test.txt") + if err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + path string + exists bool + wantErr bool + }{ + {"file exists", tmpFile.Name(), true, false}, + {"file does not exist", "testt.txt", false, false}, + {"other error e.g cannot access file", "\000x", false, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := FileExists(tt.path) + if (err != nil) != tt.wantErr { + t.Errorf("FileExists() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.exists { + t.Errorf("FileExists() = %v, want %v", got, tt.exists) + } + }) + } +} From 5d2b5d380704d9091179cb870d0fac7e0ff46006 Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Mon, 15 Aug 2022 12:40:06 +0200 Subject: [PATCH 2/6] boilerplate fix Signed-off-by: Samsondeen Dare --- pkg/cosign/common_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pkg/cosign/common_test.go b/pkg/cosign/common_test.go index 97fd13ef8ca..5650d698566 100644 --- a/pkg/cosign/common_test.go +++ b/pkg/cosign/common_test.go @@ -1,3 +1,18 @@ +// +// Copyright 2022 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package cosign import ( From bf79f0dcf72e6314afc9f734e746c6a7295ee99f Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Wed, 17 Aug 2022 09:44:01 +0200 Subject: [PATCH 3/6] move FileExists to internal package Signed-off-by: Samsondeen Dare --- cmd/cosign/cli/generate/generate_key_pair.go | 7 +++-- .../cli/importkeypair/import_key_pair.go | 6 ++-- go.mod | 2 +- internal/pkg/cosign/common.go | 28 +++++++++++++++++++ {pkg => internal/pkg}/cosign/common_test.go | 0 pkg/cosign/common.go | 12 -------- 6 files changed, 37 insertions(+), 18 deletions(-) create mode 100644 internal/pkg/cosign/common.go rename {pkg => internal/pkg}/cosign/common_test.go (100%) diff --git a/cmd/cosign/cli/generate/generate_key_pair.go b/cmd/cosign/cli/generate/generate_key_pair.go index 037e5a5e901..cfcd5c83270 100644 --- a/cmd/cosign/cli/generate/generate_key_pair.go +++ b/cmd/cosign/cli/generate/generate_key_pair.go @@ -18,16 +18,17 @@ package generate import ( "context" "crypto" - "errors" "fmt" "io" "os" "strings" + "github.com/pkg/errors" "github.com/sigstore/cosign/pkg/cosign/git" "github.com/sigstore/cosign/pkg/cosign/git/github" "github.com/sigstore/cosign/pkg/cosign/git/gitlab" + icos "github.com/sigstore/cosign/internal/pkg/cosign" "github.com/sigstore/cosign/pkg/cosign" "github.com/sigstore/cosign/pkg/cosign/kubernetes" "github.com/sigstore/sigstore/pkg/cryptoutils" @@ -85,9 +86,9 @@ func GenerateKeyPairCmd(ctx context.Context, kmsVal string, args []string) error return err } - fileExists, err := cosign.FileExists("cosign.key") + fileExists, err := icos.FileExists("cosign.key") if err != nil { - return err + return errors.Wrap(err, "error checking if cosign.key exists") } if fileExists { diff --git a/cmd/cosign/cli/importkeypair/import_key_pair.go b/cmd/cosign/cli/importkeypair/import_key_pair.go index 3a29898627d..4c8b1f203c4 100644 --- a/cmd/cosign/cli/importkeypair/import_key_pair.go +++ b/cmd/cosign/cli/importkeypair/import_key_pair.go @@ -21,6 +21,8 @@ import ( "io" "os" + "github.com/pkg/errors" + icos "github.com/sigstore/cosign/internal/pkg/cosign" "github.com/sigstore/cosign/pkg/cosign" ) @@ -36,9 +38,9 @@ func ImportKeyPairCmd(ctx context.Context, keyVal string, args []string) error { return err } - fileExists, err := cosign.FileExists("import-cosign.key\000x") + fileExists, err := icos.FileExists("import-cosign.key") if err != nil { - return err + return errors.Wrap(err, "error checking if import-cosign.key exists") } if fileExists { diff --git a/go.mod b/go.mod index 1e989650610..46e2f2875b5 100644 --- a/go.mod +++ b/go.mod @@ -52,6 +52,7 @@ require ( github.com/oklog/run v1.1.0 github.com/open-policy-agent/opa v0.43.0 github.com/pierrec/lz4 v2.6.1+incompatible + github.com/pkg/errors v0.9.1 github.com/ryanuber/go-glob v1.0.0 github.com/secure-systems-lab/go-securesystemslib v0.4.0 github.com/sigstore/fulcio v0.1.2-0.20220114150912-86a2036f9bc7 @@ -220,7 +221,6 @@ require ( github.com/opentracing/opentracing-go v1.2.0 // indirect github.com/pelletier/go-toml v1.9.5 // indirect github.com/pelletier/go-toml/v2 v2.0.1 // indirect - github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_golang v1.12.2 // indirect github.com/prometheus/client_model v0.2.0 // indirect diff --git a/internal/pkg/cosign/common.go b/internal/pkg/cosign/common.go new file mode 100644 index 00000000000..74a9ae9b6d5 --- /dev/null +++ b/internal/pkg/cosign/common.go @@ -0,0 +1,28 @@ +// Copyright 2021 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cosign + +import "os" + +func FileExists(filename string) (bool, error) { + info, err := os.Stat(filename) + if os.IsNotExist(err) { + return false, nil + } + if err != nil { + return false, err + } + return !info.IsDir(), nil +} diff --git a/pkg/cosign/common_test.go b/internal/pkg/cosign/common_test.go similarity index 100% rename from pkg/cosign/common_test.go rename to internal/pkg/cosign/common_test.go diff --git a/pkg/cosign/common.go b/pkg/cosign/common.go index 1915f81f645..0992843bd80 100644 --- a/pkg/cosign/common.go +++ b/pkg/cosign/common.go @@ -26,18 +26,6 @@ import ( "golang.org/x/term" ) -// TODO(jason): Move this to an internal package. -func FileExists(filename string) (bool, error) { - info, err := os.Stat(filename) - if os.IsNotExist(err) { - return false, nil - } - if err != nil { - return false, err - } - return !info.IsDir(), nil -} - // ConfirmPrompt prompts the user for confirmation for an action. Supports skipping // the confirmation prompt when skipConfirmation is set. // TODO(jason): Move this to an internal package. From 73ab403e184228188b395aedf9eb3852f81b729a Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Wed, 17 Aug 2022 14:12:17 +0200 Subject: [PATCH 4/6] remove archived pkg/errors Signed-off-by: Samsondeen Dare --- cmd/cosign/cli/generate/generate_key_pair.go | 2 +- cmd/cosign/cli/importkeypair/import_key_pair.go | 3 +-- go.mod | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cmd/cosign/cli/generate/generate_key_pair.go b/cmd/cosign/cli/generate/generate_key_pair.go index cfcd5c83270..92e0240e16d 100644 --- a/cmd/cosign/cli/generate/generate_key_pair.go +++ b/cmd/cosign/cli/generate/generate_key_pair.go @@ -88,7 +88,7 @@ func GenerateKeyPairCmd(ctx context.Context, kmsVal string, args []string) error fileExists, err := icos.FileExists("cosign.key") if err != nil { - return errors.Wrap(err, "error checking if cosign.key exists") + return fmt.Errorf("error checking if cosign.key exists: %w", err) } if fileExists { diff --git a/cmd/cosign/cli/importkeypair/import_key_pair.go b/cmd/cosign/cli/importkeypair/import_key_pair.go index 4c8b1f203c4..0cbd75faf60 100644 --- a/cmd/cosign/cli/importkeypair/import_key_pair.go +++ b/cmd/cosign/cli/importkeypair/import_key_pair.go @@ -21,7 +21,6 @@ import ( "io" "os" - "github.com/pkg/errors" icos "github.com/sigstore/cosign/internal/pkg/cosign" "github.com/sigstore/cosign/pkg/cosign" ) @@ -40,7 +39,7 @@ func ImportKeyPairCmd(ctx context.Context, keyVal string, args []string) error { fileExists, err := icos.FileExists("import-cosign.key") if err != nil { - return errors.Wrap(err, "error checking if import-cosign.key exists") + return fmt.Errorf("error checking if import-cosign.key exists: %w", err) } if fileExists { diff --git a/go.mod b/go.mod index 46e2f2875b5..1e989650610 100644 --- a/go.mod +++ b/go.mod @@ -52,7 +52,6 @@ require ( github.com/oklog/run v1.1.0 github.com/open-policy-agent/opa v0.43.0 github.com/pierrec/lz4 v2.6.1+incompatible - github.com/pkg/errors v0.9.1 github.com/ryanuber/go-glob v1.0.0 github.com/secure-systems-lab/go-securesystemslib v0.4.0 github.com/sigstore/fulcio v0.1.2-0.20220114150912-86a2036f9bc7 @@ -221,6 +220,7 @@ require ( github.com/opentracing/opentracing-go v1.2.0 // indirect github.com/pelletier/go-toml v1.9.5 // indirect github.com/pelletier/go-toml/v2 v2.0.1 // indirect + github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_golang v1.12.2 // indirect github.com/prometheus/client_model v0.2.0 // indirect From 777c36bb81bbc1d6b18ad4a7561888edbfb09b9f Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Wed, 17 Aug 2022 15:07:37 +0200 Subject: [PATCH 5/6] comments Signed-off-by: Samsondeen Dare --- cmd/cosign/cli/generate/generate_key_pair.go | 2 +- cmd/cosign/cli/importkeypair/import_key_pair.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/cosign/cli/generate/generate_key_pair.go b/cmd/cosign/cli/generate/generate_key_pair.go index 92e0240e16d..d9e42aa28b0 100644 --- a/cmd/cosign/cli/generate/generate_key_pair.go +++ b/cmd/cosign/cli/generate/generate_key_pair.go @@ -88,7 +88,7 @@ func GenerateKeyPairCmd(ctx context.Context, kmsVal string, args []string) error fileExists, err := icos.FileExists("cosign.key") if err != nil { - return fmt.Errorf("error checking if cosign.key exists: %w", err) + return fmt.Errorf("failed checking if cosign.key exists: %w", err) } if fileExists { diff --git a/cmd/cosign/cli/importkeypair/import_key_pair.go b/cmd/cosign/cli/importkeypair/import_key_pair.go index 0cbd75faf60..cdcb0a01e3a 100644 --- a/cmd/cosign/cli/importkeypair/import_key_pair.go +++ b/cmd/cosign/cli/importkeypair/import_key_pair.go @@ -39,7 +39,7 @@ func ImportKeyPairCmd(ctx context.Context, keyVal string, args []string) error { fileExists, err := icos.FileExists("import-cosign.key") if err != nil { - return fmt.Errorf("error checking if import-cosign.key exists: %w", err) + return fmt.Errorf("failed checking if import-cosign.key exists: %w", err) } if fileExists { From d12ac3dcb1377bc6c957c0859b8fcc2ca80dd3f4 Mon Sep 17 00:00:00 2001 From: Samsondeen Dare Date: Wed, 17 Aug 2022 16:17:22 +0200 Subject: [PATCH 6/6] comments Signed-off-by: Samsondeen Dare --- cmd/cosign/cli/generate/generate_key_pair.go | 2 +- internal/pkg/cosign/common.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/cosign/cli/generate/generate_key_pair.go b/cmd/cosign/cli/generate/generate_key_pair.go index d9e42aa28b0..72365713076 100644 --- a/cmd/cosign/cli/generate/generate_key_pair.go +++ b/cmd/cosign/cli/generate/generate_key_pair.go @@ -18,12 +18,12 @@ package generate import ( "context" "crypto" + "errors" "fmt" "io" "os" "strings" - "github.com/pkg/errors" "github.com/sigstore/cosign/pkg/cosign/git" "github.com/sigstore/cosign/pkg/cosign/git/github" "github.com/sigstore/cosign/pkg/cosign/git/gitlab" diff --git a/internal/pkg/cosign/common.go b/internal/pkg/cosign/common.go index 74a9ae9b6d5..0dca870f8f1 100644 --- a/internal/pkg/cosign/common.go +++ b/internal/pkg/cosign/common.go @@ -1,4 +1,4 @@ -// Copyright 2021 The Sigstore Authors. +// Copyright 2022 The Sigstore Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License.