Refactoring error messages to use %w formatting directive and fix log…

…ging issue (#85) * refactored to handle error wrapping inside the log package Signed-off-by: chaosinthecrd <[email protected]> * using %w directive wherever possible Signed-off-by: chaosinthecrd <[email protected]> --------- Signed-off-by: chaosinthecrd <[email protected]>
in-toto · Dec 4, 2023 · 1d30fe2 · 1d30fe2
1 parent 7ec4004
commit 1d30fe2
Show file tree

Hide file tree

Showing 15 changed files with 66 additions and 49 deletions.
diff --git a/attestation/aws-iid/aws-iid.go b/attestation/aws-iid/aws-iid.go
@@ -134,20 +134,20 @@ func (a *Attestor) getIID() error {
 	svc := ec2metadata.New(&a.session, a.conf)
 	iid, err := svc.GetDynamicData(docPath)
 	if err != nil {
-		return fmt.Errorf("failed to get instance identity document: %v", err)
+		return fmt.Errorf("failed to get instance identity document: %w", err)
 	}
 
 	sig, err := svc.GetDynamicData(sigPath)
 	if err != nil {
-		return fmt.Errorf("failed to get signature: %v", err)
+		return fmt.Errorf("failed to get signature: %w", err)
 	}
 
 	a.RawIID = iid
 	a.RawSig = sig
 
 	err = json.Unmarshal([]byte(a.RawIID), &a.EC2InstanceIdentityDocument)
 	if err != nil {
-		return fmt.Errorf("failed to unmarshal iid: %v", err)
+		return fmt.Errorf("failed to unmarshal iid: %w", err)
 	}
 
 	return nil
@@ -161,17 +161,17 @@ func (a *Attestor) Verify() error {
 	docHash := sha256.Sum256([]byte(a.RawIID))
 	sigBytes, err := base64.StdEncoding.DecodeString(a.RawSig)
 	if err != nil {
-		return fmt.Errorf("failed to decode signature: %v", err)
+		return fmt.Errorf("failed to decode signature: %w", err)
 	}
 
 	pubKey, err := getAWSCAPublicKey()
 	if err != nil {
-		return fmt.Errorf("failed to get AWS public key: %v", err)
+		return fmt.Errorf("failed to get AWS public key: %w", err)
 	}
 
 	pubKeyBytes, err := x509.MarshalPKIXPublicKey(pubKey)
 	if err != nil {
-		return fmt.Errorf("failed to marshal public key: %v", err)
+		return fmt.Errorf("failed to marshal public key: %w", err)
 	}
 
 	pem := pem.EncodeToMemory(&pem.Block{
@@ -182,12 +182,12 @@ func (a *Attestor) Verify() error {
 	a.PublicKey = string(pem)
 
 	if err != nil {
-		return fmt.Errorf("failed to encode public key: %v", err)
+		return fmt.Errorf("failed to encode public key: %w", err)
 	}
 
 	err = rsa.VerifyPKCS1v15(pubKey, crypto.SHA256, docHash[:], sigBytes)
 	if err != nil {
-		log.Debugf("(attestation/aws-iid) failed to verify signature: %v", err)
+		log.Debugf("(attestation/aws-iid) failed to verify signature: %w", err)
 		return nil
 	}
 
@@ -200,25 +200,25 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {
 	if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.EC2InstanceIdentityDocument.InstanceID), hashes); err == nil {
 		subjects[fmt.Sprintf("instanceid:%s", a.EC2InstanceIdentityDocument.InstanceID)] = ds
 	} else {
-		log.Debugf("(attestation/aws) failed to record aws instanceid subject: %v", err)
+		log.Debugf("(attestation/aws) failed to record aws instanceid subject: %w", err)
 	}
 
 	if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.EC2InstanceIdentityDocument.AccountID), hashes); err == nil {
 		subjects[fmt.Sprintf("accountid:%s", a.EC2InstanceIdentityDocument.AccountID)] = ds
 	} else {
-		log.Debugf("(attestation/aws) failed to record aws accountid subject: %v", err)
+		log.Debugf("(attestation/aws) failed to record aws accountid subject: %w", err)
 	}
 
 	if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.EC2InstanceIdentityDocument.ImageID), hashes); err == nil {
 		subjects[fmt.Sprintf("imageid:%s", a.EC2InstanceIdentityDocument.ImageID)] = ds
 	} else {
-		log.Debugf("(attestation/aws) failed to record aws imageid subject: %v", err)
+		log.Debugf("(attestation/aws) failed to record aws imageid subject: %w", err)
 	}
 
 	if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.EC2InstanceIdentityDocument.PrivateIP), hashes); err == nil {
 		subjects[fmt.Sprintf("privateip:%s", a.EC2InstanceIdentityDocument.PrivateIP)] = ds
 	} else {
-		log.Debugf("(attestation/aws) failed to record aws privateip subject: %v", err)
+		log.Debugf("(attestation/aws) failed to record aws privateip subject: %w", err)
 	}
 
 	return subjects
