fix validation error handling on API (#2217)

Signed-off-by: Bob Callaway <bcallaway@google.com>
sigstore · Aug 26, 2024 · c16f610 · c16f610
1 parent a676186
commit c16f610
Show file tree

Hide file tree

Showing 18 changed files with 83 additions and 56 deletions.
diff --git a/pkg/api/entries.go b/pkg/api/entries.go
@@ -187,7 +187,8 @@ func createLogEntry(params entries.CreateLogEntryParams) (models.LogEntry, middl
 	}
 	leaf, err := types.CanonicalizeEntry(ctx, entry)
 	if err != nil {
-		if _, ok := (err).(types.ValidationError); ok {
+		var validationErr *types.InputValidationError
+		if errors.As(err, &validationErr) {
 			return nil, handleRekorAPIError(params, http.StatusBadRequest, err, fmt.Sprintf(validationError, err))
 		}
 		return nil, handleRekorAPIError(params, http.StatusInternalServerError, err, failedToGenerateCanonicalEntry)
@@ -415,8 +416,9 @@ func GetLogEntryByUUIDHandler(params entries.GetLogEntryByUUIDParams) middleware
 		if errors.Is(err, ErrNotFound) {
 			return handleRekorAPIError(params, http.StatusNotFound, err, "")
 		}
-		if _, ok := (err).(types.ValidationError); ok {
-			return handleRekorAPIError(params, http.StatusBadRequest, err, fmt.Sprintf("incorrectly formatted uuid %s", params.EntryUUID), params.EntryUUID)
+		var validationErr *types.InputValidationError
+		if errors.As(err, &validationErr) {
+			return handleRekorAPIError(params, http.StatusBadRequest, err, fmt.Sprintf("validation error: %v", err), params.EntryUUID)
 		}
 		return handleRekorAPIError(params, http.StatusInternalServerError, err, trillianCommunicationError)
 	}
@@ -602,7 +604,7 @@ func retrieveUUIDFromTree(ctx context.Context, uuid string, tid int64) (models.L
 
 	hashValue, err := hex.DecodeString(uuid)
 	if err != nil {
-		return models.LogEntry{}, types.ValidationError(err)
+		return models.LogEntry{}, &types.InputValidationError{Err: fmt.Errorf("parsing UUID: %w", err)}
 	}
 
 	tc := trillianclient.NewTrillianClient(ctx, api.logClient, tid)

diff --git a/pkg/types/alpine/v0.0.1/entry.go b/pkg/types/alpine/v0.0.1/entry.go
@@ -108,7 +108,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 
 func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*x509.PublicKey, *alpine.Package, error) {
 	if err := v.validate(); err != nil {
-		return nil, nil, types.ValidationError(err)
+		return nil, nil, &types.InputValidationError{Err: err}
 	}
 
 	g, ctx := errgroup.WithContext(ctx)
@@ -150,7 +150,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*x509.PublicKey,
 
 		computedSHA := hex.EncodeToString(hasher.Sum(nil))
 		if oldSHA != "" && computedSHA != oldSHA {
-			return closePipesOnError(types.ValidationError(fmt.Errorf("SHA mismatch: %s != %s", computedSHA, oldSHA)))
+			return closePipesOnError(&types.InputValidationError{Err: fmt.Errorf("SHA mismatch: %s != %s", computedSHA, oldSHA)})
 		}
 
 		select {
@@ -169,7 +169,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*x509.PublicKey,
 
 		keyObj, err := x509.NewPublicKey(keyReadCloser)
 		if err != nil {
-			return closePipesOnError(types.ValidationError(err))
+			return closePipesOnError(&types.InputValidationError{Err: err})
 		}
 
 		select {
@@ -186,7 +186,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*x509.PublicKey,
 	g.Go(func() error {
 		apk := alpine.Package{}
 		if err := apk.Unmarshal(apkR); err != nil {
-			return closePipesOnError(types.ValidationError(err))
+			return closePipesOnError(&types.InputValidationError{Err: err})
 		}
 
 		key = <-keyResult
@@ -195,7 +195,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*x509.PublicKey,
 		}
 
 		if err := apk.VerifySignature(key.CryptoPubKey()); err != nil {
-			return closePipesOnError(types.ValidationError(err))
+			return closePipesOnError(&types.InputValidationError{Err: err})
 		}
 
 		apkObj = &apk

diff --git a/pkg/types/alpine/v0.0.1/entry_test.go b/pkg/types/alpine/v0.0.1/entry_test.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
+	"errors"
 	"os"
 	"reflect"
 	"testing"
@@ -163,7 +164,8 @@ func TestCrossFieldValidation(t *testing.T) {
 		if (err == nil) != tc.expectCanonicalizeSuccess {
 			t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
 		} else if err != nil {
-			if _, ok := err.(types.ValidationError); !ok {
+			var validationErr *types.InputValidationError
+			if !errors.As(err, &validationErr) {
 				t.Errorf("canonicalize returned an unexpected error that isn't of type types.ValidationError: %v", err)
 			}
 		}

diff --git a/pkg/types/error.go b/pkg/types/error.go
@@ -17,4 +17,15 @@ package types
 
 // ValidationError indicates that there is an issue with the content in the HTTP Request that
 // should result in an HTTP 400 Bad Request error being returned to the client
+//
+// Deprecated: use InputValidationError instead to take advantage of Go's error wrapping
 type ValidationError error
+
+// InputValidationError indicates that there is an issue with the content in the HTTP Request that
+// should result in an HTTP 400 Bad Request error being returned to the client
+type InputValidationError struct {
+	Err error
+}
+
+func (v *InputValidationError) Error() string { return v.Err.Error() }
+func (v *InputValidationError) Unwrap() error { return v.Err }
diff --git a/pkg/types/hashedrekord/v0.0.1/entry.go b/pkg/types/hashedrekord/v0.0.1/entry.go
@@ -108,7 +108,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 func (v *V001Entry) Canonicalize(_ context.Context) ([]byte, error) {
 	sigObj, keyObj, err := v.validate()
 	if err != nil {
-		return nil, types.ValidationError(err)
+		return nil, &types.InputValidationError{Err: err}
 	}
 
 	canonicalEntry := models.HashedrekordV001Schema{}
@@ -144,34 +144,34 @@ func (v *V001Entry) Canonicalize(_ context.Context) ([]byte, error) {
 func (v *V001Entry) validate() (pki.Signature, pki.PublicKey, error) {
 	sig := v.HashedRekordObj.Signature
 	if sig == nil {
-		return nil, nil, types.ValidationError(errors.New("missing signature"))
+		return nil, nil, &types.InputValidationError{Err: errors.New("missing signature")}
 	}
 	// Hashed rekord type only works for x509 signature types
 	sigObj, err := x509.NewSignatureWithOpts(bytes.NewReader(sig.Content), options.WithED25519ph())
 	if err != nil {
-		return nil, nil, types.ValidationError(err)
+		return nil, nil, &types.InputValidationError{Err: err}
 	}
 
 	key := sig.PublicKey
 	if key == nil {
-		return nil, nil, types.ValidationError(errors.New("missing public key"))
+		return nil, nil, &types.InputValidationError{Err: errors.New("missing public key")}
 	}
 	keyObj, err := x509.NewPublicKey(bytes.NewReader(key.Content))
 	if err != nil {
-		return nil, nil, types.ValidationError(err)
+		return nil, nil, &types.InputValidationError{Err: err}
 	}
 
 	data := v.HashedRekordObj.Data
 	if data == nil {
-		return nil, nil, types.ValidationError(errors.New("missing data"))
+		return nil, nil, &types.InputValidationError{Err: errors.New("missing data")}
 	}
 
 	hash := data.Hash
 	if hash == nil {
-		return nil, nil, types.ValidationError(errors.New("missing hash"))
+		return nil, nil, &types.InputValidationError{Err: errors.New("missing hash")}
 	}
 	if !govalidator.IsHash(swag.StringValue(hash.Value), swag.StringValue(hash.Algorithm)) {
-		return nil, nil, types.ValidationError(errors.New("invalid value for hash"))
+		return nil, nil, &types.InputValidationError{Err: errors.New("invalid value for hash")}
 	}
 
 	var alg crypto.Hash
@@ -189,7 +189,7 @@ func (v *V001Entry) validate() (pki.Signature, pki.PublicKey, error) {
 		return nil, nil, err
 	}
 	if err := sigObj.Verify(nil, keyObj, options.WithDigest(decoded), options.WithCryptoSignerOpts(alg)); err != nil {
-		return nil, nil, types.ValidationError(fmt.Errorf("verifying signature: %w", err))
+		return nil, nil, &types.InputValidationError{Err: fmt.Errorf("verifying signature: %w", err)}
 	}
 
 	return sigObj, keyObj, nil

diff --git a/pkg/types/hashedrekord/v0.0.1/entry_test.go b/pkg/types/hashedrekord/v0.0.1/entry_test.go
@@ -29,6 +29,7 @@ import (
 	"crypto/x509/pkix"
 	"encoding/hex"
 	"encoding/pem"
+	"errors"
 	"math/big"
 	"reflect"
 	"testing"
@@ -451,7 +452,8 @@ func TestCrossFieldValidation(t *testing.T) {
 		if (err == nil) != tc.expectCanonicalizeSuccess {
 			t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
 		} else if err != nil {
-			if _, ok := err.(types.ValidationError); !ok {
+			var validationErr *types.InputValidationError
+			if !errors.As(err, &validationErr) {
 				t.Errorf("canonicalize returned an unexpected error that isn't of type types.ValidationError: %v", err)
 			}
 		}

diff --git a/pkg/types/helm/v0.0.1/entry.go b/pkg/types/helm/v0.0.1/entry.go
@@ -118,7 +118,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 
 func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*helm.Provenance, *pgp.PublicKey, *pgp.Signature, error) {
 	if err := v.validate(); err != nil {
-		return nil, nil, nil, types.ValidationError(err)
+		return nil, nil, nil, &types.InputValidationError{Err: err}
 	}
 
 	g, ctx := errgroup.WithContext(ctx)
@@ -148,7 +148,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*helm.Provenance
 
 		keyObj, err := pgp.NewPublicKey(keyReadCloser)
 		if err != nil {
-			return closePipesOnError(types.ValidationError(err))
+			return closePipesOnError(&types.InputValidationError{Err: err})
 		}
 
 		select {
@@ -165,7 +165,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*helm.Provenance
 	g.Go(func() error {
 
 		if err := provenance.Unmarshal(provenanceR); err != nil {
-			return closePipesOnError(types.ValidationError(err))
+			return closePipesOnError(&types.InputValidationError{Err: err})
 		}
 
 		key = <-keyResult
@@ -177,12 +177,12 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*helm.Provenance
 		var err error
 		sig, err = pgp.NewSignature(provenance.Block.ArmoredSignature.Body)
 		if err != nil {
-			return closePipesOnError(types.ValidationError(err))
+			return closePipesOnError(&types.InputValidationError{Err: err})
 		}
 
 		// Verify signature
 		if err := sig.Verify(bytes.NewReader(provenance.Block.Bytes), key); err != nil {
-			return closePipesOnError(types.ValidationError(err))
+			return closePipesOnError(&types.InputValidationError{Err: err})
 		}
 
 		select {

diff --git a/pkg/types/helm/v0.0.1/entry_test.go b/pkg/types/helm/v0.0.1/entry_test.go
@@ -18,6 +18,7 @@ package helm
 import (
 	"bytes"
 	"context"
+	"errors"
 	"os"
 	"reflect"
 	"testing"
@@ -193,7 +194,8 @@ func TestCrossFieldValidation(t *testing.T) {
 			if (err == nil) != tc.expectCanonicalizeSuccess {
 				t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
 			} else if err != nil {
-				if _, ok := err.(types.ValidationError); !ok {
+				var validationErr *types.InputValidationError
+				if !errors.As(err, &validationErr) {
 					t.Errorf("canonicalize returned an unexpected error that isn't of type types.ValidationError: %v", err)
 				}
 			}

diff --git a/pkg/types/jar/v0.0.1/entry.go b/pkg/types/jar/v0.0.1/entry.go
@@ -112,7 +112,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 
 func (v *V001Entry) fetchExternalEntities(_ context.Context) (*pkcs7.PublicKey, *pkcs7.Signature, error) {
 	if err := v.validate(); err != nil {
-		return nil, nil, types.ValidationError(err)
+		return nil, nil, &types.InputValidationError{Err: err}
 	}
 
 	oldSHA := ""
@@ -132,12 +132,12 @@ func (v *V001Entry) fetchExternalEntities(_ context.Context) (*pkcs7.PublicKey,
 
 	computedSHA := hex.EncodeToString(hasher.Sum(nil))
 	if oldSHA != "" && computedSHA != oldSHA {
-		return nil, nil, types.ValidationError(fmt.Errorf("SHA mismatch: %s != %s", computedSHA, oldSHA))
+		return nil, nil, &types.InputValidationError{Err: fmt.Errorf("SHA mismatch: %s != %s", computedSHA, oldSHA)}
 	}
 
 	zipReader, err := zip.NewReader(bytes.NewReader(b.Bytes()), n)
 	if err != nil {
-		return nil, nil, types.ValidationError(err)
+		return nil, nil, &types.InputValidationError{Err: err}
 	}
 
 	// Checking that uncompressed metadata files are within acceptable bounds before reading into memory.
@@ -149,39 +149,38 @@ func (v *V001Entry) fetchExternalEntities(_ context.Context) (*pkcs7.PublicKey,
 			continue
 		}
 		if f.UncompressedSize64 > viper.GetUint64("max_jar_metadata_size") && viper.GetUint64("max_jar_metadata_size") > 0 {
-			return nil, nil, types.ValidationError(
-				fmt.Errorf("uncompressed jar metadata of size %d exceeds max allowed size %d", f.UncompressedSize64, viper.GetUint64("max_jar_metadata_size")))
+			return nil, nil, &types.InputValidationError{Err: fmt.Errorf("uncompressed jar metadata of size %d exceeds max allowed size %d", f.UncompressedSize64, viper.GetUint64("max_jar_metadata_size"))}
 		}
 	}
 
 	// this ensures that the JAR is signed and the signature verifies, as
 	// well as checks that the hashes in the signed manifest are all valid
 	jarObjs, err := jarutils.Verify(zipReader, false)
 	if err != nil {
-		return nil, nil, types.ValidationError(err)
+		return nil, nil, &types.InputValidationError{Err: err}
 	}
 	switch len(jarObjs) {
 	case 0:
-		return nil, nil, types.ValidationError(errors.New("no signatures detected in JAR archive"))
+		return nil, nil, &types.InputValidationError{Err: errors.New("no signatures detected in JAR archive")}
 	case 1:
 	default:
-		return nil, nil, types.ValidationError(errors.New("multiple signatures detected in JAR; unable to process"))
+		return nil, nil, &types.InputValidationError{Err: errors.New("multiple signatures detected in JAR; unable to process")}
 	}
 
 	// we need to find and extract the PKCS7 bundle from the JAR file manually
 	sigPKCS7, err := extractPKCS7SignatureFromJAR(zipReader)
 	if err != nil {
-		return nil, nil, types.ValidationError(err)
+		return nil, nil, &types.InputValidationError{Err: err}
 	}
 
 	keyObj, err := pkcs7.NewPublicKey(bytes.NewReader(sigPKCS7))
 	if err != nil {
-		return nil, nil, types.ValidationError(err)
+		return nil, nil, &types.InputValidationError{Err: err}
 	}
 
 	sigObj, err := pkcs7.NewSignature(bytes.NewReader(sigPKCS7))
 	if err != nil {
-		return nil, nil, types.ValidationError(err)
+		return nil, nil, &types.InputValidationError{Err: err}
 	}
 
 	// if we get here, all goroutines succeeded without error

diff --git a/pkg/types/jar/v0.0.1/entry_test.go b/pkg/types/jar/v0.0.1/entry_test.go
@@ -18,6 +18,7 @@ package jar
 import (
 	"bytes"
 	"context"
+	"errors"
 	"os"
 	"reflect"
 	"strings"
@@ -124,7 +125,8 @@ Hr/+CxFvaJWmpYqNkLDGRU+9orzh5hI2RrcuaQ==
 		if (err == nil) != tc.expectCanonicalizeSuccess {
 			t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
 		} else if err != nil {
-			if _, ok := err.(types.ValidationError); !ok {
+			var validationErr *types.InputValidationError
+			if !errors.As(err, &validationErr) {
 				t.Errorf("canonicalize returned an unexpected error that isn't of type types.ValidationError: %v", err)
 			}
 		}

diff --git a/pkg/types/rekord/v0.0.1/entry.go b/pkg/types/rekord/v0.0.1/entry.go
@@ -162,7 +162,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (pki.PublicKey, p
 
 		computedSHA := hex.EncodeToString(hasher.Sum(nil))
 		if oldSHA != "" && computedSHA != oldSHA {
-			return closePipesOnError(types.ValidationError(fmt.Errorf("SHA mismatch: %s != %s", computedSHA, oldSHA)))
+			return closePipesOnError(&types.InputValidationError{Err: fmt.Errorf("SHA mismatch: %s != %s", computedSHA, oldSHA)})
 		}
 
 		select {
@@ -182,7 +182,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (pki.PublicKey, p
 
 		signature, err := af.NewSignature(sigReadCloser)
 		if err != nil {
-			return closePipesOnError(types.ValidationError(err))
+			return closePipesOnError(&types.InputValidationError{Err: err})
 		}
 
 		select {
@@ -202,7 +202,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (pki.PublicKey, p
 
 		key, err := af.NewPublicKey(keyReadCloser)
 		if err != nil {
-			return closePipesOnError(types.ValidationError(err))
+			return closePipesOnError(&types.InputValidationError{Err: err})
 		}
 
 		select {
@@ -226,7 +226,7 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (pki.PublicKey, p
 
 		var err error
 		if err = sigObj.Verify(sigR, keyObj); err != nil {
-			return closePipesOnError(types.ValidationError(err))
+			return closePipesOnError(&types.InputValidationError{Err: err})
 		}
 
 		select {

diff --git a/pkg/types/rekord/v0.0.1/entry_test.go b/pkg/types/rekord/v0.0.1/entry_test.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
+	"errors"
 	"os"
 	"reflect"
 	"testing"
@@ -242,7 +243,8 @@ func TestCrossFieldValidation(t *testing.T) {
 		if (err == nil) != tc.expectCanonicalizeSuccess {
 			t.Errorf("unexpected result from Canonicalize for '%v': %v", tc.caseDesc, err)
 		} else if err != nil {
-			if _, ok := err.(types.ValidationError); !ok {
+			var validationErr *types.InputValidationError
+			if !errors.As(err, &validationErr) {
 				t.Errorf("canonicalize returned an unexpected error that isn't of type types.ValidationError: %v", err)
 			}
 		}

diff --git a/pkg/types/rfc3161/v0.0.1/entry.go b/pkg/types/rfc3161/v0.0.1/entry.go
@@ -121,7 +121,7 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {
 
 func (v *V001Entry) Canonicalize(_ context.Context) ([]byte, error) {
 	if v.tsrContent == nil {
-		return nil, errors.New("tsr content must be set before canonicalizing")
+		return nil, &types.InputValidationError{Err: errors.New("tsr content must be set before canonicalizing")}
 	}
 	canonicalEntry := models.Rfc3161V001Schema{
 		Tsr: &models.Rfc3161V001SchemaTsr{