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

Security fixes in GitHub actions for Unpinned Tags and Workflow yml that doesn't specifically define perms #1013

Merged
merged 18 commits into from
Feb 18, 2025

Conversation

darrenge
Copy link
Contributor

@darrenge darrenge commented Feb 12, 2025

This PR contains fixes for two separate security issues found here: /~https://github.com/microsoft/garnet/security/code-scanning

  1. Unpinned tag for a non-immutable Action in workflow -- Using a tag for a 3rd party Action that is not pinned to a commit can lead to executing an untrusted Action through a supply chain attack. To fix this, you tag a commit SHA to the 3rd party action call.
    For example: "uses: benchmark-action/github-action-benchmark@v1" becomes "uses: benchmark-action/github-action-benchmark@d48d326"

Each commit SHA was selected from the releases folder of each 3rd party tool. All of these were tested EXCEPT for line 74 of docker-linux.yml because that is the final build \ push step.

One of the tasks (Create PR task) in Helm needed fixed, but there was a security policy change that was preventing this task from even working, so I removed that task which also removed the initial security issue.

  1. Workflow does not contain permissions -- If a GitHub Actions job or workflow has no explicit permissions set, then the repository permissions are used. Repositories created under organizations inherit the organization permissions. Often these permissions do not adhere to the principle of least privilege and can be reduced to read-only,

This is pretty simple fix where you just add this part to yml files that were missing it
permissions:
contents: read

@darrenge darrenge requested a review from badrishc February 12, 2025 01:02
…t any more. There was a fix for the Unpinned Tag in that section, so remove the task will also remove that security issue.
…elete whole thing. It does still have the security fix in it.
@darrenge darrenge marked this pull request as ready for review February 18, 2025 17:58
@TalZaccai TalZaccai merged commit 133c5b1 into main Feb 18, 2025
15 checks passed
@TalZaccai TalZaccai deleted the darrenge/FixIssuesFromCodeScan branch February 18, 2025 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants