-
Notifications
You must be signed in to change notification settings - Fork 498
Add note about third-party pull requests for GitHub Actions #3516
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
base: main
Are you sure you want to change the base?
Conversation
|
@dotnet-policy-service agree |
|
Learn Build status updates of commit a6b4feb: ✅ Validation status: passed
For more details, please refer to the build report. |
PoliCheck Scan ReportThe following report lists PoliCheck issues in PR files. Before you merge the PR, you must fix all severity-1 and severity-2 issues. The AI Review Details column lists suggestions for either removing or replacing the terms. If you find a false positive result, mention it in a PR comment and include this text: #policheck-false-positive. This feedback helps reduce false positives in future scans. ✅ No issues foundMore information about PoliCheckInformation: PoliCheck | Severity Guidance | Term |
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a 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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
a6b4feb to
4309804
Compare
|
Learn Build status updates of commit 4309804: ✅ Validation status: passed
For more details, please refer to the build report. |
PoliCheck Scan ReportThe following report lists PoliCheck issues in PR files. Before you merge the PR, you must fix all severity-1 and severity-2 issues. The AI Review Details column lists suggestions for either removing or replacing the terms. If you find a false positive result, mention it in a PR comment and include this text: #policheck-false-positive. This feedback helps reduce false positives in future scans. ✅ No issues foundMore information about PoliCheckInformation: PoliCheck | Severity Guidance | Term |
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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
4309804 to
42b8389
Compare
PoliCheck Scan ReportThe following report lists PoliCheck issues in PR files. Before you merge the PR, you must fix all severity-1 and severity-2 issues. The AI Review Details column lists suggestions for either removing or replacing the terms. If you find a false positive result, mention it in a PR comment and include this text: #policheck-false-positive. This feedback helps reduce false positives in future scans. ✅ No issues foundMore information about PoliCheckInformation: PoliCheck | Severity Guidance | Term |
|
Learn Build status updates of commit 42b8389: ✅ Validation status: passed
For more details, please refer to the build report. |
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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
|
Hello @eXpl0it3r, thanks for taking the time to work on the documentation. There are actually two scenarios that I think are being conflated here. I looked back at the DNF Discord messages and found the one you reacted to. When I wrote that I was assuming the other person was talking about a fork running a GitHub Actions workflow in their own repository (a fork). There is another that is worth distinguishing.
Note that I hope this explains the situation. If you'd like to move forward with this PR, maybe this nuance can be expanded on in your diff? |
|
Thank you for looking at this PR and clarifying your statement, @joelverhagen! Happy to update this! Do you have any doc links regarding the "However, the pull_request event will fail to produce an OIDC token due to protections in the GitHub Actions platform." statement? Is this documented somewhere? |
No, I was unable to find a doc. However, I have tested it and it is referenced on https://docs.pypi.org/trusted-publishers/security-model/. When I tested it, they implemented the block by not setting |
I think it's important to communicate to the user, that pull requests by strangers won't be able to circumvent the trust publishing policy.
Without such a hint, it's unclear whether an "approved" user (someone who has contributed before and won't need a manual approval for the GitHub Actions run), would be able to create a PR that changes the GitHub Actions step in such a way, that malicious code is released as new package, because the trust publishing policy is set up for the given repository.
Feel free to make suggestions on the words or change the wording directly.
Pinging @joelverhagen, given that I have more or less used his wording from the .NET Foundation Discord server.