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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e7a95d5
[JENKINS-66661] Add tests for ForkPR trust strategies
Dohbedoh Jun 28, 2023
6199783
[JENKINS-66661] Fix fallback behavior of Fork PR trust criteria
Dohbedoh Jun 28, 2023
77e6904
[JENKINS-66661] Skip PR if cannot query collaborators / permissions
Dohbedoh Jul 5, 2023
6237240
[JENKINS-66661] Fail Trust revision retrieval if cannot query collabo…
Dohbedoh Jul 5, 2023
28899a6
Merge branch 'master' into JENKINS-66661
Dohbedoh Oct 11, 2024
7a33e92
Merge branch 'master' into JENKINS-66661
Dohbedoh Nov 15, 2024
8cfe4a2
Merge branch 'master' into JENKINS-66661
Dohbedoh Mar 18, 2025
d7958b0
Merge branch 'master' into JENKINS-66661
Dohbedoh Mar 27, 2025
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
118 changes: 76 additions & 42 deletions src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1049,7 +1049,7 @@ protected final void retrieve(
request.setPermissionsSource(new GitHubPermissionsSource() {
@Override
public GHPermissionType fetch(String username) throws IOException, InterruptedException {
return ghRepository.getPermission(username);
return getPermissions(ghRepository, username, listener);
}
});

Expand Down Expand Up @@ -1270,35 +1270,44 @@ private static void retrievePullRequest(
}
}

if (request.process(
new PullRequestSCMHead(pr, branchName, strategy == ChangeRequestCheckoutStrategy.MERGE),
null,
new SCMSourceRequest.ProbeLambda<PullRequestSCMHead, Void>() {
@NonNull
@Override
public SCMSourceCriteria.Probe create(
@NonNull PullRequestSCMHead head, @Nullable Void revisionInfo)
throws IOException, InterruptedException {
boolean trusted = request.isTrusted(head);
if (!trusted) {
listener.getLogger().format(" (not from a trusted source)%n");
try {
if (request.process(
new PullRequestSCMHead(pr, branchName, strategy == ChangeRequestCheckoutStrategy.MERGE),
null,
new SCMSourceRequest.ProbeLambda<PullRequestSCMHead, Void>() {
@NonNull
@Override
public SCMSourceCriteria.Probe create(
@NonNull PullRequestSCMHead head, @Nullable Void revisionInfo)
throws IOException, InterruptedException {
boolean trusted = request.isTrusted(head);
if (!trusted) {
listener.getLogger().format(" (not from a trusted source)%n");
}
return new GitHubSCMProbe(
apiUri, credentials, ghRepository, trusted ? head : head.getTarget(), null);
}
return new GitHubSCMProbe(
apiUri, credentials, ghRepository, trusted ? head : head.getTarget(), null);
}
},
new SCMSourceRequest.LazyRevisionLambda<PullRequestSCMHead, SCMRevision, Void>() {
@NonNull
@Override
public SCMRevision create(@NonNull PullRequestSCMHead head, @Nullable Void ignored)
throws IOException, InterruptedException {
},
new SCMSourceRequest.LazyRevisionLambda<PullRequestSCMHead, SCMRevision, Void>() {
@NonNull
@Override
public SCMRevision create(@NonNull PullRequestSCMHead head, @Nullable Void ignored)
throws IOException, InterruptedException {

return createPullRequestSCMRevision(pr, head, listener, ghRepository);
}
},
new MergabilityWitness(pr, strategy, listener),
new CriteriaWitness(listener))) {
listener.getLogger().format("%n Pull request %d processed (query completed)%n", number);
return createPullRequestSCMRevision(pr, head, listener, ghRepository);
}
},
new MergabilityWitness(pr, strategy, listener),
new CriteriaWitness(listener))) {
listener.getLogger().format("%n Pull request %d processed (query completed)%n", number);
}
} catch (WrappedException e) {
try {
e.unwrap();
} catch (Exception ue) {
LOGGER.log(Level.FINE, "Failed to process pull request", ue);
}
listener.getLogger().format("%n Failed to process pull request %d, skipping%n%n", number);
}
}
}
Expand Down Expand Up @@ -1591,26 +1600,52 @@ private Set<String> updateCollaboratorNames(
@NonNull GHRepository ghRepository)
throws IOException {
if (credentials == null && (apiUri == null || GITHUB_URL.equals(apiUri))) {
// anonymous access to GitHub will never get list of collaborators and will
// burn an API call, so no point in even trying
listener.getLogger().println("Anonymous cannot query list of collaborators, assuming none");
return collaboratorNames = Collections.emptySet();
listener.getLogger().println(" Anonymous cannot query list of collaborators");
throw new IOException("Anonymous cannot query list of collaborators");
} else {
try {
return collaboratorNames = new HashSet<>(ghRepository.getCollaboratorNames());
} catch (FileNotFoundException e) {
// not permitted
listener.getLogger().println("Not permitted to query list of collaborators, assuming none");
return collaboratorNames = Collections.emptySet();
// listener.getLogger().println(" Not permitted to query list of collaborators");
listener.getLogger().format(" Not permitted to query list of collaborators: %s", e.getMessage());
throw e;
} 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();
// listener.getLogger().println(" Not permitted to query list of
// collaborators");
listener.getLogger()
.format(" Not permitted to query list of collaborators: %s", e.getMessage());
} else {
throw e;
// listener.getLogger().println(" Failed to query list of collaborators");
listener.getLogger().format(" Failed to query list of collaborators: %s", e.getMessage());
}
throw e;
}
}
}

