mirror of
https://github.com/ossf/scorecard.git
synced 2024-09-17 11:57:12 +03:00
Remove PullRequest check (#771)
Co-authored-by: Azeem Shaikh <azeems@google.com>
This commit is contained in:
Azeem Shaikh
GitHub
parent
59e14eef80
commit
1e6d99eb20
85
README.md
85
README.md
@ -7,22 +7,22 @@
|
||||
|
||||
<!-- vim-markdown-toc GFM -->
|
||||
|
||||
* [Motivation](#motivation)
|
||||
* [Goals](#goals)
|
||||
* [Scorecard Checks](#scorecard-checks)
|
||||
* [Usage](#usage)
|
||||
* [Docker](#docker)
|
||||
* [Using repository URL](#using-repository-url)
|
||||
* [Using a Package manager](#using-a-package-manager)
|
||||
* [Running specific checks](#running-specific-checks)
|
||||
* [Authentication](#authentication)
|
||||
* [Understanding Scorecard results](#understanding-scorecard-results)
|
||||
* [Formatting Results](#formatting-results)
|
||||
* [Public Data](#public-data)
|
||||
* [Adding a Scorecard Check](#adding-a-scorecard-check)
|
||||
* [Troubleshooting](#troubleshooting)
|
||||
* [Supportability](#supportability)
|
||||
* [Contributing](#contributing)
|
||||
* [Motivation](#motivation)
|
||||
* [Goals](#goals)
|
||||
* [Scorecard Checks](#scorecard-checks)
|
||||
* [Usage](#usage)
|
||||
* [Docker](#docker)
|
||||
* [Using repository URL](#using-repository-url)
|
||||
* [Using a Package manager](#using-a-package-manager)
|
||||
* [Running specific checks](#running-specific-checks)
|
||||
* [Authentication](#authentication)
|
||||
* [Understanding Scorecard results](#understanding-scorecard-results)
|
||||
* [Formatting Results](#formatting-results)
|
||||
* [Public Data](#public-data)
|
||||
* [Adding a Scorecard Check](#adding-a-scorecard-check)
|
||||
* [Troubleshooting](#troubleshooting)
|
||||
* [Supportability](#supportability)
|
||||
* [Contributing](#contributing)
|
||||
|
||||
<!-- vim-markdown-toc -->
|
||||
|
||||
@ -43,27 +43,25 @@ passed! All D's ... and an A!"
|
||||
|
||||
The following checks are all run against the target project by default:
|
||||
|
||||
Name | Description
|
||||
------------------ | -----------
|
||||
Active | Did the project get any commits in the last 90 days?
|
||||
Name | Description
|
||||
--------------------------- | -----------
|
||||
Active | Did the project get any commits in the last 90 days?
|
||||
Automatic-Dependency-Update | Does the project use tools to automatically update its dependencies?
|
||||
Binary-Artifacts | Is the project free of checked-in binaries?
|
||||
Branch-Protection | Does the project use [Branch Protection](https://docs.github.com/en/free-pro-team@latest/github/administering-a-repository/about-protected-branches) ?
|
||||
CI-Tests | Does the project run tests in CI, e.g. [GitHub Actions](https://docs.github.com/en/free-pro-team@latest/actions), [Prow](https://github.com/kubernetes/test-infra/tree/master/prow)?
|
||||
CII-Best-Practices | Does the project have a [CII Best Practices Badge](https://bestpractices.coreinfrastructure.org/en)?
|
||||
Code-Review | Does the project require code review before code is merged?
|
||||
Contributors | Does the project have contributors from at least two different organizations?
|
||||
Fuzzing | Does the project use fuzzing tools, e.g. [OSS-Fuzz](https://github.com/google/oss-fuzz)?
|
||||
Frozen-Deps | Does the project declare and freeze [dependencies](https://docs.github.com/en/free-pro-team@latest/github/visualizing-repository-data-with-graphs/about-the-dependency-graph#supported-package-ecosystems)?
|
||||
Packaging | Does the project build and publish official packages from CI/CD, e.g. [GitHub Publishing](https://docs.github.com/en/free-pro-team@latest/actions/guides/about-packaging-with-github-actions#workflows-for-publishing-packages) ?
|
||||
Pull-Requests | Does the project use [Pull Requests](https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/about-pull-requests) for all code changes?
|
||||
SAST | Does the project use static code analysis tools, e.g. [CodeQL](https://docs.github.com/en/free-pro-team@latest/github/finding-security-vulnerabilities-and-errors-in-your-code/enabling-code-scanning-for-a-repository#enabling-code-scanning-using-actions), [SonarCloud](https://sonarcloud.io)?
|
||||
Security-Policy | Does the project contain a [security policy](https://docs.github.com/en/free-pro-team@latest/github/managing-security-vulnerabilities/adding-a-security-policy-to-your-repository)?
|
||||
Signed-Releases | Does the project cryptographically [sign releases](https://wiki.debian.org/Creating%20signed%20GitHub%20releases)?
|
||||
Signed-Tags | Does the project cryptographically sign release tags?
|
||||
Token-Permissions | Does the project declare GitHub workflow tokens as [read only](https://docs.github.com/en/actions/reference/authentication-in-a-workflow)?
|
||||
Vulnerabilities | Does the project have unfixed vulnerabilities? Uses the [OSV service](https://osv.dev).
|
||||
|
||||
Binary-Artifacts | Is the project free of checked-in binaries?
|
||||
Branch-Protection | Does the project use [Branch Protection](https://docs.github.com/en/free-pro-team@latest/github/administering-a-repository/about-protected-branches) ?
|
||||
CI-Tests | Does the project run tests in CI, e.g. [GitHub Actions](https://docs.github.com/en/free-pro-team@latest/actions), [Prow](https://github.com/kubernetes/test-infra/tree/master/prow)?
|
||||
CII-Best-Practices | Does the project have a [CII Best Practices Badge](https://bestpractices.coreinfrastructure.org/en)?
|
||||
Code-Review | Does the project require code review before code is merged?
|
||||
Contributors | Does the project have contributors from at least two different organizations?
|
||||
Fuzzing | Does the project use fuzzing tools, e.g. [OSS-Fuzz](https://github.com/google/oss-fuzz)?
|
||||
Frozen-Deps | Does the project declare and freeze [dependencies](https://docs.github.com/en/free-pro-team@latest/github/visualizing-repository-data-with-graphs/about-the-dependency-graph#supported-package-ecosystems)?
|
||||
Packaging | Does the project build and publish official packages from CI/CD, e.g. [GitHub Publishing](https://docs.github.com/en/free-pro-team@latest/actions/guides/about-packaging-with-github-actions#workflows-for-publishing-packages) ?
|
||||
SAST | Does the project use static code analysis tools, e.g. [CodeQL](https://docs.github.com/en/free-pro-team@latest/github/finding-security-vulnerabilities-and-errors-in-your-code/enabling-code-scanning-for-a-repository#enabling-code-scanning-using-actions), [SonarCloud](https://sonarcloud.io)?
|
||||
Security-Policy | Does the project contain a [security policy](https://docs.github.com/en/free-pro-team@latest/github/managing-security-vulnerabilities/adding-a-security-policy-to-your-repository)?
|
||||
Signed-Releases | Does the project cryptographically [sign releases](https://wiki.debian.org/Creating%20signed%20GitHub%20releases)?
|
||||
Signed-Tags | Does the project cryptographically sign release tags?
|
||||
Token-Permissions | Does the project declare GitHub workflow tokens as [read only](https://docs.github.com/en/actions/reference/authentication-in-a-workflow)?
|
||||
Vulnerabilities | Does the project have unfixed vulnerabilities? Uses the [OSV service](https://osv.dev).
|
||||
|
||||
To see detailed information about each check and remediation steps, check out
|
||||
the [checks documentation page](checks/checks.md).
|
||||
@ -76,7 +74,7 @@ the [checks documentation page](checks/checks.md).
|
||||
|
||||
The `GITHUB_AUTH_TOKEN` has to be set to a valid [token](#Authentication)
|
||||
|
||||
``` shell
|
||||
```shell
|
||||
docker run -e GITHUB_AUTH_TOKEN=token gcr.io/openssf/scorecard:latest --show-details --repo=https://github.com/ossf/scorecard
|
||||
```
|
||||
|
||||
@ -146,13 +144,14 @@ Token-Permissions: Pass 10
|
||||
```
|
||||
|
||||
For more details why a check fails, use the `--show-details` option:
|
||||
|
||||
```
|
||||
./scorecard --repo=github.com/kubernetes/kubernetes --checks Frozen-Deps --show-details
|
||||
Starting [Frozen-Deps]
|
||||
Finished [Frozen-Deps]
|
||||
|
||||
RESULTS
|
||||
-------
|
||||
## RESULTS
|
||||
|
||||
Repo: github.com/kubernetes/kubernetes
|
||||
Frozen-Deps: Fail 10
|
||||
...
|
||||
@ -233,10 +232,10 @@ For example, `--checks=CI-Tests,Code-Review`.
|
||||
Before running Scorecard, you need to, either:
|
||||
|
||||
- [create a GitHub access token](https://docs.github.com/en/free-pro-team@latest/developers/apps/about-apps#personal-access-tokens)
|
||||
and set it in an environment variable called `GITHUB_AUTH_TOKEN`, `GITHUB_TOKEN`, `GH_AUTH_TOKEN` or `GH_TOKEN`. This helps to avoid
|
||||
the GitHub's
|
||||
[api rate limits](https://developer.github.com/v3/#rate-limiting) with
|
||||
unauthenticated requests.
|
||||
and set it in an environment variable called `GITHUB_AUTH_TOKEN`,
|
||||
`GITHUB_TOKEN`, `GH_AUTH_TOKEN` or `GH_TOKEN`. This helps to avoid the
|
||||
GitHub's [api rate limits](https://developer.github.com/v3/#rate-limiting)
|
||||
with unauthenticated requests.
|
||||
|
||||
```shell
|
||||
# For posix platforms, e.g. linux, mac:
|
||||
|
||||
@ -1,84 +0,0 @@
|
||||
// Copyright 2020 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 (
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
"github.com/google/go-github/v32/github"
|
||||
|
||||
"github.com/ossf/scorecard/v2/checker"
|
||||
sce "github.com/ossf/scorecard/v2/errors"
|
||||
)
|
||||
|
||||
// CheckPullRequests is the registered name for PullRequests.
|
||||
const CheckPullRequests = "Pull-Requests"
|
||||
|
||||
//nolint:gochecknoinits
|
||||
func init() {
|
||||
registerCheck(CheckPullRequests, PullRequests)
|
||||
}
|
||||
|
||||
func PullRequests(c *checker.CheckRequest) checker.CheckResult {
|
||||
commits, _, err := c.Client.Repositories.ListCommits(c.Ctx, c.Owner, c.Repo, &github.CommitsListOptions{})
|
||||
if err != nil {
|
||||
e := sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("Client.Repositories.ListCommits: %v", err))
|
||||
return checker.CreateRuntimeErrorResult(CheckPullRequests, e)
|
||||
}
|
||||
|
||||
total := 0
|
||||
totalWithPrs := 0
|
||||
for _, commit := range commits {
|
||||
isBot := false
|
||||
committer := commit.GetCommitter().GetLogin()
|
||||
for _, substring := range []string{"bot", "gardener"} {
|
||||
if strings.Contains(committer, substring) {
|
||||
isBot = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if isBot {
|
||||
c.Dlogger.Debug("skip commit from bot account: %s", committer)
|
||||
continue
|
||||
}
|
||||
|
||||
total++
|
||||
|
||||
// check for gerrit use via Reviewed-on
|
||||
commitMessage := commit.GetCommit().GetMessage()
|
||||
if strings.Contains(commitMessage, "\nReviewed-on: ") {
|
||||
totalWithPrs++
|
||||
c.Dlogger.Debug("Gerrit reviewed commit: %s", commit.GetSHA())
|
||||
continue
|
||||
}
|
||||
|
||||
prs, _, err := c.Client.PullRequests.ListPullRequestsWithCommit(c.Ctx, c.Owner, c.Repo, commit.GetSHA(),
|
||||
&github.PullRequestListOptions{})
|
||||
if err != nil {
|
||||
e := sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("Client.PullRequests.ListPullRequestsWithCommit: %v", err))
|
||||
return checker.CreateRuntimeErrorResult(CheckPullRequests, e)
|
||||
}
|
||||
if len(prs) > 0 {
|
||||
totalWithPrs++
|
||||
c.Dlogger.Debug("commit with PR: %s", commit.GetSHA())
|
||||
} else {
|
||||
c.Dlogger.Debug("found commit without PR: %s, committer: %s", commit.GetSHA(), commit.GetCommitter().GetLogin())
|
||||
}
|
||||
}
|
||||
|
||||
reason := fmt.Sprintf("%d ouf of %d commits have a PR", totalWithPrs, total)
|
||||
return checker.CreateProportionalScoreResult(CheckPullRequests, reason, totalWithPrs, total)
|
||||
}
|
||||
@ -116,17 +116,6 @@ The checks works by (1) looking for the following files in the root directory: g
|
||||
- For Dockerfiles and GitHub workflows, pin dependencies by hash. See example [gitcache-docker.yaml](https://github.com/ossf/scorecard/blob/main/.github/workflows/gitcache-docker.yaml#L36) and [Dockerfile](https://github.com/ossf/scorecard/blob/main/cron/worker/Dockerfile) examples.
|
||||
- To help update your dependencies after pinning them, use tools such as Github's [dependabot](https://github.blog/2020-06-01-keep-all-your-packages-up-to-date-with-dependabot/) or [renovate bot](https://github.com/renovatebot/renovate).
|
||||
|
||||
## Pull-Requests
|
||||
|
||||
This check tries to determine if the project requires pull requests for all changes to the default branch.
|
||||
Reviewing code improves quality of code in general. In addition, it ensures compromised contributors cannot intentionally inject malicious code. A low score is therefore considered `High` risk.
|
||||
The check works by looking at recent commits (first page, ~30) and uses the GitHub API to search for associated pull requests.
|
||||
|
||||
**Remediation steps**
|
||||
- Always open a pull request for any change you intend to make, big or small.
|
||||
- Make "pull requests" mandatory in your repository configuration. E.g. [GitHub](https://docs.github.com/en/github/administering-a-repository/about-protected-branches#require-pull-request-reviews-before-merging)
|
||||
- Enforce the rule for administrators / code owners as well. E.g. [GitHub](https://docs.github.com/en/github/administering-a-repository/about-protected-branches#include-administrators)
|
||||
|
||||
## SAST
|
||||
|
||||
This check tries to determine if the project uses static code analysis systems.
|
||||
|
||||
@ -227,27 +227,6 @@ checks:
|
||||
To help update your dependencies after pinning them, use tools such as
|
||||
Github's [dependabot](https://github.blog/2020-06-01-keep-all-your-packages-up-to-date-with-dependabot/)
|
||||
or [renovate bot](https://github.com/renovatebot/renovate).
|
||||
Pull-Requests:
|
||||
risk: High
|
||||
description: >-
|
||||
This check tries to determine if the project requires pull requests for
|
||||
all changes to the default branch.
|
||||
|
||||
Reviewing code improves quality of code in general. In addition, it ensures
|
||||
compromised contributors cannot intentionally inject malicious code. A low
|
||||
score is therefore considered `High` risk.
|
||||
|
||||
The check works by looking at recent commits (first page, ~30) and uses
|
||||
the GitHub API to search for associated pull requests.
|
||||
remediation:
|
||||
- >-
|
||||
Always open a pull request for any change you intend to make, big or small.
|
||||
- >-
|
||||
Make "pull requests" mandatory in your repository configuration. E.g.
|
||||
[GitHub](https://docs.github.com/en/github/administering-a-repository/about-protected-branches#require-pull-request-reviews-before-merging)
|
||||
- >-
|
||||
Enforce the rule for administrators / code owners as well. E.g.
|
||||
[GitHub](https://docs.github.com/en/github/administering-a-repository/about-protected-branches#include-administrators)
|
||||
SAST:
|
||||
risk: Medium
|
||||
description: >-
|
||||
|
||||
@ -1,58 +0,0 @@
|
||||
// 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.
|
||||
|
||||
package e2e
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
. "github.com/onsi/ginkgo"
|
||||
. "github.com/onsi/gomega"
|
||||
|
||||
"github.com/ossf/scorecard/v2/checker"
|
||||
"github.com/ossf/scorecard/v2/checks"
|
||||
scut "github.com/ossf/scorecard/v2/utests"
|
||||
)
|
||||
|
||||
var _ = Describe("E2E TEST:PullRequests", func() {
|
||||
Context("E2E TEST:Validating use of pull requests", func() {
|
||||
It("Should return use of pull requests", func() {
|
||||
dl := scut.TestDetailLogger{}
|
||||
req := checker.CheckRequest{
|
||||
Ctx: context.Background(),
|
||||
Client: ghClient,
|
||||
HTTPClient: httpClient,
|
||||
RepoClient: nil,
|
||||
Owner: "apache",
|
||||
Repo: "airflow",
|
||||
GraphClient: graphClient,
|
||||
Dlogger: &dl,
|
||||
}
|
||||
expected := scut.TestReturn{
|
||||
Errors: nil,
|
||||
Score: checker.MaxResultScore,
|
||||
NumberOfWarn: 0,
|
||||
NumberOfInfo: 0,
|
||||
NumberOfDebug: 30,
|
||||
}
|
||||
result := checks.PullRequests(&req)
|
||||
// UPGRADEv2: to remove.
|
||||
// Old version.
|
||||
Expect(result.Error).Should(BeNil())
|
||||
Expect(result.Pass).Should(BeTrue())
|
||||
// New version.
|
||||
Expect(scut.ValidateTestReturn(nil, "pull request used", &expected, &result, &dl)).Should(BeTrue())
|
||||
})
|
||||
})
|
||||
})
|
||||
Loading…
Reference in New Issue
Block a user