Fork Deployment Safety 🔒 #331
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Copilot reviewed 9 out of 9 changed files in this pull request and generated no suggestions.
Comments skipped due to low confidence (1)
src/functions/prechecks.js:81
- The comment should be more descriptive to explain why the
forkBypassis set to true. Suggestion: 'Setting forkBypass to true because the stable branch is being used as the deployment target, even though the command is executed on a fork.'
forkBypass = true // even though this command is taking place on a fork, the stable branch is the one being used as the deployment target
Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fork Deployment Safety
This pull request is a continuation of safety measures being applied to this Action to make sure it is as safe and reliable as possible!
Forks are the lifeblood of the open source ecosystem. However, they should be treated as untrusted by default until they can be reviewed and approved. If you have a project using this Action in an open source nature, you might receive a pull request originating from a fork. Using the default setup of this Action, it respects your branch protection settings if you were to
.deploythat PR fork. This means that if you don't require any reviews, you will deploy that fork. This is inherently risky and this PR helps to protect us a bit more when deploying PR forks in the wild (if even applicable).If your branch protection settings (or rulesets) require that a PR must have an approval then this Action will respect that and reject deployments that don't meet that criteria. However, there are a few edge cases where you could bypass those protections:
skip_reviewson an environment, then you can also skip reviewing a PR or a PR fork before it can be deployedBoth of these avenues will no longer exist after this PR merges. Forks will be treated with highest level of restrictions when being deployed by this Action. Admins will now be required to comply with required reviews (on deploys of forks) and
skip_reviewswill no longer apply to forks (meaning that you cannot skip reviews on forks - they are now mandatory). Both.deployand.noopdeployments will be impacted by this change. In the past,.noopdeployments did not require a review at all, but now they do (for forks). These changes will not effect normal PRs that don't originate from forks.