updated per code review

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Two-Hearts · Dec 28, 2023 · f1901d1 · f1901d1
1 parent 5e8387f
commit f1901d1
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 6 deletions.
diff --git a/cmd/notation/internal/plugin/plugin.go b/cmd/notation/internal/plugin/plugin.go
@@ -80,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 reached the 256 MiB size limit", resp.Request.Method, resp.Request.URL)
+		return fmt.Errorf("%s %q: https response reached the %d MiB size limit", resp.Request.Method, resp.Request.URL, MaxPluginSourceBytes/1024/1024)
 	}
 	return nil
 }
diff --git a/cmd/notation/plugin/install.go b/cmd/notation/plugin/install.go
@@ -107,7 +107,7 @@ 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 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().BoolVar(&opts.isURL, "url", false, fmt.Sprintf("install plugin from an HTTPS URL. The default 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")

diff --git a/internal/osutil/file.go b/internal/osutil/file.go
@@ -16,7 +16,6 @@ package osutil
 import (
 	"crypto/sha256"
 	"encoding/hex"
-	"errors"
 	"fmt"
 	"io"
 	"io/fs"
@@ -120,7 +119,7 @@ func CopyFromReaderToDir(src io.Reader, dst string, perm fs.FileMode) error {
 		if err != nil {
 			return err
 		}
-		return errors.New("file reached the 256 MiB size limit")
+		return fmt.Errorf("file reached the %d MiB size limit", MaxFileBytes/1024/1024)
 	}
 	if err := dstFile.Chmod(perm); err != nil {
 		_ = dstFile.Close()
@@ -158,7 +157,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 sha256sum 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
 }
diff --git a/specs/commandline/plugin.md b/specs/commandline/plugin.md
@@ -52,7 +52,7 @@ Flags:
       --force              force the installation of the plugin
   -h, --help               help for install
       --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
+      --url                install plugin from an HTTPS URL. The default plugin download timeout is 10m0s
   -v, --verbose            verbose mode
 
 Aliases:

diff --git a/test/e2e/suite/plugin/install.go b/test/e2e/suite/plugin/install.go
@@ -122,6 +122,13 @@ var _ = Describe("notation plugin install", func() {
 		})
 	})
 
+	It("with valid plugin URL but mismatched SHA-256 checksum", func() {
+		Host(nil, func(notation *utils.ExecOpts, _ *Artifact, vhost *utils.VirtualHost) {
+			notation.ExpectFailure().Exec("plugin", "install", "--url", PluginURL, "--sha256sum", "abcd").
+				MatchErrContent("Error: plugin installation failed: plugin SHA-256 checksum does not match user input. Expecting abcd\n")
+		})
+	})
+
 	It("with invalid plugin URL scheme", func() {
 		Host(nil, func(notation *utils.ExecOpts, _ *Artifact, vhost *utils.VirtualHost) {
 			notation.ExpectFailure().Exec("plugin", "install", "--url", "http://invalid", "--sha256sum", "abcd").