[WIP] Add an external contributor approval mechanism of Pull Requests - #1556
[WIP] Add an external contributor approval mechanism of Pull Requests#1556olamy wants to merge 5 commits into
Conversation
f88dffe to
6744880
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
| </dependency> | ||
| <dependency> | ||
| <groupId>org.jenkins-ci.plugins</groupId> | ||
| <artifactId>branch-api</artifactId> |
There was a problem hiding this comment.
Architecturally this is wrong—branch sources do not depend on project specifics.
Now I recall that I actually started work on this problem a while back but review stalled in discussions about security implications: jenkinsci/scm-api-plugin#180
There was a problem hiding this comment.
Correction: a related problem, about PRs specifically changing Pipeline script, which is riskier. Perhaps there should be a unified DX allowing maintainers to permit or deny builds from forks based on whether or not Pipeline script changed; approval of a given commit vs. any commit to that PR; whitelisting of a given author. In the case of GH specifically, there are as I recall several modes available to GHA though I doubt a Jenkins controller could inspect those settings without elevated permissions.
There was a problem hiding this comment.
Yes, this layering approach is perfectly right.
BUT I would prefer the feature proposed here (it's actually not a feature, but get back on track with what GHA has been proposing for years) to not depend on another change that has been stalled for 3 years.
The idea here is to prevent the execution of any code without approval. (Jenkinsfile or not)
But rather than focusing on the "ideal" technical approach, we could focus on delivering features users have been expecting for years now.
Anyway, I will try a separate plugin approach because my goal here is to have happy users enjoying some interesting features.
There was a problem hiding this comment.
But frankly, such a feature should be a natural part of this plugin; users should not have to install anything else.
That's definitely not a user-friendly approach (IMO)
Signed-off-by: Olivier Lamy <olamy@apache.org>
| * The job starts out disabled and only builds once an administrator has approved it. Pull | ||
| * requests from a trusted login, or carrying a trusted label, are approved for you. | ||
| */ | ||
| public static class TrustExternalApproval extends GitHubForkTrustPolicy { |
There was a problem hiding this comment.
No existing code is being changed here AFAICT. So this could be released as a separate plugin for experimental purposes.
3 participants
Description
Adds a new fork PR trust policy: External approval required.
Right now, trusting fork PRs is all-or-nothing (nobody, everybody, contributors,
or write access). This adds a middle ground: fork PRs are discovered as usual, but
each one waits for a person to approve it before it builds. Handy for public repos
where you want CI on fork PRs, but only after someone has taken a look.
How it works
won't build on its own.
Configurepermission approves it from the job's PendingApproval page (or reverts it). Approving enables the job and starts a build.
The approval is saved next to the job, so it sticks across restarts.
Options
approved. If new commits are pushed, the PR goes back to Pending.
automatically. Matched ignoring case, like GitHub does.
automatically. Checked once, when the PR is first seen.
Screenshots
Configuration
Blocked PR
Run approval
Trying it out
set to External approval required.
it goes back to Pending.
without asking.
Submitter checklist
Reviewer checklist
Documentation changes
Users/aliases to notify