ossf / scorecard

OpenSSF Scorecard - Security health metrics for Open Source
https://scorecard.dev
Apache License 2.0
4.3k stars 470 forks source link

Scorecard fails to detect frozen dependencies in Ruby applications (including Rails apps) #977

Closed david-a-wheeler closed 2 years ago

david-a-wheeler commented 2 years ago

Describe the bug One of scorecard requirements is "frozen dependencies". In Ruby applications, including Rails, frozen dependencies are implemented via a Gemfile.lock file. However, such Ruby applications (such as the CII Best Practices badge project) are incorrectly marked as not having frozen dependencies.

Reproduction steps Steps to reproduce the behavior:

  1. View the OpenSSF metrics dashboard for the CII Best Practices badge web application
  2. Under Scorecard, Frozen deps, it shows "?"
  3. Yet the CII Best Practices badge application repo clearly shows a Gemfile.lock file (not just a Gemfile).

Expected behavior I expected full credit for having frozen dependencies.

Additional context This is part of a larger effort to get other OpenSSF projects to adopt the scorecard work. We're starting by looking at the CII Best Practices badge & seeing what's there, what isn't, and what could be changed.

naveensrinivasan commented 2 years ago

The issue isn't with the gemfile.lock that scorecard is able to identify. It is the other dependencies that aren't pinned.

{
  "date": "2021-09-07",
  "repo": {
    "name": "github.com/coreinfrastructure/best-practices-badge",
    "commit": "7304fc9a44ffe48087948b0d015c12506138ac02"
  },
  "scorecard": {
    "version": "",
    "commit": ""
  },
  "checks": [
    {
      "details": [
        "Info: ruby lock file detected: Gemfile.lock",
        "Warn: unpinned dependency detected (job 'build'): .github/workflows/main.yml:23",
        "Warn: unpinned dependency detected (job 'build'): .github/workflows/main.yml:37",
        "Warn: unpinned dependency detected in dockerfiles/2.5.1-stretch/Dockerfile: 'circleci/ruby:2.5.1-stretch-node-browsers-legacy'",
        "Warn: unpinned dependency detected in dockerfiles/2.6.3-stretch/Dockerfile: 'circleci/ruby:2.6.3-stretch-node-browsers'",
        "Warn: unpinned dependency detected in dockerfiles/2.6.6-stretch/Dockerfile: 'circleci/ruby:2.6.6-stretch-node-browsers'",
        "Warn: unpinned dependency detected in dockerfiles/2.7.2-browsers/Dockerfile: 'cimg/ruby:2.7.2-browsers'",
        "Warn: unpinned dependency detected in dockerfiles/2.7.2-buster/Dockerfile: 'circleci/ruby:2.7.2-buster-node-browsers'",
        "Warn: unpinned dependency detected in dockerfiles/3.0.0-browsers/Dockerfile: 'cimg/ruby:3.0.0-browsers'",
        "Warn: unpinned dependency detected in dockerfiles/3.0.1-browsers/Dockerfile: 'cimg/ruby:3.0.1-browsers'",
        "Info: no insecure (unpinned) dependency downloads found in Dockerfiles",
        "Info: no insecure (unpinned) dependency downloads found in shell scripts",
        "Info: no insecure (unpinned) dependency downloads found in GitHub workflows"
      ],
      "score": 6,
      "reason": "unpinned dependencies detected -- score normalized to 6",
      "name": "Pinned-Dependencies"
    }
  ],
  "metadata": null
}
naveensrinivasan commented 2 years ago

go run . --repo=https://github.com/coreinfrastructure/best-practices-badge --checks=Pinned-Dependencies --show-details --format=json | jq will give the details on the issue

david-a-wheeler commented 2 years ago

Thanks for the details. Let me see here:

"Warn: unpinned dependency detected (job 'build'): .github/workflows/main.yml:23",

That's for:

uses: actions/checkout@v2`

That's the usual recommended way to checkout projects on GitHub in a GitHub action, and it does specify the major version number. What would be recommended instead?

"Warn: unpinned dependency detected (job 'build'): .github/workflows/main.yml:37",

That's for:

        uses: devmasx/brakeman-linter-action@v1.0.0

That's to invoke Brakeman (a static analysis tool), and it specifies a version of the linter action. What would you suggest instead?

   "Warn: unpinned dependency detected in dockerfiles/2.5.1-stretch/Dockerfile: 'circleci/ruby:2.5.1-stretch-node-browsers-legacy'",
   "Warn: unpinned dependency detected in dockerfiles/2.6.3-stretch/Dockerfile: 'circleci/ruby:2.6.3-stretch-node-browsers'",
   "Warn: unpinned dependency detected in dockerfiles/2.6.6-stretch/Dockerfile: 'circleci/ruby:2.6.6-stretch-node-browsers'",
   "Warn: unpinned dependency detected in dockerfiles/2.7.2-browsers/Dockerfile: 'cimg/ruby:2.7.2-browsers'",
   "Warn: unpinned dependency detected in dockerfiles/2.7.2-buster/Dockerfile: 'circleci/ruby:2.7.2-buster-node-browsers'",
   "Warn: unpinned dependency detected in dockerfiles/3.0.0-browsers/Dockerfile: 'cimg/ruby:3.0.0-browsers'",
   "Warn: unpinned dependency detected in dockerfiles/3.0.1-browsers/Dockerfile: 'cimg/ruby:3.0.1-browsers'",

This is weird. These Dockerfiles are used to generate the container images, and those containers are frozen. Indeed, only the image generated from the 3.0.1-browsers directory is in use, via .circleci/config.yml. That image is actually in my personal space (that should probably get moved to OpenSSF at some point), and that would be reasonable to complain about. But since the image is created and then frozen as an image, it's odd to complain about it not being frozen.

naveensrinivasan commented 2 years ago

Thanks for the details. Let me see here:

"Warn: unpinned dependency detected (job 'build'): .github/workflows/main.yml:23",

That's for:

uses: actions/checkout@v2`

