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

Pinned-Dependencies incorrectly flags a pinned dependency downloaded from GitHub #3339

Closed
martincostello opened this issue Aug 2, 2023 · 2 comments · Fixed by #3694
Closed

Comments

@martincostello
Copy link
Contributor

Is your feature request related to a problem? Please describe.

I have a GitHub Actions workflow that uses rhysd/actionlint to lint GitHub Actions workflow files. As per its own documentation it is used by running a bash script to download and install the tool.

I however changed the value of main to the SHA so that it is pinned:

- name: Lint workflows
  shell: bash
  env:
    ACTIONLINT_VERSION: '7b75d16d41920ec126e6f3269db0c6f3ab613c38' # v1.6.25
  run: |
    echo "::add-matcher::.github/actionlint-matcher.json"
    bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/${ACTIONLINT_VERSION}/scripts/download-actionlint.bash")
    ./actionlint -color

This was then flagged as an unpinned dependency as the code doesn't walk the whole step to find the referenced environment variable is a SHA to pin the download. It would be nice if it did, but it's understandable that it doesn't support that.

I then refactored to remove the environment variable like so:

- name: Lint workflows
  shell: bash
  run: |
    echo "::add-matcher::.github/actionlint-matcher.json"
    bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/7b75d16d41920ec126e6f3269db0c6f3ab613c38/scripts/download-actionlint.bash")
    ./actionlint -color

This however still flags the warning:

Reason

dependency not pinned by hash detected -- score normalized to 6

Details

Warn: downloadThenRun not pinned by hash: .github/workflows/lint.yml:37
Info: GitHub-owned GitHubActions are pinned
Info: Third-party GitHubActions are pinned
Info: Dockerfile dependencies are pinned
Info: Pip installs are pinned

Describe the solution you'd like

The Pinned-Dependencies rule is able to correctly determine that URLs of this format to raw GitHub content in repositories are pinned.

Describe alternatives you've considered

None.

Additional context

None.

@github-actions
Copy link

This issue is stale because it has been open for 60 days with no activity.

@martincostello
Copy link
Contributor Author

I'm going to have a quick look to see if I can fix this myself.

spencerschrock pushed a commit that referenced this issue Nov 30, 2023
* Trust pinned GitHub download URLs

 Trust files that are downloaded from `raw.githubusercontent.com` where the file's ref is a Git SHA and therefore immutable.
Resolves #3339.
Signed-off-by: martincostello <[email protected]>

* Move logic to function

- Add `hasUnpinnedURLs` function.
- Add test cases for different URLs.
Signed-off-by: martincostello <[email protected]>

* Fix formatting

Appease the linter.
Signed-off-by: martincostello <[email protected]>

* Suppress lint warnings

Suppress warning on three long URLs.
Signed-off-by: martincostello <[email protected]>

* Address peer review

Address peer review feedback.
Signed-off-by: martincostello <[email protected]>

* Fix lint warning

Fix lint warning.
Signed-off-by: martincostello <[email protected]>
ashearin pushed a commit to kgangerlm/scorecard-gitlab that referenced this issue Dec 4, 2023
* Trust pinned GitHub download URLs

 Trust files that are downloaded from `raw.githubusercontent.com` where the file's ref is a Git SHA and therefore immutable.
Resolves ossf#3339.
Signed-off-by: martincostello <[email protected]>

* Move logic to function

- Add `hasUnpinnedURLs` function.
- Add test cases for different URLs.
Signed-off-by: martincostello <[email protected]>

* Fix formatting

Appease the linter.
Signed-off-by: martincostello <[email protected]>

* Suppress lint warnings

Suppress warning on three long URLs.
Signed-off-by: martincostello <[email protected]>

* Address peer review

Address peer review feedback.
Signed-off-by: martincostello <[email protected]>

* Fix lint warning

Fix lint warning.
Signed-off-by: martincostello <[email protected]>
Signed-off-by: Allen Shearin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants