-
Notifications
You must be signed in to change notification settings - Fork 7
Read config and .codeowners from base ref instead of head #76
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
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Summary of ChangesHello @BakerNet, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the security posture of recommended GitHub Actions workflows by modifying the Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Codeowners approval required for this PR: |
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.
Code Review
This pull request updates the documentation to recommend checking out github.base_ref in the example GitHub Actions workflow. The goal is to enhance security by preventing pull request authors from modifying .codeowners files to bypass reviews. While this is a positive change, I've pointed out in a comment that it doesn't fully mitigate the security risk, as the workflow file itself can still be modified by the PR author when using the pull_request trigger. For a more secure setup, I've recommended using the pull_request_target trigger instead.
README.md
Outdated
| - name: 'Checkout Code Repository' | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ github.base_ref }} # use the base_ref to prevent bypass by PR author |
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.
While using ref: ${{ github.base_ref }} is a good step to prevent using potentially malicious .codeowners files from the pull request branch, this setup is still vulnerable. Because the workflow is triggered by pull_request, the workflow file itself is sourced from the pull request branch. A malicious actor could simply remove this ref line in their PR to bypass the protection.
For robust security, you should use the pull_request_target trigger instead of pull_request. Workflows triggered by pull_request_target run using the workflow file from the base branch, which prevents malicious modifications to the workflow itself. When using pull_request_target, actions/checkout defaults to checking out the base ref, making your action secure against this type of bypass.
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.
Using pull_request_target is NOT the right answer here. But the issue is worth paying attention to. Will switch to base ref in Codeowners Plus itself
Related Issue(s)
closes: #74
Summary / Background
Update docs to use
base_refforactions/checkoutto prevent PR authors from removing required reviews in their PR