That's the usual recommended way to checkout projects on GitHub in a GitHub action, and it does specify the major version number. What would be recommended instead?

"Warn: unpinned dependency detected (job 'build'): .github/workflows/main.yml:37",

That's for:

        uses: devmasx/brakeman-linter-action@v1.0.0

That's to invoke Brakeman (a static analysis tool), and it specifies a version of the linter action. What would you suggest instead?

   "Warn: unpinned dependency detected in dockerfiles/2.5.1-stretch/Dockerfile: 'circleci/ruby:2.5.1-stretch-node-browsers-legacy'",
   "Warn: unpinned dependency detected in dockerfiles/2.6.3-stretch/Dockerfile: 'circleci/ruby:2.6.3-stretch-node-browsers'",
   "Warn: unpinned dependency detected in dockerfiles/2.6.6-stretch/Dockerfile: 'circleci/ruby:2.6.6-stretch-node-browsers'",
   "Warn: unpinned dependency detected in dockerfiles/2.7.2-browsers/Dockerfile: 'cimg/ruby:2.7.2-browsers'",
   "Warn: unpinned dependency detected in dockerfiles/2.7.2-buster/Dockerfile: 'circleci/ruby:2.7.2-buster-node-browsers'",
   "Warn: unpinned dependency detected in dockerfiles/3.0.0-browsers/Dockerfile: 'cimg/ruby:3.0.0-browsers'",
   "Warn: unpinned dependency detected in dockerfiles/3.0.1-browsers/Dockerfile: 'cimg/ruby:3.0.1-browsers'",

This is weird. These Dockerfiles are used to generate the container images, and those containers are frozen. Indeed, only the image generated from the 3.0.1-browsers directory is in use, via .circleci/config.yml. That image is actually in my personal space (that should probably get moved to OpenSSF at some point), and that would be reasonable to complain about. But since the image is created and then frozen as an image, it's odd to complain about it not being frozen.

It is recommended to pin dependencies by hash instead of tag, because tags can be moved.

https://github.com/ossf/scorecard/blob/main/docs/checks.md#pinned-dependencies

For Dockerfiles and GitHub workflows, pin dependencies by hash. See example https://github.com/ossf/scorecard/blob/f55b86d6627cc3717e3a0395e03305e81b9a09be/.github/workflows/main.yml#L27 and https://github.com/ossf/scorecard/blob/main/cron/worker/Dockerfile examples.

laurentsimon commented 2 years ago

This seems like the error message/doc is not good enough. There's been another issue with the sample complain. Marking this issue as good first issue since it only requires changing messages. Please reply if you disagree.

Note: we have someone working on giving some points to repos that pin the GitHub-owned actions by version https://github.com/ossf/scorecard/issues/802

nanikjava commented 2 years ago

@laurentsimon happy to work on this as you mentioned it's related to #802

naveensrinivasan commented 2 years ago

The scope of the issue is to give a better error message.

nanikjava commented 2 years ago

The scope of the issue is to give a better error message.

Does this mean that the error message listed in this https://github.com/ossf/scorecard/issues/977#issuecomment-914712055, which can be extracted as

"Warn: unpinned dependency detected (job 'build'): .github/workflows/main.yml:23",

"Warn: unpinned dependency detected in dockerfiles/2.5.1-stretch/Dockerfile: 'circleci/ruby:2.5.1-stretch-node-browsers-legacy'",

"Info: no insecure (unpinned) dependency downloads found in XXXXXX",

the above message need to be changed to a better message ? so what should it says ?

laurentsimon commented 2 years ago
  1. unpinned dependency detected => dependency not pinned by hash instead.
  2. no insecure (unpinned) dependency downloads founds in XXX => no insecure (not pinned by hash) dependency downloads The second message is a little too verbose, feel free to experiment if you find a better one :-)
david-a-wheeler commented 2 years ago

I wasn't seeing the detailed messages at all, because I was looking at metrics.openssf.org's page. I think we need to pull the details into metrics.openssf.org & show those details, e.g., as a tooltip. I've now locally installed scorecard & now I can see the detailed messages. Of course, it's always good to clarify the messages as much as practical...!

nanikjava commented 2 years ago

Thanks all for the clarification.

@laurentsimon please assign this ticket to me. Thank you :)

laurentsimon commented 2 years ago

Assigned. Thank you for your help!

david-a-wheeler commented 2 years ago

FYI: This PR pins dependencies noted: https://github.com/coreinfrastructure/best-practices-badge/pull/1647

It revealed that scorecard doesn't check .circleci configuration files for pinned dependencies. That probably should be added (if you care about pinning), but that should be a different issue.

laurentsimon commented 2 years ago

Great. You went above and beyond .. we currently don't even support circleci. I created https://github.com/ossf/scorecard/issues/994 for tracking

david-a-wheeler commented 2 years ago

@laurentsimon - there's no point in pinning just the GitHub actions when the majority of our stuff happens in CircleCI :-).

laurentsimon commented 2 years ago

I know :-) but we need to surface this in scorecard for you to get more points :-)

nanikjava commented 2 years ago

@laurentsimon PR has been merged can we close this ticket ?

laurentsimon commented 2 years ago

absolutely, thanks fo the work!