updated per code review

Signed-off-by: Patrick Zheng <[email protected]>

internal/semver/semver.go

@@ -22,11 +22,11 @@ import (
 	"golang.org/x/mod/semver"
 )
 
-// semVerRegEx is takenfrom https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string
+// semVerRegEx is taken from https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string
 var semVerRegEx = regexp.MustCompile(`^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$`)
 
-// IsValidSemver returns true if version is a valid semantic version
-func IsValidSemver(version string) bool {
+// IsValid returns true if version is a valid semantic version
+func IsValid(version string) bool {
 	return semVerRegEx.MatchString(version)
 }
 
@@ -35,10 +35,10 @@ func IsValidSemver(version string) bool {
 // The result will be 0 if v == w, -1 if v < w, or +1 if v > w.
 func ComparePluginVersion(v, w string) (int, error) {
 	// sanity check
-	if !IsValidSemver(v) {
+	if !IsValid(v) {
 		return 0, fmt.Errorf("%s is not a valid semantic version", v)
 	}
-	if !IsValidSemver(w) {
+	if !IsValid(w) {
 		return 0, fmt.Errorf("%s is not a valid semantic version", w)
 	}
 

plugin/manager.go

@@ -184,8 +184,10 @@ func (m *CLIManager) Install(ctx context.Context, installOpts CLIInstallOptions)
 		return nil, nil, fmt.Errorf("failed to get the system path of plugin %s: %w", pluginName, err)
 	}
 	// clean up before installation, this guarantees idempotent for install
-	if err := os.RemoveAll(pluginDirPath); err != nil {
-		return nil, nil, fmt.Errorf("failed to clean up %s before installation: %w", pluginDirPath, err)
+	if err := m.Uninstall(ctx, pluginName); err != nil {
+		if !errors.Is(err, os.ErrNotExist) {
+			return nil, nil, fmt.Errorf("failed to clean up plugin %s before installation: %w", pluginName, err)
+		}
 	}
 	if installFromNonDir {
 		if err := file.CopyToDir(pluginExecutableFile, pluginDirPath); err != nil {
@@ -258,13 +260,14 @@ func parsePluginFromDir(path string) (string, string, error) {
 			if err != nil {
 				return err
 			}
-			if isExec {
-				if foundPluginExecutableFile {
-					return errors.New("found more than one plugin executable files")
-				}
-				foundPluginExecutableFile = true
-				pluginExecutableFile = p
+			if !isExec {
+				return nil
+			}
+			if foundPluginExecutableFile {
+				return errors.New("found more than one plugin executable files")
 			}
+			foundPluginExecutableFile = true
+			pluginExecutableFile = p
 		}
 		return nil
 	}); err != nil {

plugin/manager_unix.go

@@ -32,8 +32,9 @@ func isExecutableFile(filePath string) (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	if !fi.Mode().IsRegular() {
+	mode := fi.Mode()
+	if !mode.IsRegular() {
 		return false, ErrNotRegularFile
 	}
-	return fi.Mode()&0100 != 0, nil
+	return mode.Perm()&0100 != 0, nil
 }

plugin/manager_windows.go

@@ -16,6 +16,7 @@ package plugin
 import (
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/notaryproject/notation-go/plugin/proto"
 )
@@ -33,5 +34,5 @@ func isExecutableFile(filePath string) (bool, error) {
 	if !fi.Mode().IsRegular() {
 		return false, ErrNotRegularFile
 	}
-	return filepath.Ext(filepath.Base(filePath)) == ".exe", nil
+	return strings.EqualFold(filepath.Ext(filepath.Base(filePath)), ".exe"), nil
 }

verifier/helpers.go

@@ -149,7 +149,7 @@ func getVerificationPluginMinVersion(signerInfo *signature.SignerInfo) (string,
 	if strings.TrimSpace(version) == "" {
 		return "", fmt.Errorf("%v from extended attribute is an empty string", HeaderVerificationPluginMinVersion)
 	}
-	if !notationsemver.IsValidSemver(version) {
+	if !notationsemver.IsValid(version) {
 		return "", fmt.Errorf("%v from extended attribute is not a valid SemVer", HeaderVerificationPluginMinVersion)
 	}
 	return version, nil

verifier/verifier.go

@@ -232,7 +232,7 @@ func (v *verifier) processSignature(ctx context.Context, sigBlob []byte, envelop
 		pluginVersion := metadata.Version
 
 		//checking if the plugin version is in valid semver format
-		if !notationsemver.IsValidSemver(pluginVersion) {
+		if !notationsemver.IsValid(pluginVersion) {
 			return notation.ErrorVerificationInconclusive{Msg: fmt.Sprintf("plugin %s has pluginVersion %s which is not in valid semver format", verificationPluginName, pluginVersion)}
 		}