fix: enforce hash format and fix regression (#49)

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>

Author: migmartri

app/cli/internal/action/artifact_download.go

@@ -54,9 +54,9 @@ func (a *ArtifactDownload) Run(downloadPath, digest string) error {
 
 	client := casclient.New(a.artifactsCASConn)
 	ctx := context.Background()
-	info, err := client.Describe(ctx, h.Hex)
+	info, err := client.Describe(ctx, h.String())
 	if err != nil {
-		return fmt.Errorf("resource with digest %s not found", h)
+		return fmt.Errorf("artifact with digest %s not found", h)
 	}
 
 	if downloadPath == "" {
@@ -84,7 +84,7 @@ func (a *ArtifactDownload) Run(downloadPath, digest string) error {
 	go renderOperationStatus(ctx, client.ProgressStatus, info.Size)
 	defer close(client.ProgressStatus)
 
-	err = client.Download(ctx, w, h.Hex)
+	err = client.Download(ctx, w, h.String())
 	if err != nil {
 		a.Logger.Debug().Err(err).Msg("problem downloading file")
 		return errors.New("problem downloading file")

internal/casclient/downloader.go

@@ -22,20 +22,23 @@ import (
 	"io"
 
 	v1 "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1"
+	cr_v1 "github.com/google/go-containerregistry/pkg/v1"
 	"google.golang.org/genproto/googleapis/bytestream"
 )
 
 // Download downloads a file from the CAS and writes it to the provided writer
 func (c *Client) Download(ctx context.Context, w io.Writer, digest string) error {
-	if digest == "" {
-		return errors.New("a digest is required")
+	// Check digest format, including the algorithm and the hex portion
+	h, err := cr_v1.NewHash(digest)
+	if err != nil {
+		return fmt.Errorf("decoding digest: %w", err)
 	}
 
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
 	// Open the stream to start reading chunks
-	reader, err := bytestream.NewByteStreamClient(c.conn).Read(ctx, &bytestream.ReadRequest{ResourceName: digest})
+	reader, err := bytestream.NewByteStreamClient(c.conn).Read(ctx, &bytestream.ReadRequest{ResourceName: h.Hex})
 	if err != nil {
 		return fmt.Errorf("creating the gRPC client: %w", err)
 	}
@@ -76,8 +79,14 @@ func (c *Client) Download(ctx context.Context, w io.Writer, digest string) error
 // Describe returns the metadata of a resource by its digest
 // We use this to get the filename and the total size of the artifact
 func (c *Client) Describe(ctx context.Context, digest string) (*ResourceInfo, error) {
+	// Check digest format, including the algorithm and the hex portion
+	h, err := cr_v1.NewHash(digest)
+	if err != nil {
+		return nil, fmt.Errorf("decoding digest: %w", err)
+	}
+
 	client := v1.NewResourceServiceClient(c.conn)
-	resp, err := client.Describe(ctx, &v1.ResourceServiceDescribeRequest{Digest: digest})
+	resp, err := client.Describe(ctx, &v1.ResourceServiceDescribeRequest{Digest: h.Hex})
 	if err != nil {
 		return nil, fmt.Errorf("contacting API to get resource Info: %w", err)
 	}

internal/casclient/uploader.go

@@ -56,19 +56,25 @@ func (c *Client) UploadFile(ctx context.Context, filepath string) (*UpDownStatus
 		return nil, fmt.Errorf("rewinding file pointer: %w", err)
 	}
 
-	return c.Upload(ctx, f, path.Base(filepath), hash.Hex)
+	return c.Upload(ctx, f, path.Base(filepath), hash.String())
 }
 
 func (c *Client) Upload(ctx context.Context, r io.Reader, filename, digest string) (*UpDownStatus, error) {
+	// Check digest format, including the algorithm and the hex portion
+	h, err := cr_v1.NewHash(digest)
+	if err != nil {
+		return nil, fmt.Errorf("decoding digest: %w", err)
+	}
+
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
-	resource, err := encodeResource(filename, digest)
+	resource, err := encodeResource(filename, h.String())
 	if err != nil {
 		return nil, fmt.Errorf("encoding resource name: %w", err)
 	}
 
-	c.logger.Info().Msgf("uploading %s - sha256:%s", filename, digest)
+	c.logger.Info().Msgf("uploading %s - %s", filename, h)
 
 	stream, err := bytestream.NewByteStreamClient(c.conn).Write(ctx)
 	if err != nil {
@@ -127,7 +133,7 @@ doUpload:
 
 		latestStatus = &UpDownStatus{
 			Filename:       filename,
-			Digest:         digest,
+			Digest:         h.String(),
 			ProcessedBytes: totalUploaded,
 		}
 
@@ -156,13 +162,16 @@ func encodeResource(fileName, digest string) (string, error) {
 		return "", fmt.Errorf("file name is empty")
 	}
 
-	if digest == "" {
-		return "", fmt.Errorf("digest is empty")
+	// Check digest format, including the algorithm and the hex portion
+	h, err := cr_v1.NewHash(digest)
+	if err != nil {
+		return "", fmt.Errorf("decoding digest: %w", err)
 	}
 
 	var encodedResource bytes.Buffer
 	enc := gob.NewEncoder(&encodedResource)
-	r := &v1.CASResource{FileName: fileName, Digest: digest}
+	// Currently we only support SHA256
+	r := &v1.CASResource{FileName: fileName, Digest: h.Hex}
 
 	if err := enc.Encode(r); err != nil {
 		return "", err

internal/casclient/uploader_test.go

@@ -19,13 +19,16 @@ import (
 	"bytes"
 	"encoding/base64"
 	"encoding/gob"
+	"fmt"
 	"testing"
 
 	v1 "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestEncodeResource(t *testing.T) {
+	const validDigestHex = "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c"
+
 	testCases := []struct {
 		name      string
 		fileName  string
@@ -44,10 +47,22 @@ func TestEncodeResource(t *testing.T) {
 			wantError: true,
 		},
 		{
-			name:     "valid fields",
-			digest:   "deadbeef",
+			name:      "uncompleted digest",
+			digest:    "deadbeef",
+			fileName:  "foo.txt",
+			wantError: true,
+		},
+		{
+			name:      "invalid digest",
+			digest:    "sha256:deadbeef",
+			fileName:  "foo.txt",
+			wantError: true,
+		},
+		{
+			name:     "valid",
+			digest:   fmt.Sprintf("sha256:%s", validDigestHex),
 			fileName: "foo.txt",
-			want:     &v1.CASResource{FileName: "foo.txt", Digest: "deadbeef"},
+			want:     &v1.CASResource{FileName: "foo.txt", Digest: validDigestHex},
 		},
 	}