-
Notifications
You must be signed in to change notification settings - Fork 928
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
Use isolated self-hosted runners non-PR CI jobs #5402
Conversation
Motivation: An attack may send an PR to try to install an improper code on self-hosted runners after sending a normal PR. GitHub Actions only allows non-forked worflows to access secrets. If we isolate self-hosted runners for non-forked workflows, the vulunerable code in the attacker's PR can't acquire sensitive information. Modifications: - Split `self-hosted` runners into `self-hosted-for-pr` and `self-hosted-for-non-pr`. - `self-hosted-for-non-pr` is run with GitHub Actions secrets. - `self-hosted-for-pr` is used for CI buids of pull requests. Result: Respond to possible vulnerabilities by using self-hosted. Note: `self-hosted` is no longer used for GitHub Actions workflows. A new job that needs to run on self-hosted runners should choose `self-hosted-for-non-pr` or `self-hosted-for-pr` explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I don't fully understand this but let me ask you tomorrow. 😉
When a non-maintainer changes workflow files, the workflows will be rejected �from execution: https://github.com/line/armeria/actions/runs/7610334781/job/20723579232?pr=5402#step:2:5 |
This PR is now ready to review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some minor questions but looks good! Thanks @ikhoon 🙇 👍 🙇
PR_NUMBER=$(echo "$GITHUB_REF" | awk -F / '{print $3}') | ||
# To obtain sufficient quota for gh cli, it is recommended to set PAT in the `GH_TOKEN` environment variable. | ||
# Check if there are any changes in .github/actions or .github/workflows | ||
WORKFLOW_CHANGES=$(gh -R line/armeria pr diff "$PR_NUMBER" --name-only | grep -c '^.github/workflows\|^.github/actions') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question) Is it possible for external contributor change this script itself (which isn't in workflows
, actions
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file is not directly accessed by GitHub Actions. It was added for version control. If there are any changes to this file, we must manually update the file in self-hosted runners.
Thanks for the quick review. ❤️ |
Motivation:
An attack may send an PR to try to install an improper code on self-hosted runners after sending a normal PR.
GitHub Actions only allows non-forked workflows to access secrets. If we isolate self-hosted runners for non-forked workflows, the vulnerable code in the attacker's PR can't acquire sensitive information.
Modifications:
self-hosted
runners intoself-hosted-for-pr
andself-hosted-for-non-pr
.self-hosted-for-non-pr
is run with GitHub Actions secrets.self-hosted-for-pr
is used for CI builds of pull requests.check-workflow-push-permission.sh
is not directly used in GitHub workflow but is triggered by the GitHub Actions runner before starting a jobResult:
Respond to possible vulnerabilities by using self-hosted.