fix: notation plugin install error messages and tests

cmd/notation/internal/plugin/plugin.go

@@ -52,7 +52,8 @@ const (
 // URL
 const DownloadPluginFromURLTimeout = 10 * time.Minute
 
-// DownloadPluginFromURL downloads plugin file from url to a tmp directory
+// DownloadPluginFromURL downloads plugin source from url to a tmp file on file
+// system
 func DownloadPluginFromURL(ctx context.Context, pluginURL string, tmpFile io.Writer) error {
 	// Get the data
 	client := httputil.NewAuthClient(ctx, &http.Client{Timeout: DownloadPluginFromURLTimeout})
@@ -79,7 +80,7 @@ func DownloadPluginFromURL(ctx context.Context, pluginURL string, tmpFile io.Wri
 		return err
 	}
 	if lr.N == 0 {
-		return fmt.Errorf("%s %q: https response reaches the %d MiB size limit", resp.Request.Method, resp.Request.URL, MaxPluginSourceBytes)
+		return fmt.Errorf("%s %q: https response reached the 256 MiB size limit", resp.Request.Method, resp.Request.URL)
 	}
 	return nil
 }

cmd/notation/plugin/install.go

@@ -58,7 +58,7 @@ func installCommand(opts *pluginInstallOpts) *cobra.Command {
 	command := &cobra.Command{
 		Use:     "install [flags] <--file|--url> <plugin_source>",
 		Aliases: []string{"add"},
-		Short:   "Install plugin",
+		Short:   "Install a plugin",
 		Long: `Install a plugin
 
 Example - Install plugin from file system:
@@ -73,6 +73,9 @@ Example - Install plugin from file system regardless if it's already installed:
 Example - Install plugin from file system with .tar.gz:
   notation plugin install --file wabbit-plugin-v1.0.tar.gz
 
+Example - Install plugin from file system with a single plugin executable file:
+  notation plugin install --file notation-wabbit-plugin
+
 Example - Install plugin from URL, SHA256 checksum is required:
   notation plugin install --url https://wabbit-networks.com/intaller/linux/amd64/wabbit-plugin-v1.0.tar.gz --sha256sum f8a75d9234db90069d9eb5660e5374820edf36d710bd063f4ef81e7063d3810b
 `,
@@ -84,10 +87,10 @@ Example - Install plugin from URL, SHA256 checksum is required:
 				case opts.isURL:
 					return errors.New("missing plugin URL")
 				}
-				return errors.New("missing plugin source")
+				return errors.New("missing plugin source location")
 			}
 			if len(args) > 1 {
-				return fmt.Errorf("can only insall one plugin at a time, but got %v", args)
+				return fmt.Errorf("can only install one plugin at a time, but got %v", args)
 			}
 			opts.pluginSource = args[0]
 			return nil
@@ -103,8 +106,8 @@ Example - Install plugin from URL, SHA256 checksum is required:
 		},
 	}
 	opts.LoggingFlagOpts.ApplyFlags(command.Flags())
-	command.Flags().BoolVar(&opts.isFile, "file", false, "install plugin from a file in file system")
-	command.Flags().BoolVar(&opts.isURL, "url", false, "install plugin from an HTTPS URL. The timeout of the download HTTPS request is set to 10 minutes")
+	command.Flags().BoolVar(&opts.isFile, "file", false, "install plugin from a file on file system")
+	command.Flags().BoolVar(&opts.isURL, "url", false, "install plugin from an HTTPS URL. The default plugin download timeout is 10 minutes")
 	command.Flags().StringVar(&opts.inputChecksum, "sha256sum", "", "must match SHA256 of the plugin source, required when \"--url\" flag is set")
 	command.Flags().BoolVar(&opts.force, "force", false, "force the installation of the plugin")
 	command.MarkFlagsMutuallyExclusive("file", "url")