@@ -232,7 +232,7 @@ func getAWSCAPublicKey() (*rsa.PublicKey, error) {
 
 	cert, err := x509.ParseCertificate(block.Bytes)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse certificate: %v", err)
+		return nil, fmt.Errorf("failed to parse certificate: %w", err)
 	}
 
 	return cert.PublicKey.(*rsa.PublicKey), nil

diff --git a/attestation/commandrun/tracing_linux.go b/attestation/commandrun/tracing_linux.go
@@ -112,12 +112,12 @@ func (p *ptraceContext) runTrace() error {
 		if status.Stopped() && isPtraceTrap {
 			injectedSig = 0
 			if err := p.nextSyscall(pid); err != nil {
-				log.Debugf("(tracing) got error while processing syscall: %v", err)
+				log.Debugf("(tracing) got error while processing syscall: %w", err)
 			}
 		}
 
 		if err := unix.PtraceSyscall(pid, injectedSig); err != nil {
-			log.Debugf("(tracing) got error from ptrace syscall: %v", err)
+			log.Debugf("(tracing) got error from ptrace syscall: %w", err)
 		}
 	}
 }

diff --git a/attestation/context.go b/attestation/context.go
@@ -185,7 +185,7 @@ func (ctx *AttestationContext) runAttestor(attestor Attestor) error {
 	log.Infof("Starting %v attestor...", attestor.Name())
 	startTime := time.Now()
 	if err := attestor.Attest(ctx); err != nil {
-		log.Errorf("Error running %v attestor: %v", attestor.Name(), err)
+		log.Errorf("Error running %v attestor: %w", attestor.Name(), err)
 		ctx.completedAttestors = append(ctx.completedAttestors, CompletedAttestor{
 			Attestor:  attestor,
 			StartTime: startTime,

diff --git a/attestation/factory.go b/attestation/factory.go
@@ -35,7 +35,7 @@ type Attestor interface {
 }
 
 // Subjecter allows attestors to expose bits of information that will be added to
-// the in-toto statement as subjects. External services such as Rekor and Archivist
+// the in-toto statement as subjects. External services such as Rekor and Archivista
 // use in-toto subjects as indexes back to attestations.
 type Subjecter interface {
 	Subjects() map[string]cryptoutil.DigestSet

diff --git a/attestation/gcp-iit/gcp-iit.go b/attestation/gcp-iit/gcp-iit.go
@@ -102,6 +102,7 @@ func (a *Attestor) Attest(ctx *attestation.AttestationContext) error {
 	tokenURL := identityTokenURL(defaultIdentityTokenHost, defaultServiceAccount)
 	identityToken, err := getMetadata(tokenURL)
 	if err != nil {
+		// status.Errorf does not support %w directive
 		return status.Errorf(codes.Internal, "unable to retrieve valid identity token: %v", err)
 	}
 
@@ -150,7 +151,7 @@ func (a *Attestor) getInstanceData() {
 	for k, v := range endpoints {
 		data, err := getMetadata(v)
 		if err != nil {
-			log.Warnf("failed to retrieve gcp metadata from %v: %v", v, err)
+			log.Warnf("failed to retrieve gcp metadata from %v: %w", v, err)
 			continue
 		}
 		metadata[k] = string(data)
@@ -165,7 +166,7 @@ func (a *Attestor) getInstanceData() {
 
 	projID, projNum, err := parseJWTProjectInfo(a.JWT)
 	if err != nil {
-		log.Warnf("unable to parse gcp project info from JWT: %v\n", err)
+		log.Warnf("unable to parse gcp project info from JWT: %w\n", err)
 	}
 
 	a.ProjectID = projID
@@ -179,31 +180,31 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {
 	if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.InstanceID), hashes); err == nil {
 		subjects[fmt.Sprintf("instanceid:%v", a.InstanceID)] = ds
 	} else {
-		log.Debugf("(attestation/gcp) failed to record gcp instanceid subject: %v", err)
+		log.Debugf("(attestation/gcp) failed to record gcp instanceid subject: %w", err)
 	}
 
 	if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.InstanceHostname), hashes); err == nil {
 		subjects[fmt.Sprintf("instancename:%v", a.InstanceHostname)] = ds
 	} else {
-		log.Debugf("(attestation/gcp) failed to record gcp instancename subject: %v", err)
+		log.Debugf("(attestation/gcp) failed to record gcp instancename subject: %w", err)
 	}
 
 	if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectID), hashes); err == nil {
 		subjects[fmt.Sprintf("projectid:%v", a.ProjectID)] = ds
 	} else {
-		log.Debugf("(attestation/gcp) failed to record gcp projectid subject: %v", err)
+		log.Debugf("(attestation/gcp) failed to record gcp projectid subject: %w", err)
 	}
 
 	if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectNumber), hashes); err == nil {
 		subjects[fmt.Sprintf("projectnumber:%v", a.ProjectNumber)] = ds
 	} else {
-		log.Debugf("(attestation/gcp) failed to record gcp projectnumber subject: %v", err)
+		log.Debugf("(attestation/gcp) failed to record gcp projectnumber subject: %w", err)
 	}
 
 	if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ClusterUID), hashes); err == nil {
 		subjects[fmt.Sprintf("clusteruid:%v", a.ClusterUID)] = ds
 	} else {
-		log.Debugf("(attestation/gcp) failed to record gcp clusteruid subject: %v", err)
+		log.Debugf("(attestation/gcp) failed to record gcp clusteruid subject: %w", err)
 	}
 
 	return subjects

