fix panic when os.Stat returns an error besides ErrNotExists (sigstor…

…e#2162) * fix panic when os.Stat returns an error besides ErrNotExists Signed-off-by: Samsondeen Dare <[email protected]> * boilerplate fix Signed-off-by: Samsondeen Dare <[email protected]> * move FileExists to internal package Signed-off-by: Samsondeen Dare <[email protected]> * remove archived pkg/errors Signed-off-by: Samsondeen Dare <[email protected]> * comments Signed-off-by: Samsondeen Dare <[email protected]> * comments Signed-off-by: Samsondeen Dare <[email protected]> Signed-off-by: Samsondeen Dare <[email protected]>
cldmnky · Aug 21, 2022 · e06aacb · e06aacb
1 parent e4b1c9f
commit e06aacb
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 11 deletions.
diff --git a/cmd/cosign/cli/generate/generate_key_pair.go b/cmd/cosign/cli/generate/generate_key_pair.go
@@ -28,6 +28,7 @@ import (
 	"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,7 +86,12 @@ func GenerateKeyPairCmd(ctx context.Context, kmsVal string, args []string) error
 		return err
 	}
 
-	if cosign.FileExists("cosign.key") {
+	fileExists, err := icos.FileExists("cosign.key")
+	if err != nil {
+		return fmt.Errorf("failed checking if cosign.key exists: %w", 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
@@ -21,6 +21,7 @@ import (
 	"io"
 	"os"
 
+	icos "github.com/sigstore/cosign/internal/pkg/cosign"
 	"github.com/sigstore/cosign/pkg/cosign"
 )
 
@@ -36,7 +37,12 @@ func ImportKeyPairCmd(ctx context.Context, keyVal string, args []string) error {
 		return err
 	}
 
-	if cosign.FileExists("import-cosign.key") {
+	fileExists, err := icos.FileExists("import-cosign.key")
+	if err != nil {
+		return fmt.Errorf("failed checking if import-cosign.key exists: %w", 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/internal/pkg/cosign/common.go b/internal/pkg/cosign/common.go
@@ -0,0 +1,28 @@
+// 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 "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/internal/pkg/cosign/common_test.go b/internal/pkg/cosign/common_test.go
@@ -0,0 +1,51 @@
+//
+// 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 (
+	"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)
+			}
+		})
+	}
+}
diff --git a/pkg/cosign/common.go b/pkg/cosign/common.go
@@ -26,15 +26,6 @@ import (
 	"golang.org/x/term"
 )
 
-// TODO(jason): Move this to an internal package.
-func FileExists(filename string) bool {
-	info, err := os.Stat(filename)
-	if os.IsNotExist(err) {
-		return false
-	}
-	return !info.IsDir()
-}
-
 // 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.