@@ -135,7 +138,7 @@ func install(command *cobra.Command, opts *pluginInstallOpts) error {
 		}
 		tmpFile, err := os.CreateTemp("", notationPluginDownloadTmpFile)
 		if err != nil {
-			return fmt.Errorf("failed to create notationPluginDownloadTmpFile: %w", err)
+			return fmt.Errorf("failed to create temporary file required for downloading plugin: %w", err)
 		}
 		defer os.Remove(tmpFile.Name())
 		defer tmpFile.Close()
@@ -200,17 +203,17 @@ func installPlugin(ctx context.Context, inputPath string, inputChecksum string,
 // from a fs.FS
 //
 // Note: zip.ReadCloser implments fs.FS
-func installPluginFromFS(ctx context.Context, pluginFs fs.FS, force bool) error {
+func installPluginFromFS(ctx context.Context, pluginFS fs.FS, force bool) error {
 	// set up logger
 	logger := log.GetLogger(ctx)
 	root := "."
 	// extracting all regular files from root into tmpDir
 	tmpDir, err := os.MkdirTemp("", notationPluginTmpDir)
 	if err != nil {
-		return fmt.Errorf("failed to create notationPluginTmpDir: %w", err)
+		return fmt.Errorf("failed to create temporary directory: %w", err)
 	}
 	defer os.RemoveAll(tmpDir)
-	if err := fs.WalkDir(pluginFs, root, func(path string, d fs.DirEntry, err error) error {
+	if err := fs.WalkDir(pluginFS, root, func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
@@ -223,12 +226,12 @@ func installPluginFromFS(ctx context.Context, pluginFs fs.FS, force bool) error
 			return err
 		}
 		// only accept regular files.
-		// it is required by github-advanced-security to check for `..` in fName
+		// check for `..` in fName to avoid zip slip vulnerability
 		if !info.Mode().IsRegular() || strings.Contains(fName, "..") {
 			return nil
 		}
 		logger.Debugf("Extracting file %s...", fName)
-		rc, err := pluginFs.Open(path)
+		rc, err := pluginFS.Open(path)
 		if err != nil {
 			return err
 		}
@@ -264,7 +267,7 @@ func installPluginFromTarGz(ctx context.Context, tarGzPath string, force bool) e
 	// extracting all regular files into tmpDir
 	tmpDir, err := os.MkdirTemp("", notationPluginTmpDir)
 	if err != nil {
-		return fmt.Errorf("failed to create notationPluginTmpDir: %w", err)
+		return fmt.Errorf("failed to create temporary directory: %w", err)
 	}
 	defer os.RemoveAll(tmpDir)
 	for {
@@ -276,7 +279,7 @@ func installPluginFromTarGz(ctx context.Context, tarGzPath string, force bool) e
 			return err
 		}
 		// only accept regular files.
-		// it is required by github-advanced-security to check for `..` in fName
+		// check for `..` in fName to avoid zip slip vulnerability
 		if !header.FileInfo().Mode().IsRegular() || strings.Contains(header.Name, "..") {
 			continue
 		}
@@ -307,9 +310,9 @@ func installPluginWithOptions(ctx context.Context, opts plugin.CLIInstallOptions
 		return err
 	}
 	if existingPluginMetadata != nil {
-		fmt.Printf("Succussefully installed plugin %s, updated the version from %s to %s\n", newPluginMetadata.Name, existingPluginMetadata.Version, newPluginMetadata.Version)
+		fmt.Printf("Successfully updated plugin %s from version %s to %s\n", newPluginMetadata.Name, existingPluginMetadata.Version, newPluginMetadata.Version)
 	} else {
-		fmt.Printf("Succussefully installed plugin %s, version %s\n", newPluginMetadata.Name, newPluginMetadata.Version)
+		fmt.Printf("Successfully installed plugin %s, version %s\n", newPluginMetadata.Name, newPluginMetadata.Version)
 	}
 	return nil
 }

cmd/notation/plugin/uninstall.go

@@ -87,7 +87,7 @@ func uninstallPlugin(command *cobra.Command, opts *pluginUninstallOpts) error {
 	}
 	mgr := plugin.NewCLIManager(dir.PluginFS())
 	if err := mgr.Uninstall(ctx, pluginName); err != nil {
-		return fmt.Errorf("failed to uninstall %s: %w", pluginName, err)
+		return fmt.Errorf("failed to uninstall plugin %s: %w", pluginName, err)
 	}
 	fmt.Printf("Successfully uninstalled plugin %s\n", pluginName)
 	return nil

internal/osutil/file.go

@@ -16,6 +16,7 @@ package osutil
 import (
 	"crypto/sha256"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"io"
 	"io/fs"
@@ -119,7 +120,7 @@ func CopyFromReaderToDir(src io.Reader, dst string, perm fs.FileMode) error {
 		if err != nil {
 			return err
 		}
-		return fmt.Errorf("file reaches the %d MiB size limit", MaxFileBytes)
+		return errors.New("file reached the 256 MiB size limit")
 	}
 	if err := dstFile.Chmod(perm); err != nil {
 		_ = dstFile.Close()
@@ -157,7 +158,7 @@ func ValidateSHA256Sum(path string, checksum string) error {
 	sha256sum := sha256Hash.Sum(nil)
 	enc := hex.EncodeToString(sha256sum[:])
 	if !strings.EqualFold(enc, checksum) {
-		return fmt.Errorf("plugin checksum does not match user input. Expecting %s", checksum)
+		return fmt.Errorf("plugin sha256sum does not match user input. Expecting %s", checksum)
 	}
 	return nil
 }

internal/osutil/file_test.go

@@ -261,11 +261,11 @@ func TestCopyToDir(t *testing.T) {
 }
 
 func TestValidateChecksum(t *testing.T) {
-	expectedErrorMsg := "plugin checksum does not match user input. Expecting abcd123"
+	expectedErrorMsg := "plugin sha256sum does not match user input. Expecting abcd123"
 	if err := ValidateSHA256Sum("./testdata/test", "abcd123"); err == nil || err.Error() != expectedErrorMsg {
-		t.Fatalf("expected err %s, got %v", expectedErrorMsg, err)
+		t.Fatalf("expected err %s, but got %v", expectedErrorMsg, err)
 	}
 	if err := ValidateSHA256Sum("./testdata/test", "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"); err != nil {
-		t.Fatalf("expected nil err, got %v", err)
+		t.Fatalf("expected nil err, but got %v", err)
 	}
 }

specs/commandline/plugin.md

@@ -48,11 +48,11 @@ Usage:
 
 Flags:
   -d, --debug              debug mode
-      --file               install plugin from a file in file system
-      --force              force the installation of a plugin
+      --file               install plugin from a file on file system
+      --force              force the installation of the plugin
   -h, --help               help for install
-      --sha256sum string   must match SHA256 of the plugin source
-      --url                install plugin from an HTTPS URL
+      --sha256sum string   must match SHA256 of the plugin source, required when "--url" flag is set
+      --url                install plugin from an HTTPS URL. The default plugin download timeout is 10 minutes
   -v, --verbose            verbose mode
 
 Aliases:
@@ -68,7 +68,9 @@ Usage:
   notation plugin uninstall [flags] <plugin_name>
 
 Flags:
+  -d, --debug         debug mode
   -h, --help          help for remove
+  -v, --verbose       verbose mode
   -y, --yes           do not prompt for confirmation
 Aliases:
   uninstall, remove, rm
@@ -80,7 +82,7 @@ Aliases:
 
 ### Install a plugin from file system
 
-Install a Notation plugin from file system. Plugin file supports `.zip` and `.tar.gz` format. The checksum validation is optional for this case. 
+Install a Notation plugin from file system. Plugin file supports `.zip`, `.tar.gz`, and single plugin executable file format. The checksum validation is optional for this case. 
 
 ```shell
 $ notation plugin install --file <file_path>
@@ -95,13 +97,13 @@ Successfully installed plugin <plugin name>, version <x.y.z>
 If the entered plugin checksum digest doesn't match the published checksum, Notation will return an error message and will not start installation.
 
 ```console
-Error: plugin installation failed: plugin checksum does not match user input. Expecting <sha256sum>
+Error: plugin installation failed: plugin sha256sum does not match user input. Expecting <sha256sum>
 ```
 
 If the plugin version is higher than the existing plugin, Notation will start installation and overwrite the existing plugin.
 
 ```console
-Successfully installed plugin <plugin name>, updated the version from <x.y.z> to <a.b.c>
+Successfully updated plugin <plugin name> from version <x.y.z> to <a.b.c>
 ```
 
 If the plugin version is equal to the existing plugin, Notation will not start installation and return the following message. Users can use a flag `--force` to skip plugin version check and force the installation.
@@ -113,7 +115,7 @@ Error: plugin installation failed: plugin <plugin-name> with version <x.y.z> alr
 If the plugin version is lower than the existing plugin, Notation will return an error message and will not start installation. Users can use a flag `--force` to skip plugin version check and force the installation.
 
 ```console
-Error: failed to install plugin: <plugin-name>. The installing plugin version <a.b.c> is lower than the existing plugin version <x.y.z>.
+Error: plugin installation failed: failed to install plugin <plugin-name>. The installing plugin version <a.b.c> is lower than the existing plugin version <x.y.z>.
 It is not recommended to install an older version. To force the installation, use the "--force" option.
 ```
 ### Install a plugin from URL
@@ -134,7 +136,7 @@ Upon successful execution, the plugin is uninstalled from the plugin directory.
 
 ```shell
 Are you sure you want to uninstall plugin "<plugin name>"? [y/n] y
-Successfully uninstalled <plugin_name> 
+Successfully uninstalled plugin <plugin_name> 
 ```
 
 Uninstall the plugin without prompt for confirmation.
@@ -147,7 +149,7 @@ If the plugin is not found, an error is returned showing the syntax for the plug
 
 ```shell
 Error: unable to find plugin <plugin_name>. 
-To view a list of installed plugins, use "notation plugin list"
+To view a list of installed plugins, use `notation plugin list`
 ```
 
 ### List installed plugins

test/e2e/internal/notation/init.go

@@ -33,32 +33,34 @@ const (
 )
 
 const (
-	envKeyRegistryHost            = "NOTATION_E2E_REGISTRY_HOST"
-	envKeyRegistryUsername        = "NOTATION_E2E_REGISTRY_USERNAME"
-	envKeyRegistryPassword        = "NOTATION_E2E_REGISTRY_PASSWORD"
-	envKeyDomainRegistryHost      = "NOTATION_E2E_DOMAIN_REGISTRY_HOST"
-	envKeyNotationBinPath         = "NOTATION_E2E_BINARY_PATH"
-	envKeyNotationOldBinPath      = "NOTATION_E2E_OLD_BINARY_PATH"
-	envKeyNotationPluginPath      = "NOTATION_E2E_PLUGIN_PATH"
-	envKeyNotationPluginTarGzPath = "NOTATION_E2E_PLUGIN_TAR_GZ_PATH"
-	envKeyNotationConfigPath      = "NOTATION_E2E_CONFIG_PATH"
-	envKeyOCILayoutPath           = "NOTATION_E2E_OCI_LAYOUT_PATH"
-	envKeyTestRepo                = "NOTATION_E2E_TEST_REPO"
-	envKeyTestTag                 = "NOTATION_E2E_TEST_TAG"
+	envKeyRegistryHost                      = "NOTATION_E2E_REGISTRY_HOST"
+	envKeyRegistryUsername                  = "NOTATION_E2E_REGISTRY_USERNAME"
+	envKeyRegistryPassword                  = "NOTATION_E2E_REGISTRY_PASSWORD"
+	envKeyDomainRegistryHost                = "NOTATION_E2E_DOMAIN_REGISTRY_HOST"
+	envKeyNotationBinPath                   = "NOTATION_E2E_BINARY_PATH"
+	envKeyNotationOldBinPath                = "NOTATION_E2E_OLD_BINARY_PATH"
+	envKeyNotationPluginPath                = "NOTATION_E2E_PLUGIN_PATH"
+	envKeyNotationPluginTarGzPath           = "NOTATION_E2E_PLUGIN_TAR_GZ_PATH"
+	envKeyNotationMaliciouPluginArchivePath = "NOTATION_E2E_MALICIOUS_PLUGIN_ARCHIVE_PATH"
+	envKeyNotationConfigPath                = "NOTATION_E2E_CONFIG_PATH"
+	envKeyOCILayoutPath                     = "NOTATION_E2E_OCI_LAYOUT_PATH"
+	envKeyTestRepo                          = "NOTATION_E2E_TEST_REPO"
+	envKeyTestTag                           = "NOTATION_E2E_TEST_TAG"
 )
 
 var (
 	// NotationBinPath is the notation binary path.
 	NotationBinPath string
 	// NotationOldBinPath is the path of an old version notation binary for
 	// testing forward compatibility.
-	NotationOldBinPath         string
-	NotationE2EPluginPath      string
-	NotationE2EPluginTarGzPath string
-	NotationE2EConfigPath      string
-	NotationE2ELocalKeysDir    string
-	NotationE2ETrustPolicyDir  string
-	NotationE2EConfigJsonDir   string
+	NotationOldBinPath                    string
+	NotationE2EPluginPath                 string
+	NotationE2EPluginTarGzPath            string
+	NotationE2EMaliciousPluginArchivePath string
+	NotationE2EConfigPath                 string
+	NotationE2ELocalKeysDir               string
+	NotationE2ETrustPolicyDir             string
+	NotationE2EConfigJsonDir              string
 )
 
 var (
@@ -93,6 +95,7 @@ func setUpNotationValues() {
 	// set Notation e2e-plugin path
 	setPathValue(envKeyNotationPluginPath, &NotationE2EPluginPath)
 	setPathValue(envKeyNotationPluginTarGzPath, &NotationE2EPluginTarGzPath)
+	setPathValue(envKeyNotationMaliciouPluginArchivePath, &NotationE2EMaliciousPluginArchivePath)
 
 	// set Notation configuration paths
 	setPathValue(envKeyNotationConfigPath, &NotationE2EConfigPath)

test/e2e/run.sh

@@ -73,7 +73,7 @@ go install -mod=mod github.com/onsi/ginkgo/v2/ginkgo@v2.9.5
 
 # build e2e plugin and tar.gz
 PLUGIN_NAME=notation-e2e-plugin
-( cd $CWD/plugin && go build -o ./bin/$PLUGIN_NAME . && echo "e2e plugin built." && tar -czvf ./bin/$PLUGIN_NAME.tar.gz -C ./bin/ $PLUGIN_NAME)
+( cd $CWD/plugin && go build -o ./bin/$PLUGIN_NAME . && echo "e2e plugin built." && tar -czvf ./bin/$PLUGIN_NAME.tar.gz -C ./bin/ $PLUGIN_NAME )
 
 # setup registry
 case $REGISTRY_NAME in
@@ -108,6 +108,7 @@ export NOTATION_E2E_TEST_REPO=e2e
 export NOTATION_E2E_TEST_TAG=v1
 export NOTATION_E2E_PLUGIN_PATH=$CWD/plugin/bin/$PLUGIN_NAME
 export NOTATION_E2E_PLUGIN_TAR_GZ_PATH=$CWD/plugin/bin/$PLUGIN_NAME.tar.gz
+export NOTATION_E2E_MALICIOUS_PLUGIN_ARCHIVE_PATH=$CWD/testdata/malicious-plugin
 
 # run tests
 ginkgo -r -p -v