ossf/scorecard
1
1
Fork 0
mirror of https://github.com/ossf/scorecard.git synced 2024-09-11 00:45:36 +03:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity

✨ Add custom remediation for workflow permissions/pinned dependencies (#1885)

Browse Source 
* draft

* update

* updates

* updates

* updates

* updates

* updates

* updates
This commit is contained in:
laurentsimon 2022-05-06 12:52:30 -07:00 committed by GitHub
parent 22694dcd41
commit 8c97d46a36
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 323 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
checker/check_result.go
View File

@ -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.

67
checks/permissions.go
View File

@ -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

13
checks/pinned_dependencies.go
View File

@ -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),
})
}

8
checks/raw/binary_artifact.go
View File

@ -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
}

89
checks/remediations.go Normal file
View File

@ -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,
}
}

16
pkg/sarif.go
View File

@ -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},
}
}

92
pkg/sarif_test.go
View File

@ -127,6 +127,60 @@ func TestSARIFOutput(t *testing.T) {
result ScorecardResult
policy spol.ScorecardPolicy
}{
{
name: "check with detail remediation",
showDetails: true,
expected: "./testdata/check-remediation.sarif",
logLevel: log.DebugLevel,
policy: spol.ScorecardPolicy{
Version: 1,
Policies: map[string]*spol.CheckPolicy{
"Check-Name": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
"Check-Name2": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_DISABLED,
},
},
},
result: ScorecardResult{
Repo: RepoInfo{
Name: repoName,
CommitSHA: repoCommit,
},
Scorecard: ScorecardInfo{
Version: scorecardVersion,
CommitSHA: scorecardCommit,
},
Date: date,
Checks: []checker.CheckResult{
{
Details2: []checker.CheckDetail{
{
Type: checker.DetailWarn,
Msg: checker.LogMessage{
Text: "warn message",
Path: "src/file1.cpp",
Type: checker.FileTypeSource,
Offset: 5,
Snippet: "if (bad) {BUG();}",
Remediation: &checker.Remediation{
HelpMarkdown: "this is the custom markdown help",
HelpText: "this is the custom text help",
},
},
},
},
Score: 5,
Reason: "half score reason",
Name: "Check-Name",
},
},
Metadata: []string{},
},
},
{
name: "check-1",
showDetails: true,
@ -135,11 +189,11 @@ func TestSARIFOutput(t *testing.T) {
policy: spol.ScorecardPolicy{
Version: 1,
Policies: map[string]*spol.CheckPolicy{
"Check-Name": &spol.CheckPolicy{
"Check-Name": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
"Check-Name2": &spol.CheckPolicy{
"Check-Name2": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_DISABLED,
},
@ -185,11 +239,11 @@ func TestSARIFOutput(t *testing.T) {
policy: spol.ScorecardPolicy{
Version: 1,
Policies: map[string]*spol.CheckPolicy{
"Check-Name": &spol.CheckPolicy{
"Check-Name": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
"Check-Name2": &spol.CheckPolicy{
"Check-Name2": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_DISABLED,
},
@ -234,15 +288,15 @@ func TestSARIFOutput(t *testing.T) {
policy: spol.ScorecardPolicy{
Version: 1,
Policies: map[string]*spol.CheckPolicy{
"Check-Name": &spol.CheckPolicy{
"Check-Name": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
"Check-Name2": &spol.CheckPolicy{
"Check-Name2": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
"Check-Name3": &spol.CheckPolicy{
"Check-Name3": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
@ -341,15 +395,15 @@ func TestSARIFOutput(t *testing.T) {
policy: spol.ScorecardPolicy{
Version: 1,
Policies: map[string]*spol.CheckPolicy{
"Check-Name": &spol.CheckPolicy{
"Check-Name": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
"Check-Name2": &spol.CheckPolicy{
"Check-Name2": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
"Check-Name3": &spol.CheckPolicy{
"Check-Name3": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
@ -448,7 +502,7 @@ func TestSARIFOutput(t *testing.T) {
policy: spol.ScorecardPolicy{
Version: 1,
Policies: map[string]*spol.CheckPolicy{
"Check-Name": &spol.CheckPolicy{
"Check-Name": {
Score: 5,
Mode: spol.CheckPolicy_ENFORCED,
},
@ -496,7 +550,7 @@ func TestSARIFOutput(t *testing.T) {
policy: spol.ScorecardPolicy{
Version: 1,
Policies: map[string]*spol.CheckPolicy{
"Check-Name": &spol.CheckPolicy{
"Check-Name": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
@ -540,15 +594,15 @@ func TestSARIFOutput(t *testing.T) {
policy: spol.ScorecardPolicy{
Version: 1,
Policies: map[string]*spol.CheckPolicy{
"Check-Name": &spol.CheckPolicy{
"Check-Name": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
"Check-Name2": &spol.CheckPolicy{
"Check-Name2": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_DISABLED,
},
"Check-Name3": &spol.CheckPolicy{
"Check-Name3": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_DISABLED,
},
@ -647,19 +701,19 @@ func TestSARIFOutput(t *testing.T) {
policy: spol.ScorecardPolicy{
Version: 1,
Policies: map[string]*spol.CheckPolicy{
"Check-Name4": &spol.CheckPolicy{
"Check-Name4": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
"Check-Name": &spol.CheckPolicy{
"Check-Name": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
"Check-Name5": &spol.CheckPolicy{
"Check-Name5": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},
"Check-Name6": &spol.CheckPolicy{
"Check-Name6": {
Score: checker.MaxResultScore,
Mode: spol.CheckPolicy_ENFORCED,
},

76
pkg/testdata/check-remediation.sarif vendored Normal file
View File

@ -0,0 +1,76 @@
{
"$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
"version": "2.1.0",
"runs": [
{
"automationDetails": {
"id": "supply-chain/local/ccbc59901773ab4c051dfcea0cc4201a1567abdd-17 Aug 21 18:57 +0000"
},
"tool": {
"driver": {
"name": "Scorecard",
"informationUri": "https://github.com/ossf/scorecard",
"semanticVersion": "1.2.3",
"rules": [
{
"id": "CheckNameID",
"name": "Check-Name",
"helpUri": "https://github.com/ossf/scorecard/blob/main/docs/checks.md#check-name",
"shortDescription": {
"text": "Check-Name"
},
"fullDescription": {
"text": "short description"
},
"help": {
"text": "short description",
"markdown": "**Remediation (click \"Show more\" below)**:\n\n- not-used1\n\n- not-used2\n\n\n\n**Severity**: High\n\n\n\n**Details**:\n\nlong description\n\n other line"
},
"defaultConfiguration": {
"level": "error"
},
"properties": {
"precision": "high",
"problem.severity": "error",
"security-severity": "7.0",
"tags": [
"tag1",
"tag2"
]
}
}
]
}
},
"results": [
{
"ruleId": "CheckNameID",
"ruleIndex": 0,
"message": {
"text": "score is 5: warn message\nRemediation tip: this is the custom markdown help\nClick Remediation section below for further remediation help"
},
"locations": [
{
"physicalLocation": {
"region": {
"startLine": 5,
"endLine": 5,
"snippet": {
"text": "if (bad) {BUG();}"
}
},
"artifactLocation": {
"uri": "src/file1.cpp",
"uriBaseId": "%SRCROOT%"
}
},
"message": {
"text": "warn message\nRemediation tip: this is the custom markdown help"
}
}
]
}
]
}
]
}