-
-
Notifications
You must be signed in to change notification settings - Fork 369
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
Unable to deploy PRs #3559
Comments
We disabled PR deployments due to security concerns, because it would allow you to access |
Deploy uses secrets enabled for the deploy event. Clicking the deloy button can be done by every repo member (not by externals), right? |
Yes. So you can access any secret if you have push perms but they should only be available with admin perms. |
I might be wrong, but aren't just secrets with the deploy filter exposed to the compiler? |
I'm not sure about this, but how is this important? The workflow to get a secret that's only intended for deployments would be:
|
As clicking the deploy button can only be done by a repo member. Isn't it the same risk as this:
|
In general, yes, however, you can easily block that using branch protections. Blocking PRs is much harder and probably nobody will do it as it drastically reduces usability. Maybe it's the best idea to be able to disable the deploy button completely, but if it's enabled allow all events? And add a warning similar to PR secrets? |
IMO, we should have a config to enable or disable deployment on PRs. We can keep it disabled by default. However, for private environments the risk is minimal. |
…but if they're enabled, allow for all events. Also add warning that you should only enable it if you trust the users with push access. closes #3559 --------- Co-authored-by: Robert Kaussow <xoxys@rknet.org>
Component
server, web-ui
Describe the bug
I noticed that PRs can no longer be deployed using the web UI due to this change #3522
However this has broken our UX where our team deploys the PRs in dev/alpha environments to carry out testing before the PR is merged. Please suggest.
System Info
Additional context
No response
Validations
next
version already [https://woodpecker-ci.org/faq#which-version-of-woodpecker-should-i-use]The text was updated successfully, but these errors were encountered: