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

🐛 Adding line numbers to token-permissions and a couple other places (#1363)

Browse Source 
* Adding line numbers to token-permissions and a couple other places

* Fix deadlink for security policy

Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com>
Co-authored-by: Furkan Türkal <furkan.turkal@trendyol.com>

* Updating formatting

Co-authored-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com>
Co-authored-by: Furkan Türkal <furkan.turkal@trendyol.com>
This commit is contained in:
Chris McGehee 2021-12-06 08:05:52 -08:00 committed by GitHub
parent 1eb4d0e73e
commit 38b5199e9e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 119 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
checker/check_result.go
View File

@ -58,6 +58,10 @@ const (
FileTypeURL
)
// OffsetDefault is used if we can't determine the offset, for example when referencing a file but not a
// specific location in the file.
const OffsetDefault = 1
// LogMessage is a structure that encapsulates detail's information.
// This allows updating the definition easily.
//nolint

14
checks/dependency_update_tool.go
View File

@ -61,20 +61,18 @@ func fileExists(name string, dl checker.DetailLogger, data fileparser.FileCbData
switch strings.ToLower(name) {
case ".github/dependabot.yml":
dl.Info3(&checker.LogMessage{
Path: name,
Type: checker.FileTypeSource,
// Source file must have line number > 0.
Offset: 1,
Path: name,
Type: checker.FileTypeSource,
Offset: checker.OffsetDefault,
Text: "dependabot detected",
})
// https://docs.renovatebot.com/configuration-options/
case ".github/renovate.json", ".github/renovate.json5", ".renovaterc.json", "renovate.json",
"renovate.json5", ".renovaterc":
dl.Info3(&checker.LogMessage{
Path: name,
Type: checker.FileTypeSource,
// Source file must have line number > 0.
Offset: 1,
Path: name,
Type: checker.FileTypeSource,
Offset: checker.OffsetDefault,
Text: "renovate detected",
})
default:

14
checks/packaging.go
View File

@ -62,20 +62,18 @@ func Packaging(c *checker.CheckRequest) checker.CheckResult {
}
if len(runs) > 0 {
c.Dlogger.Info3(&checker.LogMessage{
Path: fp,
Type: checker.FileTypeSource,
// Source file must have line number > 0.
Offset: 1,
Path: fp,
Type: checker.FileTypeSource,
Offset: checker.OffsetDefault,
Text: fmt.Sprintf("GitHub publishing workflow used in run %s", runs[0].URL),
})
return checker.CreateMaxScoreResult(CheckPackaging,
"publishing workflow detected")
}
c.Dlogger.Info3(&checker.LogMessage{
Path: fp,
Type: checker.FileTypeSource,
// Source file must have line number > 0.
Offset: 1,
Path: fp,
Type: checker.FileTypeSource,
Offset: checker.OffsetDefault,
Text: "GitHub publishing workflow not used in runs",
})
}

59
checks/permissions.go
View File

@ -60,13 +60,16 @@ func validatePermission(permissionKey string, permissionValue *actionlint.Permis
return sce.WithMessage(sce.ErrScorecardInternal, errInvalidGitHubWorkflow.Error())
}
val := permissionValue.Value.Value
lineNumber := checker.OffsetDefault
if permissionValue.Value.Pos != nil {
lineNumber = permissionValue.Value.Pos.Line
}
if strings.EqualFold(val, "write") {
if isPermissionOfInterest(permissionKey, ignoredPermissions) {
dl.Warn3(&checker.LogMessage{
Path: path,
Type: checker.FileTypeSource,
// TODO: set line.
Offset: 1,
Path: path,
Type: checker.FileTypeSource,
Offset: lineNumber,
Text: fmt.Sprintf("'%v' permission set to '%v'", permissionKey, val),
// TODO: set Snippet.
})
@ -75,10 +78,9 @@ func validatePermission(permissionKey string, permissionValue *actionlint.Permis
// Only log for debugging, otherwise
// it may confuse users.
dl.Debug3(&checker.LogMessage{
Path: path,
Type: checker.FileTypeSource,
// TODO: set line.
Offset: 1,
Path: path,
Type: checker.FileTypeSource,
Offset: lineNumber,
Text: fmt.Sprintf("'%v' permission set to '%v'", permissionKey, val),
// TODO: set Snippet.
})
@ -87,10 +89,9 @@ func validatePermission(permissionKey string, permissionValue *actionlint.Permis
}
dl.Info3(&checker.LogMessage{
Path: path,
Type: checker.FileTypeSource,
// TODO: set line correctly.
Offset: 1,
Path: path,
Type: checker.FileTypeSource,
Offset: lineNumber,
Text: fmt.Sprintf("'%v' permission set to '%v'", permissionKey, val),
// TODO: set Snippet.
})
@ -125,22 +126,23 @@ func validatePermissions(permissions *actionlint.Permissions, path string,
scopeIsSet := permissions != nil && len(permissions.Scopes) > 0
if permissions == nil || (!allIsSet && !scopeIsSet) {
dl.Info3(&checker.LogMessage{
Path: path,
Type: checker.FileTypeSource,
// TODO: set line correctly.
Offset: 1,
Path: path,
Type: checker.FileTypeSource,
Offset: checker.OffsetDefault,
Text: "permissions set to 'none'",
// TODO: set Snippet.
})
}
if allIsSet {
val := permissions.All.Value
lineNumber := checker.OffsetDefault
if permissions.All.Pos != nil {
lineNumber = permissions.All.Pos.Line
}
if !strings.EqualFold(val, "read-all") && val != "" {
dl.Warn3(&checker.LogMessage{
Path: path,
Type: checker.FileTypeSource,
// TODO: set line correctly.
Offset: 1,
Path: path,
Type: checker.FileTypeSource,
Offset: lineNumber,
Text: fmt.Sprintf("permissions set to '%v'", val),
// TODO: set Snippet.
})
@ -149,10 +151,9 @@ func validatePermissions(permissions *actionlint.Permissions, path string,
}
dl.Info3(&checker.LogMessage{
Path: path,
Type: checker.FileTypeSource,
// TODO: set line correctly.
Offset: 1,
Path: path,
Type: checker.FileTypeSource,
Offset: lineNumber,
Text: fmt.Sprintf("permissions set to '%v'", val),
// TODO: set Snippet.
})
@ -170,7 +171,7 @@ func validateTopLevelPermissions(workflow *actionlint.Workflow, path string,
dl.Warn3(&checker.LogMessage{
Path: path,
Type: checker.FileTypeSource,
Offset: 1,
Offset: checker.OffsetDefault,
Text: "no permission defined",
})
recordAllPermissionsWrite(pdata.topLevelWritePermissions)
@ -189,10 +190,14 @@ func validateRunLevelPermissions(workflow *actionlint.Workflow, path string,
// For most workflows, no write permissions are needed,
// so only top-level read-only permissions need to be declared.
if job.Permissions == nil {
lineNumber := checker.OffsetDefault
if job.Pos != nil {
lineNumber = job.Pos.Line
}
dl.Debug3(&checker.LogMessage{
Path: path,
Type: checker.FileTypeSource,
Offset: 1,
Offset: lineNumber,
Text: "no permission defined",
})
continue

55
checks/permissions_test.go
View File

@ -286,3 +286,58 @@ func TestGithubTokenPermissions(t *testing.T) {
})
}
}
func TestGithubTokenPermissionsLineNumber(t *testing.T) {
t.Parallel()
tests := []struct {
name string
filename string
expected []struct {
lineNumber int
}
}{
{
name: "Job level write permission",
filename: "./testdata/github-workflow-permissions-run-no-codeql-write.yaml",
expected: []struct {
lineNumber int
}{
{
lineNumber: 22,
},
},
},
{
name: "Workflow level write permission",
filename: "./testdata/github-workflow-permissions-writeall.yaml",
expected: []struct {
lineNumber int
}{
{
lineNumber: 16,
},
},
},
}
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
content, err := os.ReadFile(tt.filename)
if err != nil {
t.Errorf("cannot read file: %v", err)
}
dl := scut.TestDetailLogger{}
testValidateGitHubActionTokenPermissions(tt.filename, content, &dl)
for _, expectedLog := range tt.expected {
isExpectedLog := func(logMessage checker.LogMessage, logType checker.DetailType) bool {
return logMessage.Offset == expectedLog.lineNumber && logMessage.Path == tt.filename &&
logType == checker.DetailWarn
}
if !scut.ValidateLogMessage(isExpectedLog, &dl) {
t.Errorf("test failed: log message not present: %+v", tt.expected)
}
}
})
}
}

12
checks/pinned_dependencies.go
View File

@ -154,19 +154,19 @@ func createReturnValuesForGitHubActionsWorkflowPinned(r worklowPinningResult, in
if r.gitHubOwned != notPinned {
score += 2
// TODO: set Snippet and line numbers.
dl.Info3(&checker.LogMessage{
Type: checker.FileTypeSource,
Text: fmt.Sprintf("%s %s", "GitHub-owned", infoMsg),
Type: checker.FileTypeSource,
Offset: checker.OffsetDefault,
Text: fmt.Sprintf("%s %s", "GitHub-owned", infoMsg),
})
}
if r.thirdParties != notPinned {
score += 8
// TODO: set Snippet and line numbers.
dl.Info3(&checker.LogMessage{
Type: checker.FileTypeSource,
Text: fmt.Sprintf("%s %s", "Third-party", infoMsg),
Type: checker.FileTypeSource,
Offset: checker.OffsetDefault,
Text: fmt.Sprintf("%s %s", "Third-party", infoMsg),
})
}

3
checks/sast.go
View File

@ -184,8 +184,7 @@ func codeQLInCheckDefinitions(c *checker.CheckRequest) (int, error) {
c.Dlogger.Debug3(&checker.LogMessage{
Path: result.Path,
Type: checker.FileTypeSource,
// Source file must have line number > 0.
Offset: 1,
Offset: checker.OffsetDefault,
Text: "CodeQL detected",
})
}

21
checks/security_policy.go
View File

@ -46,20 +46,18 @@ func SecurityPolicy(c *checker.CheckRequest) checker.CheckResult {
strings.EqualFold(name, ".github/security.md") ||
strings.EqualFold(name, "docs/security.md") {
c.Dlogger.Info3(&checker.LogMessage{
Path: name,
Type: checker.FileTypeSource,
// Source file must have line number > 0.
Offset: 1,
Path: name,
Type: checker.FileTypeSource,
Offset: checker.OffsetDefault,
Text: "security policy detected",
})
*pdata = true
return false, nil
} else if isSecurityRstFound(name) {
c.Dlogger.Info3(&checker.LogMessage{
Path: name,
Type: checker.FileTypeSource,
// Source file must have line number > 0.
Offset: 1,
Path: name,
Type: checker.FileTypeSource,
Offset: checker.OffsetDefault,
Text: "security policy detected",
})
*pdata = true
@ -97,10 +95,9 @@ func SecurityPolicy(c *checker.CheckRequest) checker.CheckResult {
strings.EqualFold(name, ".github/security.md") ||
strings.EqualFold(name, "docs/security.md") {
dl.Info3(&checker.LogMessage{
Path: name,
Type: checker.FileTypeSource,
// Source file must have line number > 0.
Offset: 1,
Path: name,
Type: checker.FileTypeSource,
Offset: checker.OffsetDefault,
Text: "security policy detected in .github folder",
})
*pdata = true