diff --git a/checks/errors.go b/checks/errors.go index 86efc4df..dd8d8c54 100644 --- a/checks/errors.go +++ b/checks/errors.go @@ -27,7 +27,7 @@ var ( errInternalInvalidShellCode = errors.New("invalid shell code") errInternalCommitishNil = errors.New("commitish is nil") errInternalBranchNotFound = errors.New("branch not found") - errInvalidGitHubWorkflowFile = errors.New("invalid GitHub workflow file") + errInvalidGitHubWorkflow = errors.New("invalid GitHub workflow") errInternalNoReviews = errors.New("no reviews found") errInternalNoCommits = errors.New("no commits found") ) diff --git a/checks/fuzzing.go b/checks/fuzzing.go index 92af6885..c49ab457 100644 --- a/checks/fuzzing.go +++ b/checks/fuzzing.go @@ -43,8 +43,8 @@ func Fuzzing(c *checker.CheckRequest) checker.CheckResult { if *results.Total > 0 { return checker.CreateMaxScoreResult(CheckFuzzing, - "project is fuzzed by OSS-Fuzz") + "project is fuzzed in OSS-Fuzz") } - return checker.CreateMinScoreResult(CheckFuzzing, "project is not fuzzed by OSS-Fuzz") + return checker.CreateMinScoreResult(CheckFuzzing, "project is not fuzzed in OSS-Fuzz") } diff --git a/checks/packaging.go b/checks/packaging.go index 5c1dc266..c53fd09a 100644 --- a/checks/packaging.go +++ b/checks/packaging.go @@ -53,7 +53,7 @@ func Packaging(c *checker.CheckRequest) checker.CheckResult { return checker.CreateRuntimeErrorResult(CheckPackaging, e) } - if !isPackagingWorkflow(string(fc), fp, c) { + if !isPackagingWorkflow(string(fc), fp, c.Dlogger) { continue } @@ -68,58 +68,94 @@ func Packaging(c *checker.CheckRequest) checker.CheckResult { if *runs.TotalCount > 0 { c.Dlogger.Info("workflow %v used in run: %s", fp, runs.WorkflowRuns[0].GetHTMLURL()) return checker.CreateMaxScoreResult(CheckPackaging, - "packaging workflow detected") + "publishing workflow detected") } c.Dlogger.Info("workflow %v not used in runs", fp) } - return checker.CreateMinScoreResult(CheckPackaging, - "no packaging workflow used") + c.Dlogger.Warn("no publishing GitHub workflow detected") + + return checker.CreateInconclusiveResult(CheckPackaging, + "no published package detected") } -func isPackagingWorkflow(s, fp string, c *checker.CheckRequest) bool { - // nodejs packages - if strings.Contains(s, "uses: actions/setup-node@") { +// A packaging workflow. +func isPackagingWorkflow(s, fp string, dl checker.DetailLogger) bool { + // Nodejs packages. + if strings.Contains(s, "actions/setup-node@") { r1 := regexp.MustCompile(`(?s)registry-url.*https://registry\.npmjs\.org`) r2 := regexp.MustCompile(`(?s)npm.*publish`) if r1.MatchString(s) && r2.MatchString(s) { - c.Dlogger.Info("candidate node packaging workflow using npm: %s", fp) + dl.Info("candidate node publishing workflow using npm: %s", fp) return true } } - if strings.Contains(s, "uses: actions/setup-java@") { + // Java packages. + if strings.Contains(s, "actions/setup-java@") { // Java packages with maven. r1 := regexp.MustCompile(`(?s)mvn.*deploy`) if r1.MatchString(s) { - c.Dlogger.Info("candidate java packaging workflow using maven: %s", fp) + dl.Info("candidate java publishing workflow using maven: %s", fp) return true } // Java packages with gradle. r2 := regexp.MustCompile(`(?s)gradle.*publish`) if r2.MatchString(s) { - c.Dlogger.Info("candidate java packaging workflow using gradle: %s", fp) + dl.Info("candidate java publishing workflow using gradle: %s", fp) return true } } + // Ruby packages. + r := regexp.MustCompile(`(?s)gem.*push`) + if r.MatchString(s) { + dl.Info("ruby publishing workflow using gem: %s", fp) + return true + } + + // NuGet packages. + r = regexp.MustCompile(`(?s)nuget.*push`) + if r.MatchString(s) { + dl.Info("nuget publishing workflow: %s", fp) + return true + } + + // Docker packages. + if strings.Contains(s, "docker/build-push-action@") { + dl.Info("candidate docker publishing workflow: %s", fp) + return true + } + + r = regexp.MustCompile(`(?s)docker.*push`) + if r.MatchString(s) { + dl.Info("candidate docker publishing workflow: %s", fp) + return true + } + + // Python packages. if strings.Contains(s, "actions/setup-python@") && strings.Contains(s, "pypa/gh-action-pypi-publish@master") { - c.Dlogger.Info("candidate python packaging workflow using pypi: %s", fp) + dl.Info("candidate python publishing workflow using pypi: %s", fp) return true } - if strings.Contains(s, "uses: docker/build-push-action@") { - c.Dlogger.Info("candidate docker publishing workflow: %s", fp) + // Go packages. + if strings.Contains(s, "actions/setup-go") && + strings.Contains(s, "goreleaser/goreleaser-action@") { + dl.Info("candidate golang publishing workflow: %s", fp) return true } - if strings.Contains(s, "docker push") { - c.Dlogger.Info("candidate docker publishing workflow: %s", fp) + // Rust packages. + // https://doc.rust-lang.org/cargo/reference/publishing.html. + r = regexp.MustCompile(`(?s)cargo.*publish`) + if r.MatchString(s) { + dl.Info("candidate rust publishing workflow using cargo: %s", fp) return true } - c.Dlogger.Debug("not a packaging workflow: %s", fp) + dl.Debug("not a publishing workflow: %s", fp) return false } diff --git a/checks/permissions.go b/checks/permissions.go index 9f3d3ad4..8589499e 100644 --- a/checks/permissions.go +++ b/checks/permissions.go @@ -36,29 +36,35 @@ func init() { // Each field correpsonds to a GitHub permission type, and // will hold true if declared non-write, false otherwise. type permissionCbData struct { - writePermissions map[string]bool + topLevelWritePermissions map[string]bool + runLevelWritePermissions map[string]bool } // TokenPermissions runs Token-Permissions check. func TokenPermissions(c *checker.CheckRequest) checker.CheckResult { - data := permissionCbData{writePermissions: make(map[string]bool)} + // data is shared across all GitHub workflows. + data := permissionCbData{ + topLevelWritePermissions: make(map[string]bool), + runLevelWritePermissions: make(map[string]bool), + } err := CheckFilesContent(".github/workflows/*", false, c, validateGitHubActionTokenPermissions, &data) return createResultForLeastPrivilegeTokens(data, err) } func validatePermission(key string, value interface{}, path string, - dl checker.DetailLogger, pdata *permissionCbData) error { + dl checker.DetailLogger, pPermissions map[string]bool, + ignoredPermissions map[string]bool) error { val, ok := value.(string) if !ok { //nolint - return sce.Create(sce.ErrScorecardInternal, errInvalidGitHubWorkflowFile.Error()) + return sce.Create(sce.ErrScorecardInternal, errInvalidGitHubWorkflow.Error()) } if strings.EqualFold(val, "write") { - if isPermissionOfInterest(key) { + if isPermissionOfInterest(key, ignoredPermissions) { dl.Warn("'%v' permission set to '%v' in %v", key, val, path) - recordPermissionWrite(key, pdata) + recordPermissionWrite(key, pPermissions) } else { // Only log for debugging, otherwise // it may confuse users. @@ -72,44 +78,36 @@ func validatePermission(key string, value interface{}, path string, } func validateMapPermissions(values map[interface{}]interface{}, path string, - dl checker.DetailLogger, pdata *permissionCbData) error { + dl checker.DetailLogger, pPermissions map[string]bool, + ignoredPermissions map[string]bool) error { // Iterate over the permission, verify keys and values are strings. for k, v := range values { key, ok := k.(string) if !ok { //nolint - return sce.Create(sce.ErrScorecardInternal, errInvalidGitHubWorkflowFile.Error()) + return sce.Create(sce.ErrScorecardInternal, errInvalidGitHubWorkflow.Error()) } - if err := validatePermission(key, v, path, dl, pdata); err != nil { + if err := validatePermission(key, v, path, dl, pPermissions, ignoredPermissions); err != nil { return err } } return nil } -func recordPermissionWrite(name string, pdata *permissionCbData) { - pdata.writePermissions[name] = true +func recordPermissionWrite(name string, pPermissions map[string]bool) { + pPermissions[name] = true } -func recordAllPermissionsWrite(pdata *permissionCbData) { +func recordAllPermissionsWrite(pPermissions map[string]bool) { // Special case: `all` does not correspond // to a GitHub permission. - pdata.writePermissions["all"] = true + pPermissions["all"] = true } -func validateReadPermissions(config map[interface{}]interface{}, path string, - dl checker.DetailLogger, pdata *permissionCbData) error { - var permissions interface{} - - // Check if permissions are set explicitly. - permissions, ok := config["permissions"] - if !ok { - dl.Warn("no permission defined in %v", path) - recordAllPermissionsWrite(pdata) - return nil - } - +func validatePermissions(permissions interface{}, path string, + dl checker.DetailLogger, pPermissions map[string]bool, + ignoredPermissions map[string]bool) error { // Check the type of our values. switch val := permissions.(type) { // Empty string is nil type. @@ -120,34 +118,96 @@ func validateReadPermissions(config map[interface{}]interface{}, path string, case string: if !strings.EqualFold(val, "read-all") && val != "" { dl.Warn("permissions set to '%v' in %v", val, path) - recordAllPermissionsWrite(pdata) + recordAllPermissionsWrite(pPermissions) return nil } dl.Info("permission set to '%v' in %v", val, path) // Map type. case map[interface{}]interface{}: - if err := validateMapPermissions(val, path, dl, pdata); err != nil { + if err := validateMapPermissions(val, path, dl, pPermissions, ignoredPermissions); err != nil { return err } // Invalid type. default: //nolint - return sce.Create(sce.ErrScorecardInternal, errInvalidGitHubWorkflowFile.Error()) + return sce.Create(sce.ErrScorecardInternal, errInvalidGitHubWorkflow.Error()) } - return nil } -func isPermissionOfInterest(name string) bool { - return strings.EqualFold(name, "statuses") || - strings.EqualFold(name, "checks") || - strings.EqualFold(name, "security-events") || - strings.EqualFold(name, "deployments") || - strings.EqualFold(name, "contents") || - strings.EqualFold(name, "packages") || - strings.EqualFold(name, "options") +func validateTopLevelPermissions(config map[interface{}]interface{}, path string, + dl checker.DetailLogger, pdata *permissionCbData) error { + // Check if permissions are set explicitly. + permissions, ok := config["permissions"] + if !ok { + dl.Warn("no permission defined in %v", path) + recordAllPermissionsWrite(pdata.topLevelWritePermissions) + return nil + } + + return validatePermissions(permissions, path, dl, + pdata.topLevelWritePermissions, map[string]bool{}) +} + +func validateRunLevelPermissions(config map[interface{}]interface{}, path string, + dl checker.DetailLogger, pdata *permissionCbData, + ignoredPermissions map[string]bool) error { + var jobs interface{} + + jobs, ok := config["jobs"] + if !ok { + return nil + } + + mjobs, ok := jobs.(map[interface{}]interface{}) + if !ok { + //nolint:wrapcheck + return sce.Create(sce.ErrScorecardInternal, errInvalidGitHubWorkflow.Error()) + } + + for _, value := range mjobs { + job, ok := value.(map[interface{}]interface{}) + if !ok { + //nolint:wrapcheck + return sce.Create(sce.ErrScorecardInternal, errInvalidGitHubWorkflow.Error()) + } + // Run-level permissions may be left undefined. + // For most workflows, no write permissions are needed, + // so only top-level read-only permissions need to be declared. + permissions, ok := job["permissions"] + if !ok { + dl.Debug("no permission defined in %v", path) + continue + } + err := validatePermissions(permissions, path, dl, + pdata.runLevelWritePermissions, ignoredPermissions) + if err != nil { + return err + } + } + return nil +} + +func isPermissionOfInterest(name string, ignoredPermissions map[string]bool) bool { + permissions := []string{ + "statuses", "checks", "security-events", + "deployments", "contents", "packages", "actions", + } + for _, p := range permissions { + _, present := ignoredPermissions[p] + if strings.EqualFold(name, p) && !present { + return true + } + } + return false +} + +func permissionIsPresent(result permissionCbData, name string) bool { + _, ok1 := result.topLevelWritePermissions[name] + _, ok2 := result.runLevelWritePermissions[name] + return ok1 || ok2 } // Calculate the score. @@ -155,29 +215,32 @@ func calculateScore(result permissionCbData) int { // See list https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/. // Note: there are legitimate reasons to use some of the permissions like checks, deployments, etc. // in CI/CD systems https://docs.travis-ci.com/user/github-oauth-scopes/. - if _, ok := result.writePermissions["all"]; ok { + + if permissionIsPresent(result, "all") { return checker.MinResultScore } + // Start with a perfect score. score := float32(checker.MaxResultScore) + // status: https://docs.github.com/en/rest/reference/repos#statuses. // May allow an attacker to change the result of pre-submit and get a PR merged. // Low risk: -0.5. - if _, ok := result.writePermissions["statuses"]; ok { + if permissionIsPresent(result, "statuses") { score -= 0.5 } // checks. // May allow an attacker to edit checks to remove pre-submit and introduce a bug. // Low risk: -0.5. - if _, ok := result.writePermissions["checks"]; ok { + if permissionIsPresent(result, "checks") { score -= 0.5 } // secEvents. // May allow attacker to read vuln reports before patch available. // Low risk: -1 - if _, ok := result.writePermissions["security-events"]; ok { + if permissionIsPresent(result, "security-events") { score-- } @@ -186,31 +249,34 @@ func calculateScore(result permissionCbData) int { // and tiny chance an attacker can trigger a remote // service with code they own if server accepts code/location var unsanitized. // Low risk: -1 - if _, ok := result.writePermissions["deployments"]; ok { + if permissionIsPresent(result, "deployments") { score-- } // contents. // Allows attacker to commit unreviewed code. // High risk: -10 - if _, ok := result.writePermissions["contents"]; ok { + if permissionIsPresent(result, "contents") { score -= checker.MaxResultScore } - // packages. + // packages: https://docs.github.com/en/packages/learn-github-packages/about-permissions-for-github-packages. // Allows attacker to publish packages. // High risk: -10 - if _, ok := result.writePermissions["packages"]; ok { + if permissionIsPresent(result, "packages") { score -= checker.MaxResultScore } // actions. // May allow an attacker to steal GitHub secrets by adding a malicious workflow/action. // High risk: -10 - if _, ok := result.writePermissions["actions"]; ok { + if permissionIsPresent(result, "actions") { score -= checker.MaxResultScore } + // 2. Run-level permissions. + + // We're done, calculate the final score. if score < checker.MinResultScore { return checker.MinResultScore } @@ -237,7 +303,10 @@ func createResultForLeastPrivilegeTokens(result permissionCbData, err error) che func testValidateGitHubActionTokenPermissions(pathfn string, content []byte, dl checker.DetailLogger) checker.CheckResult { - data := permissionCbData{writePermissions: make(map[string]bool)} + data := permissionCbData{ + topLevelWritePermissions: make(map[string]bool), + runLevelWritePermissions: make(map[string]bool), + } _, err := validateGitHubActionTokenPermissions(pathfn, content, dl, &data) return createResultForLeastPrivilegeTokens(data, err) } @@ -264,12 +333,19 @@ func validateGitHubActionTokenPermissions(path string, content []byte, sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("yaml.Unmarshal: %v", err)) } - // 1. Check that each file uses 'content: read' only or 'none'. + // 1. Top-level permission definitions. //nolint // https://docs.github.com/en/actions/reference/authentication-in-a-workflow#example-1-passing-the-github_token-as-an-input, // https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/, // https://docs.github.com/en/actions/reference/authentication-in-a-workflow#modifying-the-permissions-for-the-github_token. - if err := validateReadPermissions(workflow, path, dl, pdata); err != nil { + if err := validateTopLevelPermissions(workflow, path, dl, pdata); err != nil { + return false, err + } + + // 2. Run-level permission definitions, + // see https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idpermissions. + ignoredPermissions := createIgnoredPermissions(string(content), path, dl) + if err := validateRunLevelPermissions(workflow, path, dl, pdata, ignoredPermissions); err != nil { return false, err } @@ -279,3 +355,34 @@ func validateGitHubActionTokenPermissions(path string, content []byte, return true, nil } + +func createIgnoredPermissions(s, fp string, dl checker.DetailLogger) map[string]bool { + ignoredPermissions := make(map[string]bool) + if requiresPackagesPermissions(s, fp, dl) { + ignoredPermissions["packages"] = true + } + if isCodeQlAnalysisWorkflow(s, fp, dl) { + ignoredPermissions["security-events"] = true + } + return ignoredPermissions +} + +func isCodeQlAnalysisWorkflow(s, fp string, dl checker.DetailLogger) bool { + if strings.Contains(s, "github/codeql-action/analyze@") { + dl.Debug("codeql workflow detected: %v", fp) + return true + } + dl.Debug("not a codeql workflow: %v", fp) + return false +} + +// A packaging workflow using GitHub's supported packages: +// https://docs.github.com/en/packages. +func requiresPackagesPermissions(s, fp string, dl checker.DetailLogger) bool { + // TODO: add support for GitHub registries. + // Example: https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-npm-registry. + // This feature requires parsing actions properly. + // For now, we just re-use the Packaging check to verify that the + // workflow is a packaging workflow. + return isPackagingWorkflow(s, fp, dl) +} diff --git a/checks/permissions_test.go b/checks/permissions_test.go index 5d148b8e..fc04051b 100644 --- a/checks/permissions_test.go +++ b/checks/permissions_test.go @@ -33,80 +33,146 @@ func TestGithubTokenPermissions(t *testing.T) { expected scut.TestReturn }{ { - name: "Write all test", + name: "run workflow codeql write test", + filename: "./testdata/github-workflow-permissions-run-codeql-write.yaml", + expected: scut.TestReturn{ + Errors: nil, + Score: checker.MaxResultScore, + NumberOfWarn: 0, + NumberOfInfo: 1, + NumberOfDebug: 3, + }, + }, + { + name: "run workflow codeql write test", + filename: "./testdata/github-workflow-permissions-run-no-codeql-write.yaml", + expected: scut.TestReturn{ + Errors: nil, + Score: checker.MaxResultScore - 1, + NumberOfWarn: 1, + NumberOfInfo: 1, + NumberOfDebug: 2, + }, + }, + { + name: "run workflow run write test", + filename: "./testdata/github-workflow-permissions-run-writes-2.yaml", + expected: scut.TestReturn{ + Errors: nil, + Score: checker.MinResultScore, + NumberOfWarn: 3, + NumberOfInfo: 2, + NumberOfDebug: 2, + }, + }, + { + name: "run package workflow write test", + filename: "./testdata/github-workflow-permissions-run-package-workflow-write.yaml", + expected: scut.TestReturn{ + Errors: nil, + Score: checker.MaxResultScore, + NumberOfWarn: 0, + NumberOfInfo: 2, + NumberOfDebug: 2, + }, + }, + { + name: "run package write test", + filename: "./testdata/github-workflow-permissions-run-package-write.yaml", + expected: scut.TestReturn{ + Errors: nil, + Score: checker.MinResultScore, + NumberOfWarn: 1, + NumberOfInfo: 1, + NumberOfDebug: 2, + }, + }, + { + name: "run writes test", + filename: "./testdata/github-workflow-permissions-run-writes.yaml", + expected: scut.TestReturn{ + Errors: nil, + Score: checker.MaxResultScore, + NumberOfWarn: 0, + NumberOfInfo: 1, + NumberOfDebug: 3, + }, + }, + { + name: "write all test", filename: "./testdata/github-workflow-permissions-writeall.yaml", expected: scut.TestReturn{ Errors: nil, Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 0, - NumberOfDebug: 0, + NumberOfDebug: 3, }, }, { - name: "Read all test", + name: "read all test", filename: "./testdata/github-workflow-permissions-readall.yaml", expected: scut.TestReturn{ Errors: nil, Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 1, - NumberOfDebug: 0, + NumberOfDebug: 3, }, }, { - name: "No permission test", + name: "no permission test", filename: "./testdata/github-workflow-permissions-absent.yaml", expected: scut.TestReturn{ Errors: nil, Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 0, - NumberOfDebug: 0, + NumberOfDebug: 3, }, }, { - name: "Writes test", + name: "writes test", filename: "./testdata/github-workflow-permissions-writes.yaml", expected: scut.TestReturn{ Errors: nil, Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 1, - NumberOfDebug: 1, + NumberOfDebug: 4, }, }, { - name: "Reads test", + name: "reads test", filename: "./testdata/github-workflow-permissions-reads.yaml", expected: scut.TestReturn{ Errors: nil, Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 10, - NumberOfDebug: 0, + NumberOfDebug: 3, }, }, { - name: "Nones test", + name: "nones test", filename: "./testdata/github-workflow-permissions-nones.yaml", expected: scut.TestReturn{ Errors: nil, Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 10, - NumberOfDebug: 0, + NumberOfDebug: 3, }, }, { - name: "None test", + name: "none test", filename: "./testdata/github-workflow-permissions-none.yaml", expected: scut.TestReturn{ Errors: nil, Score: checker.MaxResultScore, NumberOfWarn: 0, NumberOfInfo: 1, - NumberOfDebug: 0, + NumberOfDebug: 3, }, }, { @@ -117,7 +183,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore - 1, NumberOfWarn: 2, NumberOfInfo: 2, - NumberOfDebug: 1, + NumberOfDebug: 4, }, }, { @@ -128,7 +194,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MaxResultScore - 2, NumberOfWarn: 2, NumberOfInfo: 3, - NumberOfDebug: 0, + NumberOfDebug: 3, }, }, { @@ -139,7 +205,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 2, - NumberOfDebug: 0, + NumberOfDebug: 3, }, }, { @@ -150,7 +216,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 2, - NumberOfDebug: 0, + NumberOfDebug: 3, }, }, { @@ -161,7 +227,7 @@ func TestGithubTokenPermissions(t *testing.T) { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 1, - NumberOfDebug: 0, + NumberOfDebug: 3, }, }, } diff --git a/checks/sast.go b/checks/sast.go index be51883c..fcd7fdd2 100644 --- a/checks/sast.go +++ b/checks/sast.go @@ -168,13 +168,13 @@ func codeQLInCheckDefinitions(c *checker.CheckRequest) (int, error) { } for _, result := range results.CodeResults { - c.Dlogger.Info("CodeQL detected: %s", result.GetPath()) + c.Dlogger.Debug("CodeQL detected: %s", result.GetPath()) } // TODO: check if it's enabled as cron or presubmit. // TODO: check which branches it is enabled on. We should find main. if *results.Total > 0 { - c.Dlogger.Info("tool detected: CodeQL") + c.Dlogger.Info("SAST tool detected: CodeQL") return checker.MaxResultScore, nil } diff --git a/checks/signed_releases.go b/checks/signed_releases.go index 6c147b99..8f6ec0dc 100644 --- a/checks/signed_releases.go +++ b/checks/signed_releases.go @@ -56,7 +56,7 @@ func SignedReleases(c *checker.CheckRequest) checker.CheckResult { if len(assets) == 0 { continue } - c.Dlogger.Debug("release found: %s", r.GetTagName()) + c.Dlogger.Debug("GitHub release found: %s", r.GetTagName()) totalReleases++ signed := false for _, asset := range assets { @@ -81,7 +81,10 @@ func SignedReleases(c *checker.CheckRequest) checker.CheckResult { } if totalReleases == 0 { - return checker.CreateInconclusiveResult(CheckSignedReleases, "no release found") + // GitHub-specific message. + c.Dlogger.Warn("no GitHub releases found") + // Generic summary. + return checker.CreateInconclusiveResult(CheckSignedReleases, "no releases found") } reason := fmt.Sprintf("%d out of %d artifacts are signed", totalSigned, totalReleases) diff --git a/checks/testdata/github-workflow-permissions-run-codeql-write.yaml b/checks/testdata/github-workflow-permissions-run-codeql-write.yaml new file mode 100644 index 00000000..fb2a2b49 --- /dev/null +++ b/checks/testdata/github-workflow-permissions-run-codeql-write.yaml @@ -0,0 +1,26 @@ +# Copyright 2021 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. +name: write-and-read workflow +on: [push] +permissions: read-all + +jobs: + Explore-GitHub-Actions: + runs-on: ubuntu-latest + permissions: + security-events: write + steps: + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v1 + run: echo "write-and-read workflow" \ No newline at end of file diff --git a/checks/testdata/github-workflow-permissions-run-no-codeql-write.yaml b/checks/testdata/github-workflow-permissions-run-no-codeql-write.yaml new file mode 100644 index 00000000..efa40b7a --- /dev/null +++ b/checks/testdata/github-workflow-permissions-run-no-codeql-write.yaml @@ -0,0 +1,26 @@ +# Copyright 2021 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. +name: write-and-read workflow +on: [push] +permissions: read-all + +jobs: + Explore-GitHub-Actions: + runs-on: ubuntu-latest + permissions: + security-events: write + steps: + - name: Perform CodeQL Analysis + uses: github/some-action/analyze@v1 + run: echo "write-and-read workflow" diff --git a/checks/testdata/github-workflow-permissions-run-package-workflow-write.yaml b/checks/testdata/github-workflow-permissions-run-package-workflow-write.yaml new file mode 100644 index 00000000..6267fda7 --- /dev/null +++ b/checks/testdata/github-workflow-permissions-run-package-workflow-write.yaml @@ -0,0 +1,26 @@ +# Copyright 2021 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. +name: write-and-read workflow +on: [push] +permissions: read-all + +jobs: + Explore-GitHub-Actions: + runs-on: ubuntu-latest + permissions: + packages: write + steps: + - name: some name + run: echo "write-and-read workflow" + uses: docker/build-push-action@1.2.3 \ No newline at end of file diff --git a/checks/testdata/github-workflow-permissions-run-package-write.yaml b/checks/testdata/github-workflow-permissions-run-package-write.yaml new file mode 100644 index 00000000..bb5fe955 --- /dev/null +++ b/checks/testdata/github-workflow-permissions-run-package-write.yaml @@ -0,0 +1,25 @@ +# Copyright 2021 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. +name: write-and-read workflow +on: [push] +permissions: read-all + +jobs: + Explore-GitHub-Actions: + runs-on: ubuntu-latest + permissions: + packages: write + steps: + - name: some name + run: echo "write-and-read workflow" diff --git a/checks/testdata/github-workflow-permissions-run-writes-2.yaml b/checks/testdata/github-workflow-permissions-run-writes-2.yaml new file mode 100644 index 00000000..b08170e9 --- /dev/null +++ b/checks/testdata/github-workflow-permissions-run-writes-2.yaml @@ -0,0 +1,28 @@ +# Copyright 2021 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. +name: write-and-read workflow +on: [push] +permissions: + packages: read + pull-requests: read + +jobs: + Explore-GitHub-Actions: + runs-on: ubuntu-latest + permissions: + contents: write + packages: write + actions: write + steps: + - run: echo "write-and-read workflow" \ No newline at end of file diff --git a/checks/testdata/github-workflow-permissions-run-writes.yaml b/checks/testdata/github-workflow-permissions-run-writes.yaml new file mode 100644 index 00000000..cfb7c84e --- /dev/null +++ b/checks/testdata/github-workflow-permissions-run-writes.yaml @@ -0,0 +1,24 @@ +# Copyright 2021 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. +name: write-and-read workflow +on: [push] +permissions: read-all + +jobs: + Explore-GitHub-Actions: + runs-on: ubuntu-latest + permissions: + pull-requests: write + steps: + - run: echo "write-and-read workflow" \ No newline at end of file diff --git a/docs/checks.md b/docs/checks.md index d3ab5ab3..6be5b29c 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -164,7 +164,7 @@ The check does not verify the signature itself and currently relies on GitHub's This check tries to determine if a project's GitHub workflows follow the principle of least privilege, i.e. if the GitHub tokens are set read-only by default. A compromised token with write access may be used by attackers to push malicious code into the project. A low score is therefore considered `High` risk. -For each workflow yaml file, the check looks for the permissions keyword. If it is set globally as read-only for the entire file, this check succeeds. Otherwise it fails. The check cannot detect if the "read-only" GitHub permission settings is enabled, as there is no API available. +For each workflow yaml file, the check looks for the permission definitions. To obtain the highest score, the permissions should be set as read-only at the [top level](https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#permissions) and the required write permissions should be declared at the [run-level](https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idpermissions). The check cannot detect if the "read-only" GitHub permission settings is enabled, as there is no API available. **Remediation steps** - Set permissions as `read-all` or `contents: read` as described in GitHub's [documentation](https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#permissions). diff --git a/docs/checks/checks.yaml b/docs/checks/checks.yaml index d93f2057..227a5713 100644 --- a/docs/checks/checks.yaml +++ b/docs/checks/checks.yaml @@ -305,9 +305,10 @@ checks: push malicious code into the project. A low score is therefore considered `High` risk. For each workflow yaml file, the check looks - for the permissions keyword. If it is set globally as read-only for the entire file, - this check succeeds. Otherwise it fails. The check cannot detect if the "read-only" - GitHub permission settings is enabled, as there is no API available. + for the permission definitions. To obtain the highest score, the permissions should be set as + read-only at the [top level](https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#permissions) + and the required write permissions should be declared at the [run-level](https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idpermissions). + The check cannot detect if the "read-only" GitHub permission settings is enabled, as there is no API available. remediation: - >- Set permissions as `read-all` or `contents: read` as described in diff --git a/e2e/permissions_test.go b/e2e/permissions_test.go index 61715438..2d4b02f6 100644 --- a/e2e/permissions_test.go +++ b/e2e/permissions_test.go @@ -48,7 +48,7 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() { Score: checker.MinResultScore, NumberOfWarn: 1, NumberOfInfo: 2, - NumberOfDebug: 0, + NumberOfDebug: 3, } result := checks.TokenPermissions(&req) // UPGRADEv2: to remove. diff --git a/e2e/sast_test.go b/e2e/sast_test.go index 136fc930..f35450a7 100644 --- a/e2e/sast_test.go +++ b/e2e/sast_test.go @@ -43,8 +43,8 @@ var _ = Describe("E2E TEST:SAST", func() { Errors: nil, Score: 7, NumberOfWarn: 1, - NumberOfInfo: 2, - NumberOfDebug: 0, + NumberOfInfo: 1, + NumberOfDebug: 1, } result := checks.SAST(&req) // UPGRADEv2: to remove.