🐛 Surface workflow verification errors with doc link (#408)

* Wrap verification errors with single type.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* update link.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* remove duplicate 'workflow verification failed' text.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* clean up other pointless wraps.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Spin verification errors into custom type.

This lets us better control which errors we surface as a 400 vs hide as 500.

Signed-off-by: Spencer Schrock <sschrock@google.com>

---------

Signed-off-by: Spencer Schrock <sschrock@google.com>

app/server/post_results.go

@@ -62,6 +62,7 @@ var (
 	errCertMissingURI           = errors.New("certificate has no URIs")
 	errCertWorkflowPathEmpty    = errors.New("cert workflow path is empty")
 	errMismatchedCertAndRequest = errors.New("repository and branch of cert doesn't match that of request")
+	errNotDefaultBranch         = errors.New("branch of cert isn't the repo's default branch")
 )
 
 type certInfo struct {
@@ -108,21 +109,11 @@ func PostResultsHandler(params results.PostResultParams) middleware.Responder {
 	if err == nil {
 		return results.NewPostResultCreated().WithPayload("successfully verified and published ScorecardResult")
 	}
-	if errors.Is(err, errMismatchedCertAndRequest) ||
-		errors.Is(err, errGlobalVarsOrDefaults) ||
-		errors.Is(err, errGlobalWriteAll) ||
-		errors.Is(err, errGlobalWrite) ||
-		errors.Is(err, errScorecardJobNotFound) ||
-		errors.Is(err, errNonScorecardJobHasTokenWrite) ||
-		errors.Is(err, errJobHasContainerOrServices) ||
-		errors.Is(err, errScorecardJobRunsOn) ||
-		errors.Is(err, errUnallowedStepName) ||
-		errors.Is(err, errScorecardJobEnvVars) ||
-		errors.Is(err, errScorecardJobDefaults) ||
-		errors.Is(err, errEmptyStepUses) {
+	var vErr verificationError
+	if errors.As(err, &vErr) {
 		return results.NewPostResultBadRequest().WithPayload(&models.Error{
 			Code:    http.StatusBadRequest,
-			Message: err.Error(),
+			Message: vErr.Error(),
 		})
 	}
 	log.Println(err)
@@ -146,11 +137,11 @@ func processRequest(host, org, repo string, scorecardResult *models.VerifiedScor
 	if info.repoFullName != fullName(org, repo) ||
 		(info.repoBranchRef != scorecardResult.Branch &&
 			info.repoBranchRef != fmt.Sprintf("refs/heads/%s", scorecardResult.Branch)) {
-		return fmt.Errorf("%w", errMismatchedCertAndRequest)
+		return verificationError{e: errMismatchedCertAndRequest}
 	}
 
 	if err := getAndVerifyWorkflowContent(ctx, scorecardResult, info); err != nil {
-		return fmt.Errorf("error verifying workflow: %w", err)
+		return fmt.Errorf("workflow verification failed: %w", err)
 	}
 
 	// Save scorecard results (results.json, score.txt) to GCS
@@ -210,7 +201,7 @@ func getAndVerifyWorkflowContent(ctx context.Context,
 	defaultBranch := repoClient.GetDefaultBranch()
 	if scorecardResult.Branch != defaultBranch &&
 		scorecardResult.Branch != fmt.Sprintf("refs/heads/%s", defaultBranch) {
-		return fmt.Errorf("branch of cert isn't the repo's default branch")
+		return verificationError{e: errNotDefaultBranch}
 	}
 
 	// Use the cert commit SHA if the workflow file is in the repo being analyzed.
@@ -222,19 +213,16 @@ func getAndVerifyWorkflowContent(ctx context.Context,
 
 	contents, _, _, err := client.Repositories.GetContents(ctx, org, repo, path, opts)
 	if err != nil {
-		return fmt.Errorf("error downloading workflow contents from repo: %v", err)
+		return fmt.Errorf("error downloading workflow contents from repo: %w", err)
 	}
 
 	workflowContent, err := contents.GetContent()
 	if err != nil {
-		return fmt.Errorf("error decoding workflow contents: %v", err)
+		return fmt.Errorf("error decoding workflow contents: %w", err)
 	}
 
 	// Verify scorecard workflow.
-	if err := verifyScorecardWorkflow(workflowContent); err != nil {
-		return fmt.Errorf("workflow could not be verified: %v", err)
-	}
-	return nil
+	return verifyScorecardWorkflow(workflowContent)
 }
 
 func writeToBlobStore(ctx context.Context, bucketURL, filename string, data []byte) error {
@@ -285,7 +273,7 @@ func extractAndVerifyCertForPayload(ctx context.Context, payload []byte) (*x509.
 		return nil, fmt.Errorf("error extracting certificate from entry: %w", err)
 	}
 	if len(certs) > 1 {
-		return nil, fmt.Errorf("%w", errMultipleCerts)
+		return nil, errMultipleCerts
 	}
 
 	cert := certs[0]

app/server/verify_workflow.go

@@ -22,6 +22,10 @@ import (
 	"github.com/rhysd/actionlint"
 )
 
+const (
+	workflowRestrictionLink = "https://github.com/ossf/scorecard-action#workflow-restrictions"
+)
+
 var (
 	errActionlintParse              = errors.New("errors during actionlint.Parse")
 	errGlobalVarsOrDefaults         = errors.New("workflow contains global env vars or defaults")
@@ -47,6 +51,18 @@ var ubuntuRunners = map[string]bool{
 	"ubuntu-18.04":  true,
 }
 
+type verificationError struct {
+	e error
+}
+
+func (ve verificationError) Error() string {
+	return fmt.Sprintf("workflow verification failed: %v, see %s for details.", ve.e, workflowRestrictionLink)
+}
+
+func (ve verificationError) Unwrap() error {
+	return ve.e
+}
+
 func verifyScorecardWorkflow(workflowContent string) error {
 	// Verify workflow contents using actionlint.
 	workflow, lintErrs := actionlint.Parse([]byte(workflowContent))
@@ -56,49 +72,49 @@ func verifyScorecardWorkflow(workflowContent string) error {
 
 	// Verify that there are no global env vars or defaults.
 	if workflow.Env != nil || workflow.Defaults != nil {
-		return fmt.Errorf("%w", errGlobalVarsOrDefaults)
+		return verificationError{e: errGlobalVarsOrDefaults}
 	}
 
 	if workflow.Permissions != nil {
 		globalPerms := workflow.Permissions
 		// Verify that the all scope, if set, isn't write-all.
 		if globalPerms.All != nil && globalPerms.All.Value == "write-all" {
-			return fmt.Errorf("%w", errGlobalWriteAll)
+			return verificationError{e: errGlobalWriteAll}
 		}
 
 		// Verify that there are no global permissions (including id-token) set to write.
 		for globalPerm, val := range globalPerms.Scopes {
 			if val.Value.Value == "write" {
-				return fmt.Errorf("%w: permission for %v is set to write",
-					errGlobalWrite, globalPerm)
+				return verificationError{e: fmt.Errorf("%w: permission for %v is set to write",
+					errGlobalWrite, globalPerm)}
 			}
 		}
 	}
 
 	// Find the (first) job with a step that calls scorecard-action.
 	scorecardJob := findScorecardJob(workflow.Jobs)
 	if scorecardJob == nil {
-		return fmt.Errorf("%w", errScorecardJobNotFound)
+		return verificationError{e: errScorecardJobNotFound}
 	}
 
 	// Make sure other jobs don't have id-token permissions.
 	for _, job := range workflow.Jobs {
 		if job != scorecardJob && job.Permissions != nil {
 			idToken := job.Permissions.Scopes["id-token"]
 			if idToken != nil && idToken.Value.Value == "write" {
-				return fmt.Errorf("%w", errNonScorecardJobHasTokenWrite)
+				return verificationError{e: errNonScorecardJobHasTokenWrite}
 			}
 		}
 	}
 
 	// Verify that there is no job container or services.
 	if scorecardJob.Container != nil || len(scorecardJob.Services) > 0 {
-		return fmt.Errorf("%w", errJobHasContainerOrServices)
+		return verificationError{e: errJobHasContainerOrServices}
 	}
 
 	labels := scorecardJob.RunsOn.Labels
 	if len(labels) != 1 {
-		return fmt.Errorf("%w", errScorecardJobRunsOn)
+		return verificationError{e: errScorecardJobRunsOn}
 	}
 	label := labels[0].Value
 	if _, ok := ubuntuRunners[label]; !ok {
@@ -107,12 +123,12 @@ func verifyScorecardWorkflow(workflowContent string) error {
 
 	// Verify that there are no job env vars set.
 	if scorecardJob.Env != nil {
-		return fmt.Errorf("%w", errScorecardJobEnvVars)
+		return verificationError{e: errScorecardJobEnvVars}
 	}
 
 	// Verify that there are no job defaults set.
 	if scorecardJob.Defaults != nil {
-		return fmt.Errorf("%w", errScorecardJobDefaults)
+		return verificationError{e: errScorecardJobDefaults}
 	}
 
 	// Get steps in job.
@@ -122,7 +138,7 @@ func verifyScorecardWorkflow(workflowContent string) error {
 	for _, step := range steps {
 		stepUses := getStepUses(step)
 		if stepUses == nil {
-			return fmt.Errorf("%w", errEmptyStepUses)
+			return verificationError{e: errEmptyStepUses}
 		}
 		stepName := getStepName(stepUses.Value)
 
@@ -136,7 +152,7 @@ func verifyScorecardWorkflow(workflowContent string) error {
 			// Needed for e2e tests
 			"gcr.io/openssf/scorecard-action":
 		default:
-			return fmt.Errorf("%w: %s", errUnallowedStepName, stepName)
+			return verificationError{e: fmt.Errorf("%w: %s", errUnallowedStepName, stepName)}
 		}
 	}