Skip to content

[JENKINS-66661] Fix fallback behavior of Fork PR trust criteria #711

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,23 @@ protected final void retrieve(
request.setPermissionsSource(new GitHubPermissionsSource() {
@Override
public GHPermissionType fetch(String username) throws IOException, InterruptedException {
return ghRepository.getPermission(username);
try {
return ghRepository.getPermission(username);
} catch (FileNotFoundException e) {
listener.getLogger()
.println(" Not permitted to query list of permissions, assuming none");
return GHPermissionType.NONE;
} catch (HttpException e) {
if (e.getResponseCode() == HttpServletResponse.SC_UNAUTHORIZED
|| e.getResponseCode() == HttpServletResponse.SC_FORBIDDEN
|| e.getResponseCode() == HttpServletResponse.SC_NOT_FOUND) {
listener.getLogger()
.println(" Not permitted to query list of permissions, assuming none");
return GHPermissionType.NONE;
} else {
throw new WrappedException(e);
}
}
}
});

Expand Down Expand Up @@ -1580,6 +1596,7 @@ private Set<String> updateCollaboratorNames(
return collaboratorNames = Collections.emptySet();
} catch (HttpException e) {
if (e.getResponseCode() == HttpServletResponse.SC_UNAUTHORIZED
|| e.getResponseCode() == HttpServletResponse.SC_FORBIDDEN
|| e.getResponseCode() == HttpServletResponse.SC_NOT_FOUND) {
listener.getLogger().println("Not permitted to query list of collaborators, assuming none");
return collaboratorNames = Collections.emptySet();
Expand Down Expand Up @@ -1866,8 +1883,7 @@ public SCMRevision getTrustedRevision(SCMRevision revision, final TaskListener l
try {
wrapped.unwrap();
} catch (HttpException e) {
listener.getLogger()
.format("It seems %s is unreachable, assuming no trusted collaborators%n", apiUri);
listener.getLogger().format("It seems %s is unreachable, assuming not trusted%n", apiUri);
collaboratorNames = Collections.singleton(repoOwner);
}
}
Expand Down Expand Up @@ -2928,7 +2944,21 @@ public GHPermissionType fetch(String username) throws IOException, InterruptedEx
String fullName = repoOwner + "/" + repository;
repo = github.getRepository(fullName);
}
return repo.getPermission(username);
try {
return repo.getPermission(username);
} catch (FileNotFoundException e) {
listener.getLogger().println("Not permitted to query list of permissions, assuming none");
return GHPermissionType.NONE;
} catch (HttpException e) {
if (e.getResponseCode() == HttpServletResponse.SC_UNAUTHORIZED
|| e.getResponseCode() == HttpServletResponse.SC_FORBIDDEN
|| e.getResponseCode() == HttpServletResponse.SC_NOT_FOUND) {
listener.getLogger().println("Not permitted to query list of permissions, assuming none");
return GHPermissionType.NONE;
} else {
throw new WrappedException(e);
}
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import java.util.*;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.servlet.http.HttpServletResponse;
import jenkins.branch.BranchSource;
import jenkins.model.Jenkins;
import jenkins.plugins.git.GitSCMSource;
Expand Down Expand Up @@ -716,6 +717,227 @@ public void getTrustedRevisionReturnsRevisionIfRepoOwnerAndPullRequestBranchOwne
sameInstance(revision));
}

@Test
public void fetchForkPRFromCollaborators() throws IOException, InterruptedException {
source.setTraits(List.of(new ForkPullRequestDiscoveryTrait(
EnumSet.of(ChangeRequestCheckoutStrategy.MERGE),
new ForkPullRequestDiscoveryTrait.TrustContributors())));
githubApi.stubFor(get(urlMatching("(/api/v3)?/repos/cloudbeers/yolo/collaborators"))
.inScenario("PR from Fork not Collaborators")
.whenScenarioStateIs(Scenario.STARTED)
.willReturn(aResponse()
.withHeader("Content-Type", "application/json; charset=utf-8")
.withBodyFile("body-cloudbeers-yolo-collaborators.json")));

SCMHeadObserver.Collector collector = SCMHeadObserver.collect();
source.fetch(
(SCMSourceCriteria) (probe, listener) -> probe.stat("README.md").getType() == SCMFile.Type.REGULAR_FILE,
collector,
null,
null);

Map<String, SCMHead> byName = new HashMap<>();
Map<String, SCMRevision> revByName = new HashMap<>();
for (Map.Entry<SCMHead, SCMRevision> h : collector.result().entrySet()) {
byName.put(h.getKey().getName(), h.getKey());
revByName.put(h.getKey().getName(), h.getValue());
}

assertThat(byName.keySet(), containsInAnyOrder("PR-2", "PR-3", "PR-4"));

// Source owner is a collaborator, the trusted revision must be the PR head / merge
for (SCMRevision revision : revByName.values()) {
assertThat(
source.getTrustedRevision(revision, new LogTaskListener(Logger.getAnonymousLogger(), Level.INFO)),
is(revision));
}
}

@Test
public void fetchForkPRCannotFindCollaborators() throws IOException, InterruptedException {
source.setTraits(List.of(new ForkPullRequestDiscoveryTrait(
EnumSet.of(ChangeRequestCheckoutStrategy.MERGE),
new ForkPullRequestDiscoveryTrait.TrustContributors())));
for (Integer errorCode : new int[] {
HttpServletResponse.SC_NOT_FOUND, HttpServletResponse.SC_UNAUTHORIZED, HttpServletResponse.SC_FORBIDDEN
}) {
githubApi.stubFor(get(urlMatching("(/api/v3)?/repos/cloudbeers/yolo/collaborators"))
.inScenario("PR from Fork " + errorCode + " Collaborators")
.whenScenarioStateIs(Scenario.STARTED)
.willReturn(aResponse().withStatus(errorCode)));

SCMHeadObserver.Collector collector = SCMHeadObserver.collect();
source.fetch(
(SCMSourceCriteria)
(probe, listener) -> probe.stat("README.md").getType() == SCMFile.Type.REGULAR_FILE,
collector,
null,
null);

Map<String, SCMHead> byName = new HashMap<>();
Map<String, SCMRevision> revByName = new HashMap<>();
for (Map.Entry<SCMHead, SCMRevision> h : collector.result().entrySet()) {
byName.put(h.getKey().getName(), h.getKey());
revByName.put(h.getKey().getName(), h.getValue());
}

assertThat(byName.keySet(), containsInAnyOrder("PR-2", "PR-3", "PR-4"));

// Cannot find collaborators, the trusted revision must be the base (target) revision, not the head
for (SCMRevision revision : revByName.values()) {
assertThat(
source.getTrustedRevision(
revision, new LogTaskListener(Logger.getAnonymousLogger(), Level.INFO)),
is(((PullRequestSCMRevision) revision).getTarget()));
}
}
}

@Test
public void fetchForkPRFromNonCollaborators() throws IOException, InterruptedException {
source.setTraits(List.of(new ForkPullRequestDiscoveryTrait(
EnumSet.of(ChangeRequestCheckoutStrategy.MERGE),
new ForkPullRequestDiscoveryTrait.TrustContributors())));
githubApi.stubFor(get(urlMatching("(/api/v3)?/repos/cloudbeers/yolo/collaborators"))
.inScenario("PR from Fork not Collaborators")
.whenScenarioStateIs(Scenario.STARTED)
.willReturn(aResponse()
.withHeader("Content-Type", "application/json; charset=utf-8")
.withBodyFile("body-cloudbeers-yolo-collaborators-none.json")));

SCMHeadObserver.Collector collector = SCMHeadObserver.collect();
source.fetch(
(SCMSourceCriteria) (probe, listener) -> probe.stat("README.md").getType() == SCMFile.Type.REGULAR_FILE,
collector,
null,
null);

Map<String, SCMHead> byName = new HashMap<>();
Map<String, SCMRevision> revByName = new HashMap<>();
for (Map.Entry<SCMHead, SCMRevision> h : collector.result().entrySet()) {
byName.put(h.getKey().getName(), h.getKey());
revByName.put(h.getKey().getName(), h.getValue());
}

assertThat(byName.keySet(), containsInAnyOrder("PR-2", "PR-3", "PR-4"));

// Source owner is not a collaborator, the trusted revision must be the base (target) revision, not the head
for (SCMRevision revision : revByName.values()) {
assertThat(
source.getTrustedRevision(revision, new LogTaskListener(Logger.getAnonymousLogger(), Level.INFO)),
is(((PullRequestSCMRevision) revision).getTarget()));
}
}

@Test
public void fetchForkPRWithPermissions() throws IOException, InterruptedException {
source.setTraits(List.of(new ForkPullRequestDiscoveryTrait(
EnumSet.of(ChangeRequestCheckoutStrategy.MERGE), new ForkPullRequestDiscoveryTrait.TrustPermission())));
githubApi.stubFor(get(urlMatching("(/api/v3)?/repos/cloudbeers/yolo/collaborators/stephenc/permission"))
.inScenario("PR from Fork with Permissions")
.whenScenarioStateIs(Scenario.STARTED)
.willReturn(aResponse()
.withHeader("Content-Type", "application/json; charset=utf-8")
.withBodyFile("body-cloudbeers-yolo-collaborators-stephenc-permission.json")));

SCMHeadObserver.Collector collector = SCMHeadObserver.collect();
source.fetch(
(SCMSourceCriteria) (probe, listener) -> probe.stat("README.md").getType() == SCMFile.Type.REGULAR_FILE,
collector,
null,
null);

Map<String, SCMHead> byName = new HashMap<>();
Map<String, SCMRevision> revByName = new HashMap<>();
for (Map.Entry<SCMHead, SCMRevision> h : collector.result().entrySet()) {
byName.put(h.getKey().getName(), h.getKey());
revByName.put(h.getKey().getName(), h.getValue());
}

assertThat(byName.keySet(), containsInAnyOrder("PR-2", "PR-3", "PR-4"));

// Source owner has sufficient permissions, the trusted revision must be the PR head / merge
for (SCMRevision revision : revByName.values()) {
assertThat(
source.getTrustedRevision(revision, new LogTaskListener(Logger.getAnonymousLogger(), Level.INFO)),
is(revision));
}
}

@Test
public void fetchForkPRCannotFindPermissions() throws IOException, InterruptedException {
source.setTraits(List.of(new ForkPullRequestDiscoveryTrait(
EnumSet.of(ChangeRequestCheckoutStrategy.MERGE), new ForkPullRequestDiscoveryTrait.TrustPermission())));
for (Integer errorCode : new int[] {
HttpServletResponse.SC_NOT_FOUND, HttpServletResponse.SC_UNAUTHORIZED, HttpServletResponse.SC_FORBIDDEN
}) {
githubApi.stubFor(get(urlMatching("(/api/v3)?/repos/cloudbeers/yolo/collaborators/stephenc/permission"))
.inScenario("PR from Fork " + errorCode + " Permissions")
.whenScenarioStateIs(Scenario.STARTED)
.willReturn(aResponse().withStatus(errorCode)));

SCMHeadObserver.Collector collector = SCMHeadObserver.collect();
source.fetch(
(SCMSourceCriteria)
(probe, listener) -> probe.stat("README.md").getType() == SCMFile.Type.REGULAR_FILE,
collector,
null,
null);

Map<String, SCMHead> byName = new HashMap<>();
Map<String, SCMRevision> revByName = new HashMap<>();
for (Map.Entry<SCMHead, SCMRevision> h : collector.result().entrySet()) {
byName.put(h.getKey().getName(), h.getKey());
revByName.put(h.getKey().getName(), h.getValue());
}

assertThat(byName.keySet(), containsInAnyOrder("PR-2", "PR-3", "PR-4"));

// Cannot find collaborators, the trusted revision must be the base (target) revision, not the head
for (SCMRevision revision : revByName.values()) {
assertThat(
source.getTrustedRevision(
revision, new LogTaskListener(Logger.getAnonymousLogger(), Level.INFO)),
is(((PullRequestSCMRevision) revision).getTarget()));
}
}
}

@Test
public void fetchForkPRInsufficientPermissions() throws IOException, InterruptedException {
source.setTraits(List.of(new ForkPullRequestDiscoveryTrait(
EnumSet.of(ChangeRequestCheckoutStrategy.MERGE), new ForkPullRequestDiscoveryTrait.TrustPermission())));
githubApi.stubFor(get(urlMatching("(/api/v3)?/repos/cloudbeers/yolo/collaborators/stephenc/permission"))
.inScenario("PR from Fork Permissions")
.whenScenarioStateIs(Scenario.STARTED)
.willReturn(aResponse()
.withHeader("Content-Type", "application/json; charset=utf-8")
.withBodyFile("body-cloudbeers-yolo-collaborators-stephenc-permission-read.json")));

SCMHeadObserver.Collector collector = SCMHeadObserver.collect();
source.fetch(
(SCMSourceCriteria) (probe, listener) -> probe.stat("README.md").getType() == SCMFile.Type.REGULAR_FILE,
collector,
null,
null);

Map<String, SCMHead> byName = new HashMap<>();
Map<String, SCMRevision> revByName = new HashMap<>();
for (Map.Entry<SCMHead, SCMRevision> h : collector.result().entrySet()) {
byName.put(h.getKey().getName(), h.getKey());
revByName.put(h.getKey().getName(), h.getValue());
}

assertThat(byName.keySet(), containsInAnyOrder("PR-2", "PR-3", "PR-4"));

// Cannot find collaborators, the trusted revision must be the base (target) revision, not the head
for (SCMRevision revision : revByName.values()) {
assertThat(
source.getTrustedRevision(revision, new LogTaskListener(Logger.getAnonymousLogger(), Level.INFO)),
is(((PullRequestSCMRevision) revision).getTarget()));
}
}

private PullRequestSCMRevision createRevision(String sourceOwner) {
PullRequestSCMHead head = new PullRequestSCMHead(
"",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[]
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"permission": "read",
"user": {
"login": "stephenc",
"id": 209336,
"node_id": "MjA5MzM2OlVzZXIyMDkzMzYK",
"avatar_url": "https://avatars.githubusercontent.com/u/209336?v=3",
"gravatar_id": "",
"url": "https://api.github.com//users/stephenc",
"html_url": "https://api.github.com/stephenc",
"followers_url": "https://api.github.com//users/stephenc/followers",
"following_url": "https://api.github.com//users/stephenc/following{/other_user}",
"gists_url": "https://api.github.com//users/stephenc/gists{/gist_id}",
"starred_url": "https://api.github.com//users/stephenc/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com//users/stephenc/subscriptions",
"organizations_url": "https://api.github.com//users/stephenc/orgs",
"repos_url": "https://api.github.com//users/stephenc/repos",
"events_url": "https://api.github.com//users/stephenc/events{/privacy}",
"received_events_url": "https://api.github.com//users/stephenc/received_events",
"type": "User",
"site_admin": false,
"permissions": {
"admin": false,
"maintain": false,
"push": false,
"triage": false,
"pull": true
},
"role_name": "read"
},
"role_name": "read"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"permission": "write",
"user": {
"login": "stephenc",
"id": 209336,
"node_id": "MjA5MzM2OlVzZXIyMDkzMzYK",
"avatar_url": "https://avatars.githubusercontent.com/u/209336?v=3",
"gravatar_id": "",
"url": "https://api.github.com//users/stephenc",
"html_url": "https://api.github.com/stephenc",
"followers_url": "https://api.github.com//users/stephenc/followers",
"following_url": "https://api.github.com//users/stephenc/following{/other_user}",
"gists_url": "https://api.github.com//users/stephenc/gists{/gist_id}",
"starred_url": "https://api.github.com//users/stephenc/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com//users/stephenc/subscriptions",
"organizations_url": "https://api.github.com//users/stephenc/orgs",
"repos_url": "https://api.github.com//users/stephenc/repos",
"events_url": "https://api.github.com//users/stephenc/events{/privacy}",
"received_events_url": "https://api.github.com//users/stephenc/received_events",
"type": "User",
"site_admin": false,
"permissions": {
"admin": false,
"maintain": false,
"push": true,
"triage": true,
"pull": true
},
"role_name": "write"
},
"role_name": "write"
}
Loading