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

feat: Split validations to required (stopping release) and optional (not stopping the release) #101

Merged
merged 5 commits into from
Jul 23, 2024

Conversation

andreaangiolillo
Copy link
Collaborator

@andreaangiolillo andreaangiolillo commented Jul 23, 2024

Proposed changes

This PR splits the validations into two required validations, which will stop the release and file an GH issue in the repo, and optional validation which will not stop the release but only file an issue in the repo.

I tested my changes in a fork: https://github.com/andreaangiolillo/openapi-test/actions/runs/10060430357
Screenshot 2024-07-23 at 15 10 31

Example of an issue created: andreaangiolillo#1

@andreaangiolillo andreaangiolillo marked this pull request as ready for review July 23, 2024 14:24
@andreaangiolillo andreaangiolillo requested a review from a team as a code owner July 23, 2024 14:24
Copy link
Collaborator

@matt-condon matt-condon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM very nice PR 🫶

runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(interest piqued) do we normally pin these actions versions?
Any particular reason?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The security team recommended using a specific SHA version for actions to prevent the risk of using a maliciously modified action. For GitHub actions like actions/checkout, this isn’t necessary because they are considered secure. However, I copied and pasted from another action xD

Based on The GitHub Actions Worm: Compromising GitHub Repositories Through the Actions Dependency Tree (paloaltonetworks.com)

How to Protect Your Workflows and Assets
Multiple security controls can prevent or raise the difficulty of successfully attacking repositories using the worm. In order of effectiveness, controls include:

  • Set GITHUB_TOKEN and PAT contents permission to the minimum required — with special attention to reducing write permissions against the repository — to prevent infection by the worm. Consider using GitHub’s actions-permissions project to reduce workflow permissions. In general, implement strict PBAC (Pipeline-Based Access Controls) to make sure the workflow is granted with the least privileges and access it needs to fulfill its purpose.
  • Configure branch and tag protection to further prevent infection and protect the codebase.
  • Monitor and limit outbound network connections from workflow runners to prevent the download of malicious code into pipelines and prevent malware from reporting to C2 servers.
  • Pin actions using a commit hash to reduce the risk of using a maliciously modified action.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!
@Luke-Sanderson could you double-check to make sure any actions introduced for Postman have pinned versions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was using the version tag rather than commit hash. I assume this has the same security implications? I chose to use the version tag because that was used in the Go-SDK

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use the secrets exfiltrated in the flow to infect the repository with malicious code. Overwriting a commit while keeping its hash the same isn’t possible, so we can’t abuse a commit hash reference. We still have two options:

  • Infecting by pushing code to a branch: We can use the GITHUB_TOKEN or PAT used in a job to push malicious code to a branch if the token is granted with the contents:write permission.
  • Infecting by creating a tag: We can use the GITHUB_TOKEN or PAT used in a job to create a malicious tag or override an existing tag to infect dependent repositories referencing the action by a tag.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay makes sense. I have only used actions/checkout and actions/cache so far but I'll keep that in mind if I use any external actions in the future.

@andreaangiolillo andreaangiolillo merged commit 84b73d9 into main Jul 23, 2024
6 checks passed
@andreaangiolillo andreaangiolillo deleted the feat_test_postman_foas_follow_up branch July 23, 2024 14:33
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