diff --git a/attestation/github/github.go b/attestation/github/github.go
@@ -140,13 +140,13 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {
 	if pipelineSubj, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.PipelineUrl), hashes); err == nil {
 		subjects[fmt.Sprintf("pipelineurl:%v", a.PipelineUrl)] = pipelineSubj
 	} else {
-		log.Debugf("(attestation/github) failed to record github pipelineurl subject: %v", err)
+		log.Debugf("(attestation/github) failed to record github pipelineurl subject: %w", err)
 	}
 
 	if projectSubj, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectUrl), hashes); err == nil {
 		subjects[fmt.Sprintf("projecturl:%v", a.ProjectUrl)] = projectSubj
 	} else {
-		log.Debugf("(attestation/github) failed to record github projecturl subject: %v", err)
+		log.Debugf("(attestation/github) failed to record github projecturl subject: %w", err)
 	}
 
 	return subjects

diff --git a/attestation/gitlab/gitlab.go b/attestation/gitlab/gitlab.go
@@ -122,19 +122,19 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {
 	if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.PipelineUrl), hashes); err == nil {
 		subjects[fmt.Sprintf("pipelineurl:%v", a.PipelineUrl)] = ds
 	} else {
-		log.Debugf("(attestation/gitlab) failed to record gitlab pipelineurl subject: %v", err)
+		log.Debugf("(attestation/gitlab) failed to record gitlab pipelineurl subject: %w", err)
 	}
 
 	if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.JobUrl), hashes); err == nil {
 		subjects[fmt.Sprintf("joburl:%v", a.JobUrl)] = ds
 	} else {
-		log.Debugf("(attestation/gitlab) failed to record gitlab joburl subject: %v", err)
+		log.Debugf("(attestation/gitlab) failed to record gitlab joburl subject: %w", err)
 	}
 
 	if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectUrl), hashes); err == nil {
 		subjects[fmt.Sprintf("projecturl:%v", a.ProjectUrl)] = ds
 	} else {
-		log.Debugf("(attestation/gitlab) failed to record gitlab projecturl subject: %v", err)
+		log.Debugf("(attestation/gitlab) failed to record gitlab projecturl subject: %w", err)
 	}
 
 	return subjects

diff --git a/attestation/maven/maven.go b/attestation/maven/maven.go
@@ -121,14 +121,14 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {
 	if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(projectSubject), hashes); err == nil {
 		subjects[projectSubject] = ds
 	} else {
-		log.Debugf("(attestation/maven) failed to record %v subject: %v", projectSubject, err)
+		log.Debugf("(attestation/maven) failed to record %v subject: %w", projectSubject, err)
 	}
 
 	for _, dep := range a.Dependencies {
 		depSubject := fmt.Sprintf("dependency:%v/%v@%v", dep.GroupId, dep.ArtifactId, dep.Version)
 		depDigest, err := cryptoutil.CalculateDigestSetFromBytes([]byte(depSubject), hashes)
 		if err != nil {
-			log.Debugf("(attestation/maven) failed to record %v subject: %v", depSubject, err)
+			log.Debugf("(attestation/maven) failed to record %v subject: %w", depSubject, err)
 		}
 
 		subjects[depSubject] = depDigest

diff --git a/attestation/oci/oci.go b/attestation/oci/oci.go
@@ -99,7 +99,7 @@ func (m *Manifest) getImageID(ctx *attestation.AttestationContext, tarFilePath s
 
 			imageID, err := cryptoutil.CalculateDigestSetFromBytes(b, ctx.Hashes())
 			if err != nil {
-				log.Debugf("(attestation/oci) error calculating image id: %v", err)
+				log.Debugf("(attestation/oci) error calculating image id: %w", err)
 				return nil, err
 			}
 
@@ -127,18 +127,18 @@ func (a *Attestor) RunType() attestation.RunType {
 
 func (a *Attestor) Attest(ctx *attestation.AttestationContext) error {
 	if err := a.getCandidate(ctx); err != nil {
-		log.Debugf("(attestation/oci) error getting candidate: %v", err)
+		log.Debugf("(attestation/oci) error getting candidate: %w", err)
 		return err
 	}
 
 	if err := a.parseMaifest(ctx); err != nil {
-		log.Debugf("(attestation/oci) error parsing manifest: %v", err)
+		log.Debugf("(attestation/oci) error parsing manifest: %w", err)
 		return err
 	}
 
 	imageID, err := a.Manifest[0].getImageID(ctx, a.tarFilePath)
 	if err != nil {
-		log.Debugf("(attestation/oci) error getting image id: %v", err)
+		log.Debugf("(attestation/oci) error getting image id: %w", err)
 		return err
 	}
 
@@ -241,7 +241,7 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {
 	for _, tag := range a.ImageTags {
 		hash, err := cryptoutil.CalculateDigestSetFromBytes([]byte(tag), hashes)
 		if err != nil {
-			log.Debugf("(attestation/oci) error calculating image tag: %v", err)
+			log.Debugf("(attestation/oci) error calculating image tag: %w", err)
 			continue
 		}
 		subj[fmt.Sprintf("imagetag:%s", tag)] = hash

diff --git a/attestation/sarif/sarif.go b/attestation/sarif/sarif.go
@@ -71,7 +71,7 @@ func (a *Attestor) RunType() attestation.RunType {
 
 func (a *Attestor) Attest(ctx *attestation.AttestationContext) error {
 	if err := a.getCandidate(ctx); err != nil {
-		log.Debugf("(attestation/sarif) error getting candidate: %v", err)
+		log.Debugf("(attestation/sarif) error getting candidate: %w", err)
 		return err
 	}
 
@@ -113,7 +113,7 @@ func (a *Attestor) getCandidate(ctx *attestation.AttestationContext) error {
 
 		//check to see if we can unmarshal into sarif type
 		if err := json.Unmarshal(reportBytes, &a.Report); err != nil {
-			log.Debugf("(attestation/sarif) error unmarshaling report: %v", err)
+			log.Debugf("(attestation/sarif) error unmarshaling report: %w", err)
 			continue
 		}
 

diff --git a/log/log.go b/log/log.go
@@ -14,6 +14,10 @@
 
 package log
 
+import (
+	"fmt"
+)
+
 var log Logger = SilentLogger{}
 
 // Logger is used by witness library code to print out relevant information at runtime.
@@ -40,23 +44,35 @@ func GetLogger() Logger {
 }
 
 func Errorf(format string, args ...interface{}) {
-	log.Errorf(format, args...)
+	err := fmt.Errorf(format, args...)
+	log.Error(err)
 }
 
 func Error(args ...interface{}) {
 	log.Error(args...)
 }
 
 func Warnf(format string, args ...interface{}) {
-	log.Warnf(format, args...)
+	// We want to wrap the error if there is one.
+	for _, a := range args {
+		if _, ok := a.(error); ok {
+			err := fmt.Errorf(format, args...)
+			log.Warn(err)
+		}
+	}
 }
 
 func Warn(args ...interface{}) {
 	log.Warn(args...)
 }
 
 func Debugf(format string, args ...interface{}) {
-	log.Debugf(format, args...)
+	for _, a := range args {
+		if _, ok := a.(error); ok {
+			err := fmt.Errorf(format, args...)
+			log.Debug(err)
+		}
+	}
 }
 
 func Debug(args ...interface{}) {

diff --git a/policy/policy.go b/policy/policy.go
@@ -238,7 +238,7 @@ func (step Step) checkFunctionaries(verifiedStatements []source.VerifiedCollecti
 		for _, verifier := range verifiedStatement.Verifiers {
 			verifierID, err := verifier.KeyID()
 			if err != nil {
-				log.Debugf("(policy) skipping verifier: could not get key id: %v", err)
+				log.Debugf("(policy) skipping verifier: could not get key id: %w", err)
 				continue
 			}