✨ Add custom remediation for workflow permissions/pinned dependencies

checker/check_result.go

@@ -93,6 +93,18 @@ type CheckResult struct {
 	Reason   string        `json:"-"` // A sentence describing the check result (score, etc)
 }
 
+// Remediation represents a remediation.
+type Remediation struct {
+	// Code snippet for humans.
+	Snippet string
+	// Diff for machines.
+	Diff string
+	// Help text for humans.
+	HelpText string
+	// Help text in markdown format for humans.
+	HelpMarkdown string
+}
+
 // CheckDetail contains information for each detail.
 type CheckDetail struct {
 	Msg  LogMessage
@@ -103,12 +115,13 @@ type CheckDetail struct {
 // This allows updating the definition easily.
 // nolint:govet
 type LogMessage struct {
-	Text      string   // A short string explaining why the detail was recorded/logged.
-	Path      string   // Fullpath to the file.
-	Type      FileType // Type of file.
-	Offset    uint     // Offset in the file of Path (line for source/text files).
-	EndOffset uint     // End of offset in the file, e.g. if the command spans multiple lines.
-	Snippet   string   // Snippet of code
+	Text        string       // A short string explaining why the detail was recorded/logged.
+	Path        string       // Fullpath to the file.
+	Type        FileType     // Type of file.
+	Offset      uint         // Offset in the file of Path (line for source/text files).
+	EndOffset   uint         // End of offset in the file, e.g. if the command spans multiple lines.
+	Snippet     string       // Snippet of code
+	Remediation *Remediation // Remediation information, if any.
 }
 
 // CreateProportionalScore creates a proportional score.

checks/permissions.go

@@ -82,6 +82,11 @@ func TokenPermissions(c *checker.CheckRequest) checker.CheckResult {
 	data := permissionCbData{
 		workflows: make(map[string]permissions),
 	}
+
+	if err := remdiationSetup(c); err != nil {
+		createResultForLeastPrivilegeTokens(data, err)
+	}
+
 	err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{
 		Pattern:       ".github/workflows/*",
 		CaseSensitive: false,
@@ -157,22 +162,24 @@ func validatePermission(permissionKey permission, permissionValue *actionlint.Pe
 	if strings.EqualFold(val, "write") {
 		if isPermissionOfInterest(permissionKey, ignoredPermissions) {
 			dl.Warn(&checker.LogMessage{
-				Path:   path,
-				Type:   checker.FileTypeSource,
-				Offset: lineNumber,
-				Text:   fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val),
-				// TODO: set Snippet.
+				Path:        path,
+				Type:        checker.FileTypeSource,
+				Offset:      lineNumber,
+				Text:        fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val),
+				Snippet:     val,
+				Remediation: createWorkflowPermissionRemediation(path),
 			})
 			recordPermissionWrite(pPermissions, permissionKey)
 		} else {
 			// Only log for debugging, otherwise
 			// it may confuse users.
 			dl.Debug(&checker.LogMessage{
-				Path:   path,
-				Type:   checker.FileTypeSource,
-				Offset: lineNumber,
-				Text:   fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val),
-				// TODO: set Snippet.
+				Path:        path,
+				Type:        checker.FileTypeSource,
+				Offset:      lineNumber,
+				Text:        fmt.Sprintf("%s '%v' permission set to '%v'", permLevel, permissionKey, val),
+				Snippet:     val,
+				Remediation: createWorkflowPermissionRemediation(path),
 			})
 		}
 		return nil
@@ -243,22 +250,24 @@ func validatePermissions(permissions *actionlint.Permissions, permLevel, path st
 		lineNumber := fileparser.GetLineNumber(permissions.All.Pos)
 		if !strings.EqualFold(val, "read-all") && val != "" {
 			dl.Warn(&checker.LogMessage{
-				Path:   path,
-				Type:   checker.FileTypeSource,
-				Offset: lineNumber,
-				Text:   fmt.Sprintf("%s permissions set to '%v'", permLevel, val),
-				// TODO: set Snippet.
+				Path:        path,
+				Type:        checker.FileTypeSource,
+				Offset:      lineNumber,
+				Text:        fmt.Sprintf("%s permissions set to '%v'", permLevel, val),
+				Snippet:     val,
+				Remediation: createWorkflowPermissionRemediation(path),
 			})
 			recordAllPermissionsWrite(pdata, permLevel, path)
 			return nil
 		}
 
 		dl.Info(&checker.LogMessage{
-			Path:   path,
-			Type:   checker.FileTypeSource,
-			Offset: lineNumber,
-			Text:   fmt.Sprintf("%s permissions set to '%v'", permLevel, val),
-			// TODO: set Snippet.
+			Path:        path,
+			Type:        checker.FileTypeSource,
+			Offset:      lineNumber,
+			Text:        fmt.Sprintf("%s permissions set to '%v'", permLevel, val),
+			Snippet:     val,
+			Remediation: createWorkflowPermissionRemediation(path),
 		})
 	} else /* scopeIsSet == true */ if err := validateMapPermissions(permissions.Scopes,
 		permLevel, path, dl, getWritePermissionsMap(pdata, path, permLevel), ignoredPermissions); err != nil {
@@ -273,10 +282,11 @@ func validateTopLevelPermissions(workflow *actionlint.Workflow, path string,
 	// Check if permissions are set explicitly.
 	if workflow.Permissions == nil {
 		dl.Warn(&checker.LogMessage{
-			Path:   path,
-			Type:   checker.FileTypeSource,
-			Offset: checker.OffsetDefault,
-			Text:   fmt.Sprintf("no %s permission defined", topLevelPermission),
+			Path:        path,
+			Type:        checker.FileTypeSource,
+			Offset:      checker.OffsetDefault,
+			Text:        fmt.Sprintf("no %s permission defined", topLevelPermission),
+			Remediation: createWorkflowPermissionRemediation(path),
 		})
 		recordAllPermissionsWrite(pdata, topLevelPermission, path)
 		return nil
@@ -296,10 +306,11 @@ func validatejobLevelPermissions(workflow *actionlint.Workflow, path string,
 		// so only top-level read-only permissions need to be declared.
 		if job.Permissions == nil {
 			dl.Debug(&checker.LogMessage{
-				Path:   path,
-				Type:   checker.FileTypeSource,
-				Offset: fileparser.GetLineNumber(job.Pos),
-				Text:   fmt.Sprintf("no %s permission defined", jobLevelPermission),
+				Path:        path,
+				Type:        checker.FileTypeSource,
+				Offset:      fileparser.GetLineNumber(job.Pos),
+				Text:        fmt.Sprintf("no %s permission defined", jobLevelPermission),
+				Remediation: createWorkflowPermissionRemediation(path),
 			})
 			recordAllPermissionsWrite(pdata, jobLevelPermission, path)
 			continue

checks/pinned_dependencies.go

@@ -63,6 +63,10 @@ func PinnedDependencies(c *checker.CheckRequest) checker.CheckResult {
 	}
 	*/
 
+	if remErr := remdiationSetup(c); remErr != nil {
+		return checker.CreateRuntimeErrorResult(CheckPinnedDependencies, remErr)
+	}
+
 	// GitHub actions.
 	actionScore, actionErr := isGitHubActionsWorkflowPinned(c)
 	if actionErr != nil {
@@ -702,10 +706,11 @@ var validateGitHubActionWorkflow fileparser.DoWhileTrueOnFileContent = func(
 			if !match {
 				dl.Warn(&checker.LogMessage{
 					Path: pathfn, Type: checker.FileTypeSource,
-					Offset:    uint(execAction.Uses.Pos.Line),
-					EndOffset: uint(execAction.Uses.Pos.Line), // `Uses` always span a single line.
-					Snippet:   execAction.Uses.Value,
-					Text:      fmt.Sprintf("%s action not pinned by hash", owner),
+					Offset:      uint(execAction.Uses.Pos.Line),
+					EndOffset:   uint(execAction.Uses.Pos.Line), // `Uses` always span a single line.
+					Snippet:     execAction.Uses.Value,
+					Text:        fmt.Sprintf("%s action not pinned by hash", owner),
+					Remediation: createWorkflowPinningRemediation(pathfn),
 				})
 			}
 

checks/raw/binary_artifact.go

@@ -95,10 +95,10 @@ var checkBinaryFileContent fileparser.DoWhileTrueOnFileContent = func(path strin
 	exists1 := binaryFileTypes[t.Extension]
 	if exists1 {
 		*pfiles = append(*pfiles, checker.File{
-                        Path:   path,
-                        Type:   checker.FileTypeBinary,
-                        Offset: checker.OffsetDefault,
-                })
+			Path:   path,
+			Type:   checker.FileTypeBinary,
+			Offset: checker.OffsetDefault,
+		})
 		return true, nil
 	}
 

checks/remediations.go

@@ -0,0 +1,89 @@
+// Copyright 2022 Security Scorecard Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package checks
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+	"sync"
+
+	"github.com/ossf/scorecard/v4/checker"
+	"github.com/ossf/scorecard/v4/clients"
+)
+
+var (
+	remediationBranch   string
+	remediationRepo     string
+	remediationOnce     *sync.Once
+	remediationSetupErr error
+)
+
+var (
+	workflowText = "update your workflow using https://app.stepsecurity.io/secureworkflow/%s/%s/%s?enable=%s"
+	//nolint
+	workflowMarkdown = "update your workflow using [https://app.stepsecurity.io](https://app.stepsecurity.io/secureworkflow/%s/%s/%s?enable=%s)"
+)
+
+//nolint:gochecknoinits
+func init() {
+	remediationOnce = new(sync.Once)
+}
+
+func remdiationSetup(c *checker.CheckRequest) error {
+	remediationOnce.Do(func() {
+		// Get the branch for remediation.
+		b, err := c.RepoClient.GetDefaultBranch()
+		if err != nil && !errors.Is(err, clients.ErrUnsupportedFeature) {
+			remediationSetupErr = err
+			return
+		}
+		if b.Name != nil {
+			remediationBranch = *b.Name
+			uri := c.Repo.URI()
+			parts := strings.Split(uri, "/")
+			if len(parts) != 3 {
+				remediationSetupErr = fmt.Errorf("%w: %s", errInvalidArgLength, uri)
+				return
+			}
+			remediationRepo = fmt.Sprintf("%s/%s", parts[1], parts[2])
+		}
+	})
+
+	return remediationSetupErr
+}
+
+func createWorkflowPermissionRemediation(filepath string) *checker.Remediation {
+	return createWorkflowRemediation(filepath, "permissions")
+}
+
+func createWorkflowPinningRemediation(filepath string) *checker.Remediation {
+	return createWorkflowRemediation(filepath, "pin")
+}
+
+func createWorkflowRemediation(path, t string) *checker.Remediation {
+	p := strings.TrimPrefix(path, ".github/workflows/")
+	if remediationBranch == "" || remediationRepo == "" {
+		return nil
+	}
+
+	text := fmt.Sprintf(workflowText, remediationRepo, p, remediationBranch, t)
+	markdown := fmt.Sprintf(workflowMarkdown, remediationRepo, p, remediationBranch, t)
+
+	return &checker.Remediation{
+		HelpText:     text,
+		HelpMarkdown: markdown,
+	}
+}

pkg/sarif.go

@@ -64,7 +64,8 @@ type location struct {
 	PhysicalLocation physicalLocation `json:"physicalLocation"`
 	//nolint
 	// This is optional https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#location-object.
-	Message *text `json:"message,omitempty"`
+	Message        *text `json:"message,omitempty"`
+	HasRemediation bool  `json:"-"`
 }
 
 //nolint
@@ -300,6 +301,12 @@ func detailsToLocations(details []checker.CheckDetail,
 			Message: &text{Text: d.Msg.Text},
 		}
 
+		// Add remediaiton information
+		if d.Msg.Remediation != nil {
+			loc.Message.Text = fmt.Sprintf("%s\nRemediation tip: %s", loc.Message.Text, d.Msg.Remediation.HelpMarkdown)
+			loc.HasRemediation = true
+		}
+
 		// Set the region depending on the file type.
 		loc.PhysicalLocation.Region = detailToRegion(&d)
 		locs = append(locs, loc)
@@ -428,12 +435,17 @@ func createSARIFRule(checkName, checkID, descURL, longDesc, shortDesc, risk stri
 }
 
 func createSARIFCheckResult(pos int, checkID, message string, loc *location) result {
+	t := fmt.Sprintf("%s\nClick Remediation section below to solve this issue", message)
+	if loc.HasRemediation {
+		t = fmt.Sprintf("%s\nClick Remediation section below for further remediation help", message)
+	}
+
 	return result{
 		RuleID: checkID,
 		// https://github.com/microsoft/sarif-tutorials/blob/main/docs/2-Basics.md#level
 		// Level:     scoreToLevel(minScore, score),
 		RuleIndex: pos,
-		Message:   text{Text: fmt.Sprintf("%s\nClick Remediation section below to solve this issue", message)},
+		Message:   text{Text: t},
 		Locations: []location{*loc},
 	}
 }