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

Use isolated self-hosted runners non-PR CI jobs #5402

Merged
merged 20 commits into from
Jan 23, 2024

Conversation

ikhoon
Copy link
Contributor

@ikhoon ikhoon commented Jan 22, 2024

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:

  • 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 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 job

Result:

Respond to possible vulnerabilities by using self-hosted.

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.
Copy link
Contributor

@minwoox minwoox left a 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. 😉

@ikhoon
Copy link
Contributor Author

ikhoon commented Jan 22, 2024

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
When a maintainer changes workflow files, the workflows are allowed to run: https://github.com/line/armeria/actions/runs/7610692738/job/20724558885?pr=5402#step:2:5

@ikhoon
Copy link
Contributor Author

ikhoon commented Jan 22, 2024

This PR is now ready to review.

Copy link
Contributor

@jrhee17 jrhee17 left a 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')
Copy link
Contributor

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)

Copy link
Contributor Author

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.

.github/workflows/publish-release.yml Outdated Show resolved Hide resolved
@ikhoon
Copy link
Contributor Author

ikhoon commented Jan 23, 2024

Thanks for the quick review. ❤️

@ikhoon ikhoon merged commit 524ebd8 into line:main Jan 23, 2024
14 checks passed
@ikhoon ikhoon deleted the isolated-github-actions branch June 28, 2024 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants