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.

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

BUG: Token Permissions Check suggests fix files that are actually right #2482

Closed
joycebrum opened this issue Nov 23, 2022 · 3 comments · Fixed by #2486
Closed

BUG: Token Permissions Check suggests fix files that are actually right #2482

joycebrum opened this issue Nov 23, 2022 · 3 comments · Fixed by #2486
Labels
kind/bug Something isn't working

Comments

@joycebrum
Copy link
Contributor

joycebrum commented Nov 23, 2022

Describe the bug

In the example bellow from the SARIF JSON, the Token Permission Score is actually 9 but the details mentions only files with permission set to read and all of them with a "update your workflow using <app.stepsecurity link>", but there is nothing to be fixed since the

    {
      "name": "Token-Permissions",
      "score": 9,
      "reason": "non read-only tokens detected in GitHub workflows",
      "details": [
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/build_test.yml:16: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/build_test.yml/main?enable=permissions",
        "Info: topLevel permissions set to 'read-all': .github/workflows/cflite_pr.yml:12: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/cflite_pr.yml/main?enable=permissions",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/cifuzz.yml:9: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/cifuzz.yml/main?enable=permissions",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/codeql.yml:24: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/codeql.yml/main?enable=permissions",
        "Info: jobLevel 'actions' permission set to 'read': .github/workflows/codeql.yml:34: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/codeql.yml/main?enable=permissions",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/coverity.yml:13: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/coverity.yml/main?enable=permissions",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/development_freeze.yml:11: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/development_freeze.yml/main?enable=permissions",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/differential-shellcheck.yml:11: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/differential-shellcheck.yml/main?enable=permissions",
        "Warn: jobLevel 'security-events' permission set to 'write': .github/workflows/differential-shellcheck.yml:19: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/differential-shellcheck.yml/main?enable=permissions",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/issue_labeler.yml:9: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/issue_labeler.yml/main?enable=permissions",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/labeler.yml:11: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/labeler.yml/main?enable=permissions",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/linter.yml:14: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/linter.yml/main?enable=permissions",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/mkosi.yml:45: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/mkosi.yml/main?enable=permissions",
        "Info: topLevel permissions set to 'read-all': .github/workflows/scorecards.yml:20: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/scorecards.yml/main?enable=permissions",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/unit_tests.yml:13: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/unit_tests.yml/main?enable=permissions"
      ],
      "documentation": {
        "short": "Determines if the project's workflows follow the principle of least privilege.",
        "url": "https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#token-permissions"
      }
    },

Reproduction steps
Steps to reproduce the behavior:

  1. run scorecard --repo=https://github.com/systemd/systemd --show-details

Expected behavior
I though on two possible behavior to this output

The first one is less verbose on the details and does not explicitly mention all the files verified, just the ones that are not complied with Token-Permissions

    {
      "name": "Token-Permissions",
      "score": 9,
      "reason": "non read-only tokens detected in GitHub workflows",
      "details": [
        "Warn: jobLevel 'security-events' permission set to 'write': .github/workflows/differential-shellcheck.yml:19: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/differential-shellcheck.yml/main?enable=permissions",
      ],
      "documentation": {
        "short": "Determines if the project's workflows follow the principle of least privilege.",
        "url": "https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#token-permissions"
      }
    },

The second one still mention them (for log or just info purpose) but remove the wrong "fix it" part, since it is not a fixable info

 {
      "name": "Token-Permissions",
      "score": 9,
      "reason": "non read-only tokens detected in GitHub workflows",
      "details": [
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/build_test.yml:16",
        "Info: topLevel permissions set to 'read-all': .github/workflows/cflite_pr.yml:12",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/cifuzz.yml:9",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/codeql.yml:24",
        "Info: jobLevel 'actions' permission set to 'read': .github/workflows/codeql.yml:34",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/coverity.yml:13",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/development_freeze.yml:11",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/differential-shellcheck.yml:11",
        "Warn: jobLevel 'security-events' permission set to 'write': .github/workflows/differential-shellcheck.yml:19: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/differential-shellcheck.yml/main?enable=permissions",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/issue_labeler.yml:9",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/labeler.yml:11",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/linter.yml:14",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/mkosi.yml:45",
        "Info: topLevel permissions set to 'read-all': .github/workflows/scorecards.yml:20",
        "Info: topLevel 'contents' permission set to 'read': .github/workflows/unit_tests.yml:13"
      ],
      "documentation": {
        "short": "Determines if the project's workflows follow the principle of least privilege.",
        "url": "https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#token-permissions"
      }
    },

Additional info
Valuable Feedback from discussion at ossf/scorecard-action#1017

@joycebrum joycebrum added the kind/bug Something isn't working label Nov 23, 2022
@laurentsimon
Copy link
Contributor

Good catch. The remediation should not show for read token. The second option is the expected one.

@laurentsimon
Copy link
Contributor

Sent #2486
Thanks!

@varunsh-coder
Copy link
Contributor

Hi @laurentsimon, there is a related issue here: security-events and other scopes should not ding the score if used at the job level. I proposed this in #2338. This was fixed for several scopes in the PRs related to that issue, but not all of the scopes were addressed.

I will check the code and confirm which scopes are not yet fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants