Merge pull request #349 from hyperledger-firefly/fix-deploy

Verify stack type on deploy command and validate chaincode

Author: EnriqueL8

cmd/deploy_ethereum.go

@@ -23,6 +23,7 @@ import (
 	"github.com/hyperledger-firefly/cli/internal/docker"
 	"github.com/hyperledger-firefly/cli/internal/log"
 	"github.com/hyperledger-firefly/cli/internal/stacks"
+	"github.com/hyperledger-firefly/cli/pkg/types"
 	"github.com/spf13/cobra"
 )
 
@@ -55,6 +56,9 @@ solc --combined-json abi,bin contract.sol > contract.json
 		if err := stackManager.LoadStack(stackName); err != nil {
 			return err
 		}
+		if !stackManager.Stack.BlockchainProvider.Equals(types.BlockchainProviderEthereum) {
+			return fmt.Errorf("stack '%s' is not an Ethereum stack (blockchain provider: %s). Use 'ff deploy fabric' for Fabric stacks", stackName, stackManager.Stack.BlockchainProvider)
+		}
 		contractNames, err := stackManager.GetContracts(filename, args[2:])
 		if err != nil {
 			return err

cmd/deploy_fabric.go

@@ -23,6 +23,7 @@ import (
 	"github.com/hyperledger-firefly/cli/internal/docker"
 	"github.com/hyperledger-firefly/cli/internal/log"
 	"github.com/hyperledger-firefly/cli/internal/stacks"
+	"github.com/hyperledger-firefly/cli/pkg/types"
 	"github.com/spf13/cobra"
 )
 
@@ -49,6 +50,9 @@ var deployFabricCmd = &cobra.Command{
 		if err := stackManager.LoadStack(stackName); err != nil {
 			return err
 		}
+		if !stackManager.Stack.BlockchainProvider.Equals(types.BlockchainProviderFabric) {
+			return fmt.Errorf("stack '%s' is not a Fabric stack (blockchain provider: %s). Use 'ff deploy ethereum' for Ethereum-based stacks", stackName, stackManager.Stack.BlockchainProvider)
+		}
 		contractAddress, err := stackManager.DeployContract(filename, filename, 0, args[2:])
 		if err != nil {
 			return fmt.Errorf("%s. usage: %s deploy <stack_name> <filename> <channel> <chaincode> <version>", err.Error(), ExecutableName)

cmd/init_cardano.go

@@ -49,8 +49,10 @@ var initCardanoCmd = &cobra.Command{
 			return err
 		}
 		if err := stackManager.InitStack(&initOptions); err != nil {
-			if err := stackManager.RemoveStack(); err != nil {
-				return err
+			// Try to clean up, but log rather than return any error to not mask the original error
+			if verr := stackManager.RemoveStack(); verr != nil {
+				l := log.LoggerFromContext(ctx)
+				l.Info(fmt.Sprintf("Error whilst removing the stack: %s", verr.Error()))
 			}
 			return err
 		}

cmd/init_fabric.go

@@ -54,8 +54,10 @@ var initFabricCmd = &cobra.Command{
 			return err
 		}
 		if err := stackManager.InitStack(&initOptions); err != nil {
-			if err := stackManager.RemoveStack(); err != nil {
-				return err
+			// Try to clean up, but log rather than return any error to not mask the original error
+			if verr := stackManager.RemoveStack(); verr != nil {
+				l := log.LoggerFromContext(ctx)
+				l.Info(fmt.Sprintf("Error whilst removing the stack: %s", verr.Error()))
 			}
 			return err
 		}

cmd/init_tezos.go

@@ -50,8 +50,10 @@ var initTezosCmd = &cobra.Command{
 			return err
 		}
 		if err := stackManager.InitStack(&initOptions); err != nil {
-			if err := stackManager.RemoveStack(); err != nil {
-				return err
+			// Try to clean up, but log rather than return any error to not mask the original error
+			if verr := stackManager.RemoveStack(); verr != nil {
+				l := log.LoggerFromContext(ctx)
+				l.Info(fmt.Sprintf("Error whilst removing the stack: %s", verr.Error()))
 			}
 			return err
 		}

cmd/pull.go

@@ -63,16 +63,9 @@ Pull the images for a stack .
 		if err := stackManager.LoadStack(stackName); err != nil {
 			return err
 		}
-		if spin != nil {
-			spin.Start()
-		}
-		if err := stackManager.PullStack(&pullOptions); err != nil {
-			return err
-		}
-		if spin != nil {
-			spin.Stop()
-		}
-		return nil
+		return withSpinner(spin, func() error {
+			return stackManager.PullStack(&pullOptions)
+		})
 	},
 }
 

cmd/root.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/briandowns/spinner"
 	"github.com/mattn/go-isatty"
 	"github.com/spf13/cobra"
 
@@ -42,6 +43,16 @@ var logger log.Logger = &log.StdoutLogger{
 // name of the executable, this is for the help messages
 var ExecutableName string = os.Args[0]
 
+// withSpinner runs fn with the spinner (if configured) running, and stops it
+// before returning so an error is not overlaid by the spinner in the user's terminal
+func withSpinner(spin *spinner.Spinner, fn func() error) error {
+	if spin != nil {
+		spin.Start()
+		defer spin.Stop()
+	}
+	return fn()
+}
+
 func GetFireflyASCIIArt() string {
 	s := ""
 	s += "\u001b[33m    _______           ________     \u001b[0m\n"   // yellow

cmd/start.go

@@ -71,15 +71,12 @@ This command will start a stack and run it in the background.
 			fmt.Println("this will take a few seconds longer since this is the first time you're running this stack...")
 		}
 
-		if spin != nil {
-			spin.Start()
-		}
-		messages, err := stackManager.StartStack(&startOptions)
-		if err != nil {
+		var messages []string
+		if err := withSpinner(spin, func() (err error) {
+			messages, err = stackManager.StartStack(&startOptions)
+			return err
+		}); err != nil {
 			return err
-		}
-		if spin != nil {
-			spin.Stop()
 		}
 		fmt.Print("\n\n")
 		for _, message := range messages {

internal/blockchain/fabric/fabric_provider.go

@@ -17,11 +17,14 @@
 package fabric
 
 import (
+	"archive/tar"
+	"compress/gzip"
 	"context"
 	_ "embed"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"path"
 	"path/filepath"
@@ -438,7 +441,7 @@ func (p *FabricProvider) queryInstalled() (*QueryInstalledResponse, error) {
 	var res *QueryInstalledResponse
 	err = json.Unmarshal([]byte(str), &res)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to parse queryinstalled response as JSON: %w. Raw output: %s", err, str)
 	}
 	return res, nil
 }
@@ -515,11 +518,58 @@ func (p *FabricProvider) GetContracts(filename string, extraArgs []string) ([]st
 	return []string{filename}, nil
 }
 
+// validateChaincodePackage checks that the chaincode package file exists and is a valid tar.gz file.
+// Fabric chaincode packages created by 'peer lifecycle chaincode package' are tar.gz files.
+func (p *FabricProvider) validateChaincodePackage(filename string) error {
+	// Check if file exists
+	fileInfo, err := os.Stat(filename)
+	if os.IsNotExist(err) {
+		return fmt.Errorf("chaincode package file not found: %s", filename)
+	}
+	if err != nil {
+		return fmt.Errorf("error accessing chaincode package file: %w", err)
+	}
+	if fileInfo.IsDir() {
+		return fmt.Errorf("chaincode package path is a directory, expected a file: %s", filename)
+	}
+
+	// Verify the file is a valid gzip archive
+	file, err := os.Open(filename)
+	if err != nil {
+		return fmt.Errorf("failed to open chaincode package file: %w", err)
+	}
+	defer file.Close()
+
+	gzReader, err := gzip.NewReader(file)
+	if err != nil {
+		return fmt.Errorf("invalid chaincode package file format. Expected a gzip-compressed tar archive (.tar.gz or .tgz) created by 'peer lifecycle chaincode package'. The file '%s' does not appear to be a valid gzip file: %w", filename, err)
+	}
+	defer gzReader.Close()
+
+	// Verify the gzip contains a valid tar archive by reading at least one header
+	tarReader := tar.NewReader(gzReader)
+	_, err = tarReader.Next()
+	if err == io.EOF {
+		return fmt.Errorf("invalid chaincode package: the tar.gz archive is empty")
+	}
+	if err != nil {
+		return fmt.Errorf("invalid chaincode package file format. The file '%s' is gzip-compressed but does not contain a valid tar archive: %w", filename, err)
+	}
+
+	return nil
+}
+
 func (p *FabricProvider) DeployContract(filename, contractName, instanceName string, member *types.Organization, extraArgs []string) (*types.ContractDeploymentResult, error) {
 	filename, err := filepath.Abs(filename)
 	if err != nil {
 		return nil, err
 	}
+
+	// Validate that the chaincode package file exists and is a valid gzip file
+	if err := p.validateChaincodePackage(filename); err != nil {
+		return nil, err
+	}
+
 	switch {
 	case len(extraArgs) < 1:
 		return nil, fmt.Errorf("channel not set")

internal/blockchain/fabric/fabric_provider_test.go

@@ -1,6 +1,9 @@
 package fabric
 
 import (
+	"archive/tar"
+	"bytes"
+	"compress/gzip"
 	"context"
 	"fmt"
 	"os"
@@ -404,3 +407,141 @@ func TestRegisterIdentity(t *testing.T) {
 	})
 
 }
+
+func TestValidateChaincodePackage(t *testing.T) {
+	p := &FabricProvider{}
+
+	// Helper function to create a valid tar.gz file
+	createValidTarGz := func(t *testing.T, filename string) {
+		var buf bytes.Buffer
+		gzWriter := gzip.NewWriter(&buf)
+		tarWriter := tar.NewWriter(gzWriter)
+
+		// Add a file to the tar archive
+		content := []byte("test chaincode content")
+		header := &tar.Header{
+			Name: "metadata.json",
+			Mode: 0644,
+			Size: int64(len(content)),
+		}
+		err := tarWriter.WriteHeader(header)
+		assert.NoError(t, err)
+		_, err = tarWriter.Write(content)
+		assert.NoError(t, err)
+
+		err = tarWriter.Close()
+		assert.NoError(t, err)
+		err = gzWriter.Close()
+		assert.NoError(t, err)
+
+		err = os.WriteFile(filename, buf.Bytes(), 0644)
+		assert.NoError(t, err)
+	}
+
+	t.Run("valid tar.gz file", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		validTarGzFile := filepath.Join(tmpDir, "valid.tar.gz")
+		createValidTarGz(t, validTarGzFile)
+
+		err := p.validateChaincodePackage(validTarGzFile)
+		assert.NoError(t, err)
+	})
+
+	t.Run("file not found", func(t *testing.T) {
+		err := p.validateChaincodePackage("/nonexistent/path/chaincode.tar.gz")
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "chaincode package file not found")
+	})
+
+	t.Run("path is a directory", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		err := p.validateChaincodePackage(tmpDir)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "chaincode package path is a directory")
+	})
+
+	t.Run("invalid - plain text file", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		plainTextFile := filepath.Join(tmpDir, "plain.txt")
+		err := os.WriteFile(plainTextFile, []byte("this is not a gzip file"), 0644)
+		assert.NoError(t, err)
+
+		err = p.validateChaincodePackage(plainTextFile)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "invalid chaincode package file format")
+		assert.Contains(t, err.Error(), "does not appear to be a valid gzip file")
+	})
+
+	t.Run("invalid - zip file", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		// ZIP files start with PK (0x50, 0x4B)
+		zipLikeFile := filepath.Join(tmpDir, "fake.zip")
+		err := os.WriteFile(zipLikeFile, []byte{0x50, 0x4B, 0x03, 0x04, 0x00, 0x00}, 0644)
+		assert.NoError(t, err)
+
+		err = p.validateChaincodePackage(zipLikeFile)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "invalid chaincode package file format")
+	})
+
+	t.Run("invalid - random binary", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		randomFile := filepath.Join(tmpDir, "random.bin")
+		err := os.WriteFile(randomFile, []byte{0xDE, 0xAD, 0xBE, 0xEF}, 0644)
+		assert.NoError(t, err)
+
+		err = p.validateChaincodePackage(randomFile)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "invalid chaincode package file format")
+	})
+
+	t.Run("invalid - gzip but not tar (plain gzipped content)", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		gzipOnlyFile := filepath.Join(tmpDir, "gzip_only.gz")
+
+		var buf bytes.Buffer
+		gzWriter := gzip.NewWriter(&buf)
+		_, err := gzWriter.Write([]byte("this is gzipped but not a tar archive"))
+		assert.NoError(t, err)
+		err = gzWriter.Close()
+		assert.NoError(t, err)
+
+		err = os.WriteFile(gzipOnlyFile, buf.Bytes(), 0644)
+		assert.NoError(t, err)
+
+		err = p.validateChaincodePackage(gzipOnlyFile)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "does not contain a valid tar archive")
+	})
+
+	t.Run("invalid - empty tar.gz", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		emptyTarGzFile := filepath.Join(tmpDir, "empty.tar.gz")
+
+		var buf bytes.Buffer
+		gzWriter := gzip.NewWriter(&buf)
+		tarWriter := tar.NewWriter(gzWriter)
+		// Close without adding any files
+		err := tarWriter.Close()
+		assert.NoError(t, err)
+		err = gzWriter.Close()
+		assert.NoError(t, err)
+
+		err = os.WriteFile(emptyTarGzFile, buf.Bytes(), 0644)
+		assert.NoError(t, err)
+
+		err = p.validateChaincodePackage(emptyTarGzFile)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "tar.gz archive is empty")
+	})
+
+	t.Run("invalid - empty file", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		emptyFile := filepath.Join(tmpDir, "empty.tar.gz")
+		err := os.WriteFile(emptyFile, []byte{}, 0644)
+		assert.NoError(t, err)
+
+		err = p.validateChaincodePackage(emptyFile)
+		assert.Error(t, err)
+	})
+}