private GHPermissionType getPermissions(
@NonNull GHRepository repo, @NonNull String username, @NonNull TaskListener listener) throws IOException {

try {
return repo.getPermission(username);
} catch (FileNotFoundException e) {
// listener.getLogger().println(" Not permitted to query list of permissions");
listener.getLogger().format(" Not permitted to query list of permissions: %s", e.getMessage());
throw new WrappedException(e);
} 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");
listener.getLogger().format(" Not permitted to query list of permissions: %s", e.getMessage());
} else {
// listener.getLogger().println(" Failed to query list of permissions");
listener.getLogger().format(" Failed to query list of permissions: %s", e.getMessage());
}
throw new WrappedException(e);
}
}

Expand Down Expand Up @@ -1883,9 +1918,8 @@ 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);
collaboratorNames = Collections.singleton(repoOwner);
listener.getLogger().format("Cannot retrieve trusted revision: %s%n");
throw e;
}
}
PullRequestSCMRevision rev = (PullRequestSCMRevision) revision;
Expand Down Expand Up @@ -2941,7 +2975,7 @@ public GHPermissionType fetch(String username) throws IOException, InterruptedEx
String fullName = repoOwner + "/" + repository;
repo = github.getRepository(fullName);
}
return repo.getPermission(username);
return getPermissions(repo, username, listener);
}

@Override
Expand Down
68 changes: 46 additions & 22 deletions src/test/java/org/jenkinsci/plugins/github_branch_source/AbstractGitHubWireMockTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@
import static com.github.tomakehurst.wiremock.client.WireMock.*;
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;

import com.cloudbees.plugins.credentials.common.StandardUsernameCredentials;
import com.github.tomakehurst.wiremock.common.FileSource;
import com.github.tomakehurst.wiremock.common.SingleRootFileSource;
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
import com.github.tomakehurst.wiremock.extension.Parameters;
import com.github.tomakehurst.wiremock.extension.ResponseTransformer;
import com.github.tomakehurst.wiremock.extension.requestfilter.RequestFilterAction;
import com.github.tomakehurst.wiremock.extension.requestfilter.StubRequestFilter;
import com.github.tomakehurst.wiremock.http.Request;
import com.github.tomakehurst.wiremock.http.Response;
import com.github.tomakehurst.wiremock.junit.WireMockRule;
Expand Down Expand Up @@ -38,30 +41,47 @@ public abstract class AbstractGitHubWireMockTest {
@Rule
public WireMockRule githubApi = factory.getRule(WireMockConfiguration.options()
.dynamicPort()
.needClientAuth(true)
.usingFilesUnderClasspath("api")
.extensions(new ResponseTransformer() {
@Override
public Response transform(Request request, Response response, FileSource files, Parameters parameters) {
if ("application/json"
.equals(response.getHeaders().getContentTypeHeader().mimeTypePart())) {
return Response.Builder.like(response)
.but()
.body(response.getBodyAsString()
.replace(
"https://api.github.com/", "http://localhost:" + githubApi.port() + "/")
.replace(
"https://raw.githubusercontent.com/",
"http://localhost:" + githubRaw.port() + "/"))
.build();
}
return response;
}
.extensions(
new ResponseTransformer() {
@Override
public Response transform(
Request request, Response response, FileSource files, Parameters parameters) {
if ("application/json"
.equals(response.getHeaders()
.getContentTypeHeader()
.mimeTypePart())) {
return Response.Builder.like(response)
.but()
.body(response.getBodyAsString()
.replace(
"https://api.github.com/",
"http://localhost:" + githubApi.port() + "/")
.replace(
"https://raw.githubusercontent.com/",
"http://localhost:" + githubRaw.port() + "/"))
.build();
}
return response;
}

@Override
public String getName() {
return "url-rewrite";
}
}));
@Override
public String getName() {
return "url-rewrite";
}
},
new StubRequestFilter() {
@Override
public RequestFilterAction filter(Request request) {
return RequestFilterAction.continueWith(request);
}

@Override
public String getName() {
return "test-auth";
}
}));

@Before
public void prepareMockGitHub() {
Expand Down Expand Up @@ -89,4 +109,8 @@ void prepareMockGitHubFileMappings() {
new SingleRootFileSource("src/test/resources/raw/mappings"),
new SingleRootFileSource("src/test/resources/raw/__files"));
}

StandardUsernameCredentials getCredentials() {
return null;
}
}
Loading