[JENKINS-40652] origin pr builds not treated as trusted - fix for regression in org repos origin PRs

[mslusarczyk]

JENKINS-40652 is still broken, steps to replicate:

• setup multibranch job on an org repo (owner 'myOrg') using valid scan credentials for myOrg
• create PR from origin
• observe that PR is not trusted during the scan

Faulty line is here. When scan credentials are valid collaboratorNames is set to actual collaborators and it does not contain 'myOrg'

[ath88]

I am experiencing the same thing. 👍

[ubante]

I'm also experiencing this problem and am manually starting builds. So looking forward to this merge and the resulting version.

[jonathancolby-olx]

also experiencing this problem. Jenkins ver. 2.46.3

PR events from forks trigger builds correctly, but not from origin. We get the "not from a trusted source" error for PRs from origin branches.

[ButkiewiczP]

Has there been any movement on getting this reviewed and merged in? We're currently experiencing this as a company and I'd rather not start adding custom built plugins to our Jenkins instance.

[jglick]

I think both this and #109 should be closed in favor of something like #96, which uses the official API now that one exists, rather than the flaky tricks originally necessary. But first @stephenc was going to refactor the UI so that an administrator can select the most appropriate algorithm for a given organization folder. Until that refactoring lands there is little point in new PRs in this repository.

[ButkiewiczP]

@jglick Is there a timeline for #96 ? or for JENKINS-43426? Seems they've been open for a while. My understanding is that without this fix, Github Organization folders are almost completely useless as they can't build any PRs.

[jglick]

All changes here are pending big PRs coming shortly from @stephenc.

[stephenc]

Should be fixed by JENKINS-43507 changes or in the worst case by the changes in 2.2.2 of the GHBS plugin

[nicbou]

Hi @stephenc, I saw your name a lot in the past hour, so may I ask you what the status of this issue is? I can't figure out which ticket/PR is telling the truth. Some were merged in January, some are rejected, some are in progress and some others are waiting for other PRs.

Is there any chance a layman can understand when this feature will work?

[stephenc]

@nicbou it all depends on what you mean by work...

• this PR was to just include the repo owner in the list of collaborators... which should now be no longer needed as a PR from the repo owner is an origin PR and origin PRs are always trusted... so in that sense one aspect of this issue was fixed by JENKINS-43507 and should Just Work™ with 2.2.0 or newer

• there are other people who are scanning with a user / token that doesn't have permission to list the collaborators... those people will probably still have issues until scanning is done with a credential that has the required permission

• finally there are issues with the whole "let's check the collaborators" approach, so for that we have added a new trust strategy... switching to that will use the permissions API in github and check if the user has admin or write permission... and seemingly that permissions API does not need as much permissions as querying the list of collaborators.
  
  Upgrading to GHBS 2.2.2 will let you switch like in the above

[nicbou]

Thanks a lot for the detailed answer. We are shamefully lagging behind in terms of Jenkins updates, and I didn't want to change anything before understanding what fixes what.

In my case, an origin PR from an admin (me) failed, even when I also added myself as a contributor. I am not the owner, but I have full access to the repo as a member of my organization.

If I understand correctly,

• This PR does not solve my issue, as I am not a owner, but already a collaborator
• The user/token does not solve my issue, as the credentials are those of another user with permissions equivalent to mine (admin rights on repo)
• The last one could fix the issue. Trusting everyone should not be a problem, since those who can access the repository are all within slapping distance.

Again, thanks for the detailed answer!