Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🐛 Add scorecard-action to the security-events allowlist in Token Permissions check #2153

Merged
merged 4 commits into from
Aug 16, 2022
Merged

🐛 Add scorecard-action to the security-events allowlist in Token Permissions check #2153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
82ce244
fails tests
spencerschrock Aug 16, 2022
3224bdd
update tests to reflect number of exepected debug msgs (one fewer per…
spencerschrock Aug 16, 2022
acc7f00
Replace strings.Cut usage with strings.Split since we dont use go1.18…
spencerschrock Aug 16, 2022
b02c20d
fix number of debug messages in e2e tests. also a result of deduplica…
spencerschrock Aug 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 26 additions & 26 deletions checks/permissions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore - 1,
NumberOfWarn: 1,
NumberOfInfo: 1,
NumberOfDebug: 5,
NumberOfDebug: 4,
},
},
{
Expand All @@ -67,7 +67,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MinResultScore,
NumberOfWarn: 3,
NumberOfInfo: 2,
NumberOfDebug: 5,
NumberOfDebug: 4,
},
},
{
Expand All @@ -78,7 +78,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
{
Expand All @@ -89,7 +89,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 1,
NumberOfDebug: 5,
NumberOfDebug: 4,
},
},
{
Expand All @@ -100,7 +100,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
{
Expand All @@ -111,7 +111,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
{
Expand All @@ -122,7 +122,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
{
Expand All @@ -133,7 +133,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 0,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
{
Expand All @@ -144,7 +144,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfDebug: 7,
NumberOfDebug: 6,
},
},
{
Expand All @@ -155,7 +155,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 10,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
{
Expand All @@ -166,7 +166,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 10,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
{
Expand All @@ -177,7 +177,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
{
Expand All @@ -188,7 +188,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore - 1,
NumberOfWarn: 2,
NumberOfInfo: 2,
NumberOfDebug: 7,
NumberOfDebug: 6,
},
},
{
Expand All @@ -199,7 +199,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore - 2,
NumberOfWarn: 2,
NumberOfInfo: 3,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
{
Expand All @@ -210,7 +210,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 2,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
{
Expand All @@ -221,7 +221,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 2,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
{
Expand All @@ -232,7 +232,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 1,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
{
Expand All @@ -254,7 +254,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 1,
NumberOfDebug: 5,
NumberOfDebug: 4,
},
},
{
Expand All @@ -265,7 +265,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfDebug: 5,
NumberOfDebug: 4,
},
},
{
Expand All @@ -276,7 +276,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
{
Expand All @@ -287,7 +287,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: 9,
NumberOfWarn: 1,
NumberOfInfo: 3,
NumberOfDebug: 5,
NumberOfDebug: 4,
},
},
{
Expand All @@ -298,7 +298,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore - 1,
NumberOfWarn: 1,
NumberOfInfo: 1,
NumberOfDebug: 5,
NumberOfDebug: 4,
},
},
{
Expand All @@ -312,7 +312,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore - 1,
NumberOfWarn: 1,
NumberOfInfo: 2,
NumberOfDebug: 11,
NumberOfDebug: 9,
},
},
{
Expand All @@ -326,7 +326,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MinResultScore,
NumberOfWarn: 2,
NumberOfInfo: 1,
NumberOfDebug: 11,
NumberOfDebug: 9,
},
},
{
Expand All @@ -340,7 +340,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MinResultScore,
NumberOfWarn: 1,
NumberOfInfo: 1,
NumberOfDebug: 12,
NumberOfDebug: 10,
},
},
{
Expand All @@ -353,7 +353,7 @@ func TestGithubTokenPermissions(t *testing.T) {
Score: checker.MaxResultScore,
NumberOfWarn: 0,
NumberOfInfo: 1,
NumberOfDebug: 6,
NumberOfDebug: 5,
},
},
}
Expand Down
123 changes: 36 additions & 87 deletions checks/raw/permissions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,111 +348,60 @@ func createIgnoredPermissions(workflow *actionlint.Workflow, fp string,

// Scanning tool run externally and SARIF file uploaded.
func isSARIFUploadWorkflow(workflow *actionlint.Workflow, fp string, pdata *permissionCbData) bool {
//nolint
// CodeQl analysis workflow automatically sends sarif file to GitHub.
// https://docs.github.com/en/code-security/secure-coding/integrating-with-code-scanning/uploading-a-sarif-file-to-github#about-sarif-file-uploads-for-code-scanning.
// `The CodeQL action uploads the SARIF file automatically when it completes analysis`.
if isCodeQlAnalysisWorkflow(workflow, fp, pdata) {
return true
}

//nolint
// Third-party scanning tools use the SARIF-upload action from code-ql.
// https://docs.github.com/en/code-security/secure-coding/integrating-with-code-scanning/uploading-a-sarif-file-to-github#uploading-a-code-scanning-analysis-with-github-actions
// We only support CodeQl today.
if isSARIFUploadAction(workflow, fp, pdata) {
return true
}

// TODO: some third party tools may upload directly thru their actions.
// Very unlikely.
// See https://github.com/marketplace for tools.

return false
return isAllowedWorkflow(workflow, fp, pdata)
}

// CodeQl run externally and SARIF file uploaded.
func isSARIFUploadAction(workflow *actionlint.Workflow, fp string, pdata *permissionCbData) bool {
for _, job := range workflow.Jobs {
for _, step := range job.Steps {
uses := fileparser.GetUses(step)
if uses == nil {
continue
}
if strings.HasPrefix(uses.Value, "github/codeql-action/upload-sarif@") {
pdata.results.TokenPermissions = append(pdata.results.TokenPermissions,
checker.TokenPermission{
File: &checker.File{
Path: fp,
Type: checker.FileTypeSource,
Offset: fileparser.GetLineNumber(uses.Pos),
// TODO: set Snippet.
},
Type: checker.PermissionLevelUnknown,
Msg: stringPointer("codeql SARIF upload workflow detected"),
// TODO: Job
})

return true
}
}
func isAllowedWorkflow(workflow *actionlint.Workflow, fp string, pdata *permissionCbData) bool {
allowlist := map[string]bool{
//nolint
// CodeQl analysis workflow automatically sends sarif file to GitHub.
// https://docs.github.com/en/code-security/secure-coding/integrating-with-code-scanning/uploading-a-sarif-file-to-github#about-sarif-file-uploads-for-code-scanning.
// `The CodeQL action uploads the SARIF file automatically when it completes analysis`.
"github/codeql-action/analyze": true,

//nolint
// Third-party scanning tools use the SARIF-upload action from code-ql.
// https://docs.github.com/en/code-security/secure-coding/integrating-with-code-scanning/uploading-a-sarif-file-to-github#uploading-a-code-scanning-analysis-with-github-actions
// We only support CodeQl today.
"github/codeql-action/upload-sarif": true,

// allow our own action, which writes sarif files
// https://github.com/ossf/scorecard-action
"ossf/scorecard-action": true,
}
pdata.results.TokenPermissions = append(pdata.results.TokenPermissions,
checker.TokenPermission{
File: &checker.File{
Path: fp,
Type: checker.FileTypeSource,
Offset: checker.OffsetDefault,
},
Type: checker.PermissionLevelUnknown,
Msg: stringPointer("not a codeql upload SARIF workflow"),

// TODO: Job
})

return false
}
tokenPermissions := checker.TokenPermission{
File: &checker.File{
Path: fp,
Type: checker.FileTypeSource,
Offset: checker.OffsetDefault,
// TODO: set Snippet.
},
Type: checker.PermissionLevelUnknown,
// TODO: Job
}

// nolint
// CodeQl run within GitHub worklow automatically bubbled up to
// security events, see
// https://docs.github.com/en/code-security/secure-coding/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning.
func isCodeQlAnalysisWorkflow(workflow *actionlint.Workflow, fp string, pdata *permissionCbData) bool {
for _, job := range workflow.Jobs {
for _, step := range job.Steps {
uses := fileparser.GetUses(step)
if uses == nil {
continue
}
if strings.HasPrefix(uses.Value, "github/codeql-action/analyze@") {
pdata.results.TokenPermissions = append(pdata.results.TokenPermissions,
checker.TokenPermission{
File: &checker.File{
Path: fp,
Type: checker.FileTypeSource,
Offset: fileparser.GetLineNumber(uses.Pos),
},
Type: checker.PermissionLevelUnknown,
Msg: stringPointer("codeql workflow detected"),
// TODO: Job
})

// remove any version pinning for the comparison
uses.Value = strings.Split(uses.Value, "@")[0]
if allowlist[uses.Value] {
tokenPermissions.File.Offset = fileparser.GetLineNumber(uses.Pos)
tokenPermissions.Msg = stringPointer("allowed SARIF workflow detected")
pdata.results.TokenPermissions = append(pdata.results.TokenPermissions, tokenPermissions)
return true
}
}
}

pdata.results.TokenPermissions = append(pdata.results.TokenPermissions,
checker.TokenPermission{
File: &checker.File{
Path: fp,
Type: checker.FileTypeSource,
Offset: checker.OffsetDefault,
},
Type: checker.PermissionLevelUnknown,
Msg: stringPointer("not a codeql workflow"),
})

tokenPermissions.Msg = stringPointer("not a SARIF workflow, or not an allowed one")
pdata.results.TokenPermissions = append(pdata.results.TokenPermissions, tokenPermissions)
return false
}

Expand Down
Loading