Correctly check if the package is up to date

pkg/commands/compute/deploy.go

@@ -2,15 +2,13 @@ package compute
 
 import (
 	"bytes"
-	"crypto/sha512"
 	"errors"
 	"fmt"
 	"io"
 	"io/fs"
 	"net/http"
 	"os"
 	"path/filepath"
-	"sort"
 	"strings"
 	"time"
 
@@ -92,7 +90,7 @@ func NewDeployCommand(parent cmd.Registerer, g *global.Data, m manifest.Data) *D
 
 // Exec implements the command interface.
 func (c *DeployCommand) Exec(in io.Reader, out io.Writer) (err error) {
-	fnActivateTrial, source, serviceID, pkgPath, hashSum, err := setupDeploy(c, out)
+	fnActivateTrial, source, serviceID, pkgPath, filesHash, err := setupDeploy(c, out)
 	if err != nil {
 		return err
 	}
@@ -150,7 +148,7 @@ func (c *DeployCommand) Exec(in io.Reader, out io.Writer) (err error) {
 	}
 
 	cont, err = processPackage(
-		c, hashSum, pkgPath, serviceID, serviceVersion.Number, spinner, out,
+		c, filesHash, pkgPath, serviceID, serviceVersion.Number, spinner, out,
 	)
 	if err != nil {
 		return err
@@ -217,7 +215,7 @@ func validStatusCodeRange(status int) bool {
 func setupDeploy(c *DeployCommand, out io.Writer) (
 	fnActivateTrial activator,
 	source manifest.Source,
-	serviceID, pkgPath, hashSum string,
+	serviceID, pkgPath, filesHash string,
 	err error,
 ) {
 	defaultActivator := func(customerID string) error { return nil }
@@ -235,15 +233,15 @@ func setupDeploy(c *DeployCommand, out io.Writer) (
 		cmd.DisplayServiceID(serviceID, flag, source, out)
 	}
 
-	pkgPath, hashSum, err = validatePackage(c.Manifest, c.Package, c.Globals.Verbose(), c.Globals.ErrLog, out)
+	pkgPath, filesHash, err = validatePackage(c.Manifest, c.Package, c.Globals.Verbose(), c.Globals.ErrLog, out)
 	if err != nil {
 		return defaultActivator, source, serviceID, "", "", err
 	}
 
 	endpoint, _ := c.Globals.Endpoint()
 	fnActivateTrial = preconfigureActivateTrial(endpoint, token, c.Globals.HTTPClient)
 
-	return fnActivateTrial, source, serviceID, pkgPath, hashSum, err
+	return fnActivateTrial, source, serviceID, pkgPath, filesHash, err
 }
 
 // validatePackage short-circuits the deploy command if the user hasn't first
@@ -257,21 +255,21 @@ func validatePackage(
 	verbose bool,
 	errLog fsterr.LogInterface,
 	out io.Writer,
-) (pkgPath, hashSum string, err error) {
+) (pkgPath, filesHash string, err error) {
 	err = data.File.ReadError()
 	if err != nil {
 		if packageFlag == "" {
 			if errors.Is(err, os.ErrNotExist) {
 				err = fsterr.ErrReadingManifest
 			}
-			return pkgPath, hashSum, err
+			return pkgPath, filesHash, err
 		}
 
 		// NOTE: Before returning the manifest read error, we'll attempt to read
 		// the manifest from within the given package archive.
 		err := readManifestFromPackageArchive(&data, packageFlag, verbose, out)
 		if err != nil {
-			return pkgPath, hashSum, err
+			return pkgPath, filesHash, err
 		}
 	}
 
@@ -281,22 +279,22 @@ func validatePackage(
 		errLog.AddWithContext(err, map[string]any{
 			"Package path": packageFlag,
 		})
-		return pkgPath, hashSum, err
+		return pkgPath, filesHash, err
 	}
 
 	pkgSize, err := packageSize(pkgPath)
 	if err != nil {
 		errLog.AddWithContext(err, map[string]any{
 			"Package path": pkgPath,
 		})
-		return pkgPath, hashSum, fsterr.RemediationError{
+		return pkgPath, filesHash, fsterr.RemediationError{
 			Inner:       fmt.Errorf("error reading package size: %w", err),
 			Remediation: "Run `fastly compute build` to produce a Compute@Edge package, alternatively use the --package flag to reference a package outside of the current project.",
 		}
 	}
 
 	if pkgSize > MaxPackageSize {
-		return pkgPath, hashSum, fsterr.RemediationError{
+		return pkgPath, filesHash, fsterr.RemediationError{
 			Inner:       fmt.Errorf("package size is too large (%d bytes)", pkgSize),
 			Remediation: fsterr.PackageSizeRemediation,
 		}
@@ -319,15 +317,15 @@ func validatePackage(
 			"Package path": pkgPath,
 			"Package size": pkgSize,
 		})
-		return pkgPath, hashSum, err
+		return pkgPath, filesHash, err
 	}
 
-	hashSum, err = getHashSum(contents)
+	filesHash, err = getFilesHash(pkgPath)
 	if err != nil {
 		return pkgPath, "", err
 	}
 
-	return pkgPath, hashSum, nil
+	return pkgPath, filesHash, nil
 }
 
 // readManifestFromPackageArchive extracts the manifest file from the given
@@ -782,16 +780,16 @@ func checkServiceID(serviceID string, client api.Interface) error {
 	return nil
 }
 
-// pkgCompare compares the local package hashsum against the existing service
+// pkgCompare compares the local package files hash against the existing service
 // package version and exits early with message if identical.
-func pkgCompare(client api.Interface, serviceID string, version int, hashSum string, out io.Writer) (bool, error) {
+func pkgCompare(client api.Interface, serviceID string, version int, filesHash string, out io.Writer) (bool, error) {
 	p, err := client.GetPackage(&fastly.GetPackageInput{
 		ServiceID:      serviceID,
 		ServiceVersion: version,
 	})
 
 	if err == nil {
-		if hashSum == p.Metadata.HashSum {
+		if filesHash == p.Metadata.FilesHash {
 			text.Info(out, "Skipping package deployment, local and service version are identical. (service %v, version %v) ", serviceID, version)
 			return false, nil
 		}
@@ -800,22 +798,6 @@ func pkgCompare(client api.Interface, serviceID string, version int, hashSum str
 	return true, nil
 }
 
-// getHashSum creates a SHA 512 hash from the given file contents in a specific order.
-func getHashSum(contents map[string]*bytes.Buffer) (hash string, err error) {
-	h := sha512.New()
-	keys := make([]string, 0, len(contents))
-	for k := range contents {
-		keys = append(keys, k)
-	}
-	sort.Strings(keys)
-	for _, fname := range keys {
-		if _, err := io.Copy(h, contents[fname]); err != nil {
-			return "", err
-		}
-	}
-	return fmt.Sprintf("%x", h.Sum(nil)), nil
-}
-
 // pkgUpload uploads the package to the specified service and version.
 func pkgUpload(spinner text.Spinner, client api.Interface, serviceID string, version int, path string) error {
 	err := spinner.Start()
@@ -1126,12 +1108,12 @@ func processSetupCreation(
 
 func processPackage(
 	c *DeployCommand,
-	hashSum, pkgPath, serviceID string,
+	filesHash, pkgPath, serviceID string,
 	serviceVersion int,
 	spinner text.Spinner,
 	out io.Writer,
 ) (cont bool, err error) {
-	cont, err = pkgCompare(c.Globals.APIClient, serviceID, serviceVersion, hashSum, out)
+	cont, err = pkgCompare(c.Globals.APIClient, serviceID, serviceVersion, filesHash, out)
 	if err != nil {
 		c.Globals.ErrLog.AddWithContext(err, map[string]any{
 			"Package path":    pkgPath,

pkg/commands/compute/deploy_test.go

@@ -1970,7 +1970,8 @@ func getPackageIdentical(i *fastly.GetPackageInput) (*fastly.Package, error) {
 		ServiceID:      i.ServiceID,
 		ServiceVersion: i.ServiceVersion,
 		Metadata: fastly.PackageMetadata{
-			HashSum: "bf634ccf8be5c8417cf562466ece47ea61056ddeb07273a3d861e8ad757ed3577bc182006d04093c301467cadfd2b1805eedebd1e7cfa0404c723680f2dbc01e",
+			FilesHash: "d8786807216a37608ecd0bc2357c86f883faad89043141f0a147f2c186ce0212333d31229399c131539205908f5cf0884ea64552782544ff9b27416cd5b996b2",
+			HashSum:   "bf634ccf8be5c8417cf562466ece47ea61056ddeb07273a3d861e8ad757ed3577bc182006d04093c301467cadfd2b1805eedebd1e7cfa0404c723680f2dbc01e",
 		},
 	}, nil
 }

pkg/commands/compute/hashfiles.go

@@ -3,16 +3,14 @@ package compute
 import (
 	"archive/tar"
 	"bytes"
-	"compress/gzip"
 	"crypto/sha512"
-	"errors"
 	"fmt"
 	"io"
-	"os"
 	"path/filepath"
 	"sort"
 
 	"github.com/kennygrant/sanitize"
+	"github.com/mholt/archiver/v3"
 
 	"github.com/fastly/cli/pkg/cmd"
 	"github.com/fastly/cli/pkg/global"
@@ -68,27 +66,9 @@ func (c *HashFilesCommand) Exec(in io.Reader, out io.Writer) (err error) {
 		}
 	}
 
-	var r io.Reader
-	// G304 (CWE-22): Potential file inclusion via variable
-	// #nosec
-	r, err = os.Open(pkg)
+	hash, err := getFilesHash(pkg)
 	if err != nil {
-		return fmt.Errorf("failed to open package '%s': %w", pkg, err)
-	}
-
-	zr, err := gzip.NewReader(r)
-	if err != nil {
-		return fmt.Errorf("failed to create a gzip reader: %w", err)
-	}
-
-	files, err := c.ReadFilesFromPackage(tar.NewReader(zr))
-	if err != nil {
-		return fmt.Errorf("failed to read files within the package: %w", err)
-	}
-
-	hash, err := c.GetFilesHash(files)
-	if err != nil {
-		return fmt.Errorf("failed to generate hash from package files: %w", err)
+		return err
 	}
 
 	text.Output(out, hash)
@@ -104,48 +84,21 @@ func (c *HashFilesCommand) Build(in io.Reader, out io.Writer) error {
 	return c.buildCmd.Exec(in, output)
 }
 
-// ReadFilesFromPackage reads all files within the provided package tar and
-// generates a map data structure where the key is the filename and the value is
-// the file contents.
-func (c *HashFilesCommand) ReadFilesFromPackage(tr *tar.Reader) (map[string]*bytes.Buffer, error) {
-	// Store the content of every file within the package.
+// getFilesHash returns a hash of all the files in the package in sorted filename order.
+func getFilesHash(pkgPath string) (string, error) {
 	contents := make(map[string]*bytes.Buffer)
-
-	// Track overall package size.
-	var pkgSize int64
-
-	for {
-		hdr, err := tr.Next()
-		if err == io.EOF {
-			break
-		}
-		if err != nil {
-			return nil, err
-		}
-
-		// Avoids G110: Potential DoS vulnerability via decompression bomb (gosec).
-		pkgSize += hdr.Size
-		if pkgSize > MaxPackageSize {
-			return nil, errors.New("package size exceeded 100MB limit")
-		}
-
-		if hdr.Typeflag != tar.TypeReg {
-			continue
-		}
-
-		contents[hdr.Name] = &bytes.Buffer{}
-
-		_, err = io.CopyN(contents[hdr.Name], tr, hdr.Size)
-		if err != nil {
-			return nil, err
+	if err := validate(pkgPath, func(f archiver.File) error {
+		// This is safe to do - we already verified it in validate().
+		filename := f.Header.(*tar.Header).Name
+		contents[filename] = &bytes.Buffer{}
+		if _, err := io.Copy(contents[filename], f); err != nil {
+			return fmt.Errorf("error reading %s: %w", filename, err)
 		}
+		return nil
+	}); err != nil {
+		return "", err
 	}
 
-	return contents, nil
-}
-
-// GetFilesHash returns a hash of all the filecontent in sorted filename order.
-func (c *HashFilesCommand) GetFilesHash(contents map[string]*bytes.Buffer) (string, error) {
 	keys := make([]string, 0, len(contents))
 	for k := range contents {
 		keys = append(keys, k)
@@ -154,7 +107,7 @@ func (c *HashFilesCommand) GetFilesHash(contents map[string]*bytes.Buffer) (stri
 	h := sha512.New()
 	for _, fname := range keys {
 		if _, err := io.Copy(h, contents[fname]); err != nil {
-			return "", err
+			return "", fmt.Errorf("failed to generate hash from package files: %w", err)
 		}
 	}
 	return fmt.Sprintf("%x", h.Sum(nil)), nil

pkg/commands/compute/hashsum.go

@@ -1,8 +1,10 @@
 package compute
 
 import (
+	"crypto/sha512"
 	"fmt"
 	"io"
+	"os"
 
 	"github.com/fastly/cli/pkg/cmd"
 	fsterr "github.com/fastly/cli/pkg/errors"
@@ -48,7 +50,7 @@ func (c *HashsumCommand) Exec(in io.Reader, out io.Writer) (err error) {
 		}
 	}
 
-	_, hashSum, err := validatePackage(c.Manifest, c.Package, c.Globals.Verbose(), c.Globals.ErrLog, out)
+	pkgPath, _, err := validatePackage(c.Manifest, c.Package, c.Globals.Verbose(), c.Globals.ErrLog, out)
 	if err != nil {
 		var skipBuildMsg string
 		if c.SkipBuild {
@@ -64,6 +66,11 @@ func (c *HashsumCommand) Exec(in io.Reader, out io.Writer) (err error) {
 		text.Break(out)
 	}
 
+	hashSum, err := getHashSum(pkgPath)
+	if err != nil {
+		return err
+	}
+
 	text.Output(out, hashSum)
 	return nil
 }
@@ -76,3 +83,27 @@ func (c *HashsumCommand) Build(in io.Reader, out io.Writer) error {
 	}
 	return c.buildCmd.Exec(in, output)
 }
+
+// getHashSum returns a hash of the package.
+func getHashSum(pkg string) (string, error) {
+	// gosec flagged this:
+	// G304 (CWE-22): Potential file inclusion via variable
+	// Disabling as we trust the source of the filepath variable.
+	/* #nosec */
+	f, err := os.Open(pkg)
+	if err != nil {
+		return "", err
+	}
+
+	h := sha512.New()
+	if _, err := io.Copy(h, f); err != nil {
+		f.Close()
+		return "", err
+	}
+
+	if err = f.Close(); err != nil {
+		return "", err
+	}
+
+	return fmt.Sprintf("%x", h.Sum(nil)), nil
+}