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 %d MiB size limit", resp.Request.Method, resp.Request.URL, MaxPluginSourceBytes/1024/1024)
 	}
 	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, fmt.Sprintf("install plugin from an HTTPS URL. The plugin download timeout is %s", notationplugin.DownloadPluginFromURLTimeout))
 	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()
@@ -157,11 +160,11 @@ func install(command *cobra.Command, opts *pluginInstallOpts) error {
 // installPlugin installs the plugin given plugin source path
 func installPlugin(ctx context.Context, inputPath string, inputChecksum string, force bool) error {
 	// sanity check
-	inputFileStat, err := os.Stat(inputPath)
+	inputFileInfo, err := os.Stat(inputPath)
 	if err != nil {
 		return err
 	}
-	if !inputFileStat.Mode().IsRegular() {
+	if !inputFileInfo.Mode().IsRegular() {
 		return fmt.Errorf("%s is not a valid file", inputPath)
 	}
 	// checksum check
@@ -182,12 +185,21 @@ func installPlugin(ctx context.Context, inputPath string, inputChecksum string,
 			return err
 		}
 		defer rc.Close()
+		// check for '..' in file name to avoid zip slip vulnerability
+		for _, f := range rc.File {
+			if strings.Contains(f.Name, "..") {
+				return fmt.Errorf("file name in zip cannot contain '..', but found %q", f.Name)
+			}
+		}
 		return installPluginFromFS(ctx, rc, force)
 	case notationplugin.MediaTypeGzip:
 		// when file is gzip, required to be tar
 		return installPluginFromTarGz(ctx, inputPath, force)
 	default:
 		// input file is not in zip or gzip, try install directly
+		if inputFileInfo.Size() >= osutil.MaxFileBytes {
+			return fmt.Errorf("file size reached the %d MiB size limit", osutil.MaxFileBytes/1024/1024)
+		}
 		installOpts := plugin.CLIInstallOptions{
 			PluginPath: inputPath,
 			Overwrite:  force,
@@ -200,17 +212,18 @@ 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 {
+	var pluginFileSize int64
+	if err := fs.WalkDir(pluginFS, root, func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
@@ -222,13 +235,17 @@ func installPluginFromFS(ctx context.Context, pluginFs fs.FS, force bool) error
 		if err != nil {
 			return err
 		}
-		// only accept regular files.
-		// it is required by github-advanced-security to check for `..` in fName
-		if !info.Mode().IsRegular() || strings.Contains(fName, "..") {
+		// only accept regular files
+		if !info.Mode().IsRegular() {
 			return nil
 		}
+		// check for plugin file size to avoid zip bomb vulnerability
+		pluginFileSize += info.Size()
+		if pluginFileSize >= osutil.MaxFileBytes {
+			return fmt.Errorf("total file size reached the %d MiB size limit", osutil.MaxFileBytes/1024/1024)
+		}
 		logger.Debugf("Extracting file %s...", fName)
-		rc, err := pluginFs.Open(path)
+		rc, err := pluginFS.Open(path)
 		if err != nil {
 			return err
 		}
@@ -264,9 +281,10 @@ 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)
+	var pluginFileSize int64
 	for {
 		header, err := tarReader.Next()
 		if err != nil {
@@ -275,11 +293,19 @@ 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
-		if !header.FileInfo().Mode().IsRegular() || strings.Contains(header.Name, "..") {
+		// check for '..' in file name to avoid zip slip vulnerability
+		if strings.Contains(header.Name, "..") {
+			return fmt.Errorf("file name in tar.gz cannot contain '..', but found %q", header.Name)
+		}
+		// only accept regular files
+		if !header.FileInfo().Mode().IsRegular() {
 			continue
 		}
+		// check for plugin file size to avoid zip bomb vulnerability
+		pluginFileSize += header.FileInfo().Size()
+		if pluginFileSize >= osutil.MaxFileBytes {
+			return fmt.Errorf("total file size reached the %d MiB size limit", osutil.MaxFileBytes/1024/1024)
+		}
 		fName := filepath.Base(header.Name)
 		logger.Debugf("Extracting file %s...", fName)
 		tmpFilePath := filepath.Join(tmpDir, fName)
@@ -307,9 +333,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

@@ -26,7 +26,7 @@ import (
 )
 
 // MaxFileBytes is the maximum file bytes.
-// When used, the value should strictly less than this number.
+// When used, the value should be strictly less than this number.
 var MaxFileBytes int64 = 256 * 1024 * 1024 // 256 MiB
 
 // WriteFile writes to a path with all parent directories created.
@@ -104,25 +104,18 @@ func IsRegularFile(path string) (bool, error) {
 }
 
 // CopyFromReaderToDir copies file from src to dst where dst is the destination
-// file path. The file size must be less than 256 MiB.
+// file path.
 func CopyFromReaderToDir(src io.Reader, dst string, perm fs.FileMode) error {
 	dstFile, err := os.Create(dst)
 	if err != nil {
 		return err
 	}
-	lr := &io.LimitedReader{
-		R: src,
-		N: MaxFileBytes,
-	}
-	if _, err := io.Copy(dstFile, lr); err != nil || lr.N == 0 {
-		_ = dstFile.Close()
-		if err != nil {
-			return err
-		}
-		return fmt.Errorf("file reaches the %d MiB size limit", MaxFileBytes)
+	if _, err := io.Copy(dstFile, src); err != nil {
+		dstFile.Close()
+		return err
 	}
 	if err := dstFile.Chmod(perm); err != nil {
-		_ = dstFile.Close()
+		dstFile.Close()
 		return err
 	}
 	return dstFile.Close()
@@ -157,7 +150,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 SHA-256 checksum 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 SHA-256 checksum 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 plugin download timeout is 10m0s
   -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 the host file system. `.zip`, `.tar.gz`, and `single plugin executable file` formats are supported. In this scenario, SHA-256 checksum validation is optional.
 
 ```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,12 +115,12 @@ 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
 
-Install a Notation plugin from a remote location and verify the plugin checksum. Notation only supports installing plugins from an HTTPS URL, which means that the URL must start with "https://".
+Install a Notation plugin from a URL. Notation only supports HTTPS URL, which means that the URL must start with "https://". The URL MUST point to a resource in `.zip`, `.tar.gz`, or `single plugin executable file` format.  In this scenario, the SHA-256 checksum of the resource MUST be provided.
 
 ```shell
 $ notation plugin install --sha256sum <digest> --url <HTTPS_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